On Tue, Apr 11, 2023 at 08:21:02AM +0200, Lukas Wunner wrote:
> In 2013, commits
>
> 2e35afaefe64 ("PCI: pciehp: Add reset_slot() method")
> 608c388122c7 ("PCI: Add slot reset option to pci_dev_reset()")
>
> amended PCIe hotplug to mask Presence Detect Changed events during a
> Secondary Bus Reset. The reset thus no longer causes gratuitous slot
> bringdown and bringup.
>
> However the commits neglected to serialize reset with code paths reading
> slot registers. For instance, a slot bringup due to an earlier hotplug
> event may see the Presence Detect State bit cleared during a concurrent
> Secondary Bus Reset.
> ...
> Signed-off-by: Lukas Wunner <lukas@xxxxxxxxx>
Applied to pci/hotplug for v6.4, thanks!
> Cc: stable@xxxxxxxxxxxxxxx # v4.19+
> Cc: Dan Stein <dstein@xxxxxxx>
> Cc: Ashok Raj <ashok.raj@xxxxxxxxx>
> Cc: Alex Michon <amichon@xxxxxxxxxxxxx>
> Cc: Xiongfeng Wang <wangxiongfeng2@xxxxxxxxxx>
> Cc: Alex Williamson <alex.williamson@xxxxxxxxxx>
> Cc: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> Cc: Sathyanarayanan Kuppuswamy <sathyanarayanan.kuppuswamy@xxxxxxxxxxxxxxx>
> ---
> Link to v1:
> https://lore.kernel.org/linux-pci/908047f7699d9de9ec2efd6b79aa752d73dab4b6.1595329748.git.lukas@xxxxxxxxx/
>
> In v1 I tried to solve the problem by changing the locking order for
> Secondary Bus Resets. That didn't really work out. I've experimented
> with various other approaches and finally came up with this simple one
> which lends itself well for backporting to stable. I apologize for the
> long time this has taken.
>
> drivers/pci/hotplug/pciehp_pci.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
>
> diff --git a/drivers/pci/hotplug/pciehp_pci.c b/drivers/pci/hotplug/pciehp_pci.c
> index d17f3bf..ad12515 100644
> --- a/drivers/pci/hotplug/pciehp_pci.c
> +++ b/drivers/pci/hotplug/pciehp_pci.c
> @@ -63,7 +63,14 @@ int pciehp_configure_device(struct controller *ctrl)
>
> pci_assign_unassigned_bridge_resources(bridge);
> pcie_bus_configure_settings(parent);
> +
> + /*
> + * Release reset_lock during driver binding
> + * to avoid AB-BA deadlock with device_lock.
> + */
> + up_read(&ctrl->reset_lock);
> pci_bus_add_devices(parent);
> + down_read_nested(&ctrl->reset_lock, ctrl->depth);
>
> out:
> pci_unlock_rescan_remove();
> @@ -104,7 +111,15 @@ void pciehp_unconfigure_device(struct controller *ctrl, bool presence)
> list_for_each_entry_safe_reverse(dev, temp, &parent->devices,
> bus_list) {
> pci_dev_get(dev);
> +
> + /*
> + * Release reset_lock during driver unbinding
> + * to avoid AB-BA deadlock with device_lock.
> + */
> + up_read(&ctrl->reset_lock);
> pci_stop_and_remove_bus_device(dev);
> + down_read_nested(&ctrl->reset_lock, ctrl->depth);
> +
> /*
> * Ensure that no new Requests will be generated from
> * the device.
> --
> 2.39.1
>
