Re: [PATCH v6 08/11] eal: pci: introduce RTE_KDRV_VFIO_NOIOMMUi driver mode

[Thomas Monjalon <thomas.monjalon@6wind.com>]

2016-01-21 22:47, Santosh Shukla:
> On Thu, Jan 21, 2016 at 8:16 PM, Thomas Monjalon
> <thomas.monjalon@6wind.com> wrote:
> > 2016-01-21 17:34, Santosh Shukla:
> >> On Thu, Jan 21, 2016 at 4:58 PM, Thomas Monjalon
> >> <thomas.monjalon@6wind.com> wrote:
> >> > 2016-01-21 16:43, Santosh Shukla:
> >> >> David Marchand <david.marchand@6wind.com> wrote:
> >> >> > This is a mode (specific to vfio), not a new kernel driver.
> >> >> >
> >> >> Yes, Specific to VFIO and this is why noiommu appended after vfio i.e..
> >> >> __VFIO and __VFIO_NOIOMMU.
> >> >
> >> > Woaaa! Your logic is really disappointing :)
> >> > Specific to VFIO => append _NOIOMMU
> >> > If it's for VFIO, it should be called VFIO (that's my logic).
> >> >
> >> I am confused by reading your comment. vfio works for default iommu
> >> and now with noiommu. drv->kdrv need to know driver mode for vfio
> >> case. So that user can simply read drv->kdrv value in their driver and
> >> accordingly use vfio rd/wr api for example {pread/pwrite}. This is how
> >> rte_eal_pci_vfio_read/write_bar() api implemented.
> >
> > Sorry I don't understand. Why EAL read/write functions should be different
> > depending of the VFIO mode?
> 
> no, EAL rd/wr functions are not different for vfio or vfio modes {same
> for iommu or noiommu}. Pl. see pci_eal_read/write_bar() api. Those
> apis currently used for VFIO, Irrespective of vfio mode. If required,
> we can add UIO bar_rd/wr api too. pci_eal_rd/wr_bar() are abstract
> apis. Underneath implementation can be vfio or uio type.

It means you agree the suffix _NOIOMMU is not needed?
It seems we go nowhere in this discussion. You said
"drv->kdrv need to know driver mode for vfio"
and after
"Those apis currently used for VFIO, Irrespective of vfio mode"
That's why I assume your first assumption was wrong.

> >> > Why do we care to parse noiommu only?
> >>
> >> Because pmd drivers example virtio can work with vfio only in
> >> _noiommu_ mode. In particular, virtio spec 0.95 / legacy virtio.
> >
> > Please could you explain the limitation (except IOMMU availability)?
> 
> Ok.
> 
> I believe - we both agree that noiommu mode is a need for pmd drivers
> like virtio, right? if so then other reason is implementation driven

No, noiommu is a need for some environment having no IOMMU.
But in my understanding, virtio could run with a nested IOMMU.

> i.e..
> 
> Pl. look at virtio_pci.c in this patch.. VIRTIO_RD/WR/_1/2/4()
> implementation. They are in-use and applicable to  virtio spec 0.95,
> so far support uio/ioport-way rd/wr. Now to support vfio-way rd/wr -
> need to check drv->kdrv value, that value should be of vfio_noiommu
> types __not__  generic _vfio types.

I still don't understand why it would not work with VFIO w/IOMMU.

> >> So at
> >> the initialization (example .. virtio-net) of such pmd driver, pmd
> >> driver should know that vfio-with-noiommu mode enabled or not? for
> >> that pmd driver simply checks drv->kdrv value.
> >
> > If a check is needed, I would prefer using your function
> > pci_vfio_is_noiommu() and remove driver modes from struct rte_kernel_driver.
> 
> I don't think calling pci_vfio_no_iommu() inside
> virtio_reg_rd/wr_1/2/3() would be a good idea.

Why? The value may be cached in the priv properties.