iommu/vt-d: Make intel_iommu_drain_pasid_prq() cover faults for RID

PATCH In review Lu Baolu 2 participants 4 messages Jan 21, 2025 – Feb 11, 2025

Subsystems (3)

Stable branch IOMMU Intel IOMMU (VT-d)

lore

Share & tools

lore

Lu Baolu

author maintainer

2025-01-21 02:31 UTC #0 lore raw

PATCH v2 1/1 iommu/vt-d: Make intel_iommu_drain_pasid_prq() cover faults for RID

This driver supports page faults on PCI RID since commit <9f831c16c69e> ("iommu/vt-d: Remove the pasid present check in prq_event_thread") by allowing the reporting of page faults with the pasid_present field cleared to the upper layer for further handling. The fundamental assumption here is that the detach or replace operations act as a fence for page faults. This implies that all pending page faults associated with a specific RID or PASID are flushed when a domain is detached or replaced from a device RID or PASID.

However, the intel_iommu_drain_pasid_prq() helper does not correctly handle faults for RID. This leads to faults potentially remaining pending in the iommu hardware queue even after the domain is detached, thereby violating the aforementioned assumption.

Fix this issue by extending intel_iommu_drain_pasid_prq() to cover faults for RID.

Fixes 9f831c16c69e ("iommu/vt-d: Remove the pasid present check in prq_event_thread")

Cc [email protected]

Signed-off-by Lu Baolu <[email protected]>

Reviewed-by Kevin Tian <[email protected]>

---
 drivers/iommu/intel/prq.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

Change log:
v2:
- Add check on page faults targeting RID.

v1: https://lore.kernel.org/linux-iommu/[email protected]/

drivers/iommu/intel/prq.c +3 -1

index c2d792db52c3..064194399b38 100644
@@ -87,7 +87,9 @@ void intel_iommu_drain_pasid_prq(struct device *dev, u32 pasid)
 		struct page_req_dsc *req;
 
 		req = &iommu->prq[head / sizeof(*req)];
