Skip to contentSkip to conversation

KVM: nVMX: Add full nested support for "load IA32_PERF_GLOBAL_CTRL" VM-{Entry,Exit} control

PATCH Applied Oliver Upton 2 participants 11 messages Nov 14, 2019 – Nov 19, 2019

Subsystems (3)
x86 KVM KVM x86
lore
Share & tools
lore

Oliver Upton

author
#0 lore raw

v5 cover KVM: nVMX: Add full nested support for "load IA32_PERF_GLOBAL_CTRL" VM-{Entry,Exit} control

[v1] https://lore.kernel.org/r/[email protected]
[v2] https://lore.kernel.org/r/[email protected]
[v3] https://lore.kernel.org/r/[email protected]
[v4] https://lore.kernel.org/r/[email protected]

v1 => v2:
- Add Krish's Co-developed-by and Signed-off-by tags.
- Fix minor nit to kvm-unit-tests to use 'host' local variable throughout test_load_pgc()
- Teach guest_state_test_main() to check guest state from within nested VM
- Update proposed tests to use guest/host state checks, wherein the value is checked from MSR_CORE_PERF_GLOBAL_CTRL.
- Changelog line wrapping

v2 => v3:
- Remove the value unchanged condition from
kvm_is_valid_perf_global_ctrl
- Add line to changelog for patch 3/8

v3 => v4:
- Allow tests to set the guest func multiple times
- Style fixes throughout kvm-unit-tests patches, per Krish's review

v4 => v5:
- Rebased kernel and kvm-unit-tests patches
- Reordered and reworked patches to now WARN on a failed kvm_set_msr()
- Dropped patch to alow resetting guest in kvm-unit-tests, as the functionality is no longer needed.

This patchset exposes the "load IA32_PERF_GLOBAL_CTRL" to guests for nested VM-entry and VM-exit. There already was some existing code that supported the VM-exit ctrl, though it had an issue and was not exposed to the guest anyway. These patches are based on the original set that Krish Sadhukhan sent out earlier this year.

Oliver Upton (6):
KVM: nVMX: Expose load IA32_PERF_GLOBAL_CTRL VM-{Entry,Exit} control
KVM: nVMX: Load GUEST_IA32_PERF_GLOBAL_CTRL MSR on VM-Entry
KVM: nVMX: Use kvm_set_msr to load IA32_PERF_GLOBAL_CTRL on VM-Exit
KVM: nVMX: Check HOST_IA32_PERF_GLOBAL_CTRL on VM-Entry
KVM: nVMX: Check GUEST_IA32_PERF_GLOBAL_CTRL on VM-Entry
KVM: VMX: Add helper to check reserved bits in IA32_PERF_GLOBAL_CTRL

 arch/x86/kvm/pmu.h           |  6 ++++++
 arch/x86/kvm/vmx/nested.c    | 51 +++++++++++++++++++++++++++++++++++++++++++++++++--
 arch/x86/kvm/vmx/nested.h    |  1 +
 arch/x86/kvm/vmx/pmu_intel.c |  5 ++++-
Signature · 1 line
2.23.0.187.g17f5b7556c-goog
author
#1 lore raw

PATCH v5 1/8 KVM: VMX: Add helper to check reserved bits in IA32_PERF_GLOBAL_CTRL

Create a helper function to check the validity of a proposed value for IA32_PERF_GLOBAL_CTRL from the existing check in intel_pmu_set_msr().

Per Intel's SDM, the reserved bits in IA32_PERF_GLOBAL_CTRL must be cleared for the corresponding host/guest state fields.

Suggested-by Jim Mattson <[email protected]>
Co-developed-by Krish Sadhukhan <[email protected]>
Signed-off-by Krish Sadhukhan <[email protected]>
Signed-off-by Oliver Upton <[email protected]>
Reviewed-by Jim Mattson <[email protected]>
Reviewed-by Peter Shier <[email protected]>
---
 arch/x86/kvm/pmu.h           | 6 ++++++
 arch/x86/kvm/vmx/pmu_intel.c | 2 +-
 2 files changed, 7 insertions(+), 1 deletion(-)
arch/x86/kvm/pmu.h +6 -0
index 58265f761c3b..1631ac852ce0 100644
@@ -79,6 +79,12 @@ static inline bool pmc_is_enabled(struct kvm_pmc *pmc)
 	return kvm_x86_ops->pmu_ops->pmc_is_enabled(pmc);
 }
 
