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

[Jerin Jacob Kollanukkaran <jerinj@marvell.com>]

> -----Original Message-----
> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> Sent: Tuesday, July 9, 2019 7:00 PM
> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; David Marchand
> <david.marchand@redhat.com>
> Cc: dev <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>; Ben
> Walker <benjamin.walker@intel.com>
> Subject: Re: [EXT] Re: [dpdk-dev] [PATCH] bus/pci: fix IOVA as VA mode
> selection
> 
> On 09-Jul-19 1:11 PM, Jerin Jacob Kollanukkaran wrote:
> >> -----Original Message-----
> >> From: Burakov, Anatoly <anatoly.burakov@intel.com>
> >> Sent: Tuesday, July 9, 2019 5:10 PM
> >> To: Jerin Jacob Kollanukkaran <jerinj@marvell.com>; David Marchand
> >> <david.marchand@redhat.com>
> >> Cc: dev <dev@dpdk.org>; Thomas Monjalon <thomas@monjalon.net>;
> Ben
> >> Walker <benjamin.walker@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@marvell.com> wrote:
> >>>>> From: Jerin Jacob <mailto:jerinj@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".

It is "need" as performance is horrible without it as is per packet SW translation.
A "need" for DPDK performance perspective.

> 
> >>
> >> 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).

For KNI, we already submitted a patch to support IOVA as VA.
I don’t know about igb_uio, if IOVA as PA, we might as well disable IOMMU.
Is igb_uio enables IOMMU? I don’t see any reference.
grep -ri "iommu" kernel/linux/igb_uio/

Again, it is not device attribute, it is system attribute. In current KNI case it
Fall backs to PA irrespective of device capability so we don’t need any
separate flag from driver.

Even if we introduce a flag, what it is supposed to do?

> 
> > 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

So in what category i40e driver comes? By default, pci bus should return PA for class.
If VA supported then return VA.
So how new flag will help? 

> 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.

What's your view on this existing problem?


> >
> >
> >>
> >> [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