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

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Mon, 10 Dec 2012, Ian Campbell wrote:
> On Fri, 2012-12-07 at 17:35 +0000, Stefano Stabellini wrote:
> > 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.
> 
> As well as my previous query about the meaning of the tree structure I
> think that even if we could get away with this in this particular case
> we are going to need this sort of infrastructure once we start doing
> proper filtering of dom0 vs xen nodes in the tree.

Maybe. However in that case we could just have a generic
filter_device_tree_nodes function that takes an array of strings
(compatible string? device tree paths? I would go for both in a struct,
but the former would probably suffice), and filter the DT based on
those. That would be very useful in the long run. This is very ad-hoc
for the /chosen/modules/module@<N> path.

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