| Message ID | 20230120-vv-volatile-regions-v1-3-b42b21ee8d0b@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | cxl: add support for listing and creating volatile regions | expand |
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.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > --- > 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; Why not change collect_minsize() to return int and propagate the error up through create_region_validate_config()? It seems more confusing to hide a special value in size like this. Ira > + } > > 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 >
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.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> Looks good, Reviewed-by: Dan Williams <dan.j.williams@intel.com>
On Tue, 2023-02-07 at 19:55 -0800, Ira Weiny wrote: > Vishal Verma wrote: <..> > > > > 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; > > Why not change collect_minsize() to return int and propagate the error up > through create_region_validate_config()? > > It seems more confusing to hide a special value in size like this. > Hm, true, though the default case should never get hit. In fact I was planning to leave it out entirely until gcc warned that I can't skip cases if switching for an enum. I think the comment is maybe misleading in that it makes one think that this is some special handling. It would probably be clearer to make size = 0 in the initial declaration, and make the default case a no-op (maybe with a comment that we would never get here). Does that sound better? >
Verma, Vishal L wrote: > On Tue, 2023-02-07 at 19:55 -0800, Ira Weiny wrote: > > Vishal Verma wrote: > <..> > > > > > > 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; > > > > Why not change collect_minsize() to return int and propagate the error up > > through create_region_validate_config()? > > > > It seems more confusing to hide a special value in size like this. > > > Hm, true, though the default case should never get hit. In fact I was > planning to leave it out entirely until gcc warned that I can't skip > cases if switching for an enum. I think the comment is maybe misleading > in that it makes one think that this is some special handling. It would > probably be clearer to make size = 0 in the initial declaration, and > make the default case a no-op (maybe with a comment that we would never > get here). Does that sound better? > > To me the question is whether size == 0 is an error or just a valid size which something at the higher level determines is an error. To me the default case here is that mode is incorrectly set to get a valid size. Your comment implied that as well. Having size == 0 is ok generally as this is a 'min size'. So I will not argue the point too much. Ira
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.williams@intel.com> > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> Reviewed-by: Fan Ni <fan.ni@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 > >
On Fri, 2023-02-10 at 01:04 +0000, Fan Ni wrote: > 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.williams@intel.com> > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > Reviewed-by: Fan Ni <fan.ni@samsung.com> Hi Fan, Would you mind responding on v2 of this series - b4 doesn't want to pick up trailers from v1 now that v2 has been sent out. > > 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. Ah thanks, I'll take a look and send separate cleanup patches.
On Fri, Feb 10, 2023 at 01:10:44AM +0000, Verma, Vishal L wrote: > On Fri, 2023-02-10 at 01:04 +0000, Fan Ni wrote: > > 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.williams@intel.com> > > > Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> > > > > Reviewed-by: Fan Ni <fan.ni@samsung.com> > > Hi Fan, > > Would you mind responding on v2 of this series - b4 doesn't want to > pick up trailers from v1 now that v2 has been sent out. Ah, almost missed v2. Will response on that. Thanks. > > > > > 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. > > Ah thanks, I'll take a look and send separate cleanup patches. > > >
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;
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.williams@intel.com> Signed-off-by: Vishal Verma <vishal.l.verma@intel.com> --- 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(-)