[dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[Santosh Shukla]

Introducing below api for pci bar region rd/wr.
Api's are:
- rte_eal_pci_read_bar
- rte_eal_pci_write_bar

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v5-->v6:
- update api infor in rte_eal_version.map file
  suggested by david manchand.

 lib/librte_eal/bsdapp/eal/eal_pci.c             |   19 ++++++++++++
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |    3 ++
 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 +++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |    3 ++
 7 files changed, 131 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 95c32c1..2e535ea 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -479,6 +479,25 @@ int rte_eal_pci_write_config(const struct rte_pci_device *dev,
 	return -1;
 }
 
+int rte_eal_pci_read_bar(const struct rte_pci_device *device __rte_unused,
+			 void *buf __rte_unused, size_t len __rte_unused,
+			 off_t offset __rte_unused,
+			 int bar_idx __rte_unused)
+
+{
+	/* NA */
+	return 1;
+}
+
+int rte_eal_pci_write_bar(const struct rte_pci_device *device __rte_unused,
+			  const void *buf __rte_unused, size_t len __rte_unused,
+			  off_t offset __rte_unused,
+			  int bar_idx __rte_unused)
+{
+	/* NA */
+	return 1;
+}
+
 /* Init the PCI EAL subsystem */
 int
 rte_eal_pci_init(void)
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 1b28170..7c7dcf0 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -141,4 +141,7 @@ DPDK_2.3 {
 
 	rte_eal_pci_map_device;
 	rte_eal_pci_unmap_device;
+	rte_eal_pci_read_bar;
+	rte_eal_pci_write_bar;
+
 } DPDK_2.2;
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 a6c7e16..34c4558 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -156,6 +156,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,
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index b9937c4..371b6c1 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -144,4 +144,7 @@ DPDK_2.3 {
 
 	rte_eal_pci_map_device;
 	rte_eal_pci_unmap_device;
+	rte_eal_pci_read_bar;
+	rte_eal_pci_write_bar;
+
 } DPDK_2.2;
-- 
1.7.9.5

[dpdk-dev] [PATCH v6 2/8] 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 34c4558..b704ffd 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -687,6 +687,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;
@@ -881,6 +882,25 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 			return -1;
 		}
 
+		/* chk for io port region */
+		ret = pread64(vfio_dev_fd, &ioport_bar, sizeof(ioport_bar),
+			      VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX)
+			      + PCI_BASE_ADDRESS_0 + i*4);
+
+		if (ret != sizeof(ioport_bar)) {
+			RTE_LOG(ERR, EAL,
+				"Cannot read command (%x) from config space!\n",
+				PCI_BASE_ADDRESS_0 + i*4);
+			return -1;
+		}
+
+		if (ioport_bar & PCI_BASE_ADDRESS_SPACE_IO) {
+			RTE_LOG(INFO, EAL,
+				"Ignore mapping IO port bar(%d) addr: %x\n",
+				 i, ioport_bar);
+			continue;
+		}
+
 		/* skip non-mmapable BARs */
 		if ((reg.flags & VFIO_REGION_INFO_FLAG_MMAP) == 0)
 			continue;
-- 
1.7.9.5

[dpdk-dev] [PATCH v6 3/8] eal/linux: never check iopl for arm

[Santosh Shukla]

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

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
Suggested-by: Stephen Hemminger <stephen@networkplumber.org>
Acked-by: Jan Viktorin <viktorin@rehivetech.com>
Acked-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal.c |    2 ++
 1 file changed, 2 insertions(+)

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

[dpdk-dev] [PATCH v6 4/8] 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

[dpdk-dev] [PATCH v6 5/8] virtio: move io header and api from virtio_pci.h

[Santosh Shukla]

Moving io api and header file i.e. sys/io.h to separate file virtio_io.h

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v5-->v6:
- included new file virtio_io.h, has in/out api and sys/io.h.

 drivers/net/virtio/virtio_io.h  |  114 +++++++++++++++++++++++++++++++++++++++
 drivers/net/virtio/virtio_pci.c |    1 +
 drivers/net/virtio/virtio_pci.h |   30 -----------
 3 files changed, 115 insertions(+), 30 deletions(-)
 create mode 100644 drivers/net/virtio/virtio_io.h

diff --git a/drivers/net/virtio/virtio_io.h b/drivers/net/virtio/virtio_io.h
new file mode 100644
index 0000000..bfa1341
--- /dev/null
+++ b/drivers/net/virtio/virtio_io.h
@@ -0,0 +1,114 @@
+/*
+ *   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_IO_H_
+#define _VIRTIO_IO_H_
+
+#include <rte_common.h>
+#include "virtio_logs.h"
+
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
+
+#ifdef __FreeBSD__
+#include <sys/types.h>
+#include <machine/cpufunc.h>
+
+static inline void
+outb_p(unsigned char data, unsigned int port)
+{
+
+	outb(port, (u_char)data);
+}
+
+static inline void
+outw_p(unsigned short data, unsigned int port)
+{
+	outw(port, (u_short)data);
+}
+
+static inline void
+outl_p(unsigned int data, unsigned int port)
+{
+	outl(port, (u_int)data);
+}
+
+#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;
+}
+
+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 /* X86 */
+#endif /* _VIRTIO_IO_H_ */
diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index a9f179f..064f234 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -37,6 +37,7 @@
  #include <fcntl.h>
 #endif
 
+#include "virtio_io.h"
 #include "virtio_pci.h"
 #include "virtio_logs.h"
 #include "virtqueue.h"
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 99572a0..2fe5835 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -35,14 +35,6 @@
 #define _VIRTIO_PCI_H_
 
 #include <stdint.h>
-
-#ifdef __FreeBSD__
-#include <sys/types.h>
-#include <machine/cpufunc.h>
-#else
-#include <sys/io.h>
-#endif
-
 #include <rte_ethdev.h>
 
 struct virtqueue;
