Re: [Xen-devel] [PATCH v2 23/41] arm : acpi create chosen node for DOM0

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Parth,

On 17/05/15 21:03, Parth Dixit wrote:
> Create a chosen node for DOM0 with
>  - bootargs
>  - initrd

I would have merge this patch with #22. It doesn't contain
controversial/difficult code.

> Signed-off-by: Naresh Bhat <naresh.bhat@xxxxxxxxxx>
> Signed-off-by: Parth Dixit <parth.dixit@xxxxxxxxxx>
> ---
>  xen/arch/arm/domain_build.c | 46 
> +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index c830702..e688a78 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1219,6 +1219,47 @@ static int handle_node(struct domain *d, struct 
> kernel_info *kinfo,
>      return res;
>  }
>  #ifdef CONFIG_ACPI
> +static int make_chosen_node(struct domain *d, const struct kernel_info 
> *kinfo)
> +{
> +    int res = 0;

Not necessary to initialize.

> +    const char *bootargs = NULL;
> +    const struct bootmodule *mod = kinfo->kernel_bootmodule;
> +    void *fdt = kinfo->fdt;
> +
> +    DPRINT("Create bootargs chosen node\n");

The name of the node is "chosen" not "bootargs chosen".

> +
> +    if ( mod && mod->cmdline[0] )
> +         bootargs = &mod->cmdline[0];
> +    res = fdt_begin_node(fdt, "chosen");
> +    if ( res )
> +        return res;
> +
> +    res = fdt_property(fdt, "bootargs", bootargs, (strlen(bootargs)+1));

strlen(bootargs) + 1. And the ( ) around it is not necessary.

Furthermore, you are assuming that bootargs is always present here.
Although if the module is not present, the variable will be NULL (see
your check above) and Xen will crash.

> +    if ( res )
> +       return res;
> +
> +    /*
> +     * If the bootloader provides an initrd, we must create a placeholder
> +     * for the initrd properties. The values will be replaced later.
> +     */
> +    if ( mod && mod->size )
> +    {
> +        u64 a = 0;
> +        res = fdt_property(kinfo->fdt, "linux,initrd-start", &a, sizeof(a));
> +        if ( res )
> +            return res;
> +
> +        res = fdt_property(kinfo->fdt, "linux,initrd-end", &a, sizeof(a));
> +        if ( res )
> +            return res;
> +    }
> +
> +    res = fdt_end_node(fdt);
> +
> +    return res;
> +}
> +
>  /*
>   * Prepare a minimal DTB for DOM0 which contains
>   * bootargs, initrd, memory information,
> @@ -1259,6 +1300,11 @@ static int create_acpi_dtb(struct domain *d, struct 
> kernel_info *kinfo, struct m
>      if ( ret )
>          return ret;
>  
> +    /* Create a chosen node for DOM0 */
> +    ret = make_chosen_node(d, kinfo);
> +    if ( ret )
> +        goto err;
> +
>      ret = fdt_end_node(kinfo->fdt);
>      if ( ret < 0 )
>          goto err;
> 

Regards,

-- 
Julien Grall

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