[PATCH 1/2] iommu/vt-d: Fix missing PASID in dev TLB flush with cache_tag_flush_all

[PATCH 1/2] iommu/vt-d: Fix missing PASID in dev TLB flush with cache_tag_flush_all

[Ethan MILON]

The function cache_tag_flush_all() was originally implemented with
incorrect device TLB invalidation logic that does not handle PASID, in
commit c4d27ffaa8eb ("iommu/vt-d: Add cache tag invalidation helpers")

This causes regressions where full address space TLB invalidations occur
with a PASID attached, such as during transparent hugepage unmapping in
SVA configurations or when calling iommu_flush_iotlb_all(). In these
cases, the device receives a TLB invalidation that lacks PASID.

This incorrect logic was later extracted into
cache_tag_flush_devtlb_all(), in commit 3297d047cd7f ("iommu/vt-d:
Refactor IOTLB and Dev-IOTLB flush for batching")

The fix replaces the call to cache_tag_flush_devtlb_all() with
cache_tag_flush_devtlb_psi(), which properly handles PASID.

Fixes: 4f609dbff51b ("iommu/vt-d: Use cache helpers in arch_invalidate_secondary_tlbs")
Fixes: 4e589a53685c ("iommu/vt-d: Use cache_tag_flush_all() in flush_iotlb_all")

Signed-off-by: Ethan Milon <ethan.milon@eviden.com>
---
 drivers/iommu/intel/cache.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/drivers/iommu/intel/cache.c b/drivers/iommu/intel/cache.c
index 47692cbfaabd..c8b79de84d3f 100644
--- a/drivers/iommu/intel/cache.c
+++ b/drivers/iommu/intel/cache.c
@@ -422,22 +422,6 @@ static void cache_tag_flush_devtlb_psi(struct dmar_domain *domain, struct cache_
 					     domain->qi_batch);
 }
 
