[RFC PATCH 0/4] Updates for CXL Event Records

shiju.jose@huawei.com posted 4 patches 1 month, 1 week ago
drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
include/cxl/event.h      |  20 +++-
2 files changed, 190 insertions(+), 31 deletions(-)
[RFC PATCH 0/4] Updates for CXL Event Records
Posted by shiju.jose@huawei.com 1 month, 1 week ago
From: Shiju Jose <shiju.jose@huawei.com>

CXL spec rev 3.1 CXL Event Records has updated w.r.t CXL spec rev 3.0.
Add updates for the above spec changes in the CXL events records and CXL
trace events implementation.

Note: Please apply following fix patch first if not present.
https://patchwork.kernel.org/project/cxl/patch/20241014143003.1170-1-shiju.jose@huawei.com/

Shiju Jose (4):
  cxl/events: Updates for CXL Common Event Record Format
  cxl/events: Updates for CXL General Media Event Record
  cxl/events: Updates for CXL DRAM Event Record
  cxl/events: Updates for CXL Memory Module Event Record

 drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
 include/cxl/event.h      |  20 +++-
 2 files changed, 190 insertions(+), 31 deletions(-)

-- 
2.34.1
Re: [RFC PATCH 0/4] Updates for CXL Event Records
Posted by Jonathan Cameron 1 month, 1 week ago
On Wed, 16 Oct 2024 17:33:45 +0100
<shiju.jose@huawei.com> wrote:

> From: Shiju Jose <shiju.jose@huawei.com>
> 
> CXL spec rev 3.1 CXL Event Records has updated w.r.t CXL spec rev 3.0.
> Add updates for the above spec changes in the CXL events records and CXL
> trace events implementation.
> 
> Note: Please apply following fix patch first if not present.
> https://patchwork.kernel.org/project/cxl/patch/20241014143003.1170-1-shiju.jose@huawei.com/
> 
> Shiju Jose (4):
>   cxl/events: Updates for CXL Common Event Record Format
>   cxl/events: Updates for CXL General Media Event Record
>   cxl/events: Updates for CXL DRAM Event Record
>   cxl/events: Updates for CXL Memory Module Event Record
> 
>  drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
>  include/cxl/event.h      |  20 +++-
>  2 files changed, 190 insertions(+), 31 deletions(-)
> 

Hi Shiju,

Why are these an RFC?  Seem in a good state to me and the
questions I'm seeing are naming stuff that to me doesn't
justify RFC status.

Jonathan
RE: [RFC PATCH 0/4] Updates for CXL Event Records
Posted by Shiju Jose 1 month, 1 week ago

>-----Original Message-----
>From: Jonathan Cameron <jonathan.cameron@huawei.com>
>Sent: 18 October 2024 12:04
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: dave.jiang@intel.com; dan.j.williams@intel.com; alison.schofield@intel.com;
>vishal.l.verma@intel.com; ira.weiny@intel.com; dave@stgolabs.net; linux-
>cxl@vger.kernel.org; linux-kernel@vger.kernel.org; Linuxarm
><linuxarm@huawei.com>; tanxiaofei <tanxiaofei@huawei.com>; Zengtao (B)
><prime.zeng@hisilicon.com>
>Subject: Re: [RFC PATCH 0/4] Updates for CXL Event Records
>
>On Wed, 16 Oct 2024 17:33:45 +0100
><shiju.jose@huawei.com> wrote:
>
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> CXL spec rev 3.1 CXL Event Records has updated w.r.t CXL spec rev 3.0.
>> Add updates for the above spec changes in the CXL events records and
>> CXL trace events implementation.
>>
>> Note: Please apply following fix patch first if not present.
>> https://patchwork.kernel.org/project/cxl/patch/20241014143003.1170-1-s
>> hiju.jose@huawei.com/
>>
>> Shiju Jose (4):
>>   cxl/events: Updates for CXL Common Event Record Format
>>   cxl/events: Updates for CXL General Media Event Record
>>   cxl/events: Updates for CXL DRAM Event Record
>>   cxl/events: Updates for CXL Memory Module Event Record
>>
>>  drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
>>  include/cxl/event.h      |  20 +++-
>>  2 files changed, 190 insertions(+), 31 deletions(-)
>>
>
>Hi Shiju,
>
>Why are these an RFC?  Seem in a good state to me and the questions I'm seeing
>are naming stuff that to me doesn't justify RFC status.
>
>Jonathan

