[dpdk-dev] [PATCH 0/3] eal: uio irq fixes and enhancements

[dpdk-dev] [PATCH 0/3] eal: uio irq fixes and enhancements

[Stephen Hemminger]

This set of patches starts out with fixing a regression where
uio_pci_generic broke link state interrupt, then adds better
management of PCI config space.

Will leave up to document writers to update various release
notes and API manuals as they see fit.

Also, needs what ever shared library map file updates which
maybe required when using dynamic libraries. But that should
not stop acceptance of this patch set.

Stephen Hemminger (3):
  uio: fix irq handling with igb_uio
  pci: add ability to read/write config space
  eal: use pci_uio_read/write config to enable/disable INTX

 lib/librte_eal/bsdapp/eal/eal_pci.c                | 76 ++++++++++++++++++++++
 lib/librte_eal/common/include/rte_pci.h            | 29 +++++++++
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 70 ++++++++++++++++----
 lib/librte_eal/linuxapp/eal/eal_pci.c              | 50 ++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci_init.h         | 11 ++++
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c          | 29 +++++++--
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c         | 16 +++++
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |  3 +-
 8 files changed, 264 insertions(+), 20 deletions(-)

-- 
2.1.4

[dpdk-dev] [PATCH 1/3] uio: fix irq handling with igb_uio

[Stephen Hemminger]

The introduction of uio_pci_generic broke interrupt handling with
igb_uio. The igb_uio device uses the kernel read/write method to
enable disable IRQ's; the uio_pci_generic has to use PCI intx
config read/write to enable disable interrupts.

Since igb_uio uses MSI-X the PCI intx config read/write won't
work.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 40 ++++++++++++++++++++--
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c          | 15 +++++---
 .../linuxapp/eal/include/exec-env/rte_interrupts.h |  3 +-
 3 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 66deda2..3a84b3c 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -361,7 +361,7 @@ vfio_disable_msix(struct rte_intr_handle *intr_handle) {
 #endif
 
 static int
-uio_intr_disable(struct rte_intr_handle *intr_handle)
+uio_intx_intr_disable(struct rte_intr_handle *intr_handle)
 {
 	unsigned char command_high;
 
@@ -385,7 +385,7 @@ uio_intr_disable(struct rte_intr_handle *intr_handle)
 }
 
 static int
-uio_intr_enable(struct rte_intr_handle *intr_handle)
+uio_intx_intr_enable(struct rte_intr_handle *intr_handle)
 {
 	unsigned char command_high;
 
@@ -408,6 +408,34 @@ uio_intr_enable(struct rte_intr_handle *intr_handle)
 	return 0;
 }
 
+static int
+uio_intr_disable(struct rte_intr_handle *intr_handle)
+{
+	const int value = 0;
+
+	if (write(intr_handle->fd, &value, sizeof(value)) < 0) {
+		RTE_LOG(ERR, EAL,
+			"Error disabling interrupts for fd %d (%s)\n",
+			intr_handle->fd, strerror(errno));
+		return -1;
+	}
+	return 0;
+}
+
+static int
+uio_intr_enable(struct rte_intr_handle *intr_handle)
+{
+	const int value = 1;
+
+	if (write(intr_handle->fd, &value, sizeof(value)) < 0) {
+		RTE_LOG(ERR, EAL,
+			"Error enabling interrupts for fd %d (%s)\n",
+			intr_handle->fd, strerror(errno));
+		return -1;
+	}
+	return 0;
+}
+
 int
 rte_intr_callback_register(struct rte_intr_handle *intr_handle,
 			rte_intr_callback_fn cb, void *cb_arg)
@@ -556,6 +584,10 @@ rte_intr_enable(struct rte_intr_handle *intr_handle)
 		if (uio_intr_enable(intr_handle))
 			return -1;
 		break;
+	case RTE_INTR_HANDLE_UIO_INTX:
+		if (uio_intx_intr_enable(intr_handle))
+			return -1;
+		break;
 	/* not used at this moment */
 	case RTE_INTR_HANDLE_ALARM:
 		return -1;
@@ -596,6 +628,10 @@ rte_intr_disable(struct rte_intr_handle *intr_handle)
 		if (uio_intr_disable(intr_handle))
 			return -1;
 		break;
+	case RTE_INTR_HANDLE_UIO_INTX:
+		if (uio_intx_intr_disable(intr_handle))
+			return -1;
+		break;
 	/* not used at this moment */
 	case RTE_INTR_HANDLE_ALARM:
 		return -1;
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index 2d1c69b..b5116a7 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -299,7 +299,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 			devname, strerror(errno));
 		return -1;
 	}
