All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] Add virtio support in arm/arm64
@ 2015-12-04 17:35 Santosh Shukla
  2015-12-04 17:35 ` [PATCH 1/6] virtio: Introduce config RTE_VIRTIO_INC_VECTOR Santosh Shukla
                   ` (6 more replies)
  0 siblings, 7 replies; 22+ messages in thread
From: Santosh Shukla @ 2015-12-04 17:35 UTC (permalink / raw)
  To: dev

This patch set add basic infrastrucure to run virtio-net-pci pmd driver for
arm64/arm. Tested on ThunderX platfrom. Verified for existing dpdk(s) test
applications like:
- ovs-dpdk-vhost-user: across the VM's, for the use-cases like guest2guest and
  Host2Guest
- testpmd application: Tested for max virtio-net-pci interface currently
  supported in kernel i.e. 31 interface. 

Builds successfully for armv7/v8/thunderX and x86_64/i686 platforms. Made sure
that patch changes donot break for x86_64 case. Done similar tests for x86_64
too.

Patch summary:
- First patch adds RTE_VIRTIO_INC_VECTOR config, much needed for archs like
  arm/arm64 as they don't support vectored implementation, also wont able to
  build.
- Second patch is in-general fix for i686.
- Third patch is to emulate x86-style of {in,out}[b,w,l] api support for armv7/v8.
  As virtio-net-pci pmd driver uses those apis for port rd/wr {b,w,l}
- Fourth patch to enable VIRTIO_PMD feature in armv7/v8/thunderX config.
- Fifth patch to disable iopl syscall, As arm/arm64 linux kernel doesn't support
  them.
- Sixth patch introduces ioport memdevice called /dev/igb_ioport by which virtio
  pmd driver could able to rd/wr PCI_IOBAR.
  {applicable for arm/arm64 only, tested for arm64 as of now} 

  On Sixth patch: There is other way to achieve desired by adding support in
  linux kernel character memory driver such that user could do file rd/wr for
  half-word/word size data. Currently character memory driver support byte rd/wr
  via interface `/dev/port`. Such proposal already discussed long back at lkml
  thread [2] but couldn;t got much atttaction though! I am going to send out
  similar implementation as an RFC to dpdk-dev thread and to linux-kernel
  community{todo}. However like to listen to current approach
  review/comment/feedback.

Thanks in advance.

patchset developed on upstream dpdk commit 538020a then pulled jerin's
dependancy patches [1] on top.


[1] http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/29068
[2] https://lkml.org/lkml/2014/5/10/189

Santosh Shukla (6):
  virtio: Introduce config RTE_VIRTIO_INC_VECTOR
  config: i686: set RTE_VIRTIO_INC_VECTOR=n
  virtio: armv7/v8: Introdice api to emulate x86-style of PCI/ISA
    ioport access
  config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD
  linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init()
  virtio: arm/arm64: memory mapped IO support in pmd driver

 config/common_linuxapp                             |    1 +
 config/defconfig_arm-armv7a-linuxapp-gcc           |    6 +-
 config/defconfig_arm64-armv8a-linuxapp-gcc         |    6 +-
 config/defconfig_i686-native-linuxapp-gcc          |    1 +
 config/defconfig_i686-native-linuxapp-icc          |    1 +
 drivers/net/virtio/Makefile                        |    2 +-
 drivers/net/virtio/virtio_ethdev.c                 |  138 ++++++++++++-
 drivers/net/virtio/virtio_pci.h                    |   15 ++
 drivers/net/virtio/virtio_rxtx.c                   |    7 +
 .../common/include/arch/arm/rte_isa_io.h           |  212 ++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal.c                  |    3 +
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c          |   80 +++++++-
 12 files changed, 465 insertions(+), 7 deletions(-)
 create mode 100644 lib/librte_eal/common/include/arch/arm/rte_isa_io.h

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH 1/6] virtio: Introduce config RTE_VIRTIO_INC_VECTOR
  2015-12-04 17:35 [PATCH 0/6] Add virtio support in arm/arm64 Santosh Shukla
@ 2015-12-04 17:35 ` Santosh Shukla
  2015-12-04 17:35 ` [PATCH 2/6] config: i686: set RTE_VIRTIO_INC_VECTOR=n Santosh Shukla
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Santosh Shukla @ 2015-12-04 17:35 UTC (permalink / raw)
  To: dev

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

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

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

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 2/6] config: i686: set RTE_VIRTIO_INC_VECTOR=n
  2015-12-04 17:35 [PATCH 0/6] Add virtio support in arm/arm64 Santosh Shukla
  2015-12-04 17:35 ` [PATCH 1/6] virtio: Introduce config RTE_VIRTIO_INC_VECTOR Santosh Shukla
@ 2015-12-04 17:35 ` Santosh Shukla
  2015-12-04 17:35 ` [PATCH 3/6] virtio: armv7/v8: Introdice api to emulate x86-style of PCI/ISA ioport access Santosh Shukla
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Santosh Shukla @ 2015-12-04 17:35 UTC (permalink / raw)
  To: dev

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

So setting RTE_VIRTIO_INC_VECTOR to 'n'.

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

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 3/6] virtio: armv7/v8: Introdice api to emulate x86-style of PCI/ISA ioport access
  2015-12-04 17:35 [PATCH 0/6] Add virtio support in arm/arm64 Santosh Shukla
  2015-12-04 17:35 ` [PATCH 1/6] virtio: Introduce config RTE_VIRTIO_INC_VECTOR Santosh Shukla
  2015-12-04 17:35 ` [PATCH 2/6] config: i686: set RTE_VIRTIO_INC_VECTOR=n Santosh Shukla
@ 2015-12-04 17:35 ` Santosh Shukla
  2015-12-07 17:09   ` Stephen Hemminger
  2015-12-04 17:35 ` [PATCH 4/6] config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD Santosh Shukla
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 22+ messages in thread
From: Santosh Shukla @ 2015-12-04 17:35 UTC (permalink / raw)
  To: dev

Currently virtio_pci address space accessed in x86-style of ioport apis example:
{in,out}[bwl] and {in_p,out_p}[bwl]. Architecture like arm does IO access in
memory-mapped way, as because they donot support direct IO instructions. So
introducing a helper api for arm/arm64 who'll provide ioport access in
x86-style.

Also adding support for arm/arm64 in virtio_pci.h header file.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
 drivers/net/virtio/virtio_pci.h                    |   15 ++
 .../common/include/arch/arm/rte_isa_io.h           |  212 ++++++++++++++++++++
 2 files changed, 227 insertions(+)
 create mode 100644 lib/librte_eal/common/include/arch/arm/rte_isa_io.h

diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 47f722a..72f5f6a 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -40,9 +40,15 @@
 #include <sys/types.h>
 #include <machine/cpufunc.h>
 #else
+
+#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+#include <rte_isa_io.h>
+#else /* !ARM64 , !ARM  */
 #include <sys/io.h>
 #endif
 
+#endif
+
 #include <rte_ethdev.h>
 
 struct virtqueue;
@@ -165,7 +171,11 @@ struct virtqueue;
 
 struct virtio_hw {
 	struct virtqueue *cvq;
+#if defined(RTE_ARCH_ARM64)
+	uint64_t    io_base;
+#else /* !ARM64 */
 	uint32_t    io_base;
+#endif
 	uint32_t    guest_features;
 	uint32_t    max_tx_queues;
 	uint32_t    max_rx_queues;
@@ -226,8 +236,13 @@ outl_p(unsigned int data, unsigned int port)
 }
 #endif
 
+#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+#define VIRTIO_PCI_REG_ADDR(hw, reg) \
+	(unsigned long)((hw)->io_base + (reg))
+#else /* !ARM , !ARM64 */
 #define VIRTIO_PCI_REG_ADDR(hw, reg) \
 	(unsigned short)((hw)->io_base + (reg))
+#endif
 
 #define VIRTIO_READ_REG_1(hw, reg) \
 	inb((VIRTIO_PCI_REG_ADDR((hw), (reg))))
diff --git a/lib/librte_eal/common/include/arch/arm/rte_isa_io.h b/lib/librte_eal/common/include/arch/arm/rte_isa_io.h
new file mode 100644
index 0000000..14f806e
--- /dev/null
+++ b/lib/librte_eal/common/include/arch/arm/rte_isa_io.h
@@ -0,0 +1,212 @@
+/*-
+ *  BSD LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ *   Copyright(c) 2010-2015 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *  ARM helper api to emulate x86-style of {in , out}[bwl] api used for
+ *  accessing PCI/ISA IO address space.
+ *
+ *   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.
+ */
+
+/*
+ * @File
+ * Currently virtio pci does IO access in x86-way i.e. IO_RESOURCE_IO way, It
+ * access the pci address space by port_number. The ARM doesn't have
+ * instructions for direct IO access. In ARM: IO's are memory mapped.
+ *
+ * Below helper api allow virtio_pci driver to access IO's for arm/arm64 arch
+ * in x86-style of apis example: {in , out}[bwl] and {in_p , out_p}[bwl].
+ */
+#ifndef _RTE_ISA_IO_H_
+#define _RTE_ISA_IO_H_
+
+#include <stdint.h>
+#include <inttypes.h>
+
+#if defined(RTE_ARCH_ARM)
+/*
+ * Generic IO read/write api for arm: Refer TRM
+ */
+static inline void raw_writeb(uint8_t val, uint32_t addr)
+{
+	asm volatile("strb %0, [%1]" : : "r" (val), "r" (addr));
+}
+
+static inline void raw_writew(uint16_t val, uint32_t addr)
+{
+	asm volatile("strh %0, [%1]" : : "r" (val), "r" (addr));
+}
+
+static inline void raw_writel(uint32_t val, uint32_t addr)
+{
+	asm volatile("str %0, [%1]" : : "r" (val), "r" (addr));
+}
+
+static inline uint8_t raw_readb(uint32_t addr)
+{
+	uint8_t val;
+	asm volatile("ldrb %0, [%1]" : "=r" (val) : "r" (addr));
+	return val;
+}
+
+static inline uint16_t raw_readw(uint32_t addr)
+{
+	uint16_t val;
+	asm volatile("ldrh %0, [%1]" : "=r" (val) : "r" (addr));
+	return val;
+}
+
+static inline uint32_t raw_readl(uint32_t addr)
+{
+	uint32_t val;
+	asm volatile("ldr %0, [%1]" : "=r" (val) : "r" (addr));
+	return val;
+}
+
+#elif defined(RTE_ARCH_ARM64)
+
+/*
+ * Generic IO read/write api for arm64: Refer TRM
+ */
+static inline void raw_writeb(uint8_t val, uint64_t addr)
+{
+	asm volatile("strb %w0, [%1]" : : "r" (val), "r" (addr));
+}
+
+static inline void raw_writew(uint16_t val, uint64_t addr)
+{
+	asm volatile("strh %w0, [%1]" : : "r" (val), "r" (addr));
+}
+
+static inline void raw_writel(uint32_t val, uint64_t addr)
+{
+	asm volatile("str %w0, [%1]" : : "r" (val), "r" (addr));
+}
+
+static inline uint8_t raw_readb(uint64_t addr)
+{
+	uint8_t val;
+	asm volatile("ldrb %w0, [%1]" : "=r" (val) : "r" (addr));
+	return val;
+}
+
+static inline uint16_t raw_readw(uint64_t addr)
+{
+	uint16_t val;
+	asm volatile("ldrh %w0, [%1]" : "=r" (val) : "r" (addr));
+	return val;
+}
+
+static inline uint32_t raw_readl(uint64_t addr)
+{
+	uint32_t val;
+	asm volatile("ldr %w0, [%1]" : "=r" (val) : "r" (addr));
+	return val;
+}
+#else /* !ARM64 && !ARM */
+
+#define raw_writeb(val, addr)
+#define raw_writew(val, addr)
+#define raw_writel(val, addr)
+#define raw_readb(addr)
+#define raw_readw(addr)
+#define raw_readl(addr)
+
+#endif /* ARM64 or ARM */
+
+/* Emulate x86-style of ioport api implementation for arm/arm64. Included API
+ * - {in, out}{b, w, l}()
+ * - {in_p, out_p} {b, w, l} ()
+ *
+ * */
+
+static inline uint8_t inb(unsigned long addr)
+{
+	return raw_readb(addr);
+}
+
+static inline uint16_t inw(unsigned long addr)
+{
+	return raw_readw(addr);
+}
+
+static inline uint32_t inl(unsigned long addr)
+{
+	return raw_readl(addr);
+}
+
+static inline void outb(uint8_t value, unsigned long addr)
+{
+	raw_writeb(value, addr);
+}
+
+static inline void outw(uint16_t value, unsigned long addr)
+{
+	raw_writew(value, addr);
+}
+
+static inline void outl(uint32_t value, unsigned long addr)
+{
+	raw_writel(value, addr);
+}
+
+static inline uint8_t inb_p(unsigned long addr)
+{
+	return inb(addr);
+}
+
+static inline uint16_t inw_p(unsigned long addr)
+{
+	return inw(addr);
+}
+
+static inline uint32_t inl_p(unsigned long addr)
+{
+	return inl(addr);
+}
+
+static inline void outb_p(uint8_t value, unsigned long addr)
+{
+	outb(value, addr);
+}
+
+static inline void outw_p(uint16_t value, unsigned long addr)
+{
+	outw(value, addr);
+}
+
+static inline void outl_p(uint32_t value, unsigned long addr)
+{
+	outl(value, addr);
+}
+
+#endif /* _RTE_ISA_IO_H_ */
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 4/6] config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD
  2015-12-04 17:35 [PATCH 0/6] Add virtio support in arm/arm64 Santosh Shukla
                   ` (2 preceding siblings ...)
  2015-12-04 17:35 ` [PATCH 3/6] virtio: armv7/v8: Introdice api to emulate x86-style of PCI/ISA ioport access Santosh Shukla
