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

[PATCH -ndctl] 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 spec.

This is analogous to 'ndctl sanitize-dimm'.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 Documentation/cxl/cxl-sanitize-memdev.txt | 59 +++++++++++++++++++++++
 Documentation/cxl/meson.build             |  1 +
 cxl/builtin.h                             |  1 +
 cxl/cxl.c                                 |  1 +
 cxl/lib/libcxl.c                          | 15 ++++++
 cxl/lib/libcxl.sym                        |  1 +
 cxl/libcxl.h                              |  1 +
 cxl/memdev.c                              | 48 ++++++++++++++++++
 8 files changed, 127 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..22212898542a
--- /dev/null
+++ b/Documentation/cxl/cxl-sanitize-memdev.txt
@@ -0,0 +1,59 @@
+// 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 memdev(s).
+
+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.
+
+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
+-------
+
+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.
+
+include::verbose-option.txt[]
+
+include::../copyright.txt[]
+
+SEE ALSO
+--------
+linkcxl:cxl-wait-sanitize[1],
+linkcxl:cxl-disable-memdev[1],
+linkcxl:cxl-list[1],
diff --git a/Documentation/cxl/meson.build b/Documentation/cxl/meson.build
index 8085c1c2c87e..99e6ee782a1c 100644
--- a/Documentation/cxl/meson.build
+++ b/Documentation/cxl/meson.build
@@ -49,6 +49,7 @@ cxl_manpages = [
   'cxl-monitor.txt',
   'cxl-update-firmware.txt',
   'cxl-set-alert-config.txt',
+  'cxl-sanitize-memdev.txt',
   'cxl-wait-sanitize.txt',
 ]
 
