public inbox for linux-pci@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] PCI/P2PDMA: Avoid returning a provider for non_mappable_bars
@ 2026-04-21 17:43 Matt Evans
  2026-04-21 18:14 ` Logan Gunthorpe
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Matt Evans @ 2026-04-21 17:43 UTC (permalink / raw)
  To: Bjorn Helgaas, Logan Gunthorpe, Ankit Agrawal, Leon Romanovsky,
	Alex Williamson, Niklas Schnelle
  Cc: linux-pci, linux-kernel

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 */
 
-- 
2.47.3


^ permalink raw reply related	[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
                   ` (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 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 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-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

* 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

end of thread, other threads:[~2026-04-22 15:19 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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 15:19     ` Leon Romanovsky
2026-04-22  9:19 ` Niklas Schnelle
2026-04-22 15:01 ` Alex Williamson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox