All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] arm64: boot fixes/updates
@ 2015-03-13 16:14 Mark Rutland
  2015-03-13 16:14 ` [PATCH 1/4] arm64: apply alternatives for !SMP kernels Mark Rutland
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Mark Rutland @ 2015-03-13 16:14 UTC (permalink / raw)
  To: linux-arm-kernel

Hi all,

The following patches address a few boot related issues I spotted
recently.

The first patch fixes alternatives for !SMP kernels, the second fixes a
potential issue with boot mode detection, and the final patches give us
some better logging of the boot mode, fixing a latent bug in mismatch
detection (though until now this has not been a problem).

Ard, Marc, I guess our recent boot patches are all going to clash. Do we
need to sort that out between us or are we relying on the arm64
maintainers? ;)

Thanks,
Mark.

Mark Rutland (4):
  arm64: apply alternatives for !SMP kernels
  arm64: head.S: fix __boot_cpu_mode alignment
  arm64: fix hyp mode mismatch detection
  arm64: log CPU boot modes

 arch/arm64/Kconfig                |  4 ++++
 arch/arm64/include/asm/smp_plat.h |  2 ++
 arch/arm64/kernel/head.S          |  4 ++--
 arch/arm64/kernel/setup.c         | 25 +++++++++++++++++++++++++
 arch/arm64/kernel/smp.c           |  2 +-
 5 files changed, 34 insertions(+), 3 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/4] arm64: apply alternatives for !SMP kernels
  2015-03-13 16:14 [PATCH 0/4] arm64: boot fixes/updates Mark Rutland
@ 2015-03-13 16:14 ` Mark Rutland
  2015-03-17 16:01   ` Ard Biesheuvel
  2015-03-13 16:14 ` [PATCH 2/4] arm64: head.S: fix __boot_cpu_mode alignment Mark Rutland
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Mark Rutland @ 2015-03-13 16:14 UTC (permalink / raw)
  To: linux-arm-kernel

Currently we only perform alternative patching for kernels built with
CONFIG_SMP, as we call apply_alternatives_all() in smp.c, which is only
built for CONFIG_SMP. Thus !SMP kernels may not have necessary
alternatives patched in.

This patch ensures that we call apply_alternatives_all() once all CPUs
are booted, even for !SMP kernels, by having the smp_init_cpus() stub
call this for !SMP kernels via up_late_init. A new wrapper,
do_post_cpus_up_work, is added so we can hook other calls here later
(e.g. boot mode logging).

Fixes: e039ee4ee3fcf174 ("arm64: add alternative runtime patching")
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Andre Przywara <andre.przywara@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
---
 arch/arm64/Kconfig                |  4 ++++
 arch/arm64/include/asm/smp_plat.h |  2 ++
 arch/arm64/kernel/setup.c         | 12 ++++++++++++
 arch/arm64/kernel/smp.c           |  2 +-
 4 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1b8e973..0d46deb 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -470,6 +470,10 @@ config HOTPLUG_CPU
 
 source kernel/Kconfig.preempt
 
+config UP_LATE_INIT
+       def_bool y
+       depends on !SMP
+
 config HZ
 	int
 	default 100
diff --git a/arch/arm64/include/asm/smp_plat.h b/arch/arm64/include/asm/smp_plat.h
index 59e2823..8dcd61e 100644
--- a/arch/arm64/include/asm/smp_plat.h
+++ b/arch/arm64/include/asm/smp_plat.h
@@ -40,4 +40,6 @@ static inline u32 mpidr_hash_size(void)
 extern u64 __cpu_logical_map[NR_CPUS];
 #define cpu_logical_map(cpu)    __cpu_logical_map[cpu]
 
+void __init do_post_cpus_up_work(void);
+
 #endif /* __ASM_SMP_PLAT_H */
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index e8420f6..781f469 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -207,6 +207,18 @@ static void __init smp_build_mpidr_hash(void)
 }
 #endif
 
