[RFC] ACPI IPMI changes

[RFC] ACPI IPMI changes

[Matthew Garrett]

The ACPI IPMI code doesn't seem to work on many real-world devices. Part of
that is because many shipping systems have a _CID of PNP0C01 or PNP0C02 and
the PNP core never lets the IPMI driver bind, but it's also a problem if
it's being used for AC adapter state (since they're typically built in)
and on some systems that have IPMI opregions declared outside the IPMI
device's scope (spec violation, but what else is new). These patches don't
do anything for the first problem (that's going to have to be handled in
the PNP core) but handle the latter two by ensuring that IPMI can be
initialised before any built-in ACPI drivers, and by providing support for
a fallback IPMI handler that just uses the first IPMI device in the system.

-- 
Matthew Garrett | mjg59@srcf.ucam.org

[PATCH 1/4] ACPI: Reorder IPMI driver before any other ACPI drivers

[Matthew Garrett]

Drivers may make calls that require the ACPI IPMI driver to have been
initialised already, so make sure that it appears earlier in the build
order.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/acpi/Makefile | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 47199e2..82422fe 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -47,6 +47,10 @@ acpi-y				+= video_detect.o
 endif
 
 # These are (potentially) separate modules
+
+# IPMI may be used by other drivers, so it has to initialise before them
+obj-$(CONFIG_ACPI_IPMI)		+= acpi_ipmi.o
+
 obj-$(CONFIG_ACPI_AC) 		+= ac.o
 obj-$(CONFIG_ACPI_BUTTON)	+= button.o
 obj-$(CONFIG_ACPI_FAN)		+= fan.o
@@ -70,6 +74,5 @@ processor-y			+= processor_idle.o processor_thermal.o
 processor-$(CONFIG_CPU_FREQ)	+= processor_perflib.o
 
 obj-$(CONFIG_ACPI_PROCESSOR_AGGREGATOR) += acpi_pad.o
-obj-$(CONFIG_ACPI_IPMI)		+= acpi_ipmi.o
 
 obj-$(CONFIG_ACPI_APEI)		+= apei/
-- 
1.7.11.4

[PATCH 2/4] IPMI: Change link order

[Matthew Garrett]

IPMI must be initialised before ACPI in order to ensure that any IPMI
services are available before ACPI driver initialisation attempts to use
any IPMI operation regions.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/Makefile      | 4 ++++
 drivers/char/Makefile | 1 -
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/Makefile b/drivers/Makefile
index 5b42184..9d26b04 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -14,6 +14,10 @@ obj-$(CONFIG_PARISC)		+= parisc/
 obj-$(CONFIG_RAPIDIO)		+= rapidio/
 obj-y				+= video/
 obj-y				+= idle/
+
+# IPMI must come before ACPI in order to provide IPMI opregion support
+obj-$(CONFIG_IPMI_HANDLER)	+= char/ipmi/
+
 obj-$(CONFIG_ACPI)		+= acpi/
 obj-$(CONFIG_SFI)		+= sfi/
 # PnP must come after ACPI since it will eventually need to check if acpi
diff --git a/drivers/char/Makefile b/drivers/char/Makefile
index d0b27a3..7ff1d0d 100644
--- a/drivers/char/Makefile
+++ b/drivers/char/Makefile
@@ -52,7 +52,6 @@ obj-$(CONFIG_TELCLOCK)		+= tlclk.o
 obj-$(CONFIG_MWAVE)		+= mwave/
 obj-$(CONFIG_AGP)		+= agp/
 obj-$(CONFIG_PCMCIA)		+= pcmcia/
-obj-$(CONFIG_IPMI_HANDLER)	+= ipmi/
 
 obj-$(CONFIG_HANGCHECK_TIMER)	+= hangcheck-timer.o
 obj-$(CONFIG_TCG_TPM)		+= tpm/
-- 
1.7.11.4

[PATCH 3/4] IPMI: Add a callback to indicate that probing has finished

[Matthew Garrett]

