Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Kirti Wankhede]

Resending this mail again, somehow my previous mail didn't reached every 
to everyone's inbox.

On 2/2/2016 3:16 AM, Kirti Wankhede wrote:
> Design for vGPU Driver:
> Main purpose of vGPU driver is to provide a common interface for vGPU
> management that can be used by differnt GPU drivers.
>
> This module would provide a generic interface to create the device, add
> it to vGPU bus, add device to IOMMU group and then add it to vfio group.
>
> High Level block diagram:
>
>
> +--------------+    vgpu_register_driver()+---------------+
> |     __init() +------------------------->+               |
> |              |                          |               |
> |              +<-------------------------+    vgpu.ko    |
> | vfio_vgpu.ko |   probe()/remove()       |               |
> |              |                +---------+               +---------+
> +--------------+                |         +-------+-------+         |
>                                  |                 ^                 |
>                                  | callback        |                 |
>                                  |         +-------+--------+        |
>                                  |         |vgpu_register_device()   |
>                                  |         |                |        |
>                                  +---^-----+-----+    +-----+------+-+
>                                      | nvidia.ko |    |  i915.ko   |
>                                      |           |    |            |
>                                      +-----------+    +------------+
>
> vGPU driver provides two types of registration interfaces:
> 1. Registration interface for vGPU bus driver:
>
> /**
>   * struct vgpu_driver - vGPU device driver
>   * @name: driver name
>   * @probe: called when new device created
>   * @remove: called when device removed
>   * @driver: device driver structure
>   *
>   **/
> struct vgpu_driver {
>          const char *name;
>          int  (*probe)  (struct device *dev);
>          void (*remove) (struct device *dev);
>          struct device_driver    driver;
> };
>
> int  vgpu_register_driver(struct vgpu_driver *drv, struct module *owner);
> void vgpu_unregister_driver(struct vgpu_driver *drv);
>
> VFIO bus driver for vgpu, should use this interface to register with
> vGPU driver. With this, VFIO bus driver for vGPU devices is responsible
> to add vGPU device to VFIO group.
>
> 2. GPU driver interface
>
> /**
>   * struct gpu_device_ops - Structure to be registered for each physical
> GPU to
>   * register the device to vgpu module.
>   *
>   * @owner:              The module owner.
>   * @vgpu_supported_config: Called to get information about supported
>   *                       vgpu types.
>   *                      @dev : pci device structure of physical GPU.
>   *                      @config: should return string listing supported
>   *                      config
>   *                      Returns integer: success (0) or error (< 0)
>   * @vgpu_create:        Called to allocate basic resouces in graphics
>   *                      driver for a particular vgpu.
>   *                      @dev: physical pci device structure on which vgpu
>   *                            should be created
>   *                      @uuid: uuid for which VM it is intended to
>   *                      @instance: vgpu instance in that VM
>   *                      @vgpu_id: This represents the type of vgpu to be
>   *                                created
>   *                      Returns integer: success (0) or error (< 0)
>   * @vgpu_destroy:       Called to free resources in graphics driver for
>   *                      a vgpu instance of that VM.
>   *                      @dev: physical pci device structure to which
>   *                      this vgpu points to.
>   *                      @uuid: uuid for which the vgpu belongs to.
>   *                      @instance: vgpu instance in that VM
>   *                      Returns integer: success (0) or error (< 0)
>   *                      If VM is running and vgpu_destroy is called that
>   *                      means the vGPU is being hotunpluged. Return error
>   *                      if VM is running and graphics driver doesn't
>   *                      support vgpu hotplug.
>   * @vgpu_start:         Called to do initiate vGPU initialization
>   *                      process in graphics driver when VM boots before
>   *                      qemu starts.
>   *                      @uuid: UUID which is booting.
>   *                      Returns integer: success (0) or error (< 0)
>   * @vgpu_shutdown:      Called to teardown vGPU related resources for
>   *                      the VM
>   *                      @uuid: UUID which is shutting down .
>   *                      Returns integer: success (0) or error (< 0)
>   * @read:               Read emulation callback
>   *                      @vdev: vgpu device structure
>   *                      @buf: read buffer
>   *                      @count: number bytes to read
>   *                      @address_space: specifies for which address space
>   *                      the request is: pci_config_space, IO register
>   *                      space or MMIO space.
>   *                      Retuns number on bytes read on success or error.
>   * @write:              Write emulation callback
>   *                      @vdev: vgpu device structure
>   *                      @buf: write buffer
>   *                      @count: number bytes to be written
>   *                      @address_space: specifies for which address space
>   *                      the request is: pci_config_space, IO register
>   *                      space or MMIO space.
>   *                      Retuns number on bytes written on success or
> error.
>   * @vgpu_set_irqs:      Called to send about interrupts configuration
>   *                      information that qemu set.
>   *                      @vdev: vgpu device structure
>   *                      @flags, index, start, count and *data : same as
>   *                      that of struct vfio_irq_set of
>   *                      VFIO_DEVICE_SET_IRQS API.
>   *
>   * Physical GPU that support vGPU should be register with vgpu module with
>   * gpu_device_ops structure.
>   */
>
> struct gpu_device_ops {
>          struct module   *owner;
>          int     (*vgpu_supported_config)(struct pci_dev *dev,
>                       char  *config);
>          int     (*vgpu_create)(struct pci_dev *dev, uuid_le uuid,
>                                 uint32_t instance, uint32_t vgpu_id);
>          int     (*vgpu_destroy)(struct pci_dev *dev, uuid_le uuid,
>                                  uint32_t instance);
>          int     (*vgpu_start)(uuid_le uuid);
>          int     (*vgpu_shutdown)(uuid_le uuid);
>          ssize_t (*read) (struct vgpu_device *vdev, char *buf,
>               size_t count, uint32_t address_space,
>               loff_t pos);
>          ssize_t (*write)(struct vgpu_device *vdev, char *buf,
>               size_t count, uint32_t address_space,
>               loff_t pos);
>          int     (*vgpu_set_irqs)(struct vgpu_device *vdev,
>                   uint32_t flags, unsigned index,
>                    unsigned start, unsigned count,
>                                   void *data);
> };
>
> int  vgpu_register_device(struct pci_dev *dev, const struct
> gpu_device_ops *ops);
> void vgpu_unregister_device(struct pci_dev *dev);
>
> This registration interface should be used by GPU drivers to register
> each physical device to vGPU driver.
>
> This patch set is to review major APIs and design structure before
> continuing development and don't consider as final patch for review.
> These registration interfaces would evolve as the development proceed.
>
> Regards,
> Kirti
>
> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> Signed-off-by: Neo Jia <cjia@nvidia.com>
> ---
> ---
>   drivers/vgpu/vgpu-driver.c |  137 ++++++++++++++
>   drivers/vgpu/vgpu_dev.c    |  421
> ++++++++++++++++++++++++++++++++++++++++++++
>   include/linux/vgpu.h       |  189 ++++++++++++++++++++
>   3 files changed, 747 insertions(+), 0 deletions(-)
>   create mode 100644 drivers/vgpu/vgpu-driver.c
>   create mode 100644 drivers/vgpu/vgpu_dev.c
>   create mode 100644 include/linux/vgpu.h
>
> diff --git a/drivers/vgpu/vgpu-driver.c b/drivers/vgpu/vgpu-driver.c
> new file mode 100644
> index 0000000..6b62f19
> --- /dev/null
> +++ b/drivers/vgpu/vgpu-driver.c
> @@ -0,0 +1,137 @@
> +/*
> + * VGPU driver
> + *
> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
> + *     Author: Neo Jia <cjia@nvidia.com>
> + *           Kirti Wankhede <kwankhede@nvidia.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/fs.h>
> +#include <linux/vfio.h>
> +#include <linux/iommu.h>
> +#include <linux/sysfs.h>
> +#include <linux/ctype.h>
> +#include <linux/vgpu.h>
> +
> +#include "vgpu_private.h"
> +
> +static int vgpu_device_attach_iommu(struct vgpu_device *vgpu_dev)
> +{
> +        int retval = 0;
> +        struct iommu_group *group = NULL;
> +
> +        group = iommu_group_alloc();
> +        if (IS_ERR(group)) {
> +                printk(KERN_ERR "VGPU: failed to allocate group!\n");
> +                return PTR_ERR(group);
> +        }
> +
> +        retval = iommu_group_add_device(group, &vgpu_dev->dev);
> +        if (retval) {
> +                printk(KERN_ERR "VGPU: failed to add dev to group!\n");
> +                iommu_group_put(group);
> +                return retval;
> +        }
> +
> +        vgpu_dev->group = group;
> +
> +        printk(KERN_INFO "VGPU: group_id = %d \n", iommu_group_id(group));
> +        return retval;
> +}
> +
> +static void vgpu_device_detach_iommu(struct vgpu_device *vgpu_dev)
> +{
> +        iommu_group_put(vgpu_dev->dev.iommu_group);
> +        iommu_group_remove_device(&vgpu_dev->dev);
> +        printk(KERN_INFO "VGPU: detaching iommu \n");
> +}
> +
> +static int vgpu_device_probe(struct device *dev)
> +{
> +    struct vgpu_driver *drv = to_vgpu_driver(dev->driver);
> +    struct vgpu_device *vgpu_dev = to_vgpu_device(dev);
> +    int status = 0;
> +
> +    status = vgpu_device_attach_iommu(vgpu_dev);
> +    if (status) {
> +        printk(KERN_ERR "Failed to attach IOMMU\n");
> +        return status;
> +    }
> +
> +    if (drv && drv->probe) {
> +        status = drv->probe(dev);
> +    }
> +
> +    return status;
> +}
> +
> +static int vgpu_device_remove(struct device *dev)
> +{
> +    struct vgpu_driver *drv = to_vgpu_driver(dev->driver);
> +    struct vgpu_device *vgpu_dev = to_vgpu_device(dev);
> +    int status = 0;
> +
> +    if (drv && drv->remove) {
> +        drv->remove(dev);
> +    }
> +
> +    vgpu_device_detach_iommu(vgpu_dev);
> +
> +    return status;
> +}
> +
> +struct bus_type vgpu_bus_type = {
> +    .name        = "vgpu",
> +    .probe        = vgpu_device_probe,
> +    .remove        = vgpu_device_remove,
> +};
> +EXPORT_SYMBOL_GPL(vgpu_bus_type);
> +
> +/**
> + * vgpu_register_driver - register a new vGPU driver
> + * @drv: the driver to register
> + * @owner: owner module of driver ro register
> + *
> + * Returns a negative value on error, otherwise 0.
> + */
> +int vgpu_register_driver(struct vgpu_driver *drv, struct module *owner)
> +{
> +    /* initialize common driver fields */
> +    drv->driver.name = drv->name;
> +    drv->driver.bus = &vgpu_bus_type;
> +    drv->driver.owner = owner;
> +
> +    /* register with core */
> +    return driver_register(&drv->driver);
> +}
> +EXPORT_SYMBOL(vgpu_register_driver);
> +
> +/**
> + * vgpu_unregister_driver - unregister vGPU driver
> + * @drv: the driver to unregister
> + *
> + */
> +void vgpu_unregister_driver(struct vgpu_driver *drv)
> +{
> +    driver_unregister(&drv->driver);
> +}
> +EXPORT_SYMBOL(vgpu_unregister_driver);
> +
> +int vgpu_bus_register(void)
> +{
> +    return bus_register(&vgpu_bus_type);
> +}
> +
> +void vgpu_bus_unregister(void)
> +{
> +    bus_unregister(&vgpu_bus_type);
> +}
> +
> diff --git a/drivers/vgpu/vgpu_dev.c b/drivers/vgpu/vgpu_dev.c
> new file mode 100644
> index 0000000..e5a92a1
> --- /dev/null
> +++ b/drivers/vgpu/vgpu_dev.c
> @@ -0,0 +1,421 @@
> +/*
> + * VGPU Core Driver
> + *
> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
> + *     Author: Neo Jia <cjia@nvidia.com>
> + *           Kirti Wankhede <kwankhede@nvidia.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/init.h>
> +#include <linux/module.h>
> +#include <linux/device.h>
> +#include <linux/kernel.h>
> +#include <linux/fs.h>
> +#include <linux/poll.h>
> +#include <linux/slab.h>
> +#include <linux/cdev.h>
> +#include <linux/sched.h>
> +#include <linux/wait.h>
> +#include <linux/uuid.h>
> +#include <linux/vfio.h>
> +#include <linux/iommu.h>
> +#include <linux/sysfs.h>
> +#include <linux/ctype.h>
> +#include <linux/vgpu.h>
> +
> +#include "vgpu_private.h"
> +
> +#define DRIVER_VERSION    "0.1"
> +#define DRIVER_AUTHOR    "NVIDIA Corporation"
> +#define DRIVER_DESC    "VGPU Core Driver"
> +
> +/*
> + * #defines
> + */
> +
> +#define VGPU_CLASS_NAME        "vgpu"
> +
> +/*
> + * Global Structures
> + */
> +
> +static struct vgpu {
> +    struct list_head    vgpu_devices_list;
> +    struct mutex        vgpu_devices_lock;
> +    struct list_head    gpu_devices_list;
> +    struct mutex        gpu_devices_lock;
> +} vgpu;
> +
> +
> +static struct class vgpu_class;
> +
> +/*
> + * Function prototypes
> + */
> +
> +unsigned int vgpu_poll(struct file *file, poll_table *wait);
> +long vgpu_unlocked_ioctl(struct file *file, unsigned int cmd, unsigned
> long i_arg);
> +int vgpu_mmap(struct file *file, struct vm_area_struct *vma);
> +
> +int vgpu_open(struct inode *inode, struct file *file);
> +int vgpu_close(struct inode *inode, struct file *file);
> +ssize_t vgpu_read(struct file *file, char __user * buf,
> +              size_t len, loff_t * ppos);
> +ssize_t vgpu_write(struct file *file, const char __user *data,
> +               size_t len, loff_t *ppos);
> +
> +/*
> + * Functions
> + */
> +
> +struct vgpu_device *get_vgpu_device_from_group(struct iommu_group *group)
> +{
> +    struct vgpu_device *vdev = NULL;
> +
> +    mutex_lock(&vgpu.vgpu_devices_lock);
> +    list_for_each_entry(vdev, &vgpu.vgpu_devices_list, list) {
> +        if (vdev->group) {
> +            if (iommu_group_id(vdev->group) == iommu_group_id(group)) {
> +                mutex_unlock(&vgpu.vgpu_devices_lock);
> +                return vdev;
> +            }
> +        }
> +    }
> +    mutex_unlock(&vgpu.vgpu_devices_lock);
> +    return NULL;
> +}
> +
> +EXPORT_SYMBOL_GPL(get_vgpu_device_from_group);
> +
> +int vgpu_register_device(struct pci_dev *dev, const struct
> gpu_device_ops *ops)
> +{
> +    int ret = 0;
> +    struct gpu_device *gpu_dev, *tmp;
> +
> +    if (!dev)
> +        return -EINVAL;
> +
> +        gpu_dev = kzalloc(sizeof(*gpu_dev), GFP_KERNEL);
> +        if (!gpu_dev)
> +                return -ENOMEM;
> +
> +    gpu_dev->dev = dev;
> +        gpu_dev->ops = ops;
> +
> +        mutex_lock(&vgpu.gpu_devices_lock);
> +
> +        /* Check for duplicates */
> +        list_for_each_entry(tmp, &vgpu.gpu_devices_list, gpu_next) {
> +                if (tmp->dev == dev) {
> +                        mutex_unlock(&vgpu.gpu_devices_lock);
> +                        kfree(gpu_dev);
> +                        return -EINVAL;
> +                }
> +        }
> +
> +    ret = vgpu_create_pci_device_files(dev);
> +    if (ret) {
> +        mutex_unlock(&vgpu.gpu_devices_lock);
> +        kfree(gpu_dev);
> +        return ret;
> +    }
> +        list_add(&gpu_dev->gpu_next, &vgpu.gpu_devices_list);
> +
> +    printk(KERN_INFO "VGPU: Registered dev 0x%x 0x%x, class 0x%x\n",
> dev->vendor, dev->device, dev->class);
> +        mutex_unlock(&vgpu.gpu_devices_lock);
> +
> +        return 0;
> +}
> +EXPORT_SYMBOL(vgpu_register_device);
> +
> +void vgpu_unregister_device(struct pci_dev *dev)
> +{
> +        struct gpu_device *gpu_dev;
> +
> +        mutex_lock(&vgpu.gpu_devices_lock);
> +        list_for_each_entry(gpu_dev, &vgpu.gpu_devices_list, gpu_next) {
> +                if (gpu_dev->dev == dev) {
> +            printk(KERN_INFO "VGPU: Unregistered dev 0x%x 0x%x, class
> 0x%x\n", dev->vendor, dev->device, dev->class);
> +            vgpu_remove_pci_device_files(dev);
> +                        list_del(&gpu_dev->gpu_next);
> +                        mutex_unlock(&vgpu.gpu_devices_lock);
> +                        kfree(gpu_dev);
> +                        return;
> +                }
> +        }
> +        mutex_unlock(&vgpu.gpu_devices_lock);
> +}
> +EXPORT_SYMBOL(vgpu_unregister_device);
> +
> +/*
> + * Helper Functions
> + */
> +
> +static struct vgpu_device *vgpu_device_alloc(uuid_le uuid, int
> instance, char *name)
> +{
> +    struct vgpu_device *vgpu_dev = NULL;
> +
> +    vgpu_dev = kzalloc(sizeof(*vgpu_dev), GFP_KERNEL);
> +    if (!vgpu_dev)
> +        return ERR_PTR(-ENOMEM);
> +
> +    kref_init(&vgpu_dev->kref);
> +    memcpy(&vgpu_dev->uuid, &uuid, sizeof(uuid_le));
> +    vgpu_dev->vgpu_instance = instance;
> +    strcpy(vgpu_dev->dev_name, name);
> +
> +    mutex_lock(&vgpu.vgpu_devices_lock);
> +    list_add(&vgpu_dev->list, &vgpu.vgpu_devices_list);
> +    mutex_unlock(&vgpu.vgpu_devices_lock);
> +
> +    return vgpu_dev;
> +}
> +
> +static void vgpu_device_free(struct vgpu_device *vgpu_dev)
> +{
> +    if (vgpu_dev) {
> +        mutex_lock(&vgpu.vgpu_devices_lock);
> +        list_del(&vgpu_dev->list);
> +        mutex_unlock(&vgpu.vgpu_devices_lock);
> +        kfree(vgpu_dev);
> +    }
> +    return;
> +}
> +
> +struct vgpu_device *vgpu_drv_get_vgpu_device_by_uuid(uuid_le uuid, int
> instance)
> +{
> +    struct vgpu_device *vdev = NULL;
> +
> +    mutex_lock(&vgpu.vgpu_devices_lock);
> +    list_for_each_entry(vdev, &vgpu.vgpu_devices_list, list) {
> +        if ((uuid_le_cmp(vdev->uuid, uuid) == 0) &&
> +                (vdev->vgpu_instance == instance)) {
> +            mutex_unlock(&vgpu.vgpu_devices_lock);
> +            return vdev;
> +        }
> +    }
> +    mutex_unlock(&vgpu.vgpu_devices_lock);
> +    return NULL;
> +}
> +
> +static void vgpu_device_release(struct device *dev)
> +{
> +    struct vgpu_device *vgpu_dev = to_vgpu_device(dev);
> +    vgpu_device_free(vgpu_dev);
> +}
> +
> +int create_vgpu_device(struct pci_dev *pdev, uuid_le uuid, uint32_t
> instance, uint32_t vgpu_id)
> +{
> +    char name[64];
> +    int numChar = 0;
> +    int retval = 0;
> +    struct vgpu_device *vgpu_dev = NULL;
> +    struct gpu_device *gpu_dev;
> +
> +    printk(KERN_INFO "VGPU: %s: device ", __FUNCTION__);
> +
> +    numChar = sprintf(name, "%pUb-%d", uuid.b, instance);
> +    name[numChar] = '\0';
> +
> +    vgpu_dev = vgpu_device_alloc(uuid, instance, name);
> +    if (IS_ERR(vgpu_dev)) {
> +        return PTR_ERR(vgpu_dev);
> +    }
> +
> +    vgpu_dev->vgpu_id     = vgpu_id;
> +    vgpu_dev->dev.parent  = NULL;
> +    vgpu_dev->dev.bus     = &vgpu_bus_type;
> +    vgpu_dev->dev.release = vgpu_device_release;
> +    dev_set_name(&vgpu_dev->dev, "%s", name);
> +
> +    retval = device_register(&vgpu_dev->dev);
> +    if (retval)
> +        goto create_failed1;
> +
> +    printk(KERN_INFO "UUID %pUb \n", vgpu_dev->uuid.b);
> +
> +    mutex_lock(&vgpu.gpu_devices_lock);
> +    list_for_each_entry(gpu_dev, &vgpu.gpu_devices_list, gpu_next) {
> +        if (gpu_dev->dev == pdev) {
> +            vgpu_dev->gpu_dev = gpu_dev;
> +            if (gpu_dev->ops->vgpu_create) {
> +                retval = gpu_dev->ops->vgpu_create(pdev, vgpu_dev->uuid,
> +                                   instance, vgpu_id);
> +                if (retval) {
> +                    mutex_unlock(&vgpu.gpu_devices_lock);
> +                    goto create_failed2;
> +                }
> +            }
> +            break;
> +        }
> +    }
> +    if (!vgpu_dev->gpu_dev) {
> +        retval = -EINVAL;
> +        goto create_failed2;
> +    }
> +
> +    mutex_unlock(&vgpu.gpu_devices_lock);
> +
> +    return retval;
> +
> +create_failed2:
> +    device_unregister(&vgpu_dev->dev);
> +
> +create_failed1:
> +    vgpu_device_free(vgpu_dev);
> +
> +    return retval;
> +}
> +
> +void destroy_vgpu_device(struct vgpu_device *vgpu_dev)
> +{
> +    printk(KERN_INFO "VGPU: destroying device %s ", vgpu_dev->dev_name);
> +    if (vgpu_dev->gpu_dev->ops->vgpu_destroy) {
> +        int retval = 0;
> +        retval =
> vgpu_dev->gpu_dev->ops->vgpu_destroy(vgpu_dev->gpu_dev->dev,
> +                                  vgpu_dev->uuid,
> +                                  vgpu_dev->vgpu_instance);
> +    /* if vendor driver doesn't return success that means vendor driver
> doesn't
> +     * support hot-unplug */
> +        if (retval)
> +            return;
> +    }
> +
> +    device_unregister(&vgpu_dev->dev);
> +}
> +
> +void destroy_vgpu_device_by_uuid(uuid_le uuid, int instance)
> +{
> +    struct vgpu_device *vdev, *vgpu_dev = NULL;
> +
> +    mutex_lock(&vgpu.vgpu_devices_lock);
> +
> +    // search VGPU device
> +    list_for_each_entry(vdev, &vgpu.vgpu_devices_list, list) {
> +        if ((uuid_le_cmp(vdev->uuid, uuid) == 0) &&
> +                (vdev->vgpu_instance == instance)) {
> +            vgpu_dev = vdev;
> +            break;
> +        }
> +    }
> +
> +    mutex_unlock(&vgpu.vgpu_devices_lock);
> +    if (vgpu_dev)
> +        destroy_vgpu_device(vgpu_dev);
> +}
> +
> +void get_vgpu_supported_types(struct device *dev, char *str)
> +{
> +    struct gpu_device *gpu_dev;
> +
> +    mutex_lock(&vgpu.gpu_devices_lock);
> +    list_for_each_entry(gpu_dev, &vgpu.gpu_devices_list, gpu_next) {
> +        if (&gpu_dev->dev->dev == dev) {
> +            if (gpu_dev->ops->vgpu_supported_config)
> +                gpu_dev->ops->vgpu_supported_config(gpu_dev->dev, str);
> +            break;
> +        }
> +    }
> +    mutex_unlock(&vgpu.gpu_devices_lock);
> +}
> +
> +int vgpu_start_callback(struct vgpu_device *vgpu_dev)
> +{
> +    int ret = 0;
> +
> +    mutex_lock(&vgpu.gpu_devices_lock);
> +    if (vgpu_dev->gpu_dev->ops->vgpu_start)
> +        ret = vgpu_dev->gpu_dev->ops->vgpu_start(vgpu_dev->uuid);
> +    mutex_unlock(&vgpu.gpu_devices_lock);
> +    return ret;
> +}
> +
> +int vgpu_shutdown_callback(struct vgpu_device *vgpu_dev)
> +{
> +    int ret = 0;
> +
> +    mutex_lock(&vgpu.gpu_devices_lock);
> +    if (vgpu_dev->gpu_dev->ops->vgpu_shutdown)
> +        ret = vgpu_dev->gpu_dev->ops->vgpu_shutdown(vgpu_dev->uuid);
> +    mutex_unlock(&vgpu.gpu_devices_lock);
> +    return ret;
> +}
> +
> +int vgpu_set_irqs_callback(struct vgpu_device *vgpu_dev, uint32_t flags,
> +                           unsigned index, unsigned start, unsigned count,
> +                           void *data)
> +{
> +       int ret = 0;
> +
> +       mutex_lock(&vgpu.gpu_devices_lock);
> +       if (vgpu_dev->gpu_dev->ops->vgpu_set_irqs)
> +               ret = vgpu_dev->gpu_dev->ops->vgpu_set_irqs(vgpu_dev,
> flags,
> +                                                          index, start,
> count, data);
> +       mutex_unlock(&vgpu.gpu_devices_lock);
> +       return ret;
> +}
> +
> +char *vgpu_devnode(struct device *dev, umode_t *mode)
> +{
> +    return kasprintf(GFP_KERNEL, "vgpu/%s", dev_name(dev));
> +}
> +
> +static void release_vgpubus_dev(struct device *dev)
> +{
> +    struct vgpu_device *vgpu_dev = to_vgpu_device(dev);
> +    destroy_vgpu_device(vgpu_dev);
> +}
> +
> +static struct class vgpu_class = {
> +    .name        = VGPU_CLASS_NAME,
> +    .owner        = THIS_MODULE,
> +    .class_attrs    = vgpu_class_attrs,
> +    .dev_groups    = vgpu_dev_groups,
> +    .devnode    = vgpu_devnode,
> +    .dev_release    = release_vgpubus_dev,
> +};
> +
> +static int __init vgpu_init(void)
> +{
> +    int rc = 0;
> +
> +    memset(&vgpu, 0 , sizeof(vgpu));
> +
> +    mutex_init(&vgpu.vgpu_devices_lock);
> +    INIT_LIST_HEAD(&vgpu.vgpu_devices_list);
> +    mutex_init(&vgpu.gpu_devices_lock);
> +    INIT_LIST_HEAD(&vgpu.gpu_devices_list);
> +
> +    rc = class_register(&vgpu_class);
> +    if (rc < 0) {
> +        printk(KERN_ERR "Error: failed to register vgpu class\n");
> +        goto failed1;
> +    }
> +
> +    rc = vgpu_bus_register();
> +    if (rc < 0) {
> +        printk(KERN_ERR "Error: failed to register vgpu bus\n");
> +        class_unregister(&vgpu_class);
> +    }
> +
> +failed1:
> +    return rc;
> +}
> +
> +static void __exit vgpu_exit(void)
> +{
> +    vgpu_bus_unregister();
> +    class_unregister(&vgpu_class);
> +}
> +
> +module_init(vgpu_init)
> +module_exit(vgpu_exit)
> +
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL");
> +MODULE_AUTHOR(DRIVER_AUTHOR);
> +MODULE_DESCRIPTION(DRIVER_DESC);
> diff --git a/include/linux/vgpu.h b/include/linux/vgpu.h
> new file mode 100644
> index 0000000..8595df5
> --- /dev/null
> +++ b/include/linux/vgpu.h
> @@ -0,0 +1,189 @@
> +/*
> + * VGPU definition
> + *
> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
> + *     Author:
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef VGPU_H
> +#define VGPU_H
> +
> +// Common Data structures
> +
> +struct pci_bar_info {
> +    uint64_t start;
> +    uint64_t end;
> +    int flags;
> +};
> +
> +enum vgpu_emul_space_e {
> +    vgpu_emul_space_config = 0, /*!< PCI configuration space */
> +    vgpu_emul_space_io = 1,     /*!< I/O register space */
> +    vgpu_emul_space_mmio = 2    /*!< Memory-mapped I/O space */
> +};
> +
> +struct gpu_device;
> +
> +/*
> + * VGPU device
> + */
> +struct vgpu_device {
> +    struct kref        kref;
> +    struct device        dev;
> +    struct gpu_device    *gpu_dev;
> +    struct iommu_group    *group;
> +#define DEVICE_NAME_LEN        (64)
> +    char            dev_name[DEVICE_NAME_LEN];
> +    uuid_le            uuid;
> +    uint32_t        vgpu_instance;
> +    uint32_t        vgpu_id;
> +    char            config_space[0x100];          // 4KB PCI cfg space
> +    struct pci_bar_info    bar[VFIO_PCI_NUM_REGIONS];
> +    struct device_attribute    *dev_attr_vgpu_status;
> +    int            vgpu_device_status;
> +
> +    void            *driver_data;
> +
> +    struct list_head    list;
> +};
> +
> +
> +/**
> + * struct gpu_device_ops - Structure to be registered for each physical
> GPU to
> + * register the device to vgpu module.
> + *
> + * @owner:            The module owner.
> + * @vgpu_supported_config:    Called to get information about supported
> vgpu types.
> + *                @dev : pci device structure of physical GPU.
> + *                @config: should return string listing supported config
> + *                Returns integer: success (0) or error (< 0)
> + * @vgpu_create:        Called to allocate basic resouces in graphics
> + *                driver for a particular vgpu.
> + *                @dev: physical pci device structure on which vgpu
> + *                      should be created
> + *                @uuid: uuid for which VM it is intended to
> + *                @instance: vgpu instance in that VM
> + *                @vgpu_id: This represents the type of vgpu to be
> + *                      created
> + *                Returns integer: success (0) or error (< 0)
> + * @vgpu_destroy:        Called to free resources in graphics driver for
> + *                a vgpu instance of that VM.
> + *                @dev: physical pci device structure to which
> + *                this vgpu points to.
> + *                @uuid: uuid for which the vgpu belongs to.
> + *                @instance: vgpu instance in that VM
> + *                Returns integer: success (0) or error (< 0)
> + *                If VM is running and vgpu_destroy is called that
> + *                means the vGPU is being hotunpluged. Return error
> + *                if VM is running and graphics driver doesn't
> + *                support vgpu hotplug.
> + * @vgpu_start:            Called to do initiate vGPU initialization
> + *                process in graphics driver when VM boots before
> + *                qemu starts.
> + *                @uuid: UUID which is booting.
> + *                Returns integer: success (0) or error (< 0)
> + * @vgpu_shutdown:        Called to teardown vGPU related resources for
> + *                the VM
> + *                @uuid: UUID which is shutting down .
> + *                Returns integer: success (0) or error (< 0)
> + * @read:            Read emulation callback
> + *                @vdev: vgpu device structure
> + *                @buf: read buffer
> + *                @count: number bytes to read
> + *                @address_space: specifies for which address space
> + *                the request is: pci_config_space, IO register
> + *                space or MMIO space.
> + *                Retuns number on bytes read on success or error.
> + * @write:            Write emulation callback
> + *                @vdev: vgpu device structure
> + *                @buf: write buffer
> + *                @count: number bytes to be written
> + *                @address_space: specifies for which address space
> + *                the request is: pci_config_space, IO register
> + *                space or MMIO space.
> + *                Retuns number on bytes written on success or error.
> + * @vgpu_set_irqs:        Called to send about interrupts configuration
> + *                information that qemu set.
> + *                @vdev: vgpu device structure
> + *                @flags, index, start, count and *data : same as
> + *                that of struct vfio_irq_set of
> + *                VFIO_DEVICE_SET_IRQS API.
> + *
> + * Physical GPU that support vGPU should be register with vgpu module with
> + * gpu_device_ops structure.
> + */
> +
> +struct gpu_device_ops {
> +    struct module   *owner;
> +    int    (*vgpu_supported_config)(struct pci_dev *dev, char *config);
> +    int     (*vgpu_create)(struct pci_dev *dev, uuid_le uuid,
> +                   uint32_t instance, uint32_t vgpu_id);
> +    int     (*vgpu_destroy)(struct pci_dev *dev, uuid_le uuid,
> +                    uint32_t instance);
> +    int     (*vgpu_start)(uuid_le uuid);
> +    int     (*vgpu_shutdown)(uuid_le uuid);
> +    ssize_t (*read) (struct vgpu_device *vdev, char *buf, size_t count,
> +             uint32_t address_space, loff_t pos);
> +    ssize_t (*write)(struct vgpu_device *vdev, char *buf, size_t count,
> +             uint32_t address_space,loff_t pos);
> +    int     (*vgpu_set_irqs)(struct vgpu_device *vdev, uint32_t flags,
> +                 unsigned index, unsigned start, unsigned count,
> +                 void *data);
> +
> +};
> +
> +/*
> + * Physical GPU
> + */
> +struct gpu_device {
> +    struct pci_dev                  *dev;
> +    const struct gpu_device_ops     *ops;
> +    struct list_head                gpu_next;
> +};
> +
> +/**
> + * struct vgpu_driver - vGPU device driver
> + * @name: driver name
> + * @probe: called when new device created
> + * @remove: called when device removed
> + * @driver: device driver structure
> + *
> + **/
> +struct vgpu_driver {
> +    const char *name;
> +    int  (*probe)  (struct device *dev);
> +    void (*remove) (struct device *dev);
> +    struct device_driver    driver;
> +};
> +
> +static inline struct vgpu_driver *to_vgpu_driver(struct device_driver
> *drv)
> +{
> +    return drv ? container_of(drv, struct vgpu_driver, driver) : NULL;
> +}
> +
> +static inline struct vgpu_device *to_vgpu_device(struct device *dev)
> +{
> +    return dev ? container_of(dev, struct vgpu_device, dev) : NULL;
> +}
> +
> +extern struct bus_type vgpu_bus_type;
> +
> +#define dev_is_vgpu(d) ((d)->bus == &vgpu_bus_type)
> +
> +extern int  vgpu_register_device(struct pci_dev *dev, const struct
> gpu_device_ops *ops);
> +extern void vgpu_unregister_device(struct pci_dev *dev);
> +
> +extern int  vgpu_register_driver(struct vgpu_driver *drv, struct module
> *owner);
> +extern void vgpu_unregister_driver(struct vgpu_driver *drv);
> +
> +extern int vgpu_map_virtual_bar(uint64_t virt_bar_addr, uint64_t
> phys_bar_addr, uint32_t len, uint32_t flags);
> +extern int vgpu_dma_do_translate(dma_addr_t * gfn_buffer, uint32_t count);
> +
> +struct vgpu_device *get_vgpu_device_from_group(struct iommu_group *group);
> +
> +#endif /* VGPU_H */
> +

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Kirti Wankhede [mailto:kwankhede@nvidia.com]
> Sent: Tuesday, February 02, 2016 9:48 AM
> 
> Resending this mail again, somehow my previous mail didn't reached every
> to everyone's inbox.
> 
> On 2/2/2016 3:16 AM, Kirti Wankhede wrote:
> > Design for vGPU Driver:
> > Main purpose of vGPU driver is to provide a common interface for vGPU
> > management that can be used by differnt GPU drivers.

