Hi Alex,
On 3/9/24 00:05, Alex Williamson wrote:
Currently for devices requiring masking at the irqchip for INTx, ie. devices without DisINTx support, the IRQ is enabled in request_irq() and subsequently disabled as necessary to align with the masked status flag. This presents a window where the interrupt could fire between these events, resulting in the IRQ incrementing the disable depth twice. This would be unrecoverable for a user since the masked flag prevents nested enables through vfio.
Instead, invert the logic using IRQF_NO_AUTOEN such that exclusive INTx is never auto-enabled, then unmask as required. Cc: stable@vger.kernel.org Fixes: 89e1f7d4c66d ("vfio: Add PCI device driver") Reviewed-by: Kevin Tian kevin.tian@intel.com Signed-off-by: Alex Williamson alex.williamson@redhat.com
drivers/vfio/pci/vfio_pci_intrs.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c index 237beac83809..136101179fcb 100644 --- a/drivers/vfio/pci/vfio_pci_intrs.c +++ b/drivers/vfio/pci/vfio_pci_intrs.c @@ -296,8 +296,15 @@ static int vfio_intx_set_signal(struct vfio_pci_core_device *vdev, int fd) ctx->trigger = trigger;
- /*
* Devices without DisINTx support require an exclusive interrupt,* IRQ masking is performed at the IRQ chip. The masked status is* protected by vdev->irqlock. Setup the IRQ without auto-enable and* unmask as necessary below under lock. DisINTx is unmodified by* the IRQ configuration and may therefore use auto-enable.
If I remember correctly the main reason why the
vdev->pci_2_3 path is left unchanged is due to the fact the irq may not be exclusive and setting IRQF_NO_AUTOEN could be wrong in that case. May be worth to precise in the commit msg or here? Besides Reviewed-by: Eric Auger eric.auger@redhat.com Eric
*/
irqflags = 0;
irqflags = IRQF_NO_AUTOEN;ret = request_irq(pdev->irq, vfio_intx_handler, irqflags, ctx->name, vdev); @@ -308,13 +315,9 @@ static int vfio_intx_set_signal(struct vfio_pci_core_device *vdev, int fd) return ret; }
- /*
* INTx disable will stick across the new irq setup,* disable_irq won't.*/- if (!vdev->pci_2_3 && ctx->masked)
disable_irq_nosync(pdev->irq);
- if (!vdev->pci_2_3 && !ctx->masked)
enable_irq(pdev->irq);return 0;