Re: [PATCH v3 4/8] iommu/amd: Add debugfs support to dump IOMMU command buffer

[Dheeraj Kumar Srivastava <dheerajkumar.srivastava@amd.com>]

Hi Vasant,

On 3/13/2025 4:08 PM, Vasant Hegde wrote:
> Dheeraj,
> 
> 
> On 2/6/2025 11:29 AM, Dheeraj Kumar Srivastava wrote:
>> IOMMU driver sends command to IOMMU hardware via command buffer. In cases
>> where IOMMU hardware fails to process commands in command buffer, dumping
>> it is a valuable input to debug the issue.
>>
>> IOMMU hardware processes command buffer entry at offset equals to the head
>> pointer. Dumping just the entry at the head pointer may not always be
>> useful. The current head may not be pointing to the entry of the command
>> buffer which is causing the issue. IOMMU Hardware may have processed the
>> entry and updated the head pointer. So dumping the entire command buffer
>> gives a broad understanding of what hardware was/is doing. The command
>> buffer dump will have all entries from start to end of the command buffer.
>> Along with that, it will have a head and tail command buffer pointer
>> register dump to facilitate where the IOMMU driver and hardware are in
>> the command buffer for injecting and processing the entries respectively.
>>
>> Command buffer is a per IOMMU data structure. So dumping on per IOMMU
>> basis.
>> eg. To get command buffer dump for iommu<x>
>>      #cat /sys/kernel/debug/iommu/amd/iommu<x>/cmdbuf
>>
>> Signed-off-by: Dheeraj Kumar Srivastava <dheerajkumar.srivastava@amd.com>
>> ---
>>   drivers/iommu/amd/amd_iommu_types.h |  7 +++++++
>>   drivers/iommu/amd/debugfs.c         | 26 ++++++++++++++++++++++++++
>>   drivers/iommu/amd/iommu.c           |  7 -------
>>   3 files changed, 33 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
>> index 0bbda60d3cdc..c4ca6d9c6b51 100644
>> --- a/drivers/iommu/amd/amd_iommu_types.h
>> +++ b/drivers/iommu/amd/amd_iommu_types.h
>> @@ -892,6 +892,13 @@ struct dev_table_entry {
>>   	};
>>   };
>>   
>> +/*
>> + * Structure defining one entry in the command buffer
>> + */
>> +struct iommu_cmd {
>> +	u32 data[4];
>> +};
>> +
>>   /*
>>    * Structure to sture persistent DTE flags from IVHD
>>    */
>> diff --git a/drivers/iommu/amd/debugfs.c b/drivers/iommu/amd/debugfs.c
>> index dc5fdc905754..bf718e7258c5 100644
>> --- a/drivers/iommu/amd/debugfs.c
>> +++ b/drivers/iommu/amd/debugfs.c
>> @@ -103,6 +103,30 @@ static int iommu_capability_show(struct seq_file *m, void *unused)
>>   }
>>   DEFINE_SHOW_STORE_ATTRIBUTE(iommu_capability);
>>   
>> +static int iommu_cmdbuf_show(struct seq_file *m, void *unused)
>> +{
>> +	struct amd_iommu *iommu = m->private;
>> +	struct iommu_cmd *cmd;
>> +	unsigned long flag;
>> +	u32 head, tail;
>> +	int i;
>> +
>> +	raw_spin_lock_irqsave(&iommu->lock, flag);
>> +	head = readl(iommu->mmio_base + MMIO_CMD_HEAD_OFFSET);
>> +	tail = readl(iommu->mmio_base + MMIO_CMD_TAIL_OFFSET);
>> +	seq_printf(m, "CMD Buffer Head Offset:%d Tail Offset:%d\n",
>> +		   (head >> 4) & 0x7fff, (tail >> 4) & 0x7fff);
>> +	for (i = 0; i < CMD_BUFFER_ENTRIES; i++) {
>> +		cmd = (struct iommu_cmd *)(iommu->cmd_buf + i * sizeof(*cmd));
>> +		seq_printf(m, "%3d: %08x%08x%08x%08x\n", i, cmd->data[0],
> 
> Can you give space between words so that its easy to read?
> 

Sure. Will update in v4.

Thanks
Dheeraj

> -Vasant
> 
>