Re: [Xen-devel] [PATCH RFC 20/35] xen/arm: Prepare a min DT for DOM0

[Julien Grall <julien.grall@xxxxxxxxxx>]

Hi Parth,

On 04/02/2015 14:02, parth.dixit@xxxxxxxxxx wrote:
> From: Naresh Bhat <naresh.bhat@xxxxxxxxxx>
>
> This patch prepare a DT from scratch for DOM0 for
> ACPI-case only. Basically the DT contains minmal
> required informations such as DOM0 bootargs, memory
> and ACPI RSDP informations only.
>
> Signed-off-by: Naresh Bhat <naresh.bhat@xxxxxxxxxx>
> ---
>   xen/arch/arm/domain_build.c | 67 ++++++++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index de180d8..bb7f043 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -18,6 +18,7 @@
>   #include <asm/psci.h>
>   #include <asm/setup.h>
>   #include <asm/cpufeature.h>
> +#include <asm/acpi.h>
>
>   #include <asm/gic.h>
>   #include <xen/irq.h>
> @@ -61,6 +62,9 @@ custom_param("dom0_mem", parse_dom0_mem);
>    */
>   #define DOM0_FDT_EXTRA_SIZE (128 + sizeof(struct fdt_reserve_entry))
>
> +/* Reserve DOM0 FDT size in ACPI case only */
> +#define DOM0_FDT_MIN_SIZE 4096
> +
>   struct vcpu *__init alloc_dom0_vcpu0(struct domain *dom0)
>   {
>       if ( opt_dom0_max_vcpus == 0 )
> @@ -1151,6 +1155,63 @@ static int handle_node(struct domain *d, struct 
> kernel_info *kinfo,
>       return res;
>   }
>
> +/*
> + * Prepare a minimal DTB for DOM0 which contains
> + * bootargs, memory information,
> + * ACPI RSDP pointer.
> + */
> +static int prepare_dtb_acpi(struct domain *d, struct kernel_info *kinfo)
> +{
> +    int new_size;
> +    int ret;
> +
> +    DPRINT("Prepare a min DTB for DOM0\n");
> +
> +    /* Allocate min size for DT */
> +    new_size = DOM0_FDT_MIN_SIZE;
> +    kinfo->fdt = xmalloc_bytes(DOM0_FDT_MIN_SIZE);
> +
> +    if ( kinfo->fdt == NULL )
> +        return -ENOMEM;
> +
> +    /* Create a new empty DT for DOM0 */
> +    ret = fdt_create(kinfo->fdt, new_size);
> +    if ( ret < 0 )
> +        goto err;
> +
> +    /* Reserve the memory space for new DT */
Wrong comment.

> +    ret = fdt_finish_reservemap(kinfo->fdt);
> +    if ( ret < 0 )
> +        goto err;
> +
> +    ret = fdt_begin_node(kinfo->fdt, "/");
> +    if ( ret < 0 )
> +        goto err;
> +
> +    ret = fdt_property_cell(kinfo->fdt, "#address-cells", 2);
> +    if ( ret )
> +        return ret;
> +
> +    ret = fdt_property_cell(kinfo->fdt, "#size-cells", 1);
> +    if ( ret )
> +        return ret;
> +
> +    ret = fdt_end_node(kinfo->fdt);
> +    if ( ret < 0 )
> +        goto err;
> +
> +    ret = fdt_finish(kinfo->fdt);
> +    if ( ret < 0 )
> +        goto err;
> +
> +    return 0;
> +
> +  err:
> +    printk("Device tree generation failed (%d).\n", ret);
> +    xfree(kinfo->fdt);
> +    return -EINVAL;
> +}
> +
>   static int prepare_dtb(struct domain *d, struct kernel_info *kinfo)
>   {
>       const void *fdt;
> @@ -1305,7 +1366,11 @@ int construct_dom0(struct domain *d)
I bet the change in construct_dom0 should be more complex than the 4 
lines. For instance, initrd_load fixup the DTB when the initrd is present.
How initrd should be loaded in presence of ACPI?

>       allocate_memory(d, &kinfo);
>
> -    rc = prepare_dtb(d, &kinfo);
> +    if (acpi_disabled)
> +        rc = prepare_dtb(d, &kinfo);
The name prepare_dtb is now confusing. We should find a better name.

> +    else
> +        rc = prepare_dtb_acpi(d, &kinfo);
> +
>       if ( rc < 0 )
>           return rc;
Regards,

--
Julien Grall

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