-	dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
 
 	snprintf(cfgname, sizeof(cfgname),
 			"/sys/class/uio/uio%u/device/config", uio_num);
@@ -310,10 +309,16 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		return -1;
 	}
 
-	/* set bus master that is not done by uio_pci_generic */
-	if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
-		RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
-		return -1;
+	if (dev->kdrv == RTE_KDRV_IGB_UIO)
+		dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
+	else {
+		dev->intr_handle.type = RTE_INTR_HANDLE_UIO_INTX;
+
+		/* set bus master that is not done by uio_pci_generic */
+		if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
+			RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
+			return -1;
+		}
 	}
 
 	/* allocate the mapping details for secondary processes*/
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
index 6a159c7..bdeb3fc 100644
--- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
@@ -40,7 +40,8 @@
 
 enum rte_intr_handle_type {
 	RTE_INTR_HANDLE_UNKNOWN = 0,
-	RTE_INTR_HANDLE_UIO,      /**< uio device handle */
+	RTE_INTR_HANDLE_UIO,          /**< uio device handle */
+	RTE_INTR_HANDLE_UIO_INTX,     /**< uio generic handle */
 	RTE_INTR_HANDLE_VFIO_LEGACY,  /**< vfio device handle (legacy) */
 	RTE_INTR_HANDLE_VFIO_MSI,     /**< vfio device handle (MSI) */
 	RTE_INTR_HANDLE_VFIO_MSIX,    /**< vfio device handle (MSIX) */
-- 
2.1.4

[dpdk-dev] [PATCH 2/3] pci: add ability to read/write config space

[Stephen Hemminger]

Add new ability to read/write PCI registers in device.
This is needed by BNX2X driver and is generally useful
for other code.

The BSD code has not been tested but is included to show
how the feature is cross-platform.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c        | 76 ++++++++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_pci.h    | 29 ++++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci.c      | 50 ++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci_init.h | 11 +++++
 lib/librte_eal/linuxapp/eal/eal_pci_uio.c  | 14 ++++++
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 16 +++++++
 6 files changed, 196 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 30f0232..b104e5f 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -490,6 +490,82 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 	return 1;
 }
 
+/* Read PCI config space. */
+int rte_eal_pci_read_config(const struct rte_pci_device *dev,
+			    void *buf, size_t len, off_t offset)
+{
+	struct pci_io io = {
+		.pi_sel = {
+			.pc_domain = dev->addr.domain,
+			.pc_bus = dev->addr.bus,
+			.pc_dev = dev->addr.devid,
+			.pc_func = dev->addr.function,
+		},
+		.pi_reg = offset,
+		.pi_width = len;
+	};
+
+	if (len == 0 || len > sizeof(io.pi_data)) {
+		RTE_LOG(ERR, EAL, "%s(): invalid register length\n", __func__);
+		return -1;
+	}
+
+	fd = open("/dev/pci", O_RDONLY);
+	if (fd < 0) {
+		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
+		return -1;
+	}
+
+	if (ioctl(fd, PCIOCREAD, &io) < 0) {
+		RTE_LOG(ERR, EAL, "%s(): error with ioctl on /dev/pci: %s\n",
+			__func__, strerror(errno));
+		close(fd);
+		return -1;
+	}
+
+	close(fd);
+	memcpy(buf, io.pi_data, len);
+	return 0;
+}
+
+/* Write PCI config space. */
+int rte_eal_pci_write_config(const struct rte_pci_device *device,
+			     const void *buf, size_t len, off_t offset)
+{
+	struct pci_io io = {
+		.pi_sel = {
+			.pc_domain = dev->addr.domain,
+			.pc_bus = dev->addr.bus,
+			.pc_dev = dev->addr.devid,
+			.pc_func = dev->addr.function,
+		},
+		.pi_reg = offset,
+		.pi_width = len;
+	};
+
+	if (len == 0 || len > sizeof(io.pi_data)) {
+		RTE_LOG(ERR, EAL, "%s(): invalid register length\n", __func__);
+		return -1;
+	}
+	memcpy(io.pi_data, buf, len);
+
+	fd = open("/dev/pci", O_RDONLY);
+	if (fd < 0) {
+		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
+		return -1;
+	}
+
+	if (ioctl(fd, PCIOCWRITE, &io) < 0) {
+		RTE_LOG(ERR, EAL, "%s(): error with ioctl on /dev/pci: %s\n",
+			__func__, strerror(errno));
+		close(fd);
+		return -1;
+	}
+
+	close(fd);
+	return 0;
+}
+
 /* Init the PCI EAL subsystem */
 int
 rte_eal_pci_init(void)
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 223d3cd..f4836f6 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -393,6 +393,35 @@ void rte_eal_pci_register(struct rte_pci_driver *driver);
  */
 void rte_eal_pci_unregister(struct rte_pci_driver *driver);
 
