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

["Christian König" <christian.koenig@amd.com>]

On 9/15/26 16:22, Matt Evans wrote:
> Hi Christian,
> 
> On 15/09/2026 12:13, Christian König wrote:
>> On 9/15/26 09:20, Leon Romanovsky wrote:
>>> On Mon, Sep 14, 2026 at 01:13:36PM +0100, Matt Evans wrote:
>>>> Hi Leon,
>>>>
>>>> On 14/09/2026 12:54, Leon Romanovsky wrote:
>>>>> On Mon, Sep 14, 2026 at 08:36:47AM -0300, Jason Gunthorpe wrote:
>>>>>> On Sun, Sep 13, 2026 at 07:52:44PM +0300, Leon Romanovsky wrote:
>>>>>>> On Fri, Sep 11, 2026 at 10:41:57PM +0100, Matt Evans wrote:
>>>>>>>> Expand the VFIO DMABUF revocation state to three states:
>>>>>>>> Not revoked, temporarily revoked, and permanently revoked.
>>>>>>>
>>>>>>> The thing is that "temporarily revoked" is actually the standard
>>>>>>> invalidate_mappings/move_notify mechanism of DMABUF, which wasn't good
>>>>>>> for VFIO.
>>>>>>
>>>>>> I think temporarily revokes here means it is revoked from a dmabuf
>>>>>> perspective
>>>>>
>>>>> My guess is that this is more of a "change owner" operation than a
>>>>> revoke operation.
>>>>>
>>>>> The main issue here is that we have to guess the semantics instead of
>>>>> having a properly named and documented operation.
>>>>
>>>> Apologies if the cover letter for the series and patch commit message (which
>>>> cover this) are unclear about the motivations and semantics.  On the commit
>>>> message, can you suggest clarifications:
>>>>
>>>> "This is useful for lifecycle management, to reclaim VFIO PCI BAR
>>>> ranges previously delegated to a subordinate client process: by
>>>> revoking, the driver process can ensure that the loaned resources are
>>>> made inaccessible when the client is deemed "done".  The original
>>>> DMABUF is defunct, and BAR resources can then be safely re-exported
>>>> for use by new clients."
>>>>
>>>> Given what I'll explain below, do give suggestions please.  There is more
>>>> context in the cover letter (the volume of which I didn't think appropriate
>>>> for the commit message).
>>>
>>> 1. Do not mix "driver" and "client" in the same description. For a
>>> non-native English speaker, "driver" has a very specific meaning in the
>>> context of the Linux kernel.
>>
>> In the context of DMA-buf it has also proven vital to clearly use the terms importer and exporter to describe the different roles a driver can have.
>>
>> I can't count how often there was confusion because people (me included) just used "driver" and it wasn't clear which role was meant.
> 
> :)  I see where the quote can be clearer.  FWIW the "driver process" was
> referring to a userspace driver (which is a legitimate use of the D-word
> with VFIO, but still).  I'll clarify that, and will indicate this
> mechanism is used by userspace to influence the VFIO _exporter_ behaviour.
> 
>>> 2. Explain the lifecycle in the commit message, and why "revoke", which
>>> is effectively what the importer does, is not sufficient.
>>>
>>> 3. The more you put in the cover letter, the less likely people are to
>>> read it.
>>>
>>> 4. Commit messages should describe the patches themselves, since they
>>> are what remains visible in the git log, unlike the cover letter.
>>>
>>> 5. I would call what you describe as "temporarily revoke" is actually "reclaim".
>>
>> +1
> 
> I don't follow here, sorry.  Would you please elaborate?
> 
> Currently VFIO uses the priv->revoked flag to track whether
> it-the-exporter had previously done dma_buf_invalidate_mappings() on a
> buffer and is now causing all .attach requests to fail.  Do you mean
> that (even without this series) you want to call that concept
> priv->reclaimed instead?

That sounds like a permanent revoke. 

> Or do you mean that you don't like the words "temporary"/"permanent" and
> are looking for another name for a temporarily unavailable buffer?  (If
> so, I find "not revoked", "reclaimed", "revoked" much less clear than
> not/temp/perm revoked, as such names give no hint as to what to expect.
> But I may have misunderstood what you're getting at.)

Yeah it's pretty much the naming I would clarify.

The original idea of notifying the importer that it need to re-create the mapping was resource reclaim.

But when you have some IOCTL or sysfs or whatever to disable a DMA-buf permanently I would call that revoke.

I'm still not 100% sure what a temporary revoke should be. If you have some IOCTL/sysfs/whatever to temporary say to importers "You can't use that resource" then that most likely won't fly.
 
> From the importer side, there is no change from this patch: they might
> observe an invalidate_mappings() and find attach() of a given buffer now
> fails, same as before this patch.  This is only about guaranteeing the
> impossibility of an importer ever being able to re-attach in future.

That sounds reasonable.

When a resource becomes unavailable you seriously need completely destroy it, re-create it and then import it again into other drivers which want to use it should it ever become available again.

Regards,
Christian.

> 
> 
> Thanks,
> 
> 
> Matt
> 
> 
> PS:  A hypothetical alternative way of doing what this patch is doing is
> instead to have:
> 
>  - existing priv->revoked
>  - new 	    priv->revoked_flag_is_immutable
> 
> Then, the ioctl triggers an invalidate_mappings() for a targeted DMABUF,
> sets revoked = true, and revoked_flag_is_immutable = true.  The new flag
> prevents a future vfio_pci_dma_buf_move(false) from clearing revoked.
> For example, a reset doing `move(true); reset; move(false);` can make
> all DMABUFs available to attach again, except for those marked
> immutably-revoked.  (That is equivalent to this new "permanent" state.)
> 
>