+static inline bool kvm_valid_perf_global_ctrl(struct kvm_pmu *pmu,
+						 u64 data)
+{
+	return !(pmu->global_ctrl_mask & data);
+}
+
 /* returns general purpose PMC with the specified MSR. Note that it can be
  * used for both PERFCTRn and EVNTSELn; that is why it accepts base as a
  * paramenter to tell them apart.
arch/x86/kvm/vmx/pmu_intel.c +1 -1
index 3e9c059099e9..8cd2cc2fe986 100644
@@ -223,7 +223,7 @@ static int intel_pmu_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
 	case MSR_CORE_PERF_GLOBAL_CTRL:
 		if (pmu->global_ctrl == data)
 			return 0;
-		if (!(data & pmu->global_ctrl_mask)) {
+		if (kvm_valid_perf_global_ctrl(pmu, data)) {
 			global_ctrl_changed(pmu, data);
 			return 0;
 		}
Signature · 1 line
2.24.0.rc1.363.gb1bccd3e3d-goog
author
#2 lore raw

PATCH v5 2/8 KVM: nVMX: Check GUEST_IA32_PERF_GLOBAL_CTRL on VM-Entry

In reply to Oliver Upton

Add condition to nested_vmx_check_guest_state() to check the validity of GUEST_IA32_PERF_GLOBAL_CTRL. Per Intel's SDM Vol 3 26.3.1.1:

If the "load IA32_PERF_GLOBAL_CTRL" VM-entry control is 1, bits reserved in the IA32_PERF_GLOBAL_CTRL MSR must be 0 in the field for that register.

Suggested-by Jim Mattson <[email protected]>
Co-developed-by Krish Sadhukhan <[email protected]>
Signed-off-by Krish Sadhukhan <[email protected]>
Signed-off-by Oliver Upton <[email protected]>
Reviewed-by Jim Mattson <[email protected]>
Reviewed-by Peter Shier <[email protected]>
---
 arch/x86/kvm/vmx/nested.c | 6 ++++++
 1 file changed, 6 insertions(+)
arch/x86/kvm/vmx/nested.c +6 -0
index e76eb4f07f6c..c19975c57d69 100644
@@ -10,6 +10,7 @@
 #include "hyperv.h"
 #include "mmu.h"
 #include "nested.h"
+#include "pmu.h"
 #include "trace.h"
 #include "x86.h"
 

@@ -2779,6 +2780,11 @@ static int nested_vmx_check_guest_state(struct kvm_vcpu *vcpu,
 		return -EINVAL;
 	}
 
+	if ((vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL) &&
+	    CC(!kvm_valid_perf_global_ctrl(vcpu_to_pmu(vcpu),
+					   vmcs12->guest_ia32_perf_global_ctrl)))
+		return -EINVAL;
+
 	/*
 	 * If the load IA32_EFER VM-entry control is 1, the following checks
 	 * are performed on the field for the IA32_EFER MSR:
Signature · 1 line
2.24.0.rc1.363.gb1bccd3e3d-goog
author
#3 lore raw

PATCH v5 3/8 KVM: nVMX: Check HOST_IA32_PERF_GLOBAL_CTRL on VM-Entry

In reply to Oliver Upton

Add a consistency check on nested vm-entry for host's IA32_PERF_GLOBAL_CTRL from vmcs12. Per Intel's SDM Vol 3 26.2.2:

If the "load IA32_PERF_GLOBAL_CTRL"
VM-exit control is 1, bits reserved in the IA32_PERF_GLOBAL_CTRL MSR must be 0 in the field for that register"

Suggested-by Jim Mattson <[email protected]>
Co-developed-by Krish Sadhukhan <[email protected]>
Signed-off-by Krish Sadhukhan <[email protected]>
Signed-off-by Oliver Upton <[email protected]>
Reviewed-by Jim Mattson <[email protected]>
Reviewed-by Peter Shier <[email protected]>
---
 arch/x86/kvm/vmx/nested.c | 5 +++++
 1 file changed, 5 insertions(+)
arch/x86/kvm/vmx/nested.c +5 -0
index c19975c57d69..f9ae7bc0a421 100644
@@ -2665,6 +2665,11 @@ static int nested_vmx_check_host_state(struct kvm_vcpu *vcpu,
 	    CC(!kvm_pat_valid(vmcs12->host_ia32_pat)))
 		return -EINVAL;
 
+	if ((vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL) &&
+	    CC(!kvm_valid_perf_global_ctrl(vcpu_to_pmu(vcpu),
+					   vmcs12->host_ia32_perf_global_ctrl)))
+		return -EINVAL;
+
 #ifdef CONFIG_X86_64
 	ia32e = !!(vcpu->arch.efer & EFER_LMA);
 #else
Signature · 1 line
2.24.0.rc1.363.gb1bccd3e3d-goog
author
#4 lore raw

PATCH v5 4/8 KVM: nVMX: Use kvm_set_msr to load IA32_PERF_GLOBAL_CTRL on VM-Exit

In reply to Oliver Upton

The existing implementation for loading the IA32_PERF_GLOBAL_CTRL MSR on VM-exit was incorrect, as the next call to atomic_switch_perf_msrs() could cause this value to be overwritten. Instead, call kvm_set_msr() which will allow atomic_switch_perf_msrs() to correctly set the values.

Define a macro, SET_MSR_OR_WARN(), to set the MSR with kvm_set_msr() and WARN on failure.

Suggested-by Jim Mattson <[email protected]>
Co-developed-by Krish Sadhukhan <[email protected]>
Signed-off-by Krish Sadhukhan <[email protected]>
Signed-off-by Oliver Upton <[email protected]>
Reviewed-by Jim Mattson <[email protected]>
Reviewed-by Peter Shier <[email protected]>
---
 arch/x86/kvm/vmx/nested.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
arch/x86/kvm/vmx/nested.c +12 -2
index f9ae7bc0a421..ecdc706f171b 100644
@@ -28,6 +28,16 @@ module_param(nested_early_check, bool, S_IRUGO);
 	failed;								\
 })
 
+#define SET_MSR_OR_WARN(vcpu, idx, data)				\
+({									\
+	bool failed = kvm_set_msr(vcpu, idx, data);			\
+	if (failed)							\
+		pr_warn_ratelimited(					\
+				"%s cannot write MSR (0x%x, 0x%llx)\n",	\
+				__func__, idx, data);			\
+	failed;								\
+})
+
 /*
  * Hyper-V requires all of these, so mark them as supported even though
  * they are just treated the same as all-context.

@@ -3867,8 +3877,8 @@ static void load_vmcs12_host_state(struct kvm_vcpu *vcpu,
 		vcpu->arch.pat = vmcs12->host_ia32_pat;
 	}
 	if (vmcs12->vm_exit_controls & VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL)
-		vmcs_write64(GUEST_IA32_PERF_GLOBAL_CTRL,
-			vmcs12->host_ia32_perf_global_ctrl);
+		SET_MSR_OR_WARN(vcpu, MSR_CORE_PERF_GLOBAL_CTRL,
+				vmcs12->host_ia32_perf_global_ctrl);
 
 	/* Set L1 segment info according to Intel SDM
 	    27.5.2 Loading Host Segment and Descriptor-Table Registers */
