Skip to content

tests: replace keystone with llvm-mc firmware build pipeline#57

Draft
jeromehaxhiaj-qti wants to merge 3 commits into
qualcomm:mainfrom
jeromehaxhiaj-qti:retire_keystone
Draft

tests: replace keystone with llvm-mc firmware build pipeline#57
jeromehaxhiaj-qti wants to merge 3 commits into
qualcomm:mainfrom
jeromehaxhiaj-qti:retire_keystone

Conversation

@jeromehaxhiaj-qti
Copy link
Copy Markdown
Contributor

Summary

Retire the keystone runtime assembler from Qbox CPU tests and replace it
with a configure-time firmware build pipeline based on the LLVM toolchain
(llvm-mc + ld.lld + llvm-objcopy).

  • Each test now ships a paired .S (and optional .ld) file that is
    assembled, linked, and objcopied to a raw .bin at CMake configure
    time. The test loads the .bin at runtime via the new
    CpuTestBenchBase::load_firmware_binary() helper, which patches a
    trailing window of .quad / .word placeholders for runtime
    constants like MMIO_ADDR and NUM_WRITES.
  • Removes the keystone CPM fetch and the patches under patch/keystone/.
    LLVM tools are now a hard build requirement on all platforms; the
    install-dependencies script is updated to cover them.
  • Introduces three CMake helpers in tests/qbox/CMakeLists.txt
    (qbox_build_firmware_bin, qbox_attach_firmware, qbox_add_cpu_test)
    plus per-arch wrappers qbox_add_cpu_<arch>_test so each test reads
    as a single line at the call site.

What's in this PR

scripts: install llvm/lld for the firmware build pipeline (67abcce)

Adds llvm and lld to install_dependencies.sh's package lists for
Ubuntu (apt), macOS (brew), and MSYS2 (pacboy). On macOS Homebrew
ships ld.lld in the separate lld formula, so both formulae are
required.

tests: replace keystone with llvm-mc firmware build pipeline (54cf583)

The bulk of the change. Highlights:

New CMake helpers (in tests/qbox/CMakeLists.txt):

  • qbox_build_firmware_bin(<name> <arch> <src_dir> [EXTRA_LD_ARGS ...])
    assembles <src_dir>/<name>.S, links with ld.lld using
    <src_dir>/<name>.ld (or per-directory firmware.ld fallback),
    objcopies to a raw .bin via a <name>-firmware custom target.
  • qbox_attach_firmware(<target> <fw_name>) — wires the build dependency
    and defines FIRMWARE_BIN_PATH for the test executable.
  • qbox_add_cpu_test(<target> <timeout_s> <sources>... [ARCH <arch>] [FIRMWARE <name>] [FIRMWARE_SRC_DIR <dir>] [EXTRA_LD_ARGS ...])
    collapses the build + attach + add_test boilerplate into one call.
    By default the firmware base name is the stem (NAME_WE) of the first
    source file; FIRMWARE overrides it.
  • qbox_add_cpu_<arch>_test(<target> <timeout_s> <source>.cc) — per-arch
    thin wrappers in cpu/<arch>/CMakeLists.txt pinning ARCH=<arch>.
    This is what call sites use, so each test is one line.

