On Tuesday, April 8th, 2025 at 9:07 AM, Alejandro Vallejo <agarc...@amd.com>
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
> * Remove trailing ) from printks
> * Return ENODATA for missing kernel
> * Re-work "max domains" warning and print limit
> * fdt_cell_as_u32/directly return values
> * Remove "pairs" looping from fdt_get_reg_prop() and only grab 1.
> * Use addr_cells and size_cells
> ---
> xen/arch/x86/domain-builder/core.c | 11 ++++++
> xen/arch/x86/domain-builder/fdt.c | 57 ++++++++++++++++++++++++++++++
> xen/arch/x86/setup.c | 5 ---
> 3 files changed, 68 insertions(+), 5 deletions(-)
>
> diff --git a/xen/arch/x86/domain-builder/core.c
> b/xen/arch/x86/domain-builder/core.c
> index c50eff34fb..eda7fa7a8f 100644
> --- a/xen/arch/x86/domain-builder/core.c
> +++ b/xen/arch/x86/domain-builder/core.c
> @@ -59,6 +59,17 @@ void __init builder_init(struct boot_info *bi)
>
> printk(XENLOG_INFO " Number of domains: %d\n", bi->nr_domains);
>
> }
> + else
> + {
> + unsigned int i;
> +
> + /* Find first unknown boot module to use as Dom0 kernel */
> + printk("Falling back to using first boot module as dom0\n");
> + i = first_boot_module_index(bi, BOOTMOD_UNKNOWN);
> + bi->mods[i].type = BOOTMOD_KERNEL;
>
> + bi->domains[0].kernel = &bi->mods[i];
>
> + bi->nr_domains = 1;
>
> + }
> }
>
> /*
> diff --git a/xen/arch/x86/domain-builder/fdt.c
> b/xen/arch/x86/domain-builder/fdt.c
> index 9ebc8fd0e4..a037c8b6cb 100644
> --- 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);
> +
> + fdt_for_each_subnode(node, fdt, dom_node)
> + {
> + if ( fdt_node_check_compatible(fdt, node, "multiboot,kernel") == 0 )
> + {
> + int idx;
> +
> + if ( bd->kernel )
>
> + {
> + printk(XENLOG_ERR "Duplicate kernel module for domain %s\n",
Looks like it should be XENLOG_WARNING since the loop continues.
Also, I would use either Capitalized or lower case messages everywhere
for consistency.
> + name);
> + continue;
> + }
> +
> + idx = fdt_read_multiboot_module(fdt, node, address_cells,
> + size_cells, bi);
> + if ( idx < 0 )
> + {
> + printk(" failed processing kernel module for domain %s\n",
I think this printout should have XENLOG_ERR in it since it's on the
error code path.
> + name);
> + return idx;
> + }
> +
> + printk(" kernel: boot module %d\n", idx);
> + bi->mods[idx].type = BOOTMOD_KERNEL;
>
> + bd->kernel = &bi->mods[idx];
>
> + }
> + }
> +
> + if ( !bd->kernel )
>
> + {
> + printk(XENLOG_ERR "ERR: no kernel assigned to domain\n");
Drop "ERR" since it is already XENLOG_ERR level?
> + return -ENODATA;
> + }
> +
> + return 0;
> +}
> +
> static int __init find_hyperlaunch_node(const void *fdt)
> {
> int hv_node = fdt_path_offset(fdt, "/chosen/hypervisor");
> @@ -217,9 +263,20 @@ int __init walk_hyperlaunch_fdt(struct boot_info *bi)
>
> fdt_for_each_subnode(node, fdt, hv_node)
> {
> + if ( bi->nr_domains >= MAX_NR_BOOTDOMS )
>
> + {
> + printk(XENLOG_WARNING
> + "WARN: only creating first %u domains\n", MAX_NR_BOOTDOMS);
Drop "WARN" since it is already XENLOG_WARNING level?
> + break;
> + }
> +
> ret = fdt_node_check_compatible(fdt, node, "xen,domain");
> if ( ret == 0 )
> + {
> + if ( (ret = process_domain_node(bi, fdt, node)) < 0 )
> + break;
> bi->nr_domains++;
>
> + }
> }
>
> /* Until multi-domain construction is added, throw an error /
> diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
> index e5d78bcb48..00e8c8a2a3 100644
> --- a/xen/arch/x86/setup.c
> +++ b/xen/arch/x86/setup.c
> @@ -1284,11 +1284,6 @@ void asmlinkage __init noreturn __start_xen(void)
>
> builder_init(bi);
>
> - / Find first unknown boot module to use as Dom0 kernel */
> - i = first_boot_module_index(bi, BOOTMOD_UNKNOWN);
> - bi->mods[i].type = BOOTMOD_KERNEL;
>
> - bi->domains[0].kernel = &bi->mods[i];
>
> -
> if ( pvh_boot )
> {
> /* pvh_init() already filled in e820_raw */
> --
> 2.43.0
