Re: [PATCH 2/2] xen/arm: Account for domU dtb bootmodule size separately

[Julien Grall <julien@xxxxxxx>]

Hi Michal,

On 11/07/2023 09:29, Michal Orzel wrote:
> At the moment, we limit the allocation size when creating a domU dtb to
> 4KB, which is not enough when using a passthrough dtb with several nodes.
> Improve the handling by accounting for a dtb bootmodule (if present)
> size separately, while keeping 4KB for the Xen generated nodes (still
> plenty of space for new nodes). Also, cap the allocation size to 2MB,
> which is the max dtb size allowed.
>
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> ---
> Note for the future:
> As discussed with Julien, really the best way would be to generate dtb directly
> in the guest memory, where no allocation would be necessary. This of course
> requires some rework. The solution in this patch is good enough for now and
> can be treated as an intermediated step to support dtb creation of various
> sizes.
Thanks for summarizing our discussion :).

> ---
>   xen/arch/arm/domain_build.c | 18 +++++++++++++-----
>   1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index f2134f24b971..1dc0eca37bd6 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -3257,14 +3257,15 @@ static int __init domain_handle_dtb_bootmodule(struct 
> domain *d,
>   }
>   
>   /*
> - * The max size for DT is 2MB. However, the generated DT is small, 4KB
> - * are enough for now, but we might have to increase it in the future.
> + * The max size for DT is 2MB. However, the generated DT is small (not 
> including
> + * domU passthrough DT nodes whose size we account separately), 4KB are enough
> + * for now, but we might have to increase it in the future.
>    */
>   #define DOMU_DTB_SIZE 4096
>   static int __init prepare_dtb_domU(struct domain *d, struct kernel_info 
> *kinfo)
>   {
>       int addrcells, sizecells;
> -    int ret;
> +    int ret, fdt_size = DOMU_DTB_SIZE;
Can fdt_size be unsigned?

>   
>       kinfo->phandle_gic = GUEST_PHANDLE_GIC;
>       kinfo->gnttab_start = GUEST_GNTTAB_BASE;
> @@ -3273,11 +3274,18 @@ static int __init prepare_dtb_domU(struct domain *d, 
> struct kernel_info *kinfo)
>       addrcells = GUEST_ROOT_ADDRESS_CELLS;
>       sizecells = GUEST_ROOT_SIZE_CELLS;
>   
> -    kinfo->fdt = xmalloc_bytes(DOMU_DTB_SIZE);
> +    /* Account for domU passthrough DT size */
> +    if ( kinfo->dtb_bootmodule )
> +        fdt_size += kinfo->dtb_bootmodule->size;
> +
> +    /* Cap to max DT size if needed */
> +    fdt_size = min(fdt_size, SZ_2M);
> +
> +    kinfo->fdt = xmalloc_bytes(fdt_size);
>       if ( kinfo->fdt == NULL )
>           return -ENOMEM;
>   
> -    ret = fdt_create(kinfo->fdt, DOMU_DTB_SIZE);
> +    ret = fdt_create(kinfo->fdt, fdt_size);
>       if ( ret < 0 )
>           goto err;
>   
Cheers,

--
Julien Grall