[PATCH 1/2] mach-ux500: cache operations are atomic on PL310

[PATCH 1/2] mach-ux500: cache operations are atomic on PL310

[Srinidhi KASAGAR]

Apply ERRATA_753970 for ux500 variant of cache sync too

Signed-off-by: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-ux500/cache-l2x0.c |   11 ++++++++---
 1 files changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/arm/mach-ux500/cache-l2x0.c b/arch/arm/mach-ux500/cache-l2x0.c
index 122ddde..8aaa21a 100644
--- a/arch/arm/mach-ux500/cache-l2x0.c
+++ b/arch/arm/mach-ux500/cache-l2x0.c
@@ -12,7 +12,7 @@
 
 static void __iomem *l2x0_base;
 
-static inline void ux500_cache_wait(void __iomem *reg, unsigned long mask)
+static inline void ux500_cache_wait_way(void __iomem *reg, unsigned long mask)
 {
 	/* wait for the operation to complete */
 	while (readl_relaxed(reg) & mask)
@@ -21,8 +21,13 @@ static inline void ux500_cache_wait(void __iomem *reg, unsigned long mask)
 
 static inline void ux500_cache_sync(void)
 {
+#ifdef CONFIG_ARM_ERRATA_753970
+	/* write to an unmmapped register */
+	writel_relaxed(0, l2x0_base + L2X0_DUMMY_REG);
+#else
 	writel_relaxed(0, l2x0_base + L2X0_CACHE_SYNC);
-	ux500_cache_wait(l2x0_base + L2X0_CACHE_SYNC, 1);
+#endif
+	/* cache operations by line are atomic in PL310 */
 }
 
 /*
@@ -46,7 +51,7 @@ static void ux500_l2x0_inv_all(void)
 
 	/* invalidate all ways */
 	writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_INV_WAY);
-	ux500_cache_wait(l2x0_base + L2X0_INV_WAY, l2x0_way_mask);
+	ux500_cache_wait_way(l2x0_base + L2X0_INV_WAY, l2x0_way_mask);
 	ux500_cache_sync();
 }
 
-- 
1.7.4.3

[PATCH 2/2] mach-ux500: enable ARM errata 764369

[Srinidhi KASAGAR]

This applies ARM errata 764369 for all ux500 platforms.

Signed-off-by: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
---
 arch/arm/mach-ux500/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mach-ux500/Kconfig b/arch/arm/mach-ux500/Kconfig
index a3e0c86..52af004 100644
--- a/arch/arm/mach-ux500/Kconfig
+++ b/arch/arm/mach-ux500/Kconfig
@@ -7,6 +7,7 @@ config UX500_SOC_COMMON
 	select HAS_MTU
 	select ARM_ERRATA_753970
 	select ARM_ERRATA_754322
+	select ARM_ERRATA_764369
 
 menu "Ux500 SoC"
 
-- 
1.7.4.3

[PATCH 1/2] mach-ux500: cache operations are atomic on PL310

[Will Deacon]

Hi guys,

On Thu, Jan 12, 2012 at 05:37:42AM +0000, Srinidhi KASAGAR wrote:
> Apply ERRATA_753970 for ux500 variant of cache sync too
> 
> Signed-off-by: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  arch/arm/mach-ux500/cache-l2x0.c |   11 ++++++++---
>  1 files changed, 8 insertions(+), 3 deletions(-)

I hadn't noticed the existence of this file before, but this patch really
shows why it's not a good idea to copy files out of core ARM code and into
the mach-* directories. I see that the commit introducing this file 458eef2f
("mach-ux500: factor out l2x0 handling code") mentions that mach-imx does
the same thing, but I can't find the code there.

On top of that, it seems as though you provide an inv_all implementation
but your disable function is empty. Surely this can lead to data loss?

Will

[PATCH 1/2] mach-ux500: cache operations are atomic on PL310

[Srinidhi KASAGAR]

