DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Gaëtan Rivet" <gaetan.rivet@6wind.com>
To: Bruce Richardson <bruce.richardson@intel.com>
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
Subject: Re: [dpdk-dev] [PATCH V2 3/5] Add Intel FPGA BUS Lib Code
Date: Wed, 21 Mar 2018 17:21:29 +0100	[thread overview]
Message-ID: <20180321162129.4r5k2r5g4ftt2i23@bidouze.vm.6wind.com> (raw)
In-Reply-To: <20180321154156.GB11100@bricha3-MOBL.ger.corp.intel.com>

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

  reply	other threads:[~2018-03-21 16:21 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-21  7:51 [dpdk-dev] [PATCH V2 0/5] Introduce Intel FPGA BUS Rosen Xu
2018-03-21  7:51 ` [dpdk-dev] [PATCH V2 1/5] Add Intel FPGA BUS Command Parse Code Rosen Xu
2018-03-21  7:51 ` [dpdk-dev] [PATCH V2 2/5] Add Intel FPGA BUS Probe Code Rosen Xu
2018-03-21  9:07   ` Shreyansh Jain
2018-03-21  9:10     ` Shreyansh Jain
2018-03-21 10:05   ` Gaëtan Rivet
2018-03-21  7:51 ` [dpdk-dev] [PATCH V2 3/5] Add Intel FPGA BUS Lib Code Rosen Xu
2018-03-21  9:28   ` Shreyansh Jain
2018-03-21 10:20   ` Gaëtan Rivet
2018-03-21 13:35     ` Bruce Richardson
2018-03-21 14:02       ` Shreyansh Jain
2018-03-21 14:06       ` Xu, Rosen
2018-03-21 14:14       ` Gaëtan Rivet
2018-03-21 14:31         ` Gaëtan Rivet
2018-03-21 15:41           ` Bruce Richardson
2018-03-21 16:21             ` Gaëtan Rivet [this message]
2018-03-21  7:51 ` [dpdk-dev] [PATCH V2 4/5] Add Intel FPGA BUS Rawdev Code Rosen Xu
2018-03-21  7:51 ` [dpdk-dev] [PATCH V2 5/5] Add Intel OPAE Share Code Rosen Xu
2018-03-21 10:00 ` [dpdk-dev] [PATCH V2 0/5] Introduce Intel FPGA BUS Gaëtan Rivet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180321162129.4r5k2r5g4ftt2i23@bidouze.vm.6wind.com \
    --to=gaetan.rivet@6wind.com \
    --cc=bruce.richardson@intel.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=hao.wu@intel.com \
    --cc=rosen.xu@intel.com \
    --cc=shreyansh.jain@nxp.com \
    --cc=tianfei.zhang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).