* [PATCH v2 1/4] PCI: Consider alignment of hot-added bridges when distributing available resources
[not found] <20190311162154.3852-1-nicholas.johnson-opensource@outlook.com.au>
@ 2019-03-11 16:23 ` Nicholas Johnson
2019-03-11 16:23 ` [PATCH v2 2/4] PCI: Cleanup comments in setup-bus.c to meet kernel coding style guidelines Nicholas Johnson
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Nicholas Johnson @ 2019-03-11 16:23 UTC (permalink / raw)
To: linux-kernel
Cc: linux-pci, bhelgaas, mika.westerberg, corbet, 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 ed960436d..75827fb06 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1852,34 +1852,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;
@@ -1889,25 +1903,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
@@ -1921,80 +1942,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;
}
}
@@ -2002,22 +2022,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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/4] PCI: Cleanup comments in setup-bus.c to meet kernel coding style guidelines
[not found] <20190311162154.3852-1-nicholas.johnson-opensource@outlook.com.au>
2019-03-11 16:23 ` [PATCH v2 1/4] PCI: Consider alignment of hot-added bridges when distributing available resources Nicholas Johnson
@ 2019-03-11 16:23 ` Nicholas Johnson
2019-04-10 22:06 ` Bjorn Helgaas
2019-03-11 16:23 ` [PATCH v2 3/4] PCI: Fix serious bug when sizing bridges with additional size Nicholas Johnson
2019-03-11 16:23 ` [PATCH v2 4/4] PCI: modify kernel parameters to differentiate between MMIO and MMIO_PREF sizes Nicholas Johnson
3 siblings, 1 reply; 6+ messages in thread
From: Nicholas Johnson @ 2019-03-11 16:23 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.
Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
---
drivers/pci/setup-bus.c | 265 ++++++++++++++++++++--------------------
1 file changed, 130 insertions(+), 135 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 75827fb06..0ce641282 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)
{
u16 io;
@@ -752,9 +751,11 @@ static void pci_bridge_check_ranges(struct pci_bus *bus)
if (io)
b_res[0].flags |= IORESOURCE_IO;
- /* DECchip 21050 pass 2 errata: the bridge may miss an address
- disconnect boundary by one PCI data phase.
- Workaround: do not use prefetching on this device. */
+ /*
+ * DECchip 21050 pass 2 errata: the bridge may miss an address
+ * disconnect boundary by one PCI data phase. Workaround: do not use
+ * prefetching on this device.
+ */
if (bridge->vendor == PCI_VENDOR_ID_DEC && bridge->device == 0x0001)
return;
@@ -789,10 +790,11 @@ 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). */
+/*
+ * 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,
unsigned long type_mask, unsigned long type)
{
@@ -820,8 +822,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
@@ -870,8 +874,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;
@@ -891,10 +895,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)
@@ -995,12 +998,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,
@@ -1010,7 +1013,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_free_bus_resource(bus,
mask | IORESOURCE_PREFETCH, type);
@@ -1050,10 +1053,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;
@@ -1066,8 +1068,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)
@@ -1128,8 +1132,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;
@@ -1161,10 +1165,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;
@@ -1175,9 +1176,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;
@@ -1256,9 +1256,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;
@@ -1271,9 +1270,9 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
additional_mem_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;
@@ -1308,18 +1307,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_mem_size,
@@ -1352,8 +1350,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)
{
@@ -1464,10 +1462,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);
@@ -1532,16 +1529,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;
@@ -1561,10 +1557,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;
@@ -1589,8 +1582,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,
@@ -1751,8 +1744,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;
@@ -1774,13 +1767,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. */
@@ -2040,8 +2034,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;
@@ -2051,9 +2046,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);
@@ -2205,8 +2200,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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/4] PCI: Cleanup comments in setup-bus.c to meet kernel coding style guidelines
2019-03-11 16:23 ` [PATCH v2 2/4] PCI: Cleanup comments in setup-bus.c to meet kernel coding style guidelines Nicholas Johnson
@ 2019-04-10 22:06 ` Bjorn Helgaas
0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2019-04-10 22:06 UTC (permalink / raw)
To: Nicholas Johnson; +Cc: linux-kernel, linux-pci, mika.westerberg, corbet
Hi Nicholas,
On Mon, Mar 11, 2019 at 04:23:23PM +0000, Nicholas Johnson wrote:
> 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.
Thanks for doing this; I do accept cleanup patches like this. It does
simplify things if they are last in a series because that makes it
less likely that the "real" patches will cause conflicts.
Don't repost this series just to fix that; I fixed the (trivial)
conflict by hand already. Just FYI for the future :)
Bjorn
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 3/4] PCI: Fix serious bug when sizing bridges with additional size
[not found] <20190311162154.3852-1-nicholas.johnson-opensource@outlook.com.au>
2019-03-11 16:23 ` [PATCH v2 1/4] PCI: Consider alignment of hot-added bridges when distributing available resources Nicholas Johnson
2019-03-11 16:23 ` [PATCH v2 2/4] PCI: Cleanup comments in setup-bus.c to meet kernel coding style guidelines Nicholas Johnson
@ 2019-03-11 16:23 ` Nicholas Johnson
2019-03-11 16:23 ` [PATCH v2 4/4] PCI: modify kernel parameters to differentiate between MMIO and MMIO_PREF sizes Nicholas Johnson
3 siblings, 0 replies; 6+ messages in thread
From: Nicholas Johnson @ 2019-03-11 16:23 UTC (permalink / raw)
To: linux-kernel
Cc: linux-pci, bhelgaas, mika.westerberg, corbet, 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 | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index 0ce641282..efe899b02 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -791,11 +791,16 @@ 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).
+ * 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_free_bus_resource(struct pci_bus *bus,
+static struct resource *find_bus_resource_of_type(struct pci_bus *bus,
unsigned long type_mask, unsigned long type)
{
int i;
@@ -804,7 +809,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;
@@ -903,14 +908,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) {
@@ -1015,7 +1022,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;
@@ -1023,6 +1030,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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 4/4] PCI: modify kernel parameters to differentiate between MMIO and MMIO_PREF sizes
[not found] <20190311162154.3852-1-nicholas.johnson-opensource@outlook.com.au>
` (2 preceding siblings ...)
2019-03-11 16:23 ` [PATCH v2 3/4] PCI: Fix serious bug when sizing bridges with additional size Nicholas Johnson
@ 2019-03-11 16:23 ` Nicholas Johnson
2019-04-10 22:52 ` Bjorn Helgaas
3 siblings, 1 reply; 6+ messages in thread
From: Nicholas Johnson @ 2019-03-11 16:23 UTC (permalink / raw)
To: linux-kernel
Cc: linux-pci, bhelgaas, mika.westerberg, corbet, Nicholas Johnson
Add new kernel parameters:
- hp_io_size=nn
- hp_mmio_size=nnM
- hp_mmio_pref_size=nnM
Depreciate old kernel parameters and print warning when used:
- hpiosize=nn
- hpmemsize=nnM
Make hp_io_size function identically to hpiosize.
Make hp_mmio_size set additional size for MMIO bridge window.
Make hp_mmio_pref_size set additional size for MMIO_PREF bridge window.
Override old kernel parameters if any of the new ones are specified.
Make sure old kernel parameters still work identically when none of the
new kernel parameters are specified.
Make sure it is easy to remove old kernel parameters from the source
when it is deemed that sufficient time has passed to allow users to
migrate to the new kernel parameters.
Update documentation to reflect changes.
Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
---
.../admin-guide/kernel-parameters.txt | 21 ++++++++--
drivers/pci/pci.c | 39 +++++++++++++++++--
drivers/pci/setup-bus.c | 26 +++++++------
include/linux/pci.h | 3 +-
4 files changed, 69 insertions(+), 20 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 858b6c0b9..29406ae8f 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -3326,12 +3326,27 @@
the default.
off: Turn ECRC off
on: Turn ECRC on.
- hpiosize=nn[KMG] The fixed amount of bus space which is
+
+ hpiosize=nn[KMG] Depreciated. Overridden by hp_io_size
+ value if any of hp_io_size, hp_mmio_size
+ or hp_mmio_pref_size are used. This sets
+ hp_io_size to the given value if not
+ overridden.
+
+ hpmemsize=nn[KMG] Depreciated. Overridden if any of
+ hp_io_size, hp_mmio_size or
+ hp_mmio_pref_size are used. This sets
+ hp_mmio_size and hp_mmio_pref_size to
+ the given value if not overridden.
+ hp_io_size=nn[KMG] The fixed amount of bus space which is
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.
+ hp_mmio_size=nn[KMG] The fixed amount of bus space which is
+ reserved for hotplug bridge's 32-bit mmio window.
Default size is 2 megabytes.
+ hp_mmio_pref_size=nn[KMG] The fixed amount of bus space
+ which is reserved for hotplug bridge's 64-bit
+ mmio_pref window. Default size is 2 megabytes.
hpbussize=nn The minimum amount of additional bus numbers
reserved for buses below a hotplug bridge.
Default is 1.
diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index c25acace7..64978bf84 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -85,12 +85,15 @@ 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)
-/* pci=hpmemsize=nnM,hpiosize=nn can override this */
+#define DEFAULT_HOTPLUG_MMIO_SIZE (2*1024*1024)
+#define DEFAULT_HOTPLUG_MMIO_PREF_SIZE (2*1024*1024)
+/* Override with pci=hp_io_size=nn,hp_mmio_size=nnM,hp_mmio_pref_size=nnM */
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
+/* Override with pci=hpbussize=nn,assign-busses */
unsigned long pci_hotplug_bus_size = DEFAULT_HOTPLUG_BUS_SIZE;
enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_DEFAULT;
@@ -6144,6 +6147,15 @@ EXPORT_SYMBOL(pci_fixup_cardbus);
static int __init pci_setup(char *str)
{
+ /*
+ * Depreciated pci=hpmemsize=nnM but keep the functionality for now.
+ * If none of hp_io_size, hp_mmio_size or hp_mmio_pref_size are set
+ * then override hp_mmio_size and hp_mmio_pref_size with hpmemsize.
+ */
+ unsigned long pci_hotplug_io_size_old = DEFAULT_HOTPLUG_IO_SIZE;
+ unsigned long pci_hotplug_mem_size = DEFAULT_HOTPLUG_MMIO_SIZE;
+ bool use_new_pci_hotplug_params = 0;
+
while (str) {
char *k = strchr(str, ',');
if (k)
@@ -6176,9 +6188,23 @@ static int __init pci_setup(char *str)
} else if (!strncmp(str, "ecrc=", 5)) {
pcie_ecrc_get_policy(str + 5);
} else if (!strncmp(str, "hpiosize=", 9)) {
- pci_hotplug_io_size = memparse(str + 9, &str);
+ printk(KERN_WARNING "PCI: Depreciated option 'hpiosize', use 'hp_io_size' instead\n");
+ pci_hotplug_io_size_old =
+ memparse(str + 9, &str);
} else if (!strncmp(str, "hpmemsize=", 10)) {
+ printk(KERN_WARNING "PCI: Depreciated option 'hpmemsize', use 'hp_mmio_size' and 'hp_mmio_pref_size' instead\n");
pci_hotplug_mem_size = memparse(str + 10, &str);
+ } else if (!strncmp(str, "hp_io_size=", 11)) {
+ use_new_pci_hotplug_params = 1;
+ pci_hotplug_io_size = memparse(str + 11, &str);
+ } else if (!strncmp(str, "hp_mmio_size=", 13)) {
+ use_new_pci_hotplug_params = 1;
+ pci_hotplug_mmio_size =
+ memparse(str + 13, &str);
+ } else if (!strncmp(str, "hp_mmio_pref_size=", 18)) {
+ use_new_pci_hotplug_params = 1;
+ pci_hotplug_mmio_pref_size =
+ memparse(str + 18, &str);
} else if (!strncmp(str, "hpbussize=", 10)) {
pci_hotplug_bus_size =
simple_strtoul(str + 10, &str, 0);
@@ -6204,6 +6230,11 @@ static int __init pci_setup(char *str)
}
str = k;
}
+ if (!use_new_pci_hotplug_params) {
+ pci_hotplug_io_size = pci_hotplug_io_size_old;
+ pci_hotplug_mmio_size = pci_hotplug_mem_size;
+ pci_hotplug_mmio_pref_size = pci_hotplug_mem_size;
+ }
return 0;
}
early_param("pci", pci_setup);
diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
index efe899b02..56d227227 100644
--- a/drivers/pci/setup-bus.c
+++ b/drivers/pci/setup-bus.c
@@ -1223,7 +1223,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;
@@ -1256,8 +1257,9 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
case PCI_CLASS_BRIDGE_PCI:
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_io_size = pci_hotplug_io_size;
+ additional_mmio_size = pci_hotplug_mmio_size;
+ additional_mmio_pref_size = pci_hotplug_mmio_pref_size;
}
/* Fall through */
default:
@@ -1275,9 +1277,8 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
prefmask |= IORESOURCE_MEM_64;
ret = pbus_size_mem(bus, prefmask, prefmask,
prefmask, prefmask,
- realloc_head ? 0 : additional_mem_size,
- additional_mem_size, realloc_head);
-
+ realloc_head ? 0 : additional_mmio_pref_size,
+ additional_mmio_pref_size, realloc_head);
/*
* If successful, all non-prefetchable resources and any
* 32-bit prefetchable resources will go in the
@@ -1298,9 +1299,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
@@ -1309,7 +1310,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;
}
@@ -1329,8 +1331,8 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
* prefetchable resource in a 64-bit prefetchable 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 65f1d8c2f..b30d55697 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1959,7 +1959,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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 4/4] PCI: modify kernel parameters to differentiate between MMIO and MMIO_PREF sizes
2019-03-11 16:23 ` [PATCH v2 4/4] PCI: modify kernel parameters to differentiate between MMIO and MMIO_PREF sizes Nicholas Johnson
@ 2019-04-10 22:52 ` Bjorn Helgaas
0 siblings, 0 replies; 6+ messages in thread
From: Bjorn Helgaas @ 2019-04-10 22:52 UTC (permalink / raw)
To: Nicholas Johnson; +Cc: linux-kernel, linux-pci, mika.westerberg, corbet
On Mon, Mar 11, 2019 at 04:23:55PM +0000, Nicholas Johnson wrote:
> Add new kernel parameters:
> - hp_io_size=nn
> - hp_mmio_size=nnM
> - hp_mmio_pref_size=nnM
>
> Depreciate old kernel parameters and print warning when used:
> - hpiosize=nn
> - hpmemsize=nnM
s/Depreciate/Deprecate/ (several occurrences)
> Make hp_io_size function identically to hpiosize.
> Make hp_mmio_size set additional size for MMIO bridge window.
> Make hp_mmio_pref_size set additional size for MMIO_PREF bridge window.
Help me understand this -- I think the point of this is today,
a value specified with "hpmemsize=" applies to both the MMIO
(non-prefetchable) and the MMIO_PREF window, and you want to control
them separately, right?
That seems reasonable. I also agree that your proposed new parameter
names are nicer than the existing ones. But the renames make this
patch much bigger than it would be otherwise, and I'm not really
convinced that the user confusion of deprecating the old ones is worth
the trouble.
What I would suggest is either dropping the rename or splitting this
into two patches:
1) add "hpmemprefsize=" parameter
2) rename the parameters
Then the first patch would be a little smaller but would have all the
important functionality, and the second would be pretty easy to review
because it only changes names.
> Override old kernel parameters if any of the new ones are specified.
>
> Make sure old kernel parameters still work identically when none of the
> new kernel parameters are specified.
>
> Make sure it is easy to remove old kernel parameters from the source
> when it is deemed that sufficient time has passed to allow users to
> migrate to the new kernel parameters.
>
> Update documentation to reflect changes.
>
> Signed-off-by: Nicholas Johnson <nicholas.johnson-opensource@outlook.com.au>
> ---
> .../admin-guide/kernel-parameters.txt | 21 ++++++++--
> drivers/pci/pci.c | 39 +++++++++++++++++--
> drivers/pci/setup-bus.c | 26 +++++++------
> include/linux/pci.h | 3 +-
> 4 files changed, 69 insertions(+), 20 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 858b6c0b9..29406ae8f 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3326,12 +3326,27 @@
> the default.
> off: Turn ECRC off
> on: Turn ECRC on.
> - hpiosize=nn[KMG] The fixed amount of bus space which is
> +
> + hpiosize=nn[KMG] Depreciated. Overridden by hp_io_size
> + value if any of hp_io_size, hp_mmio_size
> + or hp_mmio_pref_size are used. This sets
> + hp_io_size to the given value if not
> + overridden.
> +
> + hpmemsize=nn[KMG] Depreciated. Overridden if any of
> + hp_io_size, hp_mmio_size or
> + hp_mmio_pref_size are used. This sets
> + hp_mmio_size and hp_mmio_pref_size to
> + the given value if not overridden.
> + hp_io_size=nn[KMG] The fixed amount of bus space which is
> 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.
> + hp_mmio_size=nn[KMG] The fixed amount of bus space which is
> + reserved for hotplug bridge's 32-bit mmio window.
> Default size is 2 megabytes.
> + hp_mmio_pref_size=nn[KMG] The fixed amount of bus space
> + which is reserved for hotplug bridge's 64-bit
> + mmio_pref window. Default size is 2 megabytes.
> hpbussize=nn The minimum amount of additional bus numbers
> reserved for buses below a hotplug bridge.
> Default is 1.
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index c25acace7..64978bf84 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -85,12 +85,15 @@ 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)
> -/* pci=hpmemsize=nnM,hpiosize=nn can override this */
> +#define DEFAULT_HOTPLUG_MMIO_SIZE (2*1024*1024)
> +#define DEFAULT_HOTPLUG_MMIO_PREF_SIZE (2*1024*1024)
> +/* Override with pci=hp_io_size=nn,hp_mmio_size=nnM,hp_mmio_pref_size=nnM */
> 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
> +/* Override with pci=hpbussize=nn,assign-busses */
> unsigned long pci_hotplug_bus_size = DEFAULT_HOTPLUG_BUS_SIZE;
>
> enum pcie_bus_config_types pcie_bus_config = PCIE_BUS_DEFAULT;
> @@ -6144,6 +6147,15 @@ EXPORT_SYMBOL(pci_fixup_cardbus);
>
> static int __init pci_setup(char *str)
> {
> + /*
> + * Depreciated pci=hpmemsize=nnM but keep the functionality for now.
> + * If none of hp_io_size, hp_mmio_size or hp_mmio_pref_size are set
> + * then override hp_mmio_size and hp_mmio_pref_size with hpmemsize.
> + */
> + unsigned long pci_hotplug_io_size_old = DEFAULT_HOTPLUG_IO_SIZE;
> + unsigned long pci_hotplug_mem_size = DEFAULT_HOTPLUG_MMIO_SIZE;
> + bool use_new_pci_hotplug_params = 0;
> +
> while (str) {
> char *k = strchr(str, ',');
> if (k)
> @@ -6176,9 +6188,23 @@ static int __init pci_setup(char *str)
> } else if (!strncmp(str, "ecrc=", 5)) {
> pcie_ecrc_get_policy(str + 5);
> } else if (!strncmp(str, "hpiosize=", 9)) {
> - pci_hotplug_io_size = memparse(str + 9, &str);
> + printk(KERN_WARNING "PCI: Depreciated option 'hpiosize', use 'hp_io_size' instead\n");
> + pci_hotplug_io_size_old =
> + memparse(str + 9, &str);
> } else if (!strncmp(str, "hpmemsize=", 10)) {
> + printk(KERN_WARNING "PCI: Depreciated option 'hpmemsize', use 'hp_mmio_size' and 'hp_mmio_pref_size' instead\n");
> pci_hotplug_mem_size = memparse(str + 10, &str);
> + } else if (!strncmp(str, "hp_io_size=", 11)) {
> + use_new_pci_hotplug_params = 1;
> + pci_hotplug_io_size = memparse(str + 11, &str);
> + } else if (!strncmp(str, "hp_mmio_size=", 13)) {
> + use_new_pci_hotplug_params = 1;
> + pci_hotplug_mmio_size =
> + memparse(str + 13, &str);
> + } else if (!strncmp(str, "hp_mmio_pref_size=", 18)) {
> + use_new_pci_hotplug_params = 1;
> + pci_hotplug_mmio_pref_size =
> + memparse(str + 18, &str);
> } else if (!strncmp(str, "hpbussize=", 10)) {
> pci_hotplug_bus_size =
> simple_strtoul(str + 10, &str, 0);
> @@ -6204,6 +6230,11 @@ static int __init pci_setup(char *str)
> }
> str = k;
> }
> + if (!use_new_pci_hotplug_params) {
> + pci_hotplug_io_size = pci_hotplug_io_size_old;
> + pci_hotplug_mmio_size = pci_hotplug_mem_size;
> + pci_hotplug_mmio_pref_size = pci_hotplug_mem_size;
> + }
> return 0;
> }
> early_param("pci", pci_setup);
> diff --git a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
> index efe899b02..56d227227 100644
> --- a/drivers/pci/setup-bus.c
> +++ b/drivers/pci/setup-bus.c
> @@ -1223,7 +1223,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;
>
> @@ -1256,8 +1257,9 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
> case PCI_CLASS_BRIDGE_PCI:
> 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_io_size = pci_hotplug_io_size;
> + additional_mmio_size = pci_hotplug_mmio_size;
> + additional_mmio_pref_size = pci_hotplug_mmio_pref_size;
> }
> /* Fall through */
> default:
> @@ -1275,9 +1277,8 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
> prefmask |= IORESOURCE_MEM_64;
> ret = pbus_size_mem(bus, prefmask, prefmask,
> prefmask, prefmask,
> - realloc_head ? 0 : additional_mem_size,
> - additional_mem_size, realloc_head);
> -
> + realloc_head ? 0 : additional_mmio_pref_size,
> + additional_mmio_pref_size, realloc_head);
> /*
> * If successful, all non-prefetchable resources and any
> * 32-bit prefetchable resources will go in the
> @@ -1298,9 +1299,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
> @@ -1309,7 +1310,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;
> }
> @@ -1329,8 +1331,8 @@ void __pci_bus_size_bridges(struct pci_bus *bus, struct list_head *realloc_head)
> * prefetchable resource in a 64-bit prefetchable 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 65f1d8c2f..b30d55697 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1959,7 +1959,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
>
^ permalink raw reply [flat|nested] 6+ messages in thread