All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] ntb: Fixes and enhancements to ntb tools
@ 2016-06-03 20:50 Logan Gunthorpe
  2016-06-03 20:50 ` [PATCH 1/3] ntb_perf: Allow limiting the size of the memory windows Logan Gunthorpe
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Logan Gunthorpe @ 2016-06-03 20:50 UTC (permalink / raw)
  To: Jon Mason, Dave Jiang, Allen Hubbe, John Kading, Sudip Mukherjee,
	Arnd Bergmann
  Cc: linux-ntb, linux-kernel, Logan Gunthorpe

Hi,

I've been working on developing an experimental NTB driver for some
custom hardware and I've found the need to make some minor enhancements
to the NTB layer.

1) I've modified ntb_perf to take an option similar to ntb_transport
which limits the memory window size. This was useful seeing the mws
I'm dealing with are much larger than the available coherent memory I
can allocate.

2) I've added code to ntb_perf and ntb_transport to check that there
are enough scratchpad registers. (As I was hit by a problem where my
hardware did not have enough for ntb_transport, and I would have liked
better information on the cause of the issue.)

3) Added support to debug and test memory windows to ntb_tool. A
coherent buffer is added when the link comes up, and then a debugfs
file for each mw and peer mw is added which allows reading and writing
the buffer. This was useful for me to debug window alignment issues I
was having.

I'm happy to make any revisions to these patches if anyone finds any
issues.

Thanks,

Logan


Logan Gunthorpe (3):
  ntb_perf: Allow limiting the size of the memory windows
  ntb_transport: Check the number of spads the hardware supports
  ntb_tool: Add memory window debug support

 drivers/ntb/ntb_transport.c |   9 +-
 drivers/ntb/test/ntb_perf.c |  16 ++-
 drivers/ntb/test/ntb_tool.c | 258 +++++++++++++++++++++++++++++++++++++++++++-
 3 files changed, 278 insertions(+), 5 deletions(-)

--
2.1.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] ntb_perf: Allow limiting the size of the memory windows
  2016-06-03 20:50 [PATCH 0/3] ntb: Fixes and enhancements to ntb tools Logan Gunthorpe
@ 2016-06-03 20:50 ` Logan Gunthorpe
  2016-06-03 21:03   ` Jiang, Dave
  2016-06-03 20:50 ` [PATCH 2/3] ntb_transport: Check the number of spads the hardware supports Logan Gunthorpe
  2016-06-03 20:50 ` [PATCH 3/3] ntb_tool: Add memory window debug support Logan Gunthorpe
  2 siblings, 1 reply; 14+ messages in thread
From: Logan Gunthorpe @ 2016-06-03 20:50 UTC (permalink / raw)
  To: Jon Mason, Dave Jiang, Allen Hubbe, John Kading, Sudip Mukherjee,
	Arnd Bergmann
  Cc: linux-ntb, linux-kernel, Logan Gunthorpe

On my system, dma_alloc_coherent won't produce memory anywhere
near the size of the BAR. So I needed a way to limit this.

It's pretty much copied straight from ntb_transport.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/ntb/test/ntb_perf.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
index 8dfce9c..30635c8 100644
--- a/drivers/ntb/test/ntb_perf.c
+++ b/drivers/ntb/test/ntb_perf.c
@@ -83,6 +83,10 @@ MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
 
 static struct dentry *perf_debugfs_dir;
 
+static unsigned long max_mw_size;
+module_param(max_mw_size, ulong, 0644);
+MODULE_PARM_DESC(max_mw_size, "Limit size of large memory windows");
+
 static unsigned int seg_order = 19; /* 512K */
 module_param(seg_order, uint, 0644);
 MODULE_PARM_DESC(seg_order, "size order [n^2] of buffer segment for testing");
@@ -472,6 +476,10 @@ static void perf_link_work(struct work_struct *work)
 	dev_dbg(&perf->ntb->pdev->dev, "%s called\n", __func__);
 
 	size = perf->mw.phys_size;
+
+	if (max_mw_size && size > max_mw_size)
+		size = max_mw_size;
+
 	ntb_peer_spad_write(ndev, MW_SZ_HIGH, upper_32_bits(size));
 	ntb_peer_spad_write(ndev, MW_SZ_LOW, lower_32_bits(size));
 	ntb_peer_spad_write(ndev, VERSION, PERF_VERSION);
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/3] ntb_transport: Check the number of spads the hardware supports
  2016-06-03 20:50 [PATCH 0/3] ntb: Fixes and enhancements to ntb tools Logan Gunthorpe
  2016-06-03 20:50 ` [PATCH 1/3] ntb_perf: Allow limiting the size of the memory windows Logan Gunthorpe
