Date:   Thu, 11 May 2023 08:44:27 -0700
From:   Reinette Chatre <reinette.chatre@...el.com>
To:     jgg@...dia.com, yishaih@...dia.com,
        shameerali.kolothum.thodi@...wei.com, kevin.tian@...el.com,
        alex.williamson@...hat.com
Cc:     tglx@...utronix.de, darwi@...utronix.de, kvm@...r.kernel.org,
        dave.jiang@...el.com, jing2.liu@...el.com, ashok.raj@...el.com,
        fenghua.yu@...el.com, tom.zanussi@...ux.intel.com,
        reinette.chatre@...el.com, linux-kernel@...r.kernel.org
Subject: [PATCH V5 00/11] vfio/pci: Support dynamic allocation of MSI-X interrupts

Changes since V4:
- V4: https://lore.kernel.org/lkml/cover.1682615447.git.reinette.chatre@intel.com/
- Add Kevin's Reviewed-by tag as applicable.
- Treat non-existing INTx interrupt context as kernel bug with WARN. This
  exposed an issue in the scenario where INTx mask/unmask may occur without
  INTx enabled. This is fixed by obtaining the interrupt context later
  (right before use) within impacted functions: vfio_pci_intx_mask() and
  vfio_pci_intx_unmask_handler(). (Kevin)
- Treat pci_irq_vector() returning '0' for a MSI/MSI-X interrupt as a kernel
  bug via a WARN instead of ignoring this value. (Kevin)
- Improve accuracy of comments. (Kevin)
- Please refer to individual patches for local changes.

Changes since V3:
- V3: https://lore.kernel.org/lkml/cover.1681837892.git.reinette.chatre@intel.com/
- Be considerate about layout and size with changes to
  struct vfio_pci_core_device. Keep flags together and transition all to
  use bitfields. (Alex and Jason)
- Do not free dynamically allocated interrupts on error path. (Alex)
- Please refer to individual patches for localized changes.

Changes since V2:
- V2: https://lore.kernel.org/lkml/cover.1680038771.git.reinette.chatre@intel.com/
- During testing of V2 "kernel test robot" reported issues resulting from
  include/linux/pci.h missing a stub for pci_msix_can_alloc_dyn() when
  CONFIG_PCI_MSI=n. A separate fix was sent to address this. The fix can
  be found in the kernel (since v6.3-rc7) as
  commit 195d8e5da3ac ("PCI/MSI: Provide missing stub for pci_msix_can_alloc_dyn()")
- Biggest change is the transition to "active contexts" for both MSI and MSI-X.
  Interrupt contexts have always been allocated when the interrupts are
  allocated while they are only used while interrupts are
  enabled. In this series interrupt contexts are made dynamic, while doing
  so their allocation is moved to match how they are used: allocated when
  interrupts are enabled. Whether a Linux interrupt number exists determines
  whether an interrupt can be enabled.
  Previous policy (up to V2) that an allocated interrupt has an interrupt
  context no longer applies. Instead, an interrupt context has a
  handler/trigger, aka "active contexts". (Alex)
- Re-ordered patches in support of "active contexts".
- Only free interrupts on MSI-X teardown and otherwise use the
  allocated interrupts as a cache. (Alex)
- Using unsigned int for the vector broke the unwind loop within
  vfio_msi_set_block(). (Alex)
- Introduce new "has_dyn_msix" property of virtual device instead of
  querying support every time. (Alex)
- Some smaller changes, please refer to individual patches.

Changes since RFC V1:
- RFC V1: https://lore.kernel.org/lkml/cover.1678911529.git.reinette.chatre@intel.com/
- Improved changelogs.
- Simplify interface so that vfio_irq_ctx_alloc_single() returns pointer to
  allocated context. (Alex)
- Remove vfio_irq_ctx_range_allocated() and associated attempts to maintain
  invalid error path behavior. (Alex and Kevin)
- Add pointer to interrupt context as function parameter to
  vfio_irq_ctx_free(). (Alex)
- Ensure variables are initialized. (Dan Carpenter)
- Only support dynamic allocation if device supports it. (Alex)

Qemu allocates interrupts incrementally at the time the guest unmasks an
interrupt, for example each time a Linux guest runs request_irq().

Dynamic allocation of MSI-X interrupts was not possible until v6.2 [1].
This prompted Qemu to, when allocating a new interrupt, first release all
previously allocated interrupts (including disable of MSI-X) followed
by re-allocation of all interrupts that includes the new interrupt.
Please see [2] for a detailed discussion about this issue.

Releasing and re-allocating interrupts may be acceptable if all
interrupts are unmasked during device initialization. If unmasking of
interrupts occur during runtime this may result in lost interrupts.
For example, consider an accelerator device with multiple work queues,
each work queue having a dedicated interrupt. A work queue can be
enabled at any time with its associated interrupt unmasked while other
work queues are already active. Having all interrupts released and MSI-X
disabled to enable the new work queue will impact active work queues.

This series builds on the recent interrupt sub-system core changes
that added support for dynamic MSI-X allocation after initial MSI-X
enabling.

Add support for dynamic MSI-X allocation to vfio-pci. A flag
indicating lack of support for dynamic allocation already exist:
VFIO_IRQ_INFO_NORESIZE and has always been set for MSI and MSI-X. With
support for dynamic MSI-X the flag is cleared for MSI-X when supported,
enabling Qemu to modify its behavior.

Any feedback is appreciated

Reinette

[1] commit 34026364df8e ("PCI/MSI: Provide post-enable dynamic allocation interfaces for MSI-X")
[2] https://lore.kernel.org/kvm/MWHPR11MB188603D0D809C1079F5817DC8C099@MWHPR11MB1886.namprd11.prod.outlook.com/#t

Reinette Chatre (11):
  vfio/pci: Consolidate irq cleanup on MSI/MSI-X disable
  vfio/pci: Remove negative check on unsigned vector
  vfio/pci: Prepare for dynamic interrupt context storage
  vfio/pci: Move to single error path
  vfio/pci: Use xarray for interrupt context storage
  vfio/pci: Remove interrupt context counter
  vfio/pci: Update stale comment
  vfio/pci: Use bitfield for struct vfio_pci_core_device flags
  vfio/pci: Probe and store ability to support dynamic MSI-X
  vfio/pci: Support dynamic MSI-X
  vfio/pci: Clear VFIO_IRQ_INFO_NORESIZE for MSI-X

 drivers/vfio/pci/vfio_pci_core.c  |   8 +-
 drivers/vfio/pci/vfio_pci_intrs.c | 305 ++++++++++++++++++++----------
 include/linux/vfio_pci_core.h     |  26 +--
 include/uapi/linux/vfio.h         |   3 +
 4 files changed, 229 insertions(+), 113 deletions(-)


base-commit: 457391b0380335d5e9a5babdec90ac53928b23b4
-- 
2.34.1

Powered by blists - more mailing lists