Re: [PATCH v2 2/7] xen/domain: introduce domid_alloc()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.04.2025 01:05, dmkhn@xxxxxxxxx wrote:
> From: Denis Mukhin <dmukhin@xxxxxxxx>
> 
> Move domain ID allocation during domain creation to a dedicated
> function domid_alloc().
> 
> Allocation algorithm:
> - If an explicit domain ID is provided, verify its availability and
>   use it if ID is unused;
> - Otherwise, perform an exhaustive search for the first available ID
>   within the [0..DOMID_FIRST_RESERVED) range, excluding hardware_domid.
> 
> This minimizes the use of max_init_domid in the code and, thus, is a
> prerequisite change for enabling console input rotation across domains
> with console input permission on x86 platforms (which currently is
> limited to dom0, PV shim and Xen).

By removing the updating of max_init_domid you do - afaict - break the
remaining use site(s) of the variable.

> @@ -1003,6 +1004,12 @@ static struct domain *__init create_dom0(struct 
> boot_info *bi)
>  
>      image = &bi->mods[idx];
>  
> +    rc = domid_alloc(get_initial_domain_id());
> +    if ( rc < 0 )
> +        panic("Cannot use domain ID %d (rc = %d)\n",
> +              get_initial_domain_id(), rc);
> +    domid = rc;
> +
>      if ( opt_dom0_pvh )
>      {
>          dom0_cfg.flags |= (XEN_DOMCTL_CDF_hvm |

Why does this need to move up, ...

> @@ -1017,7 +1024,6 @@ static struct domain *__init create_dom0(struct 
> boot_info *bi)
>          dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
>  
>      /* Create initial domain.  Not d0 for pvshim. */
> -    domid = get_initial_domain_id();

... disconnecting the logic from the comment that is relevant there, ...

>      d = domain_create(domid, &dom0_cfg, pv_shim ? 0 : CDF_privileged);

... and not so much here?

> --- a/xen/include/xen/domain.h
> +++ b/xen/include/xen/domain.h
> @@ -37,6 +37,9 @@ void arch_get_domain_info(const struct domain *d,
>  
>  domid_t get_initial_domain_id(void);
>  
> +#define DOMID_AUTO               (-1)
> +int domid_alloc(int hint);

Imo it would be better to use e.g. DOMID_INVALID as the "give me whatever
is available" indicator, allowing the function parameter to properly be
domid_t.

But first of all - can we please take a step back and re-evaluate whether
all of this re-arrangement that you're doing (not just in the patch here)
is really needed? It seems to me that it must be possible to do whatever
you ultimately want to do without re-writing quite a few pretty central
pieces that have been serving us fine for a long time. That is, rather
than make our code fit your desires, make your plans fit within the code
base we have.

Jan