* [PATCH] iommu/vt-d: fix intel iommu iotlb sync hardlockup & retry
@ 2026-02-02 2:09 Guanghui Feng
2026-02-04 9:32 ` Baolu Lu
0 siblings, 1 reply; 9+ messages in thread
From: Guanghui Feng @ 2026-02-02 2:09 UTC (permalink / raw)
To: dwmw2, baolu.lu, joro, will, robin.murphy, iommu, linux-kernel
Cc: alikernel-developer
Device-TLB Invalidation Response Time-out (ITE) handling was added in
commit: 6ba6c3a4cacfd68bf970e3e04e2ff0d66fa0f695.
When an ITE occurs, iommu will sets the ITE (Invalidation Time-out
Error) field in the Fault Status Register. No new descriptors are
fetched from the Invalidation Queue until software clears the ITE field
in the Fault Status Register. Tail pointer Register updates by software
while the ITE field is Set does not cause descriptor fetches by
hardware. 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.
But in the qi_check_fault process, it is implemented by default
according to the 2009 commit: 6ba6c3a4cacfd68bf970e3e04e2ff0d66fa0f695,
that is, only one struct qi_desc is submitted at a time. A qi_desc request is
immediately followed by a wait_desc/QI_IWD_TYPE for
synchronization. Therefore, the IOMMU driver implementation
considers invalid queue entries at odd positions to be
wait_desc. After ITE is set, hardware aborts any pending
inv_wait_dsc commands in hardware. Therefore, qi_check_fault
iterates through odd-position as wait_desc entries and sets
desc_status to QI_ABORT. However, the current implementation
allows multiple struct qi_desc to be submitted simultaneously,
followed by one wait_desc, so it's no longer guaranteed that
odd-position entries will be wait_desc. When the number of submitted
struct qi_desc is even, wait_desc's desc_status will not be set to QI_ABORT,
qi_check_fault will return 0, and qi_submit_sync will then
execute in an infinite loop and cause a hard lockup when
interrupts are disabled and the PCIe device does not respond to
Device-TLB Invalidation requests.
Additionally, if the device remains online and an IOMMU ITE
occurs, simply returning -EAGAIN is sufficient. When processing
the -EAGAIN result, qi_submit_sync will automatically reclaim
all submitted struct qi_desc and resubmit the requests.
Through this modification:
1. Correctly triggers the resubmission of struct qi_desc when
an ITE occurs.
2. Prevents the IOMMU driver from disabling interrupts and
executing in an infinite loop within qi_submit_sync when an
ITE occurs, avoiding hardlockup.
Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
---
drivers/iommu/intel/dmar.c | 18 +++---------------
1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index ec975c73cfe6..f31f0095f9a8 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1271,7 +1271,7 @@ static void qi_dump_fault(struct intel_iommu *iommu, u32 fault)
static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
{
u32 fault;
- int head, tail;
+ int head;
struct device *dev;
u64 iqe_err, ite_sid;
struct q_inval *qi = iommu->qi;
@@ -1312,12 +1312,6 @@ static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
* No new descriptors are fetched until the ITE is cleared.
*/
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;
-
/*
* SID field is valid only when the ITE field is Set in FSTS_REG
* see Intel VT-d spec r4.1, section 11.4.9.9
@@ -1328,12 +1322,6 @@ static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
writel(DMA_FSTS_ITE, iommu->reg + DMAR_FSTS_REG);
pr_info("Invalidation Time-out Error (ITE) cleared\n");
- do {
- if (qi->desc_status[head] == QI_IN_USE)
- qi->desc_status[head] = QI_ABORT;
- head = (head - 2 + QI_LENGTH) % QI_LENGTH;
- } while (head != tail);
-
/*
* If device was released or isn't present, no need to retry
* the ATS invalidate request anymore.
@@ -1347,8 +1335,8 @@ static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
!pci_device_is_present(to_pci_dev(dev)))
return -ETIMEDOUT;
}
- if (qi->desc_status[wait_index] == QI_ABORT)
- return -EAGAIN;
+
+ return -EAGAIN;
}
if (fault & DMA_FSTS_ICE) {
--
2.43.7
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iommu/vt-d: fix intel iommu iotlb sync hardlockup & retry
2026-02-02 2:09 [PATCH] iommu/vt-d: fix intel iommu iotlb sync hardlockup & retry Guanghui Feng
@ 2026-02-04 9:32 ` Baolu Lu
2026-02-05 10:28 ` guanghuifeng
0 siblings, 1 reply; 9+ messages in thread
From: Baolu Lu @ 2026-02-04 9:32 UTC (permalink / raw)
To: Guanghui Feng, dwmw2, joro, will, robin.murphy, iommu, linux-kernel
Cc: baolu.lu, alikernel-developer
On 2/2/2026 10:09 AM, Guanghui Feng wrote:
> Device-TLB Invalidation Response Time-out (ITE) handling was added in
> commit: 6ba6c3a4cacfd68bf970e3e04e2ff0d66fa0f695.
>
> When an ITE occurs, iommu will sets the ITE (Invalidation Time-out
> Error) field in the Fault Status Register. No new descriptors are
> fetched from the Invalidation Queue until software clears the ITE field
> in the Fault Status Register. Tail pointer Register updates by software
> while the ITE field is Set does not cause descriptor fetches by
> hardware. 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.
>
> But in the qi_check_fault process, it is implemented by default
> according to the 2009 commit: 6ba6c3a4cacfd68bf970e3e04e2ff0d66fa0f695,
> that is, only one struct qi_desc is submitted at a time. A qi_desc request is
> immediately followed by a wait_desc/QI_IWD_TYPE for
> synchronization. Therefore, the IOMMU driver implementation
> considers invalid queue entries at odd positions to be
> wait_desc. After ITE is set, hardware aborts any pending
> inv_wait_dsc commands in hardware. Therefore, qi_check_fault
> iterates through odd-position as wait_desc entries and sets
> desc_status to QI_ABORT. However, the current implementation
> allows multiple struct qi_desc to be submitted simultaneously,
> followed by one wait_desc, so it's no longer guaranteed that
> odd-position entries will be wait_desc. When the number of submitted
> struct qi_desc is even, wait_desc's desc_status will not be set to QI_ABORT,
> qi_check_fault will return 0, and qi_submit_sync will then
> execute in an infinite loop and cause a hard lockup when
> interrupts are disabled and the PCIe device does not respond to
> Device-TLB Invalidation requests.
Yes. This appears a real software bug.
>
> Additionally, if the device remains online and an IOMMU ITE
> occurs, simply returning -EAGAIN is sufficient. When processing
> the -EAGAIN result, qi_submit_sync will automatically reclaim
> all submitted struct qi_desc and resubmit the requests.
>
> Through this modification:
> 1. Correctly triggers the resubmission of struct qi_desc when
> an ITE occurs.
> 2. Prevents the IOMMU driver from disabling interrupts and
> executing in an infinite loop within qi_submit_sync when an
> ITE occurs, avoiding hardlockup.
But I think this fix changes the behavior of the driver.
Previously, when an ITE error was detected, it cleared the ITE so that
hardware could keep going, aborted all wait-descriptors that were being
handled by hardware, and returned -EAGAIN if its own wait-descriptor was
impacted.
This patch changes the behavior; it returns -EAGAIN directly whenever it
detects an ITE error, regardless of whether its wait-desc is impacted.
In the single-threaded case, it works as expected, but race condition
might occur when qi_submit_sync() is called in multiple threads at the
same time.
>
> Signed-off-by: Guanghui Feng<guanghuifeng@linux.alibaba.com>
> ---
> drivers/iommu/intel/dmar.c | 18 +++---------------
> 1 file changed, 3 insertions(+), 15 deletions(-)
Have you tried to fix it by dropping the "odd position" assumption? For
example, removing "head |= 1" and decrementing by 1 instead of 2 in the
loop?
do {
if (qi->desc_status[head] == QI_IN_USE)
qi->desc_status[head] = QI_ABORT;
head = (head - 2 + QI_LENGTH) % QI_LENGTH;
} while (head != tail);
Thanks,
baolu
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iommu/vt-d: fix intel iommu iotlb sync hardlockup & retry
2026-02-04 9:32 ` Baolu Lu
@ 2026-02-05 10:28 ` guanghuifeng
2026-02-06 2:55 ` Baolu Lu
2026-02-09 5:17 ` Tian, Kevin
0 siblings, 2 replies; 9+ messages in thread
From: guanghuifeng @ 2026-02-05 10:28 UTC (permalink / raw)
To: Baolu Lu, dwmw2, joro, will, robin.murphy, iommu, linux-kernel; +Cc: xunlei
在 2026/2/4 17:32, Baolu Lu 写道:
> On 2/2/2026 10:09 AM, Guanghui Feng wrote:
>> Device-TLB Invalidation Response Time-out (ITE) handling was added in
>> commit: 6ba6c3a4cacfd68bf970e3e04e2ff0d66fa0f695.
>>
>> When an ITE occurs, iommu will sets the ITE (Invalidation Time-out
>> Error) field in the Fault Status Register. No new descriptors are
>> fetched from the Invalidation Queue until software clears the ITE field
>> in the Fault Status Register. Tail pointer Register updates by software
>> while the ITE field is Set does not cause descriptor fetches by
>> hardware. 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.
>>
>> But in the qi_check_fault process, it is implemented by default
>> according to the 2009 commit: 6ba6c3a4cacfd68bf970e3e04e2ff0d66fa0f695,
>> that is, only one struct qi_desc is submitted at a time. A qi_desc
>> request is
>> immediately followed by a wait_desc/QI_IWD_TYPE for
>> synchronization. Therefore, the IOMMU driver implementation
>> considers invalid queue entries at odd positions to be
>> wait_desc. After ITE is set, hardware aborts any pending
>> inv_wait_dsc commands in hardware. Therefore, qi_check_fault
>> iterates through odd-position as wait_desc entries and sets
>> desc_status to QI_ABORT. However, the current implementation
>> allows multiple struct qi_desc to be submitted simultaneously,
>> followed by one wait_desc, so it's no longer guaranteed that
>> odd-position entries will be wait_desc. When the number of submitted
>> struct qi_desc is even, wait_desc's desc_status will not be set to
>> QI_ABORT,
>> qi_check_fault will return 0, and qi_submit_sync will then
>> execute in an infinite loop and cause a hard lockup when
>> interrupts are disabled and the PCIe device does not respond to
>> Device-TLB Invalidation requests.
>
> Yes. This appears a real software bug.
>
>>
>> Additionally, if the device remains online and an IOMMU ITE
>> occurs, simply returning -EAGAIN is sufficient. When processing
>> the -EAGAIN result, qi_submit_sync will automatically reclaim
>> all submitted struct qi_desc and resubmit the requests.
>>
>> Through this modification:
>> 1. Correctly triggers the resubmission of struct qi_desc when
>> an ITE occurs.
>> 2. Prevents the IOMMU driver from disabling interrupts and
>> executing in an infinite loop within qi_submit_sync when an
>> ITE occurs, avoiding hardlockup.
>
> But I think this fix changes the behavior of the driver.
>
> Previously, when an ITE error was detected, it cleared the ITE so that
> hardware could keep going, aborted all wait-descriptors that were being
> handled by hardware, and returned -EAGAIN if its own wait-descriptor was
> impacted.
>
> This patch changes the behavior; it returns -EAGAIN directly whenever it
> detects an ITE error, regardless of whether its wait-desc is impacted.
> In the single-threaded case, it works as expected, but race condition
> might occur when qi_submit_sync() is called in multiple threads at the
> same time.
>
>>
>> Signed-off-by: Guanghui Feng<guanghuifeng@linux.alibaba.com>
>> ---
>> drivers/iommu/intel/dmar.c | 18 +++---------------
>> 1 file changed, 3 insertions(+), 15 deletions(-)
>
> Have you tried to fix it by dropping the "odd position" assumption? For
> example, removing "head |= 1" and decrementing by 1 instead of 2 in the
> loop?
>
> do {
> if (qi->desc_status[head] == QI_IN_USE)
> qi->desc_status[head] = QI_ABORT;
> head = (head - 2 + QI_LENGTH) % QI_LENGTH;
> } while (head != tail);
>
> Thanks,
> baolu
Thank you for your reply.
There are a few points that need clarification:
The descriptors between head and tail are requests that have not been
fetched and executed.
Regarding the requests before the head:
Method 1: Does the IOMMU update the head address register immediately
after fetching the descriptor?
Method 2: Or does the IOMMU update the head register only after fetching
and executing the request?
The current Intel IOMMU VT-d specification does not describe this
behavior in detail.
Does the IOMMU currently use Method 1?
Therefore, after an ITE timeout, it's necessary to resend the requests
before the head index
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iommu/vt-d: fix intel iommu iotlb sync hardlockup & retry
2026-02-05 10:28 ` guanghuifeng
@ 2026-02-06 2:55 ` Baolu Lu
2026-02-08 10:22 ` guanghuifeng
2026-02-09 5:17 ` Tian, Kevin
1 sibling, 1 reply; 9+ messages in thread
From: Baolu Lu @ 2026-02-06 2:55 UTC (permalink / raw)
To: guanghuifeng, dwmw2, joro, will, robin.murphy, iommu, linux-kernel; +Cc: xunlei
On 2/5/26 18:28, guanghuifeng@linux.alibaba.com wrote:
>
> 在 2026/2/4 17:32, Baolu Lu 写道:
>> On 2/2/2026 10:09 AM, Guanghui Feng wrote:
>>> Device-TLB Invalidation Response Time-out (ITE) handling was added in
>>> commit: 6ba6c3a4cacfd68bf970e3e04e2ff0d66fa0f695.
>>>
>>> When an ITE occurs, iommu will sets the ITE (Invalidation Time-out
>>> Error) field in the Fault Status Register. No new descriptors are
>>> fetched from the Invalidation Queue until software clears the ITE field
>>> in the Fault Status Register. Tail pointer Register updates by software
>>> while the ITE field is Set does not cause descriptor fetches by
>>> hardware. 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.
>>>
>>> But in the qi_check_fault process, it is implemented by default
>>> according to the 2009 commit: 6ba6c3a4cacfd68bf970e3e04e2ff0d66fa0f695,
>>> that is, only one struct qi_desc is submitted at a time. A qi_desc
>>> request is
>>> immediately followed by a wait_desc/QI_IWD_TYPE for
>>> synchronization. Therefore, the IOMMU driver implementation
>>> considers invalid queue entries at odd positions to be
>>> wait_desc. After ITE is set, hardware aborts any pending
>>> inv_wait_dsc commands in hardware. Therefore, qi_check_fault
>>> iterates through odd-position as wait_desc entries and sets
>>> desc_status to QI_ABORT. However, the current implementation
>>> allows multiple struct qi_desc to be submitted simultaneously,
>>> followed by one wait_desc, so it's no longer guaranteed that
>>> odd-position entries will be wait_desc. When the number of submitted
>>> struct qi_desc is even, wait_desc's desc_status will not be set to
>>> QI_ABORT,
>>> qi_check_fault will return 0, and qi_submit_sync will then
>>> execute in an infinite loop and cause a hard lockup when
>>> interrupts are disabled and the PCIe device does not respond to
>>> Device-TLB Invalidation requests.
>>
>> Yes. This appears a real software bug.
>>
>>>
>>> Additionally, if the device remains online and an IOMMU ITE
>>> occurs, simply returning -EAGAIN is sufficient. When processing
>>> the -EAGAIN result, qi_submit_sync will automatically reclaim
>>> all submitted struct qi_desc and resubmit the requests.
>>>
>>> Through this modification:
>>> 1. Correctly triggers the resubmission of struct qi_desc when
>>> an ITE occurs.
>>> 2. Prevents the IOMMU driver from disabling interrupts and
>>> executing in an infinite loop within qi_submit_sync when an
>>> ITE occurs, avoiding hardlockup.
>>
>> But I think this fix changes the behavior of the driver.
>>
>> Previously, when an ITE error was detected, it cleared the ITE so that
>> hardware could keep going, aborted all wait-descriptors that were being
>> handled by hardware, and returned -EAGAIN if its own wait-descriptor was
>> impacted.
>>
>> This patch changes the behavior; it returns -EAGAIN directly whenever it
>> detects an ITE error, regardless of whether its wait-desc is impacted.
>> In the single-threaded case, it works as expected, but race condition
>> might occur when qi_submit_sync() is called in multiple threads at the
>> same time.
>>
>>>
>>> Signed-off-by: Guanghui Feng<guanghuifeng@linux.alibaba.com>
>>> ---
>>> drivers/iommu/intel/dmar.c | 18 +++---------------
>>> 1 file changed, 3 insertions(+), 15 deletions(-)
>>
>> Have you tried to fix it by dropping the "odd position" assumption? For
>> example, removing "head |= 1" and decrementing by 1 instead of 2 in the
>> loop?
>>
>> do {
>> if (qi->desc_status[head] == QI_IN_USE)
>> qi->desc_status[head] = QI_ABORT;
>> head = (head - 2 + QI_LENGTH) % QI_LENGTH;
>> } while (head != tail);
>>
>> Thanks,
>> baolu
>
> Thank you for your reply.
>
> There are a few points that need clarification:
> The descriptors between head and tail are requests that have not been
> fetched and executed.
>
>
> Regarding the requests before the head:
> Method 1: Does the IOMMU update the head address register immediately
> after fetching the descriptor?
> Method 2: Or does the IOMMU update the head register only after fetching
> and executing the request?
>
> The current Intel IOMMU VT-d specification does not describe this
> behavior in detail.
> Does the IOMMU currently use Method 1?
>
> Therefore, after an ITE timeout, it's necessary to resend the requests
> before the head index
An obvious race that I can think of is something like this:
Thread A placed a dev-tlb-inv-desc in the invalidation queue. After
that, thread B placed an iotlb-inv-desc in the queue. Now the requests
in the queue look like this:
dev-tlb-inv-desc for A
iotlb-inv-desc for B
Then a device TLB invalidation timeout error happens and triggers the
ITE bit to be set in the fault register. Thread B sees this in its
qi_check_fault(), clears the ITE bit, and returns -EAGAIN. Then thread A
will loop infinitely waiting for DONE in its wait-desc.
The qi_submit_sync() logic has been there for years. Changing its
behavior without enough validation on real hardware will cause
unexpected issues. The better approach I would suggest is to fix the
outdated logic.
Thanks,
baolu
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] iommu/vt-d: fix intel iommu iotlb sync hardlockup & retry
2026-02-06 2:55 ` Baolu Lu
@ 2026-02-08 10:22 ` guanghuifeng
2026-02-09 5:52 ` Tian, Kevin
0 siblings, 1 reply; 9+ messages in thread
From: guanghuifeng @ 2026-02-08 10:22 UTC (permalink / raw)
To: Baolu Lu, dwmw2, joro, will, robin.murphy, iommu, linux-kernel; +Cc: xunlei
在 2026/2/6 10:55, Baolu Lu 写道:
> On 2/5/26 18:28, guanghuifeng@linux.alibaba.com wrote:
>>
>> 在 2026/2/4 17:32, Baolu Lu 写道:
>>> On 2/2/2026 10:09 AM, Guanghui Feng wrote:
>>>> Device-TLB Invalidation Response Time-out (ITE) handling was added in
>>>> commit: 6ba6c3a4cacfd68bf970e3e04e2ff0d66fa0f695.
>>>>
>>>> When an ITE occurs, iommu will sets the ITE (Invalidation Time-out
>>>> Error) field in the Fault Status Register. No new descriptors are
>>>> fetched from the Invalidation Queue until software clears the ITE
>>>> field
>>>> in the Fault Status Register. Tail pointer Register updates by
>>>> software
>>>> while the ITE field is Set does not cause descriptor fetches by
>>>> hardware. 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.
>>>>
>>>> But in the qi_check_fault process, it is implemented by default
>>>> according to the 2009 commit:
>>>> 6ba6c3a4cacfd68bf970e3e04e2ff0d66fa0f695,
>>>> that is, only one struct qi_desc is submitted at a time. A qi_desc
>>>> request is
>>>> immediately followed by a wait_desc/QI_IWD_TYPE for
>>>> synchronization. Therefore, the IOMMU driver implementation
>>>> considers invalid queue entries at odd positions to be
>>>> wait_desc. After ITE is set, hardware aborts any pending
>>>> inv_wait_dsc commands in hardware. Therefore, qi_check_fault
>>>> iterates through odd-position as wait_desc entries and sets
>>>> desc_status to QI_ABORT. However, the current implementation
>>>> allows multiple struct qi_desc to be submitted simultaneously,
>>>> followed by one wait_desc, so it's no longer guaranteed that
>>>> odd-position entries will be wait_desc. When the number of submitted
>>>> struct qi_desc is even, wait_desc's desc_status will not be set to
>>>> QI_ABORT,
>>>> qi_check_fault will return 0, and qi_submit_sync will then
>>>> execute in an infinite loop and cause a hard lockup when
>>>> interrupts are disabled and the PCIe device does not respond to
>>>> Device-TLB Invalidation requests.
>>>
>>> Yes. This appears a real software bug.
>>>
>>>>
>>>> Additionally, if the device remains online and an IOMMU ITE
>>>> occurs, simply returning -EAGAIN is sufficient. When processing
>>>> the -EAGAIN result, qi_submit_sync will automatically reclaim
>>>> all submitted struct qi_desc and resubmit the requests.
>>>>
>>>> Through this modification:
>>>> 1. Correctly triggers the resubmission of struct qi_desc when
>>>> an ITE occurs.
>>>> 2. Prevents the IOMMU driver from disabling interrupts and
>>>> executing in an infinite loop within qi_submit_sync when an
>>>> ITE occurs, avoiding hardlockup.
>>>
>>> But I think this fix changes the behavior of the driver.
>>>
>>> Previously, when an ITE error was detected, it cleared the ITE so that
>>> hardware could keep going, aborted all wait-descriptors that were being
>>> handled by hardware, and returned -EAGAIN if its own wait-descriptor
>>> was
>>> impacted.
>>>
>>> This patch changes the behavior; it returns -EAGAIN directly
>>> whenever it
>>> detects an ITE error, regardless of whether its wait-desc is impacted.
>>> In the single-threaded case, it works as expected, but race condition
>>> might occur when qi_submit_sync() is called in multiple threads at the
>>> same time.
>>>
>>>>
>>>> Signed-off-by: Guanghui Feng<guanghuifeng@linux.alibaba.com>
>>>> ---
>>>> drivers/iommu/intel/dmar.c | 18 +++---------------
>>>> 1 file changed, 3 insertions(+), 15 deletions(-)
>>>
>>> Have you tried to fix it by dropping the "odd position" assumption? For
>>> example, removing "head |= 1" and decrementing by 1 instead of 2 in the
>>> loop?
>>>
>>> do {
>>> if (qi->desc_status[head] == QI_IN_USE)
>>> qi->desc_status[head] = QI_ABORT;
>>> head = (head - 2 + QI_LENGTH) % QI_LENGTH;
>>> } while (head != tail);
>>>
>>> Thanks,
>>> baolu
>>
>> Thank you for your reply.
>>
>> There are a few points that need clarification:
>> The descriptors between head and tail are requests that have not been
>> fetched and executed.
>>
>>
>> Regarding the requests before the head:
>> Method 1: Does the IOMMU update the head address register immediately
>> after fetching the descriptor?
>> Method 2: Or does the IOMMU update the head register only after
>> fetching and executing the request?
>>
>> The current Intel IOMMU VT-d specification does not describe this
>> behavior in detail.
>> Does the IOMMU currently use Method 1?
>>
>> Therefore, after an ITE timeout, it's necessary to resend the
>> requests before the head index
>
> An obvious race that I can think of is something like this:
>
> Thread A placed a dev-tlb-inv-desc in the invalidation queue. After
> that, thread B placed an iotlb-inv-desc in the queue. Now the requests
> in the queue look like this:
>
> dev-tlb-inv-desc for A
> iotlb-inv-desc for B
>
> Then a device TLB invalidation timeout error happens and triggers the
> ITE bit to be set in the fault register. Thread B sees this in its
> qi_check_fault(), clears the ITE bit, and returns -EAGAIN. Then thread A
> will loop infinitely waiting for DONE in its wait-desc.
>
> The qi_submit_sync() logic has been there for years. Changing its
> behavior without enough validation on real hardware will cause
> unexpected issues. The better approach I would suggest is to fix the
> outdated logic.
>
> Thanks,
> baolu
Thank you for your reply.
From the IOMMU VT-d documentation, it is known that the IOTLB maintenance
process has sequential dependencies, for example, a context IOTLB flush must
precede a PASID IOTLB flush. To ensure the execution order:
Method 1: Requests in the invalid queue are executed sequentially,
requiring
only that the software submission order is valid. The IOMMU updates the
head pointer only after completing a request.
Method 2: Requests in the invalid queue are executed in parallel and out
of order.
The software ensures the execution order of multiple requests by adding
wait_desc entries.
If the Intel IOMMU uses Method 1, and only updates the head pointer upon
completion
of a request (even if a timeout occurs during the execution process
after fetching the request),
then after an ITE timeout, clearing the ITE status in any context will
trigger the IOMMU to re-fetch
and execute the request, without requiring resubmission of the
descriptor request.
Therefore, could you please help me to confirm the current behavior
pattern of the Intel IOMMU execution process?
Thanks
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] iommu/vt-d: fix intel iommu iotlb sync hardlockup & retry
2026-02-05 10:28 ` guanghuifeng
2026-02-06 2:55 ` Baolu Lu
@ 2026-02-09 5:17 ` Tian, Kevin
2026-02-09 7:59 ` [PATCH v2] iommu/vt-d: fix intel iommu iotlb sync hardlockup and retry Guanghui Feng
1 sibling, 1 reply; 9+ messages in thread
From: Tian, Kevin @ 2026-02-09 5:17 UTC (permalink / raw)
To: guanghuifeng, Baolu Lu, dwmw2, joro, will, robin.murphy, iommu,
linux-kernel
Cc: xunlei
> From: guanghuifeng@linux.alibaba.com <guanghuifeng@linux.alibaba.com>
> Sent: Thursday, February 5, 2026 6:28 PM
>
> There are a few points that need clarification:
> The descriptors between head and tail are requests that have not been
> fetched and executed.
>
>
> Regarding the requests before the head:
> Method 1: Does the IOMMU update the head address register immediately
> after fetching the descriptor?
> Method 2: Or does the IOMMU update the head register only after fetching
> and executing the request?
>
> The current Intel IOMMU VT-d specification does not describe this
> behavior in detail.
> Does the IOMMU currently use Method 1?
>
It's clearly documented in the VT-d spec, 6.5.2 Queued Invalidation
Interface:
"
Invalidation Queue Head Register: This register points to the invalidation
descriptor in the IQ that hardware will process next. The Invalidation
Queue Head register is incremented by hardware after fetching a valid
descriptor from the IQ.
"
"
Hardware implementations may fetch one or more descriptors together.
However, hardware must increment the Invalidation Queue Head Register
only after verifying the fetched descriptor to be valid.
"
the wait descriptor is for synchronizing with hardware completion.
^ permalink raw reply [flat|nested] 9+ messages in thread
* RE: [PATCH] iommu/vt-d: fix intel iommu iotlb sync hardlockup & retry
2026-02-08 10:22 ` guanghuifeng
@ 2026-02-09 5:52 ` Tian, Kevin
0 siblings, 0 replies; 9+ messages in thread
From: Tian, Kevin @ 2026-02-09 5:52 UTC (permalink / raw)
To: guanghuifeng, Baolu Lu, dwmw2, joro, will, robin.murphy, iommu,
linux-kernel
Cc: xunlei
> From: guanghuifeng@linux.alibaba.com <guanghuifeng@linux.alibaba.com>
> Sent: Sunday, February 8, 2026 6:23 PM
>
> 在 2026/2/6 10:55, Baolu Lu 写道:
> >
> > An obvious race that I can think of is something like this:
> >
> > Thread A placed a dev-tlb-inv-desc in the invalidation queue. After
> > that, thread B placed an iotlb-inv-desc in the queue. Now the requests
> > in the queue look like this:
> >
> > dev-tlb-inv-desc for A
> > iotlb-inv-desc for B
> >
> > Then a device TLB invalidation timeout error happens and triggers the
> > ITE bit to be set in the fault register. Thread B sees this in its
> > qi_check_fault(), clears the ITE bit, and returns -EAGAIN. Then thread A
> > will loop infinitely waiting for DONE in its wait-desc.
loop infinitely or exit the loop by happening to capture the next ITE
from newer submissions...
> >
> > The qi_submit_sync() logic has been there for years. Changing its
> > behavior without enough validation on real hardware will cause
> > unexpected issues. The better approach I would suggest is to fix the
> > outdated logic.
> >
> > Thanks,
> > baolu
>
> Thank you for your reply.
>
> From the IOMMU VT-d documentation, it is known that the IOTLB
> maintenance
> process has sequential dependencies, for example, a context IOTLB flush
> must
> precede a PASID IOTLB flush. To ensure the execution order:
>
>
> Method 1: Requests in the invalid queue are executed sequentially,
> requiring
> only that the software submission order is valid. The IOMMU updates the
> head pointer only after completing a request.
>
>
> Method 2: Requests in the invalid queue are executed in parallel and out
> of order.
> The software ensures the execution order of multiple requests by adding
> wait_desc entries.
this is what the spec describes. Or more accurately it talks about the
hardware *fetching* one or more descriptors together, likely implying
the possibility of parallel execution.
>
> If the Intel IOMMU uses Method 1, and only updates the head pointer upon
> completion
> of a request (even if a timeout occurs during the execution process
> after fetching the request),
> then after an ITE timeout, clearing the ITE status in any context will
> trigger the IOMMU to re-fetch
> and execute the request, without requiring resubmission of the
> descriptor request.
>
> Therefore, could you please help me to confirm the current behavior
> pattern of the Intel IOMMU execution process?
>
the spec is already clear about that process and the current sw flow (
though with a bug as Baolu pointed out) matches that process.
Multiple CPUs may be submitting descriptors to the invalidation
queue in parallel and in the poll loop (polling the wait status to
be QI_DONE or QI_ABORT), but there is only one CPU receiving
the ITE timeout interrupt.
The current logic is to have the CPU serving the interrupt to
update all wait descriptors before the head pointer to QI_ABORT,
effectively aborting all waiting CPUs to resubmit their descriptors.
that is why there is a check at the start of qi_check_fault():
if (qi->desc_status[wait_index] == QI_ABORT)
return -EAGAIN;
It's broken now after the driver allows submitting multiple
descriptors in a batch, which breaks the assumption that the
wait descriptor always sits in the odd slots.
But your change skips that step by simply returning -EAGAIN on
that only CPU, with all other CPUs still in the wait loop as their
wait descriptor status is still QI_IN_USE.
this patch may work for you in a scenario where new descriptors
are continuously queued and new ITE timeout happens to be
triggered on those polling CPUs.
but it's not reliable and not a correct fix. you should still try to
update all wait descriptors' status to QI_ABORT.
btw suppose the ITE timeout is defined to be large enough to
capture the max possible PCIe ATS invalidation timeout from
device side, but looks in your case the device is still responding
(just for some reason the response is too slow to fit the IOMMU
assumption). that's why retry could succeed?
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] iommu/vt-d: fix intel iommu iotlb sync hardlockup and retry
2026-02-09 5:17 ` Tian, Kevin
@ 2026-02-09 7:59 ` Guanghui Feng
2026-03-06 7:07 ` Baolu Lu
0 siblings, 1 reply; 9+ messages in thread
From: Guanghui Feng @ 2026-02-09 7:59 UTC (permalink / raw)
To: dwmw2, baolu.lu, joro, will, robin.murphy; +Cc: iommu, linux-kernel
Device-TLB Invalidation Response Time-out (ITE) handling was added in
commit: 6ba6c3a4cacf.
When an ITE occurs, iommu will sets the ITE (Invalidation Time-out
Error) field in the Fault Status Register. No new descriptors are
fetched from the Invalidation Queue until software clears the ITE field
in the Fault Status Register. Tail pointer Register updates by software
while the ITE field is Set does not cause descriptor fetches by
hardware. 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.
But in the qi_check_fault process, it is implemented by default
according to the 2009 commit: 6ba6c3a4cacf, that is, only one
struct qi_desc is submitted at a time. A qi_desc request is
immediately followed by a wait_desc/QI_IWD_TYPE for
synchronization. Therefore, the IOMMU driver implementation
considers invalid queue entries at odd positions to be
wait_desc. After ITE is set, hardware aborts any pending
inv_wait_dsc commands in hardware. Therefore, qi_check_fault
iterates through odd-position as wait_desc entries and sets
desc_status to QI_ABORT. However, the current implementation
allows multiple struct qi_desc to be submitted simultaneously,
followed by one wait_desc, so it's no longer guaranteed that
odd-position entries will be wait_desc. When the number of submitted
struct qi_desc is even, wait_desc's desc_status will not be set to QI_ABORT,
qi_check_fault will return 0, and qi_submit_sync will then
execute in an infinite loop and cause a hard lockup when
interrupts are disabled and the PCIe device does not respond to
Device-TLB Invalidation requests.
Additionally, if the device remains online and an IOMMU ITE
occurs, simply returning -EAGAIN is sufficient. When processing
the -EAGAIN result, qi_submit_sync will automatically reclaim
all submitted struct qi_desc and resubmit the requests.
Through this modification:
1. Correctly triggers the resubmission of struct qi_desc when
an ITE occurs.
2. Prevents the IOMMU driver from disabling interrupts and
executing in an infinite loop within qi_submit_sync when an
3. Correctly handling simultaneous requests from multiple CPUs
and multiple contexts that result in timeouts.
Signed-off-by: Guanghui Feng <guanghuifeng@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 ec975c73cfe6..6938800e9884 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v2] iommu/vt-d: fix intel iommu iotlb sync hardlockup and retry
2026-02-09 7:59 ` [PATCH v2] iommu/vt-d: fix intel iommu iotlb sync hardlockup and retry Guanghui Feng
@ 2026-03-06 7:07 ` Baolu Lu
0 siblings, 0 replies; 9+ messages in thread
From: Baolu Lu @ 2026-03-06 7:07 UTC (permalink / raw)
To: Guanghui Feng, dwmw2, joro, will, robin.murphy; +Cc: iommu, linux-kernel
On 2/9/26 15:59, Guanghui Feng wrote:
> Device-TLB Invalidation Response Time-out (ITE) handling was added in
> commit: 6ba6c3a4cacf.
>
> When an ITE occurs, iommu will sets the ITE (Invalidation Time-out
> Error) field in the Fault Status Register. No new descriptors are
> fetched from the Invalidation Queue until software clears the ITE field
> in the Fault Status Register. Tail pointer Register updates by software
> while the ITE field is Set does not cause descriptor fetches by
> hardware. 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.
>
> But in the qi_check_fault process, it is implemented by default
> according to the 2009 commit: 6ba6c3a4cacf, that is, only one
> struct qi_desc is submitted at a time. A qi_desc request is
> immediately followed by a wait_desc/QI_IWD_TYPE for
> synchronization. Therefore, the IOMMU driver implementation
> considers invalid queue entries at odd positions to be
> wait_desc. After ITE is set, hardware aborts any pending
> inv_wait_dsc commands in hardware. Therefore, qi_check_fault
> iterates through odd-position as wait_desc entries and sets
> desc_status to QI_ABORT. However, the current implementation
> allows multiple struct qi_desc to be submitted simultaneously,
> followed by one wait_desc, so it's no longer guaranteed that
> odd-position entries will be wait_desc. When the number of submitted
> struct qi_desc is even, wait_desc's desc_status will not be set to QI_ABORT,
> qi_check_fault will return 0, and qi_submit_sync will then
> execute in an infinite loop and cause a hard lockup when
> interrupts are disabled and the PCIe device does not respond to
> Device-TLB Invalidation requests.
---
> Additionally, if the device remains online and an IOMMU ITE
> occurs, simply returning -EAGAIN is sufficient. When processing
> the -EAGAIN result, qi_submit_sync will automatically reclaim
> all submitted struct qi_desc and resubmit the requests.
>
> Through this modification:
> 1. Correctly triggers the resubmission of struct qi_desc when
> an ITE occurs.
> 2. Prevents the IOMMU driver from disabling interrupts and
> executing in an infinite loop within qi_submit_sync when an
> 3. Correctly handling simultaneous requests from multiple CPUs
> and multiple contexts that result in timeouts.
---
The two paragraphs above don't match the code. Would you mind cleaning
them up?
>
> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
Fixes: 8a1d82462540 ("iommu/vt-d: Multiple descriptors per
qi_submit_sync()")
Cc: stable@vger.kernel.org
> ---
> 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 ec975c73cfe6..6938800e9884 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);
>
> /*
The code itself looks good to me.
Thanks,
baolu
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-03-06 7:08 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-02-02 2:09 [PATCH] iommu/vt-d: fix intel iommu iotlb sync hardlockup & retry Guanghui Feng
2026-02-04 9:32 ` Baolu Lu
2026-02-05 10:28 ` guanghuifeng
2026-02-06 2:55 ` Baolu Lu
2026-02-08 10:22 ` guanghuifeng
2026-02-09 5:52 ` Tian, Kevin
2026-02-09 5:17 ` Tian, Kevin
2026-02-09 7:59 ` [PATCH v2] iommu/vt-d: fix intel iommu iotlb sync hardlockup and retry Guanghui Feng
2026-03-06 7:07 ` Baolu Lu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®