Re: Patch "PCI: Preserve bridge window resource type flags" has been added to the 6.17-stable tree

-- i. > commit b871817e3ba3e462dbc10491a2786cd3fb9dc064 > Author: Ilpo Järvinen ilpo.jarvinen@linux.intel.com > Date: Fri Aug 29 16:10:59 2025 +0300 > > PCI: Preserve bridge window resource type flags > > [ Upstream commit 8278c6914306f35f32d73bdf2a918950919a0051 ] > > When a bridge window is found unused or fails to assign, the flags of the > associated resource are cleared. Clearing flags is problematic as it also > removes the type information of the resource which is needed later. > > Thus, always preserve the bridge window type flags and use IORESOURCE_UNSET > and IORESOURCE_DISABLED to indicate the status of the bridge window. Also, > when initializing resources, make sure all valid bridge windows do get > their type flags set. > > Change various places that relied on resource flags being cleared to check > for IORESOURCE_UNSET and IORESOURCE_DISABLED to allow bridge window > resource to retain their type flags. Add pdev_resource_assignable() and > pdev_resource_should_fit() helpers to filter out disabled bridge windows > during resource fitting; the latter combines more common checks into the > helper. > > When reading the bridge windows from the registers, instead of leaving the > resource flags cleared for bridge windows that are not enabled, always > set up the flags and set IORESOURCE_UNSET | IORESOURCE_DISABLED as needed. > > When resource fitting or assignment fails for a bridge window resource, or > the bridge window is not needed, mark the resource with IORESOURCE_UNSET or > IORESOURCE_DISABLED, respectively. > > Use dummy zero resource in resource_show() for backwards compatibility as > lspci will otherwise misrepresent disabled bridge windows. > > This change fixes an issue which highlights the importance of keeping the > resource type flags intact: > > At the end of __assign_resources_sorted(), reset_resource() is called, > previously clearing the flags. Later, pci_prepare_next_assign_round() > attempted to release bridge resources using > pci_bus_release_bridge_resources() that calls into > pci_bridge_release_resources() that assumes type flags are still present. > As type flags were cleared, IORESOURCE_MEM_64 was not set leading to > resources under an incorrect bridge window to be released (idx = 1 > instead of idx = 2). While the assignments performed later covered this > problem so that the wrongly released resources got assigned in the end, > it was still causing extra release+assign pairs. > > There are other reasons why the resource flags should be retained in > upcoming changes too. > > Removing the flag reset for non-bridge window resource is left as future > work, in part because it has a much higher regression potential due to > pci_enable_resources() that will start to work also for those resources > then and due to what endpoint drivers might assume about resources. > > Despite the Fixes tag, backporting this (at least any time soon) is highly > discouraged. The issue fixed is borderline cosmetic as the later > assignments normally cover the problem entirely. Also there might be > non-obvious dependencies. > > Fixes: 5b28541552ef ("PCI: Restrict 64-bit prefetchable bridge windows to 64-bit resources") > Signed-off-by: Ilpo Järvinen ilpo.jarvinen@linux.intel.com > Signed-off-by: Bjorn Helgaas bhelgaas@google.com > Link: https://patch.msgid.link/20250829131113.36754-11-ilpo.jarvinen@linux.intel.c... > Signed-off-by: Sasha Levin sashal@kernel.org > > diff --git a/drivers/pci/bus.c b/drivers/pci/bus.c > index b77fd30bbfd9d..58b5388423ee3 100644 > --- a/drivers/pci/bus.c > +++ b/drivers/pci/bus.c > @@ -204,6 +204,9 @@ static int pci_bus_alloc_from_region(struct pci_bus *bus, struct resource *res, > if (!r) > continue; > > + if (r->flags & (IORESOURCE_UNSET|IORESOURCE_DISABLED)) > + continue; > + > /* type_mask must match */ > if ((res->flags ^ r->flags) & type_mask) > continue; > diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c > index 5eea14c1f7f5f..162a5241c7f70 100644 > --- a/drivers/pci/pci-sysfs.c > +++ b/drivers/pci/pci-sysfs.c > @@ -177,6 +177,13 @@ static ssize_t resource_show(struct device *dev, struct device_attribute *attr, > > for (i = 0; i < max; i++) { > struct resource *res = &pci_dev->resource[i]; > + struct resource zerores = {}; > + > + /* For backwards compatibility */ > + if (i >= PCI_BRIDGE_RESOURCES && i <= PCI_BRIDGE_RESOURCE_END && > + res->flags & (IORESOURCE_UNSET | IORESOURCE_DISABLED)) > + res = &zerores; > + > pci_resource_to_user(pci_dev, i, res, &start, &end); > len += sysfs_emit_at(buf, len, "0x%016llx 0x%016llx 0x%016llx\n", > (unsigned long long)start, > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index f41128f91ca76..f31d27c7708a6 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c > @@ -419,13 +419,17 @@ static void pci_read_bridge_io(struct pci_dev *dev, struct resource *res, > limit |= ((unsigned long) io_limit_hi << 16); > } > > + res->flags = (io_base_lo & PCI_IO_RANGE_TYPE_MASK) | IORESOURCE_IO; > + > if (base <= limit) { > - res->flags = (io_base_lo & PCI_IO_RANGE_TYPE_MASK) | IORESOURCE_IO; > region.start = base; > region.end = limit + io_granularity - 1; > pcibios_bus_to_resource(dev->bus, res, &region); > if (log) > pci_info(dev, " bridge window %pR\n", res); > + } else { > + resource_set_range(res, 0, 0); > + res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED; > } > } > > @@ -440,13 +444,18 @@ static void pci_read_bridge_mmio(struct pci_dev *dev, struct resource *res, > pci_read_config_word(dev, PCI_MEMORY_LIMIT, &mem_limit_lo); > base = ((unsigned long) mem_base_lo & PCI_MEMORY_RANGE_MASK) << 16; > limit = ((unsigned long) mem_limit_lo & PCI_MEMORY_RANGE_MASK) << 16; > + > + res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM; > + > if (base <= limit) { > - res->flags = (mem_base_lo & PCI_MEMORY_RANGE_TYPE_MASK) | IORESOURCE_MEM; > region.start = base; > region.end = limit + 0xfffff; > pcibios_bus_to_resource(dev->bus, res, &region); > if (log) > pci_info(dev, " bridge window %pR\n", res); > + } else { > + resource_set_range(res, 0, 0); > + res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED; > } > } > > @@ -489,16 +498,20 @@ static void pci_read_bridge_mmio_pref(struct pci_dev *dev, struct resource *res, > return; > } > > + res->flags = (mem_base_lo & PCI_PREF_RANGE_TYPE_MASK) | IORESOURCE_MEM | > + IORESOURCE_PREFETCH; > + if (res->flags & PCI_PREF_RANGE_TYPE_64) > + res->flags |= IORESOURCE_MEM_64; > + > if (base <= limit) { > - res->flags = (mem_base_lo & PCI_PREF_RANGE_TYPE_MASK) | > - IORESOURCE_MEM | IORESOURCE_PREFETCH; > - if (res->flags & PCI_PREF_RANGE_TYPE_64) > - res->flags |= IORESOURCE_MEM_64; > region.start = base; > region.end = limit + 0xfffff; > pcibios_bus_to_resource(dev->bus, res, &region); > if (log) > pci_info(dev, " bridge window %pR\n", res); > + } else { > + resource_set_range(res, 0, 0); > + res->flags |= IORESOURCE_UNSET | IORESOURCE_DISABLED; > } > } > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index 203c8ebef7029..8078ee30e675f 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -154,6 +154,31 @@ static bool pdev_resources_assignable(struct pci_dev *dev) > return true; > } > > +static bool pdev_resource_assignable(struct pci_dev *dev, struct resource *res) > +{ > + int idx = pci_resource_num(dev, res); > + > + if (!res->flags) > + return false; > + > + if (idx >= PCI_BRIDGE_RESOURCES && idx <= PCI_BRIDGE_RESOURCE_END && > + res->flags & IORESOURCE_DISABLED) > + return false; > + > + return true; > +} > + > +static bool pdev_resource_should_fit(struct pci_dev *dev, struct resource *res) > +{ > + if (res->parent) > + return false; > + > + if (res->flags & IORESOURCE_PCI_FIXED) > + return false; > + > + return pdev_resource_assignable(dev, res); > +} > + > /* Sort resources by alignment */ > static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head) > { > @@ -169,10 +194,7 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head) > resource_size_t r_align; > struct list_head *n; > > - if (r->flags & IORESOURCE_PCI_FIXED) > - continue; > - > - if (!(r->flags) || r->parent) > + if (!pdev_resource_should_fit(dev, r)) > continue; > > r_align = pci_resource_alignment(dev, r); > @@ -221,8 +243,15 @@ bool pci_resource_is_optional(const struct pci_dev *dev, int resno) > return false; > } > > -static inline void reset_resource(struct resource *res) > +static inline void reset_resource(struct pci_dev *dev, struct resource *res) > { > + int idx = pci_resource_num(dev, res); > + > + if (idx >= PCI_BRIDGE_RESOURCES && idx <= PCI_BRIDGE_RESOURCE_END) { > + res->flags |= IORESOURCE_UNSET; > + return; > + } > + > res->start = 0; > res->end = 0; > res->flags = 0; > @@ -568,7 +597,7 @@ static void __assign_resources_sorted(struct list_head *head, > 0 /* don't care */); > } > > - reset_resource(res); > + reset_resource(dev, res); > } > > free_list(head); > @@ -997,8 +1026,11 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, > > if (r->parent || !(r->flags & IORESOURCE_IO)) > continue; > - r_size = resource_size(r); > > + if (!pdev_resource_assignable(dev, r)) > + continue; > + > + r_size = resource_size(r); > if (r_size < SZ_1K) > /* Might be re-aligned for ISA */ > size += r_size; > @@ -1017,6 +1049,9 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, > size0 = calculate_iosize(size, min_size, size1, 0, 0, > resource_size(b_res), min_align); > > + if (size0) > + b_res->flags &= ~IORESOURCE_DISABLED; > + > size1 = size0; > if (realloc_head && (add_size > 0 || children_add_size > 0)) { > size1 = calculate_iosize(size, min_size, size1, add_size, > @@ -1028,13 +1063,14 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size, > if (bus->self && (b_res->start || b_res->end)) > pci_info(bus->self, "disabling bridge window %pR to %pR (unused)\n", > b_res, &bus->busn_res); > - b_res->flags = 0; > + b_res->flags |= IORESOURCE_DISABLED; > return; > } > > resource_set_range(b_res, min_align, size0); > b_res->flags |= IORESOURCE_STARTALIGN; > if (bus->self && size1 > size0 && realloc_head) { > + b_res->flags &= ~IORESOURCE_DISABLED; > add_to_list(realloc_head, bus->self, b_res, size1-size0, > min_align); > pci_info(bus->self, "bridge window %pR to %pR add_size %llx\n", > @@ -1180,11 +1216,13 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, > const char *r_name = pci_resource_name(dev, i); > resource_size_t r_size; > > - if (r->parent || (r->flags & IORESOURCE_PCI_FIXED) || > - !pdev_resources_assignable(dev) || > - ((r->flags & mask) != type && > - (r->flags & mask) != type2 && > - (r->flags & mask) != type3)) > + if (!pdev_resources_assignable(dev) || > + !pdev_resource_should_fit(dev, r)) > + continue; > + > + if ((r->flags & mask) != type && > + (r->flags & mask) != type2 && > + (r->flags & mask) != type3) > continue; > r_size = resource_size(r); > > @@ -1235,6 +1273,9 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, > min_align = max(min_align, win_align); > size0 = calculate_memsize(size, min_size, 0, 0, resource_size(b_res), min_align); > > + if (size0) > + b_res->flags &= ~IORESOURCE_DISABLED; > + > if (bus->self && size0 && > !pbus_upstream_space_available(bus, mask | IORESOURCE_PREFETCH, type, > size0, min_align)) { > @@ -1267,13 +1308,14 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask, > if (bus->self && (b_res->start || b_res->end)) > pci_info(bus->self, "disabling bridge window %pR to %pR (unused)\n", > b_res, &bus->busn_res); > - b_res->flags = 0; > + b_res->flags |= IORESOURCE_DISABLED; > return 0; > } > > resource_set_range(b_res, min_align, size0); > b_res->flags |= IORESOURCE_STARTALIGN; > if (bus->self && size1 > size0 && realloc_head) { > + b_res->flags &= ~IORESOURCE_DISABLED; > add_to_list(realloc_head, bus->self, b_res, size1-size0, add_align); > pci_info(bus->self, "bridge window %pR to %pR add_size %llx add_align %llx\n", > b_res, &bus->busn_res, > @@ -1705,7 +1747,6 @@ static void pci_bridge_release_resources(struct pci_bus *bus, > { > struct pci_dev *dev = bus->self; > struct resource *r; > - unsigned int old_flags; > struct resource *b_res; > int idx, ret; > > @@ -1742,17 +1783,15 @@ static void pci_bridge_release_resources(struct pci_bus *bus, > /* If there are children, release them all */ > release_child_resources(r); > > - type = old_flags = r->flags & PCI_RES_TYPE_MASK; > ret = pci_release_resource(dev, PCI_BRIDGE_RESOURCES + idx); > if (ret) > return; > > + type = r->flags & PCI_RES_TYPE_MASK; > /* Avoiding touch the one without PREF */ > if (type & IORESOURCE_PREFETCH) > type = IORESOURCE_PREFETCH; > __pci_setup_bridge(bus, type); > - /* For next child res under same bridge */ > - r->flags = old_flags; > } > > enum release_type { > @@ -2230,21 +2269,9 @@ static void pci_prepare_next_assign_round(struct list_head *fail_head, > } > > /* Restore size and flags */ > - list_for_each_entry(fail_res, fail_head, list) { > - struct resource *res = fail_res->res; > - struct pci_dev *dev = fail_res->dev; > - int idx = pci_resource_num(dev, res); > - > + list_for_each_entry(fail_res, fail_head, list) > restore_dev_resource(fail_res); > > - if (!pci_is_bridge(dev)) > - continue; > - > - if (idx >= PCI_BRIDGE_RESOURCES && > - idx <= PCI_BRIDGE_RESOURCE_END) > - res->flags = 0; > - } > - > free_list(fail_head); > } > > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c > index 0468c058b5987..c5ef8ef54d3ce 100644 > --- a/drivers/pci/setup-res.c > +++ b/drivers/pci/setup-res.c > @@ -359,6 +359,9 @@ int pci_assign_resource(struct pci_dev *dev, int resno) > > res->flags &= ~IORESOURCE_UNSET; > res->flags &= ~IORESOURCE_STARTALIGN; > + if (resno >= PCI_BRIDGE_RESOURCES && resno <= PCI_BRIDGE_RESOURCE_END) > + res->flags &= ~IORESOURCE_DISABLED; > + > pci_info(dev, "%s %pR: assigned\n", res_name, res); > if (resno < PCI_BRIDGE_RESOURCES) > pci_update_resource(dev, resno); >