drivers/iommu/amd/amd_iommu_types.h | 4 ++++ drivers/iommu/amd/iommu.c | 31 +++++++++++++++++++++++++++-- 2 files changed, 33 insertions(+), 2 deletions(-)
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.1Hey 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
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
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
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
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
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 >
© 2016 - 2026 Red Hat, Inc.