On Wed, Jun 07, 2023, Jinrong Liang wrote:
From: Jinrong Liang cloudliang@tencent.com
This commit adds test cases for unsupported input values in the
Avoid "this commit" and "this patch", simply state what the patch does as a command, e.g. "Add test cases for ...".
PMU event filter. The tests cover unsupported "action" values, unsupported "flags" values, and unsupported "nevents" values. All these cases should return an error, as they are currently not supported by the filter. Additionally, the patch tests setting non-exist fixed counters in the fixed bitmap doesn't fail.
Signed-off-by: Jinrong Liang cloudliang@tencent.com
.../kvm/x86_64/pmu_event_filter_test.c | 48 +++++++++++++++++-- 1 file changed, 45 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c index 26f674c32cde..7555e0f4290c 100644 --- a/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c +++ b/tools/testing/selftests/kvm/x86_64/pmu_event_filter_test.c @@ -11,9 +11,7 @@ */ #define _GNU_SOURCE /* for program_invocation_short_name */ -#include "test_util.h" -#include "kvm_util.h" -#include "processor.h" +#include "pmu.h" /*
- In lieu of copying perf_event.h into tools...
@@ -32,6 +30,10 @@ #define MAX_FILTER_EVENTS 300 #define MAX_TEST_EVENTS 10 +#define PMU_EVENT_FILTER_INVALID_ACTION (KVM_PMU_EVENT_DENY + 1) +#define PMU_EVENT_FILTER_INVALID_FLAGS (KVM_PMU_EVENT_FLAG_MASKED_EVENTS + 1) +#define PMU_EVENT_FILTER_INVALID_NEVENTS (MAX_FILTER_EVENTS + 1)
/*
- This is how the event selector and unit mask are stored in an AMD
- core performance event-select register. Intel's format is similar,
@@ -762,6 +764,7 @@ static void test_filter_ioctl(struct kvm_vcpu *vcpu) { uint64_t e = ~0ul; int r;
- struct __kvm_pmu_event_filter f;
Reverse xmas tree.
/* * Unfortunately having invalid bits set in event data is expected to @@ -780,6 +783,45 @@ static void test_filter_ioctl(struct kvm_vcpu *vcpu) KVM_PMU_EVENT_FLAG_MASKED_EVENTS, KVM_PMU_EVENT_ALLOW); TEST_ASSERT(r == 0, "Valid PMU Event Filter is failing");
- /*
* Testing unsupported "action" input values should return an error.
Omit the "Testing", KVM's behavior isn't specific to "testing", any unsupported action should fail.
/* Unsupported actions should be rejected by KVM. */
Though honestly, I would forego the comments entirely, the macro name plus the assert make it quite clear what's being tested.
* Currently, only values 0 or 1 are supported.
Drop this part of the comment, it will become stale if PMU_EVENT_FILTER_INVALID_ACTION is modified, and readers can look at the definition of PMU_EVENT_FILTER_INVALID_ACTION if they really care about the actual value.
*/- f = base_event_filter;
- f.action = PMU_EVENT_FILTER_INVALID_ACTION;
- r = do_vcpu_set_pmu_event_filter(vcpu, &f);
- TEST_ASSERT(r != 0, "Set invalid action is expected to fail.");
Ignore the bad precedent set by this test, the preferred way to check for 0 and !0 is TEST_ASSERT(r, ...) and TEST_ASSERT(!r, ...);
And no punctuation in the assert, i.e. drop the period.
- /*
* Testing unsupported "flags" input values should return an error.* Currently, only values 0 or 1 are supported.*/
Same here.
- f = base_event_filter;
- f.flags = PMU_EVENT_FILTER_INVALID_FLAGS;
- r = do_vcpu_set_pmu_event_filter(vcpu, &f);
- TEST_ASSERT(r != 0, "Set invalid flags is expected to fail.");
- /*
* Testing unsupported "nevents" input values should return an error.* Currently, only values less than or equal to* MAX_FILTER_EVENTS are supported.
And here.
*/- f = base_event_filter;
- f.nevents = PMU_EVENT_FILTER_INVALID_NEVENTS;
- r = do_vcpu_set_pmu_event_filter(vcpu, &f);
- TEST_ASSERT(r != 0,
"Setting PMU event filters that exceeds the maximum supported value should fail");
To avoid splitting lines,
TEST_ASSERT(r, "Exceeding the max number of filter events should fail");
- /*
* In this case, setting non-exist fixed counters in the fixed bitmap* doesn't fail.*/
And here.
- f = base_event_filter;
- f.fixed_counter_bitmap = ~GENMASK_ULL(X86_INTEL_MAX_FIXED_CTR_NUM, 0);
- r = do_vcpu_set_pmu_event_filter(vcpu, &f);
- TEST_ASSERT(r == 0,
"Setting invalid or non-exist fixed cunters in the fixed bitmap fail.");
Something like so to avoid multiple lines.
TEST_ASSERT(!r, "Masking non-existent fixed counters should be allowed");