[PATCH -ndctl v2 0/2] cxl: Support memdev sanitation

[PATCH -ndctl v2 0/2] cxl: Support memdev sanitation

[Davidlohr Bueso]

Hi,

Changes from v1 (https://lore.kernel.org/linux-cxl/20230423015920.11384-1-dave@stgolabs.net/)
    o Added a LIBCXL_6 section for new symbols.
    o s/sanitation/sanitization
    o In patch 2's manpage documented that sanitize does not
      involve explicit CPU cacheline management.
    o s/host_path/dev_path in patch 2.

These are the cxl-tool new subcommands for device sanitation for the kernel
side equivalent (v6.5+).

Obviously the naming of the interfaces are derived from what is currently there
for ndctl. However note that the new cxl wait-sanitize and sanitize-memdev
commands do not rely on pmem security, such as ndctl_dimm_get_security().
However, the kernel does consult pmem security before sending the approving
the operation, so no harm no foul.

This code is also available at:
     https://github.com/davidlohr/ndctl/tree/cxl-memdev-sanitation-v2

Thanks!

Davidlohr Bueso (2):
  cxl/memdev: Introduce wait-sanitize functionality
  cxl/memdev: Introduce sanitize-memdev functionality

 Documentation/cxl/cxl-sanitize-memdev.txt | 68 ++++++++++++++++
 Documentation/cxl/cxl-wait-sanitize.txt   | 39 ++++++++++
 Documentation/cxl/lib/libcxl.txt          |  2 +
 Documentation/cxl/meson.build             |  2 +
 cxl/builtin.h                             |  2 +
 cxl/cxl.c                                 |  2 +
 cxl/lib/libcxl.c                          | 94 +++++++++++++++++++++++
 cxl/lib/libcxl.sym                        |  6 ++
 cxl/libcxl.h                              |  2 +
 cxl/memdev.c                              | 73 ++++++++++++++++++
 10 files changed, 290 insertions(+)
 create mode 100644 Documentation/cxl/cxl-sanitize-memdev.txt
 create mode 100644 Documentation/cxl/cxl-wait-sanitize.txt

--
2.41.0

[PATCH 1/2] cxl/memdev: Introduce wait-sanitize functionality

[Davidlohr Bueso]

Add a new cxl_memdev_wait_sanitize() to libcxl to support
waiting for sanitize operation to be done in the background,
if any.

This is analogous to 'ndctl wait-overwrite'.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 Documentation/cxl/cxl-wait-sanitize.txt | 35 +++++++++++
 Documentation/cxl/lib/libcxl.txt        |  1 +
 Documentation/cxl/meson.build           |  1 +
 cxl/builtin.h                           |  1 +
 cxl/cxl.c                               |  1 +
 cxl/lib/libcxl.c                        | 78 +++++++++++++++++++++++++
 cxl/lib/libcxl.sym                      |  5 ++
 cxl/libcxl.h                            |  1 +
 cxl/memdev.c                            | 26 +++++++++
 9 files changed, 149 insertions(+)
 create mode 100644 Documentation/cxl/cxl-wait-sanitize.txt

diff --git a/Documentation/cxl/cxl-wait-sanitize.txt b/Documentation/cxl/cxl-wait-sanitize.txt
new file mode 100644
index 000000000000..365b11597afd
--- /dev/null
+++ b/Documentation/cxl/cxl-wait-sanitize.txt
@@ -0,0 +1,35 @@
+// SPDX-License-Identifier: GPL-2.0
+
+cxl-wait-sanitize(1)
+====================
+
+NAME
+----
+cxl-wait-sanitize - wait for a sanitization operation to complete
+
+SYNOPSIS
+--------
+[verse]
+'cxl wait-sanitize' <mem0> [<mem1>..<memN>] [<options>]
+
+DESCRIPTION
+-----------
+The kernel provides a POLL(2) capable sysfs file ('security/state') to
+indicate the state of device sanitization. When sanitization is in progress,
+this command waits for a change in the state of this file across all
+specified memdevs.
+
+OPTIONS
+-------
+<memdev>::
+include::memdev-option.txt[]
+
+-b::
+--bus=::
+include::bus-option.txt[]
+
+-v::
+--verbose::
+	Emit debug messages.
+
+include::../copyright.txt[]
diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
index 31bc85511270..3ea816bde471 100644
--- a/Documentation/cxl/lib/libcxl.txt
+++ b/Documentation/cxl/lib/libcxl.txt
@@ -135,6 +135,7 @@ int cxl_memdev_write_label(struct cxl_memdev *memdev, void *buf, size_t length,
 struct cxl_cmd *cxl_cmd_new_get_partition(struct cxl_memdev *memdev);
 struct cxl_cmd *cxl_cmd_new_set_partition(struct cxl_memdev *memdev,
 					  unsigned long long volatile_size);
+int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev);
 
 ----
 
diff --git a/Documentation/cxl/meson.build b/Documentation/cxl/meson.build
index a6d77ab8cbc2..ebf214ae30df 100644
--- a/Documentation/cxl/meson.build
+++ b/Documentation/cxl/meson.build
@@ -45,6 +45,7 @@ cxl_manpages = [
   'cxl-disable-region.txt',
   'cxl-enable-region.txt',
   'cxl-destroy-region.txt',
+  'cxl-wait-sanitize.txt',
   'cxl-monitor.txt',
 ]
 
diff --git a/cxl/builtin.h b/cxl/builtin.h
index 9baa43b8a2ac..04f613703eac 100644
--- a/cxl/builtin.h
+++ b/cxl/builtin.h
@@ -22,6 +22,7 @@ int cmd_create_region(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_enable_region(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_disable_region(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_destroy_region(int argc, const char **argv, struct cxl_ctx *ctx);
+int cmd_wait_sanitize(int argc, const char **argv, struct cxl_ctx *ctx);
 #ifdef ENABLE_LIBTRACEFS
 int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx);
 #else
diff --git a/cxl/cxl.c b/cxl/cxl.c
index 3be7026f43d3..bf55e8bcb2f7 100644
--- a/cxl/cxl.c
+++ b/cxl/cxl.c
@@ -76,6 +76,7 @@ static struct cmd_struct commands[] = {
 	{ "enable-region", .c_fn = cmd_enable_region },
 	{ "disable-region", .c_fn = cmd_disable_region },
 	{ "destroy-region", .c_fn = cmd_destroy_region },
+	{ "wait-sanitize", .c_fn = cmd_wait_sanitize },
 	{ "monitor", .c_fn = cmd_monitor },
 };
 
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 769cd8a75de9..172dfb47a2dd 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -7,6 +7,7 @@
 #include <stdlib.h>
 #include <dirent.h>
 #include <unistd.h>
+#include <poll.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/ioctl.h>
@@ -3968,6 +3969,83 @@ CXL_EXPORT struct cxl_cmd *cxl_cmd_new_set_partition(struct cxl_memdev *memdev,
 	return cmd;
 }
 
+CXL_EXPORT int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev)
+{
+	struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
+	struct pollfd fds;
+	char buf[SYSFS_ATTR_SIZE];
+	int fd = 0, rc;
+	char *path = memdev->dev_buf;
+	int len = memdev->buf_len;
+
+	if (snprintf(path, len,
+		     "%s/security/state", memdev->dev_path) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+		    cxl_memdev_get_devname(memdev));
+		return -ERANGE;
+	}
+
+	fd = open(path, O_RDONLY|O_CLOEXEC);
+	if (fd < 0) {
+		rc = -errno;
+		err(ctx, "open: %s\n", strerror(errno));
+		return rc;
+	}
+	memset(&fds, 0, sizeof(fds));
+	fds.fd = fd;
+
+	rc = sysfs_read_attr(ctx, path, buf);
+	if (rc < 0) {
+		rc = -EOPNOTSUPP;
+		goto out;
+	}
+	/* skipping if we aren't in sanitize state */
+	if (strncmp(buf, "sanitize", 8) != 0) {
+		rc = 0;
+		goto out;
+	}
+
+	for (;;) {
+		rc = sysfs_read_attr(ctx, path, buf);
+		if (rc < 0) {
+			rc = -EOPNOTSUPP;
+			break;
+		}
+
+		if (strncmp(buf, "sanitize", 8) == 0) {
+			rc = poll(&fds, 1, -1);
+			if (rc < 0) {
+				rc = -errno;
+				err(ctx, "poll error: %s\n", strerror(errno));
+				break;
+			}
+			dbg(ctx, "poll wake: revents: %d\n", fds.revents);
+			if (pread(fd, buf, 1, 0) == -1) {
+				rc = -errno;
+				break;
+			}
+			fds.revents = 0;
+		} else {
+			if (strncmp(buf, "disabled", 8) == 0)
+				rc = 1;
+			break;
+		}
+	}
+
+	if (rc == 1)
+		dbg(ctx, "%s: sanitize complete\n",
+		    cxl_memdev_get_devname(memdev));
+	else if (rc == 0)
+		dbg(ctx, "%s: sanitize skipped\n",
+		    cxl_memdev_get_devname(memdev));
+	else
+		dbg(ctx, "%s: sanitize error waiting for complete\n",
+		    cxl_memdev_get_devname(memdev));
+ out:
+	close(fd);
+	return rc;
+}
+
 CXL_EXPORT int cxl_cmd_submit(struct cxl_cmd *cmd)
 {
 	struct cxl_memdev *memdev = cmd->memdev;
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index c6545c717d50..efc7c1090a5c 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -250,3 +250,8 @@ global:
 	cxl_region_get_daxctl_region;
 	cxl_port_get_parent_dport;
 } LIBCXL_4;
+
+LIBCXL_6 {
+global:
+	cxl_memdev_wait_sanitize;
+} LIBCXL_5;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index 0218d730298f..c1656cb77103 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -416,6 +416,7 @@ unsigned long long cxl_cmd_partition_get_next_volatile_size(struct cxl_cmd *cmd)
 unsigned long long cxl_cmd_partition_get_next_persistent_size(struct cxl_cmd *cmd);
 struct cxl_cmd *cxl_cmd_new_set_partition(struct cxl_memdev *memdev,
 		unsigned long long volatile_size);
+int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev);
 
 enum cxl_setpartition_mode {
 	CXL_SETPART_NEXTBOOT,
diff --git a/cxl/memdev.c b/cxl/memdev.c
index 0b3ad0223bec..3134a6f6ab27 100644
--- a/cxl/memdev.c
+++ b/cxl/memdev.c
@@ -135,6 +135,11 @@ static const struct option free_dpa_options[] = {
 	OPT_END(),
 };
 
+static const struct option wait_sanitize_options[] = {
+	BASE_OPTIONS(),
+	OPT_END(),
+};
+
 enum reserve_dpa_mode {
 	DPA_ALLOC,
 	DPA_FREE,
@@ -653,6 +658,16 @@ out_err:
 	return rc;
 }
 
+static int action_wait_sanitize(struct cxl_memdev *memdev,
+				struct action_context *actx)
+{
+	/*
+	 * It's perfectly ok for the device to be active
+	 * or enabled, so no need to check anything here.
+	 */
+	return cxl_memdev_wait_sanitize(memdev);
+}
+
 static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx,
 			 int (*action)(struct cxl_memdev *memdev,
 				       struct action_context *actx),
@@ -893,3 +908,14 @@ int cmd_free_dpa(int argc, const char **argv, struct cxl_ctx *ctx)
 
 	return count >= 0 ? 0 : EXIT_FAILURE;
 }
