2018-05-01 19:53:28

by Gary R Hook

[permalink] [raw]
Subject: [PATCH 0/2] Tweak AMD IOMMU logging

Update the AMD IOMMU log messages to be more precise, and
add a log message for a new event type.

---

Gary R Hook (2):
iommu/amd - Update the PASID information printed to the system log
iommu/amd - Update logging information for new event type


drivers/iommu/amd_iommu.c | 39 +++++++++++++++++++++------------------
drivers/iommu/amd_iommu_types.h | 1 +
2 files changed, 22 insertions(+), 18 deletions(-)

--


2018-05-01 19:53:29

by Gary R Hook

[permalink] [raw]
Subject: [PATCH 1/2] iommu/amd - Update the PASID information printed to the system log

Provide detailed data for each event, as appropriate.

Signed-off-by: Gary R Hook <[email protected]>
---
drivers/iommu/amd_iommu.c | 31 +++++++++++++------------------
1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 8c469b51185f..a557565d4413 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -544,7 +544,7 @@ static void amd_iommu_report_page_fault(u16 devid, u16 domain_id,
static void iommu_print_event(struct amd_iommu *iommu, void *__evt)
{
struct device *dev = iommu->iommu.dev;
- int type, devid, domid, flags;
+ int type, devid, pasid, flags;
volatile u32 *event = __evt;
int count = 0;
u64 address;
@@ -552,7 +552,7 @@ static void iommu_print_event(struct amd_iommu *iommu, void *__evt)
retry:
type = (event[1] >> EVENT_TYPE_SHIFT) & EVENT_TYPE_MASK;
devid = (event[0] >> EVENT_DEVID_SHIFT) & EVENT_DEVID_MASK;
- domid = (event[1] >> EVENT_DOMID_SHIFT) & EVENT_DOMID_MASK;
+ pasid = PPR_PASID(*(u64 *)&event[0]);
flags = (event[1] >> EVENT_FLAGS_SHIFT) & EVENT_FLAGS_MASK;
address = (u64)(((u64)event[3]) << 32) | event[2];

@@ -567,7 +567,7 @@ static void iommu_print_event(struct amd_iommu *iommu, void *__evt)
}

if (type == EVENT_TYPE_IO_FAULT) {
- amd_iommu_report_page_fault(devid, domid, address, flags);
+ amd_iommu_report_page_fault(devid, pasid, address, flags);
return;
} else {
dev_err(dev, "AMD-Vi: Event logged [");
@@ -575,10 +575,9 @@ static void iommu_print_event(struct amd_iommu *iommu, void *__evt)

switch (type) {
case EVENT_TYPE_ILL_DEV:
- dev_err(dev, "ILLEGAL_DEV_TABLE_ENTRY device=%02x:%02x.%x "
- "address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "ILLEGAL_DEV_TABLE_ENTRY device=%02x:%02x.%x pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
- address, flags);
+ pasid, address, flags);
dump_dte_entry(devid);
break;
case EVENT_TYPE_DEV_TAB_ERR:
@@ -588,34 +587,30 @@ static void iommu_print_event(struct amd_iommu *iommu, void *__evt)
address, flags);
break;
case EVENT_TYPE_PAGE_TAB_ERR:
- dev_err(dev, "PAGE_TAB_HARDWARE_ERROR device=%02x:%02x.%x "
- "domain=0x%04x address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "PAGE_TAB_HARDWARE_ERROR device=%02x:%02x.%x domain=0x%04x address=0x%016llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
- domid, address, flags);
+ pasid, address, flags);
break;
case EVENT_TYPE_ILL_CMD:
dev_err(dev, "ILLEGAL_COMMAND_ERROR address=0x%016llx]\n", address);
dump_command(address);
break;
case EVENT_TYPE_CMD_HARD_ERR:
- dev_err(dev, "COMMAND_HARDWARE_ERROR address=0x%016llx "
- "flags=0x%04x]\n", address, flags);
+ dev_err(dev, "COMMAND_HARDWARE_ERROR address=0x%016llx flags=0x%04x]\n",
+ address, flags);
break;
case EVENT_TYPE_IOTLB_INV_TO:
- dev_err(dev, "IOTLB_INV_TIMEOUT device=%02x:%02x.%x "
- "address=0x%016llx]\n",
+ dev_err(dev, "IOTLB_INV_TIMEOUT device=%02x:%02x.%x address=0x%016llx]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
address);
break;
case EVENT_TYPE_INV_DEV_REQ:
- dev_err(dev, "INVALID_DEVICE_REQUEST device=%02x:%02x.%x "
- "address=0x%016llx flags=0x%04x]\n",
+ dev_err(dev, "INVALID_DEVICE_REQUEST device=%02x:%02x.%x pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
- address, flags);
+ pasid, address, flags);
break;
default:
- dev_err(dev, KERN_ERR "UNKNOWN event[0]=0x%08x event[1]=0x%08x "
- "event[2]=0x%08x event[3]=0x%08x\n",
+ dev_err(dev, "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]);
}



2018-05-01 19:54:45

by Gary R Hook

[permalink] [raw]
Subject: [PATCH 2/2] iommu/amd - Update logging information for new event type

A new events have been defined in the AMD IOMMU spec:

0x09 - "invalid PPR request"

Add support for logging this type of event.

Signed-off-by: Gary R Hook <[email protected]>
~
~
~
---
drivers/iommu/amd_iommu.c | 10 +++++++++-
drivers/iommu/amd_iommu_types.h | 1 +
2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index a557565d4413..009c6d801fae 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -544,7 +544,7 @@ static void amd_iommu_report_page_fault(u16 devid, u16 domain_id,
static void iommu_print_event(struct amd_iommu *iommu, void *__evt)
{
struct device *dev = iommu->iommu.dev;
- int type, devid, pasid, flags;
+ int type, devid, pasid, flags, tag;
volatile u32 *event = __evt;
int count = 0;
u64 address;
@@ -609,6 +609,14 @@ static void iommu_print_event(struct amd_iommu *iommu, void *__evt)
PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
pasid, address, flags);
break;
+ case EVENT_TYPE_INV_PPR_REQ:
+ pasid = ((event[0] >> 16) & 0xFFFF)
+ | ((event[1] << 6) & 0xF0000);
+ tag = event[1] & 0x03FF;
+ dev_err(dev, "INVALID_PPR_REQUEST device=%02x:%02x.%x pasid=0x%05x address=0x%016llx flags=0x%04x]\n",
+ PCI_BUS_NUM(devid), PCI_SLOT(devid), PCI_FUNC(devid),
+ pasid, address, flags);
+ break;
default:
dev_err(dev, "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]);
diff --git a/drivers/iommu/amd_iommu_types.h b/drivers/iommu/amd_iommu_types.h
index 1c9b080276c9..986cbe0cc189 100644
--- a/drivers/iommu/amd_iommu_types.h
+++ b/drivers/iommu/amd_iommu_types.h
@@ -133,6 +133,7 @@
#define EVENT_TYPE_CMD_HARD_ERR 0x6
#define EVENT_TYPE_IOTLB_INV_TO 0x7
#define EVENT_TYPE_INV_DEV_REQ 0x8
+#define EVENT_TYPE_INV_PPR_REQ 0x9
#define EVENT_DEVID_MASK 0xffff
#define EVENT_DEVID_SHIFT 0
#define EVENT_DOMID_MASK 0xffff


2018-05-03 14:02:59

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 1/2] iommu/amd - Update the PASID information printed to the system log

On Tue, May 01, 2018 at 02:52:52PM -0500, Gary R Hook wrote:
> @@ -567,7 +567,7 @@ static void iommu_print_event(struct amd_iommu *iommu, void *__evt)
> }
>
> if (type == EVENT_TYPE_IO_FAULT) {
> - amd_iommu_report_page_fault(devid, domid, address, flags);
> + amd_iommu_report_page_fault(devid, pasid, address, flags);

According to the spec this could be a domid or a pasid, it would be good
to make that visible in the error message, depending on the value of the
GN flag in the event entry.

But that can be done in a separate patch, I applied these two, thanks.


2018-05-03 18:11:00

by Gary R Hook

[permalink] [raw]
Subject: Re: [PATCH 1/2] iommu/amd - Update the PASID information printed to the system log

On 05/03/2018 08:57 AM, Joerg Roedel wrote:
> On Tue, May 01, 2018 at 02:52:52PM -0500, Gary R Hook wrote:
>> @@ -567,7 +567,7 @@ static void iommu_print_event(struct amd_iommu *iommu, void *__evt)
>> }
>>
>> if (type == EVENT_TYPE_IO_FAULT) {
>> - amd_iommu_report_page_fault(devid, domid, address, flags);
>> + amd_iommu_report_page_fault(devid, pasid, address, flags);
>
> According to the spec this could be a domid or a pasid, it would be good
> to make that visible in the error message, depending on the value of the
> GN flag in the event entry.
>
> But that can be done in a separate patch, I applied these two, thanks.


Yes, I didn't quite get this right. Both values should be passed along.
Or perhaps the entire event could be passed in and decoded by
amd_iommu_report_page_fault()?

2018-05-03 20:29:55

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH 1/2] iommu/amd - Update the PASID information printed to the system log

On Thu, May 03, 2018 at 01:09:18PM -0500, Gary R Hook wrote:
> Yes, I didn't quite get this right. Both values should be passed along. Or
> perhaps the entire event could be passed in and decoded by
> amd_iommu_report_page_fault()?

You already pass in the flags, you can chose the correct message by
looking at the GN flag there and treat the value as a domid or a pasid.


Joerg