RISCV: Add KVM_GET_REG_LIST API

PATCH Superseded Haibo Xu 4 participants 14 messages May 11, 2023 – May 18, 2023

Subsystems (6)

Documentation KVM Kselftest RISC-V +2 more subsystems

lore

Share & tools

lore

Haibo Xu

author

2023-05-11 09:16 UTC #0 lore raw

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 enabled the KVM_GET_REG_LIST API in riscv and patch 2 added the corresponding kselftest for checking possible register regressions.

Both patches were ported from arm64 and tested with Linux 6.4-rc1 on a Qemu riscv virt machine.

Haibo Xu (2):
riscv: kvm: Add KVM_GET_REG_LIST API support
KVM: selftests: Add riscv get-reg-list test

 Documentation/virt/kvm/api.rst                |   2 +-
 arch/riscv/kvm/vcpu.c                         | 346 +++++++
 tools/testing/selftests/kvm/Makefile          |   3 +
 .../selftests/kvm/include/riscv/processor.h   |   3 +
 .../selftests/kvm/riscv/get-reg-list.c        | 869 ++++++++++++++++++
 5 files changed, 1222 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/kvm/riscv/get-reg-list.c

Signature · 7 lines

2.34.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Haibo Xu

author

2023-05-11 09:16 UTC #1 lore raw

PATCH 1/2 riscv: kvm: Add KVM_GET_REG_LIST API support

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'd be straightforward to enable it on riscv with similar code structure.

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

---
 Documentation/virt/kvm/api.rst |   2 +-
 arch/riscv/kvm/vcpu.c          | 346 +++++++++++++++++++++++++++++++++
 2 files changed, 347 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 +346 -0

358 lines not on this page · raw message

Signature · 7 lines

2.34.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Andrew Jones

Reviewed-by on v1 1/2

2023-05-11 10:44 UTC #2 lore raw

Re: [PATCH 1/2] riscv: kvm: Add KVM_GET_REG_LIST API support

Quoted 5 lines · 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'd be straightforward

s/it'd be/it is/

Quoted 31 lines

to enable it on riscv with similar code structure.

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

---
 Documentation/virt/kvm/api.rst |   2 +-
 arch/riscv/kvm/vcpu.c          | 346 +++++++++++++++++++++++++++++++++
 2 files changed, 347 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 +2 -0

index 8bd9f2a8a0b9..fb8834e4fa15 100644
@@ -657,6 +657,334 @@ static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
+static inline unsigned long num_config_regs(void)
+{

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 671

Quoted 10 lines

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

+	return sizeof(struct kvm_riscv_config) / sizeof(unsigned long);
+}
+
+static int copy_config_reg_indices(u64 __user *uindices)
+{
+	unsigned int i;
+	int n = num_config_regs();
+
+	for (i = 0; i < n; i++) {
+		u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CONFIG | i;

^ this should be size-ulong

  u64 size = IS_ENABLED(CONFIG_32BIT) ? KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
  u64 reg = KVM_REG_RISCV | size | KVM_REG_RISCV_CONFIG | i;

Quoted 13 lines

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

+
+		if (uindices) {
+			if (put_user(reg, uindices))
+				return -EFAULT;
+			uindices++;
+		}
+	}
+
+	return n;
+}
+
+static inline unsigned long num_core_regs(void)
+{

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 694

Quoted 10 lines

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

+	return sizeof(struct kvm_riscv_core) / sizeof(unsigned long);
+}
+
+static int copy_core_reg_indices(u64 __user *uindices)
+{
+	unsigned int i;
+	int n = num_core_regs();
+
+	for (i = 0; i < n; i++) {
+		u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | i;

^ size-ulong

Quoted 20 lines

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

+
+		if (uindices) {
+			if (put_user(reg, uindices))
+				return -EFAULT;
+			uindices++;
+		}
+	}
+
+	return n;
+}
+
+static inline unsigned long num_csr_regs(void)
+{
+	unsigned long n = sizeof(struct kvm_riscv_csr) / sizeof(unsigned long);
+
+	if (kvm_riscv_aia_available())
+		n += sizeof(struct kvm_riscv_aia_csr) / sizeof(unsigned long);
+
+	return n;
+}

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 724

Quoted 10 lines

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

+
+static int copy_csr_reg_indices(u64 __user *uindices)
+{
+	unsigned int i;
+	int n1 = sizeof(struct kvm_riscv_csr) / sizeof(unsigned long);
+	int n2 = 0;
+
+	/* copy general csr regs */
+	for (i = 0; i < n1; i++) {
+		u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR |

^ size-ulong

Quoted 5 lines

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

+				  KVM_REG_RISCV_CSR_GENERAL | i;
+
+		if (uindices) {
+			if (put_user(reg, uindices))
+				return -EFAULT;

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 739

Quoted 10 lines

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

+			uindices++;
+		}
+	}
+
+	/* copy AIA csr regs */
+	if (kvm_riscv_aia_available()) {
+		n2 = sizeof(struct kvm_riscv_aia_csr) / sizeof(unsigned long);
+
+		for (i = 0; i < n2; i++) {
+			u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR |

^ size-ulong

Quoted 73 lines

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

+					  KVM_REG_RISCV_CSR_AIA | i;
+
+			if (uindices) {
+				if (put_user(reg, uindices))
+					return -EFAULT;
+				uindices++;
+			}
+		}
+	}
+
+	return n1 + n2;
+}
+
+static inline unsigned long num_timer_regs(void)
+{
+	return sizeof(struct kvm_riscv_timer) / sizeof(unsigned long);
+}
+
+static int copy_timer_reg_indices(u64 __user *uindices)
+{
+	unsigned int i;
+	int n = num_timer_regs();
+
+	for (i = 0; i < n; i++) {
+		u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | i;
+
+		if (uindices) {
+			if (put_user(reg, uindices))
+				return -EFAULT;
+			uindices++;
+		}
+	}
+
+	return n;
+}
+
+static inline unsigned long num_fp_f_regs(const struct kvm_vcpu *vcpu)
+{
+	const struct kvm_cpu_context *cntx = &vcpu->arch.guest_context;
+
+	if (riscv_isa_extension_available(vcpu->arch.isa, f))
+		return sizeof(cntx->fp.f) / sizeof(u32);
+	else
+		return 0;
+}
+
+static int copy_fp_f_reg_indices(const struct kvm_vcpu *vcpu,
+					u64 __user *uindices)
+{
+	unsigned int i;
+	int n = num_fp_f_regs(vcpu);
+
+	for (i = 0; i < n; i++) {
+		u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | i;
+
+		if (uindices) {
+			if (put_user(reg, uindices))
+				return -EFAULT;
+			uindices++;
+		}
+	}
+
+	return n;
+}
+
+static inline unsigned long num_fp_d_regs(const struct kvm_vcpu *vcpu)
+{
+	const struct kvm_cpu_context *cntx = &vcpu->arch.guest_context;
+
+	if (riscv_isa_extension_available(vcpu->arch.isa, d))
+		return sizeof(cntx->fp.d.f) / sizeof(u64) + 1;
+	else
+		return 0;

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 822

Quoted 10 lines

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

+}
+
+static int copy_fp_d_reg_indices(const struct kvm_vcpu *vcpu,
+					u64 __user *uindices)
+{
+	unsigned int i;
+	int n = num_fp_d_regs(vcpu);
+	u64 reg;
+
+	/* copy fp.d.f indeices */

indices

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 833

Quoted 11 lines

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

+	for (i = 0; i < n-1; i++) {
+		reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | i;
+
+		if (uindices) {
+			if (put_user(reg, uindices))
+				return -EFAULT;
+			uindices++;
+		}
+	}
+
+	/* copy fp.d.fcsr indeices */

indices

Quoted 11 lines

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

+	reg = KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_D | i;
+	if (uindices) {
+		if (put_user(reg, uindices))
+			return -EFAULT;
+	}
+
+	return n;
+}
+
+static inline unsigned long num_isa_ext_regs(void)
+{

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 854

Quoted 10 lines

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

+	return KVM_RISCV_ISA_EXT_MAX;
+}
+
+static int copy_isa_ext_reg_indices(u64 __user *uindices)
+{
+	unsigned int i;
+	int n = num_isa_ext_regs();
+
+	for (i = 0; i < n; i++) {
+		u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | i;

^ size-ulong

Quoted 6 lines

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

+
+		if (uindices) {
+			if (put_user(reg, uindices))
+				return -EFAULT;
+			uindices++;
+		}

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 870

Quoted 10 lines

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

+	}
+
+	return n;
+}
+
+static inline unsigned long num_sbi_ext_regs(void)
+{
+	/* number of KVM_REG_RISCV_SBI_SINGLE +
+	 *  2x(number of KVM_REG_RISCV_SBI_MULTI)
+	 */

Please use an opening wing '/*' on comments.

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 882

Quoted 12 lines

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

+	return KVM_RISCV_SBI_EXT_MAX + 2*(KVM_REG_RISCV_SBI_MULTI_REG_LAST+1);
+}
+
+static int copy_sbi_ext_reg_indices(u64 __user *uindices)
+{
+	unsigned int i;
+	int n;
+
+	/* copy KVM_REG_RISCV_SBI_SINGLE */
+	n = KVM_RISCV_SBI_EXT_MAX;
+	for (i = 0; i < n; i++) {
+		u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_SBI_EXT |

^ size-ulong

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 895

Quoted 13 lines

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

+				  KVM_REG_RISCV_SBI_SINGLE | i;
+
+		if (uindices) {
+			if (put_user(reg, uindices))
+				return -EFAULT;
+			uindices++;
+		}
+	}
+
+	/* copy KVM_REG_RISCV_SBI_MULTI */
+	n = KVM_REG_RISCV_SBI_MULTI_REG_LAST + 1;
+	for (i = 0; i < n; i++) {
+		u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_SBI_EXT |

^ size-ulong

Quoted 123 lines

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

+				  KVM_REG_RISCV_SBI_MULTI_EN | i;
+
+		if (uindices) {
+			if (put_user(reg, uindices))
+				return -EFAULT;
+			uindices++;
+		}
+
+		reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_SBI_EXT |
+			  KVM_REG_RISCV_SBI_MULTI_DIS | i;
+
+		if (uindices) {
+			if (put_user(reg, uindices))
+				return -EFAULT;
+			uindices++;
+		}
+	}
+
+	return num_sbi_ext_regs();
+}
+
+/**
+ * kvm_riscv_vcpu_num_regs - how many registers do we present via KVM_GET/SET_ONE_REG
+ *
+ * This is for all registers.
+ */
+static unsigned long kvm_riscv_vcpu_num_regs(struct kvm_vcpu *vcpu)
+{
+	unsigned long res = 0;
+
+	res += num_config_regs();
+	res += num_core_regs();
+	res += num_csr_regs();
+	res += num_timer_regs();
+	res += num_fp_f_regs(vcpu);
+	res += num_fp_d_regs(vcpu);
+	res += num_isa_ext_regs();
+	res += num_sbi_ext_regs();
+
+	return res;
+}
+
+/**
+ * kvm_riscv_vcpu_copy_reg_indices - get indices of all registers.
+ */
+static int kvm_riscv_vcpu_copy_reg_indices(struct kvm_vcpu *vcpu,
+				u64 __user *uindices)
+{
+	int ret;
+
+	ret = copy_config_reg_indices(uindices);
+	if (ret < 0)
+		return ret;
+	uindices += ret;
+
+	ret = copy_core_reg_indices(uindices);
+	if (ret < 0)
+		return ret;
+	uindices += ret;
+
+	ret = copy_csr_reg_indices(uindices);
+	if (ret < 0)
+		return ret;
+	uindices += ret;
+
+	ret = copy_timer_reg_indices(uindices);
+	if (ret < 0)
+		return ret;
+	uindices += ret;
+
+	ret = copy_fp_f_reg_indices(vcpu, uindices);
+	if (ret < 0)
+		return ret;
+	uindices += ret;
+
+	ret = copy_fp_d_reg_indices(vcpu, uindices);
+	if (ret < 0)
+		return ret;
+	uindices += ret;
+
+	ret = copy_isa_ext_reg_indices(uindices);
+	if (ret < 0)
+		return ret;
+	uindices += ret;
+
+	ret = copy_sbi_ext_reg_indices(uindices);
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
 static int kvm_riscv_vcpu_set_reg(struct kvm_vcpu *vcpu,
 				  const struct kvm_one_reg *reg)
 {

@@ -758,6 +1086,24 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 			r = kvm_riscv_vcpu_get_reg(vcpu, &reg);
 		break;
 	}
+	case KVM_GET_REG_LIST: {
+		struct kvm_reg_list __user *user_list = argp;
+		struct kvm_reg_list reg_list;
+		unsigned int n;
+
+		r = -EFAULT;
+		if (copy_from_user(&reg_list, user_list, sizeof(reg_list)))
+			break;
+		n = reg_list.n;
+		reg_list.n = kvm_riscv_vcpu_num_regs(vcpu);
+		if (copy_to_user(user_list, &reg_list, sizeof(reg_list)))
+			break;
+		r = -E2BIG;
+		if (n < reg_list.n)
+			break;
+		r = kvm_riscv_vcpu_copy_reg_indices(vcpu, user_list->reg);
+		break;
+	}
 	default:
 		break;
 	}

Signature · 1 line

2.34.1

Otherwise,

Reviewed-by Andrew Jones <[email protected]>

Thanks,
drew

Haibo Xu

2023-05-15 06:21 UTC #3 lore raw

Re: [PATCH 1/2] riscv: kvm: Add KVM_GET_REG_LIST API support

Quoted 428 lines · On Thu, May 11, 2023 at 6:44?PM Andrew Jones <[email protected]> wrote:

Quoted 5 lines · 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'd be straightforward

s/it'd be/it is/

Quoted 31 lines

to enable it on riscv with similar code structure.

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

---
 Documentation/virt/kvm/api.rst |   2 +-
 arch/riscv/kvm/vcpu.c          | 346 +++++++++++++++++++++++++++++++++
 2 files changed, 347 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 +2 -0

index 8bd9f2a8a0b9..fb8834e4fa15 100644
@@ -657,6 +657,334 @@ static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu,
      return 0;
 }
 
+static inline unsigned long num_config_regs(void)
+{

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 671

Quoted 10 lines

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

+     return sizeof(struct kvm_riscv_config) / sizeof(unsigned long);
+}
+
+static int copy_config_reg_indices(u64 __user *uindices)
+{
+     unsigned int i;
+     int n = num_config_regs();
+
+     for (i = 0; i < n; i++) {
+             u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CONFIG | i;

^ this should be size-ulong

  u64 size = IS_ENABLED(CONFIG_32BIT) ? KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
  u64 reg = KVM_REG_RISCV | size | KVM_REG_RISCV_CONFIG | i;

Quoted 13 lines

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

+
+             if (uindices) {
+                     if (put_user(reg, uindices))
+                             return -EFAULT;
+                     uindices++;
+             }
+     }
+
+     return n;
+}
+
+static inline unsigned long num_core_regs(void)
+{

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 694

Quoted 10 lines

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

+     return sizeof(struct kvm_riscv_core) / sizeof(unsigned long);
+}
+
+static int copy_core_reg_indices(u64 __user *uindices)
+{
+     unsigned int i;
+     int n = num_core_regs();
+
+     for (i = 0; i < n; i++) {
+             u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | i;

^ size-ulong

Quoted 20 lines

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

+
+             if (uindices) {
+                     if (put_user(reg, uindices))
+                             return -EFAULT;
+                     uindices++;
+             }
+     }
+
+     return n;
+}
+
+static inline unsigned long num_csr_regs(void)
+{
+     unsigned long n = sizeof(struct kvm_riscv_csr) / sizeof(unsigned long);
+
+     if (kvm_riscv_aia_available())
+             n += sizeof(struct kvm_riscv_aia_csr) / sizeof(unsigned long);
+
+     return n;
+}

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 724

Quoted 10 lines

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

+
+static int copy_csr_reg_indices(u64 __user *uindices)
+{
+     unsigned int i;
+     int n1 = sizeof(struct kvm_riscv_csr) / sizeof(unsigned long);
+     int n2 = 0;
+
+     /* copy general csr regs */
+     for (i = 0; i < n1; i++) {
+             u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR |

^ size-ulong

Quoted 5 lines

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

+                               KVM_REG_RISCV_CSR_GENERAL | i;
+
+             if (uindices) {
+                     if (put_user(reg, uindices))
+                             return -EFAULT;

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 739

Quoted 10 lines

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

+                     uindices++;
+             }
+     }
+
+     /* copy AIA csr regs */
+     if (kvm_riscv_aia_available()) {
+             n2 = sizeof(struct kvm_riscv_aia_csr) / sizeof(unsigned long);
+
+             for (i = 0; i < n2; i++) {
+                     u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR |

^ size-ulong

Quoted 73 lines

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

+                                       KVM_REG_RISCV_CSR_AIA | i;
+
+                     if (uindices) {
+                             if (put_user(reg, uindices))
+                                     return -EFAULT;
+                             uindices++;
+                     }
+             }
+     }
+
+     return n1 + n2;
+}
+
+static inline unsigned long num_timer_regs(void)
+{
+     return sizeof(struct kvm_riscv_timer) / sizeof(unsigned long);
+}
+
+static int copy_timer_reg_indices(u64 __user *uindices)
+{
+     unsigned int i;
+     int n = num_timer_regs();
+
+     for (i = 0; i < n; i++) {
+             u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | i;
+
+             if (uindices) {
+                     if (put_user(reg, uindices))
+                             return -EFAULT;
+                     uindices++;
+             }
+     }
+
+     return n;
+}
+
+static inline unsigned long num_fp_f_regs(const struct kvm_vcpu *vcpu)
+{
+     const struct kvm_cpu_context *cntx = &vcpu->arch.guest_context;
+
+     if (riscv_isa_extension_available(vcpu->arch.isa, f))
+             return sizeof(cntx->fp.f) / sizeof(u32);
+     else
+             return 0;
+}
+
+static int copy_fp_f_reg_indices(const struct kvm_vcpu *vcpu,
+                                     u64 __user *uindices)
+{
+     unsigned int i;
+     int n = num_fp_f_regs(vcpu);
+
+     for (i = 0; i < n; i++) {
+             u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | i;
+
+             if (uindices) {
+                     if (put_user(reg, uindices))
+                             return -EFAULT;
+                     uindices++;
+             }
+     }
+
+     return n;
+}
+
+static inline unsigned long num_fp_d_regs(const struct kvm_vcpu *vcpu)
+{
+     const struct kvm_cpu_context *cntx = &vcpu->arch.guest_context;
+
+     if (riscv_isa_extension_available(vcpu->arch.isa, d))
+             return sizeof(cntx->fp.d.f) / sizeof(u64) + 1;
+     else
+             return 0;

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 822

Quoted 10 lines

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

+}
+
+static int copy_fp_d_reg_indices(const struct kvm_vcpu *vcpu,
+                                     u64 __user *uindices)
+{
+     unsigned int i;
+     int n = num_fp_d_regs(vcpu);
+     u64 reg;
+
+     /* copy fp.d.f indeices */

indices

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 833

Quoted 11 lines

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

+     for (i = 0; i < n-1; i++) {
+             reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | i;
+
+             if (uindices) {
+                     if (put_user(reg, uindices))
+                             return -EFAULT;
+                     uindices++;
+             }
+     }
+
+     /* copy fp.d.fcsr indeices */

indices

Quoted 11 lines

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

+     reg = KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_D | i;
+     if (uindices) {
+             if (put_user(reg, uindices))
+                     return -EFAULT;
+     }
+
+     return n;
+}
+
+static inline unsigned long num_isa_ext_regs(void)
+{

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 854

Quoted 10 lines

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

+     return KVM_RISCV_ISA_EXT_MAX;
+}
+
+static int copy_isa_ext_reg_indices(u64 __user *uindices)
+{
+     unsigned int i;
+     int n = num_isa_ext_regs();
+
+     for (i = 0; i < n; i++) {
+             u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | i;

^ size-ulong

Quoted 6 lines

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

+
+             if (uindices) {
+                     if (put_user(reg, uindices))
+                             return -EFAULT;
+                     uindices++;
+             }

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 870

Quoted 10 lines

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

+     }
+
+     return n;
+}
+
+static inline unsigned long num_sbi_ext_regs(void)
+{
+     /* number of KVM_REG_RISCV_SBI_SINGLE +
+      *  2x(number of KVM_REG_RISCV_SBI_MULTI)
+      */

Please use an opening wing '/*' on comments.

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 882

Quoted 12 lines

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

+     return KVM_RISCV_SBI_EXT_MAX + 2*(KVM_REG_RISCV_SBI_MULTI_REG_LAST+1);
+}
+
+static int copy_sbi_ext_reg_indices(u64 __user *uindices)
+{
+     unsigned int i;
+     int n;
+
+     /* copy KVM_REG_RISCV_SBI_SINGLE */
+     n = KVM_RISCV_SBI_EXT_MAX;
+     for (i = 0; i < n; i++) {
+             u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_SBI_EXT |

^ size-ulong

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 895

Quoted 13 lines

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

+                               KVM_REG_RISCV_SBI_SINGLE | i;
+
+             if (uindices) {
+                     if (put_user(reg, uindices))
+                             return -EFAULT;
+                     uindices++;
+             }
+     }
+
+     /* copy KVM_REG_RISCV_SBI_MULTI */
+     n = KVM_REG_RISCV_SBI_MULTI_REG_LAST + 1;
+     for (i = 0; i < n; i++) {
+             u64 reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_SBI_EXT |

^ size-ulong

Quoted 121 lines

arch/riscv/kvm/vcpu.c +110 -1

+                               KVM_REG_RISCV_SBI_MULTI_EN | i;
+
+             if (uindices) {
+                     if (put_user(reg, uindices))
+                             return -EFAULT;
+                     uindices++;
+             }
+
+             reg = KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_SBI_EXT |
+                       KVM_REG_RISCV_SBI_MULTI_DIS | i;
+
+             if (uindices) {
+                     if (put_user(reg, uindices))
+                             return -EFAULT;
+                     uindices++;
+             }
+     }
+
+     return num_sbi_ext_regs();
+}
+
+/**
+ * kvm_riscv_vcpu_num_regs - how many registers do we present via KVM_GET/SET_ONE_REG
+ *
+ * This is for all registers.
+ */
+static unsigned long kvm_riscv_vcpu_num_regs(struct kvm_vcpu *vcpu)
+{
+     unsigned long res = 0;
+
+     res += num_config_regs();
+     res += num_core_regs();
+     res += num_csr_regs();
+     res += num_timer_regs();
+     res += num_fp_f_regs(vcpu);
+     res += num_fp_d_regs(vcpu);
+     res += num_isa_ext_regs();
+     res += num_sbi_ext_regs();
+
+     return res;
+}
+
+/**
+ * kvm_riscv_vcpu_copy_reg_indices - get indices of all registers.
+ */
+static int kvm_riscv_vcpu_copy_reg_indices(struct kvm_vcpu *vcpu,
+                             u64 __user *uindices)
+{
+     int ret;
+
+     ret = copy_config_reg_indices(uindices);
+     if (ret < 0)
+             return ret;
+     uindices += ret;
+
+     ret = copy_core_reg_indices(uindices);
+     if (ret < 0)
+             return ret;
+     uindices += ret;
+
+     ret = copy_csr_reg_indices(uindices);
+     if (ret < 0)
+             return ret;
+     uindices += ret;
+
+     ret = copy_timer_reg_indices(uindices);
+     if (ret < 0)
+             return ret;
+     uindices += ret;
+
+     ret = copy_fp_f_reg_indices(vcpu, uindices);
+     if (ret < 0)
+             return ret;
+     uindices += ret;
+
+     ret = copy_fp_d_reg_indices(vcpu, uindices);
+     if (ret < 0)
+             return ret;
+     uindices += ret;
+
+     ret = copy_isa_ext_reg_indices(uindices);
+     if (ret < 0)
+             return ret;
+     uindices += ret;
+
+     ret = copy_sbi_ext_reg_indices(uindices);
+     if (ret < 0)
+             return ret;
+
+     return 0;
+}
+
 static int kvm_riscv_vcpu_set_reg(struct kvm_vcpu *vcpu,
                                const struct kvm_one_reg *reg)
 {

@@ -758,6 +1086,24 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
                      r = kvm_riscv_vcpu_get_reg(vcpu, &reg);
              break;
      }
