[PATCH v4 1/4] PCI: Consider alignment of hot-added bridges when distributing available resources

[PATCH v4 1/4] PCI: Consider alignment of hot-added bridges when distributing available resources

[Nicholas Johnson]

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.

Change extend_bridge_window to resize the actual resource, rather than
using add_list and dev_res->add_size. If an additional resource entry
exists for the given resource, zero out the add_size field to avoid it
interfering. Because add_size is considered optional when allocating,
using add_size could cause issues in some cases, because successful
resource distribution requires sizes to be guaranteed. Such cases
include hot-adding nested hotplug bridges in one enumeration, and
potentially others which are yet to be encountered.

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

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index ec44a0f3a..a1ca8a11f 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1815,34 +1815,48 @@ 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;
+	/*
+	 * 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.
+	 */
+	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;
+	res->end = res->start + new_size - 1;
 
-	dev_res->add_size = available - resource_size(res);
-	pci_dbg(bridge, "bridge window %pR extended by %pa\n", res,
-		&dev_res->add_size);
+	/*
+	 * If a list entry exists, we need to remove any additional size
+	 * requested because that could interfere with the alignment and
+	 * sizing done when distributing resources, causing resources to
+	 * fail to allocate later on.
+	 */
+	dev_res = res_to_dev_res(add_list, res);
+	if (dev_res)
+		dev_res->add_size = 0;
 }
 
 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 +1866,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 +1905,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 +1985,19 @@ 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;
 
+	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 */
-	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);
 
 	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.20.1

Re: [PATCH v4 1/4] PCI: Consider alignment of hot-added bridges when distributing available resources

[Bjorn Helgaas]

On Wed, Apr 17, 2019 at 02:16:36PM +0000, Nicholas Johnson wrote:
> 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.

It would be nice to have a concrete example including the actual
hotplug bridge windows and the resources required by a hot-added
device.  That would help show exactly what the current code is doing
wrong and how your patch fixes it.

> Change extend_bridge_window to resize the actual resource, rather than
> using add_list and dev_res->add_size. If an additional resource entry
> exists for the given resource, zero out the add_size field to avoid it
> interfering. Because add_size is considered optional when allocating,
> using add_size could cause issues in some cases, because successful
> resource distribution requires sizes to be guaranteed.

Could this be a separate patch, or is it impossible to separate from
the pci_bus_distribute_available_resources() changes?  If it could be
split, it would make it easier to review, backport, isolate problems,
etc.

> Such cases
> include hot-adding nested hotplug bridges in one enumeration, and
> potentially others which are yet to be encountered.

Obviously hot-adding nested bridges may require more resources, but
the connection to extend_bridge_window() and add_size is not obvious
to me.

> Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> ---
>  drivers/pci/setup-bus.c | 203 ++++++++++++++++++++++------------------
>  1 file changed, 110 insertions(+), 93 deletions(-)
> 
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index ec44a0f3a..a1ca8a11f 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1815,34 +1815,48 @@ 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;
> +	/*
> +	 * 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.
> +	 */
> +	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;
> +	res->end = res->start + new_size - 1;
>  
> -	dev_res->add_size = available - resource_size(res);
> -	pci_dbg(bridge, "bridge window %pR extended by %pa\n", res,
> -		&dev_res->add_size);
> +	/*
> +	 * If a list entry exists, we need to remove any additional size
> +	 * requested because that could interfere with the alignment and
> +	 * sizing done when distributing resources, causing resources to
> +	 * fail to allocate later on.
> +	 */
> +	dev_res = res_to_dev_res(add_list, res);
> +	if (dev_res)
> +		dev_res->add_size = 0;
>  }
>  
>  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 +1866,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 +1905,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 +1985,19 @@ 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;
>  
> +	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 */
> -	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);
>  
>  	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.20.1
> 

[PATCH v4 2/4] PCI: Fix serious bug when sizing bridges with additional size

[Nicholas Johnson]

Change find_free_bus_resource function 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 a1ca8a11f..5b9ee9945 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.20.1

Re: [PATCH v4 2/4] PCI: Fix serious bug when sizing bridges with additional size

[Bjorn Helgaas]

