| Message ID | 20250123050903.92336-2-vinayak.kh@samsung.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | CXL CCI Media Operations | expand |
On Thu, 23 Jan 2025 10:39:02 +0530 Vinayak Holikatti <vinayak.kh@samsung.com> wrote: Hi Vinayak, Thanks for your patch! Good to add support for this. Various comments inline, but all fairly minor things. thanks, Jonathan > CXL spec 3.1 section 8.2.9.9.5.3 describes media operations commands. > CXL devices supports media operations discovery command. Please don't indent the commit message. Maybe this is a side effect of some tooling but definitely clean it up before sending a v2. > > Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com> +CC linux-cxl to increase chance of review and let people know this exists. > --- > hw/cxl/cxl-mailbox-utils.c | 130 ++++++++++++++++++++++++++++++++++++- > 1 file changed, 128 insertions(+), 2 deletions(-) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 9c7ea5bc35..2315d07fb1 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -87,8 +87,9 @@ enum { > #define GET_LSA 0x2 > #define SET_LSA 0x3 > SANITIZE = 0x44, > - #define OVERWRITE 0x0 > - #define SECURE_ERASE 0x1 > + #define OVERWRITE 0x0 > + #define SECURE_ERASE 0x1 > + #define MEDIA_OPERATIONS 0x2 Trivial but I've given up trying to keep these aligned. It's a fools game as the names get steadily longer. As such better to just leave the existing pair alone. > PERSISTENT_MEM = 0x45, > #define GET_SECURITY_STATE 0x0 > MEDIA_AND_POISON = 0x43, > @@ -1721,6 +1722,127 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, > return CXL_MBOX_BG_STARTED; > } > > +enum { > + MEDIA_OP_GENERAL = 0x0, I'd name them so the field id explicit. MEDIA_OP_CLASS_GENERAL etc > + MEDIA_OP_SANITIZE = 0x1, > + MEDIA_OP_CLASS_MAX, No comma on terminating entry. We don't want it to be easy to add stuff after it. > +} MEDIA_OPERATION_CLASS; The enum type is never used. So might as well keep it anonymous like we do for other enums in this file. > + > +enum { > + MEDIA_OP_SUB_DISCOVERY = 0x0, This set of class and subcalss is similar to the enum you add the MEDIA_OPERATIONS define to above. I'd take a similar strategy with enum { MEDIA_OP_CLASS_GENERAL = 0x0, #define MEDIA_OP_GEN_SUBC_DISCOVERY 0x0 MEDIA_OP_CLASS_SANITIZE = 0x1, #define MEDIA_OP_SAN_SUBC_SANITIZE 0x0 #define MEDIA_OP_SAN_SUBC_ZERO 0x1 or something like that. } > + MEDIA_OP_SUB_SANITIZE = 0x0, > + MEDIA_OP_SUB_ZERO = 0x1, > + MEDIA_OP_SUB_CLASS_MAX No need for SUB_CLASS_MAX as you don't seem to use it. > +} MEDIA_OPERATION_SUB_CLASS; > + > +struct media_op_supported_list_entry { > + uint8_t media_op_class; > + uint8_t media_op_subclass; > +}; > + > +struct media_op_discovery_out_pl { > + uint64_t dpa_range_granularity; > + uint16_t total_supported_operations; > + uint16_t num_of_supported_operations; > + struct media_op_supported_list_entry entry[0]; entry[] which is the c spec defined way to do variable length last elements. The [0] was I think a weird extension that we have moved away from. > +}; Not strictly necessary but I'd mark it packed as chances of future breakage are high with a structure starting at byte 0xC. > + > +#define MAX_SUPPORTED_OPS 3 I'd avoid explicit define for this and just use ARRAY_SIZE() on the array of structures to find out. > +struct media_op_supported_list_entry media_op_matrix[MAX_SUPPORTED_OPS] = { Use the defines above rather than the numeric values. Then it's obvious what this is, also mark it static const. static const struct media_op_supported_list_entry media_op_matrix[] = { MEDIA_OP_CLASS_GENERAL, MEDIA_OP_GEN_SUBC_DISCOVERY }, { MEDIA_OP_CLASS_SANITIZE, MEDIA_OP_SAN_SUBC_SANITIZE }, { MEDIA_OP_CLASS_SANITIZE, MEDIA_OP_SAN_SUBC_ZERO }, }; > + {0, 0}, > + {1, 0}, > + {1, 1} }; > + > +static CXLRetCode cmd_media_operations(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 media_operation_class; struct { uint8_t media_operation_class; etc for alignment. > + uint8_t media_operation_subclass; > + uint8_t rsvd[2]; > + uint32_t dpa_range_count; > + union { > + struct { > + uint64_t starting_dpa; > + uint64_t length; > + } dpa_range_list[0]; [] > + struct { > + uint16_t start_index; > + uint16_t num_supported_ops; > + } discovery_osa; > + }; This is a little tricky as in theory you can have a variable number of DPA Range List elements and then the operation specific arguments. However, general always provides a range count of 0. Also both sanitize and zero have no osa elemetns. Add a comment about this so we don't think it looks wrong in future + do notice that this approach doesn't generalize if a new operation allows dpa ranges and operation specific parameters. > + } QEMU_PACKED *media_op_in_pl = (void *)payload_in; > + > + uint8_t media_op_cl = media_op_in_pl->media_operation_class; > + uint8_t media_op_subclass = media_op_in_pl->media_operation_subclass; > + uint32_t dpa_range_count = media_op_in_pl->dpa_range_count; > + > + if (len_in < sizeof(*media_op_in_pl)) { > + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; > + } Test this before getting values to fill in media_op_cl local variables etc. It's both logically correct and may constrain the compiler not to get too smart if it can see enough to realize what len_in is. > + > + switch (media_op_cl) { > + case MEDIA_OP_GENERAL: > + switch (media_op_subclass) { > + case MEDIA_OP_SUB_DISCOVERY: Given there is only one element, maybe cleaner as if (media_op_subclass != MEDIA_OP_SUB_DISCOVERY) { return CXL_MBOX_UNSUPPORTED; } AS reduces indent of the following, helping readability a litle. > + int count = 0; > + struct media_op_discovery_out_pl *media_out_pl = > + (void *)payload_out; > + int num_ops = media_op_in_pl->discovery_osa.num_supported_ops; > + int start_index = media_op_in_pl->discovery_osa.start_index; > + > + /* As per spec CXL 3.1 8.2.9.9.5.3 dpa_range_count */ > + /* should be zero for discovery sub class command */ Local style is multiline comment as /* * As per spec CXL 3.1... * should be zero... */ > + if (dpa_range_count) { > + return CXL_MBOX_INVALID_INPUT; > + } > + > + if ((start_index >= MEDIA_OP_CLASS_MAX) || > + (num_ops > MAX_SUPPORTED_OPS)) { Check here should be for num_ops + start_index > MAX_SUPPORTED OPS Comparing start_index against MEDIA_OP_CLASS_MAX doesn't make sense to me as I believe it's an index into the array of Class / subclass pairs not the class array. > + return CXL_MBOX_INVALID_INPUT; > + } > + > + media_out_pl->dpa_range_granularity = CXL_CAPACITY_MULTIPLIER; > + media_out_pl->total_supported_operations = MAX_SUPPORTED_OPS; > + if (num_ops > 0) { > + for (int i = start_index; i < MAX_SUPPORTED_OPS; i++) { > + media_out_pl->entry[count].media_op_class = > + media_op_matrix[i].media_op_class; > + media_out_pl->entry[count].media_op_subclass = > + media_op_matrix[i].media_op_subclass; > + count++; > + if (count == num_ops) { > + goto disc_out; break should be enough and removes need for goto and label. > + } > + } > + } > +disc_out: > + media_out_pl->num_of_supported_operations = count; > + *len_out = sizeof(struct media_op_discovery_out_pl) + > + (sizeof(struct media_op_supported_list_entry) * count); indent this line. > + break; I'd return CXL_MBOX_SUCCESS; > + default: > + return CXL_MBOX_UNSUPPORTED; > + } > + break; then this break isn't needed. > + case MEDIA_OP_SANITIZE: > + switch (media_op_subclass) { > + No blank line here yet. > + default: > + return CXL_MBOX_UNSUPPORTED; > + } Similar. Return in all paths so no break. > + break; > + default: > + return CXL_MBOX_UNSUPPORTED; > + } > + > + return CXL_MBOX_SUCCESS; > +} > + > static CXLRetCode cmd_get_security_state(const struct cxl_cmd *cmd, > uint8_t *payload_in, > size_t len_in, > @@ -2864,6 +2986,10 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { > CXL_MBOX_SECURITY_STATE_CHANGE | > CXL_MBOX_BACKGROUND_OPERATION | > CXL_MBOX_BACKGROUND_OPERATION_ABORT)}, > + [SANITIZE][MEDIA_OPERATIONS] = { "MEDIA_OPERATIONS", cmd_media_operations, > + ~0, > + (CXL_MBOX_IMMEDIATE_DATA_CHANGE | > + CXL_MBOX_BACKGROUND_OPERATION)}, > [PERSISTENT_MEM][GET_SECURITY_STATE] = { "GET_SECURITY_STATE", > cmd_get_security_state, 0, 0 }, > [MEDIA_AND_POISON][GET_POISON_LIST] = { "MEDIA_AND_POISON_GET_POISON_LIST",
On 24/01/25 02:56PM, Jonathan Cameron wrote: >On Thu, 23 Jan 2025 10:39:02 +0530 >Vinayak Holikatti <vinayak.kh@samsung.com> wrote: > >Hi Vinayak, > >Thanks for your patch! Good to add support for this. > >Various comments inline, but all fairly minor things. > >thanks, > >Jonathan > > >> CXL spec 3.1 section 8.2.9.9.5.3 describes media operations commands. >> CXL devices supports media operations discovery command. > >Please don't indent the commit message. Maybe this is a side effect >of some tooling but definitely clean it up before sending a v2. > >> >> Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com> >+CC linux-cxl to increase chance of review and let people know this >exists. > >> --- >> hw/cxl/cxl-mailbox-utils.c | 130 ++++++++++++++++++++++++++++++++++++- >> 1 file changed, 128 insertions(+), 2 deletions(-) >> >> diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c >> index 9c7ea5bc35..2315d07fb1 100644 >> --- a/hw/cxl/cxl-mailbox-utils.c >> +++ b/hw/cxl/cxl-mailbox-utils.c >> @@ -87,8 +87,9 @@ enum { >> #define GET_LSA 0x2 >> #define SET_LSA 0x3 >> SANITIZE = 0x44, >> - #define OVERWRITE 0x0 >> - #define SECURE_ERASE 0x1 >> + #define OVERWRITE 0x0 >> + #define SECURE_ERASE 0x1 >> + #define MEDIA_OPERATIONS 0x2 > >Trivial but I've given up trying to keep these aligned. >It's a fools game as the names get steadily longer. Ok Will Fix in V2 > >As such better to just leave the existing pair alone. > >> PERSISTENT_MEM = 0x45, >> #define GET_SECURITY_STATE 0x0 >> MEDIA_AND_POISON = 0x43, >> @@ -1721,6 +1722,127 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, >> return CXL_MBOX_BG_STARTED; >> } >> >> +enum { >> + MEDIA_OP_GENERAL = 0x0, >I'd name them so the field id explicit. > >MEDIA_OP_CLASS_GENERAL >etc Ok will fix in V2 > >> + MEDIA_OP_SANITIZE = 0x1, >> + MEDIA_OP_CLASS_MAX, >No comma on terminating entry. We don't want it to be easy to add >stuff after it. > >> +} MEDIA_OPERATION_CLASS; >The enum type is never used. So might as well keep it anonymous >like we do for other enums in this file. > >> + >> +enum { >> + MEDIA_OP_SUB_DISCOVERY = 0x0, >This set of class and subcalss is similar to the enum you add >the MEDIA_OPERATIONS define to above. >I'd take a similar strategy with > ok will fix >enum { > MEDIA_OP_CLASS_GENERAL = 0x0, > #define MEDIA_OP_GEN_SUBC_DISCOVERY 0x0 > MEDIA_OP_CLASS_SANITIZE = 0x1, > #define MEDIA_OP_SAN_SUBC_SANITIZE 0x0 > #define MEDIA_OP_SAN_SUBC_ZERO 0x1 > >or something like that. ok will fix >} >> + MEDIA_OP_SUB_SANITIZE = 0x0, >> + MEDIA_OP_SUB_ZERO = 0x1, >> + MEDIA_OP_SUB_CLASS_MAX >No need for SUB_CLASS_MAX as you don't seem to use it. > >> +} MEDIA_OPERATION_SUB_CLASS; >> + >> +struct media_op_supported_list_entry { >> + uint8_t media_op_class; >> + uint8_t media_op_subclass; >> +}; >> + >> +struct media_op_discovery_out_pl { >> + uint64_t dpa_range_granularity; >> + uint16_t total_supported_operations; >> + uint16_t num_of_supported_operations; >> + struct media_op_supported_list_entry entry[0]; >entry[] ok will change it in V2 > >which is the c spec defined way to do variable length last elements. >The [0] was I think a weird extension that we have moved away from. > >> +}; > >Not strictly necessary but I'd mark it packed as chances of future breakage >are high with a structure starting at byte 0xC. > ok will fix >> + >> +#define MAX_SUPPORTED_OPS 3 >I'd avoid explicit define for this and just use ARRAY_SIZE() on the >array of structures to find out. > >> +struct media_op_supported_list_entry media_op_matrix[MAX_SUPPORTED_OPS] = { > >Use the defines above rather than the numeric values. >Then it's obvious what this is, also mark it static const. > ok will fix >static const struct media_op_supported_list_entry media_op_matrix[] = > { MEDIA_OP_CLASS_GENERAL, MEDIA_OP_GEN_SUBC_DISCOVERY }, > { MEDIA_OP_CLASS_SANITIZE, MEDIA_OP_SAN_SUBC_SANITIZE }, > { MEDIA_OP_CLASS_SANITIZE, MEDIA_OP_SAN_SUBC_ZERO }, >}; > >> + {0, 0}, >> + {1, 0}, >> + {1, 1} }; >> + >> +static CXLRetCode cmd_media_operations(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 media_operation_class; > struct { > uint8_t media_operation_class; > >etc for alignment. > >> + uint8_t media_operation_subclass; >> + uint8_t rsvd[2]; >> + uint32_t dpa_range_count; >> + union { >> + struct { >> + uint64_t starting_dpa; >> + uint64_t length; >> + } dpa_range_list[0]; >[] here its under union, compiler errors if not given the [0] number of element. May be try some other way > >> + struct { >> + uint16_t start_index; >> + uint16_t num_supported_ops; >> + } discovery_osa; >> + }; > >This is a little tricky as in theory you can have a variable number >of DPA Range List elements and then the operation specific arguments. > >However, general always provides a range count of 0. Also both sanitize >and zero have no osa elemetns. Add a comment >about this so we don't think it looks wrong in future + do notice that >this approach doesn't generalize if a new operation allows dpa ranges >and operation specific parameters. > Will add approriate comment > >> + } QEMU_PACKED *media_op_in_pl = (void *)payload_in; >> + >> + uint8_t media_op_cl = media_op_in_pl->media_operation_class; >> + uint8_t media_op_subclass = media_op_in_pl->media_operation_subclass; >> + uint32_t dpa_range_count = media_op_in_pl->dpa_range_count; >> + >> + if (len_in < sizeof(*media_op_in_pl)) { >> + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; >> + } > >Test this before getting values to fill in media_op_cl local variables etc. >It's both logically correct and may constrain the compiler not to get too smart >if it can see enough to realize what len_in is. > ok >> + >> + switch (media_op_cl) { >> + case MEDIA_OP_GENERAL: >> + switch (media_op_subclass) { >> + case MEDIA_OP_SUB_DISCOVERY: >Given there is only one element, maybe cleaner as > if (media_op_subclass != MEDIA_OP_SUB_DISCOVERY) { > return CXL_MBOX_UNSUPPORTED; > } >AS reduces indent of the following, helping readability a litle. ok > >> + int count = 0; >> + struct media_op_discovery_out_pl *media_out_pl = >> + (void *)payload_out; >> + int num_ops = media_op_in_pl->discovery_osa.num_supported_ops; >> + int start_index = media_op_in_pl->discovery_osa.start_index; >> + >> + /* As per spec CXL 3.1 8.2.9.9.5.3 dpa_range_count */ >> + /* should be zero for discovery sub class command */ >Local style is multiline comment as > /* > * As per spec CXL 3.1... > * should be zero... > */ > ok >> + if (dpa_range_count) { >> + return CXL_MBOX_INVALID_INPUT; >> + } >> + >> + if ((start_index >= MEDIA_OP_CLASS_MAX) || >> + (num_ops > MAX_SUPPORTED_OPS)) { > >Check here should be for num_ops + start_index > MAX_SUPPORTED OPS >Comparing start_index against MEDIA_OP_CLASS_MAX doesn't make sense to me >as I believe it's an index into the array of Class / subclass pairs not >the class array. > ok > >> + return CXL_MBOX_INVALID_INPUT; >> + } >> + >> + media_out_pl->dpa_range_granularity = CXL_CAPACITY_MULTIPLIER; >> + media_out_pl->total_supported_operations = MAX_SUPPORTED_OPS; >> + if (num_ops > 0) { >> + for (int i = start_index; i < MAX_SUPPORTED_OPS; i++) { >> + media_out_pl->entry[count].media_op_class = >> + media_op_matrix[i].media_op_class; >> + media_out_pl->entry[count].media_op_subclass = >> + media_op_matrix[i].media_op_subclass; >> + count++; >> + if (count == num_ops) { >> + goto disc_out; > >break should be enough and removes need for goto and label. > ok >> + } >> + } >> + } >> +disc_out: >> + media_out_pl->num_of_supported_operations = count; >> + *len_out = sizeof(struct media_op_discovery_out_pl) + >> + (sizeof(struct media_op_supported_list_entry) * count); > >indent this line. > ok >> + break; >I'd > return CXL_MBOX_SUCCESS; > >> + default: >> + return CXL_MBOX_UNSUPPORTED; >> + } >> + break; >then this break isn't needed. ok >> + case MEDIA_OP_SANITIZE: >> + switch (media_op_subclass) { >> + >No blank line here yet. >> + default: >> + return CXL_MBOX_UNSUPPORTED; >> + } >Similar. Return in all paths so no break. ok >> + break; >> + default: >> + return CXL_MBOX_UNSUPPORTED; >> + } >> + >> + return CXL_MBOX_SUCCESS; >> +} >> + >> static CXLRetCode cmd_get_security_state(const struct cxl_cmd *cmd, >> uint8_t *payload_in, >> size_t len_in, >> @@ -2864,6 +2986,10 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { >> CXL_MBOX_SECURITY_STATE_CHANGE | >> CXL_MBOX_BACKGROUND_OPERATION | >> CXL_MBOX_BACKGROUND_OPERATION_ABORT)}, >> + [SANITIZE][MEDIA_OPERATIONS] = { "MEDIA_OPERATIONS", cmd_media_operations, >> + ~0, >> + (CXL_MBOX_IMMEDIATE_DATA_CHANGE | >> + CXL_MBOX_BACKGROUND_OPERATION)}, >> [PERSISTENT_MEM][GET_SECURITY_STATE] = { "GET_SECURITY_STATE", >> cmd_get_security_state, 0, 0 }, >> [MEDIA_AND_POISON][GET_POISON_LIST] = { "MEDIA_AND_POISON_GET_POISON_LIST", >
diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 9c7ea5bc35..2315d07fb1 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -87,8 +87,9 @@ enum { #define GET_LSA 0x2 #define SET_LSA 0x3 SANITIZE = 0x44, - #define OVERWRITE 0x0 - #define SECURE_ERASE 0x1 + #define OVERWRITE 0x0 + #define SECURE_ERASE 0x1 + #define MEDIA_OPERATIONS 0x2 PERSISTENT_MEM = 0x45, #define GET_SECURITY_STATE 0x0 MEDIA_AND_POISON = 0x43, @@ -1721,6 +1722,127 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, return CXL_MBOX_BG_STARTED; } +enum { + MEDIA_OP_GENERAL = 0x0, + MEDIA_OP_SANITIZE = 0x1, + MEDIA_OP_CLASS_MAX, +} MEDIA_OPERATION_CLASS; + +enum { + MEDIA_OP_SUB_DISCOVERY = 0x0, + MEDIA_OP_SUB_SANITIZE = 0x0, + MEDIA_OP_SUB_ZERO = 0x1, + MEDIA_OP_SUB_CLASS_MAX +} MEDIA_OPERATION_SUB_CLASS; + +struct media_op_supported_list_entry { + uint8_t media_op_class; + uint8_t media_op_subclass; +}; + +struct media_op_discovery_out_pl { + uint64_t dpa_range_granularity; + uint16_t total_supported_operations; + uint16_t num_of_supported_operations; + struct media_op_supported_list_entry entry[0]; +}; + +#define MAX_SUPPORTED_OPS 3 +struct media_op_supported_list_entry media_op_matrix[MAX_SUPPORTED_OPS] = { + {0, 0}, + {1, 0}, + {1, 1} }; + +static CXLRetCode cmd_media_operations(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 media_operation_class; + uint8_t media_operation_subclass; + uint8_t rsvd[2]; + uint32_t dpa_range_count; + union { + struct { + uint64_t starting_dpa; + uint64_t length; + } dpa_range_list[0]; + struct { + uint16_t start_index; + uint16_t num_supported_ops; + } discovery_osa; + }; + } QEMU_PACKED *media_op_in_pl = (void *)payload_in; + + uint8_t media_op_cl = media_op_in_pl->media_operation_class; + uint8_t media_op_subclass = media_op_in_pl->media_operation_subclass; + uint32_t dpa_range_count = media_op_in_pl->dpa_range_count; + + if (len_in < sizeof(*media_op_in_pl)) { + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; + } + + switch (media_op_cl) { + case MEDIA_OP_GENERAL: + switch (media_op_subclass) { + case MEDIA_OP_SUB_DISCOVERY: + int count = 0; + struct media_op_discovery_out_pl *media_out_pl = + (void *)payload_out; + int num_ops = media_op_in_pl->discovery_osa.num_supported_ops; + int start_index = media_op_in_pl->discovery_osa.start_index; + + /* As per spec CXL 3.1 8.2.9.9.5.3 dpa_range_count */ + /* should be zero for discovery sub class command */ + if (dpa_range_count) { + return CXL_MBOX_INVALID_INPUT; + } + + if ((start_index >= MEDIA_OP_CLASS_MAX) || + (num_ops > MAX_SUPPORTED_OPS)) { + return CXL_MBOX_INVALID_INPUT; + } + + media_out_pl->dpa_range_granularity = CXL_CAPACITY_MULTIPLIER; + media_out_pl->total_supported_operations = MAX_SUPPORTED_OPS; + if (num_ops > 0) { + for (int i = start_index; i < MAX_SUPPORTED_OPS; i++) { + media_out_pl->entry[count].media_op_class = + media_op_matrix[i].media_op_class; + media_out_pl->entry[count].media_op_subclass = + media_op_matrix[i].media_op_subclass; + count++; + if (count == num_ops) { + goto disc_out; + } + } + } +disc_out: + media_out_pl->num_of_supported_operations = count; + *len_out = sizeof(struct media_op_discovery_out_pl) + + (sizeof(struct media_op_supported_list_entry) * count); + break; + default: + return CXL_MBOX_UNSUPPORTED; + } + break; + case MEDIA_OP_SANITIZE: + switch (media_op_subclass) { + + default: + return CXL_MBOX_UNSUPPORTED; + } + break; + default: + return CXL_MBOX_UNSUPPORTED; + } + + return CXL_MBOX_SUCCESS; +} + static CXLRetCode cmd_get_security_state(const struct cxl_cmd *cmd, uint8_t *payload_in, size_t len_in, @@ -2864,6 +2986,10 @@ static const struct cxl_cmd cxl_cmd_set[256][256] = { CXL_MBOX_SECURITY_STATE_CHANGE | CXL_MBOX_BACKGROUND_OPERATION | CXL_MBOX_BACKGROUND_OPERATION_ABORT)}, + [SANITIZE][MEDIA_OPERATIONS] = { "MEDIA_OPERATIONS", cmd_media_operations, + ~0, + (CXL_MBOX_IMMEDIATE_DATA_CHANGE | + CXL_MBOX_BACKGROUND_OPERATION)}, [PERSISTENT_MEM][GET_SECURITY_STATE] = { "GET_SECURITY_STATE", cmd_get_security_state, 0, 0 }, [MEDIA_AND_POISON][GET_POISON_LIST] = { "MEDIA_AND_POISON_GET_POISON_LIST",
CXL spec 3.1 section 8.2.9.9.5.3 describes media operations commands. CXL devices supports media operations discovery command. Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com> --- hw/cxl/cxl-mailbox-utils.c | 130 ++++++++++++++++++++++++++++++++++++- 1 file changed, 128 insertions(+), 2 deletions(-)