Re: [dpdk-dev] [PATCH v6 6/8] virtio: add vfio api to rd/wr ioport space

[Santosh Shukla <sshukla@mvista.com>]

On Mon, Feb 1, 2016 at 6:18 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> On Fri, Jan 29, 2016 at 11:51:55PM +0530, Santosh Shukla wrote:
>> For vfio case - Use pread/pwrite api to access virtio
>> ioport space.
>>
>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>> Signed-off-by: Rizwan Ansari <ransari@mvista.com>
>> Signed-off-by: Rakesh Krishnamurthy <rakeshk@mvista.com>
>> ---
>> v5-->v6:
>> - renamed inport_in/out to vfio_in/out
>> - Renamed file from virtio_vfio_rw.h to virtio_vfio_io.h
>>
>>  drivers/net/virtio/virtio_vfio_io.h |  104 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 104 insertions(+)
>>  create mode 100644 drivers/net/virtio/virtio_vfio_io.h
>>
>> diff --git a/drivers/net/virtio/virtio_vfio_io.h b/drivers/net/virtio/virtio_vfio_io.h
>> new file mode 100644
>> index 0000000..218d4ed
>> --- /dev/null
>> +++ b/drivers/net/virtio/virtio_vfio_io.h
> ...
>> @@ -0,0 +1,104 @@
>> +#ifndef _VIRTIO_VFIO_IO_H_
>> +#define _VIRTIO_VFIO_IO_H_
>> +
>> +#include "virtio_logs.h"
>> +#if defined(RTE_EAL_VFIO) && defined(RTE_LIBRTE_EAL_LINUXAPP)
>

Yes, Now that we have pci_eal_read/write_bar() dummy implementation
for freebsd so we don't need both checks. I overlooked it, sending v7
for this patch now. Thanks.

> Won't it cause build failure if above "#if ..." is false, as
> virtio_read/write_reg_x() reference them unconditionally?
>
> BTW, why above check is needed? We have rte_eal_pci_read/write_bar()
> implementation with both VFIO and BSD, don't we?
>
>
>> +#endif /* _VIRTIO_VFIO_RW_H_ */
>                          ^^^^
> You forgot to do rename here.
>

My bad (:-
>
> BTW, I didn't follow the noIOMMU discussion; how did it end? Do we still
> need that? Is this patch a full story to enable virtio on ARM?
>
Ok, We agreed that explicit __noiommu suffix not required, atleast for
rte_xx_drv struct{}, as because sooner than later we'll have virtio
working for both flavours ie... iommu/noiommu. My only worry was
parsing for _noiommu and default vfio case, as because noiomu needed
user to preset "enable_noiommu_" param for vfio driver to do mode
switch. But we wont need that parsing as because if param is not set
then binding won't happen, which Thomas rightly pointed out, therefore
I choose to drop resource parsing for virtio-for-vfio case, now virtio
driver to check only drv->kdrv == RTE_KDRV_VFIO so to make sure
interface attached to vfio or not.

But perhaps when we have both flavours working for virtio, then we
should at least prompt a  INFO message on console that virtio pmd
driver attached to default vfio or noIOMMU.

So we don't need explicit _noIOMMU.

Yes this patch is to enable non-x86 arch to use virtio pmd driver
(virtio 0.95 spec). After this patch merges-in, I am planning to
- replace sys/io.h entirely
- Add raw_read/raw_writel() api for arm/arm64 {Already proposed
similar implementation in v2} so that they could use virtio 1.0spec
mapped memory, for both UIO/VFIO mode.