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 | 95 +++++++++++++++++++++++++++++++++++++
include/hw/cxl/cxl_device.h | 2 +
2 files changed, 97 insertions(+)
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 5fd7f850c4..115c2dc66b 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -78,6 +78,7 @@ enum {
#define GET_LOG 0x1
#define GET_LOG_CAPABILITIES 0x2
#define CLEAR_LOG 0x3
+ #define POPULATE_LOG 0x4
FEATURES = 0x05,
#define GET_SUPPORTED 0x0
#define GET_FEATURE 0x1
@@ -1149,6 +1150,94 @@ static CXLRetCode cmd_logs_clear_log(const struct cxl_cmd *cmd,
return CXL_MBOX_SUCCESS;
}
+static void cxl_rebuild_cel(CXLCCI *cci);
+
+static int get_populate_duration(uint32_t total_mem)
+{
+ int secs = 0;
+
+ if (total_mem <= 512) {
+ secs = 4;
+ } else if (total_mem <= 1024) {
+ secs = 8;
+ } else if (total_mem <= 2 * 1024) {
+ secs = 15;
+ } else if (total_mem <= 4 * 1024) {
+ secs = 30;
+ } else if (total_mem <= 8 * 1024) {
+ secs = 60;
+ } else if (total_mem <= 16 * 1024) {
+ secs = 2 * 60;
+ } else if (total_mem <= 32 * 1024) {
+ secs = 4 * 60;
+ } else if (total_mem <= 64 * 1024) {
+ secs = 8 * 60;
+ } else if (total_mem <= 128 * 1024) {
+ secs = 15 * 60;
+ } else if (total_mem <= 256 * 1024) {
+ secs = 30 * 60;
+ } else if (total_mem <= 512 * 1024) {
+ secs = 60 * 60;
+ } else if (total_mem <= 1024 * 1024) {
+ secs = 120 * 60;
+ } else {
+ secs = 240 * 60; /* max 4 hrs */
+ }
+
+ return secs;
+}
+
+/* CXL r3.1 Section 8.2.9.5.5: Populate log (Opcode 0404h) */
+static CXLRetCode cmd_logs_populate_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;
+ uint32_t total_mem = 0;
+ int secs = 0;
+ struct {
+ QemuUUID uuid;
+ } QEMU_PACKED QEMU_ALIGNED(8) * populate_log = (void *)payload_in;
+
+ cap_id = valid_log_check(&populate_log->uuid, cci);
+ if (cap_id == -1) {
+ return CXL_MBOX_INVALID_LOG;
+ }
+
+ if (cci->supported_log_cap[cap_id].param_flags.populate_log_supported) {
+ switch (cap_id) {
+ case CEL:
+ cci->log = CEL;
+ total_mem = (1 << 16) * sizeof(struct cel_log);
+ secs = get_populate_duration(total_mem >> 20);
+ goto start_bg;
+ break;
+ default:
+ return CXL_MBOX_UNSUPPORTED;
+ }
+ }
+ return CXL_MBOX_UNSUPPORTED;
+
+start_bg:
+ cci->bg.runtime = secs * 1000UL;
+ *len_out = 0;
+ return CXL_MBOX_BG_STARTED;
+}
+
+static void __do_populate(CXLCCI *cci)
+{
+ switch (cci->log) {
+ case CEL:
+ cxl_rebuild_cel(cci);
+ break;
+ default:
+ break;
+ }
+}
+
/* CXL r3.1 section 8.2.9.6: Features */
/*
* Get Supported Features output payload
@@ -2918,6 +3007,9 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
cmd_logs_get_log_capabilities, 0x10, 0 },
[LOGS][CLEAR_LOG] = { "LOGS_CLEAR_LOG", cmd_logs_clear_log, 0x10,
CXL_MBOX_IMMEDIATE_LOG_CHANGE},
+ [LOGS][POPULATE_LOG] = { "LOGS_POPULATE_LOG", cmd_logs_populate_log, 0x10,
+ (CXL_MBOX_IMMEDIATE_LOG_CHANGE |
+ CXL_MBOX_BACKGROUND_OPERATION)},
[FEATURES][GET_SUPPORTED] = { "FEATURES_GET_SUPPORTED",
cmd_features_get_supported, 0x8, 0 },
[FEATURES][GET_FEATURE] = { "FEATURES_GET_FEATURE",
@@ -3098,6 +3190,9 @@ static void bg_timercb(void *opaque)
case 0x0201: /* fw transfer */
__do_firmware_xfer(cci);
break;
+ case 0x0404: /* populate_ log */
+ __do_populate(cci);
+ break;
case 0x4400: /* sanitize */
{
CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index e7cb99a1d2..d90d0d4a8f 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -230,6 +230,8 @@ typedef struct CXLCCI {
} cel_log[1 << 16];
size_t cel_size;
+ enum Logs log;
+
/* get log capabilities */
CXLLogCapabilities supported_log_cap[MAX_LOG_TYPE];
--
2.34.1On Mon, 3 Feb 2025 11:29:50 +0530
Arpit Kumar <arpit1.kumar@samsung.com> wrote:
> 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>
Likewise, the CEL isn't a dynamic thing. Asking to populate
it makes little sense so the log capability should always
say this is not supported.
I suspect you had this running with a different log and flipped
to CEL for purposes of up streaming?
Anyhow, choose something else. Maybe component state dump or ECS log?
> ---
> hw/cxl/cxl-mailbox-utils.c | 95 +++++++++++++++++++++++++++++++++++++
> include/hw/cxl/cxl_device.h | 2 +
> 2 files changed, 97 insertions(+)
>
> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
> index 5fd7f850c4..115c2dc66b 100644
> --- a/hw/cxl/cxl-mailbox-utils.c
> +++ b/hw/cxl/cxl-mailbox-utils.c
> @@ -78,6 +78,7 @@ enum {
> #define GET_LOG 0x1
> #define GET_LOG_CAPABILITIES 0x2
> #define CLEAR_LOG 0x3
> + #define POPULATE_LOG 0x4
> FEATURES = 0x05,
> #define GET_SUPPORTED 0x0
> #define GET_FEATURE 0x1
> @@ -1149,6 +1150,94 @@ static CXLRetCode cmd_logs_clear_log(const struct cxl_cmd *cmd,
> return CXL_MBOX_SUCCESS;
> }
>
> +static void cxl_rebuild_cel(CXLCCI *cci);
> +
> +static int get_populate_duration(uint32_t total_mem)
> +{
> + int secs = 0;
> +
> + if (total_mem <= 512) {
> + secs = 4;
> + } else if (total_mem <= 1024) {
> + secs = 8;
> + } else if (total_mem <= 2 * 1024) {
> + secs = 15;
> + } else if (total_mem <= 4 * 1024) {
> + secs = 30;
> + } else if (total_mem <= 8 * 1024) {
> + secs = 60;
> + } else if (total_mem <= 16 * 1024) {
> + secs = 2 * 60;
> + } else if (total_mem <= 32 * 1024) {
> + secs = 4 * 60;
> + } else if (total_mem <= 64 * 1024) {
> + secs = 8 * 60;
> + } else if (total_mem <= 128 * 1024) {
> + secs = 15 * 60;
> + } else if (total_mem <= 256 * 1024) {
> + secs = 30 * 60;
> + } else if (total_mem <= 512 * 1024) {
> + secs = 60 * 60;
> + } else if (total_mem <= 1024 * 1024) {
> + secs = 120 * 60;
> + } else {
> + secs = 240 * 60; /* max 4 hrs */
> + }
> +
> + return secs;
> +}
> +
> +/* CXL r3.1 Section 8.2.9.5.5: Populate log (Opcode 0404h) */
> +static CXLRetCode cmd_logs_populate_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;
> + uint32_t total_mem = 0;
> + int secs = 0;
> + struct {
> + QemuUUID uuid;
> + } QEMU_PACKED QEMU_ALIGNED(8) * populate_log = (void *)payload_in;
> +
> + cap_id = valid_log_check(&populate_log->uuid, cci);
> + if (cap_id == -1) {
> + return CXL_MBOX_INVALID_LOG;
> + }
> +
> + if (cci->supported_log_cap[cap_id].param_flags.populate_log_supported) {
> + switch (cap_id) {
> + case CEL:
Similar to before, a callback to fill the log inside the cap entry
is probably going to be the most extensible way to do this rather
than using an ID and a switch statement that gets steadily more
complex and doesn't easily allow for different device emulation to
make different choices on what to fill with (e.g. faking component
state dump for a type 3 vs a type 2 device - once supported in qemu).
> + cci->log = CEL;
> + total_mem = (1 << 16) * sizeof(struct cel_log);
> + secs = get_populate_duration(total_mem >> 20);
Why would the CEL be based on memory size?
> + goto start_bg;
> + break;
> + default:
> + return CXL_MBOX_UNSUPPORTED;
> + }
> + }
> + return CXL_MBOX_UNSUPPORTED;
> +
> +start_bg:
> + cci->bg.runtime = secs * 1000UL;
> + *len_out = 0;
> + return CXL_MBOX_BG_STARTED;
> +}
> +
> +static void __do_populate(CXLCCI *cci)
> +{
> + switch (cci->log) {
> + case CEL:
> + cxl_rebuild_cel(cci);
> + break;
> + default:
> + break;
> + }
> +}
> +
> /* CXL r3.1 section 8.2.9.6: Features */
> /*
> * Get Supported Features output payload
> @@ -2918,6 +3007,9 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
> cmd_logs_get_log_capabilities, 0x10, 0 },
> [LOGS][CLEAR_LOG] = { "LOGS_CLEAR_LOG", cmd_logs_clear_log, 0x10,
> CXL_MBOX_IMMEDIATE_LOG_CHANGE},
> + [LOGS][POPULATE_LOG] = { "LOGS_POPULATE_LOG", cmd_logs_populate_log, 0x10,
> + (CXL_MBOX_IMMEDIATE_LOG_CHANGE |
> + CXL_MBOX_BACKGROUND_OPERATION)},
> [FEATURES][GET_SUPPORTED] = { "FEATURES_GET_SUPPORTED",
> cmd_features_get_supported, 0x8, 0 },
> [FEATURES][GET_FEATURE] = { "FEATURES_GET_FEATURE",
> @@ -3098,6 +3190,9 @@ static void bg_timercb(void *opaque)
> case 0x0201: /* fw transfer */
> __do_firmware_xfer(cci);
> break;
> + case 0x0404: /* populate_ log */
> + __do_populate(cci);
> + break;
> case 0x4400: /* sanitize */
> {
> CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
> index e7cb99a1d2..d90d0d4a8f 100644
> --- a/include/hw/cxl/cxl_device.h
> +++ b/include/hw/cxl/cxl_device.h
> @@ -230,6 +230,8 @@ typedef struct CXLCCI {
> } cel_log[1 << 16];
> size_t cel_size;
>
> + enum Logs log;
> +
> /* get log capabilities */
> CXLLogCapabilities supported_log_cap[MAX_LOG_TYPE];
>
On 04/02/25 10:58AM, Jonathan Cameron wrote:
>On Mon, 3 Feb 2025 11:29:50 +0530
>Arpit Kumar <arpit1.kumar@samsung.com> wrote:
>
>> 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>
>
>Likewise, the CEL isn't a dynamic thing. Asking to populate
>it makes little sense so the log capability should always
>say this is not supported.
>
>I suspect you had this running with a different log and flipped
>to CEL for purposes of up streaming?
>
>Anyhow, choose something else. Maybe component state dump or ECS log?
>
Thanks for the review Jonathan, will update the same in V2 patch
and return unsupported for CEL.
>> ---
>> hw/cxl/cxl-mailbox-utils.c | 95 +++++++++++++++++++++++++++++++++++++
>> include/hw/cxl/cxl_device.h | 2 +
>> 2 files changed, 97 insertions(+)
>>
>> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
>> index 5fd7f850c4..115c2dc66b 100644
>> --- a/hw/cxl/cxl-mailbox-utils.c
>> +++ b/hw/cxl/cxl-mailbox-utils.c
>> @@ -78,6 +78,7 @@ enum {
>> #define GET_LOG 0x1
>> #define GET_LOG_CAPABILITIES 0x2
>> #define CLEAR_LOG 0x3
>> + #define POPULATE_LOG 0x4
>> FEATURES = 0x05,
>> #define GET_SUPPORTED 0x0
>> #define GET_FEATURE 0x1
>> @@ -1149,6 +1150,94 @@ static CXLRetCode cmd_logs_clear_log(const struct cxl_cmd *cmd,
>> return CXL_MBOX_SUCCESS;
>> }
>>
>> +static void cxl_rebuild_cel(CXLCCI *cci);
>> +
>> +static int get_populate_duration(uint32_t total_mem)
>> +{
>> + int secs = 0;
>> +
>> + if (total_mem <= 512) {
>> + secs = 4;
>> + } else if (total_mem <= 1024) {
>> + secs = 8;
>> + } else if (total_mem <= 2 * 1024) {
>> + secs = 15;
>> + } else if (total_mem <= 4 * 1024) {
>> + secs = 30;
>> + } else if (total_mem <= 8 * 1024) {
>> + secs = 60;
>> + } else if (total_mem <= 16 * 1024) {
>> + secs = 2 * 60;
>> + } else if (total_mem <= 32 * 1024) {
>> + secs = 4 * 60;
>> + } else if (total_mem <= 64 * 1024) {
>> + secs = 8 * 60;
>> + } else if (total_mem <= 128 * 1024) {
>> + secs = 15 * 60;
>> + } else if (total_mem <= 256 * 1024) {
>> + secs = 30 * 60;
>> + } else if (total_mem <= 512 * 1024) {
>> + secs = 60 * 60;
>> + } else if (total_mem <= 1024 * 1024) {
>> + secs = 120 * 60;
>> + } else {
>> + secs = 240 * 60; /* max 4 hrs */
>> + }
>> +
>> + return secs;
>> +}
>> +
>> +/* CXL r3.1 Section 8.2.9.5.5: Populate log (Opcode 0404h) */
>> +static CXLRetCode cmd_logs_populate_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;
>> + uint32_t total_mem = 0;
>> + int secs = 0;
>> + struct {
>> + QemuUUID uuid;
>> + } QEMU_PACKED QEMU_ALIGNED(8) * populate_log = (void *)payload_in;
>> +
>> + cap_id = valid_log_check(&populate_log->uuid, cci);
>> + if (cap_id == -1) {
>> + return CXL_MBOX_INVALID_LOG;
>> + }
>> +
>> + if (cci->supported_log_cap[cap_id].param_flags.populate_log_supported) {
>> + switch (cap_id) {
>> + case CEL:
>Similar to before, a callback to fill the log inside the cap entry
>is probably going to be the most extensible way to do this rather
>than using an ID and a switch statement that gets steadily more
>complex and doesn't easily allow for different device emulation to
>make different choices on what to fill with (e.g. faking component
>state dump for a type 3 vs a type 2 device - once supported in qemu).
>
okay
>> + cci->log = CEL;
>> + total_mem = (1 << 16) * sizeof(struct cel_log);
>> + secs = get_populate_duration(total_mem >> 20);
>
>Why would the CEL be based on memory size?
>
CEL is not dependent on memory size. It is representing CEL buffer size.
total_mem variable is misleading in this case. will take care in next
iteration.
>> + goto start_bg;
>> + break;
>> + default:
>> + return CXL_MBOX_UNSUPPORTED;
>> + }
>> + }
>> + return CXL_MBOX_UNSUPPORTED;
>> +
>> +start_bg:
>> + cci->bg.runtime = secs * 1000UL;
>> + *len_out = 0;
>> + return CXL_MBOX_BG_STARTED;
>> +}
>> +
>> +static void __do_populate(CXLCCI *cci)
>> +{
>> + switch (cci->log) {
>> + case CEL:
>> + cxl_rebuild_cel(cci);
>> + break;
>> + default:
>> + break;
>> + }
>> +}
>> +
>> /* CXL r3.1 section 8.2.9.6: Features */
>> /*
>> * Get Supported Features output payload
>> @@ -2918,6 +3007,9 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
>> cmd_logs_get_log_capabilities, 0x10, 0 },
>> [LOGS][CLEAR_LOG] = { "LOGS_CLEAR_LOG", cmd_logs_clear_log, 0x10,
>> CXL_MBOX_IMMEDIATE_LOG_CHANGE},
>> + [LOGS][POPULATE_LOG] = { "LOGS_POPULATE_LOG", cmd_logs_populate_log, 0x10,
>> + (CXL_MBOX_IMMEDIATE_LOG_CHANGE |
>> + CXL_MBOX_BACKGROUND_OPERATION)},
>> [FEATURES][GET_SUPPORTED] = { "FEATURES_GET_SUPPORTED",
>> cmd_features_get_supported, 0x8, 0 },
>> [FEATURES][GET_FEATURE] = { "FEATURES_GET_FEATURE",
>> @@ -3098,6 +3190,9 @@ static void bg_timercb(void *opaque)
>> case 0x0201: /* fw transfer */
>> __do_firmware_xfer(cci);
>> break;
>> + case 0x0404: /* populate_ log */
>> + __do_populate(cci);
>> + break;
>> case 0x4400: /* sanitize */
>> {
>> CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
>> diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
>> index e7cb99a1d2..d90d0d4a8f 100644
>> --- a/include/hw/cxl/cxl_device.h
>> +++ b/include/hw/cxl/cxl_device.h
>> @@ -230,6 +230,8 @@ typedef struct CXLCCI {
>> } cel_log[1 << 16];
>> size_t cel_size;
>>
>> + enum Logs log;
>> +
>> /* get log capabilities */
>> CXLLogCapabilities supported_log_cap[MAX_LOG_TYPE];
>>
>
© 2016 - 2025 Red Hat, Inc.