Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT
From: Conor Dooley
Date: Wed Jun 28 2023 - 12:50:27 EST
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@xxxxxxxxxxxx> 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@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxx>
---
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
Attachment:
signature.asc
Description: PGP signature