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

[Matt Evans <matt@ozlabs.org>]

Hi Logan, Jason, Alex,

On 05/08/2026 21:50, Logan Gunthorpe wrote:
> 
> 
> 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.

Yes, this has gone round in a bit of a circle, apologies.

I went back to the original change and concluded vfio-pci _doesn't_
always need a provider, so this work to refactor P2PDMA to provide a
provider even when CONFIG_PCI_P2PDMA=n is unnecessary.

The original original need was to break the dependency of vfio-pci on
P2PDMA, since the series made vfio-pci dependent on
CONFIG_VFIO_PCI_DMABUF.  I'd missed that the _export_ of a DMABUF
doesn't need a provider (after all, the phys_vec has the PFNs).

So we can just do this:

- VFIO exports DMABUF internally for mmap()
- It attempts to get a provider for the export, but if
CONFIG_PCI_P2PDMA=n then provider is NULL.
- The DMABUF works fine for CPU access through the VMA
- If someone were to fish out the DMABUF from the VMA/fds then it cannot
be imported unless the provider is valid (dma_buf_map_attachment()
returns -EINVAL)
- Drop the P2PDMA changes entirely

IOW, when CONFIG_PCI_P2PDMA=n you obviously don't get to import DMABUFs
into another driver for P2P, so the provider is unused ... and when
P2PDMA is supported, the VFIO BAR VMA's DMABUF has a provider, so could
be imported for P2P.  The struct vfio_pci_dma_buf only needs to have a
valid provider if the DMABUF could possibly be imported of course.

The behaviour of pcim_p2pdma_provider() is unchanged, as are any other
consumers such as mlx5.

This is way simpler.  I will drop these first two P2PDMA refactor
patches from this series.  Sorry for the churn and time spent on these;
though a cleanup/refactor could have merit separately it sounds like
there's discussion needed on exactly what that goal should be.


Matt