[PATCH v6 0/2] iommu/arm-smmu-v3: Parse out event records

[PATCH v6 0/2] iommu/arm-smmu-v3: Parse out event records

[Pranjal Shrivastava]

Enhance the arm-smmu-v3 driver to parse out useful information from
event records into a structure for better event handling & logging.

Some sample events, powered by QEMU:

1. Bad StreamID:

[   10.940765] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x02 received:
[   10.941260] arm-smmu-v3 arm-smmu-v3.0.auto:  0x0000000800000002
[   10.941735] arm-smmu-v3 arm-smmu-v3.0.auto:  0x0000000000000000
[   10.942211] arm-smmu-v3 arm-smmu-v3.0.auto:  0x0000000000000000
[   10.942706] arm-smmu-v3 arm-smmu-v3.0.auto:  0x0000000000000000
[   10.943357] arm-smmu-v3 arm-smmu-v3.0.auto: event: C_BAD_STREAMID client: 0000:00:01.0 sid: 0x8 ssid: 0x0

2. Permission Fault:

[    7.477618] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x13 received:
[    7.478185] arm-smmu-v3 arm-smmu-v3.0.auto:  0x0000000800000013
[    7.478666] arm-smmu-v3 arm-smmu-v3.0.auto:  0x0000020000000000
[    7.479443] arm-smmu-v3 arm-smmu-v3.0.auto:  0x00000000fffff040
[    7.479929] arm-smmu-v3 arm-smmu-v3.0.auto:  0x0000000000000000
[    7.480410] arm-smmu-v3 arm-smmu-v3.0.auto: event: F_PERMISSION client: 0000:00:01.0 sid: 0x8 ssid: 0x0 iova: 0xfffff040 ipa: 0x0
[    7.481258] arm-smmu-v3 arm-smmu-v3.0.auto: unpriv data write s1 "Input address caused fault" stag: 0x0

3. STE Fetch Fault:

[    7.548667] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x03 received:
[    7.549304] arm-smmu-v3 arm-smmu-v3.0.auto:  0x0000000800000003
[    7.549800] arm-smmu-v3 arm-smmu-v3.0.auto:  0x0000000000000000
[    7.550298] arm-smmu-v3 arm-smmu-v3.0.auto:  0x0000000000000000
[    7.550780] arm-smmu-v3 arm-smmu-v3.0.auto:  0x00000000000001e0
[    7.551568] arm-smmu-v3 arm-smmu-v3.0.auto: event: F_STE_FETCH client: 0000:00:01.0 sid: 0x8 ssid: 0x0 fetch_addr: 0x1e0


5. Unknown / Implementation-defined Fault:

[   13.517857] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0b received:
[   13.518339] arm-smmu-v3 arm-smmu-v3.0.auto:  0x000000080000000b
[   13.518814] arm-smmu-v3 arm-smmu-v3.0.auto:  0x0000028800000000
[   13.519311] arm-smmu-v3 arm-smmu-v3.0.auto:  0x00000000ffffe004
[   13.519789] arm-smmu-v3 arm-smmu-v3.0.auto:  0x0000000000000000
[   13.520294] arm-smmu-v3 arm-smmu-v3.0.auto: event: UNKNOWN client: 0000:00:01.0 sid: 0x8 ssid: 0x0

v6
 * Re-structured the logs in 2 lines with prefixes as suggested.
 * Masked address fields appropriately to avoid issues with RES0
 * Added a member for fetch_address to demarcate it separately.
 * Used `FIELD_GET` where required instead of masking.
 * Renamed `raw_evt` to `evt` and `evt` to `event` as suggested.
 * Renamed `arm_smmu_handle_evt` to `arm_smmu_handle_event`.

v5
 https://lore.kernel.org/all/20241112083018.1662104-1-praan@google.com/
 * Replaced strings with used macros to conserve stack space
 * Fixed IPA log truncation,(e.g. logged 0xfffff instead of 0xfffff040)
 * Bounded evt->id by ARRAY_SIZE to avoid inflating event_str
 * Restored the original raw dump format
 * Decoupled "raw" event from `struct arm_smmu_event`
 * Moved the raw event log before the decoded/pretty log
 * Improved the sid.ssid format to be more readable
 * Zero-initialized the `arm_smmu_event` var in `arm_smmu_handle_evt`
 * Refactored struct arm_smmu_events to use bitfields instead of bools
 * Removed `struct arm_smmu_device` field from `struct arm_smmu_event`
 * Removed `master_name` field to use `dev_name(event->dev)` instead
 * Pahole reports a total of 3-byte holes & struct size = 40 bytes
 * Moved most constant parts of the log to the fmt string
 * Moved the event decoding within `arm_smmu_handle_evt`
 * Renamed `arm_smmu_get_event_from_raw` => `arm_smmu_decode_event`
 * Renamed a few EVT_IDs for consistency as per Daniel's suggestions
 * Renamed "master" to "client" in the logs as suggested in reviews
 * Corrected "F_VMS_FAULT" => "F_VMS_FETCH"
 * Re-ordered event strings in event_str as they appear in the spec
 * Removed less useful comments

