[PATCH 0/4] Add DPA->HPA translation to dram & general_media

[PATCH 0/4] Add DPA->HPA translation to dram & general_media

[alison.schofield]

From: Alison Schofield <alison.schofield@intel.com>

Add HPA translations to CXL events: cxl_dram and cxl_general_media

Patches 1 & 2:
Before adding the new support, do some housekeeping and move related
helpers to the region driver because there is no looking up region
related info without CONFIG_CXL_REGION.

Patch 3:
The new functionality is introduced - cxl_dram & cxl_general_media
events will lookup and log the DPA->HPA translation along with the
region name and region uuid.

Patch 4:
Apply the new lookup helpers to the existing cxl_poison event, so it
can be the same, share in the new goodness, and also tidy up its
implementation.

An update to the cxl_event unit test is posted separately.


Alison Schofield (4):
  cxl/region: Move cxl_dpa_to_region() work to the region driver
  cxl/region: Move cxl_trace_hpa() work to the region driver
  cxl/core: Add region info to cxl_general_media and cxl_dram events
  cxl/core: Remove cxlr dependency from cxl_poison trace events

 drivers/cxl/core/core.h   |  20 +++++
 drivers/cxl/core/mbox.c   |  22 ++++--
 drivers/cxl/core/memdev.c |  52 +------------
 drivers/cxl/core/region.c | 151 +++++++++++++++++++++++++++++++++++++-
 drivers/cxl/core/trace.c  |  91 -----------------------
 drivers/cxl/core/trace.h  |  76 +++++++++++++------
 drivers/cxl/cxlmem.h      |   3 +-
 7 files changed, 239 insertions(+), 176 deletions(-)


base-commit: 4cece764965020c22cff7665b18a012006359095
-- 
2.37.3

[PATCH 1/4] cxl/region: Move cxl_dpa_to_region() work to the region driver

[alison.schofield]

From: Alison Schofield <alison.schofield@intel.com>

This helper belongs in the region driver as it is only useful
with CONFIG_CXL_REGION. Add stubs in core.h for when the region
driver is not built.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 drivers/cxl/core/core.h   |  7 +++++++
 drivers/cxl/core/memdev.c | 44 ---------------------------------------
 drivers/cxl/core/region.c | 44 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 51 insertions(+), 44 deletions(-)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index bc5a95665aa0..6ceba8037101 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -27,7 +27,14 @@ void cxl_decoder_kill_region(struct cxl_endpoint_decoder *cxled);
 int cxl_region_init(void);
 void cxl_region_exit(void);
 int cxl_get_poison_by_endpoint(struct cxl_port *port);
+struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
+
 #else
+static inline
+struct cxl_region cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
+{
+	return NULL;
+}
 static inline int cxl_get_poison_by_endpoint(struct cxl_port *port)
 {
 	return 0;
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index d4e259f3a7e9..0277726afd04 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -251,50 +251,6 @@ int cxl_trigger_poison_list(struct cxl_memdev *cxlmd)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_trigger_poison_list, CXL);
 
-struct cxl_dpa_to_region_context {
-	struct cxl_region *cxlr;
-	u64 dpa;
-};
-
-static int __cxl_dpa_to_region(struct device *dev, void *arg)
-{
-	struct cxl_dpa_to_region_context *ctx = arg;
-	struct cxl_endpoint_decoder *cxled;
-	u64 dpa = ctx->dpa;
-
-	if (!is_endpoint_decoder(dev))
-		return 0;
-
-	cxled = to_cxl_endpoint_decoder(dev);
-	if (!cxled->dpa_res || !resource_size(cxled->dpa_res))
-		return 0;
-
-	if (dpa > cxled->dpa_res->end || dpa < cxled->dpa_res->start)
-		return 0;
-
-	dev_dbg(dev, "dpa:0x%llx mapped in region:%s\n", dpa,
-		dev_name(&cxled->cxld.region->dev));
-
-	ctx->cxlr = cxled->cxld.region;
-
-	return 1;
-}
-
-static struct cxl_region *cxl_dpa_to_region(struct cxl_memdev *cxlmd, u64 dpa)
-{
-	struct cxl_dpa_to_region_context ctx;
-	struct cxl_port *port;
-
-	ctx = (struct cxl_dpa_to_region_context) {
-		.dpa = dpa,
-	};
-	port = cxlmd->endpoint;
-	if (port && is_cxl_endpoint(port) && cxl_num_decoders_committed(port))
-		device_for_each_child(&port->dev, &ctx, __cxl_dpa_to_region);
-
-	return ctx.cxlr;
-}
-
 static int cxl_validate_poison_dpa(struct cxl_memdev *cxlmd, u64 dpa)
 {
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 5c186e0a39b9..4b227659e3f8 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2679,6 +2679,50 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port)
 	return rc;
 }
 
+struct cxl_dpa_to_region_context {
+	struct cxl_region *cxlr;
+	u64 dpa;
+};
+
+static int __cxl_dpa_to_region(struct device *dev, void *arg)
+{
+	struct cxl_dpa_to_region_context *ctx = arg;
+	struct cxl_endpoint_decoder *cxled;
+	u64 dpa = ctx->dpa;
+
+	if (!is_endpoint_decoder(dev))
+		return 0;
+
+	cxled = to_cxl_endpoint_decoder(dev);
+	if (!cxled->dpa_res || !resource_size(cxled->dpa_res))
+		return 0;
+
+	if (dpa > cxled->dpa_res->end || dpa < cxled->dpa_res->start)
+		return 0;
+
+	dev_dbg(dev, "dpa:0x%llx mapped in region:%s\n", dpa,
+		dev_name(&cxled->cxld.region->dev));
+
+	ctx->cxlr = cxled->cxld.region;
+
+	return 1;
+}
+
+struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
+{
+	struct cxl_dpa_to_region_context ctx;
+	struct cxl_port *port;
+
+	ctx = (struct cxl_dpa_to_region_context) {
+		.dpa = dpa,
+	};
+	port = cxlmd->endpoint;
+	if (port && is_cxl_endpoint(port) && cxl_num_decoders_committed(port))
+		device_for_each_child(&port->dev, &ctx, __cxl_dpa_to_region);
+
+	return ctx.cxlr;
+}
+
 static struct lock_class_key cxl_pmem_region_key;
 
 static struct cxl_pmem_region *cxl_pmem_region_alloc(struct cxl_region *cxlr)
-- 
2.37.3

[PATCH 2/4] cxl/region: Move cxl_trace_hpa() work to the region driver

[alison.schofield]

From: Alison Schofield <alison.schofield@intel.com>

This work belongs in the region driver as it is only useful with
CONFIG_CXL_REGION. Add stubs in core.h for when the region driver
is not built.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 drivers/cxl/core/core.h   |  7 +++
 drivers/cxl/core/region.c | 91 +++++++++++++++++++++++++++++++++++++++
 drivers/cxl/core/trace.c  | 91 ---------------------------------------
 drivers/cxl/core/trace.h  |  2 -
 4 files changed, 98 insertions(+), 93 deletions(-)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 6ceba8037101..24454a1cb250 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -28,8 +28,15 @@ int cxl_region_init(void);
 void cxl_region_exit(void);
 int cxl_get_poison_by_endpoint(struct cxl_port *port);
 struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
+u64 cxl_trace_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
+		  u64 dpa);
 
 #else
+static inline u64
+cxl_trace_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd, u64 dpa)
+{
+	return ULLONG_MAX;
+}
 static inline
 struct cxl_region cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
 {
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 4b227659e3f8..45eb9c560fd6 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2723,6 +2723,97 @@ struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
 	return ctx.cxlr;
 }
 