+     case KVM_GET_REG_LIST: {
+             struct kvm_reg_list __user *user_list = argp;
+             struct kvm_reg_list reg_list;
+             unsigned int n;
+
+             r = -EFAULT;
+             if (copy_from_user(&reg_list, user_list, sizeof(reg_list)))
+                     break;
+             n = reg_list.n;
+             reg_list.n = kvm_riscv_vcpu_num_regs(vcpu);
+             if (copy_to_user(user_list, &reg_list, sizeof(reg_list)))
+                     break;
+             r = -E2BIG;
+             if (n < reg_list.n)
+                     break;
+             r = kvm_riscv_vcpu_copy_reg_indices(vcpu, user_list->reg);
+             break;
+     }
      default:
              break;
      }
--

Quoted 2 lines

2.34.1

Otherwise,

Quoted 4 lines

Reviewed-by Andrew Jones <[email protected]>
Thanks,
drew

Thanks for your review, Andrew! The comments will be addressed in next version.

Andrew Jones

2023-05-11 16:40 UTC #4 lore raw

Re: [PATCH 1/2] riscv: kvm: Add KVM_GET_REG_LIST API support

In reply to Haibo Xu

Quoted 31 lines · 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'd be straightforward to enable it on riscv with similar code structure.
Signed-off-by Haibo Xu <[email protected]>
---
 Documentation/virt/kvm/api.rst |   2 +-
 arch/riscv/kvm/vcpu.c          | 346 +++++++++++++++++++++++++++++++++
 2 files changed, 347 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 +0 -0
index 8bd9f2a8a0b9..fb8834e4fa15 100644

arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 662

Quoted 6 lines

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

