All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Ferenc Wagner <wferi@niif.hu>
Cc: linux-kernel@vger.kernel.org,
	openipmi-developer@lists.sourceforge.net,
	Corey Minyard <minyard@acm.org>, dann frazier <dannf@hp.com>
Subject: Re: modprobe ipmi_si hangs under 2.6.30-rc5
Date: Sat, 16 May 2009 17:09:59 -0700	[thread overview]
Message-ID: <20090516170959.8057c9e4.akpm@linux-foundation.org> (raw)
In-Reply-To: <87tz3qnh7o.fsf@tac.ki.iif.hu>

[-- Attachment #1: Type: text/plain, Size: 2719 bytes --]

On Tue, 12 May 2009 23:28:59 +0200 Ferenc Wagner <wferi@niif.hu> wrote:

> Hi,
> 
> On a Dell PowerEdge 2650 2.6.30-rc5 can't load the ipmi_si module.
> 2.6.29 has no such problems:
> 
> [   46.292160] IPMI System Interface driver.
> [   46.300321] ipmi_si: Trying SMBIOS-specified smic state machine at i/o address 0xecf4, slave address 0x20, irq 0
> [   46.408212] ipmi: Found new BMC (man_id: 0x0002a2,  prod_id: 0x0000, dev_id: 0x00)
> [   46.423454] IPMI smic interface initialized
> 
> But under 2.6.30-rc5:
> 
> # modprobe ipmi_si bt_debug=7 smic_debug=7 kcs_debug=7
> 
> [  867.851694] IPMI System Interface driver.
> [  867.859771] ipmi_si: Trying SMBIOS-specified smic state machine at i/o address 0xecf4, slave address 0x20, irq 0
> [  867.880250] start_smic_transaction - 18 01
> [  867.880263] smic_event - smic->smic_timeout = 2000000, time = 0
> ...
>
> [  867.996243] smic_event - smic->smic_timeout = 2000000, time = 0
> [  867.996250] smic_event - smic->smic_timeout = 2000000, time = 0
> [  867.996256] smic_event - smic->smic_timeout = 2000000, time = 0
> [...]
> 
> 
> This goes on through megabytes.  The modprobe process didn't terminate
> for 10 minutes, and there was some output like this (cleaned from
> smic_events all around the place by hand):
> 
> INFO: task modprobe:6414 blocked for more than 120 seconds.
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
> modprobe      D f6805780 0  6414   4077
>  f48dfe68 00000082 00000000 f6805780
>  f48dfe48 f90271cf c046acc0 f64f6a0c
>  2002132c f6da1300 f6da14b4 c182acc0
>  00000002 f64f6a0c 186f5ef2 000000ca
>  f48dfe30 c0123c40 00000000 f64f69e0
>  f48dfe30 00000282 00000001 c032456c
> Call Trace:
>  [<f90271cf>] ? i_ipmi_request+0x8f5/0x916 [ipmi_msghandler]
>  [<c0123c40>] ? check_preempt_wakeup+0x146/0x180
>  [<c01292c3>] ? try_to_wake_up+0x1f3/0x1fd
>  [<c011c70a>] ? default_spin_lock_flags+0x8/0xe
>  [<c031b81a>] schedule+0x8/0x17
>  [<f9028745>] ipmi_register_smi+0x3ca/0xba1 [ipmi_msghandler]
>  [<c028681c>] ? put_device+0xf/0x11
>  [<c028782a>] ? device_add+0x497/0x516
>  [<c013f2e7>] ? autoremove_wake_function+0x0/0x33
>  [<fc2d939c>] try_smi_init+0x58f/0x726 [ipmi_si]
>  [<fc2dc3e5>] init_ipmi_si+0x3e2/0x748 [ipmi_si]
>  [<c010304f>] do_one_initcall+0x4a/0x115
>  [<fc2dc003>] ? init_ipmi_si+0x0/0x748 [ipmi_si]
>  [<c0142c0b>] ? __blocking_notifier_call_chain+0x40/0x4c
>  [<c014ff97>] sys_init_module+0x87/0x18b
>  [<c0107c94>] sysenter_do_call+0x12/0x28
> 

Well there have only been a handful of changes to ipmi since 2.6.29. 
Could you try a mini-bisection?

Apply revert-1.patch, test
Apply revert-2.patch, test
Apply revert-3.patch, test
Apply revert-4.patch, test

Attached.

Thanks.

[-- Attachment #2: revert-1.patch --]
[-- Type: application/octet-stream, Size: 6339 bytes --]

From: Andrew Morton <akpm@linux-foundation.org>

commit 4dec302ff71ebf48f5784a2d2fc5e3745e6d4d52
Author: dann frazier <dannf@hp.com>
Date:   Tue Apr 21 12:24:05 2009 -0700

    ipmi: add oem message handling
    
    Enable userspace to receive messages that a BMC transmits using an OEM
    medium.  This is used by the HP iLO2.
    
    Based on code originally written by Patrick Schoeller.
    
    Signed-off-by: dann frazier <dannf@hp.com>
    Signed-off-by: Corey Minyard <cminyard@mvista.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/char/ipmi/ipmi_msghandler.c |  138 --------------------------
 include/linux/ipmi.h                |    2 
 include/linux/ipmi_msgdefs.h        |    2 
 3 files changed, 5 insertions(+), 137 deletions(-)

diff -puN drivers/char/ipmi/ipmi_msghandler.c~revert-1 drivers/char/ipmi/ipmi_msghandler.c
--- a/drivers/char/ipmi/ipmi_msghandler.c~revert-1
+++ a/drivers/char/ipmi/ipmi_msghandler.c
@@ -3284,114 +3284,6 @@ static int handle_lan_get_msg_cmd(ipmi_s
 	return rv;
 }
 
-/*
- * This routine will handle "Get Message" command responses with
- * channels that use an OEM Medium. The message format belongs to
- * the OEM.  See IPMI 2.0 specification, Chapter 6 and
- * Chapter 22, sections 22.6 and 22.24 for more details.
- */
-static int handle_oem_get_msg_cmd(ipmi_smi_t          intf,
-				  struct ipmi_smi_msg *msg)
-{
-	struct cmd_rcvr       *rcvr;
-	int                   rv = 0;
-	unsigned char         netfn;
-	unsigned char         cmd;
-	unsigned char         chan;
-	ipmi_user_t           user = NULL;
-	struct ipmi_system_interface_addr *smi_addr;
-	struct ipmi_recv_msg  *recv_msg;
-
-	/*
-	 * We expect the OEM SW to perform error checking
-	 * so we just do some basic sanity checks
-	 */
-	if (msg->rsp_size < 4) {
-		/* Message not big enough, just ignore it. */
-		ipmi_inc_stat(intf, invalid_commands);
-		return 0;
-	}
-
-	if (msg->rsp[2] != 0) {
-		/* An error getting the response, just ignore it. */
-		return 0;
-	}
-
-	/*
-	 * This is an OEM Message so the OEM needs to know how
-	 * handle the message. We do no interpretation.
-	 */
-	netfn = msg->rsp[0] >> 2;
-	cmd = msg->rsp[1];
-	chan = msg->rsp[3] & 0xf;
-
-	rcu_read_lock();
-	rcvr = find_cmd_rcvr(intf, netfn, cmd, chan);
-	if (rcvr) {
-		user = rcvr->user;
-		kref_get(&user->refcount);
-	} else
-		user = NULL;
-	rcu_read_unlock();
-
-	if (user == NULL) {
-		/* We didn't find a user, just give up. */
-		ipmi_inc_stat(intf, unhandled_commands);
-
-		/*
-		 * Don't do anything with these messages, just allow
-		 * them to be freed.
-		 */
-
-		rv = 0;
-	} else {
-		/* Deliver the message to the user. */
-		ipmi_inc_stat(intf, handled_commands);
-
-		recv_msg = ipmi_alloc_recv_msg();
-		if (!recv_msg) {
-			/*
-			 * We couldn't allocate memory for the
-			 * message, so requeue it for handling
-			 * later.
-			 */
-			rv = 1;
-			kref_put(&user->refcount, free_user);
-		} else {
-			/*
-			 * OEM Messages are expected to be delivered via
-			 * the system interface to SMS software.  We might
-			 * need to visit this again depending on OEM
-			 * requirements
-			 */
-			smi_addr = ((struct ipmi_system_interface_addr *)
-				    &(recv_msg->addr));
-			smi_addr->addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE;
-			smi_addr->channel = IPMI_BMC_CHANNEL;
-			smi_addr->lun = msg->rsp[0] & 3;
-
-			recv_msg->user = user;
-			recv_msg->user_msg_data = NULL;
-			recv_msg->recv_type = IPMI_OEM_RECV_TYPE;
-			recv_msg->msg.netfn = msg->rsp[0] >> 2;
-			recv_msg->msg.cmd = msg->rsp[1];
-			recv_msg->msg.data = recv_msg->msg_data;
-
-			/*
-			 * The message starts at byte 4 which follows the
-			 * the Channel Byte in the "GET MESSAGE" command
-			 */
-			recv_msg->msg.data_len = msg->rsp_size - 4;
-			memcpy(recv_msg->msg_data,
-			       &(msg->rsp[4]),
-			       msg->rsp_size - 4);
-			deliver_response(recv_msg);
-		}
-	}
-
-	return rv;
-}
-
 static void copy_event_into_recv_msg(struct ipmi_recv_msg *recv_msg,
 				     struct ipmi_smi_msg  *msg)
 {
@@ -3647,17 +3539,6 @@ static int handle_new_recv_msg(ipmi_smi_
 			goto out;
 		}
 
-		/*
-		** We need to make sure the channels have been initialized.
-		** The channel_handler routine will set the "curr_channel"
-		** equal to or greater than IPMI_MAX_CHANNELS when all the
-		** channels for this interface have been initialized.
-		*/
-		if (intf->curr_channel < IPMI_MAX_CHANNELS) {
-			requeue = 1;     /* Just put the message back for now */
-			goto out;
-		}
-
 		switch (intf->channels[chan].medium) {
 		case IPMI_CHANNEL_MEDIUM_IPMB:
 			if (msg->rsp[4] & 0x04) {
@@ -3693,20 +3574,11 @@ static int handle_new_recv_msg(ipmi_smi_
 			break;
 
 		default:
-			/* Check for OEM Channels.  Clients had better
-			   register for these commands. */
-			if ((intf->channels[chan].medium
-			     >= IPMI_CHANNEL_MEDIUM_OEM_MIN)
-			    && (intf->channels[chan].medium
-				<= IPMI_CHANNEL_MEDIUM_OEM_MAX)) {
-				requeue = handle_oem_get_msg_cmd(intf, msg);
-			} else {
-				/*
-				 * We don't handle the channel type, so just
-				 * free the message.
-				 */
-				requeue = 0;
-			}
+			/*
+			 * We don't handle the channel type, so just
+			 * free the message.
+			 */
+			requeue = 0;
 		}
 
 	} else if ((msg->rsp[0] == ((IPMI_NETFN_APP_REQUEST|1) << 2))
diff -puN include/linux/ipmi.h~revert-1 include/linux/ipmi.h
--- a/include/linux/ipmi.h~revert-1
+++ a/include/linux/ipmi.h
@@ -198,8 +198,6 @@ struct kernel_ipmi_msg {
 					      response.  When you send a
 					      response message, this will
 					      be returned. */
-#define IPMI_OEM_RECV_TYPE		5 /* The response for OEM Channels */
-
 /* Note that async events and received commands do not have a completion
    code as the first byte of the incoming data, unlike a response. */
 
diff -puN include/linux/ipmi_msgdefs.h~revert-1 include/linux/ipmi_msgdefs.h
--- a/include/linux/ipmi_msgdefs.h~revert-1
+++ a/include/linux/ipmi_msgdefs.h
@@ -115,7 +115,5 @@
 #define IPMI_CHANNEL_MEDIUM_USB1	10
 #define IPMI_CHANNEL_MEDIUM_USB2	11
 #define IPMI_CHANNEL_MEDIUM_SYSINTF	12
-#define IPMI_CHANNEL_MEDIUM_OEM_MIN	0x60
-#define IPMI_CHANNEL_MEDIUM_OEM_MAX	0x7f
 
 #endif /* __LINUX_IPMI_MSGDEFS_H */
_

[-- Attachment #3: revert-2.patch --]
[-- Type: application/octet-stream, Size: 7181 bytes --]

From: Andrew Morton <akpm@linux-foundation.org>

commit 25176ed670121e1e0aae5c8161713c332b786538
Author: Corey Minyard <cminyard@mvista.com>
Date:   Tue Apr 21 12:24:04 2009 -0700

    ipmi: fix statistics counting issues
    
    Bela Lubkin noticed that the statistics for send IPMB and LAN commands
    in the IPMI driver could be incremented even if an error occurred.  Move
    the increments to the proper place to avoid this.
    
    Also add some statistics for retransmissions that failed, and some little
    helper functions to neaten up the code a little.
    
    Signed-off-by: Corey Minyard <cminyard@mvista.com>
    Cc: Bela Lubkin <blubkin@vmware.com>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/char/ipmi/ipmi_msghandler.c |   73 +++++++-------------------
 1 file changed, 22 insertions(+), 51 deletions(-)

diff -puN drivers/char/ipmi/ipmi_msghandler.c~revert-2 drivers/char/ipmi/ipmi_msghandler.c
--- a/drivers/char/ipmi/ipmi_msghandler.c~revert-2
+++ a/drivers/char/ipmi/ipmi_msghandler.c
@@ -285,11 +285,6 @@ enum ipmi_stat_indexes {
 	/* Events that were received with the proper format. */
 	IPMI_STAT_events,
 
-	/* Retransmissions on IPMB that failed. */
-	IPMI_STAT_dropped_rexmit_ipmb_commands,
-
-	/* Retransmissions on LAN that failed. */
-	IPMI_STAT_dropped_rexmit_lan_commands,
 
 	/* This *must* remain last, add new values above this. */
 	IPMI_NUM_STATS
@@ -450,20 +445,6 @@ static DEFINE_MUTEX(smi_watchers_mutex);
 #define ipmi_get_stat(intf, stat) \
 	((unsigned int) atomic_read(&(intf)->stats[IPMI_STAT_ ## stat]))
 
-static int is_lan_addr(struct ipmi_addr *addr)
-{
-	return addr->addr_type == IPMI_LAN_ADDR_TYPE;
-}
-
-static int is_ipmb_addr(struct ipmi_addr *addr)
-{
-	return addr->addr_type == IPMI_IPMB_ADDR_TYPE;
-}
-
-static int is_ipmb_bcast_addr(struct ipmi_addr *addr)
-{
-	return addr->addr_type == IPMI_IPMB_BROADCAST_ADDR_TYPE;
-}
 
 static void free_recv_msg_list(struct list_head *q)
 {
@@ -620,7 +601,8 @@ ipmi_addr_equal(struct ipmi_addr *addr1,
 		return (smi_addr1->lun == smi_addr2->lun);
 	}
 
-	if (is_ipmb_addr(addr1) || is_ipmb_bcast_addr(addr1)) {
+	if ((addr1->addr_type == IPMI_IPMB_ADDR_TYPE)
+	    || (addr1->addr_type == IPMI_IPMB_BROADCAST_ADDR_TYPE)) {
 		struct ipmi_ipmb_addr *ipmb_addr1
 		    = (struct ipmi_ipmb_addr *) addr1;
 		struct ipmi_ipmb_addr *ipmb_addr2
@@ -630,7 +612,7 @@ ipmi_addr_equal(struct ipmi_addr *addr1,
 			&& (ipmb_addr1->lun == ipmb_addr2->lun));
 	}
 
-	if (is_lan_addr(addr1)) {
+	if (addr1->addr_type == IPMI_LAN_ADDR_TYPE) {
 		struct ipmi_lan_addr *lan_addr1
 			= (struct ipmi_lan_addr *) addr1;
 		struct ipmi_lan_addr *lan_addr2
@@ -662,13 +644,14 @@ int ipmi_validate_addr(struct ipmi_addr 
 	    || (addr->channel < 0))
 		return -EINVAL;
 
-	if (is_ipmb_addr(addr) || is_ipmb_bcast_addr(addr)) {
+	if ((addr->addr_type == IPMI_IPMB_ADDR_TYPE)
+	    || (addr->addr_type == IPMI_IPMB_BROADCAST_ADDR_TYPE)) {
 		if (len < sizeof(struct ipmi_ipmb_addr))
 			return -EINVAL;
 		return 0;
 	}
 
-	if (is_lan_addr(addr)) {
+	if (addr->addr_type == IPMI_LAN_ADDR_TYPE) {
 		if (len < sizeof(struct ipmi_lan_addr))
 			return -EINVAL;
 		return 0;
@@ -1520,7 +1503,8 @@ static int i_ipmi_request(ipmi_user_t   
 			memcpy(&(smi_msg->data[2]), msg->data, msg->data_len);
 		smi_msg->data_size = msg->data_len + 2;
 		ipmi_inc_stat(intf, sent_local_commands);
-	} else if (is_ipmb_addr(addr) || is_ipmb_bcast_addr(addr)) {
+	} else if ((addr->addr_type == IPMI_IPMB_ADDR_TYPE)
+		   || (addr->addr_type == IPMI_IPMB_BROADCAST_ADDR_TYPE)) {
 		struct ipmi_ipmb_addr *ipmb_addr;
 		unsigned char         ipmb_seq;
 		long                  seqid;
@@ -1599,6 +1583,8 @@ static int i_ipmi_request(ipmi_user_t   
 
 			spin_lock_irqsave(&(intf->seq_lock), flags);
 
+			ipmi_inc_stat(intf, sent_ipmb_commands);
+
 			/*
 			 * Create a sequence number with a 1 second
 			 * timeout and 4 retries.
@@ -1620,8 +1606,6 @@ static int i_ipmi_request(ipmi_user_t   
 				goto out_err;
 			}
 
-			ipmi_inc_stat(intf, sent_ipmb_commands);
-
 			/*
 			 * Store the sequence number in the message,
 			 * so that when the send message response
@@ -1651,7 +1635,7 @@ static int i_ipmi_request(ipmi_user_t   
 			 */
 			spin_unlock_irqrestore(&(intf->seq_lock), flags);
 		}
-	} else if (is_lan_addr(addr)) {
+	} else if (addr->addr_type == IPMI_LAN_ADDR_TYPE) {
 		struct ipmi_lan_addr  *lan_addr;
 		unsigned char         ipmb_seq;
 		long                  seqid;
@@ -1712,6 +1696,8 @@ static int i_ipmi_request(ipmi_user_t   
 
 			spin_lock_irqsave(&(intf->seq_lock), flags);
 
+			ipmi_inc_stat(intf, sent_lan_commands);
+
 			/*
 			 * Create a sequence number with a 1 second
 			 * timeout and 4 retries.
@@ -1733,8 +1719,6 @@ static int i_ipmi_request(ipmi_user_t   
 				goto out_err;
 			}
 
-			ipmi_inc_stat(intf, sent_lan_commands);
-
 			/*
 			 * Store the sequence number in the message,
 			 * so that when the send message response
@@ -1953,10 +1937,6 @@ static int stat_file_read_proc(char *pag
 		       ipmi_get_stat(intf, invalid_events));
 	out += sprintf(out, "events:                      %u\n",
 		       ipmi_get_stat(intf, events));
-	out += sprintf(out, "failed rexmit LAN msgs:      %u\n",
-		       ipmi_get_stat(intf, dropped_rexmit_lan_commands));
-	out += sprintf(out, "failed rexmit IPMB msgs:     %u\n",
-		       ipmi_get_stat(intf, dropped_rexmit_ipmb_commands));
 
 	return (out - ((char *) page));
 }
@@ -3750,7 +3730,7 @@ static void check_msg_timeout(ipmi_smi_t
 		list_add_tail(&msg->link, timeouts);
 		if (ent->broadcast)
 			ipmi_inc_stat(intf, timed_out_ipmb_broadcasts);
-		else if (is_lan_addr(&ent->recv_msg->addr))
+		else if (ent->recv_msg->addr.addr_type == IPMI_LAN_ADDR_TYPE)
 			ipmi_inc_stat(intf, timed_out_lan_commands);
 		else
 			ipmi_inc_stat(intf, timed_out_ipmb_commands);
@@ -3764,17 +3744,15 @@ static void check_msg_timeout(ipmi_smi_t
 		 */
 		ent->timeout = MAX_MSG_TIMEOUT;
 		ent->retries_left--;
+		if (ent->recv_msg->addr.addr_type == IPMI_LAN_ADDR_TYPE)
+			ipmi_inc_stat(intf, retransmitted_lan_commands);
+		else
+			ipmi_inc_stat(intf, retransmitted_ipmb_commands);
+
 		smi_msg = smi_from_recv_msg(intf, ent->recv_msg, slot,
 					    ent->seqid);
-		if (!smi_msg) {
-			if (is_lan_addr(&ent->recv_msg->addr))
-				ipmi_inc_stat(intf,
-					      dropped_rexmit_lan_commands);
-			else
-				ipmi_inc_stat(intf,
-					      dropped_rexmit_ipmb_commands);
+		if (!smi_msg)
 			return;
-		}
 
 		spin_unlock_irqrestore(&intf->seq_lock, *flags);
 
@@ -3786,17 +3764,10 @@ static void check_msg_timeout(ipmi_smi_t
 		 * resent.
 		 */
 		handlers = intf->handlers;
-		if (handlers) {
-			if (is_lan_addr(&ent->recv_msg->addr))
-				ipmi_inc_stat(intf,
-					      retransmitted_lan_commands);
-			else
-				ipmi_inc_stat(intf,
-					      retransmitted_ipmb_commands);
-
+		if (handlers)
 			intf->handlers->sender(intf->send_info,
 					       smi_msg, 0);
-		} else
+		else
 			ipmi_free_smi_msg(smi_msg);
 
 		spin_lock_irqsave(&intf->seq_lock, *flags);
_

[-- Attachment #4: revert-3.patch --]
[-- Type: application/octet-stream, Size: 7500 bytes --]

From: Andrew Morton <akpm@linux-foundation.org>

commit 40112ae7504745799e75ef418057f0d2cb745050
Author: Corey Minyard <minyard@acm.org>
Date:   Tue Apr 21 12:24:03 2009 -0700

    ipmi: test for event buffer before using
    
    The IPMI driver would attempt to use the event buffer even if that
    didn't exist on the BMC.  This patch modified the IPMI driver to check
    for the event buffer's existence before trying to use it.
    
    Signed-off-by: Corey Minyard <minyard@acm.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/char/ipmi/ipmi_si_intf.c |  148 +++++------------------------
 include/linux/ipmi_msgdefs.h     |    6 -
 2 files changed, 29 insertions(+), 125 deletions(-)

diff -puN drivers/char/ipmi/ipmi_si_intf.c~revert-3 drivers/char/ipmi/ipmi_si_intf.c
--- a/drivers/char/ipmi/ipmi_si_intf.c~revert-3
+++ a/drivers/char/ipmi/ipmi_si_intf.c
@@ -82,6 +82,12 @@
 #define SI_SHORT_TIMEOUT_USEC  250 /* .25ms when the SM request a
 				      short timeout */
 
+/* Bit for BMC global enables. */
+#define IPMI_BMC_RCV_MSG_INTR     0x01
+#define IPMI_BMC_EVT_MSG_INTR     0x02
+#define IPMI_BMC_EVT_MSG_BUFF     0x04
+#define IPMI_BMC_SYS_LOG          0x08
+
 enum si_intf_state {
 	SI_NORMAL,
 	SI_GETTING_FLAGS,
@@ -214,9 +220,6 @@ struct smi_info {
 			     OEM2_DATA_AVAIL)
 	unsigned char       msg_flags;
 
-	/* Does the BMC have an event buffer? */
-	char		    has_event_buffer;
-
 	/*
 	 * If set to true, this will request events the next time the
 	 * state machine is idle.
@@ -965,8 +968,7 @@ static void request_events(void *send_in
 {
 	struct smi_info *smi_info = send_info;
 
-	if (atomic_read(&smi_info->stop_operation) ||
-				!smi_info->has_event_buffer)
+	if (atomic_read(&smi_info->stop_operation))
 		return;
 
 	atomic_set(&smi_info->req_events, 1);
@@ -2405,38 +2407,13 @@ static struct of_platform_driver ipmi_of
 };
 #endif /* CONFIG_PPC_OF */
 
-static int wait_for_msg_done(struct smi_info *smi_info)
-{
-	enum si_sm_result     smi_result;
-
-	smi_result = smi_info->handlers->event(smi_info->si_sm, 0);
-	for (;;) {
-		if (smi_result == SI_SM_CALL_WITH_DELAY ||
-		    smi_result == SI_SM_CALL_WITH_TICK_DELAY) {
-			schedule_timeout_uninterruptible(1);
-			smi_result = smi_info->handlers->event(
-				smi_info->si_sm, 100);
-		} else if (smi_result == SI_SM_CALL_WITHOUT_DELAY) {
-			smi_result = smi_info->handlers->event(
-				smi_info->si_sm, 0);
-		} else
-			break;
-	}
-	if (smi_result == SI_SM_HOSED)
-		/*
-		 * We couldn't get the state machine to run, so whatever's at
-		 * the port is probably not an IPMI SMI interface.
-		 */
-		return -ENODEV;
-
-	return 0;
-}
 
 static int try_get_dev_id(struct smi_info *smi_info)
 {
 	unsigned char         msg[2];
 	unsigned char         *resp;
 	unsigned long         resp_len;
+	enum si_sm_result     smi_result;
 	int                   rv = 0;
 
 	resp = kmalloc(IPMI_MAX_MSG_LENGTH, GFP_KERNEL);
@@ -2451,98 +2428,35 @@ static int try_get_dev_id(struct smi_inf
 	msg[1] = IPMI_GET_DEVICE_ID_CMD;
 	smi_info->handlers->start_transaction(smi_info->si_sm, msg, 2);
 
-	rv = wait_for_msg_done(smi_info);
-	if (rv)
-		goto out;
-
-	resp_len = smi_info->handlers->get_result(smi_info->si_sm,
-						  resp, IPMI_MAX_MSG_LENGTH);
-
-	/* Check and record info from the get device id, in case we need it. */
-	rv = ipmi_demangle_device_id(resp, resp_len, &smi_info->device_id);
-
- out:
-	kfree(resp);
-	return rv;
-}
-
-static int try_enable_event_buffer(struct smi_info *smi_info)
-{
-	unsigned char         msg[3];
-	unsigned char         *resp;
-	unsigned long         resp_len;
-	int                   rv = 0;
-
-	resp = kmalloc(IPMI_MAX_MSG_LENGTH, GFP_KERNEL);
-	if (!resp)
-		return -ENOMEM;
-
-	msg[0] = IPMI_NETFN_APP_REQUEST << 2;
-	msg[1] = IPMI_GET_BMC_GLOBAL_ENABLES_CMD;
-	smi_info->handlers->start_transaction(smi_info->si_sm, msg, 2);
-
-	rv = wait_for_msg_done(smi_info);
-	if (rv) {
-		printk(KERN_WARNING
-		       "ipmi_si: Error getting response from get global,"
-		       " enables command, the event buffer is not"
-		       " enabled.\n");
-		goto out;
-	}
-
-	resp_len = smi_info->handlers->get_result(smi_info->si_sm,
-						  resp, IPMI_MAX_MSG_LENGTH);
-
-	if (resp_len < 4 ||
-			resp[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2 ||
-			resp[1] != IPMI_GET_BMC_GLOBAL_ENABLES_CMD   ||
-			resp[2] != 0) {
-		printk(KERN_WARNING
-		       "ipmi_si: Invalid return from get global"
-		       " enables command, cannot enable the event"
-		       " buffer.\n");
-		rv = -EINVAL;
-		goto out;
+	smi_result = smi_info->handlers->event(smi_info->si_sm, 0);
+	for (;;) {
+		if (smi_result == SI_SM_CALL_WITH_DELAY ||
+		    smi_result == SI_SM_CALL_WITH_TICK_DELAY) {
+			schedule_timeout_uninterruptible(1);
+			smi_result = smi_info->handlers->event(
+				smi_info->si_sm, 100);
+		} else if (smi_result == SI_SM_CALL_WITHOUT_DELAY) {
+			smi_result = smi_info->handlers->event(
+				smi_info->si_sm, 0);
+		} else
+			break;
 	}
-
-	if (resp[3] & IPMI_BMC_EVT_MSG_BUFF)
-		/* buffer is already enabled, nothing to do. */
-		goto out;
-
-	msg[0] = IPMI_NETFN_APP_REQUEST << 2;
-	msg[1] = IPMI_SET_BMC_GLOBAL_ENABLES_CMD;
-	msg[2] = resp[3] | IPMI_BMC_EVT_MSG_BUFF;
-	smi_info->handlers->start_transaction(smi_info->si_sm, msg, 3);
-
-	rv = wait_for_msg_done(smi_info);
-	if (rv) {
-		printk(KERN_WARNING
-		       "ipmi_si: Error getting response from set global,"
-		       " enables command, the event buffer is not"
-		       " enabled.\n");
+	if (smi_result == SI_SM_HOSED) {
+		/*
+		 * We couldn't get the state machine to run, so whatever's at
+		 * the port is probably not an IPMI SMI interface.
+		 */
+		rv = -ENODEV;
 		goto out;
 	}
 
+	/* Otherwise, we got some data. */
 	resp_len = smi_info->handlers->get_result(smi_info->si_sm,
 						  resp, IPMI_MAX_MSG_LENGTH);
 
-	if (resp_len < 3 ||
-			resp[0] != (IPMI_NETFN_APP_REQUEST | 1) << 2 ||
-			resp[1] != IPMI_SET_BMC_GLOBAL_ENABLES_CMD) {
-		printk(KERN_WARNING
-		       "ipmi_si: Invalid return from get global,"
-		       "enables command, not enable the event"
-		       " buffer.\n");
-		rv = -EINVAL;
-		goto out;
-	}
+	/* Check and record info from the get device id, in case we need it. */
+	rv = ipmi_demangle_device_id(resp, resp_len, &smi_info->device_id);
 
-	if (resp[2] != 0)
-		/*
-		 * An error when setting the event buffer bit means
-		 * that the event buffer is not supported.
-		 */
-		rv = -ENOENT;
  out:
 	kfree(resp);
 	return rv;
@@ -2933,10 +2847,6 @@ static int try_smi_init(struct smi_info 
 	new_smi->intf_num = smi_num;
 	smi_num++;
 
-	rv = try_enable_event_buffer(new_smi);
-	if (rv == 0)
-		new_smi->has_event_buffer = 1;
-
 	/*
 	 * Start clearing the flags before we enable interrupts or the
 	 * timer to avoid racing with the timer.
diff -puN include/linux/ipmi_msgdefs.h~revert-3 include/linux/ipmi_msgdefs.h
--- a/include/linux/ipmi_msgdefs.h~revert-3
+++ a/include/linux/ipmi_msgdefs.h
@@ -58,12 +58,6 @@
 #define IPMI_READ_EVENT_MSG_BUFFER_CMD	0x35
 #define IPMI_GET_CHANNEL_INFO_CMD	0x42
 
-/* Bit for BMC global enables. */
-#define IPMI_BMC_RCV_MSG_INTR     0x01
-#define IPMI_BMC_EVT_MSG_INTR     0x02
-#define IPMI_BMC_EVT_MSG_BUFF     0x04
-#define IPMI_BMC_SYS_LOG          0x08
-
 #define IPMI_NETFN_STORAGE_REQUEST		0x0a
 #define IPMI_NETFN_STORAGE_RESPONSE		0x0b
 #define IPMI_ADD_SEL_ENTRY_CMD		0x44
_

[-- Attachment #5: revert-4.patch --]
[-- Type: application/octet-stream, Size: 1149 bytes --]

From: Andrew Morton <akpm@linux-foundation.org>

commit 8b32b5d0dca2f5ab632e8bedcd57fe4c109c13fe
Author: Corey Minyard <minyard@acm.org>
Date:   Tue Apr 21 12:24:02 2009 -0700

    ipmi: fix platform return check
    
    The wrong return value is being tested when allocating a platform device
    in the IPMI SI code.  Check the right value.
    
    Signed-off-by: Corey Minyard <minyard@acm.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 drivers/char/ipmi/ipmi_si_intf.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff -puN drivers/char/ipmi/ipmi_si_intf.c~revert-4 drivers/char/ipmi/ipmi_si_intf.c
--- a/drivers/char/ipmi/ipmi_si_intf.c~revert-4
+++ a/drivers/char/ipmi/ipmi_si_intf.c
@@ -2863,7 +2863,7 @@ static int try_smi_init(struct smi_info 
 		 */
 		new_smi->pdev = platform_device_alloc("ipmi_si",
 						      new_smi->intf_num);
-		if (!new_smi->pdev) {
+		if (rv) {
 			printk(KERN_ERR
 			       "ipmi_si_intf:"
 			       " Unable to allocate platform device\n");
_

  reply	other threads:[~2009-05-17  0:10 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-05-12 21:28 modprobe ipmi_si hangs under 2.6.30-rc5 Ferenc Wagner
2009-05-17  0:09 ` Andrew Morton [this message]
2009-05-18 17:33   ` Ferenc Wagner
2009-05-18 18:40     ` [Openipmi-developer] " Corey Minyard
2009-05-19  6:26       ` Ferenc Wagner
2009-05-19 21:38         ` Corey Minyard
2009-05-20 16:15           ` Ferenc Wagner
2009-05-20 16:34             ` Corey Minyard
2009-05-21  9:47               ` Ferenc Wagner
2009-05-21 12:37                 ` Corey Minyard
2009-05-27 12:10                 ` Bela Lubkin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090516170959.8057c9e4.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=dannf@hp.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=minyard@acm.org \
    --cc=openipmi-developer@lists.sourceforge.net \
    --cc=wferi@niif.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.