linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] KVM: VMX: Add helper+macros to do sec exec adjustment
@ 2020-09-23 16:50 Sean Christopherson
  2020-09-23 16:50 ` [PATCH 1/4] KVM: VMX: Rename vmx_*_supported() helpers to cpu_has_vmx_*() Sean Christopherson
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Sean Christopherson @ 2020-09-23 16:50 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

Add a helper function and macro wrappers to consolidate code for adjusting
secondary execution controls based on guest CPUID.  The adjustments are
effectively 10+ lines of copy+paste for each control, with slight tweaks
to account for annoying differences, e.g. XSAVES has additional checks.

Patches 1-3 are prep work to make INVPCID and RDTSCP align with the
"standard" nomenclature so that they don't require special casing.

Sean Christopherson (4):
  KVM: VMX: Rename vmx_*_supported() helpers to cpu_has_vmx_*()
  KVM: VMX: Unconditionally clear CPUID.INVPCID if !CPUID.PCID
  KVM: VMX: Rename RDTSCP secondary exec control name to insert "ENABLE"
  KVM: VMX: Add a helper and macros to reduce boilerplate for sec exec
    ctls

 arch/x86/include/asm/vmx.h                    |   2 +-
 arch/x86/kvm/vmx/capabilities.h               |  10 +-
 arch/x86/kvm/vmx/nested.c                     |   4 +-
 arch/x86/kvm/vmx/vmx.c                        | 150 +++++++-----------
 .../selftests/kvm/include/x86_64/vmx.h        |   2 +-
 5 files changed, 64 insertions(+), 104 deletions(-)

-- 
2.28.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/4] KVM: VMX: Rename vmx_*_supported() helpers to cpu_has_vmx_*()
  2020-09-23 16:50 [PATCH 0/4] KVM: VMX: Add helper+macros to do sec exec adjustment Sean Christopherson
@ 2020-09-23 16:50 ` Sean Christopherson
  2020-09-23 16:50 ` [PATCH 2/4] KVM: VMX: Unconditionally clear CPUID.INVPCID if !CPUID.PCID Sean Christopherson
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 12+ messages in thread
From: Sean Christopherson @ 2020-09-23 16:50 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

Rename helpers for a few controls to conform to the more prevelant style
of cpu_has_vmx_<feature>().  Consistent names will allow adding macros
to consolidate the boilerplate code for adjusting secondary execution
controls.

No functional change intended.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kvm/vmx/capabilities.h |  8 ++++----
 arch/x86/kvm/vmx/vmx.c          | 14 +++++++-------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
index 4bbd8b448d22..5761736d373a 100644
--- a/arch/x86/kvm/vmx/capabilities.h
+++ b/arch/x86/kvm/vmx/capabilities.h
@@ -196,7 +196,7 @@ static inline bool cpu_has_vmx_ple(void)
 		SECONDARY_EXEC_PAUSE_LOOP_EXITING;
 }
 
-static inline bool vmx_rdrand_supported(void)
+static inline bool cpu_has_vmx_rdrand(void)
 {
 	return vmcs_config.cpu_based_2nd_exec_ctrl &
 		SECONDARY_EXEC_RDRAND_EXITING;
@@ -233,7 +233,7 @@ static inline bool cpu_has_vmx_encls_vmexit(void)
 		SECONDARY_EXEC_ENCLS_EXITING;
 }
 
-static inline bool vmx_rdseed_supported(void)
+static inline bool cpu_has_vmx_rdseed(void)
 {
 	return vmcs_config.cpu_based_2nd_exec_ctrl &
 		SECONDARY_EXEC_RDSEED_EXITING;
@@ -244,13 +244,13 @@ static inline bool cpu_has_vmx_pml(void)
 	return vmcs_config.cpu_based_2nd_exec_ctrl & SECONDARY_EXEC_ENABLE_PML;
 }
 
-static inline bool vmx_xsaves_supported(void)
+static inline bool cpu_has_vmx_xsaves(void)
 {
 	return vmcs_config.cpu_based_2nd_exec_ctrl &
 		SECONDARY_EXEC_XSAVES;
 }
 
-static inline bool vmx_waitpkg_supported(void)
+static inline bool cpu_has_vmx_waitpkg(void)
 {
 	return vmcs_config.cpu_based_2nd_exec_ctrl &
 		SECONDARY_EXEC_ENABLE_USR_WAIT_PAUSE;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 6f9a0c6d5dc5..cfed29329e4f 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -4112,7 +4112,7 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 	if (!enable_pml)
 		exec_control &= ~SECONDARY_EXEC_ENABLE_PML;
 
-	if (vmx_xsaves_supported()) {
+	if (cpu_has_vmx_xsaves()) {
 		/* Exposing XSAVES only when XSAVE is exposed */
 		bool xsaves_enabled =
 			boot_cpu_has(X86_FEATURE_XSAVE) &&
@@ -4170,7 +4170,7 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 		}
 	}
 
-	if (vmx_rdrand_supported()) {
+	if (cpu_has_vmx_rdrand()) {
 		bool rdrand_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDRAND);
 		if (rdrand_enabled)
 			exec_control &= ~SECONDARY_EXEC_RDRAND_EXITING;
@@ -4185,7 +4185,7 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 		}
 	}
 
-	if (vmx_rdseed_supported()) {
+	if (cpu_has_vmx_rdseed()) {
 		bool rdseed_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDSEED);
 		if (rdseed_enabled)
 			exec_control &= ~SECONDARY_EXEC_RDSEED_EXITING;
@@ -4200,7 +4200,7 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 		}
 	}
 
-	if (vmx_waitpkg_supported()) {
+	if (cpu_has_vmx_waitpkg()) {
 		bool waitpkg_enabled =
 			guest_cpuid_has(vcpu, X86_FEATURE_WAITPKG);
 
@@ -4308,7 +4308,7 @@ static void init_vmcs(struct vcpu_vmx *vmx)
 	if (vmx->vpid != 0)
 		vmcs_write16(VIRTUAL_PROCESSOR_ID, vmx->vpid);
 
-	if (vmx_xsaves_supported())
+	if (cpu_has_vmx_xsaves())
 		vmcs_write64(XSS_EXIT_BITMAP, VMX_XSS_EXIT_BITMAP);
 
 	if (enable_pml) {
@@ -7271,14 +7271,14 @@ static __init void vmx_set_cpu_caps(void)
 
 	/* CPUID 0xD.1 */
 	supported_xss = 0;
-	if (!vmx_xsaves_supported())
+	if (!cpu_has_vmx_xsaves())
 		kvm_cpu_cap_clear(X86_FEATURE_XSAVES);
 
 	/* CPUID 0x80000001 */
 	if (!cpu_has_vmx_rdtscp())
 		kvm_cpu_cap_clear(X86_FEATURE_RDTSCP);
 
-	if (vmx_waitpkg_supported())
+	if (cpu_has_vmx_waitpkg())
 		kvm_cpu_cap_check_and_set(X86_FEATURE_WAITPKG);
 }
 
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/4] KVM: VMX: Unconditionally clear CPUID.INVPCID if !CPUID.PCID
  2020-09-23 16:50 [PATCH 0/4] KVM: VMX: Add helper+macros to do sec exec adjustment Sean Christopherson
  2020-09-23 16:50 ` [PATCH 1/4] KVM: VMX: Rename vmx_*_supported() helpers to cpu_has_vmx_*() Sean Christopherson
