From ddf77bbe785a5ae1c701ef70fb6e41a89168e341 Mon Sep 17 00:00:00 2001 From: Chiranjeevi Rapolu Date: Thu, 18 Jun 2015 23:47:43 -0700 Subject: [PATCH] Fix assertion crash in __wait_evt() mutex_lock() is called from MEC1322_IRQ_ACPIEC0_IBF interrupt context, causing deadlock and assertion in __wait_evt(). In the interrupt context it now checks for mutex lock first. If the mutex is already locked,, it will disable ACPI interrupts and defer the memmap mutex lock. Added LPC interrupt disable/enable functions as needed. Increased deferred function count where needed. BRANCH=None BUG=chrome-os-partner:40820 TEST=Test for suspend-resume, cold, warm reboots and other general stability. Change-Id: I3dda0d4635a6b6281faf200c8c7b6fcba8877254 Signed-off-by: Chiranjeevi Rapolu Reviewed-on: https://chromium-review.googlesource.com/280418 Reviewed-by: Randall Spangler Reviewed-by: Shawn N Commit-Queue: Divya Jyothi Tested-by: Divya Jyothi --- board/glados/board.h | 2 +- board/rambi/board.h | 3 +++ board/samus/board.h | 2 +- chip/it83xx/lpc.c | 12 ++++++++++++ chip/lm4/lpc.c | 12 ++++++++++++ chip/mec1322/lpc.c | 12 ++++++++++++ chip/npcx/lpc.c | 13 +++++++++++++ common/acpi.c | 34 +++++++++++++++++++++++++++++++++- common/host_command.c | 6 ++++++ include/host_command.h | 5 +++++ include/lpc.h | 7 +++++++ 11 files changed, 105 insertions(+), 3 deletions(-) diff --git a/board/glados/board.h b/board/glados/board.h index 6768b61749..4762f3916c 100644 --- a/board/glados/board.h +++ b/board/glados/board.h @@ -75,7 +75,7 @@ #define I2C_PORT_USB_CHARGER_2 MEC1322_I2C3 #undef DEFERRABLE_MAX_COUNT -#define DEFERRABLE_MAX_COUNT 10 +#define DEFERRABLE_MAX_COUNT 11 #ifndef __ASSEMBLER__ diff --git a/board/rambi/board.h b/board/rambi/board.h index 84d378d4df..f08e6a0266 100644 --- a/board/rambi/board.h +++ b/board/rambi/board.h @@ -48,6 +48,9 @@ #define CONFIG_WIRELESS_SUSPEND \ (EC_WIRELESS_SWITCH_WLAN | EC_WIRELESS_SWITCH_WLAN_POWER) +#undef DEFERRABLE_MAX_COUNT +#define DEFERRABLE_MAX_COUNT 9 + #ifndef __ASSEMBLER__ /* I2C ports */ diff --git a/board/samus/board.h b/board/samus/board.h index f1eb461c27..ee793ff8d2 100644 --- a/board/samus/board.h +++ b/board/samus/board.h @@ -81,7 +81,7 @@ /* Do we want EC_WIRELESS_SWITCH_WWAN as well? */ #undef DEFERRABLE_MAX_COUNT -#define DEFERRABLE_MAX_COUNT 9 +#define DEFERRABLE_MAX_COUNT 10 #ifndef __ASSEMBLER__ diff --git a/chip/it83xx/lpc.c b/chip/it83xx/lpc.c index 9db2719b87..95fcb23aee 100644 --- a/chip/it83xx/lpc.c +++ b/chip/it83xx/lpc.c @@ -638,6 +638,18 @@ void lpcrst_interrupt(enum gpio_signal signal) lpc_get_pltrst_asserted() ? "" : "de"); } +/* Enable LPC ACPI-EC interrupts */ +void lpc_enable_acpi_interrupts(void) +{ + task_enable_irq(IT83XX_IRQ_PMC_IN); +} + +/* Disable LPC ACPI-EC interrupts */ +void lpc_disable_acpi_interrupts(void) +{ + task_disable_irq(IT83XX_IRQ_PMC_IN); +} + static void lpc_resume(void) { /* Mask all host events until the host unmasks them itself. */ diff --git a/chip/lm4/lpc.c b/chip/lm4/lpc.c index 149145f3c0..2cca35f796 100644 --- a/chip/lm4/lpc.c +++ b/chip/lm4/lpc.c @@ -677,6 +677,18 @@ static void lpc_post_sysjump(void) memcpy(event_mask, prev_mask, sizeof(event_mask)); } +/* Enable LPC ACPI-EC interrupts */ +void lpc_enable_acpi_interrupts(void) +{ + LM4_LPC_LPCIM |= LM4_LPC_INT_MASK(LPC_CH_ACPI, 6); +} + +/* Disable LPC ACPI-EC interrupts */ +void lpc_disable_acpi_interrupts(void) +{ + LM4_LPC_LPCIM &= ~(LM4_LPC_INT_MASK(LPC_CH_ACPI, 6)); +} + static void lpc_init(void) { /* Enable LPC clock in run and sleep modes. */ diff --git a/chip/mec1322/lpc.c b/chip/mec1322/lpc.c index 7cdc120330..c486f75e77 100644 --- a/chip/mec1322/lpc.c +++ b/chip/mec1322/lpc.c @@ -557,6 +557,18 @@ int lpc_get_pltrst_asserted(void) return (MEC1322_LPC_BUS_MONITOR & (1<<1)) ? 1 : 0; } +/* Enable LPC ACPI-EC0 interrupts */ +void lpc_enable_acpi_interrupts(void) +{ + task_enable_irq(MEC1322_IRQ_ACPIEC0_IBF); +} + +/* Disable LPC ACPI-EC0 interrupts */ +void lpc_disable_acpi_interrupts(void) +{ + task_disable_irq(MEC1322_IRQ_ACPIEC0_IBF); +} + /* On boards without a host, this command is used to set up LPC */ static int lpc_command_init(int argc, char **argv) { diff --git a/chip/npcx/lpc.c b/chip/npcx/lpc.c index 79e0675ede..595edf28e9 100644 --- a/chip/npcx/lpc.c +++ b/chip/npcx/lpc.c @@ -646,6 +646,19 @@ static void lpc_init(void) /* initial IO port address via SIB-write modules */ system_lpc_host_register_init(); } + +/* Enable LPC ACPI-EC interrupts */ +void lpc_enable_acpi_interrupts(void) +{ + SET_BIT(NPCX_HIPMCTL(PM_CHAN_1), 0); +} + +/* Disable LPC ACPI-EC interrupts */ +void lpc_disable_acpi_interrupts(void) +{ + CLEAR_BIT(NPCX_HIPMCTL(PM_CHAN_1), 0); +} + /* * Set prio to higher than default; this way LPC memory mapped data is ready * before other inits try to initialize their memmap data. diff --git a/common/acpi.c b/common/acpi.c index 425f7e2337..b5cb763c40 100644 --- a/common/acpi.c +++ b/common/acpi.c @@ -41,6 +41,17 @@ static void acpi_unlock_memmap_deferred(void) } DECLARE_DEFERRED(acpi_unlock_memmap_deferred); +/* + * Deferred function to lock memmap and to enable LPC interrupts. + * This is due to LPC interrupt was unable to acquire memmap mutex. + */ +static void deferred_host_lock_memmap(void) +{ + host_lock_memmap(); + lpc_enable_acpi_interrupts(); +} +DECLARE_DEFERRED(deferred_host_lock_memmap); + /* * This handles AP writes to the EC via the ACPI I/O port. There are only a few * ACPI commands (EC_CMD_ACPI_*), but they are all handled here. @@ -180,8 +191,29 @@ int acpi_ap_to_ec(int is_cmd, uint8_t value, uint8_t *resultptr) *resultptr = evt_index; retval = 1; } else if (acpi_cmd == EC_CMD_ACPI_BURST_ENABLE && !acpi_data_count) { + /* + * TODO: The kernel only enables BURST when doing multi-byte + * value reads over the ACPI port. We don't do such reads + * when our memmap data can be accessed directly over LPC, + * so on LM4, for example, this is dead code. We might want + * to re-add the CONFIG, now that we have overhead of one + * deferred function. + */ + if (host_memmap_is_locked()) { + /* + * If already locked by a task, we can not acquire + * the mutex and will have to wait in the interrupt + * context. But then the task will not get chance to + * release the mutex. This will create deadlock + * situation. To avoid the deadlock, disable ACPI + * interrupts and defer locking. + */ + lpc_disable_acpi_interrupts(); + hook_call_deferred(deferred_host_lock_memmap, 0); + } else { + host_lock_memmap(); + } /* Enter burst mode */ - host_lock_memmap(); lpc_set_acpi_status_mask(EC_LPC_STATUS_BURST_MODE); /* diff --git a/common/host_command.c b/common/host_command.c index da73d2dbd5..bc40a68441 100644 --- a/common/host_command.c +++ b/common/host_command.c @@ -88,6 +88,12 @@ void host_lock_memmap() mutex_lock(&memmap_write_mutex); } +/* Returns host memmap lock status */ +int host_memmap_is_locked(void) +{ + return (memmap_write_mutex.lock != 0); +} + void host_unlock_memmap() { mutex_unlock(&memmap_write_mutex); diff --git a/include/host_command.h b/include/host_command.h index 2667cb9a27..0d1ff998f3 100644 --- a/include/host_command.h +++ b/include/host_command.h @@ -130,6 +130,11 @@ uint8_t *host_get_memmap(int offset); */ void host_lock_memmap(void); +/* + * Returns status of host memmap lock. + */ +int host_memmap_is_locked(void); + /** * Release the memmap write mutex. This function should be called once * a multi-byte variable read from ACPI is done, and when updating multi-byte diff --git a/include/lpc.h b/include/lpc.h index c38f711bda..c90a54a71c 100644 --- a/include/lpc.h +++ b/include/lpc.h @@ -113,4 +113,11 @@ void lpc_clear_acpi_status_mask(uint8_t mask); */ int lpc_get_pltrst_asserted(void); + +/* Disable LPC ACPI interrupts */ +void lpc_disable_acpi_interrupts(void); + +/* Enable LPC ACPI interrupts */ +void lpc_enable_acpi_interrupts(void); + #endif /* __CROS_EC_LPC_H */