[dpdk-dev] [PATCH v11 12/13] pci: use bus driver for attach/detach

Jan Blunck jblunck at infradead.org
Wed Feb 15 12:08:38 CET 2017


On Thu, Jan 19, 2017 at 5:45 AM, Thomas Monjalon
<thomas.monjalon at 6wind.com> wrote:
> From: Shreyansh Jain <shreyansh.jain at nxp.com>
>
> Given a bus, attach and detach callbacks allow the implementation to
> handles calls from EAL for attaching or detaching a named device.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain at nxp.com>
> Reviewed-by: Ferruh Yigit <ferruh.yigit at intel.com>
> ---
>  lib/librte_eal/bsdapp/eal/eal_pci.c     |  2 +
>  lib/librte_eal/common/eal_common_dev.c  | 56 ++++++++++++++++-----
>  lib/librte_eal/common/eal_common_pci.c  | 89 +++++++++++++++++++++++++++++++--
>  lib/librte_eal/common/include/rte_bus.h | 31 ++++++++++++
>  lib/librte_eal/common/include/rte_pci.h | 45 +++++++++++------
>  lib/librte_eal/linuxapp/eal/eal_pci.c   |  2 +
>  6 files changed, 193 insertions(+), 32 deletions(-)
>
> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
> index 103ad70..38a75af 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> @@ -670,6 +670,8 @@ struct rte_pci_bus rte_pci_bus = {
>         .bus = {
>                 .scan = rte_eal_pci_scan,
>                 .probe = rte_eal_pci_probe,
> +               .attach = rte_eal_pci_attach,
> +               .detach = rte_eal_pci_detach,
>         },
>         .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list),
>         .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
> diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
> index 4f3b493..97d0cf5 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -38,6 +38,7 @@
>  #include <sys/queue.h>
>
>  #include <rte_dev.h>
> +#include <rte_bus.h>
>  #include <rte_devargs.h>
>  #include <rte_debug.h>
>  #include <rte_devargs.h>
> @@ -106,23 +107,37 @@ rte_eal_dev_init(void)
>
>  int rte_eal_dev_attach(const char *name, const char *devargs)
>  {
> -       struct rte_pci_addr addr;
> +       int ret = 1;
> +       struct rte_bus *bus;
>
>         if (name == NULL || devargs == NULL) {
>                 RTE_LOG(ERR, EAL, "Invalid device or arguments provided\n");
>                 return -EINVAL;
>         }
>
> -       if (eal_parse_pci_DomBDF(name, &addr) == 0) {
> -               if (rte_eal_pci_probe_one(&addr) < 0)
> +       FOREACH_BUS(bus) {
> +               if (!bus->attach) {
> +                       RTE_LOG(DEBUG, EAL, "Bus (%s) doesn't implement"
> +                               " attach.\n", bus->name);
> +                       continue;
> +               }
> +               ret = bus->attach(name);

Enforcing a globally unique naming scheme for the low-level device
makes this complicated for users. There are buses that enumerate the
devices just by an integer so we will have conflicts.

I think it is better to change the signature of rte_eal_dev_attach()
instead and find the correct bus based on its name. Also I believe the
API is more user friendly if we pass the complete (raw) devargs string
instead of the name/args pair.


> +               if (!ret) /* device successfully attached */
> +                       return ret;
> +               if (ret > 0) /* device not found on bus */
> +                       continue;
> +               else
>                         goto err;
> +       }
>
> -       } else {
> -               if (rte_eal_vdev_init(name, devargs))
> +       if (ret > 0) {
> +               /* In case the device was not found on any bus, search VDEV */
> +               ret = rte_eal_vdev_init(name, devargs);
> +               if (ret)
>                         goto err;
>         }
>
> -       return 0;
> +       return ret;
>
>  err:
>         RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n", name);
> @@ -131,21 +146,38 @@ int rte_eal_dev_attach(const char *name, const char *devargs)
>
>  int rte_eal_dev_detach(const char *name)
>  {
> -       struct rte_pci_addr addr;
> +       int ret = 1;
> +       struct rte_bus *bus;
>
>         if (name == NULL) {
>                 RTE_LOG(ERR, EAL, "Invalid device provided.\n");
>                 return -EINVAL;
>         }
>
> -       if (eal_parse_pci_DomBDF(name, &addr) == 0) {
> -               if (rte_eal_pci_detach(&addr) < 0)
> +       FOREACH_BUS(bus) {
> +               if (!bus->detach) {
> +                       RTE_LOG(DEBUG, EAL, "Bus (%s) doesn't implement"
> +                               " detach.\n", bus->name);
> +                       continue;
> +               }
> +
> +               ret = bus->detach(name);
> +               if (!ret) /* device successfully detached */
> +                       return ret;
> +               if (ret > 0) /* device not found on the bus */
> +                       continue;
> +               else
>                         goto err;
> -       } else {
> -               if (rte_eal_vdev_uninit(name))
> +       }
> +
> +       if (ret > 0) {
> +               /* In case the device was not found on any bus, search VDEV */
> +               ret = rte_eal_vdev_uninit(name);
> +               if (ret)
>                         goto err;
>         }
> -       return 0;
> +
> +       return ret;
>
>  err:
>         RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n", name);
> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
> index 5149e6e..6866b04 100644
> --- a/lib/librte_eal/common/eal_common_pci.c
> +++ b/lib/librte_eal/common/eal_common_pci.c
> @@ -358,19 +358,98 @@ rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
>  }
>
>  /*
> + * Attach device specific by its name
> + */
> +int
> +rte_eal_pci_attach(const char *device_name)
> +{
> +       struct rte_pci_device *dev = NULL;
> +       struct rte_pci_driver *drv = NULL;
> +       struct rte_pci_addr addr;
> +       int ret;
> +
> +       if (!device_name)
> +               return -1;
> +
> +       memset(&addr, 0, sizeof(addr));
> +       if (eal_parse_pci_DomBDF(device_name, &addr)) {
> +               /* Device doesn't match PCI BDF naming format */
> +               return 1;
> +       }
> +
> +       if (pci_update_device(&addr) < 0)
> +               goto err_return;
> +
> +       FOREACH_DEVICE_ON_PCIBUS(dev) {
> +               if (rte_eal_compare_pci_addr(&dev->addr, &addr))
> +                       continue;
> +
> +               FOREACH_DRIVER_ON_PCIBUS(drv) {
> +                       ret = rte_pci_match(drv, dev);
> +                       if (ret) {
> +                               /* Match of device and driver failed */
> +                               RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match"
> +                                       " the device (%s)\n", drv->driver.name,
> +                                       device_name);
> +                               continue;
> +                       }
> +
> +                       RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n",
> +                               dev->id.vendor_id, dev->id.device_id,
> +                               drv->driver.name);
> +
> +                       if (drv->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
> +                               /* map resources for devices that use
> +                                * igb_uio
> +                                */
> +                               ret = rte_eal_pci_map_device(dev);
> +                               if (ret != 0)
> +                                       goto err_return;
> +                       }
> +
> +                       /* reference driver structure */
> +                       dev->driver = drv;
> +
> +                       /* call the driver probe() function */
> +                       ret = drv->probe(drv, dev);
> +                       if (ret) {
> +                               dev->driver = NULL;
> +                               if (drv->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
> +                                       rte_eal_pci_unmap_device(dev);
> +                       }
> +                       return ret;
> +               }
> +       }
> +
> +       return 1;
> +err_return:
> +       RTE_LOG(WARNING, EAL, "Requested device " PCI_PRI_FMT
> +                       " cannot be used\n", dev->addr.domain, dev->addr.bus,
> +                       dev->addr.devid, dev->addr.function);
> +       return -1;
> +}
> +
> +/*
>   * Detach device specified by its pci address.
>   */
>  int
> -rte_eal_pci_detach(const struct rte_pci_addr *addr)
> +rte_eal_pci_detach(const char *device_name)
>  {
>         struct rte_pci_device *dev = NULL;
> -       int ret = 0;
> +       struct rte_pci_addr addr;
> +       int ret = 1;
>
> -       if (addr == NULL)
> +       if (!device_name)
>                 return -1;
>
> +       memset(&addr, 0, sizeof(addr));
> +       if (eal_parse_pci_DomBDF(device_name, &addr)) {
> +               /* Device doesn't match PCI BDF naming format */
> +               return 1;
> +       }
> +
>         FOREACH_DEVICE_ON_PCIBUS(dev) {
> -               if (rte_eal_compare_pci_addr(&dev->addr, addr))
> +               if (rte_eal_compare_pci_addr(&dev->addr, &addr))
>                         continue;
>
>                 ret = rte_eal_pci_detach_dev(dev);
> @@ -385,8 +464,8 @@ rte_eal_pci_detach(const struct rte_pci_addr *addr)
>                 free(dev);
>                 return 0;
>         }
> -       return -1;
>
> +       return ret;
>  err_return:
>         RTE_LOG(WARNING, EAL, "Requested device " PCI_PRI_FMT
>                         " cannot be used\n", dev->addr.domain, dev->addr.bus,
> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> index 19954f4..9a096ec 100644
> --- a/lib/librte_eal/common/include/rte_bus.h
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -59,6 +59,8 @@ TAILQ_HEAD(rte_bus_list, rte_bus);
>  /* Bus list exposed */
>  extern struct rte_bus_list rte_bus_list;
>
> +#define FOREACH_BUS(bus_p)     TAILQ_FOREACH(bus_p, &rte_bus_list, next)
> +
>  /**
>   * Bus specific scan for devices attached on the bus.
>   * For each bus object, the scan would be reponsible for finding devices and
> @@ -85,6 +87,33 @@ typedef int (*rte_bus_scan_t)(void);
>  typedef int (*rte_bus_probe_t)(void);
>
>  /**
> + * Attach a device to a bus, assuming it is 'connected' to the bus.
> + * A bus is responsible for scanning for devices. Attaching a new device is
> + * for reenabling the device after being detached/removed.
> + *
> + * @param device_name
> + *     Name of the device to attach.
> + *
> + * @return
> + *     0 for successful attach
> + *     !0 for unsuccessful attach (or incorrect device name)
> + */
> +typedef int (*rte_bus_attach_t)(const char *device_name);
> +
> +/**
> + * Detach a named device from a bus. Implementation would check the existence
> + * of device on the bus and detach it.
> + *
> + * @param device_name
> + *     Name of the device to detach
> + *
> + * @return
> + *     0 for successful detaching
> + *     !0 if device not found or can't detach
> + */
> +typedef int (*rte_bus_detach_t)(const char *device_name);
> +
> +/**
>   * A structure describing a generic bus.
>   */
>  struct rte_bus {
> @@ -92,6 +121,8 @@ struct rte_bus {
>         const char *name;            /**< Name of the bus */
>         rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
>         rte_bus_probe_t probe;       /**< Probe devices on bus */
> +       rte_bus_attach_t attach;     /**< Attach a named device */
> +       rte_bus_detach_t detach;     /**< Detach a named device */
>  };
>
>  /**
> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
> index bab8df4..215d732 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -418,6 +418,36 @@ int
>  rte_eal_pci_probe(void);
>
>  /**
> + * Search and attach a PCI device to PCI Bus
> + * Implements rte_bus->attach
> + *
> + * @param device_name
> + *     Name of the device to search and attach
> + *
> + * @return
> + *     0 for successful removal of device
> + *     >0 if device not found on bus
> + *     <0 in case of error in removal.
> + */
> +int
> +rte_eal_pci_attach(const char *device_name);
> +
> +/**
> + * Search and detach a PCI device from PCI Bus
> + * Implements rte_bus->detach
> + *
> + * @param device_name
> + *     Name of the device to search and detach
> + *
> + * @return
> + *     0 for successful detaching of device
> + *     >0 if device not found on bus
> + *     <0 in case of error in removal.
> + */
> +int
> +rte_eal_pci_detach(const char *device_name);
> +
> +/**
>   * Map the PCI device resources in user space virtual memory address
>   *
>   * Note that driver should not call this function when flag
> @@ -491,21 +521,6 @@ void pci_unmap_resource(void *requested_addr, size_t size);
>  int rte_eal_pci_probe_one(const struct rte_pci_addr *addr);
>
>  /**
> - * Close the single PCI device.
> - *
> - * Scan the content of the PCI bus, and find the pci device specified by pci
> - * address, then call the remove() function for registered driver that has a
> - * matching entry in its id_table for discovered device.
> - *
> - * @param addr
> - *     The PCI Bus-Device-Function address to close.
> - * @return
> - *   - 0 on success.
> - *   - Negative on error.
> - */
> -int rte_eal_pci_detach(const struct rte_pci_addr *addr);
> -
> -/**
>   * Dump the content of the PCI bus.
>   *
>   * @param f
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index d6f9855..01f0c9a 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -719,6 +719,8 @@ struct rte_pci_bus rte_pci_bus = {
>         .bus = {
>                 .scan = rte_eal_pci_scan,
>                 .probe = rte_eal_pci_probe,
> +               .attach = rte_eal_pci_attach,
> +               .detach = rte_eal_pci_detach,
>         },
>         .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list),
>         .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
> --
> 2.7.0
>


More information about the dev mailing list