Re: [Xen-devel] [PATCH V4 2/4] x86/xsaves: enable xsaves/xrstors in xen

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.08.15 at 12:12, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 25/08/15 11:54, Shuai Ruan wrote:
>> --- a/xen/arch/x86/hvm/hvm.c
>> +++ b/xen/arch/x86/hvm/hvm.c
>> @@ -2148,8 +2148,12 @@ static int hvm_save_cpu_xsave_states(struct domain 
>> *d, hvm_domain_context_t *h)
>>          ctxt->xfeature_mask = xfeature_mask;
>>          ctxt->xcr0 = v->arch.xcr0;
>>          ctxt->xcr0_accum = v->arch.xcr0_accum;
>> -        memcpy(&ctxt->save_area, v->arch.xsave_area,
>> -               size - offsetof(struct hvm_hw_cpu_xsave, save_area));
>> +    if ( cpu_has_xsaves )
> 
> Stray hard tab.
> 
>> +            save_xsave_states(v, (void *)&ctxt->save_area,
>> +                              size - offsetof(struct 
>> hvm_hw_cpu_xsave,save_area));
> 
> These offsetof()s can become far shorter by using offsetof(*ctxt,
> save_area).

Are you mixing this up with sizeof()? If anything, offsetof(typeof(),).

>> --- a/xen/arch/x86/traps.c
>> +++ b/xen/arch/x86/traps.c
>> @@ -936,9 +936,10 @@ void pv_cpuid(struct cpu_user_regs *regs)
>>          if ( regs->_ecx == 1 )
>>          {
>>              a &= XSTATE_FEATURE_XSAVEOPT |
>> -                 XSTATE_FEATURE_XSAVEC |
>> -                 (cpu_has_xgetbv1 ? XSTATE_FEATURE_XGETBV1 : 0) |
>> -                 (cpu_has_xsaves ? XSTATE_FEATURE_XSAVES : 0);
>> +                 XSTATE_FEATURE_XSAVEC;
>> +             /* PV guest will not support xsaves. */
>> +                 /* (cpu_has_xgetbv1 ? XSTATE_FEATURE_XGETBV1 : 0) |
>> +                 (cpu_has_xsaves ? XSTATE_FEATURE_XSAVES : 0); */
> 
> Don't leave this code commented out like this.  Just delete it.

Agreed, but - mind reminding me again why supporting them for
PV guests isn't going to work?

Jan


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