Commit e3728b50 authored by Rafael J. Wysocki's avatar Rafael J. Wysocki
Browse files

ACPI: PM: s2idle: Avoid possible race related to the EC GPE 



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: 56b99184 ("PM: sleep: Simplify suspend-to-idle control flow")
Cc: 5.4+ <stable@vger.kernel.org> # 5.4+
Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
parent f0ac20c3

drivers/acpi/sleep.c

+31 −13
Original line number Diff line number Diff line
@@ -990,21 +990,28 @@ static void acpi_s2idle_sync(void) 
	acpi_os_wait_events_complete(); /* synchronize Notify handling */

}



static void acpi_s2idle_wake(void)

static bool acpi_s2idle_wake(void)

{

	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.

		 * 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 (!acpi_sci_irq_valid() ||

	    irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq)))

		return;

		if (irqd_is_wakeup_armed(irq_get_irq_data(acpi_sci_irq)))

			return true;



		/*

	 * If there are EC events to process, the wakeup may be a spurious one

	 * coming from the EC.

		 * If there are no EC events to process, the wakeup is regarded

		 * as a genuine one.

		 */

	if (acpi_ec_dispatch_gpe()) {

		if (!acpi_ec_dispatch_gpe())

			return true;



		/*

		 * Cancel the wakeup and process all pending events in case

		 * there are any wakeup ones in there.
@@ -1017,8 +1024,19 @@ static void acpi_s2idle_wake(void) 


		acpi_s2idle_sync();



		/*

		 * 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)

include/linux/suspend.h

+1 −1
Original line number Diff line number Diff line
@@ -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);

kernel/power/suspend.c

+5 −4
Original line number Diff line number Diff line
@@ -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);

CRA Git | Maintained and supported by SUSTech CRA and CCSE