* [PATCH 0/2] arm64: entry: Move ct_user_exit calls earlier
@ 2019-08-01 15:41 James Morse
2019-08-01 15:41 ` [PATCH 1/2] arm64: entry: Move ct_user_exit before any user of RCU James Morse
2019-08-01 15:41 ` [PATCH 2/2] arm64: entry: Move ct_user_exit before we can take another exception James Morse
0 siblings, 2 replies; 5+ messages in thread
From: James Morse @ 2019-08-01 15:41 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Catalin Marinas, Will Deacon, James Morse, Masami Hiramatsu
Hi,
Masami's kprobes series adds a bunch of warnings to do_debug_exception().
When booted with nohz_full and lockdep, these things go off. This is
because the entry code is calling some of the C handlers before
ct_user_exit.
Patch 2 is for consistency, it doesn't look like anything minds
this today.
Thanks,
James Morse (2):
arm64: entry: Move ct_user_exit before any user of RCU
arm64: entry: Move ct_user_exit before we can take another exception
arch/arm64/kernel/entry.S | 32 +++++++++++++++++---------------
1 file changed, 17 insertions(+), 15 deletions(-)
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] arm64: entry: Move ct_user_exit before any user of RCU
2019-08-01 15:41 [PATCH 0/2] arm64: entry: Move ct_user_exit calls earlier James Morse
@ 2019-08-01 15:41 ` James Morse
2019-08-01 15:41 ` [PATCH 2/2] arm64: entry: Move ct_user_exit before we can take another exception James Morse
1 sibling, 0 replies; 5+ messages in thread
From: James Morse @ 2019-08-01 15:41 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Catalin Marinas, Will Deacon, James Morse, Masami Hiramatsu
When taking an SError or Debug exception from EL0, we run the C
handler for these exceptions before updating the context tracking
code and unmasking lower priority interrupts.
When booting with nohz_full lockdep tells us we got this wrong:
| =============================
| WARNING: suspicious RCU usage
| 5.3.0-rc2-00010-gb4b5e9dcb11b-dirty #11271 Not tainted
| -----------------------------
| include/linux/rcupdate.h:643 rcu_read_unlock() used illegally wh!
|
| other info that might help us debug this:
|
|
| RCU used illegally from idle CPU!
| rcu_scheduler_active = 2, debug_locks = 1
| RCU used illegally from extended quiescent state!
| 1 lock held by a.out/432:
| #0: 00000000c7a79515 (rcu_read_lock){....}, at: brk_handler+0x00
|
| stack backtrace:
| CPU: 1 PID: 432 Comm: a.out Not tainted 5.3.0-rc2-00010-gb4b5e9d1
| Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno De8
| Call trace:
| dump_backtrace+0x0/0x140
| show_stack+0x14/0x20
| dump_stack+0xbc/0x104
| lockdep_rcu_suspicious+0xf8/0x108
| brk_handler+0x164/0x1b0
| do_debug_exception+0x11c/0x278
| el0_dbg+0x14/0x20
Move the ct_user_exit calls to be before do_debug_exception()
and do_serror() respectively.
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Fixes: 6c81fe7925cc4c42 ("arm64: enable context tracking")
Signed-off-by: James Morse <james.morse@arm.com>
---
The context to this is Masami Hiramatsu's patch adding sanity checks
for this:
Link: https://lore.kernel.org/r/156466954920.8995.14798868044005509434.stgit@devnote2
---
arch/arm64/kernel/entry.S | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 320a30dbe35e..28681034d599 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -902,13 +902,14 @@ el0_dbg:
* Debug exception handling
*/
tbnz x24, #0, el0_inv // EL0 only
+ mrs x24, far_el1
gic_prio_kentry_setup tmp=x3
- mrs x0, far_el1
+ ct_user_exit
+ mov x0, x24
mov x1, x25
mov x2, sp
bl do_debug_exception
enable_da_f
- ct_user_exit
b ret_to_user
el0_inv:
enable_daif
@@ -958,13 +959,14 @@ ENDPROC(el1_error)
el0_error:
kernel_entry 0
el0_error_naked:
- mrs x1, esr_el1
+ mrs x25, esr_el1
gic_prio_kentry_setup tmp=x2
+ ct_user_exit
enable_dbg
mov x0, sp
+ mov x1, x25
bl do_serror
enable_da_f
- ct_user_exit
b ret_to_user
ENDPROC(el0_error)
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] arm64: entry: Move ct_user_exit before we can take another exception
2019-08-01 15:41 [PATCH 0/2] arm64: entry: Move ct_user_exit calls earlier James Morse
2019-08-01 15:41 ` [PATCH 1/2] arm64: entry: Move ct_user_exit before any user of RCU James Morse
@ 2019-08-01 15:41 ` James Morse
2019-08-01 16:39 ` Will Deacon
1 sibling, 1 reply; 5+ messages in thread
From: James Morse @ 2019-08-01 15:41 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Catalin Marinas, Will Deacon, James Morse, Masami Hiramatsu
When taking an exception from EL0 we unmask exceptions before calling
ct_user_exit. This means we could take an interrupt or be single-stepped
in the gap. The entry from EL1 code sees that we took an exception from
the kernel, whereas the context_tracking code believes we are still in
user-space.
To keep these things consistent, move the ct_user_exit calls before
any unmask of exceptions.
Fixes: 6c81fe7925cc4c42 ("arm64: enable context tracking")
Signed-off-by: James Morse <james.morse@arm.com>
---
arch/arm64/kernel/entry.S | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 28681034d599..88f4ab21cb66 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -792,8 +792,8 @@ el0_cp15:
/*
* Trapped CP15 (MRC, MCR, MRRC, MCRR) instructions
*/
- enable_daif
ct_user_exit
+ enable_daif
mov x0, x25
mov x1, sp
bl do_cp15instr
@@ -805,8 +805,8 @@ el0_da:
* Data abort handling
*/
mrs x26, far_el1
- enable_daif
ct_user_exit
+ enable_daif
clear_address_tag x0, x26
mov x1, x25
mov x2, sp
@@ -818,11 +818,11 @@ el0_ia:
*/
mrs x26, far_el1
gic_prio_kentry_setup tmp=x0
+ ct_user_exit
enable_da_f
#ifdef CONFIG_TRACE_IRQFLAGS
bl trace_hardirqs_off
#endif
- ct_user_exit
mov x0, x26
mov x1, x25
mov x2, sp
@@ -832,8 +832,8 @@ el0_fpsimd_acc:
/*
* Floating Point or Advanced SIMD access
*/
- enable_daif
ct_user_exit
+ enable_daif
mov x0, x25
mov x1, sp
bl do_fpsimd_acc
@@ -842,8 +842,8 @@ el0_sve_acc:
/*
* Scalable Vector Extension access
*/
- enable_daif
ct_user_exit
+ enable_daif
mov x0, x25
mov x1, sp
bl do_sve_acc
@@ -852,8 +852,8 @@ el0_fpsimd_exc:
/*
* Floating Point, Advanced SIMD or SVE exception
*/
- enable_daif
ct_user_exit
+ enable_daif
mov x0, x25
mov x1, sp
bl do_fpsimd_exc
@@ -868,11 +868,11 @@ el0_sp_pc:
* Stack or PC alignment exception handling
*/
gic_prio_kentry_setup tmp=x0
+ ct_user_exit
enable_da_f
#ifdef CONFIG_TRACE_IRQFLAGS
bl trace_hardirqs_off
#endif
- ct_user_exit
mov x0, x26
mov x1, x25
mov x2, sp
@@ -882,8 +882,8 @@ el0_undef:
/*
* Undefined instruction
*/
- enable_daif
ct_user_exit
+ enable_daif
mov x0, sp
bl do_undefinstr
b ret_to_user
@@ -891,8 +891,8 @@ el0_sys:
/*
* System instructions, for trapped cache maintenance instructions
*/
- enable_daif
ct_user_exit
+ enable_daif
mov x0, x25
mov x1, sp
bl do_sysinstr
@@ -912,8 +912,8 @@ el0_dbg:
enable_da_f
b ret_to_user
el0_inv:
- enable_daif
ct_user_exit
+ enable_daif
mov x0, sp
mov x1, #BAD_SYNC
mov x2, x25
@@ -926,13 +926,13 @@ el0_irq:
kernel_entry 0
el0_irq_naked:
gic_prio_irq_setup pmr=x20, tmp=x0
+ ct_user_exit
enable_da_f
#ifdef CONFIG_TRACE_IRQFLAGS
bl trace_hardirqs_off
#endif
- ct_user_exit
#ifdef CONFIG_HARDEN_BRANCH_PREDICTOR
tbz x22, #55, 1f
bl do_el0_irq_bp_hardening
--
2.20.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] arm64: entry: Move ct_user_exit before we can take another exception
2019-08-01 15:41 ` [PATCH 2/2] arm64: entry: Move ct_user_exit before we can take another exception James Morse
@ 2019-08-01 16:39 ` Will Deacon
2019-08-02 12:43 ` James Morse
0 siblings, 1 reply; 5+ messages in thread
From: Will Deacon @ 2019-08-01 16:39 UTC (permalink / raw)
To: James Morse; +Cc: Catalin Marinas, Masami Hiramatsu, linux-arm-kernel
On Thu, Aug 01, 2019 at 04:41:50PM +0100, James Morse wrote:
> When taking an exception from EL0 we unmask exceptions before calling
> ct_user_exit. This means we could take an interrupt or be single-stepped
> in the gap. The entry from EL1 code sees that we took an exception from
> the kernel, whereas the context_tracking code believes we are still in
> user-space.
>
> To keep these things consistent, move the ct_user_exit calls before
> any unmask of exceptions.
>
> Fixes: 6c81fe7925cc4c42 ("arm64: enable context tracking")
> Signed-off-by: James Morse <james.morse@arm.com>
> ---
> arch/arm64/kernel/entry.S | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 28681034d599..88f4ab21cb66 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -792,8 +792,8 @@ el0_cp15:
> /*
> * Trapped CP15 (MRC, MCR, MRRC, MCRR) instructions
> */
> - enable_daif
> ct_user_exit
> + enable_daif
> mov x0, x25
> mov x1, sp
> bl do_cp15instr
> @@ -805,8 +805,8 @@ el0_da:
> * Data abort handling
> */
> mrs x26, far_el1
> - enable_daif
> ct_user_exit
> + enable_daif
This strikes me as a bit dodgy, since we end up in context_tracking_exit(),
which calls local_irq_{save,restore}() and I think our accounting is
probably off at this point. I think we need to call via
user_{enter,exit}_irqoff() instead to make this work.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] arm64: entry: Move ct_user_exit before we can take another exception
2019-08-01 16:39 ` Will Deacon
@ 2019-08-02 12:43 ` James Morse
0 siblings, 0 replies; 5+ messages in thread
From: James Morse @ 2019-08-02 12:43 UTC (permalink / raw)
To: Will Deacon; +Cc: Catalin Marinas, Masami Hiramatsu, linux-arm-kernel
Hi Will,
On 01/08/2019 17:39, Will Deacon wrote:
> On Thu, Aug 01, 2019 at 04:41:50PM +0100, James Morse wrote:
>> When taking an exception from EL0 we unmask exceptions before calling
>> ct_user_exit. This means we could take an interrupt or be single-stepped
>> in the gap. The entry from EL1 code sees that we took an exception from
>> the kernel, whereas the context_tracking code believes we are still in
>> user-space.
>>
>> To keep these things consistent, move the ct_user_exit calls before
>> any unmask of exceptions.
>> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
>> index 28681034d599..88f4ab21cb66 100644
>> --- a/arch/arm64/kernel/entry.S
>> +++ b/arch/arm64/kernel/entry.S
>> @@ -792,8 +792,8 @@ el0_cp15:
>> /*
>> * Trapped CP15 (MRC, MCR, MRRC, MCRR) instructions
>> */
>> - enable_daif
>> ct_user_exit
>> + enable_daif
>> mov x0, x25
>> mov x1, sp
>> bl do_cp15instr
>> @@ -805,8 +805,8 @@ el0_da:
>> * Data abort handling
>> */
>> mrs x26, far_el1
>> - enable_daif
>> ct_user_exit
>> + enable_daif
> This strikes me as a bit dodgy, since we end up in context_tracking_exit(),
> which calls local_irq_{save,restore}() and I think our accounting is
> probably off at this point.
Huh, I'm sure I had all those options turned on...
~
local_irq_save() calls trace_hardirqs_off() unconditionally, whereas local_irq_restore()
uses the saved flags to decide interrupts aren't enabled by this restore call, so the
accounting ends up being correct. It calls trace_hardirqs_off() a second time.
The asserts check trace_hardirqs_off() is called with interrupts masked, which it is in
this case.
> I think we need to call via
> user_{enter,exit}_irqoff() instead to make this work.
Sure, that would be clearer.
Thanks,
James
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-08-02 12:43 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-01 15:41 [PATCH 0/2] arm64: entry: Move ct_user_exit calls earlier James Morse
2019-08-01 15:41 ` [PATCH 1/2] arm64: entry: Move ct_user_exit before any user of RCU James Morse
2019-08-01 15:41 ` [PATCH 2/2] arm64: entry: Move ct_user_exit before we can take another exception James Morse
2019-08-01 16:39 ` Will Deacon
2019-08-02 12:43 ` James Morse
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.