* [PATCH v3 0/3] CXL CCI Media Operations [not found] <CGME20250220052733epcas5p41f847e5d0233a62703693565ee37374c@epcas5p4.samsung.com> @ 2025-02-20 5:27 ` Vinayak Holikatti 2025-02-20 5:27 ` [PATCH v3 1/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations discovery commands cxl r3.2 (8.2.10.9.5.3) Vinayak Holikatti ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Vinayak Holikatti @ 2025-02-20 5:27 UTC (permalink / raw) To: qemu-devel Cc: gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore, Vinayak Holikatti CXL CCI media operations commands implmented as per CXL Specification 3.2 8.2.10.9.5.3 1) General(00h) - Discovery (00h) 2) Sanitize(01h - Sanitize (00h) Write zeros (01h) The patches are generated against the Johnathan's tree https://gitlab.com/jic23/qemu.git and branch cxl-2024-11-27. Changes V2 -> V3 - Addressed review comments from Jonathan Cameron. - Updated CXL Spec r3.2 references Changes V1->V2 - Addressed the review comments from Jonathan Cameron. - Modularied the media operations class & subclass handling into separate functions. Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com> Vinayak Holikatti (3): hw/cxl/cxl-mailbox-utils: Add support for Media operations discovery commands cxl r3.2 (8.2.10.9.5.3) hw/cxl: factor out calculation of sanitize duration from cmd_santize_overwrite hw/cxl/cxl-mailbox-utils: Add support for Media operations Sanitize and Write Zeros commands cxl r3.2(8.2.10.9.5.3) hw/cxl/cxl-mailbox-utils.c | 402 +++++++++++++++++++++++++++++++++--- include/hw/cxl/cxl_device.h | 4 + 2 files changed, 380 insertions(+), 26 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations discovery commands cxl r3.2 (8.2.10.9.5.3) 2025-02-20 5:27 ` [PATCH v3 0/3] CXL CCI Media Operations Vinayak Holikatti @ 2025-02-20 5:27 ` Vinayak Holikatti 2025-02-20 15:10 ` Jonathan Cameron via 2025-02-20 5:27 ` [PATCH v3 2/3] hw/cxl: factor out calculation of sanitize duration from cmd_santize_overwrite Vinayak Holikatti 2025-02-20 5:27 ` [PATCH v3 3/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations Sanitize and Write Zeros commands cxl r3.2(8.2.10.9.5.3) Vinayak Holikatti 2 siblings, 1 reply; 10+ messages in thread From: Vinayak Holikatti @ 2025-02-20 5:27 UTC (permalink / raw) To: qemu-devel Cc: gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore, Vinayak Holikatti CXL spec 3.2 section 8.2.10.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 | 133 +++++++++++++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 9c7ea5bc35..f55a2fe614 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -89,6 +89,7 @@ enum { SANITIZE = 0x44, #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,134 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, return CXL_MBOX_BG_STARTED; } +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 + MEDIA_OP_CLASS_MAX +}; + +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[]; +} QEMU_PACKED; + +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 }, +}; + +static CXLRetCode media_operations_discovery(uint8_t *payload_in, + size_t len_in, + uint8_t *payload_out, + size_t *len_out) +{ + struct { + uint8_t media_operation_class; + uint8_t media_operation_subclass; + uint8_t rsvd[2]; + uint32_t dpa_range_count; + struct { + uint16_t start_index; + uint16_t num_ops; + } discovery_osa; + } QEMU_PACKED *media_op_in_disc_pl = (void *)payload_in; + int count = 0; + int num_ops = 0; + int start_index = 0; + int i = 0; + uint32_t dpa_range_count = 0; + struct media_op_discovery_out_pl *media_out_pl = NULL; + + if (len_in < sizeof(*media_op_in_disc_pl)) { + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; + } + + media_out_pl = (struct media_op_discovery_out_pl *)payload_out; + num_ops = media_op_in_disc_pl->discovery_osa.num_ops; + start_index = media_op_in_disc_pl->discovery_osa.start_index; + dpa_range_count = media_op_in_disc_pl->dpa_range_count; + + /* + * As per spec CXL 3.2 8.2.10.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 + num_ops > ARRAY_SIZE(media_op_matrix)) { + return CXL_MBOX_INVALID_INPUT; + } + + media_out_pl->dpa_range_granularity = CXL_CACHE_LINE_SIZE; + media_out_pl->total_supported_operations = + ARRAY_SIZE(media_op_matrix); + if (num_ops > 0) { + for (i = start_index; i < start_index + num_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) { + break; + } + } + } + + 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); + return CXL_MBOX_SUCCESS; +} + +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; + } QEMU_PACKED *media_op_in_common_pl = (void *)payload_in; + uint8_t media_op_cl = 0; + uint8_t media_op_subclass = 0; + + if (len_in < sizeof(*media_op_in_common_pl)) { + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; + } + + media_op_cl = media_op_in_common_pl->media_operation_class; + media_op_subclass = media_op_in_common_pl->media_operation_subclass; + + switch (media_op_cl) { + case MEDIA_OP_CLASS_GENERAL: + if (media_op_subclass != MEDIA_OP_GEN_SUBC_DISCOVERY) { + return CXL_MBOX_UNSUPPORTED; + } + + return media_operations_discovery(payload_in, len_in, payload_out, + len_out); + default: + return CXL_MBOX_UNSUPPORTED; + } +} + static CXLRetCode cmd_get_security_state(const struct cxl_cmd *cmd, uint8_t *payload_in, size_t len_in, @@ -2864,6 +2993,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", -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations discovery commands cxl r3.2 (8.2.10.9.5.3) 2025-02-20 5:27 ` [PATCH v3 1/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations discovery commands cxl r3.2 (8.2.10.9.5.3) Vinayak Holikatti @ 2025-02-20 15:10 ` Jonathan Cameron via 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2025-02-20 15:10 UTC (permalink / raw) To: Vinayak Holikatti Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore On Thu, 20 Feb 2025 10:57:22 +0530 Vinayak Holikatti <vinayak.kh@samsung.com> wrote: > CXL spec 3.2 section 8.2.10.9.5.3 describes media operations commands. > CXL devices supports media operations discovery command. > > Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com> Hi Vinayak, Rather than go around again, I've applied this to my CXL staging tree with the following diff (comments inline below!) Let me know if I messed up the changes (it wouldn't be the first time :() Jonathan diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index d401c68a38..167a87a7b1 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -1709,7 +1709,6 @@ enum { MEDIA_OP_CLASS_SANITIZE = 0x1, #define MEDIA_OP_SAN_SUBC_SANITIZE 0x0 #define MEDIA_OP_SAN_SUBC_ZERO 0x1 - MEDIA_OP_CLASS_MAX }; struct media_op_supported_list_entry { @@ -1745,31 +1744,25 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in, uint16_t num_ops; } discovery_osa; } QEMU_PACKED *media_op_in_disc_pl = (void *)payload_in; + struct media_op_discovery_out_pl *media_out_pl = + (struct media_op_discovery_out_pl *)payload_out; + int num_ops, start_index, i; int count = 0; - int num_ops = 0; - int start_index = 0; - int i = 0; - uint32_t dpa_range_count = 0; - struct media_op_discovery_out_pl *media_out_pl = NULL; if (len_in < sizeof(*media_op_in_disc_pl)) { return CXL_MBOX_INVALID_PAYLOAD_LENGTH; } - media_out_pl = (struct media_op_discovery_out_pl *)payload_out; num_ops = media_op_in_disc_pl->discovery_osa.num_ops; start_index = media_op_in_disc_pl->discovery_osa.start_index; - dpa_range_count = media_op_in_disc_pl->dpa_range_count; /* - * As per spec CXL 3.2 8.2.10.9.5.3 dpa_range_count - * should be zero for discovery sub class command + * As per spec CXL r3.2 8.2.10.9.5.3 dpa_range_count should be zero and start + * index should not exceed the total number of entries for discovery sub + * class command. */ - if (dpa_range_count) { - return CXL_MBOX_INVALID_INPUT; - } - - if (start_index + num_ops > ARRAY_SIZE(media_op_matrix)) { + if (media_op_in_disc_pl->dpa_range_count || + start_index > ARRAY_SIZE(media_op_matrix)) { return CXL_MBOX_INVALID_INPUT; } @@ -1790,8 +1783,7 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in, } 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); + *len_out = sizeof(*media_out_pl) + count * sizeof(*media_out_pl->entry); return CXL_MBOX_SUCCESS; } > --- > hw/cxl/cxl-mailbox-utils.c | 133 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 133 insertions(+) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 9c7ea5bc35..f55a2fe614 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -89,6 +89,7 @@ enum { > SANITIZE = 0x44, > #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,134 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, > return CXL_MBOX_BG_STARTED; > } > > +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 > + MEDIA_OP_CLASS_MAX Not used. So I'll drop this last entry. We can bring it back easily when we need it. > +}; > + > +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[]; > +} QEMU_PACKED; > + > +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 }, > +}; > + > +static CXLRetCode media_operations_discovery(uint8_t *payload_in, > + size_t len_in, > + uint8_t *payload_out, > + size_t *len_out) > +{ > + struct { > + uint8_t media_operation_class; > + uint8_t media_operation_subclass; > + uint8_t rsvd[2]; > + uint32_t dpa_range_count; > + struct { > + uint16_t start_index; > + uint16_t num_ops; > + } discovery_osa; > + } QEMU_PACKED *media_op_in_disc_pl = (void *)payload_in; > + int count = 0; > + int num_ops = 0; > + int start_index = 0; > + int i = 0; These values are always overwritten so no need to init. > + uint32_t dpa_range_count = 0; This one is only used once so I'll switch to just using the data directly. > + struct media_op_discovery_out_pl *media_out_pl = NULL; Might as well set this initially. > + > + if (len_in < sizeof(*media_op_in_disc_pl)) { > + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; > + } > + > + media_out_pl = (struct media_op_discovery_out_pl *)payload_out; > + num_ops = media_op_in_disc_pl->discovery_osa.num_ops; > + start_index = media_op_in_disc_pl->discovery_osa.start_index; > + dpa_range_count = media_op_in_disc_pl->dpa_range_count; > + > + /* > + * As per spec CXL 3.2 8.2.10.9.5.3 dpa_range_count For local consistency used r3.2 I theory the spec has a revision and a version though published specs are all version 1.0 > + * should be zero for discovery sub class command > + */ > + if (dpa_range_count) { > + return CXL_MBOX_INVALID_INPUT; > + } > + > + if (start_index + num_ops > ARRAY_SIZE(media_op_matrix)) { > + return CXL_MBOX_INVALID_INPUT; I've read the spec again and think I was wrong last time :( I believe it is only the start_index that we need to check., If the end is beyond what was requested we just return fewer. Sorry about that! I'll fix that up and change the comment above to also talk about this. As these two checks are based on same sentence in the spec I'll combine them using || > + } > + > + media_out_pl->dpa_range_granularity = CXL_CACHE_LINE_SIZE; > + media_out_pl->total_supported_operations = > + ARRAY_SIZE(media_op_matrix); > + if (num_ops > 0) { > + for (i = start_index; i < start_index + num_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) { > + break; > + } > + } > + } > + > + 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); For this we can use the more compact *len_out = sizeof(*media_out_pl) + sizeof(*media_out_pl->entry) * count; > + return CXL_MBOX_SUCCESS; > +} > + > +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; > + } QEMU_PACKED *media_op_in_common_pl = (void *)payload_in; > + uint8_t media_op_cl = 0; > + uint8_t media_op_subclass = 0; > + > + if (len_in < sizeof(*media_op_in_common_pl)) { > + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; > + } > + > + media_op_cl = media_op_in_common_pl->media_operation_class; > + media_op_subclass = media_op_in_common_pl->media_operation_subclass; > + > + switch (media_op_cl) { > + case MEDIA_OP_CLASS_GENERAL: > + if (media_op_subclass != MEDIA_OP_GEN_SUBC_DISCOVERY) { > + return CXL_MBOX_UNSUPPORTED; > + } > + > + return media_operations_discovery(payload_in, len_in, payload_out, > + len_out); > + default: > + return CXL_MBOX_UNSUPPORTED; > + } > +} > + > static CXLRetCode cmd_get_security_state(const struct cxl_cmd *cmd, > uint8_t *payload_in, > size_t len_in, > @@ -2864,6 +2993,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", ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 1/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations discovery commands cxl r3.2 (8.2.10.9.5.3) @ 2025-02-20 15:10 ` Jonathan Cameron via 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron via @ 2025-02-20 15:10 UTC (permalink / raw) To: Vinayak Holikatti Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore On Thu, 20 Feb 2025 10:57:22 +0530 Vinayak Holikatti <vinayak.kh@samsung.com> wrote: > CXL spec 3.2 section 8.2.10.9.5.3 describes media operations commands. > CXL devices supports media operations discovery command. > > Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com> Hi Vinayak, Rather than go around again, I've applied this to my CXL staging tree with the following diff (comments inline below!) Let me know if I messed up the changes (it wouldn't be the first time :() Jonathan diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index d401c68a38..167a87a7b1 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -1709,7 +1709,6 @@ enum { MEDIA_OP_CLASS_SANITIZE = 0x1, #define MEDIA_OP_SAN_SUBC_SANITIZE 0x0 #define MEDIA_OP_SAN_SUBC_ZERO 0x1 - MEDIA_OP_CLASS_MAX }; struct media_op_supported_list_entry { @@ -1745,31 +1744,25 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in, uint16_t num_ops; } discovery_osa; } QEMU_PACKED *media_op_in_disc_pl = (void *)payload_in; + struct media_op_discovery_out_pl *media_out_pl = + (struct media_op_discovery_out_pl *)payload_out; + int num_ops, start_index, i; int count = 0; - int num_ops = 0; - int start_index = 0; - int i = 0; - uint32_t dpa_range_count = 0; - struct media_op_discovery_out_pl *media_out_pl = NULL; if (len_in < sizeof(*media_op_in_disc_pl)) { return CXL_MBOX_INVALID_PAYLOAD_LENGTH; } - media_out_pl = (struct media_op_discovery_out_pl *)payload_out; num_ops = media_op_in_disc_pl->discovery_osa.num_ops; start_index = media_op_in_disc_pl->discovery_osa.start_index; - dpa_range_count = media_op_in_disc_pl->dpa_range_count; /* - * As per spec CXL 3.2 8.2.10.9.5.3 dpa_range_count - * should be zero for discovery sub class command + * As per spec CXL r3.2 8.2.10.9.5.3 dpa_range_count should be zero and start + * index should not exceed the total number of entries for discovery sub + * class command. */ - if (dpa_range_count) { - return CXL_MBOX_INVALID_INPUT; - } - - if (start_index + num_ops > ARRAY_SIZE(media_op_matrix)) { + if (media_op_in_disc_pl->dpa_range_count || + start_index > ARRAY_SIZE(media_op_matrix)) { return CXL_MBOX_INVALID_INPUT; } @@ -1790,8 +1783,7 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in, } 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); + *len_out = sizeof(*media_out_pl) + count * sizeof(*media_out_pl->entry); return CXL_MBOX_SUCCESS; } > --- > hw/cxl/cxl-mailbox-utils.c | 133 +++++++++++++++++++++++++++++++++++++ > 1 file changed, 133 insertions(+) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 9c7ea5bc35..f55a2fe614 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -89,6 +89,7 @@ enum { > SANITIZE = 0x44, > #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,134 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, > return CXL_MBOX_BG_STARTED; > } > > +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 > + MEDIA_OP_CLASS_MAX Not used. So I'll drop this last entry. We can bring it back easily when we need it. > +}; > + > +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[]; > +} QEMU_PACKED; > + > +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 }, > +}; > + > +static CXLRetCode media_operations_discovery(uint8_t *payload_in, > + size_t len_in, > + uint8_t *payload_out, > + size_t *len_out) > +{ > + struct { > + uint8_t media_operation_class; > + uint8_t media_operation_subclass; > + uint8_t rsvd[2]; > + uint32_t dpa_range_count; > + struct { > + uint16_t start_index; > + uint16_t num_ops; > + } discovery_osa; > + } QEMU_PACKED *media_op_in_disc_pl = (void *)payload_in; > + int count = 0; > + int num_ops = 0; > + int start_index = 0; > + int i = 0; These values are always overwritten so no need to init. > + uint32_t dpa_range_count = 0; This one is only used once so I'll switch to just using the data directly. > + struct media_op_discovery_out_pl *media_out_pl = NULL; Might as well set this initially. > + > + if (len_in < sizeof(*media_op_in_disc_pl)) { > + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; > + } > + > + media_out_pl = (struct media_op_discovery_out_pl *)payload_out; > + num_ops = media_op_in_disc_pl->discovery_osa.num_ops; > + start_index = media_op_in_disc_pl->discovery_osa.start_index; > + dpa_range_count = media_op_in_disc_pl->dpa_range_count; > + > + /* > + * As per spec CXL 3.2 8.2.10.9.5.3 dpa_range_count For local consistency used r3.2 I theory the spec has a revision and a version though published specs are all version 1.0 > + * should be zero for discovery sub class command > + */ > + if (dpa_range_count) { > + return CXL_MBOX_INVALID_INPUT; > + } > + > + if (start_index + num_ops > ARRAY_SIZE(media_op_matrix)) { > + return CXL_MBOX_INVALID_INPUT; I've read the spec again and think I was wrong last time :( I believe it is only the start_index that we need to check., If the end is beyond what was requested we just return fewer. Sorry about that! I'll fix that up and change the comment above to also talk about this. As these two checks are based on same sentence in the spec I'll combine them using || > + } > + > + media_out_pl->dpa_range_granularity = CXL_CACHE_LINE_SIZE; > + media_out_pl->total_supported_operations = > + ARRAY_SIZE(media_op_matrix); > + if (num_ops > 0) { > + for (i = start_index; i < start_index + num_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) { > + break; > + } > + } > + } > + > + 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); For this we can use the more compact *len_out = sizeof(*media_out_pl) + sizeof(*media_out_pl->entry) * count; > + return CXL_MBOX_SUCCESS; > +} > + > +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; > + } QEMU_PACKED *media_op_in_common_pl = (void *)payload_in; > + uint8_t media_op_cl = 0; > + uint8_t media_op_subclass = 0; > + > + if (len_in < sizeof(*media_op_in_common_pl)) { > + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; > + } > + > + media_op_cl = media_op_in_common_pl->media_operation_class; > + media_op_subclass = media_op_in_common_pl->media_operation_subclass; > + > + switch (media_op_cl) { > + case MEDIA_OP_CLASS_GENERAL: > + if (media_op_subclass != MEDIA_OP_GEN_SUBC_DISCOVERY) { > + return CXL_MBOX_UNSUPPORTED; > + } > + > + return media_operations_discovery(payload_in, len_in, payload_out, > + len_out); > + default: > + return CXL_MBOX_UNSUPPORTED; > + } > +} > + > static CXLRetCode cmd_get_security_state(const struct cxl_cmd *cmd, > uint8_t *payload_in, > size_t len_in, > @@ -2864,6 +2993,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", ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/3] hw/cxl: factor out calculation of sanitize duration from cmd_santize_overwrite 2025-02-20 5:27 ` [PATCH v3 0/3] CXL CCI Media Operations Vinayak Holikatti 2025-02-20 5:27 ` [PATCH v3 1/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations discovery commands cxl r3.2 (8.2.10.9.5.3) Vinayak Holikatti @ 2025-02-20 5:27 ` Vinayak Holikatti 2025-03-05 8:49 ` Jonathan Cameron via 2025-02-20 5:27 ` [PATCH v3 3/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations Sanitize and Write Zeros commands cxl r3.2(8.2.10.9.5.3) Vinayak Holikatti 2 siblings, 1 reply; 10+ messages in thread From: Vinayak Holikatti @ 2025-02-20 5:27 UTC (permalink / raw) To: qemu-devel Cc: gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore, Vinayak Holikatti Move the code of calculation of sanitize duration into function for usability by other sanitize routines Estimate times based on: https://pmem.io/documents/NVDIMM_DSM_Interface-V1.8.pdf Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com> --- hw/cxl/cxl-mailbox-utils.c | 61 ++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index f55a2fe614..2428d85fef 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -1656,34 +1656,10 @@ static void __do_sanitization(CXLType3Dev *ct3d) cxl_discard_all_event_records(&ct3d->cxl_dstate); } -/* - * CXL r3.1 Section 8.2.9.9.5.1: Sanitize (Opcode 4400h) - * - * Once the Sanitize command has started successfully, the device shall be - * placed in the media disabled state. If the command fails or is interrupted - * by a reset or power failure, it shall remain in the media disabled state - * until a successful Sanitize command has been completed. During this state: - * - * 1. Memory writes to the device will have no effect, and all memory reads - * will return random values (no user data returned, even for locations that - * the failed Sanitize operation didn’t sanitize yet). - * - * 2. Mailbox commands shall still be processed in the disabled state, except - * that commands that access Sanitized areas shall fail with the Media Disabled - * error code. - */ -static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, - uint8_t *payload_in, - size_t len_in, - uint8_t *payload_out, - size_t *len_out, - CXLCCI *cci) +static int get_sanitize_duration(uint64_t total_mem) { - CXLType3Dev *ct3d = CXL_TYPE3(cci->d); - uint64_t total_mem; /* in Mb */ - int secs; + int secs = 0; - total_mem = (ct3d->cxl_dstate.vmem_size + ct3d->cxl_dstate.pmem_size) >> 20; if (total_mem <= 512) { secs = 4; } else if (total_mem <= 1024) { @@ -1712,6 +1688,39 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, secs = 240 * 60; /* max 4 hrs */ } + return secs; +} + +/* + * CXL r3.1 Section 8.2.9.9.5.1: Sanitize (Opcode 4400h) + * + * Once the Sanitize command has started successfully, the device shall be + * placed in the media disabled state. If the command fails or is interrupted + * by a reset or power failure, it shall remain in the media disabled state + * until a successful Sanitize command has been completed. During this state: + * + * 1. Memory writes to the device will have no effect, and all memory reads + * will return random values (no user data returned, even for locations that + * the failed Sanitize operation didn’t sanitize yet). + * + * 2. Mailbox commands shall still be processed in the disabled state, except + * that commands that access Sanitized areas shall fail with the Media Disabled + * error code. + */ +static CXLRetCode cmd_sanitize_overwrite(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); + uint64_t total_mem; /* in Mb */ + int secs; + + total_mem = (ct3d->cxl_dstate.vmem_size + ct3d->cxl_dstate.pmem_size) >> 20; + secs = get_sanitize_duration(total_mem); + /* EBUSY other bg cmds as of now */ cci->bg.runtime = secs * 1000UL; *len_out = 0; -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] hw/cxl: factor out calculation of sanitize duration from cmd_santize_overwrite 2025-02-20 5:27 ` [PATCH v3 2/3] hw/cxl: factor out calculation of sanitize duration from cmd_santize_overwrite Vinayak Holikatti @ 2025-03-05 8:49 ` Jonathan Cameron via 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2025-03-05 8:49 UTC (permalink / raw) To: Vinayak Holikatti Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore On Thu, 20 Feb 2025 10:57:23 +0530 Vinayak Holikatti <vinayak.kh@samsung.com> wrote: > Move the code of calculation of sanitize duration into function > for usability by other sanitize routines > > Estimate times based on: > https://pmem.io/documents/NVDIMM_DSM_Interface-V1.8.pdf > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com> Hi. Authorship of this patch is confused. Should Davidlohr have a Co-developed-by tag? (I think those are used in qemu) If not should the From tag be set to Davidlohr? I'm going to post a series for potential merge without this fixed as I'd like some more review anyway, but good to clarify what is going on in this patch. Jonathan ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] hw/cxl: factor out calculation of sanitize duration from cmd_santize_overwrite @ 2025-03-05 8:49 ` Jonathan Cameron via 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron via @ 2025-03-05 8:49 UTC (permalink / raw) To: Vinayak Holikatti Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore On Thu, 20 Feb 2025 10:57:23 +0530 Vinayak Holikatti <vinayak.kh@samsung.com> wrote: > Move the code of calculation of sanitize duration into function > for usability by other sanitize routines > > Estimate times based on: > https://pmem.io/documents/NVDIMM_DSM_Interface-V1.8.pdf > > Signed-off-by: Davidlohr Bueso <dave@stgolabs.net> > Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com> Hi. Authorship of this patch is confused. Should Davidlohr have a Co-developed-by tag? (I think those are used in qemu) If not should the From tag be set to Davidlohr? I'm going to post a series for potential merge without this fixed as I'd like some more review anyway, but good to clarify what is going on in this patch. Jonathan ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 3/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations Sanitize and Write Zeros commands cxl r3.2(8.2.10.9.5.3) 2025-02-20 5:27 ` [PATCH v3 0/3] CXL CCI Media Operations Vinayak Holikatti 2025-02-20 5:27 ` [PATCH v3 1/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations discovery commands cxl r3.2 (8.2.10.9.5.3) Vinayak Holikatti 2025-02-20 5:27 ` [PATCH v3 2/3] hw/cxl: factor out calculation of sanitize duration from cmd_santize_overwrite Vinayak Holikatti @ 2025-02-20 5:27 ` Vinayak Holikatti 2025-02-20 15:10 ` Jonathan Cameron via 2 siblings, 1 reply; 10+ messages in thread From: Vinayak Holikatti @ 2025-02-20 5:27 UTC (permalink / raw) To: qemu-devel Cc: gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore, Vinayak Holikatti CXL spec 3.2 section 8.2.10.9.5.3 describes media operations commands. CXL devices supports media operations Sanitize and Write zero command. Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com> --- hw/cxl/cxl-mailbox-utils.c | 210 +++++++++++++++++++++++++++++++++++- include/hw/cxl/cxl_device.h | 4 + 2 files changed, 213 insertions(+), 1 deletion(-) diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 2428d85fef..7b4e4daeb2 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -1731,6 +1731,130 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, return CXL_MBOX_BG_STARTED; } +struct dpa_range_list_entry { + uint64_t starting_dpa; + uint64_t length; +} QEMU_PACKED; + +struct CXLSanitizeInfo { + uint32_t dpa_range_count; + uint8_t fill_value; + struct dpa_range_list_entry dpa_range_list[]; +} QEMU_PACKED; + +static uint64_t get_vmr_size(CXLType3Dev *ct3d, MemoryRegion **vmr) +{ + MemoryRegion *mr; + if (ct3d->hostvmem) { + mr = host_memory_backend_get_memory(ct3d->hostvmem); + if (vmr) { + *vmr = mr; + } + return memory_region_size(mr); + } + return 0; +} + +static uint64_t get_pmr_size(CXLType3Dev *ct3d, MemoryRegion **pmr) +{ + MemoryRegion *mr; + if (ct3d->hostpmem) { + mr = host_memory_backend_get_memory(ct3d->hostpmem); + if (pmr) { + *pmr = mr; + } + return memory_region_size(mr); + } + return 0; +} + +static uint64_t get_dc_size(CXLType3Dev *ct3d, MemoryRegion **dc_mr) +{ + MemoryRegion *mr; + if (ct3d->dc.host_dc) { + mr = host_memory_backend_get_memory(ct3d->dc.host_dc); + if (dc_mr) { + *dc_mr = mr; + } + return memory_region_size(mr); + } + return 0; +} + +static int validate_dpa_addr(CXLType3Dev *ct3d, uint64_t dpa_addr, + size_t length) +{ + uint64_t vmr_size, pmr_size, dc_size; + + if ((dpa_addr % CXL_CACHE_LINE_SIZE) || + (length % CXL_CACHE_LINE_SIZE) || + (length <= 0)) { + return -EINVAL; + } + + vmr_size = get_vmr_size(ct3d, NULL); + pmr_size = get_pmr_size(ct3d, NULL); + dc_size = get_dc_size(ct3d, NULL); + + if (dpa_addr + length > vmr_size + pmr_size + dc_size) { + return -EINVAL; + } + + if (dpa_addr > vmr_size + pmr_size) { + if (!ct3_test_region_block_backed(ct3d, dpa_addr, length)) { + return -ENODEV; + } + } + + return 0; +} + +static int sanitize_range(CXLType3Dev *ct3d, uint64_t dpa_addr, size_t length, + uint8_t fill_value) +{ + + uint64_t vmr_size, pmr_size; + AddressSpace *as = NULL; + MemTxAttrs mem_attrs = {0}; + + vmr_size = get_vmr_size(ct3d, NULL); + pmr_size = get_pmr_size(ct3d, NULL); + + if (dpa_addr < vmr_size) { + as = &ct3d->hostvmem_as; + } else if (dpa_addr < vmr_size + pmr_size) { + as = &ct3d->hostpmem_as; + } else { + if (!ct3_test_region_block_backed(ct3d, dpa_addr, length)) { + return -ENODEV; + } + as = &ct3d->dc.host_dc_as; + } + + return address_space_set(as, dpa_addr, fill_value, length, mem_attrs); +} + +/* Perform the actual device zeroing */ +static void __do_sanitize(CXLType3Dev *ct3d) +{ + struct CXLSanitizeInfo *san_info = ct3d->media_op_sanitize; + int dpa_range_count = san_info->dpa_range_count; + int rc = 0, i; + + for (i = 0; i < dpa_range_count; i++) { + rc = sanitize_range(ct3d, san_info->dpa_range_list[i].starting_dpa, + san_info->dpa_range_list[i].length, + san_info->fill_value); + if (rc) { + goto exit; + } + } +exit: + g_free(ct3d->media_op_sanitize); + ct3d->media_op_sanitize = NULL; + return; +} + enum { MEDIA_OP_CLASS_GENERAL = 0x0, #define MEDIA_OP_GEN_SUBC_DISCOVERY 0x0 @@ -1823,6 +1947,70 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in, return CXL_MBOX_SUCCESS; } +static CXLRetCode media_operations_sanitize(CXLType3Dev *ct3d, + uint8_t *payload_in, + size_t len_in, + uint8_t *payload_out, + size_t *len_out, + uint8_t fill_value, + CXLCCI *cci) +{ + struct media_operations_sanitize { + uint8_t media_operation_class; + uint8_t media_operation_subclass; + uint8_t rsvd[2]; + uint32_t dpa_range_count; + struct { + uint64_t starting_dpa; + uint64_t length; + } dpa_range_list[]; + } QEMU_PACKED *media_op_in_sanitize_pl = (void *)payload_in; + uint32_t dpa_range_count = media_op_in_sanitize_pl->dpa_range_count; + uint64_t total_mem = 0; + int secs = 0, i; + + if (dpa_range_count == 0) { + return CXL_MBOX_SUCCESS; + } + + if (len_in < (sizeof(*media_op_in_sanitize_pl) + + (dpa_range_count * sizeof(struct dpa_range_list_entry)))) { + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; + } + + for (i = 0; i < dpa_range_count; i++) { + uint64_t start_dpa = + media_op_in_sanitize_pl->dpa_range_list[i].starting_dpa; + uint64_t length = media_op_in_sanitize_pl->dpa_range_list[i].length; + if (validate_dpa_addr(ct3d, start_dpa, length)) { + return CXL_MBOX_INVALID_INPUT; + } + total_mem += media_op_in_sanitize_pl->dpa_range_list[i].length; + } + ct3d->media_op_sanitize = g_malloc0(sizeof(struct CXLSanitizeInfo) + + (dpa_range_count * + sizeof(struct dpa_range_list_entry))); + + if (ct3d->media_op_sanitize) { + ct3d->media_op_sanitize->dpa_range_count = dpa_range_count; + ct3d->media_op_sanitize->fill_value = fill_value; + memcpy(ct3d->media_op_sanitize->dpa_range_list, + media_op_in_sanitize_pl->dpa_range_list, + (dpa_range_count * + sizeof(struct dpa_range_list_entry))); + secs = get_sanitize_duration(total_mem >> 20); + } + + /* EBUSY other bg cmds as of now */ + cci->bg.runtime = secs * 1000UL; + *len_out = 0; + /* + * media op sanitize is targeted so no need to disable media or + * clear event logs + */ + return CXL_MBOX_BG_STARTED; +} + static CXLRetCode cmd_media_operations(const struct cxl_cmd *cmd, uint8_t *payload_in, size_t len_in, @@ -1836,6 +2024,7 @@ static CXLRetCode cmd_media_operations(const struct cxl_cmd *cmd, uint8_t rsvd[2]; uint32_t dpa_range_count; } QEMU_PACKED *media_op_in_common_pl = (void *)payload_in; + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); uint8_t media_op_cl = 0; uint8_t media_op_subclass = 0; @@ -1853,7 +2042,20 @@ static CXLRetCode cmd_media_operations(const struct cxl_cmd *cmd, } return media_operations_discovery(payload_in, len_in, payload_out, - len_out); + len_out); + case MEDIA_OP_CLASS_SANITIZE: + switch (media_op_subclass) { + case MEDIA_OP_SAN_SUBC_SANITIZE: + return media_operations_sanitize(ct3d, payload_in, len_in, + payload_out, len_out, 0xF, + cci); + case MEDIA_OP_SAN_SUBC_ZERO: + return media_operations_sanitize(ct3d, payload_in, len_in, + payload_out, len_out, 0, + cci); + default: + return CXL_MBOX_UNSUPPORTED; + } default: return CXL_MBOX_UNSUPPORTED; } @@ -3170,6 +3372,12 @@ static void bg_timercb(void *opaque) cxl_dev_enable_media(&ct3d->cxl_dstate); } break; + case 0x4402: /* Media Operations sanitize */ + { + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); + __do_sanitize(ct3d); + } + break; case 0x4304: /* scan media */ { CXLType3Dev *ct3d = CXL_TYPE3(cci->d); diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h index a64739be25..3c5711249b 100644 --- a/include/hw/cxl/cxl_device.h +++ b/include/hw/cxl/cxl_device.h @@ -581,6 +581,8 @@ typedef struct CXLSetFeatureInfo { size_t data_size; } CXLSetFeatureInfo; +struct CXLSanitizeInfo; + struct CXLType3Dev { /* Private */ PCIDevice parent_obj; @@ -651,6 +653,8 @@ struct CXLType3Dev { uint8_t num_regions; /* 0-8 regions */ CXLDCRegion regions[DCD_MAX_NUM_REGION]; } dc; + + struct CXLSanitizeInfo *media_op_sanitize; }; #define TYPE_CXL_TYPE3 "cxl-type3" -- 2.34.1 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations Sanitize and Write Zeros commands cxl r3.2(8.2.10.9.5.3) 2025-02-20 5:27 ` [PATCH v3 3/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations Sanitize and Write Zeros commands cxl r3.2(8.2.10.9.5.3) Vinayak Holikatti @ 2025-02-20 15:10 ` Jonathan Cameron via 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron @ 2025-02-20 15:10 UTC (permalink / raw) To: Vinayak Holikatti Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore On Thu, 20 Feb 2025 10:57:24 +0530 Vinayak Holikatti <vinayak.kh@samsung.com> wrote: > CXL spec 3.2 section 8.2.10.9.5.3 describes media operations commands. > CXL devices supports media operations Sanitize and Write zero command. > > Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com> Another one where I made some minor tweaks whilst applying. Let me know if I messed up! Comments on why folow. diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 4aa695cfa9..a48a551bfc 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -1796,7 +1796,7 @@ static int sanitize_range(CXLType3Dev *ct3d, uint64_t dpa_addr, size_t length, uint64_t vmr_size, pmr_size; AddressSpace *as = NULL; - MemTxAttrs mem_attrs = {0}; + MemTxAttrs mem_attrs = {}; vmr_size = get_vmr_size(ct3d, NULL); pmr_size = get_pmr_size(ct3d, NULL); @@ -1820,7 +1820,8 @@ static void __do_sanitize(CXLType3Dev *ct3d) { struct CXLSanitizeInfo *san_info = ct3d->media_op_sanitize; int dpa_range_count = san_info->dpa_range_count; - int rc = 0, i; + int rc = 0; + int i; for (i = 0; i < dpa_range_count; i++) { rc = sanitize_range(ct3d, san_info->dpa_range_list[i].starting_dpa, @@ -1933,21 +1934,19 @@ static CXLRetCode media_operations_sanitize(CXLType3Dev *ct3d, uint8_t media_operation_subclass; uint8_t rsvd[2]; uint32_t dpa_range_count; - struct { - uint64_t starting_dpa; - uint64_t length; - } dpa_range_list[]; + struct dpa_range_list_entry dpa_range_list[]; } QEMU_PACKED *media_op_in_sanitize_pl = (void *)payload_in; uint32_t dpa_range_count = media_op_in_sanitize_pl->dpa_range_count; uint64_t total_mem = 0; + size_t dpa_range_list_size; int secs = 0, i; if (dpa_range_count == 0) { return CXL_MBOX_SUCCESS; } - if (len_in < (sizeof(*media_op_in_sanitize_pl) + - (dpa_range_count * sizeof(struct dpa_range_list_entry)))) { + dpa_range_list_size = dpa_range_count * sizeof(struct dpa_range_list_entry); + if (len_in < (sizeof(*media_op_in_sanitize_pl) + dpa_range_list_size)) { return CXL_MBOX_INVALID_PAYLOAD_LENGTH; } @@ -1955,24 +1954,21 @@ static CXLRetCode media_operations_sanitize(CXLType3Dev *ct3d, uint64_t start_dpa = media_op_in_sanitize_pl->dpa_range_list[i].starting_dpa; uint64_t length = media_op_in_sanitize_pl->dpa_range_list[i].length; + if (validate_dpa_addr(ct3d, start_dpa, length)) { return CXL_MBOX_INVALID_INPUT; } - total_mem += media_op_in_sanitize_pl->dpa_range_list[i].length; + total_mem += length; } ct3d->media_op_sanitize = g_malloc0(sizeof(struct CXLSanitizeInfo) + - (dpa_range_count * - sizeof(struct dpa_range_list_entry))); - - if (ct3d->media_op_sanitize) { - ct3d->media_op_sanitize->dpa_range_count = dpa_range_count; - ct3d->media_op_sanitize->fill_value = fill_value; - memcpy(ct3d->media_op_sanitize->dpa_range_list, - media_op_in_sanitize_pl->dpa_range_list, - (dpa_range_count * - sizeof(struct dpa_range_list_entry))); - secs = get_sanitize_duration(total_mem >> 20); - } + dpa_range_list_size); + + ct3d->media_op_sanitize->dpa_range_count = dpa_range_count; + ct3d->media_op_sanitize->fill_value = fill_value; + memcpy(ct3d->media_op_sanitize->dpa_range_list, + media_op_in_sanitize_pl->dpa_range_list, + dpa_range_list_size); + secs = get_sanitize_duration(total_mem >> 20); /* EBUSY other bg cmds as of now */ cci->bg.runtime = secs * 1000UL; > --- > hw/cxl/cxl-mailbox-utils.c | 210 +++++++++++++++++++++++++++++++++++- > include/hw/cxl/cxl_device.h | 4 + > 2 files changed, 213 insertions(+), 1 deletion(-) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 2428d85fef..7b4e4daeb2 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -1731,6 +1731,130 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, > return CXL_MBOX_BG_STARTED; > } > > +struct dpa_range_list_entry { > + uint64_t starting_dpa; > + uint64_t length; > +} QEMU_PACKED; > + > +struct CXLSanitizeInfo { > + uint32_t dpa_range_count; > + uint8_t fill_value; > + struct dpa_range_list_entry dpa_range_list[]; > +} QEMU_PACKED; > + > +static uint64_t get_vmr_size(CXLType3Dev *ct3d, MemoryRegion **vmr) > +{ > + MemoryRegion *mr; > + if (ct3d->hostvmem) { > + mr = host_memory_backend_get_memory(ct3d->hostvmem); > + if (vmr) { > + *vmr = mr; > + } > + return memory_region_size(mr); > + } > + return 0; > +} > + > +static uint64_t get_pmr_size(CXLType3Dev *ct3d, MemoryRegion **pmr) > +{ > + MemoryRegion *mr; > + if (ct3d->hostpmem) { > + mr = host_memory_backend_get_memory(ct3d->hostpmem); > + if (pmr) { > + *pmr = mr; > + } > + return memory_region_size(mr); > + } > + return 0; > +} > + > +static uint64_t get_dc_size(CXLType3Dev *ct3d, MemoryRegion **dc_mr) > +{ > + MemoryRegion *mr; > + if (ct3d->dc.host_dc) { > + mr = host_memory_backend_get_memory(ct3d->dc.host_dc); > + if (dc_mr) { > + *dc_mr = mr; > + } > + return memory_region_size(mr); > + } > + return 0; > +} > + > +static int validate_dpa_addr(CXLType3Dev *ct3d, uint64_t dpa_addr, > + size_t length) > +{ > + uint64_t vmr_size, pmr_size, dc_size; > + > + if ((dpa_addr % CXL_CACHE_LINE_SIZE) || > + (length % CXL_CACHE_LINE_SIZE) || extra spce before the || > + (length <= 0)) { > + return -EINVAL; > + } > + > + vmr_size = get_vmr_size(ct3d, NULL); > + pmr_size = get_pmr_size(ct3d, NULL); > + dc_size = get_dc_size(ct3d, NULL); > + > + if (dpa_addr + length > vmr_size + pmr_size + dc_size) { > + return -EINVAL; > + } > + > + if (dpa_addr > vmr_size + pmr_size) { > + if (!ct3_test_region_block_backed(ct3d, dpa_addr, length)) { > + return -ENODEV; > + } > + } > + > + return 0; > +} > + > +static int sanitize_range(CXLType3Dev *ct3d, uint64_t dpa_addr, size_t length, > + uint8_t fill_value) > +{ > + > + uint64_t vmr_size, pmr_size; > + AddressSpace *as = NULL; > + MemTxAttrs mem_attrs = {0}; No need for the 0. {} is same thing. > + > + vmr_size = get_vmr_size(ct3d, NULL); > + pmr_size = get_pmr_size(ct3d, NULL); > + > + if (dpa_addr < vmr_size) { > + as = &ct3d->hostvmem_as; > + } else if (dpa_addr < vmr_size + pmr_size) { > + as = &ct3d->hostpmem_as; > + } else { > + if (!ct3_test_region_block_backed(ct3d, dpa_addr, length)) { > + return -ENODEV; > + } > + as = &ct3d->dc.host_dc_as; > + } > + > + return address_space_set(as, dpa_addr, fill_value, length, mem_attrs); > +} > + > +/* Perform the actual device zeroing */ > +static void __do_sanitize(CXLType3Dev *ct3d) > +{ > + struct CXLSanitizeInfo *san_info = ct3d->media_op_sanitize; > + int dpa_range_count = san_info->dpa_range_count; > + int rc = 0, i; I'll spit this. Generally mixing assignment and non assignment declarations can be a small readabilty issue. > + > + for (i = 0; i < dpa_range_count; i++) { > + rc = sanitize_range(ct3d, san_info->dpa_range_list[i].starting_dpa, > + san_info->dpa_range_list[i].length, > + san_info->fill_value); > + if (rc) { > + goto exit; > + } > + } > +exit: > + g_free(ct3d->media_op_sanitize); > + ct3d->media_op_sanitize = NULL; > + return; > +} > + > enum { > MEDIA_OP_CLASS_GENERAL = 0x0, > #define MEDIA_OP_GEN_SUBC_DISCOVERY 0x0 > @@ -1823,6 +1947,70 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in, > return CXL_MBOX_SUCCESS; > } > > +static CXLRetCode media_operations_sanitize(CXLType3Dev *ct3d, > + uint8_t *payload_in, > + size_t len_in, > + uint8_t *payload_out, > + size_t *len_out, > + uint8_t fill_value, > + CXLCCI *cci) > +{ > + struct media_operations_sanitize { > + uint8_t media_operation_class; > + uint8_t media_operation_subclass; > + uint8_t rsvd[2]; > + uint32_t dpa_range_count; > + struct { > + uint64_t starting_dpa; > + uint64_t length; > + } dpa_range_list[]; struct dpa_range_list_entry dpa_range_list[] which makes the > + } QEMU_PACKED *media_op_in_sanitize_pl = (void *)payload_in; > + uint32_t dpa_range_count = media_op_in_sanitize_pl->dpa_range_count; > + uint64_t total_mem = 0; > + int secs = 0, i; As above. > + > + if (dpa_range_count == 0) { > + return CXL_MBOX_SUCCESS; > + } > + > + if (len_in < (sizeof(*media_op_in_sanitize_pl) + > + (dpa_range_count * sizeof(struct dpa_range_list_entry)))) { Lots of use is made of this size of the dpa_range list. Deserves a local variable. > + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; > + } > + > + for (i = 0; i < dpa_range_count; i++) { > + uint64_t start_dpa = > + media_op_in_sanitize_pl->dpa_range_list[i].starting_dpa; > + uint64_t length = media_op_in_sanitize_pl->dpa_range_list[i].length; > + if (validate_dpa_addr(ct3d, start_dpa, length)) { > + return CXL_MBOX_INVALID_INPUT; > + } > + total_mem += media_op_in_sanitize_pl->dpa_range_list[i].length; totalmem += length. > + } > + ct3d->media_op_sanitize = g_malloc0(sizeof(struct CXLSanitizeInfo) + > + (dpa_range_count * > + sizeof(struct dpa_range_list_entry))); > + > + if (ct3d->media_op_sanitize) { This can't fail. g_malloc0 failing kills qemu anyway (makes memory handling simple ;) > + ct3d->media_op_sanitize->dpa_range_count = dpa_range_count; > + ct3d->media_op_sanitize->fill_value = fill_value; > + memcpy(ct3d->media_op_sanitize->dpa_range_list, > + media_op_in_sanitize_pl->dpa_range_list, Align after the ( > + (dpa_range_count * > + sizeof(struct dpa_range_list_entry))); > + secs = get_sanitize_duration(total_mem >> 20); > + } > + > + /* EBUSY other bg cmds as of now */ > + cci->bg.runtime = secs * 1000UL; > + *len_out = 0; > + /* > + * media op sanitize is targeted so no need to disable media or > + * clear event logs > + */ > + return CXL_MBOX_BG_STARTED; > +} > + > static CXLRetCode cmd_media_operations(const struct cxl_cmd *cmd, > uint8_t *payload_in, > size_t len_in, > @@ -1836,6 +2024,7 @@ static CXLRetCode cmd_media_operations(const struct cxl_cmd *cmd, > uint8_t rsvd[2]; > uint32_t dpa_range_count; > } QEMU_PACKED *media_op_in_common_pl = (void *)payload_in; > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > uint8_t media_op_cl = 0; > uint8_t media_op_subclass = 0; > > @@ -1853,7 +2042,20 @@ static CXLRetCode cmd_media_operations(const struct cxl_cmd *cmd, > } > > return media_operations_discovery(payload_in, len_in, payload_out, > - len_out); > + len_out); > + case MEDIA_OP_CLASS_SANITIZE: > + switch (media_op_subclass) { > + case MEDIA_OP_SAN_SUBC_SANITIZE: > + return media_operations_sanitize(ct3d, payload_in, len_in, > + payload_out, len_out, 0xF, > + cci); > + case MEDIA_OP_SAN_SUBC_ZERO: > + return media_operations_sanitize(ct3d, payload_in, len_in, > + payload_out, len_out, 0, > + cci); > + default: > + return CXL_MBOX_UNSUPPORTED; > + } > default: > return CXL_MBOX_UNSUPPORTED; > } > @@ -3170,6 +3372,12 @@ static void bg_timercb(void *opaque) > cxl_dev_enable_media(&ct3d->cxl_dstate); > } > break; > + case 0x4402: /* Media Operations sanitize */ > + { > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > + __do_sanitize(ct3d); > + } > + break; > case 0x4304: /* scan media */ > { > CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h > index a64739be25..3c5711249b 100644 > --- a/include/hw/cxl/cxl_device.h > +++ b/include/hw/cxl/cxl_device.h > @@ -581,6 +581,8 @@ typedef struct CXLSetFeatureInfo { > size_t data_size; > } CXLSetFeatureInfo; > > +struct CXLSanitizeInfo; > + > struct CXLType3Dev { > /* Private */ > PCIDevice parent_obj; > @@ -651,6 +653,8 @@ struct CXLType3Dev { > uint8_t num_regions; /* 0-8 regions */ > CXLDCRegion regions[DCD_MAX_NUM_REGION]; > } dc; > + > + struct CXLSanitizeInfo *media_op_sanitize; > }; > > #define TYPE_CXL_TYPE3 "cxl-type3" ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 3/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations Sanitize and Write Zeros commands cxl r3.2(8.2.10.9.5.3) @ 2025-02-20 15:10 ` Jonathan Cameron via 0 siblings, 0 replies; 10+ messages in thread From: Jonathan Cameron via @ 2025-02-20 15:10 UTC (permalink / raw) To: Vinayak Holikatti Cc: qemu-devel, gost.dev, linux-cxl, nifan.cxl, dave, vishak.g, krish.reddy, a.manzanares, alok.rathore On Thu, 20 Feb 2025 10:57:24 +0530 Vinayak Holikatti <vinayak.kh@samsung.com> wrote: > CXL spec 3.2 section 8.2.10.9.5.3 describes media operations commands. > CXL devices supports media operations Sanitize and Write zero command. > > Signed-off-by: Vinayak Holikatti <vinayak.kh@samsung.com> Another one where I made some minor tweaks whilst applying. Let me know if I messed up! Comments on why folow. diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c index 4aa695cfa9..a48a551bfc 100644 --- a/hw/cxl/cxl-mailbox-utils.c +++ b/hw/cxl/cxl-mailbox-utils.c @@ -1796,7 +1796,7 @@ static int sanitize_range(CXLType3Dev *ct3d, uint64_t dpa_addr, size_t length, uint64_t vmr_size, pmr_size; AddressSpace *as = NULL; - MemTxAttrs mem_attrs = {0}; + MemTxAttrs mem_attrs = {}; vmr_size = get_vmr_size(ct3d, NULL); pmr_size = get_pmr_size(ct3d, NULL); @@ -1820,7 +1820,8 @@ static void __do_sanitize(CXLType3Dev *ct3d) { struct CXLSanitizeInfo *san_info = ct3d->media_op_sanitize; int dpa_range_count = san_info->dpa_range_count; - int rc = 0, i; + int rc = 0; + int i; for (i = 0; i < dpa_range_count; i++) { rc = sanitize_range(ct3d, san_info->dpa_range_list[i].starting_dpa, @@ -1933,21 +1934,19 @@ static CXLRetCode media_operations_sanitize(CXLType3Dev *ct3d, uint8_t media_operation_subclass; uint8_t rsvd[2]; uint32_t dpa_range_count; - struct { - uint64_t starting_dpa; - uint64_t length; - } dpa_range_list[]; + struct dpa_range_list_entry dpa_range_list[]; } QEMU_PACKED *media_op_in_sanitize_pl = (void *)payload_in; uint32_t dpa_range_count = media_op_in_sanitize_pl->dpa_range_count; uint64_t total_mem = 0; + size_t dpa_range_list_size; int secs = 0, i; if (dpa_range_count == 0) { return CXL_MBOX_SUCCESS; } - if (len_in < (sizeof(*media_op_in_sanitize_pl) + - (dpa_range_count * sizeof(struct dpa_range_list_entry)))) { + dpa_range_list_size = dpa_range_count * sizeof(struct dpa_range_list_entry); + if (len_in < (sizeof(*media_op_in_sanitize_pl) + dpa_range_list_size)) { return CXL_MBOX_INVALID_PAYLOAD_LENGTH; } @@ -1955,24 +1954,21 @@ static CXLRetCode media_operations_sanitize(CXLType3Dev *ct3d, uint64_t start_dpa = media_op_in_sanitize_pl->dpa_range_list[i].starting_dpa; uint64_t length = media_op_in_sanitize_pl->dpa_range_list[i].length; + if (validate_dpa_addr(ct3d, start_dpa, length)) { return CXL_MBOX_INVALID_INPUT; } - total_mem += media_op_in_sanitize_pl->dpa_range_list[i].length; + total_mem += length; } ct3d->media_op_sanitize = g_malloc0(sizeof(struct CXLSanitizeInfo) + - (dpa_range_count * - sizeof(struct dpa_range_list_entry))); - - if (ct3d->media_op_sanitize) { - ct3d->media_op_sanitize->dpa_range_count = dpa_range_count; - ct3d->media_op_sanitize->fill_value = fill_value; - memcpy(ct3d->media_op_sanitize->dpa_range_list, - media_op_in_sanitize_pl->dpa_range_list, - (dpa_range_count * - sizeof(struct dpa_range_list_entry))); - secs = get_sanitize_duration(total_mem >> 20); - } + dpa_range_list_size); + + ct3d->media_op_sanitize->dpa_range_count = dpa_range_count; + ct3d->media_op_sanitize->fill_value = fill_value; + memcpy(ct3d->media_op_sanitize->dpa_range_list, + media_op_in_sanitize_pl->dpa_range_list, + dpa_range_list_size); + secs = get_sanitize_duration(total_mem >> 20); /* EBUSY other bg cmds as of now */ cci->bg.runtime = secs * 1000UL; > --- > hw/cxl/cxl-mailbox-utils.c | 210 +++++++++++++++++++++++++++++++++++- > include/hw/cxl/cxl_device.h | 4 + > 2 files changed, 213 insertions(+), 1 deletion(-) > > diff --git a/hw/cxl/cxl-mailbox-utils.c b/hw/cxl/cxl-mailbox-utils.c > index 2428d85fef..7b4e4daeb2 100644 > --- a/hw/cxl/cxl-mailbox-utils.c > +++ b/hw/cxl/cxl-mailbox-utils.c > @@ -1731,6 +1731,130 @@ static CXLRetCode cmd_sanitize_overwrite(const struct cxl_cmd *cmd, > return CXL_MBOX_BG_STARTED; > } > > +struct dpa_range_list_entry { > + uint64_t starting_dpa; > + uint64_t length; > +} QEMU_PACKED; > + > +struct CXLSanitizeInfo { > + uint32_t dpa_range_count; > + uint8_t fill_value; > + struct dpa_range_list_entry dpa_range_list[]; > +} QEMU_PACKED; > + > +static uint64_t get_vmr_size(CXLType3Dev *ct3d, MemoryRegion **vmr) > +{ > + MemoryRegion *mr; > + if (ct3d->hostvmem) { > + mr = host_memory_backend_get_memory(ct3d->hostvmem); > + if (vmr) { > + *vmr = mr; > + } > + return memory_region_size(mr); > + } > + return 0; > +} > + > +static uint64_t get_pmr_size(CXLType3Dev *ct3d, MemoryRegion **pmr) > +{ > + MemoryRegion *mr; > + if (ct3d->hostpmem) { > + mr = host_memory_backend_get_memory(ct3d->hostpmem); > + if (pmr) { > + *pmr = mr; > + } > + return memory_region_size(mr); > + } > + return 0; > +} > + > +static uint64_t get_dc_size(CXLType3Dev *ct3d, MemoryRegion **dc_mr) > +{ > + MemoryRegion *mr; > + if (ct3d->dc.host_dc) { > + mr = host_memory_backend_get_memory(ct3d->dc.host_dc); > + if (dc_mr) { > + *dc_mr = mr; > + } > + return memory_region_size(mr); > + } > + return 0; > +} > + > +static int validate_dpa_addr(CXLType3Dev *ct3d, uint64_t dpa_addr, > + size_t length) > +{ > + uint64_t vmr_size, pmr_size, dc_size; > + > + if ((dpa_addr % CXL_CACHE_LINE_SIZE) || > + (length % CXL_CACHE_LINE_SIZE) || extra spce before the || > + (length <= 0)) { > + return -EINVAL; > + } > + > + vmr_size = get_vmr_size(ct3d, NULL); > + pmr_size = get_pmr_size(ct3d, NULL); > + dc_size = get_dc_size(ct3d, NULL); > + > + if (dpa_addr + length > vmr_size + pmr_size + dc_size) { > + return -EINVAL; > + } > + > + if (dpa_addr > vmr_size + pmr_size) { > + if (!ct3_test_region_block_backed(ct3d, dpa_addr, length)) { > + return -ENODEV; > + } > + } > + > + return 0; > +} > + > +static int sanitize_range(CXLType3Dev *ct3d, uint64_t dpa_addr, size_t length, > + uint8_t fill_value) > +{ > + > + uint64_t vmr_size, pmr_size; > + AddressSpace *as = NULL; > + MemTxAttrs mem_attrs = {0}; No need for the 0. {} is same thing. > + > + vmr_size = get_vmr_size(ct3d, NULL); > + pmr_size = get_pmr_size(ct3d, NULL); > + > + if (dpa_addr < vmr_size) { > + as = &ct3d->hostvmem_as; > + } else if (dpa_addr < vmr_size + pmr_size) { > + as = &ct3d->hostpmem_as; > + } else { > + if (!ct3_test_region_block_backed(ct3d, dpa_addr, length)) { > + return -ENODEV; > + } > + as = &ct3d->dc.host_dc_as; > + } > + > + return address_space_set(as, dpa_addr, fill_value, length, mem_attrs); > +} > + > +/* Perform the actual device zeroing */ > +static void __do_sanitize(CXLType3Dev *ct3d) > +{ > + struct CXLSanitizeInfo *san_info = ct3d->media_op_sanitize; > + int dpa_range_count = san_info->dpa_range_count; > + int rc = 0, i; I'll spit this. Generally mixing assignment and non assignment declarations can be a small readabilty issue. > + > + for (i = 0; i < dpa_range_count; i++) { > + rc = sanitize_range(ct3d, san_info->dpa_range_list[i].starting_dpa, > + san_info->dpa_range_list[i].length, > + san_info->fill_value); > + if (rc) { > + goto exit; > + } > + } > +exit: > + g_free(ct3d->media_op_sanitize); > + ct3d->media_op_sanitize = NULL; > + return; > +} > + > enum { > MEDIA_OP_CLASS_GENERAL = 0x0, > #define MEDIA_OP_GEN_SUBC_DISCOVERY 0x0 > @@ -1823,6 +1947,70 @@ static CXLRetCode media_operations_discovery(uint8_t *payload_in, > return CXL_MBOX_SUCCESS; > } > > +static CXLRetCode media_operations_sanitize(CXLType3Dev *ct3d, > + uint8_t *payload_in, > + size_t len_in, > + uint8_t *payload_out, > + size_t *len_out, > + uint8_t fill_value, > + CXLCCI *cci) > +{ > + struct media_operations_sanitize { > + uint8_t media_operation_class; > + uint8_t media_operation_subclass; > + uint8_t rsvd[2]; > + uint32_t dpa_range_count; > + struct { > + uint64_t starting_dpa; > + uint64_t length; > + } dpa_range_list[]; struct dpa_range_list_entry dpa_range_list[] which makes the > + } QEMU_PACKED *media_op_in_sanitize_pl = (void *)payload_in; > + uint32_t dpa_range_count = media_op_in_sanitize_pl->dpa_range_count; > + uint64_t total_mem = 0; > + int secs = 0, i; As above. > + > + if (dpa_range_count == 0) { > + return CXL_MBOX_SUCCESS; > + } > + > + if (len_in < (sizeof(*media_op_in_sanitize_pl) + > + (dpa_range_count * sizeof(struct dpa_range_list_entry)))) { Lots of use is made of this size of the dpa_range list. Deserves a local variable. > + return CXL_MBOX_INVALID_PAYLOAD_LENGTH; > + } > + > + for (i = 0; i < dpa_range_count; i++) { > + uint64_t start_dpa = > + media_op_in_sanitize_pl->dpa_range_list[i].starting_dpa; > + uint64_t length = media_op_in_sanitize_pl->dpa_range_list[i].length; > + if (validate_dpa_addr(ct3d, start_dpa, length)) { > + return CXL_MBOX_INVALID_INPUT; > + } > + total_mem += media_op_in_sanitize_pl->dpa_range_list[i].length; totalmem += length. > + } > + ct3d->media_op_sanitize = g_malloc0(sizeof(struct CXLSanitizeInfo) + > + (dpa_range_count * > + sizeof(struct dpa_range_list_entry))); > + > + if (ct3d->media_op_sanitize) { This can't fail. g_malloc0 failing kills qemu anyway (makes memory handling simple ;) > + ct3d->media_op_sanitize->dpa_range_count = dpa_range_count; > + ct3d->media_op_sanitize->fill_value = fill_value; > + memcpy(ct3d->media_op_sanitize->dpa_range_list, > + media_op_in_sanitize_pl->dpa_range_list, Align after the ( > + (dpa_range_count * > + sizeof(struct dpa_range_list_entry))); > + secs = get_sanitize_duration(total_mem >> 20); > + } > + > + /* EBUSY other bg cmds as of now */ > + cci->bg.runtime = secs * 1000UL; > + *len_out = 0; > + /* > + * media op sanitize is targeted so no need to disable media or > + * clear event logs > + */ > + return CXL_MBOX_BG_STARTED; > +} > + > static CXLRetCode cmd_media_operations(const struct cxl_cmd *cmd, > uint8_t *payload_in, > size_t len_in, > @@ -1836,6 +2024,7 @@ static CXLRetCode cmd_media_operations(const struct cxl_cmd *cmd, > uint8_t rsvd[2]; > uint32_t dpa_range_count; > } QEMU_PACKED *media_op_in_common_pl = (void *)payload_in; > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > uint8_t media_op_cl = 0; > uint8_t media_op_subclass = 0; > > @@ -1853,7 +2042,20 @@ static CXLRetCode cmd_media_operations(const struct cxl_cmd *cmd, > } > > return media_operations_discovery(payload_in, len_in, payload_out, > - len_out); > + len_out); > + case MEDIA_OP_CLASS_SANITIZE: > + switch (media_op_subclass) { > + case MEDIA_OP_SAN_SUBC_SANITIZE: > + return media_operations_sanitize(ct3d, payload_in, len_in, > + payload_out, len_out, 0xF, > + cci); > + case MEDIA_OP_SAN_SUBC_ZERO: > + return media_operations_sanitize(ct3d, payload_in, len_in, > + payload_out, len_out, 0, > + cci); > + default: > + return CXL_MBOX_UNSUPPORTED; > + } > default: > return CXL_MBOX_UNSUPPORTED; > } > @@ -3170,6 +3372,12 @@ static void bg_timercb(void *opaque) > cxl_dev_enable_media(&ct3d->cxl_dstate); > } > break; > + case 0x4402: /* Media Operations sanitize */ > + { > + CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > + __do_sanitize(ct3d); > + } > + break; > case 0x4304: /* scan media */ > { > CXLType3Dev *ct3d = CXL_TYPE3(cci->d); > diff --git a/include/hw/cxl/cxl_device.h b/include/hw/cxl/cxl_device.h > index a64739be25..3c5711249b 100644 > --- a/include/hw/cxl/cxl_device.h > +++ b/include/hw/cxl/cxl_device.h > @@ -581,6 +581,8 @@ typedef struct CXLSetFeatureInfo { > size_t data_size; > } CXLSetFeatureInfo; > > +struct CXLSanitizeInfo; > + > struct CXLType3Dev { > /* Private */ > PCIDevice parent_obj; > @@ -651,6 +653,8 @@ struct CXLType3Dev { > uint8_t num_regions; /* 0-8 regions */ > CXLDCRegion regions[DCD_MAX_NUM_REGION]; > } dc; > + > + struct CXLSanitizeInfo *media_op_sanitize; > }; > > #define TYPE_CXL_TYPE3 "cxl-type3" ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-03-05 8:50 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <CGME20250220052733epcas5p41f847e5d0233a62703693565ee37374c@epcas5p4.samsung.com>
2025-02-20 5:27 ` [PATCH v3 0/3] CXL CCI Media Operations Vinayak Holikatti
2025-02-20 5:27 ` [PATCH v3 1/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations discovery commands cxl r3.2 (8.2.10.9.5.3) Vinayak Holikatti
2025-02-20 15:10 ` Jonathan Cameron
2025-02-20 15:10 ` Jonathan Cameron via
2025-02-20 5:27 ` [PATCH v3 2/3] hw/cxl: factor out calculation of sanitize duration from cmd_santize_overwrite Vinayak Holikatti
2025-03-05 8:49 ` Jonathan Cameron
2025-03-05 8:49 ` Jonathan Cameron via
2025-02-20 5:27 ` [PATCH v3 3/3] hw/cxl/cxl-mailbox-utils: Add support for Media operations Sanitize and Write Zeros commands cxl r3.2(8.2.10.9.5.3) Vinayak Holikatti
2025-02-20 15:10 ` Jonathan Cameron
2025-02-20 15:10 ` Jonathan Cameron via
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.