Hi Alex,

On 14/10/2026 00:13, Alex Williamson wrote:
On Tue,  6 Oct 2026 20:36:37 +0100
Matt Evans <[email protected]> wrote:
+#ifdef CONFIG_VFIO_PCI_DMABUF
+int vfio_pci_core_feature_dma_buf_revoke(
+       struct vfio_pci_core_device *vdev, u32 flags,
+       struct vfio_device_feature_dma_buf_revoke __user *arg,
+       size_t argsz)
+{
+       struct vfio_device_feature_dma_buf_revoke db_revoke;
+       struct vfio_pci_dma_buf *priv;
+       struct dma_buf *dmabuf;
+       int ret;
+
+       if (!vdev->pci_ops || !vdev->pci_ops->get_dmabuf_phys)
+               return -EOPNOTSUPP;
+
+       ret = vfio_check_feature(flags, argsz,
+                                VFIO_DEVICE_FEATURE_SET,
+                                sizeof(db_revoke));
+       if (ret != 1)
+               return ret;
+
+       if (copy_from_user(&db_revoke, arg, sizeof(db_revoke)))
+               return -EFAULT;
+
+       dmabuf = dma_buf_get(db_revoke.dmabuf_fd);
+       if (IS_ERR(dmabuf))
+               return PTR_ERR(dmabuf);
+
+       priv = dmabuf->priv;
+       /*
+        * Sanity-check the DMABUF is really a vfio_pci_dma_buf _and_
+        * relates to the VFIO device it was provided with.
+        *
+        * If the DMABUF relates to this vdev then priv->vdev is
+        * stable because this open fd prevents cleanup.
+        *
+        * If it relates to a different vdev, reading priv->vdev might
+        * race with a concurrent cleanup on that device.  But if so,
+        * it points to a non-matching vdev or NULL and is unusable
+        * either way.
+        */
+       if (dmabuf->ops != &vfio_pci_dmabuf_ops ||
+           READ_ONCE(priv->vdev) != vdev) {
+               ret = -ENODEV;
+               goto out_put_buf;
+       }
+
+       /*
+        * memory_lock(R) is taken to stop vfio_pci_dev_set_hot_reset()
+        * from getting it and then blocking all devices in the dev_set behind
+        * this revoke's drain.
+        */
+       down_read(&vdev->memory_lock);
+       down_write(&vdev->dmabuf_lock);
+       if (priv->status == VFIO_PCI_DMABUF_DEAD) {
+               ret = -EBADFD;

Kind of ugly to special case the condition in the caller that's already
tested in the function.  Why not return an errno for this purpose?
Thanks,

Sure, especially since that comparison's already there.

(Rather than generating -EBADFD in the function (that error doesn't make a lot of sense outside of this ioctl), it can return $anything and get converted here.)

Thanks,


Matt


Alex

+       } else {
+               vfio_pci_dma_buf_set_status(priv, VFIO_PCI_DMABUF_DEAD);
+               ret = 0;
+       }
+       up_write(&vdev->dmabuf_lock);
+       up_read(&vdev->memory_lock);
+
+out_put_buf:
+       dma_buf_put(dmabuf);
+
+       return ret;
+}
+#endif /* CONFIG_VFIO_PCI_DMABUF */


Reply via email to