On Wed, Jun 10, 2026 at 04:43:21PM +0100, Matt Evans wrote:
Hi Matt,
[...]
** With the goal of taking vdev->memory_lock in a world where* vdev might not still exist:** 1. Take the resv lock on the DMABUF:* - If racing cleanup got in first, the buffer is revoked;* stop/exit if so.* - If we got in first, the buffer is not revoked so vdev is* non-NULL, accessible, and cleanup _has not yet put the* VFIO device registration_. So, the device refcount must* be >0.** 2. Take vfio_device registration (refcount guaranteed >0* hereafter).** 3. Unlock the DMABUF's resv lock:* - A racing cleanup can now complete.* - But, the device refcount >0, meaning the vfio_device* (and vfio_pcie_core device vdev) have not yet been* freed. vdev is accessible, even if the DMABUF has been* revoked or cleanup has happened, because* vfio_unregister_group_dev() can't complete.** 4. Take the vdev->memory_lock* - Either the DMABUF is usable, or has been cleaned up.* Whichever, it can no longer change under us.* - Test the DMABUF revocation status again: if it was* revoked between 1 and 4 return a SIGBUS. Otherwise,* return a PFN.* - It's not necessary to also take the resv lock, because* the status/vdev can't change while memory_lock is held.* */* 5. Unlock, done.- dma_resv_lock(priv->dmabuf->resv, NULL);
- if (priv->revoked) {
pr_debug_ratelimited("%s VA 0x%lx, pgoff 0x%lx: DMABUF revoked/cleaned up\n",__func__, vmf->address, vma->vm_pgoff);dma_resv_unlock(priv->dmabuf->resv);return VM_FAULT_SIGBUS;- }
- /* If the buffer isn't revoked, vdev is valid */ vdev = priv->vdev;
- if (!vfio_device_try_get_registration(&vdev->vdev)) {
/** If vdev != NULL (above), the registration should* already be >0 and so this try_get should never* fail.*/dev_warn(&vdev->pdev->dev, "%s: Unexpected registration failure\n",__func__);dma_resv_unlock(priv->dmabuf->resv);return VM_FAULT_SIGBUS;- }
- dma_resv_unlock(priv->dmabuf->resv);
scoped_guard(rwsem_read, &vdev->memory_lock) {
if (!priv->revoked) { int pres = vfio_pci_dma_buf_find_pfn(priv, vma, vmf->address,/* Revocation status must be re-read, under memory_lock */
Wait, I noticed that the is_aligned_for_order() check from mainline was removed here. Was that intentional?
For hugepage faults (order > 0), we must ensure the PFN and address are properly aligned before calling vfio_pci_vmf_insert_pfn().
In the current upstream code, we have: if (is_aligned_for_order(vma, addr, pfn, order))
Should we restore that check here?
@@ -1766,6 +1827,7 @@ static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf, __func__, order, pfn, vmf->address, vma->vm_pgoff, (unsigned int)ret);
- vfio_device_put_registration(&vdev->vdev); return ret;
} @@ -1774,7 +1836,7 @@ static vm_fault_t vfio_pci_mmap_page_fault(struct vm_fault *vmf) return vfio_pci_mmap_huge_fault(vmf, 0); } -static const struct vm_operations_struct vfio_pci_mmap_ops = { +const struct vm_operations_struct vfio_pci_mmap_ops = { .fault = vfio_pci_mmap_page_fault,
Nit: Instead of making this global, should we add a helper? E.g.:
void vfio_pci_set_vma_ops(struct vm_area_struct *vma) { vma->vm_ops = &vfio_pci_mmap_ops; }
[...]
+static int vfio_pci_dma_buf_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma) +{
- struct vfio_pci_dma_buf *priv = dmabuf->priv;
- /*
* If we observe that the buffer is revoked now then refuse* the mmap(). This is a belt-and-braces early failure to* ease debugging a revoked buffer being used. Userspace* might also race an mmap() against an explicit revocation,* or an action doing a temporary revoke; race scenarios are* still safe because the fault handler ultimately prevents* access to a revoked buffer if it isn't caught here.*/- if (READ_ONCE(priv->revoked))
return -ENODEV;- if ((vma->vm_flags & VM_SHARED) == 0)
return -EINVAL;- /*
* dma_buf_mmap_internal() has asserted that the VMA is* contained within the DMABUF size before calling this.*/- vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
- vma->vm_page_prot = pgprot_decrypted(vma->vm_page_prot);
- /* See comments in vfio_pci_core_mmap() re VM_ALLOW_ANY_UNCACHED. */
- vm_flags_set(vma, VM_ALLOW_ANY_UNCACHED | VM_IO | VM_PFNMAP |
VM_DONTEXPAND | VM_DONTDUMP);- vma->vm_private_data = priv;
- vma->vm_ops = &vfio_pci_mmap_ops;
- return 0;
+} #endif /* CONFIG_VFIO_PCI_DMABUF */
Thanks, Praan
linaro-mm-sig@lists.linaro.org