[dpdk-dev] [PATCH v4 00/14] Add virtio support for arm/arm64

[dpdk-dev] [PATCH v4 00/14] Add virtio support for arm/arm64

[Santosh Shukla]

Hi,

This v4 patch uses vfio-noiommu-way to access virtio-net pci interface.
Tested for arm64 thunderX platform. Patch builds for
x86/i386/arm/armv8/thunderX. Tested with testpmd application.

Refer v3 [1] cover letter for dependancy description:
Step to enable vfio-noiommu mode:
- modprobe vfio-pci
echo 1 > /sys/module/vfio/parameters/enable_unsafe_*
- then bind 
./tools/dpdk_nic_bind.py -b vfio-pci 0000:00:03.0

- Testpmd application to try out for:
./app/testpmd -c 0x3 -n 4 -- -i --portmask=0x0  --nb-cores=1 --port-topology=chained

On host side ping to tapX interface and observe pkt_cnt on guest side.

For patch history from v1-->v3 pl. refer v3 cover letter [1]

v3--> v4:
- Incorporated v3 review comments, Thanks to Stephen, Yuan, Bruce for comment!
- Tested for Huawei patch series titled "[PATCH v2 0/4] fix the issue that DPDK
  takes over virtio device blindly". 
- Patch no 11 and 13 are testonly patches used for this patch series [Anatoly/
  Yuan]

Major change in series:
- Introducing vfio interface parse api in virtio pmd driver
- Added vfio device specific private header in struct virtio_hw{}
- Dummy in/oub x86-style api, just to pass build error for non-x86 arch for vfio
  mode.
- VIRTIO_REG_RD/WR API(s) are now able to do rd/wr for both interfaces i.e. for
  vfio and igb_uio/ioport bar. Tested for both mode for x86 and also only for
  vfio for arm64 (non-x86) archs.

So to try-out complete patc-set w/o cut-n-paste pain clone this [2]

Thanks!.

[1] http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/31117
[2] https://github.com/sshukla82/dpdk.git branch master-virtio-vfio-v4

Anatoly Burakov (1):
  vfio: Support for no-IOMMU mode

Santosh Shukla (12):
  virtio: Introduce config RTE_VIRTIO_INC_VECTOR
  config: i686: set RTE_VIRTIO_INC_VECTOR=n
  linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init()
  linuxapp/vfio: ignore mapping for ioport region
  virtio_pci.h: build fix for sys/io.h for non-x86 arch
  eal: pci: vfio: add rd/wr func for pci bar space
  virtio: vfio: add api support to rd/wr ioport bar
  virtio: pci: extend virtio pci rw api for vfio interface
  virtio: ethdev: check for vfio interface
  virtio: pci: add dummy func definition for in/outb for non-x86 arch
  config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD
  virtio: enable vfio in pmd driver

Yuanhan Liu (1):
  eal: pci: export pci_[un]map_device

 config/common_linuxapp                          |    1 +
 config/defconfig_arm-armv7a-linuxapp-gcc        |    4 +-
 config/defconfig_arm64-armv8a-linuxapp-gcc      |    4 +-
 config/defconfig_i686-native-linuxapp-gcc       |    1 +
 config/defconfig_i686-native-linuxapp-icc       |    1 +
 drivers/net/virtio/Makefile                     |    2 +-
 drivers/net/virtio/virtio_ethdev.c              |  124 ++++++++++++++-
 drivers/net/virtio/virtio_pci.h                 |  128 +++++++++++++--
 drivers/net/virtio/virtio_rxtx.c                |    7 +
 drivers/net/virtio/virtio_vfio_rw.h             |  107 +++++++++++++
 lib/librte_eal/bsdapp/eal/eal_pci.c             |    4 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |    7 +
 lib/librte_eal/common/eal_common_pci.c          |    4 +-
 lib/librte_eal/common/eal_private.h             |   18 ---
 lib/librte_eal/common/include/rte_pci.h         |   65 ++++++++
 lib/librte_eal/linuxapp/eal/Makefile            |    1 +
 lib/librte_eal/linuxapp/eal/eal.c               |    2 +
 lib/librte_eal/linuxapp/eal/eal_pci.c           |   41 ++++-
 lib/librte_eal/linuxapp/eal/eal_pci_init.h      |   28 ++++
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c      |  191 ++++++++++++++++-------
 lib/librte_eal/linuxapp/eal/eal_pci_vfio_dma.c  |   84 ++++++++++
 lib/librte_eal/linuxapp/eal/eal_vfio.h          |    5 +
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |    7 +
 23 files changed, 740 insertions(+), 96 deletions(-)
 create mode 100644 drivers/net/virtio/virtio_vfio_rw.h
 create mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_vfio_dma.c

-- 
1.7.9.5

[dpdk-dev] [PATCH v4 01/14] virtio: Introduce config RTE_VIRTIO_INC_VECTOR

[Santosh Shukla]

virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx
instructions. For arm64 in particular, virtio vector implementation does not
exist(todo).

So virtio pmd driver wont build for targets like i686, arm64.  By making
RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work
in non-vectored virtio mode.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
 config/common_linuxapp           |    1 +
 drivers/net/virtio/Makefile      |    2 +-
 drivers/net/virtio/virtio_rxtx.c |    7 +++++++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/config/common_linuxapp b/config/common_linuxapp
index 74bc515..8677697 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -274,6 +274,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
 CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
 CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DRIVER=n
 CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
+CONFIG_RTE_VIRTIO_INC_VECTOR=y
 
 #
 # Compile burst-oriented VMXNET3 PMD driver
diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
index 43835ba..25a842d 100644
--- a/drivers/net/virtio/Makefile
+++ b/drivers/net/virtio/Makefile
@@ -50,7 +50,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtqueue.c
 SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_pci.c
 SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx.c
 SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_ethdev.c
-SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple.c
+SRCS-$(CONFIG_RTE_VIRTIO_INC_VECTOR) += virtio_rxtx_simple.c
 
 # this lib depends upon:
 DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_eal lib/librte_ether
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 74b39ef..23be1ff 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -438,7 +438,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
 
 	dev->data->rx_queues[queue_idx] = vq;
 
+#ifdef RTE_VIRTIO_INC_VECTOR
 	virtio_rxq_vec_setup(vq);
+#endif
 
 	return 0;
 }
@@ -464,7 +466,10 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 			const struct rte_eth_txconf *tx_conf)
 {
 	uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
+
+#ifdef RTE_VIRTIO_INC_VECTOR
 	struct virtio_hw *hw = dev->data->dev_private;
+#endif
 	struct virtqueue *vq;
 	uint16_t tx_free_thresh;
 	int ret;
@@ -477,6 +482,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 		return -EINVAL;
 	}
 
+#ifdef RTE_VIRTIO_INC_VECTOR
 	/* Use simple rx/tx func if single segment and no offloads */
 	if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS &&
 	     !vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
@@ -485,6 +491,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
 		dev->rx_pkt_burst = virtio_recv_pkts_vec;
 		use_simple_rxtx = 1;
 	}
+#endif
 
 	ret = virtio_dev_queue_setup(dev, VTNET_TQ, queue_idx, vtpci_queue_idx,
 			nb_desc, socket_id, &vq);
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 02/14] config: i686: set RTE_VIRTIO_INC_VECTOR=n

[Santosh Shukla]

i686 target config example:
config/defconfig_i686-native-linuxapp-gcc says "Vectorized PMD is not supported
on 32-bit".

So setting RTE_VIRTIO_INC_VECTOR to 'n'.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
 config/defconfig_i686-native-linuxapp-gcc |    1 +
 config/defconfig_i686-native-linuxapp-icc |    1 +
 2 files changed, 2 insertions(+)

diff --git a/config/defconfig_i686-native-linuxapp-gcc b/config/defconfig_i686-native-linuxapp-gcc
index a90de9b..a4b1c49 100644
--- a/config/defconfig_i686-native-linuxapp-gcc
+++ b/config/defconfig_i686-native-linuxapp-gcc
@@ -49,3 +49,4 @@ CONFIG_RTE_LIBRTE_KNI=n
 # Vectorized PMD is not supported on 32-bit
 #
 CONFIG_RTE_IXGBE_INC_VECTOR=n
+CONFIG_RTE_VIRTIO_INC_VECTOR=n
diff --git a/config/defconfig_i686-native-linuxapp-icc b/config/defconfig_i686-native-linuxapp-icc
index c021321..f8eb6ad 100644
--- a/config/defconfig_i686-native-linuxapp-icc
+++ b/config/defconfig_i686-native-linuxapp-icc
@@ -49,3 +49,4 @@ CONFIG_RTE_LIBRTE_KNI=n
 # Vectorized PMD is not supported on 32-bit
 #
 CONFIG_RTE_IXGBE_INC_VECTOR=n
+CONFIG_RTE_VIRTIO_INC_VECTOR=n
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 03/14] linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init()

[Santosh Shukla]

iopl() syscall not supported in linux-arm/arm64 so always return 0 value.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Jan Viktorin <viktorin@rehivetech.com>
---
v3->v4:
- moved #ifdef to elseif -> suggested by Stephen.

lib/librte_eal/linuxapp/eal/eal.c |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 635ec36..a2a3485 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -715,6 +715,8 @@ rte_eal_iopl_init(void)
 	if (iopl(3) != 0)
 		return -1;
 	return 0;
+#elif defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+	return 0; /* iopl syscall not supported for ARM/ARM64 */
 #else
 	return -1;
 #endif
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 04/14] linuxapp/vfio: ignore mapping for ioport region

[Santosh Shukla]

vfio_pci_mmap() try to map all pci bars. ioport region are not mapped in
vfio/kernel so ignore mmaping for ioport.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v3->v4: per review comment from Stephen and Yuan.
- removed ioport_bar var declaration with in func to top of func
- rearrange log message to fit with in 80 line

lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 74f91ba..abde779 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -573,6 +573,7 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 	struct pci_map *maps;
 	uint32_t msix_table_offset = 0;
 	uint32_t msix_table_size = 0;
+	uint32_t ioport_bar;
 
 	dev->intr_handle.fd = -1;
 	dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
@@ -760,6 +761,25 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 			return -1;
 		}
 
+		/* chk for io port region */
+		ret = pread64(vfio_dev_fd, &ioport_bar, sizeof(ioport_bar),
+			      VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX)
+			      + PCI_BASE_ADDRESS_0 + i*4);
+
+		if (ret != sizeof(ioport_bar)) {
+			RTE_LOG(ERR, EAL,
+				"Cannot read command (%x) from config space!\n",
+				PCI_BASE_ADDRESS_0 + i*4);
+			return -1;
+		}
+
+		if (ioport_bar & PCI_BASE_ADDRESS_SPACE_IO) {
+			RTE_LOG(INFO, EAL,
+				"Ignore mapping IO port bar(%d) addr: %x\n",
+				 i, ioport_bar);
+			continue;
+		}
+
 		/* skip non-mmapable BARs */
 		if ((reg.flags & VFIO_REGION_INFO_FLAG_MMAP) == 0)
 			continue;
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 05/14] virtio_pci.h: build fix for sys/io.h for non-x86 arch

[Santosh Shukla]

make sure sys/io.h used only for x86 archs. This fixes build error
arm64/arm case.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
 drivers/net/virtio/virtio_pci.h |    2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 47f722a..8b5b031 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -40,8 +40,10 @@
 #include <sys/types.h>
 #include <machine/cpufunc.h>
 #else
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
 #include <sys/io.h>
 #endif
+#endif
 
 #include <rte_ethdev.h>
 
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 06/14] eal: pci: vfio: add rd/wr func for pci bar space

[Santosh Shukla]

Introducing below api for pci bar space rd/wr. Currently used for
pci iobar rd/wr.

Api's are:
- rte_eal_pci_read_bar
- rte_eal_pci_write_bar

virtio when used for vfio-mode then virtio driver will use these api
to do rd/wr operation on ioport pci bar.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v3->v4:
- Using RTE_SET_USED(_var_) for unused variable for !VFIO_PRESENT case.
  As per v3 review comment from Bruce. 

