From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751212AbdEaSVC (ORCPT ); Wed, 31 May 2017 14:21:02 -0400 Received: from mx1.redhat.com ([209.132.183.28]:55828 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751009AbdEaSVA (ORCPT ); Wed, 31 May 2017 14:21:00 -0400 DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com CEAFC3A58DA Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=alex.williamson@redhat.com DKIM-Filter: OpenDKIM Filter v2.11.0 mx1.redhat.com CEAFC3A58DA Date: Wed, 31 May 2017 12:20:39 -0600 From: Alex Williamson To: Eric Auger Cc: eric.auger.pro@gmail.com, linux-kernel@vger.kernel.org, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, pbonzini@redhat.com, marc.zyngier@arm.com, christoffer.dall@linaro.org, drjones@redhat.com, wei@redhat.com Subject: Re: [PATCH 02/10] VFIO: platform: Introduce direct EOI interrupt handler Message-ID: <20170531122039.0b41d5b7@w520.home> In-Reply-To: <1495656803-28011-3-git-send-email-eric.auger@redhat.com> References: <1495656803-28011-1-git-send-email-eric.auger@redhat.com> <1495656803-28011-3-git-send-email-eric.auger@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Wed, 31 May 2017 18:20:50 +0000 (UTC) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 24 May 2017 22:13:15 +0200 Eric Auger wrote: > We add two new fields in vfio_platform_irq: deoi and handler. > > If deoi is set, this means the physical IRQ attached to the virtual > IRQ is directly deactivated by the guest and the VFIO driver does > not need to disable the physical IRQ and mask it at VFIO level. > > The handler pointer points to either the automasked or "deoi" handler. > This latter is the same as the one used for edge sensitive IRQs. > A wrapper handler is introduced that calls the chosen handler function. > > The irq lock is taken/released in the wrapper handler. eventfd_signal > can be called in regions not allowed to sleep. > > Signed-off-by: Eric Auger > > --- > --- > drivers/vfio/platform/vfio_platform_irq.c | 24 +++++++++++++++++------- > drivers/vfio/platform/vfio_platform_private.h | 2 ++ > 2 files changed, 19 insertions(+), 7 deletions(-) > > diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c > index 831f0b0..2f82459 100644 > --- a/drivers/vfio/platform/vfio_platform_irq.c > +++ b/drivers/vfio/platform/vfio_platform_irq.c > @@ -148,11 +148,8 @@ static int vfio_platform_set_irq_unmask(struct vfio_platform_device *vdev, > static irqreturn_t vfio_automasked_irq_handler(int irq, void *dev_id) > { > struct vfio_platform_irq *irq_ctx = dev_id; > - unsigned long flags; > int ret = IRQ_NONE; > > - spin_lock_irqsave(&irq_ctx->lock, flags); > - > if (!irq_ctx->masked && !irq_ctx->automasked) { > ret = IRQ_HANDLED; > > @@ -161,8 +158,6 @@ static irqreturn_t vfio_automasked_irq_handler(int irq, void *dev_id) > irq_ctx->automasked = true; > } > > - spin_unlock_irqrestore(&irq_ctx->lock, flags); > - > if (ret == IRQ_HANDLED) > eventfd_signal(irq_ctx->trigger, 1); > > @@ -178,6 +173,19 @@ static irqreturn_t vfio_irq_handler(int irq, void *dev_id) > return IRQ_HANDLED; > } > > +static irqreturn_t vfio_wrapper_handler(int irq, void *dev_id) > +{ > + struct vfio_platform_irq *irq_ctx = dev_id; > + unsigned long flags; > + irqreturn_t ret; > + > + spin_lock_irqsave(&irq_ctx->lock, flags); > + ret = irq_ctx->handler(irq, dev_id); > + spin_unlock_irqrestore(&irq_ctx->lock, flags); > + > + return ret; > +} > + > static int vfio_set_trigger(struct vfio_platform_device *vdev, int index, > int fd, irq_handler_t handler) > { > @@ -208,9 +216,10 @@ static int vfio_set_trigger(struct vfio_platform_device *vdev, int index, > } > > irq->trigger = trigger; > + irq->handler = handler; > > irq_set_status_flags(irq->hwirq, IRQ_NOAUTOEN); > - ret = request_irq(irq->hwirq, handler, 0, irq->name, irq); > + ret = request_irq(irq->hwirq, vfio_wrapper_handler, 0, irq->name, irq); Noting here that vfio-platform already requests an exclusive interrupt (flags = 0), which I think is something that's going to need work for vfio-pci since we'll request a shared interrupt by default if the device supports DisINTx masking. If we're doing this special trick at the GIC, we'll starve other devices if they share an interrupt. > if (ret) { > kfree(irq->name); > eventfd_ctx_put(trigger); > @@ -232,7 +241,8 @@ static int vfio_platform_set_irq_trigger(struct vfio_platform_device *vdev, > struct vfio_platform_irq *irq = &vdev->irqs[index]; > irq_handler_t handler; > > - if (vdev->irqs[index].flags & VFIO_IRQ_INFO_AUTOMASKED) > + if (vdev->irqs[index].flags & VFIO_IRQ_INFO_AUTOMASKED && > + !irq->deoi) nit, strange linewrap > handler = vfio_automasked_irq_handler; > else > handler = vfio_irq_handler; > diff --git a/drivers/vfio/platform/vfio_platform_private.h b/drivers/vfio/platform/vfio_platform_private.h > index 8a3cfa9..b80a380 100644 > --- a/drivers/vfio/platform/vfio_platform_private.h > +++ b/drivers/vfio/platform/vfio_platform_private.h > @@ -38,6 +38,8 @@ struct vfio_platform_irq { > spinlock_t lock; > struct virqfd *unmask; > struct virqfd *mask; > + bool deoi; > + irqreturn_t (*handler)(int irq, void *dev_id); > }; > > struct vfio_platform_region {