Re: [PATCH v3 11/25] cxl/mem: Expose DCD partition capabilities in sysfs

2024-08-23 Thread Jonathan Cameron
On Thu, 22 Aug 2024 21:28:41 -0500
Ira Weiny  wrote:

> Dave Jiang wrote:
> > 
> > 
> > On 8/16/24 7:44 AM, [email protected] wrote:  
> > > From: Navneet Singh 
> > > 
> > > To properly configure CXL regions on Dynamic Capacity Devices (DCD),
> > > user space will need to know the details of the DC partitions available.
> > > 
> > > Expose dynamic capacity capabilities through sysfs.
> > > 
> > > Signed-off-by: Navneet Singh 
> > > Co-developed-by: Ira Weiny 
> > > Signed-off-by: Ira Weiny 
> > > 
> > > ---
> > > Changes:
> > > [iweiny: remove review tags]
> > > [Davidlohr/Fan/Jonathan: omit 'dc' attribute directory if device is not 
> > > DC]
> > > [Jonathan: update documentation for dc visibility]
> > > [Jonathan: Add a comment to DC region X attributes to ensure visibility 
> > > checks work]
> > > [iweiny: push sysfs version to 6.12]
> > > ---
> > >  Documentation/ABI/testing/sysfs-bus-cxl | 12 
> > >  drivers/cxl/core/memdev.c   | 97 
> > > +
> > >  2 files changed, 109 insertions(+)
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl 
> > > b/Documentation/ABI/testing/sysfs-bus-cxl
> > > index 957717264709..6227ae0ab3fc 100644
> > > --- a/Documentation/ABI/testing/sysfs-bus-cxl
> > > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > > @@ -54,6 +54,18 @@ Description:
> > >   identically named field in the Identify Memory Device Output
> > >   Payload in the CXL-2.0 specification.
> > >  
> > > +What:/sys/bus/cxl/devices/memX/dc/region_count
> > > + /sys/bus/cxl/devices/memX/dc/regionY_size  
> > 
> > Just make it into 2 separate entries?  
> 
> Do you mean in the docs?

Assuming yes, then I think it would be cleaner as two separate entries
+ Maybe even one for the directory which can then have
the visibility statement.

> 
> Ira
> 
> > 
> > DJ  
> > > +Date:August, 2024
> > > +KernelVersion:   v6.12
> > > +Contact: [email protected]
> > > +Description:
> > > + (RO) Dynamic Capacity (DC) region information.  The dc
> > > + directory is only visible on devices which support Dynamic
> > > + Capacity.
> > > + The region_count is the number of Dynamic Capacity (DC)
> > > + partitions (regions) supported on the device.
> > > + regionY_size is the size of each of those partitions.
> > >  
> > >  What:/sys/bus/cxl/devices/memX/pmem/qos_class
> > >  Date:May, 2023  
> 
> [snip]
> 




Re: [PATCH v3 11/25] cxl/mem: Expose DCD partition capabilities in sysfs

2024-08-23 Thread Dave Jiang



On 8/22/24 7:28 PM, Ira Weiny wrote:
> Dave Jiang wrote:
>>
>>
>> On 8/16/24 7:44 AM, [email protected] wrote:
>>> From: Navneet Singh 
>>>
>>> To properly configure CXL regions on Dynamic Capacity Devices (DCD),
>>> user space will need to know the details of the DC partitions available.
>>>
>>> Expose dynamic capacity capabilities through sysfs.
>>>
>>> Signed-off-by: Navneet Singh 
>>> Co-developed-by: Ira Weiny 
>>> Signed-off-by: Ira Weiny 
>>>
>>> ---
>>> Changes:
>>> [iweiny: remove review tags]
>>> [Davidlohr/Fan/Jonathan: omit 'dc' attribute directory if device is not DC]
>>> [Jonathan: update documentation for dc visibility]
>>> [Jonathan: Add a comment to DC region X attributes to ensure visibility 
>>> checks work]
>>> [iweiny: push sysfs version to 6.12]
>>> ---
>>>  Documentation/ABI/testing/sysfs-bus-cxl | 12 
>>>  drivers/cxl/core/memdev.c   | 97 
>>> +
>>>  2 files changed, 109 insertions(+)
>>>
>>> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl 
>>> b/Documentation/ABI/testing/sysfs-bus-cxl
>>> index 957717264709..6227ae0ab3fc 100644
>>> --- a/Documentation/ABI/testing/sysfs-bus-cxl
>>> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
>>> @@ -54,6 +54,18 @@ Description:
>>> identically named field in the Identify Memory Device Output
>>> Payload in the CXL-2.0 specification.
>>>  
>>> +What:  /sys/bus/cxl/devices/memX/dc/region_count
>>> +   /sys/bus/cxl/devices/memX/dc/regionY_size
>>
>> Just make it into 2 separate entries?
> 
> Do you mean in the docs?

