subject:"\[PATCH\] staging\: unisys\: added virtpci info entry"

[PATCH] staging: unisys: added virtpci info entry

2014-07-16 Thread Erik Arfvidson

This patch adds the virtpci debugfs directory and the info entry
inside of it.

Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
---
v6: Edit version description and add MAX_BUF documentation
v5: Adjusted comments and formatting, remove info_debugfs_entry unused dentry 
structure
v4: Fixed comments, upper bound buffer, removed #define for virtpci and info,
modified printvbus to work with scnprintf and added and extra element to
print_vbus_info struct
v3: Fixed formating and comments. Also added debufs_remove_recursive() and
simple simple_read_from_buffer()
v2: Fixed comments and applied Dan Carpenter suggestions

 drivers/staging/unisys/virtpci/virtpci.c | 110 ++-
 1 file changed, 108 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/unisys/virtpci/virtpci.c 
b/drivers/staging/unisys/virtpci/virtpci.c
index 7d840b0..a80617f 100644
--- a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -36,6 +36,7 @@
 #include linux/mod_devicetable.h
 #include linux/if_ether.h
 #include linux/version.h
+#include linux/debugfs.h
 #include version.h
 #include guestlinuxdebug.h
 #include timskmodutils.h
@@ -58,6 +59,11 @@ struct driver_private {
 
 #define BUS_ID(x) dev_name(x)
 
+/* MAX_BUF = 4 busses x ( 32 devices/bus + 1 busline) x 80 characters
+ * = 10,560 bytes ~ 2^14 = 16,384 bytes
+ */
+#define MAX_BUF 16384
+
 #include virtpci.h
 
 /* this is shorter than using __FILE__ (full path name) in
@@ -100,6 +106,12 @@ static int virtpci_device_resume(struct device *dev);
 static int virtpci_device_probe(struct device *dev);
 static int virtpci_device_remove(struct device *dev);
 
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset);
+
+static const struct file_operations debugfs_info_fops = {
+   .read = info_debugfs_read,
+};
 
 /*/
 /* Globals   */
@@ -140,6 +152,13 @@ static DEFINE_RWLOCK(VpcidevListLock);
 /* filled in with info about this driver, wrt it servicing client busses */
 static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo;
 
+/*/
+/* debugfs entries   */
+/*/
+/* dentry is used to create the debugfs entry directory
+ * for virtpci
+ */
+static struct dentry *virtpci_debugfs_dir;
 
 struct virtpci_busdev {
struct device virtpci_bus_device;
@@ -1377,6 +1396,90 @@ void virtpci_unregister_driver(struct virtpci_driver 
*drv)
 EXPORT_SYMBOL_GPL(virtpci_unregister_driver);
 
 /*/
+/* debugfs filesystem functions  */
+/*/
+struct print_vbus_info {
+   int *str_pos;
+   char *buf;
+   size_t *len;
+};
+
+static int print_vbus(struct device *vbus, void *data)
+{
+   struct print_vbus_info *p = (struct print_vbus_info *)data;
+
+   *p-str_pos += scnprintf(p-buf + *p-str_pos, *p-len - *p-str_pos,
+   bus_id:%s\n, dev_name(vbus));
+   return 0;
+}
+
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset)
+{
+   ssize_t bytes_read = 0;
+   int str_pos = 0;
+   struct virtpci_dev *tmpvpcidev;
+   unsigned long flags;
+   struct print_vbus_info printparam;
+   char *vbuf;
+
+   if (len  MAX_BUF)
+   len = MAX_BUF;
+   vbuf = kzalloc(len, GFP_KERNEL);
+   if (!vbuf)
+   return -ENOMEM;
+
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+Virtual PCI Bus devices\n);
+   printparam.str_pos = str_pos;
+   printparam.buf = vbuf;
+   printparam.len = len;
+   if (bus_for_each_dev(virtpci_bus_type, NULL,
+(void *) printparam, print_vbus))
+   LOGERR(Failed to find bus\n);
+
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+   \n Virtual PCI devices\n);
+   read_lock_irqsave(VpcidevListLock, flags);
+   tmpvpcidev = VpcidevListHead;
+   while (tmpvpcidev) {
+   if (tmpvpcidev-devtype == VIRTHBA_TYPE) {
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+   [%d:%d] VHba:%08x:%08x 
max-config:%d-%d-%d-%d,
+   tmpvpcidev-busNo, tmpvpcidev-deviceNo,
+   tmpvpcidev-scsi.wwnn.wwnn1,
+   tmpvpcidev-scsi.wwnn.wwnn2,
+   tmpvpcidev-scsi.max.max_channel,
+

[PATCH] staging: unisys: added virtpci info entry

2014-07-15 Thread Erik Arfvidson

This patch adds the virtpci debugfs directory and the info entry
inside of it.

Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
---
v4: Fixed comments, upper bound buffer, removed #define for virtpci and info,
modified printvbus to work with scnprintf and added and extra element to
print_vbus_info struct
v3: Fixed formating and comments. Also added debufs_remove_recursive() and
simple simple_read_from_buffer()
v2: Fixed comments and applied Dan Carpenter suggestions
 drivers/staging/unisys/virtpci/virtpci.c | 111 ++-
 1 file changed, 108 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/unisys/virtpci/virtpci.c 
b/drivers/staging/unisys/virtpci/virtpci.c
index 7d840b0..1929137 100644
--- a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -36,6 +36,7 @@
 #include linux/mod_devicetable.h
 #include linux/if_ether.h
 #include linux/version.h
+#include linux/debugfs.h
 #include version.h
 #include guestlinuxdebug.h
 #include timskmodutils.h
@@ -57,6 +58,7 @@ struct driver_private {
 #endif
 
 #define BUS_ID(x) dev_name(x)
+#define MAX_BUF 16384
 
 #include virtpci.h
 
@@ -100,6 +102,12 @@ static int virtpci_device_resume(struct device *dev);
 static int virtpci_device_probe(struct device *dev);
 static int virtpci_device_remove(struct device *dev);
 
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset);
+
+static const struct file_operations debugfs_info_fops = {
+   .read = info_debugfs_read,
+};
 
 /*/
 /* Globals   */
@@ -139,7 +147,17 @@ static DEFINE_RWLOCK(VpcidevListLock);
 
 /* filled in with info about this driver, wrt it servicing client busses */
 static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo;
-
+/*/
+/* debugfs entries   */
+/*/
+/* dentry is used to create the debugfs entry directory
+ * for virtpci
+ */
+static struct dentry *virtpci_debugfs_dir;
+static struct dentry *info_debugfs_entry;
+/* info_debugfs_entry is used to tell virtpci to display current info
+ * kept in the driver
+ */
 
 struct virtpci_busdev {
struct device virtpci_bus_device;
@@ -1375,6 +1393,89 @@ void virtpci_unregister_driver(struct virtpci_driver 
*drv)
DBGINF(Leaving\n);
 }
 EXPORT_SYMBOL_GPL(virtpci_unregister_driver);
+/*/
+/* debugfs filesystem functions  */
+/*/
+struct print_vbus_info {
+   int *str_pos;
+   char *buf;
+   size_t *len;
+};
+
+static int print_vbus(struct device *vbus, void *data)
+{
+   struct print_vbus_info *p = (struct print_vbus_info *)data;
+
+   *p-str_pos += scnprintf(p-buf + *p-str_pos, *p-len - *p-str_pos,
+   bus_id:%s\n, dev_name(vbus));
+   return 0;
+}
+
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset)
+{
+   ssize_t bytes_read = 0;
+   int str_pos = 0;
+   struct virtpci_dev *tmpvpcidev;
+   unsigned long flags;
+   struct print_vbus_info printparam;
+   char *vbuf;
+
+   if (len  MAX_BUF)
+   len = MAX_BUF;
+   vbuf = kzalloc(len, GFP_KERNEL);
+   if (!vbuf)
+   return -ENOMEM;
+
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+Virtual PCI Bus devices\n);
+   printparam.str_pos = str_pos;
+   printparam.buf = vbuf;
+   printparam.len = len;
+   if (bus_for_each_dev(virtpci_bus_type, NULL,
+(void *) printparam, print_vbus))
+   LOGERR(Failed to find bus\n);
+
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+   \n Virtual PCI devices\n);
+   read_lock_irqsave(VpcidevListLock, flags);
+   tmpvpcidev = VpcidevListHead;
+   while (tmpvpcidev) {
+   if (tmpvpcidev-devtype == VIRTHBA_TYPE) {
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+   [%d:%d] VHba:%08x:%08x 
max-config:%d-%d-%d-%d,
+   tmpvpcidev-busNo, tmpvpcidev-deviceNo,
+   tmpvpcidev-scsi.wwnn.wwnn1,
+   tmpvpcidev-scsi.wwnn.wwnn2,
+   tmpvpcidev-scsi.max.max_channel,
+   tmpvpcidev-scsi.max.max_id,
+   tmpvpcidev-scsi.max.max_lun,
+   tmpvpcidev-scsi.max.cmd_per_lun);
+   } else {
+

Re: [PATCH] staging: unisys: added virtpci info entry

2014-07-15 Thread Greg KH

On Tue, Jul 15, 2014 at 10:36:43AM -0400, Erik Arfvidson wrote:
 This patch adds the virtpci debugfs directory and the info entry
 inside of it.
 
 Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
 Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
 ---
 v4: Fixed comments, upper bound buffer, removed #define for virtpci and info,
 modified printvbus to work with scnprintf and added and extra element to
 print_vbus_info struct
 v3: Fixed formating and comments. Also added debufs_remove_recursive() and
 simple simple_read_from_buffer()
 v2: Fixed comments and applied Dan Carpenter suggestions
  drivers/staging/unisys/virtpci/virtpci.c | 111 
 ++-
  1 file changed, 108 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/staging/unisys/virtpci/virtpci.c 
 b/drivers/staging/unisys/virtpci/virtpci.c
 index 7d840b0..1929137 100644
 --- a/drivers/staging/unisys/virtpci/virtpci.c
 +++ b/drivers/staging/unisys/virtpci/virtpci.c
 @@ -36,6 +36,7 @@
  #include linux/mod_devicetable.h
  #include linux/if_ether.h
  #include linux/version.h
 +#include linux/debugfs.h
  #include version.h
  #include guestlinuxdebug.h
  #include timskmodutils.h
 @@ -57,6 +58,7 @@ struct driver_private {
  #endif
  
  #define BUS_ID(x) dev_name(x)
 +#define MAX_BUF 16384

Lovely magic number, care to explain why this is this size?

  
  #include virtpci.h
  
 @@ -100,6 +102,12 @@ static int virtpci_device_resume(struct device *dev);
  static int virtpci_device_probe(struct device *dev);
  static int virtpci_device_remove(struct device *dev);
  
 +static ssize_t info_debugfs_read(struct file *file, char __user *buf,
 +   size_t len, loff_t *offset);
 +
 +static const struct file_operations debugfs_info_fops = {
 + .read = info_debugfs_read,
 +};
  
  /*/
  /* Globals   */
 @@ -139,7 +147,17 @@ static DEFINE_RWLOCK(VpcidevListLock);
  
  /* filled in with info about this driver, wrt it servicing client busses */
  static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo;
 -
 +/*/
 +/* debugfs entries   */
 +/*/
 +/* dentry is used to create the debugfs entry directory
 + * for virtpci
 + */
 +static struct dentry *virtpci_debugfs_dir;
 +static struct dentry *info_debugfs_entry;
 +/* info_debugfs_entry is used to tell virtpci to display current info
 + * kept in the driver
 + */

Odd to comment both above and below the variables, pick one style and
stick to it.

Also, no empty line ater the Bus_DriverInfo variable and the comment?

And again, you don't need the info_debugfs_entry variable at all.  You
set it once and then never touch it again, seems like a waste to have
it, right?


  
  struct virtpci_busdev {
   struct device virtpci_bus_device;
 @@ -1375,6 +1393,89 @@ void virtpci_unregister_driver(struct virtpci_driver 
 *drv)
   DBGINF(Leaving\n);
  }
  EXPORT_SYMBOL_GPL(virtpci_unregister_driver);
 +/*/
 +/* debugfs filesystem functions  */
 +/*/

Again, no blank line before the comment?


 +struct print_vbus_info {
 + int *str_pos;
 + char *buf;
 + size_t *len;
 +};
 +
 +static int print_vbus(struct device *vbus, void *data)
 +{
 + struct print_vbus_info *p = (struct print_vbus_info *)data;
 +
 + *p-str_pos += scnprintf(p-buf + *p-str_pos, *p-len - *p-str_pos,
 + bus_id:%s\n, dev_name(vbus));
 + return 0;
 +}
 +
 +static ssize_t info_debugfs_read(struct file *file, char __user *buf,
 +   size_t len, loff_t *offset)
 +{
 + ssize_t bytes_read = 0;
 + int str_pos = 0;
 + struct virtpci_dev *tmpvpcidev;
 + unsigned long flags;
 + struct print_vbus_info printparam;
 + char *vbuf;
 +
 + if (len  MAX_BUF)
 + len = MAX_BUF;
 + vbuf = kzalloc(len, GFP_KERNEL);
 + if (!vbuf)
 + return -ENOMEM;
 +
 + str_pos += scnprintf(vbuf + str_pos, len - str_pos,
 +  Virtual PCI Bus devices\n);
 + printparam.str_pos = str_pos;
 + printparam.buf = vbuf;
 + printparam.len = len;
 + if (bus_for_each_dev(virtpci_bus_type, NULL,
 +  (void *) printparam, print_vbus))
 + LOGERR(Failed to find bus\n);
 +
 + str_pos += scnprintf(vbuf + str_pos, len - str_pos,
 + \n Virtual PCI devices\n);
 + read_lock_irqsave(VpcidevListLock, flags);
 + tmpvpcidev = VpcidevListHead;
 + while (tmpvpcidev) {
 + if (tmpvpcidev-devtype == VIRTHBA_TYPE) {
 + str_pos += scnprintf(vbuf + str_pos, len - str_pos,
 + [%d:%d] VHba:%08x:%08x

Re: [PATCH] staging: unisys: added virtpci info entry

2014-07-15 Thread Erik Arfvidson



On 07/15/2014 10:45 AM, Greg KH wrote:

[SNIP]
+#define MAX_BUF 16384
Lovely magic number, care to explain why this is this size?


Assuming we have the maximum possible configuration:
4 busses, 32 devices per bus, and we assume max of 80
characters per linegives us 10,560 bytes which rounds
up to 2^14.


[SNIP]
thanks, greg k-h 


Cheers,
Erik Arfvidson
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: unisys: added virtpci info entry

2014-07-15 Thread Erik Arfvidson

This patch adds the virtpci debugfs directory and the info entry
inside of it.

Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
---
v5: Adjusted comments and formatting, remove unused function
v4: Fixed comments, upper bound buffer, removed #define for virtpci and info,
modified printvbus to work with scnprintf and added and extra element to
print_vbus_info struct
v3: Fixed formating and comments. Also added debufs_remove_recursive() and
simple simple_read_from_buffer()
v2: Fixed comments and applied Dan Carpenter suggestions

 drivers/staging/unisys/virtpci/virtpci.c | 106 ++-
 1 file changed, 104 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/unisys/virtpci/virtpci.c 
b/drivers/staging/unisys/virtpci/virtpci.c
index 7d840b0..f498116 100644
--- a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -36,6 +36,7 @@
 #include linux/mod_devicetable.h
 #include linux/if_ether.h
 #include linux/version.h
+#include linux/debugfs.h
 #include version.h
 #include guestlinuxdebug.h
 #include timskmodutils.h
@@ -57,6 +58,7 @@ struct driver_private {
 #endif
 
 #define BUS_ID(x) dev_name(x)
+#define MAX_BUF 16384
 
 #include virtpci.h
 
@@ -100,6 +102,12 @@ static int virtpci_device_resume(struct device *dev);
 static int virtpci_device_probe(struct device *dev);
 static int virtpci_device_remove(struct device *dev);
 
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset);
+
+static const struct file_operations debugfs_info_fops = {
+   .read = info_debugfs_read,
+};
 
 /*/
 /* Globals   */
@@ -140,6 +148,13 @@ static DEFINE_RWLOCK(VpcidevListLock);
 /* filled in with info about this driver, wrt it servicing client busses */
 static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo;
 
+/*/
+/* debugfs entries   */
+/*/
+/* dentry is used to create the debugfs entry directory
+ * for virtpci
+ */
+static struct dentry *virtpci_debugfs_dir;
 
 struct virtpci_busdev {
struct device virtpci_bus_device;
@@ -1377,6 +1392,90 @@ void virtpci_unregister_driver(struct virtpci_driver 
*drv)
 EXPORT_SYMBOL_GPL(virtpci_unregister_driver);
 
 /*/
+/* debugfs filesystem functions  */
+/*/
+struct print_vbus_info {
+   int *str_pos;
+   char *buf;
+   size_t *len;
+};
+
+static int print_vbus(struct device *vbus, void *data)
+{
+   struct print_vbus_info *p = (struct print_vbus_info *)data;
+
+   *p-str_pos += scnprintf(p-buf + *p-str_pos, *p-len - *p-str_pos,
+   bus_id:%s\n, dev_name(vbus));
+   return 0;
+}
+
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset)
+{
+   ssize_t bytes_read = 0;
+   int str_pos = 0;
+   struct virtpci_dev *tmpvpcidev;
+   unsigned long flags;
+   struct print_vbus_info printparam;
+   char *vbuf;
+
+   if (len  MAX_BUF)
+   len = MAX_BUF;
+   vbuf = kzalloc(len, GFP_KERNEL);
+   if (!vbuf)
+   return -ENOMEM;
+
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+Virtual PCI Bus devices\n);
+   printparam.str_pos = str_pos;
+   printparam.buf = vbuf;
+   printparam.len = len;
+   if (bus_for_each_dev(virtpci_bus_type, NULL,
+(void *) printparam, print_vbus))
+   LOGERR(Failed to find bus\n);
+
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+   \n Virtual PCI devices\n);
+   read_lock_irqsave(VpcidevListLock, flags);
+   tmpvpcidev = VpcidevListHead;
+   while (tmpvpcidev) {
+   if (tmpvpcidev-devtype == VIRTHBA_TYPE) {
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+   [%d:%d] VHba:%08x:%08x 
max-config:%d-%d-%d-%d,
+   tmpvpcidev-busNo, tmpvpcidev-deviceNo,
+   tmpvpcidev-scsi.wwnn.wwnn1,
+   tmpvpcidev-scsi.wwnn.wwnn2,
+   tmpvpcidev-scsi.max.max_channel,
+   tmpvpcidev-scsi.max.max_id,
+   tmpvpcidev-scsi.max.max_lun,
+   tmpvpcidev-scsi.max.cmd_per_lun);
+   } else {
+   str_pos += scnprintf(vbuf + str_pos, len

Re: [PATCH] staging: unisys: added virtpci info entry

2014-07-15 Thread Greg KH

On Tue, Jul 15, 2014 at 11:31:03AM -0400, Erik Arfvidson wrote:
 
 On 07/15/2014 10:45 AM, Greg KH wrote:
 [SNIP]
 +#define MAX_BUF 16384
 Lovely magic number, care to explain why this is this size?
 
 Assuming we have the maximum possible configuration:
 4 busses, 32 devices per bus, and we assume max of 80
 characters per linegives us 10,560 bytes which rounds
 up to 2^14.

Then _DOCUMENT IT_.

So in 5 years, when you look at the code again, you will remember this
is how you came up with that number.

thanks,

greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: unisys: added virtpci info entry

2014-07-15 Thread Greg KH

On Tue, Jul 15, 2014 at 11:38:16AM -0400, Erik Arfvidson wrote:
 This patch adds the virtpci debugfs directory and the info entry
 inside of it.
 
 Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
 Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
 ---
 v5: Adjusted comments and formatting, remove unused function

What unused function?

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: unisys: added virtpci info entry

2014-07-14 Thread Erik Arfvidson



On 07/11/2014 08:09 PM, Greg KH wrote:
[snip]

+   if (!vbuf)
+   return -ENOMEM;
+
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+Virtual PCI Bus devices\n);

Why the leading ' '?


It's the formatting the original author chose to output the information. So 
that it would look something like this:

 Virtual PCI Bus devices
INFOFOOBLAHBLAHBLAH
INFOFOOBLAHBLAHBLAH
INFOFOOBLAHBLAHBLAH
 Virtual PCI devices
INFOFOOBLAHBLAHBLAH
INFOFOOBLAHBLAHBLAH
INFOFOOBLAHBLAHBLAH

Instead of this:

Virtual PCI Bus devices
INFOFOOBLAHBLAHBLAH
INFOFOOBLAHBLAHBLAH
INFOFOOBLAHBLAHBLAH
Virtual PCI devices
INFOFOOBLAHBLAHBLAH
INFOFOOBLAHBLAHBLAH
INFOFOOBLAHBLAHBLAH
 


[snip]
greg k-h

Thanks for the comments and suggestions. I should be sending the next 
patch shortly :)


Cheers,
Erik Arfvidson
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: unisys: added virtpci info entry

2014-07-12 Thread Dan Carpenter

On Fri, Jul 11, 2014 at 07:11:45PM -0400, Erik Arfvidson wrote:
 +static int print_vbus(struct device *vbus, void *data)
 +{
 + struct print_vbus_info *p = (struct print_vbus_info *)data;
 +
 + *p-length += sprintf(p-buf + *p-length, bus_id:%s\n,
 +   dev_name(vbus));

This sprintf() can corrupt memory if you pass too short a len.

 + return 0;
 +}

regards,
dan carpenter

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH] staging: unisys: added virtpci info entry

2014-07-11 Thread Erik Arfvidson

This patch adds the virtpci debugfs directory and the info entry
inside of it.

Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
---
v3: Fixed formating and comments. Also added debufs_remove_recursive() and
simple simple_read_from_buffer()
v2: fixed comments and applied Dan Carpenter suggestions
 drivers/staging/unisys/virtpci/virtpci.c | 108 ++-
 1 file changed, 105 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/unisys/virtpci/virtpci.c 
b/drivers/staging/unisys/virtpci/virtpci.c
index 7d840b0..6ea29f5 100644
--- a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -36,6 +36,7 @@
 #include linux/mod_devicetable.h
 #include linux/if_ether.h
 #include linux/version.h
+#include linux/debugfs.h
 #include version.h
 #include guestlinuxdebug.h
 #include timskmodutils.h
@@ -100,6 +101,12 @@ static int virtpci_device_resume(struct device *dev);
 static int virtpci_device_probe(struct device *dev);
 static int virtpci_device_remove(struct device *dev);
 
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset);
+
+static const struct file_operations debugfs_info_fops = {
+   .read = info_debugfs_read,
+};
 
 /*/
 /* Globals   */
@@ -139,7 +146,19 @@ static DEFINE_RWLOCK(VpcidevListLock);
 
 /* filled in with info about this driver, wrt it servicing client busses */
 static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo;
-
+/*/
+/* DebugFS entries   */
+/*/
+/* dentry is used to create the debugfs entry directory
+ * for virtpci
+ */
+static struct dentry *virtpci_debugfs_dir;
+static struct dentry *info_debugfs_entry;
+/* info_debugfs_entry is used to tell virtpci to display current info
+ * kept in the driver
+ */
+#define DIR_DEBUGFS_ENTRY virtpci
+#define INFO_DEBUGFS_ENTRY_FN info
 
 struct virtpci_busdev {
struct device virtpci_bus_device;
@@ -1375,6 +1394,85 @@ void virtpci_unregister_driver(struct virtpci_driver 
*drv)
DBGINF(Leaving\n);
 }
 EXPORT_SYMBOL_GPL(virtpci_unregister_driver);
+/*/
+/* debugfs filesystem functions  */
+/*/
+struct print_vbus_info {
+   int *length;
+   char *buf;
+};
+
+static int print_vbus(struct device *vbus, void *data)
+{
+   struct print_vbus_info *p = (struct print_vbus_info *)data;
+
+   *p-length += sprintf(p-buf + *p-length, bus_id:%s\n,
+ dev_name(vbus));
+   return 0;
+}
+
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset)
+{
+   ssize_t bytes_read = 0;
+   int str_pos = 0;
+   struct virtpci_dev *tmpvpcidev;
+   unsigned long flags;
+   struct print_vbus_info printparam;
+   char *vbuf;
+
+   vbuf = kzalloc(len, GFP_KERNEL);
+   if (!vbuf)
+   return -ENOMEM;
+
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+Virtual PCI Bus devices\n);
+   printparam.length = str_pos;
+   printparam.buf = vbuf;
+   if (bus_for_each_dev(virtpci_bus_type, NULL,
+(void *) printparam, print_vbus))
+   LOGERR(Failed to find bus\n);
+
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+   \n Virtual PCI devices\n);
+   read_lock_irqsave(VpcidevListLock, flags);
+   tmpvpcidev = VpcidevListHead;
+   while (tmpvpcidev) {
+   if (tmpvpcidev-devtype == VIRTHBA_TYPE) {
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+   [%d:%d] VHba:%08x:%08x 
max-config:%d-%d-%d-%d,
+   tmpvpcidev-busNo, tmpvpcidev-deviceNo,
+   tmpvpcidev-scsi.wwnn.wwnn1,
+   tmpvpcidev-scsi.wwnn.wwnn2,
+   tmpvpcidev-scsi.max.max_channel,
+   tmpvpcidev-scsi.max.max_id,
+   tmpvpcidev-scsi.max.max_lun,
+   tmpvpcidev-scsi.max.cmd_per_lun);
+   } else {
+   str_pos += scnprintf(vbuf + str_pos, len - str_pos,
+   [%d:%d] 
VNic:%02x:%02x:%02x:%02x:%02x:%02x num_rcv_bufs:%d mtu:%d,
+   tmpvpcidev-busNo, tmpvpcidev-deviceNo,
+   tmpvpcidev-net.mac_addr[0],
+

Re: [PATCH] staging: unisys: added virtpci info entry

2014-07-11 Thread Greg KH

On Fri, Jul 11, 2014 at 07:11:45PM -0400, Erik Arfvidson wrote:
 This patch adds the virtpci debugfs directory and the info entry
 inside of it.
 
 Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
 Signed-off-by: Benjamin Romer benjamin.ro...@unisys.com
 ---
 v3: Fixed formating and comments. Also added debufs_remove_recursive() and
 simple simple_read_from_buffer()
 v2: fixed comments and applied Dan Carpenter suggestions
  drivers/staging/unisys/virtpci/virtpci.c | 108 
 ++-
  1 file changed, 105 insertions(+), 3 deletions(-)
 
 diff --git a/drivers/staging/unisys/virtpci/virtpci.c 
 b/drivers/staging/unisys/virtpci/virtpci.c
 index 7d840b0..6ea29f5 100644
 --- a/drivers/staging/unisys/virtpci/virtpci.c
 +++ b/drivers/staging/unisys/virtpci/virtpci.c
 @@ -36,6 +36,7 @@
  #include linux/mod_devicetable.h
  #include linux/if_ether.h
  #include linux/version.h
 +#include linux/debugfs.h
  #include version.h
  #include guestlinuxdebug.h
  #include timskmodutils.h
 @@ -100,6 +101,12 @@ static int virtpci_device_resume(struct device *dev);
  static int virtpci_device_probe(struct device *dev);
  static int virtpci_device_remove(struct device *dev);
  
 +static ssize_t info_debugfs_read(struct file *file, char __user *buf,
 +   size_t len, loff_t *offset);
 +
 +static const struct file_operations debugfs_info_fops = {
 + .read = info_debugfs_read,
 +};
  
  /*/
  /* Globals   */
 @@ -139,7 +146,19 @@ static DEFINE_RWLOCK(VpcidevListLock);
  
  /* filled in with info about this driver, wrt it servicing client busses */
  static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo;
 -
 +/*/
 +/* DebugFS entries   */
 +/*/
 +/* dentry is used to create the debugfs entry directory
 + * for virtpci
 + */
 +static struct dentry *virtpci_debugfs_dir;
 +static struct dentry *info_debugfs_entry;
 +/* info_debugfs_entry is used to tell virtpci to display current info
 + * kept in the driver
 + */

You don't need this variable as all you do is set it, and then never do
anything with it.  Worse case, make it a local variable.  As you never
check it, just don't even do that...

 +#define DIR_DEBUGFS_ENTRY virtpci
 +#define INFO_DEBUGFS_ENTRY_FN info

You only reference these #defines once, no need for them at all.

  
  struct virtpci_busdev {
   struct device virtpci_bus_device;
 @@ -1375,6 +1394,85 @@ void virtpci_unregister_driver(struct virtpci_driver 
 *drv)
   DBGINF(Leaving\n);
  }
  EXPORT_SYMBOL_GPL(virtpci_unregister_driver);
 +/*/
 +/* debugfs filesystem functions  */
 +/*/
 +struct print_vbus_info {
 + int *length;
 + char *buf;
 +};
 +
 +static int print_vbus(struct device *vbus, void *data)
 +{
 + struct print_vbus_info *p = (struct print_vbus_info *)data;
 +
 + *p-length += sprintf(p-buf + *p-length, bus_id:%s\n,
 +   dev_name(vbus));
 + return 0;
 +}
 +
 +static ssize_t info_debugfs_read(struct file *file, char __user *buf,
 +   size_t len, loff_t *offset)
 +{
 + ssize_t bytes_read = 0;
 + int str_pos = 0;
 + struct virtpci_dev *tmpvpcidev;
 + unsigned long flags;
 + struct print_vbus_info printparam;
 + char *vbuf;
 +
 + vbuf = kzalloc(len, GFP_KERNEL);

Sorry I missed this last time, but what happens if userspace asks for
10Gb?  Nasty problems will happen if you try to allocate that.  Or
100Mb.  Set a reasonable upper bound here please.

Remember:
All input is evil.

 + if (!vbuf)
 + return -ENOMEM;
 +
 + str_pos += scnprintf(vbuf + str_pos, len - str_pos,
 +  Virtual PCI Bus devices\n);

Why the leading ' '?

 + printparam.length = str_pos;
 + printparam.buf = vbuf;
 + if (bus_for_each_dev(virtpci_bus_type, NULL,
 +  (void *) printparam, print_vbus))
 + LOGERR(Failed to find bus\n);
 +
 + str_pos += scnprintf(vbuf + str_pos, len - str_pos,
 + \n Virtual PCI devices\n);
 + read_lock_irqsave(VpcidevListLock, flags);
 + tmpvpcidev = VpcidevListHead;
 + while (tmpvpcidev) {
 + if (tmpvpcidev-devtype == VIRTHBA_TYPE) {
 + str_pos += scnprintf(vbuf + str_pos, len - str_pos,
 + [%d:%d] VHba:%08x:%08x 
 max-config:%d-%d-%d-%d,
 + tmpvpcidev-busNo, tmpvpcidev-deviceNo,
 + tmpvpcidev-scsi.wwnn.wwnn1,
 + tmpvpcidev-scsi.wwnn.wwnn2,
 + tmpvpcidev-scsi.max.max_channel,
 +

[PATCH] staging: unisys: added virtpci info entry

2014-07-10 Thread Erik Arfvidson

This patch adds the virtpci debugfs directory and the info entry
inside of it.

Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
---
 drivers/staging/unisys/virtpci/virtpci.c | 123 ++-
 1 file changed, 119 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/unisys/virtpci/virtpci.c 
b/drivers/staging/unisys/virtpci/virtpci.c
index 7d840b0..722bc1b 100644
--- a/drivers/staging/unisys/virtpci/virtpci.c
+++ b/drivers/staging/unisys/virtpci/virtpci.c
@@ -36,6 +36,7 @@
 #include linux/mod_devicetable.h
 #include linux/if_ether.h
 #include linux/version.h
+#include linux/debugfs.h
 #include version.h
 #include guestlinuxdebug.h
 #include timskmodutils.h
@@ -100,6 +101,12 @@ static int virtpci_device_resume(struct device *dev);
 static int virtpci_device_probe(struct device *dev);
 static int virtpci_device_remove(struct device *dev);
 
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset);
+
+static const struct file_operations debugfs_info_fops = {
+   .read = info_debugfs_read,
+};
 
 /*/
 /* Globals   */
@@ -139,7 +146,19 @@ static DEFINE_RWLOCK(VpcidevListLock);
 
 /* filled in with info about this driver, wrt it servicing client busses */
 static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo;
-
+/*/
+/* DebugFS entries   */
+/*/
+/* dentry is used to create the debugfs entry directory
+ * for virtpci
+ */
+static struct dentry *virtpci_debugfs_dir;
+static struct dentry *info_debugfs_entry;
+/* info_debugfs_entry is used to tell virtpci to display current info
+ * kept in the driver
+ */
+#define DIR_DEBUGFS_ENTRY virtpci
+#define INFO_DEBUGFS_ENTRY_FN info
 
 struct virtpci_busdev {
struct device virtpci_bus_device;
@@ -588,7 +607,8 @@ static void delete_all(void)
 /* deletes all vnics or vhbas
  * returns 0 failure, 1 success,
  */
-static int delete_all_virt(VIRTPCI_DEV_TYPE devtype, struct del_vbus_guestpart 
*delparams)
+static int delete_all_virt(VIRTPCI_DEV_TYPE devtype,
+   struct del_vbus_guestpart *delparams)
 {
int i;
unsigned char busid[BUS_ID_SIZE];
@@ -1375,6 +1395,97 @@ void virtpci_unregister_driver(struct virtpci_driver 
*drv)
DBGINF(Leaving\n);
 }
 EXPORT_SYMBOL_GPL(virtpci_unregister_driver);
+/*/
+/* debugfs filesystem functions
*/
+/*/
+struct print_vbus_info {
+   int *length;
+   char *buf;
+};
+
+static int print_vbus(struct device *vbus, void *data)
+{
+   struct print_vbus_info *p = (struct print_vbus_info *)data;
+
+   *p-length += sprintf(p-buf + *p-length, bus_id:%s\n,
+ dev_name(vbus));
+   return 0;   /* no error */
+}
+
+static ssize_t info_debugfs_read(struct file *file, char __user *buf,
+ size_t len, loff_t *offset)
+{
+   int str_pos = 0;
+   struct virtpci_dev *tmpvpcidev;
+   unsigned long flags;
+   struct print_vbus_info printparam;
+   char *vbuf;
+   loff_t pos = *offset;
+
+   if (pos  0)
+   return -EINVAL;
+
+   if (pos  0 || !len)
+   return 0;
+
+   vbuf = kzalloc(len, GFP_KERNEL);
+   if (!vbuf)
+   return -ENOMEM;
+
+
+   str_pos += snprintf(vbuf + str_pos, len - str_pos,
+   \n Virtual PCI Bus devices\n);
+   printparam.length = str_pos;
+   printparam.buf = vbuf;
+   if (bus_for_each_dev(virtpci_bus_type, NULL,
+(void *) printparam, print_vbus))
+   LOGERR(Failed to find bus\n);
+
+   str_pos += snprintf(vbuf + str_pos, len - str_pos,
+   \n Virtual PCI devices\n);
+   read_lock_irqsave(VpcidevListLock, flags);
+   tmpvpcidev = VpcidevListHead;
+   while (tmpvpcidev) {
+   if (tmpvpcidev-devtype == VIRTHBA_TYPE) {
+   str_pos += snprintf(vbuf + str_pos, len - str_pos,
+   [%d:%d] VHba:%08x:%08x 
max-config:%d-%d-%d-%d,
+   tmpvpcidev-busNo, tmpvpcidev-deviceNo,
+   tmpvpcidev-scsi.wwnn.wwnn1,
+   tmpvpcidev-scsi.wwnn.wwnn2,
+   tmpvpcidev-scsi.max.max_channel,
+   tmpvpcidev-scsi.max.max_id,
+   tmpvpcidev-scsi.max.max_lun,
+   tmpvpcidev-scsi.max.cmd_per_lun);
+   } else {
+   str_pos += snprintf(vbuf + str_pos, len - str_pos,
+

Re: [PATCH] staging: unisys: added virtpci info entry

2014-07-10 Thread Dan Carpenter

On Thu, Jul 10, 2014 at 02:45:12PM -0400, Erik Arfvidson wrote:
 + str_pos += snprintf(vbuf + str_pos, len - str_pos, \n);
 + if (copy_to_user(buf, vbuf, str_pos)) {

The length checking here still isn't correct.  snprintf() returns the
number of bytes which would have been printed if there were space
available, so str_pos could be larger than len.  So we have fixed
the problem of corruption kernel memory but it still corrupts user
memory.

I think it all works correctly if you use scnprintf() instead of
snprintf() but think it through because I'm not positive.

 + kfree(vbuf);
 + return -EFAULT;
 + }
 +

regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH] staging: unisys: added virtpci info entry

2014-07-10 Thread Greg KH

On Thu, Jul 10, 2014 at 02:45:12PM -0400, Erik Arfvidson wrote:
 This patch adds the virtpci debugfs directory and the info entry
 inside of it.
 
 Signed-off-by: Erik Arfvidson erik.arfvid...@unisys.com
 ---
  drivers/staging/unisys/virtpci/virtpci.c | 123 
 ++-
  1 file changed, 119 insertions(+), 4 deletions(-)
 
 diff --git a/drivers/staging/unisys/virtpci/virtpci.c 
 b/drivers/staging/unisys/virtpci/virtpci.c
 index 7d840b0..722bc1b 100644
 --- a/drivers/staging/unisys/virtpci/virtpci.c
 +++ b/drivers/staging/unisys/virtpci/virtpci.c
 @@ -36,6 +36,7 @@
  #include linux/mod_devicetable.h
  #include linux/if_ether.h
  #include linux/version.h
 +#include linux/debugfs.h
  #include version.h
  #include guestlinuxdebug.h
  #include timskmodutils.h
 @@ -100,6 +101,12 @@ static int virtpci_device_resume(struct device *dev);
  static int virtpci_device_probe(struct device *dev);
  static int virtpci_device_remove(struct device *dev);
  
 +static ssize_t info_debugfs_read(struct file *file, char __user *buf,
 +   size_t len, loff_t *offset);
 +
 +static const struct file_operations debugfs_info_fops = {
 + .read = info_debugfs_read,
 +};
  
  /*/
  /* Globals   */
 @@ -139,7 +146,19 @@ static DEFINE_RWLOCK(VpcidevListLock);
  
  /* filled in with info about this driver, wrt it servicing client busses */
  static ULTRA_VBUS_DEVICEINFO Bus_DriverInfo;
 -
 +/*/
 +/* DebugFS entries   */
 +/*/
 +/* dentry is used to create the debugfs entry directory
 + * for virtpci
 + */
 +static struct dentry *virtpci_debugfs_dir;
 +static struct dentry *info_debugfs_entry;
 +/* info_debugfs_entry is used to tell virtpci to display current info
 + * kept in the driver
 + */
 +#define DIR_DEBUGFS_ENTRY virtpci
 +#define INFO_DEBUGFS_ENTRY_FN info
  
  struct virtpci_busdev {
   struct device virtpci_bus_device;
 @@ -588,7 +607,8 @@ static void delete_all(void)
  /* deletes all vnics or vhbas
   * returns 0 failure, 1 success,
   */
 -static int delete_all_virt(VIRTPCI_DEV_TYPE devtype, struct 
 del_vbus_guestpart *delparams)
 +static int delete_all_virt(VIRTPCI_DEV_TYPE devtype,
 + struct del_vbus_guestpart *delparams)
  {
   int i;
   unsigned char busid[BUS_ID_SIZE];
 @@ -1375,6 +1395,97 @@ void virtpci_unregister_driver(struct virtpci_driver 
 *drv)
   DBGINF(Leaving\n);
  }
  EXPORT_SYMBOL_GPL(virtpci_unregister_driver);
 +/*/
 +/* debugfs filesystem functions  
 */
 +/*/
 +struct print_vbus_info {
 + int *length;
 + char *buf;
 +};
 +
 +static int print_vbus(struct device *vbus, void *data)
 +{
 + struct print_vbus_info *p = (struct print_vbus_info *)data;
 +
 + *p-length += sprintf(p-buf + *p-length, bus_id:%s\n,
 +   dev_name(vbus));
 + return 0;   /* no error */

Of course 0 is no error, that's by definition how Linux kernel
functions work.  No need to add that comment, especially to the right of
the line (which is not kernel coding style at all.)

 +}
 +
 +static ssize_t info_debugfs_read(struct file *file, char __user *buf,
 +   size_t len, loff_t *offset)
 +{
 + int str_pos = 0;
 + struct virtpci_dev *tmpvpcidev;
 + unsigned long flags;
 + struct print_vbus_info printparam;
 + char *vbuf;
 + loff_t pos = *offset;
 +
 + if (pos  0)
 + return -EINVAL;
 +
 + if (pos  0 || !len)
 + return 0;

So no moving about the file?  Why not?  Why not just use
simple_read_from_buffer() with the string you generate here?  That
handles all the nasty offset mess for you automatically.

 +
 + vbuf = kzalloc(len, GFP_KERNEL);
 + if (!vbuf)
 + return -ENOMEM;
 +
 +

2 blank lines?

Yes, I'm being picky, but this is new code.

 + str_pos += snprintf(vbuf + str_pos, len - str_pos,
 + \n Virtual PCI Bus devices\n);

You start the buffer with an empty line?

 + printparam.length = str_pos;
 + printparam.buf = vbuf;
 + if (bus_for_each_dev(virtpci_bus_type, NULL,
 +  (void *) printparam, print_vbus))
 + LOGERR(Failed to find bus\n);
 +
 + str_pos += snprintf(vbuf + str_pos, len - str_pos,
 + \n Virtual PCI devices\n);
 + read_lock_irqsave(VpcidevListLock, flags);
 + tmpvpcidev = VpcidevListHead;
 + while (tmpvpcidev) {
 + if (tmpvpcidev-devtype == VIRTHBA_TYPE) {
 + str_pos += snprintf(vbuf + str_pos, len - str_pos,
 + [%d:%d] VHba:%08x:%08x

[PATCH] staging: unisys: added virtpci info entry

[PATCH] staging: unisys: added virtpci info entry

Re: [PATCH] staging: unisys: added virtpci info entry

Re: [PATCH] staging: unisys: added virtpci info entry

[PATCH] staging: unisys: added virtpci info entry

Re: [PATCH] staging: unisys: added virtpci info entry

Re: [PATCH] staging: unisys: added virtpci info entry

Re: [PATCH] staging: unisys: added virtpci info entry

Re: [PATCH] staging: unisys: added virtpci info entry

[PATCH] staging: unisys: added virtpci info entry

Re: [PATCH] staging: unisys: added virtpci info entry

[PATCH] staging: unisys: added virtpci info entry

Re: [PATCH] staging: unisys: added virtpci info entry

Re: [PATCH] staging: unisys: added virtpci info entry

14 matches

Site Navigation

Mail list logo

Footer information