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

[Alejandro Vallejo <agarciav@amd.com>]

On Wed Apr 9, 2025 at 11:07 PM BST, Denis Mukhin wrote:
> On Tuesday, April 8th, 2025 at 9:07 AM, Alejandro Vallejo <agarciav@amd.com> wrote:
>
>> 
>> 
>> From: "Daniel P. Smith" dpsmith@apertussolutions.com
>> 
>> 
>> Look for a subnode of type `multiboot,ramdisk` within a domain node and
>> parse via the fdt_read_multiboot_module() helper. After a successful
>> helper call, the module index is returned and the module is guaranteed
>> to be in the module list.
>> 
>> Fix unused typo in adjacent comment.
>> 
>> Signed-off-by: Daniel P. Smith dpsmith@apertussolutions.com
>> 
>> Signed-off-by: Jason Andryuk jason.andryuk@amd.com
>> 
>> Signed-off-by: Alejandro Vallejo agarciav@amd.com
>> 
>> ---
>> v3:
>> * Reworded commit message to state the helper postconditions.
>> * Wrapped long line
>> * Fix ramdisk -> module rename
>> 
>> * Move ramdisk parsing from later patch
>> * Remove initrdidx indent
>> ---
>> xen/arch/x86/domain-builder/fdt.c | 29 +++++++++++++++++++++++++++++
>> xen/arch/x86/setup.c | 4 ++--
>> 2 files changed, 31 insertions(+), 2 deletions(-)
>> 
>> diff --git a/xen/arch/x86/domain-builder/fdt.c b/xen/arch/x86/domain-builder/fdt.c
>> index bc9903a9de..0f5fd01557 100644
>> --- 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;
>> + }
>> + else if ( fdt_node_check_compatible(fdt, node,
>> + "multiboot,ramdisk") == 0 )
>> + {
>> + int idx;
>> +
>> + if ( bd->module )
>> 
>> + {
>> + printk(XENLOG_ERR "Duplicate ramdisk module for domain %s)\n",
>
> I would start the message with lower case so it is consistent with the other one.

As mentioned before, this is due to how it's meant to be rendered. This
is a standalone message, hence the uppercase (consistent with the
duplicate kernel).

Will change the XENLOG_ERR into XENLOG_WARNING though.

>
>> + 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);
>
> Prepend the log message with XENLOG_ERR ?

Indeed.

>
>> + return -EINVAL;
>> + }
>> +
>> + printk(" ramdisk: boot module %d\n", idx);
>> + bi->mods[idx].type = BOOTMOD_RAMDISK;
>> 
>> + bd->module = &bi->mods[idx];
>> 
>> +
>> + continue;
>> }
>> }
>> 
>> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
>> index ca4415d020..3dfa81b48c 100644
>> --- a/xen/arch/x86/setup.c
>> +++ b/xen/arch/x86/setup.c
>> @@ -2149,11 +2149,11 @@ void asmlinkage __init noreturn __start_xen(void)
>> * At this point all capabilities that consume boot modules should have
>> * claimed their boot modules. Find the first unclaimed boot module and
>> * claim it as the initrd ramdisk. Do a second search to see if there are
>> - * any remaining unclaimed boot modules, and report them as unusued initrd
>> + * any remaining unclaimed boot modules, and report them as unused initrd
>> * candidates.
>> */
>> initrdidx = first_boot_module_index(bi, BOOTMOD_UNKNOWN);
>> - if ( initrdidx < MAX_NR_BOOTMODS )
>> + if ( !bi->hyperlaunch_enabled && initrdidx < MAX_NR_BOOTMODS )
>> 
>> {
>> bi->mods[initrdidx].type = BOOTMOD_RAMDISK;
>> 
>> bi->domains[0].module = &bi->mods[initrdidx];
>> 
>> --
>> 2.43.0

Cheers,
Alejandro