Re: [PATCH v5 05/12] x86/hyperlaunch: locate dom0 kernel with hyperlaunch

["Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>]

On 4/24/25 12:10, Alejandro Vallejo wrote:
> From: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
>
> Look for a subnode of type `multiboot,kernel` within a domain node. If
> found, locate it using the multiboot module helper to generically ensure
> it lives in the module list. If the bootargs property is present and
> there was not an MB1 string, then use the command line from the device
> tree definition.
>
> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> Signed-off-by: Alejandro Vallejo <agarciav@xxxxxxx>
> ---
> v5:
>    * Add domain name to the error path on no-kernel.
> ---
>   xen/arch/x86/setup.c            |  1 +
>   xen/common/domain-builder/fdt.c | 64 +++++++++++++++++++++++++++++++--
>   2 files changed, 62 insertions(+), 3 deletions(-)
>
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index bd13d9d196..7e756302ae 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1290,6 +1290,7 @@ void asmlinkage __init noreturn __start_xen(void)
>           i = first_boot_module_index(bi, BOOTMOD_UNKNOWN);
>           bi->mods[i].type = BOOTMOD_KERNEL;
>           bi->domains[0].kernel = &bi->mods[i];
> +        bi->nr_domains = 1;
>           bi->hyperlaunch_enabled = false;
>       }
>   
> diff --git a/xen/common/domain-builder/fdt.c b/xen/common/domain-builder/fdt.c
> index 11f0436e85..662c715483 100644
> --- a/xen/common/domain-builder/fdt.c
> +++ b/xen/common/domain-builder/fdt.c
> @@ -99,9 +99,9 @@ static int __init fdt_prop_as_reg(const struct fdt_property 
> *prop,
>    * @return              -EINVAL on malformed nodes, otherwise
>    *                      index inside `bi->mods`
>    */
> -int __init fdt_read_multiboot_module(const void *fdt, int node,
> -                                     int address_cells, int size_cells,
> -                                     struct boot_info *bi)
> +static int __init fdt_read_multiboot_module(const void *fdt, int node,
> +                                            int address_cells, int size_cells,
> +                                            struct boot_info *bi)
Shouldn't this belong to when the function was introduced? Though 
looking there, you push it out into the header? Guess the real question 
is it static or not?
>   {
>       const struct fdt_property *prop;
>       uint64_t addr, size;
> @@ -185,6 +185,52 @@ int __init fdt_read_multiboot_module(const void *fdt, int 
> node,
>       return idx;
>   }
>   
> +static int __init process_domain_node(
> +    struct boot_info *bi, const void *fdt, int dom_node)
> +{
> +    int node;
> +    struct boot_domain *bd = &bi->domains[bi->nr_domains];
> +    const char *name = fdt_get_name(fdt, dom_node, NULL) ?: "unknown";
> +    int address_cells = fdt_address_cells(fdt, dom_node);
> +    int size_cells = fdt_size_cells(fdt, dom_node);
> +
> +    fdt_for_each_subnode(node, fdt, dom_node)
> +    {
> +        if ( !fdt_node_check_compatible(fdt, node, "multiboot,kernel") )
> +        {
> +            int idx;
> +
> +            if ( bd->kernel )
> +            {
> +                printk(XENLOG_WARNING
> +                       "  duplicate kernel for domain %s\n", name);
> +                continue;
> +            }
> +
> +            idx = fdt_read_multiboot_module(fdt, node, address_cells,
> +                                            size_cells, bi);
> +            if ( idx < 0 )
> +            {
> +                printk(XENLOG_ERR
> +                       "  failed processing kernel for domain %s\n", name);
> +                return idx;
> +            }
> +
> +            printk(XENLOG_INFO "  kernel: multiboot-index=%d\n", idx);
> +            bi->mods[idx].type = BOOTMOD_KERNEL;
> +            bd->kernel = &bi->mods[idx];
> +        }
> +    }
> +
> +    if ( !bd->kernel )
> +    {
> +        printk(XENLOG_ERR "error: no kernel assigned to domain %s\n", name);
> +        return -ENODATA;
> +    }
> +
> +    return 0;
> +}
> +
>   static int __init find_hyperlaunch_node(const void *fdt)
>   {
>       int hv_node = fdt_path_offset(fdt, "/chosen/hypervisor");
> @@ -248,8 +294,20 @@ int __init walk_hyperlaunch_fdt(struct boot_info *bi)
>   
>       fdt_for_each_subnode(node, fdt, hv_node)
>       {
> +        if ( bi->nr_domains >= MAX_NR_BOOTDOMS )
> +        {
> +            printk(XENLOG_WARNING "warning: only creating first %u domains\n",
> +                   MAX_NR_BOOTDOMS);
> +            break;
> +        }
> +
>           if ( !fdt_node_check_compatible(fdt, node, "xen,domain") )
> +        {
> +            if ( (ret = process_domain_node(bi, fdt, node)) < 0 )
> +                break;
> +
>               bi->nr_domains++;
> +        }
>       }
>   
>       /* Until multi-domain construction is added, throw an error */