On 2020-03-23 23:19, Jerin Jacob wrote:
On Mon, Mar 23, 2020 at 11:11 PM dwilder <dwil...@us.ibm.com> wrote:

Thanks you for your review Jerin.  See my responses are inline.

On 2020-03-20 06:24, Jerin Jacob wrote:
> On Fri, Feb 21, 2020 at 4:22 AM David Wilder <dwil...@us.ibm.com>
> wrote:
>>
>> If --no-huge is set and iova-mode has not been specified force VA
>> mode.
>> If --no-huge and --iova-mode=PA is requested error out as this is
>> an impossible configuration.
>>
>> Signed-off-by: David Wilder <dwil...@us.ibm.com>
>> ---
>>  lib/librte_eal/linux/eal/eal.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/lib/librte_eal/linux/eal/eal.c
>> b/lib/librte_eal/linux/eal/eal.c
>> index 9530ee55f..d3a0a1731 100644
>> --- a/lib/librte_eal/linux/eal/eal.c
>> +++ b/lib/librte_eal/linux/eal/eal.c
>> @@ -1062,9 +1062,16 @@ rte_eal_init(int argc, char **argv)
>>
>>         /* if no EAL option "--iova-mode=<pa|va>", use bus IOVA scheme
>> */
>>         if (internal_config.iova_mode == RTE_IOVA_DC) {
>> +
>>                 /* autodetect the IOVA mapping mode */
>>                 enum rte_iova_mode iova_mode =
>> rte_bus_get_iommu_class();
>>
>> +               if (iova_mode == RTE_IOVA_PA &&
>> !rte_eal_has_hugepages()) {
>> +                       iova_mode = RTE_IOVA_VA;

>
> What if igb_uio or vfio_nommu has been loaded(i.e no iommu support
> enabled from the driver)? This would fail.

Yes they would fail.  If igb_uio or vfio_nommu (or any driver) cant be
forced to VA mode it cant be used with out hugepages.  Drivers can be
available but not used therefor we print a warning message.

I think, the warning will not be enough as the system will fail anyway.

iova_mode == RTE_IOVA_PA && rte_eal_has_hugepages() == 0 && no_iommu == 1
case, we need to return error.

iova_mode == RTE_IOVA_PA && rte_eal_has_hugepages() == 0 && no_iommu == 0
case warning is enough.


I have a simpler solution.

The goal here is to make --no-huge work when at least one bus/driver wants PA mode.
A user can always override the selected mode with --iova-mode=va.
So why not just make --no-huge the same as "--no-huge --iovs-mode=va" ?

I am thinking:

@@ -1060,6 +1060,11 @@ rte_eal_init(int argc, char **argv)

        phys_addrs = rte_eal_using_phys_addrs() != 0;

+       if (!phys_addrs) {
+               internal_config.iova_mode = RTE_IOVA_VA;
+ RTE_LOG(INFO, EAL, "Physical addresses are unavailable, selecting IOVA as VA mode.\n");
+       }
+
/* if no EAL option "--iova-mode=<pa|va>", use bus IOVA scheme */
        if (internal_config.iova_mode == RTE_IOVA_DC) {
                /* autodetect the IOVA mapping mode */

If a device cant run in VA mode, it will fail to initialize and report why.


>
>> +                       RTE_LOG(WARNING, EAL, "Some buses want 'PA'
>> but forcing 'VA' because --no-huge is requested.\n");
>> +                       RTE_LOG(WARNING, EAL, "Not all buses may be
>> able to initialize.\n");
>> +               }
>> +
>>                 if (iova_mode == RTE_IOVA_DC) {
>>                         RTE_LOG(DEBUG, EAL, "Buses did not request a
>> specific IOVA mode.\n");
>>
>> @@ -1111,6 +1118,13 @@ rte_eal_init(int argc, char **argv)
>>                         internal_config.iova_mode;
>>         }
>>
>> +       if (rte_eal_iova_mode() == RTE_IOVA_PA &&
>> +           rte_eal_has_hugepages() == 0) {
>> +               rte_eal_init_alert("Cannot use IOVA as 'PA' with
>> --no-huge");
>
> Top of the tree already detecting this case. am I missing anything?
>
> [master]dell[dpdk.org] $ sudo ./build/app/test/dpdk-test  -c 0x3
> --no-huge --iova-mode=pa
> EAL: Detected 56 lcore(s)
> EAL: Detected 2 NUMA nodes
> EAL: Static memory layout is selected, amount of reserved memory can
> be adjusted with -m or --socket-mem
> EAL: Multi-process socket /var/run/dpdk/rte/mp_socket
> EAL: FATAL: Cannot use IOVA as 'PA' since physical addresses are not
> available
> EAL: Cannot use IOVA as 'PA' since physical addresses are not available
>

The check you reference is reporting that physical address are not
available, for example no permissions to read /proc/self/pagemap.  In
this case, if --no-huge is set then PA mode is not allowed. There is no guarantee that physical address are persistent with out using hugepages.

Since this check is under the following, Yes, make sense for the check.
The old command has explicit  --iova-mode=pa. So it is in the
different code paths.

/* if no EAL option "--iova-mode=<pa|va>", use bus IOVA scheme */
        if (internal_config.iova_mode == RTE_IOVA_DC) {



>> +               rte_errno = EINVAL;
>> +               return -1;
>> +       }
>> +
>>         if (rte_eal_iova_mode() == RTE_IOVA_PA && !phys_addrs) {
>>                 rte_eal_init_alert("Cannot use IOVA as 'PA' since
>> physical addresses are not available");
>>                 rte_errno = EINVAL;
>> --
>> 2.25.0
>>

Reply via email to