RISCV: Add KVM_GET_REG_LIST API

PATCH Superseded Haibo Xu 3 participants 22 messages Jul 1, 2023 – Jul 4, 2023

Subsystems (6)

Documentation KVM Kselftest RISC-V +2 more subsystems

lore

Share & tools

lore

Haibo Xu

author

2023-07-01 13:37 UTC #0 lore raw

v5 cover RISCV: Add KVM_GET_REG_LIST API

KVM_GET_REG_LIST will dump all register IDs that are available to KVM_GET/SET_ONE_REG and It's very useful to identify some platform regression issue during VM migration.

Patch 1-7 re-structured the get-reg-list test in aarch64 to make some of the code as common test framework that can be shared by riscv.

Patch 8 move reject_set check logic to a function so as to check for different errno for different registers.
Patch 9 move finalize_vcpu back to run_test so that riscv can implement its specific operation.
Patch 10 change to do the get/set operation only on present-blessed list. Patch 11 add the skip_set facilities so that riscv can skip set operation on some registers.
Patch 12 enabled the KVM_GET_REG_LIST API in riscv.
patch 13 added the corresponding kselftest for checking possible register regressions.

The get-reg-list kvm selftest was ported from aarch64 and tested with Linux v6.4 on a Qemu riscv64 virt machine.

---
Changed since v4:
* Rebase to v6.4
* Address Andrew's suggestions and comments:
Added skip_set concept
Updated errno check logic
Modified finalize_vcpu as weak function

Andrew Jones (7):
KVM: arm64: selftests: Replace str_with_index with strdup_printf
KVM: arm64: selftests: Drop SVE cap check in print_reg
KVM: arm64: selftests: Remove print_reg's dependency on vcpu_config
KVM: arm64: selftests: Rename vcpu_config and add to kvm_util.h
KVM: arm64: selftests: Delete core_reg_fixup
KVM: arm64: selftests: Split get-reg-list test code
KVM: arm64: selftests: Finish generalizing get-reg-list

Haibo Xu (6):
KVM: arm64: selftests: Move reject_set check logic to a function
KVM: arm64: selftests: Move finalize_vcpu back to run_test
KVM: selftests: Only do get/set tests on present blessed list
KVM: selftests: Add skip_set facility to get_reg_list test
KVM: riscv: Add KVM_GET_REG_LIST API support
KVM: riscv: selftests: Add get-reg-list test

 Documentation/virt/kvm/api.rst                |   2 +-
 arch/riscv/kvm/vcpu.c                         | 375 +++++++++
 tools/testing/selftests/kvm/Makefile          |  11 +-
 .../selftests/kvm/aarch64/get-reg-list.c      | 544 ++----------
 tools/testing/selftests/kvm/get-reg-list.c    | 395 +++++++++
 .../selftests/kvm/include/kvm_util_base.h     |  21 +
 .../selftests/kvm/include/riscv/processor.h   |   3 +
 .../testing/selftests/kvm/include/test_util.h |   2 +
 tools/testing/selftests/kvm/lib/test_util.c   |  15 +
 .../selftests/kvm/riscv/get-reg-list.c        | 780 ++++++++++++++++++
 10 files changed, 1655 insertions(+), 493 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/get-reg-list.c
 create mode 100644 tools/testing/selftests/kvm/riscv/get-reg-list.c

Signature · 1 line

2.34.1

Haibo Xu

author

2023-07-01 13:37 UTC #1 lore raw

PATCH v5 1/13 KVM: arm64: selftests: Replace str_with_index with strdup_printf

From: Andrew Jones <[email protected]>

The original author of aarch64/get-reg-list.c (me) was wearing tunnel vision goggles when implementing str_with_index(). There's no reason to have such a special case string function. Instead, take inspiration from glib and implement strdup_printf. The implementation builds on vasprintf() which requires _GNU_SOURCE, but we require _GNU_SOURCE in most files already.

Signed-off-by Andrew Jones <[email protected]>

Signed-off-by Haibo Xu <[email protected]>

---
 .../selftests/kvm/aarch64/get-reg-list.c      | 23 ++++---------------
 .../testing/selftests/kvm/include/test_util.h |  2 ++
 tools/testing/selftests/kvm/lib/test_util.c   | 15 ++++++++++++
 3 files changed, 22 insertions(+), 18 deletions(-)

tools/testing/selftests/kvm/aarch64/get-reg-list.c +5 ~~-18~~

index d4e1f4af29d6..c152523a5ed4 100644
@@ -132,19 +132,6 @@ static bool find_reg(__u64 regs[], __u64 nr_regs, __u64 reg)
 	return false;
 }
 
-static const char *str_with_index(const char *template, __u64 index)
-{
-	char *str, *p;
-	int n;
-
-	str = strdup(template);
-	p = strstr(str, "##");
-	n = sprintf(p, "%lld", index);
-	strcat(p + n, strstr(template, "##") + 2);
-
-	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

@@ -163,7 +150,7 @@ static const char *core_id_to_str(struct vcpu_config *c, __u64 id)
 	     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, "%s: Unexpected regs.regs index: %lld", config_name(c), idx);
-		return str_with_index("KVM_REG_ARM_CORE_REG(regs.regs[##])", idx);
+		return strdup_printf("KVM_REG_ARM_CORE_REG(regs.regs[%lld])", idx);
 	case KVM_REG_ARM_CORE_REG(regs.sp):
 		return "KVM_REG_ARM_CORE_REG(regs.sp)";
 	case KVM_REG_ARM_CORE_REG(regs.pc):

@@ -178,12 +165,12 @@ static const char *core_id_to_str(struct vcpu_config *c, __u64 id)
 	     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, "%s: Unexpected spsr index: %lld", config_name(c), idx);
-		return str_with_index("KVM_REG_ARM_CORE_REG(spsr[##])", idx);
+		return strdup_printf("KVM_REG_ARM_CORE_REG(spsr[%lld])", 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, "%s: Unexpected fp_regs.vregs index: %lld", config_name(c), idx);
-		return str_with_index("KVM_REG_ARM_CORE_REG(fp_regs.vregs[##])", idx);
+		return strdup_printf("KVM_REG_ARM_CORE_REG(fp_regs.vregs[%lld])", idx);
 	case KVM_REG_ARM_CORE_REG(fp_regs.fpsr):
 		return "KVM_REG_ARM_CORE_REG(fp_regs.fpsr)";
 	case KVM_REG_ARM_CORE_REG(fp_regs.fpcr):

@@ -212,13 +199,13 @@ static const char *sve_id_to_str(struct vcpu_config *c, __u64 id)
 		n = (id >> 5) & (KVM_ARM64_SVE_NUM_ZREGS - 1);
 		TEST_ASSERT(id == KVM_REG_ARM64_SVE_ZREG(n, 0),
 			    "%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);
+		return strdup_printf("KVM_REG_ARM64_SVE_ZREG(%lld, 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),
 			    "%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);
+		return strdup_printf("KVM_REG_ARM64_SVE_PREG(%lld, 0)", n);
 	case KVM_REG_ARM64_SVE_FFR_BASE:
 		TEST_ASSERT(id == KVM_REG_ARM64_SVE_FFR(0),
 			    "%s: Unexpected bits set in SVE FFR id: 0x%llx", config_name(c), id);

tools/testing/selftests/kvm/include/test_util.h +2 -0

index a6e9f215ce70..7e0182f837b5 100644
@@ -186,4 +186,6 @@ static inline uint32_t atoi_non_negative(const char *name, const char *num_str)
 	return num;
 }
 
+char *strdup_printf(const char *fmt, ...) __attribute__((format(printf, 1, 2), nonnull(1)));
+
 #endif /* SELFTEST_KVM_TEST_UTIL_H */

tools/testing/selftests/kvm/lib/test_util.c +15 -0

index b772193f6c18..3e36019eeb4a 100644
@@ -5,6 +5,9 @@
  * Copyright (C) 2020, Google LLC.
  */
 
+#define _GNU_SOURCE
+#include <stdio.h>
+#include <stdarg.h>
 #include <assert.h>
 #include <ctype.h>
 #include <limits.h>

@@ -377,3 +380,15 @@ int atoi_paranoid(const char *num_str)
 
 	return num;
 }
+
+char *strdup_printf(const char *fmt, ...)
+{
+	va_list ap;
+	char *str;
+
+	va_start(ap, fmt);
+	vasprintf(&str, fmt, ap);
+	va_end(ap);
+
+	return str;
+}

Signature · 1 line

2.34.1

Haibo Xu

author

2023-07-01 13:37 UTC #2 lore raw

PATCH v5 2/13 KVM: arm64: selftests: Drop SVE cap check in print_reg

In reply to Haibo Xu

From: Andrew Jones <[email protected]>

The check doesn't prove much anyway, as the reg lists could be messed up too. Just drop the check to simplify making print_reg more independent.

Signed-off-by Andrew Jones <[email protected]>

Signed-off-by Haibo Xu <[email protected]>

---
 .../testing/selftests/kvm/aarch64/get-reg-list.c  | 15 +--------------
 1 file changed, 1 insertion(+), 14 deletions(-)

tools/testing/selftests/kvm/aarch64/get-reg-list.c +1 ~~-14~~

