Re: [PATCH 01/10] vfio: platform: Add automasked field to vfio_platform_irq

Marc Zyngier
2017-05-25T18:01:31Z

Hi Eric,

On Wed, May 24 2017 at 10:13:14 pm BST, Eric Auger <eric.auger@redhat.com> wrote:
> For direct EOI modality we will need to differentiate a userspace
> masking from the IRQ handler auto-masking.
>
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> ---
> 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.

Also, it may be worth having a helper (is_masked?) to abstract both
cases.

> 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.
--
Jazz is not dead, it just smell funny.