[PATCH] ARM: avoid SMP DT initialisation on UP-only capable systems

[PATCH] ARM: avoid SMP DT initialisation on UP-only capable systems

[Russell King]

arm_dt_init_cpu_maps() initialises the CPU possible map even when the
boot CPU indicates that it is not SMP capable.  This can happen with
SoCs where the SoC may have a single UP-only CPU, or may have a pair of
SMP CPUs - and this is the only difference.

One solution is to increase the number of DT files by forcing peopl to
properly describe the hardware: this means all the iMX6DL DT files need
to be duplicated for iMX6S and a whole raft of updates to boot loaders
and the like.

Another solution is to decide that it is inappropriate to initialise the
cpu_possible map with anything but the boot CPU on non-SMP capable
systems.  This is implemented by this patch.

The situation currently may provoke a warning on earlier kernels,
printed during SMP bringup where we attempt to bring CPU1 online inspite
of CPU0 being a UP-only CPU, and this only failing due to the lack of
SMP operations.

Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
---
 arch/arm/kernel/devtree.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index 65addcbf5b30..bd72ce91d7a2 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -170,15 +170,18 @@ void __init arm_dt_init_cpu_maps(void)
 		return;
 	}
 
-	/*
-	 * Since the boot CPU node contains proper data, and all nodes have
-	 * a reg property, the DT CPU list can be considered valid and the
-	 * logical map created in smp_setup_processor_id() can be overridden
-	 */
-	for (i = 0; i < cpuidx; i++) {
-		set_cpu_possible(i, true);
-		cpu_logical_map(i) = tmp_map[i];
-		pr_debug("cpu logical map 0x%x\n", cpu_logical_map(i));
+	if (is_smp()) {
+		/*
+		 * Since the boot CPU node contains proper data, and all
+		 * nodes have a reg property, the DT CPU list can be
+		 * considered valid and the logical map created in
+		 * smp_setup_processor_id() can be overridden
+		 */
+		for (i = 0; i < cpuidx; i++) {
+			set_cpu_possible(i, true);
+			cpu_logical_map(i) = tmp_map[i];
+			pr_debug("cpu logical map 0x%x\n", cpu_logical_map(i));
+		}
 	}
 }
 
-- 
2.1.0

[PATCH] ARM: avoid SMP DT initialisation on UP-only capable systems

[Mark Rutland]

On Wed, Nov 25, 2015 at 10:43:02AM +0000, Russell King wrote:
> arm_dt_init_cpu_maps() initialises the CPU possible map even when the
> boot CPU indicates that it is not SMP capable.  This can happen with
> SoCs where the SoC may have a single UP-only CPU, or may have a pair of
> SMP CPUs - and this is the only difference.
> 
> One solution is to increase the number of DT files by forcing peopl to
> properly describe the hardware: this means all the iMX6DL DT files need
> to be duplicated for iMX6S and a whole raft of updates to boot loaders
> and the like.
>
> Another solution is to decide that it is inappropriate to initialise the
> cpu_possible map with anything but the boot CPU on non-SMP capable
> systems.  This is implemented by this patch.
> 
> The situation currently may provoke a warning on earlier kernels,
> printed during SMP bringup where we attempt to bring CPU1 online inspite
> of CPU0 being a UP-only CPU, and this only failing due to the lack of
> SMP operations.

I think that even if we work around this, we should have a warning
regarding the erroneous DT.

There are plenty of other things people may get wrong in their DT that
we may or may not be able to detect and/or workaround. We should
generally discourage relying on fixups and encourage people to do the
right thing.

> Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> ---
>  arch/arm/kernel/devtree.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
> index 65addcbf5b30..bd72ce91d7a2 100644
> --- a/arch/arm/kernel/devtree.c
> +++ b/arch/arm/kernel/devtree.c
> @@ -170,15 +170,18 @@ void __init arm_dt_init_cpu_maps(void)
>  		return;
>  	}
>  
> -	/*
> -	 * Since the boot CPU node contains proper data, and all nodes have
> -	 * a reg property, the DT CPU list can be considered valid and the
> -	 * logical map created in smp_setup_processor_id() can be overridden
> -	 */
> -	for (i = 0; i < cpuidx; i++) {
> -		set_cpu_possible(i, true);
> -		cpu_logical_map(i) = tmp_map[i];
> -		pr_debug("cpu logical map 0x%x\n", cpu_logical_map(i));
> +	if (is_smp()) {

Can we not change this to something like:

if (!is_smp() && cpuidx >1) {
	pr_warn("Boot CPU is UP, but DT contains multiple CPUs. Skipping!\n");
} else {

Other than that, this looks good to me.

If you're happy with the above:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

> +		/*
> +		 * Since the boot CPU node contains proper data, and all
> +		 * nodes have a reg property, the DT CPU list can be
> +		 * considered valid and the logical map created in
> +		 * smp_setup_processor_id() can be overridden
> +		 */
> +		for (i = 0; i < cpuidx; i++) {
> +			set_cpu_possible(i, true);
> +			cpu_logical_map(i) = tmp_map[i];
> +			pr_debug("cpu logical map 0x%x\n", cpu_logical_map(i));
> +		}
>  	}
>  }
>  
> -- 
> 2.1.0
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

[PATCH] ARM: avoid SMP DT initialisation on UP-only capable systems

[Russell King - ARM Linux]

On Wed, Nov 25, 2015 at 11:35:38AM +0000, Mark Rutland wrote:
> On Wed, Nov 25, 2015 at 10:43:02AM +0000, Russell King wrote:
> > arm_dt_init_cpu_maps() initialises the CPU possible map even when the
> > boot CPU indicates that it is not SMP capable.  This can happen with
> > SoCs where the SoC may have a single UP-only CPU, or may have a pair of
> > SMP CPUs - and this is the only difference.
> > 
> > One solution is to increase the number of DT files by forcing peopl to
> > properly describe the hardware: this means all the iMX6DL DT files need
> > to be duplicated for iMX6S and a whole raft of updates to boot loaders
> > and the like.
> >
> > Another solution is to decide that it is inappropriate to initialise the
> > cpu_possible map with anything but the boot CPU on non-SMP capable
> > systems.  This is implemented by this patch.
> > 
> > The situation currently may provoke a warning on earlier kernels,
> > printed during SMP bringup where we attempt to bring CPU1 online inspite
> > of CPU0 being a UP-only CPU, and this only failing due to the lack of
> > SMP operations.
> 
> I think that even if we work around this, we should have a warning
> regarding the erroneous DT.
> 
> There are plenty of other things people may get wrong in their DT that
> we may or may not be able to detect and/or workaround. We should
> generally discourage relying on fixups and encourage people to do the
> right thing.

Well, this is the problem with a static description of the hardware -
it very quickly becomes unmanagable when you have lots of different
combinations.  For the SR platforms, there are already the base vs
pro Hummingboard, Cubox-i, Hummingboard edge, Hummingboard Edge,
Hummingboard Gate, each the choice of iMX6S, DL, D, Q or more uSOMs.
To cover these "properly" we're going to need 24 DT files rather
than 12 if we're going to require the CPUs entries to correctly
describe the hardware.

Given that CPUs are discoverable from the hardware, this is getting
to be insane.  Yes, there's cases where we need the CPUs described
in hardware, but to require the description for all platforms even
where CPUs are discoverable is IMHO insane.

I think we need a better solution for this, rather than telling people
their DT files need to fully describe the hardware, and at the same
time, be correct.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: avoid SMP DT initialisation on UP-only capable systems

[Lucas Stach]

Am Mittwoch, den 25.11.2015, 11:49 +0000 schrieb Russell King - ARM
Linux:
> On Wed, Nov 25, 2015 at 11:35:38AM +0000, Mark Rutland wrote:
> > On Wed, Nov 25, 2015 at 10:43:02AM +0000, Russell King wrote:
> > > arm_dt_init_cpu_maps() initialises the CPU possible map even when the
> > > boot CPU indicates that it is not SMP capable.  This can happen with
> > > SoCs where the SoC may have a single UP-only CPU, or may have a pair of
> > > SMP CPUs - and this is the only difference.
> > > 
> > > One solution is to increase the number of DT files by forcing peopl to
> > > properly describe the hardware: this means all the iMX6DL DT files need
> > > to be duplicated for iMX6S and a whole raft of updates to boot loaders
> > > and the like.
> > >
> > > Another solution is to decide that it is inappropriate to initialise the
> > > cpu_possible map with anything but the boot CPU on non-SMP capable
> > > systems.  This is implemented by this patch.
> > > 
> > > The situation currently may provoke a warning on earlier kernels,
> > > printed during SMP bringup where we attempt to bring CPU1 online inspite
> > > of CPU0 being a UP-only CPU, and this only failing due to the lack of
> > > SMP operations.
> > 
> > I think that even if we work around this, we should have a warning
> > regarding the erroneous DT.
> > 
> > There are plenty of other things people may get wrong in their DT that
> > we may or may not be able to detect and/or workaround. We should
> > generally discourage relying on fixups and encourage people to do the
> > right thing.
> 
> Well, this is the problem with a static description of the hardware -
> it very quickly becomes unmanagable when you have lots of different
> combinations.  For the SR platforms, there are already the base vs
> pro Hummingboard, Cubox-i, Hummingboard edge, Hummingboard Edge,
> Hummingboard Gate, each the choice of iMX6S, DL, D, Q or more uSOMs.
> To cover these "properly" we're going to need 24 DT files rather
> than 12 if we're going to require the CPUs entries to correctly
> describe the hardware.
> 
> Given that CPUs are discoverable from the hardware, this is getting
> to be insane.  Yes, there's cases where we need the CPUs described
> in hardware, but to require the description for all platforms even
> where CPUs are discoverable is IMHO insane.
> 
> I think we need a better solution for this, rather than telling people
> their DT files need to fully describe the hardware, and at the same
> time, be correct.
> 
What we do in barebox is to look into the SCU to find out how many CPU
cores are actually present and fix up the DT passed to the kernel
accordingly by deleting any superfluous CPU nodes. This satisfies both
the need to keep the required DT source files to a minimum, while also
providing correct information to the kernel.

Regards,
Lucas
-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |

[PATCH] ARM: avoid SMP DT initialisation on UP-only capable systems

[Russell King - ARM Linux]

On Wed, Nov 25, 2015 at 01:50:31PM +0100, Lucas Stach wrote:
> What we do in barebox is to look into the SCU to find out how many CPU
> cores are actually present and fix up the DT passed to the kernel
> accordingly by deleting any superfluous CPU nodes. This satisfies both
> the need to keep the required DT source files to a minimum, while also
> providing correct information to the kernel.

... which is a function of the boot loader, but not all boot loaders
do this.  So, if we're using u-boot, what option do we have?

I've never been able to successfully build and boot uboot myself (it
always fails one way or another with my toolchains - if I get it to
link, it silently fails on the hardware, but if someone else builds
it for me from exactly the same source, it works fine) so count me
out of fixing uboot in this regard.

