[dpdk-dev] [PATCH v2 05/14] pci: introduce PCI lib and bus
Shreyansh Jain
shreyansh.jain at nxp.com
Mon Sep 18 14:18:59 CEST 2017
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.rivet at 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.
More information about the dev
mailing list