Re: [PATCH 08/10] vfio: Make vfio_device_ops pass a 'struct vfio_device *' instead of 'void *'

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Mar 09, 2021 at 09:52:30PM -0800, Dan Williams wrote:
> On Tue, Mar 9, 2021 at 1:39 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
> >
> > This is the standard kernel pattern, the ops associated with a struct get
> > the struct pointer in for typesafety. The expected design is to use
> > container_of to cleanly go from the subsystem level type to the driver
> > level type without having any type erasure in a void *.
> 
> This patch alone is worth the price of admission.

Right, this is how I got here as I was going to do the mdev stuff and
we end up doing transformations like this:

-static long mtty_ioctl(struct mdev_device *mdev, unsigned int cmd,
+static long mtty_ioctl(struct vfio_device *vdev, unsigned int cmd,
 			unsigned long arg)

And if the 'struct vfio_device *vdev' was left a 'void *' then the
compiler doesn't get to help any more :(

> Seems like it would be worth adding
> to_vfio_{pci,platform,fsl_mc}_device() helpers in this patch as well.

I have mixed feelings on these one-liners. If people feel they are
worthwhile I'll add them

> I've sometimes added runtime type safety to to_* helpers for early
> warning of mistakes that happen when refactoring...
> 
> static inline struct vfio_pci_device *
> to_vfio_pci_device(struct vfio_device *core_dev)
> {
>         if (dev_WARN_ONCE(core_dev->dev, core_dev->ops != &vfio_pci_ops,
>                           "not a vfio_pci_device!\n"))
>                 return NULL;
>         return container_of(core_vdev, struct vfio_pci_device, vdev);

In this case I don't think we need to worry as everything is tidy in a
single module such that the compilation units can't see other
container_of options anyhow and vfio core isn't going to accidently
call an ops with the wrong type.

Jason