Re: [PATCH] xen/arm: Sanity test specified domain's memory for being a multiple of a page size

["Alejandro Vallejo" <alejandro.garciavallejo@xxxxxxx>]

On Wed Oct 7, 2026 at 2:48 PM CEST, Michal Orzel wrote:
> We require memory to allocate for a domain as RAM to be a multiple of a
> page size. However, we neither document this nor sanity test. Specifying
> memory size that does not conform to this requirement fails the domain
> memory allocation in the non-obvious way that is difficult to parse for
> the user (printing over-allocation messages followed by the panic that Xen
> could not allocate the requested amount of memory).
>
> While there, fix indentation from tabs to spaces for "memory" parameter in
> booting.txt.
>
> Signed-off-by: Michal Orzel <michal.orzel@xxxxxxx>
> ---
>  docs/misc/arm/device-tree/booting.txt   | 4 ++--
>  docs/misc/xen-command-line.pandoc       | 5 +++--
>  xen/arch/arm/domain_build.c             | 6 ++++++
>  xen/common/device-tree/dom0less-build.c | 7 +++++++
>  4 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/docs/misc/arm/device-tree/booting.txt 
> b/docs/misc/arm/device-tree/booting.txt
> index bcb06bc796bf..7dfac2062a37 100644
> --- a/docs/misc/arm/device-tree/booting.txt
> +++ b/docs/misc/arm/device-tree/booting.txt
> @@ -155,8 +155,8 @@ with the following properties:
>  
>  - memory
>  
> -     A 64-bit integer specifying the amount of kilobytes of RAM to
> -    allocate to the guest.
> +    A 64-bit integer specifying the amount of kilobytes of RAM to
> +    allocate to the guest. Must be a multiple of 4KB (the page size).

nit: s/KB/KiB

But page size isn't always 4KiB on all ports. PowerPC has 64KiB pages.
IMO, 4KB shouldn't be here or elsewhere in documentation.

>  
>  - cpus
>  
> diff --git a/docs/misc/xen-command-line.pandoc 
> b/docs/misc/xen-command-line.pandoc
> index ea0e3368b1d6..d49db353d8a8 100644
> --- a/docs/misc/xen-command-line.pandoc
> +++ b/docs/misc/xen-command-line.pandoc
> @@ -1020,8 +1020,9 @@ and architecture-specific domain limits.
>  > `= <size>`
>  
>  Set the amount of memory for the initial domain (dom0). It must be
> -greater than zero. This parameter is required (and only used) when the 
> initial
> -domain is not described in the Device-Tree.
> +greater than zero and a multiple of the page size (4KB). This parameter is
> +required (and only used) when the initial domain is not described in the
> +Device-Tree.

Same 4KB remark.

>  
>  ### dom0_mem (x86)
>  > `= List of ( min:<sz> | max:<sz> | <sz> )`
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 72d531618045..bc3d1ebad1c4 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -1881,6 +1881,12 @@ static int __init construct_dom0(struct domain *d)
>          warning_add("PLEASE SPECIFY dom0_mem PARAMETER - USING 512M FOR 
> NOW\n");
>          dom0_mem = MB(512);
>      }
> +    else if ( dom0_mem % PAGE_SIZE )
> +    {
> +        printk("%pd: specified dom0_mem (%"PRIu64"B) is not a multiple of a 
> page size\n",
> +               d, dom0_mem);
> +        return -EINVAL;
> +    }

nit: XENLOG_WARNING?

Also, is this is meant to be self-healing? If so the return should just be:

    dom0_mem = round_pgdown(dom0_mem);

and continue. And if it isn't meant to be self-healing, why not just panic()?

>  
>      d->max_pages = dom0_mem >> PAGE_SHIFT;
>  
> diff --git a/xen/common/device-tree/dom0less-build.c 
> b/xen/common/device-tree/dom0less-build.c
> index fcbeb8adbd73..b3f24e5d555f 100644
> --- a/xen/common/device-tree/dom0less-build.c
> +++ b/xen/common/device-tree/dom0less-build.c
> @@ -789,6 +789,13 @@ static int __init construct_domU(struct kernel_info 
> *kinfo,
>      }
>      kinfo->unassigned_mem = (paddr_t)mem * SZ_1K;
>  
> +    if ( kinfo->unassigned_mem % PAGE_SIZE )
> +    {
> +        printk("%pd: specified \"memory\" (%"PRIu64"KB) is not a multiple of 
> a page size\n",

Seeing how PowerPC has 64KiB it might be prudent to print PAGE_SIZE to
avoid surprises later.

Same remark about panic() vs self-healing (either is fine. It's this
middle ground that looks off).

> +               d, mem);
> +        return -EINVAL;
> +    }
> +
>      rc = domain_p2m_set_allocation(d, mem, node);
>      if ( rc != 0 )
>          return rc;

Cheers,
Alejandro