Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>
Reviewed-by: Alok Rathore <alok.rathore@samsung.com>
Reviewed-by: Krishna Kanth Reddy <krish.reddy@samsung.com>
---
hw/cxl/cxl-mailbox-utils.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 3d66a425a9..5fd7f850c4 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -77,6 +77,7 @@ enum {
#define GET_SUPPORTED 0x0
#define GET_LOG 0x1
#define GET_LOG_CAPABILITIES 0x2
+ #define CLEAR_LOG 0x3
FEATURES = 0x05,
#define GET_SUPPORTED 0x0
#define GET_FEATURE 0x1
@@ -1115,6 +1116,39 @@ static CXLRetCode cmd_logs_get_log_capabilities(const struct cxl_cmd *cmd,
return CXL_MBOX_SUCCESS;
}
+/* CXL r3.1 Section 8.2.9.5.4: Clear Log (Opcode 0403h) */
+static CXLRetCode cmd_logs_clear_log(const struct cxl_cmd *cmd,
+ uint8_t *payload_in,
+ size_t len_in,
+ uint8_t *payload_out,
+ size_t *len_out,
+ CXLCCI *cci)
+{
+ int32_t cap_id;
+ struct {
+ QemuUUID uuid;
+ } QEMU_PACKED QEMU_ALIGNED(8) * clear_log = (void *)payload_in;
+
+ cap_id = valid_log_check(&clear_log->uuid, cci);
+ if (cap_id == -1) {
+ return CXL_MBOX_INVALID_LOG;
+ }
+
+ if (cci->supported_log_cap[cap_id].param_flags.clear_log_supported) {
+ switch (cap_id) {
+ case CEL:
+ memset(cci->cel_log, 0, (1 << 16) * sizeof(struct cel_log));
+ cci->cel_size = 0;
+ break;
+ default:
+ return CXL_MBOX_UNSUPPORTED;
+ }
+ } else {
+ return CXL_MBOX_UNSUPPORTED;
+ }
+ return CXL_MBOX_SUCCESS;
+}
+
/* CXL r3.1 section 8.2.9.6: Features */
/*
* Get Supported Features output payload
@@ -2882,6 +2916,8 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
[LOGS][GET_LOG] = { "LOGS_GET_LOG", cmd_logs_get_log, 0x18, 0 },
[LOGS][GET_LOG_CAPABILITIES] = { "LOGS_GET_LOG_CAPABILITIES",
cmd_logs_get_log_capabilities, 0x10, 0 },
+ [LOGS][CLEAR_LOG] = { "LOGS_CLEAR_LOG", cmd_logs_clear_log, 0x10,
+ CXL_MBOX_IMMEDIATE_LOG_CHANGE},
[FEATURES][GET_SUPPORTED] = { "FEATURES_GET_SUPPORTED",
cmd_features_get_supported, 0x8, 0 },
[FEATURES][GET_FEATURE] = { "FEATURES_GET_FEATURE",
--
2.34.1On Mon, 3 Feb 2025 11:29:49 +0530
Arpit Kumar <arpit1.kumar@samsung.com> wrote:
Add some description of what is being added here.
Key issue in here is that clearing the CEL doesn't make
sense. It is a description of what the device supports, there
is no state to clear in it. To add this command you need
to pick a different log.
Jonathan
> Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>
> Reviewed-by: Alok Rathore <alok.rathore@samsung.com>
> Reviewed-by: Krishna Kanth Reddy <krish.reddy@samsung.com>
> ---
> hw/cxl/cxl-mailbox-utils.c | 36 ++++++++++++++++++++++++++++++++++++
> 1 file changed, 36 insertions(+)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 3d66a425a9..5fd7f850c4 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -77,6 +77,7 @@ enum {
> #define GET_SUPPORTED 0x0
> #define GET_LOG 0x1
> #define GET_LOG_CAPABILITIES 0x2
> + #define CLEAR_LOG 0x3
> FEATURES = 0x05,
> #define GET_SUPPORTED 0x0
> #define GET_FEATURE 0x1
> @@ -1115,6 +1116,39 @@ static CXLRetCode cmd_logs_get_log_capabilities(const struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
>
> +/* CXL r3.1 Section 8.2.9.5.4: Clear Log (Opcode 0403h) */
> +static CXLRetCode cmd_logs_clear_log(const struct cxl_cmd *cmd,
> + uint8_t *payload_in,
> + size_t len_in,
> + uint8_t *payload_out,
> + size_t *len_out,
> + CXLCCI *cci)
> +{
> + int32_t cap_id;
> + struct {
> + QemuUUID uuid;
> + } QEMU_PACKED QEMU_ALIGNED(8) * clear_log = (void *)payload_in;
> +
> + cap_id = valid_log_check(&clear_log->uuid, cci);
> + if (cap_id == -1) {
> + return CXL_MBOX_INVALID_LOG;
> + }
Follow on from previous patch, if this returns the cap pointer,
the following code wont have to index the array and should end up simpler.
> +
> + if (cci->supported_log_cap[cap_id].param_flags.clear_log_supported) {
I would flip this.
if (!(cap->param_flags & PARAM_FLAG_CLEAR_LOG_SUPPORTED)) {
return CXL_MBOX_UNSUPPORTED;
}
> + switch (cap_id) {
> + case CEL:
So if we return the cap as suggested, it will have to reference what it is
or provide a callback (which might be cleaner as this grows).
However, what does clearly the command effects log mean?
This makes no sense. So if you want to implement clear_log you
need to implement a different log to clear.
> + memset(cci->cel_log, 0, (1 << 16) * sizeof(struct cel_log));
> + cci->cel_size = 0;
> + break;
> + default:
> + return CXL_MBOX_UNSUPPORTED;
> + }
> + } else {
> + return CXL_MBOX_UNSUPPORTED;
> + }
> + return CXL_MBOX_SUCCESS;
> +}
> +
> /* CXL r3.1 section 8.2.9.6: Features */
> /*
> * Get Supported Features output payload
> @@ -2882,6 +2916,8 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
> [LOGS][GET_LOG] = { "LOGS_GET_LOG", cmd_logs_get_log, 0x18, 0 },
> [LOGS][GET_LOG_CAPABILITIES] = { "LOGS_GET_LOG_CAPABILITIES",
> cmd_logs_get_log_capabilities, 0x10, 0 },
> + [LOGS][CLEAR_LOG] = { "LOGS_CLEAR_LOG", cmd_logs_clear_log, 0x10,
> + CXL_MBOX_IMMEDIATE_LOG_CHANGE},
> [FEATURES][GET_SUPPORTED] = { "FEATURES_GET_SUPPORTED",
> cmd_features_get_supported, 0x8, 0 },
> [FEATURES][GET_FEATURE] = { "FEATURES_GET_FEATURE",
On 04/02/25 10:53AM, Jonathan Cameron wrote:
>On Mon, 3 Feb 2025 11:29:49 +0530
>Arpit Kumar <arpit1.kumar@samsung.com> wrote:
>
>Add some description of what is being added here.
>
>Key issue in here is that clearing the CEL doesn't make
>sense. It is a description of what the device supports, there
>is no state to clear in it. To add this command you need
>to pick a different log.
>
>Jonathan
>
Thanks for the review Jonathan, will modify the code accordingly
in V2 patch.
>
>> Signed-off-by: Arpit Kumar <arpit1.kumar@samsung.com>
>> Reviewed-by: Alok Rathore <alok.rathore@samsung.com>
>> Reviewed-by: Krishna Kanth Reddy <krish.reddy@samsung.com>
>> ---
>> hw/cxl/cxl-mailbox-utils.c | 36 ++++++++++++++++++++++++++++++++++++
>> 1 file changed, 36 insertions(+)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index 3d66a425a9..5fd7f850c4 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>> @@ -77,6 +77,7 @@ enum {
>> #define GET_SUPPORTED 0x0
>> #define GET_LOG 0x1
>> #define GET_LOG_CAPABILITIES 0x2
>> + #define CLEAR_LOG 0x3
>> FEATURES = 0x05,
>> #define GET_SUPPORTED 0x0
>> #define GET_FEATURE 0x1
>> @@ -1115,6 +1116,39 @@ static CXLRetCode cmd_logs_get_log_capabilities(const struct cxl_cmd *cmd,
>> return CXL_MBOX_SUCCESS;
>> }
>>
>> +/* CXL r3.1 Section 8.2.9.5.4: Clear Log (Opcode 0403h) */
>> +static CXLRetCode cmd_logs_clear_log(const struct cxl_cmd *cmd,
>> + uint8_t *payload_in,
>> + size_t len_in,
>> + uint8_t *payload_out,
>> + size_t *len_out,
>> + CXLCCI *cci)
>> +{
>> + int32_t cap_id;
>> + struct {
>> + QemuUUID uuid;
>> + } QEMU_PACKED QEMU_ALIGNED(8) * clear_log = (void *)payload_in;
>> +
>> + cap_id = valid_log_check(&clear_log->uuid, cci);
>> + if (cap_id == -1) {
>> + return CXL_MBOX_INVALID_LOG;
>> + }
>
>Follow on from previous patch, if this returns the cap pointer,
>the following code wont have to index the array and should end up simpler.
>
okay
>> +
>> + if (cci->supported_log_cap[cap_id].param_flags.clear_log_supported) {
>I would flip this.
> if (!(cap->param_flags & PARAM_FLAG_CLEAR_LOG_SUPPORTED)) {
> return CXL_MBOX_UNSUPPORTED;
> }
>
>
>> + switch (cap_id) {
>> + case CEL:
>
>So if we return the cap as suggested, it will have to reference what it is
>or provide a callback (which might be cleaner as this grows).
>
>However, what does clearly the command effects log mean?
>This makes no sense. So if you want to implement clear_log you
>need to implement a different log to clear.
>
okay
>> + memset(cci->cel_log, 0, (1 << 16) * sizeof(struct cel_log));
>> + cci->cel_size = 0;
>> + break;
>> + default:
>> + return CXL_MBOX_UNSUPPORTED;
>> + }
>> + } else {
>> + return CXL_MBOX_UNSUPPORTED;
>> + }
>> + return CXL_MBOX_SUCCESS;
>> +}
>> +
>> /* CXL r3.1 section 8.2.9.6: Features */
>> /*
>> * Get Supported Features output payload
>> @@ -2882,6 +2916,8 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
>> [LOGS][GET_LOG] = { "LOGS_GET_LOG", cmd_logs_get_log, 0x18, 0 },
>> [LOGS][GET_LOG_CAPABILITIES] = { "LOGS_GET_LOG_CAPABILITIES",
>> cmd_logs_get_log_capabilities, 0x10, 0 },
>> + [LOGS][CLEAR_LOG] = { "LOGS_CLEAR_LOG", cmd_logs_clear_log, 0x10,
>> + CXL_MBOX_IMMEDIATE_LOG_CHANGE},
>> [FEATURES][GET_SUPPORTED] = { "FEATURES_GET_SUPPORTED",
>> cmd_features_get_supported, 0x8, 0 },
>> [FEATURES][GET_FEATURE] = { "FEATURES_GET_FEATURE",
>
© 2016 - 2026 Red Hat, Inc.