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

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.

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.

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)
 {
 #ifdef CONFIG_P2PDMA
	return true;
 #else
	/* CONFIG_P2PDMA_CORE only */
	return false;
 #endif
 }

I can't yet see places [1] would be used (IIUC that's indicating "core
is present"? and feels ~similar to _provider returning something) but I
don't know all the consumers well enough yet.  Agree the two groups of
functionality could be named more distinctly, though.


Cheers,


Matt

(PS: Wanted to v.tactfully ask, as I'm eager to avoid as much
renaming/refactoring in this pair of P2PDMA patches as possible since
folks now seem happy with them, what could be a fix in this series vs a
revisit.)


> Did some of APIs get mixed into the genalloc family that should not
> have?
> 
> Jason
>