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

[Alison Schofield <alison.schofield@intel.com>]

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

>