[PATCH v3] xen/domain: unify domain ID allocation

[dmkhn@xxxxxxxxx]

From: Denis Mukhin <dmukhin@xxxxxxxx>

Unify the logic of domain ID allocation, so that both the initial domain
creation and the usage by domctl use the same helper function across
architectures (Arm and x86).

Wrap the domain ID allocation as an arch-independent function domid_alloc() in
common/domain.c.

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.

Move the is_free_domid() helper closer to domid_alloc(). Simplify
is_free_domid() by removing the domain ID range check, as the ID is now
guaranteed to be within the valid range. Additionally, update the predicate to
return a bool value instead of an int.

Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>
---
Link to v2: 
https://lore.kernel.org/xen-devel/20250331230508.440198-3-dmukhin@xxxxxxxx/
CI: https://gitlab.com/xen-project/people/dmukhin/xen/-/pipelines/1770801847

I hope by moving it to an isolated thread the review will be simpler.

Changes since v2:
- added max_init_domid change so the patch does not need to depend on
  other patches
- moved uses of domid_alloc() close to domain_create()
- use DOMID_INVALID as the indicator for doing an exhaustive domain ID search
---
 xen/arch/arm/dom0less-build.c | 19 ++++++++-------
 xen/arch/arm/domain_build.c   | 19 +++++++++++----
 xen/arch/x86/setup.c          |  8 +++++--
 xen/common/domain.c           | 45 +++++++++++++++++++++++++++++++++++
 xen/common/domctl.c           | 45 ++++-------------------------------
 xen/include/xen/domain.h      |  2 ++
 6 files changed, 81 insertions(+), 57 deletions(-)

diff --git a/xen/arch/arm/dom0less-build.c b/xen/arch/arm/dom0less-build.c
index bd15563750..387ba938ba 100644
--- a/xen/arch/arm/dom0less-build.c
+++ b/xen/arch/arm/dom0less-build.c
@@ -993,15 +993,13 @@ void __init create_domUs(void)
             .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
         };
         unsigned int flags = 0U;
+        domid_t domid;
         uint32_t val;
         int rc;
 
         if ( !dt_device_is_compatible(node, "xen,domain") )
             continue;
 
-        if ( (max_init_domid + 1) >= DOMID_FIRST_RESERVED )
-            panic("No more domain IDs available\n");
-
         if ( dt_find_property(node, "xen,static-mem", NULL) )
         {
             if ( llc_coloring_enabled )
@@ -1125,12 +1123,15 @@ void __init create_domUs(void)
         if ( !llc_coloring_enabled && llc_colors_str )
             panic("'llc-colors' found, but LLC coloring is disabled\n");
 
-        /*
-         * The variable max_init_domid is initialized with zero, so here it's
-         * very important to use the pre-increment operator to call
-         * domain_create() with a domid > 0. (domid == 0 is reserved for Dom0)
-         */
-        d = domain_create(++max_init_domid, &d_cfg, flags);
+        rc = domid_alloc(DOMID_INVALID);
+        if ( rc < 0 )
+            panic("Error allocating ID for domain %s (rc = %d)\n",
+                  dt_node_name(node), rc);
+        domid = rc;
+        if ( max_init_domid < domid )
+            max_init_domid = domid;
+
+        d = domain_create(domid, &d_cfg, flags);
         if ( IS_ERR(d) )
             panic("Error creating domain %s (rc = %ld)\n",
                   dt_node_name(node), PTR_ERR(d));
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 634333cdde..2ee6ffd30b 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -2370,6 +2370,7 @@ void __init create_dom0(void)
         .grant_opts = XEN_DOMCTL_GRANT_version(opt_gnttab_max_version),
     };
     unsigned int flags = CDF_privileged;
+    domid_t domid;
     int rc;
 
     /* The vGIC for DOM0 is exactly emulating the hardware GIC */
@@ -2394,19 +2395,27 @@ void __init create_dom0(void)
     if ( !llc_coloring_enabled )
         flags |= CDF_directmap;
 
-    dom0 = domain_create(0, &dom0_cfg, flags);
+    rc = domid_alloc(get_initial_domain_id());
+    if ( rc < 0 )
+        panic("Error allocating domain ID %d (rc = %d)\n",
+              get_initial_domain_id(), rc);
+    domid = rc;
+
+    dom0 = domain_create(domid, &dom0_cfg, flags);
     if ( IS_ERR(dom0) )
-        panic("Error creating domain 0 (rc = %ld)\n", PTR_ERR(dom0));
+        panic("Error creating domain %d (rc = %ld)\n", domid, PTR_ERR(dom0));
 
     if ( llc_coloring_enabled && (rc = dom0_set_llc_colors(dom0)) )
-        panic("Error initializing LLC coloring for domain 0 (rc = %d)\n", rc);
+        panic("Error initializing LLC coloring for domain %d (rc = %d)\n",
+              domid, rc);
 
     if ( alloc_dom0_vcpu0(dom0) == NULL )