Signature · 1 line
2.24.0.rc1.363.gb1bccd3e3d-goog
author
#5 lore raw

PATCH v5 5/8 KVM: nVMX: Load GUEST_IA32_PERF_GLOBAL_CTRL MSR on VM-Entry

In reply to Oliver Upton

Add condition to prepare_vmcs02 which loads IA32_PERF_GLOBAL_CTRL on VM-entry if the "load IA32_PERF_GLOBAL_CTRL" bit on the VM-entry control is set. Use SET_MSR_OR_WARN() rather than directly writing to the field to avoid overwrite by atomic_switch_perf_msrs().

Suggested-by Jim Mattson <[email protected]>
Co-developed-by Krish Sadhukhan <[email protected]>
Signed-off-by Krish Sadhukhan <[email protected]>
Signed-off-by Oliver Upton <[email protected]>
Reviewed-by Jim Mattson <[email protected]>
Reviewed-by Peter Shier <[email protected]>
---
 arch/x86/kvm/vmx/nested.c | 5 +++++
 1 file changed, 5 insertions(+)
arch/x86/kvm/vmx/nested.c +5 -0
index ecdc706f171b..64e15c6f6944 100644
@@ -2441,6 +2441,11 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12,
 	if (!enable_ept)
 		vcpu->arch.walk_mmu->inject_page_fault = vmx_inject_page_fault_nested;
 
