[dpdk-dev,v7,11/21] eal/soc: implement probing of drivers
Commit Message
Each SoC PMD registers a set of callback for scanning its own bus/infra and
matching devices to drivers when probe is called.
This patch introduces the infra for calls to SoC scan on rte_eal_soc_init()
and match on rte_eal_soc_probe().
Patch also adds test case for scan and probe.
Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
--
v4:
- Update test_soc for descriptive test function names
- Comments over test functions
- devinit and devuninint --> probe/remove
- RTE_VERIFY at some places
---
app/test/test_soc.c | 205 ++++++++++++++++++++++-
lib/librte_eal/bsdapp/eal/rte_eal_version.map | 4 +
lib/librte_eal/common/eal_common_soc.c | 213 +++++++++++++++++++++++-
lib/librte_eal/common/include/rte_soc.h | 75 ++++++++-
lib/librte_eal/linuxapp/eal/eal.c | 5 +
lib/librte_eal/linuxapp/eal/eal_soc.c | 21 ++-
lib/librte_eal/linuxapp/eal/rte_eal_version.map | 4 +
7 files changed, 519 insertions(+), 8 deletions(-)
Comments
On 28 October 2016 at 20:26, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> Each SoC PMD registers a set of callback for scanning its own bus/infra and
> matching devices to drivers when probe is called.
> This patch introduces the infra for calls to SoC scan on rte_eal_soc_init()
> and match on rte_eal_soc_probe().
>
> Patch also adds test case for scan and probe.
>
> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> --
> v4:
> - Update test_soc for descriptive test function names
> - Comments over test functions
> - devinit and devuninint --> probe/remove
> - RTE_VERIFY at some places
> ---
> app/test/test_soc.c | 205 ++++++++++++++++++++++-
> lib/librte_eal/bsdapp/eal/rte_eal_version.map | 4 +
> lib/librte_eal/common/eal_common_soc.c | 213 +++++++++++++++++++++++-
> lib/librte_eal/common/include/rte_soc.h | 75 ++++++++-
> lib/librte_eal/linuxapp/eal/eal.c | 5 +
> lib/librte_eal/linuxapp/eal/eal_soc.c | 21 ++-
> lib/librte_eal/linuxapp/eal/rte_eal_version.map | 4 +
> 7 files changed, 519 insertions(+), 8 deletions(-)
>
.....
> /**
> + * SoC device scan callback, called from rte_eal_soc_init.
> + * For various SoC, the bus on which devices are attached maynot be compliant
> + * to a standard platform (or platform bus itself). In which case, extra
> + * steps are implemented by PMD to scan over the bus and add devices to SoC
> + * device list.
> + */
> +typedef void (soc_scan_t)(void);
I'm still not sure about the purpose of soc_scan, and how to use it.
If it's for each driver, it should at least struct rte_soc_driver * as
its parameter.
If it's for each bus, why it is in rte_soc_driver?
I know you will implement bus driver in the future, but we need to
make it clear for current simplified implementation.
> +
> +/**
> + * Custom device<=>driver match callback for SoC
> + * Unlike PCI, SoC devices don't have a fixed definition of device
> + * identification. PMDs can implement a specific matching function in which
> + * driver and device objects are provided to perform custom match.
> + */
> +typedef int (soc_match_t)(struct rte_soc_driver *, struct rte_soc_device *);
> +
> +/**
> * A structure describing a SoC driver.
> */
> struct rte_soc_driver {
> @@ -104,6 +120,8 @@ struct rte_soc_driver {
> struct rte_driver driver; /**< Inherit core driver. */
> soc_probe_t *probe; /**< Device probe */
> soc_remove_t *remove; /**< Device remove */
> + soc_scan_t *scan_fn; /**< Callback for scanning SoC bus*/
> + soc_match_t *match_fn; /**< Callback to match dev<->drv */
> const struct rte_soc_id *id_table; /**< ID table, NULL terminated */
> };
>
> @@ -146,12 +164,63 @@ rte_eal_compare_soc_addr(const struct rte_soc_addr *a0,
> }
>
> /**
> + * Default function for matching the Soc driver with device. Each driver can
> + * either use this function or define their own soc matching function.
> + * This function relies on the compatible string extracted from sysfs. But,
> + * a SoC might have different way of identifying its devices. Such SoC can
> + * override match_fn.
> + *
> + * @return
> + * 0 on success
> + * -1 when no match found
> + */
> +int
> +rte_eal_soc_match_compat(struct rte_soc_driver *drv,
> + struct rte_soc_device *dev);
> +
> +/**
> + * Probe SoC devices for registered drivers.
> + *
> + * @return
> + * 0 on success
> + * !0 in case of any failure in probe
> + */
> +int rte_eal_soc_probe(void);
> +
> +/**
> + * Probe the single SoC device.
> + */
> +int rte_eal_soc_probe_one(const struct rte_soc_addr *addr);
> +
> +/**
> + * Close the single SoC device.
> + *
> + * Scan the SoC devices and find the SoC device specified by the SoC
> + * address, then call the remove() function for registered driver
> + * that has a matching entry in its id_table for discovered device.
> + *
> + * @param addr
> + * The SoC address to close.
> + * @return
> + * - 0 on success.
> + * - Negative on error.
> + */
> +int rte_eal_soc_detach(const struct rte_soc_addr *addr);
> +
> +/**
> * Dump discovered SoC devices.
> + *
> + * @param f
> + * File to dump device info in.
> */
> void rte_eal_soc_dump(FILE *f);
>
> /**
> * Register a SoC driver.
> + *
> + * @param driver
> + * Object for SoC driver to register
> + * @return void
> */
> void rte_eal_soc_register(struct rte_soc_driver *driver);
>
> @@ -167,6 +236,10 @@ RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
>
> /**
> * Unregister a SoC driver.
> + *
> + * @param driver
> + * Object for SoC driver to unregister
> + * @return void
> */
> void rte_eal_soc_unregister(struct rte_soc_driver *driver);
>
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 098ba02..bd775f3 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -70,6 +70,7 @@
> #include <rte_cpuflags.h>
> #include <rte_interrupts.h>
> #include <rte_pci.h>
> +#include <rte_soc.h>
> #include <rte_dev.h>
> #include <rte_devargs.h>
> #include <rte_common.h>
> @@ -890,6 +891,10 @@ rte_eal_init(int argc, char **argv)
> if (rte_eal_pci_probe())
> rte_panic("Cannot probe PCI\n");
>
> + /* Probe & Initialize SoC devices */
> + if (rte_eal_soc_probe())
> + rte_panic("Cannot probe SoC\n");
> +
> rte_eal_mcfg_complete();
>
> return fctret;
> diff --git a/lib/librte_eal/linuxapp/eal/eal_soc.c b/lib/librte_eal/linuxapp/eal/eal_soc.c
> index 04848b9..3929a76 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_soc.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_soc.c
> @@ -44,13 +44,28 @@
> #include <rte_log.h>
> #include <rte_soc.h>
>
> -#include "eal_internal_cfg.h"
> -#include "eal_filesystem.h"
> -#include "eal_private.h"
> +#include <eal_internal_cfg.h>
> +#include <eal_filesystem.h>
> +#include <eal_private.h>
>
> /* Init the SoC EAL subsystem */
> int
> rte_eal_soc_init(void)
> {
> + struct rte_soc_driver *drv;
> +
> + /* SoC is disabled by default */
> + if (!internal_config.enable_soc)
> + return 0;
> +
> + /* For each registered driver, call their scan routine to perform any
> + * custom scan for devices (for example, custom buses)
> + */
> + TAILQ_FOREACH(drv, &soc_driver_list, next) {
> + RTE_VERIFY(drv->scan_fn);
> + drv->scan_fn();
> + /* Ignore all errors from this */
> + }
> +
> return 0;
> }
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index 0155025..c28e093 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -175,7 +175,11 @@ DPDK_16.11 {
> rte_eal_dev_attach;
> rte_eal_dev_detach;
> rte_eal_map_resource;
> + rte_eal_soc_detach;
> rte_eal_soc_dump;
> + rte_eal_soc_match;
> + rte_eal_soc_probe;
> + rte_eal_soc_probe_one;
> rte_eal_soc_register;
> rte_eal_soc_unregister;
> rte_eal_unmap_resource;
> --
> 2.7.4
>
On Thursday 10 November 2016 09:00 AM, Jianbo Liu wrote:
> On 28 October 2016 at 20:26, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> Each SoC PMD registers a set of callback for scanning its own bus/infra and
>> matching devices to drivers when probe is called.
>> This patch introduces the infra for calls to SoC scan on rte_eal_soc_init()
>> and match on rte_eal_soc_probe().
>>
>> Patch also adds test case for scan and probe.
>>
>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> --
>> v4:
>> - Update test_soc for descriptive test function names
>> - Comments over test functions
>> - devinit and devuninint --> probe/remove
>> - RTE_VERIFY at some places
>> ---
>> app/test/test_soc.c | 205 ++++++++++++++++++++++-
>> lib/librte_eal/bsdapp/eal/rte_eal_version.map | 4 +
>> lib/librte_eal/common/eal_common_soc.c | 213 +++++++++++++++++++++++-
>> lib/librte_eal/common/include/rte_soc.h | 75 ++++++++-
>> lib/librte_eal/linuxapp/eal/eal.c | 5 +
>> lib/librte_eal/linuxapp/eal/eal_soc.c | 21 ++-
>> lib/librte_eal/linuxapp/eal/rte_eal_version.map | 4 +
>> 7 files changed, 519 insertions(+), 8 deletions(-)
>>
> ......
>
>> /**
>> + * SoC device scan callback, called from rte_eal_soc_init.
>> + * For various SoC, the bus on which devices are attached maynot be compliant
>> + * to a standard platform (or platform bus itself). In which case, extra
>> + * steps are implemented by PMD to scan over the bus and add devices to SoC
>> + * device list.
>> + */
>> +typedef void (soc_scan_t)(void);
>
> I'm still not sure about the purpose of soc_scan, and how to use it.
For each device to be used by DPDK, which cannot be scanned/identified
using the existing PCI/VDEV methods (sysfs/bus/pci), 'soc_scan_t'
provides a way for driver to make those devices part of device lists.
Ideally, 'scan' is not a function of a driver. It is a bus function -
which is missing in this case.
> If it's for each driver, it should at least struct rte_soc_driver * as
> its parameter.
Its for each driver - assuming that each non-PCI driver which implements
it knows how to find devices which it can control (for example, special
area in sysfs, or even platform bus).
> If it's for each bus, why it is in rte_soc_driver?
Short answer - lack of a better place. It should be in dev.h probably
(rte_device/driver) but it would look out of place (as that represents
PCI devices also which cannot implement it - all PCI devices are scanned
in one go irrespective of driver)
> I know you will implement bus driver in the future, but we need to
> make it clear for current simplified implementation.
Current implementation makes only a single assumption - that rather than
relying on EAL for identifying devices (as being done now), next best
option in existing framework (driver) should have control of finding
devices.
This is primarily to make the SoC work parallel to PCI implementation
without much top-down changes in EAL.
Bus model, improvises it by moving this implementation a little above in
hierarchy - in rte_bus<-rte_driver<-PMD.
I understand your apprehension - 'driver-scanning-for-devices' is indeed
not correct real world analogy. It is just a place holder for enabling
those drivers/PMDs which cannot work in absence of the right model.
And that is still work in progress.
>
>> +
>> +/**
>> + * Custom device<=>driver match callback for SoC
>> + * Unlike PCI, SoC devices don't have a fixed definition of device
>> + * identification. PMDs can implement a specific matching function in which
>> + * driver and device objects are provided to perform custom match.
>> + */
>> +typedef int (soc_match_t)(struct rte_soc_driver *, struct rte_soc_device *);
>> +
>> +/**
>> * A structure describing a SoC driver.
>> */
>> struct rte_soc_driver {
>> @@ -104,6 +120,8 @@ struct rte_soc_driver {
>> struct rte_driver driver; /**< Inherit core driver. */
>> soc_probe_t *probe; /**< Device probe */
>> soc_remove_t *remove; /**< Device remove */
>> + soc_scan_t *scan_fn; /**< Callback for scanning SoC bus*/
>> + soc_match_t *match_fn; /**< Callback to match dev<->drv */
>> const struct rte_soc_id *id_table; /**< ID table, NULL terminated */
>> };
>>
>> @@ -146,12 +164,63 @@ rte_eal_compare_soc_addr(const struct rte_soc_addr *a0,
>> }
>>
>> /**
>> + * Default function for matching the Soc driver with device. Each driver can
>> + * either use this function or define their own soc matching function.
>> + * This function relies on the compatible string extracted from sysfs. But,
>> + * a SoC might have different way of identifying its devices. Such SoC can
>> + * override match_fn.
>> + *
>> + * @return
>> + * 0 on success
>> + * -1 when no match found
>> + */
>> +int
>> +rte_eal_soc_match_compat(struct rte_soc_driver *drv,
>> + struct rte_soc_device *dev);
>> +
>> +/**
>> + * Probe SoC devices for registered drivers.
>> + *
>> + * @return
>> + * 0 on success
>> + * !0 in case of any failure in probe
>> + */
>> +int rte_eal_soc_probe(void);
>> +
>> +/**
>> + * Probe the single SoC device.
>> + */
>> +int rte_eal_soc_probe_one(const struct rte_soc_addr *addr);
>> +
>> +/**
>> + * Close the single SoC device.
>> + *
>> + * Scan the SoC devices and find the SoC device specified by the SoC
>> + * address, then call the remove() function for registered driver
>> + * that has a matching entry in its id_table for discovered device.
>> + *
>> + * @param addr
>> + * The SoC address to close.
>> + * @return
>> + * - 0 on success.
>> + * - Negative on error.
>> + */
>> +int rte_eal_soc_detach(const struct rte_soc_addr *addr);
>> +
>> +/**
>> * Dump discovered SoC devices.
>> + *
>> + * @param f
>> + * File to dump device info in.
>> */
>> void rte_eal_soc_dump(FILE *f);
>>
>> /**
>> * Register a SoC driver.
>> + *
>> + * @param driver
>> + * Object for SoC driver to register
>> + * @return void
>> */
>> void rte_eal_soc_register(struct rte_soc_driver *driver);
>>
>> @@ -167,6 +236,10 @@ RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
>>
>> /**
>> * Unregister a SoC driver.
>> + *
>> + * @param driver
>> + * Object for SoC driver to unregister
>> + * @return void
>> */
>> void rte_eal_soc_unregister(struct rte_soc_driver *driver);
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
>> index 098ba02..bd775f3 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>> @@ -70,6 +70,7 @@
>> #include <rte_cpuflags.h>
>> #include <rte_interrupts.h>
>> #include <rte_pci.h>
>> +#include <rte_soc.h>
>> #include <rte_dev.h>
>> #include <rte_devargs.h>
>> #include <rte_common.h>
>> @@ -890,6 +891,10 @@ rte_eal_init(int argc, char **argv)
>> if (rte_eal_pci_probe())
>> rte_panic("Cannot probe PCI\n");
>>
>> + /* Probe & Initialize SoC devices */
>> + if (rte_eal_soc_probe())
>> + rte_panic("Cannot probe SoC\n");
>> +
>> rte_eal_mcfg_complete();
>>
>> return fctret;
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_soc.c b/lib/librte_eal/linuxapp/eal/eal_soc.c
>> index 04848b9..3929a76 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_soc.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_soc.c
>> @@ -44,13 +44,28 @@
>> #include <rte_log.h>
>> #include <rte_soc.h>
>>
>> -#include "eal_internal_cfg.h"
>> -#include "eal_filesystem.h"
>> -#include "eal_private.h"
>> +#include <eal_internal_cfg.h>
>> +#include <eal_filesystem.h>
>> +#include <eal_private.h>
>>
>> /* Init the SoC EAL subsystem */
>> int
>> rte_eal_soc_init(void)
>> {
>> + struct rte_soc_driver *drv;
>> +
>> + /* SoC is disabled by default */
>> + if (!internal_config.enable_soc)
>> + return 0;
>> +
>> + /* For each registered driver, call their scan routine to perform any
>> + * custom scan for devices (for example, custom buses)
>> + */
>> + TAILQ_FOREACH(drv, &soc_driver_list, next) {
>> + RTE_VERIFY(drv->scan_fn);
>> + drv->scan_fn();
>> + /* Ignore all errors from this */
>> + }
>> +
>> return 0;
>> }
>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> index 0155025..c28e093 100644
>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>> @@ -175,7 +175,11 @@ DPDK_16.11 {
>> rte_eal_dev_attach;
>> rte_eal_dev_detach;
>> rte_eal_map_resource;
>> + rte_eal_soc_detach;
>> rte_eal_soc_dump;
>> + rte_eal_soc_match;
>> + rte_eal_soc_probe;
>> + rte_eal_soc_probe_one;
>> rte_eal_soc_register;
>> rte_eal_soc_unregister;
>> rte_eal_unmap_resource;
>> --
>> 2.7.4
>>
>
On 10 November 2016 at 14:10, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> On Thursday 10 November 2016 09:00 AM, Jianbo Liu wrote:
>>
>> On 28 October 2016 at 20:26, Shreyansh Jain <shreyansh.jain@nxp.com>
>> wrote:
>>>
>>> Each SoC PMD registers a set of callback for scanning its own bus/infra
>>> and
>>> matching devices to drivers when probe is called.
>>> This patch introduces the infra for calls to SoC scan on
>>> rte_eal_soc_init()
>>> and match on rte_eal_soc_probe().
>>>
>>> Patch also adds test case for scan and probe.
>>>
>>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>> --
>>> v4:
>>> - Update test_soc for descriptive test function names
>>> - Comments over test functions
>>> - devinit and devuninint --> probe/remove
>>> - RTE_VERIFY at some places
>>> ---
>>> app/test/test_soc.c | 205
>>> ++++++++++++++++++++++-
>>> lib/librte_eal/bsdapp/eal/rte_eal_version.map | 4 +
>>> lib/librte_eal/common/eal_common_soc.c | 213
>>> +++++++++++++++++++++++-
>>> lib/librte_eal/common/include/rte_soc.h | 75 ++++++++-
>>> lib/librte_eal/linuxapp/eal/eal.c | 5 +
>>> lib/librte_eal/linuxapp/eal/eal_soc.c | 21 ++-
>>> lib/librte_eal/linuxapp/eal/rte_eal_version.map | 4 +
>>> 7 files changed, 519 insertions(+), 8 deletions(-)
>>>
>> ......
>>
>>> /**
>>> + * SoC device scan callback, called from rte_eal_soc_init.
>>> + * For various SoC, the bus on which devices are attached maynot be
>>> compliant
>>> + * to a standard platform (or platform bus itself). In which case, extra
>>> + * steps are implemented by PMD to scan over the bus and add devices to
>>> SoC
>>> + * device list.
>>> + */
>>> +typedef void (soc_scan_t)(void);
>>
>>
>> I'm still not sure about the purpose of soc_scan, and how to use it.
>
>
> For each device to be used by DPDK, which cannot be scanned/identified using
> the existing PCI/VDEV methods (sysfs/bus/pci), 'soc_scan_t' provides a way
> for driver to make those devices part of device lists.
>
> Ideally, 'scan' is not a function of a driver. It is a bus function - which
> is missing in this case.
>
>> If it's for each driver, it should at least struct rte_soc_driver * as
>> its parameter.
>
>
> Its for each driver - assuming that each non-PCI driver which implements it
> knows how to find devices which it can control (for example, special area in
> sysfs, or even platform bus).
>
Considering there are several drivers in a platform bus, each driver
call the scan function, like the rte_eal_soc_scan_platform_bus() you
implemented.
The first will add soc devices to the list, but the remaining calls
are redundant.
The other issue is adding the driver parameter. Do you need extra
information from driver to scan the bus?
>> If it's for each bus, why it is in rte_soc_driver?
>
>
> Short answer - lack of a better place. It should be in dev.h probably
> (rte_device/driver) but it would look out of place (as that represents PCI
> devices also which cannot implement it - all PCI devices are scanned in one
> go irrespective of driver)
>
>> I know you will implement bus driver in the future, but we need to
>> make it clear for current simplified implementation.
>
>
> Current implementation makes only a single assumption - that rather than
> relying on EAL for identifying devices (as being done now), next best option
> in existing framework (driver) should have control of finding devices.
>
> This is primarily to make the SoC work parallel to PCI implementation
> without much top-down changes in EAL.
>
> Bus model, improvises it by moving this implementation a little above in
> hierarchy - in rte_bus<-rte_driver<-PMD.
>
> I understand your apprehension - 'driver-scanning-for-devices' is indeed not
> correct real world analogy. It is just a place holder for enabling those
> drivers/PMDs which cannot work in absence of the right model.
> And that is still work in progress.
>
>
>>
>>> +
>>> +/**
>>> + * Custom device<=>driver match callback for SoC
>>> + * Unlike PCI, SoC devices don't have a fixed definition of device
>>> + * identification. PMDs can implement a specific matching function in
>>> which
>>> + * driver and device objects are provided to perform custom match.
>>> + */
>>> +typedef int (soc_match_t)(struct rte_soc_driver *, struct rte_soc_device
>>> *);
>>> +
>>> +/**
>>> * A structure describing a SoC driver.
>>> */
>>> struct rte_soc_driver {
>>> @@ -104,6 +120,8 @@ struct rte_soc_driver {
>>> struct rte_driver driver; /**< Inherit core driver. */
>>> soc_probe_t *probe; /**< Device probe */
>>> soc_remove_t *remove; /**< Device remove */
>>> + soc_scan_t *scan_fn; /**< Callback for scanning SoC
>>> bus*/
>>> + soc_match_t *match_fn; /**< Callback to match
>>> dev<->drv */
>>> const struct rte_soc_id *id_table; /**< ID table, NULL terminated
>>> */
>>> };
>>>
>>> @@ -146,12 +164,63 @@ rte_eal_compare_soc_addr(const struct rte_soc_addr
>>> *a0,
>>> }
>>>
>>> /**
>>> + * Default function for matching the Soc driver with device. Each driver
>>> can
>>> + * either use this function or define their own soc matching function.
>>> + * This function relies on the compatible string extracted from sysfs.
>>> But,
>>> + * a SoC might have different way of identifying its devices. Such SoC
>>> can
>>> + * override match_fn.
>>> + *
>>> + * @return
>>> + * 0 on success
>>> + * -1 when no match found
>>> + */
>>> +int
>>> +rte_eal_soc_match_compat(struct rte_soc_driver *drv,
>>> + struct rte_soc_device *dev);
>>> +
>>> +/**
>>> + * Probe SoC devices for registered drivers.
>>> + *
>>> + * @return
>>> + * 0 on success
>>> + * !0 in case of any failure in probe
>>> + */
>>> +int rte_eal_soc_probe(void);
>>> +
>>> +/**
>>> + * Probe the single SoC device.
>>> + */
>>> +int rte_eal_soc_probe_one(const struct rte_soc_addr *addr);
>>> +
>>> +/**
>>> + * Close the single SoC device.
>>> + *
>>> + * Scan the SoC devices and find the SoC device specified by the SoC
>>> + * address, then call the remove() function for registered driver
>>> + * that has a matching entry in its id_table for discovered device.
>>> + *
>>> + * @param addr
>>> + * The SoC address to close.
>>> + * @return
>>> + * - 0 on success.
>>> + * - Negative on error.
>>> + */
>>> +int rte_eal_soc_detach(const struct rte_soc_addr *addr);
>>> +
>>> +/**
>>> * Dump discovered SoC devices.
>>> + *
>>> + * @param f
>>> + * File to dump device info in.
>>> */
>>> void rte_eal_soc_dump(FILE *f);
>>>
>>> /**
>>> * Register a SoC driver.
>>> + *
>>> + * @param driver
>>> + * Object for SoC driver to register
>>> + * @return void
>>> */
>>> void rte_eal_soc_register(struct rte_soc_driver *driver);
>>>
>>> @@ -167,6 +236,10 @@ RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
>>>
>>> /**
>>> * Unregister a SoC driver.
>>> + *
>>> + * @param driver
>>> + * Object for SoC driver to unregister
>>> + * @return void
>>> */
>>> void rte_eal_soc_unregister(struct rte_soc_driver *driver);
>>>
>>> diff --git a/lib/librte_eal/linuxapp/eal/eal.c
>>> b/lib/librte_eal/linuxapp/eal/eal.c
>>> index 098ba02..bd775f3 100644
>>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>>> @@ -70,6 +70,7 @@
>>> #include <rte_cpuflags.h>
>>> #include <rte_interrupts.h>
>>> #include <rte_pci.h>
>>> +#include <rte_soc.h>
>>> #include <rte_dev.h>
>>> #include <rte_devargs.h>
>>> #include <rte_common.h>
>>> @@ -890,6 +891,10 @@ rte_eal_init(int argc, char **argv)
>>> if (rte_eal_pci_probe())
>>> rte_panic("Cannot probe PCI\n");
>>>
>>> + /* Probe & Initialize SoC devices */
>>> + if (rte_eal_soc_probe())
>>> + rte_panic("Cannot probe SoC\n");
>>> +
>>> rte_eal_mcfg_complete();
>>>
>>> return fctret;
>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_soc.c
>>> b/lib/librte_eal/linuxapp/eal/eal_soc.c
>>> index 04848b9..3929a76 100644
>>> --- a/lib/librte_eal/linuxapp/eal/eal_soc.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal_soc.c
>>> @@ -44,13 +44,28 @@
>>> #include <rte_log.h>
>>> #include <rte_soc.h>
>>>
>>> -#include "eal_internal_cfg.h"
>>> -#include "eal_filesystem.h"
>>> -#include "eal_private.h"
>>> +#include <eal_internal_cfg.h>
>>> +#include <eal_filesystem.h>
>>> +#include <eal_private.h>
>>>
>>> /* Init the SoC EAL subsystem */
>>> int
>>> rte_eal_soc_init(void)
>>> {
>>> + struct rte_soc_driver *drv;
>>> +
>>> + /* SoC is disabled by default */
>>> + if (!internal_config.enable_soc)
>>> + return 0;
>>> +
>>> + /* For each registered driver, call their scan routine to perform
>>> any
>>> + * custom scan for devices (for example, custom buses)
>>> + */
>>> + TAILQ_FOREACH(drv, &soc_driver_list, next) {
>>> + RTE_VERIFY(drv->scan_fn);
>>> + drv->scan_fn();
>>> + /* Ignore all errors from this */
>>> + }
>>> +
>>> return 0;
>>> }
>>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> index 0155025..c28e093 100644
>>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>> @@ -175,7 +175,11 @@ DPDK_16.11 {
>>> rte_eal_dev_attach;
>>> rte_eal_dev_detach;
>>> rte_eal_map_resource;
>>> + rte_eal_soc_detach;
>>> rte_eal_soc_dump;
>>> + rte_eal_soc_match;
>>> + rte_eal_soc_probe;
>>> + rte_eal_soc_probe_one;
>>> rte_eal_soc_register;
>>> rte_eal_soc_unregister;
>>> rte_eal_unmap_resource;
>>> --
>>> 2.7.4
>>>
>>
>
>
> --
> -
> Shreyansh
On Thursday 10 November 2016 01:11 PM, Jianbo Liu wrote:
> On 10 November 2016 at 14:10, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> On Thursday 10 November 2016 09:00 AM, Jianbo Liu wrote:
>>>
>>> On 28 October 2016 at 20:26, Shreyansh Jain <shreyansh.jain@nxp.com>
>>> wrote:
>>>>
>>>> Each SoC PMD registers a set of callback for scanning its own bus/infra
>>>> and
>>>> matching devices to drivers when probe is called.
>>>> This patch introduces the infra for calls to SoC scan on
>>>> rte_eal_soc_init()
>>>> and match on rte_eal_soc_probe().
>>>>
>>>> Patch also adds test case for scan and probe.
>>>>
>>>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>>> --
>>>> v4:
>>>> - Update test_soc for descriptive test function names
>>>> - Comments over test functions
>>>> - devinit and devuninint --> probe/remove
>>>> - RTE_VERIFY at some places
>>>> ---
>>>> app/test/test_soc.c | 205
>>>> ++++++++++++++++++++++-
>>>> lib/librte_eal/bsdapp/eal/rte_eal_version.map | 4 +
>>>> lib/librte_eal/common/eal_common_soc.c | 213
>>>> +++++++++++++++++++++++-
>>>> lib/librte_eal/common/include/rte_soc.h | 75 ++++++++-
>>>> lib/librte_eal/linuxapp/eal/eal.c | 5 +
>>>> lib/librte_eal/linuxapp/eal/eal_soc.c | 21 ++-
>>>> lib/librte_eal/linuxapp/eal/rte_eal_version.map | 4 +
>>>> 7 files changed, 519 insertions(+), 8 deletions(-)
>>>>
>>> ......
>>>
>>>> /**
>>>> + * SoC device scan callback, called from rte_eal_soc_init.
>>>> + * For various SoC, the bus on which devices are attached maynot be
>>>> compliant
>>>> + * to a standard platform (or platform bus itself). In which case, extra
>>>> + * steps are implemented by PMD to scan over the bus and add devices to
>>>> SoC
>>>> + * device list.
>>>> + */
>>>> +typedef void (soc_scan_t)(void);
>>>
>>>
>>> I'm still not sure about the purpose of soc_scan, and how to use it.
>>
>>
>> For each device to be used by DPDK, which cannot be scanned/identified using
>> the existing PCI/VDEV methods (sysfs/bus/pci), 'soc_scan_t' provides a way
>> for driver to make those devices part of device lists.
>>
>> Ideally, 'scan' is not a function of a driver. It is a bus function - which
>> is missing in this case.
>>
>>> If it's for each driver, it should at least struct rte_soc_driver * as
>>> its parameter.
>>
>>
>> Its for each driver - assuming that each non-PCI driver which implements it
>> knows how to find devices which it can control (for example, special area in
>> sysfs, or even platform bus).
>>
>
> Considering there are several drivers in a platform bus, each driver
> call the scan function, like the rte_eal_soc_scan_platform_bus() you
> implemented.
> The first will add soc devices to the list, but the remaining calls
> are redundant.
Indeed. This is exactly the issue we will face if we try and move this
scan/match logic to PCI - all devices are identified in one step.
There is a difference in principle here:
A SoC device/driver combination is essentially focused towards a single
type of bus<->devices. For example, a NXP PMD would implement a scan
function which would scan for all devices on NXP's bus. This would not
conflict with another XYZ SoC PMD which scans its specific bus.
There is caveat to this - the platform bus. There can be multiple
drivers which can serve platform bus compliant devices. First
PMD->scan() initiated for such a bus/device would leave all other scans
redundant.
More similar caveats will come if we consider somewhat generic buses. At
least I couldn't find any interest for such devices in the ML when I
picked this series (from where Jan left it).
Probably when more common type of PMDs come in, some default scan
implementation can check for skipping those devices which are already
added. It would be redundant but harmless.
>
> The other issue is adding the driver parameter. Do you need extra
> information from driver to scan the bus?
>
>>> If it's for each bus, why it is in rte_soc_driver?
>>
>>
>> Short answer - lack of a better place. It should be in dev.h probably
>> (rte_device/driver) but it would look out of place (as that represents PCI
>> devices also which cannot implement it - all PCI devices are scanned in one
>> go irrespective of driver)
>>
>>> I know you will implement bus driver in the future, but we need to
>>> make it clear for current simplified implementation.
>>
>>
>> Current implementation makes only a single assumption - that rather than
>> relying on EAL for identifying devices (as being done now), next best option
>> in existing framework (driver) should have control of finding devices.
>>
>> This is primarily to make the SoC work parallel to PCI implementation
>> without much top-down changes in EAL.
>>
>> Bus model, improvises it by moving this implementation a little above in
>> hierarchy - in rte_bus<-rte_driver<-PMD.
>>
>> I understand your apprehension - 'driver-scanning-for-devices' is indeed not
>> correct real world analogy. It is just a place holder for enabling those
>> drivers/PMDs which cannot work in absence of the right model.
>> And that is still work in progress.
>>
>>
>>>
>>>> +
>>>> +/**
>>>> + * Custom device<=>driver match callback for SoC
>>>> + * Unlike PCI, SoC devices don't have a fixed definition of device
>>>> + * identification. PMDs can implement a specific matching function in
>>>> which
>>>> + * driver and device objects are provided to perform custom match.
>>>> + */
>>>> +typedef int (soc_match_t)(struct rte_soc_driver *, struct rte_soc_device
>>>> *);
>>>> +
>>>> +/**
>>>> * A structure describing a SoC driver.
>>>> */
>>>> struct rte_soc_driver {
>>>> @@ -104,6 +120,8 @@ struct rte_soc_driver {
>>>> struct rte_driver driver; /**< Inherit core driver. */
>>>> soc_probe_t *probe; /**< Device probe */
>>>> soc_remove_t *remove; /**< Device remove */
>>>> + soc_scan_t *scan_fn; /**< Callback for scanning SoC
>>>> bus*/
>>>> + soc_match_t *match_fn; /**< Callback to match
>>>> dev<->drv */
>>>> const struct rte_soc_id *id_table; /**< ID table, NULL terminated
>>>> */
>>>> };
>>>>
>>>> @@ -146,12 +164,63 @@ rte_eal_compare_soc_addr(const struct rte_soc_addr
>>>> *a0,
>>>> }
>>>>
>>>> /**
>>>> + * Default function for matching the Soc driver with device. Each driver
>>>> can
>>>> + * either use this function or define their own soc matching function.
>>>> + * This function relies on the compatible string extracted from sysfs.
>>>> But,
>>>> + * a SoC might have different way of identifying its devices. Such SoC
>>>> can
>>>> + * override match_fn.
>>>> + *
>>>> + * @return
>>>> + * 0 on success
>>>> + * -1 when no match found
>>>> + */
>>>> +int
>>>> +rte_eal_soc_match_compat(struct rte_soc_driver *drv,
>>>> + struct rte_soc_device *dev);
>>>> +
>>>> +/**
>>>> + * Probe SoC devices for registered drivers.
>>>> + *
>>>> + * @return
>>>> + * 0 on success
>>>> + * !0 in case of any failure in probe
>>>> + */
>>>> +int rte_eal_soc_probe(void);
>>>> +
>>>> +/**
>>>> + * Probe the single SoC device.
>>>> + */
>>>> +int rte_eal_soc_probe_one(const struct rte_soc_addr *addr);
>>>> +
>>>> +/**
>>>> + * Close the single SoC device.
>>>> + *
>>>> + * Scan the SoC devices and find the SoC device specified by the SoC
>>>> + * address, then call the remove() function for registered driver
>>>> + * that has a matching entry in its id_table for discovered device.
>>>> + *
>>>> + * @param addr
>>>> + * The SoC address to close.
>>>> + * @return
>>>> + * - 0 on success.
>>>> + * - Negative on error.
>>>> + */
>>>> +int rte_eal_soc_detach(const struct rte_soc_addr *addr);
>>>> +
>>>> +/**
>>>> * Dump discovered SoC devices.
>>>> + *
>>>> + * @param f
>>>> + * File to dump device info in.
>>>> */
>>>> void rte_eal_soc_dump(FILE *f);
>>>>
>>>> /**
>>>> * Register a SoC driver.
>>>> + *
>>>> + * @param driver
>>>> + * Object for SoC driver to register
>>>> + * @return void
>>>> */
>>>> void rte_eal_soc_register(struct rte_soc_driver *driver);
>>>>
>>>> @@ -167,6 +236,10 @@ RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
>>>>
>>>> /**
>>>> * Unregister a SoC driver.
>>>> + *
>>>> + * @param driver
>>>> + * Object for SoC driver to unregister
>>>> + * @return void
>>>> */
>>>> void rte_eal_soc_unregister(struct rte_soc_driver *driver);
>>>>
>>>> diff --git a/lib/librte_eal/linuxapp/eal/eal.c
>>>> b/lib/librte_eal/linuxapp/eal/eal.c
>>>> index 098ba02..bd775f3 100644
>>>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>>>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>>>> @@ -70,6 +70,7 @@
>>>> #include <rte_cpuflags.h>
>>>> #include <rte_interrupts.h>
>>>> #include <rte_pci.h>
>>>> +#include <rte_soc.h>
>>>> #include <rte_dev.h>
>>>> #include <rte_devargs.h>
>>>> #include <rte_common.h>
>>>> @@ -890,6 +891,10 @@ rte_eal_init(int argc, char **argv)
>>>> if (rte_eal_pci_probe())
>>>> rte_panic("Cannot probe PCI\n");
>>>>
>>>> + /* Probe & Initialize SoC devices */
>>>> + if (rte_eal_soc_probe())
>>>> + rte_panic("Cannot probe SoC\n");
>>>> +
>>>> rte_eal_mcfg_complete();
>>>>
>>>> return fctret;
>>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_soc.c
>>>> b/lib/librte_eal/linuxapp/eal/eal_soc.c
>>>> index 04848b9..3929a76 100644
>>>> --- a/lib/librte_eal/linuxapp/eal/eal_soc.c
>>>> +++ b/lib/librte_eal/linuxapp/eal/eal_soc.c
>>>> @@ -44,13 +44,28 @@
>>>> #include <rte_log.h>
>>>> #include <rte_soc.h>
>>>>
>>>> -#include "eal_internal_cfg.h"
>>>> -#include "eal_filesystem.h"
>>>> -#include "eal_private.h"
>>>> +#include <eal_internal_cfg.h>
>>>> +#include <eal_filesystem.h>
>>>> +#include <eal_private.h>
>>>>
>>>> /* Init the SoC EAL subsystem */
>>>> int
>>>> rte_eal_soc_init(void)
>>>> {
>>>> + struct rte_soc_driver *drv;
>>>> +
>>>> + /* SoC is disabled by default */
>>>> + if (!internal_config.enable_soc)
>>>> + return 0;
>>>> +
>>>> + /* For each registered driver, call their scan routine to perform
>>>> any
>>>> + * custom scan for devices (for example, custom buses)
>>>> + */
>>>> + TAILQ_FOREACH(drv, &soc_driver_list, next) {
>>>> + RTE_VERIFY(drv->scan_fn);
>>>> + drv->scan_fn();
>>>> + /* Ignore all errors from this */
>>>> + }
>>>> +
>>>> return 0;
>>>> }
>>>> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>>> b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>>> index 0155025..c28e093 100644
>>>> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>>> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
>>>> @@ -175,7 +175,11 @@ DPDK_16.11 {
>>>> rte_eal_dev_attach;
>>>> rte_eal_dev_detach;
>>>> rte_eal_map_resource;
>>>> + rte_eal_soc_detach;
>>>> rte_eal_soc_dump;
>>>> + rte_eal_soc_match;
>>>> + rte_eal_soc_probe;
>>>> + rte_eal_soc_probe_one;
>>>> rte_eal_soc_register;
>>>> rte_eal_soc_unregister;
>>>> rte_eal_unmap_resource;
>>>> --
>>>> 2.7.4
>>>>
>>>
>>
>>
>> --
>> -
>> Shreyansh
>
2016-11-10 14:40, Shreyansh Jain:
> On Thursday 10 November 2016 01:11 PM, Jianbo Liu wrote:
> > On 10 November 2016 at 14:10, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> >> On Thursday 10 November 2016 09:00 AM, Jianbo Liu wrote:
> >>> I'm still not sure about the purpose of soc_scan, and how to use it.
> >>
> >>
> >> For each device to be used by DPDK, which cannot be scanned/identified using
> >> the existing PCI/VDEV methods (sysfs/bus/pci), 'soc_scan_t' provides a way
> >> for driver to make those devices part of device lists.
> >>
> >> Ideally, 'scan' is not a function of a driver. It is a bus function - which
> >> is missing in this case.
> >>
> >>> If it's for each driver, it should at least struct rte_soc_driver * as
> >>> its parameter.
> >>
> >>
> >> Its for each driver - assuming that each non-PCI driver which implements it
> >> knows how to find devices which it can control (for example, special area in
> >> sysfs, or even platform bus).
> >>
> >
> > Considering there are several drivers in a platform bus, each driver
> > call the scan function, like the rte_eal_soc_scan_platform_bus() you
> > implemented.
> > The first will add soc devices to the list, but the remaining calls
> > are redundant.
>
> Indeed. This is exactly the issue we will face if we try and move this
> scan/match logic to PCI - all devices are identified in one step.
>
> There is a difference in principle here:
> A SoC device/driver combination is essentially focused towards a single
> type of bus<->devices. For example, a NXP PMD would implement a scan
> function which would scan for all devices on NXP's bus. This would not
> conflict with another XYZ SoC PMD which scans its specific bus.
>
> There is caveat to this - the platform bus. There can be multiple
> drivers which can serve platform bus compliant devices. First
> PMD->scan() initiated for such a bus/device would leave all other scans
> redundant.
>
> More similar caveats will come if we consider somewhat generic buses. At
> least I couldn't find any interest for such devices in the ML when I
> picked this series (from where Jan left it).
>
> Probably when more common type of PMDs come in, some default scan
> implementation can check for skipping those devices which are already
> added. It would be redundant but harmless.
If several drivers use the same bus, it means the bus is standard enough
to be implemented in EAL. So the scan function of this bus should be
called only once when calling the generic EAL scan function.
On 10 November 2016 at 17:26, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:
> 2016-11-10 14:40, Shreyansh Jain:
>> On Thursday 10 November 2016 01:11 PM, Jianbo Liu wrote:
>> > On 10 November 2016 at 14:10, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>> >> On Thursday 10 November 2016 09:00 AM, Jianbo Liu wrote:
>> >>> I'm still not sure about the purpose of soc_scan, and how to use it.
>> >>
>> >>
>> >> For each device to be used by DPDK, which cannot be scanned/identified using
>> >> the existing PCI/VDEV methods (sysfs/bus/pci), 'soc_scan_t' provides a way
>> >> for driver to make those devices part of device lists.
>> >>
>> >> Ideally, 'scan' is not a function of a driver. It is a bus function - which
>> >> is missing in this case.
>> >>
>> >>> If it's for each driver, it should at least struct rte_soc_driver * as
>> >>> its parameter.
>> >>
>> >>
>> >> Its for each driver - assuming that each non-PCI driver which implements it
>> >> knows how to find devices which it can control (for example, special area in
>> >> sysfs, or even platform bus).
>> >>
>> >
>> > Considering there are several drivers in a platform bus, each driver
>> > call the scan function, like the rte_eal_soc_scan_platform_bus() you
>> > implemented.
>> > The first will add soc devices to the list, but the remaining calls
>> > are redundant.
>>
>> Indeed. This is exactly the issue we will face if we try and move this
>> scan/match logic to PCI - all devices are identified in one step.
>>
>> There is a difference in principle here:
>> A SoC device/driver combination is essentially focused towards a single
>> type of bus<->devices. For example, a NXP PMD would implement a scan
>> function which would scan for all devices on NXP's bus. This would not
>> conflict with another XYZ SoC PMD which scans its specific bus.
>>
>> There is caveat to this - the platform bus. There can be multiple
>> drivers which can serve platform bus compliant devices. First
>> PMD->scan() initiated for such a bus/device would leave all other scans
>> redundant.
>>
>> More similar caveats will come if we consider somewhat generic buses. At
>> least I couldn't find any interest for such devices in the ML when I
>> picked this series (from where Jan left it).
>>
>> Probably when more common type of PMDs come in, some default scan
>> implementation can check for skipping those devices which are already
>> added. It would be redundant but harmless.
>
> If several drivers use the same bus, it means the bus is standard enough
> to be implemented in EAL. So the scan function of this bus should be
> called only once when calling the generic EAL scan function.
Agree. rte_eal_soc_scan_platform_bus can be the scanning func for platform bus.
On Thursday 10 November 2016 02:56 PM, Thomas Monjalon wrote:
> 2016-11-10 14:40, Shreyansh Jain:
>> On Thursday 10 November 2016 01:11 PM, Jianbo Liu wrote:
>>> On 10 November 2016 at 14:10, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>>>> On Thursday 10 November 2016 09:00 AM, Jianbo Liu wrote:
>>>>> I'm still not sure about the purpose of soc_scan, and how to use it.
>>>>
>>>>
>>>> For each device to be used by DPDK, which cannot be scanned/identified using
>>>> the existing PCI/VDEV methods (sysfs/bus/pci), 'soc_scan_t' provides a way
>>>> for driver to make those devices part of device lists.
>>>>
>>>> Ideally, 'scan' is not a function of a driver. It is a bus function - which
>>>> is missing in this case.
>>>>
>>>>> If it's for each driver, it should at least struct rte_soc_driver * as
>>>>> its parameter.
>>>>
>>>>
>>>> Its for each driver - assuming that each non-PCI driver which implements it
>>>> knows how to find devices which it can control (for example, special area in
>>>> sysfs, or even platform bus).
>>>>
>>>
>>> Considering there are several drivers in a platform bus, each driver
>>> call the scan function, like the rte_eal_soc_scan_platform_bus() you
>>> implemented.
>>> The first will add soc devices to the list, but the remaining calls
>>> are redundant.
>>
>> Indeed. This is exactly the issue we will face if we try and move this
>> scan/match logic to PCI - all devices are identified in one step.
>>
>> There is a difference in principle here:
>> A SoC device/driver combination is essentially focused towards a single
>> type of bus<->devices. For example, a NXP PMD would implement a scan
>> function which would scan for all devices on NXP's bus. This would not
>> conflict with another XYZ SoC PMD which scans its specific bus.
>>
>> There is caveat to this - the platform bus. There can be multiple
>> drivers which can serve platform bus compliant devices. First
>> PMD->scan() initiated for such a bus/device would leave all other scans
>> redundant.
>>
>> More similar caveats will come if we consider somewhat generic buses. At
>> least I couldn't find any interest for such devices in the ML when I
>> picked this series (from where Jan left it).
>>
>> Probably when more common type of PMDs come in, some default scan
>> implementation can check for skipping those devices which are already
>> added. It would be redundant but harmless.
>
> If several drivers use the same bus, it means the bus is standard enough
> to be implemented in EAL. So the scan function of this bus should be
> called only once when calling the generic EAL scan function.
>
In the current model, without a bus like object, this can only be
implemented as a hack. This is because:
- If each driver has its scan, some of them (those on a common bus)
would have their scan nullified
- Then, EAL would initiate the scan on their behalf. (rte_eal_init)
- Whereas, for those drivers which are 'special' scan, EAL would have to
call each driver's scan.
- This selection is manual code change (nullifying the scan function).
- And then, EAL would have various 'generic' scan's chained together
other than driver->scan().
A cleaner model would have been that EAL always calls the drivers->scan().
Obviously, this issue vanishes as soon as we have the bus->scan() like
implementation where a bus would represent multiple devices/drivers.
@@ -87,14 +87,65 @@ static int test_compare_addr(void)
*/
struct test_wrapper {
struct rte_soc_driver soc_drv;
+ struct rte_soc_device soc_dev;
};
+static int empty_pmd0_probe(struct rte_soc_driver *drv,
+ struct rte_soc_device *dev);
+static int empty_pmd0_remove(struct rte_soc_device *dev);
+
+static void always_find_dev0_cb(void);
+static int match_dev0_by_name(struct rte_soc_driver *drv,
+ struct rte_soc_device *dev);
+
+static void always_find_dev1_cb(void);
+static int match_dev1_by_name(struct rte_soc_driver *drv,
+ struct rte_soc_device *dev);
+
+/**
+ * Dummy probe handler for PMD driver 'pmd0'.
+ *
+ * @param drv
+ * driver object
+ * @param dev
+ * device object
+ * @return
+ * 0 on success
+ */
+static int
+empty_pmd0_probe(struct rte_soc_driver *drv __rte_unused,
+ struct rte_soc_device *dev __rte_unused)
+{
+ return 0;
+}
+
+/**
+ * Remove handler for PMD driver 'pmd0'.
+ *
+ * @param dev
+ * device to remove
+ * @return
+ * 0 on success
+ */
+static int
+empty_pmd0_remove(struct rte_soc_device *dev)
+{
+ /* Release the memory associated with dev->addr.name */
+ free(dev->addr.name);
+
+ return 0;
+}
+
struct test_wrapper empty_pmd0 = {
.soc_drv = {
.driver = {
.name = "empty_pmd0"
},
- },
+ .probe = empty_pmd0_probe,
+ .remove = empty_pmd0_remove,
+ .scan_fn = always_find_dev0_cb,
+ .match_fn = match_dev0_by_name,
+ }
};
struct test_wrapper empty_pmd1 = {
@@ -102,9 +153,87 @@ struct test_wrapper empty_pmd1 = {
.driver = {
.name = "empty_pmd1"
},
+ .scan_fn = always_find_dev1_cb,
+ .match_fn = match_dev1_by_name,
},
};
+/**
+ * Bus scan by PMD 'pmd0' for adding device 'dev0'
+ *
+ * @param void
+ * @return void
+ */
+static void
+always_find_dev0_cb(void)
+{
+ /* SoC's scan would scan devices on its bus and add to
+ * soc_device_list
+ */
+ empty_pmd0.soc_dev.addr.name = strdup("empty_pmd0_dev");
+
+ TAILQ_INSERT_TAIL(&soc_device_list, &empty_pmd0.soc_dev, next);
+}
+
+/**
+ * Match device 'dev0' with driver PMD pmd0
+ *
+ * @param drv
+ * Driver with this matching needs to be done; unused here
+ * @param dev
+ * device to be matched against driver
+ * @return
+ * 0 on successful matched
+ * 1 if driver<=>device don't match
+ */
+static int
+match_dev0_by_name(struct rte_soc_driver *drv __rte_unused,
+ struct rte_soc_device *dev)
+{
+ if (!dev->addr.name || strcmp(dev->addr.name, "empty_pmd0_dev"))
+ return 0;
+
+ return 1;
+}
+
+/**
+ * Bus scan by PMD 'pmd0' for adding device 'dev1'
+ *
+ * @param void
+ * @return void
+ */
+static void
+always_find_dev1_cb(void)
+{
+ /* SoC's scan would scan devices on its bus and add to
+ * soc_device_list
+ */
+ empty_pmd0.soc_dev.addr.name = strdup("empty_pmd1_dev");
+
+ TAILQ_INSERT_TAIL(&soc_device_list, &empty_pmd1.soc_dev, next);
+}
+
+/**
+ * Match device 'dev1' with driver PMD pmd0
+ *
+ * @param drv
+ * Driver with this matching needs to be done; unused here
+ * @param dev
+ * device to be matched against driver
+ * @return
+ * 0 on successful matched
+ * 1 if driver<=>device don't match
+ */
+static int
+match_dev1_by_name(struct rte_soc_driver *drv __rte_unused,
+ struct rte_soc_device *dev)
+{
+ if (!dev->addr.name || strcmp(dev->addr.name, "empty_pmd1_dev"))
+ return 0;
+
+ return 1;
+}
+
static int
count_registered_socdrvs(void)
{
@@ -148,13 +277,68 @@ test_register_unregister(void)
return 0;
}
+/* Test Probe (scan and match) functionality */
+static int
+test_soc_scan_and_match(void)
+{
+ int drv_count = 0;
+ struct rte_soc_driver *drv;
+
+ /* Registering dummy drivers */
+ rte_eal_soc_register(&empty_pmd0.soc_drv);
+ rte_eal_soc_register(&empty_pmd1.soc_drv);
+ /* Assuming that test_register_unregister is working, not verifying
+ * that drivers are indeed registered
+ */
+
+ /* rte_eal_soc_init is called by rte_eal_init, which in turn calls the
+ * scan_fn of each driver.
+ */
+ TAILQ_FOREACH(drv, &soc_driver_list, next) {
+ if (drv && drv->scan_fn)
+ drv->scan_fn();
+ drv_count++;
+ }
+
+ /* rte_eal_init() would perform other inits here */
+
+ /* Probe would link the SoC devices<=>drivers */
+ rte_eal_soc_probe();
+
+ /* Unregistering dummy drivers */
+ rte_eal_soc_unregister(&empty_pmd0.soc_drv);
+ rte_eal_soc_unregister(&empty_pmd1.soc_drv);
+
+ /* Verify the Unregistering has removed the driver from list */
+ TAILQ_FOREACH(drv, &soc_driver_list, next) {
+ if (drv)
+ drv_count--;
+ }
+
+ free(empty_pmd0.soc_dev.addr.name);
+
+ /* If drv_count is anything other than 0, Unregistering failed */
+ if (drv_count) {
+ printf("%s has failed\n", __func__);
+ return 1;
+ }
+
+ printf("%s has been successful\n", __func__);
+ return 0;
+}
+
/* save real devices and drivers until the tests finishes */
struct soc_driver_list real_soc_driver_list =
TAILQ_HEAD_INITIALIZER(real_soc_driver_list);
+/* save real devices and drivers until the tests finishes */
+struct soc_device_list real_soc_device_list =
+ TAILQ_HEAD_INITIALIZER(real_soc_device_list);
+
static int test_soc_setup(void)
{
struct rte_soc_driver *drv;
+ struct rte_soc_device *dev;
/* no real drivers for the test */
while (!TAILQ_EMPTY(&soc_driver_list)) {
@@ -163,12 +347,20 @@ static int test_soc_setup(void)
TAILQ_INSERT_TAIL(&real_soc_driver_list, drv, next);
}
+ /* And, no real devices for the test */
+ while (!TAILQ_EMPTY(&soc_device_list)) {
+ dev = TAILQ_FIRST(&soc_device_list);
+ TAILQ_REMOVE(&soc_device_list, dev, next);
+ TAILQ_INSERT_TAIL(&real_soc_device_list, dev, next);
+ }
+
return 0;
}
static int test_soc_cleanup(void)
{
struct rte_soc_driver *drv;
+ struct rte_soc_device *dev;
/* bring back real drivers after the test */
while (!TAILQ_EMPTY(&real_soc_driver_list)) {
@@ -177,6 +369,13 @@ static int test_soc_cleanup(void)
rte_eal_soc_register(drv);
}
+ /* And, bring back real devices after the test */
+ while (!TAILQ_EMPTY(&real_soc_device_list)) {
+ dev = TAILQ_FIRST(&real_soc_device_list);
+ TAILQ_REMOVE(&real_soc_device_list, dev, next);
+ TAILQ_INSERT_TAIL(&soc_device_list, dev, next);
+ }
+
return 0;
}
@@ -192,6 +391,10 @@ test_soc(void)
if (test_register_unregister())
return -1;
+ /* Assuming test_register_unregister has succeeded */
+ if (test_soc_scan_and_match())
+ return -1;
+
if (test_soc_cleanup())
return -1;
@@ -171,7 +171,11 @@ DPDK_16.11 {
rte_eal_dev_attach;
rte_eal_dev_detach;
rte_eal_map_resource;
+ rte_eal_soc_detach;
rte_eal_soc_dump;
+ rte_eal_soc_match;
+ rte_eal_soc_probe;
+ rte_eal_soc_probe_one;
rte_eal_soc_register;
rte_eal_soc_unregister;
rte_eal_unmap_resource;
@@ -36,6 +36,8 @@
#include <sys/queue.h>
#include <rte_log.h>
+#include <rte_common.h>
+#include <rte_soc.h>
#include "eal_private.h"
@@ -45,6 +47,208 @@ struct soc_driver_list soc_driver_list =
struct soc_device_list soc_device_list =
TAILQ_HEAD_INITIALIZER(soc_device_list);
+int
+rte_eal_soc_match_compat(struct rte_soc_driver *drv,
+ struct rte_soc_device *dev)
+{
+ int i, j;
+
+ RTE_VERIFY(drv != NULL && drv->id_table != NULL);
+ RTE_VERIFY(dev != NULL && dev->id != NULL);
+
+ for (i = 0; drv->id_table[i].compatible; ++i) {
+ const char *drv_compat = drv->id_table[i].compatible;
+
+ for (j = 0; dev->id[j].compatible; ++j) {
+ const char *dev_compat = dev->id[j].compatible;
+
+ if (!strcmp(drv_compat, dev_compat))
+ return 0;
+ }
+ }
+
+ return 1;
+}
+
+static int
+rte_eal_soc_probe_one_driver(struct rte_soc_driver *drv,
+ struct rte_soc_device *dev)
+{
+ int ret = 1;
+
+ RTE_VERIFY(drv->match_fn);
+
+ ret = drv->match_fn(drv, dev);
+ if (ret) {
+ RTE_LOG(DEBUG, EAL,
+ " match function failed, skipping\n");
+ return ret;
+ }
+
+ dev->driver = drv;
+ RTE_VERIFY(drv->probe != NULL);
+ return drv->probe(drv, dev);
+}
+
+static int
+soc_probe_all_drivers(struct rte_soc_device *dev)
+{
+ struct rte_soc_driver *drv = NULL;
+ int rc = 0;
+
+ if (dev == NULL)
+ return -1;
+
+ TAILQ_FOREACH(drv, &soc_driver_list, next) {
+ rc = rte_eal_soc_probe_one_driver(drv, dev);
+ if (rc < 0)
+ /* negative value is an error */
+ return -1;
+ if (rc > 0)
+ /* positive value means driver doesn't support it */
+ continue;
+ return 0;
+ }
+ return 1;
+}
+
+/* If the IDs match, call the remove() function of the driver. */
+static int
+rte_eal_soc_detach_dev(struct rte_soc_driver *drv,
+ struct rte_soc_device *dev)
+{
+ int ret;
+
+ if ((drv == NULL) || (dev == NULL))
+ return -EINVAL;
+
+ ret = drv->match_fn(drv, dev);
+ if (ret) {
+ RTE_LOG(DEBUG, EAL, " driver (%s) didn't match device (%s)\n",
+ drv->driver.name, dev->addr.name);
+ return ret;
+ }
+
+ RTE_LOG(DEBUG, EAL, "SoC device %s\n",
+ dev->addr.name);
+
+ RTE_LOG(DEBUG, EAL, " remove driver: %s\n", drv->driver.name);
+
+ if (drv->remove && (drv->remove(dev) < 0))
+ return -1; /* negative value is an error */
+
+ /* clear driver structure */
+ dev->driver = NULL;
+
+ return 0;
+}
+
+/*
+ * Call the remove() function of all registered drivers for the device.
+ *
+ * @param dev
+ * Device for which detach is to be performed
+ * @return
+ * 0 when successful
+ * -1 if deinitialization fails
+ * 1 if no driver is found for this device.
+ */
+static int
+soc_detach_all_drivers(struct rte_soc_device *dev)
+{
+ struct rte_soc_driver *dr = NULL;
+ int rc = 0;
+
+ if (dev == NULL)
+ return -1;
+
+ TAILQ_FOREACH(dr, &soc_driver_list, next) {
+ rc = rte_eal_soc_detach_dev(dr, dev);
+ if (rc < 0)
+ /* negative value is an error */
+ return -1;
+ if (rc > 0)
+ /* positive value means driver doesn't support it */
+ continue;
+ return 0;
+ }
+ return 1;
+}
+
+int
+rte_eal_soc_detach(const struct rte_soc_addr *addr)
+{
+ struct rte_soc_device *dev = NULL;
+ int ret = 0;
+
+ if (addr == NULL)
+ return -1;
+
+ TAILQ_FOREACH(dev, &soc_device_list, next) {
+ if (rte_eal_compare_soc_addr(&dev->addr, addr))
+ continue;
+
+ ret = soc_detach_all_drivers(dev);
+ if (ret < 0)
+ goto err_return;
+
+ TAILQ_REMOVE(&soc_device_list, dev, next);
+ return 0;
+ }
+ return -1;
+
+err_return:
+ RTE_LOG(WARNING, EAL, "Requested device %s cannot be used\n",
+ dev->addr.name);
+ return -1;
+}
+
+int
+rte_eal_soc_probe_one(const struct rte_soc_addr *addr)
+{
+ struct rte_soc_device *dev = NULL;
+ int ret = 0;
+
+ if (addr == NULL)
+ return -1;
+
+ TAILQ_FOREACH(dev, &soc_device_list, next) {
+ if (rte_eal_compare_soc_addr(&dev->addr, addr))
+ continue;
+
+ ret = soc_probe_all_drivers(dev);
+ if (ret < 0)
+ goto err_return;
+ return 0;
+ }
+ return -1;
+
+err_return:
+ RTE_LOG(WARNING, EAL,
+ "Requested device %s cannot be used\n", addr->name);
+ return -1;
+}
+
+int
+rte_eal_soc_probe(void)
+{
+ struct rte_soc_device *dev = NULL;
+ int ret = 0;
+
+ TAILQ_FOREACH(dev, &soc_device_list, next) {
+ ret = soc_probe_all_drivers(dev);
+ if (ret < 0) {
+ RTE_LOG(DEBUG, EAL, "Requested device %s"
+ " cannot be used\n", dev->addr.name);
+ /* Failure for a particular device is logged and
+ * ignored
+ */
+ }
+ }
+
+ return ret;
+}
+
/* dump one device */
static int
soc_dump_one_device(FILE *f, struct rte_soc_device *dev)
@@ -61,7 +265,6 @@ soc_dump_one_device(FILE *f, struct rte_soc_device *dev)
return 0;
}
-/* dump devices on the bus to an output stream */
void
rte_eal_soc_dump(FILE *f)
{
@@ -75,14 +278,18 @@ rte_eal_soc_dump(FILE *f)
}
}
-/* register a driver */
void
rte_eal_soc_register(struct rte_soc_driver *driver)
{
+ /* For a valid soc driver, match and scan function
+ * should be provided.
+ */
+ RTE_VERIFY(driver != NULL);
+ RTE_VERIFY(driver->match_fn != NULL);
+ RTE_VERIFY(driver->scan_fn != NULL);
TAILQ_INSERT_TAIL(&soc_driver_list, driver, next);
}
-/* unregister a driver */
void
rte_eal_soc_unregister(struct rte_soc_driver *driver)
{
@@ -62,7 +62,6 @@ extern struct soc_device_list soc_device_list;
TAILQ_HEAD(soc_driver_list, rte_soc_driver); /**< SoC drivers in D-linked Q. */
TAILQ_HEAD(soc_device_list, rte_soc_device); /**< SoC devices in D-linked Q. */
-
struct rte_soc_id {
const char *compatible; /**< OF compatible specification */
uint64_t priv_data; /**< SoC Driver specific data */
@@ -97,6 +96,23 @@ typedef int (soc_probe_t)(struct rte_soc_driver *, struct rte_soc_device *);
typedef int (soc_remove_t)(struct rte_soc_device *);
/**
+ * SoC device scan callback, called from rte_eal_soc_init.
+ * For various SoC, the bus on which devices are attached maynot be compliant
+ * to a standard platform (or platform bus itself). In which case, extra
+ * steps are implemented by PMD to scan over the bus and add devices to SoC
+ * device list.
+ */
+typedef void (soc_scan_t)(void);
+
+/**
+ * Custom device<=>driver match callback for SoC
+ * Unlike PCI, SoC devices don't have a fixed definition of device
+ * identification. PMDs can implement a specific matching function in which
+ * driver and device objects are provided to perform custom match.
+ */
+typedef int (soc_match_t)(struct rte_soc_driver *, struct rte_soc_device *);
+
+/**
* A structure describing a SoC driver.
*/
struct rte_soc_driver {
@@ -104,6 +120,8 @@ struct rte_soc_driver {
struct rte_driver driver; /**< Inherit core driver. */
soc_probe_t *probe; /**< Device probe */
soc_remove_t *remove; /**< Device remove */
+ soc_scan_t *scan_fn; /**< Callback for scanning SoC bus*/
+ soc_match_t *match_fn; /**< Callback to match dev<->drv */
const struct rte_soc_id *id_table; /**< ID table, NULL terminated */
};
@@ -146,12 +164,63 @@ rte_eal_compare_soc_addr(const struct rte_soc_addr *a0,
}
/**
+ * Default function for matching the Soc driver with device. Each driver can
+ * either use this function or define their own soc matching function.
+ * This function relies on the compatible string extracted from sysfs. But,
+ * a SoC might have different way of identifying its devices. Such SoC can
+ * override match_fn.
+ *
+ * @return
+ * 0 on success
+ * -1 when no match found
+ */
+int
+rte_eal_soc_match_compat(struct rte_soc_driver *drv,
+ struct rte_soc_device *dev);
+
+/**
+ * Probe SoC devices for registered drivers.
+ *
+ * @return
+ * 0 on success
+ * !0 in case of any failure in probe
+ */
+int rte_eal_soc_probe(void);
+
+/**
+ * Probe the single SoC device.
+ */
+int rte_eal_soc_probe_one(const struct rte_soc_addr *addr);
+
+/**
+ * Close the single SoC device.
+ *
+ * Scan the SoC devices and find the SoC device specified by the SoC
+ * address, then call the remove() function for registered driver
+ * that has a matching entry in its id_table for discovered device.
+ *
+ * @param addr
+ * The SoC address to close.
+ * @return
+ * - 0 on success.
+ * - Negative on error.
+ */
+int rte_eal_soc_detach(const struct rte_soc_addr *addr);
+
+/**
* Dump discovered SoC devices.
+ *
+ * @param f
+ * File to dump device info in.
*/
void rte_eal_soc_dump(FILE *f);
/**
* Register a SoC driver.
+ *
+ * @param driver
+ * Object for SoC driver to register
+ * @return void
*/
void rte_eal_soc_register(struct rte_soc_driver *driver);
@@ -167,6 +236,10 @@ RTE_PMD_EXPORT_NAME(nm, __COUNTER__)
/**
* Unregister a SoC driver.
+ *
+ * @param driver
+ * Object for SoC driver to unregister
+ * @return void
*/
void rte_eal_soc_unregister(struct rte_soc_driver *driver);
@@ -70,6 +70,7 @@
#include <rte_cpuflags.h>
#include <rte_interrupts.h>
#include <rte_pci.h>
+#include <rte_soc.h>
#include <rte_dev.h>
#include <rte_devargs.h>
#include <rte_common.h>
@@ -890,6 +891,10 @@ rte_eal_init(int argc, char **argv)
if (rte_eal_pci_probe())
rte_panic("Cannot probe PCI\n");
+ /* Probe & Initialize SoC devices */
+ if (rte_eal_soc_probe())
+ rte_panic("Cannot probe SoC\n");
+
rte_eal_mcfg_complete();
return fctret;
@@ -44,13 +44,28 @@
#include <rte_log.h>
#include <rte_soc.h>
-#include "eal_internal_cfg.h"
-#include "eal_filesystem.h"
-#include "eal_private.h"
+#include <eal_internal_cfg.h>
+#include <eal_filesystem.h>
+#include <eal_private.h>
/* Init the SoC EAL subsystem */
int
rte_eal_soc_init(void)
{
+ struct rte_soc_driver *drv;
+
+ /* SoC is disabled by default */
+ if (!internal_config.enable_soc)
+ return 0;
+
+ /* For each registered driver, call their scan routine to perform any
+ * custom scan for devices (for example, custom buses)
+ */
+ TAILQ_FOREACH(drv, &soc_driver_list, next) {
+ RTE_VERIFY(drv->scan_fn);
+ drv->scan_fn();
+ /* Ignore all errors from this */
+ }
+
return 0;
}
@@ -175,7 +175,11 @@ DPDK_16.11 {
rte_eal_dev_attach;
rte_eal_dev_detach;
rte_eal_map_resource;
+ rte_eal_soc_detach;
rte_eal_soc_dump;
+ rte_eal_soc_match;
+ rte_eal_soc_probe;
+ rte_eal_soc_probe_one;
rte_eal_soc_register;
rte_eal_soc_unregister;
rte_eal_unmap_resource;