[PATCH] PCI: Don't report fully optional resources as assignment failures

[PATCH] PCI: Don't report fully optional resources as assignment failures

[Anthony Pighin]

A hotplug bridge may request a window with required size 0 plus
add_size headroom.  For example, the PCI core speculatively requests
a non-prefetchable MMIO reserve for a hotplug bridge even when no
downstream BAR currently requires that space.

reassign_resources_sorted() temporarily grows an unassigned resource
from its required size to the required size plus add_size.  If
pci_assign_resource() fails, the failure is ignored because the
additional space is optional, but the resource is left unassigned at
the enlarged size.

Since commit 96336ec70264 ("PCI: Perform reset_resource() and build
fail list in sync") and commit 2499f5348431 ("PCI: Rework optional
resource handling"), the __assign_resources_sorted() out path
unconditionally adds every remaining unassigned resource to fail_head.
The headroom-enlarged window is therefore reported as though a required
resource failed.

pci_assign_unassigned_bridge_resources() interprets the non-empty
fail_head as grounds for another assignment round and releases bridge
windows with whole_subtree.  On a system whose < 4GB root-bus MMIO
aperture was already fully allocated, hotplugging an endpoint that
required only a 64-bit prefetchable BAR produced:

  pcieport 0000:00:03.3: bridge window [mem size 0x00000000] add_size 200000
  pcieport 0000:00:03.3: bridge window [mem size 0x00200000]: failed to assign
  PCI: No. 2 try to assign unassigned res
  pcieport 0000:00:01.3: bridge window [mem 0xe6800000-0xe68fffff]: releasing
  igb 0000:02:00.0 mgmt: PCIe link lost

The endpoint's required prefetchable BAR had already been assigned; only
the non-prefetchable hotplug reserve had failed.  Releasing the sibling
bridge window removed MMIO from the active igb device.

Save the required size before attempting the optional allocation and
restore it when that allocation fails.  A fully optional bridge window
is then left at size 0.  In the __assign_resources_sorted() out path,
only report unassigned resources that have non-zero size and are not
otherwise optional.  A zero-sized resource has no required part and
does not represent a required failure.

This leaves the speculative reserve unassigned without triggering
another assignment round.  Required resource failures continue to be
reported and retried as before.

Fixes: 96336ec70264 ("PCI: Perform reset_resource() and build fail list in sync")
Fixes: 2499f5348431 ("PCI: Rework optional resource handling")
Cc: stable@vger.kernel.org
Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>
---
 drivers/pci/setup-bus.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index c0a949f2c995..c16f0a5f9456 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -450,12 +450,19 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
 		add_size = add_res->add_size;
 		align = add_res->min_align;
 		if (!resource_assigned(res)) {
-			resource_set_range(res, align,
-					   resource_size(res) + add_size);
+			resource_size_t req_size = resource_size(res);
+
+			resource_set_range(res, align, req_size + add_size);
 			if (pci_assign_resource(dev, idx)) {
 				pci_dbg(dev,
 					"%s %pR: ignoring failure in optional allocation\n",
 					res_name, res);
+				/*
+				 * Restore the required size so a fully optional
+				 * resource is left zero-sized and not later
+				 * mistaken for a required failure.
+				 */
+				resource_set_range(res, align, req_size);
 			}
 		} else if (add_size > 0 || !IS_ALIGNED(res->start, align)) {
 			res->flags |= add_res->flags &
@@ -743,7 +750,13 @@ static void __assign_resources_sorted(struct list_head *head,
 		if (resource_assigned(res))
 			continue;
 
-		if (fail_head) {
+		/*
+		 * Report only required failures.  Skip optional and
+		 * zero-sized resources; reporting them would trigger
+		 * another release/retry round.
+		 */
+		if (fail_head && resource_size(res) &&
+		    !pci_resource_is_optional(dev, pci_resource_num(dev, res))) {
 			pci_dev_res_add_to_list(fail_head, dev, res,
 						0 /* don't care */,
 						0 /* don't care */);
-- 
2.43.0

Re: [PATCH] PCI: Don't report fully optional resources as assignment failures

[sashiko-bot]

> A hotplug bridge may request a window with required size 0 plus
> add_size headroom.  For example, the PCI core speculatively requests
> a non-prefetchable MMIO reserve for a hotplug bridge even when no
> downstream BAR currently requires that space.
> 
> reassign_resources_sorted() temporarily grows an unassigned resource
> from its required size to the required size plus add_size.  If
> pci_assign_resource() fails, the failure is ignored because the
> additional space is optional, but the resource is left unassigned at
> the enlarged size.
> 
> Since commit 96336ec70264 ("PCI: Perform reset_resource() and build
> fail list in sync") and commit 2499f5348431 ("PCI: Rework optional
> resource handling"), the __assign_resources_sorted() out path
> unconditionally adds every remaining unassigned resource to fail_head.
> [ ... ]
> 
> Fixes: 96336ec70264 ("PCI: Perform reset_resource() and build fail list in sync")
> Fixes: 2499f5348431 ("PCI: Rework optional resource handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717180029.1829888-1-anthony.pighin@nokia.com?part=1

Re: [PATCH] PCI: Don't report fully optional resources as assignment failures

[Ilpo Järvinen]

On Fri, 17 Jul 2026, Anthony Pighin wrote:

> A hotplug bridge may request a window with required size 0 plus
> add_size headroom.  For example, the PCI core speculatively requests
> a non-prefetchable MMIO reserve for a hotplug bridge even when no
> downstream BAR currently requires that space.
> 
> reassign_resources_sorted() temporarily grows an unassigned resource
> from its required size to the required size plus add_size.  If
> pci_assign_resource() fails, the failure is ignored because the
> additional space is optional, but the resource is left unassigned at
> the enlarged size.
> 
> Since commit 96336ec70264 ("PCI: Perform reset_resource() and build
> fail list in sync") and commit 2499f5348431 ("PCI: Rework optional
> resource handling"), the __assign_resources_sorted() out path
> unconditionally adds every remaining unassigned resource to fail_head.
> The headroom-enlarged window is therefore reported as though a required
> resource failed.
> 
> pci_assign_unassigned_bridge_resources() interprets the non-empty
> fail_head as grounds for another assignment round and releases bridge
> windows with whole_subtree.  On a system whose < 4GB root-bus MMIO
> aperture was already fully allocated, hotplugging an endpoint that
> required only a 64-bit prefetchable BAR produced:
> 
>   pcieport 0000:00:03.3: bridge window [mem size 0x00000000] add_size 200000
>   pcieport 0000:00:03.3: bridge window [mem size 0x00200000]: failed to assign
>   PCI: No. 2 try to assign unassigned res
>   pcieport 0000:00:01.3: bridge window [mem 0xe6800000-0xe68fffff]: releasing
>   igb 0000:02:00.0 mgmt: PCIe link lost
> 
> The endpoint's required prefetchable BAR had already been assigned; only
> the non-prefetchable hotplug reserve had failed.  Releasing the sibling
> bridge window removed MMIO from the active igb device.
> 
> Save the required size before attempting the optional allocation and
> restore it when that allocation fails.  A fully optional bridge window
> is then left at size 0.  In the __assign_resources_sorted() out path,
> only report unassigned resources that have non-zero size and are not
> otherwise optional.  A zero-sized resource has no required part and
> does not represent a required failure.
>
> This leaves the speculative reserve unassigned without triggering
> another assignment round.  Required resource failures continue to be
> reported and retried as before.
> 
> Fixes: 96336ec70264 ("PCI: Perform reset_resource() and build fail list in sync")
> Fixes: 2499f5348431 ("PCI: Rework optional resource handling")
> Cc: stable@vger.kernel.org
> Signed-off-by: Anthony Pighin <anthony.pighin@nokia.com>
> ---
>  drivers/pci/setup-bus.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index c0a949f2c995..c16f0a5f9456 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -450,12 +450,19 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
>  		add_size = add_res->add_size;
>  		align = add_res->min_align;
>  		if (!resource_assigned(res)) {
> -			resource_set_range(res, align,
> -					   resource_size(res) + add_size);
> +			resource_size_t req_size = resource_size(res);
> +
> +			resource_set_range(res, align, req_size + add_size);
>  			if (pci_assign_resource(dev, idx)) {
>  				pci_dbg(dev,
>  					"%s %pR: ignoring failure in optional allocation\n",
>  					res_name, res);
> +				/*
> +				 * Restore the required size so a fully optional
> +				 * resource is left zero-sized and not later
> +				 * mistaken for a required failure.
> +				 */
> +				resource_set_range(res, align, req_size);

This part seems valid, the resource should not be left into enlarged 
state if the assignment fails.

>  			}
>  		} else if (add_size > 0 || !IS_ALIGNED(res->start, align)) {
>  			res->flags |= add_res->flags &
> @@ -743,7 +750,13 @@ static void __assign_resources_sorted(struct list_head *head,
>  		if (resource_assigned(res))
>  			continue;
>  
> -		if (fail_head) {
> +		/*
> +		 * Report only required failures.  Skip optional and
> +		 * zero-sized resources; reporting them would trigger
> +		 * another release/retry round.
> +		 */
> +		if (fail_head && resource_size(res) &&
> +		    !pci_resource_is_optional(dev, pci_resource_num(dev, res))) {

This, however, will break some cases I think.

We first want to try fitting also optional resources, if those optional 
resources never go into fail_head, there won't be new pass and not enough 
upstream bridge windows will be released so the algorithm won't really end 
up even trying to fit the optional resources after this change if FW left 
some bridge window too small.

Only after that has been tried, it should try to fallback into fitting 
only required resources.

So, I think some other solution should be created to solve your issue.
I don't even know what that issue is since you don't show what happens 
with the extra pass after release and why it fails.

-- 
 i.

Re: [PATCH] PCI: Don't report fully optional resources as assignment failures

[Anthony Pighin]

v2: https://lore.kernel.org/linux-pci/20260727193714.598562-1-anthony.pighin@nokia.com/

On Mon, 20 Jul 2026, Ilpo Järvinen wrote:

> This part seems valid, the resource should not be left into enlarged
> state if the assignment fails.

Kept in v2.  reassign_resources_sorted() saves the required size and
restores it when the optional attempt fails.

> > +		if (fail_head && resource_size(res) &&
> > +		    !pci_resource_is_optional(dev, pci_resource_num(dev, res))) {
>
> This, however, will break some cases I think.
>
> We first want to try fitting also optional resources, if those optional
> resources never go into fail_head, there won't be new pass and not enough
> upstream bridge windows will be released so the algorithm won't really end
> up even trying to fit the optional resources after this change if FW left
> some bridge window too small.
>
> Only after that has been tried, it should try to fallback into fitting
> only required resources.

You're right about the pci_resource_is_optional() check.  It also covers
IOV BARs and disabled ROMs, which have a real requested size, so it would
have kept them out of fail_head and killed the release/retry rounds they
depend on.  The effect is worse in the root bus path, where a pass failing
only on optional resources would break the loop before the iteration that
passes add_list.  v2 drops it and keeps only the zero-size check.

The case where FW left a window too small should still work, though.
pbus_size_mem() sizes the window from the required child resources, so
such a window has a non-zero required size and still reaches fail_head
and the retry round.  So does a child BAR that fails to fit.  Only a
window with required size 0, where nothing downstream needs that type at
all, is skipped.

reassign_resources_sorted() also still tries the enlarged window where it
always did, so the optional fit is attempted as before; v2 only restores
the size when that attempt fails.  What v2 gives up in the size-0 case is
the extra pass, which releases live sibling windows to win headroom
nothing requested.

> I don't even know what that issue is since you don't show what happens
> with the extra pass after release and why it fails.

Apologies, v1 omitted it.  v2 carries the full sequence.  The extra pass
does not fail to assign, it succeeds, and that is the problem.  The
failing reserve is on 00:03.5, but pci_prepare_next_assign_round()
releases the window of its parent bus, so every non-prefetchable window
on bus 00 goes with whole_subtree, along with every BAR below them, and
each bridge is reprogrammed right away.  The reassignment only covers the
hotplugged bridge, so none of that is restored.  00:03.5 ends up with a
2MB window nothing asked for, and the igb loses its BARs while its driver
is bound.

Thanks,
Anthony

Re: [PATCH] PCI: Don't report fully optional resources as assignment failures

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 4802 bytes --]

On Mon, 27 Jul 2026, Anthony Pighin wrote:

> v2: https://lore.kernel.org/linux-pci/20260727193714.598562-1-anthony.pighin@nokia.com/
> 
> On Mon, 20 Jul 2026, Ilpo Järvinen wrote:
> 
> > This part seems valid, the resource should not be left into enlarged
> > state if the assignment fails.
> 
> Kept in v2.  reassign_resources_sorted() saves the required size and
> restores it when the optional attempt fails.
> 
> > > +		if (fail_head && resource_size(res) &&
> > > +		    !pci_resource_is_optional(dev, pci_resource_num(dev, res))) {
> >
> > This, however, will break some cases I think.
> >
> > We first want to try fitting also optional resources, if those optional
> > resources never go into fail_head, there won't be new pass and not enough
> > upstream bridge windows will be released so the algorithm won't really end
> > up even trying to fit the optional resources after this change if FW left
> > some bridge window too small.
> >
> > Only after that has been tried, it should try to fallback into fitting
> > only required resources.
> 
> You're right about the pci_resource_is_optional() check.  It also covers
> IOV BARs and disabled ROMs, which have a real requested size, so it would
> have kept them out of fail_head and killed the release/retry rounds they
> depend on.  The effect is worse in the root bus path, where a pass failing
> only on optional resources would break the loop before the iteration that
> passes add_list.  v2 drops it and keeps only the zero-size check.
> 
> The case where FW left a window too small should still work, though.
> pbus_size_mem() sizes the window from the required child resources, so
> such a window has a non-zero required size and still reaches fail_head
> and the retry round. So does a child BAR that fails to fit.  Only a
> window with required size 0, where nothing downstream needs that type at
> all, is skipped.

So a window which contains only optional BARs would not result another 
pass to retry? While probably rare, there are such bridge windows 
because we recently hit that corner case in pbus_size_mem_optional() where 
the entire window was optional with required size being zero, and I had to 
fix the logic there. Even if there wouldn't be such devices, I think your 
reasoning is still not entirely sound.

> reassign_resources_sorted() also still tries the enlarged window where it
> always did, so the optional fit is attempted as before; v2 only restores
> the size when that attempt fails.  What v2 gives up in the size-0 case is
> the extra pass, which releases live sibling windows to win headroom
> nothing requested.
> 
> > I don't even know what that issue is since you don't show what happens
> > with the extra pass after release and why it fails.
> 
> Apologies, v1 omitted it.  v2 carries the full sequence.

Unfortunately, v2 didn't tell me what happened to igb's BAR (in the end) 
as you cut the log out after the driver failed.

> The extra pass
> does not fail to assign, it succeeds, and that is the problem.  The
> failing reserve is on 00:03.5, but pci_prepare_next_assign_round()
> releases the window of its parent bus, so every non-prefetchable window
> on bus 00 goes with whole_subtree, along with every BAR below them, and
> each bridge is reprogrammed right away.  The reassignment only covers the
> hotplugged bridge, so none of that is restored.  00:03.5 ends up with a
> 2MB window nothing asked for, and the igb loses its BARs while its driver
> is bound.

So the entire fitting and assignment operation moved the igb BAR or igb's
BAR is entirely gone (partial assignment failure)?

In any case, the resource fitting algorithm should not be touching BARs 
that are in use! ...But we likely lack check for that, which should be 
fixed. I'm not even entirely sure though how that can be checked, is it 
enough the check if a BAR has at least one child resource?

I would not be very surprised if there are other similar problems that 
can only occur in hotplug case where it goes to tear down something that 
is in use. For bridge windows, there's some attempt to prevent releasing 
windows that still have children, but for the actual BARs, I don't recall 
seeing effort to prevent releasing them. Maybe it could be placed into 
something generic such as pci_release_resource(), not all its callers seem 
to handle errors from it though so some extra work to deal with those is 
required as well.

And as mentioned in v2 email, release_child_resources() works on a wrong 
level to be able to detect things properly so a PCI level replacement may 
have to be added to be able to access PCI level structs to know what 
resource is BAR, bridge window and what is a child resource for BAR.

-- 
 i.