diff mbox

[dpdk-dev,v4,17/26] virtio: Use port IO to get PCI resource.

Message ID 1423444455-11330-18-git-send-email-changchun.ouyang@intel.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Ouyang Changchun Feb. 9, 2015, 1:14 a.m. UTC 
  Make virtio not require UIO for some security reasons, this is to match 6Wind's virtio-net-pmd.

Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
---
changes in v3:
  Remove macro RTE_EAL_PORT_IO;
  virtio pmd could support uio and ioports method to get the address;  

 lib/librte_pmd_virtio/Makefile        |   2 +
 lib/librte_pmd_virtio/virtio_ethdev.c | 136 +++++++++++++++++++++++++++++-----
 2 files changed, 121 insertions(+), 17 deletions(-)

Comments

Stephen Hemminger Feb. 11, 2015, 4:32 a.m. UTC | #1 
On Mon,  9 Feb 2015 09:14:06 +0800
Ouyang Changchun <changchun.ouyang@intel.com> wrote:

> Make virtio not require UIO for some security reasons, this is to match 6Wind's virtio-net-pmd.
> 
> Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>

This breaks applications that use link state interrupt.

If the non-UIO mode is used link state interrupt fd is not available.
Ouyang Changchun Feb. 11, 2015, 4:50 a.m. UTC | #2 
> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, February 11, 2015 12:33 PM
> To: Ouyang, Changchun
> Cc: dev@dpdk.org; Xie, Huawei; Cao, Waterman; Xu, Qian Q
> Subject: Re: [PATCH v4 17/26] virtio: Use port IO to get PCI resource.
> 
> On Mon,  9 Feb 2015 09:14:06 +0800
> Ouyang Changchun <changchun.ouyang@intel.com> wrote:
> 
> > Make virtio not require UIO for some security reasons, this is to match
> 6Wind's virtio-net-pmd.
> >
> > Signed-off-by: Changchun Ouyang <changchun.ouyang@intel.com>
> 
> This breaks applications that use link state interrupt.
> 
> If the non-UIO mode is used link state interrupt fd is not available.

If application use link state interrupt, then let it work in uio mode rather than proc/ioports mode,
As we know proc/ioports hasn't interrupt, 6wind implementation also don't introduce interrupt in it.

Thanks
Changchun
Vincent Jardin Feb. 11, 2015, 7:29 a.m. UTC | #3 
On 11/02/2015 05:50, Ouyang, Changchun wrote:
> As we know proc/ioports hasn't interrupt, 6wind implementation also don't introduce interrupt in it.
correct, it is running without interrupt, they are not mandatory.
Stephen Hemminger Feb. 11, 2015, 1:50 p.m. UTC | #4 
But driver needs to tell application via driver flags
Thomas Monjalon Feb. 11, 2015, 2:16 p.m. UTC | #5 
2015-02-11 05:50, Stephen Hemminger:
> But driver needs to tell application via driver flags

I think it's done:
> +	pci_dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
> +	pci_drv->drv_flags |= RTE_PCI_DRV_INTR_LSC;

In my understanding, Changchun chose to use ioports as a fallback if uio
is not available. Seems OK.
Ouyang Changchun Feb. 12, 2015, 12:52 a.m. UTC | #6 
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, February 11, 2015 10:16 PM
> To: Stephen Hemminger
> Cc: dev@dpdk.org; Vincent JARDIN; Ouyang, Changchun
> Subject: Re: [dpdk-dev] [PATCH v4 17/26] virtio: Use port IO to get PCI
> resource.
> 
> 2015-02-11 05:50, Stephen Hemminger:
> > But driver needs to tell application via driver flags
> 
> I think it's done:
> > +	pci_dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
> > +	pci_drv->drv_flags |= RTE_PCI_DRV_INTR_LSC;
> 
> In my understanding, Changchun chose to use ioports as a fallback if uio is
> not available. Seems OK.

Thomas is absolutely correct!  :-)

And on the other hand, without uio, disable the interrupt in driver flag:
+	/* can't support lsc interrupt without uio */
+	pci_drv->drv_flags &= ~RTE_PCI_DRV_INTR_LSC;

Thanks
Changchun
diff mbox

Patch

diff --git a/lib/librte_pmd_virtio/Makefile b/lib/librte_pmd_virtio/Makefile
index 456095b..08fa27a 100644
--- a/lib/librte_pmd_virtio/Makefile
+++ b/lib/librte_pmd_virtio/Makefile
@@ -54,4 +54,6 @@  DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_eal lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_mempool lib/librte_mbuf
 DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_net lib/librte_malloc
 
+CFLAGS_virtio_ethdev.o += -Wno-cast-qual
+
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index 8cd2d51..1163d42 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -38,6 +38,7 @@ 
 #include <unistd.h>
 #ifdef RTE_EXEC_ENV_LINUXAPP
 #include <dirent.h>
+#include <fcntl.h>
 #endif
 
 #include <rte_ethdev.h>
@@ -408,11 +409,13 @@  static void
 virtio_dev_close(struct rte_eth_dev *dev)
 {
 	struct virtio_hw *hw = dev->data->dev_private;
+	struct rte_pci_device *pci_dev = dev->pci_dev;
 
 	PMD_INIT_LOG(DEBUG, "virtio_dev_close");
 
 	/* reset the NIC */
-	vtpci_irq_config(hw, VIRTIO_MSI_NO_VECTOR);
+	if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
+		vtpci_irq_config(hw, VIRTIO_MSI_NO_VECTOR);
 	vtpci_reset(hw);
 	virtio_dev_free_mbufs(dev);
 }
@@ -845,9 +848,9 @@  parse_sysfs_value(const char *filename, unsigned long *val)
 	return 0;
 }
 
-static int get_uio_dev(struct rte_pci_addr *loc, char *buf, unsigned int buflen)
+static int get_uio_dev(struct rte_pci_addr *loc, char *buf, unsigned int buflen,
+			unsigned int *uio_num)
 {
-	unsigned int uio_num;
 	struct dirent *e;
 	DIR *dir;
 	char dirname[PATH_MAX];
@@ -884,18 +887,18 @@  static int get_uio_dev(struct rte_pci_addr *loc, char *buf, unsigned int buflen)
 
 		/* first try uio%d */
 		errno = 0;
-		uio_num = strtoull(e->d_name + shortprefix_len, &endptr, 10);
+		*uio_num = strtoull(e->d_name + shortprefix_len, &endptr, 10);
 		if (errno == 0 && endptr != (e->d_name + shortprefix_len)) {
-			snprintf(buf, buflen, "%s/uio%u", dirname, uio_num);
+			snprintf(buf, buflen, "%s/uio%u", dirname, *uio_num);
 			break;
 		}
 
 		/* then try uio:uio%d */
 		errno = 0;
-		uio_num = strtoull(e->d_name + longprefix_len, &endptr, 10);
+		*uio_num = strtoull(e->d_name + longprefix_len, &endptr, 10);
 		if (errno == 0 && endptr != (e->d_name + longprefix_len)) {
 			snprintf(buf, buflen, "%s/uio:uio%u", dirname,
-				     uio_num);
+				     *uio_num);
 			break;
 		}
 	}
@@ -928,13 +931,16 @@  virtio_has_msix(const struct rte_pci_addr *loc)
 }
 
 /* Extract I/O port numbers from sysfs */
-static int virtio_resource_init(struct rte_pci_device *pci_dev)
+static int virtio_resource_init_by_uio(struct rte_pci_device *pci_dev)
 {
 	char dirname[PATH_MAX];
 	char filename[PATH_MAX];
 	unsigned long start, size;
+	unsigned int uio_num;
+	struct rte_pci_driver *pci_drv =
+			(struct rte_pci_driver *)pci_dev->driver;
 
-	if (get_uio_dev(&pci_dev->addr, dirname, sizeof(dirname)) < 0)
+	if (get_uio_dev(&pci_dev->addr, dirname, sizeof(dirname), &uio_num) < 0)
 		return -1;
 
 	/* get portio size */
@@ -959,8 +965,100 @@  static int virtio_resource_init(struct rte_pci_device *pci_dev)
 	PMD_INIT_LOG(DEBUG,
 		     "PCI Port IO found start=0x%lx with size=0x%lx",
 		     start, size);
+
+	/* save fd */
+	memset(dirname, 0, sizeof(dirname));
+	snprintf(dirname, sizeof(dirname), "/dev/uio%u", uio_num);
+	pci_dev->intr_handle.fd = open(dirname, O_RDWR);
+	if (pci_dev->intr_handle.fd < 0) {
+		PMD_INIT_LOG(ERR, "Cannot open %s: %s\n",
+			devname, strerror(errno));
+		return -1;
+	}
+
+	pci_dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
+	pci_drv->drv_flags |= RTE_PCI_DRV_INTR_LSC;
+
 	return 0;
 }
+
+/* Extract port I/O numbers from proc/ioports */
+static int virtio_resource_init_by_ioports(struct rte_pci_device *pci_dev)
+{
+	uint16_t start, end;
+	int size;
+	FILE *fp;
+	char *line = NULL;
+	char pci_id[16];
+	int found = 0;
+	size_t linesz;
+	struct rte_pci_driver *pci_drv =
+			(struct rte_pci_driver *)pci_dev->driver;
+
+	snprintf(pci_id, sizeof(pci_id), PCI_PRI_FMT,
+		 pci_dev->addr.domain,
+		 pci_dev->addr.bus,
+		 pci_dev->addr.devid,
+		 pci_dev->addr.function);
+
+	fp = fopen("/proc/ioports", "r");
+	if (fp == NULL) {
+		PMD_INIT_LOG(ERR, "%s(): can't open ioports", __func__);
+		return -1;
+	}
+
+	while (getdelim(&line, &linesz, '\n', fp) > 0) {
+		char *ptr = line;
+		char *left;
+		int n;
+
+		n = strcspn(ptr, ":");
+		ptr[n] = 0;
+		left = &ptr[n+1];
+
+		while (*left && isspace(*left))
+			left++;
+
+		if (!strncmp(left, pci_id, strlen(pci_id))) {
+			found = 1;
+
+			while (*ptr && isspace(*ptr))
+				ptr++;
+
+			sscanf(ptr, "%04hx-%04hx", &start, &end);
+			size = end - start + 1;
+
+			break;
+		}
+	}
+
+	free(line);
+	fclose(fp);
+
+	if (!found)
+		return -1;
+
+	pci_dev->mem_resource[0].addr = (void *)(uintptr_t)(uint32_t)start;
+	pci_dev->mem_resource[0].len =  (uint64_t)size;
+	PMD_INIT_LOG(DEBUG,
+		"PCI Port IO found start=0x%x with size=0x%x",
+		start, size);
+
+	/* can't support lsc interrupt without uio */
+	pci_drv->drv_flags &= ~RTE_PCI_DRV_INTR_LSC;
+
+	return 0;
+}
+
+/* Extract I/O port numbers from sysfs */
+static int virtio_resource_init(struct rte_pci_device *pci_dev)
+{
+	if (virtio_resource_init_by_uio(pci_dev) == 0)
+		return 0;
+	else
+		return virtio_resource_init_by_ioports(pci_dev);
+}
+
 #else
 static int
 virtio_has_msix(const struct rte_pci_addr *loc __rte_unused)
@@ -969,7 +1067,7 @@  virtio_has_msix(const struct rte_pci_addr *loc __rte_unused)
 	return 0;
 }
 
-static int virtio_resource_init(struct rte_pci_device *pci_dev __rte_unused)
+static int virtio_resource_init(struct rte_pci_device *pci_dev)
 {
 	/* no setup required */
 	return 0;
@@ -1124,7 +1222,8 @@  eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 			pci_dev->id.device_id);
 
 	/* Setup interrupt callback  */
-	rte_intr_callback_register(&pci_dev->intr_handle,
+	if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
+		rte_intr_callback_register(&pci_dev->intr_handle,
 				   virtio_interrupt_handler, eth_dev);
 
 	virtio_dev_cq_start(eth_dev);
@@ -1136,7 +1235,6 @@  static struct eth_driver rte_virtio_pmd = {
 	{
 		.name = "rte_virtio_pmd",
 		.id_table = pci_id_virtio_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 	},
 	.eth_dev_init = eth_virtio_dev_init,
 	.dev_private_size = sizeof(struct virtio_hw),
@@ -1183,6 +1281,7 @@  virtio_dev_configure(struct rte_eth_dev *dev)
 {
 	const struct rte_eth_rxmode *rxmode = &dev->data->dev_conf.rxmode;
 	struct virtio_hw *hw = dev->data->dev_private;
+	struct rte_pci_device *pci_dev = dev->pci_dev;
 
 	PMD_INIT_LOG(DEBUG, "configure");
 
@@ -1200,10 +1299,11 @@  virtio_dev_configure(struct rte_eth_dev *dev)
 		return -ENOTSUP;
 	}
 
-	if (vtpci_irq_config(hw, 0) == VIRTIO_MSI_NO_VECTOR) {
-		PMD_DRV_LOG(ERR, "failed to set config vector");
-		return -EBUSY;
-	}
+	if (pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)
+		if (vtpci_irq_config(hw, 0) == VIRTIO_MSI_NO_VECTOR) {
+			PMD_DRV_LOG(ERR, "failed to set config vector");
+			return -EBUSY;
+		}
 
 	return 0;
 }
@@ -1214,9 +1314,11 @@  virtio_dev_start(struct rte_eth_dev *dev)
 {
 	uint16_t nb_queues, i;
 	struct virtio_hw *hw = dev->data->dev_private;
+	struct rte_pci_device *pci_dev = dev->pci_dev;
 
 	/* check if lsc interrupt feature is enabled */
-	if (dev->data->dev_conf.intr_conf.lsc) {
+	if ((dev->data->dev_conf.intr_conf.lsc) &&
+		(pci_dev->driver->drv_flags & RTE_PCI_DRV_INTR_LSC)) {
 		if (!vtpci_with_feature(hw, VIRTIO_NET_F_STATUS)) {
 			PMD_DRV_LOG(ERR, "link status not supported by host");
 			return -ENOTSUP;