Skip to contentSkip to conversation

ARM: l2x0: make sure I&D are not locked down on init

PATCH Superseded Linus Walleij 3 participants 4 messages Sep 4, 2011 Arm (32-bit)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Linus Walleij

author
#0 lore raw

PATCH 1/1 ARM: l2x0: make sure I&D are not locked down on init

From: Linus Walleij <[email protected]>

Fighting unfixed U-Boots and other beasts that may the cache in a locked-down state when starting the kernel, we make sure to disable all cache lock-down when initializing the l2x0 so we are in a known state.

Cc Srinidhi Kasagar <[email protected]>
Cc Rabin Vincent <[email protected]>
Cc Adrian Bunk <[email protected]>
Cc Rob Herring <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Reported-by Jan Rinze <[email protected]>
Tested-by Robert Marklund <[email protected]>
Signed-off-by Linus Walleij <[email protected]>
---
 arch/arm/mm/cache-l2x0.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
arch/arm/mm/cache-l2x0.c +4 -0
index 44c0867..b03c835 100644
@@ -338,6 +338,10 @@ void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
 		writel_relaxed(1, l2x0_base + L2X0_CTRL);
 	}
 
+	/* Make sure that I&D is not locked down when starting */
+	writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_D);
+	writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_I);
+
 	outer_cache.inv_range = l2x0_inv_range;
 	outer_cache.clean_range = l2x0_clean_range;
 	outer_cache.flush_range = l2x0_flush_range;
Signature · 1 line
1.7.3.2
#1 lore raw

PATCH 1/1 ARM: l2x0: make sure I&D are not locked down on init

Quoted 25 lines · Linus Walleij

From: Linus Walleij<[email protected]>

Fighting unfixed U-Boots and other beasts that may the cache in a locked-down state when starting the kernel, we make sure to disable all cache lock-down when initializing the l2x0 so we are in a known state.

Cc Srinidhi Kasagar<[email protected]>
Cc Rabin Vincent<[email protected]>
Cc Adrian Bunk<[email protected]>
Cc Rob Herring<[email protected]>
Cc Catalin Marinas<[email protected]>
Cc Will Deacon<[email protected]>
Reported-by Jan Rinze<[email protected]>
Tested-by Robert Marklund<[email protected]>
Signed-off-by Linus Walleij<[email protected]>
---
  arch/arm/mm/cache-l2x0.c |    4 ++++
  1 files changed, 4 insertions(+), 0 deletions(-)
arch/arm/mm/cache-l2x0.c +0 -0
index 44c0867..b03c835 100644

arch/arm/mm/cache-l2x0.c void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask) · line 344

Quoted 7 lines
arch/arm/mm/cache-l2x0.c +4 -0
@@ -338,6 +338,10 @@ void __init l2x0_init(void __iomem *base, __u32 aux_val, __u32 aux_mask)
  		writel_relaxed(1, l2x0_base + L2X0_CTRL);
  	}
 
+	/* Make sure that I&D is not locked down when starting */
+	writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_D);
+	writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_I);
+

Few points on this patch Linus W.
1. You should do above before enabling the L2X0, so you need to move this up.

2. You are taking care of only one master. i.e CPU0. You need to do that for CPU1 and if there are more than two CPU's, then for them too.

3. Even after taking care of 1 and 2, the arch, which enable L2X0 controller using secure API,s would have already enabled L2X0, so they need to do the above in arch code to be effective and correct. That should be fine though.

At least for you intended change below change should do. It's taking care of only two CPUs though.

Regards
Santosh

(quoted diff) +0 -0

b/arch/arm/include/asm/hardware/cache-l2x0.h index 16bd480..e04e947 100644

arch/arm/include/asm/hardware/cache-l2x0.h +4 -2
@@ -45,8 +45,10 @@
  #define L2X0_CLEAN_INV_LINE_PA		0x7F0
  #define L2X0_CLEAN_INV_LINE_IDX		0x7F8
  #define L2X0_CLEAN_INV_WAY		0x7FC
-#define L2X0_LOCKDOWN_WAY_D		0x900
-#define L2X0_LOCKDOWN_WAY_I		0x904
+#define L2X0_LOCKDOWN_WAY_D0		0x900
+#define L2X0_LOCKDOWN_WAY_D1		0x908
+#define L2X0_LOCKDOWN_WAY_I0		0x904
+#define L2X0_LOCKDOWN_WAY_I1		0x90C
  #define L2X0_TEST_OPERATION		0xF00
  #define L2X0_LINE_DATA			0xF10
  #define L2X0_LINE_TAG			0xF30
arch/arm/mm/cache-l2x0.c +0 -0
index 44c0867..f95b269 100644
@@ -322,6 +322,12 @@ void __init l2x0_init(void __iomem *base, __u32

aux_val, __u32 aux_mask)
way_size = 1 << (way_size + 3); l2x0_size = ways * way_size * SZ_1K;

