[PATCH 2/2] RISC-V: Extract base ISA from device tree

[Atish Patra]

On Tue, Feb 15, 2022 at 9:04 PM Tsukasa OI <research_trasio at irq.a4lg.com> wrote:
>
> This commit replaces print_isa function to extract base ISA from device
> tree ("riscv,isa").  It uses a subset of Tsukasa's riscv_fill_hwcap ISA
> parser.
>
> Design choices (1):
>
> It constructs base ISA string from the original string (does not cut
> the original string like Atish's).  This is to strip version numbers
> (too minor to represent as major ISA) but this behavior may be
> debatable.
>

A simpler solution would be to just invoke
__riscv_isa_extension_available for all the base
extensions (imafdc) and print the base isa string.
We don't need to parse the ISA string again here.

> Design choices (2):
>
> It skips when single-letter 'S' or 'U' is encountered.  It improves
> familiarity with regular ISA string.
>

I don't think this is necessary because only Qemu appends 'S' & 'U' in
the isa string.
Your patch in Qemu will fix that for future releases of Qemu.
All other commonly used platforms (hardware/spike) already use the
correct ISA string.

> This commit is intended to be squashed into Atish's isa_framework_v4
> PATCH 6/6.
>
> Signed-off-by: Tsukasa OI <research_trasio at irq.a4lg.com>
> ---
>  arch/riscv/kernel/cpu.c | 83 ++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 74 insertions(+), 9 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index 6f9660c0a973..3f9607a66d95 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -101,17 +101,82 @@ static void print_isa_ext(struct seq_file *f)
>
>  static void print_isa(struct seq_file *f, const char *isa)
>  {
> -       char *ext_start;
> -       int isa_len = strlen(isa);
> -       int base_isa_len = isa_len;
> -
> -       ext_start = strnchr(isa, isa_len, '_');
> -       if (ext_start)
> -               base_isa_len = isa_len - strlen(ext_start);
> +       unsigned char parse_break = 0;
> +       char base_isa[RISCV_ISA_EXT_BASE + 5];
> +       char *base_isa_end = base_isa;
> +       const char *isa_end = isa;
> +#if IS_ENABLED(CONFIG_32BIT)
> +       if (!strncmp(isa, "rv32", 4))
> +               isa_end += 4;
> +#elif IS_ENABLED(CONFIG_64BIT)
> +       if (!strncmp(isa, "rv64", 4))
> +               isa_end += 4;
> +#endif
> +       if (isa != isa_end) {
> +               strncpy(base_isa, isa, isa_end - isa);
> +               base_isa_end += isa_end - isa;
> +               for (; *isa_end; ++isa_end) {
> +                       switch (*isa_end++) {
> +                       case 'x':
> +                       case 'z':
> +                               parse_break = 1;
> +                               break;
> +                       case 's':
> +                               if (*isa_end != 'u') {
> +                                       parse_break = 1;
> +                                       break;
> +                               }
> +                               /**
> +                                * Workaround for invalid single-letter 's'
> +                                * (QEMU).  Should break for valid ISA string.
> +                                */
> +                               fallthrough;
> +                       default:
> +                               if (unlikely(!islower(isa_end[-1])
> +                                   || base_isa_end == base_isa + sizeof(base_isa))) {
> +                                       parse_break = 2;
> +                                       break;
> +                               }
> +                               switch (isa_end[-1]) {
> +                               case 's':
> +                               case 'u':
> +                                       break;
> +                               default:
> +                                       *base_isa_end++ = isa_end[-1];
> +                               }
> +                               /* Skip version number */
> +                               if (!isdigit(*isa_end))
> +                                       break;
> +                               while (isdigit(*++isa_end))
> +                                       ;
> +                               if (*isa_end != 'p')
> +                                       break;
> +                               if (!isdigit(*++isa_end)) {
> +                                       --isa_end;
> +                                       break;
> +                               }
> +                               while (isdigit(*++isa_end))
> +                                       ;
> +                               break;
> +                       }
> +                       if (*isa_end != '_')
> +                               --isa_end;
> +                       if (parse_break)
> +                               break;
> +               }
> +       }
>
> -       /* Print only the base ISA as it is */
> +       /**
> +        * Print only the base ISA
> +        * (original ISA string as is if an error is encountered)
> +        */
>         seq_puts(f, "isa\t\t: ");
> -       seq_write(f, isa, base_isa_len);
> +       if (parse_break < 2) {
> +               seq_write(f, base_isa, base_isa_end - base_isa);
> +       } else {
> +               isa_end += strlen(isa_end);
> +               seq_write(f, isa, isa_end - isa);
> +       }
>         seq_puts(f, "\n");
>  }
>
> --
> 2.32.0
>


-- 
Regards,
Atish