Some IPMI callbacks may want to know how many IPMI devices were registered
or perform some specific action after probing has been completed. Add a
new callback to handle that.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/char/ipmi/ipmi_msghandler.c | 15 +++++++++++++++
 drivers/char/ipmi/ipmi_si_intf.c    |  6 +++++-
 include/linux/ipmi.h                |  3 ++-
 include/linux/ipmi_smi.h            |  5 +++++
 4 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 2c29942..3aacb6d 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -571,6 +571,9 @@ int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher)
 		kfree(e);
 	}
 
+	if (watcher->smi_probe_complete)
+		watcher->smi_probe_complete();
+
 	mutex_unlock(&smi_watchers_mutex);
 
 	return 0;
@@ -2807,6 +2810,18 @@ void ipmi_poll_interface(ipmi_user_t user)
 }
 EXPORT_SYMBOL(ipmi_poll_interface);
 
+void ipmi_smi_probe_complete(void)
+{
+	struct ipmi_smi_watcher *w;
+
+	mutex_lock(&smi_watchers_mutex);
+	list_for_each_entry(w, &smi_watchers, link) {
+		if (w->smi_probe_complete)
+			w->smi_probe_complete();
+	}
+	mutex_unlock(&smi_watchers_mutex);
+}
+
 int ipmi_register_smi(struct ipmi_smi_handlers *handlers,
 		      void		       *send_info,
 		      struct ipmi_device_id    *device_id,
diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 83f85cf..75e3c4f 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -3408,6 +3408,7 @@ static int __devinit init_ipmi_si(void)
 	/* type will only have been set if we successfully registered an si */
 	if (type) {
 		mutex_unlock(&smi_infos_lock);
+		ipmi_smi_probe_complete();
 		return 0;
 	}
 
@@ -3422,8 +3423,10 @@ static int __devinit init_ipmi_si(void)
 	}
 	mutex_unlock(&smi_infos_lock);
 
-	if (type)
+	if (type) {
+		ipmi_smi_probe_complete();
 		return 0;
+	}
 
 	if (si_trydefaults) {
 		mutex_lock(&smi_infos_lock);
@@ -3444,6 +3447,7 @@ static int __devinit init_ipmi_si(void)
 		return -ENODEV;
 	} else {
 		mutex_unlock(&smi_infos_lock);
+		ipmi_smi_probe_complete();
 		return 0;
 	}
 }
diff --git a/include/linux/ipmi.h b/include/linux/ipmi.h
index 48dcba9..6c8faaa 100644
--- a/include/linux/ipmi.h
+++ b/include/linux/ipmi.h
@@ -435,12 +435,13 @@ struct ipmi_smi_watcher {
 	   a module (generally just set it to "THIS_MODULE"). */
 	struct module *owner;
 
-	/* These two are called with read locks held for the interface
+	/* These three are called with read locks held for the interface
 	   the watcher list.  So you can add and remove users from the
 	   IPMI interface, send messages, etc., but you cannot add
 	   or remove SMI watchers or SMI interfaces. */
 	void (*new_smi)(int if_num, struct device *dev);
 	void (*smi_gone)(int if_num);
+	void (*smi_probe_complete)(void);
 };
 
 int ipmi_smi_watcher_register(struct ipmi_smi_watcher *watcher);
