On Wed, Feb 23, 2022 at 2:18 AM Atish Patra <atishp@xxxxxxxxxxxx> wrote:
>
> From: Tsukasa OI <research_trasio@xxxxxxxxxxxx>
>
> Current hart ISA ("riscv,isa") parser don't correctly parse:
>
> 1. Multi-letter extensions
> 2. Version numbers
>
> All ISA extensions ratified recently has multi-letter extensions
> (except 'H'). The current "riscv,isa" parser that is easily confused
> by multi-letter extensions and "p" in version numbers can be a huge
> problem for adding new extensions through the device tree.
>
> Leaving it would create incompatible hacks and would make "riscv,isa"
> value unreliable.
>
> This commit implements minimal parser for "riscv,isa" strings. With this,
> we can safely ignore multi-letter extensions and version numbers.
>
> [Improved commit text and fixed a bug around 's' in base extension]
> Signed-off-by: Atish Patra <atishp@xxxxxxxxxxxx>
> [Fixed workaround for QEMU]
> Signed-off-by: Tsukasa OI <research_trasio@xxxxxxxxxxxx>
> Tested-by: Heiko Stuebner <heiko@xxxxxxxxx>
Looks good to me.
Reviewed-by: Anup Patel <anup@xxxxxxxxxxxxxx>
Regards,
Anup
> ---
> arch/riscv/kernel/cpufeature.c | 72 ++++++++++++++++++++++++++++------
> 1 file changed, 61 insertions(+), 11 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index dd3d57eb4eea..72c5f6ef56b5 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -7,6 +7,7 @@
> */
>
> #include <linux/bitmap.h>
> +#include <linux/ctype.h>
> #include <linux/of.h>
> #include <asm/processor.h>
> #include <asm/hwcap.h>
> @@ -66,7 +67,7 @@ void __init riscv_fill_hwcap(void)
> struct device_node *node;
> const char *isa;
> char print_str[NUM_ALPHA_EXTS + 1];
> - size_t i, j, isa_len;
> + int i, j;
> static unsigned long isa2hwcap[256] = {0};
>
> isa2hwcap['i'] = isa2hwcap['I'] = COMPAT_HWCAP_ISA_I;
> @@ -92,23 +93,72 @@ void __init riscv_fill_hwcap(void)
> continue;
> }
>
> - i = 0;
> - isa_len = strlen(isa);
> #if IS_ENABLED(CONFIG_32BIT)
> if (!strncmp(isa, "rv32", 4))
> - i += 4;
> + isa += 4;
> #elif IS_ENABLED(CONFIG_64BIT)
> if (!strncmp(isa, "rv64", 4))
> - i += 4;
> + isa += 4;
> #endif
> - for (; i < isa_len; ++i) {
> - this_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
> + for (; *isa; ++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 'x':
> + case 'z':
> + ext_long = true;
> + /* Multi-letter extension must be delimited */
> + for (; *isa && *isa != '_'; ++isa)
> + if (!islower(*isa) && !isdigit(*isa))
> + ext_err = true;
> + break;
> + default:
> + if (unlikely(!islower(*ext))) {
> + ext_err = true;
> + break;
> + }
> + /* Find next extension */
> + if (!isdigit(*isa))
> + break;
> + /* Skip the minor version */
> + while (isdigit(*++isa))
> + ;
> + if (*isa != 'p')
> + break;
> + if (!isdigit(*++isa)) {
> + --isa;
> + break;
> + }
> + /* Skip the major version */
> + while (isdigit(*++isa))
> + ;
> + break;
> + }
> + if (*isa != '_')
> + --isa;
> /*
> - * TODO: X, Y and Z extension parsing for Host ISA
> - * bitmap will be added in-future.
> + * TODO: Full version-aware handling including
> + * multi-letter extensions will be added in-future.
> */
> - if ('a' <= isa[i] && isa[i] < 'x')
> - this_isa |= (1UL << (isa[i] - 'a'));
> + if (ext_err || ext_long)
> + continue;
> + this_hwcap |= isa2hwcap[(unsigned char)(*ext)];
> + this_isa |= (1UL << (*ext - 'a'));
> }
>
> /*
> --
> 2.30.2
>
