On Wed, 7 Feb 2024 16:06:34 +0530
<sthanneeru.opensrc@xxxxxxxxxx> wrote:
> From: Srinivasulu Thanneeru <sthanneeru.opensrc@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
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
