Skip to contentSkip to conversation

ARM: Add check for Cortex-A15 errata 798181 ECO

PATCH New Rob Herring 3 participants 4 messages Jul 15, 2013 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: Add check for Cortex-A15 errata 798181 ECO

From: Rob Herring <[email protected]>

The work-around for A15 errata 798181 is not needed if appropriate ECO fixes have been applied to r3p2 and earlier core revisions. This can be checked by reading REVIDR register bits 4 and 9. If only bit 4 is set, then the IPI broadcast can be skipped.

---

Compile tested only.

 arch/arm/include/asm/cputype.h |  1 +
 arch/arm/kernel/smp_tlb.c      | 13 +++++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)
arch/arm/include/asm/cputype.h +1 -0
index ec635ff..ac1c46a 100644
@@ -9,6 +9,7 @@
 #define CPUID_TCM	2
 #define CPUID_TLBTYPE	3
 #define CPUID_MPIDR	5
+#define CPUID_REVIDR	6
 
 #ifdef CONFIG_CPU_V7M
 #define CPUID_EXT_PFR0	0x40
arch/arm/kernel/smp_tlb.c +11 -2
index 9a52a07..7fd64b7 100644
@@ -74,10 +74,21 @@ static inline void ipi_flush_bp_all(void *ignored)
 static int erratum_a15_798181(void)
 {
 	unsigned int midr = read_cpuid_id();
+	unsigned int revidr;
 
 	/* Cortex-A15 r0p0..r3p2 affected */
 	if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2)
 		return 0;
+
+	/* Check for Cortex A15 <= r3p2 with ECO fix */
+	revidr = read_cpuid(CPUID_REVIDR);
+	if ((revidr & 0x210) == 0x210)
+		return 0;
+
+	dummy_flush_tlb_a15_erratum();
+	if (revidr & 0x10)
+		return 0;
+
 	return 1;
 }
 #else
@@ -97,7 +108,6 @@ 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);
 }
 
@@ -109,7 +119,6 @@ static void broadcast_tlb_mm_a15_erratum(struct mm_struct *mm)
 	if (!erratum_a15_798181())
 		return;
 
-	dummy_flush_tlb_a15_erratum();
 	this_cpu = get_cpu();
 	for_each_online_cpu(cpu) {
 		if (cpu == this_cpu)
Signature · 1 line
1.8.1.2
#1 lore raw

PATCH 1/1 ARM: Add check for Cortex-A15 errata 798181 ECO

Quoted 8 lines · Rob Herring
arch/arm/kernel/smp_tlb.c +0 -0
index 9a52a07..7fd64b7 100644
@@ -74,10 +74,21 @@ static inline void ipi_flush_bp_all(void *ignored)
 static int erratum_a15_798181(void)
 {
        unsigned int midr = read_cpuid_id();

arch/arm/kernel/smp_tlb.c static inline void ipi_flush_bp_all(void *ignored) · line 86

Quoted 10 lines
arch/arm/kernel/smp_tlb.c +6 -0
+       unsigned int revidr;
 
        /* Cortex-A15 r0p0..r3p2 affected */
        if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2)
                return 0;
+
+       /* Check for Cortex A15 <= r3p2 with ECO fix */
+       revidr = read_cpuid(CPUID_REVIDR);
+       if ((revidr & 0x210) == 0x210)
+               return 0;

Reading and evaluating all this on every invalidate seems suboptimal. It should be possible to read just once and cache the result.

I would have had the same comment about the original patch, but ARM pushes those to Russell in secret so it wasn't on the list. :-)

-Olof

#2 lore raw

PATCH 1/1 ARM: Add check for Cortex-A15 errata 798181 ECO

Quoted 11 lines · Olof Johansson
Quoted 10 lines · Rob Herring
arch/arm/kernel/smp_tlb.c +1 -0
index 9a52a07..7fd64b7 100644
@@ -74,10 +74,21 @@ static inline void ipi_flush_bp_all(void *ignored)
 static int erratum_a15_798181(void)
 {
        unsigned int midr = read_cpuid_id();
+       unsigned int revidr;
 
Quoted 11 lines

arch/arm/kernel/smp_tlb.c static inline void ipi_flush_bp_all(void *ignored) · line 86

Quoted 8 lines
arch/arm/kernel/smp_tlb.c +5 -0
        /* Cortex-A15 r0p0..r3p2 affected */
        if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2)
                return 0;
+
+       /* Check for Cortex A15 <= r3p2 with ECO fix */
+       revidr = read_cpuid(CPUID_REVIDR);
+       if ((revidr & 0x210) == 0x210)
+               return 0;

Reading and evaluating all this on every invalidate seems suboptimal. It should be possible to read just once and cache the result.

Yes, but I'm not sure it is so clear. 2 coproc reads may be faster than a load potentially from memory. But then how much more code do we have to fetch. How about something like this:

static int erratum_a15_798181(void)

{
	static int errata_fix_needed = -1;

	if (unlikely(errata_fix_needed == -1)) {
		unsigned int midr = read_cpuid_id();
		unsigned int revidr = read_cpuid(CPUID_REVIDR);

		/* Cortex-A15 r0p0..r3p2 w/o ECO fix affected */
		if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2 ||
		    (revidr & 0x210) == 0x210) {
			errata_fix_needed = 0;
			return 0;
		}
		if (revidr & 0x10)
			errata_fix_needed = 1;

		errata_fix_needed = 2;
	}

	if (errata_fix_needed)
		dummy_flush_tlb_a15_erratum();
	return (errata_fix_needed == 2) ? 1: 0;
}
Quoted 2 lines

I would have had the same comment about the original patch, but ARM pushes those to Russell in secret so it wasn't on the list. :-)

And I would have commented that we need to handle ECO fixes...

Rob

#3 lore raw

PATCH 1/1 ARM: Add check for Cortex-A15 errata 798181 ECO

In reply to Olof Johansson
Quoted 23 lines · Olof Johansson
Quoted 8 lines · Rob Herring
arch/arm/kernel/smp_tlb.c +0 -0
index 9a52a07..7fd64b7 100644
@@ -74,10 +74,21 @@ static inline void ipi_flush_bp_all(void *ignored)
 static int erratum_a15_798181(void)
 {
        unsigned int midr = read_cpuid_id();

arch/arm/kernel/smp_tlb.c static inline void ipi_flush_bp_all(void *ignored) · line 86

Quoted 10 lines
arch/arm/kernel/smp_tlb.c +6 -0
+       unsigned int revidr;
 
        /* Cortex-A15 r0p0..r3p2 affected */
        if ((midr & 0xff0ffff0) != 0x410fc0f0 || midr > 0x413fc0f2)
                return 0;
+
+       /* Check for Cortex A15 <= r3p2 with ECO fix */
+       revidr = read_cpuid(CPUID_REVIDR);
+       if ((revidr & 0x210) == 0x210)
+               return 0;

Reading and evaluating all this on every invalidate seems suboptimal. It should be possible to read just once and cache the result.

Quoted 2 lines

I would have had the same comment about the original patch, but ARM pushes those to Russell in secret so it wasn't on the list. :-)

For a single read, there's probably not much between reading the CP15 register and doing the tests, vs reading a bool from memory to decide whether we need to do it.

For more than that (as the above patch is growing) it probably makes sense to think about using a bool in memory now.