oxidecomputer / oxidecomputer/hubris
Audit i2c devices using validate-with-raw-read
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 3.6k
- Forks
- 239
- Avg merge
- 1d 12h
- Merged PRs (30d)
- 23
Description
When the SP reports the inventory of devices to the control plane, it checks whether each device is present and valid. Most devices have a driver in drv-i2c-devices that implements the Validate trait for this purpose, doing some device-specific validation (like checking the model number via PMBus).
Originally, if there was no driver, the i2c codegen would silently fall back to a generic implementation of Validate that just checks whether the device ACKs a single-byte i2c read (with no write beforehand). This unfortunately made it easy to accidentally use the fallback if you, for example, put the driver in a folder where codegen couldn't find it (https://github.com/oxidecomputer/hubris/issues/2616), or added a new device without knowing that this validation feature exists.
https://github.com/oxidecomputer/hubris/pull/2617 changed this so that you must explicitly opt in to the fallback implementation using the validate-with-raw-read setting, or else the build will fail. This required adding validate-with-raw-read = true to a bunch of devices that had been silently using the fallback. I don't know whether their original authors intended to use the fallback or even knew it existed. We should check each of those devices (commented with a link to this issue) to see:
- If the fallback behavior is correct for this device and the read doesn't have unwanted side effects
- If it would be better to write a device-specific validation function
- If a driver for this device already exists somewhere where the i2c codegen can't find it
Note that https://github.com/oxidecomputer/hubris/issues/2616 might make big changes to this area of the codebase, and could potentially make this ticket obsolete.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in the i2c codegen and the drv-i2c-devices drivers, then identify devices that added validate-with-raw-read after PR 2617. For each device, inspect whether a raw read is safe, whether device-specific validation is preferable, or whether an existing driver is undiscoverable. The work is done when every affected device has an intentional validation choice documented in its configuration or driver.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- build-system, embedded-iot
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 45/100