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