-		if (!req->pasid_present || req->pasid != pasid) {
+		if (req->rid != sid ||
+		    (req->pasid_present && pasid != req->pasid) ||
+		    (!req->pasid_present && pasid != IOMMU_NO_PASID)) {
 			head = (head + sizeof(*req)) & PRQ_RING_MASK;
 			continue;
 		}

Signature · 1 line

2.43.0

Yi Liu

Reviewed-by on v2 1/1

2025-01-21 06:56 UTC #1 lore raw

Quoted 37 lines · Lu Baolu

This driver supports page faults on PCI RID since commit <9f831c16c69e> ("iommu/vt-d: Remove the pasid present check in prq_event_thread") by allowing the reporting of page faults with the pasid_present field cleared to the upper layer for further handling. The fundamental assumption here is that the detach or replace operations act as a fence for page faults. This implies that all pending page faults associated with a specific RID or PASID are flushed when a domain is detached or replaced from a device RID or PASID.
However, the intel_iommu_drain_pasid_prq() helper does not correctly handle faults for RID. This leads to faults potentially remaining pending in the iommu hardware queue even after the domain is detached, thereby violating the aforementioned assumption.
Fix this issue by extending intel_iommu_drain_pasid_prq() to cover faults for RID.
Fixes 9f831c16c69e ("iommu/vt-d: Remove the pasid present check in prq_event_thread")
Cc [email protected]
Suggested-by Kevin Tian <[email protected]>
Signed-off-by Lu Baolu <[email protected]>
Reviewed-by Kevin Tian <[email protected]>
---
  drivers/iommu/intel/prq.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
Change log:
v2:
- Add check on page faults targeting RID.
v1: https://lore.kernel.org/linux-iommu/[email protected]/
drivers/iommu/intel/prq.c +0 -0
index c2d792db52c3..064194399b38 100644

drivers/iommu/intel/prq.c void intel_iommu_drain_pasid_prq(struct device *dev, u32 pasid) · line 90

Quoted 5 lines

drivers/iommu/intel/prq.c +1 -1

@@ -87,7 +87,9 @@ void intel_iommu_drain_pasid_prq(struct device *dev, u32 pasid)
  		struct page_req_dsc *req;
 
  		req = &iommu->prq[head / sizeof(*req)];
-		if (!req->pasid_present || req->pasid != pasid) {
+		if (req->rid != sid ||

Does intel-iommu driver managed pasid per-bdf? or global? If the prior one, the rid check is needed even in the old time that does not PRIs in the RID path.

drivers/iommu/intel/prq.c void intel_iommu_drain_pasid_prq(struct device *dev, u32 pasid) · line 95

Quoted 5 lines

drivers/iommu/intel/prq.c +2 -0

+		    (req->pasid_present && pasid != req->pasid) ||
+		    (!req->pasid_present && pasid != IOMMU_NO_PASID)) {
  			head = (head + sizeof(*req)) & PRQ_RING_MASK;
  			continue;
  		}

LGTM.

Reviewed-by Yi Liu <[email protected]>

Signature · 2 lines

Regards,
Yi Liu

Baolu Lu

author maintainer

2025-02-05 02:28 UTC #2 lore raw

Quoted 33 lines · Yi Liu

Quoted 32 lines · Lu Baolu
This driver supports page faults on PCI RID since commit <9f831c16c69e> ("iommu/vt-d: Remove the pasid present check in prq_event_thread") by allowing the reporting of page faults with the pasid_present field cleared to the upper layer for further handling. The fundamental assumption here is that the detach or replace operations act as a fence for page faults. This implies that all pending page faults associated with a specific RID or PASID are flushed when a domain is detached or replaced from a device RID or PASID.
However, the intel_iommu_drain_pasid_prq() helper does not correctly handle faults for RID. This leads to faults potentially remaining pending in the iommu hardware queue even after the domain is detached, thereby violating the aforementioned assumption.
Fix this issue by extending intel_iommu_drain_pasid_prq() to cover faults for RID.
Fixes 9f831c16c69e ("iommu/vt-d: Remove the pasid present check in prq_event_thread")
Cc [email protected]
Suggested-by Kevin Tian <[email protected]>
Signed-off-by Lu Baolu <[email protected]>
Reviewed-by Kevin Tian <[email protected]>
---
drivers/iommu/intel/prq.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
Change log:
v2:
- Add check on page faults targeting RID.
v1: https://lore.kernel.org/linux-iommu/20250120080144.810455-1- [email protected]/

Quoted 14 lines

drivers/iommu/intel/prq.c void intel_iommu_drain_pasid_prq(struct device *dev, · line 90
Quoted 10 lines
drivers/iommu/intel/prq.c +1 -1
index c2d792db52c3..064194399b38 100644
@@ -87,7 +87,9 @@ void intel_iommu_drain_pasid_prq(struct device *dev, 
 u32 pasid)
           struct page_req_dsc *req;
           req = &iommu->prq[head / sizeof(*req)];
-        if (!req->pasid_present || req->pasid != pasid) {
+        if (req->rid != sid ||
Does intel-iommu driver managed pasid per-bdf? or global? If the prior one, the rid check is needed even in the old time that does not PRIs in the RID path.

Do you mean that this fix should be back ported farther than the fix tag commit?

The iommu driver doesn't manage the PASID. IOMMUFD and SVA do actually. The SVA uses global PASID. IOMMUFD doesn't yet support PASIDs, pending the merging of your patches. Therefore, per-BDF PASID management is not currently implemented in the Linux tree. Anything I overlooked?

---
baolu

Yi Liu

2025-02-11 08:58 UTC #3 lore raw

Quoted 49 lines · Baolu Lu

Quoted 33 lines · Yi Liu
Quoted 32 lines · Lu Baolu
This driver supports page faults on PCI RID since commit <9f831c16c69e> ("iommu/vt-d: Remove the pasid present check in prq_event_thread") by allowing the reporting of page faults with the pasid_present field cleared to the upper layer for further handling. The fundamental assumption here is that the detach or replace operations act as a fence for page faults. This implies that all pending page faults associated with a specific RID or PASID are flushed when a domain is detached or replaced from a device RID or PASID.
However, the intel_iommu_drain_pasid_prq() helper does not correctly handle faults for RID. This leads to faults potentially remaining pending in the iommu hardware queue even after the domain is detached, thereby violating the aforementioned assumption.
Fix this issue by extending intel_iommu_drain_pasid_prq() to cover faults for RID.
Fixes 9f831c16c69e ("iommu/vt-d: Remove the pasid present check in prq_event_thread")
Cc [email protected]
Suggested-by Kevin Tian <[email protected]>
Signed-off-by Lu Baolu <[email protected]>
Reviewed-by Kevin Tian <[email protected]>
---
drivers/iommu/intel/prq.c | 4 +++-
  1 file changed, 3 insertions(+), 1 deletion(-)
Change log:
v2:
- Add check on page faults targeting RID.
v1: https://lore.kernel.org/linux-iommu/20250120080144.810455-1- [email protected]/
Quoted 14 lines
drivers/iommu/intel/prq.c void intel_iommu_drain_pasid_prq(struct device *dev, · line 90
Quoted 10 lines
drivers/iommu/intel/prq.c +1 -1
index c2d792db52c3..064194399b38 100644
@@ -87,7 +87,9 @@ void intel_iommu_drain_pasid_prq(struct device *dev, 
 u32 pasid)
           struct page_req_dsc *req;
           req = &iommu->prq[head / sizeof(*req)];
-        if (!req->pasid_present || req->pasid != pasid) {
+        if (req->rid != sid ||
Does intel-iommu driver managed pasid per-bdf? or global? If the prior one, the rid check is needed even in the old time that does not PRIs in the RID path.

Quoted 2 lines

Do you mean that this fix should be back ported farther than the fix tag commit?

correct.

Quoted 4 lines

The iommu driver doesn't manage the PASID. IOMMUFD and SVA do actually. The SVA uses global PASID. IOMMUFD doesn't yet support PASIDs, pending the merging of your patches. Therefore, per-BDF PASID management is not currently implemented in the Linux tree. Anything I overlooked?

then, it's fine. No need to backport it I suppose.

Signature · 2 lines

Regards,
Yi Liu