oxidecomputer / oxidecomputer/hubris
spd: page selection and mux emulation are unfaithful
@bcantrill is already working on this.
Since Oct 4, 2022.
- Dominant language
- Rust
- Stars
- 3.6k
- Forks
- 239
- Avg merge
- 1d 12h
- Merged PRs (30d)
- 23
Description
The SPD proxy emulates the page-selected state maintained by real SPD devices. However, it does so globally; there is a single variable tracking the selected page for all devices on all banks. Given the muxed bank design necessitated by limited SPD addressing, the collection of virtual SPD slaves that can respond to a page-select command (sent to addresses 0b011_011xr, where x is the page to select) should be limited to the bank(s) currently accessible through the mux. That is, the selected page must be per-bank, just as voffs is per-slave. By making this global, sequences of commands like the following will result in accesses to the wrong page:
Select bank 0
Select page 0
Read...
Select page 1
Select bank 1
Select page 0
Read...
Select bank 0
Read... <--- Gets page 0, rightly expects page 1
I do not have any evidence that the AMD master currently uses such sequences, but it would have every right to expect this to behave properly, and its behaviour may change in future without warning.
A similar but related issue is that the emulated mux does not allow simultaneous selection of multiple downstream buses, although in a real LTC4306 (or PCA9545A), the downstream bus selection is a bitmask. Here we're a bit safer because AMD do not specify the use of a specific mux in this application, so it is conceivable that someone might choose one that allows only a single downstream bus to be selected at one time, and selecting more than one bank to do page selection would be a very fine optimisation that would make sense only after much more obvious optimisations, e.g., doing multibyte reads of the SPD data itself. That said, it would not be at all surprising if the authors of this/future firmware were sloppy or made assumptions that don't always hold, so it is probably also a good idea to allow this in the mux emulation. Certainly any master would be confused by setting register 3 to 0xc0 and having us set the selected segment to None and thus disconnecting all downstream buses.
Again, I have no current evidence that this is causing any problem with actual AMD firmware.
Edited: Serial Presence Detect, not Scalable Data Port.
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.
Assessment
This issue has not been assessed yet.