While I agree the assessment that this is stable material, could we
postpone queueing to stable a bit more? This change has relatively high
regression potential and high impact for individual system that regresses,
and is complicated change no matter what already given the diffstat alone.
So try e.g. after 6.18 is released would seem more prudent for me as it
would give more time to iron out problems in the rc-phase before any
stable sees this change.
I'm also very worried this change is queued without the
commit 1cdffa51ecc4 ("PCI: Enable bridge even if bridge window fails to
assign"), that's pretty much straight into territory I've personally
charted to contain ways to break things which is why I added 1cdffa51ecc4.
There are also the supporting arch cleanups to the same functionality as
1cdffa51ecc4 (done through weak magic) that are related to 1cdffa51ecc4.
--
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, ®ion);
> 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, ®ion);
> 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, ®ion);
> 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);
>