[PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range

[PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range

[Guanghui Feng]

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

Re: [PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range

[Baolu Lu]

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

[PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range

[Guanghui Feng]

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

Re: [PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range

[Samiullah Khawaja]

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>

Re: [PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range

[Baolu Lu]

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

Re: [PATCH] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range

[Baolu Lu]

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

[PATCH v3] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range

[Guanghui Feng]

When an Invalidation Queue Error (IQE) occurs, hardware halts fetching
and IQH points at the faulting descriptor. The previous code only
checked whether IQH matched the first descriptor index of the current
submission, missing faults on any other descriptor within the batch.

Expand the check to cover the entire submission range [index, wait_index],
accounting for circular wrap-around.

Furthermore, after detecting IQE, the old recovery only replaced the
single faulting slot with a copy of the wait descriptor and immediately
returned -EINVAL. This left two problems:

  a) Hardware resumed fetching and could hit another invalid descriptor
     in the same abandoned batch, raising a second IQE that no submitter
     would claim — permanently deadlocking the queue.

  b) The caller reclaimed all batch slots (QI_FREE) while hardware might
     still be asynchronously processing descriptors from that batch,
     allowing concurrent overwrite and descriptor corruption.

Fix both by introducing qi_drain_remaining_descs(): upon IQE detection,
overwrite the stranded slots in [IQH, wait_index) with no-op wait
descriptors, resubmit the wait descriptor at wait_index, then clear IQE.
The caller's existing poll loop naturally waits for QI_DONE — which per
VT-d spec §6.5.2.12 is only written after ALL preceding descriptors
complete — guaranteeing hardware has fully drained the batch before
slots are reclaimed.

Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
Signed-off-by: bikuan.zbk <bikuan.zbk@alibaba-inc.com>
---
 drivers/iommu/intel/dmar.c | 78 ++++++++++++++++++++++++++++++--------
 1 file changed, 63 insertions(+), 15 deletions(-)

diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index ba675b08cd20..db92fb247131 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1344,6 +1344,43 @@ static void qi_dump_fault(struct intel_iommu *iommu, u32 fault)
 	       (unsigned long long)desc->qw1);
 }
 
+/**
+ * qi_drain_remaining_descs - drain descriptors stranded by an IQE
+ * @iommu: the affected IOMMU
+ * @wait_index: slot index of the wait descriptor of the current submission
+ * @shift: qi_shift(iommu), converts a slot index into a byte offset
+ * @options: QI_OPT_* flags of the current submission
+ *
+ * Overwrite the slots in [IQH, @wait_index) with no-op descriptors, resubmit
+ * the wait descriptor at @wait_index, and clear IQE so hardware resumes and
+ * eventually signals QI_DONE. Must be called with qi->q_lock held.
+ */
+static void qi_drain_remaining_descs(struct intel_iommu *iommu,
+				     int wait_index, int shift,
+				     unsigned long options)
+{
+	struct q_inval *qi = iommu->qi;
+	struct qi_desc desc;
+	int head_idx;
+	int cur;
+
+	head_idx = (readl(iommu->reg + DMAR_IQH_REG)) >> shift;
+
+	memset(&desc, 0, sizeof(desc));
+	desc.qw0 = QI_IWD_TYPE;
+	for (cur = head_idx; cur != wait_index; cur = (cur + 1) % QI_LENGTH)
+		memcpy(qi->desc + (cur << shift), &desc, 1 << shift);
+
+	desc.qw0 = QI_IWD_STATUS_DATA(QI_DONE) |
+			QI_IWD_STATUS_WRITE | QI_IWD_TYPE;
+	if (options & QI_OPT_WAIT_DRAIN)
+		desc.qw0 |= QI_IWD_PRQ_DRAIN;
+	desc.qw1 = virt_to_phys(&qi->desc_status[wait_index]);
+	memcpy(qi->desc + (wait_index << shift), &desc, 1 << shift);
+
+	writel(DMA_FSTS_IQE, iommu->reg + DMAR_FSTS_REG);
+}
+
 static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
 {
 	u32 fault;
@@ -1366,21 +1403,22 @@ 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) {
-			struct qi_desc *desc = qi->desc + head;
+		head_idx = head >> shift;
 
-			/*
-			 * desc->qw2 and desc->qw3 are either reserved or
-			 * used by software as private data. We won't print
-			 * out these two qw's for security consideration.
-			 */
-			memcpy(desc, qi->desc + (wait_index << shift),
-			       1 << shift);
-			writel(DMA_FSTS_IQE, iommu->reg + DMAR_FSTS_REG);
-			pr_info("Invalidation Queue Error (IQE) cleared\n");
+		/*
+		 * 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))
 			return -EINVAL;
-		}
 	}
 
 	/*
@@ -1452,7 +1490,7 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
 	int wait_index, index;
 	unsigned long flags;
 	int offset, shift;
-	int rc, i;
+	int rc = 0, fault, i;
 	u64 type;
 
 	if (!qi)
@@ -1528,9 +1566,19 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
 		 * a deadlock where the interrupt context can wait indefinitely
 		 * for free slots in the queue.
 		 */
-		rc = qi_check_fault(iommu, index, wait_index);
-		if (rc)
+		fault = qi_check_fault(iommu, index, wait_index);
+		if (fault == -EINVAL) {
+			/*
+			 * IQE in our batch: drain the remaining descriptors
+			 * and keep polling until hardware completes.
+			 */
+			qi_drain_remaining_descs(iommu, wait_index, shift,
+						  options);
+			rc = -EINVAL;
+		} else if (fault) {
+			rc = fault;
 			break;
+		}
 
 		raw_spin_unlock(&qi->q_lock);
 		cpu_relax();
-- 
2.43.7

Re: [PATCH v3] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range

[Baolu Lu]

On 9/14/2026 10:19 PM, Guanghui Feng wrote:
> When an Invalidation Queue Error (IQE) occurs, hardware halts fetching
> and IQH points at the faulting descriptor. The previous code only
> checked whether IQH matched the first descriptor index of the current
> submission, missing faults on any other descriptor within the batch.
> 
> Expand the check to cover the entire submission range [index, wait_index],
> accounting for circular wrap-around.
> 
> Furthermore, after detecting IQE, the old recovery only replaced the
> single faulting slot with a copy of the wait descriptor and immediately
> returned -EINVAL. This left two problems:
> 
>    a) Hardware resumed fetching and could hit another invalid descriptor
>       in the same abandoned batch, raising a second IQE that no submitter
>       would claim — permanently deadlocking the queue.
> 
>    b) The caller reclaimed all batch slots (QI_FREE) while hardware might
>       still be asynchronously processing descriptors from that batch,
>       allowing concurrent overwrite and descriptor corruption.
> 
> Fix both by introducing qi_drain_remaining_descs(): upon IQE detection,
> overwrite the stranded slots in [IQH, wait_index) with no-op wait
> descriptors, resubmit the wait descriptor at wait_index, then clear IQE.
> The caller's existing poll loop naturally waits for QI_DONE — which per
> VT-d spec §6.5.2.12 is only written after ALL preceding descriptors
> complete — guaranteeing hardware has fully drained the batch before
> slots are reclaimed.
> 
> Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
> Signed-off-by: bikuan.zbk <bikuan.zbk@alibaba-inc.com>
> ---
>   drivers/iommu/intel/dmar.c | 78 ++++++++++++++++++++++++++++++--------
>   1 file changed, 63 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
> index ba675b08cd20..db92fb247131 100644
> --- a/drivers/iommu/intel/dmar.c
> +++ b/drivers/iommu/intel/dmar.c
> @@ -1344,6 +1344,43 @@ static void qi_dump_fault(struct intel_iommu *iommu, u32 fault)
>   	       (unsigned long long)desc->qw1);
>   }
>   
> +/**
> + * qi_drain_remaining_descs - drain descriptors stranded by an IQE
> + * @iommu: the affected IOMMU
> + * @wait_index: slot index of the wait descriptor of the current submission
> + * @shift: qi_shift(iommu), converts a slot index into a byte offset
> + * @options: QI_OPT_* flags of the current submission
> + *
> + * Overwrite the slots in [IQH, @wait_index) with no-op descriptors, resubmit
> + * the wait descriptor at @wait_index, and clear IQE so hardware resumes and
> + * eventually signals QI_DONE. Must be called with qi->q_lock held.
> + */
> +static void qi_drain_remaining_descs(struct intel_iommu *iommu,
> +				     int wait_index, int shift,
> +				     unsigned long options)

This is only an internal helper, so there is no need to add a kernel-doc
comment like this.

> +{
> +	struct q_inval *qi = iommu->qi;
> +	struct qi_desc desc;
> +	int head_idx;
> +	int cur;
> +
> +	head_idx = (readl(iommu->reg + DMAR_IQH_REG)) >> shift;
> +
> +	memset(&desc, 0, sizeof(desc));
> +	desc.qw0 = QI_IWD_TYPE;
> +	for (cur = head_idx; cur != wait_index; cur = (cur + 1) % QI_LENGTH)
> +		memcpy(qi->desc + (cur << shift), &desc, 1 << shift);
> +
> +	desc.qw0 = QI_IWD_STATUS_DATA(QI_DONE) |
> +			QI_IWD_STATUS_WRITE | QI_IWD_TYPE;
> +	if (options & QI_OPT_WAIT_DRAIN)
> +		desc.qw0 |= QI_IWD_PRQ_DRAIN;
> +	desc.qw1 = virt_to_phys(&qi->desc_status[wait_index]);
> +	memcpy(qi->desc + (wait_index << shift), &desc, 1 << shift);
> +
> +	writel(DMA_FSTS_IQE, iommu->reg + DMAR_FSTS_REG);
> +}

I would like this helper to be self-contained; that is, it should
replace all abandoned descriptors with wait descriptors and then wait
until hardware fetch passes those descriptors. Something like this (not
tested yet):

+static int qi_drain_remaining_descs(struct intel_iommu *iommu, int head,
+				    int wait_index)
+{
+	struct q_inval *qi = iommu->qi;
+	int shift = qi_shift(iommu);
+	struct qi_desc wait_desc = {};
+	struct qi_desc nop_desc = {};
+	cycles_t start;
+
+	nop_desc.qw0 = QI_IWD_FENCE | QI_IWD_TYPE;
+
+	wait_desc.qw0 = QI_IWD_STATUS_DATA(QI_DONE) |
+			QI_IWD_STATUS_WRITE | QI_IWD_FENCE | QI_IWD_TYPE;
+	wait_desc.qw1 = virt_to_phys(&qi->desc_status[wait_index]);
+
+	while (head != wait_index) {
+		memcpy(qi->desc + (head << shift), &nop_desc, 1 << shift);
+		head = (head + 1) % QI_LENGTH;
+	}
+
+	WRITE_ONCE(qi->desc_status[wait_index], QI_IN_USE);
+	memcpy(qi->desc + (wait_index << shift), &wait_desc, 1 << shift);
+
+	/*
+	 * Order the descriptor rewrites before the writel() that restarts
+	 * the fetch engine.
+	 */
+	wmb();
+
+	writel(DMA_FSTS_IQE, iommu->reg + DMAR_FSTS_REG);
+
+	start = get_cycles();
+	while (READ_ONCE(qi->desc_status[wait_index]) != QI_DONE) {
+		if (DMAR_OPERATION_TIMEOUT < (get_cycles() - start)) {
+			pr_err("Timeout draining invalidation queue after IQE\n");
+			return -ETIMEDOUT;
+		}
+		cpu_relax();
+	}
+
+	return 0;
+}

> +
>   static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
>   {
>   	u32 fault;
> @@ -1366,21 +1403,22 @@ 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) {
> -			struct qi_desc *desc = qi->desc + head;
> +		head_idx = head >> shift;
>   
> -			/*
> -			 * desc->qw2 and desc->qw3 are either reserved or
> -			 * used by software as private data. We won't print
> -			 * out these two qw's for security consideration.
> -			 */
> -			memcpy(desc, qi->desc + (wait_index << shift),
> -			       1 << shift);
> -			writel(DMA_FSTS_IQE, iommu->reg + DMAR_FSTS_REG);
> -			pr_info("Invalidation Queue Error (IQE) cleared\n");
> +		/*
> +		 * 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))
>   			return -EINVAL;
> -		}
>   	}

Then call the above helper directly here.

@@ -1366,18 +1409,25 @@ static int qi_check_fault(struct intel_iommu 
*iommu, int index, int wait_index)
  	 * is cleared.
  	 */
  	if (fault & DMA_FSTS_IQE) {
+		int head_idx, ret;
+
  		head = readl(iommu->reg + DMAR_IQH_REG);
-		if ((head >> shift) == index) {
-			struct qi_desc *desc = qi->desc + head;
+		head_idx = (head >> shift) % QI_LENGTH;
+
+		/*
+		 * 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)) {
+			ret = qi_drain_remaining_descs(iommu, head_idx, wait_index);
+			if (ret)
+				return ret;

-			/*
-			 * desc->qw2 and desc->qw3 are either reserved or
-			 * used by software as private data. We won't print
-			 * out these two qw's for security consideration.
-			 */
-			memcpy(desc, qi->desc + (wait_index << shift),
-			       1 << shift);
-			writel(DMA_FSTS_IQE, iommu->reg + DMAR_FSTS_REG);
  			pr_info("Invalidation Queue Error (IQE) cleared\n");
  			return -EINVAL;
  		}

>   
>   	/*
> @@ -1452,7 +1490,7 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
>   	int wait_index, index;
>   	unsigned long flags;
>   	int offset, shift;
> -	int rc, i;
> +	int rc = 0, fault, i;
>   	u64 type;
>   
>   	if (!qi)
> @@ -1528,9 +1566,19 @@ int qi_submit_sync(struct intel_iommu *iommu, struct qi_desc *desc,
>   		 * a deadlock where the interrupt context can wait indefinitely
>   		 * for free slots in the queue.
>   		 */
> -		rc = qi_check_fault(iommu, index, wait_index);
> -		if (rc)
> +		fault = qi_check_fault(iommu, index, wait_index);
> +		if (fault == -EINVAL) {
> +			/*
> +			 * IQE in our batch: drain the remaining descriptors
> +			 * and keep polling until hardware completes.
> +			 */
> +			qi_drain_remaining_descs(iommu, wait_index, shift,
> +						  options);
> +			rc = -EINVAL;
> +		} else if (fault) {
> +			rc = fault;
>   			break;
> +		}

As a result, there is no need to change the caller of qi_check_fault().

>   
>   		raw_spin_unlock(&qi->q_lock);
>   		cpu_relax();

Thanks,
baolu

[PATCH v4] iommu/vt-d: Fix IQE handling to cover all descriptors in submission range

[Guanghui Feng]

When an Invalidation Queue Error (IQE) occurs, hardware halts fetching
and IQH points at the faulting descriptor. The previous code only
checked whether IQH matched the first descriptor index of the current
submission, missing faults on any other descriptor within the batch.

Expand the check to cover the entire submission range [index, wait_index],
accounting for circular wrap-around.

Furthermore, after detecting IQE, the old recovery only replaced the
single faulting slot with a copy of the wait descriptor and immediately
returned -EINVAL. This left two problems:

  a) Hardware resumed fetching and could hit another invalid descriptor
     in the same abandoned batch, raising a second IQE that no submitter
     would claim — permanently deadlocking the queue.

  b) The caller reclaimed all batch slots (QI_FREE) while hardware might
     still be asynchronously processing descriptors from that batch,
     allowing concurrent overwrite and descriptor corruption.

