riscv: improve boot time isa extensions handling
Subsystems (5)
v5 cover riscv: improve boot time isa extensions handling
Generally, riscv ISA extensions are fixed for any specific hardware platform, so a hart's features won't change after booting, this chacteristic makes it straightforward to use a static branch to check a specific ISA extension is supported or not to optimize performance.
However, some ISA extensions such as SVPBMT and ZICBOM are handled via. the alternative sequences.
Basically, for ease of maintenance, we prefer to use static branches in C code, but recently, Samuel found that the static branch usage in cpu_relax() breaks building with CONFIG_CC_OPTIMIZE_FOR_SIZE[1]. As Samuel pointed out, "Having a static branch in cpu_relax() is problematic because that function is widely inlined, including in some quite complex functions like in the VDSO. A quick measurement shows this static branch is responsible by itself for around 40% of the jump table."
Samuel's findings pointed out one of a few downsides of static branches usage in C code to handle ISA extensions detected at boot time:
static branch's metadata in the __jump_table section, which is not discarded after ISA extensions are finalized, wastes some space.
I want to try to solve the issue for all possible dynamic handling of ISA extensions at boot time. Inspired by Mark[2], this patch introduces riscv_has_extension_*() helpers, which work like static branches but are patched using alternatives, thus the metadata can be freed after patching.
Since v4
- collect Reviewed-by and Acked-by tag
- rebase on the latest riscv for-next
- add Andrew's patch to add ADD16 and SUB16 rela types
- adopt Conor's nit comment to patch9
Since v3
- collect Reviewed-by tag and remove Heiko's reviewed-by from patch5
- address Conor and Andrew comments
- fix two building errors of !MMU and RV32
Since v2
- rebase on riscv-next
- collect Reviewed-by tag
- fix jal imm construction
- combine Heiko's code and my code for jal patching, thus add Co-developed-by tag
- address comments from Conor
Since v1
- rebase on v6.1-rc7 + Heiko's alternative improvements[3]
- collect Reviewed-by tag
- add one patch to update jal offsets in patched alternatives
- add one patch to switch to relative alternative entries
- add patches to patch vdso
[1]https://lore.kernel.org/linux-riscv/20220922060958.44203-1-samuel at sholland.org/ [2]https://lore.kernel.org/linux-arm-kernel/20220912162210.3626215-8-mark.rutland at arm.com/ [3]https://lore.kernel.org/linux-riscv/20221130225614.1594256-1-heiko at sntech.de/
Andrew Jones (2):
riscv: module: Add ADD16 and SUB16 rela types
riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely()
Jisheng Zhang (11):
riscv: move riscv_noncoherent_supported() out of ZICBOM probe
riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier
riscv: hwcap: make ISA extension ids can be used in asm
riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA
extensions
riscv: introduce riscv_has_extension_[un]likely()
riscv: fpu: switch has_fpu() to riscv_has_extension_likely()
riscv: module: move find_section to module.h
riscv: switch to relative alternative entries
riscv: alternative: patch alternatives in the vDSO
riscv: cpu_relax: switch to riscv_has_extension_likely()
riscv: remove riscv_isa_ext_keys[] array and related usage
arch/riscv/errata/sifive/errata.c | 3 +- arch/riscv/errata/thead/errata.c | 11 ++- arch/riscv/include/asm/alternative-macros.h | 20 ++--- arch/riscv/include/asm/alternative.h | 17 ++-- arch/riscv/include/asm/errata_list.h | 9 +- arch/riscv/include/asm/hwcap.h | 98 +++++++++++---------- arch/riscv/include/asm/module.h | 16 ++++ arch/riscv/include/asm/switch_to.h | 3 +- arch/riscv/include/asm/vdso.h | 4 + arch/riscv/include/asm/vdso/processor.h | 2 +- arch/riscv/kernel/alternative.c | 29 ++++++ arch/riscv/kernel/cpufeature.c | 79 +++-------------- arch/riscv/kernel/module.c | 31 +++---- arch/riscv/kernel/setup.c | 3 + arch/riscv/kernel/vdso.c | 5 -- arch/riscv/kernel/vdso/vdso.lds.S | 7 ++ arch/riscv/kvm/tlb.c | 3 +- 17 files changed, 176 insertions(+), 164 deletions(-)
Signature · 1 line
2.38.1
PATCH v5 1/13 riscv: move riscv_noncoherent_supported() out of ZICBOM probe
It's a bit weird to call riscv_noncoherent_supported() each time when insmoding a module. Move the calling out of feature patch func.
--- arch/riscv/kernel/cpufeature.c | 1 - arch/riscv/kernel/setup.c | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-)
arch/riscv/kernel/cpufeature.c +0 -1
@@ -298,7 +298,6 @@ static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)if (!riscv_isa_extension_available (NULL ,ZICBOM ))return false; -riscv_noncoherent_supported ();return true; }
arch/riscv/kernel/setup.c +3 -0
@@ -300,6 +300,9 @@ void __init setup_arch(char **cmdline_p)riscv_init_cbom_blocksize ();riscv_fill_hwcap ();apply_boot_alternatives (); +if (IS_ENABLED (CONFIG_RISCV_ISA_ZICBOM ) && +riscv_isa_extension_available (NULL ,ZICBOM )) +riscv_noncoherent_supported (); }static int __init topology_init (void )
Signature · 1 line
2.38.1
PATCH v5 2/13 riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier
In reply to Jisheng ZhangCurrently riscv_cpufeature_patch_func() does nothing at the RISCV_ALTERNATIVES_EARLY_BOOT stage. Add a check to detect whether we are in this stage and exit early. This will allow us to use riscv_cpufeature_patch_func() for scanning of all ISA extensions.
--- arch/riscv/kernel/cpufeature.c | 3 +++ 1 file changed, 3 insertions(+)
arch/riscv/kernel/cpufeature.c +3 -0
@@ -330,6 +330,9 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,struct alt_entry *alt;u32 tmp; +if (stage ==RISCV_ALTERNATIVES_EARLY_BOOT ) +return ; +for (alt = begin; alt < end; alt++) {if (alt->vendor_id !=0 )continue ;
Signature · 1 line
2.38.1
PATCH v5 3/13 riscv: hwcap: make ISA extension ids can be used in asm
In reply to Jisheng ZhangSo that ISA extensions can be used in assembly files, convert the multi-letter RISC-V ISA extension IDs enums to macros. In order to make them visible, move the #ifndef __ASSEMBLY__ guard to a later point in the header
--- arch/riscv/include/asm/hwcap.h | 45 ++++++++++++++++------------------ 1 file changed, 21 insertions(+), 24 deletions(-)
arch/riscv/include/asm/hwcap.h +21 -24
@@ -12,20 +12,6 @@#include <linux/bits.h> #include <uapi/asm/hwcap.h> -#ifndef __ASSEMBLY__ -#include <linux/jump_label.h> -/* -* This yields a mask that user programs can use to figure out what -* instruction set this cpu supports. -*/ -#define ELF_HWCAP (elf_hwcap) - -enum { -CAP_HWCAP =1 , -}; - -extern unsigned long elf_hwcap; -#define RISCV_ISA_EXT_a ('a' - 'a')#define RISCV_ISA_EXT_c ('c' - 'a')#define RISCV_ISA_EXT_d ('d' - 'a')
@@ -46,23 +32,34 @@ extern unsigned long elf_hwcap;#define RISCV_ISA_EXT_BASE 26/* -* This enum represent the logical ID for each multi-letter RISC-V ISA extension. +* These macros represent the logical ID for each multi-letter RISC-V ISA extension. * The logical ID should start from RISCV_ISA_EXT_BASE and must not exceed * RISCV_ISA_EXT_MAX. 0-25 range is reserved for single letter * extensions while all the multi-letter extensions should define the next * available logical extension id. * Entries are sorted alphabetically. */ -enum riscv_isa_ext_id { -RISCV_ISA_EXT_SSCOFPMF =RISCV_ISA_EXT_BASE , -RISCV_ISA_EXT_SSTC , -RISCV_ISA_EXT_SVINVAL , -RISCV_ISA_EXT_SVPBMT , -RISCV_ISA_EXT_ZICBOM , -RISCV_ISA_EXT_ZIHINTPAUSE , -RISCV_ISA_EXT_ID_MAX +#define RISCV_ISA_EXT_SSCOFPMF 26 +#define RISCV_ISA_EXT_SSTC 27 +#define RISCV_ISA_EXT_SVINVAL 28 +#define RISCV_ISA_EXT_SVPBMT 29 +#define RISCV_ISA_EXT_ZICBOM 30 +#define RISCV_ISA_EXT_ZIHINTPAUSE 31 + +#ifndef __ASSEMBLY__ +#include <linux/jump_label.h> +/* +* This yields a mask that user programs can use to figure out what +* instruction set this cpu supports. +*/ +#define ELF_HWCAP (elf_hwcap) + +enum { +CAP_HWCAP =1 , }; -static_assert (RISCV_ISA_EXT_ID_MAX <=RISCV_ISA_EXT_MAX ); + +extern unsigned long elf_hwcap; +/* * This enum represents the logical ID for each RISC-V ISA extension static
Signature · 1 line
2.38.1
PATCH v5 4/13 riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions
In reply to Jisheng Zhangriscv_cpufeature_patch_func() currently only scans a limited set of cpufeatures, explicitly defined with macros. Extend it to probe for all ISA extensions.
--- arch/riscv/include/asm/errata_list.h | 9 ++-- arch/riscv/kernel/cpufeature.c | 64 ++++------------------------ 2 files changed, 11 insertions(+), 62 deletions(-)
arch/riscv/include/asm/errata_list.h +3 -6
@@ -7,6 +7,7 @@#include <asm/alternative.h> #include <asm/csr.h> +#include <asm/hwcap.h> #include <asm/vendorid_list.h> #ifdef CONFIG_ERRATA_SIFIVE
@@ -22,10 +23,6 @@#define ERRATA_THEAD_NUMBER 3#endif -#define CPUFEATURE_SVPBMT 0 -#define CPUFEATURE_ZICBOM 1 -#define CPUFEATURE_NUMBER 2 -#ifdef __ASSEMBLY__#define ALT_INSN_FAULT (x) \
@@ -55,7 +52,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) \
@@ -129,7 +126,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" \
arch/riscv/kernel/cpufeature.c +8 -56
85 lines not on this page · raw message
Signature · 1 line
2.38.1
PATCH v5 5/13 riscv: introduce riscv_has_extension_[un]likely()
In reply to Jisheng ZhangGenerally, riscv ISA extensions are fixed for any specific hardware platform, so a hart's features won't change after booting. This chacteristic makes it straightforward to use a static branch to check if a specific ISA extension is supported or not to optimize performance.
However, some ISA extensions such as SVPBMT and ZICBOM are handled via. the alternative sequences.
Basically, for ease of maintenance, we prefer to use static branches in C code, but recently, Samuel found that the static branch usage in cpu_relax() breaks building with CONFIG_CC_OPTIMIZE_FOR_SIZE[1]. As Samuel pointed out, "Having a static branch in cpu_relax() is problematic because that function is widely inlined, including in some quite complex functions like in the VDSO. A quick measurement shows this static branch is responsible by itself for around 40% of the jump table."
Samuel's findings pointed out one of a few downsides of static branches usage in C code to handle ISA extensions detected at boot time:
static branch's metadata in the __jump_table section, which is not discarded after ISA extensions are finalized, wastes some space.
I want to try to solve the issue for all possible dynamic handling of ISA extensions at boot time. Inspired by Mark[2], this patch introduces riscv_has_extension_*() helpers, which work like static branches but are patched using alternatives, thus the metadata can be freed after patching.
--- arch/riscv/include/asm/hwcap.h | 37 ++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+)
arch/riscv/include/asm/hwcap.h +37 -0
@@ -8,6 +8,7 @@#ifndef _ASM_RISCV_HWCAP_H#define _ASM_RISCV_HWCAP_H +#include <asm/alternative-macros.h> #include <asm/errno.h> #include <linux/bits.h> #include <uapi/asm/hwcap.h>
@@ -99,6 +100,42 @@ static __always_inline int riscv_isa_ext2key(int num) } } +static __always_inline bool +riscv_has_extension_likely (const unsigned long ext) +{ +compiletime_assert (ext <RISCV_ISA_EXT_MAX , +"ext must be < RISCV_ISA_EXT_MAX" ); + +asm_volatile_goto ( + ALTERNATIVE("j %l[l_no]", "nop", 0, %[ext], 1) + : + : [ext] "i" (ext) + : + : l_no); + +return true; +l_no: +return false; +} + +static __always_inline bool +riscv_has_extension_unlikely (const unsigned long ext) +{ +compiletime_assert (ext <RISCV_ISA_EXT_MAX , +"ext must be < RISCV_ISA_EXT_MAX" ); + +asm_volatile_goto ( + ALTERNATIVE("nop", "j %l[l_yes]", 0, %[ext], 1) + : + : [ext] "i" (ext) + : + : l_yes); + +return false; +l_yes: +return true; +} +unsigned long riscv_isa_extension_base (const unsigned long *isa_bitmap);#define riscv_isa_extension_mask (ext) BIT_MASK(RISCV_ISA_EXT_##ext)
Signature · 1 line
2.38.1
PATCH v5 6/13 riscv: fpu: switch has_fpu() to riscv_has_extension_likely()
In reply to Jisheng ZhangSwitch has_fpu() from static branch to the new helper riscv_has_extension_likely().
--- arch/riscv/include/asm/switch_to.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
arch/riscv/include/asm/switch_to.h +2 -1
@@ -59,7 +59,8 @@ static inline void __switch_to_aux(struct task_struct *prev, static __always_inline bool has_fpu(void) { -return static_branch_likely (&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_FPU ]); +return riscv_has_extension_likely (RISCV_ISA_EXT_f) || +riscv_has_extension_likely (RISCV_ISA_EXT_d); }#else static __always_inline bool has_fpu(void) { return false; }
Signature · 1 line
2.38.1
Re: [PATCH v5 06/13] riscv: fpu: switch has_fpu() to riscv_has_extension_likely()
Hi,
Quoted 16 lines · Jisheng Zhang
Switch
has_fpu()from static branch to the new helperriscv_has_extension_likely().Signed-off-by Jisheng Zhang <[email protected]>Reviewed-by Andrew Jones <[email protected]>Reviewed-by Heiko Stuebner <[email protected]>Reviewed-by Conor Dooley <[email protected]>--- arch/riscv/include/asm/switch_to.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)arch/riscv/include/asm/switch_to.h +0
-0
arch/riscv/include/asm/switch_to.h static inline void __switch_to_aux(struct task_struct *prev, · line 63
Quoted 6 lines
arch/riscv/include/asm/switch_to.h +2
-1@@ -59,7 +59,8 @@ static inline void __switch_to_aux(struct task_struct *prev, static __always_inline bool has_fpu(void) { -return static_branch_likely (&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_FPU ]); +return riscv_has_extension_likely (RISCV_ISA_EXT_f) || +riscv_has_extension_likely (RISCV_ISA_EXT_d);
This causes programs to crash on kernels that are compiled with CONFIG_RISCV_ALTERNATIVE=n. Since CONFIG_RISCV_ALTERNATIVE isn't selectable, this is a problem.
You can try this out for yourself using the WireGuard test suite:
ARCH=riscv64 make -C tools/testing/selftests/wireguard/qemu -j$(nproc)
And you'll see the crash:
[ 2.172093] init.sh[45]: unhandled signal 4 code 0x1 at 0x00ffffff945a2170 in libc.so[ffffff94562000+8c000] [ 2.174306] CPU: 0 PID: 45 Comm: init.sh Not tainted 6.3.0-rc3+ #1 [ 2.174981] Hardware name: riscv-virtio,qemu (DT) [ 2.175639] epc : 00ffffff945a2170 ra : 00aaaaaae7332820 sp : 00fffffffd3e6c00 [ 2.176287] gp : 00aaaaaae73aff40 tp : 00ffffff945f1a50 t0 : 0000000000000000 [ 2.176858] t1 : 00aaaaaae7331f9c t2 : 0000000000000002 s0 : 00fffffffd3e6de0 [ 2.177427] s1 : 0000000000000002 a0 : 00aaaaaae73b7380 a1 : 00fffffffd3e6dc8 [ 2.177990] a2 : 00fffffffd3e6de0 a3 : 0000000000000000 a4 : 0000000000000000 [ 2.178524] a5 : 0000000000000002 a6 : 000000000000008b a7 : 0000000000000010 [ 2.179081] s2 : 00aaaaaae73327f0 s3 : 00ffffff945ef990 s4 : 00ffffff945f1988 [ 2.179796] s5 : 00ffffff945f1b48 s6 : 0000000000000000 s7 : 00000000000000e0 [ 2.180366] s8 : 00ffffff945f1d58 s9 : 00ffffff945ecb88 s10: 00ffffff945f17e0 [ 2.185464] s11: 0000000000000001 t3 : 00ffffff945a213c t4 : 0000000300000000 [ 2.186106] t5 : 0000000000000003 t6 : ffffffffffffffff [ 2.186520] status: 0000000200000020 badaddr: 000000000000b920 cause: 0000000000000002
I bisected it to this commit:
702e64550b12 ("riscv: fpu: switch has_fpu() to riscv_has_extension_likely()")Thanks,
Jason
PATCH 1/1 riscv: require alternatives framework when selecting FPU support
When moving switch_to's has_fpu() over to using riscv_has_extension_ likely() rather than static branchs, the FPU code gained a dependency on the alternatives framework. If CONFIG_RISCV_ALTERNATIVE isn't selected when CONFIG_FPU is, then has_fpu() returns false, and switch_to does not work as intended. So select CONFIG_RISCV_ALTERNATIVE when CONFIG_FPU is selected.
--- arch/riscv/Kconfig | 1 + 1 file changed, 1 insertion(+)
arch/riscv/Kconfig +1 -0
@@ -467,6 +467,7 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE config FPU bool "FPU support" default y + select RISCV_ALTERNATIVE help Say N here if you want to disable all floating-point related procedure in the kernel.
Signature · 1 line
2.40.0
Re: [PATCH] riscv: require alternatives framework when selecting FPU support
Quoted 33 lines · Jason A. Donenfeld
When moving switch_to's
has_fpu()over to usingriscv_has_extension_likely()rather than static branchs, the FPU code gained a dependency on the alternatives framework. IfCONFIG_RISCV_ALTERNATIVEisn't selected whenCONFIG_FPUis, thenhas_fpu()returns false, andswitch_todoes not work as intended. So selectCONFIG_RISCV_ALTERNATIVEwhenCONFIG_FPUis selected.Fixes 702e64550b12 ("riscv: fpu: switch has_fpu() to riscv_has_extension_likely()")Link https://lore.kernel.org/all/ZBruFRwt3rUVngPu at zx2c4.com/Cc Jisheng Zhang <[email protected]>Cc Andrew Jones <[email protected]>Cc Heiko Stuebner <[email protected]>Cc Conor Dooley <[email protected]>Signed-off-by Jason A. Donenfeld <[email protected]>--- arch/riscv/Kconfig | 1 + 1 file changed, 1 insertion(+)arch/riscv/Kconfig +1
-0@@ -467,6 +467,7 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE config FPU bool "FPU support" default y + select RISCV_ALTERNATIVE help Say N here if you want to disable all floating-point related procedure in the kernel.Signature · 1 line
2.40.0
I took a look to see if we missed anything else and see that we should do the same patch for KVM. I'll send one.
(It's tempting to just select RISCV_ALTERNATIVE from RISCV, but maybe we
can defer that wedding a bit longer.)
Thanks,
drew
Conor Dooley
Reviewed-byRe: [PATCH] riscv: require alternatives framework when selecting FPU support
Quoted 8 lines · Andrew Jones
Quoted 7 lines · Jason A. Donenfeld
When moving switch_to's
has_fpu()over to usingriscv_has_extension_likely()rather than static branchs, the FPU code gained a dependency on the alternatives framework. IfCONFIG_RISCV_ALTERNATIVEisn't selected whenCONFIG_FPUis, thenhas_fpu()returns false, andswitch_todoes not work as intended. So selectCONFIG_RISCV_ALTERNATIVEwhenCONFIG_FPUis selected.
Quoted 6 lines
Quoted 6 lines
Fixes 702e64550b12 ("riscv: fpu: switch has_fpu() to riscv_has_extension_likely()")Cc Jisheng Zhang <[email protected]>Cc Andrew Jones <[email protected]>Cc Heiko Stuebner <[email protected]>Cc Conor Dooley <[email protected]>
Thanks for fixing it!
Quoted 23 lines
Quoted 20 lines
Signed-off-by Jason A. Donenfeld <[email protected]>--- arch/riscv/Kconfig | 1 + 1 file changed, 1 insertion(+)arch/riscv/Kconfig +1
-0@@ -467,6 +467,7 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE config FPU bool "FPU support" default y + select RISCV_ALTERNATIVE help Say N here if you want to disable all floating-point related procedure in the kernel.Signature · 1 line
2.40.0Reviewed-by Andrew Jones <[email protected]>
Quoted 5 lines
I took a look to see if we missed anything else and see that we should do the same patch for KVM. I'll send one.
(It's tempting to just select
RISCV_ALTERNATIVEfrom RISCV, but maybe we
can defer that wedding a bit longer.)
At that point, the config option should just go away entirely, no?
Re: [PATCH] riscv: require alternatives framework when selecting FPU support
Quoted 42 lines · Conor Dooley
Quoted 8 lines · Andrew Jones
Quoted 7 lines · Jason A. Donenfeld
When moving switch_to's
has_fpu()over to usingriscv_has_extension_likely()rather than static branchs, the FPU code gained a dependency on the alternatives framework. IfCONFIG_RISCV_ALTERNATIVEisn't selected whenCONFIG_FPUis, thenhas_fpu()returns false, andswitch_todoes not work as intended. So selectCONFIG_RISCV_ALTERNATIVEwhenCONFIG_FPUis selected.Quoted 6 lines
Quoted 6 lines
Fixes 702e64550b12 ("riscv: fpu: switch has_fpu() to riscv_has_extension_likely()")Link https://lore.kernel.org/all/ZBruFRwt3rUVngPu at zx2c4.com/Cc Jisheng Zhang <[email protected]>Cc Andrew Jones <[email protected]>Cc Heiko Stuebner <[email protected]>Cc Conor Dooley <[email protected]>Thanks for fixing it!
Reviewed-by Conor Dooley <[email protected]>Quoted 23 lines
Quoted 20 lines
Signed-off-by Jason A. Donenfeld <[email protected]>--- arch/riscv/Kconfig | 1 + 1 file changed, 1 insertion(+)arch/riscv/Kconfig +1
-0@@ -467,6 +467,7 @@ config TOOLCHAIN_HAS_ZIHINTPAUSE config FPU bool "FPU support" default y + select RISCV_ALTERNATIVE help Say N here if you want to disable all floating-point related procedure in the kernel.Signature · 1 line
2.40.0Reviewed-by Andrew Jones <[email protected]>
Quoted 7 lines
Quoted 5 lines
I took a look to see if we missed anything else and see that we should do the same patch for KVM. I'll send one.
(It's tempting to just select
RISCV_ALTERNATIVEfrom RISCV, but maybe we
can defer that wedding a bit longer.)At that point, the config option should just go away entirely, no?
Ah, yes, and that makes the idea even more attractive, as we could remove several ifdefs.
Thanks,
drew
Re: [PATCH] riscv: require alternatives framework when selecting FPU support
Quoted 10 lines · Andrew Jones
Quoted 6 lines · Conor Dooley
Quoted 2 lines · Andrew Jones
(It's tempting to just select
RISCV_ALTERNATIVEfrom RISCV, but maybe we
can defer that wedding a bit longer.)At that point, the config option should just go away entirely, no?
Ah, yes, and that makes the idea even more attractive, as we could remove several ifdefs.
I went and did the cursory check, it's not compatible with XIP_KERNEL so dropping the option entirely probably isn't a possibility :/
Re: [PATCH] riscv: require alternatives framework when selecting FPU support
Quoted 7 lines · Conor Dooley
Quoted 6 lines · Andrew Jones
Quoted 5 lines · Conor Dooley
Quoted 2 lines · Andrew Jones
(It's tempting to just select
RISCV_ALTERNATIVEfrom RISCV, but maybe we
can defer that wedding a bit longer.)
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
At that point, the config option should just go away entirely, no?
Ah, yes, and that makes the idea even more attractive, as we could remove several ifdefs.
I went and did the cursory check, it's not compatible with
XIP_KERNELso dropping the option entirely probably isn't a possibility :/
What I said is only now sinking in. We're now going to be disabling FPU support on XIP kernels with this patch.
Well, technically not this patch since it wouldn't have built without Jason's changes, but that doesn't seem like the right thing to do...
Re: [PATCH] riscv: require alternatives framework when selecting FPU support
Quoted 14 lines · On Wed, Mar 22, 2023 at 9:05?PM Conor Dooley <[email protected]> wrote:
Quoted 12 lines · Conor Dooley
Quoted 10 lines · Andrew Jones
Quoted 6 lines · Conor Dooley
Quoted 2 lines · Andrew Jones
(It's tempting to just select
RISCV_ALTERNATIVEfrom RISCV, but maybe we
can defer that wedding a bit longer.)At that point, the config option should just go away entirely, no?
Ah, yes, and that makes the idea even more attractive, as we could remove several ifdefs.
Quoted 7 lines
Quoted 2 lines
I went and did the cursory check, it's not compatible with
XIP_KERNELso dropping the option entirely probably isn't a possibility :/What I said is only now sinking in. We're now going to be disabling FPU support on XIP kernels with this patch.
Well, technically not this patch since it wouldn't have built without Jason's changes, but that doesn't seem like the right thing to do...
I suppose you could have riscv_has_extension_*() fall back to something that doesn't use alternatives on XIP kernels.
Re: [PATCH] riscv: require alternatives framework when selecting FPU support
Quoted 18 lines · Jason A. Donenfeld
Quoted 17 lines · Conor Dooley
Quoted 14 lines · Conor Dooley
Quoted 10 lines · Andrew Jones
Quoted 6 lines · Conor Dooley
Quoted 2 lines · Andrew Jones
(It's tempting to just select
RISCV_ALTERNATIVEfrom RISCV, but maybe we
can defer that wedding a bit longer.)At that point, the config option should just go away entirely, no?
Ah, yes, and that makes the idea even more attractive, as we could remove several ifdefs.
I went and did the cursory check, it's not compatible with
XIP_KERNELso dropping the option entirely probably isn't a possibility :/
Quoted 7 lines
Quoted 4 lines
What I said is only now sinking in. We're now going to be disabling FPU support on XIP kernels with this patch.
Well, technically not this patch since it wouldn't have built without Jason's changes, but that doesn't seem like the right thing to do...I suppose you could have riscv_has_extension_*() fall back to something that doesn't use alternatives on XIP kernels.
Yah, something like the below I guess? Probably overlooking something silly & it's lost the benefit of the static branch that it used to have, but with the infra that we have at the moment this seemed like the sanest thing to do?
This would requiring picking up your patch Jason, but with an "if !XIP_KERNEL" added to the select.
It's only had the lightest of build tests, but I can go make it a real patch if there's not something obviously amiss.
arch/riscv/include/asm/hwcap.h +30 -20
@@ -57,18 +57,31 @@ struct riscv_isa_ext_data {unsigned int isa_ext_id; }; +unsigned long riscv_isa_extension_base (const unsigned long *isa_bitmap); + +#define riscv_isa_extension_mask (ext) BIT_MASK(RISCV_ISA_EXT_##ext) + +bool __riscv_isa_extension_available (const unsigned long *isa_bitmap,int bit); +#define riscv_isa_extension_available (isa_bitmap, ext) \ + __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext) + static __always_inline boolriscv_has_extension_likely (const unsigned long ext) {compiletime_assert (ext <RISCV_ISA_EXT_MAX ,"ext must be < RISCV_ISA_EXT_MAX" ); -asm_volatile_goto ( - ALTERNATIVE("j %l[l_no]", "nop", 0, %[ext], 1) - : - : [ext] "i" (ext) - : - : l_no); +if (IS_ENABLED (CONFIG_RISCV_ALTERNATIVE )) { +asm_volatile_goto ( + ALTERNATIVE("j %l[l_no]", "nop", 0, %[ext], 1) + : + : [ext] "i" (ext) + : + : l_no); + }else { +if (!__riscv_isa_extension_available (NULL , ext)) + goto l_no; + }return true; l_no:
@@ -81,26 +94,23 @@ riscv_has_extension_unlikely(const unsigned long ext)compiletime_assert (ext <RISCV_ISA_EXT_MAX ,"ext must be < RISCV_ISA_EXT_MAX" ); -asm_volatile_goto ( - ALTERNATIVE("nop", "j %l[l_yes]", 0, %[ext], 1) - : - : [ext] "i" (ext) - : - : l_yes); +if (IS_ENABLED (CONFIG_RISCV_ALTERNATIVE )) { +asm_volatile_goto ( + ALTERNATIVE("nop", "j %l[l_yes]", 0, %[ext], 1) + : + : [ext] "i" (ext) + : + : l_yes); + }else { +if (__riscv_isa_extension_available (NULL , ext)) + goto l_yes; + }return false; l_yes:return true; } -unsigned long riscv_isa_extension_base (const unsigned long *isa_bitmap); - -#define riscv_isa_extension_mask (ext) BIT_MASK(RISCV_ISA_EXT_##ext) - -bool __riscv_isa_extension_available (const unsigned long *isa_bitmap,int bit); -#define riscv_isa_extension_available (isa_bitmap, ext) \ - __riscv_isa_extension_available(isa_bitmap, RISCV_ISA_EXT_##ext) -#endif #endif /* _ASM_RISCV_HWCAP_H */
Re: [PATCH] riscv: require alternatives framework when selecting FPU support
Quoted 2 lines · Conor Dooley
This would requiring picking up your patch Jason, but with an "if !XIP_KERNEL" added to the select.
So the risk of making this all work is that we wind up forgetting to add select alternatives if !xip to various places that need it (fpu, kvm, maybe others? future others?), because it appears to work, thanks to the code in your patch.
But making it work is also probably a good thing, since we obviously want the fpu and maybe other things to work on xip kernels.
So maybe we should get rid of the CONFIG_RISCV_ALTERNATIVES knob entirely, making it "always enabled", and then conditonalize the alternatives code to BUILD_BUG_ON when called with CONFIG_XIP_KERNEL=y. Then, this build bug will get hit immediately by riscv_has_extension_*(), which will then require your patch, which can run in a if (IS_ENABLED(XIP_KERNEL)) block or similar.
The result of that will be:
- !xip kernels properly use the fast riscv_has_extension_*() code and any alternatives code needed, since it's always selected.
- xip kernels get a BUILD_BUG_ON if they use any alternatives-based code that doesn't have a xip fallback yet.
What do you think of that approach?
A "lighter weight" version of that approach would be to just remove all of the select RISCV_ALTERNATIVES lines, and instead make RISCV_ALTERNATIVES specify default !XIP_KERNEL. That would more or less amount to the above too, though with weirder error cases.
Jason
Re: [PATCH] riscv: require alternatives framework when selecting FPU support
Hey Jason,
I read this mail before I left work today & had a think about it on the bike home, and had a whole response thought out, got distracted and forgot it all.. Hopefully I've remembered everything I had to say!
Quoted 11 lines · Jason A. Donenfeld
Quoted 2 lines · Conor Dooley
This would requiring picking up your patch Jason, but with an "if !XIP_KERNEL" added to the select.
So the risk of making this all work is that we wind up forgetting to add
select alternatives if !xipto various places that need it (fpu, kvm, maybe others? future others?), because it appears to work, thanks to the code in your patch.But making it work is also probably a good thing, since we obviously want the fpu and maybe other things to work on xip kernels.
I'm not super pushed about the "maybe other things", since the "maybe other things" that are in my head (errata and recently added extensions) have never worked on xip kernels, and losing them isn't a regression. Since XIP_KERNEL is deemed to be a "NONPORTABLE" option, we wouldn't need alternatives to enable it for them, but changes would be required for that to make the alternatives collapse to a build time thing. Can deal with that iff someone actually does come along wanting it.
We do need to fix this so that situations like the one you hit can't happen, while not regressing the level of support for xip, so some level of "making it work" is needed, but I do agree that it needs to be done in a less footgun way.
Quoted 7 lines
So maybe we should get rid of the
CONFIG_RISCV_ALTERNATIVESknob entirely, making it "always enabled", and then conditonalize the alternatives code toBUILD_BUG_ONwhen called withCONFIG_XIP_KERNEL=y. Then, this build bug will get hit immediately by riscv_has_extension_*(), which will then require your patch, which can run in aif (IS_ENABLED(XIP_KERNEL))block or similar.
Quoted 7 lines
The result of that will be:
- !xip kernels properly use the fast riscv_has_extension_*() code and any alternatives code needed, since it's always selected.
- xip kernels get aBUILD_BUG_ONif they use any alternatives-based code that doesn't have a xip fallback yet.What do you think of that approach?
Initially I thought "great, lets always enable the alternatives framework" but I don't think we can do that.
For the has_extension() stuff a fallback is fine, but I don't think that applies to using alternatives for either errata or enabling extensions at runtime.
I just don't really want to go through and modify the alternative macros so that they're evaluated at build time for xip unless that is absolutely required down the line. (I'd rather not even do it at all.)
Most of the things that are currently selecting RISCV_ALTERNATIVE do so to patch in support for extensions or enable errata, and I don't think we should expose those config options if the alternatives that they rely on cannot be used. I think that means something like...
Quoted 4 lines
A "lighter weight" version of that approach would be to just remove all of the
select RISCV_ALTERNATIVESlines, and instead makeRISCV_ALTERNATIVESspecifydefault !XIP_KERNEL. That would more or less amount to the above too, though with weirder error cases.
...adding a "select RISCV_ALTERNATIVE if !XIP_KERNEL" to the CONFIG_RISCV entry, and similarly to what you suggest here, swapping all of the instances of "select RISCV_ALTERNATIVE" for "depends on RISCV_ALTERNATIVE". That does still mean we can drop all of the "depends on !XIP_KERNEL" that are littered around the place whereever we are using alternatives & should only get the slow path for extension checking for xip kernels.
That'd handle the issue that you pointed out, where if the select is missing, my suggested change makes it appear to work if alternatives are not enabled too.
The BUILD_BUG_ON idea is good too, probably not fixes material, but might be worth having to prevent alternatives somehow being used when XIP_KERNEL is set.
I'll try to whip something up tomorrow...
Thanks Jason,
Conor.
PATCH v5 7/13 riscv: module: move find_section to module.h
In reply to Jisheng ZhangMove find_section() to module.h so that the implementation can be shared by the alternatives code. This will allow us to use alternatives in the vdso.
--- arch/riscv/include/asm/module.h | 16 ++++++++++++++++ arch/riscv/kernel/module.c | 15 --------------- 2 files changed, 16 insertions(+), 15 deletions(-)
arch/riscv/include/asm/module.h +16 -0
@@ -5,6 +5,7 @@#define _ASM_RISCV_MODULE_H#include <asm-generic/module.h> +#include <linux/elf.h> struct module ;unsigned long module_emit_got_entry (struct module *mod,unsigned long val);
@@ -111,4 +112,19 @@ static inline struct plt_entry *get_plt_entry(unsigned long val,#endif /* CONFIG_MODULE_SECTIONS */ +static inline const Elf_Shdr *find_section (const Elf_Ehdr *hdr, +const Elf_Shdr *sechdrs, +const char *name) +{ +const Elf_Shdr *s, *se; +const char *secstrs = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset; + +for (s = sechdrs, se = sechdrs + hdr->e_shnum; s < se; s++) { +if (strcmp (name, secstrs + s->sh_name) ==0 ) +return s; + } + +return NULL ; +} +#endif /* _ASM_RISCV_MODULE_H */
arch/riscv/kernel/module.c +0 -15
@@ -429,21 +429,6 @@ void *module_alloc(unsigned long size) }#endif -static const Elf_Shdr *find_section (const Elf_Ehdr *hdr, -const Elf_Shdr *sechdrs, -const char *name) -{ -const Elf_Shdr *s, *se; -const char *secstrs = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset; - -for (s = sechdrs, se = sechdrs + hdr->e_shnum; s < se; s++) { -if (strcmp (name, secstrs + s->sh_name) ==0 ) -return s; - } - -return NULL ; -} -int module_finalize (const Elf_Ehdr *hdr,const Elf_Shdr *sechdrs,struct module *me)
Signature · 1 line
2.38.1
PATCH v5 8/13 riscv: module: Add ADD16 and SUB16 rela types
In reply to Jisheng ZhangFrom: Andrew Jones <[email protected]>
To prepare for 16-bit relocation types to be emitted in alternatives add support for ADD16 and SUB16.
--- arch/riscv/kernel/module.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
arch/riscv/kernel/module.c +16 -0
@@ -268,6 +268,13 @@ static int apply_r_riscv_align_rela(struct module *me, u32 *location,return -EINVAL ; } +static int apply_r_riscv_add16_rela (struct module *me,u32 *location, +Elf_Addr v) +{ + *(u16 *)location += (u16 )v; +return 0 ; +} +static int apply_r_riscv_add32_rela (struct module *me,u32 *location,Elf_Addr v) {
@@ -282,6 +289,13 @@ static int apply_r_riscv_add64_rela(struct module *me, u32 *location,return 0 ; } +static int apply_r_riscv_sub16_rela (struct module *me,u32 *location, +Elf_Addr v) +{ + *(u16 *)location -= (u16 )v; +return 0 ; +} +static int apply_r_riscv_sub32_rela (struct module *me,u32 *location,Elf_Addr v) {
@@ -315,8 +329,10 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location, [R_RISCV_CALL] = apply_r_riscv_call_rela, [R_RISCV_RELAX] = apply_r_riscv_relax_rela, [R_RISCV_ALIGN] = apply_r_riscv_align_rela, + [R_RISCV_ADD16] = apply_r_riscv_add16_rela, [R_RISCV_ADD32] = apply_r_riscv_add32_rela, [R_RISCV_ADD64] = apply_r_riscv_add64_rela, + [R_RISCV_SUB16] = apply_r_riscv_sub16_rela, [R_RISCV_SUB32] = apply_r_riscv_sub32_rela, [R_RISCV_SUB64] = apply_r_riscv_sub64_rela, };
Signature · 1 line
2.38.1
PATCH v5 9/13 riscv: switch to relative alternative entries
In reply to Jisheng ZhangInstead of using absolute addresses for both the old instrucions and the alternative instructions, use offsets relative to the alt_entry values. So this not only cuts the size of the alternative entry, but also meets the prerequisite for patching alternatives in the vDSO, since absolute alternative entries are subject to dynamic relocation, which is incompatible with the vDSO building.
--- arch/riscv/errata/sifive/errata.c | 3 ++- arch/riscv/errata/thead/errata.c | 11 ++++++++--- arch/riscv/include/asm/alternative-macros.h | 20 ++++++++++---------- arch/riscv/include/asm/alternative.h | 17 +++++++++++------ arch/riscv/kernel/cpufeature.c | 8 +++++--- 5 files changed, 36 insertions(+), 23 deletions(-)
arch/riscv/errata/sifive/errata.c +2 -1
@@ -107,7 +107,8 @@ void __init_or_module sifive_errata_patch_func(struct alt_entry *begin, tmp = (1U << alt->errata_id);if (cpu_req_errata & tmp) { -patch_text_nosync (alt->old_ptr, alt->alt_ptr, alt->alt_len); +patch_text_nosync (ALT_OLD_PTR (alt),ALT_ALT_PTR (alt), + alt->alt_len); cpu_apply_errata |= tmp; } }
arch/riscv/errata/thead/errata.c +8 -3
@@ -87,6 +87,7 @@ void __init_or_module thead_errata_patch_func(struct alt_entry *begin, struct alstruct alt_entry *alt;u32 cpu_req_errata =thead_errata_probe (stage, archid, impid);u32 tmp; +void *oldptr, *altptr;for (alt = begin; alt < end; alt++) {if (alt->vendor_id !=THEAD_VENDOR_ID )
@@ -96,12 +97,16 @@ void __init_or_module thead_errata_patch_func(struct alt_entry *begin, struct al tmp = (1U << alt->errata_id);if (cpu_req_errata & tmp) { + oldptr =ALT_OLD_PTR (alt); + altptr =ALT_ALT_PTR (alt); +/* On vm-alternatives, the mmu isn't running yet */ if (stage ==RISCV_ALTERNATIVES_EARLY_BOOT ) -memcpy ((void *)__pa_symbol (alt->old_ptr), - (void *)__pa_symbol (alt->alt_ptr), alt->alt_len); +memcpy ((void *)__pa_symbol (oldptr), + (void *)__pa_symbol (altptr), + alt->alt_len);else -patch_text_nosync (alt->old_ptr, alt->alt_ptr, alt->alt_len); +patch_text_nosync (oldptr, altptr, alt->alt_len); } }
arch/riscv/include/asm/alternative-macros.h +10 -10
@@ -7,11 +7,11 @@#ifdef __ASSEMBLY__ .macro ALT_ENTRY oldptr newptr vendor_id errata_id new_len - RISCV_PTR \oldptr - RISCV_PTR \newptr - REG_ASM \vendor_id - REG_ASM \new_len - .word \errata_id + .4byte \oldptr - . + .4byte \newptr - . + .2byte \vendor_id + .2byte \new_len + .4byte \errata_id .endm .macro ALT_NEW_CONTENT vendor_id, errata_id, enable = 1, new_c : vararg
@@ -59,11 +59,11 @@#include <linux/stringify.h> #define ALT_ENTRY (oldptr, newptr, vendor_id, errata_id, newlen) \ - RISCV_PTR " " oldptr "\n" \ - RISCV_PTR " " newptr "\n" \ - REG_ASM " " vendor_id "\n" \ - REG_ASM " " newlen "\n" \ - ".word " errata_id "\n" + ".4byte ((" oldptr ") - .) \n" \ + ".4byte ((" newptr ") - .) \n" \ + ".2byte " vendor_id "\n" \ + ".2byte " newlen "\n" \ + ".4byte " errata_id "\n"#define ALT_NEW_CONTENT (vendor_id, errata_id, enable, new_c) \ ".if " __stringify(enable) " == 1\n" \
arch/riscv/include/asm/alternative.h +11 -6
@@ -23,6 +23,11 @@#define RISCV_ALTERNATIVES_MODULE 1/* alternatives applied during module-init */ #define RISCV_ALTERNATIVES_EARLY_BOOT 2/* alternatives applied before mmu start */ +/* add the relative offset to the address of the offset to get the absolute address */ +#define __ALT_PTR (a, f) ((void *)&(a)->f + (a)->f) +#define ALT_OLD_PTR (a) __ALT_PTR(a, old_offset) +#define ALT_ALT_PTR (a) __ALT_PTR(a, alt_offset) +void __init apply_boot_alternatives (void );void __init apply_early_boot_alternatives (void );void apply_module_alternatives (void *start,size_t length);
@@ -31,12 +36,12 @@ void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len, int patch_offset);struct alt_entry { -void *old_ptr;/* address of original instruciton or data */ -void *alt_ptr;/* address of replacement instruction or data */ -unsigned long vendor_id;/* cpu vendor id */ -unsigned long alt_len;/* The replacement size */ -unsigned int errata_id;/* The errata id */ -} __packed; +s32 old_offset;/* offset relative to original instruction or data */ +s32 alt_offset;/* offset relative to replacement instruction or data */ +u16 vendor_id;/* cpu vendor id */ +u16 alt_len;/* The replacement size */ +u32 errata_id;/* The errata id */ +};struct errata_checkfunc_id {unsigned long vendor_id;
arch/riscv/kernel/cpufeature.c +5 -3
@@ -281,6 +281,7 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin, unsigned int stage) {struct alt_entry *alt; +void *oldptr, *altptr;if (stage ==RISCV_ALTERNATIVES_EARLY_BOOT )return ;
@@ -297,9 +298,10 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,if (!__riscv_isa_extension_available (NULL , alt->errata_id))continue ; -patch_text_nosync (alt->old_ptr, alt->alt_ptr, alt->alt_len); -riscv_alternative_fix_offsets (alt->old_ptr, alt->alt_len, - alt->old_ptr - alt->alt_ptr); + oldptr =ALT_OLD_PTR (alt); + altptr =ALT_ALT_PTR (alt); +patch_text_nosync (oldptr, altptr, alt->alt_len); + riscv_alternative_fix_offsets(oldptr, alt->alt_len, oldptr - altptr); } }#endif
Signature · 1 line
2.38.1
PATCH v5 10/13 riscv: alternative: patch alternatives in the vDSO
In reply to Jisheng ZhangMake it possible to use alternatives in the vDSO, so that better implementations can be used if possible.
--- arch/riscv/include/asm/vdso.h | 4 ++++ arch/riscv/kernel/alternative.c | 29 +++++++++++++++++++++++++++++ arch/riscv/kernel/vdso.c | 5 ----- arch/riscv/kernel/vdso/vdso.lds.S | 7 +++++++ 4 files changed, 40 insertions(+), 5 deletions(-)
arch/riscv/include/asm/vdso.h +4 -0
@@ -28,8 +28,12 @@#define COMPAT_VDSO_SYMBOL (base, name) \ (void __user *)((unsigned long)(base) + compat__vdso_##name##_offset) +extern char compat_vdso_start[], compat_vdso_end[]; +#endif /* CONFIG_COMPAT */ +extern char vdso_start[], vdso_end[]; +#endif /* !__ASSEMBLY__ */ #endif /* CONFIG_MMU */
arch/riscv/kernel/alternative.c +29 -0
@@ -11,7 +11,9 @@#include <linux/cpu.h> #include <linux/uaccess.h> #include <asm/alternative.h> +#include <asm/module.h> #include <asm/sections.h> +#include <asm/vdso.h> #include <asm/vendorid_list.h> #include <asm/sbi.h> #include <asm/csr.h>
@@ -160,6 +162,31 @@ static void __init_or_module _apply_alternatives(struct alt_entry *begin, stage); } +#ifdef CONFIG_MMU +static void __init apply_vdso_alternatives (void ) +{ +const Elf_Ehdr *hdr; +const Elf_Shdr *shdr; +const Elf_Shdr *alt; +struct alt_entry *begin, *end; + + hdr = (Elf_Ehdr *)vdso_start; + shdr = (void *)hdr + hdr->e_shoff; + alt =find_section (hdr, shdr,".alternative" ); +if (!alt) +return ; + + begin = (void *)hdr + alt->sh_offset, + end = (void *)hdr + alt->sh_offset + alt->sh_size, + +_apply_alternatives ((struct alt_entry *)begin, + (struct alt_entry *)end, +RISCV_ALTERNATIVES_BOOT ); +} +#else +static void __init apply_vdso_alternatives (void ) { } +#endif +void __init apply_boot_alternatives (void ) {/* If called on non-boot cpu things could go wrong */
@@ -168,6 +195,8 @@ void __init apply_boot_alternatives(void)_apply_alternatives ((struct alt_entry *)__alt_start, (struct alt_entry *)__alt_end,RISCV_ALTERNATIVES_BOOT ); + +apply_vdso_alternatives (); }/*
arch/riscv/kernel/vdso.c +0 -5
@@ -22,11 +22,6 @@ struct vdso_data { };#endif -extern char vdso_start[], vdso_end[]; -#ifdef CONFIG_COMPAT -extern char compat_vdso_start[], compat_vdso_end[]; -#endif -enum vvar_pages {VVAR_DATA_PAGE_OFFSET ,VVAR_TIMENS_PAGE_OFFSET ,
arch/riscv/kernel/vdso/vdso.lds.S +7 -0
@@ -40,6 +40,13 @@ SECTIONS . = 0x800; .text : { *(.text .text.*) } :text + . = ALIGN(4); + .alternative : { + __alt_start = .; + *(.alternative) + __alt_end = .; + } + .data : { *(.got.plt) *(.got) *(.data .data.* .gnu.linkonce.d.*)
Signature · 1 line
2.38.1
PATCH v5 11/13 riscv: cpu_relax: switch to riscv_has_extension_likely()
In reply to Jisheng ZhangSwitch cpu_relax() from static branch to the new helper riscv_has_extension_likely()
--- arch/riscv/include/asm/vdso/processor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/riscv/include/asm/vdso/processor.h +1 -1
@@ -10,7 +10,7 @@static inline void cpu_relax (void ) { -if (!static_branch_likely (&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE ])) { +if (!riscv_has_extension_likely (RISCV_ISA_EXT_ZIHINTPAUSE )) {#ifdef __riscv_muldivint dummy;/* In lieu of a halt instruction, induce a long-latency stall. */
Signature · 1 line
2.38.1
PATCH v5 12/13 riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely()
In reply to Jisheng ZhangFrom: Andrew Jones <[email protected]>
Switch has_svinval() from static branch to the new helper riscv_has_extension_unlikely().
--- arch/riscv/kvm/tlb.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)
arch/riscv/kvm/tlb.c +1 -2
@@ -15,8 +15,7 @@#include <asm/hwcap.h> #include <asm/insn-def.h> -#define has_svinval () \ - static_branch_unlikely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_SVINVAL]) +#define has_svinval () riscv_has_extension_unlikely(RISCV_ISA_EXT_SVINVAL) void kvm_riscv_local_hfence_gvma_vmid_gpa(unsigned long vmid, gpa_t gpa, gpa_t gpsz,
Signature · 1 line
2.38.1
PATCH v5 13/13 riscv: remove riscv_isa_ext_keys[] array and related usage
In reply to Jisheng ZhangAll users have switched to riscv_has_extension_*, remove unused definitions, vars and related setting code.
--- arch/riscv/include/asm/hwcap.h | 32 -------------------------------- arch/riscv/kernel/cpufeature.c | 9 --------- 2 files changed, 41 deletions(-)
arch/riscv/include/asm/hwcap.h +0 -32
@@ -61,20 +61,6 @@ enum {extern unsigned long elf_hwcap; - -/* -* This enum represents the logical ID for each RISC-V ISA extension static -* keys. We can use static key to optimize code path if some ISA extensions -* are available. -* Entries are sorted alphabetically. -*/ -enum riscv_isa_ext_key { -RISCV_ISA_EXT_KEY_FPU ,/* For 'F' and 'D' */ -RISCV_ISA_EXT_KEY_SVINVAL , -RISCV_ISA_EXT_KEY_ZIHINTPAUSE , -RISCV_ISA_EXT_KEY_MAX , -}; -struct riscv_isa_ext_data {/* Name of the extension displayed to userspace via /proc/cpuinfo */ char uprop[RISCV_ISA_EXT_NAME_LEN_MAX ];
@@ -82,24 +68,6 @@ struct riscv_isa_ext_data {unsigned int isa_ext_id; }; -extern struct static_key_false riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_MAX ]; - -static __always_inline int riscv_isa_ext2key(int num) -{ -switch (num) { -case RISCV_ISA_EXT_f: -return RISCV_ISA_EXT_KEY_FPU ; -case RISCV_ISA_EXT_d: -return RISCV_ISA_EXT_KEY_FPU ; -case RISCV_ISA_EXT_SVINVAL : -return RISCV_ISA_EXT_KEY_SVINVAL ; -case RISCV_ISA_EXT_ZIHINTPAUSE : -return RISCV_ISA_EXT_KEY_ZIHINTPAUSE ; -default : -return -EINVAL ; - } -} - static __always_inline boolriscv_has_extension_likely (const unsigned long ext) {
arch/riscv/kernel/cpufeature.c +0 -9
@@ -29,9 +29,6 @@ unsigned long elf_hwcap __read_mostly;/* Host ISA bitmap */ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly; -DEFINE_STATIC_KEY_ARRAY_FALSE (riscv_isa_ext_keys, RISCV_ISA_EXT_KEY_MAX); -EXPORT_SYMBOL (riscv_isa_ext_keys); -/** * riscv_isa_extension_base() - Get base extension word *
@@ -267,12 +264,6 @@ void __init riscv_fill_hwcap(void)if (elf_hwcap &BIT_MASK (i)) print_str[j++] = (char )('a' + i);pr_info ("riscv: ELF capabilities %s\n" , print_str); - -for_each_set_bit (i, riscv_isa,RISCV_ISA_EXT_MAX ) { - j =riscv_isa_ext2key (i); -if (j >=0 ) -static_branch_enable (&riscv_isa_ext_keys[j]); - } }#ifdef CONFIG_RISCV_ALTERNATIVE
Signature · 1 line
2.38.1
Palmer Dabbelt
AppliedQuoted 9 lines · Jisheng Zhang
Generally, riscv ISA extensions are fixed for any specific hardware platform, so a hart's features won't change after booting, this chacteristic makes it straightforward to use a static branch to check a specific ISA extension is supported or not to optimize performance.
However, some ISA extensions such as SVPBMT and ZICBOM are handled via. the alternative sequences.
[...]
Applied, thanks!
[01/13] riscv: move riscv_noncoherent_supported() out of ZICBOM probe
https://git.kernel.org/palmer/c/abcc445acdbe
[02/13] riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier
https://git.kernel.org/palmer/c/191b27c7c0e8 [03/13] riscv: hwcap: make ISA extension ids can be used in asm https://git.kernel.org/palmer/c/d8a3d8a75206 [04/13] riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions https://git.kernel.org/palmer/c/4bf8860760d9 [05/13] riscv: introduce riscv_has_extension_[un]likely() https://git.kernel.org/palmer/c/bdda5d554e43 [06/13] riscv: fpu: switch has_fpu() to riscv_has_extension_likely() https://git.kernel.org/palmer/c/702e64550b12 [07/13] riscv: module: move find_section to module.h https://git.kernel.org/palmer/c/e0c267e03b0c [08/13] riscv: module: Add ADD16 and SUB16 rela types https://git.kernel.org/palmer/c/1bc400ffb52b [09/13] riscv: switch to relative alternative entries https://git.kernel.org/palmer/c/8d23e94a4433 [10/13] riscv: alternative: patch alternatives in the vDSO https://git.kernel.org/palmer/c/cabfd146b371 [11/13] riscv: cpu_relax: switch to riscv_has_extension_likely() https://git.kernel.org/palmer/c/95bc69a47be2 [12/13] riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely() https://git.kernel.org/palmer/c/e8ad17d2b5f3 [13/13] riscv: remove riscv_isa_ext_keys[] array and related usage https://git.kernel.org/palmer/c/03966594e117
Best regards,
Signature · 1 line
Palmer Dabbelt <[email protected]>
This series was applied to riscv/linux.git (for-next) by Palmer Dabbelt <[email protected]>:
Hello:
This series was applied to riscv/linux.git (for-next) by Palmer Dabbelt <[email protected]>:
Quoted 9 lines · On Sun, 29 Jan 2023 01:28:43 +0800 you wrote:
Generally, riscv ISA extensions are fixed for any specific hardware platform, so a hart's features won't change after booting, this chacteristic makes it straightforward to use a static branch to check a specific ISA extension is supported or not to optimize performance.
However, some ISA extensions such as SVPBMT and ZICBOM are handled via. the alternative sequences.
[...]
Here is the summary with links:
- [v5,01/13] riscv: move riscv_noncoherent_supported() out of ZICBOM probe
https://git.kernel.org/riscv/c/abcc445acdbe - [v5,02/13] riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier https://git.kernel.org/riscv/c/191b27c7c0e8 - [v5,03/13] riscv: hwcap: make ISA extension ids can be used in asm https://git.kernel.org/riscv/c/d8a3d8a75206 - [v5,04/13] riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions https://git.kernel.org/riscv/c/4bf8860760d9 - [v5,05/13] riscv: introduce riscv_has_extension_[un]likely() https://git.kernel.org/riscv/c/bdda5d554e43 - [v5,06/13] riscv: fpu: switch has_fpu() to riscv_has_extension_likely() https://git.kernel.org/riscv/c/702e64550b12 - [v5,07/13] riscv: module: move find_section to module.h https://git.kernel.org/riscv/c/e0c267e03b0c - [v5,08/13] riscv: module: Add ADD16 and SUB16 rela types https://git.kernel.org/riscv/c/1bc400ffb52b - [v5,09/13] riscv: switch to relative alternative entries https://git.kernel.org/riscv/c/8d23e94a4433 - [v5,10/13] riscv: alternative: patch alternatives in the vDSO https://git.kernel.org/riscv/c/cabfd146b371 - [v5,11/13] riscv: cpu_relax: switch to riscv_has_extension_likely() https://git.kernel.org/riscv/c/95bc69a47be2 - [v5,12/13] riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely() https://git.kernel.org/riscv/c/e8ad17d2b5f3 - [v5,13/13] riscv: remove riscv_isa_ext_keys[] array and related usage https://git.kernel.org/riscv/c/03966594e117
You are awesome, thank you!
Signature · 2 lines
Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
Hi,
Quoted 22 lines · Jisheng Zhang
Generally, riscv ISA extensions are fixed for any specific hardware platform, so a hart's features won't change after booting, this chacteristic makes it straightforward to use a static branch to check a specific ISA extension is supported or not to optimize performance.
However, some ISA extensions such as SVPBMT and ZICBOM are handled via. the alternative sequences.
Basically, for ease of maintenance, we prefer to use static branches in C code, but recently, Samuel found that the static branch usage in
cpu_relax()breaks building with CONFIG_CC_OPTIMIZE_FOR_SIZE[1]. As Samuel pointed out, "Having a static branch incpu_relax()is problematic because that function is widely inlined, including in some quite complex functions like in the VDSO. A quick measurement shows this static branch is responsible by itself for around 40% of the jump table."Samuel's findings pointed out one of a few downsides of static branches usage in C code to handle ISA extensions detected at boot time:
static branch's metadata in the__jump_tablesection, which is not discarded after ISA extensions are finalized, wastes some space.
Quoted 6 lines
I want to try to solve the issue for all possible dynamic handling of ISA extensions at boot time. Inspired by Mark[2], this patch introduces riscv_has_extension_*() helpers, which work like static branches but are patched using alternatives, thus the metadata can be freed after patching.
This patch series results in boot failures when trying to boot the qemu sifive_u emulation. There are many log messages along the line of
[ 0.000000] WARNING: CPU: 0 PID: 0 at arch/riscv/kernel/patch.c:63 patch_insn_write+0x222/0x2f6 [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 PID: 0 Comm: swapper Tainted: G W 6.2.0-rc7-next-20230210 #1 [ 0.000000] Hardware name: SiFive HiFive Unleashed A00 (DT) [ 0.000000] epc : patch_insn_write+0x222/0x2f6 [ 0.000000] ra : patch_insn_write+0x21e/0x2f6 [ 0.000000] epc : ffffffff800068c6 ra : ffffffff800068c2 sp : ffffffff81803df0 [ 0.000000] gp : ffffffff81a1ab78 tp : ffffffff81814f80 t0 : ffffffffffffe000 [ 0.000000] t1 : 0000000000000001 t2 : 4c45203a76637369 s0 : ffffffff81803e40 [ 0.000000] s1 : 0000000000000004 a0 : 0000000000000000 a1 : ffffffffffffffff [ 0.000000] a2 : 0000000000000004 a3 : 0000000000000000 a4 : 0000000000000001 [ 0.000000] a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000052464e43 [ 0.000000] s2 : ffffffff80b4889c s3 : 000000000000082c s4 : ffffffff80b48828 [ 0.000000] s5 : 0000000000000828 s6 : ffffffff8131a0a0 s7 : 0000000000000fff [ 0.000000] s8 : 0000000008000200 s9 : ffffffff8131a520 s10: 0000000000000018 [ 0.000000] s11: 000000000000000b t3 : 0000000000000001 t4 : 000000000000000d [ 0.000000] t5 : ffffffffd8180000 t6 : ffffffff81803bc8 [ 0.000000] status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003 [ 0.000000] [<ffffffff800068c6>] patch_insn_write+0x222/0x2f6 [ 0.000000] [<ffffffff80006a36>] patch_text_nosync+0xc/0x2a [ 0.000000] [<ffffffff80003b86>] riscv_cpufeature_patch_func+0x52/0x98 [ 0.000000] [<ffffffff80003348>] _apply_alternatives+0x46/0x86 [ 0.000000] [<ffffffff80c02d36>] apply_boot_alternatives+0x3c/0xfa [ 0.000000] [<ffffffff80c03ad8>] setup_arch+0x584/0x5b8 [ 0.000000] [<ffffffff80c0075a>] start_kernel+0xa2/0x8f8 [ 0.000000] irq event stamp: 0 [ 0.000000] hardirqs last enabled at (0): [<0000000000000000>] 0x0 [ 0.000000] hardirqs last disabled at (0): [<0000000000000000>] 0x0 [ 0.000000] softirqs last enabled at (0): [<0000000000000000>] 0x0 [ 0.000000] softirqs last disabled at (0): [<0000000000000000>] 0x0 [ 0.000000] ---[ end trace 0000000000000000 ]--- [ 0.000000] ------------[ cut here ]------------
then qemu hangs until the session is aborted.
Similar messages are also seen with the "virt" emulation, but there the boot does not hang but fails to find a root device.
Guenter
---
bisect:
# bad: [6ba8a227fd19d19779005fb66ad7562608e1df83] Add linux-next specific files for 20230210
# good: [4ec5183ec48656cec489c49f989c508b68b518e3] Linux 6.2-rc7
git bisect start 'HEAD' 'v6.2-rc7'
# bad: [94613f0efc69ed41f9229ef5c294db3ec37145da] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git
git bisect bad 94613f0efc69ed41f9229ef5c294db3ec37145da
# bad: [8928ece68de4371dc6e1d3336d3029c1e18ae3b4] Merge branch 'for_next' of git://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git
git bisect bad 8928ece68de4371dc6e1d3336d3029c1e18ae3b4
# good: [78a9f460e33d103256f3abb38f02f4759710c7dc] soc: document merges
git bisect good 78a9f460e33d103256f3abb38f02f4759710c7dc
# good: [b35b2472ebafa29d0bbe79fbee1da6ef3c4ec619] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
git bisect good b35b2472ebafa29d0bbe79fbee1da6ef3c4ec619
# bad: [57a87a64b520c37dd49f5fde84d09a4adb391180] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git
git bisect bad 57a87a64b520c37dd49f5fde84d09a4adb391180
# good: [cfc8ba01cc84b24ec6eb293ec9fba893f7cd4581] Merge branch 'clk-next' of git://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
git bisect good cfc8ba01cc84b24ec6eb293ec9fba893f7cd4581
# good: [6acecfa485d3de955c35a18730c106ddf1e7600e] powerpc/kcsan: Add KCSAN Support
git bisect good 6acecfa485d3de955c35a18730c106ddf1e7600e
# good: [8a16dea453dbc3e834b162640028e505882cd11e] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git
git bisect good 8a16dea453dbc3e834b162640028e505882cd11e
# good: [6be1ff430dab9fc047762b10b2c9669399ea1f37] riscv: pgtable: Fixup comment for KERN_VIRT_SIZE
git bisect good 6be1ff430dab9fc047762b10b2c9669399ea1f37
# good: [e0c267e03b0c77c9ac79ac08eada41ba8eb1b95f] riscv: module: move find_section to module.h
git bisect good e0c267e03b0c77c9ac79ac08eada41ba8eb1b95f
# good: [e8ad17d2b5f38e595d597a3e2419d6d7cc727b17] riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely()
git bisect good e8ad17d2b5f38e595d597a3e2419d6d7cc727b17
# good: [75ab93a244a516d1d3c03c4e27d5d0deff76ebfb] Merge patch series "Zbb string optimizations"
git bisect good 75ab93a244a516d1d3c03c4e27d5d0deff76ebfb
# bad: [9daca9a5b9ac35361ce2d8d5ec10b19b7048d6cd] Merge patch series "riscv: improve boot time isa extensions handling" git bisect bad 9daca9a5b9ac35361ce2d8d5ec10b19b7048d6cd
# good: [03966594e1170303c037b0cded35c464a13a4a45] riscv: remove riscv_isa_ext_keys[] array and related usage
git bisect good 03966594e1170303c037b0cded35c464a13a4a45
# first bad commit: [9daca9a5b9ac35361ce2d8d5ec10b19b7048d6cd] Merge patch series "riscv: improve boot time isa extensions handling"
Hey Guenter,
Quoted 66 lines · Guenter Roeck
Quoted 22 lines · Jisheng Zhang
Generally, riscv ISA extensions are fixed for any specific hardware platform, so a hart's features won't change after booting, this chacteristic makes it straightforward to use a static branch to check a specific ISA extension is supported or not to optimize performance.
However, some ISA extensions such as SVPBMT and ZICBOM are handled via. the alternative sequences.
Basically, for ease of maintenance, we prefer to use static branches in C code, but recently, Samuel found that the static branch usage in
cpu_relax()breaks building with CONFIG_CC_OPTIMIZE_FOR_SIZE[1]. As Samuel pointed out, "Having a static branch incpu_relax()is problematic because that function is widely inlined, including in some quite complex functions like in the VDSO. A quick measurement shows this static branch is responsible by itself for around 40% of the jump table."Samuel's findings pointed out one of a few downsides of static branches usage in C code to handle ISA extensions detected at boot time:
static branch's metadata in the__jump_tablesection, which is not discarded after ISA extensions are finalized, wastes some space.Quoted 6 lines
I want to try to solve the issue for all possible dynamic handling of ISA extensions at boot time. Inspired by Mark[2], this patch introduces riscv_has_extension_*() helpers, which work like static branches but are patched using alternatives, thus the metadata can be freed after patching.
This patch series results in boot failures when trying to boot the qemu
sifive_uemulation. There are many log messages along the line of[ 0.000000] WARNING: CPU: 0 PID: 0 at arch/riscv/kernel/patch.c:63 patch_insn_write+0x222/0x2f6 [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 PID: 0 Comm: swapper Tainted: G W 6.2.0-rc7-next-20230210 #1 [ 0.000000] Hardware name: SiFive HiFive Unleashed A00 (DT) [ 0.000000] epc : patch_insn_write+0x222/0x2f6 [ 0.000000] ra : patch_insn_write+0x21e/0x2f6 [ 0.000000] epc : ffffffff800068c6 ra : ffffffff800068c2 sp : ffffffff81803df0 [ 0.000000] gp : ffffffff81a1ab78 tp : ffffffff81814f80 t0 : ffffffffffffe000 [ 0.000000] t1 : 0000000000000001 t2 : 4c45203a76637369 s0 : ffffffff81803e40 [ 0.000000] s1 : 0000000000000004 a0 : 0000000000000000 a1 : ffffffffffffffff [ 0.000000] a2 : 0000000000000004 a3 : 0000000000000000 a4 : 0000000000000001 [ 0.000000] a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000052464e43 [ 0.000000] s2 : ffffffff80b4889c s3 : 000000000000082c s4 : ffffffff80b48828 [ 0.000000] s5 : 0000000000000828 s6 : ffffffff8131a0a0 s7 : 0000000000000fff [ 0.000000] s8 : 0000000008000200 s9 : ffffffff8131a520 s10: 0000000000000018 [ 0.000000] s11: 000000000000000b t3 : 0000000000000001 t4 : 000000000000000d [ 0.000000] t5 : ffffffffd8180000 t6 : ffffffff81803bc8 [ 0.000000] status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003 [ 0.000000] [<ffffffff800068c6>] patch_insn_write+0x222/0x2f6 [ 0.000000] [<ffffffff80006a36>] patch_text_nosync+0xc/0x2a [ 0.000000] [<ffffffff80003b86>] riscv_cpufeature_patch_func+0x52/0x98 [ 0.000000] [<ffffffff80003348>] _apply_alternatives+0x46/0x86 [ 0.000000] [<ffffffff80c02d36>] apply_boot_alternatives+0x3c/0xfa [ 0.000000] [<ffffffff80c03ad8>] setup_arch+0x584/0x5b8 [ 0.000000] [<ffffffff80c0075a>] start_kernel+0xa2/0x8f8 [ 0.000000] irq event stamp: 0 [ 0.000000] hardirqs last enabled at (0): [<0000000000000000>] 0x0 [ 0.000000] hardirqs last disabled at (0): [<0000000000000000>] 0x0 [ 0.000000] softirqs last enabled at (0): [<0000000000000000>] 0x0 [ 0.000000] softirqs last disabled at (0): [<0000000000000000>] 0x0 [ 0.000000] ---[ end trace 0000000000000000 ]--- [ 0.000000] ------------[ cut here ]------------
Quoted 1 line
then qemu hangs until the session is aborted.
I have come across the same issue on PolarFire SoC while looking at Samuel's fixes for Zbb & the D1:
https://lore.kernel.org/all/[email protected]/
Boot over NFS still works, which I think points to a hole in how my CI is operating - it assumed the completed boot + bootrr being happy to mean that there was nothing wrong!
On the VisionFive 2 & D1 Nezha I don't see these splats though. It appears to be config related as the config I build for Icicle sees these splats in QEMU but the D1 config does not.
I'll go do some digging!
Quoted 43 lines
Similar messages are also seen with the "virt" emulation, but there the boot does not hang but fails to find a root device.
Guenter
---
bisect:# bad: [
6ba8a227fd19d19779005fb66ad7562608e1df83] Add linux-next specific files for 20230210
# good: [4ec5183ec48656cec489c49f989c508b68b518e3] Linux 6.2-rc7
git bisect start 'HEAD' 'v6.2-rc7'
# bad: [94613f0efc69ed41f9229ef5c294db3ec37145da] Merge branch 'master' ofgit://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git
git bisect bad94613f0efc69ed41f9229ef5c294db3ec37145da
# bad: [8928ece68de4371dc6e1d3336d3029c1e18ae3b4] Merge branch 'for_next' ofgit://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git
git bisect bad8928ece68de4371dc6e1d3336d3029c1e18ae3b4
# good: [78a9f460e33d103256f3abb38f02f4759710c7dc] soc: document merges
git bisect good78a9f460e33d103256f3abb38f02f4759710c7dc
# good: [b35b2472ebafa29d0bbe79fbee1da6ef3c4ec619] Merge branch 'for-next' ofgit://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
git bisect goodb35b2472ebafa29d0bbe79fbee1da6ef3c4ec619
# bad: [57a87a64b520c37dd49f5fde84d09a4adb391180] Merge branch 'for-next' ofgit://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git
git bisect bad57a87a64b520c37dd49f5fde84d09a4adb391180
# good: [cfc8ba01cc84b24ec6eb293ec9fba893f7cd4581] Merge branch 'clk-next' ofgit://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
git bisect goodcfc8ba01cc84b24ec6eb293ec9fba893f7cd4581
# good: [6acecfa485d3de955c35a18730c106ddf1e7600e] powerpc/kcsan: Add KCSAN Support
git bisect good6acecfa485d3de955c35a18730c106ddf1e7600e
# good: [8a16dea453dbc3e834b162640028e505882cd11e] Merge branch 'next' ofgit://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git
git bisect good8a16dea453dbc3e834b162640028e505882cd11e
# good: [6be1ff430dab9fc047762b10b2c9669399ea1f37] riscv: pgtable: Fixup comment forKERN_VIRT_SIZE
git bisect good6be1ff430dab9fc047762b10b2c9669399ea1f37
# good: [e0c267e03b0c77c9ac79ac08eada41ba8eb1b95f] riscv: module: movefind_sectiontomodule.h
git bisect goode0c267e03b0c77c9ac79ac08eada41ba8eb1b95f
# good: [e8ad17d2b5f38e595d597a3e2419d6d7cc727b17] riscv: KVM: Switchhas_svinval()toriscv_has_extension_unlikely()
git bisect goode8ad17d2b5f38e595d597a3e2419d6d7cc727b17
# good: [75ab93a244a516d1d3c03c4e27d5d0deff76ebfb] Merge patch series "Zbb string optimizations"
git bisect good75ab93a244a516d1d3c03c4e27d5d0deff76ebfb
# bad: [9daca9a5b9ac35361ce2d8d5ec10b19b7048d6cd] Merge patch series "riscv: improve boot time isa extensions handling" git bisect bad9daca9a5b9ac35361ce2d8d5ec10b19b7048d6cd
# good: [03966594e1170303c037b0cded35c464a13a4a45] riscv: remove riscv_isa_ext_keys[] array and related usage
git bisect good03966594e1170303c037b0cded35c464a13a4a45# first bad commit: [9daca9a5b9ac35361ce2d8d5ec10b19b7048d6cd] Merge patch series "riscv: improve boot time isa extensions handling"
Quoted 67 lines · Conor Dooley
Quoted 66 lines · Guenter Roeck
Quoted 22 lines · Jisheng Zhang
Generally, riscv ISA extensions are fixed for any specific hardware platform, so a hart's features won't change after booting, this chacteristic makes it straightforward to use a static branch to check a specific ISA extension is supported or not to optimize performance.
However, some ISA extensions such as SVPBMT and ZICBOM are handled via. the alternative sequences.
Basically, for ease of maintenance, we prefer to use static branches in C code, but recently, Samuel found that the static branch usage in
cpu_relax()breaks building with CONFIG_CC_OPTIMIZE_FOR_SIZE[1]. As Samuel pointed out, "Having a static branch incpu_relax()is problematic because that function is widely inlined, including in some quite complex functions like in the VDSO. A quick measurement shows this static branch is responsible by itself for around 40% of the jump table."Samuel's findings pointed out one of a few downsides of static branches usage in C code to handle ISA extensions detected at boot time:
static branch's metadata in the__jump_tablesection, which is not discarded after ISA extensions are finalized, wastes some space.Quoted 6 lines
I want to try to solve the issue for all possible dynamic handling of ISA extensions at boot time. Inspired by Mark[2], this patch introduces riscv_has_extension_*() helpers, which work like static branches but are patched using alternatives, thus the metadata can be freed after patching.
This patch series results in boot failures when trying to boot the qemu
sifive_uemulation. There are many log messages along the line of[ 0.000000] WARNING: CPU: 0 PID: 0 at arch/riscv/kernel/patch.c:63 patch_insn_write+0x222/0x2f6 [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 PID: 0 Comm: swapper Tainted: G W 6.2.0-rc7-next-20230210 #1 [ 0.000000] Hardware name: SiFive HiFive Unleashed A00 (DT) [ 0.000000] epc : patch_insn_write+0x222/0x2f6 [ 0.000000] ra : patch_insn_write+0x21e/0x2f6 [ 0.000000] epc : ffffffff800068c6 ra : ffffffff800068c2 sp : ffffffff81803df0 [ 0.000000] gp : ffffffff81a1ab78 tp : ffffffff81814f80 t0 : ffffffffffffe000 [ 0.000000] t1 : 0000000000000001 t2 : 4c45203a76637369 s0 : ffffffff81803e40 [ 0.000000] s1 : 0000000000000004 a0 : 0000000000000000 a1 : ffffffffffffffff [ 0.000000] a2 : 0000000000000004 a3 : 0000000000000000 a4 : 0000000000000001 [ 0.000000] a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000052464e43 [ 0.000000] s2 : ffffffff80b4889c s3 : 000000000000082c s4 : ffffffff80b48828 [ 0.000000] s5 : 0000000000000828 s6 : ffffffff8131a0a0 s7 : 0000000000000fff [ 0.000000] s8 : 0000000008000200 s9 : ffffffff8131a520 s10: 0000000000000018 [ 0.000000] s11: 000000000000000b t3 : 0000000000000001 t4 : 000000000000000d [ 0.000000] t5 : ffffffffd8180000 t6 : ffffffff81803bc8 [ 0.000000] status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003 [ 0.000000] [<ffffffff800068c6>] patch_insn_write+0x222/0x2f6 [ 0.000000] [<ffffffff80006a36>] patch_text_nosync+0xc/0x2a [ 0.000000] [<ffffffff80003b86>] riscv_cpufeature_patch_func+0x52/0x98 [ 0.000000] [<ffffffff80003348>] _apply_alternatives+0x46/0x86 [ 0.000000] [<ffffffff80c02d36>] apply_boot_alternatives+0x3c/0xfa [ 0.000000] [<ffffffff80c03ad8>] setup_arch+0x584/0x5b8 [ 0.000000] [<ffffffff80c0075a>] start_kernel+0xa2/0x8f8 [ 0.000000] irq event stamp: 0 [ 0.000000] hardirqs last enabled at (0): [<0000000000000000>] 0x0 [ 0.000000] hardirqs last disabled at (0): [<0000000000000000>] 0x0 [ 0.000000] softirqs last enabled at (0): [<0000000000000000>] 0x0 [ 0.000000] softirqs last disabled at (0): [<0000000000000000>] 0x0 [ 0.000000] ---[ end trace 0000000000000000 ]--- [ 0.000000] ------------[ cut here ]------------
Quoted 1 line
Quoted 1 line
then qemu hangs until the session is aborted.
Hmm, so this appears to be us attempting to patch in alternatives where none actually exists - seemingly F & D.
Quoted 5 lines
I have come across the same issue on PolarFire SoC while looking at Samuel's fixes for Zbb & the D1:
https://lore.kernel.org/all/20230212021534.59121-1-samuel at sholland.org/
Quoted 52 lines
Boot over NFS still works, which I think points to a hole in how my CI is operating - it assumed the completed boot + bootrr being happy to mean that there was nothing wrong!
On the VisionFive 2 & D1 Nezha I don't see these splats though. It appears to be config related as the config I build for Icicle sees these splats in QEMU but the D1 config does not.
I'll go do some digging!Quoted 43 lines
Similar messages are also seen with the "virt" emulation, but there the boot does not hang but fails to find a root device.
Guenter
---
bisect:# bad: [
6ba8a227fd19d19779005fb66ad7562608e1df83] Add linux-next specific files for 20230210
# good: [4ec5183ec48656cec489c49f989c508b68b518e3] Linux 6.2-rc7
git bisect start 'HEAD' 'v6.2-rc7'
# bad: [94613f0efc69ed41f9229ef5c294db3ec37145da] Merge branch 'master' ofgit://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git
git bisect bad94613f0efc69ed41f9229ef5c294db3ec37145da
# bad: [8928ece68de4371dc6e1d3336d3029c1e18ae3b4] Merge branch 'for_next' ofgit://git.kernel.org/pub/scm/linux/kernel/git/jack/linux-fs.git
git bisect bad8928ece68de4371dc6e1d3336d3029c1e18ae3b4
# good: [78a9f460e33d103256f3abb38f02f4759710c7dc] soc: document merges
git bisect good78a9f460e33d103256f3abb38f02f4759710c7dc
# good: [b35b2472ebafa29d0bbe79fbee1da6ef3c4ec619] Merge branch 'for-next' ofgit://git.kernel.org/pub/scm/linux/kernel/git/qcom/linux.git
git bisect goodb35b2472ebafa29d0bbe79fbee1da6ef3c4ec619
# bad: [57a87a64b520c37dd49f5fde84d09a4adb391180] Merge branch 'for-next' ofgit://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git
git bisect bad57a87a64b520c37dd49f5fde84d09a4adb391180
# good: [cfc8ba01cc84b24ec6eb293ec9fba893f7cd4581] Merge branch 'clk-next' ofgit://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git
git bisect goodcfc8ba01cc84b24ec6eb293ec9fba893f7cd4581
# good: [6acecfa485d3de955c35a18730c106ddf1e7600e] powerpc/kcsan: Add KCSAN Support
git bisect good6acecfa485d3de955c35a18730c106ddf1e7600e
# good: [8a16dea453dbc3e834b162640028e505882cd11e] Merge branch 'next' ofgit://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git
git bisect good8a16dea453dbc3e834b162640028e505882cd11e
# good: [6be1ff430dab9fc047762b10b2c9669399ea1f37] riscv: pgtable: Fixup comment forKERN_VIRT_SIZE
git bisect good6be1ff430dab9fc047762b10b2c9669399ea1f37
# good: [e0c267e03b0c77c9ac79ac08eada41ba8eb1b95f] riscv: module: movefind_sectiontomodule.h
git bisect goode0c267e03b0c77c9ac79ac08eada41ba8eb1b95f
# good: [e8ad17d2b5f38e595d597a3e2419d6d7cc727b17] riscv: KVM: Switchhas_svinval()toriscv_has_extension_unlikely()
git bisect goode8ad17d2b5f38e595d597a3e2419d6d7cc727b17
# good: [75ab93a244a516d1d3c03c4e27d5d0deff76ebfb] Merge patch series "Zbb string optimizations"
git bisect good75ab93a244a516d1d3c03c4e27d5d0deff76ebfb
# bad: [9daca9a5b9ac35361ce2d8d5ec10b19b7048d6cd] Merge patch series "riscv: improve boot time isa extensions handling" git bisect bad9daca9a5b9ac35361ce2d8d5ec10b19b7048d6cd
# good: [03966594e1170303c037b0cded35c464a13a4a45] riscv: remove riscv_isa_ext_keys[] array and related usage
git bisect good03966594e1170303c037b0cded35c464a13a4a45# first bad commit: [9daca9a5b9ac35361ce2d8d5ec10b19b7048d6cd] Merge patch series "riscv: improve boot time isa extensions handling"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230212/4e1e5386/attachment-0001.sig>
Quoted 1 line · Conor Dooley
On Sun, Feb 12, 2023 at 03:59:59PM +0000, Conor Dooley wrote:
So as not to lead anyone up the garden path, let me correct myself:
Quoted 2 lines
Hmm, so this appears to be us attempting to patch in alternatives where none actually exists - seemingly F & D.
And of course that's not true, riscv_has_extension_likely() now uses alternatives as of:bdda5d554e43 ("riscv: introduce riscv_has_extension_[un]likely()")
From a quick look, it just happens that the only users are F & D.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230212/6cae402f/attachment-0001.sig>
Quoted 5 lines · Conor Dooley
Quoted 1 line · Conor Dooley
On Sun, Feb 12, 2023 at 03:59:59PM +0000, Conor Dooley wrote:
So as not to lead anyone up the garden path, let me correct myself:
Quoted 9 lines
Quoted 2 lines
Hmm, so this appears to be us attempting to patch in alternatives where none actually exists - seemingly F & D.
And of course that's not true,
riscv_has_extension_likely()now uses alternatives as of:bdda5d554e43("riscv: introduce riscv_has_extension_[un]likely()")From a quick look, it just happens that the only users are F & D.
Samuel pointed out that this is a lockdep splat on irc. There's a patch on the list that removes the lockdep annotation entirely:
https://patchwork.kernel.org/project/linux-riscv/patch/[email protected]/
So ye, no surprises that it was config based!
Palmer posted a "better" fix for that lockdep warning a while ago:
https://lore.kernel.org/all/[email protected]/
So we'd have to duplicate/reuse that for cpufeature/errata patching.
Quoted 21 lines · Conor Dooley
Quoted 5 lines · Conor Dooley
Quoted 1 line · Conor Dooley
On Sun, Feb 12, 2023 at 03:59:59PM +0000, Conor Dooley wrote:
So as not to lead anyone up the garden path, let me correct myself:
Quoted 9 lines
Quoted 2 lines
Hmm, so this appears to be us attempting to patch in alternatives where none actually exists - seemingly F & D.
And of course that's not true,
riscv_has_extension_likely()now uses alternatives as of:bdda5d554e43("riscv: introduce riscv_has_extension_[un]likely()")From a quick look, it just happens that the only users are F & D.
Samuel pointed out that this is a lockdep splat on irc.
There's a patch on the list that removes the lockdep annotation
entirely:
https://patchwork.kernel.org/project/linux-riscv/patch/20230202114116.3695793-1-changbin.du at huawei.com/
Quoted 8 lines
So ye, no surprises that it was config based!
Palmer posted a "better" fix for that lockdep warning a while ago:
https://lore.kernel.org/all/20220322022331.32136-1-palmer at rivosinc.com/So we'd have to duplicate/reuse that for cpufeature/errata patching.
This does not (only) happen in stop_machine().
[ 0.000000] ------------[ cut here ]------------ [ 0.000000] WARNING: CPU: 0 PID: 0 at arch/riscv/kernel/patch.c:63 patch_insn_write+0x222/0x2f6 [ 0.000000] Modules linked in: [ 0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.2.0-rc7-next-20230210 #1 [ 0.000000] Hardware name: riscv-virtio,qemu (DT) [ 0.000000] epc : patch_insn_write+0x222/0x2f6 [ 0.000000] ra : patch_insn_write+0x21e/0x2f6 [ 0.000000] epc : ffffffff800068c6 ra : ffffffff800068c2 sp : ffffffff81803df0 [ 0.000000] gp : ffffffff81a1ab78 tp : ffffffff81814f80 t0 : ffffffffffffe000 [ 0.000000] t1 : fffffffffafdfb03 t2 : 4c45203a76637369 s0 : ffffffff81803e40 [ 0.000000] s1 : 0000000000000004 a0 : 0000000000000000 a1 : ffffffffffffffff [ 0.000000] a2 : 0000000000000004 a3 : 0000000000000000 a4 : 0000000000000001 [ 0.000000] a5 : 0000000000000000 a6 : 0000000000000006 a7 : 0000000000000010 [ 0.000000] s2 : ffffffff8000431a s3 : 00000000000000b2 s4 : ffffffff800040ae [ 0.000000] s5 : 00000000000000ae s6 : ffffffff8131a0a0 s7 : 0000000000000fff [ 0.000000] s8 : 0000000008000200 s9 : ffffffff8131a520 s10: 0000000000000018 [ 0.000000] s11: 0000000000000008 t3 : 0000000022e125d2 t4 : 000000000000000d [ 0.000000] t5 : ffffffffd8180000 t6 : ffffffff81803bc8 [ 0.000000] status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003 [ 0.000000] [<ffffffff800068c6>] patch_insn_write+0x222/0x2f6 [ 0.000000] [<ffffffff80006a36>] patch_text_nosync+0xc/0x2a [ 0.000000] [<ffffffff80003b86>] riscv_cpufeature_patch_func+0x52/0x98 [ 0.000000] [<ffffffff80003348>] _apply_alternatives+0x46/0x86 [ 0.000000] [<ffffffff80c02d36>] apply_boot_alternatives+0x3c/0xfa [ 0.000000] [<ffffffff80c03ad8>] setup_arch+0x584/0x5b8 [ 0.000000] [<ffffffff80c0075a>] start_kernel+0xa2/0x8f8 [ 0.000000] irq event stamp: 0 [ 0.000000] hardirqs last enabled at (0): [<0000000000000000>] 0x0 [ 0.000000] hardirqs last disabled at (0): [<0000000000000000>] 0x0 [ 0.000000] softirqs last enabled at (0): [<0000000000000000>] 0x0 [ 0.000000] softirqs last disabled at (0): [<0000000000000000>] 0x0 [ 0.000000] ---[ end trace 0000000000000000 ]---
Guenter
Quoted 24 lines · Guenter Roeck
Quoted 23 lines · Conor Dooley
Quoted 5 lines · Conor Dooley
Quoted 1 line · Conor Dooley
On Sun, Feb 12, 2023 at 03:59:59PM +0000, Conor Dooley wrote:
So as not to lead anyone up the garden path, let me correct myself:
Quoted 9 lines
Quoted 2 lines
Hmm, so this appears to be us attempting to patch in alternatives where none actually exists - seemingly F & D.
And of course that's not true,
riscv_has_extension_likely()now uses alternatives as of:bdda5d554e43("riscv: introduce riscv_has_extension_[un]likely()")From a quick look, it just happens that the only users are F & D.
Samuel pointed out that this is a lockdep splat on irc.
There's a patch on the list that removes the lockdep annotation
entirely:
https://patchwork.kernel.org/project/linux-riscv/patch/20230202114116.3695793-1-changbin.du at huawei.com/So ye, no surprises that it was config based!
Quoted 8 lines
Quoted 6 lines
Palmer posted a "better" fix for that lockdep warning a while ago:
https://lore.kernel.org/all/20220322022331.32136-1-palmer at rivosinc.com/So we'd have to duplicate/reuse that for cpufeature/errata patching.
This does not (only) happen in
stop_machine().
Yah, sorry I meant that it's the same lockdep splat as is being addressed there.
The first patch deletes the lockdep stuff entirely, so removes the splat. I was thinking that we'd need to take Palmer's (IMO better) patch and do the same thing for patching alternatives, but I figure we can just take the text_mutex itself for alternatives & not have to dance around the lock.
I'll go do that I suppose!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230212/44cbfbe8/attachment-0001.sig>
Quoted 42 lines · Conor Dooley
Quoted 24 lines · Guenter Roeck
Quoted 23 lines · Conor Dooley
Quoted 5 lines · Conor Dooley
Quoted 1 line · Conor Dooley
On Sun, Feb 12, 2023 at 03:59:59PM +0000, Conor Dooley wrote:
So as not to lead anyone up the garden path, let me correct myself:
Quoted 9 lines
Quoted 2 lines
Hmm, so this appears to be us attempting to patch in alternatives where none actually exists - seemingly F & D.
And of course that's not true,
riscv_has_extension_likely()now uses alternatives as of:bdda5d554e43("riscv: introduce riscv_has_extension_[un]likely()")From a quick look, it just happens that the only users are F & D.
Samuel pointed out that this is a lockdep splat on irc.
There's a patch on the list that removes the lockdep annotation
entirely:
https://patchwork.kernel.org/project/linux-riscv/patch/20230202114116.3695793-1-changbin.du at huawei.com/So ye, no surprises that it was config based!
Quoted 8 lines
Quoted 6 lines
Palmer posted a "better" fix for that lockdep warning a while ago:
https://lore.kernel.org/all/20220322022331.32136-1-palmer at rivosinc.com/So we'd have to duplicate/reuse that for cpufeature/errata patching.
This does not (only) happen in
stop_machine().Yah, sorry I meant that it's the same lockdep splat as is being addressed there.
The first patch deletes the lockdep stuff entirely, so removes the splat. I was thinking that we'd need to take Palmer's (IMO better) patch and do the same thing for patching alternatives, but I figure we can just take thetext_mutexitself for alternatives & not have to dance around the lock.
Quoted 1 line
I'll go do that I suppose!
Thanks a lot for the clarification. That sounds like the backtrace can be largely ignored. However, I still see that the patch series results in boot hangs with the sifive_u qemu emulation, where the log ends with "Oops - illegal instruction". Is that problem being addressed as well ?
Thanks,
Guenter
Quoted 43 lines · Guenter Roeck
Quoted 42 lines · Conor Dooley
Quoted 24 lines · Guenter Roeck
Quoted 23 lines · Conor Dooley
Quoted 5 lines · Conor Dooley
Quoted 1 line · Conor Dooley
On Sun, Feb 12, 2023 at 03:59:59PM +0000, Conor Dooley wrote:
So as not to lead anyone up the garden path, let me correct myself:
Quoted 9 lines
Quoted 2 lines
Hmm, so this appears to be us attempting to patch in alternatives where none actually exists - seemingly F & D.
And of course that's not true,
riscv_has_extension_likely()now uses alternatives as of:bdda5d554e43("riscv: introduce riscv_has_extension_[un]likely()")From a quick look, it just happens that the only users are F & D.
Samuel pointed out that this is a lockdep splat on irc.
There's a patch on the list that removes the lockdep annotation
entirely:
https://patchwork.kernel.org/project/linux-riscv/patch/20230202114116.3695793-1-changbin.du at huawei.com/So ye, no surprises that it was config based!
Quoted 8 lines
Quoted 6 lines
Palmer posted a "better" fix for that lockdep warning a while ago:
https://lore.kernel.org/all/20220322022331.32136-1-palmer at rivosinc.com/So we'd have to duplicate/reuse that for cpufeature/errata patching.
This does not (only) happen in
stop_machine().Yah, sorry I meant that it's the same lockdep splat as is being addressed there.
The first patch deletes the lockdep stuff entirely, so removes the splat. I was thinking that we'd need to take Palmer's (IMO better) patch and do the same thing for patching alternatives, but I figure we can just take thetext_mutexitself for alternatives & not have to dance around the lock.
Quoted 4 lines
Quoted 1 line
I'll go do that I suppose!
Thanks a lot for the clarification. That sounds like the backtrace can be largely ignored.
Yeah, sorry that I phrased that confusingly in the first place.
Quoted 4 lines
However, I still see that the patch series
results in boot hangs with thesifive_uqemu emulation, where the log ends with "Oops - illegal instruction". Is that problem being addressed as well ?
Hmm, if it died on the last commit in this series, then I am not sure.
If you meant with riscv/for-next or linux-next that's fixed by a patch
from Samuel:
https://patchwork.kernel.org/project/linux-riscv/patch/20230212021534.59121-3-samuel at sholland.org/
Cheers,
Conor.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230212/2cf33ed7/attachment-0001.sig>
On 2/12/23 10:45, Conor Dooley wrote:
...
Quoted 11 lines
Quoted 5 lines
However, I still see that the patch series
results in boot hangs with thesifive_uqemu emulation, where the log ends with "Oops - illegal instruction". Is that problem being addressed as well ?Hmm, if it died on the last commit in this series, then I am not sure.
If you meant with riscv/for-next or linux-next that's fixed by a patch
from Samuel:
https://patchwork.kernel.org/project/linux-riscv/patch/20230212021534.59121-3-samuel at sholland.org/
It failed after the merge, so it looks like it may have been merge damage.
Anyway, I applied
RISC-V: Don't check text_mutex during stop_machine
riscv: Fix early alternative patching
riscv: Fix Zbb alternative IDs
and the sifive_u emulation no longer crashes. However, I still get
[ 0.000000] ------------[ cut here ]------------ [ 0.000000] WARNING: CPU: 0 PID: 0 at arch/riscv/kernel/patch.c:71 patch_insn_write+0x222/0x2f6
repeated several times.
I then also tested
riscv: patch: Fixup lockdep warning in stop_machine
riscv: Fix early alternative patching
riscv: Fix Zbb alternative IDs
which works fine (no warning backtrace) for sifive_u, but gives me
WARNING: CPU: 0 PID: 0 at kernel/trace/trace_events.c:433 trace_event_raw_init+0xde/0x642
and a whole lot of
event btrfs_clear_extent_bit has unsafe dereference of argument 1
and similar messages when running the "virt" emulation. That was there before, but drowned in the noise. Ok, guess I'll need another round of bisect.
Guenter
Quoted 14 lines · Guenter Roeck
On 2/12/23 10:45, Conor Dooley wrote:
...Quoted 11 lines
Quoted 5 lines
However, I still see that the patch series
results in boot hangs with thesifive_uqemu emulation, where the log ends with "Oops - illegal instruction". Is that problem being addressed as well ?Hmm, if it died on the last commit in this series, then I am not sure.
If you meant with riscv/for-next or linux-next that's fixed by a patch
from Samuel:
https://patchwork.kernel.org/project/linux-riscv/patch/20230212021534.59121-3-samuel at sholland.org/
Quoted 5 lines
It failed after the merge, so it looks like it may have been merge damage.
Anyway, I applied
RISC-V: Don't check
text_mutexduringstop_machine
That being:
https://lore.kernel.org/all/20220322022331.32136-1-palmer at rivosinc.com/ Which handles the lockdep assertion during stop_machine...
Quoted 7 lines
riscv: Fix early alternative patching
riscv: Fix Zbb alternative IDsand the
sifive_uemulation no longer crashes. However, I still get[ 0.000000] ------------[ cut here ]------------ [ 0.000000] WARNING: CPU: 0 PID: 0 at arch/riscv/kernel/patch.c:71 patch_insn_write+0x222/0x2f6
...but doesn't prevent the early "spam" of assertion failures from the code patching for alternatives. I sent a patch to take the lock during the alternative patching which should get rid of them for you. It did for me at least!
https://lore.kernel.org/all/20230212194735.491785-1-conor at kernel.org
Quoted 5 lines
repeated several times.
I then also tested
riscv: patch: Fixup lockdep warning in
stop_machine
This one just deletes the lockdep check, so I would expect it to remove the complaints.
Quoted 6 lines
riscv: Fix early alternative patching
riscv: Fix Zbb alternative IDswhich works fine (no warning backtrace) for
sifive_u, but gives meWARNING: CPU: 0 PID: 0 at kernel/trace/trace_events.c:433 trace_event_raw_init+0xde/0x642
Hmm, do you have the full splat for this one handy?
Quoted 6 lines
and a whole lot of
event
btrfs_clear_extent_bithas unsafe dereference of argument 1and similar messages when running the "virt" emulation. That was there before, but drowned in the noise. Ok, guess I'll need another round of bisect.
Thanks for all of your testing :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230212/de94317e/attachment-0001.sig>
Quoted 48 lines · Conor Dooley
Quoted 14 lines · Guenter Roeck
On 2/12/23 10:45, Conor Dooley wrote:
...Quoted 11 lines
Quoted 5 lines
However, I still see that the patch series
results in boot hangs with thesifive_uqemu emulation, where the log ends with "Oops - illegal instruction". Is that problem being addressed as well ?Hmm, if it died on the last commit in this series, then I am not sure.
If you meant with riscv/for-next or linux-next that's fixed by a patch
from Samuel:
https://patchwork.kernel.org/project/linux-riscv/patch/20230212021534.59121-3-samuel at sholland.org/Quoted 5 lines
It failed after the merge, so it looks like it may have been merge damage.
Anyway, I applied
RISC-V: Don't check
text_mutexduringstop_machineThat being:
https://lore.kernel.org/all/20220322022331.32136-1-palmer at rivosinc.com/ Which handles the lockdep assertion duringstop_machine...Quoted 7 lines
riscv: Fix early alternative patching
riscv: Fix Zbb alternative IDsand the
sifive_uemulation no longer crashes. However, I still get[ 0.000000] ------------[ cut here ]------------ [ 0.000000] WARNING: CPU: 0 PID: 0 at arch/riscv/kernel/patch.c:71 patch_insn_write+0x222/0x2f6...but doesn't prevent the early "spam" of assertion failures from the code patching for alternatives. I sent a patch to take the lock during the alternative patching which should get rid of them for you. It did for me at least!
https://lore.kernel.org/all/20230212194735.491785-1-conor at kernel.orgQuoted 5 lines
repeated several times.
I then also tested
riscv: patch: Fixup lockdep warning in
stop_machineThis one just deletes the lockdep check, so I would expect it to remove the complaints.
Quoted 9 lines
Quoted 6 lines
riscv: Fix early alternative patching
riscv: Fix Zbb alternative IDswhich works fine (no warning backtrace) for
sifive_u, but gives meWARNING: CPU: 0 PID: 0 at kernel/trace/trace_events.c:433 trace_event_raw_init+0xde/0x642Hmm, do you have the full splat for this one handy?
[ 0.000000] ------------[ cut here ]------------
[ 0.000000] WARNING: CPU: 0 PID: 0 at kernel/trace/trace_events.c:433 trace_event_raw_init+0xde/0x642
[ 0.000000] Modules linked in:
[ 0.000000] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G W 6.2.0-rc7-next-20230210 #1
[ 0.000000] Hardware name: riscv-virtio,qemu (DT)
[ 0.000000] epc : trace_event_raw_init+0xde/0x642
[ 0.000000] ra : trace_event_raw_init+0x45a/0x642
[ 0.000000] epc : ffffffff8010571a ra : ffffffff80105a96 sp : ffffffff81803e60
[ 0.000000] gp : ffffffff81a1ab78 tp : ffffffff81814f80 t0 : 0000000000000000
[ 0.000000] t1 : 5245432d3e000000 t2 : 0000000000000000 s0 : ffffffff81803f20
[ 0.000000] s1 : 000000000000045f a0 : 0000000000000000 a1 : ffffffff81331ef0
[ 0.000000] a2 : 000000000000025c a3 : 0000000000000001 a4 : ffffffff801056fa
[ 0.000000] a5 : 000000000000002c a6 : ffffffff8192e4d8 a7 : ffffffff81157a90
[ 0.000000] s2 : 0000000000000000 s3 : ffffffff81922870 s4 : ffffffff8192e4d8
[ 0.000000] s5 : ffffffff81011c30 s6 : 000000000000000a s7 : 0000000000000021
[ 0.000000] s8 : 000000000000005c s9 : ffffffff81331ee8 s10: 0000000000000001
[ 0.000000] s11: 0000000000000000 t3 : 0000000000000007 t4 : 0000000000000070
[ 0.000000] t5 : 0000000000000025 t6 : 0000000000000009
[ 0.000000] status: 0000000200000100 badaddr: 0000000000000000 cause: 0000000000000003
[ 0.000000] [<ffffffff8010571a>] trace_event_raw_init+0xde/0x642
[ 0.000000] [<ffffffff80104d32>] event_init+0x28/0x84
[ 0.000000] [<ffffffff80c0f7ca>] trace_event_init+0x9e/0x2ae
[ 0.000000] [<ffffffff80c0f3a0>] trace_init+0x10/0x18
[ 0.000000] [<ffffffff80c00bc6>] start_kernel+0x50e/0x8f8
[ 0.000000] irq event stamp: 0
[ 0.000000] hardirqs last enabled at (0): [<0000000000000000>] 0x0
[ 0.000000] hardirqs last disabled at (0): [<0000000000000000>] 0x0
[ 0.000000] softirqs last enabled at (0): [<0000000000000000>] 0x0
[ 0.000000] softirqs last disabled at (0): [<0000000000000000>] 0x0
[ 0.000000] ---[ end trace 0000000000000000 ]---
[ 0.000000] event btrfs_clear_extent_bit has unsafe dereference of argument 1
[ 0.000000] print_fmt: "%pU: io_tree=%s ino=%llu root=%llu start=%llu len=%llu clear_bits=%s", REC->fsid, __print_symbolic(REC->owner, {IO_TREE_FS_PINNED_EXTENTS, "PINNED_EXTENTS"}, {IO_TREE_FS_EXCLUDED_EXTENTS, "EXCLUDED_EXTENTS"}, {IO_TREE_BTREE_INODE_IO, "BTREE_INODE_IO"}, {IO_TREE_INODE_IO, "INODE_IO"}, {IO_TREE_RELOC_BLOCKS, "RELOC_BLOCKS"}, {IO_TREE_TRANS_DIRTY_PAGES, "TRANS_DIRTY_PAGES"}, {IO_TREE_ROOT_DIRTY_LOG_PAGES, "ROOT_DIRTY_LOG_PAGES"}, {IO_TREE_INODE_FILE_EXTENT, "INODE_FILE_EXTENT"}, {IO_TREE_LOG_CSUM_RANGE, "LOG_CSUM_RANGE"}, {IO_TREE_SELFTEST, "SELFTEST"}), REC->ino, REC->rootid, REC->start, REC->len, __print_flags(REC->clear_bits, "|", { EXTENT_DIRTY, "DIRTY"}, { EXTENT_UPTODATE, "UPTODATE"}, { EXTENT_LOCKED, "LOCKED"}, { EXTENT_NEW, "NEW"}, { EXTENT_DELALLOC, "DELALLOC"}, { EXTENT_DEFRAG, "DEFRAG"}, { EXTENT_BOUNDARY, "BOUNDARY"}, { EXTENT_NODATASUM, "NODATASUM"}, { EXTENT_CLEAR_META_RESV, "CLEAR_META_RESV"}, { EXTENT_NEED_WAIT, "NEED_WAIT"}, { EXTENT_NORESERVE,
"NORESERVE"}, { EXTENT_QGROUP_RESERV
[ 0.000000] event btrfs_ordered_sched has unsafe dereference of argument 1
[ 0.000000] print_fmt: "%pU: work=%p (normal_work=%p) wq=%p func=%ps ordered_func=%p ordered_free=%p", REC->fsid, REC->work, REC->normal_work, REC->wq, REC->func, REC->ordered_func, REC->ordered_free
[ 0.000000] event btrfs_work_sched has unsafe dereference of argument 1
[ 0.000000] print_fmt: "%pU: work=%p (normal_work=%p) wq=%p func=%ps ordered_func=%p ordered_free=%p", REC->fsid, REC->work, REC->normal_work, REC->wq, REC->func, REC->ordered_func, REC->ordered_free
[ 0.000000] event btrfs_work_queued has unsafe dereference of argument 1
[ 0.000000] print_fmt: "%pU: work=%p (normal_work=%p) wq=%p func=%ps ordered_func=%p ordered_free=%p", REC->fsid, REC->work, REC->normal_work, REC->wq, REC->func, REC->ordered_func, REC->ordered_free
[ 0.000000] event find_free_extent_search_loop has unsafe dereference of argument 1and so on.
It bisects to "RISC-V: add zbb support to string functions", which also seems to cause various boot failures. Unfortunately that patch is difficult to revert, but marking TOOLCHAIN_HAS_ZBB as broken "fixes" it. I don't know if there is a problem with the patch or with qemu. I'll disable RISCV_ISA_ZBB in my tests for the time being to work around it.
Guenter
Quoted 9 lines
Quoted 6 lines
and a whole lot of
event
btrfs_clear_extent_bithas unsafe dereference of argument 1and similar messages when running the "virt" emulation. That was there before, but drowned in the noise. Ok, guess I'll need another round of bisect.
Thanks for all of your testing :)