arm/mm: L2CC shared mutex with ARM TZ
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
From: Etienne Carriere <[email protected]>
Secure code in TrustZone space may need to perform L2 cache maintenance operations. A shared mutex is required to synchronize linux l2cc maintenance and TZ l2cc maintenance.
The TZ mutex is an "arch_spinlock": a 32bit DDR cell (ARMv7-A mutex). Linux L2 cache driver must lock TZ mutex if enabled.
---arch/arm/include/asm/outercache.h | 9 ++++arch/arm/mm/cache-l2x0.c | 87 +++++++++++++++++++++++++++++----------
2 files changed, 74 insertions(+), 22 deletions(-)
arch/arm/include/asm/outercache.h +0 -0
@@ -35,6 +35,7 @@ struct outer_cache_fns {
#endif
void (*set_debug)(unsigned long);
void (*resume)(void);+ bool (*tz_mutex)(unsigned long);
};
#ifdef CONFIG_OUTER_CACHE
@@ -81,6 +82,13 @@ static inline void outer_resume(void)
outer_cache.resume(); }
+static inline bool outer_tz_mutex(unsigned long addr)
+{
+ if (outer_cache.tz_mutex)
+ return outer_cache.tz_mutex(addr);
+ return false;
+}
+
#else
static inline void outer_inv_range(phys_addr_t start, phys_addr_t end)
@@ -92,6 +100,7 @@ static inline void outer_flush_range(phys_addr_t start, phys_addr_t end) static inline void outer_flush_all(void) { }
static inline void outer_inv_all(void) { }
static inline void outer_disable(void) { }
+static inline bool outer_tz_mutex(unsigned long addr) { return false; }
#endif
arch/arm/mm/cache-l2x0.c +0 -0
@@ -41,6 +41,26 @@ struct l2x0_of_data {void (*resume)(void );
};
+/*
+ * arch_spinlock (single 32bit DDR mutex cell) pointer to synchronise
+ * L2CC maintenance between linux world and secure world (ARM TZ).
+ */
+arch_spinlock_t *l2x0_tz_mutex;
+
+#define l2x0_spin_lock_irqsave(flags) \
+ do { \
+ raw_spin_lock_irqsave(&l2x0_lock, flags); \
+ if (l2x0_tz_mutex) \
+ arch_spin_lock(l2x0_tz_mutex); \
+ } while (0)
+
+#define l2x0_spin_unlock_irqrestore(flags) \
+ do { \
+ if (l2x0_tz_mutex) \
+ arch_spin_unlock(l2x0_tz_mutex); \
+ raw_spin_unlock_irqrestore(&l2x0_lock, flags); \
+ } while (0)
+
static inline void cache_wait_way(void __iomem *reg, unsigned long mask)
{
/* wait for cache operation by line or way to complete */
@@ -126,9 +146,9 @@ static void l2x0_cache_sync(void)
{
unsigned long flags;
- raw_spin_lock_irqsave(&l2x0_lock, flags);
+ l2x0_spin_lock_irqsave(flags);
cache_sync();- raw_spin_unlock_irqrestore(&l2x0_lock, flags);
+ l2x0_spin_unlock_irqrestore(flags);
}
static void __l2x0_flush_all(void)
@@ -145,9 +165,9 @@ static void l2x0_flush_all(void)
unsigned long flags;
/* clean all ways */
- raw_spin_lock_irqsave(&l2x0_lock, flags);
+ l2x0_spin_lock_irqsave(flags);
__l2x0_flush_all();- raw_spin_unlock_irqrestore(&l2x0_lock, flags);
+ l2x0_spin_unlock_irqrestore(flags);
}
static void l2x0_clean_all(void)
@@ -155,11 +175,11 @@ static void l2x0_clean_all(void)
unsigned long flags;
/* clean all ways */
- raw_spin_lock_irqsave(&l2x0_lock, flags);
+ l2x0_spin_lock_irqsave(flags);
writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_CLEAN_WAY);
cache_wait_way(l2x0_base + L2X0_CLEAN_WAY, l2x0_way_mask);
cache_sync();- raw_spin_unlock_irqrestore(&l2x0_lock, flags);
+ l2x0_spin_unlock_irqrestore(flags);
}
static void l2x0_inv_all(void)
@@ -167,13 +187,13 @@ static void l2x0_inv_all(void)
unsigned long flags;
/* invalidate all ways */
- raw_spin_lock_irqsave(&l2x0_lock, flags);
+ l2x0_spin_lock_irqsave(flags);
/* Invalidating when L2 is enabled is a nono */
BUG_ON(readl(l2x0_base + L2X0_CTRL) & 1);
writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_INV_WAY);
cache_wait_way(l2x0_base + L2X0_INV_WAY, l2x0_way_mask);
cache_sync();- raw_spin_unlock_irqrestore(&l2x0_lock, flags);
+ l2x0_spin_unlock_irqrestore(flags);
}
static void l2x0_inv_range(unsigned long start, unsigned long end)
@@ -181,7 +201,7 @@ static void l2x0_inv_range(unsigned long start, unsigned long end)
void __iomem *base = l2x0_base;
unsigned long flags;
- raw_spin_lock_irqsave(&l2x0_lock, flags);
+ l2x0_spin_lock_irqsave(flags);
if (start & (CACHE_LINE_SIZE - 1)) {
start &= ~(CACHE_LINE_SIZE - 1);
debug_writel(0x03);
@@ -206,13 +226,13 @@ static void l2x0_inv_range(unsigned long start, unsigned long end)
}
if (blk_end < end) {
- raw_spin_unlock_irqrestore(&l2x0_lock, flags);
- raw_spin_lock_irqsave(&l2x0_lock, flags);
+ l2x0_spin_unlock_irqrestore(flags);
+ l2x0_spin_lock_irqsave(flags);
}
}
cache_wait(base + L2X0_INV_LINE_PA, 1);
cache_sync();- raw_spin_unlock_irqrestore(&l2x0_lock, flags);
+ l2x0_spin_unlock_irqrestore(flags);
}
static void l2x0_clean_range(unsigned long start, unsigned long end)
@@ -225,7 +245,7 @@ static void l2x0_clean_range(unsigned long start, unsigned long end)
return;
}
- raw_spin_lock_irqsave(&l2x0_lock, flags);
+ l2x0_spin_lock_irqsave(flags);
start &= ~(CACHE_LINE_SIZE - 1);
while (start < end) {
unsigned long blk_end = start + min(end - start, 4096UL);
@@ -236,13 +256,13 @@ static void l2x0_clean_range(unsigned long start, unsigned long end)
}
if (blk_end < end) {
- raw_spin_unlock_irqrestore(&l2x0_lock, flags);
- raw_spin_lock_irqsave(&l2x0_lock, flags);
+ l2x0_spin_unlock_irqrestore(flags);
+ l2x0_spin_lock_irqsave(flags);
}
}
cache_wait(base + L2X0_CLEAN_LINE_PA, 1);
cache_sync();- raw_spin_unlock_irqrestore(&l2x0_lock, flags);
+ l2x0_spin_unlock_irqrestore(flags);
}
static void l2x0_flush_range(unsigned long start, unsigned long end)
@@ -255,7 +275,7 @@ static void l2x0_flush_range(unsigned long start, unsigned long end)
return;
}
- raw_spin_lock_irqsave(&l2x0_lock, flags);
+ l2x0_spin_lock_irqsave(flags);
start &= ~(CACHE_LINE_SIZE - 1);
while (start < end) {
unsigned long blk_end = start + min(end - start, 4096UL);
@@ -268,24 +288,24 @@ static void l2x0_flush_range(unsigned long start, unsigned long end)
debug_writel(0x00);
if (blk_end < end) {
- raw_spin_unlock_irqrestore(&l2x0_lock, flags);
- raw_spin_lock_irqsave(&l2x0_lock, flags);
+ l2x0_spin_unlock_irqrestore(flags);
+ l2x0_spin_lock_irqsave(flags);
}
}
cache_wait(base + L2X0_CLEAN_INV_LINE_PA, 1);
cache_sync();- raw_spin_unlock_irqrestore(&l2x0_lock, flags);
+ l2x0_spin_unlock_irqrestore(flags);
}
static void l2x0_disable(void)
{
unsigned long flags;
- raw_spin_lock_irqsave(&l2x0_lock, flags);
+ l2x0_spin_lock_irqsave(flags);
__l2x0_flush_all();
writel_relaxed(0, l2x0_base + L2X0_CTRL);
dsb();- raw_spin_unlock_irqrestore(&l2x0_lock, flags);
+ l2x0_spin_unlock_irqrestore(flags);
}
static void l2x0_unlock(u32 cache_id)
@@ -307,6 +327,28 @@ static void l2x0_unlock(u32 cache_id)
} }
+/* Enable/disable external mutex shared with TZ code */ +static bool l2x0_tz_mutex_cfg(unsigned long addr) +{
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&l2x0_lock, flags);
+
+ if (addr && l2x0_tz_mutex && (addr != (uint)l2x0_tz_mutex)) {
+ raw_spin_unlock_irqrestore(&l2x0_lock, flags);
+ pr_err("%s: a TZ mutex is already enabled\n", __func__);
+ return false;
+ }
+
+ l2x0_tz_mutex = (arch_spinlock_t *)addr;
+ /* insure mutex ptr is updated before lock is released */
+ smp_wmb();
+
+ raw_spin_unlock_irqrestore(&l2x0_lock, flags);
+ pr_debug("\n%s: %sable TZ mutex\n\n", __func__, (addr) ? "en" : "dis");
+ return true;
+}
+
void __init l2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask)
{
u32 aux;
@@ -380,6 +422,7 @@ void __init l2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask)
outer_cache.inv_all = l2x0_inv_all;
outer_cache.disable = l2x0_disable;
outer_cache.set_debug = l2x0_set_debug;
+ outer_cache.tz_mutex = l2x0_tz_mutex_cfg;
printk(KERN_INFO "%s cache controller enabled\n", type);
printk(KERN_INFO "l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL 0x%08x, Cache size: %d B\n",--
1.7.11.3
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121113/b0f6414c/attachment-0001.html>
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
Quoted 9 lines · Etienne CARRIERE
From: Etienne Carriere <[email protected]>
Secure code in TrustZone space may need to perform L2 cache maintenance operations. A shared mutex is required to synchronize linux l2cc maintenance and TZ l2cc maintenance.
The TZ mutex is an "
arch_spinlock": a 32bit DDR cell (ARMv7-A mutex). Linux L2 cache driver must lock TZ mutex if enabled.
...
Review
Quoted 6 lines
(quoted diff) +6
-0+#define l2x0_spin_lock_irqsave(flags) \ + do { \ + raw_spin_lock_irqsave(&l2x0_lock, flags); \ + if (l2x0_tz_mutex) \ + arch_spin_lock(l2x0_tz_mutex); \ + } while (0)
Right, so, what this tells me is that the implementation of the spinlock in the secure software is potentially the same as the kernel's spinlock. The kernel's spinlock implementation is GPL'd. If the secure side spinlock implementation is a copy of the kernel's spinlock implementation, then that implementation is also GPL'd, which, as it is linked with the rest of the secure software, either the whole of the secure software suite is GPL'd _or_ it is in violation of the GPL.
I think someone has some explaining to do.
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
Quoted 10 lines
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Tuesday, November 13, 2012 6:07 PMRight, so, what this tells me is that the implementation of the spinlock in the secure software is potentially the same as the kernel's spinlock. The kernel's spinlock implementation is GPL'd. If the secure side spinlock implementation is a copy of the kernel's spinlock implementation, then that implementation is also GPL'd (...).
I think someone has some explaining to do.
It does.
The TZ implementation of the shared mutex is the ARM native mutex instructions set LDREX/STREX, protected by a native ARM IRQ disabling. I believe this is native ARMv7 natural handling and not subject to GPLization.
This is why the mutex added for TZ shared of L2 cache maintenance is a "arch_spinlock". It assumes linux handles the spinlock as a native ARM native mutex DDR cell, handled from the ARMv7-A LDREX/STREX. I think this is the weakest part of the driver since they is no insurance that linux pristine arch_spinlock will not evolve into something else (i.e features in raw_spinclock).
etienne
-----Original Message-----
To: Etienne CARRIERE
Cc: linux-arm-kernel at lists.infradead.org; Will Deacon; Marc Zyngier; Catalin Marinas; Linus Walleij (linus.walleij at linaro.org); Rabin VINCENT; Srinidhi KASAGAR
Subject: Re: [PATCH 1/2] arm/mm: L2CC shared mutex with ARM TZ
Quoted 15 lines · Etienne CARRIERE
From: Etienne Carriere <[email protected]>
Secure code in TrustZone space may need to perform L2 cache maintenance operations. A shared mutex is required to synchronize linux l2cc maintenance and TZ l2cc maintenance.
The TZ mutex is an "
arch_spinlock": a 32bit DDR cell (ARMv7-A mutex). Linux L2 cache driver must lock TZ mutex if enabled.
...(quoted diff) +6
-0+#define l2x0_spin_lock_irqsave(flags) \ + do { \ + raw_spin_lock_irqsave(&l2x0_lock, flags); \ + if (l2x0_tz_mutex) \ + arch_spin_lock(l2x0_tz_mutex); \ + } while (0)
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
Quoted 2 lines · Etienne CARRIERE ST
This is why the mutex added for TZ shared of L2 cache maintenance is a "
arch_spinlock". It assumes linux handles the spinlock as a native ARM native mutex DDR cell, handled from the ARMv7-A LDREX/STREX. I think this is the weakest part of the driver since they is no insurance that linux pristinearch_spinlockwill not evolve into something else (i.e features inraw_spinclock).
How does this work with our ticket lock approach? Can you swizzle your locking implementation by inspecting the kernel version of the code running on the non-secure side? (I sincerely hope not...)
Will
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
Sorry, I don't catch "swizzle your implementation" ?
You are referring to the raw_spinlock LOCKBREAK stuf ?
I will look deeper into linux code to check is some deadlock could appear.
The shared mutex arch spinlock is locked after raw_spinlick locking and released before l2x0 native raw_spinlock releasing. It is intended to leave kernel preemption stuff inside the linux world.
etienne
-----Original Message-----
From: Will Deacon [mailto:will.deacon at arm.com]
Sent: Wednesday, November 14, 2012 10:51 AM
To: Etienne CARRIERE ST
Cc: Russell King - ARM Linux; linux-arm-kernel at lists.infradead.org; Marc Zyngier; Catalin Marinas; Linus Walleij; Rabin VINCENT; Srinidhi KASAGAR
Subject: Re: [PATCH 1/2] arm/mm: L2CC shared mutex with ARM TZ
Quoted 2 lines · Etienne CARRIERE ST
This is why the mutex added for TZ shared of L2 cache maintenance is a "
arch_spinlock". It assumes linux handles the spinlock as a native ARM native mutex DDR cell, handled from the ARMv7-A LDREX/STREX. I think this is the weakest part of the driver since they is no insurance that linux pristinearch_spinlockwill not evolve into something else (i.e features inraw_spinclock).
How does this work with our ticket lock approach? Can you swizzle your locking implementation by inspecting the kernel version of the code running on the non-secure side? (I sincerely hope not...)
Will
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
In reply to Etienne CARRIERE STQuoted 9 lines · Etienne CARRIERE ST
Quoted 9 lines
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
Sent: Tuesday, November 13, 2012 6:07 PMRight, so, what this tells me is that the implementation of the spinlock in the secure software is potentially the same as the kernel's spinlock. The kernel's spinlock implementation is GPL'd. If the secure side spinlock implementation is a copy of the kernel's spinlock implementation, then that implementation is also GPL'd (...).
Quoted 8 lines
Quoted 1 line
I think someone has some explaining to do.
It does.
The TZ implementation of the shared mutex is the ARM native mutex instructions set LDREX/STREX, protected by a native ARM IRQ disabling. I believe this is native ARMv7 natural handling and not subject to GPLization.
Maybe, maybe not. There's more to locking than just the instructions used - there's the values which are used to represent "locked" and "unlocked", and then there's the ticket locking implementation found in recent kernels which defines the value to have more meaning than just "locked" and "unlocked".
With ticket locking, you need to know the details of the implementation to be able to lock successfully, and that basically means copying the implementation.
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
Quoted 19 lines
Sent: Wednesday, November 14, 2012 11:21 AM
From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]Quoted 7 lines
Quoted 7 lines
Right, so, what this tells me is that the implementation of the spinlock in the secure software is potentially the same as the kernel's spinlock.
The kernel's spinlock implementation is GPL'd. If the secure side spinlock implementation is a copy of the kernel's spinlock implementation, then that implementation is also GPL'd (...).Quoted 8 lines
Quoted 1 line
I think someone has some explaining to do.
It does.
The TZ implementation of the shared mutex is the ARM native mutex instructions set LDREX/STREX, protected by a native ARM IRQ disabling. I believe this is native ARMv7 natural handling and not subject to GPLization.
Quoted 7 lines
Maybe, maybe not. There's more to locking than just the instructions used
- there's the values which are used to represent "locked" and "unlocked", and then there's the ticket locking implementation found in recent kernels which defines the value to have more meaning than just "locked" and "unlocked".With ticket locking, you need to know the details of the implementation to be able to lock successfully, and that basically means copying the implementation.
Thanks for notifying. I will look at the recent release to see if our architecture complies with Kernel code.
Our mutex implementation is based on 0/1 lock/unlock value in target mutex DDR cell.
etienne
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
Quoted 5 lines · Etienne CARRIERE ST
Thanks for notifying. I will look at the recent release to see if our architecture complies with Kernel code.
Our mutex implementation is based on 0/1 lock/unlock value in target mutex DDR cell.
Right, so we have another bug here: it is incompatible with the current ARM spinlock implementation - and using it on the same memory location as a spinlock will corrupt the spinlock.
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
Quoted 9 lines
Quoted 5 lines
Thanks for notifying. I will look at the recent release to see if our architecture complies with Kernel code.
Our mutex implementation is based on 0/1 lock/unlock value in target mutex DDR cell.
Right, so we have another bug here: it is incompatible with the current ARM spinlock implementation - and using it on the same memory location as a spinlock will corrupt the spinlock.
Right.
I see our architecture complies with kernels before v3.6-rc1.
Until then, it seems we can use our patch, but we must plan something else for the more recent kernels.
etienne
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
In reply to Etienne CARRIEREQuoted 9 lines · Etienne CARRIERE
From: Etienne Carriere <[email protected]>
Secure code in TrustZone space may need to perform L2 cache
maintenance operations. A shared mutex is required to synchronize
linux l2cc maintenance and TZ l2cc maintenance.
1) Why is this not a denial-of-service risk? (i.e., why can the Normal World not simply hold the mutex forever?)
2) The memory types on both sides have to be _exactly_ the same, not just "cached", otherwise unpredictable behaviour may occur when accessing the mutex. It is not obvious how this is ensured.
3) Many people seem to delegate L2 cache maintenence requests via SMC in these situations. Can you do that instead?
Cheers
---Dave
Quoted 499 lines
The TZ mutex is an "
arch_spinlock": a 32bit DDR cell (ARMv7-A mutex).Linux L2 cache driver must lock TZ mutex if enabled.
Signed-off-by Etienne Carriere <[email protected]>---
arch/arm/include/asm/outercache.h| 9 ++++
arch/arm/mm/cache-l2x0.c| 87 +++++++++++++++++++++++++++++----------2 files changed, 74 insertions(+), 22 deletions(-)arch/arm/include/asm/ renamed from arch/arm/include/asm/outercache.h +0
-0
outercache.hindex 53426c6..7aa5eac 100644
---
a/arch/arm/include/asm/outercache.h+++
b/arch/arm/include/asm/outercache.harch/arm/include/asm/ renamed from arch/arm/include/asm/outercache.h +1
-0@@ -35,6 +35,7 @@ struct outer_cache_fns { #endif void (*set_debug)(unsigned long); void (*resume)(void); + bool (*tz_mutex)(unsigned long);};
#ifdef
CONFIG_OUTER_CACHEarch/arm/include/asm/ renamed from arch/arm/include/asm/outercache.h +7
-0@@ -81,6 +82,13 @@ static inline void outer_resume(void) outer_cache.resume(); } +static inline bool outer_tz_mutex(unsigned long addr) +{ + if (outer_cache.tz_mutex) + return outer_cache.tz_mutex(addr); + return false; +} +#else
static inline void
outer_inv_range(phys_addr_t start, phys_addr_t end)arch/arm/include/asm/ renamed from arch/arm/include/asm/outercache.h +0
-0@@ -92,6 +100,7 @@ static inline void outer_flush_range(phys_addr_t start, phys_addr_t end) static inline void outer_flush_all(void) { } static inline void outer_inv_all(void) { }static inline void
outer_disable(void){ }arch/arm/include/asm/ renamed from arch/arm/include/asm/outercache.h +1
-0+static inline bool outer_tz_mutex(unsigned long addr) { return false; } #endifarch/arm/mm/cache-l2x0.c +0
-0index a53fd2a..eacdc74 100644
---
a/arch/arm/mm/cache-l2x0.c+++
b/arch/arm/mm/cache-l2x0.carch/arm/mm/cache-l2x0.c +8
-0@@ -41,6 +41,26 @@ struct l2x0_of_data {void (*resume)(void ); }; +/* +* arch_spinlock (single 32bit DDR mutex cell) pointer to synchronise +* L2CC maintenance between linux world and secure world (ARM TZ). +*/ +arch_spinlock_t *l2x0_tz_mutex; + +#define l2x0_spin_lock_irqsave (flags) \ + do{
\arch/arm/mm/cache-l2x0.c +7
-0+raw_spin_lock_irqsave (&l2x0_lock, flags); \ +if (l2x0_tz_mutex) \ +arch_spin_lock (l2x0_tz_mutex); \ + } while (0) + +#define l2x0_spin_unlock_irqrestore (flags) \ + do{
\arch/arm/mm/cache-l2x0.c +5
-0+if (l2x0_tz_mutex) \ +arch_spin_unlock (l2x0_tz_mutex); \ +raw_spin_unlock_irqrestore (&l2x0_lock, flags); \ + }while (0 ) +static inline void
cache_wait_way(void __iomem *reg, unsigned long mask){
/* wait for cache operation by line or way to complete */arch/arm/mm/cache-l2x0.c +2
-2@@ -126,9 +146,9 @@ static void l2x0_cache_sync(void) {unsigned long flags; -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags);cache_sync (); -raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
__l2x0_flush_all(void)arch/arm/mm/cache-l2x0.c +2
-2@@ -145,9 +165,9 @@ static void l2x0_flush_all(void)unsigned long flags;/* clean all ways */ -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags);__l2x0_flush_all (); -raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_clean_all(void)arch/arm/mm/cache-l2x0.c +1
-1@@ -155,11 +175,11 @@ static void l2x0_clean_all(void)unsigned long flags;/* clean all ways */ -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags);writel_relaxed (l2x0_way_mask, l2x0_base +L2X0_CLEAN_WAY );cache_wait_way (l2x0_base +L2X0_CLEAN_WAY , l2x0_way_mask);cache_sync();arch/arm/mm/cache-l2x0.c +1
-1-raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_inv_all(void)arch/arm/mm/cache-l2x0.c +1
-1@@ -167,13 +187,13 @@ static void l2x0_inv_all(void)unsigned long flags;/* invalidate all ways */ -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags);/* Invalidating when L2 is enabled is a nono */ BUG_ON (readl (l2x0_base +L2X0_CTRL ) &1 );writel_relaxed (l2x0_way_mask, l2x0_base +L2X0_INV_WAY );cache_wait_way(l2x0_base + L2X0_INV_WAY, l2x0_way_mask); cache_sync();arch/arm/mm/cache-l2x0.c +1
-1-raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_inv_range(unsigned long start, unsigned long end)arch/arm/mm/cache-l2x0.c +4
-4@@ -181,7 +201,7 @@ static void l2x0_inv_range(unsigned long start, unsigned long end)void __iomem *base = l2x0_base;unsigned long flags; -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags);if (start & (CACHE_LINE_SIZE -1 )) { start &= ~(CACHE_LINE_SIZE -1 );debug_writel (0x03 );@@ -206,13 +226,13 @@ static void l2x0_inv_range(unsigned long start, unsigned long end) }if (blk_end < end) { -raw_spin_unlock_irqrestore (& l2x0_lock, flags); -raw_spin_lock_irqsave (& l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags); +l2x0_spin_lock_irqsave (flags); } }cache_wait (base +L2X0_INV_LINE_PA ,1 );cache_sync (); -raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_clean_range(unsigned long start, unsigned long end)arch/arm/mm/cache-l2x0.c +1
-1@@ -225,7 +245,7 @@ static void l2x0_clean_range(unsigned long start, unsigned long end) return; } -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags); start &= ~(CACHE_LINE_SIZE -1 );while (start < end) { unsigned long blk_end = start + min(end - start,4096UL);
arch/arm/mm/cache-l2x0.c +3
-3@@ -236,13 +256,13 @@ static void l2x0_clean_range(unsigned long start, unsigned long end) }if (blk_end < end) { -raw_spin_unlock_irqrestore (& l2x0_lock, flags); -raw_spin_lock_irqsave (& l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags); +l2x0_spin_lock_irqsave (flags); } }cache_wait (base +L2X0_CLEAN_LINE_PA ,1 );cache_sync (); -raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_flush_range(unsigned long start, unsigned long end)arch/arm/mm/cache-l2x0.c +1
-1@@ -255,7 +275,7 @@ static void l2x0_flush_range(unsigned long start, unsigned long end) return; } -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags); start &= ~(CACHE_LINE_SIZE -1 );while (start < end) { unsigned long blk_end = start + min(end - start,4096UL);
arch/arm/mm/cache-l2x0.c +3
-3@@ -268,24 +288,24 @@ static void l2x0_flush_range(unsigned long start, unsigned long end) debug_writel(0x00);if (blk_end < end) { -raw_spin_unlock_irqrestore (& l2x0_lock, flags); -raw_spin_lock_irqsave (& l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags); +l2x0_spin_lock_irqsave (flags); } }cache_wait (base +L2X0_CLEAN_INV_LINE_PA ,1 );cache_sync (); -raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_disable(void){
unsigned long flags;arch/arm/mm/cache-l2x0.c +2
-2-raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags);__l2x0_flush_all ();writel_relaxed (0 , l2x0_base +L2X0_CTRL );dsb (); -raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_unlock(u32 cache_id)arch/arm/mm/cache-l2x0.c +10
-0@@ -307,6 +327,28 @@ static void l2x0_unlock(u32 cache_id) } } +/* Enable/disable external mutex shared with TZ code */ +static bool l2x0_tz_mutex_cfg (unsigned long addr) +{ +unsigned long flags; + +raw_spin_lock_irqsave (&l2x0_lock, flags); + +if (addr && l2x0_tz_mutex && (addr != (uint )l2x0_tz_mutex)) { +raw_spin_unlock_irqrestore (&l2x0_lock, flags); + pr_err("%s: a TZ mutex is already enabled\n",
__func__);arch/arm/mm/cache-l2x0.c +9
-0+return false; + } + + l2x0_tz_mutex = (arch_spinlock_t *)addr; +/* insure mutex ptr is updated before lock is released */ +smp_wmb (); + +raw_spin_unlock_irqrestore (&l2x0_lock, flags); + pr_debug("\n%s: %sable TZ mutex\n\n", __func__, (addr) ? "en" :"dis");
arch/arm/mm/cache-l2x0.c +3
-0+return true; +} +void
__initl2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask){
u32 aux;arch/arm/mm/cache-l2x0.c +1
-0@@ -380,6 +422,7 @@ void __init l2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask) outer_cache.inv_all = l2x0_inv_all; outer_cache.disable = l2x0_disable; outer_cache.set_debug = l2x0_set_debug; + outer_cache.tz_mutex = l2x0_tz_mutex_cfg;printk (KERN_INFO "%s cache controller enabled\n" , type); printk(KERN_INFO "l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL0x%08x, Cache size: %d B\n",
--
1.7.11.3
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
Quoted 6 lines
Date: Tue, 13 Nov 2012 17:17:45 +0000
From: dave.martin at linaro.org
To: etienne.carriere at st.com
Subject: Re: [PATCH 1/2] arm/mm: L2CC shared mutex with ARM TZ
CC: rabin.vincent at stericsson.com; linux at arm.linux.org.uk; srinidhi.kasagar at stericsson.com; Marc.Zyngier at arm.com; Catalin.Marinas at arm.com; linus.walleij at linaro.org; Will.Deacon at arm.com; linux-arm-kernel at lists.infradead.org
Quoted 11 lines
Quoted 9 lines · Etienne CARRIERE
From: Etienne Carriere <[email protected]>
Secure code in TrustZone space may need to perform L2 cache
maintenance operations. A shared mutex is required to synchronize
linux l2cc maintenance and TZ l2cc maintenance.
If you are using PL310 with thrustzone support then the L2 cache lines are secure bit tagged ; your design should be such that the secure (TZ) side only does operations on secure cache lines and non-secure side does operations only on non-secure cache lines. So each entity (TZ and nonTZ) if maintains their own cache and ensures integrity before switching over via monitor, this might not be needed. Abhi> 1) Why is this not a denial-of-service risk? (i.e., why can the Normal> World not simply hold the mutex forever?)
Quoted 507 lines
2) The memory types on both sides have to be _exactly_ the same, not just "cached", otherwise unpredictable behaviour may occur when accessing the mutex. It is not obvious how this is ensured.
3) Many people seem to delegate L2 cache maintenence requests via SMC in these situations. Can you do that instead?
Cheers
---DaveQuoted 495 lines
The TZ mutex is an "
arch_spinlock": a 32bit DDR cell (ARMv7-A mutex).Linux L2 cache driver must lock TZ mutex if enabled.
Signed-off-by Etienne Carriere <[email protected]>---
arch/arm/include/asm/outercache.h| 9 ++++
arch/arm/mm/cache-l2x0.c| 87 +++++++++++++++++++++++++++++----------2 files changed, 74 insertions(+), 22 deletions(-)arch/arm/include/asm/ renamed from arch/arm/include/asm/outercache.h +0
-0
outercache.hindex 53426c6..7aa5eac 100644
---
a/arch/arm/include/asm/outercache.h+++
b/arch/arm/include/asm/outercache.harch/arm/include/asm/ renamed from arch/arm/include/asm/outercache.h +1
-0@@ -35,6 +35,7 @@ struct outer_cache_fns { #endif void (*set_debug)(unsigned long); void (*resume)(void); + bool (*tz_mutex)(unsigned long);};
#ifdef
CONFIG_OUTER_CACHEarch/arm/include/asm/ renamed from arch/arm/include/asm/outercache.h +7
-0@@ -81,6 +82,13 @@ static inline void outer_resume(void) outer_cache.resume(); } +static inline bool outer_tz_mutex(unsigned long addr) +{ + if (outer_cache.tz_mutex) + return outer_cache.tz_mutex(addr); + return false; +} +#else
static inline void
outer_inv_range(phys_addr_t start, phys_addr_t end)arch/arm/include/asm/ renamed from arch/arm/include/asm/outercache.h +0
-0@@ -92,6 +100,7 @@ static inline void outer_flush_range(phys_addr_t start, phys_addr_t end) static inline void outer_flush_all(void) { } static inline void outer_inv_all(void) { }static inline void
outer_disable(void){ }arch/arm/include/asm/ renamed from arch/arm/include/asm/outercache.h +1
-0+static inline bool outer_tz_mutex(unsigned long addr) { return false; } #endifarch/arm/mm/cache-l2x0.c +0
-0index a53fd2a..eacdc74 100644
---
a/arch/arm/mm/cache-l2x0.c+++
b/arch/arm/mm/cache-l2x0.carch/arm/mm/cache-l2x0.c +8
-0@@ -41,6 +41,26 @@ struct l2x0_of_data {void (*resume)(void ); }; +/* +* arch_spinlock (single 32bit DDR mutex cell) pointer to synchronise +* L2CC maintenance between linux world and secure world (ARM TZ). +*/ +arch_spinlock_t *l2x0_tz_mutex; + +#define l2x0_spin_lock_irqsave (flags) \ + do{
\arch/arm/mm/cache-l2x0.c +7
-0+raw_spin_lock_irqsave (&l2x0_lock, flags); \ +if (l2x0_tz_mutex) \ +arch_spin_lock (l2x0_tz_mutex); \ + } while (0) + +#define l2x0_spin_unlock_irqrestore (flags) \ + do{
\arch/arm/mm/cache-l2x0.c +5
-0+if (l2x0_tz_mutex) \ +arch_spin_unlock (l2x0_tz_mutex); \ +raw_spin_unlock_irqrestore (&l2x0_lock, flags); \ + }while (0 ) +static inline void
cache_wait_way(void __iomem *reg, unsigned long mask){
/* wait for cache operation by line or way to complete */arch/arm/mm/cache-l2x0.c +2
-2@@ -126,9 +146,9 @@ static void l2x0_cache_sync(void) {unsigned long flags; -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags);cache_sync (); -raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
__l2x0_flush_all(void)arch/arm/mm/cache-l2x0.c +2
-2@@ -145,9 +165,9 @@ static void l2x0_flush_all(void)unsigned long flags;/* clean all ways */ -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags);__l2x0_flush_all (); -raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_clean_all(void)arch/arm/mm/cache-l2x0.c +1
-1@@ -155,11 +175,11 @@ static void l2x0_clean_all(void)unsigned long flags;/* clean all ways */ -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags);writel_relaxed (l2x0_way_mask, l2x0_base +L2X0_CLEAN_WAY );cache_wait_way (l2x0_base +L2X0_CLEAN_WAY , l2x0_way_mask);cache_sync();arch/arm/mm/cache-l2x0.c +1
-1-raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_inv_all(void)arch/arm/mm/cache-l2x0.c +1
-1@@ -167,13 +187,13 @@ static void l2x0_inv_all(void)unsigned long flags;/* invalidate all ways */ -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags);/* Invalidating when L2 is enabled is a nono */ BUG_ON (readl (l2x0_base +L2X0_CTRL ) &1 );writel_relaxed (l2x0_way_mask, l2x0_base +L2X0_INV_WAY );cache_wait_way(l2x0_base + L2X0_INV_WAY, l2x0_way_mask); cache_sync();arch/arm/mm/cache-l2x0.c +1
-1-raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_inv_range(unsigned long start, unsigned long end)arch/arm/mm/cache-l2x0.c +4
-4@@ -181,7 +201,7 @@ static void l2x0_inv_range(unsigned long start, unsigned long end)void __iomem *base = l2x0_base;unsigned long flags; -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags);if (start & (CACHE_LINE_SIZE -1 )) { start &= ~(CACHE_LINE_SIZE -1 );debug_writel (0x03 );@@ -206,13 +226,13 @@ static void l2x0_inv_range(unsigned long start, unsigned long end) }if (blk_end < end) { -raw_spin_unlock_irqrestore (& l2x0_lock, flags); -raw_spin_lock_irqsave (& l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags); +l2x0_spin_lock_irqsave (flags); } }cache_wait (base +L2X0_INV_LINE_PA ,1 );cache_sync (); -raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_clean_range(unsigned long start, unsigned long end)arch/arm/mm/cache-l2x0.c +1
-1@@ -225,7 +245,7 @@ static void l2x0_clean_range(unsigned long start, unsigned long end) return; } -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags); start &= ~(CACHE_LINE_SIZE -1 );while (start < end) { unsigned long blk_end = start + min(end - start,4096UL);
arch/arm/mm/cache-l2x0.c +3
-3@@ -236,13 +256,13 @@ static void l2x0_clean_range(unsigned long start, unsigned long end) }if (blk_end < end) { -raw_spin_unlock_irqrestore (& l2x0_lock, flags); -raw_spin_lock_irqsave (& l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags); +l2x0_spin_lock_irqsave (flags); } }cache_wait (base +L2X0_CLEAN_LINE_PA ,1 );cache_sync (); -raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_flush_range(unsigned long start, unsigned long end)arch/arm/mm/cache-l2x0.c +1
-1@@ -255,7 +275,7 @@ static void l2x0_flush_range(unsigned long start, unsigned long end) return; } -raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags); start &= ~(CACHE_LINE_SIZE -1 );while (start < end) { unsigned long blk_end = start + min(end - start,4096UL);
arch/arm/mm/cache-l2x0.c +3
-3@@ -268,24 +288,24 @@ static void l2x0_flush_range(unsigned long start, unsigned long end) debug_writel(0x00);if (blk_end < end) { -raw_spin_unlock_irqrestore (& l2x0_lock, flags); -raw_spin_lock_irqsave (& l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags); +l2x0_spin_lock_irqsave (flags); } }cache_wait (base +L2X0_CLEAN_INV_LINE_PA ,1 );cache_sync (); -raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_disable(void){
unsigned long flags;arch/arm/mm/cache-l2x0.c +2
-2-raw_spin_lock_irqsave (&l2x0_lock, flags); +l2x0_spin_lock_irqsave (flags);__l2x0_flush_all ();writel_relaxed (0 , l2x0_base +L2X0_CTRL );dsb (); -raw_spin_unlock_irqrestore (&l2x0_lock, flags); +l2x0_spin_unlock_irqrestore (flags);}
static void
l2x0_unlock(u32 cache_id)arch/arm/mm/cache-l2x0.c +10
-0@@ -307,6 +327,28 @@ static void l2x0_unlock(u32 cache_id) } } +/* Enable/disable external mutex shared with TZ code */ +static bool l2x0_tz_mutex_cfg (unsigned long addr) +{ +unsigned long flags; + +raw_spin_lock_irqsave (&l2x0_lock, flags); + +if (addr && l2x0_tz_mutex && (addr != (uint )l2x0_tz_mutex)) { +raw_spin_unlock_irqrestore (&l2x0_lock, flags); + pr_err("%s: a TZ mutex is already enabled\n",
__func__);arch/arm/mm/cache-l2x0.c +9
-0+return false; + } + + l2x0_tz_mutex = (arch_spinlock_t *)addr; +/* insure mutex ptr is updated before lock is released */ +smp_wmb (); + +raw_spin_unlock_irqrestore (&l2x0_lock, flags); + pr_debug("\n%s: %sable TZ mutex\n\n", __func__, (addr) ? "en" :"dis");
arch/arm/mm/cache-l2x0.c +3
-0+return true; +} +void
__initl2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask){
u32 aux;arch/arm/mm/cache-l2x0.c +1
-0@@ -380,6 +422,7 @@ void __init l2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask) outer_cache.inv_all = l2x0_inv_all; outer_cache.disable = l2x0_disable; outer_cache.set_debug = l2x0_set_debug; + outer_cache.tz_mutex = l2x0_tz_mutex_cfg;printk (KERN_INFO "%s cache controller enabled\n" , type); printk(KERN_INFO "l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL0x%08x, Cache size: %d B\n",
--
Quoted 9 lines
Quoted 4 lines
1.7.11.3
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121113/ce5f15d9/attachment-0001.html>
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
Quoted 7 lines
Tue 11-13-2012 8:23 PM
From: Abhimanyu Kapur <[email protected]>Quoted 3 lines
Secure code in TrustZone space may need to perform L2 cache maintenance operations. A shared mutex is required to synchronize linux l2cc maintenance and TZ l2cc maintenance.
Quoted 6 lines
If you are using PL310 with thrustzone support then the L2 cache lines are secure bit tagged ; your design should be such that the secure (TZ) side only does operations on secure cache lines and non-secure side does operations only on non-secure cache lines. So each entity (TZ and nonTZ) if maintains their own cache and ensures integrity before switching over via monitor, this might not be needed.
I don't think 2 cores can safely write the LX20_CLEAN/INV_LINE_PA registers of the PL310 at the same time, even if targeting different lines. Each core TZ code must protect against other core (being UnSecure or TZ mode). The core IRQ disabling is required to protect from deadlock nonTZ and TZ worlds.
Regards,
etienne
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
Quoted 14 lines · Etienne CARRIERE ST
Quoted 7 lines
Tue 11-13-2012 8:23 PM
From: Abhimanyu Kapur <[email protected]>Quoted 3 lines
Secure code in TrustZone space may need to perform L2 cache maintenance operations. A shared mutex is required to synchronize linux l2cc maintenance and TZ l2cc maintenance.
Quoted 6 lines
If you are using PL310 with thrustzone support then the L2 cache lines are secure bit tagged ; your design should be such that the secure (TZ) side only does operations on secure cache lines and non-secure side does operations only on non-secure cache lines. So each entity (TZ and nonTZ) if maintains their own cache and ensures integrity before switching over via monitor, this might not be needed.
Quoted 3 lines
I don't think 2 cores can safely write the LX20_CLEAN/INV_LINE_PA registers of the PL310 at the same time, even if targeting different lines.
Actually for the clean/invalidate operations by PA you can safely write the registers from two different processors as they get serialised by the hardware. What you don't get is protection around the background operations (clean/inv by way). I think it depends on how the PL310 is wired on your hardware but trying to do a PA operation while a background one is in progress would trigger an external abort.
So the NS world could simply start a background cache operation without taking the lock while the secure world thinks that it has the lock and tries to do a PA operation which would abort.
My advise is to simply ignore the shared locking and only do atomic PA operations on the secure side. The secure side also needs to poll for the completion of any background operation that was started in non-secure world. Of course, there is still a race, in which case, depending on the hardware implementation, you would need to trap any possible aborts while in secure mode when writing the PL310 registers.
Signature · 1 line
Catalin
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
Thanks for the details.
I think we will internally use our hack in l2x0 spin locking to prevent collision, based on old arch_spinlock support.
For a definitive outer cache support in TZ, I think we will go into a RPC/SMC based architecture.
I mean a basic SMC to request secured outer cache maintenance, called from linux L2 cache driver based on its native spinlock, whatever it is.
This will indeed require some modification in L2 cache driver (cache-l2x0.c).
We will work on that once our TZ implementation support such RPC/SMC features for L2 cache maintenance.
Thanks to all for the feedbacks.
Regards,
etienne
-----Original Message-----
From: Catalin Marinas [mailto:catalin.marinas at arm.com]
Sent: Wednesday, November 14, 2012 6:22 PM
To: Etienne CARRIERE ST
Cc: Abhimanyu Kapur; Dave Martin; Rabin VINCENT; Russell King; Srinidhi KASAGAR; Marc Zyngier; Linus Walleij (linus.walleij at linaro.org); Will Deacon; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH 1/2] arm/mm: L2CC shared mutex with ARM TZ
Quoted 14 lines · Etienne CARRIERE ST
Quoted 7 lines
Tue 11-13-2012 8:23 PM
From: Abhimanyu Kapur <[email protected]>Quoted 3 lines
Secure code in TrustZone space may need to perform L2 cache maintenance operations. A shared mutex is required to synchronize linux l2cc maintenance and TZ l2cc maintenance.
Quoted 6 lines
If you are using PL310 with thrustzone support then the L2 cache lines are secure bit tagged ; your design should be such that the secure (TZ) side only does operations on secure cache lines and non-secure side does operations only on non-secure cache lines. So each entity (TZ and nonTZ) if maintains their own cache and ensures integrity before switching over via monitor, this might not be needed.
Quoted 3 lines
I don't think 2 cores can safely write the LX20_CLEAN/INV_LINE_PA registers of the PL310 at the same time, even if targeting different lines.
Actually for the clean/invalidate operations by PA you can safely write the registers from two different processors as they get serialised by the hardware. What you don't get is protection around the background operations (clean/inv by way). I think it depends on how the PL310 is wired on your hardware but trying to do a PA operation while a background one is in progress would trigger an external abort.
So the NS world could simply start a background cache operation without taking the lock while the secure world thinks that it has the lock and tries to do a PA operation which would abort.
My advise is to simply ignore the shared locking and only do atomic PA operations on the secure side. The secure side also needs to poll for the completion of any background operation that was started in non-secure world. Of course, there is still a race, in which case, depending on the hardware implementation, you would need to trap any possible aborts while in secure mode when writing the PL310 registers.
--
Catalin
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
Quoted 2 lines · Etienne CARRIERE ST
I think we will internally use our hack in l2x0 spin locking to prevent collision, based on old
arch_spinlocksupport.
Or just handle the collision in the secure code and make sure you don't start background operations. If you don't use background operations, you don't need any locking.
Signature · 1 line
Catalin
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
If I understood well, you stated that on-going background operations on L2 can jeopardize PA operations. So the secure code must insure other CPUs do not perform other L2 operations, including non-PA based operations.
Linux protects both PA and WAY operations using the very same spinlock. We must hold this lock while performing any PA operations from secure world.
I am right ?
etienne
-----Original Message-----
From: Catalin Marinas [mailto:catalin.marinas at arm.com]
Sent: Thursday, November 15, 2012 2:56 PM
To: Etienne CARRIERE ST
Cc: Abhimanyu Kapur; Dave Martin; Rabin VINCENT; Russell King; Srinidhi KASAGAR; Marc Zyngier; Linus Walleij (linus.walleij at linaro.org); Will Deacon; linux-arm-kernel at lists.infradead.org
Subject: Re: [PATCH 1/2] arm/mm: L2CC shared mutex with ARM TZ
Quoted 2 lines · Etienne CARRIERE ST
I think we will internally use our hack in l2x0 spin locking to prevent collision, based on old
arch_spinlocksupport.
Or just handle the collision in the secure code and make sure you don't start background operations. If you don't use background operations, you don't need any locking.
--
Catalin
PATCH 1/2 arm/mm: L2CC shared mutex with ARM TZ
In reply to Dave MartinQuoted 9 lines
Tue 11-13-2012 6:18 PM
From: Dave Martin <[email protected]>Quoted 3 lines
Secure code in TrustZone space may need to perform L2 cache maintenance operations. A shared mutex is required to synchronize linux l2cc maintenance and TZ l2cc maintenance.
1) Why is this not a denial-of-service risk? (i.e., why can the Normal World not simply hold the mutex forever?)
On SoC's, it is likely that some HW IP are controlled with ARM NS compliant HW support, thus needed for TZ execution (RAMs, ...). It is likely that nonTZ world, in such situation, could disable a clock, regulator, ... and arm TZ executions, including cache content. An easy "secu denial of service" hack, is just to reprogram linux based OS to prevent any or selected calls to secure code. Since linux OS is an untrusted environment, one could just reprogram it to prevent call to secure tagged features.
It is the responsibility of the TZ implementation to insure that secure content is not corrupted nor accessed to from nonTZ world.
Quoted 3 lines
2) The memory types on both sides have to be _exactly_ the same, not just "cached", otherwise unpredictable behaviour may occur when accessing the mutex. It is not obvious how this is ensured.
Ok, I will cross check this.
The mapping of the mutex/spinlock "lock" field is under responsibility of the TZ API driver. Each platform TZ API should define the expected mapping configuration.
Quoted 2 lines
3) Many people seem to delegate L2 cache maintenence requests via SMC in these situations. Can you do that instead?
We can have code that runs in TZ world and heavily processes data, benefitting from cache support, then needing the flush data to DDR. In such case, TZ would not need to request linux to request cache line maintenance.
Regards,
Etienne
Quoted 3 lines
Cheers
---Dave