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

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

On 11/25/24 15:11, Jason Andryuk wrote:
> On 2024-11-23 13:20, Daniel P. Smith wrote:
> > 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 if more than one is found.
> >
> > Signed-off-by: Daniel P. Smith <dpsmith@xxxxxxxxxxxxxxxxxxxx>
> > ---
> >   xen/arch/x86/domain_builder/core.c  | 14 +++++++
> >   xen/arch/x86/domain_builder/fdt.c   | 64 ++++++++++++++++++++++++++++-
> >   xen/arch/x86/domain_builder/fdt.h   |  5 +++
> >   xen/arch/x86/include/asm/bootinfo.h |  1 +
> >   4 files changed, 83 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/x86/domain_builder/core.c b/xen/arch/x86/ 
> > domain_builder/core.c
> > index 211359895d84..a80f3711c306 100644
> > --- a/xen/arch/x86/domain_builder/core.c
> > +++ b/xen/arch/x86/domain_builder/core.c
> > @@ -40,7 +40,21 @@ void __init builder_init(struct boot_info *bi)
> >                      ret);
> >               bi->hyperlaunch_enabled = false;
> >           }
> > +    }
> > +
> > +    if ( bi->hyperlaunch_enabled )
> > +    {
> > +        int ret;
> > +
> > +        printk(XENLOG_INFO "Hyperlauch configuration:\n");
> Hyperlaunch
Ack.

> > +        if ( (ret = walk_hyperlaunch_fdt(bi)) < 0 )
> > +        {
> > +            printk(XENLOG_INFO "  walk of device tree failed (%d)\n", 
> > ret);
> > +            bi->hyperlaunch_enabled = false;
> > +            return;
> > +        }
> > +        printk(XENLOG_INFO "  Number of domains: %d\n", bi->nr_domains);
> >       }
> >   }
> > diff --git a/xen/arch/x86/domain_builder/fdt.c b/xen/arch/x86/ 
> > domain_builder/fdt.c
> > index 3f9dda8c34c3..ff1ba58b6907 100644
> > --- a/xen/arch/x86/domain_builder/fdt.c
> > +++ b/xen/arch/x86/domain_builder/fdt.c
> > +int __init walk_hyperlaunch_fdt(struct boot_info *bi)
> > +{
> > +    int ret = 0, hv_node, node;
> > +    void *fdt = bootstrap_map_bm(&bi->mods[HYPERLAUNCH_MODULE_IDX]);
> > +
> > +    if ( unlikely(!fdt) )
> > +        return -EINVAL;
> > +
> > +    hv_node = find_hyperlaunch_node(fdt);
> > +    if ( hv_node < 0 )
> > +    {
> > +        ret = hv_node;
> > +        goto err_out;
> > +    }
> > +
> > +    fdt_for_each_subnode(node, fdt, hv_node)
> > +    {
> > +        ret = fdt_node_check_compatible(fdt, node, "xen,domain");
> > +        if ( ret == 0 )
> > +            bi->nr_domains++;
> > +    }
> > +
> > +    /* Until multi-domain construction is added, throw an error */
> > +    if ( !bi->nr_domains || bi->nr_domains > 1 )
> > +        printk(XENLOG_ERR "Hyperlaunch only supports dom0 
> > construction\n");
> You continue execution - is that intended?  It'll take the next module 
> as the kernel and try to boot?  Would you rather panic?
Yes, it was intended, and as of this commit, it will use the next module 
as the kernel. That is the boot convention at this point. In this 
scenario, the system was given a valid HL device tree that happen to 
have multiple domains defined in it. At this point in the series, a 
domain definition literally has zero effect on the boot process, so 
there is no reason to panic.
v/r,
dps