Re: [Qemu-devel] [PATCH v9 01/12] vfio: Mediated device Core driver

[Jike Song]

On 10/18/2016 05:22 AM, Kirti Wankhede wrote:
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> new file mode 100644
> index 000000000000..7db5ec164aeb
> --- /dev/null
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -0,0 +1,372 @@
> +/*
> + * Mediated device Core Driver
> + *
> + * Copyright (c) 2016, NVIDIA CORPORATION. All rights reserved.
> + *     Author: Neo Jia <address@hidden>
> + *          Kirti Wankhede <address@hidden>
> + *
> + * 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/module.h>
> +#include <linux/device.h>
> +#include <linux/slab.h>
> +#include <linux/uuid.h>
> +#include <linux/sysfs.h>
> +#include <linux/mdev.h>
> +
> +#include "mdev_private.h"
> +
> +#define DRIVER_VERSION               "0.1"
> +#define DRIVER_AUTHOR                "NVIDIA Corporation"
> +#define DRIVER_DESC          "Mediated device Core Driver"
> +
> +static LIST_HEAD(parent_list);
> +static DEFINE_MUTEX(parent_list_lock);
> +static struct class_compat *mdev_bus_compat_class;
> +

> +
> +/*
> + * mdev_register_device : Register a device
> + * @dev: device structure representing parent device.
> + * @ops: Parent device operation structure to be registered.
> + *
> + * Add device to list of registered parent devices.
> + * Returns a negative value on error, otherwise 0.
> + */
> +int mdev_register_device(struct device *dev, const struct parent_ops *ops)
> +{
> +     int ret = 0;
> +     struct parent_device *parent;
> +
> +     /* check for mandatory ops */
> +     if (!ops || !ops->create || !ops->remove || !ops->supported_type_groups)
> +             return -EINVAL;
> +
> +     dev = get_device(dev);
> +     if (!dev)
> +             return -EINVAL;
> +
> +     mutex_lock(&parent_list_lock);
> +
> +     /* Check for duplicate */
> +     parent = __find_parent_device(dev);
> +     if (parent) {
> +             ret = -EEXIST;
> +             goto add_dev_err;
> +     }
> +
> +     parent = kzalloc(sizeof(*parent), GFP_KERNEL);
> +     if (!parent) {
> +             ret = -ENOMEM;
> +             goto add_dev_err;
> +     }
> +
> +     kref_init(&parent->ref);
> +
> +     parent->dev = dev;
> +     parent->ops = ops;
> +
> +     ret = parent_create_sysfs_files(parent);
> +     if (ret) {
> +             mutex_unlock(&parent_list_lock);
> +             mdev_put_parent(parent);
> +             return ret;
> +     }
> +
> +     ret = class_compat_create_link(mdev_bus_compat_class, dev, NULL);
> +     if (ret)
> +             dev_warn(dev, "Failed to create compatibility class link\n");
> +
> +     list_add(&parent->next, &parent_list);
> +     mutex_unlock(&parent_list_lock);
> +
> +     dev_info(dev, "MDEV: Registered\n");
> +     return 0;
> +
> +add_dev_err:
> +     mutex_unlock(&parent_list_lock);
> +     put_device(dev);
> +     return ret;
> +}
> +EXPORT_SYMBOL(mdev_register_device);

> +static int __init mdev_init(void)
> +{
> +     int ret;
> +
> +     ret = mdev_bus_register();
> +     if (ret) {
> +             pr_err("Failed to register mdev bus\n");
> +             return ret;
> +     }
> +
> +     mdev_bus_compat_class = class_compat_register("mdev_bus");
> +     if (!mdev_bus_compat_class) {
> +             mdev_bus_unregister();
> +             return -ENOMEM;
> +     }
> +
> +     /*
> +      * Attempt to load known vfio_mdev.  This gives us a working environment
> +      * without the user needing to explicitly load vfio_mdev driver.
> +      */
> +     request_module_nowait("vfio_mdev");
> +
> +     return ret;
> +}
> +
> +static void __exit mdev_exit(void)
> +{
> +     class_compat_unregister(mdev_bus_compat_class);
> +     mdev_bus_unregister();
> +}
> +
> +module_init(mdev_init)
> +module_exit(mdev_exit)

Hi Kirti,

There is a possible issue: mdev_bus_register is called from mdev_init,
a module_init, equal to device_initcall if builtin to vmlinux; however,
the vendor driver, say i915.ko for intel case, have to call
mdev_register_device from its module_init: at that time, mdev_init
is still not called.

Not sure if this issue exists with nvidia.ko. Though in most cases we
are expecting users select mdev as a standalone module, we still won't
break builtin case.


Hi Alex, do you have any suggestion here?


--
Thanks,
Jike