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

[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