acpi: Ensure continuity of memmap data with a read cache

For multi-byte ACPI memmap reads, we previously had a mutex to ensure
data continuity. A better approach is to use a read cache. Since the
kernel will enable burst mode before reading a multi-byte memmap
variable and disable it afterward, we can populate the cache on the
first read after enabling burst. This solution removes deadlock bugs, is
contained entirely in acpi.c, and saves a deferred function.

BUG=chromium:514283
TEST=Manual on Glados. Add prints in acpi_read, verify that multi-byte
reads come from cache and non-burst reads continue to function as
before.
BRANCH=Cyan

Signed-off-by: Shawn Nematbakhsh <shawnn@chromium.org>
Change-Id: I74e4927bf2b433e31a9ff65d72820fa087c51722
Reviewed-on: https://chromium-review.googlesource.com/288871
Reviewed-by: Bill Richardson <wfrichar@chromium.org>
Reviewed-by: Vincent Palatin <vpalatin@chromium.org>
This commit is contained in:
Shawn Nematbakhsh
2015-07-27 14:36:53 -07:00
committed by ChromeOS Commit Bot
parent 0183e3cc7f
commit a9527fd686
10 changed files with 76 additions and 105 deletions

View File

@@ -113,7 +113,7 @@
#endif
#undef DEFERRABLE_MAX_COUNT
#define DEFERRABLE_MAX_COUNT 13
#define DEFERRABLE_MAX_COUNT 12
#ifndef __ASSEMBLER__

View File

@@ -87,7 +87,7 @@
#define I2C_PORT_USB_CHARGER_2 MEC1322_I2C3
#undef DEFERRABLE_MAX_COUNT
#define DEFERRABLE_MAX_COUNT 13
#define DEFERRABLE_MAX_COUNT 12
#define CONFIG_ALS
#define CONFIG_ALS_OPT3001

View File

@@ -49,7 +49,7 @@
(EC_WIRELESS_SWITCH_WLAN | EC_WIRELESS_SWITCH_WLAN_POWER)
#undef DEFERRABLE_MAX_COUNT
#define DEFERRABLE_MAX_COUNT 9
#define DEFERRABLE_MAX_COUNT 8
#ifndef __ASSEMBLER__

View File

@@ -82,7 +82,7 @@
/* Do we want EC_WIRELESS_SWITCH_WWAN as well? */
#undef DEFERRABLE_MAX_COUNT
#define DEFERRABLE_MAX_COUNT 10
#define DEFERRABLE_MAX_COUNT 9
#ifndef __ASSEMBLER__

View File

@@ -13,6 +13,7 @@
#include "ec_commands.h"
#include "pwm.h"
#include "timer.h"
#include "util.h"
/* Console output macros */
#define CPUTS(outstr) cputs(CC_LPC, outstr)
@@ -29,28 +30,77 @@ static int dptf_temp_sensor_id; /* last sensor ID written */
static int dptf_temp_threshold; /* last threshold written */
#endif
/*
* Keep a read cache of four bytes when burst mode is enabled, which is the
* size of the largest non-string memmap data type.
*/
#define ACPI_READ_CACHE_SIZE 4
/* Start address that indicates read cache is flushed. */
#define ACPI_READ_CACHE_FLUSHED (EC_ACPI_MEM_MAPPED_BEGIN - 1)
/* Calculate size of valid cache based upon end of memmap data. */
#define ACPI_VALID_CACHE_SIZE(addr) (MIN( \
EC_ACPI_MEM_MAPPED_SIZE + EC_ACPI_MEM_MAPPED_BEGIN - (addr), \
ACPI_READ_CACHE_SIZE))
/*
* In burst mode, read the requested memmap data and the data immediately
* following it into a cache. For future reads in burst mode, try to grab
* data from the cache. This ensures the continuity of multi-byte reads,
* which is important when dealing with data types > 8 bits.
*/
static struct {
int enabled;
uint8_t start_addr;
uint8_t data[ACPI_READ_CACHE_SIZE];
} acpi_read_cache;
/*
* Deferred function to ensure that ACPI burst mode doesn't remain enabled
* indefinitely.
*/
static void acpi_unlock_memmap_deferred(void)
static void acpi_disable_burst_deferred(void)
{
acpi_read_cache.enabled = 0;
lpc_clear_acpi_status_mask(EC_LPC_STATUS_BURST_MODE);
host_unlock_memmap();
CPUTS("ACPI force unlock mutex, missed burst disable?");
CPUTS("ACPI missed burst disable?");
}
DECLARE_DEFERRED(acpi_unlock_memmap_deferred);
DECLARE_DEFERRED(acpi_disable_burst_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)
/* Read memmapped data, returns read data or 0xff on error. */
static int acpi_read(uint8_t addr)
{
host_lock_memmap();
lpc_enable_acpi_interrupts();
uint8_t *memmap_addr = (uint8_t *)(lpc_get_memmap_range() + addr -
EC_ACPI_MEM_MAPPED_BEGIN);
/* Check for out-of-range read. */
if (addr < EC_ACPI_MEM_MAPPED_BEGIN ||
addr >= EC_ACPI_MEM_MAPPED_BEGIN + EC_ACPI_MEM_MAPPED_SIZE) {
CPRINTS("ACPI read 0x%02x (ignored)",
acpi_addr);
return 0xff;
}
/* Read from cache if enabled (burst mode). */
if (acpi_read_cache.enabled) {
/* Fetch to cache on miss. */
if (acpi_read_cache.start_addr == ACPI_READ_CACHE_FLUSHED ||
acpi_read_cache.start_addr > addr ||
addr - acpi_read_cache.start_addr >=
ACPI_READ_CACHE_SIZE) {
memcpy(acpi_read_cache.data,
memmap_addr,
ACPI_VALID_CACHE_SIZE(addr));
acpi_read_cache.start_addr = addr;
}
/* Return data from cache. */
return acpi_read_cache.data[addr - acpi_read_cache.start_addr];
} else {
/* Read directly from memmap data. */
return *memmap_addr;
}
}
DECLARE_DEFERRED(deferred_host_lock_memmap);
/*
* This handles AP writes to the EC via the ACPI I/O port. There are only a few
@@ -114,15 +164,7 @@ int acpi_ap_to_ec(int is_cmd, uint8_t value, uint8_t *resultptr)
break;
#endif
default:
if (acpi_addr >= EC_ACPI_MEM_MAPPED_BEGIN &&
acpi_addr <
EC_ACPI_MEM_MAPPED_BEGIN + EC_ACPI_MEM_MAPPED_SIZE)
result = *((uint8_t *)(lpc_get_memmap_range() +
acpi_addr -
EC_ACPI_MEM_MAPPED_BEGIN));
else
CPRINTS("ACPI read 0x%02x (ignored)",
acpi_addr);
result = acpi_read(acpi_addr);
break;
}
@@ -196,40 +238,29 @@ int acpi_ap_to_ec(int is_cmd, uint8_t value, uint8_t *resultptr)
* 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.
* to add a config to skip this code for certain chips.
*/
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();
}
acpi_read_cache.enabled = 1;
acpi_read_cache.start_addr = ACPI_READ_CACHE_FLUSHED;
/* Enter burst mode */
lpc_set_acpi_status_mask(EC_LPC_STATUS_BURST_MODE);
/*
* Unlock from deferred function in case burst mode is enabled
* Disable from deferred function in case burst mode is enabled
* for an extremely long time (ex. kernel bug / crash).
*/
hook_call_deferred(acpi_unlock_memmap_deferred, 1*SECOND);
hook_call_deferred(acpi_disable_burst_deferred, 1*SECOND);
/* ACPI 5.0-12.3.3: Burst ACK */
*resultptr = 0x90;
retval = 1;
} else if (acpi_cmd == EC_CMD_ACPI_BURST_DISABLE && !acpi_data_count) {
acpi_read_cache.enabled = 0;
/* Leave burst mode */
hook_call_deferred(acpi_unlock_memmap_deferred, -1);
hook_call_deferred(acpi_disable_burst_deferred, -1);
lpc_clear_acpi_status_mask(EC_LPC_STATUS_BURST_MODE);
host_unlock_memmap();
}
return retval;

