Re: [PATCH 02/24] iommu/amd: Introduce vIOMMU-specific events and event

"[email protected]" <[email protected]>
Newsgroups dev.linux.lists.iommu,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
在 2026/7/27 21:28, Suravee Suthikulpanit 写道:
> Adding support for new vIOMMU events:
>    * Guest Event Fault event
>    * vIOMMU Hardware Error event
>
> Also, adding support for the additional vIOMMU related flags
> in existing events.
>
> Signed-off-by: Suravee Suthikulpanit <[email protected]>
> ---
>   drivers/iommu/amd/amd_iommu_types.h |  7 ++++
>   drivers/iommu/amd/iommu.c           | 58 ++++++++++++++++++++++-------
>   2 files changed, 52 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/iommu/amd/amd_iommu_types.h b/drivers/iommu/amd/amd_iommu_types.h
> index d2c64e2e9f05..4df6a50128de 100644
> --- a/drivers/iommu/amd/amd_iommu_types.h
> +++ b/drivers/iommu/amd/amd_iommu_types.h
> @@ -149,6 +149,9 @@
>   #define EVENT_TYPE_IOTLB_INV_TO	0x7
>   #define EVENT_TYPE_INV_DEV_REQ	0x8
>   #define EVENT_TYPE_INV_PPR_REQ	0x9
> +#define EVENT_TYPE_GUEST_EVENT_FAULT	0xb
> +#define EVENT_TYPE_VIOMMU_HW_ERR	0xc
> +
>   #define EVENT_TYPE_RMP_FAULT	0xd
>   #define EVENT_TYPE_RMP_HW_ERR	0xe
>   #define EVENT_DEVID_MASK	0xffff
> @@ -261,6 +264,10 @@
>   #define EVTLOG_SIZE_MAX		SZ_512K /* 32K entries */
>   #define EVTLOG_LEN_MASK_MAX	(0xFULL << EVTLOG_SIZE_SHIFT)
>   
> +/* Constants for IO_PAGE_FAULT event */
> +#define IO_PAGE_FAULT_VFLAGS_SHIFT	27
> +#define IO_PAGE_FAULT_VFLAGS_MASK	GENMASK_ULL(27, 23)
> +
>   /* Constants for PPR Log handling */
>   #define PPRLOG_ENTRY_SIZE	0x10
>   #define PPRLOG_SIZE_SHIFT	56
> diff --git a/drivers/iommu/amd/iommu.c b/drivers/iommu/amd/iommu.c
> index 4b4dd20ebec6..50f26c8123f3 100644
> --- a/drivers/iommu/amd/iommu.c
> +++ b/drivers/iommu/amd/iommu.c
> @@ -854,7 +854,7 @@ static void amd_iommu_report_rmp_fault(struct amd_iommu *iommu, volatile u32 *ev
>   
>   static void amd_iommu_report_page_fault(struct amd_iommu *iommu,
>   					u16 devid, u16 domain_id,
> -					u64 address, int flags)
> +					u64 address, int flags, u8 vflags)
>   {
>   	struct iommu_dev_data *dev_data = NULL;
>   	struct pci_dev *pdev;
> @@ -889,13 +889,13 @@ static void amd_iommu_report_page_fault(struct amd_iommu *iommu,
>   		}
>   
>   		if (__ratelimit(&dev_data->rs)) {
> -			pci_err(pdev, "Event logged [IO_PAGE_FAULT domain=0x%04x address=0x%llx flags=0x%04x]\n",
> -				domain_id, address, flags);
> +			pci_err(pdev, "Event logged [IO_PAGE_FAULT domain=0x%04x address=0x%llx flags=0x%04x vflags=%#x]\n",
> +				domain_id, address, flags, vflags);
>   		}
>   	} else {
> -		pr_err_ratelimited("Event logged [IO_PAGE_FAULT device=%04x:%02x:%02x.%x domain=0x%04x address=0x%llx flags=0x%04x]\n",
> +		pr_err_ratelimited("Event logged [IO_PAGE_FAULT device=%04x:%02x:%02x.%x domain=0x%04x address=0x%llx flags=0x%04x vflags=%#x]\n",
>   			iommu->pci_seg->id, PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
> -			domain_id, address, flags);
> +			domain_id, address, flags, vflags);
>   	}
>   
>   out:
> @@ -932,29 +932,42 @@ static void iommu_print_event(struct amd_iommu *iommu, void *__evt)
>   	}
>   
>   	if (type == EVENT_TYPE_IO_FAULT) {
> -		amd_iommu_report_page_fault(iommu, devid, pasid, address, flags);
> +		u8 vflags = FIELD_GET(IO_PAGE_FAULT_VFLAGS_MASK, event[0]);
> +
> +		amd_iommu_report_page_fault(iommu, devid, pasid, address, flags, vflags);
>   		return;
>   	}
>   
>   	switch (type) {
>   	case EVENT_TYPE_ILL_DEV:
> -		dev_err(dev, "Event logged [ILLEGAL_DEV_TABLE_ENTRY device=%04x:%02x:%02x.%x pasid=0x%05x address=0x%llx flags=0x%04x]\n",
> +	{
> +		u8 vflags = FIELD_GET(IO_PAGE_FAULT_VFLAGS_MASK, event[0]);
> +
> +		dev_err(dev, "Event logged [ILLEGAL_DEV_TABLE_ENTRY deice=%04x:%02x:%02x.%x pasid=0x%05x address=0x%llx flags=0x%04x vflags=%#x]\n",
>   			iommu->pci_seg->id, PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
> -			pasid, address, flags);

A minor typos in the log strings: "deice" -> "device"

> +			pasid, address, flags, vflags);
>   		dev_err(dev, "Control Reg : 0x%llx\n", ctrl);
>   		dump_dte_entry(iommu, devid);
>   		break;
> +	}
>   	case EVENT_TYPE_DEV_TAB_ERR:
> -		dev_err(dev, "Event logged [DEV_TAB_HARDWARE_ERROR device=%04x:%02x:%02x.%x "
> -			"address=0x%llx flags=0x%04x]\n",
> +	{
> +		u8 vflags = FIELD_GET(IO_PAGE_FAULT_VFLAGS_MASK, event[0]);
> +
> +		dev_err(dev, "Event logged [DEV_TAB_HARDWARE_ERROR device=%04x:%02x:%02x.%x address=%#llx flags=%#04x vlfags=%#x]\n",
>   			iommu->pci_seg->id, PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
typos : "vlfags" -> "vflags".
> -			address, flags);
> +			address, flags, vflags);
>   		break;
> +	}
>   	case EVENT_TYPE_PAGE_TAB_ERR:
> -		dev_err(dev, "Event logged [PAGE_TAB_HARDWARE_ERROR device=%04x:%02x:%02x.%x pasid=0x%04x address=0x%llx flags=0x%04x]\n",
> +	{
> +		u8 vflags = FIELD_GET(IO_PAGE_FAULT_VFLAGS_MASK, event[0]);
> +
> +		dev_err(dev, "Event logged [PAGE_TAB_HARDWARE_ERROR device=%04x:%02x:%02x.%x pasid=0x%04x address=0x%llx flags=0x%04x vflags=%#x]\n",
>   			iommu->pci_seg->id, PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
> -			pasid, address, flags);
> +			pasid, address, flags, vflags);
>   		break;
> +	}
>   	case EVENT_TYPE_ILL_CMD:
>   		dev_err(dev, "Event logged [ILLEGAL_COMMAND_ERROR address=0x%llx]\n", address);
>   		dump_command(address);
> @@ -986,6 +999,25 @@ static void iommu_print_event(struct amd_iommu *iommu, void *__evt)
>   			iommu->pci_seg->id, PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
>   			pasid, address, flags, tag);
>   		break;
> +	case EVENT_TYPE_GUEST_EVENT_FAULT:
> +	{
> +		u8 gid = event[1] & 0xFFFF;
> +		u8 vflags = FIELD_GET(IO_PAGE_FAULT_VFLAGS_MASK, event[0]);

The GuestID is declared as u8 but assigned a 16-bit value masked with
0xFFFF. This truncates the upper 8 bits. Per the AMD IOMMU Specification
(Rev 3.10, Section 2.5.12, Table 68), the GuestID field is 16 bits wide
[15:0]. This should be:

     u16 gid = event[1] & 0xFFFF;

Note that the VIOMMU_HARDWARE_ERROR handler below correctly uses u16 for
the same field, so this looks like an oversight.


Additionally, per Section 2.5.12, a GUEST_EVENT_FAULT occupies two log
entries (paired event entry). The current iommu_poll_events() advances
by a single entry per iteration, so the second (paired) entry will be
misinterpreted as an unknown event type on the next iteration. The
handler should consume/skip the second entry.

> +
> +		dev_err(dev, "Event logged [GUEST_EVENT_FAULT gid=#%x flags=0x%04x vflags=%#x]\n",
> +			gid, flags, vflags);
> +		break;
> +	}
> +	case EVENT_TYPE_VIOMMU_HW_ERR:
> +	{
> +		u16 gid = event[0] & 0xFFFF;
> +		u8 src = (event[0] >> 16) & 0x3;
> +		u8 vflags = FIELD_GET(IO_PAGE_FAULT_VFLAGS_MASK, event[0]);
> +
> +		dev_err(dev, "Event logged [VIOMMU_HW_ERR gid=%#x address=%#llx src=%#x flags=%#x vflags=%#x]\n",
> +			gid, address, src, flags, vflags);
> +		break;
> +	}
>   	default:
>   		dev_err(dev, "Event logged [UNKNOWN event[0]=0x%08x event[1]=0x%08x event[2]=0x%08x event[3]=0x%08x\n",
>   			event[0], event[1], event[2], event[3]);
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.