Hello Thomas, On Wednesday 09 November 2016 03:47 PM, Thomas Monjalon wrote: > Hi Shreyansh, > > I realize that we had a lot of off-list discussions on this topic > and there was no public explanation of the status of this series.
Thanks for your email. (and all the suggestions you have given offline/IRC etc.) I was beginning to wonder that probably only Jan and me were the ones interested in this. Ironically, I felt that being EAL changes, a lot of people would come and be critic of it - giving an opportunity to get it widely accepted. > > 2016-10-28 17:56, Shreyansh Jain: > [...] >> As of now EAL is primarly focused on PCI initialization/probing. > > Right. DPDK was PCI centric. > We must give PCI its right role: a bus as other ones. > A first step was done in 16.11 (thanks to you Shreyansh, Jan and David) > to have a better device model. > The next step is to rework the bus abstraction. Or, this change can be broken into multiple steps: 1. Create a PCI parallel layer for non-PCI devices. (call it SoC, or whatever else - doesn't really matter. More below). 2. Generalize this 'soc' changeset into common 3. Complete generalization by introducing a Linux like model of bus<=>device<=>driver. Which was what my current approach was - expecting that SoC patchset would allow me to introduce NXP PMD and parallel to it I can keep pushing EAL generic changes towards generic bus arch. > > It seems a bus can be defined with the properties scan/match/notify, > leading to initialize the devices. 'notify' is something which I am not completely clear with - but, in principle, agree. > > More comments below your technical presentation. > > [...] >> This patchset introduces SoC framework which would enable SoC drivers and >> drivers to be plugged into EAL, very similar to how PCI drivers/devices are >> done today. >> >> This is a stripped down version of PCI framework which allows the SoC PMDs >> to implement their own routines for detecting devices and linking devices to >> drivers. >> >> 1) Changes to EAL >> rte_eal_init() >> |- rte_eal_pci_init(): Find PCI devices from sysfs >> |- rte_eal_soc_init(): Calls PMDs->scan_fn >> |- ... >> |- rte_eal_memzone_init() >> |- ... >> |- rte_eal_pci_probe(): Driver<=>Device initialization, PMD->devinit() >> `- rte_eal_soc_probe(): Calls PMDs->match_fn and PMDs->devinit(); >> >> 2) New device/driver structures: >> - rte_soc_driver (inheriting rte_driver) >> - rte_soc_device (inheriting rte_device) >> - rte_eth_dev and eth_driver embedded rte_soc_device and rte_soc_driver, >> respectively. >> >> 3) The SoC PMDs need to: >> - define rte_soc_driver with necessary scan and match callbacks >> - Register themselves using DRIVER_REGISTER_SOC() >> - Implement respective bus scanning in the scan callbacks to add necessary >> devices to SoC device list >> - Implement necessary eth_dev_init/uninint for ethernet instances > > These callbacks are not specific to a SoC. Agree; this is just a note - it exactly what a PCI PMD does. > By the way a SoC defines nothing specific. You are using the SoC word > as an equivalent of non-PCI. Yes, primarily because SoC is a very broad word which can encompass a variety of devices (buses/drivers). So, we have PCI set, VDEV set, and everything else represented by 'SoC'. The complete set is based on this principle: to have a generic subsystem _parallel_ to PCI (so as not to impact it) which can represent all devices not already included in PCI set'. - Actually, it is _nothing_ specific. > We must have a bus abstraction like the one you are defining for the SoC > but it must be generic and defined on top of PCI, so we can plug any > bus in it: PCI, vdev (as a software bus), any other well-defined bus, > and some driver-specific bus which can be implemented directly in the > driver (the NXP case AFAIK). Indeed - that is an ideal approach. And honestly, true attribution, it is not my original idea. It is yours, from our IRC discussion. And it was ironic as well because Declan came up with similar suggestion much earlier but no one commented on it. > >> 4) Design considerations that are same as PCI: >> - SoC initialization is being done through rte_eal_init(), just after PCI >> initialization is done. >> - As in case of PCI, probe is done after rte_eal_pci_probe() to link the >> devices detected with the drivers registered. >> - Device attach/detach functions are available and have been designed on >> the lines of PCI framework. >> - PMDs register using DRIVER_REGISTER_SOC, very similar to >> DRIVER_REGISTER_PCI for PCI devices. >> - Linked list of SoC driver and devices exists independent of the other >> driver/device list, but inheriting rte_driver/rte_driver, these are >> also part of a global list. >> >> 5) Design considerations that are different from PCI: >> - Each driver implements its own scan and match function. PCI uses the BDF >> format to read the device from sysfs, but this _may_not_ be a case for a >> SoC ethernet device. >> = This is an important change from initial proposal by Jan in [2]. >> Unlike his attempt to use /sys/bus/platform, this patch relies on the >> PMD to detect the devices. This is because SoC may require specific or >> additional info for device detection. Further, SoC may have embedded >> devices/MACs which require initialization which cannot be covered >> through sysfs parsing. >> `-> Point (6) below is a side note to above. >> = PCI based PMDs rely on EAL's capability to detect devices. This >> proposal puts the onus on PMD to detect devices, add to soc_device_list >> and wait for Probe. Matching, of device<=>driver is again PMD's >> callback. > > These PCI considerations can be described as a PCI bus implementation in EAL. Yes. Or, we can continue as it is for PCI and allow more buses to plug in. And eventually, make the whole bus thing generic. Step-by-Step. > >> 6) Adding default scan and match helpers for PMDs >> - The design warrrants the PMDs implement their own scan of devices >> on bus, and match routines for probe implementation. >> This patch introduces helpers which can be used by PMDs for scan of >> the platform bus and matching devices against the compatible string >> extracted from the scan. >> - Intention is to make it easier to integrate known SoC which expose >> platform bus compliant information (compat, sys/bus/platform...). >> - PMDs which have deviations from this standard model can implement and >> hook their bus scanning and probe match callbacks while registering >> driver. > > Yes we can have EAL helpers to scan sys/bus/platform or other common formats. > Then the PMD can use them to implement its specific bus. And: - Framework/Developers should be able to 'register/unregister' buses. -- Just like drivers/net -- So, probably a drivers/bus/pci, drivers/bus/xxx folder structure -- and compatible compilation routine -- Or maybe librte_bus? --- I still haven't got around solving this. - RTE_PMD_REGISTER_BUS probably. - just EAL helpers are not enough - or at least, not generic enough. - Drivers should be able to 'lookup' buses -- PCI PMDs find a 'pci' bus during registration, for example. -- Unlike Linux, a device tree doesn't exist to solve this issue of which PMD is connected to which bus. This needs to solved using explicit APIs -- Once found, drivers can 'associate' with a bus. - Not just bus<->device<->driver, there is pending work with respect ethernet devices. -- Removing eth_driver->pci_driver linkage. Making it generic. -- Multiple places assume eth device to be PCI, rewriting that part. -- device_insert changes for vdev > > I know that David, you and me are on the same page to agree on this generic > design. I hope you will be able to achieve this work soon. I agree with your suggestion in principle - bus model is nice to have. There is always a better design. It is about how to achieve it. I still think that current SoC patchset is a decent first step without completely discounting any future changes. (Obviously, assuming it is not NACK'd by someone because of some currently unforeseen technical argument). Because: 1. It doesn't break the existing PCI subsystem anywhere. It is disabled by default. 2. It is independent - in the sense that changes are limited to SoC specific files, with only a few changes in EAL. But, clean patches make it easier to validate. 3. It would allow more people to check/validate the overall proposal of non-PCI device existence. 4. It paves way for cleaner transit to Bus architecture - rte_device/driver inheritance is complete after SoC patchset. Except changes in eth_dev/driver which is a completely parallel change with little overlap (but wide impact) - rte_soc_driver introduces 'scan/match' which can then be generalized to rte_driver in next step. -- This is very similar to what happened in rte_device/driver change where PCI things were generalized to common. > The goal is to be able to plug a SoC driver in DPDK 17.02 with a clean design. I am already working on that - now on a higher priority. Hopefully within next few days I will send a RFC out. There are still some grey ares for me (lack of DPDK internal understand, probably) - but I am hoping I would get good support from others - at least this time. Then, probably for 17.02, we can have either candidate - and we can integrate whatever looks best (in terms of design and impact, both). > My advice to make reviews and contributions easier, is to split the work in > few steps: > - clean PCI code (generalize non-PCI stuff) > - add generic bus functions > - plug PCI in generic bus abstraction > - plug vdev in generic bus abstraction > - plug the new NXP driver I agree with above. > > Thanks > Thank you for your time and suggestions. - Shreyansh
