Skip to content

qemu_arm_mps2: fix UART receive stall and other pre-merge review findings - #21

Closed
ppannuto-claude wants to merge 1 commit into
ppannuto:qemu-arm-mps2from
ppannuto-claude:qemu-arm-mps2-review2
Closed

qemu_arm_mps2: fix UART receive stall and other pre-merge review findings#21
ppannuto-claude wants to merge 1 commit into
ppannuto:qemu-arm-mps2from
ppannuto-claude:qemu-arm-mps2-review2

Conversation

@ppannuto-claude

Copy link
Copy Markdown

Review findings from a pre-merge pass over the branch behind tock#5125, split so the behavioural fixes can be read apart from the documentation trim. Based on qemu-arm-mps2 at d164be4.

The one blocker is the first commit: the CMSDK UART latches INTSTATUS.RX only at the instant a byte arrives and only if CTRL.RxIntEn is set right then, so clearing RxIntEn across the client callback loses any byte that lands in that window — and, because the device stops accepting input while its holding register is full, every byte after it. Driving the AN385 process console over a TCP serial, the console stops answering after five short bursts and never recovers.

Verified, not just read:

  • Console death reproduced on a baseline build (dead after 5 bursts) and gone after the fix (alive until it trips an unrelated process_console line-buffer overflow at burst 65 — that one reproduces identically on qemu_rv32_virt, so it is a pre-existing capsule bug, not this board's, and is left alone here).
  • The renamed vector-table assert catches the mixup it is meant to: pointing an386's main.rs at qemu_arm_mps2_an385::CortexM3 now fails the link with "AN386 vector table missing", where before it linked cleanly with Cortex-M3 handlers in a Cortex-M4 image.
  • Both boards build and boot; c_hello and spi_loopback (SPI PASS) run on an385, c_hello on an386; blink still cycles LED0 through multiple states, so the timer change did not break the alarm path.
  • make format-check, make ci-job-syntax, make ci-job-clippy, make licensecheck, tools/ci/check-boards-readme.py all pass.

Not verified: no real MPS2 hardware; register semantics are checked against QEMU 10.2.1's device models, not ARM DDI0479C. Alarm accuracy is not measured — these boards do not use -icount, so timing claims under QEMU would be meaningless. No QEMU other than 10.2.1 was exercised, which is why boards/README.md now claims no minimum version rather than a new one.

Left out deliberately, as design questions rather than fixes: the duplication between the two per-image vector-table crates, the early_init/finish_start two-call seam, package naming, and adding an385 to ci-job-clippy's per-arch list.

"The MPS2 images share peripherals and differ in their core" was stated
in eight places across four READMEs and three module docs, and the
"purely virtual target, not a real vendor chip" framing in four. State
each once, where a reader looking for it would land.

The two board READMEs are peers, but an386's was written as a footnote
to an385's: it deferred run-app to "same as mps2-an385's" and so never
mentioned APP_ADDRESS or the largest-first .tbf packing rule, both of
which apply to it identically. Move the shared material -- the QEMU
invocation, APP_ADDRESS, packing -- into mps2_base's README, which both
boards already point at, and make those pointers actual links; nothing
in boards/README.md could reach that file otherwise. What is left per
board is what differs: machine, core, target triple, the AN386 FPU
note.

Also:

- an385's sample `make run` output quoted a size line that no longer
  matches the build, while an386's did; drop the block rather than
  carry a number that goes stale.
- The chips README listed mps2-an505 and mps2-an521 as "not yet
  implemented here". They are not part of this machine family at all --
  they live in QEMU's hw/arm/mps2-tz.c on the IoTKit/SSE-200, with a
  different peripheral layout. Say so, drop the "as of Aug 2026" stamp,
  note that an500 also differs in its PSRAM base, and use the same
  heading style as every other README here.
- boards/README.md advertised "Yes (10.2.1)", which is the version this
  was developed against, not a minimum, in a column whose other entries
  read as minimums. Neither the true minimum nor CI's pinned build has
  been tested here, so claim no version.
- semihosting.rs said a `bkpt` with no host "traps with no host to
  service it"; with C_DEBUGEN clear it raises a debug exception that
  escalates to a HardFault, which terminate()'s own doc already said.
  Fix that, the reason given for omitting `noreturn`, a broken doc
  link, a BPKT/BKPT typo, and a 2022 copyright year on a new file.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01L56it8rfV2vkZF9YRN5fa8
@ppannuto-claude

Copy link
Copy Markdown
Author

All five commits are integrated into qemu-arm-mps2 (tip 4776c26), verified by content rather than by PR state: the branch tree is byte-identical to this PR's head, and each finding is present — UART arms RxIntEn in configure() and drains with no receive outstanding, the aborts return Ok(()), the timer re-arms on a stale interrupt, transfer_byte_sync exists, tickle() sets IntEn, both per-image vector-table symbols are distinct, EarlyInit's fields are private, and the doc fixes (no dated stamp, an505/an521 moved to mps2-tz.c, no QEMU version claim, 2026 copyright, working README links) all landed.

Closing. #22 is rebased on top and now carries just the closure commit.

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