Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Message-ID: <d9bf0049963df4d1e3e03290c28acb95d833f782.camel@redhat.com>
Subject: Re: [PATCH v2] x86: KVM: Add feature flag for
 CPUID.80000021H:EAX[bit 1]
From:   Maxim Levitsky <mlevitsk@redhat.com>
To:     Jim Mattson <jmattson@google.com>, kvm@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Pawan Gupta <pawan.kumar.gupta@linux.intel.com>,
        Jiaxi Chen <jiaxi.chen@linux.intel.com>,
        Kim Phillips <kim.phillips@amd.com>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Sean Christopherson <seanjc@google.com>,
        "H. Peter Anvin" <hpa@zytor.com>, x86@kernel.org,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Borislav Petkov <bp@alien8.de>, Ingo Molnar <mingo@redhat.com>,
        Thomas Gleixner <tglx@linutronix.de>
Date:   Thu, 05 Oct 2023 19:45:00 +0300
In-Reply-To: <20231005031237.1652871-1-jmattson@google.com>
References: <20231005031237.1652871-1-jmattson@google.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.5 (3.36.5-2.fc32) 
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

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