Re: [Xen-devel] [Xen-staging] [xen staging] ARM: parse separate DT properties for different commandlines

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-10-07 at 13:23 +0100, Julien Grall wrote:
> On 10/01/2013 11:12 AM, Ian Campbell wrote:
> > On Tue, 2013-10-01 at 10:56 +0100, Ian Campbell wrote:
> >> On Thu, 2013-09-19 at 10:44 +0100, Julien Grall wrote:
> >>>
> >>> On 19 Sep 2013 09:29, "Andre Przywara" <andre.przywara@xxxxxxxxxx>
> >>> wrote:
> >>>>>
> >>>>>> @@ -261,7 +261,12 @@ const char *device_tree_bootargs(const void
> >>> *fdt)
> >>>>>>       if ( node < 0 )
> >>>>>>           return NULL;
> >>>>>>
> >>>>>> -    prop = fdt_get_property(fdt, node, "bootargs", NULL);
> >>>>>> +    prop = fdt_get_property(fdt, node, "xen,xen-bootargs", NULL);
> >>>>>> +    if ( prop == NULL )
> >>>>>> +    {
> >>>>>> +        if (fdt_get_property(fdt, node, "xen,dom0-bootargs",
> >>> NULL))
> >>>>>> +            prop = fdt_get_property(fdt, node, "bootargs", NULL);
> >>>>>> +    }
> >>>
> >>> The logic seems wrong here, we returns bootargs only if the property
> >>> "xen,dom0-bootargs" exists. We should also check if the user give the
> >>> dom0
> >>> command line via the multiboot module.
> >>
> >> Anyone investigating this? I've just been using the following, which is
> >> obviously bogus!
> > 
> > How about this:
> > 8<------------------------------------
> > 
> > From a158dee49bb59e76c0f9103f512bb4bf9489f770 Mon Sep 17 00:00:00 2001
> > From: Ian Campbell <ijc@xxxxxxxxxxxxxx>
> > Date: Fri, 20 Sep 2013 23:45:34 +0100
> > Subject: [PATCH] xen: arm: fix usage of bootargs for Xen.
> > 
> > The chosen node's bootargs property should be used for Xen if there is a 
> > dom0
> > kernel multiboot module with a command line, not just if xen,dom0-bootargs 
> > is
> > present.
> 
> From docs/misc/arm/device-tree/booting.txt:
> If no Xen specific properties are present, bootargs is for Dom0.
> 
> The current behaviour seems logic. Can you update the documentation?

You mean the patch is good but the docs need updating? Sure.

> 
> > Signed-off-by: Ian Campbell <ian.campbell@xxxxxxxxxx>
> > ---
> >  xen/common/device_tree.c |   10 ++++++++--
> >  1 file changed, 8 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> > index 27ee708..fe25508 100644
> > --- a/xen/common/device_tree.c
> > +++ b/xen/common/device_tree.c
> > @@ -242,7 +242,7 @@ static int __init device_tree_for_each_node(const void 
> > *fdt,
> >   */
> >  const char *device_tree_bootargs(const void *fdt)
> >  {
> > -    int node; 
> > +    int node;
> >      const struct fdt_property *prop;
> >  
> >      node = fdt_path_offset(fdt, "/chosen");
> > @@ -252,7 +252,13 @@ const char *device_tree_bootargs(const void *fdt)
> >      prop = fdt_get_property(fdt, node, "xen,xen-bootargs", NULL);
> >      if ( prop == NULL )
> >      {
> > -        if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL))
> > +        struct dt_mb_module *dom0_mod = NULL;
> > +
> > +        if ( early_info.modules.nr_mods >= MOD_KERNEL )
> > +            dom0_mod = &early_info.modules.module[MOD_KERNEL];
> > +
> > +        if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL) ||
> > +            ( dom0_mod && dom0_mod->cmdline[0] ) )
> >              prop = fdt_get_property(fdt, node, "bootargs", NULL);
> >      }
> >      if ( prop == NULL )
> > 
> 
> 



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