The Linux Kernel Mailing List
 help / color / mirror / Atom feed
From: Farhan Ali <alifm@linux.ibm.com>
To: Alex Williamson <alex@shazbot.org>
Cc: Matthew Rosato <mjrosato@linux.ibm.com>,
	linux-kernel@vger.kernel.org, linux-s390@vger.kernel.org,
	kvm@vger.kernel.org, borntraeger@linux.ibm.com, mattev@meta.com,
	schnelle@linux.ibm.com
Subject: Re: [PATCH v1] vfio/pci: Avoid mapping BARs for devices with non-mappable BARs
Date: Mon, 3 Aug 2026 13:44:58 -0700	[thread overview]
Message-ID: <a8f2af03-3242-40bf-bf89-e922f0768db3@linux.ibm.com> (raw)
In-Reply-To: <20260803135415.2cfd37bb@shazbot.org>


On 8/3/2026 12:54 PM, Alex Williamson wrote:
> On Mon, 3 Aug 2026 09:39:19 -0700
> Farhan Ali <alifm@linux.ibm.com> wrote:
>
>> On 7/29/2026 2:50 PM, Alex Williamson wrote:
>>> On Wed, 29 Jul 2026 14:32:45 -0700
>>> Farhan Ali <alifm@linux.ibm.com> wrote:
>>>   
>>>> On 7/29/2026 1:36 PM, Alex Williamson wrote:
>>>>> On Wed, 29 Jul 2026 13:28:46 -0700
>>>>> Farhan Ali <alifm@linux.ibm.com> wrote:
>>>>>      
>>>>>> On 7/29/2026 12:41 PM, Matthew Rosato wrote:
>>>>>>> On 7/29/26 2:11 PM, Farhan Ali wrote:
>>>>>>>> vfio_pci_core_map_bars() calls pci_iomap() to set up BAR resources, but not
>>>>>>>> all devices support having their BARs mapped by the CPU. The
>>>>>>>> non_mappable_bars flag indicates that a PCI device's BARs cannot be
>>>>>>>> accessed by the CPU. The ISM device on s390 is one such device. The BAR
>>>>>>>> size for an ISM device is 256 TiB, and attempting to map the BAR will lead
>>>>>>>> to warnings:
>>>>>>>>
>>>>>>>> vmalloc_node_range for size 281474976714752 failed: Address range
>>>>>>>> restricted to 0x2110bab00000 - 0x21903ab00000
>>>>>>>>
>>>>>>>> Use pdev->non_mappable_bars to skip pci_iomap() for such devices. This flag
>>>>>>>> is set by the PCI core at enumeration time and already serves the same
>>>>>>>> purpose in vfio_pci_probe_mmaps().
>>>>>>>>
>>>>>>>> Fixes: 05f2a68b407a ("vfio/pci: Set up BAR resources and maps in vfio_pci_core_enable()")
>>>>>>>> Reported-by: Christian Borntraeger <borntraeger@linux.ibm.com>
>>>>>>>> Signed-off-by: Farhan Ali <alifm@linux.ibm.com>
>>>>>>>> ---
>>>>>>>>      drivers/vfio/pci/vfio_pci_core.c | 3 +++
>>>>>>>>      1 file changed, 3 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>>>>>>>> index 3f11a9624b9c..6a184588ff23 100644
>>>>>>>> --- a/drivers/vfio/pci/vfio_pci_core.c
>>>>>>>> +++ b/drivers/vfio/pci/vfio_pci_core.c
>>>>>>>> @@ -554,6 +554,9 @@ static void vfio_pci_core_map_bars(struct vfio_pci_core_device *vdev)
>>>>>>>>      
>>>>>>>>      		vdev->barmap[bar] = IOMEM_ERR_PTR(-ENODEV);
>>>>>>>>      
>>>>>>>> +		if (pdev->non_mappable_bars)
>>>>>>>> +			continue;
>>>>>>>> +
>>>>>>> This would work for the ISM case at least, but I wonder: should we check
>>>>>>> vdev->bar_mmap_supported[bar] instead?
>>>>>>>
>>>>>>> My question boils down to: do we still want messages for some of the
>>>>>>> cases where we set vdev->bar_mmap_supported[bar] = false in
>>>>>>> vfio_pci_probe_mmaps()?
>>>>>> AFAIU vfio_pci_probe_mmaps() is only called in
>>>>>> vfio_pci_core_finish_enable(). Since vfio_pci_core_map_bars() is called
>>>>>> in vfio_pci_core_enable(), and before vfio_pci_core_finish_enable(),
>>>>>> bar_mmap_supported would be false here for all devices. So I don't think
>>>>>> it would work here, unless I missed something?
>>>>> Also IO Port and sub-page MMIO BARs are things that do exist.  Thanks,
>>>> Just to clarify, are you suggesting we expand this check to also avoid
>>>> mapping IO port and sub-page MMIO BARs?
>>> Sorry, no, I'm not.  I think we're conflating that the barmap is
>>> related to mmap access.  The barmap itself is holding the iomap of the
>>> BAR, used for read/write.  The only real relation to the mmap is that
>>> we request the resource via this path as well.
>>>
>>> Therefore not only is the ordering of setting up bar_mmap_supported
>>> wrong, it's flagging entirely the wrong thing here and keying on it
>>> would entirely break IO port and sub-page MMIO BAR access.  Thanks,
>>>
>>> Alex
>> Hi Alex,
>>
>> I wanted some guidance on how we should proceed with this patch? The
>> warning messages are a regression on s390 for ISM devices, so we would
>> like to fix it.
> I think the original proposal is probably the correct one.  The
> non_mmapable_bars flag doesn't restrict its application to specific BAR
> types or access, at least not beyond "can't be mapped to CPU or peers."
>
> If we can't map the BAR to the CPU, then we don't need to request the
> region or perform the pci_iomap(), which is what I understand explodes
> here.  Therefore we really only need to establish the errno in the
> barmap here.

Yes, its the call to pci_iomap() that leads to the warning.

>
> The bar_mmap_supported flag describes something else, whether the BAR
> can be mapped into the user address space.  My intention was only to
> point out that there are BARs that cannot be mapped to the user address
> space because either they're not MMIO or we can't safely map the full
> page, therefore bar_mmap_supported is an invalid test for whether we
> should request the region or iomap the BAR.
>
> Is there still a gap with the original proposal that I'm missing?

AFAICT this fix should be sufficient. I think Matt's question was to see 
if there were other cases for which bar_mmap_supported were set to 
false, that we need to consider here. But I will let him clarify to see 
if he has any concerns.

Thanks

Farhan


  parent reply	other threads:[~2026-08-03 20:45 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-29 18:11 [PATCH v1] vfio/pci: Avoid mapping BARs for devices with non-mappable BARs Farhan Ali
2026-07-29 19:41 ` Matthew Rosato
2026-07-29 20:28   ` Farhan Ali
2026-07-29 20:36     ` Alex Williamson
2026-07-29 21:32       ` Farhan Ali
2026-07-29 21:50         ` Alex Williamson
2026-08-03 16:39           ` Farhan Ali
     [not found]             ` <20260803135415.2cfd37bb@shazbot.org>
2026-08-03 20:44               ` Farhan Ali [this message]
2026-08-04 13:39                 ` Matthew Rosato
2026-08-04 14:09 ` Niklas Schnelle

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=a8f2af03-3242-40bf-bf89-e922f0768db3@linux.ibm.com \
    --to=alifm@linux.ibm.com \
    --cc=alex@shazbot.org \
    --cc=borntraeger@linux.ibm.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-s390@vger.kernel.org \
    --cc=mattev@meta.com \
    --cc=mjrosato@linux.ibm.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