[PATCH 5.4 09/66] ACPI: PM: s2idle: Avoid possible race related to the EC GPE

From: Greg Kroah-Hartman
Date: Tue Feb 18 2020 - 15:08:30 EST


From: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>

commit e3728b50cd9be7d4b1469447cdf1feb93e3b7adb upstream.

It is theoretically possible for the ACPI EC GPE to be set after the
s2idle_ops->wake() called from s2idle_loop() has returned and before
the subsequent pm_wakeup_pending() check is carried out. If that
happens, the resulting wakeup event will cause the system to resume
even though it may be a spurious one.

To avoid that race, first make the ->wake() callback in struct
platform_s2idle_ops return a bool value indicating whether or not
to let the system resume and rearrange s2idle_loop() to use that
value instad of the direct pm_wakeup_pending() call if ->wake() is
present.

Next, rework acpi_s2idle_wake() to process EC events and check
pm_wakeup_pending() before re-arming the SCI for system wakeup
to prevent it from triggering prematurely and add comments to
that function to explain the rationale for the new code flow.

Fixes: 56b991849009 ("PM: sleep: Simplify suspend-to-idle control flow")
Cc: 5.4+ <stable@xxxxxxxxxxxxxxx> # 5.4+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@xxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>

---
drivers/acpi/sleep.c | 46 ++++++++++++++++++++++++++++++++--------------
include/linux/suspend.h | 2 +-
kernel/power/suspend.c | 9 +++++----
3 files changed, 38 insertions(+), 19 deletions(-)

--- a/drivers/acpi/sleep.c
+++ b/drivers/acpi/sleep.c
@@ -977,21 +977,28 @@ static int acpi_s2idle_prepare_late(void
return 0;
}

-static void acpi_s2idle_wake(void)
+static bool acpi_s2idle_wake(void)
{
- /*
- * If IRQD_WAKEUP_ARMED is set for the SCI at this point, the SCI has
- * not triggered while suspended, so bail out.
- */
- if (!acpi_sci_irq_valid() ||
- irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq)))
- return;
-
- /*
- * If there are EC events to process, the wakeup may be a spurious one
- * coming from the EC.
- */
- if (acpi_ec_dispatch_gpe()) {
+ if (!acpi_sci_irq_valid())
+ return pm_wakeup_pending();
+
+ while (pm_wakeup_pending()) {
+ /*
+ * If IRQD_WAKEUP_ARMED is set for the SCI at this point, the
+ * SCI has not triggered while suspended, so bail out (the
+ * wakeup is pending anyway and the SCI is not the source of
+ * it).
+ */
+ if (irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq)))
+ return true;
+
+ /*
+ * If there are no EC events to process, the wakeup is regarded
+ * as a genuine one.
+ */
+ if (!acpi_ec_dispatch_gpe())
+ return true;
+
/*
* Cancel the wakeup and process all pending events in case
* there are any wakeup ones in there.
@@ -1009,8 +1016,19 @@ static void acpi_s2idle_wake(void)
acpi_ec_flush_work();
acpi_os_wait_events_complete(); /* synchronize Notify handling */

+ /*
+ * The SCI is in the "suspended" state now and it cannot produce
+ * new wakeup events till the rearming below, so if any of them
+ * are pending here, they must be resulting from the processing
+ * of EC events above or coming from somewhere else.
+ */
+ if (pm_wakeup_pending())
+ return true;
+
rearm_wake_irq(acpi_sci_irq);
}
+
+ return false;
}

static void acpi_s2idle_restore_early(void)
--- a/include/linux/suspend.h
+++ b/include/linux/suspend.h
@@ -191,7 +191,7 @@ struct platform_s2idle_ops {
int (*begin)(void);
int (*prepare)(void);
int (*prepare_late)(void);
- void (*wake)(void);
+ bool (*wake)(void);
void (*restore_early)(void);
void (*restore)(void);
void (*end)(void);
--- a/kernel/power/suspend.c
+++ b/kernel/power/suspend.c
@@ -131,11 +131,12 @@ static void s2idle_loop(void)
* to avoid them upfront.
*/
for (;;) {
- if (s2idle_ops && s2idle_ops->wake)
- s2idle_ops->wake();
-
- if (pm_wakeup_pending())
+ if (s2idle_ops && s2idle_ops->wake) {
+ if (s2idle_ops->wake())
+ break;
+ } else if (pm_wakeup_pending()) {
break;
+ }

pm_wakeup_clear(false);