linux-pci.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 1/5] PCI: Consider alignment of hot-added bridges when distributing resources
       [not found] <20190505144001.8106-1-nicholas.johnson-opensource@outlook.com.au>
@ 2019-05-05 14:40 ` Nicholas Johnson
  2019-05-05 14:40 ` [PATCH v5 2/5] PCI: Modify extend_bridge_window() to set resource size directly Nicholas Johnson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 5+ messages in thread
From: Nicholas Johnson @ 2019-05-05 14:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pci, bhelgaas, mika.westerberg, corbet, Nicholas Johnson

This patch solves the following bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=199581

An excerpt from the bug report:

==========================================================================

Here is what happens when an Intel Gigabit ET2 quad port server adapter
is hot-added:

  pci 0000:39:00.0: BAR 14: assigned [mem 0x53300000-0x6a0fffff]
                                          ^^^^^^^^^^
  pci 0000:3a:01.0: BAR 14: assigned [mem 0x53400000-0x547fffff]
                                          ^^^^^^^^^^
The above shows that the downstream bridge (3a:01.0) window is aligned
to 2 MB instead of 1 MB as is the upstream bridge (39:00.0) window. The
remaining MMIO space (0x15a00000) is assigned to the hotplug bridge
(3a:04.0) but it fails:

  pci 0000:3a:04.0: BAR 14: no space for [mem size 0x15a00000]
  pci 0000:3a:04.0: BAR 14: failed to assign [mem size 0x15a00000]

==========================================================================

Rewrite pci_bus_distribute_available_resources() to better handle
bridges with different resource alignment requirements. Pass more
details arguments recursively to track the resource start and end
addresses relative to the initial hotplug bridge. This is especially
useful for Thunderbolt with native PCI enumeration, enabling external
graphics cards and other devices with bridge alignment higher than
0x100000 bytes.

Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
---
 drivers/pci/setup-bus.c | 164 ++++++++++++++++++++--------------------
 1 file changed, 83 insertions(+), 81 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index ec44a0f3a..dae4bae12 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1839,10 +1839,10 @@ static void extend_bridge_window(struct pci_dev *bridge, struct resource *res,
 }
 
 static void pci_bus_distribute_available_resources(struct pci_bus *bus,
-	struct list_head *add_list, resource_size_t available_io,
-	resource_size_t available_mmio, resource_size_t available_mmio_pref)
+	struct list_head *add_list, struct resource io,
+	struct resource mmio, struct resource mmio_pref)
 {
-	resource_size_t remaining_io, remaining_mmio, remaining_mmio_pref;
+	resource_size_t io_per_hp, mmio_per_hp, mmio_pref_per_hp, align;
 	unsigned int normal_bridges = 0, hotplug_bridges = 0;
 	struct resource *io_res, *mmio_res, *mmio_pref_res;
 	struct pci_dev *dev, *bridge = bus->self;
@@ -1852,25 +1852,32 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 	mmio_pref_res = &bridge->resource[PCI_BRIDGE_RESOURCES + 2];
 
 	/*
-	 * Update additional resource list (add_list) to fill all the
-	 * extra resource space available for this port except the space
-	 * calculated in __pci_bus_size_bridges() which covers all the
-	 * devices currently connected to the port and below.
+	 * The alignment of this bridge is yet to be considered, hence it must
+	 * be done now before extending its bridge window. A single bridge
+	 * might not be able to occupy the whole parent region if the alignment
+	 * differs - for example, an external GPU at the end of a Thunderbolt
+	 * daisy chain.
 	 */
-	extend_bridge_window(bridge, io_res, add_list, available_io);
-	extend_bridge_window(bridge, mmio_res, add_list, available_mmio);
-	extend_bridge_window(bridge, mmio_pref_res, add_list,
-			     available_mmio_pref);
+	align = pci_resource_alignment(bridge, io_res);
+	if (!io_res->parent && align)
+		io.start = ALIGN(io.start, align);
+
+	align = pci_resource_alignment(bridge, mmio_res);
+	if (!mmio_res->parent && align)
+		mmio.start = ALIGN(mmio.start, align);
+
+	align = pci_resource_alignment(bridge, mmio_pref_res);
+	if (!mmio_pref_res->parent && align)
+		mmio_pref.start = ALIGN(mmio_pref.start, align);
 
 	/*
-	 * Calculate the total amount of extra resource space we can
-	 * pass to bridges below this one. This is basically the
-	 * extra space reduced by the minimal required space for the
-	 * non-hotplug bridges.
+	 * Update the resources to fill as much remaining resource space in the
+	 * parent bridge as possible, while considering alignment.
 	 */
-	remaining_io = available_io;
-	remaining_mmio = available_mmio;
-	remaining_mmio_pref = available_mmio_pref;
+	extend_bridge_window(bridge, io_res, add_list, resource_size(&io));
+	extend_bridge_window(bridge, mmio_res, add_list, resource_size(&mmio));
+	extend_bridge_window(bridge, mmio_pref_res, add_list,
+		resource_size(&mmio_pref));
 
 	/*
 	 * Calculate how many hotplug bridges and normal bridges there
@@ -1884,80 +1891,79 @@ static void pci_bus_distribute_available_resources(struct pci_bus *bus,
 			normal_bridges++;
 	}
 
+	/*
+	 * There is only one bridge on the bus so it gets all possible
+	 * resources which it can then distribute to the possible
+	 * hotplug bridges below.
+	 */
+	if (hotplug_bridges + normal_bridges == 1) {
+		dev = list_first_entry(&bus->devices, struct pci_dev, bus_list);
+		if (dev->subordinate)
+			pci_bus_distribute_available_resources(dev->subordinate,
+				add_list, io, mmio, mmio_pref);
+		return;
+	}
+
+	/*
+	 * Reduce the available resource space by what the
+	 * bridge and devices below it occupy.
+	 */
 	for_each_pci_bridge(dev, bus) {
-		const struct resource *res;
+		struct resource *res;
+		resource_size_t used_size;
 
 		if (dev->is_hotplug_bridge)
 			continue;
 
-		/*
-		 * Reduce the available resource space by what the
-		 * bridge and devices below it occupy.
-		 */
 		res = &dev->resource[PCI_BRIDGE_RESOURCES + 0];
-		if (!res->parent && available_io > resource_size(res))
-			remaining_io -= resource_size(res);
+		align = pci_resource_alignment(dev, res);
+		align = align ? ALIGN(io.start, align) - io.start : 0;
+		used_size = align + resource_size(res);
+		if (!res->parent && used_size <= resource_size(&io))
+			io.start += used_size;
 
 		res = &dev->resource[PCI_BRIDGE_RESOURCES + 1];
-		if (!res->parent && available_mmio > resource_size(res))
-			remaining_mmio -= resource_size(res);
+		align = pci_resource_alignment(dev, res);
+		align = align ? ALIGN(mmio.start, align) - mmio.start : 0;
+		used_size = align + resource_size(res);
+		if (!res->parent && used_size <= resource_size(&mmio))
+			mmio.start += used_size;
 
 		res = &dev->resource[PCI_BRIDGE_RESOURCES + 2];
-		if (!res->parent && available_mmio_pref > resource_size(res))
-			remaining_mmio_pref -= resource_size(res);
+		align = pci_resource_alignment(dev, res);
+		align = align ? ALIGN(mmio_pref.start, align) -
+				mmio_pref.start : 0;
+		used_size = align + resource_size(res);
+		if (!res->parent && used_size <= resource_size(&mmio_pref))
+			mmio_pref.start += used_size;
 	}
 
-	/*
-	 * There is only one bridge on the bus so it gets all available
-	 * resources which it can then distribute to the possible
-	 * hotplug bridges below.
-	 */
-	if (hotplug_bridges + normal_bridges == 1) {
-		dev = list_first_entry(&bus->devices, struct pci_dev, bus_list);
-		if (dev->subordinate) {
-			pci_bus_distribute_available_resources(dev->subordinate,
-				add_list, available_io, available_mmio,
-				available_mmio_pref);
-		}
+	if (!hotplug_bridges)
 		return;
-	}
 
 	/*
-	 * Go over devices on this bus and distribute the remaining
-	 * resource space between hotplug bridges.
+	 * Distribute any remaining resources equally between
+	 * the hotplug-capable downstream ports.
 	 */
-	for_each_pci_bridge(dev, bus) {
-		resource_size_t align, io, mmio, mmio_pref;
-		struct pci_bus *b;
+	io_per_hp = div64_ul(resource_size(&io), hotplug_bridges);
+	mmio_per_hp = div64_ul(resource_size(&mmio), hotplug_bridges);
+	mmio_pref_per_hp = div64_ul(resource_size(&mmio_pref),
+		hotplug_bridges);
 
-		b = dev->subordinate;
-		if (!b || !dev->is_hotplug_bridge)
+	for_each_pci_bridge(dev, bus) {
+		if (!dev->subordinate || !dev->is_hotplug_bridge)
 			continue;
 
-		/*
-		 * Distribute available extra resources equally between
-		 * hotplug-capable downstream ports taking alignment into
-		 * account.
-		 *
-		 * Here hotplug_bridges is always != 0.
-		 */
-		align = pci_resource_alignment(bridge, io_res);
-		io = div64_ul(available_io, hotplug_bridges);
-		io = min(ALIGN(io, align), remaining_io);
-		remaining_io -= io;
-
-		align = pci_resource_alignment(bridge, mmio_res);
-		mmio = div64_ul(available_mmio, hotplug_bridges);
-		mmio = min(ALIGN(mmio, align), remaining_mmio);
-		remaining_mmio -= mmio;
+		io.end = io.start + io_per_hp - 1;
+		mmio.end = mmio.start + mmio_per_hp - 1;
+		mmio_pref.end = mmio_pref.start + mmio_pref_per_hp - 1;
 
-		align = pci_resource_alignment(bridge, mmio_pref_res);
-		mmio_pref = div64_ul(available_mmio_pref, hotplug_bridges);
-		mmio_pref = min(ALIGN(mmio_pref, align), remaining_mmio_pref);
-		remaining_mmio_pref -= mmio_pref;
+		pci_bus_distribute_available_resources(dev->subordinate,
+			add_list, io, mmio, mmio_pref);
 
-		pci_bus_distribute_available_resources(b, add_list, io, mmio,
-						       mmio_pref);
+		io.start = io.end + 1;
+		mmio.start = mmio.end + 1;
+		mmio_pref.start = mmio_pref.end + 1;
 	}
 }
 
@@ -1965,22 +1971,18 @@ static void
 pci_bridge_distribute_available_resources(struct pci_dev *bridge,
 					  struct list_head *add_list)
 {
-	resource_size_t available_io, available_mmio, available_mmio_pref;
-	const struct resource *res;
+	struct resource io_res, mmio_res, mmio_pref_res;
 
 	if (!bridge->is_hotplug_bridge)
 		return;
 
-	/* Take the initial extra resources from the hotplug port */
-	res = &bridge->resource[PCI_BRIDGE_RESOURCES + 0];
-	available_io = resource_size(res);
-	res = &bridge->resource[PCI_BRIDGE_RESOURCES + 1];
-	available_mmio = resource_size(res);
-	res = &bridge->resource[PCI_BRIDGE_RESOURCES + 2];
-	available_mmio_pref = resource_size(res);
+	io_res = bridge->resource[PCI_BRIDGE_RESOURCES + 0];
+	mmio_res = bridge->resource[PCI_BRIDGE_RESOURCES + 1];
+	mmio_pref_res = bridge->resource[PCI_BRIDGE_RESOURCES + 2];
 
+	/* Take the initial extra resources from the hotplug port */
 	pci_bus_distribute_available_resources(bridge->subordinate,
-		add_list, available_io, available_mmio, available_mmio_pref);
+		add_list, io_res, mmio_res, mmio_pref_res);
 }
 
 void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
