On Mon, Feb 7, 2022 at 3:06 PM <[email protected]> wrote:
>
> From: Alison Schofield <[email protected]>
>
> CXL devices may support both volatile and persistent memory capacity.
> The amount of device capacity set aside for each type is typically
> established at the factory, but some devices also allow for dynamic
> re-partitioning, add a command for this purpose.
>
> Synopsis:
>
> cxl set-partition <mem0> [<mem1>..<memN>] [<options>]
>
> -t, --type=<type> 'pmem' or 'volatile' (Default: 'pmem')
> -s, --size=<size> size in bytes (Default: all partitionable capacity)
> -a, --align allow alignment correction
> -v, --verbose turn on debug
>
> Signed-off-by: Alison Schofield <[email protected]>
> ---
> Documentation/cxl/cxl-set-partition.txt | 60 ++++++++
> Documentation/cxl/meson.build | 1 +
> cxl/builtin.h | 1 +
> cxl/cxl.c | 1 +
> cxl/memdev.c | 196 ++++++++++++++++++++++++
> 5 files changed, 259 insertions(+)
> create mode 100644 Documentation/cxl/cxl-set-partition.txt
>
> diff --git a/Documentation/cxl/cxl-set-partition.txt
> b/Documentation/cxl/cxl-set-partition.txt
> new file mode 100644
> index 0000000..e20afba
> --- /dev/null
> +++ b/Documentation/cxl/cxl-set-partition.txt
> @@ -0,0 +1,60 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +cxl-set-partition(1)
> +====================
> +
> +NAME
> +----
> +cxl-set-partition - set the partitioning between volatile and persistent
> capacity on a CXL memdev
> +
> +SYNOPSIS
> +--------
> +[verse]
> +'cxl set-partition <mem> [ [<mem1>..<memN>] [<options>]'
> +
> +DESCRIPTION
> +-----------
> +Partition the device into volatile and persistent capacity.
I wonder if this should briefly describe the theory of operation of
partitioning of CXL devices. I.e. that this command is only relevant
for devices that support dual capacity (volatile + pmem) and only dual
capacity devices that have partitionable as opposed to static capacity
assignments. Maybe a reference to how to use 'cxl list' to determine
if 'cxl set-partition' is even relevant for the given memdev?
The change
> +in partitioning will become the “next” configuration, to become active
> +on the next device reset.
> +
> +Use "cxl list -m <memdev> -I" to examine the partitioning capabilities
> +of a device. A partition_alignment_bytes value of zero means there are
> +no partitionable bytes available and therefore the partitions cannot be
> +changed.
> +
> +Using this command to change the size of the persistent capacity shall
> +result in the loss of data stored.
> +
> +OPTIONS
> +-------
> +<memory device(s)>::
> +include::memdev-option.txt[]
> +
> +-t::
> +--type=::
> + Type of partition, 'pmem' or 'volatile', to create.
s/create/modify/
...since the partition is statically present, just variably sized.
> + Default: 'pmem'
> +
> +-s::
> +--size=::
> + Size of the <type> partition in bytes. Size must align to the
> + devices alignment size. Use 'cxl list -m <memdev> -I' to find
> + the 'partition_alignment_size', or, use the 'align' option.
> + Default: All partitionable capacity is assigned to <type>.
> +
> +-a::
> +--align::
> + This option allows the size of the partition to be increased to
> + meet device alignment requirements.
Perhaps reword this to say it auto-aligns so that the user can specify
the minimum size of the partition?
> +
> +-v::
> + Turn on verbose debug messages in the library (if libcxl was built
> with
> + logging and debug enabled).
> +
> +include::../copyright.txt[]
> +
> +SEE ALSO
> +--------
> +linkcxl:cxl-list[1],
> +CXL-2.0 8.2.9.5.2
> diff --git a/Documentation/cxl/meson.build b/Documentation/cxl/meson.build
> index 96f4666..e927644 100644
> --- a/Documentation/cxl/meson.build
> +++ b/Documentation/cxl/meson.build
> @@ -34,6 +34,7 @@ cxl_manpages = [
> 'cxl-disable-memdev.txt',
> 'cxl-enable-port.txt',
> 'cxl-disable-port.txt',
> + 'cxl-set-partition.txt',
> ]
>
> foreach man : cxl_manpages
> diff --git a/cxl/builtin.h b/cxl/builtin.h
> index 3123d5e..7bbad98 100644
> --- a/cxl/builtin.h
> +++ b/cxl/builtin.h
> @@ -14,4 +14,5 @@ int cmd_disable_memdev(int argc, const char **argv, struct
> cxl_ctx *ctx);
> int cmd_enable_memdev(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);
> +int cmd_set_partition(int argc, const char **argv, struct cxl_ctx *ctx);
> #endif /* _CXL_BUILTIN_H_ */
> diff --git a/cxl/cxl.c b/cxl/cxl.c
> index c20c569..ab4bbec 100644
> --- a/cxl/cxl.c
> +++ b/cxl/cxl.c
> @@ -68,6 +68,7 @@ static struct cmd_struct commands[] = {
> { "enable-memdev", .c_fn = cmd_enable_memdev },
> { "disable-port", .c_fn = cmd_disable_port },
> { "enable-port", .c_fn = cmd_enable_port },
> + { "set-partition", .c_fn = cmd_set_partition },
> };
>
> int main(int argc, const char **argv)
> diff --git a/cxl/memdev.c b/cxl/memdev.c
> index 90b33e1..5d97610 100644
> --- a/cxl/memdev.c
> +++ b/cxl/memdev.c
> @@ -6,11 +6,14 @@
> #include <unistd.h>
> #include <limits.h>
> #include <util/log.h>
> +#include <util/json.h>
> +#include <util/size.h>
> #include <cxl/libcxl.h>
> #include <util/parse-options.h>
> #include <ccan/minmax/minmax.h>
> #include <ccan/array_size/array_size.h>
>
> +#include "json.h"
> #include "filter.h"
>
> struct action_context {
> @@ -26,6 +29,9 @@ static struct parameters {
> bool verbose;
> bool serial;
> bool force;
> + bool align;
> + const char *type;
> + const char *size;
> } param;
>
> static struct log_ctx ml;
> @@ -51,6 +57,14 @@ OPT_UINTEGER('O', "offset", ¶m.offset, \
> OPT_BOOLEAN('f', "force", ¶m.force, \
> "DANGEROUS: override active memdev safety checks")
>
> +#define SET_PARTITION_OPTIONS() \
> +OPT_STRING('t', "type", ¶m.type, "type", \
> + "'pmem' or 'volatile' (Default: 'pmem')"), \
> +OPT_STRING('s', "size", ¶m.size, "size", \
> + "size in bytes (Default: all partitionable capacity)"), \
> +OPT_BOOLEAN('a', "align", ¶m.align, \
> + "allow alignment correction")
> +
> static const struct option read_options[] = {
> BASE_OPTIONS(),
> LABEL_OPTIONS(),
> @@ -82,6 +96,12 @@ static const struct option enable_options[] = {
> OPT_END(),
> };
>
> +static const struct option set_partition_options[] = {
> + BASE_OPTIONS(),
> + SET_PARTITION_OPTIONS(),
> + OPT_END(),
> +};
> +
> static int action_disable(struct cxl_memdev *memdev, struct action_context
> *actx)
> {
> if (!cxl_memdev_is_enabled(memdev))
> @@ -209,6 +229,171 @@ out:
> return rc;
> }
>
> +static unsigned long long
> +partition_align(const char *devname, unsigned long long volatile_size,
> + unsigned long long alignment, unsigned long long
> partitionable)
> +{
> + if (IS_ALIGNED(volatile_size, alignment))
> + return volatile_size;
> +
> + if (!param.align) {
> + log_err(&ml, "%s: size %lld is not partition aligned %lld\n",
> + devname, volatile_size, alignment);
> + return ULLONG_MAX;
> + }
> +
> + /* Align based on partition type to fulfill users size request */
> + if (strcmp(param.type, "pmem") == 0)
> + volatile_size = ALIGN_DOWN(volatile_size, alignment);
> + else
> + volatile_size = ALIGN(volatile_size, alignment);
> +
> + /* Fail if the align pushes size over the partitionable limit. */
> + if (volatile_size > partitionable) {
> + log_err(&ml, "%s: aligned partition size %lld exceeds
> partitionable size %lld\n",
> + devname, volatile_size, partitionable);
> + volatile_size = ULLONG_MAX;
> + }
> +
> + return volatile_size;
> +}
> +
> +static unsigned long long
> +param_size_to_volatile_size(const char *devname, unsigned long long size,
> + unsigned long long partitionable)
> +{
> + /* User omits size option. Apply all partitionable capacity to type.
> */
> + if (size == ULLONG_MAX)
> + return (strcmp(param.type, "pmem") == 0) ? 0 : partitionable;
Somewhat inefficient to keep needing to parse the string parameter
buffer. How about plumb an 'enum cxl_setpart_type type' argument to
all the functions that are parsing param.type?
Where @type is:
enum cxl_setpart_type {
CXL_SETPART_PMEM,
CXL_SETPART_VOLATILE,
};
Other than the above,
Reviewed-by: Dan Williams <[email protected]>
