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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2013-05-31 at 15:45 +0200, Andre Przywara wrote:
> Currently we use the chosen/bootargs property as the Xen commandline
> and rely on xen,dom0-bootargs for Dom0. However this brings issues
> with bootloaders, which usually build bootargs by bootscripts for a
> Linux kernel - and not for the entirely different Xen hypervisor.
> Introduce a new possible device tree property "xen,xen-bootargs"
> explicitly for the Xen hypervisor and make the selection of which to
> use more fine grained:
> - If xen,xen-bootargs is present, it will be used for Xen.
> - If xen,dom0-bootargs is present, it will be used for Dom0.
> - If xen,xen-bootargs is _not_ present, but xen,dom0-bootargs is,
>   bootargs will be used for Xen. Like the current situation.
> - If no Xen specific properties are present, bootargs is for Dom0.
> - If xen,xen-bootargs is present, but xen,dom0-bootargs is missing,
>   bootargs will be used for Dom0.
> 
> The aim is to allow common bootscripts to boot both Xen and native
> Linux with the same device tree blob. If needed, one could hard-code
> the Xen commandline into the DTB, leaving bootargs for Dom0 to be set
> by the (non Xen-aware) bootloader.
> I also have a simple patch for u-boot to transfer the content of the
> "xen_bootargs" environment variable into the xen,xen-bootargs dtb
> property.
> If you like this approach, I will send the u-boot patch to their ML.

I think I've traced through all 8 possibilities and the results seem to
make sense...

> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxxx>
> ---
>  xen/arch/arm/domain_build.c | 14 +++++++++++---
>  xen/common/device_tree.c    |  7 ++++++-
>  2 files changed, 17 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index b92c64b..952adb3 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -139,6 +139,7 @@ static int write_properties(struct domain *d, struct 
> kernel_info *kinfo,
>                              u32 address_cells, u32 size_cells)
>  {
>      const char *bootargs = NULL;
> +    int had_dom0_bootargs = 0;
>      int prop;
>  
>      if ( early_info.modules.nr_mods >= 1 &&
> @@ -169,12 +170,19 @@ static int write_properties(struct domain *d, struct 
> kernel_info *kinfo,
>           */
>          if ( device_tree_node_matches(fdt, node, "chosen") )
>          {
> -            if ( strcmp(prop_name, "bootargs") == 0 )
> +            if ( strcmp(prop_name, "xen,xen-bootargs") == 0 )
> +                continue;
> +            if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
> +            {
> +                had_dom0_bootargs = 1;
> +                bootargs = prop_data;
>                  continue;
> -            else if ( strcmp(prop_name, "xen,dom0-bootargs") == 0 )
> +            }
> +            if ( strcmp(prop_name, "bootargs") == 0 )
>              {
> -                if ( !bootargs )
> +                if ( !bootargs  && !had_dom0_bootargs ) {
>                      bootargs = prop_data;
> +                }

Xen coding style doesn't require {} around single lines.

>                  continue;
>              }
>          }
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 84d704d..c25e6d4 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -325,7 +325,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 ) {

Coding style is { on the next line

> +     if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL)) {
> +            prop = fdt_get_property(fdt, node, "bootargs", NULL);
> +        }

There's a hard tab here, but you don't need the {} anyway. To avoid
ambiguity I would stick with the outer one though.

> +    }
>      if ( prop == NULL )
>          return NULL;
>  



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