@ 2015-12-04 17:35 ` Santosh Shukla
  2015-12-04 17:35 ` [PATCH 5/6] linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init() Santosh Shukla
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 22+ messages in thread
From: Santosh Shukla @ 2015-12-04 17:35 UTC (permalink / raw)
  To: dev

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

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
 config/defconfig_arm-armv7a-linuxapp-gcc   |    6 +++++-
 config/defconfig_arm64-armv8a-linuxapp-gcc |    6 +++++-
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/config/defconfig_arm-armv7a-linuxapp-gcc b/config/defconfig_arm-armv7a-linuxapp-gcc
index 9924ff9..5bdff30 100644
--- a/config/defconfig_arm-armv7a-linuxapp-gcc
+++ b/config/defconfig_arm-armv7a-linuxapp-gcc
@@ -43,6 +43,11 @@ CONFIG_RTE_FORCE_INTRINSICS=y
 CONFIG_RTE_TOOLCHAIN="gcc"
 CONFIG_RTE_TOOLCHAIN_GCC=y
 
+# VIRTIO support for ARM
+CONFIG_RTE_LIBRTE_VIRTIO_PMD=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
 
@@ -71,7 +76,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..b3a4b28 100644
--- a/config/defconfig_arm64-armv8a-linuxapp-gcc
+++ b/config/defconfig_arm64-armv8a-linuxapp-gcc
@@ -45,8 +45,12 @@ CONFIG_RTE_TOOLCHAIN_GCC=y
 
 CONFIG_RTE_CACHE_LINE_SIZE=64
 
+# Enable VIRTIO support for ARM
+CONFIG_RTE_LIBRTE_VIRTIO_PMD=y
+# Disable VIRTIO VECTOR support
+CONFIG_RTE_VIRTIO_INC_VECTOR=n
+
 CONFIG_RTE_IXGBE_INC_VECTOR=n
-CONFIG_RTE_LIBRTE_VIRTIO_PMD=n
 CONFIG_RTE_LIBRTE_IVSHMEM=n
 CONFIG_RTE_LIBRTE_FM10K_PMD=n
 CONFIG_RTE_LIBRTE_I40E_PMD=n
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 5/6] linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init()
  2015-12-04 17:35 [PATCH 0/6] Add virtio support in arm/arm64 Santosh Shukla
                   ` (3 preceding siblings ...)
  2015-12-04 17:35 ` [PATCH 4/6] config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD Santosh Shukla
@ 2015-12-04 17:35 ` Santosh Shukla
  2015-12-09 19:58   ` Jan Viktorin
  2015-12-04 17:35 ` [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver Santosh Shukla
  2015-12-07  2:12 ` [PATCH 0/6] Add virtio support in arm/arm64 Yuanhan Liu
  6 siblings, 1 reply; 22+ messages in thread
From: Santosh Shukla @ 2015-12-04 17:35 UTC (permalink / raw)
  To: dev

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

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
---
 lib/librte_eal/linuxapp/eal/eal.c |    3 +++
 1 file changed, 3 insertions(+)

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

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver
  2015-12-04 17:35 [PATCH 0/6] Add virtio support in arm/arm64 Santosh Shukla
                   ` (4 preceding siblings ...)
  2015-12-04 17:35 ` [PATCH 5/6] linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init() Santosh Shukla
@ 2015-12-04 17:35 ` Santosh Shukla
  2015-12-07 17:08   ` Stephen Hemminger
  2015-12-08  9:47   ` Ananyev, Konstantin
  2015-12-07  2:12 ` [PATCH 0/6] Add virtio support in arm/arm64 Yuanhan Liu
  6 siblings, 2 replies; 22+ messages in thread
From: Santosh Shukla @ 2015-12-04 17:35 UTC (permalink / raw)
  To: dev; +Cc: Rizwan Ansari

This patch set add memory-mapped-IO support for arm/arm64 archs in virtio-pmd
driver. Patch creates ioport-mem device name /dev/igb_ioport. virtio_ethdev_init
function to open that device file for once, mappes 4K page_size memory such that
all entry in cat /proc/ioport {all PCI_IOBAR entry} mapped for once. For that
two ancessor api;s
- virtio_map_ioport : Maps all cat /proc/ioport pci iobars.
- virtio_set_ioport_addr : manages the each pci_iobar slot as an offset.

Tested for thunderX/arm64 platform, by creating maximum guest kernel supported
virtio-net-pci device i.e.. 31 max, then attaching all 32 interface to uio,
Verified with tespmd io_fwd application.

Signed-off-by: Santosh Shukla <sshukla@mvista.com>
Signed-off-by: Rizwan Ansari <ransari@mvista.com>
---
 drivers/net/virtio/virtio_ethdev.c        |  138 ++++++++++++++++++++++++++++-
 lib/librte_eal/linuxapp/igb_uio/igb_uio.c |   80 ++++++++++++++++-
 2 files changed, 214 insertions(+), 4 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 74c00ee..93b8784 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -63,6 +63,30 @@
 #include "virtqueue.h"
 #include "virtio_rxtx.h"
 
+#ifdef RTE_EXEC_ENV_LINUXAPP
+/* start address of first pci_iobar slot (user-space virtual-addres) */
+void *ioport_map;
+#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+
+#include <sys/mman.h>
+#define DEV_NAME		"/dev/igb_ioport"
+
+/* Keeping pci_ioport_size = 4k.
+ * So maximum mmaped pci_iobar supported =
+ * (ioport_size/pci_dev->mem_resource[0].len)
+ *
+ * note: kernel could allow maximum 32 virtio-net-pci interface, that mean
+ * maximum 32 PCI_IOBAR(s) where each PCI_IOBAR_LEN=0x20, so virtio_map_ioport()
+ * func by theory gonna support 4k/0x20 ==> 128 PCI_IOBAR(s), more than
+ * max-virtio-net-pci interface.
+ */
+#define PAGE_SIZE		4096
+#define PCI_IOPORT_SIZE		PAGE_SIZE
+#define PCI_IOPORT_MAX		128 /* 4k / 0x20 */
+
+int ioport_map_cnt;
+#endif /* ARM, ARM64 */
+#endif /* RTE_EXEC_ENV_LINUXAPP */
 
 static int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
 static int eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev);
@@ -497,6 +521,18 @@ virtio_dev_close(struct rte_eth_dev *dev)
 	hw->started = 0;
 	virtio_dev_free_mbufs(dev);
 	virtio_free_queues(dev);
+
+#ifdef RTE_EXEC_ENV_LINUXAPP
+#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+
+	/* unmap ioport memory */
+	ioport_map_cnt--;
+	if (!ioport_map_cnt)
+		munmap(ioport_map, PCI_IOPORT_SIZE);
+
+	PMD_INIT_LOG(DEBUG, "unmapping ioport_mem %d\n", ioport_map_cnt);
+#endif
+#endif
 }
 
 static void
@@ -1172,7 +1208,6 @@ static int virtio_resource_init_by_ioports(struct rte_pci_device *pci_dev)
 
 			sscanf(ptr, "%04hx-%04hx", &start, &end);
 			size = end - start + 1;
-
 			break;
 		}
 	}
@@ -1256,6 +1291,81 @@ rx_func_get(struct rte_eth_dev *eth_dev)
 		eth_dev->rx_pkt_burst = &virtio_recv_pkts;
 }
 
+#ifdef RTE_EXEC_ENV_LINUXAPP
+#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+
+static int
+virtio_map_ioport(void **resource_addr)
+{
+	int fd;
+	int ret = 0;
+
+	fd = open(DEV_NAME, O_RDWR);
+	if (fd < 0) {
+		PMD_INIT_LOG(ERR, "device file %s open error: %d\n",
+			     DEV_NAME, fd);
+		ret = -1;
+		goto out;
+	}
+
+	ioport_map = mmap(NULL, PCI_IOPORT_SIZE,
+			PROT_EXEC | PROT_WRITE | PROT_READ, MAP_SHARED, fd, 0);
+
+	if (ioport_map == MAP_FAILED) {
+		PMD_INIT_LOG(ERR, "mmap: failed to map bar Address=%p\n",
+				*resource_addr);
+		ret = -ENOMEM;
+		goto out1;
+	}
+
+	PMD_INIT_LOG(INFO, "First pci_iobar mapped at %p\n", ioport_map);
+
+out1:
+	close(fd);
+out:
+	return ret;
+}
+
+static int
+virtio_set_ioport_addr(void **resource_addr, unsigned long offset)
+{
+	int ret = 0;
+
+	if (ioport_map_cnt >= PCI_IOPORT_MAX) {
+		ret = -1;
+		PMD_INIT_LOG(ERR,
+			     "ioport_map_cnt(%d) greater than"
+			     "PCI_IOPORT_MAX(%d)\n",
+			     ioport_map_cnt, PCI_IOPORT_MAX);
+		goto out;
+	}
+	*resource_addr = (void *)((char *)ioport_map + (ioport_map_cnt)*offset);
+	ioport_map_cnt++;
+
+	PMD_INIT_LOG(DEBUG, "pci.resource_addr %p ioport_map_cnt %d\n",
+			*resource_addr, ioport_map_cnt);
+out:
+	return ret;
+}
+#else /* !ARM, !ARM64 */
+static int
+virtio_map_ioport(void *resource_addr)
+{
+	(void)resource_addr;
+	return 0;
+}
+
+static int
+virtio_set_ioport_addr(void *resource_addr, unsigned long offset)
+{
+	(void)resource_addr;
+	(void)offset;
+	return 0;
+}
+
+#endif /* ARM, ARM64 */
+#endif /* RTE_EXEC_ENV_LINUXAPP */
+
 /*
  * This function is based on probe() function in virtio_pci.c
  * It returns 0 on success.
@@ -1294,8 +1404,31 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
 	if (virtio_resource_init(pci_dev) < 0)
 		return -1;
 
+#ifdef	RTE_EXEC_ENV_LINUXAPP
+	int ret = 0;
+
+	/* Map the all IOBAR entry from /proc/ioport to 4k page_size only once.
+	 * Later virtio_set_ioport_addr() func will update correct bar_addr
+	 * eachk ioport (i.e..pci_dev->mem_resource[0].addr)
+	 */
+	if (!ioport_map) {
+		ret = virtio_map_ioport(&pci_dev->mem_resource[0].addr);
+		if (ret < 0)
+			return -1;
+	}
+
+	ret = virtio_set_ioport_addr(&pci_dev->mem_resource[0].addr,
+					pci_dev->mem_resource[0].len);
+	if (ret < 0)
+		return -1;
+
+	PMD_INIT_LOG(INFO, "ioport_map %p resource_addr %p resource_len :%ld\n",
+			ioport_map, pci_dev->mem_resource[0].addr,
+			(unsigned long)pci_dev->mem_resource[0].len);
+
+#endif
 	hw->use_msix = virtio_has_msix(&pci_dev->addr);
