[PATCH v3 0/2] Add log related mailbox commands

[PATCH v3 0/2] Add log related mailbox commands

[sthanneeru.opensrc]

From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>

Add support to expose following mailbox commands to userspace
for clearing and populating the Vendor debug log in certain
scenarios, allowing for the aggregation of results over time.

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.

---
Changes in v3:
- 'Component State Dump log' has several caveats for ioctl()
   not being a suitable ABI as pointed in v2.(Dan Williams)
- Remove Component State Dump from Clear log filter.
- Implement a seperate patch(yet to do) to address issues as pointed in v2.
- Link to v2:
https://lore.kernel.org/linux-mm/65e631951ff61_b62b294e1@dwillia2-mobl3.amr.corp.intel.com.notmuch/

Changes in v2:
Suggested by Jonathan Cameron:
- Add description for exposing these mailbox log commands to ioctl.
- Create separate patch for 'Clear log'.
- Restrict the ‘Clear log’ action to only apply to
  Vendor debug logs and Component state dump logs.
- Rename get log sublist to get supported log sublist.
- Link to v1: https://lore.kernel.org/linux-mm/20240207103634.199-1-sthanneeru.opensrc@micron.com/
---

*** BLURB HERE ***

Srinivasulu Thanneeru (2):
  cxl/mbox: Add Get Log Capabilities and Get Supported Logs Sub-List
    commands
  cxl/mbox: Add Clear Log mailbox command

 drivers/cxl/core/mbox.c      | 12 ++++++++++++
 drivers/cxl/cxlmem.h         |  3 +++
 include/uapi/linux/cxl_mem.h |  3 +++
 3 files changed, 18 insertions(+)

-- 
2.25.1

[PATCH v3 1/2] cxl/mbox: Add Get Log Capabilities and Get Supported Logs Sub-List commands

[sthanneeru.opensrc]

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.6 Get Supported Logs Sub-List.

Signed-off-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
---
 drivers/cxl/core/mbox.c      | 2 ++
 drivers/cxl/cxlmem.h         | 2 ++
 include/uapi/linux/cxl_mem.h | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 27166a411705..30bd8264292f 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -56,6 +56,8 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
 	CXL_CMD(GET_LSA, 0x8, CXL_VARIABLE_PAYLOAD, 0),
 	CXL_CMD(GET_HEALTH_INFO, 0, 0x12, 0),
 	CXL_CMD(GET_LOG, 0x18, CXL_VARIABLE_PAYLOAD, CXL_CMD_FLAG_FORCE_ENABLE),
+	CXL_CMD(GET_LOG_CAPS, 0x10, 0x4, 0),
+	CXL_CMD(GET_SUP_LOG_SUBLIST, 0x2, CXL_VARIABLE_PAYLOAD, 0),
 	CXL_CMD(SET_PARTITION_INFO, 0x0a, 0, 0),
 	CXL_CMD(SET_LSA, CXL_VARIABLE_PAYLOAD, 0, 0),
 	CXL_CMD(GET_ALERT_CONFIG, 0, 0x10, 0),
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 5303d6942b88..671e46538baa 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -529,6 +529,8 @@ 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_GET_SUP_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..49c25056c222 100644
--- a/include/uapi/linux/cxl_mem.h
+++ b/include/uapi/linux/cxl_mem.h
@@ -47,6 +47,8 @@
 	___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(GET_SUP_LOG_SUBLIST, "Get Supported Logs Sub-List"),	  \
 	___C(MAX, "invalid / last command")
 
 #define ___C(a, b) CXL_MEM_COMMAND_ID_##a
-- 
2.25.1

Re: [PATCH v3 1/2] cxl/mbox: Add Get Log Capabilities and Get Supported Logs Sub-List commands

[Jonathan Cameron]

On Wed, 13 Mar 2024 12:42:17 +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.6 Get Supported Logs Sub-List.
> 
> Signed-off-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
Seems harmless to me and potentially useful.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/core/mbox.c      | 2 ++
>  drivers/cxl/cxlmem.h         | 2 ++
>  include/uapi/linux/cxl_mem.h | 2 ++
>  3 files changed, 6 insertions(+)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 27166a411705..30bd8264292f 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -56,6 +56,8 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
>  	CXL_CMD(GET_LSA, 0x8, CXL_VARIABLE_PAYLOAD, 0),
>  	CXL_CMD(GET_HEALTH_INFO, 0, 0x12, 0),
>  	CXL_CMD(GET_LOG, 0x18, CXL_VARIABLE_PAYLOAD, CXL_CMD_FLAG_FORCE_ENABLE),
> +	CXL_CMD(GET_LOG_CAPS, 0x10, 0x4, 0),
> +	CXL_CMD(GET_SUP_LOG_SUBLIST, 0x2, CXL_VARIABLE_PAYLOAD, 0),
>  	CXL_CMD(SET_PARTITION_INFO, 0x0a, 0, 0),
>  	CXL_CMD(SET_LSA, CXL_VARIABLE_PAYLOAD, 0, 0),
>  	CXL_CMD(GET_ALERT_CONFIG, 0, 0x10, 0),
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index 5303d6942b88..671e46538baa 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -529,6 +529,8 @@ 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_GET_SUP_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..49c25056c222 100644
> --- a/include/uapi/linux/cxl_mem.h
> +++ b/include/uapi/linux/cxl_mem.h
> @@ -47,6 +47,8 @@
>  	___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(GET_SUP_LOG_SUBLIST, "Get Supported Logs Sub-List"),	  \
>  	___C(MAX, "invalid / last command")
>  
>  #define ___C(a, b) CXL_MEM_COMMAND_ID_##a

