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

[Baolu Lu]

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@xxxxxxxxxxxxxxxxx>
> > Reviewed-by: Shuai Xue <xueshuai@xxxxxxxxxxxxxxxxx>
> > ---
> >   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@xxxxxxxxxxxxxxxxx>

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