make the riscv_cpufeature_patch_func() scan all ISA extensions rather
than limited feature macros.
Signed-off-by: Jisheng Zhang <[email protected]>
---
arch/riscv/include/asm/errata_list.h | 9 ++--
arch/riscv/kernel/cpufeature.c | 61 +++-------------------------
2 files changed, 9 insertions(+), 61 deletions(-)
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index 19a771085781..722525f4fc96 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -6,6 +6,7 @@
#define ASM_ERRATA_LIST_H
#include <asm/alternative.h>
+#include <asm/hwcap.h>
#include <asm/vendorid_list.h>
#ifdef CONFIG_ERRATA_SIFIVE
@@ -20,10 +21,6 @@
#define ERRATA_THEAD_NUMBER 2
#endif
-#define CPUFEATURE_SVPBMT 0
-#define CPUFEATURE_ZICBOM 1
-#define CPUFEATURE_NUMBER 2
-
#ifdef __ASSEMBLY__
#define ALT_INSN_FAULT(x) \
@@ -53,7 +50,7 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID, \
#define ALT_SVPBMT(_val, prot) \
asm(ALTERNATIVE_2("li %0, 0\t\nnop", \
"li %0, %1\t\nslli %0,%0,%3", 0, \
- CPUFEATURE_SVPBMT, CONFIG_RISCV_ISA_SVPBMT, \
+ RISCV_ISA_EXT_SVPBMT, CONFIG_RISCV_ISA_SVPBMT, \
"li %0, %2\t\nslli %0,%0,%4", THEAD_VENDOR_ID, \
ERRATA_THEAD_PBMT, CONFIG_ERRATA_THEAD_PBMT) \
: "=r"(_val) \
@@ -127,7 +124,7 @@ asm volatile(ALTERNATIVE_2( \
"add a0, a0, %0\n\t" \
"2:\n\t" \
"bltu a0, %2, 3b\n\t" \
- "nop", 0, CPUFEATURE_ZICBOM, CONFIG_RISCV_ISA_ZICBOM, \
+ "nop", 0, RISCV_ISA_EXT_ZICBOM, CONFIG_RISCV_ISA_ZICBOM, \
"mv a0, %1\n\t" \
"j 2f\n\t" \
"3:\n\t" \
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index afa54635c180..2b1f18f97253 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -251,61 +251,11 @@ void __init riscv_fill_hwcap(void)
}
#ifdef CONFIG_RISCV_ALTERNATIVE
-static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
-{
-#ifdef CONFIG_RISCV_ISA_SVPBMT
- switch (stage) {
- case RISCV_ALTERNATIVES_EARLY_BOOT:
- return false;
- default:
- return riscv_isa_extension_available(NULL, SVPBMT);
- }
-#endif
-
- return false;
-}
-
-static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
-{
-#ifdef CONFIG_RISCV_ISA_ZICBOM
- switch (stage) {
- case RISCV_ALTERNATIVES_EARLY_BOOT:
- return false;
- default:
- return riscv_isa_extension_available(NULL, ZICBOM);
- }
-#endif
-
- return false;
-}
-
-/*
- * Probe presence of individual extensions.
- *
- * This code may also be executed before kernel relocation, so we cannot use
- * addresses generated by the address-of operator as they won't be valid in
- * this context.
- */
-static u32 __init_or_module cpufeature_probe(unsigned int stage)
-{
- u32 cpu_req_feature = 0;
-
- if (cpufeature_probe_svpbmt(stage))
- cpu_req_feature |= (1U << CPUFEATURE_SVPBMT);
-
- if (cpufeature_probe_zicbom(stage))
- cpu_req_feature |= (1U << CPUFEATURE_ZICBOM);
-
- return cpu_req_feature;
-}
-
void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
struct alt_entry *end,
unsigned int stage)
{
- u32 cpu_req_feature = cpufeature_probe(stage);
struct alt_entry *alt;
- u32 tmp;
if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
return;
@@ -313,15 +263,16 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
for (alt = begin; alt < end; alt++) {
if (alt->vendor_id != 0)
continue;
- if (alt->errata_id >= CPUFEATURE_NUMBER) {
- WARN(1, "This feature id:%d is not in kernel cpufeature list",
+ if (alt->errata_id >= RISCV_ISA_EXT_ID_MAX) {
+ WARN(1, "This extension id:%d is not in ISA extension list",
alt->errata_id);
continue;
}
- tmp = (1U << alt->errata_id);
- if (cpu_req_feature & tmp)
- patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
+ if (!test_bit(alt->errata_id, riscv_isa))
+ continue;
+
+ patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
}
}
#endif
--
2.37.2
On Thu, Oct 06, 2022 at 03:08:14PM +0800, Jisheng Zhang wrote:
> make the riscv_cpufeature_patch_func() scan all ISA extensions rather
> than limited feature macros.
>
> Signed-off-by: Jisheng Zhang <[email protected]>
> ---
> arch/riscv/include/asm/errata_list.h | 9 ++--
> arch/riscv/kernel/cpufeature.c | 61 +++-------------------------
> 2 files changed, 9 insertions(+), 61 deletions(-)
>
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> index 19a771085781..722525f4fc96 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -6,6 +6,7 @@
> #define ASM_ERRATA_LIST_H
>
> #include <asm/alternative.h>
> +#include <asm/hwcap.h>
> #include <asm/vendorid_list.h>
>
> #ifdef CONFIG_ERRATA_SIFIVE
> @@ -20,10 +21,6 @@
> #define ERRATA_THEAD_NUMBER 2
> #endif
>
> -#define CPUFEATURE_SVPBMT 0
> -#define CPUFEATURE_ZICBOM 1
> -#define CPUFEATURE_NUMBER 2
> -
> #ifdef __ASSEMBLY__
>
> #define ALT_INSN_FAULT(x) \
> @@ -53,7 +50,7 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID, \
> #define ALT_SVPBMT(_val, prot) \
> asm(ALTERNATIVE_2("li %0, 0\t\nnop", \
> "li %0, %1\t\nslli %0,%0,%3", 0, \
> - CPUFEATURE_SVPBMT, CONFIG_RISCV_ISA_SVPBMT, \
> + RISCV_ISA_EXT_SVPBMT, CONFIG_RISCV_ISA_SVPBMT, \
> "li %0, %2\t\nslli %0,%0,%4", THEAD_VENDOR_ID, \
> ERRATA_THEAD_PBMT, CONFIG_ERRATA_THEAD_PBMT) \
> : "=r"(_val) \
> @@ -127,7 +124,7 @@ asm volatile(ALTERNATIVE_2( \
> "add a0, a0, %0\n\t" \
> "2:\n\t" \
> "bltu a0, %2, 3b\n\t" \
> - "nop", 0, CPUFEATURE_ZICBOM, CONFIG_RISCV_ISA_ZICBOM, \
> + "nop", 0, RISCV_ISA_EXT_ZICBOM, CONFIG_RISCV_ISA_ZICBOM, \
> "mv a0, %1\n\t" \
> "j 2f\n\t" \
> "3:\n\t" \
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index afa54635c180..2b1f18f97253 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -251,61 +251,11 @@ void __init riscv_fill_hwcap(void)
> }
>
> #ifdef CONFIG_RISCV_ALTERNATIVE
> -static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
> -{
> -#ifdef CONFIG_RISCV_ISA_SVPBMT
> - switch (stage) {
> - case RISCV_ALTERNATIVES_EARLY_BOOT:
> - return false;
> - default:
> - return riscv_isa_extension_available(NULL, SVPBMT);
> - }
> -#endif
> -
> - return false;
> -}
> -
> -static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
> -{
> -#ifdef CONFIG_RISCV_ISA_ZICBOM
> - switch (stage) {
> - case RISCV_ALTERNATIVES_EARLY_BOOT:
> - return false;
> - default:
> - return riscv_isa_extension_available(NULL, ZICBOM);
> - }
> -#endif
> -
> - return false;
> -}
> -
> -/*
> - * Probe presence of individual extensions.
> - *
> - * This code may also be executed before kernel relocation, so we cannot use
> - * addresses generated by the address-of operator as they won't be valid in
> - * this context.
> - */
> -static u32 __init_or_module cpufeature_probe(unsigned int stage)
> -{
> - u32 cpu_req_feature = 0;
> -
> - if (cpufeature_probe_svpbmt(stage))
> - cpu_req_feature |= (1U << CPUFEATURE_SVPBMT);
> -
> - if (cpufeature_probe_zicbom(stage))
> - cpu_req_feature |= (1U << CPUFEATURE_ZICBOM);
> -
> - return cpu_req_feature;
> -}
> -
> void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> struct alt_entry *end,
> unsigned int stage)
> {
> - u32 cpu_req_feature = cpufeature_probe(stage);
> struct alt_entry *alt;
> - u32 tmp;
>
> if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> return;
> @@ -313,15 +263,16 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> for (alt = begin; alt < end; alt++) {
> if (alt->vendor_id != 0)
> continue;
> - if (alt->errata_id >= CPUFEATURE_NUMBER) {
> - WARN(1, "This feature id:%d is not in kernel cpufeature list",
> + if (alt->errata_id >= RISCV_ISA_EXT_ID_MAX) {
Ok, so RISCV_ISA_EXT_ID_MAX is used here now, but we could use
RISCV_ISA_EXT_MAX directly instead.
> + WARN(1, "This extension id:%d is not in ISA extension list",
^ the
> alt->errata_id);
> continue;
> }
>
> - tmp = (1U << alt->errata_id);
> - if (cpu_req_feature & tmp)
> - patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> + if (!test_bit(alt->errata_id, riscv_isa))
Maybe avoid using riscv_isa directly in case we want to move this function
out of cpufeature.c some day?
if (!__riscv_isa_extension_available(NULL, alt->errata_id))
> + continue;
> +
> + patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> }
> }
> #endif
> --
> 2.37.2
>
Otherwise,
Reviewed-by: Andrew Jones <[email protected]>
Am Donnerstag, 6. Oktober 2022, 09:08:14 CEST schrieb Jisheng Zhang:
> make the riscv_cpufeature_patch_func() scan all ISA extensions rather
> than limited feature macros.
>
> Signed-off-by: Jisheng Zhang <[email protected]>
> ---
> arch/riscv/include/asm/errata_list.h | 9 ++--
> arch/riscv/kernel/cpufeature.c | 61 +++-------------------------
> 2 files changed, 9 insertions(+), 61 deletions(-)
>
> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
> index 19a771085781..722525f4fc96 100644
> --- a/arch/riscv/include/asm/errata_list.h
> +++ b/arch/riscv/include/asm/errata_list.h
> @@ -6,6 +6,7 @@
> #define ASM_ERRATA_LIST_H
>
> #include <asm/alternative.h>
> +#include <asm/hwcap.h>
> #include <asm/vendorid_list.h>
>
> #ifdef CONFIG_ERRATA_SIFIVE
> @@ -20,10 +21,6 @@
> #define ERRATA_THEAD_NUMBER 2
> #endif
>
> -#define CPUFEATURE_SVPBMT 0
> -#define CPUFEATURE_ZICBOM 1
> -#define CPUFEATURE_NUMBER 2
> -
> #ifdef __ASSEMBLY__
>
> #define ALT_INSN_FAULT(x) \
> @@ -53,7 +50,7 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID, \
> #define ALT_SVPBMT(_val, prot) \
> asm(ALTERNATIVE_2("li %0, 0\t\nnop", \
> "li %0, %1\t\nslli %0,%0,%3", 0, \
> - CPUFEATURE_SVPBMT, CONFIG_RISCV_ISA_SVPBMT, \
> + RISCV_ISA_EXT_SVPBMT, CONFIG_RISCV_ISA_SVPBMT, \
> "li %0, %2\t\nslli %0,%0,%4", THEAD_VENDOR_ID, \
> ERRATA_THEAD_PBMT, CONFIG_ERRATA_THEAD_PBMT) \
> : "=r"(_val) \
> @@ -127,7 +124,7 @@ asm volatile(ALTERNATIVE_2( \
> "add a0, a0, %0\n\t" \
> "2:\n\t" \
> "bltu a0, %2, 3b\n\t" \
> - "nop", 0, CPUFEATURE_ZICBOM, CONFIG_RISCV_ISA_ZICBOM, \
> + "nop", 0, RISCV_ISA_EXT_ZICBOM, CONFIG_RISCV_ISA_ZICBOM, \
hmm, would it make sense to also at the same time extend the errata_id
in the alternatives struct to unsigned long?
Right now it's a unsigned int, and we're already at bit30 with the current extensions.
Otherwise the idea is pretty neat of allowing easy handling for all extensions
Reviewed-by: Heiko Stuebner <[email protected]>
> "mv a0, %1\n\t" \
> "j 2f\n\t" \
> "3:\n\t" \
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index afa54635c180..2b1f18f97253 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -251,61 +251,11 @@ void __init riscv_fill_hwcap(void)
> }
>
> #ifdef CONFIG_RISCV_ALTERNATIVE
> -static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
> -{
> -#ifdef CONFIG_RISCV_ISA_SVPBMT
> - switch (stage) {
> - case RISCV_ALTERNATIVES_EARLY_BOOT:
> - return false;
> - default:
> - return riscv_isa_extension_available(NULL, SVPBMT);
> - }
> -#endif
> -
> - return false;
> -}
> -
> -static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
> -{
> -#ifdef CONFIG_RISCV_ISA_ZICBOM
> - switch (stage) {
> - case RISCV_ALTERNATIVES_EARLY_BOOT:
> - return false;
> - default:
> - return riscv_isa_extension_available(NULL, ZICBOM);
> - }
> -#endif
> -
> - return false;
> -}
> -
> -/*
> - * Probe presence of individual extensions.
> - *
> - * This code may also be executed before kernel relocation, so we cannot use
> - * addresses generated by the address-of operator as they won't be valid in
> - * this context.
> - */
> -static u32 __init_or_module cpufeature_probe(unsigned int stage)
> -{
> - u32 cpu_req_feature = 0;
> -
> - if (cpufeature_probe_svpbmt(stage))
> - cpu_req_feature |= (1U << CPUFEATURE_SVPBMT);
> -
> - if (cpufeature_probe_zicbom(stage))
> - cpu_req_feature |= (1U << CPUFEATURE_ZICBOM);
> -
> - return cpu_req_feature;
> -}
> -
> void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> struct alt_entry *end,
> unsigned int stage)
> {
> - u32 cpu_req_feature = cpufeature_probe(stage);
> struct alt_entry *alt;
> - u32 tmp;
>
> if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> return;
> @@ -313,15 +263,16 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> for (alt = begin; alt < end; alt++) {
> if (alt->vendor_id != 0)
> continue;
> - if (alt->errata_id >= CPUFEATURE_NUMBER) {
> - WARN(1, "This feature id:%d is not in kernel cpufeature list",
> + if (alt->errata_id >= RISCV_ISA_EXT_ID_MAX) {
> + WARN(1, "This extension id:%d is not in ISA extension list",
> alt->errata_id);
> continue;
> }
>
> - tmp = (1U << alt->errata_id);
> - if (cpu_req_feature & tmp)
> - patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> + if (!test_bit(alt->errata_id, riscv_isa))
> + continue;
> +
> + patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> }
> }
> #endif
>
Am Freitag, 7. Oktober 2022, 13:54:31 CEST schrieb Heiko St?bner:
> Am Donnerstag, 6. Oktober 2022, 09:08:14 CEST schrieb Jisheng Zhang:
> > make the riscv_cpufeature_patch_func() scan all ISA extensions rather
> > than limited feature macros.
> >
> > Signed-off-by: Jisheng Zhang <[email protected]>
[...]
> > @@ -127,7 +124,7 @@ asm volatile(ALTERNATIVE_2( \
> > "add a0, a0, %0\n\t" \
> > "2:\n\t" \
> > "bltu a0, %2, 3b\n\t" \
> > - "nop", 0, CPUFEATURE_ZICBOM, CONFIG_RISCV_ISA_ZICBOM, \
> > + "nop", 0, RISCV_ISA_EXT_ZICBOM, CONFIG_RISCV_ISA_ZICBOM, \
>
> hmm, would it make sense to also at the same time extend the errata_id
> in the alternatives struct to unsigned long?
>
> Right now it's a unsigned int, and we're already at bit30 with the current extensions.
>
> Otherwise the idea is pretty neat of allowing easy handling for all extensions
>
> Reviewed-by: Heiko Stuebner <[email protected]>
I think I might need to take that back ... with this change each
cpufeature is tightly coupled to real extension ids, but what about
cpufeatures that do not match directly to an extension?
I.e. ZICBOM + fast-unaligned-access [0] (coming from a dt-property)
or only viable with extension 1+2+3?
Heiko
[0] https://git.kernel.org/pub/scm/linux/kernel/git/palmer/linux.git/commit/?h=riscv-hwprobe&id=9be297f7ed349945cccc85f8df9d90e5ab68c1d9