Discussion:
[dpdk-dev] [ [PATCH v2] 00/13] Add virtio support in arm/arm64
(too old to reply)
Santosh Shukla
2015-12-14 13:00:19 UTC
Permalink
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 History:
v2:
- Removed ifdef arm.. clutter from igb_uio / virtio_ethedev files
- Introduced rte_io.h header file in generic/ and arch specifics i.e.. for
armv7 --> rte_io_32.h, for armv8 --> rte_io_64.h.
- Removed RTE_ARCH_X86 ifdef clutter too and added rte_io.h header which nothing
but wraps sys/io.h for x86_64 and i686
- Moved all the RTE_ARCH_ARM/64 dependancy for igb_uio case to separate header
file named igbuio_ioport_misc.h. Now igb_uio.c will call only three function
- igbuio_iomap
- igbuio_ioport_register
- igbuio_ioport_unregister
- Moved ARM/64 specific definition to include/exec-env/rte_virt_ioport.h header
- Included virtio_ioport.c/h; has all private and public api required to map
iopci bar for non-x86 arch. Tested on thunderX and x86_64 both.
Private api includes:
- virtio_map_ioport
- virtio_set_ioport_addr
Public api includes:
- virtio_ioport_init
- virtio_ioport_unmap

- Last patch is the miscllanious format specifier fix identifid for 64bit case
during regression.


v1:
- 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}


Santosh Shukla (13):
virtio: Introduce config RTE_VIRTIO_INC_VECTOR
config: i686: set RTE_VIRTIO_INC_VECTOR=n
rte_io: armv7/v8: Introduce api to emulate x86-style of PCI/ISA
ioport access
virtio_pci: use rte_io.h for non-x86 arch
virtio: change io_base datatype from uint32_t to uint64_type
config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD
linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init()
rte_io: x86: Remove sys/io.h ifdef x86 clutter
igb_uio: ioport: map iopci region for armv7/v8
include/exec-env: ioport: add rte_virt_ioport header file
virtio_ioport: armv7/v8: mmap virtio iopci bar region
virtio_ethdev: use virtio_ioport api at device init/close
virtio_ethdev : fix format specifier error for 64bit addr case

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 | 3 +-
drivers/net/virtio/virtio_ethdev.c | 10 +-
drivers/net/virtio/virtio_ioport.c | 163 ++++++++++++++++++++
drivers/net/virtio/virtio_ioport.h | 42 +++++
drivers/net/virtio/virtio_pci.h | 6 +-
drivers/net/virtio/virtio_rxtx.c | 7 +
lib/librte_eal/common/Makefile | 1 +
lib/librte_eal/common/include/arch/arm/rte_io.h | 60 +++++++
lib/librte_eal/common/include/arch/arm/rte_io_32.h | 155 +++++++++++++++++++
lib/librte_eal/common/include/arch/arm/rte_io_64.h | 155 +++++++++++++++++++
lib/librte_eal/common/include/arch/x86/rte_io.h | 42 +++++
lib/librte_eal/common/include/generic/rte_io.h | 81 ++++++++++
lib/librte_eal/linuxapp/eal/Makefile | 2 +-
lib/librte_eal/linuxapp/eal/eal.c | 7 +-
.../eal/include/exec-env/rte_virt_ioport.h | 81 ++++++++++
lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 8 +-
.../linuxapp/igb_uio/igbuio_ioport_misc.h | 133 ++++++++++++++++
22 files changed, 957 insertions(+), 14 deletions(-)
create mode 100644 drivers/net/virtio/virtio_ioport.c
create mode 100644 drivers/net/virtio/virtio_ioport.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_32.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h
create mode 100644 lib/librte_eal/common/include/arch/x86/rte_io.h
create mode 100644 lib/librte_eal/common/include/generic/rte_io.h
create mode 100644 lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h
create mode 100644 lib/librte_eal/linuxapp/igb_uio/igbuio_ioport_misc.h
--
1.7.9.5
Santosh Shukla
2015-12-14 13:00:20 UTC
Permalink
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 <***@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 ba9e55d..275fb40 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -273,6 +273,7 @@ CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_RX=n
CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_TX=n
CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DRIVER=n
CONFIG_RTE_LIBRTE_VIRTIO_DEBUG_DUMP=n
+CONFIG_RTE_VIRTIO_INC_VECTOR=y

#
# Compile burst-oriented VMXNET3 PMD driver
diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
index 43835ba..25a842d 100644
--- a/drivers/net/virtio/Makefile
+++ b/drivers/net/virtio/Makefile
@@ -50,7 +50,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtqueue.c
SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_pci.c
SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx.c
SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_ethdev.c
-SRCS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += virtio_rxtx_simple.c
+SRCS-$(CONFIG_RTE_VIRTIO_INC_VECTOR) += virtio_rxtx_simple.c

# this lib depends upon:
DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_eal lib/librte_ether
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 74b39ef..23be1ff 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -438,7 +438,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,

dev->data->rx_queues[queue_idx] = vq;

+#ifdef RTE_VIRTIO_INC_VECTOR
virtio_rxq_vec_setup(vq);
+#endif

return 0;
}
@@ -464,7 +466,10 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
const struct rte_eth_txconf *tx_conf)
{
uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
+
+#ifdef RTE_VIRTIO_INC_VECTOR
struct virtio_hw *hw = dev->data->dev_private;
+#endif
struct virtqueue *vq;
uint16_t tx_free_thresh;
int ret;
@@ -477,6 +482,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
return -EINVAL;
}

+#ifdef RTE_VIRTIO_INC_VECTOR
/* Use simple rx/tx func if single segment and no offloads */
if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS &&
!vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
@@ -485,6 +491,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
dev->rx_pkt_burst = virtio_recv_pkts_vec;
use_simple_rxtx = 1;
}
+#endif

ret = virtio_dev_queue_setup(dev, VTNET_TQ, queue_idx, vtpci_queue_idx,
nb_desc, socket_id, &vq);
--
1.7.9.5
Santosh Shukla
2015-12-17 12:02:38 UTC
Permalink
Post by Santosh Shukla
virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx
instructions. For arm64 in particular, virtio vector implementation does not
exist(todo).
So virtio pmd driver wont build for targets like i686, arm64. By making
RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work
in non-vectored virtio mode.
---
Ping?

