From: Yuanhan Liu Date: Fri, 6 Jan 2017 10:16:17 +0000 (+0800) Subject: net/virtio: store PCI operators pointer locally X-Git-Tag: spdx-start~5168 X-Git-Url: http://git.droids-corp.org/?p=dpdk.git;a=commitdiff_plain;h=553f45932fb797e9fbd6342016e0dd53e1f61fc7 net/virtio: store PCI operators pointer locally 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 --- diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c index c3c3826326..0b0967defb 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); @@ -1336,6 +1338,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 9b47165dfa..b1f2e181b1 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 5373e390fc..3d8f79710f 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 8cb983c3ec..95636e3a85 100644 --- a/drivers/net/virtio/virtio_user_ethdev.c +++ b/drivers/net/virtio/virtio_user_ethdev.c @@ -301,7 +301,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 f0bb0899c7..b1070e0599 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