* [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and Get Supported Logs Sub-List commands @ 2024-02-07 10:36 sthanneeru.opensrc 2024-02-08 12:57 ` Jonathan Cameron 0 siblings, 1 reply; 8+ messages in thread From: sthanneeru.opensrc @ 2024-02-07 10:36 UTC (permalink / raw) To: linux-cxl, linux-mm, sthanneeru.opensrc Cc: Jonathan.Cameron, dan.j.williams, john, emirakhur, ajayjoshi, Ravis.OpenSrc, sthanneeru From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> Adding UAPI support for 1. CXL r3.1 8.2.9.5.3 Get Log Capabilities. 2. CXL r3.1 8.2.9.5.4 Clear Log commands. 3. CXL r3.1 8.2.9.5.6 Get Supported Logs Sub-List. Signed-off-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> --- drivers/cxl/core/mbox.c | 3 +++ drivers/cxl/cxlmem.h | 3 +++ include/uapi/linux/cxl_mem.h | 3 +++ 3 files changed, 9 insertions(+) diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c index 27166a411705..64a44e286488 100644 --- a/drivers/cxl/core/mbox.c +++ b/drivers/cxl/core/mbox.c @@ -64,6 +64,9 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = { CXL_CMD(SET_SHUTDOWN_STATE, 0x1, 0, 0), CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0), CXL_CMD(GET_TIMESTAMP, 0, 0x8, 0), + CXL_CMD(GET_LOG_CAPS, 0x10, 0x4, 0), + CXL_CMD(CLEAR_LOG, 0x10, 0, 0), + CXL_CMD(GET_LOG_SUBLIST, 0x2, CXL_VARIABLE_PAYLOAD, 0), }; /* diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h index 5303d6942b88..4128c810051c 100644 --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -529,6 +529,9 @@ enum cxl_opcode { CXL_MBOX_OP_SET_TIMESTAMP = 0x0301, CXL_MBOX_OP_GET_SUPPORTED_LOGS = 0x0400, CXL_MBOX_OP_GET_LOG = 0x0401, + CXL_MBOX_OP_GET_LOG_CAPS = 0x0402, + CXL_MBOX_OP_CLEAR_LOG = 0x0403, + CXL_MBOX_OP_GET_LOG_SUBLIST = 0x0405, CXL_MBOX_OP_IDENTIFY = 0x4000, CXL_MBOX_OP_GET_PARTITION_INFO = 0x4100, CXL_MBOX_OP_SET_PARTITION_INFO = 0x4101, diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h index 42066f4eb890..d2df9782a5ef 100644 --- a/include/uapi/linux/cxl_mem.h +++ b/include/uapi/linux/cxl_mem.h @@ -47,6 +47,9 @@ ___DEPRECATED(SCAN_MEDIA, "Scan Media"), \ ___DEPRECATED(GET_SCAN_MEDIA, "Get Scan Media Results"), \ ___C(GET_TIMESTAMP, "Get Timestamp"), \ + ___C(GET_LOG_CAPS, "Get Log Capabilities"), \ + ___C(CLEAR_LOG, "Clear Log"), \ + ___C(GET_LOG_SUBLIST, "Get Log Sublist"), \ ___C(MAX, "invalid / last command") #define ___C(a, b) CXL_MEM_COMMAND_ID_##a -- 2.43.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and Get Supported Logs Sub-List commands 2024-02-07 10:36 [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and Get Supported Logs Sub-List commands sthanneeru.opensrc @ 2024-02-08 12:57 ` Jonathan Cameron 2024-02-12 5:53 ` [EXT] " Srinivasulu Opensrc 2024-02-14 16:09 ` Srinivasulu Opensrc 0 siblings, 2 replies; 8+ messages in thread From: Jonathan Cameron @ 2024-02-08 12:57 UTC (permalink / raw) To: sthanneeru.opensrc Cc: linux-cxl, linux-mm, dan.j.williams, john, emirakhur, ajayjoshi, Ravis.OpenSrc, sthanneeru, shiju.jose On Wed, 7 Feb 2024 16:06:34 +0530 <sthanneeru.opensrc@micron.com> wrote: > From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> > > Adding UAPI support for > 1. CXL r3.1 8.2.9.5.3 Get Log Capabilities. > 2. CXL r3.1 8.2.9.5.4 Clear Log commands. > 3. CXL r3.1 8.2.9.5.6 Get Supported Logs Sub-List. > > Signed-off-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> Hi Srinivasulu, Whilst I can conjecture some valid reasons to expose these to userspace, can you add some examples to this patch description? We might want to filter the clear in particular to avoid a clash with the driver log handling. That is only allow it for vendor logs. Perhaps split the patch into 2 parts. The less controversial GET_LOG_CAPS and GET_LOG_SUBLIST, followed by a patch for the destructive clear log. The memory scrub handling might well access the ECS log for example (I don't think the current proposal yet does this). Jonathan > --- > drivers/cxl/core/mbox.c | 3 +++ > drivers/cxl/cxlmem.h | 3 +++ > include/uapi/linux/cxl_mem.h | 3 +++ > 3 files changed, 9 insertions(+) > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > index 27166a411705..64a44e286488 100644 > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > @@ -64,6 +64,9 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = { > CXL_CMD(SET_SHUTDOWN_STATE, 0x1, 0, 0), > CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0), > CXL_CMD(GET_TIMESTAMP, 0, 0x8, 0), > + CXL_CMD(GET_LOG_CAPS, 0x10, 0x4, 0), > + CXL_CMD(CLEAR_LOG, 0x10, 0, 0), > + CXL_CMD(GET_LOG_SUBLIST, 0x2, CXL_VARIABLE_PAYLOAD, 0), > }; > > /* > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > index 5303d6942b88..4128c810051c 100644 > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -529,6 +529,9 @@ enum cxl_opcode { > CXL_MBOX_OP_SET_TIMESTAMP = 0x0301, > CXL_MBOX_OP_GET_SUPPORTED_LOGS = 0x0400, > CXL_MBOX_OP_GET_LOG = 0x0401, > + CXL_MBOX_OP_GET_LOG_CAPS = 0x0402, > + CXL_MBOX_OP_CLEAR_LOG = 0x0403, > + CXL_MBOX_OP_GET_LOG_SUBLIST = 0x0405, Name should include something to make it clear this is getting sublist of 'supported' logs. Not the log. > CXL_MBOX_OP_IDENTIFY = 0x4000, > CXL_MBOX_OP_GET_PARTITION_INFO = 0x4100, > CXL_MBOX_OP_SET_PARTITION_INFO = 0x4101, > diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h > index 42066f4eb890..d2df9782a5ef 100644 > --- a/include/uapi/linux/cxl_mem.h > +++ b/include/uapi/linux/cxl_mem.h > @@ -47,6 +47,9 @@ > ___DEPRECATED(SCAN_MEDIA, "Scan Media"), \ > ___DEPRECATED(GET_SCAN_MEDIA, "Get Scan Media Results"), \ > ___C(GET_TIMESTAMP, "Get Timestamp"), \ > + ___C(GET_LOG_CAPS, "Get Log Capabilities"), \ > + ___C(CLEAR_LOG, "Clear Log"), \ > + ___C(GET_LOG_SUBLIST, "Get Log Sublist"), \ Likewise, mention it's list of supported logs. > ___C(MAX, "invalid / last command") > > #define ___C(a, b) CXL_MEM_COMMAND_ID_##a ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and Get Supported Logs Sub-List commands 2024-02-08 12:57 ` Jonathan Cameron @ 2024-02-12 5:53 ` Srinivasulu Opensrc 2024-02-14 16:09 ` Srinivasulu Opensrc 1 sibling, 0 replies; 8+ messages in thread From: Srinivasulu Opensrc @ 2024-02-12 5:53 UTC (permalink / raw) To: Jonathan Cameron Cc: linux-cxl, linux-mm, dan.j.williams, john, Eishan Mirakhur, Ajay Joshi, Ravis OpenSrc, Srinivasulu Thanneeru, shiju.jose Hi Jonathan, Thank you, for the inputs, please find comments inline. Regards, Srini > -----Original Message----- > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > Sent: Thursday, February 8, 2024 6:27 PM > To: Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> > Cc: linux-cxl@vger.kernel.org; linux-mm@kvack.org; > dan.j.williams@intel.com; john@jagalactic.com; Eishan Mirakhur > <emirakhur@micron.com>; Ajay Joshi <ajayjoshi@micron.com>; Ravis > OpenSrc <Ravis.OpenSrc@micron.com>; Srinivasulu Thanneeru > <sthanneeru@micron.com>; shiju.jose@huawei.com > Subject: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and > Get Supported Logs Sub-List commands > > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless > you recognize the sender and were expecting this message. > > > On Wed, 7 Feb 2024 16:06:34 +0530 > <sthanneeru.opensrc@micron.com> wrote: > > > From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> > > > > Adding UAPI support for > > 1. CXL r3.1 8.2.9.5.3 Get Log Capabilities. > > 2. CXL r3.1 8.2.9.5.4 Clear Log commands. > > 3. CXL r3.1 8.2.9.5.6 Get Supported Logs Sub-List. > > > > Signed-off-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> > > Hi Srinivasulu, > > Whilst I can conjecture some valid reasons to expose these to > userspace, can you add some examples to this patch description? > Sure, I will add description and send V2. > We might want to filter the clear in particular to avoid a clash > with the driver log handling. That is only allow it for vendor > logs. > Allowing for both of the following. * 5e1819d9-11a9-400c-811f-d60719403d86 - Vendor Debug Log * b3fab4cf-01b6-4332-943e-5e9962f23567 - Component State Dump Log > Perhaps split the patch into 2 parts. The less controversial > GET_LOG_CAPS and GET_LOG_SUBLIST, followed by a patch for the > destructive clear log. Sure, I will split the patches as suggested. > > The memory scrub handling might well > access the ECS log for example (I don't think the current proposal > yet does this). > > Jonathan > > > > --- > > drivers/cxl/core/mbox.c | 3 +++ > > drivers/cxl/cxlmem.h | 3 +++ > > include/uapi/linux/cxl_mem.h | 3 +++ > > 3 files changed, 9 insertions(+) > > > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > > index 27166a411705..64a44e286488 100644 > > --- a/drivers/cxl/core/mbox.c > > +++ b/drivers/cxl/core/mbox.c > > @@ -64,6 +64,9 @@ static struct cxl_mem_command > cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = { > > CXL_CMD(SET_SHUTDOWN_STATE, 0x1, 0, 0), > > CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0), > > CXL_CMD(GET_TIMESTAMP, 0, 0x8, 0), > > + CXL_CMD(GET_LOG_CAPS, 0x10, 0x4, 0), > > + CXL_CMD(CLEAR_LOG, 0x10, 0, 0), > > + CXL_CMD(GET_LOG_SUBLIST, 0x2, CXL_VARIABLE_PAYLOAD, 0), > > }; > > > > /* > > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > > index 5303d6942b88..4128c810051c 100644 > > --- a/drivers/cxl/cxlmem.h > > +++ b/drivers/cxl/cxlmem.h > > @@ -529,6 +529,9 @@ enum cxl_opcode { > > CXL_MBOX_OP_SET_TIMESTAMP = 0x0301, > > CXL_MBOX_OP_GET_SUPPORTED_LOGS = 0x0400, > > CXL_MBOX_OP_GET_LOG = 0x0401, > > + CXL_MBOX_OP_GET_LOG_CAPS = 0x0402, > > + CXL_MBOX_OP_CLEAR_LOG = 0x0403, > > + CXL_MBOX_OP_GET_LOG_SUBLIST = 0x0405, > > Name should include something to make it clear this is getting > sublist of 'supported' logs. Not the log. > > > CXL_MBOX_OP_IDENTIFY = 0x4000, > > CXL_MBOX_OP_GET_PARTITION_INFO = 0x4100, > > CXL_MBOX_OP_SET_PARTITION_INFO = 0x4101, > > diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h > > index 42066f4eb890..d2df9782a5ef 100644 > > --- a/include/uapi/linux/cxl_mem.h > > +++ b/include/uapi/linux/cxl_mem.h > > @@ -47,6 +47,9 @@ > > ___DEPRECATED(SCAN_MEDIA, "Scan Media"), \ > > ___DEPRECATED(GET_SCAN_MEDIA, "Get Scan Media Results"), \ > > ___C(GET_TIMESTAMP, "Get Timestamp"), \ > > + ___C(GET_LOG_CAPS, "Get Log Capabilities"), \ > > + ___C(CLEAR_LOG, "Clear Log"), \ > > + ___C(GET_LOG_SUBLIST, "Get Log Sublist"), \ > > Likewise, mention it's list of supported logs. Sure, will update "supported log" in all places in the next version V2. -- Srini > > > ___C(MAX, "invalid / last command") > > > > #define ___C(a, b) CXL_MEM_COMMAND_ID_##a ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and Get Supported Logs Sub-List commands 2024-02-08 12:57 ` Jonathan Cameron 2024-02-12 5:53 ` [EXT] " Srinivasulu Opensrc @ 2024-02-14 16:09 ` Srinivasulu Opensrc 2024-02-15 6:37 ` Srinivasulu Opensrc 2024-02-15 12:26 ` Jonathan Cameron 1 sibling, 2 replies; 8+ messages in thread From: Srinivasulu Opensrc @ 2024-02-14 16:09 UTC (permalink / raw) To: Jonathan Cameron Cc: linux-cxl, linux-mm, dan.j.williams, john, Eishan Mirakhur, Ajay Joshi, Ravis OpenSrc, Srinivasulu Thanneeru, shiju.jose > -----Original Message----- > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > Sent: Thursday, February 8, 2024 6:27 PM > To: Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> > Cc: linux-cxl@vger.kernel.org; linux-mm@kvack.org; > dan.j.williams@intel.com; john@jagalactic.com; Eishan Mirakhur > <emirakhur@micron.com>; Ajay Joshi <ajayjoshi@micron.com>; Ravis > OpenSrc <Ravis.OpenSrc@micron.com>; Srinivasulu Thanneeru > <sthanneeru@micron.com>; shiju.jose@huawei.com > Subject: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and > Get Supported Logs Sub-List commands > > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless > you recognize the sender and were expecting this message. > > > On Wed, 7 Feb 2024 16:06:34 +0530 > <sthanneeru.opensrc@micron.com> wrote: > > > From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> > > > > Adding UAPI support for > > 1. CXL r3.1 8.2.9.5.3 Get Log Capabilities. > > 2. CXL r3.1 8.2.9.5.4 Clear Log commands. > > 3. CXL r3.1 8.2.9.5.6 Get Supported Logs Sub-List. > > > > Signed-off-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> > > Hi Srinivasulu, > > Whilst I can conjecture some valid reasons to expose these to > userspace, can you add some examples to this patch description? > > We might want to filter the clear in particular to avoid a clash > with the driver log handling. That is only allow it for vendor > logs. Are you suggesting that I should restrict the "Vendor Debug Log" functionality to only apply to Vendor logs.? Why not include Component State Dumps logs that support the clear log feature? I might be overlooking something; please enlighten me. For example, following both supports Clear log in our setups. * 5e1819d9-11a9-400c-811f-d60719403d86 - Vendor Debug Log * b3fab4cf-01b6-4332-943e-5e9962f23567 - Component State Dump Log > > Perhaps split the patch into 2 parts. The less controversial > GET_LOG_CAPS and GET_LOG_SUBLIST, followed by a patch for the > destructive clear log. > > The memory scrub handling might well > access the ECS log for example (I don't think the current proposal > yet does this). > > Jonathan > > > > --- > > drivers/cxl/core/mbox.c | 3 +++ > > drivers/cxl/cxlmem.h | 3 +++ > > include/uapi/linux/cxl_mem.h | 3 +++ > > 3 files changed, 9 insertions(+) > > > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > > index 27166a411705..64a44e286488 100644 > > --- a/drivers/cxl/core/mbox.c > > +++ b/drivers/cxl/core/mbox.c > > @@ -64,6 +64,9 @@ static struct cxl_mem_command > cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = { > > CXL_CMD(SET_SHUTDOWN_STATE, 0x1, 0, 0), > > CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0), > > CXL_CMD(GET_TIMESTAMP, 0, 0x8, 0), > > + CXL_CMD(GET_LOG_CAPS, 0x10, 0x4, 0), > > + CXL_CMD(CLEAR_LOG, 0x10, 0, 0), > > + CXL_CMD(GET_LOG_SUBLIST, 0x2, CXL_VARIABLE_PAYLOAD, 0), > > }; > > > > /* > > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > > index 5303d6942b88..4128c810051c 100644 > > --- a/drivers/cxl/cxlmem.h > > +++ b/drivers/cxl/cxlmem.h > > @@ -529,6 +529,9 @@ enum cxl_opcode { > > CXL_MBOX_OP_SET_TIMESTAMP = 0x0301, > > CXL_MBOX_OP_GET_SUPPORTED_LOGS = 0x0400, > > CXL_MBOX_OP_GET_LOG = 0x0401, > > + CXL_MBOX_OP_GET_LOG_CAPS = 0x0402, > > + CXL_MBOX_OP_CLEAR_LOG = 0x0403, > > + CXL_MBOX_OP_GET_LOG_SUBLIST = 0x0405, > > Name should include something to make it clear this is getting > sublist of 'supported' logs. Not the log. > > > CXL_MBOX_OP_IDENTIFY = 0x4000, > > CXL_MBOX_OP_GET_PARTITION_INFO = 0x4100, > > CXL_MBOX_OP_SET_PARTITION_INFO = 0x4101, > > diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h > > index 42066f4eb890..d2df9782a5ef 100644 > > --- a/include/uapi/linux/cxl_mem.h > > +++ b/include/uapi/linux/cxl_mem.h > > @@ -47,6 +47,9 @@ > > ___DEPRECATED(SCAN_MEDIA, "Scan Media"), \ > > ___DEPRECATED(GET_SCAN_MEDIA, "Get Scan Media Results"), \ > > ___C(GET_TIMESTAMP, "Get Timestamp"), \ > > + ___C(GET_LOG_CAPS, "Get Log Capabilities"), \ > > + ___C(CLEAR_LOG, "Clear Log"), \ > > + ___C(GET_LOG_SUBLIST, "Get Log Sublist"), \ > > Likewise, mention it's list of supported logs. > > > ___C(MAX, "invalid / last command") > > > > #define ___C(a, b) CXL_MEM_COMMAND_ID_##a ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and Get Supported Logs Sub-List commands 2024-02-14 16:09 ` Srinivasulu Opensrc @ 2024-02-15 6:37 ` Srinivasulu Opensrc 2024-02-15 12:26 ` Jonathan Cameron 1 sibling, 0 replies; 8+ messages in thread From: Srinivasulu Opensrc @ 2024-02-15 6:37 UTC (permalink / raw) To: Jonathan Cameron Cc: linux-cxl, linux-mm, dan.j.williams, john, Eishan Mirakhur, Ajay Joshi, Ravis OpenSrc, Srinivasulu Thanneeru, shiju.jose > -----Original Message----- > From: Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> > Sent: Wednesday, February 14, 2024 9:39 PM > To: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > Cc: linux-cxl@vger.kernel.org; linux-mm@kvack.org; > dan.j.williams@intel.com; john@jagalactic.com; Eishan Mirakhur > <emirakhur@micron.com>; Ajay Joshi <ajayjoshi@micron.com>; Ravis > OpenSrc <Ravis.OpenSrc@micron.com>; Srinivasulu Thanneeru > <sthanneeru@micron.com>; shiju.jose@huawei.com > Subject: RE: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log > and Get Supported Logs Sub-List commands > > > > > -----Original Message----- > > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > > Sent: Thursday, February 8, 2024 6:27 PM > > To: Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> > > Cc: linux-cxl@vger.kernel.org; linux-mm@kvack.org; > > dan.j.williams@intel.com; john@jagalactic.com; Eishan Mirakhur > > <emirakhur@micron.com>; Ajay Joshi <ajayjoshi@micron.com>; Ravis > > OpenSrc <Ravis.OpenSrc@micron.com>; Srinivasulu Thanneeru > > <sthanneeru@micron.com>; shiju.jose@huawei.com > > Subject: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and > > Get Supported Logs Sub-List commands > > > > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless > > you recognize the sender and were expecting this message. > > > > > > On Wed, 7 Feb 2024 16:06:34 +0530 > > <sthanneeru.opensrc@micron.com> wrote: > > > > > From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> > > > > > > Adding UAPI support for > > > 1. CXL r3.1 8.2.9.5.3 Get Log Capabilities. > > > 2. CXL r3.1 8.2.9.5.4 Clear Log commands. > > > 3. CXL r3.1 8.2.9.5.6 Get Supported Logs Sub-List. > > > > > > Signed-off-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> > > > > Hi Srinivasulu, > > > > Whilst I can conjecture some valid reasons to expose these to > > userspace, can you add some examples to this patch description? > > > > We might want to filter the clear in particular to avoid a clash > > with the driver log handling. That is only allow it for vendor > > logs. > > Are you suggesting that I should restrict the "Vendor Debug Log" functionality > to > only apply to Vendor logs.? Why not include Component State Dumps logs > that > support the clear log feature? Sorry for the typo. Are you suggesting that I should restrict the "Clear log command" functionality from user space to only apply to "Vendor Debug Log".? Why not include Component State Dumps logs that support the clear log feature? > > I might be overlooking something; please enlighten me. > > For example, following both supports Clear log in our setups. > * 5e1819d9-11a9-400c-811f-d60719403d86 - Vendor Debug Log > * b3fab4cf-01b6-4332-943e-5e9962f23567 - Component State Dump Log > > > > > Perhaps split the patch into 2 parts. The less controversial > > GET_LOG_CAPS and GET_LOG_SUBLIST, followed by a patch for the > > destructive clear log. > > > > The memory scrub handling might well > > access the ECS log for example (I don't think the current proposal > > yet does this). > > > > Jonathan > > > > > > > --- > > > drivers/cxl/core/mbox.c | 3 +++ > > > drivers/cxl/cxlmem.h | 3 +++ > > > include/uapi/linux/cxl_mem.h | 3 +++ > > > 3 files changed, 9 insertions(+) > > > > > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > > > index 27166a411705..64a44e286488 100644 > > > --- a/drivers/cxl/core/mbox.c > > > +++ b/drivers/cxl/core/mbox.c > > > @@ -64,6 +64,9 @@ static struct cxl_mem_command > > cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = { > > > CXL_CMD(SET_SHUTDOWN_STATE, 0x1, 0, 0), > > > CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0), > > > CXL_CMD(GET_TIMESTAMP, 0, 0x8, 0), > > > + CXL_CMD(GET_LOG_CAPS, 0x10, 0x4, 0), > > > + CXL_CMD(CLEAR_LOG, 0x10, 0, 0), > > > + CXL_CMD(GET_LOG_SUBLIST, 0x2, CXL_VARIABLE_PAYLOAD, 0), > > > }; > > > > > > /* > > > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > > > index 5303d6942b88..4128c810051c 100644 > > > --- a/drivers/cxl/cxlmem.h > > > +++ b/drivers/cxl/cxlmem.h > > > @@ -529,6 +529,9 @@ enum cxl_opcode { > > > CXL_MBOX_OP_SET_TIMESTAMP = 0x0301, > > > CXL_MBOX_OP_GET_SUPPORTED_LOGS = 0x0400, > > > CXL_MBOX_OP_GET_LOG = 0x0401, > > > + CXL_MBOX_OP_GET_LOG_CAPS = 0x0402, > > > + CXL_MBOX_OP_CLEAR_LOG = 0x0403, > > > + CXL_MBOX_OP_GET_LOG_SUBLIST = 0x0405, > > > > Name should include something to make it clear this is getting > > sublist of 'supported' logs. Not the log. > > > > > CXL_MBOX_OP_IDENTIFY = 0x4000, > > > CXL_MBOX_OP_GET_PARTITION_INFO = 0x4100, > > > CXL_MBOX_OP_SET_PARTITION_INFO = 0x4101, > > > diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h > > > index 42066f4eb890..d2df9782a5ef 100644 > > > --- a/include/uapi/linux/cxl_mem.h > > > +++ b/include/uapi/linux/cxl_mem.h > > > @@ -47,6 +47,9 @@ > > > ___DEPRECATED(SCAN_MEDIA, "Scan Media"), \ > > > ___DEPRECATED(GET_SCAN_MEDIA, "Get Scan Media Results"), \ > > > ___C(GET_TIMESTAMP, "Get Timestamp"), \ > > > + ___C(GET_LOG_CAPS, "Get Log Capabilities"), \ > > > + ___C(CLEAR_LOG, "Clear Log"), \ > > > + ___C(GET_LOG_SUBLIST, "Get Log Sublist"), \ > > > > Likewise, mention it's list of supported logs. > > > > > ___C(MAX, "invalid / last command") > > > > > > #define ___C(a, b) CXL_MEM_COMMAND_ID_##a > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and Get Supported Logs Sub-List commands 2024-02-14 16:09 ` Srinivasulu Opensrc 2024-02-15 6:37 ` Srinivasulu Opensrc @ 2024-02-15 12:26 ` Jonathan Cameron 2024-02-19 17:32 ` Srinivasulu Opensrc 1 sibling, 1 reply; 8+ messages in thread From: Jonathan Cameron @ 2024-02-15 12:26 UTC (permalink / raw) To: Srinivasulu Opensrc Cc: linux-cxl, linux-mm, dan.j.williams, john, Eishan Mirakhur, Ajay Joshi, Ravis OpenSrc, Srinivasulu Thanneeru, shiju.jose On Wed, 14 Feb 2024 16:09:01 +0000 Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> wrote: > > -----Original Message----- > > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > > Sent: Thursday, February 8, 2024 6:27 PM > > To: Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> > > Cc: linux-cxl@vger.kernel.org; linux-mm@kvack.org; > > dan.j.williams@intel.com; john@jagalactic.com; Eishan Mirakhur > > <emirakhur@micron.com>; Ajay Joshi <ajayjoshi@micron.com>; Ravis > > OpenSrc <Ravis.OpenSrc@micron.com>; Srinivasulu Thanneeru > > <sthanneeru@micron.com>; shiju.jose@huawei.com > > Subject: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and > > Get Supported Logs Sub-List commands > > > > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless > > you recognize the sender and were expecting this message. > > > > > > On Wed, 7 Feb 2024 16:06:34 +0530 > > <sthanneeru.opensrc@micron.com> wrote: > > > > > From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> > > > > > > Adding UAPI support for > > > 1. CXL r3.1 8.2.9.5.3 Get Log Capabilities. > > > 2. CXL r3.1 8.2.9.5.4 Clear Log commands. > > > 3. CXL r3.1 8.2.9.5.6 Get Supported Logs Sub-List. > > > > > > Signed-off-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> > > > > Hi Srinivasulu, > > > > Whilst I can conjecture some valid reasons to expose these to > > userspace, can you add some examples to this patch description? > > > > We might want to filter the clear in particular to avoid a clash > > with the driver log handling. That is only allow it for vendor > > logs. > > Are you suggesting that I should restrict the "Vendor Debug Log" functionality to > only apply to Vendor logs.? Why not include Component State Dumps logs that > support the clear log feature? > > I might be overlooking something; please enlighten me. I had an old copy of the spec open. Indeed component state dumps should be safe to expose as well. > > For example, following both supports Clear log in our setups. > * 5e1819d9-11a9-400c-811f-d60719403d86 - Vendor Debug Log > * b3fab4cf-01b6-4332-943e-5e9962f23567 - Component State Dump Log > > > > > Perhaps split the patch into 2 parts. The less controversial > > GET_LOG_CAPS and GET_LOG_SUBLIST, followed by a patch for the > > destructive clear log. > > > > The memory scrub handling might well > > access the ECS log for example (I don't think the current proposal > > yet does this). > > > > Jonathan > > > > > > > --- > > > drivers/cxl/core/mbox.c | 3 +++ > > > drivers/cxl/cxlmem.h | 3 +++ > > > include/uapi/linux/cxl_mem.h | 3 +++ > > > 3 files changed, 9 insertions(+) > > > > > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > > > index 27166a411705..64a44e286488 100644 > > > --- a/drivers/cxl/core/mbox.c > > > +++ b/drivers/cxl/core/mbox.c > > > @@ -64,6 +64,9 @@ static struct cxl_mem_command > > cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = { > > > CXL_CMD(SET_SHUTDOWN_STATE, 0x1, 0, 0), > > > CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0), > > > CXL_CMD(GET_TIMESTAMP, 0, 0x8, 0), > > > + CXL_CMD(GET_LOG_CAPS, 0x10, 0x4, 0), > > > + CXL_CMD(CLEAR_LOG, 0x10, 0, 0), > > > + CXL_CMD(GET_LOG_SUBLIST, 0x2, CXL_VARIABLE_PAYLOAD, 0), > > > }; > > > > > > /* > > > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > > > index 5303d6942b88..4128c810051c 100644 > > > --- a/drivers/cxl/cxlmem.h > > > +++ b/drivers/cxl/cxlmem.h > > > @@ -529,6 +529,9 @@ enum cxl_opcode { > > > CXL_MBOX_OP_SET_TIMESTAMP = 0x0301, > > > CXL_MBOX_OP_GET_SUPPORTED_LOGS = 0x0400, > > > CXL_MBOX_OP_GET_LOG = 0x0401, > > > + CXL_MBOX_OP_GET_LOG_CAPS = 0x0402, > > > + CXL_MBOX_OP_CLEAR_LOG = 0x0403, > > > + CXL_MBOX_OP_GET_LOG_SUBLIST = 0x0405, > > > > Name should include something to make it clear this is getting > > sublist of 'supported' logs. Not the log. > > > > > CXL_MBOX_OP_IDENTIFY = 0x4000, > > > CXL_MBOX_OP_GET_PARTITION_INFO = 0x4100, > > > CXL_MBOX_OP_SET_PARTITION_INFO = 0x4101, > > > diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h > > > index 42066f4eb890..d2df9782a5ef 100644 > > > --- a/include/uapi/linux/cxl_mem.h > > > +++ b/include/uapi/linux/cxl_mem.h > > > @@ -47,6 +47,9 @@ > > > ___DEPRECATED(SCAN_MEDIA, "Scan Media"), \ > > > ___DEPRECATED(GET_SCAN_MEDIA, "Get Scan Media Results"), \ > > > ___C(GET_TIMESTAMP, "Get Timestamp"), \ > > > + ___C(GET_LOG_CAPS, "Get Log Capabilities"), \ > > > + ___C(CLEAR_LOG, "Clear Log"), \ > > > + ___C(GET_LOG_SUBLIST, "Get Log Sublist"), \ > > > > Likewise, mention it's list of supported logs. > > > > > ___C(MAX, "invalid / last command") > > > > > > #define ___C(a, b) CXL_MEM_COMMAND_ID_##a > ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and Get Supported Logs Sub-List commands 2024-02-15 12:26 ` Jonathan Cameron @ 2024-02-19 17:32 ` Srinivasulu Opensrc 2024-02-19 17:40 ` Jonathan Cameron 0 siblings, 1 reply; 8+ messages in thread From: Srinivasulu Opensrc @ 2024-02-19 17:32 UTC (permalink / raw) To: Jonathan Cameron Cc: linux-cxl, linux-mm, dan.j.williams, john, Eishan Mirakhur, Ajay Joshi, Ravis OpenSrc, Srinivasulu Thanneeru, shiju.jose > -----Original Message----- > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > Sent: Thursday, February 15, 2024 5:57 PM > To: Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> > Cc: linux-cxl@vger.kernel.org; linux-mm@kvack.org; > dan.j.williams@intel.com; john@jagalactic.com; Eishan Mirakhur > <emirakhur@micron.com>; Ajay Joshi <ajayjoshi@micron.com>; Ravis > OpenSrc <Ravis.OpenSrc@micron.com>; Srinivasulu Thanneeru > <sthanneeru@micron.com>; shiju.jose@huawei.com > Subject: Re: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log > and Get Supported Logs Sub-List commands > > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless > you recognize the sender and were expecting this message. > > > On Wed, 14 Feb 2024 16:09:01 +0000 > Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> wrote: > > > > -----Original Message----- > > > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > > > Sent: Thursday, February 8, 2024 6:27 PM > > > To: Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> > > > Cc: linux-cxl@vger.kernel.org; linux-mm@kvack.org; > > > dan.j.williams@intel.com; john@jagalactic.com; Eishan Mirakhur > > > <emirakhur@micron.com>; Ajay Joshi <ajayjoshi@micron.com>; Ravis > > > OpenSrc <Ravis.OpenSrc@micron.com>; Srinivasulu Thanneeru > > > <sthanneeru@micron.com>; shiju.jose@huawei.com > > > Subject: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log > and > > > Get Supported Logs Sub-List commands > > > > > > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless > > > you recognize the sender and were expecting this message. > > > > > > > > > On Wed, 7 Feb 2024 16:06:34 +0530 > > > <sthanneeru.opensrc@micron.com> wrote: > > > > > > > From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> > > > > > > > > Adding UAPI support for > > > > 1. CXL r3.1 8.2.9.5.3 Get Log Capabilities. > > > > 2. CXL r3.1 8.2.9.5.4 Clear Log commands. > > > > 3. CXL r3.1 8.2.9.5.6 Get Supported Logs Sub-List. > > > > > > > > Signed-off-by: Srinivasulu Thanneeru > <sthanneeru.opensrc@micron.com> > > > > > > Hi Srinivasulu, > > > > > > Whilst I can conjecture some valid reasons to expose these to > > > userspace, can you add some examples to this patch description? > > > > > > We might want to filter the clear in particular to avoid a clash > > > with the driver log handling. That is only allow it for vendor > > > logs. > > > > Are you suggesting that I should restrict the "Vendor Debug Log" > functionality to > > only apply to Vendor logs.? Why not include Component State Dumps logs > that > > support the clear log feature? > > > > I might be overlooking something; please enlighten me. > > I had an old copy of the spec open. Indeed component state dumps > should be safe to expose as well. Thank you, Jonathan. I'm considering adding the following snippet to the V2 patch for "clear log" filtering. Any initial feedback would be appreciated. -------------- --- a/drivers/cxl/core/mbox.c +++ b/drivers/cxl/core/mbox.c enum { CEL_UUID, VENDOR_DEBUG_UUID, + COMPONENT_STATE_DUMP_UUID, }; /* See CXL 2.0 Table 170. Get Log Input Payload */ static const uuid_t log_uuid[] = { [CEL_UUID] = DEFINE_CXL_CEL_UUID, [VENDOR_DEBUG_UUID] = DEFINE_CXL_VENDOR_DEBUG_UUID, + [COMPONENT_STATE_DUMP_UUID] = DEFINE_CXL_COMPONENT_STATE_DUMP_UUID, }; /* @@ -333,6 +346,12 @@ static bool cxl_payload_from_user_allowed(u16 opcode, void *payload_in) return false; break; } + + case CXL_MBOX_OP_CLEAR_LOG: { + const uuid_t *uuid = (uuid_t *)payload_in; + return (uuid_equal(uuid, &log_uuid[VENDOR_DEBUG_UUID])) || + (uuid_equal(uuid, &log_uuid[COMPONENT_STATE_DUMP_UUID])) + } } diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h --- a/drivers/cxl/cxlmem.h +++ b/drivers/cxl/cxlmem.h @@ -565,6 +565,9 @@ enum cxl_opcode { #define DEFINE_CXL_VENDOR_DEBUG_UUID \ UUID_INIT(0xe1819d9, 0x11a9, 0x400c, 0x81, 0x1f, 0xd6, 0x07, 0x19, \ 0x40, 0x3d, 0x86) +#define DEFINE_CXL_COMPONENT_STATE_DUMP_UUID + UUID_INIT(0x3fab4cf, 0x01b6, 0x4332, 0x94, 0x3e, 0x5e, 0x99, 0x62, \ + 0xf2, 0x35, 0x67) -------------- > > > > For example, following both supports Clear log in our setups. > > * 5e1819d9-11a9-400c-811f-d60719403d86 - Vendor Debug Log > > * b3fab4cf-01b6-4332-943e-5e9962f23567 - Component State Dump Log > > > > > > > > Perhaps split the patch into 2 parts. The less controversial > > > GET_LOG_CAPS and GET_LOG_SUBLIST, followed by a patch for the > > > destructive clear log. > > > > > > The memory scrub handling might well > > > access the ECS log for example (I don't think the current proposal > > > yet does this). > > > > > > Jonathan > > > > > > > > > > --- > > > > drivers/cxl/core/mbox.c | 3 +++ > > > > drivers/cxl/cxlmem.h | 3 +++ > > > > include/uapi/linux/cxl_mem.h | 3 +++ > > > > 3 files changed, 9 insertions(+) > > > > > > > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > > > > index 27166a411705..64a44e286488 100644 > > > > --- a/drivers/cxl/core/mbox.c > > > > +++ b/drivers/cxl/core/mbox.c > > > > @@ -64,6 +64,9 @@ static struct cxl_mem_command > > > cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = { > > > > CXL_CMD(SET_SHUTDOWN_STATE, 0x1, 0, 0), > > > > CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0), > > > > CXL_CMD(GET_TIMESTAMP, 0, 0x8, 0), > > > > + CXL_CMD(GET_LOG_CAPS, 0x10, 0x4, 0), > > > > + CXL_CMD(CLEAR_LOG, 0x10, 0, 0), > > > > + CXL_CMD(GET_LOG_SUBLIST, 0x2, CXL_VARIABLE_PAYLOAD, 0), > > > > }; > > > > > > > > /* > > > > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > > > > index 5303d6942b88..4128c810051c 100644 > > > > --- a/drivers/cxl/cxlmem.h > > > > +++ b/drivers/cxl/cxlmem.h > > > > @@ -529,6 +529,9 @@ enum cxl_opcode { > > > > CXL_MBOX_OP_SET_TIMESTAMP = 0x0301, > > > > CXL_MBOX_OP_GET_SUPPORTED_LOGS = 0x0400, > > > > CXL_MBOX_OP_GET_LOG = 0x0401, > > > > + CXL_MBOX_OP_GET_LOG_CAPS = 0x0402, > > > > + CXL_MBOX_OP_CLEAR_LOG = 0x0403, > > > > + CXL_MBOX_OP_GET_LOG_SUBLIST = 0x0405, > > > > > > Name should include something to make it clear this is getting > > > sublist of 'supported' logs. Not the log. > > > > > > > CXL_MBOX_OP_IDENTIFY = 0x4000, > > > > CXL_MBOX_OP_GET_PARTITION_INFO = 0x4100, > > > > CXL_MBOX_OP_SET_PARTITION_INFO = 0x4101, > > > > diff --git a/include/uapi/linux/cxl_mem.h > b/include/uapi/linux/cxl_mem.h > > > > index 42066f4eb890..d2df9782a5ef 100644 > > > > --- a/include/uapi/linux/cxl_mem.h > > > > +++ b/include/uapi/linux/cxl_mem.h > > > > @@ -47,6 +47,9 @@ > > > > ___DEPRECATED(SCAN_MEDIA, "Scan Media"), \ > > > > ___DEPRECATED(GET_SCAN_MEDIA, "Get Scan Media Results"), > \ > > > > ___C(GET_TIMESTAMP, "Get Timestamp"), \ > > > > + ___C(GET_LOG_CAPS, "Get Log Capabilities"), \ > > > > + ___C(CLEAR_LOG, "Clear Log"), \ > > > > + ___C(GET_LOG_SUBLIST, "Get Log Sublist"), \ > > > > > > Likewise, mention it's list of supported logs. > > > > > > > ___C(MAX, "invalid / last command") > > > > > > > > #define ___C(a, b) CXL_MEM_COMMAND_ID_##a > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and Get Supported Logs Sub-List commands 2024-02-19 17:32 ` Srinivasulu Opensrc @ 2024-02-19 17:40 ` Jonathan Cameron 0 siblings, 0 replies; 8+ messages in thread From: Jonathan Cameron @ 2024-02-19 17:40 UTC (permalink / raw) To: Srinivasulu Opensrc Cc: linux-cxl, linux-mm, dan.j.williams, john, Eishan Mirakhur, Ajay Joshi, Ravis OpenSrc, Srinivasulu Thanneeru, shiju.jose On Mon, 19 Feb 2024 17:32:25 +0000 Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> wrote: > > -----Original Message----- > > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > > Sent: Thursday, February 15, 2024 5:57 PM > > To: Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> > > Cc: linux-cxl@vger.kernel.org; linux-mm@kvack.org; > > dan.j.williams@intel.com; john@jagalactic.com; Eishan Mirakhur > > <emirakhur@micron.com>; Ajay Joshi <ajayjoshi@micron.com>; Ravis > > OpenSrc <Ravis.OpenSrc@micron.com>; Srinivasulu Thanneeru > > <sthanneeru@micron.com>; shiju.jose@huawei.com > > Subject: Re: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log > > and Get Supported Logs Sub-List commands > > > > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless > > you recognize the sender and were expecting this message. > > > > > > On Wed, 14 Feb 2024 16:09:01 +0000 > > Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> wrote: > > > > > > -----Original Message----- > > > > From: Jonathan Cameron <Jonathan.Cameron@Huawei.com> > > > > Sent: Thursday, February 8, 2024 6:27 PM > > > > To: Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> > > > > Cc: linux-cxl@vger.kernel.org; linux-mm@kvack.org; > > > > dan.j.williams@intel.com; john@jagalactic.com; Eishan Mirakhur > > > > <emirakhur@micron.com>; Ajay Joshi <ajayjoshi@micron.com>; Ravis > > > > OpenSrc <Ravis.OpenSrc@micron.com>; Srinivasulu Thanneeru > > > > <sthanneeru@micron.com>; shiju.jose@huawei.com > > > > Subject: [EXT] Re: [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log > > and > > > > Get Supported Logs Sub-List commands > > > > > > > > CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless > > > > you recognize the sender and were expecting this message. > > > > > > > > > > > > On Wed, 7 Feb 2024 16:06:34 +0530 > > > > <sthanneeru.opensrc@micron.com> wrote: > > > > > > > > > From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com> > > > > > > > > > > Adding UAPI support for > > > > > 1. CXL r3.1 8.2.9.5.3 Get Log Capabilities. > > > > > 2. CXL r3.1 8.2.9.5.4 Clear Log commands. > > > > > 3. CXL r3.1 8.2.9.5.6 Get Supported Logs Sub-List. > > > > > > > > > > Signed-off-by: Srinivasulu Thanneeru > > <sthanneeru.opensrc@micron.com> > > > > > > > > Hi Srinivasulu, > > > > > > > > Whilst I can conjecture some valid reasons to expose these to > > > > userspace, can you add some examples to this patch description? > > > > > > > > We might want to filter the clear in particular to avoid a clash > > > > with the driver log handling. That is only allow it for vendor > > > > logs. > > > > > > Are you suggesting that I should restrict the "Vendor Debug Log" > > functionality to > > > only apply to Vendor logs.? Why not include Component State Dumps logs > > that > > > support the clear log feature? > > > > > > I might be overlooking something; please enlighten me. > > > > I had an old copy of the spec open. Indeed component state dumps > > should be safe to expose as well. > > Thank you, Jonathan. > I'm considering adding the following snippet to the V2 patch for "clear log" filtering. > Any initial feedback would be appreciated. That's definitely an improvement and should let the discussion move forwards. Jonathan > > -------------- > --- a/drivers/cxl/core/mbox.c > +++ b/drivers/cxl/core/mbox.c > enum { > CEL_UUID, > VENDOR_DEBUG_UUID, > + COMPONENT_STATE_DUMP_UUID, > }; > > /* See CXL 2.0 Table 170. Get Log Input Payload */ > static const uuid_t log_uuid[] = { > [CEL_UUID] = DEFINE_CXL_CEL_UUID, > [VENDOR_DEBUG_UUID] = DEFINE_CXL_VENDOR_DEBUG_UUID, > + [COMPONENT_STATE_DUMP_UUID] = DEFINE_CXL_COMPONENT_STATE_DUMP_UUID, > }; > > /* > @@ -333,6 +346,12 @@ static bool cxl_payload_from_user_allowed(u16 opcode, void *payload_in) > return false; > break; > } > + > + case CXL_MBOX_OP_CLEAR_LOG: { > + const uuid_t *uuid = (uuid_t *)payload_in; > + return (uuid_equal(uuid, &log_uuid[VENDOR_DEBUG_UUID])) || > + (uuid_equal(uuid, &log_uuid[COMPONENT_STATE_DUMP_UUID])) > + } > } > > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > --- a/drivers/cxl/cxlmem.h > +++ b/drivers/cxl/cxlmem.h > @@ -565,6 +565,9 @@ enum cxl_opcode { > #define DEFINE_CXL_VENDOR_DEBUG_UUID \ > UUID_INIT(0xe1819d9, 0x11a9, 0x400c, 0x81, 0x1f, 0xd6, 0x07, 0x19, \ > 0x40, 0x3d, 0x86) > +#define DEFINE_CXL_COMPONENT_STATE_DUMP_UUID > + UUID_INIT(0x3fab4cf, 0x01b6, 0x4332, 0x94, 0x3e, 0x5e, 0x99, 0x62, \ > + 0xf2, 0x35, 0x67) > -------------- > > > > > > For example, following both supports Clear log in our setups. > > > * 5e1819d9-11a9-400c-811f-d60719403d86 - Vendor Debug Log > > > * b3fab4cf-01b6-4332-943e-5e9962f23567 - Component State Dump Log > > > > > > > > > > > Perhaps split the patch into 2 parts. The less controversial > > > > GET_LOG_CAPS and GET_LOG_SUBLIST, followed by a patch for the > > > > destructive clear log. > > > > > > > > The memory scrub handling might well > > > > access the ECS log for example (I don't think the current proposal > > > > yet does this). > > > > > > > > Jonathan > > > > > > > > > > > > > --- > > > > > drivers/cxl/core/mbox.c | 3 +++ > > > > > drivers/cxl/cxlmem.h | 3 +++ > > > > > include/uapi/linux/cxl_mem.h | 3 +++ > > > > > 3 files changed, 9 insertions(+) > > > > > > > > > > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c > > > > > index 27166a411705..64a44e286488 100644 > > > > > --- a/drivers/cxl/core/mbox.c > > > > > +++ b/drivers/cxl/core/mbox.c > > > > > @@ -64,6 +64,9 @@ static struct cxl_mem_command > > > > cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = { > > > > > CXL_CMD(SET_SHUTDOWN_STATE, 0x1, 0, 0), > > > > > CXL_CMD(GET_SCAN_MEDIA_CAPS, 0x10, 0x4, 0), > > > > > CXL_CMD(GET_TIMESTAMP, 0, 0x8, 0), > > > > > + CXL_CMD(GET_LOG_CAPS, 0x10, 0x4, 0), > > > > > + CXL_CMD(CLEAR_LOG, 0x10, 0, 0), > > > > > + CXL_CMD(GET_LOG_SUBLIST, 0x2, CXL_VARIABLE_PAYLOAD, 0), > > > > > }; > > > > > > > > > > /* > > > > > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h > > > > > index 5303d6942b88..4128c810051c 100644 > > > > > --- a/drivers/cxl/cxlmem.h > > > > > +++ b/drivers/cxl/cxlmem.h > > > > > @@ -529,6 +529,9 @@ enum cxl_opcode { > > > > > CXL_MBOX_OP_SET_TIMESTAMP = 0x0301, > > > > > CXL_MBOX_OP_GET_SUPPORTED_LOGS = 0x0400, > > > > > CXL_MBOX_OP_GET_LOG = 0x0401, > > > > > + CXL_MBOX_OP_GET_LOG_CAPS = 0x0402, > > > > > + CXL_MBOX_OP_CLEAR_LOG = 0x0403, > > > > > + CXL_MBOX_OP_GET_LOG_SUBLIST = 0x0405, > > > > > > > > Name should include something to make it clear this is getting > > > > sublist of 'supported' logs. Not the log. > > > > > > > > > CXL_MBOX_OP_IDENTIFY = 0x4000, > > > > > CXL_MBOX_OP_GET_PARTITION_INFO = 0x4100, > > > > > CXL_MBOX_OP_SET_PARTITION_INFO = 0x4101, > > > > > diff --git a/include/uapi/linux/cxl_mem.h > > b/include/uapi/linux/cxl_mem.h > > > > > index 42066f4eb890..d2df9782a5ef 100644 > > > > > --- a/include/uapi/linux/cxl_mem.h > > > > > +++ b/include/uapi/linux/cxl_mem.h > > > > > @@ -47,6 +47,9 @@ > > > > > ___DEPRECATED(SCAN_MEDIA, "Scan Media"), \ > > > > > ___DEPRECATED(GET_SCAN_MEDIA, "Get Scan Media Results"), > > \ > > > > > ___C(GET_TIMESTAMP, "Get Timestamp"), \ > > > > > + ___C(GET_LOG_CAPS, "Get Log Capabilities"), \ > > > > > + ___C(CLEAR_LOG, "Clear Log"), \ > > > > > + ___C(GET_LOG_SUBLIST, "Get Log Sublist"), \ > > > > > > > > Likewise, mention it's list of supported logs. > > > > > > > > > ___C(MAX, "invalid / last command") > > > > > > > > > > #define ___C(a, b) CXL_MEM_COMMAND_ID_##a > > > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-02-19 17:40 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-02-07 10:36 [PATCH] cxl/mbox: Add Get Log Capabilities, Clear Log and Get Supported Logs Sub-List commands sthanneeru.opensrc 2024-02-08 12:57 ` Jonathan Cameron 2024-02-12 5:53 ` [EXT] " Srinivasulu Opensrc 2024-02-14 16:09 ` Srinivasulu Opensrc 2024-02-15 6:37 ` Srinivasulu Opensrc 2024-02-15 12:26 ` Jonathan Cameron 2024-02-19 17:32 ` Srinivasulu Opensrc 2024-02-19 17:40 ` Jonathan Cameron
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox