Hi Eric,
Thanks for taking time about this issue. I agree with you, there is a
problem here. While I think on a better fix and to be sure the problem is
here, may you try this patch and tell me if the problem is solved ? (This
should work as the automasked_irq_handler doesn't do anything if the IRQ is
masked).
If you have a suggestion on a fix, you're welcome :)
diff --git a/drivers/vfio/platform/vfio_platform_irq.c
b/drivers/vfio/platform/vfio_platform_irq.c
index 6ade36b..c9bac80 100644
--- a/drivers/vfio/platform/vfio_platform_irq.c
+++ b/drivers/vfio/platform/vfio_platform_irq.c
@@ -184,6 +184,7 @@ static int vfio_set_trigger(struct vfio_platform_device
*vdev, int index,
struct eventfd_ctx *trigger;
unsigned long flags;
int ret;
+ bool masked;
if (irq->trigger) {
free_irq(irq->hwirq, irq);
@@ -208,6 +209,8 @@ static int vfio_set_trigger(struct vfio_platform_device
*vdev, int index,
irq->trigger = trigger;
+ masked = irq->masked;
+
ret = request_irq(irq->hwirq, handler, 0, irq->name, irq);
if (ret) {
kfree(irq->name);
@@ -219,7 +222,7 @@ static int vfio_set_trigger(struct vfio_platform_device
*vdev, int index,
/* if the IRQ has been masked by the user before setting an eventfd,
* then we need to make sure it is properly disabled */
spin_lock_irqsave(&irq->lock, flags);
- if (irq->masked)
+ if (masked)
disable_irq_nosync(irq->hwirq);
spin_unlock_irqrestore(&irq->lock, flags);
On Mon, Jan 19, 2015 at 1:27 PM, Eric Auger <[email protected]> wrote:
> Hi Baptiste,
>
> I think what happens on the second qemu run is:
>
> an IRQ hits immediatly after request_irq
> automasked handler sets masked = true
> in vfio_set_trigger following condition becomes true
> if (irq->masked)
> disable_irq_nosync(irq->hwirq);
> IRQ is disabled twice, in handler and in vfio_set_trigger while there is
> a single enable in resamplefd handler.
> To me you should prevent from entering ISR between request_irq and
> disable_irq.
>
> It does not happen on the first run because no IRQ hit.
>
> Please let me know if you share this understanding.
>
> Best Regards
>
> Eric
>
> On 01/19/2015 11:17 AM, Eric Auger wrote:
> > Hi Baptiste,
> >
> > sorry I was off on Friday. you're right I missed the masked field was
> > reset on init. Nethertheless with current QEMU VFIO code, IRQ runs on
> > the first run and not on the second one. I investigate on my side ...
> >
> > Best Regards
> >
> > Eric
> >
> >
> >
> >
> > On 01/16/2015 02:25 PM, Baptiste Reynal wrote:
> >> Hello Eric,
> >>
> >> I'm not sure I understand the issue here. I tried to reproduce the bug
> >> by triggering an interrupt without unmasking it, but the interrupt is
> >> unmasked when the program access to the device (vfio_platform_open
> >> reinit IRQs).
> >>
> >> May I have more details on the bug ?
> >>
> >> Thanks
> >>
> >> On Fri, Jan 9, 2015 at 12:33 PM, Antonios Motakis
> >> <[email protected]
> >> <mailto:[email protected]>> wrote:
> >>
> >> On Fri, Jan 9, 2015 at 9:39 AM, Eric Auger <[email protected]
> >> <mailto:[email protected]>> wrote:
> >> > Hi Antonios,
> >> >
> >> > when moving to 3.19rc3 I observe a regression with my xgmac use
> case
> >> > (real-time change?).
> >> >
> >> > I guess what happens is when I kill a first qemu session, guest
> does not
> >> > have time to complete the virtual IRQ and the unmask is not
> performed by
> >> > the virqfd handler. When starting a new QEMU session, the irqfd
> >> > signalling is put in place again but since the masked field is
> set, the
> >> > IRQ is left unmasked (v11 modification in vfio_set_trigger).
> >> >
> >> > The problem is that we don't discriminate between user mask
> action and
> >> > automasked handler action. In case the user did not mask, I think
> we
> >> > should reset the masked field when doing the free_irq. What do
> you think?
> >>
> >> Hello Eric,
> >>
> >> I agree, we should reset the masked field for future users.
> >>
> >> Best regards
> >> Antonios
> >>
> >> >
> >> > Best Regards
> >> >
> >> > Eric
> >> >
> >> > On 01/06/2015 11:48 AM, Antonios Motakis wrote:
> >> >> This patch series aims to implement VFIO support for platform
> >> devices that
> >> >> reside behind an IOMMU. Examples of such devices are devices
> >> behind an ARM
> >> >> SMMU, or behind a Samsung Exynos System MMU.
> >> >>
> >> >> The API used is based on the existing VFIO API that is also used
> >> with PCI
> >> >> devices. Only devices that include a basic set of IRQs and memory
> >> regions are
> >> >> targeted; devices with complex relationships with other devices
> >> on a device
> >> >> tree are not taken into account at this stage.
> >> >>
> >> >> This patch series may be applied on the following series/patches:
> >> >> - [PATCH] driver core: amba: add device binding path
> >> 'driver_override'
> >> >> - [PATCH v3 0/6] vfio: type1: support for ARM SMMUS with
> >> VFIO_IOMMU_TYPE1
> >> >>
> >> >> A copy can be cloned from the branch vfio-platform-v11 at:
> >> >> [email protected]:virtualopensystems/linux-kvm-arm.git
> >> >>
> >> >> Changes since v10:
> >> >> - Check if interrupt is already masked when setting a new
> trigger
> >> >> - Fixed kasprintf with unchecked return value in VFIO AMBA
> driver
> >> >> Changes since v9:
> >> >> - Reworked the splitting of the patches that decouple virqfd
> >> from PCI
> >> >> - Some styling issues and typos
> >> >> - Removed superfluous includes
> >> >> - AMBA devices are now named vfio-amba- suffixed by the AMBA
> >> device id
> >> >> - Several other cleanups and fixes
> >> >> Changes since v8:
> >> >> - Separate irq handler for edge and level triggered interrupts
> >> >> - Mutex based lock for VFIO fd open/release
> >> >> - Fixed bug where the first region of a platform device wasn't
> >> exposed
> >> >> - Read only regions can be MMAPed only read only
> >> >> - Code cleanups
> >> >> Changes since v7:
> >> >> - Some initial placeholder functionality for PIO resources
> >> >> - Cleaned up code for IRQ triggering, masking and unmasking
> >> >> - Some functionality has been removed from this series and
> >> posted separately:
> >> >> - VFIO_IOMMU_TYPE1 support for ARM SMMUs
> >> >> - IOMMU NOEXEC patches
> >> >> - driver_override functionality for AMBA devices
> >> >> - Several fixes
> >> >> Changes since v6:
> >> >> - Integrated support for AMBA devices
> >> >> - Numerous cleanups and fixes
> >> >> Changes since v5:
> >> >> - Full eventfd support for IRQ masking and unmasking.
> >> >> - Changed IOMMU_EXEC to IOMMU_NOEXEC, along with related flags
> >> in VFIO.
> >> >> - Other fixes based on reviewer comments.
> >> >> Changes since v4:
> >> >> - Use static offsets for each region in the VFIO device fd
> >> >> - Include patch in the series for the ARM SMMU to expose
> IOMMU_EXEC
> >> >> availability via IOMMU_CAP_DMA_EXEC
> >> >> - Rebased on VFIO multi domain support:
> >> >> - IOMMU_EXEC is now available if at least one IOMMU in the
> >> container
> >> >> supports it
> >> >> - Expose IOMMU_EXEC if available via the capability
> >> VFIO_IOMMU_PROT_EXEC
> >> >> - Some bug fixes
> >> >> Changes since v3:
> >> >> - Use Kim Phillips' driver_probe_device()
> >> >> Changes since v2:
> >> >> - Fixed Read/Write and MMAP on device regions
> >> >> - Removed dependency on Device Tree
> >> >> - Interrupts support
> >> >> - Interrupt masking/unmasking
> >> >> - Automask level sensitive interrupts
> >> >> - Introduced VFIO_DMA_MAP_FLAG_EXEC
> >> >> - Code clean ups
> >> >>
> >> >> Antonios Motakis (20):
> >> >> vfio/platform: initial skeleton of VFIO support for platform
> >> devices
> >> >> vfio: platform: probe to devices on the platform bus
> >> >> vfio: platform: add the VFIO PLATFORM module to Kconfig
> >> >> vfio: amba: VFIO support for AMBA devices
> >> >> vfio: amba: add the VFIO for AMBA devices module to Kconfig
> >> >> vfio/platform: return info for bound device
> >> >> vfio/platform: return info for device memory mapped IO regions
> >> >> vfio/platform: read and write support for the device fd
> >> >> vfio/platform: support MMAP of MMIO regions
> >> >> vfio/platform: return IRQ info
> >> >> vfio/platform: initial interrupts support code
> >> >> vfio/platform: trigger an interrupt via eventfd
> >> >> vfio/platform: support for level sensitive interrupts
> >> >> vfio: add a vfio_ prefix to virqfd_enable and virqfd_disable
> and
> >> >> export
> >> >> vfio: virqfd: rename vfio_pci_virqfd_init and
> vfio_pci_virqfd_exit
> >> >> vfio: add local lock for virqfd instead of depending on VFIO
> PCI
> >> >> vfio: pass an opaque pointer on virqfd initialization
> >> >> vfio: move eventfd support code for VFIO_PCI to a separate file
> >> >> vfio: initialize the virqfd workqueue in VFIO generic code
> >> >> vfio/platform: implement IRQ masking/unmasking via an eventfd
> >> >>
> >> >> drivers/vfio/Kconfig | 1 +
> >> >> drivers/vfio/Makefile | 5 +-
> >> >> drivers/vfio/pci/vfio_pci.c | 8 -
> >> >> drivers/vfio/pci/vfio_pci_intrs.c | 238 +-----------
> >> >> drivers/vfio/pci/vfio_pci_private.h | 3 -
> >> >> drivers/vfio/platform/Kconfig | 19 +
> >> >> drivers/vfio/platform/Makefile | 8 +
> >> >> drivers/vfio/platform/vfio_amba.c | 115 ++++++
> >> >> drivers/vfio/platform/vfio_platform.c | 103 +++++
> >> >> drivers/vfio/platform/vfio_platform_common.c | 520
> >> ++++++++++++++++++++++++++
> >> >> drivers/vfio/platform/vfio_platform_irq.c | 340
> >> +++++++++++++++++
> >> >> drivers/vfio/platform/vfio_platform_private.h | 82 ++++
> >> >> drivers/vfio/vfio.c | 8 +
> >> >> drivers/vfio/virqfd.c | 213 +++++++++++
> >> >> include/linux/vfio.h | 27 ++
> >> >> include/uapi/linux/vfio.h | 2 +
> >> >> 16 files changed, 1456 insertions(+), 236 deletions(-)
> >> >> create mode 100644 drivers/vfio/platform/Kconfig
> >> >> create mode 100644 drivers/vfio/platform/Makefile
> >> >> create mode 100644 drivers/vfio/platform/vfio_amba.c
> >> >> create mode 100644 drivers/vfio/platform/vfio_platform.c
> >> >> create mode 100644 drivers/vfio/platform/vfio_platform_common.c
> >> >> create mode 100644 drivers/vfio/platform/vfio_platform_irq.c
> >> >> create mode 100644 drivers/vfio/platform/vfio_platform_private.h
> >> >> create mode 100644 drivers/vfio/virqfd.c
> >> >>
> >> >
> >>
> >>
> >
>
>
_______________________________________________
iommu mailing list
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
