Re: [PATCH v3 13/16] x86/hyperlaunch: specify dom0 mode with device tree

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.04.2025 16:09, Daniel P. Smith wrote:
> On 4/16/25 09:38, Jan Beulich wrote:
>> On 16.04.2025 15:32, Daniel P. Smith wrote:
>>> On 4/10/25 07:57, Jan Beulich wrote:
>>>> On 08.04.2025 18:07, Alejandro Vallejo wrote:
>>>>> --- a/xen/arch/x86/domain-builder/fdt.c
>>>>> +++ b/xen/arch/x86/domain-builder/fdt.c
>>>>> @@ -193,6 +193,25 @@ static int __init process_domain_node(
>>>>>                bd->domid = (domid_t)val;
>>>>>                printk("  domid: %d\n", bd->domid);
>>>>>            }
>>>>> +        else if ( strncmp(prop_name, "mode", name_len) == 0 )
>>>>> +        {
>>>>> +            if ( fdt_prop_as_u32(prop, &bd->mode) != 0 )
>>>>> +            {
>>>>> +                printk("  failed processing mode for domain %s\n", name);
>>>>> +                return -EINVAL;
>>>>> +            }
>>>>> +
>>>>> +            printk("  mode: ");
>>>>> +            if ( !(bd->mode & BUILD_MODE_PARAVIRT) )
>>>>> +            {
>>>>> +                if ( bd->mode & BUILD_MODE_ENABLE_DM )
>>>>> +                    printk("HVM\n");
>>>>> +                else
>>>>> +                    printk("PVH\n");
>>>>> +            }
>>>>> +            else
>>>>> +                printk("PV\n");
>>>>> +        }
>>>>
>>>> My prior questions here remain: What's the significance of
>>>> BUILD_MODE_ENABLE_DM when set alongside BUILD_MODE_PARAVIRT? What about
>>>> any of the other bits being set?
>>>
>>>   From boot-domain.h:
>>>                                             /* On     | Off    */
>>> #define BUILD_MODE_PARAVIRT      (1 << 0) /* PV     | PVH/HVM */
>>> #define BUILD_MODE_ENABLE_DM     (1 << 1) /* HVM    | PVH     */
>>>
>>> The logic says, if BUILD_MODE_PARAVIRT bit is not set, thus an HVM
>>> domain, check if BUILD_MODE_ENABLE_DM has been set. This is determin if
>>> the domain is what the toolstack differentiates as either an HVM or PVH
>>> domain. As you should know, there is no case of a PV domain requiring a
>>> backing device mode (DM) domain. IOW, BUILD_MODE_ENABLE_DM is only
>>> relevant to an HVM domain.
>>
>> And hence should (my conclusion) never be set for a PV one.
> 
> Yes and?

And it being wrongly set should be rejected, rather than silently ignored.

Jan