v4
 https://lore.kernel.org/all/20241018180022.807928-1-praan@google.com/
 * Re-arranged the series to first introduce struct arm_smmu_event
 * Improved the complex ternary expression that prints TTRnW info
 * Added consistent spacing to the logs & resized log strings
 * Moved ratelimiting within `arm_smmu_dump_event`
 * Refactored master_name printing by getting a ref to the device
 * Refactored `arm_smmu_handle_evt` to avoid redundant master lookup

v3
 https://lore.kernel.org/all/20240928005143.2378938-1-praan@google.com/
 * Fixed a potential race and null pointer deref for arm_smmu_master
 * Improved the logging approach by using multiple strings
 * Added logs for STAG & STALL fields for relevant events
 * Invoked the log function within `arm_smmu_handle_evt` routine
 * Rebased the changes

v2
 https://lore.kernel.org/linux-iommu/20240827193026.3993039-1-praan@google.com/
 * Addressed review comments
 * Introduced `struct arm_smmu_event` to hold relevant event fields
 * Broke out helper functions to populate & dump event info
 * Modified the event handler routines to use `struct arm_smmu_event`

v1
 https://lore.kernel.org/linux-iommu/20240816211722.1404070-1-praan@google.com/

 * Replaced strings with used macros to conserve stack space
 * Fixed IPA log truncation,(e.g. logged 0xfffff instead of 0xfffff040)
 * Bounded evt->id by ARRAY_SIZE to avoid inflating event_str
 * Restored the original raw dump format
 * Decoupled "raw" event from `struct arm_smmu_event`
 * Moved the raw event log before the decoded/pretty log
 * Improved the sid.ssid format to be more readable
 * Zero-initialized the `arm_smmu_event` var in `arm_smmu_handle_evt`
 * Refactored struct arm_smmu_events to use bitfields instead of bools
 * Removed `struct arm_smmu_device` field from `struct arm_smmu_event`
 * Removed `master_name` field to use `dev_name(event->dev)` instead
 * Pahole reports a total of 3-byte holes & struct size = 40 bytes
 * Moved most constant parts of the log to the fmt string
 * Moved the event decoding within `arm_smmu_handle_evt`
 * Renamed `arm_smmu_get_event_from_raw` => `arm_smmu_decode_event`
 * Renamed a few EVT_IDs for consistency as per Daniel's suggestions
 * Renamed "master" to "client" in the logs as suggested in reviews
 * Corrected "F_VMS_FAULT" => "F_VMS_FETCH"
 * Re-ordered event strings in event_str as they appear in the spec
 * Removed less useful comments

v4
 https://lore.kernel.org/all/20241018180022.807928-1-praan@google.com/
 * Re-arranged the series to first introduce struct arm_smmu_event
 * Improved the complex ternary expression that prints TTRnW info
 * Added consistent spacing to the logs & resized log strings
 * Moved ratelimiting within `arm_smmu_dump_event`
 * Refactored master_name printing by getting a ref to the device
 * Refactored `arm_smmu_handle_evt` to avoid redundant master lookup

v3
 https://lore.kernel.org/all/20240928005143.2378938-1-praan@google.com/
 * Fixed a potential race and null pointer deref for arm_smmu_master
 * Improved the logging approach by using multiple strings
 * Added logs for STAG & STALL fields for relevant events
 * Invoked the log function within `arm_smmu_handle_evt` routine
 * Rebased the changes

v2
 https://lore.kernel.org/linux-iommu/20240827193026.3993039-1-praan@google.com/
 * Addressed review comments
 * Introduced `struct arm_smmu_event` to hold relevant event fields
 * Broke out helper functions to populate & dump event info
 * Modified the event handler routines to use `struct arm_smmu_event`

v1
 https://lore.kernel.org/linux-iommu/20240816211722.1404070-1-praan@google.com/

Pranjal Shrivastava (2):
  iommu/arm-smmu-v3: Introduce struct arm_smmu_event
  iommu/arm-smmu-v3: Log better event records

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 157 ++++++++++++++++----
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  30 ++++
 2 files changed, 162 insertions(+), 25 deletions(-)

-- 
2.47.0.338.g60cca15819-goog

[PATCH v6 1/2] iommu/arm-smmu-v3: Introduce struct arm_smmu_event

[Pranjal Shrivastava]

