| Message ID | SL2P216MB01879766498AA7746C2E5FB780C00@SL2P216MB0187.KORP216.PROD.OUTLOOK.COM |
|---|---|
| State | Superseded |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
| Series | Patch series to support Thunderbolt without any BIOS support | expand |
On Fri, Jul 26, 2019 at 12:54:22PM +0000, Nicholas Johnson wrote: > Remove checks for resource size in extend_bridge_window(). This is > necessary to allow the pci_bus_distribute_available_resources() to > function when the kernel parameter pci=hpmemsize=nn[KMG] is used to > allocate resources. Because the kernel parameter sets the size of all > hotplug bridges to be the same, there are problems when nested hotplug > bridges are encountered. Fitting a downstream hotplug bridge with size X > and normal bridges with size Y into parent hotplug bridge with size X is > impossible, and hence the downstream hotplug bridge needs to shrink to > fit into its parent. Maybe you could show the topology here which needs shrinking. > Add check for if bridge is extended or shrunken and adjust pci_dbg to > reflect this. > > Reset the resource if its new size is zero (if we have run out of a > bridge window resource). If it is set to zero size and left, it can > cause significant problems when it comes to enabling devices. Same comment here about explaining the "significant problems". > > Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au> > --- > drivers/pci/setup-bus.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > index a072781ab..7e1dc892a 100644 > --- a/drivers/pci/setup-bus.c > +++ b/drivers/pci/setup-bus.c > @@ -1823,13 +1823,19 @@ static void extend_bridge_window(struct pci_dev *bridge, struct resource *res, Since it is also shrinking now maybe name it adjust_bridge_window() instead?
On Tue, Oct 08, 2019 at 03:09:07PM +0300, mika.westerberg@linux.intel.com wrote: > On Fri, Jul 26, 2019 at 12:54:22PM +0000, Nicholas Johnson wrote: > > Remove checks for resource size in extend_bridge_window(). This is > > necessary to allow the pci_bus_distribute_available_resources() to > > function when the kernel parameter pci=hpmemsize=nn[KMG] is used to > > allocate resources. Because the kernel parameter sets the size of all > > hotplug bridges to be the same, there are problems when nested hotplug > > bridges are encountered. Fitting a downstream hotplug bridge with size X > > and normal bridges with size Y into parent hotplug bridge with size X is > > impossible, and hence the downstream hotplug bridge needs to shrink to > > fit into its parent. > > Maybe you could show the topology here which needs shrinking. > > > Add check for if bridge is extended or shrunken and adjust pci_dbg to > > reflect this. > > > > Reset the resource if its new size is zero (if we have run out of a > > bridge window resource). If it is set to zero size and left, it can > > cause significant problems when it comes to enabling devices. > > Same comment here about explaining the "significant problems". I have in the past, but because the problems are very hard to describe succinctly, it just turns into a nightmare. I can try to do it again. > > > > Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au> > > --- > > drivers/pci/setup-bus.c | 16 +++++++++++----- > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > > index a072781ab..7e1dc892a 100644 > > --- a/drivers/pci/setup-bus.c > > +++ b/drivers/pci/setup-bus.c > > @@ -1823,13 +1823,19 @@ static void extend_bridge_window(struct pci_dev *bridge, struct resource *res, > > Since it is also shrinking now maybe name it adjust_bridge_window() instead? I am happy to do this. If we can drop the pci_dbg() calls, then I might be able to drop this function entirely. During the development of this patch, that is exactly what I did. How important are the pci_dbg calls to you? Another option is to simply print something with pci_dbg that simply says the bridge size has been set to maximum possible while still fitting in parent. That will remove the need for logic to detect if it shrunk or extended.
On Wed, Oct 23, 2019 at 09:16:10AM +0000, Nicholas Johnson wrote: > On Tue, Oct 08, 2019 at 03:09:07PM +0300, mika.westerberg@linux.intel.com wrote: > > On Fri, Jul 26, 2019 at 12:54:22PM +0000, Nicholas Johnson wrote: > > > Remove checks for resource size in extend_bridge_window(). This is > > > necessary to allow the pci_bus_distribute_available_resources() to > > > function when the kernel parameter pci=hpmemsize=nn[KMG] is used to > > > allocate resources. Because the kernel parameter sets the size of all > > > hotplug bridges to be the same, there are problems when nested hotplug > > > bridges are encountered. Fitting a downstream hotplug bridge with size X > > > and normal bridges with size Y into parent hotplug bridge with size X is > > > impossible, and hence the downstream hotplug bridge needs to shrink to > > > fit into its parent. > > > > Maybe you could show the topology here which needs shrinking. > > > > > Add check for if bridge is extended or shrunken and adjust pci_dbg to > > > reflect this. > > > > > > Reset the resource if its new size is zero (if we have run out of a > > > bridge window resource). If it is set to zero size and left, it can > > > cause significant problems when it comes to enabling devices. > > > > Same comment here about explaining the "significant problems". > I have in the past, but because the problems are very hard to describe succinctly, it just turns into a > nightmare. I can try to do it again. > > > > > > > Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au> > > > --- > > > drivers/pci/setup-bus.c | 16 +++++++++++----- > > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > > > diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c > > > index a072781ab..7e1dc892a 100644 > > > --- a/drivers/pci/setup-bus.c > > > +++ b/drivers/pci/setup-bus.c > > > @@ -1823,13 +1823,19 @@ static void extend_bridge_window(struct pci_dev *bridge, struct resource *res, > > > > Since it is also shrinking now maybe name it adjust_bridge_window() instead? > I am happy to do this. > > If we can drop the pci_dbg() calls, then I might be able to drop this > function entirely. During the development of this patch, that is exactly > what I did. How important are the pci_dbg calls to you? Well they are still useful when debugging resource allocation issues (and they match similar we do when extending number of buses). I would like to keep them if possible.
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c index a072781ab..7e1dc892a 100644 --- a/drivers/pci/setup-bus.c +++ b/drivers/pci/setup-bus.c @@ -1823,13 +1823,19 @@ static void extend_bridge_window(struct pci_dev *bridge, struct resource *res, if (res->parent) return; - if (resource_size(res) >= new_size) - return; - - add_size = new_size - resource_size(res); - pci_dbg(bridge, "bridge window %pR extended by %pa\n", res, &add_size); + if (new_size > resource_size(res)) { + add_size = new_size - resource_size(res); + pci_dbg(bridge, "bridge window %pR extended by %pa\n", res, + &add_size); + } else if (new_size < resource_size(res)) { + add_size = resource_size(res) - new_size; + pci_dbg(bridge, "bridge window %pR shrunken by %pa\n", res, + &add_size); + } res->end = res->start + new_size - 1; remove_from_list(add_list, res); + if (!new_size) + reset_resource(res); } static void pci_bus_distribute_available_resources(struct pci_bus *bus,
Remove checks for resource size in extend_bridge_window(). This is necessary to allow the pci_bus_distribute_available_resources() to function when the kernel parameter pci=hpmemsize=nn[KMG] is used to allocate resources. Because the kernel parameter sets the size of all hotplug bridges to be the same, there are problems when nested hotplug bridges are encountered. Fitting a downstream hotplug bridge with size X and normal bridges with size Y into parent hotplug bridge with size X is impossible, and hence the downstream hotplug bridge needs to shrink to fit into its parent. Add check for if bridge is extended or shrunken and adjust pci_dbg to reflect this. Reset the resource if its new size is zero (if we have run out of a bridge window resource). If it is set to zero size and left, it can cause significant problems when it comes to enabling devices. Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au> --- drivers/pci/setup-bus.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)