kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/6]  KVM: arm64: selftests: Fix get-reg-list
@ 2021-05-07 20:04 Andrew Jones
  2021-05-07 20:04 ` [PATCH 1/6] KVM: arm64: selftests: get-reg-list: Factor out printing Andrew Jones
                   ` (5 more replies)
  0 siblings, 6 replies; 8+ messages in thread
From: Andrew Jones @ 2021-05-07 20:04 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, ricarkol, eric.auger, alexandru.elisei, pbonzini

Since KVM commit 11663111cd49 ("KVM: arm64: Hide PMU registers from
userspace when not available") the get-reg-list* tests have been
failing with

  ...
  ... There are 74 missing registers.
  The following lines are missing registers:
  ...

where the 74 missing registers are all PMU registers. This isn't a
bug in KVM that the selftest found, even though it's true that a
KVM userspace that wasn't setting the KVM_ARM_VCPU_PMU_V3 VCPU
flag, but still expecting the PMU registers to be in the reg-list,
would suddenly no longer have their expectations met. In that case,
the expectations were wrong, though, so that KVM userspace needs to
be fixed, and so does this selftest.

We could fix the test with a one-liner since we just need a

  init->features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;

in prepare_vcpu_init(), but that's too easy, so here's a 6 patch patch
series instead :-)  The reason for all the patches and the heavy diffstat
is to prepare for other vcpu configuration testing, e.g. ptrauth and mte.
With the refactoring done in this series, we should now be able to easily
add register sublists and vcpu configs to the get-reg-list test, as the
last patch demonstrates with the pmu fix.

Thanks,
drew


Andrew Jones (6):
  KVM: arm64: selftests: get-reg-list: Factor out printing
  KVM: arm64: selftests: get-reg-list: Introduce vcpu configs
  KVM: arm64: selftests: get-reg-list: Prepare to run multiple configs
    at once
  KVM: arm64: selftests: get-reg-list: Provide config selection option
  KVM: arm64: selftests: get-reg-list: Remove get-reg-list-sve
  KVM: arm64: selftests: get-reg-list: Split base and pmu registers

 tools/testing/selftests/kvm/.gitignore        |   1 -
 tools/testing/selftests/kvm/Makefile          |   1 -
 .../selftests/kvm/aarch64/get-reg-list-sve.c  |   3 -
 .../selftests/kvm/aarch64/get-reg-list.c      | 442 +++++++++++++-----
 4 files changed, 313 insertions(+), 134 deletions(-)
 delete mode 100644 tools/testing/selftests/kvm/aarch64/get-reg-list-sve.c

-- 
2.30.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/6] KVM: arm64: selftests: get-reg-list: Factor out printing
  2021-05-07 20:04 [PATCH 0/6] KVM: arm64: selftests: Fix get-reg-list Andrew Jones
@ 2021-05-07 20:04 ` Andrew Jones
  2021-05-07 20:04 ` [PATCH 2/6] KVM: arm64: selftests: get-reg-list: Introduce vcpu configs Andrew Jones
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2021-05-07 20:04 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, ricarkol, eric.auger, alexandru.elisei, pbonzini

A later patch will need the printing to be factored out of the test
code. To factor out the printing we also factor out the reg-list
initialization.

No functional change intended.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../selftests/kvm/aarch64/get-reg-list.c      | 62 +++++++++++++------
 1 file changed, 42 insertions(+), 20 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
index 486932164cf2..44d560a8ca45 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -27,6 +27,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <assert.h>
 #include "kvm_util.h"
 #include "test_util.h"
 #include "processor.h"
@@ -336,12 +337,47 @@ static void check_supported(void)
 	}
 }
 
-int main(int ac, char **av)
+static bool fixup_core_regs;
+
+static void reg_list_init(struct kvm_vm *vm)
 {
 	struct kvm_vcpu_init init = { .target = -1, };
+
+	prepare_vcpu_init(&init);
+	aarch64_vcpu_add_default(vm, 0, &init, NULL);
+	finalize_vcpu(vm, 0);
+
+	reg_list = vcpu_get_reg_list(vm, 0);
+
+	if (fixup_core_regs)
+		core_reg_fixup();
+}
+
+static void print_reg_list(bool print_list, bool print_filtered)
+{
+	struct kvm_vm *vm;
+	int i;
+
+	assert(print_list || print_filtered);
+
+	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+	reg_list_init(vm);
+
+	putchar('\n');
+	for_each_reg(i) {
+		__u64 id = reg_list->reg[i];
+		if ((print_list && !filter_reg(id)) ||
+		    (print_filtered && filter_reg(id)))
+			print_reg(id);
+	}
+	putchar('\n');
+}
+
+int main(int ac, char **av)
+{
 	int new_regs = 0, missing_regs = 0, i;
 	int failed_get = 0, failed_set = 0, failed_reject = 0;
-	bool print_list = false, print_filtered = false, fixup_core_regs = false;
+	bool print_list = false, print_filtered = false;
 	struct kvm_vm *vm;
 	__u64 *vec_regs;
 
@@ -358,28 +394,14 @@ int main(int ac, char **av)
 			TEST_FAIL("Unknown option: %s\n", av[i]);
 	}
 
-	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
-	prepare_vcpu_init(&init);
-	aarch64_vcpu_add_default(vm, 0, &init, NULL);
-	finalize_vcpu(vm, 0);
-
-	reg_list = vcpu_get_reg_list(vm, 0);
-
-	if (fixup_core_regs)
-		core_reg_fixup();
-
 	if (print_list || print_filtered) {
-		putchar('\n');
-		for_each_reg(i) {
-			__u64 id = reg_list->reg[i];
-			if ((print_list && !filter_reg(id)) ||
-			    (print_filtered && filter_reg(id)))
-				print_reg(id);
-		}
-		putchar('\n');
+		print_reg_list(print_list, print_filtered);
 		return 0;
 	}
 
