From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: vfio/pci: Support mmap() of a VFIO DMABUF
Date: Thu, 28 May 2026 12:28:53 +1000
Message-ID: <review-patch7-20260527102319.100128-8-mattev@meta.com>
In-Reply-To: <20260527102319.100128-8-mattev@meta.com>
References: <20260527102319.100128-1-mattev@meta.com> <20260527102319.100128-8-mattev@meta.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

This is the most complex patch. Adds `fops->mmap` for DMABUF fds and handles the lifecycle where the device fd might close while a DMABUF VMA fault is in-flight.

**The 5-step synchronization protocol** in `vfio_pci_mmap_huge_fault` is the core contribution. The logic is:

1. Lock DMABUF resv -> check revoked/vdev
2. `vfio_device_try_get_registration` (guaranteed >0 here)
3. Unlock resv (cleanup can now proceed)
4. Take `memory_lock`, re-check revocation
5. Service fault or SIGBUS

This is correct but very subtle. The key invariant is that cleanup holds `resv` when setting `revoked`, so if the fault sees `!revoked` under `resv`, the device registration must still be positive.

**`vfio_pci_dma_buf_mmap` hardcodes UC**:
```c
+	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
```
This is overridden in patch 9 to support WC. Fine for the series ordering.

**Missing `.mmap` for non-DMABUF case**: The `vfio_pci_dmabuf_ops` struct conditionally includes `.mmap` only under `CONFIG_VFIO_PCI_DMABUF`. But the BAR mmap path (which always creates DMABUFs) never goes through `dma_buf_mmap()` -- it installs `vfio_pci_mmap_ops` directly. So this is correct: `.mmap` is only needed for explicitly-exported DMABUFs.

---
Generated by Claude Code Patch Reviewer