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

[dmkhn@xxxxxxxxx]

On Thu, May 22, 2025 at 09:01:51AM +0200, Jan Beulich wrote:
> On 22.05.2025 02:09, dmkhn@xxxxxxxxx wrote:
> > On Wed, May 21, 2025 at 09:31:34AM +0200, Jan Beulich wrote:
> >> On 21.05.2025 02:00, dmkhn@xxxxxxxxx wrote:
> >>> --- a/xen/arch/arm/tee/ffa.c
> >>> +++ b/xen/arch/arm/tee/ffa.c
> >>> @@ -331,10 +331,9 @@ static int ffa_domain_init(struct domain *d)
> >>>       * reserved for the hypervisor and we only support secure endpoints 
> >>> using
> >>>       * FF-A IDs with BIT 15 set to 1 so make sure those are not used by 
> >>> Xen.
> >>>       */
> >>> -    BUILD_BUG_ON(DOMID_FIRST_RESERVED >= UINT16_MAX);
> >>
> >> Why's this being moved to common code? It certainly may have a purpose here
> >> (which I'm simply unaware of); I don't see what purpose it has in common
> >> code.
> >
> > My understanding having DOMID_FIRST_RESERVED compile-time checks in one 
> > place
> > is good for testability: the check in question also applies to x86.
> >
> > I will drop that hunk.
> 
> And also the other one, unless you can explain what exactly you're checking.
> The connection between DOMID_FIRST_RESERVED and UINT16_MAX is at best
> indirect, through domid_t. Yet if domid_t was widened (possible in principle,
> but breaking the ABI) that check would end up wrong without the compiler
> noticing (unless DOMID_FIRST_RESERVED was also bumped, which however is an
> independent thing).

diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
index 82b9c05a76..452d9f63dc 100644
--- a/xen/include/public/xen.h
+++ b/xen/include/public/xen.h
@@ -572,7 +572,7 @@ DEFINE_XEN_GUEST_HANDLE(mmuext_op_t);
 #endif
 
 /* Domain ids >= DOMID_FIRST_RESERVED cannot be used for ordinary domains. */
-#define DOMID_FIRST_RESERVED xen_mk_uint(0x7FF0)
+#define DOMID_FIRST_RESERVED xen_mk_uint(0xFFFF7FF0)
 
 /* DOMID_SELF is used in certain contexts to refer to oneself. */
 #define DOMID_SELF           xen_mk_uint(0x7FF0)


The above patch ^^ pretty much passes the compilation on x86.
The compile time check in question currently exists for Arm only but could
catch such mistake when the limit is bumped (if any), it is easy to
spot.

> 
> >>> --- a/xen/common/Kconfig
> >>> +++ b/xen/common/Kconfig
> >>> @@ -576,4 +576,11 @@ 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 number of user domains"
> >>> + range 1 32752
> >>> + default 32752
> >>> + help
> >>> +   Specifies the maximum number of domains a user can create.
> >>
> >> My prior comment remains: The description and help needs to be accurate, in
> >> order to not cause any confusion. In a true dom0less environment I'm not
> >> sure the "user" can create any domains (post boot, that is). And when there
> >> is Dom0 (or late hwdom), the number specified already isn't the number of
> >> domains one can create (again, post boot, which is how I understand "user
> >> domains"). If someone picked 1 as the value here, it's unclear to me how
> >> late hwdom or dom0less would work in the first place.
> >
> > Do you think something like the following will be more accurate?
> >
> >     config MAX_DOMID
> >        int "Maximum number of domains"
> >        range 1 32752
> >        default 32752
> >        help
> >          Specifies the maximum number of domains: dom0 or late hwdom,
> >          predefined domains, post-boot domains, excluding Xen system domains
> >          (domid >= DOMID_FIRST_RESERVED).
> 
> Especially the mention of DOMID_FIRST_RESERVED is too much of an 
> implementation
> detail here, imo. Beyond that - maybe, but I'm not overly happy this way 
> either.

Will the following description will be satisfactory?

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

> 
> As an aside - MAX_DOMID and "Maximum number of domains" are conflicting
> with one another, too: Do you mean "maximum ID" or "maximum number of"? The 
> two
> are different by 1.

That would be "maximum ID", thank you.

> 
> Jan