+void __init do_post_cpus_up_work(void)
+{
+	apply_alternatives_all();
+}
+
+#ifdef CONFIG_UP_LATE_INIT
+void __init up_late_init(void)
+{
+	do_post_cpus_up_work();
+}
+#endif /* CONFIG_UP_LATE_INIT */
+
 static void __init setup_processor(void)
 {
 	struct cpu_info *cpu_info;
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 328b8ce..4257369 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -309,7 +309,7 @@ void cpu_die(void)
 void __init smp_cpus_done(unsigned int max_cpus)
 {
 	pr_info("SMP: Total of %d processors activated.\n", num_online_cpus());
-	apply_alternatives_all();
+	do_post_cpus_up_work();
 }
 
 void __init smp_prepare_boot_cpu(void)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/4] arm64: head.S: fix __boot_cpu_mode alignment
  2015-03-13 16:14 [PATCH 0/4] arm64: boot fixes/updates Mark Rutland
  2015-03-13 16:14 ` [PATCH 1/4] arm64: apply alternatives for !SMP kernels Mark Rutland
@ 2015-03-13 16:14 ` Mark Rutland
  2015-03-13 16:14 ` [PATCH 3/4] arm64: fix hyp mode mismatch detection Mark Rutland
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2015-03-13 16:14 UTC (permalink / raw)
  To: linux-arm-kernel

We currently align the allocation of the __boot_cpu_mode data _after_
allocating the label. Thus the label may point to padding before the
data. While we seem to have gotten away with this so far, it's not a
good idea to rely on any existing alignment in the object.

Perform the alignment padding before allocating the label to avoid this
possibility.

Fixes: c218bca74eeafa2f ("arm64: Relax the kernel cache requirements for boot")
Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/head.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 8ce88e0..07f9305 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -585,8 +585,8 @@ ENDPROC(set_cpu_boot_mode_flag)
  * zeroing of .bss would clobber it.
  */
 	.pushsection	.data..cacheline_aligned
-ENTRY(__boot_cpu_mode)
 	.align	L1_CACHE_SHIFT
+ENTRY(__boot_cpu_mode)
 	.long	BOOT_CPU_MODE_EL2
 	.long	0
 	.popsection
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/4] arm64: fix hyp mode mismatch detection
  2015-03-13 16:14 [PATCH 0/4] arm64: boot fixes/updates Mark Rutland
  2015-03-13 16:14 ` [PATCH 1/4] arm64: apply alternatives for !SMP kernels Mark Rutland
  2015-03-13 16:14 ` [PATCH 2/4] arm64: head.S: fix __boot_cpu_mode alignment Mark Rutland
@ 2015-03-13 16:14 ` Mark Rutland
  2015-03-13 20:21   ` Ard Biesheuvel
  2015-03-17 15:58   ` Ard Biesheuvel
  2015-03-13 16:14 ` [PATCH 4/4] arm64: log CPU boot modes Mark Rutland
  2015-03-14  7:59 ` [PATCH 0/4] arm64: boot fixes/updates Ard Biesheuvel
  4 siblings, 2 replies; 14+ messages in thread
From: Mark Rutland @ 2015-03-13 16:14 UTC (permalink / raw)
  To: linux-arm-kernel

Commit 828e9834e9a5b7e6 ("arm64: head: create a new function for setting
the boot_cpu_mode flag") added BOOT_CPU_MODE_EL1, a nonzero value
replacing uses of zero. However it failed to update __boot_cpu_mode
appropriately.

A CPU booted at EL2 writes BOOT_CPU_MODE_EL2 to __boot_cpu_mode[0], and
a CPU booted at EL1 writes BOOT_CPU_MODE_EL1 to __boot_cpu_mode[1].
Later is_hyp_mode_mismatched() determines there to be a mismatch if
__boot_cpu_mode[0] != __boot_cpu_mode[1].

If all CPUs are booted at EL1, __boot_cpu_mode[0] will be set to
BOOT_CPU_MODE_EL1, but __boot_cpu_mode[1] will retain its initial value
of zero, and is_hyp_mode_mismatched will erroneously determine that the
boot modes are mismatched. This hasn't been a problem so far, but later
patches which will make use of is_hyp_mode_mismatched() expect it to
work correctly.

This patch initialises __boot_cpu_mode[1] to BOOT_CPU_MODE_EL1, fixing
the erroneous mismatch detection when all CPUs are booted at EL1.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/head.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 07f9305..d17649d 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -588,7 +588,7 @@ ENDPROC(set_cpu_boot_mode_flag)
 	.align	L1_CACHE_SHIFT
 ENTRY(__boot_cpu_mode)
 	.long	BOOT_CPU_MODE_EL2
-	.long	0
+	.long	BOOT_CPU_MODE_EL1
 	.popsection
 
 #ifdef CONFIG_SMP
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/4] arm64: log CPU boot modes
  2015-03-13 16:14 [PATCH 0/4] arm64: boot fixes/updates Mark Rutland
                   ` (2 preceding siblings ...)
  2015-03-13 16:14 ` [PATCH 3/4] arm64: fix hyp mode mismatch detection Mark Rutland