+ /* Clear the I and D lock-down way registers */
+ writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_D0);
+ writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_D1);
+ writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_I0);
+ writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_I1);
+
/*
* Check if l2x0 controller is already enabled.
* If you are booting from non-secure mode

#2 lore raw

PATCH 1/1 ARM: l2x0: make sure I&D are not locked down on init

Hi Santosh,

Quoted 6 lines · Santosh
arch/arm/include/asm/hardware/cache-l2x0.h +0 -0
index 16bd480..e04e947 100644

arch/arm/include/asm/hardware/cache-l2x0.h line 51

Quoted 9 lines
arch/arm/include/asm/hardware/cache-l2x0.h +4 -2
@@ -45,8 +45,10 @@
  #define L2X0_CLEAN_INV_LINE_PA		0x7F0
  #define L2X0_CLEAN_INV_LINE_IDX		0x7F8
  #define L2X0_CLEAN_INV_WAY		0x7FC
-#define L2X0_LOCKDOWN_WAY_D		0x900
-#define L2X0_LOCKDOWN_WAY_I		0x904
+#define L2X0_LOCKDOWN_WAY_D0		0x900
+#define L2X0_LOCKDOWN_WAY_D1		0x908
+#define L2X0_LOCKDOWN_WAY_I0		0x904
+#define L2X0_LOCKDOWN_WAY_I1		0x90C

Maybe you could macroify these to take the CPU number as an argument?

Quoted 5 lines
arch/arm/mm/cache-l2x0.c +0 -0
index 44c0867..f95b269 100644

arch/arm/mm/cache-l2x0.c void __init l2x0_init(void __iomem *base, __u32 · line 330

Quoted 9 lines
arch/arm/mm/cache-l2x0.c +5 -0
@@ -322,6 +322,12 @@ void __init l2x0_init(void __iomem *base, __u32 
 aux_val, __u32 aux_mask)
  	way_size = 1 << (way_size + 3);
  	l2x0_size = ways * way_size * SZ_1K;
 
+	/* Clear the I and D lock-down way registers */
+	writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_D0);
+	writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_D1);
+	writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_I0);
+	writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_I1);

Then you could write this as a for loop from 0 to 7 for the PL310. For L210/220 I think there's just a single register pair.

Will

#3 lore raw

PATCH 1/1 ARM: l2x0: make sure I&D are not locked down on init

Quoted 12 lines · Will Deacon

Hi Santosh,

Quoted 9 lines · Santosh
arch/arm/include/asm/hardware/cache-l2x0.h +0 -0
index 16bd480..e04e947 100644
@@ -45,8 +45,10 @@
   #define L2X0_CLEAN_INV_LINE_PA		0x7F0
   #define L2X0_CLEAN_INV_LINE_IDX		0x7F8
   #define L2X0_CLEAN_INV_WAY		0x7FC
Quoted 9 lines

arch/arm/include/asm/hardware/cache-l2x0.h line 51

Quoted 6 lines
arch/arm/include/asm/hardware/cache-l2x0.h +4 -2
-#define L2X0_LOCKDOWN_WAY_D		0x900
-#define L2X0_LOCKDOWN_WAY_I		0x904
+#define L2X0_LOCKDOWN_WAY_D0		0x900
+#define L2X0_LOCKDOWN_WAY_D1		0x908
+#define L2X0_LOCKDOWN_WAY_I0		0x904
+#define L2X0_LOCKDOWN_WAY_I1		0x90C

Maybe you could macroify these to take the CPU number as an argument?

Should work as well.

Quoted 9 lines
Quoted 9 lines
arch/arm/mm/cache-l2x0.c +0 -0
index 44c0867..f95b269 100644
@@ -322,6 +322,12 @@ void __init l2x0_init(void __iomem *base, __u32
 aux_val, __u32 aux_mask)
   	way_size = 1<<  (way_size + 3);
   	l2x0_size = ways * way_size * SZ_1K;
Quoted 9 lines

arch/arm/mm/cache-l2x0.c void __init l2x0_init(void __iomem *base, __u32 · line 330

Quoted 5 lines
arch/arm/mm/cache-l2x0.c +5 -0
+	/* Clear the I and D lock-down way registers */
+	writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_D0);
+	writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_D1);
+	writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_I0);
+	writel_relaxed(0x0, l2x0_base + L2X0_LOCKDOWN_WAY_I1);

Then you could write this as a for loop from 0 to 7 for the PL310. For L210/220 I think there's just a single register pair.

Pl310 data is what I know and you are right. Loop is good idea considering we just want to clear them and not set any specific values.

Regards
Santosh