On 03.07.2013, at 19:17, Scott Wood wrote: > 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.
Yep. Once I understood that point things became clear to me :). > >> >> 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... Would be nice to fix nevertheless. I'm probably not going to be the last person forgetting how this works. Alex -- To unsubscribe from this list: send the line "unsubscribe kvm-ppc" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
