Re: [PATCH v2] xen: arm: fix usage of bootargs for Xen.

[Julien Grall <julien.grall@linaro.org>]

On 10/21/2013 10:21 AM, Ian Campbell wrote:
> 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.
>
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.linaro.org>

> ---
> v2: Tweak the docs to reflect this change.
> ---
>   docs/misc/arm/device-tree/booting.txt |  2 ++
>   xen/common/device_tree.c              | 10 ++++++++--
>   2 files changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/docs/misc/arm/device-tree/booting.txt b/docs/misc/arm/device-tree/booting.txt
> index 08ed775..8da1e0b 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -39,6 +39,8 @@ lines for Xen and Dom0. The logic is the following:
>    - 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 a kernel boot module is present and has a bootargs property then
> +   the top-level bootargs will 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.
> diff --git a/xen/common/device_tree.c b/xen/common/device_tree.c
> index af0fb04..f7d0215 100644
> --- a/xen/common/device_tree.c
> +++ b/xen/common/device_tree.c
> @@ -240,7 +240,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");
> @@ -250,7 +250,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];

I'm wondering if it's really usefull to test nr_mods? We can have 
nr_mods >= MOD_KERNEL (for instance MOD_INITRD), but the kernel command 
is not set.
As the structure will be zeroed during boot Xen, it's not harmful to 
remove the check.

-- 
Julien Grall