@ 2020-09-23 16:50 ` Sean Christopherson
  2020-09-23 17:15   ` Jim Mattson
  2020-09-23 16:50 ` [PATCH 3/4] KVM: VMX: Rename RDTSCP secondary exec control name to insert "ENABLE" Sean Christopherson
  2020-09-23 16:50 ` [PATCH 4/4] KVM: VMX: Add a helper and macros to reduce boilerplate for sec exec ctls Sean Christopherson
  3 siblings, 1 reply; 12+ messages in thread
From: Sean Christopherson @ 2020-09-23 16:50 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

If PCID is not exposed to the guest, clear INVPCID in the guest's CPUID
even if the VMCS INVPCID enable is not supported.  This will allow
consolidating the secondary execution control adjustment code without
having to special case INVPCID.

Technically, this fixes a bug where !CPUID.PCID && CPUID.INVCPID would
result in unexpected guest behavior (#UD instead of #GP/#PF), but KVM
doesn't support exposing INVPCID if it's not supported in the VMCS, i.e.
such a config is broken/bogus no matter what.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index cfed29329e4f..57e48c5a1e91 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -4149,16 +4149,22 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 		}
 	}
 
+	/*
+	 * Expose INVPCID if and only if PCID is also exposed to the guest.
+	 * INVPCID takes a #UD when it's disabled in the VMCS, but a #GP or #PF
+	 * if CR4.PCIDE=0.  Enumerating CPUID.INVPCID=1 would lead to incorrect
+	 * behavior from the guest perspective (it would expect #GP or #PF).
+	 */
+	if (!guest_cpuid_has(vcpu, X86_FEATURE_PCID))
+		guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID);
+
 	if (cpu_has_vmx_invpcid()) {
 		/* Exposing INVPCID only when PCID is exposed */
 		bool invpcid_enabled =
-			guest_cpuid_has(vcpu, X86_FEATURE_INVPCID) &&
-			guest_cpuid_has(vcpu, X86_FEATURE_PCID);
+			guest_cpuid_has(vcpu, X86_FEATURE_INVPCID);
 
-		if (!invpcid_enabled) {
+		if (!invpcid_enabled)
 			exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
-			guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID);
-		}
 
 		if (nested) {
 			if (invpcid_enabled)
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/4] KVM: VMX: Rename RDTSCP secondary exec control name to insert "ENABLE"
  2020-09-23 16:50 [PATCH 0/4] KVM: VMX: Add helper+macros to do sec exec adjustment Sean Christopherson
  2020-09-23 16:50 ` [PATCH 1/4] KVM: VMX: Rename vmx_*_supported() helpers to cpu_has_vmx_*() Sean Christopherson
  2020-09-23 16:50 ` [PATCH 2/4] KVM: VMX: Unconditionally clear CPUID.INVPCID if !CPUID.PCID Sean Christopherson
