[PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi

linux-riscv.lists.infradead.org archive mirror
 help / color / mirror / Atom feed

* [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi
@ 2023-12-13 17:09 Andrew Jones
  2023-12-13 17:09 ` [PATCH v2 1/6] RISC-V: KVM: Don't add SBI multi regs in get-reg-list Andrew Jones
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Andrew Jones @ 2023-12-13 17:09 UTC (permalink / raw)
  To: kvm-riscv, linux-riscv; +Cc: anup, atishp, palmer, haibo1.xu

SBI extension UAPI is currently almost the same as the ISA extension
UAPI. This series closes the remaining gap by ensuring when an SBI
extension is not available that its register returns ENOENT when
accessed by userspace. We also drop the SBI multi registers from
get-reg-list (ISA multi registers aren't there either) and make
several improvements to the get-reg-list kselftest.

This series is based on Anup's riscv_kvm_more_exts_v1 branch.

Based on kvm-riscv/riscv_kvm_queue

v2:
 - Rebased on kvm-riscv/riscv_kvm_queue which is based on v6.7-rc5

Thanks,
drew


Andrew Jones (6):
  RISC-V: KVM: Don't add SBI multi regs in get-reg-list
  KVM: riscv: selftests: Drop SBI multi registers
  RISC-V: KVM: Make SBI uapi consistent with ISA uapi
  KVM: riscv: selftests: Add RISCV_SBI_EXT_REG
  KVM: riscv: selftests: Use register subtypes
  RISC-V: KVM: selftests: Treat SBI ext regs like ISA ext regs

 arch/riscv/include/asm/kvm_vcpu_sbi.h         |  10 +-
 arch/riscv/kvm/vcpu_onereg.c                  |  53 ++---
 arch/riscv/kvm/vcpu_sbi.c                     |  75 +++---
 arch/riscv/kvm/vcpu_sbi_replace.c             |   2 +-
 .../selftests/kvm/include/kvm_util_base.h     |   1 +
 .../selftests/kvm/include/riscv/processor.h   |  40 ++--
 .../selftests/kvm/lib/riscv/processor.c       |   4 +-
 .../selftests/kvm/riscv/get-reg-list.c        | 220 +++++++++++++-----
 8 files changed, 254 insertions(+), 151 deletions(-)

-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH v2 1/6] RISC-V: KVM: Don't add SBI multi regs in get-reg-list
  2023-12-13 17:09 [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Andrew Jones
@ 2023-12-13 17:09 ` Andrew Jones
  2023-12-13 17:23   ` Anup Patel
  2023-12-13 17:09 ` [PATCH v2 2/6] KVM: riscv: selftests: Drop SBI multi registers Andrew Jones
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2023-12-13 17:09 UTC (permalink / raw)
  To: kvm-riscv, linux-riscv; +Cc: anup, atishp, palmer, haibo1.xu

The multi regs are derived from the single registers. Only list the
single registers in get-reg-list. This also makes the SBI extension
register listing consistent with the ISA extension register listing.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Haibo Xu <haibo1.xu@intel.com>
---
 arch/riscv/kvm/vcpu_onereg.c | 36 ++----------------------------------
 1 file changed, 2 insertions(+), 34 deletions(-)

diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index f8c9fa0c03c5..f9bfa8a5db21 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -933,20 +933,12 @@ static inline unsigned long num_isa_ext_regs(const struct kvm_vcpu *vcpu)
 
 static inline unsigned long num_sbi_ext_regs(void)
 {
-	/*
-	 * number of KVM_REG_RISCV_SBI_SINGLE +
-	 * 2 x (number of KVM_REG_RISCV_SBI_MULTI)
-	 */
-	return KVM_RISCV_SBI_EXT_MAX + 2*(KVM_REG_RISCV_SBI_MULTI_REG_LAST+1);
+	return KVM_RISCV_SBI_EXT_MAX;
 }
 
 static int copy_sbi_ext_reg_indices(u64 __user *uindices)
 {
-	int n;
-
-	/* copy KVM_REG_RISCV_SBI_SINGLE */
-	n = KVM_RISCV_SBI_EXT_MAX;
-	for (int i = 0; i < n; i++) {
+	for (int i = 0; i < KVM_RISCV_SBI_EXT_MAX; i++) {
 		u64 size = IS_ENABLED(CONFIG_32BIT) ?
 			   KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
 		u64 reg = KVM_REG_RISCV | size | KVM_REG_RISCV_SBI_EXT |
@@ -959,30 +951,6 @@ static int copy_sbi_ext_reg_indices(u64 __user *uindices)
 		}
 	}
 
-	/* copy KVM_REG_RISCV_SBI_MULTI */
-	n = KVM_REG_RISCV_SBI_MULTI_REG_LAST + 1;
-	for (int i = 0; i < n; i++) {
-		u64 size = IS_ENABLED(CONFIG_32BIT) ?
-			   KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
-		u64 reg = KVM_REG_RISCV | size | KVM_REG_RISCV_SBI_EXT |
-			  KVM_REG_RISCV_SBI_MULTI_EN | i;
-
-		if (uindices) {
-			if (put_user(reg, uindices))
-				return -EFAULT;
-			uindices++;
-		}
-
-		reg = KVM_REG_RISCV | size | 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();
 }
 
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH v2 2/6] KVM: riscv: selftests: Drop SBI multi registers
  2023-12-13 17:09 [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Andrew Jones
  2023-12-13 17:09 ` [PATCH v2 1/6] RISC-V: KVM: Don't add SBI multi regs in get-reg-list Andrew Jones
@ 2023-12-13 17:09 ` Andrew Jones
  2023-12-13 17:24   ` Anup Patel
  2023-12-13 17:09 ` [PATCH v2 3/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Andrew Jones
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2023-12-13 17:09 UTC (permalink / raw)
  To: kvm-riscv, linux-riscv; +Cc: anup, atishp, palmer, haibo1.xu

These registers are no longer getting added to get-reg-list.
We keep sbi_ext_multi_id_to_str() for printing, even though
we don't expect it to normally be used, because it may be
useful for debug.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 tools/testing/selftests/kvm/riscv/get-reg-list.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c
index 5d86c761784e..27d07a32a1ef 100644
--- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
+++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
@@ -571,8 +571,6 @@ static __u64 base_regs[] = {
 	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_EXPERIMENTAL,
 	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_VENDOR,
 	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_DBCN,
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_MULTI_EN | 0,
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_MULTI_DIS | 0,
 };
 
 /*
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH v2 3/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi
  2023-12-13 17:09 [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Andrew Jones
  2023-12-13 17:09 ` [PATCH v2 1/6] RISC-V: KVM: Don't add SBI multi regs in get-reg-list Andrew Jones
  2023-12-13 17:09 ` [PATCH v2 2/6] KVM: riscv: selftests: Drop SBI multi registers Andrew Jones
@ 2023-12-13 17:09 ` Andrew Jones
  2023-12-13 17:35   ` Anup Patel
  2023-12-13 17:09 ` [PATCH v2 4/6] KVM: riscv: selftests: Add RISCV_SBI_EXT_REG Andrew Jones
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2023-12-13 17:09 UTC (permalink / raw)
  To: kvm-riscv, linux-riscv; +Cc: anup, atishp, palmer, haibo1.xu

When an SBI extension cannot be enabled, that's a distinct state vs.
enabled and disabled. Modify enum kvm_riscv_sbi_ext_status to
accommodate it, which allows KVM userspace to tell the difference
in state too, as the SBI extension register will disappear when it
cannot be enabled, i.e. accesses to it return ENOENT. get-reg-list is
updated as well to only add SBI extension registers to the list which
may be enabled. Returning ENOENT for SBI extension registers which
cannot be enabled makes them consistent with ISA extension registers.
Any SBI extensions which were enabled by default are still enabled by
default, if they can be enabled at all.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 arch/riscv/include/asm/kvm_vcpu_sbi.h | 10 ++--
 arch/riscv/kvm/vcpu_onereg.c          | 23 +++++---
 arch/riscv/kvm/vcpu_sbi.c             | 75 +++++++++++++++------------
 arch/riscv/kvm/vcpu_sbi_replace.c     |  2 +-
 4 files changed, 65 insertions(+), 45 deletions(-)

diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
index 6a453f7f8b56..bffda0ac59b6 100644
--- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
+++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
@@ -15,9 +15,10 @@
 #define KVM_SBI_VERSION_MINOR 0
 
 enum kvm_riscv_sbi_ext_status {
-	KVM_RISCV_SBI_EXT_UNINITIALIZED,
-	KVM_RISCV_SBI_EXT_AVAILABLE,
-	KVM_RISCV_SBI_EXT_UNAVAILABLE,
+	KVM_RISCV_SBI_EXT_STATUS_UNINITIALIZED,
+	KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE,
+	KVM_RISCV_SBI_EXT_STATUS_ENABLED,
+	KVM_RISCV_SBI_EXT_STATUS_DISABLED,
 };
 
 struct kvm_vcpu_sbi_context {
@@ -36,7 +37,7 @@ struct kvm_vcpu_sbi_extension {
 	unsigned long extid_start;
 	unsigned long extid_end;
 
-	bool default_unavail;
+	bool default_disabled;
 
 	/**
 	 * SBI extension handler. It can be defined for a given extension or group of
@@ -61,6 +62,7 @@ int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu,
 				   const struct kvm_one_reg *reg);
 const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
 				struct kvm_vcpu *vcpu, unsigned long extid);
+bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx);
 int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run);
 void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu);
 
diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
index f9bfa8a5db21..48262be73aa0 100644
--- a/arch/riscv/kvm/vcpu_onereg.c
+++ b/arch/riscv/kvm/vcpu_onereg.c
@@ -931,27 +931,34 @@ static inline unsigned long num_isa_ext_regs(const struct kvm_vcpu *vcpu)
 	return copy_isa_ext_reg_indices(vcpu, NULL);;
 }
 
-static inline unsigned long num_sbi_ext_regs(void)
+static int copy_sbi_ext_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
 {
-	return KVM_RISCV_SBI_EXT_MAX;
-}
+	unsigned int n = 0;
 
-static int copy_sbi_ext_reg_indices(u64 __user *uindices)
-{
 	for (int i = 0; i < KVM_RISCV_SBI_EXT_MAX; i++) {
 		u64 size = IS_ENABLED(CONFIG_32BIT) ?
 			   KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
 		u64 reg = KVM_REG_RISCV | size | KVM_REG_RISCV_SBI_EXT |
 			  KVM_REG_RISCV_SBI_SINGLE | i;
 
+		if (!riscv_vcpu_supports_sbi_ext(vcpu, i))
+			continue;
+
 		if (uindices) {
 			if (put_user(reg, uindices))
 				return -EFAULT;
 			uindices++;
 		}
+
+		n++;
 	}
 
-	return num_sbi_ext_regs();
+	return n;
+}
+
+static unsigned long num_sbi_ext_regs(struct kvm_vcpu *vcpu)
+{
+	return copy_sbi_ext_reg_indices(vcpu, NULL);
 }
 
 /*
@@ -970,7 +977,7 @@ unsigned long kvm_riscv_vcpu_num_regs(struct kvm_vcpu *vcpu)
 	res += num_fp_f_regs(vcpu);
 	res += num_fp_d_regs(vcpu);
 	res += num_isa_ext_regs(vcpu);
-	res += num_sbi_ext_regs();
+	res += num_sbi_ext_regs(vcpu);
 
 	return res;
 }
@@ -1018,7 +1025,7 @@ int kvm_riscv_vcpu_copy_reg_indices(struct kvm_vcpu *vcpu,
 		return ret;
 	uindices += ret;
 
-	ret = copy_sbi_ext_reg_indices(uindices);
+	ret = copy_sbi_ext_reg_indices(vcpu, uindices);
 	if (ret < 0)
 		return ret;
 
diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
index a04ff98085d9..dcdff4458190 100644
--- a/arch/riscv/kvm/vcpu_sbi.c
+++ b/arch/riscv/kvm/vcpu_sbi.c
@@ -80,6 +80,34 @@ static const struct kvm_riscv_sbi_extension_entry sbi_ext[] = {
 	},
 };
 
+static const struct kvm_riscv_sbi_extension_entry *
+riscv_vcpu_get_sbi_ext(struct kvm_vcpu *vcpu, unsigned long idx)
+{
+	const struct kvm_riscv_sbi_extension_entry *sext = NULL;
+
+	if (idx >= KVM_RISCV_SBI_EXT_MAX)
+		return NULL;
+
+	for (int i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
+		if (sbi_ext[i].ext_idx == idx) {
+			sext = &sbi_ext[i];
+			break;
+		}
+	}
+
+	return sext;
+}
+
+bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx)
+{
+	struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
+	const struct kvm_riscv_sbi_extension_entry *sext;
+
+	sext = riscv_vcpu_get_sbi_ext(vcpu, idx);
+
+	return sext && scontext->ext_status[sext->ext_idx] != KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE;
+}
+
 void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run)
 {
 	struct kvm_cpu_context *cp = &vcpu->arch.guest_context;
@@ -140,28 +168,19 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu,
 					 unsigned long reg_num,
 					 unsigned long reg_val)
 {
-	unsigned long i;
-	const struct kvm_riscv_sbi_extension_entry *sext = NULL;
 	struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
-
-	if (reg_num >= KVM_RISCV_SBI_EXT_MAX)
-		return -ENOENT;
+	const struct kvm_riscv_sbi_extension_entry *sext;
 
 	if (reg_val != 1 && reg_val != 0)
 		return -EINVAL;
 
-	for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
-		if (sbi_ext[i].ext_idx == reg_num) {
-			sext = &sbi_ext[i];
-			break;
-		}
-	}
-	if (!sext)
+	sext = riscv_vcpu_get_sbi_ext(vcpu, reg_num);
+	if (!sext || scontext->ext_status[sext->ext_idx] == KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE)
 		return -ENOENT;
 
 	scontext->ext_status[sext->ext_idx] = (reg_val) ?
-			KVM_RISCV_SBI_EXT_AVAILABLE :
-			KVM_RISCV_SBI_EXT_UNAVAILABLE;
+			KVM_RISCV_SBI_EXT_STATUS_ENABLED :
+			KVM_RISCV_SBI_EXT_STATUS_DISABLED;
 
 	return 0;
 }
@@ -170,24 +189,16 @@ static int riscv_vcpu_get_sbi_ext_single(struct kvm_vcpu *vcpu,
 					 unsigned long reg_num,
 					 unsigned long *reg_val)
 {
-	unsigned long i;
-	const struct kvm_riscv_sbi_extension_entry *sext = NULL;
 	struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
+	const struct kvm_riscv_sbi_extension_entry *sext;
 
-	if (reg_num >= KVM_RISCV_SBI_EXT_MAX)
-		return -ENOENT;
-
-	for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
-		if (sbi_ext[i].ext_idx == reg_num) {
-			sext = &sbi_ext[i];
-			break;
-		}
-	}
-	if (!sext)
+	sext = riscv_vcpu_get_sbi_ext(vcpu, reg_num);
+	if (!sext || scontext->ext_status[sext->ext_idx] == KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE)
 		return -ENOENT;
 
 	*reg_val = scontext->ext_status[sext->ext_idx] ==
-				KVM_RISCV_SBI_EXT_AVAILABLE;
+				KVM_RISCV_SBI_EXT_STATUS_ENABLED;
+
 	return 0;
 }
 
@@ -325,7 +336,7 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
 		if (ext->extid_start <= extid && ext->extid_end >= extid) {
 			if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX ||
 			    scontext->ext_status[entry->ext_idx] ==
-						KVM_RISCV_SBI_EXT_AVAILABLE)
+						KVM_RISCV_SBI_EXT_STATUS_ENABLED)
 				return ext;
 
 			return NULL;
@@ -413,12 +424,12 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu)
 
 		if (ext->probe && !ext->probe(vcpu)) {
 			scontext->ext_status[entry->ext_idx] =
-				KVM_RISCV_SBI_EXT_UNAVAILABLE;
+				KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE;
 			continue;
 		}
 
-		scontext->ext_status[entry->ext_idx] = ext->default_unavail ?
-					KVM_RISCV_SBI_EXT_UNAVAILABLE :
-					KVM_RISCV_SBI_EXT_AVAILABLE;
+		scontext->ext_status[entry->ext_idx] = ext->default_disabled ?
+					KVM_RISCV_SBI_EXT_STATUS_DISABLED :
+					KVM_RISCV_SBI_EXT_STATUS_ENABLED;
 	}
 }
diff --git a/arch/riscv/kvm/vcpu_sbi_replace.c b/arch/riscv/kvm/vcpu_sbi_replace.c
index 23b57c931b15..9c2ab3dfa93a 100644
--- a/arch/riscv/kvm/vcpu_sbi_replace.c
+++ b/arch/riscv/kvm/vcpu_sbi_replace.c
@@ -204,6 +204,6 @@ static int kvm_sbi_ext_dbcn_handler(struct kvm_vcpu *vcpu,
 const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_dbcn = {
 	.extid_start = SBI_EXT_DBCN,
 	.extid_end = SBI_EXT_DBCN,
-	.default_unavail = true,
+	.default_disabled = true,
 	.handler = kvm_sbi_ext_dbcn_handler,
 };
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH v2 4/6] KVM: riscv: selftests: Add RISCV_SBI_EXT_REG
  2023-12-13 17:09 [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Andrew Jones
                   ` (2 preceding siblings ...)
  2023-12-13 17:09 ` [PATCH v2 3/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Andrew Jones
@ 2023-12-13 17:09 ` Andrew Jones
  2023-12-13 17:36   ` Anup Patel
  2023-12-13 17:09 ` [PATCH v2 5/6] KVM: riscv: selftests: Use register subtypes Andrew Jones
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2023-12-13 17:09 UTC (permalink / raw)
  To: kvm-riscv, linux-riscv; +Cc: anup, atishp, palmer, haibo1.xu

While adding RISCV_SBI_EXT_REG(), acknowledge that some registers
have subtypes and extend __kvm_reg_id() to take a subtype field.
Then, update all macros to set the new field appropriately. The
general CSR macro gets renamed to include "GENERAL", but the other
macros, like the new RISCV_SBI_EXT_REG, just use the SINGLE subtype.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 .../selftests/kvm/include/riscv/processor.h   | 40 +++++++++++--------
 .../selftests/kvm/lib/riscv/processor.c       |  4 +-
 2 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h
index 5b62a3d2aa9b..e70ccda2011b 100644
--- a/tools/testing/selftests/kvm/include/riscv/processor.h
+++ b/tools/testing/selftests/kvm/include/riscv/processor.h
@@ -10,10 +10,10 @@
 #include "kvm_util.h"
 #include <linux/stringify.h>
 
-static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx,
-				    uint64_t  size)
+static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t subtype,
+				    uint64_t idx, uint64_t size)
 {
-	return KVM_REG_RISCV | type | idx | size;
+	return KVM_REG_RISCV | type | subtype | idx | size;
 }
 
 #if __riscv_xlen == 64
@@ -22,24 +22,30 @@ static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx,
 #define KVM_REG_SIZE_ULONG	KVM_REG_SIZE_U32
 #endif
 
-#define RISCV_CONFIG_REG(name)	__kvm_reg_id(KVM_REG_RISCV_CONFIG, \
-					     KVM_REG_RISCV_CONFIG_REG(name), \
-					     KVM_REG_SIZE_ULONG)
+#define RISCV_CONFIG_REG(name)		__kvm_reg_id(KVM_REG_RISCV_CONFIG, 0,		\
+						     KVM_REG_RISCV_CONFIG_REG(name),	\
+						     KVM_REG_SIZE_ULONG)
 
-#define RISCV_CORE_REG(name)	__kvm_reg_id(KVM_REG_RISCV_CORE, \
-					     KVM_REG_RISCV_CORE_REG(name), \
-					     KVM_REG_SIZE_ULONG)
+#define RISCV_CORE_REG(name)		__kvm_reg_id(KVM_REG_RISCV_CORE, 0,		\
+						     KVM_REG_RISCV_CORE_REG(name),	\
+						     KVM_REG_SIZE_ULONG)
 
-#define RISCV_CSR_REG(name)	__kvm_reg_id(KVM_REG_RISCV_CSR, \
-					     KVM_REG_RISCV_CSR_REG(name), \
-					     KVM_REG_SIZE_ULONG)
+#define RISCV_GENERAL_CSR_REG(name)	__kvm_reg_id(KVM_REG_RISCV_CSR,			\
+						     KVM_REG_RISCV_CSR_GENERAL,		\
+						     KVM_REG_RISCV_CSR_REG(name),	\
+						     KVM_REG_SIZE_ULONG)
 
-#define RISCV_TIMER_REG(name)	__kvm_reg_id(KVM_REG_RISCV_TIMER, \
-					     KVM_REG_RISCV_TIMER_REG(name), \
-					     KVM_REG_SIZE_U64)
+#define RISCV_TIMER_REG(name)		__kvm_reg_id(KVM_REG_RISCV_TIMER, 0,		\
+						     KVM_REG_RISCV_TIMER_REG(name),	\
+						     KVM_REG_SIZE_U64)
 
-#define RISCV_ISA_EXT_REG(idx)	__kvm_reg_id(KVM_REG_RISCV_ISA_EXT, \
-					     idx, KVM_REG_SIZE_ULONG)
+#define RISCV_ISA_EXT_REG(idx)		__kvm_reg_id(KVM_REG_RISCV_ISA_EXT,		\
+						     KVM_REG_RISCV_ISA_SINGLE,		\
+						     idx, KVM_REG_SIZE_ULONG)
+
+#define RISCV_SBI_EXT_REG(idx)		__kvm_reg_id(KVM_REG_RISCV_SBI_EXT,		\
+						     KVM_REG_RISCV_SBI_SINGLE,		\
+						     idx, KVM_REG_SIZE_ULONG)
 
 /* L3 index Bit[47:39] */
 #define PGTBL_L3_INDEX_MASK			0x0000FF8000000000ULL
diff --git a/tools/testing/selftests/kvm/lib/riscv/processor.c b/tools/testing/selftests/kvm/lib/riscv/processor.c
index d146ca71e0c0..6c25f7843ef4 100644
--- a/tools/testing/selftests/kvm/lib/riscv/processor.c
+++ b/tools/testing/selftests/kvm/lib/riscv/processor.c
@@ -201,7 +201,7 @@ void riscv_vcpu_mmu_setup(struct kvm_vcpu *vcpu)
 	satp = (vm->pgd >> PGTBL_PAGE_SIZE_SHIFT) & SATP_PPN;
 	satp |= SATP_MODE_48;
 
-	vcpu_set_reg(vcpu, RISCV_CSR_REG(satp), satp);
+	vcpu_set_reg(vcpu, RISCV_GENERAL_CSR_REG(satp), satp);
 }
 
 void vcpu_arch_dump(FILE *stream, struct kvm_vcpu *vcpu, uint8_t indent)
@@ -315,7 +315,7 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id,
 	vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.pc), (unsigned long)guest_code);
 
 	/* Setup default exception vector of guest */
-	vcpu_set_reg(vcpu, RISCV_CSR_REG(stvec), (unsigned long)guest_unexp_trap);
+	vcpu_set_reg(vcpu, RISCV_GENERAL_CSR_REG(stvec), (unsigned long)guest_unexp_trap);
 
 	return vcpu;
 }
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH v2 5/6] KVM: riscv: selftests: Use register subtypes
  2023-12-13 17:09 [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Andrew Jones
                   ` (3 preceding siblings ...)
  2023-12-13 17:09 ` [PATCH v2 4/6] KVM: riscv: selftests: Add RISCV_SBI_EXT_REG Andrew Jones
@ 2023-12-13 17:09 ` Andrew Jones
  2023-12-13 17:39   ` Anup Patel
  2023-12-13 17:09 ` [PATCH v2 6/6] RISC-V: KVM: selftests: Treat SBI ext regs like ISA ext regs Andrew Jones
  2023-12-14  5:19 ` [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Anup Patel
  6 siblings, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2023-12-13 17:09 UTC (permalink / raw)
  To: kvm-riscv, linux-riscv; +Cc: anup, atishp, palmer, haibo1.xu

Always use register subtypes in the get-reg-list test when registers
have them. The only registers neglecting to do so were ISA extension
registers. While we don't really need to use KVM_REG_RISCV_ISA_SINGLE
(since it's zero), the main purpose is to avoid confusion and to
self-document the tests. Also add print support for the multi
registers like SBI extensions have, even though they're only used for
debugging.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Haibo Xu <haibo1.xu@intel.com>
---
 .../selftests/kvm/riscv/get-reg-list.c        | 113 +++++++++++-------
 1 file changed, 73 insertions(+), 40 deletions(-)

diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c
index 27d07a32a1ef..4bcc597d34b9 100644
--- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
+++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
@@ -28,31 +28,31 @@ bool filter_reg(__u64 reg)
 	 *
 	 * Note: The below list is alphabetically sorted.
 	 */
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_A:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_C:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_D:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_F:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_H:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_I:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_M:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_V:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SMSTATEEN:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SSAIA:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SSTC:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVINVAL:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVNAPOT:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVPBMT:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZBA:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZBB:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZBS:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICBOM:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICBOZ:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICNTR:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICOND:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICSR:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZIFENCEI:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZIHINTPAUSE:
-	case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZIHPM:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_A:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_C:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_D:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_F:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_H:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_I:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_M:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_V:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SMSTATEEN:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SSAIA:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SSTC:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVINVAL:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVNAPOT:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVPBMT:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBA:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBB:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBS:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICBOM:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICBOZ:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICNTR:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICOND:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICSR:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZIFENCEI:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZIHINTPAUSE:
+	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZIHPM:
 		return true;
 	/* AIA registers are always available when Ssaia can't be disabled */
 	case KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(siselect):
@@ -335,15 +335,10 @@ static const char *fp_d_id_to_str(const char *prefix, __u64 id)
 }
 
 #define KVM_ISA_EXT_ARR(ext)		\
-[KVM_RISCV_ISA_EXT_##ext] = "KVM_RISCV_ISA_EXT_" #ext
+[KVM_RISCV_ISA_EXT_##ext] = "KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_" #ext
 
-static const char *isa_ext_id_to_str(const char *prefix, __u64 id)
+static const char *isa_ext_single_id_to_str(__u64 reg_off)
 {
-	/* reg_off is the offset into unsigned long kvm_isa_ext_arr[] */
-	__u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_ISA_EXT);
-
-	assert((id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_ISA_EXT);
-
 	static const char * const kvm_isa_ext_reg_name[] = {
 		KVM_ISA_EXT_ARR(A),
 		KVM_ISA_EXT_ARR(C),
@@ -373,11 +368,48 @@ static const char *isa_ext_id_to_str(const char *prefix, __u64 id)
 	};
 
 	if (reg_off >= ARRAY_SIZE(kvm_isa_ext_reg_name))
-		return strdup_printf("%lld /* UNKNOWN */", reg_off);
+		return strdup_printf("KVM_REG_RISCV_ISA_SINGLE | %lld /* UNKNOWN */", reg_off);
 
 	return kvm_isa_ext_reg_name[reg_off];
 }
 
+static const char *isa_ext_multi_id_to_str(__u64 reg_subtype, __u64 reg_off)
+{
+	const char *unknown = "";
+
+	if (reg_off > KVM_REG_RISCV_ISA_MULTI_REG_LAST)
+		unknown = " /* UNKNOWN */";
+
+	switch (reg_subtype) {
+	case KVM_REG_RISCV_ISA_MULTI_EN:
+		return strdup_printf("KVM_REG_RISCV_ISA_MULTI_EN | %lld%s", reg_off, unknown);
+	case KVM_REG_RISCV_ISA_MULTI_DIS:
+		return strdup_printf("KVM_REG_RISCV_ISA_MULTI_DIS | %lld%s", reg_off, unknown);
+	}
+
+	return strdup_printf("%lld | %lld /* UNKNOWN */", reg_subtype, reg_off);
+}
+
+static const char *isa_ext_id_to_str(const char *prefix, __u64 id)
+{
+	__u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_ISA_EXT);
+	__u64 reg_subtype = reg_off & KVM_REG_RISCV_SUBTYPE_MASK;
+
+	assert((id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_ISA_EXT);
+
+	reg_off &= ~KVM_REG_RISCV_SUBTYPE_MASK;
+
+	switch (reg_subtype) {
+	case KVM_REG_RISCV_ISA_SINGLE:
+		return isa_ext_single_id_to_str(reg_off);
+	case KVM_REG_RISCV_ISA_MULTI_EN:
+	case KVM_REG_RISCV_ISA_MULTI_DIS:
+		return isa_ext_multi_id_to_str(reg_subtype, reg_off);
+	}
+
+	return strdup_printf("%lld | %lld /* UNKNOWN */", reg_subtype, reg_off);
+}
+
 #define KVM_SBI_EXT_ARR(ext)		\
 [ext] = "KVM_REG_RISCV_SBI_SINGLE | " #ext
 
@@ -583,12 +615,12 @@ static __u64 base_skips_set[] = {
 
 static __u64 zicbom_regs[] = {
 	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(zicbom_block_size),
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICBOM,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICBOM,
 };
 
 static __u64 zicboz_regs[] = {
 	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(zicboz_block_size),
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICBOZ,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICBOZ,
 };
 
 static __u64 aia_regs[] = {
@@ -599,12 +631,12 @@ static __u64 aia_regs[] = {
 	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(siph),
 	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(iprio1h),
 	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(iprio2h),
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SSAIA,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SSAIA,
 };
 
 static __u64 smstateen_regs[] = {
 	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_SMSTATEEN | KVM_REG_RISCV_CSR_SMSTATEEN_REG(sstateen0),
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SMSTATEEN,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SMSTATEEN,
 };
 
 static __u64 fp_f_regs[] = {
@@ -641,7 +673,7 @@ static __u64 fp_f_regs[] = {
 	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),
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_F,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_F,
 };
 
 static __u64 fp_d_regs[] = {
@@ -678,7 +710,7 @@ static __u64 fp_d_regs[] = {
 	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),
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_D,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_D,
 };
 
 #define SUBLIST_BASE \
@@ -702,7 +734,8 @@ static __u64 fp_d_regs[] = {
 #define KVM_ISA_EXT_SIMPLE_CONFIG(ext, extu)			\
 static __u64 regs_##ext[] = {					\
 	KVM_REG_RISCV | KVM_REG_SIZE_ULONG |			\
-	KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_##extu,	\
+	KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE |	\
+	KVM_RISCV_ISA_EXT_##extu,				\
 };								\
 static struct vcpu_reg_list config_##ext = {			\
 	.sublists = {						\
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* [PATCH v2 6/6] RISC-V: KVM: selftests: Treat SBI ext regs like ISA ext regs
  2023-12-13 17:09 [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Andrew Jones
                   ` (4 preceding siblings ...)
  2023-12-13 17:09 ` [PATCH v2 5/6] KVM: riscv: selftests: Use register subtypes Andrew Jones
@ 2023-12-13 17:09 ` Andrew Jones
  2023-12-13 17:42   ` Anup Patel
  2023-12-14  5:19 ` [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Anup Patel
  6 siblings, 1 reply; 14+ messages in thread
From: Andrew Jones @ 2023-12-13 17:09 UTC (permalink / raw)
  To: kvm-riscv, linux-riscv; +Cc: anup, atishp, palmer, haibo1.xu

SBI extension registers may not be present and indeed when
running on a platform without sscofpmf the PMU SBI extension
is not. Move the SBI extension registers from the base set of
registers to the filter list. Individual configs should test
for any that may or may not be present separately. Since
the PMU extension may disappear and the DBCN extension is only
present in later kernels, separate them from the rest into
their own configs. The rest are lumped together into the same
config.

Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
 .../selftests/kvm/include/kvm_util_base.h     |   1 +
 .../selftests/kvm/riscv/get-reg-list.c        | 105 +++++++++++++++---
 2 files changed, 92 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
index a18db6a7b3cf..e112ee30867f 100644
--- a/tools/testing/selftests/kvm/include/kvm_util_base.h
+++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
@@ -129,6 +129,7 @@ struct vcpu_reg_sublist {
 	const char *name;
 	long capability;
 	int feature;
+	int feature_type;
 	bool finalize;
 	__u64 *regs;
 	__u64 regs_n;
diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c
index 4bcc597d34b9..b8da2e86bf9c 100644
--- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
+++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
@@ -12,6 +12,11 @@
 
 #define REG_MASK (KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK)
 
+enum {
+	VCPU_FEATURE_ISA_EXT = 0,
+	VCPU_FEATURE_SBI_EXT,
+};
+
 static bool isa_ext_cant_disable[KVM_RISCV_ISA_EXT_MAX];
 
 bool filter_reg(__u64 reg)
@@ -53,6 +58,21 @@ bool filter_reg(__u64 reg)
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZIFENCEI:
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZIHINTPAUSE:
 	case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZIHPM:
+	/*
+	 * Like ISA_EXT registers, SBI_EXT registers are only visible when the
+	 * host supports them and disabling them does not affect the visibility
+	 * of the SBI_EXT register itself.
+	 */
+	case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_V01:
+	case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_TIME:
+	case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_IPI:
+	case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_RFENCE:
+	case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_SRST:
+	case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_HSM:
+	case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_PMU:
+	case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_DBCN:
+	case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_EXPERIMENTAL:
+	case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_VENDOR:
 		return true;
 	/* AIA registers are always available when Ssaia can't be disabled */
 	case KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(siselect):
@@ -75,12 +95,12 @@ bool check_reject_set(int err)
 	return err == EINVAL;
 }
 
-static inline bool vcpu_has_ext(struct kvm_vcpu *vcpu, int ext)
+static bool vcpu_has_ext(struct kvm_vcpu *vcpu, uint64_t ext_id)
 {
 	int ret;
 	unsigned long value;
 
-	ret = __vcpu_get_reg(vcpu, RISCV_ISA_EXT_REG(ext), &value);
+	ret = __vcpu_get_reg(vcpu, ext_id, &value);
 	return (ret) ? false : !!value;
 }
 
@@ -88,6 +108,7 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c)
 {
 	unsigned long isa_ext_state[KVM_RISCV_ISA_EXT_MAX] = { 0 };
 	struct vcpu_reg_sublist *s;
+	uint64_t feature;
 	int rc;
 
 	for (int i = 0; i < KVM_RISCV_ISA_EXT_MAX; i++)
@@ -103,15 +124,31 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c)
 			isa_ext_cant_disable[i] = true;
 	}
 
+	for (int i = 0; i < KVM_RISCV_SBI_EXT_MAX; i++) {
+		rc = __vcpu_set_reg(vcpu, RISCV_SBI_EXT_REG(i), 0);
+		TEST_ASSERT(!rc || (rc == -1 && errno == ENOENT), "Unexpected error");
+	}
+
 	for_each_sublist(c, s) {
 		if (!s->feature)
 			continue;
 
+		switch (s->feature_type) {
+		case VCPU_FEATURE_ISA_EXT:
+			feature = RISCV_ISA_EXT_REG(s->feature);
+			break;
+		case VCPU_FEATURE_SBI_EXT:
+			feature = RISCV_SBI_EXT_REG(s->feature);
+			break;
+		default:
+			TEST_FAIL("Unknown feature type");
+		}
+
 		/* Try to enable the desired extension */
-		__vcpu_set_reg(vcpu, RISCV_ISA_EXT_REG(s->feature), 1);
+		__vcpu_set_reg(vcpu, feature, 1);
 
 		/* Double check whether the desired extension was enabled */
-		__TEST_REQUIRE(vcpu_has_ext(vcpu, s->feature),
+		__TEST_REQUIRE(vcpu_has_ext(vcpu, feature),
 			       "%s not available, skipping tests\n", s->name);
 	}
 }
@@ -593,16 +630,6 @@ static __u64 base_regs[] = {
 	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_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_V01,
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_TIME,
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_IPI,
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_RFENCE,
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_SRST,
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_HSM,
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_PMU,
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_EXPERIMENTAL,
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_VENDOR,
-	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_DBCN,
 };
 
 /*
@@ -613,6 +640,17 @@ static __u64 base_skips_set[] = {
 	KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(state),
 };
 
+static __u64 sbi_base_regs[] = {
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_V01,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_TIME,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_IPI,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_RFENCE,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_SRST,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_HSM,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_EXPERIMENTAL,
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_VENDOR,
+};
+
 static __u64 zicbom_regs[] = {
 	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(zicbom_block_size),
 	KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICBOM,
@@ -716,6 +754,9 @@ static __u64 fp_d_regs[] = {
 #define SUBLIST_BASE \
 	{"base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), \
 	 .skips_set = base_skips_set, .skips_set_n = ARRAY_SIZE(base_skips_set),}
+#define SUBLIST_SBI_BASE \
+	{"sbi-base", .feature_type = VCPU_FEATURE_SBI_EXT, .feature = KVM_RISCV_SBI_EXT_V01, \
+	 .regs = sbi_base_regs, .regs_n = ARRAY_SIZE(sbi_base_regs),}
 #define SUBLIST_ZICBOM \
 	{"zicbom", .feature = KVM_RISCV_ISA_EXT_ZICBOM, .regs = zicbom_regs, .regs_n = ARRAY_SIZE(zicbom_regs),}
 #define SUBLIST_ZICBOZ \
@@ -750,6 +791,26 @@ static struct vcpu_reg_list config_##ext = {			\
 	},							\
 }								\
 
+#define KVM_SBI_EXT_SIMPLE_CONFIG(ext, extu)			\
+static __u64 regs_sbi_##ext[] = {				\
+	KVM_REG_RISCV | KVM_REG_SIZE_ULONG |			\
+	KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE |	\
+	KVM_RISCV_SBI_EXT_##extu,				\
+};								\
+static struct vcpu_reg_list config_sbi_##ext = {		\
+	.sublists = {						\
+		SUBLIST_BASE,					\
+		{						\
+			.name = "sbi-"#ext,			\
+			.feature_type = VCPU_FEATURE_SBI_EXT,	\
+			.feature = KVM_RISCV_SBI_EXT_##extu,	\
+			.regs = regs_sbi_##ext,			\
+			.regs_n = ARRAY_SIZE(regs_sbi_##ext),	\
+		},						\
+		{0},						\
+	},							\
+}								\
+
 #define KVM_ISA_EXT_SUBLIST_CONFIG(ext, extu)			\
 static struct vcpu_reg_list config_##ext = {			\
 	.sublists = {						\
@@ -759,8 +820,21 @@ static struct vcpu_reg_list config_##ext = {			\
 	},							\
 }								\
 
+#define KVM_SBI_EXT_SUBLIST_CONFIG(ext, extu)			\
+static struct vcpu_reg_list config_sbi_##ext = {		\
+	.sublists = {						\
+		SUBLIST_BASE,					\
+		SUBLIST_SBI_##extu,				\
+		{0},						\
+	},							\
+}								\
+
 /* Note: The below list is alphabetically sorted. */
 
+KVM_SBI_EXT_SUBLIST_CONFIG(base, BASE);
+KVM_SBI_EXT_SIMPLE_CONFIG(pmu, PMU);
+KVM_SBI_EXT_SIMPLE_CONFIG(dbcn, DBCN);
+
 KVM_ISA_EXT_SUBLIST_CONFIG(aia, AIA);
 KVM_ISA_EXT_SUBLIST_CONFIG(fp_f, FP_F);
 KVM_ISA_EXT_SUBLIST_CONFIG(fp_d, FP_D);
@@ -783,6 +857,9 @@ KVM_ISA_EXT_SIMPLE_CONFIG(zihintpause, ZIHINTPAUSE);
 KVM_ISA_EXT_SIMPLE_CONFIG(zihpm, ZIHPM);
 
 struct vcpu_reg_list *vcpu_configs[] = {
+	&config_sbi_base,
+	&config_sbi_pmu,
+	&config_sbi_dbcn,
 	&config_aia,
 	&config_fp_f,
 	&config_fp_d,
-- 
2.43.0


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v2 1/6] RISC-V: KVM: Don't add SBI multi regs in get-reg-list
  2023-12-13 17:09 ` [PATCH v2 1/6] RISC-V: KVM: Don't add SBI multi regs in get-reg-list Andrew Jones
@ 2023-12-13 17:23   ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2023-12-13 17:23 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm-riscv, linux-riscv, atishp, palmer, haibo1.xu

On Wed, Dec 13, 2023 at 10:39 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> The multi regs are derived from the single registers. Only list the
> single registers in get-reg-list. This also makes the SBI extension
> register listing consistent with the ISA extension register listing.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Haibo Xu <haibo1.xu@intel.com>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  arch/riscv/kvm/vcpu_onereg.c | 36 ++----------------------------------
>  1 file changed, 2 insertions(+), 34 deletions(-)
>
> diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
> index f8c9fa0c03c5..f9bfa8a5db21 100644
> --- a/arch/riscv/kvm/vcpu_onereg.c
> +++ b/arch/riscv/kvm/vcpu_onereg.c
> @@ -933,20 +933,12 @@ static inline unsigned long num_isa_ext_regs(const struct kvm_vcpu *vcpu)
>
>  static inline unsigned long num_sbi_ext_regs(void)
>  {
> -       /*
> -        * number of KVM_REG_RISCV_SBI_SINGLE +
> -        * 2 x (number of KVM_REG_RISCV_SBI_MULTI)
> -        */
> -       return KVM_RISCV_SBI_EXT_MAX + 2*(KVM_REG_RISCV_SBI_MULTI_REG_LAST+1);
> +       return KVM_RISCV_SBI_EXT_MAX;
>  }
>
>  static int copy_sbi_ext_reg_indices(u64 __user *uindices)
>  {
> -       int n;
> -
> -       /* copy KVM_REG_RISCV_SBI_SINGLE */
> -       n = KVM_RISCV_SBI_EXT_MAX;
> -       for (int i = 0; i < n; i++) {
> +       for (int i = 0; i < KVM_RISCV_SBI_EXT_MAX; i++) {
>                 u64 size = IS_ENABLED(CONFIG_32BIT) ?
>                            KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
>                 u64 reg = KVM_REG_RISCV | size | KVM_REG_RISCV_SBI_EXT |
> @@ -959,30 +951,6 @@ static int copy_sbi_ext_reg_indices(u64 __user *uindices)
>                 }
>         }
>
> -       /* copy KVM_REG_RISCV_SBI_MULTI */
> -       n = KVM_REG_RISCV_SBI_MULTI_REG_LAST + 1;
> -       for (int i = 0; i < n; i++) {
> -               u64 size = IS_ENABLED(CONFIG_32BIT) ?
> -                          KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
> -               u64 reg = KVM_REG_RISCV | size | KVM_REG_RISCV_SBI_EXT |
> -                         KVM_REG_RISCV_SBI_MULTI_EN | i;
> -
> -               if (uindices) {
> -                       if (put_user(reg, uindices))
> -                               return -EFAULT;
> -                       uindices++;
> -               }
> -
> -               reg = KVM_REG_RISCV | size | 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();
>  }
>
> --
> 2.43.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v2 2/6] KVM: riscv: selftests: Drop SBI multi registers
  2023-12-13 17:09 ` [PATCH v2 2/6] KVM: riscv: selftests: Drop SBI multi registers Andrew Jones
@ 2023-12-13 17:24   ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2023-12-13 17:24 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm-riscv, linux-riscv, atishp, palmer, haibo1.xu

On Wed, Dec 13, 2023 at 10:39 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> These registers are no longer getting added to get-reg-list.
> We keep sbi_ext_multi_id_to_str() for printing, even though
> we don't expect it to normally be used, because it may be
> useful for debug.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  tools/testing/selftests/kvm/riscv/get-reg-list.c | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c
> index 5d86c761784e..27d07a32a1ef 100644
> --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
> @@ -571,8 +571,6 @@ static __u64 base_regs[] = {
>         KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_EXPERIMENTAL,
>         KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_VENDOR,
>         KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_DBCN,
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_MULTI_EN | 0,
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_MULTI_DIS | 0,
>  };
>
>  /*
> --
> 2.43.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v2 3/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi
  2023-12-13 17:09 ` [PATCH v2 3/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Andrew Jones
@ 2023-12-13 17:35   ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2023-12-13 17:35 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm-riscv, linux-riscv, atishp, palmer, haibo1.xu

On Wed, Dec 13, 2023 at 10:39 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> When an SBI extension cannot be enabled, that's a distinct state vs.
> enabled and disabled. Modify enum kvm_riscv_sbi_ext_status to
> accommodate it, which allows KVM userspace to tell the difference
> in state too, as the SBI extension register will disappear when it
> cannot be enabled, i.e. accesses to it return ENOENT. get-reg-list is
> updated as well to only add SBI extension registers to the list which
> may be enabled. Returning ENOENT for SBI extension registers which
> cannot be enabled makes them consistent with ISA extension registers.
> Any SBI extensions which were enabled by default are still enabled by
> default, if they can be enabled at all.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  arch/riscv/include/asm/kvm_vcpu_sbi.h | 10 ++--
>  arch/riscv/kvm/vcpu_onereg.c          | 23 +++++---
>  arch/riscv/kvm/vcpu_sbi.c             | 75 +++++++++++++++------------
>  arch/riscv/kvm/vcpu_sbi_replace.c     |  2 +-
>  4 files changed, 65 insertions(+), 45 deletions(-)
>
> diff --git a/arch/riscv/include/asm/kvm_vcpu_sbi.h b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> index 6a453f7f8b56..bffda0ac59b6 100644
> --- a/arch/riscv/include/asm/kvm_vcpu_sbi.h
> +++ b/arch/riscv/include/asm/kvm_vcpu_sbi.h
> @@ -15,9 +15,10 @@
>  #define KVM_SBI_VERSION_MINOR 0
>
>  enum kvm_riscv_sbi_ext_status {
> -       KVM_RISCV_SBI_EXT_UNINITIALIZED,
> -       KVM_RISCV_SBI_EXT_AVAILABLE,
> -       KVM_RISCV_SBI_EXT_UNAVAILABLE,
> +       KVM_RISCV_SBI_EXT_STATUS_UNINITIALIZED,
> +       KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE,
> +       KVM_RISCV_SBI_EXT_STATUS_ENABLED,
> +       KVM_RISCV_SBI_EXT_STATUS_DISABLED,
>  };
>
>  struct kvm_vcpu_sbi_context {
> @@ -36,7 +37,7 @@ struct kvm_vcpu_sbi_extension {
>         unsigned long extid_start;
>         unsigned long extid_end;
>
> -       bool default_unavail;
> +       bool default_disabled;
>
>         /**
>          * SBI extension handler. It can be defined for a given extension or group of
> @@ -61,6 +62,7 @@ int kvm_riscv_vcpu_get_reg_sbi_ext(struct kvm_vcpu *vcpu,
>                                    const struct kvm_one_reg *reg);
>  const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
>                                 struct kvm_vcpu *vcpu, unsigned long extid);
> +bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx);
>  int kvm_riscv_vcpu_sbi_ecall(struct kvm_vcpu *vcpu, struct kvm_run *run);
>  void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu);
>
> diff --git a/arch/riscv/kvm/vcpu_onereg.c b/arch/riscv/kvm/vcpu_onereg.c
> index f9bfa8a5db21..48262be73aa0 100644
> --- a/arch/riscv/kvm/vcpu_onereg.c
> +++ b/arch/riscv/kvm/vcpu_onereg.c
> @@ -931,27 +931,34 @@ static inline unsigned long num_isa_ext_regs(const struct kvm_vcpu *vcpu)
>         return copy_isa_ext_reg_indices(vcpu, NULL);;
>  }
>
> -static inline unsigned long num_sbi_ext_regs(void)
> +static int copy_sbi_ext_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
>  {
> -       return KVM_RISCV_SBI_EXT_MAX;
> -}
> +       unsigned int n = 0;
>
> -static int copy_sbi_ext_reg_indices(u64 __user *uindices)
> -{
>         for (int i = 0; i < KVM_RISCV_SBI_EXT_MAX; i++) {
>                 u64 size = IS_ENABLED(CONFIG_32BIT) ?
>                            KVM_REG_SIZE_U32 : KVM_REG_SIZE_U64;
>                 u64 reg = KVM_REG_RISCV | size | KVM_REG_RISCV_SBI_EXT |
>                           KVM_REG_RISCV_SBI_SINGLE | i;
>
> +               if (!riscv_vcpu_supports_sbi_ext(vcpu, i))
> +                       continue;
> +
>                 if (uindices) {
>                         if (put_user(reg, uindices))
>                                 return -EFAULT;
>                         uindices++;
>                 }
> +
> +               n++;
>         }
>
> -       return num_sbi_ext_regs();
> +       return n;
> +}
> +
> +static unsigned long num_sbi_ext_regs(struct kvm_vcpu *vcpu)
> +{
> +       return copy_sbi_ext_reg_indices(vcpu, NULL);
>  }
>
>  /*
> @@ -970,7 +977,7 @@ unsigned long kvm_riscv_vcpu_num_regs(struct kvm_vcpu *vcpu)
>         res += num_fp_f_regs(vcpu);
>         res += num_fp_d_regs(vcpu);
>         res += num_isa_ext_regs(vcpu);
> -       res += num_sbi_ext_regs();
> +       res += num_sbi_ext_regs(vcpu);
>
>         return res;
>  }
> @@ -1018,7 +1025,7 @@ int kvm_riscv_vcpu_copy_reg_indices(struct kvm_vcpu *vcpu,
>                 return ret;
>         uindices += ret;
>
> -       ret = copy_sbi_ext_reg_indices(uindices);
> +       ret = copy_sbi_ext_reg_indices(vcpu, uindices);
>         if (ret < 0)
>                 return ret;
>
> diff --git a/arch/riscv/kvm/vcpu_sbi.c b/arch/riscv/kvm/vcpu_sbi.c
> index a04ff98085d9..dcdff4458190 100644
> --- a/arch/riscv/kvm/vcpu_sbi.c
> +++ b/arch/riscv/kvm/vcpu_sbi.c
> @@ -80,6 +80,34 @@ static const struct kvm_riscv_sbi_extension_entry sbi_ext[] = {
>         },
>  };
>
> +static const struct kvm_riscv_sbi_extension_entry *
> +riscv_vcpu_get_sbi_ext(struct kvm_vcpu *vcpu, unsigned long idx)
> +{
> +       const struct kvm_riscv_sbi_extension_entry *sext = NULL;
> +
> +       if (idx >= KVM_RISCV_SBI_EXT_MAX)
> +               return NULL;
> +
> +       for (int i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
> +               if (sbi_ext[i].ext_idx == idx) {
> +                       sext = &sbi_ext[i];
> +                       break;
> +               }
> +       }
> +
> +       return sext;
> +}
> +
> +bool riscv_vcpu_supports_sbi_ext(struct kvm_vcpu *vcpu, int idx)
> +{
> +       struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
> +       const struct kvm_riscv_sbi_extension_entry *sext;
> +
> +       sext = riscv_vcpu_get_sbi_ext(vcpu, idx);
> +
> +       return sext && scontext->ext_status[sext->ext_idx] != KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE;
> +}
> +
>  void kvm_riscv_vcpu_sbi_forward(struct kvm_vcpu *vcpu, struct kvm_run *run)
>  {
>         struct kvm_cpu_context *cp = &vcpu->arch.guest_context;
> @@ -140,28 +168,19 @@ static int riscv_vcpu_set_sbi_ext_single(struct kvm_vcpu *vcpu,
>                                          unsigned long reg_num,
>                                          unsigned long reg_val)
>  {
> -       unsigned long i;
> -       const struct kvm_riscv_sbi_extension_entry *sext = NULL;
>         struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
> -
> -       if (reg_num >= KVM_RISCV_SBI_EXT_MAX)
> -               return -ENOENT;
> +       const struct kvm_riscv_sbi_extension_entry *sext;
>
>         if (reg_val != 1 && reg_val != 0)
>                 return -EINVAL;
>
> -       for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
> -               if (sbi_ext[i].ext_idx == reg_num) {
> -                       sext = &sbi_ext[i];
> -                       break;
> -               }
> -       }
> -       if (!sext)
> +       sext = riscv_vcpu_get_sbi_ext(vcpu, reg_num);
> +       if (!sext || scontext->ext_status[sext->ext_idx] == KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE)
>                 return -ENOENT;
>
>         scontext->ext_status[sext->ext_idx] = (reg_val) ?
> -                       KVM_RISCV_SBI_EXT_AVAILABLE :
> -                       KVM_RISCV_SBI_EXT_UNAVAILABLE;
> +                       KVM_RISCV_SBI_EXT_STATUS_ENABLED :
> +                       KVM_RISCV_SBI_EXT_STATUS_DISABLED;
>
>         return 0;
>  }
> @@ -170,24 +189,16 @@ static int riscv_vcpu_get_sbi_ext_single(struct kvm_vcpu *vcpu,
>                                          unsigned long reg_num,
>                                          unsigned long *reg_val)
>  {
> -       unsigned long i;
> -       const struct kvm_riscv_sbi_extension_entry *sext = NULL;
>         struct kvm_vcpu_sbi_context *scontext = &vcpu->arch.sbi_context;
> +       const struct kvm_riscv_sbi_extension_entry *sext;
>
> -       if (reg_num >= KVM_RISCV_SBI_EXT_MAX)
> -               return -ENOENT;
> -
> -       for (i = 0; i < ARRAY_SIZE(sbi_ext); i++) {
> -               if (sbi_ext[i].ext_idx == reg_num) {
> -                       sext = &sbi_ext[i];
> -                       break;
> -               }
> -       }
> -       if (!sext)
> +       sext = riscv_vcpu_get_sbi_ext(vcpu, reg_num);
> +       if (!sext || scontext->ext_status[sext->ext_idx] == KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE)
>                 return -ENOENT;
>
>         *reg_val = scontext->ext_status[sext->ext_idx] ==
> -                               KVM_RISCV_SBI_EXT_AVAILABLE;
> +                               KVM_RISCV_SBI_EXT_STATUS_ENABLED;
> +
>         return 0;
>  }
>
> @@ -325,7 +336,7 @@ const struct kvm_vcpu_sbi_extension *kvm_vcpu_sbi_find_ext(
>                 if (ext->extid_start <= extid && ext->extid_end >= extid) {
>                         if (entry->ext_idx >= KVM_RISCV_SBI_EXT_MAX ||
>                             scontext->ext_status[entry->ext_idx] ==
> -                                               KVM_RISCV_SBI_EXT_AVAILABLE)
> +                                               KVM_RISCV_SBI_EXT_STATUS_ENABLED)
>                                 return ext;
>
>                         return NULL;
> @@ -413,12 +424,12 @@ void kvm_riscv_vcpu_sbi_init(struct kvm_vcpu *vcpu)
>
>                 if (ext->probe && !ext->probe(vcpu)) {
>                         scontext->ext_status[entry->ext_idx] =
> -                               KVM_RISCV_SBI_EXT_UNAVAILABLE;
> +                               KVM_RISCV_SBI_EXT_STATUS_UNAVAILABLE;
>                         continue;
>                 }
>
> -               scontext->ext_status[entry->ext_idx] = ext->default_unavail ?
> -                                       KVM_RISCV_SBI_EXT_UNAVAILABLE :
> -                                       KVM_RISCV_SBI_EXT_AVAILABLE;
> +               scontext->ext_status[entry->ext_idx] = ext->default_disabled ?
> +                                       KVM_RISCV_SBI_EXT_STATUS_DISABLED :
> +                                       KVM_RISCV_SBI_EXT_STATUS_ENABLED;
>         }
>  }
> diff --git a/arch/riscv/kvm/vcpu_sbi_replace.c b/arch/riscv/kvm/vcpu_sbi_replace.c
> index 23b57c931b15..9c2ab3dfa93a 100644
> --- a/arch/riscv/kvm/vcpu_sbi_replace.c
> +++ b/arch/riscv/kvm/vcpu_sbi_replace.c
> @@ -204,6 +204,6 @@ static int kvm_sbi_ext_dbcn_handler(struct kvm_vcpu *vcpu,
>  const struct kvm_vcpu_sbi_extension vcpu_sbi_ext_dbcn = {
>         .extid_start = SBI_EXT_DBCN,
>         .extid_end = SBI_EXT_DBCN,
> -       .default_unavail = true,
> +       .default_disabled = true,
>         .handler = kvm_sbi_ext_dbcn_handler,
>  };
> --
> 2.43.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v2 4/6] KVM: riscv: selftests: Add RISCV_SBI_EXT_REG
  2023-12-13 17:09 ` [PATCH v2 4/6] KVM: riscv: selftests: Add RISCV_SBI_EXT_REG Andrew Jones
@ 2023-12-13 17:36   ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2023-12-13 17:36 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm-riscv, linux-riscv, atishp, palmer, haibo1.xu

On Wed, Dec 13, 2023 at 10:39 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> While adding RISCV_SBI_EXT_REG(), acknowledge that some registers
> have subtypes and extend __kvm_reg_id() to take a subtype field.
> Then, update all macros to set the new field appropriately. The
> general CSR macro gets renamed to include "GENERAL", but the other
> macros, like the new RISCV_SBI_EXT_REG, just use the SINGLE subtype.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  .../selftests/kvm/include/riscv/processor.h   | 40 +++++++++++--------
>  .../selftests/kvm/lib/riscv/processor.c       |  4 +-
>  2 files changed, 25 insertions(+), 19 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/riscv/processor.h b/tools/testing/selftests/kvm/include/riscv/processor.h
> index 5b62a3d2aa9b..e70ccda2011b 100644
> --- a/tools/testing/selftests/kvm/include/riscv/processor.h
> +++ b/tools/testing/selftests/kvm/include/riscv/processor.h
> @@ -10,10 +10,10 @@
>  #include "kvm_util.h"
>  #include <linux/stringify.h>
>
> -static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx,
> -                                   uint64_t  size)
> +static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t subtype,
> +                                   uint64_t idx, uint64_t size)
>  {
> -       return KVM_REG_RISCV | type | idx | size;
> +       return KVM_REG_RISCV | type | subtype | idx | size;
>  }
>
>  #if __riscv_xlen == 64
> @@ -22,24 +22,30 @@ static inline uint64_t __kvm_reg_id(uint64_t type, uint64_t idx,
>  #define KVM_REG_SIZE_ULONG     KVM_REG_SIZE_U32
>  #endif
>
> -#define RISCV_CONFIG_REG(name) __kvm_reg_id(KVM_REG_RISCV_CONFIG, \
> -                                            KVM_REG_RISCV_CONFIG_REG(name), \
> -                                            KVM_REG_SIZE_ULONG)
> +#define RISCV_CONFIG_REG(name)         __kvm_reg_id(KVM_REG_RISCV_CONFIG, 0,           \
> +                                                    KVM_REG_RISCV_CONFIG_REG(name),    \
> +                                                    KVM_REG_SIZE_ULONG)
>
> -#define RISCV_CORE_REG(name)   __kvm_reg_id(KVM_REG_RISCV_CORE, \
> -                                            KVM_REG_RISCV_CORE_REG(name), \
> -                                            KVM_REG_SIZE_ULONG)
> +#define RISCV_CORE_REG(name)           __kvm_reg_id(KVM_REG_RISCV_CORE, 0,             \
> +                                                    KVM_REG_RISCV_CORE_REG(name),      \
> +                                                    KVM_REG_SIZE_ULONG)
>
> -#define RISCV_CSR_REG(name)    __kvm_reg_id(KVM_REG_RISCV_CSR, \
> -                                            KVM_REG_RISCV_CSR_REG(name), \
> -                                            KVM_REG_SIZE_ULONG)
> +#define RISCV_GENERAL_CSR_REG(name)    __kvm_reg_id(KVM_REG_RISCV_CSR,                 \
> +                                                    KVM_REG_RISCV_CSR_GENERAL,         \
> +                                                    KVM_REG_RISCV_CSR_REG(name),       \
> +                                                    KVM_REG_SIZE_ULONG)
>
> -#define RISCV_TIMER_REG(name)  __kvm_reg_id(KVM_REG_RISCV_TIMER, \
> -                                            KVM_REG_RISCV_TIMER_REG(name), \
> -                                            KVM_REG_SIZE_U64)
> +#define RISCV_TIMER_REG(name)          __kvm_reg_id(KVM_REG_RISCV_TIMER, 0,            \
> +                                                    KVM_REG_RISCV_TIMER_REG(name),     \
> +                                                    KVM_REG_SIZE_U64)
>
> -#define RISCV_ISA_EXT_REG(idx) __kvm_reg_id(KVM_REG_RISCV_ISA_EXT, \
> -                                            idx, KVM_REG_SIZE_ULONG)
> +#define RISCV_ISA_EXT_REG(idx)         __kvm_reg_id(KVM_REG_RISCV_ISA_EXT,             \
> +                                                    KVM_REG_RISCV_ISA_SINGLE,          \
> +                                                    idx, KVM_REG_SIZE_ULONG)
> +
> +#define RISCV_SBI_EXT_REG(idx)         __kvm_reg_id(KVM_REG_RISCV_SBI_EXT,             \
> +                                                    KVM_REG_RISCV_SBI_SINGLE,          \
> +                                                    idx, KVM_REG_SIZE_ULONG)
>
>  /* L3 index Bit[47:39] */
>  #define PGTBL_L3_INDEX_MASK                    0x0000FF8000000000ULL
> diff --git a/tools/testing/selftests/kvm/lib/riscv/processor.c b/tools/testing/selftests/kvm/lib/riscv/processor.c
> index d146ca71e0c0..6c25f7843ef4 100644
> --- a/tools/testing/selftests/kvm/lib/riscv/processor.c
> +++ b/tools/testing/selftests/kvm/lib/riscv/processor.c
> @@ -201,7 +201,7 @@ void riscv_vcpu_mmu_setup(struct kvm_vcpu *vcpu)
>         satp = (vm->pgd >> PGTBL_PAGE_SIZE_SHIFT) & SATP_PPN;
>         satp |= SATP_MODE_48;
>
> -       vcpu_set_reg(vcpu, RISCV_CSR_REG(satp), satp);
> +       vcpu_set_reg(vcpu, RISCV_GENERAL_CSR_REG(satp), satp);
>  }
>
>  void vcpu_arch_dump(FILE *stream, struct kvm_vcpu *vcpu, uint8_t indent)
> @@ -315,7 +315,7 @@ struct kvm_vcpu *vm_arch_vcpu_add(struct kvm_vm *vm, uint32_t vcpu_id,
>         vcpu_set_reg(vcpu, RISCV_CORE_REG(regs.pc), (unsigned long)guest_code);
>
>         /* Setup default exception vector of guest */
> -       vcpu_set_reg(vcpu, RISCV_CSR_REG(stvec), (unsigned long)guest_unexp_trap);
> +       vcpu_set_reg(vcpu, RISCV_GENERAL_CSR_REG(stvec), (unsigned long)guest_unexp_trap);
>
>         return vcpu;
>  }
> --
> 2.43.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v2 5/6] KVM: riscv: selftests: Use register subtypes
  2023-12-13 17:09 ` [PATCH v2 5/6] KVM: riscv: selftests: Use register subtypes Andrew Jones
@ 2023-12-13 17:39   ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2023-12-13 17:39 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm-riscv, linux-riscv, atishp, palmer, haibo1.xu

On Wed, Dec 13, 2023 at 10:39 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> Always use register subtypes in the get-reg-list test when registers
> have them. The only registers neglecting to do so were ISA extension
> registers. While we don't really need to use KVM_REG_RISCV_ISA_SINGLE
> (since it's zero), the main purpose is to avoid confusion and to
> self-document the tests. Also add print support for the multi
> registers like SBI extensions have, even though they're only used for
> debugging.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Haibo Xu <haibo1.xu@intel.com>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  .../selftests/kvm/riscv/get-reg-list.c        | 113 +++++++++++-------
>  1 file changed, 73 insertions(+), 40 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c
> index 27d07a32a1ef..4bcc597d34b9 100644
> --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
> @@ -28,31 +28,31 @@ bool filter_reg(__u64 reg)
>          *
>          * Note: The below list is alphabetically sorted.
>          */
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_A:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_C:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_D:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_F:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_H:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_I:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_M:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_V:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SMSTATEEN:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SSAIA:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SSTC:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVINVAL:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVNAPOT:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SVPBMT:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZBA:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZBB:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZBS:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICBOM:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICBOZ:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICNTR:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICOND:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICSR:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZIFENCEI:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZIHINTPAUSE:
> -       case KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZIHPM:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_A:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_C:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_D:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_F:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_H:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_I:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_M:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_V:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SMSTATEEN:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SSAIA:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SSTC:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVINVAL:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVNAPOT:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SVPBMT:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBA:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBB:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZBS:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICBOM:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICBOZ:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICNTR:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICOND:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICSR:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZIFENCEI:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZIHINTPAUSE:
> +       case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZIHPM:
>                 return true;
>         /* AIA registers are always available when Ssaia can't be disabled */
>         case KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(siselect):
> @@ -335,15 +335,10 @@ static const char *fp_d_id_to_str(const char *prefix, __u64 id)
>  }
>
>  #define KVM_ISA_EXT_ARR(ext)           \
> -[KVM_RISCV_ISA_EXT_##ext] = "KVM_RISCV_ISA_EXT_" #ext
> +[KVM_RISCV_ISA_EXT_##ext] = "KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_" #ext
>
> -static const char *isa_ext_id_to_str(const char *prefix, __u64 id)
> +static const char *isa_ext_single_id_to_str(__u64 reg_off)
>  {
> -       /* reg_off is the offset into unsigned long kvm_isa_ext_arr[] */
> -       __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_ISA_EXT);
> -
> -       assert((id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_ISA_EXT);
> -
>         static const char * const kvm_isa_ext_reg_name[] = {
>                 KVM_ISA_EXT_ARR(A),
>                 KVM_ISA_EXT_ARR(C),
> @@ -373,11 +368,48 @@ static const char *isa_ext_id_to_str(const char *prefix, __u64 id)
>         };
>
>         if (reg_off >= ARRAY_SIZE(kvm_isa_ext_reg_name))
> -               return strdup_printf("%lld /* UNKNOWN */", reg_off);
> +               return strdup_printf("KVM_REG_RISCV_ISA_SINGLE | %lld /* UNKNOWN */", reg_off);
>
>         return kvm_isa_ext_reg_name[reg_off];
>  }
>
> +static const char *isa_ext_multi_id_to_str(__u64 reg_subtype, __u64 reg_off)
> +{
> +       const char *unknown = "";
> +
> +       if (reg_off > KVM_REG_RISCV_ISA_MULTI_REG_LAST)
> +               unknown = " /* UNKNOWN */";
> +
> +       switch (reg_subtype) {
> +       case KVM_REG_RISCV_ISA_MULTI_EN:
> +               return strdup_printf("KVM_REG_RISCV_ISA_MULTI_EN | %lld%s", reg_off, unknown);
> +       case KVM_REG_RISCV_ISA_MULTI_DIS:
> +               return strdup_printf("KVM_REG_RISCV_ISA_MULTI_DIS | %lld%s", reg_off, unknown);
> +       }
> +
> +       return strdup_printf("%lld | %lld /* UNKNOWN */", reg_subtype, reg_off);
> +}
> +
> +static const char *isa_ext_id_to_str(const char *prefix, __u64 id)
> +{
> +       __u64 reg_off = id & ~(REG_MASK | KVM_REG_RISCV_ISA_EXT);
> +       __u64 reg_subtype = reg_off & KVM_REG_RISCV_SUBTYPE_MASK;
> +
> +       assert((id & KVM_REG_RISCV_TYPE_MASK) == KVM_REG_RISCV_ISA_EXT);
> +
> +       reg_off &= ~KVM_REG_RISCV_SUBTYPE_MASK;
> +
> +       switch (reg_subtype) {
> +       case KVM_REG_RISCV_ISA_SINGLE:
> +               return isa_ext_single_id_to_str(reg_off);
> +       case KVM_REG_RISCV_ISA_MULTI_EN:
> +       case KVM_REG_RISCV_ISA_MULTI_DIS:
> +               return isa_ext_multi_id_to_str(reg_subtype, reg_off);
> +       }
> +
> +       return strdup_printf("%lld | %lld /* UNKNOWN */", reg_subtype, reg_off);
> +}
> +
>  #define KVM_SBI_EXT_ARR(ext)           \
>  [ext] = "KVM_REG_RISCV_SBI_SINGLE | " #ext
>
> @@ -583,12 +615,12 @@ static __u64 base_skips_set[] = {
>
>  static __u64 zicbom_regs[] = {
>         KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(zicbom_block_size),
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICBOM,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICBOM,
>  };
>
>  static __u64 zicboz_regs[] = {
>         KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(zicboz_block_size),
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_ZICBOZ,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICBOZ,
>  };
>
>  static __u64 aia_regs[] = {
> @@ -599,12 +631,12 @@ static __u64 aia_regs[] = {
>         KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(siph),
>         KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(iprio1h),
>         KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(iprio2h),
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SSAIA,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SSAIA,
>  };
>
>  static __u64 smstateen_regs[] = {
>         KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_SMSTATEEN | KVM_REG_RISCV_CSR_SMSTATEEN_REG(sstateen0),
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_SMSTATEEN,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_SMSTATEEN,
>  };
>
>  static __u64 fp_f_regs[] = {
> @@ -641,7 +673,7 @@ static __u64 fp_f_regs[] = {
>         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),
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_F,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_F,
>  };
>
>  static __u64 fp_d_regs[] = {
> @@ -678,7 +710,7 @@ static __u64 fp_d_regs[] = {
>         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),
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_D,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_D,
>  };
>
>  #define SUBLIST_BASE \
> @@ -702,7 +734,8 @@ static __u64 fp_d_regs[] = {
>  #define KVM_ISA_EXT_SIMPLE_CONFIG(ext, extu)                   \
>  static __u64 regs_##ext[] = {                                  \
>         KVM_REG_RISCV | KVM_REG_SIZE_ULONG |                    \
> -       KVM_REG_RISCV_ISA_EXT | KVM_RISCV_ISA_EXT_##extu,       \
> +       KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE |      \
> +       KVM_RISCV_ISA_EXT_##extu,                               \
>  };                                                             \
>  static struct vcpu_reg_list config_##ext = {                   \
>         .sublists = {                                           \
> --
> 2.43.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v2 6/6] RISC-V: KVM: selftests: Treat SBI ext regs like ISA ext regs
  2023-12-13 17:09 ` [PATCH v2 6/6] RISC-V: KVM: selftests: Treat SBI ext regs like ISA ext regs Andrew Jones
@ 2023-12-13 17:42   ` Anup Patel
  0 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2023-12-13 17:42 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm-riscv, linux-riscv, atishp, palmer, haibo1.xu

On Wed, Dec 13, 2023 at 10:40 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> SBI extension registers may not be present and indeed when
> running on a platform without sscofpmf the PMU SBI extension
> is not. Move the SBI extension registers from the base set of
> registers to the filter list. Individual configs should test
> for any that may or may not be present separately. Since
> the PMU extension may disappear and the DBCN extension is only
> present in later kernels, separate them from the rest into
> their own configs. The rest are lumped together into the same
> config.
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>

Looks good to me.

Reviewed-by: Anup Patel <anup@brainfault.org>

Regards,
Anup

> ---
>  .../selftests/kvm/include/kvm_util_base.h     |   1 +
>  .../selftests/kvm/riscv/get-reg-list.c        | 105 +++++++++++++++---
>  2 files changed, 92 insertions(+), 14 deletions(-)
>
> diff --git a/tools/testing/selftests/kvm/include/kvm_util_base.h b/tools/testing/selftests/kvm/include/kvm_util_base.h
> index a18db6a7b3cf..e112ee30867f 100644
> --- a/tools/testing/selftests/kvm/include/kvm_util_base.h
> +++ b/tools/testing/selftests/kvm/include/kvm_util_base.h
> @@ -129,6 +129,7 @@ struct vcpu_reg_sublist {
>         const char *name;
>         long capability;
>         int feature;
> +       int feature_type;
>         bool finalize;
>         __u64 *regs;
>         __u64 regs_n;
> diff --git a/tools/testing/selftests/kvm/riscv/get-reg-list.c b/tools/testing/selftests/kvm/riscv/get-reg-list.c
> index 4bcc597d34b9..b8da2e86bf9c 100644
> --- a/tools/testing/selftests/kvm/riscv/get-reg-list.c
> +++ b/tools/testing/selftests/kvm/riscv/get-reg-list.c
> @@ -12,6 +12,11 @@
>
>  #define REG_MASK (KVM_REG_ARCH_MASK | KVM_REG_SIZE_MASK)
>
> +enum {
> +       VCPU_FEATURE_ISA_EXT = 0,
> +       VCPU_FEATURE_SBI_EXT,
> +};
> +
>  static bool isa_ext_cant_disable[KVM_RISCV_ISA_EXT_MAX];
>
>  bool filter_reg(__u64 reg)
> @@ -53,6 +58,21 @@ bool filter_reg(__u64 reg)
>         case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZIFENCEI:
>         case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZIHINTPAUSE:
>         case KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZIHPM:
> +       /*
> +        * Like ISA_EXT registers, SBI_EXT registers are only visible when the
> +        * host supports them and disabling them does not affect the visibility
> +        * of the SBI_EXT register itself.
> +        */
> +       case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_V01:
> +       case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_TIME:
> +       case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_IPI:
> +       case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_RFENCE:
> +       case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_SRST:
> +       case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_HSM:
> +       case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_PMU:
> +       case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_DBCN:
> +       case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_EXPERIMENTAL:
> +       case KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_VENDOR:
>                 return true;
>         /* AIA registers are always available when Ssaia can't be disabled */
>         case KVM_REG_RISCV_CSR | KVM_REG_RISCV_CSR_AIA | KVM_REG_RISCV_CSR_AIA_REG(siselect):
> @@ -75,12 +95,12 @@ bool check_reject_set(int err)
>         return err == EINVAL;
>  }
>
> -static inline bool vcpu_has_ext(struct kvm_vcpu *vcpu, int ext)
> +static bool vcpu_has_ext(struct kvm_vcpu *vcpu, uint64_t ext_id)
>  {
>         int ret;
>         unsigned long value;
>
> -       ret = __vcpu_get_reg(vcpu, RISCV_ISA_EXT_REG(ext), &value);
> +       ret = __vcpu_get_reg(vcpu, ext_id, &value);
>         return (ret) ? false : !!value;
>  }
>
> @@ -88,6 +108,7 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c)
>  {
>         unsigned long isa_ext_state[KVM_RISCV_ISA_EXT_MAX] = { 0 };
>         struct vcpu_reg_sublist *s;
> +       uint64_t feature;
>         int rc;
>
>         for (int i = 0; i < KVM_RISCV_ISA_EXT_MAX; i++)
> @@ -103,15 +124,31 @@ void finalize_vcpu(struct kvm_vcpu *vcpu, struct vcpu_reg_list *c)
>                         isa_ext_cant_disable[i] = true;
>         }
>
> +       for (int i = 0; i < KVM_RISCV_SBI_EXT_MAX; i++) {
> +               rc = __vcpu_set_reg(vcpu, RISCV_SBI_EXT_REG(i), 0);
> +               TEST_ASSERT(!rc || (rc == -1 && errno == ENOENT), "Unexpected error");
> +       }
> +
>         for_each_sublist(c, s) {
>                 if (!s->feature)
>                         continue;
>
> +               switch (s->feature_type) {
> +               case VCPU_FEATURE_ISA_EXT:
> +                       feature = RISCV_ISA_EXT_REG(s->feature);
> +                       break;
> +               case VCPU_FEATURE_SBI_EXT:
> +                       feature = RISCV_SBI_EXT_REG(s->feature);
> +                       break;
> +               default:
> +                       TEST_FAIL("Unknown feature type");
> +               }
> +
>                 /* Try to enable the desired extension */
> -               __vcpu_set_reg(vcpu, RISCV_ISA_EXT_REG(s->feature), 1);
> +               __vcpu_set_reg(vcpu, feature, 1);
>
>                 /* Double check whether the desired extension was enabled */
> -               __TEST_REQUIRE(vcpu_has_ext(vcpu, s->feature),
> +               __TEST_REQUIRE(vcpu_has_ext(vcpu, feature),
>                                "%s not available, skipping tests\n", s->name);
>         }
>  }
> @@ -593,16 +630,6 @@ static __u64 base_regs[] = {
>         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_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_V01,
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_TIME,
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_IPI,
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_RFENCE,
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_SRST,
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_HSM,
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_PMU,
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_EXPERIMENTAL,
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_VENDOR,
> -       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_DBCN,
>  };
>
>  /*
> @@ -613,6 +640,17 @@ static __u64 base_skips_set[] = {
>         KVM_REG_RISCV | KVM_REG_SIZE_U64 | KVM_REG_RISCV_TIMER | KVM_REG_RISCV_TIMER_REG(state),
>  };
>
> +static __u64 sbi_base_regs[] = {
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_V01,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_TIME,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_IPI,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_RFENCE,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_SRST,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_HSM,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_EXPERIMENTAL,
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE | KVM_RISCV_SBI_EXT_VENDOR,
> +};
> +
>  static __u64 zicbom_regs[] = {
>         KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_CONFIG | KVM_REG_RISCV_CONFIG_REG(zicbom_block_size),
>         KVM_REG_RISCV | KVM_REG_SIZE_ULONG | KVM_REG_RISCV_ISA_EXT | KVM_REG_RISCV_ISA_SINGLE | KVM_RISCV_ISA_EXT_ZICBOM,
> @@ -716,6 +754,9 @@ static __u64 fp_d_regs[] = {
>  #define SUBLIST_BASE \
>         {"base", .regs = base_regs, .regs_n = ARRAY_SIZE(base_regs), \
>          .skips_set = base_skips_set, .skips_set_n = ARRAY_SIZE(base_skips_set),}
> +#define SUBLIST_SBI_BASE \
> +       {"sbi-base", .feature_type = VCPU_FEATURE_SBI_EXT, .feature = KVM_RISCV_SBI_EXT_V01, \
> +        .regs = sbi_base_regs, .regs_n = ARRAY_SIZE(sbi_base_regs),}
>  #define SUBLIST_ZICBOM \
>         {"zicbom", .feature = KVM_RISCV_ISA_EXT_ZICBOM, .regs = zicbom_regs, .regs_n = ARRAY_SIZE(zicbom_regs),}
>  #define SUBLIST_ZICBOZ \
> @@ -750,6 +791,26 @@ static struct vcpu_reg_list config_##ext = {                       \
>         },                                                      \
>  }                                                              \
>
> +#define KVM_SBI_EXT_SIMPLE_CONFIG(ext, extu)                   \
> +static __u64 regs_sbi_##ext[] = {                              \
> +       KVM_REG_RISCV | KVM_REG_SIZE_ULONG |                    \
> +       KVM_REG_RISCV_SBI_EXT | KVM_REG_RISCV_SBI_SINGLE |      \
> +       KVM_RISCV_SBI_EXT_##extu,                               \
> +};                                                             \
> +static struct vcpu_reg_list config_sbi_##ext = {               \
> +       .sublists = {                                           \
> +               SUBLIST_BASE,                                   \
> +               {                                               \
> +                       .name = "sbi-"#ext,                     \
> +                       .feature_type = VCPU_FEATURE_SBI_EXT,   \
> +                       .feature = KVM_RISCV_SBI_EXT_##extu,    \
> +                       .regs = regs_sbi_##ext,                 \
> +                       .regs_n = ARRAY_SIZE(regs_sbi_##ext),   \
> +               },                                              \
> +               {0},                                            \
> +       },                                                      \
> +}                                                              \
> +
>  #define KVM_ISA_EXT_SUBLIST_CONFIG(ext, extu)                  \
>  static struct vcpu_reg_list config_##ext = {                   \
>         .sublists = {                                           \
> @@ -759,8 +820,21 @@ static struct vcpu_reg_list config_##ext = {                       \
>         },                                                      \
>  }                                                              \
>
> +#define KVM_SBI_EXT_SUBLIST_CONFIG(ext, extu)                  \
> +static struct vcpu_reg_list config_sbi_##ext = {               \
> +       .sublists = {                                           \
> +               SUBLIST_BASE,                                   \
> +               SUBLIST_SBI_##extu,                             \
> +               {0},                                            \
> +       },                                                      \
> +}                                                              \
> +
>  /* Note: The below list is alphabetically sorted. */
>
> +KVM_SBI_EXT_SUBLIST_CONFIG(base, BASE);
> +KVM_SBI_EXT_SIMPLE_CONFIG(pmu, PMU);
> +KVM_SBI_EXT_SIMPLE_CONFIG(dbcn, DBCN);
> +
>  KVM_ISA_EXT_SUBLIST_CONFIG(aia, AIA);
>  KVM_ISA_EXT_SUBLIST_CONFIG(fp_f, FP_F);
>  KVM_ISA_EXT_SUBLIST_CONFIG(fp_d, FP_D);
> @@ -783,6 +857,9 @@ KVM_ISA_EXT_SIMPLE_CONFIG(zihintpause, ZIHINTPAUSE);
>  KVM_ISA_EXT_SIMPLE_CONFIG(zihpm, ZIHPM);
>
>  struct vcpu_reg_list *vcpu_configs[] = {
> +       &config_sbi_base,
> +       &config_sbi_pmu,
> +       &config_sbi_dbcn,
>         &config_aia,
>         &config_fp_f,
>         &config_fp_d,
> --
> 2.43.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

* Re: [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi
  2023-12-13 17:09 [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Andrew Jones
                   ` (5 preceding siblings ...)
  2023-12-13 17:09 ` [PATCH v2 6/6] RISC-V: KVM: selftests: Treat SBI ext regs like ISA ext regs Andrew Jones
@ 2023-12-14  5:19 ` Anup Patel
  6 siblings, 0 replies; 14+ messages in thread
From: Anup Patel @ 2023-12-14  5:19 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm-riscv, linux-riscv, atishp, palmer, haibo1.xu

On Wed, Dec 13, 2023 at 10:39 PM Andrew Jones <ajones@ventanamicro.com> wrote:
>
> SBI extension UAPI is currently almost the same as the ISA extension
> UAPI. This series closes the remaining gap by ensuring when an SBI
> extension is not available that its register returns ENOENT when
> accessed by userspace. We also drop the SBI multi registers from
> get-reg-list (ISA multi registers aren't there either) and make
> several improvements to the get-reg-list kselftest.
>
> This series is based on Anup's riscv_kvm_more_exts_v1 branch.
>
> Based on kvm-riscv/riscv_kvm_queue
>
> v2:
>  - Rebased on kvm-riscv/riscv_kvm_queue which is based on v6.7-rc5
>
> Thanks,
> drew
>
>
> Andrew Jones (6):
>   RISC-V: KVM: Don't add SBI multi regs in get-reg-list
>   KVM: riscv: selftests: Drop SBI multi registers
>   RISC-V: KVM: Make SBI uapi consistent with ISA uapi
>   KVM: riscv: selftests: Add RISCV_SBI_EXT_REG
>   KVM: riscv: selftests: Use register subtypes
>   RISC-V: KVM: selftests: Treat SBI ext regs like ISA ext regs

Queued this series for Linux-6.8

Thanks,
Anup

>
>  arch/riscv/include/asm/kvm_vcpu_sbi.h         |  10 +-
>  arch/riscv/kvm/vcpu_onereg.c                  |  53 ++---
>  arch/riscv/kvm/vcpu_sbi.c                     |  75 +++---
>  arch/riscv/kvm/vcpu_sbi_replace.c             |   2 +-
>  .../selftests/kvm/include/kvm_util_base.h     |   1 +
>  .../selftests/kvm/include/riscv/processor.h   |  40 ++--
>  .../selftests/kvm/lib/riscv/processor.c       |   4 +-
>  .../selftests/kvm/riscv/get-reg-list.c        | 220 +++++++++++++-----
>  8 files changed, 254 insertions(+), 151 deletions(-)
>
> --
> 2.43.0
>

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

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

end of thread, other threads:[~2023-12-14  5:19 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-13 17:09 [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Andrew Jones
2023-12-13 17:09 ` [PATCH v2 1/6] RISC-V: KVM: Don't add SBI multi regs in get-reg-list Andrew Jones
2023-12-13 17:23   ` Anup Patel
2023-12-13 17:09 ` [PATCH v2 2/6] KVM: riscv: selftests: Drop SBI multi registers Andrew Jones
2023-12-13 17:24   ` Anup Patel
2023-12-13 17:09 ` [PATCH v2 3/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Andrew Jones
2023-12-13 17:35   ` Anup Patel
2023-12-13 17:09 ` [PATCH v2 4/6] KVM: riscv: selftests: Add RISCV_SBI_EXT_REG Andrew Jones
2023-12-13 17:36   ` Anup Patel
2023-12-13 17:09 ` [PATCH v2 5/6] KVM: riscv: selftests: Use register subtypes Andrew Jones
2023-12-13 17:39   ` Anup Patel
2023-12-13 17:09 ` [PATCH v2 6/6] RISC-V: KVM: selftests: Treat SBI ext regs like ISA ext regs Andrew Jones
2023-12-13 17:42   ` Anup Patel
2023-12-14  5:19 ` [PATCH v2 0/6] RISC-V: KVM: Make SBI uapi consistent with ISA uapi Anup Patel

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