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

[Maxim Levitsky]

У ср, 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@xxxxxxxxxx>
> ---
> 
> 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@xxxxxxxxxx>

Best regards,
	Maxim Levitsky