Re: [PATCH v5 07/13] hw/mem/cxl_type3: Add DC extent list representative and get DC extent list mailbox support

2024-03-06 Thread Jonathan Cameron via 
On Mon,  4 Mar 2024 11:34:02 -0800
nifan@gmail.com wrote:

> From: Fan Ni 
> 
> Add dynamic capacity extent list representative to the definition of
> CXLType3Dev and add get DC extent list mailbox command per
> CXL.spec.3.1:.8.2.9.9.9.2.
> 
> Signed-off-by: Fan Ni 
Hi Fan,

A small thing in here around the assert to ensure we don't overflow
the mailbox.  We don't need that, just check what fits and return
fewer extents than asked for if they won't fit.

>  
> +/*
> + * CXL r3.1 section 8.2.9.9.9.2:
> + * Get Dynamic Capacity Extent List (Opcode 4801h)
> + */
> +static CXLRetCode cmd_dcd_get_dyn_cap_ext_list(const struct cxl_cmd *cmd,
> +   uint8_t *payload_in,
> +   size_t len_in,
> +   uint8_t *payload_out,
> +   size_t *len_out,
> +   CXLCCI *cci)

> +uint16_t record_count = 0, i = 0, record_done = 0;
> +uint16_t out_pl_len;
> +uint32_t start_extent_id = in->start_extent_id;

> +CXLDCExtentList *extent_list = >dc.extents;
> +CXLDCExtent *ent;
> +
> +if (start_extent_id > ct3d->dc.total_extent_count) {
> +return CXL_MBOX_INVALID_INPUT;
> +}
> +
> +record_count = MIN(in->extent_cnt,
> +   ct3d->dc.total_extent_count - start_extent_id);
Should clamp this using the length so that it fits in the mailbox...
> +
> +out_pl_len = sizeof(*out) + record_count * sizeof(out->records[0]);
> +assert(out_pl_len <= CXL_MAILBOX_MAX_PAYLOAD_SIZE);
and get rid of this nasty assert.

The command is not obliged to even try to return as many as requested,
it can return any smaller number (1 or more) with the assumption the driver
just asks for the next ones..

> +
> +stl_le_p(>count, record_count);
> +stl_le_p(>total_extents, ct3d->dc.total_extent_count);
> +stl_le_p(>generation_num, ct3d->dc.ext_list_gen_seq);
> +
> +if (record_count > 0) {
> +QTAILQ_FOREACH(ent, extent_list, node) {
> +if (i++ < start_extent_id) {
> +continue;
> +}
> +stq_le_p(>records[record_done].start_dpa, ent->start_dpa);

Maybe a local variable for out->records[record_done]?

CXLDCExtentRaw *out_rec = >records[record]done];
stq_le_p(_rec->len, ent->len);
etc

> +stq_le_p(>records[record_done].len, ent->len);
> +memcpy(>records[record_done].tag, ent->tag, 0x10);
> +stw_le_p(>records[record_done].shared_seq, ent->shared_seq);
> +
> +record_done++;
> +if (record_done == record_count) {
> +break;
> +}
> +}
> +}
> +
> +*len_out = out_pl_len;
> +return CXL_MBOX_SUCCESS;
> +}

[PATCH v5 07/13] hw/mem/cxl_type3: Add DC extent list representative and get DC extent list mailbox support

2024-03-04 Thread nifan . cxl 
From: Fan Ni 

Add dynamic capacity extent list representative to the definition of
CXLType3Dev and add get DC extent list mailbox command per
CXL.spec.3.1:.8.2.9.9.9.2.

Signed-off-by: Fan Ni 
---
 hw/cxl/cxl-mailbox-utils.c  | 71 -
 hw/mem/cxl_type3.c  |  1 +
 include/hw/cxl/cxl_device.h | 22 
 3 files changed, 93 insertions(+), 1 deletion(-)

diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c
index 8309f27a2b..425b378a2c 100644
--- a/hw/cxl/cxl-mailbox-utils.c
+++ b/hw/cxl/cxl-mailbox-utils.c
@@ -84,6 +84,7 @@ enum {
 #define CLEAR_POISON   0x2
 DCD_CONFIG  = 0x48,
 #define GET_DC_CONFIG  0x0
+#define GET_DYN_CAP_EXT_LIST   0x1
 PHYSICAL_SWITCH = 0x51,
 #define IDENTIFY_SWITCH_DEVICE  0x0
 #define GET_PHYSICAL_PORT_STATE 0x1
@@ -1325,7 +1326,8 @@ static CXLRetCode cmd_dcd_get_dyn_cap_config(const struct 
cxl_cmd *cmd,
  * to use.
  */
 stl_le_p(_out->num_extents_supported, CXL_NUM_EXTENTS_SUPPORTED);
-stl_le_p(_out->num_extents_available, CXL_NUM_EXTENTS_SUPPORTED);
+stl_le_p(_out->num_extents_available, CXL_NUM_EXTENTS_SUPPORTED -
+ ct3d->dc.total_extent_count);
 stl_le_p(_out->num_tags_supported, CXL_NUM_TAGS_SUPPORTED);
 stl_le_p(_out->num_tags_available, CXL_NUM_TAGS_SUPPORTED);
 
@@ -1333,6 +1335,70 @@ static CXLRetCode cmd_dcd_get_dyn_cap_config(const 
struct cxl_cmd *cmd,
 return CXL_MBOX_SUCCESS;
 }
 
+/*
+ * CXL r3.1 section 8.2.9.9.9.2:
+ * Get Dynamic Capacity Extent List (Opcode 4801h)
+ */
+static CXLRetCode cmd_dcd_get_dyn_cap_ext_list(const struct cxl_cmd *cmd,
+   uint8_t *payload_in,
+   size_t len_in,
+   uint8_t *payload_out,
+   size_t *len_out,
+   CXLCCI *cci)
+{
+CXLType3Dev *ct3d = CXL_TYPE3(cci->d);
+struct {
+uint32_t extent_cnt;
+uint32_t start_extent_id;
+} QEMU_PACKED *in = (void *)payload_in;
+struct {
+uint32_t count;
+uint32_t total_extents;
+uint32_t generation_num;
+uint8_t rsvd[4];
+CXLDCExtentRaw records[];
+} QEMU_PACKED *out = (void *)payload_out;
+uint16_t record_count = 0, i = 0, record_done = 0;
+uint16_t out_pl_len;
+uint32_t start_extent_id = in->start_extent_id;
+CXLDCExtentList *extent_list = >dc.extents;
+CXLDCExtent *ent;
+
+if (start_extent_id > ct3d->dc.total_extent_count) {
+return CXL_MBOX_INVALID_INPUT;
+}
+
+record_count = MIN(in->extent_cnt,
+   ct3d->dc.total_extent_count - start_extent_id);
+
+out_pl_len = sizeof(*out) + record_count * sizeof(out->records[0]);
+assert(out_pl_len <= CXL_MAILBOX_MAX_PAYLOAD_SIZE);
+
+stl_le_p(>count, record_count);
+stl_le_p(>total_extents, ct3d->dc.total_extent_count);
+stl_le_p(>generation_num, ct3d->dc.ext_list_gen_seq);
+
+if (record_count > 0) {
+QTAILQ_FOREACH(ent, extent_list, node) {
+if (i++ < start_extent_id) {
+continue;
+}
+stq_le_p(>records[record_done].start_dpa, ent->start_dpa);
+stq_le_p(>records[record_done].len, ent->len);
+memcpy(>records[record_done].tag, ent->tag, 0x10);
+stw_le_p(>records[record_done].shared_seq, ent->shared_seq);
+
+record_done++;
+if (record_done == record_count) {
+break;
+}
+}
+}
+
+*len_out = out_pl_len;
+return CXL_MBOX_SUCCESS;
+}
+
 #define IMMEDIATE_CONFIG_CHANGE (1 << 1)
 #define IMMEDIATE_DATA_CHANGE (1 << 2)
 #define IMMEDIATE_POLICY_CHANGE (1 << 3)
@@ -1380,6 +1446,9 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = {
 static const struct cxl_cmd cxl_cmd_set_dcd[256][256] = {
 [DCD_CONFIG][GET_DC_CONFIG] = { "DCD_GET_DC_CONFIG",
 cmd_dcd_get_dyn_cap_config, 2, 0 },
+[DCD_CONFIG][GET_DYN_CAP_EXT_LIST] = {
+"DCD_GET_DYNAMIC_CAPACITY_EXTENT_LIST", cmd_dcd_get_dyn_cap_ext_list,
+8, 0 },
 };
 
 static const struct cxl_cmd cxl_cmd_set_sw[256][256] = {
diff --git a/hw/mem/cxl_type3.c b/hw/mem/cxl_type3.c
index 2b380a260b..102fa8151e 100644
--- a/hw/mem/cxl_type3.c
+++ b/hw/mem/cxl_type3.c
@@ -673,6 +673,7 @@ static bool cxl_create_dc_regions(CXLType3Dev *ct3d, Error 
**errp)
 region_base += region->len;
 ct3d->dc.total_capacity += region->len;
 }
+QTAILQ_INIT(>dc.extents);
 
 return true;
 }
diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h
index 265679302c..8148bcc34b 100644
--- a/include/hw/cxl/cxl_device.h
+++ b/include/hw/cxl/cxl_device.h
@@ -424,6 +424,25 @@ typedef QLIST_HEAD(, CXLPoison) CXLPoisonList;
 
 #define DCD_MAX_NUM_REGION 8
 
+typedef struct