Re: [RFC PATCH v2 12/16] vfio/pci: Restore device state from slot_reset()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: rwsem ABBA/starvation deadlock in AER slot reset via pci_set_power_state
--

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

vfio/pci: Restore device state from slot_reset()
    
A host reset clears device configuration. Add slot_reset() to restore
the state saved at open before tearing down interrupts. MSI-X shutdown
requires the restored BARs to access its table.

> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 667c5813f6c7c..a7b7499e071c1 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
[ ... ]
> +/* Caller holds memory_lock. Discard the pre-reset PM snapshot. */
> +static int vfio_pci_recovery_restore_state(struct vfio_pci_core_device *vdev)
> +{
> +	struct pci_dev *pdev = vdev->pdev;
> +	int ret;
> +
> +	if (!vdev->pci_saved_state)
> +		return -ENODATA;
> +	ret = pci_load_saved_state(pdev, vdev->pci_saved_state);
> +	if (ret)
> +		return ret;
> +
> +	kfree(vdev->pm_save);
> +	vdev->pm_save = NULL;
> +	ret = pci_set_power_state(pdev, PCI_D0);

[Severity: High]
Can this introduce an ABBA starvation deadlock with pci_bus_sem?

During an AER slot reset, pcie_do_recovery() already holds a read lock on
pci_bus_sem via pci_walk_bus(). The AER framework then calls
vfio_pci_core_aer_slot_reset(), which calls into this function and
attempts to restore the device to D0 by calling pci_set_power_state().

Internally, this transitions ASPM via pcie_aspm_pm_state_change(), which
unconditionally executes a nested down_read(&pci_bus_sem) because the
locked parameter is passed as false:

drivers/pci/pcie/aspm.c:pcie_aspm_pm_state_change() {
    ...
    if (!locked)
        down_read(&pci_bus_sem);
    ...
}

If a concurrent thread, such as PCI hotplug or another subsystem, attempts
to acquire down_write(&pci_bus_sem) between these two read lock
acquisitions, the fair rwsem implementation will queue the writer. The
nested down_read() will then block waiting for the writer, creating an
unresolvable starvation ABBA deadlock.

As noted in the commit message, the final patches in this series do not
appear to address or resolve this bug. Are there plans to address this
deadlock before merging?

> +	if (ret)
> +		return ret;
> +
> +	vfio_pci_recovery_mask_saved_intx(vdev);
> +	pci_restore_state(pdev);
> +	return 0;
> +}

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