+static bool cxl_is_hpa_in_range(u64 hpa, struct cxl_region *cxlr, int pos)
+{
+	struct cxl_region_params *p = &cxlr->params;
+	int gran = p->interleave_granularity;
+	int ways = p->interleave_ways;
+	u64 offset;
+
+	/* Is the hpa within this region at all */
+	if (hpa < p->res->start || hpa > p->res->end) {
+		dev_dbg(&cxlr->dev,
+			"Addr trans fail: hpa 0x%llx not in region\n", hpa);
+		return false;
+	}
+
+	/* Is the hpa in an expected chunk for its pos(-ition) */
+	offset = hpa - p->res->start;
+	offset = do_div(offset, gran * ways);
+	if ((offset >= pos * gran) && (offset < (pos + 1) * gran))
+		return true;
+
+	dev_dbg(&cxlr->dev,
+		"Addr trans fail: hpa 0x%llx not in expected chunk\n", hpa);
+
+	return false;
+}
+
+static u64 cxl_dpa_to_hpa(u64 dpa,  struct cxl_region *cxlr,
+			  struct cxl_endpoint_decoder *cxled)
+{
+	u64 dpa_offset, hpa_offset, bits_upper, mask_upper, hpa;
+	struct cxl_region_params *p = &cxlr->params;
+	int pos = cxled->pos;
+	u16 eig = 0;
+	u8 eiw = 0;
+
+	ways_to_eiw(p->interleave_ways, &eiw);
+	granularity_to_eig(p->interleave_granularity, &eig);
+
+	/*
+	 * The device position in the region interleave set was removed
+	 * from the offset at HPA->DPA translation. To reconstruct the
+	 * HPA, place the 'pos' in the offset.
+	 *
+	 * The placement of 'pos' in the HPA is determined by interleave
+	 * ways and granularity and is defined in the CXL Spec 3.0 Section
+	 * 8.2.4.19.13 Implementation Note: Device Decode Logic
+	 */
+
+	/* Remove the dpa base */
+	dpa_offset = dpa - cxl_dpa_resource_start(cxled);
+
+	mask_upper = GENMASK_ULL(51, eig + 8);
+
+	if (eiw < 8) {
+		hpa_offset = (dpa_offset & mask_upper) << eiw;
+		hpa_offset |= pos << (eig + 8);
+	} else {
+		bits_upper = (dpa_offset & mask_upper) >> (eig + 8);
+		bits_upper = bits_upper * 3;
+		hpa_offset = ((bits_upper << (eiw - 8)) + pos) << (eig + 8);
+	}
+
+	/* The lower bits remain unchanged */
+	hpa_offset |= dpa_offset & GENMASK_ULL(eig + 7, 0);
+
+	/* Apply the hpa_offset to the region base address */
+	hpa = hpa_offset + p->res->start;
+
+	if (!cxl_is_hpa_in_range(hpa, cxlr, cxled->pos))
+		return ULLONG_MAX;
+
+	return hpa;
+}
+
+u64 cxl_trace_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
+		  u64 dpa)
+{
+	struct cxl_region_params *p = &cxlr->params;
+	struct cxl_endpoint_decoder *cxled = NULL;
+
+	for (int i = 0; i <  p->nr_targets; i++) {
+		cxled = p->targets[i];
+		if (cxlmd == cxled_to_memdev(cxled))
+			break;
+	}
+	if (!cxled || cxlmd != cxled_to_memdev(cxled))
+		return ULLONG_MAX;
+
+	return cxl_dpa_to_hpa(dpa, cxlr, cxled);
+}
+
 static struct lock_class_key cxl_pmem_region_key;
 
 static struct cxl_pmem_region *cxl_pmem_region_alloc(struct cxl_region *cxlr)
diff --git a/drivers/cxl/core/trace.c b/drivers/cxl/core/trace.c
index d0403dc3c8ab..7f2a9dd0d0e3 100644
--- a/drivers/cxl/core/trace.c
+++ b/drivers/cxl/core/trace.c
@@ -6,94 +6,3 @@
 
 #define CREATE_TRACE_POINTS
 #include "trace.h"
-
-static bool cxl_is_hpa_in_range(u64 hpa, struct cxl_region *cxlr, int pos)
-{
-	struct cxl_region_params *p = &cxlr->params;
-	int gran = p->interleave_granularity;
-	int ways = p->interleave_ways;
-	u64 offset;
-
-	/* Is the hpa within this region at all */
-	if (hpa < p->res->start || hpa > p->res->end) {
-		dev_dbg(&cxlr->dev,
-			"Addr trans fail: hpa 0x%llx not in region\n", hpa);
-		return false;
-	}
-
-	/* Is the hpa in an expected chunk for its pos(-ition) */
-	offset = hpa - p->res->start;
-	offset = do_div(offset, gran * ways);
-	if ((offset >= pos * gran) && (offset < (pos + 1) * gran))
-		return true;
-
-	dev_dbg(&cxlr->dev,
-		"Addr trans fail: hpa 0x%llx not in expected chunk\n", hpa);
-
-	return false;
-}
-
-static u64 cxl_dpa_to_hpa(u64 dpa,  struct cxl_region *cxlr,
-			  struct cxl_endpoint_decoder *cxled)
-{
-	u64 dpa_offset, hpa_offset, bits_upper, mask_upper, hpa;
-	struct cxl_region_params *p = &cxlr->params;
-	int pos = cxled->pos;
-	u16 eig = 0;
-	u8 eiw = 0;
-
-	ways_to_eiw(p->interleave_ways, &eiw);
-	granularity_to_eig(p->interleave_granularity, &eig);
-
-	/*
-	 * The device position in the region interleave set was removed
-	 * from the offset at HPA->DPA translation. To reconstruct the
-	 * HPA, place the 'pos' in the offset.
-	 *
-	 * The placement of 'pos' in the HPA is determined by interleave
-	 * ways and granularity and is defined in the CXL Spec 3.0 Section
-	 * 8.2.4.19.13 Implementation Note: Device Decode Logic
-	 */
-
-	/* Remove the dpa base */
-	dpa_offset = dpa - cxl_dpa_resource_start(cxled);
-
-	mask_upper = GENMASK_ULL(51, eig + 8);
-
-	if (eiw < 8) {
-		hpa_offset = (dpa_offset & mask_upper) << eiw;
-		hpa_offset |= pos << (eig + 8);
-	} else {
-		bits_upper = (dpa_offset & mask_upper) >> (eig + 8);
-		bits_upper = bits_upper * 3;
-		hpa_offset = ((bits_upper << (eiw - 8)) + pos) << (eig + 8);
-	}
-
-	/* The lower bits remain unchanged */
-	hpa_offset |= dpa_offset & GENMASK_ULL(eig + 7, 0);
-
-	/* Apply the hpa_offset to the region base address */
-	hpa = hpa_offset + p->res->start;
-
-	if (!cxl_is_hpa_in_range(hpa, cxlr, cxled->pos))
-		return ULLONG_MAX;
-
-	return hpa;
-}
-
-u64 cxl_trace_hpa(struct cxl_region *cxlr, struct cxl_memdev *cxlmd,
-		  u64 dpa)
-{
-	struct cxl_region_params *p = &cxlr->params;
-	struct cxl_endpoint_decoder *cxled = NULL;
-
-	for (int i = 0; i <  p->nr_targets; i++) {
-		cxled = p->targets[i];
-		if (cxlmd == cxled_to_memdev(cxled))
-			break;
-	}
-	if (!cxled || cxlmd != cxled_to_memdev(cxled))
-		return ULLONG_MAX;
-
-	return cxl_dpa_to_hpa(dpa, cxlr, cxled);
-}
diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
index e5f13260fc52..161bdb5734b0 100644
--- a/drivers/cxl/core/trace.h
+++ b/drivers/cxl/core/trace.h
@@ -642,8 +642,6 @@ TRACE_EVENT(cxl_memory_module,
 #define cxl_poison_overflow(flags, time)				\
 	(flags & CXL_POISON_FLAG_OVERFLOW ? le64_to_cpu(time) : 0)
 
-u64 cxl_trace_hpa(struct cxl_region *cxlr, struct cxl_memdev *memdev, u64 dpa);
-
 TRACE_EVENT(cxl_poison,
 
 	TP_PROTO(struct cxl_memdev *cxlmd, struct cxl_region *cxlr,
-- 
2.37.3

[PATCH 3/4] cxl/core: Add region info to cxl_general_media and cxl_dram events

[alison.schofield]

From: Alison Schofield <alison.schofield@intel.com>

User space may need to know which region, if any, maps the DPAs
(device physical addresses) reported in a cxl_general_media or
cxl_dram event. Since the mapping can change, the kernel provides
this information at the time the event occurs. This informs user
space that at event <timestamp> this <region> mapped this <DPA>
to this <HPA>.

Add the same region info that is included in the cxl_poison trace
event: the DPA->HPA translation, region name, and region uuid.
Introduce and use new helpers that lookup that region info using
the struct cxl_memdev and a DPA.

The new fields are inserted in the trace event and no existing
fields are modified. If the DPA is not mapped, user will see:
hpa=ULLONG_MAX, region="", and uuid=0

This work must be protected by dpa_rwsem & region_rwsem since
it is looking up region mappings.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 drivers/cxl/core/core.h   |  6 +++++
 drivers/cxl/core/mbox.c   | 17 ++++++++++----
 drivers/cxl/core/region.c |  8 +++++++
 drivers/cxl/core/trace.h  | 47 ++++++++++++++++++++++++++++++++++-----
 4 files changed, 69 insertions(+), 9 deletions(-)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 24454a1cb250..848ef6904beb 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -30,8 +30,14 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port);
 struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
 u64 cxl_trace_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
 		  u64 dpa);
+const char *cxl_trace_to_region_name(const struct cxl_memdev *cxlmd, u64 dpa);
 
 #else
+static inline
+const char *cxl_trace_to_region_name(const struct cxl_memdev *cxlmd, u64 dpa)
+{
+	return NULL;
+}
 static inline u64
 cxl_trace_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd, u64 dpa)
 {
diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 9adda4795eb7..3c1c37d5fcb0 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -842,14 +842,23 @@ void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
 			    enum cxl_event_type event_type,
 			    const uuid_t *uuid, union cxl_event *evt)
 {
+	if (event_type == CXL_CPER_EVENT_MEM_MODULE) {
+		trace_cxl_memory_module(cxlmd, type, &evt->mem_module);
+		return;
+	}
+	if (event_type == CXL_CPER_EVENT_GENERIC) {
+		trace_cxl_generic_event(cxlmd, type, uuid, &evt->generic);
+		return;
+	}
+
+	/* Protect trace events that do DPA->HPA translations */
+	guard(rwsem_read)(&cxl_region_rwsem);
+	guard(rwsem_read)(&cxl_dpa_rwsem);
+
 	if (event_type == CXL_CPER_EVENT_GEN_MEDIA)
 		trace_cxl_general_media(cxlmd, type, &evt->gen_media);
 	else if (event_type == CXL_CPER_EVENT_DRAM)
 		trace_cxl_dram(cxlmd, type, &evt->dram);
-	else if (event_type == CXL_CPER_EVENT_MEM_MODULE)
-		trace_cxl_memory_module(cxlmd, type, &evt->mem_module);
-	else
-		trace_cxl_generic_event(cxlmd, type, uuid, &evt->generic);
 }
 EXPORT_SYMBOL_NS_GPL(cxl_event_trace_record, CXL);
 
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 45eb9c560fd6..a5b1eaee1e58 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2723,6 +2723,14 @@ struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
 	return ctx.cxlr;
 }
 