index c152523a5ed4..915272c342f9 100644
@@ -100,16 +100,6 @@ static const char *config_name(struct vcpu_config *c)
 	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)
 {
 	/*

@@ -287,10 +277,7 @@ static void print_reg(struct vcpu_config *c, __u64 id)
 		printf("\tKVM_REG_ARM_FW_FEAT_BMAP_REG(%lld),\n", id & 0xffff);
 		break;
 	case KVM_REG_ARM64_SVE:
-		if (has_cap(c, KVM_CAP_ARM_SVE))
-			printf("\t%s,\n", sve_id_to_str(c, id));
-		else
-			TEST_FAIL("%s: KVM_REG_ARM64_SVE is an unexpected coproc type in reg id: 0x%llx", config_name(c), id);
+		printf("\t%s,\n", sve_id_to_str(c, id));
 		break;
 	default:
 		TEST_FAIL("%s: Unexpected coproc type: 0x%llx in reg id: 0x%llx",

Signature · 1 line

2.34.1

Haibo Xu

author

2023-07-01 13:37 UTC #3 lore raw

PATCH v5 3/13 KVM: arm64: selftests: Remove print_reg's dependency on vcpu_config

In reply to Haibo Xu

From: Andrew Jones <[email protected]>

print_reg() and its helpers only use the vcpu_config pointer for config_name(). So just pass the config name in instead, which is used as a prefix in asserts. print_reg() can now be compiled independently of config_name().

Signed-off-by Andrew Jones <[email protected]>

Signed-off-by Haibo Xu <[email protected]>

---
 .../selftests/kvm/aarch64/get-reg-list.c      | 52 +++++++++----------
 1 file changed, 26 insertions(+), 26 deletions(-)

tools/testing/selftests/kvm/aarch64/get-reg-list.c +26 ~~-26~~

index 915272c342f9..424285d39965 100644
@@ -128,7 +128,7 @@ static bool find_reg(__u64 regs[], __u64 nr_regs, __u64 reg)
 #define CORE_SPSR_XX_NR_WORDS	2
 #define CORE_FPREGS_XX_NR_WORDS	4
 
-static const char *core_id_to_str(struct vcpu_config *c, __u64 id)
+static const char *core_id_to_str(const char *prefix, __u64 id)
 {
 	__u64 core_off = id & ~REG_MASK, idx;

@@ -139,7 +139,7 @@ static const char *core_id_to_str(struct vcpu_config *c, __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, "%s: Unexpected regs.regs index: %lld", config_name(c), idx);
+		TEST_ASSERT(idx < 31, "%s: Unexpected regs.regs index: %lld", prefix, idx);
 		return strdup_printf("KVM_REG_ARM_CORE_REG(regs.regs[%lld])", idx);
 	case KVM_REG_ARM_CORE_REG(regs.sp):
 		return "KVM_REG_ARM_CORE_REG(regs.sp)";

@@ -154,12 +154,12 @@ static const char *core_id_to_str(struct vcpu_config *c, __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, "%s: Unexpected spsr index: %lld", config_name(c), idx);
+		TEST_ASSERT(idx < KVM_NR_SPSR, "%s: Unexpected spsr index: %lld", prefix, idx);
 		return strdup_printf("KVM_REG_ARM_CORE_REG(spsr[%lld])", 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, "%s: Unexpected fp_regs.vregs index: %lld", config_name(c), idx);
+		TEST_ASSERT(idx < 32, "%s: Unexpected fp_regs.vregs index: %lld", prefix, idx);
 		return strdup_printf("KVM_REG_ARM_CORE_REG(fp_regs.vregs[%lld])", idx);
 	case KVM_REG_ARM_CORE_REG(fp_regs.fpsr):
 		return "KVM_REG_ARM_CORE_REG(fp_regs.fpsr)";

@@ -167,11 +167,11 @@ static const char *core_id_to_str(struct vcpu_config *c, __u64 id)
 		return "KVM_REG_ARM_CORE_REG(fp_regs.fpcr)";
 	}
 
-	TEST_FAIL("%s: Unknown core reg id: 0x%llx", config_name(c), id);
+	TEST_FAIL("%s: Unknown core reg id: 0x%llx", prefix, id);
 	return NULL;
 }
 
-static const char *sve_id_to_str(struct vcpu_config *c, __u64 id)
+static const char *sve_id_to_str(const char *prefix, __u64 id)
 {
 	__u64 sve_off, n, i;

@@ -181,37 +181,37 @@ static const char *sve_id_to_str(struct vcpu_config *c, __u64 id)
 	sve_off = id & ~(REG_MASK | ((1ULL << 5) - 1));
 	i = id & (KVM_ARM64_SVE_MAX_SLICES - 1);
 
-	TEST_ASSERT(i == 0, "%s: Currently we don't expect slice > 0, reg id 0x%llx", config_name(c), id);
+	TEST_ASSERT(i == 0, "%s: Currently we don't expect slice > 0, reg id 0x%llx", prefix, 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),
-			    "%s: Unexpected bits set in SVE ZREG id: 0x%llx", config_name(c), id);
+			    "%s: Unexpected bits set in SVE ZREG id: 0x%llx", prefix, id);
 		return strdup_printf("KVM_REG_ARM64_SVE_ZREG(%lld, 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),
-			    "%s: Unexpected bits set in SVE PREG id: 0x%llx", config_name(c), id);
+			    "%s: Unexpected bits set in SVE PREG id: 0x%llx", prefix, id);
 		return strdup_printf("KVM_REG_ARM64_SVE_PREG(%lld, 0)", n);
 	case KVM_REG_ARM64_SVE_FFR_BASE:
 		TEST_ASSERT(id == KVM_REG_ARM64_SVE_FFR(0),
-			    "%s: Unexpected bits set in SVE FFR id: 0x%llx", config_name(c), id);
+			    "%s: Unexpected bits set in SVE FFR id: 0x%llx", prefix, id);
 		return "KVM_REG_ARM64_SVE_FFR(0)";
 	}
 
 	return NULL;
 }
 
-static void print_reg(struct vcpu_config *c, __u64 id)
+static void print_reg(const char *prefix, __u64 id)
 {
 	unsigned op0, op1, crn, crm, op2;
 	const char *reg_size = NULL;
 
 	TEST_ASSERT((id & KVM_REG_ARCH_MASK) == KVM_REG_ARM64,
-		    "%s: KVM_REG_ARM64 missing in reg id: 0x%llx", config_name(c), id);
+		    "%s: KVM_REG_ARM64 missing in reg id: 0x%llx", prefix, id);
 
 	switch (id & KVM_REG_SIZE_MASK) {
 	case KVM_REG_SIZE_U8:

@@ -243,16 +243,16 @@ static void print_reg(struct vcpu_config *c, __u64 id)
 		break;
 	default:
 		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);
+			  prefix, (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(c, id));
+		printf("\tKVM_REG_ARM64 | %s | KVM_REG_ARM_CORE | %s,\n", reg_size, core_id_to_str(prefix, id));
 		break;
 	case KVM_REG_ARM_DEMUX:
 		TEST_ASSERT(!(id & ~(REG_MASK | KVM_REG_ARM_DEMUX_ID_MASK | KVM_REG_ARM_DEMUX_VAL_MASK)),
-			    "%s: Unexpected bits set in DEMUX reg id: 0x%llx", config_name(c), id);
+			    "%s: Unexpected bits set in DEMUX reg id: 0x%llx", prefix, 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;

@@ -263,25 +263,25 @@ static void print_reg(struct vcpu_config *c, __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),
-			    "%s: Unexpected bits set in SYSREG reg id: 0x%llx", config_name(c), id);
+			    "%s: Unexpected bits set in SYSREG reg id: 0x%llx", prefix, 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),
-			    "%s: Unexpected bits set in FW reg id: 0x%llx", config_name(c), id);
+			    "%s: Unexpected bits set in FW reg id: 0x%llx", prefix, id);
 		printf("\tKVM_REG_ARM_FW_REG(%lld),\n", id & 0xffff);
 		break;
 	case KVM_REG_ARM_FW_FEAT_BMAP:
 		TEST_ASSERT(id == KVM_REG_ARM_FW_FEAT_BMAP_REG(id & 0xffff),
-			    "%s: Unexpected bits set in the bitmap feature FW reg id: 0x%llx", config_name(c), id);
+			    "%s: Unexpected bits set in the bitmap feature FW reg id: 0x%llx", prefix, id);
 		printf("\tKVM_REG_ARM_FW_FEAT_BMAP_REG(%lld),\n", id & 0xffff);
 		break;
 	case KVM_REG_ARM64_SVE:
-		printf("\t%s,\n", sve_id_to_str(c, id));
+		printf("\t%s,\n", sve_id_to_str(prefix, id));
 		break;
 	default:
 		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);
+			  prefix, (id & KVM_REG_ARM_COPROC_MASK) >> KVM_REG_ARM_COPROC_SHIFT, id);
 	}
 }

@@ -410,7 +410,7 @@ static void run_test(struct vcpu_config *c)
 			__u64 id = reg_list->reg[i];
 			if ((print_list && !filter_reg(id)) ||
 			    (print_filtered && filter_reg(id)))
-				print_reg(c, id);
+				print_reg(config_name(c), id);
 		}
 		putchar('\n');
 		return;

@@ -438,7 +438,7 @@ static void run_test(struct vcpu_config *c)
 		ret = __vcpu_get_reg(vcpu, reg_list->reg[i], &addr);
 		if (ret) {
 			printf("%s: Failed to get ", config_name(c));
-			print_reg(c, reg.id);
+			print_reg(config_name(c), reg.id);
 			putchar('\n');
 			++failed_get;
 		}

@@ -450,7 +450,7 @@ static void run_test(struct vcpu_config *c)
 				ret = __vcpu_ioctl(vcpu, 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);
+					print_reg(config_name(c), reg.id);
 					putchar('\n');
 					++failed_reject;
 				}

@@ -462,7 +462,7 @@ static void run_test(struct vcpu_config *c)
 			ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);
 			if (ret) {
 				printf("%s: Failed to set ", config_name(c));
-				print_reg(c, reg.id);
+				print_reg(config_name(c), reg.id);
 				putchar('\n');
 				++failed_set;
 			}

@@ -500,7 +500,7 @@ static void run_test(struct vcpu_config *c)
 		       "Consider adding them to the blessed reg "
 		       "list with the following lines:\n\n", config_name(c), new_regs);
 		for_each_new_reg(i)
-			print_reg(c, reg_list->reg[i]);
+			print_reg(config_name(c), reg_list->reg[i]);
 		putchar('\n');
 	}

@@ -508,7 +508,7 @@ static void run_test(struct vcpu_config *c)
 		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(c, blessed_reg[i]);
+			print_reg(config_name(c), blessed_reg[i]);
 		putchar('\n');
 	}

Signature · 1 line

2.34.1

Haibo Xu

author

2023-07-01 13:38 UTC #4 lore raw

PATCH v5 4/13 KVM: arm64: selftests: Rename vcpu_config and add to kvm_util.h

In reply to Haibo Xu

From: Andrew Jones <[email protected]>

Rename vcpu_config to vcpu_reg_list to be more specific and add it to kvm_util.h. While it may not get used outside get-reg-list tests, exporting it doesn't hurt, as long as it has a unique enough name. This is a step in the direction of sharing most of the get- reg-list test code between architectures.

Signed-off-by Andrew Jones <[email protected]>

Signed-off-by Haibo Xu <[email protected]>

---
 .../selftests/kvm/aarch64/get-reg-list.c      | 60 +++++++------------
 .../selftests/kvm/include/kvm_util_base.h     | 16 +++++
 2 files changed, 38 insertions(+), 38 deletions(-)

tools/testing/selftests/kvm/aarch64/get-reg-list.c +22 ~~-38~~

index 424285d39965..aae2056379f7 100644
@@ -37,23 +37,7 @@
 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 *vcpu_configs[];
+static struct vcpu_reg_list *vcpu_configs[];
 static int vcpu_configs_n;
 
 #define for_each_sublist(c, s)							\

@@ -74,9 +58,9 @@ static int vcpu_configs_n;
 	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)
+static const char *config_name(struct vcpu_reg_list *c)
 {
-	struct reg_sublist *s;
+	struct vcpu_reg_sublist *s;
 	int len = 0;
 
 	if (c->name)

@@ -342,18 +326,18 @@ static void core_reg_fixup(void)
 	reg_list = tmp;
 }
 
-static void prepare_vcpu_init(struct vcpu_config *c, struct kvm_vcpu_init *init)
+static void prepare_vcpu_init(struct vcpu_reg_list *c, struct kvm_vcpu_init *init)
 {
-	struct reg_sublist *s;
+	struct vcpu_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_vcpu *vcpu, struct vcpu_config *c)
+static void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c)
 {
-	struct reg_sublist *s;
+	struct vcpu_reg_sublist *s;
 	int feature;
 
 	for_each_sublist(c, s) {

@@ -364,9 +348,9 @@ static void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_config *c)
 	}
 }
 
-static void check_supported(struct vcpu_config *c)
+static void check_supported(struct vcpu_reg_list *c)
 {
-	struct reg_sublist *s;
+	struct vcpu_reg_sublist *s;
 
 	for_each_sublist(c, s) {
 		if (!s->capability)

@@ -382,14 +366,14 @@ static bool print_list;
 static bool print_filtered;
 static bool fixup_core_regs;
 
-static void run_test(struct vcpu_config *c)
+static void run_test(struct vcpu_reg_list *c)
 {
 	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;
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
-	struct reg_sublist *s;
+	struct vcpu_reg_sublist *s;
 
 	check_supported(c);

@@ -526,7 +510,7 @@ static void run_test(struct vcpu_config *c)
 
 static void help(void)
 {
-	struct vcpu_config *c;
+	struct vcpu_reg_list *c;
 	int i;
 
 	printf(

@@ -550,9 +534,9 @@ static void help(void)
 	);
 }
 
-static struct vcpu_config *parse_config(const char *config)
+static struct vcpu_reg_list *parse_config(const char *config)
 {
-	struct vcpu_config *c;
+	struct vcpu_reg_list *c;
 	int i;
 
 	if (config[8] != '=')

@@ -572,7 +556,7 @@ static struct vcpu_config *parse_config(const char *config)
 
 int main(int ac, char **av)
 {
-	struct vcpu_config *c, *sel = NULL;
+	struct vcpu_reg_list *c, *sel = NULL;
 	int i, ret = 0;
 	pid_t pid;

@@ -1053,14 +1037,14 @@ static __u64 pauth_generic_regs[] = {
 		.regs_n		= ARRAY_SIZE(pauth_generic_regs),	\
 	}
 
-static struct vcpu_config vregs_config = {
+static struct vcpu_reg_list vregs_config = {
 	.sublists = {
 	BASE_SUBLIST,
 	VREGS_SUBLIST,
 	{0},
 	},
 };
-static struct vcpu_config vregs_pmu_config = {
+static struct vcpu_reg_list vregs_pmu_config = {
 	.sublists = {
 	BASE_SUBLIST,
 	VREGS_SUBLIST,

@@ -1068,14 +1052,14 @@ static struct vcpu_config vregs_pmu_config = {
 	{0},
 	},
 };
-static struct vcpu_config sve_config = {
+static struct vcpu_reg_list sve_config = {
 	.sublists = {
 	BASE_SUBLIST,
 	SVE_SUBLIST,
 	{0},
 	},
 };
-static struct vcpu_config sve_pmu_config = {
+static struct vcpu_reg_list sve_pmu_config = {
 	.sublists = {
 	BASE_SUBLIST,
 	SVE_SUBLIST,

@@ -1083,7 +1067,7 @@ static struct vcpu_config sve_pmu_config = {
 	{0},
 	},
 };
-static struct vcpu_config pauth_config = {
+static struct vcpu_reg_list pauth_config = {
 	.sublists = {
 	BASE_SUBLIST,
 	VREGS_SUBLIST,

@@ -1091,7 +1075,7 @@ static struct vcpu_config pauth_config = {
 	{0},
 	},
 };
-static struct vcpu_config pauth_pmu_config = {
+static struct vcpu_reg_list pauth_pmu_config = {
 	.sublists = {
 	BASE_SUBLIST,
 	VREGS_SUBLIST,

@@ -1101,7 +1085,7 @@ static struct vcpu_config pauth_pmu_config = {
 	},
 };
 
-static struct vcpu_config *vcpu_configs[] = {
+static struct vcpu_reg_list *vcpu_configs[] = {
 	&vregs_config,
 	&vregs_pmu_config,
 	&sve_config,

tools/testing/selftests/kvm/include/kvm_util_base.h +16 -0

index a089c356f354..ac4aaa21deee 100644
@@ -15,6 +15,7 @@
 #include <linux/kernel.h>
 #include <linux/kvm.h>
 #include "linux/rbtree.h"
+#include <linux/types.h>
 
 #include <asm/atomic.h>

@@ -124,6 +125,21 @@ struct kvm_vm {
 	uint32_t memslots[NR_MEM_REGIONS];
 };
 
+struct vcpu_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_reg_list {
+	char *name;
+	struct vcpu_reg_sublist sublists[];
+};
 
 #define kvm_for_each_vcpu(vm, i, vcpu)			\
 	for ((i) = 0; (i) <= (vm)->last_vcpu_id; (i)++)	\

Signature · 1 line

2.34.1

Haibo Xu

author

2023-07-01 13:38 UTC #5 lore raw

PATCH v5 5/13 KVM: arm64: selftests: Delete core_reg_fixup

In reply to Haibo Xu

From: Andrew Jones <[email protected]>

core_reg_fixup() complicates sharing the get-reg-list test with other architectures. Rather than work at keeping it, with plenty of #ifdeffery, just delete it, as it's unlikely to test a kernel based on anything older than v5.2 with the get-reg-list test, which is a test meant to check for regressions in new kernels. (And, an older version of the test can still be used for older kernels if necessary.)

Signed-off-by Andrew Jones <[email protected]>

Signed-off-by Haibo Xu <[email protected]>

---
 .../selftests/kvm/aarch64/get-reg-list.c      | 83 +++----------------
 1 file changed, 10 insertions(+), 73 deletions(-)

tools/testing/selftests/kvm/aarch64/get-reg-list.c +10 ~~-73~~

index aae2056379f7..c8b44389d2ee 100644
@@ -17,12 +17,10 @@
  * by running the test with the --list command line argument.
  *
  * Note, the blessed list should be created from the oldest possible
- * kernel. We can't go older than v4.15, though, because that's the first
- * release to expose the ID system registers in KVM_GET_REG_LIST, see
- * commit 93390c0a1b20 ("arm64: KVM: Hide unsupported AArch64 CPU features
- * from guests"). Also, one must use the --core-reg-fixup command line
- * option when running on an older kernel that doesn't include df205b5c6328
- * ("KVM: arm64: Filter out invalid core register IDs in KVM_GET_REG_LIST")
+ * kernel. We can't go older than v5.2, though, because that's the first
+ * release which includes df205b5c6328 ("KVM: arm64: Filter out invalid
+ * core register IDs in KVM_GET_REG_LIST"). Without that commit the core
+ * registers won't match expectations.
  */
 #include <stdio.h>
 #include <stdlib.h>

@@ -269,63 +267,6 @@ static void print_reg(const char *prefix, __u64 id)
 	}
 }
 
