From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?iso-8859-1?Q?Ga=EBtan?= Rivet <gaetan.rivet@6wind.com>
Subject: Re: [PATCH V2 3/5] Add Intel FPGA BUS Lib Code
Date: Wed, 21 Mar 2018 17:21:29 +0100
Message-ID: <20180321162129.4r5k2r5g4ftt2i23@bidouze.vm.6wind.com>
References: <1521618694-140757-1-git-send-email-rosen.xu@intel.com>
 <1521618694-140757-4-git-send-email-rosen.xu@intel.com>
 <20180321102025.v2zeijxrsauuvkss@bidouze.vm.6wind.com>
 <20180321133509.GA15364@bricha3-MOBL.ger.corp.intel.com>
 <20180321141405.mdtibaq75o7dbx4o@bidouze.vm.6wind.com>
 <20180321143131.5yptsqcsypmgdtwj@bidouze.vm.6wind.com>
 <20180321154156.GB11100@bricha3-MOBL.ger.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Transfer-Encoding: 8bit
Cc: Rosen Xu <rosen.xu@intel.com>, dev@dpdk.org, declan.doherty@intel.com,
 shreyansh.jain@nxp.com, tianfei.zhang@intel.com, hao.wu@intel.com
To: Bruce Richardson <bruce.richardson@intel.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mail-wm0-f44.google.com (mail-wm0-f44.google.com [74.125.82.44])
 by dpdk.org (Postfix) with ESMTP id 3CDCD2C58
 for <dev@dpdk.org>; Wed, 21 Mar 2018 17:21:45 +0100 (CET)
Received: by mail-wm0-f44.google.com with SMTP id h21so10758689wmd.1
 for <dev@dpdk.org>; Wed, 21 Mar 2018 09:21:45 -0700 (PDT)
Content-Disposition: inline
In-Reply-To: <20180321154156.GB11100@bricha3-MOBL.ger.corp.intel.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Wed, Mar 21, 2018 at 03:41:56PM +0000, Bruce Richardson wrote:
> On Wed, Mar 21, 2018 at 03:31:31PM +0100, Gaëtan Rivet wrote:
> > 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@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.
> > 
> > -- 
> Thanks, Gaëtan.
> 
> So, just to confirm I understand the suggestion correctly, you are
> proposing that we create and use the FPGA bus as normal, except that the
> scan will not report any devices. Then when an FPGA PCI device is
> discovered, we use the hotplug infrastructure to trigger a real scan of the
> FPGA for AFU devices, and do the whole driver mapping etc. process then.
> Is that the basic idea you suggest?

Yes.

-- 
Gaëtan Rivet
6WIND