2015-06-03 17:33:43

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH] ACPI / init: Switch over platform to the ACPI mode later

On Sat, 2015-05-30 at 14:21 +0200, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Commit 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before
> timekeeping_init()" moved the ACPI subsystem initialization,
> including the ACPI mode enabling, to an earlier point in the
> initialization sequence, to allow the timekeeping subsystem
> use ACPI early. Unfortunately, that resulted in boot regressions
> on some systems and the early ACPI initialization was moved toward
> its original position in the kernel initialization code by commit
> c4e1acbb35e4 "ACPI / init: Invoke early ACPI initialization later".
>
> However, that turns out to be insufficient, as boot is still broken
> on the Tyan S8812 mainboard.
>
> To fix that issue, split the ACPI early initialization code into
> two pieces so the majority of it still located in acpi_early_init()
> and the part switching over the platform into the ACPI mode goes into
> a new function, acpi_subsystem_init(), executed at the original early
> ACPI initialization spot.
>
> That fixes the Tyan S8812 boot problem, but still allows ACPI
> tables to be loaded earlier which is useful to the EFI code in
> efi_enter_virtual_mode().
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=97141
> Reported-and-tested-by: Marius Tolzmann <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>

Can you add comments to acpi_early_init() and acpi_subsystem_init() to
clarify what ACPI features are enabled at each phase, and what
dependency they have in the boot sequence? (The same goes to
early_acpi_boot_init() and acpi_boot_init().)

Otherwise, the change looks good.

Acked-by: Toshi Kani <[email protected]>

Thanks,
-Toshi


2015-06-09 23:33:18

by Toshi Kani

[permalink] [raw]
Subject: Re: [PATCH] ACPI / init: Switch over platform to the ACPI mode later

On Wed, 2015-06-10 at 01:48 +0200, Rafael J. Wysocki wrote:
> On Wednesday, June 03, 2015 11:13:57 AM Toshi Kani wrote:
> > On Sat, 2015-05-30 at 14:21 +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <[email protected]>
> > >
> > > Commit 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before
> > > timekeeping_init()" moved the ACPI subsystem initialization,
> > > including the ACPI mode enabling, to an earlier point in the
> > > initialization sequence, to allow the timekeeping subsystem
> > > use ACPI early. Unfortunately, that resulted in boot regressions
> > > on some systems and the early ACPI initialization was moved toward
> > > its original position in the kernel initialization code by commit
> > > c4e1acbb35e4 "ACPI / init: Invoke early ACPI initialization later".
> > >
> > > However, that turns out to be insufficient, as boot is still broken
> > > on the Tyan S8812 mainboard.
> > >
> > > To fix that issue, split the ACPI early initialization code into
> > > two pieces so the majority of it still located in acpi_early_init()
> > > and the part switching over the platform into the ACPI mode goes into
> > > a new function, acpi_subsystem_init(), executed at the original early
> > > ACPI initialization spot.
> > >
> > > That fixes the Tyan S8812 boot problem, but still allows ACPI
> > > tables to be loaded earlier which is useful to the EFI code in
> > > efi_enter_virtual_mode().
> > >
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=97141
> > > Reported-and-tested-by: Marius Tolzmann <[email protected]>
> > > Signed-off-by: Rafael J. Wysocki <[email protected]>
> >
> > Can you add comments to acpi_early_init() and acpi_subsystem_init() to
> > clarify what ACPI features are enabled at each phase, and what
> > dependency they have in the boot sequence? (The same goes to
> > early_acpi_boot_init() and acpi_boot_init().)
>
> OK, update follows.

Very helpful. Thanks for the update.
-Toshi

2015-06-09 23:22:50

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] ACPI / init: Switch over platform to the ACPI mode later

