[PATCH 1/2] cdx: create sysfs bin files for cdx resources

[PATCH 1/2] cdx: create sysfs bin files for cdx resources

[Abhijit Gangurde]

Resource binary file contains the content of the memory regions.
These resources<x> devices can be used to mmap the MMIO regions in
the user-space.

Co-developed-by: Puneet Gupta <puneet.gupta@amd.com>
Signed-off-by: Puneet Gupta <puneet.gupta@amd.com>
Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
---
 Documentation/ABI/testing/sysfs-bus-cdx |   7 ++
 drivers/cdx/cdx.c                       | 118 +++++++++++++++++++++++-
 include/linux/cdx/cdx_bus.h             |  10 ++
 3 files changed, 134 insertions(+), 1 deletion(-)

diff --git a/Documentation/ABI/testing/sysfs-bus-cdx b/Documentation/ABI/testing/sysfs-bus-cdx
index 8c067ff99e54..e84277531414 100644
--- a/Documentation/ABI/testing/sysfs-bus-cdx
+++ b/Documentation/ABI/testing/sysfs-bus-cdx
@@ -98,6 +98,13 @@ Description:
 
 		  # echo 1 > /sys/bus/cdx/devices/.../remove
 
+What:		/sys/bus/cdx/devices/.../resource<N>
+Date:		July 2023
+Contact:	puneet.gupta@amd.com
+Description:
+		The resource binary file contains the content of the memory
+		regions. These files can be m'maped from userspace.
+
 What:		/sys/bus/cdx/devices/.../modalias
 Date:		July 2023
 Contact:	nipun.gupta@amd.com
diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
index c5873980a9d3..4edf64f9e98d 100644
--- a/drivers/cdx/cdx.c
+++ b/drivers/cdx/cdx.c
@@ -78,6 +78,8 @@ static DEFINE_MUTEX(cdx_controller_lock);
 
 static char *compat_node_name = "xlnx,versal-net-cdx";
 
