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

[Haitao Shan <maillists.shan@xxxxxxxxx>]

Yes. According to spec, while CPUID leaf 1 ECX.XSAVE reports whether
processor supports XSAVE, CPUID leaf 1 ECX.OSXSAVE just reflects
CR4.OSXSAVE, which is served as an indication from kernel (and hence
VMM, in Xen's case) to apps (including PV kernels, in xen's case) that
XSAVE is enabled and can be used safely.

Shan Haitao

2010/11/9 Ian Campbell <Ian.Campbell@xxxxxxxxxxxxx>:
> On Tue, 2010-11-09 at 10:51 +0000, Jan Beulich wrote:
>> >>> On 09.11.10 at 11:43, Ian Campbell <Ian.Campbell@xxxxxxxxxx> wrote:
>> > On Tue, 2010-11-09 at 06:22 +0000, 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
>> >>
>> >
>> >> 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
>> > */
>> >
>> > For how long has the hypervisor had this behaviour? IIRC older
>> > hypervisors did not correctly expose/mask the *XSAVE CPUID flags and
>> > would causes PV guests to crash when they used *XSAVE features which
>> > weren't actually available.
>> >
>> > In other words have you confirmed that this patch does not break the
>> > kernel running on older hypervisors such as Xen 4.0?
>>
>> The problem was only with the XSAVE cpuid bit, not the OSXSAVE
>> one (which gets turned on only when CR4.OSXSAVE gets set).
>
> So if OSXSAVE is enabled we can also infer that XSAVE is safe to use,
> because XSAVE was fixed/implemented before OSXSAVE was? Seems
> reasonable.
>
> Ian.
>
>>
>> Jan
>>
>> >> +       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 */
>> >>  }
>> >
>> >
>> > _______________________________________________
>> > Xen-devel mailing list
>> > Xen-devel@xxxxxxxxxxxxxxxxxxx
>> > http://lists.xensource.com/xen-devel
>>
>>
>>
>
>
>

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