Unfortunately I just got around to trying linux 4.1.6 (which contains this commit) on the Versalogic Tiger (VL-EPM-24) http://www.versalogic.com/tiger and this commit breaks it, seems to be back to the old behavior reported here: http://marc.info/?t=143092384600002&r=1&w=2 Before reversing the commit it shows: pci 0000:00:1c.0: BAR 13: assigned [io 0x2000-0x2fff] After reversing the commit it shows: pci 0000:00:1c.0: BAR 13: assigned [io 0x3000-0x3fff] I don't have a solution to propose at this point and haven't tried the latest upstream so I'll just be building with this commit reversed (which solves the problem) until I have a chance to look into this further. Regards, George McCollister On Fri, Jul 3, 2015 at 8:09 PM, Rafael J. Wysocki <rjw@xxxxxxxxxxxxx> wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > > This effectively reverts the following three commits: > > 7bc10388ccdd ACPI / resources: free memory on error in add_region_before() > 0f1b414d1907 ACPI / PNP: Avoid conflicting resource reservations > b9a5e5e18fbf ACPI / init: Fix the ordering of acpi_reserve_resources() > > (commit b9a5e5e18fbf introduced regressions some of which, but not > all, were addressed by commit 0f1b414d1907 and commit 7bc10388ccdd > was a fixup on top of the latter) and causes ACPI fixed hardware > resources to be reserved at the fs_initcall_sync stage of system > initialization. > > The story is as follows. First, a boot regression was reported due > to an apparent resource reservation ordering change after a commit > that shouldn't lead to such changes. Investigation led to the > conclusion that the problem happened because acpi_reserve_resources() > was executed at the device_initcall() stage of system initialization > which wasn't strictly ordered with respect to driver initialization > (and with respect to the initialization of the pcieport driver in > particular), so a random change causing the device initcalls to be > run in a different order might break things. > > The response to that was to attempt to run acpi_reserve_resources() > as soon as we knew that ACPI would be in use (commit b9a5e5e18fbf). > However, that turned out to be too early, because it caused resource > reservations made by the PNP system driver to fail on at least one > system and that failure was addressed by commit 0f1b414d1907. > > That fix still turned out to be insufficient, though, because > calling acpi_reserve_resources() before the fs_initcall stage of > system initialization caused a boot regression to happen on the > eCAFE EC-800-H20G/S netbook. That meant that we only could call > acpi_reserve_resources() at the fs_initcall initialization stage > or later, but then we might just as well call it after the PNP > initalization in which case commit 0f1b414d1907 wouldn't be > necessary any more. > > For this reason, the changes made by commit 0f1b414d1907 are reverted > (along with a memory leak fixup on top of that commit), the changes > made by commit b9a5e5e18fbf that went too far are reverted too and > acpi_reserve_resources() is changed into fs_initcall_sync, which > will cause it to be executed after the PNP subsystem initialization > (which is an fs_initcall) and before device initcalls (including > the pcieport driver initialization) which should avoid the initial > issue. > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=100581 > Link: http://marc.info/?t=143092384600002&r=1&w=2 > Link: https://bugzilla.kernel.org/show_bug.cgi?id=99831 > Link: http://marc.info/?t=143389402600001&r=1&w=2 > Fixes: b9a5e5e18fbf "ACPI / init: Fix the ordering of acpi_reserve_resources()" > Reported-by: Roland Dreier <roland@xxxxxxxxxxxxxxx> > Reported-by-and-tested-by: <g5983969@xxxxxxxxx> > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx> > --- > drivers/acpi/osl.c | 12 ++- > drivers/acpi/resource.c | 162 ------------------------------------------------ > drivers/pnp/system.c | 35 ++-------- > include/linux/acpi.h | 10 -- > 4 files changed, 18 insertions(+), 201 deletions(-) > > Index: linux-pm/drivers/acpi/resource.c > =================================================================== > --- linux-pm.orig/drivers/acpi/resource.c > +++ linux-pm/drivers/acpi/resource.c > @@ -26,7 +26,6 @@ > #include <linux/device.h> > #include <linux/export.h> > #include <linux/ioport.h> > -#include <linux/list.h> > #include <linux/slab.h> > > #ifdef CONFIG_X86 > @@ -622,164 +621,3 @@ int acpi_dev_filter_resource_type(struct > return (type & types) ? 0 : 1; > } > EXPORT_SYMBOL_GPL(acpi_dev_filter_resource_type); > - > -struct reserved_region { > - struct list_head node; > - u64 start; > - u64 end; > -}; > - > -static LIST_HEAD(reserved_io_regions); > -static LIST_HEAD(reserved_mem_regions); > - > -static int request_range(u64 start, u64 end, u8 space_id, unsigned long flags, > - char *desc) > -{ > - unsigned int length = end - start + 1; > - struct resource *res; > - > - res = space_id == ACPI_ADR_SPACE_SYSTEM_IO ? > - request_region(start, length, desc) : > - request_mem_region(start, length, desc); > - if (!res) > - return -EIO; > - > - res->flags &= ~flags; > - return 0; > -} > - > -static int add_region_before(u64 start, u64 end, u8 space_id, > - unsigned long flags, char *desc, > - struct list_head *head) > -{ > - struct reserved_region *reg; > - int error; > - > - reg = kmalloc(sizeof(*reg), GFP_KERNEL); > - if (!reg) > - return -ENOMEM; > - > - error = request_range(start, end, space_id, flags, desc); > - if (error) { > - kfree(reg); > - return error; > - } > - > - reg->start = start; > - reg->end = end; > - list_add_tail(®->node, head); > - return 0; > -} > - > -/** > - * acpi_reserve_region - Reserve an I/O or memory region as a system resource. > - * @start: Starting address of the region. > - * @length: Length of the region. > - * @space_id: Identifier of address space to reserve the region from. > - * @flags: Resource flags to clear for the region after requesting it. > - * @desc: Region description (for messages). > - * > - * Reserve an I/O or memory region as a system resource to prevent others from > - * using it. If the new region overlaps with one of the regions (in the given > - * address space) already reserved by this routine, only the non-overlapping > - * parts of it will be reserved. > - * > - * Returned is either 0 (success) or a negative error code indicating a resource > - * reservation problem. It is the code of the first encountered error, but the > - * routine doesn't abort until it has attempted to request all of the parts of > - * the new region that don't overlap with other regions reserved previously. > - * > - * The resources requested by this routine are never released. > - */ > -int acpi_reserve_region(u64 start, unsigned int length, u8 space_id, > - unsigned long flags, char *desc) > -{ > - struct list_head *regions; > - struct reserved_region *reg; > - u64 end = start + length - 1; > - int ret = 0, error = 0; > - > - if (space_id == ACPI_ADR_SPACE_SYSTEM_IO) > - regions = &reserved_io_regions; > - else if (space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) > - regions = &reserved_mem_regions; > - else > - return -EINVAL; > - > - if (list_empty(regions)) > - return add_region_before(start, end, space_id, flags, desc, regions); > - > - list_for_each_entry(reg, regions, node) > - if (reg->start == end + 1) { > - /* The new region can be prepended to this one. */ > - ret = request_range(start, end, space_id, flags, desc); > - if (!ret) > - reg->start = start; > - > - return ret; > - } else if (reg->start > end) { > - /* No overlap. Add the new region here and get out. */ > - return add_region_before(start, end, space_id, flags, > - desc, ®->node); > - } else if (reg->end == start - 1) { > - goto combine; > - } else if (reg->end >= start) { > - goto overlap; > - } > - > - /* The new region goes after the last existing one. */ > - return add_region_before(start, end, space_id, flags, desc, regions); > - > - overlap: > - /* > - * The new region overlaps an existing one. > - * > - * The head part of the new region immediately preceding the existing > - * overlapping one can be combined with it right away. > - */ > - if (reg->start > start) { > - error = request_range(start, reg->start - 1, space_id, flags, desc); > - if (error) > - ret = error; > - else > - reg->start = start; > - } > - > - combine: > - /* > - * The new region is adjacent to an existing one. If it extends beyond > - * that region all the way to the next one, it is possible to combine > - * all three of them. > - */ > - while (reg->end < end) { > - struct reserved_region *next = NULL; > - u64 a = reg->end + 1, b = end; > - > - if (!list_is_last(®->node, regions)) { > - next = list_next_entry(reg, node); > - if (next->start <= end) > - b = next->start - 1; > - } > - error = request_range(a, b, space_id, flags, desc); > - if (!error) { > - if (next && next->start == b + 1) { > - reg->end = next->end; > - list_del(&next->node); > - kfree(next); > - } else { > - reg->end = end; > - break; > - } > - } else if (next) { > - if (!ret) > - ret = error; > - > - reg = next; > - } else { > - break; > - } > - } > - > - return ret ? ret : error; > -} > -EXPORT_SYMBOL_GPL(acpi_reserve_region); > Index: linux-pm/drivers/acpi/osl.c > =================================================================== > --- linux-pm.orig/drivers/acpi/osl.c > +++ linux-pm/drivers/acpi/osl.c > @@ -175,10 +175,14 @@ static void __init acpi_request_region ( > if (!addr || !length) > return; > > - acpi_reserve_region(addr, length, gas->space_id, 0, desc); > + /* Resources are never freed */ > + if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_IO) > + request_region(addr, length, desc); > + else if (gas->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) > + request_mem_region(addr, length, desc); > } > > -static void __init acpi_reserve_resources(void) > +static int __init acpi_reserve_resources(void) > { > acpi_request_region(&acpi_gbl_FADT.xpm1a_event_block, acpi_gbl_FADT.pm1_event_length, > "ACPI PM1a_EVT_BLK"); > @@ -207,7 +211,10 @@ static void __init acpi_reserve_resource > if (!(acpi_gbl_FADT.gpe1_block_length & 0x1)) > acpi_request_region(&acpi_gbl_FADT.xgpe1_block, > acpi_gbl_FADT.gpe1_block_length, "ACPI GPE1_BLK"); > + > + return 0; > } > +fs_initcall_sync(acpi_reserve_resources); > > void acpi_os_printf(const char *fmt, ...) > { > @@ -1862,7 +1869,6 @@ acpi_status __init acpi_os_initialize(vo > > acpi_status __init acpi_os_initialize1(void) > { > - acpi_reserve_resources(); > kacpid_wq = alloc_workqueue("kacpid", 0, 1); > kacpi_notify_wq = alloc_workqueue("kacpi_notify", 0, 1); > kacpi_hotplug_wq = alloc_ordered_workqueue("kacpi_hotplug", 0); > Index: linux-pm/drivers/pnp/system.c > =================================================================== > --- linux-pm.orig/drivers/pnp/system.c > +++ linux-pm/drivers/pnp/system.c > @@ -7,7 +7,6 @@ > * Bjorn Helgaas <bjorn.helgaas@xxxxxx> > */ > > -#include <linux/acpi.h> > #include <linux/pnp.h> > #include <linux/device.h> > #include <linux/init.h> > @@ -23,41 +22,25 @@ static const struct pnp_device_id pnp_de > {"", 0} > }; > > -#ifdef CONFIG_ACPI > -static bool __reserve_range(u64 start, unsigned int length, bool io, char *desc) > -{ > - u8 space_id = io ? ACPI_ADR_SPACE_SYSTEM_IO : ACPI_ADR_SPACE_SYSTEM_MEMORY; > - return !acpi_reserve_region(start, length, space_id, IORESOURCE_BUSY, desc); > -} > -#else > -static bool __reserve_range(u64 start, unsigned int length, bool io, char *desc) > -{ > - struct resource *res; > - > - res = io ? request_region(start, length, desc) : > - request_mem_region(start, length, desc); > - if (res) { > - res->flags &= ~IORESOURCE_BUSY; > - return true; > - } > - return false; > -} > -#endif > - > static void reserve_range(struct pnp_dev *dev, struct resource *r, int port) > { > char *regionid; > const char *pnpid = dev_name(&dev->dev); > resource_size_t start = r->start, end = r->end; > - bool reserved; > + struct resource *res; > > regionid = kmalloc(16, GFP_KERNEL); > if (!regionid) > return; > > snprintf(regionid, 16, "pnp %s", pnpid); > - reserved = __reserve_range(start, end - start + 1, !!port, regionid); > - if (!reserved) > + if (port) > + res = request_region(start, end - start + 1, regionid); > + else > + res = request_mem_region(start, end - start + 1, regionid); > + if (res) > + res->flags &= ~IORESOURCE_BUSY; > + else > kfree(regionid); > > /* > @@ -66,7 +49,7 @@ static void reserve_range(struct pnp_dev > * have double reservations. > */ > dev_info(&dev->dev, "%pR %s reserved\n", r, > - reserved ? "has been" : "could not be"); > + res ? "has been" : "could not be"); > } > > static void reserve_resources_of_dev(struct pnp_dev *dev) > Index: linux-pm/include/linux/acpi.h > =================================================================== > --- linux-pm.orig/include/linux/acpi.h > +++ linux-pm/include/linux/acpi.h > @@ -309,9 +309,6 @@ int acpi_check_region(resource_size_t st > > int acpi_resources_are_enforced(void); > > -int acpi_reserve_region(u64 start, unsigned int length, u8 space_id, > - unsigned long flags, char *desc); > - > #ifdef CONFIG_HIBERNATION > void __init acpi_no_s4_hw_signature(void); > #endif > @@ -507,13 +504,6 @@ static inline int acpi_check_region(reso > return 0; > } > > -static inline int acpi_reserve_region(u64 start, unsigned int length, > - u8 space_id, unsigned long flags, > - char *desc) > -{ > - return -ENXIO; > -} > - > struct acpi_table_header; > static inline int acpi_table_parse(char *id, > int (*handler)(struct acpi_table_header *)) > > -- > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
