Skip to content

Commit

Permalink
Merge patch series "riscv: allow case-insensitive ISA string parsing"
Browse files Browse the repository at this point in the history
Yangyu Chen <cyy@cyyself.name> says:

This patchset allows case-insensitive ISA string parsing, which is
needed in the ACPI environment. As the RISC-V Hart Capabilities Table
(RHCT) description in UEFI Forum ECR[1] shows the format of the ISA
string is defined in the RISC-V unprivileged specification[2]. However,
the RISC-V unprivileged specification defines the ISA naming strings are
case-insensitive while the current ISA string parser in the kernel only
accepts lowercase letters. In this case, the kernel should allow
case-insensitive ISA string parsing. Moreover, this reason has been
discussed in Conor's patch[3]. And I have also checked the current ISA
string parsing in the recent ACPI support patch[4] will also call
`riscv_fill_hwcap` function as DT we use now.

The original motivation for my patch v1[5] is that some SoC generators
will provide generated DT with illegal ISA string in dt-binding such as
rocket-chip, which will even cause kernel panic in some cases as I
mentioned in v1[5]. Now, the rocket-chip has been fixed in PR #3333[6].
However, when using some specific version of rocket-chip with
illegal ISA string in DT, this patchset will also work for parsing
uppercase letters correctly in DT, thus will have better compatibility.

In summary, this patch not only works for case-insensitive ISA string
parsing to meet the requirements in ECR[1] but also can be a workaround
for some specific versions of rocket-chip.

* b4-shazam-merge:
  dt-bindings: riscv: drop invalid comment about riscv,isa lower-case reasoning
  riscv: allow case-insensitive ISA string parsing

Link: https://lore.kernel.org/r/tencent_E6911C8D71F5624E432A1AFDF86804C3B509@qq.com
Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
Palmer Dabbelt committed Jun 6, 2023

Unverified

No user is associated with the committer email.
2 parents 90502d5 + 9e320d7 commit 748462b
Showing 3 changed files with 20 additions and 20 deletions.
2 changes: 1 addition & 1 deletion Documentation/devicetree/bindings/riscv/cpus.yaml
Original file line number Diff line number Diff line change
@@ -94,7 +94,7 @@ properties:

While the isa strings in ISA specification are case
insensitive, letters in the riscv,isa string must be all
lowercase to simplify parsing.
lowercase.
$ref: "/schemas/types.yaml#/definitions/string"
pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$

3 changes: 2 additions & 1 deletion arch/riscv/kernel/cpu.c
Original file line number Diff line number Diff line change
@@ -5,6 +5,7 @@

#include <linux/acpi.h>
#include <linux/cpu.h>
#include <linux/ctype.h>
#include <linux/init.h>
#include <linux/seq_file.h>
#include <linux/of.h>
@@ -44,7 +45,7 @@ int riscv_of_processor_hartid(struct device_node *node, unsigned long *hart)
pr_warn("CPU with hartid=%lu has no \"riscv,isa\" property\n", *hart);
return -ENODEV;
}
if (isa[0] != 'r' || isa[1] != 'v') {
if (tolower(isa[0]) != 'r' || tolower(isa[1]) != 'v') {
pr_warn("CPU with hartid=%lu has an invalid ISA of \"%s\"\n", *hart, isa);
return -ENODEV;
}
35 changes: 17 additions & 18 deletions arch/riscv/kernel/cpufeature.c
Original file line number Diff line number Diff line change
@@ -150,13 +150,10 @@ void __init riscv_fill_hwcap(void)
}

temp = isa;
#if IS_ENABLED(CONFIG_32BIT)
if (!strncmp(isa, "rv32", 4))
if (IS_ENABLED(CONFIG_32BIT) && !strncasecmp(isa, "rv32", 4))
isa += 4;
#elif IS_ENABLED(CONFIG_64BIT)
if (!strncmp(isa, "rv64", 4))
else if (IS_ENABLED(CONFIG_64BIT) && !strncasecmp(isa, "rv64", 4))
isa += 4;
#endif
/* The riscv,isa DT property must start with rv64 or rv32 */
if (temp == isa)
continue;
@@ -180,13 +177,15 @@ void __init riscv_fill_hwcap(void)
break;
}
fallthrough;
case 'S':
case 'x':
case 'X':
case 'z':
case 'Z':
ext_long = true;
/* Multi-letter extension must be delimited */
for (; *isa && *isa != '_'; ++isa)
if (unlikely(!islower(*isa)
&& !isdigit(*isa)))
if (unlikely(!isalnum(*isa)))
ext_err = true;
/* Parse backwards */
ext_end = isa;
@@ -197,7 +196,7 @@ void __init riscv_fill_hwcap(void)
/* Skip the minor version */
while (isdigit(*--ext_end))
;
if (ext_end[0] != 'p'
if (tolower(ext_end[0]) != 'p'
|| !isdigit(ext_end[-1])) {
/* Advance it to offset the pre-decrement */
++ext_end;
@@ -209,7 +208,7 @@ void __init riscv_fill_hwcap(void)
++ext_end;
break;
default:
if (unlikely(!islower(*ext))) {
if (unlikely(!isalpha(*ext))) {
ext_err = true;
break;
}
@@ -219,7 +218,7 @@ void __init riscv_fill_hwcap(void)
/* Skip the minor version */
while (isdigit(*++isa))
;
if (*isa != 'p')
if (tolower(*isa) != 'p')
break;
if (!isdigit(*++isa)) {
--isa;
@@ -233,18 +232,18 @@ void __init riscv_fill_hwcap(void)
if (*isa != '_')
--isa;

#define SET_ISA_EXT_MAP(name, bit) \
do { \
if ((ext_end - ext == sizeof(name) - 1) && \
!memcmp(ext, name, sizeof(name) - 1) && \
riscv_isa_extension_check(bit)) \
set_bit(bit, this_isa); \
} while (false) \
#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, this_isa); \
} while (false) \

if (unlikely(ext_err))
continue;
if (!ext_long) {
int nr = *ext - 'a';
int nr = tolower(*ext) - 'a';

if (riscv_isa_extension_check(nr)) {
this_hwcap |= isa2hwcap[nr];

0 comments on commit 748462b

Please sign in to comment.