summaryrefslogtreecommitdiffstats
path: root/drivers/acpi/ec.c
diff options
context:
space:
mode:
authorLv Zheng <lv.zheng@intel.com>2015-09-24 14:54:54 +0800
committerRafael J. Wysocki <rafael.j.wysocki@intel.com>2015-09-26 01:46:25 +0200
commit611975470740b38084610e0a64ff4c6ff5aec89a (patch)
tree434854d3649eeeebc77c0371f439c0a52d5e588b /drivers/acpi/ec.c
parent0700c047f66101c7d0d23e3583f1ae9c3c2dd71c (diff)
downloadlinux-611975470740b38084610e0a64ff4c6ff5aec89a.tar.bz2
ACPI / EC: Fix a race issue in acpi_ec_guard_event()
In acpi_ec_guard_event(), EC transaction state machine variables should be checked with the EC spinlock locked. The bug doesn't trigger any real issue now because this bug can only occur when the ec_event_clearing=event mode is applied while there is no user currently using this mode. Signed-off-by: Lv Zheng <lv.zheng@intel.com> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Diffstat (limited to 'drivers/acpi/ec.c')
-rw-r--r--drivers/acpi/ec.c42
1 files changed, 29 insertions, 13 deletions
diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index 2db0912bb2e6..f61a7c834540 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -441,17 +441,31 @@ static void acpi_ec_complete_query(struct acpi_ec *ec)
static bool acpi_ec_guard_event(struct acpi_ec *ec)
{
+ bool guarded = true;
+ unsigned long flags;
+
+ spin_lock_irqsave(&ec->lock, flags);
+ /*
+ * If firmware SCI_EVT clearing timing is "event", we actually
+ * don't know when the SCI_EVT will be cleared by firmware after
+ * evaluating _Qxx, so we need to re-check SCI_EVT after waiting an
+ * acceptable period.
+ *
+ * The guarding period begins when EC_FLAGS_QUERY_PENDING is
+ * flagged, which means SCI_EVT check has just been performed.
+ * But if the current transaction is ACPI_EC_COMMAND_QUERY, the
+ * guarding should have already been performed (via
+ * EC_FLAGS_QUERY_GUARDING) and should not be applied so that the
+ * ACPI_EC_COMMAND_QUERY transaction can be transitioned into
+ * ACPI_EC_COMMAND_POLL state immediately.
+ */
if (ec_event_clearing == ACPI_EC_EVT_TIMING_STATUS ||
ec_event_clearing == ACPI_EC_EVT_TIMING_QUERY ||
!test_bit(EC_FLAGS_QUERY_PENDING, &ec->flags) ||
(ec->curr && ec->curr->command == ACPI_EC_COMMAND_QUERY))
- return false;
-
- /*
- * Postpone the query submission to allow the firmware to proceed,
- * we shouldn't check SCI_EVT before the firmware reflagging it.
- */
- return true;
+ guarded = false;
+ spin_unlock_irqrestore(&ec->lock, flags);
+ return guarded;
}
static int ec_transaction_polled(struct acpi_ec *ec)
@@ -597,6 +611,7 @@ static int ec_guard(struct acpi_ec *ec)
unsigned long guard = usecs_to_jiffies(ec_polling_guard);
unsigned long timeout = ec->timestamp + guard;
+ /* Ensure guarding period before polling EC status */
do {
if (ec_busy_polling) {
/* Perform busy polling */
@@ -606,11 +621,13 @@ static int ec_guard(struct acpi_ec *ec)
} else {
/*
* Perform wait polling
- *
- * For SCI_EVT clearing timing of "event",
- * performing guarding before re-checking the
- * SCI_EVT. Otherwise, such guarding is not needed
- * due to the old practices.
+ * 1. Wait the transaction to be completed by the
+ * GPE handler after the transaction enters
+ * ACPI_EC_COMMAND_POLL state.
+ * 2. A special guarding logic is also required
+ * for event clearing mode "event" before the
+ * transaction enters ACPI_EC_COMMAND_POLL
+ * state.
*/
if (!ec_transaction_polled(ec) &&
!acpi_ec_guard_event(ec))
@@ -620,7 +637,6 @@ static int ec_guard(struct acpi_ec *ec)
guard))
return 0;
}
- /* Guard the register accesses for the polling modes */
} while (time_before(jiffies, timeout));
return -ETIME;
}