According to RISC-V Hart Capabilities Table (RHCT) description in UEFI
Forum ECR, the format of the ISA string is defined in the RISC-V
unprivileged specification which is case-insensitive. However, the
current ISA string parser in the kernel does not support ISA strings
with uppercase letters.
This patch modifies the ISA string parser in the kernel to support
case-insensitive ISA string parsing.
Signed-off-by: Yangyu Chen <[email protected]>
---
arch/riscv/kernel/cpu.c | 3 ++-
arch/riscv/kernel/cpufeature.c | 25 ++++++++++++-------------
2 files changed, 14 insertions(+), 14 deletions(-)
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 8400f0cc9704..52b92a267121 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -4,6 +4,7 @@
*/
#include <linux/cpu.h>
+#include <linux/ctype.h>
#include <linux/init.h>
#include <linux/seq_file.h>
#include <linux/of.h>
@@ -41,7 +42,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;
}
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 59d58ee0f68d..d1991c12e546 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -119,13 +119,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;
@@ -136,6 +133,7 @@ void __init riscv_fill_hwcap(void)
bool ext_long = false, ext_err = false;
switch (*ext) {
+ case 'S':
case 's':
/**
* Workaround for invalid single-letter 's' & 'u'(QEMU).
@@ -143,19 +141,20 @@ void __init riscv_fill_hwcap(void)
* not valid ISA extensions. It works until multi-letter
* extension starting with "Su" appears.
*/
- if (ext[-1] != '_' && ext[1] == 'u') {
+ if (ext[-1] != '_' && tolower(ext[1]) == 'u') {
++isa;
ext_err = true;
break;
}
fallthrough;
+ 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;
@@ -166,7 +165,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;
@@ -178,7 +177,7 @@ void __init riscv_fill_hwcap(void)
++ext_end;
break;
default:
- if (unlikely(!islower(*ext))) {
+ if (unlikely(!isalpha(*ext))) {
ext_err = true;
break;
}
@@ -188,7 +187,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;
@@ -205,7 +204,7 @@ void __init riscv_fill_hwcap(void)
#define SET_ISA_EXT_MAP(name, bit) \
do { \
if ((ext_end - ext == sizeof(name) - 1) && \
- !memcmp(ext, name, sizeof(name) - 1) && \
+ !strncasecmp(ext, name, sizeof(name) - 1) && \
riscv_isa_extension_check(bit)) \
set_bit(bit, this_isa); \
} while (false) \
@@ -213,7 +212,7 @@ void __init riscv_fill_hwcap(void)
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];
--
2.40.0
Sorry for the cover letter being detached with patches. My mailing
service provider will override the Message-IDs which caused this
trouble. I will send the cover letter first to get the correct Message-ID
before sending the rest of the patches the next time.
The cover letter is at https://lore.kernel.org/linux-riscv/[email protected]/
The patches is at https://lore.kernel.org/linux-riscv/[email protected]/
On Sat, Apr 29, 2023 at 12:26:00AM +0800, Yangyu Chen wrote:
> Sorry for the cover letter being detached with patches. My mailing
> service provider will override the Message-IDs which caused this
> trouble. I will send the cover letter first to get the correct Message-ID
> before sending the rest of the patches the next time.
At least it doesn't overwrite the in-reply-to headers also, so the
patches are at least threaded.
> The cover letter is at https://lore.kernel.org/linux-riscv/[email protected]/
> The patches is at https://lore.kernel.org/linux-riscv/[email protected]/
>
Hey Yangyu,
On Fri, Apr 28, 2023 at 10:16:00PM +0800, Yangyu Chen wrote:
> According to RISC-V Hart Capabilities Table (RHCT) description in UEFI
> Forum ECR, the format of the ISA string is defined in the RISC-V
> unprivileged specification which is case-insensitive. However, the
> current ISA string parser in the kernel does not support ISA strings
> with uppercase letters.
>
> This patch modifies the ISA string parser in the kernel to support
> case-insensitive ISA string parsing.
@Palmer, @Sunil
Just to note, we probably should get this applied *before* we enable ACPI,
right?
>
> Signed-off-by: Yangyu Chen <[email protected]>
> ---
> arch/riscv/kernel/cpu.c | 3 ++-
> arch/riscv/kernel/cpufeature.c | 25 ++++++++++++-------------
> 2 files changed, 14 insertions(+), 14 deletions(-)
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 59d58ee0f68d..d1991c12e546 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -119,13 +119,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;
> @@ -136,6 +133,7 @@ void __init riscv_fill_hwcap(void)
> bool ext_long = false, ext_err = false;
>
> switch (*ext) {
> + case 'S':
Capital S should never be emitted by QEMU, so there's no need to have
this use the workaround, right? IOW, move it between s & X.
Otherwise, this looks good to me:
Reviewed-by: Conor Dooley <[email protected]>
Thanks,
Conor.
On Fri, Apr 28, 2023 at 10:16:00PM +0800, Yangyu Chen wrote:
> According to RISC-V Hart Capabilities Table (RHCT) description in UEFI
> Forum ECR, the format of the ISA string is defined in the RISC-V
> unprivileged specification which is case-insensitive. However, the
> current ISA string parser in the kernel does not support ISA strings
> with uppercase letters.
>
> This patch modifies the ISA string parser in the kernel to support
> case-insensitive ISA string parsing.
>
> Signed-off-by: Yangyu Chen <[email protected]>
> ---
> arch/riscv/kernel/cpu.c | 3 ++-
> arch/riscv/kernel/cpufeature.c | 25 ++++++++++++-------------
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index 8400f0cc9704..52b92a267121 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -4,6 +4,7 @@
> */
>
> #include <linux/cpu.h>
> +#include <linux/ctype.h>
> #include <linux/init.h>
> #include <linux/seq_file.h>
> #include <linux/of.h>
> @@ -41,7 +42,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;
> }
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 59d58ee0f68d..d1991c12e546 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -119,13 +119,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;
> @@ -136,6 +133,7 @@ void __init riscv_fill_hwcap(void)
> bool ext_long = false, ext_err = false;
>
> switch (*ext) {
> + case 'S':
> case 's':
> /**
> * Workaround for invalid single-letter 's' & 'u'(QEMU).
> @@ -143,19 +141,20 @@ void __init riscv_fill_hwcap(void)
> * not valid ISA extensions. It works until multi-letter
> * extension starting with "Su" appears.
> */
> - if (ext[-1] != '_' && ext[1] == 'u') {
> + if (ext[-1] != '_' && tolower(ext[1]) == 'u') {
> ++isa;
> ext_err = true;
> break;
> }
> fallthrough;
> + 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;
> @@ -166,7 +165,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;
> @@ -178,7 +177,7 @@ void __init riscv_fill_hwcap(void)
> ++ext_end;
> break;
> default:
> - if (unlikely(!islower(*ext))) {
> + if (unlikely(!isalpha(*ext))) {
> ext_err = true;
> break;
> }
> @@ -188,7 +187,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;
> @@ -205,7 +204,7 @@ void __init riscv_fill_hwcap(void)
> #define SET_ISA_EXT_MAP(name, bit) \
> do { \
> if ((ext_end - ext == sizeof(name) - 1) && \
> - !memcmp(ext, name, sizeof(name) - 1) && \
> + !strncasecmp(ext, name, sizeof(name) - 1) && \
> riscv_isa_extension_check(bit)) \
> set_bit(bit, this_isa); \
> } while (false) \
> @@ -213,7 +212,7 @@ void __init riscv_fill_hwcap(void)
> 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];
> --
> 2.40.0
>
Reviewed-by: Andrew Jones <[email protected]>
Thanks,
drew
On Fri, Apr 28, 2023 at 10:16:00PM +0800, Yangyu Chen wrote:
> @@ -205,7 +204,7 @@ void __init riscv_fill_hwcap(void)
> #define SET_ISA_EXT_MAP(name, bit) \
> do { \
> if ((ext_end - ext == sizeof(name) - 1) && \
> - !memcmp(ext, name, sizeof(name) - 1) && \
> + !strncasecmp(ext, name, sizeof(name) - 1) && \
> riscv_isa_extension_check(bit)) \
> set_bit(bit, this_isa); \
> } while (false) \
I was doing some poking in this area the other day & noticed that the \s
being misaligned isn't just the diff's fault and they are now misaligned
in the code.
If you are re-submitting, then I think you could align them.
Thanks,
Conor.