Re: [PATCH v5] x86: make Viridian support optional

[Jan Beulich <jbeulich@xxxxxxxx>]

On 09.10.2025 18:33, Grygorii Strashko wrote:
> On 08.10.25 19:04, Jan Beulich wrote:
>> On 30.09.2025 14:52, Grygorii Strashko wrote:
>>> --- a/xen/arch/x86/hvm/Kconfig
>>> +++ b/xen/arch/x86/hvm/Kconfig
>>> @@ -62,6 +62,16 @@ config ALTP2M
>>>           If unsure, stay with defaults.
>>>   +config VIRIDIAN
>>> +    bool "Hyper-V enlightenments for guests" if EXPERT
>>> +    default y
>>> +    help
>>> +      Support optimizations for Hyper-V guests such as faster hypercalls,
>>> +      efficient timer and interrupt handling, and enhanced paravirtualized
>>> +      I/O. This is to improve performance and compatibility of Windows VMs.
>>
>> What is "paravirtualized I/O" about in this context?
> 
> It was phrased this way and agreed upon from the very beginning [1]
> 
> [1] 
> https://patchwork.kernel.org/project/xen-devel/patch/20250317071919.3766976-1-Sergiy_Kibrik@xxxxxxxx/#26305207

Hmm, I'm not sure I would call this "agreed upon". Plus this doesn't answer
my question.

>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -701,9 +701,12 @@ int hvm_domain_initialise(struct domain *d,
>>>       if ( hvm_tsc_scaling_supported )
>>>           d->arch.hvm.tsc_scaling_ratio = hvm_default_tsc_scaling_ratio;
>>>   -    rc = viridian_domain_init(d);
>>> -    if ( rc )
>>> -        goto fail2;
>>> +    if ( is_viridian_domain(d) )
>>> +    {
>>> +        rc = viridian_domain_init(d);
>>> +        if ( rc )
>>> +            goto fail2;
>>> +    }
>>
>> While this looks okay to me, ...
>>
>>> @@ -739,7 +742,8 @@ void hvm_domain_relinquish_resources(struct domain *d)
>>>       if ( hvm_funcs.nhvm_domain_relinquish_resources )
>>>           alternative_vcall(hvm_funcs.nhvm_domain_relinquish_resources, d);
>>>   -    viridian_domain_deinit(d);
>>> +    if ( is_viridian_domain(d) )
>>> +        viridian_domain_deinit(d);
>>
>> ... I wonder if viridian_{domain,vcpu}_deinit() better wouldn't be tolerant
>> to be called anyway, thus avoiding the need for conditionals here and below
>> (and perhaps being a little more robust overall). Thoughts?
> 
> I think There are no limits for perfection. But at some point - need to stop.
> 
> Hence Viridian code will be removed from build when !VIRIDIAN - changing 
> above code will
> require to add stubs for viridian_{domain,vcpu}_deinit().

Fair point.

Jan