-	hw->io_base = (uint32_t)(uintptr_t)pci_dev->mem_resource[0].addr;
+	hw->io_base = (unsigned long)(uintptr_t)pci_dev->mem_resource[0].addr;
 
 	/* Reset the device although not necessary at startup */
 	vtpci_reset(hw);
@@ -1430,7 +1563,6 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
 		rte_intr_callback_unregister(&pci_dev->intr_handle,
 						virtio_interrupt_handler,
 						eth_dev);
-
 	PMD_INIT_LOG(DEBUG, "dev_uninit completed");
 
 	return 0;
diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index f5617d2..b154a44 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -324,6 +324,61 @@ igbuio_dom0_pci_mmap(struct uio_info *info, struct vm_area_struct *vma)
 }
 #endif
 
+#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+#ifdef CONFIG_HAS_IOPORT_MAP
+/*
+ * mmap driver to map x86-style PCI_IOBAR (i.e..cat /proc/ioport pci-bar-memory)
+ * from kernel-space virtual address to user-space virtual address. This module
+ * required for non-x86 archs example arm/arm64, as those archs donot do
+ * IO_MAP_IO types access, Infact supports memory-mapped-IO. That is because
+ * arm/arm64 doesn't support direct IO instruction, so the design approach is to
+ * map `cat /proc/ioport` PCI_IOBAR's kernel-space virtual-addr to user-space
+ * virtual-addr. Therefore the need for mmap-driver.
+ */
+#include <linux/fs.h>		/* file_operations */
+#include <linux/miscdevice.h>
+#include <linux/mm.h>		/* VM_IO */
+#include <linux/uaccess.h>
+#include <asm/page.h>
+#include <linux/sched.h>
+#include <linux/pid.h>
+
+void *__iomem mapped_io; /* ioport addr of `cat /proc/ioport` */
+
+static int igb_ioport_mmap(struct file *file, struct vm_area_struct *vma)
+{
+	struct page *npage;
+	int ret = 0;
+
+	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
+	npage = vmalloc_to_page(mapped_io);
+	ret = remap_pfn_range(vma, vma->vm_start,
+				page_to_pfn(npage),
+				vma->vm_end - vma->vm_start,
+				vma->vm_page_prot);
+	if (ret) {
+		pr_info("Error: Failed to remap pfn=%lu error=%d\n",
+			page_to_pfn(npage), ret);
+	}
+	return 0;
+}
+
+static const struct file_operations igb_ioport_fops = {
+	.mmap		= igb_ioport_mmap,
+};
+
+static struct miscdevice igb_ioport_dev = {
+	.minor	= MISC_DYNAMIC_MINOR,
+	.name	= "igb_ioport",
+	.fops	= &igb_ioport_fops
+};
+#else /* !CONFIG_HAS_IOPORT_MAP */
+
+#error "CONFIG_HAS_IOPORT_MAP not supported for $RTE_ARCH"
+
+#endif /* CONFIG_HAS_IOPORT_MAP */
+#endif /* RTE_ARCH_ARM, RTE_ARCH_ARM64 */
+
 /* Remap pci resources described by bar #pci_bar in uio resource n. */
 static int
 igbuio_pci_setup_iomem(struct pci_dev *dev, struct uio_info *info,
@@ -365,11 +420,22 @@ igbuio_pci_setup_ioport(struct pci_dev *dev, struct uio_info *info,
 	if (addr == 0 || len == 0)
 		return -EINVAL;
 
+#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+	/*
+	 * TODO: KNI/procfs:: porttype entry need to be added for ARM/ARM64,
+	 * for below mmap driver;
+	 * */
+	mapped_io = ioport_map(addr, 0);
+	info->port[n].name = name;
+	info->port[n].start = (unsigned long)(uintptr_t)mapped_io;
+	info->port[n].size = len;
+	info->port[n].porttype = UIO_PORT_X86;
+#else
 	info->port[n].name = name;
 	info->port[n].start = addr;
 	info->port[n].size = len;
 	info->port[n].porttype = UIO_PORT_X86;
-
+#endif
 	return 0;
 }
 
@@ -615,6 +681,15 @@ igbuio_pci_init_module(void)
 {
 	int ret;
 
+#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+	ret = misc_register(&igb_ioport_dev);
+	if (ret < 0) {
+		pr_info("Error: failed to register ioport map driver (%d)\n",
+				ret);
+		return ret;
+	}
+#endif
+
 	ret = igbuio_config_intr_mode(intr_mode);
 	if (ret < 0)
 		return ret;
@@ -625,6 +700,9 @@ igbuio_pci_init_module(void)
 static void __exit
 igbuio_pci_exit_module(void)
 {
+#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
+	misc_deregister(&igb_ioport_dev);
+#endif
 	pci_unregister_driver(&igbuio_pci_driver);
 }
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] Add virtio support in arm/arm64
  2015-12-04 17:35 [PATCH 0/6] Add virtio support in arm/arm64 Santosh Shukla
                   ` (5 preceding siblings ...)
  2015-12-04 17:35 ` [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver Santosh Shukla
@ 2015-12-07  2:12 ` Yuanhan Liu
  2015-12-08 12:59   ` Xie, Huawei
  6 siblings, 1 reply; 22+ messages in thread
From: Yuanhan Liu @ 2015-12-07  2:12 UTC (permalink / raw)
  To: Santosh Shukla; +Cc: dev

On Fri, Dec 04, 2015 at 11:05:13PM +0530, Santosh Shukla wrote:
> This patch set add basic infrastrucure to run virtio-net-pci pmd driver for
> arm64/arm. Tested on ThunderX platfrom. Verified for existing dpdk(s) test
> applications like:
> - ovs-dpdk-vhost-user: across the VM's, for the use-cases like guest2guest and
>   Host2Guest
> - testpmd application: Tested for max virtio-net-pci interface currently
>   supported in kernel i.e. 31 interface. 


Hi Santosh,

Here is a quick notice that I don't have time to review your patches
this one or two weeks. Sorry for that.

(Not quite sure Huawei has the bandwidth or not, btw)

	--yliu

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver
  2015-12-04 17:35 ` [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver Santosh Shukla
@ 2015-12-07 17:08   ` Stephen Hemminger
  2015-12-08 12:53     ` Santosh Shukla
  2015-12-08  9:47   ` Ananyev, Konstantin
  1 sibling, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2015-12-07 17:08 UTC (permalink / raw)
  To: Santosh Shukla; +Cc: dev, Rizwan Ansari

On Fri,  4 Dec 2015 23:05:19 +0530
Santosh Shukla <sshukla@mvista.com> wrote:

>  
> +#ifdef RTE_EXEC_ENV_LINUXAPP
> +/* start address of first pci_iobar slot (user-space virtual-addres) */
> +void *ioport_map;
> +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> +
> +#include <sys/mman.h>
> +#define DEV_NAME		"/dev/igb_ioport"
> +
> +/* Keeping pci_ioport_size = 4k.
> + * So maximum mmaped pci_iobar supported =
> + * (ioport_size/pci_dev->mem_resource[0].len)
> + *
> + * note: kernel could allow maximum 32 virtio-net-pci interface, that mean
> + * maximum 32 PCI_IOBAR(s) where each PCI_IOBAR_LEN=0x20, so virtio_map_ioport()
> + * func by theory gonna support 4k/0x20 ==> 128 PCI_IOBAR(s), more than
> + * max-virtio-net-pci interface.
> + */
> +#define PAGE_SIZE		4096
> +#define PCI_IOPORT_SIZE		PAGE_SIZE
> +#define PCI_IOPORT_MAX		128 /* 4k / 0x20 */
> +
> +int ioport_map_cnt;
> +#endif /* ARM, ARM64 */
> +#endif /* RTE_EXEC_ENV_LINUXAPP */

These variables should be static.

Also, it is should be possible to extract the I/O bar stuff in a generic way through sysfs
and not depend on a character device. The long term goal for DPDK acceptance is to
eliminate (or at least reduce to a minumum) any requirement for special kernel drivers.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/6] virtio: armv7/v8: Introdice api to emulate x86-style of PCI/ISA ioport access
  2015-12-04 17:35 ` [PATCH 3/6] virtio: armv7/v8: Introdice api to emulate x86-style of PCI/ISA ioport access Santosh Shukla
@ 2015-12-07 17:09   ` Stephen Hemminger
  2015-12-08 15:35     ` Santosh Shukla
  0 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2015-12-07 17:09 UTC (permalink / raw)
  To: Santosh Shukla; +Cc: dev

On Fri,  4 Dec 2015 23:05:16 +0530
Santosh Shukla <sshukla@mvista.com> wrote:

> +#if defined(RTE_ARCH_ARM64)
> +	uint64_t    io_base;
> +#else /* !ARM64 */
>  	uint32_t    io_base;
> +#endif

Is there a typedef that could be used instead of having #ifdef clutter?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver
  2015-12-04 17:35 ` [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver Santosh Shukla
  2015-12-07 17:08   ` Stephen Hemminger
@ 2015-12-08  9:47   ` Ananyev, Konstantin
  2015-12-08 12:55     ` Santosh Shukla
  1 sibling, 1 reply; 22+ messages in thread
From: Ananyev, Konstantin @ 2015-12-08  9:47 UTC (permalink / raw)
  To: Santosh Shukla, dev; +Cc: Rizwan Ansari

Hi,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Santosh Shukla
> Sent: Friday, December 04, 2015 5:35 PM
> To: dev@dpdk.org
> Cc: Rizwan Ansari
> Subject: [dpdk-dev] [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver
> 
> This patch set add memory-mapped-IO support for arm/arm64 archs in virtio-pmd
> driver. Patch creates ioport-mem device name /dev/igb_ioport. virtio_ethdev_init
> function to open that device file for once, mappes 4K page_size memory such that
> all entry in cat /proc/ioport {all PCI_IOBAR entry} mapped for once. For that
> two ancessor api;s
> - virtio_map_ioport : Maps all cat /proc/ioport pci iobars.
> - virtio_set_ioport_addr : manages the each pci_iobar slot as an offset.
> 
> Tested for thunderX/arm64 platform, by creating maximum guest kernel supported
> virtio-net-pci device i.e.. 31 max, then attaching all 32 interface to uio,
> Verified with tespmd io_fwd application.
> 
> Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> Signed-off-by: Rizwan Ansari <ransari@mvista.com>
> ---

Is it possible to rework patch a bit to minimise number of #ifdef ARM ... 
spread across the code? 
Group arch related code into separate file(s), use union for fields with sizes, etc?
Konstantin  

>  drivers/net/virtio/virtio_ethdev.c        |  138 ++++++++++++++++++++++++++++-
>  lib/librte_eal/linuxapp/igb_uio/igb_uio.c |   80 ++++++++++++++++-
>  2 files changed, 214 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index 74c00ee..93b8784 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -63,6 +63,30 @@
>  #include "virtqueue.h"
>  #include "virtio_rxtx.h"
> 
> +#ifdef RTE_EXEC_ENV_LINUXAPP
> +/* start address of first pci_iobar slot (user-space virtual-addres) */
> +void *ioport_map;
> +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> +
> +#include <sys/mman.h>
> +#define DEV_NAME		"/dev/igb_ioport"
> +
> +/* Keeping pci_ioport_size = 4k.
> + * So maximum mmaped pci_iobar supported =
> + * (ioport_size/pci_dev->mem_resource[0].len)
> + *
> + * note: kernel could allow maximum 32 virtio-net-pci interface, that mean
> + * maximum 32 PCI_IOBAR(s) where each PCI_IOBAR_LEN=0x20, so virtio_map_ioport()
> + * func by theory gonna support 4k/0x20 ==> 128 PCI_IOBAR(s), more than
> + * max-virtio-net-pci interface.
> + */
> +#define PAGE_SIZE		4096
> +#define PCI_IOPORT_SIZE		PAGE_SIZE
> +#define PCI_IOPORT_MAX		128 /* 4k / 0x20 */
> +
> +int ioport_map_cnt;
> +#endif /* ARM, ARM64 */
> +#endif /* RTE_EXEC_ENV_LINUXAPP */
> 
>  static int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
>  static int eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev);
> @@ -497,6 +521,18 @@ virtio_dev_close(struct rte_eth_dev *dev)
>  	hw->started = 0;
>  	virtio_dev_free_mbufs(dev);
>  	virtio_free_queues(dev);
> +
> +#ifdef RTE_EXEC_ENV_LINUXAPP
> +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> +
> +	/* unmap ioport memory */
> +	ioport_map_cnt--;
> +	if (!ioport_map_cnt)
> +		munmap(ioport_map, PCI_IOPORT_SIZE);
> +
> +	PMD_INIT_LOG(DEBUG, "unmapping ioport_mem %d\n", ioport_map_cnt);
> +#endif
> +#endif
>  }
> 
>  static void
> @@ -1172,7 +1208,6 @@ static int virtio_resource_init_by_ioports(struct rte_pci_device *pci_dev)
> 
>  			sscanf(ptr, "%04hx-%04hx", &start, &end);
>  			size = end - start + 1;
> -
>  			break;
>  		}
>  	}
> @@ -1256,6 +1291,81 @@ rx_func_get(struct rte_eth_dev *eth_dev)
>  		eth_dev->rx_pkt_burst = &virtio_recv_pkts;
>  }
> 
> +#ifdef RTE_EXEC_ENV_LINUXAPP
> +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> +
> +static int
> +virtio_map_ioport(void **resource_addr)
> +{
> +	int fd;
> +	int ret = 0;
> +
> +	fd = open(DEV_NAME, O_RDWR);
> +	if (fd < 0) {
> +		PMD_INIT_LOG(ERR, "device file %s open error: %d\n",
> +			     DEV_NAME, fd);
> +		ret = -1;
> +		goto out;
> +	}
> +
> +	ioport_map = mmap(NULL, PCI_IOPORT_SIZE,
> +			PROT_EXEC | PROT_WRITE | PROT_READ, MAP_SHARED, fd, 0);
> +
> +	if (ioport_map == MAP_FAILED) {
> +		PMD_INIT_LOG(ERR, "mmap: failed to map bar Address=%p\n",
> +				*resource_addr);
> +		ret = -ENOMEM;
> +		goto out1;
> +	}
> +
> +	PMD_INIT_LOG(INFO, "First pci_iobar mapped at %p\n", ioport_map);
> +
> +out1:
> +	close(fd);
> +out:
> +	return ret;
> +}
> +
> +static int
> +virtio_set_ioport_addr(void **resource_addr, unsigned long offset)
> +{
> +	int ret = 0;
> +
> +	if (ioport_map_cnt >= PCI_IOPORT_MAX) {
> +		ret = -1;
> +		PMD_INIT_LOG(ERR,
> +			     "ioport_map_cnt(%d) greater than"
> +			     "PCI_IOPORT_MAX(%d)\n",
> +			     ioport_map_cnt, PCI_IOPORT_MAX);
> +		goto out;
> +	}
> +	*resource_addr = (void *)((char *)ioport_map + (ioport_map_cnt)*offset);
> +	ioport_map_cnt++;
> +
> +	PMD_INIT_LOG(DEBUG, "pci.resource_addr %p ioport_map_cnt %d\n",
> +			*resource_addr, ioport_map_cnt);
> +out:
> +	return ret;
> +}
> +#else /* !ARM, !ARM64 */
> +static int
> +virtio_map_ioport(void *resource_addr)
> +{
> +	(void)resource_addr;
> +	return 0;
> +}
> +
> +static int
> +virtio_set_ioport_addr(void *resource_addr, unsigned long offset)
> +{
> +	(void)resource_addr;
> +	(void)offset;
> +	return 0;
> +}
> +
> +#endif /* ARM, ARM64 */
> +#endif /* RTE_EXEC_ENV_LINUXAPP */
> +
>  /*
>   * This function is based on probe() function in virtio_pci.c
>   * It returns 0 on success.
> @@ -1294,8 +1404,31 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
>  	if (virtio_resource_init(pci_dev) < 0)
>  		return -1;
> 
> +#ifdef	RTE_EXEC_ENV_LINUXAPP
> +	int ret = 0;
> +
> +	/* Map the all IOBAR entry from /proc/ioport to 4k page_size only once.
> +	 * Later virtio_set_ioport_addr() func will update correct bar_addr
> +	 * eachk ioport (i.e..pci_dev->mem_resource[0].addr)
> +	 */
> +	if (!ioport_map) {
> +		ret = virtio_map_ioport(&pci_dev->mem_resource[0].addr);
> +		if (ret < 0)
> +			return -1;
> +	}
> +
> +	ret = virtio_set_ioport_addr(&pci_dev->mem_resource[0].addr,
> +					pci_dev->mem_resource[0].len);
> +	if (ret < 0)
> +		return -1;
> +
> +	PMD_INIT_LOG(INFO, "ioport_map %p resource_addr %p resource_len :%ld\n",
> +			ioport_map, pci_dev->mem_resource[0].addr,
> +			(unsigned long)pci_dev->mem_resource[0].len);
> +
> +#endif
>  	hw->use_msix = virtio_has_msix(&pci_dev->addr);
> -	hw->io_base = (uint32_t)(uintptr_t)pci_dev->mem_resource[0].addr;
> +	hw->io_base = (unsigned long)(uintptr_t)pci_dev->mem_resource[0].addr;
> 
>  	/* Reset the device although not necessary at startup */
>  	vtpci_reset(hw);
> @@ -1430,7 +1563,6 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
>  		rte_intr_callback_unregister(&pci_dev->intr_handle,
>  						virtio_interrupt_handler,
>  						eth_dev);
> -
>  	PMD_INIT_LOG(DEBUG, "dev_uninit completed");
> 
>  	return 0;
> diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> index f5617d2..b154a44 100644
> --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> @@ -324,6 +324,61 @@ igbuio_dom0_pci_mmap(struct uio_info *info, struct vm_area_struct *vma)
>  }
>  #endif
> 
> +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> +#ifdef CONFIG_HAS_IOPORT_MAP
> +/*
> + * mmap driver to map x86-style PCI_IOBAR (i.e..cat /proc/ioport pci-bar-memory)
> + * from kernel-space virtual address to user-space virtual address. This module
> + * required for non-x86 archs example arm/arm64, as those archs donot do
> + * IO_MAP_IO types access, Infact supports memory-mapped-IO. That is because
> + * arm/arm64 doesn't support direct IO instruction, so the design approach is to
> + * map `cat /proc/ioport` PCI_IOBAR's kernel-space virtual-addr to user-space
> + * virtual-addr. Therefore the need for mmap-driver.
> + */
> +#include <linux/fs.h>		/* file_operations */
> +#include <linux/miscdevice.h>
> +#include <linux/mm.h>		/* VM_IO */
> +#include <linux/uaccess.h>
> +#include <asm/page.h>
> +#include <linux/sched.h>
> +#include <linux/pid.h>
> +
> +void *__iomem mapped_io; /* ioport addr of `cat /proc/ioport` */
> +
> +static int igb_ioport_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> +	struct page *npage;
> +	int ret = 0;
> +
> +	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> +	npage = vmalloc_to_page(mapped_io);
> +	ret = remap_pfn_range(vma, vma->vm_start,
> +				page_to_pfn(npage),
> +				vma->vm_end - vma->vm_start,
> +				vma->vm_page_prot);
> +	if (ret) {
> +		pr_info("Error: Failed to remap pfn=%lu error=%d\n",
> +			page_to_pfn(npage), ret);
> +	}
> +	return 0;
> +}
> +
> +static const struct file_operations igb_ioport_fops = {
> +	.mmap		= igb_ioport_mmap,
> +};
> +
> +static struct miscdevice igb_ioport_dev = {
> +	.minor	= MISC_DYNAMIC_MINOR,
> +	.name	= "igb_ioport",
> +	.fops	= &igb_ioport_fops
> +};
> +#else /* !CONFIG_HAS_IOPORT_MAP */
> +
> +#error "CONFIG_HAS_IOPORT_MAP not supported for $RTE_ARCH"
> +
> +#endif /* CONFIG_HAS_IOPORT_MAP */
> +#endif /* RTE_ARCH_ARM, RTE_ARCH_ARM64 */
> +
>  /* Remap pci resources described by bar #pci_bar in uio resource n. */
>  static int
>  igbuio_pci_setup_iomem(struct pci_dev *dev, struct uio_info *info,
> @@ -365,11 +420,22 @@ igbuio_pci_setup_ioport(struct pci_dev *dev, struct uio_info *info,
>  	if (addr == 0 || len == 0)
>  		return -EINVAL;
> 
> +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> +	/*
> +	 * TODO: KNI/procfs:: porttype entry need to be added for ARM/ARM64,
> +	 * for below mmap driver;
> +	 * */
> +	mapped_io = ioport_map(addr, 0);
> +	info->port[n].name = name;
> +	info->port[n].start = (unsigned long)(uintptr_t)mapped_io;
> +	info->port[n].size = len;
> +	info->port[n].porttype = UIO_PORT_X86;
> +#else
>  	info->port[n].name = name;
>  	info->port[n].start = addr;
>  	info->port[n].size = len;
>  	info->port[n].porttype = UIO_PORT_X86;
> -
> +#endif
>  	return 0;
>  }
> 
> @@ -615,6 +681,15 @@ igbuio_pci_init_module(void)
>  {
>  	int ret;
> 
> +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> +	ret = misc_register(&igb_ioport_dev);
> +	if (ret < 0) {
> +		pr_info("Error: failed to register ioport map driver (%d)\n",
> +				ret);
> +		return ret;
> +	}
> +#endif
> +
>  	ret = igbuio_config_intr_mode(intr_mode);
>  	if (ret < 0)
>  		return ret;
> @@ -625,6 +700,9 @@ igbuio_pci_init_module(void)
>  static void __exit
>  igbuio_pci_exit_module(void)
>  {
> +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> +	misc_deregister(&igb_ioport_dev);
> +#endif
>  	pci_unregister_driver(&igbuio_pci_driver);
>  }
> 
> --
> 1.7.9.5

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver
  2015-12-07 17:08   ` Stephen Hemminger
@ 2015-12-08 12:53     ` Santosh Shukla
  2015-12-09 18:59       ` Santosh Shukla
  0 siblings, 1 reply; 22+ messages in thread
From: Santosh Shukla @ 2015-12-08 12:53 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Rizwan Ansari

On Mon, Dec 7, 2015 at 10:38 PM, Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Fri,  4 Dec 2015 23:05:19 +0530
> Santosh Shukla <sshukla@mvista.com> wrote:
>
> >
> > +#ifdef RTE_EXEC_ENV_LINUXAPP
> > +/* start address of first pci_iobar slot (user-space virtual-addres) */
> > +void *ioport_map;
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +
> > +#include <sys/mman.h>
> > +#define DEV_NAME             "/dev/igb_ioport"
> > +
> > +/* Keeping pci_ioport_size = 4k.
> > + * So maximum mmaped pci_iobar supported =
> > + * (ioport_size/pci_dev->mem_resource[0].len)
> > + *
> > + * note: kernel could allow maximum 32 virtio-net-pci interface, that
> mean
> > + * maximum 32 PCI_IOBAR(s) where each PCI_IOBAR_LEN=0x20, so
> virtio_map_ioport()
> > + * func by theory gonna support 4k/0x20 ==> 128 PCI_IOBAR(s), more than
> > + * max-virtio-net-pci interface.
> > + */
> > +#define PAGE_SIZE            4096
> > +#define PCI_IOPORT_SIZE              PAGE_SIZE
> > +#define PCI_IOPORT_MAX               128 /* 4k / 0x20 */
> > +
> > +int ioport_map_cnt;
> > +#endif /* ARM, ARM64 */
> > +#endif /* RTE_EXEC_ENV_LINUXAPP */
>
> These variables should be static.
>
>
(Sorry for delayed follow, Was travelling..)
Right,


> Also, it is should be possible to extract the I/O bar stuff in a generic
> way through sysfs
> and not depend on a character device. The long term goal for DPDK
> acceptance is to
> eliminate (or at least reduce to a minumum) any requirement for special
> kernel drivers.
>

I agree. Existing implementation does read pci_iobar for start address and
size, But for non-x86 arch, we need someway to map pci_iobar and thats-why
thought of adding device file for a purpose, as archs like arm lack iopl()
privileged io syscall support, However iopl() too quite native driver
design assumption.

I have few idea in my mind such that - Right now I am updating
ioport_mapped addr {kernel-virtual-addr-io-memory} to
/sys/bus/pci/pci_bus_xxxx/xxx/map field, instead of mapping their, I'll try
to map to uio's pci interface and then use existing pci_map_resource() api
to mmap kernel-virtual-io-address to user-space-virtual-ioaddr. We'll come
back on this.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver
  2015-12-08  9:47   ` Ananyev, Konstantin
@ 2015-12-08 12:55     ` Santosh Shukla
  0 siblings, 0 replies; 22+ messages in thread
From: Santosh Shukla @ 2015-12-08 12:55 UTC (permalink / raw)
  To: Ananyev, Konstantin; +Cc: dev, Rizwan Ansari

On Tue, Dec 8, 2015 at 3:17 PM, Ananyev, Konstantin <
konstantin.ananyev@intel.com> wrote:

> Hi,
>
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Santosh Shukla
> > Sent: Friday, December 04, 2015 5:35 PM
> > To: dev@dpdk.org
> > Cc: Rizwan Ansari
> > Subject: [dpdk-dev] [PATCH 6/6] virtio: arm/arm64: memory mapped IO
> support in pmd driver
> >
> > This patch set add memory-mapped-IO support for arm/arm64 archs in
> virtio-pmd
> > driver. Patch creates ioport-mem device name /dev/igb_ioport.
> virtio_ethdev_init
> > function to open that device file for once, mappes 4K page_size memory
> such that
> > all entry in cat /proc/ioport {all PCI_IOBAR entry} mapped for once. For
> that
> > two ancessor api;s
> > - virtio_map_ioport : Maps all cat /proc/ioport pci iobars.
> > - virtio_set_ioport_addr : manages the each pci_iobar slot as an offset.
> >
> > Tested for thunderX/arm64 platform, by creating maximum guest kernel
> supported
> > virtio-net-pci device i.e.. 31 max, then attaching all 32 interface to
> uio,
> > Verified with tespmd io_fwd application.
> >
> > Signed-off-by: Santosh Shukla <sshukla@mvista.com>
> > Signed-off-by: Rizwan Ansari <ransari@mvista.com>
> > ---
>
> Is it possible to rework patch a bit to minimise number of #ifdef ARM ...
> spread across the code?
> Group arch related code into separate file(s), use union for fields with
> sizes, etc?
> Konstantin
>
>
Sure. Thanks.


> >  drivers/net/virtio/virtio_ethdev.c        |  138
> ++++++++++++++++++++++++++++-
> >  lib/librte_eal/linuxapp/igb_uio/igb_uio.c |   80 ++++++++++++++++-
> >  2 files changed, 214 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/virtio/virtio_ethdev.c
> b/drivers/net/virtio/virtio_ethdev.c
> > index 74c00ee..93b8784 100644
> > --- a/drivers/net/virtio/virtio_ethdev.c
> > +++ b/drivers/net/virtio/virtio_ethdev.c
> > @@ -63,6 +63,30 @@
> >  #include "virtqueue.h"
> >  #include "virtio_rxtx.h"
> >
> > +#ifdef RTE_EXEC_ENV_LINUXAPP
> > +/* start address of first pci_iobar slot (user-space virtual-addres) */
> > +void *ioport_map;
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +
> > +#include <sys/mman.h>
> > +#define DEV_NAME             "/dev/igb_ioport"
> > +
> > +/* Keeping pci_ioport_size = 4k.
> > + * So maximum mmaped pci_iobar supported =
> > + * (ioport_size/pci_dev->mem_resource[0].len)
> > + *
> > + * note: kernel could allow maximum 32 virtio-net-pci interface, that
> mean
> > + * maximum 32 PCI_IOBAR(s) where each PCI_IOBAR_LEN=0x20, so
> virtio_map_ioport()
> > + * func by theory gonna support 4k/0x20 ==> 128 PCI_IOBAR(s), more than
> > + * max-virtio-net-pci interface.
> > + */
> > +#define PAGE_SIZE            4096
> > +#define PCI_IOPORT_SIZE              PAGE_SIZE
> > +#define PCI_IOPORT_MAX               128 /* 4k / 0x20 */
> > +
> > +int ioport_map_cnt;
> > +#endif /* ARM, ARM64 */
> > +#endif /* RTE_EXEC_ENV_LINUXAPP */
> >
> >  static int eth_virtio_dev_init(struct rte_eth_dev *eth_dev);
> >  static int eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev);
> > @@ -497,6 +521,18 @@ virtio_dev_close(struct rte_eth_dev *dev)
> >       hw->started = 0;
> >       virtio_dev_free_mbufs(dev);
> >       virtio_free_queues(dev);
> > +
> > +#ifdef RTE_EXEC_ENV_LINUXAPP
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +
> > +     /* unmap ioport memory */
> > +     ioport_map_cnt--;
> > +     if (!ioport_map_cnt)
> > +             munmap(ioport_map, PCI_IOPORT_SIZE);
> > +
> > +     PMD_INIT_LOG(DEBUG, "unmapping ioport_mem %d\n", ioport_map_cnt);
> > +#endif
> > +#endif
> >  }
> >
> >  static void
> > @@ -1172,7 +1208,6 @@ static int virtio_resource_init_by_ioports(struct
> rte_pci_device *pci_dev)
> >
> >                       sscanf(ptr, "%04hx-%04hx", &start, &end);
> >                       size = end - start + 1;
> > -
> >                       break;
> >               }
> >       }
> > @@ -1256,6 +1291,81 @@ rx_func_get(struct rte_eth_dev *eth_dev)
> >               eth_dev->rx_pkt_burst = &virtio_recv_pkts;
> >  }
> >
> > +#ifdef RTE_EXEC_ENV_LINUXAPP
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +
> > +static int
> > +virtio_map_ioport(void **resource_addr)
> > +{
> > +     int fd;
> > +     int ret = 0;
> > +
> > +     fd = open(DEV_NAME, O_RDWR);
> > +     if (fd < 0) {
> > +             PMD_INIT_LOG(ERR, "device file %s open error: %d\n",
> > +                          DEV_NAME, fd);
> > +             ret = -1;
> > +             goto out;
> > +     }
> > +
> > +     ioport_map = mmap(NULL, PCI_IOPORT_SIZE,
> > +                     PROT_EXEC | PROT_WRITE | PROT_READ, MAP_SHARED,
> fd, 0);
> > +
> > +     if (ioport_map == MAP_FAILED) {
> > +             PMD_INIT_LOG(ERR, "mmap: failed to map bar Address=%p\n",
> > +                             *resource_addr);
> > +             ret = -ENOMEM;
> > +             goto out1;
> > +     }
> > +
> > +     PMD_INIT_LOG(INFO, "First pci_iobar mapped at %p\n", ioport_map);
> > +
> > +out1:
> > +     close(fd);
> > +out:
> > +     return ret;
> > +}
> > +
> > +static int
> > +virtio_set_ioport_addr(void **resource_addr, unsigned long offset)
> > +{
> > +     int ret = 0;
> > +
> > +     if (ioport_map_cnt >= PCI_IOPORT_MAX) {
> > +             ret = -1;
> > +             PMD_INIT_LOG(ERR,
> > +                          "ioport_map_cnt(%d) greater than"
> > +                          "PCI_IOPORT_MAX(%d)\n",
> > +                          ioport_map_cnt, PCI_IOPORT_MAX);
> > +             goto out;
> > +     }
> > +     *resource_addr = (void *)((char *)ioport_map +
> (ioport_map_cnt)*offset);
> > +     ioport_map_cnt++;
> > +
> > +     PMD_INIT_LOG(DEBUG, "pci.resource_addr %p ioport_map_cnt %d\n",
> > +                     *resource_addr, ioport_map_cnt);
> > +out:
> > +     return ret;
> > +}
> > +#else /* !ARM, !ARM64 */
> > +static int
> > +virtio_map_ioport(void *resource_addr)
> > +{
> > +     (void)resource_addr;
> > +     return 0;
> > +}
> > +
> > +static int
> > +virtio_set_ioport_addr(void *resource_addr, unsigned long offset)
> > +{
> > +     (void)resource_addr;
> > +     (void)offset;
> > +     return 0;
> > +}
> > +
> > +#endif /* ARM, ARM64 */
> > +#endif /* RTE_EXEC_ENV_LINUXAPP */
> > +
> >  /*
> >   * This function is based on probe() function in virtio_pci.c
> >   * It returns 0 on success.
> > @@ -1294,8 +1404,31 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
> >       if (virtio_resource_init(pci_dev) < 0)
> >               return -1;
> >
> > +#ifdef       RTE_EXEC_ENV_LINUXAPP
> > +     int ret = 0;
> > +
> > +     /* Map the all IOBAR entry from /proc/ioport to 4k page_size only
> once.
> > +      * Later virtio_set_ioport_addr() func will update correct bar_addr
> > +      * eachk ioport (i.e..pci_dev->mem_resource[0].addr)
> > +      */
> > +     if (!ioport_map) {
> > +             ret = virtio_map_ioport(&pci_dev->mem_resource[0].addr);
> > +             if (ret < 0)
> > +                     return -1;
> > +     }
> > +
> > +     ret = virtio_set_ioport_addr(&pci_dev->mem_resource[0].addr,
> > +                                     pci_dev->mem_resource[0].len);
> > +     if (ret < 0)
> > +             return -1;
> > +
> > +     PMD_INIT_LOG(INFO, "ioport_map %p resource_addr %p resource_len
> :%ld\n",
> > +                     ioport_map, pci_dev->mem_resource[0].addr,
> > +                     (unsigned long)pci_dev->mem_resource[0].len);
> > +
> > +#endif
> >       hw->use_msix = virtio_has_msix(&pci_dev->addr);
> > -     hw->io_base = (uint32_t)(uintptr_t)pci_dev->mem_resource[0].addr;
> > +     hw->io_base = (unsigned
> long)(uintptr_t)pci_dev->mem_resource[0].addr;
> >
> >       /* Reset the device although not necessary at startup */
> >       vtpci_reset(hw);
> > @@ -1430,7 +1563,6 @@ eth_virtio_dev_uninit(struct rte_eth_dev *eth_dev)
> >               rte_intr_callback_unregister(&pci_dev->intr_handle,
> >                                               virtio_interrupt_handler,
> >                                               eth_dev);
> > -
> >       PMD_INIT_LOG(DEBUG, "dev_uninit completed");
> >
> >       return 0;
> > diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > index f5617d2..b154a44 100644
> > --- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > +++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
> > @@ -324,6 +324,61 @@ igbuio_dom0_pci_mmap(struct uio_info *info, struct
> vm_area_struct *vma)
> >  }
> >  #endif
> >
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +#ifdef CONFIG_HAS_IOPORT_MAP
> > +/*
> > + * mmap driver to map x86-style PCI_IOBAR (i.e..cat /proc/ioport
> pci-bar-memory)
> > + * from kernel-space virtual address to user-space virtual address.
> This module
> > + * required for non-x86 archs example arm/arm64, as those archs donot do
> > + * IO_MAP_IO types access, Infact supports memory-mapped-IO. That is
> because
> > + * arm/arm64 doesn't support direct IO instruction, so the design
> approach is to
> > + * map `cat /proc/ioport` PCI_IOBAR's kernel-space virtual-addr to
> user-space
> > + * virtual-addr. Therefore the need for mmap-driver.
> > + */
> > +#include <linux/fs.h>                /* file_operations */
> > +#include <linux/miscdevice.h>
> > +#include <linux/mm.h>                /* VM_IO */
> > +#include <linux/uaccess.h>
> > +#include <asm/page.h>
> > +#include <linux/sched.h>
> > +#include <linux/pid.h>
> > +
> > +void *__iomem mapped_io; /* ioport addr of `cat /proc/ioport` */
> > +
> > +static int igb_ioport_mmap(struct file *file, struct vm_area_struct
> *vma)
> > +{
> > +     struct page *npage;
> > +     int ret = 0;
> > +
> > +     vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> > +     npage = vmalloc_to_page(mapped_io);
> > +     ret = remap_pfn_range(vma, vma->vm_start,
> > +                             page_to_pfn(npage),
> > +                             vma->vm_end - vma->vm_start,
> > +                             vma->vm_page_prot);
> > +     if (ret) {
> > +             pr_info("Error: Failed to remap pfn=%lu error=%d\n",
> > +                     page_to_pfn(npage), ret);
> > +     }
> > +     return 0;
> > +}
> > +
> > +static const struct file_operations igb_ioport_fops = {
> > +     .mmap           = igb_ioport_mmap,
> > +};
> > +
> > +static struct miscdevice igb_ioport_dev = {
> > +     .minor  = MISC_DYNAMIC_MINOR,
> > +     .name   = "igb_ioport",
> > +     .fops   = &igb_ioport_fops
> > +};
> > +#else /* !CONFIG_HAS_IOPORT_MAP */
> > +
> > +#error "CONFIG_HAS_IOPORT_MAP not supported for $RTE_ARCH"
> > +
> > +#endif /* CONFIG_HAS_IOPORT_MAP */
> > +#endif /* RTE_ARCH_ARM, RTE_ARCH_ARM64 */
> > +
> >  /* Remap pci resources described by bar #pci_bar in uio resource n. */
> >  static int
> >  igbuio_pci_setup_iomem(struct pci_dev *dev, struct uio_info *info,
> > @@ -365,11 +420,22 @@ igbuio_pci_setup_ioport(struct pci_dev *dev,
> struct uio_info *info,
> >       if (addr == 0 || len == 0)
> >               return -EINVAL;
> >
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +     /*
> > +      * TODO: KNI/procfs:: porttype entry need to be added for
> ARM/ARM64,
> > +      * for below mmap driver;
> > +      * */
> > +     mapped_io = ioport_map(addr, 0);
> > +     info->port[n].name = name;
> > +     info->port[n].start = (unsigned long)(uintptr_t)mapped_io;
> > +     info->port[n].size = len;
> > +     info->port[n].porttype = UIO_PORT_X86;
> > +#else
> >       info->port[n].name = name;
> >       info->port[n].start = addr;
> >       info->port[n].size = len;
> >       info->port[n].porttype = UIO_PORT_X86;
> > -
> > +#endif
> >       return 0;
> >  }
> >
> > @@ -615,6 +681,15 @@ igbuio_pci_init_module(void)
> >  {
> >       int ret;
> >
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +     ret = misc_register(&igb_ioport_dev);
> > +     if (ret < 0) {
> > +             pr_info("Error: failed to register ioport map driver
> (%d)\n",
> > +                             ret);
> > +             return ret;
> > +     }
> > +#endif
> > +
> >       ret = igbuio_config_intr_mode(intr_mode);
> >       if (ret < 0)
> >               return ret;
> > @@ -625,6 +700,9 @@ igbuio_pci_init_module(void)
> >  static void __exit
> >  igbuio_pci_exit_module(void)
> >  {
> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> > +     misc_deregister(&igb_ioport_dev);
> > +#endif
> >       pci_unregister_driver(&igbuio_pci_driver);
> >  }
> >
> > --
> > 1.7.9.5
>
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] Add virtio support in arm/arm64
  2015-12-07  2:12 ` [PATCH 0/6] Add virtio support in arm/arm64 Yuanhan Liu
@ 2015-12-08 12:59   ` Xie, Huawei
  2015-12-10  6:16     ` Santosh Shukla
  0 siblings, 1 reply; 22+ messages in thread
From: Xie, Huawei @ 2015-12-08 12:59 UTC (permalink / raw)
  To: Yuanhan Liu, Santosh Shukla; +Cc: dev



> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Monday, December 07, 2015 10:12 AM
> To: Santosh Shukla
> Cc: dev@dpdk.org; thomas.monjalon@6wind.com; Xie, Huawei
> Subject: Re: [PATCH 0/6] Add virtio support in arm/arm64
> 
> On Fri, Dec 04, 2015 at 11:05:13PM +0530, Santosh Shukla wrote:
> > This patch set add basic infrastrucure to run virtio-net-pci pmd driver
> for
> > arm64/arm. Tested on ThunderX platfrom. Verified for existing dpdk(s)
> test
> > applications like:
> > - ovs-dpdk-vhost-user: across the VM's, for the use-cases like
> guest2guest and
> >   Host2Guest
> > - testpmd application: Tested for max virtio-net-pci interface currently
> >   supported in kernel i.e. 31 interface.
> 
> 
> Hi Santosh,
> 
> Here is a quick notice that I don't have time to review your patches
> this one or two weeks. Sorry for that.
> 
> (Not quite sure Huawei has the bandwidth or not, btw)
Sorry, just back from vocation. We will review your patch.
> 
> 	--yliu

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 3/6] virtio: armv7/v8: Introdice api to emulate x86-style of PCI/ISA ioport access
  2015-12-07 17:09   ` Stephen Hemminger
@ 2015-12-08 15:35     ` Santosh Shukla
  0 siblings, 0 replies; 22+ messages in thread
From: Santosh Shukla @ 2015-12-08 15:35 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev

On Mon, Dec 7, 2015 at 10:39 PM, Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Fri,  4 Dec 2015 23:05:16 +0530
> Santosh Shukla <sshukla@mvista.com> wrote:
>
> > +#if defined(RTE_ARCH_ARM64)
> > +     uint64_t    io_base;
> > +#else /* !ARM64 */
> >       uint32_t    io_base;
> > +#endif
>
> Is there a typedef that could be used instead of having #ifdef clutter?
>

I am not sure if there is any for arm. Can you pl. point me any code
snippet (perhaps arch specific) example?

BTW: I am thinking to change io_base to word_size {arch size aligned} in v2
patch revision and By doing that we don't need care for ifdefs. Currently
uin32_t hold good for io;s ranging from 0 to 65535.

Wanted to keep something like below

unsigned long io_base;

for 32 bit case - 4 byte
for 64 bit case - 8 byte.

Does that make sense?

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver
  2015-12-08 12:53     ` Santosh Shukla
@ 2015-12-09 18:59       ` Santosh Shukla
  2015-12-09 19:04         ` Stephen Hemminger
  0 siblings, 1 reply; 22+ messages in thread
From: Santosh Shukla @ 2015-12-09 18:59 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Rizwan Ansari

On Tue, Dec 8, 2015 at 6:23 PM, Santosh Shukla <sshukla@mvista.com> wrote:
>
>
> On Mon, Dec 7, 2015 at 10:38 PM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
>>
>> On Fri,  4 Dec 2015 23:05:19 +0530
>> Santosh Shukla <sshukla@mvista.com> wrote:
>>
>> >
>> > +#ifdef RTE_EXEC_ENV_LINUXAPP
>> > +/* start address of first pci_iobar slot (user-space virtual-addres) */
>> > +void *ioport_map;
>> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
>> > +
>> > +#include <sys/mman.h>
>> > +#define DEV_NAME             "/dev/igb_ioport"
>> > +
>> > +/* Keeping pci_ioport_size = 4k.
>> > + * So maximum mmaped pci_iobar supported =
>> > + * (ioport_size/pci_dev->mem_resource[0].len)
>> > + *
>> > + * note: kernel could allow maximum 32 virtio-net-pci interface, that
>> > mean
>> > + * maximum 32 PCI_IOBAR(s) where each PCI_IOBAR_LEN=0x20, so
>> > virtio_map_ioport()
>> > + * func by theory gonna support 4k/0x20 ==> 128 PCI_IOBAR(s), more than
>> > + * max-virtio-net-pci interface.
>> > + */
>> > +#define PAGE_SIZE            4096
>> > +#define PCI_IOPORT_SIZE              PAGE_SIZE
>> > +#define PCI_IOPORT_MAX               128 /* 4k / 0x20 */
>> > +
>> > +int ioport_map_cnt;
>> > +#endif /* ARM, ARM64 */
>> > +#endif /* RTE_EXEC_ENV_LINUXAPP */
>>
>> These variables should be static.
>>
>
> (Sorry for delayed follow, Was travelling..)
> Right,
>
>>
>> Also, it is should be possible to extract the I/O bar stuff in a generic
>> way through sysfs
>> and not depend on a character device. The long term goal for DPDK
>> acceptance is to
>> eliminate (or at least reduce to a minumum) any requirement for special
>> kernel drivers.
>
>
> I agree. Existing implementation does read pci_iobar for start address and
> size, But for non-x86 arch, we need someway to map pci_iobar and thats-why
> thought of adding device file for a purpose, as archs like arm lack iopl()
> privileged io syscall support, However iopl() too quite native driver design
> assumption.
>
> I have few idea in my mind such that - Right now I am updating ioport_mapped
> addr {kernel-virtual-addr-io-memory} to /sys/bus/pci/pci_bus_xxxx/xxx/map
> field, instead of mapping their, I'll try to map to uio's pci interface and
> then use existing pci_map_resource() api to mmap kernel-virtual-io-address
> to user-space-virtual-ioaddr. We'll come back on this.
>


Spent sometime digging dpdk's uio/pci source code, Intent was to map
pci ioport region via uio-way. In order to achieve that I tried to
hack the virtio-net-pci pmd driver. Right now in virtio-net-pci case,
It creates two sysfs entry for pci bars: resource0 /1.

Resource0; is ioport region
Resource1; is iomem region.

By appending a RTE_PCI_DRV_NEED_MAPPING flag to drv_flag and passing
hw->io_base = resource1 type pci.mem_resource[slot].addr; where slot
=1. Resource1 is IORESOURCE_MEM type so uio/pci driver able to mmap.
That way I could get the valid user-space virtual address. However
this hack did not worked for me because at qemu side: virtio-pxe.rom
has virtio_headers located at ioport pci region and guest driver
writing at iomem region, that's why driver init failed. Note that
default driver doesn't use resource1 memory.

This made me think that either I had to add dependent code in kernel
or something similar proposed in this patch.
It is because:
- uio driver and dependent user-space pci api's in dpdk mmaps
IORESOURCE_MEM types address only {refer igbuio_setup_bars() and in
particular function pci_parse_sysfs_resource()}.
- That mmap in userspace backed by arch specific api
pci_mmap_page_range() in kernel.
- pci_mmap_page_range() does not support mmaping to IO_RESOURCE_IO type memory.
- Having said that, we need routine or a way to to map pci_iobar
region from kernel virtual-address to user-space virtual address.

In x86 case; iopl() exports the ioport address to userspace. But for
non-x86 case, we need special device file to mmap ioport kernel
address space to user space.  Also I grepped dpdk source code little
more and noticed that xen dom0 implementation is quite similar too.
They are too using /dev/dom0_mem special device file.

IMO, there is approach-2 to achieve desired which I mentioned in
cover-letter too. By adding /dev/ioport device file support in kernel,
so that user could do byte/word/halfword rd/wr, rightnow kernel
support only byte long rd/wr (i.e.. /dev/port file driver/char/mem.c).
In that way; We care to do file open/rd/wr/close for /dev/ioport. No
low level extra arch level api introduced in this patch series. Pretty
clean code. But then we do have kernel device file dependency which is
a cons. I personally like this approach as we are not dependant on
kernel side driver, all desired dependency is in dpdk source base. I
am going to send out approach-2 patch series, perhaps in this week for
comment.

Pl. let me know if you have better approach in mind, Otherwise I am
planning to work on V2 patch revision, incorporating other review
comment.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver
  2015-12-09 18:59       ` Santosh Shukla
@ 2015-12-09 19:04         ` Stephen Hemminger
  2015-12-09 19:19           ` Santosh Shukla
  0 siblings, 1 reply; 22+ messages in thread
From: Stephen Hemminger @ 2015-12-09 19:04 UTC (permalink / raw)
  To: Santosh Shukla; +Cc: dev, Rizwan Ansari

On Thu, 10 Dec 2015 00:29:30 +0530
Santosh Shukla <sshukla@mvista.com> wrote:

> On Tue, Dec 8, 2015 at 6:23 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> >
> >
> > On Mon, Dec 7, 2015 at 10:38 PM, Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> >>
> >> On Fri,  4 Dec 2015 23:05:19 +0530
> >> Santosh Shukla <sshukla@mvista.com> wrote:
> >>
> >> >
> >> > +#ifdef RTE_EXEC_ENV_LINUXAPP
> >> > +/* start address of first pci_iobar slot (user-space virtual-addres) */
> >> > +void *ioport_map;
> >> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> >> > +
> >> > +#include <sys/mman.h>
> >> > +#define DEV_NAME             "/dev/igb_ioport"
> >> > +
> >> > +/* Keeping pci_ioport_size = 4k.
> >> > + * So maximum mmaped pci_iobar supported =
> >> > + * (ioport_size/pci_dev->mem_resource[0].len)
> >> > + *
> >> > + * note: kernel could allow maximum 32 virtio-net-pci interface, that
> >> > mean
> >> > + * maximum 32 PCI_IOBAR(s) where each PCI_IOBAR_LEN=0x20, so
> >> > virtio_map_ioport()
> >> > + * func by theory gonna support 4k/0x20 ==> 128 PCI_IOBAR(s), more than
> >> > + * max-virtio-net-pci interface.
> >> > + */
> >> > +#define PAGE_SIZE            4096
> >> > +#define PCI_IOPORT_SIZE              PAGE_SIZE
> >> > +#define PCI_IOPORT_MAX               128 /* 4k / 0x20 */
> >> > +
> >> > +int ioport_map_cnt;
> >> > +#endif /* ARM, ARM64 */
> >> > +#endif /* RTE_EXEC_ENV_LINUXAPP */
> >>
> >> These variables should be static.
> >>
> >
> > (Sorry for delayed follow, Was travelling..)
> > Right,
> >
> >>
> >> Also, it is should be possible to extract the I/O bar stuff in a generic
> >> way through sysfs
> >> and not depend on a character device. The long term goal for DPDK
> >> acceptance is to
> >> eliminate (or at least reduce to a minumum) any requirement for special
> >> kernel drivers.
> >
> >
> > I agree. Existing implementation does read pci_iobar for start address and
> > size, But for non-x86 arch, we need someway to map pci_iobar and thats-why
> > thought of adding device file for a purpose, as archs like arm lack iopl()
> > privileged io syscall support, However iopl() too quite native driver design
> > assumption.
> >
> > I have few idea in my mind such that - Right now I am updating ioport_mapped
> > addr {kernel-virtual-addr-io-memory} to /sys/bus/pci/pci_bus_xxxx/xxx/map
> > field, instead of mapping their, I'll try to map to uio's pci interface and
> > then use existing pci_map_resource() api to mmap kernel-virtual-io-address
> > to user-space-virtual-ioaddr. We'll come back on this.
> >
> 
> 
> Spent sometime digging dpdk's uio/pci source code, Intent was to map
> pci ioport region via uio-way. In order to achieve that I tried to
> hack the virtio-net-pci pmd driver. Right now in virtio-net-pci case,
> It creates two sysfs entry for pci bars: resource0 /1.
> 
> Resource0; is ioport region
> Resource1; is iomem region.
> 
> By appending a RTE_PCI_DRV_NEED_MAPPING flag to drv_flag and passing
> hw->io_base = resource1 type pci.mem_resource[slot].addr; where slot
> =1. Resource1 is IORESOURCE_MEM type so uio/pci driver able to mmap.
> That way I could get the valid user-space virtual address. However
> this hack did not worked for me because at qemu side: virtio-pxe.rom
> has virtio_headers located at ioport pci region and guest driver
> writing at iomem region, that's why driver init failed. Note that
> default driver doesn't use resource1 memory.
> 
> This made me think that either I had to add dependent code in kernel
> or something similar proposed in this patch.
> It is because:
> - uio driver and dependent user-space pci api's in dpdk mmaps
> IORESOURCE_MEM types address only {refer igbuio_setup_bars() and in
> particular function pci_parse_sysfs_resource()}.
> - That mmap in userspace backed by arch specific api
> pci_mmap_page_range() in kernel.
> - pci_mmap_page_range() does not support mmaping to IO_RESOURCE_IO type memory.
> - Having said that, we need routine or a way to to map pci_iobar
> region from kernel virtual-address to user-space virtual address.

There a couple of gotcha's with this. It turns out the iomem region
is not mappable on some platforms. I think GCE was one of them.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver
  2015-12-09 19:04         ` Stephen Hemminger
@ 2015-12-09 19:19           ` Santosh Shukla
  2015-12-09 19:57             ` Stephen Hemminger
  0 siblings, 1 reply; 22+ messages in thread
From: Santosh Shukla @ 2015-12-09 19:19 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: dev, Rizwan Ansari

On Thu, Dec 10, 2015 at 12:34 AM, Stephen Hemminger
<stephen@networkplumber.org> wrote:
> On Thu, 10 Dec 2015 00:29:30 +0530
> Santosh Shukla <sshukla@mvista.com> wrote:
>
>> On Tue, Dec 8, 2015 at 6:23 PM, Santosh Shukla <sshukla@mvista.com> wrote:
>> >
>> >
>> > On Mon, Dec 7, 2015 at 10:38 PM, Stephen Hemminger
>> > <stephen@networkplumber.org> wrote:
>> >>
>> >> On Fri,  4 Dec 2015 23:05:19 +0530
>> >> Santosh Shukla <sshukla@mvista.com> wrote:
>> >>
>> >> >
>> >> > +#ifdef RTE_EXEC_ENV_LINUXAPP
>> >> > +/* start address of first pci_iobar slot (user-space virtual-addres) */
>> >> > +void *ioport_map;
>> >> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
>> >> > +
>> >> > +#include <sys/mman.h>
>> >> > +#define DEV_NAME             "/dev/igb_ioport"
>> >> > +
>> >> > +/* Keeping pci_ioport_size = 4k.
>> >> > + * So maximum mmaped pci_iobar supported =
>> >> > + * (ioport_size/pci_dev->mem_resource[0].len)
>> >> > + *
>> >> > + * note: kernel could allow maximum 32 virtio-net-pci interface, that
>> >> > mean
>> >> > + * maximum 32 PCI_IOBAR(s) where each PCI_IOBAR_LEN=0x20, so
>> >> > virtio_map_ioport()
>> >> > + * func by theory gonna support 4k/0x20 ==> 128 PCI_IOBAR(s), more than
>> >> > + * max-virtio-net-pci interface.
>> >> > + */
>> >> > +#define PAGE_SIZE            4096
>> >> > +#define PCI_IOPORT_SIZE              PAGE_SIZE
>> >> > +#define PCI_IOPORT_MAX               128 /* 4k / 0x20 */
>> >> > +
>> >> > +int ioport_map_cnt;
>> >> > +#endif /* ARM, ARM64 */
>> >> > +#endif /* RTE_EXEC_ENV_LINUXAPP */
>> >>
>> >> These variables should be static.
>> >>
>> >
>> > (Sorry for delayed follow, Was travelling..)
>> > Right,
>> >
>> >>
>> >> Also, it is should be possible to extract the I/O bar stuff in a generic
>> >> way through sysfs
>> >> and not depend on a character device. The long term goal for DPDK
>> >> acceptance is to
>> >> eliminate (or at least reduce to a minumum) any requirement for special
>> >> kernel drivers.
>> >
>> >
>> > I agree. Existing implementation does read pci_iobar for start address and
>> > size, But for non-x86 arch, we need someway to map pci_iobar and thats-why
>> > thought of adding device file for a purpose, as archs like arm lack iopl()
>> > privileged io syscall support, However iopl() too quite native driver design
>> > assumption.
>> >
>> > I have few idea in my mind such that - Right now I am updating ioport_mapped
>> > addr {kernel-virtual-addr-io-memory} to /sys/bus/pci/pci_bus_xxxx/xxx/map
>> > field, instead of mapping their, I'll try to map to uio's pci interface and
>> > then use existing pci_map_resource() api to mmap kernel-virtual-io-address
>> > to user-space-virtual-ioaddr. We'll come back on this.
>> >
>>
>>
>> Spent sometime digging dpdk's uio/pci source code, Intent was to map
>> pci ioport region via uio-way. In order to achieve that I tried to
>> hack the virtio-net-pci pmd driver. Right now in virtio-net-pci case,
>> It creates two sysfs entry for pci bars: resource0 /1.
>>
>> Resource0; is ioport region
>> Resource1; is iomem region.
>>
>> By appending a RTE_PCI_DRV_NEED_MAPPING flag to drv_flag and passing
>> hw->io_base = resource1 type pci.mem_resource[slot].addr; where slot
>> =1. Resource1 is IORESOURCE_MEM type so uio/pci driver able to mmap.
>> That way I could get the valid user-space virtual address. However
>> this hack did not worked for me because at qemu side: virtio-pxe.rom
>> has virtio_headers located at ioport pci region and guest driver
>> writing at iomem region, that's why driver init failed. Note that
>> default driver doesn't use resource1 memory.
>>
>> This made me think that either I had to add dependent code in kernel
>> or something similar proposed in this patch.
>> It is because:
>> - uio driver and dependent user-space pci api's in dpdk mmaps
>> IORESOURCE_MEM types address only {refer igbuio_setup_bars() and in
>> particular function pci_parse_sysfs_resource()}.
>> - That mmap in userspace backed by arch specific api
>> pci_mmap_page_range() in kernel.
>> - pci_mmap_page_range() does not support mmaping to IO_RESOURCE_IO type memory.
>> - Having said that, we need routine or a way to to map pci_iobar
>> region from kernel virtual-address to user-space virtual address.
>
> There a couple of gotcha's with this. It turns out the iomem region
> is not mappable on some platforms. I think GCE was one of them.

afaik, In linux kernel if arch supports pci_mmap_page_range then iomem
region should map, right? I am confused by reading your reply, also I
am not aware of GCE? which platform is GCE, please suggest.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver
  2015-12-09 19:19           ` Santosh Shukla
@ 2015-12-09 19:57             ` Stephen Hemminger
  0 siblings, 0 replies; 22+ messages in thread
From: Stephen Hemminger @ 2015-12-09 19:57 UTC (permalink / raw)
  To: Santosh Shukla; +Cc: dev, Rizwan Ansari

On Thu, 10 Dec 2015 00:49:08 +0530
Santosh Shukla <sshukla@mvista.com> wrote:

> On Thu, Dec 10, 2015 at 12:34 AM, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > On Thu, 10 Dec 2015 00:29:30 +0530
> > Santosh Shukla <sshukla@mvista.com> wrote:
> >
> >> On Tue, Dec 8, 2015 at 6:23 PM, Santosh Shukla <sshukla@mvista.com> wrote:
> >> >
> >> >
> >> > On Mon, Dec 7, 2015 at 10:38 PM, Stephen Hemminger
> >> > <stephen@networkplumber.org> wrote:
> >> >>
> >> >> On Fri,  4 Dec 2015 23:05:19 +0530
> >> >> Santosh Shukla <sshukla@mvista.com> wrote:
> >> >>
> >> >> >
> >> >> > +#ifdef RTE_EXEC_ENV_LINUXAPP
> >> >> > +/* start address of first pci_iobar slot (user-space virtual-addres) */
> >> >> > +void *ioport_map;
> >> >> > +#if defined(RTE_ARCH_ARM) || defined(RTE_ARCH_ARM64)
> >> >> > +
> >> >> > +#include <sys/mman.h>
> >> >> > +#define DEV_NAME             "/dev/igb_ioport"
> >> >> > +
> >> >> > +/* Keeping pci_ioport_size = 4k.
> >> >> > + * So maximum mmaped pci_iobar supported =
> >> >> > + * (ioport_size/pci_dev->mem_resource[0].len)
> >> >> > + *
> >> >> > + * note: kernel could allow maximum 32 virtio-net-pci interface, that
> >> >> > mean
> >> >> > + * maximum 32 PCI_IOBAR(s) where each PCI_IOBAR_LEN=0x20, so
> >> >> > virtio_map_ioport()
> >> >> > + * func by theory gonna support 4k/0x20 ==> 128 PCI_IOBAR(s), more than
> >> >> > + * max-virtio-net-pci interface.
> >> >> > + */
> >> >> > +#define PAGE_SIZE            4096
> >> >> > +#define PCI_IOPORT_SIZE              PAGE_SIZE
> >> >> > +#define PCI_IOPORT_MAX               128 /* 4k / 0x20 */
> >> >> > +
> >> >> > +int ioport_map_cnt;
> >> >> > +#endif /* ARM, ARM64 */
> >> >> > +#endif /* RTE_EXEC_ENV_LINUXAPP */
> >> >>
> >> >> These variables should be static.
> >> >>
> >> >
> >> > (Sorry for delayed follow, Was travelling..)
> >> > Right,
> >> >
> >> >>
> >> >> Also, it is should be possible to extract the I/O bar stuff in a generic
> >> >> way through sysfs
> >> >> and not depend on a character device. The long term goal for DPDK
> >> >> acceptance is to
> >> >> eliminate (or at least reduce to a minumum) any requirement for special
> >> >> kernel drivers.
> >> >
> >> >
> >> > I agree. Existing implementation does read pci_iobar for start address and
> >> > size, But for non-x86 arch, we need someway to map pci_iobar and thats-why
> >> > thought of adding device file for a purpose, as archs like arm lack iopl()
> >> > privileged io syscall support, However iopl() too quite native driver design
> >> > assumption.
> >> >
> >> > I have few idea in my mind such that - Right now I am updating ioport_mapped
> >> > addr {kernel-virtual-addr-io-memory} to /sys/bus/pci/pci_bus_xxxx/xxx/map
> >> > field, instead of mapping their, I'll try to map to uio's pci interface and
> >> > then use existing pci_map_resource() api to mmap kernel-virtual-io-address
> >> > to user-space-virtual-ioaddr. We'll come back on this.
> >> >
> >>
> >>
> >> Spent sometime digging dpdk's uio/pci source code, Intent was to map
> >> pci ioport region via uio-way. In order to achieve that I tried to
> >> hack the virtio-net-pci pmd driver. Right now in virtio-net-pci case,
> >> It creates two sysfs entry for pci bars: resource0 /1.
> >>
> >> Resource0; is ioport region
> >> Resource1; is iomem region.
> >>
> >> By appending a RTE_PCI_DRV_NEED_MAPPING flag to drv_flag and passing
> >> hw->io_base = resource1 type pci.mem_resource[slot].addr; where slot
> >> =1. Resource1 is IORESOURCE_MEM type so uio/pci driver able to mmap.
> >> That way I could get the valid user-space virtual address. However
> >> this hack did not worked for me because at qemu side: virtio-pxe.rom
> >> has virtio_headers located at ioport pci region and guest driver
> >> writing at iomem region, that's why driver init failed. Note that
> >> default driver doesn't use resource1 memory.
> >>
> >> This made me think that either I had to add dependent code in kernel
> >> or something similar proposed in this patch.
> >> It is because:
> >> - uio driver and dependent user-space pci api's in dpdk mmaps
> >> IORESOURCE_MEM types address only {refer igbuio_setup_bars() and in
> >> particular function pci_parse_sysfs_resource()}.
> >> - That mmap in userspace backed by arch specific api
> >> pci_mmap_page_range() in kernel.
> >> - pci_mmap_page_range() does not support mmaping to IO_RESOURCE_IO type memory.
> >> - Having said that, we need routine or a way to to map pci_iobar
> >> region from kernel virtual-address to user-space virtual address.
> >
> > There a couple of gotcha's with this. It turns out the iomem region
> > is not mappable on some platforms. I think GCE was one of them.
> 
> afaik, In linux kernel if arch supports pci_mmap_page_range then iomem
> region should map, right? I am confused by reading your reply, also I
> am not aware of GCE? which platform is GCE, please suggest.

I think it was Google Compute Environment that reported an memory region
which was huge and not accessible, they have there own vhost.

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 5/6] linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init()
  2015-12-04 17:35 ` [PATCH 5/6] linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init() Santosh Shukla
@ 2015-12-09 19:58   ` Jan Viktorin
  0 siblings, 0 replies; 22+ messages in thread