On Wed, Apr 17, 2019 at 02:16:50PM +0000, Nicholas Johnson wrote:
> Change find_free_bus_resource function 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.

A concrete example for this would be helpful, e.g., a dmesg log
showing space for a single resource being reserved multiple times in
different windows.  This is probably too big for a commit log, so you
could open an issue at bugzilla.kernel.org, attach the complete dmesg
logs there, maybe include snippets here, and include the URL for more
details.

General comment: in English text, include "()" on function names as a
hint to the reader, e.g., you can write

  Change find_free_bus_resource() to not ...

instead of

  Change find_free_bus_resource function ...

> 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 a1ca8a11f..5b9ee9945 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.20.1
> 

[PATCH v4 3/4] PCI: Modify kernel parameters to differentiate between MMIO and MMIO_PREF sizes

[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 only control MMIO size,
rather than controlling both MMIO and MMIO_PREF sizes.

Update kernel-parameters documentation to reflect the above changes.

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

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2b8ee90bb..f33b590c2 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3361,7 +3361,10 @@
 				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.
+				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..e0502831f 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;
@@ -6245,7 +6247,11 @@ 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);
+			} else if (!strncmp(str, "hpmemprefsize=", 14)) {
+				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 5b9ee9945..95149165a 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.20.1

Re: [PATCH v4 3/4] PCI: Modify kernel parameters to differentiate between MMIO and MMIO_PREF sizes

[Bjorn Helgaas]

On Wed, Apr 17, 2019 at 02:16:59PM +0000, Nicholas Johnson wrote:
> Add kernel parameter pci=hpmemprefsize=nn[KMG] to control MMIO_PREF
> size for PCI hotplug bridges.
> 
> Change behaviour of pci=hpmemsize=nn[KMG] to only control MMIO size,
> rather than controlling both MMIO and MMIO_PREF sizes.

If I understand correctly, this patch as-is changes the behavior of
just specifying "pci=hpmemsize", and I think that will require some
users to change their boot configuration.

If the user doesn't specify "pci=hpmemprefsize", I think we might want
to preserve the existing behavior, i.e.,

  - "pci=hpmemsize=XX" affects both MMIO and MMIO_PREF windows, as it
    does today

  - "pci=hpmemprefsize=XX" affects only MMIO_PREF window, use default
    for MMIO window size

  - "pci=hpmemsize=XX pci=hpmemprefsize=YY" sets MMIO window size to
    XX and MMIO_PREF window size to YY

That way a user will not see any change in behavior unless he/she
actually uses the new kernel parameter.

> Update kernel-parameters documentation to reflect the above changes.
> 
> Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> ---
>  .../admin-guide/kernel-parameters.txt         |  5 +++-
>  drivers/pci/pci.c                             | 12 ++++++---
>  drivers/pci/setup-bus.c                       | 25 +++++++++++--------
>  include/linux/pci.h                           |  3 ++-
>  4 files changed, 29 insertions(+), 16 deletions(-)
> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 2b8ee90bb..f33b590c2 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3361,7 +3361,10 @@
>  				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.
> +				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..e0502831f 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;
> @@ -6245,7 +6247,11 @@ 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);
> +			} else if (!strncmp(str, "hpmemprefsize=", 14)) {
> +				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 5b9ee9945..95149165a 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.20.1
> 

[PATCH v4 4/4] PCI: Cleanup block comments in setup-bus.c to meet kernel style guidelines

[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.

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

diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 95149165a..bdce5b734 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)
@@ -315,10 +311,9 @@ static unsigned long pci_fail_res_type_mask(struct list_head *fail_head)
 		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);
 }
@@ -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))
@@ -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. */
@@ -730,9 +727,11 @@ int pci_claim_bridge_resource(struct pci_dev *bridge, int i)
 	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);
@@ -1023,10 +1023,9 @@ static int pbus_size_mem(struct pci_bus *bus, unsigned long mask,
 			}
 #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;
@@ -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,10 +1530,7 @@ 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;
@@ -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,
@@ -1727,8 +1717,8 @@ static enum enable_type pci_realloc_detect(struct pci_bus *bus,
  */
 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;
@@ -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. */
@@ -2016,8 +2007,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;
@@ -2027,9 +2019,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);
 
@@ -2181,8 +2173,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.20.1