From: Matt Evans <mattev@meta.com>
To: Alex Williamson <alex@shazbot.org>
Cc: Bjorn Helgaas <bhelgaas@google.com>,
Logan Gunthorpe <logang@deltatee.com>,
Ankit Agrawal <ankita@nvidia.com>,
Leon Romanovsky <leon@kernel.org>,
Niklas Schnelle <schnelle@linux.ibm.com>,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] PCI/P2PDMA: Avoid returning a provider for non_mappable_bars
Date: Thu, 23 Apr 2026 17:46:57 +0100 [thread overview]
Message-ID: <34d884a3-ae25-49b2-9063-8783ff50e095@meta.com> (raw)
In-Reply-To: <20260422090146.6faf00f8@shazbot.org>
Hi Alex,
On 22/04/2026 16:01, Alex Williamson wrote:
>
> 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,
That makes sense, yes, it'll avoid the WARN that'd otherwise occur in
pci_p2pdma_add_resource().
I'll do a v2 adding this check to pcim_p2pdma_init() as well. Hopefully
Logan & Niklas's R-B will still apply.
Thanks,
Matt
prev parent reply other threads:[~2026-04-23 16:47 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
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-23 14:55 ` Niklas Schnelle
2026-04-26 11:22 ` Leon Romanovsky
2026-04-23 15:48 ` Matt Evans
2026-04-22 9:19 ` Niklas Schnelle
2026-04-22 15:01 ` Alex Williamson
2026-04-23 16:46 ` Matt Evans [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=34d884a3-ae25-49b2-9063-8783ff50e095@meta.com \
--to=mattev@meta.com \
--cc=alex@shazbot.org \
--cc=ankita@nvidia.com \
--cc=bhelgaas@google.com \
--cc=leon@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=logang@deltatee.com \
--cc=schnelle@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox