Look for a subnode of type `multiboot,ramdisk` within a domain node. If
found, process the reg property for the MB1 module index.

Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
---
Changes since v1:
- switch to nested else/if
- dropped ternary name selection
---
 xen/arch/x86/domain-builder/fdt.c | 26 +++++++++++++++++++++++
 xen/arch/x86/setup.c              | 35 +++++++++++++++++--------------
 2 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/xen/arch/x86/domain-builder/fdt.c b/xen/arch/x86/domain-builder/fdt.c
index 1094c8dc8838..27bc37ad45c9 100644
--- 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 )
+        {
+            int idx = dom0less_module_node(fdt, node, size_size, address_size);
+            if ( idx < 0 )
+            {
+                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;
+            }
+
+            printk("  ramdisk: boot module %d\n", idx);
+            bi->mods[idx].type = BOOTMOD_RAMDISK;
+            bd->ramdisk = &bi->mods[idx];
+
+            continue;
         }
     }
 
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 3dd04b9afca7..25ff029ecdda 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1103,7 +1103,7 @@ void asmlinkage __init noreturn __start_xen(void)
     char *kextra;
     void *bsp_stack;
     struct cpu_info *info = get_cpu_info(), *bsp_info;
-    unsigned int initrdidx, num_parked = 0;
+    unsigned int num_parked = 0;
     struct boot_info *bi;
     unsigned long nr_pages, raw_max_page;
     int i, j, bytes = 0;
@@ -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 )
     {
-        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);
+        }
     }
 
     /*
-- 
2.30.2

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

On 2024-12-26 11: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.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com>
> ---
> Changes since v1:
> - switch to nested else/if
> - dropped ternary name selection
> ---
>   xen/arch/x86/domain-builder/fdt.c | 26 +++++++++++++++++++++++
>   xen/arch/x86/setup.c              | 35 +++++++++++++++++--------------
>   2 files changed, 45 insertions(+), 16 deletions(-)
> 
> diff --git a/xen/arch/x86/domain-builder/fdt.c b/xen/arch/x86/domain-builder/fdt.c
> index 1094c8dc8838..27bc37ad45c9 100644
> --- 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 )
> +        {
> +            int idx = dom0less_module_node(fdt, node, size_size, address_size);

Your next patch has the hl_module_index() parsing you want moved into 
this patch.

Regards,
Jason