Note that it's not that the kernel doesn't work, it's just that people
are complaining that they see warnings from the kernel during their
boot.  So, unless we're going to start providing iMX6S and iMX6D
specific DT blobs (which will double the number of .dtb files we end
up with on iMX6), I think we're stuck with users complaining about
this.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: avoid SMP DT initialisation on UP-only capable systems

[Jon Nettleton]

On Wed, Nov 25, 2015 at 1:56 PM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Nov 25, 2015 at 01:50:31PM +0100, Lucas Stach wrote:
>> What we do in barebox is to look into the SCU to find out how many CPU
>> cores are actually present and fix up the DT passed to the kernel
>> accordingly by deleting any superfluous CPU nodes. This satisfies both
>> the need to keep the required DT source files to a minimum, while also
>> providing correct information to the kernel.
>
> ... which is a function of the boot loader, but not all boot loaders
> do this.  So, if we're using u-boot, what option do we have?
>
> I've never been able to successfully build and boot uboot myself (it
> always fails one way or another with my toolchains - if I get it to
> link, it silently fails on the hardware, but if someone else builds
> it for me from exactly the same source, it works fine) so count me
> out of fixing uboot in this regard.
>
> Note that it's not that the kernel doesn't work, it's just that people
> are complaining that they see warnings from the kernel during their
> boot.  So, unless we're going to start providing iMX6S and iMX6D
> specific DT blobs (which will double the number of .dtb files we end
> up with on iMX6), I think we're stuck with users complaining about
> this.

I have been looking into doing some of this cleanup in u-boot
recently.  More and more this seems like a hack because the
device-tree descriptions belong to the kernel.  If the bootloader
isn't generating them it seems counter intuitive that it should be
responsible for altering them so the kernel doesn't complain about
incorrect hardware configuration.  It seems as though device-tree
could benefit from some logic definitions that could load device-tree
snippets or overlays depending on the hardware that is probed.

Or in the case of SR hardware we just generate 24 device tree files
and push this probing back into each bootloader  implementation to
detect and load accordingly.

-Jon