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

[Julien Grall <julien.grall@xxxxxxxxxx>]

On 20 August 2013 16:21, Andre Przywara <andre.przywara@xxxxxxxxxx> 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 will send out a appropriate u-boot patch, which writes the content
> of the "xen_bootargs" environment variable into the xen,xen-bootargs
> dtb property.

Since we plan to support multiboot, is it relevant to send a u-boot patch
for a temporary workaround?

We could use a u-boot script to create the correct properties/nodes in
the device tree. What do you think?

> v1 .. v2:
>  - fix whitespace issues
> v2 .. v3:
>  - add documentation
>
> Signed-off-by: Andre Przywara <andre.przywara@xxxxxxxxxx>

Reviewed-by: Julien Grall <julien.grall@xxxxxxxxxx>

BTW, I have modified this code with my latest patch series. I will
rebase it on top
of this patch.

Cheers,

> ---
>  docs/misc/arm/device-tree/booting.txt | 28 +++++++++++++++++++++++++++-
>  xen/arch/arm/domain_build.c           | 15 +++++++++++----
>  xen/common/device_tree.c              |  7 ++++++-
>  3 files changed, 44 insertions(+), 6 deletions(-)
>
> diff --git a/docs/misc/arm/device-tree/booting.txt 
> b/docs/misc/arm/device-tree/booting.txt
> index 94cd3f1..08ed775 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -1,3 +1,6 @@
> +Dom0 kernel and ramdisk modules
> +================================
> +
>  Xen is passed the dom0 kernel and initrd via a reference in the /chosen
>  node of the device tree.
>
> @@ -22,4 +25,27 @@ properties:
>
>  - bootargs (optional)
>
> -       Command line associated with this module
> +       Command line associated with this module. This is deprecated and 
> should
> +       be replaced by the bootargs variations described below.
> +
> +
> +Command lines
> +=============
> +
> +Xen also checks for properties directly under /chosen to find suitable 
> command
> +lines for Xen and Dom0. The logic is the following:
> +
> + - 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.
> + - 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.
> +
> +Most of these cases is to make booting with Xen-unaware bootloaders easier.
> +For those you would hardcode the Xen commandline in the DTB under
> +/chosen/xen,xen-bootargs and would let the bootloader set the Dom0 command
> +line by writing bootargs (as for native Linux).
> +A Xen-aware bootloader would set xen,xen-bootargs for Xen, xen,dom0-bootargs
> +for Dom0 and bootargs for native Linux.
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 01492bb..c82ece1 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -140,6 +140,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 &&
> @@ -166,15 +167,21 @@ static int write_properties(struct domain *d, struct 
> kernel_info *kinfo,
>           *
>           * * remember xen,dom0-bootargs if we don't already have
>           *   bootargs (from module #1, above).
> -         * * remove bootargs and xen,dom0-bootargs.
> +         * * remove bootargs,  xen,dom0-bootargs and xen,xen-bootargs.
>           */
>          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;
>                  continue;
>              }
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index 84d704d..e22bd22 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 )
> +    {
> +        if (fdt_get_property(fdt, node, "xen,dom0-bootargs", NULL))
> +            prop = fdt_get_property(fdt, node, "bootargs", NULL);
> +    }
>      if ( prop == NULL )
>          return NULL;

-- 
Julien Grall

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