On Wednesday, June 03, 2015 11:13:57 AM Toshi Kani wrote:
> On Sat, 2015-05-30 at 14:21 +0200, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Commit 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before
> > timekeeping_init()" moved the ACPI subsystem initialization,
> > including the ACPI mode enabling, to an earlier point in the
> > initialization sequence, to allow the timekeeping subsystem
> > use ACPI early. Unfortunately, that resulted in boot regressions
> > on some systems and the early ACPI initialization was moved toward
> > its original position in the kernel initialization code by commit
> > c4e1acbb35e4 "ACPI / init: Invoke early ACPI initialization later".
> >
> > However, that turns out to be insufficient, as boot is still broken
> > on the Tyan S8812 mainboard.
> >
> > To fix that issue, split the ACPI early initialization code into
> > two pieces so the majority of it still located in acpi_early_init()
> > and the part switching over the platform into the ACPI mode goes into
> > a new function, acpi_subsystem_init(), executed at the original early
> > ACPI initialization spot.
> >
> > That fixes the Tyan S8812 boot problem, but still allows ACPI
> > tables to be loaded earlier which is useful to the EFI code in
> > efi_enter_virtual_mode().
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=97141
> > Reported-and-tested-by: Marius Tolzmann <[email protected]>
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
>
> Can you add comments to acpi_early_init() and acpi_subsystem_init() to
> clarify what ACPI features are enabled at each phase, and what
> dependency they have in the boot sequence? (The same goes to
> early_acpi_boot_init() and acpi_boot_init().)

OK, update follows.

---
From: Rafael J. Wysocki <[email protected]>
Subject: ACPI / init: Switch over platform to the ACPI mode later

Commit 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before
timekeeping_init()" moved the ACPI subsystem initialization,
including the ACPI mode enabling, to an earlier point in the
initialization sequence, to allow the timekeeping subsystem
use ACPI early. Unfortunately, that resulted in boot regressions
on some systems and the early ACPI initialization was moved toward
its original position in the kernel initialization code by commit
c4e1acbb35e4 "ACPI / init: Invoke early ACPI initialization later".

However, that turns out to be insufficient, as boot is still broken
on the Tyan S8812 mainboard.

To fix that issue, split the ACPI early initialization code into
two pieces so the majority of it still located in acpi_early_init()
and the part switching over the platform into the ACPI mode goes into
a new function, acpi_subsystem_init(), executed at the original early
ACPI initialization spot.

That fixes the Tyan S8812 boot problem, but still allows ACPI
tables to be loaded earlier which is useful to the EFI code in
efi_enter_virtual_mode().

Link: https://bugzilla.kernel.org/show_bug.cgi?id=97141
Fixes: 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before timekeeping_init()"
Reported-and-tested-by: Marius Tolzmann <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
Acked-by: Toshi Kani <[email protected]>
---
drivers/acpi/bus.c | 56 +++++++++++++++++++++++++++++++++++++--------------
include/linux/acpi.h | 2 +
init/main.c | 1
3 files changed, 44 insertions(+), 15 deletions(-)

Index: linux-pm/drivers/acpi/bus.c
===================================================================
--- linux-pm.orig/drivers/acpi/bus.c
+++ linux-pm/drivers/acpi/bus.c
@@ -470,6 +470,16 @@ static int __init acpi_bus_init_irq(void
return 0;
}

