Skip to contentSkip to conversation

iommu/amd: Enable PASID and ATS capabilities in the correct order

PATCH In review Easwar Hariharan 3 participants 3 messages Jul 3, 2025 – Jul 5, 2025

Subsystems (2)
IOMMU AMD IOMMU (AMD-Vi)
lore
Share & tools
lore

Easwar Hariharan

author
#0 lore raw

PATCH 1/1 iommu/amd: Enable PASID and ATS capabilities in the correct order

Per the PCIe spec, behavior of the PASID capability is undefined if the value of the PASID Enable bit changes while the Enable bit of the function's ATS control register is Set. Unfortunately, pdev_enable_caps() does exactly that by ordering enabling ATS for the device before enabling PASID.

Cc Suravee Suthikulpanit <[email protected]>
Cc Vasant Hegde <[email protected]>
Cc Jason Gunthorpe <[email protected]>
Cc Jerry Snitselaar <[email protected]>
Fixes eda8c2860ab679 ("iommu/amd: Enable device ATS/PASID/PRI capabilities independently")
Signed-off-by Easwar Hariharan <[email protected]>
---
 drivers/iommu/amd/iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/amd/iommu.c +1 -1
index 3117d99cf83d..8b8d3e843743 100644
@@ -634,8 +634,8 @@ static inline void pdev_disable_cap_pasid(struct pci_dev *pdev)
 
 static void pdev_enable_caps(struct pci_dev *pdev)
 {
-	pdev_enable_cap_ats(pdev);
 	pdev_enable_cap_pasid(pdev);
+	pdev_enable_cap_ats(pdev);
 	pdev_enable_cap_pri(pdev);
 }
Signature · 1 line
2.43.0
Reviewed-by on v1 1/1
#1 lore raw
Quoted 12 lines · Easwar Hariharan

Per the PCIe spec, behavior of the PASID capability is undefined if the value of the PASID Enable bit changes while the Enable bit of the function's ATS control register is Set. Unfortunately, pdev_enable_caps() does exactly that by ordering enabling ATS for the device before enabling PASID.

Cc Suravee Suthikulpanit <[email protected]>
Cc Vasant Hegde <[email protected]>
Cc Jason Gunthorpe <[email protected]>
Cc Jerry Snitselaar <[email protected]>
Fixes eda8c2860ab679 ("iommu/amd: Enable device ATS/PASID/PRI capabilities independently")
Signed-off-by Easwar Hariharan <[email protected]>
Quoted 3 lines
---
 drivers/iommu/amd/iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by Jason Gunthorpe <[email protected]>

The other two drivers look like they do it Ok.

Jason

maintainer Reviewed-by on v1 1/1
#2 lore raw
In reply to Easwar Hariharan
Quoted 11 lines · Easwar Hariharan

Per the PCIe spec, behavior of the PASID capability is undefined if the value of the PASID Enable bit changes while the Enable bit of the function's ATS control register is Set. Unfortunately, pdev_enable_caps() does exactly that by ordering enabling ATS for the device before enabling PASID.

Quoted 1 line
Cc Suravee Suthikulpanit <[email protected]>
Cc Vasant Hegde <[email protected]>
Cc Jason Gunthorpe <[email protected]>
Cc Jerry Snitselaar <[email protected]>
Fixes eda8c2860ab679 ("iommu/amd: Enable device ATS/PASID/PRI capabilities independently")
Signed-off-by Easwar Hariharan <[email protected]>

Patch looks good.

Reviewed-by Vasant Hegde <[email protected]>

-Vasant

Quoted 18 lines
---
 drivers/iommu/amd/iommu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iommu/amd/iommu.c +1 -1
index 3117d99cf83d..8b8d3e843743 100644
@@ -634,8 +634,8 @@ static inline void pdev_disable_cap_pasid(struct pci_dev *pdev)
 
 static void pdev_enable_caps(struct pci_dev *pdev)
 {
-	pdev_enable_cap_ats(pdev);
 	pdev_enable_cap_pasid(pdev);
+	pdev_enable_cap_ats(pdev);
 	pdev_enable_cap_pri(pdev);
 }