[PATCH] IPMI driver updates

Some people found some bugs and some missing functions in the IPMI driver,
so I have patching things together for the next release.  The attached
patch moves to version 33 of the driver and contains:

* SMBIOS table support for specifying register spacing.  This allows
  non-contiguous registers to be specified and some machines do
  this.
* ACPI table updates to support all the possible register sizes and
  bit offsets into the registers for the IPMI information.
* Support for command line parameters to specify register
  spacing, sizes, and bit offsets.
* Support for power control with IPMI.  This allows a halt to
  power down a machine with IPMI.
* A fix for a race condition with interrupts enabled on an
  SMP machine.  A lock was released then reclaimed, but
  there was code later that assumed that had not happened.
* A fix for protecting the driver against bad responses from
  the controller chip.  In the past, the driver had assumed that
  the controller chip would not give it bad data.  This has
  turned out to be a bad assumption
* ACPI interrupt handlers now return a return value, adjust
  accordingly.

Thank you to all the people who helped me with this.
Signed-off-by: Corey Minyard <minyard@acm.org>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

Documentation/IPMI.txt

@@ -340,6 +340,8 @@ You can change this at module load time (for a module) with:
   modprobe ipmi_si.o type=<type1>,<type2>....
        ports=<port1>,<port2>... addrs=<addr1>,<addr2>...
        irqs=<irq1>,<irq2>... trydefaults=[0|1]
+       regspacings=<sp1>,<sp2>,... regsizes=<size1>,<size2>,...
+       regshifts=<shift1>,<shift2>,...
 
 Each of these except si_trydefaults is a list, the first item for the
 first interface, second item for the second interface, etc.
@@ -361,12 +363,35 @@ si_trydefaults sets whether the standard IPMI interface at 0xca2 and
 any interfaces specified by ACPE are tried.  By default, the driver
 tries it, set this value to zero to turn this off.
 
+The next three parameters have to do with register layout.  The
+registers used by the interfaces may not appear at successive
+locations and they may not be in 8-bit registers.  These parameters
+allow the layout of the data in the registers to be more precisely
+specified.
+
+The regspacings parameter give the number of bytes between successive
+register start addresses.  For instance, if the regspacing is set to 4
+and the start address is 0xca2, then the address for the second
+register would be 0xca6.  This defaults to 1.
+
+The regsizes parameter gives the size of a register, in bytes.  The
+data used by IPMI is 8-bits wide, but it may be inside a larger
+register.  This parameter allows the read and write type to specified.
+It may be 1, 2, 4, or 8.  The default is 1.
+
+Since the register size may be larger than 32 bits, the IPMI data may not
+be in the lower 8 bits.  The regshifts parameter give the amount to shift
+the data to get to the actual IPMI data.
+
 When compiled into the kernel, the addresses can be specified on the
 kernel command line as:
 
   ipmi_si.type=<type1>,<type2>...
        ipmi_si.ports=<port1>,<port2>... ipmi_si.addrs=<addr1>,<addr2>...
        ipmi_si.irqs=<irq1>,<irq2>... ipmi_si.trydefaults=[0|1]
+       ipmi_si.regspacings=<sp1>,<sp2>,...
+       ipmi_si.regsizes=<size1>,<size2>,...
+       ipmi_si.regshifts=<shift1>,<shift2>,...
 
 It works the same as the module parameters of the same names.
 

drivers/char/ipmi/Kconfig

@@ -57,4 +57,11 @@ config IPMI_WATCHDOG
        help
          This enables the IPMI watchdog timer.
 
+config IPMI_POWEROFF
+       tristate 'IPMI Poweroff'
+       depends on IPMI_HANDLER
+       help
+         This enables a function to power off the system with IPMI if
+	 the IPMI management controller is capable of this.
+
 endmenu

drivers/char/ipmi/Makefile

@@ -8,6 +8,7 @@ obj-$(CONFIG_IPMI_HANDLER) += ipmi_msghandler.o
 obj-$(CONFIG_IPMI_DEVICE_INTERFACE) += ipmi_devintf.o
 obj-$(CONFIG_IPMI_SI) += ipmi_si.o
 obj-$(CONFIG_IPMI_WATCHDOG) += ipmi_watchdog.o
+obj-$(CONFIG_IPMI_POWEROFF) += ipmi_poweroff.o
 
 ipmi_si.o:	$(ipmi_si-objs)
 	$(LD) -r -o $@ $(ipmi_si-objs)

