ARM: Do not run dummy_flush_tlb_a15_erratum() on non-Cortex-A15

PATCH New Fabio Estevam 4 participants 6 messages Jul 23, 2013 Arm (32-bit)

lore

Share & tools

lore

Fabio Estevam

author

2013-07-23 05:19 UTC #0 lore raw

PATCH 1/1 ARM: Do not run dummy_flush_tlb_a15_erratum() on non-Cortex-A15

From: Fabio Estevam <[email protected]>

Commit 93dc688 (ARM: 7684/1: errata: Workaround for Cortex-A15 erratum 798181 (TLBI/DSB operations)) causes the following undefined instruction error on a mx53 (Cortex-A8):

Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
CPU: 0 PID: 275 Comm: modprobe Not tainted 3.11.0-rc2-next-20130722-00009-g9b0f371 #881
task: df46cc00 ti: df48e000 task.ti: df48e000
PC is at check_and_switch_context+0x17c/0x4d0
LR is at check_and_switch_context+0xdc/0x4d0

This problem happens because check_and_switch_context() calls dummy_flush_tlb_a15_erratum() without checking if we are really running on a Cortex-A15 or not.

To avoid this issue, always check if we are running on a Cortex-A15 or not (via erratum_a15_798181()) inside dummy_flush_tlb_a15_erratum(), so that we do not need to keep doing the same check in all occurrences of dummy_flush_tlb_a15_erratum().

Signed-off-by Fabio Estevam <[email protected]>

---
 arch/arm/include/asm/tlbflush.h | 18 ++++++++++++++++++
 arch/arm/kernel/smp_tlb.c       | 23 -----------------------
 2 files changed, 18 insertions(+), 23 deletions(-)

arch/arm/include/asm/tlbflush.h +18 -0

index fdbb9e3..2478b6f 100644
@@ -443,9 +443,22 @@ static inline void local_flush_bp_all(void)
 		isb();
 }
 
+#include <asm/cputype.h>
 #ifdef CONFIG_ARM_ERRATA_798181
+static inline int erratum_a15_798181(void)
+{
+	unsigned int midr = read_cpuid_id();
+
+	/* Cortex-A15 r0p0..r3p2 affected */
+	if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2)
+		return 0;
+	return 1;
+}
+
 static inline void dummy_flush_tlb_a15_erratum(void)
 {
+	if (!erratum_a15_798181())
+		return;
 	/*
 	 * Dummy TLBIMVAIS. Using the unmapped address 0 and ASID 0.
 	 */

@@ -453,6 +466,11 @@ static inline void dummy_flush_tlb_a15_erratum(void)
 	dsb();
 }
 #else
+static inline int erratum_a15_798181(void)
+{
+	return 0;
+}
+
 static inline void dummy_flush_tlb_a15_erratum(void)
 {
 }

arch/arm/kernel/smp_tlb.c +0 ~~-23~~

index a98b62d..29e9038 100644
@@ -70,23 +70,6 @@ static inline void ipi_flush_bp_all(void *ignored)
 	local_flush_bp_all();
 }
 
