Re: [PATCH v5 2/9] PCI/P2PDMA: Add CONFIG_PCI_P2PDMA_CORE

[Logan Gunthorpe]

On 2026-08-05 10:40, Jason Gunthorpe wrote:
> On Wed, Aug 05, 2026 at 05:28:27PM +0100, Matt Evans wrote:
>> Hi both (Logan thanks for your comments!),
>>
>> On 05/08/2026 01:39, Jason Gunthorpe wrote:
>>> On Tue, Aug 04, 2026 at 10:19:11AM -0600, Logan Gunthorpe wrote:
>>>
>>>> There's a vague convention for this already: the term 'p2pmem' is often
>>>> used for cases where the driver uses the allocator, etc. (I think I had
>>>> this intention when I wrote the code and have since forgotten about
>>>> it).
>>>
>>> I've been calling it the genalloc layer and the core layer. p2pmem
>>> would be OK to refer to the genalloc stuff. So if you want to have
>>> CONFIG_PCI_P2PDMA and CONFIG_PCI_P2PMEM that seem sOk
>>>
>>>> code into it's own file, potentially renaming some functions. Then, in
>>>> the end, we would probably have a pcim_p2pdma_supported() function and a
>>>> pcim_p2pmem_supported() function, the latter being used by existing use
>>>> cases.
>>>
>>> Not quite sure why we need this?
>>
>> ( [1] )
>>
>>> Matt, the mlx5 stuff is the same as VFIO, it just uses the "core"
>>> layer and does not use the genalloc. So there shouldn't be an issue
>>> here, if the genalloc is off then the mlx5 stuff should still
>>> work. There shouldn't be a case where CONFIG_PCI_P2PDMA=y and mlx5 is
>>> broken?
>>
>> Oh, when CONFIG_PCI_P2PDMA=y it's all good.
>>
>> The issue is when CONFIG_PCI_P2PDMA=n, as mlx5 still seems to permit a
>> DMABUF export solely because pcim_p2pdma_provider() succeeds.  (This
>> patch's CONFIG_PCI_P2PDMA_CORE enables that.)  mlx5 assumes that getting
>> a provider means P2P DMA is also available.
> 
> That's my point, the mlx5 should work fine with CONFIG_PCI_P2PDMA_CORE
> only or it is split wrong.
> 
>> Later, DMABUF attach would fail, but it'd be good to keep the original
>> failure mode where UVERBS_METHOD_DMABUF_ALLOC fails early if no P2PDMA.
> 
> Why does it fail? It should not fail :)
> 
>> I was thinking something trivial like the following would let things
>> like IB fail the DMABUF_ALLOC early still, instead of making the
>> assupmtion that having a provider means having P2P DMA.  E.g. OK
>> provider's available, but test for P2P DMA support too:
>>
>>  bool pcim_p2pdma_supported(void)
>>  {
> 
> it seems illogical, if you have a provider you have p2p dma, things
> are split wrong if this is not true.
> 
> The split should be only around the genalloc and related
> (sysfs,etc,etc) not anything mlx5 uses.
> 
> The only think that should stop working without genalloc (ie
> CONFIG_PCI_P2PDMA=n) is nvme.

Hmm, seems I made a few mistakes in my reply. Though I'm less certain I
understand the issue anymore.

I did confuse in my response pcim_p2pdma_provider() and
pcim_p2pdma_supported(). The later choice I really dislike. My thinking
was we'd have two versions of pcim_p2p[dma|mem]_provider()... But I'm
not sure that's necessary.

I thought there were callers of pcim_p2pdma_provider() in the p2pmem
code (non-core) section. But digging deeper today, it seems like
pci_p2pdma_add_resource() is the one caller, which I'd expect would be
compiled out when CONFIG_PCI_P2PDMA=n. Which seems fine. So I'm not sure
I understand the root issue anymore.

Logan