+
+int cmd_wait_sanitize(int argc, const char **argv, struct cxl_ctx *ctx)
+{
+	int count = memdev_action(argc, argv, ctx, action_wait_sanitize,
+		wait_sanitize_options,
+		"cxl wait-sanitize <mem0> [<mem1>..<memn>] [<options>]");
+	log_info(&ml, "sanitization completed on %d mem device%s\n",
+		 count >= 0 ? count : 0, count > 1 ? "s" : "");
+
+	return count >= 0 ? 0 : EXIT_FAILURE;
+}
-- 
2.41.0

[PATCH 2/2] cxl/memdev: Introduce sanitize-memdev functionality

[Davidlohr Bueso]

Add a new cxl_memdev_sanitize() to libcxl to support triggering memory
device sanitation, in either Sanitize and/or Secure Erase, per the
CXL 3.0 specs.

This is analogous to 'ndctl sanitize-dimm'.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 Documentation/cxl/cxl-sanitize-memdev.txt | 68 +++++++++++++++++++++++
 Documentation/cxl/cxl-wait-sanitize.txt   |  4 ++
 Documentation/cxl/lib/libcxl.txt          |  1 +
 Documentation/cxl/meson.build             |  1 +
 cxl/builtin.h                             |  1 +
 cxl/cxl.c                                 |  1 +
 cxl/lib/libcxl.c                          | 16 ++++++
 cxl/lib/libcxl.sym                        |  1 +
 cxl/libcxl.h                              |  1 +
 cxl/memdev.c                              | 47 ++++++++++++++++
 10 files changed, 141 insertions(+)
 create mode 100644 Documentation/cxl/cxl-sanitize-memdev.txt

