diff mbox series

[v3,8/9] KVM: x86: Expose Architectural LBR CPUID leaf

Message ID 20210303135756.1546253-9-like.xu@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series KVM: x86/pmu: Guest Architectural LBR Enabling | expand

Commit Message

Like Xu March 3, 2021, 1:57 p.m. UTC 
If CPUID.(EAX=07H, ECX=0):EDX[19] is set to 1, then KVM supports Arch
LBRs and CPUID leaf 01CH indicates details of the Arch LBRs capabilities.
Currently, KVM only supports the current host LBR depth for guests,
which is also the maximum supported depth on the host.

Signed-off-by: Like Xu <like.xu@linux.intel.com>
---
 arch/x86/kvm/cpuid.c   | 25 ++++++++++++++++++++++++-
 arch/x86/kvm/vmx/vmx.c |  2 ++
 2 files changed, 26 insertions(+), 1 deletion(-)

Comments

Sean Christopherson March 3, 2021, 5:34 p.m. UTC | #1 
On Wed, Mar 03, 2021, Like Xu wrote:
> If CPUID.(EAX=07H, ECX=0):EDX[19] is set to 1, then KVM supports Arch
> LBRs and CPUID leaf 01CH indicates details of the Arch LBRs capabilities.
> Currently, KVM only supports the current host LBR depth for guests,
> which is also the maximum supported depth on the host.
> 
> Signed-off-by: Like Xu <like.xu@linux.intel.com>
> ---
>  arch/x86/kvm/cpuid.c   | 25 ++++++++++++++++++++++++-
>  arch/x86/kvm/vmx/vmx.c |  2 ++
>  2 files changed, 26 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> index b4247f821277..4473324fe7be 100644
> --- a/arch/x86/kvm/cpuid.c
> +++ b/arch/x86/kvm/cpuid.c
> @@ -450,7 +450,7 @@ void kvm_set_cpu_caps(void)
>  		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
>  		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP) |
>  		F(MD_CLEAR) | F(AVX512_VP2INTERSECT) | F(FSRM) |
> -		F(SERIALIZE) | F(TSXLDTRK) | F(AVX512_FP16)
> +		F(SERIALIZE) | F(TSXLDTRK) | F(AVX512_FP16) | F(ARCH_LBR)
>  	);
>  
>  	/* TSC_ADJUST and ARCH_CAPABILITIES are emulated in software. */

...

> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 2f307689a14b..034708a3df20 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -7258,6 +7258,8 @@ static __init void vmx_set_cpu_caps(void)
>  		kvm_cpu_cap_clear(X86_FEATURE_INVPCID);
>  	if (vmx_pt_mode_is_host_guest())
>  		kvm_cpu_cap_check_and_set(X86_FEATURE_INTEL_PT);
> +	if (cpu_has_vmx_arch_lbr())
> +		kvm_cpu_cap_check_and_set(X86_FEATURE_ARCH_LBR);

Using kvm_cpu_cap_check_and_set(), which queries boot_cpu_has(), is only
necessary if a feature is not exposed by default in kvm_set_cpu_caps().  That's
why INTEL_PT uses it.  ARCH_LBR on the other hand is set in the "enable by
default" mask.

That being said, it's probably a bad idea to advertise ARCH_LBR by default.  In
the unlikely case that AMD adds support for ARCH_LBR, enable-by-default means
guest will be able to use ARCH_LBR on old KVMs that presumably would lack support
for ARCH_LBR on SVM.

TL;DR: omit F(ARCH_LBR) or replace it with "0 /* ARCH_LBR */".