From: Jan Viktorin @ 2015-12-09 19:58 UTC (permalink / raw)
  To: Santosh Shukla; +Cc: dev

On Fri,  4 Dec 2015 23:05:18 +0530
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>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] Add virtio support in arm/arm64
  2015-12-08 12:59   ` Xie, Huawei
@ 2015-12-10  6:16     ` Santosh Shukla
  2015-12-10  6:31       ` Yuanhan Liu
  0 siblings, 1 reply; 22+ messages in thread
From: Santosh Shukla @ 2015-12-10  6:16 UTC (permalink / raw)
  To: Xie, Huawei; +Cc: dev

On Tue, Dec 8, 2015 at 6:29 PM, Xie, Huawei <huawei.xie@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
>> Sent: Monday, December 07, 2015 10:12 AM
>> To: Santosh Shukla
>> Cc: dev@dpdk.org; thomas.monjalon@6wind.com; Xie, Huawei
>> Subject: Re: [PATCH 0/6] Add virtio support in arm/arm64
>>
>> On Fri, Dec 04, 2015 at 11:05:13PM +0530, Santosh Shukla wrote:
>> > This patch set add basic infrastrucure to run virtio-net-pci pmd driver
>> for
>> > arm64/arm. Tested on ThunderX platfrom. Verified for existing dpdk(s)
>> test
>> > applications like:
>> > - ovs-dpdk-vhost-user: across the VM's, for the use-cases like
>> guest2guest and
>> >   Host2Guest
>> > - testpmd application: Tested for max virtio-net-pci interface currently
>> >   supported in kernel i.e. 31 interface.
>>
>>
>> Hi Santosh,
>>
>> Here is a quick notice that I don't have time to review your patches
>> this one or two weeks. Sorry for that.
>>
>> (Not quite sure Huawei has the bandwidth or not, btw)
> Sorry, just back from vocation. We will review your patch.

Ping,

I have started working on V2 patch set based on review comment so far
received. It would be  great if I get your review comment too.

Also few of my patch code base changes for V2 version gonna overlap
with Yuanhan latest virtio 1.0 patch set titled "[dpdk-dev] [PATCH 0/6
for 2.3] initial virtio 1.0 enabling ", Specially patch 4 and 5. So I
would appreciate any review feedback. That avoid overlapping for me.
Some questions though:
- Shall I rebase Yuanhan's virtio 1.0 patch set, then start v2? By
doing that I could leverage "viritio: switch to 64 bit features". In
this patch series, I did took care of 32 bit vs 64 bit changes for
arm64 case.

>>
>>       --yliu

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH 0/6] Add virtio support in arm/arm64
  2015-12-10  6:16     ` Santosh Shukla