any review / comment on this patch much appreciated. Thanks
Post by Santosh Shukla
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 ba9e55d..275fb40 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -273,6 +273,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
DEPDIRS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += lib/librte_eal lib/librte_ether
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 74b39ef..23be1ff 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -438,7 +438,9 @@ virtio_dev_rx_queue_setup(struct rte_eth_dev *dev,
dev->data->rx_queues[queue_idx] = vq;
+#ifdef RTE_VIRTIO_INC_VECTOR
virtio_rxq_vec_setup(vq);
+#endif
return 0;
}
@@ -464,7 +466,10 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
const struct rte_eth_txconf *tx_conf)
{
uint8_t vtpci_queue_idx = 2 * queue_idx + VTNET_SQ_TQ_QUEUE_IDX;
+
+#ifdef RTE_VIRTIO_INC_VECTOR
struct virtio_hw *hw = dev->data->dev_private;
+#endif
struct virtqueue *vq;
uint16_t tx_free_thresh;
int ret;
@@ -477,6 +482,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
return -EINVAL;
}
+#ifdef RTE_VIRTIO_INC_VECTOR
/* Use simple rx/tx func if single segment and no offloads */
if ((tx_conf->txq_flags & VIRTIO_SIMPLE_FLAGS) == VIRTIO_SIMPLE_FLAGS &&
!vtpci_with_feature(hw, VIRTIO_NET_F_MRG_RXBUF)) {
@@ -485,6 +491,7 @@ virtio_dev_tx_queue_setup(struct rte_eth_dev *dev,
dev->rx_pkt_burst = virtio_recv_pkts_vec;
use_simple_rxtx = 1;
}
+#endif
ret = virtio_dev_queue_setup(dev, VTNET_TQ, queue_idx, vtpci_queue_idx,
nb_desc, socket_id, &vq);
--
1.7.9.5
Thomas Monjalon
2015-12-17 12:03:45 UTC
Permalink
Post by Santosh Shukla
Post by Santosh Shukla
virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx
instructions. For arm64 in particular, virtio vector implementation does not
exist(todo).
So virtio pmd driver wont build for targets like i686, arm64. By making
RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work
in non-vectored virtio mode.
---
Ping?
any review / comment on this patch much appreciated. Thanks
Why not check for SSE/AVX support instead of adding yet another config option?
Santosh Shukla
2015-12-17 12:18:48 UTC
Permalink
On Thu, Dec 17, 2015 at 5:33 PM, Thomas Monjalon
Post by Thomas Monjalon
Post by Santosh Shukla
Post by Santosh Shukla
virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx
instructions. For arm64 in particular, virtio vector implementation does not
exist(todo).
So virtio pmd driver wont build for targets like i686, arm64. By making
RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work
in non-vectored virtio mode.
---
Ping?
any review / comment on this patch much appreciated. Thanks
Why not check for SSE/AVX support instead of adding yet another config option?
Ok, keeping a check for sse/avx across the patch wont stand true for
future virtio vectored implementation lets say for arm/arm64 cases
i.e.. sse2neon types. That implies user suppose to keep on appending /
adding checks for see2neon for example and so forth.

On other hand, motivation of including INC_VEC config was inspired
from IXGBE and other pmd drivers who support vectored sse/avx _rx path
and also could work w/o vectored mode. Current virtio is missing such
support and arm dont have vectored sse2neon types implementation right
now so its a blocker for arm case. Also keeping virtio pmd driver
flexible enough to work in non-vectored mode is a requirement/ a
feature.
Stephen Hemminger
2015-12-17 23:24:35 UTC
Permalink
On Thu, 17 Dec 2015 17:32:38 +0530
Post by Santosh Shukla
Post by Santosh Shukla
virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx
instructions. For arm64 in particular, virtio vector implementation does not
exist(todo).
So virtio pmd driver wont build for targets like i686, arm64. By making
RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work
in non-vectored virtio mode.
---
Ping?
any review / comment on this patch much appreciated. Thanks
The patches I posted (and were ignored by Intel) to support indirect
and any layout should have much bigger performance gain than all this
low level SSE bit twiddling.
Yuanhan Liu
2015-12-18 01:31:54 UTC
Permalink
Post by Stephen Hemminger
On Thu, 17 Dec 2015 17:32:38 +0530
Post by Santosh Shukla
Post by Santosh Shukla
virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx
instructions. For arm64 in particular, virtio vector implementation does not
exist(todo).
So virtio pmd driver wont build for targets like i686, arm64. By making
RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work
in non-vectored virtio mode.
---
Ping?
any review / comment on this patch much appreciated. Thanks
The patches I posted (and were ignored by Intel) to support indirect
Sorry, I thought it was reviewed and got applied (and I just started to
review patches for virtio recently).

So, would you please send it out again? I will have a review and test ASAP.

--yliu
Post by Stephen Hemminger
and any layout should have much bigger performance gain than all this
low level SSE bit twiddling.
Xie, Huawei
2015-12-18 09:52:29 UTC
Permalink
Post by Stephen Hemminger
On Thu, 17 Dec 2015 17:32:38 +0530
Post by Santosh Shukla
Post by Santosh Shukla
virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx
instructions. For arm64 in particular, virtio vector implementation does not
exist(todo).
So virtio pmd driver wont build for targets like i686, arm64. By making
RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work
in non-vectored virtio mode.
---
Ping?
any review / comment on this patch much appreciated. Thanks
The patches I posted (and were ignored by Intel) to support indirect
and any layout should have much bigger performance gain than all this
low level SSE bit twiddling.
Hi Stephen:
We only did SSE twiddling to RX, which almost doubles the performance
comparing to normal path in virtio/vhost performance test case. Indirect
and any layout feature enabling are mostly for TX. We also did some
optimization for single segment and non-offload case in TX, without
using SSE, which also gives ~60% performance improvement, in Qian's
result. My optimization is mostly for single segment and non-offload
case, which i calls simple rx/tx.
I plan to add virtio/vhost performance benchmark so that we could easily
measure the performance difference for each patch.

Indirect and any layout features are useful for multiple segment
transmitted packet mbufs. I had acked your patch at the first time, and
thought it is applied. I don't understand why you say it is ignored by
Intel.
Thomas Monjalon
2015-12-18 10:41:29 UTC
Permalink
Post by Xie, Huawei
Post by Stephen Hemminger
On Thu, 17 Dec 2015 17:32:38 +0530
Post by Santosh Shukla
Post by Santosh Shukla
virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx
instructions. For arm64 in particular, virtio vector implementation does not
exist(todo).
So virtio pmd driver wont build for targets like i686, arm64. By making
RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work
in non-vectored virtio mode.
---
Ping?
any review / comment on this patch much appreciated. Thanks
The patches I posted (and were ignored by Intel) to support indirect
and any layout should have much bigger performance gain than all this
low level SSE bit twiddling.
We only did SSE twiddling to RX, which almost doubles the performance
comparing to normal path in virtio/vhost performance test case. Indirect
and any layout feature enabling are mostly for TX. We also did some
optimization for single segment and non-offload case in TX, without
using SSE, which also gives ~60% performance improvement, in Qian's
result. My optimization is mostly for single segment and non-offload
case, which i calls simple rx/tx.
I plan to add virtio/vhost performance benchmark so that we could easily
measure the performance difference for each patch.
Indirect and any layout features are useful for multiple segment
transmitted packet mbufs. I had acked your patch at the first time, and
thought it is applied. I don't understand why you say it is ignored by
Intel.
There was an error and Stephen never replied nor pinged about it:
http://dpdk.org/ml/archives/dev/2015-October/026984.html
It happens.
Reminder: it is the responsibility of the author to get patches reviewed
and accepted.
Please let's avoid useless blaming.
Stephen Hemminger
2015-12-18 17:33:42 UTC
Permalink
On Fri, 18 Dec 2015 09:52:29 +0000
Post by Xie, Huawei
Post by Stephen Hemminger
low level SSE bit twiddling.
We only did SSE twiddling to RX, which almost doubles the performance
comparing to normal path in virtio/vhost performance test case. Indirect
and any layout feature enabling are mostly for TX. We also did some
optimization for single segment and non-offload case in TX, without
using SSE, which also gives ~60% performance improvement, in Qian's
result. My optimization is mostly for single segment and non-offload
case, which i calls simple rx/tx.
I plan to add virtio/vhost performance benchmark so that we could easily
measure the performance difference for each patch.
Indirect and any layout features are useful for multiple segment
transmitted packet mbufs. I had acked your patch at the first time, and
thought it is applied. I don't understand why you say it is ignored by
Intel.
Sorry, did not mean to blame Intel, ... more that why didn't it get in 2.2?

It turns out any layout/indirect helps all transmits because they can then
take a single tx descriptor rather than multiple.
Thomas Monjalon
2015-12-18 18:11:21 UTC
Permalink
Post by Stephen Hemminger
On Fri, 18 Dec 2015 09:52:29 +0000
Post by Xie, Huawei
Post by Stephen Hemminger
low level SSE bit twiddling.
We only did SSE twiddling to RX, which almost doubles the performance
comparing to normal path in virtio/vhost performance test case. Indirect
and any layout feature enabling are mostly for TX. We also did some
optimization for single segment and non-offload case in TX, without
using SSE, which also gives ~60% performance improvement, in Qian's
result. My optimization is mostly for single segment and non-offload
case, which i calls simple rx/tx.
I plan to add virtio/vhost performance benchmark so that we could easily
measure the performance difference for each patch.
Indirect and any layout features are useful for multiple segment
transmitted packet mbufs. I had acked your patch at the first time, and
thought it is applied. I don't understand why you say it is ignored by
Intel.
Sorry, did not mean to blame Intel, ... more that why didn't it get in 2.2?
I've already answered to this question:
http://dpdk.org/ml/archives/dev/2015-December/030540.html
There was a compilation error and you have not followed up.
Santosh Shukla
2015-12-18 12:46:36 UTC
Permalink
On Fri, Dec 18, 2015 at 4:54 AM, Stephen Hemminger
Post by Stephen Hemminger
On Thu, 17 Dec 2015 17:32:38 +0530
Post by Santosh Shukla
Post by Santosh Shukla
virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx
instructions. For arm64 in particular, virtio vector implementation does not
exist(todo).
So virtio pmd driver wont build for targets like i686, arm64. By making
RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work
in non-vectored virtio mode.
---
Ping?
any review / comment on this patch much appreciated. Thanks
The patches I posted (and were ignored by Intel) to support indirect
and any layout should have much bigger performance gain than all this
low level SSE bit twiddling.
I little confused - do we care for this patch?
Yuanhan Liu
2015-12-22 06:26:19 UTC
Permalink
Post by Santosh Shukla
On Fri, Dec 18, 2015 at 4:54 AM, Stephen Hemminger
Post by Stephen Hemminger
On Thu, 17 Dec 2015 17:32:38 +0530
Post by Santosh Shukla
Post by Santosh Shukla
virtio_recv_pkts_vec and other virtio vector friend apis are written for sse/avx
instructions. For arm64 in particular, virtio vector implementation does not
exist(todo).
So virtio pmd driver wont build for targets like i686, arm64. By making
RTE_VIRTIO_INC_VECTOR=n, Driver can build for non-sse/avx targets and will work
in non-vectored virtio mode.
---
Ping?
any review / comment on this patch much appreciated. Thanks
The patches I posted (and were ignored by Intel) to support indirect
and any layout should have much bigger performance gain than all this
low level SSE bit twiddling.
I little confused - do we care for this patch?
Santosh,

As a reviewer that still have a lot of work to do, I don't have the
bandwidth to review _all_ your patches carefully __once__. That is
to say, I will only comment when I find something should be commented,
from time to time when I put more thoughts there. For other patches
I've no comment, it could mean that it's okay to me so far, or I'm
not quite sure it's okay but I don't find anything obvious wrong.
Hence, I put no comments so far. But later, when get time, I will
revisit them, think more, and either ACK it, or comment it.

So, you could simply keep those patches unchanged if they received
no comments, and fix other comments, and send out a new version at
anytime that is proper to you.

--yliu
Santosh Shukla
2015-12-14 13:00:21 UTC
Permalink
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 <***@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
Santosh Shukla
2015-12-17 12:03:24 UTC
Permalink
Post by Santosh Shukla
config/defconfig_i686-native-linuxapp-gcc says "Vectorized PMD is not supported
on 32-bit".
So setting RTE_VIRTIO_INC_VECTOR to 'n'.
---
ping? review comment please.
Post by Santosh Shukla
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
Santosh Shukla
2015-12-14 13:00:22 UTC
Permalink
Introducing rte_io.h header file to emulate x86-style of ioport rd/wr api
example {in,out}[bwl] and {in_p,out_p}[bwl]. Api support added for armv7 and
armv8 both.

Current use-case for this api is for virtio_pci module that does x86-style
rd/wr.

Tested for armv8/ThunderX platform and build successfully for armv7.

Signed-off-by: Santosh Shukla <***@mvista.com>
---
lib/librte_eal/common/Makefile | 1 +
lib/librte_eal/common/include/arch/arm/rte_io.h | 60 ++++++++
lib/librte_eal/common/include/arch/arm/rte_io_32.h | 155 ++++++++++++++++++++
lib/librte_eal/common/include/arch/arm/rte_io_64.h | 155 ++++++++++++++++++++
lib/librte_eal/common/include/generic/rte_io.h | 81 ++++++++++
5 files changed, 452 insertions(+)
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_32.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h
create mode 100644 lib/librte_eal/common/include/generic/rte_io.h

diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
index f5ea0ee..1021e1d 100644
--- a/lib/librte_eal/common/Makefile
+++ b/lib/librte_eal/common/Makefile
@@ -48,6 +48,7 @@ endif

GENERIC_INC := rte_atomic.h rte_byteorder.h rte_cycles.h rte_prefetch.h
GENERIC_INC += rte_spinlock.h rte_memcpy.h rte_cpuflags.h rte_rwlock.h
+GENERIC_INC += rte_io.h
# defined in mk/arch/$(RTE_ARCH)/rte.vars.mk
ARCH_DIR ?= $(RTE_ARCH)
ARCH_INC := $(notdir $(wildcard $(RTE_SDK)/lib/librte_eal/common/include/arch/$(ARCH_DIR)/*.h))
diff --git a/lib/librte_eal/common/include/arch/arm/rte_io.h b/lib/librte_eal/common/include/arch/arm/rte_io.h
new file mode 100644
index 0000000..b4f1613
--- /dev/null
+++ b/lib/librte_eal/common/include/arch/arm/rte_io.h
@@ -0,0 +1,60 @@
+/*
+ * 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.
+ */
+
+#ifndef _RTE_IO_ARM_H_
+#define _RTE_IO_ARM_H_
+
+/*
+ * @File
+ * Use-case:
+ * 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 pmd driver to access IO's for arm/arm64
+ * arch in x86-style of apis example: {in , out}[bwl] and {in_p , out_p}[bwl].
+ */
+
+#ifdef RTE_ARCH_64
+#include <rte_io_64.h>
+#else
+#include <rte_io_32.h>
+#endif
+
+#endif /* _RTE_IO_ARM_H_ */
diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_32.h b/lib/librte_eal/common/include/arch/arm/rte_io_32.h
new file mode 100644
index 0000000..0e79427
--- /dev/null
+++ b/lib/librte_eal/common/include/arch/arm/rte_io_32.h
@@ -0,0 +1,155 @@
+/*
+ * 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.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_IO_ARM32_H_
+#define _RTE_IO_ARM32_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include "generic/rte_io.h"
+
+/*
+ * 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;
+}
+
+/**
+ * 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);
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_IO_ARM32_H_ */
diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_64.h b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
new file mode 100644
index 0000000..b601f2a
--- /dev/null
+++ b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
@@ -0,0 +1,155 @@
+/*
+ * 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.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_IO_ARM64_H_
+#define _RTE_IO_ARM64_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include "generic/rte_io.h"
+
+/*
+ * 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;
+}
+
+/**
+ * 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);
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_IO_ARM64_H_ */
diff --git a/lib/librte_eal/common/include/generic/rte_io.h b/lib/librte_eal/common/include/generic/rte_io.h
new file mode 100644
index 0000000..7cc4279
--- /dev/null
+++ b/lib/librte_eal/common/include/generic/rte_io.h
@@ -0,0 +1,81 @@
+/*
+ * 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.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_IO_H_
+#define _RTE_IO_H_
+
+/**
+ * @file
+ *
+ * IO operations.
+ *
+ * This file defines an API for IO rd/wr inline-functions, API's of the style
+ * {in , out}[bwl] and {in_p, out_p} [bwl]. which are architecture-dependent.
+ * Used by non-x86 archs. In particular used by arm/arm64 arch.
+ */
+
+#include <stdint.h>
+
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
+#include <sys/io.h>
+#else
+
+/**
+ * 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);
+static inline uint16_t inw(unsigned long addr);
+static inline uint32_t inl(unsigned long addr);
+
+static inline void outb(uint8_t value, unsigned long addr);
+static inline void outw(uint16_t value, unsigned long addr);
+static inline void outl(uint32_t value, unsigned long addr);
+
+static inline uint8_t inb_p(unsigned long addr);
+static inline uint16_t inw_p(unsigned long addr);
+static inline uint32_t inl_p(unsigned long addr);
+
+static inline void outb_p(uint8_t value, unsigned long addr);
+static inline void outw_p(uint16_t value, unsigned long addr);
+static inline void outl_p(uint32_t value, unsigned long addr);
+
+#endif
+
+#endif /* _RTE_IO_H_ */
+
--
1.7.9.5
Jerin Jacob
2015-12-14 14:25:13 UTC
Permalink
Post by Santosh Shukla
Introducing rte_io.h header file to emulate x86-style of ioport rd/wr api
example {in,out}[bwl] and {in_p,out_p}[bwl]. Api support added for armv7 and
armv8 both.
Current use-case for this api is for virtio_pci module that does x86-style
rd/wr.
Tested for armv8/ThunderX platform and build successfully for armv7.
---
lib/librte_eal/common/Makefile | 1 +
lib/librte_eal/common/include/arch/arm/rte_io.h | 60 ++++++++
lib/librte_eal/common/include/arch/arm/rte_io_32.h | 155 ++++++++++++++++++++
lib/librte_eal/common/include/arch/arm/rte_io_64.h | 155 ++++++++++++++++++++
lib/librte_eal/common/include/generic/rte_io.h | 81 ++++++++++
5 files changed, 452 insertions(+)
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_32.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h
create mode 100644 lib/librte_eal/common/include/generic/rte_io.h
diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
index f5ea0ee..1021e1d 100644
--- a/lib/librte_eal/common/Makefile
+++ b/lib/librte_eal/common/Makefile
@@ -48,6 +48,7 @@ endif
GENERIC_INC := rte_atomic.h rte_byteorder.h rte_cycles.h rte_prefetch.h
GENERIC_INC += rte_spinlock.h rte_memcpy.h rte_cpuflags.h rte_rwlock.h
+GENERIC_INC += rte_io.h
# defined in mk/arch/$(RTE_ARCH)/rte.vars.mk
ARCH_DIR ?= $(RTE_ARCH)
ARCH_INC := $(notdir $(wildcard $(RTE_SDK)/lib/librte_eal/common/include/arch/$(ARCH_DIR)/*.h))
diff --git a/lib/librte_eal/common/include/arch/arm/rte_io.h b/lib/librte_eal/common/include/arch/arm/rte_io.h
new file mode 100644
index 0000000..b4f1613
--- /dev/null
+++ b/lib/librte_eal/common/include/arch/arm/rte_io.h
@@ -0,0 +1,60 @@
+/*
+ * 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
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_IO_ARM_H_
+#define _RTE_IO_ARM_H_
+
+/*
+ * 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 pmd driver to access IO's for arm/arm64
+ * arch in x86-style of apis example: {in , out}[bwl] and {in_p , out_p}[bwl].
+ */
+
+#ifdef RTE_ARCH_64
+#include <rte_io_64.h>
+#else
+#include <rte_io_32.h>
+#endif
+
+#endif /* _RTE_IO_ARM_H_ */
diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_32.h b/lib/librte_eal/common/include/arch/arm/rte_io_32.h
new file mode 100644
index 0000000..0e79427
--- /dev/null
+++ b/lib/librte_eal/common/include/arch/arm/rte_io_32.h
@@ -0,0 +1,155 @@
+/*
+ * 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.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_IO_ARM32_H_
+#define _RTE_IO_ARM32_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include "generic/rte_io.h"
+
+/*
+ * Generic IO read/write api for arm: Refer TRM
+ */
+static inline void raw_writeb(uint8_t val, uint32_t addr)
+{
armv7 and armv8 instructions looks same for both raw_* implementation.
The difference in length of "addr" can be abstracted as uintptr_t to
reuse the code.

Use AT&T sytax on inline assembly as mentioned in
doc/guides/contributing/coding_style.rst

Inline ASM in C code
~~~~~~~~~~~~~~~~~~~~

The ``asm`` and ``volatile`` keywords do not have underscores. The AT&T
syntax should be used.
Input and output operands should be named to avoid confusion, as shown
in the following example:

.. code-block:: c

asm volatile("outb %[val], %[port]"
: :
[port] "dN" (port),
[val] "a" (val));
Post by Santosh Shukla
+ 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;
+}
+
+/**
+ * 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);
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_IO_ARM32_H_ */
diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_64.h b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
new file mode 100644
index 0000000..b601f2a
--- /dev/null
+++ b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
@@ -0,0 +1,155 @@
+/*
+ * 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.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_IO_ARM64_H_
+#define _RTE_IO_ARM64_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include "generic/rte_io.h"
+
+/*
+ * 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;
+}
+
+/**
+ * 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);
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_IO_ARM64_H_ */
diff --git a/lib/librte_eal/common/include/generic/rte_io.h b/lib/librte_eal/common/include/generic/rte_io.h
new file mode 100644
index 0000000..7cc4279
--- /dev/null
+++ b/lib/librte_eal/common/include/generic/rte_io.h
@@ -0,0 +1,81 @@
+/*
+ * 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.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_IO_H_
+#define _RTE_IO_H_
+
+/**
+ *
+ * IO operations.
+ *
+ * This file defines an API for IO rd/wr inline-functions, API's of the style
+ * {in , out}[bwl] and {in_p, out_p} [bwl]. which are architecture-dependent.
+ * Used by non-x86 archs. In particular used by arm/arm64 arch.
+ */
+
+#include <stdint.h>
+
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
Shouldn't be
#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686) ||
defined(RTE_ARCH_X86_X32) ?
Post by Santosh Shukla
+#include <sys/io.h>
+#else
+
+/**
+ * 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);
+static inline uint16_t inw(unsigned long addr);
+static inline uint32_t inl(unsigned long addr);
+
+static inline void outb(uint8_t value, unsigned long addr);
+static inline void outw(uint16_t value, unsigned long addr);
+static inline void outl(uint32_t value, unsigned long addr);
+
+static inline uint8_t inb_p(unsigned long addr);
+static inline uint16_t inw_p(unsigned long addr);
+static inline uint32_t inl_p(unsigned long addr);
+
+static inline void outb_p(uint8_t value, unsigned long addr);
+static inline void outw_p(uint16_t value, unsigned long addr);
+static inline void outl_p(uint32_t value, unsigned long addr);
+
+#endif
+
+#endif /* _RTE_IO_H_ */
+
--
1.7.9.5
Santosh Shukla
2015-12-14 16:29:48 UTC
Permalink
On Mon, Dec 14, 2015 at 7:55 PM, Jerin Jacob
Post by Jerin Jacob
Post by Santosh Shukla
Introducing rte_io.h header file to emulate x86-style of ioport rd/wr api
example {in,out}[bwl] and {in_p,out_p}[bwl]. Api support added for armv7 and
armv8 both.
Current use-case for this api is for virtio_pci module that does x86-style
rd/wr.
Tested for armv8/ThunderX platform and build successfully for armv7.
---
lib/librte_eal/common/Makefile | 1 +
lib/librte_eal/common/include/arch/arm/rte_io.h | 60 ++++++++
lib/librte_eal/common/include/arch/arm/rte_io_32.h | 155 ++++++++++++++++++++
lib/librte_eal/common/include/arch/arm/rte_io_64.h | 155 ++++++++++++++++++++
lib/librte_eal/common/include/generic/rte_io.h | 81 ++++++++++
5 files changed, 452 insertions(+)
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_32.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h
create mode 100644 lib/librte_eal/common/include/generic/rte_io.h
diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
index f5ea0ee..1021e1d 100644
--- a/lib/librte_eal/common/Makefile
+++ b/lib/librte_eal/common/Makefile
@@ -48,6 +48,7 @@ endif
GENERIC_INC := rte_atomic.h rte_byteorder.h rte_cycles.h rte_prefetch.h
GENERIC_INC += rte_spinlock.h rte_memcpy.h rte_cpuflags.h rte_rwlock.h
+GENERIC_INC += rte_io.h
# defined in mk/arch/$(RTE_ARCH)/rte.vars.mk
ARCH_DIR ?= $(RTE_ARCH)
ARCH_INC := $(notdir $(wildcard $(RTE_SDK)/lib/librte_eal/common/include/arch/$(ARCH_DIR)/*.h))
diff --git a/lib/librte_eal/common/include/arch/arm/rte_io.h b/lib/librte_eal/common/include/arch/arm/rte_io.h
new file mode 100644
index 0000000..b4f1613
--- /dev/null
+++ b/lib/librte_eal/common/include/arch/arm/rte_io.h
@@ -0,0 +1,60 @@
+/*
+ * 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
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_IO_ARM_H_
+#define _RTE_IO_ARM_H_
+
+/*
+ * 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 pmd driver to access IO's for arm/arm64
+ * arch in x86-style of apis example: {in , out}[bwl] and {in_p , out_p}[bwl].
+ */
+
+#ifdef RTE_ARCH_64
+#include <rte_io_64.h>
+#else
+#include <rte_io_32.h>
+#endif
+
+#endif /* _RTE_IO_ARM_H_ */
diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_32.h b/lib/librte_eal/common/include/arch/arm/rte_io_32.h
new file mode 100644
index 0000000..0e79427
--- /dev/null
+++ b/lib/librte_eal/common/include/arch/arm/rte_io_32.h
@@ -0,0 +1,155 @@
+/*
+ * 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.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_IO_ARM32_H_
+#define _RTE_IO_ARM32_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include "generic/rte_io.h"
+
+/*
+ * Generic IO read/write api for arm: Refer TRM
+ */
+static inline void raw_writeb(uint8_t val, uint32_t addr)
+{
armv7 and armv8 instructions looks same for both raw_* implementation.
The difference in length of "addr" can be abstracted as uintptr_t to
reuse the code.
Use AT&T sytax on inline assembly as mentioned in
doc/guides/contributing/coding_style.rst
Inline ASM in C code
~~~~~~~~~~~~~~~~~~~~
The ``asm`` and ``volatile`` keywords do not have underscores. The AT&T
syntax should be used.
Input and output operands should be named to avoid confusion, as shown
.. code-block:: c
asm volatile("outb %[val], %[port]"
[port] "dN" (port),
[val] "a" (val));
Ok.
Post by Jerin Jacob
Post by Santosh Shukla
+ 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;
+}
+
+/**
+ * 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);
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_IO_ARM32_H_ */
diff --git a/lib/librte_eal/common/include/arch/arm/rte_io_64.h b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
new file mode 100644
index 0000000..b601f2a
--- /dev/null
+++ b/lib/librte_eal/common/include/arch/arm/rte_io_64.h
@@ -0,0 +1,155 @@
+/*
+ * 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.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_IO_ARM64_H_
+#define _RTE_IO_ARM64_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include "generic/rte_io.h"
+
+/*
+ * 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;
+}
+
+/**
+ * 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);
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_IO_ARM64_H_ */
diff --git a/lib/librte_eal/common/include/generic/rte_io.h b/lib/librte_eal/common/include/generic/rte_io.h
new file mode 100644
index 0000000..7cc4279
--- /dev/null
+++ b/lib/librte_eal/common/include/generic/rte_io.h
@@ -0,0 +1,81 @@
+/*
+ * 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.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_IO_H_
+#define _RTE_IO_H_
+
+/**
+ *
+ * IO operations.
+ *
+ * This file defines an API for IO rd/wr inline-functions, API's of the style
+ * {in , out}[bwl] and {in_p, out_p} [bwl]. which are architecture-dependent.
+ * Used by non-x86 archs. In particular used by arm/arm64 arch.
+ */
+
+#include <stdint.h>
+
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
Shouldn't be
#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686) ||
defined(RTE_ARCH_X86_X32) ?
sys/io.h code incident in dpdk using similar ifdef so used, I guess
your are right on including X86_X32 as because it is now
include/generic/ direcotry. We'll do in v3. Thanks
Post by Jerin Jacob
Post by Santosh Shukla
+#include <sys/io.h>
+#else
+
+/**
+ * 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);
+static inline uint16_t inw(unsigned long addr);
+static inline uint32_t inl(unsigned long addr);
+
+static inline void outb(uint8_t value, unsigned long addr);
+static inline void outw(uint16_t value, unsigned long addr);
+static inline void outl(uint32_t value, unsigned long addr);
+
+static inline uint8_t inb_p(unsigned long addr);
+static inline uint16_t inw_p(unsigned long addr);
+static inline uint32_t inl_p(unsigned long addr);
+
+static inline void outb_p(uint8_t value, unsigned long addr);
+static inline void outw_p(uint16_t value, unsigned long addr);
+static inline void outl_p(uint32_t value, unsigned long addr);
+
+#endif
+
+#endif /* _RTE_IO_H_ */
+
--
1.7.9.5
Santosh Shukla
2015-12-14 13:00:23 UTC
Permalink
Use rte_io.h for non-x86 arch.

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

diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 47f722a..3f4ff80 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -40,7 +40,11 @@
#include <sys/types.h>
#include <machine/cpufunc.h>
#else
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
#include <sys/io.h>
+#else
+#include <rte_io.h>
+#endif
#endif

#include <rte_ethdev.h>
--
1.7.9.5
Jerin Jacob
2015-12-14 14:28:21 UTC
Permalink
Post by Santosh Shukla
Use rte_io.h for non-x86 arch.
---
drivers/net/virtio/virtio_pci.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 47f722a..3f4ff80 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -40,7 +40,11 @@
#include <sys/types.h>
#include <machine/cpufunc.h>
#else
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
#include <sys/io.h>
+#else
+#include <rte_io.h>
I believe this patch can be squashed with patch 08/13
Post by Santosh Shukla
+#endif
#endif
#include <rte_ethdev.h>
--
1.7.9.5
Santosh Shukla
2015-12-14 15:29:06 UTC
Permalink
On Mon, Dec 14, 2015 at 7:58 PM, Jerin Jacob
Post by Jerin Jacob
Post by Santosh Shukla
Use rte_io.h for non-x86 arch.
---
drivers/net/virtio/virtio_pci.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 47f722a..3f4ff80 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -40,7 +40,11 @@
#include <sys/types.h>
#include <machine/cpufunc.h>
#else
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
#include <sys/io.h>
+#else
+#include <rte_io.h>
I believe this patch can be squashed with patch 08/13
Yes. Thanks.
Post by Jerin Jacob
Post by Santosh Shukla
+#endif
#endif
#include <rte_ethdev.h>
--
1.7.9.5
Santosh Shukla
2015-12-14 13:00:24 UTC
Permalink
In x86 case io_base to store ioport address not more than 65535 ioports. i.e..0
to ffff but in non-x86 case in particular arm64 it need to store more than 32
bit address so changing io_base datatype from 32 to 64.

Signed-off-by: Santosh Shukla <***@mvista.com>
---
drivers/net/virtio/virtio_ethdev.c | 2 +-
drivers/net/virtio/virtio_pci.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d928339..620e0d4 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1291,7 +1291,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
return -1;

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 = (uint64_t)(uintptr_t)pci_dev->mem_resource[0].addr;

/* Reset the device although not necessary at startup */
vtpci_reset(hw);
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 3f4ff80..f3e4178 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -169,7 +169,7 @@ struct virtqueue;

struct virtio_hw {
struct virtqueue *cvq;
- uint32_t io_base;
+ uint64_t io_base;
uint32_t guest_features;
uint32_t max_tx_queues;
uint32_t max_rx_queues;
@@ -231,7 +231,7 @@ outl_p(unsigned int data, unsigned int port)
#endif

#define VIRTIO_PCI_REG_ADDR(hw, reg) \
- (unsigned short)((hw)->io_base + (reg))
+ (unsigned long)((hw)->io_base + (reg))

#define VIRTIO_READ_REG_1(hw, reg) \
inb((VIRTIO_PCI_REG_ADDR((hw), (reg))))
--
1.7.9.5
Yuanhan Liu
2015-12-16 13:48:50 UTC
Permalink
Post by Santosh Shukla
In x86 case io_base to store ioport address not more than 65535 ioports. i.e..0
to ffff but in non-x86 case in particular arm64 it need to store more than 32
bit address so changing io_base datatype from 32 to 64.
---
drivers/net/virtio/virtio_ethdev.c | 2 +-
drivers/net/virtio/virtio_pci.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d928339..620e0d4 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1291,7 +1291,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
return -1;
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 = (uint64_t)(uintptr_t)pci_dev->mem_resource[0].addr;
I'd suggest to move the io_base assignment (and cast) into virtio_ioport_init()
so that we could do the correct cast there, say cast it to uint32_t for
X86, and uint64_t for others.

--yliu
Post by Santosh Shukla
/* Reset the device although not necessary at startup */
vtpci_reset(hw);
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 3f4ff80..f3e4178 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -169,7 +169,7 @@ struct virtqueue;
struct virtio_hw {
struct virtqueue *cvq;
- uint32_t io_base;
+ uint64_t io_base;
uint32_t guest_features;
uint32_t max_tx_queues;
uint32_t max_rx_queues;
@@ -231,7 +231,7 @@ outl_p(unsigned int data, unsigned int port)
#endif
#define VIRTIO_PCI_REG_ADDR(hw, reg) \
- (unsigned short)((hw)->io_base + (reg))
+ (unsigned long)((hw)->io_base + (reg))
#define VIRTIO_READ_REG_1(hw, reg) \
inb((VIRTIO_PCI_REG_ADDR((hw), (reg))))
--
1.7.9.5
Santosh Shukla
2015-12-16 14:01:57 UTC
Permalink
On Wed, Dec 16, 2015 at 7:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
In x86 case io_base to store ioport address not more than 65535 ioports. i.e..0
to ffff but in non-x86 case in particular arm64 it need to store more than 32
bit address so changing io_base datatype from 32 to 64.
---
drivers/net/virtio/virtio_ethdev.c | 2 +-
drivers/net/virtio/virtio_pci.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d928339..620e0d4 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1291,7 +1291,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
return -1;
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 = (uint64_t)(uintptr_t)pci_dev->mem_resource[0].addr;
I'd suggest to move the io_base assignment (and cast) into virtio_ioport_init()
so that we could do the correct cast there, say cast it to uint32_t for
X86, and uint64_t for others.
Ok.

This was deliberately done considering your 1.0 virtio spec patch do
care for uint64_t types and in arm64 case, If I plan to use those
future patches, IMO it make more sense to me keep it in uint64_t way;

Also in x86 case max address could of type 0x1000-101f and so forth;
changing data-type to uint64_t default wont effect such address,
right? And hw->io_base by looking at virtio_pci.h function like
inb/outb etc.. takes io_base address as unsigned long types which is
arch dependent; i.e.. 4 byte for 32 bit and 8 for 64 bit so the lower
level rd/wr apis are taking care of data-types accordingly.
Post by Yuanhan Liu
--yliu
Post by Santosh Shukla
/* Reset the device although not necessary at startup */
vtpci_reset(hw);
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 3f4ff80..f3e4178 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -169,7 +169,7 @@ struct virtqueue;
struct virtio_hw {
struct virtqueue *cvq;
- uint32_t io_base;
+ uint64_t io_base;
uint32_t guest_features;
uint32_t max_tx_queues;
uint32_t max_rx_queues;
@@ -231,7 +231,7 @@ outl_p(unsigned int data, unsigned int port)
#endif
#define VIRTIO_PCI_REG_ADDR(hw, reg) \
- (unsigned short)((hw)->io_base + (reg))
+ (unsigned long)((hw)->io_base + (reg))
#define VIRTIO_READ_REG_1(hw, reg) \
inb((VIRTIO_PCI_REG_ADDR((hw), (reg))))
--
1.7.9.5
Yuanhan Liu
2015-12-16 14:23:26 UTC
Permalink
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 7:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
In x86 case io_base to store ioport address not more than 65535 ioports. i.e..0
to ffff but in non-x86 case in particular arm64 it need to store more than 32
bit address so changing io_base datatype from 32 to 64.
---
drivers/net/virtio/virtio_ethdev.c | 2 +-
drivers/net/virtio/virtio_pci.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d928339..620e0d4 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1291,7 +1291,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
return -1;
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 = (uint64_t)(uintptr_t)pci_dev->mem_resource[0].addr;
I'd suggest to move the io_base assignment (and cast) into virtio_ioport_init()
so that we could do the correct cast there, say cast it to uint32_t for
X86, and uint64_t for others.
Ok.
This was deliberately done considering your 1.0 virtio spec patch do
care for uint64_t types and in arm64 case, If I plan to use those
future patches, IMO it make more sense to me keep it in uint64_t way;
I did different cast, 32 bit for legacy virtio pci device, and 64 bit
for modern virtio pci device.
Post by Santosh Shukla
Also in x86 case max address could of type 0x1000-101f and so forth;
changing data-type to uint64_t default wont effect such address,
right?
Right, but what's the harm of doing the right cast? :)
Post by Santosh Shukla
And hw->io_base by looking at virtio_pci.h function like
inb/outb etc.. takes io_base address as unsigned long types which is
arch dependent; i.e.. 4 byte for 32 bit and 8 for 64 bit so the lower
level rd/wr apis are taking care of data-types accordingly.
Didn't get it. inb/outb takes "unsigned short" arguments, but not
"unsigned long".

--yliu
Santosh Shukla
2015-12-16 14:39:40 UTC
Permalink
On Wed, Dec 16, 2015 at 7:53 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 7:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
In x86 case io_base to store ioport address not more than 65535 ioports. i.e..0
to ffff but in non-x86 case in particular arm64 it need to store more than 32
bit address so changing io_base datatype from 32 to 64.
---
drivers/net/virtio/virtio_ethdev.c | 2 +-
drivers/net/virtio/virtio_pci.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d928339..620e0d4 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1291,7 +1291,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
return -1;
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 = (uint64_t)(uintptr_t)pci_dev->mem_resource[0].addr;
I'd suggest to move the io_base assignment (and cast) into virtio_ioport_init()
so that we could do the correct cast there, say cast it to uint32_t for
X86, and uint64_t for others.
Ok.
This was deliberately done considering your 1.0 virtio spec patch do
care for uint64_t types and in arm64 case, If I plan to use those
future patches, IMO it make more sense to me keep it in uint64_t way;
I did different cast, 32 bit for legacy virtio pci device, and 64 bit
for modern virtio pci device.
Post by Santosh Shukla
Also in x86 case max address could of type 0x1000-101f and so forth;
changing data-type to uint64_t default wont effect such address,
right?
Right, but what's the harm of doing the right cast? :)
Agree.
Post by Yuanhan Liu
Post by Santosh Shukla
And hw->io_base by looking at virtio_pci.h function like
inb/outb etc.. takes io_base address as unsigned long types which is
arch dependent; i.e.. 4 byte for 32 bit and 8 for 64 bit so the lower
level rd/wr apis are taking care of data-types accordingly.
Didn't get it. inb/outb takes "unsigned short" arguments, but not
"unsigned long".
sys/io.h in x86 case using unsigned short int types..

include/asm-generic/io.h for arm64 using it unsigned long (from linux
header files)

In such case keeping
#define VIRTIO_PCI_REG_ADDR(hw, reg) \
(unsigned short)((hw)->io_base + (reg))

would be x86 specific and what I thought and used in this patch is

#define VIRTIO_PCI_REG_ADDR(hw, reg) \
(unsigned long)((hw)->io_base + (reg))

to avoid ifdef ARM or non-x86..clutter, I know data-type is not right
fit for x86 sys/io.h but considering possible address inside
hw->io_base, wont effect functionality and performance my any mean.
That is why at virtio_ethdev_init() i choose to keep it in hw->io_base
= (uint64_t) types.

Otherwise I'll have to duplicate VIRTIO_PCI_REG_XXX definition for
non-x86 case, Pl. suggest better alternative. Thanks
Post by Yuanhan Liu
--yliu
Yuanhan Liu
2015-12-16 14:58:46 UTC
Permalink
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 7:53 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 7:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
In x86 case io_base to store ioport address not more than 65535 ioports. i.e..0
to ffff but in non-x86 case in particular arm64 it need to store more than 32
bit address so changing io_base datatype from 32 to 64.
---
drivers/net/virtio/virtio_ethdev.c | 2 +-
drivers/net/virtio/virtio_pci.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d928339..620e0d4 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1291,7 +1291,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
return -1;
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 = (uint64_t)(uintptr_t)pci_dev->mem_resource[0].addr;
I'd suggest to move the io_base assignment (and cast) into virtio_ioport_init()
so that we could do the correct cast there, say cast it to uint32_t for
X86, and uint64_t for others.
Ok.
This was deliberately done considering your 1.0 virtio spec patch do
care for uint64_t types and in arm64 case, If I plan to use those
future patches, IMO it make more sense to me keep it in uint64_t way;
I did different cast, 32 bit for legacy virtio pci device, and 64 bit
for modern virtio pci device.
Post by Santosh Shukla
Also in x86 case max address could of type 0x1000-101f and so forth;
changing data-type to uint64_t default wont effect such address,
right?
Right, but what's the harm of doing the right cast? :)
Agree.
Post by Yuanhan Liu
Post by Santosh Shukla
And hw->io_base by looking at virtio_pci.h function like
inb/outb etc.. takes io_base address as unsigned long types which is
arch dependent; i.e.. 4 byte for 32 bit and 8 for 64 bit so the lower
level rd/wr apis are taking care of data-types accordingly.
Didn't get it. inb/outb takes "unsigned short" arguments, but not
"unsigned long".
sys/io.h in x86 case using unsigned short int types..
include/asm-generic/io.h for arm64 using it unsigned long (from linux
header files)
In such case keeping
#define VIRTIO_PCI_REG_ADDR(hw, reg) \
(unsigned short)((hw)->io_base + (reg))
would be x86 specific and what I thought and used in this patch is
#define VIRTIO_PCI_REG_ADDR(hw, reg) \
(unsigned long)((hw)->io_base + (reg))
to avoid ifdef ARM or non-x86..clutter, I know data-type is not right
fit for x86 sys/io.h but considering possible address inside
hw->io_base, wont effect functionality and performance my any mean.
That is why at virtio_ethdev_init() i choose to keep it in hw->io_base
= (uint64_t) types.
Otherwise I'll have to duplicate VIRTIO_PCI_REG_XXX definition for
non-x86 case, Pl. suggest better alternative. Thanks
My understanding is that if you have done the right cast in the first
time (at the io_base assignment), casting from a short type to a longer
type will not matter: the upper bits will be filled with zero.

So, I guess we are fine here. I'm thinking that the extra cast in
VIRTIO_PCI_REG_ADDR() is not necessary, as C will do the right
cast for different inb(), say cast it to "unsigned short" for x86,
and "unsigned long" for your arm implementation. The same to
other io helpers.

--yliu
Santosh Shukla
2015-12-16 15:05:58 UTC
Permalink
On Wed, Dec 16, 2015 at 8:28 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 7:53 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 7:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
In x86 case io_base to store ioport address not more than 65535 ioports. i.e..0
to ffff but in non-x86 case in particular arm64 it need to store more than 32
bit address so changing io_base datatype from 32 to 64.
---
drivers/net/virtio/virtio_ethdev.c | 2 +-
drivers/net/virtio/virtio_pci.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d928339..620e0d4 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1291,7 +1291,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
return -1;
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 = (uint64_t)(uintptr_t)pci_dev->mem_resource[0].addr;
I'd suggest to move the io_base assignment (and cast) into virtio_ioport_init()
so that we could do the correct cast there, say cast it to uint32_t for
X86, and uint64_t for others.
Ok.
This was deliberately done considering your 1.0 virtio spec patch do
care for uint64_t types and in arm64 case, If I plan to use those
future patches, IMO it make more sense to me keep it in uint64_t way;
I did different cast, 32 bit for legacy virtio pci device, and 64 bit
for modern virtio pci device.
Post by Santosh Shukla
Also in x86 case max address could of type 0x1000-101f and so forth;
changing data-type to uint64_t default wont effect such address,
right?
Right, but what's the harm of doing the right cast? :)
Agree.
Post by Yuanhan Liu
Post by Santosh Shukla
And hw->io_base by looking at virtio_pci.h function like
inb/outb etc.. takes io_base address as unsigned long types which is
arch dependent; i.e.. 4 byte for 32 bit and 8 for 64 bit so the lower
level rd/wr apis are taking care of data-types accordingly.
Didn't get it. inb/outb takes "unsigned short" arguments, but not
"unsigned long".
sys/io.h in x86 case using unsigned short int types..
include/asm-generic/io.h for arm64 using it unsigned long (from linux
header files)
In such case keeping
#define VIRTIO_PCI_REG_ADDR(hw, reg) \
(unsigned short)((hw)->io_base + (reg))
would be x86 specific and what I thought and used in this patch is
#define VIRTIO_PCI_REG_ADDR(hw, reg) \
(unsigned long)((hw)->io_base + (reg))
to avoid ifdef ARM or non-x86..clutter, I know data-type is not right
fit for x86 sys/io.h but considering possible address inside
hw->io_base, wont effect functionality and performance my any mean.
That is why at virtio_ethdev_init() i choose to keep it in hw->io_base
= (uint64_t) types.
Otherwise I'll have to duplicate VIRTIO_PCI_REG_XXX definition for
non-x86 case, Pl. suggest better alternative. Thanks
My understanding is that if you have done the right cast in the first
time (at the io_base assignment), casting from a short type to a longer
type will not matter: the upper bits will be filled with zero.
So, I guess we are fine here. I'm thinking that the extra cast in
VIRTIO_PCI_REG_ADDR() is not necessary, as C will do the right
cast for different inb(), say cast it to "unsigned short" for x86,
and "unsigned long" for your arm implementation. The same to
other io helpers.
so to summarize and correct me if i misunderstood,
keep hw->io_base = (uint64_t)
and remove extra cast {i.e.. (unsigned short) for x86 or (unsigned
long) for non-x86/arm64 case} in VIRTIO_PCI_REG_ADDR().

did I got everything alright?
Post by Yuanhan Liu
--yliu
Yuanhan Liu
2015-12-17 07:19:27 UTC
Permalink
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 8:28 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 7:53 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 7:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
In x86 case io_base to store ioport address not more than 65535 ioports. i.e..0
to ffff but in non-x86 case in particular arm64 it need to store more than 32
bit address so changing io_base datatype from 32 to 64.
---
drivers/net/virtio/virtio_ethdev.c | 2 +-
drivers/net/virtio/virtio_pci.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d928339..620e0d4 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1291,7 +1291,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
return -1;
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 = (uint64_t)(uintptr_t)pci_dev->mem_resource[0].addr;
I'd suggest to move the io_base assignment (and cast) into virtio_ioport_init()
so that we could do the correct cast there, say cast it to uint32_t for
X86, and uint64_t for others.
Ok.
This was deliberately done considering your 1.0 virtio spec patch do
care for uint64_t types and in arm64 case, If I plan to use those
future patches, IMO it make more sense to me keep it in uint64_t way;
I did different cast, 32 bit for legacy virtio pci device, and 64 bit
for modern virtio pci device.
Post by Santosh Shukla
Also in x86 case max address could of type 0x1000-101f and so forth;
changing data-type to uint64_t default wont effect such address,
right?
Right, but what's the harm of doing the right cast? :)
Agree.
Post by Yuanhan Liu
Post by Santosh Shukla
And hw->io_base by looking at virtio_pci.h function like
inb/outb etc.. takes io_base address as unsigned long types which is
arch dependent; i.e.. 4 byte for 32 bit and 8 for 64 bit so the lower
level rd/wr apis are taking care of data-types accordingly.
Didn't get it. inb/outb takes "unsigned short" arguments, but not
"unsigned long".
sys/io.h in x86 case using unsigned short int types..
include/asm-generic/io.h for arm64 using it unsigned long (from linux
header files)
In such case keeping
#define VIRTIO_PCI_REG_ADDR(hw, reg) \
(unsigned short)((hw)->io_base + (reg))
would be x86 specific and what I thought and used in this patch is
#define VIRTIO_PCI_REG_ADDR(hw, reg) \
(unsigned long)((hw)->io_base + (reg))
to avoid ifdef ARM or non-x86..clutter, I know data-type is not right
fit for x86 sys/io.h but considering possible address inside
hw->io_base, wont effect functionality and performance my any mean.
That is why at virtio_ethdev_init() i choose to keep it in hw->io_base
= (uint64_t) types.
Otherwise I'll have to duplicate VIRTIO_PCI_REG_XXX definition for
non-x86 case, Pl. suggest better alternative. Thanks
My understanding is that if you have done the right cast in the first
time (at the io_base assignment), casting from a short type to a longer
type will not matter: the upper bits will be filled with zero.
So, I guess we are fine here. I'm thinking that the extra cast in
VIRTIO_PCI_REG_ADDR() is not necessary, as C will do the right
cast for different inb(), say cast it to "unsigned short" for x86,
and "unsigned long" for your arm implementation. The same to
other io helpers.
so to summarize and correct me if i misunderstood,
keep hw->io_base = (uint64_t)
I still want a different explicit cast for x86 and non-x86. And
actually, we should cast it to (unsigned short) but not (uint32_t)
for x86, don't we?

On the other hand, we may cast it to uint64_t unconditionally,
and then have an explicit sanity check for io_base for x86, say

if ((unsigned short)hw->io_base != hw->io_base) {
PMD_INIT_LOG(ERR, "invalid io port: %"PRIx64, ...);
return -1;
}

It's better than the (unsigned short) cast, as the later simply hides
issue when something went wrong, though it's not rare.

What do you think of that?
Post by Santosh Shukla
and remove extra cast {i.e.. (unsigned short) for x86 or (unsigned
long) for non-x86/arm64 case} in VIRTIO_PCI_REG_ADDR().
Technically speaking, yes, we don't need this kind of cast.

--yliu
Santosh Shukla
2015-12-17 08:17:56 UTC
Permalink
On Thu, Dec 17, 2015 at 12:49 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 8:28 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 7:53 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 7:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
In x86 case io_base to store ioport address not more than 65535 ioports. i.e..0
to ffff but in non-x86 case in particular arm64 it need to store more than 32
bit address so changing io_base datatype from 32 to 64.
---
drivers/net/virtio/virtio_ethdev.c | 2 +-
drivers/net/virtio/virtio_pci.h | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index d928339..620e0d4 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1291,7 +1291,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
return -1;
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 = (uint64_t)(uintptr_t)pci_dev->mem_resource[0].addr;
I'd suggest to move the io_base assignment (and cast) into virtio_ioport_init()
so that we could do the correct cast there, say cast it to uint32_t for
X86, and uint64_t for others.
Ok.
This was deliberately done considering your 1.0 virtio spec patch do
care for uint64_t types and in arm64 case, If I plan to use those
future patches, IMO it make more sense to me keep it in uint64_t way;
I did different cast, 32 bit for legacy virtio pci device, and 64 bit
for modern virtio pci device.
Post by Santosh Shukla
Also in x86 case max address could of type 0x1000-101f and so forth;
changing data-type to uint64_t default wont effect such address,
right?
Right, but what's the harm of doing the right cast? :)
Agree.
Post by Yuanhan Liu
Post by Santosh Shukla
And hw->io_base by looking at virtio_pci.h function like
inb/outb etc.. takes io_base address as unsigned long types which is
arch dependent; i.e.. 4 byte for 32 bit and 8 for 64 bit so the lower
level rd/wr apis are taking care of data-types accordingly.
Didn't get it. inb/outb takes "unsigned short" arguments, but not
"unsigned long".
sys/io.h in x86 case using unsigned short int types..
include/asm-generic/io.h for arm64 using it unsigned long (from linux
header files)
In such case keeping
#define VIRTIO_PCI_REG_ADDR(hw, reg) \
(unsigned short)((hw)->io_base + (reg))
would be x86 specific and what I thought and used in this patch is
#define VIRTIO_PCI_REG_ADDR(hw, reg) \
(unsigned long)((hw)->io_base + (reg))
to avoid ifdef ARM or non-x86..clutter, I know data-type is not right
fit for x86 sys/io.h but considering possible address inside
hw->io_base, wont effect functionality and performance my any mean.
That is why at virtio_ethdev_init() i choose to keep it in hw->io_base
= (uint64_t) types.
Otherwise I'll have to duplicate VIRTIO_PCI_REG_XXX definition for
non-x86 case, Pl. suggest better alternative. Thanks
My understanding is that if you have done the right cast in the first
time (at the io_base assignment), casting from a short type to a longer
type will not matter: the upper bits will be filled with zero.
So, I guess we are fine here. I'm thinking that the extra cast in
VIRTIO_PCI_REG_ADDR() is not necessary, as C will do the right
cast for different inb(), say cast it to "unsigned short" for x86,
and "unsigned long" for your arm implementation. The same to
other io helpers.
so to summarize and correct me if i misunderstood,
keep hw->io_base = (uint64_t)
I still want a different explicit cast for x86 and non-x86. And
actually, we should cast it to (unsigned short) but not (uint32_t)
for x86, don't we?
On the other hand, we may cast it to uint64_t unconditionally,
and then have an explicit sanity check for io_base for x86, say
if ((unsigned short)hw->io_base != hw->io_base) {
PMD_INIT_LOG(ERR, "invalid io port: %"PRIx64, ...);
return -1;
}
It's better than the (unsigned short) cast, as the later simply hides
issue when something went wrong, though it's not rare.
What do you think of that?
Liked the later part which is casting uint64_t unconditionally and
keeping a (unsigned short) check for x86 case. We'll do it in v3.
Post by Yuanhan Liu
Post by Santosh Shukla
and remove extra cast {i.e.. (unsigned short) for x86 or (unsigned
long) for non-x86/arm64 case} in VIRTIO_PCI_REG_ADDR().
Technically speaking, yes, we don't need this kind of cast.
Ok.
Post by Yuanhan Liu
--yliu
Santosh Shukla
2015-12-14 13:00:25 UTC
Permalink
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 <***@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 cbebd64..d840dc2 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

@@ -70,7 +75,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
Jerin Jacob
2015-12-14 14:31:29 UTC
Permalink
Post by Santosh Shukla
Enable RTE_LIBRTE_VIRTIO_PMD for armv7/v8 and setting RTE_VIRTIO_INC_VEC=n.
Builds successfully for armv7/v8.
---
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 cbebd64..d840dc2 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
+
Enable the config at last in the patch series, so that it won't
functionally break for arm/arm64 on this patch.
Post by Santosh Shukla
# ARM doesn't have support for vmware TSC map
CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=n
@@ -70,7 +75,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
Santosh Shukla
2015-12-14 16:16:43 UTC
Permalink
On Mon, Dec 14, 2015 at 8:01 PM, Jerin Jacob
Post by Jerin Jacob
Post by Santosh Shukla
Enable RTE_LIBRTE_VIRTIO_PMD for armv7/v8 and setting RTE_VIRTIO_INC_VEC=n.
Builds successfully for armv7/v8.
---
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 cbebd64..d840dc2 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
+
Enable the config at last in the patch series, so that it won't
functionally break for arm/arm64 on this patch.
ok, we'll keep it at top in v3. Thanks
Post by Jerin Jacob
Post by Santosh Shukla
# ARM doesn't have support for vmware TSC map
CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=n
@@ -70,7 +75,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
Jianbo Liu
2015-12-15 05:36:55 UTC
Permalink
Post by Santosh Shukla
Enable RTE_LIBRTE_VIRTIO_PMD for armv7/v8 and setting RTE_VIRTIO_INC_VEC=n.
Builds successfully for armv7/v8.
---
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 cbebd64..d840dc2 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
I don't think the above line is needed since already enabled in
config/common_linuxapp.
Post by Santosh Shukla
+# Disable VIRTIO VECTOR support
+CONFIG_RTE_VIRTIO_INC_VECTOR=n
+
# ARM doesn't have support for vmware TSC map
CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=n
@@ -70,7 +75,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
Santosh Shukla
2015-12-14 13:00:26 UTC
Permalink
iopl() syscall not supported in linux-arm/arm64 so always return 0 value.

Signed-off-by: Santosh Shukla <***@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
Jan Viktorin
2015-12-14 14:34:21 UTC
Permalink
I believe, I've already acked this patch. I can see no change here so I
assume it's still the same.

On Mon, 14 Dec 2015 18:30:26 +0530
Post by Santosh Shukla
iopl() syscall not supported in linux-arm/arm64 so always return 0 value.
Acked-by: Jan Viktorin <***@rehivetech.com>
Santosh Shukla
2015-12-14 15:04:27 UTC
Permalink
Post by Jan Viktorin
I believe, I've already acked this patch. I can see no change here so I
assume it's still the same.
On Mon, 14 Dec 2015 18:30:26 +0530
Post by Santosh Shukla
iopl() syscall not supported in linux-arm/arm64 so always return 0 value.
Sorry that I missed in v2 patch! Gonna add in next revision. Thanks
Jerin Jacob
2015-12-14 14:37:18 UTC
Permalink
Post by Santosh Shukla
iopl() syscall not supported in linux-arm/arm64 so always return 0 value.
---
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 */
I guess for other architectures also iopl not supported.I think better
to move this function to eal. Else this function will return 'true' for
ppc64

or have at least postive logic,
#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686) ||
defined(RTE_ARCH_X86_X32)
Post by Santosh Shukla
+#endif
return -1;
#endif
}
--
1.7.9.5
Santosh Shukla
2015-12-14 15:24:08 UTC
Permalink
On Mon, Dec 14, 2015 at 8:07 PM, Jerin Jacob
Post by Jerin Jacob
Post by Santosh Shukla
iopl() syscall not supported in linux-arm/arm64 so always return 0 value.
---
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 */
I guess for other architectures also iopl not supported.I think better
to move this function to eal. Else this function will return 'true' for
ppc64
didn't understood. This func is in eal right? and for ppc64, function
will return -1 (false). Although i could include ppc64 / tile or
invert the logic such a way that non-x86 arch to return default true
value.

However iopl() used for virtio and only two arch using x86/ now arm. I
am not sure ppc64/tile or other arch has any plan to use virtio pmd
thus care for iopl().
Post by Jerin Jacob
or have at least postive logic,
#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686) ||
defined(RTE_ARCH_X86_X32)
Post by Santosh Shukla
+#endif
return -1;
#endif
}
--
1.7.9.5
Jerin Jacob
2015-12-14 15:56:07 UTC
Permalink
Post by Santosh Shukla
On Mon, Dec 14, 2015 at 8:07 PM, Jerin Jacob
Post by Jerin Jacob
Post by Santosh Shukla
iopl() syscall not supported in linux-arm/arm64 so always return 0 value.
---
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 */
I guess for other architectures also iopl not supported.I think better
to move this function to eal. Else this function will return 'true' for
ppc64
didn't understood. This func is in eal right? and for ppc64, function
meant to abstract through lib/librte_eal/common/include/arch/
to avoid #ifdef clutter
Post by Santosh Shukla
will return -1 (false). Although i could include ppc64 / tile or
invert the logic such a way that non-x86 arch to return default true
value.
However iopl() used for virtio and only two arch using x86/ now arm. I
am not sure ppc64/tile or other arch has any plan to use virtio pmd
thus care for iopl().
Why not? With your patch, dpdk-virtio has very minimal dependency on
architecture (implementing raw_*) or even we can have generic routine for that
Post by Santosh Shukla
Post by Jerin Jacob
or have at least postive logic,
#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686) ||
defined(RTE_ARCH_X86_X32)
Post by Santosh Shukla
+#endif
return -1;
#endif
}
--
1.7.9.5
Santosh Shukla
2015-12-14 16:13:30 UTC
Permalink
On Mon, Dec 14, 2015 at 9:26 PM, Jerin Jacob
Post by Jerin Jacob
Post by Santosh Shukla
On Mon, Dec 14, 2015 at 8:07 PM, Jerin Jacob
Post by Jerin Jacob
Post by Santosh Shukla
iopl() syscall not supported in linux-arm/arm64 so always return 0 value.
---
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 */
I guess for other architectures also iopl not supported.I think better
to move this function to eal. Else this function will return 'true' for
ppc64
didn't understood. This func is in eal right? and for ppc64, function
meant to abstract through lib/librte_eal/common/include/arch/
to avoid #ifdef clutter
make sense to me :)
Post by Jerin Jacob
Post by Santosh Shukla
will return -1 (false). Although i could include ppc64 / tile or
invert the logic such a way that non-x86 arch to return default true
value.
However iopl() used for virtio and only two arch using x86/ now arm. I
am not sure ppc64/tile or other arch has any plan to use virtio pmd
thus care for iopl().
Why not? With your patch, dpdk-virtio has very minimal dependency on
architecture (implementing raw_*) or even we can have generic routine for that
Right!

We'll do in v3, Thanks!!
Post by Jerin Jacob
Post by Santosh Shukla
Post by Jerin Jacob
or have at least postive logic,
#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686) ||
defined(RTE_ARCH_X86_X32)
Post by Santosh Shukla
+#endif
return -1;
#endif
}
--
1.7.9.5
Santosh Shukla
2015-12-14 13:00:27 UTC
Permalink
Replace sys/io.h with rte_io.h, Get rid of ifdef X86 clutter

Signed-off-by: Santosh Shukla <***@mvista.com>
---
drivers/net/virtio/virtio_pci.h | 4 ---
lib/librte_eal/common/include/arch/x86/rte_io.h | 42 +++++++++++++++++++++++
lib/librte_eal/linuxapp/eal/eal.c | 4 +--
3 files changed, 43 insertions(+), 7 deletions(-)
create mode 100644 lib/librte_eal/common/include/arch/x86/rte_io.h

diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index f3e4178..c51d980 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -40,12 +40,8 @@
#include <sys/types.h>
#include <machine/cpufunc.h>
#else
-#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
-#include <sys/io.h>
-#else
#include <rte_io.h>
#endif
-#endif

#include <rte_ethdev.h>

diff --git a/lib/librte_eal/common/include/arch/x86/rte_io.h b/lib/librte_eal/common/include/arch/x86/rte_io.h
new file mode 100644
index 0000000..ffd6888
--- /dev/null
+++ b/lib/librte_eal/common/include/arch/x86/rte_io.h
@@ -0,0 +1,42 @@
+/*
+ * 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.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_IO_X86_H_
+#define _RTE_IO_X86_H_
+
+#include "generic/rte_io.h"
+
+#endif /* _RTE_IO_X86_H_ */
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 2617037..29950f3 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -49,9 +49,7 @@
#include <errno.h>
#include <sys/mman.h>
#include <sys/queue.h>
-#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
-#include <sys/io.h>
-#endif
+#include <rte_io.h>

#include <rte_common.h>
#include <rte_debug.h>
--
1.7.9.5
Santosh Shukla
2015-12-14 13:00:28 UTC
Permalink
Module maps iopci region by creating misc device file /dev/igb_ioport.
Applicable for non-x86 arch, tested for arm64/ThuderX platform.
Including three api to register/unregister ioport misc device
- igbuio_ioport_register
- igbuio_ioport_unregister
- igbuio_iomap

Signed-off-by: Santosh Shukla <***@mvista.com>
Signed-off-by: Rizwan Ansari <***@mvista.com>
Signed-off-by: Rakesh Krishnamurthy <***@mvista.com>
---
lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 8 +-
.../linuxapp/igb_uio/igbuio_ioport_misc.h | 133 ++++++++++++++++++++
2 files changed, 140 insertions(+), 1 deletion(-)
create mode 100644 lib/librte_eal/linuxapp/igb_uio/igbuio_ioport_misc.h

diff --git a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
index f5617d2..155bf39 100644
--- a/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
+++ b/lib/librte_eal/linuxapp/igb_uio/igb_uio.c
@@ -39,6 +39,7 @@
#include <rte_pci_dev_features.h>

#include "compat.h"
+#include "igbuio_ioport_misc.h"

#ifdef RTE_PCI_CONFIG
#define PCI_SYS_FILE_BUF_SIZE 10
@@ -366,7 +367,7 @@ igbuio_pci_setup_ioport(struct pci_dev *dev, struct uio_info *info,
return -EINVAL;

info->port[n].name = name;
- info->port[n].start = addr;
+ info->port[n].start = igbuio_iomap(addr);
info->port[n].size = len;
info->port[n].porttype = UIO_PORT_X86;

@@ -615,6 +616,10 @@ igbuio_pci_init_module(void)
{
int ret;

+ ret = igbuio_ioport_register();
+ if (ret < 0)
+ return ret;
+
ret = igbuio_config_intr_mode(intr_mode);
if (ret < 0)
return ret;
@@ -625,6 +630,7 @@ igbuio_pci_init_module(void)
static void __exit
igbuio_pci_exit_module(void)
{
+ igbuio_ioport_unregister();
pci_unregister_driver(&igbuio_pci_driver);
}

diff --git a/lib/librte_eal/linuxapp/igb_uio/igbuio_ioport_misc.h b/lib/librte_eal/linuxapp/igb_uio/igbuio_ioport_misc.h
new file mode 100644
index 0000000..04e2c28
--- /dev/null
+++ b/lib/librte_eal/linuxapp/igb_uio/igbuio_ioport_misc.h
@@ -0,0 +1,133 @@
+/*-
+ * GPL LICENSE SUMMARY
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2 of the GNU General Public License as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ * The full GNU General Public License is included in this distribution
+ * in the file called LICENSE.GPL.
+ *
+ * Contact Information:
+ * Cavium Networks
+ */
+
+#ifndef _IGBUIO_IOPORT_MISC_H_
+#define _IGBUIO_IOPORT_MISC_H_
+
+unsigned long igbuio_iomap(unsigned long addr);
+int igbuio_ioport_register(void);
+void igbuio_ioport_unregister(void);
+
+#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 <asm/io.h>
+#include <linux/sched.h>
+#include <linux/pid.h>
+
+void *__iomem mapped_io; /* ioport addr of `cat /proc/ioport` */
+
+static int igbuio_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 igbuio_ioport_fops = {
+ .mmap = igbuio_ioport_mmap,
+};
+
+static struct miscdevice igbuio_ioport_dev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "igb_ioport",
+ .fops = &igbuio_ioport_fops
+};
+
+unsigned long igbuio_iomap(unsigned long addr)
+{
+ mapped_io = ioport_map(addr, 0);
+ return (unsigned long)(uintptr_t)mapped_io;
+}
+
+int igbuio_ioport_register(void)
+{
+ int ret;
+ ret = misc_register(&igbuio_ioport_dev);
+ if (ret < 0) {
+ pr_info("Error: failed to register ioport map driver (%d)\n",
+ ret);
+ return ret;
+ }
+ return ret;
+}
+
+void igbuio_ioport_unregister(void)
+{
+ misc_deregister(&igbuio_ioport_dev);
+}
+
+#else /* !CONFIG_HAS_IOPORT_MAP */
+
+#error "CONFIG_HAS_IOPORT_MAP not supported for $RTE_ARCH"
+
+#endif /* CONFIG_HAS_IOPORT_MAP */
+
+#else /* !RTE_ARCH_ARM, !RTE_ARCH_ARM64 */
+
+unsigned long igbuio_iomap(unsigned long addr)
+{
+ /* non-arm case : simply return addr */
+ return addr;
+}
+
+int igbuio_ioport_register(void)
+{
+ /* non-arm case : do nothing */
+ return 0;
+}
+
+void igbuio_ioport_unregister(void)
+{
+ /* non-arm case : do nothing */
+ return;
+}
+
+#endif /* RTE_ARCH_ARM , RTE_ARCH_ARM64 */
+
+#endif /* _IGBUIO_IOPORT_MISC_H_ */
--
1.7.9.5
Santosh Shukla
2015-12-14 13:00:29 UTC
Permalink
including virtio_ioport header file has iopci bar page_size, bar_len and
device filename info.

Signed-off-by: Santosh Shukla <***@mvista.com>
---
lib/librte_eal/linuxapp/eal/Makefile | 2 +-
.../eal/include/exec-env/rte_virt_ioport.h | 81 ++++++++++++++++++++
2 files changed, 82 insertions(+), 1 deletion(-)
create mode 100644 lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h

diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 26eced5..1252e05 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -115,7 +115,7 @@ ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
CFLAGS_eal_thread.o += -Wno-return-type
endif

-INC := rte_interrupts.h rte_kni_common.h rte_dom0_common.h
+INC := rte_interrupts.h rte_kni_common.h rte_dom0_common.h rte_virt_ioport.h

SYMLINK-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP)-include/exec-env := \
$(addprefix include/exec-env/,$(INC))
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h
new file mode 100644
index 0000000..8098eaf
--- /dev/null
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h
@@ -0,0 +1,81 @@
+/*
+ * This file is provided under a dual BSD/LGPLv2 license. When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GNU LESSER GENERAL PUBLIC LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2.1 of the GNU Lesser General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Contact Information:
+ * Cavium Networks
+ *
+ *
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+#ifndef _RTE_VIRTIO_IOPORT_H_
+#define _RTE_VIRTIO_IOPORT_H_
+
+#define VIRT_IOPORT_DEV "/dev/igb_ioport"
+
+/**
+ * Keeping pci_virt_ioport_size = 4k.
+ * So maximum mmaped pci_iobar supported =
+ * (virt_ioport_size/pci_dev->mem_resource[0].len)
+ * where, pci_dev->mem_resource[0].len == virtio_pci_iobar_len i.e.. 32 byte
+ *
+ * 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_VIRT_IOPORT_SIZE PAGE_SIZE
+#define PCI_VIRT_IOBAR_LEN 32
+#define PCI_VIRT_IOPORT_MAX (PCI_VIRT_IOPORT_SIZE/PCI_VIRT_IOBAR_LEN)
+ /* 128 max ioport */
+
+#endif /* _RTE_VIRTIO_IOPORT_H_ */
--
1.7.9.5
Jerin Jacob
2015-12-14 14:43:58 UTC
Permalink
Post by Santosh Shukla
including virtio_ioport header file has iopci bar page_size, bar_len and
device filename info.
---
lib/librte_eal/linuxapp/eal/Makefile | 2 +-
.../eal/include/exec-env/rte_virt_ioport.h | 81 ++++++++++++++++++++
2 files changed, 82 insertions(+), 1 deletion(-)
create mode 100644 lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 26eced5..1252e05 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -115,7 +115,7 @@ ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
CFLAGS_eal_thread.o += -Wno-return-type
endif
-INC := rte_interrupts.h rte_kni_common.h rte_dom0_common.h
+INC := rte_interrupts.h rte_kni_common.h rte_dom0_common.h rte_virt_ioport.h
SYMLINK-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP)-include/exec-env := \
$(addprefix include/exec-env/,$(INC))
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h
new file mode 100644
index 0000000..8098eaf
--- /dev/null
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h
@@ -0,0 +1,81 @@
+/*
+ * This file is provided under a dual BSD/LGPLv2 license. When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GNU LESSER GENERAL PUBLIC LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2.1 of the GNU Lesser General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Cavium Networks
+ *
+ *
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+#ifndef _RTE_VIRTIO_IOPORT_H_
+#define _RTE_VIRTIO_IOPORT_H_
+
+#define VIRT_IOPORT_DEV "/dev/igb_ioport"
+
+/**
+ * Keeping pci_virt_ioport_size = 4k.
+ * So maximum mmaped pci_iobar supported =
+ * (virt_ioport_size/pci_dev->mem_resource[0].len)
+ * where, pci_dev->mem_resource[0].len == virtio_pci_iobar_len i.e.. 32 byte
+ *
+ * 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
IMO, We can remove this PAGE_SIZE macro and set PCI_VIRT_IOPORT_SIZE as
4096 directly. It will remove the confusion between
kernel page size
Post by Santosh Shukla
+#define PCI_VIRT_IOPORT_SIZE PAGE_SIZE
+#define PCI_VIRT_IOBAR_LEN 32
+#define PCI_VIRT_IOPORT_MAX (PCI_VIRT_IOPORT_SIZE/PCI_VIRT_IOBAR_LEN)
+ /* 128 max ioport */
+
+#endif /* _RTE_VIRTIO_IOPORT_H_ */
--
1.7.9.5
Santosh Shukla
2015-12-14 16:17:30 UTC
Permalink
On Mon, Dec 14, 2015 at 8:13 PM, Jerin Jacob
Post by Jerin Jacob
Post by Santosh Shukla
including virtio_ioport header file has iopci bar page_size, bar_len and
device filename info.
---
lib/librte_eal/linuxapp/eal/Makefile | 2 +-
.../eal/include/exec-env/rte_virt_ioport.h | 81 ++++++++++++++++++++
2 files changed, 82 insertions(+), 1 deletion(-)
create mode 100644 lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 26eced5..1252e05 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -115,7 +115,7 @@ ifeq ($(CONFIG_RTE_TOOLCHAIN_GCC),y)
CFLAGS_eal_thread.o += -Wno-return-type
endif
-INC := rte_interrupts.h rte_kni_common.h rte_dom0_common.h
+INC := rte_interrupts.h rte_kni_common.h rte_dom0_common.h rte_virt_ioport.h
SYMLINK-$(CONFIG_RTE_LIBRTE_EAL_LINUXAPP)-include/exec-env := \
$(addprefix include/exec-env/,$(INC))
diff --git a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h
new file mode 100644
index 0000000..8098eaf
--- /dev/null
+++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h
@@ -0,0 +1,81 @@
+/*
+ * This file is provided under a dual BSD/LGPLv2 license. When using or
+ * redistributing this file, you may do so under either license.
+ *
+ * GNU LESSER GENERAL PUBLIC LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of version 2.1 of the GNU Lesser General Public License
+ * as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St - Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Cavium Networks
+ *
+ *
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+#ifndef _RTE_VIRTIO_IOPORT_H_
+#define _RTE_VIRTIO_IOPORT_H_
+
+#define VIRT_IOPORT_DEV "/dev/igb_ioport"
+
+/**
+ * Keeping pci_virt_ioport_size = 4k.
+ * So maximum mmaped pci_iobar supported =
+ * (virt_ioport_size/pci_dev->mem_resource[0].len)
+ * where, pci_dev->mem_resource[0].len == virtio_pci_iobar_len i.e.. 32 byte
+ *
+ * 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
IMO, We can remove this PAGE_SIZE macro and set PCI_VIRT_IOPORT_SIZE as
4096 directly. It will remove the confusion between
kernel page size
yes.
Post by Jerin Jacob
Post by Santosh Shukla
+#define PCI_VIRT_IOPORT_SIZE PAGE_SIZE
+#define PCI_VIRT_IOBAR_LEN 32
+#define PCI_VIRT_IOPORT_MAX (PCI_VIRT_IOPORT_SIZE/PCI_VIRT_IOBAR_LEN)
+ /* 128 max ioport */
+
+#endif /* _RTE_VIRTIO_IOPORT_H_ */
--
1.7.9.5
Santosh Shukla
2015-12-14 13:00:30 UTC
Permalink
Introducing module to mmap iopci bar region. Applicable for linuxapp for non-x86
archs, Tested for arm64/ThunderX platform for linux. For that adding two global
api.
- virtio_ioport_init
- virtio_ioport_unmap

Signed-off-by: Santosh Shukla <***@mvista.com>
Signed-off-by: Rizwan Ansari <***@mvista.com>
Signed-off-by: Rakesh Krishnamurthy <***@mvista.com>
---
drivers/net/virtio/Makefile | 1 +
drivers/net/virtio/virtio_ioport.c | 163 ++++++++++++++++++++++++++++++++++++
drivers/net/virtio/virtio_ioport.h | 42 ++++++++++
3 files changed, 206 insertions(+)
create mode 100644 drivers/net/virtio/virtio_ioport.c
create mode 100644 drivers/net/virtio/virtio_ioport.h

diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
index 25a842d..5cba6d3 100644
--- a/drivers/net/virtio/Makefile
+++ b/drivers/net/virtio/Makefile
@@ -50,6 +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_ioport.c
SRCS-$(CONFIG_RTE_VIRTIO_INC_VECTOR) += virtio_rxtx_simple.c

# this lib depends upon:
diff --git a/drivers/net/virtio/virtio_ioport.c b/drivers/net/virtio/virtio_ioport.c
new file mode 100644
index 0000000..ffeb8e9
--- /dev/null
+++ b/drivers/net/virtio/virtio_ioport.c
@@ -0,0 +1,163 @@
+/*
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#include "virtio_ioport.h"
+
+#if defined(RTE_EXEC_ENV_LINUXAPP) && (defined(RTE_ARCH_ARM) || \
+ defined(RTE_ARCH_ARM64))
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#include <exec-env/rte_virt_ioport.h>
+
+#include "virtio_logs.h"
+
+/* start address of first pci_iobar slot (user-space virtual-addres) */
+void *ioport_map;
+/**
+ * ioport map count,
+ * Use-case: virtio-net-pci.
+ * Keeps track of number of virtio-net-pci device mapped/unmapped. Max device
+ * support by linux kernel is 31, so ioport_map_cnt can not be greater than 31.
+ */
+static int ioport_map_cnt;
+
+static int
+virtio_map_ioport(void **resource_addr)
+{
+ int fd;
+ int ret = 0;
+
+ /* avoid -Werror=unused-parameter, keep compiler happy */
+ (void)resource_addr;
+ fd = open(VIRT_IOPORT_DEV, 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_VIRT_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_VIRT_IOPORT_MAX) {
+ ret = -1;
+ PMD_INIT_LOG(ERR,
+ "ioport_map_cnt(%d) greater than"
+ "PCI_VIRT_IOPORT_MAX(%d)\n",
+ ioport_map_cnt, PCI_VIRT_IOPORT_MAX);
+ return ret;
+ }
+ *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);
+ return ret;
+}
+
+int virtio_ioport_init(struct rte_pci_resource *mem_resource)
+{
+ 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 for
+ * each ioport (i.e..pci_dev->mem_resource[0].addr)
+ */
+ if (!ioport_map) {
+ ret = virtio_map_ioport(&mem_resource->addr);
+ if (ret < 0)
+ return ret;
+ PMD_INIT_LOG(INFO, "ioport_map %p\n", ioport_map);
+ }
+
+ ret = virtio_set_ioport_addr(&mem_resource->addr, mem_resource->len);
+ if (ret < 0)
+ return ret;
+
+ PMD_INIT_LOG(INFO, "resource_addr %p resource_len :%ld\n",
+ mem_resource->addr, (unsigned long)mem_resource->len);
+ return ret;
+}
+
+void virtio_ioport_unmap(void)
+{
+ /* unmap ioport memory */
+ ioport_map_cnt--;
+ if (!ioport_map_cnt)
+ munmap(ioport_map, PCI_VIRT_IOPORT_SIZE);
+
+ PMD_INIT_LOG(DEBUG, "unmapping ioport_mem %d\n", ioport_map_cnt);
+}
+
+#else /* !LINUXAPP && !ARM/64 */
+
+int virtio_ioport_init(struct rte_pci_resource *mem_resource)
+{
+ (void)mem_resource;
+ return 0;
+}
+
+void virtio_ioport_unmap(void)
+{
+ return;
+}
+
+#endif /* LINUXAPP, ARM/64 */
diff --git a/drivers/net/virtio/virtio_ioport.h b/drivers/net/virtio/virtio_ioport.h
new file mode 100644
index 0000000..bf79551
--- /dev/null
+++ b/drivers/net/virtio/virtio_ioport.h
@@ -0,0 +1,42 @@
+/*
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+#ifndef _VIRTIO_IOPORT_H_
+#define _VIRTIO_IOPORT_H_
+
+#include <rte_pci.h>
+
+int virtio_ioport_init(struct rte_pci_resource *mem_resource);
+void virtio_ioport_unmap(void);
+
+#endif /* _VIRTIO_IOPORT_H_ */
--
1.7.9.5
Yuanhan Liu
2015-12-16 13:29:40 UTC
Permalink
Post by Santosh Shukla
Introducing module to mmap iopci bar region. Applicable for linuxapp for non-x86
archs, Tested for arm64/ThunderX platform for linux. For that adding two global
api.
- virtio_ioport_init
- virtio_ioport_unmap
---
drivers/net/virtio/Makefile | 1 +
drivers/net/virtio/virtio_ioport.c | 163 ++++++++++++++++++++++++++++++++++++
drivers/net/virtio/virtio_ioport.h | 42 ++++++++++
3 files changed, 206 insertions(+)
create mode 100644 drivers/net/virtio/virtio_ioport.c
create mode 100644 drivers/net/virtio/virtio_ioport.h
diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
index 25a842d..5cba6d3 100644
--- a/drivers/net/virtio/Makefile
+++ b/drivers/net/virtio/Makefile
@@ -50,6 +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_ioport.c
SRCS-$(CONFIG_RTE_VIRTIO_INC_VECTOR) += virtio_rxtx_simple.c
diff --git a/drivers/net/virtio/virtio_ioport.c b/drivers/net/virtio/virtio_ioport.c
new file mode 100644
index 0000000..ffeb8e9
--- /dev/null
+++ b/drivers/net/virtio/virtio_ioport.c
@@ -0,0 +1,163 @@
+/*
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#include "virtio_ioport.h"
+
+#if defined(RTE_EXEC_ENV_LINUXAPP) && (defined(RTE_ARCH_ARM) || \
+ defined(RTE_ARCH_ARM64))
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#include <exec-env/rte_virt_ioport.h>
+
+#include "virtio_logs.h"
+
+/* start address of first pci_iobar slot (user-space virtual-addres) */
+void *ioport_map;
You still forgot "static"?
Post by Santosh Shukla
+/**
+ * ioport map count,
+ * Use-case: virtio-net-pci.
How about removing above two lines; it's quite meaningless here, but
instead a bit redundant.
Post by Santosh Shukla
+ * Keeps track of number of virtio-net-pci device mapped/unmapped. Max device
+ * support by linux kernel is 31, so ioport_map_cnt can not be greater than 31.
+ */
+static int ioport_map_cnt;
+
+static int
+virtio_map_ioport(void **resource_addr)
+{
+ int fd;
+ int ret = 0;
+
+ /* avoid -Werror=unused-parameter, keep compiler happy */
+ (void)resource_addr;
Using __rte_unused is more elegant.
Post by Santosh Shukla
+ fd = open(VIRT_IOPORT_DEV, 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_VIRT_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);
+
+ close(fd);
+ return ret;
+}
+
+static int
+virtio_set_ioport_addr(void **resource_addr, unsigned long offset)
+{
+ int ret = 0;
+
+ if (ioport_map_cnt >= PCI_VIRT_IOPORT_MAX) {
+ ret = -1;
+ PMD_INIT_LOG(ERR,
+ "ioport_map_cnt(%d) greater than"
+ "PCI_VIRT_IOPORT_MAX(%d)\n",
+ ioport_map_cnt, PCI_VIRT_IOPORT_MAX);
+ return ret;
+ }
+ *resource_addr = (void *)((char *)ioport_map + (ioport_map_cnt)*offset);
Redundant (), and the void * cast seems to be unnecessary.
Post by Santosh Shukla
+ ioport_map_cnt++;
+
+ PMD_INIT_LOG(DEBUG, "pci.resource_addr %p ioport_map_cnt %d\n",
+ *resource_addr, ioport_map_cnt);
+ return ret;
+}
+
+int virtio_ioport_init(struct rte_pci_resource *mem_resource)
+{
+ 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 for
+ * each ioport (i.e..pci_dev->mem_resource[0].addr)
+ */
+ if (!ioport_map) {
+ ret = virtio_map_ioport(&mem_resource->addr);
+ if (ret < 0)
+ return ret;
+ PMD_INIT_LOG(INFO, "ioport_map %p\n", ioport_map);
+ }
+
+ ret = virtio_set_ioport_addr(&mem_resource->addr, mem_resource->len);
+ if (ret < 0)
+ return ret;
+
+ PMD_INIT_LOG(INFO, "resource_addr %p resource_len :%ld\n",
+ mem_resource->addr, (unsigned long)mem_resource->len);
+ return ret;
+}
+
+void virtio_ioport_unmap(void)
+{
+ /* unmap ioport memory */
Redundant comment.
Post by Santosh Shukla
+ ioport_map_cnt--;
+ if (!ioport_map_cnt)
+ munmap(ioport_map, PCI_VIRT_IOPORT_SIZE);
+
+ PMD_INIT_LOG(DEBUG, "unmapping ioport_mem %d\n", ioport_map_cnt);
+}
+
+#else /* !LINUXAPP && !ARM/64 */
+
+int virtio_ioport_init(struct rte_pci_resource *mem_resource)
+{
+ (void)mem_resource;
ditto.
Post by Santosh Shukla
+ return 0;
+}
+
+void virtio_ioport_unmap(void)
+{
+ return;
+}
+
+#endif /* LINUXAPP, ARM/64 */
diff --git a/drivers/net/virtio/virtio_ioport.h b/drivers/net/virtio/virtio_ioport.h
new file mode 100644
index 0000000..bf79551
--- /dev/null
+++ b/drivers/net/virtio/virtio_ioport.h
@@ -0,0 +1,42 @@
+/*
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+#ifndef _VIRTIO_IOPORT_H_
+#define _VIRTIO_IOPORT_H_
+
+#include <rte_pci.h>
+
+int virtio_ioport_init(struct rte_pci_resource *mem_resource);
+void virtio_ioport_unmap(void);
+
+#endif /* _VIRTIO_IOPORT_H_ */
--
1.7.9.5
Santosh Shukla
2015-12-16 14:20:51 UTC
Permalink
On Wed, Dec 16, 2015 at 6:59 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
Introducing module to mmap iopci bar region. Applicable for linuxapp for non-x86
archs, Tested for arm64/ThunderX platform for linux. For that adding two global
api.
- virtio_ioport_init
- virtio_ioport_unmap
---
drivers/net/virtio/Makefile | 1 +
drivers/net/virtio/virtio_ioport.c | 163 ++++++++++++++++++++++++++++++++++++
drivers/net/virtio/virtio_ioport.h | 42 ++++++++++
3 files changed, 206 insertions(+)
create mode 100644 drivers/net/virtio/virtio_ioport.c
create mode 100644 drivers/net/virtio/virtio_ioport.h
diff --git a/drivers/net/virtio/Makefile b/drivers/net/virtio/Makefile
index 25a842d..5cba6d3 100644
--- a/drivers/net/virtio/Makefile
+++ b/drivers/net/virtio/Makefile
@@ -50,6 +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_ioport.c
SRCS-$(CONFIG_RTE_VIRTIO_INC_VECTOR) += virtio_rxtx_simple.c
diff --git a/drivers/net/virtio/virtio_ioport.c b/drivers/net/virtio/virtio_ioport.c
new file mode 100644
index 0000000..ffeb8e9
--- /dev/null
+++ b/drivers/net/virtio/virtio_ioport.c
@@ -0,0 +1,163 @@
+/*
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+
+#include "virtio_ioport.h"
+
+#if defined(RTE_EXEC_ENV_LINUXAPP) && (defined(RTE_ARCH_ARM) || \
+ defined(RTE_ARCH_ARM64))
+
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <unistd.h>
+#include <sys/mman.h>
+#include <exec-env/rte_virt_ioport.h>
+
+#include "virtio_logs.h"
+
+/* start address of first pci_iobar slot (user-space virtual-addres) */
+void *ioport_map;
You still forgot "static"?
I misunderstood last comment, sorry we'll do.
Post by Yuanhan Liu
Post by Santosh Shukla
+/**
+ * ioport map count,
+ * Use-case: virtio-net-pci.
How about removing above two lines; it's quite meaningless here, but
instead a bit redundant.
ok.
Post by Yuanhan Liu
Post by Santosh Shukla
+ * Keeps track of number of virtio-net-pci device mapped/unmapped. Max device
+ * support by linux kernel is 31, so ioport_map_cnt can not be greater than 31.
+ */
+static int ioport_map_cnt;
+
+static int
+virtio_map_ioport(void **resource_addr)
+{
+ int fd;
+ int ret = 0;
+
+ /* avoid -Werror=unused-parameter, keep compiler happy */
+ (void)resource_addr;
Using __rte_unused is more elegant.
ok.
Post by Yuanhan Liu
Post by Santosh Shukla
+ fd = open(VIRT_IOPORT_DEV, 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_VIRT_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);
+
+ close(fd);
+ return ret;
+}
+
+static int
+virtio_set_ioport_addr(void **resource_addr, unsigned long offset)
+{
+ int ret = 0;
+
+ if (ioport_map_cnt >= PCI_VIRT_IOPORT_MAX) {
+ ret = -1;
+ PMD_INIT_LOG(ERR,
+ "ioport_map_cnt(%d) greater than"
+ "PCI_VIRT_IOPORT_MAX(%d)\n",
+ ioport_map_cnt, PCI_VIRT_IOPORT_MAX);
+ return ret;
+ }
+ *resource_addr = (void *)((char *)ioport_map + (ioport_map_cnt)*offset);
Redundant (), and the void * cast seems to be unnecessary.
(void *) is unnecessary, but couldn't get the redundant() part?
Post by Yuanhan Liu
Post by Santosh Shukla
+ ioport_map_cnt++;
+
+ PMD_INIT_LOG(DEBUG, "pci.resource_addr %p ioport_map_cnt %d\n",
+ *resource_addr, ioport_map_cnt);
+ return ret;
+}
+
+int virtio_ioport_init(struct rte_pci_resource *mem_resource)
+{
+ 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 for
+ * each ioport (i.e..pci_dev->mem_resource[0].addr)
+ */
+ if (!ioport_map) {
+ ret = virtio_map_ioport(&mem_resource->addr);
+ if (ret < 0)
+ return ret;
+ PMD_INIT_LOG(INFO, "ioport_map %p\n", ioport_map);
+ }
+
+ ret = virtio_set_ioport_addr(&mem_resource->addr, mem_resource->len);
+ if (ret < 0)
+ return ret;
+
+ PMD_INIT_LOG(INFO, "resource_addr %p resource_len :%ld\n",
+ mem_resource->addr, (unsigned long)mem_resource->len);
+ return ret;
+}
+
+void virtio_ioport_unmap(void)
+{
+ /* unmap ioport memory */
Redundant comment.
ok.
Post by Yuanhan Liu
Post by Santosh Shukla
+ ioport_map_cnt--;
+ if (!ioport_map_cnt)
+ munmap(ioport_map, PCI_VIRT_IOPORT_SIZE);
+
+ PMD_INIT_LOG(DEBUG, "unmapping ioport_mem %d\n", ioport_map_cnt);
+}
+
+#else /* !LINUXAPP && !ARM/64 */
+
+int virtio_ioport_init(struct rte_pci_resource *mem_resource)
+{
+ (void)mem_resource;
ditto.
Is it redundant comment or your suggesting to use : r / (void) / __rte_unused?
Post by Yuanhan Liu
Post by Santosh Shukla
+ return 0;
+}
+
+void virtio_ioport_unmap(void)
+{
+ return;
+}
+
+#endif /* LINUXAPP, ARM/64 */
diff --git a/drivers/net/virtio/virtio_ioport.h b/drivers/net/virtio/virtio_ioport.h
new file mode 100644
index 0000000..bf79551
--- /dev/null
+++ b/drivers/net/virtio/virtio_ioport.h
@@ -0,0 +1,42 @@
+/*
+ * BSD LICENSE
+ *
+ * Copyright(c) 2015 Cavium Networks. All rights reserved.
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ *
+ * * Redistributions of source code must retain the above copyright
+ * notice, this list of conditions and the following disclaimer.
+ * * Redistributions in binary form must reproduce the above copyright
+ * notice, this list of conditions and the following disclaimer in
+ * the documentation and/or other materials provided with the
+ * distribution.
+ * * Neither the name of Intel Corporation nor the names of its
+ * contributors may be used to endorse or promote products derived
+ * from this software without specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ *
+ */
+#ifndef _VIRTIO_IOPORT_H_
+#define _VIRTIO_IOPORT_H_
+
+#include <rte_pci.h>
+
+int virtio_ioport_init(struct rte_pci_resource *mem_resource);
+void virtio_ioport_unmap(void);
+
+#endif /* _VIRTIO_IOPORT_H_ */
--
1.7.9.5
Yuanhan Liu
2015-12-16 14:37:16 UTC
Permalink
On Wed, Dec 16, 2015 at 07:50:51PM +0530, Santosh Shukla wrote:
...
Post by Santosh Shukla
Post by Yuanhan Liu
Post by Santosh Shukla
+ *resource_addr = (void *)((char *)ioport_map + (ioport_map_cnt)*offset);
Redundant (), and the void * cast seems to be unnecessary.
(void *) is unnecessary, but couldn't get the redundant() part?
I meant the () of "(ioport_map_cnt)*offset".
Post by Santosh Shukla
Post by Yuanhan Liu
Post by Santosh Shukla
+ ioport_map_cnt++;
+
+ PMD_INIT_LOG(DEBUG, "pci.resource_addr %p ioport_map_cnt %d\n",
+ *resource_addr, ioport_map_cnt);
+ return ret;
+}
+
Is it redundant comment or your suggesting to use : r / (void) / __rte_unused?
You should always use __rte_unused instead of (void) cast. Note that you
may need check your other patches, to make sure you not miss other such
usage.

--yliu
Santosh Shukla
2015-12-16 14:40:56 UTC
Permalink
On Wed, Dec 16, 2015 at 8:07 PM, Yuanhan Liu
Post by Yuanhan Liu
...
Post by Santosh Shukla
Post by Yuanhan Liu
Post by Santosh Shukla
+ *resource_addr = (void *)((char *)ioport_map + (ioport_map_cnt)*offset);
Redundant (), and the void * cast seems to be unnecessary.
(void *) is unnecessary, but couldn't get the redundant() part?
I meant the () of "(ioport_map_cnt)*offset".
ok.
Post by Yuanhan Liu
Post by Santosh Shukla
Post by Yuanhan Liu
Post by Santosh Shukla
+ ioport_map_cnt++;
+
+ PMD_INIT_LOG(DEBUG, "pci.resource_addr %p ioport_map_cnt %d\n",
+ *resource_addr, ioport_map_cnt);
+ return ret;
+}
+
Is it redundant comment or your suggesting to use : r / (void) / __rte_unused?
You should always use __rte_unused instead of (void) cast. Note that you
may need check your other patches, to make sure you not miss other such
usage.
yup, noted down. Thanks
Post by Yuanhan Liu
--yliu
Santosh Shukla
2015-12-14 13:00:31 UTC
Permalink
Call virtio_ioport_init at device init and virtio_ioport_unmap at device close.

Signed-off-by: Santosh Shukla <***@mvista.com>
---
drivers/net/virtio/virtio_ethdev.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 620e0d4..017d49f 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -62,7 +62,7 @@
#include "virtio_logs.h"
#include "virtqueue.h"
#include "virtio_rxtx.h"
-
+#include "virtio_ioport.h"

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 +497,7 @@ virtio_dev_close(struct rte_eth_dev *dev)
hw->started = 0;
virtio_dev_free_mbufs(dev);
virtio_free_queues(dev);
+ virtio_ioport_unmap();
}