diff --git a/cxl/builtin.h b/cxl/builtin.h
index c483f301e5e0..29c8ad2a0ad9 100644
--- a/cxl/builtin.h
+++ b/cxl/builtin.h
@@ -16,6 +16,7 @@ int cmd_reserve_dpa(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_free_dpa(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_update_fw(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_set_alert_config(int argc, const char **argv, struct cxl_ctx *ctx);
+int cmd_sanitize_memdev(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_wait_sanitize(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_disable_port(int argc, const char **argv, struct cxl_ctx *ctx);
 int cmd_enable_port(int argc, const char **argv, struct cxl_ctx *ctx);
diff --git a/cxl/cxl.c b/cxl/cxl.c
index 16436671dc53..9c9f217c5a93 100644
--- a/cxl/cxl.c
+++ b/cxl/cxl.c
@@ -80,6 +80,7 @@ static struct cmd_struct commands[] = {
 	{ "disable-region", .c_fn = cmd_disable_region },
 	{ "destroy-region", .c_fn = cmd_destroy_region },
 	{ "monitor", .c_fn = cmd_monitor },
+	{ "sanitize-memdev", .c_fn = cmd_sanitize_memdev },
 };
 
 int main(int argc, const char **argv)
diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
index 91eedd1c4688..4f44bf1b6185 100644
--- a/cxl/lib/libcxl.c
+++ b/cxl/lib/libcxl.c
@@ -1414,6 +1414,21 @@ CXL_EXPORT int cxl_memdev_get_id(struct cxl_memdev *memdev)
 	return memdev->id;
 }
 
+CXL_EXPORT int cxl_memdev_sanitize(struct cxl_memdev *memdev, 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\n");
+}
+
 CXL_EXPORT int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev,
 					int timeout_ms)
 {
diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
index 304d7fa735d4..89a4c63cb874 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -281,4 +281,5 @@ global:
 	cxl_memdev_get_ram_qos_class;
 	cxl_region_qos_class_mismatch;
 	cxl_port_decoders_committed;
+	cxl_memdev_sanitize;
 } LIBCXL_6;
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index fc6dd0085440..a722bab8a65b 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -79,6 +79,7 @@ bool cxl_memdev_fw_update_in_progress(struct cxl_memdev *memdev);
 size_t cxl_memdev_fw_update_get_remaining(struct cxl_memdev *memdev);
 int cxl_memdev_update_fw(struct cxl_memdev *memdev, const char *fw_path);
 int cxl_memdev_cancel_fw_update(struct cxl_memdev *memdev);
+int cxl_memdev_sanitize(struct cxl_memdev *memdev, char *op);
 int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev, int timeout_ms);
 
 /* ABI spelling mistakes are forever */
diff --git a/cxl/memdev.c b/cxl/memdev.c
index 6e44d1578d03..60d1515b19f3 100644
--- a/cxl/memdev.c
+++ b/cxl/memdev.c
@@ -35,6 +35,8 @@ static struct parameters {
 	bool align;
 	bool cancel;
 	bool wait;
+	bool sanitize;
+	bool secure_erase;
 	const char *type;
 	const char *size;
 	const char *decoder_filter;
@@ -160,6 +162,12 @@ OPT_STRING('\0', "pmem-err-alert",                                            \
 	   &param.corrected_pmem_err_alert, "'on' or 'off'",                  \
 	   "enable or disable corrected pmem error warning alert")
 
+#define SANITIZE_OPTIONS()			      \
+OPT_BOOLEAN('e', "secure-erase", &param.secure_erase, \
+	    "secure erase a memdev"),		      \
+OPT_BOOLEAN('s', "sanitize", &param.sanitize,	      \
+	    "sanitize a memdev")
+
 #define WAIT_SANITIZE_OPTIONS()                \
 OPT_INTEGER('t', "timeout", &param.timeout,    \
 	    "time in milliseconds to wait for overwrite completion (default: infinite)")
@@ -226,6 +234,12 @@ static const struct option set_alert_options[] = {
 	OPT_END(),
 };
 
+static const struct option sanitize_options[] = {
+	BASE_OPTIONS(),
+	SANITIZE_OPTIONS(),
+	OPT_END(),
+};
+
 static const struct option wait_sanitize_options[] = {
 	BASE_OPTIONS(),
 	WAIT_SANITIZE_OPTIONS(),
@@ -772,6 +786,27 @@ out_err:
 	return rc;
 }
 
+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 action_wait_sanitize(struct cxl_memdev *memdev,
 				struct action_context *actx)
 {
@@ -1228,6 +1263,19 @@ int cmd_set_alert_config(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, "sanitize %s on %d mem device%s\n",
+		 count >= 0 ? "completed/started" : "failed",
+		 count >= 0 ? count : 0,  count > 1 ? "s" : "");
+
+	return count >= 0 ? 0 : EXIT_FAILURE;
+}
+
 int cmd_wait_sanitize(int argc, const char **argv, struct cxl_ctx *ctx)
 {
 	int count = memdev_action(
-- 
2.46.1

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

[Alison Schofield]

+ nvdimm@lists.linux.dev

On Sat, Sep 28, 2024 at 02:16:42PM -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 spec.
> 
> This is analogous to 'ndctl sanitize-dimm'.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>

Hi David,

I'm wrangling patches for ndctl now and need your help with this one.

Looking at the lore history, you posted a patchset for wait-sanitize &
sanitize-memdev in Apr'03.[1] and update with rev2.[2] Later in Oct'23
Dan posted a patchset with wait-sanitize and a unit test that was merged
in ndctl v80.[3,4]  A quick look at the code tells me Dan did not just
grab your implementation. It differs.

Can you confirm that the two features as a set are what you want today?
Are the last comments from Vishal and I addressed? [2]
Can the existing unit test be expanded with a sanitize-memdev test case?

Thanks!

-- Alison


[1] https://lore.kernel.org/linux-cxl/20230423015920.11384-1-dave@stgolabs.net/
[2] https://lore.kernel.org/linux-cxl/20230713195455.19769-1-dave@stgolabs.net/
[3] https://lore.kernel.org/all/169657749402.1491881.12666757616880845510.stgit@dwillia2-xfh.jf.intel.com/
[4] https://lore.kernel.org/nvdimm/72bdf880b2cafd42163638d9e7e1d848c1d2d3a9.camel@intel.com/






> ---
>  Documentation/cxl/cxl-sanitize-memdev.txt | 59 +++++++++++++++++++++++
>  Documentation/cxl/meson.build             |  1 +
>  cxl/builtin.h                             |  1 +
>  cxl/cxl.c                                 |  1 +
>  cxl/lib/libcxl.c                          | 15 ++++++
>  cxl/lib/libcxl.sym                        |  1 +
>  cxl/libcxl.h                              |  1 +
>  cxl/memdev.c                              | 48 ++++++++++++++++++
>  8 files changed, 127 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..22212898542a
> --- /dev/null
> +++ b/Documentation/cxl/cxl-sanitize-memdev.txt
> @@ -0,0 +1,59 @@
> +// 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 memdev(s).
> +
> +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.
> +
> +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
> +-------
> +
> +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.
> +
> +include::verbose-option.txt[]
> +
> +include::../copyright.txt[]
> +
> +SEE ALSO
> +--------
> +linkcxl:cxl-wait-sanitize[1],
> +linkcxl:cxl-disable-memdev[1],
> +linkcxl:cxl-list[1],
> diff --git a/Documentation/cxl/meson.build b/Documentation/cxl/meson.build
> index 8085c1c2c87e..99e6ee782a1c 100644
> --- a/Documentation/cxl/meson.build
> +++ b/Documentation/cxl/meson.build
> @@ -49,6 +49,7 @@ cxl_manpages = [
>    'cxl-monitor.txt',
>    'cxl-update-firmware.txt',
>    'cxl-set-alert-config.txt',
> +  'cxl-sanitize-memdev.txt',
>    'cxl-wait-sanitize.txt',
>  ]
>  
> diff --git a/cxl/builtin.h b/cxl/builtin.h
> index c483f301e5e0..29c8ad2a0ad9 100644
> --- a/cxl/builtin.h
> +++ b/cxl/builtin.h
> @@ -16,6 +16,7 @@ int cmd_reserve_dpa(int argc, const char **argv, struct cxl_ctx *ctx);
>  int cmd_free_dpa(int argc, const char **argv, struct cxl_ctx *ctx);
>  int cmd_update_fw(int argc, const char **argv, struct cxl_ctx *ctx);
>  int cmd_set_alert_config(int argc, const char **argv, struct cxl_ctx *ctx);
> +int cmd_sanitize_memdev(int argc, const char **argv, struct cxl_ctx *ctx);
>  int cmd_wait_sanitize(int argc, const char **argv, struct cxl_ctx *ctx);
>  int cmd_disable_port(int argc, const char **argv, struct cxl_ctx *ctx);
>  int cmd_enable_port(int argc, const char **argv, struct cxl_ctx *ctx);
> diff --git a/cxl/cxl.c b/cxl/cxl.c
> index 16436671dc53..9c9f217c5a93 100644
> --- a/cxl/cxl.c
> +++ b/cxl/cxl.c
> @@ -80,6 +80,7 @@ static struct cmd_struct commands[] = {
>  	{ "disable-region", .c_fn = cmd_disable_region },
>  	{ "destroy-region", .c_fn = cmd_destroy_region },
>  	{ "monitor", .c_fn = cmd_monitor },
> +	{ "sanitize-memdev", .c_fn = cmd_sanitize_memdev },
>  };
>  
>  int main(int argc, const char **argv)
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 91eedd1c4688..4f44bf1b6185 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -1414,6 +1414,21 @@ CXL_EXPORT int cxl_memdev_get_id(struct cxl_memdev *memdev)
>  	return memdev->id;
>  }
>  
> +CXL_EXPORT int cxl_memdev_sanitize(struct cxl_memdev *memdev, 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\n");
> +}
> +
>  CXL_EXPORT int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev,
>  					int timeout_ms)
>  {
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 304d7fa735d4..89a4c63cb874 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -281,4 +281,5 @@ global:
>  	cxl_memdev_get_ram_qos_class;
>  	cxl_region_qos_class_mismatch;
>  	cxl_port_decoders_committed;
> +	cxl_memdev_sanitize;
>  } LIBCXL_6;
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index fc6dd0085440..a722bab8a65b 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -79,6 +79,7 @@ bool cxl_memdev_fw_update_in_progress(struct cxl_memdev *memdev);
>  size_t cxl_memdev_fw_update_get_remaining(struct cxl_memdev *memdev);
>  int cxl_memdev_update_fw(struct cxl_memdev *memdev, const char *fw_path);
>  int cxl_memdev_cancel_fw_update(struct cxl_memdev *memdev);
> +int cxl_memdev_sanitize(struct cxl_memdev *memdev, char *op);
>  int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev, int timeout_ms);
>  
>  /* ABI spelling mistakes are forever */
> diff --git a/cxl/memdev.c b/cxl/memdev.c
> index 6e44d1578d03..60d1515b19f3 100644
> --- a/cxl/memdev.c
> +++ b/cxl/memdev.c
> @@ -35,6 +35,8 @@ static struct parameters {
>  	bool align;
>  	bool cancel;
>  	bool wait;
> +	bool sanitize;
> +	bool secure_erase;
>  	const char *type;
>  	const char *size;
>  	const char *decoder_filter;
> @@ -160,6 +162,12 @@ OPT_STRING('\0', "pmem-err-alert",                                            \
>  	   &param.corrected_pmem_err_alert, "'on' or 'off'",                  \
>  	   "enable or disable corrected pmem error warning alert")
>  
> +#define SANITIZE_OPTIONS()			      \
> +OPT_BOOLEAN('e', "secure-erase", &param.secure_erase, \
> +	    "secure erase a memdev"),		      \
> +OPT_BOOLEAN('s', "sanitize", &param.sanitize,	      \
> +	    "sanitize a memdev")
> +
>  #define WAIT_SANITIZE_OPTIONS()                \
>  OPT_INTEGER('t', "timeout", &param.timeout,    \
>  	    "time in milliseconds to wait for overwrite completion (default: infinite)")
> @@ -226,6 +234,12 @@ static const struct option set_alert_options[] = {
>  	OPT_END(),
>  };
>  
> +static const struct option sanitize_options[] = {
> +	BASE_OPTIONS(),
> +	SANITIZE_OPTIONS(),
> +	OPT_END(),
> +};
> +
>  static const struct option wait_sanitize_options[] = {
>  	BASE_OPTIONS(),
>  	WAIT_SANITIZE_OPTIONS(),
> @@ -772,6 +786,27 @@ out_err:
>  	return rc;
>  }
>  
> +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 action_wait_sanitize(struct cxl_memdev *memdev,
>  				struct action_context *actx)
>  {
> @@ -1228,6 +1263,19 @@ int cmd_set_alert_config(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, "sanitize %s on %d mem device%s\n",
> +		 count >= 0 ? "completed/started" : "failed",
> +		 count >= 0 ? count : 0,  count > 1 ? "s" : "");
> +
> +	return count >= 0 ? 0 : EXIT_FAILURE;
> +}
> +
>  int cmd_wait_sanitize(int argc, const char **argv, struct cxl_ctx *ctx)
>  {
>  	int count = memdev_action(
> -- 
> 2.46.1
> 
> 

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

[Davidlohr Bueso]

Hi Alison,

On Mon, 30 Sep 2024, Alison Schofield wrote:\n
>+ nvdimm@lists.linux.dev
>
>On Sat, Sep 28, 2024 at 02:16:42PM -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 spec.
>>
>> This is analogous to 'ndctl sanitize-dimm'.
>>
>> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
>
>Hi David,
>
>I'm wrangling patches for ndctl now and need your help with this one.

Ah I missed that, sorry I didn't Cc you.

>
>Looking at the lore history, you posted a patchset for wait-sanitize &
>sanitize-memdev in Apr'03.[1] and update with rev2.[2] Later in Oct'23
>Dan posted a patchset with wait-sanitize and a unit test that was merged
>in ndctl v80.[3,4]  A quick look at the code tells me Dan did not just
>grab your implementation. It differs.

While the implementations of wait-sanitize are slightly different between
what I posted and Dan's patches, it is not fundamentally very different
(poll on the file).

>
>Can you confirm that the two features as a set are what you want today?

I can.

>Are the last comments from Vishal and I addressed? [2]

So I had made sure to address the libcxl.sym comments but just noticed
that the -s option wanted to be removed as it will the the default. So
only have the -e option for secure erase alternative (and not allow for
both to be specified at the same time).

      cxl sanitize-memdev -e mem0 <-- secure erase
      cxl sanitize-memdev mem0 <-- sanitize

>Can the existing unit test be expanded with a sanitize-memdev test case?

Good point. Yeah we could trivially replace those

      echo 1 > /sys/bus/cxl/devices/${inactive}/security/sanitize && err $LINENO

with

      "$CXL" sanitize-memdev $inactive || err $LINENO


Thanks,
Davidlohr

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

[Davidlohr Bueso]

On Mon, 30 Sep 2024, Davidlohr Bueso wrote:\n

>     cxl sanitize-memdev -e mem0 <-- secure erase
>     cxl sanitize-memdev mem0 <-- sanitize

Not related to this patch (I will post v2), but just for future reference, and
perhaps someone has thoughts. Whenever the kernel supports Media Operation
(4402h in 3.1), I see this utility expanding to something like:

       cxl sanitize-memdev --zero A1-A2 [B1-B2 C1-C2] <-- zero-out ranges
       cxl sanitize-memdev --zero mem0 <-- internally use all the mem0 range
       cxl sanitize-memdev A1-A2 [B1-B2 C1-C2] <-- sanitize ranges
       cxl sanitize-memdev -e A1-A2 [B1-B2 C1-C2] <-- error

... and perhaps the kernel would need a security/zero as well as a
security/{sanitize_range,zero_range} set of files.

Of course the underlying memdev for the specified ranges would still need to be
offline entirely, just as is now.

RE: [PATCH -ndctl] cxl/memdev: Introduce sanitize-memdev functionality

[Yasunori Gotou (Fujitsu)]

Hello,
This is reminder.

This patch seems to be no progress because no one has reviewed it,
can someone please review it?

I think this command should be included for end users.

Thanks,
---
Yasunori Goto


> -----Original Message-----
> From: Davidlohr Bueso <dave@stgolabs.net>
> Sent: Sunday, September 29, 2024 6:17 AM
> To: vishal.l.verma@intel.com
> Cc: Gotou, Yasunori/五島 康文 <y-goto@fujitsu.com>; dave.jiang@intel.com;
> dan.j.williams@intel.com; linux-cxl@vger.kernel.org; dave@stgolabs.net
> Subject: [PATCH -ndctl] cxl/memdev: Introduce sanitize-memdev
> functionality
> 
> 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 spec.
> 
> This is analogous to 'ndctl sanitize-dimm'.
> 
> Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
> ---
>  Documentation/cxl/cxl-sanitize-memdev.txt | 59
> +++++++++++++++++++++++
>  Documentation/cxl/meson.build             |  1 +
>  cxl/builtin.h                             |  1 +
>  cxl/cxl.c                                 |  1 +
>  cxl/lib/libcxl.c                          | 15 ++++++
>  cxl/lib/libcxl.sym                        |  1 +
>  cxl/libcxl.h                              |  1 +
>  cxl/memdev.c                              | 48
> ++++++++++++++++++
>  8 files changed, 127 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..22212898542a
> --- /dev/null
> +++ b/Documentation/cxl/cxl-sanitize-memdev.txt
> @@ -0,0 +1,59 @@
> +// 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 memdev(s).
> +
> +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.
> +
> +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
> +-------
> +
> +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.
> +
> +include::verbose-option.txt[]
> +
> +include::../copyright.txt[]
> +
> +SEE ALSO
> +--------
> +linkcxl:cxl-wait-sanitize[1],
> +linkcxl:cxl-disable-memdev[1],
> +linkcxl:cxl-list[1],
> diff --git a/Documentation/cxl/meson.build
> b/Documentation/cxl/meson.build index 8085c1c2c87e..99e6ee782a1c
> 100644
> --- a/Documentation/cxl/meson.build
> +++ b/Documentation/cxl/meson.build
> @@ -49,6 +49,7 @@ cxl_manpages = [
>    'cxl-monitor.txt',
>    'cxl-update-firmware.txt',
>    'cxl-set-alert-config.txt',
> +  'cxl-sanitize-memdev.txt',
>    'cxl-wait-sanitize.txt',
>  ]
> 
> diff --git a/cxl/builtin.h b/cxl/builtin.h index c483f301e5e0..29c8ad2a0ad9
> 100644
> --- a/cxl/builtin.h
> +++ b/cxl/builtin.h
> @@ -16,6 +16,7 @@ int cmd_reserve_dpa(int argc, const char **argv, struct
> cxl_ctx *ctx);  int cmd_free_dpa(int argc, const char **argv, struct cxl_ctx
> *ctx);  int cmd_update_fw(int argc, const char **argv, struct cxl_ctx *ctx);  int
> cmd_set_alert_config(int argc, const char **argv, struct cxl_ctx *ctx);
> +int cmd_sanitize_memdev(int argc, const char **argv, struct cxl_ctx
> +*ctx);
>  int cmd_wait_sanitize(int argc, const char **argv, struct cxl_ctx *ctx);  int
> cmd_disable_port(int argc, const char **argv, struct cxl_ctx *ctx);  int
> cmd_enable_port(int argc, const char **argv, struct cxl_ctx *ctx); diff --git
> a/cxl/cxl.c b/cxl/cxl.c index 16436671dc53..9c9f217c5a93 100644
> --- a/cxl/cxl.c
> +++ b/cxl/cxl.c
> @@ -80,6 +80,7 @@ static struct cmd_struct commands[] = {
>  	{ "disable-region", .c_fn = cmd_disable_region },
>  	{ "destroy-region", .c_fn = cmd_destroy_region },
>  	{ "monitor", .c_fn = cmd_monitor },
> +	{ "sanitize-memdev", .c_fn = cmd_sanitize_memdev },
>  };
> 
>  int main(int argc, const char **argv)
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c index 91eedd1c4688..4f44bf1b6185
> 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -1414,6 +1414,21 @@ CXL_EXPORT int cxl_memdev_get_id(struct
> cxl_memdev *memdev)
>  	return memdev->id;
>  }
> 
> +CXL_EXPORT int cxl_memdev_sanitize(struct cxl_memdev *memdev, 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\n"); }
> +
>  CXL_EXPORT int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev,
>  					int timeout_ms)
>  {
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym index
> 304d7fa735d4..89a4c63cb874 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -281,4 +281,5 @@ global:
>  	cxl_memdev_get_ram_qos_class;
>  	cxl_region_qos_class_mismatch;
>  	cxl_port_decoders_committed;
> +	cxl_memdev_sanitize;
>  } LIBCXL_6;
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h index fc6dd0085440..a722bab8a65b
> 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -79,6 +79,7 @@ bool cxl_memdev_fw_update_in_progress(struct
> cxl_memdev *memdev);  size_t
> cxl_memdev_fw_update_get_remaining(struct cxl_memdev *memdev);  int
> cxl_memdev_update_fw(struct cxl_memdev *memdev, const char *fw_path);
> int cxl_memdev_cancel_fw_update(struct cxl_memdev *memdev);
> +int cxl_memdev_sanitize(struct cxl_memdev *memdev, char *op);
>  int cxl_memdev_wait_sanitize(struct cxl_memdev *memdev, int timeout_ms);
> 
>  /* ABI spelling mistakes are forever */ diff --git a/cxl/memdev.c
> b/cxl/memdev.c index 6e44d1578d03..60d1515b19f3 100644
> --- a/cxl/memdev.c
> +++ b/cxl/memdev.c
> @@ -35,6 +35,8 @@ static struct parameters {
>  	bool align;
>  	bool cancel;
>  	bool wait;
> +	bool sanitize;
> +	bool secure_erase;
>  	const char *type;
>  	const char *size;
>  	const char *decoder_filter;
> @@ -160,6 +162,12 @@ OPT_STRING('\0', "pmem-err-alert",
> \
>  	   &param.corrected_pmem_err_alert, "'on' or 'off'",
> \
>  	   "enable or disable corrected pmem error warning alert")
> 
> +#define SANITIZE_OPTIONS()			      \
> +OPT_BOOLEAN('e', "secure-erase", &param.secure_erase, \
> +	    "secure erase a memdev"),		      \
> +OPT_BOOLEAN('s', "sanitize", &param.sanitize,	      \
> +	    "sanitize a memdev")
> +
>  #define WAIT_SANITIZE_OPTIONS()                \
>  OPT_INTEGER('t', "timeout", &param.timeout,    \
>  	    "time in milliseconds to wait for overwrite completion (default:
> infinite)") @@ -226,6 +234,12 @@ static const struct option
> set_alert_options[] = {
>  	OPT_END(),
>  };
> 
> +static const struct option sanitize_options[] = {
> +	BASE_OPTIONS(),
> +	SANITIZE_OPTIONS(),
> +	OPT_END(),
> +};
> +
>  static const struct option wait_sanitize_options[] = {
>  	BASE_OPTIONS(),
>  	WAIT_SANITIZE_OPTIONS(),
> @@ -772,6 +786,27 @@ out_err:
>  	return rc;
>  }
> 
> +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 action_wait_sanitize(struct cxl_memdev *memdev,
>  				struct action_context *actx)
>  {
> @@ -1228,6 +1263,19 @@ int cmd_set_alert_config(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, "sanitize %s on %d mem device%s\n",
> +		 count >= 0 ? "completed/started" : "failed",
> +		 count >= 0 ? count : 0,  count > 1 ? "s" : "");
> +
> +	return count >= 0 ? 0 : EXIT_FAILURE;
> +}
> +
>  int cmd_wait_sanitize(int argc, const char **argv, struct cxl_ctx *ctx)  {
>  	int count = memdev_action(
> --
> 2.46.1

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

[Alison Schofield]

On Mon, Sep 30, 2024 at 02:39:33PM -0700, Davidlohr Bueso wrote:
> On Mon, 30 Sep 2024, Davidlohr Bueso wrote:\n

Hi David,

Checking on patches we noted as needing review in last months collab,
and now I'm thinking this one is pending a v2 from you. Is that right?

--Alison

> 
> >     cxl sanitize-memdev -e mem0 <-- secure erase
> >     cxl sanitize-memdev mem0 <-- sanitize
> 
> Not related to this patch (I will post v2), but just for future reference, and
> perhaps someone has thoughts. Whenever the kernel supports Media Operation
> (4402h in 3.1), I see this utility expanding to something like:
> 
>       cxl sanitize-memdev --zero A1-A2 [B1-B2 C1-C2] <-- zero-out ranges
>       cxl sanitize-memdev --zero mem0 <-- internally use all the mem0 range
>       cxl sanitize-memdev A1-A2 [B1-B2 C1-C2] <-- sanitize ranges
>       cxl sanitize-memdev -e A1-A2 [B1-B2 C1-C2] <-- error
> 
> ... and perhaps the kernel would need a security/zero as well as a
> security/{sanitize_range,zero_range} set of files.
> 
> Of course the underlying memdev for the specified ranges would still need to be
> offline entirely, just as is now.

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

[Davidlohr Bueso]

On Tue, 18 Feb 2025, Alison Schofield wrote:

>On Mon, Sep 30, 2024 at 02:39:33PM -0700, Davidlohr Bueso wrote:
>> On Mon, 30 Sep 2024, Davidlohr Bueso wrote:\n
>
>Hi David,
>
>Checking on patches we noted as needing review in last months collab,
>and now I'm thinking this one is pending a v2 from you. Is that right?

Yes, that is right. I will try to get it out soon, its just been low on
my list.

Thanks,
Davidlohr