On Tue, 30 May 2017 14:45:54 +0200 Auger Eric <[email protected]> wrote:
> Hi Marc, > > On 25/05/2017 20:05, Marc Zyngier wrote: > > Hi Eric, > > > > On Wed, May 24 2017 at 10:13:14 pm BST, Eric Auger <[email protected]> > > wrote: > >> For direct EOI modality we will need to differentiate a userspace > >> masking from the IRQ handler auto-masking. > >> > >> Signed-off-by: Eric Auger <[email protected]> > >> --- > >> drivers/vfio/platform/vfio_platform_irq.c | 10 ++++++---- > >> drivers/vfio/platform/vfio_platform_private.h | 1 + > >> 2 files changed, 7 insertions(+), 4 deletions(-) > >> > >> diff --git a/drivers/vfio/platform/vfio_platform_irq.c > >> b/drivers/vfio/platform/vfio_platform_irq.c > >> index 46d4750..831f0b0 100644 > >> --- a/drivers/vfio/platform/vfio_platform_irq.c > >> +++ b/drivers/vfio/platform/vfio_platform_irq.c > >> @@ -29,7 +29,7 @@ static void vfio_platform_mask(struct vfio_platform_irq > >> *irq_ctx) > >> > >> spin_lock_irqsave(&irq_ctx->lock, flags); > >> > >> - if (!irq_ctx->masked) { > >> + if (!irq_ctx->masked && !irq_ctx->automasked) { > > > > Could you please expand a bit on what this automasked variable covers? > > It'd be good to document how masked and automasked differ in behaviour. > > Yes sure. So automasked is set by the physical IRQ handler only, for > level sensitive IRQ (AUTOMASKED interrupts). masked is set through the > userspace API (VFIO_DEVICE_SET_IRQS and ACTION_MASK) when masking the > IRQ. VFIO ACTION_UNMASK resets both. This would make more sense if you at the same time renamed 'masked' to 'usermasked'. Thanks, Alex > > > > Also, it may be worth having a helper (is_masked?) to abstract both > > cases. > > Sure > > Eric > > > >> disable_irq_nosync(irq_ctx->hwirq); > >> irq_ctx->masked = true; > >> } > >> @@ -89,9 +89,10 @@ static void vfio_platform_unmask(struct > >> vfio_platform_irq *irq_ctx) > >> > >> spin_lock_irqsave(&irq_ctx->lock, flags); > >> > >> - if (irq_ctx->masked) { > >> + if (irq_ctx->masked || irq_ctx->automasked) { > >> enable_irq(irq_ctx->hwirq); > >> irq_ctx->masked = false; > >> + irq_ctx->automasked = false; > >> } > >> > >> spin_unlock_irqrestore(&irq_ctx->lock, flags); > >> @@ -152,12 +153,12 @@ static irqreturn_t vfio_automasked_irq_handler(int > >> irq, void *dev_id) > >> > >> spin_lock_irqsave(&irq_ctx->lock, flags); > >> > >> - if (!irq_ctx->masked) { > >> + if (!irq_ctx->masked && !irq_ctx->automasked) { > >> ret = IRQ_HANDLED; > >> > >> /* automask maskable interrupts */ > >> disable_irq_nosync(irq_ctx->hwirq); > >> - irq_ctx->masked = true; > >> + irq_ctx->automasked = true; > >> } > >> > >> spin_unlock_irqrestore(&irq_ctx->lock, flags); > >> @@ -315,6 +316,7 @@ int vfio_platform_irq_init(struct vfio_platform_device > >> *vdev) > >> vdev->irqs[i].count = 1; > >> vdev->irqs[i].hwirq = hwirq; > >> vdev->irqs[i].masked = false; > >> + vdev->irqs[i].automasked = false; > >> } > >> > >> vdev->num_irqs = cnt; > >> diff --git a/drivers/vfio/platform/vfio_platform_private.h > >> b/drivers/vfio/platform/vfio_platform_private.h > >> index 85ffe5d..8a3cfa9 100644 > >> --- a/drivers/vfio/platform/vfio_platform_private.h > >> +++ b/drivers/vfio/platform/vfio_platform_private.h > >> @@ -34,6 +34,7 @@ struct vfio_platform_irq { > >> char *name; > >> struct eventfd_ctx *trigger; > >> bool masked; > >> + bool automasked; > >> spinlock_t lock; > >> struct virqfd *unmask; > >> struct virqfd *mask; > > > > Thanks, > > > > M. > >