-#ifdef CONFIG_ARM_ERRATA_798181
-static int erratum_a15_798181(void)
-{
-	unsigned int midr = read_cpuid_id();
-
-	/* Cortex-A15 r0p0..r3p2 affected */
-	if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2)
-		return 0;
-	return 1;
-}
-#else
-static int erratum_a15_798181(void)
-{
-	return 0;
-}
-#endif
-
 static void ipi_flush_tlb_a15_erratum(void *arg)
 {
 	dmb();

@@ -94,9 +77,6 @@ static void ipi_flush_tlb_a15_erratum(void *arg)
 
 static void broadcast_tlb_a15_erratum(void)
 {
-	if (!erratum_a15_798181())
-		return;
-
 	dummy_flush_tlb_a15_erratum();
 	smp_call_function(ipi_flush_tlb_a15_erratum, NULL, 1);
 }

@@ -106,9 +86,6 @@ static void broadcast_tlb_mm_a15_erratum(struct mm_struct *mm)
 	int this_cpu;
 	cpumask_t mask = { CPU_BITS_NONE };
 
-	if (!erratum_a15_798181())
-		return;
-
 	dummy_flush_tlb_a15_erratum();
 	this_cpu = get_cpu();
 	a15_erratum_get_cpumask(this_cpu, mm, &mask);

Signature · 1 line

1.8.1.2

Paul Walmsley

2013-07-23 07:04 UTC #1 lore raw

PATCH 1/1 ARM: Do not run dummy_flush_tlb_a15_erratum() on non-Cortex-A15

Quoted 16 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>
Commit 93dc688 (ARM: 7684/1: errata: Workaround for Cortex-A15 erratum 798181 (TLBI/DSB operations)) causes the following undefined instruction error on a mx53 (Cortex-A8):
Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
CPU: 0 PID: 275 Comm: modprobe Not tainted 3.11.0-rc2-next-20130722-00009-g9b0f371 #881
task: df46cc00 ti: df48e000 task.ti: df48e000
PC is at check_and_switch_context+0x17c/0x4d0
LR is at check_and_switch_context+0xdc/0x4d0
This problem happens because check_and_switch_context() calls dummy_flush_tlb_a15_erratum() without checking if we are really running on a Cortex-A15 or not.

Quoted 6 lines

To avoid this issue, always check if we are running on a Cortex-A15 or not (via erratum_a15_798181()) inside dummy_flush_tlb_a15_erratum(), so that we do not need to keep doing the same check in all occurrences of dummy_flush_tlb_a15_erratum().
Signed-off-by Fabio Estevam <[email protected]>

Nice work. This fixes the v3.11-rc boot regression on most OMAP3/AM33xx devices.

Tested-by Paul Walmsley <[email protected]> # for OMAP2,3,4,AM33xx

- Paul

Roger Quadros

2013-07-23 08:13 UTC #2 lore raw

PATCH 1/1 ARM: Do not run dummy_flush_tlb_a15_erratum() on non-Cortex-A15

In reply to Fabio Estevam

Hi Fabio,

Quoted 95 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

Commit 93dc688 (ARM: 7684/1: errata: Workaround for Cortex-A15 erratum 798181 (TLBI/DSB operations)) causes the following undefined instruction error on a mx53 (Cortex-A8):

Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
CPU: 0 PID: 275 Comm: modprobe Not tainted 3.11.0-rc2-next-20130722-00009-g9b0f371 #881
task: df46cc00 ti: df48e000 task.ti: df48e000
PC is at check_and_switch_context+0x17c/0x4d0
LR is at check_and_switch_context+0xdc/0x4d0

This problem happens because check_and_switch_context() calls dummy_flush_tlb_a15_erratum() without checking if we are really running on a Cortex-A15 or not.

Signed-off-by Fabio Estevam <[email protected]>

---
 arch/arm/include/asm/tlbflush.h | 18 ++++++++++++++++++
 arch/arm/kernel/smp_tlb.c       | 23 -----------------------
 2 files changed, 18 insertions(+), 23 deletions(-)

arch/arm/include/asm/tlbflush.h +18 -0

index fdbb9e3..2478b6f 100644
@@ -443,9 +443,22 @@ static inline void local_flush_bp_all(void)
 		isb();
 }
 
+#include <asm/cputype.h>
 #ifdef CONFIG_ARM_ERRATA_798181
+static inline int erratum_a15_798181(void)
+{
+	unsigned int midr = read_cpuid_id();
+
+	/* Cortex-A15 r0p0..r3p2 affected */
+	if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2)
+		return 0;
+	return 1;
+}
+
 static inline void dummy_flush_tlb_a15_erratum(void)
 {
+	if (!erratum_a15_798181())
+		return;
 	/*
 	 * Dummy TLBIMVAIS. Using the unmapped address 0 and ASID 0.
 	 */

@@ -453,6 +466,11 @@ static inline void dummy_flush_tlb_a15_erratum(void)
 	dsb();
 }
 #else
+static inline int erratum_a15_798181(void)
+{
+	return 0;
+}
+
 static inline void dummy_flush_tlb_a15_erratum(void)
 {
 }

arch/arm/kernel/smp_tlb.c +0 ~~-17~~

index a98b62d..29e9038 100644
@@ -70,23 +70,6 @@ static inline void ipi_flush_bp_all(void *ignored)
 	local_flush_bp_all();
 }
 
-#ifdef CONFIG_ARM_ERRATA_798181
-static int erratum_a15_798181(void)
-{
-	unsigned int midr = read_cpuid_id();
-
-	/* Cortex-A15 r0p0..r3p2 affected */
-	if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2)
-		return 0;
-	return 1;
-}
-#else
-static int erratum_a15_798181(void)
-{
-	return 0;
-}
-#endif
-
 static void ipi_flush_tlb_a15_erratum(void *arg)
 {
 	dmb();

arch/arm/kernel/smp_tlb.c static void ipi_flush_tlb_a15_erratum(void *arg) · line 99

Quoted 6 lines

arch/arm/kernel/smp_tlb.c +0 -3

@@ -94,9 +77,6 @@ static void ipi_flush_tlb_a15_erratum(void *arg)
 
 static void broadcast_tlb_a15_erratum(void)
 {
-	if (!erratum_a15_798181())
-		return;
-

Removing this if statement will cause "ipi_flush_tlb_a15_erratum" to be called on non A15 platforms. So I would just keep it to be safe.

arch/arm/kernel/smp_tlb.c static void broadcast_tlb_mm_a15_erratum(struct mm_struct *mm) · line 111

Quoted 10 lines

arch/arm/kernel/smp_tlb.c +0 -3

 	dummy_flush_tlb_a15_erratum();
 	smp_call_function(ipi_flush_tlb_a15_erratum, NULL, 1);
 }

@@ -106,9 +86,6 @@ static void broadcast_tlb_mm_a15_erratum(struct mm_struct *mm)
 	int this_cpu;
 	cpumask_t mask = { CPU_BITS_NONE };
 
-	if (!erratum_a15_798181())
-		return;
-

same here.

arch/arm/kernel/smp_tlb.c static void broadcast_tlb_mm_a15_erratum(struct mm_struct *mm) · line 91

Quoted 4 lines

arch/arm/kernel/smp_tlb.c +0 -0

 	dummy_flush_tlb_a15_erratum();
 	this_cpu = get_cpu();
 	a15_erratum_get_cpumask(this_cpu, mm, &mask);

cheers,
-roger

Fabio Estevam

author

2013-07-23 11:02 UTC #3 lore raw

PATCH 1/1 ARM: Do not run dummy_flush_tlb_a15_erratum() on non-Cortex-A15

Hi Roger,

Quoted 6 lines · Roger Quadros

arch/arm/kernel/smp_tlb.c static void ipi_flush_tlb_a15_erratum(void *arg) · line 99
Quoted 3 lines
arch/arm/kernel/smp_tlb.c +0 -3
-     if (!erratum_a15_798181())
-             return;
-
Removing this if statement will cause "ipi_flush_tlb_a15_erratum" to be called on non A15 platforms. So I would just keep it to be safe.

I just moved the if statement into the the beginning of dummy_flush_tlb_a15_erratum(), so ipi_flush_tlb_a15_erratum will not be called on non-A15 systems.

Regards,

Fabio Esteva,

Roger Quadros

2013-07-23 11:13 UTC #4 lore raw

PATCH 1/1 ARM: Do not run dummy_flush_tlb_a15_erratum() on non-Cortex-A15

Quoted 8 lines · Fabio Estevam

Hi Roger,
Quoted 4 lines · Roger Quadros
Quoted 3 lines
arch/arm/kernel/smp_tlb.c +0 -3
-     if (!erratum_a15_798181())
-             return;
-

Quoted 6 lines

Quoted 2 lines
Removing this if statement will cause "ipi_flush_tlb_a15_erratum" to be called on non A15 platforms. So I would just keep it to be safe.
I just moved the if statement into the the beginning of dummy_flush_tlb_a15_erratum(), so ipi_flush_tlb_a15_erratum will not be called on non-A15 systems.

It will be called. Please see the code below after your patch is applied.

static void broadcast_tlb_a15_erratum(void)
{
dummy_flush_tlb_a15_erratum(); smp_call_function(ipi_flush_tlb_a15_erratum, NULL, 1);
}

cheers,
-roger

Catalin Marinas

2013-07-23 11:07 UTC #5 lore raw

PATCH 1/1 ARM: Do not run dummy_flush_tlb_a15_erratum() on non-Cortex-A15

In reply to Roger Quadros

Quoted 97 lines · Roger Quadros

Quoted 96 lines · Fabio Estevam

From: Fabio Estevam <[email protected]>

Commit 93dc688 (ARM: 7684/1: errata: Workaround for Cortex-A15 erratum 798181 (TLBI/DSB operations)) causes the following undefined instruction error on a mx53 (Cortex-A8):

Internal error: Oops - undefined instruction: 0 [#1] SMP ARM
CPU: 0 PID: 275 Comm: modprobe Not tainted 3.11.0-rc2-next-20130722-00009-g9b0f371 #881
task: df46cc00 ti: df48e000 task.ti: df48e000
PC is at check_and_switch_context+0x17c/0x4d0
LR is at check_and_switch_context+0xdc/0x4d0

This problem happens because check_and_switch_context() calls dummy_flush_tlb_a15_erratum() without checking if we are really running on a Cortex-A15 or not.

Signed-off-by Fabio Estevam <[email protected]>

---
 arch/arm/include/asm/tlbflush.h | 18 ++++++++++++++++++
 arch/arm/kernel/smp_tlb.c       | 23 -----------------------
 2 files changed, 18 insertions(+), 23 deletions(-)

arch/arm/include/asm/tlbflush.h +18 -0

index fdbb9e3..2478b6f 100644
@@ -443,9 +443,22 @@ static inline void local_flush_bp_all(void)
 		isb();
 }
 
+#include <asm/cputype.h>
 #ifdef CONFIG_ARM_ERRATA_798181
+static inline int erratum_a15_798181(void)
+{
+	unsigned int midr = read_cpuid_id();
+
+	/* Cortex-A15 r0p0..r3p2 affected */
+	if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2)
+		return 0;
+	return 1;
+}
+
 static inline void dummy_flush_tlb_a15_erratum(void)
 {
+	if (!erratum_a15_798181())
+		return;
 	/*
 	 * Dummy TLBIMVAIS. Using the unmapped address 0 and ASID 0.
 	 */

@@ -453,6 +466,11 @@ static inline void dummy_flush_tlb_a15_erratum(void)
 	dsb();
 }
 #else
+static inline int erratum_a15_798181(void)
+{
+	return 0;
+}
+
 static inline void dummy_flush_tlb_a15_erratum(void)
 {
 }

arch/arm/kernel/smp_tlb.c +0 ~~-17~~

index a98b62d..29e9038 100644
@@ -70,23 +70,6 @@ static inline void ipi_flush_bp_all(void *ignored)
 	local_flush_bp_all();
 }
 
-#ifdef CONFIG_ARM_ERRATA_798181
-static int erratum_a15_798181(void)
-{
-	unsigned int midr = read_cpuid_id();
-
-	/* Cortex-A15 r0p0..r3p2 affected */
-	if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2)
-		return 0;
-	return 1;
-}
-#else
-static int erratum_a15_798181(void)
-{
-	return 0;
-}
-#endif
-
 static void ipi_flush_tlb_a15_erratum(void *arg)
 {
 	dmb();

@@ -94,9 +77,6 @@ static void ipi_flush_tlb_a15_erratum(void *arg)

Quoted 8 lines

arch/arm/kernel/smp_tlb.c static void ipi_flush_tlb_a15_erratum(void *arg) · line 99
Quoted 5 lines
arch/arm/kernel/smp_tlb.c +0 -3
 static void broadcast_tlb_a15_erratum(void)
 {
-	if (!erratum_a15_798181())
-		return;
-
Removing this if statement will cause "ipi_flush_tlb_a15_erratum" to be called on non A15 platforms. So I would just keep it to be safe.

I agree, this must stay. But to avoid erratum_a15_798181() called too many times, I would rather add the condition in check_and_switch_context() where dummy_flush_tlb_a15_erratum() is called.

An alternative (and Will has a patch) is to always use cpumask_setall(&tlb_flush_pending) in flush_context() and the check_and_switch_context() function doesn't need to guarantee any broadcast. In this case we simply remove dummy_flush_tlb_a15_erratum() in context.c (but I guess for cc: stable we can use an 'if (erratum...)').

Signature · 1 line

Catalin