On Mon, 17 Mar 2025 16:31:29 +0000 anisa.su...@gmail.com wrote: > From: Anisa Su <anisa...@samsung.com> > > FM DCD Management command 0x5600 implemented per CXL 3.2 Spec Section > 7.6.7.6.1 > > Signed-off-by: Anisa Su <anisa...@samsung.com>
It's been too long so I'll look over this again as a precursor to looking at the later patches... Might well duplicate things I said before. :( > --- > hw/cxl/cxl-mailbox-utils.c | 67 ++++++++++++++++++++++++++++++++++++++ > hw/cxl/i2c_mctp_cxl.c | 6 +++- > 2 files changed, 72 insertions(+), 1 deletion(-) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 1b62d36101..e9991fd1a7 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -122,6 +122,8 @@ enum { > #define MANAGEMENT_COMMAND 0x0 > MHD = 0x55, > #define GET_MHD_INFO 0x0 > + FMAPI_DCD_MGMT = 0x56, > + #define GET_DCD_INFO 0x0 > }; > > /* CCI Message Format CXL r3.1 Figure 7-19 */ > @@ -3341,6 +3343,62 @@ static CXLRetCode cmd_dcd_release_dyn_cap(const struct > cxl_cmd *cmd, > return CXL_MBOX_SUCCESS; > } > > +/* > + * CXL r3.2 section 7.6.7.6.1: Get DCD Info (Opcode 5600h) > + */ > +static CXLRetCode cmd_fm_get_dcd_info(const struct cxl_cmd *cmd, > + uint8_t *payload_in, > + size_t len_in, > + uint8_t *payload_out, > + size_t *len_out, > + CXLCCI *cci) > +{ > + struct { > + uint8_t num_hosts; > + uint8_t num_regions_supported; > + uint8_t rsvd1[2]; > + uint16_t add_select_policy_bitmask; > + uint8_t rsvd2[2]; > + uint16_t release_select_policy_bitmask; > + uint8_t sanitize_on_release_bitmask; > + uint8_t rsvd3; > + uint64_t total_dynamic_capacity; > + uint64_t region_blk_size_bitmasks[8]; > + } QEMU_PACKED *out; You could set this directly to (void *)payload_out; here > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > + CXLDCRegion region; Why not use a pointer? Only copying from the region I think. > + int i; > + > + if (ct3d->dc.num_regions == 0) { This check shouldn't be needed as we don't register the commands if we have no regions. > + return CXL_MBOX_UNSUPPORTED; > + } > + > + out = (void *)payload_out; > + > + /* TODO: num hosts set to 1 for now */ > + out->num_hosts = 1; > + out->num_regions_supported = ct3d->dc.num_regions; > + stq_le_p(&out->total_dynamic_capacity, > + ct3d->dc.total_capacity / CXL_CAPACITY_MULTIPLIER); > + > + for (i = 0; i < ct3d->dc.num_regions; i++) { > + region = ct3d->dc.regions[i]; > + memcpy(&out->region_blk_size_bitmasks[i], > + ®ion.supported_blk_size_bitmask, 8); > + } > + > + *len_out = sizeof(*out); > + return CXL_MBOX_SUCCESS; > +} > + > static const struct cxl_cmd cxl_cmd_set[256][256] = { > [INFOSTAT][BACKGROUND_OPERATION_ABORT] = { "BACKGROUND_OPERATION_ABORT", > cmd_infostat_bg_op_abort, 0, 0 }, > @@ -3462,6 +3520,11 @@ static const struct cxl_cmd cxl_cmd_set_sw[256][256] = > { > cmd_tunnel_management_cmd, ~0, 0 }, > }; > > +static const struct cxl_cmd cxl_cmd_set_fm_dcd[256][256] = { > + [FMAPI_DCD_MGMT][GET_DCD_INFO] = { "GET_DCD_INFO", > + cmd_fm_get_dcd_info, 0, 0}, space before } > +}; > + > /* > * While the command is executing in the background, the device should > * update the percentage complete in the Background Command Status Register > @@ -3764,7 +3827,11 @@ void cxl_initialize_t3_fm_owned_ld_mctpcci(CXLCCI > *cci, DeviceState *d, > DeviceState *intf, > size_t payload_max) > { > + CXLType3Dev *ct3d = CXL_TYPE3(d); Blank line here. Run checkpatch over the patch series. Might catch little things like this. > cxl_copy_cci_commands(cci, cxl_cmd_set_t3_fm_owned_ld_mctp); > + if (ct3d->dc.num_regions) { length, len_out); > s->len = s->pos;