From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by dpdk.org (Postfix) with ESMTP id 843352B91; Fri, 6 Jan 2017 11:14:38 +0100 (CET) Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga104.jf.intel.com with ESMTP; 06 Jan 2017 02:14:38 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,323,1477983600"; d="scan'208";a="919564020" Received: from yliu-dev.sh.intel.com ([10.239.67.162]) by orsmga003.jf.intel.com with ESMTP; 06 Jan 2017 02:14:36 -0800 From: Yuanhan Liu To: dev@dpdk.org Cc: Yuanhan Liu , stable@dpdk.org, Juho Snellman , Yaron Illouz Date: Fri, 6 Jan 2017 18:16:19 +0800 Message-Id: <1483697780-12088-6-git-send-email-yuanhan.liu@linux.intel.com> X-Mailer: git-send-email 1.9.0 In-Reply-To: <1483697780-12088-1-git-send-email-yuanhan.liu@linux.intel.com> References: <1482922962-21036-1-git-send-email-yuanhan.liu@linux.intel.com> <1483697780-12088-1-git-send-email-yuanhan.liu@linux.intel.com> Subject: [dpdk-dev] [PATCH v3 5/6] net/virtio: fix multiple process support X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 06 Jan 2017 10:14:39 -0000 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@dpdk.org Cc: Juho Snellman Cc: Yaron Illouz Reported-by: Juho Snellman Reported-by: Yaron Illouz Signed-off-by: Yuanhan Liu --- v3: - update release note and nic matrix v2: - fixed PCI remap, so that virtio 1.0 also works --- doc/guides/nics/features/virtio.ini | 1 + doc/guides/rel_notes/release_17_02.rst | 5 ++++ 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 +- 6 files changed, 64 insertions(+), 5 deletions(-) diff --git a/doc/guides/nics/features/virtio.ini b/doc/guides/nics/features/virtio.ini index 41830c1..f5de291 100644 --- a/doc/guides/nics/features/virtio.ini +++ b/doc/guides/nics/features/virtio.ini @@ -22,3 +22,4 @@ ARMv8 = Y x86-32 = Y x86-64 = Y Usage doc = Y +Multiprocess aware = Y diff --git a/doc/guides/rel_notes/release_17_02.rst b/doc/guides/rel_notes/release_17_02.rst index 3b65038..a4260de 100644 --- a/doc/guides/rel_notes/release_17_02.rst +++ b/doc/guides/rel_notes/release_17_02.rst @@ -54,6 +54,11 @@ Resolved Issues Also, make sure to start the actual text at the margin. ========================================================= + * **fixed virtio multiple process support.** + + Fixed few regressions introduced in recent releases that break the virtio + multiple process support. + EAL ~~~ 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 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev, } /* + * 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 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *dev, 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 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *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 @@ static int virtio_dev_xstats_get_names(struct rte_eth_dev *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 @@ 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 @@ 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 @@ int vtpci_init(struct rte_pci_device *, 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 @@ 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, -- 1.9.0