Re: [PATCH 3/5] ARM: l2x0: Errata fix for flush by Way operation can cause data corruption

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 14, 2011 at 1:33 PM, Andrei Warkentin <andreiw@xxxxxxxxxxxx> wrote:
>
> Fair enough, but you're doing it right now :-). I believe the smarter
> approach would be to start abstracting all accesses to secure-only
> resources (like the DCR reg). This would be your "hypervisor"
> interface. Then provide an implementation for your TI secure monitor.
> Obviously over time :).
>

Santosh,

Maybe this can influence you in some ways. I have this old patch
sitting around, although it does #ifdef around the TI stuff, simply
because I wasn't really interested in moving it out.
I group the errata by revs so it's simpler to see what you need and
what you don't. Obviously there aren't that many, but it does provide
a pattern to follow...

Thanks,
A
From be41c69f48d5886c76148c2ad378dae78e590534 Mon Sep 17 00:00:00 2001
From: Andrei Warkentin <andreiw@xxxxxxxxxxxx>
Date: Mon, 14 Feb 2011 15:34:06 -0600
Subject: [PATCH] ARM PL310: Cleanup errata handling for cache controller.

Adds a revision option for PL310 cache controller. All errata
now depend on the revision selected. Picking a particular revision
results in selecting all required errata. CACHE_PL310_REV_UNKNOWN
is used when manual control is desired.

Signed-off-by: Andrei Warkentin <andreiw@xxxxxxxxxxxx>

