From mboxrd@z Thu Jan 1 00:00:00 1970 From: Alexander Graf Subject: Re: [PATCH] kvm: ppc: booke: Restore SPRG3 when entering guest Date: Fri, 18 Jul 2014 12:55:16 +0200 Message-ID: <53C8FD14.1080907@suse.de> References: <1405490564-20119-1-git-send-email-Bharat.Bhushan@freescale.com> <53C7F593.2010804@suse.de> <53C7F981.9080405@suse.de> <53C7F9F7.30800@suse.de> <1405643339.7714.46.camel@snotra.buserror.net> <53C86B48.6090402@suse.de> <1405643769.7714.47.camel@snotra.buserror.net> <53C86C39.7020609@suse.de> <1405644545.7714.52.camel@snotra.buserror.net> <2e4d6b2a3fb744cc9c39179054710cff@BLUPR03MB566.namprd03.prod.outlook.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Cc: "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org" , Stuart Yoder To: "Bharat.Bhushan@freescale.com" , Scott Wood Return-path: In-Reply-To: <2e4d6b2a3fb744cc9c39179054710cff@BLUPR03MB566.namprd03.prod.outlook.com> Sender: kvm-ppc-owner@vger.kernel.org List-Id: kvm.vger.kernel.org On 18.07.14 11:57, Bharat.Bhushan@freescale.com wrote: > >> -----Original Message----- >> From: Wood Scott-B07421 >> Sent: Friday, July 18, 2014 6:19 AM >> To: Alexander Graf >> Cc: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; 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@freescale.com wrote: >>>>>>>>>> -----Original Message----- >>>>>>>>>> From: Alexander Graf [mailto:agraf@suse.de] >>>>>>>>>> Sent: Thursday, July 17, 2014 9:41 PM >>>>>>>>>> To: Bhushan Bharat-R65777; kvm-ppc@vger.kernel.org >>>>>>>>>> Cc: kvm@vger.kernel.org; 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 >>>>>>>>>>> --- >>>>>>>>>>> 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 :). Alex