Re: [PATCH RFC 01/26] dma-buf: Introduce DMA-buf mapping types

[Alistair Popple <apopple@nvidia.com>]

On 2026-10-22 at 18:25 +1100, Thomas Hellström <thomas.hellstrom@linux.intel.com> wrote...
> On Thu, 2026-10-22 at 17:52 +1100, Alistair Popple wrote:
> > On 2026-10-21 at 23:37 +1100, Jason Gunthorpe <jgg@nvidia.com>
> > wrote...
> > > On Wed, Oct 21, 2026 at 08:24:40AM +0200, Thomas Hellström wrote:
> > > > > struct dma_buf_mapping_exp_ops
> > > > >    Base type for the per-mapping type exporter provided
> > > > > functions.
> > > > > This
> > > > >    would be the map/unmap callbacks. Each mapping type can
> > > > > provide
> > > > > its own
> > > > >    functions for map/unmap type operations with optimal type
> > > > > signatures.
> > > > 
> > > > Does it make sense to try to land this patch first, plus a
> > > > conversion
> > > > of pcie_p2p so we could agree on what the contract should look
> > > > like?
> > > 
> > > I think it would be good to focus on this patch as the main 'crux'
> > > of
> > > the series. The API contract is the main agreement. I'm relucant to
> > > merge code that has no users, so it woudl be ebtter to go up to at
> > > least through SGT.
> > >  
> > > > s/mapping/interconnect/? My gut feeling is that "interconnect"
> > > > best
> > > > describes what's typically going on, but "mapping" perhaps better
> > > > describes all cases.
> > > 
> > > My intention was to cover more than just the phyiscal transport. We
> > > do
> > > really want to get away from scatterlist, so having PCIe map with
> > > scatterlist and PCIe map with <something else> still ends up with
> > > two
> > > mapping types in the system.
> > > 
> > > This is why I choose the name over interconnect.
> > > 
> > > > Location of common protocols which typically carry cross-
> > > > subsystem
> > > > dependencies?
> > > > drivers/dma-buf/mapping_types? drivers/dma-buf/interconnects?
> > > 
> > > I'd encourage people to design more like PCI where the actual
> > > *interconnect* has its own home and there is just some minor call
> > > outs
> > > from dmabuf to help the matching logic.
> > > 
> > > > As discussed previously: 
> > > > Agree on theoretically possible matches at attach time, Match at
> > > > mapping time and guarantee the match is valid until
> > > > invalidate_mappings().
> > > 
> > > I was thinking about this some more, and I'm less sure now.
> > > 
> > > If I think about mlx5, in a multi-path PCIe configuration, it can
> > > move
> > > the translation around within a single PCIe path, but it cannot
> > > move
> > > it to another path without creating disruption for the
> > > application. This is a HW limitation in how the internal routing
> > > works..
> > > 
> > > I strongly suspect this will be the norm for any hardware that
> > > doesn't
> > > rely on decorated page tables to select the IO path..
> > > 
> > > So, while I agree we should support hardware that can fully change
> > > the
> > > path on each and every map, we also need to suport hardware that
> > > must
> > > pin the path during attach.
> > > 
> > > Probably this points to needing a 'pin' operation for the mapping
> > > selection after attach.
> > 
> > If I'm understanding things correctly we're expecting a change in
> > path between
> > attach and map time to be rare in practice. Therefore the need to
> > handle this
> > case is mostly as a fallback to not break things. So instead of
> > pinning could
> > new attachments just invalidate all existing incompatible attachments
> > on the
> > list by setting eg. attach->valid = 0?
> 
> I agree, (although I think the "invalidate_attachment" needs to be
> discussed). In the end the importer should be given a choice, either
> pin  and accept that subsequent attachers might see errors or accept
> app disruption.
> 
> > 
> > Attachment could also scan existing attachments to preferentially
> > pick matching
> > mapping_types that are compatible with existing attachments, thus
> > avoiding
> > invalidation where possible.
> 
> Current design appears to be that attach only determines the set of
> theoretically possible mapping types, disregarding other attachments. 
> Taking other attachments into account is also problematic because the
> dma-resv lock is not held.

Oh I see. Do you foresee any problems just taking the dma-resv lock to walk the
current attachments? If so we could probably resolve it with RCU, but I'd rather
keep it simple if we can.

> Looking at current GPU drivers, what you describe above is typically
> done at pin() time, but if some importers want an early very likely
> decision on what mapping_type to use, one could introduce a separate
> function that match the mapping_types, but that would be very similar
> to the map() function but without returning a sgt/mapping, or a pin()
> without actually pinning. One could reuse the invalidate_mappings()
> callback to indicate the mapping type is no longer valid?

I did wonder about that, but a callback for attachment invalidation seems like
it might be a pain for importers to syncrhonize with. What I'm thinking, which I
think is similar to what you describe, is something like the following:

1. Importer gets the likley attachment by passing a list of mapping types in
prefered order to dma_buf_mapping_attach() which returns the dma_buf_attachment
that would work based on current attachments and what the exporter provides:

attach = dma_buf_mapping_attach(...)

2. Takes dmabuf->resv lock.

3. Pins the attachment (if required):

rc = dma_buf_pin(attach)

DMA-BUF core revalidates the attachment is still valid (ie. by checking the
match again or via some other mechanism to detect the attachment path/location
has changed such as a sequence/generation number).

On error the importer can either fail or try to create a new attachment.

4. Map

rc = dma_buf_[sgt|pcie_p2p]_map(attach, ...)

Follows the same revalidation process as dma_buf_pin().

Thoughts?

 - Alistair

> /Thomas
> 
> 
> > 
> > Map time functions (eg. dma_buf_sgt_map_attachment) would check
> > attach->valid
> > and return a failure if a subsequent attachment/event had invalidated
> > the
> > mapping type.
> > 
> > Importers with invalidated attachments would then have to retry
> > creating the
> > attachment, probably getting a different mapping_type. Would this be
> > acceptable?
> > Or do some importers absolutely require the mapping_type to never
> > change once
> > the intial match/attachment has happened?
> > 
> >  - Alistair
> > 
> > > > > +/*
> > > > > + * Match items are generated by the importer using the
> > > > > DMA_BUF_IMAPPING_*() and
> > > > > + * the exporter using the DMA_BUF_EMAPPING_*() functions. Each
> > > > > mapping type
> > > > > + * defines its own signature with its own data to make the
> > > > > match and
> > > > > attachment.
> > > > > + */
> > > > > +struct dma_buf_mapping_match {
> > > > > +	const struct dma_buf_mapping_type *type;
> > > > > +	const struct dma_buf_mapping_exp_ops *exp_ops;
> > > > > +	union {
> > > > > +		/* Each mapping_type has unique match
> > > > > parameters
> > > > > here */
> > > > > +	};
> > > > 
> > > > Do we really need to add all global mapping types here? Then we'd
> > > > need
> > > > something for driver-private mapping types as well.
> > > 
> > > IIRC it is fixed size up front like this because of how the
> > > structure's memory is managed in this design..
> > > 
> > > If you want a driver private thing then I'd add a 'u64
> > > driver_private[2]' to the union and the driver can cast over it.
> > > 
> > > But this is the kernel, if you want to add an xe_mapping_type, then
> > > just add it. I don't know we need to do driver private things.
> > > 
> > > Jason