-/*
- * Older kernels listed each 32-bit word of CORE registers separately.
- * For 64 and 128-bit registers we need to ignore the extra words. We
- * also need to fixup the sizes, because the older kernels stated all
- * registers were 64-bit, even when they weren't.
- */
-static void core_reg_fixup(void)
-{
-	struct kvm_reg_list *tmp;
-	__u64 id, core_off;
-	int i;
-
-	tmp = calloc(1, sizeof(*tmp) + reg_list->n * sizeof(__u64));
-
-	for (i = 0; i < reg_list->n; ++i) {
-		id = reg_list->reg[i];
-
-		if ((id & KVM_REG_ARM_COPROC_MASK) != KVM_REG_ARM_CORE) {
-			tmp->reg[tmp->n++] = id;
-			continue;
-		}
-
-		core_off = id & ~REG_MASK;
-
-		switch (core_off) {
-		case 0x52: case 0xd2: case 0xd6:
-			/*
-			 * These offsets are pointing at padding.
-			 * We need to ignore them too.
-			 */
-			continue;
-		case KVM_REG_ARM_CORE_REG(fp_regs.vregs[0]) ...
-		     KVM_REG_ARM_CORE_REG(fp_regs.vregs[31]):
-			if (core_off & 3)
-				continue;
-			id &= ~KVM_REG_SIZE_MASK;
-			id |= KVM_REG_SIZE_U128;
-			tmp->reg[tmp->n++] = id;
-			continue;
-		case KVM_REG_ARM_CORE_REG(fp_regs.fpsr):
-		case KVM_REG_ARM_CORE_REG(fp_regs.fpcr):
-			id &= ~KVM_REG_SIZE_MASK;
-			id |= KVM_REG_SIZE_U32;
-			tmp->reg[tmp->n++] = id;
-			continue;
-		default:
-			if (core_off & 1)
-				continue;
-			tmp->reg[tmp->n++] = id;
-			break;
-		}
-	}
-
-	free(reg_list);
-	reg_list = tmp;
-}
-
 static void prepare_vcpu_init(struct vcpu_reg_list *c, struct kvm_vcpu_init *init)
 {
 	struct vcpu_reg_sublist *s;

@@ -364,7 +305,6 @@ static void check_supported(struct vcpu_reg_list *c)
 
 static bool print_list;
 static bool print_filtered;
-static bool fixup_core_regs;
 
 static void run_test(struct vcpu_reg_list *c)
 {

@@ -385,9 +325,6 @@ static void run_test(struct vcpu_reg_list *c)
 
 	reg_list = vcpu_get_reg_list(vcpu);
 
-	if (fixup_core_regs)
-		core_reg_fixup();
-
 	if (print_list || print_filtered) {
 		putchar('\n');
 		for_each_reg(i) {

@@ -515,7 +452,7 @@ static void help(void)
 
 	printf(
 	"\n"
-	"usage: get-reg-list [--config=<selection>] [--list] [--list-filtered] [--core-reg-fixup]\n\n"
+	"usage: get-reg-list [--config=<selection>] [--list] [--list-filtered]\n\n"
 	" --config=<selection>        Used to select a specific vcpu configuration for the test/listing\n"
 	"                             '<selection>' may be\n");

@@ -529,7 +466,6 @@ static void help(void)
 	"\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"
 	);
 }

@@ -561,9 +497,7 @@ int main(int ac, char **av)
 	pid_t pid;
 
 	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)
+		if (strncmp(av[i], "--config", 8) == 0)
 			sel = parse_config(av[i]);
 		else if (strcmp(av[i], "--list") == 0)
 			print_list = true;

@@ -606,8 +540,11 @@ int main(int ac, char **av)
 }
 
 /*
- * The current blessed list was primed with the output of kernel version
+ * The original blessed list was primed with the output of kernel version
  * v4.15 with --core-reg-fixup and then later updated with new registers.
+ * (The --core-reg-fixup option and it's fixup function have been removed
+ * from the test, as it's unlikely to use this type of test on a kernel
+ * older than v5.2.)
  *
  * The blessed list is up to date with kernel version v6.4 (or so we hope)
  */

