Re: [RFC PATCH v2 1/2] dma-buf: Add support for private interconnects

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On Fri, 2025-09-26 at 13:00 -0300, Jason Gunthorpe wrote:
> On Fri, Sep 26, 2025 at 04:51:29PM +0200, Christian König wrote:
> > On 26.09.25 16:41, Jason Gunthorpe wrote:
> > > On Fri, Sep 26, 2025 at 03:51:21PM +0200, Thomas Hellström wrote:
> > > 
> > > > Well both exporter and exporter has specific information WRT
> > > > this. The
> > > > ultimate decision is done in the exporter attach() callback,
> > > > just like
> > > > pcie_p2p. And the exporter acknowledges that by setting the
> > > > dma_buf_attachment::interconnect_attach field. In analogy with
> > > > the
> > > > dma_buf_attachment::peer2peer member.
> > > 
> > > Having a single option seems too limited to me..
> > 
> > Yeah, agree.
> > 
> > > I think it would be nice if the importer could supply a list of
> > > 'interconnects' it can accept, eg:
> > > 
> > >  - VRAM offset within this specific VRAM memory
> > >  - dma_addr_t for this struct device
> > >  - "IOVA" for this initiator on a private interconnect
> > >  - PCI bar slice
> > >  - phys_addr_t (used between vfio, kvm, iommufd)
> > 
> > I would rather say that the exporter should provide the list of
> > what
> > interconnects the buffer might be accessible through.
> 
> Either direction works, I sketched it like this because I thought
> there were more importers than exporters, and in the flow it is easy
> for the importer to provide a list on the stack
> 
> I didn't sketch further, but I think the exporter and importer should
> both be providing a compatible list and then in almost all cases the
> core code should do the matching.
> 
> If the importer works as I showed, then the exporter version would be
> in an op:
> 
> int exporter_negotiate_op(struct dma_buf *dmabuf,
>    struct dma_buf_interconnect_negotiation *importer_support, size_t
> importer_len)
> {
>      struct dma_buf_interconnect_negotiation exporter_support[2] = {
>          [0] = {.interconnect = myself->xe_vram},
>          [1] = {.interconnect = &dmabuf_generic_dma_addr_t,
> .interconnect_args = exporter_dev},
>      };
>      return dma_buf_helper_negotiate(dmabuf, exporter_support,
>              ARRAY_SIZE(exporter_support), importer_support,
> importer_len);
> }
> 
> Which the dma_buf_negotiate() calls.
> 
> The core code does the matching generically, probably there is a
> struct dma_buf_interconnect match() op it uses to help this process.
> 
> I don't think importer or exporter should be open coding any
> matching.
> 
> For example, we have some systems with multipath PCI. This could
> actually support those properly. The RDMA NIC has two struct devices
> it operates with different paths, so it would write out two
> &dmabuf_generic_dma_addr_t's - one for each.
> 
> The GPU would do the same. The core code can have generic code to
> evaluate if P2P is possible and estimate some QOR between the
> options.

This sounds OK with me. I have some additional questions, though,

1) Everybody agrees that the interconnect used is a property of the
attachment? It should be negotiated during attach()?

2) dma-buf pcie-p2p allows transparent fallback to system memory dma-
buf. I think that is a good thing to keep even for other interconnects
(if possible). Like if someone wants to pull the network cable, we
could trigger a move_notify() and on next map() we'd fall back. Any
ideas around this?

Thanks,
Thomas



> 
> Jason