Re: 答复: [外部邮件] [PATCH 03/20] iommu/vt-d: Fix CACHE_TAG_NESTING_DEVTLB polluting shared variables in flush loop

[Baolu Lu <baolu.lu@linux.intel.com>]

On 8/4/2026 11:16 AM, Li,Rongqing wrote:
> 
> 
>> From: Guanghui Feng <guanghuifeng@linux.alibaba.com>
>>
>> In cache_tag_flush_range(), the CACHE_TAG_NESTING_DEVTLB case modifies the
>> shared local variables 'addr' and 'mask' before falling through to
>> CACHE_TAG_DEVTLB. This causes all subsequent CACHE_TAG_DEVTLB entries in
>> the same loop iteration to incorrectly use the full-range flush parameters
>> (addr=0, mask=MAX_AGAW_PFN_WIDTH) instead of the precisely calculated PSI
>> range. This is not the intended behavior, as regular DEVTLB entries should always
>> perform targeted range-based invalidation.
>>
>> Fix this by having CACHE_TAG_NESTING_DEVTLB directly call
>> cache_tag_flush_devtlb_psi() with the full-range constants and break, instead of
>> modifying shared variables and falling through. This ensures
>> CACHE_TAG_DEVTLB always uses the original calculated addr and mask for
>> precise range flush.
>>
>> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
>> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
>> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>> ---
>>   drivers/iommu/intel/cache.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/cache.c b/drivers/iommu/intel/cache.c index
>> fdc88817709f..26a758b0f501 100644
>> --- a/drivers/iommu/intel/cache.c
>> +++ b/drivers/iommu/intel/cache.c
>> @@ -454,9 +454,8 @@ void cache_tag_flush_range(struct dmar_domain
>> *domain, unsigned long start,
>>   			 * affected by a change in S2. So just flush the entire
>>   			 * device cache.
>>   			 */
>> -			addr = 0;
>> -			mask = MAX_AGAW_PFN_WIDTH;
>> -			fallthrough;
>> +			cache_tag_flush_devtlb_psi(domain, tag, 0,
>> MAX_AGAW_PFN_WIDTH);
>> +			break;
>>   		case CACHE_TAG_DEVTLB:
>>   			cache_tag_flush_devtlb_psi(domain, tag, addr, mask);
>>   			break;
>> --
>> 2.43.0
> 
> This patch introduces a subtle side effect on the tracing logic later in this function.
> At the end of cache_tag_flush_range(), trace_cache_tag_flush_range(tag, start, end, addr, mask) is called to log
> the flush operation.
> 
> With this patch: bypassed the assignment and used break, addr and mask retain their original range values.
> This causes the tracepoint to log an incorrect, smaller range while the actual hardware execution was a full-range flush.

The tracepoint here records what the caller requested: a specific cache-
invalidation type for a specific range. In this helper, we may widen the
invalidation range for implementation reasons (as described in the
comments), but that does not change the caller’s original intent.
Therefore, this tracepoint should log the caller-requested range.

If we want to observe the actual invalidation range sent to hardware,
that is already covered by the qi_submit trace event, which logs the
real invalidation descriptors submitted by the driver.

> 
> My patch has not this issue:
> https://lore.kernel.org/linux-iommu/20260605003950.1720-1-lirongqing@baidu.com/
Thanks,
baolu