vfio: Make vfio_unpin_pages() return void

There's only one caller that checks its return value with a WARN_ON_ONCE,
while all other callers don't check the return value at all. Above that,
an undo function should not fail. So, simplify the API to return void by
embedding similar WARN_ONs.

Also for users to pinpoint which condition fails, separate WARN_ON lines,
yet remove the "driver->ops->unpin_pages" check, since it's unreasonable
for callers to unpin on something totally random that wasn't even pinned.
And remove NULL pointer checks for they would trigger oops vs. warnings.
Note that npage is already validated in the vfio core, thus drop the same
check in the type1 code.

Suggested-by: Christoph Hellwig <hch@infradead.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
Reviewed-by: Kirti Wankhede <kwankhede@nvidia.com>
Tested-by: Terrence Xu <terrence.xu@intel.com>
Signed-off-by: Nicolin Chen <nicolinc@nvidia.com>
Link: https://lore.kernel.org/r/20220723020256.30081-2-nicolinc@nvidia.com
Signed-off-by: Alex Williamson <alex.williamson@redhat.com>

diff --git a/Documentation/driver-api/vfio-mediated-device.rst b/Documentation/driver-api/vfio-mediated-device.rst
index 1c57815619fdfbbf56219852edaf1ef18a51024f..b0fdf76b339a4e08d45265e687a73182472e7b01 100644 (file)
--- a/Documentation/driver-api/vfio-mediated-device.rst
+++ b/Documentation/driver-api/vfio-mediated-device.rst
@@ -265,7 +265,7 @@ driver::
        int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn,
                                  int npage, int prot, unsigned long *phys_pfn);
 
-       int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn,
+       void 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/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index ecd5bb37b63a2aec2b3dcbbb68f6f9f724df9c49..4d32a27489588ce6213a935c5023c74db3bf1b1e 100644 (file)
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -231,18 +231,15 @@ static void intel_gvt_cleanup_vgpu_type_groups(struct intel_gvt *gvt)
 static void gvt_unpin_guest_page(struct intel_vgpu *vgpu, unsigned long gfn,
                unsigned long size)
 {
-       struct drm_i915_private *i915 = vgpu->gvt->gt->i915;
        int total_pages;
        int npage;
-       int ret;
 
        total_pages = roundup(size, PAGE_SIZE) / PAGE_SIZE;
 
        for (npage = 0; npage < total_pages; npage++) {
                unsigned long cur_gfn = gfn + npage;
 
-               ret = vfio_unpin_pages(&vgpu->vfio_device, &cur_gfn, 1);
-               drm_WARN_ON(&i915->drm, ret != 1);
+               vfio_unpin_pages(&vgpu->vfio_device, &cur_gfn, 1);
        }
 }
 

diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
index b3ce8073cfb1fe54f629e59912c0a5ca114c831c..92b10aafae28b89188cca45f48938f59bffb90c8 100644 (file)
--- a/drivers/vfio/vfio.c
+++ b/drivers/vfio/vfio.c
@@ -1983,31 +1983,24 @@ EXPORT_SYMBOL(vfio_pin_pages);
  *                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 vfio_device *device, unsigned long *user_pfn,
-                    int npage)
+void vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn,
+                     int npage)
 {
        struct vfio_container *container;
        struct vfio_iommu_driver *driver;
-       int ret;
 
-       if (!user_pfn || !npage || !vfio_assert_device_open(device))
-               return -EINVAL;
+       if (WARN_ON(npage <= 0 || npage > VFIO_PIN_PAGES_MAX_ENTRIES))
+               return;
 
-       if (npage > VFIO_PIN_PAGES_MAX_ENTRIES)
-               return -E2BIG;
+       if (WARN_ON(!vfio_assert_device_open(device)))
+               return;
 
        /* group->container cannot change while a vfio device is open */
        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,
-                                              npage);
-       else
-               ret = -ENOTTY;
 
-       return ret;
+       driver->ops->unpin_pages(container->iommu_data, user_pfn, npage);
 }
 EXPORT_SYMBOL(vfio_unpin_pages);
 

diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h
index 4a7db1f3c33e7e9d9c4d779e24d8396bc06ef726..6a8424b407c7edc7b011f5aabfbdfea6b79ead98 100644 (file)
--- a/drivers/vfio/vfio.h
+++ b/drivers/vfio/vfio.h
@@ -53,7 +53,7 @@ struct vfio_iommu_driver_ops {
                                     unsigned long *user_pfn,
                                     int npage, int prot,
                                     unsigned long *phys_pfn);
-       int             (*unpin_pages)(void *iommu_data,
+       void            (*unpin_pages)(void *iommu_data,
                                       unsigned long *user_pfn, int npage);
        void            (*register_device)(void *iommu_data,
                                           struct vfio_device *vdev);

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 026a1d2553a269c7ad48225a21875fe5178091f9..e49fbe9968efe8e65d84caf54c9f01fa5a8ef574 100644 (file)
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -949,20 +949,16 @@ pin_done:
        return ret;
 }
 
-static int vfio_iommu_type1_unpin_pages(void *iommu_data,
-                                       unsigned long *user_pfn,
-                                       int npage)
+static void vfio_iommu_type1_unpin_pages(void *iommu_data,
+                                        unsigned long *user_pfn, int npage)
 {
        struct vfio_iommu *iommu = iommu_data;
        bool do_accounting;
        int i;
 
-       if (!iommu || !user_pfn || npage <= 0)
-               return -EINVAL;
-
        /* Supported for v2 version only */
-       if (!iommu->v2)
-               return -EACCES;
+       if (WARN_ON(!iommu->v2))
+               return;
 
        mutex_lock(&iommu->lock);
 
@@ -980,7 +976,8 @@ static int vfio_iommu_type1_unpin_pages(void *iommu_data,
        }
 
        mutex_unlock(&iommu->lock);
-       return i > 0 ? i : -EINVAL;
+
+       WARN_ON(i != npage);
 }
 
 static long vfio_sync_unpin(struct vfio_dma *dma, struct vfio_domain *domain,

diff --git a/include/linux/vfio.h b/include/linux/vfio.h
index 19cefbaa3d06a056288b3fd7d08ff9df8a72ac93..9f7d74c24925af1fc984b5eb2da57936523a0b9e 100644 (file)
--- a/include/linux/vfio.h
+++ b/include/linux/vfio.h
@@ -163,8 +163,8 @@ bool vfio_file_has_dev(struct file *file, struct vfio_device *device);
 
 int vfio_pin_pages(struct vfio_device *device, unsigned long *user_pfn,
                   int npage, int prot, unsigned long *phys_pfn);
-int vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn,
-                    int npage);
+void vfio_unpin_pages(struct vfio_device *device, unsigned long *user_pfn,
+                     int npage);
 int vfio_dma_rw(struct vfio_device *device, dma_addr_t user_iova,
                void *data, size_t len, bool write);