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

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Sep 25, 2025 at 05:48:36PM +0200, Thomas Hellström wrote:
> On Thu, 2025-09-25 at 15:08 +0200, Thomas Hellström wrote:
> > Add a function to the dma_buf_attach_ops to indicate whether the
> > connection is a private interconnect. If so the function returns
> > the address to an interconnect-defined structure that can be
> > used for further negotiating.
> > 
> > Also add a field to the dma_buf_attachment that indicates whether
> > a private interconnect is used by the attachment.
> > 
> > Signed-off-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> > ---
> >  include/linux/dma-buf.h | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> > 
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index d58e329ac0e7..e7191edb2125 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -475,6 +475,19 @@ struct dma_buf_attach_ops {
> >  	 * point to the new location of the DMA-buf.
> >  	 */
> >  	void (*move_notify)(struct dma_buf_attachment *attach);
> > +
> > +	/**
> > +	 * @supports_interconnect: [optional] - Does the driver
> > support a local interconnect?
> > +	 *
> > +	 * Does the importer support a private interconnect? The
> > interconnect is
> > +	 * identified using a unique address defined by the exporter
> > and declared
> > +	 * in a protocol header.
> 
> Actually we'd probably want to use something like a
> 
> struct dma_buf_interconnect {
> 	const char *name;

Maybe a field for private protocol type too?

> };
> 
> Here, and for globally known interconnects have them
> instantiated somewhere common since there could be multiple exporters.
> 
> >  (RFC: Should this be a struct instead).
> > +	 *
> > +	 * Return: A pointer to the interconnect-private attach_ops
> > structure if supported,
> > +	 * %NULL otherwise.
> > +	 */
> > +	const void *(*supports_interconnect)(struct
> > dma_buf_attachment *attach,
> > +					     const void
> > *interconnect);
> 
> And similarly for stricter type-checking the return value could be a
> struct dma_buf_interconnect_attach_ops {
> };
> 
> which is subclassed for each interconnect-private attach ops.
> 
> >  };
> >  
> >  /**
> > @@ -484,6 +497,7 @@ struct dma_buf_attach_ops {
> >   * @node: list of dma_buf_attachment, protected by dma_resv lock of
> > the dmabuf.
> >   * @peer2peer: true if the importer can handle peer resources
> > without pages.
> >   * @priv: exporter specific attachment data.
> > + * @interconnect: Private interconnect to use if any, NULL
> > otherwise.
> >   * @importer_ops: importer operations for this attachment, if
> > provided
> >   * dma_buf_map/unmap_attachment() must be called with the dma_resv
> > lock held.
> >   * @importer_priv: importer specific attachment data.
> > @@ -503,6 +517,7 @@ struct dma_buf_attachment {
> >  	struct list_head node;
> >  	bool peer2peer;
> >  	const struct dma_buf_attach_ops *importer_ops;
> > +	const void *interconnect;
> 
> const struct dma_buf_interconnect *interconnect;
> 

In general I think not using void * would be a good idea rather base
classes that can be subclassed. Since you have thought about this
already and there aren't comments on this patch maybe best to respin
with the subclasssing concepts?

Matt

> >  	void *importer_priv;
> >  	void *priv;
> >  };
> 
> /Thomas
>