arm/arm64: a trio of boot mode detection fixes

PATCH New Mark Rutland 4 participants 9 messages Jul 9, 2013 – Jul 23, 2013

Subsystems (2)

Arm (32-bit) arm64

lore

Share & tools

lore

Mark Rutland

author reviewer

2013-07-09 14:16 UTC #0 lore raw

cover arm/arm64: a trio of boot mode detection fixes

The following patches fix a couple of issues in the boot mode detection code where in the case of mismatched boot modes we can corrupt a small portion of memory or fail to detect the fact we've booted CPUs with mismatched modes due to the boot mode being logged incoherently.

Thanks,
Mark.

Mark Rutland (3):
arm: hyp: fix macro parameterisation
arm: virt: ensure visibility of __boot_cpu_mode
arm64: virt: ensure visibility of __boot_cpu_mode

 arch/arm/include/asm/virt.h   | 12 ++++++++++++
 arch/arm/kernel/hyp-stub.S    |  4 ++--
 arch/arm/kernel/setup.c       |  2 ++
 arch/arm64/include/asm/virt.h | 13 +++++++++++++
 4 files changed, 29 insertions(+), 2 deletions(-)

Signature · 1 line

1.8.1.1

Mark Rutland

author reviewer

2013-07-09 14:16 UTC #1 lore raw

PATCH 1/3 arm: hyp: fix macro parameterisation

Currently, compare_cpu_mode_with_primary uses a mixture of macro arguments and hardcoded registers, and does so incorrectly, as it stores (__boot_cpu_mode_offset | BOOT_CPU_MODE_MISMATCH) to (__boot_cpu_mode + &__boot_cpu_mode_offset), which could corrupt an arbitrary portion of memory.

This patch fixes up compare_cpu_mode_with_primary to use the macro arguments, correctly updating __boot_cpu_mode.

Signed-off-by Mark Rutland <[email protected]>

Cc Dave Martin <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Christoffer Dall <[email protected]>

---
 arch/arm/kernel/hyp-stub.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/kernel/hyp-stub.S +2 -2

index 1315c4c..bda76ce 100644
@@ -56,8 +56,8 @@ ENTRY(__boot_cpu_mode)
 	ldr	\reg3, [\reg2]
 	ldr	\reg1, [\reg2, \reg3]
 	cmp	\mode, \reg1		@ matches primary CPU boot mode?
-	orrne	r7, r7, #BOOT_CPU_MODE_MISMATCH
-	strne	r7, [r5, r6]		@ record what happened and give up
+	orrne	\reg1, \reg1, #BOOT_CPU_MODE_MISMATCH
+	strne	\reg1, [\reg2, \reg3]	@ record what happened and give up
 	.endm
 
 #else	/* ZIMAGE */

Signature · 1 line

1.8.1.1

Dave Martin

2013-07-09 16:29 UTC #2 lore raw

PATCH 1/3 arm: hyp: fix macro parameterisation

Quoted 6 lines · Mark Rutland

Currently, compare_cpu_mode_with_primary uses a mixture of macro arguments and hardcoded registers, and does so incorrectly, as it stores (__boot_cpu_mode_offset | BOOT_CPU_MODE_MISMATCH) to (__boot_cpu_mode + &__boot_cpu_mode_offset), which could corrupt an arbitrary portion of memory.

Quoted 7 lines

This patch fixes up compare_cpu_mode_with_primary to use the macro arguments, correctly updating __boot_cpu_mode.
Signed-off-by Mark Rutland <[email protected]>
Cc Dave Martin <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Christoffer Dall <[email protected]>

May bad originally my the looks of it. Thanks for spotting this.

Acked-by Dave Martin <[email protected]>

Quoted 27 lines

---
 arch/arm/kernel/hyp-stub.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

arch/arm/kernel/hyp-stub.S +2 -2

index 1315c4c..bda76ce 100644
@@ -56,8 +56,8 @@ ENTRY(__boot_cpu_mode)
 	ldr	\reg3, [\reg2]
 	ldr	\reg1, [\reg2, \reg3]
 	cmp	\mode, \reg1		@ matches primary CPU boot mode?
-	orrne	r7, r7, #BOOT_CPU_MODE_MISMATCH
-	strne	r7, [r5, r6]		@ record what happened and give up
+	orrne	\reg1, \reg1, #BOOT_CPU_MODE_MISMATCH
+	strne	\reg1, [\reg2, \reg3]	@ record what happened and give up
 	.endm
 
 #else	/* ZIMAGE */

Signature · 7 lines

1.8.1.1


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

Mark Rutland

author reviewer

2013-07-09 14:16 UTC #3 lore raw

PATCH 2/3 arm: virt: ensure visibility of __boot_cpu_mode

In reply to Mark Rutland

Secondary CPUs write to __boot_cpu_mode with caches disabled, and thus a cached value of __boot_cpu_mode may be incoherent with that in memory. This could lead to a failure to detect mismatched boot modes.

This patch adds flushing to ensure that writes by secondaries to __boot_cpu_mode are made visible before we test against it.

