[v2,08/21] KVM: selftests: Split PMU caps sub-tests to avoid writing MSR after KVM_RUN
Commit Message
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
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);
> }
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
@@ -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);
}