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

["Orzel, Michal" <michal.orzel@xxxxxxx>]

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.

> +
> +#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.

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

~Michal