@ 2020-09-23 16:50 ` Sean Christopherson
  2020-09-23 17:10   ` Jim Mattson
  2020-09-23 16:50 ` [PATCH 4/4] KVM: VMX: Add a helper and macros to reduce boilerplate for sec exec ctls Sean Christopherson
  3 siblings, 1 reply; 12+ messages in thread
From: Sean Christopherson @ 2020-09-23 16:50 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

Rename SECONDARY_EXEC_RDTSCP to SECONDARY_EXEC_ENABLE_RDTSCP in
preparation for consolidating the logic for adjusting secondary exec
controls based on the guest CPUID model.

No functional change intended.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/include/asm/vmx.h                       |  2 +-
 arch/x86/kvm/vmx/capabilities.h                  |  2 +-
 arch/x86/kvm/vmx/nested.c                        |  4 ++--
 arch/x86/kvm/vmx/vmx.c                           | 10 +++++-----
 tools/testing/selftests/kvm/include/x86_64/vmx.h |  2 +-
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
index cd7de4b401fe..f8ba5289ecb0 100644
--- a/arch/x86/include/asm/vmx.h
+++ b/arch/x86/include/asm/vmx.h
@@ -52,7 +52,7 @@
 #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES VMCS_CONTROL_BIT(VIRT_APIC_ACCESSES)
 #define SECONDARY_EXEC_ENABLE_EPT               VMCS_CONTROL_BIT(EPT)
 #define SECONDARY_EXEC_DESC			VMCS_CONTROL_BIT(DESC_EXITING)
-#define SECONDARY_EXEC_RDTSCP			VMCS_CONTROL_BIT(RDTSCP)
+#define SECONDARY_EXEC_ENABLE_RDTSCP		VMCS_CONTROL_BIT(RDTSCP)
 #define SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE   VMCS_CONTROL_BIT(VIRTUAL_X2APIC)
 #define SECONDARY_EXEC_ENABLE_VPID              VMCS_CONTROL_BIT(VPID)
 #define SECONDARY_EXEC_WBINVD_EXITING		VMCS_CONTROL_BIT(WBINVD_EXITING)
diff --git a/arch/x86/kvm/vmx/capabilities.h b/arch/x86/kvm/vmx/capabilities.h
index 5761736d373a..3a1861403d73 100644
--- a/arch/x86/kvm/vmx/capabilities.h
+++ b/arch/x86/kvm/vmx/capabilities.h
@@ -151,7 +151,7 @@ static inline bool vmx_umip_emulated(void)
 static inline bool cpu_has_vmx_rdtscp(void)
 {
 	return vmcs_config.cpu_based_2nd_exec_ctrl &
-		SECONDARY_EXEC_RDTSCP;
+		SECONDARY_EXEC_ENABLE_RDTSCP;
 }
 
 static inline bool cpu_has_vmx_virtualize_x2apic_mode(void)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index b6ce9ce91029..c5808d798e4c 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2286,7 +2286,7 @@ static void prepare_vmcs02_early(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
 		/* Take the following fields only from vmcs12 */
 		exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
 				  SECONDARY_EXEC_ENABLE_INVPCID |
-				  SECONDARY_EXEC_RDTSCP |
+				  SECONDARY_EXEC_ENABLE_RDTSCP |
 				  SECONDARY_EXEC_XSAVES |
 				  SECONDARY_EXEC_ENABLE_USR_WAIT_PAUSE |
 				  SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
@@ -6393,7 +6393,7 @@ void nested_vmx_setup_ctls_msrs(struct nested_vmx_msrs *msrs, u32 ept_caps)
 	msrs->secondary_ctls_low = 0;
 	msrs->secondary_ctls_high &=
 		SECONDARY_EXEC_DESC |
-		SECONDARY_EXEC_RDTSCP |
+		SECONDARY_EXEC_ENABLE_RDTSCP |
 		SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
 		SECONDARY_EXEC_WBINVD_EXITING |
 		SECONDARY_EXEC_APIC_REGISTER_VIRT |
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 57e48c5a1e91..5180529f6531 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2421,7 +2421,7 @@ static __init int setup_vmcs_config(struct vmcs_config *vmcs_conf,
 			SECONDARY_EXEC_UNRESTRICTED_GUEST |
 			SECONDARY_EXEC_PAUSE_LOOP_EXITING |
 			SECONDARY_EXEC_DESC |
-			SECONDARY_EXEC_RDTSCP |
+			SECONDARY_EXEC_ENABLE_RDTSCP |
 			SECONDARY_EXEC_ENABLE_INVPCID |
 			SECONDARY_EXEC_APIC_REGISTER_VIRT |
 			SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
@@ -4137,15 +4137,15 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 	if (cpu_has_vmx_rdtscp()) {
 		bool rdtscp_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP);
 		if (!rdtscp_enabled)
-			exec_control &= ~SECONDARY_EXEC_RDTSCP;
+			exec_control &= ~SECONDARY_EXEC_ENABLE_RDTSCP;
 
 		if (nested) {
 			if (rdtscp_enabled)
 				vmx->nested.msrs.secondary_ctls_high |=
-					SECONDARY_EXEC_RDTSCP;
+					SECONDARY_EXEC_ENABLE_RDTSCP;
 			else
 				vmx->nested.msrs.secondary_ctls_high &=
-					~SECONDARY_EXEC_RDTSCP;
+					~SECONDARY_EXEC_ENABLE_RDTSCP;
 		}
 	}
 
@@ -7340,7 +7340,7 @@ static int vmx_check_intercept(struct kvm_vcpu *vcpu,
 	 * Because it is marked as EmulateOnUD, we need to intercept it here.
 	 */
 	case x86_intercept_rdtscp:
-		if (!nested_cpu_has2(vmcs12, SECONDARY_EXEC_RDTSCP)) {
+		if (!nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENABLE_RDTSCP)) {
 			exception->vector = UD_VECTOR;
 			exception->error_code_valid = false;
 			return X86EMUL_PROPAGATE_FAULT;
diff --git a/tools/testing/selftests/kvm/include/x86_64/vmx.h b/tools/testing/selftests/kvm/include/x86_64/vmx.h
index 16fa21ebb99c..54d624dd6c10 100644
--- a/tools/testing/selftests/kvm/include/x86_64/vmx.h
+++ b/tools/testing/selftests/kvm/include/x86_64/vmx.h
@@ -48,7 +48,7 @@
 #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001
 #define SECONDARY_EXEC_ENABLE_EPT		0x00000002
 #define SECONDARY_EXEC_DESC			0x00000004
-#define SECONDARY_EXEC_RDTSCP			0x00000008
+#define SECONDARY_EXEC_ENABLE_RDTSCP		0x00000008
 #define SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE	0x00000010
 #define SECONDARY_EXEC_ENABLE_VPID		0x00000020
 #define SECONDARY_EXEC_WBINVD_EXITING		0x00000040
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/4] KVM: VMX: Add a helper and macros to reduce boilerplate for sec exec ctls
  2020-09-23 16:50 [PATCH 0/4] KVM: VMX: Add helper+macros to do sec exec adjustment Sean Christopherson
                   ` (2 preceding siblings ...)
  2020-09-23 16:50 ` [PATCH 3/4] KVM: VMX: Rename RDTSCP secondary exec control name to insert "ENABLE" Sean Christopherson
@ 2020-09-23 16:50 ` Sean Christopherson
  2020-09-23 17:20   ` Paolo Bonzini
  3 siblings, 1 reply; 12+ messages in thread
From: Sean Christopherson @ 2020-09-23 16:50 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel

Add a helper function and several wrapping macros to consolidate the
copy-paste code in vmx_compute_secondary_exec_control() for adjusting
controls that are dependent on guest CPUID bits.

No functional change intended.

Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
---
 arch/x86/kvm/vmx/vmx.c | 128 +++++++++++++----------------------------
 1 file changed, 41 insertions(+), 87 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 5180529f6531..b786cfb74f4f 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -4073,6 +4073,38 @@ u32 vmx_exec_control(struct vcpu_vmx *vmx)
 }
 
 
+static inline void
+vmx_adjust_secondary_exec_control(struct vcpu_vmx *vmx, u32 *exec_control,
+				  u32 control, bool enabled, bool exiting)
+{
+	if (enabled == exiting)
+		*exec_control &= ~control;
+	if (nested) {
+		if (enabled)
+			vmx->nested.msrs.secondary_ctls_high |= control;
+		else
+			vmx->nested.msrs.secondary_ctls_high &= ~control;
+	}
+}
+
+#define vmx_adjust_sec_exec_control(vmx, exec_control, name, feat_name, ctrl_name, exiting) \
+({									 \
+	bool __enabled;							 \
+									 \
+	if (cpu_has_vmx_##name()) {					 \
+		__enabled = guest_cpuid_has(&(vmx)->vcpu,		 \
+					    X86_FEATURE_##feat_name);	 \
+		vmx_adjust_secondary_exec_control(vmx, exec_control,	 \
+			SECONDARY_EXEC_##ctrl_name, __enabled, exiting); \
+	}								 \
+})
+
+#define vmx_adjust_sec_exec_feature(vmx, exec_control, lname, uname) \
+	vmx_adjust_sec_exec_control(vmx, exec_control, lname, uname, ENABLE_##uname, false)
+
+#define vmx_adjust_sec_exec_exiting(vmx, exec_control, lname, uname) \
+	vmx_adjust_sec_exec_control(vmx, exec_control, lname, uname, uname##_EXITING, true)
+
 static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 {
 	struct kvm_vcpu *vcpu = &vmx->vcpu;
@@ -4121,33 +4153,12 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 
 		vcpu->arch.xsaves_enabled = xsaves_enabled;
 
-		if (!xsaves_enabled)
-			exec_control &= ~SECONDARY_EXEC_XSAVES;
-
-		if (nested) {
-			if (xsaves_enabled)
-				vmx->nested.msrs.secondary_ctls_high |=
-					SECONDARY_EXEC_XSAVES;
-			else
-				vmx->nested.msrs.secondary_ctls_high &=
-					~SECONDARY_EXEC_XSAVES;
-		}
+		vmx_adjust_secondary_exec_control(vmx, &exec_control,
+						  SECONDARY_EXEC_XSAVES,
+						  xsaves_enabled, false);
 	}
 
-	if (cpu_has_vmx_rdtscp()) {
-		bool rdtscp_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP);
-		if (!rdtscp_enabled)
-			exec_control &= ~SECONDARY_EXEC_ENABLE_RDTSCP;
-
-		if (nested) {
-			if (rdtscp_enabled)
-				vmx->nested.msrs.secondary_ctls_high |=
-					SECONDARY_EXEC_ENABLE_RDTSCP;
-			else
-				vmx->nested.msrs.secondary_ctls_high &=
-					~SECONDARY_EXEC_ENABLE_RDTSCP;
-		}
-	}
+	vmx_adjust_sec_exec_feature(vmx, &exec_control, rdtscp, RDTSCP);
 
 	/*
 	 * Expose INVPCID if and only if PCID is also exposed to the guest.
@@ -4157,71 +4168,14 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 	 */
 	if (!guest_cpuid_has(vcpu, X86_FEATURE_PCID))
 		guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID);
+	vmx_adjust_sec_exec_feature(vmx, &exec_control, invpcid, INVPCID);
 
-	if (cpu_has_vmx_invpcid()) {
-		/* Exposing INVPCID only when PCID is exposed */
-		bool invpcid_enabled =
-			guest_cpuid_has(vcpu, X86_FEATURE_INVPCID);
 
-		if (!invpcid_enabled)
-			exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
+	vmx_adjust_sec_exec_exiting(vmx, &exec_control, rdrand, RDRAND);
+	vmx_adjust_sec_exec_exiting(vmx, &exec_control, rdseed, RDSEED);
 
-		if (nested) {
-			if (invpcid_enabled)
-				vmx->nested.msrs.secondary_ctls_high |=
-					SECONDARY_EXEC_ENABLE_INVPCID;
-			else
-				vmx->nested.msrs.secondary_ctls_high &=
-					~SECONDARY_EXEC_ENABLE_INVPCID;
-		}
-	}
-
-	if (cpu_has_vmx_rdrand()) {
-		bool rdrand_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDRAND);
-		if (rdrand_enabled)
-			exec_control &= ~SECONDARY_EXEC_RDRAND_EXITING;
-
-		if (nested) {
-			if (rdrand_enabled)
-				vmx->nested.msrs.secondary_ctls_high |=
-					SECONDARY_EXEC_RDRAND_EXITING;
-			else
-				vmx->nested.msrs.secondary_ctls_high &=
-					~SECONDARY_EXEC_RDRAND_EXITING;
-		}
-	}
-
-	if (cpu_has_vmx_rdseed()) {
-		bool rdseed_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDSEED);
-		if (rdseed_enabled)
-			exec_control &= ~SECONDARY_EXEC_RDSEED_EXITING;
-
-		if (nested) {
-			if (rdseed_enabled)
-				vmx->nested.msrs.secondary_ctls_high |=
-					SECONDARY_EXEC_RDSEED_EXITING;
-			else
-				vmx->nested.msrs.secondary_ctls_high &=
-					~SECONDARY_EXEC_RDSEED_EXITING;
-		}
-	}
-
-	if (cpu_has_vmx_waitpkg()) {
-		bool waitpkg_enabled =
-			guest_cpuid_has(vcpu, X86_FEATURE_WAITPKG);
-
-		if (!waitpkg_enabled)
-			exec_control &= ~SECONDARY_EXEC_ENABLE_USR_WAIT_PAUSE;
-
-		if (nested) {
-			if (waitpkg_enabled)
-				vmx->nested.msrs.secondary_ctls_high |=
-					SECONDARY_EXEC_ENABLE_USR_WAIT_PAUSE;
-			else
-				vmx->nested.msrs.secondary_ctls_high &=
-					~SECONDARY_EXEC_ENABLE_USR_WAIT_PAUSE;
-		}
-	}
+	vmx_adjust_sec_exec_control(vmx, &exec_control, waitpkg, WAITPKG,
+				    ENABLE_USR_WAIT_PAUSE, false);
 
 	vmx->secondary_exec_control = exec_control;
 }
-- 
2.28.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/4] KVM: VMX: Rename RDTSCP secondary exec control name to insert "ENABLE"
  2020-09-23 16:50 ` [PATCH 3/4] KVM: VMX: Rename RDTSCP secondary exec control name to insert "ENABLE" Sean Christopherson