static void
@@ -1290,6 +1291,9 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
if (virtio_resource_init(pci_dev) < 0)
return -1;

+ if (virtio_ioport_init(&pci_dev->mem_resource[0]))
+ return -1;
+
hw->use_msix = virtio_has_msix(&pci_dev->addr);
hw->io_base = (uint64_t)(uintptr_t)pci_dev->mem_resource[0].addr;
--
1.7.9.5
Santosh Shukla
2015-12-14 13:00:32 UTC
Permalink
fix format specifier for func virtio_resource_init_by_ioports.
%04hx-%04hx couldn't read 64bit address correctly that lead to wrong value in
%mem_resource[0].addr / len; result in testpmd failure.
For example, so to read this address 00001040-0000105f; default format-specifier
could read 0-0 that lead to below error on arm64/ThunderX

Unhandled fault: alignment fault (0x92000021) at 0x0000007fb5040002

Signed-off-by: Santosh Shukla <***@mvista.com>
Signed-off-by: Rakesh Krishnamurhty <***@mvista.com>
---
drivers/net/virtio/virtio_ethdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 017d49f..8107aef 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1169,7 +1169,7 @@ static int virtio_resource_init_by_ioports(struct rte_pci_device *pci_dev)
while (*ptr && isspace(*ptr))
ptr++;

