iommu/vt-d: Unconditionally flush device TLB for pasid table updates

The caching mode of an IOMMU is irrelevant to the behavior of the device TLB. Previously, commit <304b3bde24b5> ("iommu/vt-d: Remove caching mode check before device TLB flush") removed this redundant check in the domain unmap path.

Checking the caching mode before flushing the device TLB after a pasid table entry is updated is unnecessary and can lead to inconsistent behavior.

Extends this consistency by removing the caching mode check in the pasid table update path.

Suggested-by Yi Liu <[email protected]>

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

---
  drivers/iommu/intel/pasid.c | 12 +++---------
  1 file changed, 3 insertions(+), 9 deletions(-)

drivers/iommu/intel/pasid.c +3 -9

index 5792c817cefa..dc00eac6be31 100644
@@ -264,9 +264,7 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev,
  	else
  		iommu->flush.flush_iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH);
 
-	/* Device IOTLB doesn't need to be flushed in caching mode. */
-	if (!cap_caching_mode(iommu->cap))
-		devtlb_invalidation_with_pasid(iommu, dev, pasid);
+	devtlb_invalidation_with_pasid(iommu, dev, pasid);
  }
 
  /*

@@ -493,9 +491,7 @@ int intel_pasid_setup_dirty_tracking(struct intel_iommu *iommu,
 
  	iommu->flush.flush_iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH);
 
-	/* Device IOTLB doesn't need to be flushed in caching mode. */
-	if (!cap_caching_mode(iommu->cap))
-		devtlb_invalidation_with_pasid(iommu, dev, pasid);
+	devtlb_invalidation_with_pasid(iommu, dev, pasid);
 
  	return 0;
  }

@@ -572,9 +568,7 @@ void intel_pasid_setup_page_snoop_control(struct intel_iommu *iommu,
  	pasid_cache_invalidation_with_pasid(iommu, did, pasid);
  	qi_flush_piotlb(iommu, did, pasid, 0, -1, 0);
 
-	/* Device IOTLB doesn't need to be flushed in caching mode. */
-	if (!cap_caching_mode(iommu->cap))
-		devtlb_invalidation_with_pasid(iommu, dev, pasid);
+	devtlb_invalidation_with_pasid(iommu, dev, pasid);
  }
 
  /**

Quoted 11 lines · Lu Baolu

The caching mode of an IOMMU is irrelevant to the behavior of the device TLB. Previously, commit <304b3bde24b5> ("iommu/vt-d: Remove caching mode check before device TLB flush") removed this redundant check in the domain unmap path.

Checking the caching mode before flushing the device TLB after a pasid table entry is updated is unnecessary and can lead to inconsistent behavior.

Extends this consistency by removing the caching mode check in the pasid table update path.

I'm wondering if a fix tag is needed here. Before this patch, the guest kernel does not issue device TLB invalidation. This may be a problem for the emulated devices that support ATS capability. The cache in device side would be stale. Although some vIOMMU like QEMU virtual VT-d would notify the emulated devices to flush their cache when handling the unmap event. [1]. But this is not required by VT-d spec. So it's possible that other vIOMMU may not do it. So this patch appears to fix an issue, so a fix tag may be necessary. How about your thought?

Quoted 13 lines · Yi Liu

Quoted 11 lines · Lu Baolu

The caching mode of an IOMMU is irrelevant to the behavior of the device TLB. Previously, commit <304b3bde24b5> ("iommu/vt-d: Remove caching mode check before device TLB flush") removed this redundant check in the domain unmap path.

Checking the caching mode before flushing the device TLB after a pasid table entry is updated is unnecessary and can lead to inconsistent behavior.

Extends this consistency by removing the caching mode check in the pasid table update path.

Quoted 8 lines

I'm wondering if a fix tag is needed here. Before this patch, the guest kernel does not issue device TLB invalidation. This may be a problem for the emulated devices that support ATS capability. The cache in device side would be stale. Although some vIOMMU like QEMU virtual VT-d would notify the emulated devices to flush their cache when handling the unmap event. [1]. But this is not required by VT-d spec. So it's possible that other vIOMMU may not do it. So this patch appears to fix an issue, so a fix tag may be necessary. How about your thought?

It depends on whether this solves any real or potential issues.

The caching mode of an IOMMU is irrelevant to the behavior of the device TLB. Previously, commit <304b3bde24b5> ("iommu/vt-d: Remove caching mode check before device TLB flush") removed this redundant check in the domain unmap path.

Checking the caching mode before flushing the device TLB after a pasid table entry is updated is unnecessary and can lead to inconsistent behavior.

Extends this consistency by removing the caching mode check in the pasid table update path.

Suggested-by Yi Liu<[email protected]>

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

---
  drivers/iommu/intel/pasid.c | 12 +++---------
  1 file changed, 3 insertions(+), 9 deletions(-)