| Message ID | 20200212191424.1715577-2-jeffrey.t.kirsher@intel.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | Intel Wired LAN/RDMA Driver Updates 2020-02-11 | expand |
On Wed, Feb 12, 2020 at 11:14:00AM -0800, Jeff Kirsher wrote: > From: Dave Ertman <david.m.ertman@intel.com> > > This is the initial implementation of the Virtual Bus, > virtbus_device and virtbus_driver. The virtual bus is > a software based bus intended to support registering > virtbus_devices and virtbus_drivers and provide matching > between them and probing of the registered drivers. > > The bus will support probe/remove shutdown and > suspend/resume callbacks. > > Kconfig and Makefile alterations are included > > Signed-off-by: Dave Ertman <david.m.ertman@intel.com> > Signed-off-by: Kiran Patil <kiran.patil@intel.com> > Tested-by: Andrew Bowers <andrewx.bowers@intel.com> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> This looks a lot better, and is more of what I was thinking. Some minor comments below: > +/** > + * virtbus_dev_register - add a virtual bus device > + * @vdev: virtual bus device to add > + */ > +int virtbus_dev_register(struct virtbus_device *vdev) > +{ > + int ret; > + > + if (!vdev->release) { > + dev_err(&vdev->dev, "virtbus_device .release callback NULL\n"); "virtbus_device MUST have a .release callback that does something!\n" > + return -EINVAL; > + } > + > + device_initialize(&vdev->dev); > + > + vdev->dev.bus = &virtual_bus_type; > + vdev->dev.release = virtbus_dev_release; > + /* All device IDs are automatically allocated */ > + ret = ida_simple_get(&virtbus_dev_ida, 0, 0, GFP_KERNEL); > + if (ret < 0) { > + dev_err(&vdev->dev, "get IDA idx for virtbus device failed!\n"); > + put_device(&vdev->dev); If you allocate the number before device_initialize(), no need to call put_device(). Just a minor thing, no big deal. > + return ret; > + } > + > + vdev->id = ret; > + dev_set_name(&vdev->dev, "%s.%d", vdev->name, vdev->id); > + > + dev_dbg(&vdev->dev, "Registering virtbus device '%s'\n", > + dev_name(&vdev->dev)); > + > + ret = device_add(&vdev->dev); > + if (ret) > + goto device_add_err; > + > + return 0; > + > +device_add_err: > + dev_err(&vdev->dev, "Add device to virtbus failed!\n"); Print the return error here too? > + put_device(&vdev->dev); > + ida_simple_remove(&virtbus_dev_ida, vdev->id); You need to do this before put_device(). > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(virtbus_dev_register); > + > --- /dev/null > +++ b/include/linux/virtual_bus.h > @@ -0,0 +1,57 @@ > +/* SPDX-License-Identifier: GPL-2.0-only */ > +/* > + * virtual_bus.h - lightweight software bus > + * > + * Copyright (c) 2019-20 Intel Corporation > + * > + * Please see Documentation/driver-api/virtual_bus.rst for more information > + */ > + > +#ifndef _VIRTUAL_BUS_H_ > +#define _VIRTUAL_BUS_H_ > + > +#include <linux/device.h> > + > +struct virtbus_device { > + struct device dev; > + const char *name; > + void (*release)(struct virtbus_device *); > + int id; > + const struct virtbus_dev_id *matched_element; > +}; Any reason you need to make "struct virtbus_device" a public structure at all? Why not just make it private and have the release function pointer be passed as part of the register function? That will keep people from poking around in here. > + > +/* The memory for the table is expected to remain allocated for the duration > + * of the pairing between driver and device. The pointer for the matching > + * element will be copied to the matched_element field of the virtbus_device. I don't understand this last sentance, what are you trying to say? We save off a pointer to the element, so it better not go away, is that what you mean? Why would this happen? > + */ > +struct virtbus_driver { > + int (*probe)(struct virtbus_device *); > + int (*remove)(struct virtbus_device *); > + void (*shutdown)(struct virtbus_device *); > + int (*suspend)(struct virtbus_device *, pm_message_t); > + int (*resume)(struct virtbus_device *); Can all of these be const pointers such that we will not change them? > + struct device_driver driver; > + const struct virtbus_dev_id *id_table; > +}; thanks, greg k-h
On Fri, Feb 14, 2020 at 09:02:40AM -0800, Greg KH wrote: > > +/** > > + * virtbus_dev_register - add a virtual bus device > > + * @vdev: virtual bus device to add > > + */ > > +int virtbus_dev_register(struct virtbus_device *vdev) > > +{ > > + int ret; > > + > > + if (!vdev->release) { > > + dev_err(&vdev->dev, "virtbus_device .release callback NULL\n"); > > "virtbus_device MUST have a .release callback that does something!\n" > > > + return -EINVAL; > > + } > > + > > + device_initialize(&vdev->dev); > > + > > + vdev->dev.bus = &virtual_bus_type; > > + vdev->dev.release = virtbus_dev_release; > > + /* All device IDs are automatically allocated */ > > + ret = ida_simple_get(&virtbus_dev_ida, 0, 0, GFP_KERNEL); > > + if (ret < 0) { > > + dev_err(&vdev->dev, "get IDA idx for virtbus device failed!\n"); > > + put_device(&vdev->dev); > > If you allocate the number before device_initialize(), no need to call > put_device(). Just a minor thing, no big deal. If *_regster does put_device on error then it must always do put_device on any error, for instance the above return -EINVAL with no put_device leaks memory. Generally I find the design and audit of drivers simpler if the register doesn't do device_initialize or put_device - have them distinct and require the caller to manage this. For instance look at ice_init_peer_devices() and ask who frees the alloc_ordered_workqueue() if virtbus_dev_register() fails.. It is not all easy to tell if this is right or not.. > > + put_device(&vdev->dev); > > + ida_simple_remove(&virtbus_dev_ida, vdev->id); > > You need to do this before put_device(). Shouldn't it be in the release function? The ida index should not be re-used until the kref goes to zero.. > > +struct virtbus_device { > > + struct device dev; > > + const char *name; > > + void (*release)(struct virtbus_device *); > > + int id; > > + const struct virtbus_dev_id *matched_element; > > +}; > > Any reason you need to make "struct virtbus_device" a public structure > at all? The general point of this scheme is to do this in a public header: +struct iidc_virtbus_object { + struct virtbus_device vdev; + struct iidc_peer_dev *peer_dev; +}; And then this when the driver binds: +int irdma_probe(struct virtbus_device *vdev) +{ + struct iidc_virtbus_object *vo = + container_of(vdev, struct iidc_virtbus_object, vdev); + struct iidc_peer_dev *ldev = vo->peer_dev; So the virtbus_device is in a public header to enable the container_of construction. Jason
On Fri, Feb 14, 2020 at 04:34:55PM -0400, Jason Gunthorpe wrote: > On Fri, Feb 14, 2020 at 09:02:40AM -0800, Greg KH wrote: > > > +/** > > > + * virtbus_dev_register - add a virtual bus device > > > + * @vdev: virtual bus device to add > > > + */ > > > +int virtbus_dev_register(struct virtbus_device *vdev) > > > +{ > > > + int ret; > > > + > > > + if (!vdev->release) { > > > + dev_err(&vdev->dev, "virtbus_device .release callback NULL\n"); > > > > "virtbus_device MUST have a .release callback that does something!\n" > > > > > + return -EINVAL; > > > + } > > > + > > > + device_initialize(&vdev->dev); > > > + > > > + vdev->dev.bus = &virtual_bus_type; > > > + vdev->dev.release = virtbus_dev_release; > > > + /* All device IDs are automatically allocated */ > > > + ret = ida_simple_get(&virtbus_dev_ida, 0, 0, GFP_KERNEL); > > > + if (ret < 0) { > > > + dev_err(&vdev->dev, "get IDA idx for virtbus device failed!\n"); > > > + put_device(&vdev->dev); > > > > If you allocate the number before device_initialize(), no need to call > > put_device(). Just a minor thing, no big deal. > > If *_regster does put_device on error then it must always do > put_device on any error, for instance the above return -EINVAL with > no put_device leaks memory. That's why I said to move the ida_simple_get() call to before device_initialize() is called. Once device_initialize() is called, you HAVE to call put_device(). Just trying to make code smaller :) greg k-h
On Fri, Feb 14, 2020 at 04:34:55PM -0400, Jason Gunthorpe wrote: > On Fri, Feb 14, 2020 at 09:02:40AM -0800, Greg KH wrote: > > > + put_device(&vdev->dev); > > > + ida_simple_remove(&virtbus_dev_ida, vdev->id); > > > > You need to do this before put_device(). > > Shouldn't it be in the release function? The ida index should not be > re-used until the kref goes to zero.. Doesn't really matter, once you have unregistered it, you can reuse it. But yes, putting it in release() is the safest thing to do. > > > +struct virtbus_device { > > > + struct device dev; > > > + const char *name; > > > + void (*release)(struct virtbus_device *); > > > + int id; > > > + const struct virtbus_dev_id *matched_element; > > > +}; > > > > Any reason you need to make "struct virtbus_device" a public structure > > at all? > > The general point of this scheme is to do this in a public header: > > +struct iidc_virtbus_object { > + struct virtbus_device vdev; > + struct iidc_peer_dev *peer_dev; > +}; > > And then this when the driver binds: Ah, yes, nevermind, I missed that. greg k-h
On 2/12/2020 1:14 PM, Jeff Kirsher wrote: > From: Dave Ertman <david.m.ertman@intel.com> > > This is the initial implementation of the Virtual Bus, > virtbus_device and virtbus_driver. The virtual bus is > a software based bus intended to support registering > virtbus_devices and virtbus_drivers and provide matching > between them and probing of the registered drivers. > > The bus will support probe/remove shutdown and > suspend/resume callbacks. > > Kconfig and Makefile alterations are included > > Signed-off-by: Dave Ertman <david.m.ertman@intel.com> > Signed-off-by: Kiran Patil <kiran.patil@intel.com> > Tested-by: Andrew Bowers <andrewx.bowers@intel.com> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > --- > Documentation/driver-api/virtual_bus.rst | 59 +++++ > drivers/bus/Kconfig | 11 + > drivers/bus/Makefile | 1 + > drivers/bus/virtual_bus.c | 267 +++++++++++++++++++++++ > include/linux/mod_devicetable.h | 8 + > include/linux/virtual_bus.h | 57 +++++ > scripts/mod/devicetable-offsets.c | 3 + > scripts/mod/file2alias.c | 8 + > 8 files changed, 414 insertions(+) > create mode 100644 Documentation/driver-api/virtual_bus.rst > create mode 100644 drivers/bus/virtual_bus.c > create mode 100644 include/linux/virtual_bus.h > > diff --git a/Documentation/driver-api/virtual_bus.rst b/Documentation/driver-api/virtual_bus.rst > new file mode 100644 > index 000000000000..5f35c19171d7 > --- /dev/null > +++ b/Documentation/driver-api/virtual_bus.rst > @@ -0,0 +1,59 @@ > +=============================== > +Virtual Bus Devices and Drivers > +=============================== > + > +See <linux/virtual_bus.h> for the models for virtbus_device and virtbus_driver. > +This bus is meant to be a lightweight software based bus to attach generic > +devices and drivers to so that a chunk of data can be passed between them. > + > +One use case example is an rdma driver needing to connect with several > +different types of PCI LAN devices to be able to request resources from > +them (queue sets). Each LAN driver that supports rdma will register a > +virtbus_device on the virtual bus for each physical function. The rdma > +driver will register as a virtbus_driver on the virtual bus to be > +matched up with multiple virtbus_devices and receive a pointer to a > +struct containing the callbacks that the PCI LAN drivers support for > +registering with them. > + > +Sections in this document: > + Virtbus devices > + Virtbus drivers > + Device Enumeration > + Device naming and driver binding > + Virtual Bus API entry points > + > +Virtbus devices > +~~~~~~~~~~~~~~~ > +Virtbus_devices support the minimal device functionality. Devices will > +accept a name, and then, when added to the virtual bus, an automatically > +generated index is concatenated onto it for the virtbus_device->name. > + > +Virtbus drivers > +~~~~~~~~~~~~~~~ > +Virtbus drivers register with the virtual bus to be matched with virtbus > +devices. They expect to be registered with a probe and remove callback, > +and also support shutdown, suspend, and resume callbacks. They otherwise > +follow the standard driver behavior of having discovery and enumeration > +handled in the bus infrastructure. > + > +Virtbus drivers register themselves with the API entry point virtbus_drv_reg > +and unregister with virtbus_drv_unreg. > + If you are mentioning API names, please keep the API documentation match with API name in header file. > +Device Enumeration > +~~~~~~~~~~~~~~~~~~ > +Enumeration is handled automatically by the bus infrastructure via the > +ida_simple methods. > + > +Device naming and driver binding > +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > +The virtbus_device.dev.name is the canonical name for the device. It is > +built from two other parts: > + > + - virtbus_device.name (also used for matching). > + - virtbus_device.id (generated automatically from ida_simple calls) > + > +Virtbus device IDs are always in "<name>.<instance>" format. Instances are > +automatically selected through an ida_simple_get so are positive integers. > +Names are taken from the device name field. Name is taken from the device name field. Driver IDs are simple <name>. > +Need to extract the name from the Virtual Device compare to name of the > +driver. > diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig > index 6095b6df8a81..2e8b89c1761a 100644 > --- a/drivers/bus/Kconfig > +++ b/drivers/bus/Kconfig > @@ -202,4 +202,15 @@ config DA8XX_MSTPRI > > source "drivers/bus/fsl-mc/Kconfig" > > +config VIRTUAL_BUS > + tristate "Software based Virtual Bus" > + help > + Provides a software bus for virtbus_devices to be added to it > + and virtbus_drivers to be registered on it. Will create a match> + between the driver and device, then call the driver's probe with > + the virtbus_device's struct. below text reads better for "Will create a match.." It matches driver and device based on id and calls driver's probe routine. > + One example is the irdma driver needing to connect with various > + PCI LAN drivers to request resources (queues) to be able to perform > + its function. > + > endmenu > diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile > index 1320bcf9fa9d..6721c77dc71b 100644 > --- a/drivers/bus/Makefile > +++ b/drivers/bus/Makefile > @@ -34,3 +34,4 @@ obj-$(CONFIG_UNIPHIER_SYSTEM_BUS) += uniphier-system-bus.o > obj-$(CONFIG_VEXPRESS_CONFIG) += vexpress-config.o > > obj-$(CONFIG_DA8XX_MSTPRI) += da8xx-mstpri.o > +obj-$(CONFIG_VIRTUAL_BUS) += virtual_bus.o > diff --git a/drivers/bus/virtual_bus.c b/drivers/bus/virtual_bus.c > new file mode 100644 > index 000000000000..85d2dbfa3376 > --- /dev/null > +++ b/drivers/bus/virtual_bus.c > @@ -0,0 +1,267 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * virtual_bus.c - lightweight software based bus for virtual devices > + * > + * Copyright (c) 2019-20 Intel Corporation > + * > + * Please see Documentation/driver-api/virtual_bus.rst for > + * more information > + */ > + > +#include <linux/string.h> > +#include <linux/virtual_bus.h> > +#include <linux/of_irq.h> > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/pm_runtime.h> > +#include <linux/pm_domain.h> > +#include <linux/acpi.h> > +#include <linux/device.h> > + > +MODULE_LICENSE("GPL v2"); > +MODULE_DESCRIPTION("Lightweight Virtual Bus"); Last time Greg had comment about "Lightweight". In the bus/Kconfig, it is named as "software based virtual bus". Just say "Virtual bus". > +MODULE_AUTHOR("David Ertman <david.m.ertman@intel.com>"); > +MODULE_AUTHOR("Kiran Patil <kiran.patil@intel.com>"); > + > +static DEFINE_IDA(virtbus_dev_ida); > + > +static const > +struct virtbus_dev_id *virtbus_match_id(const struct virtbus_dev_id *id, > + struct virtbus_device *vdev) > +{ > + while (id->name[0]) { > + if (!strcmp(vdev->name, id->name)) { > + vdev->matched_element = id; I am yet to review the actual usage of matched_element field in subsequent patches, but assigning this inside the match routine doesn't look correct. > + return id; > + } > + id++; > + } > + return NULL; > +} > + > +static int virtbus_match(struct device *dev, struct device_driver *drv) > +{ > + struct virtbus_driver *vdrv = to_virtbus_drv(drv); > + struct virtbus_device *vdev = to_virtbus_dev(dev); > + > + return virtbus_match_id(vdrv->id_table, vdev) != NULL; > +} > + > +static int virtbus_probe(struct device *dev) > +{ > + return dev->driver->probe(dev); > +} > + > +static int virtbus_remove(struct device *dev) > +{ > + return dev->driver->remove(dev); > +} > + > +static void virtbus_shutdown(struct device *dev) > +{ > + dev->driver->shutdown(dev); > +} > + > +static int virtbus_suspend(struct device *dev, pm_message_t state) > +{ > + if (dev->driver->suspend) > + return dev->driver->suspend(dev, state); > + > + return 0; > +} > + > +static int virtbus_resume(struct device *dev) > +{ > + if (dev->driver->resume) > + return dev->driver->resume(dev); > + > + return 0; > +} > + > +struct bus_type virtual_bus_type = { static struct bus_type. > + .name = "virtbus", > + .match = virtbus_match, > + .probe = virtbus_probe, > + .remove = virtbus_remove, > + .shutdown = virtbus_shutdown, > + .suspend = virtbus_suspend, > + .resume = virtbus_resume, > +}; > + > +/** > + * virtbus_dev_release - Destroy a virtbus device > + * @vdev: virtual device to release > + */ > +static void virtbus_dev_release(struct device *_dev) > +{ > + struct virtbus_device *vdev = to_virtbus_dev(_dev); > + > + ida_simple_remove(&virtbus_dev_ida, vdev->id); Device is not yet released. It is getting released below. Please move ida_simple_remove() after completing the release call. Save vdev->id first. This makes mirror sequence of dev_register() once you move ida_get before device_initialize() as Greg suggested. > + vdev->release(vdev); > +} > + > +/** > + * virtbus_dev_register - add a virtual bus device > + * @vdev: virtual bus device to add > + */ > +int virtbus_dev_register(struct virtbus_device *vdev) > +{ > + int ret; > + > + if (!vdev->release) { > + dev_err(&vdev->dev, "virtbus_device .release callback NULL\n"); > + return -EINVAL; > + } > + > + device_initialize(&vdev->dev); > + > + vdev->dev.bus = &virtual_bus_type; > + vdev->dev.release = virtbus_dev_release; > + /* All device IDs are automatically allocated */ > + ret = ida_simple_get(&virtbus_dev_ida, 0, 0, GFP_KERNEL); > + if (ret < 0) { > + dev_err(&vdev->dev, "get IDA idx for virtbus device failed!\n"); > + put_device(&vdev->dev); > + return ret; > + } > + > + vdev->id = ret; > + dev_set_name(&vdev->dev, "%s.%d", vdev->name, vdev->id); > + > + dev_dbg(&vdev->dev, "Registering virtbus device '%s'\n", > + dev_name(&vdev->dev)); > + > + ret = device_add(&vdev->dev); > + if (ret) > + goto device_add_err; > + > + return 0; > + > +device_add_err: > + dev_err(&vdev->dev, "Add device to virtbus failed!\n"); Print error code along with the error line is more useful. > + put_device(&vdev->dev); > + ida_simple_remove(&virtbus_dev_ida, vdev->id); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(virtbus_dev_register); > + > +/** > + * virtbus_dev_unregister - remove a virtual bus device > + * vdev: virtual bus device we are removing > + */ > +void virtbus_dev_unregister(struct virtbus_device *vdev) > +{ > + device_del(&vdev->dev); > + put_device(&vdev->dev); > +} > +EXPORT_SYMBOL_GPL(virtbus_dev_unregister); > + > +static int virtbus_drv_probe(struct device *_dev) > +{ > + struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver); > + struct virtbus_device *vdev = to_virtbus_dev(_dev); > + int ret; > + > + ret = dev_pm_domain_attach(_dev, true); > + if (ret) { > + dev_warn(_dev, "Failed to attatch to PM Domain : %d\n", ret); > + return ret; > + } > + > + ret = vdrv->probe(vdev); > + if (ret) { > + dev_err(&vdev->dev, "Probe returned error\n"); > + dev_pm_domain_detach(_dev, true); > + } > + > + return ret; > +} > + > +static int virtbus_drv_remove(struct device *_dev) > +{ > + struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver); > + struct virtbus_device *vdev = to_virtbus_dev(_dev); > + int ret = 0; > + > + ret = vdrv->remove(vdev); > + dev_pm_domain_detach(_dev, true); > + > + return ret; > +} > + > +static void virtbus_drv_shutdown(struct device *_dev) > +{ > + struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver); > + struct virtbus_device *vdev = to_virtbus_dev(_dev); > + > + vdrv->shutdown(vdev); > +} > + > +static int virtbus_drv_suspend(struct device *_dev, pm_message_t state) > +{ > + struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver); > + struct virtbus_device *vdev = to_virtbus_dev(_dev); > + > + if (vdrv->suspend) > + return vdrv->suspend(vdev, state); > + > + return 0; > +} > + > +static int virtbus_drv_resume(struct device *_dev) > +{ > + struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver); > + struct virtbus_device *vdev = to_virtbus_dev(_dev); > + > + if (vdrv->resume) > + return vdrv->resume(vdev); > + > + return 0; > +} > + > +/** > + * __virtbus_drv_register - register a driver for virtual bus devices > + * @vdrv: virtbus_driver structure > + * @owner: owning module/driver > + */ > +int __virtbus_drv_register(struct virtbus_driver *vdrv, struct module *owner) > +{ Lets keep name consistent with other subsystems such as, pci_register_driver(), spi_register_driver() etc as virtbus_register_driver() and unregister(). > + if (!vdrv->probe || !vdrv->remove || !vdrv->shutdown || !vdrv->id_table) > + return -EINVAL; > + > + vdrv->driver.owner = owner; > + vdrv->driver.bus = &virtual_bus_type; > + vdrv->driver.probe = virtbus_drv_probe; > + vdrv->driver.remove = virtbus_drv_remove; > + vdrv->driver.shutdown = virtbus_drv_shutdown; > + vdrv->driver.suspend = virtbus_drv_suspend; > + vdrv->driver.resume = virtbus_drv_resume; > + > + return driver_register(&vdrv->driver); > +} > +EXPORT_SYMBOL_GPL(__virtbus_drv_register); > + > +/** > + * virtbus_drv_unregister - unregister a driver for virtual bus devices > + * @drv: virtbus_driver structure > + */ > +void virtbus_drv_unregister(struct virtbus_driver *vdrv) > +{ > + driver_unregister(&vdrv->driver); > +} > +EXPORT_SYMBOL_GPL(virtbus_drv_unregister);
On Fri, Feb 14, 2020 at 03:43:41PM -0500, Greg KH wrote: > On Fri, Feb 14, 2020 at 04:34:55PM -0400, Jason Gunthorpe wrote: > > On Fri, Feb 14, 2020 at 09:02:40AM -0800, Greg KH wrote: > > > > +/** > > > > + * virtbus_dev_register - add a virtual bus device > > > > + * @vdev: virtual bus device to add > > > > + */ > > > > +int virtbus_dev_register(struct virtbus_device *vdev) > > > > +{ > > > > + int ret; > > > > + > > > > + if (!vdev->release) { > > > > + dev_err(&vdev->dev, "virtbus_device .release callback NULL\n"); > > > > > > "virtbus_device MUST have a .release callback that does something!\n" > > > > > > > + return -EINVAL; > > > > + } > > > > + > > > > + device_initialize(&vdev->dev); > > > > + > > > > + vdev->dev.bus = &virtual_bus_type; > > > > + vdev->dev.release = virtbus_dev_release; > > > > + /* All device IDs are automatically allocated */ > > > > + ret = ida_simple_get(&virtbus_dev_ida, 0, 0, GFP_KERNEL); > > > > + if (ret < 0) { > > > > + dev_err(&vdev->dev, "get IDA idx for virtbus device failed!\n"); > > > > + put_device(&vdev->dev); > > > > > > If you allocate the number before device_initialize(), no need to call > > > put_device(). Just a minor thing, no big deal. > > > > If *_regster does put_device on error then it must always do > > put_device on any error, for instance the above return -EINVAL with > > no put_device leaks memory. > > That's why I said to move the ida_simple_get() call to before > device_initialize() is called. Once device_initialize() is called, you > HAVE to call put_device(). Yes put_device() becomes mandatory, but if the ida is moved up then the caller doesn't know how to handle an error: if (ida_simple_get() < 0) return -EINVAL; // caller must do kfree device_initialize(); if (device_register()) return -EINVAL // caller must do put_device If the device_initialize is bundled in the function the best answer is to always do device_initialize() and never do put_device(). The caller must realize the unwind switches from kfree to put_device (tricky and uglyifies the goto unwind!). This is the pattern something like platform_device_register() uses, and with a random survey I found only __ipmi_bmc_register() getting it right. Even then it seems to have a bug related to bmc_reg_mutex due to the ugly split goto unwind.. I prefer to see device_initialize done shortly after allocation, that seems to be the most likely to end up correct.. Jason
On Wed, Feb 12, 2020 at 11:14:00AM -0800, Jeff Kirsher wrote: > From: Dave Ertman <david.m.ertman@intel.com> > > This is the initial implementation of the Virtual Bus, > virtbus_device and virtbus_driver. The virtual bus is > a software based bus intended to support registering > virtbus_devices and virtbus_drivers and provide matching > between them and probing of the registered drivers. > > The bus will support probe/remove shutdown and > suspend/resume callbacks. > > Kconfig and Makefile alterations are included Can you please include the various sysfs paths that are generated by all of this for irdma in some commit message? I'm particularly interested to see how all the parentage turned out for the virtual device. Is PM going to work? IIRC PM will require that the virtual bus device goes through PM states in the right order relative to the PCI device. This all turned out OK? The concept certainly seems like what I imagined I'm not totally sold on 'virtual bus' as a name for this 'multi function pci device' thing, but you can work that out with Greg :) Thanks, Jason
On Fri, Feb 14, 2020 at 08:01:54PM -0400, Jason Gunthorpe wrote: > On Fri, Feb 14, 2020 at 03:43:41PM -0500, Greg KH wrote: > > On Fri, Feb 14, 2020 at 04:34:55PM -0400, Jason Gunthorpe wrote: > > > On Fri, Feb 14, 2020 at 09:02:40AM -0800, Greg KH wrote: > > > > > +/** > > > > > + * virtbus_dev_register - add a virtual bus device > > > > > + * @vdev: virtual bus device to add > > > > > + */ > > > > > +int virtbus_dev_register(struct virtbus_device *vdev) > > > > > +{ > > > > > + int ret; > > > > > + > > > > > + if (!vdev->release) { > > > > > + dev_err(&vdev->dev, "virtbus_device .release callback NULL\n"); > > > > > > > > "virtbus_device MUST have a .release callback that does something!\n" > > > > > > > > > + return -EINVAL; > > > > > + } > > > > > + > > > > > + device_initialize(&vdev->dev); > > > > > + > > > > > + vdev->dev.bus = &virtual_bus_type; > > > > > + vdev->dev.release = virtbus_dev_release; > > > > > + /* All device IDs are automatically allocated */ > > > > > + ret = ida_simple_get(&virtbus_dev_ida, 0, 0, GFP_KERNEL); > > > > > + if (ret < 0) { > > > > > + dev_err(&vdev->dev, "get IDA idx for virtbus device failed!\n"); > > > > > + put_device(&vdev->dev); > > > > > > > > If you allocate the number before device_initialize(), no need to call > > > > put_device(). Just a minor thing, no big deal. > > > > > > If *_regster does put_device on error then it must always do > > > put_device on any error, for instance the above return -EINVAL with > > > no put_device leaks memory. > > > > That's why I said to move the ida_simple_get() call to before > > device_initialize() is called. Once device_initialize() is called, you > > HAVE to call put_device(). > > Yes put_device() becomes mandatory, but if the ida is moved up then > the caller doesn't know how to handle an error: > > if (ida_simple_get() < 0) > return -EINVAL; // caller must do kfree > device_initialize(); > if (device_register()) > return -EINVAL // caller must do put_device No, call put_device() before returning. Ugh, anyway, this is all trivial stuff, the code is correct as-is, nevermind. If it bugs me enough, I'll send a patch that ends up removing one more line of code than adding :) greg k-h
> -----Original Message----- > From: Greg KH <gregkh@linuxfoundation.org> > Sent: Friday, February 14, 2020 12:45 PM > To: Jason Gunthorpe <jgg@ziepe.ca> > Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; davem@davemloft.net; > Ertman, David M <david.m.ertman@intel.com>; netdev@vger.kernel.org; > linux-rdma@vger.kernel.org; nhorman@redhat.com; > sassmann@redhat.com; parav@mellanox.com; galpress@amazon.com; > selvin.xavier@broadcom.com; sriharsha.basavapatna@broadcom.com; > benve@cisco.com; bharat@chelsio.com; xavier.huwei@huawei.com; > yishaih@mellanox.com; leonro@mellanox.com; mkalderon@marvell.com; > aditr@vmware.com; Patil, Kiran <kiran.patil@intel.com>; Bowers, AndrewX > <andrewx.bowers@intel.com> > Subject: Re: [RFC PATCH v4 01/25] virtual-bus: Implementation of Virtual Bus > > On Fri, Feb 14, 2020 at 04:34:55PM -0400, Jason Gunthorpe wrote: > > On Fri, Feb 14, 2020 at 09:02:40AM -0800, Greg KH wrote: > > > > + put_device(&vdev->dev); > > > > + ida_simple_remove(&virtbus_dev_ida, vdev->id); > > > > > > You need to do this before put_device(). > > > > Shouldn't it be in the release function? The ida index should not be > > re-used until the kref goes to zero.. The IDA should not be reused until the virtbus_device is unregistered. We don't want another device with the same name and same IDA to be registered, so the IDA has to remain unique until the device is unregistered, that is why I am moving it to before put_device, but remember, this index is just to ensure unique naming among the devices registered on the bus. There could (and will) be several foo_rdma devices created (one per PF) and we need to keep them all straight. > > Doesn't really matter, once you have unregistered it, you can reuse it. > But yes, putting it in release() is the safest thing to do. > > > > > +struct virtbus_device { > > > > + struct device dev; > > > > + const char *name; > > > > + void (*release)(struct virtbus_device *); > > > > + int id; > > > > + const struct virtbus_dev_id *matched_element; > > > > +}; > > > > > > Any reason you need to make "struct virtbus_device" a public structure > > > at all? > > > > The general point of this scheme is to do this in a public header: > > > > +struct iidc_virtbus_object { > > + struct virtbus_device vdev; > > + struct iidc_peer_dev *peer_dev; > > +}; > > > > And then this when the driver binds: > > Ah, yes, nevermind, I missed that. > > greg k-h -DaveE
On Thu, Feb 20, 2020 at 06:55:28PM +0000, Ertman, David M wrote: > > From: Greg KH <gregkh@linuxfoundation.org> > > Sent: Friday, February 14, 2020 12:45 PM > > To: Jason Gunthorpe <jgg@ziepe.ca> > > Cc: Kirsher, Jeffrey T <jeffrey.t.kirsher@intel.com>; davem@davemloft.net; > > Ertman, David M <david.m.ertman@intel.com>; netdev@vger.kernel.org; > > linux-rdma@vger.kernel.org; nhorman@redhat.com; > > sassmann@redhat.com; parav@mellanox.com; galpress@amazon.com; > > selvin.xavier@broadcom.com; sriharsha.basavapatna@broadcom.com; > > benve@cisco.com; bharat@chelsio.com; xavier.huwei@huawei.com; > > yishaih@mellanox.com; leonro@mellanox.com; mkalderon@marvell.com; > > aditr@vmware.com; Patil, Kiran <kiran.patil@intel.com>; Bowers, AndrewX > > <andrewx.bowers@intel.com> > > Subject: Re: [RFC PATCH v4 01/25] virtual-bus: Implementation of Virtual Bus > > > > On Fri, Feb 14, 2020 at 04:34:55PM -0400, Jason Gunthorpe wrote: > > > On Fri, Feb 14, 2020 at 09:02:40AM -0800, Greg KH wrote: > > > > > + put_device(&vdev->dev); > > > > > + ida_simple_remove(&virtbus_dev_ida, vdev->id); > > > > > > > > You need to do this before put_device(). > > > > > > Shouldn't it be in the release function? The ida index should not be > > > re-used until the kref goes to zero.. > > The IDA should not be reused until the virtbus_device is unregistered. We > don't want another device with the same name and same IDA to be > registered, Unregistration of the virtbus_device always happens before release. release is the point when the kref goes to zero and you can be confident there are no other threads reading the index or the device name. Remember, the put_device() above is not guarenteed to be the one that calls to release.. Jason
diff --git a/Documentation/driver-api/virtual_bus.rst b/Documentation/driver-api/virtual_bus.rst new file mode 100644 index 000000000000..5f35c19171d7 --- /dev/null +++ b/Documentation/driver-api/virtual_bus.rst @@ -0,0 +1,59 @@ +=============================== +Virtual Bus Devices and Drivers +=============================== + +See <linux/virtual_bus.h> for the models for virtbus_device and virtbus_driver. +This bus is meant to be a lightweight software based bus to attach generic +devices and drivers to so that a chunk of data can be passed between them. + +One use case example is an rdma driver needing to connect with several +different types of PCI LAN devices to be able to request resources from +them (queue sets). Each LAN driver that supports rdma will register a +virtbus_device on the virtual bus for each physical function. The rdma +driver will register as a virtbus_driver on the virtual bus to be +matched up with multiple virtbus_devices and receive a pointer to a +struct containing the callbacks that the PCI LAN drivers support for +registering with them. + +Sections in this document: + Virtbus devices + Virtbus drivers + Device Enumeration + Device naming and driver binding + Virtual Bus API entry points + +Virtbus devices +~~~~~~~~~~~~~~~ +Virtbus_devices support the minimal device functionality. Devices will +accept a name, and then, when added to the virtual bus, an automatically +generated index is concatenated onto it for the virtbus_device->name. + +Virtbus drivers +~~~~~~~~~~~~~~~ +Virtbus drivers register with the virtual bus to be matched with virtbus +devices. They expect to be registered with a probe and remove callback, +and also support shutdown, suspend, and resume callbacks. They otherwise +follow the standard driver behavior of having discovery and enumeration +handled in the bus infrastructure. + +Virtbus drivers register themselves with the API entry point virtbus_drv_reg +and unregister with virtbus_drv_unreg. + +Device Enumeration +~~~~~~~~~~~~~~~~~~ +Enumeration is handled automatically by the bus infrastructure via the +ida_simple methods. + +Device naming and driver binding +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +The virtbus_device.dev.name is the canonical name for the device. It is +built from two other parts: + + - virtbus_device.name (also used for matching). + - virtbus_device.id (generated automatically from ida_simple calls) + +Virtbus device IDs are always in "<name>.<instance>" format. Instances are +automatically selected through an ida_simple_get so are positive integers. +Names are taken from the device name field. Driver IDs are simple <name>. +Need to extract the name from the Virtual Device compare to name of the +driver. diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig index 6095b6df8a81..2e8b89c1761a 100644 --- a/drivers/bus/Kconfig +++ b/drivers/bus/Kconfig @@ -202,4 +202,15 @@ config DA8XX_MSTPRI source "drivers/bus/fsl-mc/Kconfig" +config VIRTUAL_BUS + tristate "Software based Virtual Bus" + help + Provides a software bus for virtbus_devices to be added to it + and virtbus_drivers to be registered on it. Will create a match + between the driver and device, then call the driver's probe with + the virtbus_device's struct. + One example is the irdma driver needing to connect with various + PCI LAN drivers to request resources (queues) to be able to perform + its function. + endmenu diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile index 1320bcf9fa9d..6721c77dc71b 100644 --- a/drivers/bus/Makefile +++ b/drivers/bus/Makefile @@ -34,3 +34,4 @@ obj-$(CONFIG_UNIPHIER_SYSTEM_BUS) += uniphier-system-bus.o obj-$(CONFIG_VEXPRESS_CONFIG) += vexpress-config.o obj-$(CONFIG_DA8XX_MSTPRI) += da8xx-mstpri.o +obj-$(CONFIG_VIRTUAL_BUS) += virtual_bus.o diff --git a/drivers/bus/virtual_bus.c b/drivers/bus/virtual_bus.c new file mode 100644 index 000000000000..85d2dbfa3376 --- /dev/null +++ b/drivers/bus/virtual_bus.c @@ -0,0 +1,267 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * virtual_bus.c - lightweight software based bus for virtual devices + * + * Copyright (c) 2019-20 Intel Corporation + * + * Please see Documentation/driver-api/virtual_bus.rst for + * more information + */ + +#include <linux/string.h> +#include <linux/virtual_bus.h> +#include <linux/of_irq.h> +#include <linux/module.h> +#include <linux/init.h> +#include <linux/pm_runtime.h> +#include <linux/pm_domain.h> +#include <linux/acpi.h> +#include <linux/device.h> + +MODULE_LICENSE("GPL v2"); +MODULE_DESCRIPTION("Lightweight Virtual Bus"); +MODULE_AUTHOR("David Ertman <david.m.ertman@intel.com>"); +MODULE_AUTHOR("Kiran Patil <kiran.patil@intel.com>"); + +static DEFINE_IDA(virtbus_dev_ida); + +static const +struct virtbus_dev_id *virtbus_match_id(const struct virtbus_dev_id *id, + struct virtbus_device *vdev) +{ + while (id->name[0]) { + if (!strcmp(vdev->name, id->name)) { + vdev->matched_element = id; + return id; + } + id++; + } + return NULL; +} + +static int virtbus_match(struct device *dev, struct device_driver *drv) +{ + struct virtbus_driver *vdrv = to_virtbus_drv(drv); + struct virtbus_device *vdev = to_virtbus_dev(dev); + + return virtbus_match_id(vdrv->id_table, vdev) != NULL; +} + +static int virtbus_probe(struct device *dev) +{ + return dev->driver->probe(dev); +} + +static int virtbus_remove(struct device *dev) +{ + return dev->driver->remove(dev); +} + +static void virtbus_shutdown(struct device *dev) +{ + dev->driver->shutdown(dev); +} + +static int virtbus_suspend(struct device *dev, pm_message_t state) +{ + if (dev->driver->suspend) + return dev->driver->suspend(dev, state); + + return 0; +} + +static int virtbus_resume(struct device *dev) +{ + if (dev->driver->resume) + return dev->driver->resume(dev); + + return 0; +} + +struct bus_type virtual_bus_type = { + .name = "virtbus", + .match = virtbus_match, + .probe = virtbus_probe, + .remove = virtbus_remove, + .shutdown = virtbus_shutdown, + .suspend = virtbus_suspend, + .resume = virtbus_resume, +}; + +/** + * virtbus_dev_release - Destroy a virtbus device + * @vdev: virtual device to release + */ +static void virtbus_dev_release(struct device *_dev) +{ + struct virtbus_device *vdev = to_virtbus_dev(_dev); + + ida_simple_remove(&virtbus_dev_ida, vdev->id); + vdev->release(vdev); +} + +/** + * virtbus_dev_register - add a virtual bus device + * @vdev: virtual bus device to add + */ +int virtbus_dev_register(struct virtbus_device *vdev) +{ + int ret; + + if (!vdev->release) { + dev_err(&vdev->dev, "virtbus_device .release callback NULL\n"); + return -EINVAL; + } + + device_initialize(&vdev->dev); + + vdev->dev.bus = &virtual_bus_type; + vdev->dev.release = virtbus_dev_release; + /* All device IDs are automatically allocated */ + ret = ida_simple_get(&virtbus_dev_ida, 0, 0, GFP_KERNEL); + if (ret < 0) { + dev_err(&vdev->dev, "get IDA idx for virtbus device failed!\n"); + put_device(&vdev->dev); + return ret; + } + + vdev->id = ret; + dev_set_name(&vdev->dev, "%s.%d", vdev->name, vdev->id); + + dev_dbg(&vdev->dev, "Registering virtbus device '%s'\n", + dev_name(&vdev->dev)); + + ret = device_add(&vdev->dev); + if (ret) + goto device_add_err; + + return 0; + +device_add_err: + dev_err(&vdev->dev, "Add device to virtbus failed!\n"); + put_device(&vdev->dev); + ida_simple_remove(&virtbus_dev_ida, vdev->id); + + return ret; +} +EXPORT_SYMBOL_GPL(virtbus_dev_register); + +/** + * virtbus_dev_unregister - remove a virtual bus device + * vdev: virtual bus device we are removing + */ +void virtbus_dev_unregister(struct virtbus_device *vdev) +{ + device_del(&vdev->dev); + put_device(&vdev->dev); +} +EXPORT_SYMBOL_GPL(virtbus_dev_unregister); + +static int virtbus_drv_probe(struct device *_dev) +{ + struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver); + struct virtbus_device *vdev = to_virtbus_dev(_dev); + int ret; + + ret = dev_pm_domain_attach(_dev, true); + if (ret) { + dev_warn(_dev, "Failed to attatch to PM Domain : %d\n", ret); + return ret; + } + + ret = vdrv->probe(vdev); + if (ret) { + dev_err(&vdev->dev, "Probe returned error\n"); + dev_pm_domain_detach(_dev, true); + } + + return ret; +} + +static int virtbus_drv_remove(struct device *_dev) +{ + struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver); + struct virtbus_device *vdev = to_virtbus_dev(_dev); + int ret = 0; + + ret = vdrv->remove(vdev); + dev_pm_domain_detach(_dev, true); + + return ret; +} + +static void virtbus_drv_shutdown(struct device *_dev) +{ + struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver); + struct virtbus_device *vdev = to_virtbus_dev(_dev); + + vdrv->shutdown(vdev); +} + +static int virtbus_drv_suspend(struct device *_dev, pm_message_t state) +{ + struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver); + struct virtbus_device *vdev = to_virtbus_dev(_dev); + + if (vdrv->suspend) + return vdrv->suspend(vdev, state); + + return 0; +} + +static int virtbus_drv_resume(struct device *_dev) +{ + struct virtbus_driver *vdrv = to_virtbus_drv(_dev->driver); + struct virtbus_device *vdev = to_virtbus_dev(_dev); + + if (vdrv->resume) + return vdrv->resume(vdev); + + return 0; +} + +/** + * __virtbus_drv_register - register a driver for virtual bus devices + * @vdrv: virtbus_driver structure + * @owner: owning module/driver + */ +int __virtbus_drv_register(struct virtbus_driver *vdrv, struct module *owner) +{ + if (!vdrv->probe || !vdrv->remove || !vdrv->shutdown || !vdrv->id_table) + return -EINVAL; + + vdrv->driver.owner = owner; + vdrv->driver.bus = &virtual_bus_type; + vdrv->driver.probe = virtbus_drv_probe; + vdrv->driver.remove = virtbus_drv_remove; + vdrv->driver.shutdown = virtbus_drv_shutdown; + vdrv->driver.suspend = virtbus_drv_suspend; + vdrv->driver.resume = virtbus_drv_resume; + + return driver_register(&vdrv->driver); +} +EXPORT_SYMBOL_GPL(__virtbus_drv_register); + +/** + * virtbus_drv_unregister - unregister a driver for virtual bus devices + * @drv: virtbus_driver structure + */ +void virtbus_drv_unregister(struct virtbus_driver *vdrv) +{ + driver_unregister(&vdrv->driver); +} +EXPORT_SYMBOL_GPL(virtbus_drv_unregister); + +static int __init virtual_bus_init(void) +{ + return bus_register(&virtual_bus_type); +} + +static void __exit virtual_bus_exit(void) +{ + bus_unregister(&virtual_bus_type); + ida_destroy(&virtbus_dev_ida); +} + +module_init(virtual_bus_init); +module_exit(virtual_bus_exit); diff --git a/include/linux/mod_devicetable.h b/include/linux/mod_devicetable.h index e3596db077dc..442f82128a2f 100644 --- a/include/linux/mod_devicetable.h +++ b/include/linux/mod_devicetable.h @@ -821,4 +821,12 @@ struct wmi_device_id { const void *context; }; +#define VIRTBUS_NAME_SIZE 20 +#define VIRTBUS_MODULE_PREFIX "virtbus:" + +struct virtbus_dev_id { + char name[VIRTBUS_NAME_SIZE]; + kernel_ulong_t driver_data; +}; + #endif /* LINUX_MOD_DEVICETABLE_H */ diff --git a/include/linux/virtual_bus.h b/include/linux/virtual_bus.h new file mode 100644 index 000000000000..2cbc0e72e182 --- /dev/null +++ b/include/linux/virtual_bus.h @@ -0,0 +1,57 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * virtual_bus.h - lightweight software bus + * + * Copyright (c) 2019-20 Intel Corporation + * + * Please see Documentation/driver-api/virtual_bus.rst for more information + */ + +#ifndef _VIRTUAL_BUS_H_ +#define _VIRTUAL_BUS_H_ + +#include <linux/device.h> + +struct virtbus_device { + struct device dev; + const char *name; + void (*release)(struct virtbus_device *); + int id; + const struct virtbus_dev_id *matched_element; +}; + +/* The memory for the table is expected to remain allocated for the duration + * of the pairing between driver and device. The pointer for the matching + * element will be copied to the matched_element field of the virtbus_device. + */ +struct virtbus_driver { + int (*probe)(struct virtbus_device *); + int (*remove)(struct virtbus_device *); + void (*shutdown)(struct virtbus_device *); + int (*suspend)(struct virtbus_device *, pm_message_t); + int (*resume)(struct virtbus_device *); + struct device_driver driver; + const struct virtbus_dev_id *id_table; +}; + +static inline +struct virtbus_device *to_virtbus_dev(struct device *dev) +{ + return container_of(dev, struct virtbus_device, dev); +} + +static inline +struct virtbus_driver *to_virtbus_drv(struct device_driver *drv) +{ + return container_of(drv, struct virtbus_driver, driver); +} + +int virtbus_dev_register(struct virtbus_device *vdev); +void virtbus_dev_unregister(struct virtbus_device *vdev); +int __virtbus_drv_register(struct virtbus_driver *vdrv, struct module *owner); +void virtbus_drv_unregister(struct virtbus_driver *vdrv); + +#define virtbus_drv_register(vdrv) \ + __virtbus_drv_register(vdrv, THIS_MODULE) + +#endif /* _VIRTUAL_BUS_H_ */ diff --git a/scripts/mod/devicetable-offsets.c b/scripts/mod/devicetable-offsets.c index 054405b90ba4..9a6099bf90c8 100644 --- a/scripts/mod/devicetable-offsets.c +++ b/scripts/mod/devicetable-offsets.c @@ -231,5 +231,8 @@ int main(void) DEVID(wmi_device_id); DEVID_FIELD(wmi_device_id, guid_string); + DEVID(virtbus_dev_id); + DEVID_FIELD(virtbus_dev_id, name); + return 0; } diff --git a/scripts/mod/file2alias.c b/scripts/mod/file2alias.c index c91eba751804..713fdfe010b0 100644 --- a/scripts/mod/file2alias.c +++ b/scripts/mod/file2alias.c @@ -1335,6 +1335,13 @@ static int do_wmi_entry(const char *filename, void *symval, char *alias) return 1; } +static int do_virtbus_entry(const char *filename, void *symval, char *alias) +{ + DEF_FIELD_ADDR(symval, virtbus_dev_id, name); + sprintf(alias, VIRTBUS_MODULE_PREFIX "%s", *name); + return 1; +} + /* Does namelen bytes of name exactly match the symbol? */ static bool sym_is(const char *name, unsigned namelen, const char *symbol) { @@ -1407,6 +1414,7 @@ static const struct devtable devtable[] = { {"typec", SIZE_typec_device_id, do_typec_entry}, {"tee", SIZE_tee_client_device_id, do_tee_entry}, {"wmi", SIZE_wmi_device_id, do_wmi_entry}, + {"virtbus", SIZE_virtbus_dev_id, do_virtbus_entry}, }; /* Create MODULE_ALIAS() statements.