@ 2015-03-13 16:14 ` Mark Rutland
  2015-03-17 15:58   ` Ard Biesheuvel
  2015-03-14  7:59 ` [PATCH 0/4] arm64: boot fixes/updates Ard Biesheuvel
  4 siblings, 1 reply; 14+ messages in thread
From: Mark Rutland @ 2015-03-13 16:14 UTC (permalink / raw)
  To: linux-arm-kernel

We currently don't log the boot mode for arm64 as we do for arm, and
without KVM the user is provided with no indication as to which mode(s)
CPUs were booted in, which can seriously hinder debugging in some cases.

Add logging to the boot path once all CPUs are up. Where CPUs are
mismatched in violation of the boot protocol, WARN and set a taint (as
we do for CPU other CPU feature mismatches) given that the
firmware/bootloader is buggy and should be fixed.

Signed-off-by: Mark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Marc Zyngier <marc.zyngier@arm.com>
Cc: Will Deacon <will.deacon@arm.com>
---
 arch/arm64/kernel/setup.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 781f469..1480894 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -62,6 +62,7 @@
 #include <asm/memblock.h>
 #include <asm/psci.h>
 #include <asm/efi.h>
+#include <asm/virt.h>
 
 unsigned int processor_id;
 EXPORT_SYMBOL(processor_id);
@@ -207,8 +208,20 @@ static void __init smp_build_mpidr_hash(void)
 }
 #endif
 
+static void __init hyp_mode_check(void)
+{
+	if (is_hyp_mode_available())
+		pr_info("CPU: All CPU(s) started at EL2\n");
+	else if (is_hyp_mode_mismatched())
+		WARN_TAINT(1, TAINT_CPU_OUT_OF_SPEC,
+			   "CPU: CPUs started in inconsistent modes");
+	else
+		pr_info("CPU: All CPU(s) started at EL1\n");
+}
+
 void __init do_post_cpus_up_work(void)
 {
+	hyp_mode_check();
 	apply_alternatives_all();
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/4] arm64: fix hyp mode mismatch detection
  2015-03-13 16:14 ` [PATCH 3/4] arm64: fix hyp mode mismatch detection Mark Rutland