Signature · 1 line

2.34.1

Haibo Xu

author

2023-07-01 13:38 UTC #6 lore raw

PATCH v5 6/13 KVM: arm64: selftests: Split get-reg-list test code

In reply to Haibo Xu

From: Andrew Jones <[email protected]>

Split the arch-neutral test code out of aarch64/get-reg-list.c into get-reg-list.c. To do this we invent a new make variable $(SPLIT_TESTS) which expects common parts to be in the KVM selftests root and the counterparts to have the same name, but be in $(ARCH_DIR).

There's still some work to be done to de-aarch64 the common get-reg-list.c, but we leave that to the next patch to avoid modifying too much code while moving it.

Signed-off-by Andrew Jones <[email protected]>

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/Makefile          |  10 +-
 .../selftests/kvm/aarch64/get-reg-list.c      | 361 +----------------
 tools/testing/selftests/kvm/get-reg-list.c    | 371 ++++++++++++++++++
 3 files changed, 385 insertions(+), 357 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/get-reg-list.c

tools/testing/selftests/kvm/Makefile +8 -2

index 4761b768b773..d90cad19c9ee 100644
@@ -139,7 +139,6 @@ TEST_GEN_PROGS_EXTENDED_x86_64 += x86_64/nx_huge_pages_test
 TEST_GEN_PROGS_aarch64 += aarch64/aarch32_id_regs
 TEST_GEN_PROGS_aarch64 += aarch64/arch_timer
 TEST_GEN_PROGS_aarch64 += aarch64/debug-exceptions
-TEST_GEN_PROGS_aarch64 += aarch64/get-reg-list
 TEST_GEN_PROGS_aarch64 += aarch64/hypercalls
 TEST_GEN_PROGS_aarch64 += aarch64/page_fault_test
 TEST_GEN_PROGS_aarch64 += aarch64/psci_test

@@ -151,6 +150,7 @@ TEST_GEN_PROGS_aarch64 += access_tracking_perf_test
 TEST_GEN_PROGS_aarch64 += demand_paging_test
 TEST_GEN_PROGS_aarch64 += dirty_log_test
 TEST_GEN_PROGS_aarch64 += dirty_log_perf_test
+TEST_GEN_PROGS_aarch64 += get-reg-list
 TEST_GEN_PROGS_aarch64 += kvm_create_max_vcpus
 TEST_GEN_PROGS_aarch64 += kvm_page_table_test
 TEST_GEN_PROGS_aarch64 += memslot_modification_stress_test

@@ -179,6 +179,8 @@ TEST_GEN_PROGS_riscv += kvm_page_table_test
 TEST_GEN_PROGS_riscv += set_memory_region_test
 TEST_GEN_PROGS_riscv += kvm_binary_stats_test
 
+SPLIT_TESTS += get-reg-list
+
 TEST_PROGS += $(TEST_PROGS_$(ARCH_DIR))
 TEST_GEN_PROGS += $(TEST_GEN_PROGS_$(ARCH_DIR))
 TEST_GEN_PROGS_EXTENDED += $(TEST_GEN_PROGS_EXTENDED_$(ARCH_DIR))

@@ -224,8 +226,12 @@ LIBKVM_C_OBJ := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBKVM_C))
 LIBKVM_S_OBJ := $(patsubst %.S, $(OUTPUT)/%.o, $(LIBKVM_S))
 LIBKVM_STRING_OBJ := $(patsubst %.c, $(OUTPUT)/%.o, $(LIBKVM_STRING))
 LIBKVM_OBJS = $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ) $(LIBKVM_STRING_OBJ)
+SPLIT_TESTS_TARGETS := $(patsubst %, $(OUTPUT)/%, $(SPLIT_TESTS))
+SPLIT_TESTS_OBJS := $(patsubst %, $(ARCH_DIR)/%.o, $(SPLIT_TESTS))
+
+EXTRA_CLEAN += $(LIBKVM_OBJS) $(SPLIT_TESTS_OBJS) cscope.*
 
-EXTRA_CLEAN += $(LIBKVM_OBJS) cscope.*
+$(SPLIT_TESTS_TARGETS): $(SPLIT_TESTS_OBJS)
 
 x := $(shell mkdir -p $(sort $(dir $(LIBKVM_C_OBJ) $(LIBKVM_S_OBJ))))
 $(LIBKVM_C_OBJ): $(OUTPUT)/%.o: %.c

tools/testing/selftests/kvm/aarch64/get-reg-list.c +6 ~~-355~~

403 lines not on this page · raw message

tools/testing/selftests/kvm/get-reg-list.c new file +371 -0

371 lines not on this page · raw message

Signature · 1 line

2.34.1

Haibo Xu

author

