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

[Shuai Xue <xueshuai@linux.alibaba.com>]

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>

Thanks.
Shuai