Thanks for composing this design which is a good start.

> >
> > This module would provide a generic interface to create the device, add
> > it to vGPU bus, add device to IOMMU group and then add it to vfio group.
> >
> > High Level block diagram:
> >
> >
> > +--------------+    vgpu_register_driver()+---------------+
> > |     __init() +------------------------->+               |
> > |              |                          |               |
> > |              +<-------------------------+    vgpu.ko    |
> > | vfio_vgpu.ko |   probe()/remove()       |               |
> > |              |                +---------+               +---------+
> > +--------------+                |         +-------+-------+         |
> >                                  |                 ^                 |
> >                                  | callback        |                 |
> >                                  |         +-------+--------+        |
> >                                  |         |vgpu_register_device()   |
> >                                  |         |                |        |
> >                                  +---^-----+-----+    +-----+------+-+
> >                                      | nvidia.ko |    |  i915.ko   |
> >                                      |           |    |            |
> >                                      +-----------+    +------------+
> >
> > vGPU driver provides two types of registration interfaces:

Looks you missed callbacks which vgpu.ko provides to vfio_vgpu.ko,
e.g. to retrieve basic region/resource info, etc...

Also for GPU driver interfaces, better to identify the caller. E.g. it's
easy to understand life-cycle management would come from sysfs
by mgmt. stack like libvirt. What about @read and @write? what's
the connection between this vgpu core driver and specific hypervisor?
etc. Better to connect all necessary dots so we can refine all 
necessary requirements on this proposal.

[...]
> >
> > 2. GPU driver interface
> >
> > /**
> >   * struct gpu_device_ops - Structure to be registered for each physical
> > GPU to
> >   * register the device to vgpu module.
> >   *
> >   * @owner:              The module owner.
> >   * @vgpu_supported_config: Called to get information about supported
> >   *                       vgpu types.
> >   *                      @dev : pci device structure of physical GPU.
> >   *                      @config: should return string listing supported
> >   *                      config
> >   *                      Returns integer: success (0) or error (< 0)
> >   * @vgpu_create:        Called to allocate basic resouces in graphics
> >   *                      driver for a particular vgpu.
> >   *                      @dev: physical pci device structure on which vgpu
> >   *                            should be created
> >   *                      @uuid: uuid for which VM it is intended to
> >   *                      @instance: vgpu instance in that VM
> >   *                      @vgpu_id: This represents the type of vgpu to be
> >   *                                created
> >   *                      Returns integer: success (0) or error (< 0)

Specifically for Intel GVT-g we didn't hard partition resource among vGPUs.
Instead we allow user to accurately control how many physical resources
are allocated to a vGPU. So this interface should be extensible to allow
vendor specific resource control.

And for UUID, I remember Alex had a concern on using it in kernel. 
Honestly speaking I don't have a good idea here. In Xen side there is a VM ID
which can be easily used as the index. But for KVM, what would be the best
identifier to associate with a VM?

> >   * @vgpu_destroy:       Called to free resources in graphics driver for
> >   *                      a vgpu instance of that VM.
> >   *                      @dev: physical pci device structure to which
> >   *                      this vgpu points to.
> >   *                      @uuid: uuid for which the vgpu belongs to.
> >   *                      @instance: vgpu instance in that VM
> >   *                      Returns integer: success (0) or error (< 0)
> >   *                      If VM is running and vgpu_destroy is called that
> >   *                      means the vGPU is being hotunpluged. Return error
> >   *                      if VM is running and graphics driver doesn't
> >   *                      support vgpu hotplug.
> >   * @vgpu_start:         Called to do initiate vGPU initialization
> >   *                      process in graphics driver when VM boots before
> >   *                      qemu starts.
> >   *                      @uuid: UUID which is booting.
> >   *                      Returns integer: success (0) or error (< 0)
> >   * @vgpu_shutdown:      Called to teardown vGPU related resources for
> >   *                      the VM
> >   *                      @uuid: UUID which is shutting down .
> >   *                      Returns integer: success (0) or error (< 0)
> >   * @read:               Read emulation callback
> >   *                      @vdev: vgpu device structure
> >   *                      @buf: read buffer
> >   *                      @count: number bytes to read
> >   *                      @address_space: specifies for which address space

and suppose there'll be an 'offset' as required by usual emulation.

> >   *                      the request is: pci_config_space, IO register
> >   *                      space or MMIO space.
> >   *                      Retuns number on bytes read on success or error.
> >   * @write:              Write emulation callback
> >   *                      @vdev: vgpu device structure
> >   *                      @buf: write buffer
> >   *                      @count: number bytes to be written
> >   *                      @address_space: specifies for which address space
> >   *                      the request is: pci_config_space, IO register
> >   *                      space or MMIO space.
> >   *                      Retuns number on bytes written on success or
> > error.
> >   * @vgpu_set_irqs:      Called to send about interrupts configuration
> >   *                      information that qemu set.
> >   *                      @vdev: vgpu device structure
> >   *                      @flags, index, start, count and *data : same as
> >   *                      that of struct vfio_irq_set of
> >   *                      VFIO_DEVICE_SET_IRQS API.

any elaboration how this will be used in your case?

> >   *
> >   * Physical GPU that support vGPU should be register with vgpu module with
> >   * gpu_device_ops structure.
> >   */
> >

Also it'd be good design to allow extensible usages, such as statistics, and
other vendor specific control knobs (e.g. foreground/background VM switch
in Intel GVT-g, etc.)


Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Gerd Hoffmann]

  Hi,

> And for UUID, I remember Alex had a concern on using it in kernel. 
> Honestly speaking I don't have a good idea here. In Xen side there is a VM ID
> which can be easily used as the index. But for KVM, what would be the best
> identifier to associate with a VM?

The vgpu code doesn't need to associate the vgpu device with a vm in the
first place.  You get all guest address space information from qemu, via
vfio iommu interface.

When qemu does't use kvm (tcg mode), things should still work fine.
Using vfio-based vgpu devices with non-qemu apps (some kind of test
suite for example) should work fine too.

cheers,
  Gerd

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Tue, Feb 02, 2016 at 09:00:43AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > And for UUID, I remember Alex had a concern on using it in kernel. 
> > Honestly speaking I don't have a good idea here. In Xen side there is a VM ID
> > which can be easily used as the index. But for KVM, what would be the best
> > identifier to associate with a VM?
> 
> The vgpu code doesn't need to associate the vgpu device with a vm in the
> first place.  You get all guest address space information from qemu, via
> vfio iommu interface.
> 
> When qemu does't use kvm (tcg mode), things should still work fine.
> Using vfio-based vgpu devices with non-qemu apps (some kind of test
> suite for example) should work fine too.

Hi Gerd and Kevin,

I thought Alex had agreed with the UUID as long as it is not tied with VM,
probably it is just his comment gets lost in our previous long email thread.

Thanks,
Neo

> 
> cheers,
>   Gerd
> 

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia [mailto:cjia@nvidia.com]
> Sent: Tuesday, February 02, 2016 4:13 PM
> 
> On Tue, Feb 02, 2016 at 09:00:43AM +0100, Gerd Hoffmann wrote:
> >   Hi,
> >
> > > And for UUID, I remember Alex had a concern on using it in kernel.
> > > Honestly speaking I don't have a good idea here. In Xen side there is a VM ID
> > > which can be easily used as the index. But for KVM, what would be the best
> > > identifier to associate with a VM?
> >
> > The vgpu code doesn't need to associate the vgpu device with a vm in the
> > first place.  You get all guest address space information from qemu, via
> > vfio iommu interface.
> >
> > When qemu does't use kvm (tcg mode), things should still work fine.
> > Using vfio-based vgpu devices with non-qemu apps (some kind of test
> > suite for example) should work fine too.
> 
> Hi Gerd and Kevin,
> 
> I thought Alex had agreed with the UUID as long as it is not tied with VM,
> probably it is just his comment gets lost in our previous long email thread.
> 

I think the point is... what is the value to introduce a UUID here? If
what Gerd describes is enough, we can simply invent a vgpu ID which
is returned at vgpu_create, and then used as the index for other
interfaces.

But I still need to think about whether there's value to have a VM
identifier within vgpu core driver, especially regarding to how this
vgpu core driver connects to KVM hypervisor or other hypervisor.
I saw another long thread about that part. Jike has started his 
vacation now. I'll follow up with it tomorrow.

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Tue, Feb 02, 2016 at 08:18:44AM +0000, Tian, Kevin wrote:
> > From: Neo Jia [mailto:cjia@nvidia.com]
> > Sent: Tuesday, February 02, 2016 4:13 PM
> > 
> > On Tue, Feb 02, 2016 at 09:00:43AM +0100, Gerd Hoffmann wrote:
> > >   Hi,
> > >
> > > > And for UUID, I remember Alex had a concern on using it in kernel.
> > > > Honestly speaking I don't have a good idea here. In Xen side there is a VM ID
> > > > which can be easily used as the index. But for KVM, what would be the best
> > > > identifier to associate with a VM?
> > >
> > > The vgpu code doesn't need to associate the vgpu device with a vm in the
> > > first place.  You get all guest address space information from qemu, via
> > > vfio iommu interface.
> > >
> > > When qemu does't use kvm (tcg mode), things should still work fine.
> > > Using vfio-based vgpu devices with non-qemu apps (some kind of test
> > > suite for example) should work fine too.
> > 
> > Hi Gerd and Kevin,
> > 
> > I thought Alex had agreed with the UUID as long as it is not tied with VM,
> > probably it is just his comment gets lost in our previous long email thread.
> > 
> 
> I think the point is... what is the value to introduce a UUID here? If
> what Gerd describes is enough, we can simply invent a vgpu ID which
> is returned at vgpu_create, and then used as the index for other
> interfaces.
> 

Hi Kevin,

It can just be a plain UUID, and the meaning of the UUID is up to upper layer SW, for
example with libvirt, you can create a new "vgpu group" object representing a
list of vgpu device. so the UUID will be the input on vgpu_create instead of
return value.

For the TCG mode, this should just work as long as libvirt can create the proper
internal objects there plus other vfio iommu interface Gerd has called out,
although the vector->kvm_interrupt part might need some tweaks.

Thanks,
Neo

> But I still need to think about whether there's value to have a VM
> identifier within vgpu core driver, especially regarding to how this
> vgpu core driver connects to KVM hypervisor or other hypervisor.
> I saw another long thread about that part. Jike has started his 
> vacation now. I'll follow up with it tomorrow.
> 
> Thanks
> Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Alex Williamson]

On Tue, 2016-02-02 at 00:31 -0800, Neo Jia wrote:
> On Tue, Feb 02, 2016 at 08:18:44AM +0000, Tian, Kevin wrote:
> > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > Sent: Tuesday, February 02, 2016 4:13 PM
> > > 
> > > On Tue, Feb 02, 2016 at 09:00:43AM +0100, Gerd Hoffmann wrote:
> > > >   Hi,
> > > > 
> > > > > And for UUID, I remember Alex had a concern on using it in kernel.
> > > > > Honestly speaking I don't have a good idea here. In Xen side there is a VM ID
> > > > > which can be easily used as the index. But for KVM, what would be the best
> > > > > identifier to associate with a VM?
> > > > 
> > > > The vgpu code doesn't need to associate the vgpu device with a vm in the
> > > > first place.  You get all guest address space information from qemu, via
> > > > vfio iommu interface.
> > > > 
> > > > When qemu does't use kvm (tcg mode), things should still work fine.
> > > > Using vfio-based vgpu devices with non-qemu apps (some kind of test
> > > > suite for example) should work fine too.
> > > 
> > > Hi Gerd and Kevin,
> > > 
> > > I thought Alex had agreed with the UUID as long as it is not tied with VM,
> > > probably it is just his comment gets lost in our previous long email thread.
> > > 
> > 
> > I think the point is... what is the value to introduce a UUID here? If
> > what Gerd describes is enough, we can simply invent a vgpu ID which
> > is returned at vgpu_create, and then used as the index for other
> > interfaces.
> > 
> 
> Hi Kevin,
> 
> It can just be a plain UUID, and the meaning of the UUID is up to upper layer SW, for
> example with libvirt, you can create a new "vgpu group" object representing a
> list of vgpu device. so the UUID will be the input on vgpu_create instead of
> return value.

