ARM ERRATA 775420 and 782773 workarounds

Review

Quoted 11 lines · Simon Horman

(quoted diff) +10 -0

+config ARM_ERRATA_775420
+       bool "ARM errata: A data cache maintenance operation which aborts, might lead to deadlock"
+       depends on CPU_V7
+       help
+         This option enables the workaround for the 775420 Cortex-A9 (r2p2,
+         r2p6,r2p8,r2p10,r3p0) erratum. In case a date cache maintenance
+         operation aborts with MMU exception, it might cause the processor
+         deadlock. This workaround puts DSB before executing ISB at the
+         beginning of the abort exception handler.
+
 endmenu

The only case where we can get an abort on cache maintenance is v7_coherent_user_range(). I don't think we have any ISB on the exception handling path for this function, so we could just add the DSB there:

Quoted 17 lines · Catalin Marinas

arch/arm/Kconfig config PL310_ERRATA_769419 · line 1426

Quoted 11 lines · Simon Horman

arch/arm/Kconfig +10 -0

+config ARM_ERRATA_775420
+       bool "ARM errata: A data cache maintenance operation which aborts, might lead to deadlock"
+       depends on CPU_V7
+       help
+         This option enables the workaround for the 775420 Cortex-A9 (r2p2,
+         r2p6,r2p8,r2p10,r3p0) erratum. In case a date cache maintenance
+         operation aborts with MMU exception, it might cause the processor
+         deadlock. This workaround puts DSB before executing ISB at the
+         beginning of the abort exception handler.
+
 endmenu

The only case where we can get an abort on cache maintenance is v7_coherent_user_range(). I don't think we have any ISB on the exception handling path for this function, so we could just add the DSB there:

I think that an advantage of Abe-san's implementation is that it might to be a bit more robust. But your proposal is certainly much cleaner and for that reason I agree it is a good option.

I've updated the patch, but since the code is now all yours I'm unsure if the author should be changed or not.

Quoted 4 lines · Simon Horman

(quoted diff) +0 -0

@@ -1423,6 +1423,15 @@ config ARM_ERRATA_775420
          deadlock. This workaround puts DSB before executing ISB at the
          beginning of the abort exception handler.
 

Review config ARM_ERRATA_775420 · line 1431

Quoted 6 lines

(quoted diff) +6 -0

+config ARM_ERRATA_782773
+	bool "ARM errata: Updating a translation entry might cause an unexpected translation fault"
+	depends on CPU_V7
+	help
+	  This option enables the workaround for the 782773 Cortex-A9 (all r0,
+	  ,r2 and r3 revisions) erratum. It might cause MMU exception in case

Seems to be an extra comma here.

arch/arm/Kconfig config ARM_ERRATA_775420 · line 1433

Quoted 8 lines · Simon Horman

arch/arm/Kconfig +8 -0

+config ARM_ERRATA_782773
+     bool "ARM errata: Updating a translation entry might cause an unexpected translation fault"
+     depends on CPU_V7
+     help
+       This option enables the workaround for the 782773 Cortex-A9 (all r0,
+       r2 and r3 revisions) erratum. It might cause MMU exception in case
+       page table walk happens just after updating the existing
+       with setting page table in L1 data cache.

What if we're running on a SMP system where the L1 caches are mandated to be in write-allocate mode? This write will immediately cause the cache line to be brought back into the cache.

Quoted 5 lines · Stephen Boyd

Quoted 4 lines · Simon Horman

arch/arm/Kconfig +0 -0

@@ -1423,6 +1423,15 @@ config ARM_ERRATA_775420
          deadlock. This workaround puts DSB before executing ISB at the
          beginning of the abort exception handler.
 

Quoted 8 lines

arch/arm/Kconfig config ARM_ERRATA_775420

Quoted 6 lines

arch/arm/Kconfig +6 -0

+config ARM_ERRATA_782773
+   bool "ARM errata: Updating a translation entry might cause an unexpected translation fault"
+   depends on CPU_V7
+   help
+     This option enables the workaround for the 782773 Cortex-A9 (all r0,
+     ,r2 and r3 revisions) erratum. It might cause MMU exception in case

Seems to be an extra comma here.

Thanks, here is an updated version.

From: Kouei Abe <[email protected]>

arm: Add ARM ERRATA 782773 workaround

Signed-off-by Kouei Abe <[email protected]>

Signed-off-by Simon Horman <[email protected]>

Quoted 19 lines · Simon Horman

Quoted 5 lines · Stephen Boyd

Quoted 4 lines · Simon Horman

(quoted diff) +0 -0

@@ -1423,6 +1423,15 @@ config ARM_ERRATA_775420
          deadlock. This workaround puts DSB before executing ISB at the
          beginning of the abort exception handler.
 

Quoted 8 lines

Review config ARM_ERRATA_775420 · line 1431

Quoted 6 lines

(quoted diff) +6 -0

+config ARM_ERRATA_782773
+   bool "ARM errata: Updating a translation entry might cause an unexpected translation fault"
+   depends on CPU_V7
+   help
+     This option enables the workaround for the 782773 Cortex-A9 (all r0,
+     ,r2 and r3 revisions) erratum. It might cause MMU exception in case

Seems to be an extra comma here.

Thanks, here is an updated version.

From: Kouei Abe <[email protected]>

Quoted 4 lines

arm: Add ARM ERRATA 782773 workaround

Signed-off-by Kouei Abe <[email protected]>

Signed-off-by Simon Horman <[email protected]>

I would add some text to the commit log as well, even though it matches the Kconfig entry.

Have you hit this in practice? In general the kernel shouldn't access kernel virtual address corresponding to a page table that is being changed. For user address space it is possible but the kernel can handle use translation faults, even though they may be spurious.

Quoted 20 lines · Catalin Marinas

Quoted 19 lines · Simon Horman

Quoted 5 lines · Stephen Boyd

Quoted 4 lines · Simon Horman

(quoted diff) +0 -0

@@ -1423,6 +1423,15 @@ config ARM_ERRATA_775420
          deadlock. This workaround puts DSB before executing ISB at the
          beginning of the abort exception handler.
 

Quoted 8 lines

arch/arm/Kconfig config ARM_ERRATA_775420

Quoted 6 lines

arch/arm/Kconfig +6 -0

+config ARM_ERRATA_782773
+   bool "ARM errata: Updating a translation entry might cause an unexpected translation fault"
+   depends on CPU_V7
+   help
+     This option enables the workaround for the 782773 Cortex-A9 (all r0,
+     ,r2 and r3 revisions) erratum. It might cause MMU exception in case

Seems to be an extra comma here.

Thanks, here is an updated version.

From: Kouei Abe <[email protected]>

Quoted 7 lines

Quoted 4 lines

arm: Add ARM ERRATA 782773 workaround

Signed-off-by Kouei Abe <[email protected]>

Signed-off-by Simon Horman <[email protected]>

I would add some text to the commit log as well, even though it matches the Kconfig entry.

Sure, an updated patch is below.
I also reworded the text to make it easier on my eyes, I don't think the meaning has been altered.

Quoted 4 lines

Have you hit this in practice? In general the kernel shouldn't access kernel virtual address corresponding to a page table that is being changed. For user address space it is possible but the kernel can handle use translation faults, even though they may be spurious.

I believe that Abe-san's team have come up against this, I can confirm that if it is important.

----------------------------------------------------------------
From: Kouei Abe <[email protected]>

arm: Add ARM ERRATA 782773 workaround

This is a workaround for Errata 782773 which effects all r0, r2 and r3 revisions.

The work-around avoids the possibility of an MMU exception in the case where a page table walk occurs immediately after a page table update that hasn't been flushed from the L1 data cache.

Cc Catalin Marinas <[email protected]>

Signed-off-by Kouei Abe <[email protected]>

Signed-off-by Simon Horman <[email protected]>

---

v2
* Reword Kconfig description
* Add some details to changelog entry

arch/arm/Kconfig +9 -0

index 48c19d4..2b76164 100644
@@ -1423,6 +1423,15 @@ config ARM_ERRATA_775420
 	 to deadlock. This workaround puts DSB before executing ISB if
 	 an abort may occur on cache maintenance.
 
+config ARM_ERRATA_782773
+	bool "ARM errata: Updating a translation entry might cause an unexpected translation fault"
+	depends on CPU_V7
+	help
+	  This option enables the workaround for the 782773 Cortex-A9 (all r0,
+	  r2 and r3 revisions) erratum. It might cause MMU exception in case
+	  page table walk happens just after updating the existing
+	  with setting page table in L1 data cache.
+
 endmenu
 
 source "arch/arm/common/Kconfig"

arch/arm/mm/proc-v7-2level.S +8 -0

index fd045e7..9207b9f 100644
@@ -103,9 +103,17 @@ ENTRY(cpu_v7_set_pte_ext)
 	tstne	r1, #L_PTE_PRESENT
 	moveq	r3, #0
 
