* [PATCH V2 0/3] riscv: Enable LOCKDEP
@ 2020-06-27 13:57 guoren
2020-06-27 13:57 ` [PATCH V2 1/3] riscv: Fixup static_obj() fail guoren
` (3 more replies)
0 siblings, 4 replies; 24+ messages in thread
From: guoren @ 2020-06-27 13:57 UTC (permalink / raw)
To: palmerdabbelt, paul.walmsley, anup, greentime.hu, zong.li, aou,
tglx, tycho, nickhu
Cc: linux-riscv, Guo Ren, linux-kernel, linux-csky
From: Guo Ren <guoren@linux.alibaba.com>
Lockdep is needed by proving the spinlocks and rwlocks. To support it,
we need to add TRACE_IRQFLAGS codes in kernel/entry.S. These patches
follow Documentation/irqflags-tracing.txt.
Fixup 2 bugs that block the lockdep implementation.
---
Changes in v2
- Remove sX regs recovery codes which are unnecessary, because
callee will handle them. Thx Greentime :)
- Move "restore a0 - a7" to handle_syscall, but if _TIF_SYSCALL_WORK
is set, "restore a1 - a7" is still duplicated. I prefer a C wrapper
for syscall.
Guo Ren (2):
riscv: Fixup static_obj() fail
riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT
Zong Li (1):
riscv: Fixup lockdep_assert_held with wrong param cpu_running
arch/riscv/Kconfig | 3 +++
arch/riscv/kernel/entry.S | 33 ++++++++++++++++++++++++++++++++-
arch/riscv/kernel/smpboot.c | 1 -
arch/riscv/kernel/vmlinux.lds.S | 2 +-
4 files changed, 36 insertions(+), 3 deletions(-)
--
2.7.4
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 24+ messages in thread* [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-06-27 13:57 [PATCH V2 0/3] riscv: Enable LOCKDEP guoren @ 2020-06-27 13:57 ` guoren 2020-09-11 20:45 ` Aurelien Jarno 2020-06-27 13:57 ` [PATCH V2 2/3] riscv: Fixup lockdep_assert_held with wrong param cpu_running guoren ` (2 subsequent siblings) 3 siblings, 1 reply; 24+ messages in thread From: guoren @ 2020-06-27 13:57 UTC (permalink / raw) To: palmerdabbelt, paul.walmsley, anup, greentime.hu, zong.li, aou, tglx, tycho, nickhu Cc: linux-riscv, Guo Ren, linux-kernel, linux-csky From: Guo Ren <guoren@linux.alibaba.com> When enable LOCKDEP, static_obj() will cause error. Because some __initdata static variables is before _stext: static int static_obj(const void *obj) { unsigned long start = (unsigned long) &_stext, end = (unsigned long) &_end, addr = (unsigned long) obj; /* * static variable? */ if ((addr >= start) && (addr < end)) return 1; [ 0.067192] INFO: trying to register non-static key. [ 0.067325] the code is fine but needs lockdep annotation. [ 0.067449] turning off the locking correctness validator. [ 0.067718] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.7.0-rc7-dirty #44 [ 0.067945] Call Trace: [ 0.068369] [<ffffffe00020323c>] walk_stackframe+0x0/0xa4 [ 0.068506] [<ffffffe000203422>] show_stack+0x2a/0x34 [ 0.068631] [<ffffffe000521e4e>] dump_stack+0x94/0xca [ 0.068757] [<ffffffe000255a4e>] register_lock_class+0x5b8/0x5bc [ 0.068969] [<ffffffe000255abe>] __lock_acquire+0x6c/0x1d5c [ 0.069101] [<ffffffe0002550fe>] lock_acquire+0xae/0x312 [ 0.069228] [<ffffffe000989a8e>] _raw_spin_lock_irqsave+0x40/0x5a [ 0.069357] [<ffffffe000247c64>] complete+0x1e/0x50 [ 0.069479] [<ffffffe000984c38>] rest_init+0x1b0/0x28a [ 0.069660] [<ffffffe0000016a2>] 0xffffffe0000016a2 [ 0.069779] [<ffffffe000001b84>] 0xffffffe000001b84 [ 0.069953] [<ffffffe000001092>] 0xffffffe000001092 static __initdata DECLARE_COMPLETION(kthreadd_done); noinline void __ref rest_init(void) { ... complete(&kthreadd_done); Signed-off-by: Guo Ren <guoren@linux.alibaba.com> --- arch/riscv/kernel/vmlinux.lds.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S index e6f8016..f3586e3 100644 --- a/arch/riscv/kernel/vmlinux.lds.S +++ b/arch/riscv/kernel/vmlinux.lds.S @@ -22,6 +22,7 @@ SECTIONS /* Beginning of code and text segment */ . = LOAD_OFFSET; _start = .; + _stext = .; HEAD_TEXT_SECTION . = ALIGN(PAGE_SIZE); @@ -54,7 +55,6 @@ SECTIONS . = ALIGN(SECTION_ALIGN); .text : { _text = .; - _stext = .; TEXT_TEXT SCHED_TEXT CPUIDLE_TEXT -- 2.7.4 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-06-27 13:57 ` [PATCH V2 1/3] riscv: Fixup static_obj() fail guoren @ 2020-09-11 20:45 ` Aurelien Jarno 2020-09-12 2:39 ` Guo Ren 0 siblings, 1 reply; 24+ messages in thread From: Aurelien Jarno @ 2020-09-11 20:45 UTC (permalink / raw) To: guoren Cc: tycho, aou, nickhu, anup, palmerdabbelt, linux-kernel, linux-csky, Guo Ren, zong.li, paul.walmsley, greentime.hu, tglx, linux-riscv Hi, On 2020-06-27 13:57, guoren@kernel.org wrote: > From: Guo Ren <guoren@linux.alibaba.com> > > When enable LOCKDEP, static_obj() will cause error. Because some > __initdata static variables is before _stext: > > static int static_obj(const void *obj) > { > unsigned long start = (unsigned long) &_stext, > end = (unsigned long) &_end, > addr = (unsigned long) obj; > > /* > * static variable? > */ > if ((addr >= start) && (addr < end)) > return 1; > > [ 0.067192] INFO: trying to register non-static key. > [ 0.067325] the code is fine but needs lockdep annotation. > [ 0.067449] turning off the locking correctness validator. > [ 0.067718] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.7.0-rc7-dirty #44 > [ 0.067945] Call Trace: > [ 0.068369] [<ffffffe00020323c>] walk_stackframe+0x0/0xa4 > [ 0.068506] [<ffffffe000203422>] show_stack+0x2a/0x34 > [ 0.068631] [<ffffffe000521e4e>] dump_stack+0x94/0xca > [ 0.068757] [<ffffffe000255a4e>] register_lock_class+0x5b8/0x5bc > [ 0.068969] [<ffffffe000255abe>] __lock_acquire+0x6c/0x1d5c > [ 0.069101] [<ffffffe0002550fe>] lock_acquire+0xae/0x312 > [ 0.069228] [<ffffffe000989a8e>] _raw_spin_lock_irqsave+0x40/0x5a > [ 0.069357] [<ffffffe000247c64>] complete+0x1e/0x50 > [ 0.069479] [<ffffffe000984c38>] rest_init+0x1b0/0x28a > [ 0.069660] [<ffffffe0000016a2>] 0xffffffe0000016a2 > [ 0.069779] [<ffffffe000001b84>] 0xffffffe000001b84 > [ 0.069953] [<ffffffe000001092>] 0xffffffe000001092 > > static __initdata DECLARE_COMPLETION(kthreadd_done); > > noinline void __ref rest_init(void) > { > ... > complete(&kthreadd_done); > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > --- > arch/riscv/kernel/vmlinux.lds.S | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S > index e6f8016..f3586e3 100644 > --- a/arch/riscv/kernel/vmlinux.lds.S > +++ b/arch/riscv/kernel/vmlinux.lds.S > @@ -22,6 +22,7 @@ SECTIONS > /* Beginning of code and text segment */ > . = LOAD_OFFSET; > _start = .; > + _stext = .; > HEAD_TEXT_SECTION > . = ALIGN(PAGE_SIZE); > > @@ -54,7 +55,6 @@ SECTIONS > . = ALIGN(SECTION_ALIGN); > .text : { > _text = .; > - _stext = .; > TEXT_TEXT > SCHED_TEXT > CPUIDLE_TEXT This patch has been backported to kernel 5.8.4. This causes the kernel to crash when trying to execute the init process: [ 3.484586] AppArmor: AppArmor sha1 policy hashing enabled [ 4.749835] Freeing unused kernel memory: 492K [ 4.752017] Run /init as init process [ 4.753571] usercopy: Kernel memory overwrite attempt detected to kernel text (offset 507879, size 11)! [ 4.754838] ------------[ cut here ]------------ [ 4.755651] kernel BUG at mm/usercopy.c:99! [ 4.756445] Kernel BUG [#1] [ 4.756815] Modules linked in: [ 4.757542] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.8.0-1-riscv64 #1 Debian 5.8.7-1 [ 4.758372] epc: ffffffe0003b5120 ra : ffffffe0003b5120 sp : ffffffe07f783ca0 [ 4.758960] gp : ffffffe000cc7230 tp : ffffffe07f77cec0 t0 : ffffffe000cdafc0 [ 4.759772] t1 : 0000000000000064 t2 : 0000000000000000 s0 : ffffffe07f783cf0 [ 4.760534] s1 : ffffffe00095d780 a0 : 000000000000005b a1 : 0000000000000020 [ 4.761309] a2 : 0000000000000005 a3 : 0000000000000000 a4 : ffffffe000c1f340 [ 4.761848] a5 : ffffffe000c1f340 a6 : 0000000000000000 a7 : 0000000000000087 [ 4.762684] s2 : ffffffe000941848 s3 : 000000000007bfe7 s4 : 000000000000000b [ 4.763500] s5 : 0000000000000000 s6 : ffffffe00091cc00 s7 : fffffffffffff000 [ 4.764376] s8 : 0000003ffffff000 s9 : ffffffe0769f3200 s10: 000000000000000b [ 4.765208] s11: ffffffe07d548c40 t3 : 0000000000000000 t4 : 000000000001dcd0 [ 4.766059] t5 : ffffffe000cc8510 t6 : ffffffe000cd64aa [ 4.766712] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003 [ 4.768308] ---[ end trace 1f8e733e834d4c3e ]--- [ 4.769129] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b [ 4.770070] SMP: stopping secondary CPUs [ 4.771110] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]--- Note that this is with CONFIG_HARDENED_USERCOPY=y Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-09-11 20:45 ` Aurelien Jarno @ 2020-09-12 2:39 ` Guo Ren 2020-09-14 10:38 ` Aurelien Jarno 0 siblings, 1 reply; 24+ messages in thread From: Guo Ren @ 2020-09-12 2:39 UTC (permalink / raw) To: Guo Ren, Palmer Dabbelt, Paul Walmsley, Anup Patel, Greentime Hu, Zong Li, Albert Ou, Thomas Gleixner, Tycho Andersen, Nick Hu, linux-riscv, Guo Ren, Linux Kernel Mailing List, linux-csky It's come from mm/usercopy.c /* Is this address range in the kernel text area? */ static inline void check_kernel_text_object(const unsigned long ptr, unsigned long n, bool to_user) { unsigned long textlow = (unsigned long)_stext; unsigned long texthigh = (unsigned long)_etext; unsigned long textlow_linear, texthigh_linear; if (overlaps(ptr, n, textlow, texthigh)) usercopy_abort("kernel text", NULL, to_user, ptr - textlow, n); The __init_text/data areas will be freed after bootup, so I think it should be: - unsigned long textlow = (unsigned long)_stext; + unsigned long textlow = (unsigned long)_text; That means _stext should include init_text/data and _text is only for freeable. On Sat, Sep 12, 2020 at 5:01 AM Aurelien Jarno <aurelien@aurel32.net> wrote: > > Hi, > > On 2020-06-27 13:57, guoren@kernel.org wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > > > When enable LOCKDEP, static_obj() will cause error. Because some > > __initdata static variables is before _stext: > > > > static int static_obj(const void *obj) > > { > > unsigned long start = (unsigned long) &_stext, > > end = (unsigned long) &_end, > > addr = (unsigned long) obj; > > > > /* > > * static variable? > > */ > > if ((addr >= start) && (addr < end)) > > return 1; > > > > [ 0.067192] INFO: trying to register non-static key. > > [ 0.067325] the code is fine but needs lockdep annotation. > > [ 0.067449] turning off the locking correctness validator. > > [ 0.067718] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.7.0-rc7-dirty #44 > > [ 0.067945] Call Trace: > > [ 0.068369] [<ffffffe00020323c>] walk_stackframe+0x0/0xa4 > > [ 0.068506] [<ffffffe000203422>] show_stack+0x2a/0x34 > > [ 0.068631] [<ffffffe000521e4e>] dump_stack+0x94/0xca > > [ 0.068757] [<ffffffe000255a4e>] register_lock_class+0x5b8/0x5bc > > [ 0.068969] [<ffffffe000255abe>] __lock_acquire+0x6c/0x1d5c > > [ 0.069101] [<ffffffe0002550fe>] lock_acquire+0xae/0x312 > > [ 0.069228] [<ffffffe000989a8e>] _raw_spin_lock_irqsave+0x40/0x5a > > [ 0.069357] [<ffffffe000247c64>] complete+0x1e/0x50 > > [ 0.069479] [<ffffffe000984c38>] rest_init+0x1b0/0x28a > > [ 0.069660] [<ffffffe0000016a2>] 0xffffffe0000016a2 > > [ 0.069779] [<ffffffe000001b84>] 0xffffffe000001b84 > > [ 0.069953] [<ffffffe000001092>] 0xffffffe000001092 > > > > static __initdata DECLARE_COMPLETION(kthreadd_done); > > > > noinline void __ref rest_init(void) > > { > > ... > > complete(&kthreadd_done); > > > > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > > --- > > arch/riscv/kernel/vmlinux.lds.S | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S > > index e6f8016..f3586e3 100644 > > --- a/arch/riscv/kernel/vmlinux.lds.S > > +++ b/arch/riscv/kernel/vmlinux.lds.S > > @@ -22,6 +22,7 @@ SECTIONS > > /* Beginning of code and text segment */ > > . = LOAD_OFFSET; > > _start = .; > > + _stext = .; > > HEAD_TEXT_SECTION > > . = ALIGN(PAGE_SIZE); > > > > @@ -54,7 +55,6 @@ SECTIONS > > . = ALIGN(SECTION_ALIGN); > > .text : { > > _text = .; > > - _stext = .; > > TEXT_TEXT > > SCHED_TEXT > > CPUIDLE_TEXT > > > This patch has been backported to kernel 5.8.4. This causes the kernel > to crash when trying to execute the init process: > > [ 3.484586] AppArmor: AppArmor sha1 policy hashing enabled > [ 4.749835] Freeing unused kernel memory: 492K > [ 4.752017] Run /init as init process > [ 4.753571] usercopy: Kernel memory overwrite attempt detected to kernel text (offset 507879, size 11)! > [ 4.754838] ------------[ cut here ]------------ > [ 4.755651] kernel BUG at mm/usercopy.c:99! > [ 4.756445] Kernel BUG [#1] > [ 4.756815] Modules linked in: > [ 4.757542] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 5.8.0-1-riscv64 #1 Debian 5.8.7-1 > [ 4.758372] epc: ffffffe0003b5120 ra : ffffffe0003b5120 sp : ffffffe07f783ca0 > [ 4.758960] gp : ffffffe000cc7230 tp : ffffffe07f77cec0 t0 : ffffffe000cdafc0 > [ 4.759772] t1 : 0000000000000064 t2 : 0000000000000000 s0 : ffffffe07f783cf0 > [ 4.760534] s1 : ffffffe00095d780 a0 : 000000000000005b a1 : 0000000000000020 > [ 4.761309] a2 : 0000000000000005 a3 : 0000000000000000 a4 : ffffffe000c1f340 > [ 4.761848] a5 : ffffffe000c1f340 a6 : 0000000000000000 a7 : 0000000000000087 > [ 4.762684] s2 : ffffffe000941848 s3 : 000000000007bfe7 s4 : 000000000000000b > [ 4.763500] s5 : 0000000000000000 s6 : ffffffe00091cc00 s7 : fffffffffffff000 > [ 4.764376] s8 : 0000003ffffff000 s9 : ffffffe0769f3200 s10: 000000000000000b > [ 4.765208] s11: ffffffe07d548c40 t3 : 0000000000000000 t4 : 000000000001dcd0 > [ 4.766059] t5 : ffffffe000cc8510 t6 : ffffffe000cd64aa > [ 4.766712] status: 0000000000000120 badaddr: 0000000000000000 cause: 0000000000000003 > [ 4.768308] ---[ end trace 1f8e733e834d4c3e ]--- > [ 4.769129] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b > [ 4.770070] SMP: stopping secondary CPUs > [ 4.771110] ---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0000000b ]--- > > Note that this is with CONFIG_HARDENED_USERCOPY=y > > Aurelien > > -- > Aurelien Jarno GPG: 4096R/1DDD8C9B > aurelien@aurel32.net http://www.aurel32.net -- Best Regards Guo Ren ML: https://lore.kernel.org/linux-csky/ _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-09-12 2:39 ` Guo Ren @ 2020-09-14 10:38 ` Aurelien Jarno 2020-09-24 7:36 ` Andreas Schwab 2020-10-05 8:25 ` Andreas Schwab 0 siblings, 2 replies; 24+ messages in thread From: Aurelien Jarno @ 2020-09-14 10:38 UTC (permalink / raw) To: Guo Ren Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt, Linux Kernel Mailing List, linux-csky, Guo Ren, Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv On 2020-09-12 10:39, Guo Ren wrote: > It's come from mm/usercopy.c > /* Is this address range in the kernel text area? */ > static inline void check_kernel_text_object(const unsigned long ptr, > unsigned long n, bool to_user) > { > unsigned long textlow = (unsigned long)_stext; > unsigned long texthigh = (unsigned long)_etext; > unsigned long textlow_linear, texthigh_linear; > > if (overlaps(ptr, n, textlow, texthigh)) > usercopy_abort("kernel text", NULL, to_user, ptr - textlow, n); > > The __init_text/data areas will be freed after bootup, so I think it should be: > - unsigned long textlow = (unsigned long)_stext; > + unsigned long textlow = (unsigned long)_text; > > That means _stext should include init_text/data and _text is only for freeable. I have no idea if it is the right thing to do or not, but I can confirm this fixes the issue. How should we proceed to get that fixed in time for 5.9? For the older branches where it has been backported (so far 5.7 and 5.8), should we just get that commit reverted instead? Thanks, Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-09-14 10:38 ` Aurelien Jarno @ 2020-09-24 7:36 ` Andreas Schwab 2020-09-24 16:19 ` Guo Ren 2020-10-05 8:25 ` Andreas Schwab 1 sibling, 1 reply; 24+ messages in thread From: Andreas Schwab @ 2020-09-24 7:36 UTC (permalink / raw) To: Guo Ren Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt, Linux Kernel Mailing List, linux-csky, Guo Ren, Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv On Sep 14 2020, Aurelien Jarno wrote: > How should we proceed to get that fixed in time for 5.9? For the older > branches where it has been backported (so far 5.7 and 5.8), should we > just get that commit reverted instead? Can this please be resolved ASAP? Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-09-24 7:36 ` Andreas Schwab @ 2020-09-24 16:19 ` Guo Ren 2020-09-29 18:51 ` Aurelien Jarno 2020-10-05 19:14 ` Atish Patra 0 siblings, 2 replies; 24+ messages in thread From: Guo Ren @ 2020-09-24 16:19 UTC (permalink / raw) To: Andreas Schwab Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt, Linux Kernel Mailing List, linux-csky, Guo Ren, Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv How about this, revert the commit and don't free INIT_DATA_SECTION. I think the solution is safe enough, but wast a little memory. diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S index f3586e3..34d00d9 100644 --- a/arch/riscv/kernel/vmlinux.lds.S +++ b/arch/riscv/kernel/vmlinux.lds.S @@ -22,13 +22,11 @@ SECTIONS /* Beginning of code and text segment */ . = LOAD_OFFSET; _start = .; - _stext = .; HEAD_TEXT_SECTION . = ALIGN(PAGE_SIZE); __init_begin = .; INIT_TEXT_SECTION(PAGE_SIZE) - INIT_DATA_SECTION(16) . = ALIGN(8); __soc_early_init_table : { __soc_early_init_table_start = .; @@ -55,6 +53,7 @@ SECTIONS . = ALIGN(SECTION_ALIGN); .text : { _text = .; + _stext = .; TEXT_TEXT SCHED_TEXT CPUIDLE_TEXT @@ -67,6 +66,8 @@ SECTIONS _etext = .; } + INIT_DATA_SECTION(16) + /* Start of data section */ _sdata = .; RO_DATA(SECTION_ALIGN) On Thu, Sep 24, 2020 at 3:36 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > On Sep 14 2020, Aurelien Jarno wrote: > > > How should we proceed to get that fixed in time for 5.9? For the older > > branches where it has been backported (so far 5.7 and 5.8), should we > > just get that commit reverted instead? > > Can this please be resolved ASAP? > > Andreas. > > -- > Andreas Schwab, schwab@linux-m68k.org > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > "And now for something completely different." -- Best Regards Guo Ren ML: https://lore.kernel.org/linux-csky/ _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-09-24 16:19 ` Guo Ren @ 2020-09-29 18:51 ` Aurelien Jarno 2020-10-05 19:14 ` Atish Patra 1 sibling, 0 replies; 24+ messages in thread From: Aurelien Jarno @ 2020-09-29 18:51 UTC (permalink / raw) To: Guo Ren Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt, Linux Kernel Mailing List, linux-csky, Guo Ren, Andreas Schwab, Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv Hi, On 2020-09-25 00:19, Guo Ren wrote: > How about this, revert the commit and don't free INIT_DATA_SECTION. I > think the solution is safe enough, but wast a little memory. > > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S > index f3586e3..34d00d9 100644 > --- a/arch/riscv/kernel/vmlinux.lds.S > +++ b/arch/riscv/kernel/vmlinux.lds.S > @@ -22,13 +22,11 @@ SECTIONS > /* Beginning of code and text segment */ > . = LOAD_OFFSET; > _start = .; > - _stext = .; > HEAD_TEXT_SECTION > . = ALIGN(PAGE_SIZE); > > __init_begin = .; > INIT_TEXT_SECTION(PAGE_SIZE) > - INIT_DATA_SECTION(16) > . = ALIGN(8); > __soc_early_init_table : { > __soc_early_init_table_start = .; > @@ -55,6 +53,7 @@ SECTIONS > . = ALIGN(SECTION_ALIGN); > .text : { > _text = .; > + _stext = .; > TEXT_TEXT > SCHED_TEXT > CPUIDLE_TEXT > @@ -67,6 +66,8 @@ SECTIONS > _etext = .; > } > > + INIT_DATA_SECTION(16) > + > /* Start of data section */ > _sdata = .; > RO_DATA(SECTION_ALIGN) > This patch doesn't apply, as tabs have been converted to space somewhere. After fixing that, the patch applies and I confirm that it fixes the problem. Tested-by: Aurelien Jarno <aurelien@aurel32.net> Thanks, Aurelien -- Aurelien Jarno GPG: 4096R/1DDD8C9B aurelien@aurel32.net http://www.aurel32.net _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-09-24 16:19 ` Guo Ren 2020-09-29 18:51 ` Aurelien Jarno @ 2020-10-05 19:14 ` Atish Patra 2020-10-06 16:46 ` Guo Ren 1 sibling, 1 reply; 24+ messages in thread From: Atish Patra @ 2020-10-05 19:14 UTC (permalink / raw) To: Guo Ren Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt, Linux Kernel Mailing List, linux-csky, Guo Ren, Andreas Schwab, Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv On Thu, Sep 24, 2020 at 9:19 AM Guo Ren <guoren@kernel.org> wrote: > > How about this, revert the commit and don't free INIT_DATA_SECTION. I > think the solution is safe enough, but wast a little memory. > > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S > index f3586e3..34d00d9 100644 > --- a/arch/riscv/kernel/vmlinux.lds.S > +++ b/arch/riscv/kernel/vmlinux.lds.S > @@ -22,13 +22,11 @@ SECTIONS > /* Beginning of code and text segment */ > . = LOAD_OFFSET; > _start = .; > - _stext = .; > HEAD_TEXT_SECTION > . = ALIGN(PAGE_SIZE); > > __init_begin = .; > INIT_TEXT_SECTION(PAGE_SIZE) > - INIT_DATA_SECTION(16) > . = ALIGN(8); > __soc_early_init_table : { > __soc_early_init_table_start = .; > @@ -55,6 +53,7 @@ SECTIONS > . = ALIGN(SECTION_ALIGN); > .text : { > _text = .; > + _stext = .; > TEXT_TEXT > SCHED_TEXT > CPUIDLE_TEXT > @@ -67,6 +66,8 @@ SECTIONS > _etext = .; > } > > + INIT_DATA_SECTION(16) > + I think you need to move EXIT_DATA as well. Currently, we have init data & text in one section. In general it is better idea to separate those similar to ARM64. Additionally, ARM64 applies different mapping for init data & text as the init data section is marked as non-executable[1] However, we don't modify any permission for any init sections. Should we do that as well ? [1] https://patchwork.kernel.org/patch/9572869/ > /* Start of data section */ > _sdata = .; > RO_DATA(SECTION_ALIGN) > > On Thu, Sep 24, 2020 at 3:36 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > > > On Sep 14 2020, Aurelien Jarno wrote: > > > > > How should we proceed to get that fixed in time for 5.9? For the older > > > branches where it has been backported (so far 5.7 and 5.8), should we > > > just get that commit reverted instead? > > > > Can this please be resolved ASAP? > > > > Andreas. > > > > -- > > Andreas Schwab, schwab@linux-m68k.org > > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > > "And now for something completely different." > > > > -- > Best Regards > Guo Ren > > ML: https://lore.kernel.org/linux-csky/ > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv -- Regards, Atish _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-10-05 19:14 ` Atish Patra @ 2020-10-06 16:46 ` Guo Ren 2020-10-06 20:38 ` Atish Patra 0 siblings, 1 reply; 24+ messages in thread From: Guo Ren @ 2020-10-06 16:46 UTC (permalink / raw) To: Atish Patra Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt, Linux Kernel Mailing List, linux-csky, Guo Ren, Andreas Schwab, Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv On Tue, Oct 6, 2020 at 3:14 AM Atish Patra <atishp@atishpatra.org> wrote: > > On Thu, Sep 24, 2020 at 9:19 AM Guo Ren <guoren@kernel.org> wrote: > > > > How about this, revert the commit and don't free INIT_DATA_SECTION. I > > think the solution is safe enough, but wast a little memory. > > > > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S > > index f3586e3..34d00d9 100644 > > --- a/arch/riscv/kernel/vmlinux.lds.S > > +++ b/arch/riscv/kernel/vmlinux.lds.S > > @@ -22,13 +22,11 @@ SECTIONS > > /* Beginning of code and text segment */ > > . = LOAD_OFFSET; > > _start = .; > > - _stext = .; > > HEAD_TEXT_SECTION > > . = ALIGN(PAGE_SIZE); > > > > __init_begin = .; > > INIT_TEXT_SECTION(PAGE_SIZE) > > - INIT_DATA_SECTION(16) > > . = ALIGN(8); > > __soc_early_init_table : { > > __soc_early_init_table_start = .; > > @@ -55,6 +53,7 @@ SECTIONS > > . = ALIGN(SECTION_ALIGN); > > .text : { > > _text = .; > > + _stext = .; > > TEXT_TEXT > > SCHED_TEXT > > CPUIDLE_TEXT > > @@ -67,6 +66,8 @@ SECTIONS > > _etext = .; > > } > > > > + INIT_DATA_SECTION(16) > > + > > I think you need to move EXIT_DATA as well. Currently, we have init > data & text in one section. It's not related to this issue. There is two check code problem: 1. static int static_obj(const void *obj) { unsigned long start = (unsigned long) &_stext, end = (unsigned long) &_end, addr = (unsigned long) obj; /* * static variable? */ if ((addr >= start) && (addr < end)) return 1; 2. /* Is this address range in the kernel text area? */ static inline void check_kernel_text_object(const unsigned long ptr, unsigned long n, bool to_user) { unsigned long textlow = (unsigned long)_stext; unsigned long texthigh = (unsigned long)_etext; unsigned long textlow_linear, texthigh_linear; if (overlaps(ptr, n, textlow, texthigh)) usercopy_abort("kernel text", NULL, to_user, ptr - textlow, n); The patch of commit: a0fa4027dc911 (riscv: Fixup static_obj() fail) broke 2th. > In general it is better idea to separate those similar to ARM64. > Additionally, ARM64 applies different mapping for init data & text > as the init data section is marked as non-executable[1] Yes, it's safer to protect init text & init data, but it's should be another patch. > > However, we don't modify any permission for any init sections. Should > we do that as well ? Agree, we should do that. > > [1] https://patchwork.kernel.org/patch/9572869/ > > > /* Start of data section */ > > _sdata = .; > > RO_DATA(SECTION_ALIGN) > > > > On Thu, Sep 24, 2020 at 3:36 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > > > > > On Sep 14 2020, Aurelien Jarno wrote: > > > > > > > How should we proceed to get that fixed in time for 5.9? For the older > > > > branches where it has been backported (so far 5.7 and 5.8), should we > > > > just get that commit reverted instead? > > > > > > Can this please be resolved ASAP? > > > > > > Andreas. > > > > > > -- > > > Andreas Schwab, schwab@linux-m68k.org > > > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > > > "And now for something completely different." > > > > > > > > -- > > Best Regards > > Guo Ren > > > > ML: https://lore.kernel.org/linux-csky/ > > > > _______________________________________________ > > linux-riscv mailing list > > linux-riscv@lists.infradead.org > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > -- > Regards, > Atish -- Best Regards Guo Ren ML: https://lore.kernel.org/linux-csky/ _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-10-06 16:46 ` Guo Ren @ 2020-10-06 20:38 ` Atish Patra 2020-10-07 14:45 ` Guo Ren 0 siblings, 1 reply; 24+ messages in thread From: Atish Patra @ 2020-10-06 20:38 UTC (permalink / raw) To: Guo Ren Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt, Linux Kernel Mailing List, linux-csky, Guo Ren, Andreas Schwab, Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv On Tue, Oct 6, 2020 at 9:46 AM Guo Ren <guoren@kernel.org> wrote: > > On Tue, Oct 6, 2020 at 3:14 AM Atish Patra <atishp@atishpatra.org> wrote: > > > > On Thu, Sep 24, 2020 at 9:19 AM Guo Ren <guoren@kernel.org> wrote: > > > > > > How about this, revert the commit and don't free INIT_DATA_SECTION. I > > > think the solution is safe enough, but wast a little memory. > > > > > > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S > > > index f3586e3..34d00d9 100644 > > > --- a/arch/riscv/kernel/vmlinux.lds.S > > > +++ b/arch/riscv/kernel/vmlinux.lds.S > > > @@ -22,13 +22,11 @@ SECTIONS > > > /* Beginning of code and text segment */ > > > . = LOAD_OFFSET; > > > _start = .; > > > - _stext = .; > > > HEAD_TEXT_SECTION > > > . = ALIGN(PAGE_SIZE); > > > > > > __init_begin = .; > > > INIT_TEXT_SECTION(PAGE_SIZE) > > > - INIT_DATA_SECTION(16) > > > . = ALIGN(8); > > > __soc_early_init_table : { > > > __soc_early_init_table_start = .; > > > @@ -55,6 +53,7 @@ SECTIONS > > > . = ALIGN(SECTION_ALIGN); > > > .text : { > > > _text = .; > > > + _stext = .; > > > TEXT_TEXT > > > SCHED_TEXT > > > CPUIDLE_TEXT > > > @@ -67,6 +66,8 @@ SECTIONS > > > _etext = .; > > > } > > > > > > + INIT_DATA_SECTION(16) > > > + > > > > I think you need to move EXIT_DATA as well. Currently, we have init > > data & text in one section. > It's not related to this issue. There is two check code problem: Yes. But we shouldn't move only INIT_DATA_SECTION out of __init section while leaving percpu & exit data in the __init section. Here is what I have in mind. diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S index 9795359cb9da..4432cef8184e 100644 --- a/arch/riscv/kernel/vmlinux.lds.S +++ b/arch/riscv/kernel/vmlinux.lds.S @@ -26,13 +26,11 @@ SECTIONS /* Beginning of code and text segment */ . = LOAD_OFFSET; _start = .; _start = .; - _stext = .; HEAD_TEXT_SECTION . = ALIGN(PAGE_SIZE); __init_begin = .; INIT_TEXT_SECTION(PAGE_SIZE) - INIT_DATA_SECTION(16) . = ALIGN(8); __soc_early_init_table : { __soc_early_init_table_start = .; @@ -49,16 +47,13 @@ SECTIONS { EXIT_TEXT } - .exit.data : - { - EXIT_DATA - } - PERCPU_SECTION(L1_CACHE_BYTES) + __init_end = .; . = ALIGN(SECTION_ALIGN); .text : { _text = .; + _stext = .; TEXT_TEXT SCHED_TEXT CPUIDLE_TEXT @@ -77,6 +72,17 @@ SECTIONS #endif /* Start of data section */ + __init_data_begin = .; + INIT_DATA_SECTION(16) + .exit.data : + { + EXIT_DATA + } + + PERCPU_SECTION(L1_CACHE_BYTES) + + __init_data_end = .; + As you correctly pointed out, this wastes around ~200K init memory that is wasted. That is not an ideal solution. The other alternative is to move __init_text section after _text as well similar to other architectures. But that won't work for RISC-V as we jump from _start to __start_kernel(in __init section) in head.S. A JAL instruction can't be fit because __start_kernel is now too far. We can't replace JAL with a JALR because that would require an additional instruction and violates image header format. Any other ideas to solve this problem without wasting memory ? > 1. static int static_obj(const void *obj) > { > unsigned long start = (unsigned long) &_stext, > end = (unsigned long) &_end, > addr = (unsigned long) obj; > > /* > * static variable? > */ > if ((addr >= start) && (addr < end)) > return 1; > > 2. /* Is this address range in the kernel text area? */ > static inline void check_kernel_text_object(const unsigned long ptr, > unsigned long n, bool to_user) > { > unsigned long textlow = (unsigned long)_stext; > unsigned long texthigh = (unsigned long)_etext; > unsigned long textlow_linear, texthigh_linear; > > if (overlaps(ptr, n, textlow, texthigh)) > usercopy_abort("kernel text", NULL, to_user, ptr - > textlow, n); > > The patch of commit: a0fa4027dc911 (riscv: Fixup static_obj() fail) broke 2th. > > > In general it is better idea to separate those similar to ARM64. > > Additionally, ARM64 applies different mapping for init data & text > > as the init data section is marked as non-executable[1] > Yes, it's safer to protect init text & init data, but it's should be > another patch. > Yes. I will send the patch based on this fix. > > > > However, we don't modify any permission for any init sections. Should > > we do that as well ? > Agree, we should do that. > > > > > [1] https://patchwork.kernel.org/patch/9572869/ > > > > > /* Start of data section */ > > > _sdata = .; > > > RO_DATA(SECTION_ALIGN) > > > > > > On Thu, Sep 24, 2020 at 3:36 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > > > > > > > On Sep 14 2020, Aurelien Jarno wrote: > > > > > > > > > How should we proceed to get that fixed in time for 5.9? For the older > > > > > branches where it has been backported (so far 5.7 and 5.8), should we > > > > > just get that commit reverted instead? > > > > > > > > Can this please be resolved ASAP? > > > > > > > > Andreas. > > > > > > > > -- > > > > Andreas Schwab, schwab@linux-m68k.org > > > > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > > > > "And now for something completely different." > > > > > > > > > > > > -- > > > Best Regards > > > Guo Ren > > > > > > ML: https://lore.kernel.org/linux-csky/ > > > > > > _______________________________________________ > > > linux-riscv mailing list > > > linux-riscv@lists.infradead.org > > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > > > > > -- > > Regards, > > Atish > > > > -- > Best Regards > Guo Ren > > ML: https://lore.kernel.org/linux-csky/ -- Regards, Atish _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-10-06 20:38 ` Atish Patra @ 2020-10-07 14:45 ` Guo Ren 0 siblings, 0 replies; 24+ messages in thread From: Guo Ren @ 2020-10-07 14:45 UTC (permalink / raw) To: Atish Patra Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt, Linux Kernel Mailing List, linux-csky, Guo Ren, Andreas Schwab, Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv On Wed, Oct 7, 2020 at 4:39 AM Atish Patra <atishp@atishpatra.org> wrote: > > On Tue, Oct 6, 2020 at 9:46 AM Guo Ren <guoren@kernel.org> wrote: > > > > On Tue, Oct 6, 2020 at 3:14 AM Atish Patra <atishp@atishpatra.org> wrote: > > > > > > On Thu, Sep 24, 2020 at 9:19 AM Guo Ren <guoren@kernel.org> wrote: > > > > > > > > How about this, revert the commit and don't free INIT_DATA_SECTION. I > > > > think the solution is safe enough, but wast a little memory. > > > > > > > > diff --git a/arch/riscv/kernel/vmlinux.lds.S b/arch/riscv/kernel/vmlinux.lds.S > > > > index f3586e3..34d00d9 100644 > > > > --- a/arch/riscv/kernel/vmlinux.lds.S > > > > +++ b/arch/riscv/kernel/vmlinux.lds.S > > > > @@ -22,13 +22,11 @@ SECTIONS > > > > /* Beginning of code and text segment */ > > > > . = LOAD_OFFSET; > > > > _start = .; > > > > - _stext = .; > > > > HEAD_TEXT_SECTION > > > > . = ALIGN(PAGE_SIZE); > > > > > > > > __init_begin = .; > > > > INIT_TEXT_SECTION(PAGE_SIZE) > > > > - INIT_DATA_SECTION(16) > > > > . = ALIGN(8); > > > > __soc_early_init_table : { > > > > __soc_early_init_table_start = .; > > > > @@ -55,6 +53,7 @@ SECTIONS > > > > . = ALIGN(SECTION_ALIGN); > > > > .text : { > > > > _text = .; > > > > + _stext = .; > > > > TEXT_TEXT > > > > SCHED_TEXT > > > > CPUIDLE_TEXT > > > > @@ -67,6 +66,8 @@ SECTIONS > > > > _etext = .; > > > > } > > > > > > > > + INIT_DATA_SECTION(16) > > > > + > > > > > > I think you need to move EXIT_DATA as well. Currently, we have init > > > data & text in one section. > > It's not related to this issue. There is two check code problem: > > Yes. But we shouldn't move only INIT_DATA_SECTION out of __init section > while leaving percpu & exit data in the __init section. Here is what I > have in mind. > > diff --git a/arch/riscv/kernel/vmlinux.lds.S > b/arch/riscv/kernel/vmlinux.lds.S > index 9795359cb9da..4432cef8184e 100644 > --- a/arch/riscv/kernel/vmlinux.lds.S > +++ b/arch/riscv/kernel/vmlinux.lds.S > @@ -26,13 +26,11 @@ SECTIONS > /* Beginning of code and text segment */ > . = LOAD_OFFSET; > _start = .; > _start = .; > - _stext = .; > HEAD_TEXT_SECTION > . = ALIGN(PAGE_SIZE); > > __init_begin = .; > INIT_TEXT_SECTION(PAGE_SIZE) > - INIT_DATA_SECTION(16) > . = ALIGN(8); > __soc_early_init_table : { > __soc_early_init_table_start = .; > @@ -49,16 +47,13 @@ SECTIONS > { > EXIT_TEXT > } > - .exit.data : > - { > - EXIT_DATA > - } > - PERCPU_SECTION(L1_CACHE_BYTES) > + > __init_end = .; > > . = ALIGN(SECTION_ALIGN); > .text : { > _text = .; > + _stext = .; > TEXT_TEXT > SCHED_TEXT > CPUIDLE_TEXT > @@ -77,6 +72,17 @@ SECTIONS > #endif > > /* Start of data section */ > + __init_data_begin = .; > + INIT_DATA_SECTION(16) > + .exit.data : > + { > + EXIT_DATA > + } > + > + PERCPU_SECTION(L1_CACHE_BYTES) > + > + __init_data_end = .; > + > > As you correctly pointed out, this wastes around ~200K init memory > that is wasted. > That is not an ideal solution. > > The other alternative is to move __init_text section after _text as > well similar to other architectures. But that won't work > for RISC-V as we jump from _start to __start_kernel(in __init section) > in head.S. A JAL instruction can't be fit because > __start_kernel is now too far. We can't replace JAL with a JALR > because that would require an additional > instruction and violates image header format. > > Any other ideas to solve this problem without wasting memory ? How about: diff --git a/arch/riscv/include/asm/sections.h b/arch/riscv/include/asm/sections.h new file mode 100644 index 00000000..2317b9e --- /dev/null +++ b/arch/riscv/include/asm/sections.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ + +#ifndef _ASM_RISCV_SECTIONS_H +#define _ASM_RISCV_SECTIONS_H + +#define arch_is_kernel_data arch_is_kernel_data + +#include <asm-generic/sections.h> + +extern bool init_mem_is_free; + +static inline int arch_is_kernel_data(unsigned long addr) +{ + if (init_mem_is_free) + return 0; + + return addr >= (unsigned long)__init_begin && + addr < (unsigned long)__init_end; +} +#endif /* _ASM_RISCV_SECTIONS_H */ diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c index 2c6dd32..9ebd5eb4 100644 --- a/arch/riscv/kernel/setup.c +++ b/arch/riscv/kernel/setup.c @@ -17,6 +17,7 @@ #include <linux/sched/task.h> #include <linux/swiotlb.h> #include <linux/smp.h> +#include <linux/poison.h> #include <asm/cpu_ops.h> #include <asm/setup.h> @@ -112,3 +113,11 @@ static int __init topology_init(void) return 0; } subsys_initcall(topology_init); + +bool init_mem_is_free = false; + +void free_initmem(void) +{ + free_initmem_default(POISON_FREE_INITMEM); + init_mem_is_free = true; +} > > > 1. static int static_obj(const void *obj) > > { > > unsigned long start = (unsigned long) &_stext, > > end = (unsigned long) &_end, > > addr = (unsigned long) obj; > > > > /* > > * static variable? > > */ > > if ((addr >= start) && (addr < end)) > > return 1; > > > > 2. /* Is this address range in the kernel text area? */ > > static inline void check_kernel_text_object(const unsigned long ptr, > > unsigned long n, bool to_user) > > { > > unsigned long textlow = (unsigned long)_stext; > > unsigned long texthigh = (unsigned long)_etext; > > unsigned long textlow_linear, texthigh_linear; > > > > if (overlaps(ptr, n, textlow, texthigh)) > > usercopy_abort("kernel text", NULL, to_user, ptr - > > textlow, n); > > > > The patch of commit: a0fa4027dc911 (riscv: Fixup static_obj() fail) broke 2th. > > > > > In general it is better idea to separate those similar to ARM64. > > > Additionally, ARM64 applies different mapping for init data & text > > > as the init data section is marked as non-executable[1] > > Yes, it's safer to protect init text & init data, but it's should be > > another patch. > > > > Yes. I will send the patch based on this fix. > > > > > > > However, we don't modify any permission for any init sections. Should > > > we do that as well ? > > Agree, we should do that. > > > > > > > > [1] https://patchwork.kernel.org/patch/9572869/ > > > > > > > /* Start of data section */ > > > > _sdata = .; > > > > RO_DATA(SECTION_ALIGN) > > > > > > > > On Thu, Sep 24, 2020 at 3:36 PM Andreas Schwab <schwab@linux-m68k.org> wrote: > > > > > > > > > > On Sep 14 2020, Aurelien Jarno wrote: > > > > > > > > > > > How should we proceed to get that fixed in time for 5.9? For the older > > > > > > branches where it has been backported (so far 5.7 and 5.8), should we > > > > > > just get that commit reverted instead? > > > > > > > > > > Can this please be resolved ASAP? > > > > > > > > > > Andreas. > > > > > > > > > > -- > > > > > Andreas Schwab, schwab@linux-m68k.org > > > > > GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 > > > > > "And now for something completely different." > > > > > > > > > > > > > > > > -- > > > > Best Regards > > > > Guo Ren > > > > > > > > ML: https://lore.kernel.org/linux-csky/ > > > > > > > > _______________________________________________ > > > > linux-riscv mailing list > > > > linux-riscv@lists.infradead.org > > > > http://lists.infradead.org/mailman/listinfo/linux-riscv > > > > > > > > > > > > -- > > > Regards, > > > Atish > > > > > > > > -- > > Best Regards > > Guo Ren > > > > ML: https://lore.kernel.org/linux-csky/ > > > > -- > Regards, > Atish -- Best Regards Guo Ren ML: https://lore.kernel.org/linux-csky/ _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-09-14 10:38 ` Aurelien Jarno 2020-09-24 7:36 ` Andreas Schwab @ 2020-10-05 8:25 ` Andreas Schwab 2020-10-05 16:39 ` Palmer Dabbelt 1 sibling, 1 reply; 24+ messages in thread From: Andreas Schwab @ 2020-10-05 8:25 UTC (permalink / raw) To: Guo Ren Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt, Linux Kernel Mailing List, linux-csky, Guo Ren, Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv On Sep 14 2020, Aurelien Jarno wrote: > How should we proceed to get that fixed in time for 5.9? For the older > branches where it has been backported (so far 5.7 and 5.8), should we > just get that commit reverted instead? Why is this still broken? Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-10-05 8:25 ` Andreas Schwab @ 2020-10-05 16:39 ` Palmer Dabbelt 2020-10-05 18:40 ` Andreas Schwab 2020-10-06 16:55 ` Guo Ren 0 siblings, 2 replies; 24+ messages in thread From: Palmer Dabbelt @ 2020-10-05 16:39 UTC (permalink / raw) To: schwab Cc: tycho, aou, nickhu, anup, linux-kernel, linux-csky, guoren, guoren, zong.li, Paul Walmsley, greentime.hu, tglx, linux-riscv On Mon, 05 Oct 2020 01:25:22 PDT (-0700), schwab@linux-m68k.org wrote: > On Sep 14 2020, Aurelien Jarno wrote: > >> How should we proceed to get that fixed in time for 5.9? For the older >> branches where it has been backported (so far 5.7 and 5.8), should we >> just get that commit reverted instead? > > Why is this still broken? Sorry, I hadn't seen this. I'm not seeing a boot failure on 5.9-rc8 with just CONFIG_HARDENED_USERCPOY=y in addition to defconfig (on QEMU, though I doubt that's relevant here). It looks like the fix is to essentially revert this, which I'm fine with, but I'd prefer to have a failing test to make sure this doesn't break again. Guo: I don't see an actual patch (signed off and such) posted for this, do you mind posting one? Otherwise I'll take a crack at constructing the revert myself. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-10-05 16:39 ` Palmer Dabbelt @ 2020-10-05 18:40 ` Andreas Schwab 2020-10-05 19:45 ` Palmer Dabbelt 2020-10-06 16:55 ` Guo Ren 1 sibling, 1 reply; 24+ messages in thread From: Andreas Schwab @ 2020-10-05 18:40 UTC (permalink / raw) To: Palmer Dabbelt Cc: tycho, aou, nickhu, anup, linux-kernel, linux-csky, guoren, guoren, zong.li, Paul Walmsley, greentime.hu, tglx, linux-riscv On Okt 05 2020, Palmer Dabbelt wrote: > On Mon, 05 Oct 2020 01:25:22 PDT (-0700), schwab@linux-m68k.org wrote: >> On Sep 14 2020, Aurelien Jarno wrote: >> >>> How should we proceed to get that fixed in time for 5.9? For the older >>> branches where it has been backported (so far 5.7 and 5.8), should we >>> just get that commit reverted instead? >> >> Why is this still broken? > > Sorry, I hadn't seen this. I'm not seeing a boot failure on 5.9-rc8 with just > CONFIG_HARDENED_USERCPOY=y in addition to defconfig (on QEMU, though I doubt > that's relevant here). I don't see a boot failure either, but eventually you will get crashes like this, and resources are not properly released: [ 4560.936645] usercopy: Kernel memory overwrite attempt detected to kernel text (offset 241626, size 16)! [ 4560.945324] ------------[ cut here ]------------ [ 4560.949954] kernel BUG at mm/usercopy.c:99! [ 4560.954030] Kernel BUG [#1] [ 4560.956805] Modules linked in: nfsv3 nfs_acl rfkill mmc_block sf_pdma i2c_ocores virt_dma spi_sifive uio_pdrv_genirq uio loop drm drm_panel_orientation_quirks rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache af_packet mscc macsec macb ptp pps_core phylink of_mdio fixed_phy libphy pwm_sifive mmc_spi crc_itu_t crc7 of_mmc_spi mmc_core spi_bitbang sunrpc sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua [ 4560.995103] CPU: 2 PID: 23806 Comm: nis Not tainted 5.8.10-1-default #1 openSUSE Tumbleweed (unreleased) [ 4561.004563] epc: ffffffe00036140e ra : ffffffe00036140e sp : ffffffe004bc7d60 [ 4561.011679] gp : ffffffe00127ee60 tp : ffffffe1b05d0000 t0 : ffffffe001297ca0 [ 4561.018886] t1 : ffffffe001297c30 t2 : 0000000000000000 s0 : ffffffe004bc7d80 [ 4561.026093] s1 : ffffffe00003afda a0 : 000000000000005b a1 : ffffffe1f7d67588 [ 4561.033298] a2 : ffffffe1f7d6c108 a3 : 0000000000000000 a4 : ffffffe000043e80 [ 4561.040506] a5 : ffffffe1f7d6be80 a6 : 0000000000000144 a7 : 0000000000000000 [ 4561.047712] s2 : 0000000000000010 s3 : 0000000000000000 s4 : ffffffe00003afea [ 4561.054918] s5 : ffffffe1f7e00e80 s6 : 0000002af4a2c2e0 s7 : fffffffffffff000 [ 4561.062124] s8 : 0000003ffffff000 s9 : ffffffe19f985400 s10: 0000000000000010 [ 4561.069329] s11: ffffffe1f7e00e80 t3 : 0000000000038fa8 t4 : 0000000000038fa8 [ 4561.076533] t5 : 0000000000000001 t6 : ffffffe00128e062 [ 4561.081832] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003 [ 4561.089821] ---[ end trace a7c93e7595e9c2cc ]--- [ 4561.095589] BUG: Bad rss-counter state mm:00000000c54f4c29 type:MM_ANONPAGES val:1 Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1 "And now for something completely different." _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-10-05 18:40 ` Andreas Schwab @ 2020-10-05 19:45 ` Palmer Dabbelt 2020-10-05 21:12 ` Atish Patra 0 siblings, 1 reply; 24+ messages in thread From: Palmer Dabbelt @ 2020-10-05 19:45 UTC (permalink / raw) To: schwab Cc: tycho, aou, nickhu, anup, linux-kernel, linux-csky, guoren, guoren, zong.li, Paul Walmsley, greentime.hu, tglx, linux-riscv On Mon, 05 Oct 2020 11:40:54 PDT (-0700), schwab@linux-m68k.org wrote: > On Okt 05 2020, Palmer Dabbelt wrote: > >> On Mon, 05 Oct 2020 01:25:22 PDT (-0700), schwab@linux-m68k.org wrote: >>> On Sep 14 2020, Aurelien Jarno wrote: >>> >>>> How should we proceed to get that fixed in time for 5.9? For the older >>>> branches where it has been backported (so far 5.7 and 5.8), should we >>>> just get that commit reverted instead? >>> >>> Why is this still broken? >> >> Sorry, I hadn't seen this. I'm not seeing a boot failure on 5.9-rc8 with just >> CONFIG_HARDENED_USERCPOY=y in addition to defconfig (on QEMU, though I doubt >> that's relevant here). > > I don't see a boot failure either, but eventually you will get crashes > like this, and resources are not properly released: > > [ 4560.936645] usercopy: Kernel memory overwrite attempt detected to kernel text (offset 241626, size 16)! > [ 4560.945324] ------------[ cut here ]------------ > [ 4560.949954] kernel BUG at mm/usercopy.c:99! > [ 4560.954030] Kernel BUG [#1] > [ 4560.956805] Modules linked in: nfsv3 nfs_acl rfkill mmc_block sf_pdma i2c_ocores virt_dma spi_sifive uio_pdrv_genirq uio loop drm drm_panel_orientation_quirks rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache af_packet mscc macsec macb ptp pps_core phylink of_mdio fixed_phy libphy pwm_sifive mmc_spi crc_itu_t crc7 of_mmc_spi mmc_core spi_bitbang sunrpc sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua > [ 4560.995103] CPU: 2 PID: 23806 Comm: nis Not tainted 5.8.10-1-default #1 openSUSE Tumbleweed (unreleased) > [ 4561.004563] epc: ffffffe00036140e ra : ffffffe00036140e sp : ffffffe004bc7d60 > [ 4561.011679] gp : ffffffe00127ee60 tp : ffffffe1b05d0000 t0 : ffffffe001297ca0 > [ 4561.018886] t1 : ffffffe001297c30 t2 : 0000000000000000 s0 : ffffffe004bc7d80 > [ 4561.026093] s1 : ffffffe00003afda a0 : 000000000000005b a1 : ffffffe1f7d67588 > [ 4561.033298] a2 : ffffffe1f7d6c108 a3 : 0000000000000000 a4 : ffffffe000043e80 > [ 4561.040506] a5 : ffffffe1f7d6be80 a6 : 0000000000000144 a7 : 0000000000000000 > [ 4561.047712] s2 : 0000000000000010 s3 : 0000000000000000 s4 : ffffffe00003afea > [ 4561.054918] s5 : ffffffe1f7e00e80 s6 : 0000002af4a2c2e0 s7 : fffffffffffff000 > [ 4561.062124] s8 : 0000003ffffff000 s9 : ffffffe19f985400 s10: 0000000000000010 > [ 4561.069329] s11: ffffffe1f7e00e80 t3 : 0000000000038fa8 t4 : 0000000000038fa8 > [ 4561.076533] t5 : 0000000000000001 t6 : ffffffe00128e062 > [ 4561.081832] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003 > [ 4561.089821] ---[ end trace a7c93e7595e9c2cc ]--- > [ 4561.095589] BUG: Bad rss-counter state mm:00000000c54f4c29 type:MM_ANONPAGES val:1 Ah, I must have misunderstood. I guess I just assumed "init crashes" meant on boot, not just at some time later. I just sent out a patch reverting this, LMK if it fixes the issue. I have some work stuff to do, but I'll try to find some time tonight to look into fixing both of the bugs -- otherwise I'll just take the revert (assuming it does actually fix the issue for you and passes the tests). I saw Atish post after I started writing this: I agree we need to sort of the kernel's memory map, I just think it's too late for 5.9. > Andreas. _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-10-05 19:45 ` Palmer Dabbelt @ 2020-10-05 21:12 ` Atish Patra 2020-10-05 21:17 ` Palmer Dabbelt 0 siblings, 1 reply; 24+ messages in thread From: Atish Patra @ 2020-10-05 21:12 UTC (permalink / raw) To: Palmer Dabbelt Cc: linux-riscv, Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, linux-kernel@vger.kernel.org List, linux-csky, Guo Ren, Andreas Schwab, Zong Li, Paul Walmsley, Guo Ren, Thomas Gleixner, Greentime Hu On Mon, Oct 5, 2020 at 12:46 PM Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > On Mon, 05 Oct 2020 11:40:54 PDT (-0700), schwab@linux-m68k.org wrote: > > On Okt 05 2020, Palmer Dabbelt wrote: > > > >> On Mon, 05 Oct 2020 01:25:22 PDT (-0700), schwab@linux-m68k.org wrote: > >>> On Sep 14 2020, Aurelien Jarno wrote: > >>> > >>>> How should we proceed to get that fixed in time for 5.9? For the older > >>>> branches where it has been backported (so far 5.7 and 5.8), should we > >>>> just get that commit reverted instead? > >>> > >>> Why is this still broken? > >> > >> Sorry, I hadn't seen this. I'm not seeing a boot failure on 5.9-rc8 with just > >> CONFIG_HARDENED_USERCPOY=y in addition to defconfig (on QEMU, though I doubt > >> that's relevant here). > > > > I don't see a boot failure either, but eventually you will get crashes > > like this, and resources are not properly released: > > > > [ 4560.936645] usercopy: Kernel memory overwrite attempt detected to kernel text (offset 241626, size 16)! > > [ 4560.945324] ------------[ cut here ]------------ > > [ 4560.949954] kernel BUG at mm/usercopy.c:99! > > [ 4560.954030] Kernel BUG [#1] > > [ 4560.956805] Modules linked in: nfsv3 nfs_acl rfkill mmc_block sf_pdma i2c_ocores virt_dma spi_sifive uio_pdrv_genirq uio loop drm drm_panel_orientation_quirks rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache af_packet mscc macsec macb ptp pps_core phylink of_mdio fixed_phy libphy pwm_sifive mmc_spi crc_itu_t crc7 of_mmc_spi mmc_core spi_bitbang sunrpc sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua > > [ 4560.995103] CPU: 2 PID: 23806 Comm: nis Not tainted 5.8.10-1-default #1 openSUSE Tumbleweed (unreleased) > > [ 4561.004563] epc: ffffffe00036140e ra : ffffffe00036140e sp : ffffffe004bc7d60 > > [ 4561.011679] gp : ffffffe00127ee60 tp : ffffffe1b05d0000 t0 : ffffffe001297ca0 > > [ 4561.018886] t1 : ffffffe001297c30 t2 : 0000000000000000 s0 : ffffffe004bc7d80 > > [ 4561.026093] s1 : ffffffe00003afda a0 : 000000000000005b a1 : ffffffe1f7d67588 > > [ 4561.033298] a2 : ffffffe1f7d6c108 a3 : 0000000000000000 a4 : ffffffe000043e80 > > [ 4561.040506] a5 : ffffffe1f7d6be80 a6 : 0000000000000144 a7 : 0000000000000000 > > [ 4561.047712] s2 : 0000000000000010 s3 : 0000000000000000 s4 : ffffffe00003afea > > [ 4561.054918] s5 : ffffffe1f7e00e80 s6 : 0000002af4a2c2e0 s7 : fffffffffffff000 > > [ 4561.062124] s8 : 0000003ffffff000 s9 : ffffffe19f985400 s10: 0000000000000010 > > [ 4561.069329] s11: ffffffe1f7e00e80 t3 : 0000000000038fa8 t4 : 0000000000038fa8 > > [ 4561.076533] t5 : 0000000000000001 t6 : ffffffe00128e062 > > [ 4561.081832] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003 > > [ 4561.089821] ---[ end trace a7c93e7595e9c2cc ]--- > > [ 4561.095589] BUG: Bad rss-counter state mm:00000000c54f4c29 type:MM_ANONPAGES val:1 > > Ah, I must have misunderstood. I guess I just assumed "init crashes" meant on > boot, not just at some time later. I just sent out a patch reverting this, LMK > if it fixes the issue. I have some work stuff to do, but I'll try to find some > time tonight to look into fixing both of the bugs -- otherwise I'll just take > the revert (assuming it does actually fix the issue for you and passes the > tests). > > I saw Atish post after I started writing this: I agree we need to sort of the > kernel's memory map, I just think it's too late for 5.9. > Yes. It is definitely a for-next material. I will try to take a stab at this if nobody else has an objection. > > Andreas. > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv -- Regards, Atish _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-10-05 21:12 ` Atish Patra @ 2020-10-05 21:17 ` Palmer Dabbelt 0 siblings, 0 replies; 24+ messages in thread From: Palmer Dabbelt @ 2020-10-05 21:17 UTC (permalink / raw) To: atishp Cc: linux-riscv, tycho, aou, nickhu, anup, linux-kernel, linux-csky, guoren, schwab, zong.li, Paul Walmsley, guoren, tglx, greentime.hu On Mon, 05 Oct 2020 14:12:44 PDT (-0700), atishp@atishpatra.org wrote: > On Mon, Oct 5, 2020 at 12:46 PM Palmer Dabbelt <palmerdabbelt@google.com> wrote: >> >> On Mon, 05 Oct 2020 11:40:54 PDT (-0700), schwab@linux-m68k.org wrote: >> > On Okt 05 2020, Palmer Dabbelt wrote: >> > >> >> On Mon, 05 Oct 2020 01:25:22 PDT (-0700), schwab@linux-m68k.org wrote: >> >>> On Sep 14 2020, Aurelien Jarno wrote: >> >>> >> >>>> How should we proceed to get that fixed in time for 5.9? For the older >> >>>> branches where it has been backported (so far 5.7 and 5.8), should we >> >>>> just get that commit reverted instead? >> >>> >> >>> Why is this still broken? >> >> >> >> Sorry, I hadn't seen this. I'm not seeing a boot failure on 5.9-rc8 with just >> >> CONFIG_HARDENED_USERCPOY=y in addition to defconfig (on QEMU, though I doubt >> >> that's relevant here). >> > >> > I don't see a boot failure either, but eventually you will get crashes >> > like this, and resources are not properly released: >> > >> > [ 4560.936645] usercopy: Kernel memory overwrite attempt detected to kernel text (offset 241626, size 16)! >> > [ 4560.945324] ------------[ cut here ]------------ >> > [ 4560.949954] kernel BUG at mm/usercopy.c:99! >> > [ 4560.954030] Kernel BUG [#1] >> > [ 4560.956805] Modules linked in: nfsv3 nfs_acl rfkill mmc_block sf_pdma i2c_ocores virt_dma spi_sifive uio_pdrv_genirq uio loop drm drm_panel_orientation_quirks rpcsec_gss_krb5 auth_rpcgss nfsv4 dns_resolver nfs lockd grace fscache af_packet mscc macsec macb ptp pps_core phylink of_mdio fixed_phy libphy pwm_sifive mmc_spi crc_itu_t crc7 of_mmc_spi mmc_core spi_bitbang sunrpc sg dm_multipath dm_mod scsi_dh_rdac scsi_dh_emc scsi_dh_alua >> > [ 4560.995103] CPU: 2 PID: 23806 Comm: nis Not tainted 5.8.10-1-default #1 openSUSE Tumbleweed (unreleased) >> > [ 4561.004563] epc: ffffffe00036140e ra : ffffffe00036140e sp : ffffffe004bc7d60 >> > [ 4561.011679] gp : ffffffe00127ee60 tp : ffffffe1b05d0000 t0 : ffffffe001297ca0 >> > [ 4561.018886] t1 : ffffffe001297c30 t2 : 0000000000000000 s0 : ffffffe004bc7d80 >> > [ 4561.026093] s1 : ffffffe00003afda a0 : 000000000000005b a1 : ffffffe1f7d67588 >> > [ 4561.033298] a2 : ffffffe1f7d6c108 a3 : 0000000000000000 a4 : ffffffe000043e80 >> > [ 4561.040506] a5 : ffffffe1f7d6be80 a6 : 0000000000000144 a7 : 0000000000000000 >> > [ 4561.047712] s2 : 0000000000000010 s3 : 0000000000000000 s4 : ffffffe00003afea >> > [ 4561.054918] s5 : ffffffe1f7e00e80 s6 : 0000002af4a2c2e0 s7 : fffffffffffff000 >> > [ 4561.062124] s8 : 0000003ffffff000 s9 : ffffffe19f985400 s10: 0000000000000010 >> > [ 4561.069329] s11: ffffffe1f7e00e80 t3 : 0000000000038fa8 t4 : 0000000000038fa8 >> > [ 4561.076533] t5 : 0000000000000001 t6 : ffffffe00128e062 >> > [ 4561.081832] status: 0000000200000120 badaddr: 0000000000000000 cause: 0000000000000003 >> > [ 4561.089821] ---[ end trace a7c93e7595e9c2cc ]--- >> > [ 4561.095589] BUG: Bad rss-counter state mm:00000000c54f4c29 type:MM_ANONPAGES val:1 >> >> Ah, I must have misunderstood. I guess I just assumed "init crashes" meant on >> boot, not just at some time later. I just sent out a patch reverting this, LMK >> if it fixes the issue. I have some work stuff to do, but I'll try to find some >> time tonight to look into fixing both of the bugs -- otherwise I'll just take >> the revert (assuming it does actually fix the issue for you and passes the >> tests). >> >> I saw Atish post after I started writing this: I agree we need to sort of the >> kernel's memory map, I just think it's too late for 5.9. >> > > Yes. It is definitely a for-next material. I will try to take a stab > at this if nobody else has an objection. WFM. Thanks! > >> > Andreas. >> >> _______________________________________________ >> linux-riscv mailing list >> linux-riscv@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/linux-riscv _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 1/3] riscv: Fixup static_obj() fail 2020-10-05 16:39 ` Palmer Dabbelt 2020-10-05 18:40 ` Andreas Schwab @ 2020-10-06 16:55 ` Guo Ren 1 sibling, 0 replies; 24+ messages in thread From: Guo Ren @ 2020-10-06 16:55 UTC (permalink / raw) To: Palmer Dabbelt Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Linux Kernel Mailing List, linux-csky, Guo Ren, Andreas Schwab, Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv On Tue, Oct 6, 2020 at 12:39 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > On Mon, 05 Oct 2020 01:25:22 PDT (-0700), schwab@linux-m68k.org wrote: > > On Sep 14 2020, Aurelien Jarno wrote: > > > >> How should we proceed to get that fixed in time for 5.9? For the older > >> branches where it has been backported (so far 5.7 and 5.8), should we > >> just get that commit reverted instead? > > > > Why is this still broken? > > Sorry, I hadn't seen this. I'm not seeing a boot failure on 5.9-rc8 with just > CONFIG_HARDENED_USERCPOY=y in addition to defconfig (on QEMU, though I doubt > that's relevant here). It looks like the fix is to essentially revert this, > which I'm fine with, but I'd prefer to have a failing test to make sure this > doesn't break again. > > Guo: I don't see an actual patch (signed off and such) posted for this, do you > mind posting one? Otherwise I'll take a crack at constructing the revert > myself. Please have a look: https://lore.kernel.org/linux-riscv/1602002973-92934-1-git-send-email-guoren@kernel.org/T/#u The only revert couldn't solve the static_obj problem. -- Best Regards Guo Ren ML: https://lore.kernel.org/linux-csky/ _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH V2 2/3] riscv: Fixup lockdep_assert_held with wrong param cpu_running 2020-06-27 13:57 [PATCH V2 0/3] riscv: Enable LOCKDEP guoren 2020-06-27 13:57 ` [PATCH V2 1/3] riscv: Fixup static_obj() fail guoren @ 2020-06-27 13:57 ` guoren 2020-09-29 22:12 ` Atish Patra 2020-06-27 13:57 ` [PATCH V2 3/3] riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT guoren 2020-07-09 22:06 ` [PATCH V2 0/3] riscv: Enable LOCKDEP Palmer Dabbelt 3 siblings, 1 reply; 24+ messages in thread From: guoren @ 2020-06-27 13:57 UTC (permalink / raw) To: palmerdabbelt, paul.walmsley, anup, greentime.hu, zong.li, aou, tglx, tycho, nickhu Cc: linux-riscv, Guo Ren, linux-kernel, linux-csky From: Zong Li <zong.li@sifive.com> The cpu_running is not a lock-class, it lacks the dep_map member in completion. It causes the error as follow: arch/riscv/kernel/smpboot.c: In function '__cpu_up': ./include/linux/lockdep.h:364:52: error: 'struct completion' has no member named 'dep_map' 364 | #define lockdep_is_held(lock) lock_is_held(&(lock)->dep_map) | ^~ ./include/asm-generic/bug.h:113:25: note: in definition of macro 'WARN_ON' 113 | int __ret_warn_on = !!(condition); \ | ^~~~~~~~~ ./include/linux/lockdep.h:390:27: note: in expansion of macro 'lockdep_is_held' 390 | WARN_ON(debug_locks && !lockdep_is_held(l)); \ | ^~~~~~~~~~~~~~~ arch/riscv/kernel/smpboot.c:118:2: note: in expansion of macro 'lockdep_assert_held' 118 | lockdep_assert_held(&cpu_running); There are a lot of archs which use cpu_running in smpboot.c (arm, arm64, openrisc, xtensa, s390, x86, mips), but none of them try lockdep_assert_held(&cpu_running.wait.lock). So Just remove it. Signed-off-by: Zong Li <zong.li@sifive.com> Signed-off-by: Guo Ren <guoren@linux.alibaba.com> --- arch/riscv/kernel/smpboot.c | 1 - 1 file changed, 1 deletion(-) diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c index 4e99227..defc4e1 100644 --- a/arch/riscv/kernel/smpboot.c +++ b/arch/riscv/kernel/smpboot.c @@ -121,7 +121,6 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle) ret = start_secondary_cpu(cpu, tidle); if (!ret) { - lockdep_assert_held(&cpu_running); wait_for_completion_timeout(&cpu_running, msecs_to_jiffies(1000)); -- 2.7.4 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH V2 2/3] riscv: Fixup lockdep_assert_held with wrong param cpu_running 2020-06-27 13:57 ` [PATCH V2 2/3] riscv: Fixup lockdep_assert_held with wrong param cpu_running guoren @ 2020-09-29 22:12 ` Atish Patra 0 siblings, 0 replies; 24+ messages in thread From: Atish Patra @ 2020-09-29 22:12 UTC (permalink / raw) To: Guo Ren Cc: tycho, Albert Ou, Nick Hu, Anup Patel, Palmer Dabbelt, linux-kernel@vger.kernel.org List, linux-csky, Guo Ren, Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv On Sat, Jun 27, 2020 at 6:58 AM <guoren@kernel.org> wrote: > > From: Zong Li <zong.li@sifive.com> > > The cpu_running is not a lock-class, it lacks the dep_map member in > completion. It causes the error as follow: > > arch/riscv/kernel/smpboot.c: In function '__cpu_up': > ./include/linux/lockdep.h:364:52: error: 'struct completion' has no member named 'dep_map' > 364 | #define lockdep_is_held(lock) lock_is_held(&(lock)->dep_map) > | ^~ > ./include/asm-generic/bug.h:113:25: note: in definition of macro 'WARN_ON' > 113 | int __ret_warn_on = !!(condition); \ > | ^~~~~~~~~ > ./include/linux/lockdep.h:390:27: note: in expansion of macro 'lockdep_is_held' > 390 | WARN_ON(debug_locks && !lockdep_is_held(l)); \ > | ^~~~~~~~~~~~~~~ > arch/riscv/kernel/smpboot.c:118:2: note: in expansion of macro 'lockdep_assert_held' > 118 | lockdep_assert_held(&cpu_running); > > There are a lot of archs which use cpu_running in smpboot.c (arm, > arm64, openrisc, xtensa, s390, x86, mips), but none of them try > lockdep_assert_held(&cpu_running.wait.lock). So Just remove it. > > Signed-off-by: Zong Li <zong.li@sifive.com> > Signed-off-by: Guo Ren <guoren@linux.alibaba.com> > --- > arch/riscv/kernel/smpboot.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c > index 4e99227..defc4e1 100644 > --- a/arch/riscv/kernel/smpboot.c > +++ b/arch/riscv/kernel/smpboot.c > @@ -121,7 +121,6 @@ int __cpu_up(unsigned int cpu, struct task_struct *tidle) > > ret = start_secondary_cpu(cpu, tidle); > if (!ret) { > - lockdep_assert_held(&cpu_running); > wait_for_completion_timeout(&cpu_running, > msecs_to_jiffies(1000)); > > -- > 2.7.4 > > > _______________________________________________ > linux-riscv mailing list > linux-riscv@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-riscv Thanks for catching this. Reviewed-by: Atish Patra <atish.patra@wdc.com> -- Regards, Atish _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH V2 3/3] riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT 2020-06-27 13:57 [PATCH V2 0/3] riscv: Enable LOCKDEP guoren 2020-06-27 13:57 ` [PATCH V2 1/3] riscv: Fixup static_obj() fail guoren 2020-06-27 13:57 ` [PATCH V2 2/3] riscv: Fixup lockdep_assert_held with wrong param cpu_running guoren @ 2020-06-27 13:57 ` guoren 2020-07-09 22:06 ` [PATCH V2 0/3] riscv: Enable LOCKDEP Palmer Dabbelt 3 siblings, 0 replies; 24+ messages in thread From: guoren @ 2020-06-27 13:57 UTC (permalink / raw) To: palmerdabbelt, paul.walmsley, anup, greentime.hu, zong.li, aou, tglx, tycho, nickhu Cc: linux-riscv, Guo Ren, linux-kernel, linux-csky From: Guo Ren <guoren@linux.alibaba.com> Lockdep is needed by proving the spinlocks and rwlocks. To suupport it, we need fixup TRACE_IRQFLAGS_SUPPORT in kernel/entry.S. This patch follow Documentation/irqflags-tracing.txt. Signed-off-by: Guo Ren <guoren@linux.alibaba.com> Cc: Greentime Hu <greentime.hu@sifive.com> --- arch/riscv/Kconfig | 3 +++ arch/riscv/kernel/entry.S | 34 +++++++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index 58d6f66..d5d5100 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -179,6 +179,9 @@ config PGTABLE_LEVELS default 3 if 64BIT default 2 +config LOCKDEP_SUPPORT + def_bool y + source "arch/riscv/Kconfig.socs" menu "Platform type" diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S index cae7e6d..45c81e4 100644 --- a/arch/riscv/kernel/entry.S +++ b/arch/riscv/kernel/entry.S @@ -97,19 +97,26 @@ _save_context: la gp, __global_pointer$ .option pop - la ra, ret_from_exception +#ifdef CONFIG_TRACE_IRQFLAGS + call trace_hardirqs_off +#endif /* * MSB of cause differentiates between * interrupts and exceptions */ bge s4, zero, 1f + la ra, ret_from_exception + /* Handle interrupts */ move a0, sp /* pt_regs */ la a1, handle_arch_irq REG_L a1, (a1) jr a1 1: +#ifdef CONFIG_TRACE_IRQFLAGS + call trace_hardirqs_on +#endif /* * Exceptions run with interrupts enabled or disabled depending on the * state of SR_PIE in m/sstatus. @@ -119,6 +126,7 @@ _save_context: csrs CSR_STATUS, SR_IE 1: + la ra, ret_from_exception /* Handle syscalls */ li t0, EXC_SYSCALL beq s4, t0, handle_syscall @@ -137,6 +145,17 @@ _save_context: tail do_trap_unknown handle_syscall: +#ifdef CONFIG_TRACE_IRQFLAGS + /* Recover a0 - a7 for system calls */ + REG_L a0, PT_A0(sp) + REG_L a1, PT_A1(sp) + REG_L a2, PT_A2(sp) + REG_L a3, PT_A3(sp) + REG_L a4, PT_A4(sp) + REG_L a5, PT_A5(sp) + REG_L a6, PT_A6(sp) + REG_L a7, PT_A7(sp) +#endif /* save the initial A0 value (needed in signal handlers) */ REG_S a0, PT_ORIG_A0(sp) /* @@ -190,6 +209,9 @@ ret_from_syscall_rejected: ret_from_exception: REG_L s0, PT_STATUS(sp) csrc CSR_STATUS, SR_IE +#ifdef CONFIG_TRACE_IRQFLAGS + call trace_hardirqs_off +#endif #ifdef CONFIG_RISCV_M_MODE /* the MPP value is too large to be used as an immediate arg for addi */ li t0, SR_MPP @@ -216,6 +238,16 @@ resume_userspace: csrw CSR_SCRATCH, tp restore_all: +#ifdef CONFIG_TRACE_IRQFLAGS + REG_L s1, PT_STATUS(sp) + andi t0, s1, SR_PIE + beqz t0, 1f + call trace_hardirqs_on + j 2f +1: + call trace_hardirqs_off +2: +#endif REG_L a0, PT_STATUS(sp) /* * The current load reservation is effectively part of the processor's -- 2.7.4 _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply related [flat|nested] 24+ messages in thread
* Re: [PATCH V2 0/3] riscv: Enable LOCKDEP 2020-06-27 13:57 [PATCH V2 0/3] riscv: Enable LOCKDEP guoren ` (2 preceding siblings ...) 2020-06-27 13:57 ` [PATCH V2 3/3] riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT guoren @ 2020-07-09 22:06 ` Palmer Dabbelt 2020-07-09 23:15 ` Guo Ren 3 siblings, 1 reply; 24+ messages in thread From: Palmer Dabbelt @ 2020-07-09 22:06 UTC (permalink / raw) To: guoren Cc: tycho, aou, nickhu, anup, linux-kernel, linux-csky, guoren, zong.li, Paul Walmsley, greentime.hu, tglx, linux-riscv On Sat, 27 Jun 2020 06:57:05 PDT (-0700), guoren@kernel.org wrote: > From: Guo Ren <guoren@linux.alibaba.com> > > Lockdep is needed by proving the spinlocks and rwlocks. To support it, > we need to add TRACE_IRQFLAGS codes in kernel/entry.S. These patches > follow Documentation/irqflags-tracing.txt. > > Fixup 2 bugs that block the lockdep implementation. > > --- > Changes in v2 > - Remove sX regs recovery codes which are unnecessary, because > callee will handle them. Thx Greentime :) > > - Move "restore a0 - a7" to handle_syscall, but if _TIF_SYSCALL_WORK > is set, "restore a1 - a7" is still duplicated. I prefer a C wrapper > for syscall. > > Guo Ren (2): > riscv: Fixup static_obj() fail > riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT > > Zong Li (1): > riscv: Fixup lockdep_assert_held with wrong param cpu_running > > arch/riscv/Kconfig | 3 +++ > arch/riscv/kernel/entry.S | 33 ++++++++++++++++++++++++++++++++- > arch/riscv/kernel/smpboot.c | 1 - > arch/riscv/kernel/vmlinux.lds.S | 2 +- > 4 files changed, 36 insertions(+), 3 deletions(-) These are on for-next. As far as I can tell lockdep is working, but I'm just doing some simple boot tests. Thanks! _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH V2 0/3] riscv: Enable LOCKDEP 2020-07-09 22:06 ` [PATCH V2 0/3] riscv: Enable LOCKDEP Palmer Dabbelt @ 2020-07-09 23:15 ` Guo Ren 0 siblings, 0 replies; 24+ messages in thread From: Guo Ren @ 2020-07-09 23:15 UTC (permalink / raw) To: Palmer Dabbelt Cc: Tycho Andersen, Albert Ou, Nick Hu, Anup Patel, Linux Kernel Mailing List, linux-csky, Guo Ren, Zong Li, Paul Walmsley, Greentime Hu, Thomas Gleixner, linux-riscv Thank you, Palmer On Fri, Jul 10, 2020 at 6:06 AM Palmer Dabbelt <palmerdabbelt@google.com> wrote: > > On Sat, 27 Jun 2020 06:57:05 PDT (-0700), guoren@kernel.org wrote: > > From: Guo Ren <guoren@linux.alibaba.com> > > > > Lockdep is needed by proving the spinlocks and rwlocks. To support it, > > we need to add TRACE_IRQFLAGS codes in kernel/entry.S. These patches > > follow Documentation/irqflags-tracing.txt. > > > > Fixup 2 bugs that block the lockdep implementation. > > > > --- > > Changes in v2 > > - Remove sX regs recovery codes which are unnecessary, because > > callee will handle them. Thx Greentime :) > > > > - Move "restore a0 - a7" to handle_syscall, but if _TIF_SYSCALL_WORK > > is set, "restore a1 - a7" is still duplicated. I prefer a C wrapper > > for syscall. > > > > Guo Ren (2): > > riscv: Fixup static_obj() fail > > riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT > > > > Zong Li (1): > > riscv: Fixup lockdep_assert_held with wrong param cpu_running > > > > arch/riscv/Kconfig | 3 +++ > > arch/riscv/kernel/entry.S | 33 ++++++++++++++++++++++++++++++++- > > arch/riscv/kernel/smpboot.c | 1 - > > arch/riscv/kernel/vmlinux.lds.S | 2 +- > > 4 files changed, 36 insertions(+), 3 deletions(-) > > These are on for-next. As far as I can tell lockdep is working, but I'm just > doing some simple boot tests. > > Thanks! -- Best Regards Guo Ren ML: https://lore.kernel.org/linux-csky/ _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv ^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2020-10-07 14:45 UTC | newest] Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-06-27 13:57 [PATCH V2 0/3] riscv: Enable LOCKDEP guoren 2020-06-27 13:57 ` [PATCH V2 1/3] riscv: Fixup static_obj() fail guoren 2020-09-11 20:45 ` Aurelien Jarno 2020-09-12 2:39 ` Guo Ren 2020-09-14 10:38 ` Aurelien Jarno 2020-09-24 7:36 ` Andreas Schwab 2020-09-24 16:19 ` Guo Ren 2020-09-29 18:51 ` Aurelien Jarno 2020-10-05 19:14 ` Atish Patra 2020-10-06 16:46 ` Guo Ren 2020-10-06 20:38 ` Atish Patra 2020-10-07 14:45 ` Guo Ren 2020-10-05 8:25 ` Andreas Schwab 2020-10-05 16:39 ` Palmer Dabbelt 2020-10-05 18:40 ` Andreas Schwab 2020-10-05 19:45 ` Palmer Dabbelt 2020-10-05 21:12 ` Atish Patra 2020-10-05 21:17 ` Palmer Dabbelt 2020-10-06 16:55 ` Guo Ren 2020-06-27 13:57 ` [PATCH V2 2/3] riscv: Fixup lockdep_assert_held with wrong param cpu_running guoren 2020-09-29 22:12 ` Atish Patra 2020-06-27 13:57 ` [PATCH V2 3/3] riscv: Enable LOCKDEP_SUPPORT & fixup TRACE_IRQFLAGS_SUPPORT guoren 2020-07-09 22:06 ` [PATCH V2 0/3] riscv: Enable LOCKDEP Palmer Dabbelt 2020-07-09 23:15 ` Guo Ren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).