lib/librte_eal/common/include/rte_pci.h    |   38 ++++++++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci.c      |   37 +++++++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci_init.h |    6 +++++
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   28 ++++++++++++++++++++
 4 files changed, 109 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 334c12e..53437cc 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -471,6 +471,44 @@ int rte_eal_pci_read_config(const struct rte_pci_device *device,
 			    void *buf, size_t len, off_t offset);
 
 /**
+ * Read PCI bar space.
+ *
+ * @param device
+ *   A pointer to a rte_pci_device structure describing the device
+ *   to use
+ * @param buf
+ *   A data buffer where the bytes should be read into
+ * @param len
+ *   The length of the data buffer.
+ * @param offset
+ *   The offset into PCI bar space
+ * @param bar_idx
+ *   The pci bar index (valid range is 0..5)
+ */
+int rte_eal_pci_read_bar(const struct rte_pci_device *device,
+			 void *buf, size_t len, off_t offset, int bar_idx);
+
+/**
+ * Write PCI bar space.
+ *
+ * @param device
+ *   A pointer to a rte_pci_device structure describing the device
+ *   to use
+ * @param buf
+ *   A data buffer containing the bytes should be written
+ * @param len
+ *   The length of the data buffer.
+ * @param offset
+ *   The offset into PCI config space
+ * @param bar_idx
+ *   The pci bar index (valid range is 0..5)
+*/
+int rte_eal_pci_write_bar(const struct rte_pci_device *device,
+			  const void *buf, size_t len, off_t offset,
+			  int bar_idx);
+
+
+/**
  * Write PCI config space.
  *
  * @param device
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index bc5b5be..8c1a49d 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -621,6 +621,43 @@ int rte_eal_pci_write_config(const struct rte_pci_device *device,
 	}
 }
 
+int rte_eal_pci_read_bar(const struct rte_pci_device *device,
+			 void *buf, size_t len, off_t offset,
+			 int bar_idx)
+
+{
+#ifdef VFIO_PRESENT
+	const struct rte_intr_handle *intr_handle = &device->intr_handle;
+	return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
+#else
+	/* UIO's not applicable */
+	RTE_SET_USED(device);
+	RTE_SET_USED(buf);
+	RTE_SET_USED(len);
+	RTE_SET_USED(offset);
+	RTE_SET_USED(bar_idx);
+	return 0;
+#endif
+}
+
+int rte_eal_pci_write_bar(const struct rte_pci_device *device,
+			  const void *buf, size_t len, off_t offset,
+			  int bar_idx)
+{
+#ifdef VFIO_PRESENT
+	const struct rte_intr_handle *intr_handle = &device->intr_handle;
+	return pci_vfio_write_bar(intr_handle, buf, len, offset, bar_idx);
+#else
+	/* UIO's not applicable */
+	RTE_SET_USED(device);
+	RTE_SET_USED(buf);
+	RTE_SET_USED(len);
+	RTE_SET_USED(offset);
+	RTE_SET_USED(bar_idx);
+	return 0;
+#endif
+}
+
 /* Init the PCI EAL subsystem */
 int
 rte_eal_pci_init(void)
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
index a17c708..3bc592b 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
@@ -68,6 +68,12 @@ int pci_vfio_read_config(const struct rte_intr_handle *intr_handle,
 int pci_vfio_write_config(const struct rte_intr_handle *intr_handle,
 			  const void *buf, size_t len, off_t offs);
 
+int pci_vfio_read_bar(const struct rte_intr_handle *intr_handle,
+		      void *buf, size_t len, off_t offs, int bar_idx);
+
+int pci_vfio_write_bar(const struct rte_intr_handle *intr_handle,
+		       const void *buf, size_t len, off_t offs, int bar_idx);
+
 /* map VFIO resource prototype */
 int pci_vfio_map_resource(struct rte_pci_device *dev);
 int pci_vfio_get_group_fd(int iommu_group_fd);
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index abde779..df407ef 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -93,6 +93,34 @@ pci_vfio_write_config(const struct rte_intr_handle *intr_handle,
 	       VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX) + offs);
 }
 
+int
+pci_vfio_read_bar(const struct rte_intr_handle *intr_handle,
+		  void *buf, size_t len, off_t offs, int bar_idx)
+{
+	if (bar_idx < VFIO_PCI_BAR0_REGION_INDEX
+	   || bar_idx > VFIO_PCI_BAR5_REGION_INDEX) {
+		RTE_LOG(ERR, EAL, "invalid bar_idx!\n");
+		return -1;
+	}
+
+	return pread64(intr_handle->vfio_dev_fd, buf, len,
+		       VFIO_GET_REGION_ADDR(bar_idx) + offs);
+}
+
+int
+pci_vfio_write_bar(const struct rte_intr_handle *intr_handle,
+		   const void *buf, size_t len, off_t offs, int bar_idx)
+{
+	if (bar_idx < VFIO_PCI_BAR0_REGION_INDEX
+	   || bar_idx > VFIO_PCI_BAR5_REGION_INDEX) {
+		RTE_LOG(ERR, EAL, "invalid bar_idx!\n");
+		return -1;
+	}
+
+	return pwrite64(intr_handle->vfio_dev_fd, buf, len,
+			VFIO_GET_REGION_ADDR(bar_idx) + offs);
+}
+
 /* get PCI BAR number where MSI-X interrupts are */
 static int
 pci_vfio_get_msix_bar(int fd, int *msix_bar, uint32_t *msix_table_offset,
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 07/14] virtio: vfio: add api support to rd/wr ioport bar

[Santosh Shukla]

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>
---
v3->v4:
- Corrected debug error message for oub_ class of apis
- renamed file from virtio_vfio.h to virtio_vfio_rw.h
- Removed #ifdef , #else clutter so that now no #else condition to handle for!
  this file will work for all the arch which is using vfio interface.

drivers/net/virtio/virtio_vfio_rw.h |  107 +++++++++++++++++++++++++++++++++++
 1 file changed, 107 insertions(+)
 create mode 100644 drivers/net/virtio/virtio_vfio_rw.h

diff --git a/drivers/net/virtio/virtio_vfio_rw.h b/drivers/net/virtio/virtio_vfio_rw.h
new file mode 100644
index 0000000..80a67f4
--- /dev/null
+++ b/drivers/net/virtio/virtio_vfio_rw.h
@@ -0,0 +1,107 @@
+/*
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 Cavium Networks. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *   * Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *   * Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in
+ *     the documentation and/or other materials provided with the
+ *     distribution.
+ *   * Neither the name of Intel Corporation nor the names of its
+ *     contributors may be used to endorse or promote products derived
+ *     from this software without specific prior written permission.
+ *
+ *    THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *    "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *    LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *    A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *    OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *    SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *    LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *    DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *    THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *    (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *    OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+#ifndef _VIRTIO_VFIO_RW_H_
+#define _VIRTIO_VFIO_RW_H_
+
+#if defined(RTE_EAL_VFIO) && defined(RTE_LIBRTE_EAL_LINUXAPP)
+
+#include <stdint.h>
+#include <rte_pci.h>
+#include "virtio_logs.h"
+
+/* vfio rd/rw virtio apis */
+static inline void ioport_inb(const struct rte_pci_device *pci_dev,
+			      uint8_t reg, uint8_t *val)
+{
+	if (rte_eal_pci_read_bar(pci_dev, (uint8_t *)val, sizeof(uint8_t), reg,
+				 0) <= 0) {
+		PMD_DRV_LOG(ERR, "Can't read from PCI bar space");
+		return;
+	}
+}
+
+static inline void ioport_inw(const struct rte_pci_device *pci_dev,
+			      uint16_t reg, uint16_t *val)
+{
+	if (rte_eal_pci_read_bar(pci_dev, (uint16_t *)val, sizeof(uint16_t),
+				 reg, 0) <= 0) {
+		PMD_DRV_LOG(ERR, "Can't read from PCI bar space");
+		return;
+	}
+}
+
+static inline void ioport_inl(const struct rte_pci_device *pci_dev,
+			      uint32_t reg, uint32_t *val)
+{
+	if (rte_eal_pci_read_bar(pci_dev, (uint32_t *)val, sizeof(uint32_t),
+				 reg, 0) <= 0) {
+		PMD_DRV_LOG(ERR, "Can't read from PCI bar space");
+		return;
+	}
+}
+
+static inline void ioport_outb_p(const struct rte_pci_device *pci_dev,
+				 uint8_t reg, uint8_t val)
+{
+	if (rte_eal_pci_write_bar(pci_dev, (uint8_t *)&val, sizeof(uint8_t),
+				  reg, 0) <= 0) {
+		PMD_DRV_LOG(ERR, "Can't write to PCI bar space");
+		return;
+	}
+}
+
+
+static inline void ioport_outw_p(const struct rte_pci_device *pci_dev,
+				 uint16_t reg, uint16_t val)
+{
+	if (rte_eal_pci_write_bar(pci_dev, (uint16_t *)&val, sizeof(uint16_t),
+				  reg, 0) <= 0) {
+		PMD_DRV_LOG(ERR, "Can't write to PCI bar space");
+		return;
+	}
+}
+
+
+static inline void ioport_outl_p(const struct rte_pci_device *pci_dev,
+				 uint32_t reg, uint32_t val)
+{
+	if (rte_eal_pci_write_bar(pci_dev, (uint32_t *)&val, sizeof(uint32_t),
+				  reg, 0) <= 0) {
+		PMD_DRV_LOG(ERR, "Can't write to PCI bar space");
+		return;
+	}
+}
+
+#endif /* RTE_EAL_VFIO && RTE_XX_EAL_LINUXAPP */
+#endif /* _VIRTIO_VFIO_RW_H_ */
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface

[Santosh Shukla]

So far virtio handle rw access for uio / ioport interface, This patch to extend
the support for vfio interface. For that introducing private struct
virtio_vfio_dev{
	- is_vfio
	- pci_dev
	};
Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v3->v4:
- Removed #indef RTE_EAL_VFIO and made it arch agnostic such now virtio_pci
  rd/wr api to handle both vfio and ig_uio/ioport interfaces, depending upon
  is_vfio flags set or unset.
- Tested for x86 for igb_uio and vfio interface, also  tested for arm64 for vfio
  interface.

drivers/net/virtio/virtio_pci.h |   84 ++++++++++++++++++++++++++++++++-------
 1 file changed, 70 insertions(+), 14 deletions(-)

diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 8b5b031..8526c07 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -46,6 +46,8 @@
 #endif
 
 #include <rte_ethdev.h>
+#include <stdbool.h>
+#include "virtio_vfio_rw.h"
 
 struct virtqueue;
 
@@ -165,6 +167,14 @@ struct virtqueue;
  */
 #define VIRTIO_MAX_VIRTQUEUES 8
 
+/* For vfio only */
+struct virtio_vfio_dev {
+	bool		is_vfio;	/* True: vfio i/f,
+					 * False: not a vfio i/f
+					 */
+	struct rte_pci_device *pci_dev; /* vfio dev */
+};
+
 struct virtio_hw {
 	struct virtqueue *cvq;
 	uint32_t    io_base;
@@ -176,6 +186,7 @@ struct virtio_hw {
 	uint8_t	    use_msix;
 	uint8_t     started;
 	uint8_t     mac_addr[ETHER_ADDR_LEN];
+	struct virtio_vfio_dev dev;
 };
 
 /*
@@ -231,20 +242,65 @@ outl_p(unsigned int data, unsigned int port)
 #define VIRTIO_PCI_REG_ADDR(hw, reg) \
 	(unsigned short)((hw)->io_base + (reg))
 
-#define VIRTIO_READ_REG_1(hw, reg) \
-	inb((VIRTIO_PCI_REG_ADDR((hw), (reg))))
-#define VIRTIO_WRITE_REG_1(hw, reg, value) \
-	outb_p((unsigned char)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
-
-#define VIRTIO_READ_REG_2(hw, reg) \
-	inw((VIRTIO_PCI_REG_ADDR((hw), (reg))))
-#define VIRTIO_WRITE_REG_2(hw, reg, value) \
-	outw_p((unsigned short)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
-
-#define VIRTIO_READ_REG_4(hw, reg) \
-	inl((VIRTIO_PCI_REG_ADDR((hw), (reg))))
-#define VIRTIO_WRITE_REG_4(hw, reg, value) \
-	outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
+#define VIRTIO_READ_REG_1(hw, reg)					\
+({									\
+	uint8_t ret;							\
+	struct virtio_vfio_dev *vdev;					\
+	(vdev) = (&(hw)->dev);						\
+	(((vdev)->is_vfio) ?						\
+	(ioport_inb(((vdev)->pci_dev), reg, &ret)) :			\
+	((ret) = (inb((VIRTIO_PCI_REG_ADDR((hw), (reg)))))));		\
+	ret;								\
+})
+
+#define VIRTIO_WRITE_REG_1(hw, reg, value)				\
+({									\
+	struct virtio_vfio_dev *vdev;					\
+	(vdev) = (&(hw)->dev);						\
+	(((vdev)->is_vfio) ?						\
+	(ioport_outb_p(((vdev)->pci_dev), reg, (uint8_t)(value))) :	\
+	(outb_p((unsigned char)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))))); \
+})
+
+#define VIRTIO_READ_REG_2(hw, reg)					\
+({									\
+	uint16_t ret;							\
+	struct virtio_vfio_dev *vdev;					\
+	(vdev) = (&(hw)->dev);						\
+	(((vdev)->is_vfio) ?						\
+	(ioport_inw(((vdev)->pci_dev), reg, &ret)) :			\
+	((ret) = (inw((VIRTIO_PCI_REG_ADDR((hw), (reg)))))));		\
+	ret;								\
+})
+
+#define VIRTIO_WRITE_REG_2(hw, reg, value)				\
+({									\
+	struct virtio_vfio_dev *vdev;					\
+	(vdev) = (&(hw)->dev);						\
+	(((vdev)->is_vfio) ?						\
+	(ioport_outw_p(((vdev)->pci_dev), reg, (uint16_t)(value))) :	\
+	(outw_p((unsigned short)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))))); \
+})
+
+#define VIRTIO_READ_REG_4(hw, reg)					\
+({									\
+	uint32_t ret;							\
+	struct virtio_vfio_dev *vdev;					\
+	(vdev) = (&(hw)->dev);						\
+	(((vdev)->is_vfio) ?						\
+	(ioport_inl(((vdev)->pci_dev), reg, &ret)) :			\
+	((ret) = (inl((VIRTIO_PCI_REG_ADDR((hw), (reg)))))));		\
+	ret;								\
+})
+
+#define VIRTIO_WRITE_REG_4(hw, reg, value)				\
+({									\
+	struct virtio_vfio_dev *vdev;					\
+	(vdev) = (&(hw)->dev);						\
+	(((vdev)->is_vfio) ?						\
+	(ioport_outl_p(((vdev)->pci_dev), reg, (uint32_t)(value))) :	\
+	(outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))))); \
+})
 
 static inline int
 vtpci_with_feature(struct virtio_hw *hw, uint32_t bit)
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 09/14] virtio: ethdev: check for vfio interface

[Santosh Shukla]

Introducing api to check interface type is vfio or not, if interface is vfio
then update struct virtio_vfio_dev {}.

Those two apis are:
- virtio_chk_for_vfio
- virtio_hw_init_by_vfio

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v3->v4:
- Removed RTE_PCI_DRV_NEED_MAPPING drv flag (as per Review comment from Stephen
  and Suggested by Yuan)
- Introducing vfio interface parsing api which will set/unset is_vfio flag at
  runtime.

drivers/net/virtio/virtio_ethdev.c |  112 +++++++++++++++++++++++++++++++++++-
 1 file changed, 110 insertions(+), 2 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d928339..8f2260f 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1202,6 +1202,105 @@ static int virtio_resource_init(struct rte_pci_device *pci_dev)
 		return virtio_resource_init_by_ioports(pci_dev);
 }
 
+static int virtio_chk_for_vfio(struct rte_pci_device *pci_dev)
+{
+	/*
+	 * 1. check whether vfio-noiommu mode is enabled
+	 * 2. verify pci device attached to vfio-noiommu driver
+	 * root@arm64:/sys/bus/pci/drivers/vfio-pci/0000:00:01.0/iommu_group#
+	 * > cat name
+	 * > vfio-noiommu
+	 */
+
+	/* 1. Chk for vfio: noiommu mode set or not in kernel driver */
+	struct rte_pci_addr *loc;
+	FILE *fp;
+	const char *path = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode";
+	char filename[PATH_MAX] = {0};
+	char buf[PATH_MAX] = {0};
+
+	fp = fopen(path, "r");
+	if (fp == NULL) {
+		PMD_INIT_LOG(ERR, "can't open %s\n", path);
+		return -1;
+	}
+
+	if (fread(buf, sizeof(char), 1, fp) != 1) {
+		PMD_INIT_LOG(ERR, "can't read from file %s\n", path);
+		fclose(fp);
+		return -1;
+	}
+
+	if (strncmp(buf, "Y", 1) != 0) {
+		PMD_INIT_LOG(ERR, "[%s]: vfio: noiommu mode not set\n", path);
+		fclose(fp);
+		return -1;
+	}
+
+	fclose(fp);
+
+	/* 2. Verify pci device attached to vfio-noiommu driver */
+
+	/* 2.1 chk whether attached driver is vfio-noiommu or not */
+	loc = &pci_dev->addr;
+	snprintf(filename, sizeof(filename),
+		     SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/iommu_group/name",
+		     loc->domain, loc->bus, loc->devid, loc->function);
+
+	/* check for vfio-noiommu */
+	fp = fopen(filename, "r");
+	if (fp == NULL) {
+		PMD_INIT_LOG(ERR, "can't open %s\n", filename);
+		return -1;
+	}
+
+	if (fread(buf, sizeof(char), sizeof("vfio-noiommu"), fp) !=
+		  sizeof("vfio-noiommu")) {
+		PMD_INIT_LOG(ERR, "can't read from file %s\n", filename);
+		fclose(fp);
+		return -1;
+	}
+
+	if (strncmp(buf, "vfio-noiommu", strlen("vfio-noiommu")) != 0) {
+		PMD_INIT_LOG(ERR, "not a vfio-noiommu driver\n");
+		fclose(fp);
+		return -1;
+	}
+
+	fclose(fp);
+
+	/* todo: vfio interrupt handling */
+	return 0;
+}
+
+/* Init virtio by vfio-way */
+static int virtio_hw_init_by_vfio(struct virtio_hw *hw,
+				  struct rte_pci_device *pci_dev)
+{
+	struct virtio_vfio_dev *vdev;
+
+	vdev = &hw->dev;
+	if (virtio_chk_for_vfio(pci_dev) < 0) {
+		vdev->is_vfio = false;
+		vdev->pci_dev = NULL;
+		return -1;
+	}
+
+	/* .. So attached interface is vfio */
+	vdev->is_vfio = true;
+	vdev->pci_dev = pci_dev;
+
+	/* For debug use only */
+	const struct rte_intr_handle *intr_handle;
+	RTE_SET_USED(intr_handle); /* to keep compilar happy */
+	intr_handle = &pci_dev->intr_handle;
+	PMD_INIT_LOG(DEBUG, "vdev->pci_dev %p intr_handle %p vfio_dev_fd %d\n",
+			     vdev->pci_dev, intr_handle,
+			     intr_handle->vfio_dev_fd);
+
+	return 0;
+}
+
 #else
 static int
 virtio_has_msix(const struct rte_pci_addr *loc __rte_unused)
