Re: [PATCH 00/10] v3: Boot modules for Hyperlaunch

[Jan Beulich <jbeulich@xxxxxxxx>]

On 01.07.2023 09:18, Christopher Clark wrote:
> This is a v3 series of work for Hyperlaunch for the Xen hypervisor,
> an update to v2 and implementing a subset of the v1 series and functionality,
> with changes made to address the community feedback provided on the patches.
> Changes since the earlier versions of the series are described below.
> 
> The patches in this series are primarily derived from patches 2-4 of the
> v1 full series, in a series of smaller patches for ease of review.
> 
> Thanks to AMD for supporting this work.
> 
> Documentation on Hyperlaunch:
> https://wiki.xenproject.org/wiki/Hyperlaunch
> 
> v1 Hyperlaunch patch series:
> https://lists.xenproject.org/archives/html/xen-devel/2022-07/msg00345.html
> 
> thanks
> 
> Christopher
> 
> 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.
> 
> Christopher Clark (10):
>   x86 setup: move x86 boot module counting into a new boot_info struct
>   x86 setup: per-arch bootmodule structure, headroom field
>   x86 setup: change bootstrap map to accept new boot module structures
>   x86 setup: porting dom0 construction logic to boot module structures
>   xsm: switch XSM init to boot info structures
>   x86 setup, microcode: switch to the new bootinfo structures

While up to here things are an integral part of your hyperlaunch work, ...

>   x86 boot: define paddr_t and add macros for typedefing struct pointers
>   x86, arm, riscv: add per-arch bootinfo headers
>   arm setup: use common integer-typed bootmod definition
>   x86 setup: refactor efi, pvh and multiboot entrypoints to new boot
>     info

... I'm getting the impression that the rest is unrelated tidying. I
wonder if we really want to take that churn right now; besides this
needlessly bloating the series and pulling away attention from the
functionally important parts, I could also see there being a need for
changes later in the course of your hyperlaunch work, which might then
have a knock-on effect on what's being carried through here to very
early boot code. Early boot code, tending to be fragile, would perhaps
best not be altered more than necessary. I wonder what the other x86
maintainers think ...

Jan