Re: [dpdk-dev] [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver

[Santosh Shukla <sshukla@mvista.com>]

On Tue, Dec 8, 2015 at 3:17 PM, Ananyev, Konstantin <
konstantin.ananyev@intel.com> wrote:

> Hi,
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Santosh Shukla
> > Sent: Friday, December 04, 2015 5:35 PM
> > To: dev@dpdk.org
> > Cc: Rizwan Ansari
> > Subject: [dpdk-dev] [PATCH 6/6] virtio: arm/arm64: memory mapped IO
> support in pmd driver
> >
> > This patch set add memory-mapped-IO support for arm/arm64 archs in
> virtio-pmd
> > driver. Patch creates ioport-mem device name /dev/igb_ioport.
> virtio_ethdev_init
> > function to open that device file for once, mappes 4K page_size memory
> such that
> > all entry in cat /proc/ioport {all PCI_IOBAR entry} mapped for once. For
> that
> > two ancessor api;s
> > - virtio_map_ioport : Maps all cat /proc/ioport pci iobars.
> > - virtio_set_ioport_addr : manages the each pci_iobar slot as an offset.
> >
> > Tested for thunderX/arm64 platform, by creating maximum guest kernel
> supported
> > virtio-net-pci device i.e.. 31 max, then attaching all 32 interface to
> uio,
> > Verified with tespmd io_fwd application.
> >
> > Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> > Signed-off-by: Rizwan Ansari <ransari@mvista.com>
> > ---
>
> Is it possible to rework patch a bit to minimise number of #ifdef ARM ...
> spread across the code?
> Group arch related code into separate file(s), use union for fields with
> sizes, etc?
> Konstantin
>
>
Sure. Thanks.


> >  drivers/net/virtio/virtio_ethdev.c        |  138
> ++++++++++++++++++++++++++++-
> >  lib/librte_eal/linuxapp/igb_uio/igb_uio.c |   80 ++++++++++++++++-
> >  2 files changed, 214 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> > index 74c00ee..93b8784 100644
> > --- a/drivers/net/virtio/virtio_ethdev.c
> > +++ b/drivers/net/virtio/virtio_ethdev.c
> > @@ -63,6 +63,30 @@
> >  #include "virtqueue.h"
> >  #include "virtio_rxtx.h"
> >
> > +#ifdef RTE_EXEC_ENV_LINUXAPP
> > +/* start address of first pci_iobar slot (user-space virtual-addres) */
> > +void *ioport_map;
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +
> > +#include <sys/mman.h>
> > +#define DEV_NAME             "/dev/igb_ioport"
> > +
> > +/* Keeping pci_ioport_size = 4k.
> > + * So maximum mmaped pci_iobar supported =
> > + * (ioport_size/pci_dev->mem_resource[0].len)
> > + *
> > + * note: kernel could allow maximum 32 virtio-net-pci interface, that
> mean
> > + * maximum 32 PCI_IOBAR(s) where each PCI_IOBAR_LEN=0x20, so
> virtio_map_ioport()
> > + * func by theory gonna support 4k/0x20 ==> 128 PCI_IOBAR(s), more than
> > + * max-virtio-net-pci interface.
> > + */
> > +#define PAGE_SIZE            4096
> > +#define PCI_IOPORT_SIZE              PAGE_SIZE
> > +#define PCI_IOPORT_MAX               128 /* 4k / 0x20 */
> > +
> > +int ioport_map_cnt;
> > +#endif /* ARM, ARM64 */
> > +#endif /* RTE_EXEC_ENV_LINUXAPP */
> >
> >  static int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
> >  static int eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev);
> > @@ -497,6 +521,18 @@ virtio_dev_close(struct rte_eth_dev *dev)
> >       hw->started = 0;
> >       virtio_dev_free_mbufs(dev);
> >       virtio_free_queues(dev);
> > +
> > +#ifdef RTE_EXEC_ENV_LINUXAPP
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +
> > +     /* unmap ioport memory */
> > +     ioport_map_cnt--;
> > +     if (!ioport_map_cnt)
> > +             munmap(ioport_map, PCI_IOPORT_SIZE);
> > +
> > +     PMD_INIT_LOG(DEBUG, "unmapping ioport_mem %d\n", ioport_map_cnt);
> > +#endif
> > +#endif
> >  }
> >
> >  static void
> > @@ -1172,7 +1208,6 @@ static int virtio_resource_init_by_ioports(struct
> rte_pci_device *pci_dev)
> >
> >                       sscanf(ptr, "%04hx-%04hx", &start, &end);
> >                       size = end - start + 1;
> > -
> >                       break;
> >               }
> >       }
> > @@ -1256,6 +1291,81 @@ rx_func_get(struct rte_eth_dev *eth_dev)
> >               eth_dev->rx_pkt_burst = &virtio_recv_pkts;
> >  }
> >
> > +#ifdef RTE_EXEC_ENV_LINUXAPP
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +
> > +static int
> > +virtio_map_ioport(void **resource_addr)
> > +{
> > +     int fd;
> > +     int ret = 0;
> > +
> > +     fd = open(DEV_NAME, O_RDWR);
> > +     if (fd < 0) {
> > +             PMD_INIT_LOG(ERR, "device file %s open error: %d\n",
> > +                          DEV_NAME, fd);
> > +             ret = -1;
> > +             goto out;
> > +     }
> > +
> > +     ioport_map = mmap(NULL, PCI_IOPORT_SIZE,
> > +                     PROT_EXEC | PROT_WRITE | PROT_READ, MAP_SHARED,
> fd, 0);
> > +
> > +     if (ioport_map == MAP_FAILED) {
> > +             PMD_INIT_LOG(ERR, "mmap: failed to map bar Address=%p\n",
> > +                             *resource_addr);
> > +             ret = -ENOMEM;
> > +             goto out1;
> > +     }
> > +
> > +     PMD_INIT_LOG(INFO, "First pci_iobar mapped at %p\n", ioport_map);
> > +
> > +out1:
> > +     close(fd);
> > +out:
> > +     return ret;
> > +}
> > +
> > +static int
> > +virtio_set_ioport_addr(void **resource_addr, unsigned long offset)
> > +{
> > +     int ret = 0;
> > +
> > +     if (ioport_map_cnt >= PCI_IOPORT_MAX) {
> > +             ret = -1;
> > +             PMD_INIT_LOG(ERR,
> > +                          "ioport_map_cnt(%d) greater than"
> > +                          "PCI_IOPORT_MAX(%d)\n",
> > +                          ioport_map_cnt, PCI_IOPORT_MAX);
> > +             goto out;
> > +     }
> > +     *resource_addr = (void *)((char *)ioport_map +
> (ioport_map_cnt)*offset);
> > +     ioport_map_cnt++;
> > +
> > +     PMD_INIT_LOG(DEBUG, "pci.resource_addr %p ioport_map_cnt %d\n",
> > +                     *resource_addr, ioport_map_cnt);
> > +out:
> > +     return ret;
> > +}
> > +#else /* !ARM, !ARM64 */
> > +static int
> > +virtio_map_ioport(void *resource_addr)
> > +{
> > +     (void)resource_addr;
> > +     return 0;
> > +}
> > +
> > +static int
> > +virtio_set_ioport_addr(void *resource_addr, unsigned long offset)
> > +{
> > +     (void)resource_addr;
> > +     (void)offset;
> > +     return 0;
> > +}
> > +
> > +#endif /* ARM, ARM64 */
> > +#endif /* RTE_EXEC_ENV_LINUXAPP */
> > +
> >  /*
> >   * This function is based on probe() function in virtio_pci.c
> >   * It returns 0 on success.
> > @@ -1294,8 +1404,31 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
> >       if (virtio_resource_init(pci_dev) < 0)
> >               return -1;
> >
> > +#ifdef       RTE_EXEC_ENV_LINUXAPP
> > +     int ret = 0;
> > +
> > +     /* Map the all IOBAR entry from /proc/ioport to 4k page_size only
> once.
> > +      * Later virtio_set_ioport_addr() func will update correct bar_addr
> > +      * eachk ioport (i.e..pci_dev->mem_resource[0].addr)
> > +      */
> > +     if (!ioport_map) {
> > +             ret = virtio_map_ioport(&pci_dev->mem_resource[0].addr);
> > +             if (ret < 0)
> > +                     return -1;
> > +     }
> > +
> > +     ret = virtio_set_ioport_addr(&pci_dev->mem_resource[0].addr,
> > +                                     pci_dev->mem_resource[0].len);
> > +     if (ret < 0)
> > +             return -1;
> > +
> > +     PMD_INIT_LOG(INFO, "ioport_map %p resource_addr %p resource_len
> :%ld\n",
> > +                     ioport_map, pci_dev->mem_resource[0].addr,
> > +                     (unsigned long)pci_dev->mem_resource[0].len);
> > +
> > +#endif
> >       hw->use_msix = virtio_has_msix(&pci_dev->addr);
> > -     hw->io_base = (uint32_t)(uintptr_t)pci_dev->mem_resource[0].addr;
> > +     hw->io_base = (unsigned
> long)(uintptr_t)pci_dev->mem_resource[0].addr;
> >
> >       /* Reset the device although not necessary at startup */
> >       vtpci_reset(hw);
> > @@ -1430,7 +1563,6 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
> >               rte_intr_callback_unregister(&pci_dev->intr_handle,
> >                                               virtio_interrupt_handler,
> >                                               eth_dev);
> > -
> >       PMD_INIT_LOG(DEBUG, "dev_uninit completed");
> >
> >       return 0;
> > diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > index f5617d2..b154a44 100644
> > --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > @@ -324,6 +324,61 @@ igbuio_dom0_pci_mmap(struct uio_info *info, struct
> vm_area_struct *vma)
> >  }
> >  #endif
> >
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +#ifdef CONFIG_HAS_IOPORT_MAP
> > +/*
> > + * mmap driver to map x86-style PCI_IOBAR (i.e..cat /proc/ioport
> pci-bar-memory)
> > + * from kernel-space virtual address to user-space virtual address.
> This module
> > + * required for non-x86 archs example arm/arm64, as those archs donot do
> > + * IO_MAP_IO types access, Infact supports memory-mapped-IO. That is
> because
> > + * arm/arm64 doesn't support direct IO instruction, so the design
> approach is to
> > + * map `cat /proc/ioport` PCI_IOBAR's kernel-space virtual-addr to
> user-space
> > + * virtual-addr. Therefore the need for mmap-driver.
> > + */
> > +#include <linux/fs.h>                /* file_operations */
> > +#include <linux/miscdevice.h>
> > +#include <linux/mm.h>                /* VM_IO */
> > +#include <linux/uaccess.h>
> > +#include <asm/page.h>
> > +#include <linux/sched.h>
> > +#include <linux/pid.h>
> > +
> > +void *__iomem mapped_io; /* ioport addr of `cat /proc/ioport` */
> > +
> > +static int igb_ioport_mmap(struct file *file, struct vm_area_struct
> *vma)
> > +{
> > +     struct page *npage;
> > +     int ret = 0;
> > +
> > +     vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> > +     npage = vmalloc_to_page(mapped_io);
> > +     ret = remap_pfn_range(vma, vma->vm_start,
> > +                             page_to_pfn(npage),
> > +                             vma->vm_end - vma->vm_start,
> > +                             vma->vm_page_prot);
> > +     if (ret) {
> > +             pr_info("Error: Failed to remap pfn=%lu error=%d\n",
> > +                     page_to_pfn(npage), ret);
> > +     }
> > +     return 0;
> > +}
> > +
> > +static const struct file_operations igb_ioport_fops = {
> > +     .mmap           = igb_ioport_mmap,
> > +};
> > +
> > +static struct miscdevice igb_ioport_dev = {
> > +     .minor  = MISC_DYNAMIC_MINOR,
> > +     .name   = "igb_ioport",
> > +     .fops   = &igb_ioport_fops
> > +};
> > +#else /* !CONFIG_HAS_IOPORT_MAP */
> > +
> > +#error "CONFIG_HAS_IOPORT_MAP not supported for $RTE_ARCH"
> > +
> > +#endif /* CONFIG_HAS_IOPORT_MAP */
> > +#endif /* RTE_ARCH_ARM, RTE_ARCH_ARM64 */
> > +
> >  /* Remap pci resources described by bar #pci_bar in uio resource n. */
> >  static int
> >  igbuio_pci_setup_iomem(struct pci_dev *dev, struct uio_info *info,
> > @@ -365,11 +420,22 @@ igbuio_pci_setup_ioport(struct pci_dev *dev,
> struct uio_info *info,
> >       if (addr == 0 || len == 0)
> >               return -EINVAL;
> >
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +     /*
> > +      * TODO: KNI/procfs:: porttype entry need to be added for
> ARM/ARM64,
> > +      * for below mmap driver;
> > +      * */
> > +     mapped_io = ioport_map(addr, 0);
> > +     info->port[n].name = name;
> > +     info->port[n].start = (unsigned long)(uintptr_t)mapped_io;
> > +     info->port[n].size = len;
> > +     info->port[n].porttype = UIO_PORT_X86;
> > +#else
> >       info->port[n].name = name;
> >       info->port[n].start = addr;
> >       info->port[n].size = len;
> >       info->port[n].porttype = UIO_PORT_X86;
> > -
> > +#endif
> >       return 0;
> >  }
> >
> > @@ -615,6 +681,15 @@ igbuio_pci_init_module(void)
> >  {
> >       int ret;
> >
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +     ret = misc_register(&igb_ioport_dev);
> > +     if (ret < 0) {
> > +             pr_info("Error: failed to register ioport map driver
> (%d)\n",
> > +                             ret);
> > +             return ret;
> > +     }
> > +#endif
> > +
> >       ret = igbuio_config_intr_mode(intr_mode);
> >       if (ret < 0)
> >               return ret;
> > @@ -625,6 +700,9 @@ igbuio_pci_init_module(void)
> >  static void __exit
> >  igbuio_pci_exit_module(void)
> >  {
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +     misc_deregister(&igb_ioport_dev);
> > +#endif
> >       pci_unregister_driver(&igbuio_pci_driver);
> >  }
> >
> > --
> > 1.7.9.5
>
>