@@ -1215,6 +1314,13 @@ static int virtio_resource_init(struct rte_pci_device *pci_dev __rte_unused)
 	/* no setup required */
 	return 0;
 }
+
+static int virtio_hw_init_by_vfio(struct virtio_hw *hw __rte_unused,
+				  struct rte_pci_device *pci_dev __rte_unused)
+{
+	/* NA */
+	return 0;
+}
 #endif
 
 /*
@@ -1287,8 +1393,10 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 
 	pci_dev = eth_dev->pci_dev;
 
-	if (virtio_resource_init(pci_dev) < 0)
-		return -1;
+	if (virtio_hw_init_by_vfio(hw, pci_dev) < 0) {
+		if (virtio_resource_init(pci_dev) < 0)
+			return -1;
+	}
 
 	hw->use_msix = virtio_has_msix(&pci_dev->addr);
 	hw->io_base = (uint32_t)(uintptr_t)pci_dev->mem_resource[0].addr;
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 10/14] virtio: pci: add dummy func definition for in/outb for non-x86 arch

[Santosh Shukla]

For non-x86 arch, Compiler will throw build error for in/out apis. Including
dummy api function so to pass build.

Note that: For virtio to work for non-x86 arch - RTE_EAL_VFIO is the only
supported method. RTE_EAL_IGB_UIO is not supported for non-x86 arch.

So, Virtio support for arch and supported interface by that arch:

ARCH       IGB_UIO 	VFIO
x86		Y	Y
ARM64		N/A	Y
PPC_64		N/A	Y	(Not tested but likely should work, as vfio is
arch independent)

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v4:
- dummy inb/outb function useful for non-x86 archs, Intent to get-rid of build
  error. 

drivers/net/virtio/virtio_pci.h |   42 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 8526c07..600260a 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -239,6 +239,48 @@ outl_p(unsigned int data, unsigned int port)
 }
 #endif
 
+#if !defined(RTE_ARCH_X86_64) && !defined(RTE_ARCH_I686) && \
+		defined(RTE_EXEC_ENV_LINUXAPP)
+static inline uint8_t inb(unsigned long addr __rte_unused)
+{
+	PMD_INIT_LOG(ERR, "inb() not supported for this RTE_ARCH\n");
+	return 0;
+}
+
+static inline uint16_t inw(unsigned long addr __rte_unused)
+{
+	PMD_INIT_LOG(ERR, "inw() not supported for this RTE_ARCH\n");
+	return 0;
+}
+
+static inline uint32_t inl(unsigned long addr __rte_unused)
+{
+	PMD_INIT_LOG(ERR, "in() not supported for this RTE_ARCH\n");
+	return 0;
+}
+
+static inline void
+outb_p(unsigned char data __rte_unused, unsigned int port __rte_unused)
+{
+	PMD_INIT_LOG(ERR, "outb_p() not supported for this RTE_ARCH\n");
+	return;
+}
+
+static inline void
+outw_p(unsigned short data __rte_unused, unsigned int port __rte_unused)
+{
+	PMD_INIT_LOG(ERR, "outw_p() not supported for this RTE_ARCH\n");
+	return;
+}
+
+static inline void
+outl_p(unsigned int data __rte_unused, unsigned int port __rte_unused)
+{
+	PMD_INIT_LOG(ERR, "outl_p() not supported for this RTE_ARCH\n");
+	return;
+}
+#endif
+
 #define VIRTIO_PCI_REG_ADDR(hw, reg) \
 	(unsigned short)((hw)->io_base + (reg))
 
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 11/14] config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD

[Santosh Shukla]

Enable RTE_LIBRTE_VIRTIO_PMD for armv7/v8 and setting RTE_VIRTIO_INC_VEC=n.
Builds successfully for armv7/v8.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v2->v4:
- Removed explict setting of VIRTIO_PMD in configs (per review comment from
  Jiangbo)

config/defconfig_arm-armv7a-linuxapp-gcc   |    4 +++-
 config/defconfig_arm64-armv8a-linuxapp-gcc |    4 +++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/config/defconfig_arm-armv7a-linuxapp-gcc b/config/defconfig_arm-armv7a-linuxapp-gcc
index cbebd64..9f852ce 100644
--- a/config/defconfig_arm-armv7a-linuxapp-gcc
+++ b/config/defconfig_arm-armv7a-linuxapp-gcc
@@ -43,6 +43,9 @@ CONFIG_RTE_FORCE_INTRINSICS=y
 CONFIG_RTE_TOOLCHAIN="gcc"
 CONFIG_RTE_TOOLCHAIN_GCC=y
 
+# Disable VIRTIO VECTOR support
+CONFIG_RTE_VIRTIO_INC_VECTOR=n
+
 # ARM doesn't have support for vmware TSC map
 CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=n
 
@@ -70,7 +73,6 @@ CONFIG_RTE_LIBRTE_I40E_PMD=n
 CONFIG_RTE_LIBRTE_IXGBE_PMD=n
 CONFIG_RTE_LIBRTE_MLX4_PMD=n
 CONFIG_RTE_LIBRTE_MPIPE_PMD=n
-CONFIG_RTE_LIBRTE_VIRTIO_PMD=n
 CONFIG_RTE_LIBRTE_VMXNET3_PMD=n
 CONFIG_RTE_LIBRTE_PMD_XENVIRT=n
 CONFIG_RTE_LIBRTE_PMD_BNX2X=n
diff --git a/config/defconfig_arm64-armv8a-linuxapp-gcc b/config/defconfig_arm64-armv8a-linuxapp-gcc
index 504f3ed..1a638b3 100644
--- a/config/defconfig_arm64-armv8a-linuxapp-gcc
+++ b/config/defconfig_arm64-armv8a-linuxapp-gcc
@@ -45,8 +45,10 @@ CONFIG_RTE_TOOLCHAIN_GCC=y
 
 CONFIG_RTE_CACHE_LINE_SIZE=64
 
+# Disable VIRTIO VECTOR support
+CONFIG_RTE_VIRTIO_INC_VECTOR=n
+
 CONFIG_RTE_IXGBE_INC_VECTOR=n
-CONFIG_RTE_LIBRTE_VIRTIO_PMD=n
 CONFIG_RTE_LIBRTE_IVSHMEM=n
 CONFIG_RTE_LIBRTE_FM10K_PMD=n
 CONFIG_RTE_LIBRTE_I40E_PMD=n
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 12/14] eal: pci: export pci_[un]map_device

[Santosh Shukla]

From: Yuanhan Liu <yuanhan.liu@linux.intel.com>

Normally we could set RTE_PCI_DRV_NEED_MAPPING flag so that eal will
invoke pci_map_device internally for us. From that point view, there
is no need to export pci_map_device.

However, for virtio pmd driver, which is designed to work without
binding UIO (or something similar first), pci_map_device() will fail,
which ends up with virtio pmd driver being skipped. Therefore, we can
not set RTE_PCI_DRV_NEED_MAPPING blindly at virtio pmd driver.

Therefore, this patch exports pci_map_device, and let virtio pmd
call it when necessary.

Cc: David Marchand <david.marchand@6wind.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
Tested-by: Santosh Shukla <sshukla@mvista.com>
---
- Pulled Yuan v3 patch, just for testing and other user to try out complete set
  and test it on HW i.e. x86 and non-x86 both. It works for VFIO mode
  seemlessly.

lib/librte_eal/bsdapp/eal/eal_pci.c             |    4 ++--
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |    7 ++++++
 lib/librte_eal/common/eal_common_pci.c          |    4 ++--
 lib/librte_eal/common/eal_private.h             |   18 ---------------
 lib/librte_eal/common/include/rte_pci.h         |   27 +++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_pci.c           |    4 ++--
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |    7 ++++++
 7 files changed, 47 insertions(+), 24 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 6c21fbd..95c32c1 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -93,7 +93,7 @@ pci_unbind_kernel_driver(struct rte_pci_device *dev __rte_unused)
 
 /* Map pci device */
 int
