Re: [Xen-devel] [V3] x86/xsaves: fix overwriting between non-lazy/lazy xsaves

[Shuai Ruan <shuai.ruan@xxxxxxxxxxxxxxx>]

On Tue, Mar 08, 2016 at 03:16:29AM -0700, Jan Beulich wrote:
> >>> On 08.03.16 at 08:19, <shuai.ruan@xxxxxxxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/domctl.c
> > +++ b/xen/arch/x86/domctl.c
> > @@ -922,7 +922,7 @@ long arch_do_domctl(
> >                  ret = -EFAULT;
> >  
> >              offset += sizeof(v->arch.xcr0_accum);
> > -            if ( !ret && (cpu_has_xsaves || cpu_has_xsavec) )
> > +            if ( !ret && cpu_has_xsaves )
> 
> ... here (and similarly elsewhere) you shouldn't make the code
> continue to depend on the raw CPU feature, but you should have
> a variable (or could be a #define for now) indicating whether we're
> actively using compressed state areas. For the purpose of
> alternative patching, the most suitable thing likely would be a
> synthetic CPU feature.
> 
what I think to do here is 
Add 
#define X86_FEATURE_XSAVE_COMPACT (3*32 +17) /* use compacted xsave area */
and
#define cpu_has_xsave_compact   (boot_cpu_has(X86_FEATURE_XSAVE_COMPACT) \
                                 && boot_cpu_has(X86_FEATURE_XSAVES)

Then using cpu_has_xsave_compact instead of cpu_has_xsaves.
> In no case do I see any reason to artificially make cpu_has_* yield
> false despite the hardware actually having that feature. Doing so
> would only risk making future changes more cumbersome.
> 
Yeah, i will revert the change.

I have another thing needs your opintions.

Should we expose xsave[sc] to guest os (hvm guest)? (for nowdays ,linux guest 
can use
xsaves, I think we should expose xsave[sc] to hvm guest). It means xen can only 
use 
xsaves when cpu_has_xsave_compact is true, and hvm guest can use xsave[sc] if 
cpu_has_xsaves/cpu_has_xsavec is true.

If we do not expose xsaves to guest os, We  need change almost every
cpu_has_xsaves into cpu_has_xsave_compact.

> Please carefully go through this comment and fix all typos,
> typographical issues, and ideally also grammar. And there also is at
> least one apparent factual issue: "The reason XSTATE_FP_SSE
> should be excluded ..." seems wrong to me - I think you mean "may"
> instead of "should", because this is an optimization aspect, not a
> requirement of any sort.
> 
Ok. Please excuse my poor english.
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxx
> http://lists.xen.org/xen-devel

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel