Commit e1d4d90f authored by Lv Zheng's avatar Lv Zheng Committed by Rafael J. Wysocki

ACPI / EC: Refine command storm prevention support

This patch refines EC command storm prevention support.

Current command storming code is wrong, when the storming condition is
detected, it only flags the condition without doing anything for the
current command but performing storming prevention for the follow-up
commands. So:
1. The first command which suffers from the storming still suffers from
   storming.
2. The follow-up commands which may not suffer from the storming are
   unconditionally forced into the storming prevention mode.
Ideally, we should only enable storm prevention immediately after detection
for the current command so that the next command can try the
power/performance efficient interrupt mode again.

This patch improves the command storm prevention by disabling GPE right
after the detection and re-enabling it right before completing the command
transaction using the GPE storming prevention APIs. This thus deploys the
following GPE handling model:
1. acpi_enable_gpe()/acpi_disable_gpe() for reference count changes:
   This set of APIs are used for EC usage reference counting.
2. acpi_set_gpe(ACPI_GPE_ENABLE)/acpi_set_gpe(ACPI_GPE_DISABLE):
   This set of APIs are used for preventing GPE storm. They must be invoked
   when the reference count > 0.
   Note that as the storming prevention should always happen when there is
   an outstanding request, or GPE enabling value will be messed up by the
   races. This patch also adds BUG_ON() to enforces this rule to prevent
   future bugs.

The msleep(1) used after completing a transaction is useless now as this
sounds like a guard time only useful for platforms that need the
EC_FLAGS_MSI quirks while we have fixed GPE race issues using the previous
raw handler mode enabling. It is kept to avoid regressions. A seperate
patch which deletes EC_FLAGS_MSI quirks should take care of deleting it.
Signed-off-by: default avatarLv Zheng <lv.zheng@intel.com>
Signed-off-by: default avatarRafael J. Wysocki <rafael.j.wysocki@intel.com>
parent 9887d22a
...@@ -77,11 +77,12 @@ enum ec_command { ...@@ -77,11 +77,12 @@ enum ec_command {
enum { enum {
EC_FLAGS_QUERY_PENDING, /* Query is pending */ EC_FLAGS_QUERY_PENDING, /* Query is pending */
EC_FLAGS_GPE_STORM, /* GPE storm detected */
EC_FLAGS_HANDLERS_INSTALLED, /* Handlers for GPE and EC_FLAGS_HANDLERS_INSTALLED, /* Handlers for GPE and
* OpReg are installed */ * OpReg are installed */
EC_FLAGS_STARTED, /* Driver is started */ EC_FLAGS_STARTED, /* Driver is started */
EC_FLAGS_STOPPED, /* Driver is stopped */ EC_FLAGS_STOPPED, /* Driver is stopped */
EC_FLAGS_COMMAND_STORM, /* GPE storms occurred to the
* current command processing */
}; };
#define ACPI_EC_COMMAND_POLL 0x01 /* Available for command byte */ #define ACPI_EC_COMMAND_POLL 0x01 /* Available for command byte */
...@@ -229,8 +230,10 @@ static inline void acpi_ec_enable_gpe(struct acpi_ec *ec, bool open) ...@@ -229,8 +230,10 @@ static inline void acpi_ec_enable_gpe(struct acpi_ec *ec, bool open)
{ {
if (open) if (open)
acpi_enable_gpe(NULL, ec->gpe); acpi_enable_gpe(NULL, ec->gpe);
else else {
BUG_ON(ec->reference_count < 1);
acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE); acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_ENABLE);
}
if (acpi_ec_is_gpe_raised(ec)) { if (acpi_ec_is_gpe_raised(ec)) {
/* /*
* On some platforms, EN=1 writes cannot trigger GPE. So * On some platforms, EN=1 writes cannot trigger GPE. So
...@@ -246,8 +249,10 @@ static inline void acpi_ec_disable_gpe(struct acpi_ec *ec, bool close) ...@@ -246,8 +249,10 @@ static inline void acpi_ec_disable_gpe(struct acpi_ec *ec, bool close)
{ {
if (close) if (close)
acpi_disable_gpe(NULL, ec->gpe); acpi_disable_gpe(NULL, ec->gpe);
else else {
BUG_ON(ec->reference_count < 1);
acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE); acpi_set_gpe(NULL, ec->gpe, ACPI_GPE_DISABLE);
}
} }
static inline void acpi_ec_clear_gpe(struct acpi_ec *ec) static inline void acpi_ec_clear_gpe(struct acpi_ec *ec)
...@@ -290,6 +295,24 @@ static void acpi_ec_complete_request(struct acpi_ec *ec) ...@@ -290,6 +295,24 @@ static void acpi_ec_complete_request(struct acpi_ec *ec)
wake_up(&ec->wait); wake_up(&ec->wait);
} }
static void acpi_ec_set_storm(struct acpi_ec *ec, u8 flag)
{
if (!test_bit(flag, &ec->flags)) {
acpi_ec_disable_gpe(ec, false);
pr_debug("+++++ Polling enabled +++++\n");
set_bit(flag, &ec->flags);
}
}
static void acpi_ec_clear_storm(struct acpi_ec *ec, u8 flag)
{
if (test_bit(flag, &ec->flags)) {
clear_bit(flag, &ec->flags);
acpi_ec_enable_gpe(ec, false);
pr_debug("+++++ Polling disabled +++++\n");
}
}
/* /*
* acpi_ec_submit_flushable_request() - Increase the reference count unless * acpi_ec_submit_flushable_request() - Increase the reference count unless
* the flush operation is not in * the flush operation is not in
...@@ -404,8 +427,13 @@ static void advance_transaction(struct acpi_ec *ec) ...@@ -404,8 +427,13 @@ static void advance_transaction(struct acpi_ec *ec)
* otherwise will take a not handled IRQ as a false one. * otherwise will take a not handled IRQ as a false one.
*/ */
if (!(status & ACPI_EC_FLAG_SCI)) { if (!(status & ACPI_EC_FLAG_SCI)) {
if (in_interrupt() && t) if (in_interrupt() && t) {
++t->irq_count; if (t->irq_count < ec_storm_threshold)
++t->irq_count;
/* Allow triggering on 0 threshold */
if (t->irq_count == ec_storm_threshold)
acpi_ec_set_storm(ec, EC_FLAGS_COMMAND_STORM);
}
} }
out: out:
if (status & ACPI_EC_FLAG_SCI) if (status & ACPI_EC_FLAG_SCI)
...@@ -482,6 +510,8 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, ...@@ -482,6 +510,8 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec,
spin_unlock_irqrestore(&ec->lock, tmp); spin_unlock_irqrestore(&ec->lock, tmp);
ret = ec_poll(ec); ret = ec_poll(ec);
spin_lock_irqsave(&ec->lock, tmp); spin_lock_irqsave(&ec->lock, tmp);
if (t->irq_count == ec_storm_threshold)
acpi_ec_clear_storm(ec, EC_FLAGS_COMMAND_STORM);
pr_debug("***** Command(%s) stopped *****\n", pr_debug("***** Command(%s) stopped *****\n",
acpi_ec_cmd_string(t->command)); acpi_ec_cmd_string(t->command));
ec->curr = NULL; ec->curr = NULL;
...@@ -509,24 +539,11 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) ...@@ -509,24 +539,11 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t)
goto unlock; goto unlock;
} }
} }
/* disable GPE during transaction if storm is detected */
if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) {
/* It has to be disabled, so that it doesn't trigger. */
acpi_ec_disable_gpe(ec, false);
}
status = acpi_ec_transaction_unlocked(ec, t); status = acpi_ec_transaction_unlocked(ec, t);
if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) { if (test_bit(EC_FLAGS_COMMAND_STORM, &ec->flags))
msleep(1); msleep(1);
/* It is safe to enable the GPE outside of the transaction. */
acpi_ec_enable_gpe(ec, false);
} else if (t->irq_count > ec_storm_threshold) {
pr_info("GPE storm detected(%d GPEs), "
"transactions will use polling mode\n",
t->irq_count);
set_bit(EC_FLAGS_GPE_STORM, &ec->flags);
}
if (ec->global_lock) if (ec->global_lock)
acpi_release_global_lock(glk); acpi_release_global_lock(glk);
unlock: unlock:
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment