* [PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range
@ 2026-08-05 4:20 Guanghui Feng
2026-08-20 3:09 ` Baolu Lu
0 siblings, 1 reply; 6+ messages in thread
From: Guanghui Feng @ 2026-08-05 4:20 UTC (permalink / raw)
To: dwmw2, baolu.lu, joro, will, robin.murphy; +Cc: iommu, linux-kernel
Currently, qi_check_fault() only handles IQE (Invalidation Queue Error)
when the faulting descriptor index exactly matches the first descriptor
of the current submission (head == index). This is too restrictive in
multi-descriptor submissions where the error could occur at any
descriptor within the batch.
If the IQE is triggered by a descriptor that belongs to the current
submission but is not at the starting index, the function returns 0
without clearing the IQE fault status. Since hardware stops fetching
new descriptors until IQE is cleared, this leads to an indefinite wait
on the wait descriptor completion - effectively a deadlock.
Fix this by expanding the IQE handling condition to cover all descriptors
within the circular range [index, wait_index]. Use explicit bounds
checking that properly handles the wrap-around case of the circular
queue.
Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
Signed-off-by: bikuan.zbk <bikuan.zbk@alibaba-inc.com>
---
drivers/iommu/intel/dmar.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index 767ec092accd..8ae513593406 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1290,8 +1290,13 @@ static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
* is cleared.
*/
if (fault & DMA_FSTS_IQE) {
+ int head_idx;
+
head = readl(iommu->reg + DMAR_IQH_REG);
- if ((head >> shift) == index) {
+ head_idx = head >> shift;
+ if (index <= wait_index ?
+ (head_idx >= index && head_idx <= wait_index) :
+ (head_idx >= index || head_idx <= wait_index)) {
struct qi_desc *desc = qi->desc + head;
/*
--
2.43.7
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range
2026-08-05 4:20 [PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range Guanghui Feng
@ 2026-08-20 3:09 ` Baolu Lu
2026-08-20 14:47 ` Guanghui Feng
0 siblings, 1 reply; 6+ messages in thread
From: Baolu Lu @ 2026-08-20 3:09 UTC (permalink / raw)
To: Guanghui Feng, dwmw2, joro, will, robin.murphy; +Cc: iommu, linux-kernel
On 8/5/26 12:20, Guanghui Feng wrote:
> Currently, qi_check_fault() only handles IQE (Invalidation Queue Error)
> when the faulting descriptor index exactly matches the first descriptor
> of the current submission (head == index). This is too restrictive in
> multi-descriptor submissions where the error could occur at any
> descriptor within the batch.
>
> If the IQE is triggered by a descriptor that belongs to the current
> submission but is not at the starting index, the function returns 0
> without clearing the IQE fault status. Since hardware stops fetching
> new descriptors until IQE is cleared, this leads to an indefinite wait
> on the wait descriptor completion - effectively a deadlock.
>
> Fix this by expanding the IQE handling condition to cover all descriptors
> within the circular range [index, wait_index]. Use explicit bounds
> checking that properly handles the wrap-around case of the circular
> queue.
>
Fixes: 8a1d82462540 ("iommu/vt-d: Multiple descriptors per
qi_submit_sync()")
Cc: stable@vger.kernel.org
> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> Signed-off-by: bikuan.zbk <bikuan.zbk@alibaba-inc.com>
> ---
> drivers/iommu/intel/dmar.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index 767ec092accd..8ae513593406 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -1290,8 +1290,13 @@ static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
> * is cleared.
> */
> if (fault & DMA_FSTS_IQE) {
> + int head_idx;
> +
> head = readl(iommu->reg + DMAR_IQH_REG);
> - if ((head >> shift) == index) {
> + head_idx = head >> shift;
How about adding a brief comment like this?
/*
* The faulting descriptor can be anywhere within the current
* submission's range [index, wait_index]. Since the queue is
* circular, this submission may wrap around QI_LENGTH
* (index > wait_index in that case), so check both the
* non-wrapped and wrapped cases of the range.
*/
> + if (index <= wait_index ?
> + (head_idx >= index && head_idx <= wait_index) :
> + (head_idx >= index || head_idx <= wait_index)) {
> struct qi_desc *desc = qi->desc + head;
>
> /*
Otherwise, this looks good to me.
Thanks,
baolu
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range
2026-08-20 3:09 ` Baolu Lu
@ 2026-08-20 14:47 ` Guanghui Feng
2026-08-20 19:37 ` Samiullah Khawaja
2026-08-21 2:56 ` Baolu Lu
0 siblings, 2 replies; 6+ messages in thread
From: Guanghui Feng @ 2026-08-20 14:47 UTC (permalink / raw)
To: baolu.lu
Cc: dwmw2, guanghuifeng, iommu, joro, linux-kernel, robin.murphy,
will, bikuan . zbk
Currently, qi_check_fault() only handles IQE (Invalidation Queue Error)
when the faulting descriptor index exactly matches the first descriptor
of the current submission (head == index). This is too restrictive in
multi-descriptor submissions where the error could occur at any
descriptor within the batch.
If the IQE is triggered by a descriptor that belongs to the current
submission but is not at the starting index, the function returns 0
without clearing the IQE fault status. Since hardware stops fetching
new descriptors until IQE is cleared, this leads to an indefinite wait
on the wait descriptor completion - effectively a deadlock.
Fix this by expanding the IQE handling condition to cover all descriptors
within the circular range [index, wait_index]. Use explicit bounds
checking that properly handles the wrap-around case of the circular
queue.
Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
Signed-off-by: bikuan.zbk <bikuan.zbk@alibaba-inc.com>
---
drivers/iommu/intel/dmar.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index ba675b08cd20..ecc95af06f61 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1366,8 +1366,21 @@ static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
* is cleared.
*/
if (fault & DMA_FSTS_IQE) {
+ int head_idx;
+
head = readl(iommu->reg + DMAR_IQH_REG);
- if ((head >> shift) == index) {
+ head_idx = head >> shift;
+
+ /*
+ * The faulting descriptor can be anywhere within the current
+ * submission's range [index, wait_index]. Since the queue is
+ * circular, this submission may wrap around QI_LENGTH
+ * (index > wait_index in that case), so check both the
+ * non-wrapped and wrapped cases of the range.
+ */
+ if (index <= wait_index ?
+ (head_idx >= index && head_idx <= wait_index) :
+ (head_idx >= index || head_idx <= wait_index)) {
struct qi_desc *desc = qi->desc + head;
/*
--
2.43.7
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range
2026-08-20 14:47 ` Guanghui Feng
@ 2026-08-20 19:37 ` Samiullah Khawaja
2026-08-21 2:56 ` Baolu Lu
1 sibling, 0 replies; 6+ messages in thread
From: Samiullah Khawaja @ 2026-08-20 19:37 UTC (permalink / raw)
To: Guanghui Feng
Cc: baolu.lu, dwmw2, iommu, joro, linux-kernel, robin.murphy, will,
bikuan . zbk
On Thu, Aug 20, 2026 at 10:47:41PM +0800, Guanghui Feng wrote:
>Currently, qi_check_fault() only handles IQE (Invalidation Queue Error)
>when the faulting descriptor index exactly matches the first descriptor
>of the current submission (head == index). This is too restrictive in
>multi-descriptor submissions where the error could occur at any
>descriptor within the batch.
>
>If the IQE is triggered by a descriptor that belongs to the current
>submission but is not at the starting index, the function returns 0
>without clearing the IQE fault status. Since hardware stops fetching
>new descriptors until IQE is cleared, this leads to an indefinite wait
>on the wait descriptor completion - effectively a deadlock.
>
>Fix this by expanding the IQE handling condition to cover all descriptors
>within the circular range [index, wait_index]. Use explicit bounds
>checking that properly handles the wrap-around case of the circular
>queue.
>
>Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
>Signed-off-by: bikuan.zbk <bikuan.zbk@alibaba-inc.com>
>---
> drivers/iommu/intel/dmar.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
>index ba675b08cd20..ecc95af06f61 100644
>--- a/drivers/iommu/intel/dmar.c
>+++ b/drivers/iommu/intel/dmar.c
>@@ -1366,8 +1366,21 @@ static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
> * is cleared.
> */
> if (fault & DMA_FSTS_IQE) {
>+ int head_idx;
>+
> head = readl(iommu->reg + DMAR_IQH_REG);
>- if ((head >> shift) == index) {
>+ head_idx = head >> shift;
>+
>+ /*
>+ * The faulting descriptor can be anywhere within the current
>+ * submission's range [index, wait_index]. Since the queue is
>+ * circular, this submission may wrap around QI_LENGTH
>+ * (index > wait_index in that case), so check both the
>+ * non-wrapped and wrapped cases of the range.
>+ */
>+ if (index <= wait_index ?
>+ (head_idx >= index && head_idx <= wait_index) :
>+ (head_idx >= index || head_idx <= wait_index)) {
> struct qi_desc *desc = qi->desc + head;
>
> /*
>--
>2.43.7
>
>
Reviewed-by: Samiullah Khawaja <skhawaja@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range
2026-08-20 14:47 ` Guanghui Feng
2026-08-20 19:37 ` Samiullah Khawaja
@ 2026-08-21 2:56 ` Baolu Lu
2026-09-09 7:13 ` Baolu Lu
1 sibling, 1 reply; 6+ messages in thread
From: Baolu Lu @ 2026-08-21 2:56 UTC (permalink / raw)
To: Guanghui Feng
Cc: dwmw2, iommu, joro, linux-kernel, robin.murphy, will, bikuan . zbk
On 8/20/26 22:47, Guanghui Feng wrote:
> Currently, qi_check_fault() only handles IQE (Invalidation Queue Error)
> when the faulting descriptor index exactly matches the first descriptor
> of the current submission (head == index). This is too restrictive in
> multi-descriptor submissions where the error could occur at any
> descriptor within the batch.
>
> If the IQE is triggered by a descriptor that belongs to the current
> submission but is not at the starting index, the function returns 0
> without clearing the IQE fault status. Since hardware stops fetching
> new descriptors until IQE is cleared, this leads to an indefinite wait
> on the wait descriptor completion - effectively a deadlock.
>
> Fix this by expanding the IQE handling condition to cover all descriptors
> within the circular range [index, wait_index]. Use explicit bounds
> checking that properly handles the wrap-around case of the circular
> queue.
>
> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> Signed-off-by: bikuan.zbk <bikuan.zbk@alibaba-inc.com>
> ---
> drivers/iommu/intel/dmar.c | 15 ++++++++++++++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index ba675b08cd20..ecc95af06f61 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -1366,8 +1366,21 @@ static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
> * is cleared.
> */
> if (fault & DMA_FSTS_IQE) {
> + int head_idx;
> +
> head = readl(iommu->reg + DMAR_IQH_REG);
> - if ((head >> shift) == index) {
> + head_idx = head >> shift;
> +
> + /*
> + * The faulting descriptor can be anywhere within the current
> + * submission's range [index, wait_index]. Since the queue is
> + * circular, this submission may wrap around QI_LENGTH
> + * (index > wait_index in that case), so check both the
> + * non-wrapped and wrapped cases of the range.
> + */
> + if (index <= wait_index ?
> + (head_idx >= index && head_idx <= wait_index) :
> + (head_idx >= index || head_idx <= wait_index)) {
> struct qi_desc *desc = qi->desc + head;
>
> /*
Could you also please take a look at the comments from Sashiko?
https://sashiko.dev/#/patchset/20260805042012.2363698-1-guanghuifeng%40linux.alibaba.com
https://sashiko.dev/#/patchset/20260820144741.920858-1-guanghuifeng%40linux.alibaba.com
No worries about the pre-existing issues. I’ll take care of them.
Thanks,
baolu
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range
2026-08-21 2:56 ` Baolu Lu
@ 2026-09-09 7:13 ` Baolu Lu
0 siblings, 0 replies; 6+ messages in thread
From: Baolu Lu @ 2026-09-09 7:13 UTC (permalink / raw)
To: Guanghui Feng
Cc: dwmw2, iommu, joro, linux-kernel, robin.murphy, will, bikuan . zbk
On 8/21/2026 10:56 AM, Baolu Lu wrote:
> On 8/20/26 22:47, Guanghui Feng wrote:
>> Currently, qi_check_fault() only handles IQE (Invalidation Queue Error)
>> when the faulting descriptor index exactly matches the first descriptor
>> of the current submission (head == index). This is too restrictive in
>> multi-descriptor submissions where the error could occur at any
>> descriptor within the batch.
>>
>> If the IQE is triggered by a descriptor that belongs to the current
>> submission but is not at the starting index, the function returns 0
>> without clearing the IQE fault status. Since hardware stops fetching
>> new descriptors until IQE is cleared, this leads to an indefinite wait
>> on the wait descriptor completion - effectively a deadlock.
>>
>> Fix this by expanding the IQE handling condition to cover all descriptors
>> within the circular range [index, wait_index]. Use explicit bounds
>> checking that properly handles the wrap-around case of the circular
>> queue.
>>
>> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
>> Signed-off-by: bikuan.zbk <bikuan.zbk@alibaba-inc.com>
>> ---
>> drivers/iommu/intel/dmar.c | 15 ++++++++++++++-
>> 1 file changed, 14 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
>> index ba675b08cd20..ecc95af06f61 100644
>> --- a/drivers/iommu/intel/dmar.c
>> +++ b/drivers/iommu/intel/dmar.c
>> @@ -1366,8 +1366,21 @@ static int qi_check_fault(struct intel_iommu
>> *iommu, int index, int wait_index)
>> * is cleared.
>> */
>> if (fault & DMA_FSTS_IQE) {
>> + int head_idx;
>> +
>> head = readl(iommu->reg + DMAR_IQH_REG);
>> - if ((head >> shift) == index) {
>> + head_idx = head >> shift;
>> +
>> + /*
>> + * The faulting descriptor can be anywhere within the current
>> + * submission's range [index, wait_index]. Since the queue is
>> + * circular, this submission may wrap around QI_LENGTH
>> + * (index > wait_index in that case), so check both the
>> + * non-wrapped and wrapped cases of the range.
>> + */
>> + if (index <= wait_index ?
>> + (head_idx >= index && head_idx <= wait_index) :
>> + (head_idx >= index || head_idx <= wait_index)) {
>> struct qi_desc *desc = qi->desc + head;
>> /*
>
> Could you also please take a look at the comments from Sashiko?
>
> https://sashiko.dev/#/patchset/20260805042012.2363698-1-
> guanghuifeng%40linux.alibaba.com
>
> https://sashiko.dev/#/patchset/20260820144741.920858-1-
> guanghuifeng%40linux.alibaba.com
I think some points from the Sashiko review are valid, and we should
address them.
For example,
"
When an IQE is cleared and -EINVAL is returned, hardware is unhalted and
continues processing the rest of the abandoned batch. If another
descriptor in that batch faults, hardware halts again. Since the
original thread already returned -EINVAL and left its wait loop, a new
thread checking faults may compare that new IQE against its own
unrelated [new_index, new_wait_index] range, miss the fault, and leave
hardware permanently deadlocked.
"
and
"
When the wait loop aborts, qi_submit_sync() immediately marks the batch
slots as QI_FREE and reclaims them. But hardware may still be processing
descriptors from that batch asynchronously. Reusing those slots too
early allows concurrent overwrite, and hardware can then fetch corrupted
descriptors.
"
So it seems safer to overwrite all remaining slots in the failed batch
(starting from the faulting slot) with a wait descriptor (effectively a
no-op), then wait until hardware consumes them, and only then return
-EINVAL and let the caller abandon the batch.
As for the question, “...what if the hardware returns a large or
negative value?”, the IQ head register is in the root complex and is
expected to remain accessible as long as the platform is operational.
Thanks,
baolu
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-09 7:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-05 4:20 [PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range Guanghui Feng
2026-08-20 3:09 ` Baolu Lu
2026-08-20 14:47 ` Guanghui Feng
2026-08-20 19:37 ` Samiullah Khawaja
2026-08-21 2:56 ` Baolu Lu
2026-09-09 7:13 ` 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®