| Message ID | 20250227010111.3222742-2-seanjc@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: x86: Advertise support for WRMSRNS | expand |
On 2/26/2025 5:01 PM, Sean Christopherson wrote: > Rename the WRMSRNS instruction opcode macro so that it doesn't collide > with X86_FEATURE_WRMSRNS when using token pasting to generate references > to X86_FEATURE_WRMSRNS. KVM heavily uses token pasting to generate KVM's > set of support feature bits, and adding WRMSRNS support in KVM will run > will run afoul of the opcode macro. > > arch/x86/kvm/cpuid.c:719:37: error: pasting "X86_FEATURE_" and "" "" does not > give a valid preprocessing token > 719 | u32 __leaf = __feature_leaf(X86_FEATURE_##name); \ > | ^~~~~~~~~~~~ > > KVM has worked around one such collision in the past by #undef'ing the > problematic macro in order to avoid blocking a KVM rework, but such games > are generally undesirable, e.g. requires bleeding macro details into KVM, > risks weird behavior if what KVM is #undef'ing changes, etc. > > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > arch/x86/include/asm/msr.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h > index 001853541f1e..60b80a36d045 100644 > --- a/arch/x86/include/asm/msr.h > +++ b/arch/x86/include/asm/msr.h > @@ -300,7 +300,7 @@ do { \ > #endif /* !CONFIG_PARAVIRT_XXL */ > > /* Instruction opcode for WRMSRNS supported in binutils >= 2.40 */ > -#define WRMSRNS _ASM_BYTES(0x0f,0x01,0xc6) > +#define ASM_WRMSRNS _ASM_BYTES(0x0f,0x01,0xc6) > > /* Non-serializing WRMSR, when available. Falls back to a serializing WRMSR. */ > static __always_inline void wrmsrns(u32 msr, u64 val) > @@ -309,7 +309,7 @@ static __always_inline void wrmsrns(u32 msr, u64 val) > * WRMSR is 2 bytes. WRMSRNS is 3 bytes. Pad WRMSR with a redundant > * DS prefix to avoid a trailing NOP. > */ > - asm volatile("1: " ALTERNATIVE("ds wrmsr", WRMSRNS, X86_FEATURE_WRMSRNS) > + asm volatile("1: " ALTERNATIVE("ds wrmsr", ASM_WRMSRNS, X86_FEATURE_WRMSRNS) > "2: " _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_WRMSR) > : : "c" (msr), "a" ((u32)val), "d" ((u32)(val >> 32))); > } I hit the same build issue, thanks for fixing it. Reviewed-by: Xin Li (Intel) <xin@zytor.com>
On 2/26/2025 5:14 PM, Xin Li wrote: > On 2/26/2025 5:01 PM, Sean Christopherson wrote: >> Rename the WRMSRNS instruction opcode macro so that it doesn't collide >> with X86_FEATURE_WRMSRNS when using token pasting to generate references >> to X86_FEATURE_WRMSRNS. KVM heavily uses token pasting to generate KVM's >> set of support feature bits, and adding WRMSRNS support in KVM will run >> will run afoul of the opcode macro. >> >> arch/x86/kvm/cpuid.c:719:37: error: pasting "X86_FEATURE_" and "" >> "" does not >> give a valid preprocessing token >> 719 | u32 __leaf = >> __feature_leaf(X86_FEATURE_##name); \ >> | ^~~~~~~~~~~~ >> >> KVM has worked around one such collision in the past by #undef'ing the >> problematic macro in order to avoid blocking a KVM rework, but such games >> are generally undesirable, e.g. requires bleeding macro details into KVM, >> risks weird behavior if what KVM is #undef'ing changes, etc. >> >> Signed-off-by: Sean Christopherson <seanjc@google.com> >> --- >> arch/x86/include/asm/msr.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h >> index 001853541f1e..60b80a36d045 100644 >> --- a/arch/x86/include/asm/msr.h >> +++ b/arch/x86/include/asm/msr.h >> @@ -300,7 +300,7 @@ do { \ >> #endif /* !CONFIG_PARAVIRT_XXL */ >> /* Instruction opcode for WRMSRNS supported in binutils >= 2.40 */ >> -#define WRMSRNS _ASM_BYTES(0x0f,0x01,0xc6) >> +#define ASM_WRMSRNS _ASM_BYTES(0x0f,0x01,0xc6) >> /* Non-serializing WRMSR, when available. Falls back to a >> serializing WRMSR. */ >> static __always_inline void wrmsrns(u32 msr, u64 val) >> @@ -309,7 +309,7 @@ static __always_inline void wrmsrns(u32 msr, u64 val) >> * WRMSR is 2 bytes. WRMSRNS is 3 bytes. Pad WRMSR with a >> redundant >> * DS prefix to avoid a trailing NOP. >> */ >> - asm volatile("1: " ALTERNATIVE("ds wrmsr", WRMSRNS, >> X86_FEATURE_WRMSRNS) >> + asm volatile("1: " ALTERNATIVE("ds wrmsr", ASM_WRMSRNS, >> X86_FEATURE_WRMSRNS) >> "2: " _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_WRMSR) >> : : "c" (msr), "a" ((u32)val), "d" ((u32)(val >> 32))); >> } > > I hit the same build issue, thanks for fixing it. > > Reviewed-by: Xin Li (Intel) <xin@zytor.com> > Do we need an ack from x86 maintainers?
diff --git a/arch/x86/include/asm/msr.h b/arch/x86/include/asm/msr.h index 001853541f1e..60b80a36d045 100644 --- a/arch/x86/include/asm/msr.h +++ b/arch/x86/include/asm/msr.h @@ -300,7 +300,7 @@ do { \ #endif /* !CONFIG_PARAVIRT_XXL */ /* Instruction opcode for WRMSRNS supported in binutils >= 2.40 */ -#define WRMSRNS _ASM_BYTES(0x0f,0x01,0xc6) +#define ASM_WRMSRNS _ASM_BYTES(0x0f,0x01,0xc6) /* Non-serializing WRMSR, when available. Falls back to a serializing WRMSR. */ static __always_inline void wrmsrns(u32 msr, u64 val) @@ -309,7 +309,7 @@ static __always_inline void wrmsrns(u32 msr, u64 val) * WRMSR is 2 bytes. WRMSRNS is 3 bytes. Pad WRMSR with a redundant * DS prefix to avoid a trailing NOP. */ - asm volatile("1: " ALTERNATIVE("ds wrmsr", WRMSRNS, X86_FEATURE_WRMSRNS) + asm volatile("1: " ALTERNATIVE("ds wrmsr", ASM_WRMSRNS, X86_FEATURE_WRMSRNS) "2: " _ASM_EXTABLE_TYPE(1b, 2b, EX_TYPE_WRMSR) : : "c" (msr), "a" ((u32)val), "d" ((u32)(val >> 32))); }
Rename the WRMSRNS instruction opcode macro so that it doesn't collide with X86_FEATURE_WRMSRNS when using token pasting to generate references to X86_FEATURE_WRMSRNS. KVM heavily uses token pasting to generate KVM's set of support feature bits, and adding WRMSRNS support in KVM will run will run afoul of the opcode macro. arch/x86/kvm/cpuid.c:719:37: error: pasting "X86_FEATURE_" and "" "" does not give a valid preprocessing token 719 | u32 __leaf = __feature_leaf(X86_FEATURE_##name); \ | ^~~~~~~~~~~~ KVM has worked around one such collision in the past by #undef'ing the problematic macro in order to avoid blocking a KVM rework, but such games are generally undesirable, e.g. requires bleeding macro details into KVM, risks weird behavior if what KVM is #undef'ing changes, etc. Signed-off-by: Sean Christopherson <seanjc@google.com> --- arch/x86/include/asm/msr.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)