[PATCH 3/4 v2] ARM: L2 : Errata 588369: Clean & Invalidate do not invalidate clean lines

[PATCH 3/4 v2] ARM: L2 : Errata 588369: Clean & Invalidate do not invalidate clean lines

[Santosh Shilimkar]

This patch implements the work-around for the errata 588369. The secure API
is used to alter L2 debug regsiter because of trust-zone.

Signed-off-by: Woodruff Richard <r-woodruff2@ti.com>
Signed-off-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm/Kconfig         |   14 ++++++++++++++
 arch/arm/mm/cache-l2x0.c |   44 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 58 insertions(+), 0 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 233a222..daae2d0 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -917,6 +917,20 @@ config ARM_ERRATA_460075
 	  ACTLR register. Note that setting specific bits in the ACTLR register
 	  may not be available in non-secure mode.
 
+config PL310_ERRATA_588369
+	bool "Clean & Invalidate maintenance operations do not invalidate clean lines"
+	depends on CACHE_L2X0 && ARCH_OMAP4
+	default n
+	help
+	   The PL310 L2 cache controller implements three types of Clean &
+	   Invalidate maintenance operations: by Physical Address
+	   (offset 0x7F0), by Index/Way (0x7F8) and by Way (0x7FC).
+	   They are architecturally defined to behave as the execution of a
+	   clean operation followed immediately by an invalidate operation,
+	   both performing to the same memory location. This functionality
+	   is not correctly implemented in PL310 as clean lines are not
+	   invalidated as a result of these operations. Note that this errata
+	   uses Texas Instrument's secure monitor api.
 endmenu
 
 source "arch/arm/common/Kconfig"
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 747f9a9..4d33dc9 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -28,6 +28,21 @@
 static void __iomem *l2x0_base;
 static DEFINE_SPINLOCK(l2x0_lock);
 
+#ifdef CONFIG_PL310_ERRATA_588369
+static void debug_writel(unsigned long val)
+{
+	/*
+	 * Texas Instrument secure monitor api to modify the PL310
+	 * Debug Control Register. R0 = val
+	 */
+	__asm__ __volatile__(
+	"stmfd r13!, {r4-r8}\n"
+	"ldr r12, =0x100\n"
+	"dsb\n"
+	"smc\n"
+	"ldmfd r13!, {r4-r8}");
+}
+#endif
 static inline void sync_writel(unsigned long val, unsigned long reg,
 			       unsigned long complete_mask)
 {
@@ -59,13 +74,27 @@ static void l2x0_inv_range(unsigned long start, unsigned long end)
 
 	if (start & (CACHE_LINE_SIZE - 1)) {
 		start &= ~(CACHE_LINE_SIZE - 1);
+#ifdef CONFIG_PL310_ERRATA_588369
+		debug_writel(0x03);
+		sync_writel(start, L2X0_CLEAN_LINE_PA, 1);
+		sync_writel(start, L2X0_INV_LINE_PA, 1);
+		debug_writel(0x00);
+#else
 		sync_writel(start, L2X0_CLEAN_INV_LINE_PA, 1);
+#endif
 		start += CACHE_LINE_SIZE;
 	}
 
 	if (end & (CACHE_LINE_SIZE - 1)) {
 		end &= ~(CACHE_LINE_SIZE - 1);
+#ifdef CONFIG_PL310_ERRATA_588369
+		debug_writel(0x03);
+		sync_writel(end, L2X0_CLEAN_LINE_PA, 1);
+		sync_writel(end, L2X0_INV_LINE_PA, 1);
+		debug_writel(0x00);
+#else
 		sync_writel(end, L2X0_CLEAN_INV_LINE_PA, 1);
+#endif
 	}
 
 	for (addr = start; addr < end; addr += CACHE_LINE_SIZE)
@@ -88,8 +117,23 @@ static void l2x0_flush_range(unsigned long start, unsigned long end)
 	unsigned long addr;
 
 	start &= ~(CACHE_LINE_SIZE - 1);
+#ifdef CONFIG_PL310_ERRATA_588369
+
+	/* Disbale Write-Back and Cache Linefill */
+	debug_writel(0x03);
+
+	/* Clean by PA followed by Invalidate by PA */
+	for (addr = start; addr < end; addr += CACHE_LINE_SIZE) {
+		sync_writel(addr, L2X0_CLEAN_LINE_PA, 1);
+		sync_writel(addr, L2X0_INV_LINE_PA, 1);
+	}
+
+	/* Enable Write-Back and Cache Linefill */
+	debug_writel(0x00);
+#else
 	for (addr = start; addr < end; addr += CACHE_LINE_SIZE)
 		sync_writel(addr, L2X0_CLEAN_INV_LINE_PA, 1);
+#endif
 	cache_sync();
 }
 
