All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] xen/arm: Sanity test specified domain's memory for being a multiple of a page size
@ 2026-10-07 12:48 Michal Orzel
  2026-10-07 13:40 ` Alejandro Vallejo
  0 siblings, 1 reply; 7+ messages in thread
From: Michal Orzel @ 2026-10-07 12:48 UTC (permalink / raw)
  To: xen-devel
  Cc: Michal Orzel, Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, Anthony PERARD, Jan Beulich,
	Roger Pau Monné

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@amd.com>
---
 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).
 
 - 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.
 
 ### 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;
+    }
 
     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",
+               d, mem);
+        return -EINVAL;
+    }
+
     rc = domain_p2m_set_allocation(d, mem, node);
     if ( rc != 0 )
         return rc;
-- 
2.43.0



^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] xen/arm: Sanity test specified domain's memory for being a multiple of a page size
  2026-10-07 12:48 [PATCH] xen/arm: Sanity test specified domain's memory for being a multiple of a page size Michal Orzel
@ 2026-10-07 13:40 ` Alejandro Vallejo
  2026-10-07 14:11   ` Orzel, Michal
  0 siblings, 1 reply; 7+ messages in thread
From: Alejandro Vallejo @ 2026-10-07 13:40 UTC (permalink / raw)
  To: Michal Orzel, xen-devel
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, Anthony PERARD, Jan Beulich,
	Roger Pau Monné

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@amd.com>
> ---
>  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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xen/arm: Sanity test specified domain's memory for being a multiple of a page size
  2026-10-07 13:40 ` Alejandro Vallejo
@ 2026-10-07 14:11   ` Orzel, Michal
  2026-10-07 14:21     ` Alejandro Vallejo
  0 siblings, 1 reply; 7+ messages in thread
From: Orzel, Michal @ 2026-10-07 14:11 UTC (permalink / raw)
  To: Alejandro Vallejo, xen-devel
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, Anthony PERARD, Jan Beulich,
	Roger Pau Monné



On 07-Oct-26 15:40, Alejandro Vallejo wrote:
> 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@amd.com>
>> ---
>>  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
Is it? I don't think it matters at all seeing the number of occurrences in the code.

> 
> But page size isn't always 4KiB on all ports. PowerPC has 64KiB pages.
> IMO, 4KB shouldn't be here or elsewhere in documentation.
This is Arm doc and on Arm we only support 4KB at the moment. User should not
need to dig into the code to check what is the page size. It's better imo to
give this information right away and change in the future when we add support
for 16 and 64KB pages on Arm.

> 
>>  
>>  - 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.
Still Arm only section and see my reasoning above.

> 
>>  
>>  ### 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?
I can use XENLOG_ERR.

> 
> 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()?
1. It should not be self-healing. On Arm and in DT subsystem in general we chose
to follow the contract to bail out on unsatisfied user requests rather than
silently adjusting them.
2. I think it's always better to propagate the error to the caller if possible.
Sometimes you don't know where this function end up being used in the future. In
general if the function returns non-void I tend to leave the decision (here
panic will be executed anyway) to the caller. Printk explains why and the panic
follow. This also matches the existing paths nearby this code.

> 
>>  
>>      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.
Yes, I can print page size here.

> 
> 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

~Michal



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xen/arm: Sanity test specified domain's memory for being a multiple of a page size
  2026-10-07 14:11   ` Orzel, Michal
@ 2026-10-07 14:21     ` Alejandro Vallejo
  2026-10-07 14:31       ` Orzel, Michal
  0 siblings, 1 reply; 7+ messages in thread
From: Alejandro Vallejo @ 2026-10-07 14:21 UTC (permalink / raw)
  To: Orzel, Michal, xen-devel
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, Anthony PERARD, Jan Beulich,
	Roger Pau Monné

On Wed Oct 7, 2026 at 4:11 PM CEST, Orzel, Michal wrote:
>
>
> On 07-Oct-26 15:40, Alejandro Vallejo wrote:
> > 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@amd.com>
> >> ---
> >>  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
> Is it? I don't think it matters at all seeing the number of occurrences in the code.
>
> > 
> > But page size isn't always 4KiB on all ports. PowerPC has 64KiB pages.
> > IMO, 4KB shouldn't be here or elsewhere in documentation.
> This is Arm doc and on Arm we only support 4KB at the moment. User should not
> need to dig into the code to check what is the page size. It's better imo to
> give this information right away and change in the future when we add support
> for 16 and 64KB pages on Arm.

This is already the only reference the other ports have for dom0less.
The document shouldn't be here since the movement of dom0less to common
code, but the side effect of it is that this no longer an arm-only reference.

I won't be annoying about moving the doc on this patch because that's a
separate matter, but not adding more easily avoidable debt seems
warranted.

>
> > 
> >>  
> >>  - 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.
> Still Arm only section and see my reasoning above.
>
> > 
> >>  
> >>  ### 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?
> I can use XENLOG_ERR.
>
> > 
> > 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()?
> 1. It should not be self-healing. On Arm and in DT subsystem in general we chose
> to follow the contract to bail out on unsatisfied user requests rather than
> silently adjusting them.
> 2. I think it's always better to propagate the error to the caller if possible.
> Sometimes you don't know where this function end up being used in the future. In
> general if the function returns non-void I tend to leave the decision (here
> panic will be executed anyway) to the caller. Printk explains why and the panic
> follow. This also matches the existing paths nearby this code.

Fair enough.

>
> > 
> >>  
> >>      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.
> Yes, I can print page size here.
>
> > 
> > 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
>
> ~Michal



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xen/arm: Sanity test specified domain's memory for being a multiple of a page size
  2026-10-07 14:21     ` Alejandro Vallejo