On Fri, Jan 13, 2012 at 19:14:22 +0100, Will Deacon wrote:
> Hi guys,
> 
> On Thu, Jan 12, 2012 at 05:37:42AM +0000, Srinidhi KASAGAR wrote:
> > Apply ERRATA_753970 for ux500 variant of cache sync too
> > 
> > Signed-off-by: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
> > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > ---
> >  arch/arm/mach-ux500/cache-l2x0.c |   11 ++++++++---
> >  1 files changed, 8 insertions(+), 3 deletions(-)
> 
> I hadn't noticed the existence of this file before, but this patch really
> shows why it's not a good idea to copy files out of core ARM code and into
> the mach-* directories. I see that the commit introducing this file 458eef2f
> ("mach-ux500: factor out l2x0 handling code") mentions that mach-imx does
> the same thing, but I can't find the code there.
> 
> On top of that, it seems as though you provide an inv_all implementation
> but your disable function is empty. Surely this can lead to data loss?

We can't disable l2x0 from non secure mode and either we do not have
special SMI to handle the same and hence it is empty. So for kexec
to work on this platform we need to have a non-locking variant of
inv_all() otherwise we seems to be looping forever in spin locks
as such L1 caches are disabled at that moment in cpu_proc_fin().
So we had to override this inv_all for this machine.

Otherwise, what do you suggest? Whats the effect if we remove
that spin lock in inv_all as such I don't see much users using
inv_all.

srinidhi

[PATCH 1/2] mach-ux500: cache operations are atomic on PL310

[Will Deacon]

On Mon, Jan 16, 2012 at 11:05:29AM +0000, Srinidhi KASAGAR wrote:
> On Fri, Jan 13, 2012 at 19:14:22 +0100, Will Deacon wrote:
> > Hi guys,
> > 
> > On Thu, Jan 12, 2012 at 05:37:42AM +0000, Srinidhi KASAGAR wrote:
> > > Apply ERRATA_753970 for ux500 variant of cache sync too
> > > 
> > > Signed-off-by: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
> > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > ---
> > >  arch/arm/mach-ux500/cache-l2x0.c |   11 ++++++++---
> > >  1 files changed, 8 insertions(+), 3 deletions(-)
> > 
> > I hadn't noticed the existence of this file before, but this patch really
> > shows why it's not a good idea to copy files out of core ARM code and into
> > the mach-* directories. I see that the commit introducing this file 458eef2f
> > ("mach-ux500: factor out l2x0 handling code") mentions that mach-imx does
> > the same thing, but I can't find the code there.
> > 
> > On top of that, it seems as though you provide an inv_all implementation
> > but your disable function is empty. Surely this can lead to data loss?
> 
> We can't disable l2x0 from non secure mode and either we do not have
> special SMI to handle the same and hence it is empty. So for kexec
> to work on this platform we need to have a non-locking variant of
> inv_all() otherwise we seems to be looping forever in spin locks
> as such L1 caches are disabled at that moment in cpu_proc_fin().
> So we had to override this inv_all for this machine.

Actually, now that I've fixed kexec, we don't use inv_all in that path. It's
only used in the power management code now, it seems. But my point still
stands - invalidating a cache that is enabled is a *really* bad idea, that's
why we have this in cache-l2x0.c:

	/* Invalidating when L2 is enabled is a nono */
	BUG_ON(readl(l2x0_base + L2X0_CTRL) & 1);

> Otherwise, what do you suggest? Whats the effect if we remove
> that spin lock in inv_all as such I don't see much users using
> inv_all.

The lock needs to stay. Besides, the problem isn't with inv_all, the problem
is with not being able to disable the outer cache. So can't you just do
something nasty like:

	outer_cache.disable = NULL;

after your call to l2x0_init?

Also, if you can't disable the L2 from non-secure, does that mean that you
boot Linux with the L2 enabled?

Will

[PATCH 1/2] mach-ux500: cache operations are atomic on PL310

[Srinidhi KASAGAR]

