On 01.04.2025 14:39, Jason Andryuk wrote:
> On 2025-04-01 08:00, Jan Beulich wrote:
>> On 31.03.2025 23:43, Jason Andryuk wrote:
>>> --- a/xen/common/domain.c
>>> +++ b/xen/common/domain.c
>>> @@ -820,11 +820,15 @@ struct domain *domain_create(domid_t domid,
>>> d->is_privileged = flags & CDF_privileged;
>>>
>>> /* Sort out our idea of is_hardware_domain(). */
>>> - if ( domid == 0 || domid == hardware_domid )
>>> + if ( (flags & CDF_hardware) || domid == hardware_domid )
>>
>> Since it's || here ...
>>
>>> {
>>> if ( hardware_domid < 0 || hardware_domid >= DOMID_FIRST_RESERVED
>>> )
>>> panic("The value of hardware_dom must be a valid domain
>>> ID\n");
>>>
>>> + /* late_hwdom is only allowed for dom0. */
>>> + if ( hardware_domain && hardware_domain->domain_id )
>>> + return ERR_PTR(-EINVAL);
>>> +
>>> old_hwdom = hardware_domain;
>>> hardware_domain = d;
>>> }
>>
>> ... doesn't this code then also need to set CDF_hardware if it's unset
>> in the function argument?
>
> I don't think it matters today - later construction depends on the value
> of hardware_domain. Which is also used for the check underlying
> is_hardware_domain().
>
> But I agree that it makes sense to set it here in case the use of
> CDF_hardware expands in the future.
If we don't do this now, it'll be quite likely that it'll be forgotten
later. Hardly anyone actually tests late-hwdom these days, afaict. It
may also matter if someone looks at e.g. a dump of Xen after a crash.
Jan