diff mbox series

[4/5] KVM: Kick resamplefd for split kernel irqchip

Message ID 20200226225510.217077-1-peterx@redhat.com (mailing list archive)
State New, archived
Headers show
Series vfio/pci: Fix up breakage against split irqchip and INTx | expand

Commit Message

Peter Xu Feb. 26, 2020, 10:55 p.m. UTC
This is majorly only for X86 because that's the only one that supports
split irqchip for now.

When the irqchip is split, we face a dilemma that KVM irqfd will be
enabled, however the slow irqchip is still running in the userspace.
It means that the resamplefd in the kernel irqfds won't take any
effect and it can miss to ack INTx interrupts on EOIs.

One example is split irqchip with VFIO INTx, which will break if we
use the VFIO INTx fast path.

This patch can potentially supports the VFIO fast path again for INTx,
that the IRQ delivery will still use the fast path, while we don't
need to trap MMIOs in QEMU for the device to emulate the EIOs (see the
callers of vfio_eoi() hook).  However the EOI of the INTx will still
need to be done from the userspace by caching all the resamplefds in
QEMU and kick properly for IOAPIC EOI broadcast.

https://patchwork.kernel.org/patch/10738541/#22609933

Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 accel/kvm/kvm-all.c    | 67 ++++++++++++++++++++++++++++++++++++++++++
 accel/kvm/trace-events |  1 +
 hw/intc/ioapic.c       | 11 +++++--
 include/sysemu/kvm.h   |  4 +++
 4 files changed, 81 insertions(+), 2 deletions(-)

Comments

Paolo Bonzini Feb. 28, 2020, 10:34 a.m. UTC | #1
On 26/02/20 23:55, Peter Xu wrote:
> +
> +            /* Kick resamplefd if KVM is bypassed */
> +            kvm_resample_fd_notify(n);

This is only needed for level-triggered interrupts, so it can be placed
below the test.

Paolo

