* [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler
@ 2013-08-28 11:34 Catalin Marinas
2013-08-28 11:34 ` [PATCH 1/5] arm: Move asm macro get_thread_info to asm/assembler.h Catalin Marinas
` (5 more replies)
0 siblings, 6 replies; 14+ messages in thread
From: Catalin Marinas @ 2013-08-28 11:34 UTC (permalink / raw)
To: linux-arm-kernel
The __und_usr handler accesses the user space to read the opcode of the
faulting instruction. This is currently done with interrupts disabled
but it could potentially cause a data abort of the page table was
modified from another CPU. The data abort with interrupts disabled
triggers a might_sleep() warning in do_page_fault() or, worse, deadlock
on the pte lock when TLB ops broadcasting is enabled (ARM11MPCore or A15
together with workaround for erratum 798181).
To enable interrupts earlier, the preemption needs to be disabled in the
crunch and iwmmxt handlers. The series needs testing on such hardware.
(an alternative simpler patch would be to enable the interrupts only
around the user access instructions in __und_usr and disable them
afterwards, so I would rather do it as in this series).
Catalin Marinas (5):
arm: Move asm macro get_thread_info to asm/assembler.h
arm: Add {inc,dec}_preempt_count asm macros
arm: Disable preemption in iwmmxt_task_enable()
arm: Disable preemption in crunch_task_enable()
arm: Enable IRQs before attempting to read user space in __und_usr
arch/arm/include/asm/assembler.h | 42 ++++++++++++++++++++++++++++++++++++++
arch/arm/kernel/entry-armv.S | 11 ++++++----
arch/arm/kernel/entry-header.S | 11 ----------
arch/arm/kernel/iwmmxt.S | 15 ++++++++++----
arch/arm/mach-ep93xx/crunch-bits.S | 13 +++++++++---
arch/arm/vfp/entry.S | 28 ++++++++-----------------
arch/arm/vfp/vfphw.S | 19 ++++++-----------
7 files changed, 84 insertions(+), 55 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/5] arm: Move asm macro get_thread_info to asm/assembler.h
2013-08-28 11:34 [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler Catalin Marinas
@ 2013-08-28 11:34 ` Catalin Marinas
2013-08-28 11:34 ` [PATCH 2/5] arm: Add {inc,dec}_preempt_count asm macros Catalin Marinas
` (4 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Catalin Marinas @ 2013-08-28 11:34 UTC (permalink / raw)
To: linux-arm-kernel
asm/assembler.h is a better place for this macro since it is used by
asm files outside arch/arm/kernel/
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm/include/asm/assembler.h | 10 ++++++++++
| 11 -----------
arch/arm/vfp/entry.S | 5 ++++-
arch/arm/vfp/vfphw.S | 5 ++++-
4 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index a5fef71..8d4d50a 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -167,6 +167,16 @@
restore_irqs_notrace \oldcpsr
.endm
+/*
+ * Get current thread_info.
+ */
+ .macro get_thread_info, rd
+ ARM( mov \rd, sp, lsr #13 )
+ THUMB( mov \rd, sp )
+ THUMB( lsr \rd, \rd, #13 )
+ mov \rd, \rd, lsl #13
+ .endm
+
#define USER(x...) \
9999: x; \
.pushsection __ex_table,"a"; \
--git a/arch/arm/kernel/entry-header.S b/arch/arm/kernel/entry-header.S
index de23a9b..592d58e 100644
--- a/arch/arm/kernel/entry-header.S
+++ b/arch/arm/kernel/entry-header.S
@@ -236,11 +236,6 @@
movs pc, lr @ return & move spsr_svc into cpsr
.endm
- .macro get_thread_info, rd
- mov \rd, sp, lsr #13
- mov \rd, \rd, lsl #13
- .endm
-
@
@ 32-bit wide "mov pc, reg"
@
@@ -306,12 +301,6 @@
.endm
#endif /* ifdef CONFIG_CPU_V7M / else */
- .macro get_thread_info, rd
- mov \rd, sp
- lsr \rd, \rd, #13
- mov \rd, \rd, lsl #13
- .endm
-
@
@ 32-bit wide "mov pc, reg"
@
diff --git a/arch/arm/vfp/entry.S b/arch/arm/vfp/entry.S
index 46e1749..9cc290a 100644
--- a/arch/arm/vfp/entry.S
+++ b/arch/arm/vfp/entry.S
@@ -8,9 +8,12 @@
* it under the terms of the GNU General Public License version 2 as
* published by the Free Software Foundation.
*/
+#include <linux/init.h>
+#include <linux/linkage.h>
#include <asm/thread_info.h>
#include <asm/vfpmacros.h>
-#include "../kernel/entry-header.S"
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
@ VFP entry point.
@
diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
index 8d10dc8..33d82aa 100644
--- a/arch/arm/vfp/vfphw.S
+++ b/arch/arm/vfp/vfphw.S
@@ -14,10 +14,13 @@
* r10 points at the start of the private FP workspace in the thread structure
* sp points to a struct pt_regs (as defined in include/asm/proc/ptrace.h)
*/
+#include <linux/init.h>
+#include <linux/linkage.h>
#include <asm/thread_info.h>
#include <asm/vfpmacros.h>
#include <linux/kern_levels.h>
-#include "../kernel/entry-header.S"
+#include <asm/assembler.h>
+#include <asm/asm-offsets.h>
.macro DBGSTR, str
#ifdef DEBUG
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/5] arm: Add {inc,dec}_preempt_count asm macros
2013-08-28 11:34 [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler Catalin Marinas
2013-08-28 11:34 ` [PATCH 1/5] arm: Move asm macro get_thread_info to asm/assembler.h Catalin Marinas
@ 2013-08-28 11:34 ` Catalin Marinas
2013-08-28 11:34 ` [PATCH 3/5] arm: Disable preemption in iwmmxt_task_enable() Catalin Marinas
` (3 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Catalin Marinas @ 2013-08-28 11:34 UTC (permalink / raw)
To: linux-arm-kernel
The patch adds asm macros for inc_preempt_count and dec_preempt_count_ti
(which also gets the current thread_info) instead of open-coding them in
arch/arm/vfp/*.S files.
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm/include/asm/assembler.h | 32 ++++++++++++++++++++++++++++++++
arch/arm/vfp/entry.S | 20 +++-----------------
arch/arm/vfp/vfphw.S | 14 ++------------
3 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/arch/arm/include/asm/assembler.h b/arch/arm/include/asm/assembler.h
index 8d4d50a..29ec0b6 100644
--- a/arch/arm/include/asm/assembler.h
+++ b/arch/arm/include/asm/assembler.h
@@ -23,6 +23,7 @@
#include <asm/ptrace.h>
#include <asm/domain.h>
#include <asm/opcodes-virt.h>
+#include <asm/asm-offsets.h>
#define IOMEM(x) (x)
@@ -177,6 +178,37 @@
mov \rd, \rd, lsl #13
.endm
+/*
+ * Increment/decrement the preempt count.
+ */
+#ifdef CONFIG_PREEMPT_COUNT
+ .macro inc_preempt_count, ti, tmp
+ ldr \tmp, [\ti, #TI_PREEMPT] @ get preempt count
+ add \tmp, \tmp, #1 @ increment it
+ str \tmp, [\ti, #TI_PREEMPT]
+ .endm
+
+ .macro dec_preempt_count, ti, tmp
+ ldr \tmp, [\ti, #TI_PREEMPT] @ get preempt count
+ sub \tmp, \tmp, #1 @ decrement it
+ str \tmp, [\ti, #TI_PREEMPT]
+ .endm
+
+ .macro dec_preempt_count_ti, ti, tmp
+ get_thread_info \ti
+ dec_preempt_count \ti, \tmp
+ .endm
+#else
+ .macro inc_preempt_count, ti, tmp
+ .endm
+
+ .macro dec_preempt_count, ti, tmp
+ .endm
+
+ .macro dec_preempt_count_ti, ti, tmp
+ .endm
+#endif
+
#define USER(x...) \
9999: x; \
.pushsection __ex_table,"a"; \
diff --git a/arch/arm/vfp/entry.S b/arch/arm/vfp/entry.S
index 9cc290a..f0759e7 100644
--- a/arch/arm/vfp/entry.S
+++ b/arch/arm/vfp/entry.S
@@ -25,11 +25,7 @@
@ IRQs disabled.
@
ENTRY(do_vfp)
-#ifdef CONFIG_PREEMPT_COUNT
- ldr r4, [r10, #TI_PREEMPT] @ get preempt count
- add r11, r4, #1 @ increment it
- str r11, [r10, #TI_PREEMPT]
-#endif
+ inc_preempt_count r10, r4
enable_irq
ldr r4, .LCvfp
ldr r11, [r10, #TI_CPU] @ CPU number
@@ -38,12 +34,7 @@ ENTRY(do_vfp)
ENDPROC(do_vfp)
ENTRY(vfp_null_entry)
-#ifdef CONFIG_PREEMPT_COUNT
- get_thread_info r10
- ldr r4, [r10, #TI_PREEMPT] @ get preempt count
- sub r11, r4, #1 @ decrement it
- str r11, [r10, #TI_PREEMPT]
-#endif
+ dec_preempt_count_ti r10, r4
mov pc, lr
ENDPROC(vfp_null_entry)
@@ -56,12 +47,7 @@ ENDPROC(vfp_null_entry)
__INIT
ENTRY(vfp_testing_entry)
-#ifdef CONFIG_PREEMPT_COUNT
- get_thread_info r10
- ldr r4, [r10, #TI_PREEMPT] @ get preempt count
- sub r11, r4, #1 @ decrement it
- str r11, [r10, #TI_PREEMPT]
-#endif
+ dec_preempt_count_ti r10, r4
ldr r0, VFP_arch_address
str r0, [r0] @ set to non-zero value
mov pc, r9 @ we have handled the fault
diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S
index 33d82aa..ee4c7b6 100644
--- a/arch/arm/vfp/vfphw.S
+++ b/arch/arm/vfp/vfphw.S
@@ -177,12 +177,7 @@ vfp_hw_state_valid:
@ else it's one 32-bit instruction, so
@ always subtract 4 from the following
@ instruction address.
-#ifdef CONFIG_PREEMPT_COUNT
- get_thread_info r10
- ldr r4, [r10, #TI_PREEMPT] @ get preempt count
- sub r11, r4, #1 @ decrement it
- str r11, [r10, #TI_PREEMPT]
-#endif
+ dec_preempt_count_ti r10, r4
mov pc, r9 @ we think we have handled things
@@ -201,12 +196,7 @@ look_for_VFP_exceptions:
@ not recognised by VFP
DBGSTR "not VFP"
-#ifdef CONFIG_PREEMPT_COUNT
- get_thread_info r10
- ldr r4, [r10, #TI_PREEMPT] @ get preempt count
- sub r11, r4, #1 @ decrement it
- str r11, [r10, #TI_PREEMPT]
-#endif
+ dec_preempt_count_ti r10, r4
mov pc, lr
process_exception:
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/5] arm: Disable preemption in iwmmxt_task_enable()
2013-08-28 11:34 [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler Catalin Marinas
2013-08-28 11:34 ` [PATCH 1/5] arm: Move asm macro get_thread_info to asm/assembler.h Catalin Marinas
2013-08-28 11:34 ` [PATCH 2/5] arm: Add {inc,dec}_preempt_count asm macros Catalin Marinas
@ 2013-08-28 11:34 ` Catalin Marinas
2013-08-28 11:34 ` [PATCH 4/5] arm: Disable preemption in crunch_task_enable() Catalin Marinas
` (2 subsequent siblings)
5 siblings, 0 replies; 14+ messages in thread
From: Catalin Marinas @ 2013-08-28 11:34 UTC (permalink / raw)
To: linux-arm-kernel
This patch is in preparation for calling the iwmmxt_task_enable()
function with interrupts enabled.
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm/kernel/iwmmxt.S | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/arch/arm/kernel/iwmmxt.S b/arch/arm/kernel/iwmmxt.S
index a087838..c52f3e2 100644
--- a/arch/arm/kernel/iwmmxt.S
+++ b/arch/arm/kernel/iwmmxt.S
@@ -65,13 +65,14 @@
*/
ENTRY(iwmmxt_task_enable)
+ inc_preempt_count r10, r3
XSC(mrc p15, 0, r2, c15, c1, 0)
PJ4(mrc p15, 0, r2, c1, c0, 2)
@ CP0 and CP1 accessible?
XSC(tst r2, #0x3)
PJ4(tst r2, #0xf)
- movne pc, lr @ if so no business here
+ bne 4f @ if so no business here
@ enable access to CP0 and CP1
XSC(orr r2, r2, #0x3)
XSC(mcr p15, 0, r2, c15, c1, 0)
@@ -132,7 +133,7 @@ concan_dump:
wstrd wR15, [r1, #MMX_WR15]
2: teq r0, #0 @ anything to load?
- moveq pc, lr
+ beq 3f
concan_load:
@@ -165,8 +166,14 @@ concan_load:
@ clear CUP/MUP (only if r1 != 0)
teq r1, #0
mov r2, #0
- moveq pc, lr
+ beq 3f
tmcr wCon, r2
+
+3:
+#ifdef CONFIG_PREEMPT_COUNT
+ get_thread_info r10
+#endif
+4: dec_preempt_count r10, r3
mov pc, lr
/*
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 4/5] arm: Disable preemption in crunch_task_enable()
2013-08-28 11:34 [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler Catalin Marinas
` (2 preceding siblings ...)
2013-08-28 11:34 ` [PATCH 3/5] arm: Disable preemption in iwmmxt_task_enable() Catalin Marinas
@ 2013-08-28 11:34 ` Catalin Marinas
2013-08-28 11:34 ` [PATCH 5/5] arm: Enable IRQs before attempting to read user space in __und_usr Catalin Marinas
2014-03-31 4:11 ` [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler Yang Zhang
5 siblings, 0 replies; 14+ messages in thread
From: Catalin Marinas @ 2013-08-28 11:34 UTC (permalink / raw)
To: linux-arm-kernel
This patch is in preparation for calling the crunch_task_enable()
function with interrupts enabled.
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm/mach-ep93xx/crunch-bits.S | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/arch/arm/mach-ep93xx/crunch-bits.S b/arch/arm/mach-ep93xx/crunch-bits.S
index 0ec9bb4..890c5df 100644
--- a/arch/arm/mach-ep93xx/crunch-bits.S
+++ b/arch/arm/mach-ep93xx/crunch-bits.S
@@ -65,11 +65,13 @@
* called from prefetch exception handler with interrupts disabled
*/
ENTRY(crunch_task_enable)
+ inc_preempt_count r10, r3
+
ldr r8, =(EP93XX_APB_VIRT_BASE + 0x00130000) @ syscon addr
ldr r1, [r8, #0x80]
tst r1, #0x00800000 @ access to crunch enabled?
- movne pc, lr @ if so no business here
+ bne 2f @ if so no business here
mov r3, #0xaa @ unlock syscon swlock
str r3, [r8, #0xc0]
orr r1, r1, #0x00800000 @ enable access to crunch
@@ -142,7 +144,7 @@ crunch_save:
teq r0, #0 @ anything to load?
cfldr64eq mvdx0, [r1, #CRUNCH_MVDX0] @ mvdx0 was clobbered
- moveq pc, lr
+ beq 1f
crunch_load:
cfldr64 mvdx0, [r0, #CRUNCH_DSPSC] @ load status word
@@ -190,6 +192,11 @@ crunch_load:
cfldr64 mvdx14, [r0, #CRUNCH_MVDX14]
cfldr64 mvdx15, [r0, #CRUNCH_MVDX15]
+1:
+#ifdef CONFIG_PREEMPT_COUNT
+ get_thread_info r10
+#endif
+2: dec_preempt_count r10, r3
mov pc, lr
/*
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/5] arm: Enable IRQs before attempting to read user space in __und_usr
2013-08-28 11:34 [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler Catalin Marinas
` (3 preceding siblings ...)
2013-08-28 11:34 ` [PATCH 4/5] arm: Disable preemption in crunch_task_enable() Catalin Marinas
@ 2013-08-28 11:34 ` Catalin Marinas
2013-11-21 9:35 ` Alexey Ignatov
2014-03-31 4:11 ` [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler Yang Zhang
5 siblings, 1 reply; 14+ messages in thread
From: Catalin Marinas @ 2013-08-28 11:34 UTC (permalink / raw)
To: linux-arm-kernel
The Undef abort handler in the kernel reads the undefined instruction
from user space. If the page table was modified from another CPU, the
user access could fail and do_page_fault() will be executed with
interrupts disabled. This can potentially deadlock on ARM11MPCore or on
Cortex-A15 with erratum 798181 workaround enabled (both implying IPI for
TLB maintenance with page table lock held).
This patch enables the IRQs in __und_usr before attempting to read the
instruction from user space.
Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
---
arch/arm/kernel/entry-armv.S | 11 +++++++----
arch/arm/kernel/iwmmxt.S | 2 +-
arch/arm/mach-ep93xx/crunch-bits.S | 2 +-
arch/arm/vfp/entry.S | 3 +--
4 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index d40d0ef..c82ca96 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -411,6 +411,11 @@ __und_usr:
@
adr r9, BSYM(ret_from_exception)
+ @ IRQs must be enabled before attempting to read the instruction from
+ @ user space since that could cause a page/translation fault if the
+ @ page table was modified by another CPU.
+ enable_irq
+
tst r3, #PSR_T_BIT @ Thumb mode?
bne __und_usr_thumb
sub r4, r2, #4 @ ARM instr at LR - 4
@@ -514,7 +519,7 @@ ENDPROC(__und_usr)
* r9 = normal "successful" return address
* r10 = this threads thread_info structure
* lr = unrecognised instruction return address
- * IRQs disabled, FIQs enabled.
+ * IRQs enabled, FIQs enabled.
*/
@
@ Fall-through from Thumb-2 __und_usr
@@ -621,7 +626,6 @@ call_fpe:
#endif
do_fpe:
- enable_irq
ldr r4, .LCfp
add r10, r10, #TI_FPSTATE @ r10 = workspace
ldr pc, [r4] @ Call FP module USR entry point
@@ -649,8 +653,7 @@ __und_usr_fault_32:
b 1f
__und_usr_fault_16:
mov r1, #2
-1: enable_irq
- mov r0, sp
+1: mov r0, sp
adr lr, BSYM(ret_from_exception)
b __und_fault
ENDPROC(__und_usr_fault_32)
diff --git a/arch/arm/kernel/iwmmxt.S b/arch/arm/kernel/iwmmxt.S
index c52f3e2..da31170 100644
--- a/arch/arm/kernel/iwmmxt.S
+++ b/arch/arm/kernel/iwmmxt.S
@@ -61,7 +61,7 @@
* r9 = ret_from_exception
* lr = undefined instr exit
*
- * called from prefetch exception handler with interrupts disabled
+ * called from prefetch exception handler with interrupts enabled
*/
ENTRY(iwmmxt_task_enable)
diff --git a/arch/arm/mach-ep93xx/crunch-bits.S b/arch/arm/mach-ep93xx/crunch-bits.S
index 890c5df..85e7655 100644
--- a/arch/arm/mach-ep93xx/crunch-bits.S
+++ b/arch/arm/mach-ep93xx/crunch-bits.S
@@ -62,7 +62,7 @@
* r9 = ret_from_exception
* lr = undefined instr exit
*
- * called from prefetch exception handler with interrupts disabled
+ * called from prefetch exception handler with interrupts enabled
*/
ENTRY(crunch_task_enable)
inc_preempt_count r10, r3
diff --git a/arch/arm/vfp/entry.S b/arch/arm/vfp/entry.S
index f0759e7..fe6ca57 100644
--- a/arch/arm/vfp/entry.S
+++ b/arch/arm/vfp/entry.S
@@ -22,11 +22,10 @@
@ r9 = normal "successful" return address
@ r10 = this threads thread_info structure
@ lr = unrecognised instruction return address
-@ IRQs disabled.
+@ IRQs enabled.
@
ENTRY(do_vfp)
inc_preempt_count r10, r4
- enable_irq
ldr r4, .LCvfp
ldr r11, [r10, #TI_CPU] @ CPU number
add r10, r10, #TI_VFPSTATE @ r10 = workspace
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 5/5] arm: Enable IRQs before attempting to read user space in __und_usr
2013-08-28 11:34 ` [PATCH 5/5] arm: Enable IRQs before attempting to read user space in __und_usr Catalin Marinas
@ 2013-11-21 9:35 ` Alexey Ignatov
2013-11-21 10:29 ` Russell King - ARM Linux
0 siblings, 1 reply; 14+ messages in thread
From: Alexey Ignatov @ 2013-11-21 9:35 UTC (permalink / raw)
To: linux-arm-kernel
Catalin Marinas <catalin.marinas <at> arm.com> writes:
> The Undef abort handler in the kernel reads the undefined instruction
> from user space. If the page table was modified from another CPU, the
> user access could fail and do_page_fault() will be executed with
> interrupts disabled. This can potentially deadlock on ARM11MPCore or on
> Cortex-A15 with erratum 798181 workaround enabled (both implying IPI for
> TLB maintenance with page table lock held).
>
> This patch enables the IRQs in __und_usr before attempting to read the
> instruction from user space.
This patch moves enable_irq call from do_fpe directly to __und_usr handler,
but __und_svc handler also calls do_fpe (via call_fpe), so now this codepath
runs with disabled irqs. This behavior change doesn't look good for me.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/5] arm: Enable IRQs before attempting to read user space in __und_usr
2013-11-21 9:35 ` Alexey Ignatov
@ 2013-11-21 10:29 ` Russell King - ARM Linux
2013-11-22 9:47 ` Alexey Ignatov
0 siblings, 1 reply; 14+ messages in thread
From: Russell King - ARM Linux @ 2013-11-21 10:29 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Nov 21, 2013 at 09:35:34AM +0000, Alexey Ignatov wrote:
> Catalin Marinas <catalin.marinas <at> arm.com> writes:
>
> > The Undef abort handler in the kernel reads the undefined instruction
> > from user space. If the page table was modified from another CPU, the
> > user access could fail and do_page_fault() will be executed with
> > interrupts disabled. This can potentially deadlock on ARM11MPCore or on
> > Cortex-A15 with erratum 798181 workaround enabled (both implying IPI for
> > TLB maintenance with page table lock held).
> >
> > This patch enables the IRQs in __und_usr before attempting to read the
> > instruction from user space.
>
> This patch moves enable_irq call from do_fpe directly to __und_usr handler,
> but __und_svc handler also calls do_fpe (via call_fpe), so now this codepath
> runs with disabled irqs. This behavior change doesn't look good for me.
However, you're not executing FPA instructions in the kernel as a general
rule, so it doesn't matter.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 5/5] arm: Enable IRQs before attempting to read user space in __und_usr
2013-11-21 10:29 ` Russell King - ARM Linux
@ 2013-11-22 9:47 ` Alexey Ignatov
0 siblings, 0 replies; 14+ messages in thread
From: Alexey Ignatov @ 2013-11-22 9:47 UTC (permalink / raw)
To: linux-arm-kernel
On 10:29 Thu 21 Nov , Russell King - ARM Linux wrote:
> On Thu, Nov 21, 2013 at 09:35:34AM +0000, Alexey Ignatov wrote:
> > Catalin Marinas <catalin.marinas <at> arm.com> writes:
> >
> > > The Undef abort handler in the kernel reads the undefined instruction
> > > from user space. If the page table was modified from another CPU, the
> > > user access could fail and do_page_fault() will be executed with
> > > interrupts disabled. This can potentially deadlock on ARM11MPCore or on
> > > Cortex-A15 with erratum 798181 workaround enabled (both implying IPI for
> > > TLB maintenance with page table lock held).
> > >
> > > This patch enables the IRQs in __und_usr before attempting to read the
> > > instruction from user space.
> >
> > This patch moves enable_irq call from do_fpe directly to __und_usr handler,
> > but __und_svc handler also calls do_fpe (via call_fpe), so now this codepath
> > runs with disabled irqs. This behavior change doesn't look good for me.
>
> However, you're not executing FPA instructions in the kernel as a general
> rule, so it doesn't matter.
Theoretically, ok.
It seems that we cought this deadlock on Cortex-A15 and this patch fixes things
(testing in progress). Is there any plans to mainline?
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler
2013-08-28 11:34 [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler Catalin Marinas
` (4 preceding siblings ...)
2013-08-28 11:34 ` [PATCH 5/5] arm: Enable IRQs before attempting to read user space in __und_usr Catalin Marinas
@ 2014-03-31 4:11 ` Yang Zhang
5 siblings, 0 replies; 14+ messages in thread
From: Yang Zhang @ 2014-03-31 4:11 UTC (permalink / raw)
To: linux-arm-kernel
Catalin Marinas <catalin.marinas <at> arm.com> writes:
> (an alternative simpler patch would be to enable the interrupts only
> around the user access instructions in __und_usr and disable them
> afterwards, so I would rather do it as in this series).
Tested-by: Yang Zhang<email:tim.zhang@spreadtrum.com>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler
2014-04-01 15:41 ` Catalin Marinas
@ 2014-04-02 4:03 ` Arun KS
0 siblings, 0 replies; 14+ messages in thread
From: Arun KS @ 2014-04-02 4:03 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Apr 1, 2014 at 9:11 PM, Catalin Marinas <catalin.marinas@arm.com> wrote:
> On Tue, Apr 01, 2014 at 01:03:04PM +0100, Arun KS wrote:
>> On Thu, Mar 13, 2014 at 11:45 PM, Catalin Marinas
>> <catalin.marinas@arm.com> wrote:
>> > I've had this series in my repo for a long time with some follow-ups on
>> > the list but without any conclusive ACK or NAK. If there are no
>> > objections, I plan to send it to the patch system.
>> >
>> > The only change is rebasing to 3.14-rc6 and adding some Cc for the
>> > crunch bits.
>> >
>> > The __und_usr handler accesses the user space to read the opcode of the
>> > faulting instruction. This is currently done with interrupts disabled
>> > but it could potentially cause a data abort of the page table was
>> > modified from another CPU. The data abort with interrupts disabled
>> > triggers a might_sleep() warning in do_page_fault() or, worse, deadlock
>> > on the pte lock when TLB ops broadcasting is enabled (ARM11MPCore or A15
>> > together with workaround for erratum 798181).
>> >
>> > Catalin Marinas (5):
>> > arm: Move asm macro get_thread_info to asm/assembler.h
>> > arm: Add {inc,dec}_preempt_count asm macros
>> > arm: Disable preemption in iwmmxt_task_enable()
>> > arm: Disable preemption in crunch_task_enable()
>> > arm: Enable IRQs before attempting to read user space in __und_usr
>> >
>> > arch/arm/include/asm/assembler.h | 42 ++++++++++++++++++++++++++++++++++++++
>> > arch/arm/kernel/entry-armv.S | 11 ++++++----
>> > arch/arm/kernel/entry-header.S | 11 ----------
>> > arch/arm/kernel/iwmmxt.S | 15 ++++++++++----
>> > arch/arm/mach-ep93xx/crunch-bits.S | 13 +++++++++---
>> > arch/arm/vfp/entry.S | 28 ++++++++-----------------
>> > arch/arm/vfp/vfphw.S | 19 ++++++-----------
>> > 7 files changed, 84 insertions(+), 55 deletions(-)
>>
>> Tested-by: Arun KS<getarunks@gmail.com>
>
> Thanks. I assume it's tested only with VFP (not crunch nor iwmmxt).
Yes.
Thanks,
Arun
>
> --
> Catalin
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler
2014-04-01 12:03 ` Arun KS
@ 2014-04-01 15:41 ` Catalin Marinas
2014-04-02 4:03 ` Arun KS
0 siblings, 1 reply; 14+ messages in thread
From: Catalin Marinas @ 2014-04-01 15:41 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, Apr 01, 2014 at 01:03:04PM +0100, Arun KS wrote:
> On Thu, Mar 13, 2014 at 11:45 PM, Catalin Marinas
> <catalin.marinas@arm.com> wrote:
> > I've had this series in my repo for a long time with some follow-ups on
> > the list but without any conclusive ACK or NAK. If there are no
> > objections, I plan to send it to the patch system.
> >
> > The only change is rebasing to 3.14-rc6 and adding some Cc for the
> > crunch bits.
> >
> > The __und_usr handler accesses the user space to read the opcode of the
> > faulting instruction. This is currently done with interrupts disabled
> > but it could potentially cause a data abort of the page table was
> > modified from another CPU. The data abort with interrupts disabled
> > triggers a might_sleep() warning in do_page_fault() or, worse, deadlock
> > on the pte lock when TLB ops broadcasting is enabled (ARM11MPCore or A15
> > together with workaround for erratum 798181).
> >
> > Catalin Marinas (5):
> > arm: Move asm macro get_thread_info to asm/assembler.h
> > arm: Add {inc,dec}_preempt_count asm macros
> > arm: Disable preemption in iwmmxt_task_enable()
> > arm: Disable preemption in crunch_task_enable()
> > arm: Enable IRQs before attempting to read user space in __und_usr
> >
> > arch/arm/include/asm/assembler.h | 42 ++++++++++++++++++++++++++++++++++++++
> > arch/arm/kernel/entry-armv.S | 11 ++++++----
> > arch/arm/kernel/entry-header.S | 11 ----------
> > arch/arm/kernel/iwmmxt.S | 15 ++++++++++----
> > arch/arm/mach-ep93xx/crunch-bits.S | 13 +++++++++---
> > arch/arm/vfp/entry.S | 28 ++++++++-----------------
> > arch/arm/vfp/vfphw.S | 19 ++++++-----------
> > 7 files changed, 84 insertions(+), 55 deletions(-)
>
> Tested-by: Arun KS<getarunks@gmail.com>
Thanks. I assume it's tested only with VFP (not crunch nor iwmmxt).
--
Catalin
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler
2014-03-13 18:15 Catalin Marinas
@ 2014-04-01 12:03 ` Arun KS
2014-04-01 15:41 ` Catalin Marinas
0 siblings, 1 reply; 14+ messages in thread
From: Arun KS @ 2014-04-01 12:03 UTC (permalink / raw)
To: linux-arm-kernel
On Thu, Mar 13, 2014 at 11:45 PM, Catalin Marinas
<catalin.marinas@arm.com> wrote:
> Hi Russell,
>
> I've had this series in my repo for a long time with some follow-ups on
> the list but without any conclusive ACK or NAK. If there are no
> objections, I plan to send it to the patch system.
>
> The only change is rebasing to 3.14-rc6 and adding some Cc for the
> crunch bits.
>
> The __und_usr handler accesses the user space to read the opcode of the
> faulting instruction. This is currently done with interrupts disabled
> but it could potentially cause a data abort of the page table was
> modified from another CPU. The data abort with interrupts disabled
> triggers a might_sleep() warning in do_page_fault() or, worse, deadlock
> on the pte lock when TLB ops broadcasting is enabled (ARM11MPCore or A15
> together with workaround for erratum 798181).
>
> Catalin Marinas (5):
> arm: Move asm macro get_thread_info to asm/assembler.h
> arm: Add {inc,dec}_preempt_count asm macros
> arm: Disable preemption in iwmmxt_task_enable()
> arm: Disable preemption in crunch_task_enable()
> arm: Enable IRQs before attempting to read user space in __und_usr
>
> arch/arm/include/asm/assembler.h | 42 ++++++++++++++++++++++++++++++++++++++
> arch/arm/kernel/entry-armv.S | 11 ++++++----
> arch/arm/kernel/entry-header.S | 11 ----------
> arch/arm/kernel/iwmmxt.S | 15 ++++++++++----
> arch/arm/mach-ep93xx/crunch-bits.S | 13 +++++++++---
> arch/arm/vfp/entry.S | 28 ++++++++-----------------
> arch/arm/vfp/vfphw.S | 19 ++++++-----------
> 7 files changed, 84 insertions(+), 55 deletions(-)
Tested-by: Arun KS<getarunks@gmail.com>
Thanks,
Arun
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler
@ 2014-03-13 18:15 Catalin Marinas
2014-04-01 12:03 ` Arun KS
0 siblings, 1 reply; 14+ messages in thread
From: Catalin Marinas @ 2014-03-13 18:15 UTC (permalink / raw)
To: linux-arm-kernel
Hi Russell,
I've had this series in my repo for a long time with some follow-ups on
the list but without any conclusive ACK or NAK. If there are no
objections, I plan to send it to the patch system.
The only change is rebasing to 3.14-rc6 and adding some Cc for the
crunch bits.
The __und_usr handler accesses the user space to read the opcode of the
faulting instruction. This is currently done with interrupts disabled
but it could potentially cause a data abort of the page table was
modified from another CPU. The data abort with interrupts disabled
triggers a might_sleep() warning in do_page_fault() or, worse, deadlock
on the pte lock when TLB ops broadcasting is enabled (ARM11MPCore or A15
together with workaround for erratum 798181).
Catalin Marinas (5):
arm: Move asm macro get_thread_info to asm/assembler.h
arm: Add {inc,dec}_preempt_count asm macros
arm: Disable preemption in iwmmxt_task_enable()
arm: Disable preemption in crunch_task_enable()
arm: Enable IRQs before attempting to read user space in __und_usr
arch/arm/include/asm/assembler.h | 42 ++++++++++++++++++++++++++++++++++++++
arch/arm/kernel/entry-armv.S | 11 ++++++----
arch/arm/kernel/entry-header.S | 11 ----------
arch/arm/kernel/iwmmxt.S | 15 ++++++++++----
arch/arm/mach-ep93xx/crunch-bits.S | 13 +++++++++---
arch/arm/vfp/entry.S | 28 ++++++++-----------------
arch/arm/vfp/vfphw.S | 19 ++++++-----------
7 files changed, 84 insertions(+), 55 deletions(-)
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2014-04-02 4:03 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-28 11:34 [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler Catalin Marinas
2013-08-28 11:34 ` [PATCH 1/5] arm: Move asm macro get_thread_info to asm/assembler.h Catalin Marinas
2013-08-28 11:34 ` [PATCH 2/5] arm: Add {inc,dec}_preempt_count asm macros Catalin Marinas
2013-08-28 11:34 ` [PATCH 3/5] arm: Disable preemption in iwmmxt_task_enable() Catalin Marinas
2013-08-28 11:34 ` [PATCH 4/5] arm: Disable preemption in crunch_task_enable() Catalin Marinas
2013-08-28 11:34 ` [PATCH 5/5] arm: Enable IRQs before attempting to read user space in __und_usr Catalin Marinas
2013-11-21 9:35 ` Alexey Ignatov
2013-11-21 10:29 ` Russell King - ARM Linux
2013-11-22 9:47 ` Alexey Ignatov
2014-03-31 4:11 ` [PATCH 0/5] arm: Early IRQ enabling in the Undef user handler Yang Zhang
2014-03-13 18:15 Catalin Marinas
2014-04-01 12:03 ` Arun KS
2014-04-01 15:41 ` Catalin Marinas
2014-04-02 4:03 ` Arun KS
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.