Hi Jonathan,

I add RFC since it is v1.  I will exclude RFC in v2.  

Thanks,
Shiju
Re: [RFC PATCH 0/4] Updates for CXL Event Records
Posted by Alison Schofield 1 month, 1 week ago
On Wed, Oct 16, 2024 at 05:33:45PM +0100, shiju.jose@huawei.com wrote:
> From: Shiju Jose <shiju.jose@huawei.com>
> 
> CXL spec rev 3.1 CXL Event Records has updated w.r.t CXL spec rev 3.0.
> Add updates for the above spec changes in the CXL events records and CXL
> trace events implementation.
> 
> Note: Please apply following fix patch first if not present.
> https://patchwork.kernel.org/project/cxl/patch/20241014143003.1170-1-shiju.jose@huawei.com/
> 
> Shiju Jose (4):
>   cxl/events: Updates for CXL Common Event Record Format
>   cxl/events: Updates for CXL General Media Event Record
>   cxl/events: Updates for CXL DRAM Event Record
>   cxl/events: Updates for CXL Memory Module Event Record

Thanks, this looks useful! I didn't review line by line but do
have some feedback before for a v1:

- Suggest being more explicit in the commit msg(s). Something like:
cxl/events: Update Common Event Record to CXL spec 3.1

- I was a bit surprised that this doesn't simply append new fields
to the TP_printk() output. Is there some reason for that?

- How about updating the mock of these events to include these new
fields. I don't think this introduces any new formats, but I would
certainly eyeball all 3: dmesg tp_printk, trace file, and monitor
output because all 3 (sadly) present a bit differently.

-- Alison

> 
>  drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
>  include/cxl/event.h      |  20 +++-
>  2 files changed, 190 insertions(+), 31 deletions(-)
> 
> -- 
> 2.34.1
>
RE: [RFC PATCH 0/4] Updates for CXL Event Records
Posted by Shiju Jose 1 month, 1 week ago
>-----Original Message-----
>From: Alison Schofield <alison.schofield@intel.com>
>Sent: 16 October 2024 22:01
>To: Shiju Jose <shiju.jose@huawei.com>
>Cc: dave.jiang@intel.com; dan.j.williams@intel.com; Jonathan Cameron
><jonathan.cameron@huawei.com>; vishal.l.verma@intel.com;
>ira.weiny@intel.com; dave@stgolabs.net; linux-cxl@vger.kernel.org; linux-
>kernel@vger.kernel.org; Linuxarm <linuxarm@huawei.com>; tanxiaofei
><tanxiaofei@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>
>Subject: Re: [RFC PATCH 0/4] Updates for CXL Event Records
>
>On Wed, Oct 16, 2024 at 05:33:45PM +0100, shiju.jose@huawei.com wrote:
>> From: Shiju Jose <shiju.jose@huawei.com>
>>
>> CXL spec rev 3.1 CXL Event Records has updated w.r.t CXL spec rev 3.0.
>> Add updates for the above spec changes in the CXL events records and
>> CXL trace events implementation.
>>
>> Note: Please apply following fix patch first if not present.
>> https://patchwork.kernel.org/project/cxl/patch/20241014143003.1170-1-s
>> hiju.jose@huawei.com/
>>
>> Shiju Jose (4):
>>   cxl/events: Updates for CXL Common Event Record Format
>>   cxl/events: Updates for CXL General Media Event Record
>>   cxl/events: Updates for CXL DRAM Event Record
>>   cxl/events: Updates for CXL Memory Module Event Record
>
>Thanks, this looks useful! I didn't review line by line but do have some feedback
>before for a v1:
Hi Alison,
Thanks for the feedbacks.

>
>- Suggest being more explicit in the commit msg(s). Something like:
>cxl/events: Update Common Event Record to CXL spec 3.1
Sure. Will add.

>
>- I was a bit surprised that this doesn't simply append new fields to the
>TP_printk() output. Is there some reason for that?
Will do. Thought print new fields before region_name and region_uuid. 

