Hi Fan, Thanks for the feedbacks.
>-----Original Message----- >From: fan <nifan....@gmail.com> >Sent: 15 February 2024 18:06 >To: Shiju Jose <shiju.j...@huawei.com> >Cc: qemu-devel@nongnu.org; linux-...@vger.kernel.org; Jonathan Cameron ><jonathan.came...@huawei.com>; tanxiaofei <tanxiao...@huawei.com>; >Zengtao (B) <prime.z...@hisilicon.com>; Linuxarm <linux...@huawei.com> >Subject: Re: [PATCH v2 1/3] hw/cxl/cxl-mailbox-utils: Add support for feature >commands (8.2.9.6) > >On Fri, Nov 24, 2023 at 09:53:35PM +0800, shiju.j...@huawei.com wrote: >> From: Shiju Jose <shiju.j...@huawei.com> >> >> CXL spec 3.0 section 8.2.9.6 describes optional device specific features. >> CXL devices supports features with changeable attributes. >> Get Supported Features retrieves the list of supported device specific >> features. The settings of a feature can be retrieved using Get Feature >> and optionally modified using Set Feature. >> >> Reviewed-by: Davidlohr Bueso <d...@stgolabs.net> >> Signed-off-by: Shiju Jose <shiju.j...@huawei.com> >> --- > >Updated the references to align with cxl spec r3.1, other than that looks good >to >me. I had posted v3 of this series updated for spec r3.1. Please find here, https://lore.kernel.org/qemu-devel/20240215110146.1444-1-shiju.j...@huawei.com/T/#t > >Fan > >> hw/cxl/cxl-mailbox-utils.c | 167 >> +++++++++++++++++++++++++++++++++++++ >> 1 file changed, 167 insertions(+) >> >> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c >> index 6184f44339..1bbc9a48a6 100644 >> --- a/hw/cxl/cxl-mailbox-utils.c >> +++ b/hw/cxl/cxl-mailbox-utils.c >> @@ -66,6 +66,10 @@ enum { >> LOGS = 0x04, >> #define GET_SUPPORTED 0x0 >> #define GET_LOG 0x1 >> + FEATURES = 0x05, >> + #define GET_SUPPORTED 0x0 >> + #define GET_FEATURE 0x1 >> + #define SET_FEATURE 0x2 >> IDENTIFY = 0x40, >> #define MEMORY_DEVICE 0x0 >> CCLS = 0x41, >> @@ -785,6 +789,157 @@ static CXLRetCode cmd_logs_get_log(const struct >cxl_cmd *cmd, >> return CXL_MBOX_SUCCESS; >> } >> >> +/* CXL r3.0 section 8.2.9.6: Features */ typedef struct >> +CXLSupportedFeatureHeader { >> + uint16_t entries; >> + uint16_t nsuppfeats_dev; >> + uint32_t reserved; >> +} QEMU_PACKED CXLSupportedFeatureHeader; >> + >> +typedef struct CXLSupportedFeatureEntry { >> + QemuUUID uuid; >> + uint16_t feat_index; >> + uint16_t get_feat_size; >> + uint16_t set_feat_size; >> + uint32_t attrb_flags; >> + uint8_t get_feat_version; >> + uint8_t set_feat_version; >> + uint16_t set_feat_effects; >> + uint8_t rsvd[18]; >> +} QEMU_PACKED CXLSupportedFeatureEntry; >> + >> +enum CXL_SUPPORTED_FEATURES_LIST { >> + CXL_FEATURE_MAX >> +}; >> + >> +/* Get Feature CXL 3.0 Spec 8.2.9.6.2 */ >> +/* >> + * Get Feature input payload >> + * CXL rev 3.0 section 8.2.9.6.2; Table 8-79 */ >> +/* Get Feature : Payload in selection */ enum >> +CXL_GET_FEATURE_SELECTION { >> + CXL_GET_FEATURE_SEL_CURRENT_VALUE = 0x0, >> + CXL_GET_FEATURE_SEL_DEFAULT_VALUE = 0x1, >> + CXL_GET_FEATURE_SEL_SAVED_VALUE = 0x2, >> + CXL_GET_FEATURE_SEL_MAX >> +}; >> + >> +/* Set Feature CXL 3.0 Spec 8.2.9.6.3 */ >> +/* >> + * Set Feature input payload >> + * CXL rev 3.0 section 8.2.9.6.3; Table 8-81 */ typedef struct >> +CXLSetFeatureInHeader { >> + QemuUUID uuid; >> + uint32_t flags; >> + uint16_t offset; >> + uint8_t version; >> + uint8_t rsvd[9]; >> +} QEMU_PACKED QEMU_ALIGNED(16) CXLSetFeatureInHeader; >> + >> +/* Set Feature : Payload in flags */ >> +#define CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MASK 0x7 >> +enum CXL_SET_FEATURE_FLAG_DATA_TRANSFER { >> + CXL_SET_FEATURE_FLAG_FULL_DATA_TRANSFER = 0x0, >> + CXL_SET_FEATURE_FLAG_INITIATE_DATA_TRANSFER = 0x1, >> + CXL_SET_FEATURE_FLAG_CONTINUE_DATA_TRANSFER = 0x2, >> + CXL_SET_FEATURE_FLAG_FINISH_DATA_TRANSFER = 0x3, >> + CXL_SET_FEATURE_FLAG_ABORT_DATA_TRANSFER = 0x4, >> + CXL_SET_FEATURE_FLAG_DATA_TRANSFER_MAX >> +}; >> + >> +/* CXL r3.0 section 8.2.9.6.1: Get Supported Features (Opcode 0500h) >> +*/ static CXLRetCode cmd_features_get_supported(const struct cxl_cmd >*cmd, >> + uint8_t *payload_in, >> + size_t len_in, >> + uint8_t *payload_out, >> + size_t *len_out, >> + CXLCCI *cci) { >> + struct { >> + uint32_t count; >> + uint16_t start_index; >> + uint16_t reserved; >> + } QEMU_PACKED QEMU_ALIGNED(16) * get_feats_in = (void >> +*)payload_in; >> + >> + struct { >> + CXLSupportedFeatureHeader hdr; >> + CXLSupportedFeatureEntry feat_entries[]; >> + } QEMU_PACKED QEMU_ALIGNED(16) * get_feats_out = (void >*)payload_out; >> + uint16_t index; >> + uint16_t entry, req_entries; >> + uint16_t feat_entries = 0; >> + >> + if (get_feats_in->count < sizeof(CXLSupportedFeatureHeader) || >> + get_feats_in->start_index > CXL_FEATURE_MAX) { >> + return CXL_MBOX_INVALID_INPUT; >> + } >> + req_entries = (get_feats_in->count - >> + sizeof(CXLSupportedFeatureHeader)) / >> + sizeof(CXLSupportedFeatureEntry); >> + req_entries = MIN(req_entries, CXL_FEATURE_MAX); >> + index = get_feats_in->start_index; >> + >> + entry = 0; >> + while (entry < req_entries) { >> + switch (index) { >> + default: >> + break; >> + } >> + index++; >> + entry++; >> + } >> + >> + get_feats_out->hdr.nsuppfeats_dev = CXL_FEATURE_MAX; >> + get_feats_out->hdr.entries = feat_entries; >> + *len_out = sizeof(CXLSupportedFeatureHeader) + >> + feat_entries * >> + sizeof(CXLSupportedFeatureEntry); >> + >> + return CXL_MBOX_SUCCESS; >> +} >> + >> +/* CXL r3.0 section 8.2.9.6.2: Get Feature (Opcode 0501h) */ static >> +CXLRetCode cmd_features_get_feature(const struct cxl_cmd *cmd, >> + uint8_t *payload_in, >> + size_t len_in, >> + uint8_t *payload_out, >> + size_t *len_out, >> + CXLCCI *cci) { >> + struct { >> + QemuUUID uuid; >> + uint16_t offset; >> + uint16_t count; >> + uint8_t selection; >> + } QEMU_PACKED QEMU_ALIGNED(16) * get_feature; >> + uint16_t bytes_to_copy = 0; >> + >> + get_feature = (void *)payload_in; >> + >> + if (get_feature->selection != CXL_GET_FEATURE_SEL_CURRENT_VALUE) { >> + return CXL_MBOX_UNSUPPORTED; >> + } >> + if (get_feature->offset + get_feature->count > cci->payload_max) { >> + return CXL_MBOX_INVALID_INPUT; >> + } >> + >> + *len_out = bytes_to_copy; >> + >> + return CXL_MBOX_SUCCESS; >> +} >> + >> +/* CXL r3.0 section 8.2.9.6.3: Set Feature (Opcode 0502h) */ static >> +CXLRetCode cmd_features_set_feature(const struct cxl_cmd *cmd, >> + uint8_t *payload_in, >> + size_t len_in, >> + uint8_t *payload_out, >> + size_t *len_out, >> + CXLCCI *cci) { >> + return CXL_MBOX_SUCCESS; >> +} >> + >> /* 8.2.9.5.1.1 */ >> static CXLRetCode cmd_identify_memory_device(const struct cxl_cmd *cmd, >> uint8_t *payload_in, @@ >> -1954,6 +2109,18 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { >> [LOGS][GET_SUPPORTED] = { "LOGS_GET_SUPPORTED", >cmd_logs_get_supported, >> 0, 0 }, >> [LOGS][GET_LOG] = { "LOGS_GET_LOG", cmd_logs_get_log, 0x18, 0 }, >> + [FEATURES][GET_SUPPORTED] = { "FEATURES_GET_SUPPORTED", >> + cmd_features_get_supported, 0x8, 0 }, >> + [FEATURES][GET_FEATURE] = { "FEATURES_GET_FEATURE", >> + cmd_features_get_feature, 0x15, 0 }, >> + [FEATURES][SET_FEATURE] = { "FEATURES_SET_FEATURE", >> + cmd_features_set_feature, >> + ~0, >> + (CXL_MBOX_IMMEDIATE_CONFIG_CHANGE | >> + CXL_MBOX_IMMEDIATE_DATA_CHANGE | >> + CXL_MBOX_IMMEDIATE_POLICY_CHANGE | >> + CXL_MBOX_IMMEDIATE_LOG_CHANGE | >> + CXL_MBOX_SECURITY_STATE_CHANGE)}, >> [IDENTIFY][MEMORY_DEVICE] = { "IDENTIFY_MEMORY_DEVICE", >> cmd_identify_memory_device, 0, 0 }, >> [CCLS][GET_PARTITION_INFO] = { "CCLS_GET_PARTITION_INFO", >> -- >> 2.34.1 >> Thanks, Shiju
