Xen project Mailing List

Re: [PATCH v2 07/15] x86: Replace boot_domain with bootdomain

To: Alejandro Vallejo <agarciav@xxxxxxx>

From: Stefano Stabellini <sstabellini@xxxxxxxxxx>

Date: Mon, 9 Jun 2025 09:56:18 -0700 (PDT)

Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, xen-devel@xxxxxxxxxxxxxxxxxxxx, Jan Beulich <jbeulich@xxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Michal Orzel <michal.orzel@xxxxxxx>, "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>

Delivery-date: Mon, 09 Jun 2025 16:56:27 +0000

List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On Mon, 9 Jun 2025, Alejandro Vallejo wrote: > On Sat Jun 7, 2025 at 1:28 AM CEST, Stefano Stabellini wrote: > > On Thu, 5 Jun 2025, Alejandro Vallejo wrote: > >> No functional change intended. > >> > >> Signed-off-by: Alejandro Vallejo <agarciav@xxxxxxx> > >> --- > >> v2: > >> * Replaces the previous patch in which kernel_info replaced boot_domain > >> --- > >> xen/arch/x86/dom0_build.c | 2 +- > >> xen/arch/x86/hvm/dom0_build.c | 6 ++--- > >> xen/arch/x86/include/asm/boot-domain.h | 33 -------------------------- > >> xen/arch/x86/include/asm/bootfdt.h | 7 ++++++ > >> xen/arch/x86/include/asm/bootinfo.h | 3 +-- > >> xen/arch/x86/include/asm/dom0_build.h | 6 ++--- > >> xen/arch/x86/include/asm/setup.h | 4 ++-- > >> xen/arch/x86/pv/dom0_build.c | 6 ++--- > >> xen/arch/x86/setup.c | 16 +++++++------ > >> xen/include/xen/bootfdt.h | 4 ++++ > >> 10 files changed, 33 insertions(+), 54 deletions(-) > >> delete mode 100644 xen/arch/x86/include/asm/boot-domain.h > >> > >> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c > >> index 0b467fd4a4..1c8c5140a3 100644 > >> --- a/xen/arch/x86/dom0_build.c > >> +++ b/xen/arch/x86/dom0_build.c > >> @@ -615,7 +615,7 @@ int __init dom0_setup_permissions(struct domain *d) > >> return rc; > >> } > >> > >> -int __init construct_dom0(const struct boot_domain *bd) > >> +int __init construct_dom0(const struct bootdomain *bd) > >> { > >> int rc; > >> const struct domain *d = bd->d; > >> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c > >> index 96410344a8..85c000b259 100644 > >> --- a/xen/arch/x86/hvm/dom0_build.c > >> +++ b/xen/arch/x86/hvm/dom0_build.c > >> @@ -644,11 +644,11 @@ static bool __init check_and_adjust_load_address( > >> } > >> > >> static int __init pvh_load_kernel( > >> - const struct boot_domain *bd, paddr_t *entry, paddr_t > >> *start_info_addr) > >> + const struct bootdomain *bd, paddr_t *entry, paddr_t *start_info_addr) > >> { > >> struct domain *d = bd->d; > >> struct bootmodule *image = bd->kernel; > >> - struct bootmodule *initrd = bd->module; > >> + struct bootmodule *initrd = bd->initrd; > >> void *image_base = bootstrap_map_bm(image); > >> void *image_start = image_base + image->arch.headroom; > >> unsigned long image_len = image->size; > >> @@ -1329,7 +1329,7 @@ static void __hwdom_init pvh_setup_mmcfg(struct > >> domain *d) > >> } > >> } > >> > >> -int __init dom0_construct_pvh(const struct boot_domain *bd) > >> +int __init dom0_construct_pvh(const struct bootdomain *bd) > >> { > >> paddr_t entry, start_info; > >> struct domain *d = bd->d; > >> diff --git a/xen/arch/x86/include/asm/boot-domain.h > >> b/xen/arch/x86/include/asm/boot-domain.h > >> deleted file mode 100644 > >> index 242e9c9c2b..0000000000 > >> --- a/xen/arch/x86/include/asm/boot-domain.h > >> +++ /dev/null > >> @@ -1,33 +0,0 @@ > >> -/* SPDX-License-Identifier: GPL-2.0-or-later */ > >> -/* > >> - * Copyright (c) 2024 Apertus Solutions, LLC > >> - * Author: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx> > >> - * Copyright (c) 2024 Christopher Clark <christopher.w.clark@xxxxxxxxx> > >> - */ > >> - > >> -#ifndef __XEN_X86_BOOTDOMAIN_H__ > >> -#define __XEN_X86_BOOTDOMAIN_H__ > >> - > >> -#include <public/xen.h> > >> - > >> -struct boot_domain { > >> - domid_t domid; > >> - > >> - struct bootmodule *kernel; > >> - struct bootmodule *module; > >> - const char *cmdline; > >> - > >> - struct domain *d; > >> -}; > >> - > >> -#endif > >> - > >> -/* > >> - * Local variables: > >> - * mode: C > >> - * c-file-style: "BSD" > >> - * c-basic-offset: 4 > >> - * tab-width: 4 > >> - * indent-tabs-mode: nil > >> - * End: > >> - */ > >> diff --git a/xen/arch/x86/include/asm/bootfdt.h > >> b/xen/arch/x86/include/asm/bootfdt.h > >> index 2fc705a1cd..b30132381e 100644 > >> --- a/xen/arch/x86/include/asm/bootfdt.h > >> +++ b/xen/arch/x86/include/asm/bootfdt.h > >> @@ -4,6 +4,13 @@ > >> > >> #include <xen/types.h> > >> > >> +#include <public/xen.h> > >> + > >> +struct arch_bootdomain > >> +{ > >> + domid_t domid; > >> +}; > >> + > >> struct arch_bootmodule > >> { > >> /* > >> diff --git a/xen/arch/x86/include/asm/bootinfo.h > >> b/xen/arch/x86/include/asm/bootinfo.h > >> index f3210b7d6a..b8280ba357 100644 > >> --- a/xen/arch/x86/include/asm/bootinfo.h > >> +++ b/xen/arch/x86/include/asm/bootinfo.h > >> @@ -12,7 +12,6 @@ > >> #include <xen/init.h> > >> #include <xen/multiboot.h> > >> #include <xen/types.h> > >> -#include <asm/boot-domain.h> > >> > >> /* Max number of boot modules a bootloader can provide in addition to Xen > >> */ > >> #define MAX_NR_BOOTMODS 63 > >> @@ -34,7 +33,7 @@ struct boot_info { > >> > >> unsigned int nr_modules; > >> struct bootmodule mods[MAX_NR_BOOTMODS + 1]; > >> - struct boot_domain domains[MAX_NR_BOOTDOMS]; > >> + struct bootdomain domains[MAX_NR_BOOTDOMS]; > >> }; > >> > >> /* > >> diff --git a/xen/arch/x86/include/asm/dom0_build.h > >> b/xen/arch/x86/include/asm/dom0_build.h > >> index ff021c24af..df03189870 100644 > >> --- a/xen/arch/x86/include/asm/dom0_build.h > >> +++ b/xen/arch/x86/include/asm/dom0_build.h > >> @@ -13,9 +13,9 @@ unsigned long dom0_compute_nr_pages(struct domain *d, > >> unsigned long initrd_len); > >> int dom0_setup_permissions(struct domain *d); > >> > >> -struct boot_domain; > >> -int dom0_construct_pv(const struct boot_domain *bd); > >> -int dom0_construct_pvh(const struct boot_domain *bd); > >> +struct bootdomain; > >> +int dom0_construct_pv(const struct bootdomain *bd); > >> +int dom0_construct_pvh(const struct bootdomain *bd); > >> > >> unsigned long dom0_paging_pages(const struct domain *d, > >> unsigned long nr_pages); > >> diff --git a/xen/arch/x86/include/asm/setup.h > >> b/xen/arch/x86/include/asm/setup.h > >> index c7deaba109..a8647f0fdf 100644 > >> --- a/xen/arch/x86/include/asm/setup.h > >> +++ b/xen/arch/x86/include/asm/setup.h > >> @@ -26,8 +26,8 @@ void subarch_init_memory(void); > >> > >> void init_IRQ(void); > >> > >> -struct boot_domain; > >> -int construct_dom0(const struct boot_domain *bd); > >> +struct bootdomain; > >> +int construct_dom0(const struct bootdomain *bd); > >> > >> void setup_io_bitmap(struct domain *d); > >> > >> diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c > >> index e6c77413f5..a6f212fe0a 100644 > >> --- a/xen/arch/x86/pv/dom0_build.c > >> +++ b/xen/arch/x86/pv/dom0_build.c > >> @@ -355,7 +355,7 @@ static struct page_info * __init alloc_chunk(struct > >> domain *d, > >> return page; > >> } > >> > >> -static int __init dom0_construct(const struct boot_domain *bd) > >> +static int __init dom0_construct(const struct bootdomain *bd) > >> { > >> unsigned int i; > >> int rc, order, machine; > >> @@ -375,7 +375,7 @@ static int __init dom0_construct(const struct > >> boot_domain *bd) > >> struct vcpu *v = d->vcpu[0]; > >> > >> struct bootmodule *image = bd->kernel; > >> - struct bootmodule *initrd = bd->module; > >> + struct bootmodule *initrd = bd->initrd; > >> void *image_base; > >> unsigned long image_len; > >> void *image_start; > >> @@ -1070,7 +1070,7 @@ out: > >> return rc; > >> } > >> > >> -int __init dom0_construct_pv(const struct boot_domain *bd) > >> +int __init dom0_construct_pv(const struct bootdomain *bd) > >> { > >> unsigned long cr4 = read_cr4(); > >> int rc; > >> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > >> index e9a70c2c2b..726adad0e5 100644 > >> --- a/xen/arch/x86/setup.c > >> +++ b/xen/arch/x86/setup.c > >> @@ -297,7 +297,9 @@ static const char *cmdline_cook(const char *p, const > >> char *loader_name); > >> struct boot_info __initdata xen_boot_info = { > >> .loader = "unknown", > >> .cmdline = "", > >> - .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = { .domid = DOMID_INVALID } > >> }, > >> + .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = { > >> + .arch.domid = DOMID_INVALID > >> + }}, > >> /* > >> * There's a MAX_NR_BOOTMODS-th entry in the array. It's not off by > >> one. > >> * > >> @@ -987,7 +989,7 @@ static unsigned int __init copy_bios_e820(struct > >> e820entry *map, unsigned int li > >> } > >> > >> static size_t __init domain_cmdline_size(const struct boot_info *bi, > >> - const struct boot_domain *bd) > >> + const struct bootdomain *bd) > >> { > >> size_t s = bi->kextra ? strlen(bi->kextra) : 0; > >> const struct arch_bootmodule *abm = &bd->kernel->arch; > >> @@ -1022,7 +1024,7 @@ static struct domain *__init create_dom0(struct > >> boot_info *bi) > >> .misc_flags = opt_dom0_msr_relaxed ? XEN_X86_MSR_RELAXED : 0, > >> }, > >> }; > >> - struct boot_domain *bd = &bi->domains[0]; > >> + struct bootdomain *bd = &bi->domains[0]; > >> struct domain *d; > >> > >> if ( opt_dom0_pvh ) > >> @@ -1039,11 +1041,11 @@ static struct domain *__init create_dom0(struct > >> boot_info *bi) > >> dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu; > >> > >> /* Create initial domain. Not d0 for pvshim. */ > >> - bd->domid = get_initial_domain_id(); > >> - d = domain_create(bd->domid, &dom0_cfg, > >> + bd->arch.domid = get_initial_domain_id(); > >> + d = domain_create(bd->arch.domid, &dom0_cfg, > >> pv_shim ? 0 : CDF_privileged | CDF_hardware); > >> if ( IS_ERR(d) ) > >> - panic("Error creating d%u: %ld\n", bd->domid, PTR_ERR(d)); > >> + panic("Error creating d%u: %ld\n", bd->arch.domid, PTR_ERR(d)); > >> > >> init_dom0_cpuid_policy(d); > >> > >> @@ -2162,7 +2164,7 @@ void asmlinkage __init noreturn __start_xen(void) > >> if ( initrdidx < MAX_NR_BOOTMODS ) > >> { > >> bi->mods[initrdidx].kind = BOOTMOD_RAMDISK; > >> - bi->domains[0].module = &bi->mods[initrdidx]; > >> + bi->domains[0].initrd = &bi->mods[initrdidx]; > >> if ( first_boot_module_index(bi, BOOTMOD_UNKNOWN) < > >> MAX_NR_BOOTMODS ) > >> printk(XENLOG_WARNING > >> "Multiple initrd candidates, picking module #%u\n", > >> diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h > >> index e6d52a599f..19d2ff0f0c 100644 > >> --- a/xen/include/xen/bootfdt.h > >> +++ b/xen/include/xen/bootfdt.h > >> @@ -108,6 +108,10 @@ struct bootdomain { > >> struct bootmodule *initrd; > >> > >> const char* cmdline; > >> + > >> +#if __has_include(<asm/bootfdt.h>) > >> + struct arch_bootdomain arch; > >> +#endif > >> }; > > > > One suggestion for improvement: > > That would function today, but that field is important later on when we > retrieve the domid from the DTB. That's where it's stored until we create > them. Oh OK. In that case please add a note to the commit message > > diff --git a/xen/arch/x86/include/asm/bootfdt.h > > b/xen/arch/x86/include/asm/bootfdt.h > > index b30132381e..892a0e2373 100644 > > --- a/xen/arch/x86/include/asm/bootfdt.h > > +++ b/xen/arch/x86/include/asm/bootfdt.h > > @@ -6,11 +6,6 @@ > > > > #include <public/xen.h> > > > > -struct arch_bootdomain > > -{ > > - domid_t domid; > > -}; > > - > > struct arch_bootmodule > > { > > /* > > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > > index 726adad0e5..39f294b88b 100644 > > --- a/xen/arch/x86/setup.c > > +++ b/xen/arch/x86/setup.c > > @@ -297,9 +297,6 @@ static const char *cmdline_cook(const char *p, const > > char *loader_name); > > struct boot_info __initdata xen_boot_info = { > > .loader = "unknown", > > .cmdline = "", > > - .domains = { [0 ... MAX_NR_BOOTDOMS - 1] = { > > - .arch.domid = DOMID_INVALID > > - }}, > > /* > > * There's a MAX_NR_BOOTMODS-th entry in the array. It's not off by > > one. > > * > > @@ -1026,6 +1023,7 @@ static struct domain *__init create_dom0(struct > > boot_info *bi) > > }; > > struct bootdomain *bd = &bi->domains[0]; > > struct domain *d; > > + domid_t domid; > > > > if ( opt_dom0_pvh ) > > { > > @@ -1041,11 +1039,11 @@ static struct domain *__init create_dom0(struct > > boot_info *bi) > > dom0_cfg.flags |= XEN_DOMCTL_CDF_iommu; > > > > /* Create initial domain. Not d0 for pvshim. */ > > - bd->arch.domid = get_initial_domain_id(); > > - d = domain_create(bd->arch.domid, &dom0_cfg, > > + domid = get_initial_domain_id(); > > + d = domain_create(domid, &dom0_cfg, > > pv_shim ? 0 : CDF_privileged | CDF_hardware); > > if ( IS_ERR(d) ) > > - panic("Error creating d%u: %ld\n", bd->arch.domid, PTR_ERR(d)); > > + panic("Error creating d%u: %ld\n", domid, PTR_ERR(d)); > > > > init_dom0_cpuid_policy(d); > > > > diff --git a/xen/include/xen/bootfdt.h b/xen/include/xen/bootfdt.h > > index 19d2ff0f0c..e6d52a599f 100644 > > --- a/xen/include/xen/bootfdt.h > > +++ b/xen/include/xen/bootfdt.h > > @@ -108,10 +108,6 @@ struct bootdomain { > > struct bootmodule *initrd; > > > > const char* cmdline; > > - > > -#if __has_include(<asm/bootfdt.h>) > > - struct arch_bootdomain arch; > > -#endif > > }; > > > > /* > > Cheers, > Alejandro >

©2013 Xen Project, A Linux Foundation Collaborative Project. All Rights Reserved.
Linux Foundation is a registered trademark of The Linux Foundation.
Xen Project is a trademark of The Linux Foundation.