View File

@@ -30,9 +30,7 @@ void als_task(void)
while (1) {
for (i = 0; i < EC_ALS_ENTRIES && i < ALS_COUNT; i++) {
als_data = als_read(i, &val) == EC_SUCCESS ? val : 0;
host_lock_memmap();
mapped[i] = als_data;
host_unlock_memmap();
}
task_wait_event(SECOND);

View File

@@ -124,12 +124,6 @@ static int update_static_battery_info(void)
*/
int rv;
/*
* We're updating multi-byte memmap vars, don't allow ACPI to do
* reads while we're updating.
*/
host_lock_memmap();
/* Smart battery serial number is 16 bits */
batt_str = (char *)host_get_memmap(EC_MEMMAP_BATT_SERIAL);
memset(batt_str, 0, EC_MEMMAP_TEXT_MAX);
@@ -173,9 +167,6 @@ static int update_static_battery_info(void)
*(int *)host_get_memmap(EC_MEMMAP_BATT_LFCC) = 0;
*host_get_memmap(EC_MEMMAP_BATT_FLAG) = 0;
/* No more multi-byte memmap writes. */
host_unlock_memmap();
if (rv)
problem(PR_STATIC_UPDATE, rv);
else
@@ -220,12 +211,6 @@ static void update_dynamic_battery_info(void)
batt_present = 0;
}
/*
* We're updating multi-byte memmap vars, don't allow ACPI to do
* reads while we're updating.
*/
host_lock_memmap();
if (!(curr.batt.flags & BATT_FLAG_BAD_VOLTAGE))
*memmap_volt = curr.batt.voltage;
@@ -252,9 +237,6 @@ static void update_dynamic_battery_info(void)
send_batt_info_event++;
}
/* No more multi-byte memmap writes. */
host_unlock_memmap();
if (curr.batt.is_present == BP_YES &&
!(curr.batt.flags & BATT_FLAG_BAD_STATE_OF_CHARGE) &&
curr.batt.state_of_charge <= BATTERY_LEVEL_CRITICAL)

View File

@@ -484,10 +484,7 @@ static void pwm_fan_second(void)
rpm = fan_get_rpm_actual(fans[fan].ch);
}
/* Lock ACPI read access to memmap during multi-byte write */
host_lock_memmap();
mapped[fan] = rpm;
host_unlock_memmap();
}
/*

View File

@@ -81,24 +81,6 @@ uint8_t *host_get_memmap(int offset)
#endif
}
static struct mutex memmap_write_mutex;
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);
}
int host_get_vboot_mode(void)
{
return g_vboot_mode;

View File

@@ -123,25 +123,6 @@ struct host_command {
*/
uint8_t *host_get_memmap(int offset);
/**
* Grab the memmap write mutex. This function should be called before
* multi-byte variable reads from ACPI, and before updating multi-byte
* memmap variables anywhere else.
*/
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
* memmap variables is done.
*/
void host_unlock_memmap(void);
/**
* Process a host command and return its response
*