On Mon, Jan 16, 2012 at 15:50:08 +0100, Will Deacon wrote:
> On Mon, Jan 16, 2012 at 11:05:29AM +0000, Srinidhi KASAGAR wrote:
> > On Fri, Jan 13, 2012 at 19:14:22 +0100, Will Deacon wrote:
> > > Hi guys,
> > > 
> > > On Thu, Jan 12, 2012 at 05:37:42AM +0000, Srinidhi KASAGAR wrote:
> > > > Apply ERRATA_753970 for ux500 variant of cache sync too
> > > > 
> > > > Signed-off-by: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
> > > > Acked-by: Linus Walleij <linus.walleij@linaro.org>
> > > > ---
> > > >  arch/arm/mach-ux500/cache-l2x0.c |   11 ++++++++---
> > > >  1 files changed, 8 insertions(+), 3 deletions(-)
> > > 
> > > I hadn't noticed the existence of this file before, but this patch really
> > > shows why it's not a good idea to copy files out of core ARM code and into
> > > the mach-* directories. I see that the commit introducing this file 458eef2f
> > > ("mach-ux500: factor out l2x0 handling code") mentions that mach-imx does
> > > the same thing, but I can't find the code there.
> > > 
> > > On top of that, it seems as though you provide an inv_all implementation
> > > but your disable function is empty. Surely this can lead to data loss?
> > 
> > We can't disable l2x0 from non secure mode and either we do not have
> > special SMI to handle the same and hence it is empty. So for kexec
> > to work on this platform we need to have a non-locking variant of
> > inv_all() otherwise we seems to be looping forever in spin locks
> > as such L1 caches are disabled at that moment in cpu_proc_fin().
> > So we had to override this inv_all for this machine.
> 
> Actually, now that I've fixed kexec, we don't use inv_all in that path. It's
> only used in the power management code now, it seems. But my point still
> stands - invalidating a cache that is enabled is a *really* bad idea, that's
> why we have this in cache-l2x0.c:
> 
> 	/* Invalidating when L2 is enabled is a nono */
> 	BUG_ON(readl(l2x0_base + L2X0_CTRL) & 1);
> 
> > Otherwise, what do you suggest? Whats the effect if we remove
> > that spin lock in inv_all as such I don't see much users using
> > inv_all.
> 
> The lock needs to stay. Besides, the problem isn't with inv_all, the problem
> is with not being able to disable the outer cache. So can't you just do
> something nasty like:
> 
> 	outer_cache.disable = NULL;
> 
> after your call to l2x0_init?

hmm..patch below

> 
> Also, if you can't disable the L2 from non-secure, does that mean that you
> boot Linux with the L2 enabled?

Yes, we boot with L2 enabled.

>From 46fdbda7d2d9a9f4df0933341bcc467b3bdd03d6 Mon Sep 17 00:00:00 2001
From: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
Date: Tue, 17 Jan 2012 11:29:39 +0530
Subject: [PATCH] mach-ux500: Do not override outer.inv_all

outer.inv_all is currently being used only in kexec path.
Invalidating outer cache without disabling it is a big
nono, and so, remove the machine specific outer.inv_all
assuming that kexec does not call inv_all in its path.

And at the same time it does not prevent us overriding
outer.disable as we do not have any such secure SMI to
handle the same while kexec disables the outer cache.

Signed-off-by: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
---
 arch/arm/mach-ux500/cache-l2x0.c |   48 +++++--------------------------------
 1 files changed, 7 insertions(+), 41 deletions(-)

diff --git a/arch/arm/mach-ux500/cache-l2x0.c b/arch/arm/mach-ux500/cache-l2x0.c
index 122ddde..45111c8 100644
--- a/arch/arm/mach-ux500/cache-l2x0.c
+++ b/arch/arm/mach-ux500/cache-l2x0.c
@@ -12,44 +12,6 @@
 
 static void __iomem *l2x0_base;
 
-static inline void ux500_cache_wait(void __iomem *reg, unsigned long mask)
-{
-	/* wait for the operation to complete */
-	while (readl_relaxed(reg) & mask)
-		cpu_relax();
-}
-
-static inline void ux500_cache_sync(void)
-{
-	writel_relaxed(0, l2x0_base + L2X0_CACHE_SYNC);
-	ux500_cache_wait(l2x0_base + L2X0_CACHE_SYNC, 1);
-}
-
-/*
- * The L2 cache cannot be turned off in the non-secure world.
- * Dummy until a secure service is in place.
- */
-static void ux500_l2x0_disable(void)
-{
-}
-
-/*
- * This is only called when doing a kexec, just after turning off the L2
- * and L1 cache, and it is surrounded by a spinlock in the generic version.
- * However, we're not really turning off the L2 cache right now and the
- * PL310 does not support exclusive accesses (used to implement the spinlock).
- * So, the invalidation needs to be done without the spinlock.
- */
-static void ux500_l2x0_inv_all(void)
-{
-	uint32_t l2x0_way_mask = (1<<16) - 1;	/* Bitmask of active ways */
-
-	/* invalidate all ways */
-	writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_INV_WAY);
-	ux500_cache_wait(l2x0_base + L2X0_INV_WAY, l2x0_way_mask);
-	ux500_cache_sync();
-}
-
 static int __init ux500_l2x0_unlock(void)
 {
 	int i;
@@ -85,9 +47,13 @@ static int __init ux500_l2x0_init(void)
 	/* 64KB way size, 8 way associativity, force WA */
 	l2x0_init(l2x0_base, 0x3e060000, 0xc0000fff);
 
-	/* Override invalidate function */
-	outer_cache.disable = ux500_l2x0_disable;
-	outer_cache.inv_all = ux500_l2x0_inv_all;
+	/*
+	 * We can't disable l2 as we are in non secure mode, currently
+	 * this seems being called only during kexec path. So let's
+	 * override outer.disable with nasty assignment until we have
+	 * some SMI service available.
+	 */
+	outer_cache.disable = NULL;
 
 	return 0;
 }
