On Thu, 13 Nov 2025, Ilpo Järvinen wrote:

> BAR resize operation is implemented in the pci_resize_resource() and
> pbus_reassign_bridge_resources() functions. pci_resize_resource() can
> be called either from __resource_resize_store() from sysfs or directly
> by the driver for the Endpoint Device.
> 
> The pci_resize_resource() requires that caller has released the device
> resources that share the bridge window with the BAR to be resized as
> otherwise the bridge window is pinned in place and cannot be changed.
> 
> pbus_reassign_bridge_resources() implement rollback of the resources if
> the resize operation fails, but rollback is performed only for the
> bridge windows. Because releasing the device resources are done by the
> caller of the BAR resize interface, these functions performing the BAR
> resize do not have access to the device resources as they were before
> the resize.
> 
> pbus_reassign_bridge_resources() could try to
> __pci_bridge_assign_resources() after rolling back the bridge windows
> as they were, however, it will not guarantee the resource are assigned
> due to differences how FW and the kernel may want to assign the
> resources (alignment of the start address and tail).
> 
> In order to perform rollback robustly, the BAR resize interface has to
> be altered to also release the device resources that share the bridge
> window with the BAR to be resized.
> 
> Also, remove restoring from the entries failed list as saved list
> should now contain both the bridge windows and device resources so
> the extra restore is duplicated work.
> 
> Some drivers (currently only amdgpu) want to prevent releasing some
> resources. Add exclude_bars param to pci_resize_resource() and make
> amdgpu to pass its register BAR (BAR 5) which should never be released
> during resize operation. Normally 64-bit prefetchable resources do not
> share bridge window with it as the register BAR (32-bit only) but there
> are various fallbacks in the resource assignment logic which may make
> the resources to share the bridge window in rare cases.
> 
> This change (together with the driver side changes) is to counter the
> resource releases that had to be done to prevent resource tree
> corruption in the ("PCI: Release assigned resource before restoring
> them") change. As such, it likely restores functionality in cases where
> device resources were released to avoid resource tree conflicts which
> appeared to be "working" when such conflicts were not correctly
> detected by the kernel.
> 
> Link: 
> https://lore.kernel.org/linux-pci/[email protected]/
> Link: https://lore.kernel.org/linux-pci/[email protected]/
> Reported-by: Simon Richter <[email protected]>
> Reported-by: Alex Bennée <[email protected]>
> Signed-off-by: Ilpo Järvinen <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |   2 +-
>  drivers/gpu/drm/i915/gt/intel_region_lmem.c |   2 +-
>  drivers/gpu/drm/xe/xe_vram.c                |   2 +-
>  drivers/pci/pci-sysfs.c                     |  17 +---
>  drivers/pci/pci.h                           |   4 +-
>  drivers/pci/setup-bus.c                     | 100 +++++++++++++++-----
>  drivers/pci/setup-res.c                     |  23 ++---
>  include/linux/pci.h                         |   3 +-
>  8 files changed, 91 insertions(+), 62 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 7a899fb4de29..4e241836ae68 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1736,7 +1736,7 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device 
> *adev)
>  
>       pci_release_resource(adev->pdev, 0);
>  
> -     r = pci_resize_resource(adev->pdev, 0, rbar_size);
> +     r = pci_resize_resource(adev->pdev, 0, rbar_size, 1 << 5);
>       if (r == -ENOSPC)
>               dev_info(adev->dev,
>                        "Not enough PCI address space for a large BAR.");
> diff --git a/drivers/gpu/drm/i915/gt/intel_region_lmem.c 
> b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> index 51bb27e10a4f..7699e8fcf5ed 100644
> --- a/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> +++ b/drivers/gpu/drm/i915/gt/intel_region_lmem.c
> @@ -37,7 +37,7 @@ _resize_bar(struct drm_i915_private *i915, int resno, 
> resource_size_t size)
>  
>       _release_bars(pdev);
>  
> -     ret = pci_resize_resource(pdev, resno, bar_size);
> +     ret = pci_resize_resource(pdev, resno, bar_size, 0);
>       if (ret) {
>               drm_info(&i915->drm, "Failed to resize BAR%d to %dM (%pe)\n",
>                        resno, 1 << bar_size, ERR_PTR(ret));
> diff --git a/drivers/gpu/drm/xe/xe_vram.c b/drivers/gpu/drm/xe/xe_vram.c
> index b44ebf50fedb..00dd027057df 100644
> --- a/drivers/gpu/drm/xe/xe_vram.c
> +++ b/drivers/gpu/drm/xe/xe_vram.c
> @@ -36,7 +36,7 @@ _resize_bar(struct xe_device *xe, int resno, 
> resource_size_t size)
>       if (pci_resource_len(pdev, resno))
>               pci_release_resource(pdev, resno);
>  
> -     ret = pci_resize_resource(pdev, resno, bar_size);
> +     ret = pci_resize_resource(pdev, resno, bar_size, 0);
>       if (ret) {
>               drm_info(&xe->drm, "Failed to resize BAR%d to %dM (%pe). 
> Consider enabling 'Resizable BAR' support in your BIOS\n",
>                        resno, 1 << bar_size, ERR_PTR(ret));
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 9d6f74bd95f8..2a1b5456c2dc 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -1599,18 +1599,13 @@ static ssize_t __resource_resize_store(struct device 
> *dev, int n,
>  {
>       struct pci_dev *pdev = to_pci_dev(dev);
>       struct pci_bus *bus = pdev->bus;
> -     struct resource *b_win, *res;
>       unsigned long size;
> -     int ret, i;
> +     int ret;
>       u16 cmd;
>  
>       if (kstrtoul(buf, 0, &size) < 0)
>               return -EINVAL;
>  
> -     b_win = pbus_select_window(bus, pci_resource_n(pdev, n));
> -     if (!b_win)
> -             return -EINVAL;
> -
>       device_lock(dev);
>       if (dev->driver || pci_num_vf(pdev)) {
>               ret = -EBUSY;
> @@ -1632,15 +1627,7 @@ static ssize_t __resource_resize_store(struct device 
> *dev, int n,
>  
>       pci_remove_resource_files(pdev);
>  
> -     pci_dev_for_each_resource(pdev, res, i) {
> -             if (i >= PCI_BRIDGE_RESOURCES)
> -                     break;
> -
> -             if (b_win == pbus_select_window(bus, res))
> -                     pci_release_resource(pdev, i);
> -     }
> -
> -     ret = pci_resize_resource(pdev, n, size);
> +     ret = pci_resize_resource(pdev, n, size, 0);
>  
>       pci_assign_unassigned_bus_resources(bus);
>  
> diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h
> index bf1a577e9623..9893ea12d1f2 100644
> --- a/drivers/pci/pci.h
> +++ b/drivers/pci/pci.h
> @@ -421,8 +421,10 @@ enum pci_bar_type {
>  struct device *pci_get_host_bridge_device(struct pci_dev *dev);
>  void pci_put_host_bridge_device(struct device *dev);
>  
> +void pci_resize_resource_set_size(struct pci_dev *dev, int resno, int size);
> +int pci_do_resource_release_and_resize(struct pci_dev *dev, int resno, int 
> size,
> +                                    int exclude_bars);
>  unsigned int pci_rescan_bus_bridge_resize(struct pci_dev *bridge);
> -int pbus_reassign_bridge_resources(struct pci_bus *bus, struct resource 
> *res);
>  int __must_check pci_reassign_resource(struct pci_dev *dev, int i, 
> resource_size_t add_size, resource_size_t align);
>  
>  int pci_configure_extended_tags(struct pci_dev *dev, void *ign);
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 51f5e5a80b54..7e268960954b 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -2420,18 +2420,16 @@ 
> EXPORT_SYMBOL_GPL(pci_assign_unassigned_bridge_resources);
>   * release it when possible. If the bridge window contains assigned
>   * resources, it cannot be released.
>   */
> -int pbus_reassign_bridge_resources(struct pci_bus *bus, struct resource *res)
> +static int pbus_reassign_bridge_resources(struct pci_bus *bus, struct 
> resource *res,
> +                                       struct list_head *saved)
>  {
>       unsigned long type = res->flags;
>       struct pci_dev_resource *dev_res;
>       struct pci_dev *bridge = NULL;
> -     LIST_HEAD(saved);
>       LIST_HEAD(added);
>       LIST_HEAD(failed);
>       unsigned int i;
> -     int ret;
> -
> -     down_read(&pci_bus_sem);
> +     int ret = 0;
>  
>       while (!pci_is_root_bus(bus)) {
>               bridge = bus->self;
> @@ -2443,9 +2441,9 @@ int pbus_reassign_bridge_resources(struct pci_bus *bus, 
> struct resource *res)
>  
>               /* Ignore BARs which are still in use */
>               if (!res->child) {
> -                     ret = add_to_list(&saved, bridge, res, 0, 0);
> +                     ret = add_to_list(saved, bridge, res, 0, 0);
>                       if (ret)
> -                             goto cleanup;
> +                             return ret;
>  
>                       pci_release_resource(bridge, i);
>               } else {
> @@ -2468,34 +2466,78 @@ int pbus_reassign_bridge_resources(struct pci_bus 
> *bus, struct resource *res)
>               free_list(&added);
>  
>       if (!list_empty(&failed)) {
> -             if (pci_required_resource_failed(&failed, type)) {
> +             if (pci_required_resource_failed(&failed, type))
>                       ret = -ENOSPC;
> -                     goto cleanup;
> -             }
> -             /* Only resources with unrelated types failed (again) */
>               free_list(&failed);
> +             if (ret)
> +                     return ret;
> +
> +             /* Only resources with unrelated types failed (again) */
>       }
>  
> -     list_for_each_entry(dev_res, &saved, list) {
> +     list_for_each_entry(dev_res, saved, list) {
>               struct pci_dev *dev = dev_res->dev;
>  
>               /* Skip the bridge we just assigned resources for */
>               if (bridge == dev)
>                       continue;
>  
> +             if (!dev->subordinate)
> +                     continue;
> +
>               pci_setup_bridge(dev->subordinate);
>       }
>  
> -     free_list(&saved);
> -     up_read(&pci_bus_sem);
>       return 0;
> +}
>  
> -cleanup:
> -     /* Restore size and flags */
> -     list_for_each_entry(dev_res, &failed, list)
> -             restore_dev_resource(dev_res);
> -     free_list(&failed);
> +int pci_do_resource_release_and_resize(struct pci_dev *pdev, int resno, int 
> size,
> +                                    int exclude_bars)
> +{
> +     struct resource *res = pci_resource_n(pdev, resno);
> +     struct pci_dev_resource *dev_res;
> +     struct pci_bus *bus = pdev->bus;
> +     struct resource *b_win, *r;
> +     LIST_HEAD(saved);
> +     unsigned int i;
> +     int ret = 0;
> +
> +     b_win = pbus_select_window(bus, res);
> +     if (!b_win)
> +             return -EINVAL;
> +
> +     pci_dev_for_each_resource(pdev, r, i) {
> +             if (i >= PCI_BRIDGE_RESOURCES)
> +                     break;
> +
> +             if (exclude_bars & BIT(i))
> +                     continue;
>  
> +             if (b_win != pbus_select_window(bus, r))
> +                     continue;
> +
> +             ret = add_to_list(&saved, pdev, r, 0, 0);
> +             if (ret)
> +                     goto restore;
> +             pci_release_resource(pdev, i);
> +     }
> +
> +     pci_resize_resource_set_size(pdev, resno, size);
> +
> +     if (!bus->self)
> +             goto out;
> +
> +     down_read(&pci_bus_sem);
> +     ret = pbus_reassign_bridge_resources(bus, res, &saved);
> +     if (ret)
> +             goto restore;
> +
> +out:
> +     up_read(&pci_bus_sem);
> +     free_list(&saved);
> +     return ret;
> +
> +restore:
>       /* Revert to the old configuration */
>       list_for_each_entry(dev_res, &saved, list) {
>               struct resource *res = dev_res->res;
> @@ -2510,13 +2552,21 @@ int pbus_reassign_bridge_resources(struct pci_bus 
> *bus, struct resource *res)
>  
>               restore_dev_resource(dev_res);
>  
> -             pci_claim_resource(dev, i);
> -             pci_setup_bridge(dev->subordinate);
> -     }
> -     up_read(&pci_bus_sem);
> -     free_list(&saved);
> +             ret = pci_claim_resource(dev, i);
> +             if (ret)
> +                     continue;
>  
> -     return ret;
> +             if (i < PCI_BRIDGE_RESOURCES) {
> +                     const char *res_name = pci_resource_name(dev, i);
> +
> +                     pci_update_resource(dev, i);
> +                     pci_info(dev, "%s %pR: old value restored\n",
> +                              res_name, res);
> +             }
> +             if (dev->subordinate)
> +                     pci_setup_bridge(dev->subordinate);
> +     }
> +     goto out;
>  }
>  
>  void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
> diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> index 3d0b0b3f60c4..e4486d7030c0 100644
> --- a/drivers/pci/setup-res.c
> +++ b/drivers/pci/setup-res.c
> @@ -444,8 +444,7 @@ static bool pci_resize_is_memory_decoding_enabled(struct 
> pci_dev *dev,
>       return cmd & PCI_COMMAND_MEMORY;
>  }
>  
> -static void pci_resize_resource_set_size(struct pci_dev *dev, int resno,
> -                                      int size)
> +void pci_resize_resource_set_size(struct pci_dev *dev, int resno, int size)
>  {
>       resource_size_t res_size = pci_rebar_size_to_bytes(size);
>       struct resource *res = pci_resource_n(dev, resno);
> @@ -456,9 +455,9 @@ static void pci_resize_resource_set_size(struct pci_dev 
> *dev, int resno,
>       resource_set_size(res, res_size);
>  }
>  
> -int pci_resize_resource(struct pci_dev *dev, int resno, int size)
> +int pci_resize_resource(struct pci_dev *dev, int resno, int size,
> +                     int exclude_bars)
>  {
> -     struct resource *res = pci_resource_n(dev, resno);
>       struct pci_host_bridge *host;
>       int old, ret;
>       u32 sizes;
> @@ -468,10 +467,6 @@ int pci_resize_resource(struct pci_dev *dev, int resno, 
> int size)
>       if (host->preserve_config)
>               return -ENOTSUPP;
>  
> -     /* Make sure the resource isn't assigned before resizing it. */
> -     if (!(res->flags & IORESOURCE_UNSET))
> -             return -EBUSY;
> -
>       if (pci_resize_is_memory_decoding_enabled(dev, resno))
>               return -EBUSY;
>  
> @@ -490,19 +485,13 @@ int pci_resize_resource(struct pci_dev *dev, int resno, 
> int size)
>       if (ret)
>               return ret;
>  
> -     pci_resize_resource_set_size(dev, resno, size);
> -
> -     /* Check if the new config works by trying to assign everything. */
> -     if (dev->bus->self) {
> -             ret = pbus_reassign_bridge_resources(dev->bus, res);
> -             if (ret)
> -                     goto error_resize;
> -     }
> +     ret = pci_do_resource_release_and_resize(dev, resno, size, 
> exclude_bars);
> +     if (ret)
> +             goto error_resize;
>       return 0;
>  
>  error_resize:
>       pci_rebar_set_size(dev, resno, old);
> -     pci_resize_resource_set_size(dev, resno, old);
>       return ret;
>  }
>  EXPORT_SYMBOL(pci_resize_resource);
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d1fdf81fbe1e..cc58abbd2b20 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1428,7 +1428,8 @@ static inline int pci_rebar_bytes_to_size(u64 bytes)
>  }
>  
>  u32 pci_rebar_get_possible_sizes(struct pci_dev *pdev, int bar);
> -int __must_check pci_resize_resource(struct pci_dev *dev, int i, int size);
> +int __must_check pci_resize_resource(struct pci_dev *dev, int i, int size,
> +                                  int exlucde_bars);

It seems I managed to mistype this in the prototype, please let me know if 
you want me to send v3 addressing this and the other comments.

-- 
 i.

Reply via email to