@ 2015-12-10  6:31       ` Yuanhan Liu
  0 siblings, 0 replies; 22+ messages in thread
From: Yuanhan Liu @ 2015-12-10  6:31 UTC (permalink / raw)
  To: Santosh Shukla; +Cc: dev

On Thu, Dec 10, 2015 at 11:46:50AM +0530, Santosh Shukla wrote:
> On Tue, Dec 8, 2015 at 6:29 PM, Xie, Huawei <huawei.xie@intel.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> >> Sent: Monday, December 07, 2015 10:12 AM
> >> To: Santosh Shukla
> >> Cc: dev@dpdk.org; thomas.monjalon@6wind.com; Xie, Huawei
> >> Subject: Re: [PATCH 0/6] Add virtio support in arm/arm64
> >>
> >> On Fri, Dec 04, 2015 at 11:05:13PM +0530, Santosh Shukla wrote:
> >> > This patch set add basic infrastrucure to run virtio-net-pci pmd driver
> >> for
> >> > arm64/arm. Tested on ThunderX platfrom. Verified for existing dpdk(s)
> >> test
> >> > applications like:
> >> > - ovs-dpdk-vhost-user: across the VM's, for the use-cases like
> >> guest2guest and
> >> >   Host2Guest
> >> > - testpmd application: Tested for max virtio-net-pci interface currently
> >> >   supported in kernel i.e. 31 interface.
> >>
> >>
> >> Hi Santosh,
> >>
> >> Here is a quick notice that I don't have time to review your patches
> >> this one or two weeks. Sorry for that.
> >>
> >> (Not quite sure Huawei has the bandwidth or not, btw)
> > Sorry, just back from vocation. We will review your patch.
> 
> Ping,
> 
> I have started working on V2 patch set based on review comment so far
> received. It would be  great if I get your review comment too.
> 
> Also few of my patch code base changes for V2 version gonna overlap
> with Yuanhan latest virtio 1.0 patch set titled "[dpdk-dev] [PATCH 0/6
> for 2.3] initial virtio 1.0 enabling ", Specially patch 4 and 5. So I
> would appreciate any review feedback. That avoid overlapping for me.
> Some questions though:
> - Shall I rebase Yuanhan's virtio 1.0 patch set, then start v2? By

No, you should not develop v2 based on my virtio 1.0 support patchset;
it's in a very rough state, and it will not be applied soon: it's for
v2.3, and v2.2 is not relased yet.

So, I would suggest you (always) to make patches based on the latest
git: rebasing (if necessary) won't be too hard after all, especially
for small patch set.

> doing that I could leverage "viritio: switch to 64 bit features". In
> this patch series, I did took care of 32 bit vs 64 bit changes for
> arm64 case.

I'd suggest you to send v2, with all comments addressed. I will try
to find a time reviewing your patches next week, to see what I can
help :)

	--yliu

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2015-12-10  6:30 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-04 17:35 [PATCH 0/6] Add virtio support in arm/arm64 Santosh Shukla
2015-12-04 17:35 ` [PATCH 1/6] virtio: Introduce config RTE_VIRTIO_INC_VECTOR Santosh Shukla
2015-12-04 17:35 ` [PATCH 2/6] config: i686: set RTE_VIRTIO_INC_VECTOR=n Santosh Shukla
2015-12-04 17:35 ` [PATCH 3/6] virtio: armv7/v8: Introdice api to emulate x86-style of PCI/ISA ioport access Santosh Shukla
2015-12-07 17:09   ` Stephen Hemminger
2015-12-08 15:35     ` Santosh Shukla
2015-12-04 17:35 ` [PATCH 4/6] config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD Santosh Shukla
2015-12-04 17:35 ` [PATCH 5/6] linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init() Santosh Shukla
2015-12-09 19:58   ` Jan Viktorin
2015-12-04 17:35 ` [PATCH 6/6] virtio: arm/arm64: memory mapped IO support in pmd driver Santosh Shukla
2015-12-07 17:08   ` Stephen Hemminger
2015-12-08 12:53     ` Santosh Shukla
2015-12-09 18:59       ` Santosh Shukla
2015-12-09 19:04         ` Stephen Hemminger
2015-12-09 19:19           ` Santosh Shukla
2015-12-09 19:57             ` Stephen Hemminger
2015-12-08  9:47   ` Ananyev, Konstantin
2015-12-08 12:55     ` Santosh Shukla
2015-12-07  2:12 ` [PATCH 0/6] Add virtio support in arm/arm64 Yuanhan Liu
2015-12-08 12:59   ` Xie, Huawei
2015-12-10  6:16     ` Santosh Shukla
2015-12-10  6:31       ` Yuanhan Liu

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.