Hey,
On Mon, Sep 18, 2017 at 05:23:23PM +0530, Shreyansh Jain wrote:
> Hello Gaetan,
>
> On Monday 18 September 2017 03:01 PM, Gaetan Rivet wrote:
> >The PCI lib defines the types and methods allowing to use PCI elements.
> >
> >The PCI bus implements a bus driver for PCI devices by constructing
> >rte_bus elements using the PCI lib.
> >
> >Move the relevant code out of the EAL to their expected place.
> >
> >Signed-off-by: Gaetan Rivet <gaetan.ri...@6wind.com>
> >---
> > config/common_base | 10 +
> > drivers/bus/Makefile | 2 +
> > drivers/bus/pci/Makefile | 59 ++
> > drivers/bus/pci/bsd/Makefile | 32 ++
> > drivers/bus/pci/bsd/rte_pci.c | 670
> > ++++++++++++++++++++++
> > drivers/bus/pci/include/rte_bus_pci.h | 387 +++++++++++++
> > drivers/bus/pci/linux/Makefile | 37 ++
> > drivers/bus/pci/linux/rte_pci.c | 722
> > ++++++++++++++++++++++++
> > drivers/bus/pci/linux/rte_pci_init.h | 97 ++++
> > drivers/bus/pci/linux/rte_pci_uio.c | 567 +++++++++++++++++++
> > drivers/bus/pci/linux/rte_pci_vfio.c | 674
> > ++++++++++++++++++++++
> > drivers/bus/pci/linux/rte_vfio_mp_sync.c | 424 ++++++++++++++
> > drivers/bus/pci/private.h | 173 ++++++
> > drivers/bus/pci/rte_bus_pci_version.map | 21 +
> > drivers/bus/pci/rte_pci_common.c | 542 ++++++++++++++++++
> > drivers/bus/pci/rte_pci_common_uio.c | 234 ++++++++
> > lib/Makefile | 2 +
> > lib/librte_eal/bsdapp/eal/Makefile | 3 -
> > lib/librte_eal/bsdapp/eal/eal_pci.c | 670
> > ----------------------
> > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 15 -
> > lib/librte_eal/common/Makefile | 2 +-
> > lib/librte_eal/common/eal_common_pci.c | 580 -------------------
> > lib/librte_eal/common/eal_common_pci_uio.c | 233 --------
> > lib/librte_eal/common/include/rte_pci.h | 598 --------------------
> > lib/librte_eal/linuxapp/eal/Makefile | 10 -
> > lib/librte_eal/linuxapp/eal/eal_pci.c | 722
> > ------------------------
> > lib/librte_eal/linuxapp/eal/eal_pci_init.h | 97 ----
> > lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 567 -------------------
> > lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 674
> > ----------------------
> > lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 424 --------------
> > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 15 -
> > lib/librte_ether/rte_ethdev.h | 2 -
> > lib/librte_pci/Makefile | 48 ++
> > lib/librte_pci/include/rte_pci.h | 279 +++++++++
> > lib/librte_pci/rte_pci.c | 92 +++
> > lib/librte_pci/rte_pci_version.map | 8 +
> > mk/rte.app.mk | 3 +
> > 37 files changed, 5084 insertions(+), 4611 deletions(-)
> > create mode 100644 drivers/bus/pci/Makefile
> > create mode 100644 drivers/bus/pci/bsd/Makefile
> > create mode 100644 drivers/bus/pci/bsd/rte_pci.c
> > create mode 100644 drivers/bus/pci/include/rte_bus_pci.h
> > create mode 100644 drivers/bus/pci/linux/Makefile
> > create mode 100644 drivers/bus/pci/linux/rte_pci.c
> > create mode 100644 drivers/bus/pci/linux/rte_pci_init.h
> > create mode 100644 drivers/bus/pci/linux/rte_pci_uio.c
> > create mode 100644 drivers/bus/pci/linux/rte_pci_vfio.c
> > create mode 100644 drivers/bus/pci/linux/rte_vfio_mp_sync.c
> > create mode 100644 drivers/bus/pci/private.h
> > create mode 100644 drivers/bus/pci/rte_bus_pci_version.map
> > create mode 100644 drivers/bus/pci/rte_pci_common.c
> > create mode 100644 drivers/bus/pci/rte_pci_common_uio.c
> > delete mode 100644 lib/librte_eal/bsdapp/eal/eal_pci.c
> > delete mode 100644 lib/librte_eal/common/eal_common_pci.c
> > delete mode 100644 lib/librte_eal/common/eal_common_pci_uio.c
> > delete mode 100644 lib/librte_eal/common/include/rte_pci.h
> > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_pci.c
> > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_init.h
> > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c
> > create mode 100644 lib/librte_pci/Makefile
> > create mode 100644 lib/librte_pci/include/rte_pci.h
> > create mode 100644 lib/librte_pci/rte_pci.c
> > create mode 100644 lib/librte_pci/rte_pci_version.map
> >
>
> <lot of snip here...>
>
> >+#endif /* _PCI_PRIVATE_H_ */
> >diff --git a/drivers/bus/pci/rte_bus_pci_version.map
> >b/drivers/bus/pci/rte_bus_pci_version.map
> >new file mode 100644
> >index 0000000..eca49e9
> >--- /dev/null
> >+++ b/drivers/bus/pci/rte_bus_pci_version.map
> >@@ -0,0 +1,21 @@
> >+DPDK_17.08 {
>
> You might want to bump this to 17.11.
>
Thanks, fixing this.
> >+ global:
> >+
> >+ rte_pci_detach;
> >+ rte_pci_dump;
> >+ rte_pci_ioport_map;
> >+ rte_pci_ioport_read;
> >+ rte_pci_ioport_unmap;
> >+ rte_pci_ioport_write;
> >+ rte_pci_map_device;
> >+ rte_pci_probe;
> >+ rte_pci_probe_one;
> >+ rte_pci_read_config;
> >+ rte_pci_register;
> >+ rte_pci_scan;
> >+ rte_pci_unmap_device;
> >+ rte_pci_unregister;
> >+ rte_pci_write_config;
> >+
> >+ local: *;
> >+};
>
> This is huuuge patch :( and I am not yet through it (most of it is movement
> so I doubt anything major would be problem here).
> Just the above comment in case you are spinning a new series.
>
>
Thanks for reading the patch.
Yes, most of it is moving the code as-is to a new location.
I tried to reduce it, but at some point it does not really make sense
anymore.
I think the important thing to look for here is the build system,
dependency graph and the division of the PCI API between the lib and the
bus driver.
I divided it along the lines of the rte_pci_device being defined or not.
Anything using an rte_pci_device going to the bus and everything
else going to the lib.
I'm mostly worried about this divide. Having the rte_pci_device defined
seems mostly of the responsibility of the bus driver, in my opinion. I'd
like to hear others'.
--
Gaëtan Rivet
6WIND