+/**
+ * acpi_early_init - Initialize ACPICA and populate the ACPI namespace.
+ *
+ * The ACPI tables are accessible after this, but the handling of events has not
+ * been initialized and the global lock is not available yet, so AML should not
+ * be executed at this point.
+ *
+ * Doing this before switching the EFI runtime services to virtual mode allows
+ * the EfiBootServices memory to be freed slightly earlier on boot.
+ */
void __init acpi_early_init(void)
{
acpi_status status;
@@ -533,26 +543,42 @@ void __init acpi_early_init(void)
acpi_gbl_FADT.sci_interrupt = acpi_sci_override_gsi;
}
#endif
+ return;
+
+ error0:
+ disable_acpi();
+}
+
+/**
+ * acpi_subsystem_init - Finalize the early initialization of ACPI.
+ *
+ * Switch over the platform to the ACPI mode (if possible), initialize the
+ * handling of ACPI events, install the interrupt and global lock handlers.
+ *
+ * Doing this too early is generally unsafe, but at the same time it needs to be
+ * done before all things that really depend on ACPI. The right spot appears to
+ * be before finalizing the EFI initialization.
+ */
+void __init acpi_subsystem_init(void)
+{
+ acpi_status status;
+
+ if (acpi_disabled)
+ return;

status = acpi_enable_subsystem(~ACPI_NO_ACPI_ENABLE);
if (ACPI_FAILURE(status)) {
printk(KERN_ERR PREFIX "Unable to enable ACPI\n");
- goto error0;
+ disable_acpi();
+ } else {
+ /*
+ * If the system is using ACPI then we can be reasonably
+ * confident that any regulators are managed by the firmware
+ * so tell the regulator core it has everything it needs to
+ * know.
+ */
+ regulator_has_full_constraints();
}
-
- /*
- * If the system is using ACPI then we can be reasonably
- * confident that any regulators are managed by the firmware
- * so tell the regulator core it has everything it needs to
- * know.
- */
- regulator_has_full_constraints();
-
- return;
-
- error0:
- disable_acpi();
- return;
}

static int __init acpi_bus_init(void)
Index: linux-pm/include/linux/acpi.h
===================================================================
--- linux-pm.orig/include/linux/acpi.h
+++ linux-pm/include/linux/acpi.h
@@ -450,6 +450,7 @@ extern acpi_status acpi_pci_osc_control_
#define ACPI_OST_SC_INSERT_NOT_SUPPORTED 0x82

extern void acpi_early_init(void);
+extern void acpi_subsystem_init(void);

extern int acpi_nvs_register(__u64 start, __u64 size);

@@ -504,6 +505,7 @@ static inline const char *acpi_dev_name(
}

static inline void acpi_early_init(void) { }
+static inline void acpi_subsystem_init(void) { }

