Page MenuHomeFreeBSD

[PowerPC64] Fix OPAL IPMI driver
ClosedPublic

Authored by luporl on Wed, Mar 25, 5:27 PM.

Details

Summary

This change fixes a couple of issues with OPAL IPMI driver and
implements a mechanism to detect timeouts and discard old messages left
in receive queue, to avoid old messages from being confused with the
reply of new ones.

Details:

  • Implemented a mechanism to discard old messages left in receive queue after timeouts.
  • Added proper handling for requests with timeout == 0.
  • Fixed wrong error logic in opal_ipmi_loop.
  • Fixed issue when getting ipmi-interface-id from device tree.
  • Added some debugging printf's.
Test Plan

Issue a couple of ipmitool commands on a POWER9 machine and verify that they return the expected results.
Examples:

ipmitool power status
ipmitool sensor

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

luporl created this revision.Wed, Mar 25, 5:27 PM
luporl added inline comments.Wed, Mar 25, 5:48 PM
sys/dev/ipmi/ipmi_opal.c
89–90 ↗(On Diff #69863)

I'm considering that 0 actually means no timeout or MAX_TIMEOUT, otherwise most requests end up timing out.

137 ↗(On Diff #69863)

Waiting 100 ms for a previously timed out message sent is a guess.
One second seemed too much for me (especially on attach) while less than 100 ms seemed very little.

If 100 ms of wait time is not enough, stale messages may remain in the queue and be returned instead of replies to new requests.
My guess is that timeouts should not occur very often and a reply to a single request will not take a long time to arrive.

All timeouts I've noticed while debugging this driver occurred with requests specifying a timeout value of zero and this being treated as zero seconds timeout, instead of no timeout.

286 ↗(On Diff #69863)

Previous code would fill the first 4 bytes of sc_interface, that is not what we want on big-endian hosts.
This issue doesn't manifest itself when ipmi-interface-id is 0.

jhibbits added inline comments.Wed, Mar 25, 7:18 PM
sys/dev/ipmi/ipmi_opal.c
72 ↗(On Diff #69863)

If you make this #else case '#define EPRINTF(fmt, ...) ((void)0)' you can avoid the '#if OPAL_IPMI_DEBUG > 0' below.

Might be able to avoid those regardless.

luporl updated this revision to Diff 69865.Wed, Mar 25, 8:06 PM
  • avoid #if
luporl marked an inline comment as done.Wed, Mar 25, 8:07 PM
jhibbits accepted this revision.Wed, Mar 25, 8:18 PM
This revision is now accepted and ready to land.Wed, Mar 25, 8:18 PM
This revision was automatically updated to reflect the committed changes.