+
+/**
+ * Read PCI config space.
+ *
+ * @param device
+ *   A pointer to a rte_pci_device structure describing the device
+ *   to use
+ * @param buf
+ *   A data buffer where the bytes should be read into
+ * @param size
+ *   The length of the data buffer.
+ */
+int rte_eal_pci_read_config(const struct rte_pci_device *device,
+			    void *buf, size_t len, off_t offset);
+
+/**
+ * Write PCI config space.
+ *
+ * @param device
+ *   A pointer to a rte_pci_device structure describing the device
+ *   to use
+ * @param buf
+ *   A data buffer containing the bytes should be written
+ * @param size
+ *   The length of the data buffer.
+ */
+int rte_eal_pci_write_config(const struct rte_pci_device *device,
+			     const void *buf, size_t len, off_t offset);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index d2adc66..4da59fe 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -756,6 +756,56 @@ rte_eal_pci_close_one_driver(struct rte_pci_driver *dr __rte_unused,
 }
 #endif /* RTE_LIBRTE_EAL_HOTPLUG */
 
+/* Read PCI config space. */
+int rte_eal_pci_read_config(const struct rte_pci_device *device,
+			    void *buf, size_t len, off_t offset)
+{
+	const struct rte_intr_handle *intr_handle = &device->intr_handle;
+
+	switch (intr_handle->type) {
+	case RTE_INTR_HANDLE_UIO:
+	case RTE_INTR_HANDLE_UIO_INTX:
+		return pci_uio_read_config(intr_handle, buf, len, offset);
+
+#ifdef RTE_EAL_VFIO
+	case RTE_INTR_HANDLE_VFIO_MSIX:
+	case RTE_INTR_HANDLE_VFIO_MSI:
+	case RTE_INTR_HANDLE_VFIO_LEGACY:
+		return pci_vfio_read_config(intr_handle, buf, len, offset);
+#endif
+	default:
+		RTE_LOG(ERR, EAL,
+			"Unknown handle type of fd %d\n",
+					intr_handle->fd);
+		return -1;
+	}
+}
+
+/* Write PCI config space. */
+int rte_eal_pci_write_config(const struct rte_pci_device *device,
+			     const void *buf, size_t len, off_t offset)
+{
+	const struct rte_intr_handle *intr_handle = &device->intr_handle;
+
+	switch (intr_handle->type) {
+	case RTE_INTR_HANDLE_UIO:
+	case RTE_INTR_HANDLE_UIO_INTX:
+		return pci_uio_write_config(intr_handle, buf, len, offset);
+
+#ifdef RTE_EAL_VFIO
+	case RTE_INTR_HANDLE_VFIO_MSIX:
+	case RTE_INTR_HANDLE_VFIO_MSI:
+	case RTE_INTR_HANDLE_VFIO_LEGACY:
+		return pci_vfio_write_config(intr_handle, buf, len, offset);
+#endif
+	default:
+		RTE_LOG(ERR, EAL,
+			"Unknown handle type of fd %d\n",
+					intr_handle->fd);
+		return -1;
+	}
+}
+
 /* Init the PCI EAL subsystem */
 int
 rte_eal_pci_init(void)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
