diff mbox

[Xen-devel,v3,08/10] xen: arm: support bootmodule type detection by ordering

Message ID 1405688900-11769-8-git-send-email-ian.campbell@citrix.com
State Accepted
Commit e55370e3a0da5192fe3c29a370f26e1663707042
Headers show

Commit Message

Ian Campbell July 18, 2014, 1:08 p.m. UTC
Assign modules types based on the order in which they are defined in the FDT.
This is supported only for the dom0 kernel and ramdisk when given as the first
and second modules respectively, similar to how
http://wiki.xen.org/wiki?title=Xen_ARM_with_Virtualization_Extensions/Multiboot&oldid=11824
defined the default types from the bootloader side.

This is compatible with how Xen interprets the modules with x86 multiboot and I
think simplifies things for bootloaders which now need not contain similar
guessing code if they only care about the most basic case.

Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
---
v2: Don't rely on enum ordering for guessing
---
 xen/arch/arm/bootfdt.c      |   17 +++++++++++++++--
 xen/arch/arm/setup.c        |   14 ++++++++++++++
 xen/include/asm-arm/setup.h |    1 +
 3 files changed, 30 insertions(+), 2 deletions(-)

Comments

Julien Grall July 18, 2014, 9:02 p.m. UTC | #1
Hi Ian,

On 18/07/14 14:08, Ian Campbell wrote:
> @@ -281,10 +293,11 @@ static void __init early_print_info(void)
>                        mi->bank[i].start + mi->bank[i].size - 1);
>       printk("\n");
>       for ( i = 0 ; i < mods->nr_mods; i++ )
> -        printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %s\n",
> +        printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %-12s %s\n",
>                        i,
>                        mods->module[i].start,
>                        mods->module[i].start + mods->module[i].size,
> +                     boot_module_kind_as_string(mods->module[i].kind),
>                        mods->module[i].cmdline);
>       nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
>       for ( i = 0; i < nr_rsvd; i++ )
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 0cbe552..e53e491 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -223,6 +223,20 @@ struct bootmodule * __init boot_module_find_by_kind(bootmodule_kind kind)
>       return NULL;
>   }
>
> +const char * __init boot_module_kind_as_string(bootmodule_kind kind)
> +{
> +    switch ( kind )
> +    {
> +    case BOOTMOD_XEN:     return "Xen";
> +    case BOOTMOD_FDT:     return "Device Tree";
> +    case BOOTMOD_KERNEL:  return "Kernel";
> +    case BOOTMOD_RAMDISK: return "Ramdisk";
> +    case BOOTMOD_XSM:     return "XSM";
> +    case BOOTMOD_UNKNOWN: return "Unknown";
> +    default: BUG();
> +    }
> +}
> +

A bit strange to introduce boot_module_kind_as_string here rather than 
in the earlier patch. Likewise for changing the string format a bit above.

Anyway:

Acked-by: Julien Grall <julien.grall@linaro.org>

Regards,
Ian Campbell July 21, 2014, 10:18 a.m. UTC | #2
On Fri, 2014-07-18 at 22:02 +0100, Julien Grall wrote:
> Hi Ian,
> 
> On 18/07/14 14:08, Ian Campbell wrote:
> > @@ -281,10 +293,11 @@ static void __init early_print_info(void)
> >                        mi->bank[i].start + mi->bank[i].size - 1);
> >       printk("\n");
> >       for ( i = 0 ; i < mods->nr_mods; i++ )
> > -        printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %s\n",
> > +        printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %-12s %s\n",
> >                        i,
> >                        mods->module[i].start,
> >                        mods->module[i].start + mods->module[i].size,
> > +                     boot_module_kind_as_string(mods->module[i].kind),
> >                        mods->module[i].cmdline);
> >       nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
> >       for ( i = 0; i < nr_rsvd; i++ )
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 0cbe552..e53e491 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -223,6 +223,20 @@ struct bootmodule * __init boot_module_find_by_kind(bootmodule_kind kind)
> >       return NULL;
> >   }
> >
> > +const char * __init boot_module_kind_as_string(bootmodule_kind kind)
> > +{
> > +    switch ( kind )
> > +    {
> > +    case BOOTMOD_XEN:     return "Xen";
> > +    case BOOTMOD_FDT:     return "Device Tree";
> > +    case BOOTMOD_KERNEL:  return "Kernel";
> > +    case BOOTMOD_RAMDISK: return "Ramdisk";
> > +    case BOOTMOD_XSM:     return "XSM";
> > +    case BOOTMOD_UNKNOWN: return "Unknown";
> > +    default: BUG();
> > +    }
> > +}
> > +
> 
> A bit strange to introduce boot_module_kind_as_string here rather than 
> in the earlier patch.

