On Thu, Oct 31, 2024 at 05:11:04PM -0500, Ira Weiny wrote:
> Fan Ni wrote:
> > On Wed, Oct 30, 2024 at 04:54:47PM -0500, [email protected] wrote:
> > > From: Navneet Singh <[email protected]>
> > >
> > > CXL Dynamic Capacity Devices (DCDs) optionally support dynamic capacity
> > > with up to eight partitions (Regions) (dc0-dc7). CXL regions can now be
> > > spare and defined as dynamic capacity (dc).
> > >
> > > Add support for DCD devices. Query for DCD capabilities. Add the
> > > ability to add DC partitions to a CXL DC region.
> > >
> > > Signed-off-by: Navneet Singh <[email protected]>
> > > Co-authored-by: Sushant1 Kumar <[email protected]>
> > > Signed-off-by: Sushant1 Kumar <[email protected]>
> > > Co-authored-by: Ira Weiny <[email protected]>
> > > Signed-off-by: Ira Weiny <[email protected]>
> > >
> > > ---
> > > Changes:
> > > [iweiny: adjust to new sysfs interface.]
> > > [iweiny: Rebase to latest pending]
> > > [iweiny: Adjust DCD region code to new upstream sysfs entries]
> > > [iweiny: Ensure backwards compatibility for non-DC kernels]
> > > [iweiny: fixup help message to show DC type]
> > > [iweiny: don't double declare decoder mode is dc]
> > > [iweiny: simplify __reserve_dpa() with decoder mode to index]
> > > [iweiny: Adjust to the new region mode]
> > > ---
> > > cxl/json.c | 26 +++++++++++++++
> > > cxl/lib/libcxl.c | 95
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++-
> > > cxl/lib/libcxl.sym | 3 ++
> > > cxl/lib/private.h | 6 +++-
> > > cxl/libcxl.h | 55 +++++++++++++++++++++++++++++--
> > > cxl/memdev.c | 7 +++-
> > > cxl/region.c | 49 ++++++++++++++++++++++++++--
> > > 7 files changed, 234 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/cxl/json.c b/cxl/json.c
> > > index
> > > dcd3cc28393faf7e8adf299a857531ecdeaac50a..4276b9678d7e03eaf2aec581a08450f2a0b857f2
> > > 100644
> > > --- a/cxl/json.c
> > > +++ b/cxl/json.c
> > > @@ -754,10 +754,12 @@ err_free:
> > > return jpoison;
> > > }
> > >
> > > +#define DC_SIZE_NAME_LEN 64
> > > struct json_object *util_cxl_memdev_to_json(struct cxl_memdev *memdev,
> > > unsigned long flags)
> > > {
> > > const char *devname = cxl_memdev_get_devname(memdev);
> > > + char size_name[DC_SIZE_NAME_LEN];
> > > struct json_object *jdev, *jobj;
> > > unsigned long long serial, size;
> > > const char *fw_version;
> > > @@ -800,6 +802,17 @@ struct json_object *util_cxl_memdev_to_json(struct
> > > cxl_memdev *memdev,
> > > }
> > > }
> > >
> > > + for (int index; index < MAX_NUM_DC_REGIONS; index++) {
> >
> > index is not initialized.
> > Should be index = 0;
>
> Thanks for the review!
>
> Good catch. I'll fix up.
>
> >
> > Also, the "cxl list" looks like below, the size of each DC region is
> > attached to each DCD device, that seems not quite aligned with what
> > "_size" means for pmem/ram. Should we have a separate option for "cxl
> > list" to show DC region info??
>
> I'm not sure I follow. The pmem/ram sizes show the size of the partitions on
> the memdev. This is the same for each DC partition.
>
> Are you looking for the available size after some extents are available?
>
> In that case I think you are looking for the dax information details which
> comes after creating a region and using the -X option.
>
> 17:02:42 > ./build/cxl/cxl list -r 8 -X
> [
> {
> "region":"region8",
> "resource":1031597457408,
> "size":536870912,
> "type":"dc",
> "interleave_ways":1,
> "interleave_granularity":256,
> "decode_state":"commit",
> "daxregion":{
> "id":8,
> "size":536870912,
> "available_size":134217728,
> "align":2097152
> }
> }
> ]
>
>
> This shows an available size which can further be dissected with the new
> --extents (-N) option added in patch 5/6.
>
> 17:04:32 > ./build/cxl/cxl list -r 8 -X -N
> [
> {
> "region":"region8",
> "resource":1031597457408,
> "size":536870912,
> "type":"dc",
> "interleave_ways":1,
> "interleave_granularity":256,
> "decode_state":"commit",
> "daxregion":{
> "id":8,
> "size":536870912,
> "available_size":134217728,
> "align":2097152
> },
> "extents":[
> {
> "offset":268435456,
> "length":67108864,
> "tag":"00000000-0000-0000-0000-000000000000"
> },
> {
> "offset":134217728,
> "length":67108864,
> "tag":"00000000-0000-0000-0000-000000000000"
> }
> ]
> }
> ]
>
>
> Does this give you the information you are looking for? Or am I missing
> something in your question?
>
> Ira
I was looking for something like the "-N" option provides, so I think we
are good.
Fan
>
> >
> > Fan
> >
> > ----------
> > {
> > "memdev":"mem1",
> > "dc0_size":"2.00 GiB (2.15 GB)",
> > "dc1_size":"2.00 GiB (2.15 GB)",
> > "serial":"0xf02",
> > "host":"0000:11:00.0",
> > "firmware_version":"BWFW VERSION 00"
> > },
> > {
> > "memdev":"mem3",
> > "dc0_size":"2.00 GiB (2.15 GB)",
> > "dc1_size":"2.00 GiB (2.15 GB)",
> > "serial":"0xf03",
> > "host":"0000:12:00.0",
> > "firmware_version":"BWFW VERSION 00"
> > },
> > ----------
> >
>
> [snip]
--
Fan Ni