All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
To: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Cc: Russell King <linux@arm.linux.org.uk>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Jean-Francois Moine <moinejf@free.fr>,
	Jason Cooper <jason@lakedaemon.net>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH v2] ARM: Fix preemption disable in iwmmxt_task_enable()
Date: Sat, 12 Jul 2014 13:05:30 +0200	[thread overview]
Message-ID: <1405163130-3927-1-git-send-email-sebastian.hesselbarth@gmail.com> (raw)
In-Reply-To: <1405069813-9270-1-git-send-email-sebastian.hesselbarth@gmail.com>

commit 431a84b1a4f7d1a0085d5b91330c5053cc8e8b12
 ("ARM: 8034/1: Disable preemption in iwmmxt_task_enable()")
introduced macros {inc,dec}_preempt_count to iwmmxt_task_enable
to make it run with preemption disabled.

Unfortunately, other functions in iwmmxt.S also use concan_{save,dump,load}
sections located in iwmmxt_task_enable() to deal with iWMMXt coprocessor.
This causes an unbalanced preempt_count due to excessive dec_preempt_count
and destroyed return addresses in callers of concan_ labels due to a register
collision:

Linux version 3.16.0-rc3-00062-gd92a333-dirty (jef@armhf) (gcc version 4.8.3 (Debian 4.8.3-4) ) #5 PREEMPT Thu Jul 3 19:46:39 CEST 2014
CPU: ARMv7 Processor [560f5815] revision 5 (ARMv7), cr=10c5387d
CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
Machine model: SolidRun CuBox
...
PJ4 iWMMXt v2 coprocessor enabled.
...
Unable to handle kernel paging request at virtual address fffffffe
pgd = bb25c000
[fffffffe] *pgd=3bfde821, *pte=00000000, *ppte=00000000
Internal error: Oops: 80000007 [#1] PREEMPT ARM
Modules linked in:
CPU: 0 PID: 62 Comm: startpar Not tainted 3.16.0-rc3-00062-gd92a333-dirty #5
task: bb230b80 ti: bb256000 task.ti: bb256000
PC is at 0xfffffffe
LR is at iwmmxt_task_copy+0x44/0x4c
pc : [<fffffffe>]    lr : [<800130ac>]    psr: 40000033
sp : bb257de8  ip : 00000013  fp : bb257ea4
r10: bb256000  r9 : fffffdfe  r8 : 76e898e6
r7 : bb257ec8  r6 : bb256000  r5 : 7ea12760  r4 : 000000a0
r3 : ffffffff  r2 : 00000003  r1 : bb257df8  r0 : 00000000
Flags: nZcv  IRQs on  FIQs on  Mode SVC_32  ISA Thumb  Segment user
Control: 10c5387d  Table: 3b25c019  DAC: 00000015
Process startpar (pid: 62, stack limit = 0xbb256248)

This patch fixes the issue by moving concan_{save,dump,load} into separate
code sections and make iwmmxt_task_enable() call them in the same way the
other functions use concan_ symbols. The test for valid ownership is moved
to concan_save and is safe for the other user of it, iwmmxt_task_disable().
The register collision is also resolved by moving concan_ symbols as
{inc,dec}_preempt_count are now local to iwmmxt_task_enable().

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Reported-by: Jean-Francois Moine <moinejf@free.fr>
Fixes: 431a84b1a4f7 ("ARM: 8034/1: Disable preemption in iwmmxt_task_enable()")
---
The offending commit was intoduced past v3.15-rc1 and the corresponding fix
should also be queued up for stable v3.15+

Changelog
v1->v2:
- return immediately from concan_ instead of branch to 3f, i.e. replace
  'beq 3f' with 'moveq pc, lr' (Suggested by Catalin Marinas)

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Jean-Francois Moine <moinejf@free.fr>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
 arch/arm/kernel/iwmmxt.S | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/arm/kernel/iwmmxt.S b/arch/arm/kernel/iwmmxt.S
index a5599cfc43cb..2b32978ae905 100644
--- a/arch/arm/kernel/iwmmxt.S
+++ b/arch/arm/kernel/iwmmxt.S
@@ -94,13 +94,19 @@ ENTRY(iwmmxt_task_enable)
 
 	mrc	p15, 0, r2, c2, c0, 0
 	mov	r2, r2				@ cpwait
+	bl	concan_save
 
-	teq	r1, #0				@ test for last ownership
-	mov	lr, r9				@ normal exit from exception
-	beq	concan_load			@ no owner, skip save
+#ifdef CONFIG_PREEMPT_COUNT
+	get_thread_info r10
+#endif
+4:	dec_preempt_count r10, r3
+	mov	pc, r9				@ normal exit from exception
 
 concan_save:
 
+	teq	r1, #0				@ test for last ownership
+	beq	concan_load			@ no owner, skip save
+
 	tmrc	r2, wCon
 
 	@ CUP? wCx
@@ -138,7 +144,7 @@ concan_dump:
 	wstrd	wR15, [r1, #MMX_WR15]
 
 2:	teq	r0, #0				@ anything to load?
-	beq	3f
+	moveq	pc, lr				@ if not, return
 
 concan_load:
 
@@ -171,14 +177,9 @@ concan_load:
 	@ clear CUP/MUP (only if r1 != 0)
 	teq	r1, #0
 	mov 	r2, #0
-	beq	3f
-	tmcr	wCon, r2
+	moveq	pc, lr
 
-3:
-#ifdef CONFIG_PREEMPT_COUNT
-	get_thread_info r10
-#endif
-4:	dec_preempt_count r10, r3
+	tmcr	wCon, r2
 	mov	pc, lr
 
 /*
-- 
2.0.0


WARNING: multiple messages have this Message-ID (diff)
From: sebastian.hesselbarth@gmail.com (Sebastian Hesselbarth)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] ARM: Fix preemption disable in iwmmxt_task_enable()
Date: Sat, 12 Jul 2014 13:05:30 +0200	[thread overview]
Message-ID: <1405163130-3927-1-git-send-email-sebastian.hesselbarth@gmail.com> (raw)
In-Reply-To: <1405069813-9270-1-git-send-email-sebastian.hesselbarth@gmail.com>

commit 431a84b1a4f7d1a0085d5b91330c5053cc8e8b12
 ("ARM: 8034/1: Disable preemption in iwmmxt_task_enable()")
introduced macros {inc,dec}_preempt_count to iwmmxt_task_enable
to make it run with preemption disabled.

Unfortunately, other functions in iwmmxt.S also use concan_{save,dump,load}
sections located in iwmmxt_task_enable() to deal with iWMMXt coprocessor.
This causes an unbalanced preempt_count due to excessive dec_preempt_count
and destroyed return addresses in callers of concan_ labels due to a register
collision:

Linux version 3.16.0-rc3-00062-gd92a333-dirty (jef at armhf) (gcc version 4.8.3 (Debian 4.8.3-4) ) #5 PREEMPT Thu Jul 3 19:46:39 CEST 2014
CPU: ARMv7 Processor [560f5815] revision 5 (ARMv7), cr=10c5387d
CPU: PIPT / VIPT nonaliasing data cache, PIPT instruction cache
Machine model: SolidRun CuBox
...
PJ4 iWMMXt v2 coprocessor enabled.
...
Unable to handle kernel paging request at virtual address fffffffe
pgd = bb25c000
[fffffffe] *pgd=3bfde821, *pte=00000000, *ppte=00000000
Internal error: Oops: 80000007 [#1] PREEMPT ARM
Modules linked in:
CPU: 0 PID: 62 Comm: startpar Not tainted 3.16.0-rc3-00062-gd92a333-dirty #5
task: bb230b80 ti: bb256000 task.ti: bb256000
PC is at 0xfffffffe
LR is at iwmmxt_task_copy+0x44/0x4c
pc : [<fffffffe>]    lr : [<800130ac>]    psr: 40000033
sp : bb257de8  ip : 00000013  fp : bb257ea4
r10: bb256000  r9 : fffffdfe  r8 : 76e898e6
r7 : bb257ec8  r6 : bb256000  r5 : 7ea12760  r4 : 000000a0
r3 : ffffffff  r2 : 00000003  r1 : bb257df8  r0 : 00000000
Flags: nZcv  IRQs on  FIQs on  Mode SVC_32  ISA Thumb  Segment user
Control: 10c5387d  Table: 3b25c019  DAC: 00000015
Process startpar (pid: 62, stack limit = 0xbb256248)

This patch fixes the issue by moving concan_{save,dump,load} into separate
code sections and make iwmmxt_task_enable() call them in the same way the
other functions use concan_ symbols. The test for valid ownership is moved
to concan_save and is safe for the other user of it, iwmmxt_task_disable().
The register collision is also resolved by moving concan_ symbols as
{inc,dec}_preempt_count are now local to iwmmxt_task_enable().

Signed-off-by: Sebastian Hesselbarth <sebastian.hesselbarth@gmail.com>
Reported-by: Jean-Francois Moine <moinejf@free.fr>
Fixes: 431a84b1a4f7 ("ARM: 8034/1: Disable preemption in iwmmxt_task_enable()")
---
The offending commit was intoduced past v3.15-rc1 and the corresponding fix
should also be queued up for stable v3.15+

Changelog
v1->v2:
- return immediately from concan_ instead of branch to 3f, i.e. replace
  'beq 3f' with 'moveq pc, lr' (Suggested by Catalin Marinas)

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Jean-Francois Moine <moinejf@free.fr>
Cc: Jason Cooper <jason@lakedaemon.net>
Cc: linux-arm-kernel at lists.infradead.org
Cc: linux-kernel at vger.kernel.org
---
 arch/arm/kernel/iwmmxt.S | 23 ++++++++++++-----------
 1 file changed, 12 insertions(+), 11 deletions(-)

diff --git a/arch/arm/kernel/iwmmxt.S b/arch/arm/kernel/iwmmxt.S
index a5599cfc43cb..2b32978ae905 100644
--- a/arch/arm/kernel/iwmmxt.S
+++ b/arch/arm/kernel/iwmmxt.S
@@ -94,13 +94,19 @@ ENTRY(iwmmxt_task_enable)
 
 	mrc	p15, 0, r2, c2, c0, 0
 	mov	r2, r2				@ cpwait
+	bl	concan_save
 
-	teq	r1, #0				@ test for last ownership
-	mov	lr, r9				@ normal exit from exception
-	beq	concan_load			@ no owner, skip save
+#ifdef CONFIG_PREEMPT_COUNT
+	get_thread_info r10
+#endif
+4:	dec_preempt_count r10, r3
+	mov	pc, r9				@ normal exit from exception
 
 concan_save:
 
+	teq	r1, #0				@ test for last ownership
+	beq	concan_load			@ no owner, skip save
+
 	tmrc	r2, wCon
 
 	@ CUP? wCx
@@ -138,7 +144,7 @@ concan_dump:
 	wstrd	wR15, [r1, #MMX_WR15]
 
 2:	teq	r0, #0				@ anything to load?
-	beq	3f
+	moveq	pc, lr				@ if not, return
 
 concan_load:
 
@@ -171,14 +177,9 @@ concan_load:
 	@ clear CUP/MUP (only if r1 != 0)
 	teq	r1, #0
 	mov 	r2, #0
-	beq	3f
-	tmcr	wCon, r2
+	moveq	pc, lr
 
-3:
-#ifdef CONFIG_PREEMPT_COUNT
-	get_thread_info r10
-#endif
-4:	dec_preempt_count r10, r3
+	tmcr	wCon, r2
 	mov	pc, lr
 
 /*
-- 
2.0.0

  parent reply	other threads:[~2014-07-12 11:05 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-06  6:08 dove (marvell A510) crash on boot with config_preempt Jean-Francois Moine
2014-07-08 15:17 ` Jason Cooper
2014-07-10 12:33   ` Sebastian Hesselbarth
2014-07-10 20:55     ` Sebastian Hesselbarth
2014-07-10 22:13       ` Russell King - ARM Linux
2014-07-10 22:08 ` Russell King - ARM Linux
2014-07-11  9:10 ` [PATCH] ARM: Fix preemption disable in iwmmxt_task_enable() Sebastian Hesselbarth
2014-07-11  9:10   ` Sebastian Hesselbarth
2014-07-11 17:09   ` Catalin Marinas
2014-07-11 17:09     ` Catalin Marinas
2014-07-12 11:05   ` Sebastian Hesselbarth [this message]
2014-07-12 11:05     ` [PATCH v2] " Sebastian Hesselbarth
2014-07-14 12:08     ` Catalin Marinas
2014-07-14 12:08       ` Catalin Marinas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1405163130-3927-1-git-send-email-sebastian.hesselbarth@gmail.com \
    --to=sebastian.hesselbarth@gmail.com \
    --cc=catalin.marinas@arm.com \
    --cc=jason@lakedaemon.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=moinejf@free.fr \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.