| Message ID | 20241006214956.24339-1-dpsmith@apertussolutions.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Boot modules for Hyperlaunch | expand |
On 2024-10-06 17:49, Daniel P. Smith wrote: > The Boot Modules for Hyperlaunch series is an effort to split out preliminary > changes necessary for the introduction of the Hyperlaunch domain builder > logic. These preliminary changes revolve around introducing the struct > boot_module and struct boot_domain structures. This includes converting the > dom0 construction path to use these structures. These abstractions lay the > groundwork to transform and extend the dom0 construction logic into a limited, > but general domain builder. > > The splitting of Hyperlaunch into a set of series are twofold, to reduce the > effort in reviewing a much larger series, and to reduce the effort in handling > the knock-on effects to the construction logic from requested review changes. > > Much thanks to AMD for supporting this work. > > Documentation on Hyperlaunch: > https://wiki.xenproject.org/wiki/Hyperlaunch > > Original Hyperlaunch v1 patch series: > https://lists.xenproject.org/archives/html/xen-devel/2022-07/msg00345.html There is a lot of re-formatting of function arguments like: -static int __init pvh_load_kernel(struct domain *d, const module_t *image, - unsigned long image_headroom, - module_t *initrd, void *image_base, - const char *cmdline, paddr_t *entry, - paddr_t *start_info_addr) +static int __init pvh_load_kernel( + struct domain *d, const struct boot_module *image, + struct boot_module *initrd, void *image_base, + const char *cmdline, paddr_t *entry, paddr_t *start_info_addr) I feel like the old style is more common and I prefer it. But I also don't see it specified in CODING_STYLE. As I am not a maintainer, I'd like them to weigh in. Also, it is nicer to include a per-patch change log instead of just a cover-letter one. That will be useful in subsequent review rounds to clearly identified changed patches. Thanks, Jason
On 08/10/2024 9:07 pm, Jason Andryuk wrote: > On 2024-10-06 17:49, Daniel P. Smith wrote: >> The Boot Modules for Hyperlaunch series is an effort to split out >> preliminary >> changes necessary for the introduction of the Hyperlaunch domain builder >> logic. These preliminary changes revolve around introducing the struct >> boot_module and struct boot_domain structures. This includes >> converting the >> dom0 construction path to use these structures. These abstractions >> lay the >> groundwork to transform and extend the dom0 construction logic into a >> limited, >> but general domain builder. >> >> The splitting of Hyperlaunch into a set of series are twofold, to >> reduce the >> effort in reviewing a much larger series, and to reduce the effort in >> handling >> the knock-on effects to the construction logic from requested review >> changes. >> >> Much thanks to AMD for supporting this work. >> >> Documentation on Hyperlaunch: >> https://wiki.xenproject.org/wiki/Hyperlaunch >> >> Original Hyperlaunch v1 patch series: >> https://lists.xenproject.org/archives/html/xen-devel/2022-07/msg00345.html >> > > There is a lot of re-formatting of function arguments like: > > -static int __init pvh_load_kernel(struct domain *d, const module_t > *image, > - unsigned long image_headroom, > - module_t *initrd, void *image_base, > - const char *cmdline, paddr_t *entry, > - paddr_t *start_info_addr) > +static int __init pvh_load_kernel( > + struct domain *d, const struct boot_module *image, > + struct boot_module *initrd, void *image_base, > + const char *cmdline, paddr_t *entry, paddr_t *start_info_addr) > > I feel like the old style is more common and I prefer it. But I also > don't see it specified in CODING_STYLE. As I am not a maintainer, I'd > like them to weigh in. I already did. :) This isn't a terribly bad example, but there are others which are much worse. Given a choice between an intractable mess of parameters squeezed onto the RHS, and the same mess spread out across the whole width, prefer the latter. ~Andrew
The Boot Modules for Hyperlaunch series is an effort to split out preliminary changes necessary for the introduction of the Hyperlaunch domain builder logic. These preliminary changes revolve around introducing the struct boot_module and struct boot_domain structures. This includes converting the dom0 construction path to use these structures. These abstractions lay the groundwork to transform and extend the dom0 construction logic into a limited, but general domain builder. The splitting of Hyperlaunch into a set of series are twofold, to reduce the effort in reviewing a much larger series, and to reduce the effort in handling the knock-on effects to the construction logic from requested review changes. Much thanks to AMD for supporting this work. Documentation on Hyperlaunch: https://wiki.xenproject.org/wiki/Hyperlaunch Original Hyperlaunch v1 patch series: https://lists.xenproject.org/archives/html/xen-devel/2022-07/msg00345.html V/r, Daniel P. Smith Changes since v4: - added requested inline code comments - moved instance of struct boot_info to unit level and extern'ed - array of struct boot_module moved into struct boot_info - renamed function to multiboot_fill_bootinfo, now returns *struct boot_info - multiboot_fill_bootinfo changed to take multiboot_info_t addr as param - added missing guard that checked there were multiboot1/2 modules passed - renmaed struct elements per the review - fixed errant commit messages per the review - corrected coding style per review - attempted to repalce all open codings of page/addr translations touched - unified use of `bi` as var name for pointer ref to struct boot_info - when appropriate, ensure variables where typed, eg size_t, paddr_t, etc. - dropped all uses of "a = b = c" Changes since v3: - reduced scope to x86 only - broke changes into a smaller chunks with a linear progression - concerns about deconflicting with Arm deferred - conversion from mb1 to boot modules no longer attempted at entry points - the temporary conversion function is now the permenant means to convert - incorporated suggestion from Andy Cooper for handling bootstrap_map Changes since v2: - combined v2 patches 7 and 8 for common review - rebased the v2 series onto the current tip of staging (sorry) - fixed the placement of the patch changelogs - provided the changes description in the cover letter Changes since v1: - the v2 and v3 series implement functionality from v1 patches 2-4 - v2 series objective is to enable efficient patch review in support of merging the functionality into the hypervisor. It implements a subset of the v1 series, incorporating changes from community feedback. - the bootstrap map is made accessible early in the v2 series via both multiboot and boot module arguments until later in the series where multiboot use is retired. This allows for incremental conversion across several patches from multiboot to boot modules. - the 32-bit x86 boot environment header is removed, and changes are made to allow the new common bootinfo headers to be used instead. - Arm and RISC-V architecture bootinfo headers are added to ensure that builds on those architectures can complete correctly. - The KConfig patch to set the maximum number of boot modules allowed is not included in this series, replaced with a static maximum define. Andrew Cooper (1): x86/boot: split bootstrap_map_addr() out of bootstrap_map() Christopher Clark (1): x86/boot: move x86 boot module counting into a new boot_info struct Daniel P. Smith (42): x86/boot: move boot loader name to boot info x86/boot: move cmdline to boot info x86/boot: move mmap info to boot info x86/boot: introduce struct boot_module x86/boot: convert consider_modules to struct boot_module x86/boot: move headroom to boot modules x86/boot: convert setup.c mod refs to early_mod x86/boot: introduce boot module types x86/boot: introduce boot module flags x86/boot: add start and size fields to struct boot_module x86/boot: update struct boot_module on module relocation x86/boot: transition relocation calculations to struct boot_module x86/boot: introduce boot module interator x86/boot: introduce consumed flag for struct boot_module x86/boot: convert microcode loading to consume struct boot_info x86/boot: convert late microcode loading to struct boot_module x86/boot: use consumed boot module flag for microcode x86/boot: convert xsm policy loading to struct boot_module x86/boot: convert ramdisk locating to struct boot_module x86/boot: remove module_map usage from microcode loading x86/boot: remove module_map usage from xsm policy loading x86/boot: remove module_map usage by ramdisk loading x86/boot: convert create_dom0 to use boot info x86/boot: convert construct_dom0 to use struct boot_module x86/boot: relocate kextra into boot info x86/boot: add cmdline to struct boot_module x86/boot: convert dom0_construct_pv image param to struct boot_module x86/boot: convert dom0_construct_pv initrd param to struct boot_module x86/boot: convert dom0_construct_pvh to struct boot_module x86/boot: convert pvh_load_kernel to struct boot_module x86/boot: convert initial_images to struct boot_module x86/boot: drop the use of initial_images unit global x86/boot: remove usage of mod_end by discard_initial_images x86/boot: remove remaining early_mod references x86/boot: remove mod from struct boot_module x86/boot: introduce boot domain x86/boot: introduce domid field to struct boot_domain x86/boot: add cmdline to struct boot_domain x86/boot: add struct domain to struct boot_domain x86/boot: convert construct_dom0 to struct boot_domain x86/boot: convert dom0_construct_pv to struct boot_domain x86/boot: convert dom0_construct_pvh to struct boot_domain xen/arch/x86/cpu/microcode/core.c | 78 +++---- xen/arch/x86/dom0_build.c | 21 +- xen/arch/x86/hvm/dom0_build.c | 55 +++-- xen/arch/x86/include/asm/bootdomain.h | 37 +++ xen/arch/x86/include/asm/bootinfo.h | 91 ++++++++ xen/arch/x86/include/asm/dom0_build.h | 11 +- xen/arch/x86/include/asm/microcode.h | 12 +- xen/arch/x86/include/asm/setup.h | 11 +- xen/arch/x86/pv/dom0_build.c | 54 ++--- xen/arch/x86/setup.c | 321 ++++++++++++++++---------- xen/include/xsm/xsm.h | 14 +- xen/xsm/xsm_core.c | 15 +- xen/xsm/xsm_policy.c | 18 +- 13 files changed, 461 insertions(+), 277 deletions(-) create mode 100644 xen/arch/x86/include/asm/bootdomain.h create mode 100644 xen/arch/x86/include/asm/bootinfo.h