>  	if (vmx_umip_emulated())
>  		kvm_cpu_cap_set(X86_FEATURE_UMIP);
> -- 
> 2.29.2
>
Sean Christopherson March 3, 2021, 6:01 p.m. UTC | #2 
On Wed, Mar 03, 2021, Sean Christopherson wrote:
> On Wed, Mar 03, 2021, Like Xu wrote:
> > If CPUID.(EAX=07H, ECX=0):EDX[19] is set to 1, then KVM supports Arch
> > LBRs and CPUID leaf 01CH indicates details of the Arch LBRs capabilities.
> > Currently, KVM only supports the current host LBR depth for guests,
> > which is also the maximum supported depth on the host.
> > 
> > Signed-off-by: Like Xu <like.xu@linux.intel.com>
> > ---
> >  arch/x86/kvm/cpuid.c   | 25 ++++++++++++++++++++++++-
> >  arch/x86/kvm/vmx/vmx.c |  2 ++
> >  2 files changed, 26 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
> > index b4247f821277..4473324fe7be 100644
> > --- a/arch/x86/kvm/cpuid.c
> > +++ b/arch/x86/kvm/cpuid.c
> > @@ -450,7 +450,7 @@ void kvm_set_cpu_caps(void)
> >  		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
> >  		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP) |
> >  		F(MD_CLEAR) | F(AVX512_VP2INTERSECT) | F(FSRM) |
> > -		F(SERIALIZE) | F(TSXLDTRK) | F(AVX512_FP16)
> > +		F(SERIALIZE) | F(TSXLDTRK) | F(AVX512_FP16) | F(ARCH_LBR)
> >  	);
> >  
> >  	/* TSC_ADJUST and ARCH_CAPABILITIES are emulated in software. */
> 
> ...
> 
> > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> > index 2f307689a14b..034708a3df20 100644
> > --- a/arch/x86/kvm/vmx/vmx.c
> > +++ b/arch/x86/kvm/vmx/vmx.c
> > @@ -7258,6 +7258,8 @@ static __init void vmx_set_cpu_caps(void)
> >  		kvm_cpu_cap_clear(X86_FEATURE_INVPCID);
> >  	if (vmx_pt_mode_is_host_guest())
> >  		kvm_cpu_cap_check_and_set(X86_FEATURE_INTEL_PT);
> > +	if (cpu_has_vmx_arch_lbr())
> > +		kvm_cpu_cap_check_and_set(X86_FEATURE_ARCH_LBR);
> 
> Using kvm_cpu_cap_check_and_set(), which queries boot_cpu_has(), is only
> necessary if a feature is not exposed by default in kvm_set_cpu_caps().  That's
> why INTEL_PT uses it.  ARCH_LBR on the other hand is set in the "enable by
> default" mask.
> 
> That being said, it's probably a bad idea to advertise ARCH_LBR by default.  In
> the unlikely case that AMD adds support for ARCH_LBR, enable-by-default means
> guest will be able to use ARCH_LBR on old KVMs that presumably would lack support
> for ARCH_LBR on SVM.
> 
> TL;DR: omit F(ARCH_LBR) or replace it with "0 /* ARCH_LBR */".

Actually, I take that back.  It'll require changing SVM, but due to the XSS
interaction it's probably cleaner to leaf F(ARCH_LBR) as is, and do:

	if (!cpu_has_vmx_arch_lbr())
		kvm_cpu_cap_clear(X86_FEATURE_ARCH_LBR);

and then unconditionally clear the cap for SVM.  In a way, that's arguably
better documentation as it explicitly shows that SVM lacks supports.

More thoughts in the next patch...
diff mbox series

Patch

diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c
index b4247f821277..4473324fe7be 100644
--- a/arch/x86/kvm/cpuid.c
+++ b/arch/x86/kvm/cpuid.c
@@ -450,7 +450,7 @@  void kvm_set_cpu_caps(void)
 		F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) |
 		F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP) |
 		F(MD_CLEAR) | F(AVX512_VP2INTERSECT) | F(FSRM) |
-		F(SERIALIZE) | F(TSXLDTRK) | F(AVX512_FP16)
+		F(SERIALIZE) | F(TSXLDTRK) | F(AVX512_FP16) | F(ARCH_LBR)
 	);
 
 	/* TSC_ADJUST and ARCH_CAPABILITIES are emulated in software. */
@@ -805,6 +805,29 @@  static inline int __do_cpuid_func(struct kvm_cpuid_array *array, u32 function)
 				goto out;
 		}
 		break;
+	/* Architectural LBR */
+	case 0x1c:
+	{
+		u64 lbr_depth_mask = entry->eax & 0xff;
+
+		if (!lbr_depth_mask || !kvm_cpu_cap_has(X86_FEATURE_ARCH_LBR)) {
+			entry->eax = entry->ebx = entry->ecx = entry->edx = 0;
+			break;
+		}
+
+		/*
+		 * KVM only exposes the maximum supported depth,
+		 * which is also the fixed value used on the host.
+		 *
+		 * KVM doesn't allow VMM user sapce to adjust depth
+		 * per guest, because the guest LBR emulation depends
+		 * on the implementation of the host LBR driver.
+		 */
+		lbr_depth_mask = 1UL << (fls(lbr_depth_mask) - 1);
+		entry->eax &= ~0xff;
+		entry->eax |= lbr_depth_mask;
+		break;
+	}
 	case KVM_CPUID_SIGNATURE: {
 		static const char signature[12] = "KVMKVMKVM\0\0";
 		const u32 *sigptr = (const u32 *)signature;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 2f307689a14b..034708a3df20 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -7258,6 +7258,8 @@  static __init void vmx_set_cpu_caps(void)
 		kvm_cpu_cap_clear(X86_FEATURE_INVPCID);
 	if (vmx_pt_mode_is_host_guest())
 		kvm_cpu_cap_check_and_set(X86_FEATURE_INTEL_PT);
+	if (cpu_has_vmx_arch_lbr())
+		kvm_cpu_cap_check_and_set(X86_FEATURE_ARCH_LBR);
 
 	if (vmx_umip_emulated())
 		kvm_cpu_cap_set(X86_FEATURE_UMIP);