Skip to contentSkip to conversation

ipmi: Run a dummy command before submitting a new command

PATCH In review Matthew Garrett 3 participants 4 messages Jul 27, 2010 – Oct 26, 2010 IPMI

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ipmi: Run a dummy command before submitting a new command

Newer firmware revisions on HP's ILO3 (1.05 and later) generate state machine errors with the current IPMI code. Running through the IPMI timeout handler once before submitting the command avoids this.

---
 drivers/char/ipmi/ipmi_si_intf.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
drivers/char/ipmi/ipmi_si_intf.c +2 -0
index e39a744..3f06199 100644
@@ -317,6 +317,7 @@ static int unload_when_empty = 1;
 static int add_smi(struct smi_info *smi);
 static int try_smi_init(struct smi_info *smi);
 static void cleanup_one_si(struct smi_info *to_clean);
+static void smi_timeout(unsigned long data);
 
 static ATOMIC_NOTIFIER_HEAD(xaction_notifier_list);
 static int register_xaction_notifier(struct notifier_block *nb)
@@ -897,6 +898,7 @@ static void sender(void                *send_info,
 #endif
 
 	mod_timer(&smi_info->si_timer, jiffies + SI_TIMEOUT_JIFFIES);
+	smi_timeout((unsigned long)smi_info);
 
 	if (smi_info->thread)
 		wake_up_process(smi_info->thread);
Signature · 1 line
1.7.1.1
#1 lore raw

I don't think this is the right way to handle the problem. Though it's not going to break anything, this change is just a hack. We need to figure out why these machine exhibit this behavior. If it's a bug in the driver, then we need to fix the driver. If it's a bug in the HP firmware, then we need to document it well as such, get HP to fix their firmware, and possibly tie it into the xaction handler that's already in start_next_msg.

The only interaction with the device that this change should cause is one read from the status register, since the device should be idle at this point. If that's the case, and it's not a driver bug, you can try adding an xaction that calls smi_info->handlers->event(smi_info->si_sm, 0).

There are debugging flags in the state machines that might help debug this, too.

-corey

Quoted 31 lines · Matthew Garrett

Newer firmware revisions on HP's ILO3 (1.05 and later) generate state machine errors with the current IPMI code. Running through the IPMI timeout handler once before submitting the command avoids this.

---
  drivers/char/ipmi/ipmi_si_intf.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
drivers/char/ipmi/ipmi_si_intf.c +2 -0
index e39a744..3f06199 100644
@@ -317,6 +317,7 @@ static int unload_when_empty = 1;
  static int add_smi(struct smi_info *smi);
  static int try_smi_init(struct smi_info *smi);
  static void cleanup_one_si(struct smi_info *to_clean);
+static void smi_timeout(unsigned long data);
 
  static ATOMIC_NOTIFIER_HEAD(xaction_notifier_list);
  static int register_xaction_notifier(struct notifier_block *nb)
@@ -897,6 +898,7 @@ static void sender(void                *send_info,
  #endif
 
  	mod_timer(&smi_info->si_timer, jiffies + SI_TIMEOUT_JIFFIES);
+	smi_timeout((unsigned long)smi_info);
 
  	if (smi_info->thread)
  		wake_up_process(smi_info->thread);
#2 lore raw
Quoted 7 lines · Corey Minyard

I don't think this is the right way to handle the problem. Though it's not going to break anything, this change is just a hack. We need to figure out why these machine exhibit this behavior. If it's a bug in the driver, then we need to fix the driver. If it's a bug in the HP firmware, then we need to document it well as such, get HP to fix their firmware, and possibly tie it into the xaction handler that's already in start_next_msg.

Yeah, I agree that this isn't the optimal approach. I'm waiting to hear from HP if they have any idea what happened between 1.01 (which worked) and 1.05 (which is broken), which might give some more insight into what we're doing wrong.

Quoted 5 lines

The only interaction with the device that this change should cause is one read from the status register, since the device should be idle at this point. If that's the case, and it's not a driver bug, you can try adding an xaction that calls smi_info->handlers->event(smi_info->si_sm, 0).

I'll try to see what's going on.

Signature · 1 line
Matthew Garrett | [email protected]
#3 lore raw

I've finally had time to look at this more closely. The following patch seems to make things happy, but I still don't have a full understanding of what's going on.

(Background: Some HP ilo firmware versions are unhappy due to 3326f4f2276791561af1fd5f2020be0186459813 . Running a dummy command in schedule() works around this but clearly isn't the right answer. Turns out that this also breaks IPMI on some Suns, and Oracle reverted this in their kernel but never told me. Thoracle)

drivers/char/ipmi/ipmi_si_intf.c +2 -0
index e537610..763af8f 100644
@@ -901,6 +901,8 @@ static void sender(void                *send_info,
 
 	mod_timer(&smi_info->si_timer, jiffies + SI_TIMEOUT_JIFFIES);
 
+	smi_info->last_timeout_jiffies = jiffies;
+
 	if (smi_info->thread)
 		wake_up_process(smi_info->thread);
 
Signature · 1 line
Matthew Garrett | [email protected]