[dpdk-dev] [EXT] Re: [PATCH] bus/pci: fix IOVA as VA mode selection

[Burakov, Anatoly]

On 09-Jul-19 1:11 PM, Jerin Jacob Kollanukkaran wrote:
>> -----Original Message-----
>> From: Burakov, Anatoly <anatoly.burakov at intel.com>
>> Sent: Tuesday, July 9, 2019 5:10 PM
>> To: Jerin Jacob Kollanukkaran <jerinj at marvell.com>; David Marchand
>> <david.marchand at redhat.com>
>> Cc: dev <dev at dpdk.org>; Thomas Monjalon <thomas at monjalon.net>; Ben
>> Walker <benjamin.walker at intel.com>
>> Subject: Re: [EXT] Re: [dpdk-dev] [PATCH] bus/pci: fix IOVA as VA mode
>> selection
>>>>> ________________________________________
>>>>>
>>>>> On Mon, Jul 8, 2019 at 4:25 PM <mailto:jerinj at marvell.com> wrote:
>>>>> From: Jerin Jacob <mailto:jerinj at marvell.com>
>>>>>
>>>>> Existing logic fails to select IOVA mode as VA if driver request to
>>>>> enable IOVA as VA.
>>>>>
>>>>> IOVA as VA has more strict requirement than other modes, so enabling
>>>>> positive logic for IOVA as VA selection.
>>>>>
>>>>> This patch also updates the default IOVA mode as PA for PCI devices
>>>>> as it has to deal with DMA engines unlike the virtual devices that
>>>>> may need only IOVA as DC.
>>>>>
>>>>> We have three cases:
>>>>> - driver/hw supports IOVA as PA only
>>>>>
>>>>> [Jerin] It is not driver cap, it is more of system cap(IOMMU vs non
>>>>> IOMMU). We are already addressing that case
>>>>
>>>> I don't get how this works. How does "system capability" affect what
>>>> the device itself supports? Are we to assume that *all* hardware
>>>> support IOVA as VA by default? "System capability" is more of a bus
>>>> issue than an individual device issue, is it not?
>>>
>>> What I meant is, supporting VA vs PA is function of IOMMU(not the device
>> attribute).
>>> Ie. Device makes the  bus master request, if IOMMU available and
>>> enabled in the SYSTEM , It goes over IOMMU  and translate the IOVA to
>> physical address.
>>>
>>> Another way to put is, Is there any _PCIe_ device which need/requires
>>> RTE_PCI_DRV_NEED_IOVA_AS_PA in rte_pci_driver.drv_flags
>>>
>>>
>>
>> Previously, as far as i can tell, the flag was used to indicate support for IOVA
>> as VA mode, not *requirement* for IOVA as VA mode. For example, there
>> are multiple patches [1][2][3][4] (i'm sure i can find more!) that added IOVA
>> as VA support to various drivers, and they all were worded it in this exact way
>> - "support for IOVA as VA mode", not "require IOVA as VA mode". As far as i
>> can tell, none of these drivers *require* IOVA as VA mode - they merely use
>> this flag to indicate support for it.
> 
> Some class of devices NEED IOVA as VA for performance reasons.
> Specially the devices has HW mempool allocators. On those devices If we don’t use IOVA as VA,
> Upon getting packet from device, It needs to go over rte_mem_iova2virt() per
> packet see driver/net/dppa2. Which has real performance issue.

I wouldn't classify this as "needing" IOVA. "Need" implies it cannot 
work without it, whereas in this case it's more of a "highly 
recommended" rather than "need".

>>
>> Now suddenly it turns out that someone somewhere "knew" that "IOVA as
>> VA" flag in PCI drivers is supposed to indicate *requirement* and not
>> support, and it appears that this knowledge was not communicated nor
>> documented anywhere, and is now treated as common knowledge.
> 
> I think, the confusion here is,  I was under impression that
> # If device supports IOVA as VA and system runs with IOMMU then
> the  dpdk should run in IOVA as VA mode.
> If above statement true then we don’t really need a new flag.

Exactly. And the flag used to indicate that the device *supports* IOVA 
as VA, not that it *requires* it.

> 
> Couple of points to make forward progress:
> # If we think, there is a use case where device is IOVA as VA
> And system runs in IOMMU mode then for some reason DPDK needs
> to run in PA mode. If so, we need to create two flags
> RTE_PCI_DRV_IOVA_AS_VA - it can run either modes

There are use cases - KNI and igb_uio come to mind. Whether IOMMU uses 
VA or PA is a different from whether IOMMU is in use - there is no law 
that states that, when using IOMMU, IOVA have to have 1:1 mapping with 
VA. IOMMU requirement does not necessarily imply IOVA as VA - it is 
perfectly legal to program IOMMU to use IOVA as PA (which we currently 
do when we e.g. use VFIO for some devices and igb_uio for others).

> RTE_PCI_DRV_NEED_IOVA_AS_VA - it can run only on IOVA as VA

If we're adding a flag, we might as well not create a confusion and do 
it consistently. If IOVA as PA is supported, have a flag to indicate 
that. If IOVA as VA is supported, have a flag to indicate that. Absence 
of either flag implies inability to work in that mode. I don't see how 
this is less clear and self-documenting than having two IOVA as 
VA-related flags that have slightly different meaning and imply things 
not otherwise stated explicitly.

> # With top of tree, Currently it never runs in IOVA as VA mode.
> That’s a separate problem to fix. Which effect all the devices
> Currently supporting RTE_PCI_DRV_IOVA_AS_VA. Ie even though
> Device support RTE_PCI_DRV_IOVA_AS_VA, it is not running
> With IOMMU protection and/or root privilege is required to run DPDK.
> 
> 
>>
>> [1] http://patchwork.dpdk.org/patch/53206/
>> [2] http://patchwork.dpdk.org/patch/50274/
>> [3] http://patchwork.dpdk.org/patch/50991/
>> [4] http://patchwork.dpdk.org/patch/46134/
>>
>> --
>> Thanks,
>> Anatoly


-- 
Thanks,
Anatoly