Re: [RFC PATCH 11/19] vfio/pci: Serialize runtime PM with recovery

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing `synchronize_irq()` in runtime suspend and missing `pm_runtime_get_if_active()` in shared INTx interrupt handler.
--

commit ff5cf80e830fcc9b546457ca5f098d220807ca16
Author: Shameer Kolothum <skolothumtho@nvidia.com>

vfio/pci: Serialize runtime PM with recovery

This commit serializes runtime PM entry and exit with PCI error recovery by
holding the recovery_lock for reading. It ensures that BAR mappings and
DMA-BUF exports are handled safely without waking the device unnecessarily.

> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index bd3d79d28f27a..95884e713a4bd 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but it persists through this patch and seems
important since it can cause an AER panic and host crash.

The runtime suspend callback masks the INTx interrupt but fails to call
synchronize_irq() to wait for any executing handlers. In addition, the
interrupt handler accesses the PCI config space without verifying the device
is active.

In drivers/vfio/pci/vfio_pci_core.c:vfio_pci_core_runtime_suspend():

    vdev->pm_intx_masked = ((vdev->irq_type == VFIO_PCI_INTX_IRQ_INDEX) &&
                            vfio_pci_intx_mask(vdev));

If the VFIO device is runtime suspended while sharing an INTx IRQ line
(IRQF_SHARED), and another device on the shared line triggers an interrupt:

In drivers/vfio/pci/vfio_pci_intrs.c:vfio_intx_handler():

    } else if (!ctx->masked &&  /* may be shared */
               pci_check_and_mask_intx(vdev->pdev)) {

The handler will execute without a pm_runtime_get_if_active() check and
read the PCI config space of the D3cold device. This causes a PCIe Unsupported
Request (UR) error which can crash the host system. The read returns 0xffffffff,
which the handler interprets as a pending interrupt, returning IRQ_HANDLED and
signaling a spurious interrupt to the userspace VM.

Could synchronize_irq() be added to the suspend path, and
pm_runtime_get_if_active() to the interrupt handler to prevent accessing a
powered-off device?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901093217.8539-1-skolothumtho@nvidia.com?part=11