+	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
+	reg_list_init(vm);
+
 	/*
 	 * We only test that we can get the register and then write back the
 	 * same value. Some registers may allow other values to be written
-- 
2.30.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/6] KVM: arm64: selftests: get-reg-list: Introduce vcpu configs
  2021-05-07 20:04 [PATCH 0/6] KVM: arm64: selftests: Fix get-reg-list Andrew Jones
  2021-05-07 20:04 ` [PATCH 1/6] KVM: arm64: selftests: get-reg-list: Factor out printing Andrew Jones
@ 2021-05-07 20:04 ` Andrew Jones
  2021-05-07 20:04 ` [PATCH 3/6] KVM: arm64: selftests: get-reg-list: Prepare to run multiple configs at once Andrew Jones
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2021-05-07 20:04 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, ricarkol, eric.auger, alexandru.elisei, pbonzini

We already break register lists into sublists that get selected based
on vcpu config. However, since we only had two configs (vregs and sve),
we didn't structure the code very well to manage them. Restructure
it now to more cleanly handle register sublists that are dependent on
the vcpu config.

This patch has no intended functional change (except for the vcpu
config name now being prepended to all output), it only prepares for
later patches.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../selftests/kvm/aarch64/get-reg-list.c      | 217 ++++++++++--------
 1 file changed, 123 insertions(+), 94 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
index 44d560a8ca45..fbbeee634722 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -38,7 +38,27 @@
 #define reg_list_sve() (false)
 #endif
 
-#define REG_MASK (KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_ARM_COPROC_MASK)
+static struct kvm_reg_list *reg_list;
+static __u64 *blessed_reg, blessed_n;
+
+struct reg_sublist {
+	__u64 *regs;
+	__u64 regs_n;
+	__u64 *rejects_set;
+	__u64 rejects_set_n;
+};
+
+struct vcpu_config {
+	const char *name;
+	bool sve;
+	struct reg_sublist sublists[];
+};
+
+static struct vcpu_config vregs_config;
+static struct vcpu_config sve_config;
+
+#define for_each_sublist(c, s)							\
+	for ((s) = &(c)->sublists[0]; (s)->regs; ++(s))
 
 #define for_each_reg(i)								\
 	for ((i) = 0; (i) < reg_list->n; ++(i))
@@ -55,13 +75,6 @@
 	for_each_reg_filtered(i)						\
 		if (!find_reg(blessed_reg, blessed_n, reg_list->reg[i]))
 
-
-static struct kvm_reg_list *reg_list;
-
-static __u64 base_regs[], vregs[], sve_regs[], rejects_set[];
-static __u64 base_regs_n, vregs_n, sve_regs_n, rejects_set_n;
-static __u64 *blessed_reg, blessed_n;
-
 static bool filter_reg(__u64 reg)
 {
 	/*
@@ -97,11 +110,13 @@ static const char *str_with_index(const char *template, __u64 index)
 	return (const char *)str;
 }
 
+#define REG_MASK (KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_ARM_COPROC_MASK)
+
 #define CORE_REGS_XX_NR_WORDS	2
 #define CORE_SPSR_XX_NR_WORDS	2
 #define CORE_FPREGS_XX_NR_WORDS	4
 
-static const char *core_id_to_str(__u64 id)
+static const char *core_id_to_str(struct vcpu_config *c, __u64 id)
 {
 	__u64 core_off = id & ~REG_MASK, idx;
 
@@ -112,7 +127,7 @@ static const char *core_id_to_str(__u64 id)
 	case KVM_REG_ARM_CORE_REG(regs.regs[0]) ...
 	     KVM_REG_ARM_CORE_REG(regs.regs[30]):
 		idx = (core_off - KVM_REG_ARM_CORE_REG(regs.regs[0])) / CORE_REGS_XX_NR_WORDS;
-		TEST_ASSERT(idx < 31, "Unexpected regs.regs index: %lld", idx);
+		TEST_ASSERT(idx < 31, "%s: Unexpected regs.regs index: %lld", c->name, idx);
 		return str_with_index("KVM_REG_ARM_CORE_REG(regs.regs[##])", idx);
 	case KVM_REG_ARM_CORE_REG(regs.sp):
 		return "KVM_REG_ARM_CORE_REG(regs.sp)";
@@ -127,12 +142,12 @@ static const char *core_id_to_str(__u64 id)
 	case KVM_REG_ARM_CORE_REG(spsr[0]) ...
 	     KVM_REG_ARM_CORE_REG(spsr[KVM_NR_SPSR - 1]):
 		idx = (core_off - KVM_REG_ARM_CORE_REG(spsr[0])) / CORE_SPSR_XX_NR_WORDS;
-		TEST_ASSERT(idx < KVM_NR_SPSR, "Unexpected spsr index: %lld", idx);
+		TEST_ASSERT(idx < KVM_NR_SPSR, "%s: Unexpected spsr index: %lld", c->name, idx);
 		return str_with_index("KVM_REG_ARM_CORE_REG(spsr[##])", idx);
 	case KVM_REG_ARM_CORE_REG(fp_regs.vregs[0]) ...
 	     KVM_REG_ARM_CORE_REG(fp_regs.vregs[31]):
 		idx = (core_off - KVM_REG_ARM_CORE_REG(fp_regs.vregs[0])) / CORE_FPREGS_XX_NR_WORDS;
-		TEST_ASSERT(idx < 32, "Unexpected fp_regs.vregs index: %lld", idx);
+		TEST_ASSERT(idx < 32, "%s: Unexpected fp_regs.vregs index: %lld", c->name, idx);
 		return str_with_index("KVM_REG_ARM_CORE_REG(fp_regs.vregs[##])", idx);
 	case KVM_REG_ARM_CORE_REG(fp_regs.fpsr):
 		return "KVM_REG_ARM_CORE_REG(fp_regs.fpsr)";
@@ -140,11 +155,11 @@ static const char *core_id_to_str(__u64 id)
 		return "KVM_REG_ARM_CORE_REG(fp_regs.fpcr)";
 	}
 
-	TEST_FAIL("Unknown core reg id: 0x%llx", id);
+	TEST_FAIL("%s: Unknown core reg id: 0x%llx", c->name, id);
 	return NULL;
 }
 
-static const char *sve_id_to_str(__u64 id)
+static const char *sve_id_to_str(struct vcpu_config *c, __u64 id)
 {
 	__u64 sve_off, n, i;
 
@@ -154,37 +169,37 @@ static const char *sve_id_to_str(__u64 id)
 	sve_off = id & ~(REG_MASK | ((1ULL << 5) - 1));
 	i = id & (KVM_ARM64_SVE_MAX_SLICES - 1);
 
-	TEST_ASSERT(i == 0, "Currently we don't expect slice > 0, reg id 0x%llx", id);
+	TEST_ASSERT(i == 0, "%s: Currently we don't expect slice > 0, reg id 0x%llx", c->name, id);
 
 	switch (sve_off) {
 	case KVM_REG_ARM64_SVE_ZREG_BASE ...
 	     KVM_REG_ARM64_SVE_ZREG_BASE + (1ULL << 5) * KVM_ARM64_SVE_NUM_ZREGS - 1:
 		n = (id >> 5) & (KVM_ARM64_SVE_NUM_ZREGS - 1);
 		TEST_ASSERT(id == KVM_REG_ARM64_SVE_ZREG(n, 0),
-			    "Unexpected bits set in SVE ZREG id: 0x%llx", id);
+			    "%s: Unexpected bits set in SVE ZREG id: 0x%llx", c->name, id);
 		return str_with_index("KVM_REG_ARM64_SVE_ZREG(##, 0)", n);
 	case KVM_REG_ARM64_SVE_PREG_BASE ...
 	     KVM_REG_ARM64_SVE_PREG_BASE + (1ULL << 5) * KVM_ARM64_SVE_NUM_PREGS - 1:
 		n = (id >> 5) & (KVM_ARM64_SVE_NUM_PREGS - 1);
 		TEST_ASSERT(id == KVM_REG_ARM64_SVE_PREG(n, 0),
-			    "Unexpected bits set in SVE PREG id: 0x%llx", id);
+			    "%s: Unexpected bits set in SVE PREG id: 0x%llx", c->name, id);
 		return str_with_index("KVM_REG_ARM64_SVE_PREG(##, 0)", n);
 	case KVM_REG_ARM64_SVE_FFR_BASE:
 		TEST_ASSERT(id == KVM_REG_ARM64_SVE_FFR(0),
-			    "Unexpected bits set in SVE FFR id: 0x%llx", id);
+			    "%s: Unexpected bits set in SVE FFR id: 0x%llx", c->name, id);
 		return "KVM_REG_ARM64_SVE_FFR(0)";
 	}
 
 	return NULL;
 }
 
-static void print_reg(__u64 id)
+static void print_reg(struct vcpu_config *c, __u64 id)
 {
 	unsigned op0, op1, crn, crm, op2;
 	const char *reg_size = NULL;
 
 	TEST_ASSERT((id & KVM_REG_ARCH_MASK) == KVM_REG_ARM64,
-		    "KVM_REG_ARM64 missing in reg id: 0x%llx", id);
+		    "%s: KVM_REG_ARM64 missing in reg id: 0x%llx", c->name, id);
 
 	switch (id & KVM_REG_SIZE_MASK) {
 	case KVM_REG_SIZE_U8:
@@ -215,17 +230,17 @@ static void print_reg(__u64 id)
 		reg_size = "KVM_REG_SIZE_U2048";
 		break;
 	default:
-		TEST_FAIL("Unexpected reg size: 0x%llx in reg id: 0x%llx",
-			  (id & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT, id);
+		TEST_FAIL("%s: Unexpected reg size: 0x%llx in reg id: 0x%llx",
+			  c->name, (id & KVM_REG_SIZE_MASK) >> KVM_REG_SIZE_SHIFT, id);
 	}
 
 	switch (id & KVM_REG_ARM_COPROC_MASK) {
 	case KVM_REG_ARM_CORE:
-		printf("\tKVM_REG_ARM64 | %s | KVM_REG_ARM_CORE | %s,\n", reg_size, core_id_to_str(id));
+		printf("\tKVM_REG_ARM64 | %s | KVM_REG_ARM_CORE | %s,\n", reg_size, core_id_to_str(c, id));
 		break;
 	case KVM_REG_ARM_DEMUX:
 		TEST_ASSERT(!(id & ~(REG_MASK | KVM_REG_ARM_DEMUX_ID_MASK | KVM_REG_ARM_DEMUX_VAL_MASK)),
-			    "Unexpected bits set in DEMUX reg id: 0x%llx", id);
+			    "%s: Unexpected bits set in DEMUX reg id: 0x%llx", c->name, id);
 		printf("\tKVM_REG_ARM64 | %s | KVM_REG_ARM_DEMUX | KVM_REG_ARM_DEMUX_ID_CCSIDR | %lld,\n",
 		       reg_size, id & KVM_REG_ARM_DEMUX_VAL_MASK);
 		break;
@@ -236,23 +251,23 @@ static void print_reg(__u64 id)
 		crm = (id & KVM_REG_ARM64_SYSREG_CRM_MASK) >> KVM_REG_ARM64_SYSREG_CRM_SHIFT;
 		op2 = (id & KVM_REG_ARM64_SYSREG_OP2_MASK) >> KVM_REG_ARM64_SYSREG_OP2_SHIFT;
 		TEST_ASSERT(id == ARM64_SYS_REG(op0, op1, crn, crm, op2),
-			    "Unexpected bits set in SYSREG reg id: 0x%llx", id);
+			    "%s: Unexpected bits set in SYSREG reg id: 0x%llx", c->name, id);
 		printf("\tARM64_SYS_REG(%d, %d, %d, %d, %d),\n", op0, op1, crn, crm, op2);
 		break;
 	case KVM_REG_ARM_FW:
 		TEST_ASSERT(id == KVM_REG_ARM_FW_REG(id & 0xffff),
-			    "Unexpected bits set in FW reg id: 0x%llx", id);
+			    "%s: Unexpected bits set in FW reg id: 0x%llx", c->name, id);
 		printf("\tKVM_REG_ARM_FW_REG(%lld),\n", id & 0xffff);
 		break;
 	case KVM_REG_ARM64_SVE:
-		if (reg_list_sve())
-			printf("\t%s,\n", sve_id_to_str(id));
+		if (c->sve)
+			printf("\t%s,\n", sve_id_to_str(c, id));
 		else
-			TEST_FAIL("KVM_REG_ARM64_SVE is an unexpected coproc type in reg id: 0x%llx", id);
+			TEST_FAIL("%s: KVM_REG_ARM64_SVE is an unexpected coproc type in reg id: 0x%llx", c->name, id);
 		break;
 	default:
-		TEST_FAIL("Unexpected coproc type: 0x%llx in reg id: 0x%llx",
-			  (id & KVM_REG_ARM_COPROC_MASK) >> KVM_REG_ARM_COPROC_SHIFT, id);
+		TEST_FAIL("%s: Unexpected coproc type: 0x%llx in reg id: 0x%llx",
+			  c->name, (id & KVM_REG_ARM_COPROC_MASK) >> KVM_REG_ARM_COPROC_SHIFT, id);
 	}
 }
 
@@ -313,39 +328,39 @@ static void core_reg_fixup(void)
 	reg_list = tmp;
 }
 
-static void prepare_vcpu_init(struct kvm_vcpu_init *init)
+static void prepare_vcpu_init(struct vcpu_config *c, struct kvm_vcpu_init *init)
 {
-	if (reg_list_sve())
+	if (c->sve)
 		init->features[0] |= 1 << KVM_ARM_VCPU_SVE;
 }
 
-static void finalize_vcpu(struct kvm_vm *vm, uint32_t vcpuid)
+static void finalize_vcpu(struct kvm_vm *vm, uint32_t vcpuid, struct vcpu_config *c)
 {
 	int feature;
 
-	if (reg_list_sve()) {
+	if (c->sve) {
 		feature = KVM_ARM_VCPU_SVE;
 		vcpu_ioctl(vm, vcpuid, KVM_ARM_VCPU_FINALIZE, &feature);
 	}
 }
 
-static void check_supported(void)
+static void check_supported(struct vcpu_config *c)
 {
-	if (reg_list_sve() && !kvm_check_cap(KVM_CAP_ARM_SVE)) {
-		fprintf(stderr, "SVE not available, skipping tests\n");
+	if (c->sve && !kvm_check_cap(KVM_CAP_ARM_SVE)) {
+		fprintf(stderr, "%s: SVE not available, skipping tests\n", c->name);
 		exit(KSFT_SKIP);
 	}
 }
 
 static bool fixup_core_regs;
 
-static void reg_list_init(struct kvm_vm *vm)
+static void reg_list_init(struct kvm_vm *vm, struct vcpu_config *c)
 {
 	struct kvm_vcpu_init init = { .target = -1, };
 
-	prepare_vcpu_init(&init);
+	prepare_vcpu_init(c, &init);
 	aarch64_vcpu_add_default(vm, 0, &init, NULL);
-	finalize_vcpu(vm, 0);
+	finalize_vcpu(vm, 0, c);
 
 	reg_list = vcpu_get_reg_list(vm, 0);
 
@@ -353,7 +368,7 @@ static void reg_list_init(struct kvm_vm *vm)
 		core_reg_fixup();
 }
 
-static void print_reg_list(bool print_list, bool print_filtered)
+static void print_reg_list(struct vcpu_config *c, bool print_list, bool print_filtered)
 {
 	struct kvm_vm *vm;
 	int i;
@@ -361,27 +376,28 @@ static void print_reg_list(bool print_list, bool print_filtered)
 	assert(print_list || print_filtered);
 
 	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
-	reg_list_init(vm);
+	reg_list_init(vm, c);
 
 	putchar('\n');
 	for_each_reg(i) {
 		__u64 id = reg_list->reg[i];
 		if ((print_list && !filter_reg(id)) ||
 		    (print_filtered && filter_reg(id)))
-			print_reg(id);
+			print_reg(c, id);
 	}
 	putchar('\n');
 }
 
 int main(int ac, char **av)
 {
-	int new_regs = 0, missing_regs = 0, i;
+	struct vcpu_config *c = reg_list_sve() ? &sve_config : &vregs_config;
+	int new_regs = 0, missing_regs = 0, i, n;
 	int failed_get = 0, failed_set = 0, failed_reject = 0;
 	bool print_list = false, print_filtered = false;
 	struct kvm_vm *vm;
-	__u64 *vec_regs;
+	struct reg_sublist *s;
 
-	check_supported();
+	check_supported(c);
 
 	for (i = 1; i < ac; ++i) {
 		if (strcmp(av[i], "--core-reg-fixup") == 0)
@@ -395,12 +411,12 @@ int main(int ac, char **av)
 	}
 
 	if (print_list || print_filtered) {
-		print_reg_list(print_list, print_filtered);
+		print_reg_list(c, print_list, print_filtered);
 		return 0;
 	}
 
 	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
-	reg_list_init(vm);
+	reg_list_init(vm, c);
 
 	/*
 	 * We only test that we can get the register and then write back the
@@ -418,50 +434,52 @@ int main(int ac, char **av)
 			.id = reg_list->reg[i],
 			.addr = (__u64)&addr,
 		};
+		bool reject_reg = false;
 		int ret;
 
 		ret = _vcpu_ioctl(vm, 0, KVM_GET_ONE_REG, &reg);
 		if (ret) {
-			puts("Failed to get ");
-			print_reg(reg.id);
+			printf("%s: Failed to get ", c->name);
+			print_reg(c, reg.id);
 			putchar('\n');
 			++failed_get;
 		}
 
 		/* rejects_set registers are rejected after KVM_ARM_VCPU_FINALIZE */
