[dpdk-dev,v5,01/12] eal/bus: introduce bus abstraction
Checks
Commit Message
This patch introduces the rte_bus abstraction for devices and drivers in
EAL framework. The model is:
- One or more buses are connected to a CPU (or core)
- One or more devices are conneted to a Bus
- Drivers are running instances which manage one or more devices
- Bus is responsible for identifying devices (and interrupt propogation)
- Driver is responsible for initializing the device
This patch adds a 'rte_bus' class which rte_driver and rte_device refer.
This way, each device (rte_xxx_device) would have reference to the bus
it is based on. As well as, each driver (rte_xxx_driver) would have link
to the bus and devices on it for servicing.
__ rte_bus_list
/
+----------'---+
|rte_bus |
| driver_list------> List of rte_bus specific
| device_list---- devices
| | `-> List of rte_bus associated
| | drivers
+--|------|----+
_________/ \_________
+--------/----+ +-\---------------+
|rte_device | |rte_driver |
| rte_bus | | rte_bus |
| rte_driver | | ... |
| ... | +---------...-----+
| | |||
+---||--------+ |||
|| |||
| \ \\\
| \_____________ \\\
| \ |||
+------|---------+ +----|----------+ |||
|rte_pci_device | |rte_xxx_device | |||
| .... | | .... | |||
+----------------+ +---------------+ / | \
/ | \
_____________________/ / \
/ ___/ \
+-------------'--+ +------------'---+ +--'------------+
|rte_pci_driver | |rte_vdev_driver | |rte_xxx_driver |
| .... | | .... | | .... |
+----------------+ +----------------+ +---------------+
This patch only enables the bus references on rte_driver and rte_driver.
EAL wide global device and driver list continue to exist until an instance
of bus is added in subsequent patches.
This patch also introduces RTE_REGISTER_BUS macro on the lines of
RTE_PMD_REGISTER_XXX. Key difference is that the constructor priority has
been explicitly set to 101 so as to execute bus registration before PMD.
Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
lib/librte_eal/bsdapp/eal/Makefile | 1 +
lib/librte_eal/bsdapp/eal/rte_eal_version.map | 16 ++
lib/librte_eal/common/Makefile | 2 +-
lib/librte_eal/common/eal_common_bus.c | 190 ++++++++++++++++++++++++
lib/librte_eal/common/include/rte_bus.h | 177 ++++++++++++++++++++++
lib/librte_eal/common/include/rte_dev.h | 2 +
lib/librte_eal/linuxapp/eal/Makefile | 1 +
lib/librte_eal/linuxapp/eal/rte_eal_version.map | 16 ++
8 files changed, 404 insertions(+), 1 deletion(-)
create mode 100644 lib/librte_eal/common/eal_common_bus.c
create mode 100644 lib/librte_eal/common/include/rte_bus.h
Comments
2016-12-26 18:53, Shreyansh Jain:
> +DPDK_17.02 {
> + global:
> +
> + rte_bus_list;
> + rte_eal_bus_add_device;
> + rte_eal_bus_add_driver;
> + rte_eal_bus_get;
> + rte_eal_bus_dump;
> + rte_eal_bus_register;
> + rte_eal_bus_insert_device;
> + rte_eal_bus_remove_device;
> + rte_eal_bus_remove_driver;
> + rte_eal_bus_unregister;
I think the prefix can be just rte_bus_ instead of rte_eal_bus_.
> +/** Double linked list of buses */
> +TAILQ_HEAD(rte_bus_list, rte_bus);
> +
> +/* Global Bus list */
> +extern struct rte_bus_list rte_bus_list;
Why the bus list is public?
> +/**
> + * A structure describing a generic bus.
> + */
> +struct rte_bus {
> + TAILQ_ENTRY(rte_bus) next; /**< Next bus object in linked list */
> + struct rte_driver_list driver_list;
> + /**< List of all drivers on bus */
> + struct rte_device_list device_list;
> + /**< List of all devices on bus */
> + const char *name; /**< Name of the bus */
> +};
I am not convinced we should link a generic bus to drivers and devices.
What do you think of having rte_pci_bus being a rte_bus and linking
with rte_pci_device and rte_pci_driver lists?
I'm thinking to something like that:
struct rte_bus {
TAILQ_ENTRY(rte_bus) next;
const char *name;
rte_bus_scan_t scan;
rte_bus_match_t match;
};
struct rte_pci_bus {
struct rte_bus bus;
struct rte_pci_driver_list pci_drivers;
struct rte_pci_device_list pci_devices;
};
> +/** Helper for Bus registration. The constructor has higher priority than
> + * PMD constructors
> + */
> +#define RTE_REGISTER_BUS(nm, bus) \
> +static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \
> +{\
> + (bus).name = RTE_STR(nm);\
> + rte_eal_bus_register(&bus); \
> +}
By removing the lists from rte_bus as suggested above, do you still need
a priority for this constructor?
> struct rte_device {
> TAILQ_ENTRY(rte_device) next; /**< Next device */
> + struct rte_bus *bus; /**< Device connected to this bus */
> const struct rte_driver *driver;/**< Associated driver */
> int numa_node; /**< NUMA node connection */
> struct rte_devargs *devargs; /**< Device user arguments */
> @@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
> */
> struct rte_driver {
> TAILQ_ENTRY(rte_driver) next; /**< Next in list. */
> + struct rte_bus *bus; /**< Bus serviced by this driver */
> const char *name; /**< Driver name. */
> const char *alias; /**< Driver alias. */
> };
Do we need to know the bus associated to a driver in rte_driver?
Bus and driver are already associated in rte_device.
On Wednesday 04 January 2017 03:22 AM, Thomas Monjalon wrote:
> 2016-12-26 18:53, Shreyansh Jain:
>> +DPDK_17.02 {
>> + global:
>> +
>> + rte_bus_list;
>> + rte_eal_bus_add_device;
>> + rte_eal_bus_add_driver;
>> + rte_eal_bus_get;
>> + rte_eal_bus_dump;
>> + rte_eal_bus_register;
>> + rte_eal_bus_insert_device;
>> + rte_eal_bus_remove_device;
>> + rte_eal_bus_remove_driver;
>> + rte_eal_bus_unregister;
>
> I think the prefix can be just rte_bus_ instead of rte_eal_bus_.
Ok. I will change these.
I thought as these symbols are part of librte_eal/*, naming should
reflect that.
>
>> +/** Double linked list of buses */
>> +TAILQ_HEAD(rte_bus_list, rte_bus);
>> +
>> +/* Global Bus list */
>> +extern struct rte_bus_list rte_bus_list;
>
> Why the bus list is public?
I will revisit this. When I made it public, intention was that some
element external to EAL (e.g. drivers/bus) might require to iterate
over this list. But, as of now I don't think any such user/case exists
(except test*).
>
>> +/**
>> + * A structure describing a generic bus.
>> + */
>> +struct rte_bus {
>> + TAILQ_ENTRY(rte_bus) next; /**< Next bus object in linked list */
>> + struct rte_driver_list driver_list;
>> + /**< List of all drivers on bus */
>> + struct rte_device_list device_list;
>> + /**< List of all devices on bus */
>> + const char *name; /**< Name of the bus */
>> +};
>
> I am not convinced we should link a generic bus to drivers and devices.
> What do you think of having rte_pci_bus being a rte_bus and linking
> with rte_pci_device and rte_pci_driver lists?
This is different from what I had in mind.
You are saying:
Class: rte_bus
`-> No object instantiated for this class
Class: rte_pci_bus inheriting rte_bus
`-> object instantiated for this class.
Here, rte_bus is being treated as an abstract class which is only
inherited and rte_pci_bus is the base class which is instantiated.
And I was thinking:
Class: rte_bus
`-> Object: pci_bus (defined in */eal/eal_pci.c)
Here, rte_bus is that base class which is instantiated.
I agree that what you are suggesting is inline with current model:
rte_device -> abstract class (no one instantiates it)
`-> rte_pci_device -> Base class which inherits rte_device and
is instantiated.
When I choose not to create rte_pci_bus, it was because I didn't want
another indirection in form of rte_bus->rte_pci_bus->object.
There were no 'non-generic' bus functions which were only applicable for
rte_pci_bus. Eventually, rte_pci_bus ended up being a direct inheritance
of rte_bus.
>
> I'm thinking to something like that:
>
> struct rte_bus {
> TAILQ_ENTRY(rte_bus) next;
> const char *name;
> rte_bus_scan_t scan;
> rte_bus_match_t match;
> };
> struct rte_pci_bus {
> struct rte_bus bus;
> struct rte_pci_driver_list pci_drivers;
> struct rte_pci_device_list pci_devices;
> };
if we go by rte_bus->rte_pci_bus->(instance of rte_pci_bus), above is
fine. Though, I am in favor of rte_bus->(instance of rte_bus for PCI)
because I don't see any 'non-generic' information in rte_pci_bus which
can't be put in rte_bus.
>
>> +/** Helper for Bus registration. The constructor has higher priority than
>> + * PMD constructors
>> + */
>> +#define RTE_REGISTER_BUS(nm, bus) \
>> +static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \
>> +{\
>> + (bus).name = RTE_STR(nm);\
>> + rte_eal_bus_register(&bus); \
>> +}
>
> By removing the lists from rte_bus as suggested above, do you still need
> a priority for this constructor?
I think yes.
Even if we have rte_pci_bus as a class, object of rte_bus should be part
of Bus list *before* registration of a driver (because, driver
registration searches for bus by name).
(This is assuming that no global PCI/VDEV/XXX bus object is created
which is directly used within all PCI specific bus operations).
There was another suggestion on list which was to check for existence of
bus _within_ each driver registration and create/instantiate an object
in case no bus is registered. I didn't like the approach so I didn't use
it. From David [1], and me [2]
[1] http://dpdk.org/ml/archives/dev/2016-December/051689.html
[2] http://dpdk.org/ml/archives/dev/2016-December/051698.html
>
>> struct rte_device {
>> TAILQ_ENTRY(rte_device) next; /**< Next device */
>> + struct rte_bus *bus; /**< Device connected to this bus */
>> const struct rte_driver *driver;/**< Associated driver */
>> int numa_node; /**< NUMA node connection */
>> struct rte_devargs *devargs; /**< Device user arguments */
>> @@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
>> */
>> struct rte_driver {
>> TAILQ_ENTRY(rte_driver) next; /**< Next in list. */
>> + struct rte_bus *bus; /**< Bus serviced by this driver */
>> const char *name; /**< Driver name. */
>> const char *alias; /**< Driver alias. */
>> };
>
> Do we need to know the bus associated to a driver in rte_driver?
> Bus and driver are already associated in rte_device.
>
>
Two reasons:
1/ A driver should be associated with a bus so that if required, all bus
can be directly extracted - even when probing has not been done.
2/ device->driver would only be updated after probe. device->driver->bus
would not be valid in such cases, if required.
Overall, I don't have objections for rte_bus->rte_pci_bus=>object as
compared to rte_bus=>PCI-object. But, I would still like to get a final
confirmation of a more preferred way.
Meanwhile, I will make changes to accommodate this change to save time
in case rte_pci_bus class is final/preferred method.
-
Shreyansh
2017-01-06 16:01, Shreyansh Jain:
> On Wednesday 04 January 2017 03:22 AM, Thomas Monjalon wrote:
> > 2016-12-26 18:53, Shreyansh Jain:
> >> +/**
> >> + * A structure describing a generic bus.
> >> + */
> >> +struct rte_bus {
> >> + TAILQ_ENTRY(rte_bus) next; /**< Next bus object in linked list */
> >> + struct rte_driver_list driver_list;
> >> + /**< List of all drivers on bus */
> >> + struct rte_device_list device_list;
> >> + /**< List of all devices on bus */
> >> + const char *name; /**< Name of the bus */
> >> +};
> >
> > I am not convinced we should link a generic bus to drivers and devices.
> > What do you think of having rte_pci_bus being a rte_bus and linking
> > with rte_pci_device and rte_pci_driver lists?
>
> This is different from what I had in mind.
> You are saying:
>
> Class: rte_bus
> `-> No object instantiated for this class
> Class: rte_pci_bus inheriting rte_bus
> `-> object instantiated for this class.
>
> Here, rte_bus is being treated as an abstract class which is only
> inherited and rte_pci_bus is the base class which is instantiated.
>
> And I was thinking:
>
> Class: rte_bus
> `-> Object: pci_bus (defined in */eal/eal_pci.c)
>
> Here, rte_bus is that base class which is instantiated.
>
> I agree that what you are suggesting is inline with current model:
> rte_device -> abstract class (no one instantiates it)
> `-> rte_pci_device -> Base class which inherits rte_device and
> is instantiated.
Yes
> When I choose not to create rte_pci_bus, it was because I didn't want
> another indirection in form of rte_bus->rte_pci_bus->object.
> There were no 'non-generic' bus functions which were only applicable for
> rte_pci_bus. Eventually, rte_pci_bus ended up being a direct inheritance
> of rte_bus.
>
> > I'm thinking to something like that:
> >
> > struct rte_bus {
> > TAILQ_ENTRY(rte_bus) next;
> > const char *name;
> > rte_bus_scan_t scan;
> > rte_bus_match_t match;
> > };
> > struct rte_pci_bus {
> > struct rte_bus bus;
> > struct rte_pci_driver_list pci_drivers;
> > struct rte_pci_device_list pci_devices;
> > };
>
> if we go by rte_bus->rte_pci_bus->(instance of rte_pci_bus), above is
> fine. Though, I am in favor of rte_bus->(instance of rte_bus for PCI)
> because I don't see any 'non-generic' information in rte_pci_bus which
> can't be put in rte_bus.
The lists of drivers and devices are specific to the bus.
Your proposal was to list them as generic rte_driver/rte_device and
cast them. I'm just saying we can directly declare them with the right type,
e.g. rte_pci_driver/rte_pci_device.
In the same logic, the functions probe/remove are specifics for the bus and
should be declared in rte_pci_driver instead of the generic rte_driver.
> >> +/** Helper for Bus registration. The constructor has higher priority than
> >> + * PMD constructors
> >> + */
> >> +#define RTE_REGISTER_BUS(nm, bus) \
> >> +static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \
> >> +{\
> >> + (bus).name = RTE_STR(nm);\
> >> + rte_eal_bus_register(&bus); \
> >> +}
> >
> > By removing the lists from rte_bus as suggested above, do you still need
> > a priority for this constructor?
>
> I think yes.
> Even if we have rte_pci_bus as a class, object of rte_bus should be part
> of Bus list *before* registration of a driver (because, driver
> registration searches for bus by name).
>
> (This is assuming that no global PCI/VDEV/XXX bus object is created
> which is directly used within all PCI specific bus operations).
>
> There was another suggestion on list which was to check for existence of
> bus _within_ each driver registration and create/instantiate an object
> in case no bus is registered. I didn't like the approach so I didn't use
> it. From David [1], and me [2]
>
> [1] http://dpdk.org/ml/archives/dev/2016-December/051689.html
> [2] http://dpdk.org/ml/archives/dev/2016-December/051698.html
OK, we can keep your approach of prioritize bus registrations.
If we see an issue later, we could switch to a bus registration done
when a first driver is registered on the bus.
> >> struct rte_device {
> >> TAILQ_ENTRY(rte_device) next; /**< Next device */
> >> + struct rte_bus *bus; /**< Device connected to this bus */
> >> const struct rte_driver *driver;/**< Associated driver */
> >> int numa_node; /**< NUMA node connection */
> >> struct rte_devargs *devargs; /**< Device user arguments */
> >> @@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
> >> */
> >> struct rte_driver {
> >> TAILQ_ENTRY(rte_driver) next; /**< Next in list. */
> >> + struct rte_bus *bus; /**< Bus serviced by this driver */
> >> const char *name; /**< Driver name. */
> >> const char *alias; /**< Driver alias. */
> >> };
> >
> > Do we need to know the bus associated to a driver in rte_driver?
> > Bus and driver are already associated in rte_device.
>
> Two reasons:
> 1/ A driver should be associated with a bus so that if required, all bus
> can be directly extracted - even when probing has not been done.
I do not understand this need.
> 2/ device->driver would only be updated after probe. device->driver->bus
> would not be valid in such cases, if required.
We can update device->driver on match.
Please let's do not over-engineer if not needed.
In this case, I think we can skip rte_driver->bus.
> Overall, I don't have objections for rte_bus->rte_pci_bus=>object as
> compared to rte_bus=>PCI-object. But, I would still like to get a final
> confirmation of a more preferred way.
>
> Meanwhile, I will make changes to accommodate this change to save time
> in case rte_pci_bus class is final/preferred method.
It looks more natural to me to avoid class casting and use specialized classes
when possible. So yes I prefer instantiating rte_pci_bus.
However, I could be wrong, and will consider any argument.
Thanks
On Friday 06 January 2017 08:25 PM, Thomas Monjalon wrote:
> 2017-01-06 16:01, Shreyansh Jain:
>> On Wednesday 04 January 2017 03:22 AM, Thomas Monjalon wrote:
>>> 2016-12-26 18:53, Shreyansh Jain:
>>>> +/**
>>>> + * A structure describing a generic bus.
>>>> + */
>>>> +struct rte_bus {
>>>> + TAILQ_ENTRY(rte_bus) next; /**< Next bus object in linked list */
>>>> + struct rte_driver_list driver_list;
>>>> + /**< List of all drivers on bus */
>>>> + struct rte_device_list device_list;
>>>> + /**< List of all devices on bus */
>>>> + const char *name; /**< Name of the bus */
>>>> +};
>>>
>>> I am not convinced we should link a generic bus to drivers and devices.
>>> What do you think of having rte_pci_bus being a rte_bus and linking
>>> with rte_pci_device and rte_pci_driver lists?
>>
>> This is different from what I had in mind.
>> You are saying:
>>
>> Class: rte_bus
>> `-> No object instantiated for this class
>> Class: rte_pci_bus inheriting rte_bus
>> `-> object instantiated for this class.
>>
>> Here, rte_bus is being treated as an abstract class which is only
>> inherited and rte_pci_bus is the base class which is instantiated.
>>
>> And I was thinking:
>>
>> Class: rte_bus
>> `-> Object: pci_bus (defined in */eal/eal_pci.c)
>>
>> Here, rte_bus is that base class which is instantiated.
>>
>> I agree that what you are suggesting is inline with current model:
>> rte_device -> abstract class (no one instantiates it)
>> `-> rte_pci_device -> Base class which inherits rte_device and
>> is instantiated.
>
> Yes
>
>> When I choose not to create rte_pci_bus, it was because I didn't want
>> another indirection in form of rte_bus->rte_pci_bus->object.
>> There were no 'non-generic' bus functions which were only applicable for
>> rte_pci_bus. Eventually, rte_pci_bus ended up being a direct inheritance
>> of rte_bus.
>>
>>> I'm thinking to something like that:
>>>
>>> struct rte_bus {
>>> TAILQ_ENTRY(rte_bus) next;
>>> const char *name;
>>> rte_bus_scan_t scan;
>>> rte_bus_match_t match;
>>> };
>>> struct rte_pci_bus {
>>> struct rte_bus bus;
>>> struct rte_pci_driver_list pci_drivers;
>>> struct rte_pci_device_list pci_devices;
>>> };
>>
>> if we go by rte_bus->rte_pci_bus->(instance of rte_pci_bus), above is
>> fine. Though, I am in favor of rte_bus->(instance of rte_bus for PCI)
>> because I don't see any 'non-generic' information in rte_pci_bus which
>> can't be put in rte_bus.
>
> The lists of drivers and devices are specific to the bus.
> Your proposal was to list them as generic rte_driver/rte_device and
> cast them. I'm just saying we can directly declare them with the right type,
> e.g. rte_pci_driver/rte_pci_device.
Ok. I get your point. Already changing the code to reflect this.
>
> In the same logic, the functions probe/remove are specifics for the bus and
> should be declared in rte_pci_driver instead of the generic rte_driver.
Yes, I agree with this after above argument.
>
>
>>>> +/** Helper for Bus registration. The constructor has higher priority than
>>>> + * PMD constructors
>>>> + */
>>>> +#define RTE_REGISTER_BUS(nm, bus) \
>>>> +static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \
>>>> +{\
>>>> + (bus).name = RTE_STR(nm);\
>>>> + rte_eal_bus_register(&bus); \
>>>> +}
>>>
>>> By removing the lists from rte_bus as suggested above, do you still need
>>> a priority for this constructor?
>>
>> I think yes.
>> Even if we have rte_pci_bus as a class, object of rte_bus should be part
>> of Bus list *before* registration of a driver (because, driver
>> registration searches for bus by name).
>>
>> (This is assuming that no global PCI/VDEV/XXX bus object is created
>> which is directly used within all PCI specific bus operations).
>>
>> There was another suggestion on list which was to check for existence of
>> bus _within_ each driver registration and create/instantiate an object
>> in case no bus is registered. I didn't like the approach so I didn't use
>> it. From David [1], and me [2]
>>
>> [1] http://dpdk.org/ml/archives/dev/2016-December/051689.html
>> [2] http://dpdk.org/ml/archives/dev/2016-December/051698.html
>
> OK, we can keep your approach of prioritize bus registrations.
> If we see an issue later, we could switch to a bus registration done
> when a first driver is registered on the bus.
Thanks for confirmation.
>
>
>>>> struct rte_device {
>>>> TAILQ_ENTRY(rte_device) next; /**< Next device */
>>>> + struct rte_bus *bus; /**< Device connected to this bus */
>>>> const struct rte_driver *driver;/**< Associated driver */
>>>> int numa_node; /**< NUMA node connection */
>>>> struct rte_devargs *devargs; /**< Device user arguments */
>>>> @@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
>>>> */
>>>> struct rte_driver {
>>>> TAILQ_ENTRY(rte_driver) next; /**< Next in list. */
>>>> + struct rte_bus *bus; /**< Bus serviced by this driver */
>>>> const char *name; /**< Driver name. */
>>>> const char *alias; /**< Driver alias. */
>>>> };
>>>
>>> Do we need to know the bus associated to a driver in rte_driver?
>>> Bus and driver are already associated in rte_device.
>>
>> Two reasons:
>> 1/ A driver should be associated with a bus so that if required, all bus
>> can be directly extracted - even when probing has not been done.
>
> I do not understand this need.
For example, Looping over all drivers for plugging them out. We need to
know which bus a driver is on so that we can unplug the devices
associated with the driver on that bus.
>
>> 2/ device->driver would only be updated after probe. device->driver->bus
>> would not be valid in such cases, if required.
>
> We can update device->driver on match.
Yes, we can.
>
> Please let's do not over-engineer if not needed.
> In this case, I think we can skip rte_driver->bus.
Hm, Ok. This was more of prospective step. We can avoid it without much
impact. I will change the code.
>
>
>> Overall, I don't have objections for rte_bus->rte_pci_bus=>object as
>> compared to rte_bus=>PCI-object. But, I would still like to get a final
>> confirmation of a more preferred way.
>>
>> Meanwhile, I will make changes to accommodate this change to save time
>> in case rte_pci_bus class is final/preferred method.
>
> It looks more natural to me to avoid class casting and use specialized classes
> when possible. So yes I prefer instantiating rte_pci_bus.
> However, I could be wrong, and will consider any argument.
Ok. I will go with your argument - mostly because I am OK either way and
we can always come back if framework changes are stable.
>
> Thanks
>
On 12/26/2016 1:23 PM, Shreyansh Jain wrote:
<...>
> +
> +DPDK_17.02 {
> + global:
> +
> + rte_bus_list;
> + rte_eal_bus_add_device;
> + rte_eal_bus_add_driver;
> + rte_eal_bus_get;
> + rte_eal_bus_dump;
> + rte_eal_bus_register;
> + rte_eal_bus_insert_device;
This function added in patch 3/12, it can be good to add this function
into .map file in that patch.
<...>
Hello Ferruh,
On Monday 09 January 2017 08:52 PM, Ferruh Yigit wrote:
> On 12/26/2016 1:23 PM, Shreyansh Jain wrote:
>
> <...>
>
>> +
>> +DPDK_17.02 {
>> + global:
>> +
>> + rte_bus_list;
>> + rte_eal_bus_add_device;
>> + rte_eal_bus_add_driver;
>> + rte_eal_bus_get;
>> + rte_eal_bus_dump;
>> + rte_eal_bus_register;
>
>> + rte_eal_bus_insert_device;
>
> This function added in patch 3/12, it can be good to add this function
> into .map file in that patch.
Yes, I caught this while rebasing for v6. In fact, now this function
itself has been removed.
Nevertheless, thanks for reviewing.
>
> <...>
>
>
-
Shreyansh
@@ -78,6 +78,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_cpuflags.c
SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_string_fns.c
SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_hexdump.c
SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_devargs.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_bus.c
SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_dev.c
SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_options.c
SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_thread.c
@@ -174,3 +174,19 @@ DPDK_16.11 {
rte_eal_vdrv_unregister;
} DPDK_16.07;
+
+DPDK_17.02 {
+ global:
+
+ rte_bus_list;
+ rte_eal_bus_add_device;
+ rte_eal_bus_add_driver;
+ rte_eal_bus_get;
+ rte_eal_bus_dump;
+ rte_eal_bus_register;
+ rte_eal_bus_insert_device;
+ rte_eal_bus_remove_device;
+ rte_eal_bus_remove_driver;
+ rte_eal_bus_unregister;
+
+} DPDK_16.11;
@@ -38,7 +38,7 @@ INC += rte_per_lcore.h rte_random.h
INC += rte_tailq.h rte_interrupts.h rte_alarm.h
INC += rte_string_fns.h rte_version.h
INC += rte_eal_memconfig.h rte_malloc_heap.h
-INC += rte_hexdump.h rte_devargs.h rte_dev.h rte_vdev.h
+INC += rte_hexdump.h rte_devargs.h rte_bus.h rte_dev.h rte_vdev.h
INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
INC += rte_malloc.h rte_keepalive.h rte_time.h
new file mode 100644
@@ -0,0 +1,190 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright(c) 2016 NXP
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of NXP nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <inttypes.h>
+#include <sys/queue.h>
+
+#include <rte_bus.h>
+#include <rte_devargs.h>
+#include <rte_debug.h>
+
+#include "eal_private.h"
+
+struct rte_bus_list rte_bus_list =
+ TAILQ_HEAD_INITIALIZER(rte_bus_list);
+
+/** @internal
+ * Add a device to a bus.
+ */
+void
+rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev)
+{
+ RTE_VERIFY(bus);
+ RTE_VERIFY(dev);
+
+ TAILQ_INSERT_TAIL(&bus->device_list, dev, next);
+ dev->bus = bus;
+}
+
+/** @internal
+ * Remove a device from its bus.
+ */
+void
+rte_eal_bus_remove_device(struct rte_device *dev)
+{
+ struct rte_bus *bus;
+
+ RTE_VERIFY(dev);
+ RTE_VERIFY(dev->bus);
+
+ bus = dev->bus;
+ TAILQ_REMOVE(&bus->device_list, dev, next);
+ dev->bus = NULL;
+}
+
+/** @internal
+ * Associate a driver with a bus.
+ */
+void
+rte_eal_bus_add_driver(struct rte_bus *bus, struct rte_driver *drv)
+{
+ RTE_VERIFY(bus);
+ RTE_VERIFY(drv);
+
+ TAILQ_INSERT_TAIL(&bus->driver_list, drv, next);
+ drv->bus = bus;
+}
+
+/** @internal
+ * Disassociate a driver from bus.
+ */
+void
+rte_eal_bus_remove_driver(struct rte_driver *drv)
+{
+ struct rte_bus *bus;
+
+ RTE_VERIFY(drv);
+ RTE_VERIFY(drv->bus);
+
+ bus = drv->bus;
+ TAILQ_REMOVE(&bus->driver_list, drv, next);
+ drv->bus = NULL;
+}
+
+/**
+ * Get the bus handle using its name
+ */
+struct rte_bus *
+rte_eal_bus_get(const char *bus_name)
+{
+ struct rte_bus *bus;
+
+ RTE_VERIFY(bus_name);
+
+ TAILQ_FOREACH(bus, &rte_bus_list, next) {
+ RTE_VERIFY(bus->name);
+
+ if (!strcmp(bus_name, bus->name))
+ return bus;
+ }
+
+ /* Unable to find bus requested */
+ return NULL;
+}
+
+/* register a bus */
+void
+rte_eal_bus_register(struct rte_bus *bus)
+{
+ RTE_VERIFY(bus);
+ RTE_VERIFY(bus->name && strlen(bus->name));
+
+ /* Initialize the driver and device list associated with the bus */
+ TAILQ_INIT(&(bus->driver_list));
+ TAILQ_INIT(&(bus->device_list));
+
+ TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
+ RTE_LOG(INFO, EAL, "Registered [%s] bus.\n", bus->name);
+}
+
+/* unregister a bus */
+void
+rte_eal_bus_unregister(struct rte_bus *bus)
+{
+ /* All devices and drivers associated with the bus should have been
+ * 'device->uninit' and 'driver->remove()' already.
+ */
+ RTE_VERIFY(TAILQ_EMPTY(&(bus->driver_list)));
+ RTE_VERIFY(TAILQ_EMPTY(&(bus->device_list)));
+
+ /* TODO: For each device, call its rte_device->driver->remove()
+ * and rte_eal_bus_remove_driver()
+ */
+
+ TAILQ_REMOVE(&rte_bus_list, bus, next);
+ RTE_LOG(INFO, EAL, "Unregistered [%s] bus.\n", bus->name);
+}
+
+/* dump one bus info */
+static int
+bus_dump_one(FILE *f, struct rte_bus *bus)
+{
+ int ret;
+
+ /* For now, dump only the bus name */
+ ret = fprintf(f, " %s\n", bus->name);
+
+ /* Error in case of inability in writing to stream */
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+void
+rte_eal_bus_dump(FILE *f)
+{
+ int ret;
+ struct rte_bus *bus;
+
+ TAILQ_FOREACH(bus, &rte_bus_list, next) {
+ ret = bus_dump_one(f, bus);
+ if (ret) {
+ RTE_LOG(ERR, EAL, "Unable to write to stream (%d)\n",
+ ret);
+ break;
+ }
+ }
+}
new file mode 100644
@@ -0,0 +1,177 @@
+/*-
+ * BSD LICENSE
+ *
+ * Copyright(c) 2016 NXP
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of NXP nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_BUS_H_
+#define _RTE_BUS_H_
+
+/**
+ * @file
+ *
+ * RTE PMD Bus Abstraction interfaces
+ *
+ * This file exposes APIs and Interfaces for Bus Abstraction over the devices
+ * drivers in EAL.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdio.h>
+#include <sys/queue.h>
+
+#include <rte_log.h>
+#include <rte_dev.h>
+
+/** Double linked list of buses */
+TAILQ_HEAD(rte_bus_list, rte_bus);
+
+/* Global Bus list */
+extern struct rte_bus_list rte_bus_list;
+
+/**
+ * A structure describing a generic bus.
+ */
+struct rte_bus {
+ TAILQ_ENTRY(rte_bus) next; /**< Next bus object in linked list */
+ struct rte_driver_list driver_list;
+ /**< List of all drivers on bus */
+ struct rte_device_list device_list;
+ /**< List of all devices on bus */
+ const char *name; /**< Name of the bus */
+};
+
+/** @internal
+ * Add a device to a bus.
+ *
+ * @param bus
+ * Bus on which device is to be added
+ * @param dev
+ * Device handle
+ * @return
+ * None
+ */
+void
+rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev);
+
+/** @internal
+ * Remove a device from its bus.
+ *
+ * @param dev
+ * Device handle to remove
+ * @return
+ * None
+ */
+void
+rte_eal_bus_remove_device(struct rte_device *dev);
+
+/** @internal
+ * Associate a driver with a bus.
+ *
+ * @param bus
+ * Bus on which driver is to be added
+ * @param dev
+ * Driver handle
+ * @return
+ * None
+ */
+void
+rte_eal_bus_add_driver(struct rte_bus *bus, struct rte_driver *drv);
+
+/** @internal
+ * Disassociate a driver from its bus.
+ *
+ * @param dev
+ * Driver handle to remove
+ * @return
+ * None
+ */
+void
+rte_eal_bus_remove_driver(struct rte_driver *drv);
+
+/**
+ * Register a Bus handler.
+ *
+ * @param bus
+ * A pointer to a rte_bus structure describing the bus
+ * to be registered.
+ */
+void rte_eal_bus_register(struct rte_bus *bus);
+
+/**
+ * Unregister a Bus handler.
+ *
+ * @param bus
+ * A pointer to a rte_bus structure describing the bus
+ * to be unregistered.
+ */
+void rte_eal_bus_unregister(struct rte_bus *bus);
+
+/**
+ * Obtain handle for bus given its name.
+ *
+ * @param bus_name
+ * Name of the bus handle to search
+ * @return
+ * Pointer to Bus object if name matches any registered bus object
+ * NULL, if no matching bus found
+ */
+struct rte_bus *rte_eal_bus_get(const char *bus_name);
+
+/**
+ * Dump information of all the buses registered with EAL.
+ *
+ * @param f
+ * A valid and open output stream handle
+ *
+ * @return
+ * 0 in case of success
+ * !0 in case there is error in opening the output stream
+ */
+void rte_eal_bus_dump(FILE *f);
+
+/** Helper for Bus registration. The constructor has higher priority than
+ * PMD constructors
+ */
+#define RTE_REGISTER_BUS(nm, bus) \
+static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \
+{\
+ (bus).name = RTE_STR(nm);\
+ rte_eal_bus_register(&bus); \
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_BUS_H */
@@ -122,6 +122,7 @@ struct rte_driver;
*/
struct rte_device {
TAILQ_ENTRY(rte_device) next; /**< Next device */
+ struct rte_bus *bus; /**< Device connected to this bus */
const struct rte_driver *driver;/**< Associated driver */
int numa_node; /**< NUMA node connection */
struct rte_devargs *devargs; /**< Device user arguments */
@@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
*/
struct rte_driver {
TAILQ_ENTRY(rte_driver) next; /**< Next in list. */
+ struct rte_bus *bus; /**< Bus serviced by this driver */
const char *name; /**< Driver name. */
const char *alias; /**< Driver alias. */
};
@@ -87,6 +87,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_cpuflags.c
SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_string_fns.c
SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_hexdump.c
SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_devargs.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_bus.c
SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_dev.c
SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_options.c
SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_thread.c
@@ -178,3 +178,19 @@ DPDK_16.11 {
rte_eal_vdrv_unregister;
} DPDK_16.07;
+
+DPDK_17.02 {
+ global:
+
+ rte_bus_list;
+ rte_eal_bus_add_device;
+ rte_eal_bus_add_driver;
+ rte_eal_bus_get;
+ rte_eal_bus_dump;
+ rte_eal_bus_register;
+ rte_eal_bus_insert_device;
+ rte_eal_bus_remove_device;
+ rte_eal_bus_remove_driver;
+ rte_eal_bus_unregister;
+
+} DPDK_16.11;