[PATCH v3 2/2] cxl/mbox: Add Clear Log mailbox command

[sthanneeru.opensrc]

From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>

Adding UAPI support for CXL r3.1 8.2.9.5.4
Clear Log command.

This proposed patch will be useful for clearing and populating
the Vendor debug log in certain scenarios, allowing for the
aggregation of results over time.

Signed-off-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
---
 drivers/cxl/core/mbox.c      | 10 ++++++++++
 drivers/cxl/cxlmem.h         |  1 +
 include/uapi/linux/cxl_mem.h |  1 +
 3 files changed, 12 insertions(+)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 30bd8264292f..58437ae56140 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -57,6 +57,7 @@ static struct cxl_mem_command cxl_mem_commands[CXL_MEM_COMMAND_ID_MAX] = {
 	CXL_CMD(GET_HEALTH_INFO, 0, 0x12, 0),
 	CXL_CMD(GET_LOG, 0x18, CXL_VARIABLE_PAYLOAD, CXL_CMD_FLAG_FORCE_ENABLE),
 	CXL_CMD(GET_LOG_CAPS, 0x10, 0x4, 0),
+	CXL_CMD(CLEAR_LOG, 0x10, 0, 0),
 	CXL_CMD(GET_SUP_LOG_SUBLIST, 0x2, CXL_VARIABLE_PAYLOAD, 0),
 	CXL_CMD(SET_PARTITION_INFO, 0x0a, 0, 0),
 	CXL_CMD(SET_LSA, CXL_VARIABLE_PAYLOAD, 0, 0),
@@ -333,6 +334,15 @@ 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;
+
+		/*
+		 * Restrict the ‘Clear log’ action to only apply to
+		 * Vendor debug logs.
+		 */
+		return uuid_equal(uuid, &DEFINE_CXL_VENDOR_DEBUG_UUID);
+	}
 	default:
 		break;
 	}
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index 671e46538baa..11a5cd4f3ae1 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -530,6 +530,7 @@ enum cxl_opcode {
 	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_SUP_LOG_SUBLIST = 0x0405,
 	CXL_MBOX_OP_IDENTIFY		= 0x4000,
 	CXL_MBOX_OP_GET_PARTITION_INFO	= 0x4100,
diff --git a/include/uapi/linux/cxl_mem.h b/include/uapi/linux/cxl_mem.h
index 49c25056c222..c6c0fe27495d 100644
--- a/include/uapi/linux/cxl_mem.h
+++ b/include/uapi/linux/cxl_mem.h
@@ -48,6 +48,7 @@
 	___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_SUP_LOG_SUBLIST, "Get Supported Logs Sub-List"),	  \
 	___C(MAX, "invalid / last command")
 
-- 
2.25.1

Re: [PATCH v3 2/2] cxl/mbox: Add Clear Log mailbox command

[Jonathan Cameron]

On Wed, 13 Mar 2024 12:42:18 +0530
<sthanneeru.opensrc@micron.com> wrote:

> From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
> 
> Adding UAPI support for CXL r3.1 8.2.9.5.4
> Clear Log command.
> 
> This proposed patch will be useful for clearing and populating
> the Vendor debug log in certain scenarios, allowing for the
> aggregation of results over time.
> 
> Signed-off-by: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
Also harmless (with this restriction to vendor debug log) and useful.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

RE: [PATCH v3 0/2] Add log related mailbox commands

[Dan Williams]

sthanneeru.opensrc@ wrote:
> From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
> 
> Add support to expose following mailbox commands to userspace
> for clearing and populating the Vendor debug log in certain
> scenarios, allowing for the aggregation of results over time.
> 
> 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.
> 
> ---
> Changes in v3:
> - 'Component State Dump log' has several caveats for ioctl()
>    not being a suitable ABI as pointed in v2.(Dan Williams)
> - Remove Component State Dump from Clear log filter.
> - Implement a seperate patch(yet to do) to address issues as pointed in v2.

Circling back to this question... I had overlooked the fact that in v3.1
the "Request Abort Background Operation" command was added (8.2.9.1.5).
With that the kernel can safely support background commands with
indefinite residency. So as long as the device supports that command and
advertises that Log populate requests can be cancelled then we can build
a facility to cancel any user-submitted background commands when a
kernel internal need for the background command slot arises.

RE: [EXT] RE: [PATCH v3 0/2] Add log related mailbox commands

[Srinivasulu Opensrc]

>-----Original Message-----
>From: Dan Williams <dan.j.williams@intel.com>
>Sent: Wednesday, March 27, 2024 5:07 AM
>To: Srinivasulu Opensrc <sthanneeru.opensrc@micron.com>; linux-
>cxl@vger.kernel.org
>Cc: Jonathan.Cameron@huawei.com; 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>
>Subject: [EXT] RE: [PATCH v3 0/2] Add log related mailbox commands
>
>CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you
>recognize the sender and were expecting this message.
>
>
>sthanneeru.opensrc@ wrote:
>> From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
>>
>> Add support to expose following mailbox commands to userspace
>> for clearing and populating the Vendor debug log in certain
>> scenarios, allowing for the aggregation of results over time.
>>
>> 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.
>>
>> ---
>> Changes in v3:
>> - 'Component State Dump log' has several caveats for ioctl()
>>    not being a suitable ABI as pointed in v2.(Dan Williams)
>> - Remove Component State Dump from Clear log filter.
>> - Implement a seperate patch(yet to do) to address issues as pointed in v2.
>
>Circling back to this question... I had overlooked the fact that in v3.1
>the "Request Abort Background Operation" command was added (8.2.9.1.5).
>With that the kernel can safely support background commands with
>indefinite residency. So as long as the device supports that command and
>advertises that Log populate requests can be cancelled then we can build
>a facility to cancel any user-submitted background commands when a
>kernel internal need for the background command slot arises.

I don't have the access to test "Request Abort Background Operation."
Previously, we posted RFC for default time for background operations.
https://lore.kernel.org/linux-mm/20240207105349.301-1-sthanneeru.opensrc@micron.com/

 Could you please guide me on how to proceed with this current patch series?

Re: [EXT] RE: [PATCH v3 0/2] Add log related mailbox commands

[Jonathan Cameron]

On Mon, 1 Apr 2024 06:03:07 +0000
Srinivasulu Opensrc <sthanneeru.opensrc@micron.com> wrote:

> >-----Original Message-----
> >From: Dan Williams <dan.j.williams@intel.com>
> >Sent: Wednesday, March 27, 2024 5:07 AM
> >To: Srinivasulu Opensrc <sthanneeru.opensrc@micron.com>; linux-
> >cxl@vger.kernel.org
> >Cc: Jonathan.Cameron@huawei.com; 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>
> >Subject: [EXT] RE: [PATCH v3 0/2] Add log related mailbox commands
> >
> >CAUTION: EXTERNAL EMAIL. Do not click links or open attachments unless you
> >recognize the sender and were expecting this message.
> >
> >
> >sthanneeru.opensrc@ wrote:  
> >> From: Srinivasulu Thanneeru <sthanneeru.opensrc@micron.com>
> >>
> >> Add support to expose following mailbox commands to userspace
> >> for clearing and populating the Vendor debug log in certain
> >> scenarios, allowing for the aggregation of results over time.
> >>
> >> 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.
> >>
> >> ---
> >> Changes in v3:
> >> - 'Component State Dump log' has several caveats for ioctl()
> >>    not being a suitable ABI as pointed in v2.(Dan Williams)
> >> - Remove Component State Dump from Clear log filter.
> >> - Implement a seperate patch(yet to do) to address issues as pointed in v2.  
> >
> >Circling back to this question... I had overlooked the fact that in v3.1
> >the "Request Abort Background Operation" command was added (8.2.9.1.5).
> >With that the kernel can safely support background commands with
> >indefinite residency. So as long as the device supports that command and
> >advertises that Log populate requests can be cancelled then we can build
> >a facility to cancel any user-submitted background commands when a
> >kernel internal need for the background command slot arises.  
> 
> I don't have the access to test "Request Abort Background Operation."
> Previously, we posted RFC for default time for background operations.
> https://lore.kernel.org/linux-mm/20240207105349.301-1-sthanneeru.opensrc@micron.com/
> 
>  Could you please guide me on how to proceed with this current patch series?

Whilst QEMU emulation doesn't yet support background command aborting, it wouldn't
be that hard to add and would provide a route to test this functionality.
We've done similar in a few other cases where no one had any hardware yet.

Of course that might not help you if you have silicon that doesn't implement it
but I agree with Dan that it is a lot less problematic to allow for unbounded
background ops if we can stop them for other urgent activity.

Jonathan