Re: [PATCH] x86/hvm: don't expose XENFEAT_hvm_pirqs by default

["Roger Pau Monné" <roger.pau@citrix.com>]

On Wed, Jan 10, 2024 at 11:26:02AM +0100, Jan Beulich wrote:
> On 10.01.2024 10:53, Roger Pau Monne wrote:
> > The HVM pirq feature allows routing interrupts from both physical and emulated
> > devices over event channels, this was done a performance improvement.  However
> > its usage is fully undocumented, and the only reference implementation is in
> > Linux.  It defeats the purpose of local APIC hardware virtualization, because
> > when using it interrupts avoid the usage of the local APIC altogether.
> 
> So without sufficient APIC acceleration, isn't this arranging for degraded
> performance then? IOW should the new default perhaps be dependent on the
> degree of APIC acceleration?

Maybe, I certainly have no figures, but given the feature is not
working as expected I don't think we should block disabling it on
performance reasons.  Reliability should always be first to
performance.

> > It has also been reported to not work properly with certain devices, at least
> > when using some AMD GPUs Linux attempts to route interrupts over event
> > channels, but Xen doesn't correctly detect such routing, which leads to the
> > hypervisor complaining with:
> > 
> > (XEN) d15v0: Unsupported MSI delivery mode 7 for Dom15
> > 
> > When MSIs are attempted to be routed over event channels the entry delivery
> > mode is set to ExtINT, but Xen doesn't detect such routing and attempts to
> > inject the interrupt following the native MSI path, and the ExtINT delivery
> > mode is not supported.
> 
> Shouldn't this be properly addressed nevertheless?

There's no documentation at all about how the HVM PIRQ interface, so
every time I had to deal with it I had to guess and reverse engineer
how it's supposed to work.

> The way it's described
> it sounds as if MSI wouldn't work at all this way; I can't spot why the
> issue would only be "with certain devices". Yet that in turn doesn't look
> to be very likely - pass-through use cases, in particular SR-IOV ones,
> would certainly have noticed.

I think it has to do with when/how the MSI is updated.  I can't repro
myself, which makes fixing even more complicated.

TBH, I think the feature is simply broken, and I don't feel like
spending more time fixing it.  The fact that it was added in the first
place, and enabled by default was IMO a mistake.

If someone is willing to fix the issue, I'm fine with that, but I
certainly don't want to spend more time fixing issues for an
undocumented feature, the more that going forward such feature is
likely to be even less relevant with hardware APIC virtualization
being more widely available.

FWIW, this patch has an issue in libxl with PV guests, so will need to
send v2.

Thanks, Roger.