From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Date: Wed, 05 Jun 2013 20:59:42 +0000 Subject: Re: [RFC PATCH 6/6] KVM: PPC: Book3E: Enhance FPU laziness Message-Id: <1370465982.26139.11@snotra> List-Id: References: <300B73AA675FCE4A93EB4FC1D42459FF44F178@039-SN2MPN1-011.039d.mgd.msft.net> In-Reply-To: <300B73AA675FCE4A93EB4FC1D42459FF44F178@039-SN2MPN1-011.039d.mgd.msft.net> (from B02008@freescale.com on Wed Jun 5 04:14:21 2013) MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Caraman Mihai Claudiu-B02008 Cc: Wood Scott-B07421 , "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" , Alexander Graf On 06/05/2013 04:14:21 AM, Caraman Mihai Claudiu-B02008 wrote: > > -----Original Message----- > > From: Wood Scott-B07421 > > Sent: Wednesday, June 05, 2013 1:54 AM > > To: Caraman Mihai Claudiu-B02008 > > Cc: kvm-ppc@vger.kernel.org; kvm@vger.kernel.org; linuxppc- > > dev@lists.ozlabs.org; Caraman Mihai Claudiu-B02008 > > Subject: Re: [RFC PATCH 6/6] KVM: PPC: Book3E: Enhance FPU laziness > > > > On 06/03/2013 03:54:28 PM, Mihai Caraman wrote: > > > Adopt AltiVec approach to increase laziness by calling > > > kvmppc_load_guest_fp() > > > just before returning to guest instaed of each sched in. > > > > > > Signed-off-by: Mihai Caraman > > > > If you did this *before* adding Altivec it would have saved a > question > > in an earlier patch. :-) > > I kept asking myself about the order and in the end I decided that > this is > an improvement originated from AltiVec work. FPU may be further > cleaned up > (get rid of active state, etc). > > > > > > --- > > > arch/powerpc/kvm/booke.c | 1 + > > > arch/powerpc/kvm/e500mc.c | 2 -- > > > 2 files changed, 1 insertions(+), 2 deletions(-) > > > > > > diff --git a/arch/powerpc/kvm/booke.c b/arch/powerpc/kvm/booke.c > > > index 019496d..5382238 100644 > > > --- a/arch/powerpc/kvm/booke.c > > > +++ b/arch/powerpc/kvm/booke.c > > > @@ -1258,6 +1258,7 @@ int kvmppc_handle_exit(struct kvm_run *run, > > > struct kvm_vcpu *vcpu, > > > } else { > > > kvmppc_lazy_ee_enable(); > > > kvmppc_load_guest_altivec(vcpu); > > > + kvmppc_load_guest_fp(vcpu); > > > } > > > } > > > > > > > You should probably do these before kvmppc_lazy_ee_enable(). > > Why? I wanted to look like part of lightweight_exit. We want to minimize the portion of the code that runs with interrupts disabled while telling tracers that interrupts are enabled. We want to minimize the C code run with lazy EE in an inconsistent state. The same applies to kvm_vcpu_run()... > > Actually, I don't think this is a good idea at all. As I understand > > it, you're not supposed to take kernel ownersship of floating point > in > > non-atomic context, because an interrupt could itself call > > enable_kernel_fp(). > > So lightweight_exit isn't executed in atomic context? Ignore this, I misread what the patch was doing. I thought you were doing the opposite you did. :-P As such, this patch appears to fix the thing I was complaining about -- before, we could have taken an interrupt after kvmppc_core_vcpu_load(), and that interrupt could have claimed the floating point (unlikely with the kernel as is, but you never know what could happen in the future or out-of-tree...). > Will be lazyee fixes including kvmppc_fix_ee_before_entry() in 3.10? > 64-bit Book3E KVM is unreliable without them. Should we disable e5500 > too > for 3.10? I hope so... I meant to ask Gleb to take them while Alex was away, but I forgot about them. :-P Alex, are you back from vacation yet? -Scott