@@ -296,28 +288,6 @@ struct virtio_net_config {
 /* The alignment to use between consumer and producer parts of vring. */
 #define VIRTIO_PCI_VRING_ALIGN 4096
 
-#ifdef __FreeBSD__
-
-static inline void
-outb_p(unsigned char data, unsigned int port)
-{
-
-	outb(port, (u_char)data);
-}
-
-static inline void
-outw_p(unsigned short data, unsigned int port)
-{
-	outw(port, (u_short)data);
-}
-
-static inline void
-outl_p(unsigned int data, unsigned int port)
-{
-	outl(port, (u_int)data);
-}
-#endif
-
 static inline int
 vtpci_with_feature(struct virtio_hw *hw, uint64_t bit)
 {
-- 
1.7.9.5

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

[Santosh Shukla]

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

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
Signed-off-by: Rizwan Ansari <ransari@mvista.com>
Signed-off-by: Rakesh Krishnamurthy <rakeshk@mvista.com>
---
v5-->v6:
- renamed inport_in/out to vfio_in/out
- Renamed file from virtio_vfio_rw.h to virtio_vfio_io.h

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

diff --git a/drivers/net/virtio/virtio_vfio_io.h b/drivers/net/virtio/virtio_vfio_io.h
new file mode 100644
index 0000000..218d4ed
--- /dev/null
+++ b/drivers/net/virtio/virtio_vfio_io.h
@@ -0,0 +1,104 @@
+/*
+ *   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_IO_H_
+#define _VIRTIO_VFIO_IO_H_
+
+#include "virtio_logs.h"
+#if defined(RTE_EAL_VFIO) && defined(RTE_LIBRTE_EAL_LINUXAPP)
+
+#include <stdint.h>
+#include <rte_pci.h>
+
+/* vfio rd/rw virtio apis */
+static inline void
+vfio_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
+vfio_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
+vfio_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
+vfio_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
+vfio_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
+vfio_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

[dpdk-dev] [PATCH v6 7/8] virtio: extend pci rw api for vfio

[Santosh Shukla]

So far virtio handle rw access for uio / ioport interface,
This patch to extend the support for vfio.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
 drivers/net/virtio/virtio_io.h  |    2 +-
 drivers/net/virtio/virtio_pci.c |  110 ++++++++++++++++++++++++++++++++++-----
 2 files changed, 98 insertions(+), 14 deletions(-)

diff --git a/drivers/net/virtio/virtio_io.h b/drivers/net/virtio/virtio_io.h
index bfa1341..ce59e3f 100644
--- a/drivers/net/virtio/virtio_io.h
+++ b/drivers/net/virtio/virtio_io.h
@@ -35,7 +35,7 @@
 #define _VIRTIO_IO_H_
 
 #include <rte_common.h>
-#include "virtio_logs.h"
+#include "virtio_vfio_io.h"
 
 #if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
 
diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index 064f234..71d4a07 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -37,10 +37,10 @@
  #include <fcntl.h>
 #endif
 
-#include "virtio_io.h"
 #include "virtio_pci.h"
 #include "virtio_logs.h"
 #include "virtqueue.h"
+#include "virtio_io.h"
 
 /*
  * Following macros are derieved from linux/pci_regs.h, however,
@@ -54,20 +54,104 @@
 #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)
+		vfio_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)
+		vfio_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)
+		vfio_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)
+		vfio_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)
+		vfio_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)
+		vfio_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

[dpdk-dev] [PATCH v6 8/8] virtio: do not parse if interface is vfio

[Santosh Shukla]

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

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v5-->v6:
- Removed _noimmu and using deafult rte_kdrv_vfio for drv check.

 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 71d4a07..d73a3ad 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -515,7 +515,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)
+		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

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

[Yuanhan Liu]

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

Won't it cause build failure if above "#if ..." is false, as
virtio_read/write_reg_x() reference them unconditionally?

BTW, why above check is needed? We have rte_eal_pci_read/write_bar()
implementation with both VFIO and BSD, don't we?


> +#endif /* _VIRTIO_VFIO_RW_H_ */
                         ^^^^
You forgot to do rename here.


BTW, I didn't follow the noIOMMU discussion; how did it end? Do we still
need that? Is this patch a full story to enable virtio on ARM?

	--yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[Yuanhan Liu]

On Fri, Jan 29, 2016 at 11:51:50PM +0530, Santosh Shukla wrote:
> Introducing below api for pci bar region rd/wr.
> Api's are:
> - rte_eal_pci_read_bar
> - rte_eal_pci_write_bar
> 
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> ---
> v5-->v6:
> - update api infor in rte_eal_version.map file
>   suggested by david manchand.
> 
>  lib/librte_eal/bsdapp/eal/eal_pci.c             |   19 ++++++++++++
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |    3 ++
>  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 +++++++++++++++++
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |    3 ++
>  7 files changed, 131 insertions(+)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
> index 95c32c1..2e535ea 100644
> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
...
> +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;

I'd suggest to reference this var inside pci_vfio_read/write_bar(), and
pass device as the parmater instead.

> +
> +	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");
                                   ^^^^^
typo.


BTW, I have a question about this API. Obviously, reading/writing bar
space is supported with UIO (when memory resource is mmapped). And I
know why you introduced such 2 APIs, for reading IO bar.

So, here is the question: what are the 2 APIs for, for being gerneric
APIs to read/write bar spaces, or just to read IO bar spaces? If it's
former, the message is wrong; if it's later, you may better rename it
to rte_eal_pci_read/write_io_bar()?

David, what do you think of that?


> +		return -1;
> +	}
> +}
> +
...
> +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) {

A minor nit: it's more nature to put the '||' at the end of expression,
instead of at the front:

    if (bar_idx < VFIO_PCI_BAR0_REGION_INDEX ||
        bar_idx > VFIO_PCI_BAR5_REGION_INDEX) {


	--yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[Santosh Shukla]

On Mon, Feb 1, 2016 at 7:18 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> On Fri, Jan 29, 2016 at 11:51:50PM +0530, Santosh Shukla wrote:
>> Introducing below api for pci bar region rd/wr.
>> Api's are:
>> - rte_eal_pci_read_bar
>> - rte_eal_pci_write_bar
>>
>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>> ---
>> v5-->v6:
>> - update api infor in rte_eal_version.map file
>>   suggested by david manchand.
>>
>>  lib/librte_eal/bsdapp/eal/eal_pci.c             |   19 ++++++++++++
>>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |    3 ++
>>  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 +++++++++++++++++
>>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |    3 ++
>>  7 files changed, 131 insertions(+)
>>
>> diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
>> index 95c32c1..2e535ea 100644
>> --- a/lib/librte_eal/bsdapp/eal/eal_pci.c
>> +++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
> ...
>> +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;
>
> I'd suggest to reference this var inside pci_vfio_read/write_bar(), and
> pass device as the parmater instead.
>

(Sorry for late reply, I was travelling on Monday.)
Make sense.

>> +
>> +     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");
>                                    ^^^^^
> typo.
>

Oh, r / write / read, right? sorry for typo error (:-

>
> BTW, I have a question about this API. Obviously, reading/writing bar
> space is supported with UIO (when memory resource is mmapped). And I
> know why you introduced such 2 APIs, for reading IO bar.
>
> So, here is the question: what are the 2 APIs for, for being gerneric
> APIs to read/write bar spaces, or just to read IO bar spaces? If it's
> former, the message is wrong; if it's later, you may better rename it
> to rte_eal_pci_read/write_io_bar()?
>

Current use-case is virtio: It is used as io_bar which is first
bar[1]. But implementation is generic, can be used to do rd/wr for
other bar index too. Also vfio facilitate user to do rd/wr to pci_bars
w/o mapping that bar, So apis will be useful for such cases in future.

AFAIU: uio has read/write_config api only and Yes if bar region mapped
then no need to do rd/wr, user can directly access the pci_memory. But
use-case of this api entirely different: unmapped memory by
application context i.e.. vfio_rd/wr-way {pread/pwrite-way}.

Is above explanation convincing? Pl. let me know.

[1] https://en.wikipedia.org/wiki/PCI_configuration_space (first bar
offset 0x010)

> David, what do you think of that?
>
>
>> +             return -1;
>> +     }
>> +}
>> +
> ...
>> +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) {
>
> A minor nit: it's more nature to put the '||' at the end of expression,
> instead of at the front:
>
>     if (bar_idx < VFIO_PCI_BAR0_REGION_INDEX ||
>         bar_idx > VFIO_PCI_BAR5_REGION_INDEX) {
>
>
>         --yliu

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

[Santosh Shukla]

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

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

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

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

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

So we don't need explicit _noIOMMU.

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

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

[Yuanhan Liu]

On Tue, Feb 02, 2016 at 10:00:36AM +0530, Santosh Shukla wrote:
> >
> > BTW, I didn't follow the noIOMMU discussion; how did it end? Do we still
> > need that? Is this patch a full story to enable virtio on ARM?
> >
> Ok, We agreed that explicit __noiommu suffix not required, atleast for
> rte_xx_drv struct{}, as because sooner than later we'll have virtio
> working for both flavours ie... iommu/noiommu. My only worry was
> parsing for _noiommu and default vfio case, as because noiomu needed
> user to preset "enable_noiommu_" param for vfio driver to do mode
> switch. But we wont need that parsing as because if param is not set
> then binding won't happen, which Thomas rightly pointed out, therefore
> I choose to drop resource parsing for virtio-for-vfio case, now virtio
> driver to check only drv->kdrv == RTE_KDRV_VFIO so to make sure
> interface attached to vfio or not.
> 
> But perhaps when we have both flavours working for virtio, then we
> should at least prompt a  INFO message on console that virtio pmd
> driver attached to default vfio or noIOMMU.
> 
> So we don't need explicit _noIOMMU.

Thanks for the explanation.
> 
> Yes this patch is to enable non-x86 arch to use virtio pmd driver
> (virtio 0.95 spec). After this patch merges-in, I am planning to
> - replace sys/io.h entirely

Hmm, be more specific? Replace it with what?

> - Add raw_read/raw_writel() api for arm/arm64 {Already proposed
> similar implementation in v2} so that they could use virtio 1.0spec
> mapped memory, for both UIO/VFIO mode.

PCI memory bar mapping works both with UIO/VFIO on ARM, without
any extra efforts, right? If so, it should just work with my
patch set and yours.

	--yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[Yuanhan Liu]

On Tue, Feb 02, 2016 at 09:44:14AM +0530, Santosh Shukla wrote:
> >> +int rte_eal_pci_read_bar(const struct rte_pci_device *device,
> >> +                      void *buf, size_t len, off_t offset,
> >> +                      int bar_idx)
> >> +
> >> +{
> >> +     const struct rte_intr_handle *intr_handle = &device->intr_handle;
> >
> > I'd suggest to reference this var inside pci_vfio_read/write_bar(), and
> > pass device as the parmater instead.
> >
> 
> (Sorry for late reply, I was travelling on Monday.)
> Make sense.
> 
> >> +
> >> +     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");
> >                                    ^^^^^
> > typo.
> >
> 
> Oh, r / write / read, right? sorry for typo error (:-

Right.

> 
> >
> > BTW, I have a question about this API. Obviously, reading/writing bar
> > space is supported with UIO (when memory resource is mmapped). And I
> > know why you introduced such 2 APIs, for reading IO bar.
> >
> > So, here is the question: what are the 2 APIs for, for being gerneric
> > APIs to read/write bar spaces, or just to read IO bar spaces? If it's
> > former, the message is wrong; if it's later, you may better rename it
> > to rte_eal_pci_read/write_io_bar()?
> >
> 
> Current use-case is virtio: It is used as io_bar which is first
> bar[1]. But implementation is generic, can be used to do rd/wr for
> other bar index too. Also vfio facilitate user to do rd/wr to pci_bars
> w/o mapping that bar, So apis will be useful for such cases in future.
> 
> AFAIU: uio has read/write_config api only and Yes if bar region mapped
> then no need to do rd/wr, user can directly access the pci_memory. But
> use-case of this api entirely different: unmapped memory by
> application context i.e.. vfio_rd/wr-way {pread/pwrite-way}.
> 
> Is above explanation convincing? Pl. let me know.

TBH, not really. So, as you stated, it should be generic APIs to
read/write bar space, but limiting it to VFIO only and claiming
that read/write bar space is not support by other drivers (such
as UIO) while in fact it can (in some ways) doesn't seem right
to me.

Anyway, it's just some thoughts from me. David, comments?

	--yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[David Marchand]

On Tue, Feb 2, 2016 at 6:43 AM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> On Tue, Feb 02, 2016 at 09:44:14AM +0530, Santosh Shukla wrote:
>> Current use-case is virtio: It is used as io_bar which is first
>> bar[1]. But implementation is generic, can be used to do rd/wr for
>> other bar index too. Also vfio facilitate user to do rd/wr to pci_bars
>> w/o mapping that bar, So apis will be useful for such cases in future.
>>
>> AFAIU: uio has read/write_config api only and Yes if bar region mapped
>> then no need to do rd/wr, user can directly access the pci_memory. But
>> use-case of this api entirely different: unmapped memory by
>> application context i.e.. vfio_rd/wr-way {pread/pwrite-way}.
>>
>> Is above explanation convincing? Pl. let me know.
>
> TBH, not really. So, as you stated, it should be generic APIs to
> read/write bar space, but limiting it to VFIO only and claiming
> that read/write bar space is not support by other drivers (such
> as UIO) while in fact it can (in some ways) doesn't seem right
> to me.
>
> Anyway, it's just some thoughts from me. David, comments?

>From the very start, same opinion.
We should have a unique api to access those, and eal should hide
details like kernel drivers (uio, vfio, whatever) to the pmd.

Now the thing is, how to do this in an elegant and efficient way.


Regard,
-- 
David Marchand

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

[Santosh Shukla]

On Tue, Feb 2, 2016 at 10:49 AM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Tue, Feb 02, 2016 at 10:00:36AM +0530, Santosh Shukla wrote:
>> >
>> > BTW, I didn't follow the noIOMMU discussion; how did it end? Do we still
>> > need that? Is this patch a full story to enable virtio on ARM?
>> >
>> Ok, We agreed that explicit __noiommu suffix not required, atleast for
>> rte_xx_drv struct{}, as because sooner than later we'll have virtio
>> working for both flavours ie... iommu/noiommu. My only worry was
>> parsing for _noiommu and default vfio case, as because noiomu needed
>> user to preset "enable_noiommu_" param for vfio driver to do mode
>> switch. But we wont need that parsing as because if param is not set
>> then binding won't happen, which Thomas rightly pointed out, therefore
>> I choose to drop resource parsing for virtio-for-vfio case, now virtio
>> driver to check only drv->kdrv == RTE_KDRV_VFIO so to make sure
>> interface attached to vfio or not.
>>
>> But perhaps when we have both flavours working for virtio, then we
>> should at least prompt a  INFO message on console that virtio pmd
>> driver attached to default vfio or noIOMMU.
>>
>> So we don't need explicit _noIOMMU.
>
> Thanks for the explanation.
>>
>> Yes this patch is to enable non-x86 arch to use virtio pmd driver
>> (virtio 0.95 spec). After this patch merges-in, I am planning to
>> - replace sys/io.h entirely
>
> Hmm, be more specific? Replace it with what?
>

Just to remove ifdef clutter in general from dpdk source and mo

>> - Add raw_read/raw_writel() api for arm/arm64 {Already proposed
>> similar implementation in v2} so that they could use virtio 1.0spec
>> mapped memory, for both UIO/VFIO mode.
>
> PCI memory bar mapping works both with UIO/VFIO on ARM, without
> any extra efforts, right? If so, it should just work with my
> patch set and yours.
>

Mostl likely, That I have not tried. (todo),
>         --yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[Santosh Shukla]

On Tue, Feb 2, 2016 at 11:13 AM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Tue, Feb 02, 2016 at 09:44:14AM +0530, Santosh Shukla wrote:
>> >> +int rte_eal_pci_read_bar(const struct rte_pci_device *device,
>> >> +                      void *buf, size_t len, off_t offset,
>> >> +                      int bar_idx)
>> >> +
>> >> +{
>> >> +     const struct rte_intr_handle *intr_handle = &device->intr_handle;
>> >
>> > I'd suggest to reference this var inside pci_vfio_read/write_bar(), and
>> > pass device as the parmater instead.
>> >
>>
>> (Sorry for late reply, I was travelling on Monday.)
>> Make sense.
>>
>> >> +
>> >> +     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");
>> >                                    ^^^^^
>> > typo.
>> >
>>
>> Oh, r / write / read, right? sorry for typo error (:-
>
> Right.
>
>>
>> >
>> > BTW, I have a question about this API. Obviously, reading/writing bar
>> > space is supported with UIO (when memory resource is mmapped). And I
>> > know why you introduced such 2 APIs, for reading IO bar.
>> >
>> > So, here is the question: what are the 2 APIs for, for being gerneric
>> > APIs to read/write bar spaces, or just to read IO bar spaces? If it's
>> > former, the message is wrong; if it's later, you may better rename it
>> > to rte_eal_pci_read/write_io_bar()?
>> >
>>
>> Current use-case is virtio: It is used as io_bar which is first
>> bar[1]. But implementation is generic, can be used to do rd/wr for
>> other bar index too. Also vfio facilitate user to do rd/wr to pci_bars
>> w/o mapping that bar, So apis will be useful for such cases in future.
>>
>> AFAIU: uio has read/write_config api only and Yes if bar region mapped
>> then no need to do rd/wr, user can directly access the pci_memory. But
>> use-case of this api entirely different: unmapped memory by
>> application context i.e.. vfio_rd/wr-way {pread/pwrite-way}.
>>
>> Is above explanation convincing? Pl. let me know.
>
> TBH, not really. So, as you stated, it should be generic APIs to
> read/write bar space, but limiting it to VFIO only and claiming
> that read/write bar space is not support by other drivers (such
> as UIO) while in fact it can (in some ways) doesn't seem right
> to me.
>

I agree.. But if UIO doesn't and need could, then I am confused what
can be done? However we have a use-case for vfio so It make sense to
me use this api.  Or else If we all agree then I can export api only
for VFIO.. but it will violate EAL abstraction.


> Anyway, it's just some thoughts from me. David, comments?
>
>         --yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[Santosh Shukla]

On Tue, Feb 2, 2016 at 12:30 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> On Tue, Feb 2, 2016 at 11:13 AM, Yuanhan Liu
> <yuanhan.liu@linux.intel.com> wrote:
>> On Tue, Feb 02, 2016 at 09:44:14AM +0530, Santosh Shukla wrote:
>>> >> +int rte_eal_pci_read_bar(const struct rte_pci_device *device,
>>> >> +                      void *buf, size_t len, off_t offset,
>>> >> +                      int bar_idx)
>>> >> +
>>> >> +{
>>> >> +     const struct rte_intr_handle *intr_handle = &device->intr_handle;
>>> >
>>> > I'd suggest to reference this var inside pci_vfio_read/write_bar(), and
>>> > pass device as the parmater instead.
>>> >
>>>
>>> (Sorry for late reply, I was travelling on Monday.)
>>> Make sense.
>>>
>>> >> +
>>> >> +     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");
>>> >                                    ^^^^^
>>> > typo.
>>> >
>>>
>>> Oh, r / write / read, right? sorry for typo error (:-
>>
>> Right.
>>
>>>
>>> >
>>> > BTW, I have a question about this API. Obviously, reading/writing bar
>>> > space is supported with UIO (when memory resource is mmapped). And I
>>> > know why you introduced such 2 APIs, for reading IO bar.
>>> >
>>> > So, here is the question: what are the 2 APIs for, for being gerneric
>>> > APIs to read/write bar spaces, or just to read IO bar spaces? If it's
>>> > former, the message is wrong; if it's later, you may better rename it
>>> > to rte_eal_pci_read/write_io_bar()?
>>> >
>>>
>>> Current use-case is virtio: It is used as io_bar which is first
>>> bar[1]. But implementation is generic, can be used to do rd/wr for
>>> other bar index too. Also vfio facilitate user to do rd/wr to pci_bars
>>> w/o mapping that bar, So apis will be useful for such cases in future.
>>>
>>> AFAIU: uio has read/write_config api only and Yes if bar region mapped
>>> then no need to do rd/wr, user can directly access the pci_memory. But
>>> use-case of this api entirely different: unmapped memory by
>>> application context i.e.. vfio_rd/wr-way {pread/pwrite-way}.
>>>
>>> Is above explanation convincing? Pl. let me know.
>>
>> TBH, not really. So, as you stated, it should be generic APIs to
>> read/write bar space, but limiting it to VFIO only and claiming
>> that read/write bar space is not support by other drivers (such
>> as UIO) while in fact it can (in some ways) doesn't seem right
>> to me.
>>
>

Sorry typo
> I agree.. But if UIO doesn't and need could, then I am confused what

r / But if UIO doesn't and need could / But if UIO doesn't and vfio could

> can be done? However we have a use-case for vfio so It make sense to
> me use this api.  Or else If we all agree then I can export api only
> for VFIO.. but it will violate EAL abstraction.
>
>
>> Anyway, it's just some thoughts from me. David, comments?
>>
>>         --yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[Yuanhan Liu]

On Tue, Feb 02, 2016 at 06:50:18AM +0100, David Marchand wrote:
> On Tue, Feb 2, 2016 at 6:43 AM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> > On Tue, Feb 02, 2016 at 09:44:14AM +0530, Santosh Shukla wrote:
> >> Current use-case is virtio: It is used as io_bar which is first
> >> bar[1]. But implementation is generic, can be used to do rd/wr for
> >> other bar index too. Also vfio facilitate user to do rd/wr to pci_bars
> >> w/o mapping that bar, So apis will be useful for such cases in future.
> >>
> >> AFAIU: uio has read/write_config api only and Yes if bar region mapped
> >> then no need to do rd/wr, user can directly access the pci_memory. But
> >> use-case of this api entirely different: unmapped memory by
> >> application context i.e.. vfio_rd/wr-way {pread/pwrite-way}.
> >>
> >> Is above explanation convincing? Pl. let me know.
> >
> > TBH, not really. So, as you stated, it should be generic APIs to
> > read/write bar space, but limiting it to VFIO only and claiming
> > that read/write bar space is not support by other drivers (such
> > as UIO) while in fact it can (in some ways) doesn't seem right
> > to me.
> >
> > Anyway, it's just some thoughts from me. David, comments?
> 
> >From the very start, same opinion.
> We should have a unique api to access those, and eal should hide
> details like kernel drivers (uio, vfio, whatever) to the pmd.
> 
> Now the thing is, how to do this in an elegant and efficient way.

I was thinking that we may just make it be IO port specific read/
write functions:

	rte_eal_pci_ioport_read(dev, bar, buf, size)
	{

		return if not an IO bar;

		if (has io)
			return inb/w/l();

		if (vfio)
			return vfio_ioport_read();

		else, claim aloud that io port read is not allowed
	}

Let us not handle memory bar resource here: in such case, you should
go with rte_eal_pci_map_device() and do it with memory mapped io.

Does that make any sense?

	--yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[Santosh Shukla]

On Tue, Feb 2, 2016 at 2:19 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> On Tue, Feb 02, 2016 at 06:50:18AM +0100, David Marchand wrote:
>> On Tue, Feb 2, 2016 at 6:43 AM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
>> > On Tue, Feb 02, 2016 at 09:44:14AM +0530, Santosh Shukla wrote:
>> >> Current use-case is virtio: It is used as io_bar which is first
>> >> bar[1]. But implementation is generic, can be used to do rd/wr for
>> >> other bar index too. Also vfio facilitate user to do rd/wr to pci_bars
>> >> w/o mapping that bar, So apis will be useful for such cases in future.
>> >>
>> >> AFAIU: uio has read/write_config api only and Yes if bar region mapped
>> >> then no need to do rd/wr, user can directly access the pci_memory. But
>> >> use-case of this api entirely different: unmapped memory by
>> >> application context i.e.. vfio_rd/wr-way {pread/pwrite-way}.
>> >>
>> >> Is above explanation convincing? Pl. let me know.
>> >
>> > TBH, not really. So, as you stated, it should be generic APIs to
>> > read/write bar space, but limiting it to VFIO only and claiming
>> > that read/write bar space is not support by other drivers (such
>> > as UIO) while in fact it can (in some ways) doesn't seem right
>> > to me.
>> >
>> > Anyway, it's just some thoughts from me. David, comments?
>>
>> >From the very start, same opinion.
>> We should have a unique api to access those, and eal should hide
>> details like kernel drivers (uio, vfio, whatever) to the pmd.
>>
>> Now the thing is, how to do this in an elegant and efficient way.
>
> I was thinking that we may just make it be IO port specific read/
> write functions:
>

Ok,

>         rte_eal_pci_ioport_read(dev, bar, buf, size)
>         {
>
>                 return if not an IO bar;
>
>                 if (has io)
>                         return inb/w/l();
>

In that case, It may be r / if (has io) / if (drv->kdrv == UIO)

>                 if (vfio)
>                         return vfio_ioport_read();
>
>                 else, claim aloud that io port read is not allowed
>         }
>
> Let us not handle memory bar resource here: in such case, you should
> go with rte_eal_pci_map_device() and do it with memory mapped io.
>
> Does that make any sense?
>
I am not entirely sure.
Are you considering IGB_UIO, UIO_GENERIC and NIC_UIO: all the cases ?


>         --yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[Santosh Shukla]

On Tue, Feb 2, 2016 at 9:21 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> On Tue, Feb 2, 2016 at 2:19 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
>> On Tue, Feb 02, 2016 at 06:50:18AM +0100, David Marchand wrote:
>>> On Tue, Feb 2, 2016 at 6:43 AM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
>>> > On Tue, Feb 02, 2016 at 09:44:14AM +0530, Santosh Shukla wrote:
>>> >> Current use-case is virtio: It is used as io_bar which is first
>>> >> bar[1]. But implementation is generic, can be used to do rd/wr for
>>> >> other bar index too. Also vfio facilitate user to do rd/wr to pci_bars
>>> >> w/o mapping that bar, So apis will be useful for such cases in future.
>>> >>
>>> >> AFAIU: uio has read/write_config api only and Yes if bar region mapped
>>> >> then no need to do rd/wr, user can directly access the pci_memory. But
>>> >> use-case of this api entirely different: unmapped memory by
>>> >> application context i.e.. vfio_rd/wr-way {pread/pwrite-way}.
>>> >>
>>> >> Is above explanation convincing? Pl. let me know.
>>> >
>>> > TBH, not really. So, as you stated, it should be generic APIs to
>>> > read/write bar space, but limiting it to VFIO only and claiming
>>> > that read/write bar space is not support by other drivers (such
>>> > as UIO) while in fact it can (in some ways) doesn't seem right
>>> > to me.
>>> >
>>> > Anyway, it's just some thoughts from me. David, comments?
>>>
>>> >From the very start, same opinion.
>>> We should have a unique api to access those, and eal should hide
>>> details like kernel drivers (uio, vfio, whatever) to the pmd.
>>>
>>> Now the thing is, how to do this in an elegant and efficient way.
>>
>> I was thinking that we may just make it be IO port specific read/
>> write functions:
>>
>
> Ok,
>
>>         rte_eal_pci_ioport_read(dev, bar, buf, size)
>>         {
>>
>>                 return if not an IO bar;
>>
>>                 if (has io)
>>                         return inb/w/l();
>>
>
> In that case, It may be r / if (has io) / if (drv->kdrv == UIO)
>
>>                 if (vfio)
>>                         return vfio_ioport_read();
>>
>>                 else, claim aloud that io port read is not allowed
>>         }
>>
>> Let us not handle memory bar resource here: in such case, you should
>> go with rte_eal_pci_map_device() and do it with memory mapped io.
>>
>> Does that make any sense?
>>
> I am not entirely sure.
> Are you considering IGB_UIO, UIO_GENERIC and NIC_UIO: all the cases ?
>

Just came-up something below what Yuanhan has proposed, Does this look okay?

int rte_eal_pci_ioport_read(const struct rte_pci_device *device,
                                              void *buf, size_t len,
off_t offset,
                                              int bar_idx)
{
      if (bar_idx != 0) {
              RTE_LOG(ERR, EAL, "not a ioport bar\n");
              return -1;
       }

     switch (device->kdrv) {
     case RTE_KDRV_VFIO:
               return pci_vfio_ioport_read(device, buf, len, offset, bar_idx);
     case RTE_KDRV_IGB_UIO:
     case RTE_KDRV_UIO_GENERIC:
     case RTE_KDRV_NIC_UIO:
         {
              switch (size)
                      case 1: return inb(buf /*ioport address*/);
                      case 2: return inw(buf /* ioport address*/);
                      case 4: return inl(buf /* ioport address*/);
                      default:
                             RTE_LOG(ERR, EAL, "invalid size\n");
          }

       default:
                 RTE_LOG(ERR, EAL, "read bar not supported by driver\n");
                 return -1;
      }
}

>
>>         --yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[Santosh Shukla]

On Tue, Feb 2, 2016 at 9:48 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> On Tue, Feb 2, 2016 at 9:21 PM, Santosh Shukla <sshukla@mvista.com> wrote:
>> On Tue, Feb 2, 2016 at 2:19 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
>>> On Tue, Feb 02, 2016 at 06:50:18AM +0100, David Marchand wrote:
>>>> On Tue, Feb 2, 2016 at 6:43 AM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
>>>> > On Tue, Feb 02, 2016 at 09:44:14AM +0530, Santosh Shukla wrote:
>>>> >> Current use-case is virtio: It is used as io_bar which is first
>>>> >> bar[1]. But implementation is generic, can be used to do rd/wr for
>>>> >> other bar index too. Also vfio facilitate user to do rd/wr to pci_bars
>>>> >> w/o mapping that bar, So apis will be useful for such cases in future.
>>>> >>
>>>> >> AFAIU: uio has read/write_config api only and Yes if bar region mapped
>>>> >> then no need to do rd/wr, user can directly access the pci_memory. But
>>>> >> use-case of this api entirely different: unmapped memory by
>>>> >> application context i.e.. vfio_rd/wr-way {pread/pwrite-way}.
>>>> >>
>>>> >> Is above explanation convincing? Pl. let me know.
>>>> >
>>>> > TBH, not really. So, as you stated, it should be generic APIs to
>>>> > read/write bar space, but limiting it to VFIO only and claiming
>>>> > that read/write bar space is not support by other drivers (such
>>>> > as UIO) while in fact it can (in some ways) doesn't seem right
>>>> > to me.
>>>> >
>>>> > Anyway, it's just some thoughts from me. David, comments?
>>>>
>>>> >From the very start, same opinion.
>>>> We should have a unique api to access those, and eal should hide
>>>> details like kernel drivers (uio, vfio, whatever) to the pmd.
>>>>
>>>> Now the thing is, how to do this in an elegant and efficient way.
>>>
>>> I was thinking that we may just make it be IO port specific read/
>>> write functions:
>>>
>>
>> Ok,
>>
>>>         rte_eal_pci_ioport_read(dev, bar, buf, size)
>>>         {
>>>
>>>                 return if not an IO bar;
>>>
>>>                 if (has io)
>>>                         return inb/w/l();
>>>
>>
>> In that case, It may be r / if (has io) / if (drv->kdrv == UIO)
>>
>>>                 if (vfio)
>>>                         return vfio_ioport_read();
>>>
>>>                 else, claim aloud that io port read is not allowed
>>>         }
>>>
>>> Let us not handle memory bar resource here: in such case, you should
>>> go with rte_eal_pci_map_device() and do it with memory mapped io.
>>>
>>> Does that make any sense?
>>>
>> I am not entirely sure.
>> Are you considering IGB_UIO, UIO_GENERIC and NIC_UIO: all the cases ?
>>
>
> Just came-up something below what Yuanhan has proposed, Does this look okay?
>
> int rte_eal_pci_ioport_read(const struct rte_pci_device *device,
>                                               void *buf, size_t len,
> off_t offset,
>                                               int bar_idx)
> {
>       if (bar_idx != 0) {
>               RTE_LOG(ERR, EAL, "not a ioport bar\n");
>               return -1;
>        }
>
>      switch (device->kdrv) {
>      case RTE_KDRV_VFIO:
>                return pci_vfio_ioport_read(device, buf, len, offset, bar_idx);
>      case RTE_KDRV_IGB_UIO:
>      case RTE_KDRV_UIO_GENERIC:
>      case RTE_KDRV_NIC_UIO:
>          {
>               switch (size)
>                       case 1: return inb(buf /*ioport address*/);
>                       case 2: return inw(buf /* ioport address*/);
>                       case 4: return inl(buf /* ioport address*/);
>                       default:
>                              RTE_LOG(ERR, EAL, "invalid size\n");
>           }
>
>        default:
>                  RTE_LOG(ERR, EAL, "read bar not supported by driver\n");
>                  return -1;
>       }
> }
>

Ping?

Also can someone please review rest of series. This patchset going
through multiple revision, Each revision get one / two comment, It
would help if I get review comment for each patch.

>>
>>>         --yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[Yuanhan Liu]

On Tue, Feb 02, 2016 at 09:48:44PM +0530, Santosh Shukla wrote:
> On Tue, Feb 2, 2016 at 9:21 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> > On Tue, Feb 2, 2016 at 2:19 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> >> On Tue, Feb 02, 2016 at 06:50:18AM +0100, David Marchand wrote:
> >>> On Tue, Feb 2, 2016 at 6:43 AM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> >>> > On Tue, Feb 02, 2016 at 09:44:14AM +0530, Santosh Shukla wrote:
> >>> >> Current use-case is virtio: It is used as io_bar which is first
> >>> >> bar[1]. But implementation is generic, can be used to do rd/wr for
> >>> >> other bar index too. Also vfio facilitate user to do rd/wr to pci_bars
> >>> >> w/o mapping that bar, So apis will be useful for such cases in future.
> >>> >>
> >>> >> AFAIU: uio has read/write_config api only and Yes if bar region mapped
> >>> >> then no need to do rd/wr, user can directly access the pci_memory. But
> >>> >> use-case of this api entirely different: unmapped memory by
> >>> >> application context i.e.. vfio_rd/wr-way {pread/pwrite-way}.
> >>> >>
> >>> >> Is above explanation convincing? Pl. let me know.
> >>> >
> >>> > TBH, not really. So, as you stated, it should be generic APIs to
> >>> > read/write bar space, but limiting it to VFIO only and claiming
> >>> > that read/write bar space is not support by other drivers (such
> >>> > as UIO) while in fact it can (in some ways) doesn't seem right
> >>> > to me.
> >>> >
> >>> > Anyway, it's just some thoughts from me. David, comments?
> >>>
> >>> >From the very start, same opinion.
> >>> We should have a unique api to access those, and eal should hide
> >>> details like kernel drivers (uio, vfio, whatever) to the pmd.
> >>>
> >>> Now the thing is, how to do this in an elegant and efficient way.
> >>
> >> I was thinking that we may just make it be IO port specific read/
> >> write functions:
> >>
> >
> > Ok,
> >
> >>         rte_eal_pci_ioport_read(dev, bar, buf, size)
> >>         {
> >>
> >>                 return if not an IO bar;
> >>
> >>                 if (has io)
> >>                         return inb/w/l();
> >>
> >
> > In that case, It may be r / if (has io) / if (drv->kdrv == UIO)

Nope, I meant platform supports inb/w/l() command.

> >
> >>                 if (vfio)
> >>                         return vfio_ioport_read();
> >>
> >>                 else, claim aloud that io port read is not allowed
> >>         }
> >>
> >> Let us not handle memory bar resource here: in such case, you should
> >> go with rte_eal_pci_map_device() and do it with memory mapped io.
> >>
> >> Does that make any sense?
> >>
> > I am not entirely sure.
> > Are you considering IGB_UIO, UIO_GENERIC and NIC_UIO: all the cases ?
> >
> 
> Just came-up something below what Yuanhan has proposed, Does this look okay?
> 
> int rte_eal_pci_ioport_read(const struct rte_pci_device *device,
>                                               void *buf, size_t len,
> off_t offset,
>                                               int bar_idx)

Your implementation doesn't look right to me. But anyway, that's not
important so far; the feasibility is.

David, would you please comment on this?

	--yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[Yuanhan Liu]

On Wed, Feb 03, 2016 at 03:20:09PM +0530, Santosh Shukla wrote:
> On Tue, Feb 2, 2016 at 9:48 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> > On Tue, Feb 2, 2016 at 9:21 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> >> On Tue, Feb 2, 2016 at 2:19 PM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> >>> On Tue, Feb 02, 2016 at 06:50:18AM +0100, David Marchand wrote:
> >>>> On Tue, Feb 2, 2016 at 6:43 AM, Yuanhan Liu <yuanhan.liu@linux.intel.com> wrote:
> >>>> > On Tue, Feb 02, 2016 at 09:44:14AM +0530, Santosh Shukla wrote:
> >>>> >> Current use-case is virtio: It is used as io_bar which is first
> >>>> >> bar[1]. But implementation is generic, can be used to do rd/wr for
> >>>> >> other bar index too. Also vfio facilitate user to do rd/wr to pci_bars
> >>>> >> w/o mapping that bar, So apis will be useful for such cases in future.
> >>>> >>
> >>>> >> AFAIU: uio has read/write_config api only and Yes if bar region mapped
> >>>> >> then no need to do rd/wr, user can directly access the pci_memory. But
> >>>> >> use-case of this api entirely different: unmapped memory by
> >>>> >> application context i.e.. vfio_rd/wr-way {pread/pwrite-way}.
> >>>> >>
> >>>> >> Is above explanation convincing? Pl. let me know.
> >>>> >
> >>>> > TBH, not really. So, as you stated, it should be generic APIs to
> >>>> > read/write bar space, but limiting it to VFIO only and claiming
> >>>> > that read/write bar space is not support by other drivers (such
> >>>> > as UIO) while in fact it can (in some ways) doesn't seem right
> >>>> > to me.
> >>>> >
> >>>> > Anyway, it's just some thoughts from me. David, comments?
> >>>>
> >>>> >From the very start, same opinion.
> >>>> We should have a unique api to access those, and eal should hide
> >>>> details like kernel drivers (uio, vfio, whatever) to the pmd.
> >>>>
> >>>> Now the thing is, how to do this in an elegant and efficient way.
> >>>
> >>> I was thinking that we may just make it be IO port specific read/
> >>> write functions:
> >>>
> >>
> >> Ok,
> >>
> >>>         rte_eal_pci_ioport_read(dev, bar, buf, size)
> >>>         {
> >>>
> >>>                 return if not an IO bar;
> >>>
> >>>                 if (has io)
> >>>                         return inb/w/l();
> >>>
> >>
> >> In that case, It may be r / if (has io) / if (drv->kdrv == UIO)
> >>
> >>>                 if (vfio)
> >>>                         return vfio_ioport_read();
> >>>
> >>>                 else, claim aloud that io port read is not allowed
> >>>         }
> >>>
> >>> Let us not handle memory bar resource here: in such case, you should
> >>> go with rte_eal_pci_map_device() and do it with memory mapped io.
> >>>
> >>> Does that make any sense?
> >>>
> >> I am not entirely sure.
> >> Are you considering IGB_UIO, UIO_GENERIC and NIC_UIO: all the cases ?
> >>
> >
> > Just came-up something below what Yuanhan has proposed, Does this look okay?
> >
> > int rte_eal_pci_ioport_read(const struct rte_pci_device *device,
> >                                               void *buf, size_t len,
> > off_t offset,
> >                                               int bar_idx)
> > {
> >       if (bar_idx != 0) {
> >               RTE_LOG(ERR, EAL, "not a ioport bar\n");
> >               return -1;
> >        }
> >
> >      switch (device->kdrv) {
> >      case RTE_KDRV_VFIO:
> >                return pci_vfio_ioport_read(device, buf, len, offset, bar_idx);
> >      case RTE_KDRV_IGB_UIO:
> >      case RTE_KDRV_UIO_GENERIC:
> >      case RTE_KDRV_NIC_UIO:
> >          {
> >               switch (size)
> >                       case 1: return inb(buf /*ioport address*/);
> >                       case 2: return inw(buf /* ioport address*/);
> >                       case 4: return inl(buf /* ioport address*/);
> >                       default:
> >                              RTE_LOG(ERR, EAL, "invalid size\n");
> >           }
> >
> >        default:
> >                  RTE_LOG(ERR, EAL, "read bar not supported by driver\n");
> >                  return -1;
> >       }
> > }
> >
> 
> Ping?

Please be a bit more patient. Everybody has work got to do; and today I was
out for personal affairs the whole day.

> 
> Also can someone please review rest of series. This patchset going
> through multiple revision, Each revision get one / two comment, It
> would help if I get review comment for each patch.

The others looks good to me; if you have the EAL pci API resolved
properly, I guess I could give my ACK.

	--yliu

Re: [dpdk-dev] [PATCH v6 1/8] eal: pci: add api to rd/wr pci bar region

[David Marchand]

On Wed, Feb 3, 2016 at 12:50 PM, Yuanhan Liu
<yuanhan.liu@linux.intel.com> wrote:
> On Wed, Feb 03, 2016 at 03:20:09PM +0530, Santosh Shukla wrote:
>> Also can someone please review rest of series. This patchset going
>> through multiple revision, Each revision get one / two comment, It
>> would help if I get review comment for each patch.
>
> The others looks good to me; if you have the EAL pci API resolved
> properly, I guess I could give my ACK.

As discussed offlist, I just sent a patchset [1] that proposes a new api.
This is not perfect, but I think this should be fine for x86 / arm.
Santosh should be able to rebase his code on top of it.

[1] http://dpdk.org/ml/archives/dev/2016-February/032778.html


Regards,
-- 
David Marchand

[dpdk-dev] [PATCH v8 3/4] eal/linux: 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>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
---
v7-->v8:
- included Anatoly acked-by:

 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 ffa2dd0..4832313 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -659,6 +659,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;
@@ -853,6 +854,25 @@ pci_vfio_map_resource(struct rte_pci_device *dev)
 			return -1;
 		}
 
+		/* chk for io port region */
+		ret = pread64(vfio_dev_fd, &ioport_bar, sizeof(ioport_bar),
+			      VFIO_GET_REGION_ADDR(VFIO_PCI_CONFIG_REGION_INDEX)
+			      + PCI_BASE_ADDRESS_0 + i*4);
+
+		if (ret != sizeof(ioport_bar)) {
+			RTE_LOG(ERR, EAL,
+				"Cannot read command (%x) from config space!\n",
+				PCI_BASE_ADDRESS_0 + i*4);
+			return -1;
+		}
+
+		if (ioport_bar & PCI_BASE_ADDRESS_SPACE_IO) {
+			RTE_LOG(INFO, EAL,
+				"Ignore mapping IO port bar(%d) addr: %x\n",
+				 i, ioport_bar);
+			continue;
+		}
+
 		/* skip non-mmapable BARs */
 		if ((reg.flags & VFIO_REGION_INFO_FLAG_MMAP) == 0)
 			continue;
-- 
1.7.9.5

[dpdk-dev] [PATCH v8 4/4] eal/linux: vfio: add pci ioport support

[Santosh Shukla]

Include vfio map/rd/wr support for pci ioport.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
v7->v8:
- Remove rte_pci_ioport malloc and rte_free()/unmap() func from v7.
- removed umap from git header.

 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   36 ++++++++++++++++++----------
 1 file changed, 24 insertions(+), 12 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index 4832313..9571ed8 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -74,6 +74,7 @@ EAL_REGISTER_TAILQ(rte_vfio_tailq)
 #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)
+#define VFIO_GET_REGION_IDX(x) (x >> 40)
 
 /* per-process VFIO config */
 static struct vfio_config vfio_cfg;
@@ -999,30 +1000,41 @@ int
 pci_vfio_ioport_map(struct rte_pci_device *dev, int bar,
 		    struct rte_pci_ioport *p)
 {
-	RTE_SET_USED(dev);
-	RTE_SET_USED(bar);
-	RTE_SET_USED(p);
-	return -1;
+	if (bar < VFIO_PCI_BAR0_REGION_INDEX ||
+	    bar > VFIO_PCI_BAR5_REGION_INDEX) {
+		RTE_LOG(ERR, EAL, "invalid bar (%d)!\n", bar);
+		return -1;
+	}
+
+	p->dev = dev;
+	p->offset = VFIO_GET_REGION_ADDR(bar);
+	return 0;
 }
 
 void
 pci_vfio_ioport_read(struct rte_pci_ioport *p,
 		     void *data, size_t len, off_t offset)
 {
-	RTE_SET_USED(p);
-	RTE_SET_USED(data);
-	RTE_SET_USED(len);
-	RTE_SET_USED(offset);
+	const struct rte_intr_handle *intr_handle = &p->dev->intr_handle;
+
+	if (pread64(intr_handle->vfio_dev_fd, data,
+		    len, p->offset + offset) <= 0)
+		RTE_LOG(ERR, EAL,
+			"Can't read from PCI bar (%" PRIu64 ") : offset (%x)\n",
+			VFIO_GET_REGION_IDX(p->offset), (int)offset);
 }
 
 void
 pci_vfio_ioport_write(struct rte_pci_ioport *p,
 		      const void *data, size_t len, off_t offset)
 {
-	RTE_SET_USED(p);
-	RTE_SET_USED(data);
-	RTE_SET_USED(len);
-	RTE_SET_USED(offset);
+	const struct rte_intr_handle *intr_handle = &p->dev->intr_handle;
+
+	if (pwrite64(intr_handle->vfio_dev_fd, data,
+		     len, p->offset + offset) <= 0)
+		RTE_LOG(ERR, EAL,
+			"Can't write to PCI bar (%" PRIu64 ") : offset (%x)\n",
+			VFIO_GET_REGION_IDX(p->offset), (int)offset);
 }
 
 int
-- 
1.7.9.5

Re: [dpdk-dev] [PATCH v8 4/4] eal/linux: vfio: add pci ioport support

[Burakov, Anatoly]

> Include vfio map/rd/wr support for pci ioport.
> 
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> ---
> v7->v8:
> - Remove rte_pci_ioport malloc and rte_free()/unmap() func from v7.
> - removed umap from git header.
> 
>  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   36 ++++++++++++++++++-------
> ---
>  1 file changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> index 4832313..9571ed8 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
> @@ -74,6 +74,7 @@ EAL_REGISTER_TAILQ(rte_vfio_tailq)
>  #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)
> +#define VFIO_GET_REGION_IDX(x) (x >> 40)
> 
>  /* per-process VFIO config */
>  static struct vfio_config vfio_cfg;
> @@ -999,30 +1000,41 @@ int
>  pci_vfio_ioport_map(struct rte_pci_device *dev, int bar,
>  		    struct rte_pci_ioport *p)
>  {
> -	RTE_SET_USED(dev);
> -	RTE_SET_USED(bar);
> -	RTE_SET_USED(p);
> -	return -1;
> +	if (bar < VFIO_PCI_BAR0_REGION_INDEX ||
> +	    bar > VFIO_PCI_BAR5_REGION_INDEX) {
> +		RTE_LOG(ERR, EAL, "invalid bar (%d)!\n", bar);
> +		return -1;
> +	}
> +
> +	p->dev = dev;
> +	p->offset = VFIO_GET_REGION_ADDR(bar);
> +	return 0;
>  }
> 
>  void
>  pci_vfio_ioport_read(struct rte_pci_ioport *p,
>  		     void *data, size_t len, off_t offset)
>  {
> -	RTE_SET_USED(p);
> -	RTE_SET_USED(data);
> -	RTE_SET_USED(len);
> -	RTE_SET_USED(offset);
> +	const struct rte_intr_handle *intr_handle = &p->dev->intr_handle;
> +
> +	if (pread64(intr_handle->vfio_dev_fd, data,
> +		    len, p->offset + offset) <= 0)
> +		RTE_LOG(ERR, EAL,
> +			"Can't read from PCI bar (%" PRIu64 ") : offset
> (%x)\n",
> +			VFIO_GET_REGION_IDX(p->offset), (int)offset);
>  }
> 
>  void
>  pci_vfio_ioport_write(struct rte_pci_ioport *p,
>  		      const void *data, size_t len, off_t offset)
>  {
> -	RTE_SET_USED(p);
> -	RTE_SET_USED(data);
> -	RTE_SET_USED(len);
> -	RTE_SET_USED(offset);
> +	const struct rte_intr_handle *intr_handle = &p->dev->intr_handle;
> +
> +	if (pwrite64(intr_handle->vfio_dev_fd, data,
> +		     len, p->offset + offset) <= 0)
> +		RTE_LOG(ERR, EAL,
> +			"Can't write to PCI bar (%" PRIu64 ") : offset (%x)\n",
> +			VFIO_GET_REGION_IDX(p->offset), (int)offset);
>  }
> 
>  int
> --
> 1.7.9.5

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Re: [dpdk-dev] [PATCH v8 4/4] eal/linux: vfio: add pci ioport support

[David Marchand]

On Mon, Feb 8, 2016 at 3:13 PM, Burakov, Anatoly
<anatoly.burakov@intel.com> wrote:
>> Include vfio map/rd/wr support for pci ioport.
>>
>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>> ---
>> v7->v8:
>> - Remove rte_pci_ioport malloc and rte_free()/unmap() func from v7.
>> - removed umap from git header.
>>
>>  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   36 ++++++++++++++++++-------
>> ---
>>  1 file changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>> b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>> index 4832313..9571ed8 100644
>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>> @@ -74,6 +74,7 @@ EAL_REGISTER_TAILQ(rte_vfio_tailq)
>>  #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)
>> +#define VFIO_GET_REGION_IDX(x) (x >> 40)
>>
>>  /* per-process VFIO config */
>>  static struct vfio_config vfio_cfg;
>> @@ -999,30 +1000,41 @@ int
>>  pci_vfio_ioport_map(struct rte_pci_device *dev, int bar,
>>                   struct rte_pci_ioport *p)
>>  {
>> -     RTE_SET_USED(dev);
>> -     RTE_SET_USED(bar);
>> -     RTE_SET_USED(p);
>> -     return -1;
>> +     if (bar < VFIO_PCI_BAR0_REGION_INDEX ||
>> +         bar > VFIO_PCI_BAR5_REGION_INDEX) {
>> +             RTE_LOG(ERR, EAL, "invalid bar (%d)!\n", bar);
>> +             return -1;
>> +     }
>> +
>> +     p->dev = dev;
>> +     p->offset = VFIO_GET_REGION_ADDR(bar);
>> +     return 0;
>>  }
>>

I still think we don't need this p->dev = dev.
But that's not important.


> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Acked-by: David Marchand <david.marchand@6wind.com>


-- 
David Marchand

Re: [dpdk-dev] [PATCH v8 4/4] eal/linux: vfio: add pci ioport support

[Santosh Shukla]

On Tue, Feb 9, 2016 at 2:34 PM, David Marchand <david.marchand@6wind.com> wrote:
> On Mon, Feb 8, 2016 at 3:13 PM, Burakov, Anatoly
> <anatoly.burakov@intel.com> wrote:
>>> Include vfio map/rd/wr support for pci ioport.
>>>
>>> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
>>> ---
>>> v7->v8:
>>> - Remove rte_pci_ioport malloc and rte_free()/unmap() func from v7.
>>> - removed umap from git header.
>>>
>>>  lib/librte_eal/linuxapp/eal/eal_pci_vfio.c |   36 ++++++++++++++++++-------
>>> ---
>>>  1 file changed, 24 insertions(+), 12 deletions(-)
>>>
>>> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>>> b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>>> index 4832313..9571ed8 100644
>>> --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>>> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
>>> @@ -74,6 +74,7 @@ EAL_REGISTER_TAILQ(rte_vfio_tailq)
>>>  #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)
>>> +#define VFIO_GET_REGION_IDX(x) (x >> 40)
>>>
>>>  /* per-process VFIO config */
>>>  static struct vfio_config vfio_cfg;
>>> @@ -999,30 +1000,41 @@ int
>>>  pci_vfio_ioport_map(struct rte_pci_device *dev, int bar,
>>>                   struct rte_pci_ioport *p)
>>>  {
>>> -     RTE_SET_USED(dev);
>>> -     RTE_SET_USED(bar);
>>> -     RTE_SET_USED(p);
>>> -     return -1;
>>> +     if (bar < VFIO_PCI_BAR0_REGION_INDEX ||
>>> +         bar > VFIO_PCI_BAR5_REGION_INDEX) {
>>> +             RTE_LOG(ERR, EAL, "invalid bar (%d)!\n", bar);
>>> +             return -1;
>>> +     }
>>> +
>>> +     p->dev = dev;
>>> +     p->offset = VFIO_GET_REGION_ADDR(bar);
>>> +     return 0;
>>>  }
>>>
>
> I still think we don't need this p->dev = dev.
> But that's not important.
>
>
>> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
> Acked-by: David Marchand <david.marchand@6wind.com>
>

Thomas, Can you pl. merge this patch?

>
> --
> David Marchand

Re: [dpdk-dev] [PATCH v8 4/4] eal/linux: vfio: add pci ioport support

[Thomas Monjalon]

2016-02-18 10:55, Santosh Shukla:
> >> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >
> > Acked-by: David Marchand <david.marchand@6wind.com>
> >
> 
> Thomas, Can you pl. merge this patch?

I must merge the whole series but there are still some comments to address.
If you want to split the series, you're allowed to do it by re-sending a subset.
Please use --in-reply-to as much as possible to keep all related patches in
the same thread.