Re: [PATCH v3 8/9] vfio/pci: Permanently revoke a DMABUF on request

[Matt Evans]

Hi Praan,

On 16/06/2026 09:05, Pranjal Shrivastava wrote:
> On Wed, Jun 10, 2026 at 04:43:22PM +0100, Matt Evans wrote:
>> Expand the VFIO DMABUF revocation state to three states:
>> Not revoked, temporarily revoked, and permanently revoked.
>>
>> The first two are for existing transient revocation, e.g. across a
>> function reset, and the DMABUF is put into the last in response to a
>> new VFIO feature VFIO_DEVICE_FEATURE_DMA_BUF.
>>
>> VFIO_DEVICE_FEATURE_DMA_BUF passes a DMABUF by fd and requests that
>> the DMABUF is permanently revoked.  On success, it's guaranteed that
>> the buffer can never be imported/attached/mmap()ed in future, that
>> dynamic imports have been cleanly detached, and that all mappings have
>> been made inaccessible/PTEs zapped.
>>
>> This is useful for lifecycle management, to reclaim VFIO PCI BAR
>> ranges previously delegated to a subordinate client process: The
>> driver process can ensure that the loaned resources are revoked when
>> the client is deemed "done", and exported ranges can be safely re-used
>> elsewhere.
>>
>> Refactor the revocation code out of vfio_pci_dma_buf_move() to a
>> function common to move and the new feature request path.
>>
>> Signed-off-by: Matt Evans <matt@xxxxxxxxxx>
>> ---
>>  drivers/vfio/pci/vfio_pci_core.c   |   6 +-
>>  drivers/vfio/pci/vfio_pci_dmabuf.c | 169 ++++++++++++++++++++++-------
>>  drivers/vfio/pci/vfio_pci_priv.h   |  19 +++-
>>  include/uapi/linux/vfio.h          |  20 ++++
>>  4 files changed, 173 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>> index 508a5eca910a..064906b25467 100644
> 
> [...]
> 
>>  
>> +/* Set the DMABUF's revocation status (OK or temporarily/permanently revoked) */
>> +static void vfio_pci_dma_buf_set_status(struct vfio_pci_dma_buf *priv,
>> +					enum vfio_pci_dma_buf_status new_status)
>> +{
>> +	bool was_revoked;
>> +
>> +	lockdep_assert_held_write(&priv->vdev->memory_lock);
>> +
>> +	if (priv->status == VFIO_PCI_DMABUF_PERM_REVOKED ||
>> +	    priv->status == new_status) {
>> +		return;
>> +	}
>> +
>> +	dma_resv_lock(priv->dmabuf->resv, NULL);
>> +	was_revoked = (priv->status == VFIO_PCI_DMABUF_TEMP_REVOKED);
>> +
>> +	if (new_status != VFIO_PCI_DMABUF_OK) {
>> +		priv->status = new_status; /* Temp or permanently revoked */
>> +
>> +		if (was_revoked) {
>> +			/*
>> +			 * TEMP_REVOKED is being upgraded to
>> +			 * PERM_REVOKED.  The buffer is already gone,
>> +			 * don't wait on it again.
>> +			 */
>> +			dma_resv_unlock(priv->dmabuf->resv);
>> +			return;
>> +		}
>> +	}
>> +
>> +	dma_buf_invalidate_mappings(priv->dmabuf);
> 
> Nit: We seem to be calling this even if new_status == OK, while it works
> as importers (like IOMMUFD and RDMA core) are immune to a double 
> invalidate / revoke. I'm wondering if we could move this within the 
> if (new_status != VFIO_PCI_DMABUF_OK) block? 
> 
> Since this is only needed to be called when we TEMP/PERM _REVOKE?
> 
> I'm just worried that this may overload the dma_buf_invalidate_mappings 
> to be a state-change notification instead of a revoke / invalidate
> notification.

(In case you didn't see my reply to Kevin who made a similar request: 
https://lore.kernel.org/all/31d1265b-e264-4dc6-a8c3-1b64dc9867a1@xxxxxxxxxx/ )

Done, moved to a common `!= VFIO_PCI_DMABUF_OK` block.

Just to flag that since it's currently called for both revoke and 
un-revoke paths, someone could already rely on this as a state-change 
notification.  I don't see any current examples, but giving it a mention 
in case any readers know of any.

>> +	dma_resv_wait_timeout(priv->dmabuf->resv,
>> +			      DMA_RESV_USAGE_BOOKKEEP, false,
>> +			      MAX_SCHEDULE_TIMEOUT);
>> +	dma_resv_unlock(priv->dmabuf->resv);
>> +	if (new_status != VFIO_PCI_DMABUF_OK) {
>> +		kref_put(&priv->kref, vfio_pci_dma_buf_done);
>> +		wait_for_completion(&priv->comp);
>> +		unmap_mapping_range(priv->dmabuf->file->f_mapping,
>> +				    0, priv->size, 1);
>> +		/*
>> +		 * Re-arm the registered kref reference and the
>> +		 * completion so the post-revoke state matches the
>> +		 * post-creation state.	 An un-revoke followed by a
>> +		 * new mapping needs the kref to be non-zero before
>> +		 * kref_get(), and vfio_pci_dma_buf_cleanup()
>> +		 * delegates its drain back through this revoke
>> +		 * path on a possibly-already-revoked dma-buf.
>> +		 */
>> +		kref_init(&priv->kref);
>> +		reinit_completion(&priv->comp);
>> +	} else {
>> +		dma_resv_lock(priv->dmabuf->resv, NULL);
>> +		priv->status = VFIO_PCI_DMABUF_OK;
>> +		dma_resv_unlock(priv->dmabuf->resv);
>> +	}
>> +}
>> +
> 
> Otherwise, 
> Reviewed-by: Pranjal Shrivastava <praan@xxxxxxxxxx>
> 
> Thanks,
> Praan

Thanks!


Matt