Re: [PATCH v3] iommu/vt-d: fix intel iommu iotlb sync hardlockup and retry

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

On 3/10/26 20:59, Shuai Xue wrote:
> On 3/6/26 6:15 PM, Guanghui Feng wrote:
>> During the qi_check_fault process after an IOMMU ITE event,
>> requests at odd-numbered positions in the queue are set to
>> QI_ABORT, only satisfying single-request submissions. However,
>> qi_submit_sync now supports multiple simultaneous submissions,
>> and can't guarantee that the wait_desc will be at an odd-numbered
>> position. Therefore, if an item times out, IOMMU can't re-initiate
>> the request, resulting in an infinite polling wait.
>>
>> This patch modifies the process by setting the status of all requests
>> already fetched by IOMMU and recorded as QI_IN_USE status (including
>> wait_desc requests) to QI_ABORT, thus enabling multiple requests to
>> be resubmitted.
>>
>> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
>> Reviewed-by: Shuai Xue <xueshuai@linux.alibaba.com>
>> ---
>>   drivers/iommu/intel/dmar.c | 3 +--
>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
>> index d68c06025cac..69222dbd2af0 100644
>> --- a/drivers/iommu/intel/dmar.c
>> +++ b/drivers/iommu/intel/dmar.c
>> @@ -1314,7 +1314,6 @@ static int qi_check_fault(struct intel_iommu 
>> *iommu, int index, int wait_index)
>>       if (fault & DMA_FSTS_ITE) {
>>           head = readl(iommu->reg + DMAR_IQH_REG);
>>           head = ((head >> shift) - 1 + QI_LENGTH) % QI_LENGTH;
>> -        head |= 1;
>>           tail = readl(iommu->reg + DMAR_IQT_REG);
>>           tail = ((tail >> shift) - 1 + QI_LENGTH) % QI_LENGTH;
>> @@ -1331,7 +1330,7 @@ static int qi_check_fault(struct intel_iommu 
>> *iommu, int index, int wait_index)
>>           do {
>>               if (qi->desc_status[head] == QI_IN_USE)
>>                   qi->desc_status[head] = QI_ABORT;
>> -            head = (head - 2 + QI_LENGTH) % QI_LENGTH;
>> +            head = (head - 1 + QI_LENGTH) % QI_LENGTH;
>>           } while (head != tail);
>>           /*
> 
> Hi, all
> 
> I hacked IOMMU driver to submit a duplicate desc to trigger the bug:
> 
> @@ -1435,6 +1463,7 @@ int qi_submit_sync(struct intel_iommu *iommu, 
> struct qi_desc *desc,
>          s64 iec_start_ktime = 0;
>          struct qi_desc wait_desc;
>          int wait_index, index;
> +       int dup_slot = -1;
>          unsigned long flags;
>          int offset, shift;
>          int rc, i;
> @@ -1466,7 +1495,7 @@ int qi_submit_sync(struct intel_iommu *iommu, 
> struct qi_desc *desc,
>           * the calculation is based on:
>           * # of desc + 1 wait desc + 1 space between head and tail
>           */
> -       while (qi->free_cnt < count + 2) {
> +       while (qi->free_cnt < count + 3) {
>                  raw_spin_unlock_irqrestore(&qi->q_lock, flags);
>                  cpu_relax();
>                  raw_spin_lock_irqsave(&qi->q_lock, flags);
> @@ -1476,6 +1505,24 @@ int qi_submit_sync(struct intel_iommu *iommu, 
> struct qi_desc *desc,
>          wait_index = (index + count) % QI_LENGTH;
>          shift = qi_shift(iommu);
> 
> +       /*
> +        * DEBUG HACK: force wait_index to even position to reproduce the
> +        * ITE qi_check_fault bug where "head |= 1" causes the traversal
> +        * to skip even-indexed wait_desc. Duplicate desc[0] as an extra
> +        * submission when wait_index would be odd, so it stays harmless
> +        * (same request type, same content, just redundant).
> +        */
> +       if (wait_index & 1) {
> +               dup_slot = index;
> +               offset = index << shift;
> +               memcpy(qi->desc + offset, &desc[0], 1 << shift);
> +               qi->desc_status[index] = QI_IN_USE;
> +               index = (index + 1) % QI_LENGTH;
> +               qi->free_head = index;
> +               qi->free_cnt--;
> +               wait_index = (index + count) % QI_LENGTH;
> +       }
> +
>          for (i = 0; i < count; i++) {
>                  offset = ((index + i) % QI_LENGTH) << shift;
>                  memcpy(qi->desc + offset, &desc[i], 1 << shift);
> @@ -1532,6 +1579,8 @@ int qi_submit_sync(struct intel_iommu *iommu, 
> struct qi_desc *desc,
>           */
>          for (i = 0; i <= count; i++)
>                  qi->desc_status[(index + i) % QI_LENGTH] = QI_FREE;
> +       if (dup_slot >= 0)
> +               qi->desc_status[dup_slot] = QI_FREE;
> 
> When head is odd and tail is even, qi_check_fault will loop infinitely
> in the abort loop without this patch, since step -2 only visits
> odd-indexed slots and never reaches the even-valued tail. With step -1,
> the loop correctly visits all slots and terminates as expected.
> 
>      DMAR: VT-d detected Invalidation Time-out Error: SID 603
>      DMAR: QI HEAD: Invalidation Wait qw0 = 0x200000025, qw1 = 0x100364878
>      DMAR: DRHD: handling fault status reg 40
>      DMAR: QI PRIOR: Device-TLB Invalidation qw0 = 0x600060300000003, 
> qw1 = 0x7ffffffffffff001
>      DMAR: Invalidation Time-out Error (ITE) cleared
>      DMAR: ITE debug: head=29 tail=32 wait_index=32
>      DMAR: ITE debug: after loop, desc_status[wait_index=32]=2
> 
> Free free to add:
> 
> Tested-by: Shuai Xue <xueshuai@linux.alibaba.com>

I will queue this for v7.0-rc. Thank you!