Change-Id: I8804af5d7a476737ce259b5d6a2a132c50082d66
---
 arch/arm/Kconfig                        |   36 ++++++++++++-------
 arch/arm/configs/omap_4430sdp_defconfig |    5 ++-
 arch/arm/mm/Kconfig                     |   52 +++++++++++++++++++++++++++
 arch/arm/mm/cache-l2x0.c                |   58 +++++++++++++++++++++++-------
 4 files changed, 122 insertions(+), 29 deletions(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index cd2c427..20af15f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1014,6 +1014,28 @@ if !MMU
 source "arch/arm/Kconfig-nommu"
 endif
 
+config ARM_TRUSTZONE_UNSECURE
+       bool "TrustZone: Support for running in an unsecure domain"
+       depends on CPU_V6 || CPU_V7
+       help
+         This enables TZ-specific behaviors with respect to modifying state
+	 that is secure-only in a TrustZone system. You will need to select
+	 a specific Hypervisor/Secure Monitor.
+
+choice
+       prompt "TrustZone Hypervisor/Secure Monitor"
+       depends on ARM_TRUSTZONE_UNSECURE
+       default ARM_TRUSTZONE_UNSECURE_UNKNOWN
+       help
+         Pick the TrustZone Hypervisor/Secure Monitor, under which Linux
+	 will run in an unsecure domain.
+config ARM_TRUSTZONE_UNSECURE_UNKNOWN
+       bool "Unknown"
+config ARM_TRUSTZONE_UNSECURE_TI
+       bool "Texas Instruments Secure Monitor API"
+       depends on ARCH_OMAP4
+endchoice
+
 config ARM_ERRATA_411920
 	bool "ARM errata: Invalidation of the Instruction Cache operation can fail"
 	depends on CPU_V6 && !SMP
@@ -1090,20 +1112,6 @@ config ARM_ERRATA_742231
 	  register of the Cortex-A9 which reduces the linefill issuing
 	  capabilities of the processor.
 
-config PL310_ERRATA_588369
-	bool "Clean & Invalidate maintenance operations do not invalidate clean lines"
-	depends on CACHE_L2X0 && ARCH_OMAP4
-	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.
-
 config ARM_ERRATA_720789
 	bool "ARM errata: TLBIASIDIS and TLBIMVAIS operations can broadcast a faulty ASID"
 	depends on CPU_V7 && SMP
diff --git a/arch/arm/configs/omap_4430sdp_defconfig b/arch/arm/configs/omap_4430sdp_defconfig
index 14c1e18..ac4091c 100644
--- a/arch/arm/configs/omap_4430sdp_defconfig
+++ b/arch/arm/configs/omap_4430sdp_defconfig
@@ -13,6 +13,8 @@ CONFIG_MODULE_SRCVERSION_ALL=y
 # CONFIG_BLK_DEV_BSG is not set
 CONFIG_ARCH_OMAP=y
 CONFIG_ARCH_OMAP4=y
+CONFIG_ARM_TRUSTZONE_UNSECURE=y
+CONFIG_ARM_TRUSTZONE_UNSECURE_TI=y
 # CONFIG_ARCH_OMAP2PLUS_TYPICAL is not set
 # CONFIG_ARCH_OMAP2 is not set
 # CONFIG_ARCH_OMAP3 is not set
@@ -21,7 +23,8 @@ CONFIG_OMAP_32K_TIMER=y
 CONFIG_OMAP_DM_TIMER=y
 CONFIG_MACH_OMAP_4430SDP=y
 # CONFIG_ARM_THUMB is not set
-CONFIG_PL310_ERRATA_588369=y
+CONFIG_CACHE_PL310_REV_UNKNOWN=y
+CONFIG_CACHE_PL310_ERRATA_588369=y
 CONFIG_SMP=y
 CONFIG_NR_CPUS=2
 # CONFIG_LOCAL_TIMERS is not set
diff --git a/arch/arm/mm/Kconfig b/arch/arm/mm/Kconfig
index cc6f9d6..057fa76 100644
--- a/arch/arm/mm/Kconfig
+++ b/arch/arm/mm/Kconfig
@@ -786,6 +786,58 @@ config CACHE_PL310
 	help
 	  This option enables support for the PL310 cache controller.
 
+choice
+	prompt "PL310 revision"
+	depends on CACHE_PL310
+	default CACHE_PL310_REV_UNKNOWN
+	help
+	  This option selects the specific revision of the PL310 cache
+	  controller used. Picking a choice selects specific workarounds
+	  for that chip revision.
+config CACHE_PL310_REV_R0P0
+       bool "r0p0"
+       select CACHE_PL310_ERRATA_588369
+config CACHE_PL310_REV_R1P0
+       bool "r1p0"
+       select CACHE_PL310_ERRATA_588369
+config CACHE_PL310_REV_R2P0
+       bool "r2p0"
+       select CACHE_PL310_ERRATA_727915
+config CACHE_PL310_REV_R3P0
+       bool "r3p0"
+       select CACHE_PL310_ERRATA_727915
+config CACHE_PL310_REV_R3P1
+       bool "r3p1"
+config CACHE_PL310_REV_R3P1_50REL0
+       bool "r3p1-50rel0"
+config CACHE_PL310_REV_UNKNOWN
+       bool "Unknown revision"
+endchoice
+
+config CACHE_PL310_ERRATA_727915
+	bool "PL310 727915: Background Clean and Invalidate by Way operation can cause data corruption"
+	depends on CACHE_PL310_REV_UNKNOWN || CACHE_PL310_REV_R2P0 || CACHE_PL310_REV_R3P0
+	help
+	  PL310 implements the Clean & Invalidate by Way L2 cache maintenance
+	  operation (offset 0x7FC). This operation runs in background so that
+	  PL310 can handle normal accesses while it is in progress. Under very
+	  rare circumstances, due to this erratum, write data can be lost when
+	  PL310 treats a cacheable write transaction during a Clean & Invalidate
+	  by Way operation.
+
+config CACHE_PL310_ERRATA_588369
+	bool "PL310 588369: Clean & Invalidate maintenance operations do not invalidate clean lines"
+	depends on CACHE_PL310_REV_UNKNOWN || CACHE_PL310_REV_R0P0 || CACHE_PL310_REV_R1P0
+	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.
+
 config CACHE_TAUROS2
 	bool "Enable the Tauros2 L2 cache controller"
 	depends on (ARCH_DOVE || ARCH_MMP)
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 9abfa5d..f4237c5 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -85,8 +85,13 @@ static inline void l2x0_inv_line(unsigned long addr)
 	writel_relaxed(addr, base + L2X0_INV_LINE_PA);
 }
 
-#ifdef CONFIG_PL310_ERRATA_588369
-static void debug_writel(unsigned long val)
+#ifdef CONFIG_ARM_TRUSTZONE_UNSECURE
+/*
+ * PL310 Debug Control Register is accessible only from
+ * the secure domain.
+ */
+#ifdef CONFIG_ARM_TRUSTZONE_UNSECURE_TI
+static inline void debug_writel(unsigned long val)
 {
 	extern void omap_smc1(u32 fn, u32 arg);
 
@@ -96,31 +101,33 @@ static void debug_writel(unsigned long val)
 	 */
 	omap_smc1(0x100, val);
 }
+#else
+#error Unsupported TrustZone Hypervisor/Secure Monitor
+#endif
+#else
+/*
+ * No TrustZone or we're in the secure domain.
+ */
+static inline void debug_writel(unsigned long val)
+{
+	writel(val, l2x0_base + L2X0_DEBUG_CTRL);
+}
+#endif
 
 static inline void l2x0_flush_line(unsigned long addr)
 {
 	void __iomem *base = l2x0_base;
 
-	/* Clean by PA followed by Invalidate by PA */
+#ifdef CONFIG_CACHE_PL310_ERRATA_588369
 	cache_wait(base + L2X0_CLEAN_LINE_PA, 1);
 	writel_relaxed(addr, base + L2X0_CLEAN_LINE_PA);
 	cache_wait(base + L2X0_INV_LINE_PA, 1);
 	writel_relaxed(addr, base + L2X0_INV_LINE_PA);
-}
 #else
-
-/* Optimised out for non-errata case */
-static inline void debug_writel(unsigned long val)
-{
-}
-
-static inline void l2x0_flush_line(unsigned long addr)
-{
-	void __iomem *base = l2x0_base;
 	cache_wait(base + L2X0_CLEAN_INV_LINE_PA, 1);
 	writel_relaxed(addr, base + L2X0_CLEAN_INV_LINE_PA);
-}
 #endif
+}
 
 static void l2x0_cache_sync(void)
 {
@@ -149,9 +156,20 @@ static inline void l2x0_flush_all(void)
 
 	/* flush all ways */
 	_l2x0_lock(&l2x0_lock, flags);
+#ifdef CONFIG_CACHE_PL310_ERRATA_727915
+	debug_writel(0x03);
+#endif
+
+	/*
+	  TODO: Doesn't errata 588369 imply that this
+	  has to be replaced by a CLEAN_WAY and INV_WAY as well?
+	*/
 	writel(0xff, l2x0_base + L2X0_CLEAN_INV_WAY);
 	cache_wait_always(l2x0_base + L2X0_CLEAN_INV_WAY, 0xff);
 	cache_sync();
+#ifdef CONFIG_CACHE_PL310_ERRATA_727915
+	debug_writel(0x0);
+#endif
 	_l2x0_unlock(&l2x0_lock, flags);
 }
 
@@ -163,17 +181,25 @@ static void l2x0_inv_range(unsigned long start, unsigned long end)
 	_l2x0_lock(&l2x0_lock, flags);
 	if (start & (CACHE_LINE_SIZE - 1)) {
 		start &= ~(CACHE_LINE_SIZE - 1);
+#ifdef CONFIG_CACHE_PL310_ERRATA_588369
 		debug_writel(0x03);
+#endif
 		l2x0_flush_line(start);
+#ifdef CONFIG_CACHE_PL310_ERRATA_588369
 		debug_writel(0x00);
+#endif
 		start += CACHE_LINE_SIZE;
 	}
 
 	if (end & (CACHE_LINE_SIZE - 1)) {
 		end &= ~(CACHE_LINE_SIZE - 1);
+#ifdef CONFIG_CACHE_PL310_ERRATA_588369
 		debug_writel(0x03);
+#endif
 		l2x0_flush_line(end);
+#ifdef CONFIG_CACHE_PL310_ERRATA_588369
 		debug_writel(0x00);
+#endif
 	}
 
 	while (start < end) {
@@ -229,12 +255,16 @@ static void l2x0_flush_range(unsigned long start, unsigned long end)
 	while (start < end) {
 		unsigned long blk_end = block_end(start, end);
 
+#ifdef CONFIG_CACHE_PL310_ERRATA_588369
 		debug_writel(0x03);
+#endif
 		while (start < blk_end) {
 			l2x0_flush_line(start);
 			start += CACHE_LINE_SIZE;
 		}
+#ifdef CONFIG_CACHE_PL310_ERRATA_588369
 		debug_writel(0x00);
+#endif
 
 		if (blk_end < end) {
 			_l2x0_unlock(&l2x0_lock, flags);
-- 
1.7.0.4

[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux