On Thu, Jun 04, 2020 at 05:01:06PM +0200, Cornelia Huck wrote:
> On Sun, 17 May 2020 22:45:10 -0400
> Yan Zhao <yan.y.zhao@xxxxxxxxx> wrote:
>
> > vendor modules call macro module_vfio_pci_register_vendor_handler to
> > generate module_init and module_exit.
> > It is necessary to ensure that vendor modules always call
> > vfio_pci_register_vendor_driver() on driver loading and
> > vfio_pci_unregister_vendor_driver on driver unloading,
> > because
> > (1) at compiling time, there's only a dependency of vendor modules on
> > vfio_pci.
> > (2) at runtime,
> > - vendor modules add refs of vfio_pci on a successful calling of
> > vfio_pci_register_vendor_driver() and deref of vfio_pci on a
> > successful calling of vfio_pci_unregister_vendor_driver().
> > - vfio_pci only adds refs of vendor module on a successful probe of vendor
> > driver.
> > vfio_pci derefs vendor module when unbinding from a device.
> >
> > So, after vfio_pci is unbound from a device, the vendor module to that
> > device is free to get unloaded. However, if that vendor module does not
> > call vfio_pci_unregister_vendor_driver() in its module_exit, vfio_pci may
> > hold a stale pointer to vendor module.
> >
> > Cc: Kevin Tian <kevin.tian@xxxxxxxxx>
> > Suggested-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> > Signed-off-by: Yan Zhao <yan.y.zhao@xxxxxxxxx>
> > ---
> > include/linux/vfio.h | 27 +++++++++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
> >
> > diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> > index 3e53deb012b6..f3746608c2d9 100644
> > --- a/include/linux/vfio.h
> > +++ b/include/linux/vfio.h
> > @@ -223,4 +223,31 @@ struct vfio_pci_vendor_driver_ops {
> > };
> > int __vfio_pci_register_vendor_driver(struct vfio_pci_vendor_driver_ops *ops);
> > void vfio_pci_unregister_vendor_driver(struct vfio_device_ops *device_ops);
> > +
> > +#define vfio_pci_register_vendor_driver(__name, __probe, __remove, \
> > + __device_ops) \
> > +static struct vfio_pci_vendor_driver_ops __ops ## _node = { \
> > + .owner = THIS_MODULE, \
> > + .name = __name, \
> > + .probe = __probe, \
> > + .remove = __remove, \
> > + .device_ops = __device_ops, \
> > +}; \
> > +__vfio_pci_register_vendor_driver(&__ops ## _node)
> > +
> > +#define module_vfio_pci_register_vendor_handler(name, probe, remove, \
> > + device_ops) \
> > +static int __init device_ops ## _module_init(void) \
> > +{ \
> > + vfio_pci_register_vendor_driver(name, probe, remove, \
> > + device_ops); \
>
> What if this function fails (e.g. with -ENOMEM)?
>
right. I need to return error in that case.
Thanks for pointing it out!
Yan
> > + return 0; \
> > +}; \
> > +static void __exit device_ops ## _module_exit(void) \
> > +{ \
> > + vfio_pci_unregister_vendor_driver(device_ops); \
> > +}; \
> > +module_init(device_ops ## _module_init); \
> > +module_exit(device_ops ## _module_exit)
> > +
> > #endif /* VFIO_H */
>