-- 
1.6.0.4

[PATCH 3/4 v2] ARM: L2 : Errata 588369: Clean & Invalidate do not invalidate clean lines

[Russell King - ARM Linux]

On Thu, Dec 17, 2009 at 06:30:32PM +0530, Santosh Shilimkar wrote:
> +config PL310_ERRATA_588369
> +	bool "Clean & Invalidate maintenance operations do not invalidate clean lines"
> +	depends on CACHE_L2X0 && ARCH_OMAP4
> +	default n

default n is the default anyway, so its redundant to specify it.

> +#ifdef CONFIG_PL310_ERRATA_588369
> +static void debug_writel(unsigned long val)
> +{
> +	/*
> +	 * Texas Instrument secure monitor api to modify the PL310
> +	 * Debug Control Register. R0 = val
> +	 */
> +	__asm__ __volatile__(
> +	"stmfd r13!, {r4-r8}\n"
> +	"ldr r12, =0x100\n"
> +	"dsb\n"
> +	"smc\n"
> +	"ldmfd r13!, {r4-r8}");

Just tell the compiler that r4 to r8 are clobbered - then it'll save and
restore them itself.  Also, you can't guarantee that r0 will contain the
value unless you explicitly pass it in.  IOW:

	register unsigned long r0 asm("r0") = val;

	asm volatile(
	__asmeq(%0, r0)
	"..."
	: : "r" (r0)
	: "r4", "r5", "r6", "r7", "r8");

The use of asmeq will also ensure that '%0' is indeed r0 - some gcc
versions are buggy.

As I've said before, your patch is fine for the current version, but
not for the other cache-l2x0 changes.

[PATCH 3/4 v2] ARM: L2 : Errata 588369: Clean & Invalidate do not invalidate clean lines

[Shilimkar, Santosh]

Thanks Russell,
> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> Sent: Thursday, December 17, 2009 7:05 PM
> To: Shilimkar, Santosh
> Cc: linux-arm-kernel at lists.infradead.org; tony at atomide.com; linux-omap at vger.kernel.org; Woodruff,
> Richard; Catalin Marinas
> Subject: Re: [PATCH 3/4 v2] ARM: L2 : Errata 588369: Clean & Invalidate do not invalidate clean lines
> 
> On Thu, Dec 17, 2009 at 06:30:32PM +0530, Santosh Shilimkar wrote:
> > +config PL310_ERRATA_588369
> > +	bool "Clean & Invalidate maintenance operations do not invalidate clean lines"
> > +	depends on CACHE_L2X0 && ARCH_OMAP4
> > +	default n
> 
> default n is the default anyway, so its redundant to specify it.
ok
> 
> > +#ifdef CONFIG_PL310_ERRATA_588369
> > +static void debug_writel(unsigned long val)
> > +{
> > +	/*
> > +	 * Texas Instrument secure monitor api to modify the PL310
> > +	 * Debug Control Register. R0 = val
> > +	 */
> > +	__asm__ __volatile__(
> > +	"stmfd r13!, {r4-r8}\n"
> > +	"ldr r12, =0x100\n"
> > +	"dsb\n"
> > +	"smc\n"
> > +	"ldmfd r13!, {r4-r8}");
> 
> Just tell the compiler that r4 to r8 are clobbered - then it'll save and
> restore them itself.  Also, you can't guarantee that r0 will contain the
> value unless you explicitly pass it in.  IOW:
Perfect. I was looking for something like this.
> 	register unsigned long r0 asm("r0") = val;
> 
> 	asm volatile(
> 	__asmeq(%0, r0)
> 	"..."
> 	: : "r" (r0)
> 	: "r4", "r5", "r6", "r7", "r8");
> 
> The use of asmeq will also ensure that '%0' is indeed r0 - some gcc
> versions are buggy.
OK
> As I've said before, your patch is fine for the current version, but
> not for the other cache-l2x0 changes.
I can rebase this on top of your changes. I have that version also functional.
Just need to rebase on top of your 2 patched.

Regards,
Santosh