Re: [PATCH 20/46] cxl/mem: Add a debugfs version of 'iomem' for DPA, 'dpamem'

[Jonathan Cameron <Jonathan.Cameron@Huawei.com>]

On Thu, 23 Jun 2022 19:47:33 -0700
Dan Williams <dan.j.williams@intel.com> wrote:

> Dump the device-physial-address map for a CXL expander in /proc/iomem
> style format. E.g.:
> 
>   cat /sys/kernel/debug/cxl/mem1/dpamem
>   00000000-0fffffff : ram
>   10000000-1fffffff : pmem

Nice in general, but...

When I just checked what this looked like on my test setup. I'm 
seeing
00000000-0ffffff : pmem
  00000000-0fffff : endpoint3

Seems odd to see an endpoint nested below a pmem.  Wrong name somewhere
in a later patch. I'd expect that to be a decoder rather than the endpoint...
If I spot where that comes from whilst reviewing I'll call it out, but
didn't want to forget to raise it.

This patch is fine.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> 
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/core/core.h |    1 -
>  drivers/cxl/core/hdm.c  |   23 +++++++++++++++++++++++
>  drivers/cxl/core/port.c |    1 +
>  drivers/cxl/cxlmem.h    |    4 ++++
>  drivers/cxl/mem.c       |   23 +++++++++++++++++++++++
>  5 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index c242fa02d5e8..472ec9cb1018 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -24,7 +24,6 @@ int cxl_dpa_free(struct cxl_endpoint_decoder *cxled);
>  resource_size_t cxl_dpa_size(struct cxl_endpoint_decoder *cxled);
>  resource_size_t cxl_dpa_resource(struct cxl_endpoint_decoder *cxled);
>  
> -struct dentry *cxl_debugfs_create_dir(const char *dir);
>  int cxl_memdev_init(void);
>  void cxl_memdev_exit(void);
>  void cxl_mbox_init(void);
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index ceb4c28abc1b..c0164f9b2195 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright(c) 2022 Intel Corporation. All rights reserved. */
>  #include <linux/io-64-nonatomic-hi-lo.h>
> +#include <linux/seq_file.h>
>  #include <linux/device.h>
>  #include <linux/delay.h>
>  
> @@ -248,6 +249,28 @@ static int cxl_dpa_reserve(struct cxl_endpoint_decoder *cxled,
>  	return devm_add_action_or_reset(&port->dev, cxl_dpa_release, cxled);
>  }
>  
> +static void __cxl_dpa_debug(struct seq_file *file, struct resource *r, int depth)
> +{
> +	unsigned long long start = r->start, end = r->end;
> +
> +	seq_printf(file, "%*s%08llx-%08llx : %s\n", depth * 2, "", start, end,
> +		   r->name);
> +}
> +
> +void cxl_dpa_debug(struct seq_file *file, struct cxl_dev_state *cxlds)
> +{
> +	struct resource *p1, *p2;
> +
> +	down_read(&cxl_dpa_rwsem);
> +	for (p1 = cxlds->dpa_res.child; p1; p1 = p1->sibling) {
> +		__cxl_dpa_debug(file, p1, 0);
> +		for (p2 = p1->child; p2; p2 = p2->sibling)
> +			__cxl_dpa_debug(file, p2, 1);
> +	}
> +	up_read(&cxl_dpa_rwsem);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_dpa_debug, CXL);
> +
>  resource_size_t cxl_dpa_size(struct cxl_endpoint_decoder *cxled)
>  {
>  	resource_size_t size = 0;
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index f02b7470c20e..4e4e26ca507c 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1702,6 +1702,7 @@ struct dentry *cxl_debugfs_create_dir(const char *dir)
>  {
>  	return debugfs_create_dir(dir, cxl_debugfs);
>  }
> +EXPORT_SYMBOL_NS_GPL(cxl_debugfs_create_dir, CXL);
>  
>  static __init int cxl_core_init(void)
>  {
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index b4e5ed9eabc9..db9c889f42ab 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -385,4 +385,8 @@ struct cxl_hdm {
>  	unsigned int interleave_mask;
>  	struct cxl_port *port;
>  };
> +
> +struct seq_file;
> +struct dentry *cxl_debugfs_create_dir(const char *dir);
> +void cxl_dpa_debug(struct seq_file *file, struct cxl_dev_state *cxlds);
>  #endif /* __CXL_MEM_H__ */
> diff --git a/drivers/cxl/mem.c b/drivers/cxl/mem.c
> index a979d0b484d5..7513bea55145 100644
> --- a/drivers/cxl/mem.c
> +++ b/drivers/cxl/mem.c
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright(c) 2022 Intel Corporation. All rights reserved. */
> +#include <linux/debugfs.h>
>  #include <linux/device.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
> @@ -56,10 +57,26 @@ static void enable_suspend(void *data)
>  	cxl_mem_active_dec();
>  }
>  
> +static void remove_debugfs(void *dentry)
> +{
> +	debugfs_remove_recursive(dentry);
> +}
> +
> +static int cxl_mem_dpa_show(struct seq_file *file, void *data)
> +{
> +	struct device *dev = file->private;
> +	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
> +
> +	cxl_dpa_debug(file, cxlmd->cxlds);
> +
> +	return 0;
> +}
> +
>  static int cxl_mem_probe(struct device *dev)
>  {
>  	struct cxl_memdev *cxlmd = to_cxl_memdev(dev);
>  	struct cxl_port *parent_port;
> +	struct dentry *dentry;
>  	int rc;
>  
>  	/*
> @@ -73,6 +90,12 @@ static int cxl_mem_probe(struct device *dev)
>  	if (work_pending(&cxlmd->detach_work))
>  		return -EBUSY;
>  
> +	dentry = cxl_debugfs_create_dir(dev_name(dev));
> +	debugfs_create_devm_seqfile(dev, "dpamem", dentry, cxl_mem_dpa_show);
> +	rc = devm_add_action_or_reset(dev, remove_debugfs, dentry);
> +	if (rc)
> +		return rc;
> +
>  	rc = devm_cxl_enumerate_ports(cxlmd);
>  	if (rc)
>  		return rc;
>