+static void cdx_destroy_res_attr(struct cdx_device *cdx_dev, int num);
+
 /**
  * cdx_dev_reset - Reset a CDX device
  * @dev: CDX device
@@ -146,6 +148,7 @@ static int cdx_unregister_device(struct device *dev,
 		if (cdx_dev->enabled && cdx->ops->bus_disable)
 			cdx->ops->bus_disable(cdx, cdx_dev->bus_num);
 	} else {
+		cdx_destroy_res_attr(cdx_dev, MAX_CDX_DEV_RESOURCES);
 		kfree(cdx_dev->driver_override);
 		cdx_dev->driver_override = NULL;
 	}
@@ -641,11 +644,105 @@ static void cdx_device_release(struct device *dev)
 	kfree(cdx_dev);
 }
 
+static const struct vm_operations_struct cdx_phys_vm_ops = {
+#ifdef CONFIG_HAVE_IOREMAP_PROT
+	.access = generic_access_phys,
+#endif
+};
+
+/**
+ * cdx_mmap_resource - map a CDX resource into user memory space
+ * @fp: File pointer. Not used in this function, but required where
+ *      this API is registered as a callback.
+ * @kobj: kobject for mapping
+ * @attr: struct bin_attribute for the file being mapped
+ * @vma: struct vm_area_struct passed into the mmap
+ *
+ * Use the regular CDX mapping routines to map a CDX resource into userspace.
+ *
+ * Return: true on success, false otherwise.
+ */
+static int cdx_mmap_resource(struct file *fp, struct kobject *kobj,
+			     struct bin_attribute *attr,
+			     struct vm_area_struct *vma)
+{
+	struct cdx_device *cdx_dev = to_cdx_device(kobj_to_dev(kobj));
+	int num = (unsigned long)attr->private;
+	struct resource *res;
+	unsigned long size;
+
+	res = &cdx_dev->res[num];
+	if (iomem_is_exclusive(res->start))
+		return -EINVAL;
+
+	/* Make sure the caller is mapping a valid resource for this device */
+	size = ((cdx_resource_len(cdx_dev, num) - 1) >> PAGE_SHIFT) + 1;
+	if (vma->vm_pgoff + vma_pages(vma) > size)
+		return -EINVAL;
+
+	/*
+	 * Map memory region and vm->vm_pgoff is expected to be an
+	 * offset within that region.
+	 */
+	vma->vm_page_prot = pgprot_device(vma->vm_page_prot);
+	vma->vm_pgoff += (cdx_resource_start(cdx_dev, num) >> PAGE_SHIFT);
+	vma->vm_ops = &cdx_phys_vm_ops;
+	return io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
+				  vma->vm_end - vma->vm_start,
+				  vma->vm_page_prot);
+}
+
+static void cdx_destroy_res_attr(struct cdx_device *cdx_dev, int num)
+{
+	int i;
+
+	/* removing the bin attributes */
+	for (i = 0; i < num; i++) {
+		struct bin_attribute *res_attr;
+
+		res_attr = cdx_dev->res_attr[i];
+		if (res_attr) {
+			sysfs_remove_bin_file(&cdx_dev->dev.kobj, res_attr);
+			kfree(res_attr);
+		}
+	}
+}
+
+#define CDX_RES_ATTR_NAME_LEN	10
+static int cdx_create_res_attr(struct cdx_device *cdx_dev, int num)
+{
+	struct bin_attribute *res_attr;
+	char *res_attr_name;
+	int ret;
+
+	res_attr = kzalloc(sizeof(*res_attr) + CDX_RES_ATTR_NAME_LEN, GFP_ATOMIC);
+	if (!res_attr)
+		return -ENOMEM;
+
+	res_attr_name = (char *)(res_attr + 1);
+
+	sysfs_bin_attr_init(res_attr);
+
+	cdx_dev->res_attr[num] = res_attr;
+	sprintf(res_attr_name, "resource%d", num);
+
+	res_attr->mmap = cdx_mmap_resource;
+	res_attr->attr.name = res_attr_name;
+	res_attr->attr.mode = 0600;
+	res_attr->size = cdx_resource_len(cdx_dev, num);
+	res_attr->private = (void *)(unsigned long)num;
+	ret = sysfs_create_bin_file(&cdx_dev->dev.kobj, res_attr);
+	if (ret)
+		kfree(res_attr);
+
+	return ret;
+}
+
 int cdx_device_add(struct cdx_dev_params *dev_params)
 {
 	struct cdx_controller *cdx = dev_params->cdx;
 	struct cdx_device *cdx_dev;
-	int ret;
+	int ret, i;
 
 	cdx_dev = kzalloc(sizeof(*cdx_dev), GFP_KERNEL);
 	if (!cdx_dev)
@@ -696,7 +793,26 @@ int cdx_device_add(struct cdx_dev_params *dev_params)
 		goto fail;
 	}
 
+	/* Create resource<N> attributes */
+	for (i = 0; i < MAX_CDX_DEV_RESOURCES; i++) {
+		if (cdx_resource_flags(cdx_dev, i) & IORESOURCE_MEM) {
+			/* skip empty resources */
+			if (!cdx_resource_len(cdx_dev, i))
+				continue;
+
+			ret = cdx_create_res_attr(cdx_dev, i);
+			if (ret != 0) {
+				dev_err(&cdx_dev->dev,
+					"cdx device resource<%d> file creation failed: %d", i, ret);
+				goto resource_create_fail;
+			}
+		}
+	}
+
 	return 0;
