[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH 2/9] xen: arm: parse modules from DT during early boot.



On Fri, 2012-12-07 at 17:30 +0000, Stefano Stabellini wrote:
> On Thu, 6 Dec 2012, Ian Campbell wrote:
> > The bootloader should populate /chosen/modules/module@<N>/ for each
> > module it wishes to pass to the hypervisor. The content of these nodes
> > is described in docs/misc/arm/device-tree/booting.txt
> > 
> > The hypervisor parses for 2 types of module, linux zImages and linux
> > initrds. Currently we don't do anything with them.
> > 
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> > v4: Use /chosen/modules/module@N
> >     Identify module type by compatible property not number.
> > v3: Use a reg = < > property for the module address/length.
> > v2: Reserve the zeroeth module for Xen itself (not used yet)
> >     Use a more idiomatic DT layout
> >     Document said layout
> > ---
> >  docs/misc/arm/device-tree/booting.txt |   25 ++++++++++
> >  xen/common/device_tree.c              |   86 
> > +++++++++++++++++++++++++++++++++
> >  xen/include/xen/device_tree.h         |   14 +++++
> >  3 files changed, 125 insertions(+), 0 deletions(-)
> >  create mode 100644 docs/misc/arm/device-tree/booting.txt
> > 
> > diff --git a/docs/misc/arm/device-tree/booting.txt 
> > b/docs/misc/arm/device-tree/booting.txt
> > new file mode 100644
> > index 0000000..94cd3f1
> > --- /dev/null
> > +++ b/docs/misc/arm/device-tree/booting.txt
> > @@ -0,0 +1,25 @@
> > +Xen is passed the dom0 kernel and initrd via a reference in the /chosen
> > +node of the device tree.
> > +
> > +Each node has the form /chosen/modules/module@<N> and contains the 
> > following
> > +properties:
> > +
> > +- compatible
> > +
> > +   Must be:
> > +
> > +           "xen,<type>", "xen,multiboot-module"
> > +
> > +   where <type> must be one of:
> > +
> > +   - "linux-zimage" -- the dom0 kernel
> > +   - "linux-initrd" -- the dom0 ramdisk
> > +
> > +- reg
> > +
> > +   Specifies the physical address of the module in RAM and the
> > +   length of the module.
> > +
> > +- bootargs (optional)
> > +
> > +   Command line associated with this module
> > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> > index da0af77..4bb640e 100644
> > --- a/xen/common/device_tree.c
> > +++ b/xen/common/device_tree.c
> > @@ -270,6 +270,90 @@ static void __init process_cpu_node(const void *fdt, 
> > int node,
> >      cpumask_set_cpu(start, &cpu_possible_map);
> >  }
> >  
> > +static int __init process_chosen_modules_node(const void *fdt, int node,
> > +                                              const char *name, int *depth,
> > +                                              u32 address_cells, u32 
> > size_cells)
> > +{
> > +    const struct fdt_property *prop;
> > +    const u32 *cell;
> > +    int nr, nr_modules = 0;
> > +    struct dt_mb_module *mod;
> > +    int len;
> > +
> > +    for ( *depth = 1;
> > +          *depth >= 1;
> > +          node = fdt_next_node(fdt, node, depth) )
> > +    {
> > +        name = fdt_get_name(fdt, node, NULL);
> > +        if ( strncmp(name, "module@", strlen("module@")) == 0 ) {
> > +
> > +            if ( fdt_node_check_compatible(fdt, node,
> > +                                           "xen,multiboot-module" ) != 0 )
> > +                early_panic("%s not a compatible module node\n", name);
> > +
> > +            if ( fdt_node_check_compatible(fdt, node,
> > +                                           "xen,linux-zimage") == 0 )
> > +                nr = 1;
> > +            else if ( fdt_node_check_compatible(fdt, node,
> > +                                                "xen,linux-initrd") == 0)
> > +                nr = 2;
> > +            else
> > +                early_panic("%s not a known xen multiboot byte\n");
> > +
> > +            if ( nr > nr_modules )
> > +                nr_modules = nr;
> > +
> > +            mod = &early_info.modules.module[nr];
> > +
> > +            prop = fdt_get_property(fdt, node, "reg", NULL);
> > +            if ( !prop )
> > +                early_panic("node %s missing `reg' property\n", name);
> > +
> > +            cell = (const u32 *)prop->data;
> > +            device_tree_get_reg(&cell, address_cells, size_cells,
> > +                                &mod->start, &mod->size);
> > +
> > +            prop = fdt_get_property(fdt, node, "bootargs", &len);
> > +            if ( prop )
> > +            {
> > +                if ( len > sizeof(mod->cmdline) )
> > +                    early_panic("module %d command line too long\n", nr);
> > +
> > +                safe_strcpy(mod->cmdline, prop->data);
> > +            }
> > +            else
> > +                mod->cmdline[0] = 0;
> > +        }
> > +    }
> > +
> > +    for ( nr = 1 ; nr < nr_modules ; nr++ )
> > +    {
> > +        mod = &early_info.modules.module[nr];
> > +        if ( !mod->start || !mod->size )
> > +            early_panic("module %d  missing / invalid\n", nr);
> > +    }
> > +
> > +    early_info.modules.nr_mods = nr_modules;
> > +    return node;
> > +}
> > +
> > +static void __init process_chosen_node(const void *fdt, int node,
> > +                                       const char *name,
> > +                                       u32 address_cells, u32 size_cells)
> > +{
> > +    int depth;
> > +
> > +    for ( depth = 0;
> > +          depth >= 0;
> > +          node = fdt_next_node(fdt, node, &depth) )
> > +    {
> > +        name = fdt_get_name(fdt, node, NULL);
> > +        if ( depth == 1 && strcmp(name, "modules") == 0 )
> > +            node = process_chosen_modules_node(fdt, node, name, &depth,
> > +                                               address_cells, size_cells);
> > +    }
> > +}
> > +
> >  static int __init early_scan_node(const void *fdt,
> >                                    int node, const char *name, int depth,
> >                                    u32 address_cells, u32 size_cells,
> > @@ -279,6 +363,8 @@ static int __init early_scan_node(const void *fdt,
> >          process_memory_node(fdt, node, name, address_cells, size_cells);
> >      else if ( device_tree_type_matches(fdt, node, "cpu") )
> >          process_cpu_node(fdt, node, name, address_cells, size_cells);
> > +    else if ( device_tree_node_matches(fdt, node, "chosen") )
> > +        process_chosen_node(fdt, node, name, address_cells, size_cells);
> >  
> >      return 0;
> >  }
> 
> You have really written a lot of code here!
> I would have thought that just matching on the compatible string would
> be enough:
> 
> else if ( device_tree_node_matches(fdt, node, "linux-zimage") )
>      process_linuxzimage_node(fdt, node, name, address_cells, size_cells);
> else if ( device_tree_node_matches(fdt, node, "linux-initrd") )
>      process_linuxinitrd_node(fdt, node, name, address_cells, size_cells);
> 
> so that your process_linuxzimage_node and process_linuxinitrd_node start
> from the right node and have everything they need to parse it

Is the tree structure of Device Tree meaningless? I'd have thought that
a compatible node would only have meaning at a particular place in the
tree. Granted compatible nodes are often pretty specific and precise,
but is that inherent enough in DT that we can rely on it?


> 
> 
> 
> > diff --git a/xen/include/xen/device_tree.h b/xen/include/xen/device_tree.h
> > index 4d010c0..c383677 100644
> > --- a/xen/include/xen/device_tree.h
> > +++ b/xen/include/xen/device_tree.h
> > @@ -15,6 +15,7 @@
> >  #define DEVICE_TREE_MAX_DEPTH 16
> >  
> >  #define NR_MEM_BANKS 8
> > +#define NR_MODULES 2
> >  
> >  struct membank {
> >      paddr_t start;
> > @@ -26,8 +27,21 @@ struct dt_mem_info {
> >      struct membank bank[NR_MEM_BANKS];
> >  };
> >  
> > +struct dt_mb_module {
> > +    paddr_t start;
> > +    paddr_t size;
> > +    char cmdline[1024];
> > +};
> > +
> > +struct dt_module_info {
> > +    int nr_mods;
> > +    /* Module 0 is Xen itself, followed by the provided modules-proper */
> > +    struct dt_mb_module module[NR_MODULES + 1];
> > +};
> > +
> >  struct dt_early_info {
> >      struct dt_mem_info mem;
> > +    struct dt_module_info modules;
> >  };
> >  
> >  typedef int (*device_tree_node_func)(const void *fdt,
> > -- 
> > 1.7.9.1
> > 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.