[PATCH v4 0/7] Make the rest of the VFIO driver interface use vfio_device

List overview All Threads
Download

newer

older

[PATCH v5] drm/msm/dp: Always...

[BUG] Warning and NULL-ptr...

Jason Gunthorpe

6 May 2022 6 May '22

12:08 a.m.

Prior series have transformed other parts of VFIO from working on struct device or struct vfio_group into working directly on struct vfio_device. Based on that work we now have vfio_device's readily available in all the drivers.

Update the rest of the driver facing API to use vfio_device as an input.

The following are switched from struct device to struct vfio_device: vfio_register_notifier() vfio_unregister_notifier() vfio_pin_pages() vfio_unpin_pages() vfio_dma_rw()

The following group APIs are obsoleted and removed by just using struct vfio_device with the above: vfio_group_pin_pages() vfio_group_unpin_pages() vfio_group_iommu_domain() vfio_group_get_external_user_from_dev()

To retain the performance of the new device APIs relative to their group versions optimize how vfio_group_add_container_user() is used to avoid calling it when the driver must already guarantee the device is open and the container_users incrd.

The remaining exported VFIO group interfaces are only used by kvm, and are addressed by a parallel series.

This series is based on Christoph's gvt rework here:

https://lore.kernel.org/all/5a8b9f48-2c32-8177-1c18-e3bd7bfde558@intel.com/

and so will need the PR merged first.

I have a followup series that needs this.

This is also part of the iommufd work - moving the driver facing interface to vfio_device provides a much cleaner path to integrate with iommufd.

v4: - Use 'device' as the argument name for a struct vfio_device in vfio.c v3: https://lore.kernel.org/r/0-v3-e131a9b6b467+14b6-vfio_mdev_no_group_jgg@nvid... - Based on VFIO's gvt/iommu merge - Remove mention of mdev_legacy_get_vfio_device() from commit message - Clarify commit message for vfio_dma_rw() conversion - Talk about the open_count change in the commit message - No code change v2: https://lore.kernel.org/r/0-v2-6011bde8e0a1+5f-vfio_mdev_no_group_jgg@nvidia... - Based on Christoph's series so mdev_legacy_get_vfio_device() is removed - Reflow indenting - Use vfio_assert_device_open() and WARN_ON_ONCE instead of open coding the assertion v1: https://lore.kernel.org/r/0-v1-a8faf768d202+125dd-vfio_mdev_no_group_jgg@nvi...

Jason Gunthorpe (7): vfio: Make vfio_(un)register_notifier accept a vfio_device vfio/ccw: Remove mdev from struct channel_program vfio/mdev: Pass in a struct vfio_device * to vfio_pin/unpin_pages() vfio/mdev: Pass in a struct vfio_device * to vfio_dma_rw() drm/i915/gvt: Change from vfio_group_(un)pin_pages to vfio_(un)pin_pages vfio: Remove dead code vfio: Remove calls to vfio_group_add_container_user()

base-commit: 676d7cda1a3c19872428a9bc818577a1aafafdd5

-- 2.36.0

Show replies by date

Jason Gunthorpe

6 May 6 May

12:08 a.m.

New subject: [PATCH v4 1/7] vfio: Make vfio_(un)register_notifier accept a vfio_device

All callers have a struct vfio_device trivially available, pass it in directly and avoid calling the expensive vfio_group_get_from_dev().

Acked-by: Eric Farman farman@linux.ibm.com Reviewed-by: Jason J. Herne jjherne@linux.ibm.com Reviewed-by: Tony Krowiak akrowiak@linux.ibm.com Reviewed-by: Kevin Tian kevin.tian@intel.com Reviewed-by: Christoph Hellwig hch@lst.de Signed-off-by: Jason Gunthorpe jgg@nvidia.com --- drivers/gpu/drm/i915/gvt/kvmgt.c | 24 ++++++++++++------------ drivers/s390/cio/vfio_ccw_ops.c | 7 +++---- drivers/s390/crypto/vfio_ap_ops.c | 14 +++++++------- drivers/vfio/vfio.c | 28 +++++++++------------------- include/linux/vfio.h | 4 ++-- 5 files changed, 33 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 0787ba5c301f5e..1cec4f1fdfaced 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -810,8 +810,8 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev) vgpu->group_notifier.notifier_call = intel_vgpu_group_notifier;

events = VFIO_IOMMU_NOTIFY_DMA_UNMAP; - ret = vfio_register_notifier(vfio_dev->dev, VFIO_IOMMU_NOTIFY, &events, - &vgpu->iommu_notifier); + ret = vfio_register_notifier(vfio_dev, VFIO_IOMMU_NOTIFY, &events, + &vgpu->iommu_notifier); if (ret != 0) { gvt_vgpu_err("vfio_register_notifier for iommu failed: %d\n", ret); @@ -819,8 +819,8 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev) }

events = VFIO_GROUP_NOTIFY_SET_KVM; - ret = vfio_register_notifier(vfio_dev->dev, VFIO_GROUP_NOTIFY, &events, - &vgpu->group_notifier); + ret = vfio_register_notifier(vfio_dev, VFIO_GROUP_NOTIFY, &events, + &vgpu->group_notifier); if (ret != 0) { gvt_vgpu_err("vfio_register_notifier for group failed: %d\n", ret); @@ -873,12 +873,12 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev) vgpu->vfio_group = NULL;

undo_register: - vfio_unregister_notifier(vfio_dev->dev, VFIO_GROUP_NOTIFY, - &vgpu->group_notifier); + vfio_unregister_notifier(vfio_dev, VFIO_GROUP_NOTIFY, + &vgpu->group_notifier);

undo_iommu: - vfio_unregister_notifier(vfio_dev->dev, VFIO_IOMMU_NOTIFY, - &vgpu->iommu_notifier); + vfio_unregister_notifier(vfio_dev, VFIO_IOMMU_NOTIFY, + &vgpu->iommu_notifier); out: return ret; } @@ -907,13 +907,13 @@ static void __intel_vgpu_release(struct intel_vgpu *vgpu)

intel_gvt_release_vgpu(vgpu);

- ret = vfio_unregister_notifier(vgpu->vfio_device.dev, VFIO_IOMMU_NOTIFY, - &vgpu->iommu_notifier); + ret = vfio_unregister_notifier(&vgpu->vfio_device, VFIO_IOMMU_NOTIFY, + &vgpu->iommu_notifier); drm_WARN(&i915->drm, ret, "vfio_unregister_notifier for iommu failed: %d\n", ret);

- ret = vfio_unregister_notifier(vgpu->vfio_device.dev, VFIO_GROUP_NOTIFY, - &vgpu->group_notifier); + ret = vfio_unregister_notifier(&vgpu->vfio_device, VFIO_GROUP_NOTIFY, + &vgpu->group_notifier); drm_WARN(&i915->drm, ret, "vfio_unregister_notifier for group failed: %d\n", ret);

diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c index c4d60cdbf247bf..b49e2e9db2dc6f 100644 --- a/drivers/s390/cio/vfio_ccw_ops.c +++ b/drivers/s390/cio/vfio_ccw_ops.c @@ -183,7 +183,7 @@ static int vfio_ccw_mdev_open_device(struct vfio_device *vdev)

private->nb.notifier_call = vfio_ccw_mdev_notifier;

- ret = vfio_register_notifier(vdev->dev, VFIO_IOMMU_NOTIFY, + ret = vfio_register_notifier(vdev, VFIO_IOMMU_NOTIFY, &events, &private->nb); if (ret) return ret; @@ -204,8 +204,7 @@ static int vfio_ccw_mdev_open_device(struct vfio_device *vdev)

out_unregister: vfio_ccw_unregister_dev_regions(private); - vfio_unregister_notifier(vdev->dev, VFIO_IOMMU_NOTIFY, - &private->nb); + vfio_unregister_notifier(vdev, VFIO_IOMMU_NOTIFY, &private->nb); return ret; }

@@ -223,7 +222,7 @@ static void vfio_ccw_mdev_close_device(struct vfio_device *vdev)