diff --git a/Documentation/cxl/cxl-sanitize-memdev.txt b/Documentation/cxl/cxl-sanitize-memdev.txt
new file mode 100644
index 000000000000..25aa3f55b789
--- /dev/null
+++ b/Documentation/cxl/cxl-sanitize-memdev.txt
@@ -0,0 +1,68 @@
+// SPDX-License-Identifier: GPL-2.0
+
+cxl-sanitize-memdev(1)
+======================
+
+NAME
+----
+cxl-sanitize-memdev - Perform a cryptographic destruction or sanitization
+of the contents of the given memdevs.
+
+SYNOPSIS
+--------
+[verse]
+'cxl sanitize-memdev' <mem0> [<mem1>..<memN>] [<options>]
+
+DESCRIPTION
+-----------
+The 'sanitize-memdev' command performs two different methods of
+sanitization, per the CXL 3.0+ specification. It is required that
+the memdev be disabled before sanitizing, such that the device
+cannot be actively decoding any HPA ranges at the time. This
+permits avoiding explicit global CPU cache management, relying
+instead on the implict cache flushing when a region transitions
+between active to commited.
+
+The default is 'sanitize', but additionally, a 'secure-erase'
+option is available. If both types of operations are supplied,
+then the 'secure-erase' is performed before 'sanitize'.
+
+
+OPTIONS
+-------
+<memdev>::
+include::memdev-option.txt[]
+
+-b::
+--bus=::
+include::bus-option.txt[]
+
+-e::
+--secure-erase::
+	Erase user data by changing the media encryption keys for all user
+	data areas of the device.
+
+-s::
+--sanitize::
+	Sanitize the device to securely re-purpose or decommission it. This is
+	done by ensuring that all user data and meta data, whether it resides
+	in persistent capacity, volatile capacity, or the label storage area,
+	is made permanently unavailable by whatever means is appropriate for
+	the media type.
+
+	With this option, the sanitization request is merely submitted to the
+	kernel, and the completion is asynchronous. Depending on the medium and
+	capacity, sanitize may take tens of minutes to many hours. Subsequently,
+	'cxl wait-sanitize’ can be used to wait for the memdevs that are under
+	the sanitization.
+
+-v::
+--verbose::
+	Emit debug messages.
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkcxl:cxl-wait-sanitize[1]
+linkcxl:cxl-disable-memdev[1]
diff --git a/Documentation/cxl/cxl-wait-sanitize.txt b/Documentation/cxl/cxl-wait-sanitize.txt
index 365b11597afd..8bf38318eb3b 100644
--- a/Documentation/cxl/cxl-wait-sanitize.txt
+++ b/Documentation/cxl/cxl-wait-sanitize.txt
@@ -33,3 +33,7 @@ include::bus-option.txt[]
 	Emit debug messages.
 
 include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkcxl:cxl-sanitize-memdev[1]
