linux-kernel - RE: [PATCH v5 4/6] cxl/events: Update DRAM Event Record to CXL spec rev 3.1

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <5a0d7679f7d44f07a47eb47651f9b8ca@huawei.com>
Date: Sat, 11 Jan 2025 09:16:07 +0000
From: Shiju Jose <shiju.jose@...wei.com>
To: Ira Weiny <ira.weiny@...el.com>, "dave.jiang@...el.com"
	<dave.jiang@...el.com>, "dan.j.williams@...el.com"
	<dan.j.williams@...el.com>, Jonathan Cameron <jonathan.cameron@...wei.com>,
	"alison.schofield@...el.com" <alison.schofield@...el.com>,
	"nifan.cxl@...il.com" <nifan.cxl@...il.com>, "vishal.l.verma@...el.com"
	<vishal.l.verma@...el.com>, "dave@...olabs.net" <dave@...olabs.net>,
	"linux-cxl@...r.kernel.org" <linux-cxl@...r.kernel.org>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Linuxarm
	<linuxarm@...wei.com>, tanxiaofei <tanxiaofei@...wei.com>, "Zengtao (B)"
	<prime.zeng@...ilicon.com>
Subject: RE: [PATCH v5 4/6] cxl/events: Update DRAM Event Record to CXL spec
 rev 3.1



>-----Original Message-----
>From: Ira Weiny <ira.weiny@...el.com>
>Sent: 10 January 2025 18:27
>To: Shiju Jose <shiju.jose@...wei.com>; dave.jiang@...el.com;
>dan.j.williams@...el.com; Jonathan Cameron
><jonathan.cameron@...wei.com>; alison.schofield@...el.com;
>nifan.cxl@...il.com; vishal.l.verma@...el.com; ira.weiny@...el.com;
>dave@...olabs.net; linux-cxl@...r.kernel.org
>Cc: linux-kernel@...r.kernel.org; Linuxarm <linuxarm@...wei.com>;
>tanxiaofei <tanxiaofei@...wei.com>; Zengtao (B) <prime.zeng@...ilicon.com>;
>Shiju Jose <shiju.jose@...wei.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@...wei.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]