-- 
2.19.1


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

* [PATCH v5 2/5] PCI: Modify extend_bridge_window() to set resource size directly
       [not found] <20190505144001.8106-1-nicholas.johnson-opensource@outlook.com.au>
  2019-05-05 14:40 ` [PATCH v5 1/5] PCI: Consider alignment of hot-added bridges when distributing resources Nicholas Johnson
@ 2019-05-05 14:40 ` Nicholas Johnson
  2019-05-05 14:40 ` [PATCH v5 3/5] PCI: Fix bug resulting in double hpmemsize being assigned to MMIO window Nicholas Johnson
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 5+ messages in thread
From: Nicholas Johnson @ 2019-05-05 14:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pci, bhelgaas, mika.westerberg, corbet, Nicholas Johnson

Background
==========================================================================

In the current state, the PCI allocation could fail with Thunderbolt
under certain unusual circumstances, because add_list resources are
"optional". Guaranteed allocation requires guaranteed resource sizes.

It is difficult to give examples of these failures - because without the
previous patch in the series, the symptoms of the problem are hidden by
larger problems. This patch has been split from the previous patch and
makes little sense on its own - as it is almost impossible to see the
effect of this patch without first fixing the problems addressed by the
previous patch. So the evidence I put forward for making this change is
that because add_list resources are "optional", there could be any
number of unforeseen bugs that are yet to be encountered if the kernel
decides not to assign all of the optional size. In kernel development,
we should not play around with chance.

Moving away from add_size also allows for use of pci=hpmemsize to assign
resources. Previously, when using add_size and not allowing the add_size
to shrink, it made it impossible to distribute resources. If a hotplug
bridge has size X, and below it is some devices with non-zero size Y and
a nested hotplug bridge of same size X, fitting X+Y into size X is
mathematically impossible.

This patch solves this by dropping add_size and giving each bridge the
maximum size possible without failing resource assignment. Using
pci=hpmemsize still works as pci_assign_unassigned_root_bus_resources()
does not call pci_bus_distribute_available_resources(). At boot,
pci_assign_unassigned_root_bus_resources() is used, instead of
pci_bridge_distribute_available_resources().

By allowing to use pci=hpmemsize, it removes the reliance on the
firmware to declare the window resources under the root port, and could
pay off in the future with USB4 (which is backward-compatible to
Thunderbolt devices, and not specific to Intel systems). Users of
Thunderbolt hardware on unsupported systems will be able to specify the
resources in the kernel parameters. Users of official systems will be
able to override the default firmware window sizes to allocate much
larger resource sizes, potentially enabling Thunderbolt support for
devices with massive BARs (with a few other problems solved by later
patches in this series).

Patch notes
==========================================================================

Modify extend_bridge_window() to remove the resource from add_list and
change the resource size directly.

Modify extend_bridge_window() to reset resources that are being assigned
zero size. This is required to prevent the bridge not being enabled due
to resources with zero size. This is a direct requirement to prevent the
change away from using add_list from introducing a regression - because
before, it was not possible to end up with zero size.

Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
---
 drivers/pci/setup-bus.c | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index dae4bae12..5214815c7 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1815,27 +1815,40 @@ void __init pci_assign_unassigned_resources(void)
 }
 
 static void extend_bridge_window(struct pci_dev *bridge, struct resource *res,
-			struct list_head *add_list, resource_size_t available)
+			struct list_head *add_list, resource_size_t new_size)
 {
-	struct pci_dev_resource *dev_res;
+	resource_size_t add_size;
 
 	if (res->parent)
 		return;
 
-	if (resource_size(res) >= available)
-		return;
-
-	dev_res = res_to_dev_res(add_list, res);
-	if (!dev_res)
-		return;
+	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 {
+		add_size = resource_size(res) - new_size;
+		pci_dbg(bridge, "bridge window %pR shrunken by %pa\n", res,
+			&add_size);
+	}
 
-	/* Is there room to extend the window? */
-	if (available - resource_size(res) <= dev_res->add_size)
-		return;
+	/*
+	 * Resources requested using add_size in additional resource lists are
+	 * considered optional when allocated. Guaranteed size of allocation
+	 * is required to guarantee successful resource distribution. Hence,
+	 * the size of the actual resource must be adjusted, and the resource
+	 * removed from add_list to prevent any additional size interfering.
+	 */
+	res->end = res->start + new_size - 1;
+	remove_from_list(add_list, res);
 
-	dev_res->add_size = available - resource_size(res);
-	pci_dbg(bridge, "bridge window %pR extended by %pa\n", res,
-		&dev_res->add_size);
+	/*
+	 * If we have run out of bridge resources, we may end up with a
+	 * zero-sized resource which may cause its bridge to not be enabled.
+	 * Disabling the resource prevents any such issues.
+	 */
+	if (!new_size)
+		reset_resource(res);
 }
 
 static void pci_bus_distribute_available_resources(struct pci_bus *bus,
-- 
2.19.1


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

* [PATCH v5 3/5] PCI: Fix bug resulting in double hpmemsize being assigned to MMIO window
       [not found] <20190505144001.8106-1-nicholas.johnson-opensource@outlook.com.au>
  2019-05-05 14:40 ` [PATCH v5 1/5] PCI: Consider alignment of hot-added bridges when distributing resources Nicholas Johnson
  2019-05-05 14:40 ` [PATCH v5 2/5] PCI: Modify extend_bridge_window() to set resource size directly Nicholas Johnson
@ 2019-05-05 14:40 ` Nicholas Johnson
  2019-05-05 14:41 ` [PATCH v5 4/5] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently Nicholas Johnson
  2019-05-05 14:41 ` [PATCH v5 5/5] PCI: Cleanup block comments in setup-bus.c to match kernel style Nicholas Johnson
  4 siblings, 0 replies; 5+ messages in thread
