Re: [Xen-devel] [PATCH 9/9] xen: strip /chosen/modules/module@<N>/* from dom0 device tree

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Thu, 6 Dec 2012, Ian Campbell wrote:
> These nodes are used by Xen to find the initial modules.
> 
> Only drop the "xen,multiboot-module" compatible nodes in case someone
> else has a similar idea.
> 
> Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> ---
> v4 - /chosen/modules/modules@N not /chosen/module@N
> v3 - use a helper to filter out DT elements which are not for dom0.
>      Better than an ad-hoc break in the middle of a loop.
> ---
>  xen/arch/arm/domain_build.c |   40 ++++++++++++++++++++++++++++++++++++++--
>  1 files changed, 38 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 7a964f7..27e02e4 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -172,6 +172,40 @@ static int write_properties(struct domain *d, struct 
> kernel_info *kinfo,
>      return prop;
>  }
>  
> +/* Returns the next node in fdt (starting from offset) which should be
> + * passed through to dom0.
> + */
> +static int fdt_next_dom0_node(const void *fdt, int node,
> +                              int *depth_out,
> +                              int parents[DEVICE_TREE_MAX_DEPTH])
> +{
> +    int depth = *depth_out;
> +
> +    while ( (node = fdt_next_node(fdt, node, &depth)) &&
> +            node >= 0 && depth >= 0 )
> +    {
> +        if ( depth >= DEVICE_TREE_MAX_DEPTH )
> +            break;
> +
> +        parents[depth] = node;
> +
> +        /* Skip /chosen/modules/module@<N>/ and all subnodes */
> +        if ( depth >= 3 &&
> +             device_tree_node_matches(fdt, parents[1], "chosen") &&
> +             device_tree_node_matches(fdt, parents[2], "modules") &&
> +             device_tree_node_matches(fdt, parents[3], "module") &&
> +             fdt_node_check_compatible(fdt, parents[3],
> +                                       "xen,multiboot-module" ) == 0 )
> +            continue;
> +
> +        /* We've arrived at a node which dom0 is interested in. */
> +        break;
> +    }
> +
> +    *depth_out = depth;
> +    return node;
> +}

Can't we just skip the node if it is compatible with
"xen,multiboot-module", no matter where it lives?  This should simplify
this function greatly and you wouldn't need the parents parameter
anymore.
This way we could have a simple node blacklist based on the compatible
node all in a single function.


>  static int write_nodes(struct domain *d, struct kernel_info *kinfo,
>                         const void *fdt)
>  {
> @@ -179,11 +213,12 @@ static int write_nodes(struct domain *d, struct 
> kernel_info *kinfo,
>      int depth = 0, last_depth = -1;
>      u32 address_cells[DEVICE_TREE_MAX_DEPTH];
>      u32 size_cells[DEVICE_TREE_MAX_DEPTH];
> +    int parents[DEVICE_TREE_MAX_DEPTH];
>      int ret;
>  
>      for ( node = 0, depth = 0;
>            node >= 0 && depth >= 0;
> -          node = fdt_next_node(fdt, node, &depth) )
> +          node = fdt_next_dom0_node(fdt, node, &depth, parents) )
>      {
>          const char *name;
>  
> @@ -191,7 +226,8 @@ static int write_nodes(struct domain *d, struct 
> kernel_info *kinfo,
>  
>          if ( depth >= DEVICE_TREE_MAX_DEPTH )
>          {
> -            printk("warning: node `%s' is nested too deep\n", name);
> +            printk("warning: node `%s' is nested too deep (%d)\n",
> +                   name, depth);
>              continue;
>          }
>  
> -- 
> 1.7.9.1
> 

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