@ 2015-03-13 20:21   ` Ard Biesheuvel
  2015-03-16 10:56     ` Mark Rutland
  2015-03-17 15:58   ` Ard Biesheuvel
  1 sibling, 1 reply; 14+ messages in thread
From: Ard Biesheuvel @ 2015-03-13 20:21 UTC (permalink / raw)
  To: linux-arm-kernel

On 13 March 2015 at 17:14, Mark Rutland <mark.rutland@arm.com> wrote:
> Commit 828e9834e9a5b7e6 ("arm64: head: create a new function for setting
> the boot_cpu_mode flag") added BOOT_CPU_MODE_EL1, a nonzero value
> replacing uses of zero. However it failed to update __boot_cpu_mode
> appropriately.
>
> A CPU booted at EL2 writes BOOT_CPU_MODE_EL2 to __boot_cpu_mode[0], and
> a CPU booted at EL1 writes BOOT_CPU_MODE_EL1 to __boot_cpu_mode[1].
> Later is_hyp_mode_mismatched() determines there to be a mismatch if
> __boot_cpu_mode[0] != __boot_cpu_mode[1].
>
> If all CPUs are booted at EL1, __boot_cpu_mode[0] will be set to
> BOOT_CPU_MODE_EL1, but __boot_cpu_mode[1] will retain its initial value
> of zero, and is_hyp_mode_mismatched will erroneously determine that the
> boot modes are mismatched. This hasn't been a problem so far, but later
> patches which will make use of is_hyp_mode_mismatched() expect it to
> work correctly.
>
> This patch initialises __boot_cpu_mode[1] to BOOT_CPU_MODE_EL1, fixing
> the erroneous mismatch detection when all CPUs are booted at EL1.
>

Maybe it's just me, but isn't it *much* easier to understand to
initialise both values to 0, and use 'both are non-zero' as the error
condition?
'HYP mode available' would then be '__boot_cpu_mode[0] ==
BOOT_CPU_MODE_EL2 && __boot_cpu_mode[1] == 0'


> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> ---
>  arch/arm64/kernel/head.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 07f9305..d17649d 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -588,7 +588,7 @@ ENDPROC(set_cpu_boot_mode_flag)
>         .align  L1_CACHE_SHIFT
>  ENTRY(__boot_cpu_mode)
>         .long   BOOT_CPU_MODE_EL2
> -       .long   0
> +       .long   BOOT_CPU_MODE_EL1
>         .popsection
>
>  #ifdef CONFIG_SMP
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 0/4] arm64: boot fixes/updates
  2015-03-13 16:14 [PATCH 0/4] arm64: boot fixes/updates Mark Rutland
                   ` (3 preceding siblings ...)
  2015-03-13 16:14 ` [PATCH 4/4] arm64: log CPU boot modes Mark Rutland
@ 2015-03-14  7:59 ` Ard Biesheuvel
  2015-03-16 10:53   ` Mark Rutland
  4 siblings, 1 reply; 14+ messages in thread
From: Ard Biesheuvel @ 2015-03-14  7:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 13 March 2015 at 17:14, Mark Rutland <mark.rutland@arm.com> wrote:
> Hi all,
>
> The following patches address a few boot related issues I spotted
> recently.
>
> The first patch fixes alternatives for !SMP kernels, the second fixes a
> potential issue with boot mode detection, and the final patches give us
> some better logging of the boot mode, fixing a latent bug in mismatch
> detection (though until now this has not been a problem).
>
> Ard, Marc, I guess our recent boot patches are all going to clash. Do we
> need to sort that out between us or are we relying on the arm64
> maintainers? ;)
>

Seems we spotted and fixed the same bug at the same time :-)
(but I failed to put you on cc)

http://article.gmane.org/gmane.linux.ports.arm.kernel/400100

I can stick all of our patches on a branch for Will to pull, if people
are ok with that?
I need another couple of acks though

-- 
Ard.


> Mark Rutland (4):
>   arm64: apply alternatives for !SMP kernels
>   arm64: head.S: fix __boot_cpu_mode alignment
>   arm64: fix hyp mode mismatch detection
>   arm64: log CPU boot modes
>
>  arch/arm64/Kconfig                |  4 ++++
>  arch/arm64/include/asm/smp_plat.h |  2 ++
>  arch/arm64/kernel/head.S          |  4 ++--
>  arch/arm64/kernel/setup.c         | 25 +++++++++++++++++++++++++
>  arch/arm64/kernel/smp.c           |  2 +-
>  5 files changed, 34 insertions(+), 3 deletions(-)
>
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 0/4] arm64: boot fixes/updates
  2015-03-14  7:59 ` [PATCH 0/4] arm64: boot fixes/updates Ard Biesheuvel
@ 2015-03-16 10:53   ` Mark Rutland
  0 siblings, 0 replies; 14+ messages in thread
From: Mark Rutland @ 2015-03-16 10:53 UTC (permalink / raw)
  To: linux-arm-kernel

On Sat, Mar 14, 2015 at 07:59:02AM +0000, Ard Biesheuvel wrote:
> On 13 March 2015 at 17:14, Mark Rutland <mark.rutland@arm.com> wrote:
> > Hi all,
> >
> > The following patches address a few boot related issues I spotted
> > recently.
> >
> > The first patch fixes alternatives for !SMP kernels, the second fixes a
> > potential issue with boot mode detection, and the final patches give us
> > some better logging of the boot mode, fixing a latent bug in mismatch
> > detection (though until now this has not been a problem).
> >
> > Ard, Marc, I guess our recent boot patches are all going to clash. Do we
> > need to sort that out between us or are we relying on the arm64
> > maintainers? ;)
> >
> 
> Seems we spotted and fixed the same bug at the same time :-)
> (but I failed to put you on cc)
> 
> http://article.gmane.org/gmane.linux.ports.arm.kernel/400100

Haha. Your patch looks good to me, so I've given it my Ack. :)

> I can stick all of our patches on a branch for Will to pull, if people
> are ok with that?

That sounds fine by me.

> I need another couple of acks though

Understood.

Mark.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/4] arm64: fix hyp mode mismatch detection
  2015-03-13 20:21   ` Ard Biesheuvel
