[PATCH v3 0/5] KVM: arm64: selftests: Fix get-reg-list

[PATCH v3 0/5] KVM: arm64: selftests: Fix get-reg-list

[Andrew Jones]

v3:
 - Took Ricardo's suggestions in order to avoid needing to update
   prepare_vcpu_init, finalize_vcpu, and check_supported when adding
   new register sublists by better associating the sublists with their
   vcpu feature bits and caps [Ricardo]
 - We now dynamically generate the vcpu config name by creating them
   from its sublist names [drew]

v2:
 - Removed some cruft left over from a previous more complex design of the
   config command line parser
 - Dropped the list printing factor out patch as it's not necessary
 - Added a 'PASS' output for passing tests to allow testers to feel good
 - Changed the "up to date with kernel" comment to reference 5.13.0-rc2


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 5 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 (5):
  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      | 439 +++++++++++++-----
 4 files changed, 321 insertions(+), 123 deletions(-)
 delete mode 100644 tools/testing/selftests/kvm/aarch64/get-reg-list-sve.c

-- 
2.31.1

[PATCH v3 1/5] KVM: arm64: selftests: get-reg-list: Introduce vcpu configs

[Andrew Jones]

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).

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../selftests/kvm/aarch64/get-reg-list.c      | 265 ++++++++++++------
 1 file changed, 175 insertions(+), 90 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..7bb09ce20dde 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -37,7 +37,30 @@
 #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 {
+	const char *name;
+	long capability;
+	int feature;
+	bool finalize;
+	__u64 *regs;
+	__u64 regs_n;
+	__u64 *rejects_set;
+	__u64 rejects_set_n;
+};
+
+struct vcpu_config {
+	char *name;
+	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))
@@ -54,12 +77,41 @@
 	for_each_reg_filtered(i)						\
 		if (!find_reg(blessed_reg, blessed_n, reg_list->reg[i]))
 
+static const char *config_name(struct vcpu_config *c)
+{
+	struct reg_sublist *s;
+	int len = 0;
 
-static struct kvm_reg_list *reg_list;
+	if (c->name)
+		return c->name;
 
-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;
+	for_each_sublist(c, s)
+		len += strlen(s->name) + 1;
+
+	c->name = malloc(len);
+
+	len = 0;
+	for_each_sublist(c, s) {
+		if (!strcmp(s->name, "base"))
+			continue;
+		strcat(c->name + len, s->name);
+		len += strlen(s->name) + 1;
+		c->name[len - 1] = '+';
+	}
+	c->name[len - 1] = '\0';
+
+	return c->name;
+}
+
+static bool has_cap(struct vcpu_config *c, long capability)
+{
+	struct reg_sublist *s;
+
+	for_each_sublist(c, s)
+		if (s->capability == capability)
+			return true;
+	return false;
+}
 
 static bool filter_reg(__u64 reg)
 {
@@ -96,11 +148,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;
 
@@ -111,7 +165,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", config_name(c), 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)";
@@ -126,12 +180,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", config_name(c), 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", config_name(c), 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)";
@@ -139,11 +193,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", config_name(c), 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;
 
@@ -153,37 +207,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", config_name(c), 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", config_name(c), 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", config_name(c), 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", config_name(c), 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", config_name(c), id);
 
 	switch (id & KVM_REG_SIZE_MASK) {
 	case KVM_REG_SIZE_U8:
@@ -214,17 +268,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",
+			  config_name(c), (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", config_name(c), 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;
@@ -235,23 +289,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", config_name(c), 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", config_name(c), 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 (has_cap(c, KVM_CAP_ARM_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", config_name(c), 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",
+			  config_name(c), (id & KVM_REG_ARM_COPROC_MASK) >> KVM_REG_ARM_COPROC_SHIFT, id);
 	}
 }
 
@@ -312,40 +366,51 @@ 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())
-		init->features[0] |= 1 << KVM_ARM_VCPU_SVE;
+	struct reg_sublist *s;
+
+	for_each_sublist(c, s)
+		if (s->capability)
+			init->features[s->feature / 32] |= 1 << (s->feature % 32);
 }
 
-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)
 {
+	struct reg_sublist *s;
 	int feature;
 
-	if (reg_list_sve()) {
-		feature = KVM_ARM_VCPU_SVE;
-		vcpu_ioctl(vm, vcpuid, KVM_ARM_VCPU_FINALIZE, &feature);
+	for_each_sublist(c, s) {
+		if (s->finalize) {
+			feature = s->feature;
+			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");
-		exit(KSFT_SKIP);
+	struct reg_sublist *s;
+
+	for_each_sublist(c, s) {
+		if (s->capability && !kvm_check_cap(s->capability)) {
+			fprintf(stderr, "%s: %s not available, skipping tests\n", config_name(c), s->name);
+			exit(KSFT_SKIP);
+		}
 	}
 }
 
 int main(int ac, char **av)
 {
+	struct vcpu_config *c = reg_list_sve() ? &sve_config : &vregs_config;
 	struct kvm_vcpu_init init = { .target = -1, };
-	int new_regs = 0, missing_regs = 0, i;
+	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, fixup_core_regs = 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)
@@ -359,9 +424,9 @@ int main(int ac, char **av)
 	}
 
 	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
-	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);
 
@@ -374,7 +439,7 @@ int main(int ac, char **av)
 			__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');
 		return 0;
@@ -396,50 +461,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 ", config_name(c));
+			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) ", config_name(c), 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 ", config_name(c));
+				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;
@@ -448,31 +515,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", config_name(c), blessed_n);
+		printf("%s: Number registers:         %5lld\n", config_name(c), 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", config_name(c), 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", config_name(c), 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);
+		    config_name(c), missing_regs, failed_get, failed_set, failed_reject);
 
 	return 0;
 }
