On 04/02/25 10:58AM, Jonathan Cameron wrote:
On Mon, 3 Feb 2025 11:29:50 +0530
Arpit Kumar <arpit1.ku...@samsung.com> wrote:
Signed-off-by: Arpit Kumar <arpit1.ku...@samsung.com>
Reviewed-by: Alok Rathore <alok.rath...@samsung.com>
Reviewed-by: Krishna Kanth Reddy <krish.re...@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];
