The test is inspired by the pmu_event_filter_test which implemented by x86. On
the arm64 platform, there is the same ability to set the pmu_event_filter
through the KVM_ARM_VCPU_PMU_V3_FILTER attribute. So add the test for arm64.
The series first create the helper function which can be used
for the vpmu related tests. Then, it implement the test.
Changelog:
----------
v4->v5:
- Rebased to v6.8-rc6.
- Refactor the helper function, make it fine-grained and easy to be used.
- Namimg improvements.
- Use the kvm_device_attr_set() helper.
- Make the test descriptor array readable and clean.
- Delete the patch which moves the pmu related helper to vpmu.h.
- Remove the kvm_supports_pmu_event_filter() function since nobody will run
this on a old kernel.
v3->v4:
- Rebased to the v6.8-rc2.
v2->v3:
- Check the pmceid in guest code instead of pmu event count since different
hardware may have different event count result, check pmceid makes it stable
on different platform. [Eric]
- Some typo fixed and commit message improved.
v1->v2:
- Improve the commit message. [Eric]
- Fix the bug in [enable|disable]_counter. [Raghavendra & Marc]
- Add the check if kvm has attr KVM_ARM_VCPU_PMU_V3_FILTER.
- Add if host pmu support the test event throught pmceid0.
- Split the test_invalid_filter() to another patch. [Eric]
v1: https://lore.kernel.org/all/[email protected]/
v2: https://lore.kernel.org/all/[email protected]/
v3: https://lore.kernel.org/all/[email protected]/
v4: https://lore.kernel.org/all/[email protected]/
Shaoqin Huang (3):
KVM: selftests: aarch64: Add helper function for the vpmu vcpu
creation
KVM: selftests: aarch64: Introduce pmu_event_filter_test
KVM: selftests: aarch64: Add invalid filter test in
pmu_event_filter_test
tools/testing/selftests/kvm/Makefile | 1 +
.../kvm/aarch64/pmu_event_filter_test.c | 325 ++++++++++++++++++
.../kvm/aarch64/vpmu_counter_access.c | 33 +-
.../selftests/kvm/include/aarch64/vpmu.h | 29 ++
4 files changed, 362 insertions(+), 26 deletions(-)
create mode 100644 tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
create mode 100644 tools/testing/selftests/kvm/include/aarch64/vpmu.h
--
2.40.1
Create a vcpu with vpmu would be a common requirement for the vpmu test,
so add the helper function for the vpmu vcpu creation. And use those
helper function in the vpmu_counter_access.c test.
Use this chance to delete the meaningless ASSERT about the pmuver,
because KVM does not advertise an IMP_DEF PMU to guests.
No functional changes intended.
Signed-off-by: Shaoqin Huang <[email protected]>
---
.../kvm/aarch64/vpmu_counter_access.c | 33 ++++---------------
.../selftests/kvm/include/aarch64/vpmu.h | 29 ++++++++++++++++
2 files changed, 36 insertions(+), 26 deletions(-)
create mode 100644 tools/testing/selftests/kvm/include/aarch64/vpmu.h
diff --git a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
index 5f9713364693..e068a0a7a43b 100644
--- a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
+++ b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
@@ -16,6 +16,7 @@
#include <processor.h>
#include <test_util.h>
#include <vgic.h>
+#include <vpmu.h>
#include <perf/arm_pmuv3.h>
#include <linux/bitfield.h>
@@ -426,18 +427,8 @@ static void guest_code(uint64_t expected_pmcr_n)
/* Create a VM that has one vCPU with PMUv3 configured. */
static void create_vpmu_vm(void *guest_code)
{
- struct kvm_vcpu_init init;
- uint8_t pmuver, ec;
- uint64_t dfr0, irq = 23;
- struct kvm_device_attr irq_attr = {
- .group = KVM_ARM_VCPU_PMU_V3_CTRL,
- .attr = KVM_ARM_VCPU_PMU_V3_IRQ,
- .addr = (uint64_t)&irq,
- };
- struct kvm_device_attr init_attr = {
- .group = KVM_ARM_VCPU_PMU_V3_CTRL,
- .attr = KVM_ARM_VCPU_PMU_V3_INIT,
- };
+ uint8_t ec;
+ uint64_t irq = 23;
/* The test creates the vpmu_vm multiple times. Ensure a clean state */
memset(&vpmu_vm, 0, sizeof(vpmu_vm));
@@ -449,27 +440,17 @@ static void create_vpmu_vm(void *guest_code)
guest_sync_handler);
}
- /* Create vCPU with PMUv3 */
- vm_ioctl(vpmu_vm.vm, KVM_ARM_PREFERRED_TARGET, &init);
- init.features[0] |= (1 << KVM_ARM_VCPU_PMU_V3);
- vpmu_vm.vcpu = aarch64_vcpu_add(vpmu_vm.vm, 0, &init, guest_code);
+ vpmu_vm.vcpu = vm_vcpu_add_with_vpmu(vpmu_vm.vm, 0, guest_code);
vcpu_init_descriptor_tables(vpmu_vm.vcpu);
+
vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
GICD_BASE_GPA, GICR_BASE_GPA);
__TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
"Failed to create vgic-v3, skipping");
- /* Make sure that PMUv3 support is indicated in the ID register */
- vcpu_get_reg(vpmu_vm.vcpu,
- KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1), &dfr0);
- pmuver = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer), dfr0);
- TEST_ASSERT(pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF &&
- pmuver >= ID_AA64DFR0_EL1_PMUVer_IMP,
- "Unexpected PMUVER (0x%x) on the vCPU with PMUv3", pmuver);
-
/* Initialize vPMU */
- vcpu_ioctl(vpmu_vm.vcpu, KVM_SET_DEVICE_ATTR, &irq_attr);
- vcpu_ioctl(vpmu_vm.vcpu, KVM_SET_DEVICE_ATTR, &init_attr);
+ vpmu_set_irq(vpmu_vm.vcpu, irq);
+ vpmu_init(vpmu_vm.vcpu);
}
static void destroy_vpmu_vm(void)
diff --git a/tools/testing/selftests/kvm/include/aarch64/vpmu.h b/tools/testing/selftests/kvm/include/aarch64/vpmu.h
new file mode 100644
index 000000000000..0dfcc7ab1c4d
--- /dev/null
+++ b/tools/testing/selftests/kvm/include/aarch64/vpmu.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#include <kvm_util.h>
+
+static inline struct kvm_vcpu *vm_vcpu_add_with_vpmu(struct kvm_vm *vm,
+ uint32_t vcpu_id,
+ void *guest_code)
+{
+ struct kvm_vcpu_init init;
+
+ /* Create vCPU with PMUv3 */
+ vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init);
+ init.features[0] |= (1 << KVM_ARM_VCPU_PMU_V3);
+
+ return aarch64_vcpu_add(vm, 0, &init, guest_code);
+}
+
+static void vpmu_set_irq(struct kvm_vcpu *vcpu, int irq)
+{
+ kvm_device_attr_set(vcpu->fd, KVM_ARM_VCPU_PMU_V3_CTRL,
+ KVM_ARM_VCPU_PMU_V3_IRQ, &irq);
+}
+
+static void vpmu_init(struct kvm_vcpu *vcpu)
+{
+ kvm_device_attr_set(vcpu->fd, KVM_ARM_VCPU_PMU_V3_CTRL,
+ KVM_ARM_VCPU_PMU_V3_INIT, NULL);
+}
+
--
2.40.1
Add the invalid filter test which sets the filter beyond the event
space and sets the invalid action to double check if the
KVM_ARM_VCPU_PMU_V3_FILTER will return the expected error.
Signed-off-by: Shaoqin Huang <[email protected]>
---
.../kvm/aarch64/pmu_event_filter_test.c | 38 +++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
index 2dd8ea418f47..86714345ee97 100644
--- a/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
+++ b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
@@ -8,6 +8,7 @@
* This test checks if the guest only see the limited pmu event that userspace
* sets, if the guest can use those events which user allow, and if the guest
* can't use those events which user deny.
+ * It also checks that setting invalid filter ranges return the expected error.
* This test runs only when KVM_CAP_ARM_PMU_V3, KVM_ARM_VCPU_PMU_V3_FILTER
* is supported on the host.
*/
@@ -262,6 +263,41 @@ static void run_tests(void)
run_test(t);
}
+static void test_invalid_filter(void)
+{
+ struct kvm_pmu_event_filter invalid;
+ int ret;
+
+ pr_info("Test: test_invalid_filter\n");
+
+ memset(&vpmu_vm, 0, sizeof(vpmu_vm));
+
+ vpmu_vm.vm = vm_create(1);
+ vpmu_vm.vcpu = vm_vcpu_add_with_vpmu(vpmu_vm.vm, 0, guest_code);
+ vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
+ GICD_BASE_GPA, GICR_BASE_GPA);
+ __TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
+ "Failed to create vgic-v3, skipping");
+
+ /* The max event number is (1 << 16), set a range largeer than it. */
+ invalid = __DEFINE_FILTER(BIT(15), BIT(15) + 1, 0);
+ ret = __kvm_device_attr_set(vpmu_vm.vcpu->fd, KVM_ARM_VCPU_PMU_V3_CTRL,
+ KVM_ARM_VCPU_PMU_V3_FILTER, &invalid);
+ TEST_ASSERT(ret && errno == EINVAL, "Set Invalid filter range "
+ "ret = %d, errno = %d (expected ret = -1, errno = EINVAL)",
+ ret, errno);
+
+ /* Set the Invalid action. */
+ invalid = __DEFINE_FILTER(0, 1, 3);
+ ret = __kvm_device_attr_set(vpmu_vm.vcpu->fd, KVM_ARM_VCPU_PMU_V3_CTRL,
+ KVM_ARM_VCPU_PMU_V3_FILTER, &invalid);
+ TEST_ASSERT(ret && errno == EINVAL, "Set Invalid filter action "
+ "ret = %d, errno = %d (expected ret = -1, errno = EINVAL)",
+ ret, errno);
+
+ destroy_vpmu_vm();
+}
+
static bool kvm_pmu_support_events(void)
{
create_vpmu_vm(guest_get_pmceid);
@@ -284,4 +320,6 @@ int main(void)
TEST_REQUIRE(kvm_pmu_support_events());
run_tests();
+
+ test_invalid_filter();
}
--
2.40.1
Introduce pmu_event_filter_test for arm64 platforms. The test configures
PMUv3 for a vCPU, and sets different pmu event filters for the vCPU, and
check if the guest can see those events which user allow and can't use
those events which use deny.
This test refactor the create_vpmu_vm() and make it a wrapper for
__create_vpmu_vm(), which allows some extra init code before
KVM_ARM_VCPU_PMU_V3_INIT.
And this test use the KVM_ARM_VCPU_PMU_V3_FILTER attribute to set the
pmu event filter in KVM. And choose to filter two common event
branches_retired and instructions_retired, and let the guest to check if
it see the right pmceid register.
Signed-off-by: Shaoqin Huang <[email protected]>
---
tools/testing/selftests/kvm/Makefile | 1 +
.../kvm/aarch64/pmu_event_filter_test.c | 287 ++++++++++++++++++
2 files changed, 288 insertions(+)
create mode 100644 tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index 492e937fab00..732ca5f8bfc0 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -147,6 +147,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test
+TEST_GEN_PROGS_aarch64 += aarch64/pmu_event_filter_test
TEST_GEN_PROGS_aarch64 += aarch64/psci_test
TEST_GEN_PROGS_aarch64 += aarch64/set_id_regs
TEST_GEN_PROGS_aarch64 += aarch64/smccc_filter
diff --git a/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
new file mode 100644
index 000000000000..2dd8ea418f47
--- /dev/null
+++ b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
@@ -0,0 +1,287 @@
+
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * pmu_event_filter_test - Test user limit pmu event for guest.
+ *
+ * Copyright (c) 2023 Red Hat, Inc.
+ *
+ * This test checks if the guest only see the limited pmu event that userspace
+ * sets, if the guest can use those events which user allow, and if the guest
+ * can't use those events which user deny.
+ * This test runs only when KVM_CAP_ARM_PMU_V3, KVM_ARM_VCPU_PMU_V3_FILTER
+ * is supported on the host.
+ */
+#include <kvm_util.h>
+#include <processor.h>
+#include <vgic.h>
+#include <vpmu.h>
+#include <test_util.h>
+#include <perf/arm_pmuv3.h>
+
+struct pmu_common_event_ids {
+ uint64_t pmceid0;
+ uint64_t pmceid1;
+} max_pmce, expected_pmce;
+
+struct vpmu_vm {
+ struct kvm_vm *vm;
+ struct kvm_vcpu *vcpu;
+ int gic_fd;
+};
+
+static struct vpmu_vm vpmu_vm;
+
+#define FILTER_NR 10
+
+struct test_desc {
+ const char *name;
+ struct kvm_pmu_event_filter filter[FILTER_NR];
+};
+
+#define __DEFINE_FILTER(base, num, act) \
+ ((struct kvm_pmu_event_filter) { \
+ .base_event = base, \
+ .nevents = num, \
+ .action = act, \
+ })
+
+#define DEFINE_FILTER(base, act) __DEFINE_FILTER(base, 1, act)
+
+static void guest_code(void)
+{
+ uint64_t pmceid0 = read_sysreg(pmceid0_el0);
+ uint64_t pmceid1 = read_sysreg(pmceid1_el0);
+
+ GUEST_ASSERT_EQ(expected_pmce.pmceid0, pmceid0);
+ GUEST_ASSERT_EQ(expected_pmce.pmceid1, pmceid1);
+
+ GUEST_DONE();
+}
+
+static void guest_get_pmceid(void)
+{
+ max_pmce.pmceid0 = read_sysreg(pmceid0_el0);
+ max_pmce.pmceid1 = read_sysreg(pmceid1_el0);
+
+ GUEST_DONE();
+}
+
+static void run_vcpu(struct kvm_vcpu *vcpu)
+{
+ struct ucall uc;
+
+ while (1) {
+ vcpu_run(vcpu);
+ switch (get_ucall(vcpu, &uc)) {
+ case UCALL_DONE:
+ return;
+ case UCALL_ABORT:
+ REPORT_GUEST_ASSERT(uc);
+ break;
+ default:
+ TEST_FAIL("Unknown ucall %lu", uc.cmd);
+ }
+ }
+}
+
+static void set_pmce(struct pmu_common_event_ids *pmce, int action, int event)
+{
+ int base = 0;
+ uint64_t *pmceid = NULL;
+
+ if (event >= 0x4000) {
+ event -= 0x4000;
+ base = 32;
+ }
+
+ if (event >= 0 && event <= 0x1F) {
+ pmceid = &pmce->pmceid0;
+ } else if (event >= 0x20 && event <= 0x3F) {
+ event -= 0x20;
+ pmceid = &pmce->pmceid1;
+ } else {
+ return;
+ }
+
+ event += base;
+ if (action == KVM_PMU_EVENT_ALLOW)
+ *pmceid |= BIT(event);
+ else
+ *pmceid &= ~BIT(event);
+}
+
+static void prepare_expected_pmce(struct kvm_pmu_event_filter *filter)
+{
+ struct pmu_common_event_ids pmce_mask = { ~0, ~0 };
+ bool first_filter = true;
+
+ while (filter && filter->nevents != 0) {
+ if (first_filter) {
+ if (filter->action == KVM_PMU_EVENT_ALLOW)
+ memset(&pmce_mask, 0, sizeof(pmce_mask));
+ first_filter = false;
+ }
+
+ set_pmce(&pmce_mask, filter->action, filter->base_event);
+ filter++;
+ }
+
+ expected_pmce.pmceid0 = max_pmce.pmceid0 & pmce_mask.pmceid0;
+ expected_pmce.pmceid1 = max_pmce.pmceid1 & pmce_mask.pmceid1;
+}
+
+static void pmu_event_filter_init(struct kvm_pmu_event_filter *filter)
+{
+ while (filter && filter->nevents != 0) {
+ kvm_device_attr_set(vpmu_vm.vcpu->fd,
+ KVM_ARM_VCPU_PMU_V3_CTRL,
+ KVM_ARM_VCPU_PMU_V3_FILTER,
+ filter);
+ filter++;
+ }
+}
+
+#define GICD_BASE_GPA 0x8000000ULL
+#define GICR_BASE_GPA 0x80A0000ULL
+
+/* Create a VM that has one vCPU with PMUv3 configured. */
+static void create_vpmu_vm_with_filter(void *guest_code,
+ struct kvm_pmu_event_filter *filter)
+{
+ uint64_t irq = 23;
+
+ /* The test creates the vpmu_vm multiple times. Ensure a clean state */
+ memset(&vpmu_vm, 0, sizeof(vpmu_vm));
+
+ vpmu_vm.vm = vm_create(1);
+ vpmu_vm.vcpu = vm_vcpu_add_with_vpmu(vpmu_vm.vm, 0, guest_code);
+ vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
+ GICD_BASE_GPA, GICR_BASE_GPA);
+ __TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
+ "Failed to create vgic-v3, skipping");
+
+ pmu_event_filter_init(filter);
+
+ /* Initialize vPMU */
+ vpmu_set_irq(vpmu_vm.vcpu, irq);
+ vpmu_init(vpmu_vm.vcpu);
+}
+
+static void create_vpmu_vm(void *guest_code)
+{
+ create_vpmu_vm_with_filter(guest_code, NULL);
+}
+
+static void destroy_vpmu_vm(void)
+{
+ close(vpmu_vm.gic_fd);
+ kvm_vm_free(vpmu_vm.vm);
+}
+
+static void run_test(struct test_desc *t)
+{
+ pr_info("Test: %s\n", t->name);
+
+ create_vpmu_vm_with_filter(guest_code, t->filter);
+ prepare_expected_pmce(t->filter);
+ sync_global_to_guest(vpmu_vm.vm, expected_pmce);
+
+ run_vcpu(vpmu_vm.vcpu);
+
+ destroy_vpmu_vm();
+}
+
+static struct test_desc tests[] = {
+ {
+ .name = "without_filter",
+ .filter = {
+ { 0 }
+ },
+ },
+ {
+ .name = "member_allow_filter",
+ .filter = {
+ DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 0),
+ DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_INST_RETIRED, 0),
+ DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_BR_RETIRED, 0),
+ { 0 },
+ },
+ },
+ {
+ .name = "member_deny_filter",
+ .filter = {
+ DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 1),
+ DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_INST_RETIRED, 1),
+ DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_BR_RETIRED, 1),
+ { 0 },
+ },
+ },
+ {
+ .name = "not_member_deny_filter",
+ .filter = {
+ DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 1),
+ { 0 },
+ },
+ },
+ {
+ .name = "not_member_allow_filter",
+ .filter = {
+ DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 0),
+ { 0 },
+ },
+ },
+ {
+ .name = "deny_chain_filter",
+ .filter = {
+ DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CHAIN, 1),
+ { 0 },
+ },
+ },
+ {
+ .name = "deny_cpu_cycles_filter",
+ .filter = {
+ DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 1),
+ { 0 },
+ },
+ },
+ {
+ .name = "cancle_filter",
+ .filter = {
+ DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 0),
+ DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 1),
+ },
+ },
+ { 0 }
+};
+
+static void run_tests(void)
+{
+ struct test_desc *t;
+
+ for (t = &tests[0]; t->name; t++)
+ run_test(t);
+}
+
+static bool kvm_pmu_support_events(void)
+{
+ create_vpmu_vm(guest_get_pmceid);
+
+ memset(&max_pmce, 0, sizeof(max_pmce));
+ sync_global_to_guest(vpmu_vm.vm, max_pmce);
+ run_vcpu(vpmu_vm.vcpu);
+ sync_global_from_guest(vpmu_vm.vm, max_pmce);
+ destroy_vpmu_vm();
+
+ return max_pmce.pmceid0 &
+ (ARMV8_PMUV3_PERFCTR_BR_RETIRED |
+ ARMV8_PMUV3_PERFCTR_INST_RETIRED |
+ ARMV8_PMUV3_PERFCTR_CHAIN);
+}
+
+int main(void)
+{
+ TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_PMU_V3));
+ TEST_REQUIRE(kvm_pmu_support_events());
+
+ run_tests();
+}
--
2.40.1
Hi Shaoqin,
On 2/29/24 07:56, Shaoqin Huang wrote:
> Create a vcpu with vpmu would be a common requirement for the vpmu test,
> so add the helper function for the vpmu vcpu creation. And use those
> helper function in the vpmu_counter_access.c test.
>
> Use this chance to delete the meaningless ASSERT about the pmuver,
> because KVM does not advertise an IMP_DEF PMU to guests.
>
> No functional changes intended.
>
> Signed-off-by: Shaoqin Huang <[email protected]>
> ---
> .../kvm/aarch64/vpmu_counter_access.c | 33 ++++---------------
> .../selftests/kvm/include/aarch64/vpmu.h | 29 ++++++++++++++++
> 2 files changed, 36 insertions(+), 26 deletions(-)
> create mode 100644 tools/testing/selftests/kvm/include/aarch64/vpmu.h
>
> diff --git a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
> index 5f9713364693..e068a0a7a43b 100644
> --- a/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
> +++ b/tools/testing/selftests/kvm/aarch64/vpmu_counter_access.c
> @@ -16,6 +16,7 @@
> #include <processor.h>
> #include <test_util.h>
> #include <vgic.h>
> +#include <vpmu.h>
> #include <perf/arm_pmuv3.h>
> #include <linux/bitfield.h>
>
> @@ -426,18 +427,8 @@ static void guest_code(uint64_t expected_pmcr_n)
> /* Create a VM that has one vCPU with PMUv3 configured. */
> static void create_vpmu_vm(void *guest_code)
> {
> - struct kvm_vcpu_init init;
> - uint8_t pmuver, ec;
> - uint64_t dfr0, irq = 23;
> - struct kvm_device_attr irq_attr = {
> - .group = KVM_ARM_VCPU_PMU_V3_CTRL,
> - .attr = KVM_ARM_VCPU_PMU_V3_IRQ,
> - .addr = (uint64_t)&irq,
> - };
> - struct kvm_device_attr init_attr = {
> - .group = KVM_ARM_VCPU_PMU_V3_CTRL,
> - .attr = KVM_ARM_VCPU_PMU_V3_INIT,
> - };
> + uint8_t ec;
> + uint64_t irq = 23;
>
> /* The test creates the vpmu_vm multiple times. Ensure a clean state */
> memset(&vpmu_vm, 0, sizeof(vpmu_vm));
> @@ -449,27 +440,17 @@ static void create_vpmu_vm(void *guest_code)
> guest_sync_handler);
> }
>
> - /* Create vCPU with PMUv3 */
> - vm_ioctl(vpmu_vm.vm, KVM_ARM_PREFERRED_TARGET, &init);
> - init.features[0] |= (1 << KVM_ARM_VCPU_PMU_V3);
> - vpmu_vm.vcpu = aarch64_vcpu_add(vpmu_vm.vm, 0, &init, guest_code);
> + vpmu_vm.vcpu = vm_vcpu_add_with_vpmu(vpmu_vm.vm, 0, guest_code);
> vcpu_init_descriptor_tables(vpmu_vm.vcpu);
> +
> vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
> GICD_BASE_GPA, GICR_BASE_GPA);
> __TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
> "Failed to create vgic-v3, skipping");
>
> - /* Make sure that PMUv3 support is indicated in the ID register */
> - vcpu_get_reg(vpmu_vm.vcpu,
> - KVM_ARM64_SYS_REG(SYS_ID_AA64DFR0_EL1), &dfr0);
> - pmuver = FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer), dfr0);
> - TEST_ASSERT(pmuver != ID_AA64DFR0_EL1_PMUVer_IMP_DEF &&
> - pmuver >= ID_AA64DFR0_EL1_PMUVer_IMP,
> - "Unexpected PMUVER (0x%x) on the vCPU with PMUv3", pmuver);
> -
> /* Initialize vPMU */
> - vcpu_ioctl(vpmu_vm.vcpu, KVM_SET_DEVICE_ATTR, &irq_attr);
> - vcpu_ioctl(vpmu_vm.vcpu, KVM_SET_DEVICE_ATTR, &init_attr);
> + vpmu_set_irq(vpmu_vm.vcpu, irq);
> + vpmu_init(vpmu_vm.vcpu);
> }
>
> static void destroy_vpmu_vm(void)
> diff --git a/tools/testing/selftests/kvm/include/aarch64/vpmu.h b/tools/testing/selftests/kvm/include/aarch64/vpmu.h
> new file mode 100644
> index 000000000000..0dfcc7ab1c4d
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/include/aarch64/vpmu.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#include <kvm_util.h>
> +
> +static inline struct kvm_vcpu *vm_vcpu_add_with_vpmu(struct kvm_vm *vm,
> + uint32_t vcpu_id,
> + void *guest_code)
> +{
> + struct kvm_vcpu_init init;
> +
> + /* Create vCPU with PMUv3 */
> + vm_ioctl(vm, KVM_ARM_PREFERRED_TARGET, &init);
> + init.features[0] |= (1 << KVM_ARM_VCPU_PMU_V3);
> +
> + return aarch64_vcpu_add(vm, 0, &init, guest_code);
> +}
> +
> +static void vpmu_set_irq(struct kvm_vcpu *vcpu, int irq)
> +{
> + kvm_device_attr_set(vcpu->fd, KVM_ARM_VCPU_PMU_V3_CTRL,
> + KVM_ARM_VCPU_PMU_V3_IRQ, &irq);
> +}
> +
> +static void vpmu_init(struct kvm_vcpu *vcpu)
> +{
> + kvm_device_attr_set(vcpu->fd, KVM_ARM_VCPU_PMU_V3_CTRL,
> + KVM_ARM_VCPU_PMU_V3_INIT, NULL);
> +}
> +
Reviewed-by: Eric Auger <[email protected]>
Thanks
Eric
Hi Shaoqin,
On 2/29/24 07:56, Shaoqin Huang wrote:
> Introduce pmu_event_filter_test for arm64 platforms. The test configures
> PMUv3 for a vCPU, and sets different pmu event filters for the vCPU, and
> check if the guest can see those events which user allow and can't use
> those events which use deny.
>
> This test refactor the create_vpmu_vm() and make it a wrapper for
> __create_vpmu_vm(), which allows some extra init code before
> KVM_ARM_VCPU_PMU_V3_INIT.
>
> And this test use the KVM_ARM_VCPU_PMU_V3_FILTER attribute to set the
> pmu event filter in KVM. And choose to filter two common event
> branches_retired and instructions_retired, and let the guest to check if
> it see the right pmceid register.
>
> Signed-off-by: Shaoqin Huang <[email protected]>
> ---
> tools/testing/selftests/kvm/Makefile | 1 +
> .../kvm/aarch64/pmu_event_filter_test.c | 287 ++++++++++++++++++
> 2 files changed, 288 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
>
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 492e937fab00..732ca5f8bfc0 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -147,6 +147,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
> TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
> TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
> TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test
> +TEST_GEN_PROGS_aarch64 += aarch64/pmu_event_filter_test
> TEST_GEN_PROGS_aarch64 += aarch64/psci_test
> TEST_GEN_PROGS_aarch64 += aarch64/set_id_regs
> TEST_GEN_PROGS_aarch64 += aarch64/smccc_filter
> diff --git a/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
> new file mode 100644
> index 000000000000..2dd8ea418f47
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
> @@ -0,0 +1,287 @@
> +
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * pmu_event_filter_test - Test user limit pmu event for guest.
> + *
> + * Copyright (c) 2023 Red Hat, Inc.
> + *
> + * This test checks if the guest only see the limited pmu event that userspace
> + * sets, if the guest can use those events which user allow, and if the guest
> + * can't use those events which user deny.
> + * This test runs only when KVM_CAP_ARM_PMU_V3, KVM_ARM_VCPU_PMU_V3_FILTER
> + * is supported on the host.
> + */
> +#include <kvm_util.h>
> +#include <processor.h>
> +#include <vgic.h>
> +#include <vpmu.h>
> +#include <test_util.h>
> +#include <perf/arm_pmuv3.h>
> +
> +struct pmu_common_event_ids {
> + uint64_t pmceid0;
> + uint64_t pmceid1;
> +} max_pmce, expected_pmce;
> +
> +struct vpmu_vm {
> + struct kvm_vm *vm;
> + struct kvm_vcpu *vcpu;
> + int gic_fd;
> +};
> +
> +static struct vpmu_vm vpmu_vm;
> +
> +#define FILTER_NR 10
> +
> +struct test_desc {
> + const char *name;
> + struct kvm_pmu_event_filter filter[FILTER_NR];
> +};
> +
> +#define __DEFINE_FILTER(base, num, act) \
> + ((struct kvm_pmu_event_filter) { \
> + .base_event = base, \
> + .nevents = num, \
> + .action = act, \
> + })
> +
> +#define DEFINE_FILTER(base, act) __DEFINE_FILTER(base, 1, act)
> +
> +static void guest_code(void)
> +{
> + uint64_t pmceid0 = read_sysreg(pmceid0_el0);
> + uint64_t pmceid1 = read_sysreg(pmceid1_el0);
> +
> + GUEST_ASSERT_EQ(expected_pmce.pmceid0, pmceid0);
> + GUEST_ASSERT_EQ(expected_pmce.pmceid1, pmceid1);
> +
> + GUEST_DONE();
> +}
> +
> +static void guest_get_pmceid(void)
> +{
> + max_pmce.pmceid0 = read_sysreg(pmceid0_el0);
> + max_pmce.pmceid1 = read_sysreg(pmceid1_el0);
> +
> + GUEST_DONE();
> +}
> +
> +static void run_vcpu(struct kvm_vcpu *vcpu)
> +{
> + struct ucall uc;
> +
> + while (1) {
> + vcpu_run(vcpu);
> + switch (get_ucall(vcpu, &uc)) {
> + case UCALL_DONE:
> + return;
> + case UCALL_ABORT:
> + REPORT_GUEST_ASSERT(uc);
> + break;
> + default:
> + TEST_FAIL("Unknown ucall %lu", uc.cmd);
> + }
> + }
> +}
> +
> +static void set_pmce(struct pmu_common_event_ids *pmce, int action, int event)
> +{
> + int base = 0;
> + uint64_t *pmceid = NULL;
> +
> + if (event >= 0x4000) {
> + event -= 0x4000;
> + base = 32;
> + }
> +
> + if (event >= 0 && event <= 0x1F) {
> + pmceid = &pmce->pmceid0;
> + } else if (event >= 0x20 && event <= 0x3F) {
> + event -= 0x20;
> + pmceid = &pmce->pmceid1;
> + } else {
> + return;
> + }
> +
> + event += base;
> + if (action == KVM_PMU_EVENT_ALLOW)
> + *pmceid |= BIT(event);
> + else
> + *pmceid &= ~BIT(event);
> +}
> +
> +static void prepare_expected_pmce(struct kvm_pmu_event_filter *filter)
> +{
> + struct pmu_common_event_ids pmce_mask = { ~0, ~0 };
> + bool first_filter = true;
> +
> + while (filter && filter->nevents != 0) {
isn't filter->nevents != 0 sufficient?
> + if (first_filter) {
> + if (filter->action == KVM_PMU_EVENT_ALLOW)
> + memset(&pmce_mask, 0, sizeof(pmce_mask));
> + first_filter = false;
> + }
> +
> + set_pmce(&pmce_mask, filter->action, filter->base_event);
> + filter++;
> + }
> +
> + expected_pmce.pmceid0 = max_pmce.pmceid0 & pmce_mask.pmceid0;
> + expected_pmce.pmceid1 = max_pmce.pmceid1 & pmce_mask.pmceid1;
> +}
> +
> +static void pmu_event_filter_init(struct kvm_pmu_event_filter *filter)
> +{
> + while (filter && filter->nevents != 0) {
> + kvm_device_attr_set(vpmu_vm.vcpu->fd,
> + KVM_ARM_VCPU_PMU_V3_CTRL,
> + KVM_ARM_VCPU_PMU_V3_FILTER,
> + filter);
> + filter++;
> + }
> +}
> +
> +#define GICD_BASE_GPA 0x8000000ULL
> +#define GICR_BASE_GPA 0x80A0000ULL
I think Oliver suggested to move those defines to GIC headers?
> +
> +/* Create a VM that has one vCPU with PMUv3 configured. */
> +static void create_vpmu_vm_with_filter(void *guest_code,
> + struct kvm_pmu_event_filter *filter)
> +{
> + uint64_t irq = 23;
> +
> + /* The test creates the vpmu_vm multiple times. Ensure a clean state */
> + memset(&vpmu_vm, 0, sizeof(vpmu_vm));
> +
> + vpmu_vm.vm = vm_create(1);
> + vpmu_vm.vcpu = vm_vcpu_add_with_vpmu(vpmu_vm.vm, 0, guest_code);
> + vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
> + GICD_BASE_GPA, GICR_BASE_GPA);
> + __TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
> + "Failed to create vgic-v3, skipping");
> +
> + pmu_event_filter_init(filter);
> +
> + /* Initialize vPMU */
> + vpmu_set_irq(vpmu_vm.vcpu, irq);
> + vpmu_init(vpmu_vm.vcpu);
> +}
> +
> +static void create_vpmu_vm(void *guest_code)
> +{
> + create_vpmu_vm_with_filter(guest_code, NULL);
> +}
> +
> +static void destroy_vpmu_vm(void)
> +{
> + close(vpmu_vm.gic_fd);
> + kvm_vm_free(vpmu_vm.vm);
> +}
> +
> +static void run_test(struct test_desc *t)
> +{
> + pr_info("Test: %s\n", t->name);
> +
> + create_vpmu_vm_with_filter(guest_code, t->filter);
> + prepare_expected_pmce(t->filter);
> + sync_global_to_guest(vpmu_vm.vm, expected_pmce);
> +
> + run_vcpu(vpmu_vm.vcpu);
> +
> + destroy_vpmu_vm();
> +}
> +
> +static struct test_desc tests[] = {
> + {
> + .name = "without_filter",
> + .filter = {
> + { 0 }
> + },
> + },
> + {
> + .name = "member_allow_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 0),
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_INST_RETIRED, 0),
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_BR_RETIRED, 0),
> + { 0 },
> + },
> + },
> + {
> + .name = "member_deny_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 1),
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_INST_RETIRED, 1),
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_BR_RETIRED, 1),
> + { 0 },
> + },
> + },
> + {
> + .name = "not_member_deny_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 1),
> + { 0 },
> + },
> + },
> + {
> + .name = "not_member_allow_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 0),
> + { 0 },
> + },
> + },
> + {
> + .name = "deny_chain_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CHAIN, 1),
> + { 0 },
> + },
> + },
> + {
> + .name = "deny_cpu_cycles_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 1),
> + { 0 },
> + },
> + },
> + {
> + .name = "cancle_filter",
cancel
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 0),
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 1),
> + },
> + },
> + { 0 }
> +};
> +
> +static void run_tests(void)
> +{
> + struct test_desc *t;
> +
> + for (t = &tests[0]; t->name; t++)
> + run_test(t);
> +}
> +
> +static bool kvm_pmu_support_events(void)
> +{
> + create_vpmu_vm(guest_get_pmceid);
> +
> + memset(&max_pmce, 0, sizeof(max_pmce));
> + sync_global_to_guest(vpmu_vm.vm, max_pmce);
> + run_vcpu(vpmu_vm.vcpu);
> + sync_global_from_guest(vpmu_vm.vm, max_pmce);
> + destroy_vpmu_vm();
> +
> + return max_pmce.pmceid0 &
> + (ARMV8_PMUV3_PERFCTR_BR_RETIRED |
> + ARMV8_PMUV3_PERFCTR_INST_RETIRED |
> + ARMV8_PMUV3_PERFCTR_CHAIN);
I understood Oliver suggested to use sysfs to know if te events were
supported and to use KVM_ARM_VCPU_PMU_V3_SET_PMU. On my end I am fine
with your method though.
Thanks
Eric
> +}
> +
> +int main(void)
> +{
> + TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_PMU_V3));
> + TEST_REQUIRE(kvm_pmu_support_events());
> +
> + run_tests();
> +}
On 2/29/24 07:56, Shaoqin Huang wrote:
> Add the invalid filter test which sets the filter beyond the event
> space and sets the invalid action to double check if the
> KVM_ARM_VCPU_PMU_V3_FILTER will return the expected error.
>
> Signed-off-by: Shaoqin Huang <[email protected]>
> ---
> .../kvm/aarch64/pmu_event_filter_test.c | 38 +++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
> index 2dd8ea418f47..86714345ee97 100644
> --- a/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
> +++ b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
> @@ -8,6 +8,7 @@
> * This test checks if the guest only see the limited pmu event that userspace
> * sets, if the guest can use those events which user allow, and if the guest
> * can't use those events which user deny.
> + * It also checks that setting invalid filter ranges return the expected error.
> * This test runs only when KVM_CAP_ARM_PMU_V3, KVM_ARM_VCPU_PMU_V3_FILTER
> * is supported on the host.
> */
> @@ -262,6 +263,41 @@ static void run_tests(void)
> run_test(t);
> }
>
> +static void test_invalid_filter(void)
> +{
> + struct kvm_pmu_event_filter invalid;
> + int ret;
> +
> + pr_info("Test: test_invalid_filter\n");
> +
> + memset(&vpmu_vm, 0, sizeof(vpmu_vm));
> +
> + vpmu_vm.vm = vm_create(1);
> + vpmu_vm.vcpu = vm_vcpu_add_with_vpmu(vpmu_vm.vm, 0, guest_code);
> + vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
> + GICD_BASE_GPA, GICR_BASE_GPA);
> + __TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
> + "Failed to create vgic-v3, skipping");
> +
> + /* The max event number is (1 << 16), set a range largeer than it. */the event space is 16b on ARMv8.1 and 10b on ARMv8.0, set a range larger
than 16
s/largeer/larger
> + invalid = __DEFINE_FILTER(BIT(15), BIT(15) + 1, 0);
> + ret = __kvm_device_attr_set(vpmu_vm.vcpu->fd, KVM_ARM_VCPU_PMU_V3_CTRL,
> + KVM_ARM_VCPU_PMU_V3_FILTER, &invalid);
> + TEST_ASSERT(ret && errno == EINVAL, "Set Invalid filter range "
> + "ret = %d, errno = %d (expected ret = -1, errno = EINVAL)",
> + ret, errno);
> +
> + /* Set the Invalid action. */
s/the/an
> + invalid = __DEFINE_FILTER(0, 1, 3);
> + ret = __kvm_device_attr_set(vpmu_vm.vcpu->fd, KVM_ARM_VCPU_PMU_V3_CTRL,
> + KVM_ARM_VCPU_PMU_V3_FILTER, &invalid);
> + TEST_ASSERT(ret && errno == EINVAL, "Set Invalid filter action "
> + "ret = %d, errno = %d (expected ret = -1, errno = EINVAL)",
> + ret, errno);
> +
> + destroy_vpmu_vm();
> +}
> +
> static bool kvm_pmu_support_events(void)
> {
> create_vpmu_vm(guest_get_pmceid);
> @@ -284,4 +320,6 @@ int main(void)
> TEST_REQUIRE(kvm_pmu_support_events());
>
> run_tests();
> +
> + test_invalid_filter();
> }
Besides Reviewed-by: Eric Auger <[email protected]>
Eric
On 2/29/24 07:56, Shaoqin Huang wrote:
> Introduce pmu_event_filter_test for arm64 platforms. The test configures
> PMUv3 for a vCPU, and sets different pmu event filters for the vCPU, and
> check if the guest can see those events which user allow and can't use
> those events which use deny.
>
> This test refactor the create_vpmu_vm() and make it a wrapper for
> __create_vpmu_vm(), which allows some extra init code before
> KVM_ARM_VCPU_PMU_V3_INIT.
>
> And this test use the KVM_ARM_VCPU_PMU_V3_FILTER attribute to set the
> pmu event filter in KVM. And choose to filter two common event
> branches_retired and instructions_retired, and let the guest to check if
> it see the right pmceid register.
>
> Signed-off-by: Shaoqin Huang <[email protected]>
> ---
> tools/testing/selftests/kvm/Makefile | 1 +
> .../kvm/aarch64/pmu_event_filter_test.c | 287 ++++++++++++++++++
> 2 files changed, 288 insertions(+)
> create mode 100644 tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
>
> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
> index 492e937fab00..732ca5f8bfc0 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -147,6 +147,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
> TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
> TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
> TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test
> +TEST_GEN_PROGS_aarch64 += aarch64/pmu_event_filter_test
> TEST_GEN_PROGS_aarch64 += aarch64/psci_test
> TEST_GEN_PROGS_aarch64 += aarch64/set_id_regs
> TEST_GEN_PROGS_aarch64 += aarch64/smccc_filter
> diff --git a/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
> new file mode 100644
> index 000000000000..2dd8ea418f47
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
> @@ -0,0 +1,287 @@
> +
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * pmu_event_filter_test - Test user limit pmu event for guest.
> + *
> + * Copyright (c) 2023 Red Hat, Inc.
> + *
> + * This test checks if the guest only see the limited pmu event that userspace
> + * sets, if the guest can use those events which user allow, and if the guest
> + * can't use those events which user deny.
> + * This test runs only when KVM_CAP_ARM_PMU_V3, KVM_ARM_VCPU_PMU_V3_FILTER
> + * is supported on the host.
> + */
> +#include <kvm_util.h>
> +#include <processor.h>
> +#include <vgic.h>
> +#include <vpmu.h>
> +#include <test_util.h>
> +#include <perf/arm_pmuv3.h>
> +
> +struct pmu_common_event_ids {
> + uint64_t pmceid0;
> + uint64_t pmceid1;
> +} max_pmce, expected_pmce;
> +
> +struct vpmu_vm {
> + struct kvm_vm *vm;
> + struct kvm_vcpu *vcpu;
> + int gic_fd;
> +};
> +
> +static struct vpmu_vm vpmu_vm;
> +
> +#define FILTER_NR 10
> +
> +struct test_desc {
> + const char *name;
> + struct kvm_pmu_event_filter filter[FILTER_NR];
> +};
> +
> +#define __DEFINE_FILTER(base, num, act) \
> + ((struct kvm_pmu_event_filter) { \
> + .base_event = base, \
> + .nevents = num, \
> + .action = act, \
> + })
> +
> +#define DEFINE_FILTER(base, act) __DEFINE_FILTER(base, 1, act)
that's a pity you are never exercising filters with more than 1 events.
Eric
> +
> +static void guest_code(void)
> +{
> + uint64_t pmceid0 = read_sysreg(pmceid0_el0);
> + uint64_t pmceid1 = read_sysreg(pmceid1_el0);
> +
> + GUEST_ASSERT_EQ(expected_pmce.pmceid0, pmceid0);
> + GUEST_ASSERT_EQ(expected_pmce.pmceid1, pmceid1);
> +
> + GUEST_DONE();
> +}
> +
> +static void guest_get_pmceid(void)
> +{
> + max_pmce.pmceid0 = read_sysreg(pmceid0_el0);
> + max_pmce.pmceid1 = read_sysreg(pmceid1_el0);
> +
> + GUEST_DONE();
> +}
> +
> +static void run_vcpu(struct kvm_vcpu *vcpu)
> +{
> + struct ucall uc;
> +
> + while (1) {
> + vcpu_run(vcpu);
> + switch (get_ucall(vcpu, &uc)) {
> + case UCALL_DONE:
> + return;
> + case UCALL_ABORT:
> + REPORT_GUEST_ASSERT(uc);
> + break;
> + default:
> + TEST_FAIL("Unknown ucall %lu", uc.cmd);
> + }
> + }
> +}
> +
> +static void set_pmce(struct pmu_common_event_ids *pmce, int action, int event)
> +{
> + int base = 0;
> + uint64_t *pmceid = NULL;
> +
> + if (event >= 0x4000) {
> + event -= 0x4000;
> + base = 32;
> + }
> +
> + if (event >= 0 && event <= 0x1F) {
> + pmceid = &pmce->pmceid0;
> + } else if (event >= 0x20 && event <= 0x3F) {
> + event -= 0x20;
> + pmceid = &pmce->pmceid1;
> + } else {
> + return;
> + }
> +
> + event += base;
> + if (action == KVM_PMU_EVENT_ALLOW)
> + *pmceid |= BIT(event);
> + else
> + *pmceid &= ~BIT(event);
> +}
> +
> +static void prepare_expected_pmce(struct kvm_pmu_event_filter *filter)
> +{
> + struct pmu_common_event_ids pmce_mask = { ~0, ~0 };
> + bool first_filter = true;
> +
> + while (filter && filter->nevents != 0) {
> + if (first_filter) {
> + if (filter->action == KVM_PMU_EVENT_ALLOW)
> + memset(&pmce_mask, 0, sizeof(pmce_mask));
> + first_filter = false;
> + }
> +
> + set_pmce(&pmce_mask, filter->action, filter->base_event);
> + filter++;
> + }
> +
> + expected_pmce.pmceid0 = max_pmce.pmceid0 & pmce_mask.pmceid0;
> + expected_pmce.pmceid1 = max_pmce.pmceid1 & pmce_mask.pmceid1;
> +}
> +
> +static void pmu_event_filter_init(struct kvm_pmu_event_filter *filter)
> +{
> + while (filter && filter->nevents != 0) {
> + kvm_device_attr_set(vpmu_vm.vcpu->fd,
> + KVM_ARM_VCPU_PMU_V3_CTRL,
> + KVM_ARM_VCPU_PMU_V3_FILTER,
> + filter);
> + filter++;
> + }
> +}
> +
> +#define GICD_BASE_GPA 0x8000000ULL
> +#define GICR_BASE_GPA 0x80A0000ULL
> +
> +/* Create a VM that has one vCPU with PMUv3 configured. */
> +static void create_vpmu_vm_with_filter(void *guest_code,
> + struct kvm_pmu_event_filter *filter)
> +{
> + uint64_t irq = 23;
> +
> + /* The test creates the vpmu_vm multiple times. Ensure a clean state */
> + memset(&vpmu_vm, 0, sizeof(vpmu_vm));
> +
> + vpmu_vm.vm = vm_create(1);
> + vpmu_vm.vcpu = vm_vcpu_add_with_vpmu(vpmu_vm.vm, 0, guest_code);
> + vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
> + GICD_BASE_GPA, GICR_BASE_GPA);
> + __TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
> + "Failed to create vgic-v3, skipping");
> +
> + pmu_event_filter_init(filter);
> +
> + /* Initialize vPMU */
> + vpmu_set_irq(vpmu_vm.vcpu, irq);
> + vpmu_init(vpmu_vm.vcpu);
> +}
> +
> +static void create_vpmu_vm(void *guest_code)
> +{
> + create_vpmu_vm_with_filter(guest_code, NULL);
> +}
> +
> +static void destroy_vpmu_vm(void)
> +{
> + close(vpmu_vm.gic_fd);
> + kvm_vm_free(vpmu_vm.vm);
> +}
> +
> +static void run_test(struct test_desc *t)
> +{
> + pr_info("Test: %s\n", t->name);
> +
> + create_vpmu_vm_with_filter(guest_code, t->filter);
> + prepare_expected_pmce(t->filter);
> + sync_global_to_guest(vpmu_vm.vm, expected_pmce);
> +
> + run_vcpu(vpmu_vm.vcpu);
> +
> + destroy_vpmu_vm();
> +}
> +
> +static struct test_desc tests[] = {
> + {
> + .name = "without_filter",
> + .filter = {
> + { 0 }
> + },
> + },
> + {
> + .name = "member_allow_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 0),
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_INST_RETIRED, 0),
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_BR_RETIRED, 0),
> + { 0 },
> + },
> + },
> + {
> + .name = "member_deny_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 1),
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_INST_RETIRED, 1),
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_BR_RETIRED, 1),
> + { 0 },
> + },
> + },
> + {
> + .name = "not_member_deny_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 1),
> + { 0 },
> + },
> + },
> + {
> + .name = "not_member_allow_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 0),
> + { 0 },
> + },
> + },
> + {
> + .name = "deny_chain_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CHAIN, 1),
> + { 0 },
> + },
> + },
> + {
> + .name = "deny_cpu_cycles_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 1),
> + { 0 },
> + },
> + },
> + {
> + .name = "cancle_filter",
> + .filter = {
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 0),
> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 1),
> + },
> + },
> + { 0 }
> +};
> +
> +static void run_tests(void)
> +{
> + struct test_desc *t;
> +
> + for (t = &tests[0]; t->name; t++)
> + run_test(t);
> +}
> +
> +static bool kvm_pmu_support_events(void)
> +{
> + create_vpmu_vm(guest_get_pmceid);
> +
> + memset(&max_pmce, 0, sizeof(max_pmce));
> + sync_global_to_guest(vpmu_vm.vm, max_pmce);
> + run_vcpu(vpmu_vm.vcpu);
> + sync_global_from_guest(vpmu_vm.vm, max_pmce);
> + destroy_vpmu_vm();
> +
> + return max_pmce.pmceid0 &
> + (ARMV8_PMUV3_PERFCTR_BR_RETIRED |
> + ARMV8_PMUV3_PERFCTR_INST_RETIRED |
> + ARMV8_PMUV3_PERFCTR_CHAIN);
> +}
> +
> +int main(void)
> +{
> + TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_PMU_V3));
> + TEST_REQUIRE(kvm_pmu_support_events());
> +
> + run_tests();
> +}
Hi Eric,
Thanks for your review.
On 3/20/24 04:58, Eric Auger wrote:
> Hi Shaoqin,
>
> On 2/29/24 07:56, Shaoqin Huang wrote:
>> Introduce pmu_event_filter_test for arm64 platforms. The test configures
>> PMUv3 for a vCPU, and sets different pmu event filters for the vCPU, and
>> check if the guest can see those events which user allow and can't use
>> those events which use deny.
>>
>> This test refactor the create_vpmu_vm() and make it a wrapper for
>> __create_vpmu_vm(), which allows some extra init code before
>> KVM_ARM_VCPU_PMU_V3_INIT.
>>
>> And this test use the KVM_ARM_VCPU_PMU_V3_FILTER attribute to set the
>> pmu event filter in KVM. And choose to filter two common event
>> branches_retired and instructions_retired, and let the guest to check if
>> it see the right pmceid register.
>>
>> Signed-off-by: Shaoqin Huang <[email protected]>
>> ---
>> tools/testing/selftests/kvm/Makefile | 1 +
>> .../kvm/aarch64/pmu_event_filter_test.c | 287 ++++++++++++++++++
>> 2 files changed, 288 insertions(+)
>> create mode 100644 tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
>>
>> diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
>> index 492e937fab00..732ca5f8bfc0 100644
>> --- a/tools/testing/selftests/kvm/Makefile
>> +++ b/tools/testing/selftests/kvm/Makefile
>> @@ -147,6 +147,7 @@ TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
>> TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
>> TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
>> TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test
>> +TEST_GEN_PROGS_aarch64 += aarch64/pmu_event_filter_test
>> TEST_GEN_PROGS_aarch64 += aarch64/psci_test
>> TEST_GEN_PROGS_aarch64 += aarch64/set_id_regs
>> TEST_GEN_PROGS_aarch64 += aarch64/smccc_filter
>> diff --git a/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
>> new file mode 100644
>> index 000000000000..2dd8ea418f47
>> --- /dev/null
>> +++ b/tools/testing/selftests/kvm/aarch64/pmu_event_filter_test.c
>> @@ -0,0 +1,287 @@
>> +
>> +// SPDX-License-Identifier: GPL-2.0
>> +/*
>> + * pmu_event_filter_test - Test user limit pmu event for guest.
>> + *
>> + * Copyright (c) 2023 Red Hat, Inc.
>> + *
>> + * This test checks if the guest only see the limited pmu event that userspace
>> + * sets, if the guest can use those events which user allow, and if the guest
>> + * can't use those events which user deny.
>> + * This test runs only when KVM_CAP_ARM_PMU_V3, KVM_ARM_VCPU_PMU_V3_FILTER
>> + * is supported on the host.
>> + */
>> +#include <kvm_util.h>
>> +#include <processor.h>
>> +#include <vgic.h>
>> +#include <vpmu.h>
>> +#include <test_util.h>
>> +#include <perf/arm_pmuv3.h>
>> +
>> +struct pmu_common_event_ids {
>> + uint64_t pmceid0;
>> + uint64_t pmceid1;
>> +} max_pmce, expected_pmce;
>> +
>> +struct vpmu_vm {
>> + struct kvm_vm *vm;
>> + struct kvm_vcpu *vcpu;
>> + int gic_fd;
>> +};
>> +
>> +static struct vpmu_vm vpmu_vm;
>> +
>> +#define FILTER_NR 10
>> +
>> +struct test_desc {
>> + const char *name;
>> + struct kvm_pmu_event_filter filter[FILTER_NR];
>> +};
>> +
>> +#define __DEFINE_FILTER(base, num, act) \
>> + ((struct kvm_pmu_event_filter) { \
>> + .base_event = base, \
>> + .nevents = num, \
>> + .action = act, \
>> + })
>> +
>> +#define DEFINE_FILTER(base, act) __DEFINE_FILTER(base, 1, act)
>> +
>> +static void guest_code(void)
>> +{
>> + uint64_t pmceid0 = read_sysreg(pmceid0_el0);
>> + uint64_t pmceid1 = read_sysreg(pmceid1_el0);
>> +
>> + GUEST_ASSERT_EQ(expected_pmce.pmceid0, pmceid0);
>> + GUEST_ASSERT_EQ(expected_pmce.pmceid1, pmceid1);
>> +
>> + GUEST_DONE();
>> +}
>> +
>> +static void guest_get_pmceid(void)
>> +{
>> + max_pmce.pmceid0 = read_sysreg(pmceid0_el0);
>> + max_pmce.pmceid1 = read_sysreg(pmceid1_el0);
>> +
>> + GUEST_DONE();
>> +}
>> +
>> +static void run_vcpu(struct kvm_vcpu *vcpu)
>> +{
>> + struct ucall uc;
>> +
>> + while (1) {
>> + vcpu_run(vcpu);
>> + switch (get_ucall(vcpu, &uc)) {
>> + case UCALL_DONE:
>> + return;
>> + case UCALL_ABORT:
>> + REPORT_GUEST_ASSERT(uc);
>> + break;
>> + default:
>> + TEST_FAIL("Unknown ucall %lu", uc.cmd);
>> + }
>> + }
>> +}
>> +
>> +static void set_pmce(struct pmu_common_event_ids *pmce, int action, int event)
>> +{
>> + int base = 0;
>> + uint64_t *pmceid = NULL;
>> +
>> + if (event >= 0x4000) {
>> + event -= 0x4000;
>> + base = 32;
>> + }
>> +
>> + if (event >= 0 && event <= 0x1F) {
>> + pmceid = &pmce->pmceid0;
>> + } else if (event >= 0x20 && event <= 0x3F) {
>> + event -= 0x20;
>> + pmceid = &pmce->pmceid1;
>> + } else {
>> + return;
>> + }
>> +
>> + event += base;
>> + if (action == KVM_PMU_EVENT_ALLOW)
>> + *pmceid |= BIT(event);
>> + else
>> + *pmceid &= ~BIT(event);
>> +}
>> +
>> +static void prepare_expected_pmce(struct kvm_pmu_event_filter *filter)
>> +{
>> + struct pmu_common_event_ids pmce_mask = { ~0, ~0 };
>> + bool first_filter = true;
>> +
>> + while (filter && filter->nevents != 0) {
> isn't filter->nevents != 0 sufficient?
No. Because the filter might be NULL.
>> + if (first_filter) {
>> + if (filter->action == KVM_PMU_EVENT_ALLOW)
>> + memset(&pmce_mask, 0, sizeof(pmce_mask));
>> + first_filter = false;
>> + }
>> +
>> + set_pmce(&pmce_mask, filter->action, filter->base_event);
>> + filter++;
>> + }
>> +
>> + expected_pmce.pmceid0 = max_pmce.pmceid0 & pmce_mask.pmceid0;
>> + expected_pmce.pmceid1 = max_pmce.pmceid1 & pmce_mask.pmceid1;
>> +}
>> +
>> +static void pmu_event_filter_init(struct kvm_pmu_event_filter *filter)
>> +{
>> + while (filter && filter->nevents != 0) {
>> + kvm_device_attr_set(vpmu_vm.vcpu->fd,
>> + KVM_ARM_VCPU_PMU_V3_CTRL,
>> + KVM_ARM_VCPU_PMU_V3_FILTER,
>> + filter);
>> + filter++;
>> + }
>> +}
>> +
>> +#define GICD_BASE_GPA 0x8000000ULL
>> +#define GICR_BASE_GPA 0x80A0000ULL
> I think Oliver suggested to move those defines to GIC headers?
There is another patch[1] which move those definition to the GIC
headers, but it hasn't been merged.
[1]https://lore.kernel.org/all/[email protected]/
>> +
>> +/* Create a VM that has one vCPU with PMUv3 configured. */
>> +static void create_vpmu_vm_with_filter(void *guest_code,
>> + struct kvm_pmu_event_filter *filter)
>> +{
>> + uint64_t irq = 23;
>> +
>> + /* The test creates the vpmu_vm multiple times. Ensure a clean state */
>> + memset(&vpmu_vm, 0, sizeof(vpmu_vm));
>> +
>> + vpmu_vm.vm = vm_create(1);
>> + vpmu_vm.vcpu = vm_vcpu_add_with_vpmu(vpmu_vm.vm, 0, guest_code);
>> + vpmu_vm.gic_fd = vgic_v3_setup(vpmu_vm.vm, 1, 64,
>> + GICD_BASE_GPA, GICR_BASE_GPA);
>> + __TEST_REQUIRE(vpmu_vm.gic_fd >= 0,
>> + "Failed to create vgic-v3, skipping");
>> +
>> + pmu_event_filter_init(filter);
>> +
>> + /* Initialize vPMU */
>> + vpmu_set_irq(vpmu_vm.vcpu, irq);
>> + vpmu_init(vpmu_vm.vcpu);
>> +}
>> +
>> +static void create_vpmu_vm(void *guest_code)
>> +{
>> + create_vpmu_vm_with_filter(guest_code, NULL);
>> +}
>> +
>> +static void destroy_vpmu_vm(void)
>> +{
>> + close(vpmu_vm.gic_fd);
>> + kvm_vm_free(vpmu_vm.vm);
>> +}
>> +
>> +static void run_test(struct test_desc *t)
>> +{
>> + pr_info("Test: %s\n", t->name);
>> +
>> + create_vpmu_vm_with_filter(guest_code, t->filter);
>> + prepare_expected_pmce(t->filter);
>> + sync_global_to_guest(vpmu_vm.vm, expected_pmce);
>> +
>> + run_vcpu(vpmu_vm.vcpu);
>> +
>> + destroy_vpmu_vm();
>> +}
>> +
>> +static struct test_desc tests[] = {
>> + {
>> + .name = "without_filter",
>> + .filter = {
>> + { 0 }
>> + },
>> + },
>> + {
>> + .name = "member_allow_filter",
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 0),
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_INST_RETIRED, 0),
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_BR_RETIRED, 0),
>> + { 0 },
>> + },
>> + },
>> + {
>> + .name = "member_deny_filter",
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 1),
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_INST_RETIRED, 1),
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_BR_RETIRED, 1),
>> + { 0 },
>> + },
>> + },
>> + {
>> + .name = "not_member_deny_filter",
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 1),
>> + { 0 },
>> + },
>> + },
>> + {
>> + .name = "not_member_allow_filter",
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_SW_INCR, 0),
>> + { 0 },
>> + },
>> + },
>> + {
>> + .name = "deny_chain_filter",
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CHAIN, 1),
>> + { 0 },
>> + },
>> + },
>> + {
>> + .name = "deny_cpu_cycles_filter",
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 1),
>> + { 0 },
>> + },
>> + },
>> + {
>> + .name = "cancle_filter",
> cancel
Thanks for catching it.
>> + .filter = {
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 0),
>> + DEFINE_FILTER(ARMV8_PMUV3_PERFCTR_CPU_CYCLES, 1),
>> + },
>> + },
>> + { 0 }
>> +};
>> +
>> +static void run_tests(void)
>> +{
>> + struct test_desc *t;
>> +
>> + for (t = &tests[0]; t->name; t++)
>> + run_test(t);
>> +}
>> +
>> +static bool kvm_pmu_support_events(void)
>> +{
>> + create_vpmu_vm(guest_get_pmceid);
>> +
>> + memset(&max_pmce, 0, sizeof(max_pmce));
>> + sync_global_to_guest(vpmu_vm.vm, max_pmce);
>> + run_vcpu(vpmu_vm.vcpu);
>> + sync_global_from_guest(vpmu_vm.vm, max_pmce);
>> + destroy_vpmu_vm();
>> +
>> + return max_pmce.pmceid0 &
>> + (ARMV8_PMUV3_PERFCTR_BR_RETIRED |
>> + ARMV8_PMUV3_PERFCTR_INST_RETIRED |
>> + ARMV8_PMUV3_PERFCTR_CHAIN);
> I understood Oliver suggested to use sysfs to know if te events were
> supported and to use KVM_ARM_VCPU_PMU_V3_SET_PMU. On my end I am fine
> with your method though.
>
I think get the supported pmu events from KVM is better than get the
host supported pmu events even they are the same currently.
Thanks,
Shaoqin
> Thanks
>
> Eric
>> +}
>> +
>> +int main(void)
>> +{
>> + TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_PMU_V3));
>> + TEST_REQUIRE(kvm_pmu_support_events());
>> +
>> + run_tests();
>> +}
>
--
Shaoqin