@ 2026-10-07 14:31       ` Orzel, Michal
  2026-10-09  8:27         ` Alejandro Vallejo
  0 siblings, 1 reply; 7+ messages in thread
From: Orzel, Michal @ 2026-10-07 14:31 UTC (permalink / raw)
  To: Alejandro Vallejo, xen-devel
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, Anthony PERARD, Jan Beulich,
	Roger Pau Monné



On 07-Oct-26 16:21, Alejandro Vallejo wrote:
> On Wed Oct 7, 2026 at 4:11 PM CEST, Orzel, Michal wrote:
>>
>>
>> On 07-Oct-26 15:40, Alejandro Vallejo wrote:
>>> 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@amd.com>
>>>> ---
>>>>  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
>> Is it? I don't think it matters at all seeing the number of occurrences in the code.
>>
>>>
>>> But page size isn't always 4KiB on all ports. PowerPC has 64KiB pages.
>>> IMO, 4KB shouldn't be here or elsewhere in documentation.
>> This is Arm doc and on Arm we only support 4KB at the moment. User should not
>> need to dig into the code to check what is the page size. It's better imo to
>> give this information right away and change in the future when we add support
>> for 16 and 64KB pages on Arm.
> 
> This is already the only reference the other ports have for dom0less.
> The document shouldn't be here since the movement of dom0less to common
> code, but the side effect of it is that this no longer an arm-only reference.
> 
> I won't be annoying about moving the doc on this patch because that's a
> separate matter, but not adding more easily avoidable debt seems
> warranted.
I don't quite understand your reasoning. If you worry about dom0less != Arm,
then on that basis HAS_DOM0LESS is selected only for Arm and RISCV where
PAGE_SIZE is 4KB. I don't think I'm introducing any doubt here. I can remove the
4KB mentioning from the docs if you really insist but at least for me it's
clearer for the user to know right away what's the requirement for the size.

~Michal



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xen/arm: Sanity test specified domain's memory for being a multiple of a page size
  2026-10-07 14:31       ` Orzel, Michal
@ 2026-10-09  8:27         ` Alejandro Vallejo
  2026-10-12  8:48           ` Orzel, Michal
  0 siblings, 1 reply; 7+ messages in thread
From: Alejandro Vallejo @ 2026-10-09  8:27 UTC (permalink / raw)
  To: Orzel, Michal, xen-devel
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, Anthony PERARD, Jan Beulich,
	Roger Pau Monné

On Wed Oct 7, 2026 at 4:31 PM CEST, Orzel, Michal wrote:
>
>
> On 07-Oct-26 16:21, Alejandro Vallejo wrote:
> > On Wed Oct 7, 2026 at 4:11 PM CEST, Orzel, Michal wrote:
> >>
> >>
> >> On 07-Oct-26 15:40, Alejandro Vallejo wrote:
> >>> 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@amd.com>
> >>>> ---
> >>>>  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
> >> Is it? I don't think it matters at all seeing the number of occurrences in the code.
> >>
> >>>
> >>> But page size isn't always 4KiB on all ports. PowerPC has 64KiB pages.
> >>> IMO, 4KB shouldn't be here or elsewhere in documentation.
> >> This is Arm doc and on Arm we only support 4KB at the moment. User should not
> >> need to dig into the code to check what is the page size. It's better imo to
> >> give this information right away and change in the future when we add support
> >> for 16 and 64KB pages on Arm.
> > 
> > This is already the only reference the other ports have for dom0less.
> > The document shouldn't be here since the movement of dom0less to common
> > code, but the side effect of it is that this no longer an arm-only reference.
> > 
> > I won't be annoying about moving the doc on this patch because that's a
> > separate matter, but not adding more easily avoidable debt seems
> > warranted.
> I don't quite understand your reasoning. If you worry about dom0less != Arm,
> then on that basis HAS_DOM0LESS is selected only for Arm and RISCV where
> PAGE_SIZE is 4KB. I don't think I'm introducing any doubt here. I can remove the
> 4KB mentioning from the docs if you really insist but at least for me it's
> clearer for the user to know right away what's the requirement for the size.

It's a small adjustment to make the documentation truly generic. And
prevents the pgsz == 4KiB assumption from spreading further. Many docs
are spawned from copying other docs.

If you're so strongly against, that's fine; it's not incorrect today.
I still think the 4K should be gone, but if don't want to block the
patch on such silly things.

  Reviewed-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>

Cheers,
Alejandro

>
> ~Michal



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] xen/arm: Sanity test specified domain's memory for being a multiple of a page size
  2026-10-09  8:27         ` Alejandro Vallejo
