[Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[yakui.zhao]

From: Zhao Yakui <yakui.zhao@intel.com>

Add the IPMI opregion driver so that the AML code can communicate with BMC
throught IPMI message.
     It will create IPMI user interface for every IPMI device detected
in ACPI namespace and install the corresponding IPMI opregion space handler.

The following describes how to process the IPMI request in IPMI space handler:
    1. format the IPMI message based on the request in AML code.
    IPMI system address. Now the address type is SYSTEM_INTERFACE_ADDR_TYPE
    IPMI net function & command
    IPMI message payload
    2. send the IPMI message by using the function of ipmi_request_settime
    3. wait for the completion of IPMI message. It can be done in different
routes: One is in handled in IPMI user recv callback function. Another is
handled in timeout function.
    4. format the IPMI response and return it to ACPI AML code.

Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
---
 drivers/acpi/Kconfig  |   12 +
 drivers/acpi/Makefile |    1 +
 drivers/acpi/ipmi.c   |  583 +++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 596 insertions(+), 0 deletions(-)
 create mode 100644 drivers/acpi/ipmi.c

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 93d2c79..23cd08a 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -204,6 +204,18 @@ 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 EXPERIMENTAL
+	default n
+	select IPMI_HANDLER
+	select IPMI_SI
+	help
+	  This driver enables the ACPI to access the BMC controller. And it
+	  uses the IPMI request/response message to communicate with BMC
+	  controller, which can be found on on the server.
+
+	  To compile this driver as a module, choose M here:
 
 config ACPI_HOTPLUG_CPU
 	bool
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 7702118..61077b7 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -57,6 +57,7 @@ obj-$(CONFIG_ACPI_BATTERY)	+= battery.o
 obj-$(CONFIG_ACPI_SBS)		+= sbshc.o
 obj-$(CONFIG_ACPI_SBS)		+= sbs.o
 obj-$(CONFIG_ACPI_POWER_METER)	+= power_meter.o
+obj-$(CONFIG_ACPI_IPMI)		+= ipmi.o
 
 # processor has its own "processor." module_param namespace
 processor-y			:= processor_core.o processor_throttling.o
diff --git a/drivers/acpi/ipmi.c b/drivers/acpi/ipmi.c
new file mode 100644
index 0000000..5c74936
--- /dev/null
+++ b/drivers/acpi/ipmi.c
@@ -0,0 +1,583 @@
+/*
+ *  ipmi.c - ACPI IPMI opregion
+ *
+ *  Copyright (C) 2009 Intel Corporation
+ *  Copyright (C) 2009 Zhao Yakui <yakui.zhao@intel.com>
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or (at
+ *  your option) any later version.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License along
+ *  with this program; if not, write to the Free Software Foundation, Inc.,
+ *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/delay.h>
+#include <linux/proc_fs.h>
+#include <linux/seq_file.h>
+#include <linux/interrupt.h>
+#include <linux/list.h>
+#include <linux/spinlock.h>
+#include <linux/io.h>
+#include <acpi/acpi_bus.h>
+#include <acpi/acpi_drivers.h>
+#include <linux/ipmi.h>
+
+MODULE_AUTHOR("Zhao Yakui");
+MODULE_DESCRIPTION("ACPI IPMI Opregion driver");
+MODULE_LICENSE("GPL");
+
+#define ACPI_IPMI_CLASS			"IPMI"
+#define ACPI_IPMI_DEVICE_NAME		"IPMI_dev"
+#define IPMI_FLAGS_HANDLER_INSTALL	0
+
+#define ACPI_IPMI_OK			0
+#define ACPI_IPMI_TIMEOUT		0x10
+#define ACPI_IPMI_UNKNOWN		0x07
+/* the IPMI timeout is 30s */
+#define IPMI_TIMEOUT			(30 * HZ)
+
+
+struct acpi_ipmi_device {
+	acpi_handle handle;
+	struct acpi_device *device;
+	int if_type;
+	/* the device list attached to driver_data.ipmi_devices */
+	struct list_head head;
+	ipmi_user_t 	user_interface;
+	struct mutex	mutex_lock;
+	/* the IPMI request message list */
+	struct list_head tx_msg_list;
+	long curr_msgid;
+	/* IPMI flags */
+	unsigned long flags;
+};
+
+struct ipmi_driver_data {
+	int device_count;
+	struct list_head	ipmi_devices;
+	struct ipmi_smi_watcher	bmc_events;
+	struct ipmi_user_hndl	ipmi_hndlrs;
+};
+
+struct acpi_ipmi_msg {
+	/* message list */
+	struct list_head head;
+	/*
+	 * General speaking the addr type should be SI_ADDR_TYPE. And
+	 * the addr channel should be BMC.
+	 * In fact it can also be IPMB type. But we will have to
+	 * parse it from the Netfn command buffer. It is so complex
+	 * that it is skipped.
+	 */
+	struct ipmi_addr addr;
+	/* tx message id */
+	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;
+	/* tx data . And copy it from ACPI object buffer */
+	u8	tx_data[64];
+	int	tx_len;
+	/* get the response data */
+	u8	rx_data[64];
+	/* the response length. The netfn & cmd is excluded. */
+	int	rx_len;
+	struct acpi_ipmi_device *device;
+};
+
+/*
+ * IPMI request/response buffer.
+ * The length is 66 bytes.
+ */
+struct acpi_ipmi_buffer {
+	/* status code of a given IPMI command */
+	u8 status_code;
+	/* the length of the payload */
+	u8 length;
+	/*
+	 * the payload. Before the operation is carried out, it represents the
+	 * request message payload. After the opration is carried out, it
+	 * stores the response message returned by IPMI command.
+	 */
+	u8 data[64];
+};
+
+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 struct ipmi_driver_data driver_data = {
+	.ipmi_devices = LIST_HEAD_INIT(driver_data.ipmi_devices),
+	.bmc_events = {
+		.owner = THIS_MODULE,
+		.new_smi = ipmi_register_bmc,
+		.smi_gone = ipmi_bmc_gone,
+	},
+	.ipmi_hndlrs = {
+		.ipmi_recv_hndl = ipmi_msg_handler,
+	},
+};
+
+static
+struct acpi_ipmi_msg *acpi_alloc_ipmi_msg(struct acpi_ipmi_device *ipmi)
+{
+	struct acpi_ipmi_msg *ipmi_msg;
+
+	ipmi_msg = kzalloc(sizeof(struct acpi_ipmi_msg), GFP_KERNEL);
+	if (!ipmi_msg)	{
+		printk(KERN_DEBUG "Can't allocate memory for ipmi_msg\n");
+		return NULL;
+	}
+	init_completion(&ipmi_msg->tx_complete);
+	INIT_LIST_HEAD(&ipmi_msg->head);
+	ipmi_msg->device = ipmi;
+	return ipmi_msg;
+}
+
+static void acpi_format_ipmi_msg(struct acpi_ipmi_msg *tx_msg,
+		acpi_physical_address address,
+		acpi_integer *value)
+{
+	struct kernel_ipmi_msg *msg;
+	u8	temp_value;
+	struct acpi_ipmi_buffer *buffer;
+	struct acpi_ipmi_device *device;
+
+	msg = &tx_msg->tx_message;
+	/* get the netfn */
+	temp_value = (address >> 8) & 0xff;
+	msg->netfn = temp_value;
+	/* get the command */
+	temp_value = address & 0xff;
+	msg->cmd = temp_value;
+	msg->data = tx_msg->tx_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 */
+	msg->data_len = buffer->length;
+	memcpy(tx_msg->tx_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,
+	 * the addr type should be changed to IPMB.
+	 */
+	tx_msg->addr.addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE;
+	tx_msg->addr.channel = IPMI_BMC_CHANNEL;
+	tx_msg->addr.data[0] = 0;
+
+	/*
+	 * If the netfn is APP_REQUEST and the cmd is SEND_MESSAGE, we should
+	 * parse the IPMI request message buffer to get the IPMB address.
+	 * If so, please fix me.
+	 */
+
+	/* Get the msgid */
+	device = tx_msg->device;
+	mutex_lock(&device->mutex_lock);
+	device->curr_msgid++;
+	tx_msg->tx_msgid = device->curr_msgid;
+	mutex_unlock(&device->mutex_lock);
+}
+
+static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg,
+		acpi_integer *value, int timeout)
+{
+	struct acpi_ipmi_buffer *buffer;
+
+	/*
+	 * value is also used as output parameter. It represents the response
+	 * IPMI message returned by IPMI command.
+	 */
+	buffer = (struct acpi_ipmi_buffer *)value;
+	/* when timeout is zero, it means that the timeout happens */
+	if (!timeout) {
+		/* the status code is ACPI_IPMI_TIMEOUT */
+		buffer->status_code = ACPI_IPMI_TIMEOUT;
+		return;
+	}
+	/*
+	 * If the flag of msg_done is not set, it means that the IPMI command
+	 * is not executed correctly.
+	 * The status code will be ACPI_IPMI_UNKNOWN.
+	 */
+	if (!msg->msg_done) {
+		buffer->status_code = ACPI_IPMI_UNKNOWN;
+		return;
+	}
+	/*
+	 * If the IPMI response message is obtained correctly, the status code
+	 * will be ACPI_IPMI_OK
+	 */
+	buffer->status_code = ACPI_IPMI_OK;
+	buffer->length = msg->rx_len;
+	memcpy(buffer->data, msg->rx_data, msg->rx_len);
+	return;
+}
+static void ipmi_destroy_tx_msg(struct acpi_ipmi_device *ipmi)
+{
+	struct acpi_ipmi_msg *tx_msg = NULL;
+	int count = 20;
+
+	list_for_each_entry(tx_msg, &ipmi->tx_msg_list, head) {
+		/* wake up the sleep thread on the Tx msg */
+		complete(&tx_msg->tx_complete);
+	}
+	/* wait for about 20 ticks 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))
+		printk(KERN_DEBUG "tx msg list is not NULL\n");
+
+}
+
+static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
+{
+	struct acpi_ipmi_device *ipmi_device =
+			(struct acpi_ipmi_device *)user_msg_data;
+	int msg_found = 0;
+	struct acpi_ipmi_msg *tx_msg = NULL;
+
+	if (msg->user != ipmi_device->user_interface) {
+		printk(KERN_DEBUG "Incorrect IPMI user\n");
+		ipmi_free_recv_msg(msg);
+		return;
+	}
+	mutex_lock(&ipmi_device->mutex_lock);
+	list_for_each_entry(tx_msg, &ipmi_device->tx_msg_list, head) {
+		if (msg->msgid == tx_msg->tx_msgid) {
+			/* find the message id */
+			msg_found = 1;
+			break;
+		}
+	}
+
+	mutex_unlock(&ipmi_device->mutex_lock);
+	if (!msg_found) {
+		/* no matched msg is found . But we should free it */
+		ipmi_free_recv_msg(msg);
+		printk(KERN_DEBUG "Incorrect MSG is found \n");
+		return;
+	}
+
+	if (msg->msg.data_len > 1) {
+		/* copy the response data to Rx_data buffer */
+		memcpy(tx_msg->rx_data, msg->msg_data, msg->msg.data_len);
+		tx_msg->rx_len = msg->msg.data_len;
+		tx_msg->msg_done = 1;
+	}
+	complete(&tx_msg->tx_complete);
+	ipmi_free_recv_msg(msg);
+};
+
+static void ipmi_register_bmc(int iface, struct device *dev)
+{
+	struct acpi_ipmi_device *ipmi_device;
+	struct acpi_device *device;
+	ipmi_user_t		user;
+	int err;
+
+	if (list_empty(&driver_data.ipmi_devices))
+		return;
+
+	list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
+		device = ipmi_device->device;
+		if (ipmi_device->user_interface) {
+			/*
+			 * Only one user interface is allowed to be registered
+			 * for one IPMI device.
+			 * If we already create the user interface for
+			 * one IPMI device, skip it
+			 */
+			continue;
+		}
+		if (dev == &device->dev) {
+			/*
+			 * If the dev is identical to the ACPI device,
+			 * create the user interface.
+			 */
+			err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs,
+						ipmi_device, &user);
+			if (err == 0)
+				ipmi_device->user_interface = user;
+
+			continue;
+		}
+		/*
+		 * In fact maybe the IPMI interface can be registered by
+		 * other methods. For example: SPMI, DMI, PCI
+		 * So we should also create the user interface.
+		 */
+		err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs,
+						ipmi_device, &user);
+		if (err == 0)
+			ipmi_device->user_interface = user;
+	}
+	return;
+}
+
+static void ipmi_bmc_gone(int iface)
+{
+	struct acpi_ipmi_device *ipmi_device;
+
+	if (list_empty(&driver_data.ipmi_devices))
+		return;
+
+	list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
+		if (ipmi_device->user_interface) {
+			ipmi_destroy_user(ipmi_device->user_interface);
+			ipmi_device->user_interface = NULL;
+			/* we should also destory tx msg list */
+			ipmi_destroy_tx_msg(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
+ * by command, only write is meaningful.
+ * @address: This contains the netfn/command of IPMI request message.
+ * @bits   : not used.
+ * @value  : it is an in/out parameter. It points to the IPMI message buffer.
+ *	     Before the IPMI message is sent, it represents the actual request
+ *	     IPMI message. After the IPMI message is finished, it represents
+ *	     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)
+{
+	struct acpi_ipmi_msg *tx_msg = NULL;
+	struct acpi_ipmi_device *ipmi_device =
+			(struct acpi_ipmi_device *) handler_context;
+	int err;
+	acpi_status status;
+	/*
+	 * IPMI opregion message.
+	 * IPMI message is firstly written to the BMC and system software
+	 * can get the respsonse. So it is unmeaningful for the IPMI read
+	 * access.
+	 */
+	if ((function & ACPI_IO_MASK) == ACPI_READ) {
+		/* Read function is not supported. AE_TYPE is returned. */
+		return AE_TYPE;
+	}
+	if (!ipmi_device->user_interface) {
+		/* there doesn't exist user interface*/
+		return AE_NOT_EXIST;
+	}
+	tx_msg = acpi_alloc_ipmi_msg(ipmi_device);
+	if (!tx_msg) {
+		/* no memory is allocated */
+		return AE_NO_MEMORY;
+	}
+	acpi_format_ipmi_msg(tx_msg, address, value);
+	mutex_lock(&ipmi_device->mutex_lock);
+	list_add_tail(&tx_msg->head, &ipmi_device->tx_msg_list);
+	mutex_unlock(&ipmi_device->mutex_lock);
+	err = ipmi_request_settime(ipmi_device->user_interface,
+					&tx_msg->addr,
+					tx_msg->tx_msgid,
+					&tx_msg->tx_message,
+					NULL, 0, 0, 0);
+	if (err) {
+		status = AE_ERROR;
+		goto end_label;
+	}
+	err = wait_for_completion_timeout(&tx_msg->tx_complete, IPMI_TIMEOUT);
+
+end_label:
+	acpi_format_ipmi_response(tx_msg, value, err);
+	status = AE_OK;
+	mutex_lock(&ipmi_device->mutex_lock);
+	list_del(&tx_msg->head);
+	mutex_unlock(&ipmi_device->mutex_lock);
+	kfree(tx_msg);
+	return status;
+}
+
+static void ipmi_remove_handlers(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_handlers(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, ipmi);
+	if (ACPI_FAILURE(status)) {
+		printk(KERN_DEBUG "Can't register IPMI opregion %s\n",
+					acpi_device_bid(ipmi->device));
+		return -EINVAL;
+	}
+	set_bit(IPMI_FLAGS_HANDLER_INSTALL, &ipmi->flags);
+	return 0;
+}
+
+static int acpi_ipmi_add(struct acpi_device *device)
+{
+	struct acpi_ipmi_device *ipmi_device;
+	acpi_handle handle;
+	unsigned long long temp;
+	acpi_status status;
+	if (!device)
+		return -EINVAL;
+
+	handle = device->handle;
+	temp = 0;
+	status = acpi_evaluate_integer(handle, "_IFT", NULL, &temp);
+	if (ACPI_FAILURE(status)) {
+		printk(KERN_DEBUG "Incorrect _IFT object for %s\n",
+				acpi_device_bid(device));
+		return -ENODEV;
+	}
+	ipmi_device = kzalloc(sizeof(struct acpi_ipmi_device), GFP_KERNEL);
+	if (!ipmi_device) {
+		printk(KERN_DEBUG "Can't allocate memory space\n");
+		return -ENOMEM;
+	}
+	ipmi_device->if_type = temp;
+	switch (ipmi_device->if_type) {
+	case 1:
+	case 2:
+	case 3:
+		break;
+	default:
+		printk(KERN_DEBUG "Unknow IPMI:SI interface type %d\n",
+				ipmi_device->if_type);
+		kfree(ipmi_device);
+		return -EINVAL;
+	}
+	ipmi_device->handle = device->handle;
+	ipmi_device->device = device;
+	mutex_init(&ipmi_device->mutex_lock);
+	INIT_LIST_HEAD(&ipmi_device->head);
+	INIT_LIST_HEAD(&ipmi_device->tx_msg_list);
+
+	if (ipmi_install_handlers(ipmi_device)) {
+		/* can't register the IPMI opregion */
+		kfree(ipmi_device);
+		return -EINVAL;
+	}
+
+	/* add it to the IPMI device list */
+	list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices);
+	device->driver_data = ipmi_device;
+	return 0;
+}
+
+static int acpi_ipmi_remove(struct acpi_device *device, int type)
+{
+	struct acpi_ipmi_device *ipmi_device;
+
+	ipmi_device = acpi_driver_data(device);
+	if (!ipmi_device)
+		return 0;
+
+	if (ipmi_device->user_interface) {
+		/*
+		 * If the IPMI user interface is created, it should be
+		 * destroyed.
+		 */
+		ipmi_destroy_user(ipmi_device->user_interface);
+		ipmi_device->user_interface = NULL;
+	}
+	list_del(&ipmi_device->head);
+	if (!list_empty(&ipmi_device->tx_msg_list)) {
+		/* destroy the Tx_msg list */
+		ipmi_destroy_tx_msg(ipmi_device);
+	}
+	ipmi_remove_handlers(ipmi_device);
+	kfree(ipmi_device);
+	device->driver_data = NULL;
+	return 0;
+}
+
+static const struct acpi_device_id ipmi_device_ids[] = {
+	{"IPI0001", 0},
+	{"", 0},
+};
+
+static struct acpi_driver acpi_ipmi_driver = {
+	.name = "ipmi",
+	.class = ACPI_IPMI_CLASS,
+	.ids = ipmi_device_ids,
+	.ops = {
+		.add = acpi_ipmi_add,
+		.remove = acpi_ipmi_remove,
+		},
+};
+
+static int __init acpi_ipmi_init(void)
+{
+	int result = 0;
+
+	if (acpi_disabled)
+		return result;
+
+	result = acpi_bus_register_driver(&acpi_ipmi_driver);
+
+	if (result)
+		return result;
+
+	result = ipmi_smi_watcher_register(&driver_data.bmc_events);
+
+	if (result)
+		acpi_bus_unregister_driver(&acpi_ipmi_driver);
+
+	return result;
+}
+
+static void __exit acpi_ipmi_exit(void)
+{
+	if (acpi_disabled)
+		return;
+
+	ipmi_smi_watcher_unregister(&driver_data.bmc_events);
+	acpi_bus_unregister_driver(&acpi_ipmi_driver);
+
+	return;
+}
+
+module_init(acpi_ipmi_init);
+module_exit(acpi_ipmi_exit);
-- 
1.5.4.5

Re: [Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[Bjorn Helgaas]

On Tuesday 08 December 2009 06:58:45 pm yakui.zhao@intel.com wrote:
> From: Zhao Yakui <yakui.zhao@intel.com>
> 
> Add the IPMI opregion driver so that the AML code can communicate with BMC
> throught IPMI message.
>      It will create IPMI user interface for every IPMI device detected
> in ACPI namespace and install the corresponding IPMI opregion space handler.
> 
> The following describes how to process the IPMI request in IPMI space handler:
>     1. format the IPMI message based on the request in AML code.
>     IPMI system address. Now the address type is SYSTEM_INTERFACE_ADDR_TYPE
>     IPMI net function & command
>     IPMI message payload
>     2. send the IPMI message by using the function of ipmi_request_settime
>     3. wait for the completion of IPMI message. It can be done in different
> routes: One is in handled in IPMI user recv callback function. Another is
> handled in timeout function.
>     4. format the IPMI response and return it to ACPI AML code.

I think this should be part of the ipmi_si_intf module, not a separate
module.  This opregion code is of no use without ipmi_si_intf.  The
opregion code can still be under its own config option, but I don't
see the value in having it be a separate module.

The opregion code should definitely not use acpi_bus_register_driver()
to claim IPI0001 devices.  The ipmi_si_intf driver already uses
pnp_register_driver() to claim IPI0001 devices.  The fact that Linux
allows two drivers to claim the same device (one via pnp_register_driver()
and the other via acpi_bus_register_driver()) is a defect in Linux, and
we shouldn't take advantage of that.

Bjorn

> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> ---
>  drivers/acpi/Kconfig  |   12 +
>  drivers/acpi/Makefile |    1 +
>  drivers/acpi/ipmi.c   |  583 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 596 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/acpi/ipmi.c
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 93d2c79..23cd08a 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -204,6 +204,18 @@ 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 EXPERIMENTAL
> +	default n
> +	select IPMI_HANDLER
> +	select IPMI_SI
> +	help
> +	  This driver enables the ACPI to access the BMC controller. And it
> +	  uses the IPMI request/response message to communicate with BMC
> +	  controller, which can be found on on the server.
> +
> +	  To compile this driver as a module, choose M here:
>  
>  config ACPI_HOTPLUG_CPU
>  	bool
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 7702118..61077b7 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -57,6 +57,7 @@ obj-$(CONFIG_ACPI_BATTERY)	+= battery.o
>  obj-$(CONFIG_ACPI_SBS)		+= sbshc.o
>  obj-$(CONFIG_ACPI_SBS)		+= sbs.o
>  obj-$(CONFIG_ACPI_POWER_METER)	+= power_meter.o
> +obj-$(CONFIG_ACPI_IPMI)		+= ipmi.o
>  
>  # processor has its own "processor." module_param namespace
>  processor-y			:= processor_core.o processor_throttling.o
> diff --git a/drivers/acpi/ipmi.c b/drivers/acpi/ipmi.c
> new file mode 100644
> index 0000000..5c74936
> --- /dev/null
> +++ b/drivers/acpi/ipmi.c
> @@ -0,0 +1,583 @@
> +/*
> + *  ipmi.c - ACPI IPMI opregion
> + *
> + *  Copyright (C) 2009 Intel Corporation
> + *  Copyright (C) 2009 Zhao Yakui <yakui.zhao@intel.com>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or (at
> + *  your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful, but
> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/delay.h>
> +#include <linux/proc_fs.h>
> +#include <linux/seq_file.h>
> +#include <linux/interrupt.h>
> +#include <linux/list.h>
> +#include <linux/spinlock.h>
> +#include <linux/io.h>
> +#include <acpi/acpi_bus.h>
> +#include <acpi/acpi_drivers.h>
> +#include <linux/ipmi.h>
> +
> +MODULE_AUTHOR("Zhao Yakui");
> +MODULE_DESCRIPTION("ACPI IPMI Opregion driver");
> +MODULE_LICENSE("GPL");
> +
> +#define ACPI_IPMI_CLASS			"IPMI"
> +#define ACPI_IPMI_DEVICE_NAME		"IPMI_dev"
> +#define IPMI_FLAGS_HANDLER_INSTALL	0
> +
> +#define ACPI_IPMI_OK			0
> +#define ACPI_IPMI_TIMEOUT		0x10
> +#define ACPI_IPMI_UNKNOWN		0x07
> +/* the IPMI timeout is 30s */
> +#define IPMI_TIMEOUT			(30 * HZ)
> +
> +
> +struct acpi_ipmi_device {
> +	acpi_handle handle;
> +	struct acpi_device *device;
> +	int if_type;
> +	/* the device list attached to driver_data.ipmi_devices */
> +	struct list_head head;
> +	ipmi_user_t 	user_interface;
> +	struct mutex	mutex_lock;
> +	/* the IPMI request message list */
> +	struct list_head tx_msg_list;
> +	long curr_msgid;
> +	/* IPMI flags */
> +	unsigned long flags;
> +};
> +
> +struct ipmi_driver_data {
> +	int device_count;
> +	struct list_head	ipmi_devices;
> +	struct ipmi_smi_watcher	bmc_events;
> +	struct ipmi_user_hndl	ipmi_hndlrs;
> +};
> +
> +struct acpi_ipmi_msg {
> +	/* message list */
> +	struct list_head head;
> +	/*
> +	 * General speaking the addr type should be SI_ADDR_TYPE. And
> +	 * the addr channel should be BMC.
> +	 * In fact it can also be IPMB type. But we will have to
> +	 * parse it from the Netfn command buffer. It is so complex
> +	 * that it is skipped.
> +	 */
> +	struct ipmi_addr addr;
> +	/* tx message id */
> +	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;
> +	/* tx data . And copy it from ACPI object buffer */
> +	u8	tx_data[64];
> +	int	tx_len;
> +	/* get the response data */
> +	u8	rx_data[64];
> +	/* the response length. The netfn & cmd is excluded. */
> +	int	rx_len;
> +	struct acpi_ipmi_device *device;
> +};
> +
> +/*
> + * IPMI request/response buffer.
> + * The length is 66 bytes.
> + */
> +struct acpi_ipmi_buffer {
> +	/* status code of a given IPMI command */
> +	u8 status_code;
> +	/* the length of the payload */
> +	u8 length;
> +	/*
> +	 * the payload. Before the operation is carried out, it represents the
> +	 * request message payload. After the opration is carried out, it
> +	 * stores the response message returned by IPMI command.
> +	 */
> +	u8 data[64];
> +};
> +
> +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 struct ipmi_driver_data driver_data = {
> +	.ipmi_devices = LIST_HEAD_INIT(driver_data.ipmi_devices),
> +	.bmc_events = {
> +		.owner = THIS_MODULE,
> +		.new_smi = ipmi_register_bmc,
> +		.smi_gone = ipmi_bmc_gone,
> +	},
> +	.ipmi_hndlrs = {
> +		.ipmi_recv_hndl = ipmi_msg_handler,
> +	},
> +};
> +
> +static
> +struct acpi_ipmi_msg *acpi_alloc_ipmi_msg(struct acpi_ipmi_device *ipmi)
> +{
> +	struct acpi_ipmi_msg *ipmi_msg;
> +
> +	ipmi_msg = kzalloc(sizeof(struct acpi_ipmi_msg), GFP_KERNEL);
> +	if (!ipmi_msg)	{
> +		printk(KERN_DEBUG "Can't allocate memory for ipmi_msg\n");
> +		return NULL;
> +	}
> +	init_completion(&ipmi_msg->tx_complete);
> +	INIT_LIST_HEAD(&ipmi_msg->head);
> +	ipmi_msg->device = ipmi;
> +	return ipmi_msg;
> +}
> +
> +static void acpi_format_ipmi_msg(struct acpi_ipmi_msg *tx_msg,
> +		acpi_physical_address address,
> +		acpi_integer *value)
> +{
> +	struct kernel_ipmi_msg *msg;
> +	u8	temp_value;
> +	struct acpi_ipmi_buffer *buffer;
> +	struct acpi_ipmi_device *device;
> +
> +	msg = &tx_msg->tx_message;
> +	/* get the netfn */
> +	temp_value = (address >> 8) & 0xff;
> +	msg->netfn = temp_value;
> +	/* get the command */
> +	temp_value = address & 0xff;
> +	msg->cmd = temp_value;
> +	msg->data = tx_msg->tx_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 */
> +	msg->data_len = buffer->length;
> +	memcpy(tx_msg->tx_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,
> +	 * the addr type should be changed to IPMB.
> +	 */
> +	tx_msg->addr.addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE;
> +	tx_msg->addr.channel = IPMI_BMC_CHANNEL;
> +	tx_msg->addr.data[0] = 0;
> +
> +	/*
> +	 * If the netfn is APP_REQUEST and the cmd is SEND_MESSAGE, we should
> +	 * parse the IPMI request message buffer to get the IPMB address.
> +	 * If so, please fix me.
> +	 */
> +
> +	/* Get the msgid */
> +	device = tx_msg->device;
> +	mutex_lock(&device->mutex_lock);
> +	device->curr_msgid++;
> +	tx_msg->tx_msgid = device->curr_msgid;
> +	mutex_unlock(&device->mutex_lock);
> +}
> +
> +static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg,
> +		acpi_integer *value, int timeout)
> +{
> +	struct acpi_ipmi_buffer *buffer;
> +
> +	/*
> +	 * value is also used as output parameter. It represents the response
> +	 * IPMI message returned by IPMI command.
> +	 */
> +	buffer = (struct acpi_ipmi_buffer *)value;
> +	/* when timeout is zero, it means that the timeout happens */
> +	if (!timeout) {
> +		/* the status code is ACPI_IPMI_TIMEOUT */
> +		buffer->status_code = ACPI_IPMI_TIMEOUT;
> +		return;
> +	}
> +	/*
> +	 * If the flag of msg_done is not set, it means that the IPMI command
> +	 * is not executed correctly.
> +	 * The status code will be ACPI_IPMI_UNKNOWN.
> +	 */
> +	if (!msg->msg_done) {
> +		buffer->status_code = ACPI_IPMI_UNKNOWN;
> +		return;
> +	}
> +	/*
> +	 * If the IPMI response message is obtained correctly, the status code
> +	 * will be ACPI_IPMI_OK
> +	 */
> +	buffer->status_code = ACPI_IPMI_OK;
> +	buffer->length = msg->rx_len;
> +	memcpy(buffer->data, msg->rx_data, msg->rx_len);
> +	return;
> +}
> +static void ipmi_destroy_tx_msg(struct acpi_ipmi_device *ipmi)
> +{
> +	struct acpi_ipmi_msg *tx_msg = NULL;
> +	int count = 20;
> +
> +	list_for_each_entry(tx_msg, &ipmi->tx_msg_list, head) {
> +		/* wake up the sleep thread on the Tx msg */
> +		complete(&tx_msg->tx_complete);
> +	}
> +	/* wait for about 20 ticks 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))
> +		printk(KERN_DEBUG "tx msg list is not NULL\n");
> +
> +}
> +
> +static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
> +{
> +	struct acpi_ipmi_device *ipmi_device =
> +			(struct acpi_ipmi_device *)user_msg_data;
> +	int msg_found = 0;
> +	struct acpi_ipmi_msg *tx_msg = NULL;
> +
> +	if (msg->user != ipmi_device->user_interface) {
> +		printk(KERN_DEBUG "Incorrect IPMI user\n");
> +		ipmi_free_recv_msg(msg);
> +		return;
> +	}
> +	mutex_lock(&ipmi_device->mutex_lock);
> +	list_for_each_entry(tx_msg, &ipmi_device->tx_msg_list, head) {
> +		if (msg->msgid == tx_msg->tx_msgid) {
> +			/* find the message id */
> +			msg_found = 1;
> +			break;
> +		}
> +	}
> +
> +	mutex_unlock(&ipmi_device->mutex_lock);
> +	if (!msg_found) {
> +		/* no matched msg is found . But we should free it */
> +		ipmi_free_recv_msg(msg);
> +		printk(KERN_DEBUG "Incorrect MSG is found \n");
> +		return;
> +	}
> +
> +	if (msg->msg.data_len > 1) {
> +		/* copy the response data to Rx_data buffer */
> +		memcpy(tx_msg->rx_data, msg->msg_data, msg->msg.data_len);
> +		tx_msg->rx_len = msg->msg.data_len;
> +		tx_msg->msg_done = 1;
> +	}
> +	complete(&tx_msg->tx_complete);
> +	ipmi_free_recv_msg(msg);
> +};
> +
> +static void ipmi_register_bmc(int iface, struct device *dev)
> +{
> +	struct acpi_ipmi_device *ipmi_device;
> +	struct acpi_device *device;
> +	ipmi_user_t		user;
> +	int err;
> +
> +	if (list_empty(&driver_data.ipmi_devices))
> +		return;
> +
> +	list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
> +		device = ipmi_device->device;
> +		if (ipmi_device->user_interface) {
> +			/*
> +			 * Only one user interface is allowed to be registered
> +			 * for one IPMI device.
> +			 * If we already create the user interface for
> +			 * one IPMI device, skip it
> +			 */
> +			continue;
> +		}
> +		if (dev == &device->dev) {
> +			/*
> +			 * If the dev is identical to the ACPI device,
> +			 * create the user interface.
> +			 */
> +			err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs,
> +						ipmi_device, &user);
> +			if (err == 0)
> +				ipmi_device->user_interface = user;
> +
> +			continue;
> +		}
> +		/*
> +		 * In fact maybe the IPMI interface can be registered by
> +		 * other methods. For example: SPMI, DMI, PCI
> +		 * So we should also create the user interface.
> +		 */
> +		err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs,
> +						ipmi_device, &user);
> +		if (err == 0)
> +			ipmi_device->user_interface = user;
> +	}
> +	return;
> +}
> +
> +static void ipmi_bmc_gone(int iface)
> +{
> +	struct acpi_ipmi_device *ipmi_device;
> +
> +	if (list_empty(&driver_data.ipmi_devices))
> +		return;
> +
> +	list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
> +		if (ipmi_device->user_interface) {
> +			ipmi_destroy_user(ipmi_device->user_interface);
> +			ipmi_device->user_interface = NULL;
> +			/* we should also destory tx msg list */
> +			ipmi_destroy_tx_msg(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
> + * by command, only write is meaningful.
> + * @address: This contains the netfn/command of IPMI request message.
> + * @bits   : not used.
> + * @value  : it is an in/out parameter. It points to the IPMI message buffer.
> + *	     Before the IPMI message is sent, it represents the actual request
> + *	     IPMI message. After the IPMI message is finished, it represents
> + *	     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)
> +{
> +	struct acpi_ipmi_msg *tx_msg = NULL;
> +	struct acpi_ipmi_device *ipmi_device =
> +			(struct acpi_ipmi_device *) handler_context;
> +	int err;
> +	acpi_status status;
> +	/*
> +	 * IPMI opregion message.
> +	 * IPMI message is firstly written to the BMC and system software
> +	 * can get the respsonse. So it is unmeaningful for the IPMI read
> +	 * access.
> +	 */
> +	if ((function & ACPI_IO_MASK) == ACPI_READ) {
> +		/* Read function is not supported. AE_TYPE is returned. */
> +		return AE_TYPE;
> +	}
> +	if (!ipmi_device->user_interface) {
> +		/* there doesn't exist user interface*/
> +		return AE_NOT_EXIST;
> +	}
> +	tx_msg = acpi_alloc_ipmi_msg(ipmi_device);
> +	if (!tx_msg) {
> +		/* no memory is allocated */
> +		return AE_NO_MEMORY;
> +	}
> +	acpi_format_ipmi_msg(tx_msg, address, value);
> +	mutex_lock(&ipmi_device->mutex_lock);
> +	list_add_tail(&tx_msg->head, &ipmi_device->tx_msg_list);
> +	mutex_unlock(&ipmi_device->mutex_lock);
> +	err = ipmi_request_settime(ipmi_device->user_interface,
> +					&tx_msg->addr,
> +					tx_msg->tx_msgid,
> +					&tx_msg->tx_message,
> +					NULL, 0, 0, 0);
> +	if (err) {
> +		status = AE_ERROR;
> +		goto end_label;
> +	}
> +	err = wait_for_completion_timeout(&tx_msg->tx_complete, IPMI_TIMEOUT);
> +
> +end_label:
> +	acpi_format_ipmi_response(tx_msg, value, err);
> +	status = AE_OK;
> +	mutex_lock(&ipmi_device->mutex_lock);
> +	list_del(&tx_msg->head);
> +	mutex_unlock(&ipmi_device->mutex_lock);
> +	kfree(tx_msg);
> +	return status;
> +}
> +
> +static void ipmi_remove_handlers(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_handlers(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, ipmi);
> +	if (ACPI_FAILURE(status)) {
> +		printk(KERN_DEBUG "Can't register IPMI opregion %s\n",
> +					acpi_device_bid(ipmi->device));
> +		return -EINVAL;
> +	}
> +	set_bit(IPMI_FLAGS_HANDLER_INSTALL, &ipmi->flags);
> +	return 0;
> +}
> +
> +static int acpi_ipmi_add(struct acpi_device *device)
> +{
> +	struct acpi_ipmi_device *ipmi_device;
> +	acpi_handle handle;
> +	unsigned long long temp;
> +	acpi_status status;
> +	if (!device)
> +		return -EINVAL;
> +
> +	handle = device->handle;
> +	temp = 0;
> +	status = acpi_evaluate_integer(handle, "_IFT", NULL, &temp);
> +	if (ACPI_FAILURE(status)) {
> +		printk(KERN_DEBUG "Incorrect _IFT object for %s\n",
> +				acpi_device_bid(device));
> +		return -ENODEV;
> +	}
> +	ipmi_device = kzalloc(sizeof(struct acpi_ipmi_device), GFP_KERNEL);
> +	if (!ipmi_device) {
> +		printk(KERN_DEBUG "Can't allocate memory space\n");
> +		return -ENOMEM;
> +	}
> +	ipmi_device->if_type = temp;
> +	switch (ipmi_device->if_type) {
> +	case 1:
> +	case 2:
> +	case 3:
> +		break;
> +	default:
> +		printk(KERN_DEBUG "Unknow IPMI:SI interface type %d\n",
> +				ipmi_device->if_type);
> +		kfree(ipmi_device);
> +		return -EINVAL;
> +	}
> +	ipmi_device->handle = device->handle;
> +	ipmi_device->device = device;
> +	mutex_init(&ipmi_device->mutex_lock);
> +	INIT_LIST_HEAD(&ipmi_device->head);
> +	INIT_LIST_HEAD(&ipmi_device->tx_msg_list);
> +
> +	if (ipmi_install_handlers(ipmi_device)) {
> +		/* can't register the IPMI opregion */
> +		kfree(ipmi_device);
> +		return -EINVAL;
> +	}
> +
> +	/* add it to the IPMI device list */
> +	list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices);
> +	device->driver_data = ipmi_device;
> +	return 0;
> +}
> +
> +static int acpi_ipmi_remove(struct acpi_device *device, int type)
> +{
> +	struct acpi_ipmi_device *ipmi_device;
> +
> +	ipmi_device = acpi_driver_data(device);
> +	if (!ipmi_device)
> +		return 0;
> +
> +	if (ipmi_device->user_interface) {
> +		/*
> +		 * If the IPMI user interface is created, it should be
> +		 * destroyed.
> +		 */
> +		ipmi_destroy_user(ipmi_device->user_interface);
> +		ipmi_device->user_interface = NULL;
> +	}
> +	list_del(&ipmi_device->head);
> +	if (!list_empty(&ipmi_device->tx_msg_list)) {
> +		/* destroy the Tx_msg list */
> +		ipmi_destroy_tx_msg(ipmi_device);
> +	}
> +	ipmi_remove_handlers(ipmi_device);
> +	kfree(ipmi_device);
> +	device->driver_data = NULL;
> +	return 0;
> +}
> +
> +static const struct acpi_device_id ipmi_device_ids[] = {
> +	{"IPI0001", 0},
> +	{"", 0},
> +};
> +
> +static struct acpi_driver acpi_ipmi_driver = {
> +	.name = "ipmi",
> +	.class = ACPI_IPMI_CLASS,
> +	.ids = ipmi_device_ids,
> +	.ops = {
> +		.add = acpi_ipmi_add,
> +		.remove = acpi_ipmi_remove,
> +		},
> +};
> +
> +static int __init acpi_ipmi_init(void)
> +{
> +	int result = 0;
> +
> +	if (acpi_disabled)
> +		return result;
> +
> +	result = acpi_bus_register_driver(&acpi_ipmi_driver);
> +
> +	if (result)
> +		return result;
> +
> +	result = ipmi_smi_watcher_register(&driver_data.bmc_events);
> +
> +	if (result)
> +		acpi_bus_unregister_driver(&acpi_ipmi_driver);
> +
> +	return result;
> +}
> +
> +static void __exit acpi_ipmi_exit(void)
> +{
> +	if (acpi_disabled)
> +		return;
> +
> +	ipmi_smi_watcher_unregister(&driver_data.bmc_events);
> +	acpi_bus_unregister_driver(&acpi_ipmi_driver);
> +
> +	return;
> +}
> +
> +module_init(acpi_ipmi_init);
> +module_exit(acpi_ipmi_exit);

Re: [Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[Len Brown]

On Wed, 9 Dec 2009, Bjorn Helgaas wrote:

> On Tuesday 08 December 2009 06:58:45 pm yakui.zhao@intel.com wrote:
> > From: Zhao Yakui <yakui.zhao@intel.com>
> > 
> > Add the IPMI opregion driver so that the AML code can communicate with BMC
> > throught IPMI message.
> >      It will create IPMI user interface for every IPMI device detected
> > in ACPI namespace and install the corresponding IPMI opregion space handler.
> > 
> > The following describes how to process the IPMI request in IPMI space handler:
> >     1. format the IPMI message based on the request in AML code.
> >     IPMI system address. Now the address type is SYSTEM_INTERFACE_ADDR_TYPE
> >     IPMI net function & command
> >     IPMI message payload
> >     2. send the IPMI message by using the function of ipmi_request_settime
> >     3. wait for the completion of IPMI message. It can be done in different
> > routes: One is in handled in IPMI user recv callback function. Another is
> > handled in timeout function.
> >     4. format the IPMI response and return it to ACPI AML code.
> 
> I think this should be part of the ipmi_si_intf module, not a separate
> module.  This opregion code is of no use without ipmi_si_intf.  The
> opregion code can still be under its own config option, but I don't
> see the value in having it be a separate module.


I agree.  Corey?

thanks,
-Len Brown, Intel Open Source technology Center

> The opregion code should definitely not use acpi_bus_register_driver()
> to claim IPI0001 devices.  The ipmi_si_intf driver already uses
> pnp_register_driver() to claim IPI0001 devices.  The fact that Linux
> allows two drivers to claim the same device (one via pnp_register_driver()
> and the other via acpi_bus_register_driver()) is a defect in Linux, and
> we shouldn't take advantage of that.
> 
> Bjorn
> 
> > Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> > ---
> >  drivers/acpi/Kconfig  |   12 +
> >  drivers/acpi/Makefile |    1 +
> >  drivers/acpi/ipmi.c   |  583 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 596 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/acpi/ipmi.c
> > 
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index 93d2c79..23cd08a 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -204,6 +204,18 @@ 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 EXPERIMENTAL
> > +	default n
> > +	select IPMI_HANDLER
> > +	select IPMI_SI
> > +	help
> > +	  This driver enables the ACPI to access the BMC controller. And it
> > +	  uses the IPMI request/response message to communicate with BMC
> > +	  controller, which can be found on on the server.
> > +
> > +	  To compile this driver as a module, choose M here:
> >  
> >  config ACPI_HOTPLUG_CPU
> >  	bool
> > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> > index 7702118..61077b7 100644
> > --- a/drivers/acpi/Makefile
> > +++ b/drivers/acpi/Makefile
> > @@ -57,6 +57,7 @@ obj-$(CONFIG_ACPI_BATTERY)	+= battery.o
> >  obj-$(CONFIG_ACPI_SBS)		+= sbshc.o
> >  obj-$(CONFIG_ACPI_SBS)		+= sbs.o
> >  obj-$(CONFIG_ACPI_POWER_METER)	+= power_meter.o
> > +obj-$(CONFIG_ACPI_IPMI)		+= ipmi.o
> >  
> >  # processor has its own "processor." module_param namespace
> >  processor-y			:= processor_core.o processor_throttling.o
> > diff --git a/drivers/acpi/ipmi.c b/drivers/acpi/ipmi.c
> > new file mode 100644
> > index 0000000..5c74936
> > --- /dev/null
> > +++ b/drivers/acpi/ipmi.c
> > @@ -0,0 +1,583 @@
> > +/*
> > + *  ipmi.c - ACPI IPMI opregion
> > + *
> > + *  Copyright (C) 2009 Intel Corporation
> > + *  Copyright (C) 2009 Zhao Yakui <yakui.zhao@intel.com>
> > + *
> > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > + *
> > + *  This program is free software; you can redistribute it and/or modify
> > + *  it under the terms of the GNU General Public License as published by
> > + *  the Free Software Foundation; either version 2 of the License, or (at
> > + *  your option) any later version.
> > + *
> > + *  This program is distributed in the hope that it will be useful, but
> > + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + *  General Public License for more details.
> > + *
> > + *  You should have received a copy of the GNU General Public License along
> > + *  with this program; if not, write to the Free Software Foundation, Inc.,
> > + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> > + *
> > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/types.h>
> > +#include <linux/delay.h>
> > +#include <linux/proc_fs.h>
> > +#include <linux/seq_file.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/list.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/io.h>
> > +#include <acpi/acpi_bus.h>
> > +#include <acpi/acpi_drivers.h>
> > +#include <linux/ipmi.h>
> > +
> > +MODULE_AUTHOR("Zhao Yakui");
> > +MODULE_DESCRIPTION("ACPI IPMI Opregion driver");
> > +MODULE_LICENSE("GPL");
> > +
> > +#define ACPI_IPMI_CLASS			"IPMI"
> > +#define ACPI_IPMI_DEVICE_NAME		"IPMI_dev"
> > +#define IPMI_FLAGS_HANDLER_INSTALL	0
> > +
> > +#define ACPI_IPMI_OK			0
> > +#define ACPI_IPMI_TIMEOUT		0x10
> > +#define ACPI_IPMI_UNKNOWN		0x07
> > +/* the IPMI timeout is 30s */
> > +#define IPMI_TIMEOUT			(30 * HZ)
> > +
> > +
> > +struct acpi_ipmi_device {
> > +	acpi_handle handle;
> > +	struct acpi_device *device;
> > +	int if_type;
> > +	/* the device list attached to driver_data.ipmi_devices */
> > +	struct list_head head;
> > +	ipmi_user_t 	user_interface;
> > +	struct mutex	mutex_lock;
> > +	/* the IPMI request message list */
> > +	struct list_head tx_msg_list;
> > +	long curr_msgid;
> > +	/* IPMI flags */
> > +	unsigned long flags;
> > +};
> > +
> > +struct ipmi_driver_data {
> > +	int device_count;
> > +	struct list_head	ipmi_devices;
> > +	struct ipmi_smi_watcher	bmc_events;
> > +	struct ipmi_user_hndl	ipmi_hndlrs;
> > +};
> > +
> > +struct acpi_ipmi_msg {
> > +	/* message list */
> > +	struct list_head head;
> > +	/*
> > +	 * General speaking the addr type should be SI_ADDR_TYPE. And
> > +	 * the addr channel should be BMC.
> > +	 * In fact it can also be IPMB type. But we will have to
> > +	 * parse it from the Netfn command buffer. It is so complex
> > +	 * that it is skipped.
> > +	 */
> > +	struct ipmi_addr addr;
> > +	/* tx message id */
> > +	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;
> > +	/* tx data . And copy it from ACPI object buffer */
> > +	u8	tx_data[64];
> > +	int	tx_len;
> > +	/* get the response data */
> > +	u8	rx_data[64];
> > +	/* the response length. The netfn & cmd is excluded. */
> > +	int	rx_len;
> > +	struct acpi_ipmi_device *device;
> > +};
> > +
> > +/*
> > + * IPMI request/response buffer.
> > + * The length is 66 bytes.
> > + */
> > +struct acpi_ipmi_buffer {
> > +	/* status code of a given IPMI command */
> > +	u8 status_code;
> > +	/* the length of the payload */
> > +	u8 length;
> > +	/*
> > +	 * the payload. Before the operation is carried out, it represents the
> > +	 * request message payload. After the opration is carried out, it
> > +	 * stores the response message returned by IPMI command.
> > +	 */
> > +	u8 data[64];
> > +};
> > +
> > +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 struct ipmi_driver_data driver_data = {
> > +	.ipmi_devices = LIST_HEAD_INIT(driver_data.ipmi_devices),
> > +	.bmc_events = {
> > +		.owner = THIS_MODULE,
> > +		.new_smi = ipmi_register_bmc,
> > +		.smi_gone = ipmi_bmc_gone,
> > +	},
> > +	.ipmi_hndlrs = {
> > +		.ipmi_recv_hndl = ipmi_msg_handler,
> > +	},
> > +};
> > +
> > +static
> > +struct acpi_ipmi_msg *acpi_alloc_ipmi_msg(struct acpi_ipmi_device *ipmi)
> > +{
> > +	struct acpi_ipmi_msg *ipmi_msg;
> > +
> > +	ipmi_msg = kzalloc(sizeof(struct acpi_ipmi_msg), GFP_KERNEL);
> > +	if (!ipmi_msg)	{
> > +		printk(KERN_DEBUG "Can't allocate memory for ipmi_msg\n");
> > +		return NULL;
> > +	}
> > +	init_completion(&ipmi_msg->tx_complete);
> > +	INIT_LIST_HEAD(&ipmi_msg->head);
> > +	ipmi_msg->device = ipmi;
> > +	return ipmi_msg;
> > +}
> > +
> > +static void acpi_format_ipmi_msg(struct acpi_ipmi_msg *tx_msg,
> > +		acpi_physical_address address,
> > +		acpi_integer *value)
> > +{
> > +	struct kernel_ipmi_msg *msg;
> > +	u8	temp_value;
> > +	struct acpi_ipmi_buffer *buffer;
> > +	struct acpi_ipmi_device *device;
> > +
> > +	msg = &tx_msg->tx_message;
> > +	/* get the netfn */
> > +	temp_value = (address >> 8) & 0xff;
> > +	msg->netfn = temp_value;
> > +	/* get the command */
> > +	temp_value = address & 0xff;
> > +	msg->cmd = temp_value;
> > +	msg->data = tx_msg->tx_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 */
> > +	msg->data_len = buffer->length;
> > +	memcpy(tx_msg->tx_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,
> > +	 * the addr type should be changed to IPMB.
> > +	 */
> > +	tx_msg->addr.addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE;
> > +	tx_msg->addr.channel = IPMI_BMC_CHANNEL;
> > +	tx_msg->addr.data[0] = 0;
> > +
> > +	/*
> > +	 * If the netfn is APP_REQUEST and the cmd is SEND_MESSAGE, we should
> > +	 * parse the IPMI request message buffer to get the IPMB address.
> > +	 * If so, please fix me.
> > +	 */
> > +
> > +	/* Get the msgid */
> > +	device = tx_msg->device;
> > +	mutex_lock(&device->mutex_lock);
> > +	device->curr_msgid++;
> > +	tx_msg->tx_msgid = device->curr_msgid;
> > +	mutex_unlock(&device->mutex_lock);
> > +}
> > +
> > +static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg,
> > +		acpi_integer *value, int timeout)
> > +{
> > +	struct acpi_ipmi_buffer *buffer;
> > +
> > +	/*
> > +	 * value is also used as output parameter. It represents the response
> > +	 * IPMI message returned by IPMI command.
> > +	 */
> > +	buffer = (struct acpi_ipmi_buffer *)value;
> > +	/* when timeout is zero, it means that the timeout happens */
> > +	if (!timeout) {
> > +		/* the status code is ACPI_IPMI_TIMEOUT */
> > +		buffer->status_code = ACPI_IPMI_TIMEOUT;
> > +		return;
> > +	}
> > +	/*
> > +	 * If the flag of msg_done is not set, it means that the IPMI command
> > +	 * is not executed correctly.
> > +	 * The status code will be ACPI_IPMI_UNKNOWN.
> > +	 */
> > +	if (!msg->msg_done) {
> > +		buffer->status_code = ACPI_IPMI_UNKNOWN;
> > +		return;
> > +	}
> > +	/*
> > +	 * If the IPMI response message is obtained correctly, the status code
> > +	 * will be ACPI_IPMI_OK
> > +	 */
> > +	buffer->status_code = ACPI_IPMI_OK;
> > +	buffer->length = msg->rx_len;
> > +	memcpy(buffer->data, msg->rx_data, msg->rx_len);
> > +	return;
> > +}
> > +static void ipmi_destroy_tx_msg(struct acpi_ipmi_device *ipmi)
> > +{
> > +	struct acpi_ipmi_msg *tx_msg = NULL;
> > +	int count = 20;
> > +
> > +	list_for_each_entry(tx_msg, &ipmi->tx_msg_list, head) {
> > +		/* wake up the sleep thread on the Tx msg */
> > +		complete(&tx_msg->tx_complete);
> > +	}
> > +	/* wait for about 20 ticks 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))
> > +		printk(KERN_DEBUG "tx msg list is not NULL\n");
> > +
> > +}
> > +
> > +static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
> > +{
> > +	struct acpi_ipmi_device *ipmi_device =
> > +			(struct acpi_ipmi_device *)user_msg_data;
> > +	int msg_found = 0;
> > +	struct acpi_ipmi_msg *tx_msg = NULL;
> > +
> > +	if (msg->user != ipmi_device->user_interface) {
> > +		printk(KERN_DEBUG "Incorrect IPMI user\n");
> > +		ipmi_free_recv_msg(msg);
> > +		return;
> > +	}
> > +	mutex_lock(&ipmi_device->mutex_lock);
> > +	list_for_each_entry(tx_msg, &ipmi_device->tx_msg_list, head) {
> > +		if (msg->msgid == tx_msg->tx_msgid) {
> > +			/* find the message id */
> > +			msg_found = 1;
> > +			break;
> > +		}
> > +	}
> > +
> > +	mutex_unlock(&ipmi_device->mutex_lock);
> > +	if (!msg_found) {
> > +		/* no matched msg is found . But we should free it */
> > +		ipmi_free_recv_msg(msg);
> > +		printk(KERN_DEBUG "Incorrect MSG is found \n");
> > +		return;
> > +	}
> > +
> > +	if (msg->msg.data_len > 1) {
> > +		/* copy the response data to Rx_data buffer */
> > +		memcpy(tx_msg->rx_data, msg->msg_data, msg->msg.data_len);
> > +		tx_msg->rx_len = msg->msg.data_len;
> > +		tx_msg->msg_done = 1;
> > +	}
> > +	complete(&tx_msg->tx_complete);
> > +	ipmi_free_recv_msg(msg);
> > +};
> > +
> > +static void ipmi_register_bmc(int iface, struct device *dev)
> > +{
> > +	struct acpi_ipmi_device *ipmi_device;
> > +	struct acpi_device *device;
> > +	ipmi_user_t		user;
> > +	int err;
> > +
> > +	if (list_empty(&driver_data.ipmi_devices))
> > +		return;
> > +
> > +	list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
> > +		device = ipmi_device->device;
> > +		if (ipmi_device->user_interface) {
> > +			/*
> > +			 * Only one user interface is allowed to be registered
> > +			 * for one IPMI device.
> > +			 * If we already create the user interface for
> > +			 * one IPMI device, skip it
> > +			 */
> > +			continue;
> > +		}
> > +		if (dev == &device->dev) {
> > +			/*
> > +			 * If the dev is identical to the ACPI device,
> > +			 * create the user interface.
> > +			 */
> > +			err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs,
> > +						ipmi_device, &user);
> > +			if (err == 0)
> > +				ipmi_device->user_interface = user;
> > +
> > +			continue;
> > +		}
> > +		/*
> > +		 * In fact maybe the IPMI interface can be registered by
> > +		 * other methods. For example: SPMI, DMI, PCI
> > +		 * So we should also create the user interface.
> > +		 */
> > +		err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs,
> > +						ipmi_device, &user);
> > +		if (err == 0)
> > +			ipmi_device->user_interface = user;
> > +	}
> > +	return;
> > +}
> > +
> > +static void ipmi_bmc_gone(int iface)
> > +{
> > +	struct acpi_ipmi_device *ipmi_device;
> > +
> > +	if (list_empty(&driver_data.ipmi_devices))
> > +		return;
> > +
> > +	list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
> > +		if (ipmi_device->user_interface) {
> > +			ipmi_destroy_user(ipmi_device->user_interface);
> > +			ipmi_device->user_interface = NULL;
> > +			/* we should also destory tx msg list */
> > +			ipmi_destroy_tx_msg(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
> > + * by command, only write is meaningful.
> > + * @address: This contains the netfn/command of IPMI request message.
> > + * @bits   : not used.
> > + * @value  : it is an in/out parameter. It points to the IPMI message buffer.
> > + *	     Before the IPMI message is sent, it represents the actual request
> > + *	     IPMI message. After the IPMI message is finished, it represents
> > + *	     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)
> > +{
> > +	struct acpi_ipmi_msg *tx_msg = NULL;
> > +	struct acpi_ipmi_device *ipmi_device =
> > +			(struct acpi_ipmi_device *) handler_context;
> > +	int err;
> > +	acpi_status status;
> > +	/*
> > +	 * IPMI opregion message.
> > +	 * IPMI message is firstly written to the BMC and system software
> > +	 * can get the respsonse. So it is unmeaningful for the IPMI read
> > +	 * access.
> > +	 */
> > +	if ((function & ACPI_IO_MASK) == ACPI_READ) {
> > +		/* Read function is not supported. AE_TYPE is returned. */
> > +		return AE_TYPE;
> > +	}
> > +	if (!ipmi_device->user_interface) {
> > +		/* there doesn't exist user interface*/
> > +		return AE_NOT_EXIST;
> > +	}
> > +	tx_msg = acpi_alloc_ipmi_msg(ipmi_device);
> > +	if (!tx_msg) {
> > +		/* no memory is allocated */
> > +		return AE_NO_MEMORY;
> > +	}
> > +	acpi_format_ipmi_msg(tx_msg, address, value);
> > +	mutex_lock(&ipmi_device->mutex_lock);
> > +	list_add_tail(&tx_msg->head, &ipmi_device->tx_msg_list);
> > +	mutex_unlock(&ipmi_device->mutex_lock);
> > +	err = ipmi_request_settime(ipmi_device->user_interface,
> > +					&tx_msg->addr,
> > +					tx_msg->tx_msgid,
> > +					&tx_msg->tx_message,
> > +					NULL, 0, 0, 0);
> > +	if (err) {
> > +		status = AE_ERROR;
> > +		goto end_label;
> > +	}
> > +	err = wait_for_completion_timeout(&tx_msg->tx_complete, IPMI_TIMEOUT);
> > +
> > +end_label:
> > +	acpi_format_ipmi_response(tx_msg, value, err);
> > +	status = AE_OK;
> > +	mutex_lock(&ipmi_device->mutex_lock);
> > +	list_del(&tx_msg->head);
> > +	mutex_unlock(&ipmi_device->mutex_lock);
> > +	kfree(tx_msg);
> > +	return status;
> > +}
> > +
> > +static void ipmi_remove_handlers(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_handlers(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, ipmi);
> > +	if (ACPI_FAILURE(status)) {
> > +		printk(KERN_DEBUG "Can't register IPMI opregion %s\n",
> > +					acpi_device_bid(ipmi->device));
> > +		return -EINVAL;
> > +	}
> > +	set_bit(IPMI_FLAGS_HANDLER_INSTALL, &ipmi->flags);
> > +	return 0;
> > +}
> > +
> > +static int acpi_ipmi_add(struct acpi_device *device)
> > +{
> > +	struct acpi_ipmi_device *ipmi_device;
> > +	acpi_handle handle;
> > +	unsigned long long temp;
> > +	acpi_status status;
> > +	if (!device)
> > +		return -EINVAL;
> > +
> > +	handle = device->handle;
> > +	temp = 0;
> > +	status = acpi_evaluate_integer(handle, "_IFT", NULL, &temp);
> > +	if (ACPI_FAILURE(status)) {
> > +		printk(KERN_DEBUG "Incorrect _IFT object for %s\n",
> > +				acpi_device_bid(device));
> > +		return -ENODEV;
> > +	}
> > +	ipmi_device = kzalloc(sizeof(struct acpi_ipmi_device), GFP_KERNEL);
> > +	if (!ipmi_device) {
> > +		printk(KERN_DEBUG "Can't allocate memory space\n");
> > +		return -ENOMEM;
> > +	}
> > +	ipmi_device->if_type = temp;
> > +	switch (ipmi_device->if_type) {
> > +	case 1:
> > +	case 2:
> > +	case 3:
> > +		break;
> > +	default:
> > +		printk(KERN_DEBUG "Unknow IPMI:SI interface type %d\n",
> > +				ipmi_device->if_type);
> > +		kfree(ipmi_device);
> > +		return -EINVAL;
> > +	}
> > +	ipmi_device->handle = device->handle;
> > +	ipmi_device->device = device;
> > +	mutex_init(&ipmi_device->mutex_lock);
> > +	INIT_LIST_HEAD(&ipmi_device->head);
> > +	INIT_LIST_HEAD(&ipmi_device->tx_msg_list);
> > +
> > +	if (ipmi_install_handlers(ipmi_device)) {
> > +		/* can't register the IPMI opregion */
> > +		kfree(ipmi_device);
> > +		return -EINVAL;
> > +	}
> > +
> > +	/* add it to the IPMI device list */
> > +	list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices);
> > +	device->driver_data = ipmi_device;
> > +	return 0;
> > +}
> > +
> > +static int acpi_ipmi_remove(struct acpi_device *device, int type)
> > +{
> > +	struct acpi_ipmi_device *ipmi_device;
> > +
> > +	ipmi_device = acpi_driver_data(device);
> > +	if (!ipmi_device)
> > +		return 0;
> > +
> > +	if (ipmi_device->user_interface) {
> > +		/*
> > +		 * If the IPMI user interface is created, it should be
> > +		 * destroyed.
> > +		 */
> > +		ipmi_destroy_user(ipmi_device->user_interface);
> > +		ipmi_device->user_interface = NULL;
> > +	}
> > +	list_del(&ipmi_device->head);
> > +	if (!list_empty(&ipmi_device->tx_msg_list)) {
> > +		/* destroy the Tx_msg list */
> > +		ipmi_destroy_tx_msg(ipmi_device);
> > +	}
> > +	ipmi_remove_handlers(ipmi_device);
> > +	kfree(ipmi_device);
> > +	device->driver_data = NULL;
> > +	return 0;
> > +}
> > +
> > +static const struct acpi_device_id ipmi_device_ids[] = {
> > +	{"IPI0001", 0},
> > +	{"", 0},
> > +};
> > +
> > +static struct acpi_driver acpi_ipmi_driver = {
> > +	.name = "ipmi",
> > +	.class = ACPI_IPMI_CLASS,
> > +	.ids = ipmi_device_ids,
> > +	.ops = {
> > +		.add = acpi_ipmi_add,
> > +		.remove = acpi_ipmi_remove,
> > +		},
> > +};
> > +
> > +static int __init acpi_ipmi_init(void)
> > +{
> > +	int result = 0;
> > +
> > +	if (acpi_disabled)
> > +		return result;
> > +
> > +	result = acpi_bus_register_driver(&acpi_ipmi_driver);
> > +
> > +	if (result)
> > +		return result;
> > +
> > +	result = ipmi_smi_watcher_register(&driver_data.bmc_events);
> > +
> > +	if (result)
> > +		acpi_bus_unregister_driver(&acpi_ipmi_driver);
> > +
> > +	return result;
> > +}
> > +
> > +static void __exit acpi_ipmi_exit(void)
> > +{
> > +	if (acpi_disabled)
> > +		return;
> > +
> > +	ipmi_smi_watcher_unregister(&driver_data.bmc_events);
> > +	acpi_bus_unregister_driver(&acpi_ipmi_driver);
> > +
> > +	return;
> > +}
> > +
> > +module_init(acpi_ipmi_init);
> > +module_exit(acpi_ipmi_exit);
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[ykzhao]

On Wed, 2009-12-09 at 09:58 +0800, Zhao, Yakui wrote:

Hi, Lenb
    Any comment about this patch?
    It is used to add the support of ACPI IPMI opregion so that ACPI can
also communicate with the BMC controller.

Thanks.
   Yakui
> From: Zhao Yakui <yakui.zhao@intel.com>
> 
> Add the IPMI opregion driver so that the AML code can communicate with BMC
> throught IPMI message.
>      It will create IPMI user interface for every IPMI device detected
> in ACPI namespace and install the corresponding IPMI opregion space handler.
> 
> The following describes how to process the IPMI request in IPMI space handler:
>     1. format the IPMI message based on the request in AML code.
>     IPMI system address. Now the address type is SYSTEM_INTERFACE_ADDR_TYPE
>     IPMI net function & command
>     IPMI message payload
>     2. send the IPMI message by using the function of ipmi_request_settime
>     3. wait for the completion of IPMI message. It can be done in different
> routes: One is in handled in IPMI user recv callback function. Another is
> handled in timeout function.
>     4. format the IPMI response and return it to ACPI AML code.
> 
> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> ---
>  drivers/acpi/Kconfig  |   12 +
>  drivers/acpi/Makefile |    1 +
>  drivers/acpi/ipmi.c   |  583 +++++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 596 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/acpi/ipmi.c
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index 93d2c79..23cd08a 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -204,6 +204,18 @@ 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 EXPERIMENTAL
> +       default n
> +       select IPMI_HANDLER
> +       select IPMI_SI
> +       help
> +         This driver enables the ACPI to access the BMC controller. And it
> +         uses the IPMI request/response message to communicate with BMC
> +         controller, which can be found on on the server.
> +
> +         To compile this driver as a module, choose M here:
> 
>  config ACPI_HOTPLUG_CPU
>         bool
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 7702118..61077b7 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -57,6 +57,7 @@ obj-$(CONFIG_ACPI_BATTERY)    += battery.o
>  obj-$(CONFIG_ACPI_SBS)         += sbshc.o
>  obj-$(CONFIG_ACPI_SBS)         += sbs.o
>  obj-$(CONFIG_ACPI_POWER_METER) += power_meter.o
> +obj-$(CONFIG_ACPI_IPMI)                += ipmi.o
> 
>  # processor has its own "processor." module_param namespace
>  processor-y                    := processor_core.o processor_throttling.o
> diff --git a/drivers/acpi/ipmi.c b/drivers/acpi/ipmi.c
> new file mode 100644
> index 0000000..5c74936
> --- /dev/null
> +++ b/drivers/acpi/ipmi.c
> @@ -0,0 +1,583 @@
> +/*
> + *  ipmi.c - ACPI IPMI opregion
> + *
> + *  Copyright (C) 2009 Intel Corporation
> + *  Copyright (C) 2009 Zhao Yakui <yakui.zhao@intel.com>
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; either version 2 of the License, or (at
> + *  your option) any later version.
> + *
> + *  This program is distributed in the hope that it will be useful, but
> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License along
> + *  with this program; if not, write to the Free Software Foundation, Inc.,
> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/delay.h>
> +#include <linux/proc_fs.h>
> +#include <linux/seq_file.h>
> +#include <linux/interrupt.h>
> +#include <linux/list.h>
> +#include <linux/spinlock.h>
> +#include <linux/io.h>
> +#include <acpi/acpi_bus.h>
> +#include <acpi/acpi_drivers.h>
> +#include <linux/ipmi.h>
> +
> +MODULE_AUTHOR("Zhao Yakui");
> +MODULE_DESCRIPTION("ACPI IPMI Opregion driver");
> +MODULE_LICENSE("GPL");
> +
> +#define ACPI_IPMI_CLASS                        "IPMI"
> +#define ACPI_IPMI_DEVICE_NAME          "IPMI_dev"
> +#define IPMI_FLAGS_HANDLER_INSTALL     0
> +
> +#define ACPI_IPMI_OK                   0
> +#define ACPI_IPMI_TIMEOUT              0x10
> +#define ACPI_IPMI_UNKNOWN              0x07
> +/* the IPMI timeout is 30s */
> +#define IPMI_TIMEOUT                   (30 * HZ)
> +
> +
> +struct acpi_ipmi_device {
> +       acpi_handle handle;
> +       struct acpi_device *device;
> +       int if_type;
> +       /* the device list attached to driver_data.ipmi_devices */
> +       struct list_head head;
> +       ipmi_user_t     user_interface;
> +       struct mutex    mutex_lock;
> +       /* the IPMI request message list */
> +       struct list_head tx_msg_list;
> +       long curr_msgid;
> +       /* IPMI flags */
> +       unsigned long flags;
> +};
> +
> +struct ipmi_driver_data {
> +       int device_count;
> +       struct list_head        ipmi_devices;
> +       struct ipmi_smi_watcher bmc_events;
> +       struct ipmi_user_hndl   ipmi_hndlrs;
> +};
> +
> +struct acpi_ipmi_msg {
> +       /* message list */
> +       struct list_head head;
> +       /*
> +        * General speaking the addr type should be SI_ADDR_TYPE. And
> +        * the addr channel should be BMC.
> +        * In fact it can also be IPMB type. But we will have to
> +        * parse it from the Netfn command buffer. It is so complex
> +        * that it is skipped.
> +        */
> +       struct ipmi_addr addr;
> +       /* tx message id */
> +       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;
> +       /* tx data . And copy it from ACPI object buffer */
> +       u8      tx_data[64];
> +       int     tx_len;
> +       /* get the response data */
> +       u8      rx_data[64];
> +       /* the response length. The netfn & cmd is excluded. */
> +       int     rx_len;
> +       struct acpi_ipmi_device *device;
> +};
> +
> +/*
> + * IPMI request/response buffer.
> + * The length is 66 bytes.
> + */
> +struct acpi_ipmi_buffer {
> +       /* status code of a given IPMI command */
> +       u8 status_code;
> +       /* the length of the payload */
> +       u8 length;
> +       /*
> +        * the payload. Before the operation is carried out, it represents the
> +        * request message payload. After the opration is carried out, it
> +        * stores the response message returned by IPMI command.
> +        */
> +       u8 data[64];
> +};
> +
> +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 struct ipmi_driver_data driver_data = {
> +       .ipmi_devices = LIST_HEAD_INIT(driver_data.ipmi_devices),
> +       .bmc_events = {
> +               .owner = THIS_MODULE,
> +               .new_smi = ipmi_register_bmc,
> +               .smi_gone = ipmi_bmc_gone,
> +       },
> +       .ipmi_hndlrs = {
> +               .ipmi_recv_hndl = ipmi_msg_handler,
> +       },
> +};
> +
> +static
> +struct acpi_ipmi_msg *acpi_alloc_ipmi_msg(struct acpi_ipmi_device *ipmi)
> +{
> +       struct acpi_ipmi_msg *ipmi_msg;
> +
> +       ipmi_msg = kzalloc(sizeof(struct acpi_ipmi_msg), GFP_KERNEL);
> +       if (!ipmi_msg)  {
> +               printk(KERN_DEBUG "Can't allocate memory for ipmi_msg\n");
> +               return NULL;
> +       }
> +       init_completion(&ipmi_msg->tx_complete);
> +       INIT_LIST_HEAD(&ipmi_msg->head);
> +       ipmi_msg->device = ipmi;
> +       return ipmi_msg;
> +}
> +
> +static void acpi_format_ipmi_msg(struct acpi_ipmi_msg *tx_msg,
> +               acpi_physical_address address,
> +               acpi_integer *value)
> +{
> +       struct kernel_ipmi_msg *msg;
> +       u8      temp_value;
> +       struct acpi_ipmi_buffer *buffer;
> +       struct acpi_ipmi_device *device;
> +
> +       msg = &tx_msg->tx_message;
> +       /* get the netfn */
> +       temp_value = (address >> 8) & 0xff;
> +       msg->netfn = temp_value;
> +       /* get the command */
> +       temp_value = address & 0xff;
> +       msg->cmd = temp_value;
> +       msg->data = tx_msg->tx_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 */
> +       msg->data_len = buffer->length;
> +       memcpy(tx_msg->tx_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,
> +        * the addr type should be changed to IPMB.
> +        */
> +       tx_msg->addr.addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE;
> +       tx_msg->addr.channel = IPMI_BMC_CHANNEL;
> +       tx_msg->addr.data[0] = 0;
> +
> +       /*
> +        * If the netfn is APP_REQUEST and the cmd is SEND_MESSAGE, we should
> +        * parse the IPMI request message buffer to get the IPMB address.
> +        * If so, please fix me.
> +        */
> +
> +       /* Get the msgid */
> +       device = tx_msg->device;
> +       mutex_lock(&device->mutex_lock);
> +       device->curr_msgid++;
> +       tx_msg->tx_msgid = device->curr_msgid;
> +       mutex_unlock(&device->mutex_lock);
> +}
> +
> +static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg,
> +               acpi_integer *value, int timeout)
> +{
> +       struct acpi_ipmi_buffer *buffer;
> +
> +       /*
> +        * value is also used as output parameter. It represents the response
> +        * IPMI message returned by IPMI command.
> +        */
> +       buffer = (struct acpi_ipmi_buffer *)value;
> +       /* when timeout is zero, it means that the timeout happens */
> +       if (!timeout) {
> +               /* the status code is ACPI_IPMI_TIMEOUT */
> +               buffer->status_code = ACPI_IPMI_TIMEOUT;
> +               return;
> +       }
> +       /*
> +        * If the flag of msg_done is not set, it means that the IPMI command
> +        * is not executed correctly.
> +        * The status code will be ACPI_IPMI_UNKNOWN.
> +        */
> +       if (!msg->msg_done) {
> +               buffer->status_code = ACPI_IPMI_UNKNOWN;
> +               return;
> +       }
> +       /*
> +        * If the IPMI response message is obtained correctly, the status code
> +        * will be ACPI_IPMI_OK
> +        */
> +       buffer->status_code = ACPI_IPMI_OK;
> +       buffer->length = msg->rx_len;
> +       memcpy(buffer->data, msg->rx_data, msg->rx_len);
> +       return;
> +}
> +static void ipmi_destroy_tx_msg(struct acpi_ipmi_device *ipmi)
> +{
> +       struct acpi_ipmi_msg *tx_msg = NULL;
> +       int count = 20;
> +
> +       list_for_each_entry(tx_msg, &ipmi->tx_msg_list, head) {
> +               /* wake up the sleep thread on the Tx msg */
> +               complete(&tx_msg->tx_complete);
> +       }
> +       /* wait for about 20 ticks 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))
> +               printk(KERN_DEBUG "tx msg list is not NULL\n");
> +
> +}
> +
> +static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
> +{
> +       struct acpi_ipmi_device *ipmi_device =
> +                       (struct acpi_ipmi_device *)user_msg_data;
> +       int msg_found = 0;
> +       struct acpi_ipmi_msg *tx_msg = NULL;
> +
> +       if (msg->user != ipmi_device->user_interface) {
> +               printk(KERN_DEBUG "Incorrect IPMI user\n");
> +               ipmi_free_recv_msg(msg);
> +               return;
> +       }
> +       mutex_lock(&ipmi_device->mutex_lock);
> +       list_for_each_entry(tx_msg, &ipmi_device->tx_msg_list, head) {
> +               if (msg->msgid == tx_msg->tx_msgid) {
> +                       /* find the message id */
> +                       msg_found = 1;
> +                       break;
> +               }
> +       }
> +
> +       mutex_unlock(&ipmi_device->mutex_lock);
> +       if (!msg_found) {
> +               /* no matched msg is found . But we should free it */
> +               ipmi_free_recv_msg(msg);
> +               printk(KERN_DEBUG "Incorrect MSG is found \n");
> +               return;
> +       }
> +
> +       if (msg->msg.data_len > 1) {
> +               /* copy the response data to Rx_data buffer */
> +               memcpy(tx_msg->rx_data, msg->msg_data, msg->msg.data_len);
> +               tx_msg->rx_len = msg->msg.data_len;
> +               tx_msg->msg_done = 1;
> +       }
> +       complete(&tx_msg->tx_complete);
> +       ipmi_free_recv_msg(msg);
> +};
> +
> +static void ipmi_register_bmc(int iface, struct device *dev)
> +{
> +       struct acpi_ipmi_device *ipmi_device;
> +       struct acpi_device *device;
> +       ipmi_user_t             user;
> +       int err;
> +
> +       if (list_empty(&driver_data.ipmi_devices))
> +               return;
> +
> +       list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
> +               device = ipmi_device->device;
> +               if (ipmi_device->user_interface) {
> +                       /*
> +                        * Only one user interface is allowed to be registered
> +                        * for one IPMI device.
> +                        * If we already create the user interface for
> +                        * one IPMI device, skip it
> +                        */
> +                       continue;
> +               }
> +               if (dev == &device->dev) {
> +                       /*
> +                        * If the dev is identical to the ACPI device,
> +                        * create the user interface.
> +                        */
> +                       err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs,
> +                                               ipmi_device, &user);
> +                       if (err == 0)
> +                               ipmi_device->user_interface = user;
> +
> +                       continue;
> +               }
> +               /*
> +                * In fact maybe the IPMI interface can be registered by
> +                * other methods. For example: SPMI, DMI, PCI
> +                * So we should also create the user interface.
> +                */
> +               err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs,
> +                                               ipmi_device, &user);
> +               if (err == 0)
> +                       ipmi_device->user_interface = user;
> +       }
> +       return;
> +}
> +
> +static void ipmi_bmc_gone(int iface)
> +{
> +       struct acpi_ipmi_device *ipmi_device;
> +
> +       if (list_empty(&driver_data.ipmi_devices))
> +               return;
> +
> +       list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
> +               if (ipmi_device->user_interface) {
> +                       ipmi_destroy_user(ipmi_device->user_interface);
> +                       ipmi_device->user_interface = NULL;
> +                       /* we should also destory tx msg list */
> +                       ipmi_destroy_tx_msg(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
> + * by command, only write is meaningful.
> + * @address: This contains the netfn/command of IPMI request message.
> + * @bits   : not used.
> + * @value  : it is an in/out parameter. It points to the IPMI message buffer.
> + *          Before the IPMI message is sent, it represents the actual request
> + *          IPMI message. After the IPMI message is finished, it represents
> + *          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)
> +{
> +       struct acpi_ipmi_msg *tx_msg = NULL;
> +       struct acpi_ipmi_device *ipmi_device =
> +                       (struct acpi_ipmi_device *) handler_context;
> +       int err;
> +       acpi_status status;
> +       /*
> +        * IPMI opregion message.
> +        * IPMI message is firstly written to the BMC and system software
> +        * can get the respsonse. So it is unmeaningful for the IPMI read
> +        * access.
> +        */
> +       if ((function & ACPI_IO_MASK) == ACPI_READ) {
> +               /* Read function is not supported. AE_TYPE is returned. */
> +               return AE_TYPE;
> +       }
> +       if (!ipmi_device->user_interface) {
> +               /* there doesn't exist user interface*/
> +               return AE_NOT_EXIST;
> +       }
> +       tx_msg = acpi_alloc_ipmi_msg(ipmi_device);
> +       if (!tx_msg) {
> +               /* no memory is allocated */
> +               return AE_NO_MEMORY;
> +       }
> +       acpi_format_ipmi_msg(tx_msg, address, value);
> +       mutex_lock(&ipmi_device->mutex_lock);
> +       list_add_tail(&tx_msg->head, &ipmi_device->tx_msg_list);
> +       mutex_unlock(&ipmi_device->mutex_lock);
> +       err = ipmi_request_settime(ipmi_device->user_interface,
> +                                       &tx_msg->addr,
> +                                       tx_msg->tx_msgid,
> +                                       &tx_msg->tx_message,
> +                                       NULL, 0, 0, 0);
> +       if (err) {
> +               status = AE_ERROR;
> +               goto end_label;
> +       }
> +       err = wait_for_completion_timeout(&tx_msg->tx_complete, IPMI_TIMEOUT);
> +
> +end_label:
> +       acpi_format_ipmi_response(tx_msg, value, err);
> +       status = AE_OK;
> +       mutex_lock(&ipmi_device->mutex_lock);
> +       list_del(&tx_msg->head);
> +       mutex_unlock(&ipmi_device->mutex_lock);
> +       kfree(tx_msg);
> +       return status;
> +}
> +
> +static void ipmi_remove_handlers(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_handlers(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, ipmi);
> +       if (ACPI_FAILURE(status)) {
> +               printk(KERN_DEBUG "Can't register IPMI opregion %s\n",
> +                                       acpi_device_bid(ipmi->device));
> +               return -EINVAL;
> +       }
> +       set_bit(IPMI_FLAGS_HANDLER_INSTALL, &ipmi->flags);
> +       return 0;
> +}
> +
> +static int acpi_ipmi_add(struct acpi_device *device)
> +{
> +       struct acpi_ipmi_device *ipmi_device;
> +       acpi_handle handle;
> +       unsigned long long temp;
> +       acpi_status status;
> +       if (!device)
> +               return -EINVAL;
> +
> +       handle = device->handle;
> +       temp = 0;
> +       status = acpi_evaluate_integer(handle, "_IFT", NULL, &temp);
> +       if (ACPI_FAILURE(status)) {
> +               printk(KERN_DEBUG "Incorrect _IFT object for %s\n",
> +                               acpi_device_bid(device));
> +               return -ENODEV;
> +       }
> +       ipmi_device = kzalloc(sizeof(struct acpi_ipmi_device), GFP_KERNEL);
> +       if (!ipmi_device) {
> +               printk(KERN_DEBUG "Can't allocate memory space\n");
> +               return -ENOMEM;
> +       }
> +       ipmi_device->if_type = temp;
> +       switch (ipmi_device->if_type) {
> +       case 1:
> +       case 2:
> +       case 3:
> +               break;
> +       default:
> +               printk(KERN_DEBUG "Unknow IPMI:SI interface type %d\n",
> +                               ipmi_device->if_type);
> +               kfree(ipmi_device);
> +               return -EINVAL;
> +       }
> +       ipmi_device->handle = device->handle;
> +       ipmi_device->device = device;
> +       mutex_init(&ipmi_device->mutex_lock);
> +       INIT_LIST_HEAD(&ipmi_device->head);
> +       INIT_LIST_HEAD(&ipmi_device->tx_msg_list);
> +
> +       if (ipmi_install_handlers(ipmi_device)) {
> +               /* can't register the IPMI opregion */
> +               kfree(ipmi_device);
> +               return -EINVAL;
> +       }
> +
> +       /* add it to the IPMI device list */
> +       list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices);
> +       device->driver_data = ipmi_device;
> +       return 0;
> +}
> +
> +static int acpi_ipmi_remove(struct acpi_device *device, int type)
> +{
> +       struct acpi_ipmi_device *ipmi_device;
> +
> +       ipmi_device = acpi_driver_data(device);
> +       if (!ipmi_device)
> +               return 0;
> +
> +       if (ipmi_device->user_interface) {
> +               /*
> +                * If the IPMI user interface is created, it should be
> +                * destroyed.
> +                */
> +               ipmi_destroy_user(ipmi_device->user_interface);
> +               ipmi_device->user_interface = NULL;
> +       }
> +       list_del(&ipmi_device->head);
> +       if (!list_empty(&ipmi_device->tx_msg_list)) {
> +               /* destroy the Tx_msg list */
> +               ipmi_destroy_tx_msg(ipmi_device);
> +       }
> +       ipmi_remove_handlers(ipmi_device);
> +       kfree(ipmi_device);
> +       device->driver_data = NULL;
> +       return 0;
> +}
> +
> +static const struct acpi_device_id ipmi_device_ids[] = {
> +       {"IPI0001", 0},
> +       {"", 0},
> +};
> +
> +static struct acpi_driver acpi_ipmi_driver = {
> +       .name = "ipmi",
> +       .class = ACPI_IPMI_CLASS,
> +       .ids = ipmi_device_ids,
> +       .ops = {
> +               .add = acpi_ipmi_add,
> +               .remove = acpi_ipmi_remove,
> +               },
> +};
> +
> +static int __init acpi_ipmi_init(void)
> +{
> +       int result = 0;
> +
> +       if (acpi_disabled)
> +               return result;
> +
> +       result = acpi_bus_register_driver(&acpi_ipmi_driver);
> +
> +       if (result)
> +               return result;
> +
> +       result = ipmi_smi_watcher_register(&driver_data.bmc_events);
> +
> +       if (result)
> +               acpi_bus_unregister_driver(&acpi_ipmi_driver);
> +
> +       return result;
> +}
> +
> +static void __exit acpi_ipmi_exit(void)
> +{
> +       if (acpi_disabled)
> +               return;
> +
> +       ipmi_smi_watcher_unregister(&driver_data.bmc_events);
> +       acpi_bus_unregister_driver(&acpi_ipmi_driver);
> +
> +       return;
> +}
> +
> +module_init(acpi_ipmi_init);
> +module_exit(acpi_ipmi_exit);
> --
> 1.5.4.5
> 

Re: [Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[Corey Minyard]

Len Brown wrote:
> On Wed, 9 Dec 2009, Bjorn Helgaas wrote:
>
>   
>> On Tuesday 08 December 2009 06:58:45 pm yakui.zhao@intel.com wrote:
>>     
>>> From: Zhao Yakui <yakui.zhao@intel.com>
>>>
>>> Add the IPMI opregion driver so that the AML code can communicate with BMC
>>> throught IPMI message.
>>>      It will create IPMI user interface for every IPMI device detected
>>> in ACPI namespace and install the corresponding IPMI opregion space handler.
>>>
>>> The following describes how to process the IPMI request in IPMI space handler:
>>>     1. format the IPMI message based on the request in AML code.
>>>     IPMI system address. Now the address type is SYSTEM_INTERFACE_ADDR_TYPE
>>>     IPMI net function & command
>>>     IPMI message payload
>>>     2. send the IPMI message by using the function of ipmi_request_settime
>>>     3. wait for the completion of IPMI message. It can be done in different
>>> routes: One is in handled in IPMI user recv callback function. Another is
>>> handled in timeout function.
>>>     4. format the IPMI response and return it to ACPI AML code.
>>>       
>> I think this should be part of the ipmi_si_intf module, not a separate
>> module.  This opregion code is of no use without ipmi_si_intf.  The
>> opregion code can still be under its own config option, but I don't
>> see the value in having it be a separate module.
>>     
>
>
> I agree.  Corey?
>   
Yes, that is a good idea.

-corey

> thanks,
> -Len Brown, Intel Open Source technology Center
>
>   
>> The opregion code should definitely not use acpi_bus_register_driver()
>> to claim IPI0001 devices.  The ipmi_si_intf driver already uses
>> pnp_register_driver() to claim IPI0001 devices.  The fact that Linux
>> allows two drivers to claim the same device (one via pnp_register_driver()
>> and the other via acpi_bus_register_driver()) is a defect in Linux, and
>> we shouldn't take advantage of that.
>>
>> Bjorn
>>
>>     
>>> Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
>>> ---
>>>  drivers/acpi/Kconfig  |   12 +
>>>  drivers/acpi/Makefile |    1 +
>>>  drivers/acpi/ipmi.c   |  583 +++++++++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 596 insertions(+), 0 deletions(-)
>>>  create mode 100644 drivers/acpi/ipmi.c
>>>
>>> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
>>> index 93d2c79..23cd08a 100644
>>> --- a/drivers/acpi/Kconfig
>>> +++ b/drivers/acpi/Kconfig
>>> @@ -204,6 +204,18 @@ 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 EXPERIMENTAL
>>> +	default n
>>> +	select IPMI_HANDLER
>>> +	select IPMI_SI
>>> +	help
>>> +	  This driver enables the ACPI to access the BMC controller. And it
>>> +	  uses the IPMI request/response message to communicate with BMC
>>> +	  controller, which can be found on on the server.
>>> +
>>> +	  To compile this driver as a module, choose M here:
>>>  
>>>  config ACPI_HOTPLUG_CPU
>>>  	bool
>>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>>> index 7702118..61077b7 100644
>>> --- a/drivers/acpi/Makefile
>>> +++ b/drivers/acpi/Makefile
>>> @@ -57,6 +57,7 @@ obj-$(CONFIG_ACPI_BATTERY)	+= battery.o
>>>  obj-$(CONFIG_ACPI_SBS)		+= sbshc.o
>>>  obj-$(CONFIG_ACPI_SBS)		+= sbs.o
>>>  obj-$(CONFIG_ACPI_POWER_METER)	+= power_meter.o
>>> +obj-$(CONFIG_ACPI_IPMI)		+= ipmi.o
>>>  
>>>  # processor has its own "processor." module_param namespace
>>>  processor-y			:= processor_core.o processor_throttling.o
>>> diff --git a/drivers/acpi/ipmi.c b/drivers/acpi/ipmi.c
>>> new file mode 100644
>>> index 0000000..5c74936
>>> --- /dev/null
>>> +++ b/drivers/acpi/ipmi.c
>>> @@ -0,0 +1,583 @@
>>> +/*
>>> + *  ipmi.c - ACPI IPMI opregion
>>> + *
>>> + *  Copyright (C) 2009 Intel Corporation
>>> + *  Copyright (C) 2009 Zhao Yakui <yakui.zhao@intel.com>
>>> + *
>>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> + *
>>> + *  This program is free software; you can redistribute it and/or modify
>>> + *  it under the terms of the GNU General Public License as published by
>>> + *  the Free Software Foundation; either version 2 of the License, or (at
>>> + *  your option) any later version.
>>> + *
>>> + *  This program is distributed in the hope that it will be useful, but
>>> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
>>> + *  General Public License for more details.
>>> + *
>>> + *  You should have received a copy of the GNU General Public License along
>>> + *  with this program; if not, write to the Free Software Foundation, Inc.,
>>> + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
>>> + *
>>> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> + */
>>> +
>>> +#include <linux/kernel.h>
>>> +#include <linux/module.h>
>>> +#include <linux/init.h>
>>> +#include <linux/types.h>
>>> +#include <linux/delay.h>
>>> +#include <linux/proc_fs.h>
>>> +#include <linux/seq_file.h>
>>> +#include <linux/interrupt.h>
>>> +#include <linux/list.h>
>>> +#include <linux/spinlock.h>
>>> +#include <linux/io.h>
>>> +#include <acpi/acpi_bus.h>
>>> +#include <acpi/acpi_drivers.h>
>>> +#include <linux/ipmi.h>
>>> +
>>> +MODULE_AUTHOR("Zhao Yakui");
>>> +MODULE_DESCRIPTION("ACPI IPMI Opregion driver");
>>> +MODULE_LICENSE("GPL");
>>> +
>>> +#define ACPI_IPMI_CLASS			"IPMI"
>>> +#define ACPI_IPMI_DEVICE_NAME		"IPMI_dev"
>>> +#define IPMI_FLAGS_HANDLER_INSTALL	0
>>> +
>>> +#define ACPI_IPMI_OK			0
>>> +#define ACPI_IPMI_TIMEOUT		0x10
>>> +#define ACPI_IPMI_UNKNOWN		0x07
>>> +/* the IPMI timeout is 30s */
>>> +#define IPMI_TIMEOUT			(30 * HZ)
>>> +
>>> +
>>> +struct acpi_ipmi_device {
>>> +	acpi_handle handle;
>>> +	struct acpi_device *device;
>>> +	int if_type;
>>> +	/* the device list attached to driver_data.ipmi_devices */
>>> +	struct list_head head;
>>> +	ipmi_user_t 	user_interface;
>>> +	struct mutex	mutex_lock;
>>> +	/* the IPMI request message list */
>>> +	struct list_head tx_msg_list;
>>> +	long curr_msgid;
>>> +	/* IPMI flags */
>>> +	unsigned long flags;
>>> +};
>>> +
>>> +struct ipmi_driver_data {
>>> +	int device_count;
>>> +	struct list_head	ipmi_devices;
>>> +	struct ipmi_smi_watcher	bmc_events;
>>> +	struct ipmi_user_hndl	ipmi_hndlrs;
>>> +};
>>> +
>>> +struct acpi_ipmi_msg {
>>> +	/* message list */
>>> +	struct list_head head;
>>> +	/*
>>> +	 * General speaking the addr type should be SI_ADDR_TYPE. And
>>> +	 * the addr channel should be BMC.
>>> +	 * In fact it can also be IPMB type. But we will have to
>>> +	 * parse it from the Netfn command buffer. It is so complex
>>> +	 * that it is skipped.
>>> +	 */
>>> +	struct ipmi_addr addr;
>>> +	/* tx message id */
>>> +	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;
>>> +	/* tx data . And copy it from ACPI object buffer */
>>> +	u8	tx_data[64];
>>> +	int	tx_len;
>>> +	/* get the response data */
>>> +	u8	rx_data[64];
>>> +	/* the response length. The netfn & cmd is excluded. */
>>> +	int	rx_len;
>>> +	struct acpi_ipmi_device *device;
>>> +};
>>> +
>>> +/*
>>> + * IPMI request/response buffer.
>>> + * The length is 66 bytes.
>>> + */
>>> +struct acpi_ipmi_buffer {
>>> +	/* status code of a given IPMI command */
>>> +	u8 status_code;
>>> +	/* the length of the payload */
>>> +	u8 length;
>>> +	/*
>>> +	 * the payload. Before the operation is carried out, it represents the
>>> +	 * request message payload. After the opration is carried out, it
>>> +	 * stores the response message returned by IPMI command.
>>> +	 */
>>> +	u8 data[64];
>>> +};
>>> +
>>> +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 struct ipmi_driver_data driver_data = {
>>> +	.ipmi_devices = LIST_HEAD_INIT(driver_data.ipmi_devices),
>>> +	.bmc_events = {
>>> +		.owner = THIS_MODULE,
>>> +		.new_smi = ipmi_register_bmc,
>>> +		.smi_gone = ipmi_bmc_gone,
>>> +	},
>>> +	.ipmi_hndlrs = {
>>> +		.ipmi_recv_hndl = ipmi_msg_handler,
>>> +	},
>>> +};
>>> +
>>> +static
>>> +struct acpi_ipmi_msg *acpi_alloc_ipmi_msg(struct acpi_ipmi_device *ipmi)
>>> +{
>>> +	struct acpi_ipmi_msg *ipmi_msg;
>>> +
>>> +	ipmi_msg = kzalloc(sizeof(struct acpi_ipmi_msg), GFP_KERNEL);
>>> +	if (!ipmi_msg)	{
>>> +		printk(KERN_DEBUG "Can't allocate memory for ipmi_msg\n");
>>> +		return NULL;
>>> +	}
>>> +	init_completion(&ipmi_msg->tx_complete);
>>> +	INIT_LIST_HEAD(&ipmi_msg->head);
>>> +	ipmi_msg->device = ipmi;
>>> +	return ipmi_msg;
>>> +}
>>> +
>>> +static void acpi_format_ipmi_msg(struct acpi_ipmi_msg *tx_msg,
>>> +		acpi_physical_address address,
>>> +		acpi_integer *value)
>>> +{
>>> +	struct kernel_ipmi_msg *msg;
>>> +	u8	temp_value;
>>> +	struct acpi_ipmi_buffer *buffer;
>>> +	struct acpi_ipmi_device *device;
>>> +
>>> +	msg = &tx_msg->tx_message;
>>> +	/* get the netfn */
>>> +	temp_value = (address >> 8) & 0xff;
>>> +	msg->netfn = temp_value;
>>> +	/* get the command */
>>> +	temp_value = address & 0xff;
>>> +	msg->cmd = temp_value;
>>> +	msg->data = tx_msg->tx_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 */
>>> +	msg->data_len = buffer->length;
>>> +	memcpy(tx_msg->tx_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,
>>> +	 * the addr type should be changed to IPMB.
>>> +	 */
>>> +	tx_msg->addr.addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE;
>>> +	tx_msg->addr.channel = IPMI_BMC_CHANNEL;
>>> +	tx_msg->addr.data[0] = 0;
>>> +
>>> +	/*
>>> +	 * If the netfn is APP_REQUEST and the cmd is SEND_MESSAGE, we should
>>> +	 * parse the IPMI request message buffer to get the IPMB address.
>>> +	 * If so, please fix me.
>>> +	 */
>>> +
>>> +	/* Get the msgid */
>>> +	device = tx_msg->device;
>>> +	mutex_lock(&device->mutex_lock);
>>> +	device->curr_msgid++;
>>> +	tx_msg->tx_msgid = device->curr_msgid;
>>> +	mutex_unlock(&device->mutex_lock);
>>> +}
>>> +
>>> +static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg,
>>> +		acpi_integer *value, int timeout)
>>> +{
>>> +	struct acpi_ipmi_buffer *buffer;
>>> +
>>> +	/*
>>> +	 * value is also used as output parameter. It represents the response
>>> +	 * IPMI message returned by IPMI command.
>>> +	 */
>>> +	buffer = (struct acpi_ipmi_buffer *)value;
>>> +	/* when timeout is zero, it means that the timeout happens */
>>> +	if (!timeout) {
>>> +		/* the status code is ACPI_IPMI_TIMEOUT */
>>> +		buffer->status_code = ACPI_IPMI_TIMEOUT;
>>> +		return;
>>> +	}
>>> +	/*
>>> +	 * If the flag of msg_done is not set, it means that the IPMI command
>>> +	 * is not executed correctly.
>>> +	 * The status code will be ACPI_IPMI_UNKNOWN.
>>> +	 */
>>> +	if (!msg->msg_done) {
>>> +		buffer->status_code = ACPI_IPMI_UNKNOWN;
>>> +		return;
>>> +	}
>>> +	/*
>>> +	 * If the IPMI response message is obtained correctly, the status code
>>> +	 * will be ACPI_IPMI_OK
>>> +	 */
>>> +	buffer->status_code = ACPI_IPMI_OK;
>>> +	buffer->length = msg->rx_len;
>>> +	memcpy(buffer->data, msg->rx_data, msg->rx_len);
>>> +	return;
>>> +}
>>> +static void ipmi_destroy_tx_msg(struct acpi_ipmi_device *ipmi)
>>> +{
>>> +	struct acpi_ipmi_msg *tx_msg = NULL;
>>> +	int count = 20;
>>> +
>>> +	list_for_each_entry(tx_msg, &ipmi->tx_msg_list, head) {
>>> +		/* wake up the sleep thread on the Tx msg */
>>> +		complete(&tx_msg->tx_complete);
>>> +	}
>>> +	/* wait for about 20 ticks 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))
>>> +		printk(KERN_DEBUG "tx msg list is not NULL\n");
>>> +
>>> +}
>>> +
>>> +static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
>>> +{
>>> +	struct acpi_ipmi_device *ipmi_device =
>>> +			(struct acpi_ipmi_device *)user_msg_data;
>>> +	int msg_found = 0;
>>> +	struct acpi_ipmi_msg *tx_msg = NULL;
>>> +
>>> +	if (msg->user != ipmi_device->user_interface) {
>>> +		printk(KERN_DEBUG "Incorrect IPMI user\n");
>>> +		ipmi_free_recv_msg(msg);
>>> +		return;
>>> +	}
>>> +	mutex_lock(&ipmi_device->mutex_lock);
>>> +	list_for_each_entry(tx_msg, &ipmi_device->tx_msg_list, head) {
>>> +		if (msg->msgid == tx_msg->tx_msgid) {
>>> +			/* find the message id */
>>> +			msg_found = 1;
>>> +			break;
>>> +		}
>>> +	}
>>> +
>>> +	mutex_unlock(&ipmi_device->mutex_lock);
>>> +	if (!msg_found) {
>>> +		/* no matched msg is found . But we should free it */
>>> +		ipmi_free_recv_msg(msg);
>>> +		printk(KERN_DEBUG "Incorrect MSG is found \n");
>>> +		return;
>>> +	}
>>> +
>>> +	if (msg->msg.data_len > 1) {
>>> +		/* copy the response data to Rx_data buffer */
>>> +		memcpy(tx_msg->rx_data, msg->msg_data, msg->msg.data_len);
>>> +		tx_msg->rx_len = msg->msg.data_len;
>>> +		tx_msg->msg_done = 1;
>>> +	}
>>> +	complete(&tx_msg->tx_complete);
>>> +	ipmi_free_recv_msg(msg);
>>> +};
>>> +
>>> +static void ipmi_register_bmc(int iface, struct device *dev)
>>> +{
>>> +	struct acpi_ipmi_device *ipmi_device;
>>> +	struct acpi_device *device;
>>> +	ipmi_user_t		user;
>>> +	int err;
>>> +
>>> +	if (list_empty(&driver_data.ipmi_devices))
>>> +		return;
>>> +
>>> +	list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
>>> +		device = ipmi_device->device;
>>> +		if (ipmi_device->user_interface) {
>>> +			/*
>>> +			 * Only one user interface is allowed to be registered
>>> +			 * for one IPMI device.
>>> +			 * If we already create the user interface for
>>> +			 * one IPMI device, skip it
>>> +			 */
>>> +			continue;
>>> +		}
>>> +		if (dev == &device->dev) {
>>> +			/*
>>> +			 * If the dev is identical to the ACPI device,
>>> +			 * create the user interface.
>>> +			 */
>>> +			err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs,
>>> +						ipmi_device, &user);
>>> +			if (err == 0)
>>> +				ipmi_device->user_interface = user;
>>> +
>>> +			continue;
>>> +		}
>>> +		/*
>>> +		 * In fact maybe the IPMI interface can be registered by
>>> +		 * other methods. For example: SPMI, DMI, PCI
>>> +		 * So we should also create the user interface.
>>> +		 */
>>> +		err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs,
>>> +						ipmi_device, &user);
>>> +		if (err == 0)
>>> +			ipmi_device->user_interface = user;
>>> +	}
>>> +	return;
>>> +}
>>> +
>>> +static void ipmi_bmc_gone(int iface)
>>> +{
>>> +	struct acpi_ipmi_device *ipmi_device;
>>> +
>>> +	if (list_empty(&driver_data.ipmi_devices))
>>> +		return;
>>> +
>>> +	list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
>>> +		if (ipmi_device->user_interface) {
>>> +			ipmi_destroy_user(ipmi_device->user_interface);
>>> +			ipmi_device->user_interface = NULL;
>>> +			/* we should also destory tx msg list */
>>> +			ipmi_destroy_tx_msg(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
>>> + * by command, only write is meaningful.
>>> + * @address: This contains the netfn/command of IPMI request message.
>>> + * @bits   : not used.
>>> + * @value  : it is an in/out parameter. It points to the IPMI message buffer.
>>> + *	     Before the IPMI message is sent, it represents the actual request
>>> + *	     IPMI message. After the IPMI message is finished, it represents
>>> + *	     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)
>>> +{
>>> +	struct acpi_ipmi_msg *tx_msg = NULL;
>>> +	struct acpi_ipmi_device *ipmi_device =
>>> +			(struct acpi_ipmi_device *) handler_context;
>>> +	int err;
>>> +	acpi_status status;
>>> +	/*
>>> +	 * IPMI opregion message.
>>> +	 * IPMI message is firstly written to the BMC and system software
>>> +	 * can get the respsonse. So it is unmeaningful for the IPMI read
>>> +	 * access.
>>> +	 */
>>> +	if ((function & ACPI_IO_MASK) == ACPI_READ) {
>>> +		/* Read function is not supported. AE_TYPE is returned. */
>>> +		return AE_TYPE;
>>> +	}
>>> +	if (!ipmi_device->user_interface) {
>>> +		/* there doesn't exist user interface*/
>>> +		return AE_NOT_EXIST;
>>> +	}
>>> +	tx_msg = acpi_alloc_ipmi_msg(ipmi_device);
>>> +	if (!tx_msg) {
>>> +		/* no memory is allocated */
>>> +		return AE_NO_MEMORY;
>>> +	}
>>> +	acpi_format_ipmi_msg(tx_msg, address, value);
>>> +	mutex_lock(&ipmi_device->mutex_lock);
>>> +	list_add_tail(&tx_msg->head, &ipmi_device->tx_msg_list);
>>> +	mutex_unlock(&ipmi_device->mutex_lock);
>>> +	err = ipmi_request_settime(ipmi_device->user_interface,
>>> +					&tx_msg->addr,
>>> +					tx_msg->tx_msgid,
>>> +					&tx_msg->tx_message,
>>> +					NULL, 0, 0, 0);
>>> +	if (err) {
>>> +		status = AE_ERROR;
>>> +		goto end_label;
>>> +	}
>>> +	err = wait_for_completion_timeout(&tx_msg->tx_complete, IPMI_TIMEOUT);
>>> +
>>> +end_label:
>>> +	acpi_format_ipmi_response(tx_msg, value, err);
>>> +	status = AE_OK;
>>> +	mutex_lock(&ipmi_device->mutex_lock);
>>> +	list_del(&tx_msg->head);
>>> +	mutex_unlock(&ipmi_device->mutex_lock);
>>> +	kfree(tx_msg);
>>> +	return status;
>>> +}
>>> +
>>> +static void ipmi_remove_handlers(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_handlers(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, ipmi);
>>> +	if (ACPI_FAILURE(status)) {
>>> +		printk(KERN_DEBUG "Can't register IPMI opregion %s\n",
>>> +					acpi_device_bid(ipmi->device));
>>> +		return -EINVAL;
>>> +	}
>>> +	set_bit(IPMI_FLAGS_HANDLER_INSTALL, &ipmi->flags);
>>> +	return 0;
>>> +}
>>> +
>>> +static int acpi_ipmi_add(struct acpi_device *device)
>>> +{
>>> +	struct acpi_ipmi_device *ipmi_device;
>>> +	acpi_handle handle;
>>> +	unsigned long long temp;
>>> +	acpi_status status;
>>> +	if (!device)
>>> +		return -EINVAL;
>>> +
>>> +	handle = device->handle;
>>> +	temp = 0;
>>> +	status = acpi_evaluate_integer(handle, "_IFT", NULL, &temp);
>>> +	if (ACPI_FAILURE(status)) {
>>> +		printk(KERN_DEBUG "Incorrect _IFT object for %s\n",
>>> +				acpi_device_bid(device));
>>> +		return -ENODEV;
>>> +	}
>>> +	ipmi_device = kzalloc(sizeof(struct acpi_ipmi_device), GFP_KERNEL);
>>> +	if (!ipmi_device) {
>>> +		printk(KERN_DEBUG "Can't allocate memory space\n");
>>> +		return -ENOMEM;
>>> +	}
>>> +	ipmi_device->if_type = temp;
>>> +	switch (ipmi_device->if_type) {
>>> +	case 1:
>>> +	case 2:
>>> +	case 3:
>>> +		break;
>>> +	default:
>>> +		printk(KERN_DEBUG "Unknow IPMI:SI interface type %d\n",
>>> +				ipmi_device->if_type);
>>> +		kfree(ipmi_device);
>>> +		return -EINVAL;
>>> +	}
>>> +	ipmi_device->handle = device->handle;
>>> +	ipmi_device->device = device;
>>> +	mutex_init(&ipmi_device->mutex_lock);
>>> +	INIT_LIST_HEAD(&ipmi_device->head);
>>> +	INIT_LIST_HEAD(&ipmi_device->tx_msg_list);
>>> +
>>> +	if (ipmi_install_handlers(ipmi_device)) {
>>> +		/* can't register the IPMI opregion */
>>> +		kfree(ipmi_device);
>>> +		return -EINVAL;
>>> +	}
>>> +
>>> +	/* add it to the IPMI device list */
>>> +	list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices);
>>> +	device->driver_data = ipmi_device;
>>> +	return 0;
>>> +}
>>> +
>>> +static int acpi_ipmi_remove(struct acpi_device *device, int type)
>>> +{
>>> +	struct acpi_ipmi_device *ipmi_device;
>>> +
>>> +	ipmi_device = acpi_driver_data(device);
>>> +	if (!ipmi_device)
>>> +		return 0;
>>> +
>>> +	if (ipmi_device->user_interface) {
>>> +		/*
>>> +		 * If the IPMI user interface is created, it should be
>>> +		 * destroyed.
>>> +		 */
>>> +		ipmi_destroy_user(ipmi_device->user_interface);
>>> +		ipmi_device->user_interface = NULL;
>>> +	}
>>> +	list_del(&ipmi_device->head);
>>> +	if (!list_empty(&ipmi_device->tx_msg_list)) {
>>> +		/* destroy the Tx_msg list */
>>> +		ipmi_destroy_tx_msg(ipmi_device);
>>> +	}
>>> +	ipmi_remove_handlers(ipmi_device);
>>> +	kfree(ipmi_device);
>>> +	device->driver_data = NULL;
>>> +	return 0;
>>> +}
>>> +
>>> +static const struct acpi_device_id ipmi_device_ids[] = {
>>> +	{"IPI0001", 0},
>>> +	{"", 0},
>>> +};
>>> +
>>> +static struct acpi_driver acpi_ipmi_driver = {
>>> +	.name = "ipmi",
>>> +	.class = ACPI_IPMI_CLASS,
>>> +	.ids = ipmi_device_ids,
>>> +	.ops = {
>>> +		.add = acpi_ipmi_add,
>>> +		.remove = acpi_ipmi_remove,
>>> +		},
>>> +};
>>> +
>>> +static int __init acpi_ipmi_init(void)
>>> +{
>>> +	int result = 0;
>>> +
>>> +	if (acpi_disabled)
>>> +		return result;
>>> +
>>> +	result = acpi_bus_register_driver(&acpi_ipmi_driver);
>>> +
>>> +	if (result)
>>> +		return result;
>>> +
>>> +	result = ipmi_smi_watcher_register(&driver_data.bmc_events);
>>> +
>>> +	if (result)
>>> +		acpi_bus_unregister_driver(&acpi_ipmi_driver);
>>> +
>>> +	return result;
>>> +}
>>> +
>>> +static void __exit acpi_ipmi_exit(void)
>>> +{
>>> +	if (acpi_disabled)
>>> +		return;
>>> +
>>> +	ipmi_smi_watcher_unregister(&driver_data.bmc_events);
>>> +	acpi_bus_unregister_driver(&acpi_ipmi_driver);
>>> +
>>> +	return;
>>> +}
>>> +
>>> +module_init(acpi_ipmi_init);
>>> +module_exit(acpi_ipmi_exit);
>>>       
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>>     
>
>   

Re: [Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[ykzhao]

On Wed, 2009-12-09 at 22:57 +0800, Bjorn Helgaas wrote:
> On Tuesday 08 December 2009 06:58:45 pm yakui.zhao@intel.com wrote:
> > From: Zhao Yakui <yakui.zhao@intel.com>
> >
> > Add the IPMI opregion driver so that the AML code can communicate with BMC
> > throught IPMI message.
> >      It will create IPMI user interface for every IPMI device detected
> > in ACPI namespace and install the corresponding IPMI opregion space handler.
> >
> > The following describes how to process the IPMI request in IPMI space handler:
> >     1. format the IPMI message based on the request in AML code.
> >     IPMI system address. Now the address type is SYSTEM_INTERFACE_ADDR_TYPE
> >     IPMI net function & command
> >     IPMI message payload
> >     2. send the IPMI message by using the function of ipmi_request_settime
> >     3. wait for the completion of IPMI message. It can be done in different
> > routes: One is in handled in IPMI user recv callback function. Another is
> > handled in timeout function.
> >     4. format the IPMI response and return it to ACPI AML code.
> 
> I think this should be part of the ipmi_si_intf module, not a separate
> module.  This opregion code is of no use without ipmi_si_intf.  The
> opregion code can still be under its own config option, but I don't
> see the value in having it be a separate module.

This opregion code can't work if the IPMI subsystem is not loaded. this
is right. I also agree with this point.
But this doesn't mean that this must be put into the module of
ipmi_si_intf.
In fact this opregion code only uses the IPMI API function to
communicate with IPMI system interface. At the same time its main
purpose is to handle the request from ACPI AML code. It had better be
put into the ACPI subsystem. I think that it is unnecessary to put it
into the ipmi_si_intf module.


BTW: Several modules(for example: drivers/hwmon/ibmaem.c ,
drivers/hwmon/ibmpex.c) also use the IPMI system interface to
communicate with the IPMI subsystem. But they are the different part of
ipmi_si_intf module.

> The opregion code should definitely not use acpi_bus_register_driver()
> to claim IPI0001 devices.  The ipmi_si_intf driver already uses
> pnp_register_driver() to claim IPI0001 devices.  The fact that Linux
> allows two drivers to claim the same device (one via pnp_register_driver()
> and the other via acpi_bus_register_driver()) is a defect in Linux, and
> we shouldn't take advantage of that.

If you think that this code had better not use the
acpi_bus_register_driver, I will try to enumerate this device from ACPI
device tree directly and then install the IPMI opregion handler for it.

Is this ok to you?

thanks.

> 
> Bjorn
> 
> > Signed-off-by: Zhao Yakui <yakui.zhao@intel.com>
> > ---
> >  drivers/acpi/Kconfig  |   12 +
> >  drivers/acpi/Makefile |    1 +
> >  drivers/acpi/ipmi.c   |  583 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 596 insertions(+), 0 deletions(-)
> >  create mode 100644 drivers/acpi/ipmi.c
> >
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index 93d2c79..23cd08a 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -204,6 +204,18 @@ 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 EXPERIMENTAL
> > +     default n
> > +     select IPMI_HANDLER
> > +     select IPMI_SI
> > +     help
> > +       This driver enables the ACPI to access the BMC controller. And it
> > +       uses the IPMI request/response message to communicate with BMC
> > +       controller, which can be found on on the server.
> > +
> > +       To compile this driver as a module, choose M here:
> >
> >  config ACPI_HOTPLUG_CPU
> >       bool
> > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> > index 7702118..61077b7 100644
> > --- a/drivers/acpi/Makefile
> > +++ b/drivers/acpi/Makefile
> > @@ -57,6 +57,7 @@ obj-$(CONFIG_ACPI_BATTERY)  += battery.o
> >  obj-$(CONFIG_ACPI_SBS)               += sbshc.o
> >  obj-$(CONFIG_ACPI_SBS)               += sbs.o
> >  obj-$(CONFIG_ACPI_POWER_METER)       += power_meter.o
> > +obj-$(CONFIG_ACPI_IPMI)              += ipmi.o
> >
> >  # processor has its own "processor." module_param namespace
> >  processor-y                  := processor_core.o processor_throttling.o
> > diff --git a/drivers/acpi/ipmi.c b/drivers/acpi/ipmi.c
> > new file mode 100644
> > index 0000000..5c74936
> > --- /dev/null
> > +++ b/drivers/acpi/ipmi.c
> > @@ -0,0 +1,583 @@
> > +/*
> > + *  ipmi.c - ACPI IPMI opregion
> > + *
> > + *  Copyright (C) 2009 Intel Corporation
> > + *  Copyright (C) 2009 Zhao Yakui <yakui.zhao@intel.com>
> > + *
> > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > + *
> > + *  This program is free software; you can redistribute it and/or modify
> > + *  it under the terms of the GNU General Public License as published by
> > + *  the Free Software Foundation; either version 2 of the License, or (at
> > + *  your option) any later version.
> > + *
> > + *  This program is distributed in the hope that it will be useful, but
> > + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> > + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > + *  General Public License for more details.
> > + *
> > + *  You should have received a copy of the GNU General Public License along
> > + *  with this program; if not, write to the Free Software Foundation, Inc.,
> > + *  59 Temple Place, Suite 330, Boston, MA 02111-1307 USA.
> > + *
> > + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > + */
> > +
> > +#include <linux/kernel.h>
> > +#include <linux/module.h>
> > +#include <linux/init.h>
> > +#include <linux/types.h>
> > +#include <linux/delay.h>
> > +#include <linux/proc_fs.h>
> > +#include <linux/seq_file.h>
> > +#include <linux/interrupt.h>
> > +#include <linux/list.h>
> > +#include <linux/spinlock.h>
> > +#include <linux/io.h>
> > +#include <acpi/acpi_bus.h>
> > +#include <acpi/acpi_drivers.h>
> > +#include <linux/ipmi.h>
> > +
> > +MODULE_AUTHOR("Zhao Yakui");
> > +MODULE_DESCRIPTION("ACPI IPMI Opregion driver");
> > +MODULE_LICENSE("GPL");
> > +
> > +#define ACPI_IPMI_CLASS                      "IPMI"
> > +#define ACPI_IPMI_DEVICE_NAME                "IPMI_dev"
> > +#define IPMI_FLAGS_HANDLER_INSTALL   0
> > +
> > +#define ACPI_IPMI_OK                 0
> > +#define ACPI_IPMI_TIMEOUT            0x10
> > +#define ACPI_IPMI_UNKNOWN            0x07
> > +/* the IPMI timeout is 30s */
> > +#define IPMI_TIMEOUT                 (30 * HZ)
> > +
> > +
> > +struct acpi_ipmi_device {
> > +     acpi_handle handle;
> > +     struct acpi_device *device;
> > +     int if_type;
> > +     /* the device list attached to driver_data.ipmi_devices */
> > +     struct list_head head;
> > +     ipmi_user_t     user_interface;
> > +     struct mutex    mutex_lock;
> > +     /* the IPMI request message list */
> > +     struct list_head tx_msg_list;
> > +     long curr_msgid;
> > +     /* IPMI flags */
> > +     unsigned long flags;
> > +};
> > +
> > +struct ipmi_driver_data {
> > +     int device_count;
> > +     struct list_head        ipmi_devices;
> > +     struct ipmi_smi_watcher bmc_events;
> > +     struct ipmi_user_hndl   ipmi_hndlrs;
> > +};
> > +
> > +struct acpi_ipmi_msg {
> > +     /* message list */
> > +     struct list_head head;
> > +     /*
> > +      * General speaking the addr type should be SI_ADDR_TYPE. And
> > +      * the addr channel should be BMC.
> > +      * In fact it can also be IPMB type. But we will have to
> > +      * parse it from the Netfn command buffer. It is so complex
> > +      * that it is skipped.
> > +      */
> > +     struct ipmi_addr addr;
> > +     /* tx message id */
> > +     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;
> > +     /* tx data . And copy it from ACPI object buffer */
> > +     u8      tx_data[64];
> > +     int     tx_len;
> > +     /* get the response data */
> > +     u8      rx_data[64];
> > +     /* the response length. The netfn & cmd is excluded. */
> > +     int     rx_len;
> > +     struct acpi_ipmi_device *device;
> > +};
> > +
> > +/*
> > + * IPMI request/response buffer.
> > + * The length is 66 bytes.
> > + */
> > +struct acpi_ipmi_buffer {
> > +     /* status code of a given IPMI command */
> > +     u8 status_code;
> > +     /* the length of the payload */
> > +     u8 length;
> > +     /*
> > +      * the payload. Before the operation is carried out, it represents the
> > +      * request message payload. After the opration is carried out, it
> > +      * stores the response message returned by IPMI command.
> > +      */
> > +     u8 data[64];
> > +};
> > +
> > +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 struct ipmi_driver_data driver_data = {
> > +     .ipmi_devices = LIST_HEAD_INIT(driver_data.ipmi_devices),
> > +     .bmc_events = {
> > +             .owner = THIS_MODULE,
> > +             .new_smi = ipmi_register_bmc,
> > +             .smi_gone = ipmi_bmc_gone,
> > +     },
> > +     .ipmi_hndlrs = {
> > +             .ipmi_recv_hndl = ipmi_msg_handler,
> > +     },
> > +};
> > +
> > +static
> > +struct acpi_ipmi_msg *acpi_alloc_ipmi_msg(struct acpi_ipmi_device *ipmi)
> > +{
> > +     struct acpi_ipmi_msg *ipmi_msg;
> > +
> > +     ipmi_msg = kzalloc(sizeof(struct acpi_ipmi_msg), GFP_KERNEL);
> > +     if (!ipmi_msg)  {
> > +             printk(KERN_DEBUG "Can't allocate memory for ipmi_msg\n");
> > +             return NULL;
> > +     }
> > +     init_completion(&ipmi_msg->tx_complete);
> > +     INIT_LIST_HEAD(&ipmi_msg->head);
> > +     ipmi_msg->device = ipmi;
> > +     return ipmi_msg;
> > +}
> > +
> > +static void acpi_format_ipmi_msg(struct acpi_ipmi_msg *tx_msg,
> > +             acpi_physical_address address,
> > +             acpi_integer *value)
> > +{
> > +     struct kernel_ipmi_msg *msg;
> > +     u8      temp_value;
> > +     struct acpi_ipmi_buffer *buffer;
> > +     struct acpi_ipmi_device *device;
> > +
> > +     msg = &tx_msg->tx_message;
> > +     /* get the netfn */
> > +     temp_value = (address >> 8) & 0xff;
> > +     msg->netfn = temp_value;
> > +     /* get the command */
> > +     temp_value = address & 0xff;
> > +     msg->cmd = temp_value;
> > +     msg->data = tx_msg->tx_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 */
> > +     msg->data_len = buffer->length;
> > +     memcpy(tx_msg->tx_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,
> > +      * the addr type should be changed to IPMB.
> > +      */
> > +     tx_msg->addr.addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE;
> > +     tx_msg->addr.channel = IPMI_BMC_CHANNEL;
> > +     tx_msg->addr.data[0] = 0;
> > +
> > +     /*
> > +      * If the netfn is APP_REQUEST and the cmd is SEND_MESSAGE, we should
> > +      * parse the IPMI request message buffer to get the IPMB address.
> > +      * If so, please fix me.
> > +      */
> > +
> > +     /* Get the msgid */
> > +     device = tx_msg->device;
> > +     mutex_lock(&device->mutex_lock);
> > +     device->curr_msgid++;
> > +     tx_msg->tx_msgid = device->curr_msgid;
> > +     mutex_unlock(&device->mutex_lock);
> > +}
> > +
> > +static void acpi_format_ipmi_response(struct acpi_ipmi_msg *msg,
> > +             acpi_integer *value, int timeout)
> > +{
> > +     struct acpi_ipmi_buffer *buffer;
> > +
> > +     /*
> > +      * value is also used as output parameter. It represents the response
> > +      * IPMI message returned by IPMI command.
> > +      */
> > +     buffer = (struct acpi_ipmi_buffer *)value;
> > +     /* when timeout is zero, it means that the timeout happens */
> > +     if (!timeout) {
> > +             /* the status code is ACPI_IPMI_TIMEOUT */
> > +             buffer->status_code = ACPI_IPMI_TIMEOUT;
> > +             return;
> > +     }
> > +     /*
> > +      * If the flag of msg_done is not set, it means that the IPMI command
> > +      * is not executed correctly.
> > +      * The status code will be ACPI_IPMI_UNKNOWN.
> > +      */
> > +     if (!msg->msg_done) {
> > +             buffer->status_code = ACPI_IPMI_UNKNOWN;
> > +             return;
> > +     }
> > +     /*
> > +      * If the IPMI response message is obtained correctly, the status code
> > +      * will be ACPI_IPMI_OK
> > +      */
> > +     buffer->status_code = ACPI_IPMI_OK;
> > +     buffer->length = msg->rx_len;
> > +     memcpy(buffer->data, msg->rx_data, msg->rx_len);
> > +     return;
> > +}
> > +static void ipmi_destroy_tx_msg(struct acpi_ipmi_device *ipmi)
> > +{
> > +     struct acpi_ipmi_msg *tx_msg = NULL;
> > +     int count = 20;
> > +
> > +     list_for_each_entry(tx_msg, &ipmi->tx_msg_list, head) {
> > +             /* wake up the sleep thread on the Tx msg */
> > +             complete(&tx_msg->tx_complete);
> > +     }
> > +     /* wait for about 20 ticks 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))
> > +             printk(KERN_DEBUG "tx msg list is not NULL\n");
> > +
> > +}
> > +
> > +static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
> > +{
> > +     struct acpi_ipmi_device *ipmi_device =
> > +                     (struct acpi_ipmi_device *)user_msg_data;
> > +     int msg_found = 0;
> > +     struct acpi_ipmi_msg *tx_msg = NULL;
> > +
> > +     if (msg->user != ipmi_device->user_interface) {
> > +             printk(KERN_DEBUG "Incorrect IPMI user\n");
> > +             ipmi_free_recv_msg(msg);
> > +             return;
> > +     }
> > +     mutex_lock(&ipmi_device->mutex_lock);
> > +     list_for_each_entry(tx_msg, &ipmi_device->tx_msg_list, head) {
> > +             if (msg->msgid == tx_msg->tx_msgid) {
> > +                     /* find the message id */
> > +                     msg_found = 1;
> > +                     break;
> > +             }
> > +     }
> > +
> > +     mutex_unlock(&ipmi_device->mutex_lock);
> > +     if (!msg_found) {
> > +             /* no matched msg is found . But we should free it */
> > +             ipmi_free_recv_msg(msg);
> > +             printk(KERN_DEBUG "Incorrect MSG is found \n");
> > +             return;
> > +     }
> > +
> > +     if (msg->msg.data_len > 1) {
> > +             /* copy the response data to Rx_data buffer */
> > +             memcpy(tx_msg->rx_data, msg->msg_data, msg->msg.data_len);
> > +             tx_msg->rx_len = msg->msg.data_len;
> > +             tx_msg->msg_done = 1;
> > +     }
> > +     complete(&tx_msg->tx_complete);
> > +     ipmi_free_recv_msg(msg);
> > +};
> > +
> > +static void ipmi_register_bmc(int iface, struct device *dev)
> > +{
> > +     struct acpi_ipmi_device *ipmi_device;
> > +     struct acpi_device *device;
> > +     ipmi_user_t             user;
> > +     int err;
> > +
> > +     if (list_empty(&driver_data.ipmi_devices))
> > +             return;
> > +
> > +     list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
> > +             device = ipmi_device->device;
> > +             if (ipmi_device->user_interface) {
> > +                     /*
> > +                      * Only one user interface is allowed to be registered
> > +                      * for one IPMI device.
> > +                      * If we already create the user interface for
> > +                      * one IPMI device, skip it
> > +                      */
> > +                     continue;
> > +             }
> > +             if (dev == &device->dev) {
> > +                     /*
> > +                      * If the dev is identical to the ACPI device,
> > +                      * create the user interface.
> > +                      */
> > +                     err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs,
> > +                                             ipmi_device, &user);
> > +                     if (err == 0)
> > +                             ipmi_device->user_interface = user;
> > +
> > +                     continue;
> > +             }
> > +             /*
> > +              * In fact maybe the IPMI interface can be registered by
> > +              * other methods. For example: SPMI, DMI, PCI
> > +              * So we should also create the user interface.
> > +              */
> > +             err = ipmi_create_user(iface, &driver_data.ipmi_hndlrs,
> > +                                             ipmi_device, &user);
> > +             if (err == 0)
> > +                     ipmi_device->user_interface = user;
> > +     }
> > +     return;
> > +}
> > +
> > +static void ipmi_bmc_gone(int iface)
> > +{
> > +     struct acpi_ipmi_device *ipmi_device;
> > +
> > +     if (list_empty(&driver_data.ipmi_devices))
> > +             return;
> > +
> > +     list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
> > +             if (ipmi_device->user_interface) {
> > +                     ipmi_destroy_user(ipmi_device->user_interface);
> > +                     ipmi_device->user_interface = NULL;
> > +                     /* we should also destory tx msg list */
> > +                     ipmi_destroy_tx_msg(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
> > + * by command, only write is meaningful.
> > + * @address: This contains the netfn/command of IPMI request message.
> > + * @bits   : not used.
> > + * @value  : it is an in/out parameter. It points to the IPMI message buffer.
> > + *        Before the IPMI message is sent, it represents the actual request
> > + *        IPMI message. After the IPMI message is finished, it represents
> > + *        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)
> > +{
> > +     struct acpi_ipmi_msg *tx_msg = NULL;
> > +     struct acpi_ipmi_device *ipmi_device =
> > +                     (struct acpi_ipmi_device *) handler_context;
> > +     int err;
> > +     acpi_status status;
> > +     /*
> > +      * IPMI opregion message.
> > +      * IPMI message is firstly written to the BMC and system software
> > +      * can get the respsonse. So it is unmeaningful for the IPMI read
> > +      * access.
> > +      */
> > +     if ((function & ACPI_IO_MASK) == ACPI_READ) {
> > +             /* Read function is not supported. AE_TYPE is returned. */
> > +             return AE_TYPE;
> > +     }
> > +     if (!ipmi_device->user_interface) {
> > +             /* there doesn't exist user interface*/
> > +             return AE_NOT_EXIST;
> > +     }
> > +     tx_msg = acpi_alloc_ipmi_msg(ipmi_device);
> > +     if (!tx_msg) {
> > +             /* no memory is allocated */
> > +             return AE_NO_MEMORY;
> > +     }
> > +     acpi_format_ipmi_msg(tx_msg, address, value);
> > +     mutex_lock(&ipmi_device->mutex_lock);
> > +     list_add_tail(&tx_msg->head, &ipmi_device->tx_msg_list);
> > +     mutex_unlock(&ipmi_device->mutex_lock);
> > +     err = ipmi_request_settime(ipmi_device->user_interface,
> > +                                     &tx_msg->addr,
> > +                                     tx_msg->tx_msgid,
> > +                                     &tx_msg->tx_message,
> > +                                     NULL, 0, 0, 0);
> > +     if (err) {
> > +             status = AE_ERROR;
> > +             goto end_label;
> > +     }
> > +     err = wait_for_completion_timeout(&tx_msg->tx_complete, IPMI_TIMEOUT);
> > +
> > +end_label:
> > +     acpi_format_ipmi_response(tx_msg, value, err);
> > +     status = AE_OK;
> > +     mutex_lock(&ipmi_device->mutex_lock);
> > +     list_del(&tx_msg->head);
> > +     mutex_unlock(&ipmi_device->mutex_lock);
> > +     kfree(tx_msg);
> > +     return status;
> > +}
> > +
> > +static void ipmi_remove_handlers(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_handlers(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, ipmi);
> > +     if (ACPI_FAILURE(status)) {
> > +             printk(KERN_DEBUG "Can't register IPMI opregion %s\n",
> > +                                     acpi_device_bid(ipmi->device));
> > +             return -EINVAL;
> > +     }
> > +     set_bit(IPMI_FLAGS_HANDLER_INSTALL, &ipmi->flags);
> > +     return 0;
> > +}
> > +
> > +static int acpi_ipmi_add(struct acpi_device *device)
> > +{
> > +     struct acpi_ipmi_device *ipmi_device;
> > +     acpi_handle handle;
> > +     unsigned long long temp;
> > +     acpi_status status;
> > +     if (!device)
> > +             return -EINVAL;
> > +
> > +     handle = device->handle;
> > +     temp = 0;
> > +     status = acpi_evaluate_integer(handle, "_IFT", NULL, &temp);
> > +     if (ACPI_FAILURE(status)) {
> > +             printk(KERN_DEBUG "Incorrect _IFT object for %s\n",
> > +                             acpi_device_bid(device));
> > +             return -ENODEV;
> > +     }
> > +     ipmi_device = kzalloc(sizeof(struct acpi_ipmi_device), GFP_KERNEL);
> > +     if (!ipmi_device) {
> > +             printk(KERN_DEBUG "Can't allocate memory space\n");
> > +             return -ENOMEM;
> > +     }
> > +     ipmi_device->if_type = temp;
> > +     switch (ipmi_device->if_type) {
> > +     case 1:
> > +     case 2:
> > +     case 3:
> > +             break;
> > +     default:
> > +             printk(KERN_DEBUG "Unknow IPMI:SI interface type %d\n",
> > +                             ipmi_device->if_type);
> > +             kfree(ipmi_device);
> > +             return -EINVAL;
> > +     }
> > +     ipmi_device->handle = device->handle;
> > +     ipmi_device->device = device;
> > +     mutex_init(&ipmi_device->mutex_lock);
> > +     INIT_LIST_HEAD(&ipmi_device->head);
> > +     INIT_LIST_HEAD(&ipmi_device->tx_msg_list);
> > +
> > +     if (ipmi_install_handlers(ipmi_device)) {
> > +             /* can't register the IPMI opregion */
> > +             kfree(ipmi_device);
> > +             return -EINVAL;
> > +     }
> > +
> > +     /* add it to the IPMI device list */
> > +     list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices);
> > +     device->driver_data = ipmi_device;
> > +     return 0;
> > +}
> > +
> > +static int acpi_ipmi_remove(struct acpi_device *device, int type)
> > +{
> > +     struct acpi_ipmi_device *ipmi_device;
> > +
> > +     ipmi_device = acpi_driver_data(device);
> > +     if (!ipmi_device)
> > +             return 0;
> > +
> > +     if (ipmi_device->user_interface) {
> > +             /*
> > +              * If the IPMI user interface is created, it should be
> > +              * destroyed.
> > +              */
> > +             ipmi_destroy_user(ipmi_device->user_interface);
> > +             ipmi_device->user_interface = NULL;
> > +     }
> > +     list_del(&ipmi_device->head);
> > +     if (!list_empty(&ipmi_device->tx_msg_list)) {
> > +             /* destroy the Tx_msg list */
> > +             ipmi_destroy_tx_msg(ipmi_device);
> > +     }
> > +     ipmi_remove_handlers(ipmi_device);
> > +     kfree(ipmi_device);
> > +     device->driver_data = NULL;
> > +     return 0;
> > +}
> > +
> > +static const struct acpi_device_id ipmi_device_ids[] = {
> > +     {"IPI0001", 0},
> > +     {"", 0},
> > +};
> > +
> > +static struct acpi_driver acpi_ipmi_driver = {
> > +     .name = "ipmi",
> > +     .class = ACPI_IPMI_CLASS,
> > +     .ids = ipmi_device_ids,
> > +     .ops = {
> > +             .add = acpi_ipmi_add,
> > +             .remove = acpi_ipmi_remove,
> > +             },
> > +};
> > +
> > +static int __init acpi_ipmi_init(void)
> > +{
> > +     int result = 0;
> > +
> > +     if (acpi_disabled)
> > +             return result;
> > +
> > +     result = acpi_bus_register_driver(&acpi_ipmi_driver);
> > +
> > +     if (result)
> > +             return result;
> > +
> > +     result = ipmi_smi_watcher_register(&driver_data.bmc_events);
> > +
> > +     if (result)
> > +             acpi_bus_unregister_driver(&acpi_ipmi_driver);
> > +
> > +     return result;
> > +}
> > +
> > +static void __exit acpi_ipmi_exit(void)
> > +{
> > +     if (acpi_disabled)
> > +             return;
> > +
> > +     ipmi_smi_watcher_unregister(&driver_data.bmc_events);
> > +     acpi_bus_unregister_driver(&acpi_ipmi_driver);
> > +
> > +     return;
> > +}
> > +
> > +module_init(acpi_ipmi_init);
> > +module_exit(acpi_ipmi_exit);
> 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[Bjorn Helgaas]

On Sunday 13 December 2009 03:20:27 am ykzhao wrote:
> On Wed, 2009-12-09 at 22:57 +0800, Bjorn Helgaas wrote:
> > On Tuesday 08 December 2009 06:58:45 pm yakui.zhao@intel.com wrote:
> > > From: Zhao Yakui <yakui.zhao@intel.com>
> > >
> > > Add the IPMI opregion driver so that the AML code can communicate with BMC
> > > throught IPMI message.
> > >      It will create IPMI user interface for every IPMI device detected
> > > in ACPI namespace and install the corresponding IPMI opregion space handler.
> > >
> > > The following describes how to process the IPMI request in IPMI space handler:
> > >     1. format the IPMI message based on the request in AML code.
> > >     IPMI system address. Now the address type is SYSTEM_INTERFACE_ADDR_TYPE
> > >     IPMI net function & command
> > >     IPMI message payload
> > >     2. send the IPMI message by using the function of ipmi_request_settime
> > >     3. wait for the completion of IPMI message. It can be done in different
> > > routes: One is in handled in IPMI user recv callback function. Another is
> > > handled in timeout function.
> > >     4. format the IPMI response and return it to ACPI AML code.
> > 
> > I think this should be part of the ipmi_si_intf module, not a separate
> > module.  This opregion code is of no use without ipmi_si_intf.  The
> > opregion code can still be under its own config option, but I don't
> > see the value in having it be a separate module.
> 
> This opregion code can't work if the IPMI subsystem is not loaded. this
> is right. I also agree with this point.
> But this doesn't mean that this must be put into the module of
> ipmi_si_intf.
> In fact this opregion code only uses the IPMI API function to
> communicate with IPMI system interface. At the same time its main
> purpose is to handle the request from ACPI AML code. It had better be
> put into the ACPI subsystem. I think that it is unnecessary to put it
> into the ipmi_si_intf module.
> 
> 
> BTW: Several modules(for example: drivers/hwmon/ibmaem.c ,
> drivers/hwmon/ibmpex.c) also use the IPMI system interface to
> communicate with the IPMI subsystem. But they are the different part of
> ipmi_si_intf module.
> 
> > The opregion code should definitely not use acpi_bus_register_driver()
> > to claim IPI0001 devices.  The ipmi_si_intf driver already uses
> > pnp_register_driver() to claim IPI0001 devices.  The fact that Linux
> > allows two drivers to claim the same device (one via pnp_register_driver()
> > and the other via acpi_bus_register_driver()) is a defect in Linux, and
> > we shouldn't take advantage of that.
> 
> If you think that this code had better not use the
> acpi_bus_register_driver, I will try to enumerate this device from ACPI
> device tree directly and then install the IPMI opregion handler for it.

I think there are two questions here: (1) should this be part of
ipmi_si_intf? and (2) how should it be registered?

For (1), I see your point that we do have existing modules that are
separate modules, and this is conceptually similar to those.  So
maybe this should be a separate module, too.  If they're separate
modules, I think the issues are how they communicate and how we get
the opregion part autoloaded.  It's easy to see how ipmi_si_intf
gets autoloaded -- the PNP or PCI core discovers a device and emits
a uevent that udev can match with a driver.  I don't think we want
the user to have to explicitly modprobe the opregion driver, so we
need some uevent or similar hook that udev can use to load it
automatically when appropriate.

For (2), I don't think using acpi_walk_namespace() to determine when
to install the IPMI opregion handler is quite the right thing either.

This opregion handler allows AML to use IPMI services.  Using
acpi_walk_namespace() means that we'll only install the handler
when we have an ACPI IPMI device.  But I think the opregion should
be independent of the type of IPMI device, i.e., I think we want
this opregion to work even if the underlying IPMI interface is PCI.

Bjorn

Re: [Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[ykzhao]

On Tue, 2009-12-15 at 08:18 +0800, Bjorn Helgaas wrote:
> On Sunday 13 December 2009 03:20:27 am ykzhao wrote:
> > On Wed, 2009-12-09 at 22:57 +0800, Bjorn Helgaas wrote:
> > > On Tuesday 08 December 2009 06:58:45 pm yakui.zhao@intel.com wrote:
> > > > From: Zhao Yakui <yakui.zhao@intel.com>
> > > >
> > > > Add the IPMI opregion driver so that the AML code can communicate with BMC
> > > > throught IPMI message.
> > > >      It will create IPMI user interface for every IPMI device detected
> > > > in ACPI namespace and install the corresponding IPMI opregion space handler.
> > > >
> > > > The following describes how to process the IPMI request in IPMI space handler:
> > > >     1. format the IPMI message based on the request in AML code.
> > > >     IPMI system address. Now the address type is SYSTEM_INTERFACE_ADDR_TYPE
> > > >     IPMI net function & command
> > > >     IPMI message payload
> > > >     2. send the IPMI message by using the function of ipmi_request_settime
> > > >     3. wait for the completion of IPMI message. It can be done in different
> > > > routes: One is in handled in IPMI user recv callback function. Another is
> > > > handled in timeout function.
> > > >     4. format the IPMI response and return it to ACPI AML code.
> > > 
> > > I think this should be part of the ipmi_si_intf module, not a separate
> > > module.  This opregion code is of no use without ipmi_si_intf.  The
> > > opregion code can still be under its own config option, but I don't
> > > see the value in having it be a separate module.
> > 
> > This opregion code can't work if the IPMI subsystem is not loaded. this
> > is right. I also agree with this point.
> > But this doesn't mean that this must be put into the module of
> > ipmi_si_intf.
> > In fact this opregion code only uses the IPMI API function to
> > communicate with IPMI system interface. At the same time its main
> > purpose is to handle the request from ACPI AML code. It had better be
> > put into the ACPI subsystem. I think that it is unnecessary to put it
> > into the ipmi_si_intf module.
> > 
> > 
> > BTW: Several modules(for example: drivers/hwmon/ibmaem.c ,
> > drivers/hwmon/ibmpex.c) also use the IPMI system interface to
> > communicate with the IPMI subsystem. But they are the different part of
> > ipmi_si_intf module.
> > 
> > > The opregion code should definitely not use acpi_bus_register_driver()
> > > to claim IPI0001 devices.  The ipmi_si_intf driver already uses
> > > pnp_register_driver() to claim IPI0001 devices.  The fact that Linux
> > > allows two drivers to claim the same device (one via pnp_register_driver()
> > > and the other via acpi_bus_register_driver()) is a defect in Linux, and
> > > we shouldn't take advantage of that.
> > 
> > If you think that this code had better not use the
> > acpi_bus_register_driver, I will try to enumerate this device from ACPI
> > device tree directly and then install the IPMI opregion handler for it.
> 
> I think there are two questions here: (1) should this be part of
> ipmi_si_intf? and (2) how should it be registered?
> 
> For (1), I see your point that we do have existing modules that are
> separate modules, and this is conceptually similar to those.  So
> maybe this should be a separate module, too.  If they're separate
> modules, I think the issues are how they communicate and how we get
> the opregion part autoloaded.  It's easy to see how ipmi_si_intf
> gets autoloaded -- the PNP or PCI core discovers a device and emits
> a uevent that udev can match with a driver.  I don't think we want
> the user to have to explicitly modprobe the opregion driver, so we
> need some uevent or similar hook that udev can use to load it
> automatically when appropriate.

>From the source code it seems that the ipmi_si_intf module only detects
the available BMC device. It doesn't care which part communicates with
the BMC device. So it will be better to put this code out of
ipmi_si_intf module. 

Whether the opregion code should used depends on the user configuration.
If we expect that the ACPI aml code can communicate with the BMC, we
will select this part. Correspondingly the IPMI subsystem will also be
selected as this part depends on the IPMI subsystem. If the user doesn't
care whether the AML code can communicate with the BMC, they can leave
it alone. If we expect to use the uevent, that is not related with this
patch.

> 
> For (2), I don't think using acpi_walk_namespace() to determine when
> to install the IPMI opregion handler is quite the right thing either.


> This opregion handler allows AML to use IPMI services.  Using
> acpi_walk_namespace() means that we'll only install the handler
> when we have an ACPI IPMI device.  But I think the opregion should
> be independent of the type of IPMI device, i.e., I think we want
> this opregion to work even if the underlying IPMI interface is PCI.

This opregion code is only to enable the ACPI AML code to communicate
with the BMC device. If AML code wants to access the BMC controller, we
must declare the corresponding IPMI device in ACPI namespace and add the
opregion field definition. If the IPMI device is not declared explicitly
in ACPI table, this code won't work even when there exists the BMC
device on the system.

thanks.
    Yakui

> 
> Bjorn

Re: [Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[Bjorn Helgaas]

On Tue, 2009-12-15 at 08:59 +0800, ykzhao wrote:
> On Tue, 2009-12-15 at 08:18 +0800, Bjorn Helgaas wrote:
> > On Sunday 13 December 2009 03:20:27 am ykzhao wrote:
> > > On Wed, 2009-12-09 at 22:57 +0800, Bjorn Helgaas wrote:
> > > > On Tuesday 08 December 2009 06:58:45 pm yakui.zhao@intel.com wrote:
> > > > > From: Zhao Yakui <yakui.zhao@intel.com>
> > > > >
> > > > > Add the IPMI opregion driver so that the AML code can communicate with BMC
> > > > > throught IPMI message.
> > > > >      It will create IPMI user interface for every IPMI device detected
> > > > > in ACPI namespace and install the corresponding IPMI opregion space handler.
> > > > >
> > > > > The following describes how to process the IPMI request in IPMI space handler:
> > > > >     1. format the IPMI message based on the request in AML code.
> > > > >     IPMI system address. Now the address type is SYSTEM_INTERFACE_ADDR_TYPE
> > > > >     IPMI net function & command
> > > > >     IPMI message payload
> > > > >     2. send the IPMI message by using the function of ipmi_request_settime
> > > > >     3. wait for the completion of IPMI message. It can be done in different
> > > > > routes: One is in handled in IPMI user recv callback function. Another is
> > > > > handled in timeout function.
> > > > >     4. format the IPMI response and return it to ACPI AML code.
> > > > 
> > > > I think this should be part of the ipmi_si_intf module, not a separate
> > > > module.  This opregion code is of no use without ipmi_si_intf.  The
> > > > opregion code can still be under its own config option, but I don't
> > > > see the value in having it be a separate module.
> > > 
> > > This opregion code can't work if the IPMI subsystem is not loaded. this
> > > is right. I also agree with this point.
> > > But this doesn't mean that this must be put into the module of
> > > ipmi_si_intf.
> > > In fact this opregion code only uses the IPMI API function to
> > > communicate with IPMI system interface. At the same time its main
> > > purpose is to handle the request from ACPI AML code. It had better be
> > > put into the ACPI subsystem. I think that it is unnecessary to put it
> > > into the ipmi_si_intf module.
> > > 
> > > 
> > > BTW: Several modules(for example: drivers/hwmon/ibmaem.c ,
> > > drivers/hwmon/ibmpex.c) also use the IPMI system interface to
> > > communicate with the IPMI subsystem. But they are the different part of
> > > ipmi_si_intf module.
> > > 
> > > > The opregion code should definitely not use acpi_bus_register_driver()
> > > > to claim IPI0001 devices.  The ipmi_si_intf driver already uses
> > > > pnp_register_driver() to claim IPI0001 devices.  The fact that Linux
> > > > allows two drivers to claim the same device (one via pnp_register_driver()
> > > > and the other via acpi_bus_register_driver()) is a defect in Linux, and
> > > > we shouldn't take advantage of that.
> > > 
> > > If you think that this code had better not use the
> > > acpi_bus_register_driver, I will try to enumerate this device from ACPI
> > > device tree directly and then install the IPMI opregion handler for it.
> > 
> > I think there are two questions here: (1) should this be part of
> > ipmi_si_intf? and (2) how should it be registered?
> > 
> > For (1), I see your point that we do have existing modules that are
> > separate modules, and this is conceptually similar to those.  So
> > maybe this should be a separate module, too.  If they're separate
> > modules, I think the issues are how they communicate and how we get
> > the opregion part autoloaded.  It's easy to see how ipmi_si_intf
> > gets autoloaded -- the PNP or PCI core discovers a device and emits
> > a uevent that udev can match with a driver.  I don't think we want
> > the user to have to explicitly modprobe the opregion driver, so we
> > need some uevent or similar hook that udev can use to load it
> > automatically when appropriate.
> 
> From the source code it seems that the ipmi_si_intf module only detects
> the available BMC device. It doesn't care which part communicates with
> the BMC device. So it will be better to put this code out of
> ipmi_si_intf module. 
> 
> Whether the opregion code should used depends on the user configuration.
> If we expect that the ACPI aml code can communicate with the BMC, we
> will select this part. Correspondingly the IPMI subsystem will also be
> selected as this part depends on the IPMI subsystem. If the user doesn't
> care whether the AML code can communicate with the BMC, they can leave
> it alone. If we expect to use the uevent, that is not related with this
> patch.

I'm sorry, I don't understand what you're saying here.  I expect the
opregion module to be generally useful, so distros will probably enable
the config option, so the module will be present on every machine.  How
do you plan to have it loaded?  We need some sort of automatic
mechanism; the user should not have to decide whether to load the
module.

> > For (2), I don't think using acpi_walk_namespace() to determine when
> > to install the IPMI opregion handler is quite the right thing either.

> > This opregion handler allows AML to use IPMI services.  Using
> > acpi_walk_namespace() means that we'll only install the handler
> > when we have an ACPI IPMI device.  But I think the opregion should
> > be independent of the type of IPMI device, i.e., I think we want
> > this opregion to work even if the underlying IPMI interface is PCI.
> 
> This opregion code is only to enable the ACPI AML code to communicate
> with the BMC device. If AML code wants to access the BMC controller, we
> must declare the corresponding IPMI device in ACPI namespace and add the
> opregion field definition. If the IPMI device is not declared explicitly
> in ACPI table, this code won't work even when there exists the BMC
> device on the system.

Oops, sorry, I see that you are right: the ACPI 4.0 spec, sec 5.5.2.4.3,
Declaring IPMI Operation Regions, is indeed restricted so it only works
with ACPI IPMI devices.  That is an unfortunate restriction on the
platform architecture, but maybe there's a good reason for it.

That section says that operation regions are defined within the
immediate context of the 'owning' IPMI device.  This, together with the
fact that the opregion support is only useful for ACPI devices, suggests
to me that the opregion module is different from the ibmaem and ibmpex
modules after all.  Those modules apparently can work with *any* sort of
IPMI device, so they are more independent.

I know we have an implicit assumption that a system contains only one
IPMI device, but the ACPI spec doesn't mention that, so I'd like to
avoid that assumption.  The sentence in the spec about the "'owning'
IPMI device" implies an association between an opregion and a device.
That association is easy to maintain if the opregion support is
integrated into ipmi_si_intf, but hard to maintain if they are separate.

Bjorn

Re: [Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[ykzhao]

On Tue, 2009-12-15 at 13:22 +0800, Bjorn Helgaas wrote:
> On Tue, 2009-12-15 at 08:59 +0800, ykzhao wrote:
> > On Tue, 2009-12-15 at 08:18 +0800, Bjorn Helgaas wrote:
> > > On Sunday 13 December 2009 03:20:27 am ykzhao wrote:
> > > > On Wed, 2009-12-09 at 22:57 +0800, Bjorn Helgaas wrote:
> > > > > On Tuesday 08 December 2009 06:58:45 pm yakui.zhao@intel.com wrote:
> > > > > > From: Zhao Yakui <yakui.zhao@intel.com>
> > > > > >
> > > > > > Add the IPMI opregion driver so that the AML code can communicate with BMC
> > > > > > throught IPMI message.
> > > > > >      It will create IPMI user interface for every IPMI device detected
> > > > > > in ACPI namespace and install the corresponding IPMI opregion space handler.
> > > > > >
> > > > > > The following describes how to process the IPMI request in IPMI space handler:
> > > > > >     1. format the IPMI message based on the request in AML code.
> > > > > >     IPMI system address. Now the address type is SYSTEM_INTERFACE_ADDR_TYPE
> > > > > >     IPMI net function & command
> > > > > >     IPMI message payload
> > > > > >     2. send the IPMI message by using the function of ipmi_request_settime
> > > > > >     3. wait for the completion of IPMI message. It can be done in different
> > > > > > routes: One is in handled in IPMI user recv callback function. Another is
> > > > > > handled in timeout function.
> > > > > >     4. format the IPMI response and return it to ACPI AML code.
> > > > > 
> > > > > I think this should be part of the ipmi_si_intf module, not a separate
> > > > > module.  This opregion code is of no use without ipmi_si_intf.  The
> > > > > opregion code can still be under its own config option, but I don't
> > > > > see the value in having it be a separate module.
> > > > 
> > > > This opregion code can't work if the IPMI subsystem is not loaded. this
> > > > is right. I also agree with this point.
> > > > But this doesn't mean that this must be put into the module of
> > > > ipmi_si_intf.
> > > > In fact this opregion code only uses the IPMI API function to
> > > > communicate with IPMI system interface. At the same time its main
> > > > purpose is to handle the request from ACPI AML code. It had better be
> > > > put into the ACPI subsystem. I think that it is unnecessary to put it
> > > > into the ipmi_si_intf module.
> > > > 
> > > > 
> > > > BTW: Several modules(for example: drivers/hwmon/ibmaem.c ,
> > > > drivers/hwmon/ibmpex.c) also use the IPMI system interface to
> > > > communicate with the IPMI subsystem. But they are the different part of
> > > > ipmi_si_intf module.
> > > > 
> > > > > The opregion code should definitely not use acpi_bus_register_driver()
> > > > > to claim IPI0001 devices.  The ipmi_si_intf driver already uses
> > > > > pnp_register_driver() to claim IPI0001 devices.  The fact that Linux
> > > > > allows two drivers to claim the same device (one via pnp_register_driver()
> > > > > and the other via acpi_bus_register_driver()) is a defect in Linux, and
> > > > > we shouldn't take advantage of that.
> > > > 
> > > > If you think that this code had better not use the
> > > > acpi_bus_register_driver, I will try to enumerate this device from ACPI
> > > > device tree directly and then install the IPMI opregion handler for it.
> > > 
> > > I think there are two questions here: (1) should this be part of
> > > ipmi_si_intf? and (2) how should it be registered?
> > > 
> > > For (1), I see your point that we do have existing modules that are
> > > separate modules, and this is conceptually similar to those.  So
> > > maybe this should be a separate module, too.  If they're separate
> > > modules, I think the issues are how they communicate and how we get
> > > the opregion part autoloaded.  It's easy to see how ipmi_si_intf
> > > gets autoloaded -- the PNP or PCI core discovers a device and emits
> > > a uevent that udev can match with a driver.  I don't think we want
> > > the user to have to explicitly modprobe the opregion driver, so we
> > > need some uevent or similar hook that udev can use to load it
> > > automatically when appropriate.
> > 
> > From the source code it seems that the ipmi_si_intf module only detects
> > the available BMC device. It doesn't care which part communicates with
> > the BMC device. So it will be better to put this code out of
> > ipmi_si_intf module. 
> > 
> > Whether the opregion code should used depends on the user configuration.
> > If we expect that the ACPI aml code can communicate with the BMC, we
> > will select this part. Correspondingly the IPMI subsystem will also be
> > selected as this part depends on the IPMI subsystem. If the user doesn't
> > care whether the AML code can communicate with the BMC, they can leave
> > it alone. If we expect to use the uevent, that is not related with this
> > patch.
> 
> I'm sorry, I don't understand what you're saying here.  I expect the
> opregion module to be generally useful, so distros will probably enable
> the config option, so the module will be present on every machine.  How
> do you plan to have it loaded?  We need some sort of automatic
> mechanism; the user should not have to decide whether to load the
> module.

What I said is that user needs to decide whether the IPMI opregion
module should be used when building the kernel. If we want to use it, we
will have to select it in kernel configuration. And then system will
load it in the boot time. Right? If not right, do we need care the case
that this opregion code is not loaded automatically in the boot time?

If you think that it is ok to enable the opregion part as soon as we
finish the detection of IPMI system interfaces, I can add the opregion
code into the ipmi_si_intf part. But if so, maybe we can use the ACPI
device driver to detect the BMC controller defined in ACPI namespace
instead of using PNP device driver. At the same time this mechanism
should be used firstly.

Is this OK to you?

> 
> > > For (2), I don't think using acpi_walk_namespace() to determine when
> > > to install the IPMI opregion handler is quite the right thing either.
> 
> > > This opregion handler allows AML to use IPMI services.  Using
> > > acpi_walk_namespace() means that we'll only install the handler
> > > when we have an ACPI IPMI device.  But I think the opregion should
> > > be independent of the type of IPMI device, i.e., I think we want
> > > this opregion to work even if the underlying IPMI interface is PCI.
> > 
> > This opregion code is only to enable the ACPI AML code to communicate
> > with the BMC device. If AML code wants to access the BMC controller, we
> > must declare the corresponding IPMI device in ACPI namespace and add the
> > opregion field definition. If the IPMI device is not declared explicitly
> > in ACPI table, this code won't work even when there exists the BMC
> > device on the system.
> 
> Oops, sorry, I see that you are right: the ACPI 4.0 spec, sec 5.5.2.4.3,
> Declaring IPMI Operation Regions, is indeed restricted so it only works
> with ACPI IPMI devices.  That is an unfortunate restriction on the
> platform architecture, but maybe there's a good reason for it.
> 
> That section says that operation regions are defined within the
> immediate context of the 'owning' IPMI device.  This, together with the
> fact that the opregion support is only useful for ACPI devices, suggests
> to me that the opregion module is different from the ibmaem and ibmpex
> modules after all.  Those modules apparently can work with *any* sort of
> IPMI device, so they are more independent.

The IPMI opregion code is only used for the IPMI device that defined in
ACPI namespace. If one IPMI device is not defined in ACPI namespace,
then we won't install IPMI opregion space handler for it. Corresponding
the ACPI AML code won't communicate with it.

> 
> I know we have an implicit assumption that a system contains only one
> IPMI device, but the ACPI spec doesn't mention that, so I'd like to
> avoid that assumption.  The sentence in the spec about the "'owning'
> IPMI device" implies an association between an opregion and a device.
> That association is easy to maintain if the opregion support is
> integrated into ipmi_si_intf, but hard to maintain if they are separate.

I agree that it is easier to maintain the IPMI opregion code in the
ipmi_si_intf module. But this opregion code has no relationship with
IPMI subsystem besides that it uses the IPMI API interface function. If
we put it together, maybe some other people will be confused. They will
ask why put irrelevant code into the ipmi_si_intf module that only
detects the system available BMC controller? 

I doesn't assume that the system only contains one IPMI device. In the
patch it will compare the device pointed by IPMI smi_info with the ACPI
device and create one user_interface for it, which is used to
communicate with the BMC controller. When more than one BMC controller
is found on the system, the opregion code still can work if we first
register the IPMI device that is defined in ACPI namespace. 

But in fact maybe the IPMI device defined in ACPI namespace is already
registered by other mechanism(DMI, SPMI, PCI and so on). In such case
the device pointed by IPMI smi_info is not the ACPI device. So I add a
workaround for this issue. That means that it will create user_interface
even when the dev pointed by IPMI smi_info is not a ACPI device. Then if
two IPMI devices are defined in ACPI namespace, maybe this is a problem.


> Bjorn
> 
> 

Re: [Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[Bjorn Helgaas]

On Tuesday 15 December 2009 02:22:30 am ykzhao wrote:
> On Tue, 2009-12-15 at 13:22 +0800, Bjorn Helgaas wrote:
> > On Tue, 2009-12-15 at 08:59 +0800, ykzhao wrote:
> > > On Tue, 2009-12-15 at 08:18 +0800, Bjorn Helgaas wrote:
> > > > On Sunday 13 December 2009 03:20:27 am ykzhao wrote:
> > > > > On Wed, 2009-12-09 at 22:57 +0800, Bjorn Helgaas wrote:
> > > > > > On Tuesday 08 December 2009 06:58:45 pm yakui.zhao@intel.com wrote:
> > > > > > > From: Zhao Yakui <yakui.zhao@intel.com>
> > > > > > >
> > > > > > > Add the IPMI opregion driver so that the AML code can communicate with BMC
> > > > > > > throught IPMI message.
> > > > > > >      It will create IPMI user interface for every IPMI device detected
> > > > > > > in ACPI namespace and install the corresponding IPMI opregion space handler.
> > > > > > >
> > > > > > > The following describes how to process the IPMI request in IPMI space handler:
> > > > > > >     1. format the IPMI message based on the request in AML code.
> > > > > > >     IPMI system address. Now the address type is SYSTEM_INTERFACE_ADDR_TYPE
> > > > > > >     IPMI net function & command
> > > > > > >     IPMI message payload
> > > > > > >     2. send the IPMI message by using the function of ipmi_request_settime
> > > > > > >     3. wait for the completion of IPMI message. It can be done in different
> > > > > > > routes: One is in handled in IPMI user recv callback function. Another is
> > > > > > > handled in timeout function.
> > > > > > >     4. format the IPMI response and return it to ACPI AML code.
> > > > > > 
> > > > > > I think this should be part of the ipmi_si_intf module, not a separate
> > > > > > module.  This opregion code is of no use without ipmi_si_intf.  The
> > > > > > opregion code can still be under its own config option, but I don't
> > > > > > see the value in having it be a separate module.
> > > > > 
> > > > > This opregion code can't work if the IPMI subsystem is not loaded. this
> > > > > is right. I also agree with this point.
> > > > > But this doesn't mean that this must be put into the module of
> > > > > ipmi_si_intf.
> > > > > In fact this opregion code only uses the IPMI API function to
> > > > > communicate with IPMI system interface. At the same time its main
> > > > > purpose is to handle the request from ACPI AML code. It had better be
> > > > > put into the ACPI subsystem. I think that it is unnecessary to put it
> > > > > into the ipmi_si_intf module.
> > > > > 
> > > > > 
> > > > > BTW: Several modules(for example: drivers/hwmon/ibmaem.c ,
> > > > > drivers/hwmon/ibmpex.c) also use the IPMI system interface to
> > > > > communicate with the IPMI subsystem. But they are the different part of
> > > > > ipmi_si_intf module.
> > > > > 
> > > > > > The opregion code should definitely not use acpi_bus_register_driver()
> > > > > > to claim IPI0001 devices.  The ipmi_si_intf driver already uses
> > > > > > pnp_register_driver() to claim IPI0001 devices.  The fact that Linux
> > > > > > allows two drivers to claim the same device (one via pnp_register_driver()
> > > > > > and the other via acpi_bus_register_driver()) is a defect in Linux, and
> > > > > > we shouldn't take advantage of that.
> > > > > 
> > > > > If you think that this code had better not use the
> > > > > acpi_bus_register_driver, I will try to enumerate this device from ACPI
> > > > > device tree directly and then install the IPMI opregion handler for it.
> > > > 
> > > > I think there are two questions here: (1) should this be part of
> > > > ipmi_si_intf? and (2) how should it be registered?
> > > > 
> > > > For (1), I see your point that we do have existing modules that are
> > > > separate modules, and this is conceptually similar to those.  So
> > > > maybe this should be a separate module, too.  If they're separate
> > > > modules, I think the issues are how they communicate and how we get
> > > > the opregion part autoloaded.  It's easy to see how ipmi_si_intf
> > > > gets autoloaded -- the PNP or PCI core discovers a device and emits
> > > > a uevent that udev can match with a driver.  I don't think we want
> > > > the user to have to explicitly modprobe the opregion driver, so we
> > > > need some uevent or similar hook that udev can use to load it
> > > > automatically when appropriate.
> > > 
> > > From the source code it seems that the ipmi_si_intf module only detects
> > > the available BMC device. It doesn't care which part communicates with
> > > the BMC device. So it will be better to put this code out of
> > > ipmi_si_intf module. 
> > > 
> > > Whether the opregion code should used depends on the user configuration.
> > > If we expect that the ACPI aml code can communicate with the BMC, we
> > > will select this part. Correspondingly the IPMI subsystem will also be
> > > selected as this part depends on the IPMI subsystem. If the user doesn't
> > > care whether the AML code can communicate with the BMC, they can leave
> > > it alone. If we expect to use the uevent, that is not related with this
> > > patch.
> > 
> > I'm sorry, I don't understand what you're saying here.  I expect the
> > opregion module to be generally useful, so distros will probably enable
> > the config option, so the module will be present on every machine.  How
> > do you plan to have it loaded?  We need some sort of automatic
> > mechanism; the user should not have to decide whether to load the
> > module.
> 
> What I said is that user needs to decide whether the IPMI opregion
> module should be used when building the kernel. If we want to use it, we
> will have to select it in kernel configuration.

Yes.  During Kconfig, the user (or in most cases, the distro) selects
Y/N/M.

> And then system will 
> load it in the boot time. Right?

Distros will typically select "M", so we build a module.  The module
is not part of the static kernel image.  It will not be loaded unless
we make some arrangement for it to be loaded.  I'm asking what arrangement
you propose.

> If not right, do we need care the case 
> that this opregion code is not loaded automatically in the boot time?

If the opregion code is a module, it will not be loaded until some
event causes it to be loaded.  For example, for PCI drivers, the PCI
core emits a uevent when it discovers a PCI device.  The uevent contains
the vendor/device ID, and udev can match that information with a driver
that claims the device, and load the driver automatically.  Another
way is for the user to type "modprobe <driver>" manually.  Another is
for the user to set up /etc/modprobe.conf so the driver is loaded
automatically by an init script.

I think the last two possibilities ((1) manual modprobe, (2) edit
/etc/modprobe.conf) are poor choices because they require the user
to do too much work.

The kernel already emits uevents for ACPI/PNP devices, and udev
knows how to load matching drivers.  This mechanism is enough to
cause ipmi_si_intf to be loaded automatically, and that gives us a
good hook for the opregion piece as well.

We want to attach the opregion driver exactly when ipmi_si_intf
claims an ACPI device, so integrating this code (or at least a call
to it) in ipmi_si_intf seems natural.  It's also important to *detach*
the opregion handler at the appropriate time (e.g., when the IPMI
interface is removed or the ipmi_si_intf driver is unloaded).  Again,
tight integration in ipmi_si_intf solves the detach problem nicely.

> If you think that it is ok to enable the opregion part as soon as we
> finish the detection of IPMI system interfaces, I can add the opregion
> code into the ipmi_si_intf part. But if so, maybe we can use the ACPI
> device driver to detect the BMC controller defined in ACPI namespace
> instead of using PNP device driver. At the same time this mechanism
> should be used firstly.
> 
> Is this OK to you?

I'm sorry if I'm being repetitious or not understanding you correctly.

My recent patches add PNP detection to ipmi_si_intf.c.  I think it would
be a good idea to add the opregion setup to ipmi_pnp_probe().  I think
it would be a bad idea to change ipmi_si_intf.c to use acpi_walk_namespace()
or acpi_bus_register_driver().

> > > > For (2), I don't think using acpi_walk_namespace() to determine when
> > > > to install the IPMI opregion handler is quite the right thing either.
> > 
> > > > This opregion handler allows AML to use IPMI services.  Using
> > > > acpi_walk_namespace() means that we'll only install the handler
> > > > when we have an ACPI IPMI device.  But I think the opregion should
> > > > be independent of the type of IPMI device, i.e., I think we want
> > > > this opregion to work even if the underlying IPMI interface is PCI.
> > > 
> > > This opregion code is only to enable the ACPI AML code to communicate
> > > with the BMC device. If AML code wants to access the BMC controller, we
> > > must declare the corresponding IPMI device in ACPI namespace and add the
> > > opregion field definition. If the IPMI device is not declared explicitly
> > > in ACPI table, this code won't work even when there exists the BMC
> > > device on the system.
> > 
> > Oops, sorry, I see that you are right: the ACPI 4.0 spec, sec 5.5.2.4.3,
> > Declaring IPMI Operation Regions, is indeed restricted so it only works
> > with ACPI IPMI devices.  That is an unfortunate restriction on the
> > platform architecture, but maybe there's a good reason for it.
> > 
> > That section says that operation regions are defined within the
> > immediate context of the 'owning' IPMI device.  This, together with the
> > fact that the opregion support is only useful for ACPI devices, suggests
> > to me that the opregion module is different from the ibmaem and ibmpex
> > modules after all.  Those modules apparently can work with *any* sort of
> > IPMI device, so they are more independent.
> 
> The IPMI opregion code is only used for the IPMI device that defined in
> ACPI namespace. If one IPMI device is not defined in ACPI namespace,
> then we won't install IPMI opregion space handler for it. Corresponding
> the ACPI AML code won't communicate with it.

If we want the opregion code only for ACPI devices, the easiest way
to accomplish this is to set it up when we discover the ACPI device.
It's much harder to do it later, because you have to look through all
the IPMI interfaces and figure out which ones are ACPI devices.

Bjorn

Re: [Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[ykzhao]

On Tue, 2009-12-15 at 23:27 +0800, Bjorn Helgaas wrote:
> On Tuesday 15 December 2009 02:22:30 am ykzhao wrote:
> > On Tue, 2009-12-15 at 13:22 +0800, Bjorn Helgaas wrote:
> > > On Tue, 2009-12-15 at 08:59 +0800, ykzhao wrote:
> > > > On Tue, 2009-12-15 at 08:18 +0800, Bjorn Helgaas wrote:
> > > > > On Sunday 13 December 2009 03:20:27 am ykzhao wrote:
> > > > > > On Wed, 2009-12-09 at 22:57 +0800, Bjorn Helgaas wrote:
> > > > > > > On Tuesday 08 December 2009 06:58:45 pm yakui.zhao@intel.com wrote:
> > > > > > > > From: Zhao Yakui <yakui.zhao@intel.com>
> > > > > > > >
> > > > > > > > Add the IPMI opregion driver so that the AML code can communicate with BMC
> > > > > > > > throught IPMI message.
> > > > > > > >      It will create IPMI user interface for every IPMI device detected
> > > > > > > > in ACPI namespace and install the corresponding IPMI opregion space handler.
> > > > > > > >
> > > > > > > > The following describes how to process the IPMI request in IPMI space handler:
> > > > > > > >     1. format the IPMI message based on the request in AML code.
> > > > > > > >     IPMI system address. Now the address type is SYSTEM_INTERFACE_ADDR_TYPE
> > > > > > > >     IPMI net function & command
> > > > > > > >     IPMI message payload
> > > > > > > >     2. send the IPMI message by using the function of ipmi_request_settime
> > > > > > > >     3. wait for the completion of IPMI message. It can be done in different
> > > > > > > > routes: One is in handled in IPMI user recv callback function. Another is
> > > > > > > > handled in timeout function.
> > > > > > > >     4. format the IPMI response and return it to ACPI AML code.
> > > > > > > 
> > > > > > > I think this should be part of the ipmi_si_intf module, not a separate
> > > > > > > module.  This opregion code is of no use without ipmi_si_intf.  The
> > > > > > > opregion code can still be under its own config option, but I don't
> > > > > > > see the value in having it be a separate module.
> > > > > > 
> > > > > > This opregion code can't work if the IPMI subsystem is not loaded. this
> > > > > > is right. I also agree with this point.
> > > > > > But this doesn't mean that this must be put into the module of
> > > > > > ipmi_si_intf.
> > > > > > In fact this opregion code only uses the IPMI API function to
> > > > > > communicate with IPMI system interface. At the same time its main
> > > > > > purpose is to handle the request from ACPI AML code. It had better be
> > > > > > put into the ACPI subsystem. I think that it is unnecessary to put it
> > > > > > into the ipmi_si_intf module.
> > > > > > 
> > > > > > 
> > > > > > BTW: Several modules(for example: drivers/hwmon/ibmaem.c ,
> > > > > > drivers/hwmon/ibmpex.c) also use the IPMI system interface to
> > > > > > communicate with the IPMI subsystem. But they are the different part of
> > > > > > ipmi_si_intf module.
> > > > > > 
> > > > > > > The opregion code should definitely not use acpi_bus_register_driver()
> > > > > > > to claim IPI0001 devices.  The ipmi_si_intf driver already uses
> > > > > > > pnp_register_driver() to claim IPI0001 devices.  The fact that Linux
> > > > > > > allows two drivers to claim the same device (one via pnp_register_driver()
> > > > > > > and the other via acpi_bus_register_driver()) is a defect in Linux, and
> > > > > > > we shouldn't take advantage of that.
> > > > > > 
> > > > > > If you think that this code had better not use the
> > > > > > acpi_bus_register_driver, I will try to enumerate this device from ACPI
> > > > > > device tree directly and then install the IPMI opregion handler for it.
> > > > > 
> > > > > I think there are two questions here: (1) should this be part of
> > > > > ipmi_si_intf? and (2) how should it be registered?
> > > > > 
> > > > > For (1), I see your point that we do have existing modules that are
> > > > > separate modules, and this is conceptually similar to those.  So
> > > > > maybe this should be a separate module, too.  If they're separate
> > > > > modules, I think the issues are how they communicate and how we get
> > > > > the opregion part autoloaded.  It's easy to see how ipmi_si_intf
> > > > > gets autoloaded -- the PNP or PCI core discovers a device and emits
> > > > > a uevent that udev can match with a driver.  I don't think we want
> > > > > the user to have to explicitly modprobe the opregion driver, so we
> > > > > need some uevent or similar hook that udev can use to load it
> > > > > automatically when appropriate.
> > > > 
> > > > From the source code it seems that the ipmi_si_intf module only detects
> > > > the available BMC device. It doesn't care which part communicates with
> > > > the BMC device. So it will be better to put this code out of
> > > > ipmi_si_intf module. 
> > > > 
> > > > Whether the opregion code should used depends on the user configuration.
> > > > If we expect that the ACPI aml code can communicate with the BMC, we
> > > > will select this part. Correspondingly the IPMI subsystem will also be
> > > > selected as this part depends on the IPMI subsystem. If the user doesn't
> > > > care whether the AML code can communicate with the BMC, they can leave
> > > > it alone. If we expect to use the uevent, that is not related with this
> > > > patch.
> > > 
> > > I'm sorry, I don't understand what you're saying here.  I expect the
> > > opregion module to be generally useful, so distros will probably enable
> > > the config option, so the module will be present on every machine.  How
> > > do you plan to have it loaded?  We need some sort of automatic
> > > mechanism; the user should not have to decide whether to load the
> > > module.
> > 
> > What I said is that user needs to decide whether the IPMI opregion
> > module should be used when building the kernel. If we want to use it, we
> > will have to select it in kernel configuration.
> 
> Yes.  During Kconfig, the user (or in most cases, the distro) selects
> Y/N/M.
> 
> > And then system will 
> > load it in the boot time. Right?
> 
> Distros will typically select "M", so we build a module.  The module
> is not part of the static kernel image.  It will not be loaded unless
> we make some arrangement for it to be loaded.  I'm asking what arrangement
> you propose.
> 
> > If not right, do we need care the case 
> > that this opregion code is not loaded automatically in the boot time?
> 
> If the opregion code is a module, it will not be loaded until some
> event causes it to be loaded.  For example, for PCI drivers, the PCI
> core emits a uevent when it discovers a PCI device.  The uevent contains
> the vendor/device ID, and udev can match that information with a driver
> that claims the device, and load the driver automatically.  Another
> way is for the user to type "modprobe <driver>" manually.  Another is
> for the user to set up /etc/modprobe.conf so the driver is loaded
> automatically by an init script.
> 
> I think the last two possibilities ((1) manual modprobe, (2) edit
> /etc/modprobe.conf) are poor choices because they require the user
> to do too much work.
> 
> The kernel already emits uevents for ACPI/PNP devices, and udev
> knows how to load matching drivers.  This mechanism is enough to
> cause ipmi_si_intf to be loaded automatically, and that gives us a
> good hook for the opregion piece as well.
> 
> We want to attach the opregion driver exactly when ipmi_si_intf
> claims an ACPI device, so integrating this code (or at least a call
> to it) in ipmi_si_intf seems natural.  It's also important to *detach*
> the opregion handler at the appropriate time (e.g., when the IPMI
> interface is removed or the ipmi_si_intf driver is unloaded).  Again,
> tight integration in ipmi_si_intf solves the detach problem nicely.

Thanks for pointing out this issue. 
Then I will try to add this code into the ipmi_si_intf module.

> 
> > If you think that it is ok to enable the opregion part as soon as we
> > finish the detection of IPMI system interfaces, I can add the opregion
> > code into the ipmi_si_intf part. But if so, maybe we can use the ACPI
> > device driver to detect the BMC controller defined in ACPI namespace
> > instead of using PNP device driver. At the same time this mechanism
> > should be used firstly.
> > 
> > Is this OK to you?
> 
> I'm sorry if I'm being repetitious or not understanding you correctly.
> 
> My recent patches add PNP detection to ipmi_si_intf.c.  I think it would
> be a good idea to add the opregion setup to ipmi_pnp_probe().  I think
> it would be a bad idea to change ipmi_si_intf.c to use acpi_walk_namespace()
> or acpi_bus_register_driver().
> 
> > > > > For (2), I don't think using acpi_walk_namespace() to determine when
> > > > > to install the IPMI opregion handler is quite the right thing either.
> > > 
> > > > > This opregion handler allows AML to use IPMI services.  Using
> > > > > acpi_walk_namespace() means that we'll only install the handler
> > > > > when we have an ACPI IPMI device.  But I think the opregion should
> > > > > be independent of the type of IPMI device, i.e., I think we want
> > > > > this opregion to work even if the underlying IPMI interface is PCI.
> > > > 
> > > > This opregion code is only to enable the ACPI AML code to communicate
> > > > with the BMC device. If AML code wants to access the BMC controller, we
> > > > must declare the corresponding IPMI device in ACPI namespace and add the
> > > > opregion field definition. If the IPMI device is not declared explicitly
> > > > in ACPI table, this code won't work even when there exists the BMC
> > > > device on the system.
> > > 
> > > Oops, sorry, I see that you are right: the ACPI 4.0 spec, sec 5.5.2.4.3,
> > > Declaring IPMI Operation Regions, is indeed restricted so it only works
> > > with ACPI IPMI devices.  That is an unfortunate restriction on the
> > > platform architecture, but maybe there's a good reason for it.
> > > 
> > > That section says that operation regions are defined within the
> > > immediate context of the 'owning' IPMI device.  This, together with the
> > > fact that the opregion support is only useful for ACPI devices, suggests
> > > to me that the opregion module is different from the ibmaem and ibmpex
> > > modules after all.  Those modules apparently can work with *any* sort of
> > > IPMI device, so they are more independent.
> > 
> > The IPMI opregion code is only used for the IPMI device that defined in
> > ACPI namespace. If one IPMI device is not defined in ACPI namespace,
> > then we won't install IPMI opregion space handler for it. Corresponding
> > the ACPI AML code won't communicate with it.
> 
> If we want the opregion code only for ACPI devices, the easiest way
> to accomplish this is to set it up when we discover the ACPI device.
> It's much harder to do it later, because you have to look through all
> the IPMI interfaces and figure out which ones are ACPI devices.

To discover the ACPI device happens very early. So it is inappropriate
to install the IPMI opregion space handler for ACPI in the phase of
discovering the ACPI device. 

But We can put the ACPI detection mechanism in the first order and we
can record which IPMI smi_info is registered by ACPI mechanism. Then we
can install the IPMI opregion space handler for the corresponding IPMI
device.

Thanks.
    Yakui
> 
> Bjorn
> 
> 

Re: [Resend PATCH] ACPI: Add the IPMI opregion driver to enable ACPI to access BMC controller

[Bjorn Helgaas]

On Tuesday 15 December 2009 06:08:30 pm ykzhao wrote:
> On Tue, 2009-12-15 at 23:27 +0800, Bjorn Helgaas wrote:

> > If we want the opregion code only for ACPI devices, the easiest way
> > to accomplish this is to set it up when we discover the ACPI device.
> > It's much harder to do it later, because you have to look through all
> > the IPMI interfaces and figure out which ones are ACPI devices.
> 
> To discover the ACPI device happens very early. So it is inappropriate
> to install the IPMI opregion space handler for ACPI in the phase of
> discovering the ACPI device. 
> 
> But We can put the ACPI detection mechanism in the first order and we
> can record which IPMI smi_info is registered by ACPI mechanism. Then we
> can install the IPMI opregion space handler for the corresponding IPMI
> device.

ipmi_pnp_probe() does not happen very early.  If ipmi_si_intf.c is built
statically, it happens during driver init; otherwise it happens when the
module is loaded.

Either way, I think ipmi_si_intf.c will discover the IPI0001 device
after all the Linux/ACPI core stuff is initialized, so I don't see the
problem with installing the opregion handler immediately.  If you see
a specific problem, please point it out.

Bjorn