Re: [Xen-devel] [PATCH 7/8] x86/setup: simplify handling of initrdidx when no initrd present

[Julien Grall <julien@xen.org>]

Hi David,

On 01/02/2020 01:33, David Woodhouse wrote:
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> Remove a ternary operator that made my brain hurt.
> 
> Replace it with something simpler that makes it somewhat clearer that
> the check for initrdidx < mbi->mods_count is because mbi->mods_count
> is what find_first_bit() will return when it doesn't find anything.
> 
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>   xen/arch/x86/setup.c | 9 +++++----
>   1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index 5f68a1308f..10209e6bfb 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -687,7 +687,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>       char *cmdline, *kextra, *loader;
>       unsigned int initrdidx, num_parked = 0;
>       multiboot_info_t *mbi;
> -    module_t *mod;
> +    module_t *mod, *initrd = NULL;
>       unsigned long nr_pages, raw_max_page, modules_headroom, module_map[1];
>       int i, j, e820_warn = 0, bytes = 0;
>       bool acpi_boot_table_init_done = false, relocated = false;
> @@ -1793,6 +1793,9 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>           xen_processor_pmbits |= XEN_PROCESSOR_PM_CX;
>   
>       initrdidx = find_first_bit(module_map, mbi->mods_count);
> +    if ( initrdidx < mbi->mods_count )
IIUC, the check on initrdx > 0 was pointless as bit 0 will always be 
cleared (it is used for dom0 kernel). It might be worth mentionning it 
in the commit message. Something like:

"The initrd can never be in the first module (i.e initrdidx == 0) as 
this is always used by the dom0 kernel. So the check can be simplify."

Acked-by: Julien Grall <julien@xen.org>

> +        initrd = mod + initrdidx;
> +
>       if ( bitmap_weight(module_map, mbi->mods_count) > 1 )
>           printk(XENLOG_WARNING
>                  "Multiple initrd candidates, picking module #%u\n",
> @@ -1817,9 +1820,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
>        * We're going to setup domain0 using the module(s) that we stashed safely
>        * above our heap. The second module, if present, is an initrd ramdisk.
>        */
> -    if ( construct_dom0(dom0, mod, modules_headroom,
> -                        (initrdidx > 0) && (initrdidx < mbi->mods_count)
> -                        ? mod + initrdidx : NULL, cmdline) != 0)
> +    if ( construct_dom0(dom0, mod, modules_headroom, initrd, cmdline) != 0 )
>           panic("Could not set up DOM0 guest OS\n");
>   
>       if ( cpu_has_smap )
> 

Cheers,

-- 
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel