dev.dpdk.org archive mirror
 help / color / mirror / Atom feed
From: Shreyansh Jain <shreyansh.jain@nxp.com>
To: Bruce Richardson <bruce.richardson@intel.com>
Cc: "Gaëtan Rivet" <gaetan.rivet@6wind.com>,
	"Rosen Xu" <rosen.xu@intel.com>,
	dev@dpdk.org, declan.doherty@intel.com, tianfei.zhang@intel.com,
	hao.wu@intel.com
Subject: Re: [PATCH V2 3/5] Add Intel FPGA BUS Lib Code
Date: Wed, 21 Mar 2018 19:32:45 +0530	[thread overview]
Message-ID: <CAJ5mUsW6ZxpWzGUWd6Tb6e+oG-iLu4TWnaYuEEMwT_1_Xa9o2w@mail.gmail.com> (raw)
In-Reply-To: <20180321133509.GA15364@bricha3-MOBL.ger.corp.intel.com>

On Wed, Mar 21, 2018 at 7:05 PM, Bruce Richardson
<bruce.richardson@intel.com> 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.

In my opinion, its' not a problem that a heirarchichal bus model like
this is implemented in DPDK. That's being creative :)

But, here the issue is that same work can be done by a driver which
can create devices of multiple types (bbdev, cryptodev, ethdev etc)
and then use hotplugging (assuming that is supported by relevant
driver) over respective bus. I don't think we necessarily need a bus
for that. Is there some technical hurdle in following that model? Or,
is there some cases which cannot be handled by not having a dependency
bus?

Then, there is another problem of modifying the rte_bus code so that
this bus initialization can be delayed. That is something which is not
right in my opinion. That puts a fixed constraint on the EAL to look
for a bus as being special (like vdev). If that is a blocking problem,
it too needs to be solved - maybe separately.

Frankly, its not about 'can't-do-this' - but, I think this does
warrant a good thought before being mainlined.

>
> 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

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

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-03-21  7:51 [PATCH V2 0/5] Introduce Intel FPGA BUS Rosen Xu
2018-03-21  7:51 ` [PATCH V2 1/5] Add Intel FPGA BUS Command Parse Code Rosen Xu
2018-03-21  7:51 ` [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 ` [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 [this message]
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
2018-03-21  7:51 ` [PATCH V2 4/5] Add Intel FPGA BUS Rawdev Code Rosen Xu
2018-03-21  7:51 ` [PATCH V2 5/5] Add Intel OPAE Share Code Rosen Xu
2018-03-21 10:00 ` [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=CAJ5mUsW6ZxpWzGUWd6Tb6e+oG-iLu4TWnaYuEEMwT_1_Xa9o2w@mail.gmail.com \
    --to=shreyansh.jain@nxp.com \
    --cc=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=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).