@@ -657,6 +657,334 @@ static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
+static inline unsigned long num_config_regs(void)
+{
+	return sizeof(struct kvm_riscv_config) / sizeof(unsigned long);

We can't assume all config registers are present. For example, zicbom and zicboz block size registers are only present when their respective extensions are available.

Thanks,
drew

Haibo Xu

2023-05-15 06:23 UTC #5 lore raw

Re: [PATCH 1/2] riscv: kvm: Add KVM_GET_REG_LIST API support

Quoted 44 lines · On Fri, May 12, 2023 at 12:40?AM Andrew Jones <[email protected]> wrote:

Quoted 31 lines · 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'd be straightforward to enable it on riscv with similar code structure.
Signed-off-by Haibo Xu <[email protected]>
---
 Documentation/virt/kvm/api.rst |   2 +-
 arch/riscv/kvm/vcpu.c          | 346 +++++++++++++++++++++++++++++++++
 2 files changed, 347 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 +0 -0
index 8bd9f2a8a0b9..fb8834e4fa15 100644
arch/riscv/kvm/vcpu.c static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu, · line 662
Quoted 6 lines
arch/riscv/kvm/vcpu.c +3 -0
@@ -657,6 +657,334 @@ static int kvm_riscv_vcpu_set_reg_isa_ext(struct kvm_vcpu *vcpu,
      return 0;
 }
 
+static inline unsigned long num_config_regs(void)
+{
+     return sizeof(struct kvm_riscv_config) / sizeof(unsigned long);
We can't assume all config registers are present. For example, zicbom and zicboz block size registers are only present when their respective extensions are available.

Quoted 2 lines

Thanks,
drew

Yes, I will filter out these kinds of registers in the next version.

Thanks,
Haibo

Conor Dooley

2023-05-11 22:25 UTC #6 lore raw

Re: [PATCH 1/2] riscv: kvm: Add KVM_GET_REG_LIST API support

In reply to Haibo Xu

Quoted 6 lines · 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'd be straightforward to enable it on riscv with similar code structure.

Applied on top of v6.4-rc1 this breaks the build :/

warning: Function parameter or member 'vcpu' not described in 'kvm_riscv_vcpu_num_regs'
warning: Function parameter or member 'uindices' not described in 'kvm_riscv_vcpu_copy_reg_indices'
warning: Function parameter or member 'vcpu' not described in 'kvm_riscv_vcpu_copy_reg_indices'

You have a bunch of kerneldoc comments (the ones with /**) that are not valid kerneldoc. Apparently allmodconfig catches that!

Cheers,
Conor.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230511/e86effc8/attachment-0001.sig>

Conor Dooley

2023-05-11 22:48 UTC #7 lore raw

Re: [PATCH 1/2] riscv: kvm: Add KVM_GET_REG_LIST API support

Quoted 9 lines · Conor Dooley

Quoted 6 lines · 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'd be straightforward to enable it on riscv with similar code structure.
Applied on top of v6.4-rc1 this breaks the build :/

I lied, I forgot W=1 is enabled for the allmodconfig builds in the patchwork automation.
The warnings are trivial to fix, so you should fix them anyway!

Quoted 9 lines

warning: Function parameter or member 'vcpu' not described in 'kvm_riscv_vcpu_num_regs'
warning: Function parameter or member 'uindices' not described in 'kvm_riscv_vcpu_copy_reg_indices'
warning: Function parameter or member 'vcpu' not described in 'kvm_riscv_vcpu_copy_reg_indices'
You have a bunch of kerneldoc comments (the ones with /**) that are not valid kerneldoc. Apparently allmodconfig catches that!
Cheers,
Conor.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230511/d5382f50/attachment-0001.sig>

Haibo Xu

2023-05-15 06:24 UTC #8 lore raw

Re: [PATCH 1/2] riscv: kvm: Add KVM_GET_REG_LIST API support

Quoted 7 lines · On Fri, May 12, 2023 at 6:48?AM Conor Dooley <[email protected]> wrote:

Quoted 5 lines · Conor Dooley
Quoted 4 lines · 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.

Quoted 9 lines

Quoted 4 lines
Quoted 2 lines
Since this API was already supported on arm64, it'd be straightforward to enable it on riscv with similar code structure.
Applied on top of v6.4-rc1 this breaks the build :/
I lied, I forgot W=1 is enabled for the allmodconfig builds in the patchwork automation.
The warnings are trivial to fix, so you should fix them anyway!

sure, I will fix them in the next version.

Thanks,
Haibo

Quoted 11 lines

Quoted 9 lines
warning: Function parameter or member 'vcpu' not described in 'kvm_riscv_vcpu_num_regs'
warning: Function parameter or member 'uindices' not described in 'kvm_riscv_vcpu_copy_reg_indices'
warning: Function parameter or member 'vcpu' not described in 'kvm_riscv_vcpu_copy_reg_indices'
You have a bunch of kerneldoc comments (the ones with /**) that are not valid kerneldoc. Apparently allmodconfig catches that!
Cheers,
Conor.

Haibo Xu

author

2023-05-11 09:17 UTC #9 lore raw

PATCH 2/2 KVM: selftests: Add riscv 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.2.

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

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

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

index 7a5ff646e7e7..fbb2e984297a 100644
@@ -13,6 +13,8 @@ else ifeq ($(ARCH),arm64)
 	ARCH_DIR := aarch64
 else ifeq ($(ARCH),s390)
 	ARCH_DIR := s390x
+else ifeq ($(ARCH),riscv)
+	ARCH_DIR := riscv
 else
 	ARCH_DIR := $(ARCH)
 endif

@@ -171,6 +173,7 @@ TEST_GEN_PROGS_s390x += rseq_test
 TEST_GEN_PROGS_s390x += set_memory_region_test
 TEST_GEN_PROGS_s390x += kvm_binary_stats_test
 
+TEST_GEN_PROGS_riscv += riscv/get-reg-list
 TEST_GEN_PROGS_riscv += demand_paging_test
 TEST_GEN_PROGS_riscv += dirty_log_test
 TEST_GEN_PROGS_riscv += kvm_create_max_vcpus

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

index d00d213c3805..0adf9cb34126 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_U64)
+
 /* 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 +869 -0

869 lines not on this page · raw message

Signature · 7 lines

2.34.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

Andrew Jones

2023-05-17 11:06 UTC #10 lore raw

Re: [PATCH 2/2] KVM: selftests: Add riscv get-reg-list test

Quoted 4 lines · 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.2.

v6.2? But get-reg-list is getting introduced with this patch, so it'll land in a 6.4 rc at the earliest. I don't see how can we generate the initial blessed list with anything earlier?

Quoted 35 lines

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

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

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

index 7a5ff646e7e7..fbb2e984297a 100644
@@ -13,6 +13,8 @@ else ifeq ($(ARCH),arm64)
 	ARCH_DIR := aarch64
 else ifeq ($(ARCH),s390)
 	ARCH_DIR := s390x
+else ifeq ($(ARCH),riscv)
+	ARCH_DIR := riscv
 else
 	ARCH_DIR := $(ARCH)
 endif

@@ -171,6 +173,7 @@ TEST_GEN_PROGS_s390x += rseq_test
 TEST_GEN_PROGS_s390x += set_memory_region_test
 TEST_GEN_PROGS_s390x += kvm_binary_stats_test
 
+TEST_GEN_PROGS_riscv += riscv/get-reg-list
 TEST_GEN_PROGS_riscv += demand_paging_test
 TEST_GEN_PROGS_riscv += dirty_log_test
 TEST_GEN_PROGS_riscv += kvm_create_max_vcpus

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

index d00d213c3805..0adf9cb34126 100644

tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 42

Quoted 5 lines

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

@@ -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_U64)

^ KVM_REG_SIZE_ULONG

Quoted 130 lines

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

+
 /* 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 +120 -0

new file mode 100644
index 000000000000..dec1df0c6e72
@@ -0,0 +1,869 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Check for KVM_GET_REG_LIST regressions.
+ *
+ * Copyright (C) 2020, Red Hat, Inc.
+ * Copyright (c) 2023 Intel Corporation
+ *
+ * The test was port from the get-reg-list.c on aarch64.
+ */
+#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"
+
+static struct kvm_reg_list *reg_list;
+static __u64 *blessed_reg, blessed_n;
+
+struct reg_sublist {
+	const char *name;
+	long capability;
+	__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 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))
+
+#define for_each_missing_reg(i)						\
+	for ((i) = 0; (i) < blessed_n; ++(i))			\
+		if (!find_reg(reg_list->reg, reg_list->n, blessed_reg[i]))
+
+#define for_each_new_reg(i)							\
+	for_each_reg(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;
+
+	if (c->name)
+		return c->name;
+
+	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 find_reg(__u64 regs[], __u64 nr_regs, __u64 reg)
+{
+	int i;
+
+	for (i = 0; i < nr_regs; ++i)
+		if (reg == regs[i])
+			return true;
+	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)
+
+static const char *config_id_to_str(struct vcpu_config *c, __u64 id)
+{
+	__u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_CONFIG);
+
+	/*
+	 * reg_off is the offset into struct kvm_riscv_config
+	 */
+	switch (reg_off) {
+	case KVM_REG_RISCV_CONFIG_REG(isa):
+		return "KVM_REG_RISCV_CONFIG_REG(isa)";
+	case KVM_REG_RISCV_CONFIG_REG(zicbom_block_size):
+		return "KVM_REG_RISCV_CONFIG_REG(zicbom_block_size)";
+	case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
+		return "KVM_REG_RISCV_CONFIG_REG(zicboz_block_size)";
+	case KVM_REG_RISCV_CONFIG_REG(mvendorid):
+		return "KVM_REG_RISCV_CONFIG_REG(mvendorid)";

tools/testing/selftests/kvm/riscv/get-reg-list.c line 130

Quoted 10 lines

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

+	case KVM_REG_RISCV_CONFIG_REG(marchid):
+		return "KVM_REG_RISCV_CONFIG_REG(marchid)";
+	case KVM_REG_RISCV_CONFIG_REG(mimpid):
+		return "KVM_REG_RISCV_CONFIG_REG(mimpid)";
+	default:
+		/* Config regs would grow regularly with new pseudo reg added, so
+		 * just show "##UNKNOWN##" to indicate a new pseudo config reg.
+		 */
+		printf("%s: Unknown config reg id: 0x%llx", config_name(c), id);
+		return "##UNKNOWN##";

The idea of these *to_str functions is to dump output that can be copy+pasted into a reg array (hence the trailing commas in print_reg lines). So we can't just print random lines here or return '##UNKOWN##', as that won't compile. Instead, the default should return

str_with_index("KVM_REG_RISCV_CONFIG_REG(##)", reg_off)

Quoted 22 lines

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

+	}
+}
+
+static const char *core_id_to_str(struct vcpu_config *c, __u64 id)
+{
+	__u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_CORE);
+
+	/*
+	 * reg_off is the offset into struct kvm_riscv_core
+	 */
+	switch (reg_off) {
+	case KVM_REG_RISCV_CORE_REG(regs.pc):
+		return "KVM_REG_RISCV_CORE_REG(regs.pc)";
+	case KVM_REG_RISCV_CORE_REG(regs.ra) ...
+	     KVM_REG_RISCV_CORE_REG(regs.t6):
+		return str_with_index("KVM_REG_RISCV_CORE_REG(regs.regs[##])", reg_off);
+	case KVM_REG_RISCV_CORE_REG(mode):
+		return "KVM_REG_RISCV_CORE_REG(mode)";
+	}
+
+	TEST_FAIL("%s: Unknown core reg id: 0x%llx", config_name(c), id);
+	return NULL;

tools/testing/selftests/kvm/riscv/get-reg-list.c line 162

Quoted 10 lines

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

+}
+
+static const char *general_csr_id_to_str(__u64 reg_off)
+{
+	/*
+	 * reg_off is the offset into struct kvm_riscv_csr
+	 */
+	switch (reg_off) {
+	case KVM_REG_RISCV_CSR_REG(sstatus):
+		return "KVM_REG_RISCV_CSR_REG(sstatus)";

We also need the subtype. So this should be something like

 #define CSR_GENERAL(csr) \
     "KVM_REG_RISCV_CSR_GENERAL | KVM_REG_RISCV_CSR_REG(" #csr ")"

  return CSR_GENERAL(sstatus);

and similar below for the AIA CSRs.

Quoted 11 lines

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

+	case KVM_REG_RISCV_CSR_REG(sie):
+		return "KVM_REG_RISCV_CSR_REG(sie)";
+	case KVM_REG_RISCV_CSR_REG(stvec):
+		return "KVM_REG_RISCV_CSR_REG(stvec)";
+	case KVM_REG_RISCV_CSR_REG(sscratch):
+		return "KVM_REG_RISCV_CSR_REG(sscratch)";
+	case KVM_REG_RISCV_CSR_REG(sepc):
+		return "KVM_REG_RISCV_CSR_REG(sepc)";
+	case KVM_REG_RISCV_CSR_REG(scause):
+		return "KVM_REG_RISCV_CSR_REG(scause)";
+	case KVM_REG_RISCV_CSR_REG(stval):

tools/testing/selftests/kvm/riscv/get-reg-list.c line 183

Quoted 10 lines

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

+		return "KVM_REG_RISCV_CSR_REG(stval)";
+	case KVM_REG_RISCV_CSR_REG(sip):
+		return "KVM_REG_RISCV_CSR_REG(sip)";
+	case KVM_REG_RISCV_CSR_REG(satp):
+		return "KVM_REG_RISCV_CSR_REG(satp)";
+	case KVM_REG_RISCV_CSR_REG(scounteren):
+		return "KVM_REG_RISCV_CSR_REG(scounteren)";
+	default:
+		TEST_FAIL("Unknown general csr reg: 0x%llx", reg_off);
+		return NULL;

nit: It'd be nice to have a pattern for this TEST_FAIL. Either outside the switch and not have a default or always the default. It seems to be changing function by function.

Quoted 33 lines

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

+	}
+}
+
+static const char *aia_csr_id_to_str(__u64 reg_off)
+{
+	/*
+	 * reg_off is the offset into struct kvm_riscv_aia_csr
+	 */
+	switch (reg_off) {
+	case KVM_REG_RISCV_CSR_AIA_REG(siselect):
+		return "KVM_REG_RISCV_CSR_AIA_REG(siselect)";
+	case KVM_REG_RISCV_CSR_AIA_REG(iprio1):
+		return "KVM_REG_RISCV_CSR_AIA_REG(iprio1)";
+	case KVM_REG_RISCV_CSR_AIA_REG(iprio2):
+		return "KVM_REG_RISCV_CSR_AIA_REG(iprio2)";
+	case KVM_REG_RISCV_CSR_AIA_REG(sieh):
+		return "KVM_REG_RISCV_CSR_AIA_REG(sieh)";
+	case KVM_REG_RISCV_CSR_AIA_REG(siph):
+		return "KVM_REG_RISCV_CSR_AIA_REG(siph)";
+	case KVM_REG_RISCV_CSR_AIA_REG(iprio1h):
+		return "KVM_REG_RISCV_CSR_AIA_REG(iprio1h)";
+	case KVM_REG_RISCV_CSR_AIA_REG(iprio2h):
+		return "KVM_REG_RISCV_CSR_AIA_REG(iprio2h)";
+	default:
+		TEST_FAIL("Unknown aia csr reg: 0x%llx", reg_off);
+		return NULL;
+	}
+}
+
+static const char *csr_id_to_str(struct vcpu_config *c, __u64 id)
+{
+	__u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_CSR);
+	__u64 reg_subtype = reg_off & KVM_REG_RISCV_SUBTYPE_MASK;

tools/testing/selftests/kvm/riscv/get-reg-list.c line 226

Quoted 10 lines

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

+
+	reg_off &= ~KVM_REG_RISCV_SUBTYPE_MASK;
+
+	switch (reg_subtype) {
+	case KVM_REG_RISCV_CSR_GENERAL:
+		return general_csr_id_to_str(reg_off);
+	case KVM_REG_RISCV_CSR_AIA:
+		return aia_csr_id_to_str(reg_off);
+	default:
+		TEST_FAIL("%s: Unknown csr reg id: 0x%llx", config_name(c), id);

Maybe better to report "unknown subtype"?

Quoted 85 lines

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

+		return NULL;
+	}
+}
+
+static const char *timer_id_to_str(struct vcpu_config *c, __u64 id)
+{
+	__u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_TIMER);
+
+	/*
+	 * reg_off is the offset into struct kvm_riscv_timer
+	 */
+	switch (reg_off) {
+	case KVM_REG_RISCV_TIMER_REG(frequency):
+		return "KVM_REG_RISCV_TIMER_REG(frequency)";
+	case KVM_REG_RISCV_TIMER_REG(time):
+		return "KVM_REG_RISCV_TIMER_REG(time)";
+	case KVM_REG_RISCV_TIMER_REG(compare):
+		return "KVM_REG_RISCV_TIMER_REG(compare)";
+	case KVM_REG_RISCV_TIMER_REG(state):
+		return "KVM_REG_RISCV_TIMER_REG(state)";
+	}
+
+	TEST_FAIL("%s: Unknown timer reg id: 0x%llx", config_name(c), id);
+	return NULL;
+}
+
+static const char *fp_f_id_to_str(struct vcpu_config *c, __u64 id)
+{
+	__u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_FP_F);
+
+	/*
+	 * reg_off is the offset into struct __riscv_f_ext_state
+	 */
+	switch (reg_off) {
+	case KVM_REG_RISCV_FP_F_REG(f[0]) ...
+	     KVM_REG_RISCV_FP_F_REG(f[31]):
+		return str_with_index("KVM_REG_RISCV_FP_F_REG(f[##])", reg_off);
+	case KVM_REG_RISCV_FP_F_REG(fcsr):
+		return "KVM_REG_RISCV_FP_F_REG(fcsr)";
+	}
+
+	TEST_FAIL("%s: Unknown fp_f reg id: 0x%llx", config_name(c), id);
+	return NULL;
+}
+
+static const char *fp_d_id_to_str(struct vcpu_config *c, __u64 id)
+{
+	__u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_FP_D);
+
+	/*
+	 * reg_off is the offset into struct __riscv_d_ext_state
+	 */
+	switch (reg_off) {
+	case KVM_REG_RISCV_FP_D_REG(f[0]) ...
+	     KVM_REG_RISCV_FP_D_REG(f[31]):
+		return str_with_index("KVM_REG_RISCV_FP_D_REG(f[##])", reg_off);
+	case KVM_REG_RISCV_FP_D_REG(fcsr):
+		return "KVM_REG_RISCV_FP_D_REG(fcsr)";
+	}
+
+	TEST_FAIL("%s: Unknown fp_d reg id: 0x%llx", config_name(c), id);
+	return NULL;
+}
+
+static const char *isa_ext_id_to_str(struct vcpu_config *c, __u64 id)
+{
+	/*
+	 * reg_off is the offset into unsigned long kvm_isa_ext_arr[]
+	 */
+	__u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_ISA_EXT);
+
+	static const char * const kvm_isa_ext_reg_name[] = {
+		"KVM_RISCV_ISA_EXT_A",
+		"KVM_RISCV_ISA_EXT_C",
+		"KVM_RISCV_ISA_EXT_D",
+		"KVM_RISCV_ISA_EXT_F",
+		"KVM_RISCV_ISA_EXT_H",
+		"KVM_RISCV_ISA_EXT_I",
+		"KVM_RISCV_ISA_EXT_M",
+		"KVM_RISCV_ISA_EXT_SVPBMT",
+		"KVM_RISCV_ISA_EXT_SSTC",
+		"KVM_RISCV_ISA_EXT_SVINVAL",
+		"KVM_RISCV_ISA_EXT_ZIHINTPAUSE",
+		"KVM_RISCV_ISA_EXT_ZICBOM",
+		"KVM_RISCV_ISA_EXT_ZICBOZ",

tools/testing/selftests/kvm/riscv/get-reg-list.c line 321

Quoted 10 lines

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

+		"KVM_RISCV_ISA_EXT_ZBB",
+		"KVM_RISCV_ISA_EXT_SSAIA",
+	};
+
+	if (reg_off >= ARRAY_SIZE(kvm_isa_ext_reg_name)) {
+		/* isa_ext regs would grow regularly with new isa extension added, so
+		 * just show "##UNKNOWN##" to indicate a new extension.
+		 */
+		printf("%s: Unsupported isa ext reg id 0x%llx", config_name(c), id);
+		return "##UNKNOWN##";

This should be

  return str_with_index("##", id);

so it can compile.

Quoted 18 lines

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

+	}
+
+	return kvm_isa_ext_reg_name[reg_off];
+}
+
+static const char *sbi_ext_single_id_to_str(__u64 reg_off)
+{
+	/*
+	 * reg_off is KVM_RISCV_SBI_EXT_ID
+	 */
+	static const char * const kvm_sbi_ext_reg_name[] = {
+		"KVM_RISCV_SBI_EXT_V01",
+		"KVM_RISCV_SBI_EXT_TIME",
+		"KVM_RISCV_SBI_EXT_IPI",
+		"KVM_RISCV_SBI_EXT_RFENCE",
+		"KVM_RISCV_SBI_EXT_SRST",
+		"KVM_RISCV_SBI_EXT_HSM",
+		"KVM_RISCV_SBI_EXT_PMU",

tools/testing/selftests/kvm/riscv/get-reg-list.c line 349

Quoted 10 lines

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

+		"KVM_RISCV_SBI_EXT_EXPERIMENTAL",
+		"KVM_RISCV_SBI_EXT_VENDOR",
+	};
+
+	if (reg_off >= KVM_RISCV_SBI_EXT_MAX) {
+		/* sbi_ext regs would grow regularly with new sbi extension added, so
+		 * just show "##UNKNOWN##" to indicate a new extension.
+		 */
+		printf("Unsupported sbi ext single reg 0x%llx", reg_off);
+		return "##UNKNOWN##";

  return str_with_index("##", id);

tools/testing/selftests/kvm/riscv/get-reg-list.c line 359

Quoted 10 lines

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

+	}
+
+	return kvm_sbi_ext_reg_name[reg_off];
+}
+
+static const char *sbi_ext_multi_id_to_str(__u64 reg_subtype, __u64 reg_off)
+{
+	if (reg_off > KVM_REG_RISCV_SBI_MULTI_REG_LAST) {
+		/* sbi_ext regs would grow regularly with new sbi extension added, so
+		 * just show "##UNKNOWN##" to indicate a new extension.

It'll only grow every 64 SBI extensions, so it won't be very regular.

tools/testing/selftests/kvm/riscv/get-reg-list.c line 367

Quoted 8 lines

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

+		 */
+		printf("Unsupported sbi ext multi reg: 0x%llx", reg_off);
+		return "##UNKNOWN##";
+	}
+
+	switch (reg_subtype) {
+	case KVM_REG_RISCV_SBI_MULTI_EN:
+		return str_with_index("KVM_RISCV_SBI_MULTI_EN[##]", reg_off);

KVM_RISCV_SBI_MULTI_EN[] doesn't exist, so that won't compile. This should be

str_with_index("KVM_REG_RISCV_SBI_MULTI_EN | ##", reg_off)

tools/testing/selftests/kvm/riscv/get-reg-list.c line 369

Quoted 2 lines

tools/testing/selftests/kvm/riscv/get-reg-list.c new file +2 -0
+	case KVM_REG_RISCV_SBI_MULTI_DIS:
+		return str_with_index("KVM_RISCV_SBI_MULTI_DIS[##]", reg_off);

str_with_index("KVM_REG_RISCV_SBI_MULTI_DIS | ##", reg_off)

Quoted 11 lines

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

+	}
+
+	return NULL;
+}
+
+static const char *sbi_ext_id_to_str(struct vcpu_config *c, __u64 id)
+{
+	__u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_SBI_EXT);
+	__u64 reg_subtype = reg_off & KVM_REG_RISCV_SUBTYPE_MASK;
+
+	reg_off &= ~KVM_REG_RISCV_SUBTYPE_MASK;

