linux-kernel - Re: [PATCH 01/10] vfio: platform: Add automasked field to vfio_platform

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87fufslsmm.fsf@arm.com>
Date:   Thu, 25 May 2017 19:05:05 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     Eric Auger <eric.auger@...hat.com>
Cc:     <eric.auger.pro@...il.com>, <linux-kernel@...r.kernel.org>,
        <kvm@...r.kernel.org>, <kvmarm@...ts.cs.columbia.edu>,
        <alex.williamson@...hat.com>, <pbonzini@...hat.com>,
        <christoffer.dall@...aro.org>, <drjones@...hat.com>,
        <wei@...hat.com>
Subject: Re: [PATCH 01/10] vfio: platform: Add automasked field to vfio_platform_irq

Hi Eric,

On Wed, May 24 2017 at 10:13:14 pm BST, Eric Auger <eric.auger@...hat.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@...hat.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.