-- 
1.7.4.3

[PATCH 1/2] mach-ux500: cache operations are atomic on PL310

[Will Deacon]

Hi Srinidhi,

On Tue, Jan 17, 2012 at 06:22:26AM +0000, Srinidhi KASAGAR wrote:
> On Mon, Jan 16, 2012 at 15:50:08 +0100, Will Deacon wrote:
> > The lock needs to stay. Besides, the problem isn't with inv_all, the problem
> > is with not being able to disable the outer cache. So can't you just do
> > something nasty like:
> > 
> > 	outer_cache.disable = NULL;
> > 
> > after your call to l2x0_init?
> 
> hmm..patch below

Thanks. Comments inline.

> > 
> > Also, if you can't disable the L2 from non-secure, does that mean that you
> > boot Linux with the L2 enabled?
> 
> Yes, we boot with L2 enabled.

Interesting. I'm surprised you don't have problems with stale data on the
D-side after the decompressor. Maybe you're lucky with the mappings being no
write allocate.

> From 46fdbda7d2d9a9f4df0933341bcc467b3bdd03d6 Mon Sep 17 00:00:00 2001
> From: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
> Date: Tue, 17 Jan 2012 11:29:39 +0530
> Subject: [PATCH] mach-ux500: Do not override outer.inv_all
> 
> outer.inv_all is currently being used only in kexec path.
> Invalidating outer cache without disabling it is a big
> nono, and so, remove the machine specific outer.inv_all
> assuming that kexec does not call inv_all in its path.

Please change this comment. Kexec doesn't do this anymore.