tools/testing/selftests/kvm/riscv/get-reg-list.c line 390

Quoted 10 lines

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

+
+	switch (reg_subtype) {
+	case KVM_REG_RISCV_SBI_SINGLE:
+		return sbi_ext_single_id_to_str(reg_off);
+	case KVM_REG_RISCV_SBI_MULTI_EN:
+	case KVM_REG_RISCV_SBI_MULTI_DIS:
+		return sbi_ext_multi_id_to_str(reg_subtype, reg_off);
+	}
+
+	TEST_FAIL("%s: Unknown sbi ext reg id: 0x%llx", config_name(c), id);

Maybe better to report "unknown subtype"?

Quoted 294 lines

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

+	return NULL;
+}
+
+static void print_reg(struct vcpu_config *c, __u64 id)
+{
+	const char *reg_size = NULL;
+
+	TEST_ASSERT((id & KVM_REG_ARCH_MASK) == KVM_REG_RISCV,
+		    "%s: KVM_REG_RISCV missing in reg id: 0x%llx", config_name(c), id);
+
+	switch (id & KVM_REG_SIZE_MASK) {
+	case KVM_REG_SIZE_U32:
+		reg_size = "KVM_REG_SIZE_U32";
+		break;
+	case KVM_REG_SIZE_U64:
+		reg_size = "KVM_REG_SIZE_U64";
+		break;
+	case KVM_REG_SIZE_U128:
+		reg_size = "KVM_REG_SIZE_U128";
+		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);
+	}
+
+	switch (id & KVM_REG_RISCV_TYPE_MASK) {
+	case KVM_REG_RISCV_CONFIG:
+		printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_CONFIG | %s,\n",
+				reg_size, config_id_to_str(c, id));
+		break;
+	case KVM_REG_RISCV_CORE:
+		printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_CORE | %s,\n",
+				reg_size, core_id_to_str(c, id));
+		break;
+	case KVM_REG_RISCV_CSR:
+		printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_CSR | %s,\n",
+				reg_size, csr_id_to_str(c, id));
+		break;
+	case KVM_REG_RISCV_TIMER:
+		printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_TIMER | %s,\n",
+				reg_size, timer_id_to_str(c, id));
+		break;
+	case KVM_REG_RISCV_FP_F:
+		printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_FP_F | %s,\n",
+				reg_size, fp_f_id_to_str(c, id));
+		break;
+	case KVM_REG_RISCV_FP_D:
+		printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_FP_D | %s,\n",
+				reg_size, fp_d_id_to_str(c, id));
+		break;
+	case KVM_REG_RISCV_ISA_EXT:
+		printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_ISA_EXT | %s,\n",
+				reg_size, isa_ext_id_to_str(c, id));
+		break;
+	case KVM_REG_RISCV_SBI_EXT:
+		printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_SBI_EXT | %s,\n",
+				reg_size, sbi_ext_id_to_str(c, id));
+		break;
+	default:
+		TEST_FAIL("%s: Unexpected reg type: 0x%llx in reg id: 0x%llx", config_name(c),
+				(id & KVM_REG_RISCV_TYPE_MASK) >> KVM_REG_RISCV_TYPE_SHIFT, id);
+	}
+}
+
+static inline bool vcpu_has_ext(struct kvm_vcpu *vcpu, long cap)
+{
+	int ret;
+	unsigned long value;
+
+	ret = __vcpu_get_reg(vcpu, RISCV_ISA_EXT_REG(cap), &value);
+	if (ret) {
+		printf("Failed to get cap %ld", cap);
+		return false;
+	}
+
+	return !!value;
+}
+
+static void check_supported(struct kvm_vcpu *vcpu, struct vcpu_config *c)
+{
+	struct reg_sublist *s;
+
+	for_each_sublist(c, s) {
+		if (!s->capability)
+			continue;
+
+		__TEST_REQUIRE(vcpu_has_ext(vcpu, s->capability),
+			       "%s: %s not available, skipping tests\n",
+			       config_name(c), s->name);
+	}
+}
+
+static bool print_list;
+
+static void run_test(struct vcpu_config *c)
+{
+	int new_regs = 0, missing_regs = 0, i, n;
+	int failed_get = 0, failed_set = 0;
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	struct reg_sublist *s;
+
+	vm = vm_create_barebones();
+	vcpu = __vm_vcpu_add(vm, 0);
+
+	check_supported(vcpu, c);
+
+	reg_list = vcpu_get_reg_list(vcpu);
+
+	if (print_list) {
+		putchar('\n');
+		for_each_reg(i) {
+			__u64 id = reg_list->reg[i];
+
+			if (print_list)
+				print_reg(c, id);
+		}
+		putchar('\n');
+		return;
+	}
+
+	/*
+	 * We only test that we can get the register and then write back the
+	 * same value.
+	 */
+	for_each_reg(i) {
+		uint8_t addr[128 / 8];
+		struct kvm_one_reg reg = {
+			.id = reg_list->reg[i],
+			.addr = (__u64)&addr,
+		};
+		bool reject_reg = false;
+		int ret;
+
+		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);
+			putchar('\n');
+			++failed_get;
+		}
+
+		/* rejects_set registers are skipped for setting test */
+		for_each_sublist(c, s) {
+			if (s->rejects_set && find_reg(s->rejects_set, s->rejects_set_n, reg.id)) {
+				reject_reg = true;
+				break;
+			}
+		}
+
+		if (!reject_reg) {
+			ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);
+			/* Registers that don't support set operation should pass */
+			if (ret && errno != EOPNOTSUPP) {
+				printf("%s: Failed to set ", config_name(c));
+				print_reg(c, reg.id);
+				putchar('\n');
+				++failed_set;
+			}
+		}
+	}
+
+	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;
+
+	for_each_missing_reg(i)
+		++missing_regs;
+
+	if (new_regs || missing_regs) {
+		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("\n%s: There are %d new registers.\n"
+		       "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]);
+		putchar('\n');
+	}
+
+	if (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(c, blessed_reg[i]);
+		putchar('\n');
+	}
+
+	TEST_ASSERT(!missing_regs && !failed_get && !failed_set,
+		    "%s: There are %d missing registers; "
+		    "%d registers failed get; %d registers failed set.",
+		    config_name(c), missing_regs, failed_get, failed_set);
+
+	pr_info("%s: PASS\n", config_name(c));
+	blessed_n = 0;
+	free(blessed_reg);
+	free(reg_list);
+	kvm_vm_free(vm);
+}
+
+static void help(void)
+{
+	struct vcpu_config *c;
+	int i;
+
+	printf(
+	"\n"
+	"usage: get-reg-list [--config=<selection>] [--list]\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"
+	"\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;
+	int i, ret = 0;
+	pid_t pid;
+
+	for (i = 1; i < ac; ++i) {
+		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], "--help") == 0 || strcmp(av[1], "-h") == 0)
+			help(), exit(0);
+		else
+			help(), exit(1);
+	}
+
+	if (print_list) {
+		/*
+		 * We only want to print the register list of a single config.
+		 */
+		if (!sel)
+			help(), exit(1);
+	}
+
+	for (i = 0; i < vcpu_configs_n; ++i) {
+		c = vcpu_configs[i];
+		if (sel && c != sel)
+			continue;
+
+		pid = fork();
+
+		if (!pid) {
+			run_test(c);
+			exit(0);
+		} else {
+			int wstatus;

tools/testing/selftests/kvm/riscv/get-reg-list.c line 694

Quoted 10 lines

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

+			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 ret;
+}

We should share all the code above, except print_reg(), with aarch64. I'll send a patch series that splits the arch-neutral code out of the aarch64 test that you can base this test on.

Quoted 60 lines

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

+
+/*
+ * The current blessed list was primed with the output of kernel version
+ * v6.2 and then later updated with new registers.
+ */
+static __u64 base_regs[] = {
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(isa),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(zicbom_block_size),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(mvendorid),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(marchid),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(mimpid),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.pc),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.ra),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.sp),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.gp),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.tp),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t0),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t1),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t2),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s0),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s1),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a0),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a1),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a2),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a3),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a4),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a5),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a6),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a7),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s2),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s3),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s4),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s5),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s6),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s7),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s8),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s9),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s10),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s11),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t3),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t4),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t5),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t6),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(mode),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(sstatus),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(sie),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(stvec),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(sscratch),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(sepc),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(scause),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(stval),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(sip),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(satp),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(scounteren),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(frequency),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(time),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(compare),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(state),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_A,
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_C,

tools/testing/selftests/kvm/riscv/get-reg-list.c line 764

Quoted 10 lines

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

+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_D,
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_F,
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_H,
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_I,
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_M,
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVPBMT,
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SSTC,
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVINVAL,
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICBOM,

Any register that depends on the host having the extension, like zicbom, means it shouldn't be in the base register set.

Quoted 108 lines

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

+};
+
+/*
+ * The rejects_set list registers that should skip set test.
+ *  - KVM_REG_RISCV_TIMER_REG(state): set would fail if it was not initialized properly.
+ */
+static __u64 base_rejects_set[] = {
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(state),
+};
+
+static __u64 fp_f_regs[] = {
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[0]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[1]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[2]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[3]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[4]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[5]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[6]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[7]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[8]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[9]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[10]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[11]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[12]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[13]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[14]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[15]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[16]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[17]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[18]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[19]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[20]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[21]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[22]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[23]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[24]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[25]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[26]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[27]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[28]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[29]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[30]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[31]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(fcsr),
+};
+
+static __u64 fp_d_regs[] = {
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[0]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[1]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[2]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[3]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[4]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[5]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[6]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[7]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[8]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[9]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[10]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[11]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[12]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[13]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[14]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[15]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[16]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[17]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[18]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[19]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[20]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[21]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[22]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[23]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[24]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[25]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[26]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[27]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[28]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[29]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[30]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[31]),
+	KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(fcsr),
+};
+
+#define BASE_SUBLIST \
+	{"base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), .rejects_set = base_rejects_set, \
+		.rejects_set_n = ARRAY_SIZE(base_rejects_set),}
+#define FP_F_REGS_SUBLIST \
+	{"fp_f_regs", .capability = KVM_RISCV_ISA_EXT_F, .regs = fp_f_regs, \
+		.regs_n = ARRAY_SIZE(fp_f_regs),}
+#define FP_D_REGS_SUBLIST \
+	{"fp_d_regs", .capability = KVM_RISCV_ISA_EXT_D, .regs = fp_d_regs, \
+		.regs_n = ARRAY_SIZE(fp_d_regs),}
+
+static struct vcpu_config fp_f_d_config = {
+	.sublists = {
+	BASE_SUBLIST,
+	FP_F_REGS_SUBLIST,
+	FP_D_REGS_SUBLIST,
+	{0},
+	},
+};
+
+static struct vcpu_config *vcpu_configs[] = {
+	&fp_f_d_config,
+};
+static int vcpu_configs_n = ARRAY_SIZE(vcpu_configs);

Signature · 1 line

2.34.1

Thanks,
drew

Haibo Xu

2023-05-18 04:17 UTC #11 lore raw

Re: [PATCH 2/2] KVM: selftests: Add riscv get-reg-list test

Quoted 11 lines · On Wed, May 17, 2023 at 7:06?PM Andrew Jones <[email protected]> wrote:

Quoted 4 lines · 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.2.
v6.2? But get-reg-list is getting introduced with this patch, so it'll land in a 6.4 rc at the earliest. I don't see how can we generate the initial blessed list with anything earlier?

Thanks for pointing this out!

The story is, initially, the patch was developed and verified based on our local v6.2 kernel tree.
According to my understanding, the blessed list should include the minimum set of registers that
a platform should support(pls correct me if I was wrong) and registers in v6.2 seem to meet the
requirement, so I chose the blessed list based on it.

To be frank, the comments above was a little misleading and I'll change it to v6.4 since both v6.2
and v6.4 do have similar registers support.

Quoted 38 lines

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

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

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

index 7a5ff646e7e7..fbb2e984297a 100644
@@ -13,6 +13,8 @@ else ifeq ($(ARCH),arm64)
      ARCH_DIR := aarch64
 else ifeq ($(ARCH),s390)
      ARCH_DIR := s390x
+else ifeq ($(ARCH),riscv)
+     ARCH_DIR := riscv
 else
      ARCH_DIR := $(ARCH)
 endif

@@ -171,6 +173,7 @@ TEST_GEN_PROGS_s390x += rseq_test
 TEST_GEN_PROGS_s390x += set_memory_region_test
 TEST_GEN_PROGS_s390x += kvm_binary_stats_test
 
+TEST_GEN_PROGS_riscv += riscv/get-reg-list
 TEST_GEN_PROGS_riscv += demand_paging_test
 TEST_GEN_PROGS_riscv += dirty_log_test
 TEST_GEN_PROGS_riscv += kvm_create_max_vcpus

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

index d00d213c3805..0adf9cb34126 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)

Quoted 5 lines

tools/testing/selftests/kvm/include/riscv/processor.h static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx, · line 42
Quoted 2 lines
tools/testing/selftests/kvm/include/riscv/processor.h +2 -0
+#define RISCV_ISA_EXT_REG(idx)       __kvm_reg_id(KVM_REG_RISCV_ISA_EXT, \
+                                          idx, KVM_REG_SIZE_U64)
^ KVM_REG_SIZE_ULONG

Will fix it in next version, thanks!

Quoted 146 lines

Quoted 130 lines

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

+
 /* 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 +120 -0

new file mode 100644
index 000000000000..dec1df0c6e72
@@ -0,0 +1,869 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Check for KVM_GET_REG_LIST regressions.
+ *
+ * Copyright (C) 2020, Red Hat, Inc.
+ * Copyright (c) 2023 Intel Corporation
+ *
+ * The test was port from the get-reg-list.c on aarch64.
+ */
+#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"
+
+static struct kvm_reg_list *reg_list;
+static __u64 *blessed_reg, blessed_n;
+
+struct reg_sublist {
+     const char *name;
+     long capability;
+     __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 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))
+
+#define for_each_missing_reg(i)                                              \
+     for ((i) = 0; (i) < blessed_n; ++(i))                   \
+             if (!find_reg(reg_list->reg, reg_list->n, blessed_reg[i]))
+
+#define for_each_new_reg(i)                                                  \
+     for_each_reg(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;
+
+     if (c->name)
+             return c->name;
+
+     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 find_reg(__u64 regs[], __u64 nr_regs, __u64 reg)
+{
+     int i;
+
+     for (i = 0; i < nr_regs; ++i)
+             if (reg == regs[i])
+                     return true;
+     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)
+
+static const char *config_id_to_str(struct vcpu_config *c, __u64 id)
+{
+     __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_CONFIG);
+
+     /*
+      * reg_off is the offset into struct kvm_riscv_config
+      */
+     switch (reg_off) {
+     case KVM_REG_RISCV_CONFIG_REG(isa):
+             return "KVM_REG_RISCV_CONFIG_REG(isa)";
+     case KVM_REG_RISCV_CONFIG_REG(zicbom_block_size):
+             return "KVM_REG_RISCV_CONFIG_REG(zicbom_block_size)";
+     case KVM_REG_RISCV_CONFIG_REG(zicboz_block_size):
+             return "KVM_REG_RISCV_CONFIG_REG(zicboz_block_size)";
+     case KVM_REG_RISCV_CONFIG_REG(mvendorid):
+             return "KVM_REG_RISCV_CONFIG_REG(mvendorid)";

tools/testing/selftests/kvm/riscv/get-reg-list.c line 130

Quoted 10 lines

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

+     case KVM_REG_RISCV_CONFIG_REG(marchid):
+             return "KVM_REG_RISCV_CONFIG_REG(marchid)";
+     case KVM_REG_RISCV_CONFIG_REG(mimpid):
+             return "KVM_REG_RISCV_CONFIG_REG(mimpid)";
+     default:
+             /* Config regs would grow regularly with new pseudo reg added, so
+              * just show "##UNKNOWN##" to indicate a new pseudo config reg.
+              */
+             printf("%s: Unknown config reg id: 0x%llx", config_name(c), id);
+             return "##UNKNOWN##";

Quoted 2 lines

str_with_index("KVM_REG_RISCV_CONFIG_REG(##)", reg_off)

Thanks for sharing the detailed idea, will fix it in next version!

Quoted 35 lines

Quoted 22 lines

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

+     }
+}
+
+static const char *core_id_to_str(struct vcpu_config *c, __u64 id)
+{
+     __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_CORE);
+
+     /*
+      * reg_off is the offset into struct kvm_riscv_core
+      */
+     switch (reg_off) {
+     case KVM_REG_RISCV_CORE_REG(regs.pc):
+             return "KVM_REG_RISCV_CORE_REG(regs.pc)";
+     case KVM_REG_RISCV_CORE_REG(regs.ra) ...
+          KVM_REG_RISCV_CORE_REG(regs.t6):
+             return str_with_index("KVM_REG_RISCV_CORE_REG(regs.regs[##])", reg_off);
+     case KVM_REG_RISCV_CORE_REG(mode):
+             return "KVM_REG_RISCV_CORE_REG(mode)";
+     }
+
+     TEST_FAIL("%s: Unknown core reg id: 0x%llx", config_name(c), id);
+     return NULL;

tools/testing/selftests/kvm/riscv/get-reg-list.c line 162

Quoted 10 lines

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

+}
+
+static const char *general_csr_id_to_str(__u64 reg_off)
+{
+     /*
+      * reg_off is the offset into struct kvm_riscv_csr
+      */
+     switch (reg_off) {
+     case KVM_REG_RISCV_CSR_REG(sstatus):
+             return "KVM_REG_RISCV_CSR_REG(sstatus)";

We also need the subtype. So this should be something like

Quoted 7 lines

 #define CSR_GENERAL(csr) \
     "KVM_REG_RISCV_CSR_GENERAL | KVM_REG_RISCV_CSR_REG(" #csr ")"

  return CSR_GENERAL(sstatus);

and similar below for the AIA CSRs.

Sure, thanks for the suggestion!

Quoted 22 lines

Quoted 11 lines

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

+     case KVM_REG_RISCV_CSR_REG(sie):
+             return "KVM_REG_RISCV_CSR_REG(sie)";
+     case KVM_REG_RISCV_CSR_REG(stvec):
+             return "KVM_REG_RISCV_CSR_REG(stvec)";
+     case KVM_REG_RISCV_CSR_REG(sscratch):
+             return "KVM_REG_RISCV_CSR_REG(sscratch)";
+     case KVM_REG_RISCV_CSR_REG(sepc):
+             return "KVM_REG_RISCV_CSR_REG(sepc)";
+     case KVM_REG_RISCV_CSR_REG(scause):
+             return "KVM_REG_RISCV_CSR_REG(scause)";
+     case KVM_REG_RISCV_CSR_REG(stval):

tools/testing/selftests/kvm/riscv/get-reg-list.c line 183

Quoted 10 lines

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

+             return "KVM_REG_RISCV_CSR_REG(stval)";
+     case KVM_REG_RISCV_CSR_REG(sip):
+             return "KVM_REG_RISCV_CSR_REG(sip)";
+     case KVM_REG_RISCV_CSR_REG(satp):
+             return "KVM_REG_RISCV_CSR_REG(satp)";
+     case KVM_REG_RISCV_CSR_REG(scounteren):
+             return "KVM_REG_RISCV_CSR_REG(scounteren)";
+     default:
+             TEST_FAIL("Unknown general csr reg: 0x%llx", reg_off);
+             return NULL;

Quoted 4 lines

nit: It'd be nice to have a pattern for this TEST_FAIL. Either outside the switch and not have a default or always the default. It seems to be changing function by function.

Will unify the pattern in next version.

Quoted 37 lines

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

+     }
+}
+
+static const char *aia_csr_id_to_str(__u64 reg_off)
+{
+     /*
+      * reg_off is the offset into struct kvm_riscv_aia_csr
+      */
+     switch (reg_off) {
+     case KVM_REG_RISCV_CSR_AIA_REG(siselect):
+             return "KVM_REG_RISCV_CSR_AIA_REG(siselect)";
+     case KVM_REG_RISCV_CSR_AIA_REG(iprio1):
+             return "KVM_REG_RISCV_CSR_AIA_REG(iprio1)";
+     case KVM_REG_RISCV_CSR_AIA_REG(iprio2):
+             return "KVM_REG_RISCV_CSR_AIA_REG(iprio2)";
+     case KVM_REG_RISCV_CSR_AIA_REG(sieh):
+             return "KVM_REG_RISCV_CSR_AIA_REG(sieh)";
+     case KVM_REG_RISCV_CSR_AIA_REG(siph):
+             return "KVM_REG_RISCV_CSR_AIA_REG(siph)";
+     case KVM_REG_RISCV_CSR_AIA_REG(iprio1h):
+             return "KVM_REG_RISCV_CSR_AIA_REG(iprio1h)";
+     case KVM_REG_RISCV_CSR_AIA_REG(iprio2h):
+             return "KVM_REG_RISCV_CSR_AIA_REG(iprio2h)";
+     default:
+             TEST_FAIL("Unknown aia csr reg: 0x%llx", reg_off);
+             return NULL;
+     }
+}
+
+static const char *csr_id_to_str(struct vcpu_config *c, __u64 id)
+{
+     __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_CSR);
+     __u64 reg_subtype = reg_off & KVM_REG_RISCV_SUBTYPE_MASK;
+
+     reg_off &= ~KVM_REG_RISCV_SUBTYPE_MASK;
+
+     switch (reg_subtype) {

Quoted 9 lines

tools/testing/selftests/kvm/riscv/get-reg-list.c line 226

Quoted 6 lines

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

+     case KVM_REG_RISCV_CSR_GENERAL:
+             return general_csr_id_to_str(reg_off);
+     case KVM_REG_RISCV_CSR_AIA:
+             return aia_csr_id_to_str(reg_off);
+     default:
+             TEST_FAIL("%s: Unknown csr reg id: 0x%llx", config_name(c), id);

Maybe better to report "unknown subtype"?

Yes, the default should cover all unknown subtype.

Quoted 98 lines

Quoted 85 lines

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

+             return NULL;
+     }
+}
+
+static const char *timer_id_to_str(struct vcpu_config *c, __u64 id)
+{
+     __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_TIMER);
+
+     /*
+      * reg_off is the offset into struct kvm_riscv_timer
+      */
+     switch (reg_off) {
+     case KVM_REG_RISCV_TIMER_REG(frequency):
+             return "KVM_REG_RISCV_TIMER_REG(frequency)";
+     case KVM_REG_RISCV_TIMER_REG(time):
+             return "KVM_REG_RISCV_TIMER_REG(time)";
+     case KVM_REG_RISCV_TIMER_REG(compare):
+             return "KVM_REG_RISCV_TIMER_REG(compare)";
+     case KVM_REG_RISCV_TIMER_REG(state):
+             return "KVM_REG_RISCV_TIMER_REG(state)";
+     }
+
+     TEST_FAIL("%s: Unknown timer reg id: 0x%llx", config_name(c), id);
+     return NULL;
+}
+
+static const char *fp_f_id_to_str(struct vcpu_config *c, __u64 id)
+{
+     __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_FP_F);
+
+     /*
+      * reg_off is the offset into struct __riscv_f_ext_state
+      */
+     switch (reg_off) {
+     case KVM_REG_RISCV_FP_F_REG(f[0]) ...
+          KVM_REG_RISCV_FP_F_REG(f[31]):
+             return str_with_index("KVM_REG_RISCV_FP_F_REG(f[##])", reg_off);
+     case KVM_REG_RISCV_FP_F_REG(fcsr):
+             return "KVM_REG_RISCV_FP_F_REG(fcsr)";
+     }
+
+     TEST_FAIL("%s: Unknown fp_f reg id: 0x%llx", config_name(c), id);
+     return NULL;
+}
+
+static const char *fp_d_id_to_str(struct vcpu_config *c, __u64 id)
+{
+     __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_FP_D);
+
+     /*
+      * reg_off is the offset into struct __riscv_d_ext_state
+      */
+     switch (reg_off) {
+     case KVM_REG_RISCV_FP_D_REG(f[0]) ...
+          KVM_REG_RISCV_FP_D_REG(f[31]):
+             return str_with_index("KVM_REG_RISCV_FP_D_REG(f[##])", reg_off);
+     case KVM_REG_RISCV_FP_D_REG(fcsr):
+             return "KVM_REG_RISCV_FP_D_REG(fcsr)";
+     }
+
+     TEST_FAIL("%s: Unknown fp_d reg id: 0x%llx", config_name(c), id);
+     return NULL;
+}
+
+static const char *isa_ext_id_to_str(struct vcpu_config *c, __u64 id)
+{
+     /*
+      * reg_off is the offset into unsigned long kvm_isa_ext_arr[]
+      */
+     __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_ISA_EXT);
+
+     static const char * const kvm_isa_ext_reg_name[] = {
+             "KVM_RISCV_ISA_EXT_A",
+             "KVM_RISCV_ISA_EXT_C",
+             "KVM_RISCV_ISA_EXT_D",
+             "KVM_RISCV_ISA_EXT_F",
+             "KVM_RISCV_ISA_EXT_H",
+             "KVM_RISCV_ISA_EXT_I",
+             "KVM_RISCV_ISA_EXT_M",
+             "KVM_RISCV_ISA_EXT_SVPBMT",
+             "KVM_RISCV_ISA_EXT_SSTC",
+             "KVM_RISCV_ISA_EXT_SVINVAL",
+             "KVM_RISCV_ISA_EXT_ZIHINTPAUSE",
+             "KVM_RISCV_ISA_EXT_ZICBOM",
+             "KVM_RISCV_ISA_EXT_ZICBOZ",

tools/testing/selftests/kvm/riscv/get-reg-list.c line 321

Quoted 10 lines

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

+             "KVM_RISCV_ISA_EXT_ZBB",
+             "KVM_RISCV_ISA_EXT_SSAIA",
+     };
+
+     if (reg_off >= ARRAY_SIZE(kvm_isa_ext_reg_name)) {
+             /* isa_ext regs would grow regularly with new isa extension added, so
+              * just show "##UNKNOWN##" to indicate a new extension.
+              */
+             printf("%s: Unsupported isa ext reg id 0x%llx", config_name(c), id);
+             return "##UNKNOWN##";

This should be

Quoted 4 lines

  return str_with_index("##", id);
so it can compile.

Sure, thanks!

Quoted 22 lines

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

+     }
+
+     return kvm_isa_ext_reg_name[reg_off];
+}
+
+static const char *sbi_ext_single_id_to_str(__u64 reg_off)
+{
+     /*
+      * reg_off is KVM_RISCV_SBI_EXT_ID
+      */
+     static const char * const kvm_sbi_ext_reg_name[] = {
+             "KVM_RISCV_SBI_EXT_V01",
+             "KVM_RISCV_SBI_EXT_TIME",
+             "KVM_RISCV_SBI_EXT_IPI",
+             "KVM_RISCV_SBI_EXT_RFENCE",
+             "KVM_RISCV_SBI_EXT_SRST",
+             "KVM_RISCV_SBI_EXT_HSM",
+             "KVM_RISCV_SBI_EXT_PMU",
+             "KVM_RISCV_SBI_EXT_EXPERIMENTAL",
+             "KVM_RISCV_SBI_EXT_VENDOR",
+     };
+

Quoted 9 lines

tools/testing/selftests/kvm/riscv/get-reg-list.c line 349

Quoted 6 lines

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

+     if (reg_off >= KVM_RISCV_SBI_EXT_MAX) {
+             /* sbi_ext regs would grow regularly with new sbi extension added, so
+              * just show "##UNKNOWN##" to indicate a new extension.
+              */
+             printf("Unsupported sbi ext single reg 0x%llx", reg_off);
+             return "##UNKNOWN##";

  return str_with_index("##", id);

Sure, thanks!

Quoted 4 lines

Quoted 4 lines
tools/testing/selftests/kvm/riscv/get-reg-list.c new file +4 -0
+     }
+
+     return kvm_sbi_ext_reg_name[reg_off];
+}

Quoted 9 lines

tools/testing/selftests/kvm/riscv/get-reg-list.c line 359
Quoted 6 lines
tools/testing/selftests/kvm/riscv/get-reg-list.c new file +6 -0
+
+static const char *sbi_ext_multi_id_to_str(__u64 reg_subtype, __u64 reg_off)
+{
+     if (reg_off > KVM_REG_RISCV_SBI_MULTI_REG_LAST) {
+             /* sbi_ext regs would grow regularly with new sbi extension added, so
+              * just show "##UNKNOWN##" to indicate a new extension.
It'll only grow every 64 SBI extensions, so it won't be very regular.

Yes, just in case for any illegal value.

Quoted 12 lines

tools/testing/selftests/kvm/riscv/get-reg-list.c line 367
Quoted 8 lines
tools/testing/selftests/kvm/riscv/get-reg-list.c new file +8 -0
+              */
+             printf("Unsupported sbi ext multi reg: 0x%llx", reg_off);
+             return "##UNKNOWN##";
+     }
+
+     switch (reg_subtype) {
+     case KVM_REG_RISCV_SBI_MULTI_EN:
+             return str_with_index("KVM_RISCV_SBI_MULTI_EN[##]", reg_off);
KVM_RISCV_SBI_MULTI_EN[] doesn't exist, so that won't compile. This should be

Quoted 7 lines

str_with_index("KVM_REG_RISCV_SBI_MULTI_EN | ##", reg_off)
tools/testing/selftests/kvm/riscv/get-reg-list.c line 369
Quoted 2 lines
tools/testing/selftests/kvm/riscv/get-reg-list.c new file +2 -0
+     case KVM_REG_RISCV_SBI_MULTI_DIS:
+             return str_with_index("KVM_RISCV_SBI_MULTI_DIS[##]", reg_off);
str_with_index("KVM_REG_RISCV_SBI_MULTI_DIS | ##", reg_off)

Sure, thanks!

Quoted 15 lines

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

+     }
+
+     return NULL;
+}
+
+static const char *sbi_ext_id_to_str(struct vcpu_config *c, __u64 id)
+{
+     __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_SBI_EXT);
+     __u64 reg_subtype = reg_off & KVM_REG_RISCV_SUBTYPE_MASK;
+
+     reg_off &= ~KVM_REG_RISCV_SUBTYPE_MASK;
+
+     switch (reg_subtype) {
+     case KVM_REG_RISCV_SBI_SINGLE:
+             return sbi_ext_single_id_to_str(reg_off);

Quoted 9 lines

tools/testing/selftests/kvm/riscv/get-reg-list.c line 390
Quoted 6 lines
tools/testing/selftests/kvm/riscv/get-reg-list.c new file +6 -0
+     case KVM_REG_RISCV_SBI_MULTI_EN:
+     case KVM_REG_RISCV_SBI_MULTI_DIS:
+             return sbi_ext_multi_id_to_str(reg_subtype, reg_off);
+     }
+
+     TEST_FAIL("%s: Unknown sbi ext reg id: 0x%llx", config_name(c), id);
Maybe better to report "unknown subtype"?

Sure, thanks!

Quoted 300 lines

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

