2023-07-13 12:16:42

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v5 00/11] RISC-V: Probe DT extension support using riscv,isa-extensions & riscv,isa-base

Hey,

Based on my latest iteration of deprecating riscv,isa [1], here's an
implementation of the new properties for Linux. The first few patches,
up to "RISC-V: split riscv_fill_hwcap() in 3", are all prep work that
further tames some of the extension related code, on top of my already
applied series that cleans up the ISA string parser.
Perhaps "RISC-V: shunt isa_ext_arr to cpufeature.c" is a bit gratuitous,
but I figured a bit of coalescing of extension related data structures
would be a good idea. Note that riscv,isa will still be used in the
absence of the new properties. Palmer suggested adding a Kconfig option
to turn off the fallback for DT, which I have gone and done. It's locked
behind the NONPORTABLE option for good reason.

In v2, I've also come up with a more reasonable name for the new
function I added & fixed up various comments from Drew and Evan.

In v3, there's the new commandline option that Drew suggested. I have
Also picked up a patch from Palmer that adds more helpful prints where
harts fail the checks in riscv_early_of_processor_id(), and I've
sprinkled a few more of those prints in my new additions to the
function.

v4 just rebases on v6.5-rc1 and fixes the nommu build issue due to a
missing __init.

In v5, I've fixed issues spotted by myself & Evan. I'm not the worlds
biggest fan of the strlen() calls inside the macro - but that's going to
go away again almost immediately if the scalar crypto stuff gets merged.
I also spotted an issue with a rebase I did at some point, where the
dedicated properties did not use isainfo->isa & created a bitmap for
each cpu, which would've caused the per-hart extension tracking to
break.

Cheers,
Conor.

[1] (it's in v6.5-rc1 now)

CC: Rob Herring <[email protected]>
CC: Krzysztof Kozlowski <[email protected]>
CC: Paul Walmsley <[email protected]>
CC: Palmer Dabbelt <[email protected]>
CC: Albert Ou <[email protected]>
CC: Jonathan Corbet <[email protected]>
CC: Andrew Jones <[email protected]>
CC: Heiko Stuebner <[email protected]>
CC: Evan Green <[email protected]>
CC: Sunil V L <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]

Conor Dooley (9):
RISC-V: drop a needless check in print_isa_ext()
RISC-V: shunt isa_ext_arr to cpufeature.c
RISC-V: repurpose riscv_isa_ext array in riscv_fill_hwcap()
RISC-V: add missing single letter extension definitions
RISC-V: add single letter extensions to riscv_isa_ext
RISC-V: split riscv_fill_hwcap() in 3
RISC-V: enable extension detection from dedicated properties
RISC-V: try new extension properties in of_early_processor_hartid()
RISC-V: provide Kconfig & commandline options to control parsing
"riscv,isa"

Heiko Stuebner (1):
RISC-V: don't parse dt/acpi isa string to get rv32/rv64

Palmer Dabbelt (1):
RISC-V: Provide a more helpful error message on invalid ISA strings

.../admin-guide/kernel-parameters.txt | 7 +
arch/riscv/Kconfig | 18 +
arch/riscv/include/asm/hwcap.h | 17 +-
arch/riscv/kernel/cpu.c | 179 +++---
arch/riscv/kernel/cpufeature.c | 521 ++++++++++++------
5 files changed, 439 insertions(+), 303 deletions(-)

--
2.40.1



2023-07-13 12:17:08

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v5 02/11] RISC-V: don't parse dt/acpi isa string to get rv32/rv64

From: Heiko Stuebner <[email protected]>

When filling hwcap the kernel already expects the isa string to start with
rv32 if CONFIG_32BIT and rv64 if CONFIG_64BIT.

So when recreating the runtime isa-string we can also just go the other way
to get the correct starting point for it.

Signed-off-by: Heiko Stuebner <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
Reviewed-by: Evan Green <[email protected]>
Co-developed-by: Conor Dooley <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
Changes in v3:
- Fix tabbing of print_mmu()

Changes in v2:
- Delete the whole else & pull print_mmu() above it, since that's common
code now
---
arch/riscv/kernel/cpu.c | 21 +++++++++------------
1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 3af2d214ce21..f808b67f5a27 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -257,13 +257,16 @@ static void print_isa_ext(struct seq_file *f)
*/
static const char base_riscv_exts[13] = "imafdqcbkjpvh";

