[PATCH 0/3] arm/arm64: a trio of boot mode detection fixes

[PATCH 0/3] arm/arm64: a trio of boot mode detection fixes

[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.

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(-)

-- 
1.8.1.1

[PATCH 1/3] arm: hyp: fix macro parameterisation

[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.

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

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Dave Martin <dave.martin@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Christoffer Dall <cdall@cs.columbia.edu>
---
 arch/arm/kernel/hyp-stub.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
index 1315c4c..bda76ce 100644
--- a/arch/arm/kernel/hyp-stub.S
+++ b/arch/arm/kernel/hyp-stub.S
@@ -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 */
-- 
1.8.1.1

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

[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 <mark.rutland@arm.com>
Cc: Dave Martin <dave.martin@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Christoffer Dall <cdall@cs.columbia.edu>
---
 arch/arm/include/asm/virt.h | 12 ++++++++++++
 arch/arm/kernel/setup.c     |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/arch/arm/include/asm/virt.h b/arch/arm/include/asm/virt.h
index 50af92b..4371f45 100644
--- a/arch/arm/include/asm/virt.h
+++ b/arch/arm/include/asm/virt.h
@@ -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
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index b4b1d39..4af9b38 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -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");
-- 
1.8.1.1

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

[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 <mark.rutland@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Christoffer Dall <cdall@cs.columbia.edu>
---
 arch/arm64/include/asm/virt.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/include/asm/virt.h b/arch/arm64/include/asm/virt.h
index 4398272..24f0ae9 100644
--- a/arch/arm64/include/asm/virt.h
+++ b/arch/arm64/include/asm/virt.h
@@ -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];
 }
 
-- 
1.8.1.1

[PATCH 1/3] arm: hyp: fix macro parameterisation

[Dave Martin]

On Tue, Jul 09, 2013 at 03:16:04PM +0100, Mark Rutland wrote:
> 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 <mark.rutland@arm.com>
> Cc: Dave Martin <dave.martin@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Christoffer Dall <cdall@cs.columbia.edu>

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

Acked-by: Dave Martin <Dave.Martin@arm.com>

> ---
>  arch/arm/kernel/hyp-stub.S | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/kernel/hyp-stub.S b/arch/arm/kernel/hyp-stub.S
> index 1315c4c..bda76ce 100644
> --- a/arch/arm/kernel/hyp-stub.S
> +++ b/arch/arm/kernel/hyp-stub.S
> @@ -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 */
> -- 
> 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

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

[Dave Martin]

On Tue, Jul 09, 2013 at 03:16:05PM +0100, Mark Rutland wrote:
> 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 <mark.rutland@arm.com>
> Cc: Dave Martin <dave.martin@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Christoffer Dall <cdall@cs.columbia.edu>

Acked-by: Dave Martin <Dave.Martin@arm.com>

> ---
>  arch/arm/include/asm/virt.h | 12 ++++++++++++
>  arch/arm/kernel/setup.c     |  2 ++
>  2 files changed, 14 insertions(+)
> 
> diff --git a/arch/arm/include/asm/virt.h b/arch/arm/include/asm/virt.h
> index 50af92b..4371f45 100644
> --- a/arch/arm/include/asm/virt.h
> +++ b/arch/arm/include/asm/virt.h
> @@ -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
> diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
> index b4b1d39..4af9b38 100644
> --- a/arch/arm/kernel/setup.c
> +++ b/arch/arm/kernel/setup.c
> @@ -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");
> -- 
> 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

[PATCH 0/3] arm/arm64: a trio of boot mode detection fixes

[Marc Zyngier]

On 09/07/13 15:16, Mark Rutland wrote:
> 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 <marc.zyngier@arm.com>

	M.

> 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(-)
> 


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

[PATCH 0/3] arm/arm64: a trio of boot mode detection fixes

[Mark Rutland]

On Thu, Jul 18, 2013 at 04:26:18PM +0100, Marc Zyngier wrote:
> On 09/07/13 15:16, Mark Rutland wrote:
> > 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 <marc.zyngier@arm.com>

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.

> 
> 	M.
> 
> > 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(-)
> > 
> 
> 
> -- 
> Jazz is not dead. It just smells funny...

[PATCH 0/3] arm/arm64: a trio of boot mode detection fixes

[Catalin Marinas]

On Thu, Jul 18, 2013 at 05:25:01PM +0100, Mark Rutland wrote:
> On Thu, Jul 18, 2013 at 04:26:18PM +0100, Marc Zyngier wrote:
> > On 09/07/13 15:16, Mark Rutland wrote:
> > > 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 <marc.zyngier@arm.com>
> 
> 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.

-- 
Catalin