| Message ID | 20230713195455.19769-3-dave@stgolabs.net |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cxl: Support memdev sanitation | expand |
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 >
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
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
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.
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", ¶m.type, "type", \ OPT_BOOLEAN('f', "force", ¶m.force, \ "Attempt 'expected to fail' operations") +#define SANITIZE_OPTIONS() \ +OPT_BOOLEAN('e', "secure-erase", ¶m.secure_erase, \ + "secure erase a memdev"), \ +OPT_BOOLEAN('s', "sanitize", ¶m.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; +}
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