-		if (find_reg(rejects_set, rejects_set_n, reg.id)) {
-			ret = _vcpu_ioctl(vm, 0, KVM_SET_ONE_REG, &reg);
-			if (ret != -1 || errno != EPERM) {
-				printf("Failed to reject (ret=%d, errno=%d) ", ret, errno);
-				print_reg(reg.id);
-				putchar('\n');
-				++failed_reject;
+		for_each_sublist(c, s) {
+			if (s->rejects_set && find_reg(s->rejects_set, s->rejects_set_n, reg.id)) {
+				reject_reg = true;
+				ret = _vcpu_ioctl(vm, 0, KVM_SET_ONE_REG, &reg);
+				if (ret != -1 || errno != EPERM) {
+					printf("%s: Failed to reject (ret=%d, errno=%d) ", c->name, ret, errno);
+					print_reg(c, reg.id);
+					putchar('\n');
+					++failed_reject;
+				}
+				break;
 			}
-			continue;
 		}
 
-		ret = _vcpu_ioctl(vm, 0, KVM_SET_ONE_REG, &reg);
-		if (ret) {
-			puts("Failed to set ");
-			print_reg(reg.id);
-			putchar('\n');
-			++failed_set;
+		if (!reject_reg) {
+			ret = _vcpu_ioctl(vm, 0, KVM_SET_ONE_REG, &reg);
+			if (ret) {
+				printf("%s: Failed to set ", c->name);
+				print_reg(c, reg.id);
+				putchar('\n');
+				++failed_set;
+			}
 		}
 	}
 
-	if (reg_list_sve()) {
-		blessed_n = base_regs_n + sve_regs_n;
-		vec_regs = sve_regs;
-	} else {
-		blessed_n = base_regs_n + vregs_n;
-		vec_regs = vregs;
-	}
-
+	for_each_sublist(c, s)
+		blessed_n += s->regs_n;
 	blessed_reg = calloc(blessed_n, sizeof(__u64));
-	for (i = 0; i < base_regs_n; ++i)
-		blessed_reg[i] = base_regs[i];
-	for (i = 0; i < blessed_n - base_regs_n; ++i)
-		blessed_reg[base_regs_n + i] = vec_regs[i];
+
+	n = 0;
+	for_each_sublist(c, s) {
+		for (i = 0; i < s->regs_n; ++i)
+			blessed_reg[n++] = s->regs[i];
+	}
 
 	for_each_new_reg(i)
 		++new_regs;
@@ -470,31 +488,31 @@ int main(int ac, char **av)
 		++missing_regs;
 
 	if (new_regs || missing_regs) {
-		printf("Number blessed registers: %5lld\n", blessed_n);
-		printf("Number registers:         %5lld\n", reg_list->n);
+		printf("%s: Number blessed registers: %5lld\n", c->name, blessed_n);
+		printf("%s: Number registers:         %5lld\n", c->name, reg_list->n);
 	}
 
 	if (new_regs) {
-		printf("\nThere are %d new registers.\n"
+		printf("\n%s: There are %d new registers.\n"
 		       "Consider adding them to the blessed reg "
-		       "list with the following lines:\n\n", new_regs);
+		       "list with the following lines:\n\n", c->name, new_regs);
 		for_each_new_reg(i)
-			print_reg(reg_list->reg[i]);
+			print_reg(c, reg_list->reg[i]);
 		putchar('\n');
 	}
 
 	if (missing_regs) {
-		printf("\nThere are %d missing registers.\n"
-		       "The following lines are missing registers:\n\n", missing_regs);
+		printf("\n%s: There are %d missing registers.\n"
+		       "The following lines are missing registers:\n\n", c->name, missing_regs);
 		for_each_missing_reg(i)
-			print_reg(blessed_reg[i]);
+			print_reg(c, blessed_reg[i]);
 		putchar('\n');
 	}
 
 	TEST_ASSERT(!missing_regs && !failed_get && !failed_set && !failed_reject,
-		    "There are %d missing registers; "
+		    "%s: There are %d missing registers; "
 		    "%d registers failed get; %d registers failed set; %d registers failed reject",
-		    missing_regs, failed_get, failed_set, failed_reject);
+		    c->name, missing_regs, failed_get, failed_set, failed_reject);
 
 	return 0;
 }
