linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system
@ 2020-02-11 16:51 Rafael J. Wysocki
  2020-02-11 16:52 ` [PATCH 1/2] ACPICA: Introduce acpi_any_gpe_status_set() Rafael J. Wysocki
  2020-02-11 16:53 ` [PATCH 2/2] ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system Rafael J. Wysocki
  0 siblings, 2 replies; 3+ messages in thread
From: Rafael J. Wysocki @ 2020-02-11 16:51 UTC (permalink / raw)
  To: Linux ACPI
  Cc: Linux PM, LKML, Zhang Rui, David Box, Chen Yu, Rafael J. Wysocki,
	Tsuchiya Yuto, Bob Moore, Erik Kaneda

Hi All,

On some systems the platform generates spurious SCIs while the system is
suspended to idle and those SCIs are treated as genuine system events after
the rework of the main suspend-to-idle control flow in 5.4.  The patches
here address this problem by adding a mechanism to check whether or not
SCIs generated while the system is suspended to idle are spurious and to
discard the spurious ones.

Patch [1/2] updates ACPICA to provide a way to examine the status bits of
all GPEs enabled at the moment in one go.

Patch [2/2] uses that mechanism to implement the check mentioned above.

This series is on top of https://patchwork.kernel.org/patch/11373185/ which
should appear in linux-next tomorrow.

Thanks!




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 1/2] ACPICA: Introduce acpi_any_gpe_status_set()
  2020-02-11 16:51 [PATCH 0/2] ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system Rafael J. Wysocki
@ 2020-02-11 16:52 ` Rafael J. Wysocki
  2020-02-11 16:53 ` [PATCH 2/2] ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system Rafael J. Wysocki
  1 sibling, 0 replies; 3+ messages in thread
From: Rafael J. Wysocki @ 2020-02-11 16:52 UTC (permalink / raw)
  To: Linux ACPI
  Cc: Linux PM, LKML, Zhang Rui, David Box, Chen Yu, Rafael J. Wysocki,
	Tsuchiya Yuto, Bob Moore, Erik Kaneda

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

Introduce a new helper function, acpi_any_gpe_status_set(), for
checking the status bits of all enabled GPEs in one go.

It is needed to distinguish spurious SCIs from genuine ones when
deciding whether or not to wake up the system from suspend-to-idle.

Cc: 5.4+ <stable@vger.kernel.org> # 5.4+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpica/achware.h |    2 +
 drivers/acpi/acpica/evxfgpe.c |   32 ++++++++++++++++++
 drivers/acpi/acpica/hwgpe.c   |   71 ++++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpixf.h         |    1 
 4 files changed, 106 insertions(+)