@@ -761,7 +828,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]),
@@ -797,7 +863,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,
@@ -852,11 +917,31 @@ 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);
+
+#define BASE_SUBLIST \
+	{ "base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), }
+#define VREGS_SUBLIST \
+	{ "vregs", .regs = vregs, .regs_n = ARRAY_SIZE(vregs), }
+#define SVE_SUBLIST \
+	{ "sve", .capability = KVM_CAP_ARM_SVE, .feature = KVM_ARM_VCPU_SVE, .finalize = true, \
+	  .regs = sve_regs, .regs_n = ARRAY_SIZE(sve_regs), \
+	  .rejects_set = sve_rejects_set, .rejects_set_n = ARRAY_SIZE(sve_rejects_set), }
+
+static struct vcpu_config vregs_config = {
+	.sublists = {
+	BASE_SUBLIST,
+	VREGS_SUBLIST,
+	{0},
+	},
+};
+static struct vcpu_config sve_config = {
+	.sublists = {
+	BASE_SUBLIST,
+	SVE_SUBLIST,
+	{0},
+	},
+};
-- 
2.31.1

[PATCH v3 2/5] KVM: arm64: selftests: get-reg-list: Prepare to run multiple configs at once

[Andrew Jones]

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      | 68 ++++++++++++++-----
 1 file changed, 51 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
index 7bb09ce20dde..14fc8d82e30f 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -56,8 +56,8 @@ 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[];
+static int vcpu_configs_n;
 
 #define for_each_sublist(c, s)							\
 	for ((s) = &(c)->sublists[0]; (s)->regs; ++(s))
@@ -400,29 +400,20 @@ static void check_supported(struct vcpu_config *c)
 	}
 }
 
-int main(int ac, char **av)
+static bool print_list;
+static bool print_filtered;
+static bool fixup_core_regs;
+
+static void run_test(struct vcpu_config *c)
 {
-	struct vcpu_config *c = reg_list_sve() ? &sve_config : &vregs_config;
 	struct kvm_vcpu_init init = { .target = -1, };
 	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, fixup_core_regs = 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]);
-	}
-
 	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
 	prepare_vcpu_init(c, &init);
 	aarch64_vcpu_add_default(vm, 0, &init, NULL);
@@ -442,7 +433,7 @@ int main(int ac, char **av)
 				print_reg(c, id);
 		}
 		putchar('\n');
-		return 0;
+		return;
 	}
 
 	/*
@@ -541,6 +532,44 @@ int main(int ac, char **av)
 		    "%d registers failed get; %d registers failed set; %d registers failed reject",
 		    config_name(c), missing_regs, failed_get, failed_set, failed_reject);
 
+	pr_info("%s: PASS\n", config_name(c));
+	blessed_n = 0;
+	free(blessed_reg);
+	free(reg_list);
+	kvm_vm_free(vm);
+}
+
+int main(int ac, char **av)
+{
+	struct vcpu_config *c, *sel = NULL;
+	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.
+		 */
+		sel = vcpu_configs[0];
+	}
+
+	for (i = 0; i < vcpu_configs_n; ++i) {
+		c = vcpu_configs[i];
+		if (sel && c != sel)
+			continue;
+		run_test(c);
+	}
+
 	return 0;
 }
 
@@ -945,3 +974,8 @@ static struct vcpu_config sve_config = {
 	{0},
 	},
 };
+
+static struct vcpu_config *vcpu_configs[] = {
+	reg_list_sve() ? &sve_config : &vregs_config,
+};
+static int vcpu_configs_n = ARRAY_SIZE(vcpu_configs);
-- 
2.31.1

[PATCH v3 3/5] KVM: arm64: selftests: get-reg-list: Provide config selection option

[Andrew Jones]

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      | 56 ++++++++++++++++++-
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
index 14fc8d82e30f..03e041d97a18 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -539,6 +539,52 @@ static void run_test(struct vcpu_config *c)
 	kvm_vm_free(vm);
 }
 
