From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:57284) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8SGj-00075x-Gi for qemu-devel@nongnu.org; Fri, 26 Jun 2015 07:58:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Z8SGg-0006da-5l for qemu-devel@nongnu.org; Fri, 26 Jun 2015 07:58:05 -0400 Received: from mail-vn0-f44.google.com ([209.85.216.44]:39770) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Z8SGg-0006dS-0X for qemu-devel@nongnu.org; Fri, 26 Jun 2015 07:58:02 -0400 Received: by vnav203 with SMTP id v203so15239107vna.6 for ; Fri, 26 Jun 2015 04:58:01 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1434386038-9246-7-git-send-email-eric.auger@linaro.org> References: <1434386038-9246-1-git-send-email-eric.auger@linaro.org> <1434386038-9246-7-git-send-email-eric.auger@linaro.org> From: Peter Maydell Date: Fri, 26 Jun 2015 12:57:42 +0100 Message-ID: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [RESEND PATCH v16 6/6] hw/vfio/platform: add irqfd support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Auger Cc: b.reynal@virtualopensystems.com, Peter Crosthwaite , eric.auger@st.com, vikrams@codeaurora.org, Patch Tracking , QEMU Developers , Alex Williamson , Paolo Bonzini , Christoffer Dall On 15 June 2015 at 17:33, Eric Auger wrote: > This patch aims at optimizing IRQ handling using irqfd framework. > > Instead of handling the eventfds on user-side they are handled on > kernel side using > - the KVM irqfd framework, > - the VFIO driver virqfd framework. > > the virtual IRQ completion is trapped at interrupt controller > This removes the need for fast/slow path swap. > > Overall this brings significant performance improvements. > > Signed-off-by: Alvise Rigo > Signed-off-by: Eric Auger > Reviewed-by: Alex Benn=C3=A9e > Tested-by: Vikram Sethi > > --- > v15 -> v16: > - add Vikram's T-b > > v13 -> v14: > - use connect_irq_notifier > - remove trace_vfio_platform_start_eventfd > > v12 -> v13: > - setup the new mechanism for starting irqfd, based on > LinkPropertySetter override > - use kvm_irqchip_[add,remove]_irqfd_notifier new functions: no need > to bother about gsi (hence virtualID could be removed with small > change in trace-events) > > v10 -> v11: > - Add Alex' Reviewed-by > - introduce kvm_accel in this patch and initialize it > > v5 -> v6 > - rely on kvm_irqfds_enabled() and kvm_resamplefds_enabled() > - guard KVM code with #ifdef CONFIG_KVM > > v3 -> v4: > [Alvise Rigo] > Use of VFIO Platform driver v6 unmask/virqfd feature and removal > of resamplefd handler. Physical IRQ unmasking is now done in > VFIO driver. > > v3: > [Eric Auger] > initial support with resamplefd handled on QEMU side since the > unmask was not supported on VFIO platform driver v5. > --- > hw/vfio/platform.c | 107 ++++++++++++++++++++++++++++++++++= ++++++ > include/hw/vfio/vfio-platform.h | 2 + > trace-events | 1 + > 3 files changed, 110 insertions(+) > > diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c > index 9382bb7..1d44085 100644 > --- a/hw/vfio/platform.c > +++ b/hw/vfio/platform.c > @@ -26,6 +26,7 @@ > #include "hw/sysbus.h" > #include "trace.h" > #include "hw/platform-bus.h" > +#include "sysemu/kvm.h" > > /* > * Functions used whatever the injection method > @@ -51,6 +52,7 @@ static VFIOINTp *vfio_init_intp(VFIODevice *vbasedev, > intp->pin =3D info.index; > intp->flags =3D info.flags; > intp->state =3D VFIO_IRQ_INACTIVE; > + intp->kvm_accel =3D false; > > sysbus_init_irq(sbdev, &intp->qemuirq); > > @@ -61,6 +63,13 @@ static VFIOINTp *vfio_init_intp(VFIODevice *vbasedev, > error_report("vfio: Error: trigger event_notifier_init failed ")= ; > return NULL; > } > + /* Get an eventfd for resample/unmask */ > + ret =3D event_notifier_init(&intp->unmask, 0); > + if (ret) { > + g_free(intp); > + error_report("vfio: Error: resample event_notifier_init failed e= oi"); This error message seems rather cryptic; what is it trying to tell the user= ? > + return NULL; > + } > > QLIST_INSERT_HEAD(&vdev->intp_list, intp, next); > return intp; > @@ -315,6 +324,95 @@ static int vfio_start_eventfd_injection(VFIOINTp *in= tp) > return ret; > } > > +/* > + * Functions used for irqfd > + */ > + > +#ifdef CONFIG_KVM > + > +/** > + * vfio_set_resample_eventfd - sets the resamplefd for an IRQ > + * @intp: the IRQ struct handle > + * programs the VFIO driver to unmask this IRQ when the > + * intp->unmask eventfd is triggered > + */ > +static int vfio_set_resample_eventfd(VFIOINTp *intp) > +{ > + VFIODevice *vbasedev =3D &intp->vdev->vbasedev; > + struct vfio_irq_set *irq_set; > + int argsz, ret; > + int32_t *pfd; > + > + argsz =3D sizeof(*irq_set) + sizeof(*pfd); > + irq_set =3D g_malloc0(argsz); > + irq_set->argsz =3D argsz; > + irq_set->flags =3D VFIO_IRQ_SET_DATA_EVENTFD | VFIO_IRQ_SET_ACTION_U= NMASK; > + irq_set->index =3D intp->pin; > + irq_set->start =3D 0; > + irq_set->count =3D 1; > + pfd =3D (int32_t *)&irq_set->data; > + *pfd =3D event_notifier_get_fd(&intp->unmask); > + qemu_set_fd_handler(*pfd, NULL, NULL, NULL); > + ret =3D ioctl(vbasedev->fd, VFIO_DEVICE_SET_IRQS, irq_set); > + g_free(irq_set); > + if (ret < 0) { > + error_report("vfio: Failed to set resample eventfd: %m"); > + } > + return ret; > +} > + > +static void vfio_start_irqfd_injection(SysBusDevice *sbdev, qemu_irq irq= ) > +{ > + VFIOPlatformDevice *vdev =3D VFIO_PLATFORM_DEVICE(sbdev); > + VFIOINTp *intp; > + bool found =3D false; > + > + QLIST_FOREACH(intp, &vdev->intp_list, next) { > + if (intp->qemuirq =3D=3D irq) { > + found =3D true; Stray double-space. > + break; > + } > + } > + assert(found); > + > + /* Get to a known interrupt state */ > + qemu_set_fd_handler(event_notifier_get_fd(&intp->interrupt), > + NULL, NULL, vdev); > + > + vfio_mask_single_irqindex(&vdev->vbasedev, intp->pin); > + qemu_set_irq(intp->qemuirq, 0); > + > + if (kvm_irqchip_add_irqfd_notifier(kvm_state, &intp->interrupt, > + &intp->unmask, irq) < 0) { > + goto fail_irqfd; > + } > + > + if (vfio_set_trigger_eventfd(intp, NULL) < 0) { > + goto fail_vfio; > + } > + if (vfio_set_resample_eventfd(intp) < 0) { > + goto fail_vfio; > + } > + > + /* Let'em rip */ This comment could probably be phrased more informatively :-) > + vfio_unmask_single_irqindex(&vdev->vbasedev, intp->pin); > + > + intp->kvm_accel =3D true; > + > + trace_vfio_platform_start_irqfd_injection(intp->pin, > + event_notifier_get_fd(&intp->interr= upt), > + event_notifier_get_fd(&intp->unmask= )); > + return; > +fail_vfio: > + kvm_irqchip_remove_irqfd_notifier(kvm_state, &intp->interrupt, irq); > +fail_irqfd: > + vfio_start_eventfd_injection(intp); > + vfio_unmask_single_irqindex(&vdev->vbasedev, intp->pin); > + return; > +} > + > +#endif /* CONFIG_KVM */ > + > /* VFIO skeleton */ > > static void vfio_platform_compute_needs_reset(VFIODevice *vbasedev) > @@ -548,6 +646,7 @@ static void vfio_platform_realize(DeviceState *dev, E= rror **errp) > { > VFIOPlatformDevice *vdev =3D VFIO_PLATFORM_DEVICE(dev); > SysBusDevice *sbdev =3D SYS_BUS_DEVICE(dev); > + SysBusDeviceClass *sbc =3D SYS_BUS_DEVICE_GET_CLASS(dev); > VFIODevice *vbasedev =3D &vdev->vbasedev; > VFIOINTp *intp; > int i, ret; > @@ -555,6 +654,13 @@ static void vfio_platform_realize(DeviceState *dev, = Error **errp) > vbasedev->type =3D VFIO_DEVICE_TYPE_PLATFORM; > vbasedev->ops =3D &vfio_platform_ops; > > +#ifdef CONFIG_KVM > + if (kvm_irqfds_enabled() && kvm_resamplefds_enabled() && > + vdev->irqfd_allowed) { > + sbc->connect_irq_notifier =3D vfio_start_irqfd_injection; > + } > +#endif You shouldn't need this CONFIG_KVM ifdef -- we deliberately provide "always false" versions of functions like kvm_irqfds_enabled() in sysemu/kvm.h so that we can avoid littering code with ifdefs. It looks like your commit f41389ae3c54b failed to provide the non-KVM version of kvm_resamplefds_enabled(), but the correct fix is to provide it... This implies removing the ifdefs earlier around vfio_start_irqfd_injection, but that should be OK -- we provide stub versions of functions like kvm_irqchip_add_irqfd_notifier() so the non-KVM code can still compile. > + > trace_vfio_platform_realize(vbasedev->name, vdev->compat); > > ret =3D vfio_base_device_init(vbasedev); > @@ -584,6 +690,7 @@ static Property vfio_platform_dev_properties[] =3D { > DEFINE_PROP_BOOL("x-mmap", VFIOPlatformDevice, vbasedev.allow_mmap, = true), > DEFINE_PROP_UINT32("mmap-timeout-ms", VFIOPlatformDevice, > mmap_timeout, 1100), > + DEFINE_PROP_BOOL("x-irqfd", VFIOPlatformDevice, irqfd_allowed, true)= , > DEFINE_PROP_END_OF_LIST(), > }; > > diff --git a/include/hw/vfio/vfio-platform.h b/include/hw/vfio/vfio-platf= orm.h > index 26b2ad6..c5cf1d7 100644 > --- a/include/hw/vfio/vfio-platform.h > +++ b/include/hw/vfio/vfio-platform.h > @@ -41,6 +41,7 @@ typedef struct VFIOINTp { > int state; /* inactive, pending, active */ > uint8_t pin; /* index */ > uint32_t flags; /* IRQ info flags */ > + bool kvm_accel; /* set when QEMU bypass through KVM enabled */ > } VFIOINTp; > > /* function type for user side eventfd handler */ > @@ -57,6 +58,7 @@ typedef struct VFIOPlatformDevice { > uint32_t mmap_timeout; /* delay to re-enable mmaps after interrupt *= / > QEMUTimer *mmap_timer; /* allows fast-path resume after IRQ hit */ > QemuMutex intp_mutex; /* protect the intp_list IRQ state */ > + bool irqfd_allowed; /* debug option to force irqfd on/off */ > } VFIOPlatformDevice; > > typedef struct VFIOPlatformDeviceClass { > diff --git a/trace-events b/trace-events > index 6060d36..4390381 100644 > --- a/trace-events > +++ b/trace-events > @@ -1594,6 +1594,7 @@ vfio_platform_intp_interrupt(int pin, int fd) "Inje= ct IRQ #%d (fd =3D %d)" > vfio_platform_intp_inject_pending_lockheld(int pin, int fd) "Inject pend= ing IRQ #%d (fd =3D %d)" > vfio_platform_populate_interrupts(int pin, int count, int flags) "- IRQ = index %d: count %d, flags=3D0x%x" > vfio_intp_interrupt_set_pending(int index) "irq %d is set PENDING" > +vfio_platform_start_irqfd_injection(int index, int fd, int resamplefd) "= IRQ index=3D%d, fd =3D %d, resamplefd =3D %d" > > #hw/acpi/memory_hotplug.c > mhp_acpi_invalid_slot_selected(uint32_t slot) "0x%"PRIx32 > -- > 1.8.3.2 > thanks -- PMM