Re: [PATCH v8 12/12] eal: Consolidate rte_eal_pci_probe/close_one_driver() of linuxapp and bsdapp

[David Marchand <david.marchand@6wind.com>]

On Mon, Jul 6, 2015 at 8:24 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:

> From: "Tetsuya.Mukawa" <mukawa@igel.co.jp>
>
> This patch consolidates below functions, and implements these in common
> eal code.
>  - rte_eal_pci_probe_one_driver()
>  - rte_eal_pci_close_one_driver()
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> ---
>  lib/librte_eal/bsdapp/eal/eal_pci.c    |  67 +--------------
>  lib/librte_eal/common/eal_common_pci.c | 133
> +++++++++++++++++++++++++++++-
>  lib/librte_eal/common/eal_private.h    |  39 +--------
>  lib/librte_eal/linuxapp/eal/eal_pci.c  | 143
> +--------------------------------
>  4 files changed, 136 insertions(+), 246 deletions(-)
>
> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c
> b/lib/librte_eal/bsdapp/eal/eal_pci.c
> index a27dca3..b7c34f4 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> @@ -85,7 +85,7 @@
>   */
>
>  /* unbind kernel driver for this device */
> -static int
> +int
>  pci_unbind_kernel_driver(struct rte_pci_device *dev __rte_unused)
>  {
>         RTE_LOG(ERR, EAL, "RTE_PCI_DRV_FORCE_UNBIND flag is not
> implemented "
> @@ -357,71 +357,6 @@ error:
>         return -1;
>  }
>
> -/*
> - * If vendor/device ID match, call the devinit() function of the
> - * driver.
> - */
> -int
> -rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct
> rte_pci_device *dev)
> -{
> -       const struct rte_pci_id *id_table;
> -       int ret;
> -
> -       for (id_table = dr->id_table ; id_table->vendor_id != 0;
> id_table++) {
> -
> -               /* check if device's identifiers match the driver's ones */
> -               if (id_table->vendor_id != dev->id.vendor_id &&
> -                               id_table->vendor_id != PCI_ANY_ID)
> -                       continue;
> -               if (id_table->device_id != dev->id.device_id &&
> -                               id_table->device_id != PCI_ANY_ID)
> -                       continue;
> -               if (id_table->subsystem_vendor_id !=
> dev->id.subsystem_vendor_id &&
> -                               id_table->subsystem_vendor_id !=
> PCI_ANY_ID)
> -                       continue;
> -               if (id_table->subsystem_device_id !=
> dev->id.subsystem_device_id &&
> -                               id_table->subsystem_device_id !=
> PCI_ANY_ID)
> -                       continue;
> -
> -               struct rte_pci_addr *loc = &dev->addr;
> -
> -               RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA
> socket %i\n",
> -                               loc->domain, loc->bus, loc->devid,
> loc->function,
> -                               dev->numa_node);
> -
> -               RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n",
> dev->id.vendor_id,
> -                               dev->id.device_id, dr->name);
> -
> -               /* no initialization when blacklisted, return without
> error */
> -               if (dev->devargs != NULL &&
> -                       dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI)
> {
> -
> -                       RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not
> initializing\n");
> -                       return 0;
> -               }
> -
> -               if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
> -                       /* map resources for devices that use igb_uio */
> -                       ret = pci_uio_map_resource(dev);
> -                       if (ret != 0)
> -                               return ret;
> -               } else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
> -                          rte_eal_process_type() == RTE_PROC_PRIMARY) {
> -                       /* unbind current driver */
> -                       if (pci_unbind_kernel_driver(dev) < 0)
> -                               return -1;
> -               }
> -
> -               /* reference driver structure */
> -               dev->driver = dr;
> -
> -               /* call the driver devinit() function */
> -               return dr->devinit(dr, dev);
> -       }
> -       /* return positive value if driver is not found */
> -       return 1;
> -}
> -
>  /* Init the PCI EAL subsystem */
>  int
>  rte_eal_pci_init(void)
> diff --git a/lib/librte_eal/common/eal_common_pci.c
> b/lib/librte_eal/common/eal_common_pci.c
> index afd14ec..031c7a7 100644
> --- a/lib/librte_eal/common/eal_common_pci.c
> +++ b/lib/librte_eal/common/eal_common_pci.c
> @@ -139,7 +139,7 @@ pci_unmap_resource(void *requested_addr, size_t size)
>  }
>
>  /* Map pci device */
> -int
> +static int
>  pci_map_device(struct rte_pci_device *dev)
>  {
>         int ret = -1;
> @@ -170,7 +170,7 @@ pci_map_device(struct rte_pci_device *dev)
>
>  #ifdef RTE_LIBRTE_EAL_HOTPLUG
>  /* Unmap pci device */
> -void
> +static void
>  pci_unmap_device(struct rte_pci_device *dev)
>  {
>         if (dev == NULL)
> @@ -196,6 +196,135 @@ pci_unmap_device(struct rte_pci_device *dev)
>  #endif /* RTE_LIBRTE_EAL_HOTPLUG */
>
>  /*
> + * If vendor/device ID match, call the devinit() function of the
> + * driver.
> + */
> +static int
> +rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct
> rte_pci_device *dev)
> +{
> +       int ret;
> +       const struct rte_pci_id *id_table;
> +
> +       for (id_table = dr->id_table; id_table->vendor_id != 0;
> id_table++) {
> +
> +               /* check if device's identifiers match the driver's ones */
> +               if (id_table->vendor_id != dev->id.vendor_id &&
> +                               id_table->vendor_id != PCI_ANY_ID)
> +                       continue;
> +               if (id_table->device_id != dev->id.device_id &&
> +                               id_table->device_id != PCI_ANY_ID)
> +                       continue;
> +               if (id_table->subsystem_vendor_id !=
> dev->id.subsystem_vendor_id &&
> +                               id_table->subsystem_vendor_id !=
> PCI_ANY_ID)
> +                       continue;
> +               if (id_table->subsystem_device_id !=
> dev->id.subsystem_device_id &&
> +                               id_table->subsystem_device_id !=
> PCI_ANY_ID)
> +                       continue;
> +
> +               struct rte_pci_addr *loc = &dev->addr;
> +
> +               RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA
> socket %i\n",
> +                               loc->domain, loc->bus, loc->devid,
> loc->function,
> +                               dev->numa_node);
> +
> +               RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n",
> dev->id.vendor_id,
> +                               dev->id.device_id, dr->name);
> +
> +               /* no initialization when blacklisted, return without
> error */
> +               if (dev->devargs != NULL &&
> +                       dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI)
> {
> +                       RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not
> initializing\n");
> +                       return 1;
> +               }
> +
> +               if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
> +#ifdef RTE_PCI_CONFIG
> +                       /*
> +                        * Set PCIe config space for high performance.
> +                        * Return value can be ignored.
> +                        */
> +                       pci_config_space_set(dev);
> +#endif
> +                       /* map resources for devices that use igb_uio */
> +                       ret = pci_map_device(dev);
> +                       if (ret != 0)
> +                               return ret;
> +               } else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
> +                               rte_eal_process_type() ==
> RTE_PROC_PRIMARY) {
> +                       /* unbind current driver */
> +                       if (pci_unbind_kernel_driver(dev) < 0)
> +                               return -1;
> +               }
> +
> +               /* reference driver structure */
> +               dev->driver = dr;
> +
> +               /* call the driver devinit() function */
> +               return dr->devinit(dr, dev);
> +       }
> +       /* return positive value if driver is not found */
> +       return 1;
> +}
> +
> +#ifdef RTE_LIBRTE_EAL_HOTPLUG
> +/*
> + * If vendor/device ID match, call the devuninit() function of the
> + * driver.
> + */
> +static int
> +rte_eal_pci_close_one_driver(struct rte_pci_driver *dr,
> +               struct rte_pci_device *dev)
> +{
> +       const struct rte_pci_id *id_table;
> +
> +       if ((dr == NULL) || (dev == NULL))
> +               return -EINVAL;
> +
> +       for (id_table = dr->id_table; id_table->vendor_id != 0;
> id_table++) {
> +
> +               /* check if device's identifiers match the driver's ones */
> +               if (id_table->vendor_id != dev->id.vendor_id &&
> +                               id_table->vendor_id != PCI_ANY_ID)
> +                       continue;
> +               if (id_table->device_id != dev->id.device_id &&
> +                               id_table->device_id != PCI_ANY_ID)
> +                       continue;
> +               if (id_table->subsystem_vendor_id !=
> dev->id.subsystem_vendor_id &&
> +                               id_table->subsystem_vendor_id !=
> PCI_ANY_ID)
> +                       continue;
> +               if (id_table->subsystem_device_id !=
> dev->id.subsystem_device_id &&
> +                               id_table->subsystem_device_id !=
> PCI_ANY_ID)
> +                       continue;
> +
> +               struct rte_pci_addr *loc = &dev->addr;
> +
> +               RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA
> socket %i\n",
> +                               loc->domain, loc->bus, loc->devid,
> +                               loc->function, dev->numa_node);
> +
> +               RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n",
> dev->id.vendor_id,
> +                               dev->id.device_id, dr->name);
> +
> +               /* call the driver devuninit() function */
> +               if (dr->devuninit && (dr->devuninit(dev) < 0))
> +                       return -1;      /* negative value is an error */
> +
> +               /* clear driver structure */
> +               dev->driver = NULL;
> +
> +               if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
> +                       /* unmap resources for devices that use igb_uio */
> +                       pci_unmap_device(dev);
> +
> +               return 0;
> +       }
> +
> +       /* return positive value if driver is not found */
> +       return 1;
> +}
> +#endif /* RTE_LIBRTE_EAL_HOTPLUG */
> +
> +/*
>   * If vendor/device ID match, call the devinit() function of all
>   * registered driver for the given device. Return -1 if initialization
>   * failed, return 1 if no driver is found for this device.
> diff --git a/lib/librte_eal/common/eal_private.h
> b/lib/librte_eal/common/eal_private.h
> index 95b9e30..c3659e2 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -156,23 +156,14 @@ struct rte_pci_driver;
>  struct rte_pci_device;
>
>  /**
> - * Map this device
> + * Unbind kernel driver for this device
>   *
>   * This function is private to EAL.
>   *
>   * @return
>   *   0 on success, negative on error
>   */
> -int pci_map_device(struct rte_pci_device *dev);
> -
> -#ifdef RTE_LIBRTE_EAL_HOTPLUG
> -/**
> - * Unmap this device
> - *
> - * This function is private to EAL.
> - */
> -void pci_unmap_device(struct rte_pci_device *dev);
> -#endif /* RTE_LIBRTE_EAL_HOTPLUG */
> +int pci_unbind_kernel_driver(struct rte_pci_device *dev);
>
>  /**
>   * Map the PCI resource of a PCI device in virtual memory
> @@ -194,32 +185,6 @@ void pci_uio_unmap_resource(struct rte_pci_device
> *dev);
>  #endif /* RTE_LIBRTE_EAL_HOTPLUG */
>
>  /**
> - * Mmap memory for single PCI device
> - *
> - * This function is private to EAL.
> - *
> - * @return
> - *   0 on success, negative on error
> - */
> -int rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
> -               struct rte_pci_device *dev);
> -
> -/**
> - * Munmap memory for single PCI device
> - *
> - * This function is private to EAL.
> - *
> - * @param      dr
> - *  The pointer to the pci driver structure
> - * @param      dev
> - *  The pointer to the pci device structure
> - * @return
> - *   0 on success, negative on error
> - */
> -int rte_eal_pci_close_one_driver(struct rte_pci_driver *dr,
> -               struct rte_pci_device *dev);
> -
> -/**
>   * Allocate uio resource for PCI device
>   *
>   * This function is private to EAL.
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c
> b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index 7e8df7d..ad6d1de 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -2,6 +2,7 @@
>   *   BSD LICENSE
>   *
>   *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> + *   Copyright(c) 2015 IGEL Co., Ltd.
>   *   All rights reserved.
>   *
>   *   Redistribution and use in source and binary forms, with or without
> @@ -56,7 +57,7 @@
>   */
>
>  /* unbind kernel driver for this device */
> -static int
> +int
>  pci_unbind_kernel_driver(struct rte_pci_device *dev)
>  {
>         int n;
> @@ -522,146 +523,6 @@ pci_config_space_set(struct rte_pci_device *dev)
>  }
>  #endif
>
> -/*
> - * If vendor/device ID match, call the devinit() function of the
> - * driver.
> - */
> -int
> -rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct
> rte_pci_device *dev)
> -{
> -       int ret;
> -       const struct rte_pci_id *id_table;
> -
> -       for (id_table = dr->id_table; id_table->vendor_id != 0;
> id_table++) {
> -
> -               /* check if device's identifiers match the driver's ones */
> -               if (id_table->vendor_id != dev->id.vendor_id &&
> -                               id_table->vendor_id != PCI_ANY_ID)
> -                       continue;
> -               if (id_table->device_id != dev->id.device_id &&
> -                               id_table->device_id != PCI_ANY_ID)
> -                       continue;
> -               if (id_table->subsystem_vendor_id !=
> dev->id.subsystem_vendor_id &&
> -                               id_table->subsystem_vendor_id !=
> PCI_ANY_ID)
> -                       continue;
> -               if (id_table->subsystem_device_id !=
> dev->id.subsystem_device_id &&
> -                               id_table->subsystem_device_id !=
> PCI_ANY_ID)
> -                       continue;
> -
> -               struct rte_pci_addr *loc = &dev->addr;
> -
> -               RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA
> socket %i\n",
> -                               loc->domain, loc->bus, loc->devid,
> loc->function,
> -                               dev->numa_node);
> -
> -               RTE_LOG(DEBUG, EAL, "  probe driver: %x:%x %s\n",
> dev->id.vendor_id,
> -                               dev->id.device_id, dr->name);
> -
> -               /* no initialization when blacklisted, return without
> error */
> -               if (dev->devargs != NULL &&
> -                       dev->devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI)
> {
> -                       RTE_LOG(DEBUG, EAL, "  Device is blacklisted, not
> initializing\n");
> -                       return 1;
> -               }
> -
> -               if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
> -#ifdef RTE_PCI_CONFIG
> -                       /*
> -                        * Set PCIe config space for high performance.
> -                        * Return value can be ignored.
> -                        */
> -                       pci_config_space_set(dev);
> -#endif
> -                       /* map resources for devices that use igb_uio */
> -                       ret = pci_map_device(dev);
> -                       if (ret != 0)
> -                               return ret;
> -               } else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
> -                          rte_eal_process_type() == RTE_PROC_PRIMARY) {
> -                       /* unbind current driver */
> -                       if (pci_unbind_kernel_driver(dev) < 0)
> -                               return -1;
> -               }
> -
> -               /* reference driver structure */
> -               dev->driver = dr;
> -
> -               /* call the driver devinit() function */
> -               return dr->devinit(dr, dev);
> -       }
> -       /* return positive value if driver is not found */
> -       return 1;
> -}
> -
> -#ifdef RTE_LIBRTE_EAL_HOTPLUG
> -/*
> - * If vendor/device ID match, call the devuninit() function of the
> - * driver.
> - */
> -int
> -rte_eal_pci_close_one_driver(struct rte_pci_driver *dr,
> -               struct rte_pci_device *dev)
> -{
> -       const struct rte_pci_id *id_table;
> -
> -       if ((dr == NULL) || (dev == NULL))
> -               return -EINVAL;
> -
> -       for (id_table = dr->id_table; id_table->vendor_id != 0;
> id_table++) {
> -
> -               /* check if device's identifiers match the driver's ones */
> -               if (id_table->vendor_id != dev->id.vendor_id &&
> -                   id_table->vendor_id != PCI_ANY_ID)
> -                       continue;
> -               if (id_table->device_id != dev->id.device_id &&
> -                   id_table->device_id != PCI_ANY_ID)
> -                       continue;
> -               if (id_table->subsystem_vendor_id !=
> -                   dev->id.subsystem_vendor_id &&
> -                   id_table->subsystem_vendor_id != PCI_ANY_ID)
> -                       continue;
> -               if (id_table->subsystem_device_id !=
> -                   dev->id.subsystem_device_id &&
> -                   id_table->subsystem_device_id != PCI_ANY_ID)
> -                       continue;
> -
> -               struct rte_pci_addr *loc = &dev->addr;
> -
> -               RTE_LOG(DEBUG, EAL,
> -                               "PCI device "PCI_PRI_FMT" on NUMA socket
> %i\n",
> -                               loc->domain, loc->bus, loc->devid,
> -                               loc->function, dev->numa_node);
> -
> -               RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n",
> -                               dev->id.vendor_id, dev->id.device_id,
> -                               dr->name);
> -
> -               /* call the driver devuninit() function */
> -               if (dr->devuninit && (dr->devuninit(dev) < 0))
> -                       return -1;      /* negative value is an error */
> -
> -               /* clear driver structure */
> -               dev->driver = NULL;
> -
> -               if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
> -                       /* unmap resources for devices that use igb_uio */
> -                       pci_unmap_device(dev);
> -
> -               return 0;
> -       }
> -       /* return positive value if driver is not found */
> -       return 1;
> -}
> -#else /* RTE_LIBRTE_EAL_HOTPLUG */
> -int
> -rte_eal_pci_close_one_driver(struct rte_pci_driver *dr __rte_unused,
> -               struct rte_pci_device *dev __rte_unused)
> -{
> -       RTE_LOG(ERR, EAL, "Hotplug support isn't enabled\n");
> -       return -1;
> -}
> -#endif /* RTE_LIBRTE_EAL_HOTPLUG */
> -
>  /* Init the PCI EAL subsystem */
>  int
>  rte_eal_pci_init(void)
> --
> 2.1.4
>
>

Acked-by: David Marchand <david.marchand@6wind.com>

-- 
David Marchand