@ 2016-06-03 20:50 ` Logan Gunthorpe
  2016-06-04 15:40   ` Jon Mason
  2016-06-03 20:50 ` [PATCH 3/3] ntb_tool: Add memory window debug support Logan Gunthorpe
  2 siblings, 1 reply; 14+ messages in thread
From: Logan Gunthorpe @ 2016-06-03 20:50 UTC (permalink / raw)
  To: Jon Mason, Dave Jiang, Allen Hubbe, John Kading, Sudip Mukherjee,
	Arnd Bergmann
  Cc: linux-ntb, linux-kernel, Logan Gunthorpe

I'm working on hardware that currently has a limited number of
scratchpad registers and ntb_ndev fails with no clue as to why. I
feel it is better to fail early and provide a reasonable error message
then to fail later on..

The same is done to ntb_perf, but it doesn't currently require enough
spads to actually fail.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/ntb/ntb_transport.c | 9 +++++++--
 drivers/ntb/test/ntb_perf.c | 8 ++++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 2ef9d913..34e7a7a 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1037,6 +1037,13 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
 	int node;
 	int rc, i;
 
+	mw_count = ntb_mw_count(ndev);
+	if (ntb_spad_count(ndev) < (NUM_MWS + 1 + mw_count*2)) {
+		dev_err(&ndev->dev, "Not enough scratch pad registers for %s",
+			NTB_TRANSPORT_NAME);
+		return -EIO;
+	}
+
 	if (ntb_db_is_unsafe(ndev))
 		dev_dbg(&ndev->dev,
 			"doorbell is unsafe, proceed anyway...\n");
@@ -1052,8 +1059,6 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
 
 	nt->ndev = ndev;
 
-	mw_count = ntb_mw_count(ndev);
-
 	nt->mw_count = mw_count;
 
 	nt->mw_vec = kzalloc_node(mw_count * sizeof(*nt->mw_vec),
diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
index 30635c8..1815592 100644
--- a/drivers/ntb/test/ntb_perf.c
+++ b/drivers/ntb/test/ntb_perf.c
@@ -143,8 +143,6 @@ enum {
 	VERSION = 0,
 	MW_SZ_HIGH,
 	MW_SZ_LOW,
-	SPAD_MSG,
-	SPAD_ACK,
 	MAX_SPAD
 };
 
@@ -698,6 +696,12 @@ static int perf_probe(struct ntb_client *client, struct ntb_dev *ntb)
 
 	node = dev_to_node(&pdev->dev);
 
+	if (ntb_spad_count(ntb) < MAX_SPAD) {
+		dev_err(&ntb->dev, "Not enough scratch pad registers for %s",
+			DRIVER_NAME);
+		return -EIO;
+	}
+
 	perf = kzalloc_node(sizeof(*perf), GFP_KERNEL, node);
 	if (!perf) {
 		rc = -ENOMEM;
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/3] ntb_tool: Add memory window debug support
  2016-06-03 20:50 [PATCH 0/3] ntb: Fixes and enhancements to ntb tools Logan Gunthorpe
  2016-06-03 20:50 ` [PATCH 1/3] ntb_perf: Allow limiting the size of the memory windows Logan Gunthorpe
  2016-06-03 20:50 ` [PATCH 2/3] ntb_transport: Check the number of spads the hardware supports Logan Gunthorpe
@ 2016-06-03 20:50 ` Logan Gunthorpe
  2016-06-03 21:20     ` Allen Hubbe
  2 siblings, 1 reply; 14+ messages in thread
From: Logan Gunthorpe @ 2016-06-03 20:50 UTC (permalink / raw)
  To: Jon Mason, Dave Jiang, Allen Hubbe, John Kading, Sudip Mukherjee,
	Arnd Bergmann
  Cc: linux-ntb, linux-kernel, Logan Gunthorpe

We allocate some memory window buffers when the link comes up, then we
provide debugfs files to read/write each side of the link.

This is useful for debugging the mapping when writing new drivers.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/ntb/test/ntb_tool.c | 258 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 257 insertions(+), 1 deletion(-)

diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
index 209ef7c..4c01057 100644
--- a/drivers/ntb/test/ntb_tool.c
+++ b/drivers/ntb/test/ntb_tool.c
@@ -89,6 +89,7 @@
 #include <linux/dma-mapping.h>
 #include <linux/pci.h>
 #include <linux/slab.h>
+#include <linux/uaccess.h>
 
 #include <linux/ntb.h>
 
@@ -105,11 +106,29 @@ MODULE_VERSION(DRIVER_VERSION);
 MODULE_AUTHOR(DRIVER_AUTHOR);
 MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
 
+#define MAX_MWS 16
+
+static unsigned long mw_size = 16;
+module_param(mw_size, ulong, 0644);
+MODULE_PARM_DESC(mw_size, "size order [n^2] of the memory window for testing");
+
 static struct dentry *tool_dbgfs;
 
+struct tool_mw {
+	resource_size_t size;
+	u8 __iomem *local;
+	u8 *peer;
+	dma_addr_t peer_dma;
+};
+
 struct tool_ctx {
 	struct ntb_dev *ntb;
 	struct dentry *dbgfs;
+	struct work_struct link_cleanup;
+	bool link_is_up;
+	struct delayed_work link_work;
+	int mw_count;
+	struct tool_mw mws[MAX_MWS];
 };
 
 #define SPAD_FNAME_SIZE 0x10
@@ -124,6 +143,111 @@ struct tool_ctx {
 		.write = __write,		\
 	}
 
+static int tool_setup_mw(struct tool_ctx *tc, int idx)
+{
+	int rc;
+	struct tool_mw *mw = &tc->mws[idx];
+	phys_addr_t base;
+	resource_size_t size, align, align_size;
+
+	if (mw->local)
+		return 0;
+
+	rc = ntb_mw_get_range(tc->ntb, idx, &base, &size, &align,
+			      &align_size);
+	if (rc)
+		return rc;
+
+	mw->size = min_t(resource_size_t, 1 << mw_size, size);
+	mw->size = round_up(mw->size, align);
+	mw->size = round_up(mw->size, align_size);
+
+	mw->local = ioremap_wc(base, size);
+	if (mw->local == NULL)
+		return -EFAULT;
+
+	mw->peer = dma_alloc_coherent(&tc->ntb->pdev->dev, mw->size,
+				      &mw->peer_dma, GFP_KERNEL);
+
+	if (mw->peer == NULL)
+		return -ENOMEM;
+
+	rc = ntb_mw_set_trans(tc->ntb, idx, mw->peer_dma, mw->size);
+	if (rc)
+		return rc;
+
+	return 0;
+}
+
+static void tool_free_mws(struct tool_ctx *tc)
+{
+	int i;
+
+	for (i = 0; i < tc->mw_count; i++) {
+		if (tc->mws[i].peer) {
+			ntb_mw_clear_trans(tc->ntb, i);
+			dma_free_coherent(&tc->ntb->pdev->dev, tc->mws[i].size,
+					  tc->mws[i].peer,
+					  tc->mws[i].peer_dma);
+
+		}
+
+		tc->mws[i].peer = NULL;
+		tc->mws[i].peer_dma = 0;
+
+		if (tc->mws[i].local)
+			iounmap(tc->mws[i].local);
+
+		tc->mws[i].local = NULL;
+	}
+
+	tc->mw_count = 0;
+}
+
+static int tool_setup_mws(struct tool_ctx *tc)
+{
+	int i;
+	int rc;
+
+	tc->mw_count = min(ntb_mw_count(tc->ntb), MAX_MWS);
+
+	for (i = 0; i < tc->mw_count; i++) {
+		rc = tool_setup_mw(tc, i);
+		if (rc)
+			goto err_out;
+	}
+
+	return 0;
+
+err_out:
+	tool_free_mws(tc);
+	return rc;
+}
+
+static void tool_link_work(struct work_struct *work)
+{
+	int rc;
+	struct tool_ctx *tc = container_of(work, struct tool_ctx,
+					   link_work.work);
+
+	tool_free_mws(tc);
+	rc = tool_setup_mws(tc);
+	if (rc)
+		dev_err(&tc->ntb->dev,
+			"Error setting up memory windows: %d\n", rc);
+
+	tc->link_is_up = true;
+}
+
+static void tool_link_cleanup(struct work_struct *work)
+{
+	struct tool_ctx *tc = container_of(work, struct tool_ctx,
+					   link_cleanup);
+
+	if (!tc->link_is_up)
+		cancel_delayed_work_sync(&tc->link_work);
+}
+
 static void tool_link_event(void *ctx)
 {
 	struct tool_ctx *tc = ctx;
@@ -135,6 +259,11 @@ static void tool_link_event(void *ctx)
 
 	dev_dbg(&tc->ntb->dev, "link is %s speed %d width %d\n",
 		up ? "up" : "down", speed, width);
+
+	if (up)
+		schedule_delayed_work(&tc->link_work, 2*HZ);
+	else
+		schedule_work(&tc->link_cleanup);
 }
 
 static void tool_db_event(void *ctx, int vec)
@@ -443,8 +572,112 @@ static TOOL_FOPS_RDWR(tool_peer_spad_fops,
 		      tool_peer_spad_read,
 		      tool_peer_spad_write);
 
+
+static ssize_t tool_mw_read(struct file *filep, char __user *ubuf,
+			    size_t size, loff_t *offp)
+{
+	struct tool_mw *mw = filep->private_data;
+	ssize_t rc;
+	loff_t pos = *offp;
+	void *buf;
+
+	if (mw->local == NULL)
+		return -EIO;
+	if (pos < 0)
+		return -EINVAL;
+	if (pos >= mw->size || !size)
+		return 0;
+	if (size > mw->size - pos)
+		size = mw->size - pos;
+
+	buf = kmalloc(size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	memcpy_fromio(buf, mw->local + pos, size);
+	rc = copy_to_user(ubuf, buf, size);
+	if (rc == size) {
+		rc = -EFAULT;
+		goto err_free;
+	}
+
+	size -= rc;
+	*offp = pos + size;
+	rc = size;
+
+err_free:
+	kfree(buf);
+
+	return rc;
+}
+
+static ssize_t tool_mw_write(struct file *filep, const char __user *ubuf,
+			     size_t size, loff_t *offp)
+{
+	struct tool_mw *mw = filep->private_data;
+	ssize_t rc;
+	loff_t pos = *offp;
+	void *buf;
+
+	if (pos < 0)
+		return -EINVAL;
+	if (pos >= mw->size || !size)
+		return 0;
+	if (size > mw->size - pos)
+		size = mw->size - pos;
+
+	buf = kmalloc(size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	rc = copy_from_user(buf, ubuf, size);
+	if (rc == size) {
+		rc = -EFAULT;
+		goto err_free;
+	}
+
+	size -= rc;
+	*offp = pos + size;
+	rc = size;
+
+	memcpy_toio(mw->local + pos, buf, size);
+
+err_free:
+	kfree(buf);
+
+	return rc;
+}
+
+static TOOL_FOPS_RDWR(tool_mw_fops,
+		      tool_mw_read,
+		      tool_mw_write);
+
+
+static ssize_t tool_peer_mw_read(struct file *filep, char __user *ubuf,
+				   size_t size, loff_t *offp)
+{
+	struct tool_mw *mw = filep->private_data;
+
+	return simple_read_from_buffer(ubuf, size, offp, mw->peer, mw->size);
+}
+
+static ssize_t tool_peer_mw_write(struct file *filep, const char __user *ubuf,
+				    size_t size, loff_t *offp)
+{
+	struct tool_mw *mw = filep->private_data;
+
+	return simple_write_to_buffer(mw->peer, mw->size, offp, ubuf, size);
+}
+
+static TOOL_FOPS_RDWR(tool_peer_mw_fops,
+		      tool_peer_mw_read,
+		      tool_peer_mw_write);
+
 static void tool_setup_dbgfs(struct tool_ctx *tc)
 {
+	int mw_count;
+	int i;
+
 	/* This modules is useless without dbgfs... */
 	if (!tool_dbgfs) {
 		tc->dbgfs = NULL;
@@ -473,6 +706,20 @@ static void tool_setup_dbgfs(struct tool_ctx *tc)
 
 	debugfs_create_file("peer_spad", S_IRUSR | S_IWUSR, tc->dbgfs,
 			    tc, &tool_peer_spad_fops);
+
+	mw_count = min(ntb_mw_count(tc->ntb), MAX_MWS);
+	for (i = 0; i < mw_count; i++) {
+		char buf[30];
+
+		snprintf(buf, sizeof(buf), "mw%d", i);
+		debugfs_create_file(buf, S_IRUSR | S_IWUSR, tc->dbgfs,
+				    &tc->mws[i], &tool_mw_fops);
+
+		snprintf(buf, sizeof(buf), "peer_mw%d", i);
+		debugfs_create_file(buf, S_IRUSR | S_IWUSR, tc->dbgfs,
+				    &tc->mws[i], &tool_peer_mw_fops);
+
+	}
 }
 
 static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
@@ -486,13 +733,15 @@ static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
 	if (ntb_spad_is_unsafe(ntb))
 		dev_dbg(&ntb->dev, "scratchpad is unsafe\n");
 
-	tc = kmalloc(sizeof(*tc), GFP_KERNEL);
+	tc = kzalloc(sizeof(*tc), GFP_KERNEL);
 	if (!tc) {
 		rc = -ENOMEM;
 		goto err_tc;
 	}
 
 	tc->ntb = ntb;
+	INIT_DELAYED_WORK(&tc->link_work, tool_link_work);
+	INIT_WORK(&tc->link_cleanup, tool_link_cleanup);
 
 	tool_setup_dbgfs(tc);
 
@@ -507,6 +756,8 @@ static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
 
 err_ctx:
 	debugfs_remove_recursive(tc->dbgfs);
+	cancel_delayed_work_sync(&tc->link_work);
+	cancel_work_sync(&tc->link_cleanup);
 	kfree(tc);
 err_tc:
 	return rc;
@@ -516,6 +767,11 @@ static void tool_remove(struct ntb_client *self, struct ntb_dev *ntb)
 {
 	struct tool_ctx *tc = ntb->ctx;
 
+	cancel_delayed_work_sync(&tc->link_work);
+	cancel_work_sync(&tc->link_cleanup);
+
+	tool_free_mws(tc);
+
 	ntb_clear_ctx(ntb);
 	ntb_link_disable(ntb);
 
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] ntb_perf: Allow limiting the size of the memory windows
  2016-06-03 20:50 ` [PATCH 1/3] ntb_perf: Allow limiting the size of the memory windows Logan Gunthorpe
@ 2016-06-03 21:03   ` Jiang, Dave
  2016-06-04 15:25     ` Jon Mason
  0 siblings, 1 reply; 14+ messages in thread
From: Jiang, Dave @ 2016-06-03 21:03 UTC (permalink / raw)
  To: Allen.Hubbe@emc.com, logang@deltatee.com, jdmason@kudzu.us,
	sudipm.mukherjee@gmail.com, arnd@arndb.de, john.kading@gd-ms.com
  Cc: linux-kernel@vger.kernel.org, linux-ntb@googlegroups.com

