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

[Alex Williamson <alex@shazbot.org>]

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.

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,

Alex