Re: [PATCH] vfio/pci: Don't export DMABUFs for unmappable BARs

[Matt Evans <mattev@meta.com>]

Hi Alex,

On 17/04/2026 23:31, Alex Williamson wrote:
> On Fri, 17 Apr 2026 15:25:07 +0100
> Matt Evans <mattev@meta.com> wrote:
>> On 16/04/2026 22:48, Alex Williamson wrote:
>>> On Thu, 16 Apr 2026 19:03:40 +0100
>>> Matt Evans <mattev@meta.com> wrote:
>>>> On 16/04/2026 14:14, Leon Romanovsky wrote:
>>>>> On Thu, Apr 16, 2026 at 02:05:30PM +0100, Matt Evans wrote:
>>>>
>>>> I don't understand your question, really sorry!  Can you rephrase it
>>>> please?  I want to make sure I answer it fully.
>>>>
>>>> Although mmap() fails for BARs that are unmappable (for whatever
>>>> reason), a DMABUF export for the same ones could in some slim cases
>>>> succeed -- because the checks aren't identical.  If export succeeds, it
>>>> could potentially allow P2P (or CPU via a future DMABUF mmap()) access
>>>> to something possibly unmappable, no?
>>>>
>>>> For the checks that vfio_pci_probe_mmaps() does (leading to
>>>> bar_mmap_supported[] = false), most have corresponding-but-different
>>>> checks reachable from DMABUF export:
>>>>
>>>> If a BAR is:		Then DMABUF export...:
>>>>
>>>>     size < pagesize	vfio_pci_core_fill_phys_vec() catches it
>>>>     Not IORESOURCE_MEM	pcim_p2pdma_provider() rejects it
>>>>     non_mappable_bars	... nothing?  Export allowed
>>>>
>>>> As a quick test, if I hack in non_mappable_bars=1 for my function, it
>>>> appears exporting a DMABUF from it works.
>>>>
>>>> We could add another check for non_mappable_bars, but my thinking was
>>>> that we don't want to keep adding to an independent set of DMABUF
>>>> checks, especially if a future quirk/etc. could create another scenario
>>>> where BARs aren't mappable.  I.e. we should reject DMABUF export in
>>>> exactly the same scenarios as mmap() would be rejected, symmetrically,
>>>> by testing bar_mmap_supported[].
>>>>
>>>> Hope that goes some way to answering the Q, hopefully I haven't missed
>>>> something!
>>>
>>> That's the concern as I see it as well, it's a choice whether to
>>> attempt to support sub-PAGE_SIZE mappings, but if a device is reporting
>>> non_mappable_bars are we're exporting those BARs through dma-buf for
>>> mmap, that's a problem.  Should pcim_p2pdma_provider() test this flag
>> rather than vfio_pci_dmabuf though?  Thanks,
>>
>> Do you mean "test this flag" to say 'non_mappable_bars' rather than the
>> 'vdev->bar_mmap_supported[]' flag?  (I think the former, as the latter
>> isn't as easily available down there, sorry if that's not what you
>> intended.)
> 
> Yes
> 
>> Testing non_mappable_bars in pcim_p2pdma_provider() doesn't feel quite
>> right:
>>
>> - IIUC non_mappable_bars is about preventing mapping to userspace, not
>> direct access/P2P/etc.  Splitting hairs a bit, but P2P to such a device
>> seems valid, so I think this check better stays within vfio-pci
>> (specifically limiting only userspace access).
> 
> Indeed the flag does specify userspace mapping in the comment, but I
> think it's more than that.  The only device that currently uses it is a
> device unique to s390x, where AIUI, there is no P2P DMA anyway.
> 
> Also, for "legacy" mapping of device BARs through the vfio type1
> backend, the mechanics of the mapping require the BAR can be mmap'd
> such that the user virtual address can then be used for the DMA
> mapping.  So as far as vfio has traditionally been concerned, there's
> an inherent dependency.
> 
> I could definitely see that p2p folks could balk and we need a separate
> flag... or since there's exactly one device that reports this flag,
> maybe we can tweak the description.

Thanks for explaining; with that context, ...

>> - But non_mappable_bars is just one kind of quirk, and couldn't there
>> possibly be more?  Good to avoid duplicating quirk checks in mmap() &
>> export places (so any future maintenance updates just one place and no
>> disparities arise).
> 
> I'm not sure what this is getting at, I think non_mappable_bars is
> meant to be the flag that quirks might set if for any reason we can't
> directly map the BAR.  I think "userspace" mapping is a bit of a
> red-herring, it's at least not mappable by the CPU, but I don't think
> the flag is actually meant to define a policy only for userspace.  If
> it's not mappable by the CPU... is that also enough of a restriction to
> exclude it from P2P mappings?

...and this, it makes sense then to just check non_mappable_bars, if 
it's not about userspace and has the semantic of "don't access this 
directly".

It'd be nice if it evolved to a per-BAR flag rather than per-function, 
BUT I acknowledge I was largely worrying about the shape of hypothetical 
future quirks and that isn't necessary just now.

>> All this to say:   The existng logic in vfio_pci_probe_mmaps() is the
>> right place to decide something is suitable for userspace/direct
>> mapping (mappable, non-zero sized, not IORESOURCE_IO), IMHO we just
>> need to be checking it consistently.  (VFIO export is less lenient in
>> terms of >= pagesize and imposes additional checks, and that's good
>> as long as the checks are an overlay rather than parallel
>> duplication.)
> 
> I'd actually take the opposite stance and say that vfio is not the only
> consumer of pcim_p2pdma_provider() and if a device has a flag that it
> shouldn't be mapped, the p2p subsystem should also honor that, not just
> vfio.
> 
>> (Maybe you're also flagging that there could be other checks saying "Is
>> P2P supported for this BAR?" and they could be done in a generic
>> drivers/pci place?  I think so;  VFIO export criteria be the
>> intersection of VFIO- and provider-based checks.)

(Typo, inadvertent talk like a pirate.)

> Yes (I think).  If we determine that non_mappable_bars means both CPU
> and device mappings (which is compatible with its current use case),
> then fixing pcim_p2pdma_provider() to honor the flag fixes all
> consumers.

OK, makes sense.  Seems OK to couple CPU mappings and P2P; I can imagine 
quirks (particularly for RCiEPs) where one works but the other doesn't 
but, until we've a concrete example, affecting both is safest.

For now, I'll do as you first suggested ( :) ), and if future 
more-elaborate quirks arise we can revisit.

>> I was thinking something like...
>>
>> diff --git a/drivers/vfio/pci/vfio_pci_dmabuf.c
>> b/drivers/vfio/pci/vfio_pci_dmabuf.c
>> index 00cedfe3a57d..9bb8bd153e12 100644
>> --- a/drivers/vfio/pci/vfio_pci_dmabuf.c
>> +++ b/drivers/vfio/pci/vfio_pci_dmabuf.c
>> @@ -359,6 +359,9 @@ int vfio_pci_core_get_dmabuf_phys(struct
>> vfio_pci_core_device *vdev,
>>           if (!*provider)
>>                   return -EINVAL;
>>
>> +       if (!vdev->bar_mmap_supported[region_index])
>> +               return -EINVAL;
>> +
>>           return vfio_pci_core_fill_phys_vec(
>>                   phys_vec, dma_ranges, nr_ranges,
>>                   pci_resource_start(pdev, region_index),
>>
>> (I.e. leverage logic in vfio_pci_probe_mmaps(), catch bad DMABUF
>> mappings this way, allows sub-drivers to override .get_dmabuf_phys.)
>>
>> I'll repost that as v2 if this doesn't seem an outrageous start. :)
> 
> That's fixing the leaf driver rather than the subsystem, where
> pci/p2pdma really ought to honor its own flag indicating the BAR is not
> mappable.  The precedent is already there in rejecting IO BARs.  Thanks,

All good; I appreciate the discussion, thanks, and will redo with 
checking non_mappable_bars in p2pdma.


Matt