-static void print_isa(struct seq_file *f, const char *isa)
+static void print_isa(struct seq_file *f)
{
int i;

seq_puts(f, "isa\t\t: ");
- /* Print the rv[64/32] part */
- seq_write(f, isa, 4);
+ if (IS_ENABLED(CONFIG_32BIT))
+ seq_write(f, "rv32", 4);
+ else
+ seq_write(f, "rv64", 4);
+
for (i = 0; i < sizeof(base_riscv_exts); i++) {
if (__riscv_isa_extension_available(NULL, base_riscv_exts[i] - 'a'))
/* Print only enabled the base ISA extensions */
@@ -320,27 +323,21 @@ static int c_show(struct seq_file *m, void *v)
unsigned long cpu_id = (unsigned long)v - 1;
struct riscv_cpuinfo *ci = per_cpu_ptr(&riscv_cpuinfo, cpu_id);
struct device_node *node;
- const char *compat, *isa;
+ const char *compat;

seq_printf(m, "processor\t: %lu\n", cpu_id);
seq_printf(m, "hart\t\t: %lu\n", cpuid_to_hartid_map(cpu_id));
+ print_isa(m);
+ print_mmu(m);

if (acpi_disabled) {
node = of_get_cpu_node(cpu_id, NULL);
- if (!of_property_read_string(node, "riscv,isa", &isa))
- print_isa(m, isa);

- print_mmu(m);
if (!of_property_read_string(node, "compatible", &compat) &&
strcmp(compat, "riscv"))
seq_printf(m, "uarch\t\t: %s\n", compat);

of_node_put(node);
- } else {
- if (!acpi_get_riscv_isa(NULL, cpu_id, &isa))
- print_isa(m, isa);
-
- print_mmu(m);
}

seq_printf(m, "mvendorid\t: 0x%lx\n", ci->mvendorid);
--
2.40.1


2023-07-13 12:17:46

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v5 07/11] RISC-V: add single letter extensions to riscv_isa_ext

So that riscv_fill_hwcap() can use riscv_isa_ext to probe for single
letter extensions, add them to it.
As a result, what gets spat out in /proc/cpuinfo will become borked, as
single letter extensions will be printed as part of the base extensions
and while printing from riscv_isa_arr. Take the opportunity to unify the
printing of the isa string, using the new member of riscv_isa_ext_data
in the process.

Reviewed-by: Andrew Jones <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
Changes in v2:
- Drop the multi_letter member, in exchange for calling strnlen() in two
places.
---
arch/riscv/kernel/cpu.c | 37 ++++++++++------------------------
arch/riscv/kernel/cpufeature.c | 13 ++++++++++++
2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index aa17eeb0ec9a..4f1f12f34b63 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -164,41 +164,26 @@ arch_initcall(riscv_cpuinfo_init);

#ifdef CONFIG_PROC_FS

-static void print_isa_ext(struct seq_file *f)
-{
- for (int i = 0; i < riscv_isa_ext_count; i++) {
- const struct riscv_isa_ext_data *edata = &riscv_isa_ext[i];
- if (!__riscv_isa_extension_available(NULL, edata->id))
- continue;
-
- seq_printf(f, "_%s", edata->name);
- }
-}
-
-/*
- * These are the only valid base (single letter) ISA extensions as per the spec.
- * It also specifies the canonical order in which it appears in the spec.
- * Some of the extension may just be a place holder for now (B, K, P, J).
- * This should be updated once corresponding extensions are ratified.
- */
-static const char base_riscv_exts[13] = "imafdqcbkjpvh";
-
static void print_isa(struct seq_file *f)
{
- int i;
-
seq_puts(f, "isa\t\t: ");
+
if (IS_ENABLED(CONFIG_32BIT))
seq_write(f, "rv32", 4);
else
seq_write(f, "rv64", 4);

- for (i = 0; i < sizeof(base_riscv_exts); i++) {
- if (__riscv_isa_extension_available(NULL, base_riscv_exts[i] - 'a'))
- /* Print only enabled the base ISA extensions */
- seq_write(f, &base_riscv_exts[i], 1);
+ for (int i = 0; i < riscv_isa_ext_count; i++) {
+ if (!__riscv_isa_extension_available(NULL, riscv_isa_ext[i].id))
+ continue;
+
+ /* Only multi-letter extensions are split by underscores */
+ if (strnlen(riscv_isa_ext[i].name, 2) != 1)
+ seq_puts(f, "_");
+
+ seq_printf(f, "%s", riscv_isa_ext[i].name);
}
- print_isa_ext(f);
+
seq_puts(f, "\n");
}

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 859e3831fac1..d6009bd4c186 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -144,6 +144,19 @@ static bool riscv_isa_extension_check(int id)
* New entries to this struct should follow the ordering rules described above.
*/
const struct riscv_isa_ext_data riscv_isa_ext[] = {
+ __RISCV_ISA_EXT_DATA(i, RISCV_ISA_EXT_i),
+ __RISCV_ISA_EXT_DATA(m, RISCV_ISA_EXT_m),
+ __RISCV_ISA_EXT_DATA(a, RISCV_ISA_EXT_a),
+ __RISCV_ISA_EXT_DATA(f, RISCV_ISA_EXT_f),
+ __RISCV_ISA_EXT_DATA(d, RISCV_ISA_EXT_d),
+ __RISCV_ISA_EXT_DATA(q, RISCV_ISA_EXT_q),
+ __RISCV_ISA_EXT_DATA(c, RISCV_ISA_EXT_c),
+ __RISCV_ISA_EXT_DATA(b, RISCV_ISA_EXT_b),
+ __RISCV_ISA_EXT_DATA(k, RISCV_ISA_EXT_k),
+ __RISCV_ISA_EXT_DATA(j, RISCV_ISA_EXT_j),
+ __RISCV_ISA_EXT_DATA(p, RISCV_ISA_EXT_p),
+ __RISCV_ISA_EXT_DATA(v, RISCV_ISA_EXT_v),
+ __RISCV_ISA_EXT_DATA(h, RISCV_ISA_EXT_h),
__RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
__RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
__RISCV_ISA_EXT_DATA(zicntr, RISCV_ISA_EXT_ZICNTR),
--
2.40.1


2023-07-13 12:23:31

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v5 10/11] RISC-V: try new extension properties in of_early_processor_hartid()

To fully deprecate the kernel's use of "riscv,isa",
of_early_processor_hartid() needs to first try using the new properties,
before falling back to "riscv,isa".

Reviewed-by: Andrew Jones <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
Changes in v3:
- Add some printouts to explain what went wrong while parsing harts,
so that if none are found there's at least a hint before we hit a
BUG()
---
arch/riscv/kernel/cpu.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 4f1f12f34b63..28d5af21f544 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -61,8 +61,35 @@ int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har
return -ENODEV;
}

+ if (of_property_read_string(node, "riscv,isa-base", &isa))
+ goto old_interface;
+
+ if (IS_ENABLED(CONFIG_32BIT) && strncasecmp(isa, "rv32i", 5)) {
+ pr_warn("CPU with hartid=%lu does not support rv32i", *hart);
+ return -ENODEV;
+ }
+
+ if (IS_ENABLED(CONFIG_64BIT) && strncasecmp(isa, "rv64i", 5)) {
+ pr_warn("CPU with hartid=%lu does not support rv64i", *hart);
+ return -ENODEV;
+ }
+
+ if (!of_property_present(node, "riscv,isa-extensions"))
+ return -ENODEV;
+
+ if (of_property_match_string(node, "riscv,isa-extensions", "i") < 0 ||
+ of_property_match_string(node, "riscv,isa-extensions", "m") < 0 ||
+ of_property_match_string(node, "riscv,isa-extensions", "a") < 0) {
+ pr_warn("CPU with hartid=%lu does not support ima", *hart);
+ return -ENODEV;
+ }
+
+ return 0;
+
+old_interface:
if (of_property_read_string(node, "riscv,isa", &isa)) {
- pr_warn("CPU with hartid=%lu has no \"riscv,isa\" property\n", *hart);
+ pr_warn("CPU with hartid=%lu has no \"riscv,isa-base\" or \"riscv,isa\" property\n",
+ *hart);
return -ENODEV;
}

--
2.40.1


2023-07-13 12:26:23

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v5 08/11] RISC-V: split riscv_fill_hwcap() in 3

Before adding more complexity to it, split riscv_fill_hwcap() into 3
distinct sections:
- riscv_fill_hwcap() still is the top level function, into which the
additional complexity will be added.
- riscv_fill_hwcap_from_isa_string() handles getting the information
from the riscv,isa/ACPI equivalent across harts & the various quirks
there
- riscv_parse_isa_string() does what it says on the tin.

Reviewed-by: Andrew Jones <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
Changes in v2:
- Drop unused variables
---
arch/riscv/kernel/cpufeature.c | 345 +++++++++++++++++----------------
1 file changed, 177 insertions(+), 168 deletions(-)

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index d6009bd4c186..7c661b12ac8d 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -178,29 +178,172 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {

const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);

