Re: [PATCH v3 11/16] x86/hyperlaunch: locate dom0 initrd with hyperlaunch

[Jan Beulich <jbeulich@xxxxxxxx>]

On 08.04.2025 18:07, Alejandro Vallejo wrote:
> --- a/xen/arch/x86/domain-builder/fdt.c
> +++ b/xen/arch/x86/domain-builder/fdt.c
> @@ -195,6 +195,35 @@ static int __init process_domain_node(
>                   !((char *)__va(bd->kernel->cmdline_pa))[0] )
>                  bd->kernel->fdt_cmdline = fdt_get_prop_offset(
>                      fdt, node, "bootargs", &bd->kernel->cmdline_pa);
> +
> +            continue;

With this ...

> +        }
> +        else if ( fdt_node_check_compatible(fdt, node,

... no need for "else" here?

> +                                            "multiboot,ramdisk") == 0 )
> +        {
> +            int idx;
> +
> +            if ( bd->module )
> +            {
> +                printk(XENLOG_ERR "Duplicate ramdisk module for domain 
> %s)\n",

Stray ')' in the string literal.

> +                       name);
> +                continue;
> +            }
> +
> +            idx = fdt_read_multiboot_module(fdt, node, address_cells,
> +                                            size_cells,bi);
> +            if ( idx < 0 )
> +            {
> +                printk("  failed processing ramdisk module for domain %s\n",
> +                       name);
> +                return -EINVAL;
> +            }

Along the lines of what Denis has said - please be consistent about log
messages: XENLOG_* or not, preferably no capital at the start, initial
blank padding. May apply elsewhere in the series as well.

> +            printk("  ramdisk: boot module %d\n", idx);
> +            bi->mods[idx].type = BOOTMOD_RAMDISK;
> +            bd->module = &bi->mods[idx];

The field's named "module" now, but that now ends up inconsistent with
naming used elsewhere, as is pretty noticeable here.

> +            continue;

This isn't strictly needed, is it, ...

>          }
>      }

... considering we're at the bottom of the loop?

Jan