Re: [PATCH v9 3/3] xen/domain: introduce CONFIG_MAX_DOMID

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Wed, 28 May 2025, dmkhn@xxxxxxxxx wrote:
> From: Denis Mukhin <dmkhn@xxxxxxxxx>
> 
> From: Denis Mukhin <dmukhin@xxxxxxxx>
> 
> Embedded deployments of Xen do not need to have support for more than dozen of
> domains.
> 
> Introduce build-time configuration option to limit the number of domains 
> during
> run-time.
> 
> Suggested-by: Julien Grall <julien@xxxxxxx>
> Signed-off-by: Denis Mukhin <dmukhin@xxxxxxxx>

There is one DOMID_FIRST_RESERVED check in xen/arch/arm/tee/ffa.c that
should be changed too


> ---
> Changes since v8:
> - dropped hunk w/ compile-time check for DOMID_FIRST_RESERVED
> - updated CONFIG_MAX_DOMID explanation
> - dropped public header file changes
> ---
>  xen/arch/x86/cpu/mcheck/mce.c       |  2 +-
>  xen/arch/x86/cpu/vpmu.c             |  2 +-
>  xen/common/Kconfig                  |  8 ++++++++
>  xen/common/domain.c                 | 20 +++++++++++---------
>  xen/common/sched/core.c             |  4 ++--
>  xen/drivers/passthrough/vtd/iommu.c |  2 +-
>  6 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
> index 1c348e557d..ee8ddd33b0 100644
> --- a/xen/arch/x86/cpu/mcheck/mce.c
> +++ b/xen/arch/x86/cpu/mcheck/mce.c
> @@ -1493,7 +1493,7 @@ long do_mca(XEN_GUEST_HANDLE_PARAM(xen_mc_t) u_xen_mc)
>              d = rcu_lock_domain_by_any_id(mc_msrinject->mcinj_domid);
>              if ( d == NULL )
>              {
> -                if ( mc_msrinject->mcinj_domid >= DOMID_FIRST_RESERVED )
> +                if ( mc_msrinject->mcinj_domid >= CONFIG_MAX_DOMID )
>                      return x86_mcerr("do_mca inject: incompatible flag "
>                                       "MC_MSRINJ_F_GPADDR with domain %d",
>                                       -EINVAL, domid);
> diff --git a/xen/arch/x86/cpu/vpmu.c b/xen/arch/x86/cpu/vpmu.c
> index c28192ea26..67d423e088 100644
> --- a/xen/arch/x86/cpu/vpmu.c
> +++ b/xen/arch/x86/cpu/vpmu.c
> @@ -174,7 +174,7 @@ void vpmu_do_interrupt(void)
>       * in XENPMU_MODE_ALL, for everyone.
>       */
>      if ( (vpmu_mode & XENPMU_MODE_ALL) ||
> -         (sampled->domain->domain_id >= DOMID_FIRST_RESERVED) )
> +         (sampled->domain->domain_id >= CONFIG_MAX_DOMID) )
>      {
>          sampling = choose_hwdom_vcpu();
>          if ( !sampling )
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index 3d66d09397..ef083856b8 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -579,4 +579,12 @@ config BUDDY_ALLOCATOR_SIZE
>         Amount of memory reserved for the buddy allocator to serve Xen heap,
>         working alongside the colored one.
>  
> +config MAX_DOMID
> +     int "Maximum domain ID"
> +     range 1 32752
> +     default 32752
> +     help
> +       Specifies the maximum domain ID (dom0 or late hwdom, predefined
> +       domains, post-boot domains, excluding Xen system domains).

Written like this it would seem that the maximum domain ID is usable,
e.g. that 32752 is a valid domid number. Actually 32752 is 0x7ff0 which
is DOMID_FIRST_RESERVED == DOMID_SELF and cannot be used.

I think we should change the description:


Specifies the maximum domain ID (dom0 or late hwdom, predefined domains,
post-boot domains, excluding Xen system domains). This value indicates
the first domain ID that is out of bounds and cannot be used for domain
allocation.



>  endmenu
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 129b4fcb37..87e5be35e5 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -68,7 +68,7 @@ struct domain *domain_list;
>  
>  /* Non-system domain ID allocator. */
>  static DEFINE_SPINLOCK(domid_lock);
> -static DECLARE_BITMAP(domid_bitmap, DOMID_FIRST_RESERVED);
> +static DECLARE_BITMAP(domid_bitmap, CONFIG_MAX_DOMID);
>  
>  /*
>   * Insert a domain into the domlist/hash.  This allows the domain to be 
> looked
> @@ -154,7 +154,7 @@ int domain_init_states(void)
>      ASSERT(rw_is_write_locked_by_me(&current->domain->event_lock));
>  
>      dom_state_changed = xvzalloc_array(unsigned long,
> -                                       BITS_TO_LONGS(DOMID_FIRST_RESERVED));
> +                                       BITS_TO_LONGS(CONFIG_MAX_DOMID));
>      if ( !dom_state_changed )
>          return -ENOMEM;
>  
> @@ -234,7 +234,7 @@ int get_domain_state(struct xen_domctl_get_domain_state 
> *info, struct domain *d,
>      while ( dom_state_changed )
>      {
>          dom = find_first_bit(dom_state_changed, DOMID_MASK + 1);
> -        if ( dom >= DOMID_FIRST_RESERVED )
> +        if ( dom >= CONFIG_MAX_DOMID )
>              break;
>          if ( test_and_clear_bit(dom, dom_state_changed) )
>          {
> @@ -823,7 +823,7 @@ struct domain *domain_create(domid_t domid,
>      /* Sort out our idea of is_hardware_domain(). */
>      if ( (flags & CDF_hardware) || domid == hardware_domid )
>      {
> -        if ( hardware_domid < 0 || hardware_domid >= DOMID_FIRST_RESERVED )
> +        if ( hardware_domid < 0 || hardware_domid >= CONFIG_MAX_DOMID )
>              panic("The value of hardware_dom must be a valid domain ID\n");
>  
>          /* late_hwdom is only allowed for dom0. */
> @@ -2413,9 +2413,11 @@ domid_t get_initial_domain_id(void)
>  
>  domid_t domid_alloc(domid_t domid)
>  {
> +    BUILD_BUG_ON(DOMID_FIRST_RESERVED < CONFIG_MAX_DOMID);
> +
>      spin_lock(&domid_lock);
>  
> -    if ( domid < DOMID_FIRST_RESERVED )
> +    if ( domid < CONFIG_MAX_DOMID )
>      {
>          if ( __test_and_set_bit(domid, domid_bitmap) )
>              domid = DOMID_INVALID;
> @@ -2427,13 +2429,13 @@ domid_t domid_alloc(domid_t domid)
>          const domid_t reserved_domid = get_initial_domain_id();
>          const bool reserved = __test_and_set_bit(reserved_domid, 
> domid_bitmap);
>  
> -        domid = find_next_zero_bit(domid_bitmap, DOMID_FIRST_RESERVED,
> +        domid = find_next_zero_bit(domid_bitmap, CONFIG_MAX_DOMID,
>                                     domid_last);
>  
> -        if ( domid == DOMID_FIRST_RESERVED )
> -            domid = find_next_zero_bit(domid_bitmap, DOMID_FIRST_RESERVED, 
> 0);
> +        if ( domid == CONFIG_MAX_DOMID )
> +            domid = find_next_zero_bit(domid_bitmap, CONFIG_MAX_DOMID, 0);
>  
> -        if ( domid == DOMID_FIRST_RESERVED )
> +        if ( domid == CONFIG_MAX_DOMID )
>          {
>              domid = DOMID_INVALID;
>          }
> diff --git a/xen/common/sched/core.c b/xen/common/sched/core.c
> index 9043414290..f1bfb6f6a2 100644
> --- a/xen/common/sched/core.c
> +++ b/xen/common/sched/core.c
> @@ -867,7 +867,7 @@ int sched_init_domain(struct domain *d, unsigned int 
> poolid)
>      int ret;
>  
>      ASSERT(d->cpupool == NULL);
> -    ASSERT(d->domain_id < DOMID_FIRST_RESERVED);
> +    ASSERT(d->domain_id < CONFIG_MAX_DOMID);
>  
>      if ( (ret = cpupool_add_domain(d, poolid)) )
>          return ret;
> @@ -891,7 +891,7 @@ int sched_init_domain(struct domain *d, unsigned int 
> poolid)
>  
>  void sched_destroy_domain(struct domain *d)
>  {
> -    ASSERT(d->domain_id < DOMID_FIRST_RESERVED);
> +    ASSERT(d->domain_id < CONFIG_MAX_DOMID);
>  
>      if ( d->cpupool )
>      {
> diff --git a/xen/drivers/passthrough/vtd/iommu.c 
> b/xen/drivers/passthrough/vtd/iommu.c
> index c55f02c97e..5df85ca629 100644
> --- a/xen/drivers/passthrough/vtd/iommu.c
> +++ b/xen/drivers/passthrough/vtd/iommu.c
> @@ -1509,7 +1509,7 @@ int domain_context_mapping_one(
>  
>          prev_did = context_domain_id(lctxt);
>          domid = did_to_domain_id(iommu, prev_did);
> -        if ( domid < DOMID_FIRST_RESERVED )
> +        if ( domid < CONFIG_MAX_DOMID )
>              prev_dom = rcu_lock_domain_by_id(domid);
>          else if ( pdev ? domid == pdev->arch.pseudo_domid : domid > 
> DOMID_MASK )
>              prev_dom = rcu_lock_domain(dom_io);
> -- 
> 2.34.1
> 
>