2023-07-01 13:38 UTC #7 lore raw

PATCH v5 7/13 KVM: arm64: selftests: Finish generalizing get-reg-list

In reply to Haibo Xu

From: Andrew Jones <[email protected]>

Add some unfortunate #ifdeffery to ensure the common get-reg-list.c can be compiled and run with other architectures. The next architecture to support get-reg-list should now only need to provide $(ARCH_DIR)/get-reg-list.c where arch-specific print_reg() and vcpu_configs[] get defined.

Signed-off-by Andrew Jones <[email protected]>

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/get-reg-list.c | 26 +++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

tools/testing/selftests/kvm/get-reg-list.c +21 -5

index 69bb91087081..f6ad7991a812 100644
@@ -98,6 +98,7 @@ void __weak print_reg(const char *prefix, __u64 id)
 	printf("\t0x%llx,\n", id);
 }
 
+#ifdef __aarch64__
 static void prepare_vcpu_init(struct vcpu_reg_list *c, struct kvm_vcpu_init *init)
 {
 	struct vcpu_reg_sublist *s;

@@ -120,6 +121,25 @@ static void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c)
 	}
 }
 
+static struct kvm_vcpu *vcpu_config_get_vcpu(struct vcpu_reg_list *c, struct kvm_vm *vm)
+{
+	struct kvm_vcpu_init init = { .target = -1, };
+	struct kvm_vcpu *vcpu;
+
+	prepare_vcpu_init(c, &init);
+	vcpu = __vm_vcpu_add(vm, 0);
+	aarch64_vcpu_setup(vcpu, &init);
+	finalize_vcpu(vcpu, c);
+
+	return vcpu;
+}
+#else
+static struct kvm_vcpu *vcpu_config_get_vcpu(struct vcpu_reg_list *c, struct kvm_vm *vm)
+{
+	return __vm_vcpu_add(vm, 0);
+}
+#endif
+
 static void check_supported(struct vcpu_reg_list *c)
 {
 	struct vcpu_reg_sublist *s;

@@ -139,7 +159,6 @@ static bool print_filtered;
 
 static void run_test(struct vcpu_reg_list *c)
 {
-	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;
 	struct kvm_vcpu *vcpu;

@@ -149,10 +168,7 @@ static void run_test(struct vcpu_reg_list *c)
 	check_supported(c);
 
 	vm = vm_create_barebones();
-	prepare_vcpu_init(c, &init);
-	vcpu = __vm_vcpu_add(vm, 0);
-	aarch64_vcpu_setup(vcpu, &init);
-	finalize_vcpu(vcpu, c);
+	vcpu = vcpu_config_get_vcpu(c, vm);
 
 	reg_list = vcpu_get_reg_list(vcpu);

Signature · 1 line

2.34.1

Haibo Xu

author

2023-07-01 13:38 UTC #8 lore raw

PATCH v5 8/13 KVM: arm64: selftests: Move reject_set check logic to a function

In reply to Haibo Xu

No functional changes. Just move the reject_set check logic to a function so we can check for specific errno for specific register. This is a preparation for support reject_set in riscv.

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/aarch64/get-reg-list.c | 5 +++++
 tools/testing/selftests/kvm/get-reg-list.c         | 7 ++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

tools/testing/selftests/kvm/aarch64/get-reg-list.c +5 -0

index aaf035c969ec..4aa58f1aebe3 100644
@@ -27,6 +27,11 @@ bool filter_reg(__u64 reg)
 	return false;
 }
 
+bool check_reject_set(int err)
+{
+	return err == EPERM;
+}
+
 #define REG_MASK (KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_ARM_COPROC_MASK)
 
 #define CORE_REGS_XX_NR_WORDS	2

tools/testing/selftests/kvm/get-reg-list.c +6 -1

index f6ad7991a812..79e198968860 100644
@@ -98,6 +98,11 @@ void __weak print_reg(const char *prefix, __u64 id)
 	printf("\t0x%llx,\n", id);
 }
 