-void __init riscv_fill_hwcap(void)
+static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct riscv_isainfo *isainfo,
+ unsigned long *isa2hwcap, const char *isa)
+{
+ /*
+ * For all possible cpus, we have already validated in
+ * the boot process that they at least contain "rv" and
+ * whichever of "32"/"64" this kernel supports, and so this
+ * section can be skipped.
+ */
+ isa += 4;
+
+ while (*isa) {
+ const char *ext = isa++;
+ const char *ext_end = isa;
+ bool ext_long = false, ext_err = false;
+
+ switch (*ext) {
+ case 's':
+ /*
+ * Workaround for invalid single-letter 's' & 'u'(QEMU).
+ * No need to set the bit in riscv_isa as 's' & 'u' are
+ * not valid ISA extensions. It works until multi-letter
+ * extension starting with "Su" appears.
+ */
+ if (ext[-1] != '_' && ext[1] == 'u') {
+ ++isa;
+ ext_err = true;
+ break;
+ }
+ fallthrough;
+ case 'S':
+ case 'x':
+ case 'X':
+ case 'z':
+ case 'Z':
+ /*
+ * Before attempting to parse the extension itself, we find its end.
+ * As multi-letter extensions must be split from other multi-letter
+ * extensions with an "_", the end of a multi-letter extension will
+ * either be the null character or the "_" at the start of the next
+ * multi-letter extension.
+ *
+ * Next, as the extensions version is currently ignored, we
+ * eliminate that portion. This is done by parsing backwards from
+ * the end of the extension, removing any numbers. This may be a
+ * major or minor number however, so the process is repeated if a
+ * minor number was found.
+ *
+ * ext_end is intended to represent the first character *after* the
+ * name portion of an extension, but will be decremented to the last
+ * character itself while eliminating the extensions version number.
+ * A simple re-increment solves this problem.
+ */
+ ext_long = true;
+ for (; *isa && *isa != '_'; ++isa)
+ if (unlikely(!isalnum(*isa)))
+ ext_err = true;
+
+ ext_end = isa;
+ if (unlikely(ext_err))
+ break;
+
+ if (!isdigit(ext_end[-1]))
+ break;
+
+ while (isdigit(*--ext_end))
+ ;
+
+ if (tolower(ext_end[0]) != 'p' || !isdigit(ext_end[-1])) {
+ ++ext_end;
+ break;
+ }
+
+ while (isdigit(*--ext_end))
+ ;
+
+ ++ext_end;
+ break;
+ default:
+ /*
+ * Things are a little easier for single-letter extensions, as they
+ * are parsed forwards.
+ *
+ * After checking that our starting position is valid, we need to
+ * ensure that, when isa was incremented at the start of the loop,
+ * that it arrived at the start of the next extension.
+ *
+ * If we are already on a non-digit, there is nothing to do. Either
+ * we have a multi-letter extension's _, or the start of an
+ * extension.
+ *
+ * Otherwise we have found the current extension's major version
+ * number. Parse past it, and a subsequent p/minor version number
+ * if present. The `p` extension must not appear immediately after
+ * a number, so there is no fear of missing it.
+ *
+ */
+ if (unlikely(!isalpha(*ext))) {
+ ext_err = true;
+ break;
+ }
+
+ if (!isdigit(*isa))
+ break;
+
+ while (isdigit(*++isa))
+ ;
+
+ if (tolower(*isa) != 'p')
+ break;
+
+ if (!isdigit(*++isa)) {
+ --isa;
+ break;
+ }
+
+ while (isdigit(*++isa))
+ ;
+
+ break;
+ }
+
+ /*
+ * The parser expects that at the start of an iteration isa points to the
+ * first character of the next extension. As we stop parsing an extension
+ * on meeting a non-alphanumeric character, an extra increment is needed
+ * where the succeeding extension is a multi-letter prefixed with an "_".
+ */
+ if (*isa == '_')
+ ++isa;
+
+#define SET_ISA_EXT_MAP(name, bit) \
+ do { \
+ if ((ext_end - ext == strlen(name)) && \
+ !strncasecmp(ext, name, strlen(name)) && \
+ riscv_isa_extension_check(bit)) \
+ set_bit(bit, isainfo->isa); \
+ } while (false) \
+
+ if (unlikely(ext_err))
+ continue;
+ if (!ext_long) {
+ int nr = tolower(*ext) - 'a';
+
+ if (riscv_isa_extension_check(nr)) {
+ *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);
+ }
+#undef SET_ISA_EXT_MAP
+ }
+}
+
+static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
{
struct device_node *node;
const char *isa;
- char print_str[NUM_ALPHA_EXTS + 1];
- int i, j, rc;
- unsigned long isa2hwcap[26] = {0};
+ int rc;
struct acpi_table_header *rhct;
acpi_status status;
unsigned int cpu;

- isa2hwcap['i' - 'a'] = COMPAT_HWCAP_ISA_I;
- isa2hwcap['m' - 'a'] = COMPAT_HWCAP_ISA_M;
- isa2hwcap['a' - 'a'] = COMPAT_HWCAP_ISA_A;
- isa2hwcap['f' - 'a'] = COMPAT_HWCAP_ISA_F;
- isa2hwcap['d' - 'a'] = COMPAT_HWCAP_ISA_D;
- isa2hwcap['c' - 'a'] = COMPAT_HWCAP_ISA_C;
- isa2hwcap['v' - 'a'] = COMPAT_HWCAP_ISA_V;
-
- elf_hwcap = 0;
-
- bitmap_zero(riscv_isa, RISCV_ISA_EXT_MAX);
-
if (!acpi_disabled) {
status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct);
if (ACPI_FAILURE(status))
@@ -232,158 +375,7 @@ void __init riscv_fill_hwcap(void)
}
}

