Re: [PATCH 2/4] cxl/events: Add extra validity checks for corrected memory error count in General Media Event Record

[Dave Jiang <dave.jiang@intel.com>]

On 7/16/25 3:49 AM, shiju.jose@huawei.com wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
> 
> According to the CXL Specification Revision 3.2, Section 8.2.10.2.1.1,
> Table 8-57 (General Media Event Record), the Corrected Memory Error Count
> field is valid under the following conditions:
> 1. The Threshold Event bit is set in the Memory Event Descriptor field, and
> 2. The Corrected Memory Error Count must be greater than 0 for events where
>    the Advanced Programmable Threshold Counter has expired.
> 
> Additionally, if the Advanced Programmable Corrected Memory Error Counter
> Expire bit in the Memory Event Type field is set, then the Threshold Event
> bit in the Memory Event Descriptor field shall also be set.
> 
> Add validity checks for the above conditions while reporting the event to
> the userspace.
> 
> Note: CXL spec rev3.2 Table 8-57. General Media Event Record
> Field: Corrected Memory Error Count at Event) "For events in which the
> advanced programmable threshold counter has expired, this field value
> shall be a value greater than 0. Counter expiration events in which
> the corrected memory error count is 0 shall not generate a media event
> record".
> Q: Should kernel drop the event record in this case or user space
> to handle?
> 
> Signed-off-by: Shiju Jose <shiju.jose@huawei.com>

As Jonathan mentioned, the Q doesn't belong in the commit log.

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/mbox.c  | 9 +++++++++
>  drivers/cxl/core/trace.h | 5 ++++-
>  2 files changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 2689e6453c5a..5a30d3891b17 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -926,6 +926,15 @@ void cxl_event_trace_record(const struct cxl_memdev *cxlmd,
>  			if (cxl_store_rec_gen_media((struct cxl_memdev *)cxlmd, evt))
>  				dev_dbg(&cxlmd->dev, "CXL store rec_gen_media failed\n");
>  
> +			if (evt->gen_media.media_hdr.descriptor &
> +			    CXL_GMER_EVT_DESC_THRESHOLD_EVENT)
> +				WARN_ON_ONCE((evt->gen_media.media_hdr.type &
> +					      CXL_GMER_MEM_EVT_TYPE_AP_CME_COUNTER_EXPIRE) &&
> +					     !evt->gen_media.cme_count);
> +			else
> +				WARN_ON_ONCE(evt->gen_media.media_hdr.type &
> +					     CXL_GMER_MEM_EVT_TYPE_AP_CME_COUNTER_EXPIRE);
> +
>  			trace_cxl_general_media(cxlmd, type, cxlr, hpa,
>  						hpa_alias, &evt->gen_media);
>  		} else if (event_type == CXL_CPER_EVENT_DRAM) {
> diff --git a/drivers/cxl/core/trace.h b/drivers/cxl/core/trace.h
> index a77487a257b3..c38f94ca0ca1 100644
> --- a/drivers/cxl/core/trace.h
> +++ b/drivers/cxl/core/trace.h
> @@ -506,7 +506,10 @@ TRACE_EVENT(cxl_general_media,
>  			uuid_copy(&__entry->region_uuid, &uuid_null);
>  		}
>  		__entry->cme_threshold_ev_flags = rec->cme_threshold_ev_flags;
> -		__entry->cme_count = get_unaligned_le24(rec->cme_count);
> +		if (rec->media_hdr.descriptor & CXL_GMER_EVT_DESC_THRESHOLD_EVENT)
> +			__entry->cme_count = get_unaligned_le24(rec->cme_count);
> +		else
> +			__entry->cme_count = 0;
>  	),
>  
>  	CXL_EVT_TP_printk("dpa=%llx dpa_flags='%s' " \