+	if ((vmcs12->vm_entry_controls & VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL) &&
+	    SET_MSR_OR_WARN(vcpu, MSR_CORE_PERF_GLOBAL_CTRL,
+			    vmcs12->guest_ia32_perf_global_ctrl))
+		return -EINVAL;
+
 	kvm_rsp_write(vcpu, vmcs12->guest_rsp);
 	kvm_rip_write(vcpu, vmcs12->guest_rip);
 	return 0;
Signature · 1 line
2.24.0.rc1.363.gb1bccd3e3d-goog
author
#6 lore raw

PATCH v5 6/8 KVM: nVMX: Expose load IA32_PERF_GLOBAL_CTRL VM-{Entry,Exit} control

In reply to Oliver Upton

The "load IA32_PERF_GLOBAL_CTRL" bit for VM-entry and VM-exit should only be exposed to the guest if IA32_PERF_GLOBAL_CTRL is a valid MSR. Create a new helper to allow pmu_refresh() to update the VM-Entry and VM-Exit controls to ensure PMU values are initialized when performing the is_valid_msr() check.

Suggested-by Jim Mattson <[email protected]>
Co-developed-by Krish Sadhukhan <[email protected]>
Signed-off-by Krish Sadhukhan <[email protected]>
Signed-off-by Oliver Upton <[email protected]>
Reviewed-by Jim Mattson <[email protected]>
Reviewed-by Peter Shier <[email protected]>
---
 arch/x86/kvm/vmx/nested.c    | 21 +++++++++++++++++++++
 arch/x86/kvm/vmx/nested.h    |  1 +
 arch/x86/kvm/vmx/pmu_intel.c |  3 +++
 3 files changed, 25 insertions(+)
arch/x86/kvm/vmx/nested.c +21 -0
index 64e15c6f6944..cc5297d3310f 100644
@@ -4345,6 +4345,27 @@ int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification,
 	return 0;
 }
 
+void nested_vmx_pmu_entry_exit_ctls_update(struct kvm_vcpu *vcpu)
+{
+	struct vcpu_vmx *vmx;
+
+	if (!nested_vmx_allowed(vcpu))
+		return;
+
+	vmx = to_vmx(vcpu);
+	if (kvm_x86_ops->pmu_ops->is_valid_msr(vcpu, MSR_CORE_PERF_GLOBAL_CTRL)) {
+		vmx->nested.msrs.entry_ctls_high |=
+				VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL;
+		vmx->nested.msrs.exit_ctls_high |=
+				VM_EXIT_LOAD_IA32_PERF_GLOBAL_CTRL;
+	} else {
+		vmx->nested.msrs.entry_ctls_high &=
+				~VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL;
+		vmx->nested.msrs.exit_ctls_high &=
+				~VM_ENTRY_LOAD_IA32_PERF_GLOBAL_CTRL;
+	}
+}
+
 static int nested_vmx_get_vmptr(struct kvm_vcpu *vcpu, gpa_t *vmpointer)
 {
 	gva_t gva;
arch/x86/kvm/vmx/nested.h +1 -0
index 187d39bf0bf1..440bc08e0a2f 100644
@@ -22,6 +22,7 @@ int vmx_set_vmx_msr(struct kvm_vcpu *vcpu, u32 msr_index, u64 data);
 int vmx_get_vmx_msr(struct nested_vmx_msrs *msrs, u32 msr_index, u64 *pdata);
 int get_vmx_mem_address(struct kvm_vcpu *vcpu, unsigned long exit_qualification,
 			u32 vmx_instruction_info, bool wr, int len, gva_t *ret);
+void nested_vmx_pmu_entry_exit_ctls_update(struct kvm_vcpu *vcpu);
 
 static inline struct vmcs12 *get_vmcs12(struct kvm_vcpu *vcpu)
 {
arch/x86/kvm/vmx/pmu_intel.c +3 -0
index 8cd2cc2fe986..b7b2fcdf97ff 100644
@@ -15,6 +15,7 @@
 #include "x86.h"
 #include "cpuid.h"
 #include "lapic.h"
+#include "nested.h"
 #include "pmu.h"
 
 static struct kvm_event_hw_type_mapping intel_arch_events[] = {

@@ -317,6 +318,8 @@ static void intel_pmu_refresh(struct kvm_vcpu *vcpu)
 	    (boot_cpu_has(X86_FEATURE_HLE) || boot_cpu_has(X86_FEATURE_RTM)) &&
 	    (entry->ebx & (X86_FEATURE_HLE|X86_FEATURE_RTM)))
 		pmu->reserved_bits ^= HSW_IN_TX|HSW_IN_TX_CHECKPOINTED;
+
+	nested_vmx_pmu_entry_exit_ctls_update(vcpu);
 }
 
 static void intel_pmu_init(struct kvm_vcpu *vcpu)
Signature · 1 line
2.24.0.rc1.363.gb1bccd3e3d-goog
author
#7 lore raw

PATCH v5 7/8 x86: VMX: Make guest_state_test_main() check state from nested VM

In reply to Oliver Upton

Introduce the vmx_state_area_test_data struct for sharing test expectation data with the nested VM.

Signed-off-by Oliver Upton <[email protected]>
Reviewed-by Krish Sadhukhan <[email protected]>
---
 x86/vmx_tests.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)