cp_free(&private->cp); vfio_ccw_unregister_dev_regions(private); - vfio_unregister_notifier(vdev->dev, VFIO_IOMMU_NOTIFY, &private->nb); + vfio_unregister_notifier(vdev, VFIO_IOMMU_NOTIFY, &private->nb); }

static ssize_t vfio_ccw_mdev_read_io_region(struct vfio_ccw_private *private, diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index ee0a3bf8f476ca..bfa7ee6ef532d9 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -1406,21 +1406,21 @@ static int vfio_ap_mdev_open_device(struct vfio_device *vdev) matrix_mdev->group_notifier.notifier_call = vfio_ap_mdev_group_notifier; events = VFIO_GROUP_NOTIFY_SET_KVM;

- ret = vfio_register_notifier(vdev->dev, VFIO_GROUP_NOTIFY, - &events, &matrix_mdev->group_notifier); + ret = vfio_register_notifier(vdev, VFIO_GROUP_NOTIFY, &events, + &matrix_mdev->group_notifier); if (ret) return ret;

matrix_mdev->iommu_notifier.notifier_call = vfio_ap_mdev_iommu_notifier; events = VFIO_IOMMU_NOTIFY_DMA_UNMAP; - ret = vfio_register_notifier(vdev->dev, VFIO_IOMMU_NOTIFY, - &events, &matrix_mdev->iommu_notifier); + ret = vfio_register_notifier(vdev, VFIO_IOMMU_NOTIFY, &events, + &matrix_mdev->iommu_notifier); if (ret) goto out_unregister_group; return 0;

out_unregister_group: - vfio_unregister_notifier(vdev->dev, VFIO_GROUP_NOTIFY, + vfio_unregister_notifier(vdev, VFIO_GROUP_NOTIFY, &matrix_mdev->group_notifier); return ret; } @@ -1430,9 +1430,9 @@ static void vfio_ap_mdev_close_device(struct vfio_device *vdev) struct ap_matrix_mdev *matrix_mdev = container_of(vdev, struct ap_matrix_mdev, vdev);

- vfio_unregister_notifier(vdev->dev, VFIO_IOMMU_NOTIFY, + vfio_unregister_notifier(vdev, VFIO_IOMMU_NOTIFY, &matrix_mdev->iommu_notifier); - vfio_unregister_notifier(vdev->dev, VFIO_GROUP_NOTIFY, + vfio_unregister_notifier(vdev, VFIO_GROUP_NOTIFY, &matrix_mdev->group_notifier); vfio_ap_mdev_unset_kvm(matrix_mdev); } diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 0c766384cee0f8..d70890ab9cea92 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -2269,19 +2269,16 @@ static int vfio_unregister_group_notifier(struct vfio_group *group, return ret; }

-int vfio_register_notifier(struct device *dev, enum vfio_notify_type type, - unsigned long *events, struct notifier_block *nb) +int vfio_register_notifier(struct vfio_device *device, + enum vfio_notify_type type, unsigned long *events, + struct notifier_block *nb) { - struct vfio_group *group; + struct vfio_group *group = device->group; int ret;

- if (!dev || !nb || !events || (*events == 0)) + if (!nb || !events || (*events == 0)) return -EINVAL;

- group = vfio_group_get_from_dev(dev); - if (!group) - return -ENODEV; - switch (type) { case VFIO_IOMMU_NOTIFY: ret = vfio_register_iommu_notifier(group, events, nb); @@ -2292,25 +2289,20 @@ int vfio_register_notifier(struct device *dev, enum vfio_notify_type type, default: ret = -EINVAL; } - - vfio_group_put(group); return ret; } EXPORT_SYMBOL(vfio_register_notifier);

-int vfio_unregister_notifier(struct device *dev, enum vfio_notify_type type, +int vfio_unregister_notifier(struct vfio_device *device, + enum vfio_notify_type type, struct notifier_block *nb) { - struct vfio_group *group; + struct vfio_group *group = device->group; int ret;

- if (!dev || !nb) + if (!nb) return -EINVAL;

- group = vfio_group_get_from_dev(dev); - if (!group) - return -ENODEV; - switch (type) { case VFIO_IOMMU_NOTIFY: ret = vfio_unregister_iommu_notifier(group, nb); @@ -2321,8 +2313,6 @@ int vfio_unregister_notifier(struct device *dev, enum vfio_notify_type type, default: ret = -EINVAL; } - - vfio_group_put(group); return ret; } EXPORT_SYMBOL(vfio_unregister_notifier); diff --git a/include/linux/vfio.h b/include/linux/vfio.h index 66dda06ec42d1b..a00fd722f044b0 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -178,11 +178,11 @@ enum vfio_notify_type { /* events for VFIO_GROUP_NOTIFY */ #define VFIO_GROUP_NOTIFY_SET_KVM BIT(0)

-extern int vfio_register_notifier(struct device *dev, +extern int vfio_register_notifier(struct vfio_device *device, enum vfio_notify_type type, unsigned long *required_events, struct notifier_block *nb); -extern int vfio_unregister_notifier(struct device *dev, +extern int vfio_unregister_notifier(struct vfio_device *device, enum vfio_notify_type type, struct notifier_block *nb);

-- 2.36.0

Jason Gunthorpe

12:08 a.m.

New subject: [PATCH v4 2/7] vfio/ccw: Remove mdev from struct channel_program

The next patch wants the vfio_device instead. There is no reason to store a pointer here since we can container_of back to the vfio_device.

Reviewed-by: Eric Farman farman@linux.ibm.com Signed-off-by: Jason Gunthorpe jgg@nvidia.com --- drivers/s390/cio/vfio_ccw_cp.c | 47 ++++++++++++++++++++------------- drivers/s390/cio/vfio_ccw_cp.h | 4 +-- drivers/s390/cio/vfio_ccw_fsm.c | 3 +-- 3 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index 8d1b2771c1aa02..7a1cf3091cd647 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -16,6 +16,7 @@ #include <asm/idals.h>

#include "vfio_ccw_cp.h" +#include "vfio_ccw_private.h"

struct pfn_array { /* Starting guest physical I/O address. */ @@ -98,17 +99,17 @@ static int pfn_array_alloc(struct pfn_array *pa, u64 iova, unsigned int len) * If the pin request partially succeeds, or fails completely, * all pages are left unpinned and a negative error value is returned. */ -static int pfn_array_pin(struct pfn_array *pa, struct device *mdev) +static int pfn_array_pin(struct pfn_array *pa, struct vfio_device *vdev) { int ret = 0;

- ret = vfio_pin_pages(mdev, pa->pa_iova_pfn, pa->pa_nr, + ret = vfio_pin_pages(vdev->dev, pa->pa_iova_pfn, pa->pa_nr, IOMMU_READ | IOMMU_WRITE, pa->pa_pfn);

if (ret < 0) { goto err_out; } else if (ret > 0 && ret != pa->pa_nr) { - vfio_unpin_pages(mdev, pa->pa_iova_pfn, ret); + vfio_unpin_pages(vdev->dev, pa->pa_iova_pfn, ret); ret = -EINVAL; goto err_out; } @@ -122,11 +123,11 @@ static int pfn_array_pin(struct pfn_array *pa, struct device *mdev) }

/* Unpin the pages before releasing the memory. */ -static void pfn_array_unpin_free(struct pfn_array *pa, struct device *mdev) +static void pfn_array_unpin_free(struct pfn_array *pa, struct vfio_device *vdev) { /* Only unpin if any pages were pinned to begin with */ if (pa->pa_nr) - vfio_unpin_pages(mdev, pa->pa_iova_pfn, pa->pa_nr); + vfio_unpin_pages(vdev->dev, pa->pa_iova_pfn, pa->pa_nr); pa->pa_nr = 0; kfree(pa->pa_iova_pfn); } @@ -190,8 +191,7 @@ static void convert_ccw0_to_ccw1(struct ccw1 *source, unsigned long len) * Within the domain (@mdev), copy @n bytes from a guest physical * address (@iova) to a host physical address (@to). */ -static long copy_from_iova(struct device *mdev, - void *to, u64 iova, +static long copy_from_iova(struct vfio_device *vdev, void *to, u64 iova, unsigned long n) { struct pfn_array pa = {0}; @@ -203,9 +203,9 @@ static long copy_from_iova(struct device *mdev, if (ret < 0) return ret;

- ret = pfn_array_pin(&pa, mdev); + ret = pfn_array_pin(&pa, vdev); if (ret < 0) { - pfn_array_unpin_free(&pa, mdev); + pfn_array_unpin_free(&pa, vdev); return ret; }

@@ -226,7 +226,7 @@ static long copy_from_iova(struct device *mdev, break; }

- pfn_array_unpin_free(&pa, mdev); + pfn_array_unpin_free(&pa, vdev);

return l; } @@ -423,11 +423,13 @@ static int ccwchain_loop_tic(struct ccwchain *chain,

static int ccwchain_handle_ccw(u32 cda, struct channel_program *cp) { + struct vfio_device *vdev = + &container_of(cp, struct vfio_ccw_private, cp)->vdev; struct ccwchain *chain; int len, ret;

/* Copy 2K (the most we support today) of possible CCWs */ - len = copy_from_iova(cp->mdev, cp->guest_cp, cda, + len = copy_from_iova(vdev, cp->guest_cp, cda, CCWCHAIN_LEN_MAX * sizeof(struct ccw1)); if (len) return len; @@ -508,6 +510,8 @@ static int ccwchain_fetch_direct(struct ccwchain *chain, int idx, struct channel_program *cp) { + struct vfio_device *vdev = + &container_of(cp, struct vfio_ccw_private, cp)->vdev; struct ccw1 *ccw; struct pfn_array *pa; u64 iova; @@ -526,7 +530,7 @@ static int ccwchain_fetch_direct(struct ccwchain *chain, if (ccw_is_idal(ccw)) { /* Read first IDAW to see if it's 4K-aligned or not. */ /* All subsequent IDAws will be 4K-aligned. */ - ret = copy_from_iova(cp->mdev, &iova, ccw->cda, sizeof(iova)); + ret = copy_from_iova(vdev, &iova, ccw->cda, sizeof(iova)); if (ret) return ret; } else { @@ -555,7 +559,7 @@ static int ccwchain_fetch_direct(struct ccwchain *chain,

if (ccw_is_idal(ccw)) { /* Copy guest IDAL into host IDAL */ - ret = copy_from_iova(cp->mdev, idaws, ccw->cda, idal_len); + ret = copy_from_iova(vdev, idaws, ccw->cda, idal_len); if (ret) goto out_unpin;

@@ -574,7 +578,7 @@ static int ccwchain_fetch_direct(struct ccwchain *chain, }

if (ccw_does_data_transfer(ccw)) { - ret = pfn_array_pin(pa, cp->mdev); + ret = pfn_array_pin(pa, vdev); if (ret < 0) goto out_unpin; } else { @@ -590,7 +594,7 @@ static int ccwchain_fetch_direct(struct ccwchain *chain, return 0;

out_unpin: - pfn_array_unpin_free(pa, cp->mdev); + pfn_array_unpin_free(pa, vdev); out_free_idaws: kfree(idaws); out_init: @@ -632,8 +636,10 @@ static int ccwchain_fetch_one(struct ccwchain *chain, * Returns: * %0 on success and a negative error value on failure. */ -int cp_init(struct channel_program *cp, struct device *mdev, union orb *orb) +int cp_init(struct channel_program *cp, union orb *orb) { + struct vfio_device *vdev = + &container_of(cp, struct vfio_ccw_private, cp)->vdev; /* custom ratelimit used to avoid flood during guest IPL */ static DEFINE_RATELIMIT_STATE(ratelimit_state, 5 * HZ, 1); int ret; @@ -650,11 +656,12 @@ int cp_init(struct channel_program *cp, struct device *mdev, union orb *orb) * the problem if something does break. */ if (!orb->cmd.pfch && __ratelimit(&ratelimit_state)) - dev_warn(mdev, "Prefetching channel program even though prefetch not specified in ORB"); + dev_warn( + vdev->dev, + "Prefetching channel program even though prefetch not specified in ORB");

INIT_LIST_HEAD(&cp->ccwchain_list); memcpy(&cp->orb, orb, sizeof(*orb)); - cp->mdev = mdev;

/* Build a ccwchain for the first CCW segment */ ret = ccwchain_handle_ccw(orb->cmd.cpa, cp); @@ -682,6 +689,8 @@ int cp_init(struct channel_program *cp, struct device *mdev, union orb *orb) */ void cp_free(struct channel_program *cp) { + struct vfio_device *vdev = + &container_of(cp, struct vfio_ccw_private, cp)->vdev; struct ccwchain *chain, *temp; int i;

@@ -691,7 +700,7 @@ void cp_free(struct channel_program *cp) cp->initialized = false; list_for_each_entry_safe(chain, temp, &cp->ccwchain_list, next) { for (i = 0; i < chain->ch_len; i++) { - pfn_array_unpin_free(chain->ch_pa + i, cp->mdev); + pfn_array_unpin_free(chain->ch_pa + i, vdev); ccwchain_cda_free(chain, i); } ccwchain_free(chain); diff --git a/drivers/s390/cio/vfio_ccw_cp.h b/drivers/s390/cio/vfio_ccw_cp.h index ba31240ce96594..e4c436199b4cda 100644 --- a/drivers/s390/cio/vfio_ccw_cp.h +++ b/drivers/s390/cio/vfio_ccw_cp.h @@ -37,13 +37,11 @@ struct channel_program { struct list_head ccwchain_list; union orb orb; - struct device *mdev; bool initialized; struct ccw1 *guest_cp; };

-extern int cp_init(struct channel_program *cp, struct device *mdev, - union orb *orb); +extern int cp_init(struct channel_program *cp, union orb *orb); extern void cp_free(struct channel_program *cp); extern int cp_prefetch(struct channel_program *cp); extern union orb *cp_get_orb(struct channel_program *cp, u32 intparm, u8 lpm); diff --git a/drivers/s390/cio/vfio_ccw_fsm.c b/drivers/s390/cio/vfio_ccw_fsm.c index e435a9cd92dacf..8483a266051c21 100644 --- a/drivers/s390/cio/vfio_ccw_fsm.c +++ b/drivers/s390/cio/vfio_ccw_fsm.c @@ -262,8 +262,7 @@ static void fsm_io_request(struct vfio_ccw_private *private, errstr = "transport mode"; goto err_out; } - io_region->ret_code = cp_init(&private->cp, mdev_dev(mdev), - orb); + io_region->ret_code = cp_init(&private->cp, orb); if (io_region->ret_code) { VFIO_CCW_MSG_EVENT(2, "%pUl (%x.%x.%04x): cp_init=%d\n",

-- 2.36.0

Jason Gunthorpe

12:08 a.m.

New subject: [PATCH v4 3/7] vfio/mdev: Pass in a struct vfio_device * to vfio_pin/unpin_pages()

Every caller has a readily available vfio_device pointer, use that instead of passing in a generic struct device. The struct vfio_device already contains the group we need so this avoids complexity, extra refcountings, and a confusing lifecycle model.

Reviewed-by: Christoph Hellwig hch@lst.de Acked-by: Eric Farman farman@linux.ibm.com Reviewed-by: Jason J. Herne jjherne@linux.ibm.com Reviewed-by: Tony Krowiak akrowiak@linux.ibm.com Reviewed-by: Kevin Tian kevin.tian@intel.com Signed-off-by: Jason Gunthorpe jgg@nvidia.com --- .../driver-api/vfio-mediated-device.rst | 4 +- drivers/s390/cio/vfio_ccw_cp.c | 6 +-- drivers/s390/crypto/vfio_ap_ops.c | 9 ++-- drivers/vfio/vfio.c | 46 +++++++------------ include/linux/vfio.h | 4 +- 5 files changed, 27 insertions(+), 42 deletions(-)

diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst index 784bbeb22adcf5..eded8719180fba 100644 --- a/Documentation/driver-api/vfio-mediated-device.rst +++ b/Documentation/driver-api/vfio-mediated-device.rst @@ -262,10 +262,10 @@ Translation APIs for Mediated Devices The following APIs are provided for translating user pfn to host pfn in a VFIO driver::

- extern int vfio_pin_pages(struct device *dev, unsigned long *user_pfn, + int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn, int npage, int prot, unsigned long *phys_pfn);

- extern int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn, + int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn, int npage);

These functions call back into the back-end IOMMU module by using the pin_pages diff --git a/drivers/s390/cio/vfio_ccw_cp.c b/drivers/s390/cio/vfio_ccw_cp.c index 7a1cf3091cd647..0c2be9421ab78f 100644 --- a/drivers/s390/cio/vfio_ccw_cp.c +++ b/drivers/s390/cio/vfio_ccw_cp.c @@ -103,13 +103,13 @@ static int pfn_array_pin(struct pfn_array *pa, struct vfio_device *vdev) { int ret = 0;

- ret = vfio_pin_pages(vdev->dev, pa->pa_iova_pfn, pa->pa_nr, + ret = vfio_pin_pages(vdev, pa->pa_iova_pfn, pa->pa_nr, IOMMU_READ | IOMMU_WRITE, pa->pa_pfn);

if (ret < 0) { goto err_out; } else if (ret > 0 && ret != pa->pa_nr) { - vfio_unpin_pages(vdev->dev, pa->pa_iova_pfn, ret); + vfio_unpin_pages(vdev, pa->pa_iova_pfn, ret); ret = -EINVAL; goto err_out; } @@ -127,7 +127,7 @@ static void pfn_array_unpin_free(struct pfn_array *pa, struct vfio_device *vdev) { /* Only unpin if any pages were pinned to begin with */ if (pa->pa_nr) - vfio_unpin_pages(vdev->dev, pa->pa_iova_pfn, pa->pa_nr); + vfio_unpin_pages(vdev, pa->pa_iova_pfn, pa->pa_nr); pa->pa_nr = 0; kfree(pa->pa_iova_pfn); } diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index bfa7ee6ef532d9..e8914024f5b1af 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -124,8 +124,7 @@ static void vfio_ap_free_aqic_resources(struct vfio_ap_queue *q) q->saved_isc = VFIO_AP_ISC_INVALID; } if (q->saved_pfn && !WARN_ON(!q->matrix_mdev)) { - vfio_unpin_pages(mdev_dev(q->matrix_mdev->mdev), - &q->saved_pfn, 1); + vfio_unpin_pages(&q->matrix_mdev->vdev, &q->saved_pfn, 1); q->saved_pfn = 0; } } @@ -258,7 +257,7 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q, return status; }

- ret = vfio_pin_pages(mdev_dev(q->matrix_mdev->mdev), &g_pfn, 1, + ret = vfio_pin_pages(&q->matrix_mdev->vdev, &g_pfn, 1, IOMMU_READ | IOMMU_WRITE, &h_pfn); switch (ret) { case 1: @@ -301,7 +300,7 @@ static struct ap_queue_status vfio_ap_irq_enable(struct vfio_ap_queue *q, break; case AP_RESPONSE_OTHERWISE_CHANGED: /* We could not modify IRQ setings: clear new configuration */ - vfio_unpin_pages(mdev_dev(q->matrix_mdev->mdev), &g_pfn, 1); + vfio_unpin_pages(&q->matrix_mdev->vdev, &g_pfn, 1); kvm_s390_gisc_unregister(kvm, isc); break; default: @@ -1250,7 +1249,7 @@ static int vfio_ap_mdev_iommu_notifier(struct notifier_block *nb, struct vfio_iommu_type1_dma_unmap *unmap = data; unsigned long g_pfn = unmap->iova >> PAGE_SHIFT;

- vfio_unpin_pages(mdev_dev(matrix_mdev->mdev), &g_pfn, 1); + vfio_unpin_pages(&matrix_mdev->vdev, &g_pfn, 1); return NOTIFY_OK; }

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index d70890ab9cea92..93caab1f29dbd7 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -1919,7 +1919,7 @@ EXPORT_SYMBOL(vfio_set_irqs_validate_and_prepare); /* * Pin a set of guest PFNs and return their associated host PFNs for local * domain only. - * @dev [in] : device + * @device [in] : device * @user_pfn [in]: array of user/guest PFNs to be pinned. * @npage [in] : count of elements in user_pfn array. This count should not * be greater VFIO_PIN_PAGES_MAX_ENTRIES. @@ -1927,32 +1927,26 @@ EXPORT_SYMBOL(vfio_set_irqs_validate_and_prepare); * @phys_pfn[out]: array of host PFNs * Return error or number of pages pinned. */ -int vfio_pin_pages(struct device *dev, unsigned long *user_pfn, int npage, - int prot, unsigned long *phys_pfn) +int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn, + int npage, int prot, unsigned long *phys_pfn) { struct vfio_container *container; - struct vfio_group *group; + struct vfio_group *group = device->group; struct vfio_iommu_driver *driver; int ret;

- if (!dev || !user_pfn || !phys_pfn || !npage) + if (!user_pfn || !phys_pfn || !npage) return -EINVAL;

if (npage > VFIO_PIN_PAGES_MAX_ENTRIES) return -E2BIG;

- group = vfio_group_get_from_dev(dev); - if (!group) - return -ENODEV; - - if (group->dev_counter > 1) { - ret = -EINVAL; - goto err_pin_pages; - } + if (group->dev_counter > 1) + return -EINVAL;

ret = vfio_group_add_container_user(group); if (ret) - goto err_pin_pages; + return ret;

container = group->container; driver = container->iommu_driver; @@ -1965,43 +1959,37 @@ int vfio_pin_pages(struct device *dev, unsigned long *user_pfn, int npage,

vfio_group_try_dissolve_container(group);

-err_pin_pages: - vfio_group_put(group); return ret; } EXPORT_SYMBOL(vfio_pin_pages);

/* * Unpin set of host PFNs for local domain only. - * @dev [in] : device + * @device [in] : device * @user_pfn [in]: array of user/guest PFNs to be unpinned. Number of user/guest * PFNs should not be greater than VFIO_PIN_PAGES_MAX_ENTRIES. * @npage [in] : count of elements in user_pfn array. This count should not * be greater than VFIO_PIN_PAGES_MAX_ENTRIES. * Return error or number of pages unpinned. */ -int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn, int npage) +int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn, + int npage) { struct vfio_container *container; - struct vfio_group *group; struct vfio_iommu_driver *driver; int ret;

- if (!dev || !user_pfn || !npage) + if (!user_pfn || !npage) return -EINVAL;

if (npage > VFIO_PIN_PAGES_MAX_ENTRIES) return -E2BIG;

- group = vfio_group_get_from_dev(dev); - if (!group) - return -ENODEV; - - ret = vfio_group_add_container_user(group); + ret = vfio_group_add_container_user(device->group); if (ret) - goto err_unpin_pages; + return ret;

- container = group->container; + container = device->group->container; driver = container->iommu_driver; if (likely(driver && driver->ops->unpin_pages)) ret = driver->ops->unpin_pages(container->iommu_data, user_pfn, @@ -2009,10 +1997,8 @@ int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn, int npage) else ret = -ENOTTY;

- vfio_group_try_dissolve_container(group); + vfio_group_try_dissolve_container(device->group);

-err_unpin_pages: - vfio_group_put(group); return ret; } EXPORT_SYMBOL(vfio_unpin_pages); diff --git a/include/linux/vfio.h b/include/linux/vfio.h index a00fd722f044b0..bddc70f88899c3 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -150,9 +150,9 @@ extern long vfio_external_check_extension(struct vfio_group *group,

#define VFIO_PIN_PAGES_MAX_ENTRIES (PAGE_SIZE/sizeof(unsigned long))

-extern int vfio_pin_pages(struct device *dev, unsigned long *user_pfn, +extern int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn, int npage, int prot, unsigned long *phys_pfn); -extern int vfio_unpin_pages(struct device *dev, unsigned long *user_pfn, +extern int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn, int npage);

extern int vfio_group_pin_pages(struct vfio_group *group,

-- 2.36.0

Jason Gunthorpe

12:08 a.m.

New subject: [PATCH v4 4/7] vfio/mdev: Pass in a struct vfio_device * to vfio_dma_rw()

Every caller has a readily available vfio_device pointer, use that instead of passing in a generic struct device. Change vfio_dma_rw() to take in the struct vfio_device and move the container users that would have been held by vfio_group_get_external_user_from_dev() to vfio_dma_rw() directly, like vfio_pin/unpin_pages().

Reviewed-by: Christoph Hellwig hch@lst.de Reviewed-by: Kevin Tian kevin.tian@intel.com Signed-off-by: Jason Gunthorpe jgg@nvidia.com --- drivers/gpu/drm/i915/gvt/gvt.h | 4 ++-- drivers/vfio/vfio.c | 24 +++++++++++------------- include/linux/vfio.h | 2 +- 3 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h index 03ecffc2ba56a9..5a28ee965b7f3e 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.h +++ b/drivers/gpu/drm/i915/gvt/gvt.h @@ -732,7 +732,7 @@ static inline int intel_gvt_read_gpa(struct intel_vgpu *vgpu, unsigned long gpa, { if (!vgpu->attached) return -ESRCH; - return vfio_dma_rw(vgpu->vfio_group, gpa, buf, len, false); + return vfio_dma_rw(&vgpu->vfio_device, gpa, buf, len, false); }

/** @@ -750,7 +750,7 @@ static inline int intel_gvt_write_gpa(struct intel_vgpu *vgpu, { if (!vgpu->attached) return -ESRCH; - return vfio_dma_rw(vgpu->vfio_group, gpa, buf, len, true); + return vfio_dma_rw(&vgpu->vfio_device, gpa, buf, len, true); }

void intel_gvt_debugfs_remove_vgpu(struct intel_vgpu *vgpu); diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 93caab1f29dbd7..85e1304099b8a5 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -2108,32 +2108,28 @@ EXPORT_SYMBOL(vfio_group_unpin_pages); * As the read/write of user space memory is conducted via the CPUs and is * not a real device DMA, it is not necessary to pin the user space memory. * - * The caller needs to call vfio_group_get_external_user() or - * vfio_group_get_external_user_from_dev() prior to calling this interface, - * so as to prevent the VFIO group from disposal in the middle of the call. - * But it can keep the reference to the VFIO group for several calls into - * this interface. - * After finishing using of the VFIO group, the caller needs to release the - * VFIO group by calling vfio_group_put_external_user(). - * - * @group [in] : VFIO group + * @device [in] : VFIO device * @user_iova [in] : base IOVA of a user space buffer * @data [in] : pointer to kernel buffer * @len [in] : kernel buffer length * @write : indicate read or write * Return error code on failure or 0 on success. */ -int vfio_dma_rw(struct vfio_group *group, dma_addr_t user_iova, - void *data, size_t len, bool write) +int vfio_dma_rw(struct vfio_device *device, dma_addr_t user_iova, void *data, + size_t len, bool write) { struct vfio_container *container; struct vfio_iommu_driver *driver; int ret = 0;

- if (!group || !data || len <= 0) + if (!data || len <= 0) return -EINVAL;

- container = group->container; + ret = vfio_group_add_container_user(device->group); + if (ret) + return ret; + + container = device->group->container; driver = container->iommu_driver;

if (likely(driver && driver->ops->dma_rw)) @@ -2142,6 +2138,8 @@ int vfio_dma_rw(struct vfio_group *group, dma_addr_t user_iova, else ret = -ENOTTY;

+ vfio_group_try_dissolve_container(device->group); + return ret; } EXPORT_SYMBOL(vfio_dma_rw); diff --git a/include/linux/vfio.h b/include/linux/vfio.h index bddc70f88899c3..8a151025871776 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -161,7 +161,7 @@ extern int vfio_group_pin_pages(struct vfio_group *group, extern int vfio_group_unpin_pages(struct vfio_group *group, unsigned long *user_iova_pfn, int npage);

-extern int vfio_dma_rw(struct vfio_group *group, dma_addr_t user_iova, +extern int vfio_dma_rw(struct vfio_device *device, dma_addr_t user_iova, void *data, size_t len, bool write);

extern struct iommu_domain *vfio_group_iommu_domain(struct vfio_group *group);

-- 2.36.0

Jason Gunthorpe

12:08 a.m.

New subject: [PATCH v4 5/7] drm/i915/gvt: Change from vfio_group_(un)pin_pages to vfio_(un)pin_pages

Use the existing vfio_device versions of vfio_(un)pin_pages(). There is no reason to use a group interface here, kvmgt has easy access to a vfio_device.

Delete kvmgt_vdev::vfio_group since these calls were the last users.

Reviewed-by: Kevin Tian kevin.tian@intel.com Reviewed-by: Christoph Hellwig hch@lst.de Signed-off-by: Jason Gunthorpe jgg@nvidia.com --- drivers/gpu/drm/i915/gvt/gvt.h | 1 - drivers/gpu/drm/i915/gvt/kvmgt.c | 27 ++++++--------------------- 2 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h index 5a28ee965b7f3e..2af4c83e733c6c 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.h +++ b/drivers/gpu/drm/i915/gvt/gvt.h @@ -231,7 +231,6 @@ struct intel_vgpu { struct kvm *kvm; struct work_struct release_work; atomic_t released; - struct vfio_group *vfio_group;

struct kvm_page_track_notifier_node track_node; #define NR_BKT (1 << 18) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 1cec4f1fdfaced..7655ffa97d5116 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -243,7 +243,7 @@ static void gvt_unpin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn, for (npage = 0; npage < total_pages; npage++) { unsigned long cur_gfn = gfn + npage;

- ret = vfio_group_unpin_pages(vgpu->vfio_group, &cur_gfn, 1); + ret = vfio_unpin_pages(&vgpu->vfio_device, &cur_gfn, 1); drm_WARN_ON(&i915->drm, ret != 1); } } @@ -266,8 +266,8 @@ static int gvt_pin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn, unsigned long cur_gfn = gfn + npage; unsigned long pfn;

- ret = vfio_group_pin_pages(vgpu->vfio_group, &cur_gfn, 1, - IOMMU_READ | IOMMU_WRITE, &pfn); + ret = vfio_pin_pages(&vgpu->vfio_device, &cur_gfn, 1, + IOMMU_READ | IOMMU_WRITE, &pfn); if (ret != 1) { gvt_vgpu_err("vfio_pin_pages failed for gfn 0x%lx, ret %d\n", cur_gfn, ret); @@ -804,7 +804,6 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev) struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev); unsigned long events; int ret; - struct vfio_group *vfio_group;

vgpu->iommu_notifier.notifier_call = intel_vgpu_iommu_notifier; vgpu->group_notifier.notifier_call = intel_vgpu_group_notifier; @@ -827,28 +826,19 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev) goto undo_iommu; }

- vfio_group = - vfio_group_get_external_user_from_dev(vgpu->vfio_device.dev); - if (IS_ERR_OR_NULL(vfio_group)) { - ret = !vfio_group ? -EFAULT : PTR_ERR(vfio_group); - gvt_vgpu_err("vfio_group_get_external_user_from_dev failed\n"); - goto undo_register; - } - vgpu->vfio_group = vfio_group; - ret = -EEXIST; if (vgpu->attached) - goto undo_group; + goto undo_register;

ret = -ESRCH; if (!vgpu->kvm || vgpu->kvm->mm != current->mm) { gvt_vgpu_err("KVM is required to use Intel vGPU\n"); - goto undo_group; + goto undo_register; }

ret = -EEXIST; if (__kvmgt_vgpu_exist(vgpu)) - goto undo_group; + goto undo_register;

vgpu->attached = true; kvm_get_kvm(vgpu->kvm); @@ -868,10 +858,6 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev) atomic_set(&vgpu->released, 0); return 0;

-undo_group: - vfio_group_put_external_user(vgpu->vfio_group); - vgpu->vfio_group = NULL; - undo_register: vfio_unregister_notifier(vfio_dev, VFIO_GROUP_NOTIFY, &vgpu->group_notifier); @@ -925,7 +911,6 @@ static void __intel_vgpu_release(struct intel_vgpu *vgpu) gvt_cache_destroy(vgpu);

intel_vgpu_release_msi_eventfd_ctx(vgpu); - vfio_group_put_external_user(vgpu->vfio_group);

vgpu->kvm = NULL; vgpu->attached = false;

-- 2.36.0

Wang, Zhi A

11 May 11 May

4:09 p.m.

New subject: [PATCH v4 5/7] drm/i915/gvt: Change from vfio_group_(un)pin_pages to vfio_(un)pin_pages

On 5/6/22 12:08 AM, Jason Gunthorpe wrote:

...

Use the existing vfio_device versions of vfio_(un)pin_pages(). There is no reason to use a group interface here, kvmgt has easy access to a vfio_device.

Delete kvmgt_vdev::vfio_group since these calls were the last users.

Reviewed-by: Kevin Tian kevin.tian@intel.com Reviewed-by: Christoph Hellwig hch@lst.de Signed-off-by: Jason Gunthorpe jgg@nvidia.com

drivers/gpu/drm/i915/gvt/gvt.h | 1 - drivers/gpu/drm/i915/gvt/kvmgt.c | 27 ++++++--------------------- 2 files changed, 6 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h index 5a28ee965b7f3e..2af4c83e733c6c 100644 --- a/drivers/gpu/drm/i915/gvt/gvt.h +++ b/drivers/gpu/drm/i915/gvt/gvt.h @@ -231,7 +231,6 @@ struct intel_vgpu { struct kvm *kvm; struct work_struct release_work; atomic_t released;

struct vfio_group *vfio_group;

struct kvm_page_track_notifier_node track_node;

#define NR_BKT (1 << 18) diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c index 1cec4f1fdfaced..7655ffa97d5116 100644 --- a/drivers/gpu/drm/i915/gvt/kvmgt.c +++ b/drivers/gpu/drm/i915/gvt/kvmgt.c @@ -243,7 +243,7 @@ static void gvt_unpin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn, for (npage = 0; npage < total_pages; npage++) { unsigned long cur_gfn = gfn + npage;
ret = vfio_group_unpin_pages(vgpu->vfio_group, &cur_gfn, 1);
ret = vfio_unpin_pages(&vgpu->vfio_device, &cur_gfn, 1);
drm_WARN_ON(&i915->drm, ret != 1); }
} @@ -266,8 +266,8 @@ static int gvt_pin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn, unsigned long cur_gfn = gfn + npage; unsigned long pfn;
ret = vfio_group_pin_pages(vgpu->vfio_group, &cur_gfn, 1,
			   IOMMU_READ | IOMMU_WRITE, &pfn);
ret = vfio_pin_pages(&vgpu->vfio_device, &cur_gfn, 1,
		     IOMMU_READ | IOMMU_WRITE, &pfn);
if (ret != 1) { gvt_vgpu_err("vfio_pin_pages failed for gfn 0x%lx, ret %d\n", cur_gfn, ret);
@@ -804,7 +804,6 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev) struct intel_vgpu *vgpu = vfio_dev_to_vgpu(vfio_dev); unsigned long events; int ret;

struct vfio_group *vfio_group;

vgpu->iommu_notifier.notifier_call = intel_vgpu_iommu_notifier; vgpu->group_notifier.notifier_call = intel_vgpu_group_notifier;

@@ -827,28 +826,19 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev) goto undo_iommu; }
vfio_group =
vfio_group_get_external_user_from_dev(vgpu->vfio_device.dev);
if (IS_ERR_OR_NULL(vfio_group)) {
ret = !vfio_group ? -EFAULT : PTR_ERR(vfio_group);
gvt_vgpu_err("vfio_group_get_external_user_from_dev failed\n");
goto undo_register;
}

vgpu->vfio_group = vfio_group;

ret = -EEXIST; if (vgpu->attached)
goto undo_group;
goto undo_register;
ret = -ESRCH; if (!vgpu->kvm || vgpu->kvm->mm != current->mm) { gvt_vgpu_err("KVM is required to use Intel vGPU\n");
goto undo_group;
goto undo_register;
}

ret = -EEXIST; if (__kvmgt_vgpu_exist(vgpu))
goto undo_group;
goto undo_register;
vgpu->attached = true; kvm_get_kvm(vgpu->kvm);
@@ -868,10 +858,6 @@ static int intel_vgpu_open_device(struct vfio_device *vfio_dev) atomic_set(&vgpu->released, 0); return 0;

-undo_group:

vfio_group_put_external_user(vgpu->vfio_group);

vgpu->vfio_group = NULL;

undo_register: vfio_unregister_notifier(vfio_dev, VFIO_GROUP_NOTIFY, &vgpu->group_notifier); @@ -925,7 +911,6 @@ static void __intel_vgpu_release(struct intel_vgpu *vgpu) gvt_cache_destroy(vgpu);

intel_vgpu_release_msi_eventfd_ctx(vgpu);

vfio_group_put_external_user(vgpu->vfio_group);

vgpu->kvm = NULL; vgpu->attached = false;

Acked-by: Zhi Wang zhi.a.wang@intel.com

Jason Gunthorpe

6 May 6 May

12:08 a.m.

New subject: [PATCH v4 6/7] vfio: Remove dead code

Now that callers have been updated to use the vfio_device APIs the driver facing group interface is no longer used, delete it:

- vfio_group_get_external_user_from_dev() - vfio_group_pin_pages() - vfio_group_unpin_pages() - vfio_group_iommu_domain()

Reviewed-by: Christoph Hellwig hch@lst.de Reviewed-by: Kevin Tian kevin.tian@intel.com Signed-off-by: Jason Gunthorpe jgg@nvidia.com -- FIXME: vfio_group_put_external_user() is removable too when combined with the KVM series --- drivers/vfio/vfio.c | 151 ------------------------------------------- include/linux/vfio.h | 11 ---- 2 files changed, 162 deletions(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index 85e1304099b8a5..c651c4805acd59 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -1732,44 +1732,6 @@ struct vfio_group *vfio_group_get_external_user(struct file *filep) } EXPORT_SYMBOL_GPL(vfio_group_get_external_user);

-/* - * External user API, exported by symbols to be linked dynamically. - * The external user passes in a device pointer - * to verify that: - * - A VFIO group is assiciated with the device; - * - IOMMU is set for the group. - * If both checks passed, vfio_group_get_external_user_from_dev() - * increments the container user counter to prevent the VFIO group - * from disposal before external user exits and returns the pointer - * to the VFIO group. - * - * When the external user finishes using the VFIO group, it calls - * vfio_group_put_external_user() to release the VFIO group and - * decrement the container user counter. - * - * @dev [in] : device - * Return error PTR or pointer to VFIO group. - */ - -struct vfio_group *vfio_group_get_external_user_from_dev(struct device *dev) -{ - struct vfio_group *group; - int ret; - - group = vfio_group_get_from_dev(dev); - if (!group) - return ERR_PTR(-ENODEV); - - ret = vfio_group_add_container_user(group); - if (ret) { - vfio_group_put(group); - return ERR_PTR(ret); - } - - return group; -} -EXPORT_SYMBOL_GPL(vfio_group_get_external_user_from_dev); - void vfio_group_put_external_user(struct vfio_group *group) { vfio_group_try_dissolve_container(group); @@ -2003,101 +1965,6 @@ int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn, } EXPORT_SYMBOL(vfio_unpin_pages);

-/* - * Pin a set of guest IOVA PFNs and return their associated host PFNs for a - * VFIO group. - * - * The caller needs to call vfio_group_get_external_user() or - * vfio_group_get_external_user_from_dev() prior to calling this interface, - * so as to prevent the VFIO group from disposal in the middle of the call. - * But it can keep the reference to the VFIO group for several calls into - * this interface. - * After finishing using of the VFIO group, the caller needs to release the - * VFIO group by calling vfio_group_put_external_user(). - * - * @group [in] : VFIO group - * @user_iova_pfn [in] : array of user/guest IOVA PFNs to be pinned. - * @npage [in] : count of elements in user_iova_pfn array. - * This count should not be greater - * VFIO_PIN_PAGES_MAX_ENTRIES. - * @prot [in] : protection flags - * @phys_pfn [out] : array of host PFNs - * Return error or number of pages pinned. - */ -int vfio_group_pin_pages(struct vfio_group *group, - unsigned long *user_iova_pfn, int npage, - int prot, unsigned long *phys_pfn) -{ - struct vfio_container *container; - struct vfio_iommu_driver *driver; - int ret; - - if (!group || !user_iova_pfn || !phys_pfn || !npage) - return -EINVAL; - - if (group->dev_counter > 1) - return -EINVAL; - - if (npage > VFIO_PIN_PAGES_MAX_ENTRIES) - return -E2BIG; - - container = group->container; - driver = container->iommu_driver; - if (likely(driver && driver->ops->pin_pages)) - ret = driver->ops->pin_pages(container->iommu_data, - group->iommu_group, user_iova_pfn, - npage, prot, phys_pfn); - else - ret = -ENOTTY; - - return ret; -} -EXPORT_SYMBOL(vfio_group_pin_pages); - -/* - * Unpin a set of guest IOVA PFNs for a VFIO group. - * - * The caller needs to call vfio_group_get_external_user() or - * vfio_group_get_external_user_from_dev() prior to calling this interface, - * so as to prevent the VFIO group from disposal in the middle of the call. - * But it can keep the reference to the VFIO group for several calls into - * this interface. - * After finishing using of the VFIO group, the caller needs to release the - * VFIO group by calling vfio_group_put_external_user(). - * - * @group [in] : vfio group - * @user_iova_pfn [in] : array of user/guest IOVA PFNs to be unpinned. - * @npage [in] : count of elements in user_iova_pfn array. - * This count should not be greater than - * VFIO_PIN_PAGES_MAX_ENTRIES. - * Return error or number of pages unpinned. - */ -int vfio_group_unpin_pages(struct vfio_group *group, - unsigned long *user_iova_pfn, int npage) -{ - struct vfio_container *container; - struct vfio_iommu_driver *driver; - int ret; - - if (!group || !user_iova_pfn || !npage) - return -EINVAL; - - if (npage > VFIO_PIN_PAGES_MAX_ENTRIES) - return -E2BIG; - - container = group->container; - driver = container->iommu_driver; - if (likely(driver && driver->ops->unpin_pages)) - ret = driver->ops->unpin_pages(container->iommu_data, - user_iova_pfn, npage); - else - ret = -ENOTTY; - - return ret; -} -EXPORT_SYMBOL(vfio_group_unpin_pages); - - /* * This interface allows the CPUs to perform some sort of virtual DMA on * behalf of the device. @@ -2301,24 +2168,6 @@ int vfio_unregister_notifier(struct vfio_device *device, } EXPORT_SYMBOL(vfio_unregister_notifier);

-struct iommu_domain *vfio_group_iommu_domain(struct vfio_group *group) -{ - struct vfio_container *container; - struct vfio_iommu_driver *driver; - - if (!group) - return ERR_PTR(-EINVAL); - - container = group->container; - driver = container->iommu_driver; - if (likely(driver && driver->ops->group_iommu_domain)) - return driver->ops->group_iommu_domain(container->iommu_data, - group->iommu_group); - - return ERR_PTR(-ENOTTY); -} -EXPORT_SYMBOL_GPL(vfio_group_iommu_domain); - /* * Module/class support */ diff --git a/include/linux/vfio.h b/include/linux/vfio.h index 8a151025871776..6195edd2edcd7b 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -140,8 +140,6 @@ int vfio_mig_get_next_state(struct vfio_device *device, */ extern struct vfio_group *vfio_group_get_external_user(struct file *filep); extern void vfio_group_put_external_user(struct vfio_group *group); -extern struct vfio_group *vfio_group_get_external_user_from_dev(struct device - *dev); extern bool vfio_external_group_match_file(struct vfio_group *group, struct file *filep); extern int vfio_external_user_iommu_id(struct vfio_group *group); @@ -154,18 +152,9 @@ extern int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn, int npage, int prot, unsigned long *phys_pfn); extern int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn, int npage); - -extern int vfio_group_pin_pages(struct vfio_group *group, - unsigned long *user_iova_pfn, int npage, - int prot, unsigned long *phys_pfn); -extern int vfio_group_unpin_pages(struct vfio_group *group, - unsigned long *user_iova_pfn, int npage); - extern int vfio_dma_rw(struct vfio_device *device, dma_addr_t user_iova, void *data, size_t len, bool write);

-extern struct iommu_domain *vfio_group_iommu_domain(struct vfio_group *group); - /* each type has independent events */ enum vfio_notify_type { VFIO_IOMMU_NOTIFY = 0,

-- 2.36.0

Jason Gunthorpe

12:08 a.m.

New subject: [PATCH v4 7/7] vfio: Remove calls to vfio_group_add_container_user()

When the open_device() op is called the container_users is incremented and held incremented until close_device(). Thus, so long as drivers call functions within their open_device()/close_device() region they do not need to worry about the container_users.

These functions can all only be called between open_device() and close_device():

vfio_pin_pages() vfio_unpin_pages() vfio_dma_rw() vfio_register_notifier() vfio_unregister_notifier()

Eliminate the calls to vfio_group_add_container_user() and add vfio_assert_device_open() to detect driver mis-use. This causes the close_device() op to check device->open_count so always leave it elevated while calling the op.

Reviewed-by: Christoph Hellwig hch@lst.de Reviewed-by: Kevin Tian kevin.tian@intel.com Signed-off-by: Jason Gunthorpe jgg@nvidia.com --- drivers/vfio/vfio.c | 80 ++++++++++----------------------------------- 1 file changed, 17 insertions(+), 63 deletions(-)

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c index c651c4805acd59..8bb38941c1dfd8 100644 --- a/drivers/vfio/vfio.c +++ b/drivers/vfio/vfio.c @@ -1115,6 +1115,12 @@ static int vfio_group_add_container_user(struct vfio_group *group)

static const struct file_operations vfio_device_fops;

+/* true if the vfio_device has open_device() called but not close_device() */ +static bool vfio_assert_device_open(struct vfio_device *device) +{ + return !WARN_ON_ONCE(!READ_ONCE(device->open_count)); +} + static int vfio_group_get_device_fd(struct vfio_group *group, char *buf) { struct vfio_device *device; @@ -1329,8 +1335,10 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep) struct vfio_device *device = filep->private_data;

mutex_lock(&device->dev_set->lock); - if (!--device->open_count && device->ops->close_device) + vfio_assert_device_open(device); + if (device->open_count == 1 && device->ops->close_device) device->ops->close_device(device); + device->open_count--; mutex_unlock(&device->dev_set->lock);

module_put(device->dev->driver->owner); @@ -1897,7 +1905,8 @@ int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn, struct vfio_iommu_driver *driver; int ret;

- if (!user_pfn || !phys_pfn || !npage) + if (!user_pfn || !phys_pfn || !npage || + !vfio_assert_device_open(device)) return -EINVAL;

if (npage > VFIO_PIN_PAGES_MAX_ENTRIES) @@ -1906,10 +1915,6 @@ int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn, if (group->dev_counter > 1) return -EINVAL;

- ret = vfio_group_add_container_user(group); - if (ret) - return ret; - container = group->container; driver = container->iommu_driver; if (likely(driver && driver->ops->pin_pages)) @@ -1919,8 +1924,6 @@ int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn, else ret = -ENOTTY;

- vfio_group_try_dissolve_container(group); - return ret; } EXPORT_SYMBOL(vfio_pin_pages); @@ -1941,16 +1944,12 @@ int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn, struct vfio_iommu_driver *driver; int ret;

- if (!user_pfn || !npage) + if (!user_pfn || !npage || !vfio_assert_device_open(device)) return -EINVAL;

if (npage > VFIO_PIN_PAGES_MAX_ENTRIES) return -E2BIG;

- ret = vfio_group_add_container_user(device->group); - if (ret) - return ret; - container = device->group->container; driver = container->iommu_driver; if (likely(driver && driver->ops->unpin_pages)) @@ -1959,8 +1958,6 @@ int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn, else ret = -ENOTTY;

- vfio_group_try_dissolve_container(device->group); - return ret; } EXPORT_SYMBOL(vfio_unpin_pages); @@ -1989,13 +1986,9 @@ int vfio_dma_rw(struct vfio_device *device, dma_addr_t user_iova, void *data, struct vfio_iommu_driver *driver; int ret = 0;

- if (!data || len <= 0) + if (!data || len <= 0 || !vfio_assert_device_open(device)) return -EINVAL;

- ret = vfio_group_add_container_user(device->group); - if (ret) - return ret; - container = device->group->container; driver = container->iommu_driver;

@@ -2004,9 +1997,6 @@ int vfio_dma_rw(struct vfio_device *device, dma_addr_t user_iova, void *data, user_iova, data, len, write); else ret = -ENOTTY; - - vfio_group_try_dissolve_container(device->group); - return ret; } EXPORT_SYMBOL(vfio_dma_rw); @@ -2019,10 +2009,6 @@ static int vfio_register_iommu_notifier(struct vfio_group *group, struct vfio_iommu_driver *driver; int ret;

- ret = vfio_group_add_container_user(group); - if (ret) - return -EINVAL; - container = group->container; driver = container->iommu_driver; if (likely(driver && driver->ops->register_notifier)) @@ -2030,9 +2016,6 @@ static int vfio_register_iommu_notifier(struct vfio_group *group, events, nb); else ret = -ENOTTY; - - vfio_group_try_dissolve_container(group); - return ret; }

@@ -2043,10 +2026,6 @@ static int vfio_unregister_iommu_notifier(struct vfio_group *group, struct vfio_iommu_driver *driver; int ret;

- ret = vfio_group_add_container_user(group); - if (ret) - return -EINVAL; - container = group->container; driver = container->iommu_driver; if (likely(driver && driver->ops->unregister_notifier)) @@ -2054,9 +2033,6 @@ static int vfio_unregister_iommu_notifier(struct vfio_group *group, nb); else ret = -ENOTTY; - - vfio_group_try_dissolve_container(group); - return ret; }

@@ -2085,10 +2061,6 @@ static int vfio_register_group_notifier(struct vfio_group *group, if (*events) return -EINVAL;

- ret = vfio_group_add_container_user(group); - if (ret) - return -EINVAL; - ret = blocking_notifier_chain_register(&group->notifier, nb);

/* @@ -2098,25 +2070,6 @@ static int vfio_register_group_notifier(struct vfio_group *group, if (!ret && set_kvm && group->kvm) blocking_notifier_call_chain(&group->notifier, VFIO_GROUP_NOTIFY_SET_KVM, group->kvm); - - vfio_group_try_dissolve_container(group); - - return ret; -} - -static int vfio_unregister_group_notifier(struct vfio_group *group, - struct notifier_block *nb) -{ - int ret; - - ret = vfio_group_add_container_user(group); - if (ret) - return -EINVAL; - - ret = blocking_notifier_chain_unregister(&group->notifier, nb); - - vfio_group_try_dissolve_container(group); - return ret; }

@@ -2127,7 +2080,8 @@ int vfio_register_notifier(struct vfio_device *device, struct vfio_group *group = device->group; int ret;

- if (!nb || !events || (*events == 0)) + if (!nb || !events || (*events == 0) || + !vfio_assert_device_open(device)) return -EINVAL;

switch (type) { @@ -2151,7 +2105,7 @@ int vfio_unregister_notifier(struct vfio_device *device, struct vfio_group *group = device->group; int ret;

- if (!nb) + if (!nb || !vfio_assert_device_open(device)) return -EINVAL;

switch (type) { @@ -2159,7 +2113,7 @@ int vfio_unregister_notifier(struct vfio_device *device, ret = vfio_unregister_iommu_notifier(group, nb); break; case VFIO_GROUP_NOTIFY: - ret = vfio_unregister_group_notifier(group, nb); + ret = blocking_notifier_chain_unregister(&group->notifier, nb); break; default: ret = -EINVAL;

-- 2.36.0

Alex Williamson

12 May 12 May

6:21 p.m.

On Thu, 5 May 2022 21:08:38 -0300 Jason Gunthorpe jgg@nvidia.com wrote:

...

Prior series have transformed other parts of VFIO from working on struct device or struct vfio_group into working directly on struct vfio_device. Based on that work we now have vfio_device's readily available in all the drivers.

Update the rest of the driver facing API to use vfio_device as an input.

The following are switched from struct device to struct vfio_device: vfio_register_notifier() vfio_unregister_notifier() vfio_pin_pages() vfio_unpin_pages() vfio_dma_rw()

The following group APIs are obsoleted and removed by just using struct vfio_device with the above: vfio_group_pin_pages() vfio_group_unpin_pages() vfio_group_iommu_domain() vfio_group_get_external_user_from_dev()

To retain the performance of the new device APIs relative to their group versions optimize how vfio_group_add_container_user() is used to avoid calling it when the driver must already guarantee the device is open and the container_users incrd.

The remaining exported VFIO group interfaces are only used by kvm, and are addressed by a parallel series.

This series is based on Christoph's gvt rework here:

https://lore.kernel.org/all/5a8b9f48-2c32-8177-1c18-e3bd7bfde558@intel.com/

and so will need the PR merged first.

I have a followup series that needs this.

This is also part of the iommufd work - moving the driver facing interface to vfio_device provides a much cleaner path to integrate with iommufd.

v4:

Use 'device' as the argument name for a struct vfio_device in vfio.c

v3: https://lore.kernel.org/r/0-v3-e131a9b6b467+14b6-vfio_mdev_no_group_jgg@nvid...

Based on VFIO's gvt/iommu merge

Remove mention of mdev_legacy_get_vfio_device() from commit message

Clarify commit message for vfio_dma_rw() conversion

Talk about the open_count change in the commit message

No code change

v2: https://lore.kernel.org/r/0-v2-6011bde8e0a1+5f-vfio_mdev_no_group_jgg@nvidia...

Based on Christoph's series so mdev_legacy_get_vfio_device() is removed

Reflow indenting

Use vfio_assert_device_open() and WARN_ON_ONCE instead of open coding the assertion

v1: https://lore.kernel.org/r/0-v1-a8faf768d202+125dd-vfio_mdev_no_group_jgg@nvi...

Jason Gunthorpe (7): vfio: Make vfio_(un)register_notifier accept a vfio_device vfio/ccw: Remove mdev from struct channel_program vfio/mdev: Pass in a struct vfio_device * to vfio_pin/unpin_pages() vfio/mdev: Pass in a struct vfio_device * to vfio_dma_rw() drm/i915/gvt: Change from vfio_group_(un)pin_pages to vfio_(un)pin_pages vfio: Remove dead code vfio: Remove calls to vfio_group_add_container_user()

.../driver-api/vfio-mediated-device.rst | 4 +- drivers/gpu/drm/i915/gvt/gvt.h | 5 +- drivers/gpu/drm/i915/gvt/kvmgt.c | 51 ++- drivers/s390/cio/vfio_ccw_cp.c | 47 +-- drivers/s390/cio/vfio_ccw_cp.h | 4 +- drivers/s390/cio/vfio_ccw_fsm.c | 3 +- drivers/s390/cio/vfio_ccw_ops.c | 7 +- drivers/s390/crypto/vfio_ap_ops.c | 23 +- drivers/vfio/vfio.c | 299 +++--------------- include/linux/vfio.h | 21 +- 10 files changed, 109 insertions(+), 355 deletions(-)

Applied to vfio next branch for v5.19. Thanks,

Alex

1084

Age (days ago)

1090

Last active (days ago)

dri-devel@lists.freedesktop.org

9 comments

3 participants

tags (0)

participants (3)

Alex Williamson
Jason Gunthorpe
Wang, Zhi A