ARM: vmlinux.lds.S: do not hardcode cacheline size as 32 bytes

To ensure correct alignment of cacheline-aligned data, the maximum cacheline size needs to be known at compile time.

Since Cortex-A8 and Cortex-A15 have 64-byte cachelines (and it is likely that there will be future ARMv7 implementations with the same line size) then it makes sense to assume that CPU_V7 implies a 64-byte L1 cacheline size. For CPUs with smaller caches, this will result in some harmless padding but will help with single zImage work and avoid hitting subtle bugs with misaligned data structures.

Signed-off-by Will Deacon <[email protected]>

---
?arch/arm/mm/Kconfig | ? ?2 +-
?1 files changed, 1 insertions(+), 1 deletions(-)

arch/arm/mm/Kconfig +0 -0

index 4cefb57..493e5ea5 100644
@@ -887,7 +887,7 @@ config ARM_L1_CACHE_SHIFT_6
 

?config ARM_L1_CACHE_SHIFT
? ? ? ?int
- ? ? ? default 6 if ARM_L1_CACHE_SHIFT_6
+ ? ? ? default 6 if ARM_L1_CACHE_SHIFT_6 || CPU_V7

Will,

Quoted 21 lines · Will Deacon

To ensure correct alignment of cacheline-aligned data, the maximum cacheline size needs to be known at compile time.

Since Cortex-A8 and Cortex-A15 have 64-byte cachelines (and it is likely that there will be future ARMv7 implementations with the same line size) then it makes sense to assume that CPU_V7 implies a 64-byte L1 cacheline size. For CPUs with smaller caches, this will result in some harmless padding but will help with single zImage work and avoid hitting subtle bugs with misaligned data structures.

Signed-off-by Will Deacon <[email protected]>

---
?arch/arm/mm/Kconfig | ? ?2 +-
?1 files changed, 1 insertions(+), 1 deletions(-)

arch/arm/mm/Kconfig +0 -0

index 4cefb57..493e5ea5 100644
@@ -887,7 +887,7 @@ config ARM_L1_CACHE_SHIFT_6
 

Quoted 4 lines

?config ARM_L1_CACHE_SHIFT
? ? ? ?int
- ? ? ? default 6 if ARM_L1_CACHE_SHIFT_6
+ ? ? ? default 6 if ARM_L1_CACHE_SHIFT_6 || CPU_V7

Will be really harmless on A9 ? We have L2 also to be considered here which hard codes the line size as 32.

Quoted 23 lines · Santosh

Will,

Quoted 21 lines · Will Deacon

To ensure correct alignment of cacheline-aligned data, the maximum cacheline size needs to be known at compile time.

Since Cortex-A8 and Cortex-A15 have 64-byte cachelines (and it is likely that there will be future ARMv7 implementations with the same line size) then it makes sense to assume that CPU_V7 implies a 64-byte L1 cacheline size. For CPUs with smaller caches, this will result in some harmless padding but will help with single zImage work and avoid hitting subtle bugs with misaligned data structures.

Signed-off-by Will Deacon <[email protected]>

---
?arch/arm/mm/Kconfig | ? ?2 +-
?1 files changed, 1 insertions(+), 1 deletions(-)

arch/arm/mm/Kconfig +0 -0

index 4cefb57..493e5ea5 100644
@@ -887,7 +887,7 @@ config ARM_L1_CACHE_SHIFT_6
 

Quoted 7 lines

Quoted 4 lines

?config ARM_L1_CACHE_SHIFT
? ? ? ?int
- ? ? ? default 6 if ARM_L1_CACHE_SHIFT_6
+ ? ? ? default 6 if ARM_L1_CACHE_SHIFT_6 || CPU_V7

Will be really harmless on A9 ? We have L2 also to be considered here which hard codes the line size as 32.

Going for a larger cache line size is safe: this is used for aligning data structures and similar, and aligning to 64-byte means that it's also 32-byte aligned.

Quoted 23 lines · Santosh

Will,

Quoted 21 lines · Will Deacon

To ensure correct alignment of cacheline-aligned data, the maximum cacheline size needs to be known at compile time.

Since Cortex-A8 and Cortex-A15 have 64-byte cachelines (and it is likely that there will be future ARMv7 implementations with the same line size) then it makes sense to assume that CPU_V7 implies a 64-byte L1 cacheline size. For CPUs with smaller caches, this will result in some harmless padding but will help with single zImage work and avoid hitting subtle bugs with misaligned data structures.

Signed-off-by Will Deacon <[email protected]>

---
?arch/arm/mm/Kconfig | ? ?2 +-
?1 files changed, 1 insertions(+), 1 deletions(-)

arch/arm/mm/Kconfig +0 -0

index 4cefb57..493e5ea5 100644
@@ -887,7 +887,7 @@ config ARM_L1_CACHE_SHIFT_6
 

Quoted 7 lines

Quoted 4 lines

?config ARM_L1_CACHE_SHIFT
? ? ? ?int
- ? ? ? default 6 if ARM_L1_CACHE_SHIFT_6
+ ? ? ? default 6 if ARM_L1_CACHE_SHIFT_6 || CPU_V7

Will be really harmless on A9 ? We have L2 also to be considered here which hard codes the line size as 32.

Going for a larger cache line size is safe: this is used for aligning data structures and similar, and aligning to 64-byte means that it's also 32-byte aligned.

The linker script assumes a cacheline size of 32 bytes when aligning the .data..cacheline_aligned and .data..percpu sections.

This patch updates the script to use L1_CACHE_BYTES, which should be set to 64 on platforms that require it.

Signed-off-by Will Deacon <[email protected]>

---

v2: incorporated suggestions from Stephen Boyd

Quoted 6 lines · Will Deacon

The linker script assumes a cacheline size of 32 bytes when aligning the .data..cacheline_aligned and .data..percpu sections.

This patch updates the script to use L1_CACHE_BYTES, which should be set to 64 on platforms that require it.

Quoted 4 lines

Signed-off-by Will Deacon <[email protected]>

---

v2: incorporated suggestions from Stephen Boyd

What do you think about aligning the exception fixup table to the same value?

Quoted 3 lines · Stephen Boyd

What do you think about aligning the exception fixup table to the same value?

Hmm, I'm not sure I see what that gains us over the current 32-byte alignment. Are you seeing any measurable performance difference with it being cacheline-aligned?

Quoted 4 lines · Will Deacon

Quoted 3 lines · Stephen Boyd

What do you think about aligning the exception fixup table to the same value?

Quoted 4 lines

Hmm, I'm not sure I see what that gains us over the current 32-byte alignment. Are you seeing any measurable performance difference with it being cacheline-aligned?

I haven't measured anything yet. I just see that it's another value hard-coded to 32 in the linker script and that tile has decided to use L1_CACHE_BYTES for it.

Quoted 5 lines · Stephen Boyd

Quoted 4 lines · Will Deacon

Quoted 3 lines · Stephen Boyd

What do you think about aligning the exception fixup table to the same value?

Quoted 8 lines

Quoted 4 lines

Hmm, I'm not sure I see what that gains us over the current 32-byte alignment. Are you seeing any measurable performance difference with it being cacheline-aligned?

I haven't measured anything yet. I just see that it's another value hard-coded to 32 in the linker script and that tile has decided to use L1_CACHE_BYTES for it.

Hmm, that's intriguing. The exception table entries are 8 bytes on ARM and are aligned as such by things like the USER macro, so I'm not sure why we align the section to 32 bytes. I guess it must be for performance reasons, but whether that's due to cacheline size, I don't know.

Nico, can you enlighten us please (it was introduced in 13b1f64c ("[ARM] 3008/1: the exception table is not read-only"))?

Quoted 10 lines · Will Deacon

Quoted 3 lines · Stephen Boyd

I haven't measured anything yet. I just see that it's another value hard-coded to 32 in the linker script and that tile has decided to use L1_CACHE_BYTES for it.

Hmm, that's intriguing. The exception table entries are 8 bytes on ARM and are aligned as such by things like the USER macro, so I'm not sure why we align the section to 32 bytes. I guess it must be for performance reasons, but whether that's due to cacheline size, I don't know.

Quoted 2 lines

Nico, can you enlighten us please (it was introduced in 13b1f64c ("[ARM] 3008/1: the exception table is not read-only"))?

It probably was due to the fact that the surrounding data was also cache line aligned. I don't think there is any particular reason why it should be aligned to a cache line boundary given that the list is accessed with a binary search. The minimum required alignment would be __alignof__(struct exception_table_entry).