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

On Thu, Jan 21, 2016 at 8:16 PM, Thomas Monjalon
[off-list ref] wrote:

2016-01-21 17:34, Santosh Shukla:

quoted

On Thu, Jan 21, 2016 at 4:58 PM, Thomas Monjalon
[off-list ref] wrote:

quoted

2016-01-21 16:43, Santosh Shukla:

quoted

David Marchand [off-list ref] wrote:

quoted

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.

quoted

And Yes it is called VFIO but with with specifics appended in it.

quoted

quoted

quoted

How come we need to distinguish between with/without iommu modes ?

By default vfio framework assumes iommu i.,e., iommu present. Unless user
explicitly set "enable_unsafe_noiommu_mode" param. so in my opinion, we
care to parse vfio driver for _noiommu_ mode only.

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

quoted

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.

quoted

Currently virtio-net
pmd driver does resource parsing then resource init for interfaces
like UIO/ioport, I intend to do same but only parsing, as resource
init for vfio case already taken care by pci_xx_vfio_map() api in
virtio-net pmd driver (refer Yaun recently virtio 1.0 recently
submitted rte_eal_pci_map patch)

Also Yuan in one of earlier thread inclined to remove all the resource
parsing api from virtio-net pmd driver. Pl. refer this thread [1]

[1] http://dpdk.org/dev/patchwork/patch/9862/

Yes he said
"we should try to avoid getting UIO/VFIO stuff inside virtio pmd driver".
I agree we must try to have those abstractions in EAL.

Right,

IMO vfio now has that abstraction in place and pmd driver only to
check driver type, which virtio-net pmd doing for vfio case. `

The only exception seems to be the switch ioport/PCI bar to read/write
in virtio.

Right, and only applicable for x86 arch, wont work for non-x86 archs.

quoted

quoted

Even if virtio cannot work in an IOMMU case, there is no reason to add
a VFIO_NOIOMMU type here.

quoted

quoted

Should not vfio behave the same way from an api point of view ?

Yes It should. vfio gives similar file_ops i.e.. read/write/mmap/seek etc..
I am little confused on your question, do you see any issue in vfio bar
rd/wr api implementation?

I think you should just consider the VFIO API and let the noiommu option
as a kernel configuration detail.

vfio apis _are_ considered at low level rd/wr implementation, Has
nothing to do with iommu/noiommu mode. See pci_vfio_read/write_bar()
implementation, they are using pread64/pwrite64() vfio rd/wr api.

So you agree that the VFIO API abstract the iommu availability details?

didn't understood your question,