| Message ID | 20220709010250.1001326-4-aaronlewis@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Sat, Jul 09, 2022, Aaron Lewis wrote: > Add testing for the pmu event filter's masked events. These tests run > through different ways of finding an event the guest is attempting to > program in an event list. For any given eventsel, there may be > multiple instances of it in an event list. These tests try different > ways of looking up a match to force the matching algorithm to walk the > relevant eventsel's and ensure it is able to a) find a match, b) stays > within its bounds. > > Signed-off-by: Aaron Lewis <aaronlewis@google.com> > --- > .../kvm/x86_64/pmu_event_filter_test.c | 99 +++++++++++++++++++ > 1 file changed, 99 insertions(+) > > 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 4bff4c71ac45..29abe9c88f4f 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 > @@ -18,8 +18,12 @@ > /* > * In lieu of copying perf_event.h into tools... > */ > +#define ARCH_PERFMON_EVENTSEL_EVENT 0x000000FFULL > #define ARCH_PERFMON_EVENTSEL_OS (1ULL << 17) > #define ARCH_PERFMON_EVENTSEL_ENABLE (1ULL << 22) > +#define AMD64_EVENTSEL_EVENT \ > + (ARCH_PERFMON_EVENTSEL_EVENT | (0x0FULL << 32)) > + > > union cpuid10_eax { > struct { > @@ -445,6 +449,99 @@ static bool use_amd_pmu(void) > is_zen3(entry->eax)); > } > > +#define ENCODE_MASKED_EVENT(select, mask, match, invert) \ > + KVM_PMU_EVENT_ENCODE_MASKED_EVENT(select, mask, match, invert) Eww. Use a helper. Defining a passthrough macro just to get a shorter name is silly, AFAICT all usages passes ~0ull / ~0x00 for the mask, and so that the test can do it's own type checking, e.g. @invert can/should be a boolean. > + > +static void expect_success(uint64_t count) > +{ > + if (count != NUM_BRANCHES) > + pr_info("masked filter: Branch instructions retired = %lu (expected %u)\n", > + count, NUM_BRANCHES); If the number of branches is expected to be precise, then assert on that. If not, then I don't see much value in printing anything. > + TEST_ASSERT(count, "Allowed PMU event is not counting"); > +} > + > +static void expect_failure(uint64_t count) > +{ > + if (count) > + pr_info("masked filter: Branch instructions retired = %lu (expected 0)\n", > + count); > + TEST_ASSERT(!count, "Disallowed PMU Event is counting"); Print the debug information in the assert. > +} > + > +static void run_masked_events_test(struct kvm_vm *vm, uint64_t masked_events[], > + const int nmasked_events, uint64_t event, > + uint32_t action, > + void (*expected_func)(uint64_t)) A function callback is overkill and unnecessary obfuscation. And "expect_failure" is misleading; the test isn't expected to "fail", rather the event is expected to not count. Actually, the function is completely unnecessary, the caller already passed in ALLOW vs. DENY in action. > +{ > + struct kvm_pmu_event_filter *f; > + uint64_t old_event; > + uint64_t count; > + int i; > + > + for (i = 0; i < nmasked_events; i++) { > + if ((masked_events[i] & AMD64_EVENTSEL_EVENT) != EVENT(event, 0)) > + continue; > + > + old_event = masked_events[i]; > + > + masked_events[i] = > + ENCODE_MASKED_EVENT(event, ~0x00, 0x00, 0); Why double zeros? And this easily fits on a single line. masked_events[i] = ENCODE_MASKED_EVENT(event, ~0ull, 0, 0); > + > + f = create_pmu_event_filter(masked_events, nmasked_events, action, > + KVM_PMU_EVENT_FLAG_MASKED_EVENTS); > + > + count = test_with_filter(vm, f); > + free(f); > + As alluded to above... if (action == KVM_PMU_EVENT_ALLOW) TEST_ASSERT(count, <here be verbose, helpful output>); else TEST_ASSERT(!count, <more verbose, helpful output>); > + expected_func(count); > + > + masked_events[i] = old_event; > + } > +} > + > +static void run_masked_events_tests(struct kvm_vm *vm, uint64_t masked_events[], > + const int nmasked_events, uint64_t event) > +{ > + run_masked_events_test(vm, masked_events, nmasked_events, event, > + KVM_PMU_EVENT_ALLOW, expect_success); > + run_masked_events_test(vm, masked_events, nmasked_events, event, > + KVM_PMU_EVENT_DENY, expect_failure); > +} > + > +static void test_masked_events(struct kvm_vm *vm) > +{ > + uint64_t masked_events[11]; Why '11'? > + const int nmasked_events = ARRAY_SIZE(masked_events); > + uint64_t prev_event, event, next_event; > + int i; > + > + if (use_intel_pmu()) { > + /* Instructions retired */ > + prev_event = 0xc0; > + event = INTEL_BR_RETIRED; > + /* Branch misses retired */ > + next_event = 0xc5; > + } else { > + TEST_ASSERT(use_amd_pmu(), "Unknown platform"); > + /* Retired instructions */ > + prev_event = 0xc0; > + event = AMD_ZEN_BR_RETIRED; > + /* Retired branch instructions mispredicted */ > + next_event = 0xc3; > + } > + > + for (i = 0; i < nmasked_events; i++) > + masked_events[i] = > + ENCODE_MASKED_EVENT(event, ~0x00, i+1, 0); > + > + run_masked_events_tests(vm, masked_events, nmasked_events, event); > + Why run the test twice? Hint, comment... ;-) > + masked_events[0] = ENCODE_MASKED_EVENT(prev_event, ~0x00, 0, 0); > + masked_events[1] = ENCODE_MASKED_EVENT(next_event, ~0x00, 0, 0); > + > + run_masked_events_tests(vm, masked_events, nmasked_events, event); > +} > + > int main(int argc, char *argv[]) > { > void (*guest_code)(void) = NULL; > @@ -489,6 +586,8 @@ int main(int argc, char *argv[]) > test_not_member_deny_list(vm); > test_not_member_allow_list(vm); > > + test_masked_events(vm); > + > kvm_vm_free(vm); > > test_pmu_config_disable(guest_code); > -- > 2.37.0.144.g8ac04bfd2-goog >
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 4bff4c71ac45..29abe9c88f4f 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 @@ -18,8 +18,12 @@ /* * In lieu of copying perf_event.h into tools... */ +#define ARCH_PERFMON_EVENTSEL_EVENT 0x000000FFULL #define ARCH_PERFMON_EVENTSEL_OS (1ULL << 17) #define ARCH_PERFMON_EVENTSEL_ENABLE (1ULL << 22) +#define AMD64_EVENTSEL_EVENT \ + (ARCH_PERFMON_EVENTSEL_EVENT | (0x0FULL << 32)) + union cpuid10_eax { struct { @@ -445,6 +449,99 @@ static bool use_amd_pmu(void) is_zen3(entry->eax)); } +#define ENCODE_MASKED_EVENT(select, mask, match, invert) \ + KVM_PMU_EVENT_ENCODE_MASKED_EVENT(select, mask, match, invert) + +static void expect_success(uint64_t count) +{ + if (count != NUM_BRANCHES) + pr_info("masked filter: Branch instructions retired = %lu (expected %u)\n", + count, NUM_BRANCHES); + TEST_ASSERT(count, "Allowed PMU event is not counting"); +} + +static void expect_failure(uint64_t count) +{ + if (count) + pr_info("masked filter: Branch instructions retired = %lu (expected 0)\n", + count); + TEST_ASSERT(!count, "Disallowed PMU Event is counting"); +} + +static void run_masked_events_test(struct kvm_vm *vm, uint64_t masked_events[], + const int nmasked_events, uint64_t event, + uint32_t action, + void (*expected_func)(uint64_t)) +{ + struct kvm_pmu_event_filter *f; + uint64_t old_event; + uint64_t count; + int i; + + for (i = 0; i < nmasked_events; i++) { + if ((masked_events[i] & AMD64_EVENTSEL_EVENT) != EVENT(event, 0)) + continue; + + old_event = masked_events[i]; + + masked_events[i] = + ENCODE_MASKED_EVENT(event, ~0x00, 0x00, 0); + + f = create_pmu_event_filter(masked_events, nmasked_events, action, + KVM_PMU_EVENT_FLAG_MASKED_EVENTS); + + count = test_with_filter(vm, f); + free(f); + + expected_func(count); + + masked_events[i] = old_event; + } +} + +static void run_masked_events_tests(struct kvm_vm *vm, uint64_t masked_events[], + const int nmasked_events, uint64_t event) +{ + run_masked_events_test(vm, masked_events, nmasked_events, event, + KVM_PMU_EVENT_ALLOW, expect_success); + run_masked_events_test(vm, masked_events, nmasked_events, event, + KVM_PMU_EVENT_DENY, expect_failure); +} + +static void test_masked_events(struct kvm_vm *vm) +{ + uint64_t masked_events[11]; + const int nmasked_events = ARRAY_SIZE(masked_events); + uint64_t prev_event, event, next_event; + int i; + + if (use_intel_pmu()) { + /* Instructions retired */ + prev_event = 0xc0; + event = INTEL_BR_RETIRED; + /* Branch misses retired */ + next_event = 0xc5; + } else { + TEST_ASSERT(use_amd_pmu(), "Unknown platform"); + /* Retired instructions */ + prev_event = 0xc0; + event = AMD_ZEN_BR_RETIRED; + /* Retired branch instructions mispredicted */ + next_event = 0xc3; + } + + for (i = 0; i < nmasked_events; i++) + masked_events[i] = + ENCODE_MASKED_EVENT(event, ~0x00, i+1, 0); + + run_masked_events_tests(vm, masked_events, nmasked_events, event); + + masked_events[0] = ENCODE_MASKED_EVENT(prev_event, ~0x00, 0, 0); + masked_events[1] = ENCODE_MASKED_EVENT(next_event, ~0x00, 0, 0); + + run_masked_events_tests(vm, masked_events, nmasked_events, event); +} + int main(int argc, char *argv[]) { void (*guest_code)(void) = NULL; @@ -489,6 +586,8 @@ int main(int argc, char *argv[]) test_not_member_deny_list(vm); test_not_member_allow_list(vm); + test_masked_events(vm); + kvm_vm_free(vm); test_pmu_config_disable(guest_code);
Add testing for the pmu event filter's masked events. These tests run through different ways of finding an event the guest is attempting to program in an event list. For any given eventsel, there may be multiple instances of it in an event list. These tests try different ways of looking up a match to force the matching algorithm to walk the relevant eventsel's and ensure it is able to a) find a match, b) stays within its bounds. Signed-off-by: Aaron Lewis <aaronlewis@google.com> --- .../kvm/x86_64/pmu_event_filter_test.c | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+)