Re: [XEN PATCH] x86/hvm: make ACPI PM timer support optional

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.09.2024 15:35, Sergiy Kibrik wrote:
> 18.09.24 12:41, Roger Pau Monné:
>> On Wed, Sep 18, 2024 at 12:29:39PM +0300, Sergiy Kibrik wrote:
>>> 16.09.24 22:57, Stefano Stabellini:
>>>> On Mon, 16 Sep 2024, Roger Pau Monné wrote:
>>>>> On Mon, Sep 16, 2024 at 09:37:57AM +0300, Sergiy Kibrik wrote:
>>>>>> Introduce config option X86_PMTIMER so that pmtimer driver can be 
>>>>>> disabled on
>>>>>> systems that don't need it.
>>>>>
>>>>> Same comment as in the VGA patch, you need to handle the user passing
>>>>> X86_EMU_PM.  It's not OK to just ignore the flag if the hypervisor is
>>>>> built without ACPI PM timer support.
>>>>
>>>> I also think that the flag should not be ignored. I think that Xen
>>>> should return error if a user is passing a domain feature not supported
>>>> by a particular version of the Xen build. I don't think that libxl needs
>>>> to be changed as part of this patch necessarily.
>>>
>>> It looks like toolstack always leaves X86_EMU_PM bit enabled, so that part
>>> may also require changes.
>>
>> I think you will be unable to create HVM guests, but that's kind of
>> expected if ACPI PM emulation is removed from the hypervisor (it won't
>> be an HVM guest anymore if it doesn't have ACPI PM).
>>
>> PVH guest don't set X86_EMU_PM so you should be able to create those
>> fine.
>>
> 
> would the check like this be enough?:
> 
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -758,6 +758,9 @@ static bool emulation_flags_ok(const struct domain 
> *d, uint32_t emflags)
>                (X86_EMU_ALL & ~(X86_EMU_VPCI | X86_EMU_USE_PIRQ)) &&
>                emflags != X86_EMU_LAPIC )
>               return false;
> +        if ( !is_hardware_domain(d) &&
> +             (emflags & X86_EMU_PM) && !IS_ENABLED(CONFIG_X86_PMTIMER))
> +            return false;
>       }
>       else if ( emflags != 0 && emflags != X86_EMU_PIT )
>       {

Why the is_hardware_domain() part of the check?

> (probably with X86_PMTIMER option depending on PV)

HVM you mean?

Jan