x86/vmx_tests.c +18 -3
index 1d8932fad12b..95c1c01d2966 100644
@@ -5023,13 +5023,28 @@ static void test_entry_msr_load(void)
 	test_vmx_valid_controls();
 }
 
+static struct vmx_state_area_test_data {
+	u32 msr;
+	u64 exp;
+	bool enabled;
+} vmx_state_area_test_data;
+
 static void guest_state_test_main(void)
 {
+	u64 obs;
+	struct vmx_state_area_test_data *data = &vmx_state_area_test_data;
+
 	while (1) {
-		if (vmx_get_test_stage() != 2)
-			vmcall();
-		else
+		if (vmx_get_test_stage() == 2)
 			break;
+
+		if (data->enabled) {
+			obs = rdmsr(data->msr);
+			report("Guest state is 0x%lx (expected 0x%lx)",
+			       data->exp == obs, obs, data->exp);
+		}
+
+		vmcall();
 	}
 
 	asm volatile("fnop");
Signature · 1 line
2.24.0.rc1.363.gb1bccd3e3d-goog
author
#8 lore raw

PATCH v5 8/8 x86: VMX: Add tests for nested "load IA32_PERF_GLOBAL_CTRL"

In reply to Oliver Upton

Tests to verify that KVM performs the correct checks on Host/Guest state at VM-entry, as described in SDM 26.3.1.1 "Checks on Guest Control Registers, Debug Registers, and MSRs" and SDM 26.2.2 "Checks on Host Control Registers and MSRs".

Test that KVM does the following:

If the "load IA32_PERF_GLOBAL_CTRL" VM-entry control is 1, the reserved bits of the IA32_PERF_GLOBAL_CTRL MSR must be 0 in the GUEST_IA32_PERF_GLOBAL_CTRL VMCS field. Otherwise, the VM-entry should fail with an exit reason of "VM-entry failure due to invalid guest state" (33). On a successful VM-entry, the correct value should be observed when the nested VM performs an RDMSR on IA32_PERF_GLOBAL_CTRL.

If the "load IA32_PERF_GLOBAL_CTRL" VM-exit control is 1, the reserved bits of the IA32_PERF_GLOBAL_CTRL MSR must be 0 in the HOST_IA32_PERF_GLOBAL_CTRL VMCS field. Otherwise, the VM-entry should fail with a VM-instruction error of "VM entry with invalid host-state field(s)" (8). On a successful VM-exit, the correct value should be observed when L1 performs an RDMSR on IA32_PERF_GLOBAL_CTRL.

Suggested-by Jim Mattson <[email protected]>
Co-developed-by Krish Sadhukhan <[email protected]>
Signed-off-by Krish Sadhukhan <[email protected]>
Signed-off-by Oliver Upton <[email protected]>
Reviewed-by Krish Sadhukhan <[email protected]>
---
 x86/vmx_tests.c | 160 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 160 insertions(+)
x86/vmx_tests.c +160 -0
index 95c1c01d2966..0d5e463f9887 100644
@@ -7032,6 +7032,164 @@ static void test_load_host_pat(void)
 	test_pat(HOST_PAT, "HOST_PAT", EXI_CONTROLS, EXI_LOAD_PAT);
 }
 
