[PATCH] iommu/amd: Enhance "Completion-wait Time-out" error message

[PATCH] iommu/amd: Enhance "Completion-wait Time-out" error message

[Dheeraj Kumar Srivastava]

Current IOMMU driver prints "Completion-wait Time-out" error message with
insufficient information to further debug the issue.

Enhancing the error message as following:
1. Log IOMMU PCI device ID in the error message.
2. Also dump the command and offset of the command prior to the
   COMPLETION_WAIT command to figure out for what IOMMU is waiting.
3. With "amd_iommu_dump=1" kernel command line option, dump entire
   command buffer entries including Head and Tail offset.

Signed-off-by: Dheeraj Kumar Srivastava <dheerajkumar.srivastava@amd.com>
---
 drivers/iommu/amd/amd_iommu_types.h |  4 ++++
 drivers/iommu/amd/iommu.c           | 31 +++++++++++++++++++++++++++--
 2 files changed, 33 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
index 95f63c5f6159..7576814f944d 100644
--- a/drivers/iommu/amd/amd_iommu_types.h
+++ b/drivers/iommu/amd/amd_iommu_types.h
@@ -247,6 +247,10 @@
 #define CMD_BUFFER_ENTRIES 512
 #define MMIO_CMD_SIZE_SHIFT 56
 #define MMIO_CMD_SIZE_512 (0x9ULL << MMIO_CMD_SIZE_SHIFT)
+#define MMIO_CMD_HEAD_MASK	GENMASK_ULL(18, 4)
+#define MMIO_CMD_BUFFER_HEAD(x) FIELD_GET(MMIO_CMD_HEAD_MASK, (x))
+#define MMIO_CMD_TAIL_MASK	GENMASK_ULL(18, 4)
+#define MMIO_CMD_BUFFER_TAIL(x) FIELD_GET(MMIO_CMD_TAIL_MASK, (x))
 
 /* constants for event buffer handling */
 #define EVT_BUFFER_SIZE		8192 /* 512 entries */
diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
index eb348c63a8d0..e1b4b0ea0990 100644
--- a/drivers/iommu/amd/iommu.c
+++ b/drivers/iommu/amd/iommu.c
@@ -1158,7 +1158,8 @@ irqreturn_t amd_iommu_int_handler(int irq, void *data)
 
 static int wait_on_sem(struct amd_iommu *iommu, u64 data)
 {
-	int i = 0;
+	struct iommu_cmd *cmd;
+	int i = 0, j;
 
 	while (*iommu->cmd_sem != data && i < LOOP_TIMEOUT) {
 		udelay(1);
@@ -1166,7 +1167,33 @@ static int wait_on_sem(struct amd_iommu *iommu, u64 data)
 	}
 
 	if (i == LOOP_TIMEOUT) {
-		pr_alert("Completion-Wait loop timed out\n");
+		int head, tail;
+
+		head = readl(iommu->mmio_base + MMIO_CMD_HEAD_OFFSET);
+		tail = readl(iommu->mmio_base + MMIO_CMD_TAIL_OFFSET);
+
+		pr_alert("IOMMU %04x:%02x:%02x.%01x: Completion-Wait loop timed out\n",
+			 iommu->pci_seg->id, PCI_BUS_NUM(iommu->devid),
+			 PCI_SLOT(iommu->devid), PCI_FUNC(iommu->devid));
+		if (!amd_iommu_dump) {
+			/*
+			 * On command buffer completion timeout, step back by 2 commands
+			 * to locate the actual command that is causing the issue.
+			 */
+			tail = (MMIO_CMD_BUFFER_TAIL(tail) - 2) & (CMD_BUFFER_ENTRIES - 1);
+			cmd = (struct iommu_cmd *)(iommu->cmd_buf + tail * sizeof(*cmd));
+			dump_command(iommu_virt_to_phys(cmd));
+		} else {
+			/* Dump entire command buffer along with head and tail indices */
+			pr_alert("CMD Buffer head=%d tail=%d\n", (int)(MMIO_CMD_BUFFER_HEAD(head)),
+				 (int)(MMIO_CMD_BUFFER_TAIL(tail)));
+			for (j = 0; j < CMD_BUFFER_ENTRIES; j++) {
+				cmd = (struct iommu_cmd *)(iommu->cmd_buf + j * sizeof(*cmd));
+				pr_err("%3d: %08x %08x %08x %08x\n", j, cmd->data[0], cmd->data[1],
+				       cmd->data[2], cmd->data[3]);
+			}
+		}
+
 		return -EIO;
 	}
 
-- 
2.25.1

Re: [PATCH] iommu/amd: Enhance "Completion-wait Time-out" error message

[Jörg Rödel]

Hey Dheeraj,

On Thu, Oct 16, 2025 at 08:38:09PM +0530, Dheeraj Kumar Srivastava wrote:
>  static int wait_on_sem(struct amd_iommu *iommu, u64 data)
>  {
> -	int i = 0;
> +	struct iommu_cmd *cmd;
> +	int i = 0, j;
>  
>  	while (*iommu->cmd_sem != data && i < LOOP_TIMEOUT) {
>  		udelay(1);
> @@ -1166,7 +1167,33 @@ static int wait_on_sem(struct amd_iommu *iommu, u64 data)
>  	}
>  
>  	if (i == LOOP_TIMEOUT) {
> -		pr_alert("Completion-Wait loop timed out\n");
> +		int head, tail;
> +
> +		head = readl(iommu->mmio_base + MMIO_CMD_HEAD_OFFSET);
> +		tail = readl(iommu->mmio_base + MMIO_CMD_TAIL_OFFSET);
> +
> +		pr_alert("IOMMU %04x:%02x:%02x.%01x: Completion-Wait loop timed out\n",
> +			 iommu->pci_seg->id, PCI_BUS_NUM(iommu->devid),
> +			 PCI_SLOT(iommu->devid), PCI_FUNC(iommu->devid));

Better use dev_err(&amd_iommu->dev->dev, ...) here.

> +		if (!amd_iommu_dump) {
> +			/*
> +			 * On command buffer completion timeout, step back by 2 commands
> +			 * to locate the actual command that is causing the issue.
> +			 */
> +			tail = (MMIO_CMD_BUFFER_TAIL(tail) - 2) & (CMD_BUFFER_ENTRIES - 1);
> +			cmd = (struct iommu_cmd *)(iommu->cmd_buf + tail * sizeof(*cmd));
> +			dump_command(iommu_virt_to_phys(cmd));
> +		} else {
> +			/* Dump entire command buffer along with head and tail indices */
> +			pr_alert("CMD Buffer head=%d tail=%d\n", (int)(MMIO_CMD_BUFFER_HEAD(head)),
> +				 (int)(MMIO_CMD_BUFFER_TAIL(tail)));
> +			for (j = 0; j < CMD_BUFFER_ENTRIES; j++) {
> +				cmd = (struct iommu_cmd *)(iommu->cmd_buf + j * sizeof(*cmd));
> +				pr_err("%3d: %08x %08x %08x %08x\n", j, cmd->data[0], cmd->data[1],
> +				       cmd->data[2], cmd->data[3]);
> +			}
> +		}

I don't think it makes much sense to just print the command before the failed
completion wait. In case of a timeout and amd_iommu_dump == true, just dump the
whole pending command buffer, from head to tail.

Also, please put all the parsing/dumping code into a separate function.

Regards,

	Joerg

Re: [PATCH] iommu/amd: Enhance "Completion-wait Time-out" error message

[Vasant Hegde]

Joerg,


On 10/27/2025 6:19 PM, Jörg Rödel wrote:
> Hey Dheeraj,
> 
> On Thu, Oct 16, 2025 at 08:38:09PM +0530, Dheeraj Kumar Srivastava wrote:
>>  static int wait_on_sem(struct amd_iommu *iommu, u64 data)
>>  {
>> -	int i = 0;
>> +	struct iommu_cmd *cmd;
>> +	int i = 0, j;
>>  
>>  	while (*iommu->cmd_sem != data && i < LOOP_TIMEOUT) {
>>  		udelay(1);
>> @@ -1166,7 +1167,33 @@ static int wait_on_sem(struct amd_iommu *iommu, u64 data)
>>  	}
>>  
>>  	if (i == LOOP_TIMEOUT) {
>> -		pr_alert("Completion-Wait loop timed out\n");
>> +		int head, tail;
>> +
>> +		head = readl(iommu->mmio_base + MMIO_CMD_HEAD_OFFSET);
>> +		tail = readl(iommu->mmio_base + MMIO_CMD_TAIL_OFFSET);
>> +
>> +		pr_alert("IOMMU %04x:%02x:%02x.%01x: Completion-Wait loop timed out\n",
>> +			 iommu->pci_seg->id, PCI_BUS_NUM(iommu->devid),
>> +			 PCI_SLOT(iommu->devid), PCI_FUNC(iommu->devid));
> 
> Better use dev_err(&amd_iommu->dev->dev, ...) here.
> 
>> +		if (!amd_iommu_dump) {
>> +			/*
>> +			 * On command buffer completion timeout, step back by 2 commands
>> +			 * to locate the actual command that is causing the issue.
>> +			 */
>> +			tail = (MMIO_CMD_BUFFER_TAIL(tail) - 2) & (CMD_BUFFER_ENTRIES - 1);
>> +			cmd = (struct iommu_cmd *)(iommu->cmd_buf + tail * sizeof(*cmd));
>> +			dump_command(iommu_virt_to_phys(cmd));
>> +		} else {
>> +			/* Dump entire command buffer along with head and tail indices */
>> +			pr_alert("CMD Buffer head=%d tail=%d\n", (int)(MMIO_CMD_BUFFER_HEAD(head)),
>> +				 (int)(MMIO_CMD_BUFFER_TAIL(tail)));
>> +			for (j = 0; j < CMD_BUFFER_ENTRIES; j++) {
>> +				cmd = (struct iommu_cmd *)(iommu->cmd_buf + j * sizeof(*cmd));
>> +				pr_err("%3d: %08x %08x %08x %08x\n", j, cmd->data[0], cmd->data[1],
>> +				       cmd->data[2], cmd->data[3]);
>> +			}
>> +		}
> 
> I don't think it makes much sense to just print the command before the failed
> completion wait. In case of a timeout and amd_iommu_dump == true, just dump the
> whole pending command buffer, from head to tail.

We have debugfs support to extract entire command buffer. Also many cases once
we hit completion wait timeout, buffer won't progress.. and we will hit
completion wait repetitively. Hence in V2 he has removed printing entire command
buffer.


Do you want to log entire buffer once to dmesg if amd_iommu_dump=1 ? (for first
completion wait timeout event).


-Vasant

Re: [PATCH] iommu/amd: Enhance "Completion-wait Time-out" error message

[Jörg Rödel]

On Tue, Oct 28, 2025 at 02:45:16PM +0530, Vasant Hegde wrote:
> We have debugfs support to extract entire command buffer. Also many cases once
> we hit completion wait timeout, buffer won't progress.. and we will hit
> completion wait repetitively. Hence in V2 he has removed printing entire command
> buffer.
> 
> Do you want to log entire buffer once to dmesg if amd_iommu_dump=1 ? (for first
> completion wait timeout event).

I think there is some value in printing the command buffer contents at the
point in time when the timeout happens. When it is read later from debug-fs,
the state of the command buffer might already be different, making debugging
harder.

Although dmesg is not the optimal place to dump this information, I also do not
see a better place.

Regards,

	Joerg

Re: [PATCH] iommu/amd: Enhance "Completion-wait Time-out" error message

[Vasant Hegde]

On 10/29/2025 2:00 PM, Jörg Rödel wrote:
> On Tue, Oct 28, 2025 at 02:45:16PM +0530, Vasant Hegde wrote:
>> We have debugfs support to extract entire command buffer. Also many cases once
>> we hit completion wait timeout, buffer won't progress.. and we will hit
>> completion wait repetitively. Hence in V2 he has removed printing entire command
>> buffer.
>>
>> Do you want to log entire buffer once to dmesg if amd_iommu_dump=1 ? (for first
>> completion wait timeout event).
> 
> I think there is some value in printing the command buffer contents at the
> point in time when the timeout happens. When it is read later from debug-fs,
> the state of the command buffer might already be different, making debugging
> harder.

Ack. if amd_iommu_dump=1 then we can log entire command buffer to dmesg once.


-Vasant

Re: [PATCH] iommu/amd: Enhance "Completion-wait Time-out" error message

[Dheeraj Kumar Srivastava]

Hi Joerg and Vasant,

On 10/29/2025 4:18 PM, Vasant Hegde wrote:
> 
> 
> On 10/29/2025 2:00 PM, Jörg Rödel wrote:
>> On Tue, Oct 28, 2025 at 02:45:16PM +0530, Vasant Hegde wrote:
>>> We have debugfs support to extract entire command buffer. Also many cases once
>>> we hit completion wait timeout, buffer won't progress.. and we will hit
>>> completion wait repetitively. Hence in V2 he has removed printing entire command
>>> buffer.
>>>
>>> Do you want to log entire buffer once to dmesg if amd_iommu_dump=1 ? (for first
>>> completion wait timeout event).
>>
>> I think there is some value in printing the command buffer contents at the
>> point in time when the timeout happens. When it is read later from debug-fs,
>> the state of the command buffer might already be different, making debugging
>> harder.
> 
> Ack. if amd_iommu_dump=1 then we can log entire command buffer to dmesg once.
> 

Sure. I’ll send an updated version for review.

Thanks
Dheeraj

> 
> -Vasant
> 

Re: [PATCH] iommu/amd: Enhance "Completion-wait Time-out" error message

[Dheeraj Kumar Srivastava]

Hey Joerg,

On 10/27/2025 6:19 PM, Jörg Rödel wrote:
> Hey Dheeraj,
> 
> On Thu, Oct 16, 2025 at 08:38:09PM +0530, Dheeraj Kumar Srivastava wrote:
>>   static int wait_on_sem(struct amd_iommu *iommu, u64 data)
>>   {
>> -	int i = 0;
>> +	struct iommu_cmd *cmd;
>> +	int i = 0, j;
>>   
>>   	while (*iommu->cmd_sem != data && i < LOOP_TIMEOUT) {
>>   		udelay(1);
>> @@ -1166,7 +1167,33 @@ static int wait_on_sem(struct amd_iommu *iommu, u64 data)
>>   	}
>>   
>>   	if (i == LOOP_TIMEOUT) {
>> -		pr_alert("Completion-Wait loop timed out\n");
>> +		int head, tail;
>> +
>> +		head = readl(iommu->mmio_base + MMIO_CMD_HEAD_OFFSET);
>> +		tail = readl(iommu->mmio_base + MMIO_CMD_TAIL_OFFSET);
>> +
>> +		pr_alert("IOMMU %04x:%02x:%02x.%01x: Completion-Wait loop timed out\n",
>> +			 iommu->pci_seg->id, PCI_BUS_NUM(iommu->devid),
>> +			 PCI_SLOT(iommu->devid), PCI_FUNC(iommu->devid));
> 
> Better use dev_err(&amd_iommu->dev->dev, ...) here.

In the early boot process, the command buffer is used but 
iommu->dev->dev is not set, which can lead to a kernel crash with dev_err.

Thanks
Dheeraj

> 
>> +		if (!amd_iommu_dump) {
>> +			/*
>> +			 * On command buffer completion timeout, step back by 2 commands
>> +			 * to locate the actual command that is causing the issue.
>> +			 */
>> +			tail = (MMIO_CMD_BUFFER_TAIL(tail) - 2) & (CMD_BUFFER_ENTRIES - 1);
>> +			cmd = (struct iommu_cmd *)(iommu->cmd_buf + tail * sizeof(*cmd));
>> +			dump_command(iommu_virt_to_phys(cmd));
>> +		} else {
>> +			/* Dump entire command buffer along with head and tail indices */
>> +			pr_alert("CMD Buffer head=%d tail=%d\n", (int)(MMIO_CMD_BUFFER_HEAD(head)),
>> +				 (int)(MMIO_CMD_BUFFER_TAIL(tail)));
>> +			for (j = 0; j < CMD_BUFFER_ENTRIES; j++) {
>> +				cmd = (struct iommu_cmd *)(iommu->cmd_buf + j * sizeof(*cmd));
>> +				pr_err("%3d: %08x %08x %08x %08x\n", j, cmd->data[0], cmd->data[1],
>> +				       cmd->data[2], cmd->data[3]);
>> +			}
>> +		}
> 
> I don't think it makes much sense to just print the command before the failed
> completion wait. In case of a timeout and amd_iommu_dump == true, just dump the
> whole pending command buffer, from head to tail.
> 
> Also, please put all the parsing/dumping code into a separate function.
> 
> Regards,
> 
> 	Joerg