@ 2020-09-23 17:10   ` Jim Mattson
  0 siblings, 0 replies; 12+ messages in thread
From: Jim Mattson @ 2020-09-23 17:10 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel,
	kvm list, LKML

On Wed, Sep 23, 2020 at 9:51 AM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> Rename SECONDARY_EXEC_RDTSCP to SECONDARY_EXEC_ENABLE_RDTSCP in
> preparation for consolidating the logic for adjusting secondary exec
> controls based on the guest CPUID model.
>
> No functional change intended.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
Reviewed-by: Jim Mattson <jmattson@google.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] KVM: VMX: Unconditionally clear CPUID.INVPCID if !CPUID.PCID
  2020-09-23 16:50 ` [PATCH 2/4] KVM: VMX: Unconditionally clear CPUID.INVPCID if !CPUID.PCID Sean Christopherson
@ 2020-09-23 17:15   ` Jim Mattson
  2020-09-23 17:25     ` Paolo Bonzini
  0 siblings, 1 reply; 12+ messages in thread
From: Jim Mattson @ 2020-09-23 17:15 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Paolo Bonzini, Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel,
	kvm list, LKML

On Wed, Sep 23, 2020 at 9:51 AM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> If PCID is not exposed to the guest, clear INVPCID in the guest's CPUID
> even if the VMCS INVPCID enable is not supported.  This will allow
> consolidating the secondary execution control adjustment code without
> having to special case INVPCID.
>
> Technically, this fixes a bug where !CPUID.PCID && CPUID.INVCPID would
> result in unexpected guest behavior (#UD instead of #GP/#PF), but KVM
> doesn't support exposing INVPCID if it's not supported in the VMCS, i.e.
> such a config is broken/bogus no matter what.
>
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index cfed29329e4f..57e48c5a1e91 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4149,16 +4149,22 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>                 }
>         }
>
> +       /*
> +        * Expose INVPCID if and only if PCID is also exposed to the guest.
> +        * INVPCID takes a #UD when it's disabled in the VMCS, but a #GP or #PF
> +        * if CR4.PCIDE=0.  Enumerating CPUID.INVPCID=1 would lead to incorrect
> +        * behavior from the guest perspective (it would expect #GP or #PF).
> +        */
> +       if (!guest_cpuid_has(vcpu, X86_FEATURE_PCID))
> +               guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID);
> +

