linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ARM: vfp: fix save and restore when running on pre-VFPv3 and CONFIG_VFPv3 set
@ 2012-10-13  4:46 Paul Walmsley
  2012-10-13  9:02 ` Russell King - ARM Linux
  0 siblings, 1 reply; 9+ messages in thread
From: Paul Walmsley @ 2012-10-13  4:46 UTC (permalink / raw)
  To: linux-arm-kernel


After commit 846a136881b8f73c1f74250bf6acfaa309cab1f2 ("ARM: vfp: fix
saving d16-d31 vfp registers on v6+ kernels"), the OMAP 2430SDP board
started crashing during boot with omap2plus_defconfig:

[    3.875122] mmcblk0: mmc0:e624 SD04G 3.69 GiB
[    3.915954]  mmcblk0: p1
[    4.086639] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
[    4.093719] Modules linked in:
[    4.096954] CPU: 0    Not tainted  (3.6.0-02232-g759e00b #570)
[    4.103149] PC is at vfp_reload_hw+0x1c/0x44
[    4.107666] LR is at __und_usr_fault_32+0x0/0x8

It turns out that the context save/restore fix unmasked a latent bug in 
commit 5aaf254409f8d58229107b59507a8235b715a960 ("ARM: 6203/1: Make VFPv3 
usable on ARMv6").  When CONFIG_VFPv3 is set, but the kernel is booted on 
a pre-VFPv3 core, the code attempts to save and restore the d16-d31 VFP 
registers.  These are only present on non-D16 VFPv3+, so the kernel dies 
with an undefined instruction exception.  The kernel didn't crash before 
commit 846a136 because the save and restore code only touched d0-d15, 
present on all VFP.

Fix to save and restore the d16-d31 registers only if they are
present.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dave Martin <dave.martin@linaro.org>
---
 arch/arm/include/asm/vfp.h       |    6 ++++++
 arch/arm/include/asm/vfpmacros.h |    8 ++++----
 arch/arm/vfp/vfpmodule.c         |   10 ++++++++++
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/arm/include/asm/vfp.h b/arch/arm/include/asm/vfp.h
index f4ab34f..5689798 100644
--- a/arch/arm/include/asm/vfp.h
+++ b/arch/arm/include/asm/vfp.h
@@ -82,3 +82,9 @@
 #define VFPOPDESC_UNUSED_BIT	(24)
 #define VFPOPDESC_UNUSED_MASK	(0xFF << VFPOPDESC_UNUSED_BIT)
 #define VFPOPDESC_OPDESC_MASK	(~(VFPOPDESC_LENGTH_MASK | VFPOPDESC_UNUSED_MASK))
+
+#ifndef __ASSEMBLER__
+#include <linux/types.h>
+
+extern u32 vfp_has_d16_d31;
+#endif
diff --git a/arch/arm/include/asm/vfpmacros.h b/arch/arm/include/asm/vfpmacros.h
index 6a6f1e4..11496a9 100644
--- a/arch/arm/include/asm/vfpmacros.h
+++ b/arch/arm/include/asm/vfpmacros.h
@@ -25,9 +25,9 @@
 #endif
 #ifdef CONFIG_VFPv3
 #if __LINUX_ARM_ARCH__ <= 6
-	ldr	\tmp, =elf_hwcap		    @ may not have MVFR regs
+	ldr	\tmp, =vfp_has_d16_d31		    @ have VFP regs d16-d31?
 	ldr	\tmp, [\tmp, #0]
-	tst	\tmp, #HWCAP_VFPv3D16
+	cmp	\tmp, #1
 	ldceql	p11, cr0, [\base],#32*4		    @ FLDMIAD \base!, {d16-d31}
 	addne	\base, \base, #32*4		    @ step over unused register space
 #else
@@ -49,9 +49,9 @@
 #endif
 #ifdef CONFIG_VFPv3
 #if __LINUX_ARM_ARCH__ <= 6
-	ldr	\tmp, =elf_hwcap		    @ may not have MVFR regs
+	ldr	\tmp, =vfp_has_d16_d31		    @ have VFP regs d16-d31?
 	ldr	\tmp, [\tmp, #0]
-	tst	\tmp, #HWCAP_VFPv3D16
+	cmp	\tmp, #1
 	stceql	p11, cr0, [\base],#32*4		    @ FSTMIAD \base!, {d16-d31}
 	addne	\base, \base, #32*4		    @ step over unused register space
 #else
diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index c834b32..929b53b 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -58,6 +58,12 @@ unsigned int VFP_arch;
 union vfp_state *vfp_current_hw_state[NR_CPUS];
 
 /*
+ * If the VFP on this ARM core has registers d16-d31, vfp_has_d16_d31 will
+ * be set to 1; otherwise, 0.  Only valid after startup.
+ */
+u32 vfp_has_d16_d31;
+
+/*
  * Is 'thread's most up to date state stored in this CPUs hardware?
  * Must be called from non-preemptible context.
  */
@@ -691,6 +697,8 @@ static int __init vfp_init(void)
 		thread_register_notifier(&vfp_notifier_block);
 		vfp_pm_init();
 
+		vfp_has_d16_d31 = 0;
+
 		/*
 		 * We detected VFP, and the support code is
 		 * in place; report VFP support to userspace.
@@ -706,6 +714,8 @@ static int __init vfp_init(void)
 			 */
 			if (((fmrx(MVFR0) & MVFR0_A_SIMD_MASK)) == 1)
 				elf_hwcap |= HWCAP_VFPv3D16;
+			else
+				vfp_has_d16_d31 = 1;
 		}
 #endif
 		/*
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] ARM: vfp: fix save and restore when running on pre-VFPv3 and CONFIG_VFPv3 set
  2012-10-13  4:46 [PATCH] ARM: vfp: fix save and restore when running on pre-VFPv3 and CONFIG_VFPv3 set Paul Walmsley
@ 2012-10-13  9:02 ` Russell King - ARM Linux
  2012-10-13 10:50   ` Måns Rullgård
  0 siblings, 1 reply; 9+ messages in thread
From: Russell King - ARM Linux @ 2012-10-13  9:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Oct 13, 2012 at 04:46:18AM +0000, Paul Walmsley wrote:
> 
> After commit 846a136881b8f73c1f74250bf6acfaa309cab1f2 ("ARM: vfp: fix
> saving d16-d31 vfp registers on v6+ kernels"), the OMAP 2430SDP board
> started crashing during boot with omap2plus_defconfig:
> 
> [    3.875122] mmcblk0: mmc0:e624 SD04G 3.69 GiB
> [    3.915954]  mmcblk0: p1
> [    4.086639] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
> [    4.093719] Modules linked in:
> [    4.096954] CPU: 0    Not tainted  (3.6.0-02232-g759e00b #570)
> [    4.103149] PC is at vfp_reload_hw+0x1c/0x44
> [    4.107666] LR is at __und_usr_fault_32+0x0/0x8
> 
> It turns out that the context save/restore fix unmasked a latent bug in 
> commit 5aaf254409f8d58229107b59507a8235b715a960 ("ARM: 6203/1: Make VFPv3 
> usable on ARMv6").  When CONFIG_VFPv3 is set, but the kernel is booted on 
> a pre-VFPv3 core, the code attempts to save and restore the d16-d31 VFP 
> registers.  These are only present on non-D16 VFPv3+, so the kernel dies 
> with an undefined instruction exception.  The kernel didn't crash before 
> commit 846a136 because the save and restore code only touched d0-d15, 
> present on all VFP.
> 
> Fix to save and restore the d16-d31 registers only if they are
> present.

No.  VFPv3D16 HWCAP means that the VFP supports just 16 double registers
- and it communicates this information to userspace.  If this bit is clear,
and the VFP only has 16 double registers, then _that_ is a bug.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] ARM: vfp: fix save and restore when running on pre-VFPv3 and CONFIG_VFPv3 set
  2012-10-13  9:02 ` Russell King - ARM Linux
@ 2012-10-13 10:50   ` Måns Rullgård
  2012-10-13 13:36     ` Russell King - ARM Linux
  0 siblings, 1 reply; 9+ messages in thread
From: Måns Rullgård @ 2012-10-13 10:50 UTC (permalink / raw)
  To: linux-arm-kernel

Russell King - ARM Linux <linux@arm.linux.org.uk> writes:

> On Sat, Oct 13, 2012 at 04:46:18AM +0000, Paul Walmsley wrote:
>> 
>> After commit 846a136881b8f73c1f74250bf6acfaa309cab1f2 ("ARM: vfp: fix
>> saving d16-d31 vfp registers on v6+ kernels"), the OMAP 2430SDP board
>> started crashing during boot with omap2plus_defconfig:
>> 
>> [    3.875122] mmcblk0: mmc0:e624 SD04G 3.69 GiB
>> [    3.915954]  mmcblk0: p1
>> [    4.086639] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
>> [    4.093719] Modules linked in:
>> [    4.096954] CPU: 0    Not tainted  (3.6.0-02232-g759e00b #570)
>> [    4.103149] PC is at vfp_reload_hw+0x1c/0x44
>> [    4.107666] LR is at __und_usr_fault_32+0x0/0x8
>> 
>> It turns out that the context save/restore fix unmasked a latent bug in 
>> commit 5aaf254409f8d58229107b59507a8235b715a960 ("ARM: 6203/1: Make VFPv3 
>> usable on ARMv6").  When CONFIG_VFPv3 is set, but the kernel is booted on 
>> a pre-VFPv3 core, the code attempts to save and restore the d16-d31 VFP 
>> registers.  These are only present on non-D16 VFPv3+, so the kernel dies 
>> with an undefined instruction exception.  The kernel didn't crash before 
>> commit 846a136 because the save and restore code only touched d0-d15, 
>> present on all VFP.
>> 
>> Fix to save and restore the d16-d31 registers only if they are
>> present.
>
> No.  VFPv3D16 HWCAP means that the VFP supports just 16 double registers
> - and it communicates this information to userspace.  If this bit is clear,
> and the VFP only has 16 double registers, then _that_ is a bug.

That bit will be clear on VFPv2 (because it's not v3), and this has only
16 D registers.  The high D registers are present if (vfpv3 && !vfpv3d16).
Pre-calculating this to avoid doing it in the context save/restore code
is probably a good idea.

-- 
M?ns Rullg?rd
mans at mansr.com

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] ARM: vfp: fix save and restore when running on pre-VFPv3 and CONFIG_VFPv3 set
  2012-10-13 10:50   ` Måns Rullgård
@ 2012-10-13 13:36     ` Russell King - ARM Linux
  2012-10-15  1:58       ` Paul Walmsley
  0 siblings, 1 reply; 9+ messages in thread
From: Russell King - ARM Linux @ 2012-10-13 13:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Oct 13, 2012 at 11:50:45AM +0100, M?ns Rullg?rd wrote:
> Russell King - ARM Linux <linux@arm.linux.org.uk> writes:
> 
> > On Sat, Oct 13, 2012 at 04:46:18AM +0000, Paul Walmsley wrote:
> >> 
> >> After commit 846a136881b8f73c1f74250bf6acfaa309cab1f2 ("ARM: vfp: fix
> >> saving d16-d31 vfp registers on v6+ kernels"), the OMAP 2430SDP board
> >> started crashing during boot with omap2plus_defconfig:
> >> 
> >> [    3.875122] mmcblk0: mmc0:e624 SD04G 3.69 GiB
> >> [    3.915954]  mmcblk0: p1
> >> [    4.086639] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
> >> [    4.093719] Modules linked in:
> >> [    4.096954] CPU: 0    Not tainted  (3.6.0-02232-g759e00b #570)
> >> [    4.103149] PC is at vfp_reload_hw+0x1c/0x44
> >> [    4.107666] LR is at __und_usr_fault_32+0x0/0x8
> >> 
> >> It turns out that the context save/restore fix unmasked a latent bug in 
> >> commit 5aaf254409f8d58229107b59507a8235b715a960 ("ARM: 6203/1: Make VFPv3 
> >> usable on ARMv6").  When CONFIG_VFPv3 is set, but the kernel is booted on 
> >> a pre-VFPv3 core, the code attempts to save and restore the d16-d31 VFP 
> >> registers.  These are only present on non-D16 VFPv3+, so the kernel dies 
> >> with an undefined instruction exception.  The kernel didn't crash before 
> >> commit 846a136 because the save and restore code only touched d0-d15, 
> >> present on all VFP.
> >> 
> >> Fix to save and restore the d16-d31 registers only if they are
> >> present.
> >
> > No.  VFPv3D16 HWCAP means that the VFP supports just 16 double registers
> > - and it communicates this information to userspace.  If this bit is clear,
> > and the VFP only has 16 double registers, then _that_ is a bug.
> 
> That bit will be clear on VFPv2 (because it's not v3), and this has only
> 16 D registers.  The high D registers are present if (vfpv3 && !vfpv3d16).
> Pre-calculating this to avoid doing it in the context save/restore code
> is probably a good idea.

No.  What we should do in that case is get away from this mixed logic.
The HWCAP fields should _always_ be telling us what we have, not what
we don't have.  So, this means we should have VFPv3 and VFPv4 bits,
but also VFPD16 to say "yes, we have d16-d31 registers too".

And, these bits should be set as follows:

	VFP architecture	flags
	VFPv2			VFP
	VFPv3d16		VFP|VFPv3|VFPv3D16
	VFPv3			VFP|VFPv3|VFPD16
	VFPv4			VFP|VFPv3|VFPv4|VFPD16

whereas, at the present time we have the silly situation where VFPv3D16
gives us two bits of information.  Note that the above combination sorts
this out for ever, and doesn't involve any userspace changes, and makes
this stuff much more sane - and ends up giving everyone exactly what they
need to make the appropriate decisions.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] ARM: vfp: fix save and restore when running on pre-VFPv3 and CONFIG_VFPv3 set
  2012-10-13 13:36     ` Russell King - ARM Linux
@ 2012-10-15  1:58       ` Paul Walmsley
  2012-10-15  2:39         ` Mans Rullgard
  0 siblings, 1 reply; 9+ messages in thread
From: Paul Walmsley @ 2012-10-15  1:58 UTC (permalink / raw)
  To: linux-arm-kernel


After commit 846a136881b8f73c1f74250bf6acfaa309cab1f2 ("ARM: vfp: fix
saving d16-d31 vfp registers on v6+ kernels"), the OMAP 2430SDP board
started crashing during boot with omap2plus_defconfig:

[    3.875122] mmcblk0: mmc0:e624 SD04G 3.69 GiB
[    3.915954]  mmcblk0: p1
[    4.086639] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
[    4.093719] Modules linked in:
[    4.096954] CPU: 0    Not tainted  (3.6.0-02232-g759e00b #570)
[    4.103149] PC is at vfp_reload_hw+0x1c/0x44
[    4.107666] LR is at __und_usr_fault_32+0x0/0x8

It turns out that the context save/restore fix unmasked a latent bug
in commit 5aaf254409f8d58229107b59507a8235b715a960 ("ARM: 6203/1: Make
VFPv3 usable on ARMv6").  When CONFIG_VFPv3 is set, but the kernel is
booted on a pre-VFPv3 core, the code attempts to save and restore the
d16-d31 VFP registers.  These are only present on non-D16 VFPv3+, so
this results in an undefined instruction exception.  The code didn't
crash before commit 846a136 because the save and restore code was
only touching d0-d15, present on all VFP.

Fix by implementing a request from Russell King to add a new HWCAP
flag that affirmatively indicates the presence of the d16-d31
registers:

   http://marc.info/?l=linux-arm-kernel&m=135013547905283&w=2

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dave Martin <dave.martin@linaro.org>
Cc: M?ns Rullg?rd <mans.rullgard@linaro.org>
---
Applies on v3.7-rc1.

 arch/arm/include/asm/vfpmacros.h  |   12 ++++++------
 arch/arm/include/uapi/asm/hwcap.h |    3 ++-
 arch/arm/vfp/vfpmodule.c          |    9 ++++++---
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/arch/arm/include/asm/vfpmacros.h b/arch/arm/include/asm/vfpmacros.h
index 6a6f1e4..77f40ea 100644
--- a/arch/arm/include/asm/vfpmacros.h
+++ b/arch/arm/include/asm/vfpmacros.h
@@ -27,9 +27,9 @@
 #if __LINUX_ARM_ARCH__ <= 6
 	ldr	\tmp, =elf_hwcap		    @ may not have MVFR regs
 	ldr	\tmp, [\tmp, #0]
-	tst	\tmp, #HWCAP_VFPv3D16
-	ldceql	p11, cr0, [\base],#32*4		    @ FLDMIAD \base!, {d16-d31}
-	addne	\base, \base, #32*4		    @ step over unused register space
+	tst	\tmp, #HWCAP_VFPD16
+	ldcnel	p11, cr0, [\base],#32*4		    @ FLDMIAD \base!, {d16-d31}
+	addeq	\base, \base, #32*4		    @ step over unused register space
 #else
 	VFPFMRX	\tmp, MVFR0			    @ Media and VFP Feature Register 0
 	and	\tmp, \tmp, #MVFR0_A_SIMD_MASK	    @ A_SIMD field
@@ -51,9 +51,9 @@
 #if __LINUX_ARM_ARCH__ <= 6
 	ldr	\tmp, =elf_hwcap		    @ may not have MVFR regs
 	ldr	\tmp, [\tmp, #0]
-	tst	\tmp, #HWCAP_VFPv3D16
-	stceql	p11, cr0, [\base],#32*4		    @ FSTMIAD \base!, {d16-d31}
-	addne	\base, \base, #32*4		    @ step over unused register space
+	tst	\tmp, #HWCAP_VFPD16
+	stcnel	p11, cr0, [\base],#32*4		    @ FSTMIAD \base!, {d16-d31}
+	addeq	\base, \base, #32*4		    @ step over unused register space
 #else
 	VFPFMRX	\tmp, MVFR0			    @ Media and VFP Feature Register 0
 	and	\tmp, \tmp, #MVFR0_A_SIMD_MASK	    @ A_SIMD field
diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
index f254f65..b1555d0 100644
--- a/arch/arm/include/uapi/asm/hwcap.h
+++ b/arch/arm/include/uapi/asm/hwcap.h
@@ -18,11 +18,12 @@
 #define HWCAP_THUMBEE	(1 << 11)
 #define HWCAP_NEON	(1 << 12)
 #define HWCAP_VFPv3	(1 << 13)
-#define HWCAP_VFPv3D16	(1 << 14)
+#define HWCAP_VFPv3D16	(1 << 14)	/* also set for VFPv4-D16 */
 #define HWCAP_TLS	(1 << 15)
 #define HWCAP_VFPv4	(1 << 16)
 #define HWCAP_IDIVA	(1 << 17)
 #define HWCAP_IDIVT	(1 << 18)
+#define HWCAP_VFPD16	(1 << 19)	/* set if VFP has 32 regs (not 16) */
 #define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)
 
 
diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index c834b32..f3e611c 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -701,11 +701,14 @@ static int __init vfp_init(void)
 			elf_hwcap |= HWCAP_VFPv3;
 
 			/*
-			 * Check for VFPv3 D16. CPUs in this configuration
-			 * only have 16 x 64bit registers.
+			 * Check for VFPv3 D16 and VFPv4 D16.  CPUs in
+			 * this configuration only have 16 x 64bit
+			 * registers.
 			 */
 			if (((fmrx(MVFR0) & MVFR0_A_SIMD_MASK)) == 1)
-				elf_hwcap |= HWCAP_VFPv3D16;
+				elf_hwcap |= HWCAP_VFPv3D16; /* also v4-D16 */
+			else
+				elf_hwcap |= HWCAP_VFPD16;
 		}
 #endif
 		/*
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] ARM: vfp: fix save and restore when running on pre-VFPv3 and CONFIG_VFPv3 set
  2012-10-15  1:58       ` Paul Walmsley
@ 2012-10-15  2:39         ` Mans Rullgard
  2012-10-15  5:24           ` [PATCH v3] " Paul Walmsley
  0 siblings, 1 reply; 9+ messages in thread
From: Mans Rullgard @ 2012-10-15  2:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 15 October 2012 02:58, Paul Walmsley <paul@pwsan.com> wrote:
>
> Fix by implementing a request from Russell King to add a new HWCAP
> flag that affirmatively indicates the presence of the d16-d31
> registers:

[...]

> diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
> index f254f65..b1555d0 100644
> --- a/arch/arm/include/uapi/asm/hwcap.h
> +++ b/arch/arm/include/uapi/asm/hwcap.h
> @@ -18,11 +18,12 @@
>  #define HWCAP_THUMBEE  (1 << 11)
>  #define HWCAP_NEON     (1 << 12)
>  #define HWCAP_VFPv3    (1 << 13)
> -#define HWCAP_VFPv3D16 (1 << 14)
> +#define HWCAP_VFPv3D16 (1 << 14)       /* also set for VFPv4-D16 */
>  #define HWCAP_TLS      (1 << 15)
>  #define HWCAP_VFPv4    (1 << 16)
>  #define HWCAP_IDIVA    (1 << 17)
>  #define HWCAP_IDIVT    (1 << 18)
> +#define HWCAP_VFPD16   (1 << 19)       /* set if VFP has 32 regs (not 16) */
>  #define HWCAP_IDIV     (HWCAP_IDIVA | HWCAP_IDIVT)

Please consider calling the new flag VFPD32 or so instead.  The D16 suffix
usually signifies an implementation with 16 registers, i.e. the opposite of
what this flag does.  Using the same tag to mean both things depending on
whether there's a 'v3' before it is nothing but confusing.

-- 
Mans Rullgard / mru

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3] ARM: vfp: fix save and restore when running on pre-VFPv3 and CONFIG_VFPv3 set
  2012-10-15  2:39         ` Mans Rullgard
@ 2012-10-15  5:24           ` Paul Walmsley
  2012-10-15 17:53             ` Tony Lindgren
  0 siblings, 1 reply; 9+ messages in thread
From: Paul Walmsley @ 2012-10-15  5:24 UTC (permalink / raw)
  To: linux-arm-kernel


After commit 846a136881b8f73c1f74250bf6acfaa309cab1f2 ("ARM: vfp: fix
saving d16-d31 vfp registers on v6+ kernels"), the OMAP 2430SDP board
started crashing during boot with omap2plus_defconfig:

[    3.875122] mmcblk0: mmc0:e624 SD04G 3.69 GiB
[    3.915954]  mmcblk0: p1
[    4.086639] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
[    4.093719] Modules linked in:
[    4.096954] CPU: 0    Not tainted  (3.6.0-02232-g759e00b #570)
[    4.103149] PC is at vfp_reload_hw+0x1c/0x44
[    4.107666] LR is at __und_usr_fault_32+0x0/0x8

It turns out that the context save/restore fix unmasked a latent bug
in commit 5aaf254409f8d58229107b59507a8235b715a960 ("ARM: 6203/1: Make
VFPv3 usable on ARMv6").  When CONFIG_VFPv3 is set, but the kernel is
booted on a pre-VFPv3 core, the code attempts to save and restore the
d16-d31 VFP registers.  These are only present on non-D16 VFPv3+, so
this results in an undefined instruction exception.  The code didn't
crash before commit 846a136 because the save and restore code was
only touching d0-d15, present on all VFP.

Fix by implementing a request from Russell King to add a new HWCAP
flag that affirmatively indicates the presence of the d16-d31
registers:

   http://marc.info/?l=linux-arm-kernel&m=135013547905283&w=2

and some feedback from M?ns to clarify the name of the HWCAP flag.

Signed-off-by: Paul Walmsley <paul@pwsan.com>
Cc: Tony Lindgren <tony@atomide.com>
Cc: Russell King <rmk+kernel@arm.linux.org.uk>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Dave Martin <dave.martin@linaro.org>
Cc: M?ns Rullg?rd <mans.rullgard@linaro.org>
---
Thanks for the suggestion, M?ns; certainly a readability improvement.

 arch/arm/include/asm/vfpmacros.h  |   12 ++++++------
 arch/arm/include/uapi/asm/hwcap.h |    3 ++-
 arch/arm/vfp/vfpmodule.c          |    9 ++++++---
 3 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/arch/arm/include/asm/vfpmacros.h b/arch/arm/include/asm/vfpmacros.h
index 6a6f1e4..301c1db 100644
--- a/arch/arm/include/asm/vfpmacros.h
+++ b/arch/arm/include/asm/vfpmacros.h
@@ -27,9 +27,9 @@
 #if __LINUX_ARM_ARCH__ <= 6
 	ldr	\tmp, =elf_hwcap		    @ may not have MVFR regs
 	ldr	\tmp, [\tmp, #0]
-	tst	\tmp, #HWCAP_VFPv3D16
-	ldceql	p11, cr0, [\base],#32*4		    @ FLDMIAD \base!, {d16-d31}
-	addne	\base, \base, #32*4		    @ step over unused register space
+	tst	\tmp, #HWCAP_VFPD32
+	ldcnel	p11, cr0, [\base],#32*4		    @ FLDMIAD \base!, {d16-d31}
+	addeq	\base, \base, #32*4		    @ step over unused register space
 #else
 	VFPFMRX	\tmp, MVFR0			    @ Media and VFP Feature Register 0
 	and	\tmp, \tmp, #MVFR0_A_SIMD_MASK	    @ A_SIMD field
@@ -51,9 +51,9 @@
 #if __LINUX_ARM_ARCH__ <= 6
 	ldr	\tmp, =elf_hwcap		    @ may not have MVFR regs
 	ldr	\tmp, [\tmp, #0]
-	tst	\tmp, #HWCAP_VFPv3D16
-	stceql	p11, cr0, [\base],#32*4		    @ FSTMIAD \base!, {d16-d31}
-	addne	\base, \base, #32*4		    @ step over unused register space
+	tst	\tmp, #HWCAP_VFPD32
+	stcnel	p11, cr0, [\base],#32*4		    @ FSTMIAD \base!, {d16-d31}
+	addeq	\base, \base, #32*4		    @ step over unused register space
 #else
 	VFPFMRX	\tmp, MVFR0			    @ Media and VFP Feature Register 0
 	and	\tmp, \tmp, #MVFR0_A_SIMD_MASK	    @ A_SIMD field
diff --git a/arch/arm/include/uapi/asm/hwcap.h b/arch/arm/include/uapi/asm/hwcap.h
index f254f65..3688fd1 100644
--- a/arch/arm/include/uapi/asm/hwcap.h
+++ b/arch/arm/include/uapi/asm/hwcap.h
@@ -18,11 +18,12 @@
 #define HWCAP_THUMBEE	(1 << 11)
 #define HWCAP_NEON	(1 << 12)
 #define HWCAP_VFPv3	(1 << 13)
-#define HWCAP_VFPv3D16	(1 << 14)
+#define HWCAP_VFPv3D16	(1 << 14)	/* also set for VFPv4-D16 */
 #define HWCAP_TLS	(1 << 15)
 #define HWCAP_VFPv4	(1 << 16)
 #define HWCAP_IDIVA	(1 << 17)
 #define HWCAP_IDIVT	(1 << 18)
+#define HWCAP_VFPD32	(1 << 19)	/* set if VFP has 32 regs (not 16) */
 #define HWCAP_IDIV	(HWCAP_IDIVA | HWCAP_IDIVT)
 
 
diff --git a/arch/arm/vfp/vfpmodule.c b/arch/arm/vfp/vfpmodule.c
index c834b32..3b44e0d 100644
--- a/arch/arm/vfp/vfpmodule.c
+++ b/arch/arm/vfp/vfpmodule.c
@@ -701,11 +701,14 @@ static int __init vfp_init(void)
 			elf_hwcap |= HWCAP_VFPv3;
 
 			/*
-			 * Check for VFPv3 D16. CPUs in this configuration
-			 * only have 16 x 64bit registers.
+			 * Check for VFPv3 D16 and VFPv4 D16.  CPUs in
+			 * this configuration only have 16 x 64bit
+			 * registers.
 			 */
 			if (((fmrx(MVFR0) & MVFR0_A_SIMD_MASK)) == 1)
-				elf_hwcap |= HWCAP_VFPv3D16;
+				elf_hwcap |= HWCAP_VFPv3D16; /* also v4-D16 */
+			else
+				elf_hwcap |= HWCAP_VFPD32;
 		}
 #endif
 		/*
-- 
1.7.10.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v3] ARM: vfp: fix save and restore when running on pre-VFPv3 and CONFIG_VFPv3 set
  2012-10-15  5:24           ` [PATCH v3] " Paul Walmsley
@ 2012-10-15 17:53             ` Tony Lindgren
  2012-10-16 17:13               ` Jon Hunter
  0 siblings, 1 reply; 9+ messages in thread
From: Tony Lindgren @ 2012-10-15 17:53 UTC (permalink / raw)
  To: linux-arm-kernel

* Paul Walmsley <paul@pwsan.com> [121014 22:26]:
> 
> After commit 846a136881b8f73c1f74250bf6acfaa309cab1f2 ("ARM: vfp: fix
> saving d16-d31 vfp registers on v6+ kernels"), the OMAP 2430SDP board
> started crashing during boot with omap2plus_defconfig:
> 
> [    3.875122] mmcblk0: mmc0:e624 SD04G 3.69 GiB
> [    3.915954]  mmcblk0: p1
> [    4.086639] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
> [    4.093719] Modules linked in:
> [    4.096954] CPU: 0    Not tainted  (3.6.0-02232-g759e00b #570)
> [    4.103149] PC is at vfp_reload_hw+0x1c/0x44
> [    4.107666] LR is at __und_usr_fault_32+0x0/0x8
> 
> It turns out that the context save/restore fix unmasked a latent bug
> in commit 5aaf254409f8d58229107b59507a8235b715a960 ("ARM: 6203/1: Make
> VFPv3 usable on ARMv6").  When CONFIG_VFPv3 is set, but the kernel is
> booted on a pre-VFPv3 core, the code attempts to save and restore the
> d16-d31 VFP registers.  These are only present on non-D16 VFPv3+, so
> this results in an undefined instruction exception.  The code didn't
> crash before commit 846a136 because the save and restore code was
> only touching d0-d15, present on all VFP.
> 
> Fix by implementing a request from Russell King to add a new HWCAP
> flag that affirmatively indicates the presence of the d16-d31
> registers:
> 
>    http://marc.info/?l=linux-arm-kernel&m=135013547905283&w=2
> 
> and some feedback from M?ns to clarify the name of the HWCAP flag.
> 
> Signed-off-by: Paul Walmsley <paul@pwsan.com>
> Cc: Tony Lindgren <tony@atomide.com>
> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Dave Martin <dave.martin@linaro.org>
> Cc: M?ns Rullg?rd <mans.rullgard@linaro.org>

This fixes the error above on my 2430sdp:

Tested-by: Tony Lindgren <tony@atomide.com>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3] ARM: vfp: fix save and restore when running on pre-VFPv3 and CONFIG_VFPv3 set
  2012-10-15 17:53             ` Tony Lindgren
@ 2012-10-16 17:13               ` Jon Hunter
  0 siblings, 0 replies; 9+ messages in thread
From: Jon Hunter @ 2012-10-16 17:13 UTC (permalink / raw)
  To: linux-arm-kernel


On 10/15/2012 12:53 PM, Tony Lindgren wrote:
> * Paul Walmsley <paul@pwsan.com> [121014 22:26]:
>>
>> After commit 846a136881b8f73c1f74250bf6acfaa309cab1f2 ("ARM: vfp: fix
>> saving d16-d31 vfp registers on v6+ kernels"), the OMAP 2430SDP board
>> started crashing during boot with omap2plus_defconfig:
>>
>> [    3.875122] mmcblk0: mmc0:e624 SD04G 3.69 GiB
>> [    3.915954]  mmcblk0: p1
>> [    4.086639] Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
>> [    4.093719] Modules linked in:
>> [    4.096954] CPU: 0    Not tainted  (3.6.0-02232-g759e00b #570)
>> [    4.103149] PC is at vfp_reload_hw+0x1c/0x44
>> [    4.107666] LR is at __und_usr_fault_32+0x0/0x8
>>
>> It turns out that the context save/restore fix unmasked a latent bug
>> in commit 5aaf254409f8d58229107b59507a8235b715a960 ("ARM: 6203/1: Make
>> VFPv3 usable on ARMv6").  When CONFIG_VFPv3 is set, but the kernel is
>> booted on a pre-VFPv3 core, the code attempts to save and restore the
>> d16-d31 VFP registers.  These are only present on non-D16 VFPv3+, so
>> this results in an undefined instruction exception.  The code didn't
>> crash before commit 846a136 because the save and restore code was
>> only touching d0-d15, present on all VFP.
>>
>> Fix by implementing a request from Russell King to add a new HWCAP
>> flag that affirmatively indicates the presence of the d16-d31
>> registers:
>>
>>    http://marc.info/?l=linux-arm-kernel&m=135013547905283&w=2
>>
>> and some feedback from M?ns to clarify the name of the HWCAP flag.
>>
>> Signed-off-by: Paul Walmsley <paul@pwsan.com>
>> Cc: Tony Lindgren <tony@atomide.com>
>> Cc: Russell King <rmk+kernel@arm.linux.org.uk>
>> Cc: Catalin Marinas <catalin.marinas@arm.com>
>> Cc: Dave Martin <dave.martin@linaro.org>
>> Cc: M?ns Rullg?rd <mans.rullgard@linaro.org>
> 
> This fixes the error above on my 2430sdp:
> 
> Tested-by: Tony Lindgren <tony@atomide.com>

Fixes same error on 2420 H4.

Tested-by: Jon Hunter <jon-hunter@ti.com>

Cheers
Jon

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2012-10-16 17:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-10-13  4:46 [PATCH] ARM: vfp: fix save and restore when running on pre-VFPv3 and CONFIG_VFPv3 set Paul Walmsley
2012-10-13  9:02 ` Russell King - ARM Linux
2012-10-13 10:50   ` Måns Rullgård
2012-10-13 13:36     ` Russell King - ARM Linux
2012-10-15  1:58       ` Paul Walmsley
2012-10-15  2:39         ` Mans Rullgard
2012-10-15  5:24           ` [PATCH v3] " Paul Walmsley
2012-10-15 17:53             ` Tony Lindgren
2012-10-16 17:13               ` Jon Hunter

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).