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

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

On Fri, Jan 12, 2024 at 02:40:35PM +0000, Anthony PERARD wrote:
> On Wed, Jan 10, 2024 at 01:42:02PM +0100, 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.
> > 
> > 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.
> > 
> > Disable HVM PIRQs by default and provide a per-domain option in xl.cfg to
> > enable such feature.  Also for backwards compatibility keep the feature 
> > enabled
> > for any resumed domains that don't have an explicit selection.
> > 
> > Note that the only user of the feature (Linux) is also able to handle native
> > interrupts fine, as the feature was already not used if Xen reported local 
> > APIC
> > hardware virtualization active.
> > 
> > Link: https://github.com/QubesOS/qubes-issues/issues/7971
> > Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> 
> Should we have an entry in the changelog about this patch?

Yes, indeed.  I always forget.  Will send v3 on Monday with you RB and
the changelog.

> With the patch, we will need to regenerate the golang binding, at least
> on commit.
> 
> Otherwise, patch looks fine:
> Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Thanks!