-pci_map_device(struct rte_pci_device *dev)
+rte_eal_pci_map_device(struct rte_pci_device *dev)
 {
 	int ret = -1;
 
@@ -115,7 +115,7 @@ pci_map_device(struct rte_pci_device *dev)
 
 /* Unmap pci device */
 void
-pci_unmap_device(struct rte_pci_device *dev)
+rte_eal_pci_unmap_device(struct rte_pci_device *dev)
 {
 	/* try unmapping the NIC resources */
 	switch (dev->kdrv) {
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 9d7adf1..1b28170 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -135,3 +135,10 @@ DPDK_2.2 {
 	rte_xen_dom0_supported;
 
 } DPDK_2.1;
+
+DPDK_2.3 {
+	global:
+
+	rte_eal_pci_map_device;
+	rte_eal_pci_unmap_device;
+} DPDK_2.2;
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index dcfe947..96d5113 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -188,7 +188,7 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 			pci_config_space_set(dev);
 #endif
 			/* map resources for devices that use igb_uio */
-			ret = pci_map_device(dev);
+			ret = rte_eal_pci_map_device(dev);
 			if (ret != 0)
 				return ret;
 		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
@@ -254,7 +254,7 @@ rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
 
 		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
 			/* unmap resources for devices that use igb_uio */
-			pci_unmap_device(dev);
+			rte_eal_pci_unmap_device(dev);
 
 		return 0;
 	}
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 072e672..2342fa1 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -165,24 +165,6 @@ struct rte_pci_device;
 int pci_unbind_kernel_driver(struct rte_pci_device *dev);
 
 /**
- * Map this device
- *
- * This function is private to EAL.
- *
- * @return
- *   0 on success, negative on error and positive if no driver
- *   is found for the device.
- */
-int pci_map_device(struct rte_pci_device *dev);
-
-/**
- * Unmap this device
- *
- * This function is private to EAL.
- */
-void pci_unmap_device(struct rte_pci_device *dev);
-
-/**
  * Map the PCI resource of a PCI device in virtual memory
  *
  * This function is private to EAL.
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 53437cc..0c667ff 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -523,6 +523,33 @@ int rte_eal_pci_write_bar(const struct rte_pci_device *device,
  */
 int rte_eal_pci_write_config(const struct rte_pci_device *device,
 			     const void *buf, size_t len, off_t offset);
+/**
+ * Map the PCI device resources in user space virtual memory address
+ *
+ * Note that driver should not call this function when flag
+ * RTE_PCI_DRV_NEED_MAPPING is set, as EAL will do that for
+ * you when it's on.
+ *
+ * @param dev
+ *   A pointer to a rte_pci_device structure describing the device
+ *   to use
+ *
+ * @return
+ *   0 on success, negative on error and positive if no driver
+ *   is found for the device.
+ */
+int rte_eal_pci_map_device(struct rte_pci_device *dev);
+
+/**
+ * Unmap this device
+ *
+ * @param dev
+ *   A pointer to a rte_pci_device structure describing the device
+ *   to use
+ */
+void rte_eal_pci_unmap_device(struct rte_pci_device *dev);
+
+
 
 #ifdef RTE_PCI_CONFIG
 /**
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 8c1a49d..39e0650 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -124,7 +124,7 @@ pci_get_kernel_driver_by_path(const char *filename, char *dri_name)
 
 /* Map pci device */
 int
-pci_map_device(struct rte_pci_device *dev)
+rte_eal_pci_map_device(struct rte_pci_device *dev)
 {
 	int ret = -1;
 
@@ -153,7 +153,7 @@ pci_map_device(struct rte_pci_device *dev)
 
 /* Unmap pci device */
 void
-pci_unmap_device(struct rte_pci_device *dev)
+rte_eal_pci_unmap_device(struct rte_pci_device *dev)
 {
 	/* try unmapping the NIC resources using VFIO if it exists */
 	switch (dev->kdrv) {
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index cbe175f..b9937c4 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -138,3 +138,10 @@ DPDK_2.2 {
 	rte_xen_dom0_supported;
 
 } DPDK_2.1;
+
+DPDK_2.3 {
+	global:
+
+	rte_eal_pci_map_device;
+	rte_eal_pci_unmap_device;
+} DPDK_2.2;
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 13/14] virtio: enable vfio in pmd driver

[Santosh Shukla]

Using mapping api i.e. rte_eal_pci_map_device() to create vfio container, group
id and get the vfio-dev-fd for virtio-net-pci interface. Later vfio_dev_fd used
for virtio device rd/wr operation.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v3->v4:
- Per stephens comment, removed static driver flag RTE_PCI_XXX_XX_NEED_MAPPING ,
  now vfio virtio pmd driver intialized vifio interface at runtime.

drivers/net/virtio/virtio_ethdev.c |   12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 8f2260f..ce03a24 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -497,6 +497,8 @@ virtio_dev_close(struct rte_eth_dev *dev)
 	hw->started = 0;
 	virtio_dev_free_mbufs(dev);
 	virtio_free_queues(dev);
+
+	/* For vfio case : hotunplug/unmap not supported (todo) */
 }
 
 static void
@@ -1286,6 +1288,16 @@ static int virtio_hw_init_by_vfio(struct virtio_hw *hw,
 		return -1;
 	}
 
+	/*
+	 * pci_map_device used not to actually map ioport region but
+	 * create vfio container/group and vfio-dev-fd for _this_
+	 * virtio interface.
+	 */
+	if (rte_eal_pci_map_device(pci_dev) != 0) {
+		PMD_INIT_LOG(ERR, "vfio pci mapping failed for ioport bar\n");
+		return -1;
+	}
+
 	/* .. So attached interface is vfio */
 	vdev->is_vfio = true;
 	vdev->pci_dev = pci_dev;
-- 
1.7.9.5

[dpdk-dev] [PATCH v4 14/14] vfio: Support for no-IOMMU mode

[Santosh Shukla]

From: Anatoly Burakov <anatoly.burakov@intel.com>

This commit is adding a generic mechanism to support multiple IOMMU
types. For now, it's only type 1 (x86 IOMMU) and no-IOMMU (a special
VFIO mode that doesn't use IOMMU at all), but it's easily extended
by adding necessary definitions into eal_pci_init.h and a DMA
mapping function to eal_pci_vfio_dma.c.

Since type 1 IOMMU module is no longer necessary to have VFIO,
we fix the module check to check for vfio-pci instead. It's not
ideal and triggers VFIO checks more often (and thus produces more
error output, which was the reason behind the module check in the
first place), so we compensate for that by providing more verbose
logging, indicating whether VFIO initialization has succeeded or
failed.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Signed-off-by: Santosh Shukla <sshukla@mvista.com>
Tested-by: Santosh Shukla <sshukla@mvista.com>
---
- Pulled Anatoly patch just for testing and other user to use full patchset to
  try-out / experitment for, This patchset works for me :).

lib/librte_eal/linuxapp/eal/Makefile           |    1 +
 lib/librte_eal/linuxapp/eal/eal_pci_init.h     |   22 ++++
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c     |  143 +++++++++++++++---------
 lib/librte_eal/linuxapp/eal/eal_pci_vfio_dma.c |   84 ++++++++++++++
 lib/librte_eal/linuxapp/eal/eal_vfio.h         |    5 +
 5 files changed, 202 insertions(+), 53 deletions(-)
 create mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_vfio_dma.c

diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 26eced5..5c9e9d9 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -59,6 +59,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_log.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci_uio.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci_vfio.c
+SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci_vfio_dma.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_pci_vfio_mp_sync.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_debug.c
 SRCS-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP) += eal_lcore.c
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
index 3bc592b..068800c 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
@@ -112,6 +112,28 @@ struct vfio_config {
 	struct vfio_group vfio_groups[VFIO_MAX_GROUPS];
 };
 
+/* function pointer typedef for DMA mapping functions */
+typedef  int (*vfio_dma_func_t)(int);
+
+/* Structure to hold supported IOMMU types */
+struct vfio_iommu_type {
+	int type_id;
+	const char *name;
+	vfio_dma_func_t dma_map_func;
+};
+
+/* function prototypes for different IOMMU types */
+int vfio_iommu_type1_dma_map(int container_fd);
+int vfio_iommu_noiommu_dma_map(int container_fd);
+
+/* IOMMU types we support */
+static const struct vfio_iommu_type iommu_types[] = {
+		/* x86 IOMMU, otherwise known as type 1 */
+		{ VFIO_TYPE1_IOMMU, "Type 1", &vfio_iommu_type1_dma_map},
+		/* IOMMU-less mode */
+		{ VFIO_NOIOMMU_IOMMU, "No-IOMMU", &vfio_iommu_noiommu_dma_map},
+};
+
 #endif
 
 #endif /* EAL_PCI_INIT_H_ */
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index df407ef..5c5ccea 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -72,6 +72,7 @@ EAL_REGISTER_TAILQ(rte_vfio_tailq)
 #define VFIO_DIR "/dev/vfio"
 #define VFIO_CONTAINER_PATH "/dev/vfio/vfio"
 #define VFIO_GROUP_FMT "/dev/vfio/%u"
+#define VFIO_NOIOMMU_GROUP_FMT "/dev/vfio/noiommu-%u"
 #define VFIO_GET_REGION_ADDR(x) ((uint64_t) x << 40ULL)
 
 /* per-process VFIO config */
@@ -236,42 +237,58 @@ pci_vfio_set_bus_master(int dev_fd)
 	return 0;
 }
 
-/* set up DMA mappings */
-static int
-pci_vfio_setup_dma_maps(int vfio_container_fd)
-{
-	const struct rte_memseg *ms = rte_eal_get_physmem_layout();
-	int i, ret;
-
-	ret = ioctl(vfio_container_fd, VFIO_SET_IOMMU,
-			VFIO_TYPE1_IOMMU);
-	if (ret) {
-		RTE_LOG(ERR, EAL, "  cannot set IOMMU type, "
-				"error %i (%s)\n", errno, strerror(errno));
-		return -1;
+/* pick IOMMU type. returns a pointer to vfio_iommu_type or NULL for error */
+static const struct vfio_iommu_type *
+pci_vfio_set_iommu_type(int vfio_container_fd) {
+	unsigned idx;
+	for (idx = 0; idx < RTE_DIM(iommu_types); idx++) {
+		const struct vfio_iommu_type *t = &iommu_types[idx];
+
+		int ret = ioctl(vfio_container_fd, VFIO_SET_IOMMU,
+				t->type_id);
+		if (!ret) {
+			RTE_LOG(NOTICE, EAL, "  using IOMMU type %d (%s)\n",
+					t->type_id, t->name);
+			return t;
+		}
+		/* not an error, there may be more supported IOMMU types */
+		RTE_LOG(DEBUG, EAL, "  set IOMMU type %d (%s) failed, "
+				"error %i (%s)\n", t->type_id, t->name, errno,
+				strerror(errno));
 	}
+	/* if we didn't find a suitable IOMMU type, fail */
+	return NULL;
+}
 
-	/* map all DPDK segments for DMA. use 1:1 PA to IOVA mapping */
-	for (i = 0; i < RTE_MAX_MEMSEG; i++) {
-		struct vfio_iommu_type1_dma_map dma_map;
-
-		if (ms[i].addr == NULL)
-			break;
-
-		memset(&dma_map, 0, sizeof(dma_map));
-		dma_map.argsz = sizeof(struct vfio_iommu_type1_dma_map);
-		dma_map.vaddr = ms[i].addr_64;
-		dma_map.size = ms[i].len;
-		dma_map.iova = ms[i].phys_addr;
-		dma_map.flags = VFIO_DMA_MAP_FLAG_READ | VFIO_DMA_MAP_FLAG_WRITE;
-
-		ret = ioctl(vfio_container_fd, VFIO_IOMMU_MAP_DMA, &dma_map);
+/* check if we have any supported extensions */
+static int
+pci_vfio_has_supported_extensions(int vfio_container_fd) {
+	int ret;
+	unsigned idx, n_extensions = 0;
+	for (idx = 0; idx < RTE_DIM(iommu_types); idx++) {
+		const struct vfio_iommu_type *t = &iommu_types[idx];
 
-		if (ret) {
-			RTE_LOG(ERR, EAL, "  cannot set up DMA remapping, "
-					"error %i (%s)\n", errno, strerror(errno));
+		ret = ioctl(vfio_container_fd, VFIO_CHECK_EXTENSION,
+				t->type_id);
+		if (ret < 0) {
+			RTE_LOG(ERR, EAL, "  could not get IOMMU type, "
+				"error %i (%s)\n", errno,
+				strerror(errno));
+			close(vfio_container_fd);
 			return -1;
+		} else if (ret == 1) {
+			/* we found a supported extension */
+			n_extensions++;
 		}
+		RTE_LOG(DEBUG, EAL, "  IOMMU type %d (%s) is %s\n",
+				t->type_id, t->name,
+				ret ? "supported" : "not supported");
+	}
+
+	/* if we didn't find any supported IOMMU types, fail */
+	if (!n_extensions) {
+		close(vfio_container_fd);
+		return -1;
 	}
 
 	return 0;
@@ -400,17 +417,10 @@ pci_vfio_get_container_fd(void)
 			return -1;
 		}
 
-		/* check if we support IOMMU type 1 */
-		ret = ioctl(vfio_container_fd, VFIO_CHECK_EXTENSION, VFIO_TYPE1_IOMMU);
-		if (ret != 1) {
-			if (ret < 0)
-				RTE_LOG(ERR, EAL, "  could not get IOMMU type, "
-					"error %i (%s)\n", errno,
-					strerror(errno));
-			else
-				RTE_LOG(ERR, EAL, "  unsupported IOMMU type "
-					"detected in VFIO\n");
-			close(vfio_container_fd);
+		ret = pci_vfio_has_supported_extensions(vfio_container_fd);
+		if (ret) {
+			RTE_LOG(ERR, EAL, "  no supported IOMMU "
+					"extensions found!\n");
 			return -1;
 		}
 
@@ -460,6 +470,7 @@ pci_vfio_get_group_fd(int iommu_group_no)
 
 	/* if primary, try to open the group */
 	if (internal_config.process_type == RTE_PROC_PRIMARY) {
+		/* try regular group format */
 		snprintf(filename, sizeof(filename),
 				 VFIO_GROUP_FMT, iommu_group_no);
 		vfio_group_fd = open(filename, O_RDWR);
@@ -470,7 +481,20 @@ pci_vfio_get_group_fd(int iommu_group_no)
 						strerror(errno));
 				return -1;
 			}
-			return 0;
+
+			/* special case: try no-IOMMU path as well */
+			snprintf(filename, sizeof(filename),
+					VFIO_NOIOMMU_GROUP_FMT, iommu_group_no);
+			vfio_group_fd = open(filename, O_RDWR);
+			if (vfio_group_fd < 0) {
+				if (errno != ENOENT) {
+					RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", filename,
+							strerror(errno));
+					return -1;
+				}
+				return 0;
+			}
+			/* noiommu group found */
 		}
 
 		/* if the fd is valid, create a new group for it */
@@ -689,14 +713,21 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 	}
 
 	/*
-	 * set up DMA mappings for container
+	 * pick an IOMMU type and set up DMA mappings for container
 	 *
 	 * needs to be done only once, only when at least one group is assigned to
 	 * a container and only in primary process
 	 */
 	if (internal_config.process_type == RTE_PROC_PRIMARY &&
 			vfio_cfg.vfio_container_has_dma == 0) {
-		ret = pci_vfio_setup_dma_maps(vfio_cfg.vfio_container_fd);
+		/* select an IOMMU type which we will be using */
+		const struct vfio_iommu_type *t =
+				pci_vfio_set_iommu_type(vfio_cfg.vfio_container_fd);
+		if (!t) {
+			RTE_LOG(ERR, EAL, "  %s failed to select IOMMU type\n", pci_addr);
+			return -1;
+		}
+		ret = t->dma_map_func(vfio_cfg.vfio_container_fd);
 		if (ret) {
 			RTE_LOG(ERR, EAL, "  %s DMA remapping failed, "
 					"error %i (%s)\n", pci_addr, errno, strerror(errno));
@@ -935,35 +966,41 @@ pci_vfio_enable(void)
 {
 	/* initialize group list */
 	int i;
-	int module_vfio_type1;
+	int vfio_available;
 
 	for (i = 0; i < VFIO_MAX_GROUPS; i++) {
 		vfio_cfg.vfio_groups[i].fd = -1;
 		vfio_cfg.vfio_groups[i].group_no = -1;
 	}
 
-	module_vfio_type1 = rte_eal_check_module("vfio_iommu_type1");
+	/* inform the user that we are probing for VFIO */
+	RTE_LOG(INFO, EAL, "Probing VFIO support...\n");
+
+	/* check if vfio-pci module is loaded */
+	vfio_available = rte_eal_check_module("vfio_pci");
 
 	/* return error directly */
-	if (module_vfio_type1 == -1) {
+	if (vfio_available == -1) {
 		RTE_LOG(INFO, EAL, "Could not get loaded module details!\n");
 		return -1;
 	}
 
 	/* return 0 if VFIO modules not loaded */
-	if (module_vfio_type1 == 0) {
-		RTE_LOG(INFO, EAL, "VFIO modules not all loaded, "
-			"skip VFIO support...\n");
+	if (vfio_available == 0) {
+		RTE_LOG(INFO, EAL, "VFIO modules not loaded, "
+			"skipping VFIO support...\n");
 		return 0;
 	}
 
 	vfio_cfg.vfio_container_fd = pci_vfio_get_container_fd();
 
 	/* check if we have VFIO driver enabled */
-	if (vfio_cfg.vfio_container_fd != -1)
+	if (vfio_cfg.vfio_container_fd != -1) {
+		RTE_LOG(NOTICE, EAL, "VFIO support initialized\n");
 		vfio_cfg.vfio_enabled = 1;
-	else
+	} else {
 		RTE_LOG(NOTICE, EAL, "VFIO support could not be initialized\n");
+	}
 
 	return 0;
 }
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio_dma.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio_dma.c
new file mode 100644
index 0000000..50d3563
--- /dev/null
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio_dma.c
@@ -0,0 +1,84 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <string.h>
+#include <sys/ioctl.h>
+
+#include <rte_log.h>
+#include <rte_pci.h>
+#include <rte_eal_memconfig.h>
+
+#include "eal_pci_init.h"
+
+#ifdef VFIO_PRESENT
+
+int
+vfio_iommu_type1_dma_map(int vfio_container_fd)
+{
+	const struct rte_memseg *ms = rte_eal_get_physmem_layout();
+	int i, ret;
+
+	/* map all DPDK segments for DMA. use 1:1 PA to IOVA mapping */
+	for (i = 0; i < RTE_MAX_MEMSEG; i++) {
+		struct vfio_iommu_type1_dma_map dma_map;
+
+		if (ms[i].addr == NULL)
+			break;
+
+		memset(&dma_map, 0, sizeof(dma_map));
+		dma_map.argsz = sizeof(struct vfio_iommu_type1_dma_map);
+		dma_map.vaddr = ms[i].addr_64;
+		dma_map.size = ms[i].len;
+		dma_map.iova = ms[i].phys_addr;
+		dma_map.flags = VFIO_DMA_MAP_FLAG_READ | VFIO_DMA_MAP_FLAG_WRITE;
+
+		ret = ioctl(vfio_container_fd, VFIO_IOMMU_MAP_DMA, &dma_map);
+
+		if (ret) {
+			RTE_LOG(ERR, EAL, "  cannot set up DMA remapping, "
+					"error %i (%s)\n", errno, strerror(errno));
+			return -1;
+		}
+	}
+
+	return 0;
+}
+
+int
+vfio_iommu_noiommu_dma_map(int __rte_unused vfio_container_fd)
+{
+	/* No-IOMMU mode does not need DMA mapping */
+	return 0;
+}
+
+#endif
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h
index 72ec3f6..638ee31 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.h
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h
@@ -52,6 +52,11 @@
 #define RTE_PCI_MSIX_FLAGS_QSIZE  PCI_MSIX_FLAGS_QSIZE
 #endif
 
+/* older kernels may not have no-IOMMU mode */
+#ifndef VFIO_NOIOMMU_IOMMU
+#define VFIO_NOIOMMU_IOMMU 8
+#endif
+
 #define VFIO_PRESENT
 #endif /* kernel version */
 #endif /* RTE_EAL_VFIO */
-- 
1.7.9.5

Re: [dpdk-dev] [PATCH v4 06/14] eal: pci: vfio: add rd/wr func for pci bar space

[Yuanhan Liu]

On Thu, Jan 14, 2016 at 06:58:29PM +0530, Santosh Shukla wrote:
...
> +int rte_eal_pci_read_bar(const struct rte_pci_device *device,
> +			 void *buf, size_t len, off_t offset,
> +			 int bar_idx)
> +
> +{
> +#ifdef VFIO_PRESENT
> +	const struct rte_intr_handle *intr_handle = &device->intr_handle;
> +	return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
> +#else
> +	/* UIO's not applicable */
> +	RTE_SET_USED(device);
> +	RTE_SET_USED(buf);
> +	RTE_SET_USED(len);
> +	RTE_SET_USED(offset);
> +	RTE_SET_USED(bar_idx);
> +	return 0;
> +#endif

Maybe you could do that like what pci_map_device() does:

	switch(dev->kdrv) {
	case RTE_KDRV_NIC_VFIO:
		return pci_vfio_read_bar(..);
		break;
	default:
		RTE_LOG(.... not supported by driver: %d\n"..);
		break;
	}

With that, you could get rid of those ugly RTE_SET_USED....

	--yliu

Re: [dpdk-dev] [PATCH v4 07/14] virtio: vfio: add api support to rd/wr ioport bar

[Yuanhan Liu]

On Thu, Jan 14, 2016 at 06:58:30PM +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>
> ---
...
> +/* vfio rd/rw virtio apis */
> +static inline void ioport_inb(const struct rte_pci_device *pci_dev,
> +			      uint8_t reg, uint8_t *val)

Minor nit: dpdk perfers to seperate return type and function name in
different line:

  static inline void
  ioport_inb(....)
  {

> +{
> +	if (rte_eal_pci_read_bar(pci_dev, (uint8_t *)val, sizeof(uint8_t), reg,
                                          ^^^^^^^^^^^

Unnecessary cast; and few more belows.

	--yliu

Re: [dpdk-dev] [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface

[Yuanhan Liu]

On Thu, Jan 14, 2016 at 06:58:31PM +0530, Santosh Shukla wrote:
> So far virtio handle rw access for uio / ioport interface, This patch to extend
> the support for vfio interface. For that introducing private struct
> virtio_vfio_dev{
> 	- is_vfio
> 	- pci_dev
> 	};
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
...
> +/* For vfio only */
> +struct virtio_vfio_dev {
> +	bool		is_vfio;	/* True: vfio i/f,
> +					 * False: not a vfio i/f

Well, this is weird; you are adding a flag to tell whether it's a
vfio device __inside__ a vfio struct.

Back to the topic, this flag is not necessary to me: you can
check the pci_dev->kdrv flag.

> +					 */
> +	struct rte_pci_device *pci_dev; /* vfio dev */

Note that I have already added this field into virtio_hw struct
at my latest virtio 1.0 pmd patchset.

While I told you before that you should not develop patches based
on my patcheset, I guess you can do that now. Since it should be
in good shape and close to be merged.

> +};
> +
>  struct virtio_hw {
>  	struct virtqueue *cvq;
>  	uint32_t    io_base;
> @@ -176,6 +186,7 @@ struct virtio_hw {
>  	uint8_t	    use_msix;
>  	uint8_t     started;
>  	uint8_t     mac_addr[ETHER_ADDR_LEN];
> +	struct virtio_vfio_dev dev;
>  };
>  
>  /*
> @@ -231,20 +242,65 @@ outl_p(unsigned int data, unsigned int port)
>  #define VIRTIO_PCI_REG_ADDR(hw, reg) \
>  	(unsigned short)((hw)->io_base + (reg))
>  
> -#define VIRTIO_READ_REG_1(hw, reg) \
> -	inb((VIRTIO_PCI_REG_ADDR((hw), (reg))))
> -#define VIRTIO_WRITE_REG_1(hw, reg, value) \
> -	outb_p((unsigned char)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
> -
> -#define VIRTIO_READ_REG_2(hw, reg) \
> -	inw((VIRTIO_PCI_REG_ADDR((hw), (reg))))
> -#define VIRTIO_WRITE_REG_2(hw, reg, value) \
> -	outw_p((unsigned short)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
> -
> -#define VIRTIO_READ_REG_4(hw, reg) \
> -	inl((VIRTIO_PCI_REG_ADDR((hw), (reg))))
> -#define VIRTIO_WRITE_REG_4(hw, reg, value) \
> -	outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
> +#define VIRTIO_READ_REG_1(hw, reg)					\
> +({									\
> +	uint8_t ret;							\
> +	struct virtio_vfio_dev *vdev;					\
> +	(vdev) = (&(hw)->dev);						\
> +	(((vdev)->is_vfio) ?						\
> +	(ioport_inb(((vdev)->pci_dev), reg, &ret)) :			\
> +	((ret) = (inb((VIRTIO_PCI_REG_ADDR((hw), (reg)))))));		\
> +	ret;								\
> +})

It becomes unreadable. I'd suggest to define them as iniline
functions, and use "if .. else .." instead of "?:".

	--yliu

Re: [dpdk-dev] [PATCH v4 09/14] virtio: ethdev: check for vfio interface

[Yuanhan Liu]

On Thu, Jan 14, 2016 at 06:58:32PM +0530, Santosh Shukla wrote:
> Introducing api to check interface type is vfio or not, if interface is vfio
> then update struct virtio_vfio_dev {}.
> 
> Those two apis are:
> - virtio_chk_for_vfio
> - virtio_hw_init_by_vfio
> 
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
..
> +/* Init virtio by vfio-way */
> +static int virtio_hw_init_by_vfio(struct virtio_hw *hw,
> +				  struct rte_pci_device *pci_dev)
> +{
> +	struct virtio_vfio_dev *vdev;
> +
> +	vdev = &hw->dev;
> +	if (virtio_chk_for_vfio(pci_dev) < 0) {
> +		vdev->is_vfio = false;
> +		vdev->pci_dev = NULL;
> +		return -1;
> +	}
> +
> +	/* .. So attached interface is vfio */
> +	vdev->is_vfio = true;
> +	vdev->pci_dev = pci_dev;

Normally, I don't like the way of adding yet another "virtio_hw_init_by_xxx".

As suggested in another reply, would pci_dev->kdrv checking be enough?
If so, do it in simple way.

	--ylu

Re: [dpdk-dev] [PATCH v4 11/14] config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD

[Yuanhan Liu]

On Thu, Jan 14, 2016 at 06:58:34PM +0530, Santosh Shukla wrote:
> Enable RTE_LIBRTE_VIRTIO_PMD for armv7/v8 and setting RTE_VIRTIO_INC_VEC=n.
> Builds successfully for armv7/v8.

I guess you could squash this patch to 2nd patch.

	--yliu

Re: [dpdk-dev] [PATCH v4 01/14] virtio: Introduce config RTE_VIRTIO_INC_VECTOR

[Yuanhan Liu]

On Thu, Jan 14, 2016 at 06:58:24PM +0530, Santosh Shukla wrote:
> virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx
> instructions. For arm64 in particular, virtio vector implementation does not
> exist(todo).
> 
> So virtio pmd driver wont build for targets like i686, arm64.  By making
> RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work
> in non-vectored virtio mode.

While revisiting this patch, I'm thinking you may squash both patch 2
and patch 11 into this one.

> 
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> ---
>  config/common_linuxapp           |    1 +
>  drivers/net/virtio/Makefile      |    2 +-
>  drivers/net/virtio/virtio_rxtx.c |    7 +++++++
>  3 files changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/config/common_linuxapp b/config/common_linuxapp
> index 74bc515..8677697 100644
> --- a/config/common_linuxapp
> +++ b/config/common_linuxapp
> @@ -274,6 +274,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DRIVER=n
>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
> +CONFIG_RTE_VIRTIO_INC_VECTOR=y
>  
>  #
>  # Compile burst-oriented VMXNET3 PMD driver
> diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
> index 43835ba..25a842d 100644
> --- a/drivers/net/virtio/Makefile
> +++ b/drivers/net/virtio/Makefile
> @@ -50,7 +50,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtqueue.c
>  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_pci.c
>  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx.c
>  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_ethdev.c
> -SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple.c
> +SRCS-$(CONFIG_RTE_VIRTIO_INC_VECTOR) += virtio_rxtx_simple.c
>  
>  # this lib depends upon:
>  DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_eal lib/librte_ether
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index 74b39ef..23be1ff 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -438,7 +438,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>  
>  	dev->data->rx_queues[queue_idx] = vq;
>  
> +#ifdef RTE_VIRTIO_INC_VECTOR
>  	virtio_rxq_vec_setup(vq);
> +#endif

You should put such macros for the declaration in virtio_rxtx.h as well.


And note that you may miss one:


    325                         /******************************************
    326                         *         Enqueue allocated buffers        *
    327                         *******************************************/
    328                         if (use_simple_rxtx)
==> 329                                 error = virtqueue_enqueue_recv_refill_simple(vq, m);
    330                         else
    331                                 error = virtqueue_enqueue_recv_refill(vq, m);
    332                         if (error) {
    333                                 rte_pktmbuf_free(m);
    334                                 break;
    335                         }


virtqueue_enqueue_recv_refill_simple() is defined inside virtio_rxtx_simple.c,
which is built only when CONFIG_RTE_VIRTIO_INC_VECTOR is set. But I see no
such check here.

Note that this will not break the build, as gcc just ignores it, for use_simple_rxtx
is 0 by default, thus the "if" part code is not compiled at all. Even for that,
I think it's better to put an explicit macro check here.

	--yliu

Re: [dpdk-dev] [PATCH v4 09/14] virtio: ethdev: check for vfio interface

[Santosh Shukla]

On Fri, Jan 15, 2016 at 12:05 PM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Thu, Jan 14, 2016 at 06:58:32PM +0530, Santosh Shukla wrote:
>> Introducing api to check interface type is vfio or not, if interface is vfio
>> then update struct virtio_vfio_dev {}.
>>
>> Those two apis are:
>> - virtio_chk_for_vfio
>> - virtio_hw_init_by_vfio
>>
>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> ..
>> +/* Init virtio by vfio-way */
>> +static int virtio_hw_init_by_vfio(struct virtio_hw *hw,
>> +                               struct rte_pci_device *pci_dev)
>> +{
>> +     struct virtio_vfio_dev *vdev;
>> +
>> +     vdev = &hw->dev;
>> +     if (virtio_chk_for_vfio(pci_dev) < 0) {
>> +             vdev->is_vfio = false;
>> +             vdev->pci_dev = NULL;
>> +             return -1;
>> +     }
>> +
>> +     /* .. So attached interface is vfio */
>> +     vdev->is_vfio = true;
>> +     vdev->pci_dev = pci_dev;
>
> Normally, I don't like the way of adding yet another "virtio_hw_init_by_xxx".
>
> As suggested in another reply, would pci_dev->kdrv checking be enough?
> If so, do it in simple way.
>

No, It wont be enough, Virtio could only work for vfio for _noiommu_
mode and for that user need to preset -noiommu parameter, therefore
virtio need to check that parame. pci_dev->kdrv check not enough,
Hence not used.

>         --ylu

Re: [dpdk-dev] [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface

[Santosh Shukla]

On Fri, Jan 15, 2016 at 11:57 AM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Thu, Jan 14, 2016 at 06:58:31PM +0530, Santosh Shukla wrote:
>> So far virtio handle rw access for uio / ioport interface, This patch to extend
>> the support for vfio interface. For that introducing private struct
>> virtio_vfio_dev{
>>       - is_vfio
>>       - pci_dev
>>       };
>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> ...
>> +/* For vfio only */
>> +struct virtio_vfio_dev {
>> +     bool            is_vfio;        /* True: vfio i/f,
>> +                                      * False: not a vfio i/f
>
> Well, this is weird; you are adding a flag to tell whether it's a
> vfio device __inside__ a vfio struct.
>
> Back to the topic, this flag is not necessary to me: you can
> check the pci_dev->kdrv flag.
>

yes, I'll replace is_vfio with pci_dev->kdrv.

>> +                                      */
>> +     struct rte_pci_device *pci_dev; /* vfio dev */
>
> Note that I have already added this field into virtio_hw struct
> at my latest virtio 1.0 pmd patchset.
>
> While I told you before that you should not develop patches based
> on my patcheset, I guess you can do that now. Since it should be
> in good shape and close to be merged.

Okay, Before rebasing my v5 patch on your 1.0 virtio patch, I like to
understand which qemu version support virtio 1.0 spec?
>
>> +};
>> +
>>  struct virtio_hw {
>>       struct virtqueue *cvq;
>>       uint32_t    io_base;
>> @@ -176,6 +186,7 @@ struct virtio_hw {
>>       uint8_t     use_msix;
>>       uint8_t     started;
>>       uint8_t     mac_addr[ETHER_ADDR_LEN];
>> +     struct virtio_vfio_dev dev;
>>  };
>>
>>  /*
>> @@ -231,20 +242,65 @@ outl_p(unsigned int data, unsigned int port)
>>  #define VIRTIO_PCI_REG_ADDR(hw, reg) \
>>       (unsigned short)((hw)->io_base + (reg))
>>
>> -#define VIRTIO_READ_REG_1(hw, reg) \
>> -     inb((VIRTIO_PCI_REG_ADDR((hw), (reg))))
>> -#define VIRTIO_WRITE_REG_1(hw, reg, value) \
>> -     outb_p((unsigned char)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
>> -
>> -#define VIRTIO_READ_REG_2(hw, reg) \
>> -     inw((VIRTIO_PCI_REG_ADDR((hw), (reg))))
>> -#define VIRTIO_WRITE_REG_2(hw, reg, value) \
>> -     outw_p((unsigned short)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
>> -
>> -#define VIRTIO_READ_REG_4(hw, reg) \
>> -     inl((VIRTIO_PCI_REG_ADDR((hw), (reg))))
>> -#define VIRTIO_WRITE_REG_4(hw, reg, value) \
>> -     outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
>> +#define VIRTIO_READ_REG_1(hw, reg)                                   \
>> +({                                                                   \
>> +     uint8_t ret;                                                    \
>> +     struct virtio_vfio_dev *vdev;                                   \
>> +     (vdev) = (&(hw)->dev);                                          \
>> +     (((vdev)->is_vfio) ?                                            \
>> +     (ioport_inb(((vdev)->pci_dev), reg, &ret)) :                    \
>> +     ((ret) = (inb((VIRTIO_PCI_REG_ADDR((hw), (reg)))))));           \
>> +     ret;                                                            \
>> +})
>
> It becomes unreadable. I'd suggest to define them as iniline
> functions, and use "if .. else .." instead of "?:".
>
>         --yliu

Re: [dpdk-dev] [PATCH v4 11/14] config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD

[Santosh Shukla]

On Fri, Jan 15, 2016 at 12:07 PM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Thu, Jan 14, 2016 at 06:58:34PM +0530, Santosh Shukla wrote:
>> Enable RTE_LIBRTE_VIRTIO_PMD for armv7/v8 and setting RTE_VIRTIO_INC_VEC=n.
>> Builds successfully for armv7/v8.
>
> I guess you could squash this patch to 2nd patch.
>

Yes.
>         --yliu

Re: [dpdk-dev] [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface

[Santosh Shukla]

On Fri, Jan 15, 2016 at 6:13 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> On Fri, Jan 15, 2016 at 11:57 AM, Yuanhan Liu
> <yuanhan.liu@linux.intel.com> wrote:
>> On Thu, Jan 14, 2016 at 06:58:31PM +0530, Santosh Shukla wrote:
>>> So far virtio handle rw access for uio / ioport interface, This patch to extend
>>> the support for vfio interface. For that introducing private struct
>>> virtio_vfio_dev{
>>>       - is_vfio
>>>       - pci_dev
>>>       };
>>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>> ...
>>> +/* For vfio only */
>>> +struct virtio_vfio_dev {
>>> +     bool            is_vfio;        /* True: vfio i/f,
>>> +                                      * False: not a vfio i/f
>>
>> Well, this is weird; you are adding a flag to tell whether it's a
>> vfio device __inside__ a vfio struct.
>>
>> Back to the topic, this flag is not necessary to me: you can
>> check the pci_dev->kdrv flag.
>>
>
> yes, I'll replace is_vfio with pci_dev->kdrv.
>
>>> +                                      */
>>> +     struct rte_pci_device *pci_dev; /* vfio dev */
>>
>> Note that I have already added this field into virtio_hw struct
>> at my latest virtio 1.0 pmd patchset.
>>
>> While I told you before that you should not develop patches based
>> on my patcheset, I guess you can do that now. Since it should be
>> in good shape and close to be merged.
>
> Okay, Before rebasing my v5 patch on your 1.0 virtio patch, I like to
> understand which qemu version support virtio 1.0 spec?

Ignore, I figured out in other thread,
qemu version >2.4, such as 2.4.1 or 2.5.0.

>>> +};
>>> +
>>>  struct virtio_hw {
>>>       struct virtqueue *cvq;
>>>       uint32_t    io_base;
>>> @@ -176,6 +186,7 @@ struct virtio_hw {
>>>       uint8_t     use_msix;
>>>       uint8_t     started;
>>>       uint8_t     mac_addr[ETHER_ADDR_LEN];
>>> +     struct virtio_vfio_dev dev;
>>>  };
>>>
>>>  /*
>>> @@ -231,20 +242,65 @@ outl_p(unsigned int data, unsigned int port)
>>>  #define VIRTIO_PCI_REG_ADDR(hw, reg) \
>>>       (unsigned short)((hw)->io_base + (reg))
>>>
>>> -#define VIRTIO_READ_REG_1(hw, reg) \
>>> -     inb((VIRTIO_PCI_REG_ADDR((hw), (reg))))
>>> -#define VIRTIO_WRITE_REG_1(hw, reg, value) \
>>> -     outb_p((unsigned char)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
>>> -
>>> -#define VIRTIO_READ_REG_2(hw, reg) \
>>> -     inw((VIRTIO_PCI_REG_ADDR((hw), (reg))))
>>> -#define VIRTIO_WRITE_REG_2(hw, reg, value) \
>>> -     outw_p((unsigned short)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
>>> -
>>> -#define VIRTIO_READ_REG_4(hw, reg) \
>>> -     inl((VIRTIO_PCI_REG_ADDR((hw), (reg))))
>>> -#define VIRTIO_WRITE_REG_4(hw, reg, value) \
>>> -     outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
>>> +#define VIRTIO_READ_REG_1(hw, reg)                                   \
>>> +({                                                                   \
>>> +     uint8_t ret;                                                    \
>>> +     struct virtio_vfio_dev *vdev;                                   \
>>> +     (vdev) = (&(hw)->dev);                                          \
>>> +     (((vdev)->is_vfio) ?                                            \
>>> +     (ioport_inb(((vdev)->pci_dev), reg, &ret)) :                    \
>>> +     ((ret) = (inb((VIRTIO_PCI_REG_ADDR((hw), (reg)))))));           \
>>> +     ret;                                                            \
>>> +})
>>
>> It becomes unreadable. I'd suggest to define them as iniline
>> functions, and use "if .. else .." instead of "?:".
>>

Ok.
>>         --yliu

Re: [dpdk-dev] [PATCH v4 01/14] virtio: Introduce config RTE_VIRTIO_INC_VECTOR

[Santosh Shukla]

On Fri, Jan 15, 2016 at 12:21 PM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Thu, Jan 14, 2016 at 06:58:24PM +0530, Santosh Shukla wrote:
>> virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx
>> instructions. For arm64 in particular, virtio vector implementation does not
>> exist(todo).
>>
>> So virtio pmd driver wont build for targets like i686, arm64.  By making
>> RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work
>> in non-vectored virtio mode.
>
> While revisiting this patch, I'm thinking you may squash both patch 2
> and patch 11 into this one.
>

Make sense!, we'll do in v5.

>>
>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>> ---
>>  config/common_linuxapp           |    1 +
>>  drivers/net/virtio/Makefile      |    2 +-
>>  drivers/net/virtio/virtio_rxtx.c |    7 +++++++
>>  3 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/config/common_linuxapp b/config/common_linuxapp
>> index 74bc515..8677697 100644
>> --- a/config/common_linuxapp
>> +++ b/config/common_linuxapp
>> @@ -274,6 +274,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
>>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
>>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DRIVER=n
>>  CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
>> +CONFIG_RTE_VIRTIO_INC_VECTOR=y
>>
>>  #
>>  # Compile burst-oriented VMXNET3 PMD driver
>> diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
>> index 43835ba..25a842d 100644
>> --- a/drivers/net/virtio/Makefile
>> +++ b/drivers/net/virtio/Makefile
>> @@ -50,7 +50,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtqueue.c
>>  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_pci.c
>>  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx.c
>>  SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_ethdev.c
>> -SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple.c
>> +SRCS-$(CONFIG_RTE_VIRTIO_INC_VECTOR) += virtio_rxtx_simple.c
>>
>>  # this lib depends upon:
>>  DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_eal lib/librte_ether
>> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
>> index 74b39ef..23be1ff 100644
>> --- a/drivers/net/virtio/virtio_rxtx.c
>> +++ b/drivers/net/virtio/virtio_rxtx.c
>> @@ -438,7 +438,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
>>
>>       dev->data->rx_queues[queue_idx] = vq;
>>
>> +#ifdef RTE_VIRTIO_INC_VECTOR
>>       virtio_rxq_vec_setup(vq);
>> +#endif
>
> You should put such macros for the declaration in virtio_rxtx.h as well.
>
>
> And note that you may miss one:
>
>
>     325                         /******************************************
>     326                         *         Enqueue allocated buffers        *
>     327                         *******************************************/
>     328                         if (use_simple_rxtx)
> ==> 329                                 error = virtqueue_enqueue_recv_refill_simple(vq, m);
>     330                         else
>     331                                 error = virtqueue_enqueue_recv_refill(vq, m);
>     332                         if (error) {
>     333                                 rte_pktmbuf_free(m);
>     334                                 break;
>     335                         }
>
>
> virtqueue_enqueue_recv_refill_simple() is defined inside virtio_rxtx_simple.c,
> which is built only when CONFIG_RTE_VIRTIO_INC_VECTOR is set. But I see no
> such check here.
>
> Note that this will not break the build, as gcc just ignores it, for use_simple_rxtx
> is 0 by default, thus the "if" part code is not compiled at all. Even for that,
> I think it's better to put an explicit macro check here.
>

we'll make changes in v5, Thanks for review comment.

>         --yliu

Re: [dpdk-dev] [PATCH v4 06/14] eal: pci: vfio: add rd/wr func for pci bar space

[Santosh Shukla]

On Fri, Jan 15, 2016 at 11:18 AM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Thu, Jan 14, 2016 at 06:58:29PM +0530, Santosh Shukla wrote:
> ...
>> +int rte_eal_pci_read_bar(const struct rte_pci_device *device,
>> +                      void *buf, size_t len, off_t offset,
>> +                      int bar_idx)
>> +
>> +{
>> +#ifdef VFIO_PRESENT
>> +     const struct rte_intr_handle *intr_handle = &device->intr_handle;
>> +     return pci_vfio_read_bar(intr_handle, buf, len, offset, bar_idx);
>> +#else
>> +     /* UIO's not applicable */
>> +     RTE_SET_USED(device);
>> +     RTE_SET_USED(buf);
>> +     RTE_SET_USED(len);
>> +     RTE_SET_USED(offset);
>> +     RTE_SET_USED(bar_idx);
>> +     return 0;
>> +#endif
>
> Maybe you could do that like what pci_map_device() does:
>
>         switch(dev->kdrv) {
>         case RTE_KDRV_NIC_VFIO:
>                 return pci_vfio_read_bar(..);
>                 break;
>         default:
>                 RTE_LOG(.... not supported by driver: %d\n"..);
>                 break;
>         }
>
> With that, you could get rid of those ugly RTE_SET_USED....
>

Even better, Thanks!
>         --yliu

Re: [dpdk-dev] [PATCH v4 07/14] virtio: vfio: add api support to rd/wr ioport bar

[Santosh Shukla]

On Fri, Jan 15, 2016 at 11:33 AM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Thu, Jan 14, 2016 at 06:58:30PM +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>
>> ---
> ...
>> +/* vfio rd/rw virtio apis */
>> +static inline void ioport_inb(const struct rte_pci_device *pci_dev,
>> +                           uint8_t reg, uint8_t *val)
>
> Minor nit: dpdk perfers to seperate return type and function name in
> different line:
>
>   static inline void
>   ioport_inb(....)
>   {
>

ok.
>> +{
>> +     if (rte_eal_pci_read_bar(pci_dev, (uint8_t *)val, sizeof(uint8_t), reg,
>                                           ^^^^^^^^^^^
>
> Unnecessary cast; and few more belows.
>

yes,
>         --yliu

Re: [dpdk-dev] [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface

[Yuanhan Liu]

On Fri, Jan 15, 2016 at 07:12:04PM +0530, Santosh Shukla wrote:
> On Fri, Jan 15, 2016 at 6:13 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> > On Fri, Jan 15, 2016 at 11:57 AM, Yuanhan Liu
> > <yuanhan.liu@linux.intel.com> wrote:
> >> On Thu, Jan 14, 2016 at 06:58:31PM +0530, Santosh Shukla wrote:
> >>> So far virtio handle rw access for uio / ioport interface, This patch to extend
> >>> the support for vfio interface. For that introducing private struct
> >>> virtio_vfio_dev{
> >>>       - is_vfio
> >>>       - pci_dev
> >>>       };
> >>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> >> ...
> >>> +/* For vfio only */
> >>> +struct virtio_vfio_dev {
> >>> +     bool            is_vfio;        /* True: vfio i/f,
> >>> +                                      * False: not a vfio i/f
> >>
> >> Well, this is weird; you are adding a flag to tell whether it's a
> >> vfio device __inside__ a vfio struct.
> >>
> >> Back to the topic, this flag is not necessary to me: you can
> >> check the pci_dev->kdrv flag.
> >>
> >
> > yes, I'll replace is_vfio with pci_dev->kdrv.
> >
> >>> +                                      */
> >>> +     struct rte_pci_device *pci_dev; /* vfio dev */
> >>
> >> Note that I have already added this field into virtio_hw struct
> >> at my latest virtio 1.0 pmd patchset.
> >>
> >> While I told you before that you should not develop patches based
> >> on my patcheset, I guess you can do that now. Since it should be
> >> in good shape and close to be merged.
> >
> > Okay, Before rebasing my v5 patch on your 1.0 virtio patch, I like to
> > understand which qemu version support virtio 1.0 spec?
> 
> Ignore, I figured out in other thread,
> qemu version >2.4, such as 2.4.1 or 2.5.0.

It will not matter. You can continue using the old legacy virtio, which
is the default case: my patchset keeps the backward compatibility.

What's worty noting is that virtio 1.0 uses memory mmaped bar space for
configuration, instead of ioport reading/writing. Therefore, I'd suggest
you to keep testing with legacy virtio, to make sure the VFIO stuff works.
And off course, virtio 1.0 testing is also welcome, to make sure it works
on ARM as well.

	--yliu

Re: [dpdk-dev] [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface

[Santosh Shukla]

On Mon, Jan 18, 2016 at 11:41 AM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Fri, Jan 15, 2016 at 07:12:04PM +0530, Santosh Shukla wrote:
>> On Fri, Jan 15, 2016 at 6:13 PM, Santosh Shukla <sshukla@mvista.com> wrote:
>> > On Fri, Jan 15, 2016 at 11:57 AM, Yuanhan Liu
>> > <yuanhan.liu@linux.intel.com> wrote:
>> >> On Thu, Jan 14, 2016 at 06:58:31PM +0530, Santosh Shukla wrote:
>> >>> So far virtio handle rw access for uio / ioport interface, This patch to extend
>> >>> the support for vfio interface. For that introducing private struct
>> >>> virtio_vfio_dev{
>> >>>       - is_vfio
>> >>>       - pci_dev
>> >>>       };
>> >>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>> >> ...
>> >>> +/* For vfio only */
>> >>> +struct virtio_vfio_dev {
>> >>> +     bool            is_vfio;        /* True: vfio i/f,
>> >>> +                                      * False: not a vfio i/f
>> >>
>> >> Well, this is weird; you are adding a flag to tell whether it's a
>> >> vfio device __inside__ a vfio struct.
>> >>
>> >> Back to the topic, this flag is not necessary to me: you can
>> >> check the pci_dev->kdrv flag.
>> >>
>> >
>> > yes, I'll replace is_vfio with pci_dev->kdrv.
>> >
>> >>> +                                      */
>> >>> +     struct rte_pci_device *pci_dev; /* vfio dev */
>> >>
>> >> Note that I have already added this field into virtio_hw struct
>> >> at my latest virtio 1.0 pmd patchset.
>> >>
>> >> While I told you before that you should not develop patches based
>> >> on my patcheset, I guess you can do that now. Since it should be
>> >> in good shape and close to be merged.
>> >
>> > Okay, Before rebasing my v5 patch on your 1.0 virtio patch, I like to
>> > understand which qemu version support virtio 1.0 spec?
>>
>> Ignore, I figured out in other thread,
>> qemu version >2.4, such as 2.4.1 or 2.5.0.
>
> It will not matter. You can continue using the old legacy virtio, which
> is the default case: my patchset keeps the backward compatibility.
>
> What's worty noting is that virtio 1.0 uses memory mmaped bar space for
> configuration, instead of ioport reading/writing. Therefore, I'd suggest
> you to keep testing with legacy virtio, to make sure the VFIO stuff works.
> And off course, virtio 1.0 testing is also welcome, to make sure it works
> on ARM as well.
>

I am testing for virtio 1.0 and 0.95 for arm including your patch,
soon we;ll post the patch series that is rebased on / dependent on
below patchset:
- virtio 1.0
- vfio-noiommu
- KDRV check by huawei

IMO, we should start merging the dependent patches as because I'll
have to rebase, then do regression across the platform at least for
x86/arm64 and it's quite a work now.

Beside that I have few question specific to vfio in virtio pmd driver;
- vfio don't need resource_init functionality as it uses struct
rte_pci_dev but it need parsing so to make sure
    1. user has setted no_iommu mode
    2. virtio pci device attached to vfio-no-iommu driver or not.

So for 1) I am thinking to add RTE_KDRV_VFIO_NOIOMMU mode and a helper
function like pci_vfio_is_iommu(), such that  pc_xxx_scan() function
updates dev->kdrv with RTE_KDRV_VFIO_NOIOMMU at driver probe time.

case 2) would check for _noiommu mode and then would verify that
driver is attached or not?

above two case applicable to both virtio spec 1.0 and 0.95. I have
done changes for those two case for v5 patch series,l any comment
welcome before I push patch for review.

Thanks.

>         --yliu

Re: [dpdk-dev] [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface

[Jason Wang]

On 01/14/2016 09:28 PM, Santosh Shukla wrote:
> So far virtio handle rw access for uio / ioport interface, This patch to extend
> the support for vfio interface. For that introducing private struct
> virtio_vfio_dev{
> 	- is_vfio
> 	- pci_dev
> 	};
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> ---
> v3->v4:
> - Removed #indef RTE_EAL_VFIO and made it arch agnostic such now virtio_pci
>   rd/wr api to handle both vfio and ig_uio/ioport interfaces, depending upon
>   is_vfio flags set or unset.
> - Tested for x86 for igb_uio and vfio interface, also  tested for arm64 for vfio
>   interface.
>
> drivers/net/virtio/virtio_pci.h |   84 ++++++++++++++++++++++++++++++++-------
>  1 file changed, 70 insertions(+), 14 deletions(-)

Interesting. I'm working on IOMMU cooperation with virtio[1]. For pmd,
it looks like the only thing missed is RTE_PCI_DRV_NEED_MAPPING for
virito-net pmd.

So I'm curious whether this is still necessary if IOMMU can work with
virito in the future.

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg337079.html

Thanks

>
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index 8b5b031..8526c07 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -46,6 +46,8 @@
>  #endif
>  
>  #include <rte_ethdev.h>
> +#include <stdbool.h>
> +#include "virtio_vfio_rw.h"
>  
>  struct virtqueue;
>  
> @@ -165,6 +167,14 @@ struct virtqueue;
>   */
>  #define VIRTIO_MAX_VIRTQUEUES 8
>  
> +/* For vfio only */
> +struct virtio_vfio_dev {
> +	bool		is_vfio;	/* True: vfio i/f,
> +					 * False: not a vfio i/f
> +					 */
> +	struct rte_pci_device *pci_dev; /* vfio dev */
> +};
> +
>  struct virtio_hw {
>  	struct virtqueue *cvq;
>  	uint32_t    io_base;
> @@ -176,6 +186,7 @@ struct virtio_hw {
>  	uint8_t	    use_msix;
>  	uint8_t     started;
>  	uint8_t     mac_addr[ETHER_ADDR_LEN];
> +	struct virtio_vfio_dev dev;
>  };
>  
>  /*
> @@ -231,20 +242,65 @@ outl_p(unsigned int data, unsigned int port)
>  #define VIRTIO_PCI_REG_ADDR(hw, reg) \
>  	(unsigned short)((hw)->io_base + (reg))
>  
> -#define VIRTIO_READ_REG_1(hw, reg) \
> -	inb((VIRTIO_PCI_REG_ADDR((hw), (reg))))
> -#define VIRTIO_WRITE_REG_1(hw, reg, value) \
> -	outb_p((unsigned char)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
> -
> -#define VIRTIO_READ_REG_2(hw, reg) \
> -	inw((VIRTIO_PCI_REG_ADDR((hw), (reg))))
> -#define VIRTIO_WRITE_REG_2(hw, reg, value) \
> -	outw_p((unsigned short)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
> -
> -#define VIRTIO_READ_REG_4(hw, reg) \
> -	inl((VIRTIO_PCI_REG_ADDR((hw), (reg))))
> -#define VIRTIO_WRITE_REG_4(hw, reg, value) \
> -	outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
> +#define VIRTIO_READ_REG_1(hw, reg)					\
> +({									\
> +	uint8_t ret;							\
> +	struct virtio_vfio_dev *vdev;					\
> +	(vdev) = (&(hw)->dev);						\
> +	(((vdev)->is_vfio) ?						\
> +	(ioport_inb(((vdev)->pci_dev), reg, &ret)) :			\
> +	((ret) = (inb((VIRTIO_PCI_REG_ADDR((hw), (reg)))))));		\
> +	ret;								\
> +})
> +
> +#define VIRTIO_WRITE_REG_1(hw, reg, value)				\
> +({									\
> +	struct virtio_vfio_dev *vdev;					\
> +	(vdev) = (&(hw)->dev);						\
> +	(((vdev)->is_vfio) ?						\
> +	(ioport_outb_p(((vdev)->pci_dev), reg, (uint8_t)(value))) :	\
> +	(outb_p((unsigned char)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))))); \
> +})
> +
> +#define VIRTIO_READ_REG_2(hw, reg)					\
> +({									\
> +	uint16_t ret;							\
> +	struct virtio_vfio_dev *vdev;					\
> +	(vdev) = (&(hw)->dev);						\
> +	(((vdev)->is_vfio) ?						\
> +	(ioport_inw(((vdev)->pci_dev), reg, &ret)) :			\
> +	((ret) = (inw((VIRTIO_PCI_REG_ADDR((hw), (reg)))))));		\
> +	ret;								\
> +})
> +
> +#define VIRTIO_WRITE_REG_2(hw, reg, value)				\
> +({									\
> +	struct virtio_vfio_dev *vdev;					\
> +	(vdev) = (&(hw)->dev);						\
> +	(((vdev)->is_vfio) ?						\
> +	(ioport_outw_p(((vdev)->pci_dev), reg, (uint16_t)(value))) :	\
> +	(outw_p((unsigned short)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))))); \
> +})
> +
> +#define VIRTIO_READ_REG_4(hw, reg)					\
> +({									\
> +	uint32_t ret;							\
> +	struct virtio_vfio_dev *vdev;					\
> +	(vdev) = (&(hw)->dev);						\
> +	(((vdev)->is_vfio) ?						\
> +	(ioport_inl(((vdev)->pci_dev), reg, &ret)) :			\
> +	((ret) = (inl((VIRTIO_PCI_REG_ADDR((hw), (reg)))))));		\
> +	ret;								\
> +})
> +
> +#define VIRTIO_WRITE_REG_4(hw, reg, value)				\
> +({									\
> +	struct virtio_vfio_dev *vdev;					\
> +	(vdev) = (&(hw)->dev);						\
> +	(((vdev)->is_vfio) ?						\
> +	(ioport_outl_p(((vdev)->pci_dev), reg, (uint32_t)(value))) :	\
> +	(outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))))); \
> +})
>  
>  static inline int
>  vtpci_with_feature(struct virtio_hw *hw, uint32_t bit)

Re: [dpdk-dev] [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface

[Yuanhan Liu]

On Mon, Jan 18, 2016 at 12:15:40PM +0530, Santosh Shukla wrote:
> I am testing for virtio 1.0 and 0.95 for arm including your patch,
> soon we;ll post the patch series that is rebased on / dependent on
> below patchset:
> - virtio 1.0
> - vfio-noiommu
> - KDRV check by huawei
> 
> IMO, we should start merging the dependent patches as because I'll

Yep, agreed. That's why I was keep pushing Huawei for ACK and
validation team for testing, although I have tested that. :)

> have to rebase, then do regression across the platform at least for
> x86/arm64 and it's quite a work now.
> 
> Beside that I have few question specific to vfio in virtio pmd driver;
> - vfio don't need resource_init functionality as it uses struct
> rte_pci_dev but it need parsing so to make sure
>     1. user has setted no_iommu mode
>     2. virtio pci device attached to vfio-no-iommu driver or not.
> 
> So for 1) I am thinking to add RTE_KDRV_VFIO_NOIOMMU mode and a helper
> function like pci_vfio_is_iommu(), such that  pc_xxx_scan() function
> updates dev->kdrv with RTE_KDRV_VFIO_NOIOMMU at driver probe time.

That sounds better to me. And that's also what I want to comment on
your another patch [09/14], that we should try to avoid getting UIO/VFIO
stuff inside virtio pmd driver, unless it's a must. (yes, I know
UIO is already an example here, but I don't like it, and badly, I don't
have the time to check if I can remove it.)

> 
> case 2) would check for _noiommu mode and then would verify that
> driver is attached or not?

Sorry, very limited VFIO and noiommu knowledge, and I can't answer, so
far.

	--yliu
> 
> above two case applicable to both virtio spec 1.0 and 0.95. I have
> done changes for those two case for v5 patch series,l any comment
> welcome before I push patch for review.
> 
> Thanks.

Re: [dpdk-dev] [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface

[Santosh Shukla]

On Mon, Jan 18, 2016 at 12:29 PM, Jason Wang <jasowang@redhat.com> wrote:
>
>
> On 01/14/2016 09:28 PM, Santosh Shukla wrote:
>> So far virtio handle rw access for uio / ioport interface, This patch to extend
>> the support for vfio interface. For that introducing private struct
>> virtio_vfio_dev{
>>       - is_vfio
>>       - pci_dev
>>       };
>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>> ---
>> v3->v4:
>> - Removed #indef RTE_EAL_VFIO and made it arch agnostic such now virtio_pci
>>   rd/wr api to handle both vfio and ig_uio/ioport interfaces, depending upon
>>   is_vfio flags set or unset.
>> - Tested for x86 for igb_uio and vfio interface, also  tested for arm64 for vfio
>>   interface.
>>
>> drivers/net/virtio/virtio_pci.h |   84 ++++++++++++++++++++++++++++++++-------
>>  1 file changed, 70 insertions(+), 14 deletions(-)
>
> Interesting. I'm working on IOMMU cooperation with virtio[1]. For pmd,
> it looks like the only thing missed is RTE_PCI_DRV_NEED_MAPPING for
> virito-net pmd.
>

not missing anymore, I am using rte_eal_pci_map() api so to map virtio
pci dev to userspace.

> So I'm curious whether this is still necessary if IOMMU can work with
> virito in the future.
>
So far I'm using pmd driver in vfio-noiommu way. AFAIk, pmd driver use
dma for tx side. I glanced through your patch, to me it would be
interesting to try out, but right now I am not sure. We'll come back I
guess. Thanks

> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg337079.html
>
> Thanks
>
>>
>> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
>> index 8b5b031..8526c07 100644
>> --- a/drivers/net/virtio/virtio_pci.h
>> +++ b/drivers/net/virtio/virtio_pci.h
>> @@ -46,6 +46,8 @@
>>  #endif
>>
>>  #include <rte_ethdev.h>
>> +#include <stdbool.h>
>> +#include "virtio_vfio_rw.h"
>>
>>  struct virtqueue;
>>
>> @@ -165,6 +167,14 @@ struct virtqueue;
>>   */
>>  #define VIRTIO_MAX_VIRTQUEUES 8
>>
>> +/* For vfio only */
>> +struct virtio_vfio_dev {
>> +     bool            is_vfio;        /* True: vfio i/f,
>> +                                      * False: not a vfio i/f
>> +                                      */
>> +     struct rte_pci_device *pci_dev; /* vfio dev */
>> +};
>> +
>>  struct virtio_hw {
>>       struct virtqueue *cvq;
>>       uint32_t    io_base;
>> @@ -176,6 +186,7 @@ struct virtio_hw {
>>       uint8_t     use_msix;
>>       uint8_t     started;
>>       uint8_t     mac_addr[ETHER_ADDR_LEN];
>> +     struct virtio_vfio_dev dev;
>>  };
>>
>>  /*
>> @@ -231,20 +242,65 @@ outl_p(unsigned int data, unsigned int port)
>>  #define VIRTIO_PCI_REG_ADDR(hw, reg) \
>>       (unsigned short)((hw)->io_base + (reg))
>>
>> -#define VIRTIO_READ_REG_1(hw, reg) \
>> -     inb((VIRTIO_PCI_REG_ADDR((hw), (reg))))
>> -#define VIRTIO_WRITE_REG_1(hw, reg, value) \
>> -     outb_p((unsigned char)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
>> -
>> -#define VIRTIO_READ_REG_2(hw, reg) \
>> -     inw((VIRTIO_PCI_REG_ADDR((hw), (reg))))
>> -#define VIRTIO_WRITE_REG_2(hw, reg, value) \
>> -     outw_p((unsigned short)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
>> -
>> -#define VIRTIO_READ_REG_4(hw, reg) \
>> -     inl((VIRTIO_PCI_REG_ADDR((hw), (reg))))
>> -#define VIRTIO_WRITE_REG_4(hw, reg, value) \
>> -     outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg))))
>> +#define VIRTIO_READ_REG_1(hw, reg)                                   \
>> +({                                                                   \
>> +     uint8_t ret;                                                    \
>> +     struct virtio_vfio_dev *vdev;                                   \
>> +     (vdev) = (&(hw)->dev);                                          \
>> +     (((vdev)->is_vfio) ?                                            \
>> +     (ioport_inb(((vdev)->pci_dev), reg, &ret)) :                    \
>> +     ((ret) = (inb((VIRTIO_PCI_REG_ADDR((hw), (reg)))))));           \
>> +     ret;                                                            \
>> +})
>> +
>> +#define VIRTIO_WRITE_REG_1(hw, reg, value)                           \
>> +({                                                                   \
>> +     struct virtio_vfio_dev *vdev;                                   \
>> +     (vdev) = (&(hw)->dev);                                          \
>> +     (((vdev)->is_vfio) ?                                            \
>> +     (ioport_outb_p(((vdev)->pci_dev), reg, (uint8_t)(value))) :     \
>> +     (outb_p((unsigned char)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))))); \
>> +})
>> +
>> +#define VIRTIO_READ_REG_2(hw, reg)                                   \
>> +({                                                                   \
>> +     uint16_t ret;                                                   \
>> +     struct virtio_vfio_dev *vdev;                                   \
>> +     (vdev) = (&(hw)->dev);                                          \
>> +     (((vdev)->is_vfio) ?                                            \
>> +     (ioport_inw(((vdev)->pci_dev), reg, &ret)) :                    \
>> +     ((ret) = (inw((VIRTIO_PCI_REG_ADDR((hw), (reg)))))));           \
>> +     ret;                                                            \
>> +})
>> +
>> +#define VIRTIO_WRITE_REG_2(hw, reg, value)                           \
>> +({                                                                   \
>> +     struct virtio_vfio_dev *vdev;                                   \
>> +     (vdev) = (&(hw)->dev);                                          \
>> +     (((vdev)->is_vfio) ?                                            \
>> +     (ioport_outw_p(((vdev)->pci_dev), reg, (uint16_t)(value))) :    \
>> +     (outw_p((unsigned short)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))))); \
>> +})
>> +
>> +#define VIRTIO_READ_REG_4(hw, reg)                                   \
>> +({                                                                   \
>> +     uint32_t ret;                                                   \
>> +     struct virtio_vfio_dev *vdev;                                   \
>> +     (vdev) = (&(hw)->dev);                                          \
>> +     (((vdev)->is_vfio) ?                                            \
>> +     (ioport_inl(((vdev)->pci_dev), reg, &ret)) :                    \
>> +     ((ret) = (inl((VIRTIO_PCI_REG_ADDR((hw), (reg)))))));           \
>> +     ret;                                                            \
>> +})
>> +
>> +#define VIRTIO_WRITE_REG_4(hw, reg, value)                           \
>> +({                                                                   \
>> +     struct virtio_vfio_dev *vdev;                                   \
>> +     (vdev) = (&(hw)->dev);                                          \
>> +     (((vdev)->is_vfio) ?                                            \
>> +     (ioport_outl_p(((vdev)->pci_dev), reg, (uint32_t)(value))) :    \
>> +     (outl_p((unsigned int)(value), (VIRTIO_PCI_REG_ADDR((hw), (reg)))))); \
>> +})
>>
>>  static inline int
>>  vtpci_with_feature(struct virtio_hw *hw, uint32_t bit)
>

Re: [dpdk-dev] [PATCH v4 08/14] virtio: pci: extend virtio pci rw api for vfio interface

[Santosh Shukla]

On Mon, Jan 18, 2016 at 12:47 PM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Mon, Jan 18, 2016 at 12:15:40PM +0530, Santosh Shukla wrote:
>> I am testing for virtio 1.0 and 0.95 for arm including your patch,
>> soon we;ll post the patch series that is rebased on / dependent on
>> below patchset:
>> - virtio 1.0
>> - vfio-noiommu
>> - KDRV check by huawei
>>
>> IMO, we should start merging the dependent patches as because I'll
>
> Yep, agreed. That's why I was keep pushing Huawei for ACK and
> validation team for testing, although I have tested that. :)
>
>> have to rebase, then do regression across the platform at least for
>> x86/arm64 and it's quite a work now.
>>
>> Beside that I have few question specific to vfio in virtio pmd driver;
>> - vfio don't need resource_init functionality as it uses struct
>> rte_pci_dev but it need parsing so to make sure
>>     1. user has setted no_iommu mode
>>     2. virtio pci device attached to vfio-no-iommu driver or not.
>>
>> So for 1) I am thinking to add RTE_KDRV_VFIO_NOIOMMU mode and a helper
>> function like pci_vfio_is_iommu(), such that  pc_xxx_scan() function
>> updates dev->kdrv with RTE_KDRV_VFIO_NOIOMMU at driver probe time.
>
> That sounds better to me. And that's also what I want to comment on
> your another patch [09/14], that we should try to avoid getting UIO/VFIO
> stuff inside virtio pmd driver, unless it's a must. (yes, I know
> UIO is already an example here, but I don't like it, and badly, I don't
> have the time to check if I can remove it.)
>

Understood, So I'm moving possible required driver/parsing check in
eal_pci.c rather keeping it in virtio, as those parsing/driver
dependency checks are generic, has nothing to do with virtio.

>>
>> case 2) would check for _noiommu mode and then would verify that
>> driver is attached or not?
>
> Sorry, very limited VFIO and noiommu knowledge, and I can't answer, so
> far.
>
>         --yliu
>>
>> above two case applicable to both virtio spec 1.0 and 0.95. I have
>> done changes for those two case for v5 patch series,l any comment
>> welcome before I push patch for review.
>>
>> Thanks.

Re: [dpdk-dev] [PATCH v4 00/14] Add virtio support for arm/arm64

[Xie, Huawei]

On 1/14/2016 9:29 PM, Santosh Shukla wrote:
> Hi,
>
> This v4 patch uses vfio-noiommu-way to access virtio-net pci interface.
> Tested for arm64 thunderX platform. Patch builds for
> x86/i386/arm/armv8/thunderX. Tested with testpmd application.

Hi Shukla:
I would take two week's leave from tomorrow and will not have enough
bandwidth to review your code. I think yuanhan will do that. Sorry for
the inconvenience.
Btw, when you send a new version, could you add --in-reply-to so that
all your patches are within one thread?

Re: [dpdk-dev] [PATCH v4 00/14] Add virtio support for arm/arm64

[Santosh Shukla]

On Fri, Jan 29, 2016 at 12:57 PM, Xie, Huawei <huawei.xie@intel.com> wrote:
> On 1/14/2016 9:29 PM, Santosh Shukla wrote:
>> Hi,
>>
>> This v4 patch uses vfio-noiommu-way to access virtio-net pci interface.
>> Tested for arm64 thunderX platform. Patch builds for
>> x86/i386/arm/armv8/thunderX. Tested with testpmd application.
>
> Hi Shukla:
> I would take two week's leave from tomorrow and will not have enough
> bandwidth to review your code. I think yuanhan will do that. Sorry for
> the inconvenience.
> Btw, when you send a new version, could you add --in-reply-to so that
> all your patches are within one thread?

Okay,

BTW, v4 is old version now and v5 is recent and I guess I recieved
review comment from Yuan/Thomas, sending v6.. now.