diff mbox

[NET_NEXT,RFC,3/3] ixgbe: added reg_ops file to debugfs

Message ID 20120509230950.31910.6032.stgit@localhost6.localdomain6
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Catherine Sullivan May 9, 2012, 11:09 p.m. UTC
Added the reg_ops file to debugfs with commands to read and write
a register to give users the ability to read and write individual
registers on the fly.

Signed-off-by: Catherine Sullivan <catherine.sullivan@intel.com>
---

 drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c |  112 ++++++++++++++++++++++
 1 files changed, 112 insertions(+), 0 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

stephen hemminger May 9, 2012, 11:14 p.m. UTC | #1
On Wed, 09 May 2012 16:09:50 -0700
Catherine Sullivan <catherine.sullivan@intel.com> wrote:

> Added the reg_ops file to debugfs with commands to read and write
> a register to give users the ability to read and write individual
> registers on the fly.
> 
> Signed-off-by: Catherine Sullivan <catherine.sullivan@intel.com>
> ---
> 
>  drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c |  112 ++++++++++++++++++++++
>  1 files changed, 112 insertions(+), 0 deletions(-)
> 

Aren't these registers already in ethtool? You are also
allowing write without any security checking.

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ben Hutchings May 10, 2012, 12:18 a.m. UTC | #2
On Wed, 2012-05-09 at 16:14 -0700, Stephen Hemminger wrote:
> On Wed, 09 May 2012 16:09:50 -0700
> Catherine Sullivan <catherine.sullivan@intel.com> wrote:
> 
> > Added the reg_ops file to debugfs with commands to read and write
> > a register to give users the ability to read and write individual
> > registers on the fly.
> > 
> > Signed-off-by: Catherine Sullivan <catherine.sullivan@intel.com>
> > ---
> > 
> >  drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c |  112 ++++++++++++++++++++++
> >  1 files changed, 112 insertions(+), 0 deletions(-)
> > 
> 
> Aren't these registers already in ethtool?

ethtool register access is read-only and would be very heavy-weight for
interactive debugging.  Not sure this is the best interface to
read/write registers, but it's certainly not redundant.

> You are also allowing write without any security checking.

The file permissions are set to 0600 so it's root-only.

Ben.
stephen hemminger May 10, 2012, 3:54 a.m. UTC | #3
On Thu, 10 May 2012 01:18:10 +0100
Ben Hutchings <bhutchings@solarflare.com> wrote:

> On Wed, 2012-05-09 at 16:14 -0700, Stephen Hemminger wrote:
> > On Wed, 09 May 2012 16:09:50 -0700
> > Catherine Sullivan <catherine.sullivan@intel.com> wrote:
> > 
> > > Added the reg_ops file to debugfs with commands to read and write
> > > a register to give users the ability to read and write individual
> > > registers on the fly.
> > > 
> > > Signed-off-by: Catherine Sullivan <catherine.sullivan@intel.com>
> > > ---
> > > 
> > >  drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c |  112 ++++++++++++++++++++++
> > >  1 files changed, 112 insertions(+), 0 deletions(-)
> > > 
> > 
> > Aren't these registers already in ethtool?
> 
> ethtool register access is read-only and would be very heavy-weight for
> interactive debugging.  Not sure this is the best interface to
> read/write registers, but it's certainly not redundant.

For debugging couldn't you could mmap the PCI  space? That would allow writing
a user level debugger for the hardware.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
index dfe68a9..2558014 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_debugfs.c
@@ -34,6 +34,115 @@ 
 
 static struct dentry *ixgbe_dbg_root;
 
