Re: [PATCH 3/5] cxl/mem : Expose dynamic capacity configuration to userspace

[Alison Schofield <alison.schofield@intel.com>]

On Wed, Jun 14, 2023 at 12:16:30PM -0700, Ira Weiny wrote:
> From: Navneet Singh <navneet.singh@intel.com>
> 
> Exposing driver cached dynamic capacity configuration through sysfs
> attributes.User will create one or more dynamic capacity
> cxl regions based on this information and map the dynamic capacity of
> the device into HDM ranges using one or more HDM decoders.
> 
> Signed-off-by: Navneet Singh <navneet.singh@intel.com>
> 
> ---
> [iweiny: fixups]
> [djbw: fixups, no sign-off: preview only]
> ---
>  drivers/cxl/core/memdev.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 72 insertions(+)

Add the documentation of these new attributes in this patch.
Documentation/ABI/testing/sysfs-bus-cxl

A bit of my ignorance here, but when I keep seeing the word
'regions' below, it makes me wonder whether these attributes
are in the right place?

> 
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index 5d1ba7a72567..beeb5fa3a0aa 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -99,6 +99,20 @@ 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 dc_regions_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);
> +	int len = 0;
> +
> +	len = sysfs_emit(buf, "0x%x\n", mds->nr_dc_region);

Prefer using this notation: %#llx
grep for the sysfs_emit's to see customary usage.

> +	return len;
> +}
> +
> +struct device_attribute dev_attr_dc_regions_count =
> +	__ATTR(dc_regions_count, 0444, dc_regions_count_show, NULL);
> +
>  static ssize_t serial_show(struct device *dev, struct device_attribute *attr,
>  			   char *buf)
>  {
> @@ -362,6 +376,57 @@ static struct attribute *cxl_memdev_ram_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, "0x%llx\n", mds->dc_region[pos].decode_len);
> +}
> +
> +#define SIZE_ATTR_RO(n)                                              \
> +static ssize_t dc##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(dc##n##_size)
> +SIZE_ATTR_RO(0);
> +SIZE_ATTR_RO(1);
> +SIZE_ATTR_RO(2);
> +SIZE_ATTR_RO(3);
> +SIZE_ATTR_RO(4);
> +SIZE_ATTR_RO(5);
> +SIZE_ATTR_RO(6);
> +SIZE_ATTR_RO(7);
> +
> +static struct attribute *cxl_memdev_dc_attributes[] = {
> +	&dev_attr_dc0_size.attr,
> +	&dev_attr_dc1_size.attr,
> +	&dev_attr_dc2_size.attr,
> +	&dev_attr_dc3_size.attr,
> +	&dev_attr_dc4_size.attr,
> +	&dev_attr_dc5_size.attr,
> +	&dev_attr_dc6_size.attr,
> +	&dev_attr_dc7_size.attr,
> +	&dev_attr_dc_regions_count.attr,
> +	NULL,
> +};
> +
> +static umode_t cxl_dc_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);
> +
> +	if (a == &dev_attr_dc_regions_count.attr)
> +		return a->mode;
> +
> +	if (n < mds->nr_dc_region)
> +		return a->mode;
> +
> +	return 0;
> +}
> +
>  static umode_t cxl_memdev_visible(struct kobject *kobj, struct attribute *a,
>  				  int n)
>  {
> @@ -385,10 +450,17 @@ static struct attribute_group cxl_memdev_pmem_attribute_group = {
>  	.attrs = cxl_memdev_pmem_attributes,
>  };
>  
> +static struct attribute_group cxl_memdev_dc_attribute_group = {
> +	.name = "dc",
> +	.attrs = cxl_memdev_dc_attributes,
> +	.is_visible = cxl_dc_visible,
> +};
> +
>  static const struct attribute_group *cxl_memdev_attribute_groups[] = {
>  	&cxl_memdev_attribute_group,
>  	&cxl_memdev_ram_attribute_group,
>  	&cxl_memdev_pmem_attribute_group,
> +	&cxl_memdev_dc_attribute_group,
>  	NULL,
>  };
>  
> 
> -- 
> 2.40.0
>