This is the first time it seemed necessary, since you can no longer
infer it from the module number.

>  Likewise for changing the string format a bit above.

That's just adding "%-12s" to print the type.
> 
> Anyway:
> 
> Acked-by: Julien Grall <julien.grall@linaro.org>

Thanks
> 
> Regards,
> 
> 
>
diff mbox

Patch

diff --git a/xen/arch/arm/bootfdt.c b/xen/arch/arm/bootfdt.c
index ce7fdf2..e100233 100644
--- a/xen/arch/arm/bootfdt.c
+++ b/xen/arch/arm/bootfdt.c
@@ -165,6 +165,7 @@  static void __init process_multiboot_node(const void *fdt, int node,
                                           const char *name,
                                           u32 address_cells, u32 size_cells)
 {
+    static int kind_guess = 0;
     const struct fdt_property *prop;
     const __be32 *cell;
     bootmodule_kind kind;
@@ -181,7 +182,18 @@  static void __init process_multiboot_node(const void *fdt, int node,
     else if ( fdt_node_check_compatible(fdt, node, "xen,xsm-policy") == 0 )
         kind = BOOTMOD_XSM;
     else
-        panic("%s not a known xen multiboot type\n", name);
+        kind = BOOTMOD_UNKNOWN;
+
+    /* Guess that first two unknown are kernel and ramdisk respectively. */
+    if ( kind == BOOTMOD_UNKNOWN )
+    {
+        switch ( kind_guess++ )
+        {
+        case 0: kind = BOOTMOD_KERNEL; break;
+        case 1: kind = BOOTMOD_RAMDISK; break;
+        default: break;
+        }
+    }
 
     prop = fdt_get_property(fdt, node, "reg", &len);
     if ( !prop )
@@ -281,10 +293,11 @@  static void __init early_print_info(void)
                      mi->bank[i].start + mi->bank[i].size - 1);
     printk("\n");
     for ( i = 0 ; i < mods->nr_mods; i++ )
-        printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %s\n",
+        printk("MODULE[%d]: %"PRIpaddr" - %"PRIpaddr" %-12s %s\n",
                      i,
                      mods->module[i].start,
                      mods->module[i].start + mods->module[i].size,
+                     boot_module_kind_as_string(mods->module[i].kind),
                      mods->module[i].cmdline);
     nr_rsvd = fdt_num_mem_rsv(device_tree_flattened);
     for ( i = 0; i < nr_rsvd; i++ )
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 0cbe552..e53e491 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -223,6 +223,20 @@  struct bootmodule * __init boot_module_find_by_kind(bootmodule_kind kind)
     return NULL;
 }
 
+const char * __init boot_module_kind_as_string(bootmodule_kind kind)
+{
+    switch ( kind )
+    {
+    case BOOTMOD_XEN:     return "Xen";
+    case BOOTMOD_FDT:     return "Device Tree";
+    case BOOTMOD_KERNEL:  return "Kernel";
+    case BOOTMOD_RAMDISK: return "Ramdisk";
+    case BOOTMOD_XSM:     return "XSM";
+    case BOOTMOD_UNKNOWN: return "Unknown";
+    default: BUG();
+    }
+}
+
 void __init discard_initial_modules(void)
 {
     struct bootmodules *mi = &bootinfo.modules;
diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
index e2e18c3..36e5704 100644
--- a/xen/include/asm-arm/setup.h
+++ b/xen/include/asm-arm/setup.h
@@ -64,6 +64,7 @@  struct bootmodule *add_boot_module(bootmodule_kind kind,
                                    paddr_t start, paddr_t size,
                                    const char *cmdline);
 struct bootmodule *boot_module_find_by_kind(bootmodule_kind kind);
+const char * __init boot_module_kind_as_string(bootmodule_kind kind);
 
 #endif
 /*