* Re: [PATCH] PCI/P2PDMA: Avoid returning a provider for non_mappable_bars
2026-04-21 17:43 [PATCH] PCI/P2PDMA: Avoid returning a provider for non_mappable_bars Matt Evans
@ 2026-04-21 18:14 ` Logan Gunthorpe
2026-04-21 19:49 ` Leon Romanovsky
` (2 subsequent siblings)
3 siblings, 0 replies; 7+ messages in thread
From: Logan Gunthorpe @ 2026-04-21 18:14 UTC (permalink / raw)
To: Matt Evans, Bjorn Helgaas, Ankit Agrawal, Leon Romanovsky,
Alex Williamson, Niklas Schnelle
Cc: linux-pci, linux-kernel
On 2026-04-21 11:43, Matt Evans wrote:
> Extend pcim_p2pdma_provider()'s checks to exclude functions that have
> pdev->non_mappable_bars set.
>
> Consumers such as VFIO were previously able to map these for access by
> the CPU or P2P. Update the comment on non_mappable_bars to show it
> refers to any access, not just userspace CPU access.
>
> Fixes: 372d6d1b8ae3c ("PCI/P2PDMA: Refactor to separate core P2P functionality from memory allocation")
> Signed-off-by: Matt Evans <mattev@meta.com>
This makes sense to me, thanks.
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] PCI/P2PDMA: Avoid returning a provider for non_mappable_bars
2026-04-21 17:43 [PATCH] PCI/P2PDMA: Avoid returning a provider for non_mappable_bars Matt Evans
2026-04-21 18:14 ` Logan Gunthorpe
@ 2026-04-21 19:49 ` Leon Romanovsky
2026-04-22 14:22 ` Matt Evans
2026-04-22 9:19 ` Niklas Schnelle
2026-04-22 15:01 ` Alex Williamson
3 siblings, 1 reply; 7+ messages in thread
From: Leon Romanovsky @ 2026-04-21 19:49 UTC (permalink / raw)
To: Matt Evans
Cc: Bjorn Helgaas, Logan Gunthorpe, Ankit Agrawal, Alex Williamson,
Niklas Schnelle, linux-pci, linux-kernel
On Tue, Apr 21, 2026 at 10:43:51AM -0700, Matt Evans wrote:
> Extend pcim_p2pdma_provider()'s checks to exclude functions that have
> pdev->non_mappable_bars set.
>
> Consumers such as VFIO were previously able to map these for access by
> the CPU or P2P. Update the comment on non_mappable_bars to show it
> refers to any access, not just userspace CPU access.
>
> Fixes: 372d6d1b8ae3c ("PCI/P2PDMA: Refactor to separate core P2P functionality from memory allocation")
I don't object to the patch, but this Fixes line doesn't look correct.
non_mappable_bars applies only to s390, which doesn't support p2p. That
wasn't prevented before 372d6d1b8ae3c refactoring too.
Thanks
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] PCI/P2PDMA: Avoid returning a provider for non_mappable_bars
2026-04-21 19:49 ` Leon Romanovsky
@ 2026-04-22 14:22 ` Matt Evans
2026-04-22 15:19 ` Leon Romanovsky
0 siblings, 1 reply; 7+ messages in thread
From: Matt Evans @ 2026-04-22 14:22 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Bjorn Helgaas, Logan Gunthorpe, Ankit Agrawal, Alex Williamson,
Niklas Schnelle, linux-pci, linux-kernel
Hi Leon,
On 21/04/2026 20:49, Leon Romanovsky wrote:
>
> On Tue, Apr 21, 2026 at 10:43:51AM -0700, Matt Evans wrote:
>> Extend pcim_p2pdma_provider()'s checks to exclude functions that have
>> pdev->non_mappable_bars set.
>>
>> Consumers such as VFIO were previously able to map these for access by
>> the CPU or P2P. Update the comment on non_mappable_bars to show it
>> refers to any access, not just userspace CPU access.
>>
>> Fixes: 372d6d1b8ae3c ("PCI/P2PDMA: Refactor to separate core P2P functionality from memory allocation")
>
> I don't object to the patch, but this Fixes line doesn't look correct.
> non_mappable_bars applies only to s390, which doesn't support p2p. That
> wasn't prevented before 372d6d1b8ae3c refactoring too.
Thanks; I'd chosen that commit as it adds the function that the
additional test is being added to.
As an example consumer, vfio_pci_core_get_dmabuf_phys() added in
5d74781ebc86 ("vfio/pci: Add dma-buf export support for MMIO regions")
could do an export when it shouldn't. (And with my other mmap series, a
CPU could then access it, not even P2P.) I suppose this patch could
point to 5d74781ebc86 instead (which allows the naughty DMABUF export),
but I couldn't prove VFIO DMABUF export was the only thing using
pcim_p2pdma_provider() added in 372d6d1b8ae3c (especially as that
symbol's exported).
It still seems most watertight to treat this as a fix for anything
containing 372d6d1b8ae3c, but if you feel it's not right what would be a
better one?
Also, in the other thread (linked above) Alex made the point that whilst
_today_ non_mappable_bars is only set by one s390 device, he expects
future quirks or reasons to exclude mappings would also use
non_mappable_bars so we shouldn't consider it a s390-specific thing.
Cheers,
Matt
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] PCI/P2PDMA: Avoid returning a provider for non_mappable_bars
2026-04-22 14:22 ` Matt Evans
@ 2026-04-22 15:19 ` Leon Romanovsky
0 siblings, 0 replies; 7+ messages in thread
From: Leon Romanovsky @ 2026-04-22 15:19 UTC (permalink / raw)
To: Matt Evans
Cc: Bjorn Helgaas, Logan Gunthorpe, Ankit Agrawal, Alex Williamson,
Niklas Schnelle, linux-pci, linux-kernel
On Wed, Apr 22, 2026 at 03:22:53PM +0100, Matt Evans wrote:
> Hi Leon,
>
> On 21/04/2026 20:49, Leon Romanovsky wrote:
> >
> > On Tue, Apr 21, 2026 at 10:43:51AM -0700, Matt Evans wrote:
> > > Extend pcim_p2pdma_provider()'s checks to exclude functions that have
> > > pdev->non_mappable_bars set.
> > >
> > > Consumers such as VFIO were previously able to map these for access by
> > > the CPU or P2P. Update the comment on non_mappable_bars to show it
> > > refers to any access, not just userspace CPU access.
> > >
> > > Fixes: 372d6d1b8ae3c ("PCI/P2PDMA: Refactor to separate core P2P functionality from memory allocation")
> >
> > I don't object to the patch, but this Fixes line doesn't look correct.
> > non_mappable_bars applies only to s390, which doesn't support p2p. That
> > wasn't prevented before 372d6d1b8ae3c refactoring too.
>
> Thanks; I'd chosen that commit as it adds the function that the additional
> test is being added to.
Can I suggest slightly different solution?
If CONFIG_..._S390 is set, we will unset CONFIG_PCI_P2PDMA and pcim_p2pdma_provider()
will be compiled to return NULL, without changing non_mappable_bars
description.
Thanks
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] PCI/P2PDMA: Avoid returning a provider for non_mappable_bars
2026-04-21 17:43 [PATCH] PCI/P2PDMA: Avoid returning a provider for non_mappable_bars Matt Evans
2026-04-21 18:14 ` Logan Gunthorpe
2026-04-21 19:49 ` Leon Romanovsky
@ 2026-04-22 9:19 ` Niklas Schnelle
2026-04-22 15:01 ` Alex Williamson
3 siblings, 0 replies; 7+ messages in thread
From: Niklas Schnelle @ 2026-04-22 9:19 UTC (permalink / raw)
To: Matt Evans, Bjorn Helgaas, Logan Gunthorpe, Ankit Agrawal,
Leon Romanovsky, Alex Williamson
Cc: linux-pci, linux-kernel
On Tue, 2026-04-21 at 10:43 -0700, Matt Evans wrote:
> Extend pcim_p2pdma_provider()'s checks to exclude functions that have
> pdev->non_mappable_bars set.
>
> Consumers such as VFIO were previously able to map these for access by
> the CPU or P2P. Update the comment on non_mappable_bars to show it
> refers to any access, not just userspace CPU access.
>
> Fixes: 372d6d1b8ae3c ("PCI/P2PDMA: Refactor to separate core P2P functionality from memory allocation")
> Signed-off-by: Matt Evans <mattev@meta.com>
> ---
>
> This arises from Alex Williamson's suggestion to test
> non_mappable_bars when getting the provider, with discussion here:
>
> https://lore.kernel.org/kvm/20260415181623.1021090-1-mattev@meta.com/
>
> The goal was to prevent a hole where VFIO could export DMABUFs for
> BARs marked non-mappable, and to fix for all users of the provider
> rather than just VFIO. Alex observed that non_mappable_bars should be
> taken to mean BARs weren't usable by the CPU _or_ peers and,
> considering that, its comment about userspace access wasn't quite
> right.
>
>
> drivers/pci/p2pdma.c | 3 ++-
> include/linux/pci.h | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 7c898542af8d..4a783413f466 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -318,7 +318,8 @@ struct p2pdma_provider *pcim_p2pdma_provider(struct pci_dev *pdev, int bar)
> {
> struct pci_p2pdma *p2p;
>
> - if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
> + if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM) ||
> + pdev->non_mappable_bars)
> return NULL;
>
> p2p = rcu_dereference_protected(pdev->p2pdma, 1);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 2c4454583c11..1e6802017d6b 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -508,7 +508,7 @@ struct pci_dev {
> unsigned int no_command_memory:1; /* No PCI_COMMAND_MEMORY */
> unsigned int rom_bar_overlap:1; /* ROM BAR disable broken */
> unsigned int rom_attr_enabled:1; /* Display of ROM attribute enabled? */
> - unsigned int non_mappable_bars:1; /* BARs can't be mapped to user-space */
> + unsigned int non_mappable_bars:1; /* BARs can't be mapped by CPU or peers */
> pci_dev_flags_t dev_flags;
> atomic_t enable_cnt; /* pci_enable_device has been called */
>
Sorry for the late reply. For s390's ISM device this definitely makes
sense. The BAR can indeed not be accessed by P2P and extending this
seems reasonable to me.
Feel free to add my:
Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Thanks,
Niklas
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] PCI/P2PDMA: Avoid returning a provider for non_mappable_bars
2026-04-21 17:43 [PATCH] PCI/P2PDMA: Avoid returning a provider for non_mappable_bars Matt Evans
` (2 preceding siblings ...)
2026-04-22 9:19 ` Niklas Schnelle
@ 2026-04-22 15:01 ` Alex Williamson
3 siblings, 0 replies; 7+ messages in thread
From: Alex Williamson @ 2026-04-22 15:01 UTC (permalink / raw)
To: Matt Evans
Cc: Bjorn Helgaas, Logan Gunthorpe, Ankit Agrawal, Leon Romanovsky,
Niklas Schnelle, linux-pci, linux-kernel, alex
On Tue, 21 Apr 2026 10:43:51 -0700
Matt Evans <mattev@meta.com> wrote:
> Extend pcim_p2pdma_provider()'s checks to exclude functions that have
> pdev->non_mappable_bars set.
>
> Consumers such as VFIO were previously able to map these for access by
> the CPU or P2P. Update the comment on non_mappable_bars to show it
> refers to any access, not just userspace CPU access.
>
> Fixes: 372d6d1b8ae3c ("PCI/P2PDMA: Refactor to separate core P2P functionality from memory allocation")
> Signed-off-by: Matt Evans <mattev@meta.com>
> ---
>
> This arises from Alex Williamson's suggestion to test
> non_mappable_bars when getting the provider, with discussion here:
>
> https://lore.kernel.org/kvm/20260415181623.1021090-1-mattev@meta.com/
>
> The goal was to prevent a hole where VFIO could export DMABUFs for
> BARs marked non-mappable, and to fix for all users of the provider
> rather than just VFIO. Alex observed that non_mappable_bars should be
> taken to mean BARs weren't usable by the CPU _or_ peers and,
> considering that, its comment about userspace access wasn't quite
> right.
>
>
> drivers/pci/p2pdma.c | 3 ++-
> include/linux/pci.h | 2 +-
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 7c898542af8d..4a783413f466 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -318,7 +318,8 @@ struct p2pdma_provider *pcim_p2pdma_provider(struct pci_dev *pdev, int bar)
> {
> struct pci_p2pdma *p2p;
>
> - if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
> + if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM) ||
> + pdev->non_mappable_bars)
> return NULL;
>
> p2p = rcu_dereference_protected(pdev->p2pdma, 1);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 2c4454583c11..1e6802017d6b 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -508,7 +508,7 @@ struct pci_dev {
> unsigned int no_command_memory:1; /* No PCI_COMMAND_MEMORY */
> unsigned int rom_bar_overlap:1; /* ROM BAR disable broken */
> unsigned int rom_attr_enabled:1; /* Display of ROM attribute enabled? */
> - unsigned int non_mappable_bars:1; /* BARs can't be mapped to user-space */
> + unsigned int non_mappable_bars:1; /* BARs can't be mapped by CPU or peers */
> pci_dev_flags_t dev_flags;
> atomic_t enable_cnt; /* pci_enable_device has been called */
>
Should pcim_p2pdma_init() separately test pdev->non_mappable_bars
before the rcu-deref/kzalloc of the pci_p2pdma object and return
-EOPNOTSUPP?
That then invokes the same error paths we'd see if we simply don't have
p2pdma in the kernel and handles the pci_p2pdma_add_resource() path
automatically as well.
This pcim_p2pdma_provider() test is really then just suppressing the
WARN_ON that we'd otherwise see by not finding the p2p object on the
device. Thanks,
Alex
^ permalink raw reply [flat|nested] 7+ messages in thread