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

PATCH New Fabio Estevam 6 participants 8 messages Jul 23, 2013 – Aug 1, 2013 Arm (32-bit)

lore

Share & tools

lore

Fabio Estevam

author

2013-07-23 12:06 UTC #0 lore raw

PATCH v2 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, only call dummy_flush_tlb_a15_erratum() inside check_and_switch_context() if erratum_a15_798181() returns true, which means that we are really running on a Cortex-A15.

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

---
Changes since v1:
- Add 'if (erratum_a15_798181())' inside check_and_switch_context()

 arch/arm/include/asm/tlbflush.h | 16 ++++++++++++++++
 arch/arm/kernel/smp_tlb.c       | 17 -----------------
 arch/arm/mm/context.c           |  3 ++-
 3 files changed, 18 insertions(+), 18 deletions(-)

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

index fdbb9e3..f467e9b 100644
@@ -443,7 +443,18 @@ 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)
 {
 	/*

@@ -453,6 +464,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..c2edfff 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/mm/context.c +2 -1

index b55b101..4a05444 100644
@@ -245,7 +245,8 @@ void check_and_switch_context(struct mm_struct *mm, struct task_struct *tsk)
 	if (cpumask_test_and_clear_cpu(cpu, &tlb_flush_pending)) {
 		local_flush_bp_all();
 		local_flush_tlb_all();
-		dummy_flush_tlb_a15_erratum();
+		if (erratum_a15_798181())
+			dummy_flush_tlb_a15_erratum();
 	}
 
 	atomic64_set(&per_cpu(active_asids, cpu), asid);

Signature · 1 line

1.8.1.2

Catalin Marinas

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

PATCH v2 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 5 lines

To avoid this issue, only call dummy_flush_tlb_a15_erratum() inside check_and_switch_context() if erratum_a15_798181() returns true, which means that we are really running on a Cortex-A15.
Signed-off-by Fabio Estevam <[email protected]>

Acked-by Catalin Marinas <[email protected]>

Roger Quadros

2013-07-23 14:04 UTC #2 lore raw

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

In reply to Fabio Estevam

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 5 lines

To avoid this issue, only call dummy_flush_tlb_a15_erratum() inside check_and_switch_context() if erratum_a15_798181() returns true, which means that we are really running on a Cortex-A15.
Signed-off-by Fabio Estevam <[email protected]>

Reviewed-by Roger Quadros <[email protected]>

Paul Walmsley

2013-07-24 17:00 UTC #3 lore raw

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

In reply to Fabio Estevam

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 5 lines

To avoid this issue, only call dummy_flush_tlb_a15_erratum() inside check_and_switch_context() if erratum_a15_798181() returns true, which means that we are really running on a Cortex-A15.
Signed-off-by Fabio Estevam <[email protected]>

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

Fixes the v3.11-rc boot regression that broke multiple boards.

- Paul

I just submitted my patch for 798181 ECO fix handling to the patch system today. Then I noticed this patch in your fixes branch. This is going to cause some churn as my patch needs to move the core version check back to a .c file.

I don't know if you are okay with rebasing your fixes branch or not. If my patch is applied first, then this issue can be handled by just replacing dummy_flush_tlb_a15_erratum call in context.c a call to erratum_a15_798181.

Rob

Russell King - ARM Linux

2013-08-01 10:58 UTC #5 lore raw

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

Quoted 9 lines · Rob Herring

I just submitted my patch for 798181 ECO fix handling to the patch system today. Then I noticed this patch in your fixes branch. This is going to cause some churn as my patch needs to move the core version check back to a .c file.
I don't know if you are okay with rebasing your fixes branch or not. If my patch is applied first, then this issue can be handled by just replacing dummy_flush_tlb_a15_erratum call in context.c a call to erratum_a15_798181.

Well, firstly, are you sure your patch is correct?

You're removing the call to dummy_flush_tlb_a15_erratum() before the smp_call_function() - smp_call_function() does _not_ call the function for the local CPU. So, what this means is we don't end up doing any of the workaround for the local CPU.

Secondly, doesn't the check in your case reduce down to:

	unsigned int midr = read_cpuid_id();

	/* Cortex-A15 r0p0..r3p2 affected */
	if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2 ||
	    /* ECO not fully fixed */
	    !(read_cpuid(CPUID_REVIDR) & 0x010))
		return 0;
	return 1;

because, assuming midr is not satisfied:

revidr errata_fix_needed function return
000 2 1
010 1 0
200 2 1
210 0 0

I think that the above version is still cheap enough to be inline.

Rob Herring

2013-08-01 20:09 UTC #6 lore raw

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

Quoted 6 lines · Russell King - ARM Linux

Quoted 5 lines · Rob Herring
I just submitted my patch for 798181 ECO fix handling to the patch system today. Then I noticed this patch in your fixes branch. This is going to cause some churn as my patch needs to move the core version check back to a .c file.

Quoted 6 lines

Quoted 4 lines
I don't know if you are okay with rebasing your fixes branch or not. If my patch is applied first, then this issue can be handled by just replacing dummy_flush_tlb_a15_erratum call in context.c a call to erratum_a15_798181.
Well, firstly, are you sure your patch is correct?

Yes.

Quoted 4 lines

You're removing the call to dummy_flush_tlb_a15_erratum() before the smp_call_function() - smp_call_function() does _not_ call the function for the local CPU. So, what this means is we don't end up doing any of the workaround for the local CPU.

No, dummy_flush_tlb_a15_erratum() is now called from within erratum_a15_798181 if errata_fix_needed is non-zero. The return value only means do the IPI or not.

Quoted 13 lines

Secondly, doesn't the check in your case reduce down to:

        unsigned int midr = read_cpuid_id();

        /* Cortex-A15 r0p0..r3p2 affected */
        if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2 ||
            /* ECO not fully fixed */
            !(read_cpuid(CPUID_REVIDR) & 0x010))
                return 0;
        return 1;

because, assuming midr is not satisfied:

Quoted 7 lines

revidr  errata_fix_needed       function return
000     2                       1
010     1                       0
200     2                       1
210     0                       0

I think that the above version is still cheap enough to be inline.

Only because it is incomplete. :)

Rob

Russell King - ARM Linux

2013-08-01 20:15 UTC #7 lore raw

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

Quoted 45 lines · Rob Herring

Quoted 6 lines · Russell King - ARM Linux
Quoted 5 lines · Rob Herring
I just submitted my patch for 798181 ECO fix handling to the patch system today. Then I noticed this patch in your fixes branch. This is going to cause some churn as my patch needs to move the core version check back to a .c file.
Quoted 6 lines
Quoted 4 lines
I don't know if you are okay with rebasing your fixes branch or not. If my patch is applied first, then this issue can be handled by just replacing dummy_flush_tlb_a15_erratum call in context.c a call to erratum_a15_798181.
Well, firstly, are you sure your patch is correct?
Yes.
Quoted 4 lines
You're removing the call to dummy_flush_tlb_a15_erratum() before the smp_call_function() - smp_call_function() does _not_ call the function for the local CPU. So, what this means is we don't end up doing any of the workaround for the local CPU.
No, dummy_flush_tlb_a15_erratum() is now called from within erratum_a15_798181 if errata_fix_needed is non-zero. The return value only means do the IPI or not.
Quoted 19 lines
Secondly, doesn't the check in your case reduce down to:
        unsigned int midr = read_cpuid_id();

        /* Cortex-A15 r0p0..r3p2 affected */
        if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2 ||
            /* ECO not fully fixed */
            !(read_cpuid(CPUID_REVIDR) & 0x010))
                return 0;
        return 1;
because, assuming midr is not satisfied:
revidr  errata_fix_needed       function return
000     2                       1
010     1                       0
200     2                       1
210     0                       0

Quoted 3 lines

Quoted 1 line
I think that the above version is still cheap enough to be inline.
Only because it is incomplete. :)

Your timing is very bad. I just sent the entire fixes branch (and some other stuff - which was scheduled for a long time to be sent tonight) off to Linus having waited all day. It was either that or sit on the fixes branch even longer.