diff --git a/include/linux/ipmi_smi.h b/include/linux/ipmi_smi.h
index fcb5d44..56abc7c 100644
--- a/include/linux/ipmi_smi.h
+++ b/include/linux/ipmi_smi.h
@@ -215,6 +215,11 @@ int ipmi_register_smi(struct ipmi_smi_handlers *handlers,
 int ipmi_unregister_smi(ipmi_smi_t intf);
 
 /*
+ * Indicate to the IPMI driver that probing has been completed
+ */
+void ipmi_smi_probe_complete(void);
+
+/*
  * The lower layer reports received messages through this interface.
  * The data_size should be zero if this is an asyncronous message.  If
  * the lower layer gets an error sending a message, it should format
-- 
1.7.11.4

[PATCH 4/4] ACPI: Add a default handler for IPMI operation regions

[Matthew Garrett]

The ACPI spec makes it clear that IPMI operation regions should be declared
inside the scope of an IPMI device. Based on the existence of systems for
which this is untrue, it seems likely that alternative implementations exist
that perform some kind of fallback for regions outside the scope of an IPMI
device. Add a callback to the ACPI IPMI driver to glue these operation
regions onto an IPMI device. Behaviour in the case of multiple controllers
may be unpredictable, but there's clearly no way to know the correct answer
in that case.

Signed-off-by: Matthew Garrett <mjg@redhat.com>
---
 drivers/acpi/acpi_ipmi.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c
index f40acef..15dc0e8 100644
--- a/drivers/acpi/acpi_ipmi.c
+++ b/drivers/acpi/acpi_ipmi.c
@@ -72,6 +72,7 @@ struct ipmi_driver_data {
 	struct ipmi_smi_watcher	bmc_events;
 	struct ipmi_user_hndl	ipmi_hndlrs;
 	struct mutex		ipmi_lock;
+	struct acpi_ipmi_device *global_device;
 };
 
 struct acpi_ipmi_msg {
@@ -106,9 +107,14 @@ struct acpi_ipmi_buffer {
 
 static void ipmi_register_bmc(int iface, struct device *dev);
 static void ipmi_bmc_gone(int iface);
+static void ipmi_probe_complete(void);
 static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data);
 static void acpi_add_ipmi_device(struct acpi_ipmi_device *ipmi_device);
 static void acpi_remove_ipmi_device(struct acpi_ipmi_device *ipmi_device);
+static acpi_status
+acpi_ipmi_space_handler(u32 function, acpi_physical_address address,
+			u32 bits, acpi_integer *value,
+			void *handler_context, void *region_context);
 
 static struct ipmi_driver_data driver_data = {
 	.ipmi_devices = LIST_HEAD_INIT(driver_data.ipmi_devices),
@@ -116,6 +122,7 @@ static struct ipmi_driver_data driver_data = {
 		.owner = THIS_MODULE,
 		.new_smi = ipmi_register_bmc,
 		.smi_gone = ipmi_bmc_gone,
+		.smi_probe_complete = ipmi_probe_complete,
 	},
 	.ipmi_hndlrs = {
 		.ipmi_recv_hndl = ipmi_msg_handler,
@@ -276,6 +283,32 @@ static void ipmi_msg_handler(struct ipmi_recv_msg *msg, void *user_msg_data)
 	ipmi_free_recv_msg(msg);
 };
 
+/* Must be called with driver_data.ipmi_lock held */
+static void ipmi_update_global_handler(void)
+{
+	struct acpi_ipmi_device *ipmi_device;
+	acpi_status status;
+
+	if (driver_data.global_device)
+		return;
+
+	list_for_each_entry(ipmi_device, &driver_data.ipmi_devices, head) {
+		/*
+		 * Register a handler for IPMI regions that aren't under a
+		 * specific IPMI namespace. This appears to be a spec violation
+		 * but we should do what we can.
+		 */
+		status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT,
+						      ACPI_ADR_SPACE_IPMI,
+						      &acpi_ipmi_space_handler,
+						      NULL, ipmi_device);
+		if (ACPI_SUCCESS(status)) {
+			driver_data.global_device = ipmi_device;
+			break;
+		}
+	}
+}
+
 static void ipmi_register_bmc(int iface, struct device *dev)
 {
 	struct acpi_ipmi_device *ipmi_device, *temp;
@@ -347,12 +380,29 @@ static void ipmi_bmc_gone(int iface)
 			continue;
 
 		acpi_remove_ipmi_device(ipmi_device);
+
+		if (driver_data.global_device == ipmi_device) {
+			acpi_remove_address_space_handler(ACPI_ROOT_OBJECT,
+				ACPI_ADR_SPACE_IPMI, &acpi_ipmi_space_handler);
+			driver_data.global_device = NULL;
+			ipmi_update_global_handler();
+		}
+
 		put_device(ipmi_device->smi_data.dev);
 		kfree(ipmi_device);
 		break;
 	}
+
 	mutex_unlock(&driver_data.ipmi_lock);
 }
+
+static void ipmi_probe_complete(void)
+{
+	mutex_lock(&driver_data.ipmi_lock);
+	ipmi_update_global_handler();
+	mutex_unlock(&driver_data.ipmi_lock);
+}
+
 /* --------------------------------------------------------------------------
  *			Address Space Management
  * -------------------------------------------------------------------------- */
-- 
1.7.11.4