index aa7b755..c28e5b0 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
@@ -68,6 +68,11 @@ void *pci_find_max_end_va(void);
 void *pci_map_resource(void *requested_addr, int fd, off_t offset,
 	       size_t size, int additional_flags);
 
+int pci_uio_read_config(const struct rte_intr_handle *intr_handle,
+			void *buf, size_t len, off_t offs);
+int pci_uio_write_config(const struct rte_intr_handle *intr_handle,
+			 const void *buf, size_t len, off_t offs);
+
 /* map IGB_UIO resource prototype */
 int pci_uio_map_resource(struct rte_pci_device *dev);
 
@@ -86,6 +91,12 @@ int pci_vfio_enable(void);
 int pci_vfio_is_enabled(void);
 int pci_vfio_mp_sync_setup(void);
 
+/* access config space */
+int pci_vfio_read_config(const struct rte_intr_handle *intr_handle,
+			 void *buf, size_t len, off_t offs);
+int pci_vfio_write_config(const struct rte_intr_handle *intr_handle,
+			  const void *buf, size_t len, off_t offs);
+
 /* map VFIO resource prototype */
 int pci_vfio_map_resource(struct rte_pci_device *dev);
 int pci_vfio_get_group_fd(int iommu_group_fd);
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
index b5116a7..ee7483e 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
@@ -58,6 +58,20 @@ EAL_REGISTER_TAILQ(rte_uio_tailq)
 
 #define OFF_MAX              ((uint64_t)(off_t)-1)
 
