Re: [PATCH v6 01/44] x86/boot: move x86 boot module counting into a new boot_info struct

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 17/10/2024 6:02 pm, Daniel P. Smith wrote:
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index a6e77c9ed9fc..6201ca0fad19 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -274,16 +275,28 @@ static int __init cf_check parse_acpi_param(const char 
> *s)
>  custom_param("acpi", parse_acpi_param);
>  
>  static const module_t *__initdata initial_images;
> -static unsigned int __initdata nr_initial_images;
> +
> +struct boot_info __initdata xen_boot_info;
> +
> +static struct boot_info *__init multiboot_fill_boot_info(unsigned long mbi_p)
> +{
> +    struct boot_info *bi = &xen_boot_info;
> +    const multiboot_info_t *mbi = __va(mbi_p);
> +
> +    bi->nr_modules = (mbi->flags & MBI_MODULES) ? mbi->mods_count : 0;

Having looked to end of the series, this and a few other expressions are
still awkward.

The nicer way looks like:

struct boot_info __initdata xen_boot_info = {
    .loader = "unknown",
    .cmdline = "",
};

static struct boot_info *__init multiboot_fill_boot_info(unsigned long
mbi_p)
{
    struct boot_info *bi = &xen_boot_info;
    const multiboot_info_t *mbi = __va(mbi_p);
    module_t *mods = __va(mbi->mods_addr);
    unsigned int i;

    if ( mbi->flags & MBI_MODULES )
        bi->nr_modules = mbi->mods_count;

    if ( mbi->flags & MBI_LOADERNAME )
        bi->loader = __va(mbi->boot_loader_name);

    if ( mbi->flags & MBI_CMDLINE )
        bi->cmdline = cmdline_cook(__va(mbi->cmdline), bi->loader);

    if ( mbi->flags & MBI_MEMMAP )
    {
        bi->memmap_addr = mbi->mmap_addr;
        bi->memmap_length = mbi->mmap_length;
    }


which is easier to read and follow.  Thoughts?

~Andrew