Re: [PATCH v11 00/20] VFIO support for platform and ARM AMBA devices

[Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>]

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
>> <a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org
>> <mailto:a.motakis-lrHrjnjw1UfHK3s98zE1ajGjJy/sRE9J@public.gmane.org>> wrote:
>>
>>     On Fri, Jan 9, 2015 at 9:39 AM, Eric Auger <eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org
>>     <mailto:eric.auger-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>> 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:
>>     >> git-9UaJU3cA/F/QT0dZR+AlfA@public.gmane.org: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
>>     >>
>>     >
>>
>>
>