Messages in this thread Patch in this message | | | Date | Wed, 28 Jun 2023 17:49:54 +0100 | From | Conor Dooley <> | Subject | Re: [PATCH 1/3] RISC-V: add Bitmanip/Scalar Crypto parsing from DT |
| |
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@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@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@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 [unhandled content-type:application/pgp-signature] | |