Re: [PATCH] ACPI: don't walk tables if ACPI was disabled

[Ingo Molnar <mingo@elte.hu>]

* Vegard Nossum <vegard.nossum@gmail.com> wrote:

> On Fri, Jun 20, 2008 at 11:27 PM, Vegard Nossum <vegard.nossum@gmail.com> wrote:
> > So I guess this function, pnpbios_init() needs the check as well. In
> > fact, it has this:
> >
> > #ifdef CONFIG_PNPACPI
> >        if (!acpi_disabled && !pnpacpi_disabled) {
> >                pnpbios_disabled = 1;
> >                printk(KERN_INFO "PnPBIOS: Disabled by ACPI PNP\n");
> >                return -ENODEV;
> >        }
> > #endif                          /* CONFIG_ACPI */
> >
> > ...I guess that should be changed to say if (acpi_disabled ||
> > pnpacpi_disabled)? Or... I don't understand the purpose of the
> > original test. But it seems to be there since the beginning of time
> > (or, well, v2.6.12-rc2).
> 
> Nope. I found the introduction of the change in the historical git repository:
> 
> commit 4723ebe898a32262ed49fe677897ccea47e72ff4
> Author: Adam Belay <ambx1@neo.rr.com>
> Date:   Sun Oct 24 15:07:32 2004 -0400
> 
>     [PNPBIOS] disable if ACPI is active
> 
>     As further ACPI pnp functionaility is implemented it is no longer
>     safe to run ACPI and PNPBIOS concurrently.
> 
>     We therefore take the following approach:
>     - attempt to enable ACPI support
>     - if ACPI fails (blacklist etc.) enable pnpbios support
>     - if ACPI support is not compiled in the kernel enable pnpbios support
> 
>     Signed-off-by: Adam Belay <ambx1@neo.rr.com>
> 
> and now I understand the purpose of the check; pnpbios does not depend 
> on ACPI; ACPI/pnpacpi is incompatible with pnpbios.

wow, rather old bug - i guess lockdep made it more visible.

> Yet it remains a fact that pnpbios will discover devices which then 
> ACPI code uses erroneously. Which means that my original fix for Ingo 
> probably is the right one after all. Should I submit another patch 
> which does the right thing for everything under drivers/acpi/, or can 
> you do it on your own? :-)

i havent seen the warning reappear with your fix after thousands of 
bootups - so i guess we can consider it fixed.

Len, please consider the patch below. (it's in tip/out-of-tree)

	Ingo

----------------->
commit acc85833791a5d8f84b8df601afc1cc44568dd18
Author: Vegard Nossum <vegard.nossum@gmail.com>
Date:   Fri Jun 20 15:56:40 2008 +0200

    ACPI: don't walk tables if ACPI was disabled
    
    Ingo Molnar wrote:
    > -tip auto-testing started triggering this spinlock corruption message
    > yesterday:
    >
    > [    3.976213] calling  acpi_rtc_init+0x0/0xd3
    > [    3.980213] ACPI Exception (utmutex-0263): AE_BAD_PARAMETER, Thread F7C50000 could not acquire Mutex [3] [20080321]
    > [    3.992213] BUG: spinlock bad magic on CPU#0, swapper/1
    > [    3.992213]  lock: c2508dc4, .magic: 00000000, .owner: swapper/1, .owner_cpu: 0
    
    This is apparently because some parts of ACPI, including mutexes, are not
    initialized when acpi=off is passed to the kernel.
    
    Reported-by: Ingo Molnar <mingo@elte.hu>
    Signed-off-by: Vegard Nossum <vegard.nossum@gmail.com>
    Cc: Len Brown <lenb@kernel.org>
    Cc: Zhao Yakui <yakui.zhao@intel.com>
    Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
    Cc: Alexey Starikovskiy <astarikovskiy@suse.de>
    Cc: Yinghai Lu <yhlu.kernel@gmail.com>
    Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/drivers/acpi/glue.c b/drivers/acpi/glue.c
index b4d4ce0..c3e1eeb 100644
--- a/drivers/acpi/glue.c
+++ b/drivers/acpi/glue.c
@@ -334,6 +334,9 @@ static int __init acpi_rtc_init(void)
 {
 	struct device *dev = get_rtc_dev();
 
+	if (acpi_disabled)
+		return 0;
+
 	if (dev) {
 		rtc_wake_setup();
 		rtc_info.wake_on = rtc_wake_on;
diff --git a/drivers/acpi/namespace/nsxfeval.c b/drivers/acpi/namespace/nsxfeval.c
index a8d5491..c274d1d 100644
--- a/drivers/acpi/namespace/nsxfeval.c
+++ b/drivers/acpi/namespace/nsxfeval.c
@@ -391,6 +391,9 @@ acpi_walk_namespace(acpi_object_type type,
 
 	ACPI_FUNCTION_TRACE(acpi_walk_namespace);
 
+	if (acpi_disabled)
+		return_ACPI_STATUS(AE_NO_NAMESPACE);
+
 	/* Parameter validation */
 
 	if ((type > ACPI_TYPE_LOCAL_MAX) || (!max_depth) || (!user_function)) {