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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

Thanks, Roger.