Re: [PATCH v2 07/15] x86/hyperlaunch: initial support for hyperlaunch device tree

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.12.2024 17:57, Daniel P. Smith wrote:
> --- a/xen/arch/x86/domain-builder/fdt.c
> +++ b/xen/arch/x86/domain-builder/fdt.c
> @@ -13,14 +13,77 @@
>  
>  #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;
> +        else
> +            return hv_node;
> +    }
> +    else
> +    {
> +        /* Lood for dom0less config */
> +        int node, chosen_node = fdt_path_offset(fdt, "/chosen");
> +        if ( chosen_node < 0 )

Nit: Blank line between declaration(s) and statement(s) please. I'd also
question "node" being plain int, if libfdt didn't have it like this.

> +            return -ENOENT;
> +
> +        fdt_for_each_subnode(node, fdt, chosen_node)
> +        {
> +            if ( fdt_node_check_compatible(fdt, node, "xen,domain") == 0 )
> +                return chosen_node;
> +        }
> +    }
> +
> +    return -ENODATA;
> +}
> +
>  int __init has_hyperlaunch_fdt(struct boot_info *bi)
>  {
>      int ret = 0;
>      const void *fdt = bootstrap_map_bm(&bi->mods[HYPERLAUNCH_MODULE_IDX]);
>  
> -    if ( fdt_check_header(fdt) < 0 )
> +    if ( !fdt || fdt_check_header(fdt) < 0 )
>          ret = -EINVAL;
> +    else
> +        ret = find_hyperlaunch_node(fdt);
> +
> +    bootstrap_unmap();
> +
> +    return ret < 0 ? ret : 0;
> +}
> +
> +int __init walk_hyperlaunch_fdt(struct boot_info *bi)

const?

> +{
> +    int ret = 0, hv_node, node;
> +    void *fdt = bootstrap_map_bm(&bi->mods[HYPERLAUNCH_MODULE_IDX]);

const?

Jan