<html>
<head>
<style><!--
.hmmessage P
{
margin:0px;
padding:0px
}
body.hmmessage
{
font-size: 12pt;
font-family:Calibri
}
--></style></head>
<body class='hmmessage'><div dir='ltr'><span style="font-size: 12pt;">> Date: Tue, 13 Nov 2012 17:17:45 +0000</span><br><div>> From: dave.martin@linaro.org<br>> To: etienne.carriere@st.com<br>> Subject: Re: [PATCH 1/2] arm/mm: L2CC shared mutex with ARM TZ<br>> CC: rabin.vincent@stericsson.com; linux@arm.linux.org.uk; srinidhi.kasagar@stericsson.com; Marc.Zyngier@arm.com; Catalin.Marinas@arm.com; linus.walleij@linaro.org; Will.Deacon@arm.com; linux-arm-kernel@lists.infradead.org<br>> <br>> On Tue, Nov 13, 2012 at 04:08:14PM +0000, Etienne CARRIERE wrote:<br>> > From: Etienne Carriere <etienne.carriere@stericsson.com><br>> > <br>> > <br>> > <br>> > Secure code in TrustZone space may need to perform L2 cache<br>> > <br>> > maintenance operations. A shared mutex is required to synchronize<br>> > <br>> > linux l2cc maintenance and TZ l2cc maintenance.<br>> <br><span style="font-size: 12pt;">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.</span></div><div><span style="font-size: 12pt;"><br></span></div><div><span style="font-size: 12pt;">Abhi</span></div><div><span style="font-size: 12pt;">> 1) Why is this not a denial-of-service risk? (i.e., why can the Normal</span></div><div>> World not simply hold the mutex forever?)<br>> <br>> 2) The memory types on both sides have to be _exactly_ the same, not<br>> just "cached", otherwise unpredictable behaviour may occur when accessing<br>> the mutex. It is not obvious how this is ensured.<br>> <br>> 3) Many people seem to delegate L2 cache maintenence requests via SMC<br>> in these situations. Can you do that instead?<br>> <br>> <br>> Cheers<br>> ---Dave<br>> <br>> > The TZ mutex is an "arch_spinlock": a 32bit DDR cell (ARMv7-A mutex).<br>> > <br>> > Linux L2 cache driver must lock TZ mutex if enabled.<br>> > <br>> > <br>> > <br>> > Signed-off-by: Etienne Carriere <etienne.carriere@stericsson.com><br>> > <br>> > ---<br>> > <br>> > arch/arm/include/asm/outercache.h | 9 ++++<br>> > <br>> > arch/arm/mm/cache-l2x0.c | 87 +++++++++++++++++++++++++++++----------<br>> > <br>> > 2 files changed, 74 insertions(+), 22 deletions(-)<br>> > <br>> > <br>> > <br>> > diff --git a/arch/arm/include/asm/outercache.h b/arch/arm/include/asm/<br>> > outercache.h<br>> > <br>> > index 53426c6..7aa5eac 100644<br>> > <br>> > --- a/arch/arm/include/asm/outercache.h<br>> > <br>> > +++ b/arch/arm/include/asm/outercache.h<br>> > <br>> > @@ -35,6 +35,7 @@ struct outer_cache_fns {<br>> > <br>> > #endif<br>> > <br>> > void (*set_debug)(unsigned long);<br>> > <br>> > void (*resume)(void);<br>> > <br>> > + bool (*tz_mutex)(unsigned long);<br>> > <br>> > };<br>> > <br>> > #ifdef CONFIG_OUTER_CACHE<br>> > <br>> > @@ -81,6 +82,13 @@ static inline void outer_resume(void)<br>> > <br>> > outer_cache.resume();<br>> > <br>> > }<br>> > <br>> > +static inline bool outer_tz_mutex(unsigned long addr)<br>> > <br>> > +{<br>> > <br>> > + if (outer_cache.tz_mutex)<br>> > <br>> > + return outer_cache.tz_mutex(addr);<br>> > <br>> > + return false;<br>> > <br>> > +}<br>> > <br>> > +<br>> > <br>> > #else<br>> > <br>> > static inline void outer_inv_range(phys_addr_t start, phys_addr_t end)<br>> > <br>> > @@ -92,6 +100,7 @@ static inline void outer_flush_range(phys_addr_t start,<br>> > phys_addr_t end)<br>> > <br>> > static inline void outer_flush_all(void) { }<br>> > <br>> > static inline void outer_inv_all(void) { }<br>> > <br>> > static inline void outer_disable(void) { }<br>> > <br>> > +static inline bool outer_tz_mutex(unsigned long addr) { return false; }<br>> > <br>> > #endif<br>> > <br>> > diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c<br>> > <br>> > index a53fd2a..eacdc74 100644<br>> > <br>> > --- a/arch/arm/mm/cache-l2x0.c<br>> > <br>> > +++ b/arch/arm/mm/cache-l2x0.c<br>> > <br>> > @@ -41,6 +41,26 @@ struct l2x0_of_data {<br>> > <br>> > void (*resume)(void);<br>> > <br>> > };<br>> > <br>> > +/*<br>> > <br>> > + * arch_spinlock (single 32bit DDR mutex cell) pointer to synchronise<br>> > <br>> > + * L2CC maintenance between linux world and secure world (ARM TZ).<br>> > <br>> > + */<br>> > <br>> > +arch_spinlock_t *l2x0_tz_mutex;<br>> > <br>> > +<br>> > <br>> > +#define l2x0_spin_lock_irqsave(flags) \<br>> > <br>> > + do<br>> > { <br>> > \<br>> > <br>> > + raw_spin_lock_irqsave(&l2x0_lock, flags);<br>> > \<br>> > <br>> > + if (l2x0_tz_mutex)<br>> > \<br>> > <br>> > + arch_spin_lock(l2x0_tz_mutex);<br>> > \<br>> > <br>> > + } while (0)<br>> > <br>> > +<br>> > <br>> > +#define l2x0_spin_unlock_irqrestore(flags) \<br>> > <br>> > + do<br>> > { <br>> > \<br>> > <br>> > + if (l2x0_tz_mutex)<br>> > \<br>> > <br>> > + arch_spin_unlock(l2x0_tz_mutex);<br>> > \<br>> > <br>> > + raw_spin_unlock_irqrestore(&l2x0_lock, flags);<br>> > \<br>> > <br>> > + } while (0)<br>> > <br>> > +<br>> > <br>> > static inline void cache_wait_way(void __iomem *reg, unsigned long mask)<br>> > <br>> > {<br>> > <br>> > /* wait for cache operation by line or way to complete */<br>> > <br>> > @@ -126,9 +146,9 @@ static void l2x0_cache_sync(void)<br>> > <br>> > {<br>> > <br>> > unsigned long flags;<br>> > <br>> > - raw_spin_lock_irqsave(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_lock_irqsave(flags);<br>> > <br>> > cache_sync();<br>> > <br>> > - raw_spin_unlock_irqrestore(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_unlock_irqrestore(flags);<br>> > <br>> > }<br>> > <br>> > static void __l2x0_flush_all(void)<br>> > <br>> > @@ -145,9 +165,9 @@ static void l2x0_flush_all(void)<br>> > <br>> > unsigned long flags;<br>> > <br>> > /* clean all ways */<br>> > <br>> > - raw_spin_lock_irqsave(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_lock_irqsave(flags);<br>> > <br>> > __l2x0_flush_all();<br>> > <br>> > - raw_spin_unlock_irqrestore(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_unlock_irqrestore(flags);<br>> > <br>> > }<br>> > <br>> > static void l2x0_clean_all(void)<br>> > <br>> > @@ -155,11 +175,11 @@ static void l2x0_clean_all(void)<br>> > <br>> > unsigned long flags;<br>> > <br>> > /* clean all ways */<br>> > <br>> > - raw_spin_lock_irqsave(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_lock_irqsave(flags);<br>> > <br>> > writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_CLEAN_WAY);<br>> > <br>> > cache_wait_way(l2x0_base + L2X0_CLEAN_WAY, l2x0_way_mask);<br>> > <br>> > cache_sync();<br>> > <br>> > - raw_spin_unlock_irqrestore(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_unlock_irqrestore(flags);<br>> > <br>> > }<br>> > <br>> > static void l2x0_inv_all(void)<br>> > <br>> > @@ -167,13 +187,13 @@ static void l2x0_inv_all(void)<br>> > <br>> > unsigned long flags;<br>> > <br>> > /* invalidate all ways */<br>> > <br>> > - raw_spin_lock_irqsave(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_lock_irqsave(flags);<br>> > <br>> > /* Invalidating when L2 is enabled is a nono */<br>> > <br>> > BUG_ON(readl(l2x0_base + L2X0_CTRL) & 1);<br>> > <br>> > writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_INV_WAY);<br>> > <br>> > cache_wait_way(l2x0_base + L2X0_INV_WAY, l2x0_way_mask);<br>> > <br>> > cache_sync();<br>> > <br>> > - raw_spin_unlock_irqrestore(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_unlock_irqrestore(flags);<br>> > <br>> > }<br>> > <br>> > static void l2x0_inv_range(unsigned long start, unsigned long end)<br>> > <br>> > @@ -181,7 +201,7 @@ static void l2x0_inv_range(unsigned long start, unsigned<br>> > long end)<br>> > <br>> > void __iomem *base = l2x0_base;<br>> > <br>> > unsigned long flags;<br>> > <br>> > - raw_spin_lock_irqsave(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_lock_irqsave(flags);<br>> > <br>> > if (start & (CACHE_LINE_SIZE - 1)) {<br>> > <br>> > start &= ~(CACHE_LINE_SIZE - 1);<br>> > <br>> > debug_writel(0x03);<br>> > <br>> > @@ -206,13 +226,13 @@ static void l2x0_inv_range(unsigned long start, unsigned<br>> > long end)<br>> > <br>> > }<br>> > <br>> > if (blk_end < end) {<br>> > <br>> > - raw_spin_unlock_irqrestore(&<br>> > l2x0_lock, flags);<br>> > <br>> > - raw_spin_lock_irqsave(&<br>> > l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_unlock_irqrestore<br>> > (flags);<br>> > <br>> > + l2x0_spin_lock_irqsave(flags);<br>> > <br>> > }<br>> > <br>> > }<br>> > <br>> > cache_wait(base + L2X0_INV_LINE_PA, 1);<br>> > <br>> > cache_sync();<br>> > <br>> > - raw_spin_unlock_irqrestore(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_unlock_irqrestore(flags);<br>> > <br>> > }<br>> > <br>> > static void l2x0_clean_range(unsigned long start, unsigned long end)<br>> > <br>> > @@ -225,7 +245,7 @@ static void l2x0_clean_range(unsigned long start, unsigned<br>> > long end)<br>> > <br>> > return;<br>> > <br>> > }<br>> > <br>> > - raw_spin_lock_irqsave(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_lock_irqsave(flags);<br>> > <br>> > start &= ~(CACHE_LINE_SIZE - 1);<br>> > <br>> > while (start < end) {<br>> > <br>> > unsigned long blk_end = start + min(end - start,<br>> > 4096UL);<br>> > <br>> > @@ -236,13 +256,13 @@ static void l2x0_clean_range(unsigned long start,<br>> > unsigned long end)<br>> > <br>> > }<br>> > <br>> > if (blk_end < end) {<br>> > <br>> > - raw_spin_unlock_irqrestore(&<br>> > l2x0_lock, flags);<br>> > <br>> > - raw_spin_lock_irqsave(&<br>> > l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_unlock_irqrestore<br>> > (flags);<br>> > <br>> > + l2x0_spin_lock_irqsave(flags);<br>> > <br>> > }<br>> > <br>> > }<br>> > <br>> > cache_wait(base + L2X0_CLEAN_LINE_PA, 1);<br>> > <br>> > cache_sync();<br>> > <br>> > - raw_spin_unlock_irqrestore(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_unlock_irqrestore(flags);<br>> > <br>> > }<br>> > <br>> > static void l2x0_flush_range(unsigned long start, unsigned long end)<br>> > <br>> > @@ -255,7 +275,7 @@ static void l2x0_flush_range(unsigned long start, unsigned<br>> > long end)<br>> > <br>> > return;<br>> > <br>> > }<br>> > <br>> > - raw_spin_lock_irqsave(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_lock_irqsave(flags);<br>> > <br>> > start &= ~(CACHE_LINE_SIZE - 1);<br>> > <br>> > while (start < end) {<br>> > <br>> > unsigned long blk_end = start + min(end - start,<br>> > 4096UL);<br>> > <br>> > @@ -268,24 +288,24 @@ static void l2x0_flush_range(unsigned long start,<br>> > unsigned long end)<br>> > <br>> > debug_writel(0x00);<br>> > <br>> > if (blk_end < end) {<br>> > <br>> > - raw_spin_unlock_irqrestore(&<br>> > l2x0_lock, flags);<br>> > <br>> > - raw_spin_lock_irqsave(&<br>> > l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_unlock_irqrestore<br>> > (flags);<br>> > <br>> > + l2x0_spin_lock_irqsave(flags);<br>> > <br>> > }<br>> > <br>> > }<br>> > <br>> > cache_wait(base + L2X0_CLEAN_INV_LINE_PA, 1);<br>> > <br>> > cache_sync();<br>> > <br>> > - raw_spin_unlock_irqrestore(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_unlock_irqrestore(flags);<br>> > <br>> > }<br>> > <br>> > static void l2x0_disable(void)<br>> > <br>> > {<br>> > <br>> > unsigned long flags;<br>> > <br>> > - raw_spin_lock_irqsave(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_lock_irqsave(flags);<br>> > <br>> > __l2x0_flush_all();<br>> > <br>> > writel_relaxed(0, l2x0_base + L2X0_CTRL);<br>> > <br>> > dsb();<br>> > <br>> > - raw_spin_unlock_irqrestore(&l2x0_lock, flags);<br>> > <br>> > + l2x0_spin_unlock_irqrestore(flags);<br>> > <br>> > }<br>> > <br>> > static void l2x0_unlock(u32 cache_id)<br>> > <br>> > @@ -307,6 +327,28 @@ static void l2x0_unlock(u32 cache_id)<br>> > <br>> > }<br>> > <br>> > }<br>> > <br>> > +/* Enable/disable external mutex shared with TZ code */<br>> > <br>> > +static bool l2x0_tz_mutex_cfg(unsigned long addr)<br>> > <br>> > +{<br>> > <br>> > + unsigned long flags;<br>> > <br>> > +<br>> > <br>> > + raw_spin_lock_irqsave(&l2x0_lock, flags);<br>> > <br>> > +<br>> > <br>> > + if (addr && l2x0_tz_mutex && (addr != (uint)l2x0_tz_mutex)) {<br>> > <br>> > + raw_spin_unlock_irqrestore(&l2x0_lock, flags);<br>> > <br>> > + pr_err("%s: a TZ mutex is already enabled\n",<br>> > __func__);<br>> > <br>> > + return false;<br>> > <br>> > + }<br>> > <br>> > +<br>> > <br>> > + l2x0_tz_mutex = (arch_spinlock_t *)addr;<br>> > <br>> > + /* insure mutex ptr is updated before lock is released */<br>> > <br>> > + smp_wmb();<br>> > <br>> > +<br>> > <br>> > + raw_spin_unlock_irqrestore(&l2x0_lock, flags);<br>> > <br>> > + pr_debug("\n%s: %sable TZ mutex\n\n", __func__, (addr) ? "en" :<br>> > "dis");<br>> > <br>> > + return true;<br>> > <br>> > +}<br>> > <br>> > +<br>> > <br>> > void __init l2x0_init(void __iomem *base, u32 aux_val, u32 aux_mask)<br>> > <br>> > {<br>> > <br>> > u32 aux;<br>> > <br>> > @@ -380,6 +422,7 @@ void __init l2x0_init(void __iomem *base, u32 aux_val, u32<br>> > aux_mask)<br>> > <br>> > outer_cache.inv_all = l2x0_inv_all;<br>> > <br>> > outer_cache.disable = l2x0_disable;<br>> > <br>> > outer_cache.set_debug = l2x0_set_debug;<br>> > <br>> > + outer_cache.tz_mutex = l2x0_tz_mutex_cfg;<br>> > <br>> > printk(KERN_INFO "%s cache controller enabled\n", type);<br>> > <br>> > printk(KERN_INFO "l2x0: %d ways, CACHE_ID 0x%08x, AUX_CTRL<br>> > 0x%08x, Cache size: %d B\n",<br>> > <br>> > <br>> > <br>> > --<br>> > <br>> > 1.7.11.3<br>> > <br>> > <br>> > <br>> <br>> _______________________________________________<br>> linux-arm-kernel mailing list<br>> linux-arm-kernel@lists.infradead.org<br>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel<br></div> </div></body>
</html>