+     return NULL;
+}
+
+static void print_reg(struct vcpu_config *c, __u64 id)
+{
+     const char *reg_size = NULL;
+
+     TEST_ASSERT((id & KVM_REG_ARCH_MASK) == KVM_REG_RISCV,
+                 "%s: KVM_REG_RISCV missing in reg id: 0x%llx", config_name(c), id);
+
+     switch (id & KVM_REG_SIZE_MASK) {
+     case KVM_REG_SIZE_U32:
+             reg_size = "KVM_REG_SIZE_U32";
+             break;
+     case KVM_REG_SIZE_U64:
+             reg_size = "KVM_REG_SIZE_U64";
+             break;
+     case KVM_REG_SIZE_U128:
+             reg_size = "KVM_REG_SIZE_U128";
+             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);
+     }
+
+     switch (id & KVM_REG_RISCV_TYPE_MASK) {
+     case KVM_REG_RISCV_CONFIG:
+             printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_CONFIG | %s,\n",
+                             reg_size, config_id_to_str(c, id));
+             break;
+     case KVM_REG_RISCV_CORE:
+             printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_CORE | %s,\n",
+                             reg_size, core_id_to_str(c, id));
+             break;
+     case KVM_REG_RISCV_CSR:
+             printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_CSR | %s,\n",
+                             reg_size, csr_id_to_str(c, id));
+             break;
+     case KVM_REG_RISCV_TIMER:
+             printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_TIMER | %s,\n",
+                             reg_size, timer_id_to_str(c, id));
+             break;
+     case KVM_REG_RISCV_FP_F:
+             printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_FP_F | %s,\n",
+                             reg_size, fp_f_id_to_str(c, id));
+             break;
+     case KVM_REG_RISCV_FP_D:
+             printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_FP_D | %s,\n",
+                             reg_size, fp_d_id_to_str(c, id));
+             break;
+     case KVM_REG_RISCV_ISA_EXT:
+             printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_ISA_EXT | %s,\n",
+                             reg_size, isa_ext_id_to_str(c, id));
+             break;
+     case KVM_REG_RISCV_SBI_EXT:
+             printf("\tKVM_REG_RISCV | %s | KVM_REG_RISCV_SBI_EXT | %s,\n",
+                             reg_size, sbi_ext_id_to_str(c, id));
+             break;
+     default:
+             TEST_FAIL("%s: Unexpected reg type: 0x%llx in reg id: 0x%llx", config_name(c),
+                             (id & KVM_REG_RISCV_TYPE_MASK) >> KVM_REG_RISCV_TYPE_SHIFT, id);
+     }
+}
+
+static inline bool vcpu_has_ext(struct kvm_vcpu *vcpu, long cap)
+{
+     int ret;
+     unsigned long value;
+
+     ret = __vcpu_get_reg(vcpu, RISCV_ISA_EXT_REG(cap), &value);
+     if (ret) {
+             printf("Failed to get cap %ld", cap);
+             return false;
+     }
+
+     return !!value;
+}
+
+static void check_supported(struct kvm_vcpu *vcpu, struct vcpu_config *c)
+{
+     struct reg_sublist *s;
+
+     for_each_sublist(c, s) {
+             if (!s->capability)
+                     continue;
+
+             __TEST_REQUIRE(vcpu_has_ext(vcpu, s->capability),
+                            "%s: %s not available, skipping tests\n",
+                            config_name(c), s->name);
+     }
+}
+
+static bool print_list;
+
+static void run_test(struct vcpu_config *c)
+{
+     int new_regs = 0, missing_regs = 0, i, n;
+     int failed_get = 0, failed_set = 0;
+     struct kvm_vcpu *vcpu;
+     struct kvm_vm *vm;
+     struct reg_sublist *s;
+
+     vm = vm_create_barebones();
+     vcpu = __vm_vcpu_add(vm, 0);
+
+     check_supported(vcpu, c);
+
+     reg_list = vcpu_get_reg_list(vcpu);
+
+     if (print_list) {
+             putchar('\n');
+             for_each_reg(i) {
+                     __u64 id = reg_list->reg[i];
+
+                     if (print_list)
+                             print_reg(c, id);
+             }
+             putchar('\n');
+             return;
+     }
+
+     /*
+      * We only test that we can get the register and then write back the
+      * same value.
+      */
+     for_each_reg(i) {
+             uint8_t addr[128 / 8];
+             struct kvm_one_reg reg = {
+                     .id = reg_list->reg[i],
+                     .addr = (__u64)&addr,
+             };
+             bool reject_reg = false;
+             int ret;
+
+             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);
+                     putchar('\n');
+                     ++failed_get;
+             }
+
+             /* rejects_set registers are skipped for setting test */
+             for_each_sublist(c, s) {
+                     if (s->rejects_set && find_reg(s->rejects_set, s->rejects_set_n, reg.id)) {
+                             reject_reg = true;
+                             break;
+                     }
+             }
+
+             if (!reject_reg) {
+                     ret = __vcpu_ioctl(vcpu, KVM_SET_ONE_REG, &reg);
+                     /* Registers that don't support set operation should pass */
+                     if (ret && errno != EOPNOTSUPP) {
+                             printf("%s: Failed to set ", config_name(c));
+                             print_reg(c, reg.id);
+                             putchar('\n');
+                             ++failed_set;
+                     }
+             }
+     }
+
+     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;
+
+     for_each_missing_reg(i)
+             ++missing_regs;
+
+     if (new_regs || missing_regs) {
+             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("\n%s: There are %d new registers.\n"
+                    "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]);
+             putchar('\n');
+     }
+
+     if (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(c, blessed_reg[i]);
+             putchar('\n');
+     }
+
+     TEST_ASSERT(!missing_regs && !failed_get && !failed_set,
+                 "%s: There are %d missing registers; "
+                 "%d registers failed get; %d registers failed set.",
+                 config_name(c), missing_regs, failed_get, failed_set);
+
+     pr_info("%s: PASS\n", config_name(c));
+     blessed_n = 0;
+     free(blessed_reg);
+     free(reg_list);
+     kvm_vm_free(vm);
+}
+
+static void help(void)
+{
+     struct vcpu_config *c;
+     int i;
+
+     printf(
+     "\n"
+     "usage: get-reg-list [--config=<selection>] [--list]\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"
+     "\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;
+     int i, ret = 0;
+     pid_t pid;
+
+     for (i = 1; i < ac; ++i) {
+             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], "--help") == 0 || strcmp(av[1], "-h") == 0)
+                     help(), exit(0);
+             else
+                     help(), exit(1);
+     }
+
+     if (print_list) {
+             /*
+              * We only want to print the register list of a single config.
+              */
+             if (!sel)
+                     help(), exit(1);
+     }
+
+     for (i = 0; i < vcpu_configs_n; ++i) {
+             c = vcpu_configs[i];
+             if (sel && c != sel)
+                     continue;
+
+             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;
+             }

Quoted 9 lines

tools/testing/selftests/kvm/riscv/get-reg-list.c line 694
Quoted 4 lines
tools/testing/selftests/kvm/riscv/get-reg-list.c new file +4 -0
+     }
+
+     return ret;
+}
We should share all the code above, except print_reg(), with aarch64. I'll send a patch series that splits the arch-neutral code out of the aarch64 test that you can base this test on.

Good idea! I will rebase the patch based on your work.

Quoted 65 lines

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

+
+/*
+ * The current blessed list was primed with the output of kernel version
+ * v6.2 and then later updated with new registers.
+ */
+static __u64 base_regs[] = {
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(isa),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(zicbom_block_size),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(mvendorid),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(marchid),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(mimpid),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.pc),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.ra),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.sp),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.gp),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.tp),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t0),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t1),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t2),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s0),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s1),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a0),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a1),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a2),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a3),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a4),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a5),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a6),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.a7),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s2),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s3),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s4),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s5),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s6),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s7),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s8),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s9),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s10),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.s11),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t3),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t4),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t5),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(regs.t6),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CORE | KVM_REG_RISCV_CORE_REG(mode),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(sstatus),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(sie),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(stvec),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(sscratch),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(sepc),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(scause),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(stval),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(sip),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(satp),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_REG(scounteren),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(frequency),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(time),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(compare),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(state),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_A,
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_C,
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_D,
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_F,
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_H,
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_I,
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_M,

Quoted 9 lines

tools/testing/selftests/kvm/riscv/get-reg-list.c line 764
Quoted 5 lines
tools/testing/selftests/kvm/riscv/get-reg-list.c new file +5 -0
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVPBMT,
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SSTC,
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVINVAL,
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZIHINTPAUSE,
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICBOM,
Any register that depends on the host having the extension, like zicbom, means it shouldn't be in the base register set.

Yes, this kind of registers should not in the base register. Will remove them latter.

Quoted 111 lines

Quoted 106 lines

tools/testing/selftests/kvm/riscv/get-reg-list.c new file +105 -1

+};
+
+/*
+ * The rejects_set list registers that should skip set test.
+ *  - KVM_REG_RISCV_TIMER_REG(state): set would fail if it was not initialized properly.
+ */
+static __u64 base_rejects_set[] = {
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(state),
+};
+
+static __u64 fp_f_regs[] = {
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[0]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[1]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[2]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[3]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[4]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[5]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[6]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[7]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[8]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[9]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[10]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[11]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[12]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[13]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[14]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[15]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[16]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[17]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[18]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[19]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[20]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[21]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[22]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[23]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[24]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[25]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[26]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[27]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[28]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[29]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[30]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(f[31]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_F | KVM_REG_RISCV_FP_F_REG(fcsr),
+};
+
+static __u64 fp_d_regs[] = {
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[0]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[1]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[2]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[3]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[4]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[5]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[6]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[7]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[8]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[9]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[10]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[11]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[12]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[13]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[14]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[15]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[16]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[17]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[18]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[19]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[20]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[21]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[22]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[23]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[24]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[25]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[26]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[27]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[28]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[29]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[30]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(f[31]),
+     KVM_REG_RISCV | KVM_REG_SIZE_U32 | KVM_REG_RISCV_FP_D | KVM_REG_RISCV_FP_D_REG(fcsr),
+};
+
+#define BASE_SUBLIST \
+     {"base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), .rejects_set = base_rejects_set, \
+             .rejects_set_n = ARRAY_SIZE(base_rejects_set),}
+#define FP_F_REGS_SUBLIST \
+     {"fp_f_regs", .capability = KVM_RISCV_ISA_EXT_F, .regs = fp_f_regs, \
+             .regs_n = ARRAY_SIZE(fp_f_regs),}
+#define FP_D_REGS_SUBLIST \
+     {"fp_d_regs", .capability = KVM_RISCV_ISA_EXT_D, .regs = fp_d_regs, \
+             .regs_n = ARRAY_SIZE(fp_d_regs),}
+
+static struct vcpu_config fp_f_d_config = {
+     .sublists = {
+     BASE_SUBLIST,
+     FP_F_REGS_SUBLIST,
+     FP_D_REGS_SUBLIST,
+     {0},
+     },
+};
+
+static struct vcpu_config *vcpu_configs[] = {
+     &fp_f_d_config,
+};
+static int vcpu_configs_n = ARRAY_SIZE(vcpu_configs);
--

Quoted 2 lines

2.34.1

Thanks,
drew

Andrew Jones

2023-05-18 07:32 UTC #12 lore raw

Re: [PATCH 2/2] KVM: selftests: Add riscv get-reg-list test

On Thu, May 18, 2023 at 12:17:18PM +0800, Haibo Xu wrote:
...

Quoted 9 lines

Quoted 7 lines
The idea of these *to_str functions is to dump output that can be copy+pasted into a reg array (hence the trailing commas in print_reg lines). So we can't just print random lines here or return '##UNKOWN##', as that won't compile. Instead, the default should return
str_with_index("KVM_REG_RISCV_CONFIG_REG(##)", reg_off)
Thanks for sharing the detailed idea, will fix it in next version!

I guess we could also return a string like,

"KVM_REG_RISCV_CONFIG_REG(##) /* UNKNOWN */"

as that would still compile and also convey the message that this register doesn't have a name because the test doesn't know it yet.

...

Quoted 7 lines

Quoted 4 lines
We should share all the code above, except print_reg(), with aarch64. I'll send a patch series that splits the arch-neutral code out of the aarch64 test that you can base this test on.
Good idea! I will rebase the patch based on your work.

Ok, I've pushed patches to [1]. This series introduces two things to KVM selftests. Primarily it splits the aarch64/get-reg-list test into a cross-arch get-reg-list test and an $ARCH_DIR/get-reg-list.o object file, which the cross-arch test depends on. To do that, it also introduces the concept of a "split test", a test that has a cross-arch part which depends on an arch-specific part. Using a split test is cleaner than the #ifdeffery we usually do for cross-arch tests.

I've added kvmarm at lists.linux.dev, Marc, Oliver, and Sean to the CC of this message. You'll want to add them when you post v2 as well.

[1] https://github.com/jones-drew/linux/commits/arm64/kself/get-reg-list

Thanks,
drew

Haibo Xu

2023-05-18 08:10 UTC #13 lore raw

Re: [PATCH 2/2] KVM: selftests: Add riscv get-reg-list test

Quoted 11 lines · On Thu, May 18, 2023 at 3:32?PM Andrew Jones <[email protected]> wrote:

On Thu, May 18, 2023 at 12:17:18PM +0800, Haibo Xu wrote:
...
Quoted 8 lines
Quoted 7 lines
The idea of these *to_str functions is to dump output that can be copy+pasted into a reg array (hence the trailing commas in print_reg lines). So we can't just print random lines here or return '##UNKOWN##', as that won't compile. Instead, the default should return
str_with_index("KVM_REG_RISCV_CONFIG_REG(##)", reg_off)

Quoted 9 lines

Quoted 1 line
Thanks for sharing the detailed idea, will fix it in next version!
I guess we could also return a string like,
"KVM_REG_RISCV_CONFIG_REG(##) /* UNKNOWN */"
as that would still compile and also convey the message that this register doesn't have a name because the test doesn't know it yet.

Yes, that's more friendly and self explanatory. Thanks for the suggestion!

Quoted 17 lines

...
Quoted 7 lines
Quoted 4 lines
We should share all the code above, except print_reg(), with aarch64. I'll send a patch series that splits the arch-neutral code out of the aarch64 test that you can base this test on.
Good idea! I will rebase the patch based on your work.
Ok, I've pushed patches to [1]. This series introduces two things to KVM selftests. Primarily it splits the aarch64/get-reg-list test into a cross-arch get-reg-list test and an $ARCH_DIR/get-reg-list.o object file, which the cross-arch test depends on. To do that, it also introduces the concept of a "split test", a test that has a cross-arch part which depends on an arch-specific part. Using a split test is cleaner than the #ifdeffery we usually do for cross-arch tests.

Quoted 3 lines

I've added kvmarm at lists.linux.dev, Marc, Oliver, and Sean to the CC of this message. You'll want to add them when you post v2 as well.

Sure, I will rebase your patch in v2. Thanks for your review!

Quoted 4 lines

[1] https://github.com/jones-drew/linux/commits/arm64/kself/get-reg-list
Thanks,
drew