@ 2015-03-16 10:56     ` Mark Rutland
  2015-03-16 11:03       ` Marc Zyngier
  0 siblings, 1 reply; 14+ messages in thread
From: Mark Rutland @ 2015-03-16 10:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Mar 13, 2015 at 08:21:19PM +0000, Ard Biesheuvel wrote:
> On 13 March 2015 at 17:14, Mark Rutland <mark.rutland@arm.com> wrote:
> > Commit 828e9834e9a5b7e6 ("arm64: head: create a new function for setting
> > the boot_cpu_mode flag") added BOOT_CPU_MODE_EL1, a nonzero value
> > replacing uses of zero. However it failed to update __boot_cpu_mode
> > appropriately.
> >
> > A CPU booted at EL2 writes BOOT_CPU_MODE_EL2 to __boot_cpu_mode[0], and
> > a CPU booted at EL1 writes BOOT_CPU_MODE_EL1 to __boot_cpu_mode[1].
> > Later is_hyp_mode_mismatched() determines there to be a mismatch if
> > __boot_cpu_mode[0] != __boot_cpu_mode[1].
> >
> > If all CPUs are booted at EL1, __boot_cpu_mode[0] will be set to
> > BOOT_CPU_MODE_EL1, but __boot_cpu_mode[1] will retain its initial value
> > of zero, and is_hyp_mode_mismatched will erroneously determine that the
> > boot modes are mismatched. This hasn't been a problem so far, but later
> > patches which will make use of is_hyp_mode_mismatched() expect it to
> > work correctly.
> >
> > This patch initialises __boot_cpu_mode[1] to BOOT_CPU_MODE_EL1, fixing
> > the erroneous mismatch detection when all CPUs are booted at EL1.
> >
> 
> Maybe it's just me, but isn't it *much* easier to understand to
> initialise both values to 0, and use 'both are non-zero' as the error
> condition?
> 'HYP mode available' would then be '__boot_cpu_mode[0] ==
> BOOT_CPU_MODE_EL2 && __boot_cpu_mode[1] == 0'

I agree that change would make this easier to follow.

Marc, are you happy with Ard's proposed change?

Mark.

> > Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> > Cc: Catalin Marinas <catalin.marinas@arm.com>
> > Cc: Marc Zyngier <marc.zyngier@arm.com>
> > Cc: Will Deacon <will.deacon@arm.com>
> > ---
> >  arch/arm64/kernel/head.S | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> > index 07f9305..d17649d 100644
> > --- a/arch/arm64/kernel/head.S
> > +++ b/arch/arm64/kernel/head.S
> > @@ -588,7 +588,7 @@ ENDPROC(set_cpu_boot_mode_flag)
> >         .align  L1_CACHE_SHIFT
> >  ENTRY(__boot_cpu_mode)
> >         .long   BOOT_CPU_MODE_EL2
> > -       .long   0
> > +       .long   BOOT_CPU_MODE_EL1
> >         .popsection
> >
> >  #ifdef CONFIG_SMP
> > --
> > 1.9.1
> >
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/4] arm64: fix hyp mode mismatch detection
  2015-03-16 10:56     ` Mark Rutland
@ 2015-03-16 11:03       ` Marc Zyngier
  2015-03-16 11:05         ` Ard Biesheuvel
  0 siblings, 1 reply; 14+ messages in thread
From: Marc Zyngier @ 2015-03-16 11:03 UTC (permalink / raw)
  To: linux-arm-kernel

On 16/03/15 10:56, Mark Rutland wrote:
> On Fri, Mar 13, 2015 at 08:21:19PM +0000, Ard Biesheuvel wrote:
>> On 13 March 2015 at 17:14, Mark Rutland <mark.rutland@arm.com> wrote:
>>> Commit 828e9834e9a5b7e6 ("arm64: head: create a new function for setting
>>> the boot_cpu_mode flag") added BOOT_CPU_MODE_EL1, a nonzero value
>>> replacing uses of zero. However it failed to update __boot_cpu_mode
>>> appropriately.
>>>
>>> A CPU booted at EL2 writes BOOT_CPU_MODE_EL2 to __boot_cpu_mode[0], and
>>> a CPU booted at EL1 writes BOOT_CPU_MODE_EL1 to __boot_cpu_mode[1].
>>> Later is_hyp_mode_mismatched() determines there to be a mismatch if
>>> __boot_cpu_mode[0] != __boot_cpu_mode[1].
>>>
>>> If all CPUs are booted at EL1, __boot_cpu_mode[0] will be set to
>>> BOOT_CPU_MODE_EL1, but __boot_cpu_mode[1] will retain its initial value
>>> of zero, and is_hyp_mode_mismatched will erroneously determine that the
>>> boot modes are mismatched. This hasn't been a problem so far, but later
>>> patches which will make use of is_hyp_mode_mismatched() expect it to
>>> work correctly.
>>>
>>> This patch initialises __boot_cpu_mode[1] to BOOT_CPU_MODE_EL1, fixing
>>> the erroneous mismatch detection when all CPUs are booted at EL1.
>>>
>>
>> Maybe it's just me, but isn't it *much* easier to understand to
>> initialise both values to 0, and use 'both are non-zero' as the error
>> condition?
>> 'HYP mode available' would then be '__boot_cpu_mode[0] ==
>> BOOT_CPU_MODE_EL2 && __boot_cpu_mode[1] == 0'
> 
> I agree that change would make this easier to follow.
> 
> Marc, are you happy with Ard's proposed change?

Absolutely. If that makes it more obvious, please go for it.

Thanks,

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/4] arm64: fix hyp mode mismatch detection
  2015-03-16 11:03       ` Marc Zyngier
