diff mbox series

[v5,4/6] cxl/events: Update DRAM Event Record to CXL spec rev 3.1

Message ID 20250110115556.1654-5-shiju.jose@huawei.com
State Superseded
Headers show
Series Update Event Records to CXL spec rev 3.1 | expand

Commit Message

Shiju Jose Jan. 10, 2025, 11:55 a.m. UTC 
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

Update DRAM events record and DRAM trace event for the above spec
changes. The new fields are inserted in logical places.
Includes trivial consistency of white space improvements.

Example trace print of cxl_dram trace event,

cxl_dram: memdev=mem0 host=0000:0f:00.0 serial=3 log=Informational : \
time=54799339519 uuid=601dcbb3-9c06-4eab-b8af-4e9bfb5c9624 len=128 \
flags='0x1' handle=1 related_handle=0 maint_op_class=1 \
maint_op_sub_class=3 : dpa=18680 dpa_flags='' \
descriptor='UNCORRECTABLE_EVENT|THRESHOLD_EVENT' type='Data Path Error' \
sub_type='Media Link CRC Error' transaction_type='Internal Media Scrub' \
channel=3 rank=17 nibble_mask=3b00b2 bank_group=7 bank=11 row=2 \
column=77 cor_mask=21 00 00 00 00 00 00 00 2c 00 00 00 00 00 00 00 37 00 \
00 00 00 00 00 00 42 00 00 00 00 00 00 00 validity_flags='CHANNEL|RANK|NIBBLE|\
BANK GROUP|BANK|ROW|COLUMN|CORRECTION MASK|COMPONENT|COMPONENT PLDM FORMAT' \
comp_id=01 74 c5 08 9a 1a 0b fc d2 7e 2f 31 9b 3c 81 4d \
comp_id_pldm_valid_flags='PLDM Entity ID' pldm_entity_id=74 c5 08 9a 1a 0b \
pldm_resource_id=0x00 hpa=ffffffffffffffff region= \
region_uuid=00000000-0000-0000-0000-000000000000 sub_channel=5 \
cme_threshold_ev_flags='Corrected Memory Errors in Multiple Media Components|\
Exceeded Programmable Threshold' cvme_count=148

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/cxl/core/trace.h | 71 ++++++++++++++++++++++++++++++----------
 include/cxl/event.h      |  9 +++--
 2 files changed, 60 insertions(+), 20 deletions(-)

Comments

Ira Weiny Jan. 10, 2025, 6:27 p.m. UTC | #1 
shiju.jose@ wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
> 

[snip]

>  
>  TRACE_EVENT(cxl_dram,
> @@ -527,6 +539,7 @@ TRACE_EVENT(cxl_dram,
>  		__field(u64, dpa)
>  		__field(u8, descriptor)
>  		__field(u8, type)
> +		__field(u8, sub_type)

I just noticed this with the previous patch too.  To pack the record this
should be below...

>  		__field(u8, transaction_type)
>  		__field(u8, channel)
>  		__field(u16, validity_flags)
> @@ -541,6 +554,10 @@ TRACE_EVENT(cxl_dram,
>  		__field(u8, bank)	/* Out of order to pack trace record */
>  		__field(u8, dpa_flags)	/* Out of order to pack trace record */

Here.

>  		__string(region_name, cxlr ? dev_name(&cxlr->dev) : "")
> +		__array(u8, comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
> +		__field(u8, sub_channel)
> +		__field(u8, cme_threshold_ev_flags)
> +		__field(u32, cvme_count)
>  	),

And these reordered too.  Like this:

diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
index cbaf6244d77f..6f4bf4925cbf 100644
--- a/drivers/cxl/core/trace.h
+++ b/drivers/cxl/core/trace.h
@@ -539,7 +539,6 @@ TRACE_EVENT(cxl_dram,
                __field(u64, dpa)
                __field(u8, descriptor)
                __field(u8, type)
-               __field(u8, sub_type)
                __field(u8, transaction_type)
                __field(u8, channel)
                __field(u16, validity_flags)
@@ -553,11 +552,13 @@ 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 */
-               __string(region_name, cxlr ? dev_name(&cxlr->dev) : "")
+               /* The following are out of order to pack the trace record */
                __array(u8, comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
+               __field(u32, cvme_count)
+               __field(u8, sub_type)
                __field(u8, sub_channel)
                __field(u8, cme_threshold_ev_flags)
-               __field(u32, cvme_count)
+               __string(region_name, cxlr ? dev_name(&cxlr->dev) : "")
        ),
 
        TP_fast_assign(



Other than that it looks good.  Perhaps Dave can just squash this hunk?

Ira

[snip]
Shiju Jose Jan. 11, 2025, 9:16 a.m. UTC | #2 
>-----Original Message-----
>From: Ira Weiny <ira.weiny@intel.com>
>Sent: 10 January 2025 18:27
>To: Shiju Jose <shiju.jose@huawei.com>; dave.jiang@intel.com;
>dan.j.williams@intel.com; Jonathan Cameron
><jonathan.cameron@huawei.com>; alison.schofield@intel.com;
>nifan.cxl@gmail.com; vishal.l.verma@intel.com; ira.weiny@intel.com;
>dave@stgolabs.net; linux-cxl@vger.kernel.org
>Cc: linux-kernel@vger.kernel.org; Linuxarm <linuxarm@huawei.com>;
>tanxiaofei <tanxiaofei@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>;
>Shiju Jose <shiju.jose@huawei.com>
>Subject: Re: [PATCH v5 4/6] cxl/events: Update DRAM Event Record to CXL spec
>rev 3.1
>
>shiju.jose@ wrote:
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>
>[snip]
>
>>
>>  TRACE_EVENT(cxl_dram,
>> @@ -527,6 +539,7 @@ TRACE_EVENT(cxl_dram,
>>  		__field(u64, dpa)
>>  		__field(u8, descriptor)
>>  		__field(u8, type)
>> +		__field(u8, sub_type)
>
>I just noticed this with the previous patch too.  To pack the record this should be
>below...

Hi Ira,

Thanks for reviewing and for the feedbacks. 
I have modified for your suggestions and will be in the v6.

Thanks,
Shiju

>
>>  		__field(u8, transaction_type)
>>  		__field(u8, channel)
>>  		__field(u16, validity_flags)
>> @@ -541,6 +554,10 @@ TRACE_EVENT(cxl_dram,
>>  		__field(u8, bank)	/* Out of order to pack trace record */
>>  		__field(u8, dpa_flags)	/* Out of order to pack trace record */
>
>Here.
>
>>  		__string(region_name, cxlr ? dev_name(&cxlr->dev) : "")
>> +		__array(u8, comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
>> +		__field(u8, sub_channel)
>> +		__field(u8, cme_threshold_ev_flags)
>> +		__field(u32, cvme_count)
>>  	),
>
>And these reordered too.  Like this:
>
>diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h index
>cbaf6244d77f..6f4bf4925cbf 100644
>--- a/drivers/cxl/core/trace.h
>+++ b/drivers/cxl/core/trace.h
>@@ -539,7 +539,6 @@ TRACE_EVENT(cxl_dram,
>                __field(u64, dpa)
>                __field(u8, descriptor)
>                __field(u8, type)
>-               __field(u8, sub_type)
>                __field(u8, transaction_type)
>                __field(u8, channel)
>                __field(u16, validity_flags) @@ -553,11 +552,13 @@
>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 */
>-               __string(region_name, cxlr ? dev_name(&cxlr->dev) : "")
>+               /* The following are out of order to pack the trace
>+ record */
>                __array(u8, comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
>+               __field(u32, cvme_count)
>+               __field(u8, sub_type)
>                __field(u8, sub_channel)
>                __field(u8, cme_threshold_ev_flags)
>-               __field(u32, cvme_count)
>+               __string(region_name, cxlr ? dev_name(&cxlr->dev) : "")
>        ),
>
>        TP_fast_assign(
>
>
>
>Other than that it looks good.  Perhaps Dave can just squash this hunk?
>
>Ira
>
>[snip]
diff mbox series

Patch

diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
index 241c10596f4d..cbaf6244d77f 100644
--- a/drivers/cxl/core/trace.h
+++ b/drivers/cxl/core/trace.h
@@ -478,7 +478,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
@@ -488,11 +488,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,				\
-	{ 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" }		\
+#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_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)
@@ -503,15 +509,21 @@  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, "|",			   \
-	{ CXL_DER_VALID_CHANNEL,			"CHANNEL"		}, \
-	{ CXL_DER_VALID_RANK,				"RANK"			}, \
-	{ CXL_DER_VALID_NIBBLE,				"NIBBLE"		}, \
-	{ CXL_DER_VALID_BANK_GROUP,			"BANK GROUP"		}, \
-	{ CXL_DER_VALID_BANK,				"BANK"			}, \
-	{ CXL_DER_VALID_ROW,				"ROW"			}, \
-	{ CXL_DER_VALID_COLUMN,				"COLUMN"		}, \
-	{ CXL_DER_VALID_CORRECTION_MASK,		"CORRECTION MASK"	}  \
+#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"		},	\
+	{ CXL_DER_VALID_BANK_GROUP,			"BANK GROUP"		},	\
+	{ 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_COMPONENT,			"COMPONENT"		},	\
+	{ CXL_DER_VALID_COMPONENT_ID_FORMAT,		"COMPONENT PLDM FORMAT"	},	\
+	{ CXL_DER_VALID_SUB_CHANNEL,			"SUB CHANNEL"		}	\
 )
 
 TRACE_EVENT(cxl_dram,
@@ -527,6 +539,7 @@  TRACE_EVENT(cxl_dram,
 		__field(u64, dpa)
 		__field(u8, descriptor)
 		__field(u8, type)
+		__field(u8, sub_type)
 		__field(u8, transaction_type)
 		__field(u8, channel)
 		__field(u16, validity_flags)
@@ -541,6 +554,10 @@  TRACE_EVENT(cxl_dram,
 		__field(u8, bank)	/* Out of order to pack trace record */
 		__field(u8, dpa_flags)	/* Out of order to pack trace record */
 		__string(region_name, cxlr ? dev_name(&cxlr->dev) : "")
+		__array(u8, comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE)
+		__field(u8, sub_channel)
+		__field(u8, cme_threshold_ev_flags)
+		__field(u32, cvme_count)
 	),
 
 	TP_fast_assign(
@@ -553,6 +570,7 @@  TRACE_EVENT(cxl_dram,
 		__entry->dpa &= CXL_DPA_MASK;
 		__entry->descriptor = rec->media_hdr.descriptor;
 		__entry->type = rec->media_hdr.type;
+		__entry->sub_type = rec->sub_type;
 		__entry->transaction_type = rec->media_hdr.transaction_type;
 		__entry->validity_flags = get_unaligned_le16(rec->media_hdr.validity_flags);
 		__entry->channel = rec->media_hdr.channel;
@@ -572,23 +590,40 @@  TRACE_EVENT(cxl_dram,
 			__assign_str(region_name);
 			uuid_copy(&__entry->region_uuid, &uuid_null);
 		}
+		memcpy(__entry->comp_id, &rec->component_id,
+		       CXL_EVENT_GEN_MED_COMP_ID_SIZE);
+		__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);
 	),
 
-	CXL_EVT_TP_printk("dpa=%llx dpa_flags='%s' descriptor='%s' type='%s' " \
+	CXL_EVT_TP_printk("dpa=%llx dpa_flags='%s' descriptor='%s' type='%s' sub_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 comp_id_pldm_valid_flags='%s' " \
+		"pldm_entity_id=%s pldm_resource_id=%s " \
+		"hpa=%llx region=%s region_uuid=%pUb " \
+		"sub_channel=%u cme_threshold_ev_flags='%s' cvme_count=%u",
 		__entry->dpa, show_dpa_flags(__entry->dpa_flags),
 		show_event_desc_flags(__entry->descriptor),
 		show_dram_mem_event_type(__entry->type),
+		show_mem_event_sub_type(__entry->sub_type),
 		show_trans_type(__entry->transaction_type),
 		__entry->channel, __entry->rank, __entry->nibble_mask,
 		__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),
-		__entry->hpa, __get_str(region_name), &__entry->region_uuid
+		__print_hex(__entry->comp_id, CXL_EVENT_GEN_MED_COMP_ID_SIZE),
+		show_comp_id_pldm_flags(__entry->comp_id[0]),
+		show_pldm_entity_id(__entry->validity_flags, CXL_DER_VALID_COMPONENT,
+				    CXL_DER_VALID_COMPONENT_ID_FORMAT, __entry->comp_id),
+		show_pldm_resource_id(__entry->validity_flags, CXL_DER_VALID_COMPONENT,
+				      CXL_DER_VALID_COMPONENT_ID_FORMAT, __entry->comp_id),
+		__entry->hpa, __get_str(region_name), &__entry->region_uuid,
+		__entry->sub_channel, show_cme_threshold_ev_flags(__entry->cme_threshold_ev_flags),
+		__entry->cvme_count
 	)
 );
 
diff --git a/include/cxl/event.h b/include/cxl/event.h
index 2b07adf39010..10815414f376 100644
--- a/include/cxl/event.h
+++ b/include/cxl/event.h
@@ -60,7 +60,7 @@  struct cxl_event_gen_media {
 
 /*
  * DRAM Event Record - DER
- * CXL rev 3.0 section 8.2.9.2.1.2; Table 3-44
+ * CXL rev 3.1 section 8.2.9.2.1.2; Table 8-46
  */
 #define CXL_EVENT_DER_CORRECTION_MASK_SIZE	0x20
 struct cxl_event_dram {
@@ -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;
 
 /*