From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by dpdk.org (Postfix) with ESMTP id 3724E9E7 for ; Wed, 15 Feb 2017 07:25:13 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Feb 2017 22:25:12 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.35,164,1484035200"; d="scan'208";a="1094927665" Received: from yliu-dev.sh.intel.com ([10.239.67.162]) by orsmga001.jf.intel.com with ESMTP; 14 Feb 2017 22:25:11 -0800 From: Yuanhan Liu To: Yuanhan Liu Cc: dpdk stable Date: Wed, 15 Feb 2017 14:26:45 +0800 Message-Id: <1487140012-13314-33-git-send-email-yuanhan.liu@linux.intel.com> X-Mailer: git-send-email 1.9.0 In-Reply-To: <1487140012-13314-1-git-send-email-yuanhan.liu@linux.intel.com> References: <1487140012-13314-1-git-send-email-yuanhan.liu@linux.intel.com> Subject: [dpdk-stable] patch 'net/virtio: store PCI operators pointer locally' has been queued to stable release 16.11.1 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 15 Feb 2017 06:25:13 -0000 Hi, FYI, your patch has been queued to stable release 16.11.1 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 02/18/17. So please shout if anyone has objections. Thanks. --yliu --- >>From d6e433c489dfde08a718200885845aaaa310d1bd Mon Sep 17 00:00:00 2001 From: Yuanhan Liu Date: Fri, 6 Jan 2017 18:16:17 +0800 Subject: [PATCH] net/virtio: store PCI operators pointer locally [ upstream commit 553f45932fb797e9fbd6342016e0dd53e1f61fc7 ] We used to store the vtpci_ops at virtio_hw structure. The struct, however, is stored in shared memory. That means only one value is allowed. For the multiple process model, however, the address of vtpci_ops should be different among different processes. Take virtio PMD as example, the vtpci_ops is set by the primary process, based on its own process space. If we access that address from the secondary process, that would be an illegal memory access, A crash then might happen. To make the multiple process model work, we need store the vtpci_ops in local memory but not in a shared memory. This is what the patch does: a local virtio_hw_internal array of size RTE_MAX_ETHPORTS is allocated. This new structure is used to store all these kind of info in a non-shared memory. Current, we have: - vtpci_ops - rte_pci_ioport - virtio pci mapped memory, such as common_cfg. The later two will be done in coming patches. Later patches would also set them correctly for secondary process, so that the multiple process model could work. Signed-off-by: Yuanhan Liu --- drivers/net/virtio/virtio_ethdev.c | 9 ++++++--- drivers/net/virtio/virtio_pci.c | 26 +++++++++++++------------- drivers/net/virtio/virtio_pci.h | 17 ++++++++++++++++- drivers/net/virtio/virtio_user_ethdev.c | 3 ++- drivers/net/virtio/virtqueue.h | 2 +- 5 files changed, 38 insertions(+), 19 deletions(-) diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index 4047da5..fd0ffc2 100644 --- a/drivers/net/virtio/virtio_ethdev.c +++ b/drivers/net/virtio/virtio_ethdev.c @@ -152,6 +152,8 @@ static const struct rte_virtio_xstats_name_off rte_virtio_txq_stat_strings[] = { #define VIRTIO_NB_TXQ_XSTATS (sizeof(rte_virtio_txq_stat_strings) / \ sizeof(rte_virtio_txq_stat_strings[0])) +struct virtio_hw_internal virtio_hw_internal[RTE_MAX_ETHPORTS]; + static int virtio_send_command(struct virtnet_ctl *cvq, struct virtio_pmd_ctrl *ctrl, int *dlen, int pkt_num) @@ -360,7 +362,7 @@ virtio_init_queue(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx) * Read the virtqueue size from the Queue Size field * Always power of 2 and if 0 virtqueue does not exist */ - vq_size = hw->vtpci_ops->get_queue_num(hw, vtpci_queue_idx); + vq_size = VTPCI_OPS(hw)->get_queue_num(hw, vtpci_queue_idx); PMD_INIT_LOG(DEBUG, "vq_size: %u", vq_size); if (vq_size == 0) { PMD_INIT_LOG(ERR, "virtqueue does not exist"); @@ -519,7 +521,7 @@ virtio_init_queue(struct rte_eth_dev *dev, uint16_t vtpci_queue_idx) } } - if (hw->vtpci_ops->setup_queue(hw, vq) < 0) { + if (VTPCI_OPS(hw)->setup_queue(hw, vq) < 0) { PMD_INIT_LOG(ERR, "setup_queue failed"); return -EINVAL; } @@ -1116,7 +1118,7 @@ virtio_negotiate_features(struct virtio_hw *hw, uint64_t req_features) req_features); /* Read device(host) feature bits */ - host_features = hw->vtpci_ops->get_features(hw); + host_features = VTPCI_OPS(hw)->get_features(hw); PMD_INIT_LOG(DEBUG, "host_features before negotiate = %" PRIx64, host_features); @@ -1332,6 +1334,7 @@ eth_virtio_dev_init(struct rte_eth_dev *eth_dev) return ret; } + hw->port_id = eth_dev->data->port_id; eth_dev->data->dev_flags = dev_flags; /* reset device and negotiate default features */ diff --git a/drivers/net/virtio/virtio_pci.c b/drivers/net/virtio/virtio_pci.c index 9b47165..b1f2e18 100644 --- a/drivers/net/virtio/virtio_pci.c +++ b/drivers/net/virtio/virtio_pci.c @@ -537,14 +537,14 @@ void vtpci_read_dev_config(struct virtio_hw *hw, size_t offset, void *dst, int length) { - hw->vtpci_ops->read_dev_cfg(hw, offset, dst, length); + VTPCI_OPS(hw)->read_dev_cfg(hw, offset, dst, length); } void vtpci_write_dev_config(struct virtio_hw *hw, size_t offset, const void *src, int length) { - hw->vtpci_ops->write_dev_cfg(hw, offset, src, length); + VTPCI_OPS(hw)->write_dev_cfg(hw, offset, src, length); } uint64_t @@ -557,7 +557,7 @@ vtpci_negotiate_features(struct virtio_hw *hw, uint64_t host_features) * host all support. */ features = host_features & hw->guest_features; - hw->vtpci_ops->set_features(hw, features); + VTPCI_OPS(hw)->set_features(hw, features); return features; } @@ -565,9 +565,9 @@ vtpci_negotiate_features(struct virtio_hw *hw, uint64_t host_features) void vtpci_reset(struct virtio_hw *hw) { - hw->vtpci_ops->set_status(hw, VIRTIO_CONFIG_STATUS_RESET); + VTPCI_OPS(hw)->set_status(hw, VIRTIO_CONFIG_STATUS_RESET); /* flush status write */ - hw->vtpci_ops->get_status(hw); + VTPCI_OPS(hw)->get_status(hw); } void @@ -580,21 +580,21 @@ void vtpci_set_status(struct virtio_hw *hw, uint8_t status) { if (status != VIRTIO_CONFIG_STATUS_RESET) - status |= hw->vtpci_ops->get_status(hw); + status |= VTPCI_OPS(hw)->get_status(hw); - hw->vtpci_ops->set_status(hw, status); + VTPCI_OPS(hw)->set_status(hw, status); } uint8_t vtpci_get_status(struct virtio_hw *hw) { - return hw->vtpci_ops->get_status(hw); + return VTPCI_OPS(hw)->get_status(hw); } uint8_t vtpci_isr(struct virtio_hw *hw) { - return hw->vtpci_ops->get_isr(hw); + return VTPCI_OPS(hw)->get_isr(hw); } @@ -602,7 +602,7 @@ vtpci_isr(struct virtio_hw *hw) uint16_t vtpci_irq_config(struct virtio_hw *hw, uint16_t vec) { - return hw->vtpci_ops->set_config_irq(hw, vec); + return VTPCI_OPS(hw)->set_config_irq(hw, vec); } static void * @@ -736,8 +736,8 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw, */ if (virtio_read_caps(dev, hw) == 0) { PMD_INIT_LOG(INFO, "modern virtio pci detected."); - hw->vtpci_ops = &modern_ops; - hw->modern = 1; + virtio_hw_internal[hw->port_id].vtpci_ops = &modern_ops; + hw->modern = 1; *dev_flags |= RTE_ETH_DEV_INTR_LSC; return 0; } @@ -755,7 +755,7 @@ vtpci_init(struct rte_pci_device *dev, struct virtio_hw *hw, return -1; } - hw->vtpci_ops = &legacy_ops; + virtio_hw_internal[hw->port_id].vtpci_ops = &legacy_ops; hw->use_msix = legacy_virtio_has_msix(&dev->addr); hw->modern = 0; diff --git a/drivers/net/virtio/virtio_pci.h b/drivers/net/virtio/virtio_pci.h index de271bf..268bb82 100644 --- a/drivers/net/virtio/virtio_pci.h +++ b/drivers/net/virtio/virtio_pci.h @@ -254,6 +254,7 @@ struct virtio_hw { uint8_t use_msix; uint8_t modern; uint8_t use_simple_rxtx; + uint8_t port_id; uint8_t mac_addr[ETHER_ADDR_LEN]; uint32_t notify_off_multiplier; uint8_t *isr; @@ -261,12 +262,26 @@ struct virtio_hw { struct rte_pci_device *dev; struct virtio_pci_common_cfg *common_cfg; struct virtio_net_config *dev_cfg; - const struct virtio_pci_ops *vtpci_ops; void *virtio_user_dev; struct virtqueue **vqs; }; + +/* + * While virtio_hw is stored in shared memory, this structure stores + * some infos that may vary in the multiple process model locally. + * For example, the vtpci_ops pointer. + */ +struct virtio_hw_internal { + const struct virtio_pci_ops *vtpci_ops; +}; + +#define VTPCI_OPS(hw) (virtio_hw_internal[(hw)->port_id].vtpci_ops) + +extern struct virtio_hw_internal virtio_hw_internal[RTE_MAX_ETHPORTS]; + + /* * This structure is just a reference to read * net device specific config space; it just a chodu structure diff --git a/drivers/net/virtio/virtio_user_ethdev.c b/drivers/net/virtio/virtio_user_ethdev.c index 6ca757b..d69d8fa 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -305,7 +305,8 @@ virtio_user_eth_dev_alloc(const char *name) return NULL; } - hw->vtpci_ops = &virtio_user_ops; + hw->port_id = data->port_id; + virtio_hw_internal[hw->port_id].vtpci_ops = &virtio_user_ops; hw->use_msix = 0; hw->modern = 0; hw->use_simple_rxtx = 0; diff --git a/drivers/net/virtio/virtqueue.h b/drivers/net/virtio/virtqueue.h index f0bb089..b1070e0 100644 --- a/drivers/net/virtio/virtqueue.h +++ b/drivers/net/virtio/virtqueue.h @@ -330,7 +330,7 @@ virtqueue_notify(struct virtqueue *vq) * For virtio on IA, the notificaiton is through io port operation * which is a serialization instruction itself. */ - vq->hw->vtpci_ops->notify_queue(vq->hw, vq); + VTPCI_OPS(vq->hw)->notify_queue(vq->hw, vq); } #ifdef RTE_LIBRTE_VIRTIO_DEBUG_DUMP -- 1.9.0