[PATCH v4 08/11] RISC-V: split riscv_fill_hwcap() in 3

From: Conor Dooley
Date: Mon Jul 10 2023 - 05:40:26 EST


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 <ajones@xxxxxxxxxxxxxxxx>
Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
---
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 bf7e8e8852f0..41aedeaecb61 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 == 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) {
+ 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 == 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) {
- 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