ACPICA: Save current masks of enabled GPEs after enable register writes
There is a race condition between acpi_hw_disable_all_gpes() or
acpi_enable_all_wakeup_gpes() and acpi_ev_asynch_enable_gpe() such
that if the latter wins the race, it may mistakenly enable a GPE
disabled by the former. This may lead to premature system wakeups
during system suspend and potentially to more serious consequences.
The source of the problem is how acpi_hw_low_set_gpe() works when
passed ACPI_GPE_CONDITIONAL_ENABLE as the second argument. In that
case, the GPE will be enabled if the corresponding bit is set in the
enable_for_run mask of the GPE enable register containing that bit.
However, acpi_hw_disable_all_gpes() and acpi_enable_all_wakeup_gpes()
don't modify the enable_for_run masks of GPE registers when writing
to them. In consequence, if acpi_ev_asynch_enable_gpe(), which
eventually calls acpi_hw_low_set_gpe() with the second argument
equal to ACPI_GPE_CONDITIONAL_ENABLE, is executed in parallel with
one of these functions, it may reverse changes made by them.
To fix the problem, introduce a new enable_mask field in struct
acpi_gpe_register_info in which to store the current mask of
enabled GPEs and modify acpi_hw_low_set_gpe() to take this
mask into account instead of enable_for_run when its second
argument is equal to ACPI_GPE_CONDITIONAL_ENABLE. Also modify
the low-level routines called by acpi_hw_disable_all_gpes(),
acpi_enable_all_wakeup_gpes() and acpi_enable_all_runtime_gpes()
to update the enable_mask masks of GPE registers after all
(successful) writes to those registers.
Acked-by: Lv Zheng <lv.zheng@intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
diff --git a/drivers/acpi/acpica/aclocal.h b/drivers/acpi/acpica/aclocal.h
index 80c74e9..680d23b 100644
--- a/drivers/acpi/acpica/aclocal.h
+++ b/drivers/acpi/acpica/aclocal.h
@@ -454,6 +454,7 @@
u16 base_gpe_number; /* Base GPE number for this register */
u8 enable_for_wake; /* GPEs to keep enabled when sleeping */
u8 enable_for_run; /* GPEs to keep enabled when running */
+ u8 enable_mask; /* Current mask of enabled GPEs */
};
/*
diff --git a/drivers/acpi/acpica/evgpe.c b/drivers/acpi/acpica/evgpe.c
index 2095dfb..81db932 100644
--- a/drivers/acpi/acpica/evgpe.c
+++ b/drivers/acpi/acpica/evgpe.c
@@ -134,7 +134,7 @@
/* Enable the requested GPE */
- status = acpi_hw_low_set_gpe(gpe_event_info, ACPI_GPE_ENABLE);
+ status = acpi_hw_low_set_gpe(gpe_event_info, ACPI_GPE_ENABLE_SAVE);
return_ACPI_STATUS(status);
}
@@ -213,7 +213,7 @@
if (ACPI_SUCCESS(status)) {
status =
acpi_hw_low_set_gpe(gpe_event_info,
- ACPI_GPE_DISABLE);
+ ACPI_GPE_DISABLE_SAVE);
}
if (ACPI_FAILURE(status)) {
@@ -655,7 +655,7 @@
/*
* Enable this GPE, conditionally. This means that the GPE will
- * only be physically enabled if the enable_for_run bit is set
+ * only be physically enabled if the enable_mask bit is set
* in the event_info.
*/
(void)acpi_hw_low_set_gpe(gpe_event_info, ACPI_GPE_CONDITIONAL_ENABLE);
diff --git a/drivers/acpi/acpica/hwgpe.c b/drivers/acpi/acpica/hwgpe.c
index 48ac7b7..494027f5 100644
--- a/drivers/acpi/acpica/hwgpe.c
+++ b/drivers/acpi/acpica/hwgpe.c
@@ -115,12 +115,12 @@
/* Set or clear just the bit that corresponds to this GPE */
register_bit = acpi_hw_get_gpe_register_bit(gpe_event_info);
- switch (action) {
+ switch (action & ~ACPI_GPE_SAVE_MASK) {
case ACPI_GPE_CONDITIONAL_ENABLE:
- /* Only enable if the enable_for_run bit is set */
+ /* Only enable if the corresponding enable_mask bit is set */
- if (!(register_bit & gpe_register_info->enable_for_run)) {
+ if (!(register_bit & gpe_register_info->enable_mask)) {
return (AE_BAD_PARAMETER);
}
@@ -145,6 +145,9 @@
/* Write the updated enable mask */
status = acpi_hw_write(enable_mask, &gpe_register_info->enable_address);
+ if (ACPI_SUCCESS(status) && (action & ACPI_GPE_SAVE_MASK)) {
+ gpe_register_info->enable_mask = enable_mask;
+ }
return (status);
}
@@ -262,6 +265,32 @@
/******************************************************************************
*
+ * FUNCTION: acpi_hw_gpe_enable_write
+ *
+ * PARAMETERS: enable_mask - Bit mask to write to the GPE register
+ * gpe_register_info - Gpe Register info
+ *
+ * RETURN: Status
+ *
+ * DESCRIPTION: Write the enable mask byte to the given GPE register.
+ *
+ ******************************************************************************/
+
+static acpi_status
+acpi_hw_gpe_enable_write(u8 enable_mask,
+ struct acpi_gpe_register_info *gpe_register_info)
+{
+ acpi_status status;
+
+ status = acpi_hw_write(enable_mask, &gpe_register_info->enable_address);
+ if (ACPI_SUCCESS(status)) {
+ gpe_register_info->enable_mask = enable_mask;
+ }
+ return (status);
+}
+
+/******************************************************************************
+ *
* FUNCTION: acpi_hw_disable_gpe_block
*
* PARAMETERS: gpe_xrupt_info - GPE Interrupt info
@@ -287,8 +316,8 @@
/* Disable all GPEs in this register */
status =
- acpi_hw_write(0x00,
- &gpe_block->register_info[i].enable_address);
+ acpi_hw_gpe_enable_write(0x00,
+ &gpe_block->register_info[i]);
if (ACPI_FAILURE(status)) {
return (status);
}
@@ -355,21 +384,23 @@
{
u32 i;
acpi_status status;
+ struct acpi_gpe_register_info *gpe_register_info;
/* NOTE: assumes that all GPEs are currently disabled */
/* Examine each GPE Register within the block */
for (i = 0; i < gpe_block->register_count; i++) {
- if (!gpe_block->register_info[i].enable_for_run) {
+ gpe_register_info = &gpe_block->register_info[i];
+ if (!gpe_register_info->enable_for_run) {
continue;
}
/* Enable all "runtime" GPEs in this register */
status =
- acpi_hw_write(gpe_block->register_info[i].enable_for_run,
- &gpe_block->register_info[i].enable_address);
+ acpi_hw_gpe_enable_write(gpe_register_info->enable_for_run,
+ gpe_register_info);
if (ACPI_FAILURE(status)) {
return (status);
}
@@ -399,10 +430,12 @@
{
u32 i;
acpi_status status;
+ struct acpi_gpe_register_info *gpe_register_info;
/* Examine each GPE Register within the block */
for (i = 0; i < gpe_block->register_count; i++) {
+ gpe_register_info = &gpe_block->register_info[i];
/*
* Enable all "wake" GPEs in this register and disable the
@@ -410,8 +443,8 @@
*/
status =
- acpi_hw_write(gpe_block->register_info[i].enable_for_wake,
- &gpe_block->register_info[i].enable_address);
+ acpi_hw_gpe_enable_write(gpe_register_info->enable_for_wake,
+ gpe_register_info);
if (ACPI_FAILURE(status)) {
return (status);
}