Re: [PATCH v8 5/5] PCI: Suspend iommu function prior to resetting a device

[Bjorn Helgaas]

[+cc Danny; this question was prompted by sashiko review of a
pci_ats_supported() patch at
https://lore.kernel.org/all/20261020181053.2646782-1-digonzal@xxxxxxxxxx]

On Mon, Dec 15, 2025 at 01:42:20PM -0800, Nicolin Chen wrote:
> PCIe permits a device to ignore ATS invalidation TLPs while processing a
> reset. This creates a problem visible to the OS where an ATS invalidation
> command will time out: e.g. an SVA domain will have no coordination with a
> reset event and can racily issue ATS invalidations to a resetting device.
> 
> The PCIe r6.0, sec 10.3.1 IMPLEMENTATION NOTE recommends SW to disable and
> block ATS before initiating a Function Level Reset. It also mentions that
> other reset methods could have the same vulnerability as well.
> 
> The IOMMU subsystem provides pci_dev_reset_iommu_prepare/done() callback
> helpers for this matter. Use them in all the existing reset functions.
> 
> This will attach the device to its iommu_group->blocking_domain during the
> device reset, so as to allow IOMMU driver to:
>  - invoke pci_disable_ats() and pci_enable_ats(), if necessary
>  - wait for all ATS invalidations to complete
>  - stop issuing new ATS invalidations
>  - fence any incoming ATS queries
> ...

> @@ -4885,10 +4919,20 @@ static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
>  		return -ENOTTY;
>  	}
>  
> +	rc = pci_dev_reset_iommu_prepare(dev);
> +	if (rc) {
> +		pci_err(dev, "failed to stop IOMMU for a PCI reset: %d\n", rc);
> +		return rc;
> +	}

Unlike the other reset functions, this one calls
pci_dev_reset_iommu_prepare() even when probe==true, i.e., we don't
want to actually perform the reset, we just want to know whether the
reset method might work.

It's at least unnecessary to call pci_dev_reset_iommu_prepare() and
pci_dev_reset_iommu_done() in that case, and maybe even problematic
because the pci_init_reset_methods() probe happens from
pci_init_capabilities(), which is pretty early during PCI enumeration
of the device.

I know this has already been merged as f5b16b802174 ("PCI: Suspend
iommu function prior to resetting a device"), and I'm not sure whether
we should do anything about it.  I wonder if we should just skip the
prepare/done when probe==true?

>  	rc = pci_dev_reset_slot_function(dev, probe);
>  	if (rc != -ENOTTY)
> -		return rc;
> -	return pci_parent_bus_reset(dev, probe);
> +		goto done;
> +
> +	rc = pci_parent_bus_reset(dev, probe);
> +done:
> +	pci_dev_reset_iommu_done(dev);
> +	return rc;
>  }