Re: [XEN PATCH v12 2/7] x86/pvh: Allow (un)map_pirq when dom0 is PVH

[Jan Beulich <jbeulich@xxxxxxxx>]

On 31.07.2024 10:24, Roger Pau Monné wrote:
> On Wed, Jul 31, 2024 at 09:58:28AM +0200, Jan Beulich wrote:
>> On 31.07.2024 09:50, Roger Pau Monné wrote:
>>> On Mon, Jul 08, 2024 at 07:41:19PM +0800, Jiqian Chen wrote:
>>>> --- a/xen/arch/x86/physdev.c
>>>> +++ b/xen/arch/x86/physdev.c
>>>> @@ -323,7 +323,11 @@ ret_t do_physdev_op(int cmd, 
>>>> XEN_GUEST_HANDLE_PARAM(void) arg)
>>>>          if ( !d )
>>>>              break;
>>>>  
>>>> -        ret = physdev_map_pirq(d, map.type, &map.index, &map.pirq, &msi);
>>>> +        /* Only mapping when the subject domain has a notion of PIRQ */
>>>> +        if ( !is_hvm_domain(d) || has_pirq(d) )
>>>
>>> I'm afraid this is not true.  It's fine to map interrupts to HVM
>>> domains that don't have XENFEAT_hvm_pirqs enabled.  has_pirq() simply
>>> allow HVM domains to route interrupts from devices (either emulated or
>>> passed through) over event channels.
>>>
>>> It might have worked in the past (when using a version of Xen < 4.19)
>>> because XENFEAT_hvm_pirqs was enabled by default for HVM guests.
>>>
>>> physdev_map_pirq() will work fine when used against domains that don't
>>> have XENFEAT_hvm_pirqs enabled, and it needs to be kept this way.
>>>
>>> I think you want to allow PHYSDEVOP_{,un}map_pirq for HVM domains, but
>>> keep the code in do_physdev_op() as-is.  You will have to check
>>> whether the current paths in do_physdev_op() are not making
>>> assumptions about XENFEAT_hvm_pirqs being enabled when the calling
>>> domain is of HVM type.  I don't think that's the case, but better
>>> check.
>>
>> Yet the goal is to disallow mapping into PVH domains. The use of
>> has_pirq() was aiming at that. If that predicate can't be used (anymore)
>> for this purpose, which one is appropriate now?
> 
> Why do you want to add such restriction now, when it's not currently
> present?
> 
> It was already the case that a PV dom0 could issue
> PHYSDEVOP_{,un}map_pirq operations against a PVH domU, whatever the
> result of such operation be.

Because (a) that was wrong and (b) we'd suddenly permit a PVH DomU to
issue such for itself.

Jan