Re: [PATCH v5 00/12] Hyperlaunch device tree for dom0

[Alejandro Vallejo <agarciav@xxxxxxx>]

On Thu Apr 24, 2025 at 11:19 PM BST, dmkhn wrote:
> On Thu, Apr 24, 2025 at 05:10:10PM +0100, Alejandro Vallejo wrote:
>> Hi,
>> 
>> v4: 
>> https://lore.kernel.org/xen-devel/20250417124844.11143-1-agarciav@xxxxxxx/
>> v3: 
>> https://lore.kernel.org/xen-devel/20250408160802.49870-1-agarciav@xxxxxxx/
>> v2: 
>> https://lore.kernel.org/xen-devel/20241226165740.29812-1-dpsmith@xxxxxxxxxxxxxxxxxxxx/
>> v1: 
>> https://lore.kernel.org/xen-devel/20241123182044.30687-1-dpsmith@xxxxxxxxxxxxxxxxxxxx/
>> 
>> ========= Original cover letter:
>> 
>> The Hyperlaunch device tree for dom0 series is the second split out for the
>> introduction of the Hyperlaunch domain builder logic. These changes focus on
>> introducing the ability to express a domain configuration that is then used 
>> to
>> populate the struct boot_domain structure for dom0. This ability to express a
>> domain configuration provides the next step towards a general domain builder.
>> 
>> The splitting of Hyperlaunch into a set of series are twofold, to reduce the
>> effort in reviewing a much larger series, and to reduce the effort in 
>> handling
>> the knock-on effects to the construction logic from requested review changes.
>> 
>> 
>> 
>> Alejandro Vallejo (1):
>>   x86/hyperlaunch: Add helpers to locate multiboot modules
>> 
>> Daniel P. Smith (11):
>>   kconfig: introduce domain builder config options
>>   common/hyperlaunch: introduce the domain builder
>>   x86/hyperlaunch: initial support for hyperlaunch device tree
>>   x86/hyperlaunch: locate dom0 kernel with hyperlaunch
>>   x86/hyperlaunch: obtain cmdline from device tree
>>   x86/hyperlaunch: locate dom0 initrd with hyperlaunch
>>   x86/hyperlaunch: add domain id parsing to domain config
>>   x86/hyperlaunch: specify dom0 mode with device tree
>>   x86/hyperlaunch: add memory parsing to domain config
>>   x86/hyperlaunch: add max vcpu parsing of hyperlaunch device tree
>>   x86/hyperlaunch: add capabilities to boot domain
>
> One general comment w.r.t. naming notation in:
>
>     xen/common/domain-builder/fdt.c
>     xen/common/domain-builder/fdt.h
>
> Sorry, overlooked that earlier.
>
> What do you think about using fdt_<something> notation for all functions in
> fdt.{c,h}?
>
> For example:
>
>     s/detect_fdt_kind/fdt_detect_hyperlauch_node/
                                         ^       
                                         kind?
>
>     s/find_hyperlaunch_node/fdt_find_hyperlauch_node/
>
>     s/walk_hyperlaunch_fdt/fdt_walk_hyperlauch_node/
>
>     s/process_domain_node/fdt_process_hyperlauch_node/
                                         ^
                                         domain?

Assuming the two nits above are mistakes on copy-paste, it sounds
reasonable. I'll wait for some acks on the rest and resend with those
adjustments if needed.

>
> Other than that:
>
> Reviewed-by: Denis Mukhin <dmukhin@xxxxxxxx>
>
> for the series.

Thanks a lot!

Cheers,
Alejandro