drivers/char/ipmi/ipmi_bt_sm.c

@@ -31,7 +31,7 @@
 #include <linux/ipmi_msgdefs.h>		/* for completion codes */
 #include "ipmi_si_sm.h"
 
-#define IPMI_BT_VERSION "v32"
+#define IPMI_BT_VERSION "v33"
 
 static int bt_debug = 0x00;	/* Production value 0, see following flags */
 

drivers/char/ipmi/ipmi_devintf.c

@@ -45,7 +45,7 @@
 #include <asm/semaphore.h>
 #include <linux/init.h>
 
-#define IPMI_DEVINTF_VERSION "v32"
+#define IPMI_DEVINTF_VERSION "v33"
 
 struct ipmi_file_private
 {

drivers/char/ipmi/ipmi_kcs_sm.c

@@ -42,7 +42,7 @@
 #include <linux/ipmi_msgdefs.h>		/* for completion codes */
 #include "ipmi_si_sm.h"
 
-#define IPMI_KCS_VERSION "v32"
+#define IPMI_KCS_VERSION "v33"
 
 /* Set this if you want a printout of why the state machine was hosed
    when it gets hosed. */

drivers/char/ipmi/ipmi_msghandler.c

@@ -46,7 +46,8 @@
 #include <linux/init.h>
 #include <linux/proc_fs.h>
 
-#define IPMI_MSGHANDLER_VERSION "v32"
+#define PFX "IPMI message handler: "
+#define IPMI_MSGHANDLER_VERSION "v33"
 
 struct ipmi_recv_msg *ipmi_alloc_recv_msg(void);
 static int ipmi_init_msghandler(void);
@@ -895,6 +896,12 @@ int ipmi_unregister_for_cmd(ipmi_user_t   user,
 	return rv;
 }
 
+void ipmi_user_set_run_to_completion(ipmi_user_t user, int val)
+{
+	user->intf->handlers->set_run_to_completion(user->intf->send_info,
+						    val);
+}
+
 static unsigned char
 ipmb_checksum(unsigned char *data, int size)
 {
@@ -1686,8 +1693,8 @@ channel_handler(ipmi_smi_t intf, struct ipmi_smi_msg *msg)
 			intf->curr_channel = IPMI_MAX_CHANNELS;
 			wake_up(&intf->waitq);
 
-			printk(KERN_WARNING "ipmi_msghandler: Error sending"
-			       "channel information: %d\n",
+			printk(KERN_WARNING PFX
+			       "Error sending channel information: %d\n",
 			       rv);
 		}
 	}
@@ -2351,7 +2358,7 @@ static int handle_read_event_rsp(ipmi_smi_t          intf,
 	} else {
 		/* There's too many things in the queue, discard this
 		   message. */
-		printk(KERN_WARNING "ipmi: Event queue full, discarding an"
+		printk(KERN_WARNING PFX "Event queue full, discarding an"
 		       " incoming event\n");
 	}
 
@@ -2433,8 +2440,32 @@ static int handle_new_recv_msg(ipmi_smi_t          intf,
 #endif
 	if (msg->rsp_size < 2) {
 		/* Message is too small to be correct. */
-		requeue = 0;
-	} else if ((msg->rsp[0] == ((IPMI_NETFN_APP_REQUEST|1) << 2))
+		printk(KERN_WARNING PFX "BMC returned to small a message"
+		       " for netfn %x cmd %x, got %d bytes\n",
+		       (msg->data[0] >> 2) | 1, msg->data[1], msg->rsp_size);
+
+		/* Generate an error response for the message. */
+		msg->rsp[0] = msg->data[0] | (1 << 2);
+		msg->rsp[1] = msg->data[1];
+		msg->rsp[2] = IPMI_ERR_UNSPECIFIED;
+		msg->rsp_size = 3;
+	} else if (((msg->rsp[0] >> 2) != ((msg->data[0] >> 2) | 1))/* Netfn */
+		   || (msg->rsp[1] != msg->data[1]))		  /* Command */
+	{
+		/* The response is not even marginally correct. */
+		printk(KERN_WARNING PFX "BMC returned incorrect response,"
+		       " expected netfn %x cmd %x, got netfn %x cmd %x\n",
+		       (msg->data[0] >> 2) | 1, msg->data[1],
+		       msg->rsp[0] >> 2, msg->rsp[1]);
+
+		/* Generate an error response for the message. */
+		msg->rsp[0] = msg->data[0] | (1 << 2);
+		msg->rsp[1] = msg->data[1];
+		msg->rsp[2] = IPMI_ERR_UNSPECIFIED;
+		msg->rsp_size = 3;
+	}
+
+	if ((msg->rsp[0] == ((IPMI_NETFN_APP_REQUEST|1) << 2))
 	    && (msg->rsp[1] == IPMI_SEND_MSG_CMD)
 	    && (msg->user_data != NULL))
 	{
@@ -2502,7 +2533,9 @@ static int handle_new_recv_msg(ipmi_smi_t          intf,
 			requeue = 0;
 		}
 
-	} else if (msg->rsp[1] == IPMI_READ_EVENT_MSG_BUFFER_CMD) {
+	} else if ((msg->rsp[0] == ((IPMI_NETFN_APP_REQUEST|1) << 2))
+		   && (msg->rsp[1] == IPMI_READ_EVENT_MSG_BUFFER_CMD))
+	{
 		/* It's an asyncronous event. */
 		requeue = handle_read_event_rsp(intf, msg);
 	} else {
@@ -3114,7 +3147,7 @@ static int ipmi_init_msghandler(void)
 
 	proc_ipmi_root = proc_mkdir("ipmi", NULL);
 	if (!proc_ipmi_root) {
-	    printk("Unable to create IPMI proc dir");
+	    printk(KERN_ERR PFX "Unable to create IPMI proc dir");
 	    return -ENOMEM;
 	}
 
@@ -3166,11 +3199,11 @@ static __exit void cleanup_ipmi(void)
 	/* Check for buffer leaks. */
 	count = atomic_read(&smi_msg_inuse_count);
 	if (count != 0)
-		printk("ipmi_msghandler: SMI message count %d at exit\n",
+		printk(KERN_WARNING PFX "SMI message count %d at exit\n",
 		       count);
 	count = atomic_read(&recv_msg_inuse_count);
 	if (count != 0)
-		printk("ipmi_msghandler: recv message count %d at exit\n",
+		printk(KERN_WARNING PFX "recv message count %d at exit\n",
 		       count);
 }
 module_exit(cleanup_ipmi);
@@ -3207,3 +3240,4 @@ EXPORT_SYMBOL(ipmi_get_my_address);
 EXPORT_SYMBOL(ipmi_set_my_LUN);
 EXPORT_SYMBOL(ipmi_get_my_LUN);
 EXPORT_SYMBOL(ipmi_smi_add_proc_entry);
+EXPORT_SYMBOL(ipmi_user_set_run_to_completion);

drivers/char/ipmi/ipmi_si_sm.h

@@ -52,6 +52,9 @@ struct si_sm_io
            state machine shouldn't touch these. */
 	void *info;
 	void *addr;
+	int  regspacing;
+	int  regsize;
+	int  regshift;
 };
 
 /* Results of SMI events. */

drivers/char/ipmi/ipmi_smic_sm.c

@@ -46,7 +46,7 @@
 #include <linux/ipmi_msgdefs.h>		/* for completion codes */
 #include "ipmi_si_sm.h"
 
-#define IPMI_SMIC_VERSION "v32"
+#define IPMI_SMIC_VERSION "v33"
 
 /* smic_debug is a bit-field
  *	SMIC_DEBUG_ENABLE -	turned on for now

drivers/char/ipmi/ipmi_watchdog.c

@@ -53,7 +53,7 @@
 
 #define	PFX "IPMI Watchdog: "
 
-#define IPMI_WATCHDOG_VERSION "v32"
+#define IPMI_WATCHDOG_VERSION "v33"
 
 /*
  * The IPMI command/response information for the watchdog timer.

include/linux/ipmi.h

@@ -405,6 +405,12 @@ int ipmi_unregister_for_cmd(ipmi_user_t   user,
 			    unsigned char netfn,
 			    unsigned char cmd);
 
+/*
+ * Allow run-to-completion mode to be set for the interface of
+ * a specific user.
+ */
+void ipmi_user_set_run_to_completion(ipmi_user_t user, int val);
+
 /*
  * When the user is created, it will not receive IPMI events by
  * default.  The user must set this to TRUE to get incoming events.