[dpdk-dev,v9,6/9] eal: auto detect iova mode
Checks
Commit Message
For auto detection purpose:
* Below calls moved up in the eal initialization order:
- eal_option_device_parse
- rte_bus_scan
Based on the result of rte_bus_scan_iommu_class - select iova
mapping mode.
Signed-off-by: Santosh Shukla <santosh.shukla@caviumnetworks.com>
Signed-off-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Reviewed-by: Anatoly Burakov <anatoly.burakov@intel.com>
Tested-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
v6 --> v7:
- Moved eal_option_device_parse() up in then order of eal init.
- Added run_once. (aaron suggestion).
- squashed v6 series patch no. [08/12] & [09/12] into one patch (Aaron
comment)
lib/librte_eal/bsdapp/eal/eal.c | 27 ++++++++++++++++-----------
lib/librte_eal/linuxapp/eal/eal.c | 27 ++++++++++++++++-----------
2 files changed, 32 insertions(+), 22 deletions(-)
Comments
20/09/2017 13:23, Santosh Shukla:
> For auto detection purpose:
> * Below calls moved up in the eal initialization order:
> - eal_option_device_parse
> - rte_bus_scan
>
> Based on the result of rte_bus_scan_iommu_class - select iova
> mapping mode.
It does not explain why you need to move things up.
On Friday 06 October 2017 05:49 AM, Thomas Monjalon wrote:
> 20/09/2017 13:23, Santosh Shukla:
>> For auto detection purpose:
>> * Below calls moved up in the eal initialization order:
>> - eal_option_device_parse
>> - rte_bus_scan
>>
>> Based on the result of rte_bus_scan_iommu_class - select iova
>> mapping mode.
> It does not explain why you need to move things up.
For that one should understand eal_init sequence first.
Should know about _option_device_parse and rte_bus_scan() dependency.
After that bus_scan is a need for _get_iommu_class() of api to know that
- kdrv is igb/uio/vfio etc.. That's why. Refer work history.
Again V9 series happened not for fun. I diagress on your comment.
06/10/2017 05:25, santosh:
>
> On Friday 06 October 2017 05:49 AM, Thomas Monjalon wrote:
> > 20/09/2017 13:23, Santosh Shukla:
> >> For auto detection purpose:
> >> * Below calls moved up in the eal initialization order:
> >> - eal_option_device_parse
> >> - rte_bus_scan
> >>
> >> Based on the result of rte_bus_scan_iommu_class - select iova
> >> mapping mode.
> > It does not explain why you need to move things up.
>
> For that one should understand eal_init sequence first.
> Should know about _option_device_parse and rte_bus_scan() dependency.
>
> After that bus_scan is a need for _get_iommu_class() of api to know that
> - kdrv is igb/uio/vfio etc.. That's why. Refer work history.
> Again V9 series happened not for fun. I diagress on your comment.
This is the basics of writing a commit log.
You have to explain why things are done.
You move things because of dependencies without explaining them.
And I'm pretty sure this move will cause big troubles.
For instance, have you tried shared library mode?
One more comment, you are considering only devices scanned at initialization.
What happens when a new device is plugged in?
I can push it as is, given there are some Reviewed-by and Tested-by.
I am trying to avoid you a revert of this patch when one will discover
some major bugs.
But I wonder whether it's worth given how you welcome it.
On Friday 06 October 2017 01:41 PM, Thomas Monjalon wrote:
> 06/10/2017 05:25, santosh:
>> On Friday 06 October 2017 05:49 AM, Thomas Monjalon wrote:
>>> 20/09/2017 13:23, Santosh Shukla:
>>>> For auto detection purpose:
>>>> * Below calls moved up in the eal initialization order:
>>>> - eal_option_device_parse
>>>> - rte_bus_scan
>>>>
>>>> Based on the result of rte_bus_scan_iommu_class - select iova
>>>> mapping mode.
>>> It does not explain why you need to move things up.
>> For that one should understand eal_init sequence first.
>> Should know about _option_device_parse and rte_bus_scan() dependency.
>>
>> After that bus_scan is a need for _get_iommu_class() of api to know that
>> - kdrv is igb/uio/vfio etc.. That's why. Refer work history.
>> Again V9 series happened not for fun. I diagress on your comment.
> This is the basics of writing a commit log.
> You have to explain why things are done.
> You move things because of dependencies without explaining them.
Agree, But if reader does reading from 0..5, by then he could understand
" auto detection purpose" reasoning.
Anyways, I'll add more context in patch summary in v10...sending..
> And I'm pretty sure this move will cause big troubles.
> For instance, have you tried shared library mode?
Its builds, also testpmd works.
> One more comment, you are considering only devices scanned at initialization.
> What happens when a new device is plugged in?
Should work.
in vfio mode: if PMDs(for that device) flag set to IOVA_AS_VA flag then newly
bound device will have iova=_va mapping mode.
Or else iova=_pa.
Thanks.
> I can push it as is, given there are some Reviewed-by and Tested-by.
> I am trying to avoid you a revert of this patch when one will discover
> some major bugs.
> But I wonder whether it's worth given how you welcome it.
>
@@ -541,6 +541,22 @@ rte_eal_init(int argc, char **argv)
return -1;
}
+ if (eal_option_device_parse()) {
+ rte_errno = ENODEV;
+ rte_atomic32_clear(&run_once);
+ return -1;
+ }
+
+ if (rte_bus_scan()) {
+ rte_eal_init_alert("Cannot scan the buses for devices\n");
+ rte_errno = ENODEV;
+ rte_atomic32_clear(&run_once);
+ return -1;
+ }
+
+ /* autodetect the iova mapping mode (default is iova_pa) */
+ rte_eal_get_configuration()->iova_mode = rte_bus_get_iommu_class();
+
if (internal_config.no_hugetlbfs == 0 &&
internal_config.process_type != RTE_PROC_SECONDARY &&
eal_hugepage_info_init() < 0) {
@@ -620,17 +636,6 @@ rte_eal_init(int argc, char **argv)
rte_config.master_lcore, thread_id, cpuset,
ret == 0 ? "" : "...");
- if (eal_option_device_parse()) {
- rte_errno = ENODEV;
- return -1;
- }
-
- if (rte_bus_scan()) {
- rte_eal_init_alert("Cannot scan the buses for devices\n");
- rte_errno = ENODEV;
- return -1;
- }
-
RTE_LCORE_FOREACH_SLAVE(i) {
/*
@@ -798,6 +798,22 @@ rte_eal_init(int argc, char **argv)
return -1;
}
+ if (eal_option_device_parse()) {
+ rte_errno = ENODEV;
+ rte_atomic32_clear(&run_once);
+ return -1;
+ }
+
+ if (rte_bus_scan()) {
+ rte_eal_init_alert("Cannot scan the buses for devices\n");
+ rte_errno = ENODEV;
+ rte_atomic32_clear(&run_once);
+ return -1;
+ }
+
+ /* autodetect the iova mapping mode (default is iova_pa) */
+ rte_eal_get_configuration()->iova_mode = rte_bus_get_iommu_class();
+
if (internal_config.no_hugetlbfs == 0 &&
internal_config.process_type != RTE_PROC_SECONDARY &&
internal_config.xen_dom0_support == 0 &&
@@ -895,17 +911,6 @@ rte_eal_init(int argc, char **argv)
return -1;
}
- if (eal_option_device_parse()) {
- rte_errno = ENODEV;
- return -1;
- }
-
- if (rte_bus_scan()) {
- rte_eal_init_alert("Cannot scan the buses for devices\n");
- rte_errno = ENODEV;
- return -1;
- }
-
RTE_LCORE_FOREACH_SLAVE(i) {
/*