> And at the same time it does not prevent us overriding
> outer.disable as we do not have any such secure SMI to
> handle the same while kexec disables the outer cache.
> 
> Signed-off-by: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
> ---
>  arch/arm/mach-ux500/cache-l2x0.c |   48 +++++--------------------------------
>  1 files changed, 7 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/arm/mach-ux500/cache-l2x0.c b/arch/arm/mach-ux500/cache-l2x0.c
> index 122ddde..45111c8 100644
> --- a/arch/arm/mach-ux500/cache-l2x0.c
> +++ b/arch/arm/mach-ux500/cache-l2x0.c
> @@ -12,44 +12,6 @@
>  
>  static void __iomem *l2x0_base;
>  
> -static inline void ux500_cache_wait(void __iomem *reg, unsigned long mask)
> -{
> -	/* wait for the operation to complete */
> -	while (readl_relaxed(reg) & mask)
> -		cpu_relax();
> -}
> -
> -static inline void ux500_cache_sync(void)
> -{
> -	writel_relaxed(0, l2x0_base + L2X0_CACHE_SYNC);
> -	ux500_cache_wait(l2x0_base + L2X0_CACHE_SYNC, 1);
> -}
> -
> -/*
> - * The L2 cache cannot be turned off in the non-secure world.
> - * Dummy until a secure service is in place.
> - */
> -static void ux500_l2x0_disable(void)
> -{
> -}
> -
> -/*
> - * This is only called when doing a kexec, just after turning off the L2
> - * and L1 cache, and it is surrounded by a spinlock in the generic version.
> - * However, we're not really turning off the L2 cache right now and the
> - * PL310 does not support exclusive accesses (used to implement the spinlock).
> - * So, the invalidation needs to be done without the spinlock.
> - */
> -static void ux500_l2x0_inv_all(void)
> -{
> -	uint32_t l2x0_way_mask = (1<<16) - 1;	/* Bitmask of active ways */
> -
> -	/* invalidate all ways */
> -	writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_INV_WAY);
> -	ux500_cache_wait(l2x0_base + L2X0_INV_WAY, l2x0_way_mask);
> -	ux500_cache_sync();
> -}
> -
>  static int __init ux500_l2x0_unlock(void)
>  {
>  	int i;
> @@ -85,9 +47,13 @@ static int __init ux500_l2x0_init(void)
>  	/* 64KB way size, 8 way associativity, force WA */
>  	l2x0_init(l2x0_base, 0x3e060000, 0xc0000fff);
>  
> -	/* Override invalidate function */
> -	outer_cache.disable = ux500_l2x0_disable;
> -	outer_cache.inv_all = ux500_l2x0_inv_all;
> +	/*
> +	 * We can't disable l2 as we are in non secure mode, currently
> +	 * this seems being called only during kexec path. So let's
> +	 * override outer.disable with nasty assignment until we have
> +	 * some SMI service available.
> +	 */
> +	outer_cache.disable = NULL;

Much better!

Cheers,

Will

[PATCH 1/2] mach-ux500: cache operations are atomic on PL310

[Srinidhi KASAGAR]

On Tue, Jan 17, 2012 at 11:30:23 +0100, Will Deacon wrote:
> Hi Srinidhi,
> 
> On Tue, Jan 17, 2012 at 06:22:26AM +0000, Srinidhi KASAGAR wrote:
> > On Mon, Jan 16, 2012 at 15:50:08 +0100, Will Deacon wrote:
> > > The lock needs to stay. Besides, the problem isn't with inv_all, the problem
> > > is with not being able to disable the outer cache. So can't you just do
> > > something nasty like:
> > > 
> > > 	outer_cache.disable = NULL;
> > > 
> > > after your call to l2x0_init?
> > 
> > hmm..patch below
> 
> Thanks. Comments inline.
> 
> > > 
> > > Also, if you can't disable the L2 from non-secure, does that mean that you
> > > boot Linux with the L2 enabled?
> > 
> > Yes, we boot with L2 enabled.
> 
> Interesting. I'm surprised you don't have problems with stale data on the
> D-side after the decompressor. Maybe you're lucky with the mappings being no
> write allocate.

Actually the pre bootloader makes sure that L2 is clean before relinquishing
the control to kernel decompressor.

> 
> > From 46fdbda7d2d9a9f4df0933341bcc467b3bdd03d6 Mon Sep 17 00:00:00 2001
> > From: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
> > Date: Tue, 17 Jan 2012 11:29:39 +0530
> > Subject: [PATCH] mach-ux500: Do not override outer.inv_all
> > 
> > outer.inv_all is currently being used only in kexec path.
> > Invalidating outer cache without disabling it is a big
> > nono, and so, remove the machine specific outer.inv_all
> > assuming that kexec does not call inv_all in its path.
> 
> Please change this comment. Kexec doesn't do this anymore.

Done, patch below.


>From a8cf7e2f7b58c3ef860be58fa72cd0c51e2be487 Mon Sep 17 00:00:00 2001
From: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
Date: Tue, 17 Jan 2012 11:29:39 +0530
Subject: [PATCH] mach-ux500: Do not override outer.inv_all

Invalidating outer cache without disabling it is a big
nono, and so, remove the machine specific outer.inv_all

And at the same time it does not prevent us overriding
outer.disable as we do not have any such secure SMI to
handle the same while kexec disables the outer cache.

Signed-off-by: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
---
 arch/arm/mach-ux500/cache-l2x0.c |   48 +++++--------------------------------
 1 files changed, 7 insertions(+), 41 deletions(-)

diff --git a/arch/arm/mach-ux500/cache-l2x0.c b/arch/arm/mach-ux500/cache-l2x0.c
index 122ddde..45111c8 100644
--- a/arch/arm/mach-ux500/cache-l2x0.c
+++ b/arch/arm/mach-ux500/cache-l2x0.c
@@ -12,44 +12,6 @@
 
 static void __iomem *l2x0_base;
 
-static inline void ux500_cache_wait(void __iomem *reg, unsigned long mask)
-{
-	/* wait for the operation to complete */
-	while (readl_relaxed(reg) & mask)
-		cpu_relax();
-}
-
-static inline void ux500_cache_sync(void)
-{
-	writel_relaxed(0, l2x0_base + L2X0_CACHE_SYNC);
-	ux500_cache_wait(l2x0_base + L2X0_CACHE_SYNC, 1);
-}
-
-/*
- * The L2 cache cannot be turned off in the non-secure world.
- * Dummy until a secure service is in place.
- */
-static void ux500_l2x0_disable(void)
-{
-}
-
-/*
- * This is only called when doing a kexec, just after turning off the L2
- * and L1 cache, and it is surrounded by a spinlock in the generic version.
- * However, we're not really turning off the L2 cache right now and the
- * PL310 does not support exclusive accesses (used to implement the spinlock).
- * So, the invalidation needs to be done without the spinlock.
- */
-static void ux500_l2x0_inv_all(void)
-{
-	uint32_t l2x0_way_mask = (1<<16) - 1;	/* Bitmask of active ways */
-
-	/* invalidate all ways */
-	writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_INV_WAY);
-	ux500_cache_wait(l2x0_base + L2X0_INV_WAY, l2x0_way_mask);
-	ux500_cache_sync();
-}
-
 static int __init ux500_l2x0_unlock(void)
 {
 	int i;
@@ -85,9 +47,13 @@ static int __init ux500_l2x0_init(void)
 	/* 64KB way size, 8 way associativity, force WA */
 	l2x0_init(l2x0_base, 0x3e060000, 0xc0000fff);
 
-	/* Override invalidate function */
-	outer_cache.disable = ux500_l2x0_disable;
-	outer_cache.inv_all = ux500_l2x0_inv_all;
+	/*
+	 * We can't disable l2 as we are in non secure mode, currently
+	 * this seems being called only during kexec path. So let's
+	 * override outer.disable with nasty assignment until we have
+	 * some SMI service available.
+	 */
+	outer_cache.disable = NULL;
 
 	return 0;
 }
-- 
1.7.4.3

[PATCH 1/2] mach-ux500: cache operations are atomic on PL310

[Will Deacon]

On Tue, Jan 17, 2012 at 11:27:43AM +0000, Srinidhi KASAGAR wrote:
> On Tue, Jan 17, 2012 at 11:30:23 +0100, Will Deacon wrote:
> > On Tue, Jan 17, 2012 at 06:22:26AM +0000, Srinidhi KASAGAR wrote:
> > > 
> > > Yes, we boot with L2 enabled.
> > 
> > Interesting. I'm surprised you don't have problems with stale data on the
> > D-side after the decompressor. Maybe you're lucky with the mappings being no
> > write allocate.
> 
> Actually the pre bootloader makes sure that L2 is clean before relinquishing
> the control to kernel decompressor.

I was thinking more of the decompressor populating the L2 and not flushing
it.

> From a8cf7e2f7b58c3ef860be58fa72cd0c51e2be487 Mon Sep 17 00:00:00 2001
> From: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
> Date: Tue, 17 Jan 2012 11:29:39 +0530
> Subject: [PATCH] mach-ux500: Do not override outer.inv_all
> 
> Invalidating outer cache without disabling it is a big
> nono, and so, remove the machine specific outer.inv_all
> 
> And at the same time it does not prevent us overriding
> outer.disable as we do not have any such secure SMI to
> handle the same while kexec disables the outer cache.
> 
> Signed-off-by: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
> ---
>  arch/arm/mach-ux500/cache-l2x0.c |   48 +++++--------------------------------
>  1 files changed, 7 insertions(+), 41 deletions(-)
> 
> diff --git a/arch/arm/mach-ux500/cache-l2x0.c b/arch/arm/mach-ux500/cache-l2x0.c
> index 122ddde..45111c8 100644
> --- a/arch/arm/mach-ux500/cache-l2x0.c
> +++ b/arch/arm/mach-ux500/cache-l2x0.c
> @@ -12,44 +12,6 @@
>  
>  static void __iomem *l2x0_base;
>  
> -static inline void ux500_cache_wait(void __iomem *reg, unsigned long mask)
> -{
> -	/* wait for the operation to complete */
> -	while (readl_relaxed(reg) & mask)
> -		cpu_relax();
> -}
> -
> -static inline void ux500_cache_sync(void)
> -{
> -	writel_relaxed(0, l2x0_base + L2X0_CACHE_SYNC);
> -	ux500_cache_wait(l2x0_base + L2X0_CACHE_SYNC, 1);
> -}
> -
> -/*
> - * The L2 cache cannot be turned off in the non-secure world.
> - * Dummy until a secure service is in place.
> - */
> -static void ux500_l2x0_disable(void)
> -{
> -}
> -
> -/*
> - * This is only called when doing a kexec, just after turning off the L2
> - * and L1 cache, and it is surrounded by a spinlock in the generic version.
> - * However, we're not really turning off the L2 cache right now and the
> - * PL310 does not support exclusive accesses (used to implement the spinlock).
> - * So, the invalidation needs to be done without the spinlock.
> - */
> -static void ux500_l2x0_inv_all(void)
> -{
> -	uint32_t l2x0_way_mask = (1<<16) - 1;	/* Bitmask of active ways */
> -
> -	/* invalidate all ways */
> -	writel_relaxed(l2x0_way_mask, l2x0_base + L2X0_INV_WAY);
> -	ux500_cache_wait(l2x0_base + L2X0_INV_WAY, l2x0_way_mask);
> -	ux500_cache_sync();
> -}
> -
>  static int __init ux500_l2x0_unlock(void)
>  {
>  	int i;
> @@ -85,9 +47,13 @@ static int __init ux500_l2x0_init(void)
>  	/* 64KB way size, 8 way associativity, force WA */
>  	l2x0_init(l2x0_base, 0x3e060000, 0xc0000fff);
>  
> -	/* Override invalidate function */
> -	outer_cache.disable = ux500_l2x0_disable;
> -	outer_cache.inv_all = ux500_l2x0_inv_all;
> +	/*
> +	 * We can't disable l2 as we are in non secure mode, currently
> +	 * this seems being called only during kexec path. So let's

s/being/to be/

> +	 * override outer.disable with nasty assignment until we have
> +	 * some SMI service available.
> +	 */
> +	outer_cache.disable = NULL;

This looks alright to me.

Reviewed-by: Will Deacon <will.deacon@arm.com>

Will

[PATCH 1/2] mach-ux500: cache operations are atomic on PL310

[Linus Walleij]

On Tue, Jan 17, 2012 at 2:34 PM, Will Deacon <will.deacon@arm.com> wrote:
> On Tue, Jan 17, 2012 at 11:27:43AM +0000, Srinidhi KASAGAR wrote:

>> From a8cf7e2f7b58c3ef860be58fa72cd0c51e2be487 Mon Sep 17 00:00:00 2001
>> From: srinidhi kasagar <srinidhi.kasagar@stericsson.com>
>> Date: Tue, 17 Jan 2012 11:29:39 +0530
>> Subject: [PATCH] mach-ux500: Do not override outer.inv_all
>>
>> + ? ? /*
>> + ? ? ?* We can't disable l2 as we are in non secure mode, currently
>> + ? ? ?* this seems being called only during kexec path. So let's
>
> s/being/to be/
>
>> + ? ? ?* override outer.disable with nasty assignment until we have
>> + ? ? ?* some SMI service available.
>> + ? ? ?*/
>> + ? ? outer_cache.disable = NULL;
>
> This looks alright to me.
>
> Reviewed-by: Will Deacon <will.deacon@arm.com>

Thanks guys, applied to ux500-core with the speling fix above,
I will send this for the -rc series.

Yours,
Linus Walleij

[PATCH 1/2] mach-ux500: cache operations are atomic on PL310

[Srinidhi Kasagar]

On Tue, Jan 17, 2012 at 14:34:28 +0100, Will Deacon wrote:
> On Tue, Jan 17, 2012 at 11:27:43AM +0000, Srinidhi KASAGAR wrote:
> > On Tue, Jan 17, 2012 at 11:30:23 +0100, Will Deacon wrote:
> > > On Tue, Jan 17, 2012 at 06:22:26AM +0000, Srinidhi KASAGAR wrote:
> > > > 
> > > > Yes, we boot with L2 enabled.
> > > 
> > > Interesting. I'm surprised you don't have problems with stale data on the
> > > D-side after the decompressor. Maybe you're lucky with the mappings being no
> > > write allocate.
> > 
> > Actually the pre bootloader makes sure that L2 is clean before relinquishing
> > the control to kernel decompressor.
> 
> I was thinking more of the decompressor populating the L2 and not flushing
> it.

Perhaps, we are lucky because we lock down l2 in bootloader?

srinidhi