diff --git a/lib/bdev/virtio/rte_virtio/virtio_dev.c b/lib/bdev/virtio/rte_virtio/virtio_dev.c index 0ee1732ce..0194b844a 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_dev.c +++ b/lib/bdev/virtio/rte_virtio/virtio_dev.c @@ -53,6 +53,7 @@ #include #include +#include "virtio_user/vhost.h" #include "virtio_dev.h" #include "virtio_pci.h" #include "virtio_logs.h" @@ -242,37 +243,20 @@ virtio_alloc_queues(struct virtio_dev *dev) static int virtio_negotiate_features(struct virtio_dev *dev, uint64_t req_features) { - uint64_t host_features; + uint64_t host_features = vtpci_ops(dev)->get_features(dev); + int rc; - /* Prepare guest_features: feature that driver wants to support */ - PMD_INIT_LOG(DEBUG, "guest_features before negotiate = %" PRIx64, - req_features); + PMD_INIT_LOG(DEBUG, "guest features = %" PRIx64, req_features); + PMD_INIT_LOG(DEBUG, "device features = %" PRIx64, host_features); - /* Read device(host) feature bits */ - host_features = vtpci_ops(dev)->get_features(dev); - PMD_INIT_LOG(DEBUG, "host_features before negotiate = %" PRIx64, - host_features); - - /* - * Negotiate features: Subset of device feature bits are written back - * guest feature bits. - */ - dev->req_guest_features = req_features; - dev->guest_features = vtpci_negotiate_features(dev, host_features); - PMD_INIT_LOG(DEBUG, "features after negotiate = %" PRIx64, - dev->guest_features); - - if (!vtpci_with_feature(dev, VIRTIO_F_VERSION_1)) { - if (dev->modern) { - PMD_INIT_LOG(ERR, - "VIRTIO_F_VERSION_1 features is not enabled."); - return -1; - } - - return 0; + rc = vtpci_ops(dev)->set_features(dev, req_features & host_features); + if (rc != 0) { + PMD_INIT_LOG(ERR, "failed to negotiate device features."); + return -1; } - dev->modern = 1; + PMD_INIT_LOG(DEBUG, "negotiated features = %" PRIx64, dev->negotiated_features); + vtpci_set_status(dev, VIRTIO_CONFIG_STATUS_FEATURES_OK); if (!(vtpci_get_status(dev) & VIRTIO_CONFIG_STATUS_FEATURES_OK)) { PMD_INIT_LOG(ERR, diff --git a/lib/bdev/virtio/rte_virtio/virtio_dev.h b/lib/bdev/virtio/rte_virtio/virtio_dev.h index bff71dc8a..29793537c 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_dev.h +++ b/lib/bdev/virtio/rte_virtio/virtio_dev.h @@ -47,8 +47,10 @@ struct virtio_dev { /* Device index. */ uint32_t id; - uint64_t req_guest_features; - uint64_t guest_features; + + /* Common device & guest features. */ + uint64_t negotiated_features; + int is_hw; /** Modern/legacy virtio device flag. */ diff --git a/lib/bdev/virtio/rte_virtio/virtio_pci.c b/lib/bdev/virtio/rte_virtio/virtio_pci.c index f54510ae2..a3b1a21b1 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_pci.c +++ b/lib/bdev/virtio/rte_virtio/virtio_pci.c @@ -200,16 +200,19 @@ legacy_get_features(struct virtio_dev *dev) return dst; } -static void +static int legacy_set_features(struct virtio_dev *dev, uint64_t features) { if ((features >> 32) != 0) { PMD_DRV_LOG(ERR, "only 32 bit features are allowed for legacy virtio!"); - return; + return -1; } rte_pci_ioport_write(vtpci_io(dev), &features, 4, VIRTIO_PCI_GUEST_FEATURES); + dev->negotiated_features = features; + + return 0; } static uint8_t @@ -373,11 +376,17 @@ modern_get_features(struct virtio_dev *dev) return ((uint64_t)features_hi << 32) | features_lo; } -static void +static int modern_set_features(struct virtio_dev *dev, uint64_t features) { struct virtio_hw *hw = virtio_dev_get_hw(dev); + if ((features & (1ULL << VIRTIO_F_VERSION_1)) == 0) { + PMD_INIT_LOG(ERR, + "VIRTIO_F_VERSION_1 feature is not enabled."); + return -1; + } + rte_write32(0, &hw->common_cfg->guest_feature_select); rte_write32(features & ((1ULL << 32) - 1), &hw->common_cfg->guest_feature); @@ -385,6 +394,10 @@ modern_set_features(struct virtio_dev *dev, uint64_t features) rte_write32(1, &hw->common_cfg->guest_feature_select); rte_write32(features >> 32, &hw->common_cfg->guest_feature); + + dev->negotiated_features = features; + + return 0; } static uint8_t @@ -535,21 +548,6 @@ vtpci_write_dev_config(struct virtio_dev *dev, size_t offset, vtpci_ops(dev)->write_dev_cfg(dev, offset, src, length); } -uint64_t -vtpci_negotiate_features(struct virtio_dev *dev, uint64_t host_features) -{ - uint64_t features; - - /* - * Limit negotiated features to what the driver, virtqueue, and - * host all support. - */ - features = host_features & dev->req_guest_features; - vtpci_ops(dev)->set_features(dev, features); - - return features; -} - void vtpci_reset(struct virtio_dev *dev) { diff --git a/lib/bdev/virtio/rte_virtio/virtio_pci.h b/lib/bdev/virtio/rte_virtio/virtio_pci.h index e59c1fd52..979ff5237 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_pci.h +++ b/lib/bdev/virtio/rte_virtio/virtio_pci.h @@ -196,8 +196,18 @@ struct virtio_pci_ops { uint8_t (*get_status)(struct virtio_dev *hw); void (*set_status)(struct virtio_dev *hw, uint8_t status); - uint64_t (*get_features)(struct virtio_dev *hw); - void (*set_features)(struct virtio_dev *hw, uint64_t features); + /** + * Get device features. The features might be already + * negotiated with driver (guest) features. + */ + uint64_t (*get_features)(struct virtio_dev *vdev); + + /** + * Negotiate and set device features. + * The negotiation can fail with return code -1. + * This function should also set vdev->negotiated_features field. + */ + int (*set_features)(struct virtio_dev *vdev, uint64_t features); uint8_t (*get_isr)(struct virtio_dev *hw); @@ -264,7 +274,7 @@ extern struct virtio_driver g_virtio_driver; static inline int vtpci_with_feature(struct virtio_dev *dev, uint64_t bit) { - return (dev->guest_features & (1ULL << bit)) != 0; + return (dev->negotiated_features & (1ULL << bit)) != 0; } /** diff --git a/lib/bdev/virtio/rte_virtio/virtio_user.c b/lib/bdev/virtio/rte_virtio/virtio_user.c index a50a0cb53..ffda44ea5 100644 --- a/lib/bdev/virtio/rte_virtio/virtio_user.c +++ b/lib/bdev/virtio/rte_virtio/virtio_user.c @@ -94,17 +94,31 @@ static uint64_t virtio_user_get_features(struct virtio_dev *vdev) { struct virtio_user_dev *dev = virtio_dev_get_user_dev(vdev); + uint64_t features; - /* unmask feature bits defined in vhost user protocol */ - return dev->device_features; + if (dev->ops->send_request(dev, VHOST_USER_GET_FEATURES, &features) < 0) { + PMD_INIT_LOG(ERR, "get_features failed: %s", strerror(errno)); + return 0; + } + + return features; } -static void +static int virtio_user_set_features(struct virtio_dev *vdev, uint64_t features) { struct virtio_user_dev *dev = virtio_dev_get_user_dev(vdev); + int ret; - dev->features = features & dev->device_features; + ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES, &features); + if (ret < 0) { + return -1; + } + + vdev->negotiated_features = features; + vdev->modern = vtpci_with_feature(vdev, VIRTIO_F_VERSION_1); + + return 0; } static uint8_t 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 a3b376ce9..8a96112e0 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 @@ -118,36 +118,22 @@ virtio_user_queue_setup(struct virtio_user_dev *dev, int virtio_user_start_device(struct virtio_user_dev *dev) { - uint64_t features; int ret; - /* Step 0: tell vhost to create queues */ + /* tell vhost to create queues */ if (virtio_user_queue_setup(dev, virtio_user_create_queue) < 0) - goto error; + return -1; - /* Step 1: set features */ - features = dev->features; - - printf("features = %jx\n", features); - - ret = dev->ops->send_request(dev, VHOST_USER_SET_FEATURES, &features); - if (ret < 0) - goto error; - PMD_DRV_LOG(INFO, "set features: %" PRIx64, features); - - /* Step 2: share memory regions */ + /* share memory regions */ ret = dev->ops->send_request(dev, VHOST_USER_SET_MEM_TABLE, NULL); if (ret < 0) - goto error; + return -1; - /* Step 3: kick queues */ + /* kick queues */ if (virtio_user_queue_setup(dev, virtio_user_kick_queue) < 0) - goto error; + return -1; return 0; -error: - /* TODO: free resource here or caller to check */ - return -1; } int virtio_user_stop_device(struct virtio_user_dev *dev) @@ -229,12 +215,6 @@ virtio_user_dev_init(char *path, int queue_size) goto err; } - if (dev->ops->send_request(dev, VHOST_USER_GET_FEATURES, - &dev->device_features) < 0) { - PMD_INIT_LOG(ERR, "get_features failed: %s", strerror(errno)); - goto err; - } - TAILQ_INSERT_TAIL(&g_virtio_driver.init_ctrlrs, vdev, tailq); return vdev; 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 0220a2f53..1d03dee63 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 @@ -58,10 +58,7 @@ struct virtio_user_dev { int callfds[VIRTIO_MAX_VIRTQUEUES]; int kickfds[VIRTIO_MAX_VIRTQUEUES]; uint32_t queue_size; - uint64_t features; /* the negotiated features with driver, - * and will be sync with device - */ - uint64_t device_features; /* supported features by device */ + uint8_t status; uint8_t port_id; char path[PATH_MAX];