arm64: fix invalidation of wrong __early_cpu_boot_status cacheline

In head.S, the str_l macro, which takes a source register, a symbol name and a temp register, is used to store a status value to the variable __early_cpu_boot_status. Subsequently, the value of the temp register is reused to invalidate any cachelines covering this variable.

However, since str_l resolves to

adrp \tmp, \sym
str \src, [\tmp, :lo12:\sym]

the temp register never actually holds the address of the variable but only of the 4 KB window that covers it, and reusing it leads to the wrong cacheline being invalidated. So instead, take the address explicitly before doing the store, and reuse that value to perform the cache invalidation.

Signed-off-by Ard Biesheuvel <[email protected]>

---

This deserves a cc stable, since the macro is always invoked for each CPU at boot.

Alternatively, we could add a writeback to the str_l macro, but I think in general, making any assumptions about the value of a temp register is dodgy, so I prefer this approach instead.

 arch/arm64/kernel/head.S | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

arch/arm64/kernel/head.S +2 -1

index 8b8a3d1e23fb..fafe21e49aab 100644
@@ -693,7 +693,8 @@ ENDPROC(__secondary_switched)
 
 	.macro	update_early_cpu_boot_status status, tmp1, tmp2
 	mov	\tmp2, #\status
-	str_l	\tmp2, __early_cpu_boot_status, \tmp1
+	adr_l	\tmp1, __early_cpu_boot_status
+	str	\tmp2, [\tmp1]
 	dmb	sy
 	dc	ivac, \tmp1			// Invalidate potentially stale cache line
 	.endm

Signature · 7 lines

2.5.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 19 lines · Ard Biesheuvel

In head.S, the str_l macro, which takes a source register, a symbol name and a temp register, is used to store a status value to the variable __early_cpu_boot_status. Subsequently, the value of the temp register is reused to invalidate any cachelines covering this variable.

However, since str_l resolves to

adrp \tmp, \sym
str \src, [\tmp, :lo12:\sym]

the temp register never actually holds the address of the variable but only of the 4 KB window that covers it, and reusing it leads to the wrong cacheline being invalidated. So instead, take the address explicitly before doing the store, and reuse that value to perform the cache invalidation.

Signed-off-by Ard Biesheuvel <[email protected]>

---

Quoted 2 lines

This deserves a cc stable, since the macro is always invoked for each CPU at boot.

Sounds good, also:

Fixes bb9052744f4b7ae1 ("arm64: Handle early CPU boot failures")

Quoted 20 lines · Mark Rutland

Quoted 19 lines · Ard Biesheuvel

In head.S, the str_l macro, which takes a source register, a symbol name and a temp register, is used to store a status value to the variable __early_cpu_boot_status. Subsequently, the value of the temp register is reused to invalidate any cachelines covering this variable.

However, since str_l resolves to

adrp \tmp, \sym
str \src, [\tmp, :lo12:\sym]

the temp register never actually holds the address of the variable but only of the 4 KB window that covers it, and reusing it leads to the wrong cacheline being invalidated. So instead, take the address explicitly before doing the store, and reuse that value to perform the cache invalidation.

Signed-off-by Ard Biesheuvel <[email protected]>

---

Quoted 6 lines

Quoted 2 lines

This deserves a cc stable, since the macro is always invoked for each CPU at boot.

Sounds good, also:

Fixes bb9052744f4b7ae1 ("arm64: Handle early CPU boot failures")

This first appeared in 4.6-rc1, so I don't think we need to Cc stable.

The initial patch was using adr_l as in this fix.

Catalin, was there a specific reason for that change ? I remember you mentioning something about it.