Re: [PATCH] xen/arm: check max_init_domid validity

[Michal Orzel <michal.orzel@xxxxxxx>]

Hi Bertrand,

On 28/02/2023 09:08, Bertrand Marquis wrote:
> 
> 
> Before trying to create a dom0less guest, check that max_init_domid
> increment will generate a valid domain ID, lower than
> DOMID_FIRST_RESERVED.
> 
> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> ---
>  xen/arch/arm/domain_build.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index edca23b986d2..9707eb7b1bb1 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -3879,6 +3879,9 @@ void __init create_domUs(void)
>          if ( !dt_device_is_compatible(node, "xen,domain") )
>              continue;
> 
> +        if ( (max_init_domid + 1) >= DOMID_FIRST_RESERVED )
> +            panic("No more domain IDs available\n");
Here are some of my thoughts:
1. The check if domid is >= DOMID_FIRST_RESERVED is used in quite a lot of
places in the Xen code. We might want to introduce a global function for that 
purpose
instead of repeating this check all over the codebase.

2. This check is something that could be moved to be generic. At the moment we 
do have
an ASSERT with is_system_domain in domain_create. I know domain_create can be 
called for
domids in special range so this would need to be thought through.

3. The placement of this check at the top of the function before starting to 
parse dt properties
might be problematic in the future if we decide to allow specifying static 
domids for dom0less domUs.
In a static configuration, most of the time, we do not have xenstore (either 
because of lack of xenstore
support or because of lack of dom0). AFAIKT, in Xen a domain can get to know 
its domid only through xenstore
(DOMID_SELF is not working in all the cases). Also, in a static configuration, 
it makes the life of an integrator
easy to know all the domids upfront to easily set up some communication, grant 
tables, etc.

Let me know your thoughts.

~Michal