On Thu, 09 Nov 2023, Peter Maydell wrote:
On Tue, 7 Nov 2023 at 10:13, Michael S. Tsirkin <m...@redhat.com> wrote:
From: Davidlohr Bueso <d...@stgolabs.net>
Make use of the background operations through the sanitize command, per CXL
3.0 specs. Traditionally run times can be rather long, depending on the
size of the media.
Estimate times based on:
https://pmem.io/documents/NVDIMM_DSM_Interface-V1.8.pdf
Hi; Coverity points out dead code in this function (CID 1523905):
Thanks for reporting.
+/*
+ * CXL 3.0 spec section 8.2.9.8.5.1 - Sanitize.
+ *
+ * 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;
+ if (total_mem <= 512) {
+ secs = 4;
+ } else if (total_mem <= 1024) {
+ secs = 8;
+ } else if (total_mem <= 2 * 1024) {
+ secs = 15;
+ } else if (total_mem <= 4 * 1024) {
+ secs = 30;
+ } else if (total_mem <= 8 * 1024) {
+ secs = 60;
+ } else if (total_mem <= 16 * 1024) {
+ secs = 2 * 60;
+ } else if (total_mem <= 32 * 1024) {
+ secs = 4 * 60;
+ } else if (total_mem <= 64 * 1024) {
+ secs = 8 * 60;
+ } else if (total_mem <= 128 * 1024) {
+ secs = 15 * 60;
+ } else if (total_mem <= 256 * 1024) {
+ secs = 30 * 60;
+ } else if (total_mem <= 512 * 1024) {
+ secs = 60 * 60;
+ } else if (total_mem <= 1024 * 1024) {
+ secs = 120 * 60;
+ } else {
+ secs = 240 * 60; /* max 4 hrs */
+ }
Here we have an exhaustive if ladder that sets 'secs'. None
of the values we might end up with are less than 4.
+
+ /* EBUSY other bg cmds as of now */
+ cci->bg.runtime = secs * 1000UL;
+ *len_out = 0;
+
+ cxl_dev_disable_media(&ct3d->cxl_dstate);
+
+ if (secs > 2) {
+ /* sanitize when done */
+ return CXL_MBOX_BG_STARTED;
+ } else {
...but here we have an else clause for when secs <= 2,
which can never happen.
Yeah in a later version this conditional was removed
altogether and always do the CXL_MBOX_BG_STARTED case.
https://lore.kernel.org/linux-cxl/20230418172337.19207-5-d...@stgolabs.net/
I will send a patch.
Thanks,
Davidlohr
+ __do_sanitization(ct3d);
+ cxl_dev_enable_media(&ct3d->cxl_dstate);
+
+ return CXL_MBOX_SUCCESS;
+ }
What was the intention here ?
+}
thanks
-- PMM