+static void help(void)
+{
+	struct vcpu_config *c;
+	int i;
+
+	printf(
+	"\n"
+	"usage: get-reg-list [--config=<selection>] [--list] [--list-filtered] [--core-reg-fixup]\n\n"
+	" --config=<selection>        Used to select a specific vcpu configuration for the test/listing\n"
+	"                             '<selection>' may be\n");
+
+	for (i = 0; i < vcpu_configs_n; ++i) {
+		c = vcpu_configs[i];
+		printf(
+	"                               '%s'\n", config_name(c));
+	}
+
+	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;
+	int i;
+
+	if (config[8] != '=')
+		help(), exit(1);
+
+	for (i = 0; i < vcpu_configs_n; ++i) {
+		c = vcpu_configs[i];
+		if (strcmp(config_name(c), &config[9]) == 0)
+			break;
+	}
+
+	if (i == vcpu_configs_n)
+		help(), exit(1);
+
+	return c;
+}
+
 int main(int ac, char **av)
 {
 	struct vcpu_config *c, *sel = NULL;
@@ -547,20 +593,24 @@ int main(int ac, char **av)
 	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)
+			sel = 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.
 		 */
-		sel = vcpu_configs[0];
+		if (!sel)
+			help(), exit(1);
 	}
 
 	for (i = 0; i < vcpu_configs_n; ++i) {
-- 
2.31.1

[PATCH v3 4/5] KVM: arm64: selftests: get-reg-list: Remove get-reg-list-sve

[Andrew Jones]

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, 21 insertions(+), 15 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 524c857a049c..dd36575b732a 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 daaee1888b12..5c8f3725a7f0 100644
--- a/tools/testing/selftests/kvm/Makefile
+++ b/tools/testing/selftests/kvm/Makefile
@@ -79,7 +79,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 03e041d97a18..b46b8a1fdc0c 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -27,16 +27,13 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.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
-
 static struct kvm_reg_list *reg_list;
 static __u64 *blessed_reg, blessed_n;
 
@@ -588,7 +585,8 @@ static struct vcpu_config *parse_config(const char *config)
 int main(int ac, char **av)
 {
 	struct vcpu_config *c, *sel = NULL;
-	int i;
+	int i, ret = 0;
+	pid_t pid;
 
 	for (i = 1; i < ac; ++i) {
 		if (strcmp(av[i], "--core-reg-fixup") == 0)
@@ -617,10 +615,22 @@ int main(int ac, char **av)
 		c = vcpu_configs[i];
 		if (sel && c != sel)
 			continue;
-		run_test(c);
+
+		pid = fork();
+
+		if (!pid) {
+			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;
 }
 
 /*
@@ -1026,6 +1036,7 @@ static struct vcpu_config sve_config = {
 };
 
 static struct vcpu_config *vcpu_configs[] = {
-	reg_list_sve() ? &sve_config : &vregs_config,
+	&vregs_config,
+	&sve_config,
 };
 static int vcpu_configs_n = ARRAY_SIZE(vcpu_configs);
-- 
2.31.1

[PATCH v3 5/5] KVM: arm64: selftests: get-reg-list: Split base and pmu registers

[Andrew Jones]

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 can
be tested.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 .../selftests/kvm/aarch64/get-reg-list.c      | 39 +++++++++++++++----
 1 file changed, 31 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 b46b8a1fdc0c..a16c8f05366c 100644
--- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
+++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
@@ -637,7 +637,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.13-rc3
  */
 static __u64 base_regs[] = {
 	KVM_REG_ARM64 | KVM_REG_SIZE_U64 | KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(regs.regs[0]),
@@ -829,8 +829,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 */
@@ -839,6 +837,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 */
@@ -848,8 +856,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),
@@ -913,9 +919,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[] = {
@@ -1015,6 +1018,8 @@ static __u64 sve_rejects_set[] = {
 	{ "base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), }
 #define VREGS_SUBLIST \
 	{ "vregs", .regs = vregs, .regs_n = ARRAY_SIZE(vregs), }
+#define PMU_SUBLIST \
+	{ "pmu", .regs = pmu_regs, .regs_n = ARRAY_SIZE(pmu_regs), }
 #define SVE_SUBLIST \
 	{ "sve", .capability = KVM_CAP_ARM_SVE, .feature = KVM_ARM_VCPU_SVE, .finalize = true, \
 	  .regs = sve_regs, .regs_n = ARRAY_SIZE(sve_regs), \
@@ -1027,6 +1032,14 @@ static struct vcpu_config vregs_config = {
 	{0},
 	},
 };
+static struct vcpu_config vregs_pmu_config = {
+	.sublists = {
+	BASE_SUBLIST,
+	VREGS_SUBLIST,
+	PMU_SUBLIST,
+	{0},
+	},
+};
 static struct vcpu_config sve_config = {
 	.sublists = {
 	BASE_SUBLIST,
@@ -1034,9 +1047,19 @@ static struct vcpu_config sve_config = {
 	{0},
 	},
 };
+static struct vcpu_config sve_pmu_config = {
+	.sublists = {
+	BASE_SUBLIST,
+	SVE_SUBLIST,
+	PMU_SUBLIST,
+	{0},
+	},
+};
 
 static struct vcpu_config *vcpu_configs[] = {
 	&vregs_config,
+	&vregs_pmu_config,
 	&sve_config,
+	&sve_pmu_config,
 };
 static int vcpu_configs_n = ARRAY_SIZE(vcpu_configs);
-- 
2.31.1

Re: [PATCH v3 5/5] KVM: arm64: selftests: get-reg-list: Split base and pmu registers

[Ricardo Koller]

On Mon, May 31, 2021 at 12:33:44PM +0200, Andrew Jones wrote:
> 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 can
> be tested.
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Reviewed-by: Ricardo Koller <ricarkol@google.com>

> ---
>  .../selftests/kvm/aarch64/get-reg-list.c      | 39 +++++++++++++++----
>  1 file changed, 31 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 b46b8a1fdc0c..a16c8f05366c 100644
> --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> @@ -637,7 +637,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.13-rc3
>   */
>  static __u64 base_regs[] = {
>  	KVM_REG_ARM64 | KVM_REG_SIZE_U64 | KVM_REG_ARM_CORE | KVM_REG_ARM_CORE_REG(regs.regs[0]),
> @@ -829,8 +829,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 */
> @@ -839,6 +837,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 */
> @@ -848,8 +856,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),
> @@ -913,9 +919,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[] = {
> @@ -1015,6 +1018,8 @@ static __u64 sve_rejects_set[] = {
>  	{ "base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), }
>  #define VREGS_SUBLIST \
>  	{ "vregs", .regs = vregs, .regs_n = ARRAY_SIZE(vregs), }
> +#define PMU_SUBLIST \
> +	{ "pmu", .regs = pmu_regs, .regs_n = ARRAY_SIZE(pmu_regs), }
>  #define SVE_SUBLIST \
>  	{ "sve", .capability = KVM_CAP_ARM_SVE, .feature = KVM_ARM_VCPU_SVE, .finalize = true, \
>  	  .regs = sve_regs, .regs_n = ARRAY_SIZE(sve_regs), \
> @@ -1027,6 +1032,14 @@ static struct vcpu_config vregs_config = {
>  	{0},
>  	},
>  };
> +static struct vcpu_config vregs_pmu_config = {
> +	.sublists = {
> +	BASE_SUBLIST,
> +	VREGS_SUBLIST,
> +	PMU_SUBLIST,
> +	{0},
> +	},
> +};
>  static struct vcpu_config sve_config = {
>  	.sublists = {
>  	BASE_SUBLIST,
> @@ -1034,9 +1047,19 @@ static struct vcpu_config sve_config = {
>  	{0},
>  	},
>  };
> +static struct vcpu_config sve_pmu_config = {
> +	.sublists = {
> +	BASE_SUBLIST,
> +	SVE_SUBLIST,
> +	PMU_SUBLIST,
> +	{0},
> +	},
> +};
>  
>  static struct vcpu_config *vcpu_configs[] = {
>  	&vregs_config,
> +	&vregs_pmu_config,
>  	&sve_config,
> +	&sve_pmu_config,
>  };
>  static int vcpu_configs_n = ARRAY_SIZE(vcpu_configs);
> -- 
> 2.31.1
> 

Re: [PATCH v3 1/5] KVM: arm64: selftests: get-reg-list: Introduce vcpu configs

[Ricardo Koller]

On Mon, May 31, 2021 at 12:33:40PM +0200, Andrew Jones wrote:
> 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).
> 
> Signed-off-by: Andrew Jones <drjones@redhat.com>

Reviewed-by: Ricardo Koller <ricarkol@google.com>

> ---
>  .../selftests/kvm/aarch64/get-reg-list.c      | 265 ++++++++++++------
>  1 file changed, 175 insertions(+), 90 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..7bb09ce20dde 100644
> --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> @@ -37,7 +37,30 @@
>  #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 {
> +	const char *name;
> +	long capability;
> +	int feature;
> +	bool finalize;
> +	__u64 *regs;
> +	__u64 regs_n;
> +	__u64 *rejects_set;
> +	__u64 rejects_set_n;
> +};
> +
> +struct vcpu_config {
> +	char *name;
> +	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))
> @@ -54,12 +77,41 @@
>  	for_each_reg_filtered(i)						\
>  		if (!find_reg(blessed_reg, blessed_n, reg_list->reg[i]))
>  
> +static const char *config_name(struct vcpu_config *c)
> +{
> +	struct reg_sublist *s;
> +	int len = 0;
>  
> -static struct kvm_reg_list *reg_list;
> +	if (c->name)
> +		return c->name;
>  
> -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;
> +	for_each_sublist(c, s)
> +		len += strlen(s->name) + 1;
> +
> +	c->name = malloc(len);
> +
> +	len = 0;
> +	for_each_sublist(c, s) {
> +		if (!strcmp(s->name, "base"))
> +			continue;
> +		strcat(c->name + len, s->name);
> +		len += strlen(s->name) + 1;
> +		c->name[len - 1] = '+';
> +	}
> +	c->name[len - 1] = '\0';
> +
> +	return c->name;
> +}
> +
> +static bool has_cap(struct vcpu_config *c, long capability)
> +{
> +	struct reg_sublist *s;
> +
> +	for_each_sublist(c, s)
> +		if (s->capability == capability)
> +			return true;
> +	return false;
> +}
>  
>  static bool filter_reg(__u64 reg)
>  {
> @@ -96,11 +148,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;
>  
> @@ -111,7 +165,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", config_name(c), 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)";
> @@ -126,12 +180,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", config_name(c), 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", config_name(c), 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)";
> @@ -139,11 +193,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", config_name(c), 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;
>  
> @@ -153,37 +207,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", config_name(c), 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", config_name(c), 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", config_name(c), 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", config_name(c), 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", config_name(c), id);
>  
>  	switch (id & KVM_REG_SIZE_MASK) {
>  	case KVM_REG_SIZE_U8:
> @@ -214,17 +268,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",
> +			  config_name(c), (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", config_name(c), 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;
> @@ -235,23 +289,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", config_name(c), 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", config_name(c), 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 (has_cap(c, KVM_CAP_ARM_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", config_name(c), 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",
> +			  config_name(c), (id & KVM_REG_ARM_COPROC_MASK) >> KVM_REG_ARM_COPROC_SHIFT, id);
>  	}
>  }
>  
> @@ -312,40 +366,51 @@ 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())
> -		init->features[0] |= 1 << KVM_ARM_VCPU_SVE;
> +	struct reg_sublist *s;
> +
> +	for_each_sublist(c, s)
> +		if (s->capability)
> +			init->features[s->feature / 32] |= 1 << (s->feature % 32);
>  }
>  
> -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)
>  {
> +	struct reg_sublist *s;
>  	int feature;
>  
> -	if (reg_list_sve()) {
> -		feature = KVM_ARM_VCPU_SVE;
> -		vcpu_ioctl(vm, vcpuid, KVM_ARM_VCPU_FINALIZE, &feature);
> +	for_each_sublist(c, s) {
> +		if (s->finalize) {
> +			feature = s->feature;
> +			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");
> -		exit(KSFT_SKIP);
> +	struct reg_sublist *s;
> +
> +	for_each_sublist(c, s) {
> +		if (s->capability && !kvm_check_cap(s->capability)) {
> +			fprintf(stderr, "%s: %s not available, skipping tests\n", config_name(c), s->name);
> +			exit(KSFT_SKIP);
> +		}
>  	}
>  }
>  
>  int main(int ac, char **av)
>  {
> +	struct vcpu_config *c = reg_list_sve() ? &sve_config : &vregs_config;
>  	struct kvm_vcpu_init init = { .target = -1, };
> -	int new_regs = 0, missing_regs = 0, i;
> +	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, fixup_core_regs = 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)
> @@ -359,9 +424,9 @@ int main(int ac, char **av)
>  	}
>  
>  	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
> -	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);
>  
> @@ -374,7 +439,7 @@ int main(int ac, char **av)
>  			__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');
>  		return 0;
> @@ -396,50 +461,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 ", config_name(c));
> +			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) ", config_name(c), 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 ", config_name(c));
> +				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;
> @@ -448,31 +515,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", config_name(c), blessed_n);
> +		printf("%s: Number registers:         %5lld\n", config_name(c), 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", config_name(c), 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", config_name(c), 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);
> +		    config_name(c), missing_regs, failed_get, failed_set, failed_reject);
>  
>  	return 0;
>  }
> @@ -761,7 +828,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]),
> @@ -797,7 +863,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,
> @@ -852,11 +917,31 @@ 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);
> +
> +#define BASE_SUBLIST \
> +	{ "base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), }

I don't think it matters that much for this test, but ".capability = 0"
is already taken:

#define KVM_CAP_IRQCHIP	  0

> +#define VREGS_SUBLIST \
> +	{ "vregs", .regs = vregs, .regs_n = ARRAY_SIZE(vregs), }
> +#define SVE_SUBLIST \
> +	{ "sve", .capability = KVM_CAP_ARM_SVE, .feature = KVM_ARM_VCPU_SVE, .finalize = true, \
> +	  .regs = sve_regs, .regs_n = ARRAY_SIZE(sve_regs), \
> +	  .rejects_set = sve_rejects_set, .rejects_set_n = ARRAY_SIZE(sve_rejects_set), }
> +
> +static struct vcpu_config vregs_config = {
> +	.sublists = {
> +	BASE_SUBLIST,
> +	VREGS_SUBLIST,
> +	{0},
> +	},
> +};
> +static struct vcpu_config sve_config = {
> +	.sublists = {
> +	BASE_SUBLIST,
> +	SVE_SUBLIST,
> +	{0},
> +	},
> +};
> -- 
> 2.31.1
> 

Re: [PATCH v3 2/5] KVM: arm64: selftests: get-reg-list: Prepare to run multiple configs at once

[Ricardo Koller]

On Mon, May 31, 2021 at 12:33:41PM +0200, Andrew Jones wrote:
> 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>

Reviewed-by: Ricardo Koller <ricarkol@google.com>

> ---
>  .../selftests/kvm/aarch64/get-reg-list.c      | 68 ++++++++++++++-----
>  1 file changed, 51 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> index 7bb09ce20dde..14fc8d82e30f 100644
> --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> @@ -56,8 +56,8 @@ 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[];
> +static int vcpu_configs_n;
>  
>  #define for_each_sublist(c, s)							\
>  	for ((s) = &(c)->sublists[0]; (s)->regs; ++(s))
> @@ -400,29 +400,20 @@ static void check_supported(struct vcpu_config *c)
>  	}
>  }
>  
> -int main(int ac, char **av)
> +static bool print_list;
> +static bool print_filtered;
> +static bool fixup_core_regs;
> +
> +static void run_test(struct vcpu_config *c)
>  {
> -	struct vcpu_config *c = reg_list_sve() ? &sve_config : &vregs_config;
>  	struct kvm_vcpu_init init = { .target = -1, };
>  	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, fixup_core_regs = 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]);
> -	}
> -
>  	vm = vm_create(VM_MODE_DEFAULT, DEFAULT_GUEST_PHY_PAGES, O_RDWR);
>  	prepare_vcpu_init(c, &init);
>  	aarch64_vcpu_add_default(vm, 0, &init, NULL);
> @@ -442,7 +433,7 @@ int main(int ac, char **av)
>  				print_reg(c, id);
>  		}
>  		putchar('\n');
> -		return 0;
> +		return;
>  	}
>  
>  	/*
> @@ -541,6 +532,44 @@ int main(int ac, char **av)
>  		    "%d registers failed get; %d registers failed set; %d registers failed reject",
>  		    config_name(c), missing_regs, failed_get, failed_set, failed_reject);
>  
> +	pr_info("%s: PASS\n", config_name(c));
> +	blessed_n = 0;
> +	free(blessed_reg);
> +	free(reg_list);
> +	kvm_vm_free(vm);
> +}
> +
> +int main(int ac, char **av)
> +{
> +	struct vcpu_config *c, *sel = NULL;
> +	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.
> +		 */
> +		sel = vcpu_configs[0];
> +	}
> +
> +	for (i = 0; i < vcpu_configs_n; ++i) {
> +		c = vcpu_configs[i];
> +		if (sel && c != sel)
> +			continue;
> +		run_test(c);
> +	}
> +
>  	return 0;
>  }
>  
> @@ -945,3 +974,8 @@ static struct vcpu_config sve_config = {
>  	{0},
>  	},
>  };
> +
> +static struct vcpu_config *vcpu_configs[] = {
> +	reg_list_sve() ? &sve_config : &vregs_config,
> +};
> +static int vcpu_configs_n = ARRAY_SIZE(vcpu_configs);
> -- 
> 2.31.1
> 