- sscanf(ptr, "%04hx-%04hx", &start, &end);
+ sscanf(ptr, "%hx-%hx", &start, &end);
size = end - start + 1;

break;
--
1.7.9.5
Jan Viktorin
2015-12-14 14:31:36 UTC
Permalink
Hello,

this patch set increases the number of warnings for the armv7 build. I
believe that most of them are false-positives.

virtio_ethdev.c:729, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]

xstats[count].value = *(uint64_t *)(((char *)rxvq) +

* Can you guarantee this is not an unaligned 64-bit access?

virtio_ethdev.c:747, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]

ditto...

virtio_pci.c:52, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]

*(uint32_t *)d = VIRTIO_READ_REG_4(hw, off);

* I think, here we can live with unaligned_uint32_t, don't we?

virtio_pci.c:55, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]

similar...

virtio_pci.c:75, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]

similar...

virtio_pci.c:78, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]

similar...

virtio_ring.h:144, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]

vr->desc = (struct vring_desc *) p;

* What can we do here? Should we annotate vring_desc to be aligned properly?

virtio_ring.h:145, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]

similar...

virtio_rxtx.c:733, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]

header = (struct virtio_net_hdr_mrg_rxbuf *)((char *)rxm->buf_addr +
RTE_PKTMBUF_HEADROOM - hdr_size);

