Re: [RFC v2 0/8] dma-buf: Add support for mapping dmabufs via interconnects

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Oct 30, 2025 at 06:17:11AM +0000, Kasireddy, Vivek wrote:
> It mostly looks OK to me but there are a few things that I want to discuss,
> after briefly looking at the patches in your branch:
> - I am wondering what is the benefit of the SGT compatibility stuff especially
> when Christian suggested that he'd like to see SGT usage gone from
> dma-buf

I think to get rid of SGT we do need to put it in a little well
defined box and then create alternatives and remove things using
SGT. This is a long journey, and I think this is the first step.

If SGT is some special case it will be harder to excise.

So the next steps would be to make all the exporters directly declare
a SGT and then remove the SGT related ops from dma_ops itself and
remove the compat sgt in the attach logic. This is not hard, it is all
simple mechanical work.

This way the only compat requirement is to automatically give an
import match list for a SGT only importer which is very little code in
the core.

The point is we make the SGT stuff nonspecial and fully aligned with
the mapping type in small steps. This way neither importer nor
exporter should have any special code to deal with interworking.

To remove SGT we'd want to teach the core code how to create some kind
of conversion mapping type, eg exporter uses SGT importer uses NEW so
the magic conversion mapping type does the adapatation.

In this way we can convert importers and exporters to use NEW in any
order and they still interwork with each other.

> eventually. Also, if matching fails, IMO, indicating that to the
> importer (allow_ic) and having both exporter/importer fallback to
> the current legacy mechanism would be simpler than the SGT
> compatibility stuff.

I don't want to have three paths in importers.

If the importer supports SGT it should declare it in a match and the
core code should always return a SGT match for the importer to use

The importer should not have to code 'oh it is sgt but it somehow a
little different' via an allow_ic type idea.
 
> - Also, I thought PCIe P2P (along with SGT) use-cases are already well handled
> by the existing map_dma_buf() and other interfaces. So, it might be confusing
> if the newer interfaces also provide a mechanism to handle P2P although a
> bit differently. I might be missing something here but shouldn't the existing
> allow_peer2peer and other related stuff be left alone?

P2P is part of SGT, it gets pulled into the SGT stuff as steps toward
isolating SGT properly. Again as we move things to use native SGT
exporters we would remove the exporter related allow_peer2peer items
when they become unused.
 
> - You are also adding custom attach/detach ops for each mapping_type. I think
> it makes sense to reuse existing attach/detach ops if possible and initiate the
> matching process from there, at-least initially.

I started there, but as soon as I went to adding PAL I realized the
attach/detach logic was completely different for each of the mapping
types. So this is looking alot simpler.

If the driver wants to share the same attach/detach ops for some of
its mapping types then it can just set the same function pointer to
all of them and pick up the mapping type from the attach->map_type.

> - Looks like your design doesn't call for a dma_buf_map_interconnect() or other
> similar helpers provided by dma-buf core that the importers can use. Is that
> because the return type would not be known to the core?

I don't want to have a single shared 'map' operation, that is the
whole point of this design. Each mapping type has its own ops, own
types, own function signatures that the client calls directly.

No more type confusion or trying to abuse phys_addr_t, dma_addr_t, or
scatterlist for in appropriate things. If your driver wants something
special, like IOV, then give it proper clear types so it is
understandable.

> - And, just to confirm, with your design if I want to add a new interconnect/
> mapping_type (not just IOV but in general), all that is needed is to provide custom
> attach/detach, match ops and one or more ops to map/unmap the address list
> right? Does this mean that the role of dma-buf core would be limited to just
> match and the exporters are expected to do most of the heavy lifting and
> checking for stuff like dynamic importers, resv lock held, etc?

I expect the core code would continue to provide wrappers and helpers
to call the ops that can do any required common stuff.

However, keep in mind, when the importer moves to use mapping type it
also must be upgraded to use the dynamic importer flow as this API
doesn't support non-dynamic importers using mapping type.

I will add some of these remarks to the commit messages..

Thanks!
Jason