DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested
@ 2014-06-05 14:39 Alan Carew
  2014-06-05 14:39 ` [dpdk-dev] [PATCH 1/4] [PATCH 1/4] igb_uio: Add interrupt_mode sysfs entry for igb_uio devices Alan Carew
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Alan Carew @ 2014-06-05 14:39 UTC (permalink / raw)
  To: dev

This series addresses an issue with librte_pmd_virtio where the offset to the
virtio device specific header may be incorrect depending on whether MSI-X has
been enabled or not.

If MSI-X is configured the device specific header is placed at byte offset 24
relative to the IO base address. 
If MSI-X is not configured the device specific header is placed at
byte offset 20. 

The following macro defined in virtio_pci.h is used to test the
presence of the MSI-X header and determine the correct offset: 
#define VIRTIO_PCI_CONFIG(hw) (((hw)->guest_features & VIRTIO_PCI_FLAG_MSIX) ? 24 : 20) 

However, VIRTIO_PCI_FLAG_MSIX is not a guest_feature nor is it part of the
Virtio Specification and resolves to the VIRTIO_NET_F_MAC feature as both
are #defined as 0x20. 

VIRTIO_PCI_FLAG_MSIX or similar flag should instead be set by the kernel
driver allocating resources and passed to user space for testing. 
i.e. 
#define VIRTIO_PCI_CONFIG(hw) (((hw)->intr_mode & IGBUIO_MSIX_INTR_MODE) ? 24 : 20)

To enable this testing of interrupt mode, this series allows for the kernel
driver(igb_uio) to place the configured interrupt mode into a sysfs entry.
sysfs is then parsed by eal_pci to determine the configured mode, which
allows all user space devices to correctly determine the interrupt mode,
including virtio_ethdev.

This series should be applied prior to Anatoly Burakov's
[VFIO] Add VFIO support to DPDK series

Alan Carew (4):
  igb_uio: Add interrupt_mode sysfs entry for igb_uio devices
  eal_pci: Add interrupt mode to rte_pci_device and parsing to eal_pci
  FreeBSD: Adds the equivalent interrupt mode setting and parsing
  virtio: Fixes the VIRTIO_PCI_CONFIG macro to use the correct offset
    to the Virtio header

 lib/librte_eal/bsdapp/eal/eal_pci.c                |   44 ++++++++++
 lib/librte_eal/bsdapp/nic_uio/nic_uio.c            |   14 +++
 lib/librte_eal/common/Makefile                     |    1 +
 lib/librte_eal/common/include/rte_pci.h            |    2 +
 .../common/include/rte_pci_dev_feature_defs.h      |   85 ++++++++++++++++++++
 .../common/include/rte_pci_dev_features.h          |   70 ++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci.c              |   78 ++++++++++++++++++
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c          |   48 +++++++++---
 lib/librte_pmd_virtio/virtio_ethdev.c              |    1 +
 lib/librte_pmd_virtio/virtio_pci.h                 |    4 +-
 10 files changed, 336 insertions(+), 11 deletions(-)
 create mode 100755 lib/librte_eal/common/include/rte_pci_dev_feature_defs.h
 create mode 100755 lib/librte_eal/common/include/rte_pci_dev_features.h

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

* [dpdk-dev] [PATCH 1/4] [PATCH 1/4] igb_uio: Add interrupt_mode sysfs entry for igb_uio devices
  2014-06-05 14:39 [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested Alan Carew
@ 2014-06-05 14:39 ` Alan Carew
  2014-06-05 17:20   ` Stephen Hemminger
  2014-06-05 14:39 ` [dpdk-dev] [PATCH 2/4] [PATCH 2/4] eal_pci: Add interrupt mode to rte_pci_device and parsing to eal_pci Alan Carew
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 9+ messages in thread
From: Alan Carew @ 2014-06-05 14:39 UTC (permalink / raw)
  To: dev

This patch adds an "interrupt_mode" sysfs entry for igb_uio devices,
allowing userspace eal_pci to track which interrupt mode has been enabled in kernel space.
The sysfs entry can be inspected via /sys/bus/pci/devices/<D:B:D:F/interrupt_mode

Signed-off-by: Alan Carew <alan.carew@intel.com>
---
 .../common/include/rte_pci_dev_feature_defs.h      |   85 ++++++++++++++++++++
 .../common/include/rte_pci_dev_features.h          |   70 ++++++++++++++++
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c          |   48 +++++++++---
 3 files changed, 193 insertions(+), 10 deletions(-)
 create mode 100755 lib/librte_eal/common/include/rte_pci_dev_feature_defs.h
 create mode 100755 lib/librte_eal/common/include/rte_pci_dev_features.h

