Re: [Linaro-mm-sig] [PATCH v8 06/11] dma-buf: provide phys_vec to scatter-gather mapping routine

["Christian König" <christian.koenig@amd.com>]

On 11/20/25 09:06, Leon Romanovsky wrote:
> On Thu, Nov 20, 2025 at 08:54:37AM +0100, Christian König wrote:
>> On 11/20/25 08:41, Leon Romanovsky wrote:
>>> On Thu, Nov 20, 2025 at 08:08:27AM +0100, Christian König wrote:
>>>> On 11/19/25 20:31, Jason Gunthorpe wrote:
>>>>> On Wed, Nov 19, 2025 at 02:42:18PM +0100, Christian König wrote:
>>>>>
>>>>>>>>> +	case PCI_P2PDMA_MAP_THRU_HOST_BRIDGE:
>>>>>>>>> +		dma->state = kzalloc(sizeof(*dma->state), GFP_KERNEL);
>>>>>>>>> +		if (!dma->state) {
>>>>>>>>> +			ret = -ENOMEM;
>>>>>>>>> +			goto err_free_dma;
>>>>>>>>> +		}
>>>>>>>>> +
>>>>>>>>> +		dma_iova_try_alloc(attach->dev, dma->state, 0, size);
>>>>>>>>
>>>>>>>> Oh, that is a clear no-go for the core DMA-buf code.
>>>>>>>>
>>>>>>>> It's intentionally up to the exporter how to create the DMA
>>>>>>>> addresses the importer can work with.
>>>>>>>
>>>>>>> I can't fully understand this remark?
>>>>>>
>>>>>> The exporter should be able to decide if it actually wants to use
>>>>>> P2P when the transfer has to go through the host bridge (e.g. when
>>>>>> IOMMU/bridge routing bits are enabled).
>>>>>
>>>>> Sure, but this is a simplified helper for exporters that don't have
>>>>> choices where the memory comes from.
>>>>
>>>> That is extremely questionable as justification to put that in common DMA-buf code.
>>>>
>>>>> I fully expet to see changes to this to support more use cases,
>>>>> including the one above. We should do those changes along with users
>>>>> making use of them so we can evaluate what works best.
>>>>
>>>> Yeah, exactly that's my concern.
>>>>
>>>>>> But only take that as Acked-by, I would need at least a day (or
>>>>>> week) of free time to wrap my head around all the technical details
>>>>>> again. And that is something I won't have before January or even
>>>>>> later.
>>>>>
>>>>> Sure, it is alot, and I think DRM community in general should come up
>>>>> to speed on the new DMA API and how we are pushing to see P2P work
>>>>> within Linux.
>>>>>
>>>>> So thanks, we can take the Acked-by and progress here. Interested
>>>>> parties can pick it up from this point when time allows.
>>>>
>>>> Wait a second. After sleeping a night over it I think my initial take that we really should not put that into common DMA-buf code seems to hold true.
>>>>
>>>> This is the use case for VFIO, but I absolutely want to avoid other drivers from re-using this code until be have more experience with that.
>>>>
>>>> So to move forward I now strongly think we should keep that in VFIO until somebody else comes along and needs that helper.
>>>
>>> It was put in VFIO at the beginning, but Christoph objected to it,
>>> because that will require exporting symbol for pci_p2pdma_map_type().
>>> which was universally agreed as not good idea.
>>
>> Yeah, that is exactly what I object here :)
>>
>> We can have the helper in DMA-buf *if* pci_p2pdma_map_type() is called by drivers or at least accessible. That's what I pointed out in the other mail before as well.
>>
>> The exporter must be able to make decisions based on if the transaction would go over the host bridge or not.
>>
>> Background is that in a lot of use cases you rather want to move the backing store into system memory instead of keeping it in local memory if the driver doesn't have direct access over a common upstream bridge.
>>
>> Currently drivers decide that based on if IOMMU is enabled or not (and a few other quirks), but essentially you absolutely want a function which gives this information to exporters. For the VFIO use case it doesn't matter because you can't switch the BAR for system memory.
>>
>> To unblock you, please add a big fat comment in the kerneldoc of the mapping explaining this and that it might be necessary for exporters to call pci_p2pdma_map_type() as well.
> 
> Thanks,
> 
> What do you think about it?
> 
> diff --git a/drivers/dma-buf/dma-buf-mapping.c b/drivers/dma-buf/dma-buf-mapping.c
> index a69bb73db86d..05ec84a0157b 100644
> --- a/drivers/dma-buf/dma-buf-mapping.c
> +++ b/drivers/dma-buf/dma-buf-mapping.c
> @@ -84,6 +84,11 @@ struct dma_buf_dma {
>   * PAGE_SIZE aligned.
>   *
>   * A mapping must be unmapped by using dma_buf_free_sgt().
> + *
> + * NOTE: While this function is intended for DMA-buf importers, it is critical
> + * that the DMA-buf exporter is capable of performing peer-to-peer (P2P) DMA
> + * directly between PCI devices, without routing transactions through the host
> + * bridge.

Well first of all this function is intended for exporters not importers.

Maybe write something like "This function is intended for exporters. If direct traffic routing is mandatory exporter should call routing pci_p2pdma_map_type() before calling this function.".

Regards,
Christian.

>   */
>  struct sg_table *dma_buf_phys_vec_to_sgt(struct dma_buf_attachment *attach,
>                                          struct p2pdma_provider *provider,
> (END)
> 
> 
>>
>> Regards,
>> Christian.
>>
>>>
>>> https://lore.kernel.org/all/aPYrEroyWVOvAu-5@infradead.org/
>>>
>>> Thanks
>>>
>>>>
>>>> Regards,
>>>> Christian.
>>>>
>>>>>
>>>>> We can also have a mini-community call to give a summary/etc on these
>>>>> topics.
>>>>>
>>>>> Thanks,
>>>>> Jason
>>>>
>>