diff mbox series

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

Message ID 20230423015920.11384-2-dave@stgolabs.net
State Superseded
Headers show
Series cxl: Support memdev sanitation | expand

Commit Message

Davidlohr Bueso April 23, 2023, 1:59 a.m. UTC
Add a new cxl_memdev_wait_sanitize() to libcxl to support
waiting for sanitation 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                      |  1 +
 cxl/libcxl.h                            |  1 +
 cxl/memdev.c                            | 26 +++++++++
 9 files changed, 145 insertions(+)
 create mode 100644 Documentation/cxl/cxl-wait-sanitize.txt

Comments

Verma, Vishal L April 26, 2023, 6:19 p.m. UTC | #1
On Sat, 2023-04-22 at 18:59 -0700, Davidlohr Bueso wrote:
> Add a new cxl_memdev_wait_sanitize() to libcxl to support
> waiting for sanitation 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                      |  1 +
>  cxl/libcxl.h                            |  1 +
>  cxl/memdev.c                            | 26 +++++++++
>  9 files changed, 145 insertions(+)
>  create mode 100644 Documentation/cxl/cxl-wait-sanitize.txt
> 
<..>

> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 6bc0810152e0..bf7d38f7e6fe 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -70,6 +70,7 @@ global:
>         cxl_memdev_zero_label;
>         cxl_memdev_write_label;
>         cxl_memdev_read_label;
> +       cxl_memdev_wait_sanitize;
>  local:
>          *;
>  };

Hi Davidlohr,

These mostly look good - just one thing about the symbol script
additions in both this patch and the next one -  The different LIBCXL_N
sections in this correspond to a library release, and are used for
libtool versioning.

New symbols in a new release all go together in a new section in this
file. e.g. the pending branch has a 'LIBCXL_5' section for symbols
going in the v77 release (corresponding to kernel v6.3). Assuming these
patches are targeting v78 / v6.4, its new symbols should go in a new
LIBCXL_6 section.
Davidlohr Bueso April 26, 2023, 6:25 p.m. UTC | #2
On Wed, 26 Apr 2023, Verma, Vishal L wrote:

>These mostly look good - just one thing about the symbol script
>additions in both this patch and the next one -  The different LIBCXL_N
>sections in this correspond to a library release, and are used for
>libtool versioning.
>
>New symbols in a new release all go together in a new section in this
>file. e.g. the pending branch has a 'LIBCXL_5' section for symbols
>going in the v77 release (corresponding to kernel v6.3). Assuming these
>patches are targeting v78 / v6.4, its new symbols should go in a new
>LIBCXL_6 section.

I really have no specific version of ndctl in mind here, and for the kernel
I'm "targeting" v6.5, so no hurry with this. It was mostly have the cxl
tool support there for whenever the kernel bits landed (plus recycle testing
to exercise the sysfs poll(2) support).

Would it make sense for you to update the libcxl.sym file for whenever you
pick this up? If so I can just get rid of the entries in a v2.

Thanks,
Davidlohr
Verma, Vishal L April 26, 2023, 10:01 p.m. UTC | #3
On Wed, 2023-04-26 at 11:25 -0700, Davidlohr Bueso wrote:
> On Wed, 26 Apr 2023, Verma, Vishal L wrote:
> 
> > These mostly look good - just one thing about the symbol script
> > additions in both this patch and the next one -  The different LIBCXL_N
> > sections in this correspond to a library release, and are used for
> > libtool versioning.
> > 
> > New symbols in a new release all go together in a new section in this
> > file. e.g. the pending branch has a 'LIBCXL_5' section for symbols
> > going in the v77 release (corresponding to kernel v6.3). Assuming these
> > patches are targeting v78 / v6.4, its new symbols should go in a new
> > LIBCXL_6 section.
> 
> I really have no specific version of ndctl in mind here, and for the kernel
> I'm "targeting" v6.5, so no hurry with this. It was mostly have the cxl
> tool support there for whenever the kernel bits landed (plus recycle testing
> to exercise the sysfs poll(2) support).
> 
> Would it make sense for you to update the libcxl.sym file for whenever you
> pick this up? If so I can just get rid of the entries in a v2.

Ah right - agreed on 6.5 as I think that's the target for mw-update as
well, 6.4 stuff is already 'frozen'.

Don't remove the .sym entries - I think the build might even complain -
but it is a safe bet to base on the last released version (v76), and
then just add a new section for your entries. And I can fix up the
inevitable merge-conflicts arising from this (which happens pretty
frequently for these anyway).

> 
> Thanks,
> Davidlohr
diff mbox series

Patch

diff --git a/Documentation/cxl/cxl-wait-sanitize.txt b/Documentation/cxl/cxl-wait-sanitize.txt
new file mode 100644
index 000000000000..c2ceae7ade0e
--- /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 sanitation operation to complete
+
+SYNOPSIS
+--------
+[verse]
+'cxl wait-sanitize' <mem0> [<mem1>..<memN>] [<options>]
+
+DESCRIPTION
+-----------
+The kernel provides a POLL(2) capable sysfs file ('security/sanitize') to
+indicate the state of device sanitation. When sanitation 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 f9af376137fa..9d98865e172c 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 4859bd5e2fbb..f369f45271d5 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>
@@ -3851,6 +3852,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/sanitize", 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 6bc0810152e0..bf7d38f7e6fe 100644
--- a/cxl/lib/libcxl.sym
+++ b/cxl/lib/libcxl.sym
@@ -70,6 +70,7 @@  global:
 	cxl_memdev_zero_label;
 	cxl_memdev_write_label;
 	cxl_memdev_read_label;
+	cxl_memdev_wait_sanitize;
 local:
         *;
 };
diff --git a/cxl/libcxl.h b/cxl/libcxl.h
index d699af859a86..c9c935126d4a 100644
--- a/cxl/libcxl.h
+++ b/cxl/libcxl.h
@@ -412,6 +412,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..4fc787ffe1e6 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, "sanitation completed on %d mem device%s\n",
+		 count >= 0 ? count : 0, count > 1 ? "s" : "");
+
+	return count >= 0 ? 0 : EXIT_FAILURE;
+}