+#ifdef CONFIG_ARM_ERRATA_782773
+	mrs	r2, cpsr			@ save cpsr
+	cpsid	if				@ disable interrupts
+	mcr	p15, 0, r0, c7, c14, 1		@ clean & invalidate D line
+#endif
  ARM(	str	r3, [r0, #2048]! )
  THUMB(	add	r0, r0, #2048 )
  THUMB(	str	r3, [r0] )
+#ifdef CONFIG_ARM_ERRATA_782773
+	msr	cpsr_c, r2			@ load cpsr
+#endif
 	mcr	p15, 0, r0, c7, c10, 1		@ flush_pte
 #endif
 	mov	pc, lr

Signature · 6 lines

1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-sh" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoted 38 lines · Simon Horman

Quoted 20 lines · Catalin Marinas

Quoted 19 lines · Simon Horman

Quoted 5 lines · Stephen Boyd

Quoted 4 lines · Simon Horman

(quoted diff) +0 -0

@@ -1423,6 +1423,15 @@ config ARM_ERRATA_775420
          deadlock. This workaround puts DSB before executing ISB at the
          beginning of the abort exception handler.
 

Quoted 8 lines

arch/arm/Kconfig config ARM_ERRATA_775420

Quoted 6 lines

arch/arm/Kconfig +6 -0

+config ARM_ERRATA_782773
+   bool "ARM errata: Updating a translation entry might cause an unexpected translation fault"
+   depends on CPU_V7
+   help
+     This option enables the workaround for the 782773 Cortex-A9 (all r0,
+     ,r2 and r3 revisions) erratum. It might cause MMU exception in case

Seems to be an extra comma here.

Thanks, here is an updated version.

From: Kouei Abe <[email protected]>

Quoted 7 lines

Quoted 4 lines

arm: Add ARM ERRATA 782773 workaround

Signed-off-by Kouei Abe <[email protected]>

Signed-off-by Simon Horman <[email protected]>

I would add some text to the commit log as well, even though it matches the Kconfig entry.

Sure, an updated patch is below.
I also reworded the text to make it easier on my eyes, I don't think the meaning has been altered.

Quoted 4 lines

Have you hit this in practice? In general the kernel shouldn't access kernel virtual address corresponding to a page table that is being changed. For user address space it is possible but the kernel can handle use translation faults, even though they may be spurious.

Quoted 2 lines

I believe that Abe-san's team have come up against this, I can confirm that if it is important.

I was wondering if I could get an Ack on this as you indicated in another email that you feel that the implementation is an appropriate workaround.

Quoted 39 lines · Simon Horman

Quoted 38 lines · Simon Horman

Quoted 20 lines · Catalin Marinas

Quoted 19 lines · Simon Horman

Quoted 5 lines · Stephen Boyd

Quoted 4 lines · Simon Horman

(quoted diff) +0 -0

@@ -1423,6 +1423,15 @@ config ARM_ERRATA_775420
          deadlock. This workaround puts DSB before executing ISB at the
          beginning of the abort exception handler.
 

Quoted 8 lines

arch/arm/Kconfig config ARM_ERRATA_775420

Quoted 6 lines

arch/arm/Kconfig +6 -0

+config ARM_ERRATA_782773
+   bool "ARM errata: Updating a translation entry might cause an unexpected translation fault"
+   depends on CPU_V7
+   help
+     This option enables the workaround for the 782773 Cortex-A9 (all r0,
+     ,r2 and r3 revisions) erratum. It might cause MMU exception in case

Seems to be an extra comma here.

Thanks, here is an updated version.

From: Kouei Abe <[email protected]>

Quoted 7 lines

Quoted 4 lines

arm: Add ARM ERRATA 782773 workaround

Signed-off-by Kouei Abe <[email protected]>

Signed-off-by Simon Horman <[email protected]>

I would add some text to the commit log as well, even though it matches the Kconfig entry.

Sure, an updated patch is below.
I also reworded the text to make it easier on my eyes, I don't think the meaning has been altered.

Quoted 4 lines

Have you hit this in practice? In general the kernel shouldn't access kernel virtual address corresponding to a page table that is being changed. For user address space it is possible but the kernel can handle use translation faults, even though they may be spurious.

Quoted 6 lines

Quoted 2 lines

I believe that Abe-san's team have come up against this, I can confirm that if it is important.

I was wondering if I could get an Ack on this as you indicated in another email that you feel that the implementation is an appropriate workaround.

I'd like to know whether this is actually hit in practice and the conditions. The only time when we write live (i.e. translation that's currently in use) kernel mappings is during boot, alloc_init_section() replacing (though with the same value) the initial memory map. But if that's the scenario you are getting, I would rather add another flush_pmd_entry() call before setting the pmd in alloc_init_section().

I'm not convinced a costly run-time workaround in set_pte is needed.