Re: [PATCH 2/2] x86: guard synthetic feature and bug enumerators

[Jan Beulich <jbeulich@xxxxxxxx>]

On 11.10.2025 02:30, Jason Andryuk wrote:
> On 2025-10-08 01:56, Jan Beulich wrote:
>> On 07.10.2025 21:38, Jason Andryuk wrote:
>>> On 2025-10-07 08:22, Jan Beulich wrote:
>>>> On 30.09.2025 01:36, Jason Andryuk wrote:
>>>>> On 2025-09-25 06:48, Jan Beulich wrote:
>>>>>> --- a/xen/arch/x86/include/asm/spec_ctrl.h
>>>>>> +++ b/xen/arch/x86/include/asm/spec_ctrl.h
>>>>>> @@ -73,7 +73,7 @@ static always_inline void spec_ctrl_new_
>>>>>>     
>>>>>>         /* (ab)use alternative_input() to specify clobbers. */
>>>>>>         alternative_input("", "DO_OVERWRITE_RSB xu=%=", 
>>>>>> X86_BUG_IBPB_NO_RET,
>>>>>> -                      : "rax", "rcx");
>>>>>> +                      "i" (0) : "rax", "rcx");
>>>>>
>>>>> "i" (0) is to work around the trailing comma in alternative_input() and
>>>>> does nothing?
>>>>
>>>> Yes. If more such "uses" appeared, we may want to introduce some kind of
>>>> abstraction.
>>>
>>> Thanks for confirming.
>>>
>>> Reviewed-by: Jason Andryuk <jason.andryuk@xxxxxxx>
>>
>> Thanks.
>>
>>> Though I also wondered if just #define X86_BUG_MAX/X86_SYNTH_MAX
>>> combined with a BUILD_BUG_ON might be good enough.  Your approach avoids
>>> the extra define but is more complicated.  Anyway, just a thought.
>>
>> How would that end up simplifying things? IOW what would the BUILD_BUG_ON()
>> look like that you're thinking about? After all X86_{SYNTH,BUG}_MAX aren't
>> meaningfully different from X86_NR_{SYNTH,BUG}.
> 
> Originally, I was thinking something like
>   XEN_CPUFEATURE(PDX_COMPRESSION,   X86_SYNTH(31)) /* PDX compression */
> +#define X86_SYNTH_MAX 31 /* Bump when adding flags */

I don't really like this. Especially as presented is creates an ambiguity:
Would one need to increase the value upon any flag addition, or only upon
adding a new flag with a value divisible by 32. (From the patch fragment
you appended it's clear the latter is meant, but that's not clear here,
and when one learns to routinely ignore the comment, there's a risk of also
ignoring it when it shouldn't be ignored. Whereas if the bump was required
every time a new flag was added, I would dislike the unnecessary churn. In
the end - yes, there's a reason why I did things the way done, even if
there are other aspects to it which are (for the patch itself, but imo not
once it would have gone in) not overly nice.

Jan