@ 2026-10-12  8:48           ` Orzel, Michal
  0 siblings, 0 replies; 7+ messages in thread
From: Orzel, Michal @ 2026-10-12  8:48 UTC (permalink / raw)
  To: Alejandro Vallejo, xen-devel
  Cc: Stefano Stabellini, Julien Grall, Bertrand Marquis,
	Volodymyr Babchuk, Andrew Cooper, Anthony PERARD, Jan Beulich,
	Roger Pau Monné



On 09-Oct-26 10:27, Alejandro Vallejo wrote:
> On Wed Oct 7, 2026 at 4:31 PM CEST, Orzel, Michal wrote:
>>
>>
>> On 07-Oct-26 16:21, Alejandro Vallejo wrote:
>>> On Wed Oct 7, 2026 at 4:11 PM CEST, Orzel, Michal wrote:
>>>>
>>>>
>>>> On 07-Oct-26 15:40, Alejandro Vallejo wrote:
>>>>> 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@amd.com>
>>>>>> ---
>>>>>>  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
>>>> Is it? I don't think it matters at all seeing the number of occurrences in the code.
>>>>
>>>>>
>>>>> But page size isn't always 4KiB on all ports. PowerPC has 64KiB pages.
>>>>> IMO, 4KB shouldn't be here or elsewhere in documentation.
>>>> This is Arm doc and on Arm we only support 4KB at the moment. User should not
>>>> need to dig into the code to check what is the page size. It's better imo to
>>>> give this information right away and change in the future when we add support
>>>> for 16 and 64KB pages on Arm.
>>>
>>> This is already the only reference the other ports have for dom0less.
>>> The document shouldn't be here since the movement of dom0less to common
>>> code, but the side effect of it is that this no longer an arm-only reference.
>>>
>>> I won't be annoying about moving the doc on this patch because that's a
>>> separate matter, but not adding more easily avoidable debt seems
>>> warranted.
>> I don't quite understand your reasoning. If you worry about dom0less != Arm,
>> then on that basis HAS_DOM0LESS is selected only for Arm and RISCV where
>> PAGE_SIZE is 4KB. I don't think I'm introducing any doubt here. I can remove the
>> 4KB mentioning from the docs if you really insist but at least for me it's
>> clearer for the user to know right away what's the requirement for the size.
> 
> It's a small adjustment to make the documentation truly generic. And
> prevents the pgsz == 4KiB assumption from spreading further. Many docs
> are spawned from copying other docs.
> 
> If you're so strongly against, that's fine; it's not incorrect today.
> I still think the 4K should be gone, but if don't want to block the
> patch on such silly things.
Fine, I'll remove 4KB mentioning from the docs.

> 
>   Reviewed-by: Alejandro Vallejo <alejandro.garciavallejo@amd.com>
Thanks. I'll handle the changes on commit.

~Michal



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-10-12  8:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-07 12:48 [PATCH] xen/arm: Sanity test specified domain's memory for being a multiple of a page size Michal Orzel
2026-10-07 13:40 ` Alejandro Vallejo
2026-10-07 14:11   ` Orzel, Michal
2026-10-07 14:21     ` Alejandro Vallejo
2026-10-07 14:31       ` Orzel, Michal
2026-10-09  8:27         ` Alejandro Vallejo
2026-10-12  8:48           ` Orzel, Michal

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.