Re: [PATCH v3 02/12] eal/bus: introduce bus abstraction

[Jan Blunck <jblunck@infradead.org>]

On Fri, Dec 16, 2016 at 2:10 PM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> This patch introduces the rte_bus abstraction for devices and drivers in
> EAL framework. The model is:
>  - One or more buses are connected to a CPU (or core)
>  - One or more devices are conneted to a Bus
>  - Drivers are running instances which manage one or more devices
>  - Bus is responsible for identifying devices (and interrupt propogation)
>  - Driver is responsible for initializing the device
>
> This patch adds a 'rte_bus' class which rte_driver and rte_device refer.
> This way, each device (rte_xxx_device) would have reference to the bus
> it is based on. As well as, each driver (rte_xxx_driver) would have link
> to the bus and devices on it for servicing.
>
>                                   __ rte_bus_list
>                                  /
>                      +----------'---+
>                      |rte_bus       |
>                      | driver_list------> List of rte_bus specific
>                      | device_list----    devices
>                      |              | `-> List of rte_bus associated
>                      |              |     drivers
>                      +--|------|----+
>               _________/        \_________
>     +--------/----+                     +-\---------------+
>     |rte_device   |                     |rte_driver       |
>     | rte_bus     |                     | rte_bus         |
>     | rte_driver  |                     | ...             |
>     | ...         |                     +---------...-----+
>     |             |                               |||
>     +---||--------+                               |||
>         ||                                        |||
>         | \                                        \\\
>         |  \_____________                           \\\
>         |                \                          |||
>  +------|---------+ +----|----------+               |||
>  |rte_pci_device  | |rte_xxx_device |               |||
>  | ....           | | ....          |               |||
>  +----------------+ +---------------+              / | \
>                                                   /  |  \
>                             _____________________/  /    \
>                            /                    ___/      \
>             +-------------'--+    +------------'---+    +--'------------+
>             |rte_pci_driver  |    |rte_vdev_driver |    |rte_xxx_driver |
>             | ....           |    | ....           |    | ....          |
>             +----------------+    +----------------+    +---------------+
>
> This patch only enables the bus references on rte_driver and rte_driver.
> EAL wide global device and driver list continue to exist until an instance
> of bus is added in subsequent patches.
>
> This patch also introduces RTE_REGISTER_BUS macro on the lines of
> RTE_PMD_REGISTER_XXX. Key difference is that the constructor priority has
> been explicitly set to 101 so as to execute bus registration before PMD.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>
> --
> v2:
>  - fix bsdapp compilation issue because of missing export symbols in map
>    file
> ---
>  lib/librte_eal/bsdapp/eal/Makefile              |   1 +
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  15 ++
>  lib/librte_eal/common/Makefile                  |   2 +-
>  lib/librte_eal/common/eal_common_bus.c          | 192 ++++++++++++++++++++++++
>  lib/librte_eal/common/include/rte_bus.h         | 174 +++++++++++++++++++++
>  lib/librte_eal/common/include/rte_dev.h         |   2 +
>  lib/librte_eal/linuxapp/eal/Makefile            |   1 +
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  15 ++
>  8 files changed, 401 insertions(+), 1 deletion(-)
>  create mode 100644 lib/librte_eal/common/eal_common_bus.c
>  create mode 100644 lib/librte_eal/common/include/rte_bus.h
>
> diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
> index a15b762..cce99f7 100644
> --- a/lib/librte_eal/bsdapp/eal/Makefile
> +++ b/lib/librte_eal/bsdapp/eal/Makefile
> @@ -78,6 +78,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_cpuflags.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_string_fns.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_hexdump.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_devargs.c
> +SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_bus.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_dev.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_options.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_thread.c
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index 2f81f7c..23fc1c1 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -174,3 +174,18 @@ DPDK_16.11 {
>         rte_eal_vdrv_unregister;
>
>  } DPDK_16.07;
> +
> +DPDK_17.02 {
> +       global:
> +
> +       rte_bus_list;
> +       rte_eal_bus_add_device;
> +       rte_eal_bus_add_driver;
> +       rte_eal_get_bus;
> +       rte_eal_bus_dump;
> +       rte_eal_bus_register;
> +       rte_eal_bus_remove_device;
> +       rte_eal_bus_remove_driver;
> +       rte_eal_bus_unregister;
> +
> +} DPDK_16.11;
> diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
> index a92c984..0c39414 100644
> --- a/lib/librte_eal/common/Makefile
> +++ b/lib/librte_eal/common/Makefile
> @@ -38,7 +38,7 @@ INC += rte_per_lcore.h rte_random.h
>  INC += rte_tailq.h rte_interrupts.h rte_alarm.h
>  INC += rte_string_fns.h rte_version.h
>  INC += rte_eal_memconfig.h rte_malloc_heap.h
> -INC += rte_hexdump.h rte_devargs.h rte_dev.h rte_vdev.h
> +INC += rte_hexdump.h rte_devargs.h rte_bus.h rte_dev.h rte_vdev.h
>  INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
>  INC += rte_malloc.h rte_keepalive.h rte_time.h
>
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> new file mode 100644
> index 0000000..612f64e
> --- /dev/null
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -0,0 +1,192 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2016 NXP
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of NXP nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#include <stdio.h>
> +#include <string.h>
> +#include <inttypes.h>
> +#include <sys/queue.h>
> +
> +#include <rte_bus.h>
> +#include <rte_devargs.h>
> +#include <rte_debug.h>
> +
> +#include "eal_private.h"
> +
> +struct rte_bus_list rte_bus_list =
> +       TAILQ_HEAD_INITIALIZER(rte_bus_list);
> +
> +/** @internal
> + * Add a device to a bus.
> + */
> +void
> +rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev)
> +{
> +       RTE_VERIFY(bus);
> +       RTE_VERIFY(dev);
> +
> +       TAILQ_INSERT_TAIL(&bus->device_list, dev, next);
> +       dev->bus = bus;
> +}
> +
> +/** @internal
> + * Remove a device from its bus.
> + */
> +void
> +rte_eal_bus_remove_device(struct rte_device *dev)
> +{
> +       struct rte_bus *bus;
> +
> +       RTE_VERIFY(dev);
> +       RTE_VERIFY(dev->bus);
> +
> +       bus = dev->bus;
> +       TAILQ_REMOVE(&bus->device_list, dev, next);
> +       dev->bus = NULL;
> +}
> +
> +/** @internal
> + * Associate a driver with a bus.
> + */
> +void
> +rte_eal_bus_add_driver(struct rte_bus *bus, struct rte_driver *drv)
> +{
> +       RTE_VERIFY(bus);
> +       RTE_VERIFY(drv);
> +
> +       TAILQ_INSERT_TAIL(&bus->driver_list, drv, next);
> +       drv->bus = bus;
> +}
> +
> +/** @internal
> + * Disassociate a driver from bus.
> + */
> +void
> +rte_eal_bus_remove_driver(struct rte_driver *drv)
> +{
> +       struct rte_bus *bus;
> +
> +       RTE_VERIFY(drv);
> +       RTE_VERIFY(drv->bus);
> +
> +       bus = drv->bus;
> +       TAILQ_REMOVE(&bus->driver_list, drv, next);
> +       drv->bus = NULL;
> +}
> +
> +/**
> + * Get the bus handle using its name
> + */
> +struct rte_bus *
> +rte_eal_get_bus(const char *bus_name)
> +{
> +       struct rte_bus *bus;
> +
> +       RTE_VERIFY(bus_name);
> +
> +       TAILQ_FOREACH(bus, &rte_bus_list, next) {
> +               RTE_VERIFY(bus->name);
> +
> +               if (!strcmp(bus_name, bus->name)) {
> +                       RTE_LOG(DEBUG, EAL, "Returning Bus object %p\n", bus);
> +                       return bus;
> +               }
> +       }
> +
> +       /* Unable to find bus requested */
> +       return NULL;
> +}
> +
> +/* register a bus */
> +void
> +rte_eal_bus_register(struct rte_bus *bus)
> +{
> +       RTE_VERIFY(bus);
> +       RTE_VERIFY(bus->name && strlen(bus->name));
> +
> +       /* Initialize the driver and device list associated with the bus */
> +       TAILQ_INIT(&(bus->driver_list));
> +       TAILQ_INIT(&(bus->device_list));
> +
> +       TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
> +       RTE_LOG(INFO, EAL, "Registered [%s] bus.\n", bus->name);
> +}
> +
> +/* unregister a bus */
> +void
> +rte_eal_bus_unregister(struct rte_bus *bus)
> +{
> +       /* All devices and drivers associated with the bus should have been
> +        * 'device->uninit' and 'driver->remove()' already.
> +        */
> +       RTE_VERIFY(TAILQ_EMPTY(&(bus->driver_list)));
> +       RTE_VERIFY(TAILQ_EMPTY(&(bus->device_list)));
> +
> +       /* TODO: For each device, call its rte_device->driver->remove()
> +        * and rte_eal_bus_remove_driver()
> +        */
> +
> +       TAILQ_REMOVE(&rte_bus_list, bus, next);
> +       RTE_LOG(INFO, EAL, "Unregistered [%s] bus.\n", bus->name);
> +}
> +
> +/* dump one bus info */
> +static int
> +bus_dump_one(FILE *f, struct rte_bus *bus)
> +{
> +       int ret;
> +
> +       /* For now, dump only the bus name */
> +       ret = fprintf(f, " %s\n", bus->name);
> +
> +       /* Error in case of inability in writing to stream */
> +       if (ret < 0)
> +               return ret;
> +
> +       return 0;
> +}
> +
> +void
> +rte_eal_bus_dump(FILE *f)
> +{
> +       int ret;
> +       struct rte_bus *bus;
> +
> +       TAILQ_FOREACH(bus, &rte_bus_list, next) {
> +               ret = bus_dump_one(f, bus);
> +               if (ret) {
> +                       RTE_LOG(ERR, EAL, "Unable to write to stream (%d)\n",
> +                               ret);
> +                       break;
> +               }
> +       }
> +}
> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> new file mode 100644
> index 0000000..f0297a9
> --- /dev/null
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -0,0 +1,174 @@
> +/*-
> + *   BSD LICENSE
> + *
> + *   Copyright(c) 2016 NXP
> + *   All rights reserved.
> + *
> + *   Redistribution and use in source and binary forms, with or without
> + *   modification, are permitted provided that the following conditions
> + *   are met:
> + *
> + *     * Redistributions of source code must retain the above copyright
> + *       notice, this list of conditions and the following disclaimer.
> + *     * Redistributions in binary form must reproduce the above copyright
> + *       notice, this list of conditions and the following disclaimer in
> + *       the documentation and/or other materials provided with the
> + *       distribution.
> + *     * Neither the name of NXP nor the names of its
> + *       contributors may be used to endorse or promote products derived
> + *       from this software without specific prior written permission.
> + *
> + *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> + *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> + *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> + *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> + *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> + *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> + *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> + *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> + *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> + *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> + *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> + */
> +
> +#ifndef _RTE_BUS_H_
> +#define _RTE_BUS_H_
> +
> +/**
> + * @file
> + *
> + * RTE PMD Bus Abstraction interfaces
> + *
> + * This file exposes APIs and Interfaces for Bus Abstraction over the devices
> + * drivers in EAL.
> + */
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include <stdio.h>
> +#include <sys/queue.h>
> +
> +#include <rte_log.h>
> +#include <rte_dev.h>
> +
> +/** Double linked list of buses */
> +TAILQ_HEAD(rte_bus_list, rte_bus);
> +
> +/* Global Bus list */
> +extern struct rte_bus_list rte_bus_list;
> +
> +struct rte_bus {
> +       TAILQ_ENTRY(rte_bus) next;   /**< Next bus object in linked list */
> +       struct rte_driver_list driver_list;
> +                                    /**< List of all drivers on bus */
> +       struct rte_device_list device_list;
> +                                    /**< List of all devices on bus */
> +       const char *name;            /**< Name of the bus */
> +};
> +
> +/** @internal
> + * Add a device to a bus.
> + *
> + * @param bus
> + *     Bus on which device is to be added
> + * @param dev
> + *     Device handle
> + * @return
> + *     None
> + */
> +void
> +rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev);
> +
> +/** @internal
> + * Remove a device from its bus.
> + *
> + * @param dev
> + *     Device handle to remove
> + * @return
> + *     None
> + */
> +void
> +rte_eal_bus_remove_device(struct rte_device *dev);
> +
> +/** @internal
> + * Associate a driver with a bus.
> + *
> + * @param bus
> + *     Bus on which driver is to be added
> + * @param dev
> + *     Driver handle
> + * @return
> + *     None
> + */
> +void
> +rte_eal_bus_add_driver(struct rte_bus *bus, struct rte_driver *drv);
> +
> +/** @internal
> + * Disassociate a driver from its bus.
> + *
> + * @param dev
> + *     Driver handle to remove
> + * @return
> + *     None
> + */
> +void
> +rte_eal_bus_remove_driver(struct rte_driver *drv);
> +
> +/**
> + * Register a Bus handler.
> + *
> + * @param driver
> + *   A pointer to a rte_bus structure describing the bus
> + *   to be registered.
> + */
> +void rte_eal_bus_register(struct rte_bus *bus);
> +
> +/**
> + * Unregister a Bus handler.
> + *
> + * @param driver
> + *   A pointer to a rte_bus structure describing the bus
> + *   to be unregistered.
> + */
> +void rte_eal_bus_unregister(struct rte_bus *bus);
> +
> +/**
> + * Obtain handle for bus given its name.
> + *
> + * @param bus_name
> + *     Name of the bus handle to search
> + * @return
> + *     Pointer to Bus object if name matches any registered bus object
> + *     NULL, if no matching bus found
> + */
> +struct rte_bus *rte_eal_get_bus(const char *bus_name);
> +
> +/**
> + * Dump information of all the buses registered with EAL.
> + *
> + * @param f
> + *     A valid and open output stream handle
> + *
> + * @return
> + *      0 in case of success
> + *     !0 in case there is error in opening the output stream
> + */
> +void rte_eal_bus_dump(FILE *f);
> +
> +/** Helper for Bus registration. The constructor has higher priority than
> + * PMD constructors
> + */
> +#define RTE_REGISTER_BUS(nm, bus) \
> +static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \
> +{\
> +       (bus).name = RTE_STR(nm);\
> +       rte_eal_bus_register(&bus); \
> +}
> +
> +#ifdef __cplusplus
> +}
> +#endif
> +
> +#endif /* _RTE_BUS_H */
> diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
> index 8840380..4004f9a 100644
> --- a/lib/librte_eal/common/include/rte_dev.h
> +++ b/lib/librte_eal/common/include/rte_dev.h
> @@ -122,6 +122,7 @@ struct rte_driver;
>   */
>  struct rte_device {
>         TAILQ_ENTRY(rte_device) next; /**< Next device */
> +       struct rte_bus *bus;          /**< Device connected to this bus */

Is there a reason why this isn't const?


>         struct rte_driver *driver;    /**< Associated driver */
>         int numa_node;                /**< NUMA node connection */
>         struct rte_devargs *devargs;  /**< Device user arguments */
> @@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
>   */
>  struct rte_driver {
>         TAILQ_ENTRY(rte_driver) next;  /**< Next in list. */
> +       struct rte_bus *bus;           /**< Bus serviced by this driver */

Same thing here.

>         const char *name;                   /**< Driver name. */
>         const char *alias;              /**< Driver alias. */
>  };
> diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
> index 4e206f0..aa874a5 100644
> --- a/lib/librte_eal/linuxapp/eal/Makefile
> +++ b/lib/librte_eal/linuxapp/eal/Makefile
> @@ -87,6 +87,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_cpuflags.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_string_fns.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_hexdump.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_devargs.c
> +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_bus.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_dev.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_options.c
>  SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_thread.c
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index 83721ba..c873a7f 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -178,3 +178,18 @@ DPDK_16.11 {
>         rte_eal_vdrv_unregister;
>
>  } DPDK_16.07;
> +
> +DPDK_17.02 {
> +       global:
> +
> +       rte_bus_list;
> +       rte_eal_bus_add_device;
> +       rte_eal_bus_add_driver;
> +       rte_eal_get_bus;
> +       rte_eal_bus_dump;
> +       rte_eal_bus_register;
> +       rte_eal_bus_remove_device;
> +       rte_eal_bus_remove_driver;
> +       rte_eal_bus_unregister;
> +
> +} DPDK_16.11;
> --
> 2.7.4
>