[PATCH 00/10] Embed struct vfio_device in all sub-structures

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Prologue
========

This series is part of a larger work that arose from the minor remark that
the mdev_parent_ops indirection shim is useless and complicates
things.

The entire project is about 70 patches broken into 5 subseries, each on a
theme:

#1 - (this series) Add type safety to the core VFIO
#2 - Add type safety to MDEV

  The mdev transformation is involved, compiler assistance through actual
  static type checking makes the transformation much more reliable, thus
  the first two steps add most of the missing types.

#3 - Make all mdev drivers register directly with the core code,
     delete vfio_mdev.c

#4 - Various minor tidies that arise from the above three series

#5 - Complete type annotations and remove unused code

A preview of the future series's is here:
  https://github.com/jgunthorpe/linux/pull/3/commits

It turns out a bunch of stuff exists in the way it does because the
'struct vfio_device' was not obviously available in places that naturally
wanted it. Across the project the following APIs are deleted as reorg
removes all the users:

   mdev_uuid()
   mdev_dev()
   mdev_get_drvdata()
   mdev_set_drvdata()
   struct mdev_parent_ops
   vfio_iommu_group_get()
   vfio_iommu_group_put(),
   vfio_group_get_external_user_from_dev()
   vfio_group_pin_pages()
   vfio_group_unpin_pages()
   vfio_group_get()
   vfio_device_data()

The remaining vfio_device related APIs in mdev.h and vfio.h have correct,
specific, types instead of 'void *' or 'struct device *'.

This work is related to, but seperate from, Max's series to split
vfio_pci. When layered on this vfio_pci_core will use a similiar
container_of scheme and layer the ultimate end-driver with container_of
all the way back to a vfio_device. Types are explicit and natural to
understand through all the layers.

Further mdev and pci get a similiar design with a set of core code
supporting normal 'struct device_driver's that directly create
vfio_device's.

In essence vfio becomes close to a normal driver subsystem pattern with a
bunch of device drivers creating vfio_devices'

========
This series:

The main focus of this series is to make VFIO follow the normal kernel
convention of structure embedding for structure inheritance instead of
linking using a 'void *opaque'. Here we focus on moving the vfio_device to
be a member of every struct vfio_XX_device that is linked by a
vfio_add_group_dev().

In turn this allows 'struct vfio_device *' to be used everwhere, and the
public API out of vfio.c can be cleaned to remove places using 'struct
device *' and 'void *' as surrogates to refer to the device.

While this has the minor trade off of moving 'struct vfio_device' the
clarity of the design is worth it. I can speak directly to this idea, as
I've invested a fair amount of time carefully working backwards what all
the type-erased APIs are supposed to be and it is certainly not trivial or
intuitive.

When we get into mdev land things become even more inscrutable, and while
I now have a pretty clear picture, it was hard to obtain. I think this
agrees with the kernel style ideal of being explicit in typing and not
sacrificing clarity to create opaque structs.

After this series the general rules are:
 - Any vfio_XX_device * can be obtained at no cost from a vfio_device *
   using container_of(), and the reverse is possible by &XXdev->vdev

   This is similar to how 'struct pci_device' and 'struct device' are
   interrelated.

   This allows 'device_data' to be completely removed from the vfio.c API.

 - The drvdata for a struct device points at the vfio_XX_device that
   belongs to the driver that was probed. drvdata is removed from the core
   code, and only used as part of the implementation of the struct
   device_driver.

 - The lifetime of vfio_XX_device and vfio_device are identical, they are
   the same memory.

   This follows the existing model where vfio_del_group_dev() blocks until
   all vfio_device_put()'s are completed. This in turn means the struct
   device_driver remove() blocks, and thus under the driver_lock() a bound
   driver must have a valid drvdata pointing at both vfio device
   structs. A following series exploits this further.

Most vfio_XX_device structs have data that duplicates the 'struct
device *dev' member of vfio_device, a following series removes that
duplication too.

Jason

Jason Gunthorpe (10):
  vfio: Simplify the lifetime logic for vfio_device
  vfio: Split creation of a vfio_device into init and register ops
  vfio/platform: Use vfio_init/register/unregister_group_dev
  vfio/fsl-mc: Use vfio_init/register/unregister_group_dev
  vfio/pci: Use vfio_init/register/unregister_group_dev
  vfio/mdev: Use vfio_init/register/unregister_group_dev
  vfio/mdev: Make to_mdev_device() into a static inline
  vfio: Make vfio_device_ops pass a 'struct vfio_device *' instead of
    'void *'
  vfio/pci: Replace uses of vfio_device_data() with container_of
  vfio: Remove device_data from the vfio bus driver API

 Documentation/driver-api/vfio.rst             |  48 ++--
 drivers/vfio/fsl-mc/vfio_fsl_mc.c             |  69 +++---
 drivers/vfio/fsl-mc/vfio_fsl_mc_private.h     |   1 +
 drivers/vfio/mdev/mdev_private.h              |   5 +-
 drivers/vfio/mdev/vfio_mdev.c                 |  57 +++--
 drivers/vfio/pci/vfio_pci.c                   | 109 +++++----
 drivers/vfio/pci/vfio_pci_private.h           |   1 +
 drivers/vfio/platform/vfio_amba.c             |   8 +-
 drivers/vfio/platform/vfio_platform.c         |  21 +-
 drivers/vfio/platform/vfio_platform_common.c  |  56 ++---
 drivers/vfio/platform/vfio_platform_private.h |   5 +-
 drivers/vfio/vfio.c                           | 210 ++++++------------
 include/linux/vfio.h                          |  37 +--
 13 files changed, 299 insertions(+), 328 deletions(-)

-- 
2.30.1




[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux