On Tue, 5 May 2026 10:38:29 -0700
Matt Evans <[email protected]> wrote:

> Previously BAR resource requests and the corresponding pci_iomap()
> were performed on-demand and without synchronisation, which was racy.
> Rather than add synchronisation, it's simplest to address this by
> doing both activities from vfio_pci_core_enable().
> 
> The resource allocation and/or pci_iomap() can still fail; their
> status is tracked and existing calls to vfio_pci_core_setup_barmap()
> will fail in a similar way to before.  This keeps the point of failure
> as observed by userspace the same, i.e. failures to request/map unused
> BARs are benign.
> 
> Fixes: 89e1f7d4c66d ("vfio: Add PCI device driver")
> Signed-off-by: Matt Evans <[email protected]>
> ---
>  drivers/vfio/pci/vfio_pci_core.c | 36 +++++++++++++++++++++++++++++++-
>  drivers/vfio/pci/vfio_pci_rdwr.c | 26 +++++++----------------
>  2 files changed, 42 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_core.c 
> b/drivers/vfio/pci/vfio_pci_core.c
> index 3f8d093aacf8..62931dc381d8 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -482,6 +482,39 @@ static int vfio_pci_core_runtime_resume(struct device 
> *dev)
>  }
>  #endif /* CONFIG_PM */
>  
> +/*
> + * Eager-request BAR resources, and iomap them.  Soft failures are
> + * allowed, and consumers must check the barmap before use in order to
> + * give compatible user-visible behaviour with the previous on-demand
> + * allocation method.
> + */
> +static void vfio_pci_core_map_bars(struct vfio_pci_core_device *vdev)
> +{
> +     struct pci_dev *pdev = vdev->pdev;
> +     int i;
> +
> +     for (i = 0; i < PCI_STD_NUM_BARS; i++) {
> +             int bar = i + PCI_STD_RESOURCES;
> +
> +             vdev->barmap[bar] = ERR_PTR(-ENODEV);
> +
> +             if (!pci_resource_len(pdev, i))
> +                     continue;
> +
> +             if (pci_request_selected_regions(pdev, 1 << bar, "vfio")) {
> +                     pci_dbg(vdev->pdev, "Failed to reserve region %d\n", 
> bar);
> +                     vdev->barmap[bar] = ERR_PTR(-EBUSY);
> +                     continue;
> +             }
> +
> +             vdev->barmap[bar] = pci_iomap(pdev, bar, 0);
> +             if (!vdev->barmap[bar]) {

Sashiko notes[1] correctly that we need to release the requested region
here.

[1]https://sashiko.dev/#/patchset/[email protected]

> +                     pci_dbg(vdev->pdev, "Failed to iomap region %d\n", bar);
> +                     vdev->barmap[bar] = ERR_PTR(-ENOMEM);
> +             }
> +     }
> +}
> +
>  /*
>   * The pci-driver core runtime PM routines always save the device state
>   * before going into suspended state. If the device is going into low power
> @@ -568,6 +601,7 @@ int vfio_pci_core_enable(struct vfio_pci_core_device 
> *vdev)
>       if (!vfio_vga_disabled() && vfio_pci_is_vga(pdev))
>               vdev->has_vga = true;
>  
> +     vfio_pci_core_map_bars(vdev);
>  
>       return 0;
>  
> @@ -648,7 +682,7 @@ void vfio_pci_core_disable(struct vfio_pci_core_device 
> *vdev)
>  
>       for (i = 0; i < PCI_STD_NUM_BARS; i++) {
>               bar = i + PCI_STD_RESOURCES;
> -             if (!vdev->barmap[bar])
> +             if (IS_ERR_OR_NULL(vdev->barmap[bar]))
>                       continue;
>               pci_iounmap(pdev, vdev->barmap[bar]);
>               pci_release_selected_regions(pdev, 1 << bar);
> diff --git a/drivers/vfio/pci/vfio_pci_rdwr.c 
> b/drivers/vfio/pci/vfio_pci_rdwr.c
> index 4251ee03e146..3bfbb879a005 100644
> --- a/drivers/vfio/pci/vfio_pci_rdwr.c
> +++ b/drivers/vfio/pci/vfio_pci_rdwr.c
> @@ -198,27 +198,15 @@ ssize_t vfio_pci_core_do_io_rw(struct 
> vfio_pci_core_device *vdev, bool test_mem,
>  }
>  EXPORT_SYMBOL_GPL(vfio_pci_core_do_io_rw);
>  
> +/*
> + * The barmap is set up in vfio_pci_core_enable().  Callers use this
> + * function to check that the BAR resources are requested or that the
> + * pci_iomap() was done.
> + */
>  int vfio_pci_core_setup_barmap(struct vfio_pci_core_device *vdev, int bar)
>  {
> -     struct pci_dev *pdev = vdev->pdev;
> -     int ret;
> -     void __iomem *io;
> -
> -     if (vdev->barmap[bar])
> -             return 0;
> -
> -     ret = pci_request_selected_regions(pdev, 1 << bar, "vfio");
> -     if (ret)
> -             return ret;
> -
> -     io = pci_iomap(pdev, bar, 0);
> -     if (!io) {
> -             pci_release_selected_regions(pdev, 1 << bar);
> -             return -ENOMEM;
> -     }
> -
> -     vdev->barmap[bar] = io;
> -
> +     if (IS_ERR(vdev->barmap[bar]))
> +             return PTR_ERR(vdev->barmap[bar]);
>       return 0;
>  }
>  EXPORT_SYMBOL_GPL(vfio_pci_core_setup_barmap);


Reply via email to