Jumping in at th end, but yes, this was my thinking.  A UUID is a 
perfectly fine name for a vgpu, but it should be user policy whether
than UUID matches a VM definition or is simply an arbitrary grouping of
vgpus.

> For the TCG mode, this should just work as long as libvirt can create the proper
> internal objects there plus other vfio iommu interface Gerd has called out,
> although the vector->kvm_interrupt part might need some tweaks.

Interrupts should be eventfds and whether the eventfd triggers into
userspace or into an irqfd in KVM should be completely transparent to
the vgpu code, just as is done with vfio today.  Thanks,

Alex

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Wednesday, February 03, 2016 1:11 AM
> 
> On Tue, 2016-02-02 at 00:31 -0800, Neo Jia wrote:
> > On Tue, Feb 02, 2016 at 08:18:44AM +0000, Tian, Kevin wrote:
> > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > Sent: Tuesday, February 02, 2016 4:13 PM
> > > >
> > > > On Tue, Feb 02, 2016 at 09:00:43AM +0100, Gerd Hoffmann wrote:
> > > > >   Hi,
> > > > >
> > > > > > And for UUID, I remember Alex had a concern on using it in kernel.
> > > > > > Honestly speaking I don't have a good idea here. In Xen side there is a VM ID
> > > > > > which can be easily used as the index. But for KVM, what would be the best
> > > > > > identifier to associate with a VM?
> > > > >
> > > > > The vgpu code doesn't need to associate the vgpu device with a vm in the
> > > > > first place.  You get all guest address space information from qemu, via
> > > > > vfio iommu interface.
> > > > >
> > > > > When qemu does't use kvm (tcg mode), things should still work fine.
> > > > > Using vfio-based vgpu devices with non-qemu apps (some kind of test
> > > > > suite for example) should work fine too.
> > > >
> > > > Hi Gerd and Kevin,
> > > >
> > > > I thought Alex had agreed with the UUID as long as it is not tied with VM,
> > > > probably it is just his comment gets lost in our previous long email thread.
> > > >
> > >
> > > I think the point is... what is the value to introduce a UUID here? If
> > > what Gerd describes is enough, we can simply invent a vgpu ID which
> > > is returned at vgpu_create, and then used as the index for other
> > > interfaces.
> > >
> >
> > Hi Kevin,
> >
> > It can just be a plain UUID, and the meaning of the UUID is up to upper layer SW, for
> > example with libvirt, you can create a new "vgpu group" object representing a
> > list of vgpu device. so the UUID will be the input on vgpu_create instead of
> > return value.
> 
> Jumping in at th end, but yes, this was my thinking.  A UUID is a
> perfectly fine name for a vgpu, but it should be user policy whether
> than UUID matches a VM definition or is simply an arbitrary grouping of
> vgpus.
> 

well I'm still trying to understand the necessity here. Is there any requirement
that vgpu driver needs to use UUID itself? If not, we can still invent any
simple format in vgpu driver itself, as Gerd mentioned, w/o dependency
on an user input. Then libvirt can maintain UUID->vgpu ID internally.

Actually I have a long puzzle in this area. Definitely libvirt will use UUID to
mark a VM. And obviously UUID is not recorded within KVM. Then how does
libvirt talk to KVM based on UUID? It could be a good reference to this design.

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Gerd Hoffmann]

  Hi,

> Actually I have a long puzzle in this area. Definitely libvirt will use UUID to
> mark a VM. And obviously UUID is not recorded within KVM. Then how does
> libvirt talk to KVM based on UUID? It could be a good reference to this design.

libvirt keeps track which qemu instance belongs to which vm.
qemu also gets started with "-uuid ...", so one can query qemu via
monitor ("info uuid") to figure what the uuid is.  It is also in the
smbios tables so the guest can see it in the system information table.

The uuid is not visible to the kernel though, the kvm kernel driver
doesn't know what the uuid is (and neither does vfio).  qemu uses file
handles to talk to both kvm and vfio.  qemu notifies both kvm and vfio
about anything relevant events (guest address space changes etc) and
connects file descriptors (eventfd -> irqfd).

qemu needs a sysfs node as handle to the vfio device, something
like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid if you want
have it that way, but it could be pretty much anything.  The sysfs node
will probably show up as-is in the libvirt xml when assign a vgpu to a
vm.  So the name should be something stable (i.e. when using a uuid as
name you should better not generate a new one on each boot).

cheers,
  Gerd

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Alex Williamson]

On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > Actually I have a long puzzle in this area. Definitely libvirt will use UUID to
> > mark a VM. And obviously UUID is not recorded within KVM. Then how does
> > libvirt talk to KVM based on UUID? It could be a good reference to this design.
> 
> libvirt keeps track which qemu instance belongs to which vm.
> qemu also gets started with "-uuid ...", so one can query qemu via
> monitor ("info uuid") to figure what the uuid is.  It is also in the
> smbios tables so the guest can see it in the system information table.
> 
> The uuid is not visible to the kernel though, the kvm kernel driver
> doesn't know what the uuid is (and neither does vfio).  qemu uses file
> handles to talk to both kvm and vfio.  qemu notifies both kvm and vfio
> about anything relevant events (guest address space changes etc) and
> connects file descriptors (eventfd -> irqfd).

I think the original link to using a VM UUID for the vGPU comes from
NVIDIA having a userspace component which might get launched from a udev
event as the vGPU is created or the set of vGPUs within that UUID is
started.  Using the VM UUID then gives them a way to associate that
userspace process with a VM instance.  Maybe it could register with
libvirt for some sort of service provided for the VM, I don't know.

> qemu needs a sysfs node as handle to the vfio device, something
> like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid if you want
> have it that way, but it could be pretty much anything.  The sysfs node
> will probably show up as-is in the libvirt xml when assign a vgpu to a
> vm.  So the name should be something stable (i.e. when using a uuid as
> name you should better not generate a new one on each boot).

Actually I don't think there's really a persistent naming issue, that's
probably where we diverge from the SR-IOV model.  SR-IOV cannot
dynamically add a new VF, it needs to reset the number of VFs to zero,
then re-allocate all of them up to the new desired count.  That has some
obvious implications.  I think with both vendors here, we can
dynamically allocate new vGPUs, so I would expect that libvirt would
create each vGPU instance as it's needed.  None would be created by
default without user interaction.

Personally I think using a UUID makes sense, but it needs to be
userspace policy whether that UUID has any implicit meaning like
matching the VM UUID.  Having an index within a UUID bothers me a bit,
but it doesn't seem like too much of a concession to enable the use case
that NVIDIA is trying to achieve.  Thanks,

Alex

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Alex Williamson [mailto:alex.williamson@redhat.com]
> Sent: Thursday, February 04, 2016 3:33 AM
> 
> On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
> >   Hi,
> >
> > > Actually I have a long puzzle in this area. Definitely libvirt will use UUID to
> > > mark a VM. And obviously UUID is not recorded within KVM. Then how does
> > > libvirt talk to KVM based on UUID? It could be a good reference to this design.
> >
> > libvirt keeps track which qemu instance belongs to which vm.
> > qemu also gets started with "-uuid ...", so one can query qemu via
> > monitor ("info uuid") to figure what the uuid is.  It is also in the
> > smbios tables so the guest can see it in the system information table.
> >
> > The uuid is not visible to the kernel though, the kvm kernel driver
> > doesn't know what the uuid is (and neither does vfio).  qemu uses file
> > handles to talk to both kvm and vfio.  qemu notifies both kvm and vfio
> > about anything relevant events (guest address space changes etc) and
> > connects file descriptors (eventfd -> irqfd).
> 
> I think the original link to using a VM UUID for the vGPU comes from
> NVIDIA having a userspace component which might get launched from a udev
> event as the vGPU is created or the set of vGPUs within that UUID is
> started.  Using the VM UUID then gives them a way to associate that
> userspace process with a VM instance.  Maybe it could register with
> libvirt for some sort of service provided for the VM, I don't know.

Intel doesn't have this requirement. It should be enough as long as
libvirt maintains which sysfs vgpu node is associated to a VM UUID.

> 
> > qemu needs a sysfs node as handle to the vfio device, something
> > like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid if you want
> > have it that way, but it could be pretty much anything.  The sysfs node
> > will probably show up as-is in the libvirt xml when assign a vgpu to a
> > vm.  So the name should be something stable (i.e. when using a uuid as
> > name you should better not generate a new one on each boot).
> 
> Actually I don't think there's really a persistent naming issue, that's
> probably where we diverge from the SR-IOV model.  SR-IOV cannot
> dynamically add a new VF, it needs to reset the number of VFs to zero,
> then re-allocate all of them up to the new desired count.  That has some
> obvious implications.  I think with both vendors here, we can
> dynamically allocate new vGPUs, so I would expect that libvirt would
> create each vGPU instance as it's needed.  None would be created by
> default without user interaction.
> 
> Personally I think using a UUID makes sense, but it needs to be
> userspace policy whether that UUID has any implicit meaning like
> matching the VM UUID.  Having an index within a UUID bothers me a bit,
> but it doesn't seem like too much of a concession to enable the use case
> that NVIDIA is trying to achieve.  Thanks,
> 

I would prefer to making UUID an optional parameter, while not tieing
sysfs vgpu naming to UUID. This would be more flexible to different
scenarios where UUID might not be required.

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Tue, Feb 16, 2016 at 06:49:30AM +0000, Tian, Kevin wrote:
> > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > Sent: Thursday, February 04, 2016 3:33 AM
> > 
> > On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
> > >   Hi,
> > >
> > > > Actually I have a long puzzle in this area. Definitely libvirt will use UUID to
> > > > mark a VM. And obviously UUID is not recorded within KVM. Then how does
> > > > libvirt talk to KVM based on UUID? It could be a good reference to this design.
> > >
> > > libvirt keeps track which qemu instance belongs to which vm.
> > > qemu also gets started with "-uuid ...", so one can query qemu via
> > > monitor ("info uuid") to figure what the uuid is.  It is also in the
> > > smbios tables so the guest can see it in the system information table.
> > >
> > > The uuid is not visible to the kernel though, the kvm kernel driver
> > > doesn't know what the uuid is (and neither does vfio).  qemu uses file
> > > handles to talk to both kvm and vfio.  qemu notifies both kvm and vfio
> > > about anything relevant events (guest address space changes etc) and
> > > connects file descriptors (eventfd -> irqfd).
> > 
> > I think the original link to using a VM UUID for the vGPU comes from
> > NVIDIA having a userspace component which might get launched from a udev
> > event as the vGPU is created or the set of vGPUs within that UUID is
> > started.  Using the VM UUID then gives them a way to associate that
> > userspace process with a VM instance.  Maybe it could register with
> > libvirt for some sort of service provided for the VM, I don't know.
> 
> Intel doesn't have this requirement. It should be enough as long as
> libvirt maintains which sysfs vgpu node is associated to a VM UUID.
> 
> > 
> > > qemu needs a sysfs node as handle to the vfio device, something
> > > like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid if you want
> > > have it that way, but it could be pretty much anything.  The sysfs node
> > > will probably show up as-is in the libvirt xml when assign a vgpu to a
> > > vm.  So the name should be something stable (i.e. when using a uuid as
> > > name you should better not generate a new one on each boot).
> > 
> > Actually I don't think there's really a persistent naming issue, that's
> > probably where we diverge from the SR-IOV model.  SR-IOV cannot
> > dynamically add a new VF, it needs to reset the number of VFs to zero,
> > then re-allocate all of them up to the new desired count.  That has some
> > obvious implications.  I think with both vendors here, we can
> > dynamically allocate new vGPUs, so I would expect that libvirt would
> > create each vGPU instance as it's needed.  None would be created by
> > default without user interaction.
> > 
> > Personally I think using a UUID makes sense, but it needs to be
> > userspace policy whether that UUID has any implicit meaning like
> > matching the VM UUID.  Having an index within a UUID bothers me a bit,
> > but it doesn't seem like too much of a concession to enable the use case
> > that NVIDIA is trying to achieve.  Thanks,
> > 
> 
> I would prefer to making UUID an optional parameter, while not tieing
> sysfs vgpu naming to UUID. This would be more flexible to different
> scenarios where UUID might not be required.

Hi Kevin,

Happy Chinese New Year!

I think having UUID as the vgpu device name will allow us to have an gpu vendor
agnostic solution for the upper layer software stack such as QEMU, who is
supposed to open the device.

Thanks,
Neo

> 
> Thanks
> Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia [mailto:cjia@nvidia.com]
> Sent: Tuesday, February 16, 2016 3:13 PM
> 
> On Tue, Feb 16, 2016 at 06:49:30AM +0000, Tian, Kevin wrote:
> > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > Sent: Thursday, February 04, 2016 3:33 AM
> > >
> > > On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
> > > >   Hi,
> > > >
> > > > > Actually I have a long puzzle in this area. Definitely libvirt will use UUID to
> > > > > mark a VM. And obviously UUID is not recorded within KVM. Then how does
> > > > > libvirt talk to KVM based on UUID? It could be a good reference to this design.
> > > >
> > > > libvirt keeps track which qemu instance belongs to which vm.
> > > > qemu also gets started with "-uuid ...", so one can query qemu via
> > > > monitor ("info uuid") to figure what the uuid is.  It is also in the
> > > > smbios tables so the guest can see it in the system information table.
> > > >
> > > > The uuid is not visible to the kernel though, the kvm kernel driver
> > > > doesn't know what the uuid is (and neither does vfio).  qemu uses file
> > > > handles to talk to both kvm and vfio.  qemu notifies both kvm and vfio
> > > > about anything relevant events (guest address space changes etc) and
> > > > connects file descriptors (eventfd -> irqfd).
> > >
> > > I think the original link to using a VM UUID for the vGPU comes from
> > > NVIDIA having a userspace component which might get launched from a udev
> > > event as the vGPU is created or the set of vGPUs within that UUID is
> > > started.  Using the VM UUID then gives them a way to associate that
> > > userspace process with a VM instance.  Maybe it could register with
> > > libvirt for some sort of service provided for the VM, I don't know.
> >
> > Intel doesn't have this requirement. It should be enough as long as
> > libvirt maintains which sysfs vgpu node is associated to a VM UUID.
> >
> > >
> > > > qemu needs a sysfs node as handle to the vfio device, something
> > > > like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid if you want
> > > > have it that way, but it could be pretty much anything.  The sysfs node
> > > > will probably show up as-is in the libvirt xml when assign a vgpu to a
> > > > vm.  So the name should be something stable (i.e. when using a uuid as
> > > > name you should better not generate a new one on each boot).
> > >
> > > Actually I don't think there's really a persistent naming issue, that's
> > > probably where we diverge from the SR-IOV model.  SR-IOV cannot
> > > dynamically add a new VF, it needs to reset the number of VFs to zero,
> > > then re-allocate all of them up to the new desired count.  That has some
> > > obvious implications.  I think with both vendors here, we can
> > > dynamically allocate new vGPUs, so I would expect that libvirt would
> > > create each vGPU instance as it's needed.  None would be created by
> > > default without user interaction.
> > >
> > > Personally I think using a UUID makes sense, but it needs to be
> > > userspace policy whether that UUID has any implicit meaning like
> > > matching the VM UUID.  Having an index within a UUID bothers me a bit,
> > > but it doesn't seem like too much of a concession to enable the use case
> > > that NVIDIA is trying to achieve.  Thanks,
> > >
> >
> > I would prefer to making UUID an optional parameter, while not tieing
> > sysfs vgpu naming to UUID. This would be more flexible to different
> > scenarios where UUID might not be required.
> 
> Hi Kevin,
> 
> Happy Chinese New Year!
> 
> I think having UUID as the vgpu device name will allow us to have an gpu vendor
> agnostic solution for the upper layer software stack such as QEMU, who is
> supposed to open the device.
> 

Qemu can use whatever sysfs path provided to open the device, regardless
of whether there is an UUID within the path...

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Tue, Feb 16, 2016 at 07:27:09AM +0000, Tian, Kevin wrote:
> > From: Neo Jia [mailto:cjia@nvidia.com]
> > Sent: Tuesday, February 16, 2016 3:13 PM
> > 
> > On Tue, Feb 16, 2016 at 06:49:30AM +0000, Tian, Kevin wrote:
> > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > Sent: Thursday, February 04, 2016 3:33 AM
> > > >
> > > > On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
> > > > >   Hi,
> > > > >
> > > > > > Actually I have a long puzzle in this area. Definitely libvirt will use UUID to
> > > > > > mark a VM. And obviously UUID is not recorded within KVM. Then how does
> > > > > > libvirt talk to KVM based on UUID? It could be a good reference to this design.
> > > > >
> > > > > libvirt keeps track which qemu instance belongs to which vm.
> > > > > qemu also gets started with "-uuid ...", so one can query qemu via
> > > > > monitor ("info uuid") to figure what the uuid is.  It is also in the
> > > > > smbios tables so the guest can see it in the system information table.
> > > > >
> > > > > The uuid is not visible to the kernel though, the kvm kernel driver
> > > > > doesn't know what the uuid is (and neither does vfio).  qemu uses file
> > > > > handles to talk to both kvm and vfio.  qemu notifies both kvm and vfio
> > > > > about anything relevant events (guest address space changes etc) and
> > > > > connects file descriptors (eventfd -> irqfd).
> > > >
> > > > I think the original link to using a VM UUID for the vGPU comes from
> > > > NVIDIA having a userspace component which might get launched from a udev
> > > > event as the vGPU is created or the set of vGPUs within that UUID is
> > > > started.  Using the VM UUID then gives them a way to associate that
> > > > userspace process with a VM instance.  Maybe it could register with
> > > > libvirt for some sort of service provided for the VM, I don't know.
> > >
> > > Intel doesn't have this requirement. It should be enough as long as
> > > libvirt maintains which sysfs vgpu node is associated to a VM UUID.
> > >
> > > >
> > > > > qemu needs a sysfs node as handle to the vfio device, something
> > > > > like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid if you want
> > > > > have it that way, but it could be pretty much anything.  The sysfs node
> > > > > will probably show up as-is in the libvirt xml when assign a vgpu to a
> > > > > vm.  So the name should be something stable (i.e. when using a uuid as
> > > > > name you should better not generate a new one on each boot).
> > > >
> > > > Actually I don't think there's really a persistent naming issue, that's
> > > > probably where we diverge from the SR-IOV model.  SR-IOV cannot
> > > > dynamically add a new VF, it needs to reset the number of VFs to zero,
> > > > then re-allocate all of them up to the new desired count.  That has some
> > > > obvious implications.  I think with both vendors here, we can
> > > > dynamically allocate new vGPUs, so I would expect that libvirt would
> > > > create each vGPU instance as it's needed.  None would be created by
> > > > default without user interaction.
> > > >
> > > > Personally I think using a UUID makes sense, but it needs to be
> > > > userspace policy whether that UUID has any implicit meaning like
> > > > matching the VM UUID.  Having an index within a UUID bothers me a bit,
> > > > but it doesn't seem like too much of a concession to enable the use case
> > > > that NVIDIA is trying to achieve.  Thanks,
> > > >
> > >
> > > I would prefer to making UUID an optional parameter, while not tieing
> > > sysfs vgpu naming to UUID. This would be more flexible to different
> > > scenarios where UUID might not be required.
> > 
> > Hi Kevin,
> > 
> > Happy Chinese New Year!
> > 
> > I think having UUID as the vgpu device name will allow us to have an gpu vendor
> > agnostic solution for the upper layer software stack such as QEMU, who is
> > supposed to open the device.
> > 
> 
> Qemu can use whatever sysfs path provided to open the device, regardless
> of whether there is an UUID within the path...
> 

Hi Kevin,

Then it will provide even more benefit of using UUID as libvirt can be
implemented as gpu vendor agnostic, right? :-)

The UUID can be VM UUID or vGPU group object UUID which really depends on the
high level software stack, again the benefit is gpu vendor agnostic.

Thanks,
Neo

> Thanks
> Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia [mailto:cjia@nvidia.com]
> Sent: Tuesday, February 16, 2016 3:37 PM
> 
> On Tue, Feb 16, 2016 at 07:27:09AM +0000, Tian, Kevin wrote:
> > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > Sent: Tuesday, February 16, 2016 3:13 PM
> > >
> > > On Tue, Feb 16, 2016 at 06:49:30AM +0000, Tian, Kevin wrote:
> > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > Sent: Thursday, February 04, 2016 3:33 AM
> > > > >
> > > > > On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
> > > > > >   Hi,
> > > > > >
> > > > > > > Actually I have a long puzzle in this area. Definitely libvirt will use UUID to
> > > > > > > mark a VM. And obviously UUID is not recorded within KVM. Then how does
> > > > > > > libvirt talk to KVM based on UUID? It could be a good reference to this design.
> > > > > >
> > > > > > libvirt keeps track which qemu instance belongs to which vm.
> > > > > > qemu also gets started with "-uuid ...", so one can query qemu via
> > > > > > monitor ("info uuid") to figure what the uuid is.  It is also in the
> > > > > > smbios tables so the guest can see it in the system information table.
> > > > > >
> > > > > > The uuid is not visible to the kernel though, the kvm kernel driver
> > > > > > doesn't know what the uuid is (and neither does vfio).  qemu uses file
> > > > > > handles to talk to both kvm and vfio.  qemu notifies both kvm and vfio
> > > > > > about anything relevant events (guest address space changes etc) and
> > > > > > connects file descriptors (eventfd -> irqfd).
> > > > >
> > > > > I think the original link to using a VM UUID for the vGPU comes from
> > > > > NVIDIA having a userspace component which might get launched from a udev
> > > > > event as the vGPU is created or the set of vGPUs within that UUID is
> > > > > started.  Using the VM UUID then gives them a way to associate that
> > > > > userspace process with a VM instance.  Maybe it could register with
> > > > > libvirt for some sort of service provided for the VM, I don't know.
> > > >
> > > > Intel doesn't have this requirement. It should be enough as long as
> > > > libvirt maintains which sysfs vgpu node is associated to a VM UUID.
> > > >
> > > > >
> > > > > > qemu needs a sysfs node as handle to the vfio device, something
> > > > > > like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid if you want
> > > > > > have it that way, but it could be pretty much anything.  The sysfs node
> > > > > > will probably show up as-is in the libvirt xml when assign a vgpu to a
> > > > > > vm.  So the name should be something stable (i.e. when using a uuid as
> > > > > > name you should better not generate a new one on each boot).
> > > > >
> > > > > Actually I don't think there's really a persistent naming issue, that's
> > > > > probably where we diverge from the SR-IOV model.  SR-IOV cannot
> > > > > dynamically add a new VF, it needs to reset the number of VFs to zero,
> > > > > then re-allocate all of them up to the new desired count.  That has some
> > > > > obvious implications.  I think with both vendors here, we can
> > > > > dynamically allocate new vGPUs, so I would expect that libvirt would
> > > > > create each vGPU instance as it's needed.  None would be created by
> > > > > default without user interaction.
> > > > >
> > > > > Personally I think using a UUID makes sense, but it needs to be
> > > > > userspace policy whether that UUID has any implicit meaning like
> > > > > matching the VM UUID.  Having an index within a UUID bothers me a bit,
> > > > > but it doesn't seem like too much of a concession to enable the use case
> > > > > that NVIDIA is trying to achieve.  Thanks,
> > > > >
> > > >
> > > > I would prefer to making UUID an optional parameter, while not tieing
> > > > sysfs vgpu naming to UUID. This would be more flexible to different
> > > > scenarios where UUID might not be required.
> > >
> > > Hi Kevin,
> > >
> > > Happy Chinese New Year!
> > >
> > > I think having UUID as the vgpu device name will allow us to have an gpu vendor
> > > agnostic solution for the upper layer software stack such as QEMU, who is
> > > supposed to open the device.
> > >
> >
> > Qemu can use whatever sysfs path provided to open the device, regardless
> > of whether there is an UUID within the path...
> >
> 
> Hi Kevin,
> 
> Then it will provide even more benefit of using UUID as libvirt can be
> implemented as gpu vendor agnostic, right? :-)
> 
> The UUID can be VM UUID or vGPU group object UUID which really depends on the
> high level software stack, again the benefit is gpu vendor agnostic.
> 

