[PATCH 0/3] ACPI: Fix problem with GPE storm from buttons

[PATCH 0/3] ACPI: Fix problem with GPE storm from buttons

[Rafael J. Wysocki]

Hi All,

This patch series attempts to fix a problem with GPE storms from
buttons described in the changelog of patch [3/3].

Patches [1/3] and [2/3] are ACPICA material, patch [3/3] is for Linux/ACPI.

Thanks,
Rafael

[PATCH 1/3] ACPICA: Introduce acpi_mark_gpe_for_wake()

[Rafael J. Wysocki]

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

Some callers of acpi_setup_gpe_for_wake() know in advance that there
won't be any notify handlers installed for device wake notifications
from the given GPEs (one example is button GPEs in Linux).  For those
cases, introduce acpi_mark_gpe_for_wake() to be called instead of
acpi_setup_gpe_for_wake() that will only set the ACPI_GPE_CAN_WAKE
flag for the GPE without trying to set up implicit wake notification
for it if there's no handler method.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpica/evxfgpe.c |   32 ++++++++++++++++++++++++++++++++
 include/acpi/acpixf.h         |    5 +++++
 2 files changed, 37 insertions(+)

Index: linux-pm/drivers/acpi/acpica/evxfgpe.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/evxfgpe.c
+++ linux-pm/drivers/acpi/acpica/evxfgpe.c
@@ -314,6 +314,38 @@ ACPI_EXPORT_SYMBOL(acpi_setup_gpe_for_wa
 
 /*******************************************************************************
  *
+ * FUNCTION:    acpi_mark_gpe_for_wake
+ *
+ * PARAMETERS:  gpe_device          - Parent GPE Device. NULL for GPE0/GPE1
+ *              gpe_number          - GPE level within the GPE block
+ *
+ * RETURN:      Status
+ *
+ * DESCRIPTION: Mark a GPE as having the ability to wake the system.
+ *
+ ******************************************************************************/
+
+acpi_status acpi_mark_gpe_for_wake(acpi_handle gpe_device, u32 gpe_number)
+{
+	acpi_status status = AE_BAD_PARAMETER;
+	struct acpi_gpe_event_info *gpe_event_info;
+	acpi_cpu_flags flags;
+
+	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+
+	gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device, gpe_number);
+	if (gpe_event_info) {
+		gpe_event_info->flags |= ACPI_GPE_CAN_WAKE;
+		status = AE_OK;
+	}
+
+	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	return_ACPI_STATUS(status);
+}
+ACPI_EXPORT_SYMBOL(acpi_mark_gpe_for_wake)
+
+/*******************************************************************************
+ *
  * FUNCTION:    acpi_set_gpe_wake_mask
  *
  * PARAMETERS:  gpe_device      - Parent GPE Device. NULL for GPE0/GPE1
Index: linux-pm/include/acpi/acpixf.h
===================================================================
--- linux-pm.orig/include/acpi/acpixf.h
+++ linux-pm/include/acpi/acpixf.h
@@ -662,6 +662,11 @@ ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_sta
 							parent_device,
 							acpi_handle gpe_device,
 							u32 gpe_number))
+
+ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
+				acpi_mark_gpe_for_wake(acpi_handle gpe_device,
+							u32 gpe_number))
+
 ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
 				 acpi_set_gpe_wake_mask(acpi_handle gpe_device,
 							u32 gpe_number,

[PATCH 2/3] ACPICA: Refuse enabling GPEs with no handling

[Rafael J. Wysocki]

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

Make acpi_enable_gpe() refuse to enable a given GPE if there is no
way to handle it and return an error instead.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/acpica/evxfgpe.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/acpi/acpica/evxfgpe.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/evxfgpe.c
+++ linux-pm/drivers/acpi/acpica/evxfgpe.c
@@ -129,7 +129,8 @@ acpi_status acpi_enable_gpe(acpi_handle
 	/* Ensure that we have a valid GPE number */
 
 	gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device, gpe_number);
