From 79f99ccce0599eb3661f8b28772f45187c29289b Mon Sep 17 00:00:00 2001 From: Dariusz Stojaczyk Date: Wed, 15 Nov 2017 13:25:38 +0100 Subject: [PATCH] rte_virtio: add virtio_dev->ctx field This patch initiates the removal of the vtpci layer from rte_virtio. The general idea is to provide separate virtio_dev_construct and virtio_dev_init functions. construct would just allocate and setup the struct data. init would be the one to start the device via MMIO/vhost-user. virtio_dev_construct takes 2 params - backend ops and backend context. The context is kept as void* and can be cast to proper structs inside backend ops callbacks. Change-Id: I1d6d92e1cf09b9d0c9fa4fd2cb70203e3fc7e65b Signed-off-by: Dariusz Stojaczyk Reviewed-on: https://review.gerrithub.io/387553 Tested-by: SPDK Automated Test System Reviewed-by: Daniel Verkamp Reviewed-by: Jim Harris --- lib/bdev/virtio/rte_virtio/virtio_dev.c | 2 + lib/bdev/virtio/rte_virtio/virtio_dev.h | 14 +++++ lib/bdev/virtio/rte_virtio/virtio_pci.c | 54 ++++++++++++------- lib/bdev/virtio/rte_virtio/virtio_pci.h | 7 --- lib/bdev/virtio/rte_virtio/virtio_user.c | 4 +- .../rte_virtio/virtio_user/virtio_user_dev.c | 18 +++---- .../rte_virtio/virtio_user/virtio_user_dev.h | 2 - 7 files changed, 62 insertions(+), 39 deletions(-) diff --git a/lib/bdev/virtio/rte_virtio/virtio_dev.c b/lib/bdev/virtio/rte_virtio/virtio_dev.c index fe1efe5a1..2b33bfb34 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_dev.c +++ b/lib/bdev/virtio/rte_virtio/virtio_dev.c @@ -292,6 +292,8 @@ virtio_dev_free(struct virtio_dev *dev) virtio_free_queues(dev); vtpci_ops(dev)->free_vdev(dev); vtpci_deinit(vdev_id); + pthread_mutex_destroy(&dev->mutex); + free(dev); } int diff --git a/lib/bdev/virtio/rte_virtio/virtio_dev.h b/lib/bdev/virtio/rte_virtio/virtio_dev.h index 9e7b0ad32..566393250 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_dev.h +++ b/lib/bdev/virtio/rte_virtio/virtio_dev.h @@ -72,6 +72,8 @@ */ #define SPDK_VIRTIO_QUEUE_LCORE_ID_UNUSED (UINT32_MAX - 1) +struct virtio_pci_ops; + struct virtio_dev { struct virtqueue **vqs; @@ -96,6 +98,9 @@ struct virtio_dev { /** Mutex for asynchronous virtqueue-changing operations. */ pthread_mutex_t mutex; + /** Context for the backend ops */ + void *ctx; + TAILQ_ENTRY(virtio_dev) tailq; }; @@ -181,6 +186,15 @@ uint16_t virtio_recv_pkts(struct virtqueue *vq, struct virtio_req **reqs, */ int virtio_xmit_pkt(struct virtqueue *vq, struct virtio_req *req); +/** + * Construct virtio device. This will set vdev->id field. + * The device has to be freed with \c virtio_dev_free. + * + * \param ops backend callbacks + * \param ctx argument for the backend callbacks + */ +struct virtio_dev *virtio_dev_construct(const struct virtio_pci_ops *ops, void *ctx); + int virtio_dev_init(struct virtio_dev *hw, uint64_t req_features); void virtio_dev_free(struct virtio_dev *dev); int virtio_dev_start(struct virtio_dev *hw); diff --git a/lib/bdev/virtio/rte_virtio/virtio_pci.c b/lib/bdev/virtio/rte_virtio/virtio_pci.c index 35221d0a1..7c381916f 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_pci.c +++ b/lib/bdev/virtio/rte_virtio/virtio_pci.c @@ -54,8 +54,8 @@ struct virtio_driver g_virtio_driver = { #define PCI_CAP_ID_VNDR 0x09 #define PCI_CAP_ID_MSIX 0x11 -#define virtio_dev_get_hw(hw) \ - ((struct virtio_hw *)((uintptr_t)(hw) - offsetof(struct virtio_hw, vdev))) +#define virtio_dev_get_hw(vdev) \ + ((struct virtio_hw *)((vdev)->ctx)) static inline int check_vq_phys_addr_ok(struct virtqueue *vq) @@ -74,9 +74,8 @@ check_vq_phys_addr_ok(struct virtqueue *vq) } static void -free_virtio_hw(struct virtio_dev *dev) +free_virtio_hw(struct virtio_hw *hw) { - struct virtio_hw *hw = virtio_dev_get_hw(dev); unsigned i; for (i = 0; i < 6; ++i) { @@ -87,7 +86,6 @@ free_virtio_hw(struct virtio_dev *dev) spdk_pci_device_unmap_bar(hw->pci_dev, i, hw->pci_bar[i].vaddr); } - free(dev->name); free(hw); } @@ -185,6 +183,15 @@ modern_set_features(struct virtio_dev *dev, uint64_t features) return 0; } +static void +modern_free_vdev(struct virtio_dev *vdev) +{ + struct virtio_hw *hw = virtio_dev_get_hw(vdev); + + free_virtio_hw(hw); + free(vdev->name); +} + static uint8_t modern_get_status(struct virtio_dev *dev) { @@ -281,7 +288,7 @@ const struct virtio_pci_ops modern_ops = { .set_status = modern_set_status, .get_features = modern_get_features, .set_features = modern_set_features, - .free_vdev = free_virtio_hw, + .free_vdev = modern_free_vdev, .get_queue_num = modern_get_queue_num, .setup_queue = modern_setup_queue, .del_queue = modern_del_queue, @@ -462,8 +469,6 @@ pci_enum_virtio_probe_cb(void *ctx, struct spdk_pci_device *pci_dev) return -1; } - vdev = &hw->vdev; - vdev->is_hw = 1; hw->pci_dev = pci_dev; for (i = 0; i < 6; ++i) { @@ -471,7 +476,8 @@ pci_enum_virtio_probe_cb(void *ctx, struct spdk_pci_device *pci_dev) &bar_len); if (rc != 0) { SPDK_ERRLOG("failed to memmap PCI BAR %u\n", i); - goto err; + free_virtio_hw(hw); + return -1; } hw->pci_bar[i].vaddr = bar_vaddr; @@ -483,31 +489,34 @@ pci_enum_virtio_probe_cb(void *ctx, struct spdk_pci_device *pci_dev) */ if (virtio_read_caps(hw) != 0) { SPDK_NOTICELOG("Ignoring legacy PCI device.\n"); - goto err; + free_virtio_hw(hw); + return -1; } - rc = vtpci_init(vdev, &modern_ops); - if (rc != 0) { - goto err; + vdev = virtio_dev_construct(&modern_ops, hw); + if (vdev == NULL) { + free_virtio_hw(hw); + return -1; } + vdev->is_hw = 1; vdev->modern = 1; rc = virtio_dev_pci_init(vdev); if (rc != 0) { - vtpci_deinit(vdev->id); goto err; } return 0; err: - free_virtio_hw(vdev); + virtio_dev_free(vdev); return -1; } -int -vtpci_init(struct virtio_dev *vdev, const struct virtio_pci_ops *ops) +struct virtio_dev * + virtio_dev_construct(const struct virtio_pci_ops *ops, void *ctx) { + struct virtio_dev *vdev; unsigned vdev_num; for (vdev_num = 0; vdev_num < VIRTIO_MAX_DEVICES; vdev_num++) { @@ -518,14 +527,21 @@ vtpci_init(struct virtio_dev *vdev, const struct virtio_pci_ops *ops) if (vdev_num == VIRTIO_MAX_DEVICES) { SPDK_ERRLOG("Max vhost device limit reached (%u).\n", VIRTIO_MAX_DEVICES); - return -ENOSPC; + return NULL; + } + + vdev = calloc(1, sizeof(*vdev)); + if (vdev == NULL) { + SPDK_ERRLOG("virtio device calloc failed\n"); + return NULL; } vdev->id = vdev_num; pthread_mutex_init(&vdev->mutex, NULL); + vdev->ctx = ctx; g_virtio_driver.internal[vdev_num].vtpci_ops = ops; - return 0; + return vdev; } int diff --git a/lib/bdev/virtio/rte_virtio/virtio_pci.h b/lib/bdev/virtio/rte_virtio/virtio_pci.h index 5e00b38a7..e83490983 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_pci.h +++ b/lib/bdev/virtio/rte_virtio/virtio_pci.h @@ -86,7 +86,6 @@ struct virtio_pci_ops { }; struct virtio_hw { - struct virtio_dev vdev; uint8_t use_msix; uint32_t notify_off_multiplier; uint8_t *isr; @@ -133,12 +132,6 @@ vtpci_with_feature(struct virtio_dev *dev, uint64_t bit) */ int vtpci_enumerate_pci(void); -/** - * Init virtual PCI layer for given device. This - * will set vdev->id field. - */ -int vtpci_init(struct virtio_dev *vdev, const struct virtio_pci_ops *ops); - void vtpci_reset(struct virtio_dev *); uint8_t vtpci_get_status(struct virtio_dev *); diff --git a/lib/bdev/virtio/rte_virtio/virtio_user.c b/lib/bdev/virtio/rte_virtio/virtio_user.c index ea810d8be..caa0841f0 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_user.c +++ b/lib/bdev/virtio/rte_virtio/virtio_user.c @@ -47,8 +47,8 @@ #include "spdk/string.h" -#define virtio_dev_get_user_dev(dev) \ - ((struct virtio_user_dev *)((uintptr_t)(dev) - offsetof(struct virtio_user_dev, vdev))) +#define virtio_dev_get_user_dev(vdev) \ + ((struct virtio_user_dev *)((vdev)->ctx)) static void virtio_user_read_dev_config(struct virtio_dev *vdev, size_t offset, diff --git a/lib/bdev/virtio/rte_virtio/virtio_user/virtio_user_dev.c b/lib/bdev/virtio/rte_virtio/virtio_user/virtio_user_dev.c index 42f7be3d0..cb9c9f150 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_user/virtio_user_dev.c +++ b/lib/bdev/virtio/rte_virtio/virtio_user/virtio_user_dev.c @@ -41,7 +41,7 @@ #include "spdk/util.h" #define virtio_dev_get_user_dev(dev) \ - ((struct virtio_user_dev *)((uintptr_t)(dev) - offsetof(struct virtio_user_dev, vdev))) + ((struct virtio_user_dev *)((dev)->ctx)) static int virtio_user_create_queue(struct virtio_dev *vdev, uint32_t queue_sel) @@ -198,7 +198,13 @@ virtio_user_dev_init(const char *name, const char *path, uint16_t requested_queu return NULL; } - vdev = &dev->vdev; + vdev = virtio_dev_construct(&virtio_user_ops, dev); + if (vdev == NULL) { + SPDK_ERRLOG("Failed to init device: %s\n", path); + free(dev); + return NULL; + } + vdev->is_hw = 0; vdev->name = strdup(name); if (!vdev->name) { @@ -206,11 +212,6 @@ virtio_user_dev_init(const char *name, const char *path, uint16_t requested_queu goto err; } - if (vtpci_init(vdev, &virtio_user_ops) != 0) { - SPDK_ERRLOG("Failed to init device: %s\n", path); - goto err; - } - snprintf(dev->path, PATH_MAX, "%s", path); dev->queue_size = queue_size; @@ -243,7 +244,6 @@ virtio_user_dev_init(const char *name, const char *path, uint16_t requested_queu return vdev; err: - free(vdev->name); - free(dev); + virtio_dev_free(vdev); return NULL; } diff --git a/lib/bdev/virtio/rte_virtio/virtio_user/virtio_user_dev.h b/lib/bdev/virtio/rte_virtio/virtio_user/virtio_user_dev.h index 32cd7a666..1d9692120 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_user/virtio_user_dev.h +++ b/lib/bdev/virtio/rte_virtio/virtio_user/virtio_user_dev.h @@ -45,8 +45,6 @@ #define VIRTIO_MAX_VIRTQUEUES 0x100 struct virtio_user_dev { - struct virtio_dev vdev; - /* for vhost_user backend */ int vhostfd;