Re: [PATCH 3/3] iommu/amd: Add vendor:device ID to AMD IOMMU event logs

[Pranjal Shrivastava <praan@google.com>]

On Wed, May 06, 2026 at 03:05:39PM +0000, Yigit Oguz wrote:
> Add amd_iommu_devid_str() helper that formats PCI device identity as
> SSSS:BB:DD.F VVVV:DDDD by looking up the pci_dev via
> pci_get_domain_bus_and_slot. Falls back to SSSS:BB:DD.F when the
> device is not found.
> 
> Before:
>   AMD-Vi: Event logged [IO_PAGE_FAULT device=0000:41:00.0 domain=0x000a
>   	address=0xe0000000 flags=0x0020]
> 
> After:
>   AMD-Vi: Event logged [IO_PAGE_FAULT device=0000:41:00.0 8086:1533 domain=0x000a
>   	address=0xe0000000 flags=0x0020]
> 
> Signed-off-by: Yigit Oguz <yigitogu@amazon.de>
> Signed-off-by: Lilit Janpoladyan <lilitj@amazon.com>
> Assisted-by: Claude:claude-4.6-opus
> ---
>  drivers/iommu/amd/iommu.c | 94 ++++++++++++++++++++++++---------------
>  1 file changed, 58 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 01171361f9bc..441b4a7e85d5 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -779,11 +779,34 @@ static void dump_command(unsigned long phys_addr)
>  		pr_err("CMD[%d]: %08x\n", i, cmd->data[i]);
>  }
>  
> +#define AMD_IOMMU_DEVID_SIZE	48
> +
> +static void amd_iommu_devid_str(struct amd_iommu *iommu, u16 devid, char *buf,
> +				size_t size)
> +{
> +	struct pci_dev *pdev;
> +
> +	pdev = pci_get_domain_bus_and_slot(iommu->pci_seg->id,
> +					   PCI_BUS_NUM(devid), devid & 0xff);
> +	if (pdev) {
> +		snprintf(buf, size, "%04x:%02x:%02x.%x %04x:%04x",
> +			 iommu->pci_seg->id, PCI_BUS_NUM(devid),
> +			 PCI_SLOT(devid), PCI_FUNC(devid),
> +			 pdev->vendor, pdev->device);
> +		pci_dev_put(pdev);

From a quick glance it looks like we call this in bottom halves, which
looks fine.

> +	} else {
> +		snprintf(buf, size, "%04x:%02x:%02x.%x",
> +			 iommu->pci_seg->id, PCI_BUS_NUM(devid),
> +			 PCI_SLOT(devid), PCI_FUNC(devid));
> +	}
> +}
> +
>  static void amd_iommu_report_rmp_hw_error(struct amd_iommu *iommu, volatile u32 *event)
>  {
>  	struct iommu_dev_data *dev_data = NULL;
>  	int devid, vmg_tag, flags;
>  	struct pci_dev *pdev;
> +	char devid_str[AMD_IOMMU_DEVID_SIZE];
>  	u64 spa;
>  
>  	devid   = (event[0] >> EVENT_DEVID_SHIFT) & EVENT_DEVID_MASK;
> @@ -796,15 +819,16 @@ static void amd_iommu_report_rmp_hw_error(struct amd_iommu *iommu, volatile u32
>  	if (pdev)
>  		dev_data = dev_iommu_priv_get(&pdev->dev);
>  
> +	amd_iommu_devid_str(iommu, devid, devid_str, sizeof(devid_str));

Will this iterate the global pci dev list for EVERY event? I'm wondering
if we could improve that somehow?

[------------- >8 ---------------]

Thanks,
Praan