[PATCH 0/2] KVM: x86: Advertise support for WRMSRNS

[PATCH 0/2] KVM: x86: Advertise support for WRMSRNS

[Sean Christopherson]

Advertise support for WRMSRNS, which should be trivial, but is mildly
annoying due to a token pasting collision between the instruction macro
and KVM's CPUID feature bit shenanigans.

Sean Christopherson (2):
  x86/msr: Rename the WRMSRNS opcode macro to ASM_WRMSRNS (for KVM)
  KVM: x86: Advertise support for WRMSRNS

 arch/x86/include/asm/msr.h | 4 ++--
 arch/x86/kvm/cpuid.c       | 1 +
 2 files changed, 3 insertions(+), 2 deletions(-)


base-commit: fed48e2967f402f561d80075a20c5c9e16866e53
-- 
2.48.1.711.g2feabab25a-goog

[PATCH 1/2] x86/msr: Rename the WRMSRNS opcode macro to ASM_WRMSRNS (for KVM)

[Sean Christopherson]

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)));
 }
-- 
2.48.1.711.g2feabab25a-goog

Re: [PATCH 1/2] x86/msr: Rename the WRMSRNS opcode macro to ASM_WRMSRNS (for KVM)

[Xin Li]

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>

Re: [PATCH 1/2] x86/msr: Rename the WRMSRNS opcode macro to ASM_WRMSRNS (for KVM)

[Xin Li]

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?

[PATCH 2/2] KVM: x86: Advertise support for WRMSRNS

[Sean Christopherson]

Advertise support for WRMSRNS (WRMSR non-serializing) to userspace if the
instruction is supported by the underlying CPU.  From a virtualization
perspective, the only difference between WRMSRNS and WRMSR is that VM-Exits
due to WRMSRNS set EXIT_QUALIFICATION to '1'.  WRMSRNS doesn't require a
new enabling control, shares the same basic exit reason, and behaves the
same as WRMSR with respect to MSR interception.

  WRMSR and WRMSRNS use the same basic exit reason (see Appendix C). For
  WRMSR, the exit qualification is 0, while for WRMSRNS it is 1.

Don't do anything different when emulating WRMSRNS vs. WRMSR, as KVM can't
do anything less, i.e. can't make emulation non-serializing.  The
motivation for the guest to use WRMSRNS instead of WRMSR is to avoid
immediately serializing the CPU when the necessary serialization is
guaranteed by some other mechanism, i.e. WRMSRNS being fully serializing
isn't guest-visible, just less performant.

Suggested-by: Xin Li (Intel) <xin@zytor.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/cpuid.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 97a90689a9dc..ebecfe4bea1e 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -992,6 +992,7 @@ void kvm_set_cpu_caps(void)
 		F(FZRM),
 		F(FSRS),
 		F(FSRC),
+		F(WRMSRNS),
 		F(AMX_FP16),
 		F(AVX_IFMA),
 		F(LAM),
-- 
2.48.1.711.g2feabab25a-goog

Re: [PATCH 0/2] KVM: x86: Advertise support for WRMSRNS

[Sean Christopherson]

On Wed, 26 Feb 2025 17:01:09 -0800, Sean Christopherson wrote:
> Advertise support for WRMSRNS, which should be trivial, but is mildly
> annoying due to a token pasting collision between the instruction macro
> and KVM's CPUID feature bit shenanigans.
> 
> Sean Christopherson (2):
>   x86/msr: Rename the WRMSRNS opcode macro to ASM_WRMSRNS (for KVM)
>   KVM: x86: Advertise support for WRMSRNS
> 
> [...]

Applied to kvm-x86 misc, thanks!

[1/2] x86/msr: Rename the WRMSRNS opcode macro to ASM_WRMSRNS (for KVM)
      commit: 3fa0fc95db6df904dc812fa806a55ea6bafa65c1
[2/2] KVM: x86: Advertise support for WRMSRNS
      commit: ead4dac16de22081956f7af1c795bfbebd2d5866

--
https://github.com/kvm-x86/linux/tree/next