I thought the general rule for userspace provided CPUID bits was that
kvm only made any adjustments necessary to prevent bad things from
happening at the host level. Proper guest semantics are entirely the
responsibility of userspace. Or did I misunderstand?

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] KVM: VMX: Add a helper and macros to reduce boilerplate for sec exec ctls
  2020-09-23 16:50 ` [PATCH 4/4] KVM: VMX: Add a helper and macros to reduce boilerplate for sec exec ctls Sean Christopherson
@ 2020-09-23 17:20   ` Paolo Bonzini
  2020-09-23 17:22     ` Sean Christopherson
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2020-09-23 17:20 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
	linux-kernel

On 23/09/20 18:50, Sean Christopherson wrote:
> Add a helper function and several wrapping macros to consolidate the
> copy-paste code in vmx_compute_secondary_exec_control() for adjusting
> controls that are dependent on guest CPUID bits.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
>  arch/x86/kvm/vmx/vmx.c | 128 +++++++++++++----------------------------
>  1 file changed, 41 insertions(+), 87 deletions(-)

The diffstat is enticing but the code a little less so...  Can you just
add documentation above vmx_adjust_secondary_exec_control that explains
the how/why?

Paolo

> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 5180529f6531..b786cfb74f4f 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4073,6 +4073,38 @@ u32 vmx_exec_control(struct vcpu_vmx *vmx)
>  }
>  
>  
> +static inline void
> +vmx_adjust_secondary_exec_control(struct vcpu_vmx *vmx, u32 *exec_control,
> +				  u32 control, bool enabled, bool exiting)
> +{
> +	if (enabled == exiting)
> +		*exec_control &= ~control;
> +	if (nested) {
> +		if (enabled)
> +			vmx->nested.msrs.secondary_ctls_high |= control;
> +		else
> +			vmx->nested.msrs.secondary_ctls_high &= ~control;
> +	}
> +}
> +
> +#define vmx_adjust_sec_exec_control(vmx, exec_control, name, feat_name, ctrl_name, exiting) \
> +({									 \
> +	bool __enabled;							 \
> +									 \
> +	if (cpu_has_vmx_##name()) {					 \
> +		__enabled = guest_cpuid_has(&(vmx)->vcpu,		 \
> +					    X86_FEATURE_##feat_name);	 \
> +		vmx_adjust_secondary_exec_control(vmx, exec_control,	 \
> +			SECONDARY_EXEC_##ctrl_name, __enabled, exiting); \
> +	}								 \
> +})
> +
> +#define vmx_adjust_sec_exec_feature(vmx, exec_control, lname, uname) \
> +	vmx_adjust_sec_exec_control(vmx, exec_control, lname, uname, ENABLE_##uname, false)
> +
> +#define vmx_adjust_sec_exec_exiting(vmx, exec_control, lname, uname) \
> +	vmx_adjust_sec_exec_control(vmx, exec_control, lname, uname, uname##_EXITING, true)
> +
>  static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>  {


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] KVM: VMX: Add a helper and macros to reduce boilerplate for sec exec ctls
  2020-09-23 17:20   ` Paolo Bonzini
@ 2020-09-23 17:22     ` Sean Christopherson
  2020-09-23 17:25       ` Paolo Bonzini
  0 siblings, 1 reply; 12+ messages in thread
From: Sean Christopherson @ 2020-09-23 17:22 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
	linux-kernel

On Wed, Sep 23, 2020 at 07:20:17PM +0200, Paolo Bonzini wrote:
> On 23/09/20 18:50, Sean Christopherson wrote:
> > Add a helper function and several wrapping macros to consolidate the
> > copy-paste code in vmx_compute_secondary_exec_control() for adjusting
> > controls that are dependent on guest CPUID bits.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---
> >  arch/x86/kvm/vmx/vmx.c | 128 +++++++++++++----------------------------
> >  1 file changed, 41 insertions(+), 87 deletions(-)
> 
> The diffstat is enticing but the code a little less so...  Can you just
> add documentation above vmx_adjust_secondary_exec_control that explains
> the how/why?

Ya, I'd be more than happy to add a big comment.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/4] KVM: VMX: Unconditionally clear CPUID.INVPCID if !CPUID.PCID
  2020-09-23 17:15   ` Jim Mattson
@ 2020-09-23 17:25     ` Paolo Bonzini
  0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2020-09-23 17:25 UTC (permalink / raw)
  To: Jim Mattson, Sean Christopherson
  Cc: Vitaly Kuznetsov, Wanpeng Li, Joerg Roedel, kvm list, LKML

On 23/09/20 19:15, Jim Mattson wrote:
> On Wed, Sep 23, 2020 at 9:51 AM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
>>
>> If PCID is not exposed to the guest, clear INVPCID in the guest's CPUID
>> even if the VMCS INVPCID enable is not supported.  This will allow
>> consolidating the secondary execution control adjustment code without
>> having to special case INVPCID.
>>
>> Technically, this fixes a bug where !CPUID.PCID && CPUID.INVCPID would
>> result in unexpected guest behavior (#UD instead of #GP/#PF), but KVM
>> doesn't support exposing INVPCID if it's not supported in the VMCS, i.e.
>> such a config is broken/bogus no matter what.
>>
>> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
>> ---
>>  arch/x86/kvm/vmx/vmx.c | 16 +++++++++++-----
>>  1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>> index cfed29329e4f..57e48c5a1e91 100644
>> --- a/arch/x86/kvm/vmx/vmx.c
>> +++ b/arch/x86/kvm/vmx/vmx.c
>> @@ -4149,16 +4149,22 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>>                 }
>>         }
>>
>> +       /*
>> +        * Expose INVPCID if and only if PCID is also exposed to the guest.
>> +        * INVPCID takes a #UD when it's disabled in the VMCS, but a #GP or #PF
>> +        * if CR4.PCIDE=0.  Enumerating CPUID.INVPCID=1 would lead to incorrect
>> +        * behavior from the guest perspective (it would expect #GP or #PF).
>> +        */
>> +       if (!guest_cpuid_has(vcpu, X86_FEATURE_PCID))
>> +               guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID);
>> +
> 
> I thought the general rule for userspace provided CPUID bits was that
> kvm only made any adjustments necessary to prevent bad things from
> happening at the host level. Proper guest semantics are entirely the
> responsibility of userspace. Or did I misunderstand?
> 

Yes, that's generally the idea.  INVPCID has always been a bit special
due to the secondary execution control being of the "enable" kind; this
led the original author to try and disable the instruction (which is by
itself something we do not always do, and sometimes cannot always do).

So I agree that Sean's patch is of marginal utility by itself; however
it lets him use the new macros in patch 4 and it is a good idea to
separate the small functional change into its own commit.

Paolo


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 4/4] KVM: VMX: Add a helper and macros to reduce boilerplate for sec exec ctls
  2020-09-23 17:22     ` Sean Christopherson
@ 2020-09-23 17:25       ` Paolo Bonzini
       [not found]         ` <20200925003011.21016-1-sean.j.christopherson@intel.com>
  0 siblings, 1 reply; 12+ messages in thread
From: Paolo Bonzini @ 2020-09-23 17:25 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
	linux-kernel

On 23/09/20 19:22, Sean Christopherson wrote:
> On Wed, Sep 23, 2020 at 07:20:17PM +0200, Paolo Bonzini wrote:
>> On 23/09/20 18:50, Sean Christopherson wrote:
>>> Add a helper function and several wrapping macros to consolidate the
>>> copy-paste code in vmx_compute_secondary_exec_control() for adjusting
>>> controls that are dependent on guest CPUID bits.
>>>
>>> No functional change intended.
>>>
>>> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
>>> ---
>>>  arch/x86/kvm/vmx/vmx.c | 128 +++++++++++++----------------------------
>>>  1 file changed, 41 insertions(+), 87 deletions(-)
>>
>> The diffstat is enticing but the code a little less so...  Can you just
>> add documentation above vmx_adjust_secondary_exec_control that explains
>> the how/why?
> 
> Ya, I'd be more than happy to add a big comment.
> 

Ok, I'll wait for v2 of this patch only.

Paolo


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v2 4/4] KVM: VMX: Add a helper and macros to reduce boilerplate for sec exec ctls
       [not found]         ` <20200925003011.21016-1-sean.j.christopherson@intel.com>
@ 2020-09-25 20:21           ` Paolo Bonzini
  0 siblings, 0 replies; 12+ messages in thread
From: Paolo Bonzini @ 2020-09-25 20:21 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
	linux-kernel

On 25/09/20 02:30, Sean Christopherson wrote:
> Add a helper function and several wrapping macros to consolidate the
> copy-paste code in vmx_compute_secondary_exec_control() for adjusting
> controls that are dependent on guest CPUID bits.
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> ---
> 
> v2: Comment the new helper and macros.
> 
>  arch/x86/kvm/vmx/vmx.c | 151 +++++++++++++++++------------------------
>  1 file changed, 64 insertions(+), 87 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index 5180529f6531..48673eea0c0d 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -4072,6 +4072,61 @@ u32 vmx_exec_control(struct vcpu_vmx *vmx)
>  	return exec_control;
>  }
>  
> +/*
> + * Adjust a single secondary execution control bit to intercept/allow an
> + * instruction in the guest.  This is usually done based on whether or not a
> + * feature has been exposed to the guest in order to correctly emulate faults.
> + */
> +static inline void
> +vmx_adjust_secondary_exec_control(struct vcpu_vmx *vmx, u32 *exec_control,
> +				  u32 control, bool enabled, bool exiting)
> +{
> +	/*
> +	 * If the control is for an opt-in feature, clear the control if the
> +	 * feature is not exposed to the guest, i.e. not enabled.  If the
> +	 * control is opt-out, i.e. an exiting control, clear the control if
> +	 * the feature _is_ exposed to the guest, i.e. exiting/interception is
> +	 * disabled for the associated instruction.  Note, the caller is
> +	 * responsible presetting exec_control to set all supported bits.
> +	 */
> +	if (enabled == exiting)
> +		*exec_control &= ~control;
> +
> +	/*
> +	 * Update the nested MSR settings so that a nested VMM can/can't set
> +	 * controls for features that are/aren't exposed to the guest.
> +	 */
> +	if (nested) {
> +		if (enabled)
> +			vmx->nested.msrs.secondary_ctls_high |= control;
> +		else
> +			vmx->nested.msrs.secondary_ctls_high &= ~control;
> +	}
> +}
> +
> +/*
> + * Wrapper macro for the common case of adjusting a secondary execution control
> + * based on a single guest CPUID bit, with a dedicated feature bit.  This also
> + * verifies that the control is actually supported by KVM and hardware.
> + */
> +#define vmx_adjust_sec_exec_control(vmx, exec_control, name, feat_name, ctrl_name, exiting) \
> +({									 \
> +	bool __enabled;							 \
> +									 \
> +	if (cpu_has_vmx_##name()) {					 \
> +		__enabled = guest_cpuid_has(&(vmx)->vcpu,		 \
> +					    X86_FEATURE_##feat_name);	 \
> +		vmx_adjust_secondary_exec_control(vmx, exec_control,	 \
> +			SECONDARY_EXEC_##ctrl_name, __enabled, exiting); \
> +	}								 \
> +})
> +
> +/* More macro magic for ENABLE_/opt-in versus _EXITING/opt-out controls. */
> +#define vmx_adjust_sec_exec_feature(vmx, exec_control, lname, uname) \
> +	vmx_adjust_sec_exec_control(vmx, exec_control, lname, uname, ENABLE_##uname, false)
> +
> +#define vmx_adjust_sec_exec_exiting(vmx, exec_control, lname, uname) \
> +	vmx_adjust_sec_exec_control(vmx, exec_control, lname, uname, uname##_EXITING, true)
>  
>  static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>  {
> @@ -4121,33 +4176,12 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>  
>  		vcpu->arch.xsaves_enabled = xsaves_enabled;
>  
> -		if (!xsaves_enabled)
> -			exec_control &= ~SECONDARY_EXEC_XSAVES;
> -
> -		if (nested) {
> -			if (xsaves_enabled)
> -				vmx->nested.msrs.secondary_ctls_high |=
> -					SECONDARY_EXEC_XSAVES;
> -			else
> -				vmx->nested.msrs.secondary_ctls_high &=
> -					~SECONDARY_EXEC_XSAVES;
> -		}
> +		vmx_adjust_secondary_exec_control(vmx, &exec_control,
> +						  SECONDARY_EXEC_XSAVES,
> +						  xsaves_enabled, false);
>  	}
>  
> -	if (cpu_has_vmx_rdtscp()) {
> -		bool rdtscp_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDTSCP);
> -		if (!rdtscp_enabled)
> -			exec_control &= ~SECONDARY_EXEC_ENABLE_RDTSCP;
> -
> -		if (nested) {
> -			if (rdtscp_enabled)
> -				vmx->nested.msrs.secondary_ctls_high |=
> -					SECONDARY_EXEC_ENABLE_RDTSCP;
> -			else
> -				vmx->nested.msrs.secondary_ctls_high &=
> -					~SECONDARY_EXEC_ENABLE_RDTSCP;
> -		}
> -	}
> +	vmx_adjust_sec_exec_feature(vmx, &exec_control, rdtscp, RDTSCP);
>  
>  	/*
>  	 * Expose INVPCID if and only if PCID is also exposed to the guest.
> @@ -4157,71 +4191,14 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
>  	 */
>  	if (!guest_cpuid_has(vcpu, X86_FEATURE_PCID))
>  		guest_cpuid_clear(vcpu, X86_FEATURE_INVPCID);
> +	vmx_adjust_sec_exec_feature(vmx, &exec_control, invpcid, INVPCID);
>  
> -	if (cpu_has_vmx_invpcid()) {
> -		/* Exposing INVPCID only when PCID is exposed */
> -		bool invpcid_enabled =
> -			guest_cpuid_has(vcpu, X86_FEATURE_INVPCID);
>  
> -		if (!invpcid_enabled)
> -			exec_control &= ~SECONDARY_EXEC_ENABLE_INVPCID;
> +	vmx_adjust_sec_exec_exiting(vmx, &exec_control, rdrand, RDRAND);
> +	vmx_adjust_sec_exec_exiting(vmx, &exec_control, rdseed, RDSEED);
>  
> -		if (nested) {
> -			if (invpcid_enabled)
> -				vmx->nested.msrs.secondary_ctls_high |=
> -					SECONDARY_EXEC_ENABLE_INVPCID;
> -			else
> -				vmx->nested.msrs.secondary_ctls_high &=
> -					~SECONDARY_EXEC_ENABLE_INVPCID;
> -		}
> -	}
> -
> -	if (cpu_has_vmx_rdrand()) {
> -		bool rdrand_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDRAND);
> -		if (rdrand_enabled)
> -			exec_control &= ~SECONDARY_EXEC_RDRAND_EXITING;
> -
> -		if (nested) {
> -			if (rdrand_enabled)
> -				vmx->nested.msrs.secondary_ctls_high |=
> -					SECONDARY_EXEC_RDRAND_EXITING;
> -			else
> -				vmx->nested.msrs.secondary_ctls_high &=
> -					~SECONDARY_EXEC_RDRAND_EXITING;
> -		}
> -	}
> -
> -	if (cpu_has_vmx_rdseed()) {
> -		bool rdseed_enabled = guest_cpuid_has(vcpu, X86_FEATURE_RDSEED);
> -		if (rdseed_enabled)
> -			exec_control &= ~SECONDARY_EXEC_RDSEED_EXITING;
> -
> -		if (nested) {
> -			if (rdseed_enabled)
> -				vmx->nested.msrs.secondary_ctls_high |=
> -					SECONDARY_EXEC_RDSEED_EXITING;
> -			else
> -				vmx->nested.msrs.secondary_ctls_high &=
> -					~SECONDARY_EXEC_RDSEED_EXITING;
> -		}
> -	}
> -
> -	if (cpu_has_vmx_waitpkg()) {
> -		bool waitpkg_enabled =
> -			guest_cpuid_has(vcpu, X86_FEATURE_WAITPKG);
> -
> -		if (!waitpkg_enabled)
> -			exec_control &= ~SECONDARY_EXEC_ENABLE_USR_WAIT_PAUSE;
> -
> -		if (nested) {
> -			if (waitpkg_enabled)
> -				vmx->nested.msrs.secondary_ctls_high |=
> -					SECONDARY_EXEC_ENABLE_USR_WAIT_PAUSE;
> -			else
> -				vmx->nested.msrs.secondary_ctls_high &=
> -					~SECONDARY_EXEC_ENABLE_USR_WAIT_PAUSE;
> -		}
> -	}
> +	vmx_adjust_sec_exec_control(vmx, &exec_control, waitpkg, WAITPKG,
> +				    ENABLE_USR_WAIT_PAUSE, false);
>  
>  	vmx->secondary_exec_control = exec_control;
>  }
> 

Queued with the rest, thanks.

Paolo


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2020-09-25 20:33 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-23 16:50 [PATCH 0/4] KVM: VMX: Add helper+macros to do sec exec adjustment Sean Christopherson
2020-09-23 16:50 ` [PATCH 1/4] KVM: VMX: Rename vmx_*_supported() helpers to cpu_has_vmx_*() Sean Christopherson
2020-09-23 16:50 ` [PATCH 2/4] KVM: VMX: Unconditionally clear CPUID.INVPCID if !CPUID.PCID Sean Christopherson
2020-09-23 17:15   ` Jim Mattson
2020-09-23 17:25     ` Paolo Bonzini
2020-09-23 16:50 ` [PATCH 3/4] KVM: VMX: Rename RDTSCP secondary exec control name to insert "ENABLE" Sean Christopherson
2020-09-23 17:10   ` Jim Mattson
2020-09-23 16:50 ` [PATCH 4/4] KVM: VMX: Add a helper and macros to reduce boilerplate for sec exec ctls Sean Christopherson
2020-09-23 17:20   ` Paolo Bonzini
2020-09-23 17:22     ` Sean Christopherson
2020-09-23 17:25       ` Paolo Bonzini
     [not found]         ` <20200925003011.21016-1-sean.j.christopherson@intel.com>
2020-09-25 20:21           ` [PATCH v2 " Paolo Bonzini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).