+union cpuidA_eax {
+	struct {
+		unsigned int version_id:8;
+		unsigned int num_counters_gp:8;
+		unsigned int bit_width:8;
+		unsigned int mask_length:8;
+	} split;
+	unsigned int full;
+};
+
+union cpuidA_edx {
+	struct {
+		unsigned int num_counters_fixed:5;
+		unsigned int bit_width_fixed:8;
+		unsigned int reserved:9;
+	} split;
+	unsigned int full;
+};
+
+static bool valid_pgc(u64 val)
+{
+	struct cpuid id;
+	union cpuidA_eax eax;
+	union cpuidA_edx edx;
+	u64 mask;
+
+	id = cpuid(0xA);
+	eax.full = id.a;
+	edx.full = id.d;
+	mask = ~(((1ull << eax.split.num_counters_gp) - 1) |
+		 (((1ull << edx.split.num_counters_fixed) - 1) << 32));
+
+	return !(val & mask);
+}
+
+static void test_pgc_vmlaunch(u32 xerror, u32 xreason, bool xfail, bool host)
+{
+	u32 inst_err;
+	u64 obs;
+	bool success;
+	struct vmx_state_area_test_data *data = &vmx_state_area_test_data;
+
+	if (host) {
+		success = vmlaunch_succeeds();
+		obs = rdmsr(data->msr);
+		if (!success) {
+			inst_err = vmcs_read(VMX_INST_ERROR);
+			report("vmlaunch failed, VMX Inst Error is %d (expected %d)",
+			       xerror == inst_err, inst_err, xerror);
+		} else {
+			report("Host state is 0x%lx (expected 0x%lx)",
+			       !data->enabled || data->exp == obs, obs, data->exp);
+			report("vmlaunch succeeded", success != xfail);
+		}
+	} else {
+		if (xfail) {
+			enter_guest_with_invalid_guest_state();
+		} else {
+			enter_guest();
+		}
+		report_guest_state_test("load GUEST_PERF_GLOBAL_CTRL",
+					xreason, GUEST_PERF_GLOBAL_CTRL,
+					"GUEST_PERF_GLOBAL_CTRL");
+	}
+}
+
+/*
+ * test_load_perf_global_ctrl is a generic function for testing the
+ * "load IA32_PERF_GLOBAL_CTRL" VM-{Entry,Exit} controls. This test function
+ * tests the provided ctrl_val when disabled and enabled.
+ *
+ * @nr: VMCS field number corresponding to the host/guest state field
+ * @name: Name of the above VMCS field for printing in test report
+ * @ctrl_nr: VMCS field number corresponding to the VM-{Entry,Exit} control
+ * @ctrl_val: Bit to set on the ctrl_field
+ */
+static void test_perf_global_ctrl(u32 nr, const char *name, u32 ctrl_nr,
+				  const char *ctrl_name, u64 ctrl_val)
+{
+	u64 ctrl_saved = vmcs_read(ctrl_nr);
+	u64 pgc_saved = vmcs_read(nr);
+	u64 i, val;
+	bool host = nr == HOST_PERF_GLOBAL_CTRL;
+	struct vmx_state_area_test_data *data = &vmx_state_area_test_data;
+
+	data->msr = MSR_CORE_PERF_GLOBAL_CTRL;
+	msr_bmp_init();
+	vmcs_write(ctrl_nr, ctrl_saved & ~ctrl_val);
+	data->enabled = false;
+	report_prefix_pushf("\"load IA32_PERF_GLOBAL_CTRL\"=0 on %s",
+			    ctrl_name);
+
+	for (i = 0; i < 64; i++) {
+		val = 1ull << i;
+		vmcs_write(nr, val);
+		report_prefix_pushf("%s = 0x%lx", name, val);
+		test_pgc_vmlaunch(0, VMX_VMCALL, false, host);
+		report_prefix_pop();
+	}
+	report_prefix_pop();
+
+	vmcs_write(ctrl_nr, ctrl_saved | ctrl_val);
+	data->enabled = true;
+	report_prefix_pushf("\"load IA32_PERF_GLOBAL_CTRL\"=1 on %s",
+			    ctrl_name);
+	for (i = 0; i < 64; i++) {
+		val = 1ull << i;
+		data->exp = val;
+		vmcs_write(nr, val);
+		report_prefix_pushf("%s = 0x%lx", name, val);
+		if (valid_pgc(val)) {
+			test_pgc_vmlaunch(0, VMX_VMCALL, false, host);
+		} else {
+			if (host)
+				test_pgc_vmlaunch(
+					VMXERR_ENTRY_INVALID_HOST_STATE_FIELD,
+					0,
+					true,
+					host);
+			else
+				test_pgc_vmlaunch(
+					0,
+					VMX_ENTRY_FAILURE | VMX_FAIL_STATE,
+					true,
+					host);
+		}
+		report_prefix_pop();
+	}
+
+	report_prefix_pop();
+	vmcs_write(ctrl_nr, ctrl_saved);
+	vmcs_write(nr, pgc_saved);
+}
+
+static void test_load_host_perf_global_ctrl(void)
+{
+	if (!(ctrl_exit_rev.clr & EXI_LOAD_PERF)) {
+		printf("\"load IA32_PERF_GLOBAL_CTRL\" exit control not supported\n");
+		return;
+	}
+
+	test_perf_global_ctrl(HOST_PERF_GLOBAL_CTRL, "HOST_PERF_GLOBAL_CTRL",
+				   EXI_CONTROLS, "EXI_CONTROLS", EXI_LOAD_PERF);
+}
+
+
+static void test_load_guest_perf_global_ctrl(void)
+{
+	if (!(ctrl_enter_rev.clr & ENT_LOAD_PERF)) {
+		printf("\"load IA32_PERF_GLOBAL_CTRL\" entry control not supported\n");
+		return;
+	}
+
+	test_perf_global_ctrl(GUEST_PERF_GLOBAL_CTRL, "GUEST_PERF_GLOBAL_CTRL",
+				   ENT_CONTROLS, "ENT_CONTROLS", ENT_LOAD_PERF);
+}
+
+
 /*
  * test_vmcs_field - test a value for the given VMCS field
  * @field: VMCS field

@@ -7261,6 +7419,7 @@ static void vmx_host_state_area_test(void)
 	test_host_segment_regs();
 	test_host_desc_tables();
 	test_host_addr_size();
+	test_load_host_perf_global_ctrl();
 }
 
 /*

@@ -7296,6 +7455,7 @@ static void vmx_guest_state_area_test(void)
 
 	test_load_guest_pat();
 	test_guest_efer();
+	test_load_guest_perf_global_ctrl();
 
 	/*
 	 * Let the guest finish execution
Signature · 1 line
2.24.0.rc1.363.gb1bccd3e3d-goog
maintainer Applied
#9 lore raw
In reply to Oliver Upton
Quoted 45 lines · Oliver Upton

[v1] https://lore.kernel.org/r/[email protected]
[v2] https://lore.kernel.org/r/[email protected]
[v3] https://lore.kernel.org/r/[email protected]
[v4] https://lore.kernel.org/r/[email protected]

v1 => v2:
- Add Krish's Co-developed-by and Signed-off-by tags.
- Fix minor nit to kvm-unit-tests to use 'host' local variable throughout test_load_pgc()
- Teach guest_state_test_main() to check guest state from within nested VM
- Update proposed tests to use guest/host state checks, wherein the value is checked from MSR_CORE_PERF_GLOBAL_CTRL.
- Changelog line wrapping

v2 => v3:
- Remove the value unchanged condition from
kvm_is_valid_perf_global_ctrl
- Add line to changelog for patch 3/8

v3 => v4:
- Allow tests to set the guest func multiple times
- Style fixes throughout kvm-unit-tests patches, per Krish's review

v4 => v5:
- Rebased kernel and kvm-unit-tests patches
- Reordered and reworked patches to now WARN on a failed kvm_set_msr()
- Dropped patch to alow resetting guest in kvm-unit-tests, as the functionality is no longer needed.

This patchset exposes the "load IA32_PERF_GLOBAL_CTRL" to guests for nested VM-entry and VM-exit. There already was some existing code that supported the VM-exit ctrl, though it had an issue and was not exposed to the guest anyway. These patches are based on the original set that Krish Sadhukhan sent out earlier this year.

Oliver Upton (6):
KVM: nVMX: Expose load IA32_PERF_GLOBAL_CTRL VM-{Entry,Exit} control
KVM: nVMX: Load GUEST_IA32_PERF_GLOBAL_CTRL MSR on VM-Entry
KVM: nVMX: Use kvm_set_msr to load IA32_PERF_GLOBAL_CTRL on VM-Exit
KVM: nVMX: Check HOST_IA32_PERF_GLOBAL_CTRL on VM-Entry
KVM: nVMX: Check GUEST_IA32_PERF_GLOBAL_CTRL on VM-Entry
KVM: VMX: Add helper to check reserved bits in IA32_PERF_GLOBAL_CTRL

Quoted 5 lines
 arch/x86/kvm/pmu.h           |  6 ++++++
 arch/x86/kvm/vmx/nested.c    | 51 +++++++++++++++++++++++++++++++++++++++++++++++++--
 arch/x86/kvm/vmx/nested.h    |  1 +
 arch/x86/kvm/vmx/pmu_intel.c |  5 ++++-

Queued, thanks.

But I had to squash this in patch 8:

x86/vmx_tests.c +0 -0
index 3129385..b6233ae 100644
@@ -7161,6 +7161,7 @@ static void test_perf_global_ctrl(u32 nr, const

char *name, u32 ctrl_nr,
report_prefix_pop();
}

+ data->enabled = false;
report_prefix_pop();
vmcs_write(ctrl_nr, ctrl_saved);
vmcs_write(nr, pgc_saved);

and I'm not sure about how this could have worked for you.

Paolo

author
#10 lore raw
Quoted 70 lines · Paolo Bonzini
Quoted 45 lines · Oliver Upton

[v1] https://lore.kernel.org/r/[email protected]
[v2] https://lore.kernel.org/r/[email protected]
[v3] https://lore.kernel.org/r/[email protected]
[v4] https://lore.kernel.org/r/[email protected]

v1 => v2:
- Add Krish's Co-developed-by and Signed-off-by tags.
- Fix minor nit to kvm-unit-tests to use 'host' local variable throughout test_load_pgc()
- Teach guest_state_test_main() to check guest state from within nested VM
- Update proposed tests to use guest/host state checks, wherein the value is checked from MSR_CORE_PERF_GLOBAL_CTRL.
- Changelog line wrapping

v2 => v3:
- Remove the value unchanged condition from
kvm_is_valid_perf_global_ctrl
- Add line to changelog for patch 3/8

v3 => v4:
- Allow tests to set the guest func multiple times
- Style fixes throughout kvm-unit-tests patches, per Krish's review

v4 => v5:
- Rebased kernel and kvm-unit-tests patches
- Reordered and reworked patches to now WARN on a failed kvm_set_msr()
- Dropped patch to alow resetting guest in kvm-unit-tests, as the functionality is no longer needed.

This patchset exposes the "load IA32_PERF_GLOBAL_CTRL" to guests for nested VM-entry and VM-exit. There already was some existing code that supported the VM-exit ctrl, though it had an issue and was not exposed to the guest anyway. These patches are based on the original set that Krish Sadhukhan sent out earlier this year.

Oliver Upton (6):
KVM: nVMX: Expose load IA32_PERF_GLOBAL_CTRL VM-{Entry,Exit} control
KVM: nVMX: Load GUEST_IA32_PERF_GLOBAL_CTRL MSR on VM-Entry
KVM: nVMX: Use kvm_set_msr to load IA32_PERF_GLOBAL_CTRL on VM-Exit
KVM: nVMX: Check HOST_IA32_PERF_GLOBAL_CTRL on VM-Entry
KVM: nVMX: Check GUEST_IA32_PERF_GLOBAL_CTRL on VM-Entry
KVM: VMX: Add helper to check reserved bits in IA32_PERF_GLOBAL_CTRL

Quoted 5 lines
 arch/x86/kvm/pmu.h           |  6 ++++++
 arch/x86/kvm/vmx/nested.c    | 51 +++++++++++++++++++++++++++++++++++++++++++++++++--
 arch/x86/kvm/vmx/nested.h    |  1 +
 arch/x86/kvm/vmx/pmu_intel.c |  5 ++++-

Queued, thanks.

But I had to squash this in patch 8:

x86/vmx_tests.c +1 -0
index 3129385..b6233ae 100644
@@ -7161,6 +7161,7 @@ static void test_perf_global_ctrl(u32 nr, const
 char *name, u32 ctrl_nr,
 		report_prefix_pop();
 	}
 
+	data->enabled = false;
 	report_prefix_pop();
 	vmcs_write(ctrl_nr, ctrl_saved);

vmcs_write(nr, pgc_saved);

Quoted 3 lines

and I'm not sure about how this could have worked for you.

Paolo

Thanks for the fix. This was an oversight of mine as I had only run the tests I introduced individually. I'll be more thorough in future changes, apologies.

--
Best,
Oliver