Signed-off-by Mark Rutland <[email protected]>

Cc Dave Martin <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Christoffer Dall <[email protected]>

---
 arch/arm/include/asm/virt.h | 12 ++++++++++++
 arch/arm/kernel/setup.c     |  2 ++
 2 files changed, 14 insertions(+)

arch/arm/include/asm/virt.h +12 -0

index 50af92b..4371f45 100644
@@ -29,6 +29,7 @@
 #define BOOT_CPU_MODE_MISMATCH	PSR_N_BIT
 
 #ifndef __ASSEMBLY__
+#include <asm/cacheflush.h>
 
 #ifdef CONFIG_ARM_VIRT_EXT
 /*

@@ -41,10 +42,21 @@
  */
 extern int __boot_cpu_mode;
 
+static inline void sync_boot_mode(void)
+{
+	/*
+	 * As secondaries write to __boot_cpu_mode with caches disabled, we
+	 * must flush the corresponding cache entries to ensure the visibility
+	 * of their writes.
+	 */
+	sync_cache_r(&__boot_cpu_mode);
+}
+
 void __hyp_set_vectors(unsigned long phys_vector_base);
 unsigned long __hyp_get_vectors(void);
 #else
 #define __boot_cpu_mode	(SVC_MODE)
+#define sync_boot_mode()
 #endif
 
 #ifndef ZIMAGE

arch/arm/kernel/setup.c +2 -0

index b4b1d39..4af9b38 100644
@@ -744,6 +744,8 @@ static int __init meminfo_cmp(const void *_a, const void *_b)
 void __init hyp_mode_check(void)
 {
 #ifdef CONFIG_ARM_VIRT_EXT
+	sync_boot_mode();
+
 	if (is_hyp_mode_available()) {
 		pr_info("CPU: All CPU(s) started in HYP mode.\n");
 		pr_info("CPU: Virtualization extensions available.\n");

Signature · 1 line

1.8.1.1

Dave Martin

2013-07-09 16:32 UTC #4 lore raw

PATCH 2/3 arm: virt: ensure visibility of __boot_cpu_mode

Quoted 11 lines · Mark Rutland

Secondary CPUs write to __boot_cpu_mode with caches disabled, and thus a cached value of __boot_cpu_mode may be incoherent with that in memory. This could lead to a failure to detect mismatched boot modes.
This patch adds flushing to ensure that writes by secondaries to __boot_cpu_mode are made visible before we test against it.
Signed-off-by Mark Rutland <[email protected]>
Cc Dave Martin <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Christoffer Dall <[email protected]>

Acked-by Dave Martin <[email protected]>

Quoted 60 lines

---
 arch/arm/include/asm/virt.h | 12 ++++++++++++
 arch/arm/kernel/setup.c     |  2 ++
 2 files changed, 14 insertions(+)

arch/arm/include/asm/virt.h +12 -0

index 50af92b..4371f45 100644
@@ -29,6 +29,7 @@
 #define BOOT_CPU_MODE_MISMATCH	PSR_N_BIT
 
 #ifndef __ASSEMBLY__
+#include <asm/cacheflush.h>
 
 #ifdef CONFIG_ARM_VIRT_EXT
 /*

@@ -41,10 +42,21 @@
  */
 extern int __boot_cpu_mode;
 
+static inline void sync_boot_mode(void)
+{
+	/*
+	 * As secondaries write to __boot_cpu_mode with caches disabled, we
+	 * must flush the corresponding cache entries to ensure the visibility
+	 * of their writes.
+	 */
+	sync_cache_r(&__boot_cpu_mode);
+}
+
 void __hyp_set_vectors(unsigned long phys_vector_base);
 unsigned long __hyp_get_vectors(void);
 #else
 #define __boot_cpu_mode	(SVC_MODE)
+#define sync_boot_mode()
 #endif
 
 #ifndef ZIMAGE

arch/arm/kernel/setup.c +2 -0

index b4b1d39..4af9b38 100644
@@ -744,6 +744,8 @@ static int __init meminfo_cmp(const void *_a, const void *_b)
 void __init hyp_mode_check(void)
 {
 #ifdef CONFIG_ARM_VIRT_EXT
+	sync_boot_mode();
+
 	if (is_hyp_mode_available()) {
 		pr_info("CPU: All CPU(s) started in HYP mode.\n");
 		pr_info("CPU: Virtualization extensions available.\n");

Signature · 7 lines

1.8.1.1


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

Mark Rutland

author reviewer

2013-07-09 14:16 UTC #5 lore raw

PATCH 3/3 arm64: virt: ensure visibility of __boot_cpu_mode

In reply to Mark Rutland

This patch adds flushing to ensure that writes by secondaries to __boot_cpu_mode are made visible before we test against it.

Signed-off-by Mark Rutland <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Christoffer Dall <[email protected]>

---
 arch/arm64/include/asm/virt.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

arch/arm64/include/asm/virt.h +13 -0

index 4398272..24f0ae9 100644
@@ -21,6 +21,7 @@
 #define BOOT_CPU_MODE_EL2	(0x0e12b007)
 
 #ifndef __ASSEMBLY__
+#include <asm/cacheflush.h>
 
 /*
  * __boot_cpu_mode records what mode CPUs were booted in.

@@ -36,9 +37,20 @@ extern u32 __boot_cpu_mode[2];
 void __hyp_set_vectors(phys_addr_t phys_vector_base);
 phys_addr_t __hyp_get_vectors(void);
 
+static inline void sync_boot_mode(void)
+{
+	/*
+	 * As secondaries write to __boot_cpu_mode with caches disabled, we
+	 * must flush the corresponding cache entries to ensure the visibility
+	 * of their writes.
+	 */
+	__flush_dcache_area(__boot_cpu_mode, sizeof(__boot_cpu_mode));
+}
+
 /* Reports the availability of HYP mode */
 static inline bool is_hyp_mode_available(void)
 {
+	sync_boot_mode();
 	return (__boot_cpu_mode[0] == BOOT_CPU_MODE_EL2 &&
 		__boot_cpu_mode[1] == BOOT_CPU_MODE_EL2);
 }

@@ -46,6 +58,7 @@ static inline bool is_hyp_mode_available(void)
 /* Check if the bootloader has booted CPUs in different modes */
 static inline bool is_hyp_mode_mismatched(void)
 {
+	sync_boot_mode();
 	return __boot_cpu_mode[0] != __boot_cpu_mode[1];
 }

Signature · 1 line

1.8.1.1

Marc Zyngier

2013-07-18 15:26 UTC #6 lore raw

cover arm/arm64: a trio of boot mode detection fixes

In reply to Mark Rutland

Quoted 4 lines · Mark Rutland

The following patches fix a couple of issues in the boot mode detection code where in the case of mismatched boot modes we can corrupt a small portion of memory or fail to detect the fact we've booted CPUs with mismatched modes due to the boot mode being logged incoherently.

Sorry for the delay in replying to this. The whole series looks good to me, so: Acked-by: Marc Zyngier <[email protected]>

M.

Quoted 14 lines

Thanks,
Mark.
Mark Rutland (3):
arm: hyp: fix macro parameterisation
arm: virt: ensure visibility of __boot_cpu_mode
arm64: virt: ensure visibility of __boot_cpu_mode
 arch/arm/include/asm/virt.h   | 12 ++++++++++++
 arch/arm/kernel/hyp-stub.S    |  4 ++--
 arch/arm/kernel/setup.c       |  2 ++
 arch/arm64/include/asm/virt.h | 13 +++++++++++++
 4 files changed, 29 insertions(+), 2 deletions(-)

Signature · 1 line

Jazz is not dead. It just smells funny...

Mark Rutland

author reviewer

2013-07-18 16:25 UTC #7 lore raw

cover arm/arm64: a trio of boot mode detection fixes

Quoted 8 lines · Marc Zyngier

Quoted 4 lines · Mark Rutland
The following patches fix a couple of issues in the boot mode detection code where in the case of mismatched boot modes we can corrupt a small portion of memory or fail to detect the fact we've booted CPUs with mismatched modes due to the boot mode being logged incoherently.
Sorry for the delay in replying to this. The whole series looks good to me, so: Acked-by: Marc Zyngier <[email protected]>

Cheers.

Russell,
I've dropped the first two patches into the patch system as 7786/1 and 7787/1.

Catalin,
Are you happy to pick up the third patch?

Thanks,
Mark.

Quoted 21 lines

	M.
Quoted 14 lines
Thanks,
Mark.
Mark Rutland (3):
arm: hyp: fix macro parameterisation
arm: virt: ensure visibility of __boot_cpu_mode
arm64: virt: ensure visibility of __boot_cpu_mode
 arch/arm/include/asm/virt.h   | 12 ++++++++++++
 arch/arm/kernel/hyp-stub.S    |  4 ++--
 arch/arm/kernel/setup.c       |  2 ++
 arch/arm64/include/asm/virt.h | 13 +++++++++++++
 4 files changed, 29 insertions(+), 2 deletions(-)
Signature · 1 line
Jazz is not dead. It just smells funny...

Catalin Marinas

maintainer

2013-07-23 10:01 UTC #8 lore raw

cover arm/arm64: a trio of boot mode detection fixes

Quoted 10 lines · Mark Rutland

Quoted 8 lines · Marc Zyngier
Quoted 4 lines · Mark Rutland
The following patches fix a couple of issues in the boot mode detection code where in the case of mismatched boot modes we can corrupt a small portion of memory or fail to detect the fact we've booted CPUs with mismatched modes due to the boot mode being logged incoherently.
Sorry for the delay in replying to this. The whole series looks good to me, so: Acked-by: Marc Zyngier <[email protected]>

Quoted 8 lines

Cheers.
Russell,
I've dropped the first two patches into the patch system as 7786/1 and 7787/1.
Catalin,
Are you happy to pick up the third patch?

Yes. Thanks.

Signature · 1 line

Catalin