@ 2015-03-16 11:05         ` Ard Biesheuvel
  0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2015-03-16 11:05 UTC (permalink / raw)
  To: linux-arm-kernel

On 16 March 2015 at 12:03, Marc Zyngier <marc.zyngier@arm.com> wrote:
> On 16/03/15 10:56, Mark Rutland wrote:
>> On Fri, Mar 13, 2015 at 08:21:19PM +0000, Ard Biesheuvel wrote:
>>> On 13 March 2015 at 17:14, Mark Rutland <mark.rutland@arm.com> wrote:
>>>> Commit 828e9834e9a5b7e6 ("arm64: head: create a new function for setting
>>>> the boot_cpu_mode flag") added BOOT_CPU_MODE_EL1, a nonzero value
>>>> replacing uses of zero. However it failed to update __boot_cpu_mode
>>>> appropriately.
>>>>
>>>> A CPU booted at EL2 writes BOOT_CPU_MODE_EL2 to __boot_cpu_mode[0], and
>>>> a CPU booted at EL1 writes BOOT_CPU_MODE_EL1 to __boot_cpu_mode[1].
>>>> Later is_hyp_mode_mismatched() determines there to be a mismatch if
>>>> __boot_cpu_mode[0] != __boot_cpu_mode[1].
>>>>
>>>> If all CPUs are booted at EL1, __boot_cpu_mode[0] will be set to
>>>> BOOT_CPU_MODE_EL1, but __boot_cpu_mode[1] will retain its initial value
>>>> of zero, and is_hyp_mode_mismatched will erroneously determine that the
>>>> boot modes are mismatched. This hasn't been a problem so far, but later
>>>> patches which will make use of is_hyp_mode_mismatched() expect it to
>>>> work correctly.
>>>>
>>>> This patch initialises __boot_cpu_mode[1] to BOOT_CPU_MODE_EL1, fixing
>>>> the erroneous mismatch detection when all CPUs are booted at EL1.
>>>>
>>>
>>> Maybe it's just me, but isn't it *much* easier to understand to
>>> initialise both values to 0, and use 'both are non-zero' as the error
>>> condition?
>>> 'HYP mode available' would then be '__boot_cpu_mode[0] ==
>>> BOOT_CPU_MODE_EL2 && __boot_cpu_mode[1] == 0'
>>
>> I agree that change would make this easier to follow.
>>
>> Marc, are you happy with Ard's proposed change?
>
> Absolutely. If that makes it more obvious, please go for it.
>
>

OK, thanks

Actually, simply checking for  __boot_cpu_mode[1] == 0 (i.e., no CPUs
booted at EL1) would be sufficient to implement 'is HYP mode
available'.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 4/4] arm64: log CPU boot modes
  2015-03-13 16:14 ` [PATCH 4/4] arm64: log CPU boot modes Mark Rutland
