[1/2] ARM: default machine descriptor for multiplatform

Commit Message

Arnd Bergmann April 12, 2013, 3:11 p.m. UTC

Since we now have default implementations for init_time and init_irq,
the init_machine callback is the only one that is not yet optional,
but since simple DT based platforms all have the same
of_platform_populate function call in there, we can consolidate them
as well, and then actually boot with a completely empty machine_desc.
Unofortunately we cannot just default to an empty init_machine: We
cannot call of_platform_populate before init_machine because that
does not work in case of auxdata, and we cannot call it after
init_machine either because the machine might need to run code
after adding the devices.

To take the final step, this adds support for booting without defining
any machine_desc whatsoever.

For the case that CONFIG_MULTIPLATFORM is enabled, it adds a
global machine descriptor that never matches any machine but is
used as a fallback if nothing else matches. We assume that without
CONFIG_MULTIPLATFORM, we only want to boot on the systems that the kernel
is built for, so we still retain the build-time warning for missing
machine descriptors and the run-time warning when the platform does not
match in that case.

In the case that we run on a multiplatform kernel and the machine
provides a fully populated device tree, we attempt to keep booting,
hoping that no machine specific callbacks are necessary.

Finally, this also removes the misguided "select ARCH_VEXPRESS" that
was only added to avoid a build error for allnoconfig kernels.

Signed-off-by: Arnd Bergmann <[e-mail]>
Acked-by: Nicolas Pitre <[e-mail]>
Acked-by: Olof Johansson <[e-mail]>
Cc: "Russell King - ARM Linux" <[e-mail]>
Cc: Rob Herring <[e-mail]>
---
 arch/arm/Kconfig          |  1 -
 arch/arm/kernel/devtree.c |  9 ++++++++-
 arch/arm/kernel/setup.c   | 13 +++++++++++--
 3 files changed, 19 insertions(+), 4 deletions(-)

Comments

Russell King - ARM Linux April 19, 2013, 1:28 p.m. UTC | #1

On Fri, Apr 12, 2013 at 05:11:07PM +0200, Arnd Bergmann wrote:
> @@ -752,7 +761,7 @@ void __init setup_arch(char **cmdline_p)
>  
>  	setup_processor();
>  	mdesc = setup_machine_fdt(__atags_pointer);
> -	if (!mdesc)
> +	if (!mdesc && __machine_arch_type != ~0)
>  		mdesc = setup_machine_tags(__atags_pointer, __machine_arch_type);
>  	machine_desc = mdesc;
>  	machine_name = mdesc->name;

If mdesc is NULL and __machine_arch_type is ~0, then mdesc remains NULL.
That means machine_desc is NULL (which is probably very bad), and the
initialization of machine_name causes a NULL pointer dereference.

This is clearly wrong.  mdesc must never be NULL if you're using FDT
here, so the original code should be fine.

Arnd Bergmann April 19, 2013, 2:21 p.m. UTC | #2

On Friday 19 April 2013, Russell King - ARM Linux wrote:
> On Fri, Apr 12, 2013 at 05:11:07PM +0200, Arnd Bergmann wrote:
> > @@ -752,7 +761,7 @@ void __init setup_arch(char **cmdline_p)
> >  
> >       setup_processor();
> >       mdesc = setup_machine_fdt(__atags_pointer);
> > -     if (!mdesc)
> > +     if (!mdesc && __machine_arch_type != ~0)
> >               mdesc = setup_machine_tags(__atags_pointer, __machine_arch_type);
> >       machine_desc = mdesc;
> >       machine_name = mdesc->name;
> 
> If mdesc is NULL and __machine_arch_type is ~0, then mdesc remains NULL.
> That means machine_desc is NULL (which is probably very bad), and the
> initialization of machine_name causes a NULL pointer dereference.
> 
> This is clearly wrong.  mdesc must never be NULL if you're using FDT
> here, so the original code should be fine.

Ah, right. I think was a leftover from an earlier version of the
patch where I only assigned the default platform after this.

There is also a related mistake in setup_machine_fdt, the second
change is not needed there either AFAICT.

Thanks for taking a look!

	Arnd

Patch

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1cacda4..e67d49d 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -1014,7 +1014,6 @@  config ARCH_MULTI_V7
 	bool "ARMv7 based platforms (Cortex-A, PJ4, Scorpion, Krait)"
 	default y
 	select ARCH_MULTI_V6_V7
-	select ARCH_VEXPRESS
 	select CPU_V7
 
 config ARCH_MULTI_V6_V7
diff --git a/arch/arm/kernel/devtree.c b/arch/arm/kernel/devtree.c
index 70f1bde..e6e34ba 100644
--- a/arch/arm/kernel/devtree.c
+++ b/arch/arm/kernel/devtree.c
@@ -180,6 +180,13 @@  struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
 	unsigned long dt_root;
 	const char *model;
 
+	if (IS_ENABLED(CONFIG_ARCH_MULTIPLATFORM)) {
+		DT_MACHINE_START(GENERIC_DT, "Generic DT based system")
+		MACHINE_END
+
+		mdesc_best = (struct machine_desc *)&__mach_desc_GENERIC_DT;
+	}
+
 	if (!dt_phys)
 		return NULL;
 
@@ -199,7 +206,7 @@  struct machine_desc * __init setup_machine_fdt(unsigned int dt_phys)
 			mdesc_score = score;
 		}
 	}
-	if (!mdesc_best) {
+	if (!mdesc_best && !IS_ENABLED(CONFIG_ARCH_MULTIPLATFORM)) {
 		const char *prop;
 		long size;
 
diff --git a/arch/arm/kernel/setup.c b/arch/arm/kernel/setup.c
index d343a6c..8ea8d68 100644
--- a/arch/arm/kernel/setup.c
+++ b/arch/arm/kernel/setup.c
@@ -18,6 +18,7 @@ 
 #include <linux/bootmem.h>
 #include <linux/seq_file.h>
 #include <linux/screen_info.h>
+#include <linux/of_platform.h>
 #include <linux/init.h>
 #include <linux/kexec.h>
 #include <linux/of_fdt.h>
@@ -660,9 +661,17 @@  struct screen_info screen_info = {
 
 static int __init customize_machine(void)
 {
-	/* customizes platform devices, or adds new ones */
+	/*
+	 * customizes platform devices, or adds new ones
+	 * On DT based machines, we fall back to populating the
+	 * machine from the device tree, if no callback is provided,
+	 * otherwise we would always need an init_machine callback.
+	 */
 	if (machine_desc->init_machine)
 		machine_desc->init_machine();
+	else
+		of_platform_populate(NULL, of_default_bus_match_table,
+					NULL, NULL);
 	return 0;
 }
 arch_initcall(customize_machine);
@@ -752,7 +761,7 @@  void __init setup_arch(char **cmdline_p)
 
 	setup_processor();
 	mdesc = setup_machine_fdt(__atags_pointer);
-	if (!mdesc)
+	if (!mdesc && __machine_arch_type != ~0)
 		mdesc = setup_machine_tags(__atags_pointer, __machine_arch_type);
 	machine_desc = mdesc;
 	machine_name = mdesc->name;