| Message ID | 20220413030307.133807-12-heiko@sntech.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | riscv: support for Svpbmt and D1 memory types | expand |
Reviewed-by: Guo Ren <guoren@kernel.org> On Wed, Apr 13, 2022 at 11:05 AM Heiko Stuebner <heiko@sntech.de> wrote: > > Right now the code uses a global struct to store vendor-ids > and another global variable to store the vendor-patch-function. > > There exist specific cases where we'll need to patch the kernel > at an even earlier stage, where trying to write to a static > variable might actually result in hangs. > > Also collecting the vendor-information consists of 3 sbi-ecalls > (or csr-reads) which is pretty negligible in the context of > booting a kernel. > > So rework the code to not rely on static variables and instead > collect the vendor-information when a round of alternatives is > to be applied. > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> > --- > arch/riscv/kernel/alternative.c | 51 ++++++++++++++++----------------- > 1 file changed, 24 insertions(+), 27 deletions(-) > > diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c > index e6c9de9f9ba6..27f722ae452b 100644 > --- a/arch/riscv/kernel/alternative.c > +++ b/arch/riscv/kernel/alternative.c > @@ -16,41 +16,35 @@ > #include <asm/sbi.h> > #include <asm/csr.h> > > -static struct cpu_manufacturer_info_t { > +struct cpu_manufacturer_info_t { > unsigned long vendor_id; > unsigned long arch_id; > unsigned long imp_id; > -} cpu_mfr_info; > + void (*vendor_patch_func)(struct alt_entry *begin, struct alt_entry *end, > + unsigned long archid, unsigned long impid, > + unsigned int stage); > +}; > > -static void (*vendor_patch_func)(struct alt_entry *begin, struct alt_entry *end, > - unsigned long archid, unsigned long impid, > - unsigned int stage) __initdata_or_module; > - > -static inline void __init riscv_fill_cpu_mfr_info(void) > +static void __init_or_module riscv_fill_cpu_mfr_info(struct cpu_manufacturer_info_t *cpu_mfr_info) > { > #ifdef CONFIG_RISCV_M_MODE > - cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID); > - cpu_mfr_info.arch_id = csr_read(CSR_MARCHID); > - cpu_mfr_info.imp_id = csr_read(CSR_MIMPID); > + cpu_mfr_info->vendor_id = csr_read(CSR_MVENDORID); > + cpu_mfr_info->arch_id = csr_read(CSR_MARCHID); > + cpu_mfr_info->imp_id = csr_read(CSR_MIMPID); > #else > - cpu_mfr_info.vendor_id = sbi_get_mvendorid(); > - cpu_mfr_info.arch_id = sbi_get_marchid(); > - cpu_mfr_info.imp_id = sbi_get_mimpid(); > + cpu_mfr_info->vendor_id = sbi_get_mvendorid(); > + cpu_mfr_info->arch_id = sbi_get_marchid(); > + cpu_mfr_info->imp_id = sbi_get_mimpid(); > #endif > -} > - > -static void __init init_alternative(void) > -{ > - riscv_fill_cpu_mfr_info(); > > - switch (cpu_mfr_info.vendor_id) { > + switch (cpu_mfr_info->vendor_id) { > #ifdef CONFIG_ERRATA_SIFIVE > case SIFIVE_VENDOR_ID: > - vendor_patch_func = sifive_errata_patch_func; > + cpu_mfr_info->vendor_patch_func = sifive_errata_patch_func; > break; > #endif > default: > - vendor_patch_func = NULL; > + cpu_mfr_info->vendor_patch_func = NULL; > } > } > > @@ -63,14 +57,19 @@ static void __init_or_module _apply_alternatives(struct alt_entry *begin, > struct alt_entry *end, > unsigned int stage) > { > + struct cpu_manufacturer_info_t cpu_mfr_info; > + > + riscv_fill_cpu_mfr_info(&cpu_mfr_info); > + > riscv_cpufeature_patch_func(begin, end, stage); > > - if (!vendor_patch_func) > + if (!cpu_mfr_info.vendor_patch_func) > return; > > - vendor_patch_func(begin, end, > - cpu_mfr_info.arch_id, cpu_mfr_info.imp_id, > - stage); > + cpu_mfr_info.vendor_patch_func(begin, end, > + cpu_mfr_info.arch_id, > + cpu_mfr_info.imp_id, > + stage); > } > > void __init apply_boot_alternatives(void) > @@ -78,8 +77,6 @@ void __init apply_boot_alternatives(void) > /* If called on non-boot cpu things could go wrong */ > WARN_ON(smp_processor_id() != 0); > > - init_alternative(); > - > _apply_alternatives((struct alt_entry *)__alt_start, > (struct alt_entry *)__alt_end, > RISCV_ALTERNATIVES_BOOT); > -- > 2.35.1 >
On Wed, 13 Apr 2022 at 05:05, Heiko Stuebner <heiko@sntech.de> wrote: > > Right now the code uses a global struct to store vendor-ids > and another global variable to store the vendor-patch-function. > > There exist specific cases where we'll need to patch the kernel > at an even earlier stage, where trying to write to a static > variable might actually result in hangs. > > Also collecting the vendor-information consists of 3 sbi-ecalls > (or csr-reads) which is pretty negligible in the context of > booting a kernel. > > So rework the code to not rely on static variables and instead > collect the vendor-information when a round of alternatives is > to be applied. > > Signed-off-by: Heiko Stuebner <heiko@sntech.de> Reviewed-by: Philipp Tomsich <philipp.tomsich@vrull.eu>
diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c index e6c9de9f9ba6..27f722ae452b 100644 --- a/arch/riscv/kernel/alternative.c +++ b/arch/riscv/kernel/alternative.c @@ -16,41 +16,35 @@ #include <asm/sbi.h> #include <asm/csr.h> -static struct cpu_manufacturer_info_t { +struct cpu_manufacturer_info_t { unsigned long vendor_id; unsigned long arch_id; unsigned long imp_id; -} cpu_mfr_info; + void (*vendor_patch_func)(struct alt_entry *begin, struct alt_entry *end, + unsigned long archid, unsigned long impid, + unsigned int stage); +}; -static void (*vendor_patch_func)(struct alt_entry *begin, struct alt_entry *end, - unsigned long archid, unsigned long impid, - unsigned int stage) __initdata_or_module; - -static inline void __init riscv_fill_cpu_mfr_info(void) +static void __init_or_module riscv_fill_cpu_mfr_info(struct cpu_manufacturer_info_t *cpu_mfr_info) { #ifdef CONFIG_RISCV_M_MODE - cpu_mfr_info.vendor_id = csr_read(CSR_MVENDORID); - cpu_mfr_info.arch_id = csr_read(CSR_MARCHID); - cpu_mfr_info.imp_id = csr_read(CSR_MIMPID); + cpu_mfr_info->vendor_id = csr_read(CSR_MVENDORID); + cpu_mfr_info->arch_id = csr_read(CSR_MARCHID); + cpu_mfr_info->imp_id = csr_read(CSR_MIMPID); #else - cpu_mfr_info.vendor_id = sbi_get_mvendorid(); - cpu_mfr_info.arch_id = sbi_get_marchid(); - cpu_mfr_info.imp_id = sbi_get_mimpid(); + cpu_mfr_info->vendor_id = sbi_get_mvendorid(); + cpu_mfr_info->arch_id = sbi_get_marchid(); + cpu_mfr_info->imp_id = sbi_get_mimpid(); #endif -} - -static void __init init_alternative(void) -{ - riscv_fill_cpu_mfr_info(); - switch (cpu_mfr_info.vendor_id) { + switch (cpu_mfr_info->vendor_id) { #ifdef CONFIG_ERRATA_SIFIVE case SIFIVE_VENDOR_ID: - vendor_patch_func = sifive_errata_patch_func; + cpu_mfr_info->vendor_patch_func = sifive_errata_patch_func; break; #endif default: - vendor_patch_func = NULL; + cpu_mfr_info->vendor_patch_func = NULL; } } @@ -63,14 +57,19 @@ static void __init_or_module _apply_alternatives(struct alt_entry *begin, struct alt_entry *end, unsigned int stage) { + struct cpu_manufacturer_info_t cpu_mfr_info; + + riscv_fill_cpu_mfr_info(&cpu_mfr_info); + riscv_cpufeature_patch_func(begin, end, stage); - if (!vendor_patch_func) + if (!cpu_mfr_info.vendor_patch_func) return; - vendor_patch_func(begin, end, - cpu_mfr_info.arch_id, cpu_mfr_info.imp_id, - stage); + cpu_mfr_info.vendor_patch_func(begin, end, + cpu_mfr_info.arch_id, + cpu_mfr_info.imp_id, + stage); } void __init apply_boot_alternatives(void) @@ -78,8 +77,6 @@ void __init apply_boot_alternatives(void) /* If called on non-boot cpu things could go wrong */ WARN_ON(smp_processor_id() != 0); - init_alternative(); - _apply_alternatives((struct alt_entry *)__alt_start, (struct alt_entry *)__alt_end, RISCV_ALTERNATIVES_BOOT);
Right now the code uses a global struct to store vendor-ids and another global variable to store the vendor-patch-function. There exist specific cases where we'll need to patch the kernel at an even earlier stage, where trying to write to a static variable might actually result in hangs. Also collecting the vendor-information consists of 3 sbi-ecalls (or csr-reads) which is pretty negligible in the context of booting a kernel. So rework the code to not rely on static variables and instead collect the vendor-information when a round of alternatives is to be applied. Signed-off-by: Heiko Stuebner <heiko@sntech.de> --- arch/riscv/kernel/alternative.c | 51 ++++++++++++++++----------------- 1 file changed, 24 insertions(+), 27 deletions(-)