diff --git a/lib/librte_eal/common/include/rte_pci_dev_feature_defs.h b/lib/librte_eal/common/include/rte_pci_dev_feature_defs.h
new file mode 100755
index 0000000..d23ed7d
--- /dev/null
+++ b/lib/librte_eal/common/include/rte_pci_dev_feature_defs.h
@@ -0,0 +1,85 @@
+/*-
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ *   redistributing this file, you may do so under either license.
+ * 
+ *   GPL LICENSE SUMMARY
+ * 
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ * 
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of version 2 of the GNU General Public License as
+ *   published by the Free Software Foundation.
+ * 
+ *   This program is distributed in the hope that it will be useful, but
+ *   WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *   General Public License for more details.
+ * 
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program; if not, write to the Free Software
+ *   Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ *   The full GNU General Public License is included in this distribution
+ *   in the file called LICENSE.GPL.
+ * 
+ *   Contact Information:
+ *   Intel Corporation
+ * 
+ *   BSD LICENSE
+ * 
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ * 
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ * 
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ * 
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ * 
+ */
+
+#ifndef _RTE_PCI_DEV_DEFS_H_
+#define _RTE_PCI_DEV_DEFS_H_
+
+#define RTE_PCI_DEV_FEATURE_INTR_MODE  "interrupt_mode"
+
+#define INTR_NAME_LEN 10
+
+#define IGBUIO_NONE_INTR_NAME "none"
+#define IGBUIO_LEGACY_INTR_NAME "legacy"
+#define IGBUIO_MSI_INTR_NAME "msi"
+#define IGBUIO_MSIX_INTR_NAME "msix"
+
+
+#define INTR_MODE(id, mode_name) \
+        .mode = (id), .name = (mode_name)
+
+/* interrupt mode */
+enum igbuio_intr_mode {
+	IGBUIO_NONE_INTR_MODE = 0,
+	IGBUIO_LEGACY_INTR_MODE,
+	IGBUIO_MSI_INTR_MODE,
+	IGBUIO_MSIX_INTR_MODE,
+	IGBUIO_INTR_MODE_MAX
+};
+
+#endif /* _RTE_PCI_DEV_DEFS_H_ */
diff --git a/lib/librte_eal/common/include/rte_pci_dev_features.h b/lib/librte_eal/common/include/rte_pci_dev_features.h
new file mode 100755
index 0000000..a45c056
--- /dev/null
+++ b/lib/librte_eal/common/include/rte_pci_dev_features.h
@@ -0,0 +1,70 @@
+/*-
+ * This file is provided under a dual BSD/GPLv2 license.  When using or
+ *   redistributing this file, you may do so under either license.
+ * 
+ *   GPL LICENSE SUMMARY
+ * 
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ * 
+ *   This program is free software; you can redistribute it and/or modify
+ *   it under the terms of version 2 of the GNU General Public License as
+ *   published by the Free Software Foundation.
+ * 
+ *   This program is distributed in the hope that it will be useful, but
+ *   WITHOUT ANY WARRANTY; without even the implied warranty of
+ *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *   General Public License for more details.
+ * 
+ *   You should have received a copy of the GNU General Public License
+ *   along with this program; if not, write to the Free Software
+ *   Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ *   The full GNU General Public License is included in this distribution
+ *   in the file called LICENSE.GPL.
+ * 
+ *   Contact Information:
+ *   Intel Corporation
+ * 
+ *   BSD LICENSE
+ * 
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ * 
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ * 
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ * 
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ * 
+ */
+
+#ifndef RTE_PCI_DEV_INTR_MODE
+#define RTE_PCI_DEV_INTR_MODE(id, mode_name)
+#endif
+
+RTE_PCI_DEV_INTR_MODE(IGBUIO_NONE_INTR_MODE, IGBUIO_NONE_INTR_NAME)
+RTE_PCI_DEV_INTR_MODE(IGBUIO_LEGACY_INTR_MODE, IGBUIO_LEGACY_INTR_NAME)
+RTE_PCI_DEV_INTR_MODE(IGBUIO_MSI_INTR_MODE, IGBUIO_MSI_INTR_NAME)
+RTE_PCI_DEV_INTR_MODE(IGBUIO_MSIX_INTR_MODE, IGBUIO_MSIX_INTR_NAME)
+
+#undef RTE_PCI_DEV_INTR_MODE
+
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index 09c40bf..dd1124b 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -34,6 +34,8 @@
 #include <xen/xen.h>
 #endif
 
+#include <rte_pci_dev_feature_defs.h>
+
 /**
  * MSI-X related macros, copy from linux/pci_regs.h in kernel 2.6.39,
  * but none of them in kernel 2.6.35.
@@ -49,12 +51,15 @@
 
 #define IGBUIO_NUM_MSI_VECTORS 1
 
-/* interrupt mode */
-enum igbuio_intr_mode {
-	IGBUIO_LEGACY_INTR_MODE = 0,
-	IGBUIO_MSI_INTR_MODE,
-	IGBUIO_MSIX_INTR_MODE,
-	IGBUIO_INTR_MODE_MAX
+struct rte_pci_dev_intr_mode {
+	enum igbuio_intr_mode mode;
+	const char *name;
+};
+
+/* Table of interrupt modes */
+static struct rte_pci_dev_intr_mode interrupt_modes[] = {
+#define RTE_PCI_DEV_INTR_MODE(id, mode_name) {INTR_MODE(id, mode_name)},
+#include <rte_pci_dev_features.h>
 };
 
 /**
@@ -150,8 +155,32 @@ store_max_vfs(struct device *dev, struct device_attribute *attr,
 }
 
 static DEVICE_ATTR(max_vfs, S_IRUGO | S_IWUSR, show_max_vfs, store_max_vfs);
+
+static ssize_t
+show_interrupt_mode(struct device *dev, struct device_attribute *attr,
+	     char *buf)
+{
+	unsigned i, num_modes;
+	enum igbuio_intr_mode mode;
+	struct uio_info *info;
+	struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
+
+	info = pci_get_drvdata(pdev);
+	mode = ((struct rte_uio_pci_dev *)info->priv)->mode;
+	num_modes = sizeof(interrupt_modes)/sizeof(interrupt_modes[0]);
+	
+	for (i = 0; i < num_modes; i++) {
+		if (mode == interrupt_modes[i].mode)
+			return snprintf(buf, INTR_NAME_LEN, "%s\n", interrupt_modes[i].name );
+	}
+	return snprintf(buf, INTR_NAME_LEN, "unknown\n");
+}
+
+static DEVICE_ATTR(interrupt_mode, S_IRUGO, show_interrupt_mode, NULL);
+
 static struct attribute *dev_attrs[] = {
 	&dev_attr_max_vfs.attr,
+	&dev_attr_interrupt_mode.attr,
         NULL,
 };
 
@@ -520,7 +549,7 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 #endif
 	udev->info.priv = udev;
 	udev->pdev = dev;
-	udev->mode = 0; /* set the default value for interrupt mode */
+	udev->mode = IGBUIO_LEGACY_INTR_MODE; /* set the default value for interrupt mode */
 	spin_lock_init(&udev->lock);
 
 	/* check if it need to try msix first */
@@ -610,11 +639,10 @@ igbuio_config_intr_mode(char *intr_str)
 		printk(KERN_INFO "Use MSIX interrupt by default\n");
 		return 0;
 	}
