On Tuesday, April 8th, 2025 at 9:07 AM, Alejandro Vallejo <agarc...@amd.com> wrote:
> > > From: "Daniel P. Smith" dpsm...@apertussolutions.com > > > Introduce the domain builder which is capable of consuming a device tree as > the > first boot module. If it finds a device tree as the first boot module, it will > set its type to BOOTMOD_FDT. This change only detects the boot module and > continues to boot with slight change to the boot convention that the dom0 > kernel is no longer first boot module but is the second. > > No functional change intended. > > Signed-off-by: Daniel P. Smith dpsm...@apertussolutions.com > > Signed-off-by: Jason Andryuk jason.andr...@amd.com > > --- > v3: > * Move obj-y += domain-builder/ > * Remove blank line in Makefile > * const in has_hyperlaunch_fdt() > * CONFIG_LIBFDT rename > * Use boot_info forward declaration > * Rename domainbuilder.h to domain-builder.h > * Add fdt NULL check > --- > xen/arch/x86/Makefile | 1 + > xen/arch/x86/domain-builder/Kconfig | 2 +- > xen/arch/x86/domain-builder/Makefile | 2 + > xen/arch/x86/domain-builder/core.c | 57 +++++++++++++++++++++++ > xen/arch/x86/domain-builder/fdt.c | 37 +++++++++++++++ > xen/arch/x86/domain-builder/fdt.h | 21 +++++++++ I have a general question. Wouldn't that make sense to use arch-independent placeholder for domain builder code right from the starting point? For example something like xen/common/domain-builder ? My understanding is that there's a lot of code in the domain builder which can be potentially shared/re-used with non-x86 architectures. Also, that seems to align with the intention to have common arch-independent subsystems in the code base: https://docs.google.com/presentation/d/1q9cjJZLUxUo1YzMpCxVHuL-ZOGoFaO9haHfRBK4i4Uc/edit?slide=id.g32afc87aef4_0_18#slide=id.g32afc87aef4_0_18 > xen/arch/x86/include/asm/bootinfo.h | 3 ++ > xen/arch/x86/include/asm/domain-builder.h | 8 ++++ > xen/arch/x86/setup.c | 17 ++++--- > 9 files changed, 141 insertions(+), 7 deletions(-) > create mode 100644 xen/arch/x86/domain-builder/Makefile > create mode 100644 xen/arch/x86/domain-builder/core.c > create mode 100644 xen/arch/x86/domain-builder/fdt.c > create mode 100644 xen/arch/x86/domain-builder/fdt.h > create mode 100644 xen/arch/x86/include/asm/domain-builder.h > > diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile > index f59c9665fd..deae31d627 100644 > --- a/xen/arch/x86/Makefile > +++ b/xen/arch/x86/Makefile > @@ -1,6 +1,7 @@ > obj-y += acpi/ > obj-y += boot/ > obj-y += cpu/ > +obj-y += domain-builder/ > obj-y += efi/ > obj-y += genapic/ > obj-$(CONFIG_GUEST) += guest/ > diff --git a/xen/arch/x86/domain-builder/Kconfig > b/xen/arch/x86/domain-builder/Kconfig > index 8ed493c3b5..51d549c20d 100644 > --- a/xen/arch/x86/domain-builder/Kconfig > +++ b/xen/arch/x86/domain-builder/Kconfig > @@ -3,7 +3,7 @@ menu "Domain Builder Features" > > config DOMAIN_BUILDER > bool "Domain builder (UNSUPPORTED)" if UNSUPPORTED > - select LIB_DEVICE_TREE > + select LIBFDT > help > Enables the domain builder capability to configure boot domain > construction using a flattened device tree. > diff --git a/xen/arch/x86/domain-builder/Makefile > b/xen/arch/x86/domain-builder/Makefile > new file mode 100644 > index 0000000000..b10cd56b28 > --- /dev/null > +++ b/xen/arch/x86/domain-builder/Makefile > @@ -0,0 +1,2 @@ > +obj-$(CONFIG_DOMAIN_BUILDER) += fdt.init.o > +obj-y += core.init.o > diff --git a/xen/arch/x86/domain-builder/core.c > b/xen/arch/x86/domain-builder/core.c > new file mode 100644 > index 0000000000..d6ae94f45c > --- /dev/null > +++ b/xen/arch/x86/domain-builder/core.c > @@ -0,0 +1,57 @@ > +/* SPDX-License-Identifier: GPL-2.0-only / > +/ > + * Copyright (C) 2024, Apertus Solutions, LLC > + */ > +#include <xen/err.h> > > +#include <xen/init.h> > > +#include <xen/kconfig.h> > > +#include <xen/lib.h> > > + > +#include <asm/bootinfo.h> > > + > +#include "fdt.h" > + > +void __init builder_init(struct boot_info *bi) > +{ > + if ( IS_ENABLED(CONFIG_DOMAIN_BUILDER) ) > + { > + int ret; > + > + switch ( ret = has_hyperlaunch_fdt(bi) ) > + { > + case 0: > + printk("Hyperlaunch device tree detected\n"); > + bi->hyperlaunch_enabled = true; > > + bi->mods[0].type = BOOTMOD_FDT; > > + break; > + > + case -EINVAL: > + printk("Hyperlaunch device tree was not detected\n"); > + bi->hyperlaunch_enabled = false; > > + break; > + > + case -ENOENT: > + case -ENODATA: > + printk("Device tree found, but not hyperlaunch (%d)\n", ret); > + bi->hyperlaunch_enabled = false; > > + bi->mods[0].type = BOOTMOD_FDT; > > + break; > + > + default: > + printk("Unknown error (%d) occured checking for hyperlaunch device tree\n", > + ret); > + bi->hyperlaunch_enabled = false; > > + break; > + } > + } > +} > + > +/* > + * 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/domain-builder/fdt.c > b/xen/arch/x86/domain-builder/fdt.c > new file mode 100644 > index 0000000000..aaf8c1cc16 > --- /dev/null > +++ b/xen/arch/x86/domain-builder/fdt.c > @@ -0,0 +1,37 @@ > +/ SPDX-License-Identifier: GPL-2.0-only / > +/ > + * Copyright (C) 2024, Apertus Solutions, LLC > + */ > +#include <xen/err.h> > > +#include <xen/init.h> > > +#include <xen/lib.h> > > +#include <xen/libfdt/libfdt.h> > > + > +#include <asm/bootinfo.h> > > +#include <asm/page.h> > > +#include <asm/setup.h> > > + > +#include "fdt.h" > + > +int __init has_hyperlaunch_fdt(const struct boot_info *bi) > +{ > + int ret = 0; > + const void *fdt = bootstrap_map_bm(&bi->mods[HYPERLAUNCH_MODULE_IDX]); > > + > + if ( !fdt || fdt_check_header(fdt) < 0 ) > + ret = -EINVAL; > + > + bootstrap_unmap(); > + > + return ret; > +} > + > +/* > + * 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/domain-builder/fdt.h > b/xen/arch/x86/domain-builder/fdt.h > new file mode 100644 > index 0000000000..8e62cd843e > --- /dev/null > +++ b/xen/arch/x86/domain-builder/fdt.h > @@ -0,0 +1,21 @@ > +/ SPDX-License-Identifier: (GPL-2.0-or-later OR BSD-2-Clause) */ > +#ifndef XEN_X86_FDT_H > +#define XEN_X86_FDT_H > + > +#include <xen/init.h> > > + > +struct boot_info; > + > +/* hyperlaunch fdt is required to be module 0 */ > +#define HYPERLAUNCH_MODULE_IDX 0 > + > +#ifdef CONFIG_DOMAIN_BUILDER > +int has_hyperlaunch_fdt(const struct boot_info *bi); > +#else > +static inline int __init has_hyperlaunch_fdt(const struct boot_info bi) > +{ > + return -EINVAL; > +} > +#endif > + > +#endif / XEN_X86_FDT_H */ > diff --git a/xen/arch/x86/include/asm/bootinfo.h > b/xen/arch/x86/include/asm/bootinfo.h > index 3afc214c17..82c2650fcf 100644 > --- a/xen/arch/x86/include/asm/bootinfo.h > +++ b/xen/arch/x86/include/asm/bootinfo.h > @@ -27,6 +27,7 @@ enum bootmod_type { > BOOTMOD_RAMDISK, > BOOTMOD_MICROCODE, > BOOTMOD_XSM_POLICY, > + BOOTMOD_FDT, > }; > > struct boot_module { > @@ -80,6 +81,8 @@ struct boot_info { > paddr_t memmap_addr; > size_t memmap_length; > > + bool hyperlaunch_enabled; > + > unsigned int nr_modules; > struct boot_module mods[MAX_NR_BOOTMODS + 1]; > struct boot_domain domains[MAX_NR_BOOTDOMS]; > diff --git a/xen/arch/x86/include/asm/domain-builder.h > b/xen/arch/x86/include/asm/domain-builder.h > new file mode 100644 > index 0000000000..b6d9ba94de > --- /dev/null > +++ b/xen/arch/x86/include/asm/domain-builder.h > @@ -0,0 +1,8 @@ > +#ifndef XEN_X86_DOMBUILDER_H > +#define XEN_X86_DOMBUILDER_H > + > +struct boot_info; > + > +void builder_init(struct boot_info *bi); > + > +#endif > diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c > index 83cb66e309..e5d78bcb48 100644 > --- a/xen/arch/x86/setup.c > +++ b/xen/arch/x86/setup.c > @@ -36,6 +36,7 @@ > #include <asm/bzimage.h> > > #include <asm/cpu-policy.h> > > #include <asm/desc.h> > > +#include <asm/domain-builder.h> > > #include <asm/e820.h> > > #include <asm/edd.h> > > #include <asm/genapic.h> > > @@ -1281,9 +1282,12 @@ void asmlinkage __init noreturn __start_xen(void) > bi->nr_modules); > > } > > - /* Dom0 kernel is always first */ > - bi->mods[0].type = BOOTMOD_KERNEL; > > - bi->domains[0].kernel = &bi->mods[0]; > > + builder_init(bi); > + > + /* Find first unknown boot module to use as Dom0 kernel */ > + i = first_boot_module_index(bi, BOOTMOD_UNKNOWN); > + bi->mods[i].type = BOOTMOD_KERNEL; > > + bi->domains[0].kernel = &bi->mods[i]; > > > if ( pvh_boot ) > { > @@ -1466,8 +1470,9 @@ void asmlinkage __init noreturn __start_xen(void) > xen->size = __2M_rwdata_end - _stext; > > } > > - bi->mods[0].headroom = > > - bzimage_headroom(bootstrap_map_bm(&bi->mods[0]), bi->mods[0].size); > > + bi->domains[0].kernel->headroom = > > + bzimage_headroom(bootstrap_map_bm(bi->domains[0].kernel), > > + bi->domains[0].kernel->size); > > bootstrap_unmap(); > > #ifndef highmem_start > @@ -1591,7 +1596,7 @@ void asmlinkage __init noreturn __start_xen(void) > #endif > } > > - if ( bi->mods[0].headroom && !bi->mods[0].relocated ) > > + if ( bi->domains[0].kernel->headroom && !bi->domains[0].kernel->relocated ) > > panic("Not enough memory to relocate the dom0 kernel image\n"); > for ( i = 0; i < bi->nr_modules; ++i ) > > { > -- > 2.43.0