@ 2015-03-17 15:58   ` Ard Biesheuvel
  0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2015-03-17 15:58 UTC (permalink / raw)
  To: linux-arm-kernel

On 13 March 2015 at 17:14, Mark Rutland <mark.rutland@arm.com> wrote:
> We currently don't log the boot mode for arm64 as we do for arm, and
> without KVM the user is provided with no indication as to which mode(s)
> CPUs were booted in, which can seriously hinder debugging in some cases.
>
> Add logging to the boot path once all CPUs are up. Where CPUs are
> mismatched in violation of the boot protocol, WARN and set a taint (as
> we do for CPU other CPU feature mismatches) given that the
> firmware/bootloader is buggy and should be fixed.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>

Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  arch/arm64/kernel/setup.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 781f469..1480894 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -62,6 +62,7 @@
>  #include <asm/memblock.h>
>  #include <asm/psci.h>
>  #include <asm/efi.h>
> +#include <asm/virt.h>
>
>  unsigned int processor_id;
>  EXPORT_SYMBOL(processor_id);
> @@ -207,8 +208,20 @@ static void __init smp_build_mpidr_hash(void)
>  }
>  #endif
>
> +static void __init hyp_mode_check(void)
> +{
> +       if (is_hyp_mode_available())
> +               pr_info("CPU: All CPU(s) started at EL2\n");
> +       else if (is_hyp_mode_mismatched())
> +               WARN_TAINT(1, TAINT_CPU_OUT_OF_SPEC,
> +                          "CPU: CPUs started in inconsistent modes");
> +       else
> +               pr_info("CPU: All CPU(s) started at EL1\n");
> +}
> +
>  void __init do_post_cpus_up_work(void)
>  {
> +       hyp_mode_check();
>         apply_alternatives_all();
>  }
>
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 3/4] arm64: fix hyp mode mismatch detection
  2015-03-13 16:14 ` [PATCH 3/4] arm64: fix hyp mode mismatch detection Mark Rutland
  2015-03-13 20:21   ` Ard Biesheuvel
@ 2015-03-17 15:58   ` Ard Biesheuvel
  1 sibling, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2015-03-17 15:58 UTC (permalink / raw)
  To: linux-arm-kernel

On 13 March 2015 at 17:14, Mark Rutland <mark.rutland@arm.com> wrote:
> Commit 828e9834e9a5b7e6 ("arm64: head: create a new function for setting
> the boot_cpu_mode flag") added BOOT_CPU_MODE_EL1, a nonzero value
> replacing uses of zero. However it failed to update __boot_cpu_mode
> appropriately.
>
> A CPU booted at EL2 writes BOOT_CPU_MODE_EL2 to __boot_cpu_mode[0], and
> a CPU booted at EL1 writes BOOT_CPU_MODE_EL1 to __boot_cpu_mode[1].
> Later is_hyp_mode_mismatched() determines there to be a mismatch if
> __boot_cpu_mode[0] != __boot_cpu_mode[1].
>
> If all CPUs are booted at EL1, __boot_cpu_mode[0] will be set to
> BOOT_CPU_MODE_EL1, but __boot_cpu_mode[1] will retain its initial value
> of zero, and is_hyp_mode_mismatched will erroneously determine that the
> boot modes are mismatched. This hasn't been a problem so far, but later
> patches which will make use of is_hyp_mode_mismatched() expect it to
> work correctly.
>
> This patch initialises __boot_cpu_mode[1] to BOOT_CPU_MODE_EL1, fixing
> the erroneous mismatch detection when all CPUs are booted at EL1.
>
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Marc Zyngier <marc.zyngier@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>


Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


> ---
>  arch/arm64/kernel/head.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 07f9305..d17649d 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -588,7 +588,7 @@ ENDPROC(set_cpu_boot_mode_flag)
>         .align  L1_CACHE_SHIFT
>  ENTRY(__boot_cpu_mode)
>         .long   BOOT_CPU_MODE_EL2
> -       .long   0
> +       .long   BOOT_CPU_MODE_EL1
>         .popsection
>
>  #ifdef CONFIG_SMP
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/4] arm64: apply alternatives for !SMP kernels
  2015-03-13 16:14 ` [PATCH 1/4] arm64: apply alternatives for !SMP kernels Mark Rutland
@ 2015-03-17 16:01   ` Ard Biesheuvel
  0 siblings, 0 replies; 14+ messages in thread