There is case where libvirt is not used while another mgmt. stack doesn't use
UUID, e.g. in some Xen scenarios. So it's not about GPU vendor agnostic. It's
about high level mgmt. stack agnostic. That's why we need make UUID as
optional in this vGPU-core framework.

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Tue, Feb 16, 2016 at 07:40:47AM +0000, Tian, Kevin wrote:
> > From: Neo Jia [mailto:cjia@nvidia.com]
> > Sent: Tuesday, February 16, 2016 3:37 PM
> > 
> > On Tue, Feb 16, 2016 at 07:27:09AM +0000, Tian, Kevin wrote:
> > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > Sent: Tuesday, February 16, 2016 3:13 PM
> > > >
> > > > On Tue, Feb 16, 2016 at 06:49:30AM +0000, Tian, Kevin wrote:
> > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > > Sent: Thursday, February 04, 2016 3:33 AM
> > > > > >
> > > > > > On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
> > > > > > >   Hi,
> > > > > > >
> > > > > > > > Actually I have a long puzzle in this area. Definitely libvirt will use UUID to
> > > > > > > > mark a VM. And obviously UUID is not recorded within KVM. Then how does
> > > > > > > > libvirt talk to KVM based on UUID? It could be a good reference to this design.
> > > > > > >
> > > > > > > libvirt keeps track which qemu instance belongs to which vm.
> > > > > > > qemu also gets started with "-uuid ...", so one can query qemu via
> > > > > > > monitor ("info uuid") to figure what the uuid is.  It is also in the
> > > > > > > smbios tables so the guest can see it in the system information table.
> > > > > > >
> > > > > > > The uuid is not visible to the kernel though, the kvm kernel driver
> > > > > > > doesn't know what the uuid is (and neither does vfio).  qemu uses file
> > > > > > > handles to talk to both kvm and vfio.  qemu notifies both kvm and vfio
> > > > > > > about anything relevant events (guest address space changes etc) and
> > > > > > > connects file descriptors (eventfd -> irqfd).
> > > > > >
> > > > > > I think the original link to using a VM UUID for the vGPU comes from
> > > > > > NVIDIA having a userspace component which might get launched from a udev
> > > > > > event as the vGPU is created or the set of vGPUs within that UUID is
> > > > > > started.  Using the VM UUID then gives them a way to associate that
> > > > > > userspace process with a VM instance.  Maybe it could register with
> > > > > > libvirt for some sort of service provided for the VM, I don't know.
> > > > >
> > > > > Intel doesn't have this requirement. It should be enough as long as
> > > > > libvirt maintains which sysfs vgpu node is associated to a VM UUID.
> > > > >
> > > > > >
> > > > > > > qemu needs a sysfs node as handle to the vfio device, something
> > > > > > > like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid if you want
> > > > > > > have it that way, but it could be pretty much anything.  The sysfs node
> > > > > > > will probably show up as-is in the libvirt xml when assign a vgpu to a
> > > > > > > vm.  So the name should be something stable (i.e. when using a uuid as
> > > > > > > name you should better not generate a new one on each boot).
> > > > > >
> > > > > > Actually I don't think there's really a persistent naming issue, that's
> > > > > > probably where we diverge from the SR-IOV model.  SR-IOV cannot
> > > > > > dynamically add a new VF, it needs to reset the number of VFs to zero,
> > > > > > then re-allocate all of them up to the new desired count.  That has some
> > > > > > obvious implications.  I think with both vendors here, we can
> > > > > > dynamically allocate new vGPUs, so I would expect that libvirt would
> > > > > > create each vGPU instance as it's needed.  None would be created by
> > > > > > default without user interaction.
> > > > > >
> > > > > > Personally I think using a UUID makes sense, but it needs to be
> > > > > > userspace policy whether that UUID has any implicit meaning like
> > > > > > matching the VM UUID.  Having an index within a UUID bothers me a bit,
> > > > > > but it doesn't seem like too much of a concession to enable the use case
> > > > > > that NVIDIA is trying to achieve.  Thanks,
> > > > > >
> > > > >
> > > > > I would prefer to making UUID an optional parameter, while not tieing
> > > > > sysfs vgpu naming to UUID. This would be more flexible to different
> > > > > scenarios where UUID might not be required.
> > > >
> > > > Hi Kevin,
> > > >
> > > > Happy Chinese New Year!
> > > >
> > > > I think having UUID as the vgpu device name will allow us to have an gpu vendor
> > > > agnostic solution for the upper layer software stack such as QEMU, who is
> > > > supposed to open the device.
> > > >
> > >
> > > Qemu can use whatever sysfs path provided to open the device, regardless
> > > of whether there is an UUID within the path...
> > >
> > 
> > Hi Kevin,
> > 
> > Then it will provide even more benefit of using UUID as libvirt can be
> > implemented as gpu vendor agnostic, right? :-)
> > 
> > The UUID can be VM UUID or vGPU group object UUID which really depends on the
> > high level software stack, again the benefit is gpu vendor agnostic.
> > 
> 
> There is case where libvirt is not used while another mgmt. stack doesn't use
> UUID, e.g. in some Xen scenarios. So it's not about GPU vendor agnostic. It's
> about high level mgmt. stack agnostic. That's why we need make UUID as
> optional in this vGPU-core framework.

Hi Kevin,

As long as you have to create an object to represent vGPU or vGPU group, you
will have UUID, no matter which management stack you are going to use.

UUID is the most agnostic way to represent an object, I think.

(a bit off topic since we are supposed to focus on VFIO on KVM)

Since now you are talking about Xen, I am very happy to discuss that with you.
You can check how Xen has managed its object via UUID in xapi.

Thanks,
Neo

> 
> Thanks
> Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia [mailto:cjia@nvidia.com]
> Sent: Tuesday, February 16, 2016 3:53 PM
> 
> On Tue, Feb 16, 2016 at 07:40:47AM +0000, Tian, Kevin wrote:
> > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > Sent: Tuesday, February 16, 2016 3:37 PM
> > >
> > > On Tue, Feb 16, 2016 at 07:27:09AM +0000, Tian, Kevin wrote:
> > > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > > Sent: Tuesday, February 16, 2016 3:13 PM
> > > > >
> > > > > On Tue, Feb 16, 2016 at 06:49:30AM +0000, Tian, Kevin wrote:
> > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > > > Sent: Thursday, February 04, 2016 3:33 AM
> > > > > > >
> > > > > > > On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
> > > > > > > >   Hi,
> > > > > > > >
> > > > > > > > > Actually I have a long puzzle in this area. Definitely libvirt will use UUID
> to
> > > > > > > > > mark a VM. And obviously UUID is not recorded within KVM. Then how
> does
> > > > > > > > > libvirt talk to KVM based on UUID? It could be a good reference to this
> design.
> > > > > > > >
> > > > > > > > libvirt keeps track which qemu instance belongs to which vm.
> > > > > > > > qemu also gets started with "-uuid ...", so one can query qemu via
> > > > > > > > monitor ("info uuid") to figure what the uuid is.  It is also in the
> > > > > > > > smbios tables so the guest can see it in the system information table.
> > > > > > > >
> > > > > > > > The uuid is not visible to the kernel though, the kvm kernel driver
> > > > > > > > doesn't know what the uuid is (and neither does vfio).  qemu uses file
> > > > > > > > handles to talk to both kvm and vfio.  qemu notifies both kvm and vfio
> > > > > > > > about anything relevant events (guest address space changes etc) and
> > > > > > > > connects file descriptors (eventfd -> irqfd).
> > > > > > >
> > > > > > > I think the original link to using a VM UUID for the vGPU comes from
> > > > > > > NVIDIA having a userspace component which might get launched from a udev
> > > > > > > event as the vGPU is created or the set of vGPUs within that UUID is
> > > > > > > started.  Using the VM UUID then gives them a way to associate that
> > > > > > > userspace process with a VM instance.  Maybe it could register with
> > > > > > > libvirt for some sort of service provided for the VM, I don't know.
> > > > > >
> > > > > > Intel doesn't have this requirement. It should be enough as long as
> > > > > > libvirt maintains which sysfs vgpu node is associated to a VM UUID.
> > > > > >
> > > > > > >
> > > > > > > > qemu needs a sysfs node as handle to the vfio device, something
> > > > > > > > like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid if you
> want
> > > > > > > > have it that way, but it could be pretty much anything.  The sysfs node
> > > > > > > > will probably show up as-is in the libvirt xml when assign a vgpu to a
> > > > > > > > vm.  So the name should be something stable (i.e. when using a uuid as
> > > > > > > > name you should better not generate a new one on each boot).
> > > > > > >
> > > > > > > Actually I don't think there's really a persistent naming issue, that's
> > > > > > > probably where we diverge from the SR-IOV model.  SR-IOV cannot
> > > > > > > dynamically add a new VF, it needs to reset the number of VFs to zero,
> > > > > > > then re-allocate all of them up to the new desired count.  That has some
> > > > > > > obvious implications.  I think with both vendors here, we can
> > > > > > > dynamically allocate new vGPUs, so I would expect that libvirt would
> > > > > > > create each vGPU instance as it's needed.  None would be created by
> > > > > > > default without user interaction.
> > > > > > >
> > > > > > > Personally I think using a UUID makes sense, but it needs to be
> > > > > > > userspace policy whether that UUID has any implicit meaning like
> > > > > > > matching the VM UUID.  Having an index within a UUID bothers me a bit,
> > > > > > > but it doesn't seem like too much of a concession to enable the use case
> > > > > > > that NVIDIA is trying to achieve.  Thanks,
> > > > > > >
> > > > > >
> > > > > > I would prefer to making UUID an optional parameter, while not tieing
> > > > > > sysfs vgpu naming to UUID. This would be more flexible to different
> > > > > > scenarios where UUID might not be required.
> > > > >
> > > > > Hi Kevin,
> > > > >
> > > > > Happy Chinese New Year!
> > > > >
> > > > > I think having UUID as the vgpu device name will allow us to have an gpu vendor
> > > > > agnostic solution for the upper layer software stack such as QEMU, who is
> > > > > supposed to open the device.
> > > > >
> > > >
> > > > Qemu can use whatever sysfs path provided to open the device, regardless
> > > > of whether there is an UUID within the path...
> > > >
> > >
> > > Hi Kevin,
> > >
> > > Then it will provide even more benefit of using UUID as libvirt can be
> > > implemented as gpu vendor agnostic, right? :-)
> > >
> > > The UUID can be VM UUID or vGPU group object UUID which really depends on the
> > > high level software stack, again the benefit is gpu vendor agnostic.
> > >
> >
> > There is case where libvirt is not used while another mgmt. stack doesn't use
> > UUID, e.g. in some Xen scenarios. So it's not about GPU vendor agnostic. It's
> > about high level mgmt. stack agnostic. That's why we need make UUID as
> > optional in this vGPU-core framework.
> 
> Hi Kevin,
> 
> As long as you have to create an object to represent vGPU or vGPU group, you
> will have UUID, no matter which management stack you are going to use.
> 
> UUID is the most agnostic way to represent an object, I think.
> 
> (a bit off topic since we are supposed to focus on VFIO on KVM)
> 
> Since now you are talking about Xen, I am very happy to discuss that with you.
> You can check how Xen has managed its object via UUID in xapi.
> 

Well, I'm not the expert in this area. IMHO UUID is just an user level
attribute, which can be associated to any sysfs node and managed by
mgmt. stack itself, and then the sysfs path can be opened as the
bridge between user/kernel. I don't understand the necessity of binding 
UUID internally within vGPU core framework here. Alex gave one example
of udev, but I didn't quite catch why only UUID can work there. Maybe
you can elaborate that requirement.

P.S. taking my daily Xen development experience for example, I just use 
xl w/o need to bother managing UUID (Xen hypervisor only uses VMID
instead of UUID). I don't want to eliminate such flexibility in this design. :-)

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Tue, Feb 16, 2016 at 08:10:42AM +0000, Tian, Kevin wrote:
> > From: Neo Jia [mailto:cjia@nvidia.com]
> > Sent: Tuesday, February 16, 2016 3:53 PM
> > 
> > On Tue, Feb 16, 2016 at 07:40:47AM +0000, Tian, Kevin wrote:
> > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > Sent: Tuesday, February 16, 2016 3:37 PM
> > > >
> > > > On Tue, Feb 16, 2016 at 07:27:09AM +0000, Tian, Kevin wrote:
> > > > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > > > Sent: Tuesday, February 16, 2016 3:13 PM
> > > > > >
> > > > > > On Tue, Feb 16, 2016 at 06:49:30AM +0000, Tian, Kevin wrote:
> > > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > > > > Sent: Thursday, February 04, 2016 3:33 AM
> > > > > > > >
> > > > > > > > On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
> > > > > > > > >   Hi,
> > > > > > > > >
> > > > > > > > > > Actually I have a long puzzle in this area. Definitely libvirt will use UUID
> > to
> > > > > > > > > > mark a VM. And obviously UUID is not recorded within KVM. Then how
> > does
> > > > > > > > > > libvirt talk to KVM based on UUID? It could be a good reference to this
> > design.
> > > > > > > > >
> > > > > > > > > libvirt keeps track which qemu instance belongs to which vm.
> > > > > > > > > qemu also gets started with "-uuid ...", so one can query qemu via
> > > > > > > > > monitor ("info uuid") to figure what the uuid is.  It is also in the
> > > > > > > > > smbios tables so the guest can see it in the system information table.
> > > > > > > > >
> > > > > > > > > The uuid is not visible to the kernel though, the kvm kernel driver
> > > > > > > > > doesn't know what the uuid is (and neither does vfio).  qemu uses file
> > > > > > > > > handles to talk to both kvm and vfio.  qemu notifies both kvm and vfio
> > > > > > > > > about anything relevant events (guest address space changes etc) and
> > > > > > > > > connects file descriptors (eventfd -> irqfd).
> > > > > > > >
> > > > > > > > I think the original link to using a VM UUID for the vGPU comes from
> > > > > > > > NVIDIA having a userspace component which might get launched from a udev
> > > > > > > > event as the vGPU is created or the set of vGPUs within that UUID is
> > > > > > > > started.  Using the VM UUID then gives them a way to associate that
> > > > > > > > userspace process with a VM instance.  Maybe it could register with
> > > > > > > > libvirt for some sort of service provided for the VM, I don't know.
> > > > > > >
> > > > > > > Intel doesn't have this requirement. It should be enough as long as
> > > > > > > libvirt maintains which sysfs vgpu node is associated to a VM UUID.
> > > > > > >
> > > > > > > >
> > > > > > > > > qemu needs a sysfs node as handle to the vfio device, something
> > > > > > > > > like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid if you
> > want
> > > > > > > > > have it that way, but it could be pretty much anything.  The sysfs node
> > > > > > > > > will probably show up as-is in the libvirt xml when assign a vgpu to a
> > > > > > > > > vm.  So the name should be something stable (i.e. when using a uuid as
> > > > > > > > > name you should better not generate a new one on each boot).
> > > > > > > >
> > > > > > > > Actually I don't think there's really a persistent naming issue, that's
> > > > > > > > probably where we diverge from the SR-IOV model.  SR-IOV cannot
> > > > > > > > dynamically add a new VF, it needs to reset the number of VFs to zero,
> > > > > > > > then re-allocate all of them up to the new desired count.  That has some
> > > > > > > > obvious implications.  I think with both vendors here, we can
> > > > > > > > dynamically allocate new vGPUs, so I would expect that libvirt would
> > > > > > > > create each vGPU instance as it's needed.  None would be created by
> > > > > > > > default without user interaction.
> > > > > > > >
> > > > > > > > Personally I think using a UUID makes sense, but it needs to be
> > > > > > > > userspace policy whether that UUID has any implicit meaning like
> > > > > > > > matching the VM UUID.  Having an index within a UUID bothers me a bit,
> > > > > > > > but it doesn't seem like too much of a concession to enable the use case
> > > > > > > > that NVIDIA is trying to achieve.  Thanks,
> > > > > > > >
> > > > > > >
> > > > > > > I would prefer to making UUID an optional parameter, while not tieing
> > > > > > > sysfs vgpu naming to UUID. This would be more flexible to different
> > > > > > > scenarios where UUID might not be required.
> > > > > >
> > > > > > Hi Kevin,
> > > > > >
> > > > > > Happy Chinese New Year!
> > > > > >
> > > > > > I think having UUID as the vgpu device name will allow us to have an gpu vendor
> > > > > > agnostic solution for the upper layer software stack such as QEMU, who is
> > > > > > supposed to open the device.
> > > > > >
> > > > >
> > > > > Qemu can use whatever sysfs path provided to open the device, regardless
> > > > > of whether there is an UUID within the path...
> > > > >
> > > >
> > > > Hi Kevin,
> > > >
> > > > Then it will provide even more benefit of using UUID as libvirt can be
> > > > implemented as gpu vendor agnostic, right? :-)
> > > >
> > > > The UUID can be VM UUID or vGPU group object UUID which really depends on the
> > > > high level software stack, again the benefit is gpu vendor agnostic.
> > > >
> > >
> > > There is case where libvirt is not used while another mgmt. stack doesn't use
> > > UUID, e.g. in some Xen scenarios. So it's not about GPU vendor agnostic. It's
> > > about high level mgmt. stack agnostic. That's why we need make UUID as
> > > optional in this vGPU-core framework.
> > 
> > Hi Kevin,
> > 
> > As long as you have to create an object to represent vGPU or vGPU group, you
> > will have UUID, no matter which management stack you are going to use.
> > 
> > UUID is the most agnostic way to represent an object, I think.
> > 
> > (a bit off topic since we are supposed to focus on VFIO on KVM)
> > 
> > Since now you are talking about Xen, I am very happy to discuss that with you.
> > You can check how Xen has managed its object via UUID in xapi.
> > 
> 
> Well, I'm not the expert in this area. IMHO UUID is just an user level
> attribute, which can be associated to any sysfs node and managed by
> mgmt. stack itself, and then the sysfs path can be opened as the
> bridge between user/kernel. I don't understand the necessity of binding 
> UUID internally within vGPU core framework here. Alex gave one example
> of udev, but I didn't quite catch why only UUID can work there. Maybe
> you can elaborate that requirement.

Hi Kevin,

UUID is just a way to represent an object.

It is not binding, it is just a representation. I think here we are just
creating a convenient and generic way to represent a virtual gpu device on
sysfs.

Having the UUID as part of the virtual gpu device name allows us easily find out
the <vgpu, vgpu_group or VM> mapping.

UUID can be anything, you can always use an UUID to present VMID in the example
you listed below, so you are actually gaining flexibility by using UUID instead
of VMID as it can be supported by both KVM and Xen. :-)

Thanks,
Neo

> 
> P.S. taking my daily Xen development experience for example, I just use 
> xl w/o need to bother managing UUID (Xen hypervisor only uses VMID
> instead of UUID). I don't want to eliminate such flexibility in this design. :-)
> 
> Thanks
> Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia [mailto:cjia@nvidia.com]
> Sent: Tuesday, February 16, 2016 4:49 PM
> 
> On Tue, Feb 16, 2016 at 08:10:42AM +0000, Tian, Kevin wrote:
> > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > Sent: Tuesday, February 16, 2016 3:53 PM
> > >
> > > On Tue, Feb 16, 2016 at 07:40:47AM +0000, Tian, Kevin wrote:
> > > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > > Sent: Tuesday, February 16, 2016 3:37 PM
> > > > >
> > > > > On Tue, Feb 16, 2016 at 07:27:09AM +0000, Tian, Kevin wrote:
> > > > > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > > > > Sent: Tuesday, February 16, 2016 3:13 PM
> > > > > > >
> > > > > > > On Tue, Feb 16, 2016 at 06:49:30AM +0000, Tian, Kevin wrote:
> > > > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > > > > > Sent: Thursday, February 04, 2016 3:33 AM
> > > > > > > > >
> > > > > > > > > On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
> > > > > > > > > >   Hi,
> > > > > > > > > >
> > > > > > > > > > > Actually I have a long puzzle in this area. Definitely libvirt will use
> UUID
> > > to
> > > > > > > > > > > mark a VM. And obviously UUID is not recorded within KVM. Then
> how
> > > does
> > > > > > > > > > > libvirt talk to KVM based on UUID? It could be a good reference to
> this
> > > design.
> > > > > > > > > >
> > > > > > > > > > libvirt keeps track which qemu instance belongs to which vm.
> > > > > > > > > > qemu also gets started with "-uuid ...", so one can query qemu via
> > > > > > > > > > monitor ("info uuid") to figure what the uuid is.  It is also in the
> > > > > > > > > > smbios tables so the guest can see it in the system information table.
> > > > > > > > > >
> > > > > > > > > > The uuid is not visible to the kernel though, the kvm kernel driver
> > > > > > > > > > doesn't know what the uuid is (and neither does vfio).  qemu uses
> file
> > > > > > > > > > handles to talk to both kvm and vfio.  qemu notifies both kvm and
> vfio
> > > > > > > > > > about anything relevant events (guest address space changes etc)
> and
> > > > > > > > > > connects file descriptors (eventfd -> irqfd).
> > > > > > > > >
> > > > > > > > > I think the original link to using a VM UUID for the vGPU comes from
> > > > > > > > > NVIDIA having a userspace component which might get launched from
> a udev
> > > > > > > > > event as the vGPU is created or the set of vGPUs within that UUID is
> > > > > > > > > started.  Using the VM UUID then gives them a way to associate that
> > > > > > > > > userspace process with a VM instance.  Maybe it could register with
> > > > > > > > > libvirt for some sort of service provided for the VM, I don't know.
> > > > > > > >
> > > > > > > > Intel doesn't have this requirement. It should be enough as long as
> > > > > > > > libvirt maintains which sysfs vgpu node is associated to a VM UUID.
> > > > > > > >
> > > > > > > > >
> > > > > > > > > > qemu needs a sysfs node as handle to the vfio device, something
> > > > > > > > > > like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid if
> you
> > > want
> > > > > > > > > > have it that way, but it could be pretty much anything.  The sysfs node
> > > > > > > > > > will probably show up as-is in the libvirt xml when assign a vgpu to
> a
> > > > > > > > > > vm.  So the name should be something stable (i.e. when using a uuid
> as
> > > > > > > > > > name you should better not generate a new one on each boot).
> > > > > > > > >
> > > > > > > > > Actually I don't think there's really a persistent naming issue, that's
> > > > > > > > > probably where we diverge from the SR-IOV model.  SR-IOV cannot
> > > > > > > > > dynamically add a new VF, it needs to reset the number of VFs to zero,
> > > > > > > > > then re-allocate all of them up to the new desired count.  That has some
> > > > > > > > > obvious implications.  I think with both vendors here, we can
> > > > > > > > > dynamically allocate new vGPUs, so I would expect that libvirt would
> > > > > > > > > create each vGPU instance as it's needed.  None would be created by
> > > > > > > > > default without user interaction.
> > > > > > > > >
> > > > > > > > > Personally I think using a UUID makes sense, but it needs to be
> > > > > > > > > userspace policy whether that UUID has any implicit meaning like
> > > > > > > > > matching the VM UUID.  Having an index within a UUID bothers me a
> bit,
> > > > > > > > > but it doesn't seem like too much of a concession to enable the use case
> > > > > > > > > that NVIDIA is trying to achieve.  Thanks,
> > > > > > > > >
> > > > > > > >
> > > > > > > > I would prefer to making UUID an optional parameter, while not tieing
> > > > > > > > sysfs vgpu naming to UUID. This would be more flexible to different
> > > > > > > > scenarios where UUID might not be required.
> > > > > > >
> > > > > > > Hi Kevin,
> > > > > > >
> > > > > > > Happy Chinese New Year!
> > > > > > >
> > > > > > > I think having UUID as the vgpu device name will allow us to have an gpu
> vendor
> > > > > > > agnostic solution for the upper layer software stack such as QEMU, who is
> > > > > > > supposed to open the device.
> > > > > > >
> > > > > >
> > > > > > Qemu can use whatever sysfs path provided to open the device, regardless
> > > > > > of whether there is an UUID within the path...
> > > > > >
> > > > >
> > > > > Hi Kevin,
> > > > >
> > > > > Then it will provide even more benefit of using UUID as libvirt can be
> > > > > implemented as gpu vendor agnostic, right? :-)
> > > > >
> > > > > The UUID can be VM UUID or vGPU group object UUID which really depends on
> the
> > > > > high level software stack, again the benefit is gpu vendor agnostic.
> > > > >
> > > >
> > > > There is case where libvirt is not used while another mgmt. stack doesn't use
> > > > UUID, e.g. in some Xen scenarios. So it's not about GPU vendor agnostic. It's
> > > > about high level mgmt. stack agnostic. That's why we need make UUID as
> > > > optional in this vGPU-core framework.
> > >
> > > Hi Kevin,
> > >
> > > As long as you have to create an object to represent vGPU or vGPU group, you
> > > will have UUID, no matter which management stack you are going to use.
> > >
> > > UUID is the most agnostic way to represent an object, I think.
> > >
> > > (a bit off topic since we are supposed to focus on VFIO on KVM)
> > >
> > > Since now you are talking about Xen, I am very happy to discuss that with you.
> > > You can check how Xen has managed its object via UUID in xapi.
> > >
> >
> > Well, I'm not the expert in this area. IMHO UUID is just an user level
> > attribute, which can be associated to any sysfs node and managed by
> > mgmt. stack itself, and then the sysfs path can be opened as the
> > bridge between user/kernel. I don't understand the necessity of binding
> > UUID internally within vGPU core framework here. Alex gave one example
> > of udev, but I didn't quite catch why only UUID can work there. Maybe
> > you can elaborate that requirement.
> 
> Hi Kevin,
> 
> UUID is just a way to represent an object.
> 
> It is not binding, it is just a representation. I think here we are just
> creating a convenient and generic way to represent a virtual gpu device on
> sysfs.
> 
> Having the UUID as part of the virtual gpu device name allows us easily find out
> the <vgpu, vgpu_group or VM> mapping.
> 
> UUID can be anything, you can always use an UUID to present VMID in the example
> you listed below, so you are actually gaining flexibility by using UUID instead
> of VMID as it can be supported by both KVM and Xen. :-)
> 
> Thanks,
> Neo
> 

