On 26.12.2024 17:57, Daniel P. Smith wrote:
> Look for a subnode of type `multiboot,ramdisk` within a domain node. If
> found, process the reg property for the MB1 module index.
Unlike for cmdline it doesn't look to be mix-and-match here.
> --- a/xen/arch/x86/domain-builder/fdt.c
> +++ b/xen/arch/x86/domain-builder/fdt.c
> @@ -119,6 +119,32 @@ static int __init process_domain_node(
> if ( ret > 0 )
> bd->kernel->fdt_cmdline = true;
> }
> +
> + continue;
> + }
> + else if (
> + fdt_node_check_compatible(fdt, node, "multiboot,ramdisk") == 0 )
I'm sorry, but this isn't style we use. Perhaps
else if ( fdt_node_check_compatible(
fdt, node, "multiboot,ramdisk") == 0 )
if you dislike
else if ( fdt_node_check_compatible(fdt, node,
"multiboot,ramdisk") == 0 )
> + {
> + int idx = dom0less_module_node(fdt, node, size_size,
> address_size);
> + if ( idx < 0 )
Nit: Blank line between declaration(s) and statement(s) please. (Again
at least once elsewhere in this patch.)
> + {
> + printk(" failed processing ramdisk module for domain %s\n",
> + name);
> + return -EINVAL;
> + }
> +
> + if ( idx > bi->nr_modules )
> + {
> + printk(" invalid ramdisk module index for domain node
> (%d)\n",
> + bi->nr_domains);
> + return -EINVAL;
> + }
See comments on similar printk()s in an earlier patch.
> @@ -2141,22 +2141,25 @@ void asmlinkage __init noreturn __start_xen(void)
> cpu_has_nx ? XENLOG_INFO : XENLOG_WARNING "Warning: ",
> cpu_has_nx ? "" : "not ");
>
> - /*
> - * 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
> - * candidates.
> - */
> - initrdidx = first_boot_module_index(bi, BOOTMOD_UNKNOWN);
> - if ( initrdidx < MAX_NR_BOOTMODS )
> + if ( !bi->hyperlaunch_enabled )
Can't this be "if ( !bi->hyperlaunch_enabled && initrdidx < MAX_NR_BOOTMODS )"
and then all of the churn here can be avoided? An unnecessary call to
first_boot_module_index() is unlikely to be the end of the world. Otherwise ...
> {
> - bi->mods[initrdidx].type = BOOTMOD_RAMDISK;
> - bi->domains[0].ramdisk = &bi->mods[initrdidx];
> - if ( first_boot_module_index(bi, BOOTMOD_UNKNOWN) < MAX_NR_BOOTMODS )
> - printk(XENLOG_WARNING
> - "Multiple initrd candidates, picking module #%u\n",
> - initrdidx);
> + /*
> + * 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
> + * candidates.
> + */
> + unsigned int initrdidx = first_boot_module_index(bi,
> BOOTMOD_UNKNOWN);
> + if ( initrdidx < MAX_NR_BOOTMODS )
> + {
> + bi->mods[initrdidx].type = BOOTMOD_RAMDISK;
> + bi->domains[0].ramdisk = &bi->mods[initrdidx];
> + if ( first_boot_module_index(bi, BOOTMOD_UNKNOWN) <
> MAX_NR_BOOTMODS )
> + printk(XENLOG_WARNING
> + "Multiple initrd candidates, picking module #%u\n",
> + initrdidx);
> + }
... please pay attention to line length when re-indenting. (If you still need
to re-indent, perhaps also s/unusued/unused/ in the comment, while you touch
it.)
Jan
