diff mbox series

[v2,08/21] KVM: selftests: Split PMU caps sub-tests to avoid writing MSR after KVM_RUN

Message ID 20230210003148.2646712-9-seanjc@google.com
State New
Headers
Series KVM: x86: Disallow writes to feature MSRs post-KVM_RUN |

Commit Message

Sean Christopherson Feb. 10, 2023, 12:31 a.m. UTC 
  Split the PERF_CAPABILITIES subtests into two parts so that the LBR format
testcases don't execute after KVM_RUN.  Now that KVM disallows changing
PERF_CAPABILITIES after KVM_RUN (same as guest CPUID), attempting to set
the MSR after KVM_RUN will yield false positives and/or false negatives
depending on what the test is trying to do.

Land the LBR format test in a more generic "immutable features" test in
anticipation of expanding its scope to other immutable features.

Signed-off-by: Sean Christopherson <seanjc@google.com>
---
 .../selftests/kvm/x86_64/vmx_pmu_caps_test.c  | 51 +++++++++++--------
 1 file changed, 31 insertions(+), 20 deletions(-)

Comments

Like Xu Feb. 14, 2023, 7:27 a.m. UTC | #1 
Nit, could this patch be applied before the relevant KVM modifications [1] so that
a CI bot doesn't generate an error report before this selftests patch is applied ?

[1] KVM: x86: Disallow writes to immutable feature MSRs after KVM_RUN

On 10/2/2023 8:31 am, Sean Christopherson wrote:
> Split the PERF_CAPABILITIES subtests into two parts so that the LBR format
> testcases don't execute after KVM_RUN.  Now that KVM disallows changing
> PERF_CAPABILITIES after KVM_RUN (same as guest CPUID), attempting to set
> the MSR after KVM_RUN will yield false positives and/or false negatives
> depending on what the test is trying to do.
> 
> Land the LBR format test in a more generic "immutable features" test in
> anticipation of expanding its scope to other immutable features.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   .../selftests/kvm/x86_64/vmx_pmu_caps_test.c  | 51 +++++++++++--------
>   1 file changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c b/tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c
> index c280ba1e6572..ac08c0fdd84d 100644
> --- a/tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c
> +++ b/tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c
> @@ -41,24 +41,10 @@ static void guest_code(void)
>   	wrmsr(MSR_IA32_PERF_CAPABILITIES, PMU_CAP_LBR_FMT);
>   }
>   
> -int main(int argc, char *argv[])
> +static void test_fungible_perf_capabilities(union perf_capabilities host_cap)
>   {
> -	struct kvm_vm *vm;
>   	struct kvm_vcpu *vcpu;
> -	int ret;
> -	union perf_capabilities host_cap;
> -	uint64_t val;
> -
> -	host_cap.capabilities = kvm_get_feature_msr(MSR_IA32_PERF_CAPABILITIES);
> -	host_cap.capabilities &= (PMU_CAP_FW_WRITES | PMU_CAP_LBR_FMT);
> -
> -	/* Create VM */
> -	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
> -
> -	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_PDCM));
> -
> -	TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
> -	TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
> +	struct kvm_vm *vm = vm_create_with_one_vcpu(&vcpu, guest_code);
>   
>   	/* testcase 1, set capabilities when we have PDCM bit */
>   	vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, PMU_CAP_FW_WRITES);
> @@ -70,7 +56,16 @@ int main(int argc, char *argv[])
>   	vcpu_run(vcpu);
>   	ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), PMU_CAP_FW_WRITES);
>   
> -	/* testcase 2, check valid LBR formats are accepted */
> +	kvm_vm_free(vm);
> +}
> +
> +static void test_immutable_perf_capabilities(union perf_capabilities host_cap)
> +{
> +	struct kvm_vcpu *vcpu;
> +	struct kvm_vm *vm = vm_create_with_one_vcpu(&vcpu, NULL);
> +	uint64_t val;
> +	int ret;
> +
>   	vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, 0);
>   	ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), 0);
>   
> @@ -78,8 +73,8 @@ int main(int argc, char *argv[])
>   	ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), (u64)host_cap.lbr_format);
>   
>   	/*
> -	 * Testcase 3, check that an "invalid" LBR format is rejected.  Only an
> -	 * exact match of the host's format (and 0/disabled) is allowed.
> +	 * KVM only supports the host's native LBR format, as well as '0' (to
> +	 * disable LBR support).  Verify KVM rejects all other LBR formats.
>   	 */
>   	for (val = 1; val <= PMU_CAP_LBR_FMT; val++) {
>   		if (val == (host_cap.capabilities & PMU_CAP_LBR_FMT))
> @@ -88,7 +83,23 @@ int main(int argc, char *argv[])
>   		ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
>   		TEST_ASSERT(!ret, "Bad LBR FMT = 0x%lx didn't fail", val);
>   	}
> +	kvm_vm_free(vm);
> +}
> +
> +int main(int argc, char *argv[])
> +{
> +	union perf_capabilities host_cap;
> +
> +	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_PDCM));
> +
> +	TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
> +	TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
> +
> +	host_cap.capabilities = kvm_get_feature_msr(MSR_IA32_PERF_CAPABILITIES);
> +	host_cap.capabilities &= (PMU_CAP_FW_WRITES | PMU_CAP_LBR_FMT);
> +
> +	test_fungible_perf_capabilities(host_cap);
> +	test_immutable_perf_capabilities(host_cap);
>   
>   	printf("Completed perf capability tests.\n");
> -	kvm_vm_free(vm);
>   }
Sean Christopherson Feb. 14, 2023, 5:42 p.m. UTC | #2 
On Tue, Feb 14, 2023, Like Xu wrote:
> Nit, could this patch be applied before the relevant KVM modifications [1] so that
> a CI bot doesn't generate an error report before this selftests patch is applied ?