Yes. Here you are combining all the sysfs entries into 1. I'm suggesting unique 
block per each sysfs entry with their own description. 
> 
> Ira
> 
>>
>> DJ
>>> +Date:  August, 2024
>>> +KernelVersion: v6.12
>>> +Contact:   [email protected]
>>> +Description:
>>> +   (RO) Dynamic Capacity (DC) region information.  The dc
>>> +   directory is only visible on devices which support Dynamic
>>> +   Capacity.
>>> +   The region_count is the number of Dynamic Capacity (DC)
>>> +   partitions (regions) supported on the device.
>>> +   regionY_size is the size of each of those partitions.
>>>  
>>>  What:  /sys/bus/cxl/devices/memX/pmem/qos_class
>>>  Date:  May, 2023
> 
> [snip]



Re: [PATCH v3 11/25] cxl/mem: Expose DCD partition capabilities in sysfs

2024-08-22 Thread Ira Weiny
Dave Jiang wrote:
> 
> 
> On 8/16/24 7:44 AM, [email protected] wrote:
> > From: Navneet Singh 
> > 
> > To properly configure CXL regions on Dynamic Capacity Devices (DCD),
> > user space will need to know the details of the DC partitions available.
> > 
> > Expose dynamic capacity capabilities through sysfs.
> > 
> > Signed-off-by: Navneet Singh 
> > Co-developed-by: Ira Weiny 
> > Signed-off-by: Ira Weiny 
> > 
> > ---
> > Changes:
> > [iweiny: remove review tags]
> > [Davidlohr/Fan/Jonathan: omit 'dc' attribute directory if device is not DC]
> > [Jonathan: update documentation for dc visibility]
> > [Jonathan: Add a comment to DC region X attributes to ensure visibility 
> > checks work]
> > [iweiny: push sysfs version to 6.12]
> > ---
> >  Documentation/ABI/testing/sysfs-bus-cxl | 12 
> >  drivers/cxl/core/memdev.c   | 97 
> > +
> >  2 files changed, 109 insertions(+)
> > 
> > diff --git a/Documentation/ABI/testing/sysfs-bus-cxl 
> > b/Documentation/ABI/testing/sysfs-bus-cxl
> > index 957717264709..6227ae0ab3fc 100644
> > --- a/Documentation/ABI/testing/sysfs-bus-cxl
> > +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> > @@ -54,6 +54,18 @@ Description:
> > identically named field in the Identify Memory Device Output
> > Payload in the CXL-2.0 specification.
> >  
> > +What:  /sys/bus/cxl/devices/memX/dc/region_count
> > +   /sys/bus/cxl/devices/memX/dc/regionY_size
> 
> Just make it into 2 separate entries?

Do you mean in the docs?

Ira

> 
> DJ
> > +Date:  August, 2024
> > +KernelVersion: v6.12
> > +Contact:   [email protected]
> > +Description:
> > +   (RO) Dynamic Capacity (DC) region information.  The dc
> > +   directory is only visible on devices which support Dynamic
> > +   Capacity.
> > +   The region_count is the number of Dynamic Capacity (DC)
> > +   partitions (regions) supported on the device.
> > +   regionY_size is the size of each of those partitions.
> >  
> >  What:  /sys/bus/cxl/devices/memX/pmem/qos_class
> >  Date:  May, 2023

[snip]



Re: [PATCH v3 11/25] cxl/mem: Expose DCD partition capabilities in sysfs

2024-08-16 Thread Dave Jiang



On 8/16/24 7:44 AM, [email protected] wrote:
> From: Navneet Singh 
> 
> To properly configure CXL regions on Dynamic Capacity Devices (DCD),
> user space will need to know the details of the DC partitions available.
> 
> Expose dynamic capacity capabilities through sysfs.
> 
> Signed-off-by: Navneet Singh 
> Co-developed-by: Ira Weiny 
> Signed-off-by: Ira Weiny 
> 
> ---
> Changes:
> [iweiny: remove review tags]
> [Davidlohr/Fan/Jonathan: omit 'dc' attribute directory if device is not DC]
> [Jonathan: update documentation for dc visibility]
> [Jonathan: Add a comment to DC region X attributes to ensure visibility 
> checks work]
> [iweiny: push sysfs version to 6.12]
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl | 12 
>  drivers/cxl/core/memdev.c   | 97 
> +
>  2 files changed, 109 insertions(+)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl 
> b/Documentation/ABI/testing/sysfs-bus-cxl
> index 957717264709..6227ae0ab3fc 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -54,6 +54,18 @@ Description:
>   identically named field in the Identify Memory Device Output
>   Payload in the CXL-2.0 specification.
>  
> +What:/sys/bus/cxl/devices/memX/dc/region_count
> + /sys/bus/cxl/devices/memX/dc/regionY_size