+bool __weak check_reject_set(int err)
+{
+	return true;
+}
+
 #ifdef __aarch64__
 static void prepare_vcpu_init(struct vcpu_reg_list *c, struct kvm_vcpu_init *init)
 {

@@ -216,7 +221,7 @@ static void run_test(struct vcpu_reg_list *c)
 			if (s->rejects_set && find_reg(s->rejects_set, s->rejects_set_n, reg.id)) {
 				reject_reg = true;
 				ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);
-				if (ret != -1 || errno != EPERM) {
+				if (ret != -1 || !check_reject_set(errno)) {
 					printf("%s: Failed to reject (ret=%d, errno=%d) ", config_name(c), ret, errno);
 					print_reg(config_name(c), reg.id);
 					putchar('\n');

Signature · 1 line

2.34.1

Andrew Jones

Reviewed-by on v5 8/13

2023-07-01 14:21 UTC #9 lore raw

Re: [PATCH v5 08/13] KVM: arm64: selftests: Move reject_set check logic to a function

Quoted 2 lines · Haibo Xu

No functional changes. Just move the reject_set check logic to a function so we can check for specific errno for specific register.

'for specific register' part isn't true anymore.

Quoted 53 lines

This is a preparation for support reject_set in riscv.

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/aarch64/get-reg-list.c | 5 +++++
 tools/testing/selftests/kvm/get-reg-list.c         | 7 ++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

tools/testing/selftests/kvm/aarch64/get-reg-list.c +5 -0

index aaf035c969ec..4aa58f1aebe3 100644
@@ -27,6 +27,11 @@ bool filter_reg(__u64 reg)
 	return false;
 }
 
+bool check_reject_set(int err)
+{
+	return err == EPERM;
+}
+
 #define REG_MASK (KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_ARM_COPROC_MASK)
 
 #define CORE_REGS_XX_NR_WORDS	2

tools/testing/selftests/kvm/get-reg-list.c +6 -1

index f6ad7991a812..79e198968860 100644
@@ -98,6 +98,11 @@ void __weak print_reg(const char *prefix, __u64 id)
 	printf("\t0x%llx,\n", id);
 }
 
+bool __weak check_reject_set(int err)
+{
+	return true;
+}
+
 #ifdef __aarch64__
 static void prepare_vcpu_init(struct vcpu_reg_list *c, struct kvm_vcpu_init *init)
 {

@@ -216,7 +221,7 @@ static void run_test(struct vcpu_reg_list *c)
 			if (s->rejects_set && find_reg(s->rejects_set, s->rejects_set_n, reg.id)) {
 				reject_reg = true;
 				ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);
-				if (ret != -1 || errno != EPERM) {
+				if (ret != -1 || !check_reject_set(errno)) {
 					printf("%s: Failed to reject (ret=%d, errno=%d) ", config_name(c), ret, errno);
 					print_reg(config_name(c), reg.id);
 					putchar('\n');

Signature · 1 line

2.34.1

Otherwise,

Reviewed-by Andrew Jones <[email protected]>

Haibo Xu

2023-07-01 14:35 UTC #10 lore raw

Re: [PATCH v5 08/13] KVM: arm64: selftests: Move reject_set check logic to a function

Quoted 63 lines · On Sat, Jul 1, 2023 at 10:21?PM Andrew Jones <[email protected]> wrote:

Quoted 2 lines · Haibo Xu

No functional changes. Just move the reject_set check logic to a function so we can check for specific errno for specific register.

'for specific register' part isn't true anymore.

Quoted 51 lines

This is a preparation for support reject_set in riscv.

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/aarch64/get-reg-list.c | 5 +++++
 tools/testing/selftests/kvm/get-reg-list.c         | 7 ++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

tools/testing/selftests/kvm/aarch64/get-reg-list.c +5 -0

index aaf035c969ec..4aa58f1aebe3 100644
@@ -27,6 +27,11 @@ bool filter_reg(__u64 reg)
      return false;
 }
 
+bool check_reject_set(int err)
+{
+     return err == EPERM;
+}
+
 #define REG_MASK (KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_ARM_COPROC_MASK)
 
 #define CORE_REGS_XX_NR_WORDS        2

tools/testing/selftests/kvm/get-reg-list.c +6 -2

index f6ad7991a812..79e198968860 100644
@@ -98,6 +98,11 @@ void __weak print_reg(const char *prefix, __u64 id)
      printf("\t0x%llx,\n", id);
 }
 
+bool __weak check_reject_set(int err)
+{
+     return true;
+}
+
 #ifdef __aarch64__
 static void prepare_vcpu_init(struct vcpu_reg_list *c, struct kvm_vcpu_init *init)
 {

@@ -216,7 +221,7 @@ static void run_test(struct vcpu_reg_list *c)
                      if (s->rejects_set && find_reg(s->rejects_set, s->rejects_set_n, reg.id)) {
                              reject_reg = true;
                              ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);
-                             if (ret != -1 || errno != EPERM) {
+                             if (ret != -1 || !check_reject_set(errno)) {
                                      printf("%s: Failed to reject (ret=%d, errno=%d) ", config_name(c), ret, errno);
                                      print_reg(config_name(c), reg.id);
                                      putchar('\n');
--

Quoted 2 lines

2.34.1

Otherwise,

Quoted 1 line

Reviewed-by Andrew Jones <[email protected]>

Thanks for the review! Will fix it in the next version.

Haibo Xu

author

2023-07-01 13:39 UTC #11 lore raw

PATCH v5 9/13 KVM: arm64: selftests: Move finalize_vcpu back to run_test

In reply to Haibo Xu

No functional changes. Just move the finalize_vcpu call back to run_test and do weak function trick to prepare for the opration in riscv.

Signed-off-by Haibo Xu <[email protected]>

Reviewed-by Andrew Jones <[email protected]>

---
 .../selftests/kvm/aarch64/get-reg-list.c      | 13 +++++++++++
 tools/testing/selftests/kvm/get-reg-list.c    | 22 +++++--------------
 .../selftests/kvm/include/kvm_util_base.h     |  3 +++
 3 files changed, 21 insertions(+), 17 deletions(-)

tools/testing/selftests/kvm/aarch64/get-reg-list.c +13 -0

index 4aa58f1aebe3..6a7f9de21640 100644
@@ -32,6 +32,19 @@ bool check_reject_set(int err)
 	return err == EPERM;
 }
 
+void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c)
+{
+	struct vcpu_reg_sublist *s;
+	int feature;
+
+	for_each_sublist(c, s) {
+		if (s->finalize) {
+			feature = s->feature;
+			vcpu_ioctl(vcpu, KVM_ARM_VCPU_FINALIZE, &feature);
+		}
+	}
+}
+
 #define REG_MASK (KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK | KVM_REG_ARM_COPROC_MASK)
 
 #define CORE_REGS_XX_NR_WORDS	2

tools/testing/selftests/kvm/get-reg-list.c +5 ~~-17~~

index 79e198968860..c61090806007 100644
@@ -34,9 +34,6 @@ static __u64 *blessed_reg, blessed_n;
 extern struct vcpu_reg_list *vcpu_configs[];
 extern int vcpu_configs_n;
 
-#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))

@@ -103,6 +100,10 @@ bool __weak check_reject_set(int err)
 	return true;
 }
 
+void __weak finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c)
+{
+}
+
 #ifdef __aarch64__
 static void prepare_vcpu_init(struct vcpu_reg_list *c, struct kvm_vcpu_init *init)
 {

@@ -113,19 +114,6 @@ static void prepare_vcpu_init(struct vcpu_reg_list *c, struct kvm_vcpu_init *ini
 			init->features[s->feature / 32] |= 1 << (s->feature % 32);
 }
 
-static void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c)
-{
-	struct vcpu_reg_sublist *s;
-	int feature;
-
-	for_each_sublist(c, s) {
-		if (s->finalize) {
-			feature = s->feature;
-			vcpu_ioctl(vcpu, KVM_ARM_VCPU_FINALIZE, &feature);
-		}
-	}
-}
-
 static struct kvm_vcpu *vcpu_config_get_vcpu(struct vcpu_reg_list *c, struct kvm_vm *vm)
 {
 	struct kvm_vcpu_init init = { .target = -1, };

@@ -134,7 +122,6 @@ static struct kvm_vcpu *vcpu_config_get_vcpu(struct vcpu_reg_list *c, struct kvm
 	prepare_vcpu_init(c, &init);
 	vcpu = __vm_vcpu_add(vm, 0);
 	aarch64_vcpu_setup(vcpu, &init);
-	finalize_vcpu(vcpu, c);
 
 	return vcpu;
 }

@@ -174,6 +161,7 @@ static void run_test(struct vcpu_reg_list *c)
 
 	vm = vm_create_barebones();
 	vcpu = vcpu_config_get_vcpu(c, vm);
+	finalize_vcpu(vcpu, c);
 
 	reg_list = vcpu_get_reg_list(vcpu);

tools/testing/selftests/kvm/include/kvm_util_base.h +3 -0

index ac4aaa21deee..e4480049000d 100644
@@ -141,6 +141,9 @@ struct vcpu_reg_list {
 	struct vcpu_reg_sublist sublists[];
 };
 
+#define for_each_sublist(c, s)		\
+	for ((s) = &(c)->sublists[0]; (s)->regs; ++(s))
+
 #define kvm_for_each_vcpu(vm, i, vcpu)			\
 	for ((i) = 0; (i) <= (vm)->last_vcpu_id; (i)++)	\
 		if (!((vcpu) = vm->vcpus[i]))		\

Signature · 1 line

2.34.1

Haibo Xu

author

2023-07-01 13:39 UTC #12 lore raw

PATCH v5 10/13 KVM: selftests: Only do get/set tests on present blessed list

In reply to Haibo Xu

Only do the get/set tests on present and blessed registers since we don't know the capabilities of any new ones.

Signed-off-by Haibo Xu <[email protected]>

Reviewed-by Andrew Jones <[email protected]>

---
 tools/testing/selftests/kvm/get-reg-list.c | 29 ++++++++++++++--------
 1 file changed, 18 insertions(+), 11 deletions(-)

tools/testing/selftests/kvm/get-reg-list.c +18 ~~-11~~

index c61090806007..74fb6f6fdd09 100644
@@ -49,6 +49,10 @@ extern int vcpu_configs_n;
 	for_each_reg_filtered(i)						\
 		if (!find_reg(blessed_reg, blessed_n, reg_list->reg[i]))
 
+#define for_each_present_blessed_reg(i)			\
+	for ((i) = 0; (i) < blessed_n; ++(i))		\
+		if (find_reg(reg_list->reg, reg_list->n, blessed_reg[i]))
+
 static const char *config_name(struct vcpu_reg_list *c)
 {
 	struct vcpu_reg_sublist *s;

@@ -177,6 +181,16 @@ static void run_test(struct vcpu_reg_list *c)
 		return;
 	}
 
+	for_each_sublist(c, s)
+		blessed_n += s->regs_n;
+	blessed_reg = calloc(blessed_n, sizeof(__u64));
+
+	n = 0;
+	for_each_sublist(c, s) {
+		for (i = 0; i < s->regs_n; ++i)
+			blessed_reg[n++] = s->regs[i];
+	}
+
 	/*
 	 * 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

@@ -186,8 +200,11 @@ static void run_test(struct vcpu_reg_list *c)
 	 * be written need to have the other values tested, then we should
 	 * create a new set of tests for those in a new independent test
 	 * executable.
+	 *
+	 * Only do the get/set tests on present, blessed list registers,
+	 * since we don't know the capabilities of any new registers.
 	 */
-	for_each_reg(i) {
+	for_each_present_blessed_reg(i) {
 		uint8_t addr[2048 / 8];
 		struct kvm_one_reg reg = {
 			.id = reg_list->reg[i],

@@ -230,16 +247,6 @@ static void run_test(struct vcpu_reg_list *c)
 		}
 	}
 
-	for_each_sublist(c, s)
-		blessed_n += s->regs_n;
-	blessed_reg = calloc(blessed_n, sizeof(__u64));
-
-	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;

Signature · 1 line

2.34.1

Andrew Jones

2023-07-03 08:18 UTC #13 lore raw

Re: [PATCH v5 10/13] KVM: selftests: Only do get/set tests on present blessed list

Quoted 15 lines · Haibo Xu

Only do the get/set tests on present and blessed registers since we don't know the capabilities of any new ones.
Suggested-by Andrew Jones <[email protected]>
Signed-off-by Haibo Xu <[email protected]>
Reviewed-by Andrew Jones <[email protected]>
---
 tools/testing/selftests/kvm/get-reg-list.c | 29 ++++++++++++++--------
 1 file changed, 18 insertions(+), 11 deletions(-)
tools/testing/selftests/kvm/get-reg-list.c +0 -0
index c61090806007..74fb6f6fdd09 100644

tools/testing/selftests/kvm/get-reg-list.c extern int vcpu_configs_n; · line 54

Quoted 6 lines

tools/testing/selftests/kvm/get-reg-list.c +3 -0

@@ -49,6 +49,10 @@ extern int vcpu_configs_n;
 	for_each_reg_filtered(i)						\
 		if (!find_reg(blessed_reg, blessed_n, reg_list->reg[i]))
 
+#define for_each_present_blessed_reg(i)			\
+	for ((i) = 0; (i) < blessed_n; ++(i))		\
+		if (find_reg(reg_list->reg, reg_list->n, blessed_reg[i]))

I just realized this is backwards. We need 'i' to index reg_list->reg in the body of the loop. That means we need to write this as

#define for_each_present_blessed_reg(i)                                         \
        for_each_reg(i)                                                         \
                if (find_reg(blessed_reg, blessed_n, reg_list->reg[i]))

(Which, in hindsight, makes sense since we're replacing a for_each_reg() loop.)

Thanks,
drew

Haibo Xu

2023-07-04 01:09 UTC #14 lore raw

Re: [PATCH v5 10/13] KVM: selftests: Only do get/set tests on present blessed list

Quoted 27 lines · On Mon, Jul 3, 2023 at 4:18?PM Andrew Jones <[email protected]> wrote:

Quoted 15 lines · Haibo Xu
Only do the get/set tests on present and blessed registers since we don't know the capabilities of any new ones.
Suggested-by Andrew Jones <[email protected]>
Signed-off-by Haibo Xu <[email protected]>
Reviewed-by Andrew Jones <[email protected]>
---
 tools/testing/selftests/kvm/get-reg-list.c | 29 ++++++++++++++--------
 1 file changed, 18 insertions(+), 11 deletions(-)
tools/testing/selftests/kvm/get-reg-list.c +0 -0
index c61090806007..74fb6f6fdd09 100644
tools/testing/selftests/kvm/get-reg-list.c extern int vcpu_configs_n; · line 54
Quoted 6 lines
tools/testing/selftests/kvm/get-reg-list.c +3 -0
@@ -49,6 +49,10 @@ extern int vcpu_configs_n;
      for_each_reg_filtered(i)                                                \
              if (!find_reg(blessed_reg, blessed_n, reg_list->reg[i]))
 
+#define for_each_present_blessed_reg(i)                      \
+     for ((i) = 0; (i) < blessed_n; ++(i))           \
+             if (find_reg(reg_list->reg, reg_list->n, blessed_reg[i]))
I just realized this is backwards. We need 'i' to index reg_list->reg in the body of the loop. That means we need to write this as

Quoted 7 lines

#define for_each_present_blessed_reg(i)                                         \
        for_each_reg(i)                                                         \
                if (find_reg(blessed_reg, blessed_n, reg_list->reg[i]))

(Which, in hindsight, makes sense since we're replacing a for_each_reg() loop.)

Sure, I will update it in v6.

Thanks!

Quoted 2 lines

Thanks,
drew

Haibo Xu

author

2023-07-01 13:39 UTC #15 lore raw

PATCH v5 11/13 KVM: selftests: Add skip_set facility to get_reg_list test

In reply to Haibo Xu

Add new skips_set members to vcpu_reg_sublist so as to skip set operation on some registers.

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/get-reg-list.c    | 20 +++++++++++++------
 .../selftests/kvm/include/kvm_util_base.h     |  2 ++
 2 files changed, 16 insertions(+), 6 deletions(-)

tools/testing/selftests/kvm/get-reg-list.c +14 -6

index 74fb6f6fdd09..1a32a900aeea 100644
@@ -157,6 +157,7 @@ static void run_test(struct vcpu_reg_list *c)
 {
 	int new_regs = 0, missing_regs = 0, i, n;
 	int failed_get = 0, failed_set = 0, failed_reject = 0;
+	int skipped_set = 0;
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
 	struct vcpu_reg_sublist *s;

@@ -210,7 +211,7 @@ static void run_test(struct vcpu_reg_list *c)
 			.id = reg_list->reg[i],
 			.addr = (__u64)&addr,
 		};
-		bool reject_reg = false;
+		bool reject_reg = false, skip_reg = false;
 		int ret;
 
 		ret = __vcpu_get_reg(vcpu, reg_list->reg[i], &addr);

@@ -221,8 +222,8 @@ static void run_test(struct vcpu_reg_list *c)
 			++failed_get;
 		}
 
-		/* rejects_set registers are rejected after KVM_ARM_VCPU_FINALIZE */
 		for_each_sublist(c, s) {
+			/* rejects_set registers are rejected for set operation */
 			if (s->rejects_set && find_reg(s->rejects_set, s->rejects_set_n, reg.id)) {
 				reject_reg = true;
 				ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);

@@ -234,9 +235,16 @@ static void run_test(struct vcpu_reg_list *c)
 				}
 				break;
 			}
+
+			/* skips_set registers are skipped for set operation */
+			if (s->skips_set && find_reg(s->skips_set, s->skips_set_n, reg.id)) {
+				skip_reg = true;
+				++skipped_set;
+				break;
+			}
 		}
 
-		if (!reject_reg) {
+		if (!reject_reg && !skip_reg) {
 			ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);
 			if (ret) {
 				printf("%s: Failed to set ", config_name(c));

@@ -281,9 +289,9 @@ static void run_test(struct vcpu_reg_list *c)
 	}
 
 	TEST_ASSERT(!missing_regs && !failed_get && !failed_set && !failed_reject,
-		    "%s: There are %d missing registers; "
-		    "%d registers failed get; %d registers failed set; %d registers failed reject",
-		    config_name(c), missing_regs, failed_get, failed_set, failed_reject);
+		    "%s: There are %d missing registers; %d registers failed get; "
+		    "%d registers failed set; %d registers failed reject; %d registers skipped set",
+		    config_name(c), missing_regs, failed_get, failed_set, failed_reject, skipped_set);
 
 	pr_info("%s: PASS\n", config_name(c));
 	blessed_n = 0;

tools/testing/selftests/kvm/include/kvm_util_base.h +2 -0

index e4480049000d..67c031fe89a1 100644
@@ -134,6 +134,8 @@ struct vcpu_reg_sublist {
 	__u64 regs_n;
 	__u64 *rejects_set;
 	__u64 rejects_set_n;
+	__u64 *skips_set;
+	__u64 skips_set_n;
 };
 
 struct vcpu_reg_list {

Signature · 1 line

2.34.1

Andrew Jones

Reviewed-by on v5 11/13

2023-07-01 14:28 UTC #16 lore raw

Re: [PATCH v5 11/13] KVM: selftests: Add skip_set facility to get_reg_list test

Quoted 88 lines · Haibo Xu

Add new skips_set members to vcpu_reg_sublist so as to skip set operation on some registers.

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/get-reg-list.c    | 20 +++++++++++++------
 .../selftests/kvm/include/kvm_util_base.h     |  2 ++
 2 files changed, 16 insertions(+), 6 deletions(-)

tools/testing/selftests/kvm/get-reg-list.c +14 -6

index 74fb6f6fdd09..1a32a900aeea 100644
@@ -157,6 +157,7 @@ static void run_test(struct vcpu_reg_list *c)
 {
 	int new_regs = 0, missing_regs = 0, i, n;
 	int failed_get = 0, failed_set = 0, failed_reject = 0;
+	int skipped_set = 0;
 	struct kvm_vcpu *vcpu;
 	struct kvm_vm *vm;
 	struct vcpu_reg_sublist *s;

@@ -210,7 +211,7 @@ static void run_test(struct vcpu_reg_list *c)
 			.id = reg_list->reg[i],
 			.addr = (__u64)&addr,
 		};
-		bool reject_reg = false;
+		bool reject_reg = false, skip_reg = false;
 		int ret;
 
 		ret = __vcpu_get_reg(vcpu, reg_list->reg[i], &addr);

@@ -221,8 +222,8 @@ static void run_test(struct vcpu_reg_list *c)
 			++failed_get;
 		}
 
-		/* rejects_set registers are rejected after KVM_ARM_VCPU_FINALIZE */
 		for_each_sublist(c, s) {
+			/* rejects_set registers are rejected for set operation */
 			if (s->rejects_set && find_reg(s->rejects_set, s->rejects_set_n, reg.id)) {
 				reject_reg = true;
 				ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);

@@ -234,9 +235,16 @@ static void run_test(struct vcpu_reg_list *c)
 				}
 				break;
 			}
+
+			/* skips_set registers are skipped for set operation */
+			if (s->skips_set && find_reg(s->skips_set, s->skips_set_n, reg.id)) {
+				skip_reg = true;
+				++skipped_set;
+				break;
+			}
 		}
 
