[PATCH 1/1] PCI: Account fully optional bridge windows correctly

[PATCH 1/1] PCI: Account fully optional bridge windows correctly

[Ilpo Järvinen]

pbus_size_mem_optional() adds dev_res->add_size of a bridge window into
children_add_size when the window has a non-optional part. However, if
the bridge window is fully optional, only r_size is added (which is
zero for such a window).

Also, a second dev_res entry will be added by pci_dev_res_add_to_list()
into realloc_head for the bridge window (resulting in triggering the
realloc_head-must-be-fully-consumed sanity check after a single pass of
the resource assignment algorithm):

  WARNING: drivers/pci/setup-bus.c:2153 at pci_assign_unassigned_root_bus_resources+0xa5/0x260

Correct these problems by always adding dev_res->add_size for bridge
windows and not calling pci_dev_res_add_to_list() if the dev_res entry
exists.

Fixes: 6a5e64c75e82 ("PCI: Add pbus_mem_size_optional() to handle optional sizes")
Reported-by: RavitejaX Veesam <ravitejax.veesam@intel.com>
Tested-by: RavitejaX Veesam <ravitejax.veesam@intel.com>
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/pci/setup-bus.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 32aa72456a44..0c8cbbfaf8d7 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1217,31 +1217,34 @@ static bool pbus_size_mem_optional(struct pci_dev *dev, int resno,
 	struct resource *res = pci_resource_n(dev, resno);
 	bool optional = pci_resource_is_optional(dev, resno);
 	resource_size_t r_size = resource_size(res);
-	struct pci_dev_resource *dev_res;
+	struct pci_dev_resource *dev_res = NULL;
 
 	if (!realloc_head)
 		return false;
 
-	if (!optional) {
-		/*
-		 * Only bridges have optional sizes in realloc_head at this
-		 * point. As res_to_dev_res() walks the entire realloc_head
-		 * list, skip calling it when known unnecessary.
-		 */
-		if (!pci_resource_is_bridge_win(resno))
-			return false;
-
+	/*
+	 * Only bridges have optional sizes in realloc_head at this
+	 * point. As res_to_dev_res() walks the entire realloc_head
+	 * list, skip calling it when known unnecessary.
+	 */
+	if (pci_resource_is_bridge_win(resno)) {
 		dev_res = res_to_dev_res(realloc_head, res);
 		if (dev_res) {
 			*children_add_size += dev_res->add_size;
 			*add_align = max(*add_align, dev_res->min_align);
 		}
+	}
 
+	if (!optional)
 		return false;
-	}
 
-	/* Put SRIOV requested res to the optional list */
-	pci_dev_res_add_to_list(realloc_head, dev, res, 0, align);
+	/*
+	 * Put requested res to the optional list if not there yet (SRIOV,
+	 * disabled ROM). Bridge windows with an optional part are already
+	 * on the list.
+	 */
+	if (!dev_res)
+		pci_dev_res_add_to_list(realloc_head, dev, res, 0, align);
 	*children_add_size += r_size;
 	*add_align = max(align, *add_align);
 

base-commit: 1c2b4a4c2bcb950f182eeeb33d94b565607608cf
-- 
2.39.5

Re: [PATCH 1/1] PCI: Account fully optional bridge windows correctly

[Bjorn Helgaas]

On Thu, Feb 19, 2026 at 12:34:18AM +0200, Ilpo Järvinen wrote:
> pbus_size_mem_optional() adds dev_res->add_size of a bridge window into
> children_add_size when the window has a non-optional part. However, if
> the bridge window is fully optional, only r_size is added (which is
> zero for such a window).

I guess this means we might not make a bridge window big enough for
optional resources, even if space is available?

> Also, a second dev_res entry will be added by pci_dev_res_add_to_list()
> into realloc_head for the bridge window (resulting in triggering the
> realloc_head-must-be-fully-consumed sanity check after a single pass of
> the resource assignment algorithm):
> 
>   WARNING: drivers/pci/setup-bus.c:2153 at pci_assign_unassigned_root_bus_resources+0xa5/0x260
> 
> Correct these problems by always adding dev_res->add_size for bridge
> windows and not calling pci_dev_res_add_to_list() if the dev_res entry
> exists.
> 
> Fixes: 6a5e64c75e82 ("PCI: Add pbus_mem_size_optional() to handle optional sizes")

We merged 6a5e64c75e82 for v7.0, so it looks like this fix is also
v7.0 material, right?

> Reported-by: RavitejaX Veesam <ravitejax.veesam@intel.com>

Is there a public report of this?  I didn't see anything on lore that
looked relevant.

> Tested-by: RavitejaX Veesam <ravitejax.veesam@intel.com>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---
>  drivers/pci/setup-bus.c | 29 ++++++++++++++++-------------
>  1 file changed, 16 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index 32aa72456a44..0c8cbbfaf8d7 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1217,31 +1217,34 @@ static bool pbus_size_mem_optional(struct pci_dev *dev, int resno,
>  	struct resource *res = pci_resource_n(dev, resno);
>  	bool optional = pci_resource_is_optional(dev, resno);
>  	resource_size_t r_size = resource_size(res);
> -	struct pci_dev_resource *dev_res;
> +	struct pci_dev_resource *dev_res = NULL;
>  
>  	if (!realloc_head)
>  		return false;
>  
> -	if (!optional) {
> -		/*
> -		 * Only bridges have optional sizes in realloc_head at this
> -		 * point. As res_to_dev_res() walks the entire realloc_head
> -		 * list, skip calling it when known unnecessary.
> -		 */
> -		if (!pci_resource_is_bridge_win(resno))
> -			return false;
> -
> +	/*
> +	 * Only bridges have optional sizes in realloc_head at this
> +	 * point. As res_to_dev_res() walks the entire realloc_head
> +	 * list, skip calling it when known unnecessary.
> +	 */
> +	if (pci_resource_is_bridge_win(resno)) {
>  		dev_res = res_to_dev_res(realloc_head, res);
>  		if (dev_res) {
>  			*children_add_size += dev_res->add_size;
>  			*add_align = max(*add_align, dev_res->min_align);
>  		}
> +	}
>  
> +	if (!optional)
>  		return false;
> -	}
>  
> -	/* Put SRIOV requested res to the optional list */
> -	pci_dev_res_add_to_list(realloc_head, dev, res, 0, align);
> +	/*
> +	 * Put requested res to the optional list if not there yet (SRIOV,
> +	 * disabled ROM). Bridge windows with an optional part are already
> +	 * on the list.
> +	 */
> +	if (!dev_res)
> +		pci_dev_res_add_to_list(realloc_head, dev, res, 0, align);
>  	*children_add_size += r_size;
>  	*add_align = max(align, *add_align);
>  
> 
> base-commit: 1c2b4a4c2bcb950f182eeeb33d94b565607608cf
> -- 
> 2.39.5
> 

Re: [PATCH 1/1] PCI: Account fully optional bridge windows correctly

[Ilpo Järvinen]

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

On Thu, 19 Feb 2026, Bjorn Helgaas wrote:

> On Thu, Feb 19, 2026 at 12:34:18AM +0200, Ilpo Järvinen wrote:
> > pbus_size_mem_optional() adds dev_res->add_size of a bridge window into
> > children_add_size when the window has a non-optional part. However, if
> > the bridge window is fully optional, only r_size is added (which is
> > zero for such a window).
> 
> I guess this means we might not make a bridge window big enough for
> optional resources, even if space is available?

Yes, the size calculations are off for the upstream windows.

> > Also, a second dev_res entry will be added by pci_dev_res_add_to_list()
> > into realloc_head for the bridge window (resulting in triggering the
> > realloc_head-must-be-fully-consumed sanity check after a single pass of
> > the resource assignment algorithm):
> > 
> >   WARNING: drivers/pci/setup-bus.c:2153 at pci_assign_unassigned_root_bus_resources+0xa5/0x260
> > 
> > Correct these problems by always adding dev_res->add_size for bridge
> > windows and not calling pci_dev_res_add_to_list() if the dev_res entry
> > exists.
> > 
> > Fixes: 6a5e64c75e82 ("PCI: Add pbus_mem_size_optional() to handle optional sizes")
> 
> We merged 6a5e64c75e82 for v7.0, so it looks like this fix is also
> v7.0 material, right?

Preferrably yes.

> > Reported-by: RavitejaX Veesam <ravitejax.veesam@intel.com>
> 
> Is there a public report of this?  I didn't see anything on lore that
> looked relevant.

I got an internal report a CI failure from gfx people. I couldn't 
immediately find anything from gitlab.freedesktop.org (not a guarantee 
there wouldn't be anything there).

-- 
 i.

> > Tested-by: RavitejaX Veesam <ravitejax.veesam@intel.com>
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> >  drivers/pci/setup-bus.c | 29 ++++++++++++++++-------------
> >  1 file changed, 16 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> > index 32aa72456a44..0c8cbbfaf8d7 100644
> > --- a/drivers/pci/setup-bus.c
> > +++ b/drivers/pci/setup-bus.c
> > @@ -1217,31 +1217,34 @@ static bool pbus_size_mem_optional(struct pci_dev *dev, int resno,
> >  	struct resource *res = pci_resource_n(dev, resno);
> >  	bool optional = pci_resource_is_optional(dev, resno);
> >  	resource_size_t r_size = resource_size(res);
> > -	struct pci_dev_resource *dev_res;
> > +	struct pci_dev_resource *dev_res = NULL;
> >  
> >  	if (!realloc_head)
> >  		return false;
> >  
> > -	if (!optional) {
> > -		/*
> > -		 * Only bridges have optional sizes in realloc_head at this
> > -		 * point. As res_to_dev_res() walks the entire realloc_head
> > -		 * list, skip calling it when known unnecessary.
> > -		 */
> > -		if (!pci_resource_is_bridge_win(resno))
> > -			return false;
> > -
> > +	/*
> > +	 * Only bridges have optional sizes in realloc_head at this
> > +	 * point. As res_to_dev_res() walks the entire realloc_head
> > +	 * list, skip calling it when known unnecessary.
> > +	 */
> > +	if (pci_resource_is_bridge_win(resno)) {
> >  		dev_res = res_to_dev_res(realloc_head, res);
> >  		if (dev_res) {
> >  			*children_add_size += dev_res->add_size;
> >  			*add_align = max(*add_align, dev_res->min_align);
> >  		}
> > +	}
> >  
> > +	if (!optional)
> >  		return false;
> > -	}
> >  
> > -	/* Put SRIOV requested res to the optional list */
> > -	pci_dev_res_add_to_list(realloc_head, dev, res, 0, align);
> > +	/*
> > +	 * Put requested res to the optional list if not there yet (SRIOV,
> > +	 * disabled ROM). Bridge windows with an optional part are already
> > +	 * on the list.
> > +	 */
> > +	if (!dev_res)
> > +		pci_dev_res_add_to_list(realloc_head, dev, res, 0, align);
> >  	*children_add_size += r_size;
> >  	*add_align = max(align, *add_align);
> >  
> > 
> > base-commit: 1c2b4a4c2bcb950f182eeeb33d94b565607608cf
> > -- 
> > 2.39.5
> > 
>