Fix both by introducing qi_drain_remaining_descs(): upon IQE detection,
overwrite the stranded slots in [IQH, wait_index) with fenced no-op wait
descriptors, resubmit the wait descriptor at wait_index, clear IQE, and
spin until hardware signals QI_DONE (with DMAR_OPERATION_TIMEOUT). This
guarantees hardware has fully drained the batch before the caller
reclaims slots.

Signed-off-by: Guanghui Feng <guanghuifeng@linux.alibaba.com>
---
 drivers/iommu/intel/dmar.c | 73 ++++++++++++++++++++++++++++++++------
 1 file changed, 62 insertions(+), 11 deletions(-)

diff --git a/drivers/iommu/intel/dmar.c b/drivers/iommu/intel/dmar.c
index ba675b08cd20..1aa1bdd796cf 100644
--- a/drivers/iommu/intel/dmar.c
+++ b/drivers/iommu/intel/dmar.c
@@ -1344,6 +1344,49 @@ static void qi_dump_fault(struct intel_iommu *iommu, u32 fault)
 	       (unsigned long long)desc->qw1);
 }
 
+static int qi_drain_remaining_descs(struct intel_iommu *iommu, int head,
+				    int wait_index)
+{
+	struct q_inval *qi = iommu->qi;
+	int shift = qi_shift(iommu);
+	struct qi_desc wait_desc = {};
+	struct qi_desc nop_desc = {};
+	cycles_t start;
+
+	nop_desc.qw0 = QI_IWD_FENCE | QI_IWD_TYPE;
+
+	wait_desc.qw0 = QI_IWD_STATUS_DATA(QI_DONE) | QI_IWD_PRQ_DRAIN |
+			QI_IWD_STATUS_WRITE | QI_IWD_FENCE | QI_IWD_TYPE;
+	wait_desc.qw1 = virt_to_phys(&qi->desc_status[wait_index]);
+
+	while (head != wait_index) {
+		memcpy(qi->desc + (head << shift), &nop_desc, 1 << shift);
+		head = (head + 1) % QI_LENGTH;
+	}
+
+	WRITE_ONCE(qi->desc_status[wait_index], QI_IN_USE);
+	memcpy(qi->desc + (wait_index << shift), &wait_desc, 1 << shift);
+
+	/*
+	 * Order the descriptor rewrites before the writel() that restarts
+	 * the fetch engine.
+	 */
+	wmb();
+
+	writel(DMA_FSTS_IQE, iommu->reg + DMAR_FSTS_REG);
+
+	start = get_cycles();
+	while (READ_ONCE(qi->desc_status[wait_index]) != QI_DONE) {
+		if (DMAR_OPERATION_TIMEOUT < (get_cycles() - start)) {
+			pr_err("Timeout draining invalidation queue after IQE\n");
+			return -ETIMEDOUT;
+		}
+		cpu_relax();
+	}
+
+	return 0;
+}
+
 static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
 {
 	u32 fault;
@@ -1366,18 +1409,26 @@ static int qi_check_fault(struct intel_iommu *iommu, int index, int wait_index)
 	 * is cleared.
 	 */
 	if (fault & DMA_FSTS_IQE) {
+		int head_idx, ret;
+
 		head = readl(iommu->reg + DMAR_IQH_REG);
-		if ((head >> shift) == index) {
-			struct qi_desc *desc = qi->desc + head;
-
-			/*
-			 * desc->qw2 and desc->qw3 are either reserved or
-			 * used by software as private data. We won't print
-			 * out these two qw's for security consideration.
-			 */
-			memcpy(desc, qi->desc + (wait_index << shift),
-			       1 << shift);
-			writel(DMA_FSTS_IQE, iommu->reg + DMAR_FSTS_REG);
+		head_idx = (head >> shift) % QI_LENGTH;
+
+		/*
+		 * 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)) {
+			ret = qi_drain_remaining_descs(iommu, head_idx,
+						      wait_index);
+			if (ret)
+				return ret;
+
 			pr_info("Invalidation Queue Error (IQE) cleared\n");
 			return -EINVAL;
 		}
-- 
2.43.7