Hi Jason, > Subject: Re: [RFC v2 0/8] dma-buf: Add support for mapping dmabufs via > interconnects > > 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. IMO, this SGT compatibility stuff should ideally be a separate follow-on effort (and patch series) that would also probably include updates to various drivers to add the SGT mapping type.
> > 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.. Sounds good. I'll start testing/working on IOV interconnect patches based on your design. Thanks, Vivek > > Thanks! > Jason
