From: Yuanhan Liu <yuanhan.liu@linux.intel.com>
To: dev@dpdk.org
Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>,
stable@kernel.org, Juho Snellman <jsnell@iki.fi>,
Yaron Illouz <yaroni@radcom.com>
Subject: [dpdk-dev] [PATCH v2 5/6] net/virtio: fix multiple process support
Date: Wed, 28 Dec 2016 19:02:41 +0800 [thread overview]
Message-ID: <1482922962-21036-6-git-send-email-yuanhan.liu@linux.intel.com> (raw)
In-Reply-To: <1482922962-21036-1-git-send-email-yuanhan.liu@linux.intel.com>
The introduce of virtio 1.0 support brings yet another set of ops, badly,
it's not handled correctly, that it breaks the multiple process support.
The issue is the data/function pointer may vary from different processes,
and the old used to do one time set (for primary process only). That
said, the function pointer the secondary process saw is actually from the
primary process space. Accessing it could likely result to a crash.
Kudos to the last patches, we now be able to maintain those info that may
vary among different process locally, meaning every process could have its
own copy for each of them, with the correct value set. And this is what
this patch does:
- remap the PCI (IO port for legacy device and memory map for modern
device)
- set vtpci_ops correctly
After that, multiple process would work like a charm. (At least, it
passed my fuzzy test)
Fixes: b8f04520ad71 ("virtio: use PCI ioport API")
Fixes: d5bbeefca826 ("virtio: introduce PCI implementation structure")
Fixes: 6ba1f63b5ab0 ("virtio: support specification 1.0")
Cc: stable@kernel.org
Cc: Juho Snellman <jsnell@iki.fi>
Cc: Yaron Illouz <yaroni@radcom.com>
Reported-by: Juho Snellman <jsnell@iki.fi>
Reported-by: Yaron Illouz <yaroni@radcom.com>
Signed-off-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
---
drivers/net/virtio/virtio_ethdev.c | 53 +++++++++++++++++++++++++++++++--
drivers/net/virtio/virtio_pci.c | 4 +--
drivers/net/virtio/virtio_pci.h | 4 +++
drivers/net/virtio/virtio_user_ethdev.c | 2 +-
4 files changed, 58 insertions(+), 5 deletions(-)
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 5567aa2..19d4348 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1289,6 +1289,49 @@ virtio_init_device(struct rte_eth_dev *eth_dev, uint64_t req_features)
}
/*
+ * Remap the PCI device again (IO port map for legacy device and
+ * memory map for modern device), so that the secondary process
+ * could have the PCI initiated correctly.
+ */
+static int
+virtio_remap_pci(struct rte_pci_device *pci_dev, struct virtio_hw *hw)
+{
+ if (hw->modern) {
+ /*
+ * We don't have to re-parse the PCI config space, since
+ * rte_eal_pci_map_device() makes sure the mapped address
+ * in secondary process would equal to the one mapped in
+ * the primary process: error will be returned if that
+ * requirement is not met.
+ *
+ * That said, we could simply reuse all cap pointers
+ * (such as dev_cfg, common_cfg, etc.) parsed from the
+ * primary process, which is stored in shared memory.
+ */
+ if (rte_eal_pci_map_device(pci_dev)) {
+ PMD_INIT_LOG(DEBUG, "failed to map pci device!");
+ return -1;
+ }
+ } else {
+ if (rte_eal_pci_ioport_map(pci_dev, 0, VTPCI_IO(hw)) < 0)
+ return -1;
+ }
+
+ return 0;
+}
+
+static void
+virtio_set_vtpci_ops(struct rte_pci_device *pci_dev, struct virtio_hw *hw)
+{
+ if (pci_dev == NULL)
+ VTPCI_OPS(hw) = &virtio_user_ops;
+ else if (hw->modern)
+ VTPCI_OPS(hw) = &modern_ops;
+ else
+ VTPCI_OPS(hw) = &legacy_ops;
+}
+
+/*
* This function is based on probe() function in virtio_pci.c
* It returns 0 on success.
*/
@@ -1296,7 +1339,7 @@ int
eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
{
struct virtio_hw *hw = eth_dev->data->dev_private;
- struct rte_pci_device *pci_dev;
+ struct rte_pci_device *pci_dev = eth_dev->pci_dev;
uint32_t dev_flags = RTE_ETH_DEV_DETACHABLE;
int ret;
@@ -1306,6 +1349,13 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
eth_dev->tx_pkt_burst = &virtio_xmit_pkts;
if (rte_eal_process_type() == RTE_PROC_SECONDARY) {
+ if (pci_dev) {
+ ret = virtio_remap_pci(pci_dev, hw);
+ if (ret)
+ return ret;
+ }
+
+ virtio_set_vtpci_ops(pci_dev, hw);
if (hw->use_simple_rxtx) {
eth_dev->tx_pkt_burst = virtio_xmit_pkts_simple;
eth_dev->rx_pkt_burst = virtio_recv_pkts_vec;
@@ -1325,7 +1375,6 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev)
}
hw->port_id = eth_dev->data->port_id;
- pci_dev = eth_dev->pci_dev;
if (pci_dev) {
ret = vtpci_init(pci_dev, hw, &dev_flags);
diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c
index d1e9c05..f5754e5 100644
--- a/drivers/net/virtio/virtio_pci.c
+++ b/drivers/net/virtio/virtio_pci.c
@@ -303,7 +303,7 @@ legacy_virtio_resource_init(struct rte_pci_device *pci_dev,
return 0;
}
-static const struct virtio_pci_ops legacy_ops = {
+const struct virtio_pci_ops legacy_ops = {
.read_dev_cfg = legacy_read_dev_config,
.write_dev_cfg = legacy_write_dev_config,
.reset = legacy_reset,
@@ -519,7 +519,7 @@ modern_notify_queue(struct virtio_hw *hw __rte_unused, struct virtqueue *vq)
io_write16(1, vq->notify_addr);
}
-static const struct virtio_pci_ops modern_ops = {
+const struct virtio_pci_ops modern_ops = {
.read_dev_cfg = modern_read_dev_config,
.write_dev_cfg = modern_write_dev_config,
.reset = modern_reset,
diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h
index 6b9aecf..511a1c8 100644
--- a/drivers/net/virtio/virtio_pci.h
+++ b/drivers/net/virtio/virtio_pci.h
@@ -333,4 +333,8 @@ uint8_t vtpci_isr(struct virtio_hw *);
uint16_t vtpci_irq_config(struct virtio_hw *, uint16_t);
+extern const struct virtio_pci_ops legacy_ops;
+extern const struct virtio_pci_ops modern_ops;
+extern const struct virtio_pci_ops virtio_user_ops;
+
#endif /* _VIRTIO_PCI_H_ */
diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c
index 7d2a9d9..3563952 100644
--- a/drivers/net/virtio/virtio_user_ethdev.c
+++ b/drivers/net/virtio/virtio_user_ethdev.c
@@ -212,7 +212,7 @@ virtio_user_notify_queue(struct virtio_hw *hw, struct virtqueue *vq)
strerror(errno));
}
-static const struct virtio_pci_ops virtio_user_ops = {
+const struct virtio_pci_ops virtio_user_ops = {
.read_dev_cfg = virtio_user_read_dev_config,
.write_dev_cfg = virtio_user_write_dev_config,
.reset = virtio_user_reset,
--
2.8.1
next prev parent reply other threads:[~2016-12-28 11:01 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-22 7:18 [dpdk-dev] [PATCH 0/3] net/virtio: a temp to fix few multiple process issues Yuanhan Liu
2016-12-22 7:18 ` [dpdk-dev] [PATCH 1/3] net/virtio: fix crash for secondary process Yuanhan Liu
2016-12-22 7:18 ` [dpdk-dev] [PATCH 2/3] net/virtio: fix multiple process support Yuanhan Liu
2016-12-22 7:18 ` [dpdk-dev] [PATCH 3/3] net/virtio: fix wrong Rx/Tx method for secondary process Yuanhan Liu
2016-12-22 7:23 ` [dpdk-dev] [PATCH 0/3] net/virtio: a temp to fix few multiple process issues Yuanhan Liu
2016-12-22 8:29 ` Xu, Qian Q
2016-12-28 11:02 ` [dpdk-dev] [PATCH v2 0/6] net/virtio: fix several " Yuanhan Liu
2016-12-28 11:02 ` [dpdk-dev] [PATCH v2 1/6] ethdev: fix port data mismatched in multiple process model Yuanhan Liu
2017-01-04 17:34 ` Thomas Monjalon
2017-01-05 6:25 ` Yuanhan Liu
2016-12-28 11:02 ` [dpdk-dev] [PATCH v2 2/6] net/virtio: fix wrong Rx/Tx method for secondary process Yuanhan Liu
2016-12-28 11:02 ` [dpdk-dev] [PATCH v2 3/6] net/virtio: store PCI operators pointer locally Yuanhan Liu
2016-12-28 11:02 ` [dpdk-dev] [PATCH v2 4/6] net/virtio: store IO port info locally Yuanhan Liu
2016-12-28 11:02 ` Yuanhan Liu [this message]
2016-12-28 11:14 ` [dpdk-dev] [PATCH v2 5/6] net/virtio: fix multiple process support Yuanhan Liu
2016-12-28 11:02 ` [dpdk-dev] [PATCH v2 6/6] net/virtio: remove dead structure field Yuanhan Liu
2017-01-06 10:16 ` [dpdk-dev] [PATCH v3 0/6] net/virtio: fix several multiple process issues Yuanhan Liu
2017-01-06 10:16 ` [dpdk-dev] [PATCH v3 1/6] ethdev: fix port data mismatched in multiple process model Yuanhan Liu
2017-01-06 13:12 ` Thomas Monjalon
2017-01-06 14:19 ` Yuanhan Liu
2017-01-09 7:50 ` [dpdk-dev] [PATCH v4] " Yuanhan Liu
2017-01-09 17:08 ` Thomas Monjalon
2017-01-10 14:33 ` Yuanhan Liu
2017-01-11 13:32 ` Thomas Monjalon
2017-01-12 3:10 ` Yuanhan Liu
2017-01-19 18:39 ` Ferruh Yigit
2017-01-20 7:58 ` Yuanhan Liu
2017-01-06 10:16 ` [dpdk-dev] [PATCH v3 2/6] net/virtio: fix wrong Rx/Tx method for secondary process Yuanhan Liu
2017-01-08 23:15 ` Stephen Hemminger
2017-01-09 5:19 ` Yuanhan Liu
2017-01-09 8:02 ` Xu, Qian Q
2017-01-09 8:05 ` Wei, FangfangX
2017-01-06 10:16 ` [dpdk-dev] [PATCH v3 3/6] net/virtio: store PCI operators pointer locally Yuanhan Liu
2017-01-06 10:16 ` [dpdk-dev] [PATCH v3 4/6] net/virtio: store IO port info locally Yuanhan Liu
2017-01-06 10:16 ` [dpdk-dev] [PATCH v3 5/6] net/virtio: fix multiple process support Yuanhan Liu
2017-01-06 10:16 ` [dpdk-dev] [PATCH v3 6/6] net/virtio: remove dead structure field Yuanhan Liu
2017-01-12 6:02 ` Yuanhan Liu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1482922962-21036-6-git-send-email-yuanhan.liu@linux.intel.com \
--to=yuanhan.liu@linux.intel.com \
--cc=dev@dpdk.org \
--cc=jsnell@iki.fi \
--cc=stable@kernel.org \
--cc=yaroni@radcom.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).