Re: [PATCH v3 3/5] KVM: arm64: selftests: get-reg-list: Provide config selection option

[Ricardo Koller]

On Mon, May 31, 2021 at 12:33:42PM +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>

Reviewed-by: Ricardo Koller <ricarkol@google.com>

> ---
>  .../selftests/kvm/aarch64/get-reg-list.c      | 56 ++++++++++++++++++-
>  1 file changed, 53 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/kvm/aarch64/get-reg-list.c b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> index 14fc8d82e30f..03e041d97a18 100644
> --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> @@ -539,6 +539,52 @@ static void run_test(struct vcpu_config *c)
>  	kvm_vm_free(vm);
>  }
>  
> +static void help(void)
> +{
> +	struct vcpu_config *c;
> +	int i;
> +
> +	printf(
> +	"\n"
> +	"usage: get-reg-list [--config=<selection>] [--list] [--list-filtered] [--core-reg-fixup]\n\n"
> +	" --config=<selection>        Used to select a specific vcpu configuration for the test/listing\n"
> +	"                             '<selection>' may be\n");
> +
> +	for (i = 0; i < vcpu_configs_n; ++i) {
> +		c = vcpu_configs[i];
> +		printf(
> +	"                               '%s'\n", config_name(c));
> +	}
> +
> +	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;
> +	int i;
> +
> +	if (config[8] != '=')
> +		help(), exit(1);
> +
> +	for (i = 0; i < vcpu_configs_n; ++i) {
> +		c = vcpu_configs[i];
> +		if (strcmp(config_name(c), &config[9]) == 0)
> +			break;
> +	}
> +
> +	if (i == vcpu_configs_n)
> +		help(), exit(1);
> +
> +	return c;
> +}
> +
>  int main(int ac, char **av)
>  {
>  	struct vcpu_config *c, *sel = NULL;
> @@ -547,20 +593,24 @@ int main(int ac, char **av)
>  	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)
> +			sel = 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.
>  		 */
> -		sel = vcpu_configs[0];
> +		if (!sel)
> +			help(), exit(1);
>  	}
>  
>  	for (i = 0; i < vcpu_configs_n; ++i) {
> -- 
> 2.31.1
> 

Re: [PATCH v3 4/5] KVM: arm64: selftests: get-reg-list: Remove get-reg-list-sve

[Ricardo Koller]

On Mon, May 31, 2021 at 12:33:43PM +0200, Andrew Jones wrote:
> 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>

Reviewed-by: Ricardo Koller <ricarkol@google.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, 21 insertions(+), 15 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 524c857a049c..dd36575b732a 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 daaee1888b12..5c8f3725a7f0 100644
> --- a/tools/testing/selftests/kvm/Makefile
> +++ b/tools/testing/selftests/kvm/Makefile
> @@ -79,7 +79,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 03e041d97a18..b46b8a1fdc0c 100644
> --- a/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/aarch64/get-reg-list.c
> @@ -27,16 +27,13 @@
>  #include <stdio.h>
>  #include <stdlib.h>
>  #include <string.h>
> +#include <unistd.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
> -
>  static struct kvm_reg_list *reg_list;
>  static __u64 *blessed_reg, blessed_n;
>  
> @@ -588,7 +585,8 @@ static struct vcpu_config *parse_config(const char *config)
>  int main(int ac, char **av)
>  {
>  	struct vcpu_config *c, *sel = NULL;
> -	int i;
> +	int i, ret = 0;
> +	pid_t pid;
>  
>  	for (i = 1; i < ac; ++i) {
>  		if (strcmp(av[i], "--core-reg-fixup") == 0)
> @@ -617,10 +615,22 @@ int main(int ac, char **av)
>  		c = vcpu_configs[i];
>  		if (sel && c != sel)
>  			continue;
> -		run_test(c);
> +
> +		pid = fork();
> +
> +		if (!pid) {
> +			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;
>  }
>  
>  /*
> @@ -1026,6 +1036,7 @@ static struct vcpu_config sve_config = {
>  };
>  
>  static struct vcpu_config *vcpu_configs[] = {
> -	reg_list_sve() ? &sve_config : &vregs_config,
> +	&vregs_config,
> +	&sve_config,
>  };
>  static int vcpu_configs_n = ARRAY_SIZE(vcpu_configs);
> -- 
> 2.31.1
> 

Re: [PATCH v3 1/5] KVM: arm64: selftests: get-reg-list: Introduce vcpu configs

[Andrew Jones]

On Wed, Jun 02, 2021 at 04:40:28PM -0700, Ricardo Koller wrote:
> On Mon, May 31, 2021 at 12:33:40PM +0200, Andrew Jones wrote:
> > 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).
> > 
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> 
> Reviewed-by: Ricardo Koller <ricarkol@google.com>

Thanks

> I don't think it matters that much for this test, but ".capability = 0"
> is already taken:
> 
> #define KVM_CAP_IRQCHIP	  0
>

This is a good point, and I should have checked it before assuming I
could use zero for "no special capabilities". I'm tempted to leave this
as is for now though. If we ever need to consider KVM_CAP_IRQCHIP,
then we can cross that bridge later.

Thanks,
drew

Re: [PATCH v3 0/5] KVM: arm64: selftests: Fix get-reg-list

[Andrew Jones]

On Mon, May 31, 2021 at 12:33:39PM +0200, Andrew Jones wrote:
> v3:
>  - Took Ricardo's suggestions in order to avoid needing to update
>    prepare_vcpu_init, finalize_vcpu, and check_supported when adding
>    new register sublists by better associating the sublists with their
>    vcpu feature bits and caps [Ricardo]
>  - We now dynamically generate the vcpu config name by creating them
>    from its sublist names [drew]
> 
> v2:
>  - Removed some cruft left over from a previous more complex design of the
>    config command line parser
>  - Dropped the list printing factor out patch as it's not necessary
>  - Added a 'PASS' output for passing tests to allow testers to feel good
>  - Changed the "up to date with kernel" comment to reference 5.13.0-rc2
> 
> 
> 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 5 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 (5):
>   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      | 439 +++++++++++++-----
>  4 files changed, 321 insertions(+), 123 deletions(-)
>  delete mode 100644 tools/testing/selftests/kvm/aarch64/get-reg-list-sve.c
> 
> -- 
> 2.31.1
>

Gentle ping.

I'm not sure if I'm pinging Marc or Paolo though. MAINTAINERS shows Paolo
as all kvm selftests, but I think Marc has started picking up the AArch64
specific kvm selftests.

Marc, if you've decided to maintain AArch64 kvm selftests, would you be
opposed to adding

  F: tools/testing/selftests/kvm/*/aarch64/
  F: tools/testing/selftests/kvm/aarch64/

to "KERNEL VIRTUAL MACHINE FOR ARM64 (KVM/arm64)"?

Thanks,
drew

Re: [PATCH v3 0/5] KVM: arm64: selftests: Fix get-reg-list

[Marc Zyngier]

Hi Andrew,

On Tue, 22 Jun 2021 08:07:32 +0100,
Andrew Jones <drjones@redhat.com> wrote:
> 
> On Mon, May 31, 2021 at 12:33:39PM +0200, Andrew Jones wrote:
> > v3:
> >  - Took Ricardo's suggestions in order to avoid needing to update
> >    prepare_vcpu_init, finalize_vcpu, and check_supported when adding
> >    new register sublists by better associating the sublists with their
> >    vcpu feature bits and caps [Ricardo]
> >  - We now dynamically generate the vcpu config name by creating them
> >    from its sublist names [drew]
> > 
> > v2:
> >  - Removed some cruft left over from a previous more complex design of the
> >    config command line parser
> >  - Dropped the list printing factor out patch as it's not necessary
> >  - Added a 'PASS' output for passing tests to allow testers to feel good
> >  - Changed the "up to date with kernel" comment to reference 5.13.0-rc2
> > 
> > 
> > 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 5 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 (5):
> >   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      | 439 +++++++++++++-----
> >  4 files changed, 321 insertions(+), 123 deletions(-)
> >  delete mode 100644 tools/testing/selftests/kvm/aarch64/get-reg-list-sve.c
> > 
> > -- 
> > 2.31.1
> >
> 
> Gentle ping.
> 
> I'm not sure if I'm pinging Marc or Paolo though. MAINTAINERS shows Paolo
> as all kvm selftests, but I think Marc has started picking up the AArch64
> specific kvm selftests.

I'm happy to queue this series.

> Marc, if you've decided to maintain AArch64 kvm selftests, would you be
> opposed to adding
> 
>   F: tools/testing/selftests/kvm/*/aarch64/
>   F: tools/testing/selftests/kvm/aarch64/
> 
> to "KERNEL VIRTUAL MACHINE FOR ARM64 (KVM/arm64)"?

No problem to add this, but I *will* rely on you (and whoever wants to
part-take) to do the bulk of the reviewing. Do we have a deal?

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v3 0/5] KVM: arm64: selftests: Fix get-reg-list

[Andrew Jones]

On Tue, Jun 22, 2021 at 08:32:51AM +0100, Marc Zyngier wrote:
> Hi Andrew,
> 
> On Tue, 22 Jun 2021 08:07:32 +0100,
> Andrew Jones <drjones@redhat.com> wrote:
> > 
> > On Mon, May 31, 2021 at 12:33:39PM +0200, Andrew Jones wrote:
> > > v3:
> > >  - Took Ricardo's suggestions in order to avoid needing to update
> > >    prepare_vcpu_init, finalize_vcpu, and check_supported when adding
> > >    new register sublists by better associating the sublists with their
> > >    vcpu feature bits and caps [Ricardo]
> > >  - We now dynamically generate the vcpu config name by creating them
> > >    from its sublist names [drew]
> > > 
> > > v2:
> > >  - Removed some cruft left over from a previous more complex design of the
> > >    config command line parser
> > >  - Dropped the list printing factor out patch as it's not necessary
> > >  - Added a 'PASS' output for passing tests to allow testers to feel good
> > >  - Changed the "up to date with kernel" comment to reference 5.13.0-rc2
> > > 
> > > 
> > > 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 5 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 (5):
> > >   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      | 439 +++++++++++++-----
> > >  4 files changed, 321 insertions(+), 123 deletions(-)
> > >  delete mode 100644 tools/testing/selftests/kvm/aarch64/get-reg-list-sve.c
> > > 
> > > -- 
> > > 2.31.1
> > >
> > 
> > Gentle ping.
> > 
> > I'm not sure if I'm pinging Marc or Paolo though. MAINTAINERS shows Paolo
> > as all kvm selftests, but I think Marc has started picking up the AArch64
> > specific kvm selftests.
> 
> I'm happy to queue this series.
> 
> > Marc, if you've decided to maintain AArch64 kvm selftests, would you be
> > opposed to adding
> > 
> >   F: tools/testing/selftests/kvm/*/aarch64/
> >   F: tools/testing/selftests/kvm/aarch64/
> > 
> > to "KERNEL VIRTUAL MACHINE FOR ARM64 (KVM/arm64)"?
> 
> No problem to add this, but I *will* rely on you (and whoever wants to
> part-take) to do the bulk of the reviewing. Do we have a deal?

It's a deal. Thanks!

drew

> 
> Thanks,
> 
> 	M.
> 
> -- 
> Without deviation from the norm, progress is not possible.
> 

Re: [PATCH v3 0/5] KVM: arm64: selftests: Fix get-reg-list

[Marc Zyngier]

On Mon, 31 May 2021 12:33:39 +0200, Andrew Jones wrote:
> v3:
>  - Took Ricardo's suggestions in order to avoid needing to update
>    prepare_vcpu_init, finalize_vcpu, and check_supported when adding
>    new register sublists by better associating the sublists with their
>    vcpu feature bits and caps [Ricardo]
>  - We now dynamically generate the vcpu config name by creating them
>    from its sublist names [drew]
> 
> [...]

Applied to next, thanks!

[1/5] KVM: arm64: selftests: get-reg-list: Introduce vcpu configs
      commit: 2f9ace5d4557f8ceea07969d6214c320f5e50c0c
[2/5] KVM: arm64: selftests: get-reg-list: Prepare to run multiple configs at once
      commit: 94e9223c06bece9165a36f0f56bac3552a45cbfc
[3/5] KVM: arm64: selftests: get-reg-list: Provide config selection option
      commit: f3032fcc9cf065733ce9a50057aaeffd6c464e2e
[4/5] KVM: arm64: selftests: get-reg-list: Remove get-reg-list-sve
      commit: 32edd2290889d0cd0751dd11853e5a368188066d
[5/5] KVM: arm64: selftests: get-reg-list: Split base and pmu registers
      commit: 313673bad871750c0c829def53d037868af75b67

I have also queued an additional patch adding the arm64 selftests
to the list of KVM/arm64 files:

https://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git/commit/?h=next&id=b356a831088730a3ef36848cd9f2d62dcac392bf

Cheers,

	M.
-- 
Without deviation from the norm, progress is not possible.