@@ -783,7 +801,6 @@ static __u64 base_regs[] = {
 	ARM64_SYS_REG(3, 4, 5, 0, 1),	/* IFSR32_EL2 */
 	ARM64_SYS_REG(3, 4, 5, 3, 0),	/* FPEXC32_EL2 */
 };
-static __u64 base_regs_n = ARRAY_SIZE(base_regs);
 
 static __u64 vregs[] = {
 	KVM_REG_ARM64 | KVM_REG_SIZE_U128 | KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(fp_regs.vregs[0]),
@@ -819,7 +836,6 @@ static __u64 vregs[] = {
 	KVM_REG_ARM64 | KVM_REG_SIZE_U128 | KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(fp_regs.vregs[30]),
 	KVM_REG_ARM64 | KVM_REG_SIZE_U128 | KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(fp_regs.vregs[31]),
 };
-static __u64 vregs_n = ARRAY_SIZE(vregs);
 
 static __u64 sve_regs[] = {
 	KVM_REG_ARM64_SVE_VLS,
@@ -874,11 +890,24 @@ static __u64 sve_regs[] = {
 	KVM_REG_ARM64_SVE_FFR(0),
 	ARM64_SYS_REG(3, 0, 1, 2, 0),   /* ZCR_EL1 */
 };
-static __u64 sve_regs_n = ARRAY_SIZE(sve_regs);
 
-static __u64 rejects_set[] = {
-#ifdef REG_LIST_SVE
+static __u64 sve_rejects_set[] = {
 	KVM_REG_ARM64_SVE_VLS,
-#endif
 };
-static __u64 rejects_set_n = ARRAY_SIZE(rejects_set);
+
+static struct vcpu_config vregs_config = {
+	"vregs",
+	.sublists = {
+	{ base_regs,	ARRAY_SIZE(base_regs), },
+	{ vregs,	ARRAY_SIZE(vregs), },
+	{0},
+	},
+};
+static struct vcpu_config sve_config = {
+	"sve", .sve = true,
+	.sublists = {
+	{ base_regs,	ARRAY_SIZE(base_regs), },
+	{ sve_regs,	ARRAY_SIZE(sve_regs),	sve_rejects_set,	ARRAY_SIZE(sve_rejects_set), },
+	{0},
+	},
+};
-- 
2.30.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/6] KVM: arm64: selftests: get-reg-list: Prepare to run multiple configs at once
  2021-05-07 20:04 [PATCH 0/6] KVM: arm64: selftests: Fix get-reg-list Andrew Jones
  2021-05-07 20:04 ` [PATCH 1/6] KVM: arm64: selftests: get-reg-list: Factor out printing Andrew Jones
  2021-05-07 20:04 ` [PATCH 2/6] KVM: arm64: selftests: get-reg-list: Introduce vcpu configs Andrew Jones
@ 2021-05-07 20:04 ` Andrew Jones
  2021-05-07 20:04 ` [PATCH 4/6] KVM: arm64: selftests: get-reg-list: Provide config selection option Andrew Jones
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2021-05-07 20:04 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, ricarkol, eric.auger, alexandru.elisei, pbonzini

We don't want to have to create a new binary for each vcpu config, so
prepare to run the test for multiple vcpu configs in a single binary.
We do this by factoring out the test from main() and then looping over
configs. When given '--list' we still never print more than a single
reg-list for a single vcpu config though, because it would be confusing
otherwise.

No functional change intended.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../selftests/kvm/aarch64/get-reg-list.c      | 72 +++++++++++++------
 1 file changed, 49 insertions(+), 23 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
index fbbeee634722..68d3be86d490 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -54,8 +54,7 @@ struct vcpu_config {
 	struct reg_sublist sublists[];
 };
 
-static struct vcpu_config vregs_config;
-static struct vcpu_config sve_config;
+static struct vcpu_config *vcpu_configs[];
 
 #define for_each_sublist(c, s)							\
 	for ((s) = &(c)->sublists[0]; (s)->regs; ++(s))
@@ -386,35 +385,18 @@ static void print_reg_list(struct vcpu_config *c, bool print_list, bool print_fi
 			print_reg(c, id);
 	}
 	putchar('\n');
+
+	free(reg_list);
+	kvm_vm_free(vm);
 }
 
-int main(int ac, char **av)
+static void run_test(struct vcpu_config *c)
 {
-	struct vcpu_config *c = reg_list_sve() ? &sve_config : &vregs_config;
 	int new_regs = 0, missing_regs = 0, i, n;
 	int failed_get = 0, failed_set = 0, failed_reject = 0;
-	bool print_list = false, print_filtered = false;
 	struct kvm_vm *vm;
 	struct reg_sublist *s;
 
-	check_supported(c);
-
-	for (i = 1; i < ac; ++i) {
-		if (strcmp(av[i], "--core-reg-fixup") == 0)
-			fixup_core_regs = true;
-		else if (strcmp(av[i], "--list") == 0)
-			print_list = true;
-		else if (strcmp(av[i], "--list-filtered") == 0)
-			print_filtered = true;
-		else
-			TEST_FAIL("Unknown option: %s\n", av[i]);
-	}
-
-	if (print_list || print_filtered) {
-		print_reg_list(c, print_list, print_filtered);
-		return 0;
-	}
-
 	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
 	reg_list_init(vm, c);
 
@@ -514,6 +496,45 @@ int main(int ac, char **av)
 		    "%d registers failed get; %d registers failed set; %d registers failed reject",
 		    c->name, missing_regs, failed_get, failed_set, failed_reject);
 
+	blessed_n = 0;
+	free(blessed_reg);
+	free(reg_list);
+	kvm_vm_free(vm);
+}
+
+int main(int ac, char **av)
+{
+	struct vcpu_config *c;
+	bool print_list = false, print_filtered = false;
+	int i;
+
+	for (i = 1; i < ac; ++i) {
+		if (strcmp(av[i], "--core-reg-fixup") == 0)
+			fixup_core_regs = true;
+		else if (strcmp(av[i], "--list") == 0)
+			print_list = true;
+		else if (strcmp(av[i], "--list-filtered") == 0)
+			print_filtered = true;
+		else
+			TEST_FAIL("Unknown option: %s\n", av[i]);
+	}
+
+	if (print_list || print_filtered) {
+		/*
+		 * We only want to print the register list of a single config.
+		 * TODO: Add command line support to pick which config.
+		 */
+		c = vcpu_configs[0];
+		check_supported(c);
+		print_reg_list(c, print_list, print_filtered);
+		return 0;
+	}
+
+	for (i = 0, c = vcpu_configs[0]; c; ++i, c = vcpu_configs[i]) {
+		check_supported(c);
+		run_test(c);
+	}
+
 	return 0;
 }
 
@@ -911,3 +932,8 @@ static struct vcpu_config sve_config = {
 	{0},
 	},
 };
+
+static struct vcpu_config *vcpu_configs[] = {
+	reg_list_sve() ? &sve_config : &vregs_config,
+	NULL
+};
-- 
2.30.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 4/6] KVM: arm64: selftests: get-reg-list: Provide config selection option
  2021-05-07 20:04 [PATCH 0/6] KVM: arm64: selftests: Fix get-reg-list Andrew Jones
                   ` (2 preceding siblings ...)
  2021-05-07 20:04 ` [PATCH 3/6] KVM: arm64: selftests: get-reg-list: Prepare to run multiple configs at once Andrew Jones
@ 2021-05-07 20:04 ` Andrew Jones
  2021-05-10  6:40   ` Andrew Jones
  2021-05-07 20:04 ` [PATCH 5/6] KVM: arm64: selftests: get-reg-list: Remove get-reg-list-sve Andrew Jones
  2021-05-07 20:04 ` [PATCH 6/6] KVM: arm64: selftests: get-reg-list: Split base and pmu registers Andrew Jones
  5 siblings, 1 reply; 8+ messages in thread
From: Andrew Jones @ 2021-05-07 20:04 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, ricarkol, eric.auger, alexandru.elisei, pbonzini