+resource_create_fail:
+	cdx_destroy_res_attr(cdx_dev, i);
+	device_del(&cdx_dev->dev);
 fail:
 	/*
 	 * Do not free cdx_dev here as it would be freed in
diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
index db39835b93d2..3096c31d5b9f 100644
--- a/include/linux/cdx/cdx_bus.h
+++ b/include/linux/cdx/cdx_bus.h
@@ -154,6 +154,7 @@ struct cdx_device {
 	u8 bus_num;
 	u8 dev_num;
 	struct resource res[MAX_CDX_DEV_RESOURCES];
+	struct bin_attribute *res_attr[MAX_CDX_DEV_RESOURCES];
 	u8 res_count;
 	u64 dma_mask;
 	u16 flags;
@@ -170,6 +171,15 @@ struct cdx_device {
 #define to_cdx_device(_dev) \
 	container_of(_dev, struct cdx_device, dev)
 
+#define cdx_resource_start(dev, num)	((dev)->res[(num)].start)
+#define cdx_resource_end(dev, num)	((dev)->res[(num)].end)
+#define cdx_resource_flags(dev, num)	((dev)->res[(num)].flags)
+#define cdx_resource_len(dev, num) \
+	((cdx_resource_start((dev), (num)) == 0 &&	\
+	  cdx_resource_end((dev), (num)) ==		\
+	  cdx_resource_start((dev), (num))) ? 0 :	\
+	 (cdx_resource_end((dev), (num)) -		\
+	  cdx_resource_start((dev), (num)) + 1))
 /**
  * struct cdx_driver - CDX device driver
  * @driver: Generic device driver
-- 
2.34.1

[PATCH 2/2] cdx: Create resource debugfs file for cdx device

[Abhijit Gangurde]

resource debugfs file contains host addresses of CDX device resources.
Each line of the resource file describe a region with start, end and
flag fields.

Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
---
 drivers/cdx/cdx.c           | 42 ++++++++++++++++++++++++++++++++++++-
 include/linux/cdx/cdx_bus.h |  2 ++
 2 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
index 4edf64f9e98d..e01376272ed3 100644
--- a/drivers/cdx/cdx.c
+++ b/drivers/cdx/cdx.c
@@ -65,6 +65,7 @@
 #include <linux/cdx/cdx_bus.h>
 #include <linux/iommu.h>
 #include <linux/dma-map-ops.h>
+#include <linux/debugfs.h>
 #include "cdx.h"
 
 /* Default DMA mask for devices on a CDX bus */
@@ -75,6 +76,8 @@
 static DEFINE_IDA(cdx_controller_ida);
 /* Lock to protect controller ops */
 static DEFINE_MUTEX(cdx_controller_lock);
+/* Debugfs dir for cdx bus */
+static struct dentry *cdx_debugfs_dir;
 
 static char *compat_node_name = "xlnx,versal-net-cdx";
 
@@ -149,6 +152,7 @@ static int cdx_unregister_device(struct device *dev,
 			cdx->ops->bus_disable(cdx, cdx_dev->bus_num);
 	} else {
 		cdx_destroy_res_attr(cdx_dev, MAX_CDX_DEV_RESOURCES);
+		debugfs_remove_recursive(cdx_dev->debugfs_dir);
 		kfree(cdx_dev->driver_override);
 		cdx_dev->driver_override = NULL;
 	}
@@ -552,6 +556,34 @@ static const struct attribute_group *cdx_dev_groups[] = {
 	NULL,
 };
 
+static int cdx_debug_resource_show(struct seq_file *s, void *data)
+{
+	struct cdx_device *cdx_dev = s->private;
+	int i;
+
+	for (i = 0; i < MAX_CDX_DEV_RESOURCES; i++) {
+		struct resource *res =  &cdx_dev->res[i];
+
+		seq_printf(s, "0x%016llx 0x%016llx 0x%016llx\n",
+			   (unsigned long long)res->start,
+			   (unsigned long long)res->end,
+			   (unsigned long long)res->flags);
+	}
+
+	return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(cdx_debug_resource);
+
+static void cdx_device_debugfs_init(struct cdx_device *cdx_dev)
+{
+	cdx_dev->debugfs_dir = debugfs_create_dir(dev_name(&cdx_dev->dev), cdx_debugfs_dir);
+	if (IS_ERR(cdx_dev->debugfs_dir))
+		return;
+
+	debugfs_create_file("resource", 0444, cdx_dev->debugfs_dir, cdx_dev,
+			    &cdx_debug_resource_fops);
+}
+
 static ssize_t rescan_store(const struct bus_type *bus,
 			    const char *buf, size_t count)
 {
@@ -809,6 +841,8 @@ int cdx_device_add(struct cdx_dev_params *dev_params)
 		}
 	}
 
+	cdx_device_debugfs_init(cdx_dev);
+
 	return 0;
 resource_create_fail:
 	cdx_destroy_res_attr(cdx_dev, i);
@@ -913,6 +947,12 @@ EXPORT_SYMBOL_NS_GPL(cdx_unregister_controller, CDX_BUS_CONTROLLER);
 
 static int __init cdx_bus_init(void)
 {
-	return bus_register(&cdx_bus_type);
+	int ret;
+
+	ret = bus_register(&cdx_bus_type);
+	if (!ret)
+		cdx_debugfs_dir = debugfs_create_dir(cdx_bus_type.name, NULL);
+
+	return ret;
 }
 postcore_initcall(cdx_bus_init);
diff --git a/include/linux/cdx/cdx_bus.h b/include/linux/cdx/cdx_bus.h
index 3096c31d5b9f..681fd4d644ab 100644
--- a/include/linux/cdx/cdx_bus.h
+++ b/include/linux/cdx/cdx_bus.h
@@ -128,6 +128,7 @@ struct cdx_controller {
  * @dev_num: Device number for this device
  * @res: array of MMIO region entries
  * @res_attr: resource binary attribute
+ * @debugfs_dir: debugfs directory for this device
  * @res_count: number of valid MMIO regions
  * @dma_mask: Default DMA mask
  * @flags: CDX device flags
@@ -155,6 +156,7 @@ struct cdx_device {
 	u8 dev_num;
 	struct resource res[MAX_CDX_DEV_RESOURCES];
 	struct bin_attribute *res_attr[MAX_CDX_DEV_RESOURCES];
+	struct dentry *debugfs_dir;
 	u8 res_count;
 	u64 dma_mask;
 	u16 flags;
-- 
2.34.1

Re: [PATCH 2/2] cdx: Create resource debugfs file for cdx device

[Greg KH]

On Wed, Nov 22, 2023 at 01:33:00PM +0530, Abhijit Gangurde wrote:
> resource debugfs file contains host addresses of CDX device resources.
> Each line of the resource file describe a region with start, end and
> flag fields.
> 
> Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
> ---
>  drivers/cdx/cdx.c           | 42 ++++++++++++++++++++++++++++++++++++-
>  include/linux/cdx/cdx_bus.h |  2 ++
>  2 files changed, 43 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
> index 4edf64f9e98d..e01376272ed3 100644
> --- a/drivers/cdx/cdx.c
> +++ b/drivers/cdx/cdx.c
> @@ -65,6 +65,7 @@
>  #include <linux/cdx/cdx_bus.h>
>  #include <linux/iommu.h>
>  #include <linux/dma-map-ops.h>
> +#include <linux/debugfs.h>
>  #include "cdx.h"
>  
>  /* Default DMA mask for devices on a CDX bus */
> @@ -75,6 +76,8 @@
>  static DEFINE_IDA(cdx_controller_ida);
>  /* Lock to protect controller ops */
>  static DEFINE_MUTEX(cdx_controller_lock);
> +/* Debugfs dir for cdx bus */
> +static struct dentry *cdx_debugfs_dir;
>  
>  static char *compat_node_name = "xlnx,versal-net-cdx";
>  
> @@ -149,6 +152,7 @@ static int cdx_unregister_device(struct device *dev,
>  			cdx->ops->bus_disable(cdx, cdx_dev->bus_num);
>  	} else {
>  		cdx_destroy_res_attr(cdx_dev, MAX_CDX_DEV_RESOURCES);
> +		debugfs_remove_recursive(cdx_dev->debugfs_dir);
>  		kfree(cdx_dev->driver_override);
>  		cdx_dev->driver_override = NULL;
>  	}
> @@ -552,6 +556,34 @@ static const struct attribute_group *cdx_dev_groups[] = {
>  	NULL,
>  };
>  
> +static int cdx_debug_resource_show(struct seq_file *s, void *data)
> +{
> +	struct cdx_device *cdx_dev = s->private;
> +	int i;
> +
> +	for (i = 0; i < MAX_CDX_DEV_RESOURCES; i++) {
> +		struct resource *res =  &cdx_dev->res[i];
> +
> +		seq_printf(s, "0x%016llx 0x%016llx 0x%016llx\n",
> +			   (unsigned long long)res->start,
> +			   (unsigned long long)res->end,
> +			   (unsigned long long)res->flags);

Why not just use %pR or %pr instead of creating your own style of
format?

thanks,

greg k-h

RE: [PATCH 2/2] cdx: Create resource debugfs file for cdx device

[Gangurde, Abhijit]

> > resource debugfs file contains host addresses of CDX device resources.
> > Each line of the resource file describe a region with start, end and
> > flag fields.
> >
> > Signed-off-by: Abhijit Gangurde <abhijit.gangurde@amd.com>
> > ---
> >  drivers/cdx/cdx.c           | 42 ++++++++++++++++++++++++++++++++++++-
> >  include/linux/cdx/cdx_bus.h |  2 ++
> >  2 files changed, 43 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/cdx/cdx.c b/drivers/cdx/cdx.c
> > index 4edf64f9e98d..e01376272ed3 100644
> > --- a/drivers/cdx/cdx.c
> > +++ b/drivers/cdx/cdx.c
> > @@ -65,6 +65,7 @@
> >  #include <linux/cdx/cdx_bus.h>
> >  #include <linux/iommu.h>
> >  #include <linux/dma-map-ops.h>
> > +#include <linux/debugfs.h>
> >  #include "cdx.h"
> >
> >  /* Default DMA mask for devices on a CDX bus */
> > @@ -75,6 +76,8 @@
> >  static DEFINE_IDA(cdx_controller_ida);
> >  /* Lock to protect controller ops */
> >  static DEFINE_MUTEX(cdx_controller_lock);
> > +/* Debugfs dir for cdx bus */
> > +static struct dentry *cdx_debugfs_dir;
> >
> >  static char *compat_node_name = "xlnx,versal-net-cdx";
> >
> > @@ -149,6 +152,7 @@ static int cdx_unregister_device(struct device *dev,
> >  			cdx->ops->bus_disable(cdx, cdx_dev->bus_num);
> >  	} else {
> >  		cdx_destroy_res_attr(cdx_dev, MAX_CDX_DEV_RESOURCES);
> > +		debugfs_remove_recursive(cdx_dev->debugfs_dir);
> >  		kfree(cdx_dev->driver_override);
> >  		cdx_dev->driver_override = NULL;
> >  	}
> > @@ -552,6 +556,34 @@ static const struct attribute_group
> *cdx_dev_groups[] = {
> >  	NULL,
> >  };
> >
> > +static int cdx_debug_resource_show(struct seq_file *s, void *data)
> > +{
> > +	struct cdx_device *cdx_dev = s->private;
> > +	int i;
> > +
> > +	for (i = 0; i < MAX_CDX_DEV_RESOURCES; i++) {
> > +		struct resource *res =  &cdx_dev->res[i];
> > +
> > +		seq_printf(s, "0x%016llx 0x%016llx 0x%016llx\n",
> > +			   (unsigned long long)res->start,
> > +			   (unsigned long long)res->end,
> > +			   (unsigned long long)res->flags);
> 
> Why not just use %pR or %pr instead of creating your own style of
> format?

Will correct this in next spin.

Thanks,
Abhijit