[PATCH v5 00/11] Add virtio support for arm/arm64

[PATCH v5 00/11] Add virtio support for arm/arm64

[Santosh Shukla]

Hi,

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

Patchset rebased on yuan's under review virtio-1.0 v2 patchset and using
vfio-noiommu patch. Refer my public branch [1]. 

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.

v4 --> v5:
- Introducing RTE_KDRV_VFIO_NOIOMMU driver mode
- Incorporated v4 review comments, Pl. refer each patchset for review change.

For older version(v4.. v1) patch history, refer [2].

Thanks.

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


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

Santosh Shukla (10):
  virtio: Introduce config RTE_VIRTIO_INC_VECTOR
  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
  eal: pci: introduce RTE_KDRV_VFIO_NOIOMMUi driver mode
  virtio_pci: do not parse if interface is vfio-noiommu
  virtio: pci: add dummy func definition for in/outb for non-x86 arch

 doc/guides/rel_notes/release_2_3.rst            |    3 -
 drivers/net/virtio/virtio_ethdev.c              |  302 ++++++++-
 drivers/net/virtio/virtio_ethdev.h              |    3 +-
 drivers/net/virtio/virtio_pci.c                 |  793 +----------------------
 drivers/net/virtio/virtio_pci.h                 |  120 +---
 drivers/net/virtio/virtio_rxtx.c                |   21 +-
 drivers/net/virtio/virtio_rxtx_simple.c         |   12 +-
 drivers/net/virtio/virtqueue.h                  |    4 +-
 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 -
 15 files changed, 380 insertions(+), 949 deletions(-)

-- 
1.7.9.5

[PATCH v5 01/11] 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.

Disabling RTE_VIRTIO_INC_VECTOR config for :

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

- armv7/v8 arch.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v4--> v5:
- squashed v4's RTE_VIRTIO_INC_VECTOR patches into one patch.
- Added ifdefs RTE_xx_xx_INC_VECTOR across _simple_rx_tx flag occurance in code.
 

 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_rxtx.c           |   16 +++++++++++++++-
 drivers/net/virtio/virtio_rxtx.h           |    2 ++
 8 files changed, 27 insertions(+), 4 deletions(-)

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/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
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
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 41a1366..d8169d1 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -67,7 +67,9 @@
 #define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
 	ETH_TXQ_FLAGS_NOOFFLOADS)
 
+#ifdef RTE_VIRTIO_INC_VECTOR
 static int use_simple_rxtx;
+#endif
 
 static void
 vq_ring_free_chain(struct virtqueue *vq, uint16_t desc_idx)
@@ -307,12 +309,13 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 		nbufs = 0;
 		error = ENOSPC;
 
+#ifdef RTE_VIRTIO_INC_VECTOR
 		if (use_simple_rxtx)
 			for (i = 0; i < vq->vq_nentries; i++) {
 				vq->vq_ring.avail->ring[i] = i;
 				vq->vq_ring.desc[i].flags = VRING_DESC_F_WRITE;
 			}
-
+#endif
 		memset(&vq->fake_mbuf, 0, sizeof(vq->fake_mbuf));
 		for (i = 0; i < RTE_PMD_VIRTIO_RX_MAX_BURST; i++)
 			vq->sw_ring[vq->vq_nentries + i] = &vq->fake_mbuf;
@@ -325,9 +328,11 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 			/******************************************
 			*         Enqueue allocated buffers        *
 			*******************************************/
+#ifdef RTE_VIRTIO_INC_VECTOR
 			if (use_simple_rxtx)
 				error = virtqueue_enqueue_recv_refill_simple(vq, m);
 			else
+#endif
 				error = virtqueue_enqueue_recv_refill(vq, m);
 			if (error) {
 				rte_pktmbuf_free(m);
@@ -340,6 +345,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 
 		PMD_INIT_LOG(DEBUG, "Allocated %d bufs", nbufs);
 	} else if (queue_type == VTNET_TQ) {
+#ifdef RTE_VIRTIO_INC_VECTOR
 		if (use_simple_rxtx) {
 			int mid_idx  = vq->vq_nentries >> 1;
 			for (i = 0; i < mid_idx; i++) {
@@ -357,6 +363,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int queue_type)
 			for (i = mid_idx; i < vq->vq_nentries; i++)
 				vq->vq_ring.avail->ring[i] = i;
 		}
+#endif
 	}
 }
 
@@ -423,7 +430,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;
 }
@@ -449,7 +458,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;
@@ -462,6 +474,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)) {
@@ -470,6 +483,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);
diff --git a/drivers/net/virtio/virtio_rxtx.h b/drivers/net/virtio/virtio_rxtx.h
index 831e492..9be1378 100644
--- a/drivers/net/virtio/virtio_rxtx.h
+++ b/drivers/net/virtio/virtio_rxtx.h
@@ -33,7 +33,9 @@
 
 #define RTE_PMD_VIRTIO_RX_MAX_BURST 64
 
+#ifdef RTE_VIRTIO_INC_VECTOR
 int virtio_rxq_vec_setup(struct virtqueue *rxq);
 
 int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
 	struct rte_mbuf *m);
+#endif
-- 
1.7.9.5

[PATCH v5 02/11] 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>
Acked-by: Jan Viktorin <viktorin@rehivetech.com>
Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
---
 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

[PATCH v5 03/11] 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>
---
 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

[PATCH v5 04/11] 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 99572a0..f550d22 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

[PATCH v5 05/11] 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>
---
v4--> v5:
- added switch case in rd/wr pci_bar routine, as per Yuan comment

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

diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 2224109..0c667ff 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 db947da..eb503f0 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -621,6 +621,40 @@ 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)
+
+{
+	const struct rte_intr_handle *intr_handle = &device->intr_handle;
+
+	switch (device->kdrv) {
+	case RTE_KDRV_VFIO:
+		return pci_vfio_read_bar(intr_handle, buf, len,
+					 offset, bar_idx);
+	default:
+		RTE_LOG(ERR, EAL, "write bar not supported by driver\n");
+		return -1;
+	}
+}
+
+int rte_eal_pci_write_bar(const struct rte_pci_device *device,
+			  const void *buf, size_t len, off_t offset,
+			  int bar_idx)
+{
+
+	const struct rte_intr_handle *intr_handle = &device->intr_handle;
+
+	switch (device->kdrv) {
+	case RTE_KDRV_VFIO:
+		return pci_vfio_write_bar(intr_handle, buf, len,
+					  offset, bar_idx);
+	default:
+		RTE_LOG(ERR, EAL, "write bar not supported by driver\n");
+		return -1;
+	}
+}
+
 /* 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

[PATCH v5 06/11] virtio: vfio: add api support to rd/wr ioport bar

[Santosh Shukla]

For vfio case - Use pread/pwrite api to access virtio
ioport space.

Applicable for virtio 0.95 spec.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
Signed-off-by: Rizwan Ansari <ransari@mvista.com>
Signed-off-by: Rakesh Krishnamurthy <rakeshk@mvista.com>
---
v4 --> v5:
- Removed unnecessary type casting.

 drivers/net/virtio/virtio_vfio_rw.h |  104 +++++++++++++++++++++++++++++++++++
 1 file changed, 104 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..a9ab04d
--- /dev/null
+++ b/drivers/net/virtio/virtio_vfio_rw.h
@@ -0,0 +1,104 @@
+/*
+ *   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, 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, 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, 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, &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, &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, &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

[PATCH v5 07/11] 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.

Applicable for virtio 0.95 spec.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v4--> v5:
- Replaced virtio_rd/wr_1/2/4() macro implementation with inline function, per
  Yuan review commment.

 drivers/net/virtio/virtio_pci.c |  110 ++++++++++++++++++++++++++++++++++-----
 1 file changed, 97 insertions(+), 13 deletions(-)

diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index a9f179f..0c29f1d 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -40,6 +40,7 @@
 #include "virtio_pci.h"
 #include "virtio_logs.h"
 #include "virtqueue.h"
+#include "virtio_vfio_rw.h"
 
 /*
  * Following macros are derieved from linux/pci_regs.h, however,
@@ -49,24 +50,107 @@
 #define PCI_CAPABILITY_LIST	0x34
 #define PCI_CAP_ID_VNDR		0x09
 
-
 #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))))
+static inline uint8_t
+virtio_read_reg_1(struct virtio_hw *hw, uint64_t reg_offset)
+{
+	uint8_t ret;
+	struct rte_pci_device *dev;
+
+	dev = hw->dev;
+	if (dev->kdrv == RTE_KDRV_VFIO)
+		ioport_inb(dev, reg_offset, &ret);
+	else
+		ret = inb(VIRTIO_PCI_REG_ADDR(hw, reg_offset));
+
+	return ret;
+}
+
+static inline uint16_t
+virtio_read_reg_2(struct virtio_hw *hw, uint64_t reg_offset)
+{
+	uint16_t ret;
+	struct rte_pci_device *dev;
+
+	dev = hw->dev;
+	if (dev->kdrv == RTE_KDRV_VFIO)
+		ioport_inw(dev, reg_offset, &ret);
+	else
+		ret = inw(VIRTIO_PCI_REG_ADDR(hw, reg_offset));
+
+	return ret;
+}
+
+static inline uint32_t
+virtio_read_reg_4(struct virtio_hw *hw, uint64_t reg_offset)
+{
+	uint32_t ret;
+	struct rte_pci_device *dev;
+
+	dev = hw->dev;
+	if (dev->kdrv == RTE_KDRV_VFIO)
+		ioport_inl(dev, reg_offset, &ret);
+	else
+		ret = inl(VIRTIO_PCI_REG_ADDR(hw, reg_offset));
+
+	return ret;
+}
+
+static inline void
+virtio_write_reg_1(struct virtio_hw *hw, uint64_t reg_offset, uint8_t value)
+{
+	struct rte_pci_device *dev;
+
+	dev = hw->dev;
+	if (dev->kdrv == RTE_KDRV_VFIO)
+		ioport_outb_p(dev, reg_offset, value);
+	else
+		outb_p((unsigned char)value,
+		       VIRTIO_PCI_REG_ADDR(hw, reg_offset));
+}
+
+static inline void
+virtio_write_reg_2(struct virtio_hw *hw, uint64_t reg_offset, uint16_t value)
+{
+	struct rte_pci_device *dev;
+
+	dev = hw->dev;
+	if (dev->kdrv == RTE_KDRV_VFIO)
+		ioport_outw_p(dev, reg_offset, value);
+	else
+		outw_p((unsigned short)value,
+		       VIRTIO_PCI_REG_ADDR(hw, reg_offset));
+}
+
+static inline void
+virtio_write_reg_4(struct virtio_hw *hw, uint64_t reg_offset, uint32_t value)
+{
+	struct rte_pci_device *dev;
+
+	dev = hw->dev;
+	if (dev->kdrv == RTE_KDRV_VFIO)
+		ioport_outl_p(dev, reg_offset, value);
+	else
+		outl_p((unsigned int)value,
+		       VIRTIO_PCI_REG_ADDR(hw, reg_offset));
+}
+
+#define VIRTIO_READ_REG_1(hw, reg)					\
+	virtio_read_reg_1((hw), (reg))
+#define VIRTIO_WRITE_REG_1(hw, reg, value)				\
+	virtio_write_reg_1((hw), (reg), (value))
 
-#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_2(hw, reg)					\
+	virtio_read_reg_2((hw), (reg))
+#define VIRTIO_WRITE_REG_2(hw, reg, value)				\
+	virtio_write_reg_2((hw), (reg), (value))
 
-#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_4(hw, reg)					\
+	virtio_read_reg_4((hw), (reg))
+#define VIRTIO_WRITE_REG_4(hw, reg, value)				\
+	virtio_write_reg_4((hw), (reg), (value))
 
 static void
 legacy_read_dev_config(struct virtio_hw *hw, uint64_t offset,
-- 
1.7.9.5

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

[Santosh Shukla]

Adding RTE_KDRV_VFIO_NOIOMMU mode in kernel driver. Also including
rte_vfio_is_noiommu() helper function. This function will parse
/sys/bus/pci/device/<bus_addr>/ and make sure that
- vfio noiommu mode set in kernel driver
- pci device attached to vfio-noiommu driver only

If both condition satisfies then set drv->kdrv = RTE_KDRV_VFIO_NOIOMMU

Also did similar changes in virtio_rd/wr, Changes applicable for virtio spec
0.95 only.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v4--> v5:
- Removed virtio_xx_init_by_vfio and added new driver mode.
- Now no need to parse vfio interface in virtio. As pci_eal module will take of
  vfio-noiommu driver parsing for virtio or any other future device willing to
  use vfio-noiommu driver.

 drivers/net/virtio/virtio_pci.c            |   12 ++---
 lib/librte_eal/common/include/rte_pci.h    |    1 +
 lib/librte_eal/linuxapp/eal/eal_pci.c      |   13 ++++--
 lib/librte_eal/linuxapp/eal/eal_pci_init.h |    1 +
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   69 ++++++++++++++++++++++++++++
 5 files changed, 87 insertions(+), 9 deletions(-)

diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index 0c29f1d..537c552 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -60,7 +60,7 @@ virtio_read_reg_1(struct virtio_hw *hw, uint64_t reg_offset)
 	struct rte_pci_device *dev;
 
 	dev = hw->dev;
-	if (dev->kdrv == RTE_KDRV_VFIO)
+	if (dev->kdrv == RTE_KDRV_VFIO_NOIOMMU)
 		ioport_inb(dev, reg_offset, &ret);
 	else
 		ret = inb(VIRTIO_PCI_REG_ADDR(hw, reg_offset));
@@ -75,7 +75,7 @@ virtio_read_reg_2(struct virtio_hw *hw, uint64_t reg_offset)
 	struct rte_pci_device *dev;
 
 	dev = hw->dev;
-	if (dev->kdrv == RTE_KDRV_VFIO)
+	if (dev->kdrv == RTE_KDRV_VFIO_NOIOMMU)
 		ioport_inw(dev, reg_offset, &ret);
 	else
 		ret = inw(VIRTIO_PCI_REG_ADDR(hw, reg_offset));
@@ -90,7 +90,7 @@ virtio_read_reg_4(struct virtio_hw *hw, uint64_t reg_offset)
 	struct rte_pci_device *dev;
 
 	dev = hw->dev;
-	if (dev->kdrv == RTE_KDRV_VFIO)
+	if (dev->kdrv == RTE_KDRV_VFIO_NOIOMMU)
 		ioport_inl(dev, reg_offset, &ret);
 	else
 		ret = inl(VIRTIO_PCI_REG_ADDR(hw, reg_offset));
@@ -104,7 +104,7 @@ virtio_write_reg_1(struct virtio_hw *hw, uint64_t reg_offset, uint8_t value)
 	struct rte_pci_device *dev;
 
 	dev = hw->dev;
-	if (dev->kdrv == RTE_KDRV_VFIO)
+	if (dev->kdrv == RTE_KDRV_VFIO_NOIOMMU)
 		ioport_outb_p(dev, reg_offset, value);
 	else
 		outb_p((unsigned char)value,
@@ -117,7 +117,7 @@ virtio_write_reg_2(struct virtio_hw *hw, uint64_t reg_offset, uint16_t value)
 	struct rte_pci_device *dev;
 
 	dev = hw->dev;
-	if (dev->kdrv == RTE_KDRV_VFIO)
+	if (dev->kdrv == RTE_KDRV_VFIO_NOIOMMU)
 		ioport_outw_p(dev, reg_offset, value);
 	else
 		outw_p((unsigned short)value,
@@ -130,7 +130,7 @@ virtio_write_reg_4(struct virtio_hw *hw, uint64_t reg_offset, uint32_t value)
 	struct rte_pci_device *dev;
 
 	dev = hw->dev;
-	if (dev->kdrv == RTE_KDRV_VFIO)
+	if (dev->kdrv == RTE_KDRV_VFIO_NOIOMMU)
 		ioport_outl_p(dev, reg_offset, value);
 	else
 		outl_p((unsigned int)value,
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 0c667ff..2dbc658 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -149,6 +149,7 @@ enum rte_kernel_driver {
 	RTE_KDRV_VFIO,
 	RTE_KDRV_UIO_GENERIC,
 	RTE_KDRV_NIC_UIO,
+	RTE_KDRV_VFIO_NOIOMMU,
 	RTE_KDRV_NONE,
 };
 
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index eb503f0..2936497 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -131,6 +131,7 @@ rte_eal_pci_map_device(struct rte_pci_device *dev)
 	/* try mapping the NIC resources using VFIO if it exists */
 	switch (dev->kdrv) {
 	case RTE_KDRV_VFIO:
+	case RTE_KDRV_VFIO_NOIOMMU:
 #ifdef VFIO_PRESENT
 		if (pci_vfio_is_enabled())
 			ret = pci_vfio_map_resource(dev);
@@ -158,6 +159,7 @@ rte_eal_pci_unmap_device(struct rte_pci_device *dev)
 	/* try unmapping the NIC resources using VFIO if it exists */
 	switch (dev->kdrv) {
 	case RTE_KDRV_VFIO:
+	case RTE_KDRV_VFIO_NOIOMMU:
 		RTE_LOG(ERR, EAL, "Hotplug doesn't support vfio yet\n");
 		break;
 	case RTE_KDRV_IGB_UIO:
@@ -353,9 +355,12 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 	}
 
 	if (!ret) {
-		if (!strcmp(driver, "vfio-pci"))
-			dev->kdrv = RTE_KDRV_VFIO;
-		else if (!strcmp(driver, "igb_uio"))
+		if (!strcmp(driver, "vfio-pci")) {
+			if (pci_vfio_is_noiommu(dev) == 0)
+				dev->kdrv = RTE_KDRV_VFIO_NOIOMMU;
+			else
+				dev->kdrv = RTE_KDRV_VFIO;
+		} else if (!strcmp(driver, "igb_uio"))
 			dev->kdrv = RTE_KDRV_IGB_UIO;
 		else if (!strcmp(driver, "uio_pci_generic"))
 			dev->kdrv = RTE_KDRV_UIO_GENERIC;
@@ -630,6 +635,7 @@ int rte_eal_pci_read_bar(const struct rte_pci_device *device,
 
 	switch (device->kdrv) {
 	case RTE_KDRV_VFIO:
+	case RTE_KDRV_VFIO_NOIOMMU:
 		return pci_vfio_read_bar(intr_handle, buf, len,
 					 offset, bar_idx);
 	default:
@@ -647,6 +653,7 @@ int rte_eal_pci_write_bar(const struct rte_pci_device *device,
 
 	switch (device->kdrv) {
 	case RTE_KDRV_VFIO:
+	case RTE_KDRV_VFIO_NOIOMMU:
 		return pci_vfio_write_bar(intr_handle, buf, len,
 					  offset, bar_idx);
 	default:
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_init.h b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
index 3bc592b..60b95d7 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
@@ -60,6 +60,7 @@ int pci_uio_write_config(const struct rte_intr_handle *intr_handle,
 
 int pci_vfio_enable(void);
 int pci_vfio_is_enabled(void);
+int pci_vfio_is_noiommu(struct rte_pci_device *pci_dev);
 int pci_vfio_mp_sync_setup(void);
 
 /* access config space */
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index df407ef..31d688b 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -973,4 +973,73 @@ pci_vfio_is_enabled(void)
 {
 	return vfio_cfg.vfio_enabled;
 }
+
+int
+pci_vfio_is_noiommu(struct rte_pci_device *pci_dev)
+{
+	FILE *fp;
+	struct rte_pci_addr *loc;
+	const char *path = "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode";
+	char filename[PATH_MAX] = {0};
+	char buf[PATH_MAX] = {0};
+
+	/*
+	 * 1. chk vfio-noiommu mode set in kernel driver
+	 * 2. verify pci device attached to vfio-noiommu driver
+	 * example:
+	 * cd /sys/bus/pci/drivers/vfio-pci/<virtio_dev_addr>/iommu_group
+	 * > cat name
+	 * > vfio-noiommu --> means virtio_dev attached to vfio-noiommu driver
+	 */
+
+	fp = fopen(path, "r");
+	if (fp == NULL) {
+		RTE_LOG(ERR, EAL, "can't open %s\n", path);
+		return -1;
+	}
+
+	if (fread(buf, sizeof(char), 1, fp) != 1) {
+		RTE_LOG(ERR, EAL, "can't read from file %s\n", path);
+		fclose(fp);
+		return -1;
+	}
+
+	if (strncmp(buf, "Y", 1) != 0) {
+		RTE_LOG(ERR, EAL, "[%s]: vfio: noiommu mode not set\n", path);
+		fclose(fp);
+		return -1;
+	}
+
+	fclose(fp);
+
+	/* 2. 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) {
+		RTE_LOG(ERR, EAL, "can't open %s\n", filename);
+		return -1;
+	}
+
+	if (fread(buf, sizeof(char), sizeof("vfio-noiommu"), fp) !=
+		  sizeof("vfio-noiommu")) {
+		RTE_LOG(ERR, EAL, "can't read from file %s\n", filename);
+		fclose(fp);
+		return -1;
+	}
+
+	if (strncmp(buf, "vfio-noiommu", strlen("vfio-noiommu")) != 0) {
+		RTE_LOG(ERR, EAL, "not a vfio-noiommu driver\n");
+		fclose(fp);
+		return -1;
+	}
+
+	fclose(fp);
+
+	return 0;
+}
 #endif
-- 
1.7.9.5

[PATCH v5 09/11] virtio_pci: do not parse if interface is vfio-noiommu

[Santosh Shukla]

If virtio interface attached to vfio-noiommu driver then
do not parse for virtio resource. Instead exit with return 0;

Note: Applicable for virtio spec 0.95.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v4-->v5:
- added _NOIOMMU drv check for lagecy virtio. No need for resource_init in vfio
  case. And resource_pasrsing/interface validation done by pci_eal module for
  pmd driver.
 
 drivers/net/virtio/virtio_pci.c |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index 537c552..520e540 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -514,7 +514,9 @@ virtio_resource_init_by_ioports(struct rte_pci_device *pci_dev)
 static int
 legacy_virtio_resource_init(struct rte_pci_device *pci_dev)
 {
-	if (virtio_resource_init_by_uio(pci_dev) == 0)
+	if (pci_dev->kdrv == RTE_KDRV_VFIO_NOIOMMU)
+		return 0;
+	else if (virtio_resource_init_by_uio(pci_dev) == 0)
 		return 0;
 	else
 		return virtio_resource_init_by_ioports(pci_dev);
-- 
1.7.9.5

[PATCH v5 10/11] 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)

Note: Applicable for virtio spec 0.95

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

diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index f550d22..b88f9ec 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -46,6 +46,7 @@
 #endif
 
 #include <rte_ethdev.h>
+#include "virtio_logs.h"
 
 struct virtqueue;
 
@@ -320,6 +321,51 @@ 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
+
 static inline int
 vtpci_with_feature(struct virtio_hw *hw, uint64_t bit)
 {
-- 
1.7.9.5

[PATCH v5 11/11] 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>
---
 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 60b95d7..cd42f77 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_init.h
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_init.h
@@ -113,6 +113,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 31d688b..70fcd19 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: [PATCH v5 08/11] eal: pci: introduce RTE_KDRV_VFIO_NOIOMMUi driver mode

[Burakov, Anatoly]

Hi Santosh,

> +int
> +pci_vfio_is_noiommu(struct rte_pci_device *pci_dev) {
> +	FILE *fp;
> +	struct rte_pci_addr *loc;
> +	const char *path =
> "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode";
> +	char filename[PATH_MAX] = {0};
> +	char buf[PATH_MAX] = {0};
> +
> +	/*
> +	 * 1. chk vfio-noiommu mode set in kernel driver
> +	 * 2. verify pci device attached to vfio-noiommu driver
> +	 * example:
> +	 * cd /sys/bus/pci/drivers/vfio-pci/<virtio_dev_addr>/iommu_group
> +	 * > cat name
> +	 * > vfio-noiommu --> means virtio_dev attached to vfio-noiommu
> driver
> +	 */
> +
> +	fp = fopen(path, "r");
> +	if (fp == NULL) {
> +		RTE_LOG(ERR, EAL, "can't open %s\n", path);
> +		return -1;
> +	}
> +
> +	if (fread(buf, sizeof(char), 1, fp) != 1) {
> +		RTE_LOG(ERR, EAL, "can't read from file %s\n", path);
> +		fclose(fp);
> +		return -1;
> +	}
> +
> +	if (strncmp(buf, "Y", 1) != 0) {
> +		RTE_LOG(ERR, EAL, "[%s]: vfio: noiommu mode not set\n",
> path);
> +		fclose(fp);
> +		return -1;
> +	}
> +
> +	fclose(fp);
> +
> +	/* 2. 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) {
> +		RTE_LOG(ERR, EAL, "can't open %s\n", filename);
> +		return -1;
> +	}
> +
> +	if (fread(buf, sizeof(char), sizeof("vfio-noiommu"), fp) !=
> +		  sizeof("vfio-noiommu")) {
> +		RTE_LOG(ERR, EAL, "can't read from file %s\n", filename);
> +		fclose(fp);
> +		return -1;
> +	}
> +
> +	if (strncmp(buf, "vfio-noiommu", strlen("vfio-noiommu")) != 0) {
> +		RTE_LOG(ERR, EAL, "not a vfio-noiommu driver\n");
> +		fclose(fp);
> +		return -1;
> +	}
> +
> +	fclose(fp);
> +
> +	return 0;
> +}

Since this is a public non-performance critical API, shouldn't we check if pci_dev is NULL? Otherwise the patch-set seems fine to me as far as VFIO parts are concerned.

Thanks,
Anatoly

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

[Santosh Shukla]

On Tue, Jan 19, 2016 at 7:48 PM, Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
> Hi Santosh,
>
>> +int
>> +pci_vfio_is_noiommu(struct rte_pci_device *pci_dev) {
>> +     FILE *fp;
>> +     struct rte_pci_addr *loc;
>> +     const char *path =
>> "/sys/module/vfio/parameters/enable_unsafe_noiommu_mode";
>> +     char filename[PATH_MAX] = {0};
>> +     char buf[PATH_MAX] = {0};
>> +
>> +     /*
>> +      * 1. chk vfio-noiommu mode set in kernel driver
>> +      * 2. verify pci device attached to vfio-noiommu driver
>> +      * example:
>> +      * cd /sys/bus/pci/drivers/vfio-pci/<virtio_dev_addr>/iommu_group
>> +      * > cat name
>> +      * > vfio-noiommu --> means virtio_dev attached to vfio-noiommu
>> driver
>> +      */
>> +
>> +     fp = fopen(path, "r");
>> +     if (fp == NULL) {
>> +             RTE_LOG(ERR, EAL, "can't open %s\n", path);
>> +             return -1;
>> +     }
>> +
>> +     if (fread(buf, sizeof(char), 1, fp) != 1) {
>> +             RTE_LOG(ERR, EAL, "can't read from file %s\n", path);
>> +             fclose(fp);
>> +             return -1;
>> +     }
>> +
>> +     if (strncmp(buf, "Y", 1) != 0) {
>> +             RTE_LOG(ERR, EAL, "[%s]: vfio: noiommu mode not set\n",
>> path);
>> +             fclose(fp);
>> +             return -1;
>> +     }
>> +
>> +     fclose(fp);
>> +
>> +     /* 2. 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) {
>> +             RTE_LOG(ERR, EAL, "can't open %s\n", filename);
>> +             return -1;
>> +     }
>> +
>> +     if (fread(buf, sizeof(char), sizeof("vfio-noiommu"), fp) !=
>> +               sizeof("vfio-noiommu")) {
>> +             RTE_LOG(ERR, EAL, "can't read from file %s\n", filename);
>> +             fclose(fp);
>> +             return -1;
>> +     }
>> +
>> +     if (strncmp(buf, "vfio-noiommu", strlen("vfio-noiommu")) != 0) {
>> +             RTE_LOG(ERR, EAL, "not a vfio-noiommu driver\n");
>> +             fclose(fp);
>> +             return -1;
>> +     }
>> +
>> +     fclose(fp);
>> +
>> +     return 0;
>> +}
>
> Since this is a public non-performance critical API, shouldn't we check if pci_dev is NULL? Otherwise the patch-set seems fine to me as far as VFIO parts are concerned.
>
pci_scan_one() uses this api for now and it populate pci_dev before
pci_vfio_is_noiommu() could use. So didn't though to add a check, But
you are right in case any other module want to use this api. Sending
patch now. Thanks.

> Thanks,
> Anatoly

Re: [PATCH v5 02/11] linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init()

[David Marchand]

Hello Santosh,

On Tue, Jan 19, 2016 at 12:46 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> iopl() syscall not supported in linux-arm/arm64 so always return 0 value.
>
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> Acked-by: Jan Viktorin <viktorin@rehivetech.com>
> Suggested-by: Stephen Hemminger <stephen@networkplumber.org>

I suppose when we will have more arches, this can be rewritten so that
iopl() check is only applied to x86 and all other arches get a 0
return.

How about such commit title ?
"eal/linux: never check iopl for arm"


Regards,
-- 
David Marchand

Re: [PATCH v5 05/11] eal: pci: vfio: add rd/wr func for pci bar space

[David Marchand]

Santosh,

On Tue, Jan 19, 2016 at 12:46 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> 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>
> ---
> v4--> v5:
> - added switch case in rd/wr pci_bar routine, as per Yuan comment
>
>  lib/librte_eal/common/include/rte_pci.h    |   38 ++++++++++++++++++++++++++++
>  lib/librte_eal/linuxapp/eal/eal_pci.c      |   34 +++++++++++++++++++++++++
>  lib/librte_eal/linuxapp/eal/eal_pci_init.h |    6 +++++
>  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   28 ++++++++++++++++++++
>  4 files changed, 106 insertions(+)

- What about BSD ?
Empty stub ?

- Those are new symbols, you must update rte_eal_version.map.


Regards,
-- 
David Marchand

Re: [PATCH v5 02/11] linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init()

[Santosh Shukla]

On Thu, Jan 21, 2016 at 3:11 PM, David Marchand <david.marchand@6wind.com>
wrote:

> Hello Santosh,
>
> On Tue, Jan 19, 2016 at 12:46 PM, Santosh Shukla <sshukla@mvista.com>
> wrote:
> > iopl() syscall not supported in linux-arm/arm64 so always return 0 value.
> >
> > Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> > Acked-by: Jan Viktorin <viktorin@rehivetech.com>
> > Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
>
> I suppose when we will have more arches, this can be rewritten so that
> iopl() check is only applied to x86 and all other arches get a 0
> return.
>
>
Thats correct. And which is why I am holding my other patchset which
actually move rte_eal_xx_iopl() stuff into arch specifics. I don't wanted
to mix two topic in this series. Waiting for this series to get merged then
abstract things like, iopl() and move "sys/io.h" in arch specifics and get
rid of few ifdef X86 clutter across dpdk code.


> How about such commit title ?
> "eal/linux: never check iopl for arm"
>

even better, sending v6 change for this patch now, Thanks!

>
>
> Regards,
> --
> David Marchand
>

Re: [PATCH v5 05/11] eal: pci: vfio: add rd/wr func for pci bar space

[Santosh Shukla]

On Thu, Jan 21, 2016 at 3:12 PM, David Marchand
<david.marchand@6wind.com> wrote:
> Santosh,
>
> On Tue, Jan 19, 2016 at 12:46 PM, Santosh Shukla <sshukla@mvista.com> wrote:
>> 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>
>> ---
>> v4--> v5:
>> - added switch case in rd/wr pci_bar routine, as per Yuan comment
>>
>>  lib/librte_eal/common/include/rte_pci.h    |   38 ++++++++++++++++++++++++++++
>>  lib/librte_eal/linuxapp/eal/eal_pci.c      |   34 +++++++++++++++++++++++++
>>  lib/librte_eal/linuxapp/eal/eal_pci_init.h |    6 +++++
>>  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   28 ++++++++++++++++++++
>>  4 files changed, 106 insertions(+)
>
> - What about BSD ?
> Empty stub ?
>
> - Those are new symbols, you must update rte_eal_version.map.
>
>

for both, agreed. Sending v6 for this patch now, Thanks!
> Regards,
> --
> David Marchand

Re: [PATCH v5 00/11] Add virtio support for arm/arm64

[Santosh Shukla]

Ping?

On Tue, Jan 19, 2016 at 5:16 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> Hi,
>
> Patch series uses vfio-noiommu-way to access virtio-net pci interface.
> Tested for arm64 thunderX/ x86_64 platform. Patch builds for
> x86/i386/arm/armv8/thunderX. Tested with testpmd application.
>
> Patchset rebased on yuan's under review virtio-1.0 v2 patchset and using
> vfio-noiommu patch. Refer my public branch [1].
>
> 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.
>
> v4 --> v5:
> - Introducing RTE_KDRV_VFIO_NOIOMMU driver mode
> - Incorporated v4 review comments, Pl. refer each patchset for review change.
>
> For older version(v4.. v1) patch history, refer [2].
>
> Thanks.
>
> [1]https://github.com/sshukla82/dpdk.git branch master-virtio-vfio-v5
> [2]http://comments.gmane.org/gmane.comp.networking.dpdk.devel/31402
>
>
> Anatoly Burakov (1):
>   vfio: Support for no-IOMMU mode
>
> Santosh Shukla (10):
>   virtio: Introduce config RTE_VIRTIO_INC_VECTOR
>   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
>   eal: pci: introduce RTE_KDRV_VFIO_NOIOMMUi driver mode
>   virtio_pci: do not parse if interface is vfio-noiommu
>   virtio: pci: add dummy func definition for in/outb for non-x86 arch
>
>  doc/guides/rel_notes/release_2_3.rst            |    3 -
>  drivers/net/virtio/virtio_ethdev.c              |  302 ++++++++-
>  drivers/net/virtio/virtio_ethdev.h              |    3 +-
>  drivers/net/virtio/virtio_pci.c                 |  793 +----------------------
>  drivers/net/virtio/virtio_pci.h                 |  120 +---
>  drivers/net/virtio/virtio_rxtx.c                |   21 +-
>  drivers/net/virtio/virtio_rxtx_simple.c         |   12 +-
>  drivers/net/virtio/virtqueue.h                  |    4 +-
>  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 -
>  15 files changed, 380 insertions(+), 949 deletions(-)
>
> --
> 1.7.9.5
>

Re: [PATCH v5 01/11] virtio: Introduce config RTE_VIRTIO_INC_VECTOR

[Santosh Shukla]

Ping?
On Jan 19, 2016 5:16 PM, "Santosh Shukla" <sshukla@mvista.com> 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.
>
> Disabling RTE_VIRTIO_INC_VECTOR config for :
>
> - i686 arch as i686 target config says:
>   config/defconfig_i686-native-linuxapp-gcc says "Vectorized PMD is not
>   supported on 32-bit".
>
> - armv7/v8 arch.
>
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> ---
> v4--> v5:
> - squashed v4's RTE_VIRTIO_INC_VECTOR patches into one patch.
> - Added ifdefs RTE_xx_xx_INC_VECTOR across _simple_rx_tx flag occurance in
> code.
>
>
>  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_rxtx.c           |   16 +++++++++++++++-
>  drivers/net/virtio/virtio_rxtx.h           |    2 ++
>  8 files changed, 27 insertions(+), 4 deletions(-)
>
> 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/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
> 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
> 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 41a1366..d8169d1 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -67,7 +67,9 @@
>  #define VIRTIO_SIMPLE_FLAGS ((uint32_t)ETH_TXQ_FLAGS_NOMULTSEGS | \
>         ETH_TXQ_FLAGS_NOOFFLOADS)
>
> +#ifdef RTE_VIRTIO_INC_VECTOR
>  static int use_simple_rxtx;
> +#endif
>
>  static void
>  vq_ring_free_chain(struct virtqueue *vq, uint16_t desc_idx)
> @@ -307,12 +309,13 @@ virtio_dev_vring_start(struct virtqueue *vq, int
> queue_type)
>                 nbufs = 0;
>                 error = ENOSPC;
>
> +#ifdef RTE_VIRTIO_INC_VECTOR
>                 if (use_simple_rxtx)
>                         for (i = 0; i < vq->vq_nentries; i++) {
>                                 vq->vq_ring.avail->ring[i] = i;
>                                 vq->vq_ring.desc[i].flags =
> VRING_DESC_F_WRITE;
>                         }
> -
> +#endif
>                 memset(&vq->fake_mbuf, 0, sizeof(vq->fake_mbuf));
>                 for (i = 0; i < RTE_PMD_VIRTIO_RX_MAX_BURST; i++)
>                         vq->sw_ring[vq->vq_nentries + i] = &vq->fake_mbuf;
> @@ -325,9 +328,11 @@ virtio_dev_vring_start(struct virtqueue *vq, int
> queue_type)
>                         /******************************************
>                         *         Enqueue allocated buffers        *
>                         *******************************************/
> +#ifdef RTE_VIRTIO_INC_VECTOR
>                         if (use_simple_rxtx)
>                                 error =
> virtqueue_enqueue_recv_refill_simple(vq, m);
>                         else
> +#endif
>                                 error = virtqueue_enqueue_recv_refill(vq,
> m);
>                         if (error) {
>                                 rte_pktmbuf_free(m);
> @@ -340,6 +345,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int
> queue_type)
>
>                 PMD_INIT_LOG(DEBUG, "Allocated %d bufs", nbufs);
>         } else if (queue_type == VTNET_TQ) {
> +#ifdef RTE_VIRTIO_INC_VECTOR
>                 if (use_simple_rxtx) {
>                         int mid_idx  = vq->vq_nentries >> 1;
>                         for (i = 0; i < mid_idx; i++) {
> @@ -357,6 +363,7 @@ virtio_dev_vring_start(struct virtqueue *vq, int
> queue_type)
>                         for (i = mid_idx; i < vq->vq_nentries; i++)
>                                 vq->vq_ring.avail->ring[i] = i;
>                 }
> +#endif
>         }
>  }
>
> @@ -423,7 +430,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;
>  }
> @@ -449,7 +458,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;
> @@ -462,6 +474,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)) {
> @@ -470,6 +483,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);
> diff --git a/drivers/net/virtio/virtio_rxtx.h
> b/drivers/net/virtio/virtio_rxtx.h
> index 831e492..9be1378 100644
> --- a/drivers/net/virtio/virtio_rxtx.h
> +++ b/drivers/net/virtio/virtio_rxtx.h
> @@ -33,7 +33,9 @@
>
>  #define RTE_PMD_VIRTIO_RX_MAX_BURST 64
>
> +#ifdef RTE_VIRTIO_INC_VECTOR
>  int virtio_rxq_vec_setup(struct virtqueue *rxq);
>
>  int virtqueue_enqueue_recv_refill_simple(struct virtqueue *vq,
>         struct rte_mbuf *m);
> +#endif
> --
> 1.7.9.5
>
>

Re: [PATCH v5 03/11] linuxapp/vfio: ignore mapping for ioport region

[Santosh Shukla]

Ping.
On Jan 19, 2016 5:16 PM, "Santosh Shukla" <sshukla@mvista.com> wrote:

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

Re: [PATCH v5 04/11] virtio_pci.h: build fix for sys/io.h for non-x86 arch

[Santosh Shukla]

Ping
On Jan 19, 2016 5:16 PM, "Santosh Shukla" <sshukla@mvista.com> wrote:

> 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 99572a0..f550d22 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
>
>

Re: [PATCH v5 01/11] virtio: Introduce config RTE_VIRTIO_INC_VECTOR

[Yuanhan Liu]

On Wed, Jan 27, 2016 at 07:53:21AM +0530, Santosh Shukla wrote:
> Ping?

I was on vacation late last week. And I was quite busy till now after
the vacation. So, sorry that I still don't have time to do more detailed
reviews in 1 or 2 days. Hopefully I can make it by this Friday.

BTW, I had a very glimpse of this patchset, overall, it looks much
better now, except the EAL changes (I'm not the maintainer) and the
virtio io port read/write stuff: Tetsuay suggested to add another
access wraps, but I have few concerns about that. Anyway, I don't
have time for deeper thoughts, and I will re-think it later.

	--yliu

Re: [PATCH v5 10/11] virtio: pci: add dummy func definition for in/outb for non-x86 arch

[Santosh Shukla]

Ping?

On Tue, Jan 19, 2016 at 5:16 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> 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)
>
> Note: Applicable for virtio spec 0.95
>
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> ---
>  drivers/net/virtio/virtio_pci.h |   46 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
>
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index f550d22..b88f9ec 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -46,6 +46,7 @@
>  #endif
>
>  #include <rte_ethdev.h>
> +#include "virtio_logs.h"
>
>  struct virtqueue;
>
> @@ -320,6 +321,51 @@ 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
> +
>  static inline int
>  vtpci_with_feature(struct virtio_hw *hw, uint64_t bit)
>  {
> --
> 1.7.9.5
>

Re: [PATCH v5 01/11] virtio: Introduce config RTE_VIRTIO_INC_VECTOR

[Santosh Shukla]

Hi Yuan,

On Wed, Jan 27, 2016 at 8:03 AM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Wed, Jan 27, 2016 at 07:53:21AM +0530, Santosh Shukla wrote:
>> Ping?
>
> I was on vacation late last week. And I was quite busy till now after
> the vacation. So, sorry that I still don't have time to do more detailed
> reviews in 1 or 2 days. Hopefully I can make it by this Friday.
>
> BTW, I had a very glimpse of this patchset, overall, it looks much
> better now, except the EAL changes (I'm not the maintainer) and the
> virtio io port read/write stuff: Tetsuay suggested to add another
> access wraps, but I have few concerns about that. Anyway, I don't
> have time for deeper thoughts, and I will re-think it later.
>
>         --yliu

did you got the chance for reviewing virtio specific patches? Thanks.

Re: [PATCH v5 01/11] virtio: Introduce config RTE_VIRTIO_INC_VECTOR

[Yuanhan Liu]

On Fri, Jan 29, 2016 at 10:02:26AM +0530, Santosh Shukla wrote:
> Hi Yuan,

It's Yuanhan, but not Yuan :)

> On Wed, Jan 27, 2016 at 8:03 AM, Yuanhan Liu
> <yuanhan.liu@linux.intel.com> wrote:
> > On Wed, Jan 27, 2016 at 07:53:21AM +0530, Santosh Shukla wrote:
> >> Ping?
> >
> > I was on vacation late last week. And I was quite busy till now after
> > the vacation. So, sorry that I still don't have time to do more detailed
> > reviews in 1 or 2 days. Hopefully I can make it by this Friday.
> >
> > BTW, I had a very glimpse of this patchset, overall, it looks much
> > better now, except the EAL changes (I'm not the maintainer) and the
> > virtio io port read/write stuff: Tetsuay suggested to add another
> > access wraps, but I have few concerns about that. Anyway, I don't
> > have time for deeper thoughts, and I will re-think it later.
> >
> >         --yliu
> 
> did you got the chance for reviewing virtio specific patches? Thanks.

Sorry for the long delay, it's very likely I will check your patches
this noon.

	--yliu

Re: [PATCH v5 01/11] virtio: Introduce config RTE_VIRTIO_INC_VECTOR

[Santosh Shukla]

On Fri, Jan 29, 2016 at 10:12 AM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Fri, Jan 29, 2016 at 10:02:26AM +0530, Santosh Shukla wrote:
>> Hi Yuan,
>
> It's Yuanhan, but not Yuan :)
>

Sorry for that,

>> On Wed, Jan 27, 2016 at 8:03 AM, Yuanhan Liu
>> <yuanhan.liu@linux.intel.com> wrote:
>> > On Wed, Jan 27, 2016 at 07:53:21AM +0530, Santosh Shukla wrote:
>> >> Ping?
>> >
>> > I was on vacation late last week. And I was quite busy till now after
>> > the vacation. So, sorry that I still don't have time to do more detailed
>> > reviews in 1 or 2 days. Hopefully I can make it by this Friday.
>> >
>> > BTW, I had a very glimpse of this patchset, overall, it looks much
>> > better now, except the EAL changes (I'm not the maintainer) and the
>> > virtio io port read/write stuff: Tetsuay suggested to add another
>> > access wraps, but I have few concerns about that. Anyway, I don't
>> > have time for deeper thoughts, and I will re-think it later.
>> >
>> >         --yliu
>>
>> did you got the chance for reviewing virtio specific patches? Thanks.
>
> Sorry for the long delay, it's very likely I will check your patches
> this noon.
>

Thanks
>         --yliu

Re: [PATCH v5 10/11] virtio: pci: add dummy func definition for in/outb for non-x86 arch

[Yuanhan Liu]

On Tue, Jan 19, 2016 at 05:16:11PM +0530, Santosh Shukla wrote:
> 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)
> 
> Note: Applicable for virtio spec 0.95
> 
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> ---
>  drivers/net/virtio/virtio_pci.h |   46 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
> index f550d22..b88f9ec 100644
> --- a/drivers/net/virtio/virtio_pci.h
> +++ b/drivers/net/virtio/virtio_pci.h
> @@ -46,6 +46,7 @@
>  #endif
>  
>  #include <rte_ethdev.h>
> +#include "virtio_logs.h"
>  
>  struct virtqueue;
>  
> @@ -320,6 +321,51 @@ 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;
> +}

The whole port read/write stuff is getting messy here: we not only have
to care the FreeBSD and Linux difference, but also the x86 and non-x86
difference. And you just added yet another vfio layer.

First of all, they are not belong here (virtio_pci.h). A new place like
virtio_io.h sounds much better to me. Therefore, I'd suggest you to
put all those stuff there, like the one I have just cooked up:


    #ifndef __VIRTIO_IO_H__
    
    #if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
    
    #ifdef __FreeBSD__
      #include <sys/types.h>
      #include <machine/cpufunc.h>
    
      #define outb_p outb
      #define outw_p outw
      #define outl_p outl
    #else
      #include <sys/io.h>
    #endif
    
    #else /* ! X86 */
    
    static inline uint8_t
    inb(unsigned long addr __rte_unused)
    {
    	PMD_INIT_LOG(ERR, "inb() not supported for this RTE_ARCH\n");
    	return 0;
    }
    
    
    ....
    #endif /* X86 */


    /* And put the vfio io port read/write here */
    
    #endif /* __VIRTIO_IO_H__ */

Note that you may need squash patch 4 (build fix for sys/io.h ...)
here. They both resolve one thing: to make it build on non-x86 platforms.

Another minor note is that while you are trying this way, I'd suggest
you to make a patch to introduce virtio_io.h, and then make another
patch to fix build errors on non-x86 platforms.

Another generic comment about this patchset is that it VERY okay to
include several components change in one set, but putting them in
order helps review a lot.

Say, this patch set has dependence on VFIO stuff, therefore, it'd be
much better __IF__ you can put all VFIO related patches first, and
then virtio related patches follows, but not in an interleaved way
you did. If, for somereason, you can't do that, you should at least
try to minimise the chance of interleave.

	--yliu

Re: [PATCH v5 06/11] virtio: vfio: add api support to rd/wr ioport bar

[Yuanhan Liu]

On Tue, Jan 19, 2016 at 05:16:07PM +0530, Santosh Shukla wrote:
> For vfio case - Use pread/pwrite api to access virtio
> ioport space.
> 
> Applicable for virtio 0.95 spec.
> 
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> Signed-off-by: Rizwan Ansari <ransari@mvista.com>
> Signed-off-by: Rakesh Krishnamurthy <rakeshk@mvista.com>
> ---
> v4 --> v5:
> - Removed unnecessary type casting.
> 
>  drivers/net/virtio/virtio_vfio_rw.h |  104 +++++++++++++++++++++++++++++++++++
>  1 file changed, 104 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..a9ab04d
> --- /dev/null
> +++ b/drivers/net/virtio/virtio_vfio_rw.h

Maybe virtio_vfio.h is enough, or if you have to put a sufix there, _io
(instead of _rw) is a better option to me.

> +
> +/* vfio rd/rw virtio apis */
> +static inline void
> +ioport_inb(const struct rte_pci_device *pci_dev, uint8_t reg, uint8_t *val)

vfio_inb is a better name; ioport_inb is too generic.

	--yliu

Re: [PATCH v5 06/11] virtio: vfio: add api support to rd/wr ioport bar

[Santosh Shukla]

On Fri, Jan 29, 2016 at 12:37 PM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Tue, Jan 19, 2016 at 05:16:07PM +0530, Santosh Shukla wrote:
>> For vfio case - Use pread/pwrite api to access virtio
>> ioport space.
>>
>> Applicable for virtio 0.95 spec.
>>
>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>> Signed-off-by: Rizwan Ansari <ransari@mvista.com>
>> Signed-off-by: Rakesh Krishnamurthy <rakeshk@mvista.com>
>> ---
>> v4 --> v5:
>> - Removed unnecessary type casting.
>>
>>  drivers/net/virtio/virtio_vfio_rw.h |  104 +++++++++++++++++++++++++++++++++++
>>  1 file changed, 104 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..a9ab04d
>> --- /dev/null
>> +++ b/drivers/net/virtio/virtio_vfio_rw.h
>
> Maybe virtio_vfio.h is enough, or if you have to put a sufix there, _io
> (instead of _rw) is a better option to me.
>

Ok.

>> +
>> +/* vfio rd/rw virtio apis */
>> +static inline void
>> +ioport_inb(const struct rte_pci_device *pci_dev, uint8_t reg, uint8_t *val)
>
> vfio_inb is a better name; ioport_inb is too generic.
>

Ok.
>         --yliu

Re: [PATCH v5 09/11] virtio_pci: do not parse if interface is vfio-noiommu

[Yuanhan Liu]

Two minor nits.

Firstly about your title, you should be consistent: sometimes you
use virtio_pci, and sometimes you use virtio_pic.h. And for virtio
pmd driver, "virtio: " prefix is pretty enough, no need another
extra "vfio: " or "pci: " prefix.

And the same to your EAL changes. EAL is a bigger, having more
components, thus sometimes 2 prefixs are used. And if you are
not sure how to add prefix, dig the git history to get the answer.

On Tue, Jan 19, 2016 at 05:16:10PM +0530, Santosh Shukla wrote:
> If virtio interface attached to vfio-noiommu driver then
> do not parse for virtio resource. Instead exit with return 0;
> 
> Note: Applicable for virtio spec 0.95.

And this is not necessary: io port stuff is for virtio 0.95 only.
virtio 1.0 won't use that, at all.

	--yliu

Re: [PATCH v5 09/11] virtio_pci: do not parse if interface is vfio-noiommu

[Santosh Shukla]

On Fri, Jan 29, 2016 at 12:47 PM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> Two minor nits.
>
> Firstly about your title, you should be consistent: sometimes you
> use virtio_pci, and sometimes you use virtio_pic.h. And for virtio
> pmd driver, "virtio: " prefix is pretty enough, no need another
> extra "vfio: " or "pci: " prefix.
>
> And the same to your EAL changes. EAL is a bigger, having more
> components, thus sometimes 2 prefixs are used. And if you are
> not sure how to add prefix, dig the git history to get the answer.
>
> On Tue, Jan 19, 2016 at 05:16:10PM +0530, Santosh Shukla wrote:
>> If virtio interface attached to vfio-noiommu driver then
>> do not parse for virtio resource. Instead exit with return 0;
>>
>> Note: Applicable for virtio spec 0.95.
>
> And this is not necessary: io port stuff is for virtio 0.95 only.
> virtio 1.0 won't use that, at all.
>

Okay,

I removed [08/11] patch from v5 series and modified this patch
accordingly [1]. So, ignore this patch and pl. review provided link.

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

>         --yliu

Re: [PATCH v5 10/11] virtio: pci: add dummy func definition for in/outb for non-x86 arch

[Santosh Shukla]

On Fri, Jan 29, 2016 at 12:31 PM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Tue, Jan 19, 2016 at 05:16:11PM +0530, Santosh Shukla wrote:
>> 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)
>>
>> Note: Applicable for virtio spec 0.95
>>
>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>> ---
>>  drivers/net/virtio/virtio_pci.h |   46 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 46 insertions(+)
>>
>> diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
>> index f550d22..b88f9ec 100644
>> --- a/drivers/net/virtio/virtio_pci.h
>> +++ b/drivers/net/virtio/virtio_pci.h
>> @@ -46,6 +46,7 @@
>>  #endif
>>
>>  #include <rte_ethdev.h>
>> +#include "virtio_logs.h"
>>
>>  struct virtqueue;
>>
>> @@ -320,6 +321,51 @@ 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;
>> +}
>
> The whole port read/write stuff is getting messy here: we not only have
> to care the FreeBSD and Linux difference, but also the x86 and non-x86
> difference. And you just added yet another vfio layer.
>
> First of all, they are not belong here (virtio_pci.h). A new place like
> virtio_io.h sounds much better to me. Therefore, I'd suggest you to
> put all those stuff there, like the one I have just cooked up:
>
>
>     #ifndef __VIRTIO_IO_H__
>
>     #if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
>
>     #ifdef __FreeBSD__
>       #include <sys/types.h>
>       #include <machine/cpufunc.h>
>
>       #define outb_p outb
>       #define outw_p outw
>       #define outl_p outl
>     #else
>       #include <sys/io.h>
>     #endif
>
>     #else /* ! X86 */
>
>     static inline uint8_t
>     inb(unsigned long addr __rte_unused)
>     {
>         PMD_INIT_LOG(ERR, "inb() not supported for this RTE_ARCH\n");
>         return 0;
>     }
>
>
>     ....
>     #endif /* X86 */
>
>
>     /* And put the vfio io port read/write here */
>
>     #endif /* __VIRTIO_IO_H__ */
>
> Note that you may need squash patch 4 (build fix for sys/io.h ...)
> here. They both resolve one thing: to make it build on non-x86 platforms.
>
> Another minor note is that while you are trying this way, I'd suggest
> you to make a patch to introduce virtio_io.h, and then make another
> patch to fix build errors on non-x86 platforms.
>

Ok.

> Another generic comment about this patchset is that it VERY okay to
> include several components change in one set, but putting them in
> order helps review a lot.
>
> Say, this patch set has dependence on VFIO stuff, therefore, it'd be
> much better __IF__ you can put all VFIO related patches first, and
> then virtio related patches follows, but not in an interleaved way
> you did. If, for somereason, you can't do that, you should at least
> try to minimise the chance of interleave.
>

I agree that, but this patch series dependent on other patches
including virtio 1.0 and then vfio-noiommu, its was difficult for me
to keep topic-wise sanity in patch series.

V6 will take care patch ordering. Thanks
>         --yliu

Re: [PATCH v5 09/11] virtio_pci: do not parse if interface is vfio-noiommu

[Yuanhan Liu]

On Fri, Jan 29, 2016 at 12:52:53PM +0530, Santosh Shukla wrote:
> On Fri, Jan 29, 2016 at 12:47 PM, Yuanhan Liu
> <yuanhan.liu@linux.intel.com> wrote:
> > Two minor nits.
> >
> > Firstly about your title, you should be consistent: sometimes you
> > use virtio_pci, and sometimes you use virtio_pic.h. And for virtio
> > pmd driver, "virtio: " prefix is pretty enough, no need another
> > extra "vfio: " or "pci: " prefix.
> >
> > And the same to your EAL changes. EAL is a bigger, having more
> > components, thus sometimes 2 prefixs are used. And if you are
> > not sure how to add prefix, dig the git history to get the answer.
> >
> > On Tue, Jan 19, 2016 at 05:16:10PM +0530, Santosh Shukla wrote:
> >> If virtio interface attached to vfio-noiommu driver then
> >> do not parse for virtio resource. Instead exit with return 0;
> >>
> >> Note: Applicable for virtio spec 0.95.
> >
> > And this is not necessary: io port stuff is for virtio 0.95 only.
> > virtio 1.0 won't use that, at all.
> >
> 
> Okay,
> 
> I removed [08/11] patch from v5 series and modified this patch
> accordingly [1]. So, ignore this patch and pl. review provided link.
> 
> [1] http://dpdk.org/dev/patchwork/patch/10143/

This patch actually looks good to me, despites the two minor nits.

Another note is that while sending just one single update patch,
it'd be better if you could link it to the old patch, to make it
in the same email thread, otherwise, it's difficult for me to
notice that single-alone patch: it's easily get lost.

There is another option for that: the git scissors option; you could
check the git format-patch man page for more detailed info (by searching
"scissors" keyword). I'm just not quite sure Thomas like it or not.

	--yliu

Re: [PATCH v5 10/11] virtio: pci: add dummy func definition for in/outb for non-x86 arch

[Yuanhan Liu]

On Fri, Jan 29, 2016 at 01:01:02PM +0530, Santosh Shukla wrote:
> > Another generic comment about this patchset is that it VERY okay to
> > include several components change in one set, but putting them in
> > order helps review a lot.
> >
> > Say, this patch set has dependence on VFIO stuff, therefore, it'd be
> > much better __IF__ you can put all VFIO related patches first, and
> > then virtio related patches follows, but not in an interleaved way
> > you did. If, for somereason, you can't do that, you should at least
> > try to minimise the chance of interleave.
> >
> 
> I agree that, but this patch series dependent on other patches
> including virtio 1.0 and then vfio-noiommu, its was difficult for me
> to keep topic-wise sanity in patch series.

That would not be an issue to me: just apply the dependence patches
first, and build your patches on top of that. You just need mention
the dependence info in your cover-letter.

	--yliu
> 
> V6 will take care patch ordering. Thanks

Thanks!

	--yliu

Re: [PATCH v5 09/11] virtio_pci: do not parse if interface is vfio-noiommu

[Thomas Monjalon]

2016-01-29 15:34, Yuanhan Liu:
> There is another option for that: the git scissors option; you could
> check the git format-patch man page for more detailed info (by searching
> "scissors" keyword). I'm just not quite sure Thomas like it or not.

For simple discussions, patch after scissors may be a good option.

In this series, there are too many patches sent alone and it would
be good to send the whole series  to make things flatter (while keeping
the --in-reply-to of course).

Re: [PATCH v5 09/11] virtio_pci: do not parse if interface is vfio-noiommu

[Yuanhan Liu]

On Fri, Jan 29, 2016 at 10:02:26AM +0100, Thomas Monjalon wrote:
> 2016-01-29 15:34, Yuanhan Liu:
> > There is another option for that: the git scissors option; you could
> > check the git format-patch man page for more detailed info (by searching
> > "scissors" keyword). I'm just not quite sure Thomas like it or not.
> 
> For simple discussions, patch after scissors may be a good option.
> 
> In this series, there are too many patches sent alone and it would
> be good to send the whole series  to make things flatter (while keeping
> the --in-reply-to of course).

Agreed.

	--yliu

Re: [PATCH v5 09/11] virtio_pci: do not parse if interface is vfio-noiommu

[Santosh Shukla]

On Fri, Jan 29, 2016 at 2:32 PM, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> 2016-01-29 15:34, Yuanhan Liu:
>> There is another option for that: the git scissors option; you could
>> check the git format-patch man page for more detailed info (by searching
>> "scissors" keyword). I'm just not quite sure Thomas like it or not.
>

I agree and my mistake.. sorry for confusion,

> For simple discussions, patch after scissors may be a good option.
>
> In this series, there are too many patches sent alone and it would
> be good to send the whole series  to make things flatter (while keeping
> the --in-reply-to of course).

Sending whole series taging patch version v6 version shortly, Thanks.