+int
+pci_uio_read_config(const struct rte_intr_handle *intr_handle,
+		    void *buf, size_t len, off_t offset)
+{
+	return pread(intr_handle->uio_cfg_fd, buf, len, offset);
+}
+
+int
+pci_uio_write_config(const struct rte_intr_handle *intr_handle,
+		     const void *buf, size_t len, off_t offset)
+{
+	return pwrite(intr_handle->uio_cfg_fd, buf, len, offset);
+}
+
 static int
 pci_uio_set_bus_master(int dev_fd)
 {
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index aea1fb1..092a369 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -77,6 +77,22 @@ EAL_REGISTER_TAILQ(rte_vfio_tailq)
 /* per-process VFIO config */
 static struct vfio_config vfio_cfg;
 
+int
+pci_vfio_read_config(const struct rte_intr_handle *intr_handle,
+		    void *buf, size_t len, off_t offs)
+{
+	return pread64(intr_handle->vfio_dev_fd, buf, len,
+	       VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) + offs);
+}
+
+int
+pci_vfio_write_config(const struct rte_intr_handle *intr_handle,
+		    const void *buf, size_t len, off_t offs)
+{
+	return pwrite64(intr_handle->vfio_dev_fd, buf, len,
+	       VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) + offs);
+}
+
 /* get PCI BAR number where MSI-X interrupts are */
 static int
 pci_vfio_get_msix_bar(int fd, int *msix_bar, uint32_t *msix_table_offset,
-- 
2.1.4

[dpdk-dev] [PATCH 3/3] eal: use pci_uio_read/write config to enable/disable INTX

[Stephen Hemminger]

Change the code use to enable/disable interrupts with UIO_PCI_GENERIC
driver. Instead of having magic constants for the PCI register values
use the standard defines available on Linux. Also use the new
routines available to peek/poke PCI config space.

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
---
 lib/librte_eal/linuxapp/eal/eal_interrupts.c | 30 +++++++++++++++++-----------
 1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
index 3a84b3c..3a90944 100644
--- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
+++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
@@ -41,6 +41,7 @@
 #include <string.h>
 #include <errno.h>
 #include <inttypes.h>
+#include <linux/pci_regs.h>
 #include <sys/epoll.h>
 #include <sys/signalfd.h>
 #include <sys/ioctl.h>
@@ -66,6 +67,7 @@
 
 #include "eal_private.h"
 #include "eal_vfio.h"
+#include "eal_pci_init.h"
 
 #define EAL_INTR_EPOLL_WAIT_FOREVER (-1)
 
@@ -363,18 +365,20 @@ vfio_disable_msix(struct rte_intr_handle *intr_handle) {
 static int
 uio_intx_intr_disable(struct rte_intr_handle *intr_handle)
 {
-	unsigned char command_high;
+	uint16_t cmd;
 
-	/* use UIO config file descriptor for uio_pci_generic */
-	if (pread(intr_handle->uio_cfg_fd, &command_high, 1, 5) != 1) {
+	if (pci_uio_read_config(intr_handle, &cmd, sizeof(cmd),
+				PCI_COMMAND) < 0) {
 		RTE_LOG(ERR, EAL,
 			"Error reading interrupts status for fd %d\n",
 			intr_handle->uio_cfg_fd);
 		return -1;
 	}
-	/* disable interrupts */
-	command_high |= 0x4;
-	if (pwrite(intr_handle->uio_cfg_fd, &command_high, 1, 5) != 1) {
+
+	cmd |= PCI_COMMAND_INTX_DISABLE;
+
+	if (pci_uio_write_config(intr_handle, &cmd, sizeof(cmd),
+				 PCI_COMMAND) < 0) {
 		RTE_LOG(ERR, EAL,
 			"Error disabling interrupts for fd %d\n",
 			intr_handle->uio_cfg_fd);
@@ -387,18 +391,20 @@ uio_intx_intr_disable(struct rte_intr_handle *intr_handle)
 static int
 uio_intx_intr_enable(struct rte_intr_handle *intr_handle)
 {
-	unsigned char command_high;
+	uint16_t cmd;
 
-	/* use UIO config file descriptor for uio_pci_generic */
-	if (pread(intr_handle->uio_cfg_fd, &command_high, 1, 5) != 1) {
+	if (pci_uio_read_config(intr_handle, &cmd, sizeof(cmd),
+				PCI_COMMAND) < 0) {
 		RTE_LOG(ERR, EAL,
 			"Error reading interrupts status for fd %d\n",
 			intr_handle->uio_cfg_fd);
 		return -1;
 	}
-	/* enable interrupts */
-	command_high &= ~0x4;
-	if (pwrite(intr_handle->uio_cfg_fd, &command_high, 1, 5) != 1) {
+
+	cmd &= ~PCI_COMMAND_INTX_DISABLE;
+
+	if (pci_uio_write_config(intr_handle, &cmd, sizeof(cmd),
+				 PCI_COMMAND) < 0) {
 		RTE_LOG(ERR, EAL,
 			"Error enabling interrupts for fd %d\n",
 			intr_handle->uio_cfg_fd);
-- 
2.1.4

Re: [dpdk-dev] [PATCH 1/3] uio: fix irq handling with igb_uio

[Bruce Richardson]

On Tue, Apr 28, 2015 at 09:36:38AM -0700, Stephen Hemminger wrote:
> The introduction of uio_pci_generic broke interrupt handling with
> igb_uio. The igb_uio device uses the kernel read/write method to
> enable disable IRQ's; the uio_pci_generic has to use PCI intx
> config read/write to enable disable interrupts.
> 
> Since igb_uio uses MSI-X the PCI intx config read/write won't
> work.
> 
> Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/librte_eal/linuxapp/eal/eal_interrupts.c       | 40 ++++++++++++++++++++--
>  lib/librte_eal/linuxapp/eal/eal_pci_uio.c          | 15 +++++---
>  .../linuxapp/eal/include/exec-env/rte_interrupts.h |  3 +-
>  3 files changed, 50 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal_interrupts.c b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> index 66deda2..3a84b3c 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_interrupts.c
> @@ -361,7 +361,7 @@ vfio_disable_msix(struct rte_intr_handle *intr_handle) {
>  #endif
>  
>  static int
> -uio_intr_disable(struct rte_intr_handle *intr_handle)
> +uio_intx_intr_disable(struct rte_intr_handle *intr_handle)
>  {
>  	unsigned char command_high;
>  
> @@ -385,7 +385,7 @@ uio_intr_disable(struct rte_intr_handle *intr_handle)
>  }
>  
>  static int
> -uio_intr_enable(struct rte_intr_handle *intr_handle)
> +uio_intx_intr_enable(struct rte_intr_handle *intr_handle)
>  {
>  	unsigned char command_high;
>  
> @@ -408,6 +408,34 @@ uio_intr_enable(struct rte_intr_handle *intr_handle)
>  	return 0;
>  }
>  
> +static int
> +uio_intr_disable(struct rte_intr_handle *intr_handle)
> +{
> +	const int value = 0;
> +
> +	if (write(intr_handle->fd, &value, sizeof(value)) < 0) {
> +		RTE_LOG(ERR, EAL,
> +			"Error disabling interrupts for fd %d (%s)\n",
> +			intr_handle->fd, strerror(errno));
> +		return -1;
> +	}
> +	return 0;
> +}
> +
> +static int
> +uio_intr_enable(struct rte_intr_handle *intr_handle)
> +{
> +	const int value = 1;
> +
> +	if (write(intr_handle->fd, &value, sizeof(value)) < 0) {
> +		RTE_LOG(ERR, EAL,
> +			"Error enabling interrupts for fd %d (%s)\n",
> +			intr_handle->fd, strerror(errno));
> +		return -1;
> +	}
> +	return 0;
> +}
> +
>  int
>  rte_intr_callback_register(struct rte_intr_handle *intr_handle,
>  			rte_intr_callback_fn cb, void *cb_arg)
> @@ -556,6 +584,10 @@ rte_intr_enable(struct rte_intr_handle *intr_handle)
>  		if (uio_intr_enable(intr_handle))
>  			return -1;
>  		break;
> +	case RTE_INTR_HANDLE_UIO_INTX:
> +		if (uio_intx_intr_enable(intr_handle))
> +			return -1;
> +		break;
>  	/* not used at this moment */
>  	case RTE_INTR_HANDLE_ALARM:
>  		return -1;
> @@ -596,6 +628,10 @@ rte_intr_disable(struct rte_intr_handle *intr_handle)
>  		if (uio_intr_disable(intr_handle))
>  			return -1;
>  		break;
> +	case RTE_INTR_HANDLE_UIO_INTX:
> +		if (uio_intx_intr_disable(intr_handle))
> +			return -1;
> +		break;
>  	/* not used at this moment */
>  	case RTE_INTR_HANDLE_ALARM:
>  		return -1;
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> index 2d1c69b..b5116a7 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> @@ -299,7 +299,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>  			devname, strerror(errno));
>  		return -1;
>  	}
> -	dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
>  
>  	snprintf(cfgname, sizeof(cfgname),
>  			"/sys/class/uio/uio%u/device/config", uio_num);
> @@ -310,10 +309,16 @@ pci_uio_map_resource(struct rte_pci_device *dev)
>  		return -1;
>  	}
>  
> -	/* set bus master that is not done by uio_pci_generic */
> -	if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
> -		RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
> -		return -1;
> +	if (dev->kdrv == RTE_KDRV_IGB_UIO)
> +		dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
> +	else {
> +		dev->intr_handle.type = RTE_INTR_HANDLE_UIO_INTX;
> +
> +		/* set bus master that is not done by uio_pci_generic */
> +		if (pci_uio_set_bus_master(dev->intr_handle.uio_cfg_fd)) {
> +			RTE_LOG(ERR, EAL, "Cannot set up bus mastering!\n");
> +			return -1;
> +		}
>  	}
>  
>  	/* allocate the mapping details for secondary processes*/
> diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> index 6a159c7..bdeb3fc 100644
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> @@ -40,7 +40,8 @@
>  
>  enum rte_intr_handle_type {
>  	RTE_INTR_HANDLE_UNKNOWN = 0,
> -	RTE_INTR_HANDLE_UIO,      /**< uio device handle */
> +	RTE_INTR_HANDLE_UIO,          /**< uio device handle */
> +	RTE_INTR_HANDLE_UIO_INTX,     /**< uio generic handle */
>  	RTE_INTR_HANDLE_VFIO_LEGACY,  /**< vfio device handle (legacy) */
>  	RTE_INTR_HANDLE_VFIO_MSI,     /**< vfio device handle (MSI) */
>  	RTE_INTR_HANDLE_VFIO_MSIX,    /**< vfio device handle (MSIX) */
> -- 
> 2.1.4
> 

Re: [dpdk-dev] [PATCH 0/3] eal: uio irq fixes and enhancements

[Thomas Monjalon]

2015-04-28 09:36, Stephen Hemminger:
> This set of patches starts out with fixing a regression where
> uio_pci_generic broke link state interrupt, then adds better
> management of PCI config space.
> 
> Will leave up to document writers to update various release
> notes and API manuals as they see fit.
> 
> Also, needs what ever shared library map file updates which
> maybe required when using dynamic libraries. But that should
> not stop acceptance of this patch set.

No, an incomplete patch cannot be accepted.
There are several solutions:
- Siobhan and Neil accept to work on doc and .map file
- You provide a good v2
- Someone else finish this patchset
- The bug remains (not a solution)

Re: [dpdk-dev] [PATCH 0/3] eal: uio irq fixes and enhancements

[Bruce Richardson]

On Tue, May 12, 2015 at 10:02:20PM +0200, Thomas Monjalon wrote:
> 2015-04-28 09:36, Stephen Hemminger:
> > This set of patches starts out with fixing a regression where
> > uio_pci_generic broke link state interrupt, then adds better
> > management of PCI config space.
> > 
> > Will leave up to document writers to update various release
> > notes and API manuals as they see fit.
> > 
> > Also, needs what ever shared library map file updates which
> > maybe required when using dynamic libraries. But that should
> > not stop acceptance of this patch set.
> 
> No, an incomplete patch cannot be accepted.
> There are several solutions:
> - Siobhan and Neil accept to work on doc and .map file
> - You provide a good v2
> - Someone else finish this patchset
> - The bug remains (not a solution)

Merge patch one on it's own to fix the issue? I don't think patch 1 requires
any further doc or ABI map file changes, does it?

/Bruce

Re: [dpdk-dev] [PATCH 1/3] uio: fix irq handling with igb_uio

[Thomas Monjalon]

2015-04-30 17:27, Bruce Richardson:
> On Tue, Apr 28, 2015 at 09:36:38AM -0700, Stephen Hemminger wrote:
> > The introduction of uio_pci_generic broke interrupt handling with
> > igb_uio. The igb_uio device uses the kernel read/write method to
> > enable disable IRQ's; the uio_pci_generic has to use PCI intx
> > config read/write to enable disable interrupts.
> > 
> > Since igb_uio uses MSI-X the PCI intx config read/write won't
> > work.
> > 
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> 
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>

Fixes: c112df6875a5 ("eal/linux: toggle interrupt for uio_pci_generic")

Applied this fix without the rest of the series (incomplete enhancements).
Thanks for the good work on pci interrupts.

Re: [dpdk-dev] [PATCH 0/3] eal: uio irq fixes and enhancements

[Thomas Monjalon]

2015-05-13 09:57, Bruce Richardson:
> On Tue, May 12, 2015 at 10:02:20PM +0200, Thomas Monjalon wrote:
> > 2015-04-28 09:36, Stephen Hemminger:
> > > This set of patches starts out with fixing a regression where
> > > uio_pci_generic broke link state interrupt, then adds better
> > > management of PCI config space.
> > > 
> > > Will leave up to document writers to update various release
> > > notes and API manuals as they see fit.
> > > 
> > > Also, needs what ever shared library map file updates which
> > > maybe required when using dynamic libraries. But that should
> > > not stop acceptance of this patch set.
> > 
> > No, an incomplete patch cannot be accepted.
> > There are several solutions:
> > - Siobhan and Neil accept to work on doc and .map file
> > - You provide a good v2
> > - Someone else finish this patchset
> > - The bug remains (not a solution)
> 
> Merge patch one on it's own to fix the issue? I don't think patch 1 requires
> any further doc or ABI map file changes, does it?

Yes you're right.
First patch is now applied.