* No idea how to solve this...


Any other ideas are welcome.

Regards
Jan

On Mon, 14 Dec 2015 18:30:19 +0530
Post by Santosh Shukla
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
- 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.
- Removed ifdef arm.. clutter from igb_uio / virtio_ethedev files
- Introduced rte_io.h header file in generic/ and arch specifics i.e.. for
armv7 --> rte_io_32.h, for armv8 --> rte_io_64.h.
- Removed RTE_ARCH_X86 ifdef clutter too and added rte_io.h header which nothing
but wraps sys/io.h for x86_64 and i686
- Moved all the RTE_ARCH_ARM/64 dependancy for igb_uio case to separate header
file named igbuio_ioport_misc.h. Now igb_uio.c will call only three function
- igbuio_iomap
- igbuio_ioport_register
- igbuio_ioport_unregister
- Moved ARM/64 specific definition to include/exec-env/rte_virt_ioport.h header
- Included virtio_ioport.c/h; has all private and public api required to map
iopci bar for non-x86 arch. Tested on thunderX and x86_64 both.
- virtio_map_ioport
- virtio_set_ioport_addr
- virtio_ioport_init
- virtio_ioport_unmap
- Last patch is the miscllanious format specifier fix identifid for 64bit case
during regression.
- 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}
virtio: Introduce config RTE_VIRTIO_INC_VECTOR
config: i686: set RTE_VIRTIO_INC_VECTOR=n
rte_io: armv7/v8: Introduce api to emulate x86-style of PCI/ISA
ioport access
virtio_pci: use rte_io.h for non-x86 arch
virtio: change io_base datatype from uint32_t to uint64_type
config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD
linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init()
rte_io: x86: Remove sys/io.h ifdef x86 clutter
igb_uio: ioport: map iopci region for armv7/v8
include/exec-env: ioport: add rte_virt_ioport header file
virtio_ioport: armv7/v8: mmap virtio iopci bar region
virtio_ethdev: use virtio_ioport api at device init/close
virtio_ethdev : fix format specifier error for 64bit addr case
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 | 3 +-
drivers/net/virtio/virtio_ethdev.c | 10 +-
drivers/net/virtio/virtio_ioport.c | 163 ++++++++++++++++++++
drivers/net/virtio/virtio_ioport.h | 42 +++++
drivers/net/virtio/virtio_pci.h | 6 +-
drivers/net/virtio/virtio_rxtx.c | 7 +
lib/librte_eal/common/Makefile | 1 +
lib/librte_eal/common/include/arch/arm/rte_io.h | 60 +++++++
lib/librte_eal/common/include/arch/arm/rte_io_32.h | 155 +++++++++++++++++++
lib/librte_eal/common/include/arch/arm/rte_io_64.h | 155 +++++++++++++++++++
lib/librte_eal/common/include/arch/x86/rte_io.h | 42 +++++
lib/librte_eal/common/include/generic/rte_io.h | 81 ++++++++++
lib/librte_eal/linuxapp/eal/Makefile | 2 +-
lib/librte_eal/linuxapp/eal/eal.c | 7 +-
.../eal/include/exec-env/rte_virt_ioport.h | 81 ++++++++++
lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 8 +-
.../linuxapp/igb_uio/igbuio_ioport_misc.h | 133 ++++++++++++++++
22 files changed, 957 insertions(+), 14 deletions(-)
create mode 100644 drivers/net/virtio/virtio_ioport.c
create mode 100644 drivers/net/virtio/virtio_ioport.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_32.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h
create mode 100644 lib/librte_eal/common/include/arch/x86/rte_io.h
create mode 100644 lib/librte_eal/common/include/generic/rte_io.h
create mode 100644 lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h
create mode 100644 lib/librte_eal/linuxapp/igb_uio/igbuio_ioport_misc.h
--
Jan Viktorin E-mail: ***@RehiveTech.com
System Architect Web: www.RehiveTech.com
RehiveTech
Brno, Czech Republic
Santosh Shukla
2015-12-14 16:09:51 UTC
Permalink
Post by Jan Viktorin
Hello,
this patch set increases the number of warnings for the armv7 build. I
believe that most of them are false-positives.
virtio_ethdev.c:729, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]
xstats[count].value = *(uint64_t *)(((char *)rxvq) +
* Can you guarantee this is not an unaligned 64-bit access?
virtio_ethdev.c:747, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]
ditto...
virtio_pci.c:52, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]
*(uint32_t *)d = VIRTIO_READ_REG_4(hw, off);
* I think, here we can live with unaligned_uint32_t, don't we?
Yes.
Post by Jan Viktorin
virtio_pci.c:55, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]
similar...
virtio_pci.c:75, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]
similar...
virtio_pci.c:78, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]
similar...
virtio_ring.h:144, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]
vr->desc = (struct vring_desc *) p;
* What can we do here? Should we annotate vring_desc to be aligned properly?
virtio_ring.h:145, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]
similar...
virtio_rxtx.c:733, GNU Make + GNU C Compiler (gcc), Priority: Normal
cast increases required alignment of target type [-Wcast-align]
header = (struct virtio_net_hdr_mrg_rxbuf *)((char *)rxm->buf_addr +
RTE_PKTMBUF_HEADROOM - hdr_size);
* No idea how to solve this...
These warnings are in-general virtio-pmd warnings for armv7 case.
Patchset enables virtio feature thus above warning noticed.

I spent 10 min and noticed that most of those warning are types
(uint64_t*) ((char *) ptr + offset). could avoid such warning by
(uint64_t *) (uintptr) ((char*)ptr + offset) where ptr is void *, But
this not the case in virtio code, most of such places ptr is uint8_t,
keeping then void * then typecasting to ((char *)ptr + offset) then
following proposed-way, I guess won't harm functionality and could
keep armv7 happy. And yes they are all look false-positive to me too.
Pasting code a snape which could get rid of above warning for armv7
case. Let me know your feedback, Others thought/comment welcome.
Thanks!

----------------------------------------------------------
diff --git a/drivers/net/virtio/virtio_ethdev.c
b/drivers/net/virtio/virtio_ethdev.c
index 8107aef..7ef4d13 100644
--- a/drivers/net/virtio/virtio_ethdev.c
diff --git a/drivers/net/virtio/virtio_ethdev.c
b/drivers/net/virtio/virtio_ethdev.c
index 8107aef..7ef4d13 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -726,7 +726,7 @@ virtio_dev_xstats_get(struct rte_eth_dev *dev,
struct rte_eth_xstats *xstats,
snprintf(xstats[count].name, sizeof(xstats[count].name),
"rx_q%u_%s", i,
rte_virtio_q_stat_strings[t].name);
- xstats[count].value = *(uint64_t *)(((char *)rxvq) +
+ xstats[count].value = *(uint64_t
*)(uintptr_t)(((char *)rxvq) +
rte_virtio_q_stat_strings[t].offset);
count++;
}
@@ -744,7 +744,7 @@ virtio_dev_xstats_get(struct rte_eth_dev *dev,
struct rte_eth_xstats *xstats,
snprintf(xstats[count].name, sizeof(xstats[count].name),
"tx_q%u_%s", i,
rte_virtio_q_stat_strings[t].name);
- xstats[count].value = *(uint64_t *)(((char *)txvq) +
+ xstats[count].value = *(uint64_t
*)(uintptr_t)(((char *)txvq) +
rte_virtio_q_stat_strings[t].offset);
count++;
}
diff --git a/drivers/net/virtio/virtio_ring.h b/drivers/net/virtio/virtio_ring.h
index 447760a..f2f85b4 100644
--- a/drivers/net/virtio/virtio_ring.h
+++ b/drivers/net/virtio/virtio_ring.h
@@ -141,8 +141,8 @@ vring_init(struct vring *vr, unsigned int num, uint8_t *p,
unsigned long align)
{
vr->num = num;
- vr->desc = (struct vring_desc *) p;
- vr->avail = (struct vring_avail *) (p +
+ vr->desc = (struct vring_desc *) (uintptr_t)p;
+ vr->avail = (struct vring_avail *) ((uintptr_t)p +
num * sizeof(struct vring_desc));
vr->used = (void *)
RTE_ALIGN_CEIL((uintptr_t)(&vr->avail->ring[num]), align);
diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
index 23be1ff..d636e08 100644
--- a/drivers/net/virtio/virtio_rxtx.c
+++ b/drivers/net/virtio/virtio_rxtx.c
@@ -730,7 +730,7 @@ virtio_recv_mergeable_pkts(void *rx_queue,
continue;
}

- header = (struct virtio_net_hdr_mrg_rxbuf *)((char
*)rxm->buf_addr +
+ header = (struct virtio_net_hdr_mrg_rxbuf
*)(uintptr_t)((char *)rxm->buf_addr +
RTE_PKTMBUF_HEADROOM - hdr_size);
seg_num = header->num_buffers;

-----------------------------------------------------------------------------------
Post by Jan Viktorin
Any other ideas are welcome.
Regards
Jan
On Mon, 14 Dec 2015 18:30:19 +0530
Post by Santosh Shukla
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
- 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.
- Removed ifdef arm.. clutter from igb_uio / virtio_ethedev files
- Introduced rte_io.h header file in generic/ and arch specifics i.e.. for
armv7 --> rte_io_32.h, for armv8 --> rte_io_64.h.
- Removed RTE_ARCH_X86 ifdef clutter too and added rte_io.h header which nothing
but wraps sys/io.h for x86_64 and i686
- Moved all the RTE_ARCH_ARM/64 dependancy for igb_uio case to separate header
file named igbuio_ioport_misc.h. Now igb_uio.c will call only three function
- igbuio_iomap
- igbuio_ioport_register
- igbuio_ioport_unregister
- Moved ARM/64 specific definition to include/exec-env/rte_virt_ioport.h header
- Included virtio_ioport.c/h; has all private and public api required to map
iopci bar for non-x86 arch. Tested on thunderX and x86_64 both.
- virtio_map_ioport
- virtio_set_ioport_addr
- virtio_ioport_init
- virtio_ioport_unmap
- Last patch is the miscllanious format specifier fix identifid for 64bit case
during regression.
- 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}
virtio: Introduce config RTE_VIRTIO_INC_VECTOR
config: i686: set RTE_VIRTIO_INC_VECTOR=n
rte_io: armv7/v8: Introduce api to emulate x86-style of PCI/ISA
ioport access
virtio_pci: use rte_io.h for non-x86 arch
virtio: change io_base datatype from uint32_t to uint64_type
config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD
linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init()
rte_io: x86: Remove sys/io.h ifdef x86 clutter
igb_uio: ioport: map iopci region for armv7/v8
include/exec-env: ioport: add rte_virt_ioport header file
virtio_ioport: armv7/v8: mmap virtio iopci bar region
virtio_ethdev: use virtio_ioport api at device init/close
virtio_ethdev : fix format specifier error for 64bit addr case
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 | 3 +-
drivers/net/virtio/virtio_ethdev.c | 10 +-
drivers/net/virtio/virtio_ioport.c | 163 ++++++++++++++++++++
drivers/net/virtio/virtio_ioport.h | 42 +++++
drivers/net/virtio/virtio_pci.h | 6 +-
drivers/net/virtio/virtio_rxtx.c | 7 +
lib/librte_eal/common/Makefile | 1 +
lib/librte_eal/common/include/arch/arm/rte_io.h | 60 +++++++
lib/librte_eal/common/include/arch/arm/rte_io_32.h | 155 +++++++++++++++++++
lib/librte_eal/common/include/arch/arm/rte_io_64.h | 155 +++++++++++++++++++
lib/librte_eal/common/include/arch/x86/rte_io.h | 42 +++++
lib/librte_eal/common/include/generic/rte_io.h | 81 ++++++++++
lib/librte_eal/linuxapp/eal/Makefile | 2 +-
lib/librte_eal/linuxapp/eal/eal.c | 7 +-
.../eal/include/exec-env/rte_virt_ioport.h | 81 ++++++++++
lib/librte_eal/linuxapp/igb_uio/igb_uio.c | 8 +-
.../linuxapp/igb_uio/igbuio_ioport_misc.h | 133 ++++++++++++++++
22 files changed, 957 insertions(+), 14 deletions(-)
create mode 100644 drivers/net/virtio/virtio_ioport.c
create mode 100644 drivers/net/virtio/virtio_ioport.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_32.h
create mode 100644 lib/librte_eal/common/include/arch/arm/rte_io_64.h
create mode 100644 lib/librte_eal/common/include/arch/x86/rte_io.h
create mode 100644 lib/librte_eal/common/include/generic/rte_io.h
create mode 100644 lib/librte_eal/linuxapp/eal/include/exec-env/rte_virt_ioport.h
create mode 100644 lib/librte_eal/linuxapp/igb_uio/igbuio_ioport_misc.h
--
System Architect Web: www.RehiveTech.com
RehiveTech
Brno, Czech Republic
Santosh Shukla
2015-12-16 07:48:17 UTC
Permalink
Post by Santosh Shukla
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
- 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.
- Removed ifdef arm.. clutter from igb_uio / virtio_ethedev files
- Introduced rte_io.h header file in generic/ and arch specifics i.e.. for
armv7 --> rte_io_32.h, for armv8 --> rte_io_64.h.
- Removed RTE_ARCH_X86 ifdef clutter too and added rte_io.h header which nothing
but wraps sys/io.h for x86_64 and i686
- Moved all the RTE_ARCH_ARM/64 dependancy for igb_uio case to separate header
file named igbuio_ioport_misc.h. Now igb_uio.c will call only three function
- igbuio_iomap
- igbuio_ioport_register
- igbuio_ioport_unregister
- Moved ARM/64 specific definition to include/exec-env/rte_virt_ioport.h header
- Included virtio_ioport.c/h; has all private and public api required to map
iopci bar for non-x86 arch. Tested on thunderX and x86_64 both.
- virtio_map_ioport
- virtio_set_ioport_addr
- virtio_ioport_init
- virtio_ioport_unmap
- Last patch is the miscllanious format specifier fix identifid for 64bit case
during regression.
Hi Yuanhan, Huawei and Others.

I got arch specific review comment from arm maintainers and I am waiting
for your review feedback on virtio specific patches? Is v3 patch and virtio
iopci bar mapping to user-space approach ok with all? Thanks.
Post by Santosh Shukla
- 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}
virtio: Introduce config RTE_VIRTIO_INC_VECTOR
config: i686: set RTE_VIRTIO_INC_VECTOR=n
rte_io: armv7/v8: Introduce api to emulate x86-style of PCI/ISA
ioport access
virtio_pci: use rte_io.h for non-x86 arch
virtio: change io_base datatype from uint32_t to uint64_type
config: armv7/v8: Enable RTE_LIBRTE_VIRTIO_PMD
linuxapp: eal: arm: Always return 0 for rte_eal_iopl_init()
rte_io: x86: Remove sys/io.h ifdef x86 clutter
igb_uio: ioport: map iopci region for armv7/v8
include/exec-env: ioport: add rte_virt_ioport header file
virtio_ioport: armv7/v8: mmap virtio iopci bar region
virtio_ethdev: use virtio_ioport api at device init/close
virtio_ethdev : fix format specifier error for 64bit addr case
David Marchand
2015-12-16 08:47:34 UTC
Permalink
Hello Santosh,
Post by Santosh Shukla
Hi Yuanhan, Huawei and Others.
I got arch specific review comment from arm maintainers and I am waiting
for your review feedback on virtio specific patches? Is v3 patch and virtio
iopci bar mapping to user-space approach ok with all? Thanks.
Please, don't forget to CC me as well.

I did something similar for powerpc, but there was no need to add any
remapping in igb_uio.
Is there something specific to arm that makes it impossible to reuse
resources mmapping from /sys ?
I can send a patch that should do the job for eal.

I am a bit short on time and will be on holidays for two weeks, so I can't
look at these patches before January.


Regards,
--
David Marchand
Santosh Shukla
2015-12-16 11:43:38 UTC
Permalink
On Wed, Dec 16, 2015 at 2:17 PM, David Marchand
Post by David Marchand
Hello Santosh,
Post by Santosh Shukla
Hi Yuanhan, Huawei and Others.
I got arch specific review comment from arm maintainers and I am waiting
for your review feedback on virtio specific patches? Is v3 patch and virtio
iopci bar mapping to user-space approach ok with all? Thanks.
Please, don't forget to CC me as well.
I did something similar for powerpc, but there was no need to add any
remapping in igb_uio.
Is it for mapping iopci bar? does that includes virtio

For detailed explanation refer [1]

[1] http://dpdk.org/dev/patchwork/patch/9365/
Post by David Marchand
Is there something specific to arm that makes it impossible to reuse
resources mmapping from /sys ?
/sysfs wont map resource0, it could map resource1 i.e. iomem but
virtio header resides in iopci bar region so iomem memory wont be
effective / invalid addr. For that someone to explicitly map iopci
region thus this code/pach.
Post by David Marchand
I can send a patch that should do the job for eal.
Pl. send then, My patches are waiting for review for quite a long
time. It will be good if you send now.
Post by David Marchand
I am a bit short on time and will be on holidays for two weeks, so I can't
look at these patches before January.
Regards,
--
David Marchand
David Marchand
2015-12-16 12:31:04 UTC
Permalink
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.

Signed-off-by: David Marchand <***@6wind.com>
---
lib/librte_eal/common/include/rte_pci.h | 3 ++-
lib/librte_eal/linuxapp/eal/eal_pci.c | 21 +++++++++++++++------
2 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 334c12e..8aaab4a 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -105,7 +105,8 @@ extern struct pci_device_list pci_device_list; /**< Global list of PCI devices.
/** Nb. of values in PCI resource format. */
#define PCI_RESOURCE_FMT_NVAL 3

-/** IO resource type: memory address space */
+/** IO resource type: */
+#define IORESOURCE_IO 0x00000100
#define IORESOURCE_MEM 0x00000200

/**
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index bc5b5be..9c4651d 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -236,12 +236,21 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
goto error;
}

- if (flags & IORESOURCE_MEM) {
- dev->mem_resource[i].phys_addr = phys_addr;
- dev->mem_resource[i].len = end_addr - phys_addr + 1;
- /* not mapped for now */
- dev->mem_resource[i].addr = NULL;
- }
+ /* we only care about IORESOURCE_IO or IORESOURCE_MEM */
+ if (!(flags & IORESOURCE_IO) &&
+ !(flags & IORESOURCE_MEM))
+ continue;
+
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
+ /* x86 can not remap ioports, so skip it, remapping code will
+ * look at dev->mem_resource[i].phys_addr == 0 and skip it */
+ if (flags & IORESOURCE_IO)
+ continue;
+#endif
+ dev->mem_resource[i].phys_addr = phys_addr;
+ dev->mem_resource[i].len = end_addr - phys_addr + 1;
+ /* not mapped for now */
+ dev->mem_resource[i].addr = NULL;
}
fclose(f);
return 0;
--
1.7.10.4
Yuanhan Liu
2015-12-16 12:48:34 UTC
Permalink
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
One question: this patch could be a replacement of the igbuio_iomap patch
from Santosh? If so, I like it: It's more elegant.

--yliu
Post by David Marchand
---
lib/librte_eal/common/include/rte_pci.h | 3 ++-
lib/librte_eal/linuxapp/eal/eal_pci.c | 21 +++++++++++++++------
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 334c12e..8aaab4a 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -105,7 +105,8 @@ extern struct pci_device_list pci_device_list; /**< Global list of PCI devices.
/** Nb. of values in PCI resource format. */
#define PCI_RESOURCE_FMT_NVAL 3
-/** IO resource type: memory address space */
+/** IO resource type: */
+#define IORESOURCE_IO 0x00000100
#define IORESOURCE_MEM 0x00000200
/**
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index bc5b5be..9c4651d 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -236,12 +236,21 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
goto error;
}
- if (flags & IORESOURCE_MEM) {
- dev->mem_resource[i].phys_addr = phys_addr;
- dev->mem_resource[i].len = end_addr - phys_addr + 1;
- /* not mapped for now */
- dev->mem_resource[i].addr = NULL;
- }
+ /* we only care about IORESOURCE_IO or IORESOURCE_MEM */
+ if (!(flags & IORESOURCE_IO) &&
+ !(flags & IORESOURCE_MEM))
+ continue;
+
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
+ /* x86 can not remap ioports, so skip it, remapping code will
+ * look at dev->mem_resource[i].phys_addr == 0 and skip it */
+ if (flags & IORESOURCE_IO)
+ continue;
+#endif
+ dev->mem_resource[i].phys_addr = phys_addr;
+ dev->mem_resource[i].len = end_addr - phys_addr + 1;
+ /* not mapped for now */
+ dev->mem_resource[i].addr = NULL;
}
fclose(f);
return 0;
--
1.7.10.4
David Marchand
2015-12-16 13:34:35 UTC
Permalink
Yuanhan,
Post by Yuanhan Liu
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
One question: this patch could be a replacement of the igbuio_iomap patch
from Santosh? If so, I like it: It's more elegant.
Well, yes, unless I missed something since I am no guru :-).
--
David Marchand
Yuanhan Liu
2015-12-16 13:42:00 UTC
Permalink
Post by Santosh Shukla
Yuanhan,
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
One question: this patch could be a replacement of the igbuio_iomap patch
from Santosh? If so, I like it: It's more elegant.
Well, yes, unless I missed something since I am no guru :-).
Great then. If there is a test-by, I could give my Ack :)
(I have no arm or other platform for testing).