-		if (!reject_reg) {
+		if (!reject_reg && !skip_reg) {
 			ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);
 			if (ret) {
 				printf("%s: Failed to set ", config_name(c));

@@ -281,9 +289,9 @@ static void run_test(struct vcpu_reg_list *c)
 	}
 
 	TEST_ASSERT(!missing_regs && !failed_get && !failed_set && !failed_reject,
-		    "%s: There are %d missing registers; "
-		    "%d registers failed get; %d registers failed set; %d registers failed reject",
-		    config_name(c), missing_regs, failed_get, failed_set, failed_reject);
+		    "%s: There are %d missing registers; %d registers failed get; "
+		    "%d registers failed set; %d registers failed reject; %d registers skipped set",
+		    config_name(c), missing_regs, failed_get, failed_set, failed_reject, skipped_set);
 
 	pr_info("%s: PASS\n", config_name(c));
 	blessed_n = 0;

tools/testing/selftests/kvm/include/kvm_util_base.h +2 -0

index e4480049000d..67c031fe89a1 100644
@@ -134,6 +134,8 @@ struct vcpu_reg_sublist {
 	__u64 regs_n;
 	__u64 *rejects_set;
 	__u64 rejects_set_n;
+	__u64 *skips_set;
+	__u64 skips_set_n;
 };
 
 struct vcpu_reg_list {

Signature · 1 line

2.34.1

Reviewed-by Andrew Jones <[email protected]>

Haibo Xu

2023-07-01 14:35 UTC #17 lore raw

Re: [PATCH v5 11/13] KVM: selftests: Add skip_set facility to get_reg_list test

Quoted 86 lines · On Sat, Jul 1, 2023 at 10:28?PM Andrew Jones <[email protected]> wrote:

Quoted 84 lines · Haibo Xu

Add new skips_set members to vcpu_reg_sublist so as to skip set operation on some registers.

Signed-off-by Haibo Xu <[email protected]>

---
 tools/testing/selftests/kvm/get-reg-list.c    | 20 +++++++++++++------
 .../selftests/kvm/include/kvm_util_base.h     |  2 ++
 2 files changed, 16 insertions(+), 6 deletions(-)

tools/testing/selftests/kvm/get-reg-list.c +14 -6

index 74fb6f6fdd09..1a32a900aeea 100644
@@ -157,6 +157,7 @@ static void run_test(struct vcpu_reg_list *c)
 {
      int new_regs = 0, missing_regs = 0, i, n;
      int failed_get = 0, failed_set = 0, failed_reject = 0;
+     int skipped_set = 0;
      struct kvm_vcpu *vcpu;
      struct kvm_vm *vm;
      struct vcpu_reg_sublist *s;

@@ -210,7 +211,7 @@ static void run_test(struct vcpu_reg_list *c)
                      .id = reg_list->reg[i],
                      .addr = (__u64)&addr,
              };
-             bool reject_reg = false;
+             bool reject_reg = false, skip_reg = false;
              int ret;
 
              ret = __vcpu_get_reg(vcpu, reg_list->reg[i], &addr);

@@ -221,8 +222,8 @@ static void run_test(struct vcpu_reg_list *c)
                      ++failed_get;
              }
 
