On Mon, Nov 17, 2025 at 04:52:05AM +0000, Tian, Kevin wrote:

quoted

From: Nicolin Chen <redacted>
Sent: Saturday, November 15, 2025 2:01 AM

On Fri, Nov 14, 2025 at 09:45:31AM +0000, Tian, Kevin wrote:

quoted

quoted

From: Nicolin Chen <redacted>
Sent: Tuesday, November 11, 2025 1:13 PM

+/*
+ * Per PCIe r6.3, sec 10.3.1 IMPLEMENTATION NOTE, software disables

ATS

quoted

quoted

before
+ * initiating a reset. Notify the iommu driver that enabled ATS.
+ */
+int pci_reset_iommu_prepare(struct pci_dev *dev)
+{
+	if (pci_ats_supported(dev))
+		return iommu_dev_reset_prepare(&dev->dev);
+	return 0;
+}

the comment says "driver that enabled ATS", but the code checks
whether ATS is supported.

which one is desired?

The comments says "the iommu driver that enabled ATS". It doesn't
conflict with what the PCI core checks here?

actually this is sent to all IOMMU drivers. there is no check on whether
a specific driver has enabled ATS in this path.

But the comment doesn't say "check"..

How about "Notify the iommu driver that enables/disables ATS"?

The point is that pci_enable_ats() is called in iommu drivers.

quoted

quoted

quoted

+	/* Have to call it after waiting for pending DMA transaction */
+	ret = pci_reset_iommu_prepare(dev);
+	if (ret) {
+		pci_err(dev, "failed to stop IOMMU\n");

the error message could be more informative.

OK. Perhaps print the ret value.

and mention that it's for PCI reset.

OK.

Thanks
Nicolin