On 08.04.2025 18:07, Alejandro Vallejo wrote:
> From: "Daniel P. Smith" <dpsm...@apertussolutions.com>
>
> Look for a subnode of type `multiboot,kernel` within a domain node. If
> found, locate it using the multiboot module helper to generically ensure
> it lives in the module list. If the bootargs property is present and
> there was not an MB1 string, then use the command line from the device
> tree definition.
>
> Signed-off-by: Daniel P. Smith <dpsm...@apertussolutions.com>
> Signed-off-by: Jason Andryuk <jason.andr...@amd.com>
> Signed-off-by: Alejandro Vallejo <agarc...@amd.com>
> ---
> v3:
> * Add const to fdt
> * Remove idx == NULL checks
> * Add BUILD_BUG_ON for MAX_NR_BOOTMODS fitting in a uint32_t
At least this one looks to rather belong into patch 08?
> --- a/xen/arch/x86/domain-builder/fdt.c
> +++ b/xen/arch/x86/domain-builder/fdt.c
> @@ -155,6 +155,52 @@ int __init fdt_read_multiboot_module(const void *fdt,
> int node,
> return idx;
> }
>
> +static int __init process_domain_node(
> + struct boot_info *bi, const void *fdt, int dom_node)
> +{
> + int node;
> + struct boot_domain *bd = &bi->domains[bi->nr_domains];
> + const char *name = fdt_get_name(fdt, dom_node, NULL) ?: "unknown";
> + int address_cells = fdt_address_cells(fdt, dom_node);
> + int size_cells = fdt_size_cells(fdt, dom_node);
Oh, okay - regarding my earlier comment elsewhere: If the sizes come from DT,
then of course ASSERT_UNREACHABLE() can't be used at the place where bogus
ones are rejected.
> + fdt_for_each_subnode(node, fdt, dom_node)
> + {
> + if ( fdt_node_check_compatible(fdt, node, "multiboot,kernel") == 0 )
> + {
When the loop body is merely an if() with a non-negligible body, inverting
the condition and using "continue" is usually better. Much like you do ...
> + int idx;
> +
> + if ( bd->kernel )
> + {
> + printk(XENLOG_ERR "Duplicate kernel module for domain %s\n",
> + name);
> + continue;
... here already.
Jan
