| Message ID | 1431382651-15894-2-git-send-email-sjoerd.simons@collabora.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, May 12, 2015 at 12:17:29AM +0200, Sjoerd Simons wrote: > The L2C cache should only be unlocked when the cache is setup to allow > that. In the common case the l2x0 driver sets up the cache for that to > be the case (e.g. setting L310_AUX_CTRL_NS_LOCKDOWN on L2C-310), making > unlock safe. However when a secure firmware is in use, it may not be > possible for the L2c to be configured that way making unlocking unsafe. > > To handle that case add a flag to skip unlocking the cache for machine > where this can't be done safely. > > Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> > --- > arch/arm/include/asm/outercache.h | 1 + > arch/arm/mm/cache-l2x0.c | 4 +++- > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/arch/arm/include/asm/outercache.h b/arch/arm/include/asm/outercache.h > index 563b92f..d07ca82 100644 > --- a/arch/arm/include/asm/outercache.h > +++ b/arch/arm/include/asm/outercache.h > @@ -39,6 +39,7 @@ struct outer_cache_fns { > /* This is an ARM L2C thing */ > void (*write_sec)(unsigned long, unsigned); > void (*configure)(const struct l2x0_regs *); > + bool skip_unlock; > }; > > extern struct outer_cache_fns outer_cache; > diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c > index e309c8f..fff7888 100644 > --- a/arch/arm/mm/cache-l2x0.c > +++ b/arch/arm/mm/cache-l2x0.c > @@ -136,7 +136,8 @@ static void l2c_enable(void __iomem *base, u32 aux, unsigned num_lock) > l2x0_saved_regs.aux_ctrl = aux; > l2c_configure(base); > > - l2c_unlock(base, num_lock); > + if (!outer_cache.skip_unlock) > + l2c_unlock(base, num_lock); I think we can do better here. If the non-secure lockdown access bit has been set, then proceed with the unlock: if (readl_relaxed(base + L2X0_AUX_CTRL) & L310_AUX_CTRL_NS_LOCKDOWN) l2c_unlock(base, num_lock); I don't see any need to add a flag for this. This also eliminates your second patch.
On Mon, 2015-05-11 at 23:29 +0100, Russell King - ARM Linux wrote: > On Tue, May 12, 2015 at 12:17:29AM +0200, Sjoerd Simons wrote: > > extern struct outer_cache_fns outer_cache; > > diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c > > index e309c8f..fff7888 100644 > > --- a/arch/arm/mm/cache-l2x0.c > > +++ b/arch/arm/mm/cache-l2x0.c > > @@ -136,7 +136,8 @@ static void l2c_enable(void __iomem *base, u32 aux, unsigned num_lock) > > l2x0_saved_regs.aux_ctrl = aux; > > l2c_configure(base); > > > > - l2c_unlock(base, num_lock); > > + if (!outer_cache.skip_unlock) > > + l2c_unlock(base, num_lock); > > I think we can do better here. If the non-secure lockdown access bit has > been set, then proceed with the unlock: > > if (readl_relaxed(base + L2X0_AUX_CTRL) & L310_AUX_CTRL_NS_LOCKDOWN) > l2c_unlock(base, num_lock); > > I don't see any need to add a flag for this. This also eliminates your > second patch. Main reason I added the flag like this was to simplify the changes as l2c_enable has no real knowledge about which type of cache it's running on. But sure i will have a look at re-jigging the code such that the situation is automatically detected rather then requiring the machine specific code to flag it explicitely
diff --git a/arch/arm/include/asm/outercache.h b/arch/arm/include/asm/outercache.h index 563b92f..d07ca82 100644 --- a/arch/arm/include/asm/outercache.h +++ b/arch/arm/include/asm/outercache.h @@ -39,6 +39,7 @@ struct outer_cache_fns { /* This is an ARM L2C thing */ void (*write_sec)(unsigned long, unsigned); void (*configure)(const struct l2x0_regs *); + bool skip_unlock; }; extern struct outer_cache_fns outer_cache; diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c index e309c8f..fff7888 100644 --- a/arch/arm/mm/cache-l2x0.c +++ b/arch/arm/mm/cache-l2x0.c @@ -136,7 +136,8 @@ static void l2c_enable(void __iomem *base, u32 aux, unsigned num_lock) l2x0_saved_regs.aux_ctrl = aux; l2c_configure(base); - l2c_unlock(base, num_lock); + if (!outer_cache.skip_unlock) + l2c_unlock(base, num_lock); local_irq_save(flags); __l2c_op_way(base + L2X0_INV_WAY); @@ -849,6 +850,7 @@ static int __init __l2c_init(const struct l2c_init_data *data, fns = data->outer_cache; fns.write_sec = outer_cache.write_sec; fns.configure = outer_cache.configure; + fns.skip_unlock = outer_cache.skip_unlock; if (data->fixup) data->fixup(l2x0_base, cache_id, &fns);
The L2C cache should only be unlocked when the cache is setup to allow that. In the common case the l2x0 driver sets up the cache for that to be the case (e.g. setting L310_AUX_CTRL_NS_LOCKDOWN on L2C-310), making unlock safe. However when a secure firmware is in use, it may not be possible for the L2c to be configured that way making unlocking unsafe. To handle that case add a flag to skip unlocking the cache for machine where this can't be done safely. Signed-off-by: Sjoerd Simons <sjoerd.simons@collabora.co.uk> --- arch/arm/include/asm/outercache.h | 1 + arch/arm/mm/cache-l2x0.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-)