Portability:

  • On MSYS2/Windows, ld.lld defaults to the lld-link/PE-COFF driver
    and silently ignores -flavor gnu, leaving GNU-style options like
    -T unrecognised. Pass the correct ELF emulation per arch
    (aarch64elf / hexagonelf / elf32lriscv / elf64lriscv) via
    -m on WIN32; older Linux LLDs (e.g. Ubuntu's stock llvm-18)
    don't know every emulation name, so leave -m off elsewhere.
  • hexagon-smmu-stress-test-v2.s uses Hexagon V73 ops (tlbw,
    dmwait, dmlink) that older LLVM Hexagon backends don't recognise.
    macOS via Homebrew's llvm and MSYS2/Windows both ship a recent
    enough backend; the firmware (and its test) are gated behind
    APPLE OR WIN32 (QQVPQSP-714).
  • tests/CMakeLists.txt gates each tests/qbox/cpu/<arch>/
    subdirectory on the corresponding arch being present in
    LIBQEMU_TARGETS.

Naming convention applied uniformly across cpu-test sources:

  • Filenames use dashes only (no underscores); the hexagon dir uses
    hexagon- consistently rather than the previous hex- shortening.
  • Each test owns its firmware: aarch64 reset-test was previously
    shared by reset-test-system and reset-test-cpu; the .S is now
    duplicated so each test has its own. hexagon-smmu-firmware was
    renamed to hexagon-smmu-stress-test-v2 to match its driver.
  • After these renames every qbox_add_cpu_<arch>_test call has
    firmware == source-stem, so qbox_add_cpu_test derives the
    firmware base from the first source's NAME_WE and call sites just
    read (target, timeout, source).

Coverage: all aarch64, hexagon, and riscv32 CPU tests previously
using set_firmware() (including the multi-block
smmu-router-stress-test-v2 layout via .org), and the standalone
hexagon-load-store-test harness.

tests: extend SKIP_TESTS for known-flaky and slow cpu-test configs (7778beb)

Two SKIP_TESTS entries surfaced by CI on this branch (both pre-existing
issues, unrelated to the keystone migration):

  • aarch64-dmi-test-reset:num-cpu=(4|32):threading=SINGLE segfaults
    intermittently on Ubuntu (across the
    ubuntu-{22,24}.04 × {gcc,gcc-debug,clang,clang-debug,clang-lto}
    matrix, num-cpu=4 and num-cpu=32 each fail in roughly half of
    runs while num-cpu=1 and num-cpu=2 always pass). Marked flaky.
  • hexagon-smmu-stress-test-v2 already skipped num-cpu>1 for
    COROUTINE and SINGLE as "takes too long". Extends the same skip
    to MULTI so num-cpu>1 is uniformly skipped — macOS hit a 100s
    ctest timeout on multithread-quantum:num-cpu=4:MULTI while the
    same policy at num-cpu=1 finishes in well under a second.

Test plan

  • cmake --build build --target test passes on macOS (apart from
    the hexagon-smmu-stress-test-v2:multithread-quantum:num-cpu=4
    timeout, now skipped) — verified locally
  • Windows CI: green
  • Ubuntu CI: green after the SKIP_TESTS extension (the
    aarch64-dmi-test-reset SEGFAULTs are pre-existing, now skipped)
  • macOS CI: green after the SKIP_TESTS extension
  • All renamed firmware sources (<arch>-<name>.{cc,S,ld}) round-trip
    via qbox_build_firmware_bin and load at runtime — verified by
    end-to-end runs of aarch64-simple-write-test,
    aarch64-reset-test-system, aarch64-reset-test-cpu,
    hexagon-ld-st-mmio-test, riscv32-reset-test,
    hexagon-smmu-stress-test-v2 (all pass)
  • The if(NOT TARGET ...) guard inside qbox_add_cpu_test means
    shared firmware (when used) is built once — no longer needed
    after this PR (every test owns its firmware) but the guard
    stays for safety
  • The standalone hexagon-load-store-test (which doesn't go
    through qbox_add_cpu_test) still builds and runs

Migration notes for downstream

If you depend on the now-removed set_firmware() runtime API or on the
keystone library itself, you'll need to migrate to the
qbox_add_cpu_<arch>_test helpers (see tests/qbox/cpu/*/CMakeLists.txt
for examples). The C++-side load_firmware_binary() helper expects a
firmware that ends with .quad (aarch64) or .word
(riscv32/hexagon) placeholders patched by initializer-list values at
the tail of the binary; see tests/qbox/include/test/cpu.h.

@jeromehaxhiaj-qti jeromehaxhiaj-qti marked this pull request as draft May 11, 2026 13:29
@jeromehaxhiaj-qti jeromehaxhiaj-qti marked this pull request as draft May 11, 2026 13:29
@jeromehaxhiaj-qti jeromehaxhiaj-qti force-pushed the retire_keystone branch 2 times, most recently from 6dac2fc to 17dc52a Compare May 11, 2026 14:14
Add llvm and lld to install_dependencies.sh's dependency lists (Ubuntu
apt, macOS brew, msys2 pacboy) so the script covers everything tests/qbox
will need once the keystone-to-llvm-mc migration that follows lands. On
macOS Homebrew ships ld.lld in the separate `lld` formula, not `llvm`,
so both formulae are required there.

Signed-off-by: Jerome Haxhiaj <jhaxhiaj@qti.qualcomm.com>
- aarch64-dmi-test-reset:num-cpu=(4|32) segfaults intermittently on
  Ubuntu in both SINGLE and MULTI threading (across the
  ubuntu-{22,24}.04 × {gcc,gcc-debug,clang,clang-debug,clang-lto}
  matrix, num-cpu=4 and num-cpu=32 each fail in some runs while
  num-cpu=1 and num-cpu=2 always pass). Mark all num-cpu>2 cells as
  flaky regardless of threading mode or sync-policy.

- hexagon-smmu-stress-test-v2 already skipped num-cpu>1 for COROUTINE
  and SINGLE as "takes too long". Extend the same skip to MULTI so
  num-cpu>1 is uniformly skipped — the macOS run hit a 100s ctest
  timeout on multithread-quantum:num-cpu=4:MULTI while the same
  policy at num-cpu=1 finishes in well under a second.

Signed-off-by: Jerome Haxhiaj <jhaxhiaj@qti.qualcomm.com>
Retire the keystone runtime assembler from Qbox CPU tests in favour of
firmware binaries built at CMake-configure time with llvm-mc + ld.lld
+ llvm-objcopy. Each test ships a paired .S file (with .quad/.word
placeholders for runtime-patched constants like MMIO_ADDR / NUM_WRITES)
and the new CpuTestBenchBase::load_firmware_binary() helper patches
those placeholders and ptr_load()s the result into m_mem at runtime.
Deletes patch/keystone/ and the keystone CPM fetch; LLVM tools are
now a hard build requirement (covered by the install_dependencies.sh
update in the previous commit).

Build-side helpers in tests/qbox/CMakeLists.txt:

- qbox_build_firmware_bin(<name> <arch> <src_dir> [EXTRA_LD_ARGS ...])
  assembles <src_dir>/<name>.S, links with ld.lld using
  <src_dir>/<name>.ld (or the per-directory firmware.ld fallback),
  and objcopies to a raw .bin via a <name>-firmware custom target.

- qbox_attach_firmware(<target> <fw_name>) wires the build dependency
  and defines FIRMWARE_BIN_PATH for the test executable.

- qbox_add_cpu_test(<target> <timeout_s> <sources>...
                    [ARCH <arch>] [FIRMWARE <name>]
                    [FIRMWARE_SRC_DIR <dir>] [EXTRA_LD_ARGS ...])
  collapses the build + attach + add_test boilerplate into one call.
  By default the firmware base name is the stem (NAME_WE) of the
  first source file; FIRMWARE overrides it.

- qbox_add_cpu_<arch>_test(<target> <timeout_s> <source>.cc) per-arch
  thin wrappers in cpu/<arch>/CMakeLists.txt that pin ARCH=<arch>.
  This is what call sites use, so each test reads as one line.

Portability:

- On MSYS2/Windows, ld.lld defaults to the lld-link/PE-COFF driver and
  silently ignores -flavor gnu, leaving GNU-style options like -T
  unrecognised. We pass the correct ELF emulation per arch
  (aarch64elf / hexagonelf / elf32lriscv / elf64lriscv) via -m on
  WIN32; older Linux LLDs (e.g. Ubuntu's stock llvm-18) don't know
  every emulation name, so we leave -m off elsewhere.

- hexagon-smmu-stress-test-v2.s uses Hexagon V73 ops (tlbw, dmwait,
  dmlink) that older LLVM Hexagon backends don't recognise. macOS
  via Homebrew's llvm and MSYS2/Windows both ship a recent-enough
  backend; the firmware (and its test) are gated behind APPLE OR
  WIN32 (QQVPQSP-714).

- tests/CMakeLists.txt gates each tests/qbox/cpu/<arch>/ subdirectory
  on the corresponding architecture being present in LIBQEMU_TARGETS.

Naming convention applied across all cpu-test sources:

- Filenames use dashes only, never underscores; the hexagon dir uses
  hexagon- consistently rather than the previous hex- shortening.

- Each test owns its firmware: aarch64 reset-test was previously
  shared by reset-test-system and reset-test-cpu; the .S is now
  duplicated so each has its own. hexagon-smmu-firmware was renamed
  to hexagon-smmu-stress-test-v2 to match its driver.

- After these renames every qbox_add_cpu_<arch>_test call has
  firmware == source-stem, so qbox_add_cpu_test derives the firmware
  base from the first source's NAME_WE and call sites just read
  (target, timeout, source).

Covers all aarch64, hexagon, and riscv32 CPU tests previously using
set_firmware() (including the multi-block smmu-router-stress-test-v2
layout via .org), and the standalone hexagon load-store-test harness.

Signed-off-by: Jerome Haxhiaj <jhaxhiaj@qti.qualcomm.com>
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