All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] ABI updates
@ 2014-07-04 19:51 Russell King - ARM Linux
  2014-07-04 19:52 ` [PATCH 1/4] ARM: alignment: save last kernel aligned fault location Russell King
                   ` (6 more replies)
  0 siblings, 7 replies; 27+ messages in thread
From: Russell King - ARM Linux @ 2014-07-04 19:51 UTC (permalink / raw)
  To: linux-arm-kernel

As a result of the recent discussions about android, several issues
have been identified with existing kernels:

1. HWCAP_SWP does not properly reflect whether the SWP instruction
   should be used.  Turn HWCAP_SWP off on ARMv6+ where the exclusives
   are available.

2. SWP is unsafe when running on a SMP CPU; there is no bus locking
   between the read and write parts of the instruction execution.
   Force SWP emulation on ARMv7+ where we can disable the SWP
   instruction.

3. Reporting of alignment faults - kernel mode faults are silent due to
   the requirements of the network stack, but we can note where the
   last one occurs.

Testing reveals that at least my OMAP4430 userspace (supplied from TI)
executes lots of SWP instructions, which is something that wasn't known
before this patch.

Many systems today are not configured with SWP emulation enabled on SMP.
This means that the SWP instruction is available, but unsafe.

 arch/arm/kernel/setup.c       | 29 ++++++++++++++++++++++-------
 arch/arm/kernel/swp_emulate.c |  4 ++++
 arch/arm/mm/Kconfig           |  2 +-
 arch/arm/mm/alignment.c       |  4 +++-
 4 files changed, 30 insertions(+), 9 deletions(-)

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

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

* [PATCH 1/4] ARM: alignment: save last kernel aligned fault location
  2014-07-04 19:51 [PATCH 0/4] ABI updates Russell King - ARM Linux
@ 2014-07-04 19:52 ` Russell King
  2014-07-04 19:52 ` [PATCH 2/4] ARM: SWP emulation: always enable when SMP is enabled Russell King
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 27+ messages in thread
From: Russell King @ 2014-07-04 19:52 UTC (permalink / raw)
  To: linux-arm-kernel

Save and report (via the procfs file) the last kernel unaligned fault
location.  This allows us to trivially inspect where the last fault
happened for cases which we don't expect to occur.

Since we expect the kernel to generate misalignment faults (due to
the networking layer), even when warnings are enabled, we don't log
them for the kernel.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mm/alignment.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c
index b8cb1a2688a0..0c1ab49e5f7b 100644
--- a/arch/arm/mm/alignment.c
+++ b/arch/arm/mm/alignment.c
@@ -76,6 +76,7 @@
 
 static unsigned long ai_user;
 static unsigned long ai_sys;
+static void *ai_sys_last_pc;
 static unsigned long ai_skipped;
 static unsigned long ai_half;
 static unsigned long ai_word;
