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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
