RE: [PATCH v7 00/22] Add vfio_device cdev for iommufd support

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

 



> -----Original Message-----
> From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
> Subject: [PATCH v7 00/22] Add vfio_device cdev for iommufd support
> 
> Existing VFIO provides group-centric user APIs for userspace. Userspace
> opens the /dev/vfio/$group_id first before getting device fd and hence
> getting access to device. This is not the desired model for iommufd. Per the
> conclusion of community discussion[1], iommufd provides device-centric
> kAPIs and requires its consumer (like VFIO) to be device-centric user APIs.
> Such user APIs are used to associate device with iommufd and also the I/O
> address spaces managed by the iommufd.
> 
> This series first introduces a per device file structure to be prepared for
> further enhancement and refactors the kvm-vfio code to be prepared for
> accepting device file from userspace. Afte this, adds a mechanism for
> blocking device access before iommufd bind. Then refactors the vfio to be
> able to handle cdev path (e.g. iommufd binding, no-iommufd, [de]attach
> ioas).
> This refactor includes making the device_open exclusive between group and
> cdev path, only allow single device open in cdev path; vfio-iommufd code is
> also refactored to support cdev. e.g. split the vfio_iommufd_bind() into two
> steps. Eventually, adds the cdev support for vfio device and the new ioctls,
> then makes group infrastructure optional as it is not needed when vfio
> device cdev is compiled.
> 
> This series is based on some preparation works done to vfio emulated
> devices[2] and vfio pci hot reset enhancements[3].
> 
> This series is a prerequisite for iommu nesting for vfio device[4] [5].
> 
> The complete code can be found in below branch, simple tests done to the
> legacy group path and the cdev path. Draft QEMU branch can be found at[6]
> 
> https://github.com/yiliu1765/iommufd/tree/vfio_device_cdev_v7
> (config CONFIG_IOMMUFD=y CONFIG_VFIO_DEVICE_CDEV=y)
> 
> base-commit: d28283a0c30d2f3c82d78fbe27f258671b6dc535
> 
> [1]
> https://lore.kernel.org/kvm/BN9PR11MB5433B1E4AE5B0480369F97178C189
> @BN9PR11MB5433.namprd11.prod.outlook.com/
> [2] https://lore.kernel.org/kvm/20230316121526.5644-1-yi.l.liu@xxxxxxxxx/
> [3] https://lore.kernel.org/kvm/20230316124156.12064-1-yi.l.liu@xxxxxxxxx/
> [4] https://lore.kernel.org/linux-iommu/20230309080910.607396-1-
> yi.l.liu@xxxxxxxxx/
> [5] https://lore.kernel.org/linux-iommu/20230309082207.612346-1-
> yi.l.liu@xxxxxxxxx/
> [6] https://github.com/yiliu1765/qemu/tree/iommufd_rfcv3 (it is based on
> Eric's
>     QEMU iommufd rfcv3
> (https://lore.kernel.org/kvm/20230131205305.2726330-1-
> eric.auger@xxxxxxxxxx/)
>     plus commits to align with vfio_device_cdev v7)
> 
> Change log:
> 
> v7:
>  - Split the vfio-pci hot reset changes to be separate patch series (Jason,
> Kevin)
>  - More polish on no-iommufd support (patch 11 - 13) in cdev path (Kevin)
>  - iommufd_access_detach() in patch 16 is added by Nic for emulated devices
> (Kevin, Jason)
> 
> v6: https://lore.kernel.org/kvm/20230308132903.465159-1-
> yi.l.liu@xxxxxxxxx/#t
>  - Add r-b from Jason on patch 01 - 08 and 13 in v5
>  - Based on the prerequisite mini-series which makes vfio emulated devices
>    be prepared to cdev (Jason)
>  - Add the approach to pass a set of device fds to do hot reset ownership
>    check, while the zero-length array approach is also kept. (Jason, Kevin, Alex)
>  - Drop patch 10 of v5, it is reworked by patch 13 and 17 in v6 (Jason)
>  - Store vfio_group pointer in vfio_device_file to check if user is using
>    legacy vfio container (Jason)
>  - Drop the is_cdev_device flag (introduced in patch 14 of v5) as the group
>    pointer stored in vfio_device_file can cover it.
>  - Add iommu_group check in the cdev no-iommu path patch 24 (Kevin)
>  - Add t-b from Terrence, Nicolin and Matthew (thanks for the help, some
> patches
>    are new in this version, so I just added t-b to the patches that are also
>    in v5 and no big change, for others would add in this version).
> 
> v5: https://lore.kernel.org/kvm/20230227111135.61728-1-yi.l.liu@xxxxxxxxx/
>  - Add r-b from Kevin on patch 08, 13, 14, 15 and 17.
>  - Rename patch 02 to limit the change for KVM facing kAPIs. The vfio pci
>    hot reset path only accepts group file until patch 09. (Kevin)
>  - Update comment around smp_load_acquire(&df->access_granted) (Yan)
>  - Adopt Jason's suggestion on the vfio pci hot reset path, passing zero-length
>    fd array to indicate using bound iommufd_ctx as ownership check. (Jason,
> Kevin)
>  - Direct read df->access_granted value in vfio_device_cdev_close() (Kevin,
> Yan, Jason)
>  - Wrap the iommufd get/put into a helper to refine the error path of
>    vfio_device_ioctl_bind_iommufd(). (Yan)
> 
> v4: https://lore.kernel.org/kvm/20230221034812.138051-1-yi.l.liu@xxxxxxxxx/
>  - Add r-b from Kevin on patch 09/10
>  - Add a line in devices/vfio.rst to emphasize user should add group/device to
>    KVM prior to invoke open_device op which may be called in the
> VFIO_GROUP_GET_DEVICE_FD
>    or VFIO_DEVICE_BIND_IOMMUFD ioctl.
>  - Modify VFIO_GROUP/VFIO_DEVICE_CDEV Kconfig dependency (Alex)
>  - Select VFIO_GROUP for SPAPR (Jason)
>  - Check device fully-opened in PCI hotreset path for device fd (Jason)
>  - Set df->access_granted in the caller of vfio_device_open() since
>    the caller may fail in other operations, but df->access_granted
>    does not allow a true to false change. So it should be set only when
>    the open path is really done successfully. (Yan, Kevin)
>  - Fix missing iommufd_ctx_put() in the cdev path (Yan)
>  - Fix an issue found in testing exclusion between group and cdev path.
>    vfio_device_cdev_close() should check df->access_granted before heading
>    to other operations.
>  - Update vfio.rst for iommufd/cdev
> 
> v3: https://lore.kernel.org/kvm/20230213151348.56451-1-yi.l.liu@xxxxxxxxx/
>  - Add r-b from Kevin on patch 03, 06, 07, 08.
>  - Refine the group and cdev path exclusion. Remove
> vfio_device:single_open;
>    add vfio_group::cdev_device_open_cnt to achieve exlucsion between
> group
>    path and cdev path (Kevin, Jason)
>  - Fix a bug in the error handling path (Yan Zhao)
>  - Address misc remarks from Kevin
> 
> v2: https://lore.kernel.org/kvm/20230206090532.95598-1-yi.l.liu@xxxxxxxxx/
>  - Add r-b from Kevin and Eric on patch 01 02 04.
>  - "Split kvm/vfio: Provide struct kvm_device_ops::release() insted
> of ::destroy()"
>    from this series and got applied. (Alex, Kevin, Jason, Mathhew)
>  - Add kvm_ref_lock to protect vfio_device_file->kvm instead of reusing
>    dev_set->lock as dead-lock is observed with vfio-ap which would try to
>    acquire kvm_lock. This is opposite lock order with kvm_device_release()
>    which holds kvm_lock first and then hold dev_set->lock. (Kevin)
>  - Use a separate ioctl for detaching IOAS. (Alex)
>  - Rename vfio_device_file::single_open to be is_cdev_device (Kevin, Alex)
>  - Move the vfio device cdev code into device_cdev.c and add a
> VFIO_DEVICE_CDEV
>    kconfig for it. (Kevin, Jason)
> 
> v1: https://lore.kernel.org/kvm/20230117134942.101112-1-yi.l.liu@xxxxxxxxx/
>  - Fix the circular refcount between kvm struct and device file reference.
> (JasonG)
>  - Address comments from KevinT
>  - Remained the ioctl for detach, needs to Alex's taste
> 
> (https://lore.kernel.org/kvm/BN9PR11MB5276BE9F4B0613EE859317028CFF9
> @BN9PR11MB5276.namprd11.prod.outlook.com/)
> 
> rfc: https://lore.kernel.org/kvm/20221219084718.9342-1-yi.l.liu@xxxxxxxxx/
> 
> Thanks,
> 	Yi Liu
> 
> Nicolin Chen (1):
>   iommufd/device: Add iommufd_access_detach() API
> 
> Yi Liu (21):
>   vfio: Allocate per device file structure
>   vfio: Refine vfio file kAPIs for KVM
>   vfio: Remove vfio_file_is_group()
>   vfio: Accept vfio device file in the KVM facing kAPI
>   kvm/vfio: Rename kvm_vfio_group to prepare for accepting vfio device
>     fd
>   kvm/vfio: Accept vfio device file from userspace
>   vfio: Pass struct vfio_device_file * to vfio_device_open/close()
>   vfio: Block device access via device fd until device is opened
>   vfio: Add cdev_device_open_cnt to vfio_group
>   vfio: Make vfio_device_open() single open for device cdev path
>   vfio: Make vfio_device_first_open() to accept NULL iommufd for noiommu
>   vfio-iommufd: Move noiommu support out of vfio_iommufd_bind()
>   vfio-iommufd: Split bind/attach into two steps
>   vfio: Record devid in vfio_device_file
>   vfio-iommufd: Add detach_ioas support for physical VFIO devices
>   vfio-iommufd: Add detach_ioas support for emulated VFIO devices
>   vfio: Add cdev for vfio_device
>   vfio: Add VFIO_DEVICE_BIND_IOMMUFD
>   vfio: Add VFIO_DEVICE_AT[DE]TACH_IOMMUFD_PT
>   vfio: Compile group optionally
>   docs: vfio: Add vfio device cdev description
> 
>  Documentation/driver-api/vfio.rst             | 133 +++++++-
>  Documentation/virt/kvm/devices/vfio.rst       |  52 ++-
>  drivers/gpu/drm/i915/gvt/kvmgt.c              |   1 +
>  drivers/iommu/iommufd/Kconfig                 |   4 +-
>  drivers/iommu/iommufd/device.c                |  75 ++++-
>  drivers/iommu/iommufd/iommufd_private.h       |   2 +
>  drivers/s390/cio/vfio_ccw_ops.c               |   1 +
>  drivers/s390/crypto/vfio_ap_ops.c             |   1 +
>  drivers/vfio/Kconfig                          |  27 +-
>  drivers/vfio/Makefile                         |   3 +-
>  drivers/vfio/device_cdev.c                    | 300 ++++++++++++++++++
>  drivers/vfio/fsl-mc/vfio_fsl_mc.c             |   1 +
>  drivers/vfio/group.c                          | 154 ++++++---
>  drivers/vfio/iommufd.c                        | 104 +++---
>  .../vfio/pci/hisilicon/hisi_acc_vfio_pci.c    |   2 +
>  drivers/vfio/pci/mlx5/main.c                  |   1 +
>  drivers/vfio/pci/vfio_pci.c                   |   1 +
>  drivers/vfio/platform/vfio_amba.c             |   1 +
>  drivers/vfio/platform/vfio_platform.c         |   1 +
>  drivers/vfio/vfio.h                           | 237 +++++++++++++-
>  drivers/vfio/vfio_main.c                      | 227 ++++++++++---
>  include/linux/iommufd.h                       |   1 +
>  include/linux/vfio.h                          |  29 +-
>  include/uapi/linux/kvm.h                      |  16 +-
>  include/uapi/linux/vfio.h                     |  89 ++++++
>  samples/vfio-mdev/mbochs.c                    |   1 +
>  samples/vfio-mdev/mdpy.c                      |   1 +
>  samples/vfio-mdev/mtty.c                      |   1 +
>  virt/kvm/vfio.c                               | 141 ++++----
>  29 files changed, 1361 insertions(+), 246 deletions(-)  create mode 100644
> drivers/vfio/device_cdev.c
> 
> --
> 2.34.1

Tested GVT-g / GVT-d VFIO legacy mode / compat mode / cdev mode, including negative tests. No regression be introduced.  

Tested-by: Terrence Xu <terrence.xu@xxxxxxxxx>




[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