[PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT

Conor Dooley conor at kernel.org
Wed Jun 28 09:49:54 PDT 2023


On Wed, Jun 28, 2023 at 12:10:11PM +0100, Conor Dooley wrote:
> On Wed, Jun 28, 2023 at 12:01:11PM +0200, Samuel Ortiz wrote:
> > On Tue, Jun 27, 2023 at 07:48:15PM +0100, Conor Dooley wrote:
> > > On Tue, Jun 27, 2023 at 11:14:30AM -0700, Evan Green wrote:
> > > > On Tue, Jun 27, 2023 at 7:38 AM Samuel Ortiz <sameo at rivosinc.com> wrote:
> 
> > > > It would be nice to consolidate the ones together that search for a
> > > > single string and set multiple bits, though I don't have any super
> > > > elegant ideas for how off the top of my head.
> > > 
> > > I've got a refactor of this code in progress, dropping all of these
> > > copy-paste in place of a loop. It certainly looks more elegant than
> > > this, but it will fall over a bit for these "one string matches many
> > > extensions" cases. See here:
> > > https://patchwork.kernel.org/project/linux-riscv/patch/20230626-thieving-jockstrap-d35d20b535c5@wendy/
> > > My immediate thought is to add another element to riscv_isa_ext_data,
> > > that contains "parent" extensions to check for. Should be fairly doable,
> > > I'll whip something up on top of that...
> > 
> > Nice, and thanks for the review.
> 
> > Should I wait for your refactor to be merged before pushing this one?
> 
> I don't know. I think that you should continue on with your series here,
> and whichever goes in second gets rebased on top of the other.
> I don't think it makes material difference to review of this patchset as
> to whether you rebase on top of what I'm working on, so I wouldn't
> bother until it gets merged.
> 
> Rather hacky, had less time than expected this morning:
> https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/commit/?h=riscv-extensions-strings-supersets
> Clearly there's issues with looping to RISCV_ISA_MAX_SUPERSETS & I just
> repurposed Zicsr for the sake of testing something in the time I had.
> 
> Evan, at a high level, does that look more elegant to you, or have I made
> things worse?

Got some more time this afternoon, cleaned it up a bit. On top of what
is in
https://git.kernel.org/pub/scm/linux/kernel/git/conor/linux.git/log/?h=riscv-extensions-strings
IOW, the not-yet-sent v2 of
https://lore.kernel.org/all/20230626-provable-angrily-81760e8c3cc6@wendy/
here's some infra for doing superset stuff...

Going to send my v2 without this, as there's nothing in tree right now
that actually needs this sort of thing.

-- >8 --
From 0875e1aa2bf773b53cae02490ebc69e798e491c4 Mon Sep 17 00:00:00 2001
From: Conor Dooley <conor.dooley at microchip.com>
Date: Wed, 28 Jun 2023 12:01:40 +0100
Subject: [PATCH] RISC-V: detect extension support from superset extensions

Some extensions, for example scalar crypto's Zk extension, are comprised
of anumber of sub-extensions that may be implemented independently.
Provide some infrastructure for detecting support for an extension where
only its superset appears in DT or ACPI.
SET_ISA_EXT_MAP() already served little purpose, move the code into an
inline function instead, where the code can be reused more easily by the
riscv_try_set_ext_from_supersets().

Signed-off-by: Conor Dooley <conor.dooley at microchip.com>
---
 arch/riscv/include/asm/hwcap.h |   3 +
 arch/riscv/kernel/cpufeature.c | 107 ++++++++++++++++++++++++++++-----
 2 files changed, 96 insertions(+), 14 deletions(-)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index c4d6faa5cdf8..5b41a7aa9ec5 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -73,11 +73,14 @@
 
 unsigned long riscv_get_elf_hwcap(void);
 
+#define RISCV_ISA_MAX_SUPERSETS 1
 struct riscv_isa_ext_data {
 	const unsigned int id;
 	const char *name;
 	const char *property;
 	const bool multi_letter;
+	const unsigned int num_supersets;
+	const char *supersets[RISCV_ISA_MAX_SUPERSETS];
 };
 
 extern const struct riscv_isa_ext_data riscv_isa_ext[];
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 53b38f6c0562..0d56f4a11a3e 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -104,6 +104,16 @@ static bool riscv_isa_extension_check(int id)
 	.property = #_name,				\
 	.id = _id,					\
 	.multi_letter = _multi,				\
+	.num_supersets = 0,				\
+}
+
+#define __RISCV_ISA_EXT_DATA_SUBSET(_name, _id, _multi, _num_supersets, ...) {	\
+	.name = #_name,								\
+	.property = #_name,							\
+	.id = _id,								\
+	.multi_letter = _multi,							\
+	.num_supersets = _num_supersets,					\
+	.supersets = {__VA_ARGS__},						\
 }
 
 /*
@@ -180,6 +190,39 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
 
 const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
 
+static inline int __init riscv_try_set_ext(const char *name, const unsigned int bit,
+					   const char *ext, const char *ext_end,
+					   struct riscv_isainfo *isainfo)
+{
+	if ((ext_end - ext == strlen(name)) &&
+	    !strncasecmp(ext, name, strlen(name)) &&
+	    riscv_isa_extension_check(bit)) {
+			set_bit(bit, isainfo->isa);
+			return 0;
+	}
+
+	return -EINVAL;
+}
+
+static inline void __init riscv_try_set_ext_from_supersets(struct riscv_isa_ext_data ext_data,
+							   const char *ext, const char *ext_end,
+							   struct riscv_isainfo *isainfo)
+{
+	for (int i = 0; i < ext_data.num_supersets; i++) {
+		const char *superset = ext_data.supersets[i];
+		const int bit = ext_data.id;
+		int ret;
+
+		/*
+		 * If the extension that's been found is a superset, there's no
+		 * reason to keep looking for other supersets.
+		 */
+		ret = riscv_try_set_ext(superset, bit, ext, ext_end, isainfo);
+		if (!ret)
+			return;
+	}
+}
+
 static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct riscv_isainfo *isainfo,
 					  unsigned long *isa2hwcap, const char *isa)
 {
@@ -311,14 +354,6 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc
 		if (*isa == '_')
 			++isa;
 
-#define SET_ISA_EXT_MAP(name, bit)						\
-		do {								\
-			if ((ext_end - ext == sizeof(name) - 1) &&		\
-			     !strncasecmp(ext, name, sizeof(name) - 1) &&	\
-			     riscv_isa_extension_check(bit))			\
-				set_bit(bit, isainfo->isa);			\
-		} while (false)							\
-
 		if (unlikely(ext_err))
 			continue;
 		if (!ext_long) {
@@ -328,12 +363,27 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc
 				*this_hwcap |= isa2hwcap[nr];
 				set_bit(nr, isainfo->isa);
 			}
-		} else {
+
 			for (int i = 0; i < riscv_isa_ext_count; i++)
-				SET_ISA_EXT_MAP(riscv_isa_ext[i].name,
-						riscv_isa_ext[i].id);
+				riscv_try_set_ext_from_supersets(riscv_isa_ext[i],
+								 ext, ext_end, isainfo);
+		} else {
+			for (int i = 0; i < riscv_isa_ext_count; i++) {
+				const char *name = riscv_isa_ext[i].name;
+				const int bit = riscv_isa_ext[i].id;
+				int ret;
+
+				ret = riscv_try_set_ext(name, bit, ext, ext_end, isainfo);
+
+				/*
+				 * There's no point checking if the extension that the parser has
+				 * just found is a superset, if it is the extension itself...
+				 */
+				if (!ret)
+					riscv_try_set_ext_from_supersets(riscv_isa_ext[i],
+									 ext, ext_end, isainfo);
+			}
 		}
