Linux PCI subsystem development
 help / color / mirror / Atom feed
* [PATCH for-linus 1/1] PCI: Revert early bridge resource set up
@ 2025-10-14 16:36 Ilpo Järvinen
  2025-10-14 20:39 ` Bjorn Helgaas
  0 siblings, 1 reply; 2+ messages in thread
From: Ilpo Järvinen @ 2025-10-14 16:36 UTC (permalink / raw)
  To: Bjorn Helgaas, Ilpo Järvinen, linux-pci, linux-kernel
  Cc: Val Packett, Guenter Roeck

The commit a43ac325c7cb ("PCI: Set up bridge resources earlier") moved
bridge window resources set up earlier than before. The change was
necessary to support another change that got pulled on the last minute
due to breaking s390 and other systems.

The presence of valid bridge window resources earlier than before
allows pci_assign_unassigned_root_bus_resources() call from
pci_host_probe() assign the bridge windows. Some host bridges, however,
have to wait first for the link up event before they can enumerate
successfully (see e.g. qcom_pcie_global_irq_thread()) and thus the bus
has not been enumerated yet while calling pci_host_probe().

Calling pci_assign_unassigned_root_bus_resources() without results from
enumeration can result in sizing bridge windows with too small sizes
which cannot be later corrected after the enumeration has completed
because bridge windows have become pinned in place by the other
resources.

Interestingly, it seems pci_read_bridge_bases() is not called at all in
the problematic case and the bridge window resource type setup is done
by pci_bridge_check_ranges() and sizing by the usual resource fitting
logic.

The root problem behind all this looks pretty generic. If resource
fitting is called too early, the hotplug reservation and old size lower
bounding cause the bridge windows to be assigned without children but
with non-zero size, which leads to these pinning problems. As such,
this can likely be solved on the general level but the solution does
not look trivial.

As the commit a43ac325c7cb ("PCI: Set up bridge resources earlier") was
prequisite for other change that did not end up into kernel yet, revert
it to resolve the resource assignment failures and give time to code
and test a generic solution.

Reported-by: Val Packett <val@packett.cool>
Reported-by: Guenter Roeck <linux@roeck-us.net>
Fixes: a43ac325c7cb ("PCI: Set up bridge resources earlier")
Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---

This revert should go to for-linus.


I'm not sure whether Guenter's case is exactly the same problem as
described in the commit message, I only know for sure his bisection
landed on the same commit.

My plan is to retry these changes with more supporting changes. It
looks PCI core could delay assigning the bridge window resources if
there are no child resource to put into the bridge windows. Or
alternatively the resource fitting algorithm could release empty bridge
windows as the first step. But that is too complicated change to make
now and would benefit from time spent in -next.

---
 drivers/pci/probe.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index c83e75a0ec12..0ce98e18b5a8 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -538,14 +538,10 @@ static void pci_read_bridge_windows(struct pci_dev *bridge)
 	}
 	if (io) {
 		bridge->io_window = 1;
-		pci_read_bridge_io(bridge,
-				   pci_resource_n(bridge, PCI_BRIDGE_IO_WINDOW),
-				   true);
+		pci_read_bridge_io(bridge, &res, true);
 	}
 
-	pci_read_bridge_mmio(bridge,
-			     pci_resource_n(bridge, PCI_BRIDGE_MEM_WINDOW),
-			     true);
+	pci_read_bridge_mmio(bridge, &res, true);
 
 	/*
 	 * DECchip 21050 pass 2 errata: the bridge may miss an address
@@ -583,10 +579,7 @@ static void pci_read_bridge_windows(struct pci_dev *bridge)
 			bridge->pref_64_window = 1;
 	}
 
-	pci_read_bridge_mmio_pref(bridge,
-				  pci_resource_n(bridge,
-						 PCI_BRIDGE_PREF_MEM_WINDOW),
-				  true);
+	pci_read_bridge_mmio_pref(bridge, &res, true);
 }
 
 void pci_read_bridge_bases(struct pci_bus *child)

base-commit: 2f2c7254931f41b5736e3ba12aaa9ac1bbeeeb92
-- 
2.39.5


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH for-linus 1/1] PCI: Revert early bridge resource set up
  2025-10-14 16:36 [PATCH for-linus 1/1] PCI: Revert early bridge resource set up Ilpo Järvinen
@ 2025-10-14 20:39 ` Bjorn Helgaas
  0 siblings, 0 replies; 2+ messages in thread
