Re: [Xen-devel] [PATCH v2 2/2] x86/xstate: also use alternative asm on xsave side

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 03.02.16 at 14:26, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 03/02/16 12:39, Jan Beulich wrote:
>> --- a/xen/arch/x86/xstate.c
>> +++ b/xen/arch/x86/xstate.c
>> @@ -250,27 +250,29 @@ void xsave(struct vcpu *v, uint64_t mask
>>      uint32_t hmask = mask >> 32;
>>      uint32_t lmask = mask;
>>      int word_size = mask & XSTATE_FP ? (cpu_has_fpu_sel ? 8 : 0) : -1;
>> +#define XSAVE(pfx) \
>> +        alternative_io_3(".byte " pfx "0x0f,0xae,0x27\n", \
>> +                         ".byte " pfx "0x0f,0xae,0x37\n", \
>> +                         X86_FEATURE_XSAVEOPT, \
>> +                         ".byte " pfx "0x0f,0xc7,0x27\n", \
>> +                         X86_FEATURE_XSAVEC, \
>> +                         ".byte " pfx "0x0f,0xc7,0x2f\n", \
>> +                         X86_FEATURE_XSAVES, \
> 
> Given that the options are a little out of order and using raw bytes,
> would you mind annotating the lines with the operations. e.g.
> 
> +        alternative_io_3(".byte " pfx "0x0f,0xae,0x27\n", /* xsave */ \
> +                         ".byte " pfx "0x0f,0xae,0x37\n", /* xsaveopt */ \
> +                         X86_FEATURE_XSAVEOPT, \
> +                         ".byte " pfx "0x0f,0xc7,0x27\n", /* xsavec */ \
> +                         X86_FEATURE_XSAVEC, \
> +                         ".byte " pfx "0x0f,0xc7,0x2f\n", /* xsaves */ \
> +                         X86_FEATURE_XSAVES, \
> 
> IMO, this is somewhat clearer to read.

Okay, since I had been considering this too, I've just done so.

> Otherwise,
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Thanks, Jan


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