Yeah, good call.  CI aside, this patch should definitely land before the KVM change.

> [1] KVM: x86: Disallow writes to immutable feature MSRs after KVM_RUN
diff mbox series

Patch

diff --git a/tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c b/tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c
index c280ba1e6572..ac08c0fdd84d 100644
--- a/tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c
+++ b/tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c
@@ -41,24 +41,10 @@  static void guest_code(void)
 	wrmsr(MSR_IA32_PERF_CAPABILITIES, PMU_CAP_LBR_FMT);
 }
 
-int main(int argc, char *argv[])
+static void test_fungible_perf_capabilities(union perf_capabilities host_cap)
 {
-	struct kvm_vm *vm;
 	struct kvm_vcpu *vcpu;
-	int ret;
-	union perf_capabilities host_cap;
-	uint64_t val;
-
-	host_cap.capabilities = kvm_get_feature_msr(MSR_IA32_PERF_CAPABILITIES);
-	host_cap.capabilities &= (PMU_CAP_FW_WRITES | PMU_CAP_LBR_FMT);
-
-	/* Create VM */
-	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
-
-	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_PDCM));
-
-	TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
-	TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
+	struct kvm_vm *vm = vm_create_with_one_vcpu(&vcpu, guest_code);
 
 	/* testcase 1, set capabilities when we have PDCM bit */
 	vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, PMU_CAP_FW_WRITES);
@@ -70,7 +56,16 @@  int main(int argc, char *argv[])
 	vcpu_run(vcpu);
 	ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), PMU_CAP_FW_WRITES);
 
-	/* testcase 2, check valid LBR formats are accepted */
+	kvm_vm_free(vm);
+}
+
+static void test_immutable_perf_capabilities(union perf_capabilities host_cap)
+{
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm = vm_create_with_one_vcpu(&vcpu, NULL);
+	uint64_t val;
+	int ret;
+
 	vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, 0);
 	ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), 0);
 
@@ -78,8 +73,8 @@  int main(int argc, char *argv[])
 	ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), (u64)host_cap.lbr_format);
 
 	/*
-	 * Testcase 3, check that an "invalid" LBR format is rejected.  Only an
-	 * exact match of the host's format (and 0/disabled) is allowed.
+	 * KVM only supports the host's native LBR format, as well as '0' (to
+	 * disable LBR support).  Verify KVM rejects all other LBR formats.
 	 */
 	for (val = 1; val <= PMU_CAP_LBR_FMT; val++) {
 		if (val == (host_cap.capabilities & PMU_CAP_LBR_FMT))
@@ -88,7 +83,23 @@  int main(int argc, char *argv[])
 		ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
 		TEST_ASSERT(!ret, "Bad LBR FMT = 0x%lx didn't fail", val);
 	}
+	kvm_vm_free(vm);
+}
+
+int main(int argc, char *argv[])
+{
+	union perf_capabilities host_cap;
+
+	TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_PDCM));
+
+	TEST_REQUIRE(kvm_cpu_has_p(X86_PROPERTY_PMU_VERSION));
+	TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 0);
+
+	host_cap.capabilities = kvm_get_feature_msr(MSR_IA32_PERF_CAPABILITIES);
+	host_cap.capabilities &= (PMU_CAP_FW_WRITES | PMU_CAP_LBR_FMT);
+
+	test_fungible_perf_capabilities(host_cap);
+	test_immutable_perf_capabilities(host_cap);
 
 	printf("Completed perf capability tests.\n");
-	kvm_vm_free(vm);
 }