Introduce `struct arm_smmu_event` to represent event records.
Parse out relevant fields from raw event records for ease and
use the new `struct arm_smmu_event` instead.

Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 55 ++++++++++++++-------
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 18 +++++++
 2 files changed, 54 insertions(+), 19 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 737c5b882355..3168bb31e145 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1757,17 +1757,34 @@ arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
 }
 
 /* IRQ and event handlers */
-static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
+static void arm_smmu_decode_event(u64 *raw, struct arm_smmu_event *event)
+{
+	event->id = FIELD_GET(EVTQ_0_ID, raw[0]);
+	event->sid = FIELD_GET(EVTQ_0_SID, raw[0]);
+	event->ssv = FIELD_GET(EVTQ_0_SSV, raw[0]);
+	event->ssid = event->ssv ? FIELD_GET(EVTQ_0_SSID, raw[0]) : IOMMU_NO_PASID;
+	event->privileged = FIELD_GET(EVTQ_1_PnU, raw[1]);
+	event->instruction = FIELD_GET(EVTQ_1_InD, raw[1]);
+	event->s2 = FIELD_GET(EVTQ_1_S2, raw[1]);
+	event->read = FIELD_GET(EVTQ_1_RnW, raw[1]);
+	event->stag = FIELD_GET(EVTQ_1_STAG, raw[1]);
+	event->stall = FIELD_GET(EVTQ_1_STALL, raw[1]);
+	event->class = FIELD_GET(EVTQ_1_CLASS, raw[1]);
+	event->iova = FIELD_GET(EVTQ_2_ADDR, raw[2]);
+	event->ipa = raw[3] & EVTQ_3_IPA;
+	event->fetch_addr = raw[3] & EVTQ_3_FETCH_ADDR;
+}
+
+static int arm_smmu_handle_evt(struct arm_smmu_device *smmu,
+			       struct arm_smmu_event *event)
 {
 	int ret = 0;
 	u32 perm = 0;
 	struct arm_smmu_master *master;
-	bool ssid_valid = evt[0] & EVTQ_0_SSV;
-	u32 sid = FIELD_GET(EVTQ_0_SID, evt[0]);
 	struct iopf_fault fault_evt = { };
 	struct iommu_fault *flt = &fault_evt.fault;
 
-	switch (FIELD_GET(EVTQ_0_ID, evt[0])) {
+	switch (event->id) {
 	case EVT_ID_TRANSLATION_FAULT:
 	case EVT_ID_ADDR_SIZE_FAULT:
 	case EVT_ID_ACCESS_FAULT:
@@ -1777,35 +1794,35 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
 		return -EOPNOTSUPP;
 	}
 
-	if (!(evt[1] & EVTQ_1_STALL))
+	if (!event->stall)
 		return -EOPNOTSUPP;
 
-	if (evt[1] & EVTQ_1_RnW)
+	if (event->read)
 		perm |= IOMMU_FAULT_PERM_READ;
 	else
 		perm |= IOMMU_FAULT_PERM_WRITE;
 
-	if (evt[1] & EVTQ_1_InD)
+	if (event->instruction)
 		perm |= IOMMU_FAULT_PERM_EXEC;
 
-	if (evt[1] & EVTQ_1_PnU)
+	if (event->privileged)
 		perm |= IOMMU_FAULT_PERM_PRIV;
 
 	flt->type = IOMMU_FAULT_PAGE_REQ;
 	flt->prm = (struct iommu_fault_page_request) {
 		.flags = IOMMU_FAULT_PAGE_REQUEST_LAST_PAGE,
-		.grpid = FIELD_GET(EVTQ_1_STAG, evt[1]),
+		.grpid = event->stag,
 		.perm = perm,
-		.addr = FIELD_GET(EVTQ_2_ADDR, evt[2]),
+		.addr = event->iova,
 	};
 
-	if (ssid_valid) {
+	if (event->ssv) {
 		flt->prm.flags |= IOMMU_FAULT_PAGE_REQUEST_PASID_VALID;
-		flt->prm.pasid = FIELD_GET(EVTQ_0_SSID, evt[0]);
+		flt->prm.pasid = event->ssid;
 	}
 
 	mutex_lock(&smmu->streams_mutex);
-	master = arm_smmu_find_master(smmu, sid);
+	master = arm_smmu_find_master(smmu, event->sid);
 	if (!master) {
 		ret = -EINVAL;
 		goto out_unlock;
@@ -1820,23 +1837,23 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu, u64 *evt)
 static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
 {
 	int i, ret;
+	u64 evt[EVTQ_ENT_DWORDS];
+	struct arm_smmu_event event = {0};
 	struct arm_smmu_device *smmu = dev;
 	struct arm_smmu_queue *q = &smmu->evtq.q;
 	struct arm_smmu_ll_queue *llq = &q->llq;
 	static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL,
 				      DEFAULT_RATELIMIT_BURST);
-	u64 evt[EVTQ_ENT_DWORDS];
 
 	do {
 		while (!queue_remove_raw(q, evt)) {
-			u8 id = FIELD_GET(EVTQ_0_ID, evt[0]);
-
-			ret = arm_smmu_handle_evt(smmu, evt);
+			arm_smmu_decode_event(evt, &event);
+			ret = arm_smmu_handle_evt(smmu, &event);
 			if (!ret || !__ratelimit(&rs))
 				continue;
 
-			dev_info(smmu->dev, "event 0x%02x received:\n", id);
-			for (i = 0; i < ARRAY_SIZE(evt); ++i)
+			dev_info(smmu->dev, "event 0x%02x received:\n", event.id);
+			for (i = 0; i < EVTQ_ENT_DWORDS; ++i)
 				dev_info(smmu->dev, "\t0x%016llx\n",
 					 (unsigned long long)evt[i]);
 
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 1e9952ca989f..89fe2eee5856 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -455,6 +455,7 @@ static inline unsigned int arm_smmu_cdtab_l2_idx(unsigned int ssid)
 #define EVTQ_1_TT_READ			(1UL << 44)
 #define EVTQ_2_ADDR			GENMASK_ULL(63, 0)
 #define EVTQ_3_IPA			GENMASK_ULL(51, 12)
+#define EVTQ_3_FETCH_ADDR		GENMASK_ULL(51, 3)
 
 /* PRI queue */
 #define PRIQ_ENT_SZ_SHIFT		4
@@ -771,6 +772,23 @@ struct arm_smmu_stream {
 	struct rb_node			node;
 };
 
+struct arm_smmu_event {
+	u8				stall : 1,
+					ssv : 1,
+					privileged : 1,
+					instruction : 1,
+					s2 : 1,
+					read : 1;
+	u8				id;
+	u8				class;
+	u16				stag;
+	u32				sid;
+	u32				ssid;
+	u64				iova;
+	u64				ipa;
+	u64				fetch_addr;
+};
+
 /* SMMU private data for each master */
 struct arm_smmu_master {
 	struct arm_smmu_device		*smmu;
-- 
2.47.0.338.g60cca15819-goog

[PATCH v6 2/2] iommu/arm-smmu-v3: Log better event records

[Pranjal Shrivastava]

Currently, the driver dumps the raw hex for a received event record.
Improve this by leveraging `struct arm_smmu_event` for event fields
and log human-readable event records with meaningful information.

Signed-off-by: Pranjal Shrivastava <praan@google.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 114 +++++++++++++++++---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  14 ++-
 2 files changed, 115 insertions(+), 13 deletions(-)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 3168bb31e145..22b6ed5992cb 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -83,6 +83,28 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
 	{ 0, NULL},
 };
 
+static const char * const event_str[] = {
+	[EVT_ID_BAD_STREAMID_CONFIG] = "C_BAD_STREAMID",
+	[EVT_ID_STE_FETCH_FAULT] = "F_STE_FETCH",
+	[EVT_ID_BAD_STE_CONFIG] = "C_BAD_STE",
+	[EVT_ID_STREAM_DISABLED_FAULT] = "F_STREAM_DISABLED",
+	[EVT_ID_BAD_SUBSTREAMID_CONFIG] = "C_BAD_SUBSTREAMID",
+	[EVT_ID_CD_FETCH_FAULT] = "F_CD_FETCH",
+	[EVT_ID_BAD_CD_CONFIG] = "C_BAD_CD",
+	[EVT_ID_TRANSLATION_FAULT] = "F_TRANSLATION",
+	[EVT_ID_ADDR_SIZE_FAULT] = "F_ADDR_SIZE",
+	[EVT_ID_ACCESS_FAULT] = "F_ACCESS",
+	[EVT_ID_PERMISSION_FAULT] = "F_PERMISSION",
+	[EVT_ID_VMS_FETCH_FAULT] = "F_VMS_FETCH",
+};
+
+static const char * const event_class_str[] = {
+	[0] = "CD fetch",
+	[1] = "Stage 1 translation table fetch",
+	[2] = "Input address caused fault",
+	[3] = "Reserved",
+};
+
 static int arm_smmu_domain_finalise(struct arm_smmu_domain *smmu_domain,
 				    struct arm_smmu_device *smmu, u32 flags);
 static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master);
@@ -1757,8 +1779,11 @@ arm_smmu_find_master(struct arm_smmu_device *smmu, u32 sid)
 }
 
 /* IRQ and event handlers */
-static void arm_smmu_decode_event(u64 *raw, struct arm_smmu_event *event)
+static void arm_smmu_decode_event(struct arm_smmu_device *smmu, u64 *raw,
+				  struct arm_smmu_event *event)
 {
+	struct arm_smmu_master *master;
+
 	event->id = FIELD_GET(EVTQ_0_ID, raw[0]);
 	event->sid = FIELD_GET(EVTQ_0_SID, raw[0]);
 	event->ssv = FIELD_GET(EVTQ_0_SSV, raw[0]);
@@ -1773,9 +1798,21 @@ static void arm_smmu_decode_event(u64 *raw, struct arm_smmu_event *event)
 	event->iova = FIELD_GET(EVTQ_2_ADDR, raw[2]);
 	event->ipa = raw[3] & EVTQ_3_IPA;
 	event->fetch_addr = raw[3] & EVTQ_3_FETCH_ADDR;
+	event->ttrnw = FIELD_GET(EVTQ_1_TT_READ, raw[1]);
+	event->class_tt = false;
+	event->dev = NULL;
+
+	if (event->id == EVT_ID_PERMISSION_FAULT)
+		event->class_tt = (event->class == EVTQ_1_CLASS_TT);
+
+	mutex_lock(&smmu->streams_mutex);
+	master = arm_smmu_find_master(smmu, event->sid);
+	if (master)
+		event->dev = get_device(master->dev);
+	mutex_unlock(&smmu->streams_mutex);
 }
 
-static int arm_smmu_handle_evt(struct arm_smmu_device *smmu,
+static int arm_smmu_handle_event(struct arm_smmu_device *smmu,
 			       struct arm_smmu_event *event)
 {
 	int ret = 0;
@@ -1834,9 +1871,67 @@ static int arm_smmu_handle_evt(struct arm_smmu_device *smmu,
 	return ret;
 }
 
+static void arm_smmu_dump_raw_event(struct arm_smmu_device *smmu, u64 *raw,
+				    struct arm_smmu_event *event)
+{
+	int i;
+
+	dev_err(smmu->dev, "event 0x%02x received:\n", event->id);
+
+	for (i = 0; i < EVTQ_ENT_DWORDS; ++i)
+		dev_err(smmu->dev, "\t0x%016llx\n", raw[i]);
+}
+
+#define ARM_SMMU_EVT_KNOWN(e)	((e)->id < ARRAY_SIZE(event_str) && event_str[(e)->id])
+#define ARM_SMMU_LOG_EVT_STR(e) ARM_SMMU_EVT_KNOWN(e) ? event_str[(e)->id] : "UNKNOWN"
+#define ARM_SMMU_LOG_CLIENT(e)	(e)->dev ? dev_name((e)->dev) : "(unassigned sid)"
+
+static void arm_smmu_dump_event(struct arm_smmu_device *smmu, u64 *raw,
+				struct arm_smmu_event *evt,
+				struct ratelimit_state *rs)
+{
+	if (!__ratelimit(rs))
+		return;
+
+	arm_smmu_dump_raw_event(smmu, raw, evt);
+
+	switch (evt->id) {
+	case EVT_ID_TRANSLATION_FAULT:
+	case EVT_ID_ADDR_SIZE_FAULT:
+	case EVT_ID_ACCESS_FAULT:
+	case EVT_ID_PERMISSION_FAULT:
+		dev_err(smmu->dev, "event: %s client: %s sid: %#x ssid: %#x iova: %#llx ipa: %#llx",
+			ARM_SMMU_LOG_EVT_STR(evt), ARM_SMMU_LOG_CLIENT(evt),
+			evt->sid, evt->ssid, evt->iova, evt->ipa);
+
+		dev_err(smmu->dev, "%s %s %s %s \"%s\"%s%s stag: %#x",
+			evt->privileged ? "priv" : "unpriv",
+			evt->instruction ? "inst" : "data",
+			evt->read ? "read" : "write",
+			evt->s2 ? "s2" : "s1", event_class_str[evt->class],
+			evt->class_tt ? (evt->ttrnw ? " ttd_read" : " ttd_write") : "",
+			evt->stall ? " stall" : "", evt->stag);
+
+		break;
+
+	case EVT_ID_STE_FETCH_FAULT:
+	case EVT_ID_CD_FETCH_FAULT:
+	case EVT_ID_VMS_FETCH_FAULT:
+		dev_err(smmu->dev, "event: %s client: %s sid: %#x ssid: %#x fetch_addr: %#llx",
+			ARM_SMMU_LOG_EVT_STR(evt), ARM_SMMU_LOG_CLIENT(evt),
+			evt->sid, evt->ssid, evt->fetch_addr);
+
+		break;
+
+	default:
+		dev_err(smmu->dev, "event: %s client: %s sid: %#x ssid: %#x",
+			ARM_SMMU_LOG_EVT_STR(evt), ARM_SMMU_LOG_CLIENT(evt),
+			evt->sid, evt->ssid);
+	}
+}
+
 static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
 {
-	int i, ret;
 	u64 evt[EVTQ_ENT_DWORDS];
 	struct arm_smmu_event event = {0};
 	struct arm_smmu_device *smmu = dev;
@@ -1847,16 +1942,11 @@ static irqreturn_t arm_smmu_evtq_thread(int irq, void *dev)
 
 	do {
 		while (!queue_remove_raw(q, evt)) {
-			arm_smmu_decode_event(evt, &event);
-			ret = arm_smmu_handle_evt(smmu, &event);
-			if (!ret || !__ratelimit(&rs))
-				continue;
-
-			dev_info(smmu->dev, "event 0x%02x received:\n", event.id);
-			for (i = 0; i < EVTQ_ENT_DWORDS; ++i)
-				dev_info(smmu->dev, "\t0x%016llx\n",
-					 (unsigned long long)evt[i]);
+			arm_smmu_decode_event(smmu, evt, &event);
+			if (arm_smmu_handle_event(smmu, &event))
+				arm_smmu_dump_event(smmu, evt, &event, &rs);
 
+			put_device(event.dev);
 			cond_resched();
 		}
 
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index 89fe2eee5856..0eda9d6d99e7 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -437,10 +437,18 @@ static inline unsigned int arm_smmu_cdtab_l2_idx(unsigned int ssid)
 
 #define EVTQ_0_ID			GENMASK_ULL(7, 0)
 
+#define EVT_ID_BAD_STREAMID_CONFIG	0x02
+#define EVT_ID_STE_FETCH_FAULT		0x03
+#define EVT_ID_BAD_STE_CONFIG		0x04
+#define EVT_ID_STREAM_DISABLED_FAULT	0x06
+#define EVT_ID_BAD_SUBSTREAMID_CONFIG	0x08
+#define EVT_ID_CD_FETCH_FAULT		0x09
+#define EVT_ID_BAD_CD_CONFIG		0x0a
 #define EVT_ID_TRANSLATION_FAULT	0x10
 #define EVT_ID_ADDR_SIZE_FAULT		0x11
 #define EVT_ID_ACCESS_FAULT		0x12
 #define EVT_ID_PERMISSION_FAULT		0x13
+#define EVT_ID_VMS_FETCH_FAULT		0x25
 
 #define EVTQ_0_SSV			(1UL << 11)
 #define EVTQ_0_SSID			GENMASK_ULL(31, 12)
@@ -452,6 +460,7 @@ static inline unsigned int arm_smmu_cdtab_l2_idx(unsigned int ssid)
 #define EVTQ_1_RnW			(1UL << 35)
 #define EVTQ_1_S2			(1UL << 39)
 #define EVTQ_1_CLASS			GENMASK_ULL(41, 40)
+#define EVTQ_1_CLASS_TT			0x01
 #define EVTQ_1_TT_READ			(1UL << 44)
 #define EVTQ_2_ADDR			GENMASK_ULL(63, 0)
 #define EVTQ_3_IPA			GENMASK_ULL(51, 12)
@@ -778,7 +787,9 @@ struct arm_smmu_event {
 					privileged : 1,
 					instruction : 1,
 					s2 : 1,
-					read : 1;
+					read : 1,
+					ttrnw : 1,
+					class_tt : 1;
 	u8				id;
 	u8				class;
 	u16				stag;
@@ -787,6 +798,7 @@ struct arm_smmu_event {
 	u64				iova;
 	u64				ipa;
 	u64				fetch_addr;
+	struct device			*dev;
 };
 
 /* SMMU private data for each master */
-- 
2.47.0.338.g60cca15819-goog

Re: [PATCH v6 0/2] iommu/arm-smmu-v3: Parse out event records

[Will Deacon]

On Tue, 03 Dec 2024 18:49:04 +0000, Pranjal Shrivastava wrote:
> Enhance the arm-smmu-v3 driver to parse out useful information from
> event records into a structure for better event handling & logging.
> 
> Some sample events, powered by QEMU:
> 
> 1. Bad StreamID:
> 
> [...]

Applied to will (for-joerg/arm-smmu/updates), thanks!

[1/2] iommu/arm-smmu-v3: Introduce struct arm_smmu_event
      https://git.kernel.org/will/c/43ca55f5555b
[2/2] iommu/arm-smmu-v3: Log better event records
      https://git.kernel.org/will/c/d814b70b9b90

One thing I noticed when playing around with this on Qemu, however, is
that we print the raw hex dump in reverse compared to the way in which
the record tables are described in the spec. What do you think about
doing something like the following on top of your series?


diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 143ff6336a95..d33a150b29ef 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -1880,8 +1880,8 @@ static void arm_smmu_dump_raw_event(struct arm_smmu_device *smmu, u64 *raw,
 
 	dev_err(smmu->dev, "event 0x%02x received:\n", event->id);
 
-	for (i = 0; i < EVTQ_ENT_DWORDS; ++i)
-		dev_err(smmu->dev, "\t0x%016llx\n", raw[i]);
+	for (i = EVTQ_ENT_DWORDS - 1; i >= 0; --i)
+		dev_err(smmu->dev, "[%u]\t0x%016llx\n", i, raw[i]);
 }
 
 #define ARM_SMMU_EVT_KNOWN(e)	((e)->id < ARRAY_SIZE(event_str) && event_str[(e)->id])


Then, for example, a permission fault looks like:

[   32.543815] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x13 received:
[   32.545115] arm-smmu-v3 arm-smmu-v3.0.auto: [3]	0x0000000000000000
[   32.546361] arm-smmu-v3 arm-smmu-v3.0.auto: [2]	0x00000000fffff040
[   32.548672] arm-smmu-v3 arm-smmu-v3.0.auto: [1]	0x0000020000000000
[   32.550209] arm-smmu-v3 arm-smmu-v3.0.auto: [0]	0x0000002000000013
[   32.551990] arm-smmu-v3 arm-smmu-v3.0.auto: event: F_PERMISSION client: 0000:00:04.0 sid: 0x20 ssid: 0x0 iova: 0xfffff040 ipa: 0x0
[   32.553952] arm-smmu-v3 arm-smmu-v3.0.auto: unpriv data write s1 "Input address caused fault" stag: 0x0

I started going a step further and printing the records 32 bits at a
time (again, to match the spec), but then I realised that the current
code doesn't look like it handles big-endian properly...

Finally, coccinelle suggests you could use str_read_write() instead of:

	evt->read ? "read" : "write",

which I suppose we could.

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

Re: [PATCH v6 0/2] iommu/arm-smmu-v3: Parse out event records

[Pranjal Shrivastava]

On Tue, Dec 10, 2024 at 12:17:29AM +0000, Will Deacon wrote:
> On Tue, 03 Dec 2024 18:49:04 +0000, Pranjal Shrivastava wrote:
> > Enhance the arm-smmu-v3 driver to parse out useful information from
> > event records into a structure for better event handling & logging.
> > 
> > Some sample events, powered by QEMU:
> > 
> > 1. Bad StreamID:
> > 
> > [...]
> 
> Applied to will (for-joerg/arm-smmu/updates), thanks!
> 
> [1/2] iommu/arm-smmu-v3: Introduce struct arm_smmu_event
>       https://git.kernel.org/will/c/43ca55f5555b
> [2/2] iommu/arm-smmu-v3: Log better event records
>       https://git.kernel.org/will/c/d814b70b9b90
> 
> One thing I noticed when playing around with this on Qemu, however, is
> that we print the raw hex dump in reverse compared to the way in which
> the record tables are described in the spec. What do you think about
> doing something like the following on top of your series?
> 
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 143ff6336a95..d33a150b29ef 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -1880,8 +1880,8 @@ static void arm_smmu_dump_raw_event(struct arm_smmu_device *smmu, u64 *raw,
>  
>  	dev_err(smmu->dev, "event 0x%02x received:\n", event->id);
>  
> -	for (i = 0; i < EVTQ_ENT_DWORDS; ++i)
> -		dev_err(smmu->dev, "\t0x%016llx\n", raw[i]);
> +	for (i = EVTQ_ENT_DWORDS - 1; i >= 0; --i)
> +		dev_err(smmu->dev, "[%u]\t0x%016llx\n", i, raw[i]);
>  }
>  
>  #define ARM_SMMU_EVT_KNOWN(e)	((e)->id < ARRAY_SIZE(event_str) && event_str[(e)->id])
> 
> 
> Then, for example, a permission fault looks like:
> 
> [   32.543815] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x13 received:
> [   32.545115] arm-smmu-v3 arm-smmu-v3.0.auto: [3]	0x0000000000000000
> [   32.546361] arm-smmu-v3 arm-smmu-v3.0.auto: [2]	0x00000000fffff040
> [   32.548672] arm-smmu-v3 arm-smmu-v3.0.auto: [1]	0x0000020000000000
> [   32.550209] arm-smmu-v3 arm-smmu-v3.0.auto: [0]	0x0000002000000013
> [   32.551990] arm-smmu-v3 arm-smmu-v3.0.auto: event: F_PERMISSION client: 0000:00:04.0 sid: 0x20 ssid: 0x0 iova: 0xfffff040 ipa: 0x0
> [   32.553952] arm-smmu-v3 arm-smmu-v3.0.auto: unpriv data write s1 "Input address caused fault" stag: 0x0
> 
> I started going a step further and printing the records 32 bits at a
> time (again, to match the spec), but then I realised that the current
> code doesn't look like it handles big-endian properly...

Ack. I think I can post another series to refactor this to match the
spec by logging 32-bits at a time in Little-endian. I'll give it a go..

> 
> Finally, coccinelle suggests you could use str_read_write() instead of:
> 
> 	evt->read ? "read" : "write",
> 
> which I suppose we could.

Ack. I'll use `str_read_write` do you want me to club it with the above
refactor in a series?

> 
> Cheers,
> -- 
> Will

Thanks,
Praan

Re: [PATCH v6 0/2] iommu/arm-smmu-v3: Parse out event records

[Will Deacon]

On Mon, Jan 06, 2025 at 11:29:00AM +0000, Pranjal Shrivastava wrote:
> On Tue, Dec 10, 2024 at 12:17:29AM +0000, Will Deacon wrote:
> > On Tue, 03 Dec 2024 18:49:04 +0000, Pranjal Shrivastava wrote:
> > > Enhance the arm-smmu-v3 driver to parse out useful information from
> > > event records into a structure for better event handling & logging.
> > > 
> > > Some sample events, powered by QEMU:
> > > 
> > > 1. Bad StreamID:
> > > 
> > > [...]
> > 
> > Applied to will (for-joerg/arm-smmu/updates), thanks!
> > 
> > [1/2] iommu/arm-smmu-v3: Introduce struct arm_smmu_event
> >       https://git.kernel.org/will/c/43ca55f5555b
> > [2/2] iommu/arm-smmu-v3: Log better event records
> >       https://git.kernel.org/will/c/d814b70b9b90
> > 
> > One thing I noticed when playing around with this on Qemu, however, is
> > that we print the raw hex dump in reverse compared to the way in which
> > the record tables are described in the spec. What do you think about
> > doing something like the following on top of your series?
> > 
> > 
> > diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > index 143ff6336a95..d33a150b29ef 100644
> > --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> > @@ -1880,8 +1880,8 @@ static void arm_smmu_dump_raw_event(struct arm_smmu_device *smmu, u64 *raw,
> >  
> >  	dev_err(smmu->dev, "event 0x%02x received:\n", event->id);
> >  
> > -	for (i = 0; i < EVTQ_ENT_DWORDS; ++i)
> > -		dev_err(smmu->dev, "\t0x%016llx\n", raw[i]);
> > +	for (i = EVTQ_ENT_DWORDS - 1; i >= 0; --i)
> > +		dev_err(smmu->dev, "[%u]\t0x%016llx\n", i, raw[i]);
> >  }
> >  
> >  #define ARM_SMMU_EVT_KNOWN(e)	((e)->id < ARRAY_SIZE(event_str) && event_str[(e)->id])
> > 
> > 
> > Then, for example, a permission fault looks like:
> > 
> > [   32.543815] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x13 received:
> > [   32.545115] arm-smmu-v3 arm-smmu-v3.0.auto: [3]	0x0000000000000000
> > [   32.546361] arm-smmu-v3 arm-smmu-v3.0.auto: [2]	0x00000000fffff040
> > [   32.548672] arm-smmu-v3 arm-smmu-v3.0.auto: [1]	0x0000020000000000
> > [   32.550209] arm-smmu-v3 arm-smmu-v3.0.auto: [0]	0x0000002000000013
> > [   32.551990] arm-smmu-v3 arm-smmu-v3.0.auto: event: F_PERMISSION client: 0000:00:04.0 sid: 0x20 ssid: 0x0 iova: 0xfffff040 ipa: 0x0
> > [   32.553952] arm-smmu-v3 arm-smmu-v3.0.auto: unpriv data write s1 "Input address caused fault" stag: 0x0
> > 
> > I started going a step further and printing the records 32 bits at a
> > time (again, to match the spec), but then I realised that the current
> > code doesn't look like it handles big-endian properly...
> 
> Ack. I think I can post another series to refactor this to match the
> spec by logging 32-bits at a time in Little-endian. I'll give it a go..

Thanks.

> > Finally, coccinelle suggests you could use str_read_write() instead of:
> > 
> > 	evt->read ? "read" : "write",
> > 
> > which I suppose we could.
> 
> Ack. I'll use `str_read_write` do you want me to club it with the above
> refactor in a series?

It's a bit obscure, but I suppose it preempts the inevitable patch we'll
get when somebody else runs the tool.

Will