- /*
- * For all possible cpus, we have already validated in
- * the boot process that they at least contain "rv" and
- * whichever of "32"/"64" this kernel supports, and so this
- * section can be skipped.
- */
- isa += 4;
-
- while (*isa) {
- const char *ext = isa++;
- const char *ext_end = isa;
- bool ext_long = false, ext_err = false;
-
- switch (*ext) {
- case 's':
- /*
- * Workaround for invalid single-letter 's' & 'u'(QEMU).
- * No need to set the bit in riscv_isa as 's' & 'u' are
- * not valid ISA extensions. It works until multi-letter
- * extension starting with "Su" appears.
- */
- if (ext[-1] != '_' && ext[1] == 'u') {
- ++isa;
- ext_err = true;
- break;
- }
- fallthrough;
- case 'S':
- case 'x':
- case 'X':
- case 'z':
- case 'Z':
- /*
- * Before attempting to parse the extension itself, we find its end.
- * As multi-letter extensions must be split from other multi-letter
- * extensions with an "_", the end of a multi-letter extension will
- * either be the null character or the "_" at the start of the next
- * multi-letter extension.
- *
- * Next, as the extensions version is currently ignored, we
- * eliminate that portion. This is done by parsing backwards from
- * the end of the extension, removing any numbers. This may be a
- * major or minor number however, so the process is repeated if a
- * minor number was found.
- *
- * ext_end is intended to represent the first character *after* the
- * name portion of an extension, but will be decremented to the last
- * character itself while eliminating the extensions version number.
- * A simple re-increment solves this problem.
- */
- ext_long = true;
- for (; *isa && *isa != '_'; ++isa)
- if (unlikely(!isalnum(*isa)))
- ext_err = true;
-
- ext_end = isa;
- if (unlikely(ext_err))
- break;
-
- if (!isdigit(ext_end[-1]))
- break;
-
- while (isdigit(*--ext_end))
- ;
-
- if (tolower(ext_end[0]) != 'p' || !isdigit(ext_end[-1])) {
- ++ext_end;
- break;
- }
-
- while (isdigit(*--ext_end))
- ;
-
- ++ext_end;
- break;
- default:
- /*
- * Things are a little easier for single-letter extensions, as they
- * are parsed forwards.
- *
- * After checking that our starting position is valid, we need to
- * ensure that, when isa was incremented at the start of the loop,
- * that it arrived at the start of the next extension.
- *
- * If we are already on a non-digit, there is nothing to do. Either
- * we have a multi-letter extension's _, or the start of an
- * extension.
- *
- * Otherwise we have found the current extension's major version
- * number. Parse past it, and a subsequent p/minor version number
- * if present. The `p` extension must not appear immediately after
- * a number, so there is no fear of missing it.
- *
- */
- if (unlikely(!isalpha(*ext))) {
- ext_err = true;
- break;
- }
-
- if (!isdigit(*isa))
- break;
-
- while (isdigit(*++isa))
- ;
-
- if (tolower(*isa) != 'p')
- break;
-
- if (!isdigit(*++isa)) {
- --isa;
- break;
- }
-
- while (isdigit(*++isa))
- ;
-
- break;
- }
-
- /*
- * The parser expects that at the start of an iteration isa points to the
- * first character of the next extension. As we stop parsing an extension
- * on meeting a non-alphanumeric character, an extra increment is needed
- * where the succeeding extension is a multi-letter prefixed with an "_".
- */
- if (*isa == '_')
- ++isa;
-
-#define SET_ISA_EXT_MAP(name, bit) \
- do { \
- if ((ext_end - ext == strlen(name)) && \
- !strncasecmp(ext, name, strlen(name)) && \
- riscv_isa_extension_check(bit)) \
- set_bit(bit, isainfo->isa); \
- } while (false) \
-
- if (unlikely(ext_err))
- continue;
- if (!ext_long) {
- int nr = tolower(*ext) - 'a';
-
- if (riscv_isa_extension_check(nr)) {
- 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);
- }
-#undef SET_ISA_EXT_MAP
- }
+ riscv_parse_isa_string(&this_hwcap, isainfo, isa2hwcap, isa);

/*
* Linux requires the following extensions, so we may as well
@@ -420,6 +412,23 @@ void __init riscv_fill_hwcap(void)

if (!acpi_disabled && rhct)
acpi_put_table((struct acpi_table_header *)rhct);
+}
+
+void __init riscv_fill_hwcap(void)
+{
+ char print_str[NUM_ALPHA_EXTS + 1];
+ int i, j;
+ unsigned long isa2hwcap[26] = {0};
+
+ isa2hwcap['i' - 'a'] = COMPAT_HWCAP_ISA_I;
+ isa2hwcap['m' - 'a'] = COMPAT_HWCAP_ISA_M;
+ isa2hwcap['a' - 'a'] = COMPAT_HWCAP_ISA_A;
+ isa2hwcap['f' - 'a'] = COMPAT_HWCAP_ISA_F;
+ isa2hwcap['d' - 'a'] = COMPAT_HWCAP_ISA_D;
+ isa2hwcap['c' - 'a'] = COMPAT_HWCAP_ISA_C;
+ isa2hwcap['v' - 'a'] = COMPAT_HWCAP_ISA_V;
+
+ riscv_fill_hwcap_from_isa_string(isa2hwcap);

/* We don't support systems with F but without D, so mask those out
* here. */
--
2.40.1


2023-07-13 12:34:19

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v5 05/11] RISC-V: repurpose riscv_isa_ext array in riscv_fill_hwcap()

In riscv_fill_hwcap() riscv_isa_ext array can be looped over, rather
than duplicating the list of extensions with individual
SET_ISA_EXT_MAP() usage. While at it, drop the statement-of-the-obvious
comments from the struct, rename uprop to something more suitable for
its new use & constify the members.

Reviewed-by: Andrew Jones <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
Changes in v5:
- Swap sizeof() - 1 for strlen() as pointed out by Evan

Changes in v2:
- Delete the now unused definition
---
arch/riscv/include/asm/hwcap.h | 7 ++-----
arch/riscv/kernel/cpu.c | 5 +++--
arch/riscv/kernel/cpufeature.c | 30 +++++++++---------------------
3 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 7a57e6109aef..2460ac2fc7ed 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -55,7 +55,6 @@
#define RISCV_ISA_EXT_ZIHPM 42

#define RISCV_ISA_EXT_MAX 64
-#define RISCV_ISA_EXT_NAME_LEN_MAX 32

#ifdef CONFIG_RISCV_M_MODE
#define RISCV_ISA_EXT_SxAIA RISCV_ISA_EXT_SMAIA
@@ -70,10 +69,8 @@
unsigned long riscv_get_elf_hwcap(void);

struct riscv_isa_ext_data {
- /* Name of the extension displayed to userspace via /proc/cpuinfo */
- char uprop[RISCV_ISA_EXT_NAME_LEN_MAX];
- /* The logical ISA extension ID */
- unsigned int isa_ext_id;
+ const unsigned int id;
+ const char *name;
};

extern const struct riscv_isa_ext_data riscv_isa_ext[];
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index bf93293d51f3..aa17eeb0ec9a 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -168,9 +168,10 @@ static void print_isa_ext(struct seq_file *f)
{
for (int i = 0; i < riscv_isa_ext_count; i++) {
const struct riscv_isa_ext_data *edata = &riscv_isa_ext[i];
- if (!__riscv_isa_extension_available(NULL, edata->isa_ext_id))
+ if (!__riscv_isa_extension_available(NULL, edata->id))
continue;
- seq_printf(f, "_%s", edata->uprop);
+
+ seq_printf(f, "_%s", edata->name);
}
}

diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index fb476153fffc..859e3831fac1 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -99,11 +99,10 @@ static bool riscv_isa_extension_check(int id)
return true;
}

