[PATCH] ARM: Add check for Cortex-A15 errata 798181 ECO

[PATCH] ARM: Add check for Cortex-A15 errata 798181 ECO

[Rob Herring]

From: Rob Herring <rob.herring@calxeda.com>

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.

Signed-off-by: Rob Herring <rob.herring@calxeda.com>
---

Compile tested only.

 arch/arm/include/asm/cputype.h |  1 +
 arch/arm/kernel/smp_tlb.c      | 13 +++++++++++--
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/arch/arm/include/asm/cputype.h b/arch/arm/include/asm/cputype.h
index ec635ff..ac1c46a 100644
--- a/arch/arm/include/asm/cputype.h
+++ b/arch/arm/include/asm/cputype.h
@@ -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
diff --git a/arch/arm/kernel/smp_tlb.c b/arch/arm/kernel/smp_tlb.c
index 9a52a07..7fd64b7 100644
--- a/arch/arm/kernel/smp_tlb.c
+++ b/arch/arm/kernel/smp_tlb.c
@@ -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)
-- 
1.8.1.2

[PATCH] ARM: Add check for Cortex-A15 errata 798181 ECO

[Olof Johansson]

On Mon, Jul 15, 2013 at 1:42 PM, Rob Herring <robherring2@gmail.com> wrote:
> diff --git a/arch/arm/kernel/smp_tlb.c b/arch/arm/kernel/smp_tlb.c
> index 9a52a07..7fd64b7 100644
> --- a/arch/arm/kernel/smp_tlb.c
> +++ b/arch/arm/kernel/smp_tlb.c
> @@ -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;

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

[PATCH] ARM: Add check for Cortex-A15 errata 798181 ECO

[Rob Herring]

On 07/15/2013 03:59 PM, Olof Johansson wrote:
> On Mon, Jul 15, 2013 at 1:42 PM, Rob Herring <robherring2@gmail.com> wrote:
>> diff --git a/arch/arm/kernel/smp_tlb.c b/arch/arm/kernel/smp_tlb.c
>> index 9a52a07..7fd64b7 100644
>> --- a/arch/arm/kernel/smp_tlb.c
>> +++ b/arch/arm/kernel/smp_tlb.c
>> @@ -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;
> 
> 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;
}


> 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

[PATCH] ARM: Add check for Cortex-A15 errata 798181 ECO

[Russell King - ARM Linux]

On Mon, Jul 15, 2013 at 01:59:24PM -0700, Olof Johansson wrote:
> On Mon, Jul 15, 2013 at 1:42 PM, Rob Herring <robherring2@gmail.com> wrote:
> > diff --git a/arch/arm/kernel/smp_tlb.c b/arch/arm/kernel/smp_tlb.c
> > index 9a52a07..7fd64b7 100644
> > --- a/arch/arm/kernel/smp_tlb.c
> > +++ b/arch/arm/kernel/smp_tlb.c
> > @@ -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;
> 
> 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. :-)

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.