Re: [Xen-devel] [Patch] Allowing PV-OPS kernel to detect whether XSAVE is supported

[Haitao Shan <maillists.shan@xxxxxxxxx>]

2010/11/10 Jeremy Fitzhardinge <jeremy@xxxxxxxx>:
> On 11/08/2010 10:22 PM, Haitao Shan wrote:
>> Hi, Jeremy,
>>
>> This patch allows pv-ops kernel to detect whether XSAVE is supported
>> (before masking it unconditionally through xen_cpuid).
>> Can you please have review? Thanks!
>>
>> Signed-off-by: Shan Haitao <haitao.shan@xxxxxxxxx>
>>
>> Shan Haitao
>>
>
> For future reference:
>
> Please post patches inline if possible.
>
> If you must use an attachment to prevent your mail system from
> corrupting the patch, please include a complete description of the patch
> (what is it trying to do, how does it do it, what is the outcome?) with
> signed-off-bys in the the patch itself.
OK. I will follow.

>
>> diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c index
>> fd3803e..03bfaf7 100644 --- a/arch/x86/xen/enlighten.c +++
>> b/arch/x86/xen/enlighten.c @@ -252,6 +252,13 @@ static __init void
>> xen_init_cpuid_mask(void) (1 << X86_FEATURE_MCA) | /* disable MCA */
>> (1 << X86_FEATURE_APIC) | /* disable local APIC */ (1 <<
>> X86_FEATURE_ACPI)); /* disable ACPI */ + ax = 1; + xen_cpuid(&ax, &bx,
>> &cx, &dx); + + /* Xen will set CR4.OSXSAVE if supported and not
>> disabled by force */ + if ( cx & (1 << (X86_FEATURE_XSAVE % 32)) && +
>> cx & (1 << (X86_FEATURE_OSXSAVE % 32)) ) + return;
>> cpuid_leaf1_ecx_mask &= ~(1 << (X86_FEATURE_XSAVE % 32)); /* disable
>> XSAVE */
>
>
> I cleaned this up a bit (fixed formatting to Linux style, reversed the
> sense of the if() and masked OSXSAVE as well, even though it won't make
> a difference).  But I'm still a bit concerned about the back-compat
> issues around this.
>
> What happens if the guest OS does not support XSAVE (older versions of
> Linux)?  Could usermode code see OSXSAVE set in the cpuid feature flags
> and attempt to use XSAVE, even if XSAVE isn't set?  In other words, can
> usermode ever normally see OSXSAVE set, but XSAVE clear?
I think that is not possible. If usermode does not use native CPUID
instruction, or CPUID can be virtualized any way, this won't happen.
Otherwise, usermode will either see both are set or unset. Since on a
NON-XSAVE processor, you can not set CR4.OSXSAVE and won't be
reflected to CPUID.

If user mode sees both are set, they can use it actually. So we
initially set FP and SSE in XCR0 for guest. If user mode wants to use
it, guest kernel still can manage the state using traditional FPU
save/restore mechanism. If user mode wants to access more extended
states, it has to acquire kernel's support for turning on related bit
in XCR0, which is finally controlled by Xen now.

That's the reason I chose to turn on OSXSAVE in Xen and don't
dynamically change it.

>
> We can't mask OSXSAVE to usermode because they can run the naked CPUID
> instruction, so I guess the only way to cause it to be cleared would be
> to clear CR4.OSXSAVE?  But that seems like a very expensive thing to do
> on a vcpu context switch.
Yes. This is the biggest concern when I wrote XSAVE patches.
Otherwise, you will need to switch CR4 on entry to/exit from guest
mode. Or as you said, switching CR4 when do a vcpu context switch, but
whenever Xen itself want to use XSAVE instructions, it needs to turns
on it and off it on the fly.

>
> How much testing of real usermode code have you done with this?  How
> many combinations of XSAVE support in Xen, Linux and usermode have you
> tried?
I have a few AVX test programs running both inside PV guest and HVM.
However, I have to admit that my aim is to test Xen's fpu context
switching using Xsave and guest save/restore.

>
>    J
>

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel