linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] KVM: nVMX: Use vmcs01 ctrls shadow as basis for vmcs02
@ 2021-08-10 17:19 Sean Christopherson
  2021-08-10 17:19 ` [PATCH 1/4] KVM: VMX: Use current VMCS to query WAITPKG support for MSR emulation Sean Christopherson
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Sean Christopherson @ 2021-08-10 17:19 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Zeng Guang

The goal of this series is to drop the vmx->secondary_exec_control cache
without degrading nested VM-Enter performance.  The cache is effective,
e.g. saves ~1000 cycles on nested VM-Enter, but confusing.  The worst of
the confusion could be eliminated by returning the computed value from
vmx_compute_secondary_exec_control() to make the calls to the "compute"
helper more like the other controls.  But, the nested VM-Enter path would
still have special handling for secondary exec controls, and ideally all
controls would benefit from caching, though the benefits are marginal for
other controls and thus difficult to justify.

Happily, vmcs01 already caches the calculated controls in the
controls_shadow.  The only issue is that the controls_shadow may have
dynamically toggled bits set.  However, that is not a fundamental problem,
it's simply different than what is expected by the nested VM-Enter code
and is easily remedied.

TL;DR: Get KVM's (L0's) desires for vmcs02 controls from vmcs01's
controls_shadow instead of recalculating the desired controls on every
nested VM-Enter, thus eliminating the need to have a dedicated cache for
the secondary exec controls calulation.

Sean Christopherson (4):
  KVM: VMX: Use current VMCS to query WAITPKG support for MSR emulation
  KVM: nVMX: Pull KVM L0's desired controls directly from vmcs01
  KVM: VMX: Drop caching of KVM's desired sec exec controls for vmcs01
  KVM: VMX: Hide VMCS control calculators in vmx.c

 arch/x86/kvm/vmx/nested.c | 25 ++++++++++++--------
 arch/x86/kvm/vmx/vmx.c    | 48 +++++++++++++++++++++++++++------------
 arch/x86/kvm/vmx/vmx.h    | 35 +++++-----------------------
 3 files changed, 56 insertions(+), 52 deletions(-)

-- 
2.32.0.605.g8dce9f2422-goog


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

* [PATCH 1/4] KVM: VMX: Use current VMCS to query WAITPKG support for MSR emulation
  2021-08-10 17:19 [PATCH 0/4] KVM: nVMX: Use vmcs01 ctrls shadow as basis for vmcs02 Sean Christopherson
@ 2021-08-10 17:19 ` Sean Christopherson
  2021-08-10 17:19 ` [PATCH 2/4] KVM: nVMX: Pull KVM L0's desired controls directly from vmcs01 Sean Christopherson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2021-08-10 17:19 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Zeng Guang

Use the secondary_exec_controls_get() accessor in vmx_has_waitpkg() to
effectively get the controls for the current VMCS, as opposed to using
vmx->secondary_exec_controls, which is the cached value of KVM's desired
controls for vmcs01 and truly not reflective of any particular VMCS.

While the waitpkg control is not dynamic, i.e. vmcs01 will always hold
the same waitpkg configuration as vmx->secondary_exec_controls, the same
does not hold true for vmcs02 if the L1 VMM hides the feature from L2.
If L1 hides the feature _and_ does not intercept MSR_IA32_UMWAIT_CONTROL,
L2 could incorrectly read/write L1's virtual MSR instead of taking a #GP.

Fixes: 6e3ba4abcea5 ("KVM: vmx: Emulate MSR IA32_UMWAIT_CONTROL")
Cc: stable@vger.kernel.org
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 0ecc41189292..4f151175d45a 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -521,7 +521,7 @@ static inline struct vmcs *alloc_vmcs(bool shadow)
 
 static inline bool vmx_has_waitpkg(struct vcpu_vmx *vmx)
 {
-	return vmx->secondary_exec_control &
+	return secondary_exec_controls_get(vmx) &
 		SECONDARY_EXEC_ENABLE_USR_WAIT_PAUSE;
 }
 
-- 
2.32.0.605.g8dce9f2422-goog


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

* [PATCH 2/4] KVM: nVMX: Pull KVM L0's desired controls directly from vmcs01
  2021-08-10 17:19 [PATCH 0/4] KVM: nVMX: Use vmcs01 ctrls shadow as basis for vmcs02 Sean Christopherson
  2021-08-10 17:19 ` [PATCH 1/4] KVM: VMX: Use current VMCS to query WAITPKG support for MSR emulation Sean Christopherson
@ 2021-08-10 17:19 ` Sean Christopherson
  2021-08-10 17:19 ` [PATCH 3/4] KVM: VMX: Drop caching of KVM's desired sec exec controls for vmcs01 Sean Christopherson
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2021-08-10 17:19 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Zeng Guang

When preparing controls for vmcs02, grab KVM's desired controls from
vmcs01's shadow state instead of recalculating the controls from scratch,
or in the secondary execution controls, instead of using the dedicated
cache.  Calculating secondary exec controls is eye-poppingly expensive
due to the guest CPUID checks, hence the dedicated cache, but the other
calculations aren't exactly free either.

Explicitly clear several bits (x2APIC, DESC exiting, and load EFER on
exit) as appropriate as they may be set in vmcs01, whereas the previous
implementation relied on dynamic bits being cleared in the calculator.

Intentionally propagate VM_{ENTRY,EXIT}_LOAD_IA32_PERF_GLOBAL_CTRL from
vmcs01 to vmcs02.  Whether or not PERF_GLOBAL_CTRL is loaded depends on
whether or not perf itself is active, so unless perf stops between the
exit from L1 and entry to L2, vmcs01 will hold the desired value.  This
is purely an optimization as atomic_switch_perf_msrs() will set/clear
the control as needed at VM-Enter, i.e. it avoids two extra VMWRITEs in
the case where perf is active (versus starting with the bits clear in
vmcs02, which was the previous behavior).

Cc: Zeng Guang <guang.zeng@intel.com>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/nested.c | 25 ++++++++++++++++---------
 arch/x86/kvm/vmx/vmx.h    |  6 +++++-
 2 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 0d0dd6580cfd..264a9f4c9179 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -2170,7 +2170,8 @@ static void prepare_vmcs02_early_rare(struct vcpu_vmx *vmx,
 	}
 }
 
-static void prepare_vmcs02_early(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
+static void prepare_vmcs02_early(struct vcpu_vmx *vmx, struct loaded_vmcs *vmcs01,
+				 struct vmcs12 *vmcs12)
 {
 	u32 exec_control;
 	u64 guest_efer = nested_vmx_calc_efer(vmx, vmcs12);
@@ -2181,7 +2182,7 @@ static void prepare_vmcs02_early(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
 	/*
 	 * PIN CONTROLS
 	 */
-	exec_control = vmx_pin_based_exec_ctrl(vmx);
+	exec_control = __pin_controls_get(vmcs01);
 	exec_control |= (vmcs12->pin_based_vm_exec_control &
 			 ~PIN_BASED_VMX_PREEMPTION_TIMER);
 
@@ -2197,7 +2198,7 @@ static void prepare_vmcs02_early(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
 	/*
 	 * EXEC CONTROLS
 	 */
-	exec_control = vmx_exec_control(vmx); /* L0's desires */
+	exec_control = __exec_controls_get(vmcs01); /* L0's desires */
 	exec_control &= ~CPU_BASED_INTR_WINDOW_EXITING;
 	exec_control &= ~CPU_BASED_NMI_WINDOW_EXITING;
 	exec_control &= ~CPU_BASED_TPR_SHADOW;
@@ -2234,10 +2235,11 @@ static void prepare_vmcs02_early(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
 	 * SECONDARY EXEC CONTROLS
 	 */
 	if (cpu_has_secondary_exec_ctrls()) {
-		exec_control = vmx->secondary_exec_control;
+		exec_control = __secondary_exec_controls_get(vmcs01);
 
 		/* Take the following fields only from vmcs12 */
 		exec_control &= ~(SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
+				  SECONDARY_EXEC_VIRTUALIZE_X2APIC_MODE |
 				  SECONDARY_EXEC_ENABLE_INVPCID |
 				  SECONDARY_EXEC_ENABLE_RDTSCP |
 				  SECONDARY_EXEC_XSAVES |
@@ -2245,7 +2247,9 @@ static void prepare_vmcs02_early(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
 				  SECONDARY_EXEC_VIRTUAL_INTR_DELIVERY |
 				  SECONDARY_EXEC_APIC_REGISTER_VIRT |
 				  SECONDARY_EXEC_ENABLE_VMFUNC |
-				  SECONDARY_EXEC_TSC_SCALING);
+				  SECONDARY_EXEC_TSC_SCALING |
+				  SECONDARY_EXEC_DESC);
+
 		if (nested_cpu_has(vmcs12,
 				   CPU_BASED_ACTIVATE_SECONDARY_CONTROLS))
 			exec_control |= vmcs12->secondary_vm_exec_control;
@@ -2285,8 +2289,9 @@ static void prepare_vmcs02_early(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
 	 * on the related bits (if supported by the CPU) in the hope that
 	 * we can avoid VMWrites during vmx_set_efer().
 	 */
-	exec_control = (vmcs12->vm_entry_controls | vmx_vmentry_ctrl()) &
-			~VM_ENTRY_IA32E_MODE & ~VM_ENTRY_LOAD_IA32_EFER;
+	exec_control = __vm_entry_controls_get(vmcs01);
+	exec_control |= vmcs12->vm_entry_controls;
+	exec_control &= ~(VM_ENTRY_IA32E_MODE | VM_ENTRY_LOAD_IA32_EFER);
 	if (cpu_has_load_ia32_efer()) {
 		if (guest_efer & EFER_LMA)
 			exec_control |= VM_ENTRY_IA32E_MODE;
@@ -2302,9 +2307,11 @@ static void prepare_vmcs02_early(struct vcpu_vmx *vmx, struct vmcs12 *vmcs12)
 	 * we should use its exit controls. Note that VM_EXIT_LOAD_IA32_EFER
 	 * bits may be modified by vmx_set_efer() in prepare_vmcs02().
 	 */
-	exec_control = vmx_vmexit_ctrl();
+	exec_control = __vm_exit_controls_get(vmcs01);
 	if (cpu_has_load_ia32_efer() && guest_efer != host_efer)
 		exec_control |= VM_EXIT_LOAD_IA32_EFER;
+	else
+		exec_control &= ~VM_EXIT_LOAD_IA32_EFER;
 	vm_exit_controls_set(vmx, exec_control);
 
 	/*
@@ -3347,7 +3354,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
 
 	vmx_switch_vmcs(vcpu, &vmx->nested.vmcs02);
 
-	prepare_vmcs02_early(vmx, vmcs12);
+	prepare_vmcs02_early(vmx, &vmx->vmcs01, vmcs12);
 
 	if (from_vmentry) {
 		if (unlikely(!nested_get_vmcs12_pages(vcpu))) {
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 4f151175d45a..414b440de9ac 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -418,9 +418,13 @@ static inline void lname##_controls_set(struct vcpu_vmx *vmx, u32 val)	    \
 		vmx->loaded_vmcs->controls_shadow.lname = val;		    \
 	}								    \
 }									    \
+static inline u32 __##lname##_controls_get(struct loaded_vmcs *vmcs)	    \
+{									    \
+	return vmcs->controls_shadow.lname;				    \
+}									    \
 static inline u32 lname##_controls_get(struct vcpu_vmx *vmx)		    \
 {									    \
-	return vmx->loaded_vmcs->controls_shadow.lname;			    \
+	return __##lname##_controls_get(vmx->loaded_vmcs);		    \
 }									    \
 static inline void lname##_controls_setbit(struct vcpu_vmx *vmx, u32 val)   \
 {									    \
-- 
2.32.0.605.g8dce9f2422-goog


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

* [PATCH 3/4] KVM: VMX: Drop caching of KVM's desired sec exec controls for vmcs01
  2021-08-10 17:19 [PATCH 0/4] KVM: nVMX: Use vmcs01 ctrls shadow as basis for vmcs02 Sean Christopherson
  2021-08-10 17:19 ` [PATCH 1/4] KVM: VMX: Use current VMCS to query WAITPKG support for MSR emulation Sean Christopherson
  2021-08-10 17:19 ` [PATCH 2/4] KVM: nVMX: Pull KVM L0's desired controls directly from vmcs01 Sean Christopherson
@ 2021-08-10 17:19 ` Sean Christopherson
  2021-08-10 17:19 ` [PATCH 4/4] KVM: VMX: Hide VMCS control calculators in vmx.c Sean Christopherson
  2021-08-10 17:39 ` [PATCH 0/4] KVM: nVMX: Use vmcs01 ctrls shadow as basis for vmcs02 Paolo Bonzini
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2021-08-10 17:19 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Zeng Guang

Remove the secondary execution controls cache now that it's effectively
dead code; it is only read immediately after it is written.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 20 ++++++++------------
 arch/x86/kvm/vmx/vmx.h |  3 +--
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index ae8e62df16dd..6c93122363b2 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -4212,7 +4212,7 @@ vmx_adjust_secondary_exec_control(struct vcpu_vmx *vmx, u32 *exec_control,
 #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)
+u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
 {
 	struct kvm_vcpu *vcpu = &vmx->vcpu;
 
@@ -4298,7 +4298,7 @@ static void vmx_compute_secondary_exec_control(struct vcpu_vmx *vmx)
 	if (!vcpu->kvm->arch.bus_lock_detection_enabled)
 		exec_control &= ~SECONDARY_EXEC_BUS_LOCK_DETECTION;
 
-	vmx->secondary_exec_control = exec_control;
+	return exec_control;
 }
 
 #define VMX_XSS_EXIT_BITMAP 0
@@ -4322,10 +4322,8 @@ static void init_vmcs(struct vcpu_vmx *vmx)
 
 	exec_controls_set(vmx, vmx_exec_control(vmx));
 
-	if (cpu_has_secondary_exec_ctrls()) {
-		vmx_compute_secondary_exec_control(vmx);
-		secondary_exec_controls_set(vmx, vmx->secondary_exec_control);
-	}
+	if (cpu_has_secondary_exec_ctrls())
+		secondary_exec_controls_set(vmx, vmx_secondary_exec_control(vmx));
 
 	if (kvm_vcpu_apicv_active(&vmx->vcpu)) {
 		vmcs_write64(EOI_EXIT_BITMAP0, 0);
@@ -6982,7 +6980,7 @@ static u64 vmx_get_mt_mask(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio)
 	return (cache << VMX_EPT_MT_EPTE_SHIFT) | ipat;
 }
 
-static void vmcs_set_secondary_exec_control(struct vcpu_vmx *vmx)
+static void vmcs_set_secondary_exec_control(struct vcpu_vmx *vmx, u32 new_ctl)
 {
 	/*
 	 * These bits in the secondary execution controls field
@@ -6996,7 +6994,6 @@ static void vmcs_set_secondary_exec_control(struct vcpu_vmx *vmx)
 		SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES |
 		SECONDARY_EXEC_DESC;
 
-	u32 new_ctl = vmx->secondary_exec_control;
 	u32 cur_ctl = secondary_exec_controls_get(vmx);
 
 	secondary_exec_controls_set(vmx, (new_ctl & ~mask) | (cur_ctl & mask));
@@ -7141,10 +7138,9 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
 
 	vmx_setup_uret_msrs(vmx);
 
-	if (cpu_has_secondary_exec_ctrls()) {
-		vmx_compute_secondary_exec_control(vmx);
-		vmcs_set_secondary_exec_control(vmx);
-	}
+	if (cpu_has_secondary_exec_ctrls())
+		vmcs_set_secondary_exec_control(vmx,
+						vmx_secondary_exec_control(vmx));
 
 	if (nested_vmx_allowed(vcpu))
 		to_vmx(vcpu)->msr_ia32_feature_control_valid_bits |=
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 414b440de9ac..2bd07867e9da 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -263,8 +263,6 @@ struct vcpu_vmx {
 	u64		      spec_ctrl;
 	u32		      msr_ia32_umwait_control;
 
-	u32 secondary_exec_control;
-
 	/*
 	 * loaded_vmcs points to the VMCS currently used in this vcpu. For a
 	 * non-nested (L1) guest, it always points to vmcs01. For a nested
@@ -477,6 +475,7 @@ static inline u32 vmx_vmexit_ctrl(void)
 }
 
 u32 vmx_exec_control(struct vcpu_vmx *vmx);
+u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx);
 u32 vmx_pin_based_exec_ctrl(struct vcpu_vmx *vmx);
 
 static inline struct kvm_vmx *to_kvm_vmx(struct kvm *kvm)
-- 
2.32.0.605.g8dce9f2422-goog


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

* [PATCH 4/4] KVM: VMX: Hide VMCS control calculators in vmx.c
  2021-08-10 17:19 [PATCH 0/4] KVM: nVMX: Use vmcs01 ctrls shadow as basis for vmcs02 Sean Christopherson
                   ` (2 preceding siblings ...)
  2021-08-10 17:19 ` [PATCH 3/4] KVM: VMX: Drop caching of KVM's desired sec exec controls for vmcs01 Sean Christopherson
@ 2021-08-10 17:19 ` Sean Christopherson
  2021-08-10 17:39 ` [PATCH 0/4] KVM: nVMX: Use vmcs01 ctrls shadow as basis for vmcs02 Paolo Bonzini
  4 siblings, 0 replies; 6+ messages in thread
From: Sean Christopherson @ 2021-08-10 17:19 UTC (permalink / raw)
  To: Paolo Bonzini
  Cc: Sean Christopherson, Vitaly Kuznetsov, Wanpeng Li, Jim Mattson,
	Joerg Roedel, kvm, linux-kernel, Zeng Guang

Now that nested VMX pulls KVM's desired VMCS controls from vmcs01 instead
of re-calculating on the fly, bury the helpers that do the calcluations
in vmx.c.

No functional change intended.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 arch/x86/kvm/vmx/vmx.c | 30 +++++++++++++++++++++++++++---
 arch/x86/kvm/vmx/vmx.h | 26 --------------------------
 2 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 6c93122363b2..c771575a8c9c 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -4095,7 +4095,7 @@ void set_cr4_guest_host_mask(struct vcpu_vmx *vmx)
 	vmcs_writel(CR4_GUEST_HOST_MASK, ~vcpu->arch.cr4_guest_owned_bits);
 }
 
-u32 vmx_pin_based_exec_ctrl(struct vcpu_vmx *vmx)
+static u32 vmx_pin_based_exec_ctrl(struct vcpu_vmx *vmx)
 {
 	u32 pin_based_exec_ctrl = vmcs_config.pin_based_exec_ctrl;
 
@@ -4111,6 +4111,30 @@ u32 vmx_pin_based_exec_ctrl(struct vcpu_vmx *vmx)
 	return pin_based_exec_ctrl;
 }
 
+static u32 vmx_vmentry_ctrl(void)
+{
+	u32 vmentry_ctrl = vmcs_config.vmentry_ctrl;
+
+	if (vmx_pt_mode_is_system())
+		vmentry_ctrl &= ~(VM_ENTRY_PT_CONCEAL_PIP |
+				  VM_ENTRY_LOAD_IA32_RTIT_CTL);
+	/* Loading of EFER and PERF_GLOBAL_CTRL are toggled dynamically */
+	return vmentry_ctrl &
+		~(VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL | VM_ENTRY_LOAD_IA32_EFER);
+}
+
+static u32 vmx_vmexit_ctrl(void)
+{
+	u32 vmexit_ctrl = vmcs_config.vmexit_ctrl;
+
+	if (vmx_pt_mode_is_system())
+		vmexit_ctrl &= ~(VM_EXIT_PT_CONCEAL_PIP |
+				 VM_EXIT_CLEAR_IA32_RTIT_CTL);
+	/* Loading of EFER and PERF_GLOBAL_CTRL are toggled dynamically */
+	return vmexit_ctrl &
+		~(VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL | VM_EXIT_LOAD_IA32_EFER);
+}
+
 static void vmx_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
 {
 	struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -4130,7 +4154,7 @@ static void vmx_refresh_apicv_exec_ctrl(struct kvm_vcpu *vcpu)
 	vmx_update_msr_bitmap_x2apic(vcpu);
 }
 
-u32 vmx_exec_control(struct vcpu_vmx *vmx)
+static u32 vmx_exec_control(struct vcpu_vmx *vmx)
 {
 	u32 exec_control = vmcs_config.cpu_based_exec_ctrl;
 
@@ -4212,7 +4236,7 @@ vmx_adjust_secondary_exec_control(struct vcpu_vmx *vmx, u32 *exec_control,
 #define vmx_adjust_sec_exec_exiting(vmx, exec_control, lname, uname) \
 	vmx_adjust_sec_exec_control(vmx, exec_control, lname, uname, uname##_EXITING, true)
 
-u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
+static u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx)
 {
 	struct kvm_vcpu *vcpu = &vmx->vcpu;
 
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index 2bd07867e9da..4858c5fd95f2 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -452,32 +452,6 @@ static inline void vmx_register_cache_reset(struct kvm_vcpu *vcpu)
 	vcpu->arch.regs_dirty = 0;
 }
 
-static inline u32 vmx_vmentry_ctrl(void)
-{
-	u32 vmentry_ctrl = vmcs_config.vmentry_ctrl;
-	if (vmx_pt_mode_is_system())
-		vmentry_ctrl &= ~(VM_ENTRY_PT_CONCEAL_PIP |
-				  VM_ENTRY_LOAD_IA32_RTIT_CTL);
-	/* Loading of EFER and PERF_GLOBAL_CTRL are toggled dynamically */
-	return vmentry_ctrl &
-		~(VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL | VM_ENTRY_LOAD_IA32_EFER);
-}
-
-static inline u32 vmx_vmexit_ctrl(void)
-{
-	u32 vmexit_ctrl = vmcs_config.vmexit_ctrl;
-	if (vmx_pt_mode_is_system())
-		vmexit_ctrl &= ~(VM_EXIT_PT_CONCEAL_PIP |
-				 VM_EXIT_CLEAR_IA32_RTIT_CTL);
-	/* Loading of EFER and PERF_GLOBAL_CTRL are toggled dynamically */
-	return vmexit_ctrl &
-		~(VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL | VM_EXIT_LOAD_IA32_EFER);
-}
-
-u32 vmx_exec_control(struct vcpu_vmx *vmx);
-u32 vmx_secondary_exec_control(struct vcpu_vmx *vmx);
-u32 vmx_pin_based_exec_ctrl(struct vcpu_vmx *vmx);
-
 static inline struct kvm_vmx *to_kvm_vmx(struct kvm *kvm)
 {
 	return container_of(kvm, struct kvm_vmx, kvm);
-- 
2.32.0.605.g8dce9f2422-goog


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

* Re: [PATCH 0/4] KVM: nVMX: Use vmcs01 ctrls shadow as basis for vmcs02
  2021-08-10 17:19 [PATCH 0/4] KVM: nVMX: Use vmcs01 ctrls shadow as basis for vmcs02 Sean Christopherson
                   ` (3 preceding siblings ...)
  2021-08-10 17:19 ` [PATCH 4/4] KVM: VMX: Hide VMCS control calculators in vmx.c Sean Christopherson
@ 2021-08-10 17:39 ` Paolo Bonzini
  4 siblings, 0 replies; 6+ messages in thread
From: Paolo Bonzini @ 2021-08-10 17:39 UTC (permalink / raw)
  To: Sean Christopherson
  Cc: Vitaly Kuznetsov, Wanpeng Li, Jim Mattson, Joerg Roedel, kvm,
	linux-kernel, Zeng Guang

On 10/08/21 19:19, Sean Christopherson wrote:
> The goal of this series is to drop the vmx->secondary_exec_control cache
> without degrading nested VM-Enter performance.  The cache is effective,
> e.g. saves ~1000 cycles on nested VM-Enter, but confusing.  The worst of
> the confusion could be eliminated by returning the computed value from
> vmx_compute_secondary_exec_control() to make the calls to the "compute"
> helper more like the other controls.  But, the nested VM-Enter path would
> still have special handling for secondary exec controls, and ideally all
> controls would benefit from caching, though the benefits are marginal for
> other controls and thus difficult to justify.
> 
> Happily, vmcs01 already caches the calculated controls in the
> controls_shadow.  The only issue is that the controls_shadow may have
> dynamically toggled bits set.  However, that is not a fundamental problem,
> it's simply different than what is expected by the nested VM-Enter code
> and is easily remedied.
> 
> TL;DR: Get KVM's (L0's) desires for vmcs02 controls from vmcs01's
> controls_shadow instead of recalculating the desired controls on every
> nested VM-Enter, thus eliminating the need to have a dedicated cache for
> the secondary exec controls calulation.
> 
> Sean Christopherson (4):
>    KVM: VMX: Use current VMCS to query WAITPKG support for MSR emulation
>    KVM: nVMX: Pull KVM L0's desired controls directly from vmcs01
>    KVM: VMX: Drop caching of KVM's desired sec exec controls for vmcs01
>    KVM: VMX: Hide VMCS control calculators in vmx.c
> 
>   arch/x86/kvm/vmx/nested.c | 25 ++++++++++++--------
>   arch/x86/kvm/vmx/vmx.c    | 48 +++++++++++++++++++++++++++------------
>   arch/x86/kvm/vmx/vmx.h    | 35 +++++-----------------------
>   3 files changed, 56 insertions(+), 52 deletions(-)
> 

Queued, thanks (patch 1 for 5.14, the rest for 5.15).

Paolo


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

end of thread, other threads:[~2021-08-10 17:43 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-10 17:19 [PATCH 0/4] KVM: nVMX: Use vmcs01 ctrls shadow as basis for vmcs02 Sean Christopherson
2021-08-10 17:19 ` [PATCH 1/4] KVM: VMX: Use current VMCS to query WAITPKG support for MSR emulation Sean Christopherson
2021-08-10 17:19 ` [PATCH 2/4] KVM: nVMX: Pull KVM L0's desired controls directly from vmcs01 Sean Christopherson
2021-08-10 17:19 ` [PATCH 3/4] KVM: VMX: Drop caching of KVM's desired sec exec controls for vmcs01 Sean Christopherson
2021-08-10 17:19 ` [PATCH 4/4] KVM: VMX: Hide VMCS control calculators in vmx.c Sean Christopherson
2021-08-10 17:39 ` [PATCH 0/4] KVM: nVMX: Use vmcs01 ctrls shadow as basis for vmcs02 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).