| Message ID | 20241016163349.1210-1-shiju.jose@huawei.com |
|---|---|
| Headers | show |
| Series | Updates for CXL Event Records | expand |
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 >
>-----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
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
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
>-----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
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(-)