On Fri, 2016-06-03 at 14:50 -0600, Logan Gunthorpe wrote:
> On my system, dma_alloc_coherent won't produce memory anywhere
> near the size of the BAR. So I needed a way to limit this.
> 
> It's pretty much copied straight from ntb_transport.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/ntb/test/ntb_perf.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/ntb/test/ntb_perf.c
> b/drivers/ntb/test/ntb_perf.c
> index 8dfce9c..30635c8 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -83,6 +83,10 @@ MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
>  
>  static struct dentry *perf_debugfs_dir;
>  
> +static unsigned long max_mw_size;
> +module_param(max_mw_size, ulong, 0644);
> +MODULE_PARM_DESC(max_mw_size, "Limit size of large memory windows");
> +
>  static unsigned int seg_order = 19; /* 512K */
>  module_param(seg_order, uint, 0644);
>  MODULE_PARM_DESC(seg_order, "size order [n^2] of buffer segment for
> testing");
> @@ -472,6 +476,10 @@ static void perf_link_work(struct work_struct
> *work)
>  	dev_dbg(&perf->ntb->pdev->dev, "%s called\n", __func__);
>  
>  	size = perf->mw.phys_size;
> +
> +	if (max_mw_size && size > max_mw_size)
> +		size = max_mw_size;
> +
>  	ntb_peer_spad_write(ndev, MW_SZ_HIGH, upper_32_bits(size));
>  	ntb_peer_spad_write(ndev, MW_SZ_LOW, lower_32_bits(size));
>  	ntb_peer_spad_write(ndev, VERSION, PERF_VERSION);
> -- 
> 2.1.4
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 3/3] ntb_tool: Add memory window debug support
  2016-06-03 20:50 ` [PATCH 3/3] ntb_tool: Add memory window debug support Logan Gunthorpe
@ 2016-06-03 21:20     ` Allen Hubbe
  0 siblings, 0 replies; 14+ messages in thread
From: Allen Hubbe @ 2016-06-03 21:20 UTC (permalink / raw)
  To: 'Logan Gunthorpe', 'Jon Mason',
	'Dave Jiang', 'John Kading',
	'Sudip Mukherjee', 'Arnd Bergmann'
  Cc: linux-ntb, linux-kernel

From: Logan Gunthorpe 
> We allocate some memory window buffers when the link comes up, then we
> provide debugfs files to read/write each side of the link.
> 
> This is useful for debugging the mapping when writing new drivers.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>

Thanks! This was on my wish list.

Acked-by: Allen Hubbe <Allen.Hubbe@emc.com>

> ---
>  drivers/ntb/test/ntb_tool.c | 258 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 257 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
> index 209ef7c..4c01057 100644
> --- a/drivers/ntb/test/ntb_tool.c
> +++ b/drivers/ntb/test/ntb_tool.c
> @@ -89,6 +89,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/pci.h>
>  #include <linux/slab.h>
> +#include <linux/uaccess.h>
> 
>  #include <linux/ntb.h>
> 
> @@ -105,11 +106,29 @@ MODULE_VERSION(DRIVER_VERSION);
>  MODULE_AUTHOR(DRIVER_AUTHOR);
>  MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
> 
> +#define MAX_MWS 16
> +
> +static unsigned long mw_size = 16;
> +module_param(mw_size, ulong, 0644);
> +MODULE_PARM_DESC(mw_size, "size order [n^2] of the memory window for testing");
> +
>  static struct dentry *tool_dbgfs;
> 
> +struct tool_mw {
> +	resource_size_t size;
> +	u8 __iomem *local;
> +	u8 *peer;
> +	dma_addr_t peer_dma;
> +};
> +
>  struct tool_ctx {
>  	struct ntb_dev *ntb;
>  	struct dentry *dbgfs;
> +	struct work_struct link_cleanup;
> +	bool link_is_up;
> +	struct delayed_work link_work;
> +	int mw_count;
> +	struct tool_mw mws[MAX_MWS];
>  };
> 
>  #define SPAD_FNAME_SIZE 0x10
> @@ -124,6 +143,111 @@ struct tool_ctx {
>  		.write = __write,		\
>  	}
> 
> +static int tool_setup_mw(struct tool_ctx *tc, int idx)
> +{
> +	int rc;
> +	struct tool_mw *mw = &tc->mws[idx];
> +	phys_addr_t base;
> +	resource_size_t size, align, align_size;
> +
> +	if (mw->local)
> +		return 0;
> +
> +	rc = ntb_mw_get_range(tc->ntb, idx, &base, &size, &align,
> +			      &align_size);
> +	if (rc)
> +		return rc;
> +
> +	mw->size = min_t(resource_size_t, 1 << mw_size, size);
> +	mw->size = round_up(mw->size, align);
> +	mw->size = round_up(mw->size, align_size);
> +
> +	mw->local = ioremap_wc(base, size);
> +	if (mw->local == NULL)
> +		return -EFAULT;
> +
> +	mw->peer = dma_alloc_coherent(&tc->ntb->pdev->dev, mw->size,
> +				      &mw->peer_dma, GFP_KERNEL);
> +
> +	if (mw->peer == NULL)
> +		return -ENOMEM;
> +
> +	rc = ntb_mw_set_trans(tc->ntb, idx, mw->peer_dma, mw->size);
> +	if (rc)
> +		return rc;
> +
> +	return 0;
> +}
> +
> +static void tool_free_mws(struct tool_ctx *tc)
> +{
> +	int i;
> +
> +	for (i = 0; i < tc->mw_count; i++) {
> +		if (tc->mws[i].peer) {
> +			ntb_mw_clear_trans(tc->ntb, i);
> +			dma_free_coherent(&tc->ntb->pdev->dev, tc->mws[i].size,
> +					  tc->mws[i].peer,
> +					  tc->mws[i].peer_dma);
> +
> +		}
> +
> +		tc->mws[i].peer = NULL;
> +		tc->mws[i].peer_dma = 0;
> +
> +		if (tc->mws[i].local)
> +			iounmap(tc->mws[i].local);
> +
> +		tc->mws[i].local = NULL;
> +	}
> +
> +	tc->mw_count = 0;
> +}
> +
> +static int tool_setup_mws(struct tool_ctx *tc)
> +{
> +	int i;
> +	int rc;
> +
> +	tc->mw_count = min(ntb_mw_count(tc->ntb), MAX_MWS);
> +
> +	for (i = 0; i < tc->mw_count; i++) {
> +		rc = tool_setup_mw(tc, i);
> +		if (rc)
> +			goto err_out;
> +	}
> +
> +	return 0;
> +
> +err_out:
> +	tool_free_mws(tc);
> +	return rc;
> +}
> +
> +static void tool_link_work(struct work_struct *work)
> +{
> +	int rc;
> +	struct tool_ctx *tc = container_of(work, struct tool_ctx,
> +					   link_work.work);
> +
> +	tool_free_mws(tc);
> +	rc = tool_setup_mws(tc);
> +	if (rc)
> +		dev_err(&tc->ntb->dev,
> +			"Error setting up memory windows: %d\n", rc);
> +
> +	tc->link_is_up = true;
> +}
> +
> +static void tool_link_cleanup(struct work_struct *work)
> +{
> +	struct tool_ctx *tc = container_of(work, struct tool_ctx,
> +					   link_cleanup);
> +
> +	if (!tc->link_is_up)
> +		cancel_delayed_work_sync(&tc->link_work);
> +}
> +
>  static void tool_link_event(void *ctx)
>  {
>  	struct tool_ctx *tc = ctx;
> @@ -135,6 +259,11 @@ static void tool_link_event(void *ctx)
> 
>  	dev_dbg(&tc->ntb->dev, "link is %s speed %d width %d\n",
>  		up ? "up" : "down", speed, width);
> +
> +	if (up)
> +		schedule_delayed_work(&tc->link_work, 2*HZ);
> +	else
> +		schedule_work(&tc->link_cleanup);
>  }
> 
>  static void tool_db_event(void *ctx, int vec)
> @@ -443,8 +572,112 @@ static TOOL_FOPS_RDWR(tool_peer_spad_fops,
>  		      tool_peer_spad_read,
>  		      tool_peer_spad_write);
> 
> +
> +static ssize_t tool_mw_read(struct file *filep, char __user *ubuf,
> +			    size_t size, loff_t *offp)
> +{
> +	struct tool_mw *mw = filep->private_data;
> +	ssize_t rc;
> +	loff_t pos = *offp;
> +	void *buf;
> +
> +	if (mw->local == NULL)
> +		return -EIO;
> +	if (pos < 0)
> +		return -EINVAL;
> +	if (pos >= mw->size || !size)
> +		return 0;
> +	if (size > mw->size - pos)
> +		size = mw->size - pos;
> +
> +	buf = kmalloc(size, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	memcpy_fromio(buf, mw->local + pos, size);
> +	rc = copy_to_user(ubuf, buf, size);
> +	if (rc == size) {
> +		rc = -EFAULT;
> +		goto err_free;
> +	}
> +
> +	size -= rc;
> +	*offp = pos + size;
> +	rc = size;
> +
> +err_free:
> +	kfree(buf);
> +
> +	return rc;
> +}
> +
> +static ssize_t tool_mw_write(struct file *filep, const char __user *ubuf,
> +			     size_t size, loff_t *offp)
> +{
> +	struct tool_mw *mw = filep->private_data;
> +	ssize_t rc;
> +	loff_t pos = *offp;
> +	void *buf;
> +
> +	if (pos < 0)
> +		return -EINVAL;
> +	if (pos >= mw->size || !size)
> +		return 0;
> +	if (size > mw->size - pos)
> +		size = mw->size - pos;
> +
> +	buf = kmalloc(size, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	rc = copy_from_user(buf, ubuf, size);
> +	if (rc == size) {
> +		rc = -EFAULT;
> +		goto err_free;
> +	}
> +
> +	size -= rc;
> +	*offp = pos + size;
> +	rc = size;
> +
> +	memcpy_toio(mw->local + pos, buf, size);
> +
> +err_free:
> +	kfree(buf);
> +
> +	return rc;
> +}
> +
> +static TOOL_FOPS_RDWR(tool_mw_fops,
> +		      tool_mw_read,
> +		      tool_mw_write);
> +
> +
> +static ssize_t tool_peer_mw_read(struct file *filep, char __user *ubuf,
> +				   size_t size, loff_t *offp)
> +{
> +	struct tool_mw *mw = filep->private_data;
> +
> +	return simple_read_from_buffer(ubuf, size, offp, mw->peer, mw->size);
> +}
> +
> +static ssize_t tool_peer_mw_write(struct file *filep, const char __user *ubuf,
> +				    size_t size, loff_t *offp)
> +{
> +	struct tool_mw *mw = filep->private_data;
> +
> +	return simple_write_to_buffer(mw->peer, mw->size, offp, ubuf, size);
> +}
> +
> +static TOOL_FOPS_RDWR(tool_peer_mw_fops,
> +		      tool_peer_mw_read,
> +		      tool_peer_mw_write);
> +
>  static void tool_setup_dbgfs(struct tool_ctx *tc)
>  {
> +	int mw_count;
> +	int i;
> +
>  	/* This modules is useless without dbgfs... */
>  	if (!tool_dbgfs) {
>  		tc->dbgfs = NULL;
> @@ -473,6 +706,20 @@ static void tool_setup_dbgfs(struct tool_ctx *tc)
> 
>  	debugfs_create_file("peer_spad", S_IRUSR | S_IWUSR, tc->dbgfs,
>  			    tc, &tool_peer_spad_fops);
> +
> +	mw_count = min(ntb_mw_count(tc->ntb), MAX_MWS);
> +	for (i = 0; i < mw_count; i++) {
> +		char buf[30];
> +
> +		snprintf(buf, sizeof(buf), "mw%d", i);
> +		debugfs_create_file(buf, S_IRUSR | S_IWUSR, tc->dbgfs,
> +				    &tc->mws[i], &tool_mw_fops);
> +
> +		snprintf(buf, sizeof(buf), "peer_mw%d", i);
> +		debugfs_create_file(buf, S_IRUSR | S_IWUSR, tc->dbgfs,
> +				    &tc->mws[i], &tool_peer_mw_fops);
> +
> +	}
>  }
> 
>  static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
> @@ -486,13 +733,15 @@ static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
>  	if (ntb_spad_is_unsafe(ntb))
>  		dev_dbg(&ntb->dev, "scratchpad is unsafe\n");
> 
> -	tc = kmalloc(sizeof(*tc), GFP_KERNEL);
> +	tc = kzalloc(sizeof(*tc), GFP_KERNEL);
>  	if (!tc) {
>  		rc = -ENOMEM;
>  		goto err_tc;
>  	}
> 
>  	tc->ntb = ntb;
> +	INIT_DELAYED_WORK(&tc->link_work, tool_link_work);
> +	INIT_WORK(&tc->link_cleanup, tool_link_cleanup);
> 
>  	tool_setup_dbgfs(tc);
> 
> @@ -507,6 +756,8 @@ static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
> 
>  err_ctx:
>  	debugfs_remove_recursive(tc->dbgfs);
> +	cancel_delayed_work_sync(&tc->link_work);
> +	cancel_work_sync(&tc->link_cleanup);
>  	kfree(tc);
>  err_tc:
>  	return rc;
> @@ -516,6 +767,11 @@ static void tool_remove(struct ntb_client *self, struct ntb_dev *ntb)
>  {
>  	struct tool_ctx *tc = ntb->ctx;
> 
> +	cancel_delayed_work_sync(&tc->link_work);
> +	cancel_work_sync(&tc->link_cleanup);
> +
> +	tool_free_mws(tc);
> +
>  	ntb_clear_ctx(ntb);
>  	ntb_link_disable(ntb);
> 
> --
> 2.1.4



^ permalink raw reply	[flat|nested] 14+ messages in thread

* RE: [PATCH 3/3] ntb_tool: Add memory window debug support
@ 2016-06-03 21:20     ` Allen Hubbe
  0 siblings, 0 replies; 14+ messages in thread
From: Allen Hubbe @ 2016-06-03 21:20 UTC (permalink / raw)
  To: 'Logan Gunthorpe', 'Jon Mason',
	'Dave Jiang', 'John Kading',
	'Sudip Mukherjee', 'Arnd Bergmann'
  Cc: linux-ntb, linux-kernel

From: Logan Gunthorpe 
> We allocate some memory window buffers when the link comes up, then we
> provide debugfs files to read/write each side of the link.
> 
> This is useful for debugging the mapping when writing new drivers.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>

Thanks! This was on my wish list.

Acked-by: Allen Hubbe <Allen.Hubbe@emc.com>

> ---
>  drivers/ntb/test/ntb_tool.c | 258 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 257 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
> index 209ef7c..4c01057 100644
> --- a/drivers/ntb/test/ntb_tool.c
> +++ b/drivers/ntb/test/ntb_tool.c
> @@ -89,6 +89,7 @@
>  #include <linux/dma-mapping.h>
>  #include <linux/pci.h>
>  #include <linux/slab.h>
> +#include <linux/uaccess.h>
> 
>  #include <linux/ntb.h>
> 
> @@ -105,11 +106,29 @@ MODULE_VERSION(DRIVER_VERSION);
>  MODULE_AUTHOR(DRIVER_AUTHOR);
>  MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
> 
> +#define MAX_MWS 16
> +
> +static unsigned long mw_size = 16;
> +module_param(mw_size, ulong, 0644);
> +MODULE_PARM_DESC(mw_size, "size order [n^2] of the memory window for testing");
> +
>  static struct dentry *tool_dbgfs;
> 
> +struct tool_mw {
> +	resource_size_t size;
> +	u8 __iomem *local;
> +	u8 *peer;
> +	dma_addr_t peer_dma;
> +};
> +
>  struct tool_ctx {
>  	struct ntb_dev *ntb;
>  	struct dentry *dbgfs;
> +	struct work_struct link_cleanup;
> +	bool link_is_up;
> +	struct delayed_work link_work;
> +	int mw_count;
> +	struct tool_mw mws[MAX_MWS];
>  };
> 
>  #define SPAD_FNAME_SIZE 0x10
> @@ -124,6 +143,111 @@ struct tool_ctx {
>  		.write = __write,		\
>  	}
> 
> +static int tool_setup_mw(struct tool_ctx *tc, int idx)
> +{
> +	int rc;
> +	struct tool_mw *mw = &tc->mws[idx];
> +	phys_addr_t base;
> +	resource_size_t size, align, align_size;
> +
> +	if (mw->local)
> +		return 0;
> +
> +	rc = ntb_mw_get_range(tc->ntb, idx, &base, &size, &align,
> +			      &align_size);
> +	if (rc)
> +		return rc;
> +
> +	mw->size = min_t(resource_size_t, 1 << mw_size, size);
> +	mw->size = round_up(mw->size, align);
> +	mw->size = round_up(mw->size, align_size);
> +
> +	mw->local = ioremap_wc(base, size);
> +	if (mw->local == NULL)
> +		return -EFAULT;
> +
> +	mw->peer = dma_alloc_coherent(&tc->ntb->pdev->dev, mw->size,
> +				      &mw->peer_dma, GFP_KERNEL);
> +
> +	if (mw->peer == NULL)
> +		return -ENOMEM;
> +
> +	rc = ntb_mw_set_trans(tc->ntb, idx, mw->peer_dma, mw->size);
> +	if (rc)
> +		return rc;
> +
> +	return 0;
> +}
> +
> +static void tool_free_mws(struct tool_ctx *tc)
> +{
> +	int i;
> +
> +	for (i = 0; i < tc->mw_count; i++) {
> +		if (tc->mws[i].peer) {
> +			ntb_mw_clear_trans(tc->ntb, i);
> +			dma_free_coherent(&tc->ntb->pdev->dev, tc->mws[i].size,
> +					  tc->mws[i].peer,
> +					  tc->mws[i].peer_dma);
> +
> +		}
> +
> +		tc->mws[i].peer = NULL;
> +		tc->mws[i].peer_dma = 0;
> +
> +		if (tc->mws[i].local)
> +			iounmap(tc->mws[i].local);
> +
> +		tc->mws[i].local = NULL;
> +	}
> +
> +	tc->mw_count = 0;
> +}
> +
> +static int tool_setup_mws(struct tool_ctx *tc)
> +{
> +	int i;
> +	int rc;
> +
> +	tc->mw_count = min(ntb_mw_count(tc->ntb), MAX_MWS);
> +
> +	for (i = 0; i < tc->mw_count; i++) {
> +		rc = tool_setup_mw(tc, i);
> +		if (rc)
> +			goto err_out;
> +	}
> +
> +	return 0;
> +
> +err_out:
> +	tool_free_mws(tc);
> +	return rc;
> +}
> +
> +static void tool_link_work(struct work_struct *work)
> +{
> +	int rc;
> +	struct tool_ctx *tc = container_of(work, struct tool_ctx,
> +					   link_work.work);
> +
> +	tool_free_mws(tc);
> +	rc = tool_setup_mws(tc);
> +	if (rc)
> +		dev_err(&tc->ntb->dev,
> +			"Error setting up memory windows: %d\n", rc);
> +
> +	tc->link_is_up = true;
> +}
> +
> +static void tool_link_cleanup(struct work_struct *work)
> +{
> +	struct tool_ctx *tc = container_of(work, struct tool_ctx,
> +					   link_cleanup);
> +
> +	if (!tc->link_is_up)
> +		cancel_delayed_work_sync(&tc->link_work);
> +}
> +
>  static void tool_link_event(void *ctx)
>  {
>  	struct tool_ctx *tc = ctx;
> @@ -135,6 +259,11 @@ static void tool_link_event(void *ctx)
> 
>  	dev_dbg(&tc->ntb->dev, "link is %s speed %d width %d\n",
>  		up ? "up" : "down", speed, width);
> +
> +	if (up)
> +		schedule_delayed_work(&tc->link_work, 2*HZ);
> +	else
> +		schedule_work(&tc->link_cleanup);
>  }
> 
>  static void tool_db_event(void *ctx, int vec)
> @@ -443,8 +572,112 @@ static TOOL_FOPS_RDWR(tool_peer_spad_fops,
>  		      tool_peer_spad_read,
>  		      tool_peer_spad_write);
> 
> +
> +static ssize_t tool_mw_read(struct file *filep, char __user *ubuf,
> +			    size_t size, loff_t *offp)
> +{
> +	struct tool_mw *mw = filep->private_data;
> +	ssize_t rc;
> +	loff_t pos = *offp;
> +	void *buf;
> +
> +	if (mw->local == NULL)
> +		return -EIO;
> +	if (pos < 0)
> +		return -EINVAL;
> +	if (pos >= mw->size || !size)
> +		return 0;
> +	if (size > mw->size - pos)
> +		size = mw->size - pos;
> +
> +	buf = kmalloc(size, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	memcpy_fromio(buf, mw->local + pos, size);
> +	rc = copy_to_user(ubuf, buf, size);
> +	if (rc == size) {
> +		rc = -EFAULT;
> +		goto err_free;
> +	}
> +
> +	size -= rc;
> +	*offp = pos + size;
> +	rc = size;
> +
> +err_free:
> +	kfree(buf);
> +
> +	return rc;
> +}
> +
> +static ssize_t tool_mw_write(struct file *filep, const char __user *ubuf,
> +			     size_t size, loff_t *offp)
> +{
> +	struct tool_mw *mw = filep->private_data;
> +	ssize_t rc;
> +	loff_t pos = *offp;
> +	void *buf;
> +
> +	if (pos < 0)
> +		return -EINVAL;
> +	if (pos >= mw->size || !size)
> +		return 0;
> +	if (size > mw->size - pos)
> +		size = mw->size - pos;
> +
> +	buf = kmalloc(size, GFP_KERNEL);
> +	if (!buf)
> +		return -ENOMEM;
> +
> +	rc = copy_from_user(buf, ubuf, size);
> +	if (rc == size) {
> +		rc = -EFAULT;
> +		goto err_free;
> +	}
> +
> +	size -= rc;
> +	*offp = pos + size;
> +	rc = size;
> +
> +	memcpy_toio(mw->local + pos, buf, size);
> +
> +err_free:
> +	kfree(buf);
> +
> +	return rc;
> +}
> +
> +static TOOL_FOPS_RDWR(tool_mw_fops,
> +		      tool_mw_read,
> +		      tool_mw_write);
> +
> +
> +static ssize_t tool_peer_mw_read(struct file *filep, char __user *ubuf,
> +				   size_t size, loff_t *offp)
> +{
> +	struct tool_mw *mw = filep->private_data;
> +
> +	return simple_read_from_buffer(ubuf, size, offp, mw->peer, mw->size);
> +}
> +
> +static ssize_t tool_peer_mw_write(struct file *filep, const char __user *ubuf,
> +				    size_t size, loff_t *offp)
> +{
> +	struct tool_mw *mw = filep->private_data;
> +
> +	return simple_write_to_buffer(mw->peer, mw->size, offp, ubuf, size);
> +}
> +
> +static TOOL_FOPS_RDWR(tool_peer_mw_fops,
> +		      tool_peer_mw_read,
> +		      tool_peer_mw_write);
> +
>  static void tool_setup_dbgfs(struct tool_ctx *tc)
>  {
> +	int mw_count;
> +	int i;
> +
>  	/* This modules is useless without dbgfs... */
>  	if (!tool_dbgfs) {
>  		tc->dbgfs = NULL;
> @@ -473,6 +706,20 @@ static void tool_setup_dbgfs(struct tool_ctx *tc)
> 
>  	debugfs_create_file("peer_spad", S_IRUSR | S_IWUSR, tc->dbgfs,
>  			    tc, &tool_peer_spad_fops);
> +
> +	mw_count = min(ntb_mw_count(tc->ntb), MAX_MWS);
> +	for (i = 0; i < mw_count; i++) {
> +		char buf[30];
> +
> +		snprintf(buf, sizeof(buf), "mw%d", i);
> +		debugfs_create_file(buf, S_IRUSR | S_IWUSR, tc->dbgfs,
> +				    &tc->mws[i], &tool_mw_fops);
> +
> +		snprintf(buf, sizeof(buf), "peer_mw%d", i);
> +		debugfs_create_file(buf, S_IRUSR | S_IWUSR, tc->dbgfs,
> +				    &tc->mws[i], &tool_peer_mw_fops);
> +
> +	}
>  }
> 
>  static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
> @@ -486,13 +733,15 @@ static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
>  	if (ntb_spad_is_unsafe(ntb))
>  		dev_dbg(&ntb->dev, "scratchpad is unsafe\n");
> 
> -	tc = kmalloc(sizeof(*tc), GFP_KERNEL);
> +	tc = kzalloc(sizeof(*tc), GFP_KERNEL);
>  	if (!tc) {
>  		rc = -ENOMEM;
>  		goto err_tc;
>  	}
> 
>  	tc->ntb = ntb;
> +	INIT_DELAYED_WORK(&tc->link_work, tool_link_work);
> +	INIT_WORK(&tc->link_cleanup, tool_link_cleanup);
> 
>  	tool_setup_dbgfs(tc);
> 
> @@ -507,6 +756,8 @@ static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
> 
>  err_ctx:
>  	debugfs_remove_recursive(tc->dbgfs);
> +	cancel_delayed_work_sync(&tc->link_work);
> +	cancel_work_sync(&tc->link_cleanup);
>  	kfree(tc);
>  err_tc:
>  	return rc;
> @@ -516,6 +767,11 @@ static void tool_remove(struct ntb_client *self, struct ntb_dev *ntb)
>  {
>  	struct tool_ctx *tc = ntb->ctx;
> 
> +	cancel_delayed_work_sync(&tc->link_work);
> +	cancel_work_sync(&tc->link_cleanup);
> +
> +	tool_free_mws(tc);
> +
>  	ntb_clear_ctx(ntb);
>  	ntb_link_disable(ntb);
> 
> --
> 2.1.4

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] ntb_perf: Allow limiting the size of the memory windows
  2016-06-03 21:03   ` Jiang, Dave
@ 2016-06-04 15:25     ` Jon Mason
  0 siblings, 0 replies; 14+ messages in thread
From: Jon Mason @ 2016-06-04 15:25 UTC (permalink / raw)
  To: Jiang, Dave
  Cc: Allen.Hubbe@emc.com, logang@deltatee.com,
	sudipm.mukherjee@gmail.com, arnd@arndb.de, john.kading@gd-ms.com,
	linux-kernel@vger.kernel.org, linux-ntb@googlegroups.com

On Fri, Jun 3, 2016 at 5:03 PM, Jiang, Dave <dave.jiang@intel.com> wrote:
> On Fri, 2016-06-03 at 14:50 -0600, Logan Gunthorpe wrote:
>> On my system, dma_alloc_coherent won't produce memory anywhere
>> near the size of the BAR. So I needed a way to limit this.
>>
>> It's pretty much copied straight from ntb_transport.
>>
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> Acked-by: Dave Jiang <dave.jiang@intel.com>

Applied to my ntb-next branch

Thanks,
Jon

>> ---
>>  drivers/ntb/test/ntb_perf.c | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/ntb/test/ntb_perf.c
>> b/drivers/ntb/test/ntb_perf.c
>> index 8dfce9c..30635c8 100644
>> --- a/drivers/ntb/test/ntb_perf.c
>> +++ b/drivers/ntb/test/ntb_perf.c
>> @@ -83,6 +83,10 @@ MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
>>
>>  static struct dentry *perf_debugfs_dir;
>>
>> +static unsigned long max_mw_size;
>> +module_param(max_mw_size, ulong, 0644);
>> +MODULE_PARM_DESC(max_mw_size, "Limit size of large memory windows");
>> +
>>  static unsigned int seg_order = 19; /* 512K */
>>  module_param(seg_order, uint, 0644);
>>  MODULE_PARM_DESC(seg_order, "size order [n^2] of buffer segment for
>> testing");
>> @@ -472,6 +476,10 @@ static void perf_link_work(struct work_struct
>> *work)
>>       dev_dbg(&perf->ntb->pdev->dev, "%s called\n", __func__);
>>
>>       size = perf->mw.phys_size;
>> +
>> +     if (max_mw_size && size > max_mw_size)
>> +             size = max_mw_size;
>> +
>>       ntb_peer_spad_write(ndev, MW_SZ_HIGH, upper_32_bits(size));
>>       ntb_peer_spad_write(ndev, MW_SZ_LOW, lower_32_bits(size));
>>       ntb_peer_spad_write(ndev, VERSION, PERF_VERSION);
>> --
>> 2.1.4
>>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] ntb_tool: Add memory window debug support
  2016-06-03 21:20     ` Allen Hubbe
  (?)
@ 2016-06-04 15:25     ` Jon Mason
  -1 siblings, 0 replies; 14+ messages in thread
From: Jon Mason @ 2016-06-04 15:25 UTC (permalink / raw)
  To: Allen Hubbe
  Cc: Logan Gunthorpe, Dave Jiang, John Kading, Sudip Mukherjee,
	Arnd Bergmann, linux-ntb, linux-kernel

On Fri, Jun 3, 2016 at 5:20 PM, Allen Hubbe <Allen.Hubbe@emc.com> wrote:
> From: Logan Gunthorpe
>> We allocate some memory window buffers when the link comes up, then we
>> provide debugfs files to read/write each side of the link.
>>
>> This is useful for debugging the mapping when writing new drivers.
>>
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>
> Thanks! This was on my wish list.
>
> Acked-by: Allen Hubbe <Allen.Hubbe@emc.com>

Applied to my ntb-next branch

Thanks,
Jon

>> ---
>>  drivers/ntb/test/ntb_tool.c | 258 +++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 257 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/ntb/test/ntb_tool.c b/drivers/ntb/test/ntb_tool.c
>> index 209ef7c..4c01057 100644
>> --- a/drivers/ntb/test/ntb_tool.c
>> +++ b/drivers/ntb/test/ntb_tool.c
>> @@ -89,6 +89,7 @@
>>  #include <linux/dma-mapping.h>
>>  #include <linux/pci.h>
>>  #include <linux/slab.h>
>> +#include <linux/uaccess.h>
>>
>>  #include <linux/ntb.h>
>>
>> @@ -105,11 +106,29 @@ MODULE_VERSION(DRIVER_VERSION);
>>  MODULE_AUTHOR(DRIVER_AUTHOR);
>>  MODULE_DESCRIPTION(DRIVER_DESCRIPTION);
>>
>> +#define MAX_MWS 16
>> +
>> +static unsigned long mw_size = 16;
>> +module_param(mw_size, ulong, 0644);
>> +MODULE_PARM_DESC(mw_size, "size order [n^2] of the memory window for testing");
>> +
>>  static struct dentry *tool_dbgfs;
>>
>> +struct tool_mw {
>> +     resource_size_t size;
>> +     u8 __iomem *local;
>> +     u8 *peer;
>> +     dma_addr_t peer_dma;
>> +};
>> +
>>  struct tool_ctx {
>>       struct ntb_dev *ntb;
>>       struct dentry *dbgfs;
>> +     struct work_struct link_cleanup;
>> +     bool link_is_up;
>> +     struct delayed_work link_work;
>> +     int mw_count;
>> +     struct tool_mw mws[MAX_MWS];
>>  };
>>
>>  #define SPAD_FNAME_SIZE 0x10
>> @@ -124,6 +143,111 @@ struct tool_ctx {
>>               .write = __write,               \
>>       }
>>
>> +static int tool_setup_mw(struct tool_ctx *tc, int idx)
>> +{
>> +     int rc;
>> +     struct tool_mw *mw = &tc->mws[idx];
>> +     phys_addr_t base;
>> +     resource_size_t size, align, align_size;
>> +
>> +     if (mw->local)
>> +             return 0;
>> +
>> +     rc = ntb_mw_get_range(tc->ntb, idx, &base, &size, &align,
>> +                           &align_size);
>> +     if (rc)
>> +             return rc;
>> +
>> +     mw->size = min_t(resource_size_t, 1 << mw_size, size);
>> +     mw->size = round_up(mw->size, align);
>> +     mw->size = round_up(mw->size, align_size);
>> +
>> +     mw->local = ioremap_wc(base, size);
>> +     if (mw->local == NULL)
>> +             return -EFAULT;
>> +
>> +     mw->peer = dma_alloc_coherent(&tc->ntb->pdev->dev, mw->size,
>> +                                   &mw->peer_dma, GFP_KERNEL);
>> +
>> +     if (mw->peer == NULL)
>> +             return -ENOMEM;
>> +
>> +     rc = ntb_mw_set_trans(tc->ntb, idx, mw->peer_dma, mw->size);
>> +     if (rc)
>> +             return rc;
>> +
>> +     return 0;
>> +}
>> +
>> +static void tool_free_mws(struct tool_ctx *tc)
>> +{
>> +     int i;
>> +
>> +     for (i = 0; i < tc->mw_count; i++) {
>> +             if (tc->mws[i].peer) {
>> +                     ntb_mw_clear_trans(tc->ntb, i);
>> +                     dma_free_coherent(&tc->ntb->pdev->dev, tc->mws[i].size,
>> +                                       tc->mws[i].peer,
>> +                                       tc->mws[i].peer_dma);
>> +
>> +             }
>> +
>> +             tc->mws[i].peer = NULL;
>> +             tc->mws[i].peer_dma = 0;
>> +
>> +             if (tc->mws[i].local)
>> +                     iounmap(tc->mws[i].local);
>> +
>> +             tc->mws[i].local = NULL;
>> +     }
>> +
>> +     tc->mw_count = 0;
>> +}
>> +
>> +static int tool_setup_mws(struct tool_ctx *tc)
>> +{
>> +     int i;
>> +     int rc;
>> +
>> +     tc->mw_count = min(ntb_mw_count(tc->ntb), MAX_MWS);
>> +
>> +     for (i = 0; i < tc->mw_count; i++) {
>> +             rc = tool_setup_mw(tc, i);
>> +             if (rc)
>> +                     goto err_out;
>> +     }
>> +
>> +     return 0;
>> +
>> +err_out:
>> +     tool_free_mws(tc);
>> +     return rc;
>> +}
>> +
>> +static void tool_link_work(struct work_struct *work)
>> +{
>> +     int rc;
>> +     struct tool_ctx *tc = container_of(work, struct tool_ctx,
>> +                                        link_work.work);
>> +
>> +     tool_free_mws(tc);
>> +     rc = tool_setup_mws(tc);
>> +     if (rc)
>> +             dev_err(&tc->ntb->dev,
>> +                     "Error setting up memory windows: %d\n", rc);
>> +
>> +     tc->link_is_up = true;
>> +}
>> +
>> +static void tool_link_cleanup(struct work_struct *work)
>> +{
>> +     struct tool_ctx *tc = container_of(work, struct tool_ctx,
>> +                                        link_cleanup);
>> +
>> +     if (!tc->link_is_up)
>> +             cancel_delayed_work_sync(&tc->link_work);
>> +}
>> +
>>  static void tool_link_event(void *ctx)
>>  {
>>       struct tool_ctx *tc = ctx;
>> @@ -135,6 +259,11 @@ static void tool_link_event(void *ctx)
>>
>>       dev_dbg(&tc->ntb->dev, "link is %s speed %d width %d\n",
>>               up ? "up" : "down", speed, width);
>> +
>> +     if (up)
>> +             schedule_delayed_work(&tc->link_work, 2*HZ);
>> +     else
>> +             schedule_work(&tc->link_cleanup);
>>  }
>>
>>  static void tool_db_event(void *ctx, int vec)
>> @@ -443,8 +572,112 @@ static TOOL_FOPS_RDWR(tool_peer_spad_fops,
>>                     tool_peer_spad_read,
>>                     tool_peer_spad_write);
>>
>> +
>> +static ssize_t tool_mw_read(struct file *filep, char __user *ubuf,
>> +                         size_t size, loff_t *offp)
>> +{
>> +     struct tool_mw *mw = filep->private_data;
>> +     ssize_t rc;
>> +     loff_t pos = *offp;
>> +     void *buf;
>> +
>> +     if (mw->local == NULL)
>> +             return -EIO;
>> +     if (pos < 0)
>> +             return -EINVAL;
>> +     if (pos >= mw->size || !size)
>> +             return 0;
>> +     if (size > mw->size - pos)
>> +             size = mw->size - pos;
>> +
>> +     buf = kmalloc(size, GFP_KERNEL);
>> +     if (!buf)
>> +             return -ENOMEM;
>> +
>> +     memcpy_fromio(buf, mw->local + pos, size);
>> +     rc = copy_to_user(ubuf, buf, size);
>> +     if (rc == size) {
>> +             rc = -EFAULT;
>> +             goto err_free;
>> +     }
>> +
>> +     size -= rc;
>> +     *offp = pos + size;
>> +     rc = size;
>> +
>> +err_free:
>> +     kfree(buf);
>> +
>> +     return rc;
>> +}
>> +
>> +static ssize_t tool_mw_write(struct file *filep, const char __user *ubuf,
>> +                          size_t size, loff_t *offp)
>> +{
>> +     struct tool_mw *mw = filep->private_data;
>> +     ssize_t rc;
>> +     loff_t pos = *offp;
>> +     void *buf;
>> +
>> +     if (pos < 0)
>> +             return -EINVAL;
>> +     if (pos >= mw->size || !size)
>> +             return 0;
>> +     if (size > mw->size - pos)
>> +             size = mw->size - pos;
>> +
>> +     buf = kmalloc(size, GFP_KERNEL);
>> +     if (!buf)
>> +             return -ENOMEM;
>> +
>> +     rc = copy_from_user(buf, ubuf, size);
>> +     if (rc == size) {
>> +             rc = -EFAULT;
>> +             goto err_free;
>> +     }
>> +
>> +     size -= rc;
>> +     *offp = pos + size;
>> +     rc = size;
>> +
>> +     memcpy_toio(mw->local + pos, buf, size);
>> +
>> +err_free:
>> +     kfree(buf);
>> +
>> +     return rc;
>> +}
>> +
>> +static TOOL_FOPS_RDWR(tool_mw_fops,
>> +                   tool_mw_read,
>> +                   tool_mw_write);
>> +
>> +
>> +static ssize_t tool_peer_mw_read(struct file *filep, char __user *ubuf,
>> +                                size_t size, loff_t *offp)
>> +{
>> +     struct tool_mw *mw = filep->private_data;
>> +
>> +     return simple_read_from_buffer(ubuf, size, offp, mw->peer, mw->size);
>> +}
>> +
>> +static ssize_t tool_peer_mw_write(struct file *filep, const char __user *ubuf,
>> +                                 size_t size, loff_t *offp)
>> +{
>> +     struct tool_mw *mw = filep->private_data;
>> +
>> +     return simple_write_to_buffer(mw->peer, mw->size, offp, ubuf, size);
>> +}
>> +
>> +static TOOL_FOPS_RDWR(tool_peer_mw_fops,
>> +                   tool_peer_mw_read,
>> +                   tool_peer_mw_write);
>> +
>>  static void tool_setup_dbgfs(struct tool_ctx *tc)
>>  {
>> +     int mw_count;
>> +     int i;
>> +
>>       /* This modules is useless without dbgfs... */
>>       if (!tool_dbgfs) {
>>               tc->dbgfs = NULL;
>> @@ -473,6 +706,20 @@ static void tool_setup_dbgfs(struct tool_ctx *tc)
>>
>>       debugfs_create_file("peer_spad", S_IRUSR | S_IWUSR, tc->dbgfs,
>>                           tc, &tool_peer_spad_fops);
>> +
>> +     mw_count = min(ntb_mw_count(tc->ntb), MAX_MWS);
>> +     for (i = 0; i < mw_count; i++) {
>> +             char buf[30];
>> +
>> +             snprintf(buf, sizeof(buf), "mw%d", i);
>> +             debugfs_create_file(buf, S_IRUSR | S_IWUSR, tc->dbgfs,
>> +                                 &tc->mws[i], &tool_mw_fops);
>> +
>> +             snprintf(buf, sizeof(buf), "peer_mw%d", i);
>> +             debugfs_create_file(buf, S_IRUSR | S_IWUSR, tc->dbgfs,
>> +                                 &tc->mws[i], &tool_peer_mw_fops);
>> +
>> +     }
>>  }
>>
>>  static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
>> @@ -486,13 +733,15 @@ static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
>>       if (ntb_spad_is_unsafe(ntb))
>>               dev_dbg(&ntb->dev, "scratchpad is unsafe\n");
>>
>> -     tc = kmalloc(sizeof(*tc), GFP_KERNEL);
>> +     tc = kzalloc(sizeof(*tc), GFP_KERNEL);
>>       if (!tc) {
>>               rc = -ENOMEM;
>>               goto err_tc;
>>       }
>>
>>       tc->ntb = ntb;
>> +     INIT_DELAYED_WORK(&tc->link_work, tool_link_work);
>> +     INIT_WORK(&tc->link_cleanup, tool_link_cleanup);
>>
>>       tool_setup_dbgfs(tc);
>>
>> @@ -507,6 +756,8 @@ static int tool_probe(struct ntb_client *self, struct ntb_dev *ntb)
>>
>>  err_ctx:
>>       debugfs_remove_recursive(tc->dbgfs);
>> +     cancel_delayed_work_sync(&tc->link_work);
>> +     cancel_work_sync(&tc->link_cleanup);
>>       kfree(tc);
>>  err_tc:
>>       return rc;
>> @@ -516,6 +767,11 @@ static void tool_remove(struct ntb_client *self, struct ntb_dev *ntb)
>>  {
>>       struct tool_ctx *tc = ntb->ctx;
>>
>> +     cancel_delayed_work_sync(&tc->link_work);
>> +     cancel_work_sync(&tc->link_cleanup);
>> +
>> +     tool_free_mws(tc);
>> +
>>       ntb_clear_ctx(ntb);
>>       ntb_link_disable(ntb);
>>
>> --
>> 2.1.4
>
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] ntb_transport: Check the number of spads the hardware supports
  2016-06-03 20:50 ` [PATCH 2/3] ntb_transport: Check the number of spads the hardware supports Logan Gunthorpe
@ 2016-06-04 15:40   ` Jon Mason
  2016-06-07 17:15     ` Logan Gunthorpe
  0 siblings, 1 reply; 14+ messages in thread
From: Jon Mason @ 2016-06-04 15:40 UTC (permalink / raw)
  To: Logan Gunthorpe
  Cc: Dave Jiang, Allen Hubbe, John Kading, Sudip Mukherjee,
	Arnd Bergmann, linux-ntb, linux-kernel

On Fri, Jun 03, 2016 at 02:50:32PM -0600, Logan Gunthorpe wrote:
> I'm working on hardware that currently has a limited number of
> scratchpad registers and ntb_ndev fails with no clue as to why. I
> feel it is better to fail early and provide a reasonable error message
> then to fail later on..
> 
> The same is done to ntb_perf, but it doesn't currently require enough
> spads to actually fail.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
> ---
>  drivers/ntb/ntb_transport.c | 9 +++++++--
>  drivers/ntb/test/ntb_perf.c | 8 ++++++--
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 2ef9d913..34e7a7a 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1037,6 +1037,13 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
>  	int node;
>  	int rc, i;
>  
> +	mw_count = ntb_mw_count(ndev);
> +	if (ntb_spad_count(ndev) < (NUM_MWS + 1 + mw_count*2)) {

Nit, please add spaces around '*' (per checkpatch)

> +		dev_err(&ndev->dev, "Not enough scratch pad registers for %s",
> +			NTB_TRANSPORT_NAME);
> +		return -EIO;
> +	}
> +
>  	if (ntb_db_is_unsafe(ndev))
>  		dev_dbg(&ndev->dev,
>  			"doorbell is unsafe, proceed anyway...\n");
> @@ -1052,8 +1059,6 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
>  
>  	nt->ndev = ndev;
>  
> -	mw_count = ntb_mw_count(ndev);
> -
>  	nt->mw_count = mw_count;
>  
>  	nt->mw_vec = kzalloc_node(mw_count * sizeof(*nt->mw_vec),
> diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
> index 30635c8..1815592 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -143,8 +143,6 @@ enum {
>  	VERSION = 0,
>  	MW_SZ_HIGH,
>  	MW_SZ_LOW,
> -	SPAD_MSG,
> -	SPAD_ACK,

Please explicitly point out that this is being modified in the commit
message.  I don't see them being used, so probably not a big deal
(unless Dave Jiang has something queued that will use it).

>  	MAX_SPAD
>  };
>  
> @@ -698,6 +696,12 @@ static int perf_probe(struct ntb_client *client, struct ntb_dev *ntb)
>  
>  	node = dev_to_node(&pdev->dev);
>  
> +	if (ntb_spad_count(ntb) < MAX_SPAD) {
> +		dev_err(&ntb->dev, "Not enough scratch pad registers for %s",
> +			DRIVER_NAME);
> +		return -EIO;
> +	}

Move this check above the dev_to_node assignment above.

Thanks,
Jon

> +
>  	perf = kzalloc_node(sizeof(*perf), GFP_KERNEL, node);
>  	if (!perf) {
>  		rc = -ENOMEM;
> -- 
> 2.1.4
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] ntb_transport: Check the number of spads the hardware supports
  2016-06-04 15:40   ` Jon Mason
@ 2016-06-07 17:15     ` Logan Gunthorpe
  2016-06-07 17:20       ` [PATCH v2 " Logan Gunthorpe
  0 siblings, 1 reply; 14+ messages in thread
From: Logan Gunthorpe @ 2016-06-07 17:15 UTC (permalink / raw)
  To: Jon Mason
  Cc: Dave Jiang, Allen Hubbe, John Kading, Sudip Mukherjee,
	Arnd Bergmann, linux-ntb, linux-kernel

Hi Jon,

Thanks for the feedback. I'll send an updated patch in a moment.

On 04/06/16 09:40 AM, Jon Mason wrote:
> Nit, please add spaces around '*' (per checkpatch)

I'll change this, but I did run it through checkpatch and it did not
warn about this.

> Please explicitly point out that this is being modified in the commit
> message.  I don't see them being used, so probably not a big deal
> (unless Dave Jiang has something queued that will use it).

Done. I feel like he can always add them back in when he adds the
functionality. This way, when he does, MAX_SPAD will be updated and the
check will still be correct.

> Move this check above the dev_to_node assignment above.

Done.


Logan

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 2/3] ntb_transport: Check the number of spads the hardware supports
  2016-06-07 17:15     ` Logan Gunthorpe
@ 2016-06-07 17:20       ` Logan Gunthorpe
  2016-06-07 17:24         ` Jiang, Dave
  0 siblings, 1 reply; 14+ messages in thread
From: Logan Gunthorpe @ 2016-06-07 17:20 UTC (permalink / raw)
  To: Jon Mason
  Cc: Dave Jiang, Allen Hubbe, John Kading, Sudip Mukherjee,
	Arnd Bergmann, linux-ntb, linux-kernel, Logan Gunthorpe

I'm working on hardware that currently has a limited number of
scratchpad registers and ntb_ndev fails with no clue as to why. I
feel it is better to fail early and provide a reasonable error message
then to fail later on.

The same is done to ntb_perf, but it doesn't currently require enough
spads to actually fail. I've also removed the unused SPAD_MSG and
SPAD_ACK enums so that MAX_SPAD accurately reflects the number of
spads used.

Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
---
 drivers/ntb/ntb_transport.c | 9 +++++++--
 drivers/ntb/test/ntb_perf.c | 8 ++++++--
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index 2ef9d913..6d9940a 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -1037,6 +1037,13 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
 	int node;
 	int rc, i;
 
+	mw_count = ntb_mw_count(ndev);
+	if (ntb_spad_count(ndev) < (NUM_MWS + 1 + mw_count * 2)) {
+		dev_err(&ndev->dev, "Not enough scratch pad registers for %s",
+			NTB_TRANSPORT_NAME);
+		return -EIO;
+	}
+
 	if (ntb_db_is_unsafe(ndev))
 		dev_dbg(&ndev->dev,
 			"doorbell is unsafe, proceed anyway...\n");
@@ -1052,8 +1059,6 @@ static int ntb_transport_probe(struct ntb_client *self, struct ntb_dev *ndev)
 
 	nt->ndev = ndev;
 
-	mw_count = ntb_mw_count(ndev);
-
 	nt->mw_count = mw_count;
 
 	nt->mw_vec = kzalloc_node(mw_count * sizeof(*nt->mw_vec),
diff --git a/drivers/ntb/test/ntb_perf.c b/drivers/ntb/test/ntb_perf.c
index 30635c8..4368519 100644
--- a/drivers/ntb/test/ntb_perf.c
+++ b/drivers/ntb/test/ntb_perf.c
@@ -143,8 +143,6 @@ enum {
 	VERSION = 0,
 	MW_SZ_HIGH,
 	MW_SZ_LOW,
-	SPAD_MSG,
-	SPAD_ACK,
 	MAX_SPAD
 };
 
@@ -696,6 +694,12 @@ static int perf_probe(struct ntb_client *client, struct ntb_dev *ntb)
 	int node;
 	int rc = 0;
 
+	if (ntb_spad_count(ntb) < MAX_SPAD) {
+		dev_err(&ntb->dev, "Not enough scratch pad registers for %s",
+			DRIVER_NAME);
+		return -EIO;
+	}
+
 	node = dev_to_node(&pdev->dev);
 
 	perf = kzalloc_node(sizeof(*perf), GFP_KERNEL, node);
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/3] ntb_transport: Check the number of spads the hardware supports
  2016-06-07 17:20       ` [PATCH v2 " Logan Gunthorpe
@ 2016-06-07 17:24         ` Jiang, Dave
  2016-06-09 14:51           ` Jon Mason
  0 siblings, 1 reply; 14+ messages in thread
From: Jiang, Dave @ 2016-06-07 17:24 UTC (permalink / raw)
  To: logang@deltatee.com, jdmason@kudzu.us
  Cc: Allen.Hubbe@emc.com, linux-kernel@vger.kernel.org,
	sudipm.mukherjee@gmail.com, arnd@arndb.de, john.kading@gd-ms.com,
	linux-ntb@googlegroups.com

On Tue, 2016-06-07 at 11:20 -0600, Logan Gunthorpe wrote:
> I'm working on hardware that currently has a limited number of
> scratchpad registers and ntb_ndev fails with no clue as to why. I
> feel it is better to fail early and provide a reasonable error
> message
> then to fail later on.
> 
> The same is done to ntb_perf, but it doesn't currently require enough
> spads to actually fail. I've also removed the unused SPAD_MSG and
> SPAD_ACK enums so that MAX_SPAD accurately reflects the number of
> spads used.
> 
> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>

Acked-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/ntb/ntb_transport.c | 9 +++++++--
>  drivers/ntb/test/ntb_perf.c | 8 ++++++--
>  2 files changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c
> b/drivers/ntb/ntb_transport.c
> index 2ef9d913..6d9940a 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1037,6 +1037,13 @@ static int ntb_transport_probe(struct
> ntb_client *self, struct ntb_dev *ndev)
>  	int node;
>  	int rc, i;
>  
> +	mw_count = ntb_mw_count(ndev);
> +	if (ntb_spad_count(ndev) < (NUM_MWS + 1 + mw_count * 2)) {
> +		dev_err(&ndev->dev, "Not enough scratch pad
> registers for %s",
> +			NTB_TRANSPORT_NAME);
> +		return -EIO;
> +	}
> +
>  	if (ntb_db_is_unsafe(ndev))
>  		dev_dbg(&ndev->dev,
>  			"doorbell is unsafe, proceed anyway...\n");
> @@ -1052,8 +1059,6 @@ static int ntb_transport_probe(struct
> ntb_client *self, struct ntb_dev *ndev)
>  
>  	nt->ndev = ndev;
>  
> -	mw_count = ntb_mw_count(ndev);
> -
>  	nt->mw_count = mw_count;
>  
>  	nt->mw_vec = kzalloc_node(mw_count * sizeof(*nt->mw_vec),
> diff --git a/drivers/ntb/test/ntb_perf.c
> b/drivers/ntb/test/ntb_perf.c
> index 30635c8..4368519 100644
> --- a/drivers/ntb/test/ntb_perf.c
> +++ b/drivers/ntb/test/ntb_perf.c
> @@ -143,8 +143,6 @@ enum {
>  	VERSION = 0,
>  	MW_SZ_HIGH,
>  	MW_SZ_LOW,
> -	SPAD_MSG,
> -	SPAD_ACK,
>  	MAX_SPAD
>  };
>  
> @@ -696,6 +694,12 @@ static int perf_probe(struct ntb_client *client,
> struct ntb_dev *ntb)
>  	int node;
>  	int rc = 0;
>  
> +	if (ntb_spad_count(ntb) < MAX_SPAD) {
> +		dev_err(&ntb->dev, "Not enough scratch pad registers
> for %s",
> +			DRIVER_NAME);
> +		return -EIO;
> +	}
> +
>  	node = dev_to_node(&pdev->dev);
>  
>  	perf = kzalloc_node(sizeof(*perf), GFP_KERNEL, node);
> -- 
> 2.1.4
> 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 2/3] ntb_transport: Check the number of spads the hardware supports
  2016-06-07 17:24         ` Jiang, Dave
@ 2016-06-09 14:51           ` Jon Mason
  0 siblings, 0 replies; 14+ messages in thread
From: Jon Mason @ 2016-06-09 14:51 UTC (permalink / raw)
  To: Jiang, Dave
  Cc: logang@deltatee.com, Allen.Hubbe@emc.com,
	linux-kernel@vger.kernel.org, sudipm.mukherjee@gmail.com,
	arnd@arndb.de, john.kading@gd-ms.com, linux-ntb@googlegroups.com

On Tue, Jun 7, 2016 at 1:24 PM, Jiang, Dave <dave.jiang@intel.com> wrote:
> On Tue, 2016-06-07 at 11:20 -0600, Logan Gunthorpe wrote:
>> I'm working on hardware that currently has a limited number of
>> scratchpad registers and ntb_ndev fails with no clue as to why. I
>> feel it is better to fail early and provide a reasonable error
>> message
>> then to fail later on.
>>
>> The same is done to ntb_perf, but it doesn't currently require enough
>> spads to actually fail. I've also removed the unused SPAD_MSG and
>> SPAD_ACK enums so that MAX_SPAD accurately reflects the number of
>> spads used.
>>
>> Signed-off-by: Logan Gunthorpe <logang@deltatee.com>
>
> Acked-by: Dave Jiang <dave.jiang@intel.com>

Added to the ntb-next branch.

thanks,
Jon

>> ---
>>  drivers/ntb/ntb_transport.c | 9 +++++++--
>>  drivers/ntb/test/ntb_perf.c | 8 ++++++--
>>  2 files changed, 13 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/ntb/ntb_transport.c
>> b/drivers/ntb/ntb_transport.c
>> index 2ef9d913..6d9940a 100644
>> --- a/drivers/ntb/ntb_transport.c
>> +++ b/drivers/ntb/ntb_transport.c
>> @@ -1037,6 +1037,13 @@ static int ntb_transport_probe(struct
>> ntb_client *self, struct ntb_dev *ndev)
>>       int node;
>>       int rc, i;
>>
>> +     mw_count = ntb_mw_count(ndev);
>> +     if (ntb_spad_count(ndev) < (NUM_MWS + 1 + mw_count * 2)) {
>> +             dev_err(&ndev->dev, "Not enough scratch pad
>> registers for %s",
>> +                     NTB_TRANSPORT_NAME);
>> +             return -EIO;
>> +     }
>> +
>>       if (ntb_db_is_unsafe(ndev))
>>               dev_dbg(&ndev->dev,
>>                       "doorbell is unsafe, proceed anyway...\n");
>> @@ -1052,8 +1059,6 @@ static int ntb_transport_probe(struct
>> ntb_client *self, struct ntb_dev *ndev)
>>
>>       nt->ndev = ndev;
>>
>> -     mw_count = ntb_mw_count(ndev);
>> -
>>       nt->mw_count = mw_count;
>>
>>       nt->mw_vec = kzalloc_node(mw_count * sizeof(*nt->mw_vec),
>> diff --git a/drivers/ntb/test/ntb_perf.c
>> b/drivers/ntb/test/ntb_perf.c
>> index 30635c8..4368519 100644
>> --- a/drivers/ntb/test/ntb_perf.c
>> +++ b/drivers/ntb/test/ntb_perf.c
>> @@ -143,8 +143,6 @@ enum {
>>       VERSION = 0,
>>       MW_SZ_HIGH,
>>       MW_SZ_LOW,
>> -     SPAD_MSG,
>> -     SPAD_ACK,
>>       MAX_SPAD
>>  };
>>
>> @@ -696,6 +694,12 @@ static int perf_probe(struct ntb_client *client,
>> struct ntb_dev *ntb)
>>       int node;
>>       int rc = 0;
>>
>> +     if (ntb_spad_count(ntb) < MAX_SPAD) {
>> +             dev_err(&ntb->dev, "Not enough scratch pad registers
>> for %s",
>> +                     DRIVER_NAME);
>> +             return -EIO;
>> +     }
>> +
>>       node = dev_to_node(&pdev->dev);
>>
>>       perf = kzalloc_node(sizeof(*perf), GFP_KERNEL, node);
>> --
>> 2.1.4
>>
>
> --
> You received this message because you are subscribed to the Google Groups "linux-ntb" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to linux-ntb+unsubscribe@googlegroups.com.
> To post to this group, send email to linux-ntb@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/linux-ntb/1465320253.16234.176.camel%40intel.com.
> For more options, visit https://groups.google.com/d/optout.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-06-09 14:51 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-06-03 20:50 [PATCH 0/3] ntb: Fixes and enhancements to ntb tools Logan Gunthorpe
2016-06-03 20:50 ` [PATCH 1/3] ntb_perf: Allow limiting the size of the memory windows Logan Gunthorpe
2016-06-03 21:03   ` Jiang, Dave
2016-06-04 15:25     ` Jon Mason
2016-06-03 20:50 ` [PATCH 2/3] ntb_transport: Check the number of spads the hardware supports Logan Gunthorpe
2016-06-04 15:40   ` Jon Mason
2016-06-07 17:15     ` Logan Gunthorpe
2016-06-07 17:20       ` [PATCH v2 " Logan Gunthorpe
2016-06-07 17:24         ` Jiang, Dave
2016-06-09 14:51           ` Jon Mason
2016-06-03 20:50 ` [PATCH 3/3] ntb_tool: Add memory window debug support Logan Gunthorpe
2016-06-03 21:20   ` Allen Hubbe
2016-06-03 21:20     ` Allen Hubbe
2016-06-04 15:25     ` Jon Mason

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.