> +            if (((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) !=
> +                IOAPIC_TRIGGER_LEVEL) {
>                  continue;
Peter Xu Feb. 28, 2020, 2:58 p.m. UTC | #2
On Fri, Feb 28, 2020 at 11:34:46AM +0100, Paolo Bonzini wrote:
> On 26/02/20 23:55, Peter Xu wrote:
> > +
> > +            /* Kick resamplefd if KVM is bypassed */
> > +            kvm_resample_fd_notify(n);
> 
> This is only needed for level-triggered interrupts, so it can be placed
> below the test.

Yes I was thinking it the same way, however...  I noticed that VFIO
will even mask edge INTx after getting IRQ (vfio_intx_handler).  And
if look into current KVM kernel implementation, it has done the same
logic to ack edge triggerred irqs (kvm_ioapic_update_eoi_one), we
should logically ack it here too even for edge-triggered?

Thanks,

> 
> Paolo
> 
> > +            if (((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) !=
> > +                IOAPIC_TRIGGER_LEVEL) {
> >                  continue;
>
Paolo Bonzini Feb. 28, 2020, 3:24 p.m. UTC | #3
On 28/02/20 15:58, Peter Xu wrote:
> Yes I was thinking it the same way, however...  I noticed that VFIO
> will even mask edge INTx after getting IRQ (vfio_intx_handler).

INTx is always level-triggered so that's okay.

> And if look into current KVM kernel implementation, it has done the
> same logic to ack edge triggerred irqs (kvm_ioapic_update_eoi_one),
> we should logically ack it here too even for edge-triggered?

Right, that's because it's using ack notifiers also for in-kernel PIT
and for RTC.  But yeah, it makes sense to have userspace IOAPIC
resamplefd match the functionality of the kernel ones.  Thanks!

Paolo
diff mbox series

Patch

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index d49b74512a..c7a863552b 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -159,9 +159,62 @@  static const KVMCapabilityInfo kvm_required_capabilites[] = {
 static NotifierList kvm_irqchip_change_notifiers =
     NOTIFIER_LIST_INITIALIZER(kvm_irqchip_change_notifiers);
 
+struct KVMResampleFd {
+    int gsi;
+    EventNotifier *resample_event;
+    QLIST_ENTRY(KVMResampleFd) node;
+};
+typedef struct KVMResampleFd KVMResampleFd;
+
+/*
+ * Only used with split irqchip where we need to do the resample fd
+ * kick for the kernel from userspace.
+ */
+static QLIST_HEAD(, KVMResampleFd) kvm_resample_fd_list =
+    QLIST_HEAD_INITIALIZER(kvm_resample_fd_list);
+
 #define kvm_slots_lock(kml)      qemu_mutex_lock(&(kml)->slots_lock)
 #define kvm_slots_unlock(kml)    qemu_mutex_unlock(&(kml)->slots_lock)
 
+static inline void kvm_resample_fd_remove(int gsi)
+{
+    KVMResampleFd *rfd;
+
+    QLIST_FOREACH(rfd, &kvm_resample_fd_list, node) {
+        if (rfd->gsi == gsi) {
+            QLIST_REMOVE(rfd, node);
+            break;
+        }
+    }
+}
+
+static inline void kvm_resample_fd_insert(int gsi, EventNotifier *event)
+{
+    KVMResampleFd *rfd = g_new0(KVMResampleFd, 1);
+
+    rfd->gsi = gsi;
+    rfd->resample_event = event;
+
+    QLIST_INSERT_HEAD(&kvm_resample_fd_list, rfd, node);
+}
+
+void kvm_resample_fd_notify(int gsi)
+{
+    KVMResampleFd *rfd;
+
+    if (!kvm_irqchip_is_split()) {
+        return;
+    }
+
+    QLIST_FOREACH(rfd, &kvm_resample_fd_list, node) {
+        if (rfd->gsi == gsi) {
+            event_notifier_set(rfd->resample_event);
+            trace_kvm_resample_fd_notify(gsi);
+            break;
+        }
+    }
+}
+
 int kvm_get_max_memslots(void)
 {
     KVMState *s = KVM_STATE(current_accel());
@@ -1644,6 +1697,20 @@  static int kvm_irqchip_assign_irqfd(KVMState *s, EventNotifier *event,
     if (rfd != -1) {
         irqfd.flags |= KVM_IRQFD_FLAG_RESAMPLE;
         irqfd.resamplefd = rfd;
+
+        /*
+         * When the slow irqchip (e.g. IOAPIC) is in the userspace,
+         * resamplefd will not work because the EOI of the level
+         * triggered interrupt will be delivered to userspace
+         * instead.  The userspace needs to remember the resamplefd
+         * too and kick it when we receive EOI of this IRQ.
+         */
+        assert(assign);
+        kvm_resample_fd_insert(virq, resample);
+    }
+
+    if (!assign) {
+        kvm_resample_fd_remove(virq);
     }
 
     if (!kvm_irqfds_enabled()) {
diff --git a/accel/kvm/trace-events b/accel/kvm/trace-events
index 4fb6e59d19..a68eb66534 100644
--- a/accel/kvm/trace-events
+++ b/accel/kvm/trace-events
@@ -16,4 +16,5 @@  kvm_set_ioeventfd_mmio(int fd, uint64_t addr, uint32_t val, bool assign, uint32_
 kvm_set_ioeventfd_pio(int fd, uint16_t addr, uint32_t val, bool assign, uint32_t size, bool datamatch) "fd: %d @0x%x val=0x%x assign: %d size: %d match: %d"
 kvm_set_user_memory(uint32_t slot, uint32_t flags, uint64_t guest_phys_addr, uint64_t memory_size, uint64_t userspace_addr, int ret) "Slot#%d flags=0x%x gpa=0x%"PRIx64 " size=0x%"PRIx64 " ua=0x%"PRIx64 " ret=%d"
 kvm_clear_dirty_log(uint32_t slot, uint64_t start, uint32_t size) "slot#%"PRId32" start 0x%"PRIx64" size 0x%"PRIx32
+kvm_resample_fd_notify(int gsi) "gsi %d"
 
diff --git a/hw/intc/ioapic.c b/hw/intc/ioapic.c
index 15747fe2c2..8c75465c62 100644
--- a/hw/intc/ioapic.c
+++ b/hw/intc/ioapic.c
@@ -236,8 +236,15 @@  void ioapic_eoi_broadcast(int vector)
         for (n = 0; n < IOAPIC_NUM_PINS; n++) {
             entry = s->ioredtbl[n];
 
-            if ((entry & IOAPIC_VECTOR_MASK) != vector ||
-                ((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) != IOAPIC_TRIGGER_LEVEL) {
+            if ((entry & IOAPIC_VECTOR_MASK) != vector) {
+                continue;
+            }
+
+            /* Kick resamplefd if KVM is bypassed */
+            kvm_resample_fd_notify(n);
+
+            if (((entry >> IOAPIC_LVT_TRIGGER_MODE_SHIFT) & 1) !=
+                IOAPIC_TRIGGER_LEVEL) {
                 continue;
             }
 
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 141342de98..b67552c047 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -555,4 +555,8 @@  int kvm_set_one_reg(CPUState *cs, uint64_t id, void *source);
 int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target);
 struct ppc_radix_page_info *kvm_get_radix_page_info(void);
 int kvm_get_max_memslots(void);
+
+/* Notify resamplefd for EOI of specific level triggered interrupts */
+void kvm_resample_fd_notify(int gsi);
+
 #endif