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

Andrei Warkentin
2011-02-14T21:08:59Z

[-- Attachment #1: Type: text/plain, Size: 756 bytes --]

On Mon, Feb 14, 2011 at 1:33 PM, Andrei Warkentin <andreiw@motorola.com> 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

[-- Attachment #2: 0001-ARM-PL310-Cleanup-errata-handling-for-cache-controll.patch --]
[-- Type: text/x-diff, Size: 9799 bytes --]

From be41c69f48d5886c76148c2ad378dae78e590534 Mon Sep 17 00:00:00 2001
From: Andrei Warkentin <andreiw@motorola.com>
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@motorola.com>

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