From: Ard Biesheuvel @ 2015-03-17 16:01 UTC (permalink / raw)
  To: linux-arm-kernel

On 13 March 2015 at 17:14, Mark Rutland <mark.rutland@arm.com> wrote:
> Currently we only perform alternative patching for kernels built with
> CONFIG_SMP, as we call apply_alternatives_all() in smp.c, which is only
> built for CONFIG_SMP. Thus !SMP kernels may not have necessary
> alternatives patched in.
>
> This patch ensures that we call apply_alternatives_all() once all CPUs
> are booted, even for !SMP kernels, by having the smp_init_cpus() stub
> call this for !SMP kernels via up_late_init. A new wrapper,
> do_post_cpus_up_work, is added so we can hook other calls here later
> (e.g. boot mode logging).
>
> Fixes: e039ee4ee3fcf174 ("arm64: add alternative runtime patching")
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Andre Przywara <andre.przywara@arm.com>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>


Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>


> ---
>  arch/arm64/Kconfig                |  4 ++++
>  arch/arm64/include/asm/smp_plat.h |  2 ++
>  arch/arm64/kernel/setup.c         | 12 ++++++++++++
>  arch/arm64/kernel/smp.c           |  2 +-
>  4 files changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 1b8e973..0d46deb 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -470,6 +470,10 @@ config HOTPLUG_CPU
>
>  source kernel/Kconfig.preempt
>
> +config UP_LATE_INIT
> +       def_bool y
> +       depends on !SMP
> +
>  config HZ
>         int
>         default 100
> diff --git a/arch/arm64/include/asm/smp_plat.h b/arch/arm64/include/asm/smp_plat.h
> index 59e2823..8dcd61e 100644
> --- a/arch/arm64/include/asm/smp_plat.h
> +++ b/arch/arm64/include/asm/smp_plat.h
> @@ -40,4 +40,6 @@ static inline u32 mpidr_hash_size(void)
>  extern u64 __cpu_logical_map[NR_CPUS];
>  #define cpu_logical_map(cpu)    __cpu_logical_map[cpu]
>
> +void __init do_post_cpus_up_work(void);
> +
>  #endif /* __ASM_SMP_PLAT_H */
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index e8420f6..781f469 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -207,6 +207,18 @@ static void __init smp_build_mpidr_hash(void)
>  }
>  #endif
>
> +void __init do_post_cpus_up_work(void)
> +{
> +       apply_alternatives_all();
> +}
> +
> +#ifdef CONFIG_UP_LATE_INIT
> +void __init up_late_init(void)
> +{
> +       do_post_cpus_up_work();
> +}
> +#endif /* CONFIG_UP_LATE_INIT */
> +
>  static void __init setup_processor(void)
>  {
>         struct cpu_info *cpu_info;
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 328b8ce..4257369 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -309,7 +309,7 @@ void cpu_die(void)
>  void __init smp_cpus_done(unsigned int max_cpus)
>  {
>         pr_info("SMP: Total of %d processors activated.\n", num_online_cpus());
> -       apply_alternatives_all();
> +       do_post_cpus_up_work();
>  }
>
>  void __init smp_prepare_boot_cpu(void)
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-03-17 16:01 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-13 16:14 [PATCH 0/4] arm64: boot fixes/updates Mark Rutland
2015-03-13 16:14 ` [PATCH 1/4] arm64: apply alternatives for !SMP kernels Mark Rutland
2015-03-17 16:01   ` Ard Biesheuvel
2015-03-13 16:14 ` [PATCH 2/4] arm64: head.S: fix __boot_cpu_mode alignment Mark Rutland
2015-03-13 16:14 ` [PATCH 3/4] arm64: fix hyp mode mismatch detection Mark Rutland
2015-03-13 20:21   ` Ard Biesheuvel
2015-03-16 10:56     ` Mark Rutland
2015-03-16 11:03       ` Marc Zyngier
2015-03-16 11:05         ` Ard Biesheuvel
2015-03-17 15:58   ` Ard Biesheuvel
2015-03-13 16:14 ` [PATCH 4/4] arm64: log CPU boot modes Mark Rutland
2015-03-17 15:58   ` Ard Biesheuvel
2015-03-14  7:59 ` [PATCH 0/4] arm64: boot fixes/updates Ard Biesheuvel
2015-03-16 10:53   ` Mark Rutland

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.