Re: [PATCH v6 02/12] common/hyperlaunch: introduce the domain builder

[Jan Beulich <jbeulich@suse.com>]

On 29.04.2025 14:36, Alejandro Vallejo wrote:
> @@ -1284,9 +1285,14 @@ void asmlinkage __init noreturn __start_xen(void)
>                 bi->nr_modules);
>      }
>  
> -    /* Dom0 kernel is always first */
> -    bi->mods[0].type = BOOTMOD_KERNEL;
> -    bi->domains[0].kernel = &bi->mods[0];
> +    if ( builder_init(bi) == FDT_KIND_NONE )

With this, can ...

> +    {
> +        /* Find first unknown boot module to use as dom0 kernel */
> +        i = first_boot_module_index(bi, BOOTMOD_UNKNOWN);

... i ever be anything else than 0? If not, perhaps keeping the call here is
still fine (kind of for doc purposes), but an assertion may then want adding.

> +        bi->mods[i].type = BOOTMOD_KERNEL;
> +        bi->domains[0].kernel = &bi->mods[i];
> +        bi->hyperlaunch_enabled = false;

Is this necessary, when the field is supposed to be starting out clear?

> --- /dev/null
> +++ b/xen/common/domain-builder/Makefile
> @@ -0,0 +1,2 @@
> +obj-y += fdt.init.o
> +obj-y += core.init.o

Any reason for these not both adding to obj-bin-y, like we do elsewhere for
*.init.o?

Also please sort object lists alphabetically.

> --- /dev/null
> +++ b/xen/include/xen/domain-builder.h
> @@ -0,0 +1,29 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +#ifndef __XEN_DOMAIN_BUILDER_H__
> +#define __XEN_DOMAIN_BUILDER_H__
> +
> +struct boot_info;
> +
> +/* Return status of builder_init(). Shows which boot mechanism was detected */
> +enum fdt_kind
> +{
> +    /* FDT not found. Skipped builder. */
> +    FDT_KIND_NONE,
> +    /* Found an FDT that wasn't hyperlaunch. */
> +    FDT_KIND_UNKNOWN,
> +};
> +
> +/*
> + * Initialises `bi` if it detects a compatible FDT. Otherwise returns
> + * FDT_KIND_NONE and leaves initialisation up to the caller.
> + */
> +#if IS_ENABLED(CONFIG_DOMAIN_BUILDER)

For the pre-processor it wants to be the simpler #ifdef.

Jan