On 10/31/25 06:15, Kasireddy, Vivek wrote: > 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.
Nope, just the other way around. In other words the SGT compatibility is a pre-requisite. We should first demonstrate with existing drivers that the new interface works and does what it promised to do and then extend it with new functionality. Regards, Christian. > >> >> 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