-	if (gpe_event_info) {
+	if (gpe_event_info &&
+	    (gpe_event_info->flags & ACPI_GPE_DISPATCH_MASK) != ACPI_GPE_DISPATCH_NONE) {
 		status = acpi_ev_add_gpe_reference(gpe_event_info);
 	}
 

[PATCH 3/3] ACPI / scan: No implicit wake notification for buttons

[Rafael J. Wysocki]

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

The ACPI device enumeration code in Linux assumes that buttons always
are wakeup devices, so it calls acpi_setup_gpe_for_wake() for them
which leads to undesirable side effects.  Namely, that function sets
up implicit device wake notification mechanism for a given GPE if
there is no handler method in the ACPI namespace, which from the
ACPICA's perspective means that there always is a way to handle
that GPE if enabled.  However, we don't handle wake notify events
for buttons, so if there are no handler methods for their GPEs in
the namespace, enabling a button GPE at run time leads to a GPE
storm in some cases (the GPE triggers, ACPICA carries out the
implicit wake notification for it which isn't handled, so the
GPE triggers again and so on).

To prevent that from happening use acpi_mark_gpe_for_wake()
instead of acpi_setup_gpe_for_wake() for buttons which will cause
ACPICA to only enable button GPEs if there are handler methods for
the in the namespace.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/scan.c |   29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -1423,14 +1423,13 @@ static int acpi_bus_extract_wakeup_devic
 			wakeup->sleep_state = sleep_state;
 		}
 	}
-	acpi_setup_gpe_for_wake(handle, wakeup->gpe_device, wakeup->gpe_number);
 
  out:
 	kfree(buffer.pointer);
 	return err;
 }
 
-static void acpi_bus_set_run_wake_flags(struct acpi_device *device)
+static void acpi_wakeup_gpe_init(struct acpi_device *device)
 {
 	struct acpi_device_id button_device_ids[] = {
 		{"PNP0C0C", 0},
@@ -1438,29 +1437,33 @@ static void acpi_bus_set_run_wake_flags(
 		{"PNP0C0E", 0},
 		{"", 0},
 	};
+	struct acpi_device_wakeup *wakeup = &device->wakeup;
 	acpi_status status;
 	acpi_event_status event_status;
 
-	device->wakeup.flags.notifier_present = 0;
+	wakeup->flags.notifier_present = 0;
 
 	/* Power button, Lid switch always enable wakeup */
 	if (!acpi_match_device_ids(device, button_device_ids)) {
-		device->wakeup.flags.run_wake = 1;
+		wakeup->flags.run_wake = 1;
 		if (!acpi_match_device_ids(device, &button_device_ids[1])) {
 			/* Do not use Lid/sleep button for S5 wakeup */
-			if (device->wakeup.sleep_state == ACPI_STATE_S5)
-				device->wakeup.sleep_state = ACPI_STATE_S4;
+			if (wakeup->sleep_state == ACPI_STATE_S5)
+				wakeup->sleep_state = ACPI_STATE_S4;
 		}
+		acpi_mark_gpe_for_wake(wakeup->gpe_device, wakeup->gpe_number);
 		device_set_wakeup_capable(&device->dev, true);
 		return;
 	}
 
-	status = acpi_get_gpe_status(device->wakeup.gpe_device,
-					device->wakeup.gpe_number,
-						&event_status);
-	if (status == AE_OK)
-		device->wakeup.flags.run_wake =
-				!!(event_status & ACPI_EVENT_FLAG_HANDLE);
+	acpi_setup_gpe_for_wake(device->handle, wakeup->gpe_device,
+				wakeup->gpe_number);
+	status = acpi_get_gpe_status(wakeup->gpe_device, wakeup->gpe_number,
+				     &event_status);
+	if (ACPI_FAILURE(status))
+		return;
+
+	wakeup->flags.run_wake = !!(event_status & ACPI_EVENT_FLAG_HANDLE);
 }
 
 static void acpi_bus_get_wakeup_device_flags(struct acpi_device *device)
@@ -1480,7 +1483,7 @@ static void acpi_bus_get_wakeup_device_f
 
 	device->wakeup.flags.valid = 1;
 	device->wakeup.prepare_count = 0;
-	acpi_bus_set_run_wake_flags(device);
+	acpi_wakeup_gpe_init(device);
 	/* Call _PSW/_DSW object to disable its ability to wake the sleeping
 	 * system for the ACPI device with the _PRW object.
 	 * The _PSW object is depreciated in ACPI 3.0 and is replaced by _DSW.

[Update][PATCH 2/3] ACPICA: Refuse to enable GPEs with no handling

[Rafael J. Wysocki]

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

Make acpi_enable_gpe() refuse to enable a given GPE if there is no
way to handle it and return an error instead.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Updated as requested by Bob.

---
 drivers/acpi/acpica/evxfgpe.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Index: linux-pm/drivers/acpi/acpica/evxfgpe.c
===================================================================
--- linux-pm.orig/drivers/acpi/acpica/evxfgpe.c
+++ linux-pm/drivers/acpi/acpica/evxfgpe.c
@@ -118,7 +118,7 @@ ACPI_EXPORT_SYMBOL(acpi_update_all_gpes)
 
 acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number)
 {
-	acpi_status status = AE_BAD_PARAMETER;
+	acpi_status status;
 	struct acpi_gpe_event_info *gpe_event_info;
 	acpi_cpu_flags flags;
 
@@ -129,7 +129,12 @@ acpi_status acpi_enable_gpe(acpi_handle
 	/* Ensure that we have a valid GPE number */
 
 	gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device, gpe_number);
-	if (gpe_event_info) {
+	if (!gpe_event_info) {
+		status = AE_BAD_PARAMETER;
+	} else if ((gpe_event_info->flags & ACPI_GPE_DISPATCH_MASK) ==
+					ACPI_GPE_DISPATCH_NONE) {
+		status = AE_NO_HANDLER;
+	} else {
 		status = acpi_ev_add_gpe_reference(gpe_event_info);
 	}
 

[PATCH v2 1/2] ACPICA: GPEs: Do not allow enable for GPEs that have no handler(s).

[Lv Zheng]

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

ACPICA release patch.

------------------

commit 23b5a8542283af28c3a3a4e3f81096d6e2569faa upstream

There is no point in enabling a GPE that has no handler or
GPE method. At worst, it can cause GPE floods.
Rafael Wysocki.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/acpica/evxfgpe.c |   14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/acpica/evxfgpe.c b/drivers/acpi/acpica/evxfgpe.c
index cb534fa..82e5c14 100644
--- a/drivers/acpi/acpica/evxfgpe.c
+++ b/drivers/acpi/acpica/evxfgpe.c
@@ -126,11 +126,19 @@ acpi_status acpi_enable_gpe(acpi_handle gpe_device, u32 gpe_number)
 
 	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
 
-	/* Ensure that we have a valid GPE number */
-
+	/*
+	 * Ensure that we have a valid GPE number and that there is some way
+	 * of handling the GPE (handler or a GPE method). In other words, we
+	 * won't allow a valid GPE to be enabled if there is no way to handle it.
+	 */
 	gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device, gpe_number);
 	if (gpe_event_info) {
-		status = acpi_ev_add_gpe_reference(gpe_event_info);
+		if ((gpe_event_info->flags & ACPI_GPE_DISPATCH_MASK) !=
+		    ACPI_GPE_DISPATCH_NONE) {
+			status = acpi_ev_add_gpe_reference(gpe_event_info);
+		} else {
+			status = AE_NO_HANDLER;
+		}
 	}
 
 	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
-- 
1.7.10

[PATCH v2 2/2] ACPICA: Add new GPE public interface - acpi_mark_gpe_for_wake.

[Lv Zheng]

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

ACPICA release patch.

------------------

commit c49dbfed2bc069d0038ea7e1294409bfde7c2c8c upstream

