[RFC PATCH 0/4] Updates for CXL Event Records

[RFC PATCH 0/4] Updates for CXL Event Records

[shiju.jose]

From: Shiju Jose <shiju.jose@huawei.com>

CXL spec rev 3.1 CXL Event Records has updated w.r.t CXL spec rev 3.0.
Add updates for the above spec changes in the CXL events records and CXL
trace events implementation.

Note: Please apply following fix patch first if not present.
https://patchwork.kernel.org/project/cxl/patch/20241014143003.1170-1-shiju.jose@huawei.com/

Shiju Jose (4):
  cxl/events: Updates for CXL Common Event Record Format
  cxl/events: Updates for CXL General Media Event Record
  cxl/events: Updates for CXL DRAM Event Record
  cxl/events: Updates for CXL Memory Module Event Record

 drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
 include/cxl/event.h      |  20 +++-
 2 files changed, 190 insertions(+), 31 deletions(-)

-- 
2.34.1

[RFC PATCH 1/4] cxl/events: Updates for CXL Common Event Record Format

[shiju.jose]

From: Shiju Jose <shiju.jose@huawei.com>

CXL spec 3.1 section 8.2.9.2.1 Table 8-42, Common Event Record format has
updated with Maintenance Operation Subclass information.

Add updates for the above spec change in the CXL events record and CXL
common trace event implementations.

Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/cxl/core/trace.h | 13 +++++++++----
 include/cxl/event.h      |  3 ++-
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
index 8389a94adb1a..7305974e2301 100644
--- a/drivers/cxl/core/trace.h
+++ b/drivers/cxl/core/trace.h
@@ -166,11 +166,13 @@ TRACE_EVENT(cxl_overflow,
 #define CXL_EVENT_RECORD_FLAG_MAINT_NEEDED	BIT(3)
 #define CXL_EVENT_RECORD_FLAG_PERF_DEGRADED	BIT(4)
 #define CXL_EVENT_RECORD_FLAG_HW_REPLACE	BIT(5)
+#define CXL_EVENT_RECORD_FLAG_MAINT_OP_SUB_CLASS_VALID	BIT(6)
 #define show_hdr_flags(flags)	__print_flags(flags, " | ",			   \
 	{ CXL_EVENT_RECORD_FLAG_PERMANENT,	"PERMANENT_CONDITION"		}, \
 	{ CXL_EVENT_RECORD_FLAG_MAINT_NEEDED,	"MAINTENANCE_NEEDED"		}, \
 	{ CXL_EVENT_RECORD_FLAG_PERF_DEGRADED,	"PERFORMANCE_DEGRADED"		}, \
-	{ CXL_EVENT_RECORD_FLAG_HW_REPLACE,	"HARDWARE_REPLACEMENT_NEEDED"	}  \
+	{ CXL_EVENT_RECORD_FLAG_HW_REPLACE,	"HARDWARE_REPLACEMENT_NEEDED"	},  \
+	{ CXL_EVENT_RECORD_FLAG_MAINT_OP_SUB_CLASS_VALID,	"MAINT_OP_SUB_CLASS_VALID" }	\
 )
 
 /*
@@ -197,7 +199,8 @@ TRACE_EVENT(cxl_overflow,
 	__field(u16, hdr_related_handle)			\
 	__field(u64, hdr_timestamp)				\
 	__field(u8, hdr_length)					\
-	__field(u8, hdr_maint_op_class)
+	__field(u8, hdr_maint_op_class)				\
+	__field(u8, hdr_maint_op_sub_class)
 
 #define CXL_EVT_TP_fast_assign(cxlmd, l, hdr)					\
 	__assign_str(memdev);				\
@@ -209,17 +212,19 @@ TRACE_EVENT(cxl_overflow,
 	__entry->hdr_handle = le16_to_cpu((hdr).handle);			\
 	__entry->hdr_related_handle = le16_to_cpu((hdr).related_handle);	\
 	__entry->hdr_timestamp = le64_to_cpu((hdr).timestamp);			\
-	__entry->hdr_maint_op_class = (hdr).maint_op_class
+	__entry->hdr_maint_op_class = (hdr).maint_op_class;			\
+	__entry->hdr_maint_op_sub_class = (hdr).maint_op_sub_class
 
 #define CXL_EVT_TP_printk(fmt, ...) \
 	TP_printk("memdev=%s host=%s serial=%lld log=%s : time=%llu uuid=%pUb "	\
 		"len=%d flags='%s' handle=%x related_handle=%x "		\
-		"maint_op_class=%u : " fmt,					\
+		"maint_op_class=%u maint_op_sub_class=%u : " fmt,		\
 		__get_str(memdev), __get_str(host), __entry->serial,		\
 		cxl_event_log_type_str(__entry->log),				\
 		__entry->hdr_timestamp, &__entry->hdr_uuid, __entry->hdr_length,\
 		show_hdr_flags(__entry->hdr_flags), __entry->hdr_handle,	\
 		__entry->hdr_related_handle, __entry->hdr_maint_op_class,	\
+		__entry->hdr_maint_op_sub_class,	\
 		##__VA_ARGS__)
 
 TRACE_EVENT(cxl_generic_event,
diff --git a/include/cxl/event.h b/include/cxl/event.h
index 0bea1afbd747..e1d485ad376b 100644
--- a/include/cxl/event.h
+++ b/include/cxl/event.h
@@ -18,7 +18,8 @@ struct cxl_event_record_hdr {
 	__le16 related_handle;
 	__le64 timestamp;
 	u8 maint_op_class;
-	u8 reserved[15];
+	u8 maint_op_sub_class;
+	u8 reserved[14];
 } __packed;
 
 struct cxl_event_media_hdr {
-- 
2.34.1

Re: [RFC PATCH 1/4] cxl/events: Updates for CXL Common Event Record Format

[Jonathan Cameron]

On Wed, 16 Oct 2024 17:33:46 +0100
<shiju.jose@huawei.com> wrote:

> From: Shiju Jose <shiju.jose@huawei.com>
> 
> CXL spec 3.1 section 8.2.9.2.1 Table 8-42, Common Event Record format has
> updated with Maintenance Operation Subclass information.
> 
> Add updates for the above spec change in the CXL events record and CXL
> common trace event implementations.
> 
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>

LGTM

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

[RFC PATCH 2/4] cxl/events: Updates for CXL General Media Event Record

[shiju.jose]

From: Shiju Jose <shiju.jose@huawei.com>

CXL spec rev 3.1 section 8.2.9.2.1.1 Table 8-45, General Media Event
Record has updated with following new fields and new types for Memory
Event Type and Transaction Type fields.
1. Advanced Programmable Corrected Memory Error Threshold Event Flags
2. Corrected Memory Error Count at Event
3. Memory Event Sub-Type

The component identifier format has changed (CXL spec 3.1 section
8.2.9.2.1 Table 8-44).

Add updates for the above spec changes in the CXL events record and CXL
general media trace event implementations.

Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
Question:
Want more abbreviations for the long lines of code in
show_mem_event_sub_type() and for similar in other patches?
 drivers/cxl/core/trace.h | 96 ++++++++++++++++++++++++++++++++++++----
 include/cxl/event.h      |  5 ++-
 2 files changed, 91 insertions(+), 10 deletions(-)

diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
index 7305974e2301..e638e82429bc 100644
--- a/drivers/cxl/core/trace.h
+++ b/drivers/cxl/core/trace.h
@@ -268,9 +268,29 @@ TRACE_EVENT(cxl_generic_event,
 	{ CXL_DPA_NOT_REPAIRABLE,		"NOT_REPAIRABLE"	}  \
 )
 
+/*
+ * Component ID Format
+ * CXL 3.1 section 8.2.9.2.1; Table 8-44
+ */
+#define CXL_PLDM_COMPONENT_ID_ENTITY_VALID	BIT(0)
+#define CXL_PLDM_COMPONENT_ID_RES_VALID		BIT(1)
+#define cxl_print_component_id(flags, valid_comp_id, valid_id_format, comp_id) \
+	if (flags & valid_comp_id && flags & valid_id_format) { \
+		if (comp_id[0] & CXL_PLDM_COMPONENT_ID_ENTITY_VALID) { \
+			__print("PLDM Entity ID = "); \
+			__print_hex(&comp_id[1], 6); \
+		} \
+		if (comp_id[0] & CXL_PLDM_COMPONENT_ID_RES_VALID) { \
+			__print("Resource ID = "); \
+			__print_hex(&comp_id[7], 4); \
+		} \
+	} else { \
+		__print_hex(comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE); \
+	}
+
 /*
  * General Media Event Record - GMER
- * CXL rev 3.0 Section 8.2.9.2.1.1; Table 8-43
+ * CXL rev 3.1 Section 8.2.9.2.1.1; Table 8-45
  */
 #define CXL_GMER_EVT_DESC_UNCORECTABLE_EVENT		BIT(0)
 #define CXL_GMER_EVT_DESC_THRESHOLD_EVENT		BIT(1)
