Skip to content

fix(registers): avoid widened xHCI MMIO accesses - #177

Open
ZR233 wants to merge 1 commit into
rust-osdev:mainfrom
ZR233:codex/fix-dword-mmio-access
Open

fix(registers): avoid widened xHCI MMIO accesses#177
ZR233 wants to merge 1 commit into
rust-osdev:mainfrom
ZR233:codex/fix-dword-mmio-access

Conversation

@ZR233

@ZR233 ZR233 commented Jul 16, 2026

Copy link
Copy Markdown

Problem

The aggregate volatile accessors for the Port Register Set and the Supported Protocol header expose 16-byte MMIO values. With Rust nightly 2026-07-15 on AArch64, those accesses can be lowered to widened transactions. The RK3588 DWC3 controller on OrangePi-5-Plus then stalls during xHCI initialization.

The xHCI 1.2c Requirements Specification Table 5-19 (§§5.4.8–5.4.11) defines the four port registers as Dwords at offsets 0x0, 0x4, 0x8, and 0xc with a 0x10 port stride. Figure 7-1 and Tables 7-6–7-9 (§7.2) define the Supported Protocol header as four Dwords. HCCPARAMS1.AC64 (§5.3.6, Table 5-13) only says whether the high-order halves of 64-bit pointer/address fields are implemented; it does not change these 32-bit register sizes.

Changes

  • replace the aggregate Port Register Set value with an owner that keeps the base address, port count, and mapper
  • add lifetime-bound port(index) and port_mut(index) handlers whose four fields are independent 32-bit accessors
  • add a read-only HeaderAccessor that reads four Dwords separately and reconstructs the existing Header value in ordinary memory
  • add RecordingMapper regression tests for both paths
  • document the port API migration and the widened-access fix in the changelog

The common Supported Protocol call remains unchanged:

let header = protocol.header.read_volatile();

Port callers migrate from aggregate reads and updates:

let portsc = registers.port_register_set.read_volatile_at(index).portsc;
registers.port_register_set.update_volatile_at(index, |port| {
    port.portsc.set_port_power();
});

to register-sized accessors:

let portsc = registers.port_register_set.port(index).portsc.read_volatile();
registers
    .port_register_set
    .port_mut(index)
    .portsc
    .update_volatile(|portsc| portsc.set_port_power());

Regression evidence

On the original implementation, both new tests failed with one 16-byte mapping:

actual:   [(base, 16)]
expected: [(base, 4), (base + 4, 4), (base + 8, 4), (base + 12, 4)]

Both tests pass after the change.

Validation

Passed locally:

  • cargo +stable fmt --all -- --check
  • cargo +stable test
  • cargo +stable build
  • cargo +stable clippy --all-targets
  • cargo +nightly-2026-07-15 test
  • cargo +nightly-2026-07-15 check --target aarch64-unknown-none-softfloat
  • TGOSKits local-path consumer: cargo xtask clippy --package crab-usb
  • TGOSKits local-path consumer: cargo test -p crab-usb (33 tests)
  • OrangePi-5-Plus RK3588: cargo xtask starry test board --board orangepi-5-plus (all 8 board groups passed, including xHCI enumeration and USB2 lsusb)

The requested strict Clippy command with -D warnings -D clippy::pedantic -D clippy::all reports the same 39 pre-existing diagnostics on this branch and on upstream/main@f2254c86. RUSTDOCFLAGS="-D warnings" cargo +stable doc --no-deps is likewise blocked on the base commit by the renamed private_doc_tests lint in src/lib.rs. This change introduces no additional diagnostics in either check.

This references #158 without closing it because the mapping-performance part remains unresolved. The broader capability-access audit is tracked in #176.

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.

1 participant