From: Nicholas Johnson @ 2019-05-05 14:40 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pci, bhelgaas, mika.westerberg, corbet, Nicholas Johnson

Background
==========================================================================

Solve bug report:
https://bugzilla.kernel.org/show_bug.cgi?id=203243

Currently, the kernel can sometimes assign the MMIO_PREF window
additional size into the MMIO window, resulting in double the MMIO
additional size, even if the MMIO_PREF window was successful.

This happens if in the first pass, the MMIO_PREF succeeds but the MMIO
fails. In the next pass, because MMIO_PREF is already assigned, the
attempt to assign MMIO_PREF returns an error code instead of success
(nothing more to do, already allocated).

Example of problem (more context can be found in the bug report URL):

Mainline kernel:
pci 0000:06:01.0: BAR 14: assigned [mem 0x90100000-0xa00fffff] = 256M
pci 0000:06:04.0: BAR 14: assigned [mem 0xa0200000-0xb01fffff] = 256M

Patched kernel:
pci 0000:06:01.0: BAR 14: assigned [mem 0x90100000-0x980fffff] = 128M
pci 0000:06:04.0: BAR 14: assigned [mem 0x98200000-0xa01fffff] = 128M

This was using pci=realloc,hpmemsize=128M,nocrs - on the same machine
with the same configuration, with a Ubuntu mainline kernel and a kernel
patched with this patch series.

This patch is vital for the next patch in the series. The next patch
allows the user to specify MMIO and MMIO_PREF independently. If the
MMIO_PREF is set to be very large, this bug will end up more than
doubling the MMIO size. The bug results in the MMIO_PREF being added to
the MMIO window, which means doubling if MMIO_PREF size == MMIO size.
With a large MMIO_PREF, without this patch, the MMIO window will likely
fail to be assigned altogether due to lack of 32-bit address space.

Patch notes
==========================================================================

Change find_free_bus_resource() to not skip assigned resources with
non-null parent.

Add checks in pbus_size_io() and pbus_size_mem() to return success if
resource returned from find_free_bus_resource() is already allocated.

This avoids pbus_size_io() and pbus_size_mem() returning error code to
__pci_bus_size_bridges() when a resource has been successfully assigned
in a previous pass. This fixes the existing behaviour where space for a
resource could be reserved multiple times in different parent bridge
windows. This also greatly reduces the number of failed BAR messages in
dmesg when Linux assigns resources.

Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
---
 drivers/pci/setup-bus.c | 28 +++++++++++++++++++---------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 5214815c7..e7126cc0e 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -752,11 +752,17 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
 	}
 }
 
