Re: XSAVE flavors

["Jan Beulich" <JBeulich@suse.com>]

>>> On 04.02.16 at 07:49, <shuai.ruan@linux.intel.com> wrote:
> On Tue, Feb 02, 2016 at 02:42:38AM -0700, Jan Beulich wrote:
>> >> > With this another question then is whether, when both XSAVEC
>> >> > and XSAVEOPT are available, it is indeed always better to use
>> >> > XSAVEC (as the code is doing after your enabling).
>> > Yes.
>> > But current no machine only support xsavec not support xsaves.  
>> > I enable xsavec for "xsavec is a feature".
>> 
>> But this shouldn't preclude the code being in reasonable shape
>> also for the case where a CPU has XSAVEC but no XSAVES. The
>> more that right now we don't really need XSAVES (since we don't
>> yet allow any bit to get set in XSS).
>> 
> Actually, when I enable xsaves/xsavec, I have put xsavec into
> consideration. If xsavec is used we also need to guarntee that xcomp_bv 
> never has any bits clear which are set in xstate_bv and the compaction
> bit is set. 
> 
> Those guarntee and xsavec specific code in my patch is always behind "if( 
> cpu_has_xsavec )" 
> or " if ( cpu_has_xsaves || cpu_has_xsavec )".
> Please remind me if there is some other things I am not aware.

I'm not pointing out any correctness issue here, all I'm asking is
whether the current model is really the best one performance
wise.

>> >> And I'm afraid there's yet one more issue: If my reading of the
>> >> SDM is right, then the offsets at which components get saved
>> >> by XSAVEC / XSAVES aren't fixed, but depend on RFBM (as that's
>> >> what gets stored into xcomp_bv[62:0]). xstate_comp_offsets[],
>> >> otoh, gets computed based on all available features, irrespective
>> >> of vcpu_xsave_mask() returning four different values depending
>> >> on current guest state. I can't see how get_xsave_addr() can
>> >> work correctly without honoring xcomp_bv. Nor can I convince
>> >> myself that state can't get corrupted / lost, e.g. when a save
>> >> with v->fpu_dirtied set is followed by one with v->fpu_dirtied
>> >> clear.
>> >> 
>> >> Am I misunderstanding what the SDM writes?
>> >> 
>> > Yes. you are right. This is a issue. I will find a way to solve
>> > this.
>> 
>> Thanks.
> 
> For xstate_comp_offsets is only used in get_xsave_addr when performing 
> migration. 
> I intend to recaculte xstate_comp_offsets based on the 
> vcpu->arch.xsavec_area.save_hdr.xcomp_bv 
> before get_xsave_addr is called. 

I don't think that'll suffice, as it won't deal with the lazy XSAVE[SC]
possibly overwriting data written by the non-lazy one. See the
effectively three different values returned by vcpu_xsave_mask()
(the fourth one is impossible since the function won't ever get
called with both v->fpu_dirtied and v->arch.nonlazy_xstate_used
clear).

> The patch will be sent out after Chinese New Year holiday.

That's fine of course.

Jan