-        panic("Error creating domain 0 vcpu0\n");
+        panic("Error creating domain %d vcpu0\n", domid);
 
     rc = construct_dom0(dom0);
     if ( rc )
-        panic("Could not set up DOM0 guest OS (rc = %d)\n", rc);
+        panic("Could not set up guest OS for domain %d (rc = %d)\n",
+              domid, rc);
 }
 
 /*
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 3c257f0bad..e20a416bf4 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1009,8 +1009,12 @@ static struct domain *__init create_dom0(struct 
boot_info *bi)
     if ( iommu_enabled )
         dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu;
 
-    /* Create initial domain.  Not d0 for pvshim. */
-    bd->domid = get_initial_domain_id();
+    /* Allocate initial domain ID. Not d0 for pvshim. */
+    bd->domid = domid_alloc(get_initial_domain_id());
+    if ( bd->domid < 0 )
+        panic("Error allocating domain ID %d (rc = %d)\n",
+              get_initial_domain_id(), bd->domid);
+
     d = domain_create(bd->domid, &dom0_cfg, pv_shim ? 0 : CDF_privileged);
     if ( IS_ERR(d) )
         panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(d));
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 585fd726a9..db98d9c1b2 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -66,6 +66,51 @@ DEFINE_RCU_READ_LOCK(domlist_read_lock);
 static struct domain *domain_hash[DOMAIN_HASH_SIZE];
 struct domain *domain_list;
 
+static inline bool is_free_domid(domid_t dom)
+{
+    struct domain *d = rcu_lock_domain_by_id(dom);
+
+    if ( d )
+        rcu_unlock_domain(d);
+
+    return !d;
+}
+
+/*
+ * Allocate new domain ID based on the hint.
+ *
+ * If hint is outside of valid [0..DOMID_FIRST_RESERVED] range of IDs,
+ * perform an exhaustive search of the first free domain ID excluding
+ * hardware_domid.
+ */
+int domid_alloc(int hint)
+{
+    domid_t domid;
+
+    if ( hint >= 0 && hint < DOMID_FIRST_RESERVED )
+    {
+        if ( !is_free_domid(hint) )
+            return -EEXIST;
+
+        domid = hint;
+    }
+    else
+    {
+        for ( domid = 0; domid < DOMID_FIRST_RESERVED; domid++ )
+        {
+            if ( domid == hardware_domid )
+                continue;
+            if ( is_free_domid(domid) )
+                break;
+        }
+
+        if ( domid == DOMID_FIRST_RESERVED )
+            return -ENOMEM;
+    }
+
+    return domid;
+}
+
 /*
  * Insert a domain into the domlist/hash.  This allows the domain to be looked
  * up by domid, and therefore to be the subject of hypercalls/etc.
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index bfe2e1f9f0..3d21612660 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -49,20 +49,6 @@ static int xenctl_bitmap_to_nodemask(nodemask_t *nodemask,
                                    MAX_NUMNODES);
 }
 
-static inline int is_free_domid(domid_t dom)
-{
-    struct domain *d;
-
-    if ( dom >= DOMID_FIRST_RESERVED )
-        return 0;
-
-    if ( (d = rcu_lock_domain_by_id(dom)) == NULL )
-        return 1;
-
-    rcu_unlock_domain(d);
-    return 0;
-}
-
 void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
 {
     struct vcpu *v;
@@ -421,34 +407,11 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
 
     case XEN_DOMCTL_createdomain:
     {
-        domid_t        dom;
-        static domid_t rover = 0;
+        ret = domid_alloc(op->domain);
+        if ( ret < 0 )
+            break;
 
-        dom = op->domain;
-        if ( (dom > 0) && (dom < DOMID_FIRST_RESERVED) )
-        {
-            ret = -EEXIST;
-            if ( !is_free_domid(dom) )
-                break;
-        }
-        else
-        {
-            for ( dom = rover + 1; dom != rover; dom++ )
-            {
-                if ( dom == DOMID_FIRST_RESERVED )
-                    dom = 1;
-                if ( is_free_domid(dom) )
-                    break;
-            }
-
-            ret = -ENOMEM;
-            if ( dom == rover )
-                break;
-
-            rover = dom;
-        }
-
-        d = domain_create(dom, &op->u.createdomain, false);
+        d = domain_create(ret, &op->u.createdomain, false);
         if ( IS_ERR(d) )
         {
             ret = PTR_ERR(d);
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index a34daa7d10..9f2470b095 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -38,6 +38,8 @@ void arch_get_domain_info(const struct domain *d,
 
 domid_t get_initial_domain_id(void);
 
+int domid_alloc(int hint);
+
 /* CDF_* constant. Internal flags for domain creation. */
 /* Is this a privileged domain? */
 #define CDF_privileged           (1U << 0)
-- 
2.34.1