Thanks Neo. I understand UUID has its merit in many usages. As you
may see from my earlier reply, my main concern is whether it's a must
to record this information within kernel vGPU-core framework. We can
still make it hypervisor agnostic even not using UUID, as long as there's
a unified namespace created for all vgpus, like:
	vgpu-vendor-0, vgpu-vendor-1, ...

Then high level mgmt. stack can associate UUID to that namespace. So
I hope you can help elaborate below description:

> Having the UUID as part of the virtual gpu device name allows us easily find out
> the <vgpu, vgpu_group or VM> mapping.

so we can evaluate carefully whether UUID must be included in the vgpu
device name or not. If not using UUID is OK, it can remove one more
user space dependency on using vgpu features.

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Wed, Feb 17, 2016 at 03:31:24AM +0000, Tian, Kevin wrote:
> > From: Neo Jia [mailto:cjia@nvidia.com]
> > Sent: Tuesday, February 16, 2016 4:49 PM
> > 
> > On Tue, Feb 16, 2016 at 08:10:42AM +0000, Tian, Kevin wrote:
> > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > Sent: Tuesday, February 16, 2016 3:53 PM
> > > >
> > > > On Tue, Feb 16, 2016 at 07:40:47AM +0000, Tian, Kevin wrote:
> > > > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > > > Sent: Tuesday, February 16, 2016 3:37 PM
> > > > > >
> > > > > > On Tue, Feb 16, 2016 at 07:27:09AM +0000, Tian, Kevin wrote:
> > > > > > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > > > > > Sent: Tuesday, February 16, 2016 3:13 PM
> > > > > > > >
> > > > > > > > On Tue, Feb 16, 2016 at 06:49:30AM +0000, Tian, Kevin wrote:
> > > > > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > > > > > > Sent: Thursday, February 04, 2016 3:33 AM
> > > > > > > > > >
> > > > > > > > > > On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
> > > > > > > > > > >   Hi,
> > > > > > > > > > >
> > > > > > > > > > > > Actually I have a long puzzle in this area. Definitely libvirt will use
> > UUID
> > > > to
> > > > > > > > > > > > mark a VM. And obviously UUID is not recorded within KVM. Then
> > how
> > > > does
> > > > > > > > > > > > libvirt talk to KVM based on UUID? It could be a good reference to
> > this
> > > > design.
> > > > > > > > > > >
> > > > > > > > > > > libvirt keeps track which qemu instance belongs to which vm.
> > > > > > > > > > > qemu also gets started with "-uuid ...", so one can query qemu via
> > > > > > > > > > > monitor ("info uuid") to figure what the uuid is.  It is also in the
> > > > > > > > > > > smbios tables so the guest can see it in the system information table.
> > > > > > > > > > >
> > > > > > > > > > > The uuid is not visible to the kernel though, the kvm kernel driver
> > > > > > > > > > > doesn't know what the uuid is (and neither does vfio).  qemu uses
> > file
> > > > > > > > > > > handles to talk to both kvm and vfio.  qemu notifies both kvm and
> > vfio
> > > > > > > > > > > about anything relevant events (guest address space changes etc)
> > and
> > > > > > > > > > > connects file descriptors (eventfd -> irqfd).
> > > > > > > > > >
> > > > > > > > > > I think the original link to using a VM UUID for the vGPU comes from
> > > > > > > > > > NVIDIA having a userspace component which might get launched from
> > a udev
> > > > > > > > > > event as the vGPU is created or the set of vGPUs within that UUID is
> > > > > > > > > > started.  Using the VM UUID then gives them a way to associate that
> > > > > > > > > > userspace process with a VM instance.  Maybe it could register with
> > > > > > > > > > libvirt for some sort of service provided for the VM, I don't know.
> > > > > > > > >
> > > > > > > > > Intel doesn't have this requirement. It should be enough as long as
> > > > > > > > > libvirt maintains which sysfs vgpu node is associated to a VM UUID.
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > > qemu needs a sysfs node as handle to the vfio device, something
> > > > > > > > > > > like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid if
> > you
> > > > want
> > > > > > > > > > > have it that way, but it could be pretty much anything.  The sysfs node
> > > > > > > > > > > will probably show up as-is in the libvirt xml when assign a vgpu to
> > a
> > > > > > > > > > > vm.  So the name should be something stable (i.e. when using a uuid
> > as
> > > > > > > > > > > name you should better not generate a new one on each boot).
> > > > > > > > > >
> > > > > > > > > > Actually I don't think there's really a persistent naming issue, that's
> > > > > > > > > > probably where we diverge from the SR-IOV model.  SR-IOV cannot
> > > > > > > > > > dynamically add a new VF, it needs to reset the number of VFs to zero,
> > > > > > > > > > then re-allocate all of them up to the new desired count.  That has some
> > > > > > > > > > obvious implications.  I think with both vendors here, we can
> > > > > > > > > > dynamically allocate new vGPUs, so I would expect that libvirt would
> > > > > > > > > > create each vGPU instance as it's needed.  None would be created by
> > > > > > > > > > default without user interaction.
> > > > > > > > > >
> > > > > > > > > > Personally I think using a UUID makes sense, but it needs to be
> > > > > > > > > > userspace policy whether that UUID has any implicit meaning like
> > > > > > > > > > matching the VM UUID.  Having an index within a UUID bothers me a
> > bit,
> > > > > > > > > > but it doesn't seem like too much of a concession to enable the use case
> > > > > > > > > > that NVIDIA is trying to achieve.  Thanks,
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > I would prefer to making UUID an optional parameter, while not tieing
> > > > > > > > > sysfs vgpu naming to UUID. This would be more flexible to different
> > > > > > > > > scenarios where UUID might not be required.
> > > > > > > >
> > > > > > > > Hi Kevin,
> > > > > > > >
> > > > > > > > Happy Chinese New Year!
> > > > > > > >
> > > > > > > > I think having UUID as the vgpu device name will allow us to have an gpu
> > vendor
> > > > > > > > agnostic solution for the upper layer software stack such as QEMU, who is
> > > > > > > > supposed to open the device.
> > > > > > > >
> > > > > > >
> > > > > > > Qemu can use whatever sysfs path provided to open the device, regardless
> > > > > > > of whether there is an UUID within the path...
> > > > > > >
> > > > > >
> > > > > > Hi Kevin,
> > > > > >
> > > > > > Then it will provide even more benefit of using UUID as libvirt can be
> > > > > > implemented as gpu vendor agnostic, right? :-)
> > > > > >
> > > > > > The UUID can be VM UUID or vGPU group object UUID which really depends on
> > the
> > > > > > high level software stack, again the benefit is gpu vendor agnostic.
> > > > > >
> > > > >
> > > > > There is case where libvirt is not used while another mgmt. stack doesn't use
> > > > > UUID, e.g. in some Xen scenarios. So it's not about GPU vendor agnostic. It's
> > > > > about high level mgmt. stack agnostic. That's why we need make UUID as
> > > > > optional in this vGPU-core framework.
> > > >
> > > > Hi Kevin,
> > > >
> > > > As long as you have to create an object to represent vGPU or vGPU group, you
> > > > will have UUID, no matter which management stack you are going to use.
> > > >
> > > > UUID is the most agnostic way to represent an object, I think.
> > > >
> > > > (a bit off topic since we are supposed to focus on VFIO on KVM)
> > > >
> > > > Since now you are talking about Xen, I am very happy to discuss that with you.
> > > > You can check how Xen has managed its object via UUID in xapi.
> > > >
> > >
> > > Well, I'm not the expert in this area. IMHO UUID is just an user level
> > > attribute, which can be associated to any sysfs node and managed by
> > > mgmt. stack itself, and then the sysfs path can be opened as the
> > > bridge between user/kernel. I don't understand the necessity of binding
> > > UUID internally within vGPU core framework here. Alex gave one example
> > > of udev, but I didn't quite catch why only UUID can work there. Maybe
> > > you can elaborate that requirement.
> > 
> > Hi Kevin,
> > 
> > UUID is just a way to represent an object.
> > 
> > It is not binding, it is just a representation. I think here we are just
> > creating a convenient and generic way to represent a virtual gpu device on
> > sysfs.
> > 
> > Having the UUID as part of the virtual gpu device name allows us easily find out
> > the <vgpu, vgpu_group or VM> mapping.
> > 
> > UUID can be anything, you can always use an UUID to present VMID in the example
> > you listed below, so you are actually gaining flexibility by using UUID instead
> > of VMID as it can be supported by both KVM and Xen. :-)
> > 
> > Thanks,
> > Neo
> > 
> 
> Thanks Neo. I understand UUID has its merit in many usages. As you
> may see from my earlier reply, my main concern is whether it's a must
> to record this information within kernel vGPU-core framework. We can
> still make it hypervisor agnostic even not using UUID, as long as there's
> a unified namespace created for all vgpus, like:
> 	vgpu-vendor-0, vgpu-vendor-1, ...
> 
> Then high level mgmt. stack can associate UUID to that namespace. So
> I hope you can help elaborate below description:
> 

> > Having the UUID as part of the virtual gpu device name allows us easily find out
> > the <vgpu, vgpu_group or VM> mapping.
> 

Hi Kevin,

The answer is simple, having a UUID as part of the device name will give you a
unique sysfs path that will be opened by QEMU.

vgpu-vendor-0 and vgpu-vendor-1 will not be unique as we can have multiple
virtual gpu devices per VM coming from same or different physical devices.

If you are worried about losing meaningful name here, we can create a sysfs file
to capture the vendor device description if you like.

Thanks,
Neo

> so we can evaluate carefully whether UUID must be included in the vgpu
> device name or not. If not using UUID is OK, it can remove one more
> user space dependency on using vgpu features.
> 
> Thanks
> Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia
> Sent: Wednesday, February 17, 2016 12:18 PM
> 
> On Wed, Feb 17, 2016 at 03:31:24AM +0000, Tian, Kevin wrote:
> > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > Sent: Tuesday, February 16, 2016 4:49 PM
> > >
> > > On Tue, Feb 16, 2016 at 08:10:42AM +0000, Tian, Kevin wrote:
> > > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > > Sent: Tuesday, February 16, 2016 3:53 PM
> > > > >
> > > > > On Tue, Feb 16, 2016 at 07:40:47AM +0000, Tian, Kevin wrote:
> > > > > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > > > > Sent: Tuesday, February 16, 2016 3:37 PM
> > > > > > >
> > > > > > > On Tue, Feb 16, 2016 at 07:27:09AM +0000, Tian, Kevin wrote:
> > > > > > > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > > > > > > Sent: Tuesday, February 16, 2016 3:13 PM
> > > > > > > > >
> > > > > > > > > On Tue, Feb 16, 2016 at 06:49:30AM +0000, Tian, Kevin wrote:
> > > > > > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > > > > > > > Sent: Thursday, February 04, 2016 3:33 AM
> > > > > > > > > > >
> > > > > > > > > > > On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
> > > > > > > > > > > >   Hi,
> > > > > > > > > > > >
> > > > > > > > > > > > > Actually I have a long puzzle in this area. Definitely libvirt will
> use
> > > UUID
> > > > > to
> > > > > > > > > > > > > mark a VM. And obviously UUID is not recorded within KVM.
> Then
> > > how
> > > > > does
> > > > > > > > > > > > > libvirt talk to KVM based on UUID? It could be a good reference
> to
> > > this
> > > > > design.
> > > > > > > > > > > >
> > > > > > > > > > > > libvirt keeps track which qemu instance belongs to which vm.
> > > > > > > > > > > > qemu also gets started with "-uuid ...", so one can query qemu
> via
> > > > > > > > > > > > monitor ("info uuid") to figure what the uuid is.  It is also in the
> > > > > > > > > > > > smbios tables so the guest can see it in the system information
> table.
> > > > > > > > > > > >
> > > > > > > > > > > > The uuid is not visible to the kernel though, the kvm kernel driver
> > > > > > > > > > > > doesn't know what the uuid is (and neither does vfio).  qemu uses
> > > file
> > > > > > > > > > > > handles to talk to both kvm and vfio.  qemu notifies both kvm
> and
> > > vfio
> > > > > > > > > > > > about anything relevant events (guest address space changes
> etc)
> > > and
> > > > > > > > > > > > connects file descriptors (eventfd -> irqfd).
> > > > > > > > > > >
> > > > > > > > > > > I think the original link to using a VM UUID for the vGPU comes from
> > > > > > > > > > > NVIDIA having a userspace component which might get launched
> from
> > > a udev
> > > > > > > > > > > event as the vGPU is created or the set of vGPUs within that UUID
> is
> > > > > > > > > > > started.  Using the VM UUID then gives them a way to associate
> that
> > > > > > > > > > > userspace process with a VM instance.  Maybe it could register with
> > > > > > > > > > > libvirt for some sort of service provided for the VM, I don't know.
> > > > > > > > > >
> > > > > > > > > > Intel doesn't have this requirement. It should be enough as long as
> > > > > > > > > > libvirt maintains which sysfs vgpu node is associated to a VM UUID.
> > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > > qemu needs a sysfs node as handle to the vfio device, something
> > > > > > > > > > > > like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid
> if
> > > you
> > > > > want
> > > > > > > > > > > > have it that way, but it could be pretty much anything.  The sysfs
> node
> > > > > > > > > > > > will probably show up as-is in the libvirt xml when assign a vgpu
> to
> > > a
> > > > > > > > > > > > vm.  So the name should be something stable (i.e. when using
> a uuid
> > > as
> > > > > > > > > > > > name you should better not generate a new one on each boot).
> > > > > > > > > > >
> > > > > > > > > > > Actually I don't think there's really a persistent naming issue, that's
> > > > > > > > > > > probably where we diverge from the SR-IOV model.  SR-IOV cannot
> > > > > > > > > > > dynamically add a new VF, it needs to reset the number of VFs to
> zero,
> > > > > > > > > > > then re-allocate all of them up to the new desired count.  That has
> some
> > > > > > > > > > > obvious implications.  I think with both vendors here, we can
> > > > > > > > > > > dynamically allocate new vGPUs, so I would expect that libvirt would
> > > > > > > > > > > create each vGPU instance as it's needed.  None would be created
> by
> > > > > > > > > > > default without user interaction.
> > > > > > > > > > >
> > > > > > > > > > > Personally I think using a UUID makes sense, but it needs to be
> > > > > > > > > > > userspace policy whether that UUID has any implicit meaning like
> > > > > > > > > > > matching the VM UUID.  Having an index within a UUID bothers me
> a
> > > bit,
> > > > > > > > > > > but it doesn't seem like too much of a concession to enable the use
> case
> > > > > > > > > > > that NVIDIA is trying to achieve.  Thanks,
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > I would prefer to making UUID an optional parameter, while not tieing
> > > > > > > > > > sysfs vgpu naming to UUID. This would be more flexible to different
> > > > > > > > > > scenarios where UUID might not be required.
> > > > > > > > >
> > > > > > > > > Hi Kevin,
> > > > > > > > >
> > > > > > > > > Happy Chinese New Year!
> > > > > > > > >
> > > > > > > > > I think having UUID as the vgpu device name will allow us to have an
> gpu
> > > vendor
> > > > > > > > > agnostic solution for the upper layer software stack such as QEMU, who
> is
> > > > > > > > > supposed to open the device.
> > > > > > > > >
> > > > > > > >
> > > > > > > > Qemu can use whatever sysfs path provided to open the device, regardless
> > > > > > > > of whether there is an UUID within the path...
> > > > > > > >
> > > > > > >
> > > > > > > Hi Kevin,
> > > > > > >
> > > > > > > Then it will provide even more benefit of using UUID as libvirt can be
> > > > > > > implemented as gpu vendor agnostic, right? :-)
> > > > > > >
> > > > > > > The UUID can be VM UUID or vGPU group object UUID which really depends
> on
> > > the
> > > > > > > high level software stack, again the benefit is gpu vendor agnostic.
> > > > > > >
> > > > > >
> > > > > > There is case where libvirt is not used while another mgmt. stack doesn't use
> > > > > > UUID, e.g. in some Xen scenarios. So it's not about GPU vendor agnostic. It's
> > > > > > about high level mgmt. stack agnostic. That's why we need make UUID as
> > > > > > optional in this vGPU-core framework.
> > > > >
> > > > > Hi Kevin,
> > > > >
> > > > > As long as you have to create an object to represent vGPU or vGPU group, you
> > > > > will have UUID, no matter which management stack you are going to use.
> > > > >
> > > > > UUID is the most agnostic way to represent an object, I think.
> > > > >
> > > > > (a bit off topic since we are supposed to focus on VFIO on KVM)
> > > > >
> > > > > Since now you are talking about Xen, I am very happy to discuss that with you.
> > > > > You can check how Xen has managed its object via UUID in xapi.
> > > > >
> > > >
> > > > Well, I'm not the expert in this area. IMHO UUID is just an user level
> > > > attribute, which can be associated to any sysfs node and managed by
> > > > mgmt. stack itself, and then the sysfs path can be opened as the
> > > > bridge between user/kernel. I don't understand the necessity of binding
> > > > UUID internally within vGPU core framework here. Alex gave one example
> > > > of udev, but I didn't quite catch why only UUID can work there. Maybe
> > > > you can elaborate that requirement.
> > >
> > > Hi Kevin,
> > >
> > > UUID is just a way to represent an object.
> > >
> > > It is not binding, it is just a representation. I think here we are just
> > > creating a convenient and generic way to represent a virtual gpu device on
> > > sysfs.
> > >
> > > Having the UUID as part of the virtual gpu device name allows us easily find out
> > > the <vgpu, vgpu_group or VM> mapping.
> > >
> > > UUID can be anything, you can always use an UUID to present VMID in the example
> > > you listed below, so you are actually gaining flexibility by using UUID instead
> > > of VMID as it can be supported by both KVM and Xen. :-)
> > >
> > > Thanks,
> > > Neo
> > >
> >
> > Thanks Neo. I understand UUID has its merit in many usages. As you
> > may see from my earlier reply, my main concern is whether it's a must
> > to record this information within kernel vGPU-core framework. We can
> > still make it hypervisor agnostic even not using UUID, as long as there's
> > a unified namespace created for all vgpus, like:
> > 	vgpu-vendor-0, vgpu-vendor-1, ...
> >
> > Then high level mgmt. stack can associate UUID to that namespace. So
> > I hope you can help elaborate below description:
> >
> 
> > > Having the UUID as part of the virtual gpu device name allows us easily find out
> > > the <vgpu, vgpu_group or VM> mapping.
> >
> 
> Hi Kevin,
> 
> The answer is simple, having a UUID as part of the device name will give you a
> unique sysfs path that will be opened by QEMU.
> 
> vgpu-vendor-0 and vgpu-vendor-1 will not be unique as we can have multiple
> virtual gpu devices per VM coming from same or different physical devices.

That is not a problem. We can add physical device info too like vgpu-vendor-0-0,
vgpu-vendor-1-0, ...

Please note Qemu doesn't care about the actual name. It just accepts a sysfs path
to open.

> 
> If you are worried about losing meaningful name here, we can create a sysfs file
> to capture the vendor device description if you like.
> 

Having the vgpu name descriptive is more informative imo. User can simply check
sysfs names to know raw information w/o relying on 3rd party agent to query 
information around an opaque UUID.

That's why I prefer to making UUID optional. By default vgpu name would be
some description string, and when UUID is provided, UUID can be appended
to the string to serve your purpose.

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1200 bytes --]

On 02/16/2016 10:04 PM, Tian, Kevin wrote:
>> From: Neo Jia

[meta-comment]

...

>>>>>>>>>>>> On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
>>>>>>>>>>>>>   Hi,
>>>>>>>>>>>>>
>>>>>>>>>>>>>> Actually I have a long puzzle in this area. Definitely libvirt will
>> use
>>>> UUID
>>>>>> to

With this much quoting, your mailer is breaking down.  It's not only
okay, but encouraged, to trim your message to just what you are directly
replying to...

>> Hi Kevin,
>>
>> The answer is simple, having a UUID as part of the device name will give you a
>> unique sysfs path that will be opened by QEMU.
>>
>> vgpu-vendor-0 and vgpu-vendor-1 will not be unique as we can have multiple
>> virtual gpu devices per VM coming from same or different physical devices.
> 
> That is not a problem. We can add physical device info too like vgpu-vendor-0-0,
> vgpu-vendor-1-0, ...

...rather than making readers scroll through 16k bytes of repetitions of
the same things they saw earlier in the thread, but getting worse with
each iteration due to excessive quoting.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Tue, Feb 16, 2016 at 10:09:43PM -0700, Eric Blake wrote:
> * PGP Signed by an unknown key
> 
> On 02/16/2016 10:04 PM, Tian, Kevin wrote:
> 
> 
> ...rather than making readers scroll through 16k bytes of repetitions of
> the same things they saw earlier in the thread, but getting worse with
> each iteration due to excessive quoting.
> 

Hi Eric,

Sorry about that, I will pay attention to this.

Thanks,
Neo

> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 
> 
> * Unknown Key
> * 0x2527436A

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Wed, Feb 17, 2016 at 05:04:31AM +0000, Tian, Kevin wrote:
> > From: Neo Jia
> > Sent: Wednesday, February 17, 2016 12:18 PM
> > 
> > On Wed, Feb 17, 2016 at 03:31:24AM +0000, Tian, Kevin wrote:
> > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > Sent: Tuesday, February 16, 2016 4:49 PM
> > > >
> > > > On Tue, Feb 16, 2016 at 08:10:42AM +0000, Tian, Kevin wrote:
> > > > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > > > Sent: Tuesday, February 16, 2016 3:53 PM
> > > > > >
> > > > > > On Tue, Feb 16, 2016 at 07:40:47AM +0000, Tian, Kevin wrote:
> > > > > > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > > > > > Sent: Tuesday, February 16, 2016 3:37 PM
> > > > > > > >
> > > > > > > > On Tue, Feb 16, 2016 at 07:27:09AM +0000, Tian, Kevin wrote:
> > > > > > > > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > > > > > > > Sent: Tuesday, February 16, 2016 3:13 PM
> > > > > > > > > >
> > > > > > > > > > On Tue, Feb 16, 2016 at 06:49:30AM +0000, Tian, Kevin wrote:
> > > > > > > > > > > > From: Alex Williamson [mailto:alex.williamson@redhat.com]
> > > > > > > > > > > > Sent: Thursday, February 04, 2016 3:33 AM
> > > > > > > > > > > >
> > > > > > > > > > > > On Wed, 2016-02-03 at 09:28 +0100, Gerd Hoffmann wrote:
> > > > > > > > > > > > >   Hi,
> > > > > > > > > > > > >
> > > > > > > > > > > > > > Actually I have a long puzzle in this area. Definitely libvirt will
> > use
> > > > UUID
> > > > > > to
> > > > > > > > > > > > > > mark a VM. And obviously UUID is not recorded within KVM.
> > Then
> > > > how
> > > > > > does
> > > > > > > > > > > > > > libvirt talk to KVM based on UUID? It could be a good reference
> > to
> > > > this
> > > > > > design.
> > > > > > > > > > > > >
> > > > > > > > > > > > > libvirt keeps track which qemu instance belongs to which vm.
> > > > > > > > > > > > > qemu also gets started with "-uuid ...", so one can query qemu
> > via
> > > > > > > > > > > > > monitor ("info uuid") to figure what the uuid is.  It is also in the
> > > > > > > > > > > > > smbios tables so the guest can see it in the system information
> > table.
> > > > > > > > > > > > >
> > > > > > > > > > > > > The uuid is not visible to the kernel though, the kvm kernel driver
> > > > > > > > > > > > > doesn't know what the uuid is (and neither does vfio).  qemu uses
> > > > file
> > > > > > > > > > > > > handles to talk to both kvm and vfio.  qemu notifies both kvm
> > and
> > > > vfio
> > > > > > > > > > > > > about anything relevant events (guest address space changes
> > etc)
> > > > and
> > > > > > > > > > > > > connects file descriptors (eventfd -> irqfd).
> > > > > > > > > > > >
> > > > > > > > > > > > I think the original link to using a VM UUID for the vGPU comes from
> > > > > > > > > > > > NVIDIA having a userspace component which might get launched
> > from
> > > > a udev
> > > > > > > > > > > > event as the vGPU is created or the set of vGPUs within that UUID
> > is
> > > > > > > > > > > > started.  Using the VM UUID then gives them a way to associate
> > that
> > > > > > > > > > > > userspace process with a VM instance.  Maybe it could register with
> > > > > > > > > > > > libvirt for some sort of service provided for the VM, I don't know.
> > > > > > > > > > >
> > > > > > > > > > > Intel doesn't have this requirement. It should be enough as long as
> > > > > > > > > > > libvirt maintains which sysfs vgpu node is associated to a VM UUID.
> > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > > qemu needs a sysfs node as handle to the vfio device, something
> > > > > > > > > > > > > like /sys/devices/virtual/vgpu/<name>.  <name> can be a uuid
> > if
> > > > you
> > > > > > want
> > > > > > > > > > > > > have it that way, but it could be pretty much anything.  The sysfs
> > node
> > > > > > > > > > > > > will probably show up as-is in the libvirt xml when assign a vgpu
> > to
> > > > a
> > > > > > > > > > > > > vm.  So the name should be something stable (i.e. when using
> > a uuid
> > > > as
> > > > > > > > > > > > > name you should better not generate a new one on each boot).
> > > > > > > > > > > >
> > > > > > > > > > > > Actually I don't think there's really a persistent naming issue, that's
> > > > > > > > > > > > probably where we diverge from the SR-IOV model.  SR-IOV cannot
> > > > > > > > > > > > dynamically add a new VF, it needs to reset the number of VFs to
> > zero,
> > > > > > > > > > > > then re-allocate all of them up to the new desired count.  That has
> > some
> > > > > > > > > > > > obvious implications.  I think with both vendors here, we can
> > > > > > > > > > > > dynamically allocate new vGPUs, so I would expect that libvirt would
> > > > > > > > > > > > create each vGPU instance as it's needed.  None would be created
> > by
> > > > > > > > > > > > default without user interaction.
> > > > > > > > > > > >
> > > > > > > > > > > > Personally I think using a UUID makes sense, but it needs to be
> > > > > > > > > > > > userspace policy whether that UUID has any implicit meaning like
> > > > > > > > > > > > matching the VM UUID.  Having an index within a UUID bothers me
> > a
> > > > bit,
> > > > > > > > > > > > but it doesn't seem like too much of a concession to enable the use
> > case
> > > > > > > > > > > > that NVIDIA is trying to achieve.  Thanks,
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > I would prefer to making UUID an optional parameter, while not tieing
> > > > > > > > > > > sysfs vgpu naming to UUID. This would be more flexible to different
> > > > > > > > > > > scenarios where UUID might not be required.
> > > > > > > > > >
> > > > > > > > > > Hi Kevin,
> > > > > > > > > >
> > > > > > > > > > Happy Chinese New Year!
> > > > > > > > > >
> > > > > > > > > > I think having UUID as the vgpu device name will allow us to have an
> > gpu
> > > > vendor
> > > > > > > > > > agnostic solution for the upper layer software stack such as QEMU, who
> > is
> > > > > > > > > > supposed to open the device.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Qemu can use whatever sysfs path provided to open the device, regardless
> > > > > > > > > of whether there is an UUID within the path...
> > > > > > > > >
> > > > > > > >
> > > > > > > > Hi Kevin,
> > > > > > > >
> > > > > > > > Then it will provide even more benefit of using UUID as libvirt can be
> > > > > > > > implemented as gpu vendor agnostic, right? :-)
> > > > > > > >
> > > > > > > > The UUID can be VM UUID or vGPU group object UUID which really depends
> > on
> > > > the
> > > > > > > > high level software stack, again the benefit is gpu vendor agnostic.
> > > > > > > >
> > > > > > >
> > > > > > > There is case where libvirt is not used while another mgmt. stack doesn't use
> > > > > > > UUID, e.g. in some Xen scenarios. So it's not about GPU vendor agnostic. It's
> > > > > > > about high level mgmt. stack agnostic. That's why we need make UUID as
> > > > > > > optional in this vGPU-core framework.
> > > > > >
> > > > > > Hi Kevin,
> > > > > >
> > > > > > As long as you have to create an object to represent vGPU or vGPU group, you
> > > > > > will have UUID, no matter which management stack you are going to use.
> > > > > >
> > > > > > UUID is the most agnostic way to represent an object, I think.
> > > > > >
> > > > > > (a bit off topic since we are supposed to focus on VFIO on KVM)
> > > > > >
> > > > > > Since now you are talking about Xen, I am very happy to discuss that with you.
> > > > > > You can check how Xen has managed its object via UUID in xapi.
> > > > > >
> > > > >
> > > > > Well, I'm not the expert in this area. IMHO UUID is just an user level
> > > > > attribute, which can be associated to any sysfs node and managed by
> > > > > mgmt. stack itself, and then the sysfs path can be opened as the
> > > > > bridge between user/kernel. I don't understand the necessity of binding
> > > > > UUID internally within vGPU core framework here. Alex gave one example
> > > > > of udev, but I didn't quite catch why only UUID can work there. Maybe
> > > > > you can elaborate that requirement.
> > > >
> > > > Hi Kevin,
> > > >
> > > > UUID is just a way to represent an object.
> > > >
> > > > It is not binding, it is just a representation. I think here we are just
> > > > creating a convenient and generic way to represent a virtual gpu device on
> > > > sysfs.
> > > >
> > > > Having the UUID as part of the virtual gpu device name allows us easily find out
> > > > the <vgpu, vgpu_group or VM> mapping.
> > > >
> > > > UUID can be anything, you can always use an UUID to present VMID in the example
> > > > you listed below, so you are actually gaining flexibility by using UUID instead
> > > > of VMID as it can be supported by both KVM and Xen. :-)
> > > >
> > > > Thanks,
> > > > Neo
> > > >
> > >
> > > Thanks Neo. I understand UUID has its merit in many usages. As you
> > > may see from my earlier reply, my main concern is whether it's a must
> > > to record this information within kernel vGPU-core framework. We can
> > > still make it hypervisor agnostic even not using UUID, as long as there's
> > > a unified namespace created for all vgpus, like:
> > > 	vgpu-vendor-0, vgpu-vendor-1, ...
> > >
> > > Then high level mgmt. stack can associate UUID to that namespace. So
> > > I hope you can help elaborate below description:
> > >
> > 
> > > > Having the UUID as part of the virtual gpu device name allows us easily find out
> > > > the <vgpu, vgpu_group or VM> mapping.
> > >
> > 
> > Hi Kevin,
> > 
> > The answer is simple, having a UUID as part of the device name will give you a
> > unique sysfs path that will be opened by QEMU.
> > 
> > vgpu-vendor-0 and vgpu-vendor-1 will not be unique as we can have multiple
> > virtual gpu devices per VM coming from same or different physical devices.
> 
> That is not a problem. We can add physical device info too like vgpu-vendor-0-0,
> vgpu-vendor-1-0, ...
> 
> Please note Qemu doesn't care about the actual name. It just accepts a sysfs path
> to open.

Hi Kevin,

No, I think you are making things even more complicated than it is required,
also it is not generic anymore as you are requiring the QEMU to know more than
he needs to.

The way you name those devices will require QEMU to know the relation
between virtual devices and physical devices. I don't think that is good.

My flow is like this:

libvirt creats a VM object, it will have a UUID. then it will use the UUID to
create virtual gpu devices, then it will pass the UUID to the QEMU (actually
QEMU already has the VM UUID), then it will just open up the unique path.

Also, you need to consider those 0-0 numbers are not generic as the UUID.
> 
> > 
> > If you are worried about losing meaningful name here, we can create a sysfs file
> > to capture the vendor device description if you like.
> > 
> 
> Having the vgpu name descriptive is more informative imo. User can simply check
> sysfs names to know raw information w/o relying on 3rd party agent to query 
> information around an opaque UUID.
> 

You are actually arguing against your own design here, unfortunately. If you
look at your design carefully, it is your design actually require to have a 3rd
party code to figure out the VM and virtual gpu device relation as it is
never documented in the sysfs. 

In our current design, it doesn't require any 3rd party agent as the VM UUID is
part of the QEMU command already, and the VM UUID is already embedded within the
virtual device path.

Also, it doesn't require 3rd party to retrieve information as the virtual device
will just be a directory, we will have another file within each virtual gpu
device directory, you can always cat the file to retrieve vendor information.

Let's use the UUID-$vgpu_idx as the virtual device directory name plus a vendor
description file within that directory, so we don't lose any additional
information, also capture the VM and virtual device relation.

Thanks,
Neo

> That's why I prefer to making UUID optional. By default vgpu name would be
> some description string, and when UUID is provided, UUID can be appended
> to the string to serve your purpose.
> 
> Thanks
> Kevin
> 

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia
> Sent: Wednesday, February 17, 2016 1:38 PM
> > > >
> > > >
> > >
> > > Hi Kevin,
> > >
> > > The answer is simple, having a UUID as part of the device name will give you a
> > > unique sysfs path that will be opened by QEMU.
> > >
> > > vgpu-vendor-0 and vgpu-vendor-1 will not be unique as we can have multiple
> > > virtual gpu devices per VM coming from same or different physical devices.
> >
> > That is not a problem. We can add physical device info too like vgpu-vendor-0-0,
> > vgpu-vendor-1-0, ...
> >
> > Please note Qemu doesn't care about the actual name. It just accepts a sysfs path
> > to open.
> 
> Hi Kevin,
> 
> No, I think you are making things even more complicated than it is required,
> also it is not generic anymore as you are requiring the QEMU to know more than
> he needs to.
> 
> The way you name those devices will require QEMU to know the relation
> between virtual devices and physical devices. I don't think that is good.

I don't think you get my point. Look at how device is assigned in Qemu today:

-device vfio-pci,host=02:00.0

Then in a recent patch from Alex, Qemu will accept sysfsdev as well:

-device vfio-pci,sysfsdev=/sys/devices/pci0000:00/0000:00:1c.0/0000:02:00.0

Then with vgu (one example from Alex's original post):

-device vfio-pci,sysfsdev=/sys/devices/virtual/intel-vgpu/vgpu0@0000:00:02.0

Qemu doesn't need to know the relation between virtual/physical devices at
all. It's just a path regardless of how vgpu name is created (either with your
UUID proposal or my descriptive string proposal)

> 
> My flow is like this:
> 
> libvirt creats a VM object, it will have a UUID. then it will use the UUID to
> create virtual gpu devices, then it will pass the UUID to the QEMU (actually
> QEMU already has the VM UUID), then it will just open up the unique path.

If you look at above example, it's not UUID itself being passed to Qemu. It's
the sysfsdev path.

> 
> Also, you need to consider those 0-0 numbers are not generic as the UUID.

that encoding could be flexible to include any meaningful string. libvirt can
itself manages how UUID is mapped to an actual vgpu name.

> >
> > >
> > > If you are worried about losing meaningful name here, we can create a sysfs file
> > > to capture the vendor device description if you like.
> > >
> >
> > Having the vgpu name descriptive is more informative imo. User can simply check
> > sysfs names to know raw information w/o relying on 3rd party agent to query
> > information around an opaque UUID.
> >
> 
> You are actually arguing against your own design here, unfortunately. If you
> look at your design carefully, it is your design actually require to have a 3rd
> party code to figure out the VM and virtual gpu device relation as it is
> never documented in the sysfs.

No. It's not about figuring out the relation between VM and vGPU. It's about
figuring out some raw information about vGPU itself, such as:

vgpu0@0000:00:02.0, I can immediately know it's the 1st instance created on
0000:00:02.0 device. While with an UUID, it doesn't speak anything useful here.

This is a typical way how device nodes are created within sysfs, e.g. on my
platform:

$ ls /sys/class/drm/
card0/          card0-DP-2/     card0-HDMI-A-2/ controlD64/
card0-DP-1/     card0-HDMI-A-1/ card0-VGA-1/    version

$ ls /sys/bus/pci/devices/
0000:00:00.0  0000:00:14.0  0000:00:1a.0  0000:00:1c.1  0000:00:1f.2
0000:00:02.0  0000:00:16.0  0000:00:1b.0  0000:00:1d.0  0000:00:1f.3
0000:00:03.0  0000:00:19.0  0000:00:1c.0  0000:00:1f.0  0000:02:00.0

We'd better keep such style when creating vgpu nodes in sysfs. UUID is
at most anther info suffixed to the default string (or in another file), if 
necessary.

> 
> In our current design, it doesn't require any 3rd party agent as the VM UUID is
> part of the QEMU command already, and the VM UUID is already embedded within the
> virtual device path.
> 
> Also, it doesn't require 3rd party to retrieve information as the virtual device
> will just be a directory, we will have another file within each virtual gpu
> device directory, you can always cat the file to retrieve vendor information.
> 
> Let's use the UUID-$vgpu_idx as the virtual device directory name plus a vendor
> description file within that directory, so we don't lose any additional
> information, also capture the VM and virtual device relation.
> 

I'm fine to have another node to provide more vendor specific information. But
I don't want to make UUID mandatory when creating a vGPU instance, as 
explained above. Today we can create VMs in KVM w/o using libvirt, and w/o 
the need of allocating any UUID. Same thing should be supported for vgpu
feature too.

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Wed, Feb 17, 2016 at 06:02:36AM +0000, Tian, Kevin wrote:
> > From: Neo Jia
> > Sent: Wednesday, February 17, 2016 1:38 PM
> > > > >
> > > > >
> > > >
> > > > Hi Kevin,
> > > >
> > > > The answer is simple, having a UUID as part of the device name will give you a
> > > > unique sysfs path that will be opened by QEMU.
> > > >
> > > > vgpu-vendor-0 and vgpu-vendor-1 will not be unique as we can have multiple
> > > > virtual gpu devices per VM coming from same or different physical devices.
> > >
> > > That is not a problem. We can add physical device info too like vgpu-vendor-0-0,
> > > vgpu-vendor-1-0, ...
> > >
> > > Please note Qemu doesn't care about the actual name. It just accepts a sysfs path
> > > to open.
> > 
> > Hi Kevin,
> > 
> > No, I think you are making things even more complicated than it is required,
> > also it is not generic anymore as you are requiring the QEMU to know more than
> > he needs to.
> > 
> > The way you name those devices will require QEMU to know the relation
> > between virtual devices and physical devices. I don't think that is good.
> 
> I don't think you get my point. Look at how device is assigned in Qemu today:
> 
> -device vfio-pci,host=02:00.0
> 
> Then in a recent patch from Alex, Qemu will accept sysfsdev as well:
> 
> -device vfio-pci,sysfsdev=/sys/devices/pci0000:00/0000:00:1c.0/0000:02:00.0
> 
> Then with vgu (one example from Alex's original post):
> 
> -device vfio-pci,sysfsdev=/sys/devices/virtual/intel-vgpu/vgpu0@0000:00:02.0

Hi Kevin,

I am fully aware of Alex's patch, that is just an example, but he doesn't
exclude the cases of using UUID as the device path as he mentioned in the
beginning of this long email thread, actually he has already agreed with this
UUID-$vgpu_idx path. :-)

Also, you should note that with the proposal we have, it doesn't require to have
anything like either intel-vgpu or nvidia-vgpu as the path, having a non-vendor
specific information within the vgpu device path is one of the requirement IIRC.

-device vfio-pci,sysfsdev=/sys/devices/virtual/intel-vgpu/vgpu0@0000:00:02.0

> 
> Qemu doesn't need to know the relation between virtual/physical devices at
> all. It's just a path regardless of how vgpu name is created (either with your
> UUID proposal or my descriptive string proposal)

No, with path like above, QEMU needs to know the virtual device is created from
that physical device 0000:00:02.0 right? (you have mentioned this yourself
actually below.) If QEMU doesn't want to know that, then he will transfer the
burden to the upper layer stack such as libvirt, who has to figure out the right
path of this new VM, as the vgpu<$id>, the <%id> will become another generic
number generated by the vgpu core driver. 

So why not just use UUID here?

> 
> > 
> > My flow is like this:
> > 
> > libvirt creats a VM object, it will have a UUID. then it will use the UUID to
> > create virtual gpu devices, then it will pass the UUID to the QEMU (actually
> > QEMU already has the VM UUID), then it will just open up the unique path.
> 
> If you look at above example, it's not UUID itself being passed to Qemu. It's
> the sysfsdev path.

UUID is always sent to QEMU, if you look at your QEMU command line, please.

Yes, it is a sysfs path, even with UUID, it is a sysfs path, the only difference
is that we have a uuid embedded within the device name.

> 
> > 
> > Also, you need to consider those 0-0 numbers are not generic as the UUID.
> 
> that encoding could be flexible to include any meaningful string. libvirt can
> itself manages how UUID is mapped to an actual vgpu name.
> 
> > >
> > > >
> > > > If you are worried about losing meaningful name here, we can create a sysfs file
> > > > to capture the vendor device description if you like.
> > > >
> > >
> > > Having the vgpu name descriptive is more informative imo. User can simply check
> > > sysfs names to know raw information w/o relying on 3rd party agent to query
> > > information around an opaque UUID.
> > >
> > 
> > You are actually arguing against your own design here, unfortunately. If you
> > look at your design carefully, it is your design actually require to have a 3rd
> > party code to figure out the VM and virtual gpu device relation as it is
> > never documented in the sysfs.
> 
> No. It's not about figuring out the relation between VM and vGPU. It's about
> figuring out some raw information about vGPU itself, such as:
> 
> vgpu0@0000:00:02.0, I can immediately know it's the 1st instance created on
> 0000:00:02.0 device. While with an UUID, it doesn't speak anything useful here.

Right, I get your point, but this requires the qemu to know this virtual device
is created on that physical device, isn't it, given you already know that the
virtual device is created on the physical device 0000:00:02.0, right?

> 
> This is a typical way how device nodes are created within sysfs, e.g. on my
> platform:
> 
> $ ls /sys/class/drm/
> card0/          card0-DP-2/     card0-HDMI-A-2/ controlD64/
> card0-DP-1/     card0-HDMI-A-1/ card0-VGA-1/    version
> 
> $ ls /sys/bus/pci/devices/
> 0000:00:00.0  0000:00:14.0  0000:00:1a.0  0000:00:1c.1  0000:00:1f.2
> 0000:00:02.0  0000:00:16.0  0000:00:1b.0  0000:00:1d.0  0000:00:1f.3
> 0000:00:03.0  0000:00:19.0  0000:00:1c.0  0000:00:1f.0  0000:02:00.0
> 
> We'd better keep such style when creating vgpu nodes in sysfs. UUID is
> at most anther info suffixed to the default string (or in another file), if 
> necessary.

It doesn't apply here, your above example are all physical devices.

The reason I want to have UUID is it to match the instance of problem domain
here - VM.

> 
> > 
> > In our current design, it doesn't require any 3rd party agent as the VM UUID is
> > part of the QEMU command already, and the VM UUID is already embedded within the
> > virtual device path.
> > 
> > Also, it doesn't require 3rd party to retrieve information as the virtual device
> > will just be a directory, we will have another file within each virtual gpu
> > device directory, you can always cat the file to retrieve vendor information.
> > 
> > Let's use the UUID-$vgpu_idx as the virtual device directory name plus a vendor
> > description file within that directory, so we don't lose any additional
> > information, also capture the VM and virtual device relation.
> > 
> 
> I'm fine to have another node to provide more vendor specific information. But
> I don't want to make UUID mandatory when creating a vGPU instance, as 
> explained above. Today we can create VMs in KVM w/o using libvirt, and w/o 
> the need of allocating any UUID. Same thing should be supported for vgpu
> feature too.

So, I really don't see any drawback of using UUID as part of the virtual device
directory, it is easy, simple and organically reflecting the relation between
virtual device and the owner.

Each QEMU process is representing a VM, and a UUID is associated with it. The
virtual gpu device is just a virtual device of this owner, so the path is:

-device vfio-pci,sysfsdev=/sys/devices/virtual/vgpu/$UUID-$vgpu_idx/

You can have multiple virtual device per VM and QEMU doesn't need to know which
physical device it comes from, especially it will automatically know which
virtual device it owns, so the -device vfio-pci path will be setup for free.

If your most concern is having this kind of path doesn't provide enough
information of the virtual device, we can add more sysfs attributes within the
directory of /sys/devices/virtual/vgpu/$UUID-$vgpu_idx/ to reflect the
information you want.

Even with UUID, you don't need libvirt at all. you can get uuid by running
uuidgen command, I don't need libvirt to code up and test the RFC that I have
sent out early. :-)

Thanks,
Neo