+const char *cxl_trace_to_region_name(const struct cxl_memdev *cxlmd, u64 dpa)
+{
+	struct cxl_region *cxlr = cxl_dpa_to_region(cxlmd, dpa);
+
+	/* trace __string() assignment requires "", not NULL */
+	return cxlr ? dev_name(&cxlr->dev) : "";
+}
+
 static bool cxl_is_hpa_in_range(u64 hpa, struct cxl_region *cxlr, int pos)
 {
 	struct cxl_region_params *p = &cxlr->params;
diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
index 161bdb5734b0..6ad4998aeb9a 100644
--- a/drivers/cxl/core/trace.h
+++ b/drivers/cxl/core/trace.h
@@ -14,6 +14,22 @@
 #include <cxlmem.h>
 #include "core.h"
 
+#define to_region_name(cxlmd, dpa)					\
+	(cxl_trace_to_region_name(cxlmd, dpa))
+
+#define store_region_info(cxlmd, dpa, entry_uuid, entry_hpa)		\
+	do {								\
+		struct cxl_region *cxlr;				\
+									\
+		cxlr = cxl_dpa_to_region(cxlmd, dpa);			\
+		if (cxlr) {						\
+			uuid_copy(&(entry_uuid), &cxlr->params.uuid);	\
+			entry_hpa = cxl_trace_hpa(cxlr, cxlmd, dpa);	\
+		} else {						\
+			entry_hpa = ULLONG_MAX;				\
+		}							\
+	} while (0)
+
 #define CXL_RAS_UC_CACHE_DATA_PARITY	BIT(0)
 #define CXL_RAS_UC_CACHE_ADDR_PARITY	BIT(1)
 #define CXL_RAS_UC_CACHE_BE_PARITY	BIT(2)
@@ -313,6 +329,9 @@ TRACE_EVENT(cxl_generic_event,
 	{ CXL_GMER_VALID_COMPONENT,			"COMPONENT"	}  \
 )
 
+#define to_gm_dpa(record)						\
+	(le64_to_cpu(rec->phys_addr) & CXL_DPA_MASK)
+
 TRACE_EVENT(cxl_general_media,
 
 	TP_PROTO(const struct cxl_memdev *cxlmd, enum cxl_event_log_type log,
@@ -330,10 +349,13 @@ TRACE_EVENT(cxl_general_media,
 		__field(u8, channel)
 		__field(u32, device)
 		__array(u8, comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
-		__field(u16, validity_flags)
 		/* Following are out of order to pack trace record */
+		__field(u64, hpa)
+		__field_struct(uuid_t, region_uuid)
+		__field(u16, validity_flags)
 		__field(u8, rank)
 		__field(u8, dpa_flags)
+		__string(region_name, to_region_name(cxlmd, to_gm_dpa(record)))
 	),
 
 	TP_fast_assign(
@@ -354,18 +376,23 @@ TRACE_EVENT(cxl_general_media,
 		memcpy(__entry->comp_id, &rec->component_id,
 			CXL_EVENT_GEN_MED_COMP_ID_SIZE);
 		__entry->validity_flags = get_unaligned_le16(&rec->validity_flags);
+		__assign_str(region_name, to_region_name(cxlmd, to_gm_dpa(record)));
+		store_region_info(cxlmd, to_gm_dpa(record),
+				  __entry->region_uuid, __entry->hpa);
 	),
 
 	CXL_EVT_TP_printk("dpa=%llx dpa_flags='%s' " \
 		"descriptor='%s' type='%s' transaction_type='%s' channel=%u rank=%u " \
-		"device=%x comp_id=%s validity_flags='%s'",
+		"device=%x comp_id=%s validity_flags='%s' " \
+		"hpa=%llx region=%s region_uuid=%pUb",
 		__entry->dpa, show_dpa_flags(__entry->dpa_flags),
 		show_event_desc_flags(__entry->descriptor),
 		show_mem_event_type(__entry->type),
 		show_trans_type(__entry->transaction_type),
 		__entry->channel, __entry->rank, __entry->device,
 		__print_hex(__entry->comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE),
-		show_valid_flags(__entry->validity_flags)
+		show_valid_flags(__entry->validity_flags),
+		__entry->hpa, __get_str(region_name), &__entry->region_uuid
 	)
 );
 
@@ -396,6 +423,8 @@ TRACE_EVENT(cxl_general_media,
 	{ CXL_DER_VALID_COLUMN,				"COLUMN"		}, \
 	{ CXL_DER_VALID_CORRECTION_MASK,		"CORRECTION MASK"	}  \
 )
+#define to_dram_dpa(record)						\
+	(le64_to_cpu(rec->phys_addr) & CXL_DPA_MASK)
 
 TRACE_EVENT(cxl_dram,
 
@@ -417,10 +446,13 @@ TRACE_EVENT(cxl_dram,
 		__field(u32, nibble_mask)
 		__field(u32, row)
 		__array(u8, cor_mask, CXL_EVENT_DER_CORRECTION_MASK_SIZE)
+		__field(u64, hpa)
+		__field_struct(uuid_t, region_uuid)
 		__field(u8, rank)	/* Out of order to pack trace record */
 		__field(u8, bank_group)	/* Out of order to pack trace record */
 		__field(u8, bank)	/* Out of order to pack trace record */
 		__field(u8, dpa_flags)	/* Out of order to pack trace record */
+		__string(region_name, to_region_name(cxlmd, to_dram_dpa(record)))
 	),
 
 	TP_fast_assign(
@@ -444,12 +476,16 @@ TRACE_EVENT(cxl_dram,
 		__entry->column = get_unaligned_le16(rec->column);
 		memcpy(__entry->cor_mask, &rec->correction_mask,
 			CXL_EVENT_DER_CORRECTION_MASK_SIZE);
+		__assign_str(region_name, to_region_name(cxlmd, to_dram_dpa(record)));
+		store_region_info(cxlmd, __entry->dpa, __entry->region_uuid,
+				  __entry->hpa);
 	),
 
 	CXL_EVT_TP_printk("dpa=%llx dpa_flags='%s' descriptor='%s' type='%s' " \
 		"transaction_type='%s' channel=%u rank=%u nibble_mask=%x " \
 		"bank_group=%u bank=%u row=%u column=%u cor_mask=%s " \
-		"validity_flags='%s'",
+		"validity_flags='%s'" \
+		"hpa=%llx region=%s region_uuid=%pUb",
 		__entry->dpa, show_dpa_flags(__entry->dpa_flags),
 		show_event_desc_flags(__entry->descriptor),
 		show_mem_event_type(__entry->type),
@@ -458,7 +494,8 @@ TRACE_EVENT(cxl_dram,
 		__entry->bank_group, __entry->bank,
 		__entry->row, __entry->column,
 		__print_hex(__entry->cor_mask, CXL_EVENT_DER_CORRECTION_MASK_SIZE),
-		show_dram_valid_flags(__entry->validity_flags)
+		show_dram_valid_flags(__entry->validity_flags),
+		__entry->hpa, __get_str(region_name), &__entry->region_uuid
 	)
 );
 
-- 
2.37.3

[PATCH 4/4] cxl/core: Remove cxlr dependency from cxl_poison trace events

[alison.schofield]

From: Alison Schofield <alison.schofield@intel.com>

cxl_poison trace events pass a pointer to a struct cxl_region
when poison is discovered in a mapped endpoint. This made for
easy look up of region name, uuid, and was useful in starting
the dpa->hpa translation.

Another set of trace helpers is now available that eliminates
the need to pass on that cxlr. (It was passed along a lot!)

In addition to tidying up the cxl_poison calling path, shifting
to the new helpers also means all CXL trace events will share
the same code in that regard.

Switch from a uuid array to the field_struct type uuid_t to
align on one uuid format in all CXL trace events. That is useful
when sharing region uuid lookup helpers.

No externally visible naming changes are made to cxl_poison trace
events.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 drivers/cxl/core/mbox.c   |  5 ++---
 drivers/cxl/core/memdev.c |  8 ++++----
 drivers/cxl/core/region.c |  8 ++++----
 drivers/cxl/core/trace.h  | 27 ++++++++++-----------------
 drivers/cxl/cxlmem.h      |  3 +--
 5 files changed, 21 insertions(+), 30 deletions(-)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 3c1c37d5fcb0..60a51ea3ff25 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1299,8 +1299,7 @@ int cxl_set_timestamp(struct cxl_memdev_state *mds)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_set_timestamp, CXL);
 
-int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len,
-		       struct cxl_region *cxlr)
+int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len)
 {
 	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
 	struct cxl_mbox_poison_out *po;
@@ -1332,7 +1331,7 @@ int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len,
 			break;
 
 		for (int i = 0; i < le16_to_cpu(po->count); i++)
-			trace_cxl_poison(cxlmd, cxlr, &po->record[i],
+			trace_cxl_poison(cxlmd, &po->record[i],
 					 po->flags, po->overflow_ts,
 					 CXL_POISON_TRACE_LIST);
 
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 0277726afd04..1a0b596da7b6 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -200,14 +200,14 @@ static int cxl_get_poison_by_memdev(struct cxl_memdev *cxlmd)
 	if (resource_size(&cxlds->pmem_res)) {
 		offset = cxlds->pmem_res.start;
 		length = resource_size(&cxlds->pmem_res);
-		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
+		rc = cxl_mem_get_poison(cxlmd, offset, length);
 		if (rc)
 			return rc;
 	}
 	if (resource_size(&cxlds->ram_res)) {
 		offset = cxlds->ram_res.start;
 		length = resource_size(&cxlds->ram_res);
-		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
+		rc = cxl_mem_get_poison(cxlmd, offset, length);
 		/*
 		 * Invalid Physical Address is not an error for
 		 * volatile addresses. Device support is optional.
@@ -321,7 +321,7 @@ int cxl_inject_poison(struct cxl_memdev *cxlmd, u64 dpa)
 		.address = cpu_to_le64(dpa),
 		.length = cpu_to_le32(1),
 	};
-	trace_cxl_poison(cxlmd, cxlr, &record, 0, 0, CXL_POISON_TRACE_INJECT);
+	trace_cxl_poison(cxlmd, &record, 0, 0, CXL_POISON_TRACE_INJECT);
 out:
 	up_read(&cxl_dpa_rwsem);
 	up_read(&cxl_region_rwsem);
@@ -385,7 +385,7 @@ int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa)
 		.address = cpu_to_le64(dpa),
 		.length = cpu_to_le32(1),
 	};
-	trace_cxl_poison(cxlmd, cxlr, &record, 0, 0, CXL_POISON_TRACE_CLEAR);
+	trace_cxl_poison(cxlmd, &record, 0, 0, CXL_POISON_TRACE_CLEAR);
 out:
 	up_read(&cxl_dpa_rwsem);
 	up_read(&cxl_region_rwsem);
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index a5b1eaee1e58..8cd057fc212c 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2585,7 +2585,7 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
 	if (ctx->mode == CXL_DECODER_RAM) {
 		offset = ctx->offset;
 		length = resource_size(&cxlds->ram_res) - offset;
-		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
+		rc = cxl_mem_get_poison(cxlmd, offset, length);
 		if (rc == -EFAULT)
 			rc = 0;
 		if (rc)
@@ -2603,7 +2603,7 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
 		return 0;
 	}
 
-	return cxl_mem_get_poison(cxlmd, offset, length, NULL);
+	return cxl_mem_get_poison(cxlmd, offset, length);
 }
 
 static int poison_by_decoder(struct device *dev, void *arg)
@@ -2637,7 +2637,7 @@ static int poison_by_decoder(struct device *dev, void *arg)
 	if (cxled->skip) {
 		offset = cxled->dpa_res->start - cxled->skip;
 		length = cxled->skip;
-		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
+		rc = cxl_mem_get_poison(cxlmd, offset, length);
 		if (rc == -EFAULT && cxled->mode == CXL_DECODER_RAM)
 			rc = 0;
 		if (rc)
@@ -2646,7 +2646,7 @@ static int poison_by_decoder(struct device *dev, void *arg)
 
 	offset = cxled->dpa_res->start;
 	length = cxled->dpa_res->end - offset + 1;
-	rc = cxl_mem_get_poison(cxlmd, offset, length, cxled->cxld.region);
+	rc = cxl_mem_get_poison(cxlmd, offset, length);
 	if (rc == -EFAULT && cxled->mode == CXL_DECODER_RAM)
 		rc = 0;
 	if (rc)
diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
index 6ad4998aeb9a..2cd66c04602a 100644
--- a/drivers/cxl/core/trace.h
+++ b/drivers/cxl/core/trace.h
@@ -681,23 +681,23 @@ TRACE_EVENT(cxl_memory_module,
 
 TRACE_EVENT(cxl_poison,
 
-	TP_PROTO(struct cxl_memdev *cxlmd, struct cxl_region *cxlr,
+	TP_PROTO(const struct cxl_memdev *cxlmd,
 		 const struct cxl_poison_record *record, u8 flags,
 		 __le64 overflow_ts, enum cxl_poison_trace_type trace_type),
 
-	TP_ARGS(cxlmd, cxlr, record, flags, overflow_ts, trace_type),
+	TP_ARGS(cxlmd, record, flags, overflow_ts, trace_type),
 
 	TP_STRUCT__entry(
 		__string(memdev, dev_name(&cxlmd->dev))
 		__string(host, dev_name(cxlmd->dev.parent))
 		__field(u64, serial)
 		__field(u8, trace_type)
-		__string(region, cxlr ? dev_name(&cxlr->dev) : "")
+		__string(region, to_region_name(cxlmd, cxl_poison_record_dpa(record)))
 		__field(u64, overflow_ts)
 		__field(u64, hpa)
 		__field(u64, dpa)
 		__field(u32, dpa_length)
-		__array(char, uuid, 16)
+		__field_struct(uuid_t, uuid)
 		__field(u8, source)
 		__field(u8, flags)
 	    ),
@@ -712,27 +712,20 @@ TRACE_EVENT(cxl_poison,
 		__entry->source = cxl_poison_record_source(record);
 		__entry->trace_type = trace_type;
 		__entry->flags = flags;
-		if (cxlr) {
-			__assign_str(region, dev_name(&cxlr->dev));
-			memcpy(__entry->uuid, &cxlr->params.uuid, 16);
-			__entry->hpa = cxl_trace_hpa(cxlr, cxlmd,
-						     __entry->dpa);
-		} else {
-			__assign_str(region, "");
-			memset(__entry->uuid, 0, 16);
-			__entry->hpa = ULLONG_MAX;
-		}
+		__assign_str(region, to_region_name(cxlmd, cxl_poison_record_dpa(record)));
+		store_region_info(cxlmd, __entry->dpa, __entry->uuid,
+				  __entry->hpa);
 	    ),
 
-	TP_printk("memdev=%s host=%s serial=%lld trace_type=%s region=%s "  \
-		"region_uuid=%pU hpa=0x%llx dpa=0x%llx dpa_length=0x%x "    \
+	TP_printk("memdev=%s host=%s serial=%lld trace_type=%s region=%s"  \
+		"region_uuid=%pUb hpa=0x%llx dpa=0x%llx dpa_length=0x%x "    \
 		"source=%s flags=%s overflow_time=%llu",
 		__get_str(memdev),
 		__get_str(host),
 		__entry->serial,
 		show_poison_trace_type(__entry->trace_type),
 		__get_str(region),
-		__entry->uuid,
+		&__entry->uuid,
 		__entry->hpa,
 		__entry->dpa,
 		__entry->dpa_length,
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 20fb3b35e89e..a733b31b7799 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -828,8 +828,7 @@ void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
 			    const uuid_t *uuid, union cxl_event *evt);
 int cxl_set_timestamp(struct cxl_memdev_state *mds);
 int cxl_poison_state_init(struct cxl_memdev_state *mds);
-int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len,
-		       struct cxl_region *cxlr);
+int cxl_mem_get_poison(struct cxl_memdev *cxlmd, u64 offset, u64 len);
 int cxl_trigger_poison_list(struct cxl_memdev *cxlmd);
 int cxl_inject_poison(struct cxl_memdev *cxlmd, u64 dpa);
 int cxl_clear_poison(struct cxl_memdev *cxlmd, u64 dpa);
-- 
2.37.3

Re: [PATCH 3/4] cxl/core: Add region info to cxl_general_media and cxl_dram events

[kernel test robot]

Hi,

kernel test robot noticed the following build errors:

[auto build test ERROR on 4cece764965020c22cff7665b18a012006359095]

url:    https://github.com/intel-lab-lkp/linux/commits/alison-schofield-intel-com/cxl-region-Move-cxl_dpa_to_region-work-to-the-region-driver/20240328-123905
base:   4cece764965020c22cff7665b18a012006359095
patch link:    https://lore.kernel.org/r/061d1eac5d4e270337911199f0b0633c0ff230b4.1711598777.git.alison.schofield%40intel.com
patch subject: [PATCH 3/4] cxl/core: Add region info to cxl_general_media and cxl_dram events
config: powerpc-randconfig-001-20240328 (https://download.01.org/0day-ci/archive/20240328/202403282310.8boff281-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project.git f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240328/202403282310.8boff281-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202403282310.8boff281-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/cxl/core/trace.c:5:
   drivers/cxl/core/core.h:49:9: error: returning 'void *' from a function with incompatible result type 'struct cxl_region'
           return NULL;
                  ^~~~
   include/linux/stddef.h:8:14: note: expanded from macro 'NULL'
   #define NULL ((void *)0)
                ^~~~~~~~~~~
   In file included from drivers/cxl/core/trace.c:8:
   In file included from drivers/cxl/core/./trace.h:748:
   In file included from include/trace/define_trace.h:102:
   In file included from include/trace/trace_events.h:419:
>> drivers/cxl/core/./trace.h:380:3: error: assigning to 'struct cxl_region *' from incompatible type 'struct cxl_region'
                   store_region_info(cxlmd, to_gm_dpa(record),
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/cxl/core/./trace.h:24:8: note: expanded from macro 'store_region_info'
                   cxlr = cxl_dpa_to_region(cxlmd, dpa);                   \
                        ^
   include/trace/stages/stage6_event_callback.h:135:33: note: expanded from macro 'TP_fast_assign'
   #define TP_fast_assign(args...) args
                                   ^
   include/trace/trace_events.h:44:16: note: expanded from macro 'TRACE_EVENT'
                                PARAMS(assign),                   \
                                ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:107:25: note: expanded from macro 'PARAMS'
   #define PARAMS(args...) args
                           ^
   include/trace/trace_events.h:402:4: note: expanded from macro 'DECLARE_EVENT_CLASS'
           { assign; }                                                     \
             ^~~~~~
   In file included from drivers/cxl/core/trace.c:8:
   In file included from drivers/cxl/core/./trace.h:748:
   In file included from include/trace/define_trace.h:102:
   In file included from include/trace/trace_events.h:419:
   drivers/cxl/core/./trace.h:480:3: error: assigning to 'struct cxl_region *' from incompatible type 'struct cxl_region'
                   store_region_info(cxlmd, __entry->dpa, __entry->region_uuid,
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/cxl/core/./trace.h:24:8: note: expanded from macro 'store_region_info'
                   cxlr = cxl_dpa_to_region(cxlmd, dpa);                   \
                        ^
   include/trace/stages/stage6_event_callback.h:135:33: note: expanded from macro 'TP_fast_assign'
   #define TP_fast_assign(args...) args
                                   ^
   include/trace/trace_events.h:44:16: note: expanded from macro 'TRACE_EVENT'
                                PARAMS(assign),                   \
                                ~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/tracepoint.h:107:25: note: expanded from macro 'PARAMS'
   #define PARAMS(args...) args
                           ^
   include/trace/trace_events.h:402:4: note: expanded from macro 'DECLARE_EVENT_CLASS'
           { assign; }                                                     \
             ^~~~~~
   3 errors generated.


vim +380 drivers/cxl/core/./trace.h

   336	
   337		TP_PROTO(const struct cxl_memdev *cxlmd, enum cxl_event_log_type log,
   338			 struct cxl_event_gen_media *rec),
   339	
   340		TP_ARGS(cxlmd, log, rec),
   341	
   342		TP_STRUCT__entry(
   343			CXL_EVT_TP_entry
   344			/* General Media */
   345			__field(u64, dpa)
   346			__field(u8, descriptor)
   347			__field(u8, type)
   348			__field(u8, transaction_type)
   349			__field(u8, channel)
   350			__field(u32, device)
   351			__array(u8, comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
   352			/* Following are out of order to pack trace record */
   353			__field(u64, hpa)
   354			__field_struct(uuid_t, region_uuid)
   355			__field(u16, validity_flags)
   356			__field(u8, rank)
   357			__field(u8, dpa_flags)
   358			__string(region_name, to_region_name(cxlmd, to_gm_dpa(record)))
   359		),
   360	
   361		TP_fast_assign(
   362			CXL_EVT_TP_fast_assign(cxlmd, log, rec->hdr);
   363			__entry->hdr_uuid = CXL_EVENT_GEN_MEDIA_UUID;
   364	
   365			/* General Media */
   366			__entry->dpa = le64_to_cpu(rec->phys_addr);
   367			__entry->dpa_flags = __entry->dpa & CXL_DPA_FLAGS_MASK;
   368			/* Mask after flags have been parsed */
   369			__entry->dpa &= CXL_DPA_MASK;
   370			__entry->descriptor = rec->descriptor;
   371			__entry->type = rec->type;
   372			__entry->transaction_type = rec->transaction_type;
   373			__entry->channel = rec->channel;
   374			__entry->rank = rec->rank;
   375			__entry->device = get_unaligned_le24(rec->device);
   376			memcpy(__entry->comp_id, &rec->component_id,
   377				CXL_EVENT_GEN_MED_COMP_ID_SIZE);
   378			__entry->validity_flags = get_unaligned_le16(&rec->validity_flags);
   379			__assign_str(region_name, to_region_name(cxlmd, to_gm_dpa(record)));
 > 380			store_region_info(cxlmd, to_gm_dpa(record),
   381					  __entry->region_uuid, __entry->hpa);
   382		),
   383	
   384		CXL_EVT_TP_printk("dpa=%llx dpa_flags='%s' " \
   385			"descriptor='%s' type='%s' transaction_type='%s' channel=%u rank=%u " \
   386			"device=%x comp_id=%s validity_flags='%s' " \
   387			"hpa=%llx region=%s region_uuid=%pUb",
   388			__entry->dpa, show_dpa_flags(__entry->dpa_flags),
   389			show_event_desc_flags(__entry->descriptor),
   390			show_mem_event_type(__entry->type),
   391			show_trans_type(__entry->transaction_type),
   392			__entry->channel, __entry->rank, __entry->device,
   393			__print_hex(__entry->comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE),
   394			show_valid_flags(__entry->validity_flags),
   395			__entry->hpa, __get_str(region_name), &__entry->region_uuid
   396		)
   397	);
   398	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH 1/4] cxl/region: Move cxl_dpa_to_region() work to the region driver

[Jonathan Cameron]

On Wed, 27 Mar 2024 21:36:30 -0700
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> This helper belongs in the region driver as it is only useful
> with CONFIG_CXL_REGION. Add stubs in core.h for when the region
stub

> driver is not built.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Otherwise, does what is says on the tin.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Re: [PATCH 2/4] cxl/region: Move cxl_trace_hpa() work to the region driver

[Jonathan Cameron]

On Wed, 27 Mar 2024 21:36:31 -0700
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> This work belongs in the region driver as it is only useful with
> CONFIG_CXL_REGION. Add stubs in core.h for when the region driver

1 stub.  Is there a prize for trivial comment of the day? ;)

> is not built.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

Re: [PATCH 3/4] cxl/core: Add region info to cxl_general_media and cxl_dram events

[Jonathan Cameron]

On Wed, 27 Mar 2024 21:36:32 -0700
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> User space may need to know which region, if any, maps the DPAs
> (device physical addresses) reported in a cxl_general_media or
> cxl_dram event. Since the mapping can change, the kernel provides
> this information at the time the event occurs. This informs user
> space that at event <timestamp> this <region> mapped this <DPA>
> to this <HPA>.
> 
> Add the same region info that is included in the cxl_poison trace
> event: the DPA->HPA translation, region name, and region uuid.
> Introduce and use new helpers that lookup that region info using
> the struct cxl_memdev and a DPA.
> 
> The new fields are inserted in the trace event and no existing
> fields are modified. If the DPA is not mapped, user will see:
> hpa=ULLONG_MAX, region="", and uuid=0
> 
> This work must be protected by dpa_rwsem & region_rwsem since
> it is looking up region mappings.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
One query one the stub.. Otherwise all lgtm
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/core/core.h   |  6 +++++
>  drivers/cxl/core/mbox.c   | 17 ++++++++++----
>  drivers/cxl/core/region.c |  8 +++++++
>  drivers/cxl/core/trace.h  | 47 ++++++++++++++++++++++++++++++++++-----
>  4 files changed, 69 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 24454a1cb250..848ef6904beb 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -30,8 +30,14 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port);
>  struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
>  u64 cxl_trace_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  		  u64 dpa);
> +const char *cxl_trace_to_region_name(const struct cxl_memdev *cxlmd, u64 dpa);
>  
>  #else
> +static inline
> +const char *cxl_trace_to_region_name(const struct cxl_memdev *cxlmd, u64 dpa)
> +{
> +	return NULL;

Is this ever going to be a problem for the reason you have commented below?

Maybe 
return ""
?

...

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 45eb9c560fd6..a5b1eaee1e58 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2723,6 +2723,14 @@ struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
>  	return ctx.cxlr;
>  }
>  
> +const char *cxl_trace_to_region_name(const struct cxl_memdev *cxlmd, u64 dpa)
> +{
> +	struct cxl_region *cxlr = cxl_dpa_to_region(cxlmd, dpa);
> +
> +	/* trace __string() assignment requires "", not NULL */
> +	return cxlr ? dev_name(&cxlr->dev) : "";
> +}
> +

Re: [PATCH 4/4] cxl/core: Remove cxlr dependency from cxl_poison trace events

[Jonathan Cameron]

On Wed, 27 Mar 2024 21:36:33 -0700
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> cxl_poison trace events pass a pointer to a struct cxl_region
> when poison is discovered in a mapped endpoint. This made for
> easy look up of region name, uuid, and was useful in starting
> the dpa->hpa translation.
> 
> Another set of trace helpers is now available that eliminates
> the need to pass on that cxlr. (It was passed along a lot!)
> 
> In addition to tidying up the cxl_poison calling path, shifting
> to the new helpers also means all CXL trace events will share
> the same code in that regard.
> 
> Switch from a uuid array to the field_struct type uuid_t to
> align on one uuid format in all CXL trace events. That is useful
> when sharing region uuid lookup helpers.
> 
> No externally visible naming changes are made to cxl_poison trace
> events.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Trivial formatting comment inline.
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index 6ad4998aeb9a..2cd66c04602a 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -681,23 +681,23 @@ TRACE_EVENT(cxl_memory_module,
>  

> @@ -712,27 +712,20 @@ TRACE_EVENT(cxl_poison,
>  		__entry->source = cxl_poison_record_source(record);
>  		__entry->trace_type = trace_type;
>  		__entry->flags = flags;
> -		if (cxlr) {
> -			__assign_str(region, dev_name(&cxlr->dev));
> -			memcpy(__entry->uuid, &cxlr->params.uuid, 16);
> -			__entry->hpa = cxl_trace_hpa(cxlr, cxlmd,
> -						     __entry->dpa);
> -		} else {
> -			__assign_str(region, "");
> -			memset(__entry->uuid, 0, 16);
> -			__entry->hpa = ULLONG_MAX;
> -		}
> +		__assign_str(region, to_region_name(cxlmd, cxl_poison_record_dpa(record)));
> +		store_region_info(cxlmd, __entry->dpa, __entry->uuid,
> +				  __entry->hpa);
>  	    ),
>  
> -	TP_printk("memdev=%s host=%s serial=%lld trace_type=%s region=%s "  \
> -		"region_uuid=%pU hpa=0x%llx dpa=0x%llx dpa_length=0x%x "    \
> +	TP_printk("memdev=%s host=%s serial=%lld trace_type=%s region=%s"  \
> +		"region_uuid=%pUb hpa=0x%llx dpa=0x%llx dpa_length=0x%x "    \

the spaces before the \ could do with tidying up.  

>  		"source=%s flags=%s overflow_time=%llu",
>  		__get_str(memdev),
>  		__get_str(host),
>  		__entry->serial,
>  		show_poison_trace_type(__entry->trace_type),
>  		__get_str(region),
> -		__entry->uuid,
> +		&__entry->uuid,
>  		__entry->hpa,
>  		__entry->dpa,
>  		__entry->dpa_length,

Re: [PATCH 1/4] cxl/region: Move cxl_dpa_to_region() work to the region driver

[Alison Schofield]

On Wed, Apr 03, 2024 at 06:24:48PM +0100, Jonathan Cameron wrote:
> On Wed, 27 Mar 2024 21:36:30 -0700
> alison.schofield@intel.com wrote:
> 
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > This helper belongs in the region driver as it is only useful
> > with CONFIG_CXL_REGION. Add stubs in core.h for when the region
> stub
> 
> > driver is not built.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> Otherwise, does what is says on the tin.
s/is/it

And TIL:
"It does exactly what it says on the tin" was originally an advertising
slogan in the United Kingdom, which then became a common idiomatic phrase
in that country. It colloquially means that the name of something is an
accurate description of its qualities." Wikipedia

Watch for reuse in my reviews.

Thanks Jonathan!
> 
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 

Re: [PATCH 3/4] cxl/core: Add region info to cxl_general_media and cxl_dram events

[Alison Schofield]

On Wed, Apr 03, 2024 at 09:16:13PM +0100, Jonathan Cameron wrote:
> On Wed, 27 Mar 2024 21:36:32 -0700
> alison.schofield@intel.com wrote:
> 
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > User space may need to know which region, if any, maps the DPAs
> > (device physical addresses) reported in a cxl_general_media or
> > cxl_dram event. Since the mapping can change, the kernel provides
> > this information at the time the event occurs. This informs user
> > space that at event <timestamp> this <region> mapped this <DPA>
> > to this <HPA>.
> > 
> > Add the same region info that is included in the cxl_poison trace
> > event: the DPA->HPA translation, region name, and region uuid.
> > Introduce and use new helpers that lookup that region info using
> > the struct cxl_memdev and a DPA.
> > 
> > The new fields are inserted in the trace event and no existing
> > fields are modified. If the DPA is not mapped, user will see:
> > hpa=ULLONG_MAX, region="", and uuid=0
> > 
> > This work must be protected by dpa_rwsem & region_rwsem since
> > it is looking up region mappings.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> One query one the stub.. Otherwise all lgtm
> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
> 
> > ---
> >  drivers/cxl/core/core.h   |  6 +++++
> >  drivers/cxl/core/mbox.c   | 17 ++++++++++----
> >  drivers/cxl/core/region.c |  8 +++++++
> >  drivers/cxl/core/trace.h  | 47 ++++++++++++++++++++++++++++++++++-----
> >  4 files changed, 69 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> > index 24454a1cb250..848ef6904beb 100644
> > --- a/drivers/cxl/core/core.h
> > +++ b/drivers/cxl/core/core.h
> > @@ -30,8 +30,14 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port);
> >  struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa);
> >  u64 cxl_trace_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
> >  		  u64 dpa);
> > +const char *cxl_trace_to_region_name(const struct cxl_memdev *cxlmd, u64 dpa);
> >  
> >  #else
> > +static inline
> > +const char *cxl_trace_to_region_name(const struct cxl_memdev *cxlmd, u64 dpa)
> > +{
> > +	return NULL;
> 
> Is this ever going to be a problem for the reason you have commented below?
> 
> Maybe 
> return ""
> ?
> 
> ...

Yep. I'll make it "".
Also, I'll includ Steve Rostedt on v2. He's making changes around
assign_str() so maybe this patch can align w a simpler assign_str().

Thanks Jonathan!

> 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 45eb9c560fd6..a5b1eaee1e58 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -2723,6 +2723,14 @@ struct cxl_region *cxl_dpa_to_region(const struct cxl_memdev *cxlmd, u64 dpa)
> >  	return ctx.cxlr;
> >  }
> >  
> > +const char *cxl_trace_to_region_name(const struct cxl_memdev *cxlmd, u64 dpa)
> > +{
> > +	struct cxl_region *cxlr = cxl_dpa_to_region(cxlmd, dpa);
> > +
> > +	/* trace __string() assignment requires "", not NULL */
> > +	return cxlr ? dev_name(&cxlr->dev) : "";
> > +}
> > +
> 
> 
> 

Re: [PATCH 1/4] cxl/region: Move cxl_dpa_to_region() work to the region driver

[Ira Weiny]

alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> This helper belongs in the region driver as it is only useful
> with CONFIG_CXL_REGION. Add stubs in core.h for when the region
> driver is not built.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

Re: [PATCH 2/4] cxl/region: Move cxl_trace_hpa() work to the region driver

[Ira Weiny]

alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> This work belongs in the region driver as it is only useful with
> CONFIG_CXL_REGION. Add stubs in core.h for when the region driver
> is not built.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

Re: [PATCH 3/4] cxl/core: Add region info to cxl_general_media and cxl_dram events

[Ira Weiny]

alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 

[snip]

> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index 161bdb5734b0..6ad4998aeb9a 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -14,6 +14,22 @@
>  #include <cxlmem.h>
>  #include "core.h"
>  
> +#define to_region_name(cxlmd, dpa)					\
> +	(cxl_trace_to_region_name(cxlmd, dpa))

NIT: Is this macro really needed?

> +
> +#define store_region_info(cxlmd, dpa, entry_uuid, entry_hpa)		\
> +	do {								\
> +		struct cxl_region *cxlr;				\
> +									\
> +		cxlr = cxl_dpa_to_region(cxlmd, dpa);			\
> +		if (cxlr) {						\
> +			uuid_copy(&(entry_uuid), &cxlr->params.uuid);	\
> +			entry_hpa = cxl_trace_hpa(cxlr, cxlmd, dpa);	\
> +		} else {						\

Does the record get zeroed such that uuid is 0 here?

> +			entry_hpa = ULLONG_MAX;				\
> +		}							\
> +	} while (0)
> +
>  #define CXL_RAS_UC_CACHE_DATA_PARITY	BIT(0)
>  #define CXL_RAS_UC_CACHE_ADDR_PARITY	BIT(1)
>  #define CXL_RAS_UC_CACHE_BE_PARITY	BIT(2)
> @@ -313,6 +329,9 @@ TRACE_EVENT(cxl_generic_event,
>  	{ CXL_GMER_VALID_COMPONENT,			"COMPONENT"	}  \
>  )
>  
> +#define to_gm_dpa(record)						\

Curious: what does 'gm' stand for?

> +	(le64_to_cpu(rec->phys_addr) & CXL_DPA_MASK)
> +
>  TRACE_EVENT(cxl_general_media,
>  
>  	TP_PROTO(const struct cxl_memdev *cxlmd, enum cxl_event_log_type log,
> @@ -330,10 +349,13 @@ TRACE_EVENT(cxl_general_media,
>  		__field(u8, channel)
>  		__field(u32, device)
>  		__array(u8, comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
> -		__field(u16, validity_flags)
>  		/* Following are out of order to pack trace record */
> +		__field(u64, hpa)
> +		__field_struct(uuid_t, region_uuid)
> +		__field(u16, validity_flags)
>  		__field(u8, rank)
>  		__field(u8, dpa_flags)
> +		__string(region_name, to_region_name(cxlmd, to_gm_dpa(record)))
>  	),
>  
>  	TP_fast_assign(
> @@ -354,18 +376,23 @@ TRACE_EVENT(cxl_general_media,
>  		memcpy(__entry->comp_id, &rec->component_id,
>  			CXL_EVENT_GEN_MED_COMP_ID_SIZE);
>  		__entry->validity_flags = get_unaligned_le16(&rec->validity_flags);
> +		__assign_str(region_name, to_region_name(cxlmd, to_gm_dpa(record)));
> +		store_region_info(cxlmd, to_gm_dpa(record),
> +				  __entry->region_uuid, __entry->hpa);
>  	),
>  
>  	CXL_EVT_TP_printk("dpa=%llx dpa_flags='%s' " \
>  		"descriptor='%s' type='%s' transaction_type='%s' channel=%u rank=%u " \
> -		"device=%x comp_id=%s validity_flags='%s'",
> +		"device=%x comp_id=%s validity_flags='%s' " \
> +		"hpa=%llx region=%s region_uuid=%pUb",
>  		__entry->dpa, show_dpa_flags(__entry->dpa_flags),
>  		show_event_desc_flags(__entry->descriptor),
>  		show_mem_event_type(__entry->type),
>  		show_trans_type(__entry->transaction_type),
>  		__entry->channel, __entry->rank, __entry->device,
>  		__print_hex(__entry->comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE),
> -		show_valid_flags(__entry->validity_flags)
> +		show_valid_flags(__entry->validity_flags),
> +		__entry->hpa, __get_str(region_name), &__entry->region_uuid
>  	)
>  );
>  
> @@ -396,6 +423,8 @@ TRACE_EVENT(cxl_general_media,
>  	{ CXL_DER_VALID_COLUMN,				"COLUMN"		}, \
>  	{ CXL_DER_VALID_CORRECTION_MASK,		"CORRECTION MASK"	}  \
>  )
> +#define to_dram_dpa(record)						\
> +	(le64_to_cpu(rec->phys_addr) & CXL_DPA_MASK)

Oh.  'gm' was general media.  This should be a common macro...

Perhaps.

#define rec_pa_to_dpa(record)						\
	(le64_to_cpu(rec->phys_addr) & CXL_DPA_MASK)

?

Ira

>  
>  TRACE_EVENT(cxl_dram,
>  
> @@ -417,10 +446,13 @@ TRACE_EVENT(cxl_dram,
>  		__field(u32, nibble_mask)
>  		__field(u32, row)
>  		__array(u8, cor_mask, CXL_EVENT_DER_CORRECTION_MASK_SIZE)
> +		__field(u64, hpa)
> +		__field_struct(uuid_t, region_uuid)
>  		__field(u8, rank)	/* Out of order to pack trace record */
>  		__field(u8, bank_group)	/* Out of order to pack trace record */
>  		__field(u8, bank)	/* Out of order to pack trace record */
>  		__field(u8, dpa_flags)	/* Out of order to pack trace record */
> +		__string(region_name, to_region_name(cxlmd, to_dram_dpa(record)))
>  	),
>  
>  	TP_fast_assign(
> @@ -444,12 +476,16 @@ TRACE_EVENT(cxl_dram,
>  		__entry->column = get_unaligned_le16(rec->column);
>  		memcpy(__entry->cor_mask, &rec->correction_mask,
>  			CXL_EVENT_DER_CORRECTION_MASK_SIZE);
> +		__assign_str(region_name, to_region_name(cxlmd, to_dram_dpa(record)));
> +		store_region_info(cxlmd, __entry->dpa, __entry->region_uuid,
> +				  __entry->hpa);
>  	),
>  
>  	CXL_EVT_TP_printk("dpa=%llx dpa_flags='%s' descriptor='%s' type='%s' " \
>  		"transaction_type='%s' channel=%u rank=%u nibble_mask=%x " \
>  		"bank_group=%u bank=%u row=%u column=%u cor_mask=%s " \
> -		"validity_flags='%s'",
> +		"validity_flags='%s'" \
> +		"hpa=%llx region=%s region_uuid=%pUb",
>  		__entry->dpa, show_dpa_flags(__entry->dpa_flags),
>  		show_event_desc_flags(__entry->descriptor),
>  		show_mem_event_type(__entry->type),
> @@ -458,7 +494,8 @@ TRACE_EVENT(cxl_dram,
>  		__entry->bank_group, __entry->bank,
>  		__entry->row, __entry->column,
>  		__print_hex(__entry->cor_mask, CXL_EVENT_DER_CORRECTION_MASK_SIZE),
> -		show_dram_valid_flags(__entry->validity_flags)
> +		show_dram_valid_flags(__entry->validity_flags),
> +		__entry->hpa, __get_str(region_name), &__entry->region_uuid
>  	)
>  );
>  
> -- 
> 2.37.3
> 

Re: [PATCH 4/4] cxl/core: Remove cxlr dependency from cxl_poison trace events

[Ira Weiny]

alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> cxl_poison trace events pass a pointer to a struct cxl_region
> when poison is discovered in a mapped endpoint. This made for
> easy look up of region name, uuid, and was useful in starting
> the dpa->hpa translation.
> 
> Another set of trace helpers is now available that eliminates
> the need to pass on that cxlr. (It was passed along a lot!)
> 
> In addition to tidying up the cxl_poison calling path, shifting
> to the new helpers also means all CXL trace events will share
> the same code in that regard.
> 
> Switch from a uuid array to the field_struct type uuid_t to
> align on one uuid format in all CXL trace events. That is useful
> when sharing region uuid lookup helpers.
> 
> No externally visible naming changes are made to cxl_poison trace
> events.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

LGTM:
Reviewed-by: Ira Weiny <ira.weiny@intel.com>

Re: [PATCH 3/4] cxl/core: Add region info to cxl_general_media and cxl_dram events

[Alison Schofield]

+ Steve 

On Tue, Apr 16, 2024 at 10:01:21AM -0700, Ira Weiny wrote:
> alison.schofield@ wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> 
> [snip]
> 
> > diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> > index 161bdb5734b0..6ad4998aeb9a 100644
> > --- a/drivers/cxl/core/trace.h
> > +++ b/drivers/cxl/core/trace.h
> > @@ -14,6 +14,22 @@
> >  #include <cxlmem.h>
> >  #include "core.h"
> > 

snip

> > +#define store_region_info(cxlmd, dpa, entry_uuid, entry_hpa)		\
> > +	do {								\
> > +		struct cxl_region *cxlr;				\
> > +									\
> > +		cxlr = cxl_dpa_to_region(cxlmd, dpa);			\
> > +		if (cxlr) {						\
> > +			uuid_copy(&(entry_uuid), &cxlr->params.uuid);	\
> > +			entry_hpa = cxl_trace_hpa(cxlr, cxlmd, dpa);	\
> > +		} else {						\
> 
> Does the record get zeroed such that uuid is 0 here?

It appears in my usage, but I don't know for sure. 
CC'ing Steve to tell me if the TP_PROTO macro initializes the
__field_struct to zero.

Left in the snippets below that shows its definition...

> 
> > +			entry_hpa = ULLONG_MAX;				\
> > +		}							\
> > +	} while (0)

snip


> >  TRACE_EVENT(cxl_general_media,
> >  
> >  	TP_PROTO(const struct cxl_memdev *cxlmd, enum cxl_event_log_type log,
> > @@ -330,10 +349,13 @@ TRACE_EVENT(cxl_general_media,
> >  		__field(u8, channel)
> >  		__field(u32, device)
> >  		__array(u8, comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
> > -		__field(u16, validity_flags)
> >  		/* Following are out of order to pack trace record */
> > +		__field(u64, hpa)
> > +		__field_struct(uuid_t, region_uuid)
> > +		__field(u16, validity_flags)
> >  		__field(u8, rank)
> >  		__field(u8, dpa_flags)
> > +		__string(region_name, to_region_name(cxlmd, to_gm_dpa(record)))
> >  	),
> >  
> >  	TP_fast_assign(
> > @@ -354,18 +376,23 @@ TRACE_EVENT(cxl_general_media,
> >  		memcpy(__entry->comp_id, &rec->component_id,
> >  			CXL_EVENT_GEN_MED_COMP_ID_SIZE);
> >  		__entry->validity_flags = get_unaligned_le16(&rec->validity_flags);
> > +		__assign_str(region_name, to_region_name(cxlmd, to_gm_dpa(record)));
> > +		store_region_info(cxlmd, to_gm_dpa(record),
> > +				  __entry->region_uuid, __entry->hpa);
> >  	),
> >  
> >  	CXL_EVT_TP_printk("dpa=%llx dpa_flags='%s' " \
> >  		"descriptor='%s' type='%s' transaction_type='%s' channel=%u rank=%u " \
> > -		"device=%x comp_id=%s validity_flags='%s'",
> > +		"device=%x comp_id=%s validity_flags='%s' " \
> > +		"hpa=%llx region=%s region_uuid=%pUb",
> >  		__entry->dpa, show_dpa_flags(__entry->dpa_flags),
> >  		show_event_desc_flags(__entry->descriptor),
> >  		show_mem_event_type(__entry->type),
> >  		show_trans_type(__entry->transaction_type),
> >  		__entry->channel, __entry->rank, __entry->device,
> >  		__print_hex(__entry->comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE),
> > -		show_valid_flags(__entry->validity_flags)
> > +		show_valid_flags(__entry->validity_flags),
> > +		__entry->hpa, __get_str(region_name), &__entry->region_uuid
> >  	)
> >  );
> >