On Wed May 21, 2025 at 10:54 AM CEST, Jan Beulich wrote:
> 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.
I don't think so, no. It's there for symmetry with the way the initrd is
discovered later on, as that might be on module1, or 2, or 3 depending
on whether there's microcode, or an XSM policy or anything else. in
other modules.
>
>> + 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?
It isn't necessary, but I think it gave a sense of intent. That said I'm
pondering removing that boolean though in favour of something like
bi->mods[0].type == BOOTMOD_FDT
At which point that assignment disappears.
>
>> --- /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.
True. Don't know what I was thinking.
>
> Jan
Cheers,
Alejandro