> 
> Thanks
> Kevin
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia
> Sent: Wednesday, February 17, 2016 3:26 PM
> 
> 
> >
> > Qemu doesn't need to know the relation between virtual/physical devices at
> > all. It's just a path regardless of how vgpu name is created (either with your
> > UUID proposal or my descriptive string proposal)
> 
> No, with path like above, QEMU needs to know the virtual device is created from
> that physical device 0000:00:02.0 right? (you have mentioned this yourself
> actually below.) If QEMU doesn't want to know that, then he will transfer the

I didn't say anything that Qemu needs to know it. I said "I can immediately know
..." which means a human being. :-)

Qemu only needs a path to open.


[...]
> 
> >
> > This is a typical way how device nodes are created within sysfs, e.g. on my
> > platform:
> >
> > $ ls /sys/class/drm/
> > card0/          card0-DP-2/     card0-HDMI-A-2/ controlD64/
> > card0-DP-1/     card0-HDMI-A-1/ card0-VGA-1/    version
> >
> > $ ls /sys/bus/pci/devices/
> > 0000:00:00.0  0000:00:14.0  0000:00:1a.0  0000:00:1c.1  0000:00:1f.2
> > 0000:00:02.0  0000:00:16.0  0000:00:1b.0  0000:00:1d.0  0000:00:1f.3
> > 0000:00:03.0  0000:00:19.0  0000:00:1c.0  0000:00:1f.0  0000:02:00.0
> >
> > We'd better keep such style when creating vgpu nodes in sysfs. UUID is
> > at most anther info suffixed to the default string (or in another file), if
> > necessary.
> 
> It doesn't apply here, your above example are all physical devices.
> 
> The reason I want to have UUID is it to match the instance of problem domain
> here - VM.

It doesn't matter whether it's physical or virtual. Sysfs includes many nodes
not physically existing. 

The point that I don't understand, is why you insist the only way to associate
vgpu to a VM is by encoding UUID in vgpu name. libvirt maintains many 
attributes (including other virtual devices) for a given VM, in its internal 
database. It's not a problem to reverse find a VM according to a general vgpu 
name.

[...]
> >
> > I'm fine to have another node to provide more vendor specific information. But
> > I don't want to make UUID mandatory when creating a vGPU instance, as
> > explained above. Today we can create VMs in KVM w/o using libvirt, and w/o
> > the need of allocating any UUID. Same thing should be supported for vgpu
> > feature too.
> 
> So, I really don't see any drawback of using UUID as part of the virtual device
> directory, it is easy, simple and organically reflecting the relation between
> virtual device and the owner.

"organically reflecting" only when other database is included (as libvirt is involved),
while losing the merit which a descriptive name can bring.

> 
> Each QEMU process is representing a VM, and a UUID is associated with it. The
> virtual gpu device is just a virtual device of this owner, so the path is:
> 
> -device vfio-pci,sysfsdev=/sys/devices/virtual/vgpu/$UUID-$vgpu_idx/
> 
> You can have multiple virtual device per VM and QEMU doesn't need to know which
> physical device it comes from, especially it will automatically know which
> virtual device it owns, so the -device vfio-pci path will be setup for free.

As I commented earlier, Qemu never needs to know that information
regardless of how the vgpu is named.

> 
> If your most concern is having this kind of path doesn't provide enough
> information of the virtual device, we can add more sysfs attributes within the
> directory of /sys/devices/virtual/vgpu/$UUID-$vgpu_idx/ to reflect the
> information you want.

Like Gerd said, you can have something like this:

-device vfio-pci,sysfsdev=/sys/devices/virtual/vgpu/vgpu_idx/UUID

> 
> Even with UUID, you don't need libvirt at all. you can get uuid by running
> uuidgen command, I don't need libvirt to code up and test the RFC that I have
> sent out early. :-)

although simple, it still creates unnecessary user space dependency for
kernel resource management...

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Wed, Feb 17, 2016 at 07:46:15AM +0000, Tian, Kevin wrote:
> > From: Neo Jia
> > Sent: Wednesday, February 17, 2016 3:26 PM
> > 
> > 
> 
> > 
> > If your most concern is having this kind of path doesn't provide enough
> > information of the virtual device, we can add more sysfs attributes within the
> > directory of /sys/devices/virtual/vgpu/$UUID-$vgpu_idx/ to reflect the
> > information you want.
> 
> Like Gerd said, you can have something like this:
> 
> -device vfio-pci,sysfsdev=/sys/devices/virtual/vgpu/vgpu_idx/UUID

Hi Kevin,

The vgpu_idx is not unique number at all.

For example, how to locate the path of a given VM? Whoever is going to configure
the qemu has to walk through *all* the current vgpu path to locate the UUID to
match the QEMU's VM UUID. This is not required if you have UUID as part of the
device path.

> 
> > 
> > Even with UUID, you don't need libvirt at all. you can get uuid by running
> > uuidgen command, I don't need libvirt to code up and test the RFC that I have
> > sent out early. :-)
> 
> although simple, it still creates unnecessary user space dependency for
> kernel resource management...

I think I has answered this, UUID is not a user space or kernel space
concept, it is just a generic way to represent object, it just make sure that
virtual gpu device directory can be uniquely addressed.

Thanks,
Neo

> 
> Thanks
> Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia [mailto:cjia@nvidia.com]
> Sent: Wednesday, February 17, 2016 3:55 PM
> 
> On Wed, Feb 17, 2016 at 07:46:15AM +0000, Tian, Kevin wrote:
> > > From: Neo Jia
> > > Sent: Wednesday, February 17, 2016 3:26 PM
> > >
> > >
> >
> > >
> > > If your most concern is having this kind of path doesn't provide enough
> > > information of the virtual device, we can add more sysfs attributes within the
> > > directory of /sys/devices/virtual/vgpu/$UUID-$vgpu_idx/ to reflect the
> > > information you want.
> >
> > Like Gerd said, you can have something like this:
> >
> > -device vfio-pci,sysfsdev=/sys/devices/virtual/vgpu/vgpu_idx/UUID
> 
> Hi Kevin,
> 
> The vgpu_idx is not unique number at all.
> 
> For example, how to locate the path of a given VM? Whoever is going to configure
> the qemu has to walk through *all* the current vgpu path to locate the UUID to
> match the QEMU's VM UUID. This is not required if you have UUID as part of the
> device path.

'whoever' is too strict here. I don't think UUID is required in all scenarios.

In your scenario:

- You will pass VM UUID when creating a vgpu.
- Consequently a /sys/device/virtual/vgpu/$UUID-$vgpu-id is created
- Then you can identify $UUID-$vgpu-id is right for the very VM, by matching 
all available vgpu nodes with VM UUID;

When it is a bit convenient, I don't see it significant. Looping directory is
not unusual for file/directory operations and it happens infrequently only
for vgpu life-cycle mgmt..

Please think about my original proposal carefully. I'm not opposing encoding
UUID in vgpu name. What I'm opposing is not to make it mandatory, i.e. when
UUID is not provided, we should still allow vgpu creation using some default
descriptive string.

> 
> >
> > >
> > > Even with UUID, you don't need libvirt at all. you can get uuid by running
> > > uuidgen command, I don't need libvirt to code up and test the RFC that I have
> > > sent out early. :-)
> >
> > although simple, it still creates unnecessary user space dependency for
> > kernel resource management...
> 
> I think I has answered this, UUID is not a user space or kernel space
> concept, it is just a generic way to represent object, it just make sure that
> virtual gpu device directory can be uniquely addressed.
> 

'user space dependency' means you need additional user-space operations
(say uuidgen here) before you can utilize GPU virtualization feature, which
is not necessary. In reality, UUID is not a GPU resource. It's not what GPU 
virtualization intrinsically needs to handle. Let's keep vGPU-core sub-system
modulo for its real functionalities.

So let's keep UUID as an optional parameter. When UUID is provided, it
will be included in the vGPU name then your requirement can be met.

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Wed, Feb 17, 2016 at 08:57:08AM +0000, Tian, Kevin wrote:
> > From: Neo Jia [mailto:cjia@nvidia.com]
> > Sent: Wednesday, February 17, 2016 3:55 PM
> 
> 'whoever' is too strict here. I don't think UUID is required in all scenarios.
> 
> In your scenario:
> 
> - You will pass VM UUID when creating a vgpu.
> - Consequently a /sys/device/virtual/vgpu/$UUID-$vgpu-id is created
> - Then you can identify $UUID-$vgpu-id is right for the very VM, by matching 
> all available vgpu nodes with VM UUID;
> 
> When it is a bit convenient, I don't see it significant. Looping directory is
> not unusual for file/directory operations and it happens infrequently only
> for vgpu life-cycle mgmt..

Hi Kevin,

The search is expensive, when you have 8 physical gpus and each can support up
to 32 vgpus. vgpu life-cycle management happens a lot as well in real production
scenario.

If we can make it free, why not?

> 
> Please think about my original proposal carefully. I'm not opposing encoding
> UUID in vgpu name. What I'm opposing is not to make it mandatory, i.e. when
> UUID is not provided, we should still allow vgpu creation using some default
> descriptive string.

Probably you are not quite get the generic design that we are proposing here.

The goal here is to have a unified interface for all gpu vendor, and expose that
to the upper layer software stack, so I don't think we should have an optional
vgpu device discovery path at all. 

If we have an optional case, does that mean libvirt will have a different
implementation and qemu will have a different implementation? I don't think that
is acceptable.

Since you have admitted this design is convenient and performance better, I think we 
should stay with it. 

> 
> 'user space dependency' means you need additional user-space operations
> (say uuidgen here) before you can utilize GPU virtualization feature, which
> is not necessary. In reality, UUID is not a GPU resource. It's not what GPU 
> virtualization intrinsically needs to handle. Let's keep vGPU-core sub-system
> modulo for its real functionalities.

Don't you need to create UUID to make qemu happy? I don't get this argument.

Please also note that using UUID to represent a virtual gpu device directory
doesn't mean UUID is part of a GPU resource.

> 
> So let's keep UUID as an optional parameter. When UUID is provided, it
> will be included in the vGPU name then your requirement can be met.
> 

Like I have said before, we are seeking a generic interface to allow upper layer
software stack to manage vgpu device for different vendors, so we should not really
consider "an optional path for vgpu device discovery" at all.

This is why I think we should use this UUID as a generic management interface,
and we shouldn't have anything optional.

Thanks,
Neo

> Thanks
> Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia [mailto:cjia@nvidia.com]
> Sent: Wednesday, February 17, 2016 5:35 PM
> 
> On Wed, Feb 17, 2016 at 08:57:08AM +0000, Tian, Kevin wrote:
> > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > Sent: Wednesday, February 17, 2016 3:55 PM
> >
> > 'whoever' is too strict here. I don't think UUID is required in all scenarios.
> >
> > In your scenario:
> >
> > - You will pass VM UUID when creating a vgpu.
> > - Consequently a /sys/device/virtual/vgpu/$UUID-$vgpu-id is created
> > - Then you can identify $UUID-$vgpu-id is right for the very VM, by matching
> > all available vgpu nodes with VM UUID;
> >
> > When it is a bit convenient, I don't see it significant. Looping directory is
> > not unusual for file/directory operations and it happens infrequently only
> > for vgpu life-cycle mgmt..
> 
> Hi Kevin,
> 
> The search is expensive, when you have 8 physical gpus and each can support up
> to 32 vgpus. vgpu life-cycle management happens a lot as well in real production
> scenario.
> 
> If we can make it free, why not?

I can buy-in this argument.

> 
> >
> > Please think about my original proposal carefully. I'm not opposing encoding
> > UUID in vgpu name. What I'm opposing is not to make it mandatory, i.e. when
> > UUID is not provided, we should still allow vgpu creation using some default
> > descriptive string.
> 
> Probably you are not quite get the generic design that we are proposing here.
> 
> The goal here is to have a unified interface for all gpu vendor, and expose that
> to the upper layer software stack, so I don't think we should have an optional
> vgpu device discovery path at all.
> 
> If we have an optional case, does that mean libvirt will have a different
> implementation and qemu will have a different implementation? I don't think that
> is acceptable.

libvirt can choose to always provide UUID, then there's no problem. Another
stack can choose to not use UUID. It doesn't mean each stack needs to support
different implementation. Just from kernel p.o.v we need sustain such 
flexibility.

Qemu will always have the same implementation. I explained this multiple
times. Qemu only cares about a sysfsdev path parameter. It doesn't
matter whether it has a UUID included or not.

> 
> Since you have admitted this design is convenient and performance better, I think we
> should stay with it.
> 
> >
> > 'user space dependency' means you need additional user-space operations
> > (say uuidgen here) before you can utilize GPU virtualization feature, which
> > is not necessary. In reality, UUID is not a GPU resource. It's not what GPU
> > virtualization intrinsically needs to handle. Let's keep vGPU-core sub-system
> > modulo for its real functionalities.
> 
> Don't you need to create UUID to make qemu happy? I don't get this argument.

Qemu is not a kernel component. And UUID is OPTIONAL for Qemu.

KVM is the kernel component. It doesn't use UUID at all. the relation between
UUID and VM is fully maintained in user space.

> 
> Please also note that using UUID to represent a virtual gpu device directory
> doesn't mean UUID is part of a GPU resource.

but it adds a hard dependency on another resource - UUID. 

> 
> >
> > So let's keep UUID as an optional parameter. When UUID is provided, it
> > will be included in the vGPU name then your requirement can be met.
> >
> 
> Like I have said before, we are seeking a generic interface to allow upper layer
> software stack to manage vgpu device for different vendors, so we should not really
> consider "an optional path for vgpu device discovery" at all.
> 
> This is why I think we should use this UUID as a generic management interface,
> and we shouldn't have anything optional.
> 

I don't buy-in this argument. I always think kernel design should provide 
enough flexibility, instead of assuming user space behavior.

Let me also add some Citrix friends. See how they feel about the necessity of
having UUID in vgpu name.

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Wed, Feb 17, 2016 at 09:52:04AM +0000, Tian, Kevin wrote:
> > From: Neo Jia [mailto:cjia@nvidia.com]
> > Sent: Wednesday, February 17, 2016 5:35 PM
> > 
> > On Wed, Feb 17, 2016 at 08:57:08AM +0000, Tian, Kevin wrote:
> > > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > > Sent: Wednesday, February 17, 2016 3:55 PM
> > >
> > If we can make it free, why not?
> 
> I can buy-in this argument.

Great!

> 
> Qemu is not a kernel component. And UUID is OPTIONAL for Qemu.
> 
> KVM is the kernel component. It doesn't use UUID at all. the relation between
> UUID and VM is fully maintained in user space.

Hold on ... we are talking about the vgpu.ko not KVM right?

UUID is just a generic way to represent an object, here we use a uuid to
represent a virtual gpu device.

> 
> > 
> > Please also note that using UUID to represent a virtual gpu device directory
> > doesn't mean UUID is part of a GPU resource.
> 
> but it adds a hard dependency on another resource - UUID. 
> 
> > 
> > >
> > > So let's keep UUID as an optional parameter. When UUID is provided, it
> > > will be included in the vGPU name then your requirement can be met.
> > >
> > 
> > Like I have said before, we are seeking a generic interface to allow upper layer
> > software stack to manage vgpu device for different vendors, so we should not really
> > consider "an optional path for vgpu device discovery" at all.
> > 
> > This is why I think we should use this UUID as a generic management interface,
> > and we shouldn't have anything optional.
> > 
> 
> I don't buy-in this argument. I always think kernel design should provide 
> enough flexibility, instead of assuming user space behavior.
> 

I think you are using the wrong terms here. Flexibility doesn't apply here. What
we are trying to achieve here is to have a generic interface for upper layer
software to manage vgpu device. 

> Let me also add some Citrix friends. See how they feel about the necessity of
> having UUID in vgpu name.

Sorry?

Thanks,
Neo

> 
> Thanks
> Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia [mailto:cjia@nvidia.com]
> Sent: Wednesday, February 17, 2016 6:34 PM
> 
> >
> > Qemu is not a kernel component. And UUID is OPTIONAL for Qemu.
> >
> > KVM is the kernel component. It doesn't use UUID at all. the relation between
> > UUID and VM is fully maintained in user space.
> 
> Hold on ... we are talking about the vgpu.ko not KVM right?

You said UUID is a general way to represent an object. VM is an object.
There is UUID associated with a VM. But UUID is not recorded in the
kernel KVM. It's a good reference to our design on vgpu object. If
one thing can be done in user space, then better not involve kernel side.

> > >
> > > This is why I think we should use this UUID as a generic management interface,
> > > and we shouldn't have anything optional.
> > >
> >
> > I don't buy-in this argument. I always think kernel design should provide
> > enough flexibility, instead of assuming user space behavior.
> >
> 
> I think you are using the wrong terms here. Flexibility doesn't apply here. What
> we are trying to achieve here is to have a generic interface for upper layer
> software to manage vgpu device.

Even Qemu doesn't make UUID as mandatory. It's just an optional parameter.
I'm not sure why you insist UUID mandatory for vGPU here.

> 
> > Let me also add some Citrix friends. See how they feel about the necessity of
> > having UUID in vgpu name.
> 
> Sorry?

Paul/Malcolm are experts in Citrix XenServer. So I think they may give some
suggestions from their angle. :-)

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Gerd Hoffmann]

  Hi,

> For example, how to locate the path of a given VM?

You go ask libvirt, the domain xml will have the info.

> Whoever is going to configure
> the qemu has to walk through *all* the current vgpu path to locate the UUID to
> match the QEMU's VM UUID.

No.  qemu simply uses the path it get passed from libvirt.  libvirt
simply uses whatever is stored in the domain xml.

i.e. you'll have a config like this:

<domain type='kvm'> 
  <name>rhel7-vfio</name>
  <uuid>0990b05d-4fbd-49bf-88e4-e87974c64fba</uuid>
  [ ... ]
  <devices>
    [ ... ]
    <hostdev mode='subsystem' type='pci' managed='yes'>
      <source>
        <address sysfs='/sys/devices/virtual/vgpu/${name}'/>
      </source>
      <address type='pci' domain='0x0000'
               bus='0x00' slot='0x04' function='0x0'/>
    </hostdev>

> I think I has answered this, UUID is not a user space or kernel space
> concept, it is just a generic way to represent object,

Yes.  But the above sounds like you want to use UUIDs to *link* two
objects, by assigning the same uuid to both vm and vgpu.  This is *not*
how uuids should be used.  Each object should have its own uuid.

You can use uuids to name the vgpus if you want of course.  But the vgpu
uuid will will have no relationship whatsoever to the vm uuid then.

cheers,
  Gerd

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Wed, Feb 17, 2016 at 02:08:18PM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > For example, how to locate the path of a given VM?
> 
> You go ask libvirt, the domain xml will have the info.
> 
> > Whoever is going to configure
> > the qemu has to walk through *all* the current vgpu path to locate the UUID to
> > match the QEMU's VM UUID.
> 
> No.  qemu simply uses the path it get passed from libvirt.  libvirt
> simply uses whatever is stored in the domain xml.
> 
> i.e. you'll have a config like this:
> 
> <domain type='kvm'> 
>   <name>rhel7-vfio</name>
>   <uuid>0990b05d-4fbd-49bf-88e4-e87974c64fba</uuid>
>   [ ... ]
>   <devices>
>     [ ... ]
>     <hostdev mode='subsystem' type='pci' managed='yes'>
>       <source>
>         <address sysfs='/sys/devices/virtual/vgpu/${name}'/>
>       </source>
>       <address type='pci' domain='0x0000'
>                bus='0x00' slot='0x04' function='0x0'/>
>     </hostdev>
> 
> > I think I has answered this, UUID is not a user space or kernel space
> > concept, it is just a generic way to represent object,
> 
> Yes.  But the above sounds like you want to use UUIDs to *link* two
> objects, by assigning the same uuid to both vm and vgpu.  This is *not*
> how uuids should be used.  Each object should have its own uuid.
> 
> You can use uuids to name the vgpus if you want of course.  But the vgpu
> uuid will will have no relationship whatsoever to the vm uuid then.
> 

Agree. I should made it clear that it should be a separate object.

Thanks,
Neo

> cheers,
>   Gerd
> 

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Gerd Hoffmann]

  Hi,

> The answer is simple, having a UUID as part of the device name will give you a
> unique sysfs path that will be opened by QEMU.

A descriptive name will work too, and I think it'll be easier to make
those names persistent because you don't have to store the uuids
somewhere to re-create the same setup afer reboot.

> If you are worried about losing meaningful name here, we can create a sysfs file
> to capture the vendor device description if you like.

You can also store the uuid in a sysfs file ...

cheers,
  Gerd

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Wed, Feb 17, 2016 at 07:52:53AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > The answer is simple, having a UUID as part of the device name will give you a
> > unique sysfs path that will be opened by QEMU.
> 
> A descriptive name will work too, and I think it'll be easier to make
> those names persistent because you don't have to store the uuids
> somewhere to re-create the same setup afer reboot.

Hi Gerd,

Right, UUID will be persistent cross reboot. The qemu vgpu path for a given VM will 
not get changed when it gets reboots and multiple other devices have been
created in the middle.

> 
> > If you are worried about losing meaningful name here, we can create a sysfs file
> > to capture the vendor device description if you like.
> 
> You can also store the uuid in a sysfs file ...

Another benefit is that having the UUID as part of the virtual vgpu device path will
allow whoever is going to config the QEMU to automatically discover the virtual
device sysfs for free.

Thanks,
Neo

> 
> cheers,
>   Gerd
> 

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia [mailto:cjia@nvidia.com]
> Sent: Wednesday, February 17, 2016 3:32 PM
> 
> On Wed, Feb 17, 2016 at 07:52:53AM +0100, Gerd Hoffmann wrote:
> >   Hi,
> >
> > > The answer is simple, having a UUID as part of the device name will give you a
> > > unique sysfs path that will be opened by QEMU.
> >
> > A descriptive name will work too, and I think it'll be easier to make
> > those names persistent because you don't have to store the uuids
> > somewhere to re-create the same setup afer reboot.
> 
> Hi Gerd,
> 
> Right, UUID will be persistent cross reboot. The qemu vgpu path for a given VM will
> not get changed when it gets reboots and multiple other devices have been
> created in the middle.

Curious why persistence matters here. It's completely OK to assign 
another vgpu to this VM across reboot, as long as the new vgpu
provides same capability to previous one.

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Neo Jia]

On Wed, Feb 17, 2016 at 07:51:12AM +0000, Tian, Kevin wrote:
> > From: Neo Jia [mailto:cjia@nvidia.com]
> > Sent: Wednesday, February 17, 2016 3:32 PM
> > 
> > On Wed, Feb 17, 2016 at 07:52:53AM +0100, Gerd Hoffmann wrote:
> > >   Hi,
> > >
> > > > The answer is simple, having a UUID as part of the device name will give you a
> > > > unique sysfs path that will be opened by QEMU.
> > >
> > > A descriptive name will work too, and I think it'll be easier to make
> > > those names persistent because you don't have to store the uuids
> > > somewhere to re-create the same setup afer reboot.
> > 
> > Hi Gerd,
> > 
> > Right, UUID will be persistent cross reboot. The qemu vgpu path for a given VM will
> > not get changed when it gets reboots and multiple other devices have been
> > created in the middle.
> 
> Curious why persistence matters here. It's completely OK to assign 
> another vgpu to this VM across reboot, as long as the new vgpu
> provides same capability to previous one.

Hi Kevin,

Those virtual devices might be destroyed and re-created as part of the reboot or
shutdown. The user doesn't want to change his configuration, if the
path is not associated with UUID, the user might get a different vgpu than his
previous configuration, unpredictable.

We can't change the vgpu configuration behind uses back, especially we have a
lot of physical devices and tons virtual gpus can be assigned.

Also, if user wants to move the vm configuration to a different host, it is more
nature to decouple the vgpu configuration from VM itself. So the VM will only
open up the device addressed by the corresponding UUID, what you need is just to
describe his vgpu, and it doesn't matter how you describe and doesn't matter
how many virtual devices have been created already on that physical device, your
QEMU path is always persistent.

