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

["Jan Beulich" <jbeulich@xxxxxxxx>]

>>> Shuai Ruan <shuai.ruan@xxxxxxxxxxxxxxx> 03/09/16 10:49 AM >>>
>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)

There shouldn't be any combination of conditions here. Just make sure you
never set the made up feature flag. I'd also call the thing 
"using_xsave_compact"
or some such, rather than "cpu_has_...".

>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.

I see no reason why we shouldn't expose it. There's no direct connection
between us avoiding their use and a guest being allowed to make use of them.

Btw., one more thing: Can't the exclusion of FP and SSE states in the logic
determining which state set to save be extended to also include YMM? If saved,
YMM will also always live at a fixed place (an ASSERT() or BUG_ON() to verify
would of course be desirable). And if the guest didn't touch YMM registers, the
respective bit in the mask won't be set anyway.

Jan


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