diff mbox series

[kvmtool,v1,1/2] vfio-pci: Release INTx's guest to host eventfd properly

Message ID 20190315083315.19221-1-leo.yan@linaro.org (mailing list archive)
State New, archived
Headers show
Series [kvmtool,v1,1/2] vfio-pci: Release INTx's guest to host eventfd properly | expand

Commit Message

Leo Yan March 15, 2019, 8:33 a.m. UTC
The PCI device INTx uses event fd 'unmask_fd' to signal the deassertion
of the line from guest to host; but this eventfd isn't released properly
when disable INTx.

When disable INTx this patch firstly unbinds interrupt signal by calling
ioctl VFIO_DEVICE_SET_IRQS and then it uses the new added field
'unmask_fd' in struct vfio_pci_device to close event fd.

Signed-off-by: Leo Yan <leo.yan@linaro.org>
---
 include/kvm/vfio.h |  1 +
 vfio/pci.c         | 15 ++++++++++++---
 2 files changed, 13 insertions(+), 3 deletions(-)

Comments

Jean-Philippe Brucker March 15, 2019, 2:17 p.m. UTC | #1
Hi,

On 15/03/2019 08:33, Leo Yan wrote:
> The PCI device INTx uses event fd 'unmask_fd' to signal the deassertion
> of the line from guest to host; but this eventfd isn't released properly
> when disable INTx.
> 
> When disable INTx this patch firstly unbinds interrupt signal by calling
> ioctl VFIO_DEVICE_SET_IRQS and then it uses the new added field
> 'unmask_fd' in struct vfio_pci_device to close event fd.
> 
> Signed-off-by: Leo Yan <leo.yan@linaro.org>
> ---
>  include/kvm/vfio.h |  1 +
>  vfio/pci.c         | 15 ++++++++++++---
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/include/kvm/vfio.h b/include/kvm/vfio.h
> index 60e6c54..28223cf 100644
> --- a/include/kvm/vfio.h
> +++ b/include/kvm/vfio.h
> @@ -74,6 +74,7 @@ struct vfio_pci_device {
>  
>  	unsigned long			irq_modes;
>  	int				intx_fd;
> +	int				unmask_fd;
>  	unsigned int			intx_gsi;
>  	struct vfio_pci_msi_common	msi;
>  	struct vfio_pci_msi_common	msix;
> diff --git a/vfio/pci.c b/vfio/pci.c
> index 03de3c1..c0683f6 100644
> --- a/vfio/pci.c
> +++ b/vfio/pci.c
> @@ -996,18 +996,26 @@ static void vfio_pci_disable_intx(struct kvm *kvm, struct vfio_device *vdev)
>  {
>  	struct vfio_pci_device *pdev = &vdev->pci;
>  	int gsi = pdev->intx_gsi;
> -	struct vfio_irq_set irq_set = {
> -		.argsz	= sizeof(irq_set),
> +	struct vfio_irq_set trigger_irq = {
> +		.argsz	= sizeof(trigger_irq),
>  		.flags	= VFIO_IRQ_SET_DATA_NONE | VFIO_IRQ_SET_ACTION_TRIGGER,
>  		.index	= VFIO_PCI_INTX_IRQ_INDEX,
>  	};
>  
> +	struct vfio_irq_set unmask_irq = {
> +		.argsz	= sizeof(unmask_irq),
> +		.flags	= VFIO_IRQ_SET_DATA_NONE | VFIO_IRQ_SET_ACTION_UNMASK,
> +		.index	= VFIO_PCI_INTX_IRQ_INDEX,
> +	};
> +
>  	pr_debug("user requested MSI, disabling INTx %d", gsi);
>  
> -	ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, &irq_set);
> +	ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, &trigger_irq);
> +	ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, &unmask_irq);

The patch makes sense, we do need to close unmask_fd, but I don't think
we need the additional ioctl. VFIO removes the unmask trigger when we
disable INTx in the first ioctl, so an additional ioctl to remove the
unmask trigger will return EINVAL.

Thanks,
Jean

>  	irq__del_irqfd(kvm, gsi, pdev->intx_fd);
>  
>  	close(pdev->intx_fd);
> +	close(pdev->unmask_fd);
>  }
>  
>  static int vfio_pci_enable_intx(struct kvm *kvm, struct vfio_device *vdev)
> @@ -1095,6 +1103,7 @@ static int vfio_pci_enable_intx(struct kvm *kvm, struct vfio_device *vdev)
>  	}
>  
>  	pdev->intx_fd = trigger_fd;
> +	pdev->unmask_fd = unmask_fd;
>  	/* Guest is going to ovewrite our irq_line... */
>  	pdev->intx_gsi = gsi;
>  
>
diff mbox series

Patch

diff --git a/include/kvm/vfio.h b/include/kvm/vfio.h
index 60e6c54..28223cf 100644
--- a/include/kvm/vfio.h
+++ b/include/kvm/vfio.h
@@ -74,6 +74,7 @@  struct vfio_pci_device {
 
 	unsigned long			irq_modes;
 	int				intx_fd;
+	int				unmask_fd;
 	unsigned int			intx_gsi;
 	struct vfio_pci_msi_common	msi;
 	struct vfio_pci_msi_common	msix;
diff --git a/vfio/pci.c b/vfio/pci.c
index 03de3c1..c0683f6 100644
--- a/vfio/pci.c
+++ b/vfio/pci.c
@@ -996,18 +996,26 @@  static void vfio_pci_disable_intx(struct kvm *kvm, struct vfio_device *vdev)
 {
 	struct vfio_pci_device *pdev = &vdev->pci;
 	int gsi = pdev->intx_gsi;
-	struct vfio_irq_set irq_set = {
-		.argsz	= sizeof(irq_set),
+	struct vfio_irq_set trigger_irq = {
+		.argsz	= sizeof(trigger_irq),
 		.flags	= VFIO_IRQ_SET_DATA_NONE | VFIO_IRQ_SET_ACTION_TRIGGER,
 		.index	= VFIO_PCI_INTX_IRQ_INDEX,
 	};
 
+	struct vfio_irq_set unmask_irq = {
+		.argsz	= sizeof(unmask_irq),
+		.flags	= VFIO_IRQ_SET_DATA_NONE | VFIO_IRQ_SET_ACTION_UNMASK,
+		.index	= VFIO_PCI_INTX_IRQ_INDEX,
+	};
+
 	pr_debug("user requested MSI, disabling INTx %d", gsi);
 
-	ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, &irq_set);
+	ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, &trigger_irq);
+	ioctl(vdev->fd, VFIO_DEVICE_SET_IRQS, &unmask_irq);
 	irq__del_irqfd(kvm, gsi, pdev->intx_fd);
 
 	close(pdev->intx_fd);
+	close(pdev->unmask_fd);
 }
 
 static int vfio_pci_enable_intx(struct kvm *kvm, struct vfio_device *vdev)
@@ -1095,6 +1103,7 @@  static int vfio_pci_enable_intx(struct kvm *kvm, struct vfio_device *vdev)
 	}
 
 	pdev->intx_fd = trigger_fd;
+	pdev->unmask_fd = unmask_fd;
 	/* Guest is going to ovewrite our irq_line... */
 	pdev->intx_gsi = gsi;