@@ -284,10 +304,18 @@ TRACE_EVENT(cxl_generic_event,
 #define CXL_GMER_MEM_EVT_TYPE_ECC_ERROR			0x00
 #define CXL_GMER_MEM_EVT_TYPE_INV_ADDR			0x01
 #define CXL_GMER_MEM_EVT_TYPE_DATA_PATH_ERROR		0x02
-#define show_gmer_mem_event_type(type)	__print_symbolic(type,			\
-	{ CXL_GMER_MEM_EVT_TYPE_ECC_ERROR,		"ECC Error" },		\
-	{ CXL_GMER_MEM_EVT_TYPE_INV_ADDR,		"Invalid Address" },	\
-	{ CXL_GMER_MEM_EVT_TYPE_DATA_PATH_ERROR,	"Data Path Error" }	\
+#define CXL_GMER_MEM_EVT_TYPE_TE_STATE_VIOLATION	0x03
+#define CXL_GMER_MEM_EVT_TYPE_SCRUB_MEDIA_ECC_ERROR	0x04
+#define CXL_GMER_MEM_EVT_TYPE_AP_CME_COUNTER_EXPIRE	0x05
+#define CXL_GMER_MEM_EVT_TYPE_CKID_VIOLATION		0x06
+#define show_gmer_mem_event_type(type)	__print_symbolic(type,				\
+	{ CXL_GMER_MEM_EVT_TYPE_ECC_ERROR,		"ECC Error" },			\
+	{ CXL_GMER_MEM_EVT_TYPE_INV_ADDR,		"Invalid Address" },		\
+	{ CXL_GMER_MEM_EVT_TYPE_DATA_PATH_ERROR,	"Data Path Error" },		\
+	{ CXL_GMER_MEM_EVT_TYPE_TE_STATE_VIOLATION,	"TE State Violation" },		\
+	{ CXL_GMER_MEM_EVT_TYPE_SCRUB_MEDIA_ECC_ERROR,	"Scrub Media ECC Error" },	\
+	{ CXL_GMER_MEM_EVT_TYPE_AP_CME_COUNTER_EXPIRE,	"Adv Prog CME Counter Expiration" },	\
+	{ CXL_GMER_MEM_EVT_TYPE_CKID_VIOLATION,		"CKID Violation" }		\
 )
 
 #define CXL_GMER_TRANS_UNKNOWN				0x00
