On 26.09.25 16:41, Jason Gunthorpe wrote:
On Fri, Sep 26, 2025 at 03:51:21PM +0200, Thomas Hellström wrote:
Well both exporter and exporter has specific information WRT this. The ultimate decision is done in the exporter attach() callback, just like pcie_p2p. And the exporter acknowledges that by setting the dma_buf_attachment::interconnect_attach field. In analogy with the dma_buf_attachment::peer2peer member.
Having a single option seems too limited to me..
Yeah, agree.
I think it would be nice if the importer could supply a list of 'interconnects' it can accept, eg:
- VRAM offset within this specific VRAM memory
- dma_addr_t for this struct device
- "IOVA" for this initiator on a private interconnect
- PCI bar slice
- phys_addr_t (used between vfio, kvm, iommufd)
I would rather say that the exporter should provide the list of what interconnects the buffer might be accessible through.
Having the p2p flag set by the importer was basically just the easiest approach to implement the flow, that is not necessarily a design pattern how to approach a general solution.
Regards, Christian.
The exporter has a function to run down the list and return the first compatible. Maybe something like
struct dma_buf_interconnect_negotiation { struct dma_buf_interconnect *interconnect, void *interconnect_args, }; struct dma_buf_interconnect_negotiation importer_offer[2] = { // On stack [0] = {.interconnect = myself->xe_vram}, [1] = {.interconnect = &dmabuf_generic_dma_addr_t, .interconnects_args = dev}, }; idx = dma_buf_negotiate(dmabuf, importer_offer, ARRAY_SIZE(importer_offer)); if (idx < 0) return -EOPNOTSUPP;Then you'd 'interconnect attach' with that compatible item and get back an attach. Using container_of to get the specific ops which then has a function to get the address list.
attach = dma_buf_attach_interconnect(dmabuf, importer_offer[idx], &dma_buf_attach_ops);
if (idx == 0) { xe_vram_ops = container_of(attach->ops, ..); struct device_private_address *addrs = xe_vram_ops->map(attach); [..] xe_vram_ops->unmap(attach); } dma_buf_detach_interconnect(attach);
I can imagine some scheme where if the exporter does not support interconnect then the core code will automatically look for dmabuf_generic_dma_addr_t, select it, and supply some ops that call existing dma_buf_dynamic_attach()/dma_buf_map_attachment() transparently.
So the above function mimics the dma_buf_attach_ops::allow_peer2peer bool, except it's not a single interconnect so we'd either use a set of bools, one for each potential interconnect, or a function like this. A function has the benefit that it can also provide any additional attach ops the interconnect might need.
allow_peer2peer seems to indicate if sg_page() can be used on the sgt? It doesn't have any meaning for an importer only using dma_addr_t?
In the above language it would be an interconnect exchanging 'struct page *'.. I'm a little confused by this I thought touching the struct page was forbidden?
Is this to not overload the map_attachment() and unmap_attachment() functions that otherwise could be used? Is it because they return an sg_table?
It would be good to avoid going through APIs that use sg_table in the design..
Jason