[dpdk-dev] [PATCH V2 3/5] Add Intel FPGA BUS Lib Code

[Gaëtan Rivet]

On Wed, Mar 21, 2018 at 03:14:05PM +0100, Gaëtan Rivet wrote:
> Hi Bruce,
> 
> On Wed, Mar 21, 2018 at 01:35:09PM +0000, Bruce Richardson wrote:
> > On Wed, Mar 21, 2018 at 11:20:25AM +0100, Gaëtan Rivet wrote:
> > > Hi,
> > > 
> > > I have had issues compiling a few things here, have you checked
> > > build status before submitting?
> > > 
> > > On Wed, Mar 21, 2018 at 03:51:32PM +0800, Rosen Xu wrote:
> > > > Signed-off-by: Rosen Xu <rosen.xu at intel.com>
> > > > ---
> > <snip>
> > > +
> > > > +/*
> > > > + * Scan the content of the FPGA bus, and the devices in the devices
> > > > + * list
> > > > + */
> > > 
> > > So you seem to scan your bus by reading parameters
> > > given to the --ifpga EAL option.
> > > 
> > > Can you justify why you cannot use the PCI bus, have your FPGA be probed
> > > by a PCI driver, that would take those parameters as driver parameters,
> > > and spawn raw devices (one per bitstream) as needed as a result?
> > > 
> > > I see no reason this is not feasible. Unless you duly justify this
> > > approach, it seems unacceptable to me. You are subverting generic EAL
> > > code to bend things to your approach, without clear rationale.
> > > 
> > 
> > While I agree with the comments in other emails about avoiding
> > special-cases in the code that makes things not-scalable, I would take the
> > view that using a bus-type is the correct choice for this. While you could
> > have a single device that creates other devices, that is also true for all
> > other buses as well.  [Furthermore, I think it is incorrect assume that all
> > devices on the FPGA bus would be raw devices, it's entirely possible to
> > have cryptodevs, bbdevs or compress devs implemented in the AFUs].
> > 
> > Consider what a bus driver provides: it's a generic mechanism for scanning
> > for devices - which all use a common connection method - for DPDK use, and
> > mapping device drivers to those devices. For an FPGA device which presents
> > multiple AFUs, this seems to be exactly what is required - a device driver
> > to scan for devices and present them to DPDK. The FPGA bus driver will have
> > to check each AFU and match it against the set of registered AFU device
> > drivers to ensure that the crypto AFU gets the cryptodev driver, etc.
> > 
> > Logically, therefore, it is a bus - which just happens to be a sub-bus of
> > PCI, i.e. presented as a PCI device. Consider also that it may be possible
> > or even desirable, to use blacklisting and whitelisting for those AFU
> > devices so that some AFUs could be used by one app, while others by
> > another. If we just have a single PCI device, I think we'll find ourselves
> > duplicating a lot of bus-related functionality inside the driver in that
> > case.
> > 
> > Regards,
> > /Bruce
> 
> It makes sense indeed if you need to specialize several drivers specific
> to AFU mappings.
> 
> So, taking the bus approach, it seems we almost all agree that the
> current probe is not good enough.
> 
> I think you will run into issues if you registered your bus upon probing
> a PCI driver. Instead, would it be possible to:
> 
>   * not add the EAL option --ifpga.
>   * register the ifpga bus like any other.
>   * have a PCI driver that hotplug an IFPGA device, triggering a
>     scan and probe on the ifpga bus using the PCI device parameters.
>     Essentially you would find there the parameters that could be
>     given previously to the --ifpga option.
> 
> There should not be any EAL modification necessary.
> 
> The only downside I see would be that having both IFPGA device (for
> triggering bus ops) and afterward using AFU devices on this bus could be
> confusing. However, it seems safer and still cleaner overall.

Actually, just to build upon this remark, and thinking a little more
about it:

You would not need an additional "special" type of ifpga devices.
Upon attempting to hotplug an ifpga device, you would have a new
rte_devargs having the parameters within inserted, addressed to your
bus.

During scan, you would find this devargs, read the relevant parameters
and parse them (exactly like it is now implemented). You would then be
able to spawn as needed any number of rte_afu_device within your bus and
the scan would be complete.

The only trickery would be afterward, because you'd need at least one
device that would be detached and having the name given in the devargs.
So you would have to name your rte_afu_device after the devargs passed
as parameter to the plug, essentially having all of them using the same name.

Nothing prevents you from renaming your AFU devices after bus->plug, but
the EAL will rely on it for matching and knowing the scan went ok.

-- 
Gaëtan Rivet
6WIND