Add a new command line option that allows the user to select a specific
configuration, e.g. --config:sve will give the sve config. Also provide
help text and the --help/-h options.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../selftests/kvm/aarch64/get-reg-list.c      | 76 +++++++++++++++++--
 1 file changed, 70 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
index 68d3be86d490..f5e122b6b257 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -38,6 +38,17 @@
 #define reg_list_sve() (false)
 #endif
 
+enum {
+	VREGS,
+	SVE,
+};
+
+static char * const vcpu_config_names[] = {
+	[VREGS] = "vregs",
+	[SVE] = "sve",
+	NULL
+};
+
 static struct kvm_reg_list *reg_list;
 static __u64 *blessed_reg, blessed_n;
 
@@ -502,34 +513,87 @@ static void run_test(struct vcpu_config *c)
 	kvm_vm_free(vm);
 }
 
+static void help(void)
+{
+	char * const *n;
+
+	printf(
+	"\n"
+	"usage: get-reg-list [--config:<selection>[,<selection>...]] [--list] [--list-filtered] [--core-reg-fixup]\n\n"
+	" --config:<selection>[,<selection>...] Used to select a specific vcpu configuration for the test/listing\n"
+	"                                       '<selection>' may be\n");
+
+	for (n = &vcpu_config_names[0]; *n; ++n)
+	printf("                                         '%s'\n", *n);
+
+	printf(
+	"\n"
+	" --list                                Print the register list rather than test it (requires --config)\n"
+	" --list-filtered                       Print registers that would normally be filtered out (requires --config)\n"
+	" --core-reg-fixup                      Needed when running on old kernels with broken core reg listings\n"
+	"\n"
+	);
+}
+
+static struct vcpu_config *parse_config(const char *config)
+{
+	struct vcpu_config *c = NULL;
+	int i;
+
+	if (config[8] != ':')
+		help(), exit(1);
+
+	for (i = 0; i < ARRAY_SIZE(vcpu_config_names) - 1; ++i) {
+		if (strcmp(vcpu_config_names[i], &config[9]) == 0) {
+			c = vcpu_configs[i];
+			break;
+		}
+	}
+
+	if (!c)
+		help(), exit(1);
+
+	return c;
+}
+
 int main(int ac, char **av)
 {
-	struct vcpu_config *c;
+	struct vcpu_config *c = NULL;
 	bool print_list = false, print_filtered = false;
 	int i;
 
 	for (i = 1; i < ac; ++i) {
 		if (strcmp(av[i], "--core-reg-fixup") == 0)
 			fixup_core_regs = true;
+		else if (strncmp(av[i], "--config", 8) == 0)
+			c = parse_config(av[i]);
 		else if (strcmp(av[i], "--list") == 0)
 			print_list = true;
 		else if (strcmp(av[i], "--list-filtered") == 0)
 			print_filtered = true;
+		else if (strcmp(av[i], "--help") == 0 || strcmp(av[1], "-h") == 0)
+			help(), exit(0);
 		else
-			TEST_FAIL("Unknown option: %s\n", av[i]);
+			help(), exit(1);
 	}
 
 	if (print_list || print_filtered) {
 		/*
 		 * We only want to print the register list of a single config.
-		 * TODO: Add command line support to pick which config.
 		 */
-		c = vcpu_configs[0];
+		if (!c)
+			help(), exit(1);
 		check_supported(c);
 		print_reg_list(c, print_list, print_filtered);
 		return 0;
 	}
 
+	if (c) {
+		check_supported(c);
+		run_test(c);
+		return 0;
+	}
+
 	for (i = 0, c = vcpu_configs[0]; c; ++i, c = vcpu_configs[i]) {
 		check_supported(c);
 		run_test(c);
@@ -917,7 +981,7 @@ static __u64 sve_rejects_set[] = {
 };
 
 static struct vcpu_config vregs_config = {
-	"vregs",
+	vcpu_config_names[VREGS],
 	.sublists = {
 	{ base_regs,	ARRAY_SIZE(base_regs), },
 	{ vregs,	ARRAY_SIZE(vregs), },
@@ -925,7 +989,7 @@ static struct vcpu_config vregs_config = {
 	},
 };
 static struct vcpu_config sve_config = {
-	"sve", .sve = true,
+	vcpu_config_names[SVE], .sve = true,
 	.sublists = {
 	{ base_regs,	ARRAY_SIZE(base_regs), },
 	{ sve_regs,	ARRAY_SIZE(sve_regs),	sve_rejects_set,	ARRAY_SIZE(sve_rejects_set), },
-- 
2.30.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 5/6] KVM: arm64: selftests: get-reg-list: Remove get-reg-list-sve
  2021-05-07 20:04 [PATCH 0/6] KVM: arm64: selftests: Fix get-reg-list Andrew Jones
                   ` (3 preceding siblings ...)
  2021-05-07 20:04 ` [PATCH 4/6] KVM: arm64: selftests: get-reg-list: Provide config selection option Andrew Jones
@ 2021-05-07 20:04 ` Andrew Jones
  2021-05-07 20:04 ` [PATCH 6/6] KVM: arm64: selftests: get-reg-list: Split base and pmu registers Andrew Jones
  5 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2021-05-07 20:04 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, ricarkol, eric.auger, alexandru.elisei, pbonzini

Now that we can easily run the test for multiple vcpu configs, let's
merge get-reg-list and get-reg-list-sve into just get-reg-list. We
also add a final change to make it more possible to run multiple
tests, which is to fork the test, rather than directly run it. That
allows a test to fail, but subsequent tests can still run.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 tools/testing/selftests/kvm/.gitignore        |  1 -
 tools/testing/selftests/kvm/Makefile          |  1 -
 .../selftests/kvm/aarch64/get-reg-list-sve.c  |  3 --
 .../selftests/kvm/aarch64/get-reg-list.c      | 31 ++++++++++++-------
 4 files changed, 20 insertions(+), 16 deletions(-)
 delete mode 100644 tools/testing/selftests/kvm/aarch64/get-reg-list-sve.c

diff --git a/tools/testing/selftests/kvm/.gitignore b/tools/testing/selftests/kvm/.gitignore
index bd83158e0e0b..e3745c90d8b2 100644
--- a/tools/testing/selftests/kvm/.gitignore
+++ b/tools/testing/selftests/kvm/.gitignore
@@ -1,6 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
 /aarch64/get-reg-list
-/aarch64/get-reg-list-sve
 /aarch64/vgic_init
 /s390x/memop
 /s390x/resets
diff --git a/tools/testing/selftests/kvm/Makefile b/tools/testing/selftests/kvm/Makefile
index e439d027939d..bf54fc23bcff 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -78,7 +78,6 @@ TEST_GEN_PROGS_x86_64 += set_memory_region_test
 TEST_GEN_PROGS_x86_64 += steal_time
 
 TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
-TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list-sve
 TEST_GEN_PROGS_aarch64 += aarch64/vgic_init
 TEST_GEN_PROGS_aarch64 += demand_paging_test
 TEST_GEN_PROGS_aarch64 += dirty_log_test
diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list-sve.c b/tools/testing/selftests/kvm/aarch64/get-reg-list-sve.c
deleted file mode 100644
index efba76682b4b..000000000000
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list-sve.c
+++ /dev/null
@@ -1,3 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-#define REG_LIST_SVE
-#include "get-reg-list.c"
diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
index f5e122b6b257..2f7caa0b9e57 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -27,17 +27,14 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 #include <assert.h>
+#include <sys/types.h>
+#include <sys/wait.h>
 #include "kvm_util.h"
 #include "test_util.h"
 #include "processor.h"
 
-#ifdef REG_LIST_SVE
-#define reg_list_sve() (true)
-#else
-#define reg_list_sve() (false)
-#endif
-
 enum {
 	VREGS,
 	SVE,
@@ -560,7 +557,7 @@ int main(int ac, char **av)
 {
 	struct vcpu_config *c = NULL;
 	bool print_list = false, print_filtered = false;
-	int i;
+	int i, ret = 0;
 
 	for (i = 1; i < ac; ++i) {
 		if (strcmp(av[i], "--core-reg-fixup") == 0)
@@ -595,11 +592,22 @@ int main(int ac, char **av)
 	}
 
 	for (i = 0, c = vcpu_configs[0]; c; ++i, c = vcpu_configs[i]) {
-		check_supported(c);
-		run_test(c);
+		pid_t pid = fork();
+
+		if (!pid) {
+			check_supported(c);
+			run_test(c);
+			exit(0);
+		} else {
+			int wstatus;
+			pid_t wpid = wait(&wstatus);
+			TEST_ASSERT(wpid == pid && WIFEXITED(wstatus), "wait: Unexpected return");
+			if (WEXITSTATUS(wstatus) && WEXITSTATUS(wstatus) != KSFT_SKIP)
+				ret = KSFT_FAIL;
+		}
 	}
 
-	return 0;
+	return ret;
 }
 
 /*
@@ -998,6 +1006,7 @@ static struct vcpu_config sve_config = {
 };
 
 static struct vcpu_config *vcpu_configs[] = {
-	reg_list_sve() ? &sve_config : &vregs_config,
+	&vregs_config,
+	&sve_config,
 	NULL
 };
-- 
2.30.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 6/6] KVM: arm64: selftests: get-reg-list: Split base and pmu registers
  2021-05-07 20:04 [PATCH 0/6] KVM: arm64: selftests: Fix get-reg-list Andrew Jones
                   ` (4 preceding siblings ...)
  2021-05-07 20:04 ` [PATCH 5/6] KVM: arm64: selftests: get-reg-list: Remove get-reg-list-sve Andrew Jones
@ 2021-05-07 20:04 ` Andrew Jones
  5 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2021-05-07 20:04 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, ricarkol, eric.auger, alexandru.elisei, pbonzini

Since KVM commit 11663111cd49 ("KVM: arm64: Hide PMU registers from
userspace when not available") the get-reg-list* tests have been
failing with

  ...
  ... There are 74 missing registers.
  The following lines are missing registers:
  ...

where the 74 missing registers are all PMU registers. This isn't a
bug in KVM that the selftest found, even though it's true that a
KVM userspace that wasn't setting the KVM_ARM_VCPU_PMU_V3 VCPU
flag, but still expecting the PMU registers to be in the reg-list,
would suddenly no longer have their expectations met. In that case,
the expectations were wrong, though, so that KVM userspace needs to
be fixed, and so does this selftest. The fix for this selftest is to
pull the PMU registers out of the base register sublist into their
own sublist and then create new, pmu-enabled vcpu configs which we
can be tested.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../selftests/kvm/aarch64/get-reg-list.c      | 50 ++++++++++++++++---
 1 file changed, 42 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
index 2f7caa0b9e57..9bc6e125bae7 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -37,12 +37,16 @@
 
 enum {
 	VREGS,
+	VREGS_PMU,
 	SVE,
+	SVE_PMU,
 };
 
 static char * const vcpu_config_names[] = {
 	[VREGS] = "vregs",
+	[VREGS_PMU] = "vregs+pmu",
 	[SVE] = "sve",
+	[SVE_PMU] = "sve+pmu",
 	NULL
 };
 
@@ -59,6 +63,7 @@ struct reg_sublist {
 struct vcpu_config {
 	const char *name;
 	bool sve;
+	bool pmu;
 	struct reg_sublist sublists[];
 };
 
@@ -339,6 +344,8 @@ static void prepare_vcpu_init(struct vcpu_config *c, struct kvm_vcpu_init *init)
 {
 	if (c->sve)
 		init->features[0] |= 1 << KVM_ARM_VCPU_SVE;
+	if (c->pmu)
+		init->features[0] |= 1 << KVM_ARM_VCPU_PMU_V3;
 }
 
 static void finalize_vcpu(struct kvm_vm *vm, uint32_t vcpuid, struct vcpu_config *c)
@@ -357,6 +364,10 @@ static void check_supported(struct vcpu_config *c)
 		fprintf(stderr, "%s: SVE not available, skipping tests\n", c->name);
 		exit(KSFT_SKIP);
 	}
+	if (c->pmu && !kvm_check_cap(KVM_CAP_ARM_PMU_V3)) {
+		fprintf(stderr, "%s: PMU not available, skipping tests\n", c->name);
+		exit(KSFT_SKIP);
+	}
 }
 
 static bool fixup_core_regs;
@@ -614,7 +625,7 @@ int main(int ac, char **av)
  * The current blessed list was primed with the output of kernel version
  * v4.15 with --core-reg-fixup and then later updated with new registers.
  *
- * The blessed list is up to date with kernel version v5.10-rc5
+ * The blessed list is up to date with kernel version v5.11-rc6
  */
 static __u64 base_regs[] = {
 	KVM_REG_ARM64 | KVM_REG_SIZE_U64 | KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(regs.regs[0]),
@@ -806,8 +817,6 @@ static __u64 base_regs[] = {
 	ARM64_SYS_REG(3, 0, 5, 2, 0),	/* ESR_EL1 */
 	ARM64_SYS_REG(3, 0, 6, 0, 0),	/* FAR_EL1 */
 	ARM64_SYS_REG(3, 0, 7, 4, 0),	/* PAR_EL1 */
-	ARM64_SYS_REG(3, 0, 9, 14, 1),	/* PMINTENSET_EL1 */
-	ARM64_SYS_REG(3, 0, 9, 14, 2),	/* PMINTENCLR_EL1 */
 	ARM64_SYS_REG(3, 0, 10, 2, 0),	/* MAIR_EL1 */
 	ARM64_SYS_REG(3, 0, 10, 3, 0),	/* AMAIR_EL1 */
 	ARM64_SYS_REG(3, 0, 12, 0, 0),	/* VBAR_EL1 */
@@ -816,6 +825,16 @@ static __u64 base_regs[] = {
 	ARM64_SYS_REG(3, 0, 13, 0, 4),	/* TPIDR_EL1 */
 	ARM64_SYS_REG(3, 0, 14, 1, 0),	/* CNTKCTL_EL1 */
 	ARM64_SYS_REG(3, 2, 0, 0, 0),	/* CSSELR_EL1 */
+	ARM64_SYS_REG(3, 3, 13, 0, 2),	/* TPIDR_EL0 */
+	ARM64_SYS_REG(3, 3, 13, 0, 3),	/* TPIDRRO_EL0 */
+	ARM64_SYS_REG(3, 4, 3, 0, 0),	/* DACR32_EL2 */
+	ARM64_SYS_REG(3, 4, 5, 0, 1),	/* IFSR32_EL2 */
+	ARM64_SYS_REG(3, 4, 5, 3, 0),	/* FPEXC32_EL2 */
+};
+
+static __u64 pmu_regs[] = {
+	ARM64_SYS_REG(3, 0, 9, 14, 1),	/* PMINTENSET_EL1 */
+	ARM64_SYS_REG(3, 0, 9, 14, 2),	/* PMINTENCLR_EL1 */
 	ARM64_SYS_REG(3, 3, 9, 12, 0),	/* PMCR_EL0 */
 	ARM64_SYS_REG(3, 3, 9, 12, 1),	/* PMCNTENSET_EL0 */
 	ARM64_SYS_REG(3, 3, 9, 12, 2),	/* PMCNTENCLR_EL0 */
@@ -825,8 +844,6 @@ static __u64 base_regs[] = {
 	ARM64_SYS_REG(3, 3, 9, 13, 0),	/* PMCCNTR_EL0 */
 	ARM64_SYS_REG(3, 3, 9, 14, 0),	/* PMUSERENR_EL0 */
 	ARM64_SYS_REG(3, 3, 9, 14, 3),	/* PMOVSSET_EL0 */
-	ARM64_SYS_REG(3, 3, 13, 0, 2),	/* TPIDR_EL0 */
-	ARM64_SYS_REG(3, 3, 13, 0, 3),	/* TPIDRRO_EL0 */
 	ARM64_SYS_REG(3, 3, 14, 8, 0),
 	ARM64_SYS_REG(3, 3, 14, 8, 1),
 	ARM64_SYS_REG(3, 3, 14, 8, 2),
@@ -890,9 +907,6 @@ static __u64 base_regs[] = {
 	ARM64_SYS_REG(3, 3, 14, 15, 5),
 	ARM64_SYS_REG(3, 3, 14, 15, 6),
 	ARM64_SYS_REG(3, 3, 14, 15, 7),	/* PMCCFILTR_EL0 */
-	ARM64_SYS_REG(3, 4, 3, 0, 0),	/* DACR32_EL2 */
-	ARM64_SYS_REG(3, 4, 5, 0, 1),	/* IFSR32_EL2 */
-	ARM64_SYS_REG(3, 4, 5, 3, 0),	/* FPEXC32_EL2 */
 };
 
 static __u64 vregs[] = {
@@ -996,6 +1010,15 @@ static struct vcpu_config vregs_config = {
 	{0},
 	},
 };
+static struct vcpu_config vregs_pmu_config = {
+	vcpu_config_names[VREGS_PMU], .pmu = true,
+	.sublists = {
+	{ base_regs,	ARRAY_SIZE(base_regs), },
+	{ vregs,	ARRAY_SIZE(vregs), },
+	{ pmu_regs,	ARRAY_SIZE(pmu_regs), },
+	{0},
+	},
+};
 static struct vcpu_config sve_config = {
 	vcpu_config_names[SVE], .sve = true,
 	.sublists = {
@@ -1004,9 +1027,20 @@ static struct vcpu_config sve_config = {
 	{0},
 	},
 };
+static struct vcpu_config sve_pmu_config = {
+	vcpu_config_names[SVE_PMU], .sve = true, .pmu = true,
+	.sublists = {
+	{ base_regs,	ARRAY_SIZE(base_regs), },
+	{ sve_regs,	ARRAY_SIZE(sve_regs),	sve_rejects_set,	ARRAY_SIZE(sve_rejects_set), },
+	{ pmu_regs,	ARRAY_SIZE(pmu_regs), },
+	{0},
+	},
+};
 
 static struct vcpu_config *vcpu_configs[] = {
 	&vregs_config,
+	&vregs_pmu_config,
 	&sve_config,
+	&sve_pmu_config,
 	NULL
 };
-- 
2.30.2


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 4/6] KVM: arm64: selftests: get-reg-list: Provide config selection option
  2021-05-07 20:04 ` [PATCH 4/6] KVM: arm64: selftests: get-reg-list: Provide config selection option Andrew Jones
@ 2021-05-10  6:40   ` Andrew Jones
  0 siblings, 0 replies; 8+ messages in thread
From: Andrew Jones @ 2021-05-10  6:40 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, ricarkol, eric.auger, alexandru.elisei, pbonzini

On Fri, May 07, 2021 at 10:04:14PM +0200, Andrew Jones wrote:
> Add a new command line option that allows the user to select a specific
> configuration, e.g. --config:sve will give the sve config. Also provide
> help text and the --help/-h options.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
>  .../selftests/kvm/aarch64/get-reg-list.c      | 76 +++++++++++++++++--
>  1 file changed, 70 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> index 68d3be86d490..f5e122b6b257 100644
> --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> @@ -38,6 +38,17 @@
>  #define reg_list_sve() (false)
>  #endif
>  
> +enum {
> +	VREGS,
> +	SVE,
> +};
> +
> +static char * const vcpu_config_names[] = {
> +	[VREGS] = "vregs",
> +	[SVE] = "sve",
> +	NULL
> +};
> +
>  static struct kvm_reg_list *reg_list;
>  static __u64 *blessed_reg, blessed_n;
>  
> @@ -502,34 +513,87 @@ static void run_test(struct vcpu_config *c)
>  	kvm_vm_free(vm);
>  }
>  
> +static void help(void)
> +{
> +	char * const *n;
> +
> +	printf(
> +	"\n"
> +	"usage: get-reg-list [--config:<selection>[,<selection>...]] [--list] [--list-filtered] [--core-reg-fixup]\n\n"
> +	" --config:<selection>[,<selection>...] Used to select a specific vcpu configuration for the test/listing\n"

I just realized I left this <selection>[,<selection>...] help text and
some other kruft, like the vcpu_config_names[] array, from a different
design I scrapped. That design, which used getsubopt(), was more
complicated than it was worth.

I'll send a v2 to get this cleaned up, but I'll wait a day or so first
for more comments.

Thanks,
drew


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-05-10  6:40 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-07 20:04 [PATCH 0/6] KVM: arm64: selftests: Fix get-reg-list Andrew Jones
2021-05-07 20:04 ` [PATCH 1/6] KVM: arm64: selftests: get-reg-list: Factor out printing Andrew Jones
2021-05-07 20:04 ` [PATCH 2/6] KVM: arm64: selftests: get-reg-list: Introduce vcpu configs Andrew Jones
2021-05-07 20:04 ` [PATCH 3/6] KVM: arm64: selftests: get-reg-list: Prepare to run multiple configs at once Andrew Jones
2021-05-07 20:04 ` [PATCH 4/6] KVM: arm64: selftests: get-reg-list: Provide config selection option Andrew Jones
2021-05-10  6:40   ` Andrew Jones
2021-05-07 20:04 ` [PATCH 5/6] KVM: arm64: selftests: get-reg-list: Remove get-reg-list-sve Andrew Jones
2021-05-07 20:04 ` [PATCH 6/6] KVM: arm64: selftests: get-reg-list: Split base and pmu registers Andrew Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).