Hi Logan,
On 27/05/2026 17:07, Logan Gunthorpe wrote:
On 2026-05-27 04:23, Matt Evans wrote:
The P2PDMA code currently provides two features under the same CONFIG_PCI_P2PDMA option:
- Locate providers via pcim_p2pdma_provider()
- Manage actual P2P DMA
Other code (such as vfio-pci) depends on 1, without having a hard dependency on 2.
A future commit expands the use of DMABUF in vfio-pci for non-P2P scenarios, relying on pcim_p2pdma_provider() always being present. If that depended on CONFIG_PCI_P2PDMA, it would make vfio-pci only available if CONFIG_ZONE_DEVICE is present (e.g. 64-bit systems), even when P2P is not needed.
To resolve this, introduce CONFIG_PCI_P2PDMA_CORE which contains the basic provider functionality to make it available even if the CONFIG_PCI_P2PDMA feature is disabled or unavailable due to !CONFIG_ZONE_DEVICE. Users such as vfio-pci can enable their own P2P features based off the original CONFIG_PCI_P2PDMA (available when CONFIG_ZONE_DEVICE is set).
Signed-off-by: Matt Evans mattev@meta.com
Largely this looks good to me. I have one minor nit below that you can apply or not. Either way, feel free to add:
Reviewed-by: Logan Gunthorpe logang@deltatee.com
static void pci_p2pdma_release(void *data) { struct pci_dev *pdev = data; @@ -241,11 +251,13 @@ static void pci_p2pdma_release(void *data) synchronize_rcu(); xa_destroy(&p2pdma->map_types); +#ifdef CONFIG_PCI_P2PDMA if (!p2pdma->pool) return; gen_pool_destroy(p2pdma->pool); sysfs_remove_group(&pdev->dev.kobj, &p2pmem_group); +#endif }
I'm personally not a fan of adding #ifdefs inside functions like this. This instance is small and easy to understand, but it can quickly become a bit of a mess if we start adding more features. I probably would have created a pci_p2pdma_release_pool() helper which does the inverse of pci_p2pdma_setup_pool(), it would be called in pci_p2pdma_release() and an empty implementation would be provided in the case where CONFIG_PCI_P2PDMA is not set.
That's cleaner, I'll do that. Thanks for the review.
Matt