greybus: Revert "bootrom: Implement timeouts to detect Module failures"

This reverts commit 571348253032a86e4f0102d4dfadd390d0ea7e64.

With this patch gb_bootrom_timedout was getting called in interrupt
context and then proceeding to call functions that might block. In
practical terms, this was leading to a kernel BUG at mm/vmalloc.c:1650.

Signed-off-by: Jeffrey Carlyle <jcarlyle@google.com>
Acked-by: Johan Hovold <johan@hovoldconsulting.com>
This commit is contained in:
Jeffrey Carlyle 2016-05-06 11:29:37 -07:00
parent 69c8763eb8
commit 03b4fa4b5d

View file

@ -8,48 +8,24 @@
*/
#include <linux/firmware.h>
#include <linux/jiffies.h>
#include <linux/mutex.h>
#include <linux/timer.h>
#include "bootrom.h"
#include "greybus.h"
/* Timeout, in jiffies, within which the next request must be received */
#define NEXT_REQ_TIMEOUT_J msecs_to_jiffies(1000)
struct gb_bootrom {
struct gb_connection *connection;
const struct firmware *fw;
u8 protocol_major;
u8 protocol_minor;
struct timer_list timer;
struct mutex mutex; /* Protects bootrom->fw */
};
static void free_firmware(struct gb_bootrom *bootrom)
{
if (!bootrom->fw)
return;
release_firmware(bootrom->fw);
bootrom->fw = NULL;
}
static void gb_bootrom_timedout(unsigned long data)
{
struct gb_bootrom *bootrom = (struct gb_bootrom *)data;
struct device *dev = &bootrom->connection->bundle->dev;
dev_err(dev, "Timed out waiting for request from the Module\n");
mutex_lock(&bootrom->mutex);
free_firmware(bootrom);
mutex_unlock(&bootrom->mutex);
/* TODO: Power-off Module ? */
}
/*
* The es2 chip doesn't have VID/PID programmed into the hardware and we need to
* hack that up to distinguish different modules and their firmware blobs.
@ -101,7 +77,8 @@ static int download_firmware(struct gb_bootrom *bootrom, u8 stage)
int rc;
/* Already have a firmware, free it */
free_firmware(bootrom);
if (bootrom->fw)
free_firmware(bootrom);
/*
* Create firmware name
@ -137,32 +114,25 @@ static int gb_bootrom_firmware_size_request(struct gb_operation *op)
struct device *dev = &op->connection->bundle->dev;
int ret;
/* Disable timeouts */
del_timer_sync(&bootrom->timer);
if (op->request->payload_size != sizeof(*size_request)) {
dev_err(dev, "%s: illegal size of firmware size request (%zu != %zu)\n",
__func__, op->request->payload_size,
sizeof(*size_request));
ret = -EINVAL;
goto mod_timer;
return -EINVAL;
}
mutex_lock(&bootrom->mutex);
ret = download_firmware(bootrom, size_request->stage);
if (ret) {
dev_err(dev, "%s: failed to download firmware (%d)\n", __func__,
ret);
goto unlock;
return ret;
}
if (!gb_operation_response_alloc(op, sizeof(*size_response),
GFP_KERNEL)) {
dev_err(dev, "%s: error allocating response\n", __func__);
free_firmware(bootrom);
ret = -ENOMEM;
goto unlock;
return -ENOMEM;
}
size_response = op->response->payload;
@ -170,44 +140,28 @@ static int gb_bootrom_firmware_size_request(struct gb_operation *op)
dev_dbg(dev, "%s: firmware size %d bytes\n", __func__, size_response->size);
unlock:
mutex_unlock(&bootrom->mutex);
mod_timer:
/* Refresh timeout */
mod_timer(&bootrom->timer, jiffies + NEXT_REQ_TIMEOUT_J);
return ret;
return 0;
}
static int gb_bootrom_get_firmware(struct gb_operation *op)
{
struct gb_bootrom *bootrom = gb_connection_get_data(op->connection);
const struct firmware *fw;
const struct firmware *fw = bootrom->fw;
struct gb_bootrom_get_firmware_request *firmware_request;
struct gb_bootrom_get_firmware_response *firmware_response;
struct device *dev = &op->connection->bundle->dev;
unsigned int offset, size;
int ret = 0;
/* Disable timeouts */
del_timer_sync(&bootrom->timer);
if (op->request->payload_size != sizeof(*firmware_request)) {
dev_err(dev, "%s: Illegal size of get firmware request (%zu %zu)\n",
__func__, op->request->payload_size,
sizeof(*firmware_request));
ret = -EINVAL;
goto mod_timer;
return -EINVAL;
}
mutex_lock(&bootrom->mutex);
fw = bootrom->fw;
if (!fw) {
dev_err(dev, "%s: firmware not available\n", __func__);
ret = -EINVAL;
goto unlock;
return -EINVAL;
}
firmware_request = op->request->payload;
@ -217,15 +171,13 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
if (offset >= fw->size || size > fw->size - offset) {
dev_warn(dev, "bad firmware request (offs = %u, size = %u)\n",
offset, size);
ret = -EINVAL;
goto unlock;
return -EINVAL;
}
if (!gb_operation_response_alloc(op, sizeof(*firmware_response) + size,
GFP_KERNEL)) {
dev_err(dev, "%s: error allocating response\n", __func__);
ret = -ENOMEM;
goto unlock;
return -ENOMEM;
}
firmware_response = op->response->payload;
@ -234,59 +186,36 @@ static int gb_bootrom_get_firmware(struct gb_operation *op)
dev_dbg(dev, "responding with firmware (offs = %u, size = %u)\n", offset,
size);
unlock:
mutex_unlock(&bootrom->mutex);
mod_timer:
/* Refresh timeout */
mod_timer(&bootrom->timer, jiffies + NEXT_REQ_TIMEOUT_J);
return ret;
return 0;
}
static int gb_bootrom_ready_to_boot(struct gb_operation *op)
{
struct gb_connection *connection = op->connection;
struct gb_bootrom *bootrom = gb_connection_get_data(connection);
struct gb_bootrom_ready_to_boot_request *rtb_request;
struct device *dev = &connection->bundle->dev;
u8 status;
int ret = 0;
/* Disable timeouts */
del_timer_sync(&bootrom->timer);
if (op->request->payload_size != sizeof(*rtb_request)) {
dev_err(dev, "%s: Illegal size of ready to boot request (%zu %zu)\n",
__func__, op->request->payload_size,
sizeof(*rtb_request));
ret = -EINVAL;
goto mod_timer;
return -EINVAL;
}
rtb_request = op->request->payload;
status = rtb_request->status;
/* Return error if the blob was invalid */
if (status == GB_BOOTROM_BOOT_STATUS_INVALID) {
ret = -EINVAL;
goto mod_timer;
}
if (status == GB_BOOTROM_BOOT_STATUS_INVALID)
return -EINVAL;
/*
* XXX Should we return error for insecure firmware?
*/
dev_dbg(dev, "ready to boot: 0x%x, 0\n", status);
mod_timer:
/*
* Refresh timeout, the Interface shall load the new personality and
* send a new hotplug request, which shall get rid of the bootrom
* connection. As that can take some time, increase the timeout a bit.
*/
mod_timer(&bootrom->timer, jiffies + 5 * NEXT_REQ_TIMEOUT_J);
return ret;
return 0;
}
static int gb_bootrom_request_handler(struct gb_operation *op)
@ -375,11 +304,6 @@ static int gb_bootrom_probe(struct gb_bundle *bundle,
bootrom->connection = connection;
mutex_init(&bootrom->mutex);
init_timer(&bootrom->timer);
bootrom->timer.function = gb_bootrom_timedout;
bootrom->timer.data = (unsigned long)bootrom;
greybus_set_drvdata(bundle, bootrom);
ret = gb_connection_enable_tx(connection);
@ -405,9 +329,6 @@ static int gb_bootrom_probe(struct gb_bundle *bundle,
goto err_connection_disable;
}
/* Refresh timeout */
mod_timer(&bootrom->timer, jiffies + NEXT_REQ_TIMEOUT_J);
dev_dbg(&bundle->dev, "AP_READY sent\n");
return 0;
@ -430,16 +351,9 @@ static void gb_bootrom_disconnect(struct gb_bundle *bundle)
gb_connection_disable(bootrom->connection);
/* Disable timeouts */
del_timer_sync(&bootrom->timer);
/*
* Release firmware:
*
* As the connection and the timer are already disabled, we don't need
* to lock access to bootrom->fw here.
*/
free_firmware(bootrom);
/* Release firmware */
if (bootrom->fw)
free_firmware(bootrom);
gb_connection_destroy(bootrom->connection);
kfree(bootrom);