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

Jonathan Cameron
2024-10-17T12:25:09Z

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 :)