> -----Original Message----- > From: Alexander Graf [mailto:agraf@xxxxxxx] > Sent: Friday, July 18, 2014 4:25 PM > To: Bhushan Bharat-R65777; Wood Scott-B07421 > Cc: kvm-ppc@xxxxxxxxxxxxxxx; kvm@xxxxxxxxxxxxxxx; Yoder Stuart-B08248 > Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest > > > On 18.07.14 11:57, Bharat.Bhushan@xxxxxxxxxxxxx wrote: > > > >> -----Original Message----- > >> From: Wood Scott-B07421 > >> Sent: Friday, July 18, 2014 6:19 AM > >> To: Alexander Graf > >> Cc: Bhushan Bharat-R65777; kvm-ppc@xxxxxxxxxxxxxxx; > >> kvm@xxxxxxxxxxxxxxx; Yoder > >> Stuart-B08248 > >> Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering > >> guest > >> > >> On Fri, 2014-07-18 at 02:37 +0200, Alexander Graf wrote: > >>> On 18.07.14 02:36, Scott Wood wrote: > >>>> On Fri, 2014-07-18 at 02:33 +0200, Alexander Graf wrote: > >>>>> On 18.07.14 02:28, Scott Wood wrote: > >>>>>> On Thu, 2014-07-17 at 18:29 +0200, Alexander Graf wrote: > >>>>>>> On 17.07.14 18:27, Alexander Graf wrote: > >>>>>>>> On 17.07.14 18:24, Bharat.Bhushan@xxxxxxxxxxxxx wrote: > >>>>>>>>>> -----Original Message----- > >>>>>>>>>> From: Alexander Graf [mailto:agraf@xxxxxxx] > >>>>>>>>>> Sent: Thursday, July 17, 2014 9:41 PM > >>>>>>>>>> To: Bhushan Bharat-R65777; kvm-ppc@xxxxxxxxxxxxxxx > >>>>>>>>>> Cc: kvm@xxxxxxxxxxxxxxx; Wood Scott-B07421; Yoder > >>>>>>>>>> Stuart-B08248 > >>>>>>>>>> Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when > >>>>>>>>>> entering guest > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> On 16.07.14 08:02, Bharat Bhushan wrote: > >>>>>>>>>>> SPRG3 is guest accessible and SPRG3 can be clobbered by host > >>>>>>>>>>> or another guest, So this need to be restored when loading > >>>>>>>>>>> guest > >> state. > >>>>>> SPRG3 is not guest writeable. We should be doing this so that > >>>>>> guest reads of SPRG3 through the alternative read-only SPR work, > >>>>>> not because > >>>>>> "SPRG3 can be clobbered by host or another guest". > >>>>>> > >>>>>>>>>>> Signed-off-by: Bharat Bhushan <Bharat.Bhushan@xxxxxxxxxxxxx> > >>>>>>>>>>> --- > >>>>>>>>>>> arch/powerpc/kvm/booke_interrupts.S | 2 ++ > >>>>>>>>>>> 1 file changed, 2 insertions(+) > >>>>>>>>>>> > >>>>>>>>>>> diff --git a/arch/powerpc/kvm/booke_interrupts.S > >>>>>>>>>>> b/arch/powerpc/kvm/booke_interrupts.S > >>>>>>>>>>> index 2c6deb5ef..0d3403f 100644 > >>>>>>>>>>> --- a/arch/powerpc/kvm/booke_interrupts.S > >>>>>>>>>>> +++ b/arch/powerpc/kvm/booke_interrupts.S > >>>>>>>>>>> @@ -459,6 +459,8 @@ lightweight_exit: > >>>>>>>>>>> * written directly to the shared area, so we > >>>>>>>>>>> * need to reload them here with the guest's values. > >>>>>>>>>>> */ > >>>>>>>>>>> + PPC_LD(r3, VCPU_SHARED_SPRG3, r5) > >>>>>>>>>>> + mtspr SPRN_SPRG3, r3 > >>>>>>>>>> We also need to restore it when resuming the host, no? > >>>>>>>>> I do not think host expect some meaningful value when > >>>>>>>>> returning from guest, same true for SPRG4-7. > >>>>>>>>> So there seems no reason to save host values and restore them. > >>>>>> Linux no longer uses SPRG4-7 for itself. That is not true of > >>>>>> SPRG3, as Alex points out. > >>>>>> > >>>>>>>> Hmm - arch/powerpc/include/asm/reg.h says: > >>>>>>>> > >>>>>>>> * All 32-bit: > >>>>>>>> * - SPRG3 current thread_info pointer > >>>>>>>> * (virtual on BookE, physical on others) > >>>>>>>> > >>>>>>>> but I can indeed find no trace of usage anywhere. This at least > >>>>>>>> needs to go into the patch description. > >>>>>>> Bah - it obviously is used. It's SPRN_SPRG_THREAD. And it's so > >>>>>>> incredibly important that I have no idea how we could possibly > >>>>>>> run without switching the host value back in very early. And > >>>>>>> even then our interrupt handlers wouldn't work anymore. > >>>>>>> > >>>>>>> This is more complicated :). > >>>>>> To make this work we need to avoid SPRG3 as well, or at least > >>>>>> avoid using it for something needed prior to DO_KVM. > >>>>>> > >>>>>> We also need to update the documentation in reg.h to reflect the > >>>>>> fact that we don't use SPRG4-7 anymore on e500. > >>>>> I would personally prefer if we claim SPRG3R as unsupported on > >>>>> e500v2 until we find someone who actually uses it. There's a good > >>>>> chance we'd start jumping through a lot of hoops and reduce > >>>>> overall performance for no real-world gain today. > >>>> The same problem applies to e500mc. > > We have two SPRG3 registers > > 1) SPRN_SPRG3 -- All read/write access to this register in guest > will trap and emulate, So no need to save/restore. > > 2) SPRN_SPRG3R -- This is guest read only and We do not see any user > of this register, so can leave this for now > > > >>> There we have SPRN_GSPRG3, no? > >> Oh, right. > >> > >> Since it's only a problem for PR-mode, it can be fixed without > >> needing to avoid > >> SPRG3 entirely, since PR-mode doesn't use DO_KVM. We'd only need to > >> avoid using SPRG_THREAD in __KVM_HANDLER (i.e. revert commit > >> ffe129ecd79779221fdb03305049ec8b5a8beb0f). > > Did not get why using SPRG_THREAD here is a problem? Is not this will > > always access host SPRG3 and guest read write will always trap > > (maintained in vcpu->arch->shared->reg->sprg3) > > Guest reads via SPRG3R will access the real SPRG3 register. Guest reads via > SPRG3 will trap :). Agree, Also we do not see Linux as guest is accessing SPRG3R. But what I did not get what the mentioned patch have to do with that? > > > Alex ��.n��������+%������w��{.n�����o�^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
