From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Blunck Subject: Re: [PATCH v11 09/13] pci: add bus driver Date: Wed, 15 Feb 2017 11:42:47 +0100 Message-ID: References: <1484748329-5418-1-git-send-email-shreyansh.jain@nxp.com> <1484801117-779-1-git-send-email-thomas.monjalon@6wind.com> <1484801117-779-10-git-send-email-thomas.monjalon@6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Cc: Shreyansh Jain , dev To: Thomas Monjalon Return-path: Received: from mail-wr0-f194.google.com (mail-wr0-f194.google.com [209.85.128.194]) by dpdk.org (Postfix) with ESMTP id 176B8A2F for ; Wed, 15 Feb 2017 11:42:48 +0100 (CET) Received: by mail-wr0-f194.google.com with SMTP id c4so5178297wrd.1 for ; Wed, 15 Feb 2017 02:42:48 -0800 (PST) In-Reply-To: <1484801117-779-10-git-send-email-thomas.monjalon@6wind.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Thu, Jan 19, 2017 at 5:45 AM, Thomas Monjalon wrote: > From: Shreyansh Jain > > Based on EAL Bus APIs, PCI bus callbacks and support functions are > introduced in this patch. > > EAL continues to have direct PCI init/scan calls as well. These would be > removed in subsequent patches to enable bus only PCI devices. > > Signed-off-by: Shreyansh Jain > Reviewed-by: Ferruh Yigit > Signed-off-by: Thomas Monjalon > --- > lib/librte_eal/bsdapp/eal/eal.c | 1 + > lib/librte_eal/bsdapp/eal/eal_pci.c | 11 ++++++ > lib/librte_eal/common/eal_common_pci.c | 25 +++++++++++++ > lib/librte_eal/common/include/rte_pci.h | 65 +++++++++++++++++++++++++++++++++ > lib/librte_eal/linuxapp/eal/eal_pci.c | 12 ++++++ > 5 files changed, 114 insertions(+) > > diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c > index 534aeea..ee7c9de 100644 > --- a/lib/librte_eal/bsdapp/eal/eal.c > +++ b/lib/librte_eal/bsdapp/eal/eal.c > @@ -64,6 +64,7 @@ > #include > #include > #include > +#include > #include > #include > #include > diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c > index 3a5c315..48bfe24 100644 > --- a/lib/librte_eal/bsdapp/eal/eal_pci.c > +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c > @@ -673,3 +673,14 @@ rte_eal_pci_init(void) > } > return 0; > } > + > +struct rte_pci_bus rte_pci_bus = { > + .bus = { > + .scan = rte_eal_pci_scan, > + .probe = rte_eal_pci_probe, > + }, > + .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), > + .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list), > +}; > + > +RTE_REGISTER_BUS(PCI_BUS_NAME, rte_pci_bus.bus); > diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c > index 8b4ae2d..c53e2bd 100644 > --- a/lib/librte_eal/common/eal_common_pci.c > +++ b/lib/librte_eal/common/eal_common_pci.c > @@ -71,6 +71,7 @@ > > #include > #include > +#include > #include > #include > #include > @@ -87,6 +88,8 @@ struct pci_driver_list pci_driver_list = > struct pci_device_list pci_device_list = > TAILQ_HEAD_INITIALIZER(pci_device_list); > > +extern struct rte_pci_bus rte_pci_bus; > + > #define SYSFS_PCI_DEVICES "/sys/bus/pci/devices" > > const char *pci_get_sysfs_path(void) > @@ -479,3 +482,25 @@ rte_eal_pci_unregister(struct rte_pci_driver *driver) > rte_eal_driver_unregister(&driver->driver); > TAILQ_REMOVE(&pci_driver_list, driver, next); > } > + > +/* Add a device to PCI bus */ > +void > +rte_eal_pci_add_device(struct rte_pci_device *pci_dev) > +{ > + TAILQ_INSERT_TAIL(&rte_pci_bus.device_list, pci_dev, next); > +} > + > +/* Insert a device into a predefined position in PCI bus */ > +void > +rte_eal_pci_insert_device(struct rte_pci_device *exist_pci_dev, > + struct rte_pci_device *new_pci_dev) > +{ > + TAILQ_INSERT_BEFORE(exist_pci_dev, new_pci_dev, next); > +} > + > +/* Remove a device from PCI bus */ > +void > +rte_eal_pci_remove_device(struct rte_pci_device *pci_dev) > +{ > + TAILQ_REMOVE(&rte_pci_bus.device_list, pci_dev, next); > +} > diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h > index adc20b9..957cddb 100644 > --- a/lib/librte_eal/common/include/rte_pci.h > +++ b/lib/librte_eal/common/include/rte_pci.h > @@ -85,6 +85,7 @@ extern "C" { > #include > #include > #include > +#include > > TAILQ_HEAD(pci_device_list, rte_pci_device); /**< PCI devices in D-linked Q. */ > TAILQ_HEAD(pci_driver_list, rte_pci_driver); /**< PCI drivers in D-linked Q. */ > @@ -111,6 +112,25 @@ const char *pci_get_sysfs_path(void); > /** Maximum number of PCI resources. */ > #define PCI_MAX_RESOURCE 6 > > +/** Name of PCI Bus */ > +#define PCI_BUS_NAME "PCI" > + > +/* Forward declarations */ > +struct rte_pci_device; > +struct rte_pci_driver; > + > +/** List of PCI devices */ > +TAILQ_HEAD(rte_pci_device_list, rte_pci_device); > +/** List of PCI drivers */ > +TAILQ_HEAD(rte_pci_driver_list, rte_pci_driver); > + > +/* PCI Bus iterators */ > +#define FOREACH_DEVICE_ON_PCIBUS(p) \ > + TAILQ_FOREACH(p, &(rte_pci_bus.device_list), next) > + > +#define FOREACH_DRIVER_ON_PCIBUS(p) \ > + TAILQ_FOREACH(p, &(rte_pci_bus.driver_list), next) > + > /** > * A structure describing an ID for a PCI driver. Each driver provides a > * table of these IDs for each device that it supports. > @@ -206,12 +226,22 @@ typedef int (pci_remove_t)(struct rte_pci_device *); > struct rte_pci_driver { > TAILQ_ENTRY(rte_pci_driver) next; /**< Next in list. */ > struct rte_driver driver; /**< Inherit core driver. */ > + struct rte_pci_bus *bus; /**< PCI bus reference. */ > pci_probe_t *probe; /**< Device Probe function. */ > pci_remove_t *remove; /**< Device Remove function. */ > const struct rte_pci_id *id_table; /**< ID table, NULL terminated. */ > uint32_t drv_flags; /**< Flags contolling handling of device. */ > }; > > +/** > + * Structure describing the PCI bus > + */ > +struct rte_pci_bus { > + struct rte_bus bus; /**< Inherit the generic class */ > + struct rte_pci_device_list device_list; /**< List of PCI devices */ > + struct rte_pci_driver_list driver_list; /**< List of PCI drivers */ > +}; > + > /** Device needs PCI BAR mapping (done with either IGB_UIO or VFIO) */ > #define RTE_PCI_DRV_NEED_MAPPING 0x0001 > /** Device driver supports link state interrupt */ > @@ -523,6 +553,41 @@ RTE_PMD_EXPORT_NAME(nm, __COUNTER__) > void rte_eal_pci_unregister(struct rte_pci_driver *driver); > > /** > + * Add a PCI device to the PCI Bus (append to PCI Device list). This function > + * also updates the bus references of the PCI Device (and the generic device > + * object embedded within. > + * > + * @param pci_dev > + * PCI device to add > + * @return void > + */ > +void rte_eal_pci_add_device(struct rte_pci_device *pci_dev); > + Who would be the user of this? >>From my understanding a device will show up on the bus if: 1. bus->scan() finds it 2. bus->attach(devargs) explicitly adds it to the whitelist Both methods shouldn't require us to expose the API outside of eal. > +/** > + * Insert a PCI device in the PCI Bus at a particular location in the device > + * list. It also updates the PCI Bus reference of the new devices to be > + * inserted. > + * > + * @param exist_pci_dev > + * Existing PCI device in PCI Bus > + * @param new_pci_dev > + * PCI device to be added before exist_pci_dev > + * @return void > + */ > +void rte_eal_pci_insert_device(struct rte_pci_device *exist_pci_dev, > + struct rte_pci_device *new_pci_dev); > + > +/** > + * Remove a PCI device from the PCI Bus. This sets to NULL the bus references > + * in the PCI device object as well as the generic device object. > + * > + * @param pci_device > + * PCI device to be removed from PCI Bus > + * @return void > + */ > +void rte_eal_pci_remove_device(struct rte_pci_device *pci_device); > + > +/** > * Read PCI config space. > * > * @param device > diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c > index e2fc219..c40814d 100644 > --- a/lib/librte_eal/linuxapp/eal/eal_pci.c > +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c > @@ -35,6 +35,7 @@ > #include > > #include > +#include > #include > #include > #include > @@ -723,3 +724,14 @@ rte_eal_pci_init(void) > > return 0; > } > + > +struct rte_pci_bus rte_pci_bus = { > + .bus = { > + .scan = rte_eal_pci_scan, > + .probe = rte_eal_pci_probe, > + }, > + .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), > + .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list), I don't see why this is necessary and I believe it makes people think we might want to model topology. It's better to keep it simple and have it locally in eal_common_pci.c.