-static void cache_tag_flush_devtlb_all(struct dmar_domain *domain, struct cache_tag *tag)
-{
-	struct intel_iommu *iommu = tag->iommu;
-	struct device_domain_info *info;
-	u16 sid;
-
-	info = dev_iommu_priv_get(tag->dev);
-	sid = PCI_DEVID(info->bus, info->devfn);
-
-	qi_batch_add_dev_iotlb(iommu, sid, info->pfsid, info->ats_qdep, 0,
-			       MAX_AGAW_PFN_WIDTH, domain->qi_batch);
-	if (info->dtlb_extra_inval)
-		qi_batch_add_dev_iotlb(iommu, sid, info->pfsid, info->ats_qdep, 0,
-				       MAX_AGAW_PFN_WIDTH, domain->qi_batch);
-}
-
 /*
  * Invalidates a range of IOVA from @start (inclusive) to @end (inclusive)
  * when the memory mappings in the target domain have been modified.
@@ -508,7 +492,7 @@ void cache_tag_flush_all(struct dmar_domain *domain)
 			break;
 		case CACHE_TAG_DEVTLB:
 		case CACHE_TAG_NESTING_DEVTLB:
-			cache_tag_flush_devtlb_all(domain, tag);
+			cache_tag_flush_devtlb_psi(domain, tag, 0, MAX_AGAW_PFN_WIDTH);
 			break;
 		}
 
-- 
2.50.0

[PATCH 2/2] iommu/vt-d: Deduplicate cache_tag_flush_all by reusing flush_range

[Ethan MILON]

The logic in cache_tag_flush_all() to iterate over cache tags and issue
TLB invalidations is largely duplicated in cache_tag_flush_range(), with
the only difference being the range parameters.

Extend cache_tag_flush_range() to handle a full address space flush when
called with start = 0 and end = ULONG_MAX. This allows
cache_tag_flush_all() to simply delegate to cache_tag_flush_range()

Signed-off-by: Ethan Milon <ethan.milon@eviden.com>
---
 drivers/iommu/intel/cache.c | 34 ++++++++--------------------------
 drivers/iommu/intel/trace.h |  5 -----
 2 files changed, 8 insertions(+), 31 deletions(-)

diff --git a/drivers/iommu/intel/cache.c b/drivers/iommu/intel/cache.c
index c8b79de84d3f..e69f04bd54bf 100644
--- a/drivers/iommu/intel/cache.c
+++ b/drivers/iommu/intel/cache.c
@@ -434,7 +434,13 @@ void cache_tag_flush_range(struct dmar_domain *domain, unsigned long start,
 	struct cache_tag *tag;
 	unsigned long flags;
 
-	addr = calculate_psi_aligned_address(start, end, &pages, &mask);
+	if (start == 0 && end == ULONG_MAX) {
+		addr = 0;
+		pages = -1;
+		mask = MAX_AGAW_PFN_WIDTH;
+	} else {
+		addr = calculate_psi_aligned_address(start, end, &pages, &mask);
+	}
 
 	spin_lock_irqsave(&domain->cache_lock, flags);
 	list_for_each_entry(tag, &domain->cache_tags, node) {
@@ -475,31 +481,7 @@ void cache_tag_flush_range(struct dmar_domain *domain, unsigned long start,
  */
 void cache_tag_flush_all(struct dmar_domain *domain)
 {
-	struct intel_iommu *iommu = NULL;
-	struct cache_tag *tag;
-	unsigned long flags;
-
-	spin_lock_irqsave(&domain->cache_lock, flags);
-	list_for_each_entry(tag, &domain->cache_tags, node) {
-		if (iommu && iommu != tag->iommu)
-			qi_batch_flush_descs(iommu, domain->qi_batch);
-		iommu = tag->iommu;
-
-		switch (tag->type) {
-		case CACHE_TAG_IOTLB:
-		case CACHE_TAG_NESTING_IOTLB:
-			cache_tag_flush_iotlb(domain, tag, 0, -1, 0, 0);
-			break;
-		case CACHE_TAG_DEVTLB:
-		case CACHE_TAG_NESTING_DEVTLB:
-			cache_tag_flush_devtlb_psi(domain, tag, 0, MAX_AGAW_PFN_WIDTH);
-			break;
-		}
-
-		trace_cache_tag_flush_all(tag);
-	}
-	qi_batch_flush_descs(iommu, domain->qi_batch);
-	spin_unlock_irqrestore(&domain->cache_lock, flags);
+	cache_tag_flush_range(domain, 0, ULONG_MAX, 0);
 }
 
 /*
diff --git a/drivers/iommu/intel/trace.h b/drivers/iommu/intel/trace.h
index 9defdae6ebae..6311ba3f1691 100644
--- a/drivers/iommu/intel/trace.h
+++ b/drivers/iommu/intel/trace.h
@@ -130,11 +130,6 @@ DEFINE_EVENT(cache_tag_log, cache_tag_unassign,
 	TP_ARGS(tag)
 );
 
-DEFINE_EVENT(cache_tag_log, cache_tag_flush_all,
-	TP_PROTO(struct cache_tag *tag),
-	TP_ARGS(tag)
-);
-
 DECLARE_EVENT_CLASS(cache_tag_flush,
 	TP_PROTO(struct cache_tag *tag, unsigned long start, unsigned long end,
 		 unsigned long addr, unsigned long pages, unsigned long mask),
-- 
2.50.0

Re: [PATCH 1/2] iommu/vt-d: Fix missing PASID in dev TLB flush with cache_tag_flush_all

[Baolu Lu]

On 7/9/2025 5:48 AM, Ethan MILON wrote:
> The function cache_tag_flush_all() was originally implemented with
> incorrect device TLB invalidation logic that does not handle PASID, in
> commit c4d27ffaa8eb ("iommu/vt-d: Add cache tag invalidation helpers")
> 
> This causes regressions where full address space TLB invalidations occur
> with a PASID attached, such as during transparent hugepage unmapping in
> SVA configurations or when calling iommu_flush_iotlb_all(). In these
> cases, the device receives a TLB invalidation that lacks PASID.
> 
> This incorrect logic was later extracted into
> cache_tag_flush_devtlb_all(), in commit 3297d047cd7f ("iommu/vt-d:
> Refactor IOTLB and Dev-IOTLB flush for batching")
> 
> The fix replaces the call to cache_tag_flush_devtlb_all() with
> cache_tag_flush_devtlb_psi(), which properly handles PASID.
> 
> Fixes: 4f609dbff51b ("iommu/vt-d: Use cache helpers in arch_invalidate_secondary_tlbs")
> Fixes: 4e589a53685c ("iommu/vt-d: Use cache_tag_flush_all() in flush_iotlb_all")
> 

Remove above blank line.

> Signed-off-by: Ethan Milon<ethan.milon@eviden.com>

checkpatch.pl complains:

WARNING: From:/Signed-off-by: email name mismatch: 'From: Ethan MILON 
<ethan.milon@eviden.com>' != 'Signed-off-by: Ethan Milon 
<ethan.milon@eviden.com>'

> ---
>   drivers/iommu/intel/cache.c | 18 +-----------------
>   1 file changed, 1 insertion(+), 17 deletions(-)

Other looks good to me.

Thanks,
baolu

Re: [PATCH 2/2] iommu/vt-d: Deduplicate cache_tag_flush_all by reusing flush_range

[Baolu Lu]

On 7/9/2025 5:48 AM, Ethan MILON wrote:
> The logic in cache_tag_flush_all() to iterate over cache tags and issue
> TLB invalidations is largely duplicated in cache_tag_flush_range(), with
> the only difference being the range parameters.
> 
> Extend cache_tag_flush_range() to handle a full address space flush when
> called with start = 0 and end = ULONG_MAX. This allows
> cache_tag_flush_all() to simply delegate to cache_tag_flush_range()
> 
> Signed-off-by: Ethan Milon<ethan.milon@eviden.com>

The same issue as the PATCH 1/2.

Thanks,
baplu

Re: [PATCH 1/2] iommu/vt-d: Fix missing PASID in dev TLB flush with cache_tag_flush_all

[Ethan MILON]

Hi Baolu,

Thanks for the review.

On 7/10/25 10:59, Baolu Lu wrote:
> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
> 
> 
> On 7/9/2025 5:48 AM, Ethan MILON wrote:
>> The function cache_tag_flush_all() was originally implemented with
>> incorrect device TLB invalidation logic that does not handle PASID, in
>> commit c4d27ffaa8eb ("iommu/vt-d: Add cache tag invalidation helpers")
>>
>> This causes regressions where full address space TLB invalidations occur
>> with a PASID attached, such as during transparent hugepage unmapping in
>> SVA configurations or when calling iommu_flush_iotlb_all(). In these
>> cases, the device receives a TLB invalidation that lacks PASID.
>>
>> This incorrect logic was later extracted into
>> cache_tag_flush_devtlb_all(), in commit 3297d047cd7f ("iommu/vt-d:
>> Refactor IOTLB and Dev-IOTLB flush for batching")
>>
>> The fix replaces the call to cache_tag_flush_devtlb_all() with
>> cache_tag_flush_devtlb_psi(), which properly handles PASID.
>>
>> Fixes: 4f609dbff51b ("iommu/vt-d: Use cache helpers in arch_invalidate_secondary_tlbs")
>> Fixes: 4e589a53685c ("iommu/vt-d: Use cache_tag_flush_all() in flush_iotlb_all")
>>
> 
> Remove above blank line.

Noted.

> 
>> Signed-off-by: Ethan Milon<ethan.milon@eviden.com>
> 
> checkpatch.pl complains:
> 
> WARNING: From:/Signed-off-by: email name mismatch: 'From: Ethan MILON
> <ethan.milon@eviden.com>' != 'Signed-off-by: Ethan Milon
> <ethan.milon@eviden.com>'

Understood. Our email server seems to overwrite our last names. I’ll
look into correcting that.

> 
>> ---
>>   drivers/iommu/intel/cache.c | 18 +-----------------
>>   1 file changed, 1 insertion(+), 17 deletions(-)
> 
> Other looks good to me.

Should I send a v2 with the fixes?

Thanks,
Ethan

> 
> Thanks,
> baolu

Re: [PATCH 1/2] iommu/vt-d: Fix missing PASID in dev TLB flush with cache_tag_flush_all

[Baolu Lu]

On 7/11/2025 4:21 PM, Ethan MILON wrote:
> Hi Baolu,
> 
> Thanks for the review.
> 
> On 7/10/25 10:59, Baolu Lu wrote:
>> Caution: External email. Do not open attachments or click links, unless this email comes from a known sender and you know the content is safe.
>>
>>
>> On 7/9/2025 5:48 AM, Ethan MILON wrote:
>>> The function cache_tag_flush_all() was originally implemented with
>>> incorrect device TLB invalidation logic that does not handle PASID, in
>>> commit c4d27ffaa8eb ("iommu/vt-d: Add cache tag invalidation helpers")
>>>
>>> This causes regressions where full address space TLB invalidations occur
>>> with a PASID attached, such as during transparent hugepage unmapping in
>>> SVA configurations or when calling iommu_flush_iotlb_all(). In these
>>> cases, the device receives a TLB invalidation that lacks PASID.
>>>
>>> This incorrect logic was later extracted into
>>> cache_tag_flush_devtlb_all(), in commit 3297d047cd7f ("iommu/vt-d:
>>> Refactor IOTLB and Dev-IOTLB flush for batching")
>>>
>>> The fix replaces the call to cache_tag_flush_devtlb_all() with
>>> cache_tag_flush_devtlb_psi(), which properly handles PASID.
>>>
>>> Fixes: 4f609dbff51b ("iommu/vt-d: Use cache helpers in arch_invalidate_secondary_tlbs")
>>> Fixes: 4e589a53685c ("iommu/vt-d: Use cache_tag_flush_all() in flush_iotlb_all")
>>>
>> Remove above blank line.
> Noted.
> 
>>> Signed-off-by: Ethan Milon<ethan.milon@eviden.com>
>> checkpatch.pl complains:
>>
>> WARNING: From:/Signed-off-by: email name mismatch: 'From: Ethan MILON
>> <ethan.milon@eviden.com>' != 'Signed-off-by: Ethan Milon
>> <ethan.milon@eviden.com>'
> Understood. Our email server seems to overwrite our last names. I’ll
> look into correcting that.
> 
>>> ---
>>>    drivers/iommu/intel/cache.c | 18 +-----------------
>>>    1 file changed, 1 insertion(+), 17 deletions(-)
>> Other looks good to me.
> Should I send a v2 with the fixes?

No. I can change it manually if you have no objection.

Thanks,
baolu

Re: [PATCH 1/2] iommu/vt-d: Fix missing PASID in dev TLB flush with cache_tag_flush_all

[Baolu Lu]

On 7/12/25 12:09, Baolu Lu wrote:
> On 7/11/2025 4:21 PM, Ethan MILON wrote:
>> Hi Baolu,
>>
>> Thanks for the review.
>>
>> On 7/10/25 10:59, Baolu Lu wrote:
>>> Caution: External email. Do not open attachments or click links, 
>>> unless this email comes from a known sender and you know the content 
>>> is safe.
>>>
>>>
>>> On 7/9/2025 5:48 AM, Ethan MILON wrote:
>>>> The function cache_tag_flush_all() was originally implemented with
>>>> incorrect device TLB invalidation logic that does not handle PASID, in
>>>> commit c4d27ffaa8eb ("iommu/vt-d: Add cache tag invalidation helpers")
>>>>
>>>> This causes regressions where full address space TLB invalidations 
>>>> occur
>>>> with a PASID attached, such as during transparent hugepage unmapping in
>>>> SVA configurations or when calling iommu_flush_iotlb_all(). In these
>>>> cases, the device receives a TLB invalidation that lacks PASID.
>>>>
>>>> This incorrect logic was later extracted into
>>>> cache_tag_flush_devtlb_all(), in commit 3297d047cd7f ("iommu/vt-d:
>>>> Refactor IOTLB and Dev-IOTLB flush for batching")
>>>>
>>>> The fix replaces the call to cache_tag_flush_devtlb_all() with
>>>> cache_tag_flush_devtlb_psi(), which properly handles PASID.
>>>>
>>>> Fixes: 4f609dbff51b ("iommu/vt-d: Use cache helpers in 
>>>> arch_invalidate_secondary_tlbs")
>>>> Fixes: 4e589a53685c ("iommu/vt-d: Use cache_tag_flush_all() in 
>>>> flush_iotlb_all")
>>>>
>>> Remove above blank line.
>> Noted.
>>
>>>> Signed-off-by: Ethan Milon<ethan.milon@eviden.com>
>>> checkpatch.pl complains:
>>>
>>> WARNING: From:/Signed-off-by: email name mismatch: 'From: Ethan MILON
>>> <ethan.milon@eviden.com>' != 'Signed-off-by: Ethan Milon
>>> <ethan.milon@eviden.com>'
>> Understood. Our email server seems to overwrite our last names. I’ll
>> look into correcting that.
>>
>>>> ---
>>>>    drivers/iommu/intel/cache.c | 18 +-----------------
>>>>    1 file changed, 1 insertion(+), 17 deletions(-)
>>> Other looks good to me.
>> Should I send a v2 with the fixes?
> 
> No. I can change it manually if you have no objection.

Queued these two patches for linux-next with above nits addressed. Thank
you!

--
baolu