Re: [PATCH v3 06/16] x86/hyperlaunch: introduce the domain builder

[Alejandro Vallejo <agarciav@xxxxxxx>]

On Wed Apr 9, 2025 at 7:53 PM BST, Denis Mukhin wrote:
> On Tuesday, April 8th, 2025 at 9:07 AM, Alejandro Vallejo <agarciav@xxxxxxx> 
> wrote:
>
>> 
>> 
>> From: "Daniel P. Smith" dpsmith@xxxxxxxxxxxxxxxxxxxx
>> 
>> 
>> Introduce the domain builder which is capable of consuming a device tree as 
>> the
>> first boot module. If it finds a device tree as the first boot module, it 
>> will
>> set its type to BOOTMOD_FDT. This change only detects the boot module and
>> continues to boot with slight change to the boot convention that the dom0
>> kernel is no longer first boot module but is the second.
>> 
>> No functional change intended.
>> 
>> Signed-off-by: Daniel P. Smith dpsmith@xxxxxxxxxxxxxxxxxxxx
>> 
>> Signed-off-by: Jason Andryuk jason.andryuk@xxxxxxx
>> 
>> ---
>> v3:
>> * Move obj-y += domain-builder/
>> * Remove blank line in Makefile
>> * const in has_hyperlaunch_fdt()
>> * CONFIG_LIBFDT rename
>> * Use boot_info forward declaration
>> * Rename domainbuilder.h to domain-builder.h
>> * Add fdt NULL check
>> ---
>> xen/arch/x86/Makefile | 1 +
>> xen/arch/x86/domain-builder/Kconfig | 2 +-
>> xen/arch/x86/domain-builder/Makefile | 2 +
>> xen/arch/x86/domain-builder/core.c | 57 +++++++++++++++++++++++
>> xen/arch/x86/domain-builder/fdt.c | 37 +++++++++++++++
>> xen/arch/x86/domain-builder/fdt.h | 21 +++++++++
>
> I have a general question.
>
> Wouldn't that make sense to use arch-independent placeholder for domain 
> builder
> code right from the starting point?
>
> For example something like xen/common/domain-builder ?
>
> My understanding is that there's a lot of code in the domain builder which
> can be potentially shared/re-used with non-x86 architectures.
>
> Also, that seems to align with the intention to have common arch-independent
> subsystems in the code base:
>
>    
> https://docs.google.com/presentation/d/1q9cjJZLUxUo1YzMpCxVHuL-ZOGoFaO9haHfRBK4i4Uc/edit?slide=id.g32afc87aef4_0_18#slide=id.g32afc87aef4_0_18

  https://lore.kernel.org/xen-devel/D92Z1LECR2VA.3FW5H7ZIRVXU7@xxxxxxx/

In principle, yes. I'll try to move it, but I suspect making it coexist
with dom0less will prove tricky.

>> diff --git a/xen/arch/x86/domain-builder/core.c 
>> b/xen/arch/x86/domain-builder/core.c
>> new file mode 100644
>> index 0000000000..d6ae94f45c
>> --- /dev/null
>> +++ b/xen/arch/x86/domain-builder/core.c
>> @@ -0,0 +1,57 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only /
>> +/
>> + * Copyright (C) 2024, Apertus Solutions, LLC
>> + */
>> +#include <xen/err.h>
>> 
>> +#include <xen/init.h>
>> 
>> +#include <xen/kconfig.h>
>> 
>> +#include <xen/lib.h>
>> 
>> +
>> +#include <asm/bootinfo.h>
>> 
>> +
>> +#include "fdt.h"
>> +
>> +void __init builder_init(struct boot_info *bi)
>> +{
>> + if ( IS_ENABLED(CONFIG_DOMAIN_BUILDER) )
>> + {
>> + int ret;
>> +
>> + switch ( ret = has_hyperlaunch_fdt(bi) )
>> + {
>> + case 0:
>> + printk("Hyperlaunch device tree detected\n");
>> + bi->hyperlaunch_enabled = true;

FYI, your email client seems to be collapsing the indentation of the
quoted patches. Is that intentional?

Cheers,
Alejandro