On Thu Apr 24, 2025 at 6:41 PM BST, Jason Andryuk wrote:
> On 2025-04-24 12:10, Alejandro Vallejo wrote:
>> From: "Daniel P. Smith" <dpsm...@apertussolutions.com>
>>
>> Introduce the ability to specify the desired domain id for the domain
>> definition. The domain id will be populated in the domid property of the
>> domain node in the device tree configuration.
>>
>> Signed-off-by: Daniel P. Smith <dpsm...@apertussolutions.com>
>> Signed-off-by: Alejandro Vallejo <agarc...@amd.com>
>> ---
>
>> diff --git a/xen/common/domain-builder/fdt.c
>> b/xen/common/domain-builder/fdt.c
>> index 144fcc75b5..5a5b3c8806 100644
>> --- a/xen/common/domain-builder/fdt.c
>> +++ b/xen/common/domain-builder/fdt.c
>
>> @@ -188,12 +189,54 @@ static int __init fdt_read_multiboot_module(const void
>> *fdt, int node,
>> static int __init process_domain_node(
>> struct boot_info *bi, const void *fdt, int dom_node)
>> {
>> - int node;
>> + int node, property;
>> 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_property_offset(property, fdt, dom_node)
>> + {
>> + const struct fdt_property *prop;
>> + const char *prop_name;
>> + int name_len, rc;
>> +
>> + prop = fdt_get_property_by_offset(fdt, property, NULL);
>> + if ( !prop )
>> + continue; /* silently skip */
>> +
>> + prop_name = fdt_get_string(fdt, fdt32_to_cpu(prop->nameoff),
>> &name_len);
>> +
>
> Stray blank line.
>
>> + if ( !strncmp(prop_name, "domid", name_len) )
>> + {
>> + uint32_t val = DOMID_INVALID;
>> +
>> + if ( (rc = fdt_prop_as_u32(prop, &val)) )
>> + {
>> + printk(XENLOG_ERR
>> + " failed processing domain id for domain %s\n",
>> name);
>> + return rc;
>> + }
>
> Maybe add a blank line here?
>
>> + if ( val >= DOMID_FIRST_RESERVED )
>> + {
>> + printk(XENLOG_ERR " invalid domain id for domain %s\n",
>> name);
>> + return -EINVAL;
>> + }
>> +
>
>> @@ -258,6 +301,13 @@ static int __init process_domain_node(
>> return -ENODATA;
>> }
>>
>> + if ( bd->domid == DOMID_INVALID )
>> + bd->domid = get_initial_domain_id();
>> + else if ( bd->domid != get_initial_domain_id() )
>> + printk(XENLOG_WARNING
>> + "warning: unsupported boot. d%d is not the initial
>> domain.\n",
>
> Maybe:
> "warning: d%u is not the expected initial domid (%u)\n" ?
>
> It's a strange message. The domid property is added, but it's not
> expected to actually be used?
It's just a transient message until multidomain boot is added in a later
series. It's merely informative that you're booting something you
probably didn't mean to. Or didn't mean to on this hypervisor.
>
> With the newlines addressed (and optionally the message change):
I'm happy to make those adjustments.
>
> Reviewed-by: Jason Andryuk <jason.andr...@amd.com>
Thanks
>
> Thanks,
> Jason
Cheers,
Alejandro
