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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.01.2026 17:55, Andrew Cooper wrote:
> 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.

Except that there's no XSAVESOPT nor XSAVEOPTOPT.

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

Okay, I wanted the alternative to at least be considered.

Jan