Re: [PATCH 2/4] x86/xstate: Rework XSAVE/XRSTOR given a newer toolchain baseline

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/01/2026 3:16 pm, Jan Beulich wrote:
> On 02.01.2026 17:01, Andrew Cooper wrote:
>> On 30/12/2025 1:54 pm, Andrew Cooper wrote:
>>> --- a/xen/arch/x86/xstate.c
>>> +++ b/xen/arch/x86/xstate.c
>>> @@ -310,21 +310,21 @@ void xsave(struct vcpu *v, uint64_t mask)
>>>      uint32_t hmask = mask >> 32;
>>>      uint32_t lmask = mask;
>>>      unsigned int fip_width = v->domain->arch.x87_fip_width;
>>> -#define XSAVE(pfx) \
>>> -        if ( v->arch.xcr0_accum & XSTATE_XSAVES_ONLY ) \
>>> -            asm volatile ( ".byte " pfx "0x0f,0xc7,0x2f\n" /* xsaves */ \
>>> -                           : "=m" (*ptr) \
>>> -                           : "a" (lmask), "d" (hmask), "D" (ptr) ); \
>>> -        else \
>>> -            alternative_io(".byte " pfx "0x0f,0xae,0x27\n", /* xsave */ \
>>> -                           ".byte " pfx "0x0f,0xae,0x37\n", /* xsaveopt */ 
>>> \
>>> -                           X86_FEATURE_XSAVEOPT, \
>>> -                           "=m" (*ptr), \
>>> -                           "a" (lmask), "d" (hmask), "D" (ptr))
>>> +
>>> +#define XSAVE(pfx)                                                      \
>>> +    if ( v->arch.xcr0_accum & XSTATE_XSAVES_ONLY )                      \
>>> +        asm volatile ( "xsaves %0"                                      \
>>> +                       : "=m" (*ptr)                                    \
>>> +                       : "a" (lmask), "d" (hmask) );                    \
>>> +    else                                                                \
>>> +        alternative_io("xsave %0",                                      \
>>> +                       "xsaveopt %0", X86_FEATURE_XSAVEOPT,             \
>>> +                       "=m" (*ptr),                                     \
>>> +                       "a" (lmask), "d" (hmask))
>> This loses the pfx.  I've fixed up locally and double checked the
>> disassembly.
> Question being: Do we want to stick to using the prefix form, when gas
> specifically has been offering a kind-of-suffix form instead from the
> very beginning (xsaves and xsaves64)?
>
> If we wanted to stick to the prefixes, I'd favor a form where the use
> sites don't need to supply the separating blank (i.e. the macro itself
> would insert it, as doing do with an empty prefix results in merely
> an indentation "issue" in the generated assembly). Thoughts?

I don't expect this macro to survive the fixes to use the compressed
format.  From that point of view, "closest to the original" is least churn.

One problem with using a suffix form is that you could feed in "opt"
instead of "64" and break things in rather more subtle ways.

I'll adjust the position of the space, but I think this can keep on
using prefixes in the short term.

~Andrew