From: Bjorn Helgaas @ 2025-10-14 20:39 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Bjorn Helgaas, linux-pci, linux-kernel, Val Packett,
	Guenter Roeck

On Tue, Oct 14, 2025 at 07:36:02PM +0300, Ilpo Järvinen wrote:
> The commit a43ac325c7cb ("PCI: Set up bridge resources earlier") moved
> bridge window resources set up earlier than before. The change was
> necessary to support another change that got pulled on the last minute
> due to breaking s390 and other systems.
> 
> The presence of valid bridge window resources earlier than before
> allows pci_assign_unassigned_root_bus_resources() call from
> pci_host_probe() assign the bridge windows. Some host bridges, however,
> have to wait first for the link up event before they can enumerate
> successfully (see e.g. qcom_pcie_global_irq_thread()) and thus the bus
> has not been enumerated yet while calling pci_host_probe().
> 
> Calling pci_assign_unassigned_root_bus_resources() without results from
> enumeration can result in sizing bridge windows with too small sizes
> which cannot be later corrected after the enumeration has completed
> because bridge windows have become pinned in place by the other
> resources.
> 
> Interestingly, it seems pci_read_bridge_bases() is not called at all in
> the problematic case and the bridge window resource type setup is done
> by pci_bridge_check_ranges() and sizing by the usual resource fitting
> logic.
> 
> The root problem behind all this looks pretty generic. If resource
> fitting is called too early, the hotplug reservation and old size lower
> bounding cause the bridge windows to be assigned without children but
> with non-zero size, which leads to these pinning problems. As such,
> this can likely be solved on the general level but the solution does
> not look trivial.
> 
> As the commit a43ac325c7cb ("PCI: Set up bridge resources earlier") was
> prequisite for other change that did not end up into kernel yet, revert
> it to resolve the resource assignment failures and give time to code
> and test a generic solution.
> 
> Reported-by: Val Packett <val@packett.cool>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Fixes: a43ac325c7cb ("PCI: Set up bridge resources earlier")
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

Applied to pci/for-linus for v6.18, thanks!

> ---
> 
> This revert should go to for-linus.
> 
> 
> I'm not sure whether Guenter's case is exactly the same problem as
> described in the commit message, I only know for sure his bisection
> landed on the same commit.
> 
> My plan is to retry these changes with more supporting changes. It
> looks PCI core could delay assigning the bridge window resources if
> there are no child resource to put into the bridge windows. Or
> alternatively the resource fitting algorithm could release empty bridge
> windows as the first step. But that is too complicated change to make
> now and would benefit from time spent in -next.
> 
> ---
>  drivers/pci/probe.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index c83e75a0ec12..0ce98e18b5a8 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -538,14 +538,10 @@ static void pci_read_bridge_windows(struct pci_dev *bridge)
>  	}
>  	if (io) {
>  		bridge->io_window = 1;
> -		pci_read_bridge_io(bridge,
> -				   pci_resource_n(bridge, PCI_BRIDGE_IO_WINDOW),
> -				   true);
> +		pci_read_bridge_io(bridge, &res, true);
>  	}
>  
> -	pci_read_bridge_mmio(bridge,
> -			     pci_resource_n(bridge, PCI_BRIDGE_MEM_WINDOW),
> -			     true);
> +	pci_read_bridge_mmio(bridge, &res, true);
>  
>  	/*
>  	 * DECchip 21050 pass 2 errata: the bridge may miss an address
> @@ -583,10 +579,7 @@ static void pci_read_bridge_windows(struct pci_dev *bridge)
>  			bridge->pref_64_window = 1;
>  	}
>  
> -	pci_read_bridge_mmio_pref(bridge,
> -				  pci_resource_n(bridge,
> -						 PCI_BRIDGE_PREF_MEM_WINDOW),
> -				  true);
> +	pci_read_bridge_mmio_pref(bridge, &res, true);
>  }
>  
>  void pci_read_bridge_bases(struct pci_bus *child)
> 
> base-commit: 2f2c7254931f41b5736e3ba12aaa9ac1bbeeeb92
> -- 
> 2.39.5
> 

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-10-14 20:39 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-14 16:36 [PATCH for-linus 1/1] PCI: Revert early bridge resource set up Ilpo Järvinen
2025-10-14 20:39 ` Bjorn Helgaas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox