On Mon, Dec 05, 2022 at 01:46:24AM +0800, Jisheng Zhang wrote:
> make the riscv_cpufeature_patch_func() scan all ISA extensions rather
> than limited feature macros.
Certainly looks like a nice cleanup. Perhaps for the changelog,
something along the lines of:
"riscv_cpufeature_patch_func() currently only scans a limited set of
cpufeatures, explicitly defined with macros. Extend it to probe for all
ISA extensions"
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> ---
> arch/riscv/include/asm/errata_list.h | 9 ++--
> arch/riscv/kernel/cpufeature.c | 73 +++++-----------------------
> 2 files changed, 15 insertions(+), 67 deletions(-)
> @@ -311,25 +264,23 @@ 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_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) {
> - /* do the basic patching */
> - patch_text_nosync(alt->old_ptr, alt->alt_ptr,
> - alt->alt_len);
> + if (!__riscv_isa_extension_available(NULL, alt->errata_id))
> + continue;
>
> - riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> - alt->alt_len,
> - alt->old_ptr - alt->alt_ptr);
> - riscv_alternative_fix_jal(alt->old_ptr,
> - alt->alt_len,
> - alt->old_ptr - alt->alt_ptr);
> - }
> + /* do the basic patching */
> + patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> + riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> + alt->alt_len,
> + alt->old_ptr - alt->alt_ptr);
> + riscv_alternative_fix_jal(alt->old_ptr,
> + alt->alt_len,
> + alt->old_ptr - alt->alt_ptr);
nit:
Now that you've dropped a level of indent, can alt->alt_len move up a
line?
Thanks,
Conor.
@@ -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" \
@@ -252,58 +252,11 @@ void __init riscv_fill_hwcap(void)
}
#ifdef CONFIG_RISCV_ALTERNATIVE
-static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
-{
- if (!IS_ENABLED(CONFIG_RISCV_ISA_SVPBMT))
- return false;
-
- if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
- return false;
-
- return riscv_isa_extension_available(NULL, SVPBMT);
-}
-
-static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
-{
- if (!IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM))
- return false;
-
- if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
- return false;
-
- if (!riscv_isa_extension_available(NULL, ZICBOM))
- return false;
-
- return true;
-}
-
-/*
- * 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 |= BIT(CPUFEATURE_SVPBMT);
-
- if (cpufeature_probe_zicbom(stage))
- cpu_req_feature |= BIT(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;
@@ -311,25 +264,23 @@ 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_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) {
- /* do the basic patching */
- patch_text_nosync(alt->old_ptr, alt->alt_ptr,
- alt->alt_len);
+ if (!__riscv_isa_extension_available(NULL, alt->errata_id))
+ continue;
- riscv_alternative_fix_auipc_jalr(alt->old_ptr,
- alt->alt_len,
- alt->old_ptr - alt->alt_ptr);
- riscv_alternative_fix_jal(alt->old_ptr,
- alt->alt_len,
- alt->old_ptr - alt->alt_ptr);
- }
+ /* do the basic patching */
+ patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
+ riscv_alternative_fix_auipc_jalr(alt->old_ptr,
+ alt->alt_len,
+ alt->old_ptr - alt->alt_ptr);
+ riscv_alternative_fix_jal(alt->old_ptr,
+ alt->alt_len,
+ alt->old_ptr - alt->alt_ptr);
}
}
#endif