On Tuesday, May 31, 2016 01:12:52 PM Roland Dreier wrote:
> Hi,
>
> I recently updated one of my systems from 3.10.y to 4.4.11, and
> discovered a regression that stops it from booting. It's actually
> very similar to https://bugzilla.kernel.org/show_bug.cgi?id=99831
> (which I reported about the same system last year).
>
> The problem is that commit ac212b6980d8 ("ACPI / processor: Use common
> hotplug infrastructure") changes the order that the ACPI processor and
> PnP initialization run. pnp_system_init() is run at fs_initcall time,
> while acpi_processor_init() is run from acpi_scan_init(), earlier at
> subsys_initcall time. Pre-ac212b6980d8, the ACPI processor
> initialization all ran from acpi_processor_init() at module_init time.
> So the processor driver initialization has flipped from after to
> before pnp_system_init().
>
> Just as before, the failure is that the resource allocation code puts
> some AHCI IO BARs around 0x400, and reservation fails because some
> other ACPI stuff is also there. The problem is that when
> acpi_processor_init()
> runs, it reserves a range 0x410 - 0x415 for "ACPI CPU throttle", and
> if that happens before pnp_system_init(), then I get
>
> system 00:01: [io 0x0400-0x047f] could not be reserved
>
> because that overlaps the already-reserved range. Then the PCI
> resource allocation code is free to put PCI resources into that range
> and tons of things go south after that.
Definitely the request_region() in acpi_processor.c is a bug as that file
should be about enumeration only (and we don't even know whether or not
the region will be actually used at that point).
> For now I've worked around it by commenting out the request_region()
> in acpi_processor.c but that doesn't seem like a very good long-term
> solution. Does it make sense to resurrect the patches you had to let
> ACPI and PnP coexist in resource reservation? Or could we move the
> request_region() for CPU throttle into the still-modular
> initialization done from acpi_processor_driver_init()?
In ptinciple, that can be done.
Can you please try the appended patch (untested)?
Thanks,
Rafael
---
drivers/acpi/acpi_processor.c | 9 ---------
drivers/acpi/processor_throttling.c | 9 +++++++++
2 files changed, 9 insertions(+), 9 deletions(-)
Index: linux-pm/drivers/acpi/acpi_processor.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpi_processor.c
+++ linux-pm/drivers/acpi/acpi_processor.c
@@ -331,15 +331,6 @@ static int acpi_processor_get_info(struc
pr->throttling.duty_width = acpi_gbl_FADT.duty_width;
pr->pblk = object.processor.pblk_address;
-
- /*
- * We don't care about error returns - we just try to mark
- * these reserved so that nobody else is confused into thinking
- * that this region might be unused..
- *
- * (In particular, allocating the IO range for Cardbus)
- */
- request_region(pr->throttling.address, 6, "ACPI CPU throttle");
}
/*
Index: linux-pm/drivers/acpi/processor_throttling.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_throttling.c
+++ linux-pm/drivers/acpi/processor_throttling.c
@@ -676,6 +676,15 @@ static int acpi_processor_get_throttling
if (!pr->flags.throttling)
return -ENODEV;
+ /*
+ * We don't care about error returns - we just try to mark
+ * these reserved so that nobody else is confused into thinking
+ * that this region might be unused..
+ *
+ * (In particular, allocating the IO range for Cardbus)
+ */
+ request_region(pr->throttling.address, 6, "ACPI CPU throttle");
+
pr->throttling.state = 0;
duty_mask = pr->throttling.state_count - 1;
