Re: [PATCH] hw/intc/ioapic: Update KVM routes before redelivering IRQ, on RTE update

[Peter Xu]

On Mon, Mar 06, 2023 at 09:25:35AM +0000, David Woodhouse wrote:
> On Mon, 2023-03-06 at 06:51 +0000, David Woodhouse wrote:
> > On 5 March 2023 22:36:18 GMT, Peter Xu <peterx@redhat.com> wrote:
> > > On Sun, Mar 05, 2023 at 06:43:42PM +0000, 
> > > > ---
> > > > Alternative fixes might have been just to remove the part in
> > > > ioapic_service() which delivers the IRQ via kvm_set_irq() because
> > > > surely delivering as MSI ought to work just fine anyway in all cases?
> > > > That code lacks a comment justifying its existence.
> > > 
> > > Didn't check all details, but AFAIU there're still some different paths
> > > triggered so at least it looks still clean to use the path it's for.
> > > 
> > > E.g., I think if someone traces kvm_set_irq() in kernel this specific irq
> > > triggered right after unmasking might seem to be missed misterously (but
> > > actually it was not).
> > 
> > Hm, not sure that's a reason we care about. The I/OAPIC is purely a
> > device to turn line interrupts into MSIs. Which these days need to be
> > translated by IOMMU interrupt remapping device models in userspace. I
> > don't think a user has any valid reason to expect that the kernel
> > will even know about any GSIs with any specific numbers. Tracing on
> > that in the kernel would making some dodgy assumptions.
> 
> I think if we want to fix the lack of IR translation faults from the
> IOMMU, we have to change this anyway.
> 
> At the very least, it can only use KVM to deliver the IRQ if there *is*
> a valid translation. And if not, it needs to ask the IOMMU to
> retranslate, with a 'delivering_now' flag indicating that the fault
> should be raised because this isn't a preemptive translation in
> advance.

I see that as a separate problem of what this patch wants to (rightfully)
fix.  I also agree we'll need that if e.g. we want to support ioapic in
kvm.

Sorry in advancie since I don't think I have the whole picture here.  Could
you remind me the whole purpose of having such?  Is that for part of Xen's
effort?

It was a long time ago, but IIRC such mechanism wasn't implemented when we
worked on vIOMMU IR initially, because at least at that time the "keeping
kvm irq routes always updated when IRQ entries modified" was good enough
for qemu to work with IR.

The only outlier was in-kernel ioapic, but there was a talk from Google
showing that in kernel ioapic brought mostly nothing good but more exposure
on attack surface.  It does sound reasonable since fast devices shouldn't
use ioapic at all to me, so IIRC the plan was split irqchip should be
always optimal hence no immediate concern of not supporting IR with
in-kernel ioapic.  But I definitely can miss important things along the
way.

Thanks,

-- 
Peter Xu