[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Minios-devel] [UNIKRAFT PATCH 4/7] plat/drivers: Introduce virtio PCI device
On 19.10.18 15:06, Sharan Santhanam wrote: We introduces support for virtio PCI legacy device support in this patch. The driver register on the PCI bus to probe for device with QUMRANET vendor id. The virtio PCI bus adds the virtio dev. The virtio bus is responsible for bridging the virtio device with the driver. The virtio PCI device provides configuration callback for the PCI bus operation performed on the virtio device. Maybe you should add another comment that KVM gets this driver enabled. Signed-off-by: Sharan Santhanam <sharan.santhanam@xxxxxxxxx> --- plat/drivers/include/virtio/virtio.h | 93 ++++++++++ plat/drivers/virtio/virtio_bus.c | 100 +++++++++++ plat/drivers/virtio/virtio_pci.c | 318 +++++++++++++++++++++++++++++++++++ plat/kvm/Config.uk | 12 +- plat/kvm/Makefile.uk | 4 +- 5 files changed, 525 insertions(+), 2 deletions(-) create mode 100644 plat/drivers/virtio/virtio_pci.c diff --git a/plat/drivers/include/virtio/virtio.h b/plat/drivers/include/virtio/virtio.h index d471cab..b01faf6 100644 --- a/plat/drivers/include/virtio/virtio.h +++ b/plat/drivers/include/virtio/virtio.h @@ -76,6 +76,27 @@ struct virtio_dev_id { };/**+ * The structure define a list of configuration operation on a virtio device. + */ +struct virtio_config_ops { + /** Resetting the device */ + void (*reset_device)(struct virtio_dev *vdev); + /** Set configuration option */ + int (*set_config)(struct virtio_dev *vdev, __u16 offset, + const void *buf, __u32 len); + /** Get configuration option */ + int (*get_config)(struct virtio_dev *vdev, __u16 offset, void *buf, + __u32 len, __u8 type_len); + /** Get the feature */ + __u64 (*get_features)(struct virtio_dev *vdev); + /** Set the feature */ + void (*set_features)(struct virtio_dev *vdev, __u64 features); + /** Get and Set Status */ + __u8 (*get_status)(struct virtio_dev *vdev); + void (*set_status)(struct virtio_dev *vdev, __u8 status); +}; + +/** * The structure define the virtio driver. */ struct virtio_driver { @@ -99,6 +120,8 @@ struct virtio_dev { void *priv; /* Virtio device identifier */ struct virtio_dev_id id; + /* List of the config operations */ + struct virtio_config_ops *cops; /* Reference to the virtio driver for the device */ struct virtio_driver *vdrv; /* Status of the device */ @@ -108,8 +131,78 @@ struct virtio_dev { /** * Operation exported by the virtio device. */ +int virtio_register_device(struct virtio_dev *vdev); void _virtio_register_driver(struct virtio_driver *vdrv);+/**+ * The function updates the status of the virtio device + * @param vdev + * Reference to the virtio device. + * @param status + * The status to update. + * @return + * 0 on successful updating the status. + */ +static inline int virtio_dev_status_update(struct virtio_dev *vdev, __u8 status) +{ + int rc = -ENOTSUP; + + UK_ASSERT(vdev); + + if (likely(vdev->cops->set_status)) { + vdev->cops->set_status(vdev, status); + rc = 0; + } + return rc; +} + +static inline __u64 virtio_feature_get(struct virtio_dev *vdev) +{ + __u64 features = 0; + + UK_ASSERT(vdev); + + if (likely(vdev->cops->get_features)) + features = vdev->cops->get_features(vdev); + return features; +} + +static inline void virtio_feature_set(struct virtio_dev *vdev, __u32 feature) +{ + UK_ASSERT(vdev); + + if (likely(vdev->cops->set_features)) + vdev->cops->set_features(vdev, feature); +} + +static inline int virtio_config_get(struct virtio_dev *vdev, __u16 offset, + void *buf, __u32 len, __u8 type_len) +{ + int rc = -ENOTSUP; + + UK_ASSERT(vdev); + + if (likely(vdev->cops->get_config)) + rc = vdev->cops->get_config(vdev, offset, buf, len, type_len); + + return rc; +} + +static inline int virtio_has_features(__u64 features, __u8 bpos) +{ + __u64 tmp_feature = 0; + + VIRTIO_FEATURES_UPDATE(tmp_feature, bpos); + tmp_feature &= features; + + return tmp_feature ? 1 : 0; +} + +static inline void virtio_dev_drv_up(struct virtio_dev *vdev) +{ + virtio_dev_status_update(vdev, VIRTIO_CONFIG_STATUS_DRIVER_OK); +} + #define VIRTIO_REGISTER_DRIVER(b) \ _VIRTIO_REGISTER_DRIVER(__LIBNAME__, b)diff --git a/plat/drivers/virtio/virtio_bus.c b/plat/drivers/virtio/virtio_bus.cindex 88cb99a..ef66ae8 100644 --- a/plat/drivers/virtio/virtio_bus.c +++ b/plat/drivers/virtio/virtio_bus.c @@ -52,10 +52,110 @@ static struct uk_alloc *a; /** * Driver module local function(s). */ +static int virtio_device_reinit(struct virtio_dev *vdev); +static struct virtio_driver *find_match_drv(struct virtio_dev *vdev); static int virtio_bus_init(struct uk_alloc *mem_alloc); static int virtio_bus_probe(void);/**+ * Find a match driver + * @param vdev + * Reference to the virtio device. + */ +static struct virtio_driver *find_match_drv(struct virtio_dev *vdev) +{ + int i = 0; + struct virtio_driver *drv = NULL; + + UK_TAILQ_FOREACH(drv, &virtio_drvs, next) { + while (drv->dev_ids[i].virtio_device_id != VIRTIO_ID_INVALID) { + if (drv->dev_ids[i].virtio_device_id == + vdev->id.virtio_device_id) { Hum, in case you want to keep using the virtio device id struct (previous patch), wouldn't memcmp() or any custom id compare function make sense here (even if it is comparing one u16 field for now)? + return drv; + } + } + } + return NULL; +} + +/** + * Reinitialize the virtio device + * @param vdev + * Reference to the virtio device. + */ +static int virtio_device_reinit(struct virtio_dev *vdev) +{ + int rc = 0; + + /** + * Resetting the virtio device + * This may not be necessary while initializing the device for the first + * time. + */ + if (vdev->cops->reset_device) { + vdev->cops->reset_device(vdev); + /* Set the device status */ + vdev->status = VIRTIO_DEV_RESET; + } + /* Acknowledge the virtio device */ + rc = virtio_dev_status_update(vdev, VIRTIO_CONFIG_STATUS_ACK); + if (rc != 0) { + uk_pr_err("Failed to acknowledge the virtio device %p: %d\n", + vdev, rc); + return rc; + } + + /* Acknowledge the virtio driver */ + rc = virtio_dev_status_update(vdev, VIRTIO_CONFIG_STATUS_DRIVER); + if (rc != 0) { + uk_pr_err("Failed to acknowledge the virtio driver %p: %d\n", + vdev, rc); + return rc; + } + vdev->status = VIRTIO_DEV_INITIALIZED; + uk_pr_info("Virtio device %p initialized\n", vdev); + return rc; +} + +int virtio_register_device(struct virtio_dev *vdev) +{ + struct virtio_driver *drv = NULL; + int rc = 0; + + UK_ASSERT(vdev); + /* Check for the dev with the driver list */ + drv = find_match_drv(vdev); + if (!drv) { + uk_pr_err("Failed to find the driver for the virtio device %p (id:%"__PRIu16")\n", + vdev, vdev->id.virtio_device_id); + return -EFAULT; + } + vdev->vdrv = drv; + + /* Initialize the device */ + rc = virtio_device_reinit(vdev); + if (rc != 0) { + uk_pr_err("Failed to initialize the virtio device %p (id:%"__PRIu16": %d\n", + vdev, vdev->id.virtio_device_id, rc); + return rc; + } + + + /* Calling the driver add device */ + rc = drv->add_dev(vdev); + if (rc != 0) { + uk_pr_err("Failed to add the virtio device %p: %d\n", vdev, rc); + goto virtio_dev_fail_set; + } +exit: + return rc; + +virtio_dev_fail_set: + virtio_dev_status_update(vdev, VIRTIO_CONFIG_STATUS_FAIL); Errors from virtio_dev_status_update() are ignored, right? Do you want to add a comment about this? + goto exit; +} + +/** * Probe for the virtio device. */ static int virtio_bus_probe(void) diff --git a/plat/drivers/virtio/virtio_pci.c b/plat/drivers/virtio/virtio_pci.c new file mode 100644 index 0000000..ab2f2c8 --- /dev/null +++ b/plat/drivers/virtio/virtio_pci.c @@ -0,0 +1,318 @@ +/* SPDX-License-Identifier: BSD-3-Clause */ +/* + * Authors: Sharan Santhanam <sharan.santhanam@xxxxxxxxx> + * + * Copyright (c) 2018, NEC Europe Ltd., NEC Corporation. All rights reserved. + * + * Redistribution and use in source and binary forms, with or without + * modification, are permitted provided that the following conditions + * are met: + * + * 1. Redistributions of source code must retain the above copyright + * notice, this list of conditions and the following disclaimer. + * 2. Redistributions in binary form must reproduce the above copyright + * notice, this list of conditions and the following disclaimer in the + * documentation and/or other materials provided with the distribution. + * 3. Neither the name of the copyright holder nor the names of its + * contributors may be used to endorse or promote products derived from + * this software without specific prior written permission. + * + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE + * POSSIBILITY OF SUCH DAMAGE. + * + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY. + */ + +#include <uk/config.h> +#include <uk/arch/types.h> +#include <errno.h> +#ifdef CONFIG_LIBUKALLOC +#include <uk/alloc.h> +#else +#error "Virtio PCI device requires UKALLOC" You can remove this check. Config.uk should make sure that libukalloc is enabled. +#endif /* CONFIG_LIBUKALLOC */ +#include <uk/print.h> +#include <uk/plat/lcpu.h> +#include <uk/plat/irq.h> +#include <pci/pci_bus.h> +#include <virtio/virtio_config.h> +#include <virtio/virtio.h> +#include <virtio/virtio_pci.h> + +#define VENDOR_QUMRANET_VIRTIO (0x1AF4) +#define VIRTIO_PCI_LEGACY_DEVICEID_START (0x0FFF) +#define VIRTIO_PCI_MODERN_DEVICEID_START (0x1040) + +static struct uk_alloc *a; + +/** + * The structure declares a pci device. + */ +struct virtio_pci_dev { + /* Virtio Device */ + struct virtio_dev vdev; + /* Pci base address */ + __u16 pci_base_addr; + /* ISR Address Range */ + __u16 pci_isr_addr; + /* Pci device information */ + struct pci_device *pdev; +}; + +/** + * Fetch the virtio pci information from the virtiodev. + */ +#define to_virtiopcidev(vdev) \ + __containerof(vdev, struct virtio_pci_dev, vdev) + + +/** + * Static function declaration. + */ +static void vpci_legacy_pci_dev_reset(struct virtio_dev *vdev); +static int vpci_legacy_pci_config_set(struct virtio_dev *vdev, __u16 offset, + const void *buf, __u32 len); +static int vpci_legacy_pci_config_get(struct virtio_dev *vdev, __u16 offset, + void *buf, __u32 len, + __u8 type_len __maybe_unused); +static __u64 vpci_legacy_pci_features_get(struct virtio_dev *vdev); +static void vpci_legacy_pci_features_set(struct virtio_dev *vdev, + __u64 features); +static void vpci_legacy_pci_status_set(struct virtio_dev *vdev, __u8 status); +static __u8 vpci_legacy_pci_status_get(struct virtio_dev *vdev); +static inline void virtio_device_id_add(struct virtio_dev *vdev, + __u16 pci_dev_id, __u16 vpci_dev_id_start); +static int virtio_pci_legacy_add_dev(struct pci_device *pci_dev, + struct virtio_pci_dev *vpci_dev); + +/** + * Configuration operations legacy PCI device. + */ +static struct virtio_config_ops vpci_legacy_ops = { + .reset_device = vpci_legacy_pci_dev_reset, + .set_config = vpci_legacy_pci_config_set, + .get_config = vpci_legacy_pci_config_get, + .get_features = vpci_legacy_pci_features_get, + .set_features = vpci_legacy_pci_features_set, + .get_status = vpci_legacy_pci_status_get, + .set_status = vpci_legacy_pci_status_set, +}; + +static int vpci_legacy_pci_config_set(struct virtio_dev *vdev, __u16 offset, + const void *buf, __u32 len) +{ + struct virtio_pci_dev *vpdev = NULL; + + UK_ASSERT(vdev); + vpdev = to_virtiopcidev(vdev); + + _virtio_cwrite_bytes((void *)(unsigned long)vpdev->pci_base_addr, + VIRTIO_PCI_CONFIG_OFF + offset, buf, len, 1); + + return 0; +} + +static int vpci_legacy_pci_config_get(struct virtio_dev *vdev, __u16 offset, + void *buf, __u32 len, + __u8 type_len __maybe_unused) typ_len is used in your first check. You can remove the '__maybe_unused' property. +{ + struct virtio_pci_dev *vpdev = NULL; + int rc = 0; + + UK_ASSERT(vdev); + vpdev = to_virtiopcidev(vdev); + + /* Reading an entity less than 4 bytes are atomic */ + if (type_len == len && type_len <= 4) { + _virtio_cread_bytes( + (void *) (unsigned long)vpdev->pci_base_addr, + VIRTIO_PCI_CONFIG_OFF + offset, buf, len, + type_len); + } else { + rc = virtio_cread_bytes_many( + (void *) (unsigned long)vpdev->pci_base_addr, + VIRTIO_PCI_CONFIG_OFF + offset, buf, len); + } + return rc; +} + +static __u8 vpci_legacy_pci_status_get(struct virtio_dev *vdev) +{ + struct virtio_pci_dev *vpdev = NULL; + + UK_ASSERT(vdev); + vpdev = to_virtiopcidev(vdev); + return virtio_cread8((void *) (unsigned long) vpdev->pci_base_addr, + VIRTIO_PCI_STATUS); +} + +static void vpci_legacy_pci_status_set(struct virtio_dev *vdev, __u8 status) +{ + struct virtio_pci_dev *vpdev = NULL; + __u8 curr_status = 0; + + /* Reset should be performed using the reset interface */ + UK_ASSERT(vdev || status != VIRTIO_CONFIG_STATUS_RESET); + + vpdev = to_virtiopcidev(vdev); + curr_status = vpci_legacy_pci_status_get(vdev); + status |= curr_status; + virtio_cwrite8((void *)(unsigned long) vpdev->pci_base_addr, + VIRTIO_PCI_STATUS, status); +} + +static void vpci_legacy_pci_dev_reset(struct virtio_dev *vdev) +{ + struct virtio_pci_dev *vpdev = NULL; + __u8 status; + + UK_ASSERT(vdev); + + vpdev = to_virtiopcidev(vdev); + /** + * Resetting the device. + */ + virtio_cwrite8((void *) (unsigned long)vpdev->pci_base_addr, + VIRTIO_PCI_STATUS, VIRTIO_CONFIG_STATUS_RESET); + /** + * Waiting for the resetting the device. Find a better way + * of doing this instead of repeating register read. + * + * NOTE! Spec (4.1.4.3.2) + * Need to check if we have to wait for the reset to happen. + */ + do { + status = virtio_cread8( + (void *)(unsigned long)vpdev->pci_base_addr, + VIRTIO_PCI_STATUS); + } while (status != VIRTIO_CONFIG_STATUS_RESET); +} + +static __u64 vpci_legacy_pci_features_get(struct virtio_dev *vdev) +{ + struct virtio_pci_dev *vpdev = NULL; + __u64 features; + + UK_ASSERT(vdev); + + vpdev = to_virtiopcidev(vdev); + features = virtio_cread32((void *) (unsigned long)vpdev->pci_base_addr, + VIRTIO_PCI_HOST_FEATURES); + return features; +} + +static void vpci_legacy_pci_features_set(struct virtio_dev *vdev, + __u64 features) +{ + struct virtio_pci_dev *vpdev = NULL; + + UK_ASSERT(vdev); + vpdev = to_virtiopcidev(vdev); + virtio_cwrite32((void *) (unsigned long)vpdev->pci_base_addr, + VIRTIO_PCI_GUEST_FEATURES, (__u32)features); +} + +static inline void virtio_device_id_add(struct virtio_dev *vdev, + __u16 pci_dev_id, __u16 pci_dev_id_start) +{ + vdev->id.virtio_device_id = (pci_dev_id - pci_dev_id_start); +} + +static int virtio_pci_legacy_add_dev(struct pci_device *pci_dev, + struct virtio_pci_dev *vpci_dev) +{ + /* Check the valid range of the virtio legacy device */ + if (pci_dev->id.device_id < 0x1000 || pci_dev->id.device_id > 0x103f) { + uk_pr_err("Error: Invalid Virtio Devices %d\n", The format should use PRIu16/PRIx16: "Invalid virtio device %02"PRIx16"\n" + pci_dev->id.device_id); + return -EINVAL; + } + + vpci_dev->pci_isr_addr = vpci_dev->pci_base_addr + VIRTIO_PCI_ISR; + + /* Setting the configuration operation */ + vpci_dev->vdev.cops = &vpci_legacy_ops; + + uk_pr_info("Added a virtio-pci device(%02x)\n", The format should use PRIx16: "Added virtio device %02"PRIx16"\n" + (int) pci_dev->id.device_id); + + /* Mapping the virtio device identifier */ + virtio_device_id_add(&vpci_dev->vdev, pci_dev->id.device_id, + VIRTIO_PCI_LEGACY_DEVICEID_START); + return 0; +} + + +static int virtio_pci_add_dev(struct pci_device *pci_dev) +{ + struct virtio_pci_dev *vpci_dev = NULL; + int rc = 0; + + UK_ASSERT(pci_dev != NULL); + + vpci_dev = uk_malloc(a, sizeof(*vpci_dev)); + if (!vpci_dev) { + uk_pr_err("Error in memory allocation of pci_device\n"); > + return -ENOMEM; + } + + /* Fetch PCI Device information */ + vpci_dev->pdev = pci_dev; + vpci_dev->pci_base_addr = pci_dev->base; + + /** + * Probing for the legacy virtio device. We separate the legacy probing + * with the possibility of supporting the modern PCI device in the + * future. + */ + rc = virtio_pci_legacy_add_dev(pci_dev, vpci_dev); + if (rc != 0) { + uk_pr_err("Failed(%d) to probe the legacy device\n", + rc); + goto free_pci_dev; + } + + rc = virtio_register_device(&vpci_dev->vdev); + if (rc != 0) { + uk_pr_err("Failed(%d) to register the virtio device\n", rc); + goto free_pci_dev; + } + +exit: + return rc; +free_pci_dev: + uk_free(a, vpci_dev); + goto exit; +} + +static int virtio_pci_drv_init(struct uk_alloc *drv_allocator) +{ + /* driver initialization */ + if (!drv_allocator) + return -EINVAL; + + a = drv_allocator; + return 0; +} + +static const struct pci_device_id virtio_pci_ids[] = { + {PCI_DEVICE_ID(VENDOR_QUMRANET_VIRTIO, PCI_ANY_ID)}, + /* End of Driver List */ + {PCI_ANY_DEVICE_ID}, +}; + +static struct pci_driver virtio_pci_drv = { + .device_ids = virtio_pci_ids, + .init = virtio_pci_drv_init, + .add_dev = virtio_pci_add_dev +}; +PCI_REGISTER_DRIVER(&virtio_pci_drv); diff --git a/plat/kvm/Config.uk b/plat/kvm/Config.uk index f8a8aaf..afcedc0 100644 --- a/plat/kvm/Config.uk +++ b/plat/kvm/Config.uk @@ -56,9 +56,19 @@ config VIRTIO_BUS default y depends on (ARCH_X86_64) depends on LIBUKBUS - select LIBUKALLOC + select LIBUKALLOC You are correcting white spacing from a previous commit? ;-) help Virtio bus driver for probing and operating virtio device and transport layer.+menu "Virtio"+config VIRTIO_PCI + bool "Virtio PCI device support" + default n + depends on KVM_PCI + select VIRTIO_BUS + help + Support virtio devices on PCI bus + +endmenu endif diff --git a/plat/kvm/Makefile.uk b/plat/kvm/Makefile.uk index 80a0c05..986bf4a 100644 --- a/plat/kvm/Makefile.uk +++ b/plat/kvm/Makefile.uk @@ -91,4 +91,6 @@ LIBKVMVIRTIO_CINCLUDES-y += -I$(UK_PLAT_COMMON_BASE)/include LIBKVMVIRTIO_ASINCLUDES-y += -I$(UK_PLAT_DRIVERS_BASE)/include LIBKVMVIRTIO_CINCLUDES-y += -I$(UK_PLAT_DRIVERS_BASE)/include LIBKVMVIRTIO_SRCS-$(CONFIG_VIRTIO_BUS) +=\ - $(UK_PLAT_DRIVERS_BASE)/virtio/virtio_bus.c + $(UK_PLAT_DRIVERS_BASE)/virtio/virtio_bus.c +LIBKVMVIRTIO_SRCS-$(CONFIG_VIRTIO_PCI) +=\ + $(UK_PLAT_DRIVERS_BASE)/virtio/virtio_pci.c _______________________________________________ Minios-devel mailing list Minios-devel@xxxxxxxxxxxxxxxxxxxx https://lists.xenproject.org/mailman/listinfo/minios-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |