[dpdk-dev] [PATCH v3 17/25] virtio: Use port IO to get PCI resource.
Thomas Monjalon
thomas.monjalon at 6wind.com
Fri Jan 30 00:14:27 CET 2015
Hi Changchun,
2015-01-29 15:24, Ouyang Changchun:
> Make virtio not require UIO for some security reasons, this is to match 6Wind's virtio-net-pmd.
Thanks for your effort.
I think port IO is a really interesting option but it needs more EAL rework
to be correctly integrated. Then virtio-net-pmd (http://dpdk.org/browse/virtio-net-pmd/)
will be obsolete and moved in a deprecated area.
> --- a/config/common_linuxapp
> +++ b/config/common_linuxapp
> +# Only for VIRTIO PMD currently
> +CONFIG_RTE_EAL_PORT_IO=n
This is the first problem. We must stop adding new build-time options.
We should be able to choose between PCI mapping and port IO at runtime.
> +/** Device needs port IO(done with /proc/ioports) */
> +#ifdef RTE_EAL_PORT_IO
> +#define RTE_PCI_DRV_PORT_IO 0x0002
> +#endif
A flag should never be ifdef'ed.
> @@ -574,7 +574,10 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
> /* map resources for devices that use igb_uio */
> ret = pci_map_device(dev);
> if (ret != 0)
> - return ret;
> +#ifdef RTE_EAL_PORT_IO
> + if ((dr->drv_flags & RTE_PCI_DRV_PORT_IO) == 0)
> +#endif
> + return ret;
> } else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
> rte_eal_process_type() == RTE_PROC_PRIMARY) {
> /* unbind current driver */
Why do you need this ugly return?
> --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> @@ -961,6 +961,71 @@ static int virtio_resource_init(struct rte_pci_device *pci_dev)
> start, size);
> return 0;
> }
> +
> +#ifdef RTE_EAL_PORT_IO
> +/* Extract port I/O numbers from proc/ioports */
> +static int virtio_resource_init_by_portio(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;
> +
> + 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%lx with size=0x%lx",
> + start, size);
> + return 0;
> +}
> +#endif
This part should be a Linux EAL service.
> +#ifdef RTE_EAL_PORT_IO
> +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_PORT_IO |
Why does it need PCI mapping in port IO mode?
> + RTE_PCI_DRV_INTR_LSC,
> + },
> + .eth_dev_init = eth_virtio_dev_init,
> + .dev_private_size = sizeof(struct virtio_hw),
> +};
> +#else
> static struct eth_driver rte_virtio_pmd = {
> {
> .name = "rte_virtio_pmd",
This is the biggest problem.
You are defining port IO as a different driver instead of providing a way to
choose the method for each virtio device.
I think that you should use devargs to configure the pci device.
Thanks
--
Thomas
More information about the dev
mailing list