-#define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
- { \
- .uprop = #UPROP, \
- .isa_ext_id = EXTID, \
- }
+#define __RISCV_ISA_EXT_DATA(_name, _id) { \
+ .name = #_name, \
+ .id = _id, \
+}

/*
* The canonical order of ISA extension names in the ISA string is defined in
@@ -350,8 +349,8 @@ void __init riscv_fill_hwcap(void)

#define SET_ISA_EXT_MAP(name, bit) \
do { \
- if ((ext_end - ext == sizeof(name) - 1) && \
- !strncasecmp(ext, name, sizeof(name) - 1) && \
+ if ((ext_end - ext == strlen(name)) && \
+ !strncasecmp(ext, name, strlen(name)) && \
riscv_isa_extension_check(bit)) \
set_bit(bit, isainfo->isa); \
} while (false) \
@@ -366,20 +365,9 @@ void __init riscv_fill_hwcap(void)
set_bit(nr, isainfo->isa);
}
} else {
- /* sorted alphabetically */
- SET_ISA_EXT_MAP("smaia", RISCV_ISA_EXT_SMAIA);
- SET_ISA_EXT_MAP("ssaia", RISCV_ISA_EXT_SSAIA);
- SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
- SET_ISA_EXT_MAP("sstc", RISCV_ISA_EXT_SSTC);
- SET_ISA_EXT_MAP("svinval", RISCV_ISA_EXT_SVINVAL);
- SET_ISA_EXT_MAP("svnapot", RISCV_ISA_EXT_SVNAPOT);
- SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
- SET_ISA_EXT_MAP("zba", RISCV_ISA_EXT_ZBA);
- SET_ISA_EXT_MAP("zbb", RISCV_ISA_EXT_ZBB);
- SET_ISA_EXT_MAP("zbs", RISCV_ISA_EXT_ZBS);
- SET_ISA_EXT_MAP("zicbom", RISCV_ISA_EXT_ZICBOM);
- SET_ISA_EXT_MAP("zicboz", RISCV_ISA_EXT_ZICBOZ);
- SET_ISA_EXT_MAP("zihintpause", RISCV_ISA_EXT_ZIHINTPAUSE);
+ for (int i = 0; i < riscv_isa_ext_count; i++)
+ SET_ISA_EXT_MAP(riscv_isa_ext[i].name,
+ riscv_isa_ext[i].id);
}
#undef SET_ISA_EXT_MAP
}
--
2.40.1


2023-07-13 12:36:33

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v5 04/11] RISC-V: shunt isa_ext_arr to cpufeature.c

To facilitate using one struct to define extensions, rather than having
several, shunt isa_ext_arr to cpufeature.c, where it will be used for
probing extension presence also.
As that scope of the array as widened, prefix it with riscv & drop the
type from the variable name.

Since the new array is const, print_isa() needs a wee bit of cleanup to
avoid complaints about losing the const qualifier.

Reviewed-by: Andrew Jones <[email protected]>
Reviewed-by: Evan Green <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
Changes in v2:
- Drop the empty element from the end of the array, it was adding a bug
anyway as I was not decrementing the result of ARRAY_SIZE() by one.
Likely I meant to drop it originally and forgot, as dropping the
decrement was intentional.
---
arch/riscv/include/asm/hwcap.h | 3 ++
arch/riscv/kernel/cpu.c | 75 +---------------------------------
arch/riscv/kernel/cpufeature.c | 67 ++++++++++++++++++++++++++++++
3 files changed, 72 insertions(+), 73 deletions(-)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index f041bfa7f6a0..7a57e6109aef 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -76,6 +76,9 @@ struct riscv_isa_ext_data {
unsigned int isa_ext_id;
};

+extern const struct riscv_isa_ext_data riscv_isa_ext[];
+extern const size_t riscv_isa_ext_count;
+
unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);

#define riscv_isa_extension_mask(ext) BIT_MASK(RISCV_ISA_EXT_##ext)
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index e721f15fdf17..bf93293d51f3 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -164,81 +164,10 @@ arch_initcall(riscv_cpuinfo_init);

#ifdef CONFIG_PROC_FS