diff --git a/Documentation/cxl/lib/libcxl.txt b/Documentation/cxl/lib/libcxl.txt
index 3ea816bde471..a72c827a2821 100644
--- a/Documentation/cxl/lib/libcxl.txt
+++ b/Documentation/cxl/lib/libcxl.txt
@@ -136,6 +136,7 @@ struct cxl_cmd *cxl_cmd_new_get_partition(struct cxl_memdev *memdev);
 struct cxl_cmd *cxl_cmd_new_set_partition(struct cxl_memdev *memdev,
 					  unsigned long long volatile_size);
 int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev);
+int cxl_memdev_sanitize(struct cxl_memdev *memdev, const char *op);
 
 ----
 
diff --git a/Documentation/cxl/meson.build b/Documentation/cxl/meson.build
index ebf214ae30df..ba4e4077c09c 100644
--- a/Documentation/cxl/meson.build
+++ b/Documentation/cxl/meson.build
@@ -46,6 +46,7 @@ cxl_manpages = [
   'cxl-enable-region.txt',
   'cxl-destroy-region.txt',
   'cxl-wait-sanitize.txt',
+  'cxl-sanitize-memdev.txt',
   'cxl-monitor.txt',
 ]
 
diff --git a/cxl/builtin.h b/cxl/builtin.h
index 04f613703eac..956a773ffd0e 100644
--- a/cxl/builtin.h
+++ b/cxl/builtin.h
@@ -23,6 +23,7 @@ int cmd_enable_region(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_disable_region(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_destroy_region(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_wait_sanitize(int argc, const char **argv, struct cxl_ctx *ctx);
+int cmd_sanitize_memdev(int argc, const char **argv, struct cxl_ctx *ctx);
 #ifdef ENABLE_LIBTRACEFS
 int cmd_monitor(int argc, const char **argv, struct cxl_ctx *ctx);
 #else
diff --git a/cxl/cxl.c b/cxl/cxl.c
index bf55e8bcb2f7..4520162ae4fc 100644
--- a/cxl/cxl.c
+++ b/cxl/cxl.c
@@ -77,6 +77,7 @@ static struct cmd_struct commands[] = {
 	{ "disable-region", .c_fn = cmd_disable_region },
 	{ "destroy-region", .c_fn = cmd_destroy_region },
 	{ "wait-sanitize", .c_fn = cmd_wait_sanitize },
+	{ "sanitize-memdev", .c_fn = cmd_sanitize_memdev },
 	{ "monitor", .c_fn = cmd_monitor },
 };
 
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 172dfb47a2dd..baf2a917ea6c 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -4046,6 +4046,22 @@ CXL_EXPORT int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev)
 	return rc;
 }
 
+int cxl_memdev_sanitize(struct cxl_memdev *memdev, const char *op)
+{
+	struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
+	char *path = memdev->dev_buf;
+	int len = memdev->buf_len;
+
+	if (snprintf(path, len,
+		     "%s/security/%s", memdev->dev_path, op) >= len) {
+		err(ctx, "%s: buffer too small!\n",
+		    cxl_memdev_get_devname(memdev));
+		return -ERANGE;
+	}
+
+	return sysfs_write_attr(ctx, path, "1");
+}
+
 CXL_EXPORT int cxl_cmd_submit(struct cxl_cmd *cmd)
 {
 	struct cxl_memdev *memdev = cmd->memdev;
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index efc7c1090a5c..d2431e26f211 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -254,4 +254,5 @@ global:
 LIBCXL_6 {
 global:
 	cxl_memdev_wait_sanitize;
+	cxl_memdev_sanitize;
 } LIBCXL_5;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index c1656cb77103..e29b2d8b0412 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -417,6 +417,7 @@ unsigned long long cxl_cmd_partition_get_next_persistent_size(struct cxl_cmd *cm
 struct cxl_cmd *cxl_cmd_new_set_partition(struct cxl_memdev *memdev,
 		unsigned long long volatile_size);
 int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev);
+int cxl_memdev_sanitize(struct cxl_memdev *memdev, const char *op);
 
 enum cxl_setpartition_mode {
 	CXL_SETPART_NEXTBOOT,
diff --git a/cxl/memdev.c b/cxl/memdev.c
index 3134a6f6ab27..7e621c4c6bc6 100644
--- a/cxl/memdev.c
+++ b/cxl/memdev.c
@@ -31,6 +31,8 @@ static struct parameters {
 	bool serial;
 	bool force;
 	bool align;
+	bool sanitize;
+	bool secure_erase;
 	const char *type;
 	const char *size;
 	const char *decoder_filter;
@@ -85,6 +87,12 @@ OPT_STRING('t', "type", &param.type, "type",                   \
 OPT_BOOLEAN('f', "force", &param.force,                        \
 	    "Attempt 'expected to fail' operations")
 
+#define SANITIZE_OPTIONS()			      \
+OPT_BOOLEAN('e', "secure-erase", &param.secure_erase, \
+	    "secure erase a memdev"),		      \
+OPT_BOOLEAN('s', "sanitize", &param.sanitize,	      \
+	    "sanitize a memdev")
+
 static const struct option read_options[] = {
 	BASE_OPTIONS(),
 	LABEL_OPTIONS(),
@@ -140,6 +148,12 @@ static const struct option wait_sanitize_options[] = {
 	OPT_END(),
 };
 
+static const struct option sanitize_options[] = {
+	BASE_OPTIONS(),
+	SANITIZE_OPTIONS(),
+	OPT_END(),
+};
+
 enum reserve_dpa_mode {
 	DPA_ALLOC,
 	DPA_FREE,
@@ -668,6 +682,28 @@ static int action_wait_sanitize(struct cxl_memdev *memdev,
 	return cxl_memdev_wait_sanitize(memdev);
 }
 
+static int action_sanitize_memdev(struct cxl_memdev *memdev,
+				  struct action_context *actx)
+{
+	int rc = 0;
+
+	if (cxl_memdev_is_enabled(memdev))
+		return -EBUSY;
+
+	/* let Sanitize be the default */
+	if (!param.secure_erase && !param.sanitize)
+		param.sanitize = true;
+
+	if (param.secure_erase)
+		rc = cxl_memdev_sanitize(memdev, "erase");
+	if (param.sanitize)
+		rc = cxl_memdev_sanitize(memdev, "sanitize");
+	else
+		rc = -EINVAL;
+
+	return rc;
+}
+
 static int memdev_action(int argc, const char **argv, struct cxl_ctx *ctx,
 			 int (*action)(struct cxl_memdev *memdev,
 				       struct action_context *actx),
@@ -919,3 +955,14 @@ int cmd_wait_sanitize(int argc, const char **argv, struct cxl_ctx *ctx)
 
 	return count >= 0 ? 0 : EXIT_FAILURE;
 }
+
+int cmd_sanitize_memdev(int argc, const char **argv, struct cxl_ctx *ctx)
+{
+	int count = memdev_action(argc, argv, ctx, action_sanitize_memdev,
+		sanitize_options,
+		"cxl sanitize-memdev <mem0> [<mem1>..<memn>] [<options>]");
+	log_info(&ml, "sanitization started on %d mem device%s\n",
+		 count >= 0 ? count : 0, count > 1 ? "s" : "");
+
+	return count >= 0 ? 0 : EXIT_FAILURE;
+}
-- 
2.41.0

Re: [PATCH 2/2] cxl/memdev: Introduce sanitize-memdev functionality

[Alison Schofield]

On Thu, Jul 13, 2023 at 12:54:51PM -0700, Davidlohr Bueso wrote:
> Add a new cxl_memdev_sanitize() to libcxl to support triggering memory
> device sanitation, in either Sanitize and/or Secure Erase, per the
> CXL 3.0 specs.

Hi David,

Seems like maybe the commit msg and log got outdated. You actually
do the whole sh-bang here, not just an Introduction of the libcxl
accessors.

A few tidbits follow -

> 
> This is analogous to 'ndctl sanitize-dimm'.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  Documentation/cxl/cxl-sanitize-memdev.txt | 68 +++++++++++++++++++++++
>  Documentation/cxl/cxl-wait-sanitize.txt   |  4 ++
>  Documentation/cxl/lib/libcxl.txt          |  1 +
>  Documentation/cxl/meson.build             |  1 +
>  cxl/builtin.h                             |  1 +
>  cxl/cxl.c                                 |  1 +
>  cxl/lib/libcxl.c                          | 16 ++++++
>  cxl/lib/libcxl.sym                        |  1 +
>  cxl/libcxl.h                              |  1 +
>  cxl/memdev.c                              | 47 ++++++++++++++++
>  10 files changed, 141 insertions(+)
>  create mode 100644 Documentation/cxl/cxl-sanitize-memdev.txt
> 
> diff --git a/Documentation/cxl/cxl-sanitize-memdev.txt b/Documentation/cxl/cxl-sanitize-memdev.txt
> new file mode 100644
> index 000000000000..25aa3f55b789
> --- /dev/null
> +++ b/Documentation/cxl/cxl-sanitize-memdev.txt
> @@ -0,0 +1,68 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +cxl-sanitize-memdev(1)
> +======================
> +
> +NAME
> +----
> +cxl-sanitize-memdev - Perform a cryptographic destruction or sanitization
> +of the contents of the given memdevs.
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'cxl sanitize-memdev' <mem0> [<mem1>..<memN>] [<options>]
> +
> +DESCRIPTION
> +-----------
> +The 'sanitize-memdev' command performs two different methods of
> +sanitization, per the CXL 3.0+ specification. It is required that
> +the memdev be disabled before sanitizing, such that the device
> +cannot be actively decoding any HPA ranges at the time. This
> +permits avoiding explicit global CPU cache management, relying
> +instead on the implict cache flushing when a region transitions
> +between active to commited.

s/implict/implicit
s/commited/committed

snip
> +--sanitize::
> +	Sanitize the device to securely re-purpose or decommission it. This is
> +	done by ensuring that all user data and meta data, whether it resides

s/meta data/metadata or meta-data

snip
>  
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 172dfb47a2dd..baf2a917ea6c 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -4046,6 +4046,22 @@ CXL_EXPORT int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev)
>  	return rc;
>  }
>  
> +int cxl_memdev_sanitize(struct cxl_memdev *memdev, const char *op)
> +{
> +	struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
> +	char *path = memdev->dev_buf;
> +	int len = memdev->buf_len;
> +
> +	if (snprintf(path, len,
> +		     "%s/security/%s", memdev->dev_path, op) >= len) {
> +		err(ctx, "%s: buffer too small!\n",
> +		    cxl_memdev_get_devname(memdev));
> +		return -ERANGE;
> +	}
> +
> +	return sysfs_write_attr(ctx, path, "1");

Write that attribute w new line "1\n"


> +}
> +
snip

> +static int action_sanitize_memdev(struct cxl_memdev *memdev,
> +				  struct action_context *actx)
> +{
> +	int rc = 0;
> +
> +	if (cxl_memdev_is_enabled(memdev))
> +		return -EBUSY;
> +
> +	/* let Sanitize be the default */
> +	if (!param.secure_erase && !param.sanitize)
> +		param.sanitize = true;
> +
> +	if (param.secure_erase)
> +		rc = cxl_memdev_sanitize(memdev, "erase");
> +	if (param.sanitize)
> +		rc = cxl_memdev_sanitize(memdev, "sanitize");
> +	else
> +		rc = -EINVAL;

What's the deal w supporting secure erase with sanitize?
Seems useless, but perhaps it's cheap.

Alison

> 

Re: [PATCH 2/2] cxl/memdev: Introduce sanitize-memdev functionality

[Davidlohr Bueso]

On Thu, 13 Jul 2023, Alison Schofield wrote:

>On Thu, Jul 13, 2023 at 12:54:51PM -0700, Davidlohr Bueso wrote:
>> Add a new cxl_memdev_sanitize() to libcxl to support triggering memory
>> device sanitation, in either Sanitize and/or Secure Erase, per the
>> CXL 3.0 specs.
>
>Hi David,
>
>Seems like maybe the commit msg and log got outdated. You actually
>do the whole sh-bang here, not just an Introduction of the libcxl
>accessors.

Right, the above was only a summary of the main changes to support the
objective. Not outdated.

>
>A few tidbits follow -

Thanks for having a look.

>
>>
>> This is analogous to 'ndctl sanitize-dimm'.
>>
>> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
>> ---
>>  Documentation/cxl/cxl-sanitize-memdev.txt | 68 +++++++++++++++++++++++
>>  Documentation/cxl/cxl-wait-sanitize.txt   |  4 ++
>>  Documentation/cxl/lib/libcxl.txt          |  1 +
>>  Documentation/cxl/meson.build             |  1 +
>>  cxl/builtin.h                             |  1 +
>>  cxl/cxl.c                                 |  1 +
>>  cxl/lib/libcxl.c                          | 16 ++++++
>>  cxl/lib/libcxl.sym                        |  1 +
>>  cxl/libcxl.h                              |  1 +
>>  cxl/memdev.c                              | 47 ++++++++++++++++
>>  10 files changed, 141 insertions(+)
>>  create mode 100644 Documentation/cxl/cxl-sanitize-memdev.txt
>>
>> diff --git a/Documentation/cxl/cxl-sanitize-memdev.txt b/Documentation/cxl/cxl-sanitize-memdev.txt
>> new file mode 100644
>> index 000000000000..25aa3f55b789
>> --- /dev/null
>> +++ b/Documentation/cxl/cxl-sanitize-memdev.txt
>> @@ -0,0 +1,68 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +cxl-sanitize-memdev(1)
>> +======================
>> +
>> +NAME
>> +----
>> +cxl-sanitize-memdev - Perform a cryptographic destruction or sanitization
>> +of the contents of the given memdevs.
>> +
>> +SYNOPSIS
>> +--------
>> +[verse]
>> +'cxl sanitize-memdev' <mem0> [<mem1>..<memN>] [<options>]
>> +
>> +DESCRIPTION
>> +-----------
>> +The 'sanitize-memdev' command performs two different methods of
>> +sanitization, per the CXL 3.0+ specification. It is required that
>> +the memdev be disabled before sanitizing, such that the device
>> +cannot be actively decoding any HPA ranges at the time. This
>> +permits avoiding explicit global CPU cache management, relying
>> +instead on the implict cache flushing when a region transitions
>> +between active to commited.
>
>s/implict/implicit
>s/commited/committed

Indeed.

>
>snip
>> +--sanitize::
>> +	Sanitize the device to securely re-purpose or decommission it. This is
>> +	done by ensuring that all user data and meta data, whether it resides
>
>s/meta data/metadata or meta-data

Ok

>
>snip
>>
>> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
>> index 172dfb47a2dd..baf2a917ea6c 100644
>> --- a/cxl/lib/libcxl.c
>> +++ b/cxl/lib/libcxl.c
>> @@ -4046,6 +4046,22 @@ CXL_EXPORT int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev)
>>	return rc;
>>  }
>>
>> +int cxl_memdev_sanitize(struct cxl_memdev *memdev, const char *op)
>> +{
>> +	struct cxl_ctx *ctx = cxl_memdev_get_ctx(memdev);
>> +	char *path = memdev->dev_buf;
>> +	int len = memdev->buf_len;
>> +
>> +	if (snprintf(path, len,
>> +		     "%s/security/%s", memdev->dev_path, op) >= len) {
>> +		err(ctx, "%s: buffer too small!\n",
>> +		    cxl_memdev_get_devname(memdev));
>> +		return -ERANGE;
>> +	}
>> +
>> +	return sysfs_write_attr(ctx, path, "1");
>
>Write that attribute w new line "1\n"

Sure.

>
>
>> +}
>> +
>snip
>
>> +static int action_sanitize_memdev(struct cxl_memdev *memdev,
>> +				  struct action_context *actx)
>> +{
>> +	int rc = 0;
>> +
>> +	if (cxl_memdev_is_enabled(memdev))
>> +		return -EBUSY;
>> +
>> +	/* let Sanitize be the default */
>> +	if (!param.secure_erase && !param.sanitize)
>> +		param.sanitize = true;
>> +
>> +	if (param.secure_erase)
>> +		rc = cxl_memdev_sanitize(memdev, "erase");
>> +	if (param.sanitize)
>> +		rc = cxl_memdev_sanitize(memdev, "sanitize");
>> +	else
>> +		rc = -EINVAL;
>
>What's the deal w supporting secure erase with sanitize?
>Seems useless, but perhaps it's cheap.

It is what it is. The secure erase option passed as a program
parameter seemed a natural fit - in addition, both operations
are logically grouped under the Sanitize category per specs.

Vishal was fine with this, did you have anything in mind?

Thanks,
Davidlohr

Re: [PATCH 2/2] cxl/memdev: Introduce sanitize-memdev functionality

[Alison Schofield]

On Fri, Jul 14, 2023 at 08:00:43AM -0700, Davidlohr Bueso wrote:
> On Thu, 13 Jul 2023, Alison Schofield wrote:
> 
> > On Thu, Jul 13, 2023 at 12:54:51PM -0700, Davidlohr Bueso wrote:

snip

> > > +
> > > +	/* let Sanitize be the default */
> > > +	if (!param.secure_erase && !param.sanitize)
> > > +		param.sanitize = true;
> > > +
> > > +	if (param.secure_erase)
> > > +		rc = cxl_memdev_sanitize(memdev, "erase");
> > > +	if (param.sanitize)
> > > +		rc = cxl_memdev_sanitize(memdev, "sanitize");
> > > +	else
> > > +		rc = -EINVAL;
> > 
> > What's the deal w supporting secure erase with sanitize?
> > Seems useless, but perhaps it's cheap.
> 
> It is what it is. The secure erase option passed as a program
> parameter seemed a natural fit - in addition, both operations
> are logically grouped under the Sanitize category per specs.
> 
> Vishal was fine with this, did you have anything in mind?

If the tool accepts both cmdline options at the same time, it's
implying that there is value in doing the secure-erase followed
by the sanitize.

Does the user get more value by doing both, or is secure-erase useless
when followed immediately by sanitize?

Alison


> 
> Thanks,
> Davidlohr

Re: [PATCH 2/2] cxl/memdev: Introduce sanitize-memdev functionality

[Verma, Vishal L]

On Fri, 2023-07-14 at 11:35 -0700, Alison Schofield wrote:
> On Fri, Jul 14, 2023 at 08:00:43AM -0700, Davidlohr Bueso wrote:
> > On Thu, 13 Jul 2023, Alison Schofield wrote:
> > 
> > > On Thu, Jul 13, 2023 at 12:54:51PM -0700, Davidlohr Bueso wrote:
> 
> snip
> 
> > > > +
> > > > +       /* let Sanitize be the default */
> > > > +       if (!param.secure_erase && !param.sanitize)
> > > > +               param.sanitize = true;
> > > > +
> > > > +       if (param.secure_erase)
> > > > +               rc = cxl_memdev_sanitize(memdev, "erase");
> > > > +       if (param.sanitize)
> > > > +               rc = cxl_memdev_sanitize(memdev, "sanitize");
> > > > +       else
> > > > +               rc = -EINVAL;
> > > 
> > > What's the deal w supporting secure erase with sanitize?
> > > Seems useless, but perhaps it's cheap.
> > 
> > It is what it is. The secure erase option passed as a program
> > parameter seemed a natural fit - in addition, both operations
> > are logically grouped under the Sanitize category per specs.
> > 
> > Vishal was fine with this, did you have anything in mind?
> 
> If the tool accepts both cmdline options at the same time, it's
> implying that there is value in doing the secure-erase followed
> by the sanitize.
> 
> Does the user get more value by doing both, or is secure-erase useless
> when followed immediately by sanitize?

Based on 8.2.9.8.5, sanitize is a superset of secure-erase. Using
options to specify whether we want the full sanitize vs just a secure-
erase (delete crypto keys) is fine, but we should probably explicitly
only allow one or the other.

Davidlohr,

If we're making sanitize the default, maybe just remove that option?
And for secure-erase, mention in the man page that instead of the full
sanitize, only the crypto keys will be deleted.

That removes any confusion as to whether it makes sense to supplu both
modes.