+static char ixgbe_dbg_reg_ops_buf[256] = "";
+
+/**
+ * ixgbe_dbg_reg_ops_open - prep the debugfs pokee data item when opened
+ * @inode: inode that was opened
+ * @filp:  file info
+ *
+ * Stash the adapter pointer hiding in the inode into the file pointer where
+ * we can find it later in the read and write calls
+ **/
+static int ixgbe_dbg_reg_ops_open(struct inode *inode, struct file *filp)
+{
+	filp->private_data = inode->i_private;
+	return 0;
+}
+
+/**
+ * ixgbe_dbg_reg_ops_read - read for reg_ops datum
+ * @filp: the opened file
+ * @buffer: where to write the data for the user to read
+ * @count: the size of the user's buffer
+ * @ppos: file position offset
+ **/
+static ssize_t ixgbe_dbg_reg_ops_read(struct file *filp, char __user *buffer,
+				    size_t count, loff_t *ppos)
+{
+	struct ixgbe_adapter *adapter = filp->private_data;
+	char buf[256];
+	int bytes_not_copied;
+	int len;
+
+	/* don't allow partial reads */
+	if (*ppos != 0)
+		return 0;
+
+	len = snprintf(buf, sizeof(buf), "%s: %s\n",
+		       adapter->netdev->name, ixgbe_dbg_reg_ops_buf);
+	if (count < len)
+		return -ENOSPC;
+	bytes_not_copied = copy_to_user(buffer, buf, len);
+	if (bytes_not_copied < 0)
+		return bytes_not_copied;
+
+	*ppos = len;
+	return len;
+}
+
+/**
+ * ixgbe_dbg_reg_ops_write - write into reg_ops datum
+ * @filp: the opened file
+ * @buffer: where to find the user's data
+ * @count: the length of the user's data
+ * @ppos: file position offset
+ **/
+static ssize_t ixgbe_dbg_reg_ops_write(struct file *filp,
+				     const char __user *buffer,
+				     size_t count, loff_t *ppos)
+{
+	struct ixgbe_adapter *adapter = filp->private_data;
+	int bytes_not_copied;
+
+	/* don't allow partial writes */
+	if (*ppos != 0)
+		return 0;
+	if (count >= sizeof(ixgbe_dbg_reg_ops_buf))
+		return -ENOSPC;
+
+	bytes_not_copied = copy_from_user(ixgbe_dbg_reg_ops_buf, buffer, count);
+	if (bytes_not_copied < 0)
+		return bytes_not_copied;
+	ixgbe_dbg_reg_ops_buf[count] = '\0';
+
+	if (strncmp(ixgbe_dbg_reg_ops_buf, "write", 5) == 0) {
+		u32 reg, value;
+		int cnt;
+		cnt = sscanf(&ixgbe_dbg_reg_ops_buf[5], "%x %x", &reg, &value);
+		if (cnt == 2) {
+			IXGBE_WRITE_REG(&adapter->hw, reg, value);
+			value = IXGBE_READ_REG(&adapter->hw, reg);
+			e_dev_info("write: 0x%08x = 0x%08x\n", reg, value);
+		} else {
+			e_dev_info("write reg value\n");
+		}
+	} else if (strncmp(ixgbe_dbg_reg_ops_buf, "read", 4) == 0) {
+		u32 reg, value;
+		int cnt;
+		cnt = sscanf(&ixgbe_dbg_reg_ops_buf[4], "%x", &reg);
+		if (cnt == 1) {
+			value = IXGBE_READ_REG(&adapter->hw, reg);
+			e_dev_info("read 0x%08x = 0x%08x\n", reg, value);
+		} else {
+			e_dev_info("read reg\n");
+		}
+	} else {
+		e_dev_info("Unknown command %s\n", ixgbe_dbg_reg_ops_buf);
+		e_dev_info("Available commands:\n");
+		e_dev_info("   read reg\n");
+		e_dev_info("   write reg value\n");
+	}
+	return count;
+}
+
+static const struct file_operations ixgbe_dbg_reg_ops_fops = {
+	.owner = THIS_MODULE,
+	.open =  ixgbe_dbg_reg_ops_open,
+	.read =  ixgbe_dbg_reg_ops_read,
+	.write = ixgbe_dbg_reg_ops_write,
+};
+
 static char ixgbe_dbg_netdev_ops_buf[256] = "";
 
 /**
@@ -141,6 +250,9 @@  void ixgbe_dbg_adapter_init(struct ixgbe_adapter *adapter)
 	struct dentry *pfile;
 	adapter->ixgbe_dbg_adapter = debugfs_create_dir(name, ixgbe_dbg_root);
 	if (adapter->ixgbe_dbg_adapter) {
+		pfile = debugfs_create_file("reg_ops", 0600,
+					    adapter->ixgbe_dbg_adapter, adapter,
+					    &ixgbe_dbg_reg_ops_fops);
 		pfile = debugfs_create_file("netdev_ops", 0600,
 					    adapter->ixgbe_dbg_adapter, adapter,
 					    &ixgbe_dbg_netdev_ops_fops);