While adding new enumerators one may overlook the (rare) need to bump
X86_NR_{SYNTH,BUG}. Guard against that happening by adding respective
checking. The use of BUILD_BUG_ON_ZERO(), however, entails a number of
other changes, as the expansion may not appear in the assembly produced.
Furthermore inputs to file-scope asm() are only supported in gcc15 (or
newer).
No difference in generated code (debug info, however, grows quite a bit).
An implication from the changes is that users of the alternatives patching
macros may not use unnamed asm() input operands anymore, as the "injected"
new operands would break numbering expectations.
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
v2: Fix shim build. Use named label operand in pdx.h.
--- a/xen/arch/x86/include/asm/alternative.h
+++ b/xen/arch/x86/include/asm/alternative.h
@@ -70,12 +70,12 @@ extern void alternative_instructions(voi
alt_repl_len(n2)) "-" alt_orig_len)
#define ALTINSTR_ENTRY(feature, num) \
- " .if (" STR(feature & ~ALT_FLAG_NOT) ") >= " STR(NCAPINTS * 32) "\n" \
+ " .if (%c" #feature " & " STR(~ALT_FLAG_NOT) ") >= " STR(NCAPINTS * 32) "\n" \
" .error \"alternative feature outside of featureset range\"\n" \
" .endif\n" \
" .long .LXEN%=_orig_s - .\n" /* label */ \
" .long " alt_repl_s(num)" - .\n" /* new instruction */ \
- " .word " STR(feature) "\n" /* feature bit */ \
+ " .word %c" #feature "\n" /* feature bit */ \
" .byte " alt_orig_len "\n" /* source len */ \
" .byte " alt_repl_len(num) "\n" /* replacement len */ \
" .byte " alt_pad_len "\n" /* padding len */ \
@@ -127,14 +127,14 @@ extern void alternative_instructions(voi
*/
#define alternative(oldinstr, newinstr, feature) \
asm_inline volatile ( \
- ALTERNATIVE(oldinstr, newinstr, feature) \
- ::: "memory" )
+ ALTERNATIVE(oldinstr, newinstr, [feat]) \
+ :: [feat] "i" (feature) : "memory" )
#define alternative_2(oldinstr, newinstr1, feature1, newinstr2, feature2) \
asm_inline volatile ( \
- ALTERNATIVE_2(oldinstr, newinstr1, feature1, \
- newinstr2, feature2) \
- ::: "memory" )
+ ALTERNATIVE_2(oldinstr, newinstr1, [feat1], \
+ newinstr2, [feat2]) \
+ :: [feat1] "i" (feature1), [feat2] "i" (feature2) : "memory" )
/*
* Alternative inline assembly with input.
@@ -148,14 +148,14 @@ extern void alternative_instructions(voi
*/
#define alternative_input(oldinstr, newinstr, feature, input...) \
asm_inline volatile ( \
- ALTERNATIVE(oldinstr, newinstr, feature) \
- :: input )
+ ALTERNATIVE(oldinstr, newinstr, [feat]) \
+ :: [feat] "i" (feature), ## input )
/* Like alternative_input, but with a single output argument */
#define alternative_io(oldinstr, newinstr, feature, output, input...) \
asm_inline volatile ( \
- ALTERNATIVE(oldinstr, newinstr, feature) \
- : output : input )
+ ALTERNATIVE(oldinstr, newinstr, [feat]) \
+ : output : [feat] "i" (feature), ## input )
/*
* This is similar to alternative_io. But it has two features and
@@ -168,9 +168,9 @@ extern void alternative_instructions(voi
#define alternative_io_2(oldinstr, newinstr1, feature1, newinstr2, \
feature2, output, input...) \
asm_inline volatile ( \
- ALTERNATIVE_2(oldinstr, newinstr1, feature1, \
- newinstr2, feature2) \
- : output : input )
+ ALTERNATIVE_2(oldinstr, newinstr1, [feat1], \
+ newinstr2, [feat2]) \
+ : output : [feat1] "i" (feature1), [feat2] "i" (feature2), ## input )
/* Use this macro(s) if you need more than one output parameter. */
#define ASM_OUTPUT2(a...) a
--- a/xen/arch/x86/include/asm/cpufeatures.h
+++ b/xen/arch/x86/include/asm/cpufeatures.h
@@ -6,9 +6,16 @@
/* Number of capability words covered by the featureset words. */
#define FSCAPINTS FEATURESET_NR_ENTRIES
+#if !defined(__ASSEMBLER__) && __GNUC__ >= 15
+#include <xen/macros.h>
+#define X86_CHECK_FEAT_NR(x, n) BUILD_BUG_ON_ZERO((x) / 32 >= X86_NR_ ## n)
+#else
+#define X86_CHECK_FEAT_NR(x, n) 0
+#endif
+
/* Synthetic words follow the featureset words. */
#define X86_NR_SYNTH 2
-#define X86_SYNTH(x) (FSCAPINTS * 32 + (x))
+#define X86_SYNTH(x) (FSCAPINTS * 32 + (x) + X86_CHECK_FEAT_NR(x, SYNTH))
/* Synthetic features */
XEN_CPUFEATURE(CONSTANT_TSC, X86_SYNTH( 0)) /* TSC ticks at a constant rate */
@@ -47,7 +54,8 @@ XEN_CPUFEATURE(XEN_REP_MOVSB, X86_SY
/* Bug words follow the synthetic words. */
#define X86_NR_BUG 1
-#define X86_BUG(x) ((FSCAPINTS + X86_NR_SYNTH) * 32 + (x))
+#define X86_BUG(x) ((FSCAPINTS + X86_NR_SYNTH) * 32 + (x) + \
+ X86_CHECK_FEAT_NR(x, BUG))
#define X86_BUG_FPU_PTRS X86_BUG( 0) /* (F)X{SAVE,RSTOR} doesn't save/restore FOP/FIP/FDP. */
#define X86_BUG_NULL_SEG X86_BUG( 1) /* NULL-ing a selector preserves the base and limit. */
--- a/xen/arch/x86/include/asm/cpufeatureset.h
+++ b/xen/arch/x86/include/asm/cpufeatureset.h
@@ -12,8 +12,13 @@ enum {
};
#undef XEN_CPUFEATURE
+#if __GNUC__ >= 15
+#define XEN_CPUFEATURE(name, value) asm (".equ X86_FEATURE_" #name ", %c0" \
+ :: "i" (X86_FEATURE_##name));
+#else
#define XEN_CPUFEATURE(name, value) asm (".equ X86_FEATURE_" #name ", " \
__stringify(value));
+#endif
#include <public/arch-x86/cpufeatureset.h>
#include <asm/cpufeatures.h>
--- a/xen/arch/x86/include/asm/guest/xen-hcall.h
+++ b/xen/arch/x86/include/asm/guest/xen-hcall.h
@@ -31,11 +31,13 @@
long res, tmp__; \
asm volatile ( \
ALTERNATIVE_2("call early_hypercall", \
- "vmmcall", ALT_NOT(X86_FEATURE_USE_VMCALL), \
- "vmcall", X86_FEATURE_USE_VMCALL) \
+ "vmmcall", [vmmcall], \
+ "vmcall", [vmcall]) \
: "=a" (res), "=D" (tmp__) ASM_CALL_CONSTRAINT \
: "0" (hcall), \
- "1" ((long)(a1)) \
+ "1" ((long)(a1)), \
+ [vmmcall] "i" (ALT_NOT(X86_FEATURE_USE_VMCALL)), \
+ [vmcall] "i" (X86_FEATURE_USE_VMCALL) \
: "memory" ); \
(type)res; \
})
@@ -45,12 +47,14 @@
long res, tmp__; \
asm volatile ( \
ALTERNATIVE_2("call early_hypercall", \
- "vmmcall", ALT_NOT(X86_FEATURE_USE_VMCALL), \
- "vmcall", X86_FEATURE_USE_VMCALL) \
+ "vmmcall", [vmmcall], \
+ "vmcall", [vmcall]) \
: "=a" (res), "=D" (tmp__), "=S" (tmp__) \
ASM_CALL_CONSTRAINT \
: "0" (hcall), \
- "1" ((long)(a1)), "2" ((long)(a2)) \
+ "1" ((long)(a1)), "2" ((long)(a2)), \
+ [vmmcall] "i" (ALT_NOT(X86_FEATURE_USE_VMCALL)), \
+ [vmcall] "i" (X86_FEATURE_USE_VMCALL) \
: "memory" ); \
(type)res; \
})
@@ -60,12 +64,14 @@
long res, tmp__; \
asm volatile ( \
ALTERNATIVE_2("call early_hypercall", \
- "vmmcall", ALT_NOT(X86_FEATURE_USE_VMCALL), \
- "vmcall", X86_FEATURE_USE_VMCALL) \
+ "vmmcall", [vmmcall], \
+ "vmcall", [vmcall]) \
: "=a" (res), "=D" (tmp__), "=S" (tmp__), "=d" (tmp__) \
ASM_CALL_CONSTRAINT \
: "0" (hcall), \
- "1" ((long)(a1)), "2" ((long)(a2)), "3" ((long)(a3)) \
+ "1" ((long)(a1)), "2" ((long)(a2)), "3" ((long)(a3)), \
+ [vmmcall] "i" (ALT_NOT(X86_FEATURE_USE_VMCALL)), \
+ [vmcall] "i" (X86_FEATURE_USE_VMCALL) \
: "memory" ); \
(type)res; \
})
@@ -76,13 +82,15 @@
register long _a4 asm ("r10") = ((long)(a4)); \
asm volatile ( \
ALTERNATIVE_2("call early_hypercall", \
- "vmmcall", ALT_NOT(X86_FEATURE_USE_VMCALL), \
- "vmcall", X86_FEATURE_USE_VMCALL) \
+ "vmmcall", [vmmcall], \
+ "vmcall", [vmcall]) \
: "=a" (res), "=D" (tmp__), "=S" (tmp__), "=d" (tmp__), \
"=&r" (tmp__) ASM_CALL_CONSTRAINT \
: "0" (hcall), \
"1" ((long)(a1)), "2" ((long)(a2)), "3" ((long)(a3)), \
- "4" (_a4) \
+ "4" (_a4), \
+ [vmmcall] "i" (ALT_NOT(X86_FEATURE_USE_VMCALL)), \
+ [vmcall] "i" (X86_FEATURE_USE_VMCALL) \
: "memory" ); \
(type)res; \
})
--- a/xen/arch/x86/include/asm/pdx.h
+++ b/xen/arch/x86/include/asm/pdx.h
@@ -13,9 +13,9 @@
asm_inline goto ( \
ALTERNATIVE( \
"", \
- "jmp %l0", \
- ALT_NOT(X86_FEATURE_PDX_COMPRESSION)) \
- : : : : label )
+ "jmp %l[" #label "]", \
+ [feat]) \
+ : : [feat] "i" (ALT_NOT(X86_FEATURE_PDX_COMPRESSION)) : : label )
static inline unsigned long pfn_to_pdx(unsigned long pfn)
{
--- 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");
}
extern int8_t opt_ibpb_ctxt_switch;
@@ -163,7 +163,7 @@ static always_inline void __spec_ctrl_en
* (ab)use alternative_input() to specify clobbers.
*/
alternative_input("", "DO_OVERWRITE_RSB xu=%=", X86_FEATURE_SC_RSB_IDLE,
- : "rax", "rcx");
+ "i" (0) : "rax", "rcx");
}
/* WARNING! `ret`, `call *`, `jmp *` not safe after this call. */
--- a/xen/arch/x86/include/asm/tsc.h
+++ b/xen/arch/x86/include/asm/tsc.h
@@ -29,8 +29,7 @@ static inline uint64_t rdtsc_ordered(voi
alternative_io_2("lfence; rdtsc",
"mfence; rdtsc", X86_FEATURE_MFENCE_RDTSC,
"rdtscp", X86_FEATURE_RDTSCP,
- ASM_OUTPUT2("=a" (low), "=d" (high), "=c" (aux)),
- /* no inputs */);
+ ASM_OUTPUT2("=a" (low), "=d" (high), "=c" (aux)));
return (high << 32) | low;
}On 07/01/2026 2:11 pm, Jan Beulich wrote:
> While adding new enumerators one may overlook the (rare) need to bump
> X86_NR_{SYNTH,BUG}. Guard against that happening by adding respective
> checking. The use of BUILD_BUG_ON_ZERO(), however, entails a number of
> other changes, as the expansion may not appear in the assembly produced.
> Furthermore inputs to file-scope asm() are only supported in gcc15 (or
> newer).
>
> No difference in generated code (debug info, however, grows quite a bit).
>
> An implication from the changes is that users of the alternatives patching
> macros may not use unnamed asm() input operands anymore, as the "injected"
> new operands would break numbering expectations.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> v2: Fix shim build. Use named label operand in pdx.h.
I'm pretty sure that will be rejected by Eclair as a Rule R20.12
violation (using a parameter as a regular value, and stringised), and is
a blocking rule.
But more generally... I see why you want a guard rail here, I can't
help feeling that the cure is worse than the poison.
Updating every alternative is very invasive, and this in particular
> --- 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");
> }
>
without even an explanation of why, is an immediate red flag.
Could we not split X86_SYNTH()/BUG() to take a leaf/bit pair, similar to
how we express regular features as a*32+b?
That would at least make it more obvious than currently when a new leaf
is needed, and contained it to a single header.
~Andrew
On 08.01.2026 19:12, Andrew Cooper wrote:
> On 07/01/2026 2:11 pm, Jan Beulich wrote:
>> While adding new enumerators one may overlook the (rare) need to bump
>> X86_NR_{SYNTH,BUG}. Guard against that happening by adding respective
>> checking. The use of BUILD_BUG_ON_ZERO(), however, entails a number of
>> other changes, as the expansion may not appear in the assembly produced.
>> Furthermore inputs to file-scope asm() are only supported in gcc15 (or
>> newer).
>>
>> No difference in generated code (debug info, however, grows quite a bit).
>>
>> An implication from the changes is that users of the alternatives patching
>> macros may not use unnamed asm() input operands anymore, as the "injected"
>> new operands would break numbering expectations.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> v2: Fix shim build. Use named label operand in pdx.h.
>
> I'm pretty sure that will be rejected by Eclair as a Rule R20.12
> violation (using a parameter as a regular value, and stringised), and is
> a blocking rule.
I don't think so - see the difference between the non-compliant and the
compliant examples in the spec. The label passed here isn't subject to
further macro expansion.
> But more generally... I see why you want a guard rail here, I can't
> help feeling that the cure is worse than the poison.
That's a fair position to take, albeit I don't really agree.
> Updating every alternative is very invasive, and this in particular
Well, luckily it's far from all, but mainly those which use the upper-
case macros directly. Plus the two "(ab)uses" in asm/spec_ctrl.h. The
change in asm/tsc.h is actually in our favor, I would say.
>> --- 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");
>> }
>
> without even an explanation of why, is an immediate red flag.
Honestly, to me the "(ab)use" in the comment is enough of an explanation.
Plus, really, the end result now looks more "normal" than before (in no
longer having comma and colon next to each other).
Would adding /* dummy */ after the seemingly stray input satisfy your
request for "an explanation"? Else what exactly would you expect?
> Could we not split X86_SYNTH()/BUG() to take a leaf/bit pair, similar to
> how we express regular features as a*32+b?
>
> That would at least make it more obvious than currently when a new leaf
> is needed, and contained it to a single header.
I'm pretty sure we could, but such a split would be largely artificial.
Hence why I discarded that option very early, the more that - as you say -
it still would only serve as a hint, without enforcing anything. In
particular I could easily see me using the next "major" index, but still
forgetting that X86_NR_{SYNTH,BUG} would also need bumping. (What might
help a little is if the two really moved to the end of their blocks, so
they would be more likely to be spotted when adding something to the end.
Bottom line: I'd prefer if we would stick to actually doing the checking
(or yet better derive X86_NR_{SYNTH,BUG} from the uses of
X86_{SYNTH,BUG}() [1]). I'm not particularly happy with the way the checking
is done right now, so I'm all ears towards improvement suggestions.
Jan
[1] Some initial idea: Have
XEN_CPUFEATURE(SYNTH_1ST_UNUSED, X86_SYNTH(...)) /* Must stay last. */
#define X86_NR_SYNTH ((X86_FEATURE_SYNTH_1ST_UNUSED - 1) / 32 - FSCAPINTS)
On 2026-01-07 09:11, Jan Beulich wrote:
> While adding new enumerators one may overlook the (rare) need to bump
> X86_NR_{SYNTH,BUG}. Guard against that happening by adding respective
> checking. The use of BUILD_BUG_ON_ZERO(), however, entails a number of
> other changes, as the expansion may not appear in the assembly produced.
> Furthermore inputs to file-scope asm() are only supported in gcc15 (or
> newer).
>
> No difference in generated code (debug info, however, grows quite a bit).
>
> An implication from the changes is that users of the alternatives patching
> macros may not use unnamed asm() input operands anymore, as the "injected"
> new operands would break numbering expectations.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Jason Andryuk <jason.andryuk@amd.com>
© 2016 - 2026 Red Hat, Inc.