Thanks,
Neo

> 
> Thanks
> Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Neo Jia [mailto:cjia@nvidia.com]
> Sent: Wednesday, February 17, 2016 4:41 PM
> 
> On Wed, Feb 17, 2016 at 07:51:12AM +0000, Tian, Kevin wrote:
> > > From: Neo Jia [mailto:cjia@nvidia.com]
> > > Sent: Wednesday, February 17, 2016 3:32 PM
> > >
> > > On Wed, Feb 17, 2016 at 07:52:53AM +0100, Gerd Hoffmann wrote:
> > > >   Hi,
> > > >
> > > > > The answer is simple, having a UUID as part of the device name will give you a
> > > > > unique sysfs path that will be opened by QEMU.
> > > >
> > > > A descriptive name will work too, and I think it'll be easier to make
> > > > those names persistent because you don't have to store the uuids
> > > > somewhere to re-create the same setup afer reboot.
> > >
> > > Hi Gerd,
> > >
> > > Right, UUID will be persistent cross reboot. The qemu vgpu path for a given VM will
> > > not get changed when it gets reboots and multiple other devices have been
> > > created in the middle.
> >
> > Curious why persistence matters here. It's completely OK to assign
> > another vgpu to this VM across reboot, as long as the new vgpu
> > provides same capability to previous one.
> 
> Hi Kevin,
> 
> Those virtual devices might be destroyed and re-created as part of the reboot or
> shutdown. The user doesn't want to change his configuration, if the
> path is not associated with UUID, the user might get a different vgpu than his
> previous configuration, unpredictable.

As long as libvirt keeps configuration under UUID, and then update UUID->
vgpu_path correctly, it won't be a problem:

UUID:
	config1,
	config2,

reboot1:
	UUID -> vgpu1

reboot2:
	UUID -> vgpu2

Then you can always have the configuration applied to new vgpu correctly.

Just want to state it's not a hard limitation. But as I replied in another mail,
I'm not against including UUID in the name, but please keep the option for
UUID not being used.

Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Gerd Hoffmann]

On Di, 2016-02-02 at 00:13 -0800, Neo Jia wrote:
> On Tue, Feb 02, 2016 at 09:00:43AM +0100, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > And for UUID, I remember Alex had a concern on using it in kernel. 
> > > Honestly speaking I don't have a good idea here. In Xen side there is a VM ID
> > > which can be easily used as the index. But for KVM, what would be the best
> > > identifier to associate with a VM?
> > 
> > The vgpu code doesn't need to associate the vgpu device with a vm in the
> > first place.  You get all guest address space information from qemu, via
> > vfio iommu interface.
> > 
> > When qemu does't use kvm (tcg mode), things should still work fine.
> > Using vfio-based vgpu devices with non-qemu apps (some kind of test
> > suite for example) should work fine too.
> 
> Hi Gerd and Kevin,
> 
> I thought Alex had agreed with the UUID as long as it is not tied with VM,
> probably it is just his comment gets lost in our previous long email thread.

As long as it isn't tied to a VM it is fine indeed.  We'll need names
for the devices, and using a uuid certainly is one option.  Could also
be a simple "vgpu${index}".  A more descriptive name such as
"nvidia-grid-${index}" or "igd-skl-${index}".  Or just a free-form
string where the driver can fill whatever it wants (including a uuid).

cheers,
  Gerd

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Kirti Wankhede]

On 2/2/2016 1:12 PM, Tian, Kevin wrote:
>> From: Kirti Wankhede [mailto:kwankhede@nvidia.com]
>> Sent: Tuesday, February 02, 2016 9:48 AM
>>
>> Resending this mail again, somehow my previous mail didn't reached every
>> to everyone's inbox.
>>
>> On 2/2/2016 3:16 AM, Kirti Wankhede wrote:
>>> Design for vGPU Driver:
>>> Main purpose of vGPU driver is to provide a common interface for vGPU
>>> management that can be used by differnt GPU drivers.
>
> Thanks for composing this design which is a good start.
>
>>>
>>> This module would provide a generic interface to create the device, add
>>> it to vGPU bus, add device to IOMMU group and then add it to vfio group.
>>>
>>> High Level block diagram:
>>>
>>>
>>> +--------------+    vgpu_register_driver()+---------------+
>>> |     __init() +------------------------->+               |
>>> |              |                          |               |
>>> |              +<-------------------------+    vgpu.ko    |
>>> | vfio_vgpu.ko |   probe()/remove()       |               |
>>> |              |                +---------+               +---------+
>>> +--------------+                |         +-------+-------+         |
>>>                                   |                 ^                 |
>>>                                   | callback        |                 |
>>>                                   |         +-------+--------+        |
>>>                                   |         |vgpu_register_device()   |
>>>                                   |         |                |        |
>>>                                   +---^-----+-----+    +-----+------+-+
>>>                                       | nvidia.ko |    |  i915.ko   |
>>>                                       |           |    |            |
>>>                                       +-----------+    +------------+
>>>
>>> vGPU driver provides two types of registration interfaces:
>
> Looks you missed callbacks which vgpu.ko provides to vfio_vgpu.ko,
> e.g. to retrieve basic region/resource info, etc...
>

Basic region info or resource info would come from GPU driver. So 
retrieving such info should be the part of GPU driver interface. Like I 
mentioned we need to enhance these interfaces during development as and 
when we find it useful.
vfio_vgpu.ko gets dev pointer from which it can reach to vgpu_device 
structure and then it can use GPU driver interface directly to retrieve 
such information from GPU driver.

This RFC is to focus more on different modules and its structures, how 
those modules would be inter-linked with each other and have a flexible 
design to keep the scope for enhancements.

We have identified three modules:

* vgpu.ko - vGPU core driver that provide registration interface for GPU 
driver and vGPU VFIO  driver, responsible for creating vGPU devices and 
providing management interface for vGPU devices.
* vfio_vgpu.ko - vGPU VFIO driver for vGPU device, provides VFIO 
interface that is used by QEMU.
* vfio_iommu_type1_vgpu.ko - IOMMU TYPE1 driver supporting the IOMMU 
TYPE1 v1 and v2 interface.

The above block diagram gives an overview how vgpu.ko, vfio_vgpu.ko and 
GPU drivers would be inter-linked with each other.


> Also for GPU driver interfaces, better to identify the caller. E.g. it's
> easy to understand life-cycle management would come from sysfs
> by mgmt. stack like libvirt. What about @read and @write? what's
> the connection between this vgpu core driver and specific hypervisor?
> etc. Better to connect all necessary dots so we can refine all
> necessary requirements on this proposal.
>

read and write calls are for PCI CFG space amd MMIO space read/write. 
Read/write access request from QEMU is passed to GPU driver through GPU 
driver interface.

> [...]
>>>
>>> 2. GPU driver interface
>>>
>>> /**
>>>    * struct gpu_device_ops - Structure to be registered for each physical
>>> GPU to
>>>    * register the device to vgpu module.
>>>    *
>>>    * @owner:              The module owner.
>>>    * @vgpu_supported_config: Called to get information about supported
>>>    *                       vgpu types.
>>>    *                      @dev : pci device structure of physical GPU.
>>>    *                      @config: should return string listing supported
>>>    *                      config
>>>    *                      Returns integer: success (0) or error (< 0)
>>>    * @vgpu_create:        Called to allocate basic resouces in graphics
>>>    *                      driver for a particular vgpu.
>>>    *                      @dev: physical pci device structure on which vgpu
>>>    *                            should be created
>>>    *                      @uuid: uuid for which VM it is intended to
>>>    *                      @instance: vgpu instance in that VM
>>>    *                      @vgpu_id: This represents the type of vgpu to be
>>>    *                                created
>>>    *                      Returns integer: success (0) or error (< 0)
>
> Specifically for Intel GVT-g we didn't hard partition resource among vGPUs.
> Instead we allow user to accurately control how many physical resources
> are allocated to a vGPU. So this interface should be extensible to allow
> vendor specific resource control.
>

This interface forwards the create request to vendor/GPU driver 
informing about which physical GPU this request is intended for and the 
type of vGPU. Then its vendor/GPU driver's responsibility to do 
resources allocation and manage resources in their own driver.

> And for UUID, I remember Alex had a concern on using it in kernel.
> Honestly speaking I don't have a good idea here. In Xen side there is a VM ID
> which can be easily used as the index. But for KVM, what would be the best
> identifier to associate with a VM?
>
>>>    * @vgpu_destroy:       Called to free resources in graphics driver for
>>>    *                      a vgpu instance of that VM.
>>>    *                      @dev: physical pci device structure to which
>>>    *                      this vgpu points to.
>>>    *                      @uuid: uuid for which the vgpu belongs to.
>>>    *                      @instance: vgpu instance in that VM
>>>    *                      Returns integer: success (0) or error (< 0)
>>>    *                      If VM is running and vgpu_destroy is called that
>>>    *                      means the vGPU is being hotunpluged. Return error
>>>    *                      if VM is running and graphics driver doesn't
>>>    *                      support vgpu hotplug.
>>>    * @vgpu_start:         Called to do initiate vGPU initialization
>>>    *                      process in graphics driver when VM boots before
>>>    *                      qemu starts.
>>>    *                      @uuid: UUID which is booting.
>>>    *                      Returns integer: success (0) or error (< 0)
>>>    * @vgpu_shutdown:      Called to teardown vGPU related resources for
>>>    *                      the VM
>>>    *                      @uuid: UUID which is shutting down .
>>>    *                      Returns integer: success (0) or error (< 0)
>>>    * @read:               Read emulation callback
>>>    *                      @vdev: vgpu device structure
>>>    *                      @buf: read buffer
>>>    *                      @count: number bytes to read
>>>    *                      @address_space: specifies for which address space
>
> and suppose there'll be an 'offset' as required by usual emulation.
>

Yes, Sorry I missed that in this comment.

>>>    *                      the request is: pci_config_space, IO register
>>>    *                      space or MMIO space.
>>>    *                      Retuns number on bytes read on success or error.
>>>    * @write:              Write emulation callback
>>>    *                      @vdev: vgpu device structure
>>>    *                      @buf: write buffer
>>>    *                      @count: number bytes to be written
>>>    *                      @address_space: specifies for which address space
>>>    *                      the request is: pci_config_space, IO register
>>>    *                      space or MMIO space.
>>>    *                      Retuns number on bytes written on success or
>>> error.
>>>    * @vgpu_set_irqs:      Called to send about interrupts configuration
>>>    *                      information that qemu set.
>>>    *                      @vdev: vgpu device structure
>>>    *                      @flags, index, start, count and *data : same as
>>>    *                      that of struct vfio_irq_set of
>>>    *                      VFIO_DEVICE_SET_IRQS API.
>
> any elaboration how this will be used in your case?
>

QEMU manage interrupts through VFIO_DEVICE_SET_IRQS ioctl, i.e. 
configuring, signaling, masking, and unmasking of interrupts. 
Description of VFIO_DEVICE_SET_IRQS ioctl in include/uapi/linux/vfio.h 
explains meaning of combination of parameter to VFIO driver.
In case of vGPU, interrupts are received by the owner of physical 
device, i.e. GPU driver. Then GPU driver knows for which vGPU device the 
interrupt is intended to. So interrupts related information should be 
available to GPU driver. This interface forwards interrupt related 
information from vfio_vgpu.ko to GPU driver and GPU driver should 
accordingly take necessary action.

>>>    *
>>>    * Physical GPU that support vGPU should be register with vgpu module with
>>>    * gpu_device_ops structure.
>>>    */
>>>
>
> Also it'd be good design to allow extensible usages, such as statistics, and
> other vendor specific control knobs (e.g. foreground/background VM switch
> in Intel GVT-g, etc.)
>

Can you elaborate on what other control knobs that would be needed?

Thanks,
Kirti.

>
> Thanks
> Kevin
>

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Kirti Wankhede
> Sent: Tuesday, February 02, 2016 5:25 PM
> 
> On 2/2/2016 1:12 PM, Tian, Kevin wrote:
> >> From: Kirti Wankhede [mailto:kwankhede@nvidia.com]
> >> Sent: Tuesday, February 02, 2016 9:48 AM
> >>
> >> Resending this mail again, somehow my previous mail didn't reached every
> >> to everyone's inbox.
> >>
> >> On 2/2/2016 3:16 AM, Kirti Wankhede wrote:
> >>> Design for vGPU Driver:
> >>> Main purpose of vGPU driver is to provide a common interface for vGPU
> >>> management that can be used by differnt GPU drivers.
> >
> > Thanks for composing this design which is a good start.
> >
> >>>
> >>> This module would provide a generic interface to create the device, add
> >>> it to vGPU bus, add device to IOMMU group and then add it to vfio group.
> >>>
> >>> High Level block diagram:
> >>>
> >>>
> >>> +--------------+    vgpu_register_driver()+---------------+
> >>> |     __init() +------------------------->+               |
> >>> |              |                          |               |
> >>> |              +<-------------------------+    vgpu.ko    |
> >>> | vfio_vgpu.ko |   probe()/remove()       |               |
> >>> |              |                +---------+               +---------+
> >>> +--------------+                |         +-------+-------+         |
> >>>                                   |                 ^                 |
> >>>                                   | callback        |                 |
> >>>                                   |         +-------+--------+        |
> >>>                                   |         |vgpu_register_device()   |
> >>>                                   |         |                |        |
> >>>                                   +---^-----+-----+    +-----+------+-+
> >>>                                       | nvidia.ko |    |  i915.ko   |
> >>>                                       |           |    |            |
> >>>                                       +-----------+    +------------+
> >>>
> >>> vGPU driver provides two types of registration interfaces:
> >
> > Looks you missed callbacks which vgpu.ko provides to vfio_vgpu.ko,
> > e.g. to retrieve basic region/resource info, etc...
> >
> 
> Basic region info or resource info would come from GPU driver. So
> retrieving such info should be the part of GPU driver interface. Like I
> mentioned we need to enhance these interfaces during development as and
> when we find it useful.
> vfio_vgpu.ko gets dev pointer from which it can reach to vgpu_device
> structure and then it can use GPU driver interface directly to retrieve
> such information from GPU driver.

Thanks for confirmation. Want to make sure we are on the same page
since those callbacks are not listed in current proposal yet.

> 
> This RFC is to focus more on different modules and its structures, how
> those modules would be inter-linked with each other and have a flexible
> design to keep the scope for enhancements.
> 
> We have identified three modules:
> 
> * vgpu.ko - vGPU core driver that provide registration interface for GPU
> driver and vGPU VFIO  driver, responsible for creating vGPU devices and
> providing management interface for vGPU devices.
> * vfio_vgpu.ko - vGPU VFIO driver for vGPU device, provides VFIO
> interface that is used by QEMU.
> * vfio_iommu_type1_vgpu.ko - IOMMU TYPE1 driver supporting the IOMMU
> TYPE1 v1 and v2 interface.
> 
> The above block diagram gives an overview how vgpu.ko, vfio_vgpu.ko and
> GPU drivers would be inter-linked with each other.

This part is clear.

> >>>    * @vgpu_create:        Called to allocate basic resouces in graphics
> >>>    *                      driver for a particular vgpu.
> >>>    *                      @dev: physical pci device structure on which vgpu
> >>>    *                            should be created
> >>>    *                      @uuid: uuid for which VM it is intended to
> >>>    *                      @instance: vgpu instance in that VM
> >>>    *                      @vgpu_id: This represents the type of vgpu to be
> >>>    *                                created
> >>>    *                      Returns integer: success (0) or error (< 0)
> >
> > Specifically for Intel GVT-g we didn't hard partition resource among vGPUs.
> > Instead we allow user to accurately control how many physical resources
> > are allocated to a vGPU. So this interface should be extensible to allow
> > vendor specific resource control.
> >
> 
> This interface forwards the create request to vendor/GPU driver
> informing about which physical GPU this request is intended for and the
> type of vGPU. Then its vendor/GPU driver's responsibility to do
> resources allocation and manage resources in their own driver.

However the current parameter definition disallows resource configuration
information passed from user. As I said, Intel GVT-g doesn't do static
allocation based on type. We provide flexibility to user for fine-grained
resource management.

> >>>    *
> >>>    * Physical GPU that support vGPU should be register with vgpu module with
> >>>    * gpu_device_ops structure.
> >>>    */
> >>>
> >
> > Also it'd be good design to allow extensible usages, such as statistics, and
> > other vendor specific control knobs (e.g. foreground/background VM switch
> > in Intel GVT-g, etc.)
> >
> 
> Can you elaborate on what other control knobs that would be needed?
> 

Some examples:

- foreground/background VM switch
- resource query
- various statistics info
- virtual monitor configuration
- ...

Since this vgpu core driver will become the central point for all vgpu
management, we need provide an easy way for vendor specific extension,
e.g. exposing above callbacks as sysfs nodes and then vendor can create
its own extensions under subdirectory (/intel, /nvidia, ...).


Thanks
Kevin

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Kirti Wankhede]

On 2/3/2016 11:26 AM, Tian, Kevin wrote:
[...]
>>>>>     * @vgpu_create:        Called to allocate basic resouces in graphics
>>>>>     *                      driver for a particular vgpu.
>>>>>     *                      @dev: physical pci device structure on which vgpu
>>>>>     *                            should be created
>>>>>     *                      @uuid: uuid for which VM it is intended to
>>>>>     *                      @instance: vgpu instance in that VM
>>>>>     *                      @vgpu_id: This represents the type of vgpu to be
>>>>>     *                                created
>>>>>     *                      Returns integer: success (0) or error (< 0)
>>>
>>> Specifically for Intel GVT-g we didn't hard partition resource among vGPUs.
>>> Instead we allow user to accurately control how many physical resources
>>> are allocated to a vGPU. So this interface should be extensible to allow
>>> vendor specific resource control.
>>>
>>
>> This interface forwards the create request to vendor/GPU driver
>> informing about which physical GPU this request is intended for and the
>> type of vGPU. Then its vendor/GPU driver's responsibility to do
>> resources allocation and manage resources in their own driver.
>
> However the current parameter definition disallows resource configuration
> information passed from user. As I said, Intel GVT-g doesn't do static
> allocation based on type. We provide flexibility to user for fine-grained
> resource management.
>

         int     (*vgpu_create)(struct pci_dev *dev, uuid_le uuid,
-                              uint32_t instance, uint32_t vgpu_id);
+                              uint32_t instance, char *vgpu_params);

If we change integer vgpu_id parameter to char *vgpu_param then GPU 
driver can have multiple parameters.

Suppose there is a GPU at 0000:85:00.0, then to create vgpu:
# echo "<uuid>:<vgpu_instance>:<vgpu_params string>" > 
/sys/bus/pci/devices/0000\:85\:00.0/vgpu_create

Common vgpu module will not parse vgpu_params string, it will be 
forwarded to GPU driver, then its GPU driver's responsibility to parse 
the string and act accordingly. This should give flexibility to have 
multiple parameters for GPU driver.

>>>>>     *
>>>>>     * Physical GPU that support vGPU should be register with vgpu module with
>>>>>     * gpu_device_ops structure.
>>>>>     */
>>>>>
>>>
>>> Also it'd be good design to allow extensible usages, such as statistics, and
>>> other vendor specific control knobs (e.g. foreground/background VM switch
>>> in Intel GVT-g, etc.)
>>>
>>
>> Can you elaborate on what other control knobs that would be needed?
>>
>
> Some examples:
>
> - foreground/background VM switch
> - resource query
> - various statistics info
> - virtual monitor configuration
> - ...
>
> Since this vgpu core driver will become the central point for all vgpu
> management, we need provide an easy way for vendor specific extension,
> e.g. exposing above callbacks as sysfs nodes and then vendor can create
> its own extensions under subdirectory (/intel, /nvidia, ...).
>
>

Ok. Makes sense.

Are these parameters per physical device or per vgpu device?

Adding attribute groups to gpu_device_ops structure would provide a way 
to add vendor specific extensions. I think we need two types of 
attributes here, per physical device and per vgpu device. Right?

          const struct attribute_group **dev_groups;
          const struct attribute_group **vgpu_groups;



Thanks,
Kirti.

> Thanks
> Kevin
>

Re: [Qemu-devel] [RFC PATCH v1 1/1] vGPU core driver : to provide common interface for vGPU.

[Tian, Kevin]

> From: Kirti Wankhede
> Sent: Wednesday, February 03, 2016 9:21 PM
> 
> 
> On 2/3/2016 11:26 AM, Tian, Kevin wrote:
> [...]
> >>>>>     * @vgpu_create:        Called to allocate basic resouces in graphics
> >>>>>     *                      driver for a particular vgpu.
> >>>>>     *                      @dev: physical pci device structure on which vgpu
> >>>>>     *                            should be created
> >>>>>     *                      @uuid: uuid for which VM it is intended to
> >>>>>     *                      @instance: vgpu instance in that VM
> >>>>>     *                      @vgpu_id: This represents the type of vgpu to be
> >>>>>     *                                created
> >>>>>     *                      Returns integer: success (0) or error (< 0)
> >>>
> >>> Specifically for Intel GVT-g we didn't hard partition resource among vGPUs.
> >>> Instead we allow user to accurately control how many physical resources
> >>> are allocated to a vGPU. So this interface should be extensible to allow
> >>> vendor specific resource control.
> >>>
> >>
> >> This interface forwards the create request to vendor/GPU driver
> >> informing about which physical GPU this request is intended for and the
> >> type of vGPU. Then its vendor/GPU driver's responsibility to do
> >> resources allocation and manage resources in their own driver.
> >
> > However the current parameter definition disallows resource configuration
> > information passed from user. As I said, Intel GVT-g doesn't do static
> > allocation based on type. We provide flexibility to user for fine-grained
> > resource management.
> >
> 
>          int     (*vgpu_create)(struct pci_dev *dev, uuid_le uuid,
> -                              uint32_t instance, uint32_t vgpu_id);
> +                              uint32_t instance, char *vgpu_params);
> 
> If we change integer vgpu_id parameter to char *vgpu_param then GPU
> driver can have multiple parameters.
> 
> Suppose there is a GPU at 0000:85:00.0, then to create vgpu:
> # echo "<uuid>:<vgpu_instance>:<vgpu_params string>" >
> /sys/bus/pci/devices/0000\:85\:00.0/vgpu_create
> 
> Common vgpu module will not parse vgpu_params string, it will be
> forwarded to GPU driver, then its GPU driver's responsibility to parse
> the string and act accordingly. This should give flexibility to have
> multiple parameters for GPU driver.

Yes, this way it would work.

> 
> >>>>>     *
> >>>>>     * Physical GPU that support vGPU should be register with vgpu module with
> >>>>>     * gpu_device_ops structure.
> >>>>>     */
> >>>>>
> >>>
> >>> Also it'd be good design to allow extensible usages, such as statistics, and
> >>> other vendor specific control knobs (e.g. foreground/background VM switch
> >>> in Intel GVT-g, etc.)
> >>>
> >>
> >> Can you elaborate on what other control knobs that would be needed?
> >>
> >
> > Some examples:
> >
> > - foreground/background VM switch
> > - resource query
> > - various statistics info
> > - virtual monitor configuration
> > - ...
> >
> > Since this vgpu core driver will become the central point for all vgpu
> > management, we need provide an easy way for vendor specific extension,
> > e.g. exposing above callbacks as sysfs nodes and then vendor can create
> > its own extensions under subdirectory (/intel, /nvidia, ...).
> >
> >
> 
> Ok. Makes sense.
> 
> Are these parameters per physical device or per vgpu device?

some for physical, others for vgpu.

> 
> Adding attribute groups to gpu_device_ops structure would provide a way
> to add vendor specific extensions. I think we need two types of
> attributes here, per physical device and per vgpu device. Right?
> 
>           const struct attribute_group **dev_groups;
>           const struct attribute_group **vgpu_groups;
> 
> 
> 

Agree.

Thanks
Kevin