All of lore.kernel.org
 help / color / mirror / Atom feed
* [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 ++++++++++
 arch/arm/kernel/entry-header.S   | 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";		\
diff --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.