>
>- How about updating the mock of these events to include these new fields. I
>don't think this introduces any new formats, but I would certainly eyeball all 3:
>dmesg tp_printk, trace file, and monitor output because all 3 (sadly) present a
>bit differently.
>
I will update the CXL mock test for these new fields.
 
>-- Alison
>
>>
>>  drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
>>  include/cxl/event.h      |  20 +++-
>>  2 files changed, 190 insertions(+), 31 deletions(-)
>>
>> --
>> 2.34.1
>>
>
Thanks,
Shiju
Re: [RFC PATCH 0/4] Updates for CXL Event Records
Posted by Jonathan Cameron 1 month, 1 week ago
On Thu, 17 Oct 2024 10:39:58 +0100
Shiju Jose <shiju.jose@huawei.com> wrote:

> >-----Original Message-----
> >From: Alison Schofield <alison.schofield@intel.com>
> >Sent: 16 October 2024 22:01
> >To: Shiju Jose <shiju.jose@huawei.com>
> >Cc: dave.jiang@intel.com; dan.j.williams@intel.com; Jonathan Cameron
> ><jonathan.cameron@huawei.com>; vishal.l.verma@intel.com;
> >ira.weiny@intel.com; dave@stgolabs.net; linux-cxl@vger.kernel.org; linux-
> >kernel@vger.kernel.org; Linuxarm <linuxarm@huawei.com>; tanxiaofei
> ><tanxiaofei@huawei.com>; Zengtao (B) <prime.zeng@hisilicon.com>
> >Subject: Re: [RFC PATCH 0/4] Updates for CXL Event Records
> >
> >On Wed, Oct 16, 2024 at 05:33:45PM +0100, shiju.jose@huawei.com wrote:  
> >> From: Shiju Jose <shiju.jose@huawei.com>
> >>
> >> CXL spec rev 3.1 CXL Event Records has updated w.r.t CXL spec rev 3.0.
> >> Add updates for the above spec changes in the CXL events records and
> >> CXL trace events implementation.
> >>
> >> Note: Please apply following fix patch first if not present.
> >> https://patchwork.kernel.org/project/cxl/patch/20241014143003.1170-1-s
> >> hiju.jose@huawei.com/
> >>
> >> Shiju Jose (4):
> >>   cxl/events: Updates for CXL Common Event Record Format
> >>   cxl/events: Updates for CXL General Media Event Record
> >>   cxl/events: Updates for CXL DRAM Event Record
> >>   cxl/events: Updates for CXL Memory Module Event Record  
> >
> >Thanks, this looks useful! I didn't review line by line but do have some feedback
> >before for a v1:  
> Hi Alison,
> Thanks for the feedbacks.
> 
> >
> >- Suggest being more explicit in the commit msg(s). Something like:
> >cxl/events: Update Common Event Record to CXL spec 3.1  
> Sure. Will add.
> 
> >
> >- I was a bit surprised that this doesn't simply append new fields to the
> >TP_printk() output. Is there some reason for that?  
> Will do. Thought print new fields before region_name and region_uuid. 
> 
Worth calling out that the other change is that 3.1 added the option
of a spec defined format for the component ID (various PLDM defined
fields)

I don't think we need to maintain ABI compatibility to TP_printk so to me it
makes sense to print that formatted version if that is what is provide instead
of (rather than as well as) the component IDs.

For RAS Daemon etc, that can be figured out in userspace if needed
so no point in changing the trace point, but the print being the
subfields is nice to have.

Jonathan

> >
> >- How about updating the mock of these events to include these new fields. I
> >don't think this introduces any new formats, but I would certainly eyeball all 3:
> >dmesg tp_printk, trace file, and monitor output because all 3 (sadly) present a
> >bit differently.
> >  
> I will update the CXL mock test for these new fields.
>  
> >-- Alison
> >  
> >>
> >>  drivers/cxl/core/trace.h | 201 +++++++++++++++++++++++++++++++++------
> >>  include/cxl/event.h      |  20 +++-
> >>  2 files changed, 190 insertions(+), 31 deletions(-)
> >>
> >> --
> >> 2.34.1
> >>  
> >  
> Thanks,
> Shiju