oxidecomputer / oxidecomputer/hubris

spd: page selection and mux emulation are unfaithful

Open
#323 2 comments 0 reactions 1 assignee View on GitHub

@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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.