Some potential callers of acpi_setup_gpe_for_wake may know in advance that
there won't be any notify handlers installed for device wake notifications
from the given GPE (one example is a button GPE in Linux). For these cases,
acpi_mark_gpe_for_wake should be used instead of acpi_setup_gpe_for_wake.
This will set the ACPI_GPE_CAN_WAKE flag for the GPE without trying to
setup implicit wake notification for it (since there's no handler method).
Rafael Wysocki.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Bob Moore <robert.moore@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/acpica/evxfgpe.c |   47 +++++++++++++++++++++++++++++++++++++++++
 include/acpi/acpixf.h         |    4 ++++
 2 files changed, 51 insertions(+)

diff --git a/drivers/acpi/acpica/evxfgpe.c b/drivers/acpi/acpica/evxfgpe.c
index 82e5c14..0cf159c 100644
--- a/drivers/acpi/acpica/evxfgpe.c
+++ b/drivers/acpi/acpica/evxfgpe.c
@@ -187,6 +187,53 @@ ACPI_EXPORT_SYMBOL(acpi_disable_gpe)
 
 /*******************************************************************************
  *
+ * FUNCTION:    acpi_mark_gpe_for_wake
+ *
+ * PARAMETERS:  gpe_device          - Parent GPE Device. NULL for GPE0/GPE1
+ *              gpe_number          - GPE level within the GPE block
+ *
+ * RETURN:      Status
+ *
+ * DESCRIPTION: Mark a GPE as having the ability to wake the system. Simply
+ *              sets the ACPI_GPE_CAN_WAKE flag.
+ *
+ * Some potential callers of acpi_setup_gpe_for_wake may know in advance that
+ * there won't be any notify handlers installed for device wake notifications
+ * from the given GPE (one example is a button GPE in Linux). For these cases,
+ * acpi_mark_gpe_for_wake should be used instead of acpi_setup_gpe_for_wake.
+ * This will set the ACPI_GPE_CAN_WAKE flag for the GPE without trying to
+ * setup implicit wake notification for it (since there's no handler method).
+ *
+ ******************************************************************************/
+acpi_status acpi_mark_gpe_for_wake(acpi_handle gpe_device, u32 gpe_number)
+{
+	struct acpi_gpe_event_info *gpe_event_info;
+	acpi_status status = AE_BAD_PARAMETER;
+	acpi_cpu_flags flags;
+
+	ACPI_FUNCTION_TRACE(acpi_mark_gpe_for_wake);
+
+	flags = acpi_os_acquire_lock(acpi_gbl_gpe_lock);
+
+	/* Ensure that we have a valid GPE number */
+
+	gpe_event_info = acpi_ev_get_gpe_event_info(gpe_device, gpe_number);
+	if (gpe_event_info) {
+
+		/* Mark the GPE as a possible wake event */
+
+		gpe_event_info->flags |= ACPI_GPE_CAN_WAKE;
+		status = AE_OK;
+	}
+
+	acpi_os_release_lock(acpi_gbl_gpe_lock, flags);
+	return_ACPI_STATUS(status);
+}
+
+ACPI_EXPORT_SYMBOL(acpi_mark_gpe_for_wake)
+
+/*******************************************************************************
+ *
  * FUNCTION:    acpi_setup_gpe_for_wake
  *
  * PARAMETERS:  wake_device         - Device associated with the GPE (via _PRW)
diff --git a/include/acpi/acpixf.h b/include/acpi/acpixf.h
index 854dd01..c3f38bc 100644
--- a/include/acpi/acpixf.h
+++ b/include/acpi/acpixf.h
@@ -673,6 +673,10 @@ ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
 						u32 gpe_number))
 
 ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
+				acpi_mark_gpe_for_wake(acpi_handle gpe_device,
+						       u32 gpe_number))
+
+ACPI_HW_DEPENDENT_RETURN_STATUS(acpi_status
 				acpi_setup_gpe_for_wake(acpi_handle
 							parent_device,
 							acpi_handle gpe_device,
-- 
1.7.10