Re: [PATCH v8 02/20] xen/dom0less: turn max_init_domid into a common variable

[Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>]

On 8/31/26 5:48 PM, Orzel, Michal wrote:
> On 27-Aug-26 17:18, Oleksii Kurochko wrote:
> > Until now every architecture carried its own notion of max_init_domid:
> > Arm defined a real variable (declared in asm/setup.h, defined in
> > setup.c), while ppc, riscv and x86 each provided a "#define
> > max_init_domid (0)" stub in their asm/setup.h. This duplicated the same
> > declaration across all arches and placed a purely dom0less concept in
> > arch setup headers.
> >
> > Now that the dom0less build code lives in common (xen/common/
> > device-tree/dom0less-build.c sets max_init_domid, and the console
> > serial-input switcher reads it), there is no reason for the symbol to be
> > per-arch. Provide a single declaration in <xen/dom0less-build.h>, with
> > the !CONFIG_DOM0LESS_BOOT stub kept there as well, so there is one source
> > of truth and the arch headers no longer need to mention it. Update
> > console.c to include <xen/dom0less-build.h> for the declaration instead
> > of relying on asm/setup.h.
> >
> > Place the definition in xen/common/domid.c rather than in dom0less-
> > build.c. The latter is built as dom0less-build.init.o, i.e. the whole
> > object is relocated into the .init.* sections and freed after boot,
> > whereas max_init_domid must outlive boot because it is read at runtime
> > by the console serial-input switcher. domid.c is always linked (obj-y)
> > and resides in regular (non-init) sections, so it is a correct home for
> > the variable. It is marked __ro_after_init since it is only updated
> > while creating boot-time domains and read-only afterwards, and guarded
> > by CONFIG_DOM0LESS_BOOT as domid.c itself is unconditional.
> >
> > Signed-off-by: Oleksii Kurochko <oleksii.kurochko@xxxxxxxxx>
> > Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > ---
> > Changes in v6-8:
> >   - Nothing changed. Only rebase.
> > ---
> > Changes in v5:
> >   - Add Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
> > ---
> > Changes in v4:
> >   - New patch.
> > ---
> > ---
> >   xen/arch/arm/include/asm/setup.h   | 2 --
> >   xen/arch/arm/setup.c               | 2 --
> >   xen/arch/ppc/include/asm/setup.h   | 2 --
> >   xen/arch/riscv/include/asm/setup.h | 2 --
> >   xen/arch/x86/include/asm/setup.h   | 2 --
> >   xen/common/domid.c                 | 5 +++++
> >   xen/drivers/char/console.c         | 1 +
> >   xen/include/xen/dom0less-build.h   | 7 +++++++
> >   8 files changed, 13 insertions(+), 10 deletions(-)
> >
> > diff --git a/xen/arch/arm/include/asm/setup.h b/xen/arch/arm/include/asm/setup.h
> > index 0adfa4993a8f..2af780512540 100644
> > --- a/xen/arch/arm/include/asm/setup.h
> > +++ b/xen/arch/arm/include/asm/setup.h
> > @@ -25,8 +25,6 @@ struct map_range_data
> >       struct rangeset *irq_ranges;
> >   };
> >   
> > -extern domid_t max_init_domid;
> > -
> >   void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
> >   
> >   size_t estimate_efi_size(unsigned int mem_nr_banks);
> > diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> > index 6310a47d68b6..86532d0a35b6 100644
> > --- a/xen/arch/arm/setup.c
> > +++ b/xen/arch/arm/setup.c
> > @@ -62,8 +62,6 @@ struct cpuinfo_arm __read_mostly system_cpuinfo;
> >   bool __read_mostly acpi_disabled;
> >   #endif
> >   
> > -domid_t __read_mostly max_init_domid;
> > -
> >   static __used void noreturn init_done(void)
> >   {
> >       /* Must be done past setting system_state. */
> > diff --git a/xen/arch/ppc/include/asm/setup.h b/xen/arch/ppc/include/asm/setup.h
> > index e4f64879b68c..956fa6985adb 100644
> > --- a/xen/arch/ppc/include/asm/setup.h
> > +++ b/xen/arch/ppc/include/asm/setup.h
> > @@ -1,6 +1,4 @@
> >   #ifndef __ASM_PPC_SETUP_H__
> >   #define __ASM_PPC_SETUP_H__
> >   
> > -#define max_init_domid (0)
> > -
> >   #endif /* __ASM_PPC_SETUP_H__ */
> > diff --git a/xen/arch/riscv/include/asm/setup.h 
> > b/xen/arch/riscv/include/asm/setup.h
> > index 2215894cfbb1..73ce2f293348 100644
> > --- a/xen/arch/riscv/include/asm/setup.h
> > +++ b/xen/arch/riscv/include/asm/setup.h
> > @@ -5,8 +5,6 @@
> >   
> >   #include <xen/types.h>
> >   
> > -#define max_init_domid (0)
> > -
> >   void setup_mm(void);
> >   
> >   void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
> > diff --git a/xen/arch/x86/include/asm/setup.h b/xen/arch/x86/include/asm/setup.h
> > index b01e83a8ed9f..5925c5f39cff 100644
> > --- a/xen/arch/x86/include/asm/setup.h
> > +++ b/xen/arch/x86/include/asm/setup.h
> > @@ -68,6 +68,4 @@ extern bool opt_dom0_verbose;
> >   extern bool opt_dom0_cpuid_faulting;
> >   extern bool opt_dom0_msr_relaxed;
> >   
> > -#define max_init_domid (0)
> > -
> >   #endif
> > diff --git a/xen/common/domid.c b/xen/common/domid.c
> > index b0258e477c1a..cd46cf952be6 100644
> > --- a/xen/common/domid.c
> > +++ b/xen/common/domid.c
> > @@ -9,6 +9,11 @@
> >    */
> >   
> >   #include <xen/domain.h>
> > +#include <xen/dom0less-build.h>
> NIT: "dom0less" comes before "domain" when it comes to alphabetical order I 
> think.
Agreed. With that re-ordering, <xen/types.h> also has to be added to 
<xen/dom0less-build.h>, as the latter includes <public/xen.h>, which 
uses the fixed-width types defined in <xen/types.h>. Without it, the 
build fails with "unknown type name 'uint64_t'".
I will re-order the includes and add <xen/types.h> to dom0less-build.h.

I will also added the following to commit message:

While at it, make <xen/dom0less-build.h> self-contained: it includes
<public/xen.h>, which uses the fixed-width types provided by
<xen/types.h>, so include the latter explicitly rather than relying on
the includer having pulled it in first. This becomes necessary as soon
as <xen/dom0less-build.h> comes first in an alphabetically sorted
include list, as it now does in domid.c.

> > +
> > +#ifdef CONFIG_DOM0LESS_BOOT
> > +domid_t __ro_after_init max_init_domid;
> > +#endif
> >   
> >   static DEFINE_SPINLOCK(domid_lock);
> >   static DECLARE_BITMAP(domid_bitmap, DOMID_FIRST_RESERVED);
> > diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c
> > index fcacf37c52f0..61e92491e40a 100644
> > --- a/xen/drivers/char/console.c
> > +++ b/xen/drivers/char/console.c
> > @@ -31,6 +31,7 @@
> >   #include <xen/warning.h>
> >   #include <xen/pv_console.h>
> >   #include <asm/setup.h>
> NIT: This can be dropped - I don't see anything relying on it in this file.
Checked that: nothing really depends on it + CI tests are passed.

I will also then add the following to commit message:

Update console.c to include <xen/dom0less-build.h> for the declaration
instead of relying on asm/setup.h, and drop the now unneeded
<asm/setup.h> include.

> Other than that:
> Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>
Thanks.

~ Oleksii