On Tue, Feb 07, 2023 at 12:16:29PM -0700, Vishal Verma wrote:
> Add support in libcxl to create ram regions through a new
> cxl_decoder_create_ram_region() API, which works similarly to its pmem
> sibling.
>
> Enable ram region creation in cxl-cli, with the only differences from
> the pmem flow being:
> 1/ Use the above create_ram_region API, and
> 2/ Elide setting the UUID, since ram regions don't have one
>
> Cc: Dan Williams <dan.j.willi...@intel.com>
> Signed-off-by: Vishal Verma <vishal.l.ve...@intel.com>
Reviewed-by: Fan Ni <fan...@samsung.com>
One minor thing, there exists some code format inconsistency in
cxl/region.c file (not introduced by the patch). For exmaple,
the "switch" sometimes is followed with a space but sometime not.
> ---
> Documentation/cxl/cxl-create-region.txt | 3 ++-
> cxl/lib/libcxl.c | 22 +++++++++++++++++++---
> cxl/libcxl.h | 1 +
> cxl/region.c | 32 ++++++++++++++++++++++++++++----
> cxl/lib/libcxl.sym | 1 +
> 5 files changed, 51 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/cxl/cxl-create-region.txt
> b/Documentation/cxl/cxl-create-region.txt
> index 286779e..ada0e52 100644
> --- a/Documentation/cxl/cxl-create-region.txt
> +++ b/Documentation/cxl/cxl-create-region.txt
> @@ -80,7 +80,8 @@ include::bus-option.txt[]
> -U::
> --uuid=::
> Specify a UUID for the new region. This shouldn't usually need to be
> - specified, as one will be generated by default.
> + specified, as one will be generated by default. Only applicable to
> + pmem regions.
>
> -w::
> --ways=::
> diff --git a/cxl/lib/libcxl.c b/cxl/lib/libcxl.c
> index 83f628b..c5b9b18 100644
> --- a/cxl/lib/libcxl.c
> +++ b/cxl/lib/libcxl.c
> @@ -2234,8 +2234,8 @@ cxl_decoder_get_region(struct cxl_decoder *decoder)
> return NULL;
> }
>
> -CXL_EXPORT struct cxl_region *
> -cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
> +static struct cxl_region *cxl_decoder_create_region(struct cxl_decoder
> *decoder,
> + enum cxl_decoder_mode mode)
> {
> struct cxl_ctx *ctx = cxl_decoder_get_ctx(decoder);
> char *path = decoder->dev_buf;
> @@ -2243,7 +2243,11 @@ cxl_decoder_create_pmem_region(struct cxl_decoder
> *decoder)
> struct cxl_region *region;
> int rc;
>
> - sprintf(path, "%s/create_pmem_region", decoder->dev_path);
> + if (mode == CXL_DECODER_MODE_PMEM)
> + sprintf(path, "%s/create_pmem_region", decoder->dev_path);
> + else if (mode == CXL_DECODER_MODE_RAM)
> + sprintf(path, "%s/create_ram_region", decoder->dev_path);
> +
> rc = sysfs_read_attr(ctx, path, buf);
> if (rc < 0) {
> err(ctx, "failed to read new region name: %s\n",
> @@ -2282,6 +2286,18 @@ cxl_decoder_create_pmem_region(struct cxl_decoder
> *decoder)
> return region;
> }
>
> +CXL_EXPORT struct cxl_region *
> +cxl_decoder_create_pmem_region(struct cxl_decoder *decoder)
> +{
> + return cxl_decoder_create_region(decoder, CXL_DECODER_MODE_PMEM);
> +}
> +
> +CXL_EXPORT struct cxl_region *
> +cxl_decoder_create_ram_region(struct cxl_decoder *decoder)
> +{
> + return cxl_decoder_create_region(decoder, CXL_DECODER_MODE_RAM);
> +}
> +
> CXL_EXPORT int cxl_decoder_get_nr_targets(struct cxl_decoder *decoder)
> {
> return decoder->nr_targets;
> diff --git a/cxl/libcxl.h b/cxl/libcxl.h
> index e6cca11..904156c 100644
> --- a/cxl/libcxl.h
> +++ b/cxl/libcxl.h
> @@ -213,6 +213,7 @@ cxl_decoder_get_interleave_granularity(struct cxl_decoder
> *decoder);
> unsigned int cxl_decoder_get_interleave_ways(struct cxl_decoder *decoder);
> struct cxl_region *cxl_decoder_get_region(struct cxl_decoder *decoder);
> struct cxl_region *cxl_decoder_create_pmem_region(struct cxl_decoder
> *decoder);
> +struct cxl_region *cxl_decoder_create_ram_region(struct cxl_decoder
> *decoder);
> struct cxl_decoder *cxl_decoder_get_by_name(struct cxl_ctx *ctx,
> const char *ident);
> struct cxl_memdev *cxl_decoder_get_memdev(struct cxl_decoder *decoder);
> diff --git a/cxl/region.c b/cxl/region.c
> index 38aa142..0945a14 100644
> --- a/cxl/region.c
> +++ b/cxl/region.c
> @@ -380,7 +380,22 @@ static void collect_minsize(struct cxl_ctx *ctx, struct
> parsed_params *p)
> struct json_object *jobj =
> json_object_array_get_idx(p->memdevs, i);
> struct cxl_memdev *memdev = json_object_get_userdata(jobj);
> - u64 size = cxl_memdev_get_pmem_size(memdev);
> + u64 size;
> +
> + switch(p->mode) {
> + case CXL_DECODER_MODE_RAM:
> + size = cxl_memdev_get_ram_size(memdev);
> + break;
> + case CXL_DECODER_MODE_PMEM:
> + size = cxl_memdev_get_pmem_size(memdev);
> + break;
> + default:
> + /*
> + * This will 'poison' ep_min_size with a 0, and
> + * subsequently cause the region creation to fail.
> + */
> + size = 0;
> + }
>
> if (!p->ep_min_size)
> p->ep_min_size = size;
> @@ -589,8 +604,15 @@ static int create_region(struct cxl_ctx *ctx, int *count,
> param.root_decoder);
> return -ENXIO;
> }
> + } else if (p->mode == CXL_DECODER_MODE_RAM) {
> + region = cxl_decoder_create_ram_region(p->root_decoder);
> + if (!region) {
> + log_err(&rl, "failed to create region under %s\n",
> + param.root_decoder);
> + return -ENXIO;
> + }
> } else {
> - log_err(&rl, "region type '%s' not supported yet\n",
> + log_err(&rl, "region type '%s' is not supported\n",
> param.type);
> return -EOPNOTSUPP;
> }
> @@ -602,10 +624,12 @@ static int create_region(struct cxl_ctx *ctx, int
> *count,
> goto out;
> granularity = rc;
>
> - uuid_generate(uuid);
> try(cxl_region, set_interleave_granularity, region, granularity);
> try(cxl_region, set_interleave_ways, region, p->ways);
> - try(cxl_region, set_uuid, region, uuid);
> + if (p->mode == CXL_DECODER_MODE_PMEM) {
> + uuid_generate(uuid);
> + try(cxl_region, set_uuid, region, uuid);
> + }
> try(cxl_region, set_size, region, size);
>
> for (i = 0; i < p->ways; i++) {
> diff --git a/cxl/lib/libcxl.sym b/cxl/lib/libcxl.sym
> index 9832d09..84f60ad 100644
> --- a/cxl/lib/libcxl.sym
> +++ b/cxl/lib/libcxl.sym
> @@ -246,4 +246,5 @@ global:
> LIBCXL_5 {
> global:
> cxl_region_get_mode;
> + cxl_decoder_create_ram_region;
> } LIBCXL_4;
>
> --
> 2.39.1
>
>
