| Message ID | 20240816-dcd-type2-upstream-v3-11-7c9b96cba6d7@intel.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | DCD: Add support for Dynamic Capacity Devices (DCD) | expand |
On 8/16/24 7:44 AM, ira.weiny@intel.com wrote: > From: Navneet Singh <navneet.singh@intel.com> > > 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 <navneet.singh@intel.com> > Co-developed-by: Ira Weiny <ira.weiny@intel.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > --- > 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: linux-cxl@vger.kernel.org > +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 < mds->nr_dc_region) > + return a->mode; > + > + return 0; > +} > + > +static bool cxl_memdev_dc_group_visible(struct kobject *kobj) > +{ > + 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); > + > + /* No DC regions */ > + if (!mds || mds->nr_dc_region == 0) > + return false; > + return true; > +} > + > +DEFINE_SYSFS_GROUP_VISIBLE(cxl_memdev_dc); > + > +static struct attribute_group cxl_memdev_dc_group = { > + .name = "dc", > + .attrs = cxl_memdev_dc_attributes, > + .is_visible = SYSFS_GROUP_VISIBLE(cxl_memdev_dc), > +}; > + > static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a, > int n) > { > @@ -528,6 +624,7 @@ static const struct attribute_group *cxl_memdev_attribute_groups[] = { > &cxl_memdev_ram_attribute_group, > &cxl_memdev_pmem_attribute_group, > &cxl_memdev_security_attribute_group, > + &cxl_memdev_dc_group, > NULL, > }; > >
Dave Jiang wrote: > > > On 8/16/24 7:44 AM, ira.weiny@intel.com wrote: > > From: Navneet Singh <navneet.singh@intel.com> > > > > 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 <navneet.singh@intel.com> > > Co-developed-by: Ira Weiny <ira.weiny@intel.com> > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > > > --- > > 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: linux-cxl@vger.kernel.org > > +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]
On 8/22/24 7:28 PM, Ira Weiny wrote: > Dave Jiang wrote: >> >> >> On 8/16/24 7:44 AM, ira.weiny@intel.com wrote: >>> From: Navneet Singh <navneet.singh@intel.com> >>> >>> 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 <navneet.singh@intel.com> >>> Co-developed-by: Ira Weiny <ira.weiny@intel.com> >>> Signed-off-by: Ira Weiny <ira.weiny@intel.com> >>> >>> --- >>> 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: linux-cxl@vger.kernel.org >>> +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]
On Thu, 22 Aug 2024 21:28:41 -0500 Ira Weiny <ira.weiny@intel.com> wrote: > Dave Jiang wrote: > > > > > > On 8/16/24 7:44 AM, ira.weiny@intel.com wrote: > > > From: Navneet Singh <navneet.singh@intel.com> > > > > > > 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 <navneet.singh@intel.com> > > > Co-developed-by: Ira Weiny <ira.weiny@intel.com> > > > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > > > > > > --- > > > 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: linux-cxl@vger.kernel.org > > > +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] >
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: linux-cxl@vger.kernel.org +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 < mds->nr_dc_region) + return a->mode; + + return 0; +} + +static bool cxl_memdev_dc_group_visible(struct kobject *kobj) +{ + 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); + + /* No DC regions */ + if (!mds || mds->nr_dc_region == 0) + return false; + return true; +} + +DEFINE_SYSFS_GROUP_VISIBLE(cxl_memdev_dc); + +static struct attribute_group cxl_memdev_dc_group = { + .name = "dc", + .attrs = cxl_memdev_dc_attributes, + .is_visible = SYSFS_GROUP_VISIBLE(cxl_memdev_dc), +}; + static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a, int n) { @@ -528,6 +624,7 @@ static const struct attribute_group *cxl_memdev_attribute_groups[] = { &cxl_memdev_ram_attribute_group, &cxl_memdev_pmem_attribute_group, &cxl_memdev_security_attribute_group, + &cxl_memdev_dc_group, NULL, };