@@ -130,7 +131,7 @@ static const char *usermode_action[] = {
 static int alignment_proc_show(struct seq_file *m, void *v)
 {
 	seq_printf(m, "User:\t\t%lu\n", ai_user);
-	seq_printf(m, "System:\t\t%lu\n", ai_sys);
+	seq_printf(m, "System:\t\t%lu (%pF)\n", ai_sys, ai_sys_last_pc);
 	seq_printf(m, "Skipped:\t%lu\n", ai_skipped);
 	seq_printf(m, "Half:\t\t%lu\n", ai_half);
 	seq_printf(m, "Word:\t\t%lu\n", ai_word);
@@ -794,6 +795,7 @@ do_alignment(unsigned long addr, unsigned int fsr, struct pt_regs *regs)
 		goto user;
 
 	ai_sys += 1;
+	ai_sys_last_pc = (void *)instruction_pointer(regs);
 
  fixup:
 
-- 
1.8.3.1

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

* [PATCH 2/4] ARM: SWP emulation: always enable when SMP is enabled
  2014-07-04 19:51 [PATCH 0/4] ABI updates Russell King - ARM Linux
  2014-07-04 19:52 ` [PATCH 1/4] ARM: alignment: save last kernel aligned fault location Russell King
@ 2014-07-04 19:52 ` Russell King
  2014-07-04 19:52 ` [PATCH 3/4] ARM: SWP emulation: only initialise on ARMv7 CPUs Russell King
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 27+ messages in thread
From: Russell King @ 2014-07-04 19:52 UTC (permalink / raw)
  To: linux-arm-kernel

SWP is deprecated in ARMv6 and ARMv7 CPUs, but more importantly, when
running on a SMP system, SWP doesn't guarantee atomicity.  This means
it can't really be used (by userspace) for locking purposes in a SMP
environment.

Currently, many configurations leave the SWP emulation disabled, which
means we never know if userspace executes this instruction on ARMv7
hardware.  Rectify this by enabling SWP emulation for ARMv7 with SMP
(where we can trap the instruction.)

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/mm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index c348eaee7ee2..c78c7db6aa83 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -669,7 +669,7 @@ config ARM_VIRT_EXT
 	  details.
 
 config SWP_EMULATE
-	bool "Emulate SWP/SWPB instructions"
+	bool "Emulate SWP/SWPB instructions" if !SMP
 	depends on CPU_V7
 	default y if SMP
 	select HAVE_PROC_CPU if PROC_FS
-- 
1.8.3.1

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

* [PATCH 3/4] ARM: SWP emulation: only initialise on ARMv7 CPUs
  2014-07-04 19:51 [PATCH 0/4] ABI updates Russell King - ARM Linux
  2014-07-04 19:52 ` [PATCH 1/4] ARM: alignment: save last kernel aligned fault location Russell King
  2014-07-04 19:52 ` [PATCH 2/4] ARM: SWP emulation: always enable when SMP is enabled Russell King
@ 2014-07-04 19:52 ` Russell King
  2014-07-04 19:52 ` [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives Russell King
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 27+ messages in thread
From: Russell King @ 2014-07-04 19:52 UTC (permalink / raw)
  To: linux-arm-kernel

Previous CPUs do not have the ability to trap SWP instructions, so
it's pointless initialising this code there.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/kernel/swp_emulate.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/kernel/swp_emulate.c b/arch/arm/kernel/swp_emulate.c
index b1b89882b113..67ca8578c6d8 100644
--- a/arch/arm/kernel/swp_emulate.c
+++ b/arch/arm/kernel/swp_emulate.c
@@ -27,6 +27,7 @@
 #include <linux/perf_event.h>
 
 #include <asm/opcodes.h>
+#include <asm/system_info.h>
 #include <asm/traps.h>
 #include <asm/uaccess.h>
 
@@ -266,6 +267,9 @@ static struct undef_hook swp_hook = {
  */
 static int __init swp_emulation_init(void)
 {
+	if (cpu_architecture() < CPU_ARCH_ARMv7)
+		return 0;
+
 #ifdef CONFIG_PROC_FS
 	if (!proc_create("cpu/swp_emulation", S_IRUGO, NULL, &proc_status_fops))
 		return -ENOMEM;
-- 
1.8.3.1

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-04 19:51 [PATCH 0/4] ABI updates Russell King - ARM Linux
                   ` (2 preceding siblings ...)
  2014-07-04 19:52 ` [PATCH 3/4] ARM: SWP emulation: only initialise on ARMv7 CPUs Russell King
@ 2014-07-04 19:52 ` Russell King
  2014-07-04 20:11   ` Arnd Bergmann
  2014-07-04 20:12 ` [PATCH 0/4] ABI updates Arnd Bergmann
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 27+ messages in thread
From: Russell King @ 2014-07-04 19:52 UTC (permalink / raw)
  To: linux-arm-kernel

When the CPU has support for the byte and word exclusive operations,
userspace should use them in preference to the SWP instructions.
Detect the presence of these instructions by reading the ISAR CPU ID
registers and adjust the ELF HWCAP mask appropriately.

Note that ARM1136 < r1p0 has no ISAR4, so this is explicitly detected
and the test disabled, leaving the current situation where HWCAP_SWP
is set.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/kernel/setup.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index 8a16ee5d8a95..84db893dedc2 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -393,19 +393,34 @@ static void __init cpuid_init_hwcaps(void)
 		elf_hwcap |= HWCAP_LPAE;
 }
 
-static void __init feat_v6_fixup(void)
+static void __init elf_hwcap_fixup(void)
 {
-	int id = read_cpuid_id();
-
-	if ((id & 0xff0f0000) != 0x41070000)
-		return;
+	unsigned id = read_cpuid_id();
+	unsigned sync_prim;
 
 	/*
 	 * HWCAP_TLS is available only on 1136 r1p0 and later,
 	 * see also kuser_get_tls_init.
 	 */
-	if ((((id >> 4) & 0xfff) == 0xb36) && (((id >> 20) & 3) == 0))
+	if (read_cpuid_part() == ARM_CPU_PART_ARM1136 &&
+	    ((id >> 20) & 3) == 0) {
 		elf_hwcap &= ~HWCAP_TLS;
+		return;
+	}
+
+	/* Verify if CPUID scheme is implemented */
+	if ((id & 0x000f0000) != 0x000f0000)
+		return;
+
+	/*
+	 * If the CPU supports LDREX/STREX and LDREXB/STREXB,
+	 * avoid advertising SWP; it may not be atomic with
+	 * multiprocessing cores.
+	 */
+	sync_prim = ((read_cpuid_ext(CPUID_EXT_ISAR3) >> 8) & 0xf0) |
+		    ((read_cpuid_ext(CPUID_EXT_ISAR4) >> 20) & 0x0f);
+	if (sync_prim >= 0x13)
+		elf_hwcap &= ~HWCAP_SWP;
 }
 
 /*
@@ -609,7 +624,7 @@ static void __init setup_processor(void)
 #endif
 	erratum_a15_798181_init();
 
-	feat_v6_fixup();
+	elf_hwcap_fixup();
 
 	cacheid_init();
 	cpu_init();
-- 
1.8.3.1

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-04 19:52 ` [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives Russell King
@ 2014-07-04 20:11   ` Arnd Bergmann
  2014-07-04 20:51     ` Russell King - ARM Linux
  0 siblings, 1 reply; 27+ messages in thread
From: Arnd Bergmann @ 2014-07-04 20:11 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 04 July 2014 20:52:17 Russell King wrote:
>         /*
>          * HWCAP_TLS is available only on 1136 r1p0 and later,
>          * see also kuser_get_tls_init.
>          */
> -       if ((((id >> 4) & 0xfff) == 0xb36) && (((id >> 20) & 3) == 0))
> +       if (read_cpuid_part() == ARM_CPU_PART_ARM1136 &&
> +           ((id >> 20) & 3) == 0) {
>                 elf_hwcap &= ~HWCAP_TLS;
> +               return;
> +       }
> 

Would it make sense to tie this check to the ARMv6K architecture level?

I see that cpu_architecture currently reports the same value for plain
ARMv6 (1136r0) and for ARMv6K (1136r1+ or any other ARM11), but we already
distinguish them at compile time in a number of places and it seems you
make the exact same distinction here at runtime.

	Arnd

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

* [PATCH 0/4] ABI updates
  2014-07-04 19:51 [PATCH 0/4] ABI updates Russell King - ARM Linux
                   ` (3 preceding siblings ...)
  2014-07-04 19:52 ` [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives Russell King
@ 2014-07-04 20:12 ` Arnd Bergmann
  2014-07-07 11:19 ` Tony Lindgren
  2014-07-07 13:52 ` Catalin Marinas
  6 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2014-07-04 20:12 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 04 July 2014 20:51:34 Russell King - ARM Linux wrote:
> As a result of the recent discussions about android, several issues
> have been identified with existing kernels:
> 
> 1. HWCAP_SWP does not properly reflect whether the SWP instruction
>    should be used.  Turn HWCAP_SWP off on ARMv6+ where the exclusives
>    are available.
> 
> 2. SWP is unsafe when running on a SMP CPU; there is no bus locking
>    between the read and write parts of the instruction execution.
>    Force SWP emulation on ARMv7+ where we can disable the SWP
>    instruction.
> 
> 3. Reporting of alignment faults - kernel mode faults are silent due to
>    the requirements of the network stack, but we can note where the
>    last one occurs.
> 
> Testing reveals that at least my OMAP4430 userspace (supplied from TI)
> executes lots of SWP instructions, which is something that wasn't known
> before this patch.
> 
> Many systems today are not configured with SWP emulation enabled on SMP.
> This means that the SWP instruction is available, but unsafe.

Looks all good to me, just one question for the last patch.

	Arnd

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-04 20:11   ` Arnd Bergmann
@ 2014-07-04 20:51     ` Russell King - ARM Linux
  2014-07-04 20:58       ` Arnd Bergmann
  2014-07-07  9:34       ` Will Deacon
  0 siblings, 2 replies; 27+ messages in thread
From: Russell King - ARM Linux @ 2014-07-04 20:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jul 04, 2014 at 10:11:06PM +0200, Arnd Bergmann wrote:
> On Friday 04 July 2014 20:52:17 Russell King wrote:
> >         /*
> >          * HWCAP_TLS is available only on 1136 r1p0 and later,
> >          * see also kuser_get_tls_init.
> >          */
> > -       if ((((id >> 4) & 0xfff) == 0xb36) && (((id >> 20) & 3) == 0))
> > +       if (read_cpuid_part() == ARM_CPU_PART_ARM1136 &&
> > +           ((id >> 20) & 3) == 0) {
> >                 elf_hwcap &= ~HWCAP_TLS;
> > +               return;
> > +       }
> > 
> 
> Would it make sense to tie this check to the ARMv6K architecture level?
> 
> I see that cpu_architecture currently reports the same value for plain
> ARMv6 (1136r0) and for ARMv6K (1136r1+ or any other ARM11), but we already
> distinguish them at compile time in a number of places and it seems you
> make the exact same distinction here at runtime.

Hmm, we need guidance from ARM people on that.

There may well be a better way to detect between ARMv6 and ARMv6K, which
is given by the architecture spec.  G7.3 of an early DDI0406 says that
the MPIDR (mp affinity register) aliases to MIDR for ARMv6, but is of
course implemented for ARMv6K.

This seems to be carried through to the latest ARM ARM.  So it seems
this would be a more correct way to tell ARMv6 from ARMv6K.

If so, we can certainly expand cpu_architecture() to detect between the
two and add a CPU_ARCH_ARMv6K in there.

Let's see what Will has to say about that when he's next around...
though I think it'll require another trawl through lots of
documentation.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-04 20:51     ` Russell King - ARM Linux
@ 2014-07-04 20:58       ` Arnd Bergmann
  2014-07-04 21:48         ` Russell King - ARM Linux
  2014-07-07 11:02         ` Catalin Marinas
  2014-07-07  9:34       ` Will Deacon
  1 sibling, 2 replies; 27+ messages in thread
From: Arnd Bergmann @ 2014-07-04 20:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 04 July 2014 21:51:44 Russell King - ARM Linux wrote:
> Hmm, we need guidance from ARM people on that.
> 
> There may well be a better way to detect between ARMv6 and ARMv6K, which
> is given by the architecture spec.  G7.3 of an early DDI0406 says that
> the MPIDR (mp affinity register) aliases to MIDR for ARMv6, but is of
> course implemented for ARMv6K.
> 
> This seems to be carried through to the latest ARM ARM.  So it seems
> this would be a more correct way to tell ARMv6 from ARMv6K.
> 
> If so, we can certainly expand cpu_architecture() to detect between the
> two and add a CPU_ARCH_ARMv6K in there.
> 
> Let's see what Will has to say about that when he's next around...
> though I think it'll require another trawl through lots of
> documentation.

I was thinking of a simpler check in __get_cpu_architecture, just
checking if the CPUID is ARM1136r0 since that is the only ARMv6
CPU core we support. Anything else that we currently report as
ARMv6 is actually ARMv6K.

	Arnd

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-04 20:58       ` Arnd Bergmann
@ 2014-07-04 21:48         ` Russell King - ARM Linux
  2014-07-05 18:46           ` Arnd Bergmann
  2014-07-07 11:02         ` Catalin Marinas
  1 sibling, 1 reply; 27+ messages in thread
From: Russell King - ARM Linux @ 2014-07-04 21:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jul 04, 2014 at 10:58:04PM +0200, Arnd Bergmann wrote:
> On Friday 04 July 2014 21:51:44 Russell King - ARM Linux wrote:
> > Hmm, we need guidance from ARM people on that.
> > 
> > There may well be a better way to detect between ARMv6 and ARMv6K, which
> > is given by the architecture spec.  G7.3 of an early DDI0406 says that
> > the MPIDR (mp affinity register) aliases to MIDR for ARMv6, but is of
> > course implemented for ARMv6K.
> > 
> > This seems to be carried through to the latest ARM ARM.  So it seems
> > this would be a more correct way to tell ARMv6 from ARMv6K.
> > 
> > If so, we can certainly expand cpu_architecture() to detect between the
> > two and add a CPU_ARCH_ARMv6K in there.
> > 
> > Let's see what Will has to say about that when he's next around...
> > though I think it'll require another trawl through lots of
> > documentation.
> 
> I was thinking of a simpler check in __get_cpu_architecture, just
> checking if the CPUID is ARM1136r0 since that is the only ARMv6
> CPU core we support. Anything else that we currently report as
> ARMv6 is actually ARMv6K.

I'm not prepared to use that as the basis for detecting between V6 and
V6K at the moment - we don't know that for sure.  There seems to be
some Samsung platforms which are V6 (at least they don't select V6K).

Either way, this isn't something we should rush into, because it has
the capability to break all sorts of stuff.  For example, merely
deciding to add a CPU_ARCH_ARMv6K results in a change to /proc/cpuinfo
(changing the "CPU architecture:" line) which is a user visible change.

We also have two ways to detect CPU_ARCH_ARMv6 - that needs careful
thought as well.  We also have a number of places which test for
equality with ARMv6, rather than less than or greater-or-equal.

Then there's all the ifdefs on CPU_V6K...

This is not something I want to rush into, and I certainly don't want
to make assumptions such as "ARMv6 === ARM1136 r0" - and I certainly
don't want to get stuck in a corner by making some changes and then
finding out that they're wrong later down the line when we can't then
fix it without more user visible churn.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-04 21:48         ` Russell King - ARM Linux
@ 2014-07-05 18:46           ` Arnd Bergmann
  0 siblings, 0 replies; 27+ messages in thread
From: Arnd Bergmann @ 2014-07-05 18:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Friday 04 July 2014 22:48:47 Russell King - ARM Linux wrote:
> On Fri, Jul 04, 2014 at 10:58:04PM +0200, Arnd Bergmann wrote:
> > On Friday 04 July 2014 21:51:44 Russell King - ARM Linux wrote:
> > > Hmm, we need guidance from ARM people on that.
> > > 
> > > There may well be a better way to detect between ARMv6 and ARMv6K, which
> > > is given by the architecture spec.  G7.3 of an early DDI0406 says that
> > > the MPIDR (mp affinity register) aliases to MIDR for ARMv6, but is of
> > > course implemented for ARMv6K.
> > > 
> > > This seems to be carried through to the latest ARM ARM.  So it seems
> > > this would be a more correct way to tell ARMv6 from ARMv6K.
> > > 
> > > If so, we can certainly expand cpu_architecture() to detect between the
> > > two and add a CPU_ARCH_ARMv6K in there.
> > > 
> > > Let's see what Will has to say about that when he's next around...
> > > though I think it'll require another trawl through lots of
> > > documentation.
> > 
> > I was thinking of a simpler check in __get_cpu_architecture, just
> > checking if the CPUID is ARM1136r0 since that is the only ARMv6
> > CPU core we support. Anything else that we currently report as
> > ARMv6 is actually ARMv6K.
> 
> I'm not prepared to use that as the basis for detecting between V6 and
> V6K at the moment - we don't know that for sure.  There seems to be
> some Samsung platforms which are V6 (at least they don't select V6K).

I believe we looked at that in the past and concluded it was a bug
and should be changed to V6K to reflect the actual hardware.

> Either way, this isn't something we should rush into, because it has
> the capability to break all sorts of stuff.  For example, merely
> deciding to add a CPU_ARCH_ARMv6K results in a change to /proc/cpuinfo
> (changing the "CPU architecture:" line) which is a user visible change.

Agreed, that should certainly not be done as part  of the HWCAP_SWP
change, regardless of whether we end up doing it later or in what form
we do it.

> We also have two ways to detect CPU_ARCH_ARMv6 - that needs careful
> thought as well.  We also have a number of places which test for
> equality with ARMv6, rather than less than or greater-or-equal.
> 
> Then there's all the ifdefs on CPU_V6K...

Right, good point.

> This is not something I want to rush into, and I certainly don't want
> to make assumptions such as "ARMv6 === ARM1136 r0" - and I certainly
> don't want to get stuck in a corner by making some changes and then
> finding out that they're wrong later down the line when we can't then
> fix it without more user visible churn.

Ok.

	Arnd

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-04 20:51     ` Russell King - ARM Linux
  2014-07-04 20:58       ` Arnd Bergmann
@ 2014-07-07  9:34       ` Will Deacon
  2014-07-07  9:41         ` Russell King - ARM Linux
  1 sibling, 1 reply; 27+ messages in thread
From: Will Deacon @ 2014-07-07  9:34 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Russell,

On Fri, Jul 04, 2014 at 09:51:44PM +0100, Russell King - ARM Linux wrote:
> On Fri, Jul 04, 2014 at 10:11:06PM +0200, Arnd Bergmann wrote:
> > On Friday 04 July 2014 20:52:17 Russell King wrote:
> > >         /*
> > >          * HWCAP_TLS is available only on 1136 r1p0 and later,
> > >          * see also kuser_get_tls_init.
> > >          */
> > > -       if ((((id >> 4) & 0xfff) == 0xb36) && (((id >> 20) & 3) == 0))
> > > +       if (read_cpuid_part() == ARM_CPU_PART_ARM1136 &&
> > > +           ((id >> 20) & 3) == 0) {
> > >                 elf_hwcap &= ~HWCAP_TLS;
> > > +               return;
> > > +       }
> > > 
> > 
> > Would it make sense to tie this check to the ARMv6K architecture level?
> > 
> > I see that cpu_architecture currently reports the same value for plain
> > ARMv6 (1136r0) and for ARMv6K (1136r1+ or any other ARM11), but we already
> > distinguish them at compile time in a number of places and it seems you
> > make the exact same distinction here at runtime.
> 
> Hmm, we need guidance from ARM people on that.
> 
> There may well be a better way to detect between ARMv6 and ARMv6K, which
> is given by the architecture spec.  G7.3 of an early DDI0406 says that
> the MPIDR (mp affinity register) aliases to MIDR for ARMv6, but is of
> course implemented for ARMv6K.

I don't think MPIDR exists on any 1136 (i.e. probably UNDEFs).

> This seems to be carried through to the latest ARM ARM.  So it seems
> this would be a more correct way to tell ARMv6 from ARMv6K.
> 
> If so, we can certainly expand cpu_architecture() to detect between the
> two and add a CPU_ARCH_ARMv6K in there.
> 
> Let's see what Will has to say about that when he's next around...
> though I think it'll require another trawl through lots of
> documentation.

Well, looking at the ARM11 CPUs, I think our get_cpu_architecture()
implementation will return CPU_ARCH_ARM_V7 for 1176 and 11MPCore whilst
1136 and 1156 will return CPU_ARCH_ARM_V6. So this really boils down to
whether we should go out of our way to treat 1136 r1pX as v6k instead of
v6, I'd be inclined not to bother...

Will

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-07  9:34       ` Will Deacon
@ 2014-07-07  9:41         ` Russell King - ARM Linux
  2014-07-07  9:51           ` Will Deacon
  0 siblings, 1 reply; 27+ messages in thread
From: Russell King - ARM Linux @ 2014-07-07  9:41 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 07, 2014 at 10:34:43AM +0100, Will Deacon wrote:
> Hi Russell,
> 
> On Fri, Jul 04, 2014 at 09:51:44PM +0100, Russell King - ARM Linux wrote:
> > On Fri, Jul 04, 2014 at 10:11:06PM +0200, Arnd Bergmann wrote:
> > > On Friday 04 July 2014 20:52:17 Russell King wrote:
> > > >         /*
> > > >          * HWCAP_TLS is available only on 1136 r1p0 and later,
> > > >          * see also kuser_get_tls_init.
> > > >          */
> > > > -       if ((((id >> 4) & 0xfff) == 0xb36) && (((id >> 20) & 3) == 0))
> > > > +       if (read_cpuid_part() == ARM_CPU_PART_ARM1136 &&
> > > > +           ((id >> 20) & 3) == 0) {
> > > >                 elf_hwcap &= ~HWCAP_TLS;
> > > > +               return;
> > > > +       }
> > > > 
> > > 
> > > Would it make sense to tie this check to the ARMv6K architecture level?
> > > 
> > > I see that cpu_architecture currently reports the same value for plain
> > > ARMv6 (1136r0) and for ARMv6K (1136r1+ or any other ARM11), but we already
> > > distinguish them at compile time in a number of places and it seems you
> > > make the exact same distinction here at runtime.
> > 
> > Hmm, we need guidance from ARM people on that.
> > 
> > There may well be a better way to detect between ARMv6 and ARMv6K, which
> > is given by the architecture spec.  G7.3 of an early DDI0406 says that
> > the MPIDR (mp affinity register) aliases to MIDR for ARMv6, but is of
> > course implemented for ARMv6K.
> 
> I don't think MPIDR exists on any 1136 (i.e. probably UNDEFs).

There has long been a requirement (back to ARMv4 I believe) that
instructions such as:

	mrc	p15, 0, rd, c0, c0, X

all return the MIDR unless they are implemented.  This is certainly
carried forward to the latest ARM ARMs.  This is done to explicitly
allow the presence of additional registers to be detected.

I would be very surprised if the 1136 did undef on this.

If I dig out my 1136 FPGA (which according to the docs would be r0p2)
I can explicitly verify this... provided Integrator CP still works.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-07  9:41         ` Russell King - ARM Linux
@ 2014-07-07  9:51           ` Will Deacon
  0 siblings, 0 replies; 27+ messages in thread
From: Will Deacon @ 2014-07-07  9:51 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 07, 2014 at 10:41:28AM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 07, 2014 at 10:34:43AM +0100, Will Deacon wrote:
> > I don't think MPIDR exists on any 1136 (i.e. probably UNDEFs).
> 
> There has long been a requirement (back to ARMv4 I believe) that
> instructions such as:
> 
> 	mrc	p15, 0, rd, c0, c0, X
> 
> all return the MIDR unless they are implemented.  This is certainly
> carried forward to the latest ARM ARMs.  This is done to explicitly
> allow the presence of additional registers to be detected.

You're right (and I didn't know that); the ARM ARM does indeed suggest that
opc2 == {4,5,6,7} in the instruction above will alias MIDR.

> I would be very surprised if the 1136 did undef on this.
> 
> If I dig out my 1136 FPGA (which according to the docs would be r0p2)
> I can explicitly verify this... provided Integrator CP still works.

Actually, I managed to find the following in the 1136 TRM, so your plan
could actually work:

  Note

  If the processor encounters an Opcode_2 value corresponding to an
  unimplemented or reserved ID register with CRm = c0 and Opcode_1 = 0, the
  system control coprocessor returns the value of the Main ID Register.

You live and learn!

Will

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-04 20:58       ` Arnd Bergmann
  2014-07-04 21:48         ` Russell King - ARM Linux
@ 2014-07-07 11:02         ` Catalin Marinas
  2014-07-07 11:17           ` Russell King - ARM Linux
  1 sibling, 1 reply; 27+ messages in thread
From: Catalin Marinas @ 2014-07-07 11:02 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jul 04, 2014 at 10:58:04PM +0200, Arnd Bergmann wrote:
> On Friday 04 July 2014 21:51:44 Russell King - ARM Linux wrote:
> > Hmm, we need guidance from ARM people on that.
> > 
> > There may well be a better way to detect between ARMv6 and ARMv6K, which
> > is given by the architecture spec.  G7.3 of an early DDI0406 says that
> > the MPIDR (mp affinity register) aliases to MIDR for ARMv6, but is of
> > course implemented for ARMv6K.
> > 
> > This seems to be carried through to the latest ARM ARM.  So it seems
> > this would be a more correct way to tell ARMv6 from ARMv6K.
> > 
> > If so, we can certainly expand cpu_architecture() to detect between the
> > two and add a CPU_ARCH_ARMv6K in there.
> > 
> > Let's see what Will has to say about that when he's next around...
> > though I think it'll require another trawl through lots of
> > documentation.
> 
> I was thinking of a simpler check in __get_cpu_architecture, just
> checking if the CPUID is ARM1136r0 since that is the only ARMv6
> CPU core we support. Anything else that we currently report as
> ARMv6 is actually ARMv6K.

I'm not sure that's the right approach. ARM added the CPUID scheme to
allow the software to check specific features rather than guessing
specific architecture versions. Basically there isn't a clear way to
identify whether your CPU is ARMv6K or ARMv7 from a single ID register
read (you may be able to infer by reading multiple ID regs).

I'm now trying to figure out whether the TPIDR registers actually have a
dedicated ID. I think they only come as part of the VMSA version 7 as
identified from ID_MMFR0. The ARM1136 r1p1 TRM states that ID_MMFR0[3:0]
are 0x3 which mean VMSAv7.

-- 
Catalin

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-07 11:02         ` Catalin Marinas
@ 2014-07-07 11:17           ` Russell King - ARM Linux
  2014-07-07 12:05             ` Catalin Marinas
  0 siblings, 1 reply; 27+ messages in thread
From: Russell King - ARM Linux @ 2014-07-07 11:17 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 07, 2014 at 12:02:48PM +0100, Catalin Marinas wrote:
> I'm not sure that's the right approach. ARM added the CPUID scheme to
> allow the software to check specific features rather than guessing
> specific architecture versions. Basically there isn't a clear way to
> identify whether your CPU is ARMv6K or ARMv7 from a single ID register
> read (you may be able to infer by reading multiple ID regs).
> 
> I'm now trying to figure out whether the TPIDR registers actually have a
> dedicated ID. I think they only come as part of the VMSA version 7 as
> identified from ID_MMFR0. The ARM1136 r1p1 TRM states that ID_MMFR0[3:0]
> are 0x3 which mean VMSAv7.

You can't rely on the CPUID registers on 1136, because it doesn't
advertise them as being present - the architecture field of MIDR is
0x7, not 0xf.

What we know is that if it has the MPIDR, then it must be ARMv6K (the
ARM tells us that.)  Where there is a hole is where a CPU is ARMv6K
but does not have the MPIDR, in which case we fall back to treating it
as a plain ARMv6. I don't think that can be worked around for the reason
you state above - there isn't a way to reliably identify the two apart.

It looks like 1136r1 falls into that category.  It didn't have to had
the MIDR had been updated to indicate the presence of the CPUID scheme.
It should be noted that the 1136r1 CPUID registers don't quite have the
same definition as those specified by the ARM ARM (irrespective of the
version of the ARM ARM.)

So actually, 1136r0 is the architecturally correct version, and 1136r1
is the slightly cocked up non-standard version where we have to be careful
how we treat it.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

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

* [PATCH 0/4] ABI updates
  2014-07-04 19:51 [PATCH 0/4] ABI updates Russell King - ARM Linux
                   ` (4 preceding siblings ...)
  2014-07-04 20:12 ` [PATCH 0/4] ABI updates Arnd Bergmann
@ 2014-07-07 11:19 ` Tony Lindgren
  2014-07-07 11:23   ` Russell King - ARM Linux
  2014-07-07 13:52 ` Catalin Marinas
  6 siblings, 1 reply; 27+ messages in thread
From: Tony Lindgren @ 2014-07-07 11:19 UTC (permalink / raw)
  To: linux-arm-kernel

* Russell King - ARM Linux <linux@arm.linux.org.uk> [140704 12:56]:
> As a result of the recent discussions about android, several issues
> have been identified with existing kernels:
> 
> 1. HWCAP_SWP does not properly reflect whether the SWP instruction
>    should be used.  Turn HWCAP_SWP off on ARMv6+ where the exclusives
>    are available.
> 
> 2. SWP is unsafe when running on a SMP CPU; there is no bus locking
>    between the read and write parts of the instruction execution.
>    Force SWP emulation on ARMv7+ where we can disable the SWP
>    instruction.
> 
> 3. Reporting of alignment faults - kernel mode faults are silent due to
>    the requirements of the network stack, but we can note where the
>    last one occurs.
> 
> Testing reveals that at least my OMAP4430 userspace (supplied from TI)
> executes lots of SWP instructions, which is something that wasn't known
> before this patch.
> 
> Many systems today are not configured with SWP emulation enabled on SMP.
> This means that the SWP instruction is available, but unsafe.
> 
>  arch/arm/kernel/setup.c       | 29 ++++++++++++++++++++++-------
>  arch/arm/kernel/swp_emulate.c |  4 ++++
>  arch/arm/mm/Kconfig           |  2 +-
>  arch/arm/mm/alignment.c       |  4 +++-
>  4 files changed, 30 insertions(+), 9 deletions(-)

Tried testing this on omap2 which is ARMv6 with no K, but got this:

arch/arm/kernel/setup.c: In function ?elf_hwcap_fixup?:
arch/arm/kernel/setup.c:405:2: error: implicit declaration of function ?read_cpuid_part? [-Werror=implicit-function-declaration]

That should be read_cpuid_part_number instead. After fixing that,
omap2 booted fine, so for ARMv6 with no K:

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

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

* [PATCH 0/4] ABI updates
  2014-07-07 11:19 ` Tony Lindgren
@ 2014-07-07 11:23   ` Russell King - ARM Linux
  2014-07-07 13:23     ` Tony Lindgren
  0 siblings, 1 reply; 27+ messages in thread
From: Russell King - ARM Linux @ 2014-07-07 11:23 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 07, 2014 at 04:19:48AM -0700, Tony Lindgren wrote:
> * Russell King - ARM Linux <linux@arm.linux.org.uk> [140704 12:56]:
> > As a result of the recent discussions about android, several issues
> > have been identified with existing kernels:
> > 
> > 1. HWCAP_SWP does not properly reflect whether the SWP instruction
> >    should be used.  Turn HWCAP_SWP off on ARMv6+ where the exclusives
> >    are available.
> > 
> > 2. SWP is unsafe when running on a SMP CPU; there is no bus locking
> >    between the read and write parts of the instruction execution.
> >    Force SWP emulation on ARMv7+ where we can disable the SWP
> >    instruction.
> > 
> > 3. Reporting of alignment faults - kernel mode faults are silent due to
> >    the requirements of the network stack, but we can note where the
> >    last one occurs.
> > 
> > Testing reveals that at least my OMAP4430 userspace (supplied from TI)
> > executes lots of SWP instructions, which is something that wasn't known
> > before this patch.
> > 
> > Many systems today are not configured with SWP emulation enabled on SMP.
> > This means that the SWP instruction is available, but unsafe.
> > 
> >  arch/arm/kernel/setup.c       | 29 ++++++++++++++++++++++-------
> >  arch/arm/kernel/swp_emulate.c |  4 ++++
> >  arch/arm/mm/Kconfig           |  2 +-
> >  arch/arm/mm/alignment.c       |  4 +++-
> >  4 files changed, 30 insertions(+), 9 deletions(-)
> 
> Tried testing this on omap2 which is ARMv6 with no K, but got this:
> 
> arch/arm/kernel/setup.c: In function ?elf_hwcap_fixup?:
> arch/arm/kernel/setup.c:405:2: error: implicit declaration of function ?read_cpuid_part? [-Werror=implicit-function-declaration]

That's because they rely upon my "ARM: make it easier to check the CPU part
number correctly" patch in linux-next.

> That should be read_cpuid_part_number instead. After fixing that,
> omap2 booted fine, so for ARMv6 with no K:

Great, I hope you checked /proc/cpuinfo came out correctly, as well as
noticing that it merely booted.

Thanks.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-07 11:17           ` Russell King - ARM Linux
@ 2014-07-07 12:05             ` Catalin Marinas
  2014-07-07 13:13               ` Russell King - ARM Linux
  0 siblings, 1 reply; 27+ messages in thread
From: Catalin Marinas @ 2014-07-07 12:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 07, 2014 at 12:17:12PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 07, 2014 at 12:02:48PM +0100, Catalin Marinas wrote:
> > I'm not sure that's the right approach. ARM added the CPUID scheme to
> > allow the software to check specific features rather than guessing
> > specific architecture versions. Basically there isn't a clear way to
> > identify whether your CPU is ARMv6K or ARMv7 from a single ID register
> > read (you may be able to infer by reading multiple ID regs).
> > 
> > I'm now trying to figure out whether the TPIDR registers actually have a
> > dedicated ID. I think they only come as part of the VMSA version 7 as
> > identified from ID_MMFR0. The ARM1136 r1p1 TRM states that ID_MMFR0[3:0]
> > are 0x3 which mean VMSAv7.
> 
> You can't rely on the CPUID registers on 1136, because it doesn't
> advertise them as being present - the architecture field of MIDR is
> 0x7, not 0xf.

I was hoping they changed it with r1 but looking at the TRM that's not
the case.

> So actually, 1136r0 is the architecturally correct version, and 1136r1
> is the slightly cocked up non-standard version where we have to be careful
> how we treat it.

Yes. Looking at ARM1176, it seems to be using the full CPUID scheme and
reporting VMSAv7. So I guess we can safely assume TLS presence if VMSAv7
(actually what __get_cpu_architecture checks) or ARM1136 r1+. This would
be slightly different from the current assumption that TLS is present on
ARMv6+ except ARM1136r0 but I'm not aware of any other ARMv6 non-CPUID
processor without TLS (ARM1156 has CPUID and reports PMSAv6).

-- 
Catalin

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-07 12:05             ` Catalin Marinas
@ 2014-07-07 13:13               ` Russell King - ARM Linux
  2014-07-07 13:46                 ` Catalin Marinas
  0 siblings, 1 reply; 27+ messages in thread
From: Russell King - ARM Linux @ 2014-07-07 13:13 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 07, 2014 at 01:05:52PM +0100, Catalin Marinas wrote:
> On Mon, Jul 07, 2014 at 12:17:12PM +0100, Russell King - ARM Linux wrote:
> > On Mon, Jul 07, 2014 at 12:02:48PM +0100, Catalin Marinas wrote:
> > > I'm not sure that's the right approach. ARM added the CPUID scheme to
> > > allow the software to check specific features rather than guessing
> > > specific architecture versions. Basically there isn't a clear way to
> > > identify whether your CPU is ARMv6K or ARMv7 from a single ID register
> > > read (you may be able to infer by reading multiple ID regs).
> > > 
> > > I'm now trying to figure out whether the TPIDR registers actually have a
> > > dedicated ID. I think they only come as part of the VMSA version 7 as
> > > identified from ID_MMFR0. The ARM1136 r1p1 TRM states that ID_MMFR0[3:0]
> > > are 0x3 which mean VMSAv7.
> > 
> > You can't rely on the CPUID registers on 1136, because it doesn't
> > advertise them as being present - the architecture field of MIDR is
> > 0x7, not 0xf.
> 
> I was hoping they changed it with r1 but looking at the TRM that's not
> the case.

You misunderstand.  r0 did not have the CPUID registers at all, so 0x7
is correct.  r1 added the CPUID registers, but left the MIDR with the
architecture field set to 0x7.

> > So actually, 1136r0 is the architecturally correct version, and 1136r1
> > is the slightly cocked up non-standard version where we have to be careful
> > how we treat it.
> 
> Yes. Looking at ARM1176, it seems to be using the full CPUID scheme and
> reporting VMSAv7. So I guess we can safely assume TLS presence if VMSAv7
> (actually what __get_cpu_architecture checks) or ARM1136 r1+.

*Not* ARM1136 r1, because there the CPUID registers are ignored because
MIDR does not indicate their presence.  So all ARM1136 are currently
identified as ARMv6 by the kernel.

With my proposal, ARM1136 with MPIDR would be identified as ARMv6K,
but ARM1136 r1 without MPIDR would remain identified as ARMv6.

Naturally, any CPU which reports in MIDR that it supports CPUID would
be correctly identified from the CPUID.

The problem with ARM1136 r1 is that it has CPUID registers but MIDR
indicates that it does not have these registers.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

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

* [PATCH 0/4] ABI updates
  2014-07-07 11:23   ` Russell King - ARM Linux
@ 2014-07-07 13:23     ` Tony Lindgren
  0 siblings, 0 replies; 27+ messages in thread
From: Tony Lindgren @ 2014-07-07 13:23 UTC (permalink / raw)
  To: linux-arm-kernel

* Russell King - ARM Linux <linux@arm.linux.org.uk> [140707 04:25]:
> On Mon, Jul 07, 2014 at 04:19:48AM -0700, Tony Lindgren wrote:
> > 
> > Tried testing this on omap2 which is ARMv6 with no K, but got this:
> > 
> > arch/arm/kernel/setup.c: In function ?elf_hwcap_fixup?:
> > arch/arm/kernel/setup.c:405:2: error: implicit declaration of function ?read_cpuid_part? [-Werror=implicit-function-declaration]
> 
> That's because they rely upon my "ARM: make it easier to check the CPU part
> number correctly" patch in linux-next.

OK, works with that patch applied.
 
> > That should be read_cpuid_part_number instead. After fixing that,
> > omap2 booted fine, so for ARMv6 with no K:
> 
> Great, I hope you checked /proc/cpuinfo came out correctly, as well as
> noticing that it merely booted.

Yes it's the same with and without the patches:

# cat /proc/cpuinfo 
processor       : 0
model name      : ARMv6-compatible processor rev 2 (v6l)
Features        : swp half thumb fastmult vfp edsp java 
CPU implementer : 0x41
CPU architecture: 6TEJ
CPU variant     : 0x0
CPU part        : 0xb36
CPU revision    : 2

Hardware        : Generic OMAP2430 (Flattened Device Tree)
Revision        : 0000
Serial          : 0000000000000000

And omap2's have this in the revision registers for reference:

CPU: ARMv6-compatible processor [4107b362] revision 2 (ARMv6TEJ), cr=00c5387d

Regards,

Tony

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-07 13:13               ` Russell King - ARM Linux
@ 2014-07-07 13:46                 ` Catalin Marinas
  2014-07-07 15:31                   ` Russell King - ARM Linux
  0 siblings, 1 reply; 27+ messages in thread
From: Catalin Marinas @ 2014-07-07 13:46 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 07, 2014 at 02:13:35PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 07, 2014 at 01:05:52PM +0100, Catalin Marinas wrote:
> > On Mon, Jul 07, 2014 at 12:17:12PM +0100, Russell King - ARM Linux wrote:
> > > On Mon, Jul 07, 2014 at 12:02:48PM +0100, Catalin Marinas wrote:
> > > > I'm not sure that's the right approach. ARM added the CPUID scheme to
> > > > allow the software to check specific features rather than guessing
> > > > specific architecture versions. Basically there isn't a clear way to
> > > > identify whether your CPU is ARMv6K or ARMv7 from a single ID register
> > > > read (you may be able to infer by reading multiple ID regs).
> > > > 
> > > > I'm now trying to figure out whether the TPIDR registers actually have a
> > > > dedicated ID. I think they only come as part of the VMSA version 7 as
> > > > identified from ID_MMFR0. The ARM1136 r1p1 TRM states that ID_MMFR0[3:0]
> > > > are 0x3 which mean VMSAv7.
> > > 
> > > You can't rely on the CPUID registers on 1136, because it doesn't
> > > advertise them as being present - the architecture field of MIDR is
> > > 0x7, not 0xf.
> > 
> > I was hoping they changed it with r1 but looking at the TRM that's not
> > the case.
> 
> You misunderstand.  r0 did not have the CPUID registers at all, so 0x7
> is correct.  r1 added the CPUID registers, but left the MIDR with the
> architecture field set to 0x7.

Yes, that's what I understood (and as I said above, I _was_ hoping ARM
brought the full CPUID to 1136 r1, including the MIDR arch field, but
that's not the case).

> > > So actually, 1136r0 is the architecturally correct version, and 1136r1
> > > is the slightly cocked up non-standard version where we have to be careful
> > > how we treat it.
> > 
> > Yes. Looking at ARM1176, it seems to be using the full CPUID scheme and
> > reporting VMSAv7. So I guess we can safely assume TLS presence if VMSAv7
> > (actually what __get_cpu_architecture checks) or ARM1136 r1+.
> 
> *Not* ARM1136 r1, because there the CPUID registers are ignored because
> MIDR does not indicate their presence.  So all ARM1136 are currently
> identified as ARMv6 by the kernel.

I agree.

> With my proposal, ARM1136 with MPIDR would be identified as ARMv6K,
> but ARM1136 r1 without MPIDR would remain identified as ARMv6.

The ARM1136 TRM states that r1 introduces ARMv6K features. However, I
can't find any trace of MPIDR and it may actually just return MIDR. If
that's the case, we don't have a way to classify ARMv6K here based on
MPIDR.

My original point was to ignore the v6K classification and, for the TLS
presence, just check the feature registers if full CPUID is present,
otherwise let TLS enabled for ARM1136 r1 because we know it implements
it (according to the TRM, special case without full CPUID).

-- 
Catalin

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

* [PATCH 0/4] ABI updates
  2014-07-04 19:51 [PATCH 0/4] ABI updates Russell King - ARM Linux
                   ` (5 preceding siblings ...)
  2014-07-07 11:19 ` Tony Lindgren
@ 2014-07-07 13:52 ` Catalin Marinas
  6 siblings, 0 replies; 27+ messages in thread
From: Catalin Marinas @ 2014-07-07 13:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jul 04, 2014 at 08:51:34PM +0100, Russell King - ARM Linux wrote:
> As a result of the recent discussions about android, several issues
> have been identified with existing kernels:
> 
> 1. HWCAP_SWP does not properly reflect whether the SWP instruction
>    should be used.  Turn HWCAP_SWP off on ARMv6+ where the exclusives
>    are available.
> 
> 2. SWP is unsafe when running on a SMP CPU; there is no bus locking
>    between the read and write parts of the instruction execution.
>    Force SWP emulation on ARMv7+ where we can disable the SWP
>    instruction.
> 
> 3. Reporting of alignment faults - kernel mode faults are silent due to
>    the requirements of the network stack, but we can note where the
>    last one occurs.
> 
> Testing reveals that at least my OMAP4430 userspace (supplied from TI)
> executes lots of SWP instructions, which is something that wasn't known
> before this patch.
> 
> Many systems today are not configured with SWP emulation enabled on SMP.
> This means that the SWP instruction is available, but unsafe.

The patches look fine to me (we are still discussing patch 4 but even if
it needs changing for TLS, it can be done separately from this series).

Question - should we go a step further and change the pr_debug() in
swp_emulate.c to something like pr_warn()?

Another deprecated feature is CP15 barriers (disable bit coming with the
virtualisation extensions). I think we should start emulating these soon
as well and print warnings.

-- 
Catalin

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-07 13:46                 ` Catalin Marinas
@ 2014-07-07 15:31                   ` Russell King - ARM Linux
  2014-07-07 15:59                     ` Catalin Marinas
  0 siblings, 1 reply; 27+ messages in thread
From: Russell King - ARM Linux @ 2014-07-07 15:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 07, 2014 at 02:46:24PM +0100, Catalin Marinas wrote:
> On Mon, Jul 07, 2014 at 02:13:35PM +0100, Russell King - ARM Linux wrote:
> > > > So actually, 1136r0 is the architecturally correct version, and 1136r1
> > > > is the slightly cocked up non-standard version where we have to be careful
> > > > how we treat it.
> > > 
> > > Yes. Looking at ARM1176, it seems to be using the full CPUID scheme and
> > > reporting VMSAv7. So I guess we can safely assume TLS presence if VMSAv7
> > > (actually what __get_cpu_architecture checks) or ARM1136 r1+.
> > 
> > *Not* ARM1136 r1, because there the CPUID registers are ignored because
> > MIDR does not indicate their presence.  So all ARM1136 are currently
> > identified as ARMv6 by the kernel.
> 
> I agree.
> 
> > With my proposal, ARM1136 with MPIDR would be identified as ARMv6K,
> > but ARM1136 r1 without MPIDR would remain identified as ARMv6.
> 
> The ARM1136 TRM states that r1 introduces ARMv6K features. However, I
> can't find any trace of MPIDR and it may actually just return MIDR. If
> that's the case, we don't have a way to classify ARMv6K here based on
> MPIDR.

It is documented in all sorts of places, including the 1136 TRM, that
when the MPIDR is not implemented, it returns the MIDR.

> My original point was to ignore the v6K classification and, for the TLS
> presence, just check the feature registers if full CPUID is present,
> otherwise let TLS enabled for ARM1136 r1 because we know it implements
> it (according to the TRM, special case without full CPUID).

Can we please stop this pointless wandering off of the topic.  We're
not talking about TLS stuff here - that remains the same as it ever
did.

We're talking about SWP and/or the exclusives.

Right, since you like talking about stuff which isn't covered by this
patch, I'm going to assume that there are no objections to these
changes in this patch set.  In fact, I'm going to take your divergence
from the original topic as an implicit ack against the entire patch
set... if not, please keep discussions on topic rather than letting
them ramble over all sorts of other peripheral issues.

The TLS stuff is an _entirely_ separate issue from the one which this
patch addresses, and this patch does not really alter the mechanism
by which we decide whether HWCAP_TLS is set or cleared.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-07 15:31                   ` Russell King - ARM Linux
@ 2014-07-07 15:59                     ` Catalin Marinas
  2014-07-07 16:31                       ` Russell King - ARM Linux
  0 siblings, 1 reply; 27+ messages in thread
From: Catalin Marinas @ 2014-07-07 15:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 07, 2014 at 04:31:05PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 07, 2014 at 02:46:24PM +0100, Catalin Marinas wrote:
> > On Mon, Jul 07, 2014 at 02:13:35PM +0100, Russell King - ARM Linux wrote:
> > > > > So actually, 1136r0 is the architecturally correct version, and 1136r1
> > > > > is the slightly cocked up non-standard version where we have to be careful
> > > > > how we treat it.
> > > > 
> > > > Yes. Looking at ARM1176, it seems to be using the full CPUID scheme and
> > > > reporting VMSAv7. So I guess we can safely assume TLS presence if VMSAv7
> > > > (actually what __get_cpu_architecture checks) or ARM1136 r1+.
> > > 
> > > *Not* ARM1136 r1, because there the CPUID registers are ignored because
> > > MIDR does not indicate their presence.  So all ARM1136 are currently
> > > identified as ARMv6 by the kernel.
> > 
> > I agree.
> > 
> > > With my proposal, ARM1136 with MPIDR would be identified as ARMv6K,
> > > but ARM1136 r1 without MPIDR would remain identified as ARMv6.
> > 
> > The ARM1136 TRM states that r1 introduces ARMv6K features. However, I
> > can't find any trace of MPIDR and it may actually just return MIDR. If
> > that's the case, we don't have a way to classify ARMv6K here based on
> > MPIDR.
> 
> It is documented in all sorts of places, including the 1136 TRM, that
> when the MPIDR is not implemented, it returns the MIDR.

Yes. So how do you check that an ARM1136 is v6K or not? I don't think
you can do this based on MPIDR because all ARM1136 revisions would
return MIDR when MPIDR is read (at least to my reading of the TRMs).

> > My original point was to ignore the v6K classification and, for the TLS
> > presence, just check the feature registers if full CPUID is present,
> > otherwise let TLS enabled for ARM1136 r1 because we know it implements
> > it (according to the TRM, special case without full CPUID).
> 
> Can we please stop this pointless wandering off of the topic.  We're
> not talking about TLS stuff here - that remains the same as it ever
> did.

Sorry, it went off topic. TLS is one case, SWP is another covered by the
same elf_hwcap_fixup() function (but it can be addressed separately).

> We're talking about SWP and/or the exclusives.

Going on topic again, LDREXB is present in ARM1136 r1 but your changes
would not detect it (and it's a different ID_ISAR3 format anyway). If we
care about this case, you could either pretend that ARM1136 r1 is ARMv6K
and link those decisions to v6K+ tests or check for individual features
which for ARM1136 would require a different check from standard CPUID.

-- 
Catalin

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-07 15:59                     ` Catalin Marinas
@ 2014-07-07 16:31                       ` Russell King - ARM Linux
  2014-07-07 17:50                         ` Catalin Marinas
  0 siblings, 1 reply; 27+ messages in thread
From: Russell King - ARM Linux @ 2014-07-07 16:31 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 07, 2014 at 04:59:42PM +0100, Catalin Marinas wrote:
> Going on topic again, LDREXB is present in ARM1136 r1 but your changes
> would not detect it (and it's a different ID_ISAR3 format anyway). If we
> care about this case, you could either pretend that ARM1136 r1 is ARMv6K
> and link those decisions to v6K+ tests or check for individual features
> which for ARM1136 would require a different check from standard CPUID.

Ever since ARMv6 was first added, HWCAP_SWP has been indicated for all
ARMv6 CPUs and their derivatives.  Indeed, the SWP instruction is
supported in hardware.

Hence, indicating HWCAP_SWP on ARMv6 whether or not there is LDREXB
support is /not/ incorrect in any way.

ARM1136 r1 added LDREXB as part of the ARMv6K adoption, but it kept
the SWP instruction (which can't be disabled.)  So, indicating
HWCAP_SWP also is /not/ incorrect.

However, when it comes to SMP, we would _prefer_ userspace to use
the exclusives instructions, though it seems that the SWP behaviour
on an ARMv6K SMP system with respect to other CPUs is not documented.
All things being equal, we would prefer SWP not be used there because
we don't know whether it's safe.  Unfortunately, we can't turn the
instruction off, so we can't emulate it.

However, we know that some userspace running there may well contain
SWP instructions.  So, as there aren't any bug reports, it's probably
safe to assume that SWP is safe on ARMv6K SMP systems.


So, the upshot of this is:
* SWP on all ARMv6 is fine.
* Advertising HWCAP_SWP on all ARMv6 is fine too.
* We would prefer not to advertise HWCAP_SWP on ARMv6 with support
  for the exclusives, and we can do that trivially where we can
  check the CPUID values.
* ARM1136 is either ARMv6 or ARMv6K depending on its revision -
  and thus may have LDREXB, but LDREXB is not trivially detectable.


I believe what I'm doing in this patch is architecturally correct, I
also believe it to be safe for the troublesome ARM1136 by way of
defaulting to advertising HWCAP_SWP.

The killer argument for this is that we've been advertising HWCAP_SWP
for over the last decade on these anyway, and that situation is not
changing.

What is changing is whether we advertise HWCAP_SWP on CPUs with the
architected CPUID registers (maybe some ARMv6 if we're lucky, and
all ARMv7) which also indicate that they have LDREXB support.

Hence, this is the conservative change: we could include ARM1136 r1
in this, but I don't see the need to complicate the code further
right now.

Also, adding a CPU_ARCH_ARMv6K doesn't buy us very much, and carries
many risks that we will end up breaking something.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

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

* [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives
  2014-07-07 16:31                       ` Russell King - ARM Linux
@ 2014-07-07 17:50                         ` Catalin Marinas
  0 siblings, 0 replies; 27+ messages in thread
From: Catalin Marinas @ 2014-07-07 17:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Mon, Jul 07, 2014 at 05:31:43PM +0100, Russell King - ARM Linux wrote:
> On Mon, Jul 07, 2014 at 04:59:42PM +0100, Catalin Marinas wrote:
> > Going on topic again, LDREXB is present in ARM1136 r1 but your changes
> > would not detect it (and it's a different ID_ISAR3 format anyway). If we
> > care about this case, you could either pretend that ARM1136 r1 is ARMv6K
> > and link those decisions to v6K+ tests or check for individual features
> > which for ARM1136 would require a different check from standard CPUID.
> 
> Ever since ARMv6 was first added, HWCAP_SWP has been indicated for all
> ARMv6 CPUs and their derivatives.  Indeed, the SWP instruction is
> supported in hardware.
> 
> Hence, indicating HWCAP_SWP on ARMv6 whether or not there is LDREXB
> support is /not/ incorrect in any way.
> 
> ARM1136 r1 added LDREXB as part of the ARMv6K adoption, but it kept
> the SWP instruction (which can't be disabled.)  So, indicating
> HWCAP_SWP also is /not/ incorrect.

Fine be me.

> However, when it comes to SMP, we would _prefer_ userspace to use
> the exclusives instructions, though it seems that the SWP behaviour
> on an ARMv6K SMP system with respect to other CPUs is not documented.
> All things being equal, we would prefer SWP not be used there because
> we don't know whether it's safe.  Unfortunately, we can't turn the
> instruction off, so we can't emulate it.

IIRC, ARM11MPCore is fine. Certain ARMv7 MP implementations have issues
with SWP, though not all AFAIK.

> However, we know that some userspace running there may well contain
> SWP instructions.  So, as there aren't any bug reports, it's probably
> safe to assume that SWP is safe on ARMv6K SMP systems.
> 
> 
> So, the upshot of this is:
> * SWP on all ARMv6 is fine.
> * Advertising HWCAP_SWP on all ARMv6 is fine too.
> * We would prefer not to advertise HWCAP_SWP on ARMv6 with support
>   for the exclusives, and we can do that trivially where we can
>   check the CPUID values.
> * ARM1136 is either ARMv6 or ARMv6K depending on its revision -
>   and thus may have LDREXB, but LDREXB is not trivially detectable.
> 
> 
> I believe what I'm doing in this patch is architecturally correct, I
> also believe it to be safe for the troublesome ARM1136 by way of
> defaulting to advertising HWCAP_SWP.

I agree. With the above assumptions, feel free to add my ack for the
whole series.

Acked-by: Catalin Marinas <catalin.marinas@arm.com>

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

end of thread, other threads:[~2014-07-07 17:50 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-04 19:51 [PATCH 0/4] ABI updates Russell King - ARM Linux
2014-07-04 19:52 ` [PATCH 1/4] ARM: alignment: save last kernel aligned fault location Russell King
2014-07-04 19:52 ` [PATCH 2/4] ARM: SWP emulation: always enable when SMP is enabled Russell King
2014-07-04 19:52 ` [PATCH 3/4] ARM: SWP emulation: only initialise on ARMv7 CPUs Russell King
2014-07-04 19:52 ` [PATCH 4/4] ARM: hwcap: disable HWCAP_SWP if the CPU advertises it has exclusives Russell King
2014-07-04 20:11   ` Arnd Bergmann
2014-07-04 20:51     ` Russell King - ARM Linux
2014-07-04 20:58       ` Arnd Bergmann
2014-07-04 21:48         ` Russell King - ARM Linux
2014-07-05 18:46           ` Arnd Bergmann
2014-07-07 11:02         ` Catalin Marinas
2014-07-07 11:17           ` Russell King - ARM Linux
2014-07-07 12:05             ` Catalin Marinas
2014-07-07 13:13               ` Russell King - ARM Linux
2014-07-07 13:46                 ` Catalin Marinas
2014-07-07 15:31                   ` Russell King - ARM Linux
2014-07-07 15:59                     ` Catalin Marinas
2014-07-07 16:31                       ` Russell King - ARM Linux
2014-07-07 17:50                         ` Catalin Marinas
2014-07-07  9:34       ` Will Deacon
2014-07-07  9:41         ` Russell King - ARM Linux
2014-07-07  9:51           ` Will Deacon
2014-07-04 20:12 ` [PATCH 0/4] ABI updates Arnd Bergmann
2014-07-07 11:19 ` Tony Lindgren
2014-07-07 11:23   ` Russell King - ARM Linux
2014-07-07 13:23     ` Tony Lindgren
2014-07-07 13:52 ` Catalin Marinas

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.