Skip to content

Fix/sel event data selector bits#56

Merged
datdenkikniet merged 1 commit into
datdenkikniet:mainfrom
nuclearcat:fix/sel-event-data-selector-bits
Jul 10, 2026
Merged

Fix/sel event data selector bits#56
datdenkikniet merged 1 commit into
datdenkikniet:mainfrom
nuclearcat:fix/sel-event-data-selector-bits

Conversation

@nuclearcat

Copy link
Copy Markdown
Contributor

My mistake, IPMI docs quite hard to read and easy to misinterpret, and seems best way to detect such mistakes to test it on various hardware and improve in process.
IPMI encodes the format and availability selectors for Event Data 2 and Event Data 3 in Event Data 1. The decoder assigned those selector fields in reverse: it read bits [5:4] for Event Data 2 and bits [7:6] for Event Data 3.

IPMI encodes the format and availability selectors for Event Data 2 and Event Data 3 in Event Data 1. The decoder assigned those selector fields in reverse: it read bits [5:4] for Event Data 2 and bits [7:6] for Event Data 3.

This issue was noticed while decoding Lenovo SEL entries. Those records exposed standard System Firmware Progress extension values that disappeared during EventData parsing, leading to the selector-bit mismatch.

IPMI v2.0 revision 1.1, section 29.7, Table 29-6 defines the opposite layout for both threshold and discrete events: bits [7:6] describe Event Data 2, while bits [5:4] describe Event Data 3. Swap the shifts used by EventData::parse and update the API documentation to match the normative table.

The reversal loses valid sensor-specific extension data whenever the two selectors differ. A standard System Firmware Progress record with event bytes C2 13 FF demonstrates the problem. Per Table 29-6, C2 means that Event Data 2 carries a sensor-specific extension and Event Data 3 is unspecified. Per Table 42-3, sensor type 0Fh, offset 02h, extension 13h means Starting operating system boot process. The old decoder instead treated Event Data 2 as unspecified, interpreted FF as Event Data 3, and consequently discarded 13h.

The existing 59 42 50 test did not expose the reversal because both selector fields in 59h are 01b. Add asymmetric regression tests for C2 13 FF and for the inverse Data-3-only selector layout 32 FF 23. These independently pin each selector to its correct bit field and prevent a future swap from passing accidentally.

Normative reference: Intelligent Platform Management Interface Specification, Second Generation, v2.0 revision 1.1, section 29.7 Table 29-6 and section 42 Table 42-3: https://www.intel.com/content/dam/www/public/us/en/documents/product-briefs/ipmi-second-gen-interface-spec-v2-rev1-1.pdf

Concrete firmware-progress encoding examples: Oracle documents Event Data 2 value 13h for Starting operating system boot process: https://docs.oracle.com/cd/E19121-01/sf.v20z/817-5249-17/chapter2.html

NVIDIA shows an ipmitool SEL record with Event Data c213ff decoded as System boot initiated: https://docs.nvidia.com/networking/display/bluefieldbsp453/logging

fix(doc): remove misleading power supply test heading

Remove a stale Rustdoc comment that describes the generic EventData test module as a Power Supply sensor-specific decoder.

The comment was introduced with the original event-data implementation in 8439308, directly above #[cfg(test)] mod tests. Git history shows that no Power Supply decoder was removed from or moved away from this location; the documented item has always been the test module.

Keeping the heading causes generated documentation and source readers to infer that a Sensor Type 08h decoder exists here when the module only contains generic EventData parsing tests. Remove the inaccurate heading separately from the selector-bit behavior fix.

Reference: IPMI 2.0 revision 1.1, Table 42-3 defines Sensor Type 08h as Power Supply, but this module does not implement a dedicated decoder for it: https://www.intel.com/content/dam/www/public/us/en/documents/product-briefs/ipmi-second-gen-interface-spec-v2-rev1-1.pdf
@datdenkikniet
datdenkikniet force-pushed the fix/sel-event-data-selector-bits branch from 06526d7 to 9c6782d Compare July 10, 2026 19:17

@datdenkikniet datdenkikniet left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Yeah, stuff like this is hard to get right. Thank you for being willing to submit fixes :)

@datdenkikniet
datdenkikniet merged commit 9c6782d into datdenkikniet:main Jul 10, 2026
32 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants