2015-05-11 22:17:40

by Sjoerd Simons

[permalink] [raw]
Subject: [PATCH 0/3] Fix boot on Calxeda highbank

When upgrading our trusted Calxeda server to Debian Jessie recently the machine
wedged during booting the installer. Upon further investigation, this happens
due to the transition to the generic L2C infrastructure introduced in 3.16. The
generic l2x0 code unlocks the cache during setup, however the Caldexa SMC
interface doesn't seem to allow the kernel to enable enable non-secure access
to the lock registers.. Queue Imprecise aborts and a fairly unhappy machine.

First patch in this series adds a flag to indicate to the l2x0 code the
unlocking should be skipped, second patch turns that flag on for mach-highbank,
fixing this issue. Third patch adds a (empty) configure callback for the
highbank l2c, reflecting the fact that there seeminglyisn't anything to
configured via an SMC on these maches.

Sjoerd Simons (3):
ARM: cache-l2c: Add flag to skip cache unlocking
ARM: l2c: highbank: Skip l2c unlocking
ARM: l2c: highbank: Add dummy configure function

arch/arm/include/asm/outercache.h | 1 +
arch/arm/mach-highbank/highbank.c | 9 +++++++++
arch/arm/mm/cache-l2x0.c | 4 +++-
3 files changed, 13 insertions(+), 1 deletion(-)

--
2.1.4


2015-05-11 22:17:47

by Sjoerd Simons

[permalink] [raw]
Subject: [PATCH 1/3] ARM: cache-l2c: Add flag to skip cache unlocking

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 <[email protected]>
---
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);

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);

--
2.1.4

2015-05-11 22:17:39

by Sjoerd Simons

[permalink] [raw]
Subject: [PATCH 2/3] ARM: l2c: highbank: Skip l2c unlocking

The highbank SMC interface doesn't allow configuring the cache for
unlocking from the non-secure world. So skip unlocking otherwise the
machine get imprecise abort and become unstable on boot.

Signed-off-by: Sjoerd Simons <[email protected]>
---
arch/arm/mach-highbank/highbank.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm/mach-highbank/highbank.c b/arch/arm/mach-highbank/highbank.c
index 231fba0..8e4846d 100644
--- a/arch/arm/mach-highbank/highbank.c
+++ b/arch/arm/mach-highbank/highbank.c
@@ -66,6 +66,10 @@ static void __init highbank_init_irq(void)

if (of_find_compatible_node(NULL, NULL, "arm,cortex-a9"))
highbank_scu_map_io();
+
+ if (IS_ENABLED(CONFIG_CACHE_L2X0)) {
+ outer_cache.skip_unlock = true;
+ }
}

static void highbank_power_off(void)
--
2.1.4

2015-05-11 22:17:44

by Sjoerd Simons

[permalink] [raw]
Subject: [PATCH 3/3] ARM: l2c: highbank: Add dummy configure function

Add empty configure function for the highbank l2c to reflect that the
cache can't be configured through a SMC. This prevent a useless
WARN_ONCE during early boot.

Signed-off-by: Sjoerd Simons <[email protected]>
---
arch/arm/mach-highbank/highbank.c | 5 +++++
1 file changed, 5 insertions(+)

diff --git a/arch/arm/mach-highbank/highbank.c b/arch/arm/mach-highbank/highbank.c
index 8e4846d..cee18db 100644
--- a/arch/arm/mach-highbank/highbank.c
+++ b/arch/arm/mach-highbank/highbank.c
@@ -60,6 +60,10 @@ static void highbank_l2c310_write_sec(unsigned long val, unsigned reg)
reg);
}

+static void highbank_l2c310_configure(const struct l2x0_regs *regs)
+{
+}
+
static void __init highbank_init_irq(void)
{
irqchip_init();
@@ -68,6 +72,7 @@ static void __init highbank_init_irq(void)
highbank_scu_map_io();

if (IS_ENABLED(CONFIG_CACHE_L2X0)) {
+ outer_cache.configure = highbank_l2c310_configure;
outer_cache.skip_unlock = true;
}
}
--
2.1.4

2015-05-11 22:30:16

by Russell King - ARM Linux

[permalink] [raw]
Subject: Re: [PATCH 1/3] ARM: cache-l2c: Add flag to skip cache unlocking

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 <[email protected]>
> ---
> 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.

--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.

2015-05-12 05:13:45

by Sjoerd Simons

[permalink] [raw]
Subject: Re: [PATCH 1/3] ARM: cache-l2c: Add flag to skip cache unlocking

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

--
Sjoerd Simons <[email protected]>
Collabora Ltd.