-
-	if (!strcmp(intr_str, "msix")) {
+	if (!strcmp(intr_str, IGBUIO_MSIX_INTR_NAME)) {
 		igbuio_intr_mode_preferred = IGBUIO_MSIX_INTR_MODE;
 		printk(KERN_INFO "Use MSIX interrupt\n");
-	} else if (!strcmp(intr_str, "legacy")) {
+	} else if (!strcmp(intr_str, IGBUIO_LEGACY_INTR_NAME)) {
 		igbuio_intr_mode_preferred = IGBUIO_LEGACY_INTR_MODE;
 		printk(KERN_INFO "Use legacy interrupt\n");
 	} else {
-- 
1.7.0.7

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

* [dpdk-dev] [PATCH 2/4] [PATCH 2/4] eal_pci: Add interrupt mode to rte_pci_device and parsing to eal_pci
  2014-06-05 14:39 [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested Alan Carew
  2014-06-05 14:39 ` [dpdk-dev] [PATCH 1/4] [PATCH 1/4] igb_uio: Add interrupt_mode sysfs entry for igb_uio devices Alan Carew
@ 2014-06-05 14:39 ` Alan Carew
  2014-06-05 14:39 ` [dpdk-dev] [PATCH 3/4] [PATCH 3/4] FreeBSD: Adds the equivalent interrupt mode setting and parsing Alan Carew
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Alan Carew @ 2014-06-05 14:39 UTC (permalink / raw)
  To: dev

This patch adds a shared enum between user and kernel space to rte_pci_device.
The value of intr_mode is parsed by eal_pci during pci_uio_map_resource

Signed-off-by: Alan Carew <alan.carew@intel.com>
---
 lib/librte_eal/common/Makefile          |    1 +
 lib/librte_eal/common/include/rte_pci.h |    2 +
 lib/librte_eal/linuxapp/eal/eal_pci.c   |   78 +++++++++++++++++++++++++++++++
 3 files changed, 81 insertions(+), 0 deletions(-)

diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
index 0016fc5..52d46f3 100644
--- a/lib/librte_eal/common/Makefile
+++ b/lib/librte_eal/common/Makefile
@@ -35,6 +35,7 @@ INC := rte_atomic.h rte_branch_prediction.h rte_byteorder.h rte_common.h
 INC += rte_cycles.h rte_debug.h rte_eal.h rte_errno.h rte_launch.h rte_lcore.h
 INC += rte_log.h rte_memcpy.h rte_memory.h rte_memzone.h rte_pci.h
 INC += rte_pci_dev_ids.h rte_per_lcore.h rte_prefetch.h rte_random.h
+INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
 INC += rte_rwlock.h rte_spinlock.h rte_tailq.h rte_interrupts.h rte_alarm.h
 INC += rte_string_fns.h rte_cpuflags.h rte_version.h rte_tailq_elem.h
 INC += rte_eal_memconfig.h rte_malloc_heap.h
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index c793773..5883cd9 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -81,6 +81,7 @@ extern "C" {
 #include <stdint.h>
 #include <inttypes.h>
 #include <rte_interrupts.h>
+#include <rte_pci_dev_feature_defs.h>
 
 TAILQ_HEAD(pci_device_list, rte_pci_device); /**< PCI devices in D-linked Q. */
 TAILQ_HEAD(pci_driver_list, rte_pci_driver); /**< PCI drivers in D-linked Q. */
@@ -150,6 +151,7 @@ struct rte_pci_device {
 	const struct rte_pci_driver *driver;    /**< Associated driver */
 	uint16_t max_vfs;                       /**< sriov enable if not zero */
 	int numa_node;                          /**< NUMA node connection */
+	enum igbuio_intr_mode intr_mode;        /**< Interrupt mode */
 	struct rte_devargs *devargs;            /**< Device user arguments */
 };
 
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index ac2c1fe..7dba446 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -67,6 +67,7 @@
 #include <rte_devargs.h>
 
 #include "rte_pci_dev_ids.h"
+#include "rte_pci_dev_feature_defs.h"
 #include "eal_filesystem.h"
 #include "eal_private.h"
 
@@ -89,6 +90,17 @@ struct uio_map {
 	uint64_t phaddr;
 };
 
+struct rte_pci_dev_intr_mode {
+	enum igbuio_intr_mode mode;
+	const char *name;
+};
+
+/* Table of interrupt modes */
+const struct rte_pci_dev_intr_mode interrupt_modes[] = {
+#define RTE_PCI_DEV_INTR_MODE(id, mode_name) {INTR_MODE(id, mode_name)},
+#include <rte_pci_dev_features.h>
+};
+
 /*
  * For multi-process we need to reproduce all PCI mappings in secondary
  * processes, so save them in a tailq.
@@ -106,6 +118,7 @@ TAILQ_HEAD(uio_res_list, uio_resource);
 
 static struct uio_res_list *uio_res_list = NULL;
 static int pci_parse_sysfs_value(const char *filename, uint64_t *val);
+static int pci_parse_sysfs_intr_mode(const char *filename, struct rte_pci_device *dev);
 
 /* unbind kernel driver for this device */
 static int
@@ -400,6 +413,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	int i, j;
 	char dirname[PATH_MAX];
 	char devname[PATH_MAX]; /* contains the /dev/uioX */
+    char filename[PATH_MAX];
 	void *mapaddr;
 	int uio_num;
 	uint64_t phaddr;
@@ -435,6 +449,18 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	}
 	dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
 
+	rte_snprintf(filename, sizeof(filename),
+	         	SYSFS_PCI_DEVICES "/" PCI_PRI_FMT
+	         	"/"RTE_PCI_DEV_FEATURE_INTR_MODE,
+	         	loc->domain, loc->bus, loc->devid, loc->function);
+
+    /* Get the kernel configured interrupt mode */
+	if (pci_parse_sysfs_intr_mode(filename, dev) < 0) {
+		RTE_LOG(ERR, EAL, "%s(): cannot determine interrupt_mode\n",
+				__func__);
+		return -1;
+	}
+
 	/* allocate the mapping details for secondary processes*/
 	if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == NULL) {
 		RTE_LOG(ERR, EAL,
@@ -591,6 +617,58 @@ pci_parse_sysfs_value(const char *filename, uint64_t *val)
         return 0;
 }
 
+/* 
+ * Parse a sysfs file containing a string
+ */ 
+static int
+pci_parse_sysfs_string(const char *filename, char *result_str, int max_len)
+{
+	FILE *f;
+	size_t len;
+
+	f = fopen(filename, "r");
+	if (f == NULL) {
+		RTE_LOG(ERR, EAL, "%s(): cannot open sysfs value %s\n",
+				__func__, filename);
+		return -1;
+	}
+	if (fgets(result_str, max_len, f) == NULL) {
+		RTE_LOG(ERR, EAL, "%s(): cannot read sysfs value %s\n",
+				__func__, filename);
+		fclose(f);
+		return -1;
+	}
+	len = strnlen(result_str, max_len) -1;
+	if (result_str[len] == '\n')
+		result_str[len] = '\0';
+
+	fclose(f);
+	return 0;
+}
+
+/*
+ * Determine the kernel configured interrupt mode
+ */
+static int
+pci_parse_sysfs_intr_mode(const char *filename, struct rte_pci_device *dev)
+{
+	char intr_mode[INTR_NAME_LEN];
+	unsigned int i, num_intr_modes = RTE_DIM(interrupt_modes);
+
+	if (pci_parse_sysfs_string(filename, intr_mode, RTE_DIM(intr_mode)) < 0) {
+		RTE_LOG(ERR, EAL, "%s(): cannot parse interrupt_mode\n",
+			__func__);
+		return -1;
+	}
+
+	for (i = 0; i < num_intr_modes; i++)
+		if (!strncmp(intr_mode, interrupt_modes[i].name, INTR_NAME_LEN)) {
+			dev->intr_mode = interrupt_modes[i].mode;
+			return 0;
+		}
+	return -1;
+}
+
 /* Compare two PCI device addresses. */
 static int
 pci_addr_comparison(struct rte_pci_addr *addr, struct rte_pci_addr *addr2)
-- 
1.7.0.7

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

* [dpdk-dev] [PATCH 3/4] [PATCH 3/4] FreeBSD: Adds the equivalent interrupt mode setting and parsing
  2014-06-05 14:39 [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested Alan Carew
  2014-06-05 14:39 ` [dpdk-dev] [PATCH 1/4] [PATCH 1/4] igb_uio: Add interrupt_mode sysfs entry for igb_uio devices Alan Carew
  2014-06-05 14:39 ` [dpdk-dev] [PATCH 2/4] [PATCH 2/4] eal_pci: Add interrupt mode to rte_pci_device and parsing to eal_pci Alan Carew
@ 2014-06-05 14:39 ` Alan Carew
  2014-06-05 14:39 ` [dpdk-dev] [PATCH 4/4] [PATCH 4/4] virtio: Fixes the VIRTIO_PCI_CONFIG macro to use the correct offset to the Virtio header Alan Carew
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Alan Carew @ 2014-06-05 14:39 UTC (permalink / raw)
  To: dev

This patch adds the equivalent functionality to FreeBSD as with patches 1 and 2

Signed-off-by: Alan Carew <alan.carew@intel.com>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c     |   45 +++++++++++++++++++++++++++++++
 lib/librte_eal/bsdapp/nic_uio/nic_uio.c |   14 +++++++++
 2 files changed, 59 insertions(+), 0 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 94ae461..7c270bb 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -50,6 +50,7 @@
 #include <sys/ioctl.h>
 #include <sys/pciio.h>
 #include <dev/pci/pcireg.h>
+#include <sys/sysctl.h>
 
 #include <rte_interrupts.h>
 #include <rte_log.h>
@@ -108,6 +109,44 @@ TAILQ_HEAD(uio_res_list, uio_resource);
 
 static struct uio_res_list *uio_res_list = NULL;
 
+struct rte_pci_dev_intr_mode {
+	enum igbuio_intr_mode mode;
+	const char *name;
+};
+
+/* Table of interrupt modes */
+const struct rte_pci_dev_intr_mode interrupt_modes[] = {
+#define RTE_PCI_DEV_INTR_MODE(id, mode_name) {INTR_MODE (id, mode_name)},
+#include <rte_pci_dev_features.h>
+};
+
+/*
+ * Determine the kernel configured interrupt mode
+ */
+static int
+pci_parse_intr_mode(struct rte_pci_device *dev)
+{
+	char intr_mode[INTR_NAME_LEN];
+	unsigned int i, num_intr_modes = RTE_DIM(interrupt_modes);
+	size_t sysctl_size = sizeof(intr_mode);
+
+	if (sysctlbyname("hw.nic_uio."RTE_PCI_DEV_FEATURE_INTR_MODE, &intr_mode,
+				&sysctl_size, NULL, 0) < 0) {
+		RTE_LOG(ERR, EAL,
+			"%s(): cannot get sysctlbyname: hw.nic_uio.intr_mode\n",
+			 __func__);
+		return (-1);
+	}
+	for (i = 0; i < num_intr_modes; i++) {
+		if (!strncmp(intr_mode, interrupt_modes[i].name, INTR_NAME_LEN)) {
+			dev->intr_mode = interrupt_modes[i].mode;
+			return 0;
+		}
+	}
+	return -1;
+	
+}
+
 /* unbind kernel driver for this device */
 static int
 pci_unbind_kernel_driver(struct rte_pci_device *dev)
@@ -220,6 +259,12 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		return -1;
 	}
 
+	if (pci_parse_intr_mode(dev) < 0) {
+		RTE_LOG(ERR, EAL,
+				"%s(): unable to determine interrupt mode\n", __func__);
+		return (-1);
+	}
+
 	/* save fd if in primary process */
 	dev->intr_handle.fd = open(devname, O_RDWR);
 	if (dev->intr_handle.fd < 0) {
diff --git a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
index c10e9aa..0e17d63 100644
--- a/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
+++ b/lib/librte_eal/bsdapp/nic_uio/nic_uio.c
@@ -51,6 +51,10 @@ __FBSDID("$FreeBSD$");
 #include <vm/vm_object.h>
 #include <vm/vm_page.h>
 #include <vm/vm_pager.h>
+#include <sys/sysctl.h>
+
+#include <rte_pci_dev_feature_defs.h>
+#include <rte_pci_dev_features.h>
 
 
 #define MAX_BARS (PCIR_MAX_BAR_0 + 1)
@@ -116,6 +120,16 @@ const struct device devices[] = {
 };
 #define NUM_DEVICES (sizeof(devices)/sizeof(devices[0]))
 
+static char nic_uio_intr_mode[] = {IGBUIO_NONE_INTR_NAME};
+
+TUNABLE_STR("hw.nic_uio."RTE_PCI_DEV_FEATURE_INTR_MODE, nic_uio_intr_mode, sizeof(nic_uio_intr_mode));
+
+static SYSCTL_NODE(_hw, OID_AUTO, nic_uio, CTLFLAG_RD, 0, "nic_uio");
+
+SYSCTL_STRING(_hw_nic_uio, OID_AUTO, interrupt_mode, CTLFLAG_RW,
+			&nic_uio_intr_mode, sizeof(nic_uio_intr_mode),
+			"Configured interrupt mode");
+
 
 static devclass_t nic_uio_devclass;
 
-- 
1.7.0.7

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

* [dpdk-dev] [PATCH 4/4] [PATCH 4/4] virtio: Fixes the VIRTIO_PCI_CONFIG macro to use the correct offset to the Virtio header
  2014-06-05 14:39 [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested Alan Carew
                   ` (2 preceding siblings ...)
  2014-06-05 14:39 ` [dpdk-dev] [PATCH 3/4] [PATCH 3/4] FreeBSD: Adds the equivalent interrupt mode setting and parsing Alan Carew
@ 2014-06-05 14:39 ` Alan Carew
  2014-06-05 15:22 ` [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested Stephen Hemminger
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Alan Carew @ 2014-06-05 14:39 UTC (permalink / raw)
  To: dev

This final patch address the issue of not being able to determine the correct
offet when MSI-X is disabled.

Signed-off-by: Alan Carew <alan.carew@intel.com>
---
 lib/librte_pmd_virtio/virtio_ethdev.c |    1 +
 lib/librte_pmd_virtio/virtio_pci.h    |    4 +++-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index c2b4dfb..819e7d7 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -719,6 +719,7 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 	}
 #endif
 	hw->io_base = (uint32_t)(uintptr_t)pci_dev->mem_resource[0].addr;
+	hw->intr_mode = pci_dev->intr_mode;
 
 	/* Reset the device although not necessary at startup */
 	vtpci_reset(hw);
diff --git a/lib/librte_pmd_virtio/virtio_pci.h b/lib/librte_pmd_virtio/virtio_pci.h
index 96443c7..7b3d0ef 100644
--- a/lib/librte_pmd_virtio/virtio_pci.h
+++ b/lib/librte_pmd_virtio/virtio_pci.h
@@ -44,6 +44,7 @@
 #endif
 
 #include <rte_ethdev.h>
+#include <rte_pci_dev_feature_defs.h>
 
 struct virtqueue;
 
@@ -177,6 +178,7 @@ struct virtio_hw {
 	uint16_t    subsystem_device_id;
 	uint16_t    subsystem_vendor_id;
 	uint8_t     revision_id;
+	enum        igbuio_intr_mode  intr_mode;
 	uint8_t     mac_addr[ETHER_ADDR_LEN];
 	int         adapter_stopped;
 	struct      rte_eth_stats eth_stats;
@@ -201,7 +203,7 @@ struct virtio_net_config {
  * The remaining space is defined by each driver as the per-driver
  * configuration space.
  */
-#define VIRTIO_PCI_CONFIG(hw) (((hw)->guest_features & VIRTIO_PCI_FLAG_MSIX) ? 24 : 20)
+#define VIRTIO_PCI_CONFIG(hw) (((hw)->intr_mode == IGBUIO_MSIX_INTR_MODE) ? 24 : 20)
 
 /*
  * How many bits to shift physical queue address written to QUEUE_PFN.
-- 
1.7.0.7

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

* Re: [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested
  2014-06-05 14:39 [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested Alan Carew
                   ` (3 preceding siblings ...)
  2014-06-05 14:39 ` [dpdk-dev] [PATCH 4/4] [PATCH 4/4] virtio: Fixes the VIRTIO_PCI_CONFIG macro to use the correct offset to the Virtio header Alan Carew
@ 2014-06-05 15:22 ` Stephen Hemminger
  2014-06-05 17:06 ` Stephen Hemminger
  2014-06-16 10:19 ` Carew, Alan
  6 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2014-06-05 15:22 UTC (permalink / raw)
  To: Alan Carew; +Cc: dev

On Thu,  5 Jun 2014 15:39:15 +0100
Alan Carew <alan.carew@intel.com> wrote:

> This series addresses an issue with librte_pmd_virtio where the offset to the
> virtio device specific header may be incorrect depending on whether MSI-X has
> been enabled or not.
> 
> If MSI-X is configured the device specific header is placed at byte offset 24
> relative to the IO base address. 
> If MSI-X is not configured the device specific header is placed at
> byte offset 20. 
> 
> The following macro defined in virtio_pci.h is used to test the
> presence of the MSI-X header and determine the correct offset: 
> #define VIRTIO_PCI_CONFIG(hw) (((hw)->guest_features & VIRTIO_PCI_FLAG_MSIX) ? 24 : 20) 
> 
> However, VIRTIO_PCI_FLAG_MSIX is not a guest_feature nor is it part of the
> Virtio Specification and resolves to the VIRTIO_NET_F_MAC feature as both
> are #defined as 0x20. 
> 
> VIRTIO_PCI_FLAG_MSIX or similar flag should instead be set by the kernel
> driver allocating resources and passed to user space for testing. 
> i.e. 
> #define VIRTIO_PCI_CONFIG(hw) (((hw)->intr_mode & IGBUIO_MSIX_INTR_MODE) ? 24 : 20)
> 
> To enable this testing of interrupt mode, this series allows for the kernel
> driver(igb_uio) to place the configured interrupt mode into a sysfs entry.
> sysfs is then parsed by eal_pci to determine the configured mode, which
> allows all user space devices to correctly determine the interrupt mode,
> including virtio_ethdev.
> 
> This series should be applied prior to Anatoly Burakov's
> [VFIO] Add VFIO support to DPDK series
> 
> Alan Carew (4):
>   igb_uio: Add interrupt_mode sysfs entry for igb_uio devices
>   eal_pci: Add interrupt mode to rte_pci_device and parsing to eal_pci
>   FreeBSD: Adds the equivalent interrupt mode setting and parsing
>   virtio: Fixes the VIRTIO_PCI_CONFIG macro to use the correct offset
>     to the Virtio header
> 
>  lib/librte_eal/bsdapp/eal/eal_pci.c                |   44 ++++++++++
>  lib/librte_eal/bsdapp/nic_uio/nic_uio.c            |   14 +++
>  lib/librte_eal/common/Makefile                     |    1 +
>  lib/librte_eal/common/include/rte_pci.h            |    2 +
>  .../common/include/rte_pci_dev_feature_defs.h      |   85 ++++++++++++++++++++
>  .../common/include/rte_pci_dev_features.h          |   70 ++++++++++++++++
>  lib/librte_eal/linuxapp/eal/eal_pci.c              |   78 ++++++++++++++++++
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c          |   48 +++++++++---
>  lib/librte_pmd_virtio/virtio_ethdev.c              |    1 +
>  lib/librte_pmd_virtio/virtio_pci.h                 |    4 +-
>  10 files changed, 336 insertions(+), 11 deletions(-)
>  create mode 100755 lib/librte_eal/common/include/rte_pci_dev_feature_defs.h
>  create mode 100755 lib/librte_eal/common/include/rte_pci_dev_features.h
> 

I took the simpler approach. Since DPDK devices doesn't really need the multiple
vectors of MSI-X at all, I just converted igb_uio to use MSI instead.

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

* Re: [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested
  2014-06-05 14:39 [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested Alan Carew
                   ` (4 preceding siblings ...)
  2014-06-05 15:22 ` [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested Stephen Hemminger
@ 2014-06-05 17:06 ` Stephen Hemminger
  2014-06-16 10:19 ` Carew, Alan
  6 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2014-06-05 17:06 UTC (permalink / raw)
  To: Alan Carew; +Cc: dev

On Thu,  5 Jun 2014 15:39:15 +0100
Alan Carew <alan.carew@intel.com> wrote:

> This series addresses an issue with librte_pmd_virtio where the offset to the
> virtio device specific header may be incorrect depending on whether MSI-X has
> been enabled or not.
> 
> If MSI-X is configured the device specific header is placed at byte offset 24
> relative to the IO base address. 
> If MSI-X is not configured the device specific header is placed at
> byte offset 20. 
> 
> The following macro defined in virtio_pci.h is used to test the
> presence of the MSI-X header and determine the correct offset: 
> #define VIRTIO_PCI_CONFIG(hw) (((hw)->guest_features & VIRTIO_PCI_FLAG_MSIX) ? 24 : 20) 
> 
> However, VIRTIO_PCI_FLAG_MSIX is not a guest_feature nor is it part of the
> Virtio Specification and resolves to the VIRTIO_NET_F_MAC feature as both
> are #defined as 0x20. 
> 
> VIRTIO_PCI_FLAG_MSIX or similar flag should instead be set by the kernel
> driver allocating resources and passed to user space for testing. 
> i.e. 
> #define VIRTIO_PCI_CONFIG(hw) (((hw)->intr_mode & IGBUIO_MSIX_INTR_MODE) ? 24 : 20)
> 
> To enable this testing of interrupt mode, this series allows for the kernel
> driver(igb_uio) to place the configured interrupt mode into a sysfs entry.
> sysfs is then parsed by eal_pci to determine the configured mode, which
> allows all user space devices to correctly determine the interrupt mode,
> including virtio_ethdev.

Why not scan for the IRQ in /proc/interrupts rather than changing the kernel driver.
I ask because my goal is to get rid of igb_uio and it's quirks and use
uio_pci_generic instead.

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

* Re: [dpdk-dev] [PATCH 1/4] [PATCH 1/4] igb_uio: Add interrupt_mode sysfs entry for igb_uio devices
  2014-06-05 14:39 ` [dpdk-dev] [PATCH 1/4] [PATCH 1/4] igb_uio: Add interrupt_mode sysfs entry for igb_uio devices Alan Carew
@ 2014-06-05 17:20   ` Stephen Hemminger
  0 siblings, 0 replies; 9+ messages in thread
From: Stephen Hemminger @ 2014-06-05 17:20 UTC (permalink / raw)
  To: Alan Carew; +Cc: dev

On Thu,  5 Jun 2014 15:39:16 +0100
Alan Carew <alan.carew@intel.com> wrote:

> This patch adds an "interrupt_mode" sysfs entry for igb_uio devices,
> allowing userspace eal_pci to track which interrupt mode has been enabled in kernel space.
> The sysfs entry can be inspected via /sys/bus/pci/devices/<D:B:D:F/interrupt_mode
> 
> Signed-off-by: Alan Carew <alan.carew@intel.com>
> ---
>  .../common/include/rte_pci_dev_feature_defs.h      |   85 ++++++++++++++++++++
>  .../common/include/rte_pci_dev_features.h          |   70 ++++++++++++++++
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c          |   48 +++++++++---
>  3 files changed, 193 insertions(+), 10 deletions(-)
>  create mode 100755 lib/librte_eal/common/include/rte_pci_dev_feature_defs.h
>  create mode 100755 lib/librte_eal/common/include/rte_pci_dev_features.h
> 
> diff --git a/lib/librte_eal/common/include/rte_pci_dev_feature_defs.h b/lib/librte_eal/common/include/rte_pci_dev_feature_defs.h
> new file mode 100755
> index 0000000..d23ed7d
> --- /dev/null
> +++ b/lib/librte_eal/common/include/rte_pci_dev_feature_defs.h
> @@ -0,0 +1,85 @@
> +/*-
> + * This file is provided under a dual BSD/GPLv2 license.  When using or
> + *   redistributing this file, you may do so under either license.
> + * 
> + *   GPL LICENSE SUMMARY
> + * 
> + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + * 
> + *   This program is free software; you can redistribute it and/or modify
> + *   it under the terms of version 2 of the GNU General Public License as
> + *   published by the Free Software Foundation.
> + * 
> + *   This program is distributed in the hope that it will be useful, but
> + *   WITHOUT ANY WARRANTY; without even the implied warranty of
> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *   General Public License for more details.
> + * 
> + *   You should have received a copy of the GNU General Public License
> + *   along with this program; if not, write to the Free Software
> + *   Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
> + *   The full GNU General Public License is included in this distribution
> + *   in the file called LICENSE.GPL.
> + * 
> + *   Contact Information:
> + *   Intel Corporation
> + * 
> + *   BSD LICENSE
> + * 
> + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + * 
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + * 
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + * 
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + * 
> + */
> +
> +#ifndef _RTE_PCI_DEV_DEFS_H_
> +#define _RTE_PCI_DEV_DEFS_H_
> +
> +#define RTE_PCI_DEV_FEATURE_INTR_MODE  "interrupt_mode"
> +
> +#define INTR_NAME_LEN 10
> +
> +#define IGBUIO_NONE_INTR_NAME "none"
> +#define IGBUIO_LEGACY_INTR_NAME "legacy"
> +#define IGBUIO_MSI_INTR_NAME "msi"
> +#define IGBUIO_MSIX_INTR_NAME "msix"
> +
> +
> +#define INTR_MODE(id, mode_name) \
> +        .mode = (id), .name = (mode_name)
> +
> +/* interrupt mode */
> +enum igbuio_intr_mode {
> +	IGBUIO_NONE_INTR_MODE = 0,
> +	IGBUIO_LEGACY_INTR_MODE,
> +	IGBUIO_MSI_INTR_MODE,
> +	IGBUIO_MSIX_INTR_MODE,
> +	IGBUIO_INTR_MODE_MAX
> +};
> +
> +#endif /* _RTE_PCI_DEV_DEFS_H_ */
> diff --git a/lib/librte_eal/common/include/rte_pci_dev_features.h b/lib/librte_eal/common/include/rte_pci_dev_features.h
> new file mode 100755
> index 0000000..a45c056
> --- /dev/null
> +++ b/lib/librte_eal/common/include/rte_pci_dev_features.h
> @@ -0,0 +1,70 @@
> +/*-
> + * This file is provided under a dual BSD/GPLv2 license.  When using or
> + *   redistributing this file, you may do so under either license.
> + * 
> + *   GPL LICENSE SUMMARY
> + * 
> + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + * 
> + *   This program is free software; you can redistribute it and/or modify
> + *   it under the terms of version 2 of the GNU General Public License as
> + *   published by the Free Software Foundation.
> + * 
> + *   This program is distributed in the hope that it will be useful, but
> + *   WITHOUT ANY WARRANTY; without even the implied warranty of
> + *   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *   General Public License for more details.
> + * 
> + *   You should have received a copy of the GNU General Public License
> + *   along with this program; if not, write to the Free Software
> + *   Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
> + *   The full GNU General Public License is included in this distribution
> + *   in the file called LICENSE.GPL.
> + * 
> + *   Contact Information:
> + *   Intel Corporation
> + * 
> + *   BSD LICENSE
> + * 
> + *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   All rights reserved.
> + * 
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + * 
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of Intel Corporation nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + * 
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + * 
> + */
> +
> +#ifndef RTE_PCI_DEV_INTR_MODE
> +#define RTE_PCI_DEV_INTR_MODE(id, mode_name)
> +#endif
> +
> +RTE_PCI_DEV_INTR_MODE(IGBUIO_NONE_INTR_MODE, IGBUIO_NONE_INTR_NAME)
> +RTE_PCI_DEV_INTR_MODE(IGBUIO_LEGACY_INTR_MODE, IGBUIO_LEGACY_INTR_NAME)
> +RTE_PCI_DEV_INTR_MODE(IGBUIO_MSI_INTR_MODE, IGBUIO_MSI_INTR_NAME)
> +RTE_PCI_DEV_INTR_MODE(IGBUIO_MSIX_INTR_MODE, IGBUIO_MSIX_INTR_NAME)
> +
> +#undef RTE_PCI_DEV_INTR_MODE
> +
> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> index 09c40bf..dd1124b 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> @@ -34,6 +34,8 @@
>  #include <xen/xen.h>
>  #endif
>  
> +#include <rte_pci_dev_feature_defs.h>
> +
>  /**
>   * MSI-X related macros, copy from linux/pci_regs.h in kernel 2.6.39,
>   * but none of them in kernel 2.6.35.
> @@ -49,12 +51,15 @@
>  
>  #define IGBUIO_NUM_MSI_VECTORS 1
>  
> -/* interrupt mode */
> -enum igbuio_intr_mode {
> -	IGBUIO_LEGACY_INTR_MODE = 0,
> -	IGBUIO_MSI_INTR_MODE,
> -	IGBUIO_MSIX_INTR_MODE,
> -	IGBUIO_INTR_MODE_MAX
> +struct rte_pci_dev_intr_mode {
> +	enum igbuio_intr_mode mode;
> +	const char *name;
> +};
> +
> +/* Table of interrupt modes */
> +static struct rte_pci_dev_intr_mode interrupt_modes[] = {
> +#define RTE_PCI_DEV_INTR_MODE(id, mode_name) {INTR_MODE(id, mode_name)},
> +#include <rte_pci_dev_features.h>
>  };
>  
>  /**
> @@ -150,8 +155,32 @@ store_max_vfs(struct device *dev, struct device_attribute *attr,
>  }
>  
>  static DEVICE_ATTR(max_vfs, S_IRUGO | S_IWUSR, show_max_vfs, store_max_vfs);
> +
> +static ssize_t
> +show_interrupt_mode(struct device *dev, struct device_attribute *attr,
> +	     char *buf)
> +{
> +	unsigned i, num_modes;
> +	enum igbuio_intr_mode mode;
> +	struct uio_info *info;
> +	struct pci_dev *pdev = container_of(dev, struct pci_dev, dev);
> +
> +	info = pci_get_drvdata(pdev);
> +	mode = ((struct rte_uio_pci_dev *)info->priv)->mode;
> +	num_modes = sizeof(interrupt_modes)/sizeof(interrupt_modes[0]);
> +	
> +	for (i = 0; i < num_modes; i++) {
> +		if (mode == interrupt_modes[i].mode)
> +			return snprintf(buf, INTR_NAME_LEN, "%s\n", interrupt_modes[i].name );
> +	}
> +	return snprintf(buf, INTR_NAME_LEN, "unknown\n");
> +}
> +
> +static DEVICE_ATTR(interrupt_mode, S_IRUGO, show_interrupt_mode, NULL);
> +
>  static struct attribute *dev_attrs[] = {
>  	&dev_attr_max_vfs.attr,
> +	&dev_attr_interrupt_mode.attr,
>          NULL,
>  };
>  
> @@ -520,7 +549,7 @@ igbuio_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
>  #endif
>  	udev->info.priv = udev;
>  	udev->pdev = dev;
> -	udev->mode = 0; /* set the default value for interrupt mode */
> +	udev->mode = IGBUIO_LEGACY_INTR_MODE; /* set the default value for interrupt mode */
>  	spin_lock_init(&udev->lock);
>  
>  	/* check if it need to try msix first */
> @@ -610,11 +639,10 @@ igbuio_config_intr_mode(char *intr_str)
>  		printk(KERN_INFO "Use MSIX interrupt by default\n");
>  		return 0;
>  	}
> -
> -	if (!strcmp(intr_str, "msix")) {
> +	if (!strcmp(intr_str, IGBUIO_MSIX_INTR_NAME)) {
>  		igbuio_intr_mode_preferred = IGBUIO_MSIX_INTR_MODE;
>  		printk(KERN_INFO "Use MSIX interrupt\n");
> -	} else if (!strcmp(intr_str, "legacy")) {
> +	} else if (!strcmp(intr_str, IGBUIO_LEGACY_INTR_NAME)) {
>  		igbuio_intr_mode_preferred = IGBUIO_LEGACY_INTR_MODE;
>  		printk(KERN_INFO "Use legacy interrupt\n");
>  	} else {

This assumes that all devices have same interrupt mode.
The whole intr_mode_preferred logic in this driver is borked.

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

* Re: [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested
  2014-06-05 14:39 [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested Alan Carew
                   ` (5 preceding siblings ...)
  2014-06-05 17:06 ` Stephen Hemminger
@ 2014-06-16 10:19 ` Carew, Alan
  6 siblings, 0 replies; 9+ messages in thread
From: Carew, Alan @ 2014-06-16 10:19 UTC (permalink / raw)
  To: dev

> -----Original Message-----
> From: Carew, Alan
> Sent: Thursday, June 05, 2014 3:39 PM
> To: dev@dpdk.org
> Cc: Carew, Alan
> Subject: [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio
> feature bit-flag tested
> 
> This series addresses an issue with librte_pmd_virtio where the offset to the
> virtio device specific header may be incorrect depending on whether MSI-X has
> been enabled or not.
> 
> If MSI-X is configured the device specific header is placed at byte offset 24
> relative to the IO base address.
> If MSI-X is not configured the device specific header is placed at
> byte offset 20.
> 
> The following macro defined in virtio_pci.h is used to test the
> presence of the MSI-X header and determine the correct offset:
> #define VIRTIO_PCI_CONFIG(hw) (((hw)->guest_features &
> VIRTIO_PCI_FLAG_MSIX) ? 24 : 20)
> 
> However, VIRTIO_PCI_FLAG_MSIX is not a guest_feature nor is it part of the
> Virtio Specification and resolves to the VIRTIO_NET_F_MAC feature as both
> are #defined as 0x20.
> 
> VIRTIO_PCI_FLAG_MSIX or similar flag should instead be set by the kernel
> driver allocating resources and passed to user space for testing.
> i.e.
> #define VIRTIO_PCI_CONFIG(hw) (((hw)->intr_mode &
> IGBUIO_MSIX_INTR_MODE) ? 24 : 20)
> 
> To enable this testing of interrupt mode, this series allows for the kernel
> driver(igb_uio) to place the configured interrupt mode into a sysfs entry.
> sysfs is then parsed by eal_pci to determine the configured mode, which
> allows all user space devices to correctly determine the interrupt mode,
> including virtio_ethdev.
> 
> This series should be applied prior to Anatoly Burakov's
> [VFIO] Add VFIO support to DPDK series
> 
> Alan Carew (4):
>   igb_uio: Add interrupt_mode sysfs entry for igb_uio devices
>   eal_pci: Add interrupt mode to rte_pci_device and parsing to eal_pci
>   FreeBSD: Adds the equivalent interrupt mode setting and parsing
>   virtio: Fixes the VIRTIO_PCI_CONFIG macro to use the correct offset
>     to the Virtio header
> 
>  lib/librte_eal/bsdapp/eal/eal_pci.c                |   44 ++++++++++
>  lib/librte_eal/bsdapp/nic_uio/nic_uio.c            |   14 +++
>  lib/librte_eal/common/Makefile                     |    1 +
>  lib/librte_eal/common/include/rte_pci.h            |    2 +
>  .../common/include/rte_pci_dev_feature_defs.h      |   85
> ++++++++++++++++++++
>  .../common/include/rte_pci_dev_features.h          |   70 ++++++++++++++++
>  lib/librte_eal/linuxapp/eal/eal_pci.c              |   78 ++++++++++++++++++
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c          |   48 +++++++++---
>  lib/librte_pmd_virtio/virtio_ethdev.c              |    1 +
>  lib/librte_pmd_virtio/virtio_pci.h                 |    4 +-
>  10 files changed, 336 insertions(+), 11 deletions(-)
>  create mode 100755
> lib/librte_eal/common/include/rte_pci_dev_feature_defs.h
>  create mode 100755 lib/librte_eal/common/include/rte_pci_dev_features.h

Self NAK, alternative approach and patch set to follow.

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

end of thread, other threads:[~2014-06-16 10:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-05 14:39 [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested Alan Carew
2014-06-05 14:39 ` [dpdk-dev] [PATCH 1/4] [PATCH 1/4] igb_uio: Add interrupt_mode sysfs entry for igb_uio devices Alan Carew
2014-06-05 17:20   ` Stephen Hemminger
2014-06-05 14:39 ` [dpdk-dev] [PATCH 2/4] [PATCH 2/4] eal_pci: Add interrupt mode to rte_pci_device and parsing to eal_pci Alan Carew
2014-06-05 14:39 ` [dpdk-dev] [PATCH 3/4] [PATCH 3/4] FreeBSD: Adds the equivalent interrupt mode setting and parsing Alan Carew
2014-06-05 14:39 ` [dpdk-dev] [PATCH 4/4] [PATCH 4/4] virtio: Fixes the VIRTIO_PCI_CONFIG macro to use the correct offset to the Virtio header Alan Carew
2014-06-05 15:22 ` [dpdk-dev] [PATCH 0/4] librte_pmd_virtio :Fix: virtio_pci.h non-existent virtio feature bit-flag tested Stephen Hemminger
2014-06-05 17:06 ` Stephen Hemminger
2014-06-16 10:19 ` Carew, Alan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).