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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20241017132505.00004cf4@Huawei.com>
Date: Thu, 17 Oct 2024 13:25:05 +0100
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: <shiju.jose@...wei.com>
CC: <dave.jiang@...el.com>, <dan.j.williams@...el.com>,
	<alison.schofield@...el.com>, <vishal.l.verma@...el.com>,
	<ira.weiny@...el.com>, <dave@...olabs.net>, <linux-cxl@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, <linuxarm@...wei.com>,
	<tanxiaofei@...wei.com>, <prime.zeng@...ilicon.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@...wei.com> wrote:

> From: Shiju Jose <shiju.jose@...wei.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@...wei.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@...wei.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 :)