From 6b68f03f95e3f0aeea0c47799aecb296276a7cd6 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:13:30 +0800 Subject: ACPI / IPMI: Fix potential response buffer overflow This patch enhances sanity checks on message size to avoid potential buffer overflow. The kernel IPMI message size is IPMI_MAX_MSG_LENGTH(272 bytes) while the ACPI specification defined IPMI message size is 64 bytes. The difference is not handled by the original codes. This may cause crash in the response handling codes. This patch closes this gap and also combines rx_data/tx_data to use single data/len pair since they need not be seperate. Signed-off-by: Lv Zheng Reviewed-by: Huang Ying Signed-off-by: Rafael J. Wysocki diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index a6977e1..7397135 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -52,6 +52,7 @@ MODULE_LICENSE("GPL"); #define ACPI_IPMI_UNKNOWN 0x07 /* the IPMI timeout is 5s */ #define IPMI_TIMEOUT (5 * HZ) +#define ACPI_IPMI_MAX_MSG_LENGTH 64 struct acpi_ipmi_device { /* the device list attached to driver_data.ipmi_devices */ @@ -90,11 +91,9 @@ struct acpi_ipmi_msg { struct completion tx_complete; struct kernel_ipmi_msg tx_message; int msg_done; - /* tx data . And copy it from ACPI object buffer */ - u8 tx_data[64]; - int tx_len; - u8 rx_data[64]; - int rx_len; + /* tx/rx data . And copy it from/to ACPI object buffer */ + u8 data[ACPI_IPMI_MAX_MSG_LENGTH]; + u8 rx_len; struct acpi_ipmi_device *device; }; @@ -102,7 +101,7 @@ struct acpi_ipmi_msg { struct acpi_ipmi_buffer { u8 status; u8 length; - u8 data[64]; + u8 data[ACPI_IPMI_MAX_MSG_LENGTH]; }; static void ipmi_register_bmc(int iface, struct device *dev); @@ -141,7 +140,7 @@ static struct acpi_ipmi_msg *acpi_alloc_ipmi_msg(struct acpi_ipmi_device *ipmi) #define IPMI_OP_RGN_NETFN(offset) ((offset >> 8) & 0xff) #define IPMI_OP_RGN_CMD(offset) (offset & 0xff) -static void acpi_format_ipmi_msg(struct acpi_ipmi_msg *tx_msg, +static int acpi_format_ipmi_request(struct acpi_ipmi_msg *tx_msg, acpi_physical_address address, acpi_integer *value) { @@ -157,15 +156,21 @@ static void acpi_format_ipmi_msg(struct acpi_ipmi_msg *tx_msg, */ msg->netfn = IPMI_OP_RGN_NETFN(address); msg->cmd = IPMI_OP_RGN_CMD(address); - msg->data = tx_msg->tx_data; + msg->data = tx_msg->data; /* * value is the parameter passed by the IPMI opregion space handler. * It points to the IPMI request message buffer */ buffer = (struct acpi_ipmi_buffer *)value; /* copy the tx message data */ + if (buffer->length > ACPI_IPMI_MAX_MSG_LENGTH) { + dev_WARN_ONCE(&tx_msg->device->pnp_dev->dev, true, + "Unexpected request (msg len %d).\n", + buffer->length); + return -EINVAL; + } msg->data_len = buffer->length; - memcpy(tx_msg->tx_data, buffer->data, msg->data_len); + memcpy(tx_msg->data, buffer->data, msg->data_len); /* * now the default type is SYSTEM_INTERFACE and channel type is BMC. * If the netfn is APP_REQUEST and the cmd is SEND_MESSAGE, @@ -183,6 +188,7 @@ static void acpi_format_ipmi_msg(struct acpi_ipmi_msg *tx_msg, device->curr_msgid++; tx_msg->tx_msgid = device->curr_msgid; spin_unlock_irqrestore(&device->tx_msg_lock, flags); + return 0; } static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg, @@ -214,7 +220,7 @@ static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg, */ buffer->status = ACPI_IPMI_OK; buffer->length = msg->rx_len; - memcpy(buffer->data, msg->rx_data, msg->rx_len); + memcpy(buffer->data, msg->data, msg->rx_len); } static void ipmi_flush_tx_msg(struct acpi_ipmi_device *ipmi) @@ -250,8 +256,7 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) dev_warn(&pnp_dev->dev, "Unexpected response is returned. " "returned user %p, expected user %p\n", msg->user, ipmi_device->user_interface); - ipmi_free_recv_msg(msg); - return; + goto out_msg; } spin_lock_irqsave(&ipmi_device->tx_msg_lock, flags); list_for_each_entry(tx_msg, &ipmi_device->tx_msg_list, head) { @@ -265,17 +270,21 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) if (!msg_found) { dev_warn(&pnp_dev->dev, "Unexpected response (msg id %ld) is " "returned.\n", msg->msgid); - ipmi_free_recv_msg(msg); - return; + goto out_msg; } - if (msg->msg.data_len) { - /* copy the response data to Rx_data buffer */ - memcpy(tx_msg->rx_data, msg->msg_data, msg->msg.data_len); + /* copy the response data to Rx_data buffer */ + if (msg->msg.data_len > ACPI_IPMI_MAX_MSG_LENGTH) { + dev_WARN_ONCE(&pnp_dev->dev, true, + "Unexpected response (msg len %d).\n", + msg->msg.data_len); + } else { tx_msg->rx_len = msg->msg.data_len; + memcpy(tx_msg->data, msg->msg.data, tx_msg->rx_len); tx_msg->msg_done = 1; } complete(&tx_msg->tx_complete); +out_msg: ipmi_free_recv_msg(msg); }; @@ -398,7 +407,10 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, if (!tx_msg) return AE_NO_MEMORY; - acpi_format_ipmi_msg(tx_msg, address, value); + if (acpi_format_ipmi_request(tx_msg, address, value) != 0) { + status = AE_TYPE; + goto out_msg; + } spin_lock_irqsave(&ipmi_device->tx_msg_lock, flags); list_add_tail(&tx_msg->head, &ipmi_device->tx_msg_list); spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); @@ -409,17 +421,18 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, NULL, 0, 0, 0); if (err) { status = AE_ERROR; - goto end_label; + goto out_list; } rem_time = wait_for_completion_timeout(&tx_msg->tx_complete, IPMI_TIMEOUT); acpi_format_ipmi_response(tx_msg, value, rem_time); status = AE_OK; -end_label: +out_list: spin_lock_irqsave(&ipmi_device->tx_msg_lock, flags); list_del(&tx_msg->head); spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); +out_msg: kfree(tx_msg); return status; } -- cgit v0.10.2 From 5ac557ef4951ea4b131ae45b08434546cb386ac5 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:13:39 +0800 Subject: ACPI / IPMI: Fix race caused by the unprotected ACPI IPMI transfers This patch fixes races caused by unprotected ACPI IPMI transfers. We can see that the following crashes may occur: 1. There is no tx_msg_lock held for iterating tx_msg_list in ipmi_flush_tx_msg() while it may be unlinked on failure in parallel in acpi_ipmi_space_handler() under tx_msg_lock. 2. There is no lock held for freeing tx_msg in acpi_ipmi_space_handler() while it may be accessed in parallel in ipmi_flush_tx_msg() and ipmi_msg_handler(). This patch enhances tx_msg_lock to protect all tx_msg accesses to solve this issue. Then tx_msg_lock is always held around complete() and tx_msg accesses. Signed-off-by: Lv Zheng Reviewed-by: Huang Ying Signed-off-by: Rafael J. Wysocki diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index 7397135..87307ba 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -228,11 +228,14 @@ static void ipmi_flush_tx_msg(struct acpi_ipmi_device *ipmi) struct acpi_ipmi_msg *tx_msg, *temp; int count = HZ / 10; struct pnp_dev *pnp_dev = ipmi->pnp_dev; + unsigned long flags; + spin_lock_irqsave(&ipmi->tx_msg_lock, flags); list_for_each_entry_safe(tx_msg, temp, &ipmi->tx_msg_list, head) { /* wake up the sleep thread on the Tx msg */ complete(&tx_msg->tx_complete); } + spin_unlock_irqrestore(&ipmi->tx_msg_lock, flags); /* wait for about 100ms to flush the tx message list */ while (count--) { @@ -266,11 +269,10 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) } } - spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); if (!msg_found) { dev_warn(&pnp_dev->dev, "Unexpected response (msg id %ld) is " "returned.\n", msg->msgid); - goto out_msg; + goto out_lock; } /* copy the response data to Rx_data buffer */ @@ -284,6 +286,8 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) tx_msg->msg_done = 1; } complete(&tx_msg->tx_complete); +out_lock: + spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); out_msg: ipmi_free_recv_msg(msg); }; -- cgit v0.10.2 From 8584ec6ae9cc386de344e0d33b60f76368bb73ab Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:13:47 +0800 Subject: ACPI / IPMI: Fix race caused by the timed out ACPI IPMI transfers This patch fixes races caused by timed out ACPI IPMI transfers. This patch uses timeout mechanism provided by ipmi_si to avoid the race that the msg_done flag is set but without any protection, its content can be invalid. Thanks for the suggestion of Corey Minyard. Signed-off-by: Lv Zheng Reviewed-by: Huang Ying Signed-off-by: Rafael J. Wysocki diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index 87307ba..9171a1a 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -51,7 +51,7 @@ MODULE_LICENSE("GPL"); #define ACPI_IPMI_TIMEOUT 0x10 #define ACPI_IPMI_UNKNOWN 0x07 /* the IPMI timeout is 5s */ -#define IPMI_TIMEOUT (5 * HZ) +#define IPMI_TIMEOUT (5000) #define ACPI_IPMI_MAX_MSG_LENGTH 64 struct acpi_ipmi_device { @@ -135,6 +135,7 @@ static struct acpi_ipmi_msg *acpi_alloc_ipmi_msg(struct acpi_ipmi_device *ipmi) init_completion(&ipmi_msg->tx_complete); INIT_LIST_HEAD(&ipmi_msg->head); ipmi_msg->device = ipmi; + ipmi_msg->msg_done = ACPI_IPMI_UNKNOWN; return ipmi_msg; } @@ -192,7 +193,7 @@ static int acpi_format_ipmi_request(struct acpi_ipmi_msg *tx_msg, } static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg, - acpi_integer *value, int rem_time) + acpi_integer *value) { struct acpi_ipmi_buffer *buffer; @@ -201,24 +202,17 @@ static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg, * IPMI message returned by IPMI command. */ buffer = (struct acpi_ipmi_buffer *)value; - if (!rem_time && !msg->msg_done) { - buffer->status = ACPI_IPMI_TIMEOUT; - return; - } /* - * If the flag of msg_done is not set or the recv length is zero, it - * means that the IPMI command is not executed correctly. - * The status code will be ACPI_IPMI_UNKNOWN. + * If the flag of msg_done is not set, it means that the IPMI command is + * not executed correctly. */ - if (!msg->msg_done || !msg->rx_len) { - buffer->status = ACPI_IPMI_UNKNOWN; + buffer->status = msg->msg_done; + if (msg->msg_done != ACPI_IPMI_OK) return; - } /* * If the IPMI response message is obtained correctly, the status code * will be ACPI_IPMI_OK */ - buffer->status = ACPI_IPMI_OK; buffer->length = msg->rx_len; memcpy(buffer->data, msg->data, msg->rx_len); } @@ -280,11 +274,23 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) dev_WARN_ONCE(&pnp_dev->dev, true, "Unexpected response (msg len %d).\n", msg->msg.data_len); - } else { - tx_msg->rx_len = msg->msg.data_len; - memcpy(tx_msg->data, msg->msg.data, tx_msg->rx_len); - tx_msg->msg_done = 1; + goto out_comp; } + /* response msg is an error msg */ + msg->recv_type = IPMI_RESPONSE_RECV_TYPE; + if (msg->recv_type == IPMI_RESPONSE_RECV_TYPE && + msg->msg.data_len == 1) { + if (msg->msg.data[0] == IPMI_TIMEOUT_COMPLETION_CODE) { + dev_WARN_ONCE(&pnp_dev->dev, true, + "Unexpected response (timeout).\n"); + tx_msg->msg_done = ACPI_IPMI_TIMEOUT; + } + goto out_comp; + } + tx_msg->rx_len = msg->msg.data_len; + memcpy(tx_msg->data, msg->msg.data, tx_msg->rx_len); + tx_msg->msg_done = ACPI_IPMI_OK; +out_comp: complete(&tx_msg->tx_complete); out_lock: spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); @@ -392,7 +398,7 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, { struct acpi_ipmi_msg *tx_msg; struct acpi_ipmi_device *ipmi_device = handler_context; - int err, rem_time; + int err; acpi_status status; unsigned long flags; /* @@ -422,14 +428,13 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, &tx_msg->addr, tx_msg->tx_msgid, &tx_msg->tx_message, - NULL, 0, 0, 0); + NULL, 0, 0, IPMI_TIMEOUT); if (err) { status = AE_ERROR; goto out_list; } - rem_time = wait_for_completion_timeout(&tx_msg->tx_complete, - IPMI_TIMEOUT); - acpi_format_ipmi_response(tx_msg, value, rem_time); + wait_for_completion(&tx_msg->tx_complete); + acpi_format_ipmi_response(tx_msg, value); status = AE_OK; out_list: -- cgit v0.10.2 From a1a69b297e4775298d6407357332ea1adc218396 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:13:54 +0800 Subject: ACPI / IPMI: Fix race caused by the unprotected ACPI IPMI user This patch uses reference counting to fix the race caused by the unprotected ACPI IPMI user. There are two rules for using the ipmi_si APIs: 1. In ipmi_si, ipmi_destroy_user() can ensure that no ipmi_recv_msg will be passed to ipmi_msg_handler(), but ipmi_request_settime() can not use an invalid ipmi_user_t. This means the ipmi_si users must ensure that there won't be any local references on ipmi_user_t before invoking ipmi_destroy_user(). 2. In ipmi_si, the smi_gone()/new_smi() callbacks are protected by smi_watchers_mutex, so their execution is serialized. But as a new smi can re-use a freed intf_num, it requires that the callback implementation must not use intf_num as an identification mean or it must ensure all references to the previous smi are all dropped before exiting smi_gone() callback. As the acpi_ipmi_device->user_interface check in acpi_ipmi_space_handler() can happen before setting user_interface to NULL and codes after the check in acpi_ipmi_space_handler() can happen after user_interface becomes NULL, the on-going acpi_ipmi_space_handler() still can pass an invalid acpi_ipmi_device->user_interface to ipmi_request_settime(). Such race conditions are not allowed by the IPMI layer's API design as a crash will happen in ipmi_request_settime() if something like that happens. This patch follows the ipmi_devintf.c design: 1. Invoke ipmi_destroy_user() after the reference count of acpi_ipmi_device drops to 0. References of acpi_ipmi_device dropping to 0 also means tx_msg related to this acpi_ipmi_device are all freed. This matches the IPMI layer's API calling rule on ipmi_destroy_user() and ipmi_request_settime(). 2. ipmi_flush_tx_msg() is performed so that no on-going tx_msg can still be running in acpi_ipmi_space_handler(). And it is invoked after invoking __ipmi_dev_kill() where acpi_ipmi_device is deleted from the list with a "dead" flag set, and the "dead" flag check is also introduced to the point where a tx_msg is going to be added to the tx_msg_list so that no new tx_msg can be created after returning from the __ipmi_dev_kill(). 3. The waiting codes in ipmi_flush_tx_msg() is deleted because it is not required since this patch ensures no acpi_ipmi reference is still held for ipmi_user_t before calling ipmi_destroy_user() and ipmi_destroy_user() can ensure no more ipmi_msg_handler() can happen after returning from ipmi_destroy_user(). 4. The flushing of tx_msg is also moved out of ipmi_lock in this patch. The forthcoming IPMI operation region handler installation changes also requires acpi_ipmi_device be handled in this style. The header comment of the file is also updated due to this design change. Signed-off-by: Lv Zheng Reviewed-by: Huang Ying Signed-off-by: Rafael J. Wysocki diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index 9171a1a..b285386 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -1,8 +1,9 @@ /* * acpi_ipmi.c - ACPI IPMI opregion * - * Copyright (C) 2010 Intel Corporation - * Copyright (C) 2010 Zhao Yakui + * Copyright (C) 2010, 2013 Intel Corporation + * Author: Zhao Yakui + * Lv Zheng * * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ * @@ -67,6 +68,8 @@ struct acpi_ipmi_device { long curr_msgid; unsigned long flags; struct ipmi_smi_info smi_data; + bool dead; + struct kref kref; }; struct ipmi_driver_data { @@ -107,8 +110,8 @@ struct acpi_ipmi_buffer { static void ipmi_register_bmc(int iface, struct device *dev); static void ipmi_bmc_gone(int iface); static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data); -static void acpi_add_ipmi_device(struct acpi_ipmi_device *ipmi_device); -static void acpi_remove_ipmi_device(struct acpi_ipmi_device *ipmi_device); +static int ipmi_install_space_handler(struct acpi_ipmi_device *ipmi); +static void ipmi_remove_space_handler(struct acpi_ipmi_device *ipmi); static struct ipmi_driver_data driver_data = { .ipmi_devices = LIST_HEAD_INIT(driver_data.ipmi_devices), @@ -122,6 +125,88 @@ static struct ipmi_driver_data driver_data = { }, }; +static struct acpi_ipmi_device * +ipmi_dev_alloc(int iface, struct ipmi_smi_info *smi_data, acpi_handle handle) +{ + struct acpi_ipmi_device *ipmi_device; + int err; + ipmi_user_t user; + + ipmi_device = kzalloc(sizeof(*ipmi_device), GFP_KERNEL); + if (!ipmi_device) + return NULL; + + kref_init(&ipmi_device->kref); + INIT_LIST_HEAD(&ipmi_device->head); + INIT_LIST_HEAD(&ipmi_device->tx_msg_list); + spin_lock_init(&ipmi_device->tx_msg_lock); + + ipmi_device->handle = handle; + ipmi_device->pnp_dev = to_pnp_dev(get_device(smi_data->dev)); + memcpy(&ipmi_device->smi_data, smi_data, sizeof(struct ipmi_smi_info)); + ipmi_device->ipmi_ifnum = iface; + + err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs, + ipmi_device, &user); + if (err) { + put_device(smi_data->dev); + kfree(ipmi_device); + return NULL; + } + ipmi_device->user_interface = user; + ipmi_install_space_handler(ipmi_device); + + return ipmi_device; +} + +static void ipmi_dev_release(struct acpi_ipmi_device *ipmi_device) +{ + ipmi_remove_space_handler(ipmi_device); + ipmi_destroy_user(ipmi_device->user_interface); + put_device(ipmi_device->smi_data.dev); + kfree(ipmi_device); +} + +static void ipmi_dev_release_kref(struct kref *kref) +{ + struct acpi_ipmi_device *ipmi = + container_of(kref, struct acpi_ipmi_device, kref); + + ipmi_dev_release(ipmi); +} + +static void __ipmi_dev_kill(struct acpi_ipmi_device *ipmi_device) +{ + list_del(&ipmi_device->head); + /* + * Always setting dead flag after deleting from the list or + * list_for_each_entry() codes must get changed. + */ + ipmi_device->dead = true; +} + +static struct acpi_ipmi_device *acpi_ipmi_dev_get(int iface) +{ + struct acpi_ipmi_device *temp, *ipmi_device = NULL; + + mutex_lock(&driver_data.ipmi_lock); + list_for_each_entry(temp, &driver_data.ipmi_devices, head) { + if (temp->ipmi_ifnum == iface) { + ipmi_device = temp; + kref_get(&ipmi_device->kref); + break; + } + } + mutex_unlock(&driver_data.ipmi_lock); + + return ipmi_device; +} + +static void acpi_ipmi_dev_put(struct acpi_ipmi_device *ipmi_device) +{ + kref_put(&ipmi_device->kref, ipmi_dev_release_kref); +} + static struct acpi_ipmi_msg *acpi_alloc_ipmi_msg(struct acpi_ipmi_device *ipmi) { struct acpi_ipmi_msg *ipmi_msg; @@ -220,25 +305,22 @@ static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg, static void ipmi_flush_tx_msg(struct acpi_ipmi_device *ipmi) { struct acpi_ipmi_msg *tx_msg, *temp; - int count = HZ / 10; - struct pnp_dev *pnp_dev = ipmi->pnp_dev; unsigned long flags; + /* + * NOTE: On-going ipmi_recv_msg + * ipmi_msg_handler() may still be invoked by ipmi_si after + * flushing. But it is safe to do a fast flushing on module_exit() + * without waiting for all ipmi_recv_msg(s) to complete from + * ipmi_msg_handler() as it is ensured by ipmi_si that all + * ipmi_recv_msg(s) are freed after invoking ipmi_destroy_user(). + */ spin_lock_irqsave(&ipmi->tx_msg_lock, flags); list_for_each_entry_safe(tx_msg, temp, &ipmi->tx_msg_list, head) { /* wake up the sleep thread on the Tx msg */ complete(&tx_msg->tx_complete); } spin_unlock_irqrestore(&ipmi->tx_msg_lock, flags); - - /* wait for about 100ms to flush the tx message list */ - while (count--) { - if (list_empty(&ipmi->tx_msg_list)) - break; - schedule_timeout(1); - } - if (!list_empty(&ipmi->tx_msg_list)) - dev_warn(&pnp_dev->dev, "tx msg list is not NULL\n"); } static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) @@ -302,7 +384,6 @@ static void ipmi_register_bmc(int iface, struct device *dev) { struct acpi_ipmi_device *ipmi_device, *temp; struct pnp_dev *pnp_dev; - ipmi_user_t user; int err; struct ipmi_smi_info smi_data; acpi_handle handle; @@ -312,12 +393,18 @@ static void ipmi_register_bmc(int iface, struct device *dev) if (err) return; - if (smi_data.addr_src != SI_ACPI) { - put_device(smi_data.dev); - return; - } - + if (smi_data.addr_src != SI_ACPI) + goto err_ref; handle = smi_data.addr_info.acpi_info.acpi_handle; + if (!handle) + goto err_ref; + pnp_dev = to_pnp_dev(smi_data.dev); + + ipmi_device = ipmi_dev_alloc(iface, &smi_data, handle); + if (!ipmi_device) { + dev_warn(&pnp_dev->dev, "Can't create IPMI user interface\n"); + goto err_ref; + } mutex_lock(&driver_data.ipmi_lock); list_for_each_entry(temp, &driver_data.ipmi_devices, head) { @@ -326,34 +413,18 @@ static void ipmi_register_bmc(int iface, struct device *dev) * to the device list, don't add it again. */ if (temp->handle == handle) - goto out; + goto err_lock; } - ipmi_device = kzalloc(sizeof(*ipmi_device), GFP_KERNEL); - - if (!ipmi_device) - goto out; - - pnp_dev = to_pnp_dev(smi_data.dev); - ipmi_device->handle = handle; - ipmi_device->pnp_dev = pnp_dev; - - err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs, - ipmi_device, &user); - if (err) { - dev_warn(&pnp_dev->dev, "Can't create IPMI user interface\n"); - kfree(ipmi_device); - goto out; - } - acpi_add_ipmi_device(ipmi_device); - ipmi_device->user_interface = user; - ipmi_device->ipmi_ifnum = iface; + list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices); mutex_unlock(&driver_data.ipmi_lock); - memcpy(&ipmi_device->smi_data, &smi_data, sizeof(struct ipmi_smi_info)); + put_device(smi_data.dev); return; -out: +err_lock: mutex_unlock(&driver_data.ipmi_lock); + ipmi_dev_release(ipmi_device); +err_ref: put_device(smi_data.dev); return; } @@ -361,19 +432,22 @@ out: static void ipmi_bmc_gone(int iface) { struct acpi_ipmi_device *ipmi_device, *temp; + bool dev_found = false; mutex_lock(&driver_data.ipmi_lock); list_for_each_entry_safe(ipmi_device, temp, &driver_data.ipmi_devices, head) { - if (ipmi_device->ipmi_ifnum != iface) - continue; - - acpi_remove_ipmi_device(ipmi_device); - put_device(ipmi_device->smi_data.dev); - kfree(ipmi_device); - break; + if (ipmi_device->ipmi_ifnum != iface) { + dev_found = true; + __ipmi_dev_kill(ipmi_device); + break; + } } mutex_unlock(&driver_data.ipmi_lock); + if (dev_found) { + ipmi_flush_tx_msg(ipmi_device); + acpi_ipmi_dev_put(ipmi_device); + } } /* -------------------------------------------------------------------------- * Address Space Management @@ -397,7 +471,8 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, void *handler_context, void *region_context) { struct acpi_ipmi_msg *tx_msg; - struct acpi_ipmi_device *ipmi_device = handler_context; + int iface = (long)handler_context; + struct acpi_ipmi_device *ipmi_device; int err; acpi_status status; unsigned long flags; @@ -410,20 +485,31 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, if ((function & ACPI_IO_MASK) == ACPI_READ) return AE_TYPE; - if (!ipmi_device->user_interface) + ipmi_device = acpi_ipmi_dev_get(iface); + if (!ipmi_device) return AE_NOT_EXIST; tx_msg = acpi_alloc_ipmi_msg(ipmi_device); - if (!tx_msg) - return AE_NO_MEMORY; + if (!tx_msg) { + status = AE_NO_MEMORY; + goto out_ref; + } if (acpi_format_ipmi_request(tx_msg, address, value) != 0) { status = AE_TYPE; goto out_msg; } + mutex_lock(&driver_data.ipmi_lock); + /* Do not add a tx_msg that can not be flushed. */ + if (ipmi_device->dead) { + status = AE_NOT_EXIST; + mutex_unlock(&driver_data.ipmi_lock); + goto out_msg; + } spin_lock_irqsave(&ipmi_device->tx_msg_lock, flags); list_add_tail(&tx_msg->head, &ipmi_device->tx_msg_list); spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); + mutex_unlock(&driver_data.ipmi_lock); err = ipmi_request_settime(ipmi_device->user_interface, &tx_msg->addr, tx_msg->tx_msgid, @@ -443,6 +529,8 @@ out_list: spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); out_msg: kfree(tx_msg); +out_ref: + acpi_ipmi_dev_put(ipmi_device); return status; } @@ -465,9 +553,8 @@ static int ipmi_install_space_handler(struct acpi_ipmi_device *ipmi) return 0; status = acpi_install_address_space_handler(ipmi->handle, - ACPI_ADR_SPACE_IPMI, - &acpi_ipmi_space_handler, - NULL, ipmi); + ACPI_ADR_SPACE_IPMI, &acpi_ipmi_space_handler, + NULL, (void *)((long)ipmi->ipmi_ifnum)); if (ACPI_FAILURE(status)) { struct pnp_dev *pnp_dev = ipmi->pnp_dev; dev_warn(&pnp_dev->dev, "Can't register IPMI opregion space " @@ -478,36 +565,6 @@ static int ipmi_install_space_handler(struct acpi_ipmi_device *ipmi) return 0; } -static void acpi_add_ipmi_device(struct acpi_ipmi_device *ipmi_device) -{ - - INIT_LIST_HEAD(&ipmi_device->head); - - spin_lock_init(&ipmi_device->tx_msg_lock); - INIT_LIST_HEAD(&ipmi_device->tx_msg_list); - ipmi_install_space_handler(ipmi_device); - - list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices); -} - -static void acpi_remove_ipmi_device(struct acpi_ipmi_device *ipmi_device) -{ - /* - * If the IPMI user interface is created, it should be - * destroyed. - */ - if (ipmi_device->user_interface) { - ipmi_destroy_user(ipmi_device->user_interface); - ipmi_device->user_interface = NULL; - } - /* flush the Tx_msg list */ - if (!list_empty(&ipmi_device->tx_msg_list)) - ipmi_flush_tx_msg(ipmi_device); - - list_del(&ipmi_device->head); - ipmi_remove_space_handler(ipmi_device); -} - static int __init acpi_ipmi_init(void) { int result = 0; @@ -524,7 +581,7 @@ static int __init acpi_ipmi_init(void) static void __exit acpi_ipmi_exit(void) { - struct acpi_ipmi_device *ipmi_device, *temp; + struct acpi_ipmi_device *ipmi_device; if (acpi_disabled) return; @@ -538,11 +595,17 @@ static void __exit acpi_ipmi_exit(void) * handler and free it. */ mutex_lock(&driver_data.ipmi_lock); - list_for_each_entry_safe(ipmi_device, temp, - &driver_data.ipmi_devices, head) { - acpi_remove_ipmi_device(ipmi_device); - put_device(ipmi_device->smi_data.dev); - kfree(ipmi_device); + while (!list_empty(&driver_data.ipmi_devices)) { + ipmi_device = list_first_entry(&driver_data.ipmi_devices, + struct acpi_ipmi_device, + head); + __ipmi_dev_kill(ipmi_device); + mutex_unlock(&driver_data.ipmi_lock); + + ipmi_flush_tx_msg(ipmi_device); + acpi_ipmi_dev_put(ipmi_device); + + mutex_lock(&driver_data.ipmi_lock); } mutex_unlock(&driver_data.ipmi_lock); } -- cgit v0.10.2 From e96a94edd7ae302168e17daa0198b9ef08b2109d Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:14:02 +0800 Subject: ACPI / IPMI: Use global IPMI operation region handler It is found on a real machine, in its ACPI namespace, the IPMI OperationRegions (in the ACPI000D - ACPI power meter) are not defined under the IPMI system interface device (the IPI0001 with KCS type returned from _IFT control method): Device (PMI0) { Name (_HID, "ACPI000D") // _HID: Hardware ID OperationRegion (SYSI, IPMI, 0x0600, 0x0100) Field (SYSI, BufferAcc, Lock, Preserve) { AccessAs (BufferAcc, 0x01), Offset (0x58), SCMD, 8, GCMD, 8 } OperationRegion (POWR, IPMI, 0x3000, 0x0100) Field (POWR, BufferAcc, Lock, Preserve) { AccessAs (BufferAcc, 0x01), Offset (0xB3), GPMM, 8 } } Device (PCI0) { Device (ISA) { Device (NIPM) { Name (_HID, EisaId ("IPI0001")) // _HID: Hardware ID Method (_IFT, 0, NotSerialized) // _IFT: IPMI Interface Type { Return (0x01) } } } } Current ACPI_IPMI code registers IPMI operation region handler on a per-device basis, so for the above namespace the IPMI operation region handler is registered only under the scope of \_SB.PCI0.ISA.NIPM. Thus when an IPMI operation region field of \PMI0 is accessed, there are errors reported on such platform: ACPI Error: No handlers for Region [IPMI] ACPI Error: Region IPMI(7) has no handler The solution is to install an IPMI operation region handler from root node so that every object that defines IPMI OperationRegion can get an address space handler registered. When an IPMI operation region field is accessed, the Network Function (0x06 for SYSI and 0x30 for POWR) and the Command (SCMD, GCMD, GPMM) are passed to the operation region handler, there is no system interface specified by the BIOS. The patch tries to select one system interface by monitoring the system interface notification. IPMI messages passed from the ACPI codes are sent to this selected global IPMI system interface. The ACPI_IPMI will always select the first registered IPMI interface with an ACPI handle (i.e., defined in the ACPI namespace). It's hard to determine the selection when there are multiple IPMI system interfaces defined in the ACPI namespace. According to the IPMI specification: A BMC device may make available multiple system interfaces, but only one management controller is allowed to be 'active' BMC that provides BMC functionality for the system (in case of a 'partitioned' system, there can be only one active BMC per partition). Only the system interface(s) for the active BMC allowed to respond to the 'Get Device Id' command. According to the ipmi_si desigin: The ipmi_si registeration notifications can only happen after a successful "Get Device ID" command. Thus it should be OK for non-partitioned systems to do such selection. However, we do not have much knowledge on 'partitioned' systems. References: https://bugzilla.kernel.org/show_bug.cgi?id=46741 Signed-off-by: Lv Zheng Reviewed-by: Huang Ying Signed-off-by: Rafael J. Wysocki diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index b285386..7ec4cd1 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -46,7 +46,6 @@ MODULE_AUTHOR("Zhao Yakui"); MODULE_DESCRIPTION("ACPI IPMI Opregion driver"); MODULE_LICENSE("GPL"); -#define IPMI_FLAGS_HANDLER_INSTALL 0 #define ACPI_IPMI_OK 0 #define ACPI_IPMI_TIMEOUT 0x10 @@ -66,7 +65,6 @@ struct acpi_ipmi_device { ipmi_user_t user_interface; int ipmi_ifnum; /* IPMI interface number */ long curr_msgid; - unsigned long flags; struct ipmi_smi_info smi_data; bool dead; struct kref kref; @@ -77,6 +75,14 @@ struct ipmi_driver_data { struct ipmi_smi_watcher bmc_events; struct ipmi_user_hndl ipmi_hndlrs; struct mutex ipmi_lock; + /* + * NOTE: IPMI System Interface Selection + * There is no system interface specified by the IPMI operation + * region access. We try to select one system interface with ACPI + * handle set. IPMI messages passed from the ACPI codes are sent + * to this selected global IPMI system interface. + */ + struct acpi_ipmi_device *selected_smi; }; struct acpi_ipmi_msg { @@ -110,8 +116,6 @@ struct acpi_ipmi_buffer { static void ipmi_register_bmc(int iface, struct device *dev); static void ipmi_bmc_gone(int iface); static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data); -static int ipmi_install_space_handler(struct acpi_ipmi_device *ipmi); -static void ipmi_remove_space_handler(struct acpi_ipmi_device *ipmi); static struct ipmi_driver_data driver_data = { .ipmi_devices = LIST_HEAD_INIT(driver_data.ipmi_devices), @@ -154,14 +158,12 @@ ipmi_dev_alloc(int iface, struct ipmi_smi_info *smi_data, acpi_handle handle) return NULL; } ipmi_device->user_interface = user; - ipmi_install_space_handler(ipmi_device); return ipmi_device; } static void ipmi_dev_release(struct acpi_ipmi_device *ipmi_device) { - ipmi_remove_space_handler(ipmi_device); ipmi_destroy_user(ipmi_device->user_interface); put_device(ipmi_device->smi_data.dev); kfree(ipmi_device); @@ -178,6 +180,8 @@ static void ipmi_dev_release_kref(struct kref *kref) static void __ipmi_dev_kill(struct acpi_ipmi_device *ipmi_device) { list_del(&ipmi_device->head); + if (driver_data.selected_smi == ipmi_device) + driver_data.selected_smi = NULL; /* * Always setting dead flag after deleting from the list or * list_for_each_entry() codes must get changed. @@ -185,17 +189,14 @@ static void __ipmi_dev_kill(struct acpi_ipmi_device *ipmi_device) ipmi_device->dead = true; } -static struct acpi_ipmi_device *acpi_ipmi_dev_get(int iface) +static struct acpi_ipmi_device *acpi_ipmi_dev_get(void) { - struct acpi_ipmi_device *temp, *ipmi_device = NULL; + struct acpi_ipmi_device *ipmi_device = NULL; mutex_lock(&driver_data.ipmi_lock); - list_for_each_entry(temp, &driver_data.ipmi_devices, head) { - if (temp->ipmi_ifnum == iface) { - ipmi_device = temp; - kref_get(&ipmi_device->kref); - break; - } + if (driver_data.selected_smi) { + ipmi_device = driver_data.selected_smi; + kref_get(&ipmi_device->kref); } mutex_unlock(&driver_data.ipmi_lock); @@ -416,6 +417,8 @@ static void ipmi_register_bmc(int iface, struct device *dev) goto err_lock; } + if (!driver_data.selected_smi) + driver_data.selected_smi = ipmi_device; list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices); mutex_unlock(&driver_data.ipmi_lock); put_device(smi_data.dev); @@ -443,6 +446,10 @@ static void ipmi_bmc_gone(int iface) break; } } + if (!driver_data.selected_smi) + driver_data.selected_smi = list_first_entry_or_null( + &driver_data.ipmi_devices, + struct acpi_ipmi_device, head); mutex_unlock(&driver_data.ipmi_lock); if (dev_found) { ipmi_flush_tx_msg(ipmi_device); @@ -471,7 +478,6 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, void *handler_context, void *region_context) { struct acpi_ipmi_msg *tx_msg; - int iface = (long)handler_context; struct acpi_ipmi_device *ipmi_device; int err; acpi_status status; @@ -485,7 +491,7 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, if ((function & ACPI_IO_MASK) == ACPI_READ) return AE_TYPE; - ipmi_device = acpi_ipmi_dev_get(iface); + ipmi_device = acpi_ipmi_dev_get(); if (!ipmi_device) return AE_NOT_EXIST; @@ -534,47 +540,26 @@ out_ref: return status; } -static void ipmi_remove_space_handler(struct acpi_ipmi_device *ipmi) -{ - if (!test_bit(IPMI_FLAGS_HANDLER_INSTALL, &ipmi->flags)) - return; - - acpi_remove_address_space_handler(ipmi->handle, - ACPI_ADR_SPACE_IPMI, &acpi_ipmi_space_handler); - - clear_bit(IPMI_FLAGS_HANDLER_INSTALL, &ipmi->flags); -} - -static int ipmi_install_space_handler(struct acpi_ipmi_device *ipmi) -{ - acpi_status status; - - if (test_bit(IPMI_FLAGS_HANDLER_INSTALL, &ipmi->flags)) - return 0; - - status = acpi_install_address_space_handler(ipmi->handle, - ACPI_ADR_SPACE_IPMI, &acpi_ipmi_space_handler, - NULL, (void *)((long)ipmi->ipmi_ifnum)); - if (ACPI_FAILURE(status)) { - struct pnp_dev *pnp_dev = ipmi->pnp_dev; - dev_warn(&pnp_dev->dev, "Can't register IPMI opregion space " - "handle\n"); - return -EINVAL; - } - set_bit(IPMI_FLAGS_HANDLER_INSTALL, &ipmi->flags); - return 0; -} - static int __init acpi_ipmi_init(void) { int result = 0; + acpi_status status; if (acpi_disabled) return result; mutex_init(&driver_data.ipmi_lock); + status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, + ACPI_ADR_SPACE_IPMI, &acpi_ipmi_space_handler, + NULL, NULL); + if (ACPI_FAILURE(status)) { + pr_warn("Can't register IPMI opregion space handle\n"); + return -EINVAL; + } result = ipmi_smi_watcher_register(&driver_data.bmc_events); + if (result) + pr_err("Can't register IPMI system interface watcher\n"); return result; } @@ -608,6 +593,8 @@ static void __exit acpi_ipmi_exit(void) mutex_lock(&driver_data.ipmi_lock); } mutex_unlock(&driver_data.ipmi_lock); + acpi_remove_address_space_handler(ACPI_ROOT_OBJECT, + ACPI_ADR_SPACE_IPMI, &acpi_ipmi_space_handler); } module_init(acpi_ipmi_init); -- cgit v0.10.2 From 7b9844772237e34968ffd4b086d7b5ed36b30856 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:14:11 +0800 Subject: ACPI / IPMI: Add reference counting for ACPI IPMI transfers This patch adds reference counting for ACPI IPMI transfers to tune the locking granularity of tx_msg_lock. This patch also makes the whole acpi_ipmi module's coding style consistent by using reference counting for all its objects (i.e., acpi_ipmi_device and acpi_ipmi_msg). The acpi_ipmi_msg handling is re-designed using referece counting. 1. tx_msg is always unlinked before complete(), so that it is safe to put complete() out side of tx_msg_lock. 2. tx_msg reference counters are incremented before calling ipmi_request_settime() and tx_msg_lock protection is added to ipmi_cancel_tx_msg() so that a complete() can be safely called in parellel with tx_msg unlinking in failure cases. 3. tx_msg holds a reference to acpi_ipmi_device so that it can be flushed and freed in the contexts other than acpi_ipmi_space_handler(). The lockdep_chains shows all acpi_ipmi locks are leaf locks after the tuning: 1. ipmi_lock is always leaf: irq_context: 0 [ffffffff81a943f8] smi_watchers_mutex [ffffffffa06eca60] driver_data.ipmi_lock irq_context: 0 [ffffffff82767b40] &buffer->mutex [ffffffffa00a6678] s_active#103 [ffffffffa06eca60] driver_data.ipmi_lock 2. without this patch applied, lock used by complete() is held after holding tx_msg_lock: irq_context: 0 [ffffffff82767b40] &buffer->mutex [ffffffffa00a6678] s_active#103 [ffffffffa06ecce8] &(&ipmi_device->tx_msg_lock)->rlock irq_context: 1 [ffffffffa06ecce8] &(&ipmi_device->tx_msg_lock)->rlock irq_context: 1 [ffffffffa06ecce8] &(&ipmi_device->tx_msg_lock)->rlock [ffffffffa06eccf0] &x->wait#25 irq_context: 1 [ffffffffa06ecce8] &(&ipmi_device->tx_msg_lock)->rlock [ffffffffa06eccf0] &x->wait#25 [ffffffff81e36620] &p->pi_lock irq_context: 1 [ffffffffa06ecce8] &(&ipmi_device->tx_msg_lock)->rlock [ffffffffa06eccf0] &x->wait#25 [ffffffff81e36620] &p->pi_lock [ffffffff81e5d0a8] &rq->lock 3. with this patch applied, tx_msg_lock is always leaf: irq_context: 0 [ffffffff82767b40] &buffer->mutex [ffffffffa00a66d8] s_active#107 [ffffffffa07ecdc8] &(&ipmi_device->tx_msg_lock)->rlock irq_context: 1 [ffffffffa07ecdc8] &(&ipmi_device->tx_msg_lock)->rlock Signed-off-by: Lv Zheng Reviewed-by: Huang Ying Signed-off-by: Rafael J. Wysocki diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index 7ec4cd1..b9da5ef 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -104,6 +104,7 @@ struct acpi_ipmi_msg { u8 data[ACPI_IPMI_MAX_MSG_LENGTH]; u8 rx_len; struct acpi_ipmi_device *device; + struct kref kref; }; /* IPMI request/response buffer per ACPI 4.0, sec 5.5.2.4.3.2 */ @@ -208,16 +209,20 @@ static void acpi_ipmi_dev_put(struct acpi_ipmi_device *ipmi_device) kref_put(&ipmi_device->kref, ipmi_dev_release_kref); } -static struct acpi_ipmi_msg *acpi_alloc_ipmi_msg(struct acpi_ipmi_device *ipmi) +static struct acpi_ipmi_msg *ipmi_msg_alloc(void) { + struct acpi_ipmi_device *ipmi; struct acpi_ipmi_msg *ipmi_msg; - struct pnp_dev *pnp_dev = ipmi->pnp_dev; + ipmi = acpi_ipmi_dev_get(); + if (!ipmi) + return NULL; ipmi_msg = kzalloc(sizeof(struct acpi_ipmi_msg), GFP_KERNEL); - if (!ipmi_msg) { - dev_warn(&pnp_dev->dev, "Can't allocate memory for ipmi_msg\n"); + if (!ipmi_msg) { + acpi_ipmi_dev_put(ipmi); return NULL; } + kref_init(&ipmi_msg->kref); init_completion(&ipmi_msg->tx_complete); INIT_LIST_HEAD(&ipmi_msg->head); ipmi_msg->device = ipmi; @@ -225,6 +230,32 @@ static struct acpi_ipmi_msg *acpi_alloc_ipmi_msg(struct acpi_ipmi_device *ipmi) return ipmi_msg; } +static void ipmi_msg_release(struct acpi_ipmi_msg *tx_msg) +{ + acpi_ipmi_dev_put(tx_msg->device); + kfree(tx_msg); +} + +static void ipmi_msg_release_kref(struct kref *kref) +{ + struct acpi_ipmi_msg *tx_msg = + container_of(kref, struct acpi_ipmi_msg, kref); + + ipmi_msg_release(tx_msg); +} + +static struct acpi_ipmi_msg *acpi_ipmi_msg_get(struct acpi_ipmi_msg *tx_msg) +{ + kref_get(&tx_msg->kref); + + return tx_msg; +} + +static void acpi_ipmi_msg_put(struct acpi_ipmi_msg *tx_msg) +{ + kref_put(&tx_msg->kref, ipmi_msg_release_kref); +} + #define IPMI_OP_RGN_NETFN(offset) ((offset >> 8) & 0xff) #define IPMI_OP_RGN_CMD(offset) (offset & 0xff) static int acpi_format_ipmi_request(struct acpi_ipmi_msg *tx_msg, @@ -305,7 +336,7 @@ static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg, static void ipmi_flush_tx_msg(struct acpi_ipmi_device *ipmi) { - struct acpi_ipmi_msg *tx_msg, *temp; + struct acpi_ipmi_msg *tx_msg; unsigned long flags; /* @@ -317,18 +348,47 @@ static void ipmi_flush_tx_msg(struct acpi_ipmi_device *ipmi) * ipmi_recv_msg(s) are freed after invoking ipmi_destroy_user(). */ spin_lock_irqsave(&ipmi->tx_msg_lock, flags); - list_for_each_entry_safe(tx_msg, temp, &ipmi->tx_msg_list, head) { + while (!list_empty(&ipmi->tx_msg_list)) { + tx_msg = list_first_entry(&ipmi->tx_msg_list, + struct acpi_ipmi_msg, + head); + list_del(&tx_msg->head); + spin_unlock_irqrestore(&ipmi->tx_msg_lock, flags); + /* wake up the sleep thread on the Tx msg */ complete(&tx_msg->tx_complete); + acpi_ipmi_msg_put(tx_msg); + spin_lock_irqsave(&ipmi->tx_msg_lock, flags); + } + spin_unlock_irqrestore(&ipmi->tx_msg_lock, flags); +} + +static void ipmi_cancel_tx_msg(struct acpi_ipmi_device *ipmi, + struct acpi_ipmi_msg *msg) +{ + struct acpi_ipmi_msg *tx_msg, *temp; + bool msg_found = false; + unsigned long flags; + + spin_lock_irqsave(&ipmi->tx_msg_lock, flags); + list_for_each_entry_safe(tx_msg, temp, &ipmi->tx_msg_list, head) { + if (msg == tx_msg) { + msg_found = true; + list_del(&tx_msg->head); + break; + } } spin_unlock_irqrestore(&ipmi->tx_msg_lock, flags); + + if (msg_found) + acpi_ipmi_msg_put(tx_msg); } static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) { struct acpi_ipmi_device *ipmi_device = user_msg_data; - int msg_found = 0; - struct acpi_ipmi_msg *tx_msg; + bool msg_found = false; + struct acpi_ipmi_msg *tx_msg, *temp; struct pnp_dev *pnp_dev = ipmi_device->pnp_dev; unsigned long flags; @@ -339,17 +399,19 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) goto out_msg; } spin_lock_irqsave(&ipmi_device->tx_msg_lock, flags); - list_for_each_entry(tx_msg, &ipmi_device->tx_msg_list, head) { + list_for_each_entry_safe(tx_msg, temp, &ipmi_device->tx_msg_list, head) { if (msg->msgid == tx_msg->tx_msgid) { - msg_found = 1; + msg_found = true; + list_del(&tx_msg->head); break; } } + spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); if (!msg_found) { dev_warn(&pnp_dev->dev, "Unexpected response (msg id %ld) is " "returned.\n", msg->msgid); - goto out_lock; + goto out_msg; } /* copy the response data to Rx_data buffer */ @@ -375,8 +437,7 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) tx_msg->msg_done = ACPI_IPMI_OK; out_comp: complete(&tx_msg->tx_complete); -out_lock: - spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); + acpi_ipmi_msg_put(tx_msg); out_msg: ipmi_free_recv_msg(msg); }; @@ -491,26 +552,23 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, if ((function & ACPI_IO_MASK) == ACPI_READ) return AE_TYPE; - ipmi_device = acpi_ipmi_dev_get(); - if (!ipmi_device) + tx_msg = ipmi_msg_alloc(); + if (!tx_msg) return AE_NOT_EXIST; - tx_msg = acpi_alloc_ipmi_msg(ipmi_device); - if (!tx_msg) { - status = AE_NO_MEMORY; - goto out_ref; - } + ipmi_device = tx_msg->device; if (acpi_format_ipmi_request(tx_msg, address, value) != 0) { - status = AE_TYPE; - goto out_msg; + ipmi_msg_release(tx_msg); + return AE_TYPE; } + acpi_ipmi_msg_get(tx_msg); mutex_lock(&driver_data.ipmi_lock); /* Do not add a tx_msg that can not be flushed. */ if (ipmi_device->dead) { - status = AE_NOT_EXIST; mutex_unlock(&driver_data.ipmi_lock); - goto out_msg; + ipmi_msg_release(tx_msg); + return AE_NOT_EXIST; } spin_lock_irqsave(&ipmi_device->tx_msg_lock, flags); list_add_tail(&tx_msg->head, &ipmi_device->tx_msg_list); @@ -523,20 +581,15 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, NULL, 0, 0, IPMI_TIMEOUT); if (err) { status = AE_ERROR; - goto out_list; + goto out_msg; } wait_for_completion(&tx_msg->tx_complete); acpi_format_ipmi_response(tx_msg, value); status = AE_OK; -out_list: - spin_lock_irqsave(&ipmi_device->tx_msg_lock, flags); - list_del(&tx_msg->head); - spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); out_msg: - kfree(tx_msg); -out_ref: - acpi_ipmi_dev_put(ipmi_device); + ipmi_cancel_tx_msg(ipmi_device, tx_msg); + acpi_ipmi_msg_put(tx_msg); return status; } -- cgit v0.10.2 From 2fb037b89a3125627f02c40af3e3d38c3228184b Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:14:21 +0800 Subject: ACPI / IPMI: Cleanup several acpi_ipmi_device members This (trivial) patch: 1. Deletes a member of the acpi_ipmi_device, smi_data, which is not actually used. 2. Updates a member of the acpi_ipmi_device, pnp_dev, which is only used by dev_warn() invocations, so changes it to a struct device. Signed-off-by: Lv Zheng Reviewed-by: Huang Ying Signed-off-by: Rafael J. Wysocki diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index b9da5ef..90d57c8 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -61,11 +61,10 @@ struct acpi_ipmi_device { struct list_head tx_msg_list; spinlock_t tx_msg_lock; acpi_handle handle; - struct pnp_dev *pnp_dev; + struct device *dev; ipmi_user_t user_interface; int ipmi_ifnum; /* IPMI interface number */ long curr_msgid; - struct ipmi_smi_info smi_data; bool dead; struct kref kref; }; @@ -131,7 +130,7 @@ static struct ipmi_driver_data driver_data = { }; static struct acpi_ipmi_device * -ipmi_dev_alloc(int iface, struct ipmi_smi_info *smi_data, acpi_handle handle) +ipmi_dev_alloc(int iface, struct device *dev, acpi_handle handle) { struct acpi_ipmi_device *ipmi_device; int err; @@ -147,14 +146,13 @@ ipmi_dev_alloc(int iface, struct ipmi_smi_info *smi_data, acpi_handle handle) spin_lock_init(&ipmi_device->tx_msg_lock); ipmi_device->handle = handle; - ipmi_device->pnp_dev = to_pnp_dev(get_device(smi_data->dev)); - memcpy(&ipmi_device->smi_data, smi_data, sizeof(struct ipmi_smi_info)); + ipmi_device->dev = get_device(dev); ipmi_device->ipmi_ifnum = iface; err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs, ipmi_device, &user); if (err) { - put_device(smi_data->dev); + put_device(dev); kfree(ipmi_device); return NULL; } @@ -166,7 +164,7 @@ ipmi_dev_alloc(int iface, struct ipmi_smi_info *smi_data, acpi_handle handle) static void ipmi_dev_release(struct acpi_ipmi_device *ipmi_device) { ipmi_destroy_user(ipmi_device->user_interface); - put_device(ipmi_device->smi_data.dev); + put_device(ipmi_device->dev); kfree(ipmi_device); } @@ -282,7 +280,7 @@ static int acpi_format_ipmi_request(struct acpi_ipmi_msg *tx_msg, buffer = (struct acpi_ipmi_buffer *)value; /* copy the tx message data */ if (buffer->length > ACPI_IPMI_MAX_MSG_LENGTH) { - dev_WARN_ONCE(&tx_msg->device->pnp_dev->dev, true, + dev_WARN_ONCE(tx_msg->device->dev, true, "Unexpected request (msg len %d).\n", buffer->length); return -EINVAL; @@ -389,11 +387,11 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) struct acpi_ipmi_device *ipmi_device = user_msg_data; bool msg_found = false; struct acpi_ipmi_msg *tx_msg, *temp; - struct pnp_dev *pnp_dev = ipmi_device->pnp_dev; + struct device *dev = ipmi_device->dev; unsigned long flags; if (msg->user != ipmi_device->user_interface) { - dev_warn(&pnp_dev->dev, "Unexpected response is returned. " + dev_warn(dev, "Unexpected response is returned. " "returned user %p, expected user %p\n", msg->user, ipmi_device->user_interface); goto out_msg; @@ -409,14 +407,14 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); if (!msg_found) { - dev_warn(&pnp_dev->dev, "Unexpected response (msg id %ld) is " + dev_warn(dev, "Unexpected response (msg id %ld) is " "returned.\n", msg->msgid); goto out_msg; } /* copy the response data to Rx_data buffer */ if (msg->msg.data_len > ACPI_IPMI_MAX_MSG_LENGTH) { - dev_WARN_ONCE(&pnp_dev->dev, true, + dev_WARN_ONCE(dev, true, "Unexpected response (msg len %d).\n", msg->msg.data_len); goto out_comp; @@ -426,7 +424,7 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) if (msg->recv_type == IPMI_RESPONSE_RECV_TYPE && msg->msg.data_len == 1) { if (msg->msg.data[0] == IPMI_TIMEOUT_COMPLETION_CODE) { - dev_WARN_ONCE(&pnp_dev->dev, true, + dev_WARN_ONCE(dev, true, "Unexpected response (timeout).\n"); tx_msg->msg_done = ACPI_IPMI_TIMEOUT; } @@ -445,7 +443,6 @@ out_msg: static void ipmi_register_bmc(int iface, struct device *dev) { struct acpi_ipmi_device *ipmi_device, *temp; - struct pnp_dev *pnp_dev; int err; struct ipmi_smi_info smi_data; acpi_handle handle; @@ -460,11 +457,10 @@ static void ipmi_register_bmc(int iface, struct device *dev) handle = smi_data.addr_info.acpi_info.acpi_handle; if (!handle) goto err_ref; - pnp_dev = to_pnp_dev(smi_data.dev); - ipmi_device = ipmi_dev_alloc(iface, &smi_data, handle); + ipmi_device = ipmi_dev_alloc(iface, smi_data.dev, handle); if (!ipmi_device) { - dev_warn(&pnp_dev->dev, "Can't create IPMI user interface\n"); + dev_warn(smi_data.dev, "Can't create IPMI user interface\n"); goto err_ref; } -- cgit v0.10.2 From a194aa43272d22f035e6f862db2d714658a02b36 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:14:31 +0800 Subject: ACPI / IPMI: Cleanup some initialization codes This (trivial) patch. 1. Changes dynamic mutex initialization to static initialization. 2. Removes one acpi_ipmi_init() variable initialization as it is not needed. Signed-off-by: Lv Zheng Reviewed-by: Huang Ying Signed-off-by: Rafael J. Wysocki diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index 90d57c8..f7b6598 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -127,6 +127,7 @@ static struct ipmi_driver_data driver_data = { .ipmi_hndlrs = { .ipmi_recv_hndl = ipmi_msg_handler, }, + .ipmi_lock = __MUTEX_INITIALIZER(driver_data.ipmi_lock) }; static struct acpi_ipmi_device * @@ -591,13 +592,11 @@ out_msg: static int __init acpi_ipmi_init(void) { - int result = 0; + int result; acpi_status status; if (acpi_disabled) - return result; - - mutex_init(&driver_data.ipmi_lock); + return 0; status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, ACPI_ADR_SPACE_IPMI, &acpi_ipmi_space_handler, -- cgit v0.10.2 From 935a9e3f9d57be122013113318e30e68530b474d Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:14:41 +0800 Subject: ACPI / IPMI: Cleanup some inclusion codes This (trivial) patch: 1. Deletes several useless header inclusions. 2. Kernel codes should always include instead of or where many conditional declarations are handled. Signed-off-by: Lv Zheng Reviewed-by: Huang Ying Signed-off-by: Rafael J. Wysocki diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index f7b6598..9d187fd 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -24,22 +24,9 @@ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ */ -#include #include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include +#include #include -#include -#include #include MODULE_AUTHOR("Zhao Yakui"); -- cgit v0.10.2 From 4b88e330914941c86198ca0b48f4ce16f112bbf0 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:14:51 +0800 Subject: ACPI / IPMI: Cleanup some Kconfig codes This (trivial) patch: 1. Deletes duplicate Kconfig dependency as there is "if IPMI_HANDLER" around "IPMI_SI". Signed-off-by: Lv Zheng Reviewed-by: Huang Ying Signed-off-by: Rafael J. Wysocki diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig index 22327e6..d130e2c 100644 --- a/drivers/acpi/Kconfig +++ b/drivers/acpi/Kconfig @@ -175,9 +175,10 @@ config ACPI_PROCESSOR To compile this driver as a module, choose M here: the module will be called processor. + config ACPI_IPMI tristate "IPMI" - depends on IPMI_SI && IPMI_HANDLER + depends on IPMI_SI default n help This driver enables the ACPI to access the BMC controller. And it -- cgit v0.10.2 From 50065300314e95c945665a17febc0c4e02941b06 Mon Sep 17 00:00:00 2001 From: Lv Zheng Date: Fri, 13 Sep 2013 13:15:00 +0800 Subject: ACPI / IPMI: Cleanup coding styles This patch only introduces indentation cleanups. No functional changes. Signed-off-by: Lv Zheng Signed-off-by: Rafael J. Wysocki diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index 9d187fd..ac0f52f 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -33,7 +33,6 @@ MODULE_AUTHOR("Zhao Yakui"); MODULE_DESCRIPTION("ACPI IPMI Opregion driver"); MODULE_LICENSE("GPL"); - #define ACPI_IPMI_OK 0 #define ACPI_IPMI_TIMEOUT 0x10 #define ACPI_IPMI_UNKNOWN 0x07 @@ -44,12 +43,14 @@ MODULE_LICENSE("GPL"); struct acpi_ipmi_device { /* the device list attached to driver_data.ipmi_devices */ struct list_head head; + /* the IPMI request message list */ struct list_head tx_msg_list; - spinlock_t tx_msg_lock; + + spinlock_t tx_msg_lock; acpi_handle handle; struct device *dev; - ipmi_user_t user_interface; + ipmi_user_t user_interface; int ipmi_ifnum; /* IPMI interface number */ long curr_msgid; bool dead; @@ -57,10 +58,11 @@ struct acpi_ipmi_device { }; struct ipmi_driver_data { - struct list_head ipmi_devices; - struct ipmi_smi_watcher bmc_events; - struct ipmi_user_hndl ipmi_hndlrs; - struct mutex ipmi_lock; + struct list_head ipmi_devices; + struct ipmi_smi_watcher bmc_events; + struct ipmi_user_hndl ipmi_hndlrs; + struct mutex ipmi_lock; + /* * NOTE: IPMI System Interface Selection * There is no system interface specified by the IPMI operation @@ -73,6 +75,7 @@ struct ipmi_driver_data { struct acpi_ipmi_msg { struct list_head head; + /* * General speaking the addr type should be SI_ADDR_TYPE. And * the addr channel should be BMC. @@ -82,15 +85,19 @@ struct acpi_ipmi_msg { */ struct ipmi_addr addr; long tx_msgid; + /* it is used to track whether the IPMI message is finished */ struct completion tx_complete; + struct kernel_ipmi_msg tx_message; - int msg_done; + int msg_done; + /* tx/rx data . And copy it from/to ACPI object buffer */ - u8 data[ACPI_IPMI_MAX_MSG_LENGTH]; - u8 rx_len; + u8 data[ACPI_IPMI_MAX_MSG_LENGTH]; + u8 rx_len; + struct acpi_ipmi_device *device; - struct kref kref; + struct kref kref; }; /* IPMI request/response buffer per ACPI 4.0, sec 5.5.2.4.3.2 */ @@ -132,7 +139,6 @@ ipmi_dev_alloc(int iface, struct device *dev, acpi_handle handle) INIT_LIST_HEAD(&ipmi_device->head); INIT_LIST_HEAD(&ipmi_device->tx_msg_list); spin_lock_init(&ipmi_device->tx_msg_lock); - ipmi_device->handle = handle; ipmi_device->dev = get_device(dev); ipmi_device->ipmi_ifnum = iface; @@ -169,6 +175,7 @@ static void __ipmi_dev_kill(struct acpi_ipmi_device *ipmi_device) list_del(&ipmi_device->head); if (driver_data.selected_smi == ipmi_device) driver_data.selected_smi = NULL; + /* * Always setting dead flag after deleting from the list or * list_for_each_entry() codes must get changed. @@ -203,16 +210,19 @@ static struct acpi_ipmi_msg *ipmi_msg_alloc(void) ipmi = acpi_ipmi_dev_get(); if (!ipmi) return NULL; + ipmi_msg = kzalloc(sizeof(struct acpi_ipmi_msg), GFP_KERNEL); if (!ipmi_msg) { acpi_ipmi_dev_put(ipmi); return NULL; } + kref_init(&ipmi_msg->kref); init_completion(&ipmi_msg->tx_complete); INIT_LIST_HEAD(&ipmi_msg->head); ipmi_msg->device = ipmi; ipmi_msg->msg_done = ACPI_IPMI_UNKNOWN; + return ipmi_msg; } @@ -242,11 +252,11 @@ static void acpi_ipmi_msg_put(struct acpi_ipmi_msg *tx_msg) kref_put(&tx_msg->kref, ipmi_msg_release_kref); } -#define IPMI_OP_RGN_NETFN(offset) ((offset >> 8) & 0xff) -#define IPMI_OP_RGN_CMD(offset) (offset & 0xff) +#define IPMI_OP_RGN_NETFN(offset) ((offset >> 8) & 0xff) +#define IPMI_OP_RGN_CMD(offset) (offset & 0xff) static int acpi_format_ipmi_request(struct acpi_ipmi_msg *tx_msg, - acpi_physical_address address, - acpi_integer *value) + acpi_physical_address address, + acpi_integer *value) { struct kernel_ipmi_msg *msg; struct acpi_ipmi_buffer *buffer; @@ -254,6 +264,7 @@ static int acpi_format_ipmi_request(struct acpi_ipmi_msg *tx_msg, unsigned long flags; msg = &tx_msg->tx_message; + /* * IPMI network function and command are encoded in the address * within the IPMI OpRegion; see ACPI 4.0, sec 5.5.2.4.3. @@ -261,11 +272,13 @@ static int acpi_format_ipmi_request(struct acpi_ipmi_msg *tx_msg, msg->netfn = IPMI_OP_RGN_NETFN(address); msg->cmd = IPMI_OP_RGN_CMD(address); msg->data = tx_msg->data; + /* * value is the parameter passed by the IPMI opregion space handler. * It points to the IPMI request message buffer */ buffer = (struct acpi_ipmi_buffer *)value; + /* copy the tx message data */ if (buffer->length > ACPI_IPMI_MAX_MSG_LENGTH) { dev_WARN_ONCE(tx_msg->device->dev, true, @@ -275,6 +288,7 @@ static int acpi_format_ipmi_request(struct acpi_ipmi_msg *tx_msg, } msg->data_len = buffer->length; memcpy(tx_msg->data, buffer->data, msg->data_len); + /* * now the default type is SYSTEM_INTERFACE and channel type is BMC. * If the netfn is APP_REQUEST and the cmd is SEND_MESSAGE, @@ -288,15 +302,17 @@ static int acpi_format_ipmi_request(struct acpi_ipmi_msg *tx_msg, /* Get the msgid */ device = tx_msg->device; + spin_lock_irqsave(&device->tx_msg_lock, flags); device->curr_msgid++; tx_msg->tx_msgid = device->curr_msgid; spin_unlock_irqrestore(&device->tx_msg_lock, flags); + return 0; } static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg, - acpi_integer *value) + acpi_integer *value) { struct acpi_ipmi_buffer *buffer; @@ -305,6 +321,7 @@ static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg, * IPMI message returned by IPMI command. */ buffer = (struct acpi_ipmi_buffer *)value; + /* * If the flag of msg_done is not set, it means that the IPMI command is * not executed correctly. @@ -312,6 +329,7 @@ static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg, buffer->status = msg->msg_done; if (msg->msg_done != ACPI_IPMI_OK) return; + /* * If the IPMI response message is obtained correctly, the status code * will be ACPI_IPMI_OK @@ -379,11 +397,12 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) unsigned long flags; if (msg->user != ipmi_device->user_interface) { - dev_warn(dev, "Unexpected response is returned. " - "returned user %p, expected user %p\n", - msg->user, ipmi_device->user_interface); + dev_warn(dev, + "Unexpected response is returned. returned user %p, expected user %p\n", + msg->user, ipmi_device->user_interface); goto out_msg; } + spin_lock_irqsave(&ipmi_device->tx_msg_lock, flags); list_for_each_entry_safe(tx_msg, temp, &ipmi_device->tx_msg_list, head) { if (msg->msgid == tx_msg->tx_msgid) { @@ -395,8 +414,9 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); if (!msg_found) { - dev_warn(dev, "Unexpected response (msg id %ld) is " - "returned.\n", msg->msgid); + dev_warn(dev, + "Unexpected response (msg id %ld) is returned.\n", + msg->msgid); goto out_msg; } @@ -407,6 +427,7 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) msg->msg.data_len); goto out_comp; } + /* response msg is an error msg */ msg->recv_type = IPMI_RESPONSE_RECV_TYPE; if (msg->recv_type == IPMI_RESPONSE_RECV_TYPE && @@ -418,15 +439,17 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data) } goto out_comp; } + tx_msg->rx_len = msg->msg.data_len; memcpy(tx_msg->data, msg->msg.data, tx_msg->rx_len); tx_msg->msg_done = ACPI_IPMI_OK; + out_comp: complete(&tx_msg->tx_complete); acpi_ipmi_msg_put(tx_msg); out_msg: ipmi_free_recv_msg(msg); -}; +} static void ipmi_register_bmc(int iface, struct device *dev) { @@ -436,7 +459,6 @@ static void ipmi_register_bmc(int iface, struct device *dev) acpi_handle handle; err = ipmi_get_smi_info(iface, &smi_data); - if (err) return; @@ -461,11 +483,11 @@ static void ipmi_register_bmc(int iface, struct device *dev) if (temp->handle == handle) goto err_lock; } - if (!driver_data.selected_smi) driver_data.selected_smi = ipmi_device; list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices); mutex_unlock(&driver_data.ipmi_lock); + put_device(smi_data.dev); return; @@ -484,7 +506,7 @@ static void ipmi_bmc_gone(int iface) mutex_lock(&driver_data.ipmi_lock); list_for_each_entry_safe(ipmi_device, temp, - &driver_data.ipmi_devices, head) { + &driver_data.ipmi_devices, head) { if (ipmi_device->ipmi_ifnum != iface) { dev_found = true; __ipmi_dev_kill(ipmi_device); @@ -496,14 +518,13 @@ static void ipmi_bmc_gone(int iface) &driver_data.ipmi_devices, struct acpi_ipmi_device, head); mutex_unlock(&driver_data.ipmi_lock); + if (dev_found) { ipmi_flush_tx_msg(ipmi_device); acpi_ipmi_dev_put(ipmi_device); } } -/* -------------------------------------------------------------------------- - * Address Space Management - * -------------------------------------------------------------------------- */ + /* * This is the IPMI opregion space handler. * @function: indicates the read/write. In fact as the IPMI message is driven @@ -516,17 +537,17 @@ static void ipmi_bmc_gone(int iface) * the response IPMI message returned by IPMI command. * @handler_context: IPMI device context. */ - static acpi_status acpi_ipmi_space_handler(u32 function, acpi_physical_address address, - u32 bits, acpi_integer *value, - void *handler_context, void *region_context) + u32 bits, acpi_integer *value, + void *handler_context, void *region_context) { struct acpi_ipmi_msg *tx_msg; struct acpi_ipmi_device *ipmi_device; int err; acpi_status status; unsigned long flags; + /* * IPMI opregion message. * IPMI message is firstly written to the BMC and system software @@ -539,13 +560,13 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, tx_msg = ipmi_msg_alloc(); if (!tx_msg) return AE_NOT_EXIST; - ipmi_device = tx_msg->device; if (acpi_format_ipmi_request(tx_msg, address, value) != 0) { ipmi_msg_release(tx_msg); return AE_TYPE; } + acpi_ipmi_msg_get(tx_msg); mutex_lock(&driver_data.ipmi_lock); /* Do not add a tx_msg that can not be flushed. */ @@ -558,16 +579,18 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, list_add_tail(&tx_msg->head, &ipmi_device->tx_msg_list); spin_unlock_irqrestore(&ipmi_device->tx_msg_lock, flags); mutex_unlock(&driver_data.ipmi_lock); + err = ipmi_request_settime(ipmi_device->user_interface, - &tx_msg->addr, - tx_msg->tx_msgid, - &tx_msg->tx_message, - NULL, 0, 0, IPMI_TIMEOUT); + &tx_msg->addr, + tx_msg->tx_msgid, + &tx_msg->tx_message, + NULL, 0, 0, IPMI_TIMEOUT); if (err) { status = AE_ERROR; goto out_msg; } wait_for_completion(&tx_msg->tx_complete); + acpi_format_ipmi_response(tx_msg, value); status = AE_OK; @@ -586,8 +609,9 @@ static int __init acpi_ipmi_init(void) return 0; status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, - ACPI_ADR_SPACE_IPMI, &acpi_ipmi_space_handler, - NULL, NULL); + ACPI_ADR_SPACE_IPMI, + &acpi_ipmi_space_handler, + NULL, NULL); if (ACPI_FAILURE(status)) { pr_warn("Can't register IPMI opregion space handle\n"); return -EINVAL; @@ -629,7 +653,8 @@ static void __exit acpi_ipmi_exit(void) } mutex_unlock(&driver_data.ipmi_lock); acpi_remove_address_space_handler(ACPI_ROOT_OBJECT, - ACPI_ADR_SPACE_IPMI, &acpi_ipmi_space_handler); + ACPI_ADR_SPACE_IPMI, + &acpi_ipmi_space_handler); } module_init(acpi_ipmi_init); -- cgit v0.10.2