@@ -297,6 +325,8 @@ TRACE_EVENT(cxl_generic_event,
 #define CXL_GMER_TRANS_HOST_INJECT_POISON		0x04
 #define CXL_GMER_TRANS_INTERNAL_MEDIA_SCRUB		0x05
 #define CXL_GMER_TRANS_INTERNAL_MEDIA_MANAGEMENT	0x06
+#define CXL_GMER_TRANS_INTERNAL_MEDIA_ECS		0x07
+#define CXL_GMER_TRANS_MEDIA_INITIALIZATION		0x08
 #define show_trans_type(type)	__print_symbolic(type,					\
 	{ CXL_GMER_TRANS_UNKNOWN,			"Unknown" },			\
 	{ CXL_GMER_TRANS_HOST_READ,			"Host Read" },			\
@@ -304,13 +334,16 @@ TRACE_EVENT(cxl_generic_event,
 	{ CXL_GMER_TRANS_HOST_SCAN_MEDIA,		"Host Scan Media" },		\
 	{ CXL_GMER_TRANS_HOST_INJECT_POISON,		"Host Inject Poison" },		\
 	{ CXL_GMER_TRANS_INTERNAL_MEDIA_SCRUB,		"Internal Media Scrub" },	\
-	{ CXL_GMER_TRANS_INTERNAL_MEDIA_MANAGEMENT,	"Internal Media Management" }	\
+	{ CXL_GMER_TRANS_INTERNAL_MEDIA_MANAGEMENT,	"Internal Media Management" },	\
+	{ CXL_GMER_TRANS_INTERNAL_MEDIA_ECS,		"Internal Media Error Check Scrub" },	\
+	{ CXL_GMER_TRANS_MEDIA_INITIALIZATION,		"Media Initialization" }	\
 )
 
 #define CXL_GMER_VALID_CHANNEL				BIT(0)
 #define CXL_GMER_VALID_RANK				BIT(1)
 #define CXL_GMER_VALID_DEVICE				BIT(2)
 #define CXL_GMER_VALID_COMPONENT			BIT(3)
+#define CXL_GMER_VALID_COMPONENT_ID_FORMAT		BIT(4)
 #define show_valid_flags(flags)	__print_flags(flags, "|",		   \
 	{ CXL_GMER_VALID_CHANNEL,			"CHANNEL"	}, \
 	{ CXL_GMER_VALID_RANK,				"RANK"		}, \
@@ -318,6 +351,41 @@ TRACE_EVENT(cxl_generic_event,
 	{ CXL_GMER_VALID_COMPONENT,			"COMPONENT"	}  \
 )
 
+#define CXL_GMER_CME_EV_FLAG_CME_MULTIPLE_MEDIA		BIT(0)
+#define CXL_GMER_CME_EV_FLAG_THRESHOLD_EXCEEDED		BIT(1)
+#define show_cme_threshold_ev_flags(flags)	__print_symbolic(flags,	\
+	{								\
+		CXL_GMER_CME_EV_FLAG_CME_MULTIPLE_MEDIA,		\
+		"Corrected Memory Errors in Multiple Media Components"	\
+	}, {								\
+		CXL_GMER_CME_EV_FLAG_THRESHOLD_EXCEEDED,		\
+		"Exceeded Programmable Threshold"			\
+	}								\
+)
+
+#define CXL_GMER_MEM_EVT_SUB_TYPE_NOT_REPORTED				0x00
+#define CXL_GMER_MEM_EVT_SUB_TYPE_INTERNAL_DATAPATH_ERROR		0x01
+#define CXL_GMER_MEM_EVT_SUB_TYPE_MEDIA_LINK_COMMAND_TRAINING_ERROR	0x02
+#define CXL_GMER_MEM_EVT_SUB_TYPE_MEDIA_LINK_CONTROL_TRAINING_ERROR	0x03
+#define CXL_GMER_MEM_EVT_SUB_TYPE_MEDIA_LINK_DATA_TRAINING_ERROR	0x04
+#define CXL_GMER_MEM_EVT_SUB_TYPE_MEDIA_LINK_CRC_ERROR			0x05
+#define show_mem_event_sub_type(sub_type)	__print_symbolic(sub_type, \
+	{ CXL_GMER_MEM_EVT_SUB_TYPE_NOT_REPORTED, "Not Reported" }, \
+	{ CXL_GMER_MEM_EVT_SUB_TYPE_INTERNAL_DATAPATH_ERROR, "Internal Datapath Error" }, \
+	{ \
+		CXL_GMER_MEM_EVT_SUB_TYPE_MEDIA_LINK_COMMAND_TRAINING_ERROR, \
+		"Media Link Command Training Error" \
+	}, { \
+		CXL_GMER_MEM_EVT_SUB_TYPE_MEDIA_LINK_CONTROL_TRAINING_ERROR, \
+		"Media Link Control Training Error" \
+	}, { \
+		CXL_GMER_MEM_EVT_SUB_TYPE_MEDIA_LINK_DATA_TRAINING_ERROR, \
+		"Media Link Data Training Error" \
+	}, { \
+		CXL_GMER_MEM_EVT_SUB_TYPE_MEDIA_LINK_CRC_ERROR, "Media Link CRC Error" \
+	} \
+)
+
 TRACE_EVENT(cxl_general_media,
 
 	TP_PROTO(const struct cxl_memdev *cxlmd, enum cxl_event_log_type log,
@@ -341,6 +409,9 @@ TRACE_EVENT(cxl_general_media,
 		__field(u16, validity_flags)
 		__field(u8, rank)
 		__field(u8, dpa_flags)
+		__field(u8, cme_threshold_ev_flags)
+		__field(u32, cme_count)
+		__field(u8, sub_type)
 		__string(region_name, cxlr ? dev_name(&cxlr->dev) : "")
 	),
 
@@ -363,6 +434,9 @@ TRACE_EVENT(cxl_general_media,
 			CXL_EVENT_GEN_MED_COMP_ID_SIZE);
 		__entry->validity_flags = get_unaligned_le16(&rec->media_hdr.validity_flags);
 		__entry->hpa = hpa;
+		__entry->cme_threshold_ev_flags = rec->cme_threshold_ev_flags;
+		__entry->cme_count = get_unaligned_le24(rec->cme_count);
+		__entry->sub_type = rec->sub_type;
 		if (cxlr) {
 			__assign_str(region_name);
 			uuid_copy(&__entry->region_uuid, &cxlr->params.uuid);
@@ -375,15 +449,19 @@ TRACE_EVENT(cxl_general_media,
 	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' " \
-		"hpa=%llx region=%s region_uuid=%pUb",
+		"hpa=%llx cme_threshold_ev_flags='%s' cme_count=%x " \
+		"sub_type='%s' region=%s region_uuid=%pUb",
 		__entry->dpa, show_dpa_flags(__entry->dpa_flags),
 		show_event_desc_flags(__entry->descriptor),
 		show_gmer_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),
+		cxl_print_component_id(__entry->validity_flags, CXL_GMER_VALID_COMPONENT,
+				       CXL_GMER_VALID_COMPONENT_ID_FORMAT, __entry->comp_id),
 		show_valid_flags(__entry->validity_flags),
-		__entry->hpa, __get_str(region_name), &__entry->region_uuid
+		__entry->hpa, show_cme_threshold_ev_flags(__entry->cme_threshold_ev_flags),
+		__entry->cme_count, show_mem_event_sub_type(__entry->sub_type),
+		__get_str(region_name), &__entry->region_uuid
 	)
 );
 
diff --git a/include/cxl/event.h b/include/cxl/event.h
index e1d485ad376b..ea8cd44a52e9 100644
--- a/include/cxl/event.h
+++ b/include/cxl/event.h
@@ -52,7 +52,10 @@ struct cxl_event_gen_media {
 	struct cxl_event_media_hdr media_hdr;
 	u8 device[3];
 	u8 component_id[CXL_EVENT_GEN_MED_COMP_ID_SIZE];
-	u8 reserved[46];
+	u8 cme_threshold_ev_flags;
+	u8 cme_count[3];
+	u8 sub_type;
+	u8 reserved[41];
 } __packed;
 
 /*
-- 
2.34.1

Re: [RFC PATCH 2/4] cxl/events: Updates for CXL General Media Event Record

[Jonathan Cameron]

On Wed, 16 Oct 2024 17:33:47 +0100
<shiju.jose@huawei.com> wrote:

> From: Shiju Jose <shiju.jose@huawei.com>
> 
> CXL spec rev 3.1 section 8.2.9.2.1.1 Table 8-45, General Media Event
> Record has updated with following new fields and new types for Memory
> Event Type and Transaction Type fields.
> 1. Advanced Programmable Corrected Memory Error Threshold Event Flags
> 2. Corrected Memory Error Count at Event
> 3. Memory Event Sub-Type
> 
> The component identifier format has changed (CXL spec 3.1 section
> 8.2.9.2.1 Table 8-44).
> 
> Add updates for the above spec changes in the CXL events record and CXL
> general media trace event implementations.
> 
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>

It might be worth breaking out the component ID formatting as
a separate patch.  That comes in 3.1 along with the other fields
but is perhaps more controversial?

It's a good change, but will change what is printed. I 'think'
tracepoint printing is typically not considered ABI though (unlike
the tracepoint which is!) so should be fine.

Split or not I like the component ID formatting and the reset LGTM
I also slight prefer the fact you inserted new fields in logical
places (so disagree with Alison if that's what she meant).
Good to call that out in the patch description though to highlight
it as something people might want to consider.

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


> ---
> Question:
> Want more abbreviations for the long lines of code in
> show_mem_event_sub_type() and for similar in other patches?
I raised this in internal review, but don't think it matters
that much either way :)

RE: [RFC PATCH 2/4] cxl/events: Updates for CXL General Media Event Record

[Shiju Jose]

>-----Original Message-----
>From: Jonathan Cameron <jonathan.cameron@huawei.com>
>Sent: 17 October 2024 13:25
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: dave.jiang@intel.com; dan.j.williams@intel.com; alison.schofield@intel.com;
>vishal.l.verma@intel.com; ira.weiny@intel.com; dave@stgolabs.net; linux-
>cxl@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
><linuxarm@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>; Zengtao (B)
><prime.zeng@hisilicon.com>
>Subject: Re: [RFC PATCH 2/4] cxl/events: Updates for CXL General Media Event
>Record
>
>On Wed, 16 Oct 2024 17:33:47 +0100
><shiju.jose@huawei.com> wrote:
>
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> CXL spec rev 3.1 section 8.2.9.2.1.1 Table 8-45, General Media Event
>> Record has updated with following new fields and new types for Memory
>> Event Type and Transaction Type fields.
>> 1. Advanced Programmable Corrected Memory Error Threshold Event Flags
>> 2. Corrected Memory Error Count at Event 3. Memory Event Sub-Type
>>
>> The component identifier format has changed (CXL spec 3.1 section
>> 8.2.9.2.1 Table 8-44).
>>
>> Add updates for the above spec changes in the CXL events record and
>> CXL general media trace event implementations.
>>
>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>
>It might be worth breaking out the component ID formatting as a separate
>patch.  That comes in 3.1 along with the other fields but is perhaps more
>controversial?
I will add separate patch for component ID formatting.

>
>It's a good change, but will change what is printed. I 'think'
>tracepoint printing is typically not considered ABI though (unlike the tracepoint
>which is!) so should be fine.
>
>Split or not I like the component ID formatting and the reset LGTM I also slight
>prefer the fact you inserted new fields in logical places (so disagree with Alison
>if that's what she meant).
>Good to call that out in the patch description though to highlight it as something
>people might want to consider.
Sure.
>
>Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
>
>> ---
>> Question:
>> Want more abbreviations for the long lines of code in
>> show_mem_event_sub_type() and for similar in other patches?
>I raised this in internal review, but don't think it matters that much either way :)
Ok.

Thanks,
Shiju

[RFC PATCH 3/4] cxl/events: Updates for CXL DRAM Event Record

[shiju.jose]

From: Shiju Jose <shiju.jose@huawei.com>

CXL spec 3.1 section 8.2.9.2.1.2 Table 8-46, DRAM Event Record has updated
with following new fields and new types for Memory Event Type, Transaction
Type and Validity Flags fields.
1. Component Identifier
2. Sub-channel
3. Advanced Programmable Corrected Memory Error Threshold Event Flags
4. Corrected Memory Error Count at Event
5. Memory Event Sub-Type

Add updates for the above spec changes in the CXL events record and CXL
DRAM trace event implementations.

Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/cxl/core/trace.h | 44 ++++++++++++++++++++++++++++++++--------
 include/cxl/event.h      |  7 ++++++-
 2 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
index e638e82429bc..20790dffa2b4 100644
--- a/drivers/cxl/core/trace.h
+++ b/drivers/cxl/core/trace.h
@@ -468,7 +468,7 @@ TRACE_EVENT(cxl_general_media,
 /*
  * DRAM Event Record - DER
  *
- * CXL rev 3.0 section 8.2.9.2.1.2; Table 8-44
+ * CXL rev 3.1 section 8.2.9.2.1.2; Table 8-46
  */
 /*
  * DRAM Event Record defines many fields the same as the General Media Event
@@ -478,11 +478,17 @@ TRACE_EVENT(cxl_general_media,
 #define CXL_DER_MEM_EVT_TYPE_SCRUB_MEDIA_ECC_ERROR	0x01
 #define CXL_DER_MEM_EVT_TYPE_INV_ADDR			0x02
 #define CXL_DER_MEM_EVT_TYPE_DATA_PATH_ERROR		0x03
-#define show_dram_mem_event_type(type)  __print_symbolic(type,				\
+#define CXL_DER_MEM_EVT_TYPE_TE_STATE_VIOLATION	0x04
+#define CXL_DER_MEM_EVT_TYPE_AP_CME_COUNTER_EXPIRE	0x05
+#define CXL_DER_MEM_EVT_TYPE_CKID_VIOLATION		0x06
+#define show_dram_mem_event_type(type)	__print_symbolic(type,				\
 	{ CXL_DER_MEM_EVT_TYPE_ECC_ERROR,		"ECC Error" },			\
 	{ CXL_DER_MEM_EVT_TYPE_SCRUB_MEDIA_ECC_ERROR,	"Scrub Media ECC Error" },	\
 	{ CXL_DER_MEM_EVT_TYPE_INV_ADDR,		"Invalid Address" },		\
-	{ CXL_DER_MEM_EVT_TYPE_DATA_PATH_ERROR,		"Data Path Error" }		\
+	{ CXL_DER_MEM_EVT_TYPE_DATA_PATH_ERROR,		"Data Path Error" },		\
+	{ CXL_DER_MEM_EVT_TYPE_TE_STATE_VIOLATION,	"TE State Violation" },		\
+	{ CXL_DER_MEM_EVT_TYPE_AP_CME_COUNTER_EXPIRE,	"Adv Prog CME Counter Expiration" },	\
+	{ CXL_DER_MEM_EVT_TYPE_CKID_VIOLATION,		"CKID Violation" }		\
 )
 
 #define CXL_DER_VALID_CHANNEL				BIT(0)
@@ -493,7 +499,10 @@ TRACE_EVENT(cxl_general_media,
 #define CXL_DER_VALID_ROW				BIT(5)
 #define CXL_DER_VALID_COLUMN				BIT(6)
 #define CXL_DER_VALID_CORRECTION_MASK			BIT(7)
-#define show_dram_valid_flags(flags)	__print_flags(flags, "|",			   \
+#define CXL_DER_VALID_COMPONENT				BIT(8)
+#define CXL_DER_VALID_COMPONENT_ID_FORMAT		BIT(9)
+#define CXL_DER_VALID_SUB_CHANNEL			BIT(10)
+#define show_dram_valid_flags(flags)	__print_flags(flags, "|",		   \
 	{ CXL_DER_VALID_CHANNEL,			"CHANNEL"		}, \
 	{ CXL_DER_VALID_RANK,				"RANK"			}, \
 	{ CXL_DER_VALID_NIBBLE,				"NIBBLE"		}, \
@@ -501,7 +510,9 @@ TRACE_EVENT(cxl_general_media,
 	{ CXL_DER_VALID_BANK,				"BANK"			}, \
 	{ CXL_DER_VALID_ROW,				"ROW"			}, \
 	{ CXL_DER_VALID_COLUMN,				"COLUMN"		}, \
-	{ CXL_DER_VALID_CORRECTION_MASK,		"CORRECTION MASK"	}  \
+	{ CXL_DER_VALID_CORRECTION_MASK,		"CORRECTION MASK"	}, \
+	{ CXL_DER_VALID_COMPONENT,			"COMPONENT"		}, \
+	{ CXL_DER_VALID_SUB_CHANNEL,			"SUB CHANNEL"		}  \
 )
 
 TRACE_EVENT(cxl_dram,
@@ -530,6 +541,11 @@ TRACE_EVENT(cxl_dram,
 		__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 */
+		__array(u8, comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
+		__field(u32, cvme_count)
+		__field(u8, sub_channel)
+		__field(u8, cme_threshold_ev_flags)
+		__field(u8, sub_type)
 		__string(region_name, cxlr ? dev_name(&cxlr->dev) : "")
 	),
 
@@ -554,7 +570,13 @@ TRACE_EVENT(cxl_dram,
 		__entry->column = get_unaligned_le16(rec->column);
 		memcpy(__entry->cor_mask, &rec->correction_mask,
 			CXL_EVENT_DER_CORRECTION_MASK_SIZE);
+		memcpy(__entry->comp_id, &rec->component_id,
+		       CXL_EVENT_GEN_MED_COMP_ID_SIZE);
 		__entry->hpa = hpa;
+		__entry->sub_channel = rec->sub_channel;
+		__entry->cme_threshold_ev_flags = rec->cme_threshold_ev_flags;
+		__entry->cvme_count = get_unaligned_le24(rec->cvme_count);
+		__entry->sub_type = rec->sub_type;
 		if (cxlr) {
 			__assign_str(region_name);
 			uuid_copy(&__entry->region_uuid, &cxlr->params.uuid);
@@ -567,8 +589,9 @@ TRACE_EVENT(cxl_dram,
 	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' " \
-		"hpa=%llx region=%s region_uuid=%pUb",
+		"comp_id=%s validity_flags='%s' " \
+		"hpa=%llx sub_channel=%u cme_threshold_ev_flags='%s' " \
+		"cvme_count=%x sub_type='%s' region=%s region_uuid=%pUb",
 		__entry->dpa, show_dpa_flags(__entry->dpa_flags),
 		show_event_desc_flags(__entry->descriptor),
 		show_dram_mem_event_type(__entry->type),
@@ -577,8 +600,13 @@ TRACE_EVENT(cxl_dram,
 		__entry->bank_group, __entry->bank,
 		__entry->row, __entry->column,
 		__print_hex(__entry->cor_mask, CXL_EVENT_DER_CORRECTION_MASK_SIZE),
+		cxl_print_component_id(__entry->validity_flags, CXL_DER_VALID_COMPONENT,
+				       CXL_DER_VALID_COMPONENT_ID_FORMAT, __entry->comp_id),
 		show_dram_valid_flags(__entry->validity_flags),
-		__entry->hpa, __get_str(region_name), &__entry->region_uuid
+		__entry->hpa, __entry->sub_channel,
+		show_cme_threshold_ev_flags(__entry->cme_threshold_ev_flags),
+		__entry->cvme_count, show_mem_event_sub_type(__entry->sub_type),
+		__get_str(region_name), &__entry->region_uuid
 	)
 );
 
diff --git a/include/cxl/event.h b/include/cxl/event.h
index ea8cd44a52e9..7e98492c85df 100644
--- a/include/cxl/event.h
+++ b/include/cxl/event.h
@@ -71,7 +71,12 @@ struct cxl_event_dram {
 	u8 row[3];
 	u8 column[2];
 	u8 correction_mask[CXL_EVENT_DER_CORRECTION_MASK_SIZE];
-	u8 reserved[0x17];
+	u8 component_id[CXL_EVENT_GEN_MED_COMP_ID_SIZE];
+	u8 sub_channel;
+	u8 cme_threshold_ev_flags;
+	u8 cvme_count[3];
+	u8 sub_type;
+	u8 reserved;
 } __packed;
 
 /*
-- 
2.34.1

Re: [RFC PATCH 3/4] cxl/events: Updates for CXL DRAM Event Record

[Jonathan Cameron]

On Wed, 16 Oct 2024 17:33:48 +0100
<shiju.jose@huawei.com> wrote:

> From: Shiju Jose <shiju.jose@huawei.com>
> 
> CXL spec 3.1 section 8.2.9.2.1.2 Table 8-46, DRAM Event Record has updated
> with following new fields and new types for Memory Event Type, Transaction
> Type and Validity Flags fields.
> 1. Component Identifier
> 2. Sub-channel
> 3. Advanced Programmable Corrected Memory Error Threshold Event Flags
> 4. Corrected Memory Error Count at Event
> 5. Memory Event Sub-Type
> 
> Add updates for the above spec changes in the CXL events record and CXL
> DRAM trace event implementations.
> 
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
Passing comments on two things inline.
1) There are a couple of whitespace consistency changes in here.
   Spaces to tabs for alignment.  That's fine but maybe needs a brief
   mention in the patch description.
2) Really odd that the spec didn't have a component ID field for DRAM
   errors.  They weren't all that useful before the PLDM format was added
   but still a curiosity that made me open up the 3.0 spec.  Indeed, no
   such field.

With that one line added to the patch description this looks good to me.

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

> ---
>  drivers/cxl/core/trace.h | 44 ++++++++++++++++++++++++++++++++--------
>  include/cxl/event.h      |  7 ++++++-
>  2 files changed, 42 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index e638e82429bc..20790dffa2b4 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -468,7 +468,7 @@ TRACE_EVENT(cxl_general_media,
>  /*
>   * DRAM Event Record - DER
>   *
> - * CXL rev 3.0 section 8.2.9.2.1.2; Table 8-44
> + * CXL rev 3.1 section 8.2.9.2.1.2; Table 8-46
>   */
>  /*
>   * DRAM Event Record defines many fields the same as the General Media Event
> @@ -478,11 +478,17 @@ TRACE_EVENT(cxl_general_media,
>  #define CXL_DER_MEM_EVT_TYPE_SCRUB_MEDIA_ECC_ERROR	0x01
>  #define CXL_DER_MEM_EVT_TYPE_INV_ADDR			0x02
>  #define CXL_DER_MEM_EVT_TYPE_DATA_PATH_ERROR		0x03
> -#define show_dram_mem_event_type(type)  __print_symbolic(type,				\
> +#define CXL_DER_MEM_EVT_TYPE_TE_STATE_VIOLATION	0x04
> +#define CXL_DER_MEM_EVT_TYPE_AP_CME_COUNTER_EXPIRE	0x05
> +#define CXL_DER_MEM_EVT_TYPE_CKID_VIOLATION		0x06
> +#define show_dram_mem_event_type(type)	__print_symbolic(type,				\

This change looks odd here but does print the line above into the
same formatting style as the other similar cases in the file.
Maybe worth a line in the patch description to say "Includes trivial consistency of white
space improvements" just to flag up that it was intentional.

>  	{ CXL_DER_MEM_EVT_TYPE_ECC_ERROR,		"ECC Error" },			\
>  	{ CXL_DER_MEM_EVT_TYPE_SCRUB_MEDIA_ECC_ERROR,	"Scrub Media ECC Error" },	\
>  	{ CXL_DER_MEM_EVT_TYPE_INV_ADDR,		"Invalid Address" },		\
> -	{ CXL_DER_MEM_EVT_TYPE_DATA_PATH_ERROR,		"Data Path Error" }		\
> +	{ CXL_DER_MEM_EVT_TYPE_DATA_PATH_ERROR,		"Data Path Error" },		\
> +	{ CXL_DER_MEM_EVT_TYPE_TE_STATE_VIOLATION,	"TE State Violation" },		\
> +	{ CXL_DER_MEM_EVT_TYPE_AP_CME_COUNTER_EXPIRE,	"Adv Prog CME Counter Expiration" },	\
> +	{ CXL_DER_MEM_EVT_TYPE_CKID_VIOLATION,		"CKID Violation" }		\
>  )
>  
>  #define CXL_DER_VALID_CHANNEL				BIT(0)
> @@ -493,7 +499,10 @@ TRACE_EVENT(cxl_general_media,
>  #define CXL_DER_VALID_ROW				BIT(5)
>  #define CXL_DER_VALID_COLUMN				BIT(6)
>  #define CXL_DER_VALID_CORRECTION_MASK			BIT(7)
> -#define show_dram_valid_flags(flags)	__print_flags(flags, "|",			   \
> +#define CXL_DER_VALID_COMPONENT				BIT(8)
> +#define CXL_DER_VALID_COMPONENT_ID_FORMAT		BIT(9)
> +#define CXL_DER_VALID_SUB_CHANNEL			BIT(10)
> +#define show_dram_valid_flags(flags)	__print_flags(flags, "|",		   \

As above this is a minor white space consistency change.

>  	{ CXL_DER_VALID_CHANNEL,			"CHANNEL"		}, \
>  	{ CXL_DER_VALID_RANK,				"RANK"			}, \
>  	{ CXL_DER_VALID_NIBBLE,				"NIBBLE"		}, \
> @@ -501,7 +510,9 @@ TRACE_EVENT(cxl_general_media,
>  	{ CXL_DER_VALID_BANK,				"BANK"			}, \
>  	{ CXL_DER_VALID_ROW,				"ROW"			}, \
>  	{ CXL_DER_VALID_COLUMN,				"COLUMN"		}, \
> -	{ CXL_DER_VALID_CORRECTION_MASK,		"CORRECTION MASK"	}  \
> +	{ CXL_DER_VALID_CORRECTION_MASK,		"CORRECTION MASK"	}, \
> +	{ CXL_DER_VALID_COMPONENT,			"COMPONENT"		}, \
> +	{ CXL_DER_VALID_SUB_CHANNEL,			"SUB CHANNEL"		}  \
>  )

> diff --git a/include/cxl/event.h b/include/cxl/event.h
> index ea8cd44a52e9..7e98492c85df 100644
> --- a/include/cxl/event.h
> +++ b/include/cxl/event.h
> @@ -71,7 +71,12 @@ struct cxl_event_dram {
>  	u8 row[3];
>  	u8 column[2];
>  	u8 correction_mask[CXL_EVENT_DER_CORRECTION_MASK_SIZE];
> -	u8 reserved[0x17];
> +	u8 component_id[CXL_EVENT_GEN_MED_COMP_ID_SIZE];
Odd that the general media had this field in 3.0 but DRAM didn't.
I checked though and indeed the case!

> +	u8 sub_channel;
> +	u8 cme_threshold_ev_flags;
> +	u8 cvme_count[3];
> +	u8 sub_type;
> +	u8 reserved;
>  } __packed;
>  
>  /*

[RFC PATCH 4/4] cxl/events: Updates for CXL Memory Module Event Record

[shiju.jose]

From: Shiju Jose <shiju.jose@huawei.com>

CXL spec 3.1 section 8.2.9.2.1.3 Table 8-47, Memory Module Event Record
has updated with following new fields and new info for Device Event Type
and Device Health Information fields.
1. Validity Flags
2. Component Identifier
3. Device Event Sub-Type

Add updates for the above spec changes in the CXL events record and CXL
Memory Module trace event implementations.

Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/cxl/core/trace.h | 48 +++++++++++++++++++++++++++++++++++-----
 include/cxl/event.h      |  5 ++++-
 2 files changed, 46 insertions(+), 7 deletions(-)

diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
index 20790dffa2b4..1ce43bff49c7 100644
--- a/drivers/cxl/core/trace.h
+++ b/drivers/cxl/core/trace.h
@@ -613,7 +613,7 @@ TRACE_EVENT(cxl_dram,
 /*
  * Memory Module Event Record - MMER
  *
- * CXL res 3.0 section 8.2.9.2.1.3; Table 8-45
+ * CXL res 3.1 section 8.2.9.2.1.3; Table 8-47
  */
 #define CXL_MMER_HEALTH_STATUS_CHANGE		0x00
 #define CXL_MMER_MEDIA_STATUS_CHANGE		0x01
@@ -621,27 +621,35 @@ TRACE_EVENT(cxl_dram,
 #define CXL_MMER_TEMP_CHANGE			0x03
 #define CXL_MMER_DATA_PATH_ERROR		0x04
 #define CXL_MMER_LSA_ERROR			0x05
+#define CXL_MMER_UNRECOV_SIDEBAND_BUS_ERROR	0x06
+#define CXL_MMER_MEMORY_MEDIA_FRU_ERROR		0x07
+#define CXL_MMER_POWER_MANAGEMENT_FAULT		0x08
 #define show_dev_evt_type(type)	__print_symbolic(type,			   \
 	{ CXL_MMER_HEALTH_STATUS_CHANGE,	"Health Status Change"	}, \
 	{ CXL_MMER_MEDIA_STATUS_CHANGE,		"Media Status Change"	}, \
 	{ CXL_MMER_LIFE_USED_CHANGE,		"Life Used Change"	}, \
 	{ CXL_MMER_TEMP_CHANGE,			"Temperature Change"	}, \
 	{ CXL_MMER_DATA_PATH_ERROR,		"Data Path Error"	}, \
-	{ CXL_MMER_LSA_ERROR,			"LSA Error"		}  \
+	{ CXL_MMER_LSA_ERROR,			"LSA Error"		}, \
+	{ CXL_MMER_UNRECOV_SIDEBAND_BUS_ERROR,	"Unrecoverable Internal Sideband Bus Error"	}, \
+	{ CXL_MMER_MEMORY_MEDIA_FRU_ERROR,	"Memory Media FRU Error"	}, \
+	{ CXL_MMER_POWER_MANAGEMENT_FAULT,	"Power Management Fault"	}  \
 )
 
 /*
  * Device Health Information - DHI
  *
- * CXL res 3.0 section 8.2.9.8.3.1; Table 8-100
+ * CXL res 3.1 section 8.2.9.9.3.1; Table 8-133
  */
 #define CXL_DHI_HS_MAINTENANCE_NEEDED				BIT(0)
 #define CXL_DHI_HS_PERFORMANCE_DEGRADED				BIT(1)
 #define CXL_DHI_HS_HW_REPLACEMENT_NEEDED			BIT(2)
+#define CXL_DHI_HS_MEM_CAPACITY_DEGRADED			BIT(3)
 #define show_health_status_flags(flags)	__print_flags(flags, "|",	   \
 	{ CXL_DHI_HS_MAINTENANCE_NEEDED,	"MAINTENANCE_NEEDED"	}, \
 	{ CXL_DHI_HS_PERFORMANCE_DEGRADED,	"PERFORMANCE_DEGRADED"	}, \
-	{ CXL_DHI_HS_HW_REPLACEMENT_NEEDED,	"REPLACEMENT_NEEDED"	}  \
+	{ CXL_DHI_HS_HW_REPLACEMENT_NEEDED,	"REPLACEMENT_NEEDED"	}, \
+	{ CXL_DHI_HS_MEM_CAPACITY_DEGRADED,	"MEM_CAPACITY_DEGRADED"	}  \
 )
 
 #define CXL_DHI_MS_NORMAL							0x00
@@ -695,6 +703,22 @@ TRACE_EVENT(cxl_dram,
 #define CXL_DHI_AS_COR_VOL_ERR_CNT(as)			((as & 0x10) >> 4)
 #define CXL_DHI_AS_COR_PER_ERR_CNT(as)			((as & 0x20) >> 5)
 
+#define CXL_MMER_VALID_COMPONENT			BIT(0)
+#define CXL_MMER_VALID_COMPONENT_ID_FORMAT		BIT(1)
+#define show_mem_module_valid_flags(flags)	__print_flags(flags, "|",	\
+	{ CXL_MMER_VALID_COMPONENT,	"COMPONENT"	}			\
+)
+#define CXL_MMER_DEV_EVT_SUB_TYPE_NOT_REPORTED			0x00
+#define CXL_MMER_DEV_EVT_SUB_TYPE_INVALID_CONFIG_DATA		0x01
+#define CXL_MMER_DEV_EVT_SUB_TYPE_UNSUPP_CONFIG_DATA		0x02
+#define CXL_MMER_DEV_EVT_SUB_TYPE_UNSUPP_MEM_MEDIA_FRU		0x03
+#define show_dev_event_sub_type(sub_type)	__print_symbolic(sub_type,	  \
+	{ CXL_MMER_DEV_EVT_SUB_TYPE_NOT_REPORTED,		"Not Reported" }, \
+	{ CXL_MMER_DEV_EVT_SUB_TYPE_INVALID_CONFIG_DATA,	"Invalid Config Data" }, \
+	{ CXL_MMER_DEV_EVT_SUB_TYPE_UNSUPP_CONFIG_DATA,		"Unsupported Config Data" }, \
+	{ CXL_MMER_DEV_EVT_SUB_TYPE_UNSUPP_MEM_MEDIA_FRU,	"Unsupported Memory Media FRU" } \
+)
+
 TRACE_EVENT(cxl_memory_module,
 
 	TP_PROTO(const struct cxl_memdev *cxlmd, enum cxl_event_log_type log,
@@ -717,6 +741,9 @@ TRACE_EVENT(cxl_memory_module,
 		__field(u32, cor_per_err_cnt)
 		__field(s16, device_temp)
 		__field(u8, add_status)
+		__field(u16, validity_flags)
+		__array(u8, comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
+		__field(u8, sub_type)
 	),
 
 	TP_fast_assign(
@@ -735,12 +762,17 @@ TRACE_EVENT(cxl_memory_module,
 		__entry->cor_per_err_cnt = get_unaligned_le32(rec->info.cor_per_err_cnt);
 		__entry->device_temp = get_unaligned_le16(rec->info.device_temp);
 		__entry->add_status = rec->info.add_status;
+		__entry->validity_flags = get_unaligned_le16(rec->validity_flags);
+		memcpy(__entry->comp_id, &rec->component_id,
+		       CXL_EVENT_GEN_MED_COMP_ID_SIZE);
+		__entry->sub_type = rec->sub_type;
 	),
 
 	CXL_EVT_TP_printk("event_type='%s' health_status='%s' media_status='%s' " \
 		"as_life_used=%s as_dev_temp=%s as_cor_vol_err_cnt=%s " \
 		"as_cor_per_err_cnt=%s life_used=%u device_temp=%d " \
-		"dirty_shutdown_cnt=%u cor_vol_err_cnt=%u cor_per_err_cnt=%u",
+		"dirty_shutdown_cnt=%u cor_vol_err_cnt=%u cor_per_err_cnt=%u " \
+		"validity_flags='%s' comp_id=%s sub_type='%s'",
 		show_dev_evt_type(__entry->event_type),
 		show_health_status_flags(__entry->health_status),
 		show_media_status(__entry->media_status),
@@ -750,7 +782,11 @@ TRACE_EVENT(cxl_memory_module,
 		show_one_bit_status(CXL_DHI_AS_COR_PER_ERR_CNT(__entry->add_status)),
 		__entry->life_used, __entry->device_temp,
 		__entry->dirty_shutdown_cnt, __entry->cor_vol_err_cnt,
-		__entry->cor_per_err_cnt
+		__entry->cor_per_err_cnt,
+		show_mem_module_valid_flags(__entry->validity_flags),
+		cxl_print_component_id(__entry->validity_flags, CXL_MMER_VALID_COMPONENT,
+				       CXL_MMER_VALID_COMPONENT_ID_FORMAT, __entry->comp_id),
+		show_dev_event_sub_type(__entry->sub_type)
 	)
 );
 
diff --git a/include/cxl/event.h b/include/cxl/event.h
index 7e98492c85df..18b7f96dea77 100644
--- a/include/cxl/event.h
+++ b/include/cxl/event.h
@@ -102,7 +102,10 @@ struct cxl_event_mem_module {
 	struct cxl_event_record_hdr hdr;
 	u8 event_type;
 	struct cxl_get_health_info info;
-	u8 reserved[0x3d];
+	u8 validity_flags[2];
+	u8 component_id[CXL_EVENT_GEN_MED_COMP_ID_SIZE];
+	u8 sub_type;
+	u8 reserved[0x2a];
 } __packed;
 
 union cxl_event {
-- 
2.34.1

Re: [RFC PATCH 4/4] cxl/events: Updates for CXL Memory Module Event Record

[Jonathan Cameron]

On Wed, 16 Oct 2024 17:33:49 +0100
<shiju.jose@huawei.com> wrote:

> From: Shiju Jose <shiju.jose@huawei.com>
> 
> CXL spec 3.1 section 8.2.9.2.1.3 Table 8-47, Memory Module Event Record
> has updated with following new fields and new info for Device Event Type
> and Device Health Information fields.
> 1. Validity Flags
> 2. Component Identifier
> 3. Device Event Sub-Type
> 
> Add updates for the above spec changes in the CXL events record and CXL
> Memory Module trace event implementations.
> 
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>

A few minor things inline, but with the event_sub_type naming feel
free to add
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

>  
>  	CXL_EVT_TP_printk("event_type='%s' health_status='%s' media_status='%s' " \
>  		"as_life_used=%s as_dev_temp=%s as_cor_vol_err_cnt=%s " \
>  		"as_cor_per_err_cnt=%s life_used=%u device_temp=%d " \
> -		"dirty_shutdown_cnt=%u cor_vol_err_cnt=%u cor_per_err_cnt=%u",
> +		"dirty_shutdown_cnt=%u cor_vol_err_cnt=%u cor_per_err_cnt=%u " \
> +		"validity_flags='%s' comp_id=%s sub_type='%s'",
>  		show_dev_evt_type(__entry->event_type),
>  		show_health_status_flags(__entry->health_status),
>  		show_media_status(__entry->media_status),
> @@ -750,7 +782,11 @@ TRACE_EVENT(cxl_memory_module,
>  		show_one_bit_status(CXL_DHI_AS_COR_PER_ERR_CNT(__entry->add_status)),
>  		__entry->life_used, __entry->device_temp,
>  		__entry->dirty_shutdown_cnt, __entry->cor_vol_err_cnt,
> -		__entry->cor_per_err_cnt
> +		__entry->cor_per_err_cnt,
> +		show_mem_module_valid_flags(__entry->validity_flags),
> +		cxl_print_component_id(__entry->validity_flags, CXL_MMER_VALID_COMPONENT,
> +				       CXL_MMER_VALID_COMPONENT_ID_FORMAT, __entry->comp_id),
> +		show_dev_event_sub_type(__entry->sub_type)
If we are going to reorganize for the other patches, why not move this next to the event type
field?  There isn't a validity flag for this (0 means not specified)
so fine to move it earlier I think.
>  	)
>  );
>  
> diff --git a/include/cxl/event.h b/include/cxl/event.h
> index 7e98492c85df..18b7f96dea77 100644
> --- a/include/cxl/event.h
> +++ b/include/cxl/event.h
> @@ -102,7 +102,10 @@ struct cxl_event_mem_module {
>  	struct cxl_event_record_hdr hdr;
>  	u8 event_type;
>  	struct cxl_get_health_info info;
> -	u8 reserved[0x3d];
> +	u8 validity_flags[2];
> +	u8 component_id[CXL_EVENT_GEN_MED_COMP_ID_SIZE];
> +	u8 sub_type;
maybe event_sub_type to match spec naming?

> +	u8 reserved[0x2a];
>  } __packed;
>  
>  union cxl_event {

RE: [RFC PATCH 4/4] cxl/events: Updates for CXL Memory Module Event Record

[Shiju Jose]

>-----Original Message-----
>From: Jonathan Cameron <jonathan.cameron@huawei.com>
>Sent: 17 October 2024 13:44
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: dave.jiang@intel.com; dan.j.williams@intel.com; alison.schofield@intel.com;
>vishal.l.verma@intel.com; ira.weiny@intel.com; dave@stgolabs.net; linux-
>cxl@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
><linuxarm@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>; Zengtao (B)
><prime.zeng@hisilicon.com>
>Subject: Re: [RFC PATCH 4/4] cxl/events: Updates for CXL Memory Module Event
>Record
>
>On Wed, 16 Oct 2024 17:33:49 +0100
><shiju.jose@huawei.com> wrote:
>
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> CXL spec 3.1 section 8.2.9.2.1.3 Table 8-47, Memory Module Event
>> Record has updated with following new fields and new info for Device
>> Event Type and Device Health Information fields.
>> 1. Validity Flags
>> 2. Component Identifier
>> 3. Device Event Sub-Type
>>
>> Add updates for the above spec changes in the CXL events record and
>> CXL Memory Module trace event implementations.
>>
>> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
>
>A few minor things inline, but with the event_sub_type naming feel free to add
>Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
>
>>
>>  	CXL_EVT_TP_printk("event_type='%s' health_status='%s'
>media_status='%s' " \
>>  		"as_life_used=%s as_dev_temp=%s as_cor_vol_err_cnt=%s " \
>>  		"as_cor_per_err_cnt=%s life_used=%u device_temp=%d " \
>> -		"dirty_shutdown_cnt=%u cor_vol_err_cnt=%u
>cor_per_err_cnt=%u",
>> +		"dirty_shutdown_cnt=%u cor_vol_err_cnt=%u
>cor_per_err_cnt=%u " \
>> +		"validity_flags='%s' comp_id=%s sub_type='%s'",
>>  		show_dev_evt_type(__entry->event_type),
>>  		show_health_status_flags(__entry->health_status),
>>  		show_media_status(__entry->media_status),
>> @@ -750,7 +782,11 @@ TRACE_EVENT(cxl_memory_module,
>>  		show_one_bit_status(CXL_DHI_AS_COR_PER_ERR_CNT(__entry-
>>add_status)),
>>  		__entry->life_used, __entry->device_temp,
>>  		__entry->dirty_shutdown_cnt, __entry->cor_vol_err_cnt,
>> -		__entry->cor_per_err_cnt
>> +		__entry->cor_per_err_cnt,
>> +		show_mem_module_valid_flags(__entry->validity_flags),
>> +		cxl_print_component_id(__entry->validity_flags,
>CXL_MMER_VALID_COMPONENT,
>> +
>CXL_MMER_VALID_COMPONENT_ID_FORMAT, __entry->comp_id),
>> +		show_dev_event_sub_type(__entry->sub_type)
>If we are going to reorganize for the other patches, why not move this next to
>the event type field?  There isn't a validity flag for this (0 means not specified) so
>fine to move it earlier I think.
Will do.
>>  	)
>>  );
>>
>> diff --git a/include/cxl/event.h b/include/cxl/event.h index
>> 7e98492c85df..18b7f96dea77 100644
>> --- a/include/cxl/event.h
>> +++ b/include/cxl/event.h
>> @@ -102,7 +102,10 @@ struct cxl_event_mem_module {
>>  	struct cxl_event_record_hdr hdr;
>>  	u8 event_type;
>>  	struct cxl_get_health_info info;
>> -	u8 reserved[0x3d];
>> +	u8 validity_flags[2];
>> +	u8 component_id[CXL_EVENT_GEN_MED_COMP_ID_SIZE];
>> +	u8 sub_type;
>maybe event_sub_type to match spec naming?
Will do.
>
>> +	u8 reserved[0x2a];
>>  } __packed;
>>
>>  union cxl_event {

Thanks,
Shiju

Re: [RFC PATCH 0/4] Updates for CXL Event Records

[Alison Schofield]

On Wed, Oct 16, 2024 at 05:33:45PM +0100, shiju.jose@huawei.com wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
> 
> CXL spec rev 3.1 CXL Event Records has updated w.r.t CXL spec rev 3.0.
> Add updates for the above spec changes in the CXL events records and CXL
> trace events implementation.
> 
> Note: Please apply following fix patch first if not present.
> https://patchwork.kernel.org/project/cxl/patch/20241014143003.1170-1-shiju.jose@huawei.com/
> 
> Shiju Jose (4):
>   cxl/events: Updates for CXL Common Event Record Format
>   cxl/events: Updates for CXL General Media Event Record
>   cxl/events: Updates for CXL DRAM Event Record
>   cxl/events: Updates for CXL Memory Module Event Record

Thanks, this looks useful! I didn't review line by line but do
have some feedback before for a v1:

- Suggest being more explicit in the commit msg(s). Something like:
cxl/events: Update Common Event Record to CXL spec 3.1

- I was a bit surprised that this doesn't simply append new fields
to the TP_printk() output. Is there some reason for that?

- How about updating the mock of these events to include these new
fields. I don't think this introduces any new formats, but I would
certainly eyeball all 3: dmesg tp_printk, trace file, and monitor
output because all 3 (sadly) present a bit differently.

-- Alison

> 
>  drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
>  include/cxl/event.h      |  20 +++-
>  2 files changed, 190 insertions(+), 31 deletions(-)
> 
> -- 
> 2.34.1
> 

RE: [RFC PATCH 0/4] Updates for CXL Event Records

[Shiju Jose]

>-----Original Message-----
>From: Alison Schofield <alison.schofield@intel.com>
>Sent: 16 October 2024 22:01
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: dave.jiang@intel.com; dan.j.williams@intel.com; Jonathan Cameron
><jonathan.cameron@huawei.com>; vishal.l.verma@intel.com;
>ira.weiny@intel.com; dave@stgolabs.net; linux-cxl@vger.kernel.org; linux-
>kernel@vger.kernel.org; Linuxarm <linuxarm@huawei.com>; tanxiaofei
><tanxiaofei@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>
>Subject: Re: [RFC PATCH 0/4] Updates for CXL Event Records
>
>On Wed, Oct 16, 2024 at 05:33:45PM +0100, shiju.jose@huawei.com wrote:
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> CXL spec rev 3.1 CXL Event Records has updated w.r.t CXL spec rev 3.0.
>> Add updates for the above spec changes in the CXL events records and
>> CXL trace events implementation.
>>
>> Note: Please apply following fix patch first if not present.
>> https://patchwork.kernel.org/project/cxl/patch/20241014143003.1170-1-s
>> hiju.jose@huawei.com/
>>
>> Shiju Jose (4):
>>   cxl/events: Updates for CXL Common Event Record Format
>>   cxl/events: Updates for CXL General Media Event Record
>>   cxl/events: Updates for CXL DRAM Event Record
>>   cxl/events: Updates for CXL Memory Module Event Record
>
>Thanks, this looks useful! I didn't review line by line but do have some feedback
>before for a v1:
Hi Alison,
Thanks for the feedbacks.

>
>- Suggest being more explicit in the commit msg(s). Something like:
>cxl/events: Update Common Event Record to CXL spec 3.1
Sure. Will add.

>
>- I was a bit surprised that this doesn't simply append new fields to the
>TP_printk() output. Is there some reason for that?
Will do. Thought print new fields before region_name and region_uuid. 

>
>- How about updating the mock of these events to include these new fields. I
>don't think this introduces any new formats, but I would certainly eyeball all 3:
>dmesg tp_printk, trace file, and monitor output because all 3 (sadly) present a
>bit differently.
>
I will update the CXL mock test for these new fields.
 
>-- Alison
>
>>
>>  drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
>>  include/cxl/event.h      |  20 +++-
>>  2 files changed, 190 insertions(+), 31 deletions(-)
>>
>> --
>> 2.34.1
>>
>
Thanks,
Shiju

Re: [RFC PATCH 0/4] Updates for CXL Event Records

[Jonathan Cameron]

On Thu, 17 Oct 2024 10:39:58 +0100
Shiju Jose <shiju.jose@huawei.com> wrote:

> >-----Original Message-----
> >From: Alison Schofield <alison.schofield@intel.com>
> >Sent: 16 October 2024 22:01
> >To: Shiju Jose <shiju.jose@huawei.com>
> >Cc: dave.jiang@intel.com; dan.j.williams@intel.com; Jonathan Cameron
> ><jonathan.cameron@huawei.com>; vishal.l.verma@intel.com;
> >ira.weiny@intel.com; dave@stgolabs.net; linux-cxl@vger.kernel.org; linux-
> >kernel@vger.kernel.org; Linuxarm <linuxarm@huawei.com>; tanxiaofei
> ><tanxiaofei@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>
> >Subject: Re: [RFC PATCH 0/4] Updates for CXL Event Records
> >
> >On Wed, Oct 16, 2024 at 05:33:45PM +0100, shiju.jose@huawei.com wrote:  
> >> From: Shiju Jose <shiju.jose@huawei.com>
> >>
> >> CXL spec rev 3.1 CXL Event Records has updated w.r.t CXL spec rev 3.0.
> >> Add updates for the above spec changes in the CXL events records and
> >> CXL trace events implementation.
> >>
> >> Note: Please apply following fix patch first if not present.
> >> https://patchwork.kernel.org/project/cxl/patch/20241014143003.1170-1-s
> >> hiju.jose@huawei.com/
> >>
> >> Shiju Jose (4):
> >>   cxl/events: Updates for CXL Common Event Record Format
> >>   cxl/events: Updates for CXL General Media Event Record
> >>   cxl/events: Updates for CXL DRAM Event Record
> >>   cxl/events: Updates for CXL Memory Module Event Record  
> >
> >Thanks, this looks useful! I didn't review line by line but do have some feedback
> >before for a v1:  
> Hi Alison,
> Thanks for the feedbacks.
> 
> >
> >- Suggest being more explicit in the commit msg(s). Something like:
> >cxl/events: Update Common Event Record to CXL spec 3.1  
> Sure. Will add.
> 
> >
> >- I was a bit surprised that this doesn't simply append new fields to the
> >TP_printk() output. Is there some reason for that?  
> Will do. Thought print new fields before region_name and region_uuid. 
> 
Worth calling out that the other change is that 3.1 added the option
of a spec defined format for the component ID (various PLDM defined
fields)

I don't think we need to maintain ABI compatibility to TP_printk so to me it
makes sense to print that formatted version if that is what is provide instead
of (rather than as well as) the component IDs.

For RAS Daemon etc, that can be figured out in userspace if needed
so no point in changing the trace point, but the print being the
subfields is nice to have.

Jonathan

> >
> >- How about updating the mock of these events to include these new fields. I
> >don't think this introduces any new formats, but I would certainly eyeball all 3:
> >dmesg tp_printk, trace file, and monitor output because all 3 (sadly) present a
> >bit differently.
> >  
> I will update the CXL mock test for these new fields.
>  
> >-- Alison
> >  
> >>
> >>  drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
> >>  include/cxl/event.h      |  20 +++-
> >>  2 files changed, 190 insertions(+), 31 deletions(-)
> >>
> >> --
> >> 2.34.1
> >>  
> >  
> Thanks,
> Shiju

Re: [RFC PATCH 0/4] Updates for CXL Event Records

[Jonathan Cameron]

On Wed, 16 Oct 2024 17:33:45 +0100
<shiju.jose@huawei.com> wrote:

> From: Shiju Jose <shiju.jose@huawei.com>
> 
> CXL spec rev 3.1 CXL Event Records has updated w.r.t CXL spec rev 3.0.
> Add updates for the above spec changes in the CXL events records and CXL
> trace events implementation.
> 
> Note: Please apply following fix patch first if not present.
> https://patchwork.kernel.org/project/cxl/patch/20241014143003.1170-1-shiju.jose@huawei.com/
> 
> Shiju Jose (4):
>   cxl/events: Updates for CXL Common Event Record Format
>   cxl/events: Updates for CXL General Media Event Record
>   cxl/events: Updates for CXL DRAM Event Record
>   cxl/events: Updates for CXL Memory Module Event Record
> 
>  drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
>  include/cxl/event.h      |  20 +++-
>  2 files changed, 190 insertions(+), 31 deletions(-)
> 

Hi Shiju,

Why are these an RFC?  Seem in a good state to me and the
questions I'm seeing are naming stuff that to me doesn't
justify RFC status.

Jonathan

RE: [RFC PATCH 0/4] Updates for CXL Event Records

[Shiju Jose]

>-----Original Message-----
>From: Jonathan Cameron <jonathan.cameron@huawei.com>
>Sent: 18 October 2024 12:04
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: dave.jiang@intel.com; dan.j.williams@intel.com; alison.schofield@intel.com;
>vishal.l.verma@intel.com; ira.weiny@intel.com; dave@stgolabs.net; linux-
>cxl@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
><linuxarm@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>; Zengtao (B)
><prime.zeng@hisilicon.com>
>Subject: Re: [RFC PATCH 0/4] Updates for CXL Event Records
>
>On Wed, 16 Oct 2024 17:33:45 +0100
><shiju.jose@huawei.com> wrote:
>
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> CXL spec rev 3.1 CXL Event Records has updated w.r.t CXL spec rev 3.0.
>> Add updates for the above spec changes in the CXL events records and
>> CXL trace events implementation.
>>
>> Note: Please apply following fix patch first if not present.
>> https://patchwork.kernel.org/project/cxl/patch/20241014143003.1170-1-s
>> hiju.jose@huawei.com/
>>
>> Shiju Jose (4):
>>   cxl/events: Updates for CXL Common Event Record Format
>>   cxl/events: Updates for CXL General Media Event Record
>>   cxl/events: Updates for CXL DRAM Event Record
>>   cxl/events: Updates for CXL Memory Module Event Record
>>
>>  drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
>>  include/cxl/event.h      |  20 +++-
>>  2 files changed, 190 insertions(+), 31 deletions(-)
>>
>
>Hi Shiju,
>
>Why are these an RFC?  Seem in a good state to me and the questions I'm seeing
>are naming stuff that to me doesn't justify RFC status.
>
>Jonathan

Hi Jonathan,

I add RFC since it is v1.  I will exclude RFC in v2.  

Thanks,
Shiju