On Monday 18 September 2017 05:21 PM, Gaëtan Rivet wrote:
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.

Agree. It is difficult to manage such large movement with a compile-able patch series. I think compilation success takes priority over size in such cases so as not to break bisect.

Here, I found that "eal: remove references to PCI" onwards, the compilation was breaking. You have already mentioned about this in the cover letter, it seems.


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.

Agree. I am trying to look through this.


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.

Yes, I agree with this in principle.
rte_xxx_device and rte_xxx_driver originate from the xxx_bus. Bus should be responsible for defining (scan) xxx_devices and attaching (probe) to xxx_driver. Which in turn means that all APIs dealing with xxx_device and xxx_driver should be pivoted in the Bus layer.

Though, I think here the complexity is also of crypto devices. You have already reached out to Declan through cover letter - probably his (and other crypto experts) opinion would matter here.

My opinion would be to have


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'.


+1 from side for this principle for divide.

Reply via email to