There is wait code in the QR_SC command processing, which makes it not suitable to be put into a work queue item (see bug 82611). And there is case that the SCI_EVT cannot trigger GPE, though all commands have polling mode implemented, the event cannot be polled (see bug 77431). So if the QR_SC command can be put into a seperate IRQ thread, then the work queue will not be blocked by the QR_SC command processing and we can also trigger polling using the thread. Using IRQ thread also allows us to change the EC GPE handler into the threaded IRQ model when possible. This patch thus adds the IRQ polling thread for SCI_EVT polling and removes QR_SC processing work item. The reasons why we do not put a loop in the event poller to poll event until the returned query value is 0: Some platforms return non 0 query value even when SCI_EVT=0, if we put a loop in the poller, our command flush mechanism could never execute to an end thus the system suspending process could be blocked. One SCI_EVT triggering one QR_EC is current logic and has been proven to be working for so long time. The reasons why it is not implemented directly using threaded IRQ are: 1. ACPICA doesn't support threaded IRQ as not all of the OSPMs support threaded IRQ while GPE handler registerations are done through ACPICA. 2. The IRQ processing code need to be identical for both the IRQ handler and the thread callback, while currently, though the command GPE handling is ready for both IRQ and polling mode, only the event GPE is is polled in the event polling thread and the command is polled in the user threads. So we use a standalone kernel thread, if the above situations are changed in the future, we can easily convert the code into the threaded IRQ style. The old EC_FLAGS_QUERY_PENDING is converted to EC_FLAGS_EVENT_ENABLED in this patch, so that its naming is consistent with EC_FLAGS_EVENT_PENDING. The original flag doesn't co-work with SCI_EVT well, this patch refines its usage by enforcing a event polling wakeup indication as: EC_FLAGS_EVENT_ENABLED && EC_FLAGS_EVENT_PENDING So unless the both of the flags are set, the threaded event poller will not be woken up. This patch invokes acpi_ec_submit_request() after having detected SCI_EVT and invokes acpi_ec_complete_request() before having the QR_EC command processed. This is useful for implementing GPE storm prevention for malicous "level triggered" SCI_EVT. But the storm prevention is not implemented in this patch. Since the acpi_ec_submit_request() invoked after detecting the SCI_EVT is paired with acpi_ec_complete_request() invoked after completing QR_EC command, acpi_ec_submit_flushable_request() then need to be modified to allow QR_EC command to be submitted during this period to revert the increased reference count. This period can be determined by the event polling indication: EC_FLAGS_EVENT_ENABLED && EC_FLAGS_EVENT_PENDING Without enhancing this check in acpi_ec_submit_flushable_request(), QR_EC command will not be executed to decrease the reference count added after detecting the SCI_EVT, thus the system suspending will be blocked because the reference count equals to 2. Such check is common for flushing code. Reference: https://bugzilla.kernel.org/show_bug.cgi?id=82611 Reference: https://bugzilla.kernel.org/show_bug.cgi?id=77431 Signed-off-by: Lv Zheng <lv.zheng@xxxxxxxxx> Tested-by: Ortwin Glück <odi@xxxxxx> --- drivers/acpi/ec.c | 194 ++++++++++++++++++++++++++++++++++------------- drivers/acpi/internal.h | 1 + 2 files changed, 144 insertions(+), 51 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index a76794a..7089081 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -44,6 +44,7 @@ #include <linux/slab.h> #include <linux/acpi.h> #include <linux/dmi.h> +#include <linux/kthread.h> #include <asm/io.h> #include "internal.h" @@ -75,7 +76,8 @@ enum ec_command { * when trying to clear the EC */ enum { - EC_FLAGS_QUERY_PENDING, /* Query is pending */ + EC_FLAGS_EVENT_ENABLED, /* Event is enabled */ + EC_FLAGS_EVENT_PENDING, /* Event is pending */ EC_FLAGS_GPE_STORM, /* GPE storm detected */ EC_FLAGS_HANDLERS_INSTALLED, /* Handlers for GPE and * OpReg are installed */ @@ -124,6 +126,7 @@ struct transaction { static struct acpi_ec_query_handler * acpi_ec_get_query_handler(struct acpi_ec_query_handler *handler); static void acpi_ec_put_query_handler(struct acpi_ec_query_handler *handler); +static int acpi_ec_sync_query(struct acpi_ec *ec, u8 *data); struct acpi_ec *boot_ec, *first_ec; EXPORT_SYMBOL(first_ec); @@ -149,6 +152,12 @@ static bool acpi_ec_flushed(struct acpi_ec *ec) return ec->reference_count == 1; } +static bool acpi_ec_has_pending_event(struct acpi_ec *ec) +{ + return test_bit(EC_FLAGS_EVENT_ENABLED, &ec->flags) && + test_bit(EC_FLAGS_EVENT_PENDING, &ec->flags); +} + /* -------------------------------------------------------------------------- * GPE Enhancement * -------------------------------------------------------------------------- */ @@ -177,20 +186,93 @@ static void acpi_ec_complete_request(struct acpi_ec *ec) * the flush operation is not in * progress * @ec: the EC device + * @check_event: check whether event is pending * * This function must be used before taking a new action that should hold * the reference count. If this function returns false, then the action * must be discarded or it will prevent the flush operation from being * completed. + * + * During flushing, QR_EC command need to pass this check when there is a + * pending event, so that the reference count held for the pending event + * can be decreased by the completion of the QR_EC command. */ -static bool acpi_ec_submit_flushable_request(struct acpi_ec *ec) +static bool acpi_ec_submit_flushable_request(struct acpi_ec *ec, + bool check_event) { - if (!acpi_ec_started(ec)) - return false; + if (!acpi_ec_started(ec)) { + if (!check_event || !acpi_ec_has_pending_event(ec)) + return false; + } acpi_ec_submit_request(ec); return true; } +static void acpi_ec_enable_event(struct acpi_ec *ec) +{ + unsigned long flags; + + spin_lock_irqsave(&ec->lock, flags); + /* Hold reference for pending event */ + if (!acpi_ec_submit_flushable_request(ec, false)) { + spin_unlock_irqrestore(&ec->lock, flags); + return; + } + set_bit(EC_FLAGS_EVENT_ENABLED, &ec->flags); + if (test_bit(EC_FLAGS_EVENT_PENDING, &ec->flags)) { + pr_debug("***** Event pending *****\n"); + wake_up_process(ec->thread); + spin_unlock_irqrestore(&ec->lock, flags); + return; + } + acpi_ec_complete_request(ec); + spin_unlock_irqrestore(&ec->lock, flags); +} + +static void __acpi_ec_set_event(struct acpi_ec *ec) +{ + /* Hold reference for pending event */ + if (!acpi_ec_submit_flushable_request(ec, false)) + return; + if (!test_and_set_bit(EC_FLAGS_EVENT_PENDING, &ec->flags)) { + pr_debug("***** Event pending *****\n"); + if (test_bit(EC_FLAGS_EVENT_ENABLED, &ec->flags)) { + wake_up_process(ec->thread); + return; + } + } + acpi_ec_complete_request(ec); +} + +static void __acpi_ec_complete_event(struct acpi_ec *ec) +{ + if (test_and_clear_bit(EC_FLAGS_EVENT_PENDING, &ec->flags)) { + /* Unhold reference for pending event */ + acpi_ec_complete_request(ec); + pr_debug("***** Event running *****\n"); + } +} + +int acpi_ec_wait_for_event(struct acpi_ec *ec) +{ + unsigned long flags; + + set_current_state(TASK_INTERRUPTIBLE); + while (!kthread_should_stop()) { + spin_lock_irqsave(&ec->lock, flags); + if (acpi_ec_has_pending_event(ec)) { + spin_unlock_irqrestore(&ec->lock, flags); + __set_current_state(TASK_RUNNING); + return 0; + } + spin_unlock_irqrestore(&ec->lock, flags); + schedule(); + set_current_state(TASK_INTERRUPTIBLE); + } + __set_current_state(TASK_RUNNING); + return -1; +} + /* -------------------------------------------------------------------------- * Transaction Management * -------------------------------------------------------------------------- */ @@ -298,7 +380,7 @@ static bool advance_transaction(struct acpi_ec *ec) t->flags |= ACPI_EC_COMMAND_COMPLETE; wakeup = true; } - return wakeup; + goto out; } else { if (EC_FLAGS_QUERY_HANDSHAKE && !(status & ACPI_EC_FLAG_SCI) && @@ -312,9 +394,11 @@ static bool advance_transaction(struct acpi_ec *ec) } else if ((status & ACPI_EC_FLAG_IBF) == 0) { acpi_ec_write_cmd(ec, t->command); t->flags |= ACPI_EC_COMMAND_POLL; + if (t->command == ACPI_EC_COMMAND_QUERY) + __acpi_ec_complete_event(ec); } else goto err; - return wakeup; + goto out; } err: /* @@ -325,6 +409,10 @@ err: if (in_interrupt() && t) ++t->irq_count; } +out: + if (status & ACPI_EC_FLAG_SCI && + (!t || t->flags & ACPI_EC_COMMAND_COMPLETE)) + __acpi_ec_set_event(ec); return wakeup; } @@ -335,17 +423,6 @@ static void start_transaction(struct acpi_ec *ec) (void)advance_transaction(ec); } -static int acpi_ec_sync_query(struct acpi_ec *ec, u8 *data); - -static int ec_check_sci_sync(struct acpi_ec *ec, u8 state) -{ - if (state & ACPI_EC_FLAG_SCI) { - if (!test_and_set_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) - return acpi_ec_sync_query(ec, NULL); - } - return 0; -} - static int ec_poll(struct acpi_ec *ec) { unsigned long flags; @@ -384,12 +461,13 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, unsigned long tmp; int ret = 0; + bool is_query = !!(t->command == ACPI_EC_COMMAND_QUERY); if (EC_FLAGS_MSI) udelay(ACPI_EC_MSI_UDELAY); /* start transaction */ spin_lock_irqsave(&ec->lock, tmp); /* Enable GPE for command processing (IBF=0/OBF=1) */ - if (!acpi_ec_submit_flushable_request(ec)) { + if (!acpi_ec_submit_flushable_request(ec, is_query)) { ret = -EINVAL; goto unlock; } @@ -398,10 +476,6 @@ static int acpi_ec_transaction_unlocked(struct acpi_ec *ec, pr_debug("***** Command(%s) started *****\n", acpi_ec_cmd_string(t->command)); start_transaction(ec); - if (ec->curr->command == ACPI_EC_COMMAND_QUERY) { - clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags); - pr_debug("***** Event stopped *****\n"); - } spin_unlock_irqrestore(&ec->lock, tmp); ret = ec_poll(ec); spin_lock_irqsave(&ec->lock, tmp); @@ -440,8 +514,6 @@ static int acpi_ec_transaction(struct acpi_ec *ec, struct transaction *t) status = acpi_ec_transaction_unlocked(ec, t); - /* check if we received SCI during transaction */ - ec_check_sci_sync(ec, acpi_ec_read_status(ec)); if (test_bit(EC_FLAGS_GPE_STORM, &ec->flags)) { msleep(1); /* It is safe to enable the GPE outside of the transaction. */ @@ -792,29 +864,6 @@ static int acpi_ec_sync_query(struct acpi_ec *ec, u8 *data) return 0; } -static void acpi_ec_gpe_query(void *ec_cxt) -{ - struct acpi_ec *ec = ec_cxt; - - if (!ec) - return; - mutex_lock(&ec->mutex); - acpi_ec_sync_query(ec, NULL); - mutex_unlock(&ec->mutex); -} - -static int ec_check_sci(struct acpi_ec *ec, u8 state) -{ - if (state & ACPI_EC_FLAG_SCI) { - if (!test_and_set_bit(EC_FLAGS_QUERY_PENDING, &ec->flags)) { - pr_debug("***** Event started *****\n"); - return acpi_os_execute(OSL_NOTIFY_HANDLER, - acpi_ec_gpe_query, ec); - } - } - return 0; -} - static u32 acpi_ec_gpe_handler(acpi_handle gpe_device, u32 gpe_number, void *data) { @@ -825,10 +874,46 @@ static u32 acpi_ec_gpe_handler(acpi_handle gpe_device, if (advance_transaction(ec)) wake_up(&ec->wait); spin_unlock_irqrestore(&ec->lock, flags); - ec_check_sci(ec, acpi_ec_read_status(ec)); return ACPI_INTERRUPT_HANDLED | ACPI_REENABLE_GPE; } +static int acpi_ec_event_poller(void *context) +{ + struct acpi_ec *ec = context; + + while (!acpi_ec_wait_for_event(ec)) { + pr_debug("***** Event poller started *****\n"); + mutex_lock(&ec->mutex); + (void)acpi_ec_sync_query(ec, NULL); + mutex_unlock(&ec->mutex); + pr_debug("***** Event poller stopped *****\n"); + } + return 0; +} + +static int ec_create_event_poller(struct acpi_ec *ec) +{ + struct task_struct *t; + + t = kthread_run(acpi_ec_event_poller, ec, "ec/gpe-%lu", ec->gpe); + if (IS_ERR(t)) { + pr_err("failed to create event poller %lu\n", ec->gpe); + return PTR_ERR(t); + } + get_task_struct(t); + ec->thread = t; + return 0; +} + +static void ec_delete_event_poller(struct acpi_ec *ec) +{ + struct task_struct *t = ec->thread; + + ec->thread = NULL; + kthread_stop(t); + put_task_struct(t); +} + /* -------------------------------------------------------------------------- * Address Space Management * -------------------------------------------------------------------------- */ @@ -884,7 +969,6 @@ static struct acpi_ec *make_acpi_ec(void) if (!ec) return NULL; - ec->flags = 1 << EC_FLAGS_QUERY_PENDING; mutex_init(&ec->mutex); init_waitqueue_head(&ec->wait); INIT_LIST_HEAD(&ec->list); @@ -940,15 +1024,21 @@ ec_parse_device(acpi_handle handle, u32 Level, void *context, void **retval) static int ec_install_handlers(struct acpi_ec *ec) { + int ret; acpi_status status; if (test_bit(EC_FLAGS_HANDLERS_INSTALLED, &ec->flags)) return 0; + ret = ec_create_event_poller(ec); + if (ret) + return ret; status = acpi_install_gpe_handler(NULL, ec->gpe, ACPI_GPE_EDGE_TRIGGERED, &acpi_ec_gpe_handler, ec); - if (ACPI_FAILURE(status)) + if (ACPI_FAILURE(status)) { + ec_delete_event_poller(ec); return -ENODEV; + } acpi_ec_start(ec); status = acpi_install_address_space_handler(ec->handle, @@ -968,6 +1058,7 @@ static int ec_install_handlers(struct acpi_ec *ec) acpi_ec_stop(ec); acpi_remove_gpe_handler(NULL, ec->gpe, &acpi_ec_gpe_handler); + ec_delete_event_poller(ec); return -ENODEV; } } @@ -985,6 +1076,7 @@ static void ec_remove_handlers(struct acpi_ec *ec) if (ACPI_FAILURE(acpi_remove_gpe_handler(NULL, ec->gpe, &acpi_ec_gpe_handler))) pr_err("failed to remove gpe handler\n"); + ec_delete_event_poller(ec); clear_bit(EC_FLAGS_HANDLERS_INSTALLED, &ec->flags); } @@ -1032,7 +1124,7 @@ static int acpi_ec_add(struct acpi_device *device) ret = ec_install_handlers(ec); /* EC is fully operational, allow queries */ - clear_bit(EC_FLAGS_QUERY_PENDING, &ec->flags); + acpi_ec_enable_event(ec); /* Clear stale _Q events if hardware might require that */ if (EC_FLAGS_CLEAR_ON_RESUME) { diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h index bbcfe0b..20a569c 100644 --- a/drivers/acpi/internal.h +++ b/drivers/acpi/internal.h @@ -128,6 +128,7 @@ struct acpi_ec { struct list_head list; struct transaction *curr; spinlock_t lock; + struct task_struct *thread; }; extern struct acpi_ec *first_ec; -- 1.7.10 -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
