Re: [PATCH v4 04/13] x86/hyperlaunch: initial support for hyperlaunch device tree

[dmkhn@xxxxxxxxx]

On Thu, Apr 17, 2025 at 01:48:26PM +0100, Alejandro Vallejo wrote:
> From: "Daniel P. Smith" <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> 
> Add the ability to detect both a formal hyperlaunch device tree or a dom0less
> device tree. If the hyperlaunch device tree is found, then count the number of
> domain entries, reporting an error if more than one is found.
> 
> Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> Signed-off-by: Alejandro Vallejo <agarciav@xxxxxxx>
> ---
> v4:
>   * Panic if we're booting on hyperlaunch, but walking the DTB fails.
>   * Remove inconsequential "else" clause in fdt.c
>   * Remove stub, as it's not required due to DCE
>   * Use min() rather than open-code it
> ---
>  xen/arch/x86/include/asm/bootinfo.h |  1 +
>  xen/common/domain-builder/core.c    | 11 +++++
>  xen/common/domain-builder/fdt.c     | 63 +++++++++++++++++++++++++++++
>  xen/common/domain-builder/fdt.h     |  1 +
>  4 files changed, 76 insertions(+)
> 
> diff --git a/xen/arch/x86/include/asm/bootinfo.h 
> b/xen/arch/x86/include/asm/bootinfo.h
> index 82c2650fcf..1e3d582e45 100644
> --- a/xen/arch/x86/include/asm/bootinfo.h
> +++ b/xen/arch/x86/include/asm/bootinfo.h
> @@ -84,6 +84,7 @@ struct boot_info {
>      bool hyperlaunch_enabled;
> 
>      unsigned int nr_modules;
> +    unsigned int nr_domains;
>      struct boot_module mods[MAX_NR_BOOTMODS + 1];
>      struct boot_domain domains[MAX_NR_BOOTDOMS];
>  };
> diff --git a/xen/common/domain-builder/core.c 
> b/xen/common/domain-builder/core.c
> index a5b21fc179..3b062e85ec 100644
> --- a/xen/common/domain-builder/core.c
> +++ b/xen/common/domain-builder/core.c
> @@ -43,6 +43,17 @@ void __init builder_init(struct boot_info *bi)
>              break;
>          }
>      }
> +
> +    if ( bi->hyperlaunch_enabled )
> +    {
> +        int ret;
> +
> +        printk(XENLOG_INFO "Hyperlaunch configuration:\n");
> +        if ( (ret = walk_hyperlaunch_fdt(bi)) < 0 )
> +            panic("Walk of device tree failed (%d)\n", ret);
> +
> +        printk(XENLOG_INFO "  number of domains: %u\n", bi->nr_domains);
> +    }
>  }
> 
>  /*
> diff --git a/xen/common/domain-builder/fdt.c b/xen/common/domain-builder/fdt.c
> index aaf8c1cc16..b5ff8220da 100644
> --- a/xen/common/domain-builder/fdt.c
> +++ b/xen/common/domain-builder/fdt.c
> @@ -13,6 +13,36 @@
> 
>  #include "fdt.h"
> 
> +static int __init find_hyperlaunch_node(const void *fdt)
> +{
> +    int hv_node = fdt_path_offset(fdt, "/chosen/hypervisor");
> +
> +    if ( hv_node >= 0 )
> +    {
> +        /* Anything other than zero indicates no match */
> +        if ( fdt_node_check_compatible(fdt, hv_node, "hypervisor,xen") )
> +            return -ENODATA;
> +
> +        return hv_node;
> +    }
> +    else
> +    {
> +        /* Look for dom0less config */
> +        int node, chosen_node = fdt_path_offset(fdt, "/chosen");
> +
> +        if ( chosen_node < 0 )
> +            return -ENOENT;
> +
> +        fdt_for_each_subnode(node, fdt, chosen_node)
> +        {
> +            if ( !fdt_node_check_compatible(fdt, node, "xen,domain") )
> +                return chosen_node;
> +        }
> +    }
> +
> +    return -ENODATA;
> +}
> +
>  int __init has_hyperlaunch_fdt(const struct boot_info *bi)
>  {
>      int ret = 0;
> @@ -20,7 +50,40 @@ int __init has_hyperlaunch_fdt(const struct boot_info *bi)
> 
>      if ( !fdt || fdt_check_header(fdt) < 0 )
>          ret = -EINVAL;
> +    else
> +        ret = find_hyperlaunch_node(fdt);
> +
> +    bootstrap_unmap();
> +
> +    return min(0, ret);
> +}
> +
> +int __init walk_hyperlaunch_fdt(struct boot_info *bi)
> +{
> +    int ret = 0, hv_node, node;
> +    const void *fdt = bootstrap_map_bm(&bi->mods[HYPERLAUNCH_MODULE_IDX]);
> +
> +    if ( unlikely(!fdt) )
> +        return -EINVAL;

I think this check can be converted to ASSERT() since walk_hyperlaunch_fdt()
will be called after has_hyperlaunch_fdt() where condition is checked.

> +
> +    hv_node = find_hyperlaunch_node(fdt);
> +    if ( hv_node < 0 )
> +    {
> +        ret = hv_node;
> +        goto err_out;
> +    }
> +
> +    fdt_for_each_subnode(node, fdt, hv_node)
> +    {
> +        if ( !fdt_node_check_compatible(fdt, node, "xen,domain") )
> +            bi->nr_domains++;
> +    }
> +
> +    /* Until multi-domain construction is added, throw an error */
> +    if ( bi->nr_domains != 1 )
> +        printk(XENLOG_ERR "hyperlaunch only supports dom0 construction\n");
> 
> + err_out:
>      bootstrap_unmap();
> 
>      return ret;
> diff --git a/xen/common/domain-builder/fdt.h b/xen/common/domain-builder/fdt.h
> index 97a45a6ec4..955aead497 100644
> --- a/xen/common/domain-builder/fdt.h
> +++ b/xen/common/domain-builder/fdt.h
> @@ -8,5 +8,6 @@ struct boot_info;
>  #define HYPERLAUNCH_MODULE_IDX 0
> 
>  int has_hyperlaunch_fdt(const struct boot_info *bi);
> +int walk_hyperlaunch_fdt(struct boot_info *bi);
> 
>  #endif /* __XEN_DOMAIN_BUILDER_FDT_H__ */
> --
> 2.43.0
> 
>