From: Jinrong Liang <[email protected]>
This commit adds a PEBS test that verifies all possible combinations
of PEBS-related bits in MSR_IA32_PERF_CAPABILITIES. This comprehensive
test ensures the accuracy of the PEBS feature.
Signed-off-by: Jinrong Liang <[email protected]>
---
.../selftests/kvm/x86_64/vmx_pmu_caps_test.c | 71 +++++++++++++++++++
1 file changed, 71 insertions(+)
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 02903084598f..c1b1ba44bc26 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
@@ -21,6 +21,12 @@
#define MAX_LINEAR_ADDR_MASK GENMASK_ULL(15, 8)
#define ADDR_OFS_BIT 8
+#define PMU_CAP_LBR_FMT 0x3f
+#define PMU_CAP_SMM_FREEZE BIT_ULL(12)
+#define PMU_CAP_FW_WRITES BIT_ULL(13)
+#define PMU_CAP_PERF_METRICS_AVAILABLE BIT_ULL(PERF_CAP_METRICS_IDX)
+#define PMU_CAP_PEBS_OUTPUT_PT_AVAIL BIT_ULL(PERF_CAP_PT_IDX)
+#define PMU_CAP_PEBS_ALL (PERF_CAP_PEBS_MASK | PMU_CAP_PEBS_OUTPUT_PT_AVAIL)
union perf_capabilities {
struct {
@@ -331,6 +337,70 @@ static void test_ds_area_noncanonical_address(union perf_capabilities host_cap)
kvm_vm_free(vm);
}
+static void test_pebs_bit_combinations(union perf_capabilities host_cap)
+{
+ int ret;
+ uint64_t pebs_val, val;
+ struct kvm_vcpu *vcpu;
+ struct kvm_vm *vm = vm_create_with_one_vcpu(&vcpu, NULL);
+
+ TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 1);
+ TEST_REQUIRE(host_cap.capabilities & PERF_CAP_PEBS_FORMAT);
+ TEST_REQUIRE(vcpu_get_msr(vcpu, MSR_IA32_MISC_ENABLE) &
+ MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL);
+
+ /*
+ * Test if PEBS_REC_FMT is set and the value is the same as host,
+ * the other PEBS bits are allowed to be set only if they are the
+ * same as host.
+ */
+ pebs_val = host_cap.capabilities & PMU_CAP_PEBS_ALL;
+
+ vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, pebs_val);
+ ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES),
+ (u64)pebs_val);
+
+ /* Test all PEBS bit combinations. */
+ for (val = 0x0; val <= (~0ul & PMU_CAP_PEBS_ALL); val++) {
+ /* Skips values that are not related to PEBS. */
+ if (val & (PMU_CAP_LBR_FMT | PMU_CAP_SMM_FREEZE |
+ PMU_CAP_FW_WRITES | PMU_CAP_PERF_METRICS_AVAILABLE))
+ continue;
+
+ /*
+ * Test that value of PEBS is rejected when the KVM doesn't
+ * supports Intel PT.
+ */
+ if ((val & PMU_CAP_PEBS_OUTPUT_PT_AVAIL) &&
+ (!(host_cap.capabilities & PMU_CAP_PEBS_OUTPUT_PT_AVAIL))) {
+ ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
+ TEST_ASSERT(!ret, "Bad PEBS auxiliary bits = 0x%lx didn't fail", val);
+
+ continue;
+ }
+
+ /*
+ * Test that value of PEBS is rejected when carrying
+ * PEBS_REC_FMT if the value of PEBS is not equal to host.
+ */
+ if ((val & PERF_CAP_PEBS_FORMAT) && val != pebs_val) {
+ ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
+ TEST_ASSERT(!ret, "Bad PEBS auxiliary bits = 0x%lx didn't fail", val);
+
+ continue;
+ }
+
+ /*
+ * Test that PEBS bits can be written simultaneously or
+ * independently if PEBS_REC_FMT is not carried.
+ */
+ vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
+ ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), val);
+ }
+
+ kvm_vm_free(vm);
+}
+
int main(int argc, char *argv[])
{
union perf_capabilities host_cap;
@@ -352,4 +422,5 @@ int main(int argc, char *argv[])
test_guest_wrmsr_perf_capabilities(host_cap);
test_lbr_perf_capabilities(host_cap);
test_ds_area_noncanonical_address(host_cap);
+ test_pebs_bit_combinations(host_cap);
}
--
2.31.1
On Thu, Jun 08, 2023, Jinrong Liang wrote:
> From: Jinrong Liang <[email protected]>
>
> This commit adds a PEBS test that verifies all possible combinations
> of PEBS-related bits in MSR_IA32_PERF_CAPABILITIES. This comprehensive
> test ensures the accuracy of the PEBS feature.
>
> Signed-off-by: Jinrong Liang <[email protected]>
> ---
> .../selftests/kvm/x86_64/vmx_pmu_caps_test.c | 71 +++++++++++++++++++
> 1 file changed, 71 insertions(+)
>
> 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 02903084598f..c1b1ba44bc26 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
> @@ -21,6 +21,12 @@
>
> #define MAX_LINEAR_ADDR_MASK GENMASK_ULL(15, 8)
> #define ADDR_OFS_BIT 8
> +#define PMU_CAP_LBR_FMT 0x3f
> +#define PMU_CAP_SMM_FREEZE BIT_ULL(12)
> +#define PMU_CAP_FW_WRITES BIT_ULL(13)
> +#define PMU_CAP_PERF_METRICS_AVAILABLE BIT_ULL(PERF_CAP_METRICS_IDX)
> +#define PMU_CAP_PEBS_OUTPUT_PT_AVAIL BIT_ULL(PERF_CAP_PT_IDX)
> +#define PMU_CAP_PEBS_ALL (PERF_CAP_PEBS_MASK | PMU_CAP_PEBS_OUTPUT_PT_AVAIL)
>
> union perf_capabilities {
> struct {
> @@ -331,6 +337,70 @@ static void test_ds_area_noncanonical_address(union perf_capabilities host_cap)
> kvm_vm_free(vm);
> }
>
> +static void test_pebs_bit_combinations(union perf_capabilities host_cap)
> +{
> + int ret;
Reverse xmas tree.
> + uint64_t pebs_val, val;
> + struct kvm_vcpu *vcpu;
> + struct kvm_vm *vm = vm_create_with_one_vcpu(&vcpu, NULL);
It's kinda silly, but I think it makes sense to wait until after all of the
TEST_REQUIRE()s to create the VM+vCPU.
> +
> + TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 1);
> + TEST_REQUIRE(host_cap.capabilities & PERF_CAP_PEBS_FORMAT);
> + TEST_REQUIRE(vcpu_get_msr(vcpu, MSR_IA32_MISC_ENABLE) &
> + MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL);
> +
> + /*
> + * Test if PEBS_REC_FMT is set and the value is the same as host,
> + * the other PEBS bits are allowed to be set only if they are the
> + * same as host.
> + */
> + pebs_val = host_cap.capabilities & PMU_CAP_PEBS_ALL;
> +
> + vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, pebs_val);
> + ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES),
> + (u64)pebs_val);
This cast shouldn't be necessary. And if you're going to split lines...
ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES),
host_cap.capabilities & PMU_CAP_PEBS_ALL);
Though isn't that flawed? E.g. will fail if MSR_IA32_PERF_CAPABILITIES has
non-PEBS bits set. I think what you want is something like:
guest_perf_caps = vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES);
ASSERT_EQ(guest_perf_caps & PMU_CAP_PEBS_ALL,
host_cap.capabilities & PMU_CAP_PEBS_ALL);
> +
> + /* Test all PEBS bit combinations. */
> + for (val = 0x0; val <= (~0ul & PMU_CAP_PEBS_ALL); val++) {
> + /* Skips values that are not related to PEBS. */
> + if (val & (PMU_CAP_LBR_FMT | PMU_CAP_SMM_FREEZE |
> + PMU_CAP_FW_WRITES | PMU_CAP_PERF_METRICS_AVAILABLE))
Align things by their scope, i.e.
if (val & (PMU_CAP_LBR_FMT | PMU_CAP_SMM_FREEZE
PMU_CAP_FW_WRITES | PMU_CAP_PERF_METRICS_AVAILABLE))
But even better would be to look for !PEBS, not some other values where it's not
clear they exhaustively cover all !PEBS value. E.g. can't this be?
if (val & ~PMU_CAP_PEBS_ALL)
continue;
> + continue;
> +
> + /*
> + * Test that value of PEBS is rejected when the KVM doesn't
Just "KVM", not "the KVM".
> + * supports Intel PT.
> + */
> + if ((val & PMU_CAP_PEBS_OUTPUT_PT_AVAIL) &&
> + (!(host_cap.capabilities & PMU_CAP_PEBS_OUTPUT_PT_AVAIL))) {
> + ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
> + TEST_ASSERT(!ret, "Bad PEBS auxiliary bits = 0x%lx didn't fail", val);
> +
> + continue;
> + }
> +
> + /*
> + * Test that value of PEBS is rejected when carrying
I don't quite follow what you mean by "carrying". Do you mean a non-zero value?
> + * PEBS_REC_FMT if the value of PEBS is not equal to host.
> + */
> + if ((val & PERF_CAP_PEBS_FORMAT) && val != pebs_val) {
> + ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
> + TEST_ASSERT(!ret, "Bad PEBS auxiliary bits = 0x%lx didn't fail", val);
> +
> + continue;
> + }
> +
> + /*
> + * Test that PEBS bits can be written simultaneously or
> + * independently if PEBS_REC_FMT is not carried.
> + */
> + vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
> + ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), val);
> + }
> +
> + kvm_vm_free(vm);
> +}
Sean Christopherson <[email protected]> 于2023年6月29日周四 05:55写道：
>
> On Thu, Jun 08, 2023, Jinrong Liang wrote:
> > From: Jinrong Liang <[email protected]>
> >
> > This commit adds a PEBS test that verifies all possible combinations
> > of PEBS-related bits in MSR_IA32_PERF_CAPABILITIES. This comprehensive
> > test ensures the accuracy of the PEBS feature.
> >
> > Signed-off-by: Jinrong Liang <[email protected]>
> > ---
> > .../selftests/kvm/x86_64/vmx_pmu_caps_test.c | 71 +++++++++++++++++++
> > 1 file changed, 71 insertions(+)
> >
> > 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 02903084598f..c1b1ba44bc26 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
> > @@ -21,6 +21,12 @@
> >
> > #define MAX_LINEAR_ADDR_MASK GENMASK_ULL(15, 8)
> > #define ADDR_OFS_BIT 8
> > +#define PMU_CAP_LBR_FMT 0x3f
> > +#define PMU_CAP_SMM_FREEZE BIT_ULL(12)
> > +#define PMU_CAP_FW_WRITES BIT_ULL(13)
> > +#define PMU_CAP_PERF_METRICS_AVAILABLE BIT_ULL(PERF_CAP_METRICS_IDX)
> > +#define PMU_CAP_PEBS_OUTPUT_PT_AVAIL BIT_ULL(PERF_CAP_PT_IDX)
> > +#define PMU_CAP_PEBS_ALL (PERF_CAP_PEBS_MASK | PMU_CAP_PEBS_OUTPUT_PT_AVAIL)
> >
> > union perf_capabilities {
> > struct {
> > @@ -331,6 +337,70 @@ static void test_ds_area_noncanonical_address(union perf_capabilities host_cap)
> > kvm_vm_free(vm);
> > }
> >
> > +static void test_pebs_bit_combinations(union perf_capabilities host_cap)
> > +{
> > + int ret;
>
> Reverse xmas tree.
>
> > + uint64_t pebs_val, val;
> > + struct kvm_vcpu *vcpu;
> > + struct kvm_vm *vm = vm_create_with_one_vcpu(&vcpu, NULL);
>
> It's kinda silly, but I think it makes sense to wait until after all of the
> TEST_REQUIRE()s to create the VM+vCPU.
>
> > +
> > + TEST_REQUIRE(kvm_cpu_property(X86_PROPERTY_PMU_VERSION) > 1);
> > + TEST_REQUIRE(host_cap.capabilities & PERF_CAP_PEBS_FORMAT);
> > + TEST_REQUIRE(vcpu_get_msr(vcpu, MSR_IA32_MISC_ENABLE) &
> > + MSR_IA32_MISC_ENABLE_PEBS_UNAVAIL);
> > +
> > + /*
> > + * Test if PEBS_REC_FMT is set and the value is the same as host,
> > + * the other PEBS bits are allowed to be set only if they are the
> > + * same as host.
> > + */
> > + pebs_val = host_cap.capabilities & PMU_CAP_PEBS_ALL;
> > +
> > + vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, pebs_val);
> > + ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES),
> > + (u64)pebs_val);
>
> This cast shouldn't be necessary. And if you're going to split lines...
>
> ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES),
> host_cap.capabilities & PMU_CAP_PEBS_ALL);
>
> Though isn't that flawed? E.g. will fail if MSR_IA32_PERF_CAPABILITIES has
> non-PEBS bits set. I think what you want is something like:
>
> guest_perf_caps = vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES);
>
> ASSERT_EQ(guest_perf_caps & PMU_CAP_PEBS_ALL,
> host_cap.capabilities & PMU_CAP_PEBS_ALL);
>
> > +
> > + /* Test all PEBS bit combinations. */
> > + for (val = 0x0; val <= (~0ul & PMU_CAP_PEBS_ALL); val++) {
> > + /* Skips values that are not related to PEBS. */
> > + if (val & (PMU_CAP_LBR_FMT | PMU_CAP_SMM_FREEZE |
> > + PMU_CAP_FW_WRITES | PMU_CAP_PERF_METRICS_AVAILABLE))
>
> Align things by their scope, i.e.
>
> if (val & (PMU_CAP_LBR_FMT | PMU_CAP_SMM_FREEZE
> PMU_CAP_FW_WRITES | PMU_CAP_PERF_METRICS_AVAILABLE))
>
> But even better would be to look for !PEBS, not some other values where it's not
> clear they exhaustively cover all !PEBS value. E.g. can't this be?
>
> if (val & ~PMU_CAP_PEBS_ALL)
> continue;
>
> > + continue;
> > +
> > + /*
> > + * Test that value of PEBS is rejected when the KVM doesn't
>
> Just "KVM", not "the KVM".
>
> > + * supports Intel PT.
> > + */
> > + if ((val & PMU_CAP_PEBS_OUTPUT_PT_AVAIL) &&
> > + (!(host_cap.capabilities & PMU_CAP_PEBS_OUTPUT_PT_AVAIL))) {
> > + ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
> > + TEST_ASSERT(!ret, "Bad PEBS auxiliary bits = 0x%lx didn't fail", val);
> > +
> > + continue;
> > + }
> > +
> > + /*
> > + * Test that value of PEBS is rejected when carrying
>
> I don't quite follow what you mean by "carrying". Do you mean a non-zero value?
I apologize for the confusion. Yes, by "carrying" I meant a non-zero
value. I will revise the comment to clarify the meaning and make it
more precise.
>
> > + * PEBS_REC_FMT if the value of PEBS is not equal to host.
> > + */
> > + if ((val & PERF_CAP_PEBS_FORMAT) && val != pebs_val) {
> > + ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
> > + TEST_ASSERT(!ret, "Bad PEBS auxiliary bits = 0x%lx didn't fail", val);
> > +
> > + continue;
> > + }
> > +
> > + /*
> > + * Test that PEBS bits can be written simultaneously or
> > + * independently if PEBS_REC_FMT is not carried.
> > + */
> > + vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val);
> > + ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), val);
> > + }
> > +
> > + kvm_vm_free(vm);
> > +}
Thank you for all your valuable feedback and suggestions. Your
guidance has been extremely helpful in improving the quality of the
code.