From mboxrd@z Thu Jan 1 00:00:00 1970 From: Scott Wood Subject: Re: [PATCH 3/6] KVM: PPC: Book3E: Increase FPU laziness Date: Wed, 3 Jul 2013 12:17:34 -0500 Message-ID: <1372871854.8183.132@snotra> References: <23C56B31-5145-481E-9877-F1878F66959D@suse.de> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; delsp=Yes; format=Flowed Content-Transfer-Encoding: 8BIT Cc: Caraman Mihai Claudiu-B02008 , "kvm-ppc@vger.kernel.org" , "kvm@vger.kernel.org" , "linuxppc-dev@lists.ozlabs.org" To: Alexander Graf Return-path: Received: from co9ehsobe002.messaging.microsoft.com ([207.46.163.25]:15953 "EHLO co9outboundpool.messaging.microsoft.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755696Ab3GCRRl convert rfc822-to-8bit (ORCPT ); Wed, 3 Jul 2013 13:17:41 -0400 In-Reply-To: <23C56B31-5145-481E-9877-F1878F66959D@suse.de> (from agraf@suse.de on Wed Jul 3 11:59:45 2013) Content-Disposition: inline Sender: kvm-owner@vger.kernel.org List-ID: On 07/03/2013 11:59:45 AM, Alexander Graf wrote: > > On 03.07.2013, at 17:41, Caraman Mihai Claudiu-B02008 wrote: > > >>>>> Increase FPU laziness by calling kvmppc_load_guest_fp() just > before > >>>>> returning to guest instead of each sched in. Without this > improvement > >>>>> an interrupt may also claim floting point corrupting guest > state. > >>>> > >>>> Not sure I follow. Could you please describe exactly what's > happening? > >>> > >>> This was already discussed on the list, I will forward you the > thread. > >> > >> The only thing I've seen in that thread was some pathetic > theoretical > >> case where an interrupt handler would enable fp and clobber state > >> carelessly. That's not something I'm worried about. > > > > Neither me though I don't find it pathetic. Please refer it to > Scott. > > If from Linux's point of view we look like a user space program with > active floating point registers, we don't have to worry about this > case. Kernel code that would clobber that fp state would clobber > random user space's fp state too. This patch makes it closer to how it works with a user space program. Or rather, it reduces the time window when we don't (and can't) act like a normal userspace program -- and ensures that we have interrupts disabled during that window. An interrupt can't randomly clobber FP state; it has to call enable_kernel_fp() just like KVM does. enable_kernel_fp() clears the userspace MSR_FP to ensure that the state it saves gets restored before userspace uses it again, but that won't have any effect on guest execution (especially in HV-mode). Thus kvmppc_load_guest_fp() needs to be atomic with guest entry. Conceptually it's like taking an automatic FP unavailable trap when we enter the guest, since we can't be lazy in HV-mode. > >> I really don't see where this patch improves anything tbh. It > certainly > >> makes the code flow more awkward. > > > > I was pointing you to this: The idea of FPU/AltiVec laziness that > the kernel > > is struggling to achieve is to reduce the number of store/restore > operations. > > Without this improvement we restore the unit each time we are sched > it. If an > > other process take the ownership of the unit (on SMP it's even > worse but don't > > bother with this) the kernel store the unit state to qemu task. > This can happen > > multiple times during handle_exit(). > > > > Do you see it now? > > Yup. Looks good. The code flow is very hard to follow though - there > are a lot of implicit assumptions that don't get documented anywhere. > For example the fact that we rely on giveup_fpu() to remove MSR_FP > from our thread. That's not new to this patch... -Scott