Re: [PATCH v2 2/3] PCI: Warn on missing cfg_access_lock during secondary bus reset

[Dave Jiang <dave.jiang@intel.com>]

On 5/30/24 6:04 PM, Dan Williams wrote:
> The recent adventure with adding lockdep tracking for cfg_access_lock,
> while it yielded many false positives [1], it did catch a true positive in
> the pci_reset_bus() path [2].
> 
> So, while lockdep is difficult to deploy, open coding a check that
> cfg_access_lock is held during the reset is feasible.
> 
> While this does not offer a full backtrace, it should be sufficient to
> implicate the caller of pci_bridge_secondary_bus_reset() as a path that
> needs investigation.
> 
> Link: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_134186v1/shard-dg2-1/igt@device_reset@unbind-reset-rebind.html [1]
> Link: http://lore.kernel.org/r/cfb50601-5d2a-4676-a958-1bd3f1b06654@intel.com [2]
> Cc: Dave Jiang <dave.jiang@intel.com>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/pci/pci.c |    3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 35fb1f17a589..8df32a3a0979 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4883,6 +4883,9 @@ void __weak pcibios_reset_secondary_bus(struct pci_dev *dev)
>   */
>  int pci_bridge_secondary_bus_reset(struct pci_dev *dev)
>  {
> +	if (!dev->block_cfg_access)
> +		pci_warn_once(dev, "unlocked secondary bus reset via: %pS\n",
> +			      __builtin_return_address(0));
>  	pcibios_reset_secondary_bus(dev);
>  
>  	return pci_bridge_wait_for_secondary_bus(dev, "bus reset");
>