-#undef SET_ISA_EXT_MAP
 	}
 }
 
@@ -416,6 +466,28 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
 		acpi_put_table((struct acpi_table_header *)rhct);
 }
 
+static inline bool riscv_ext_superset_present(struct riscv_isa_ext_data ext_data,
+					      struct device_node *cpu_node)
+{
+	if (!ext_data.num_supersets)
+		return false;
+
+	for (int i = 0; i < ext_data.num_supersets; i++) {
+		const char *superset = ext_data.supersets[i];
+		int ret;
+
+		/*
+		 * Once a single superset is found, there's no point looking
+		 * for any other ones.
+		 */
+		ret = of_property_match_string(cpu_node,"riscv,isa-extensions", superset);
+		if (ret >= 0)
+			return true;
+	}
+
+	return false;
+}
+
 static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
 {
 	unsigned int cpu;
@@ -435,8 +507,15 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
 			continue;
 
 		for (int i = 0; i < riscv_isa_ext_count; i++) {
-			if (of_property_match_string(cpu_node, "riscv,isa-extensions",
-						     riscv_isa_ext[i].name) < 0)
+			int ret = of_property_match_string(cpu_node, "riscv,isa-extensions",
+							   riscv_isa_ext[i].name);
+
+			/*
+			 * If the extension itself is not found it could be a
+			 * subset of another extension, so the supersets need to
+			 * be checked for too.
+			 */
+			if (ret < 0 && !riscv_ext_superset_present(riscv_isa_ext[i], cpu_node))
 				continue;
 
 			if (!riscv_isa_extension_check(riscv_isa_ext[i].id))
-- 
2.39.2

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


More information about the linux-riscv mailing list