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 01:39:40PM +0200, Jan Beulich wrote:
> On 31.07.2024 13:29, Roger Pau Monné wrote:
> > On Wed, Jul 31, 2024 at 11:55:35AM +0200, Jan Beulich wrote:
> >> On 31.07.2024 11:37, Roger Pau Monné wrote:
> >>> On Wed, Jul 31, 2024 at 11:02:01AM +0200, Jan Beulich wrote:
> >>>> On 31.07.2024 10:51, Roger Pau Monné wrote:
> >>>>> I agree with (a), but I don't think enabling PVH dom0 usage of the
> >>>>> hypercalls should be gated on this.  As said a PV dom0 is already
> >>>>> capable of issuing PHYSDEVOP_{,un}map_pirq operations against a PVH
> >>>>> domU.
> >>>>
> >>>> Okay, I can accept that as an intermediate position. We ought to deny
> >>>> such requests at some point though for PVH domains, the latest in the
> >>>> course of making vPCI work there.
> >>>
> >>> Hm, once physdev_map_pirq() works as intended against PVH domains, I
> >>> don't see why we would prevent the usage of PHYSDEVOP_{,un}map_pirq
> >>> against such domains.
> >>
> >> Well. If it can be made work as intended, then I certainly agree. However,
> >> without even the concept of pIRQ in PVH I'm having a hard time seeing how
> >> it can be made work. Iirc you were advocating for us to not introduce pIRQ
> >> into PVH.
> > 
> > From what I'm seeing here the intention is to expose
> > PHYSDEVOP_{,un}map_pirq to PVH dom0, so there must be some notion of
> > pIRQs or akin in a PVH dom0?  Even if only for passthrough needs.
> 
> Only in so far as it is an abstract, handle-like value pertaining solely
> to the target domain.
> 
> >> Maybe you're thinking of re-using the sub-ops, requiring PVH domains to
> >> pass in GSIs?
> > 
> > I think that was one my proposals, to either introduce a new
> > hypercall that takes a GSI, or to modify the PHYSDEVOP_{,un}map_pirq
> > in an ABI compatible way so that semantically the field could be a GSI
> > rather than a pIRQ.  We however would also need a way to reference an
> > MSI entry.
> 
> Of course.
> 
> > My main concern is not with pIRQs by itself, pIRQs are just an
> > abstract way to reference interrupts, my concern and what I wanted to
> > avoid on PVH is being able to route pIRQs over event channels.  IOW:
> > have interrupts from physical devices delivered over event channels.
> 
> Oh, I might have slightly misunderstood your intentions then.

My intention would be to not even use pIRQs at all, in order to avoid
the temptation of the guest itself managing interrupts using
hypercalls, hence I would have preferred that abstract interface to be
something else.

Maybe we could even expose the Xen IRQ space directly, and just use
that as interrupt handles, but since I'm not the one doing the work
I'm not sure it's fair to ask for something that would require more
changes internally to Xen.

> >> I think I suggested something along these lines also to
> >> Jiqian, yet with the now intended exposure to !has_pirq() domains I'm
> >> not sure this could be made work reliably.
> > 
> > I'm afraid I've been lacking behind on reviewing those series.
> > 
> >> Which reminds me of another question I had: What meaning does the pirq
> >> field have right now, if Dom0 would issue the request against a PVH DomU?
> >> What meaning will it have for a !has_pirq() HVM domain?
> > 
> > The pirq field could be a way to reference an interrupt.  It doesn't
> > need to be exposed to the PVH domU at all, but it's a way for the
> > device model to identify which interrupt should be mapped to which
> > domain.
> 
> Since pIRQ-s are per-domain, _that_ kind of association won't be
> helped. But yes, as per above it could serve as an abstract handle-
> like value.

I would be fine with doing the interrupt bindings based on IRQs
instead of pIRQs, but I'm afraid that would require more changes to
hypercalls and Xen internals.

At some point I need to work on a new interface to do passthrough, so
that we can remove the usage of domctls from QEMU.  That might be a
good opportunity to switch from using pIRQs.

Thanks, Roger.