Index: linux-pm/drivers/acpi/acpica/hwgpe.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/hwgpe.c
+++ linux-pm/drivers/acpi/acpica/hwgpe.c
@@ -446,6 +446,53 @@ acpi_hw_enable_wakeup_gpe_block(struct a
 
 /******************************************************************************
  *
+ * FUNCTION:    acpi_hw_get_gpe_block_status
+ *
+ * PARAMETERS:  gpe_xrupt_info      - GPE Interrupt info
+ *              gpe_block           - Gpe Block info
+ *
+ * RETURN:      Success
+ *
+ * DESCRIPTION: Produce a combined GPE status bits mask for the given block.
+ *
+ ******************************************************************************/
+
+static acpi_status
+acpi_hw_get_gpe_block_status(struct acpi_gpe_xrupt_info *gpe_xrupt_info,
+			     struct acpi_gpe_block_info *gpe_block,
+			     void *ret_ptr)
+{
+	struct acpi_gpe_register_info *gpe_register_info;
+	u64 in_enable, in_status;
+	acpi_status status;
+	u8 *ret = ret_ptr;
+	u32 i;
+
+	/* Examine each GPE Register within the block */
+
+	for (i = 0; i < gpe_block->register_count; i++) {
+		gpe_register_info = &gpe_block->register_info[i];
+
+		status = acpi_hw_read(&in_enable,
+				      &gpe_register_info->enable_address);
+		if (ACPI_FAILURE(status)) {
+			continue;
+		}
+
+		status = acpi_hw_read(&in_status,
+				      &gpe_register_info->status_address);
+		if (ACPI_FAILURE(status)) {
+			continue;
+		}
+
+		*ret |= in_enable & in_status;
+	}
+
+	return (AE_OK);
+}
+
+/******************************************************************************
+ *
  * FUNCTION:    acpi_hw_disable_all_gpes
  *
  * PARAMETERS:  None
@@ -510,4 +557,28 @@ acpi_status acpi_hw_enable_all_wakeup_gp
 	return_ACPI_STATUS(status);
 }
 
+/******************************************************************************
+ *
+ * FUNCTION:    acpi_hw_check_all_gpes
+ *
+ * PARAMETERS:  None
+ *
+ * RETURN:      Combined status of all GPEs
+ *
+ * DESCRIPTION: Check all enabled GPEs in all GPE blocks and return TRUE if the
+ *              status bit is set for at least one of them of FALSE otherwise.
+ *
+ ******************************************************************************/
+
+u8 acpi_hw_check_all_gpes(void)
+{
+	u8 ret = 0;
+
+	ACPI_FUNCTION_TRACE(acpi_hw_check_all_gpes);
+
+	(void)acpi_ev_walk_gpe_list(acpi_hw_get_gpe_block_status, &ret);
+
+	return (ret != 0);
+}
+
 #endif				/* !ACPI_REDUCED_HARDWARE */
Index: linux-pm/drivers/acpi/acpica/achware.h
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/achware.h
+++ linux-pm/drivers/acpi/acpica/achware.h
@@ -101,6 +101,8 @@ acpi_status acpi_hw_enable_all_runtime_g
 
 acpi_status acpi_hw_enable_all_wakeup_gpes(void);
 
+u8 acpi_hw_check_all_gpes(void);
+
 acpi_status
 acpi_hw_enable_runtime_gpe_block(struct acpi_gpe_xrupt_info *gpe_xrupt_info,
 				 struct acpi_gpe_block_info *gpe_block,
Index: linux-pm/drivers/acpi/acpica/evxfgpe.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/evxfgpe.c
+++ linux-pm/drivers/acpi/acpica/evxfgpe.c
@@ -795,6 +795,38 @@ acpi_status acpi_enable_all_wakeup_gpes(
 
 ACPI_EXPORT_SYMBOL(acpi_enable_all_wakeup_gpes)
 
+/******************************************************************************
+ *
+ * FUNCTION:    acpi_any_gpe_status_set
+ *
+ * PARAMETERS:  None
+ *
+ * RETURN:      Whether or not the status bit is set for any GPE
+ *
+ * DESCRIPTION: Check the status bits of all enabled GPEs and return TRUE if any
+ *              of them is set or FALSE otherwise.
+ *
+ ******************************************************************************/
+u32 acpi_any_gpe_status_set(void)
+{
+	acpi_status status;
+	u8 ret;
+
+	ACPI_FUNCTION_TRACE(acpi_any_gpe_status_set);
+
+	status = acpi_ut_acquire_mutex(ACPI_MTX_EVENTS);
+	if (ACPI_FAILURE(status)) {
+		return (FALSE);
+	}
+
+	ret = acpi_hw_check_all_gpes();
+	(void)acpi_ut_release_mutex(ACPI_MTX_EVENTS);
+
+	return (ret);
+}
+
+ACPI_EXPORT_SYMBOL(acpi_any_gpe_status_set)
+
 /*******************************************************************************
  *
  * FUNCTION:    acpi_install_gpe_block
Index: linux-pm/include/acpi/acpixf.h
===================================================================
--- linux-pm.orig/include/acpi/acpixf.h
+++ linux-pm/include/acpi/acpixf.h
@@ -752,6 +752,7 @@ ACPI_HW_DEPENDENT_RETURN_UINT32(u32 acpi
 ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_disable_all_gpes(void))
 ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_enable_all_runtime_gpes(void))
 ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status acpi_enable_all_wakeup_gpes(void))
+ACPI_HW_DEPENDENT_RETURN_UINT32(u32 acpi_any_gpe_status_set(void))
 
 ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
 				acpi_get_gpe_device(u32 gpe_index,




^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH 2/2] ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system
  2020-02-11 16:51 [PATCH 0/2] ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system Rafael J. Wysocki
  2020-02-11 16:52 ` [PATCH 1/2] ACPICA: Introduce acpi_any_gpe_status_set() Rafael J. Wysocki
@ 2020-02-11 16:53 ` Rafael J. Wysocki
  1 sibling, 0 replies; 3+ messages in thread
From: Rafael J. Wysocki @ 2020-02-11 16:53 UTC (permalink / raw)
  To: Linux ACPI
  Cc: Linux PM, LKML, Zhang Rui, David Box, Chen Yu, Rafael J. Wysocki,
	Tsuchiya Yuto, Bob Moore, Erik Kaneda

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

If the platform triggers a spurious SCI even though the status bit
is not set for any GPE when the system is suspended to idle, it will
be treated as a genuine wakeup, so avoid that by checking if any GPEs
are active at all before returning 'true' from acpi_s2idle_wake().

Link: https://bugzilla.kernel.org/show_bug.cgi?id=206413
Fixes: 56b991849009 ("PM: sleep: Simplify suspend-to-idle control flow")
Reported-by: Tsuchiya Yuto <kitakar@gmail.com>
Cc: 5.4+ <stable@vger.kernel.org> # 5.4+
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/sleep.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

Index: linux-pm/drivers/acpi/sleep.c
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.c
+++ linux-pm/drivers/acpi/sleep.c
@@ -1006,10 +1006,16 @@ static bool acpi_s2idle_wake(void)
 			return true;
 
 		/*
-		 * If there are no EC events to process, the wakeup is regarded
-		 * as a genuine one.
+		 * If there are no EC events to process and at least one of the
+		 * other enabled GPEs is active, the wakeup is regarded as a
+		 * genuine one.
+		 *
+		 * Note that the checks below must be carried out in this order
+		 * to avoid returning prematurely due to a change of the EC GPE
+		 * status bit from unset to set between the checks with the
+		 * status bits of all the other GPEs unset.
 		 */
-		if (!acpi_ec_dispatch_gpe())
+		if (acpi_any_gpe_status_set() && !acpi_ec_dispatch_gpe())
 			return true;
 
 		/*




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2020-02-11 16:54 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-11 16:51 [PATCH 0/2] ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system Rafael J. Wysocki
2020-02-11 16:52 ` [PATCH 1/2] ACPICA: Introduce acpi_any_gpe_status_set() Rafael J. Wysocki
2020-02-11 16:53 ` [PATCH 2/2] ACPI: PM: s2idle: Prevent spurious SCIs from waking up the system Rafael J. Wysocki

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).