From: Bruce Richardson <bruce.richardson@intel.com>
To: "Gaëtan Rivet" <gaetan.rivet@6wind.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 15:41:56 +0000 [thread overview]
Message-ID: <20180321154156.GB11100@bricha3-MOBL.ger.corp.intel.com> (raw)
In-Reply-To: <20180321143131.5yptsqcsypmgdtwj@bidouze.vm.6wind.com>
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?
/Bruce
next prev parent reply other threads:[~2018-03-21 15:42 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 [this message]
2018-03-21 16:21 ` Gaëtan Rivet
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=20180321154156.GB11100@bricha3-MOBL.ger.corp.intel.com \
--to=bruce.richardson@intel.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=gaetan.rivet@6wind.com \
--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).