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

[Oleksii Kurochko <oleksii.kurochko@gmail.com>]

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@gmail.com>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Changes in v6-8:
>>   - Nothing changed. Only rebase.
>> ---
>> Changes in v5:
>>   - Add Reviewed-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> 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@amd.com>

Thanks.

~ Oleksii