RISC-V: split riscv_fill_hwcap() in 3
authorConor Dooley <conor.dooley@microchip.com>
Thu, 13 Jul 2023 12:11:06 +0000 (13:11 +0100)
committerPalmer Dabbelt <palmer@rivosinc.com>
Tue, 25 Jul 2023 23:26:22 +0000 (16:26 -0700)
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@ventanamicro.com>
Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
Link: https://lore.kernel.org/r/20230713-daylight-puritan-37aeb41a4d9b@wendy
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
arch/riscv/kernel/cpufeature.c

index d6009bd..7c661b1 100644 (file)
@@ -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. */