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

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

[Guanghui Feng]

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);
 
 		/*
-- 
2.43.7

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

[Samiullah Khawaja]

On Fri, Mar 06, 2026 at 06:15:16PM +0800, 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);
>
> 		/*
>-- 
>2.43.7
>
>

Reviewed-by: Samiullah Khawaja <skhawaja@google.com>

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

[guanghuifeng]

There are some concerns:

1. During the invalid request execution process, the IOMMU first fetches 
requests

     from the invalid queue to the internal cache.


2. If an ITE timeout occurs during the execution of a request fetched to 
the cache in step 1,

     the IOMMU driver clears the ITE status, allowing IOMMU to resume 
processing requests from the invalid queue.


3. For requests already fetched in step 1 that experience an ITE 
timeout, after the IOMMU driver clears the ITE,

     will IOMMU directly discard these timed-out/cached requests? or 
will it continue to execute these cached requests again?


Currently, the IOMMU driver implementation first clears ite to resume 
IOMMU execution

before setting desc_status to QI_ABORT.

If IOMMU will re-execute requests from the cache, then the IOMMU driver 
needs to be modified.

It should first set desc_status to QI_ABORT, then execute 
writel(DMA_FSTS_ITE, iommu->reg + DMAR_FSTS_REG)

to resume IOMMU execution(In this case, some requests will be 
resubmitted and executed twice.).

Otherwise, iommu may write the QI_DONE result back to desc_status after 
execution, and the iommu driver will

simultaneously set desc_status to QI_ABORT, leading to data modification 
contention and timing issues.


Thanks.


在 2026/3/6 18:15, Guanghui Feng 写道:
> 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);
>   
>   		/*

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

[Baolu Lu]

On 3/9/26 17:05, guanghuifeng@linux.alibaba.com wrote:
> There are some concerns:
> 
> 1. During the invalid request execution process, the IOMMU first fetches 
> requests
> 
>      from the invalid queue to the internal cache.
> 
> 
> 2. If an ITE timeout occurs during the execution of a request fetched to 
> the cache in step 1,
> 
>      the IOMMU driver clears the ITE status, allowing IOMMU to resume 
> processing requests from the invalid queue.
> 
> 
> 3. For requests already fetched in step 1 that experience an ITE 
> timeout, after the IOMMU driver clears the ITE,
> 
>      will IOMMU directly discard these timed-out/cached requests? or 
> will it continue to execute these cached requests again?
> 
> 
> Currently, the IOMMU driver implementation first clears ite to resume 
> IOMMU execution
> 
> before setting desc_status to QI_ABORT.
> 
> If IOMMU will re-execute requests from the cache, then the IOMMU driver 
> needs to be modified.

You are right.

The driver logic assumes that once an ITE error is cleared, the IOMMU
will not resume its previous execution but will instead fetch new
descriptors from the queue. This behavior was introduced by commit
6ba6c3a4cacfd ("VT-d: add device IOTLB invalidation support"), which has
been part of the driver since 2009.

> 
> It should first set desc_status to QI_ABORT, then execute 
> writel(DMA_FSTS_ITE, iommu->reg + DMAR_FSTS_REG)
> 
> to resume IOMMU execution(In this case, some requests will be 
> resubmitted and executed twice.).
> 
> Otherwise, iommu may write the QI_DONE result back to desc_status after 
> execution, and the iommu driver will
> 
> simultaneously set desc_status to QI_ABORT, leading to data modification 
> contention and timing issues.
> 
> 
> Thanks.

Thanks,
baolu

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

[Baolu Lu]

On 3/10/26 14:02, Baolu Lu wrote:
> On 3/9/26 17:05, guanghuifeng@linux.alibaba.com wrote:
>> There are some concerns:
>>
>> 1. During the invalid request execution process, the IOMMU first 
>> fetches requests
>>
>>      from the invalid queue to the internal cache.
>>
>>
>> 2. If an ITE timeout occurs during the execution of a request fetched 
>> to the cache in step 1,
>>
>>      the IOMMU driver clears the ITE status, allowing IOMMU to resume 
>> processing requests from the invalid queue.
>>
>>
>> 3. For requests already fetched in step 1 that experience an ITE 
>> timeout, after the IOMMU driver clears the ITE,
>>
>>      will IOMMU directly discard these timed-out/cached requests? or 
>> will it continue to execute these cached requests again?
>>
>>
>> Currently, the IOMMU driver implementation first clears ite to resume 
>> IOMMU execution
>>
>> before setting desc_status to QI_ABORT.
>>
>> If IOMMU will re-execute requests from the cache, then the IOMMU 
>> driver needs to be modified.
> 
> You are right.
> 
> The driver logic assumes that once an ITE error is cleared, the IOMMU
> will not resume its previous execution but will instead fetch new
> descriptors from the queue. This behavior was introduced by commit
> 6ba6c3a4cacfd ("VT-d: add device IOTLB invalidation support"), which has
> been part of the driver since 2009.

By the way, section 6.5.2.11 of the spec states:

"
... At the time ITE field is Set, hardware aborts any inv_wait_dsc
commands pending in hardware and does not increment the Invalidation
Queue Head register. When software clears the ITE field in the Fault
Status Register, hardware fetches descriptor pointed by the Invalidation
Queue Head register. ...
"

This implies that the hardware must abort the previous execution and
restart by fetching new requests.

Thanks,
baolu

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

[Shuai Xue]

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

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@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!