-#define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
- { \
- .uprop = #UPROP, \
- .isa_ext_id = EXTID, \
- }
-
-/*
- * The canonical order of ISA extension names in the ISA string is defined in
- * chapter 27 of the unprivileged specification.
- *
- * Ordinarily, for in-kernel data structures, this order is unimportant but
- * isa_ext_arr defines the order of the ISA string in /proc/cpuinfo.
- *
- * The specification uses vague wording, such as should, when it comes to
- * ordering, so for our purposes the following rules apply:
- *
- * 1. All multi-letter extensions must be separated from other extensions by an
- * underscore.
- *
- * 2. Additional standard extensions (starting with 'Z') must be sorted after
- * single-letter extensions and before any higher-privileged extensions.
-
- * 3. The first letter following the 'Z' conventionally indicates the most
- * closely related alphabetical extension category, IMAFDQLCBKJTPVH.
- * If multiple 'Z' extensions are named, they must be ordered first by
- * category, then alphabetically within a category.
- *
- * 3. Standard supervisor-level extensions (starting with 'S') must be listed
- * after standard unprivileged extensions. If multiple supervisor-level
- * extensions are listed, they must be ordered alphabetically.
- *
- * 4. Standard machine-level extensions (starting with 'Zxm') must be listed
- * after any lower-privileged, standard extensions. If multiple
- * machine-level extensions are listed, they must be ordered
- * alphabetically.
- *
- * 5. Non-standard extensions (starting with 'X') must be listed after all
- * standard extensions. If multiple non-standard extensions are listed, they
- * must be ordered alphabetically.
- *
- * An example string following the order is:
- * rv64imadc_zifoo_zigoo_zafoo_sbar_scar_zxmbaz_xqux_xrux
- *
- * New entries to this struct should follow the ordering rules described above.
- */
-static struct riscv_isa_ext_data isa_ext_arr[] = {
- __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
- __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
- __RISCV_ISA_EXT_DATA(zicntr, RISCV_ISA_EXT_ZICNTR),
- __RISCV_ISA_EXT_DATA(zicsr, RISCV_ISA_EXT_ZICSR),
- __RISCV_ISA_EXT_DATA(zifencei, RISCV_ISA_EXT_ZIFENCEI),
- __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
- __RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
- __RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
- __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
- __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
- __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
- __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
- __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
- __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
- __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
- __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
- __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
- __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX),
-};
-
static void print_isa_ext(struct seq_file *f)
{
- struct riscv_isa_ext_data *edata;
- int i = 0, arr_sz;
-
- arr_sz = ARRAY_SIZE(isa_ext_arr) - 1;
-
- for (i = 0; i <= arr_sz; i++) {
- edata = &isa_ext_arr[i];
+ for (int i = 0; i < riscv_isa_ext_count; i++) {
+ const struct riscv_isa_ext_data *edata = &riscv_isa_ext[i];
if (!__riscv_isa_extension_available(NULL, edata->isa_ext_id))
continue;
seq_printf(f, "_%s", edata->uprop);
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index bdcf460ea53d..fb476153fffc 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -99,6 +99,73 @@ static bool riscv_isa_extension_check(int id)
return true;
}

+#define __RISCV_ISA_EXT_DATA(UPROP, EXTID) \
+ { \
+ .uprop = #UPROP, \
+ .isa_ext_id = EXTID, \
+ }
+
+/*
+ * The canonical order of ISA extension names in the ISA string is defined in
+ * chapter 27 of the unprivileged specification.
+ *
+ * Ordinarily, for in-kernel data structures, this order is unimportant but
+ * isa_ext_arr defines the order of the ISA string in /proc/cpuinfo.
+ *
+ * The specification uses vague wording, such as should, when it comes to
+ * ordering, so for our purposes the following rules apply:
+ *
+ * 1. All multi-letter extensions must be separated from other extensions by an
+ * underscore.
+ *
+ * 2. Additional standard extensions (starting with 'Z') must be sorted after
+ * single-letter extensions and before any higher-privileged extensions.
+ *
+ * 3. The first letter following the 'Z' conventionally indicates the most
+ * closely related alphabetical extension category, IMAFDQLCBKJTPVH.
+ * If multiple 'Z' extensions are named, they must be ordered first by
+ * category, then alphabetically within a category.
+ *
+ * 3. Standard supervisor-level extensions (starting with 'S') must be listed
+ * after standard unprivileged extensions. If multiple supervisor-level
+ * extensions are listed, they must be ordered alphabetically.
+ *
+ * 4. Standard machine-level extensions (starting with 'Zxm') must be listed
+ * after any lower-privileged, standard extensions. If multiple
+ * machine-level extensions are listed, they must be ordered
+ * alphabetically.
+ *
+ * 5. Non-standard extensions (starting with 'X') must be listed after all
+ * standard extensions. If multiple non-standard extensions are listed, they
+ * must be ordered alphabetically.
+ *
+ * An example string following the order is:
+ * rv64imadc_zifoo_zigoo_zafoo_sbar_scar_zxmbaz_xqux_xrux
+ *
+ * New entries to this struct should follow the ordering rules described above.
+ */
+const struct riscv_isa_ext_data riscv_isa_ext[] = {
+ __RISCV_ISA_EXT_DATA(zicbom, RISCV_ISA_EXT_ZICBOM),
+ __RISCV_ISA_EXT_DATA(zicboz, RISCV_ISA_EXT_ZICBOZ),
+ __RISCV_ISA_EXT_DATA(zicntr, RISCV_ISA_EXT_ZICNTR),
+ __RISCV_ISA_EXT_DATA(zicsr, RISCV_ISA_EXT_ZICSR),
+ __RISCV_ISA_EXT_DATA(zifencei, RISCV_ISA_EXT_ZIFENCEI),
+ __RISCV_ISA_EXT_DATA(zihintpause, RISCV_ISA_EXT_ZIHINTPAUSE),
+ __RISCV_ISA_EXT_DATA(zihpm, RISCV_ISA_EXT_ZIHPM),
+ __RISCV_ISA_EXT_DATA(zba, RISCV_ISA_EXT_ZBA),
+ __RISCV_ISA_EXT_DATA(zbb, RISCV_ISA_EXT_ZBB),
+ __RISCV_ISA_EXT_DATA(zbs, RISCV_ISA_EXT_ZBS),
+ __RISCV_ISA_EXT_DATA(smaia, RISCV_ISA_EXT_SMAIA),
+ __RISCV_ISA_EXT_DATA(ssaia, RISCV_ISA_EXT_SSAIA),
+ __RISCV_ISA_EXT_DATA(sscofpmf, RISCV_ISA_EXT_SSCOFPMF),
+ __RISCV_ISA_EXT_DATA(sstc, RISCV_ISA_EXT_SSTC),
+ __RISCV_ISA_EXT_DATA(svinval, RISCV_ISA_EXT_SVINVAL),
+ __RISCV_ISA_EXT_DATA(svnapot, RISCV_ISA_EXT_SVNAPOT),
+ __RISCV_ISA_EXT_DATA(svpbmt, RISCV_ISA_EXT_SVPBMT),
+};
+
+const size_t riscv_isa_ext_count = ARRAY_SIZE(riscv_isa_ext);
+
void __init riscv_fill_hwcap(void)
{
struct device_node *node;
--
2.40.1


2023-07-13 12:38:48

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v5 03/11] RISC-V: drop a needless check in print_isa_ext()

isa_ext_arr cannot be empty, as some of the extensions within it are
always built into the kernel. When this code was first added, back in
commit a9b202606c69 ("RISC-V: Improve /proc/cpuinfo output for ISA
extensions"), the array was empty and needed a dummy item & thus there
could be no extensions present. When the first multi-letter ones did
get added, it was Sscofpmf - which didn't have a Kconfig symbol to
disable it.

Remove this check, as it has been redundant since Sscofpmf was added.

Reviewed-by: Andrew Jones <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
Changes in v2:
- Reword commit message to explain why this can be dropped
---
arch/riscv/kernel/cpu.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index f808b67f5a27..e721f15fdf17 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -237,10 +237,6 @@ static void print_isa_ext(struct seq_file *f)

arr_sz = ARRAY_SIZE(isa_ext_arr) - 1;

- /* No extension support available */
- if (arr_sz <= 0)
- return;
-
for (i = 0; i <= arr_sz; i++) {
edata = &isa_ext_arr[i];
if (!__riscv_isa_extension_available(NULL, edata->isa_ext_id))
--
2.40.1


2023-07-13 12:41:58

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v5 06/11] RISC-V: add missing single letter extension definitions

To facilitate adding single letter extensions to riscv_isa_ext, add
definitions for the extensions present in base_riscv_exts that do not
already have them.

Reviewed-by: Andrew Jones <[email protected]>
Reviewed-by: Evan Green <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/include/asm/hwcap.h | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 2460ac2fc7ed..a20e4ade1b53 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -14,12 +14,17 @@
#include <uapi/asm/hwcap.h>

#define RISCV_ISA_EXT_a ('a' - 'a')
+#define RISCV_ISA_EXT_b ('b' - 'a')
#define RISCV_ISA_EXT_c ('c' - 'a')
#define RISCV_ISA_EXT_d ('d' - 'a')
#define RISCV_ISA_EXT_f ('f' - 'a')
#define RISCV_ISA_EXT_h ('h' - 'a')
#define RISCV_ISA_EXT_i ('i' - 'a')
+#define RISCV_ISA_EXT_j ('j' - 'a')
+#define RISCV_ISA_EXT_k ('k' - 'a')
#define RISCV_ISA_EXT_m ('m' - 'a')
+#define RISCV_ISA_EXT_p ('p' - 'a')
+#define RISCV_ISA_EXT_q ('q' - 'a')
#define RISCV_ISA_EXT_s ('s' - 'a')
#define RISCV_ISA_EXT_u ('u' - 'a')
#define RISCV_ISA_EXT_v ('v' - 'a')
--
2.40.1


2023-07-13 12:42:05

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v5 11/11] RISC-V: provide Kconfig & commandline options to control parsing "riscv,isa"

As it says on the tin, provide Kconfig option to control parsing the
"riscv,isa" devicetree property. If either option is used, the kernel
will fall back to parsing "riscv,isa", where "riscv,isa-base" and
"riscv,isa-extensions" are not present.
The Kconfig options are set up so that the default kernel configuration
will enable the fallback path, without needing the commandline option.

Suggested-by: Andrew Jones <[email protected]>
Suggested-by: Palmer Dabbelt <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
Changes in v4:
- add __init to fixup k210 build issue
- use Drew's revised wording

Changes in v3:
- Invert the Kconfig entry. It's now default y & not hidden by
NONPORTABLE, but its entablement will now activate the fallback
- Add a commandline option to enable the fallback on kernels that do not
enable it in Kconfig, as Drew suggested
- Default the global var to the Kconfig option & override it with the
commandline one, rather than have checks for IS_ENABLED() and for the
commandline option in riscv_fill_hwcap() &
riscv_early_of_processor_hartid()
---
.../admin-guide/kernel-parameters.txt | 7 +++++++
arch/riscv/Kconfig | 18 ++++++++++++++++++
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/kernel/cpu.c | 8 +++++++-
arch/riscv/kernel/cpufeature.c | 14 +++++++++++++-
5 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index a1457995fd41..bdc3fa712e92 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -5468,6 +5468,13 @@
[KNL] Disable ring 3 MONITOR/MWAIT feature on supported
CPUs.

+ riscv_isa_fallback [RISCV]
+ When CONFIG_RISCV_ISA_FALLBACK is not enabled, permit
+ falling back to detecting extension support by parsing
+ "riscv,isa" property on devicetree systems when the
+ replacement properties are not found. See the Kconfig
+ entry for RISCV_ISA_FALLBACK.
+
ro [KNL] Mount root device read-only on boot

rodata= [KNL]
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 4c07b9189c86..f52dd125ac5e 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -848,6 +848,24 @@ config XIP_PHYS_ADDR
be linked for and stored to. This address is dependent on your
own flash usage.

+config RISCV_ISA_FALLBACK
+ bool "Permit falling back to parsing riscv,isa for extension support by default"
+ default y
+ help
+ Parsing the "riscv,isa" devicetree property has been deprecated and
+ replaced by a list of explicitly defined strings. For compatibility
+ with existing platforms, the kernel will fall back to parsing the
+ "riscv,isa" property if the replacements are not found.
+
+ Selecting N here will result in a kernel that does not use the
+ fallback, unless the commandline "riscv_isa_fallback" parameter is
+ present.
+
+ Please see the dt-binding, located at
+ Documentation/devicetree/bindings/riscv/extensions.yaml for details
+ on the replacement properties, "riscv,isa-base" and
+ "riscv,isa-extensions".
+
endmenu # "Boot options"

config BUILTIN_DTB
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index e3cda14a486b..b7b58258f6c7 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -81,6 +81,7 @@ struct riscv_isa_ext_data {

extern const struct riscv_isa_ext_data riscv_isa_ext[];
extern const size_t riscv_isa_ext_count;
+extern bool riscv_isa_fallback;

unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);

diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 28d5af21f544..208f1a700121 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -41,7 +41,7 @@ int riscv_of_processor_hartid(struct device_node *node, unsigned long *hart)
return 0;
}

-int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *hart)
+int __init riscv_early_of_processor_hartid(struct device_node *node, unsigned long *hart)
{
const char *isa;

@@ -87,6 +87,12 @@ int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har
return 0;

old_interface:
+ if (!riscv_isa_fallback) {
+ pr_warn("CPU with hartid=%lu is invalid: this kernel does not parse \"riscv,isa\"",
+ *hart);
+ return -ENODEV;
+ }
+
if (of_property_read_string(node, "riscv,isa", &isa)) {
pr_warn("CPU with hartid=%lu has no \"riscv,isa-base\" or \"riscv,isa\" property\n",
*hart);
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index fdc71e52dc2b..71fb840ee246 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -473,6 +473,18 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
return 0;
}

+#ifdef CONFIG_RISCV_ISA_FALLBACK
+bool __initdata riscv_isa_fallback = true;
+#else
+bool __initdata riscv_isa_fallback;
+static int __init riscv_isa_fallback_setup(char *__unused)
+{
+ riscv_isa_fallback = true;
+ return 1;
+}
+early_param("riscv_isa_fallback", riscv_isa_fallback_setup);
+#endif
+
void __init riscv_fill_hwcap(void)
{
char print_str[NUM_ALPHA_EXTS + 1];
@@ -492,7 +504,7 @@ void __init riscv_fill_hwcap(void)
} else {
int ret = riscv_fill_hwcap_from_ext_list(isa2hwcap);

- if (ret) {
+ if (ret && riscv_isa_fallback) {
pr_info("Falling back to deprecated \"riscv,isa\"\n");
riscv_fill_hwcap_from_isa_string(isa2hwcap);
}
--
2.40.1


2023-07-13 12:47:31

by Conor Dooley

[permalink] [raw]
Subject: [PATCH v5 09/11] RISC-V: enable extension detection from dedicated properties

Add support for parsing the new riscv,isa-extensions property in
riscv_fill_hwcap(), by means of a new "property" member of the
riscv_isa_ext_data struct. For now, this shadows the name of the
extension for all users, however this may not be the case for all
extensions, based on how the dt-binding is written.
For the sake of backwards compatibility, fall back to the old scheme
if the new properties are not detected. For now, just inform, rather
than warn, when that happens.

Reviewed-by: Andrew Jones <[email protected]>
Signed-off-by: Conor Dooley <[email protected]>
---
Changes in v5:
- Add a missing put of the cpu node where the property is not found
- Reword the commit message, s/new/dedicated/
- Remove a this_isa bitmap & replace it with isainfo->isa, to match what
is done for the isa string codepath

Changes in v2:
- Pick a more suitable function name than fill_hwcap_new()
- Actually use the property member to read from the DT
---
arch/riscv/include/asm/hwcap.h | 1 +
arch/riscv/kernel/cpufeature.c | 78 ++++++++++++++++++++++++++++++++--
2 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index a20e4ade1b53..e3cda14a486b 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -76,6 +76,7 @@ unsigned long riscv_get_elf_hwcap(void);
struct riscv_isa_ext_data {
const unsigned int id;
const char *name;
+ const char *property;
};

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 7c661b12ac8d..fdc71e52dc2b 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -101,6 +101,7 @@ static bool riscv_isa_extension_check(int id)

#define __RISCV_ISA_EXT_DATA(_name, _id) { \
.name = #_name, \
+ .property = #_name, \
.id = _id, \
}

@@ -414,11 +415,69 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
acpi_put_table((struct acpi_table_header *)rhct);
}

+static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
+{
+ unsigned int cpu;
+
+ for_each_possible_cpu(cpu) {
+ unsigned long this_hwcap = 0;
+ struct device_node *cpu_node;
+ struct riscv_isainfo *isainfo = &hart_isa[cpu];
+
+ cpu_node = of_cpu_device_node_get(cpu);
+ if (!cpu_node) {
+ pr_warn("Unable to find cpu node\n");
+ continue;
+ }
+
+ if (!of_property_present(cpu_node, "riscv,isa-extensions")) {
+ of_node_put(cpu_node);
+ 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].property) < 0)
+ continue;
+
+ if (!riscv_isa_extension_check(riscv_isa_ext[i].id))
+ continue;
+
+ /* Only single letter extensions get set in hwcap */
+ if (strnlen(riscv_isa_ext[i].name, 2) == 1)
+ this_hwcap |= isa2hwcap[riscv_isa_ext[i].id];
+
+ set_bit(riscv_isa_ext[i].id, isainfo->isa);
+ }
+
+ of_node_put(cpu_node);
+
+ /*
+ * All "okay" harts should have same isa. Set HWCAP based on
+ * common capabilities of every "okay" hart, in case they don't.
+ */
+ if (elf_hwcap)
+ elf_hwcap &= this_hwcap;
+ else
+ elf_hwcap = this_hwcap;
+
+ if (bitmap_empty(riscv_isa, RISCV_ISA_EXT_MAX))
+ bitmap_copy(riscv_isa, isainfo->isa, RISCV_ISA_EXT_MAX);
+ else
+ bitmap_and(riscv_isa, riscv_isa, isainfo->isa, RISCV_ISA_EXT_MAX);
+ }
+
+ if (bitmap_empty(riscv_isa, RISCV_ISA_EXT_MAX))
+ return -ENOENT;
+
+ return 0;
+}
+
void __init riscv_fill_hwcap(void)
{
char print_str[NUM_ALPHA_EXTS + 1];
- int i, j;
unsigned long isa2hwcap[26] = {0};
+ int i, j;

isa2hwcap['i' - 'a'] = COMPAT_HWCAP_ISA_I;
isa2hwcap['m' - 'a'] = COMPAT_HWCAP_ISA_M;
@@ -428,10 +487,21 @@ void __init riscv_fill_hwcap(void)
isa2hwcap['c' - 'a'] = COMPAT_HWCAP_ISA_C;
isa2hwcap['v' - 'a'] = COMPAT_HWCAP_ISA_V;

- riscv_fill_hwcap_from_isa_string(isa2hwcap);
+ if (!acpi_disabled) {
+ riscv_fill_hwcap_from_isa_string(isa2hwcap);
+ } else {
+ int ret = riscv_fill_hwcap_from_ext_list(isa2hwcap);

- /* We don't support systems with F but without D, so mask those out
- * here. */
+ if (ret) {
+ pr_info("Falling back to deprecated \"riscv,isa\"\n");
+ riscv_fill_hwcap_from_isa_string(isa2hwcap);
+ }
+ }
+
+ /*
+ * We don't support systems with F but without D, so mask those out
+ * here.
+ */
if ((elf_hwcap & COMPAT_HWCAP_ISA_F) && !(elf_hwcap & COMPAT_HWCAP_ISA_D)) {
pr_info("This kernel does not support systems with F but not D\n");
elf_hwcap &= ~COMPAT_HWCAP_ISA_F;
--
2.40.1


Subject: Re: [PATCH v5 00/11] RISC-V: Probe DT extension support using riscv,isa-extensions & riscv,isa-base

Hello:

This series was applied to riscv/linux.git (for-next)
by Palmer Dabbelt <[email protected]>:

On Thu, 13 Jul 2023 13:10:58 +0100 you wrote:
> Hey,
>
> Based on my latest iteration of deprecating riscv,isa [1], here's an
> implementation of the new properties for Linux. The first few patches,
> up to "RISC-V: split riscv_fill_hwcap() in 3", are all prep work that
> further tames some of the extension related code, on top of my already
> applied series that cleans up the ISA string parser.
> Perhaps "RISC-V: shunt isa_ext_arr to cpufeature.c" is a bit gratuitous,
> but I figured a bit of coalescing of extension related data structures
> would be a good idea. Note that riscv,isa will still be used in the
> absence of the new properties. Palmer suggested adding a Kconfig option
> to turn off the fallback for DT, which I have gone and done. It's locked
> behind the NONPORTABLE option for good reason.
>
> [...]

Here is the summary with links:
- [v5,01/11] RISC-V: Provide a more helpful error message on invalid ISA strings
https://git.kernel.org/riscv/c/230598939678
- [v5,02/11] RISC-V: don't parse dt/acpi isa string to get rv32/rv64
https://git.kernel.org/riscv/c/67270fb388fe
- [v5,03/11] RISC-V: drop a needless check in print_isa_ext()
https://git.kernel.org/riscv/c/131033689da2
- [v5,04/11] RISC-V: shunt isa_ext_arr to cpufeature.c
https://git.kernel.org/riscv/c/8135ade32c0d
- [v5,05/11] RISC-V: repurpose riscv_isa_ext array in riscv_fill_hwcap()
https://git.kernel.org/riscv/c/37f988dcec05
- [v5,06/11] RISC-V: add missing single letter extension definitions
https://git.kernel.org/riscv/c/c30556e318cc
- [v5,07/11] RISC-V: add single letter extensions to riscv_isa_ext
https://git.kernel.org/riscv/c/effc122ad176
- [v5,08/11] RISC-V: split riscv_fill_hwcap() in 3
https://git.kernel.org/riscv/c/4265b0ec5ee7
- [v5,09/11] RISC-V: enable extension detection from dedicated properties
https://git.kernel.org/riscv/c/90700a4fbfaf
- [v5,10/11] RISC-V: try new extension properties in of_early_processor_hartid()
https://git.kernel.org/riscv/c/c98f136aedbd
- [v5,11/11] RISC-V: provide Kconfig & commandline options to control parsing "riscv,isa"
https://git.kernel.org/riscv/c/496ea826d1e1

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html