Re: [dpdk-dev] [PATCH v3 00/15] Introduce SoC device/driver framework for EAL

[Jan Viktorin <viktorin@rehivetech.com>]

On Sun, 18 Sep 2016 13:58:50 +0800
Jianbo Liu <jianbo.liu@linaro.org> wrote:

> On 9 September 2016 at 16:43, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> > Introduction:
> > =============
> >
> > This patch set is direct derivative of Jan's original series [1],[2].
> >
> >  - As this deviates substantially from original series, if need be I can
> >    post it as a separate patch rather than v2. Please suggest.
> >  - Also, there are comments on original v1 ([4]) which are _not_
> >    incorporated in this series as they refer to section no more in new
> >    version.
> >  - This v3 version is based on the rte_driver/device patchset v9 [10].
> >    That series introduced device structures (rte_driver/rte_device)
> >    generalizing devices into PCI, VDEV, XXX. For the purpose of this
> >    patchset, XXX=>SOC.

[...]

> >
> > 5) Design considerations that are different from PCI:
> >  - Each driver implements its own scan and match function. PCI uses the BDF
> >    format to read the device from sysfs, but this _may_not_ be a case for a
> >    SoC ethernet device.
> >    = This is an important change from initial proposal by Jan in [2]. Unlike
> >    his attempt to use /sys/bus/platform, this patch relies on the PMD to  
> 
> It could be many redundant code if Each PMD driver has the scan
> function if its own.
> I think Jan's implementation is common to many platform drivers.

I personally can find a use case for having a custom scan function.
However, we should at least provide a default implementation. Probably,
both the scan and match functions should be used to _override_ a default
behaviour. So, only drivers that require to scan devices in a specific
way would provide a custom function for this.

I agree, that this can sometimes lead to code duplication. Moreover, it
opens door for a very non-standard, unsecure and wrong-by-design
approaches. I'd like more to provide one or more scan implementations
in EAL and do not put this responsibility on PMDs.

> 
> >    detect the devices. This is because SoC may require specific or
> >    additional info for device detection. Further, SoC may have embedded  

Can you provide an example for "additional info for device detection"?

> 
> Can you give us more precise definition about SoC driver? Does it
> include the driver in ARM server?

I am sorry but I don't understand this question.

What you mean by a "driver in ARM server"? Do you mean a kernel driver?

There is no "SoC driver" in the text so what definition are asking for?

> 
> >    devices/MACs which require initialization which cannot be covered through
> >    sysfs parsing.  

I think, the description itself is incorrect.

If a device's initialization cannot be satisfied vie sysfs, it means
that you have to write a specific probe function. This is not related to
scan in any way.

However, there may be a group of devices which are not managed by the
standard platform_driver of the Linux Kernel (or other OS). In that
case, the custom scan function would be helpful. I can imagine a device
in a fully I/O coherent platform that only requires to access
the /dev/mem only (for the register space). It is unsecure but it would
work without any OS-driver. However, I consider it a corner case.
It can be useful for testing sometimes but not very helpful for
production.

We should however support mainly the standard devices which are always
represented by the OS. Otherwise, such system would introduce security
issues.

> 
> I think it can be done in devinit, not in scan function. devinit can
> be different for each driver.

+1

> 
> >    = PCI based PMDs rely on EAL's capability to detect devices. This
> >    proposal puts the onus on PMD to detect devices, add to soc_device_list
> >    and wait for Probe. Matching, of device<=>driver is again PMD's callback.
> >  

Regards
Jan

-- 
  Jan Viktorin                E-mail: Viktorin@RehiveTech.com
  System Architect            Web:    www.RehiveTech.com
  RehiveTech
  Brno, Czech Republic