-             /* rejects_set registers are rejected after KVM_ARM_VCPU_FINALIZE */
              for_each_sublist(c, s) {
+                     /* rejects_set registers are rejected for set operation */
                      if (s->rejects_set && find_reg(s->rejects_set, s->rejects_set_n, reg.id)) {
                              reject_reg = true;
                              ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);

@@ -234,9 +235,16 @@ static void run_test(struct vcpu_reg_list *c)
                              }
                              break;
                      }
+
+                     /* skips_set registers are skipped for set operation */
+                     if (s->skips_set && find_reg(s->skips_set, s->skips_set_n, reg.id)) {
+                             skip_reg = true;
+                             ++skipped_set;
+                             break;
+                     }
              }
 
-             if (!reject_reg) {
+             if (!reject_reg && !skip_reg) {
                      ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);
                      if (ret) {
                              printf("%s: Failed to set ", config_name(c));

@@ -281,9 +289,9 @@ static void run_test(struct vcpu_reg_list *c)
      }
 
      TEST_ASSERT(!missing_regs && !failed_get && !failed_set && !failed_reject,
-                 "%s: There are %d missing registers; "
-                 "%d registers failed get; %d registers failed set; %d registers failed reject",
-                 config_name(c), missing_regs, failed_get, failed_set, failed_reject);
+                 "%s: There are %d missing registers; %d registers failed get; "
+                 "%d registers failed set; %d registers failed reject; %d registers skipped set",
+                 config_name(c), missing_regs, failed_get, failed_set, failed_reject, skipped_set);
 
      pr_info("%s: PASS\n", config_name(c));
      blessed_n = 0;

tools/testing/selftests/kvm/include/kvm_util_base.h +2 -0

index e4480049000d..67c031fe89a1 100644
@@ -134,6 +134,8 @@ struct vcpu_reg_sublist {
      __u64 regs_n;
      __u64 *rejects_set;
      __u64 rejects_set_n;
+     __u64 *skips_set;
+     __u64 skips_set_n;
 };

Quoted 6 lines

Quoted 4 lines
tools/testing/selftests/kvm/include/kvm_util_base.h +0 -1
 struct vcpu_reg_list {
--
2.34.1
Reviewed-by Andrew Jones <[email protected]>

Thanks!

Haibo Xu

author

2023-07-01 13:39 UTC #18 lore raw

PATCH v5 12/13 KVM: riscv: Add KVM_GET_REG_LIST API support

In reply to Haibo Xu

KVM_GET_REG_LIST API will return all registers that are available to KVM_GET/SET_ONE_REG APIs. It's very useful to identify some platform regression issue during VM migration.

Since this API was already supported on arm64, it is straightforward to enable it on riscv with similar code structure.

Signed-off-by Haibo Xu <[email protected]>

Reviewed-by Andrew Jones <[email protected]>

---
 Documentation/virt/kvm/api.rst |   2 +-
 arch/riscv/kvm/vcpu.c          | 375 +++++++++++++++++++++++++++++++++
 2 files changed, 376 insertions(+), 1 deletion(-)

Documentation/virt/kvm/api.rst +1 -1

index add067793b90..280e89abd004 100644
@@ -3499,7 +3499,7 @@ VCPU matching underlying host.
 ---------------------
 
 :Capability: basic
-:Architectures: arm64, mips
+:Architectures: arm64, mips, riscv
 :Type: vcpu ioctl
 :Parameters: struct kvm_reg_list (in/out)
 :Returns: 0 on success; -1 on error

arch/riscv/kvm/vcpu.c +375 -0

387 lines not on this page · raw message

Signature · 1 line

2.34.1

Haibo Xu

author

2023-07-01 13:40 UTC #19 lore raw

PATCH v5 13/13 KVM: riscv: selftests: Add get-reg-list test

In reply to Haibo Xu

get-reg-list test is used to check for KVM registers regressions during VM migration which happens when destination host kernel missing registers that the source host kernel has. The blessed list registers was created by running on v6.4

Signed-off-by Haibo Xu <[email protected]>

Reviewed-by Andrew Jones <[email protected]>

---
 tools/testing/selftests/kvm/Makefile          |   1 +
 .../selftests/kvm/include/riscv/processor.h   |   3 +
 .../selftests/kvm/riscv/get-reg-list.c        | 780 ++++++++++++++++++
 3 files changed, 784 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/riscv/get-reg-list.c

tools/testing/selftests/kvm/Makefile +1 -0

index d90cad19c9ee..f7bcda903dd9 100644
@@ -174,6 +174,7 @@ TEST_GEN_PROGS_s390x += kvm_binary_stats_test
 
 TEST_GEN_PROGS_riscv += demand_paging_test
 TEST_GEN_PROGS_riscv += dirty_log_test
+TEST_GEN_PROGS_riscv += get-reg-list
 TEST_GEN_PROGS_riscv += kvm_create_max_vcpus
 TEST_GEN_PROGS_riscv += kvm_page_table_test
 TEST_GEN_PROGS_riscv += set_memory_region_test

tools/testing/selftests/kvm/include/riscv/processor.h +3 -0

index d00d213c3805..5b62a3d2aa9b 100644
@@ -38,6 +38,9 @@ static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx,
 					     KVM_REG_RISCV_TIMER_REG(name), \
 					     KVM_REG_SIZE_U64)
 
+#define RISCV_ISA_EXT_REG(idx)	__kvm_reg_id(KVM_REG_RISCV_ISA_EXT, \
+					     idx, KVM_REG_SIZE_ULONG)
+
 /* L3 index Bit[47:39] */
 #define PGTBL_L3_INDEX_MASK			0x0000FF8000000000ULL
 #define PGTBL_L3_INDEX_SHIFT			39

tools/testing/selftests/kvm/riscv/get-reg-list.c new file +780 -0

780 lines not on this page · raw message

Signature · 1 line

2.34.1

Andrew Jones

2023-07-01 14:53 UTC #20 lore raw

In reply to Haibo Xu

Quoted 18 lines · Haibo Xu

KVM_GET_REG_LIST will dump all register IDs that are available to KVM_GET/SET_ONE_REG and It's very useful to identify some platform regression issue during VM migration.
Patch 1-7 re-structured the get-reg-list test in aarch64 to make some of the code as common test framework that can be shared by riscv.
Patch 8 move reject_set check logic to a function so as to check for different errno for different registers.
Patch 9 move finalize_vcpu back to run_test so that riscv can implement its specific operation.
Patch 10 change to do the get/set operation only on present-blessed list. Patch 11 add the skip_set facilities so that riscv can skip set operation on some registers.
Patch 12 enabled the KVM_GET_REG_LIST API in riscv.
patch 13 added the corresponding kselftest for checking possible register regressions.

Quoted 2 lines

The get-reg-list kvm selftest was ported from aarch64 and tested with Linux v6.4 on a Qemu riscv64 virt machine.

Hi Haibo,

This series needs a rebase on top of these recent patches

commit ee053e03b08e ("KVM: selftests: get-reg-list: support ID register features") commit 5f0419a0083b ("KVM: selftests: get-reg-list: add Permission Indirection registers")

The conflicts will be in my patches, which do the splitting of the test. I'll do the rebase and push it to a branch for you to pick up.

Thanks,
drew

Haibo Xu

2023-07-03 02:53 UTC #21 lore raw

Quoted 25 lines · On Sat, Jul 1, 2023 at 10:53?PM Andrew Jones <[email protected]> wrote:

Quoted 18 lines · Haibo Xu
KVM_GET_REG_LIST will dump all register IDs that are available to KVM_GET/SET_ONE_REG and It's very useful to identify some platform regression issue during VM migration.
Patch 1-7 re-structured the get-reg-list test in aarch64 to make some of the code as common test framework that can be shared by riscv.
Patch 8 move reject_set check logic to a function so as to check for different errno for different registers.
Patch 9 move finalize_vcpu back to run_test so that riscv can implement its specific operation.
Patch 10 change to do the get/set operation only on present-blessed list. Patch 11 add the skip_set facilities so that riscv can skip set operation on some registers.
Patch 12 enabled the KVM_GET_REG_LIST API in riscv.
patch 13 added the corresponding kselftest for checking possible register regressions.
Quoted 2 lines
The get-reg-list kvm selftest was ported from aarch64 and tested with Linux v6.4 on a Qemu riscv64 virt machine.
Hi Haibo,

Quoted 8 lines

This series needs a rebase on top of these recent patches
commit ee053e03b08e ("KVM: selftests: get-reg-list: support ID register features") commit 5f0419a0083b ("KVM: selftests: get-reg-list: add Permission Indirection registers")
The conflicts will be in my patches, which do the splitting of the test. I'll do the rebase and push it to a branch for you to pick up.

Thanks for the heads up. I will include your changes in v6.

Quoted 2 lines

Thanks,
drew