Just make it into 2 separate entries?

DJ
> +Date:August, 2024
> +KernelVersion:   v6.12
> +Contact: [email protected]
> +Description:
> + (RO) Dynamic Capacity (DC) region information.  The dc
> + directory is only visible on devices which support Dynamic
> + Capacity.
> + The region_count is the number of Dynamic Capacity (DC)
> + partitions (regions) supported on the device.
> + regionY_size is the size of each of those partitions.
>  
>  What:/sys/bus/cxl/devices/memX/pmem/qos_class
>  Date:May, 2023
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 0277726afd04..7da1f0f5711a 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -101,6 +101,18 @@ static ssize_t pmem_size_show(struct device *dev, struct 
> device_attribute *attr,
>  static struct device_attribute dev_attr_pmem_size =
>   __ATTR(size, 0444, pmem_size_show, NULL);
>  
> +static ssize_t region_count_show(struct device *dev, struct device_attribute 
> *attr,
> +  char *buf)
> +{
> + struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> + struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
> +
> + return sysfs_emit(buf, "%d\n", mds->nr_dc_region);
> +}
> +
> +static struct device_attribute dev_attr_region_count =
> + __ATTR(region_count, 0444, region_count_show, NULL);
> +
>  static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
>  char *buf)
>  {
> @@ -448,6 +460,90 @@ static struct attribute 
> *cxl_memdev_security_attributes[] = {
>   NULL,
>  };
>  
> +static ssize_t show_size_regionN(struct cxl_memdev *cxlmd, char *buf, int 
> pos)
> +{
> + struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
> +
> + return sysfs_emit(buf, "%#llx\n", mds->dc_region[pos].decode_len);
> +}
> +
> +#define REGION_SIZE_ATTR_RO(n)   
> \
> +static ssize_t region##n##_size_show(struct device *dev, \
> +  struct device_attribute *attr, \
> +  char *buf) \
> +{\
> + return show_size_regionN(to_cxl_memdev(dev), buf, (n)); \
> +}\
> +static DEVICE_ATTR_RO(region##n##_size)
> +REGION_SIZE_ATTR_RO(0);
> +REGION_SIZE_ATTR_RO(1);
> +REGION_SIZE_ATTR_RO(2);
> +REGION_SIZE_ATTR_RO(3);
> +REGION_SIZE_ATTR_RO(4);
> +REGION_SIZE_ATTR_RO(5);
> +REGION_SIZE_ATTR_RO(6);
> +REGION_SIZE_ATTR_RO(7);
> +
> +/*
> + * RegionX attributes must be listed in order and first in this array to
> + * support the visbility checks.
> + */
> +static struct attribute *cxl_memdev_dc_attributes[] = {
> + &dev_attr_region0_size.attr,
> + &dev_attr_region1_size.attr,
> + &dev_attr_region2_size.attr,
> + &dev_attr_region3_size.attr,
> + &dev_attr_region4_size.attr,
> + &dev_attr_region5_size.attr,
> + &dev_attr_region6_size.attr,
> + &dev_attr_region7_size.attr,
> + &dev_attr_region_count.attr,
> + NULL,
> +};
> +
> +static umode_t cxl_memdev_dc_attr_visible(struct kobject *kobj, struct 
> attribute *a, int n)
> +{
> + struct device *dev = kobj_to_dev(kobj);
> + struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> + struct cxl_memdev_state *mds = 

[PATCH v3 11/25] cxl/mem: Expose DCD partition capabilities in sysfs

2024-08-16 Thread ira . weiny
From: Navneet Singh 

To properly configure CXL regions on Dynamic Capacity Devices (DCD),
user space will need to know the details of the DC partitions available.

Expose dynamic capacity capabilities through sysfs.

Signed-off-by: Navneet Singh 
Co-developed-by: Ira Weiny 
Signed-off-by: Ira Weiny 

---
Changes:
[iweiny: remove review tags]
[Davidlohr/Fan/Jonathan: omit 'dc' attribute directory if device is not DC]
[Jonathan: update documentation for dc visibility]
[Jonathan: Add a comment to DC region X attributes to ensure visibility checks 
work]
[iweiny: push sysfs version to 6.12]
---
 Documentation/ABI/testing/sysfs-bus-cxl | 12 
 drivers/cxl/core/memdev.c   | 97 +
 2 files changed, 109 insertions(+)

diff --git a/Documentation/ABI/testing/sysfs-bus-cxl 
b/Documentation/ABI/testing/sysfs-bus-cxl
index 957717264709..6227ae0ab3fc 100644
--- a/Documentation/ABI/testing/sysfs-bus-cxl
+++ b/Documentation/ABI/testing/sysfs-bus-cxl
@@ -54,6 +54,18 @@ Description:
identically named field in the Identify Memory Device Output
Payload in the CXL-2.0 specification.
 
+What:  /sys/bus/cxl/devices/memX/dc/region_count
+   /sys/bus/cxl/devices/memX/dc/regionY_size
+Date:  August, 2024
+KernelVersion: v6.12
+Contact:   [email protected]
+Description:
+   (RO) Dynamic Capacity (DC) region information.  The dc
+   directory is only visible on devices which support Dynamic
+   Capacity.
+   The region_count is the number of Dynamic Capacity (DC)
+   partitions (regions) supported on the device.
+   regionY_size is the size of each of those partitions.
 
 What:  /sys/bus/cxl/devices/memX/pmem/qos_class
 Date:  May, 2023
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index 0277726afd04..7da1f0f5711a 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -101,6 +101,18 @@ static ssize_t pmem_size_show(struct device *dev, struct 
device_attribute *attr,
 static struct device_attribute dev_attr_pmem_size =
__ATTR(size, 0444, pmem_size_show, NULL);
 
+static ssize_t region_count_show(struct device *dev, struct device_attribute 
*attr,
+char *buf)
+{
+   struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+   struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
+
+   return sysfs_emit(buf, "%d\n", mds->nr_dc_region);
+}
+
+static struct device_attribute dev_attr_region_count =
+   __ATTR(region_count, 0444, region_count_show, NULL);
+
 static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
   char *buf)
 {
@@ -448,6 +460,90 @@ static struct attribute *cxl_memdev_security_attributes[] 
= {
NULL,
 };
 
+static ssize_t show_size_regionN(struct cxl_memdev *cxlmd, char *buf, int pos)
+{
+   struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
+
+   return sysfs_emit(buf, "%#llx\n", mds->dc_region[pos].decode_len);
+}
+
+#define REGION_SIZE_ATTR_RO(n) \
+static ssize_t region##n##_size_show(struct device *dev,   \
+struct device_attribute *attr, \
+char *buf) \
+{  \
+   return show_size_regionN(to_cxl_memdev(dev), buf, (n)); \
+}  \
+static DEVICE_ATTR_RO(region##n##_size)
+REGION_SIZE_ATTR_RO(0);
+REGION_SIZE_ATTR_RO(1);
+REGION_SIZE_ATTR_RO(2);
+REGION_SIZE_ATTR_RO(3);
+REGION_SIZE_ATTR_RO(4);
+REGION_SIZE_ATTR_RO(5);
+REGION_SIZE_ATTR_RO(6);
+REGION_SIZE_ATTR_RO(7);
+
+/*
+ * RegionX attributes must be listed in order and first in this array to
+ * support the visbility checks.
+ */
+static struct attribute *cxl_memdev_dc_attributes[] = {
+   &dev_attr_region0_size.attr,
+   &dev_attr_region1_size.attr,
+   &dev_attr_region2_size.attr,
+   &dev_attr_region3_size.attr,
+   &dev_attr_region4_size.attr,
+   &dev_attr_region5_size.attr,
+   &dev_attr_region6_size.attr,
+   &dev_attr_region7_size.attr,
+   &dev_attr_region_count.attr,
+   NULL,
+};
+
+static umode_t cxl_memdev_dc_attr_visible(struct kobject *kobj, struct 
attribute *a, int n)
+{
+   struct device *dev = kobj_to_dev(kobj);
+   struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
+   struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlmd->cxlds);
+
+   /* Not a memory device */
+   if (!mds)
+   return 0;
+
+   if (a == &dev_attr_region_count.attr)
+   return a->mode;
+
+   /*
+* Show only the regions supported, regionX attributes are first in the
+* list
+*/
+   if (n <