-/* Helper function for sizing routines: find first available
-   bus resource of a given type. Note: we intentionally skip
-   the bus resources which have already been assigned (that is,
-   have non-NULL parent resource). */
-static struct resource *find_free_bus_resource(struct pci_bus *bus,
+/*
+ * Helper function for sizing routines: find first bus resource of a given
+ * type. Note: we do not skip the bus resources which have already been
+ * assigned (r->parent != NULL). This is because a resource that is already
+ * assigned (nothing more to be done) will be indistinguishable from one that
+ * failed due to lack of space if we skip assigned resources. If the caller
+ * function cannot tell the difference then it might try to place the
+ * resources in a different window, doubling up on resources or causing
+ * unforeseeable issues.
+ */
+static struct resource *find_bus_resource_of_type(struct pci_bus *bus,
 			 unsigned long type_mask, unsigned long type)
 {
 	int i;
@@ -765,7 +771,7 @@ static struct resource *find_free_bus_resource(struct pci_bus *bus,
 	pci_bus_for_each_resource(bus, r, i) {
 		if (r == &ioport_resource || r == &iomem_resource)
 			continue;
-		if (r && (r->flags & type_mask) == type && !r->parent)
+		if (r && (r->flags & type_mask) == type)
 			return r;
 	}
 	return NULL;
@@ -863,14 +869,16 @@ static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 		resource_size_t add_size, struct list_head *realloc_head)
 {
 	struct pci_dev *dev;
-	struct resource *b_res = find_free_bus_resource(bus, IORESOURCE_IO,
-							IORESOURCE_IO);
+	struct resource *b_res = find_bus_resource_of_type(bus, IORESOURCE_IO,
+					IORESOURCE_IO);
 	resource_size_t size = 0, size0 = 0, size1 = 0;
 	resource_size_t children_add_size = 0;
 	resource_size_t min_align, align;
 
 	if (!b_res)
 		return;
+	if (b_res->parent)
+		return;
 
 	min_align = window_alignment(bus, IORESOURCE_IO);
 	list_for_each_entry(dev, &bus->devices, bus_list) {
@@ -975,7 +983,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 	resource_size_t min_align, align, size, size0, size1;
 	resource_size_t aligns[18];	/* Alignments from 1Mb to 128Gb */
 	int order, max_order;
-	struct resource *b_res = find_free_bus_resource(bus,
+	struct resource *b_res = find_bus_resource_of_type(bus,
 					mask | IORESOURCE_PREFETCH, type);
 	resource_size_t children_add_size = 0;
 	resource_size_t children_add_align = 0;
@@ -983,6 +991,8 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 
 	if (!b_res)
 		return -ENOSPC;
+	if (b_res->parent)
+		return 0;
 
 	memset(aligns, 0, sizeof(aligns));
 	max_order = 0;
-- 
2.19.1


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

* [PATCH v5 4/5] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently
       [not found] <20190505144001.8106-1-nicholas.johnson-opensource@outlook.com.au>
                   ` (2 preceding siblings ...)
  2019-05-05 14:40 ` [PATCH v5 3/5] PCI: Fix bug resulting in double hpmemsize being assigned to MMIO window Nicholas Johnson
@ 2019-05-05 14:41 ` Nicholas Johnson
  2019-05-05 14:41 ` [PATCH v5 5/5] PCI: Cleanup block comments in setup-bus.c to match kernel style Nicholas Johnson
  4 siblings, 0 replies; 5+ messages in thread
From: Nicholas Johnson @ 2019-05-05 14:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pci, bhelgaas, mika.westerberg, corbet, Nicholas Johnson

Add kernel parameter pci=hpmemprefsize=nn[KMG] to control MMIO_PREF size
for PCI hotplug bridges.

Change behaviour of pci=hpmemsize=nn[KMG] to not set MMIO_PREF size if
hpmempref has been specified, rather than controlling both MMIO and
MMIO_PREF sizes unconditionally.

Update kernel-parameters documentation to reflect the above changes.

The effect of the above changes is to allow for MMIO and MMIO_PREF to be
specified independently, whilst ensuring no changes in functionality are
noticed by the user if updating kernel version with hpmemsize specified.

Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
---
 .../admin-guide/kernel-parameters.txt         |  7 +++++-
 drivers/pci/pci.c                             | 18 ++++++++++---
 drivers/pci/setup-bus.c                       | 25 +++++++++++--------
 include/linux/pci.h                           |  3 ++-
 4 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2b8ee90bb..400407c27 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3361,7 +3361,12 @@
 				reserved for hotplug bridge's IO window.
 				Default size is 256 bytes.
 		hpmemsize=nn[KMG]	The fixed amount of bus space which is
-				reserved for hotplug bridge's memory window.
+				reserved for hotplug bridge's MMIO window. If
+				hpmemprefsize is not specified, then the same
+				size is applied to hotplug bridge's MMIO_PREF
+				window. Default size is 2 megabytes.
+		hpmemprefsize=nn[KMG]	The fixed amount of bus space which is
+				reserved for hotplug bridge's MMIO_PREF window.
 				Default size is 2 megabytes.
 		hpbussize=nn	The minimum amount of additional bus numbers
 				reserved for buses below a hotplug bridge.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 7c1b362f5..7c7b95aab 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -85,10 +85,12 @@ unsigned long pci_cardbus_io_size = DEFAULT_CARDBUS_IO_SIZE;
 unsigned long pci_cardbus_mem_size = DEFAULT_CARDBUS_MEM_SIZE;
 
 #define DEFAULT_HOTPLUG_IO_SIZE		(256)
-#define DEFAULT_HOTPLUG_MEM_SIZE	(2*1024*1024)
+#define DEFAULT_HOTPLUG_MMIO_SIZE	(2*1024*1024)
+#define DEFAULT_HOTPLUG_MMIO_PREF_SIZE	(2*1024*1024)
 /* pci=hpmemsize=nnM,hpiosize=nn can override this */
 unsigned long pci_hotplug_io_size  = DEFAULT_HOTPLUG_IO_SIZE;
-unsigned long pci_hotplug_mem_size = DEFAULT_HOTPLUG_MEM_SIZE;
+unsigned long pci_hotplug_mmio_size = DEFAULT_HOTPLUG_MMIO_SIZE;
+unsigned long pci_hotplug_mmio_pref_size = DEFAULT_HOTPLUG_MMIO_PREF_SIZE;
 
 #define DEFAULT_HOTPLUG_BUS_SIZE	1
 unsigned long pci_hotplug_bus_size = DEFAULT_HOTPLUG_BUS_SIZE;
@@ -6211,6 +6213,8 @@ EXPORT_SYMBOL(pci_fixup_cardbus);
 
 static int __init pci_setup(char *str)
 {
+	bool mmio_pref_specified = false;
+
 	while (str) {
 		char *k = strchr(str, ',');
 		if (k)
@@ -6245,7 +6249,15 @@ static int __init pci_setup(char *str)
 			} else if (!strncmp(str, "hpiosize=", 9)) {
 				pci_hotplug_io_size = memparse(str + 9, &str);
 			} else if (!strncmp(str, "hpmemsize=", 10)) {
-				pci_hotplug_mem_size = memparse(str + 10, &str);
+				pci_hotplug_mmio_size =
+					memparse(str + 10, &str);
+				if (!mmio_pref_specified)
+					pci_hotplug_mmio_pref_size =
+						pci_hotplug_mmio_size;
+			} else if (!strncmp(str, "hpmemprefsize=", 14)) {
+				mmio_pref_specified = true;
+				pci_hotplug_mmio_pref_size =
+					memparse(str + 14, &str);
 			} else if (!strncmp(str, "hpbussize=", 10)) {
 				pci_hotplug_bus_size =
 					simple_strtoul(str + 10, &str, 0);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index e7126cc0e..4386ca941 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1187,7 +1187,8 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 {
 	struct pci_dev *dev;
 	unsigned long mask, prefmask, type2 = 0, type3 = 0;
-	resource_size_t additional_mem_size = 0, additional_io_size = 0;
+	resource_size_t additional_io_size = 0, additional_mmio_size = 0,
+		additional_mmio_pref_size = 0;
 	struct resource *b_res;
 	int ret;
 
@@ -1221,7 +1222,8 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 		pci_bridge_check_ranges(bus);
 		if (bus->self->is_hotplug_bridge) {
 			additional_io_size  = pci_hotplug_io_size;
-			additional_mem_size = pci_hotplug_mem_size;
+			additional_mmio_size = pci_hotplug_mmio_size;
+			additional_mmio_pref_size = pci_hotplug_mmio_pref_size;
 		}
 		/* Fall through */
 	default:
@@ -1239,9 +1241,9 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 		if (b_res[2].flags & IORESOURCE_MEM_64) {
 			prefmask |= IORESOURCE_MEM_64;
 			ret = pbus_size_mem(bus, prefmask, prefmask,
-				  prefmask, prefmask,
-				  realloc_head ? 0 : additional_mem_size,
-				  additional_mem_size, realloc_head);
+				prefmask, prefmask,
+				realloc_head ? 0 : additional_mmio_pref_size,
+				additional_mmio_pref_size, realloc_head);
 
 			/*
 			 * If successful, all non-prefetchable resources
@@ -1263,9 +1265,9 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 		if (!type2) {
 			prefmask &= ~IORESOURCE_MEM_64;
 			ret = pbus_size_mem(bus, prefmask, prefmask,
-					 prefmask, prefmask,
-					 realloc_head ? 0 : additional_mem_size,
-					 additional_mem_size, realloc_head);
+				prefmask, prefmask,
+				realloc_head ? 0 : additional_mmio_pref_size,
+				additional_mmio_pref_size, realloc_head);
 
 			/*
 			 * If successful, only non-prefetchable resources
@@ -1274,7 +1276,8 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 			if (ret == 0)
 				mask = prefmask;
 			else
-				additional_mem_size += additional_mem_size;
+				additional_mmio_size +=
+					additional_mmio_pref_size;
 
 			type2 = type3 = IORESOURCE_MEM;
 		}
@@ -1295,8 +1298,8 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 		 * window.
 		 */
 		pbus_size_mem(bus, mask, IORESOURCE_MEM, type2, type3,
-				realloc_head ? 0 : additional_mem_size,
-				additional_mem_size, realloc_head);
+			realloc_head ? 0 : additional_mmio_size,
+			additional_mmio_size, realloc_head);
 		break;
 	}
 }
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 77448215e..7ab8f5b88 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1964,7 +1964,8 @@ extern u8 pci_dfl_cache_line_size;
 extern u8 pci_cache_line_size;
 
 extern unsigned long pci_hotplug_io_size;
-extern unsigned long pci_hotplug_mem_size;
+extern unsigned long pci_hotplug_mmio_size;
+extern unsigned long pci_hotplug_mmio_pref_size;
 extern unsigned long pci_hotplug_bus_size;
 
 /* Architecture-specific versions may override these (weak) */
-- 
2.19.1


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

* [PATCH v5 5/5] PCI: Cleanup block comments in setup-bus.c to match kernel style
       [not found] <20190505144001.8106-1-nicholas.johnson-opensource@outlook.com.au>
                   ` (3 preceding siblings ...)
  2019-05-05 14:41 ` [PATCH v5 4/5] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently Nicholas Johnson
@ 2019-05-05 14:41 ` Nicholas Johnson
  4 siblings, 0 replies; 5+ messages in thread
From: Nicholas Johnson @ 2019-05-05 14:41 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-pci, bhelgaas, mika.westerberg, corbet, Nicholas Johnson

Change block comments to accepted style with asterisks on each line.

Justify block comments to 80-character limit to reduce the number of
lines where possible.

Change beginnings of sentences to have capital letter.

Place periods at the ends of sentences that are only separated by
newlines.

Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
---
 drivers/pci/setup-bus.c | 310 +++++++++++++++++++---------------------
 1 file changed, 151 insertions(+), 159 deletions(-)

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 4386ca941..400cf616c 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -51,11 +51,9 @@ static void free_list(struct list_head *head)
 /**
  * add_to_list() - add a new resource tracker to the list
  * @head:	Head of the list
- * @dev:	device corresponding to which the resource
- *		belongs
+ * @dev:	device corresponding to which the resource belongs
  * @res:	The resource to be tracked
- * @add_size:	additional size to be optionally added
- *              to the resource
+ * @add_size:	additional size to be optionally added to the resource
  */
 static int add_to_list(struct list_head *head,
 		 struct pci_dev *dev, struct resource *res,
@@ -158,7 +156,7 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
 		tmp->res = r;
 		tmp->dev = dev;
 
-		/* fallback is smallest one or list is empty*/
+		/* fallback is smallest one or list is empty */
 		n = head;
 		list_for_each_entry(dev_res, head, list) {
 			resource_size_t align;
@@ -171,7 +169,7 @@ static void pdev_sort_resources(struct pci_dev *dev, struct list_head *head)
 				break;
 			}
 		}
-		/* Insert it just before n*/
+		/* Insert it just before n */
 		list_add_tail(&tmp->list, n);
 	}
 }
@@ -181,7 +179,7 @@ static void __dev_sort_resources(struct pci_dev *dev,
 {
 	u16 class = dev->class >> 8;
 
-	/* Don't touch classless devices or host bridges or ioapics.  */
+	/* Don't touch classless devices or host bridges or ioapics */
 	if (class == PCI_CLASS_NOT_DEFINED || class == PCI_CLASS_BRIDGE_HOST)
 		return;
 
@@ -208,12 +206,10 @@ static inline void reset_resource(struct resource *res)
  *
  * @realloc_head : head of the list tracking requests requiring additional
  *             resources
- * @head     : head of the list tracking requests with allocated
- *             resources
+ * @head     : head of the list tracking requests with allocated resources
  *
- * Walk through each element of the realloc_head and try to procure
- * additional resources for the element, provided the element
- * is in the head list.
+ * Walk through each element of the realloc_head and try to procure additional
+ * resources for the element, provided the element is in the head list.
  */
 static void reassign_resources_sorted(struct list_head *realloc_head,
 		struct list_head *head)
@@ -239,7 +235,7 @@ static void reassign_resources_sorted(struct list_head *realloc_head,
 				break;
 			}
 		}
-		if (!found_match)/* just skip */
+		if (!found_match) /* just skip */
 			continue;
 
 		idx = res - &add_res->dev->resource[0];
@@ -310,15 +306,14 @@ static unsigned long pci_fail_res_type_mask(struct list_head *fail_head)
 	struct pci_dev_resource *fail_res;
 	unsigned long mask = 0;
 
-	/* check failed type */
+	/* Check failed type */
 	list_for_each_entry(fail_res, fail_head, list)
 		mask |= fail_res->flags;
 
 	/*
-	 * one pref failed resource will set IORESOURCE_MEM,
-	 * as we can allocate pref in non-pref range.
-	 * Will release all assigned non-pref sibling resources
-	 * according to that bit.
+	 * One pref failed resource will set IORESOURCE_MEM, as we can allocate
+	 * pref in non-pref range. Will release all assigned non-pref sibling
+	 * resources according to that bit.
 	 */
 	return mask & (IORESOURCE_IO | IORESOURCE_MEM | IORESOURCE_PREFETCH);
 }
@@ -328,11 +323,11 @@ static bool pci_need_to_release(unsigned long mask, struct resource *res)
 	if (res->flags & IORESOURCE_IO)
 		return !!(mask & IORESOURCE_IO);
 
-	/* check pref at first */
+	/* Check pref at first */
 	if (res->flags & IORESOURCE_PREFETCH) {
 		if (mask & IORESOURCE_PREFETCH)
 			return true;
-		/* count pref if its parent is non-pref */
+		/* Count pref if its parent is non-pref */
 		else if ((mask & IORESOURCE_MEM) &&
 			 !(res->parent->flags & IORESOURCE_PREFETCH))
 			return true;
@@ -343,7 +338,7 @@ static bool pci_need_to_release(unsigned long mask, struct resource *res)
 	if (res->flags & IORESOURCE_MEM)
 		return !!(mask & IORESOURCE_MEM);
 
-	return false;	/* should not get here */
+	return false;	/* Should not get here */
 }
 
 static void __assign_resources_sorted(struct list_head *head,
@@ -351,25 +346,24 @@ static void __assign_resources_sorted(struct list_head *head,
 				 struct list_head *fail_head)
 {
 	/*
-	 * Should not assign requested resources at first.
-	 *   they could be adjacent, so later reassign can not reallocate
-	 *   them one by one in parent resource window.
-	 * Try to assign requested + add_size at beginning
-	 *  if could do that, could get out early.
-	 *  if could not do that, we still try to assign requested at first,
-	 *    then try to reassign add_size for some resources.
+	 * Should not assign requested resources at first. They could be
+	 * adjacent, so later reassign can not reallocate them one by one in
+	 * parent resource window.
+	 *
+	 * Try to assign requested + add_size at beginning. If could do that,
+	 * could get out early. If could not do that, we still try to assign
+	 * requested at first, then try to reassign add_size for some resources.
 	 *
 	 * Separate three resource type checking if we need to release
 	 * assigned resource after requested + add_size try.
-	 *	1. if there is io port assign fail, will release assigned
-	 *	   io port.
-	 *	2. if there is pref mmio assign fail, release assigned
-	 *	   pref mmio.
-	 *	   if assigned pref mmio's parent is non-pref mmio and there
-	 *	   is non-pref mmio assign fail, will release that assigned
-	 *	   pref mmio.
-	 *	3. if there is non-pref mmio assign fail or pref mmio
-	 *	   assigned fail, will release assigned non-pref mmio.
+	 *	1. if there is io port assign fail, will release assigned io
+	 *	   port.
+	 *	2. if there is pref mmio assign fail, release assigned pref
+	 *	   mmio. If assigned pref mmio's parent is non-pref mmio
+	 *	   and there is non-pref mmio assign fail, will release that
+	 *	   assigned pref mmio.
+	 *	3. if there is non-pref mmio assign fail or pref mmio assigned
+	 *	   fail, will release assigned non-pref mmio.
 	 */
 	LIST_HEAD(save_head);
 	LIST_HEAD(local_fail_head);
@@ -398,7 +392,7 @@ static void __assign_resources_sorted(struct list_head *head,
 		/*
 		 * There are two kinds of additional resources in the list:
 		 * 1. bridge resource  -- IORESOURCE_STARTALIGN
-		 * 2. SR-IOV resource   -- IORESOURCE_SIZEALIGN
+		 * 2. SR-IOV resource  -- IORESOURCE_SIZEALIGN
 		 * Here just fix the additional alignment for bridge
 		 */
 		if (!(dev_res->res->flags & IORESOURCE_STARTALIGN))
@@ -435,7 +429,7 @@ static void __assign_resources_sorted(struct list_head *head,
 	/* Try updated head list with add_size added */
 	assign_requested_resources_sorted(head, &local_fail_head);
 
-	/* all assigned with add_size ? */
+	/* All assigned with add_size ? */
 	if (list_empty(&local_fail_head)) {
 		/* Remove head list from realloc_head list */
 		list_for_each_entry(dev_res, head, list)
@@ -445,13 +439,13 @@ static void __assign_resources_sorted(struct list_head *head,
 		return;
 	}
 
-	/* check failed type */
+	/* Check failed type */
 	fail_type = pci_fail_res_type_mask(&local_fail_head);
-	/* remove not need to be released assigned res from head list etc */
+	/* Remove not need to be released assigned res from head list etc */
 	list_for_each_entry_safe(dev_res, tmp_res, head, list)
 		if (dev_res->res->parent &&
 		    !pci_need_to_release(fail_type, dev_res->res)) {
-			/* remove it from realloc_head list */
+			/* Remove it from realloc_head list */
 			remove_from_list(realloc_head, dev_res->res);
 			remove_from_list(&save_head, dev_res->res);
 			list_del(&dev_res->list);
@@ -477,8 +471,7 @@ static void __assign_resources_sorted(struct list_head *head,
 	/* Satisfy the must-have resource requests */
 	assign_requested_resources_sorted(head, fail_head);
 
-	/* Try to satisfy any additional optional resource
-		requests */
+	/* Try to satisfy any additional optional resource requests */
 	if (realloc_head)
 		reassign_resources_sorted(realloc_head, head);
 	free_list(head);
@@ -563,17 +556,19 @@ void pci_setup_cardbus(struct pci_bus *bus)
 }
 EXPORT_SYMBOL(pci_setup_cardbus);
 
-/* Initialize bridges with base/limit values we have collected.
-   PCI-to-PCI Bridge Architecture Specification rev. 1.1 (1998)
-   requires that if there is no I/O ports or memory behind the
-   bridge, corresponding range must be turned off by writing base
-   value greater than limit to the bridge's base/limit registers.
-
-   Note: care must be taken when updating I/O base/limit registers
-   of bridges which support 32-bit I/O. This update requires two
-   config space writes, so it's quite possible that an I/O window of
-   the bridge will have some undesirable address (e.g. 0) after the
-   first write. Ditto 64-bit prefetchable MMIO.  */
+/*
+ * Initialize bridges with base/limit values we have collected. PCI-to-PCI
+ * Bridge Architecture Specification rev. 1.1 (1998) requires that if there is
+ * no I/O ports or memory behind the bridge, corresponding range must be turned
+ * off by writing base value greater than limit to the bridge's base/limit
+ * registers.
+ *
+ * Note: care must be taken when updating I/O base/limit registers of bridges
+ * which support 32-bit I/O. This update requires two config space writes, so
+ * it's quite possible that an I/O window of the bridge will have some
+ * undesirable address (e.g. 0) after the first write. Ditto 64-bit prefetchable
+ *  MMIO.
+ */
 static void pci_setup_bridge_io(struct pci_dev *bridge)
 {
 	struct resource *res;
@@ -636,9 +631,11 @@ static void pci_setup_bridge_mmio_pref(struct pci_dev *bridge)
 	struct pci_bus_region region;
 	u32 l, bu, lu;
 
-	/* Clear out the upper 32 bits of PREF limit.
-	   If PCI_PREF_BASE_UPPER32 was non-zero, this temporarily
-	   disables PREF range, which is ok. */
+	/*
+	 * Clear out the upper 32 bits of PREF limit.
+	 * If PCI_PREF_BASE_UPPER32 was non-zero, this temporarily
+	 * disables PREF range, which is ok.
+	 */
 	pci_write_config_dword(bridge, PCI_PREF_LIMIT_UPPER32, 0);
 
 	/* Set up PREF base/limit. */
@@ -702,13 +699,13 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
 		return 0;
 
 	if (pci_claim_resource(bridge, i) == 0)
-		return 0;	/* claimed the window */
+		return 0;	/* Claimed the window */
 
 	if ((bridge->class >> 8) != PCI_CLASS_BRIDGE_PCI)
 		return 0;
 
 	if (!pci_bus_clip_resource(bridge, i))
-		return -EINVAL;	/* clipping didn't change anything */
+		return -EINVAL;	/* Clipping didn't change anything */
 
 	switch (i - PCI_BRIDGE_RESOURCES) {
 	case 0:
@@ -725,14 +722,16 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
 	}
 
 	if (pci_claim_resource(bridge, i) == 0)
-		return 0;	/* claimed a smaller window */
+		return 0;	/* Claimed a smaller window */
 
 	return -EINVAL;
 }
 
-/* Check whether the bridge supports optional I/O and
-   prefetchable memory ranges. If not, the respective
-   base/limit registers must be read-only and read as 0. */
+/*
+ * Check whether the bridge supports optional I/O and prefetchable memory
+ * ranges. If not, the respective base/limit registers must be read-only and
+ * read as 0.
+ */
 static void pci_bridge_check_ranges(struct pci_bus *bus)
 {
 	struct pci_dev *bridge = bus->self;
@@ -789,8 +788,10 @@ static resource_size_t calculate_iosize(resource_size_t size,
 		size = min_size;
 	if (old_size == 1)
 		old_size = 0;
-	/* To be fixed in 2.5: we should have sort of HAVE_ISA
-	   flag in the struct pci_bus. */
+	/*
+	 * To be fixed in 2.5: we should have sort of HAVE_ISA flag in the
+	 * struct pci_bus.
+	 */
 #if defined(CONFIG_ISA) || defined(CONFIG_EISA)
 	size = (size & 0xff) + ((size & ~0xffUL) << 2);
 #endif
@@ -839,8 +840,8 @@ static resource_size_t window_alignment(struct pci_bus *bus,
 		align = PCI_P2P_DEFAULT_MEM_ALIGN;
 	else if (type & IORESOURCE_IO) {
 		/*
-		 * Per spec, I/O windows are 4K-aligned, but some
-		 * bridges have an extension to support 1K alignment.
+		 * Per spec, I/O windows are 4K-aligned, but some bridges have
+		 * an extension to support 1K alignment.
 		 */
 		if (bus->self->io_window_1k)
 			align = PCI_P2P_DEFAULT_IO_ALIGN_1K;
@@ -860,10 +861,9 @@ static resource_size_t window_alignment(struct pci_bus *bus,
  * @add_size : additional optional io window
  * @realloc_head : track the additional io window on this list
  *
- * Sizing the IO windows of the PCI-PCI bridge is trivial,
- * since these windows have 1K or 4K granularity and the IO ranges
- * of non-bridge PCI devices are limited to 256 bytes.
- * We must be careful with the ISA aliasing though.
+ * Sizing the IO windows of the PCI-PCI bridge is trivial, since these windows
+ * have 1K or 4K granularity and the IO ranges of non-bridge PCI devices are
+ * limited to 256 bytes. We must be careful with the ISA aliasing though.
  */
 static void pbus_size_io(struct pci_bus *bus, resource_size_t min_size,
 		resource_size_t add_size, struct list_head *realloc_head)
@@ -966,12 +966,12 @@ static inline resource_size_t calculate_mem_align(resource_size_t *aligns,
  * @add_size : additional optional memory window
  * @realloc_head : track the additional memory window on this list
  *
- * Calculate the size of the bus and minimal alignment which
- * guarantees that all child resources fit in this size.
+ * Calculate the size of the bus and minimal alignment which guarantees that all
+ * child resources fit in this size.
  *
  * Returns -ENOSPC if there's no available bus resource of the desired type.
- * Otherwise, sets the bus resource start/end to indicate the required
- * size, adds things to realloc_head (if supplied), and returns 0.
+ * Otherwise, sets the bus resource start/end to indicate the required size,
+ * adds things to realloc_head (if supplied), and returns 0.
  */
 static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			 unsigned long type, unsigned long type2,
@@ -981,7 +981,7 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 {
 	struct pci_dev *dev;
 	resource_size_t min_align, align, size, size0, size1;
-	resource_size_t aligns[18];	/* Alignments from 1Mb to 128Gb */
+	resource_size_t aligns[18]; /* Alignments from 1Mb to 128Gb */
 	int order, max_order;
 	struct resource *b_res = find_bus_resource_of_type(bus,
 					mask | IORESOURCE_PREFETCH, type);
@@ -1012,21 +1012,20 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 				continue;
 			r_size = resource_size(r);
 #ifdef CONFIG_PCI_IOV
-			/* put SRIOV requested res to the optional list */
+			/* Put SRIOV requested res to the optional list */
 			if (realloc_head && i >= PCI_IOV_RESOURCES &&
 					i <= PCI_IOV_RESOURCE_END) {
 				add_align = max(pci_resource_alignment(dev, r), add_align);
 				r->end = r->start - 1;
-				add_to_list(realloc_head, dev, r, r_size, 0/* don't care */);
+				add_to_list(realloc_head, dev, r, r_size, 0 /* Don't care */);
 				children_add_size += r_size;
 				continue;
 			}
 #endif
 			/*
-			 * aligns[0] is for 1MB (since bridge memory
-			 * windows are always at least 1MB aligned), so
-			 * keep "order" from being negative for smaller
-			 * resources.
+			 * aligns[0] is for 1MB (since bridge memory windows are
+			 * always at least 1MB aligned), so keep "order" from
+			 * being negative for smaller resources.
 			 */
 			align = pci_resource_alignment(dev, r);
 			order = __ffs(align) - 20;
@@ -1039,8 +1038,10 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 				continue;
 			}
 			size += max(r_size, align);
-			/* Exclude ranges with size > align from
-			   calculation of the alignment. */
+			/*
+			 * Exclude ranges with size > align from calculation of
+			 * the alignment.
+			 */
 			if (r_size <= align)
 				aligns[order] += align;
 			if (order > max_order)
@@ -1101,8 +1102,8 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 	if (b_res[0].parent)
 		goto handle_b_res_1;
 	/*
-	 * Reserve some resources for CardBus.  We reserve
-	 * a fixed amount of bus space for CardBus bridges.
+	 * Reserve some resources for CardBus.  We reserve a fixed amount of bus
+	 * space for CardBus bridges.
 	 */
 	b_res[0].start = pci_cardbus_io_size;
 	b_res[0].end = b_res[0].start + pci_cardbus_io_size - 1;
@@ -1134,10 +1135,7 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 		pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
 	}
 
-	/*
-	 * Check whether prefetchable memory is supported
-	 * by this bridge.
-	 */
+	/* Check whether prefetchable memory is supported by this bridge. */
 	pci_read_config_word(bridge, PCI_CB_BRIDGE_CONTROL, &ctrl);
 	if (!(ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0)) {
 		ctrl |= PCI_CB_BRIDGE_CTL_PREFETCH_MEM0;
@@ -1148,9 +1146,8 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 	if (b_res[2].parent)
 		goto handle_b_res_3;
 	/*
-	 * If we have prefetchable memory support, allocate
-	 * two regions.  Otherwise, allocate one region of
-	 * twice the size.
+	 * If we have prefetchable memory support, allocate two regions.
+	 * Otherwise, allocate one region of twice the size.
 	 */
 	if (ctrl & PCI_CB_BRIDGE_CTL_PREFETCH_MEM0) {
 		b_res[2].start = pci_cardbus_mem_size;
@@ -1163,7 +1160,7 @@ static void pci_bus_size_cardbus(struct pci_bus *bus,
 				 pci_cardbus_mem_size, pci_cardbus_mem_size);
 		}
 
-		/* reduce that to half */
+		/* Reduce that to half */
 		b_res_3_size = pci_cardbus_mem_size;
 	}
 
@@ -1215,7 +1212,7 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 
 	switch (bus->self->hdr_type) {
 	case PCI_HEADER_TYPE_CARDBUS:
-		/* don't size cardbuses yet. */
+		/* Don't size cardbuses yet. */
 		break;
 
 	case PCI_HEADER_TYPE_BRIDGE:
@@ -1231,9 +1228,8 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 			     additional_io_size, realloc_head);
 
 		/*
-		 * If there's a 64-bit prefetchable MMIO window, compute
-		 * the size required to put all 64-bit prefetchable
-		 * resources in it.
+		 * If there's a 64-bit prefetchable MMIO window, compute the
+		 * size required to put all 64-bit prefetchable resources in it.
 		 */
 		b_res = &bus->self->resource[PCI_BRIDGE_RESOURCES];
 		mask = IORESOURCE_MEM;
@@ -1246,9 +1242,9 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 				additional_mmio_pref_size, realloc_head);
 
 			/*
-			 * If successful, all non-prefetchable resources
-			 * and any 32-bit prefetchable resources will go in
-			 * the non-prefetchable window.
+			 * If successful, all non-prefetchable resources and any
+			 * 32-bit prefetchable resources will go in the
+			 * non-prefetchable window.
 			 */
 			if (ret == 0) {
 				mask = prefmask;
@@ -1284,18 +1280,17 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
 
 		/*
 		 * Compute the size required to put everything else in the
-		 * non-prefetchable window.  This includes:
+		 * non-prefetchable window. This includes:
 		 *
 		 *   - all non-prefetchable resources
 		 *   - 32-bit prefetchable resources if there's a 64-bit
 		 *     prefetchable window or no prefetchable window at all
-		 *   - 64-bit prefetchable resources if there's no
-		 *     prefetchable window at all
+		 *   - 64-bit prefetchable resources if there's no prefetchable
+		 *     window at all
 		 *
-		 * Note that the strategy in __pci_assign_resource() must
-		 * match that used here.  Specifically, we cannot put a
-		 * 32-bit prefetchable resource in a 64-bit prefetchable
-		 * window.
+		 * Note that the strategy in __pci_assign_resource() must match
+		 * that used here. Specifically, we cannot put a 32-bit
+		 * prefetchable resource in a 64-bit prefetchable window.
 		 */
 		pbus_size_mem(bus, mask, IORESOURCE_MEM, type2, type3,
 			realloc_head ? 0 : additional_mmio_size,
@@ -1328,8 +1323,8 @@ static void assign_fixed_resource_on_bus(struct pci_bus *b, struct resource *r)
 }
 
 /*
- * Try to assign any resources marked as IORESOURCE_PCI_FIXED, as they
- * are skipped by pbus_assign_resources_sorted().
+ * Try to assign any resources marked as IORESOURCE_PCI_FIXED, as they are
+ * skipped by pbus_assign_resources_sorted().
  */
 static void pdev_assign_fixed_resources(struct pci_dev *dev)
 {
@@ -1440,10 +1435,9 @@ static void pci_bus_allocate_resources(struct pci_bus *b)
 	struct pci_bus *child;
 
 	/*
-	 * Carry out a depth-first search on the PCI bus
-	 * tree to allocate bridge apertures. Read the
-	 * programmed bridge bases and recursively claim
-	 * the respective bridge resources.
+	 * Carry out a depth-first search on the PCI bus tree to allocate bridge
+	 * apertures. Read the programmed bridge bases and recursively claim the
+	 * respective bridge resources.
 	 */
 	if (b->self) {
 		pci_read_bridge_bases(b);
@@ -1508,16 +1502,15 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
 	b_res = &dev->resource[PCI_BRIDGE_RESOURCES];
 
 	/*
-	 *     1. if there is io port assign fail, will release bridge
-	 *	  io port.
-	 *     2. if there is non pref mmio assign fail, release bridge
-	 *	  nonpref mmio.
-	 *     3. if there is 64bit pref mmio assign fail, and bridge pref
-	 *	  is 64bit, release bridge pref mmio.
-	 *     4. if there is pref mmio assign fail, and bridge pref is
-	 *	  32bit mmio, release bridge pref mmio
-	 *     5. if there is pref mmio assign fail, and bridge pref is not
-	 *	  assigned, release bridge nonpref mmio.
+	 * 1. If there is io port assign fail, will release bridge io port.
+	 * 2. If there is non pref mmio assign fail, release bridge nonpref
+	 *    mmio.
+	 * 3. If there is 64bit pref mmio assign fail, and bridge pref is 64bit,
+	 *    release bridge pref mmio.
+	 * 4. If there is pref mmio assign fail, and bridge pref is 32bit mmio,
+	 *    release bridge pref mmio
+	 * 5. If there is pref mmio assign fail, and bridge pref is not
+	 *    assigned, release bridge nonpref mmio.
 	 */
 	if (type & IORESOURCE_IO)
 		idx = 0;
@@ -1537,25 +1530,22 @@ static void pci_bridge_release_resources(struct pci_bus *bus,
 	if (!r->parent)
 		return;
 
-	/*
-	 * if there are children under that, we should release them
-	 *  all
-	 */
+	/* If there are children under that, we should release them all */
 	release_child_resources(r);
 	if (!release_resource(r)) {
 		type = old_flags = r->flags & PCI_RES_TYPE_MASK;
 		pci_printk(KERN_DEBUG, dev, "resource %d %pR released\n",
 					PCI_BRIDGE_RESOURCES + idx, r);
-		/* keep the old size */
+		/* Keep the old size */
 		r->end = resource_size(r) - 1;
 		r->start = 0;
 		r->flags = 0;
 
-		/* avoiding touch the one without PREF */
+		/* 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 */
+		/* For next child res under same bridge */
 		r->flags = old_flags;
 	}
 }
@@ -1565,8 +1555,8 @@ enum release_type {
 	whole_subtree,
 };
 /*
- * try to release pci bridge resources that is from leaf bridge,
- * so we can allocate big new one later
+ * Try to release pci bridge resources that is from leaf bridge, so we can
+ * allocate big new one later
  */
 static void pci_bus_release_bridge_resources(struct pci_bus *bus,
 					     unsigned long type,
@@ -1691,7 +1681,7 @@ static int iov_resources_unassigned(struct pci_dev *dev, void *data)
 		pcibios_resource_to_bus(dev->bus, &region, r);
 		if (!region.start) {
 			*unassigned = true;
-			return 1; /* return early from pci_walk_bus() */
+			return 1; /* Return early from pci_walk_bus() */
 		}
 	}
 
@@ -1721,14 +1711,14 @@ static enum enable_type pci_realloc_detect(struct pci_bus *bus,
 #endif
 
 /*
- * first try will not touch pci bridge res
- * second and later try will clear small leaf bridge res
- * will stop till to the max depth if can not find good one
+ * First try will not touch pci bridge res.
+ * Second and later try will clear small leaf bridge res.
+ * Will stop till to the max depth if can not find good one.
  */
 void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
 {
-	LIST_HEAD(realloc_head); /* list of resources that
-					want additional resources */
+	LIST_HEAD(realloc_head);
+	/* List of resources that want additional resources */
 	struct list_head *add_list = NULL;
 	int tried_times = 0;
 	enum release_type rel_type = leaf_only;
@@ -1737,7 +1727,7 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
 	int pci_try_num = 1;
 	enum enable_type enable_local;
 
-	/* don't realloc if asked to do so */
+	/* Don't realloc if asked to do so */
 	enable_local = pci_realloc_detect(bus, pci_realloc_enable);
 	if (pci_realloc_enabled(enable_local)) {
 		int max_depth = pci_bus_get_depth(bus);
@@ -1750,13 +1740,14 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
 
 again:
 	/*
-	 * last try will use add_list, otherwise will try good to have as
-	 * must have, so can realloc parent bridge resource
+	 * Last try will use add_list, otherwise will try good to have as must
+	 * have, so can realloc parent bridge resource
 	 */
 	if (tried_times + 1 == pci_try_num)
 		add_list = &realloc_head;
-	/* Depth first, calculate sizes and alignments of all
-	   subordinate buses. */
+	/*
+	 * Depth first, calculate sizes and alignments of all subordinate buses.
+	 */
 	__pci_bus_size_bridges(bus, add_list);
 
 	/* Depth last, allocate resources and update the hardware. */
@@ -1765,7 +1756,7 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
 		BUG_ON(!list_empty(add_list));
 	tried_times++;
 
-	/* any device complain? */
+	/* Any device complain? */
 	if (list_empty(&fail_head))
 		goto dump;
 
@@ -1782,7 +1773,7 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
 	dev_printk(KERN_DEBUG, &bus->dev,
 		   "No. %d try to assign unassigned res\n", tried_times + 1);
 
-	/* third times and later will not check if it is leaf */
+	/* Third times and later will not check if it is leaf */
 	if ((tried_times + 1) > 2)
 		rel_type = whole_subtree;
 
@@ -1795,7 +1786,7 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
 						 fail_res->flags & PCI_RES_TYPE_MASK,
 						 rel_type);
 
-	/* restore size and flags */
+	/* Restore size and flags */
 	list_for_each_entry(fail_res, &fail_head, list) {
 		struct resource *res = fail_res->res;
 
@@ -1810,7 +1801,7 @@ void pci_assign_unassigned_root_bus_resources(struct pci_bus *bus)
 	goto again;
 
 dump:
-	/* dump the resource on buses */
+	/* Dump the resource on buses */
 	pci_bus_dump_resources(bus);
 }
 
@@ -2014,8 +2005,9 @@ pci_bridge_distribute_available_resources(struct pci_dev *bridge,
 void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
 {
 	struct pci_bus *parent = bridge->subordinate;
-	LIST_HEAD(add_list); /* list of resources that
-					want additional resources */
+	/* List of resources that want additional resources */
+	LIST_HEAD(add_list);
+
 	int tried_times = 0;
 	LIST_HEAD(fail_head);
 	struct pci_dev_resource *fail_res;
@@ -2025,9 +2017,9 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
 	__pci_bus_size_bridges(parent, &add_list);
 
 	/*
-	 * Distribute remaining resources (if any) equally between
-	 * hotplug bridges below. This makes it possible to extend the
-	 * hierarchy later without running out of resources.
+	 * Distribute remaining resources (if any) equally between hotplug
+	 * bridges below. This makes it possible to extend the hierarchy
+	 * later without running out of resources.
 	 */
 	pci_bridge_distribute_available_resources(bridge, &add_list);
 
@@ -2039,7 +2031,7 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
 		goto enable_all;
 
 	if (tried_times >= 2) {
-		/* still fail, don't need to try more */
+		/* Still fail, don't need to try more */
 		free_list(&fail_head);
 		goto enable_all;
 	}
@@ -2056,7 +2048,7 @@ void pci_assign_unassigned_bridge_resources(struct pci_dev *bridge)
 						 fail_res->flags & PCI_RES_TYPE_MASK,
 						 whole_subtree);
 
-	/* restore size and flags */
+	/* Restore size and flags */
 	list_for_each_entry(fail_res, &fail_head, list) {
 		struct resource *res = fail_res->res;
 
@@ -2147,7 +2139,7 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
 	return 0;
 
 cleanup:
-	/* restore size and flags */
+	/* Restore size and flags */
 	list_for_each_entry(dev_res, &failed, list) {
 		struct resource *res = dev_res->res;
 
@@ -2179,8 +2171,8 @@ int pci_reassign_bridge_resources(struct pci_dev *bridge, unsigned long type)
 void pci_assign_unassigned_bus_resources(struct pci_bus *bus)
 {
 	struct pci_dev *dev;
-	LIST_HEAD(add_list); /* list of resources that
-					want additional resources */
+	/* List of resources that want additional resources */
+	LIST_HEAD(add_list);
 
 	down_read(&pci_bus_sem);
 	for_each_pci_bridge(dev, bus)
-- 
2.19.1


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

end of thread, other threads:[~2019-05-05 14:41 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20190505144001.8106-1-nicholas.johnson-opensource@outlook.com.au>
2019-05-05 14:40 ` [PATCH v5 1/5] PCI: Consider alignment of hot-added bridges when distributing resources Nicholas Johnson
2019-05-05 14:40 ` [PATCH v5 2/5] PCI: Modify extend_bridge_window() to set resource size directly Nicholas Johnson
2019-05-05 14:40 ` [PATCH v5 3/5] PCI: Fix bug resulting in double hpmemsize being assigned to MMIO window Nicholas Johnson
2019-05-05 14:41 ` [PATCH v5 4/5] PCI: Add pci=hpmemprefsize parameter to set MMIO_PREF size independently Nicholas Johnson
2019-05-05 14:41 ` [PATCH v5 5/5] PCI: Cleanup block comments in setup-bus.c to match kernel style Nicholas Johnson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).