Messages in this thread | | | Date | Fri, 5 Aug 2022 14:10:50 +0800 | Subject | Re: [PATCH] selftests: kvm/x86: test if it checks all the bits in the LBR_FMT bit-field | From | Like Xu <> |
| |
On 5/8/2022 7:26 am, Sean Christopherson wrote: > On Thu, Aug 04, 2022, Like Xu wrote: >> From: Like Xu <likexu@tencent.com> >> >> User space only enable guest LBR feature when the exactly supported >> LBR format value is initialized to the MSR_IA32_PERF_CAPABILITIES. >> The input is also invalid if only partially supported bits are set. >> >> Note for PEBS feature, the PEBS_FORMAT bit field is the primary concern, >> thus if the PEBS_FORMAT input is empty, the other bits check about PEBS >> (like PEBS_TRAP or ARCH_REG) will be ignored. >> >> Signed-off-by: Like Xu <likexu@tencent.com> >> --- >> tools/testing/selftests/kvm/x86_64/vmx_pmu_caps_test.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> 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 6ec901dab61e..98483947f921 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 >> @@ -13,6 +13,7 @@ >> >> #define _GNU_SOURCE /* for program_invocation_short_name */ >> #include <sys/ioctl.h> >> +#include <linux/bitmap.h> >> >> #include "kvm_util.h" >> #include "vmx.h" >> @@ -56,7 +57,7 @@ int main(int argc, char *argv[]) >> const struct kvm_cpuid_entry2 *entry_a_0; >> struct kvm_vm *vm; >> struct kvm_vcpu *vcpu; >> - int ret; >> + int ret, bit; >> union cpuid10_eax eax; >> union perf_capabilities host_cap; >> >> @@ -97,6 +98,12 @@ int main(int argc, char *argv[]) >> ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, 0x30); >> TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail."); >> >> + /* testcase 4, reject LBR_FMT if only partially supported bits are set */ >> + for_each_set_bit(bit, (unsigned long *)&host_cap.capabilities, 6) { >> + ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, BIT_ULL(bit)); >> + TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail."); >> + } > > Rather than add another testcase and target only a subset of possible values, what > about replacing (fixing IMO) testcase #3 with fully exhaustive approach?
Yeah, much better.
> > --- > From: Sean Christopherson <seanjc@google.com> > Date: Thu, 4 Aug 2022 12:18:15 -0700 > Subject: [PATCH] KVM: selftests: Test all possible "invalid" > PERF_CAPABILITIES.LBR_FMT vals > > Test all possible input values to verify that KVM rejects all values > except the exact host value. Due to the LBR format affecting the core > functionality of LBRs, KVM can't emulate "other" formats, so even though > there are a variety of legal values, KVM should reject anything but an > exact host match. > > Suggested-by: Like Xu <like.xu.linux@gmail.com> > Signed-off-by: Sean Christopherson <seanjc@google.com> > --- > .../selftests/kvm/x86_64/vmx_pmu_caps_test.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 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 6ec901dab61e..069589c52f41 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 > @@ -59,6 +59,7 @@ int main(int argc, char *argv[]) > int ret; > union cpuid10_eax eax; > 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); > @@ -91,11 +92,17 @@ int main(int argc, char *argv[]) > vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, host_cap.lbr_format); > ASSERT_EQ(vcpu_get_msr(vcpu, MSR_IA32_PERF_CAPABILITIES), (u64)host_cap.lbr_format); > > - /* testcase 3, check invalid LBR format is rejected */ > - /* Note, on Arch LBR capable platforms, LBR_FMT in perf capability msr is 0x3f, > - * to avoid the failure, use a true invalid format 0x30 for the test. */ > - ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, 0x30); > - TEST_ASSERT(ret == 0, "Bad PERF_CAPABILITIES didn't fail."); > + /* > + * Testcase 3, check that an "invalid" LBR format is rejected. Only an > + * exact match of the host's format (and 0/disabled) is allowed. > + */ > + for (val = 1; val <= PMU_CAP_LBR_FMT; val++) { > + if (val == (host_cap.capabilities & PMU_CAP_LBR_FMT))
Emm,
if (val == host_cap.lbr_format)
to save a minor cost of logical operation.
> + continue; > + > + ret = _vcpu_set_msr(vcpu, MSR_IA32_PERF_CAPABILITIES, val); > + TEST_ASSERT(!ret, "Bad LBR FMT = 0x%lx didn't fail", val); > + } > > printf("Completed perf capability tests.\n"); > kvm_vm_free(vm); > > base-commit: 93472b79715378a2386598d6632c654a2223267b > -- >
| |