On 10/22/26 08:52, Alistair Popple wrote:
> On 2026-10-21 at 23:37 +1100, Jason Gunthorpe <[email protected]> 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?
No, that would result in ping/pong behavior which is something we explicitly
try to avoid.
This is also not something uncommon, it happens multiple times a second on a
standard X11 Linux desktop when a new buffer is started to be scanned out on an
A+A or A+I laptop.
Regards,
Christian.
>
> Attachment could also scan existing attachments to preferentially pick
> matching
> mapping_types that are compatible with existing attachments, thus avoiding
> invalidation where possible.
>
> 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