Re: [Intel-gfx] [PATCH v15 00/26] Add vfio_device cdev for iommufd support
On Tue, Jul 25, 2023 at 12:00:09PM -0600, Alex Williamson wrote: > On Mon, 24 Jul 2023 13:09:22 -0600 > Alex Williamson wrote: > > > On Tue, 18 Jul 2023 13:57:46 -0300 > > Jason Gunthorpe wrote: > > > > > On Tue, Jul 18, 2023 at 06:55:25AM -0700, Yi Liu wrote: > > > > 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. After this, adds a mechanism > > > > for > > > > blocking device access before iommufd bind. Then refactors the vfio to > > > > be > > > > able to handle cdev paths (e.g. iommufd binding, no-iommufd, [de]attach > > > > ioas). > > > > This refactor includes making the device_open exclusive between the > > > > group > > > > and the 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]. Per discussion[4], this series > > > > does not > > > > support cdev for physical devices that do not have IOMMU. Such devices > > > > only > > > > have group-centric user APIs. > > > > > > > > This series is a prerequisite for iommu nesting for vfio device[5] [6]. > > > > > > > > The complete code can be found in below branch, simple tests done to the > > > > legacy group path and the cdev path. QEMU changes are in upstreaming[7] > > > > and the complete code can be found at[8] > > > > > > > > https://github.com/yiliu1765/iommufd/tree/vfio_device_cdev_v15 > > > > (config CONFIG_IOMMUFD=y CONFIG_VFIO_DEVICE_CDEV=y) > > > > > > Alex, if you are still good with this lets make this into a shared > > > branch, do you want to do it or would you like a PR from me? > > > > Sorry, was out much of last week. Yes, my intent would be to put this > > both in a shared branch and my next branch for v6.6. Given this is > > mostly vfio, it seems like it'd make sense for me to provide that > > branch but I may not get to it until tomorrow. Thanks, > > Both series are applied to my next branch for v6.6 and I've also > published them to the v6.6/vfio/cdev branch[1]. Thanks for all the > work and collaboration on this effort! Great, I pulled it and merged the next series Thanks, Jason
Re: [Intel-gfx] [PATCH v15 00/26] Add vfio_device cdev for iommufd support
On Mon, 24 Jul 2023 13:09:22 -0600 Alex Williamson wrote: > On Tue, 18 Jul 2023 13:57:46 -0300 > Jason Gunthorpe wrote: > > > On Tue, Jul 18, 2023 at 06:55:25AM -0700, Yi Liu wrote: > > > 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. After this, adds a mechanism for > > > blocking device access before iommufd bind. Then refactors the vfio to be > > > able to handle cdev paths (e.g. iommufd binding, no-iommufd, [de]attach > > > ioas). > > > This refactor includes making the device_open exclusive between the group > > > and the 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]. Per discussion[4], this series > > > does not > > > support cdev for physical devices that do not have IOMMU. Such devices > > > only > > > have group-centric user APIs. > > > > > > This series is a prerequisite for iommu nesting for vfio device[5] [6]. > > > > > > The complete code can be found in below branch, simple tests done to the > > > legacy group path and the cdev path. QEMU changes are in upstreaming[7] > > > and the complete code can be found at[8] > > > > > > https://github.com/yiliu1765/iommufd/tree/vfio_device_cdev_v15 > > > (config CONFIG_IOMMUFD=y CONFIG_VFIO_DEVICE_CDEV=y) > > > > Alex, if you are still good with this lets make this into a shared > > branch, do you want to do it or would you like a PR from me? > > Sorry, was out much of last week. Yes, my intent would be to put this > both in a shared branch and my next branch for v6.6. Given this is > mostly vfio, it seems like it'd make sense for me to provide that > branch but I may not get to it until tomorrow. Thanks, Both series are applied to my next branch for v6.6 and I've also published them to the v6.6/vfio/cdev branch[1]. Thanks for all the work and collaboration on this effort! Alex [1]https://github.com/awilliam/linux-vfio/tree/v6.6/vfio/cdev
Re: [Intel-gfx] [PATCH v15 00/26] Add vfio_device cdev for iommufd support
On Tue, 18 Jul 2023 13:57:46 -0300 Jason Gunthorpe wrote: > On Tue, Jul 18, 2023 at 06:55:25AM -0700, Yi Liu wrote: > > 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. After this, adds a mechanism for > > blocking device access before iommufd bind. Then refactors the vfio to be > > able to handle cdev paths (e.g. iommufd binding, no-iommufd, [de]attach > > ioas). > > This refactor includes making the device_open exclusive between the group > > and the 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]. Per discussion[4], this series does > > not > > support cdev for physical devices that do not have IOMMU. Such devices only > > have group-centric user APIs. > > > > This series is a prerequisite for iommu nesting for vfio device[5] [6]. > > > > The complete code can be found in below branch, simple tests done to the > > legacy group path and the cdev path. QEMU changes are in upstreaming[7] > > and the complete code can be found at[8] > > > > https://github.com/yiliu1765/iommufd/tree/vfio_device_cdev_v15 > > (config CONFIG_IOMMUFD=y CONFIG_VFIO_DEVICE_CDEV=y) > > Alex, if you are still good with this lets make this into a shared > branch, do you want to do it or would you like a PR from me? Sorry, was out much of last week. Yes, my intent would be to put this both in a shared branch and my next branch for v6.6. Given this is mostly vfio, it seems like it'd make sense for me to provide that branch but I may not get to it until tomorrow. Thanks, Alex
Re: [Intel-gfx] [PATCH v15 00/26] Add vfio_device cdev for iommufd support
On Tue, Jul 18, 2023 at 06:55:25AM -0700, Yi Liu wrote: > 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. After this, adds a mechanism for > blocking device access before iommufd bind. Then refactors the vfio to be > able to handle cdev paths (e.g. iommufd binding, no-iommufd, [de]attach ioas). > This refactor includes making the device_open exclusive between the group > and the 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]. Per discussion[4], this series does > not > support cdev for physical devices that do not have IOMMU. Such devices only > have group-centric user APIs. > > This series is a prerequisite for iommu nesting for vfio device[5] [6]. > > The complete code can be found in below branch, simple tests done to the > legacy group path and the cdev path. QEMU changes are in upstreaming[7] > and the complete code can be found at[8] > > https://github.com/yiliu1765/iommufd/tree/vfio_device_cdev_v15 > (config CONFIG_IOMMUFD=y CONFIG_VFIO_DEVICE_CDEV=y) Alex, if you are still good with this lets make this into a shared branch, do you want to do it or would you like a PR from me? Thanks, Jason
[Intel-gfx] [PATCH v15 00/26] 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. After this, adds a mechanism for blocking device access before iommufd bind. Then refactors the vfio to be able to handle cdev paths (e.g. iommufd binding, no-iommufd, [de]attach ioas). This refactor includes making the device_open exclusive between the group and the 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]. Per discussion[4], this series does not support cdev for physical devices that do not have IOMMU. Such devices only have group-centric user APIs. This series is a prerequisite for iommu nesting for vfio device[5] [6]. The complete code can be found in below branch, simple tests done to the legacy group path and the cdev path. QEMU changes are in upstreaming[7] and the complete code can be found at[8] https://github.com/yiliu1765/iommufd/tree/vfio_device_cdev_v15 (config CONFIG_IOMMUFD=y CONFIG_VFIO_DEVICE_CDEV=y) base-commit: fdf0eaf11452d72945af31804e2a1048ee1b574c [1] https://lore.kernel.org/kvm/bn9pr11mb5433b1e4ae5b0480369f97178c...@bn9pr11mb5433.namprd11.prod.outlook.com/ [2] https://lore.kernel.org/kvm/20230327093351.44505-1-yi.l@intel.com/ - merged [3] https://lore.kernel.org/kvm/20230718105542.4138-1-yi.l@intel.com/ [4] https://lore.kernel.org/kvm/20230525095939.37ddb8ce.alex.william...@redhat.com/ [5] https://lore.kernel.org/linux-iommu/20230511143844.22693-1-yi.l@intel.com/ [6] https://lore.kernel.org/linux-iommu/20230511145110.27707-1-yi.l@intel.com/#t [7] https://lore.kernel.org/qemu-devel/20230712072528.275577-1-zhenzhong.d...@intel.com/ [8] https://github.com/yiliu1765/qemu/tree/zhenzhong/iommufd_rfcv4 Change log: v15: - Add Jason's r-b to patch 14, 17, 19, 21, 23, 24 and 26 of cdev v14 - Tweak the iommufd_ctx_from_fd() in patch 20 per Jason's suggestion (Jason) - Return -ENOTTY in vfio_df_ioctl_bind_iommufd() stub function (Jason) - Add t-b from Zhenzhong (wrote a selftest app to verify functions of this patchset by referencing https://github.com/awilliam/tests/) v14: https://lore.kernel.org/kvm/20230711025928.6438-1-yi.l@intel.com/ - Add Jason's r-b to patch 10, 11, 12, 13, 15, 16, 17 and 23 of cdev v12 - Refine iommufd_access_detach() (Jason) - Split the device_del() movement to be a separate patch (Jason) - Move kvm !null test into _vfio_device_get_kvm_safe() to save some lines and rename it to be vfio_device_get_kvm_safe() (Jason) - Make VFIO_DEVICE_CDEV depending on !SPAPR_TCE_IOMMU to suit the fact that SPAPR_TCE_IOMMU does not support cdev (Alex) - Add iommufd_ctx_from_fd() to replace vfio_get_iommufd_from_fd() (Jason) - Check cdev only ioctls in vfio_device_fops_unl_ioctl() (Jason) - patch 17, 19, 20 and 21 of v14 is newly added per above review comemnts. v13: https://lore.kernel.org/kvm/20230616093946.68711-1-yi.l@intel.com/ - vfio_device_first_open() and vfio_device_last_close() to be vfio_df_device_first_open() vfio_df_device_last_close() (Alex) - Define struct vfio_device_file::access_granted as u8 and also place the u32 devid to be behind this flag as this structure access is hot, so needs to avoid too much hole in the structure (Alex) - Use u8 instead bool in the struct vfio_device for the flags (Alex) - Define BIND, ATTACH, DETACH ioctl behind VFIO_DEVICE_FEATURE whose offset is 17 (Alex) - Drop patch 20, 21, 22 of v12 (Alex) - Per the patch drop, still needs to detect the physical devices that do not have IOMMU in the cdev registration as cdev does not support such devices. Per the suggestion from Jason, lift the IOMMU_CAP_CACHE_COHERENCY check to be in vfio_main.c so that it can fail the registration of such devices if only cdev is compiled. (Jason, Alex) - Refine the vfio.rst doc, highlight that the cdev device access is stil bound with iommu group. (Alex) - Reaffirm t-b from below folks: Nicolin Chen - Test nesting branch which is based on cdev v12, the test is done on ARM64 (SMMUv3) Matthew