--yliu
Santosh Shukla
2015-12-16 13:51:55 UTC
Permalink
On Wed, Dec 16, 2015 at 6:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
One question: this patch could be a replacement of the igbuio_iomap patch
from Santosh? If so, I like it: It's more elegant.
--yliu
I did tried similar in past but not in parse_sysfs (such that
mem.resource_addr to accept IO_RESOURCE_IO types) and observed that
pci_map_resource not able to map address hence segfault at tespmd
initialization.

i was getting these:
EAL: pci_map_resource(): cannot mmap(19, 0x7fa5c00000, 0x20, 0x0):
Invalid argument (0xffffffffffffffff)

after enabling RTE_PCI_NEED_DRV_MAPPING flags in virtio_ethdev. I
guess patch assume that flag enabled for driver right?
Post by Yuanhan Liu
Post by David Marchand
---
lib/librte_eal/common/include/rte_pci.h | 3 ++-
lib/librte_eal/linuxapp/eal/eal_pci.c | 21 +++++++++++++++------
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 334c12e..8aaab4a 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -105,7 +105,8 @@ extern struct pci_device_list pci_device_list; /**< Global list of PCI devices.
/** Nb. of values in PCI resource format. */
#define PCI_RESOURCE_FMT_NVAL 3
-/** IO resource type: memory address space */
+/** IO resource type: */
+#define IORESOURCE_IO 0x00000100
#define IORESOURCE_MEM 0x00000200
/**
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index bc5b5be..9c4651d 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -236,12 +236,21 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
goto error;
}
- if (flags & IORESOURCE_MEM) {
- dev->mem_resource[i].phys_addr = phys_addr;
- dev->mem_resource[i].len = end_addr - phys_addr + 1;
- /* not mapped for now */
- dev->mem_resource[i].addr = NULL;
- }
+ /* we only care about IORESOURCE_IO or IORESOURCE_MEM */
+ if (!(flags & IORESOURCE_IO) &&
+ !(flags & IORESOURCE_MEM))
+ continue;
+
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
+ /* x86 can not remap ioports, so skip it, remapping code will
+ * look at dev->mem_resource[i].phys_addr == 0 and skip it */
+ if (flags & IORESOURCE_IO)
+ continue;
+#endif
+ dev->mem_resource[i].phys_addr = phys_addr;
+ dev->mem_resource[i].len = end_addr - phys_addr + 1;
+ /* not mapped for now */
+ dev->mem_resource[i].addr = NULL;
}
fclose(f);
return 0;
--
1.7.10.4
Yuanhan Liu
2015-12-17 09:38:47 UTC
Permalink
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 6:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
One question: this patch could be a replacement of the igbuio_iomap patch
from Santosh? If so, I like it: It's more elegant.
--yliu
I did tried similar in past but not in parse_sysfs (such that
mem.resource_addr to accept IO_RESOURCE_IO types) and observed that
pci_map_resource not able to map address hence segfault at tespmd
initialization.
Invalid argument (0xffffffffffffffff)
That's because ARM (at least the kernel) doesn't allow an IO map:

arch/arm/kernel/bios32.c
------------------------
618 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
619 enum pci_mmap_state mmap_state, int write_combine)
620 {
621 if (mmap_state == pci_mmap_io)
622 return -EINVAL;

And with a quick glimpse of powerpc, I see no such limitation. Hence,
this peice of code may work only on Powerpc platform (and maybe a few
others we don't care).

So, apparently, this will not work for ARM.

--yliu
Santosh Shukla
2015-12-17 10:01:29 UTC
Permalink
On Thu, Dec 17, 2015 at 3:08 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 6:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
One question: this patch could be a replacement of the igbuio_iomap patch
from Santosh? If so, I like it: It's more elegant.
--yliu
I did tried similar in past but not in parse_sysfs (such that
mem.resource_addr to accept IO_RESOURCE_IO types) and observed that
pci_map_resource not able to map address hence segfault at tespmd
initialization.
Invalid argument (0xffffffffffffffff)
arch/arm/kernel/bios32.c
------------------------
618 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
619 enum pci_mmap_state mmap_state, int write_combine)
620 {
621 if (mmap_state == pci_mmap_io)
622 return -EINVAL;
And with a quick glimpse of powerpc, I see no such limitation. Hence,
this peice of code may work only on Powerpc platform (and maybe a few
others we don't care).
So, apparently, this will not work for ARM.
Right and I did shared detailed explanation on why it wont work on
this link [1], infact this patch shouldn;t work for mips too.

As I mentioned earlier I did tried similar approach and so to get
everything working like iomem is currently in dpdk; we need to add
something like pci_remap_iospace --> ioremap_page_range() but this api
not really pci_mmap_page_range types. user need to write more code on
top so to use this api efficiently, also this api looks like meant to
use by arch file only in kernel space.
Post by Yuanhan Liu
--yliu
Santosh Shukla
2015-12-17 10:02:19 UTC
Permalink
Post by Santosh Shukla
On Thu, Dec 17, 2015 at 3:08 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 6:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
One question: this patch could be a replacement of the igbuio_iomap patch
from Santosh? If so, I like it: It's more elegant.
--yliu
I did tried similar in past but not in parse_sysfs (such that
mem.resource_addr to accept IO_RESOURCE_IO types) and observed that
pci_map_resource not able to map address hence segfault at tespmd
initialization.
Invalid argument (0xffffffffffffffff)
arch/arm/kernel/bios32.c
------------------------
618 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
619 enum pci_mmap_state mmap_state, int write_combine)
620 {
621 if (mmap_state == pci_mmap_io)
622 return -EINVAL;
And with a quick glimpse of powerpc, I see no such limitation. Hence,
this peice of code may work only on Powerpc platform (and maybe a few
others we don't care).
So, apparently, this will not work for ARM.
Right and I did shared detailed explanation on why it wont work on
this link [1], infact this patch shouldn;t work for mips too.
As I mentioned earlier I did tried similar approach and so to get
everything working like iomem is currently in dpdk; we need to add
something like pci_remap_iospace --> ioremap_page_range() but this api
not really pci_mmap_page_range types. user need to write more code on
top so to use this api efficiently, also this api looks like meant to
use by arch file only in kernel space.
missed link;

[1] http://dpdk.org/dev/patchwork/patch/9365/
Post by Santosh Shukla
Post by Yuanhan Liu
--yliu
Santosh Shukla
2015-12-17 10:07:23 UTC
Permalink
Post by Santosh Shukla
Post by Santosh Shukla
On Thu, Dec 17, 2015 at 3:08 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 6:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
One question: this patch could be a replacement of the igbuio_iomap patch
from Santosh? If so, I like it: It's more elegant.
--yliu
I did tried similar in past but not in parse_sysfs (such that
mem.resource_addr to accept IO_RESOURCE_IO types) and observed that
pci_map_resource not able to map address hence segfault at tespmd
initialization.
Invalid argument (0xffffffffffffffff)
arch/arm/kernel/bios32.c
------------------------
618 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
619 enum pci_mmap_state mmap_state, int write_combine)
620 {
621 if (mmap_state == pci_mmap_io)
622 return -EINVAL;
And with a quick glimpse of powerpc, I see no such limitation. Hence,
this peice of code may work only on Powerpc platform (and maybe a few
others we don't care).
So, apparently, this will not work for ARM.
Right and I did shared detailed explanation on why it wont work on
this link [1], infact this patch shouldn;t work for mips too.
As I mentioned earlier I did tried similar approach and so to get
everything working like iomem is currently in dpdk; we need to add
something like pci_remap_iospace --> ioremap_page_range() but this api
not really pci_mmap_page_range types. user need to write more code on
top so to use this api efficiently, also this api looks like meant to
use by arch file only in kernel space.
missed link;
[1] http://dpdk.org/dev/patchwork/patch/9365/
IMO, it is worth keeping one special device file who could work across
archs like arm/arm64/powerpc and others, who could map iopci bar to
dpdk user-space. also this approach has no kernel version dependency
too. BTW; I did mentioned in second approach in to add /dev/ioport
interface in drivers/char/mem.c which could read more than byte in one
single operation, but that has kernel dependency. However that
approach too is arch agnostic.

Let me know!
Post by Santosh Shukla
Post by Santosh Shukla
Post by Yuanhan Liu
--yliu
Thomas Monjalon
2015-12-17 10:14:38 UTC
Permalink
Hi,
Post by Santosh Shukla
Post by Santosh Shukla
Post by Santosh Shukla
On Thu, Dec 17, 2015 at 3:08 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 6:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
One question: this patch could be a replacement of the igbuio_iomap patch
from Santosh? If so, I like it: It's more elegant.
--yliu
I did tried similar in past but not in parse_sysfs (such that
mem.resource_addr to accept IO_RESOURCE_IO types) and observed that
pci_map_resource not able to map address hence segfault at tespmd
initialization.
Invalid argument (0xffffffffffffffff)
arch/arm/kernel/bios32.c
------------------------
618 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
619 enum pci_mmap_state mmap_state, int write_combine)
620 {
621 if (mmap_state == pci_mmap_io)
622 return -EINVAL;
And with a quick glimpse of powerpc, I see no such limitation. Hence,
this peice of code may work only on Powerpc platform (and maybe a few
others we don't care).
So, apparently, this will not work for ARM.
Right and I did shared detailed explanation on why it wont work on
this link [1], infact this patch shouldn;t work for mips too.
As I mentioned earlier I did tried similar approach and so to get
everything working like iomem is currently in dpdk; we need to add
something like pci_remap_iospace --> ioremap_page_range() but this api
not really pci_mmap_page_range types. user need to write more code on
top so to use this api efficiently, also this api looks like meant to
use by arch file only in kernel space.
missed link;
[1] http://dpdk.org/dev/patchwork/patch/9365/
IMO, it is worth keeping one special device file who could work across
archs like arm/arm64/powerpc and others, who could map iopci bar to
dpdk user-space. also this approach has no kernel version dependency
too. BTW; I did mentioned in second approach in to add /dev/ioport
interface in drivers/char/mem.c which could read more than byte in one
single operation, but that has kernel dependency. However that
approach too is arch agnostic.
Your first approach use an out-of-tree kernel module (igb_uio), so we cannot
really say there is no kernel dependency.
We should try to remove the need for any out-of-tree kernel module.
That's why the Linux upstream approach is a better solution.
Santosh Shukla
2015-12-17 10:21:20 UTC
Permalink
On Thu, Dec 17, 2015 at 3:44 PM, Thomas Monjalon
Post by Thomas Monjalon
Hi,
Post by Santosh Shukla
Post by Santosh Shukla
Post by Santosh Shukla
On Thu, Dec 17, 2015 at 3:08 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 6:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
One question: this patch could be a replacement of the igbuio_iomap patch
from Santosh? If so, I like it: It's more elegant.
--yliu
I did tried similar in past but not in parse_sysfs (such that
mem.resource_addr to accept IO_RESOURCE_IO types) and observed that
pci_map_resource not able to map address hence segfault at tespmd
initialization.
Invalid argument (0xffffffffffffffff)
arch/arm/kernel/bios32.c
------------------------
618 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
619 enum pci_mmap_state mmap_state, int write_combine)
620 {
621 if (mmap_state == pci_mmap_io)
622 return -EINVAL;
And with a quick glimpse of powerpc, I see no such limitation. Hence,
this peice of code may work only on Powerpc platform (and maybe a few
others we don't care).
So, apparently, this will not work for ARM.
Right and I did shared detailed explanation on why it wont work on
this link [1], infact this patch shouldn;t work for mips too.
As I mentioned earlier I did tried similar approach and so to get
everything working like iomem is currently in dpdk; we need to add
something like pci_remap_iospace --> ioremap_page_range() but this api
not really pci_mmap_page_range types. user need to write more code on
top so to use this api efficiently, also this api looks like meant to
use by arch file only in kernel space.
missed link;
[1] http://dpdk.org/dev/patchwork/patch/9365/
IMO, it is worth keeping one special device file who could work across
archs like arm/arm64/powerpc and others, who could map iopci bar to
dpdk user-space. also this approach has no kernel version dependency
too. BTW; I did mentioned in second approach in to add /dev/ioport
interface in drivers/char/mem.c which could read more than byte in one
single operation, but that has kernel dependency. However that
approach too is arch agnostic.
Your first approach use an out-of-tree kernel module (igb_uio), so we cannot
really say there is no kernel dependency.
Agree but I mentioned kernel __version__ dependency.
Post by Thomas Monjalon
We should try to remove the need for any out-of-tree kernel module.
That's why the Linux upstream approach is a better solution.
IIUC, your suggesting archs like arm/arm64 to support io_mappe_io in
pci_mmap_page_range()?
Thomas Monjalon
2015-12-17 10:33:39 UTC
Permalink
Post by Santosh Shukla
On Thu, Dec 17, 2015 at 3:44 PM, Thomas Monjalon
Post by Thomas Monjalon
Hi,
Post by Santosh Shukla
Post by Santosh Shukla
Post by Santosh Shukla
On Thu, Dec 17, 2015 at 3:08 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 6:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
One question: this patch could be a replacement of the igbuio_iomap patch
from Santosh? If so, I like it: It's more elegant.
--yliu
I did tried similar in past but not in parse_sysfs (such that
mem.resource_addr to accept IO_RESOURCE_IO types) and observed that
pci_map_resource not able to map address hence segfault at tespmd
initialization.
Invalid argument (0xffffffffffffffff)
arch/arm/kernel/bios32.c
------------------------
618 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
619 enum pci_mmap_state mmap_state, int write_combine)
620 {
621 if (mmap_state == pci_mmap_io)
622 return -EINVAL;
And with a quick glimpse of powerpc, I see no such limitation. Hence,
this peice of code may work only on Powerpc platform (and maybe a few
others we don't care).
So, apparently, this will not work for ARM.
Right and I did shared detailed explanation on why it wont work on
this link [1], infact this patch shouldn;t work for mips too.
As I mentioned earlier I did tried similar approach and so to get
everything working like iomem is currently in dpdk; we need to add
something like pci_remap_iospace --> ioremap_page_range() but this api
not really pci_mmap_page_range types. user need to write more code on
top so to use this api efficiently, also this api looks like meant to
use by arch file only in kernel space.
missed link;
[1] http://dpdk.org/dev/patchwork/patch/9365/
IMO, it is worth keeping one special device file who could work across
archs like arm/arm64/powerpc and others, who could map iopci bar to
dpdk user-space. also this approach has no kernel version dependency
too. BTW; I did mentioned in second approach in to add /dev/ioport
interface in drivers/char/mem.c which could read more than byte in one
single operation, but that has kernel dependency. However that
approach too is arch agnostic.
Your first approach use an out-of-tree kernel module (igb_uio), so we cannot
really say there is no kernel dependency.
Agree but I mentioned kernel __version__ dependency.
Yes you did.
One of the main issue with out-of-tree kernel modules is the version
dependency. Probably that igb_uio from DPDK 2.3 will not compile with
the kernel 5.0.
Post by Santosh Shukla
Post by Thomas Monjalon
We should try to remove the need for any out-of-tree kernel module.
That's why the Linux upstream approach is a better solution.
IIUC, your suggesting archs like arm/arm64 to support io_mappe_io in
pci_mmap_page_range()?
I don't know what is the best solution in the kernel.
First we need to be sure that there is absolutely no solution without
kernel changes.
Then we can try a pci_mmap solution or, as you suggest, an interface in
drivers/char/mem.c
Santosh Shukla
2015-12-17 11:22:00 UTC
Permalink
On Thu, Dec 17, 2015 at 4:03 PM, Thomas Monjalon
Post by Thomas Monjalon
Post by Santosh Shukla
On Thu, Dec 17, 2015 at 3:44 PM, Thomas Monjalon
Post by Thomas Monjalon
Hi,
Post by Santosh Shukla
Post by Santosh Shukla
Post by Santosh Shukla
On Thu, Dec 17, 2015 at 3:08 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
On Wed, Dec 16, 2015 at 6:18 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
One question: this patch could be a replacement of the igbuio_iomap patch
from Santosh? If so, I like it: It's more elegant.
--yliu
I did tried similar in past but not in parse_sysfs (such that
mem.resource_addr to accept IO_RESOURCE_IO types) and observed that
pci_map_resource not able to map address hence segfault at tespmd
initialization.
Invalid argument (0xffffffffffffffff)
arch/arm/kernel/bios32.c
------------------------
618 int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
619 enum pci_mmap_state mmap_state, int write_combine)
620 {
621 if (mmap_state == pci_mmap_io)
622 return -EINVAL;
And with a quick glimpse of powerpc, I see no such limitation. Hence,
this peice of code may work only on Powerpc platform (and maybe a few
others we don't care).
So, apparently, this will not work for ARM.
Right and I did shared detailed explanation on why it wont work on
this link [1], infact this patch shouldn;t work for mips too.
As I mentioned earlier I did tried similar approach and so to get
everything working like iomem is currently in dpdk; we need to add
something like pci_remap_iospace --> ioremap_page_range() but this api
not really pci_mmap_page_range types. user need to write more code on
top so to use this api efficiently, also this api looks like meant to
use by arch file only in kernel space.
missed link;
[1] http://dpdk.org/dev/patchwork/patch/9365/
IMO, it is worth keeping one special device file who could work across
archs like arm/arm64/powerpc and others, who could map iopci bar to
dpdk user-space. also this approach has no kernel version dependency
too. BTW; I did mentioned in second approach in to add /dev/ioport
interface in drivers/char/mem.c which could read more than byte in one
single operation, but that has kernel dependency. However that
approach too is arch agnostic.
Your first approach use an out-of-tree kernel module (igb_uio), so we cannot
really say there is no kernel dependency.
Agree but I mentioned kernel __version__ dependency.
Yes you did.
One of the main issue with out-of-tree kernel modules is the version
dependency. Probably that igb_uio from DPDK 2.3 will not compile with
the kernel 5.0.
don't know kernel 5.0 feature list so I guess your may be right. is
uio obsoleted for 5.0 kernel?
Post by Thomas Monjalon
Post by Santosh Shukla
Post by Thomas Monjalon
We should try to remove the need for any out-of-tree kernel module.
That's why the Linux upstream approach is a better solution.
IIUC, your suggesting archs like arm/arm64 to support io_mappe_io in
pci_mmap_page_range()?
I don't know what is the best solution in the kernel.
First we need to be sure that there is absolutely no solution without
kernel changes.
I guess we have done enough evaluation / investigation that suggest -
so to map iopci region to userspace in arch agnostic-way -

# either we need to modify kernel
- Make sure all the non-x86 arch to support mapping for
iopci region (i.e. pci_mmap_page_range). I don;t think its a correct
approach though.
or
- include /dev/ioport char-mem device file who could do
more than byte operation, Note that this implementation does not exist
in kernel. I could send an RFC to lkml.

OR keep device file in user space (current approach)
Right now Virtio-for-arm patches are blocked on this, let me know if
someone has better approach/thought in mind.

Thanks.
Post by Thomas Monjalon
Then we can try a pci_mmap solution or, as you suggest, an interface in
drivers/char/mem.c
Yuanhan Liu
2015-12-18 05:30:53 UTC
Permalink
Post by Santosh Shukla
Post by Thomas Monjalon
Post by Santosh Shukla
Post by Thomas Monjalon
Post by Santosh Shukla
IMO, it is worth keeping one special device file who could work across
archs like arm/arm64/powerpc and others, who could map iopci bar to
dpdk user-space. also this approach has no kernel version dependency
too. BTW; I did mentioned in second approach in to add /dev/ioport
interface in drivers/char/mem.c which could read more than byte in one
single operation, but that has kernel dependency. However that
approach too is arch agnostic.
Your first approach use an out-of-tree kernel module (igb_uio), so we cannot
really say there is no kernel dependency.
Agree but I mentioned kernel __version__ dependency.
Yes you did.
One of the main issue with out-of-tree kernel modules is the version
dependency. Probably that igb_uio from DPDK 2.3 will not compile with
the kernel 5.0.
don't know kernel 5.0 feature list so I guess your may be right. is
uio obsoleted for 5.0 kernel?
Nope, Thomas meant to say that we should keep the old DPDK will work
with newer kernel, not just the newest DPDK work on newest linux kernel
only. The out-of-tree kernel makes no garantee on that.
Post by Santosh Shukla
Post by Thomas Monjalon
Post by Santosh Shukla
Post by Thomas Monjalon
We should try to remove the need for any out-of-tree kernel module.
That's why the Linux upstream approach is a better solution.
IIUC, your suggesting archs like arm/arm64 to support io_mappe_io in
pci_mmap_page_range()?
I don't know what is the best solution in the kernel.
First we need to be sure that there is absolutely no solution without
kernel changes.
I guess we have done enough evaluation / investigation that suggest -
so to map iopci region to userspace in arch agnostic-way -
# either we need to modify kernel
- Make sure all the non-x86 arch to support mapping for
iopci region (i.e. pci_mmap_page_range). I don;t think its a correct
approach though.
or
- include /dev/ioport char-mem device file who could do
more than byte operation, Note that this implementation does not exist
in kernel. I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks from
those kernel/ARM gurus? Please cc me if you do so.

--yliu
Post by Santosh Shukla
OR keep device file in user space (current approach)
Right now Virtio-for-arm patches are blocked on this, let me know if
someone has better approach/thought in mind.
Thanks.
Post by Thomas Monjalon
Then we can try a pci_mmap solution or, as you suggest, an interface in
drivers/char/mem.c
Jerin Jacob
2015-12-18 06:34:48 UTC
Permalink
Post by Yuanhan Liu
Post by Santosh Shukla
Post by Thomas Monjalon
Post by Santosh Shukla
Post by Thomas Monjalon
Post by Santosh Shukla
IMO, it is worth keeping one special device file who could work across
archs like arm/arm64/powerpc and others, who could map iopci bar to
dpdk user-space. also this approach has no kernel version dependency
too. BTW; I did mentioned in second approach in to add /dev/ioport
interface in drivers/char/mem.c which could read more than byte in one
single operation, but that has kernel dependency. However that
approach too is arch agnostic.
Your first approach use an out-of-tree kernel module (igb_uio), so we cannot
really say there is no kernel dependency.
Agree but I mentioned kernel __version__ dependency.
Yes you did.
One of the main issue with out-of-tree kernel modules is the version
dependency. Probably that igb_uio from DPDK 2.3 will not compile with
the kernel 5.0.
don't know kernel 5.0 feature list so I guess your may be right. is
uio obsoleted for 5.0 kernel?
Nope, Thomas meant to say that we should keep the old DPDK will work
with newer kernel, not just the newest DPDK work on newest linux kernel
only. The out-of-tree kernel makes no garantee on that.
Post by Santosh Shukla
Post by Thomas Monjalon
Post by Santosh Shukla
Post by Thomas Monjalon
We should try to remove the need for any out-of-tree kernel module.
That's why the Linux upstream approach is a better solution.
IIUC, your suggesting archs like arm/arm64 to support io_mappe_io in
pci_mmap_page_range()?
I don't know what is the best solution in the kernel.
First we need to be sure that there is absolutely no solution without
kernel changes.
I guess we have done enough evaluation / investigation that suggest -
so to map iopci region to userspace in arch agnostic-way -
# either we need to modify kernel
- Make sure all the non-x86 arch to support mapping for
iopci region (i.e. pci_mmap_page_range). I don;t think its a correct
approach though.
or
- include /dev/ioport char-mem device file who could do
more than byte operation, Note that this implementation does not exist
in kernel. I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks from
those kernel/ARM gurus? Please cc me if you do so.
IMO, We need to support DPDK-ARM for the old kernels also. If there is no means
to expose iomap region to userspace in the existing kernel then we add that
support through out-of-tree driver like igb_uio and when make become part of
the kernel then we can turn off of the out-of-tree driver based
on the kernel major and minor version numbers.

Jerin
Post by Yuanhan Liu
--yliu
Post by Santosh Shukla
OR keep device file in user space (current approach)
Right now Virtio-for-arm patches are blocked on this, let me know if
someone has better approach/thought in mind.
Thanks.
Post by Thomas Monjalon
Then we can try a pci_mmap solution or, as you suggest, an interface in
drivers/char/mem.c
Yuanhan Liu
2015-12-18 07:55:46 UTC
Permalink
Post by Jerin Jacob
Post by Yuanhan Liu
Post by Santosh Shukla
# either we need to modify kernel
- Make sure all the non-x86 arch to support mapping for
iopci region (i.e. pci_mmap_page_range). I don;t think its a correct
approach though.
or
- include /dev/ioport char-mem device file who could do
more than byte operation, Note that this implementation does not exist
in kernel. I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks from
those kernel/ARM gurus? Please cc me if you do so.
IMO, We need to support DPDK-ARM for the old kernels also. If there is no means
to expose iomap region to userspace in the existing kernel then we add that
support through out-of-tree driver like igb_uio and when make become part of
the kernel then we can turn off of the out-of-tree driver based
on the kernel major and minor version numbers.
Agreed.


(BTW, I have no idea why the CC list of my last reply was shortened
to Santosh only. I double checked with my sent box, the mail header
sent out looks normal:

Date: Fri, 18 Dec 2015 13:30:53 +0800 From: Yuanhan Liu <***@linux.intel.com>
To: Santosh Shukla <***@mvista.com>
Cc: Thomas Monjalon <***@6wind.com>, ***@dpdk.org, David Marchand <***@6wind.com>

Not sure it's a bug from my mutt email client, or the mailing list)

--yliu
Thomas Monjalon
2015-12-18 09:37:06 UTC
Permalink
Post by Jerin Jacob
Post by Yuanhan Liu
Post by Santosh Shukla
Post by Thomas Monjalon
One of the main issue with out-of-tree kernel modules is the version
dependency. Probably that igb_uio from DPDK 2.3 will not compile with
the kernel 5.0.
don't know kernel 5.0 feature list so I guess your may be right. is
uio obsoleted for 5.0 kernel?
Nope, Thomas meant to say that we should keep the old DPDK will work
with newer kernel, not just the newest DPDK work on newest linux kernel
only. The out-of-tree kernel makes no garantee on that.
Post by Santosh Shukla
Post by Thomas Monjalon
Post by Santosh Shukla
Post by Thomas Monjalon
We should try to remove the need for any out-of-tree kernel module.
That's why the Linux upstream approach is a better solution.
IIUC, your suggesting archs like arm/arm64 to support io_mappe_io in
pci_mmap_page_range()?
I don't know what is the best solution in the kernel.
First we need to be sure that there is absolutely no solution without
kernel changes.
I guess we have done enough evaluation / investigation that suggest -
so to map iopci region to userspace in arch agnostic-way -
# either we need to modify kernel
- Make sure all the non-x86 arch to support mapping for
iopci region (i.e. pci_mmap_page_range). I don;t think its a correct
approach though.
or
- include /dev/ioport char-mem device file who could do
more than byte operation, Note that this implementation does not exist
in kernel. I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks from
those kernel/ARM gurus? Please cc me if you do so.
IMO, We need to support DPDK-ARM for the old kernels also. If there is no means
to expose iomap region to userspace in the existing kernel then we add that
support through out-of-tree driver like igb_uio and when make become part of
the kernel then we can turn off of the out-of-tree driver based
on the kernel major and minor version numbers.
Of course, we have to support DPDK for old/current kernels.
But if we continue to implement features in out-of-tree modules without trying
to have a solution with upstream kernel, we are stuck with our legacy forever.
So, as sane policy, we should not accept new changes in our kernel modules
if there is no alternative in Linus' repo.
Santosh Shukla
2015-12-18 07:54:41 UTC
Permalink
On Fri, Dec 18, 2015 at 11:00 AM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
Post by Thomas Monjalon
Post by Santosh Shukla
Post by Thomas Monjalon
Post by Santosh Shukla
IMO, it is worth keeping one special device file who could work across
archs like arm/arm64/powerpc and others, who could map iopci bar to
dpdk user-space. also this approach has no kernel version dependency
too. BTW; I did mentioned in second approach in to add /dev/ioport
interface in drivers/char/mem.c which could read more than byte in one
single operation, but that has kernel dependency. However that
approach too is arch agnostic.
Your first approach use an out-of-tree kernel module (igb_uio), so we cannot
really say there is no kernel dependency.
Agree but I mentioned kernel __version__ dependency.
Yes you did.
One of the main issue with out-of-tree kernel modules is the version
dependency. Probably that igb_uio from DPDK 2.3 will not compile with
the kernel 5.0.
don't know kernel 5.0 feature list so I guess your may be right. is
uio obsoleted for 5.0 kernel?
Nope, Thomas meant to say that we should keep the old DPDK will work
with newer kernel, not just the newest DPDK work on newest linux kernel
only. The out-of-tree kernel makes no garantee on that.
Post by Santosh Shukla
Post by Thomas Monjalon
Post by Santosh Shukla
Post by Thomas Monjalon
We should try to remove the need for any out-of-tree kernel module.
That's why the Linux upstream approach is a better solution.
IIUC, your suggesting archs like arm/arm64 to support io_mappe_io in
pci_mmap_page_range()?
I don't know what is the best solution in the kernel.
First we need to be sure that there is absolutely no solution without
kernel changes.
I guess we have done enough evaluation / investigation that suggest -
so to map iopci region to userspace in arch agnostic-way -
# either we need to modify kernel
- Make sure all the non-x86 arch to support mapping for
iopci region (i.e. pci_mmap_page_range). I don;t think its a correct
approach though.
or
- include /dev/ioport char-mem device file who could do
more than byte operation, Note that this implementation does not exist
in kernel. I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks from
those kernel/ARM gurus? Please cc me if you do so.
The latter one I already shared old lkml thread, Pl. revisit my v1 0/6
patch [1] and in that refer [2].
Josh has already proposed to lkml but for some reason thread didn't
went far. I can restart that discussion giving dpdk use-case as an
example/ requirement.

And for the former one, I'll have to check with linux-arm why iopci
region not mappable. By grepping kernel source all i could see two
commit - latest one is 415ae101 and older one 1da177e. Both has
nothing to explain why
if (mmap_state == pci_mmap_io)
return -EINVAL;

Setting pci io region to -EINVAL, should have fundamental reason for
sure. But we'll have to check, for that I could post as a query rather
a patch to lkml.

Note that dpdk already has out-of-tree implementation for
dom0/xen-case too, it creates special device file which maps pci
resources. so keep one more igb_uio char device so to map iopci region
wont hurt much though!

[1] http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/29531
Post by Yuanhan Liu
--yliu
Post by Santosh Shukla
OR keep device file in user space (current approach)
Right now Virtio-for-arm patches are blocked on this, let me know if
someone has better approach/thought in mind.
Thanks.
Post by Thomas Monjalon
Then we can try a pci_mmap solution or, as you suggest, an interface in
drivers/char/mem.c
Yuanhan Liu
2015-12-18 08:21:39 UTC
Permalink
Post by Santosh Shukla
Post by Yuanhan Liu
Post by Santosh Shukla
I guess we have done enough evaluation / investigation that suggest -
so to map iopci region to userspace in arch agnostic-way -
# either we need to modify kernel
- Make sure all the non-x86 arch to support mapping for
iopci region (i.e. pci_mmap_page_range). I don;t think its a correct
approach though.
or
- include /dev/ioport char-mem device file who could do
more than byte operation, Note that this implementation does not exist
in kernel. I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks from
those kernel/ARM gurus? Please cc me if you do so.
The latter one I already shared old lkml thread, Pl. revisit my v1 0/6
patch [1] and in that refer [2].
Oops, sorry, a bit busy, that I didn't look at it carefully. My bad,
anyway.
Post by Santosh Shukla
Josh has already proposed to lkml but for some reason thread didn't
went far. I can restart that discussion giving dpdk use-case as an
example/ requirement.
I had a quick go through of the discussion. Both hpa and Arnd seem
to be fine with the ioctl interface on /dev/port. Have you tried
that? And if you want to restart it, ioctl might be a better option
than /dev/ioport, judging from the discussion.
Post by Santosh Shukla
And for the former one, I'll have to check with linux-arm why iopci
region not mappable. By grepping kernel source all i could see two
commit - latest one is 415ae101 and older one 1da177e. Both has
nothing to explain why
if (mmap_state == pci_mmap_io)
return -EINVAL;
Setting pci io region to -EINVAL, should have fundamental reason for
sure. But we'll have to check, for that I could post as a query rather
a patch to lkml.
No idea, such check should have been there in the very beginning
for some reason. And here is a quote from Arnd:

Only powerpc, microblaze, alpha, sparc and xtensa allow
users to mmap I/O space, even though a lot of others could.
Post by Santosh Shukla
Note that dpdk already has out-of-tree implementation for
dom0/xen-case too, it creates special device file which maps pci
resources. so keep one more igb_uio char device so to map iopci region
wont hurt much though!
Xen is not a good example here; I'm even not sure it still works
or not :)

--yliu
Post by Santosh Shukla
[1] http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/29531
Post by Yuanhan Liu
--yliu
Post by Santosh Shukla
OR keep device file in user space (current approach)
Right now Virtio-for-arm patches are blocked on this, let me know if
someone has better approach/thought in mind.
Thanks.
Post by Thomas Monjalon
Then we can try a pci_mmap solution or, as you suggest, an interface in
drivers/char/mem.c
Santosh Shukla
2015-12-18 12:55:16 UTC
Permalink
On Fri, Dec 18, 2015 at 1:51 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
Post by Yuanhan Liu
Post by Santosh Shukla
I guess we have done enough evaluation / investigation that suggest -
so to map iopci region to userspace in arch agnostic-way -
# either we need to modify kernel
- Make sure all the non-x86 arch to support mapping for
iopci region (i.e. pci_mmap_page_range). I don;t think its a correct
approach though.
or
- include /dev/ioport char-mem device file who could do
more than byte operation, Note that this implementation does not exist
in kernel. I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks from
those kernel/ARM gurus? Please cc me if you do so.
The latter one I already shared old lkml thread, Pl. revisit my v1 0/6
patch [1] and in that refer [2].
Oops, sorry, a bit busy, that I didn't look at it carefully. My bad,
anyway.
Post by Santosh Shukla
Josh has already proposed to lkml but for some reason thread didn't
went far. I can restart that discussion giving dpdk use-case as an
example/ requirement.
I had a quick go through of the discussion. Both hpa and Arnd seem
to be fine with the ioctl interface on /dev/port. Have you tried
that? And if you want to restart it, ioctl might be a better option
than /dev/ioport, judging from the discussion.
I tried legacy patch and re-writing with ioctl-way; doing changes in
dpdk port as well in kernel, had to test on quite other hw not only
arm64 though! so it will take time for me, I am travelling tomorrow so
bit delayed, We'll post patch to lkml and share dpdk-virtio feedback
perhaps by Monday.
Post by Yuanhan Liu
Post by Santosh Shukla
And for the former one, I'll have to check with linux-arm why iopci
region not mappable. By grepping kernel source all i could see two
commit - latest one is 415ae101 and older one 1da177e. Both has
nothing to explain why
if (mmap_state == pci_mmap_io)
return -EINVAL;
Setting pci io region to -EINVAL, should have fundamental reason for
sure. But we'll have to check, for that I could post as a query rather
a patch to lkml.
No idea, such check should have been there in the very beginning
Only powerpc, microblaze, alpha, sparc and xtensa allow
users to mmap I/O space, even though a lot of others could.
Post by Santosh Shukla
Note that dpdk already has out-of-tree implementation for
dom0/xen-case too, it creates special device file which maps pci
resources. so keep one more igb_uio char device so to map iopci region
wont hurt much though!
Xen is not a good example here; I'm even not sure it still works
or not :)
--yliu
Post by Santosh Shukla
[1] http://permalink.gmane.org/gmane.comp.networking.dpdk.devel/29531
Post by Yuanhan Liu
--yliu
Post by Santosh Shukla
OR keep device file in user space (current approach)
Right now Virtio-for-arm patches are blocked on this, let me know if
someone has better approach/thought in mind.
Thanks.
Post by Thomas Monjalon
Then we can try a pci_mmap solution or, as you suggest, an interface in
drivers/char/mem.c
Santosh Shukla
2015-12-29 05:56:53 UTC
Permalink
Post by Santosh Shukla
On Fri, Dec 18, 2015 at 1:51 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
Post by Yuanhan Liu
Post by Santosh Shukla
I guess we have done enough evaluation / investigation that suggest -
so to map iopci region to userspace in arch agnostic-way -
# either we need to modify kernel
- Make sure all the non-x86 arch to support mapping for
iopci region (i.e. pci_mmap_page_range). I don;t think its a correct
approach though.
or
- include /dev/ioport char-mem device file who could do
more than byte operation, Note that this implementation does not exist
in kernel. I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks from
those kernel/ARM gurus? Please cc me if you do so.
The latter one I already shared old lkml thread, Pl. revisit my v1 0/6
patch [1] and in that refer [2].
Oops, sorry, a bit busy, that I didn't look at it carefully. My bad,
anyway.
Post by Santosh Shukla
Josh has already proposed to lkml but for some reason thread didn't
went far. I can restart that discussion giving dpdk use-case as an
example/ requirement.
I had a quick go through of the discussion. Both hpa and Arnd seem
to be fine with the ioctl interface on /dev/port. Have you tried
that? And if you want to restart it, ioctl might be a better option
than /dev/ioport, judging from the discussion.
I tried legacy patch and re-writing with ioctl-way; doing changes in
dpdk port as well in kernel, had to test on quite other hw not only
arm64 though! so it will take time for me, I am travelling tomorrow so
bit delayed, We'll post patch to lkml and share dpdk-virtio feedback
perhaps by Monday.
I posted a query about /dev/ioports approach in lkml thread [1], and
Arnd suggested to use vfio framework but it looks like vfio too does
not map ioresource_io region. Same communicated to Arnd and waiting
for his reply.

In mean time I like to ask general question;
- Has anyone tried vfio/non-iommu approach for virtio pmd driver? If
not then Is there any plan? Someone planning to take up.
[1] https://lkml.org/lkml/2015/12/23/145
Burakov, Anatoly
2015-12-29 09:56:20 UTC
Permalink
Hi Santosh,
Post by Santosh Shukla
On Fri, Dec 18, 2015 at 1:51 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
Post by Yuanhan Liu
Post by Santosh Shukla
I guess we have done enough evaluation / investigation that
suggest - so to map iopci region to userspace in arch
agnostic-way -
# either we need to modify kernel
- Make sure all the non-x86 arch to support
mapping for iopci region (i.e. pci_mmap_page_range). I don;t
think its a correct approach though.
or
- include /dev/ioport char-mem device file who
could do more than byte operation, Note that this implementation
does not exist in kernel. I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks
from those kernel/ARM gurus? Please cc me if you do so.
The latter one I already shared old lkml thread, Pl. revisit my v1
0/6 patch [1] and in that refer [2].
Oops, sorry, a bit busy, that I didn't look at it carefully. My bad,
anyway.
Post by Santosh Shukla
Josh has already proposed to lkml but for some reason thread didn't
went far. I can restart that discussion giving dpdk use-case as an
example/ requirement.
I had a quick go through of the discussion. Both hpa and Arnd seem to
be fine with the ioctl interface on /dev/port. Have you tried that?
And if you want to restart it, ioctl might be a better option than
/dev/ioport, judging from the discussion.
I tried legacy patch and re-writing with ioctl-way; doing changes in
dpdk port as well in kernel, had to test on quite other hw not only
arm64 though! so it will take time for me, I am travelling tomorrow so
bit delayed, We'll post patch to lkml and share dpdk-virtio feedback
perhaps by Monday.
I posted a query about /dev/ioports approach in lkml thread [1], and Arnd
suggested to use vfio framework but it looks like vfio too does not map
ioresource_io region. Same communicated to Arnd and waiting for his reply.
In mean time I like to ask general question;
- Has anyone tried vfio/non-iommu approach for virtio pmd driver? If not
then Is there any plan? Someone planning to take up.
[1] https://lkml.org/lkml/2015/12/23/145
I have submitted a patch to support no-iommu mode, but I'm not aware of anyone trying VFIO-noiommu at all. That's probably expected since it's Christmas/New Year in a lot of places, and everyone is on a break.

That said, I'm not sure I completely understand what is it that you're asking about. The code you're referring to (in vfio_pci.c, line 854 as of kernel 4.3) is checking if a PCI BAR is available for IO (hence checking if the IORESOURCE_MEM bit is set). There isn't any "ioresource_mem" region as far as VFIO is concerned, there are only BARs, ROM, VGA and PCI config regions (linux/vfio.h, line 314 as of kernel 4.3). So if you're missing some PCI regions for VFIO to map, they would first need to be added to VFIO kernel implementation before they can be used by DPDK. That is, unless I'm misunderstanding something
Santosh Shukla
2015-12-29 10:47:28 UTC
Permalink
On Tue, Dec 29, 2015 at 3:26 PM, Burakov, Anatoly
Post by Burakov, Anatoly
Hi Santosh,
Post by Santosh Shukla
On Fri, Dec 18, 2015 at 1:51 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
Post by Yuanhan Liu
Post by Santosh Shukla
I guess we have done enough evaluation / investigation that
suggest - so to map iopci region to userspace in arch
agnostic-way -
# either we need to modify kernel
- Make sure all the non-x86 arch to support
mapping for iopci region (i.e. pci_mmap_page_range). I don;t
think its a correct approach though.
or
- include /dev/ioport char-mem device file who
could do more than byte operation, Note that this implementation
does not exist in kernel. I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks
from those kernel/ARM gurus? Please cc me if you do so.
The latter one I already shared old lkml thread, Pl. revisit my v1
0/6 patch [1] and in that refer [2].
Oops, sorry, a bit busy, that I didn't look at it carefully. My bad,
anyway.
Post by Santosh Shukla
Josh has already proposed to lkml but for some reason thread didn't
went far. I can restart that discussion giving dpdk use-case as an
example/ requirement.
I had a quick go through of the discussion. Both hpa and Arnd seem to
be fine with the ioctl interface on /dev/port. Have you tried that?
And if you want to restart it, ioctl might be a better option than
/dev/ioport, judging from the discussion.
I tried legacy patch and re-writing with ioctl-way; doing changes in
dpdk port as well in kernel, had to test on quite other hw not only
arm64 though! so it will take time for me, I am travelling tomorrow so
bit delayed, We'll post patch to lkml and share dpdk-virtio feedback
perhaps by Monday.
I posted a query about /dev/ioports approach in lkml thread [1], and Arnd
suggested to use vfio framework but it looks like vfio too does not map
ioresource_io region. Same communicated to Arnd and waiting for his reply.
In mean time I like to ask general question;
- Has anyone tried vfio/non-iommu approach for virtio pmd driver? If not
then Is there any plan? Someone planning to take up.
[1] https://lkml.org/lkml/2015/12/23/145
I have submitted a patch to support no-iommu mode, but I'm not aware of anyone trying VFIO-noiommu at all. That's probably expected since it's Christmas/New Year in a lot of places, and everyone is on a break.
That said, I'm not sure I completely understand what is it that you're asking about. The code you're referring to (in vfio_pci.c, line 854 as of kernel 4.3) is checking if a PCI BAR is available for IO (hence checking if the IORESOURCE_MEM
Thanks for reply! You comment might help to move this discuss to next level.

Look at kernel/resource.c, it exports two symbol ioport_resource and
iomem_resource and sets appropriate flag type i.e.. IORESOURCE_IO and
IORESOURCE_MEM. In virtio-net case; it creates both pci region i.e..
_io bar and _mem bar. dpdk virtio pmd driver (<= 0.95 virtio spec)
uses pci _io bar region for device initialization as virtio headers
are locate at pci _io bar region. Since it uses pci _iobar region so
likely it update pci_resource.[index].flag = IORESOURCE_IO. and vfio
mmap function wont handle ioresource_io (i guess). And that is why I
asked same to lkml thread.


bit is set). There isn't any "ioresource_mem" region as far as VFIO is
concerned, there are only BARs, ROM, VGA and PCI
config regions (linux/vfio.h, line 314 as of kernel 4.3). So if you're
missing some PCI regions for VFIO to map, they would first need to be
added to VFIO kernel implementation before they can be used by DPDK.
That is, unless I'm misunderstanding something :)
Agree. As mentioned above, I guess ioresource_io pci bar
implementation missing in vfio kernel? but before adding code support
in kernel I would like to hear from experts example, You, Alex!
(looping alex)
Post by Burakov, Anatoly
Thanks,
Anatoly
Burakov, Anatoly
2015-12-29 11:06:25 UTC
Permalink
Hi Santosh,
Post by Santosh Shukla
Look at kernel/resource.c, it exports two symbol ioport_resource and
iomem_resource and sets appropriate flag type i.e.. IORESOURCE_IO and
IORESOURCE_MEM. In virtio-net case; it creates both pci region i.e..
_io bar and _mem bar. dpdk virtio pmd driver (<= 0.95 virtio spec) uses pci
_io bar region for device initialization as virtio headers are locate at pci _io bar
region. Since it uses pci _iobar region so likely it update
pci_resource.[index].flag = IORESOURCE_IO. and vfio mmap function wont
handle ioresource_io (i guess). And that is why I asked same to lkml thread.
Yes, I can see that this is what I was misunderstanding (that IORESOURCE_IO isn't a BAR but rather a flag). Technically, if you also set IORESOURCE_MEM along with IORESOURCE_IO, then the BAR would be mapped by VFIO, but this sounds like a dangerous hack :) other than that, yes, I'm afraid it's up to kernel guys to add support for it. Once done, provided IO BARs are meant to be worked with the same way as MEM BARs, there's a good chance it would work out of the
Santosh Shukla
2015-12-29 12:23:25 UTC
Permalink
On Tue, Dec 29, 2015 at 4:36 PM, Burakov, Anatoly
Post by Burakov, Anatoly
Hi Santosh,
Post by Santosh Shukla
Look at kernel/resource.c, it exports two symbol ioport_resource and
iomem_resource and sets appropriate flag type i.e.. IORESOURCE_IO and
IORESOURCE_MEM. In virtio-net case; it creates both pci region i.e..
_io bar and _mem bar. dpdk virtio pmd driver (<= 0.95 virtio spec) uses pci
_io bar region for device initialization as virtio headers are locate at pci _io bar
region. Since it uses pci _iobar region so likely it update
pci_resource.[index].flag = IORESOURCE_IO. and vfio mmap function wont
handle ioresource_io (i guess). And that is why I asked same to lkml thread.
Yes, I can see that this is what I was misunderstanding (that IORESOURCE_IO isn't a BAR but rather a flag). Technically, if you also set IORESOURCE_MEM along with IORESOURCE_IO, then the BAR would be mapped by VFIO, but this sounds like a dangerous hack :) other than that, yes, I'm afraid it's up to kernel guys to add support for it. Once done, provided IO BARs are meant to be worked with the same way as MEM BARs, there's a good chance it would work out of the box with DPDK.
I guess so, I'll give it a try; although before that I need to port /
use your vfio/noiommu patch for virtio pmd driver and perhaps do
dependant changes, I will post my test feedback.

Thanks
Post by Burakov, Anatoly
Thanks,
Anatoly
Alex Williamson
2015-12-29 14:04:38 UTC
Permalink
Post by Santosh Shukla
On Tue, Dec 29, 2015 at 3:26 PM, Burakov, Anatoly
Post by Burakov, Anatoly
Hi Santosh,
om>
Post by Santosh Shukla
On Fri, Dec 18, 2015 at 1:51 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
Post by Yuanhan Liu
Post by Santosh Shukla
I guess we have done enough evaluation / investigation that
suggest - so to map iopci region to userspace in arch
agnostic-way -
# either we need to modify kernel
               - Make sure all the non-x86 arch to
support
mapping for iopci region (i.e. pci_mmap_page_range). I don;t
think its a correct approach though.
            or
               - include /dev/ioport char-mem device
file who
could do more than byte operation, Note that this
implementation
does not exist in kernel.  I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks
from those kernel/ARM gurus? Please cc me if you do so.
The latter one I already shared old lkml thread, Pl.
revisit my v1
0/6 patch [1] and in that refer [2].
Oops, sorry, a bit busy, that I didn't look at it carefully. My bad,
anyway.
Post by Santosh Shukla
Josh has already proposed to lkml but for some reason thread didn't
went far. I can restart that discussion giving dpdk use-
case as an
example/ requirement.
I had a quick go through of the discussion. Both hpa and Arnd seem to
be fine with the ioctl interface on /dev/port. Have you tried that?
And if you want to restart it, ioctl might be a better option than
/dev/ioport, judging from the discussion.
I tried legacy patch and re-writing with ioctl-way; doing changes in
dpdk port as well in kernel, had to test on quite other hw not only
arm64 though! so it will take time for me, I am travelling tomorrow so
bit delayed, We'll post patch to lkml and share dpdk-virtio feedback
perhaps by Monday.
I posted a query about /dev/ioports approach in lkml thread [1], and Arnd
suggested to use vfio framework but it looks like vfio too does not map
ioresource_io region. Same communicated to Arnd and waiting for his reply.
In mean time I like to ask general question;
- Has anyone tried vfio/non-iommu approach for virtio pmd driver? If not
then Is there any plan? Someone planning to take up.
[1] https://lkml.org/lkml/2015/12/23/145
I have submitted a patch to support no-iommu mode, but I'm not
aware of anyone trying VFIO-noiommu at all. That's probably
expected since it's Christmas/New Year in a lot of places, and
everyone is on a break.
That said, I'm not sure I completely understand what is it that
you're asking about. The code you're referring to (in vfio_pci.c,
line 854 as of kernel 4.3) is checking if a PCI BAR is available
for IO (hence checking if the IORESOURCE_MEM
Thanks for reply! You comment might help to move this discuss to next level.
Look at kernel/resource.c, it exports two symbol ioport_resource and
iomem_resource and sets appropriate flag type i.e.. IORESOURCE_IO and
IORESOURCE_MEM. In virtio-net case; it creates both pci region i.e..
_io bar and _mem bar. dpdk virtio pmd driver (<= 0.95 virtio spec)
uses pci _io bar region for device initialization as virtio headers
are locate at pci _io bar region. Since it uses pci _iobar region so
likely it update pci_resource.[index].flag = IORESOURCE_IO.  and vfio
mmap function wont handle ioresource_io (i guess). And that is why I
asked same to lkml thread.
bit is set). There isn't any "ioresource_mem" region as far as VFIO is
concerned, there are only BARs, ROM, VGA and PCI
config regions (linux/vfio.h, line 314 as of kernel 4.3). So if you're
missing some PCI regions for VFIO to map, they would first need to be
added to VFIO kernel implementation before they can be used by DPDK.
That is, unless I'm misunderstanding something :)
Agree. As mentioned above, I guess ioresource_io pci bar
implementation missing in vfio kernel? but before adding code support
in kernel I would like to hear from experts example, You, Alex!
(looping alex)
MMIO and I/O port space are simply regions as far as VFIO is concerned.
 The userspace API supports the concept of I/O port regions that can be
mmap'd by userspace, but the implementation does not since I/O port
regions cannot be mmap'd on x86.  Someone needs to propose some code
for vfio-pci to support it.  Thanks,

Alex
Santosh Shukla
2015-12-29 14:51:48 UTC
Permalink
On Tue, Dec 29, 2015 at 7:34 PM, Alex Williamson
Post by Alex Williamson
Post by Santosh Shukla
On Tue, Dec 29, 2015 at 3:26 PM, Burakov, Anatoly
Post by Burakov, Anatoly
Hi Santosh,
om>
Post by Santosh Shukla
On Fri, Dec 18, 2015 at 1:51 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
Post by Yuanhan Liu
Post by Santosh Shukla
I guess we have done enough evaluation / investigation that
suggest - so to map iopci region to userspace in arch
agnostic-way -
# either we need to modify kernel
- Make sure all the non-x86 arch to support
mapping for iopci region (i.e. pci_mmap_page_range). I don;t
think its a correct approach though.
or
- include /dev/ioport char-mem device file who
could do more than byte operation, Note that this
implementation
does not exist in kernel. I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks
from those kernel/ARM gurus? Please cc me if you do so.
The latter one I already shared old lkml thread, Pl. revisit my v1
0/6 patch [1] and in that refer [2].
Oops, sorry, a bit busy, that I didn't look at it carefully. My bad,
anyway.
Post by Santosh Shukla
Josh has already proposed to lkml but for some reason thread didn't
went far. I can restart that discussion giving dpdk use-
case as an
example/ requirement.
I had a quick go through of the discussion. Both hpa and Arnd seem to
be fine with the ioctl interface on /dev/port. Have you tried that?
And if you want to restart it, ioctl might be a better option than
/dev/ioport, judging from the discussion.
I tried legacy patch and re-writing with ioctl-way; doing changes in
dpdk port as well in kernel, had to test on quite other hw not only
arm64 though! so it will take time for me, I am travelling tomorrow so
bit delayed, We'll post patch to lkml and share dpdk-virtio feedback
perhaps by Monday.
I posted a query about /dev/ioports approach in lkml thread [1], and Arnd
suggested to use vfio framework but it looks like vfio too does not map
ioresource_io region. Same communicated to Arnd and waiting for his reply.
In mean time I like to ask general question;
- Has anyone tried vfio/non-iommu approach for virtio pmd driver? If not
then Is there any plan? Someone planning to take up.
[1] https://lkml.org/lkml/2015/12/23/145
I have submitted a patch to support no-iommu mode, but I'm not
aware of anyone trying VFIO-noiommu at all. That's probably
expected since it's Christmas/New Year in a lot of places, and
everyone is on a break.
That said, I'm not sure I completely understand what is it that
you're asking about. The code you're referring to (in vfio_pci.c,
line 854 as of kernel 4.3) is checking if a PCI BAR is available
for IO (hence checking if the IORESOURCE_MEM
Thanks for reply! You comment might help to move this discuss to next level.
Look at kernel/resource.c, it exports two symbol ioport_resource and
iomem_resource and sets appropriate flag type i.e.. IORESOURCE_IO and
IORESOURCE_MEM. In virtio-net case; it creates both pci region i.e..
_io bar and _mem bar. dpdk virtio pmd driver (<= 0.95 virtio spec)
uses pci _io bar region for device initialization as virtio headers
are locate at pci _io bar region. Since it uses pci _iobar region so
likely it update pci_resource.[index].flag = IORESOURCE_IO. and vfio
mmap function wont handle ioresource_io (i guess). And that is why I
asked same to lkml thread.
bit is set). There isn't any "ioresource_mem" region as far as VFIO is
concerned, there are only BARs, ROM, VGA and PCI
config regions (linux/vfio.h, line 314 as of kernel 4.3). So if you're
missing some PCI regions for VFIO to map, they would first need to be
added to VFIO kernel implementation before they can be used by DPDK.
That is, unless I'm misunderstanding something :)
Agree. As mentioned above, I guess ioresource_io pci bar
implementation missing in vfio kernel? but before adding code support
in kernel I would like to hear from experts example, You, Alex!
(looping alex)
MMIO and I/O port space are simply regions as far as VFIO is concerned.
The userspace API supports the concept of I/O port regions that can be
mmap'd by userspace, but the implementation does not since I/O port
regions cannot be mmap'd on x86. Someone needs to propose some code
for vfio-pci to support it. Thanks,
I will work on this and post an RFC soon to LKML, That RFC targetted
to map IOport region for non-x86 platform in particular. Thanks for
confirming the vfio behaviour.
Post by Alex Williamson
Alex
Santosh Shukla
2015-12-31 14:27:44 UTC
Permalink
Post by Santosh Shukla
On Tue, Dec 29, 2015 at 7:34 PM, Alex Williamson
Post by Alex Williamson
Post by Santosh Shukla
On Tue, Dec 29, 2015 at 3:26 PM, Burakov, Anatoly
Post by Burakov, Anatoly
Hi Santosh,
om>
Post by Santosh Shukla
On Fri, Dec 18, 2015 at 1:51 PM, Yuanhan Liu
Post by Yuanhan Liu
Post by Santosh Shukla
Post by Yuanhan Liu
Post by Santosh Shukla
I guess we have done enough evaluation / investigation that
suggest - so to map iopci region to userspace in arch
agnostic-way -
# either we need to modify kernel
- Make sure all the non-x86 arch to support
mapping for iopci region (i.e. pci_mmap_page_range). I
don;t
think its a correct approach though.
or
- include /dev/ioport char-mem device
file who
could do more than byte operation, Note that this
implementation
does not exist in kernel. I could send an RFC to lkml.
Maybe you could propose the two to lkml, to get some feedbacks
from those kernel/ARM gurus? Please cc me if you do so.
The latter one I already shared old lkml thread, Pl. revisit my v1
0/6 patch [1] and in that refer [2].
Oops, sorry, a bit busy, that I didn't look at it carefully. My bad,
anyway.
Post by Santosh Shukla
Josh has already proposed to lkml but for some reason thread didn't
went far. I can restart that discussion giving dpdk use-
case as an
example/ requirement.
I had a quick go through of the discussion. Both hpa and Arnd seem to
be fine with the ioctl interface on /dev/port. Have you tried that?
And if you want to restart it, ioctl might be a better option than
/dev/ioport, judging from the discussion.
I tried legacy patch and re-writing with ioctl-way; doing changes in
dpdk port as well in kernel, had to test on quite other hw not only
arm64 though! so it will take time for me, I am travelling tomorrow so
bit delayed, We'll post patch to lkml and share dpdk-virtio feedback
perhaps by Monday.
I posted a query about /dev/ioports approach in lkml thread [1], and Arnd
suggested to use vfio framework but it looks like vfio too does not map
ioresource_io region. Same communicated to Arnd and waiting for his reply.
In mean time I like to ask general question;
- Has anyone tried vfio/non-iommu approach for virtio pmd driver? If not
then Is there any plan? Someone planning to take up.
[1] https://lkml.org/lkml/2015/12/23/145
I have submitted a patch to support no-iommu mode, but I'm not
aware of anyone trying VFIO-noiommu at all. That's probably
expected since it's Christmas/New Year in a lot of places, and
everyone is on a break.
That said, I'm not sure I completely understand what is it that
you're asking about. The code you're referring to (in vfio_pci.c,
line 854 as of kernel 4.3) is checking if a PCI BAR is available
for IO (hence checking if the IORESOURCE_MEM
Thanks for reply! You comment might help to move this discuss to next level.
Look at kernel/resource.c, it exports two symbol ioport_resource and
iomem_resource and sets appropriate flag type i.e.. IORESOURCE_IO and
IORESOURCE_MEM. In virtio-net case; it creates both pci region i.e..
_io bar and _mem bar. dpdk virtio pmd driver (<= 0.95 virtio spec)
uses pci _io bar region for device initialization as virtio headers
are locate at pci _io bar region. Since it uses pci _iobar region so
likely it update pci_resource.[index].flag = IORESOURCE_IO. and vfio
mmap function wont handle ioresource_io (i guess). And that is why I
asked same to lkml thread.
bit is set). There isn't any "ioresource_mem" region as far as VFIO is
concerned, there are only BARs, ROM, VGA and PCI
config regions (linux/vfio.h, line 314 as of kernel 4.3). So if you're
missing some PCI regions for VFIO to map, they would first need to be
added to VFIO kernel implementation before they can be used by DPDK.
That is, unless I'm misunderstanding something :)
Agree. As mentioned above, I guess ioresource_io pci bar
implementation missing in vfio kernel? but before adding code support
in kernel I would like to hear from experts example, You, Alex!
(looping alex)
MMIO and I/O port space are simply regions as far as VFIO is concerned.
The userspace API supports the concept of I/O port regions that can be
mmap'd by userspace, but the implementation does not since I/O port
regions cannot be mmap'd on x86. Someone needs to propose some code
for vfio-pci to support it. Thanks,
I will work on this and post an RFC soon to LKML, That RFC targetted
to map IOport region for non-x86 platform in particular. Thanks for
confirming the vfio behaviour.
Some update:

Patches used for setup -
- Used Alex(s) vfio-noiommu linux-next patch
- then used Anatoly(s) vfio-noiommu dpdk patch [1]
- Added ioport pci bar map code support in kernel
- Did similar change at dpdk(s) eal_pci_vfio.c file

I could able to run testpmd on virtio-net dpdk's pmd driver..
I am cleaning the patches / changes in kernel and dpdk, soon posting
the v3 patchset..

Thanks for the input!

[1] http://dpdk.org/dev/patchwork/patch/9619/
Post by Santosh Shukla
Post by Alex Williamson
Alex
Bruce Richardson
2015-12-16 13:15:41 UTC
Permalink
Post by David Marchand
x86 requires a special set of instructions to access ioports, but other
architectures let you remap io resources.
So let eal remap io resources by accepting IORESOURCE_IO flag for
architectures other than x86.
---
lib/librte_eal/common/include/rte_pci.h | 3 ++-
lib/librte_eal/linuxapp/eal/eal_pci.c | 21 +++++++++++++++------
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 334c12e..8aaab4a 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -105,7 +105,8 @@ extern struct pci_device_list pci_device_list; /**< Global list of PCI devices.
/** Nb. of values in PCI resource format. */
#define PCI_RESOURCE_FMT_NVAL 3
-/** IO resource type: memory address space */
+/** IO resource type: */
+#define IORESOURCE_IO 0x00000100
#define IORESOURCE_MEM 0x00000200
/**
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index bc5b5be..9c4651d 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -236,12 +236,21 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
goto error;
}
- if (flags & IORESOURCE_MEM) {
- dev->mem_resource[i].phys_addr = phys_addr;
- dev->mem_resource[i].len = end_addr - phys_addr + 1;
- /* not mapped for now */
- dev->mem_resource[i].addr = NULL;
- }
+ /* we only care about IORESOURCE_IO or IORESOURCE_MEM */
+ if (!(flags & IORESOURCE_IO) &&
+ !(flags & IORESOURCE_MEM))
+ continue;
+
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
+ /* x86 can not remap ioports, so skip it, remapping code will
+ * look at dev->mem_resource[i].phys_addr == 0 and skip it */
+ if (flags & IORESOURCE_IO)
+ continue;
+#endif
As a tangential comment: We maybe could look to make certain preprocessor
defines available as C globals as well. There is no reason that the ifdef here
could not be implemented as a runtime check in C code.

/Bruce
David Marchand
2015-12-16 13:29:21 UTC
Permalink
Bruce,

On Wed, Dec 16, 2015 at 2:15 PM, Bruce Richardson <
Post by Bruce Richardson
Post by Santosh Shukla
+#if defined(RTE_ARCH_X86_64) || defined(RTE_ARCH_I686)
+ /* x86 can not remap ioports, so skip it, remapping code
will
Post by Santosh Shukla
+ * look at dev->mem_resource[i].phys_addr == 0 and skip it
*/
Post by Santosh Shukla
+ if (flags & IORESOURCE_IO)
+ continue;
+#endif
As a tangential comment: We maybe could look to make certain preprocessor
defines available as C globals as well. There is no reason that the ifdef here
could not be implemented as a runtime check in C code.
Well, instead of having the same information as the preprocessor define,
maybe some capability per arch/cpu would be better "arch supports io remap".
Maybe we can extend the cpuflags ?
--
David Marchand
Continue reading on narkive:
Loading...