[CFT] call AcpiLeaveSleepStatePrep after re-enabling interrupts

From: Andriy Gapon <avg_at_FreeBSD.org>
Date: Sat, 5 May 2018 09:05:13 +0300
I would like ask for help with testing this change
https://reviews.freebsd.org/D15306

I want to do this change because that call (actually,
AcpiHwLegacyWakePrep) does a memory allocation and ACPI namespace
evaluation. Although it is not very likely to run into any trouble, it
is still not safe to make those calls with interrupts disabled. Also,
AcpiLeaveSleepStatePrep is documented as called when interrupts are
enabled. witness(4) and malloc(9) do not currently check for a context
with interrupts disabled via intr_disable and we lack a facility for
doing that. So, those unsafe operations fly under the radar. But if
intr_disable in acpi_EnterSleepState was replaced with spinlock_enter
(which it probably should be), then witness and malloc would
immmediately complain.

The ACPI wakeup sequence is very sensitive to changes, but I consider
this change to be rather safe. What AcpiHwLegacyWakePrep essentially
does is writing a value corresponding to S0 into SLP_TYPx bits of PM1
Control Register(s). According to ACPI specifications that write should
be a NOP as SLP_EN bit is not set. Additionally, there are no accesses
to ACPI hardware and firmware between the old location of the call and
the new one. So, the move should not affect the interaction between
then OS and ACPI platform.

Index: sys/dev/acpica/acpi.c
===================================================================
--- sys/dev/acpica/acpi.c	(revision 333269)
+++ sys/dev/acpica/acpi.c	(working copy)
_at__at_ -2975,8 +2975,6 _at__at_ acpi_EnterSleepState(struct acpi_softc *sc, int st
 	if (sleep_result == 1 && state != ACPI_STATE_S4)
 	    AcpiWriteBitRegister(ACPI_BITREG_SCI_ENABLE, ACPI_ENABLE_EVENT);

-	AcpiLeaveSleepStatePrep(state);
-
 	if (sleep_result == 1 && state == ACPI_STATE_S3) {
 	    /*
 	     * Prevent mis-interpretation of the wakeup by power button
_at__at_ -3005,6 +3003,8 _at__at_ acpi_EnterSleepState(struct acpi_softc *sc, int st
 	/* call acpi_wakeup_machdep() again with interrupt enabled */
 	acpi_wakeup_machdep(sc, state, sleep_result, 1);

+	AcpiLeaveSleepStatePrep(state);
+
 	if (sleep_result == -1)
 		goto backout;


-- 
Andriy Gapon
Received on Sat May 05 2018 - 04:05:24 UTC

This archive was generated by hypermail 2.4.0 : Wed May 19 2021 - 11:41:15 UTC