]> Gentwo Git Trees - linux/.git/commitdiff
PCI: Fix restoring BARs on BAR resize rollback path
authorIlpo Järvinen <ilpo.jarvinen@linux.intel.com>
Thu, 13 Nov 2025 16:26:23 +0000 (18:26 +0200)
committerBjorn Helgaas <bhelgaas@google.com>
Fri, 14 Nov 2025 18:33:14 +0000 (12:33 -0600)
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() rolls back 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 __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 in how FW and the
kernel assign the resources (alignment of the start address and tail).

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
pass its register BAR (BAR 2 or 5), which should never be released during
resize operation. Normally 64-bit prefetchable resources do not share a
bridge window with the 32-bit only register BAR, but there are various
fallbacks in the resource assignment logic which may make the resources
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.

Reported-by: Simon Richter <Simon.Richter@hogyros.de>
Link: https://lore.kernel.org/linux-pci/f9a8c975-f5d3-4dd2-988e-4371a1433a60@hogyros.de/
Reported-by: Alex Bennée <alex.bennee@linaro.org>
Link: https://lore.kernel.org/linux-pci/874irqop6b.fsf@draig.linaro.org/
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
[bhelgaas: squash amdgpu BAR selection from
https://lore.kernel.org/r/20251114103053.13778-1-ilpo.jarvinen@linux.intel.com]
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Tested-by: Alex Bennée <alex.bennee@linaro.org> # AVA, AMD GPU
Reviewed-by: Christian König <christian.koenig@amd.com>
Link: https://patch.msgid.link/20251113162628.5946-7-ilpo.jarvinen@linux.intel.com
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
drivers/gpu/drm/i915/gt/intel_region_lmem.c
drivers/gpu/drm/xe/xe_vram.c
drivers/pci/pci-sysfs.c
drivers/pci/pci.h
drivers/pci/setup-bus.c
drivers/pci/setup-res.c
include/linux/pci.h

index 7a899fb4de29cb02f9a6ce267973cbaf8a0a22b7..bf0bc38e1c47f8a6eb589a2cf3f0a712400bcc77 100644 (file)
@@ -1736,7 +1736,9 @@ 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,
+                               (adev->asic_type >= CHIP_BONAIRE) ? 1 << 5
+                                                                 : 1 << 2);
        if (r == -ENOSPC)
                dev_info(adev->dev,
                         "Not enough PCI address space for a large BAR.");
index 51bb27e10a4f87190dbcaea020b10cbe1dea0fad..7699e8fcf5edf23d21cf273bad24683373f1aba5 100644 (file)
@@ -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));
index b44ebf50fedbbbe2f1ba2df1001931ef9b1e2e3a..00dd027057df1ece9bddcc36de4385e9befa18e9 100644 (file)
@@ -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));
index 9d6f74bd95f8c2184bd36718faf82e1422452baf..2a1b5456c2dc1d0435ceb0a68c5976753b75b611 100644 (file)
@@ -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);
 
index bf1a577e962374d109068996c0b824e6dfb9d5bc..9893ea12d1f294844f36599ffd684d79af89b1aa 100644 (file)
@@ -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);
index 51f5e5a80b5477b95d7db4cc44c048081a7e959a..7e268960954bdfc2c2409aa909837b76fe37a248 100644 (file)
@@ -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)
index 3d0b0b3f60c4cee73debd530e631f12eeb3fb4e3..e4486d7030c0b5fee8603ef6526446e7359bdfe7 100644 (file)
@@ -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);
index d1fdf81fbe1e427aecbc951fa3fdf65c20450b05..34ff295cd2e3fcf3014cb03f30133088996042d9 100644 (file)
@@ -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 exclude_bars);
 int pci_select_bars(struct pci_dev *dev, unsigned long flags);
 bool pci_device_is_present(struct pci_dev *pdev);
 void pci_ignore_hotplug(struct pci_dev *dev);