[v2] x86: KVM: Add feature flag for CPUID.80000021H:EAX[bit 1]

Message ID 20231005031237.1652871-1-jmattson@google.com
State New
Headers
Series [v2] x86: KVM: Add feature flag for CPUID.80000021H:EAX[bit 1] |

Commit Message

Jim Mattson Oct. 5, 2023, 3:12 a.m. UTC
  Define an X86_FEATURE_* flag for CPUID.80000021H:EAX.[bit 1], and
advertise the feature to userspace via KVM_GET_SUPPORTED_CPUID.

Per AMD's "Processor Programming Reference (PPR) for AMD Family 19h
Model 61h, Revision B1 Processors (56713-B1-PUB)," this CPUID bit
indicates that a WRMSR to MSR_FS_BASE, MSR_GS_BASE, or
MSR_KERNEL_GS_BASE is non-serializing. This is a change in previously
architected behavior.

Effectively, this CPUID bit is a "defeature" bit, or a reverse
polarity feature bit. When this CPUID bit is clear, the feature
(serialization on WRMSR to any of these three MSRs) is available. When
this CPUID bit is set, the feature is not available.

KVM_GET_SUPPORTED_CPUID must pass this bit through from the underlying
hardware, if it is set. Leaving the bit clear claims that WRMSR to
these three MSRs will be serializing in a guest running under
KVM. That isn't true. Though KVM could emulate the feature by
intercepting writes to the specified MSRs, it does not do so
today. The guest is allowed direct read/write access to these MSRs
without interception, so the innate hardware behavior is preserved
under KVM.

Signed-off-by: Jim Mattson <jmattson@google.com>
---

v1 -> v2: Added justification for this change to the commit message,
          tweaked the macro name and comment in cpufeatures.h for
	  improved clarity.

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

Comments

Borislav Petkov Oct. 5, 2023, 3:50 p.m. UTC | #1
On Wed, Oct 04, 2023 at 08:12:37PM -0700, Jim Mattson wrote:
> Define an X86_FEATURE_* flag for CPUID.80000021H:EAX.[bit 1], and
> advertise the feature to userspace via KVM_GET_SUPPORTED_CPUID.
> 
> Per AMD's "Processor Programming Reference (PPR) for AMD Family 19h
> Model 61h, Revision B1 Processors (56713-B1-PUB)," this CPUID bit
> indicates that a WRMSR to MSR_FS_BASE, MSR_GS_BASE, or
> MSR_KERNEL_GS_BASE is non-serializing. This is a change in previously
> architected behavior.
> 
> Effectively, this CPUID bit is a "defeature" bit, or a reverse
> polarity feature bit. When this CPUID bit is clear, the feature
> (serialization on WRMSR to any of these three MSRs) is available. When
> this CPUID bit is set, the feature is not available.
> 
> KVM_GET_SUPPORTED_CPUID must pass this bit through from the underlying
> hardware, if it is set. Leaving the bit clear claims that WRMSR to
> these three MSRs will be serializing in a guest running under
> KVM. That isn't true. Though KVM could emulate the feature by
> intercepting writes to the specified MSRs, it does not do so
> today. The guest is allowed direct read/write access to these MSRs
> without interception, so the innate hardware behavior is preserved
> under KVM.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
> 
> v1 -> v2: Added justification for this change to the commit message,
>           tweaked the macro name and comment in cpufeatures.h for
> 	  improved clarity.
> 
>  arch/x86/include/asm/cpufeatures.h | 1 +
>  arch/x86/kvm/cpuid.c               | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Borislav Petkov (AMD) <bp@alien8.de>
  
Maxim Levitsky Oct. 5, 2023, 4:45 p.m. UTC | #2
У ср, 2023-10-04 у 20:12 -0700, Jim Mattson пише:
> Define an X86_FEATURE_* flag for CPUID.80000021H:EAX.[bit 1], and
> advertise the feature to userspace via KVM_GET_SUPPORTED_CPUID.
> 
> Per AMD's "Processor Programming Reference (PPR) for AMD Family 19h
> Model 61h, Revision B1 Processors (56713-B1-PUB)," this CPUID bit
> indicates that a WRMSR to MSR_FS_BASE, MSR_GS_BASE, or
> MSR_KERNEL_GS_BASE is non-serializing. This is a change in previously
> architected behavior.
> 
> Effectively, this CPUID bit is a "defeature" bit, or a reverse
> polarity feature bit. When this CPUID bit is clear, the feature
> (serialization on WRMSR to any of these three MSRs) is available. When
> this CPUID bit is set, the feature is not available.
> 
> KVM_GET_SUPPORTED_CPUID must pass this bit through from the underlying
> hardware, if it is set. Leaving the bit clear claims that WRMSR to
> these three MSRs will be serializing in a guest running under
> KVM. That isn't true. Though KVM could emulate the feature by
> intercepting writes to the specified MSRs, it does not do so
> today. The guest is allowed direct read/write access to these MSRs
> without interception, so the innate hardware behavior is preserved
> under KVM.
> 
> Signed-off-by: Jim Mattson <jmattson@google.com>
> ---
> 
> v1 -> v2: Added justification for this change to the commit message,
>           tweaked the macro name and comment in cpufeatures.h for
> 	  improved clarity.
> 
>  arch/x86/include/asm/cpufeatures.h | 1 +
>  arch/x86/kvm/cpuid.c               | 3 ++-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
> index 58cb9495e40f..4af140cf5719 100644
> --- a/arch/x86/include/asm/cpufeatures.h
> +++ b/arch/x86/include/asm/cpufeatures.h
> @@ -443,6 +443,7 @@
>  
>  /* AMD-defined Extended Feature 2 EAX, CPUID level 0x80000021 (EAX), word 20 */
>  #define X86_FEATURE_NO_NESTED_DATA_BP	(20*32+ 0) /* "" No Nested Data Breakpoints */
> +#define X86_FEATURE_WRMSR_XX_BASE_NS	(20*32+ 1) /* "" WRMSR to {FS,GS,KERNEL_GS}_BASE is non-serializing */
>  #define X86_FEATURE_LFENCE_RDTSC	(20*32+ 2) /* "" LFENCE always serializing / synchronizes RDTSC */
>  #define X86_FEATURE_NULL_SEL_CLR_BASE	(20*32+ 6) /* "" Null Selector Clears Base */
>  #define X86_FEATURE_AUTOIBRS		(20*32+ 8) /* "" Automatic IBRS */
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index 0544e30b4946..93241b33e36f 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -761,7 +761,8 @@ void kvm_set_cpu_caps(void)
>  
>  	kvm_cpu_cap_mask(CPUID_8000_0021_EAX,
>  		F(NO_NESTED_DATA_BP) | F(LFENCE_RDTSC) | 0 /* SmmPgCfgLock */ |
> -		F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | 0 /* PrefetchCtlMsr */
> +		F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | 0 /* PrefetchCtlMsr */ |
> +		F(WRMSR_XX_BASE_NS)
>  	);
>  
>  	if (cpu_feature_enabled(X86_FEATURE_SRSO_NO))


Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>

Best regards,
	Maxim Levitsky
  
Sean Christopherson Oct. 20, 2023, 10:56 p.m. UTC | #3
On Wed, 04 Oct 2023 20:12:37 -0700, Jim Mattson wrote:
> Define an X86_FEATURE_* flag for CPUID.80000021H:EAX.[bit 1], and
> advertise the feature to userspace via KVM_GET_SUPPORTED_CPUID.
> 
> Per AMD's "Processor Programming Reference (PPR) for AMD Family 19h
> Model 61h, Revision B1 Processors (56713-B1-PUB)," this CPUID bit
> indicates that a WRMSR to MSR_FS_BASE, MSR_GS_BASE, or
> MSR_KERNEL_GS_BASE is non-serializing. This is a change in previously
> architected behavior.
> 
> [...]

Applied to kvm-x86 misc, thanks!

[1/1] x86: KVM: Add feature flag for CPUID.80000021H:EAX[bit 1]
      https://github.com/kvm-x86/linux/commit/329369caeccb

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

Patch

diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h
index 58cb9495e40f..4af140cf5719 100644
--- a/arch/x86/include/asm/cpufeatures.h
+++ b/arch/x86/include/asm/cpufeatures.h
@@ -443,6 +443,7 @@ 
 
 /* AMD-defined Extended Feature 2 EAX, CPUID level 0x80000021 (EAX), word 20 */
 #define X86_FEATURE_NO_NESTED_DATA_BP	(20*32+ 0) /* "" No Nested Data Breakpoints */
+#define X86_FEATURE_WRMSR_XX_BASE_NS	(20*32+ 1) /* "" WRMSR to {FS,GS,KERNEL_GS}_BASE is non-serializing */
 #define X86_FEATURE_LFENCE_RDTSC	(20*32+ 2) /* "" LFENCE always serializing / synchronizes RDTSC */
 #define X86_FEATURE_NULL_SEL_CLR_BASE	(20*32+ 6) /* "" Null Selector Clears Base */
 #define X86_FEATURE_AUTOIBRS		(20*32+ 8) /* "" Automatic IBRS */
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index 0544e30b4946..93241b33e36f 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -761,7 +761,8 @@  void kvm_set_cpu_caps(void)
 
 	kvm_cpu_cap_mask(CPUID_8000_0021_EAX,
 		F(NO_NESTED_DATA_BP) | F(LFENCE_RDTSC) | 0 /* SmmPgCfgLock */ |
-		F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | 0 /* PrefetchCtlMsr */
+		F(NULL_SEL_CLR_BASE) | F(AUTOIBRS) | 0 /* PrefetchCtlMsr */ |
+		F(WRMSR_XX_BASE_NS)
 	);
 
 	if (cpu_feature_enabled(X86_FEATURE_SRSO_NO))