static inline int early_acpi_boot_init(void)
{
Index: linux-pm/init/main.c
===================================================================
--- linux-pm.orig/init/main.c
+++ linux-pm/init/main.c
@@ -664,6 +664,7 @@ asmlinkage __visible void __init start_k

check_bugs();

+ acpi_subsystem_init();
sfi_init_late();

if (efi_enabled(EFI_RUNTIME_SERVICES)) {

2015-06-10 02:06:56

by Hanjun Guo

[permalink] [raw]
Subject: Re: [PATCH] ACPI / init: Switch over platform to the ACPI mode later

On 06/10/2015 07:48 AM, Rafael J. Wysocki wrote:
> On Wednesday, June 03, 2015 11:13:57 AM Toshi Kani wrote:
>> On Sat, 2015-05-30 at 14:21 +0200, Rafael J. Wysocki wrote:
>>> From: Rafael J. Wysocki <[email protected]>
>>>
>>> Commit 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before
>>> timekeeping_init()" moved the ACPI subsystem initialization,
>>> including the ACPI mode enabling, to an earlier point in the
>>> initialization sequence, to allow the timekeeping subsystem
>>> use ACPI early. Unfortunately, that resulted in boot regressions
>>> on some systems and the early ACPI initialization was moved toward
>>> its original position in the kernel initialization code by commit
>>> c4e1acbb35e4 "ACPI / init: Invoke early ACPI initialization later".
>>>
>>> However, that turns out to be insufficient, as boot is still broken
>>> on the Tyan S8812 mainboard.
>>>
>>> To fix that issue, split the ACPI early initialization code into
>>> two pieces so the majority of it still located in acpi_early_init()
>>> and the part switching over the platform into the ACPI mode goes into
>>> a new function, acpi_subsystem_init(), executed at the original early
>>> ACPI initialization spot.
>>>
>>> That fixes the Tyan S8812 boot problem, but still allows ACPI
>>> tables to be loaded earlier which is useful to the EFI code in
>>> efi_enter_virtual_mode().
>>>
>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=97141
>>> Reported-and-tested-by: Marius Tolzmann <[email protected]>
>>> Signed-off-by: Rafael J. Wysocki <[email protected]>
>>
>> Can you add comments to acpi_early_init() and acpi_subsystem_init() to
>> clarify what ACPI features are enabled at each phase, and what
>> dependency they have in the boot sequence? (The same goes to
>> early_acpi_boot_init() and acpi_boot_init().)
>
> OK, update follows.
>
> ---
> From: Rafael J. Wysocki <[email protected]>
> Subject: ACPI / init: Switch over platform to the ACPI mode later
>
> Commit 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before
> timekeeping_init()" moved the ACPI subsystem initialization,
> including the ACPI mode enabling, to an earlier point in the
> initialization sequence, to allow the timekeeping subsystem
> use ACPI early. Unfortunately, that resulted in boot regressions
> on some systems and the early ACPI initialization was moved toward
> its original position in the kernel initialization code by commit
> c4e1acbb35e4 "ACPI / init: Invoke early ACPI initialization later".
>
> However, that turns out to be insufficient, as boot is still broken
> on the Tyan S8812 mainboard.
>
> To fix that issue, split the ACPI early initialization code into
> two pieces so the majority of it still located in acpi_early_init()
> and the part switching over the platform into the ACPI mode goes into
> a new function, acpi_subsystem_init(), executed at the original early
> ACPI initialization spot.
>
> That fixes the Tyan S8812 boot problem, but still allows ACPI
> tables to be loaded earlier which is useful to the EFI code in
> efi_enter_virtual_mode().
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=97141
> Fixes: 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before timekeeping_init()"
> Reported-and-tested-by: Marius Tolzmann <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> Acked-by: Toshi Kani <[email protected]>
> ---
> drivers/acpi/bus.c | 56 +++++++++++++++++++++++++++++++++++++--------------
> include/linux/acpi.h | 2 +
> init/main.c | 1
> 3 files changed, 44 insertions(+), 15 deletions(-)
>
> Index: linux-pm/drivers/acpi/bus.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/bus.c
> +++ linux-pm/drivers/acpi/bus.c
> @@ -470,6 +470,16 @@ static int __init acpi_bus_init_irq(void
> return 0;
> }
>
> +/**
> + * acpi_early_init - Initialize ACPICA and populate the ACPI namespace.
> + *
> + * The ACPI tables are accessible after this, but the handling of events has not

Maybe we need to explicitly say that it's DSDT/SSDT, because static
tables such as FADT and MADT are accessible in the very early stage
of boot (in setup_arch()).

Other than that,

Reviewed-by: Hanjun Guo <[email protected]>

Thanks
Hanjun

2015-06-10 04:58:44

by joeyli

[permalink] [raw]
Subject: Re: [PATCH] ACPI / init: Switch over platform to the ACPI mode later

On Wed, Jun 10, 2015 at 01:48:29AM +0200, Rafael J. Wysocki wrote:
> On Wednesday, June 03, 2015 11:13:57 AM Toshi Kani wrote:
> > On Sat, 2015-05-30 at 14:21 +0200, Rafael J. Wysocki wrote:
> > > From: Rafael J. Wysocki <[email protected]>
> > >
> > > Commit 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before
> > > timekeeping_init()" moved the ACPI subsystem initialization,
> > > including the ACPI mode enabling, to an earlier point in the
> > > initialization sequence, to allow the timekeeping subsystem
> > > use ACPI early. Unfortunately, that resulted in boot regressions
> > > on some systems and the early ACPI initialization was moved toward
> > > its original position in the kernel initialization code by commit
> > > c4e1acbb35e4 "ACPI / init: Invoke early ACPI initialization later".
> > >
> > > However, that turns out to be insufficient, as boot is still broken
> > > on the Tyan S8812 mainboard.
> > >
> > > To fix that issue, split the ACPI early initialization code into
> > > two pieces so the majority of it still located in acpi_early_init()
> > > and the part switching over the platform into the ACPI mode goes into
> > > a new function, acpi_subsystem_init(), executed at the original early
> > > ACPI initialization spot.
> > >
> > > That fixes the Tyan S8812 boot problem, but still allows ACPI
> > > tables to be loaded earlier which is useful to the EFI code in
> > > efi_enter_virtual_mode().
> > >
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=97141
> > > Reported-and-tested-by: Marius Tolzmann <[email protected]>
> > > Signed-off-by: Rafael J. Wysocki <[email protected]>
> >
> > Can you add comments to acpi_early_init() and acpi_subsystem_init() to
> > clarify what ACPI features are enabled at each phase, and what
> > dependency they have in the boot sequence? (The same goes to
> > early_acpi_boot_init() and acpi_boot_init().)
>
> OK, update follows.
>
> ---
> From: Rafael J. Wysocki <[email protected]>
> Subject: ACPI / init: Switch over platform to the ACPI mode later
>
> Commit 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before
> timekeeping_init()" moved the ACPI subsystem initialization,
> including the ACPI mode enabling, to an earlier point in the
> initialization sequence, to allow the timekeeping subsystem
> use ACPI early. Unfortunately, that resulted in boot regressions
> on some systems and the early ACPI initialization was moved toward
> its original position in the kernel initialization code by commit
> c4e1acbb35e4 "ACPI / init: Invoke early ACPI initialization later".
>
> However, that turns out to be insufficient, as boot is still broken
> on the Tyan S8812 mainboard.
>
> To fix that issue, split the ACPI early initialization code into
> two pieces so the majority of it still located in acpi_early_init()
> and the part switching over the platform into the ACPI mode goes into
> a new function, acpi_subsystem_init(), executed at the original early
> ACPI initialization spot.
>
> That fixes the Tyan S8812 boot problem, but still allows ACPI
> tables to be loaded earlier which is useful to the EFI code in
> efi_enter_virtual_mode().
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=97141
> Fixes: 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before timekeeping_init()"
> Reported-and-tested-by: Marius Tolzmann <[email protected]>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> Acked-by: Toshi Kani <[email protected]>
> ---
> drivers/acpi/bus.c | 56 +++++++++++++++++++++++++++++++++++++--------------
> include/linux/acpi.h | 2 +
> init/main.c | 1
> 3 files changed, 44 insertions(+), 15 deletions(-)

About bko#97141, I am still curious why the platform didn't set SCI_EN
in that early stage.

Thanks for Rafael's patch.

Reviewed-by: Lee, Chun-Yi <[email protected]>


Regards
Joey Lee

2015-06-10 22:52:03

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] ACPI / init: Switch over platform to the ACPI mode later

On Wednesday, June 10, 2015 10:06:40 AM Hanjun Guo wrote:
> On 06/10/2015 07:48 AM, Rafael J. Wysocki wrote:
> > On Wednesday, June 03, 2015 11:13:57 AM Toshi Kani wrote:
> >> On Sat, 2015-05-30 at 14:21 +0200, Rafael J. Wysocki wrote:
> >>> From: Rafael J. Wysocki <[email protected]>
> >>>
> >>> Commit 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before
> >>> timekeeping_init()" moved the ACPI subsystem initialization,
> >>> including the ACPI mode enabling, to an earlier point in the
> >>> initialization sequence, to allow the timekeeping subsystem
> >>> use ACPI early. Unfortunately, that resulted in boot regressions
> >>> on some systems and the early ACPI initialization was moved toward
> >>> its original position in the kernel initialization code by commit
> >>> c4e1acbb35e4 "ACPI / init: Invoke early ACPI initialization later".
> >>>
> >>> However, that turns out to be insufficient, as boot is still broken
> >>> on the Tyan S8812 mainboard.
> >>>
> >>> To fix that issue, split the ACPI early initialization code into
> >>> two pieces so the majority of it still located in acpi_early_init()
> >>> and the part switching over the platform into the ACPI mode goes into
> >>> a new function, acpi_subsystem_init(), executed at the original early
> >>> ACPI initialization spot.
> >>>
> >>> That fixes the Tyan S8812 boot problem, but still allows ACPI
> >>> tables to be loaded earlier which is useful to the EFI code in
> >>> efi_enter_virtual_mode().
> >>>
> >>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=97141
> >>> Reported-and-tested-by: Marius Tolzmann <[email protected]>
> >>> Signed-off-by: Rafael J. Wysocki <[email protected]>
> >>
> >> Can you add comments to acpi_early_init() and acpi_subsystem_init() to
> >> clarify what ACPI features are enabled at each phase, and what
> >> dependency they have in the boot sequence? (The same goes to
> >> early_acpi_boot_init() and acpi_boot_init().)
> >
> > OK, update follows.
> >
> > ---
> > From: Rafael J. Wysocki <[email protected]>
> > Subject: ACPI / init: Switch over platform to the ACPI mode later
> >
> > Commit 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before
> > timekeeping_init()" moved the ACPI subsystem initialization,
> > including the ACPI mode enabling, to an earlier point in the
> > initialization sequence, to allow the timekeeping subsystem
> > use ACPI early. Unfortunately, that resulted in boot regressions
> > on some systems and the early ACPI initialization was moved toward
> > its original position in the kernel initialization code by commit
> > c4e1acbb35e4 "ACPI / init: Invoke early ACPI initialization later".
> >
> > However, that turns out to be insufficient, as boot is still broken
> > on the Tyan S8812 mainboard.
> >
> > To fix that issue, split the ACPI early initialization code into
> > two pieces so the majority of it still located in acpi_early_init()
> > and the part switching over the platform into the ACPI mode goes into
> > a new function, acpi_subsystem_init(), executed at the original early
> > ACPI initialization spot.
> >
> > That fixes the Tyan S8812 boot problem, but still allows ACPI
> > tables to be loaded earlier which is useful to the EFI code in
> > efi_enter_virtual_mode().
> >
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=97141
> > Fixes: 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before timekeeping_init()"
> > Reported-and-tested-by: Marius Tolzmann <[email protected]>
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > Acked-by: Toshi Kani <[email protected]>
> > ---
> > drivers/acpi/bus.c | 56 +++++++++++++++++++++++++++++++++++++--------------
> > include/linux/acpi.h | 2 +
> > init/main.c | 1
> > 3 files changed, 44 insertions(+), 15 deletions(-)
> >
> > Index: linux-pm/drivers/acpi/bus.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/bus.c
> > +++ linux-pm/drivers/acpi/bus.c
> > @@ -470,6 +470,16 @@ static int __init acpi_bus_init_irq(void
> > return 0;
> > }
> >
> > +/**
> > + * acpi_early_init - Initialize ACPICA and populate the ACPI namespace.
> > + *
> > + * The ACPI tables are accessible after this, but the handling of events has not
>
> Maybe we need to explicitly say that it's DSDT/SSDT, because static
> tables such as FADT and MADT are accessible in the very early stage
> of boot (in setup_arch()).

Well, it never says that none of the tables are accessible earlier.

> Other than that,
>
> Reviewed-by: Hanjun Guo <[email protected]>

Thanks!


--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

2015-06-11 09:36:40

by Hanjun Guo

[permalink] [raw]
Subject: Re: [PATCH] ACPI / init: Switch over platform to the ACPI mode later

On 06/11/2015 07:17 AM, Rafael J. Wysocki wrote:
> On Wednesday, June 10, 2015 10:06:40 AM Hanjun Guo wrote:
>> On 06/10/2015 07:48 AM, Rafael J. Wysocki wrote:
>>> On Wednesday, June 03, 2015 11:13:57 AM Toshi Kani wrote:
>>>> On Sat, 2015-05-30 at 14:21 +0200, Rafael J. Wysocki wrote:
>>>>> From: Rafael J. Wysocki <[email protected]>
>>>>>
>>>>> Commit 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before
>>>>> timekeeping_init()" moved the ACPI subsystem initialization,
>>>>> including the ACPI mode enabling, to an earlier point in the
>>>>> initialization sequence, to allow the timekeeping subsystem
>>>>> use ACPI early. Unfortunately, that resulted in boot regressions
>>>>> on some systems and the early ACPI initialization was moved toward
>>>>> its original position in the kernel initialization code by commit
>>>>> c4e1acbb35e4 "ACPI / init: Invoke early ACPI initialization later".
>>>>>
>>>>> However, that turns out to be insufficient, as boot is still broken
>>>>> on the Tyan S8812 mainboard.
>>>>>
>>>>> To fix that issue, split the ACPI early initialization code into
>>>>> two pieces so the majority of it still located in acpi_early_init()
>>>>> and the part switching over the platform into the ACPI mode goes into
>>>>> a new function, acpi_subsystem_init(), executed at the original early
>>>>> ACPI initialization spot.
>>>>>
>>>>> That fixes the Tyan S8812 boot problem, but still allows ACPI
>>>>> tables to be loaded earlier which is useful to the EFI code in
>>>>> efi_enter_virtual_mode().
>>>>>
>>>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=97141
>>>>> Reported-and-tested-by: Marius Tolzmann <[email protected]>
>>>>> Signed-off-by: Rafael J. Wysocki <[email protected]>
>>>>
>>>> Can you add comments to acpi_early_init() and acpi_subsystem_init() to
>>>> clarify what ACPI features are enabled at each phase, and what
>>>> dependency they have in the boot sequence? (The same goes to
>>>> early_acpi_boot_init() and acpi_boot_init().)
>>>
>>> OK, update follows.
>>>
>>> ---
>>> From: Rafael J. Wysocki <[email protected]>
>>> Subject: ACPI / init: Switch over platform to the ACPI mode later
>>>
>>> Commit 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before
>>> timekeeping_init()" moved the ACPI subsystem initialization,
>>> including the ACPI mode enabling, to an earlier point in the
>>> initialization sequence, to allow the timekeeping subsystem
>>> use ACPI early. Unfortunately, that resulted in boot regressions
>>> on some systems and the early ACPI initialization was moved toward
>>> its original position in the kernel initialization code by commit
>>> c4e1acbb35e4 "ACPI / init: Invoke early ACPI initialization later".
>>>
>>> However, that turns out to be insufficient, as boot is still broken
>>> on the Tyan S8812 mainboard.
>>>
>>> To fix that issue, split the ACPI early initialization code into
>>> two pieces so the majority of it still located in acpi_early_init()
>>> and the part switching over the platform into the ACPI mode goes into
>>> a new function, acpi_subsystem_init(), executed at the original early
>>> ACPI initialization spot.
>>>
>>> That fixes the Tyan S8812 boot problem, but still allows ACPI
>>> tables to be loaded earlier which is useful to the EFI code in
>>> efi_enter_virtual_mode().
>>>
>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=97141
>>> Fixes: 73f7d1ca3263 "ACPI / init: Run acpi_early_init() before timekeeping_init()"
>>> Reported-and-tested-by: Marius Tolzmann <[email protected]>
>>> Signed-off-by: Rafael J. Wysocki <[email protected]>
>>> Acked-by: Toshi Kani <[email protected]>
>>> ---
>>> drivers/acpi/bus.c | 56 +++++++++++++++++++++++++++++++++++++--------------
>>> include/linux/acpi.h | 2 +
>>> init/main.c | 1
>>> 3 files changed, 44 insertions(+), 15 deletions(-)
>>>
>>> Index: linux-pm/drivers/acpi/bus.c
>>> ===================================================================
>>> --- linux-pm.orig/drivers/acpi/bus.c
>>> +++ linux-pm/drivers/acpi/bus.c
>>> @@ -470,6 +470,16 @@ static int __init acpi_bus_init_irq(void
>>> return 0;
>>> }
>>>
>>> +/**
>>> + * acpi_early_init - Initialize ACPICA and populate the ACPI namespace.
>>> + *
>>> + * The ACPI tables are accessible after this, but the handling of events has not
>>
>> Maybe we need to explicitly say that it's DSDT/SSDT, because static
>> tables such as FADT and MADT are accessible in the very early stage
>> of boot (in setup_arch()).
>
> Well, it never says that none of the tables are accessible earlier.

Ah, that makes sense :)

Thanks
Hanjun