Re: [PATCH v19 12/18] vfio/pci: Add dmabuf TPH metadata storage and fd query helper

[fengchengwen <fengchengwen@huawei.com>]

Hi Alex,

On 7/10/2026 2:50 AM, Alex Williamson wrote:
> On Thu, 9 Jul 2026 19:49:24 +0800
> fengchengwen <fengchengwen@huawei.com> wrote:
> 
>> Hi Alex,
>>
>> On 7/9/2026 7:31 AM, Alex Williamson wrote:
>>> On Thu, 2 Jul 2026 20:42:18 +0800
>>> Chengwen Feng <fengchengwen@huawei.com> wrote:
>>>>
>>>> Signed-off-by: Zhiping Zhang <zhipingz@meta.com>
>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>>> ---
>>>>  drivers/vfio/pci/vfio_pci_dmabuf.c | 49 +++++++++++++++++++++++++++++-
>>>>  drivers/vfio/pci/vfio_pci_priv.h   |  6 ++++
>>>>  2 files changed, 54 insertions(+), 1 deletion(-)  
>>>
>>> Please don't send competing patches, just indicate the series is based
>>> on Zhiping's series v#.  This makes all the naming changes that Zhiping
>>> declined yet you still included their sign-off.  Thanks,  
>>
>> Thanks for the feedback on overlapping dmabuf TPH patches with Zhiping’s series.
>> There are code conflicts between the two series on PCI TPH and VFIO dmabuf logic,
>> and I also have concerns that Sashiko may not properly parse and track cross-series
>> Depends-on tags for reliable automated review.
>>
>> My current design idea is to retain this foundational metadata patch in my
>> series. This patch only implements basic storage and query helpers for TPH tags
>> on dma_buf objects, providing the underlying infrastructure. Zhiping can then
>> build the corresponding VFIO device feature ioctl layer on top of these commits
>> in his follow-up series. I will fully sync all his latest code revisions into
>> this patch to align implementation details and minimize future merge conflicts
> 
> If your concern is sashiko being able to apply and review your series
> on top of another, then I would suggest leading the series with the
> unmodified patches clearly noted in the cover letter and commit logs.
> 
> As provided here, this patch initially appears as excerpts of Zhiping
> series, again including sign-off without permission, but it introduces
> this massive gap that was previously overlooked as a result:
> 
>>>>  
>>>> +int vfio_pci_dma_buf_get_tph_by_fd(int fd, bool extended, u16 *tag, u8 *ph)
>>>> +{
>>>> +	struct dma_buf *dmabuf;
>>>> +	int ret = 0;
>>>> +
>>>> +	dmabuf = dma_buf_get(fd);
>>>> +	if (IS_ERR(dmabuf))
>>>> +		return PTR_ERR(dmabuf);
>>>> +
>>>> +	if (dmabuf->ops != &vfio_pci_dmabuf_ops) {
>>>> +		ret = -EINVAL;
>>>> +		goto out;
>>>> +	}
>>>> +
>>>> +	ret = vfio_pci_dma_buf_get_pci_tph(dmabuf, extended, tag, ph);
>>>> +out:
>>>> +	dma_buf_put(dmabuf);
>>>> +	return ret;
>>>> +}
> 
> .get_pci_tph() is proposed as a generic feature of dmabufs.  We cannot
> require that only vfio-pci provided dmabufs are supported, nor can we
> bypass the default interfaces and lifecycles.  This needs to use
> dma_buf_get_pci_tph(), rather than punch through to the vfio-pci
> implementation of .get_pci_tph(), and it needs to honor the calling
> conventions of that interface.  That's what makes this series dependent
> on Zhiping's implementation of TPH as a vfio-pci completer feature.
> 
> Already, within this patch, the caller violates the lockdep assertion
> that the dma reservation lock is held.

V20 has sent with above bugfix and based on Zhiping's v11 patchset

> 
> Additionally, dma_buf_get_pci_tph() defines that the provided values
> are only valid until invalidate_mappings, but this series has no
> plumbing to honor that requirement.
> 
> This needs significant work if we need to track the lifecycle of every
> dmabuf related TPH value.  Thanks,

Is it possible to use this as a risk warning? After all, even if the
invalid mapping is performed, the ST only causes performance loss to
the system, and there will be no function problems.

Thanks

> 
> Alex