RFC: Riscv bare metal CI job #31425

sedited · 2024-12-05T08:30:01Z

This adds a CI job for building the static consensus library and linking it to an executable. It uses newlib-cygwin as a C library for the final linking step. This ensure compatibility with this target going forward and can serve as a starting point for enabling bare metal builds for the entire kernel library. This would have also caught the error fixed in #31365.

DrahtBot · 2024-12-05T08:30:04Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31425.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	laanwj, hebasto

If your review is incorrectly listed, please copy-paste  into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33974 (cmake: Check dependencies after build option interaction by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot · 2024-12-05T09:12:36Z

🚧 At least one of the CI tasks failed.
_{Debug: https://github.com/bitcoin/bitcoin/runs/33960779294}

Hints

Try to run the tests locally, according to the documentation. However, a CI failure may still
happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being
incompatible with the current code in the target branch). If so, make sure to rebase on the latest
commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the
affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

maflcko · 2024-12-05T09:16:04Z

can serve as a starting point for enabling bare metal builds for the entire kernel library.

Interesting. Do you think this is possible at all, given that the kernel library links leveldb, which I'd presume is not bare-metal ready?

So I guess this mostly serves as a check that users can ship their own-brewed libbitcoinconsensus (or so, or subset of it)?

sedited · 2024-12-05T09:26:27Z

Interesting. Do you think this is possible at all, given that the kernel library links leveldb, which I'd presume is not bare-metal ready?

Yes, definitely not ready. I think the main hurdle is the background compaction, which I am not sure how to tackle. Maybe we'll find a solution for it eventually though, either by patching it, or allowing the user to bring their own utxos.

So I guess this mostly serves as a check that users can ship their own-brewed libbitcoinconsensus (or so, or subset of it)?

Yes, that is the goal for now.

laanwj · 2024-12-05T11:20:01Z

ci/test/03_test_script.sh

+        src/secp256k1/lib/libsecp256k1.a \
+        /opt/riscv-ilp32/riscv32-unknown-elf/lib/libstdc++.a \
+        /riscv/newlib/build/riscv32-unknown-elf/newlib/libc.a \
+        /riscv/newlib/build/riscv32-unknown-elf/newlib/libm.a \


Any specific reason to use the intermediate build and not the installed libraries from /opt/newlib, here?

laanwj · 2024-12-05T11:22:45Z

ci/test/01_base_install.sh

+    cd /riscv/newlib
+    mkdir build && cd build
+    ../configure \
+        --target=riscv32-unknown-elf --disable-newlib-io-float --enable-newlib-io-long-long --enable-newlib-io-long-double --with-arch=rv32gc --with-abi=ilp32 --disable-shared --disable-multilib\


Are you sure enabling i/o for long-double is needed? i don't believe we use this type anywhere.

Sorry, all these flags were a mess. I was experimenting with linking in a range of other functionality, as well as running it on linux directly, and didn't prune stuff out nicely. Removed most of them again.

laanwj · 2024-12-05T11:35:52Z

It uses newlib-cygwin as a C library for the final linking step.

Mentioning this because i had to look it up to be sure: newlib-cygwin has nothing to do with Windows whatsoever. It's simply a minimalist libc.

I think the main hurdle is the background compaction, which I am not sure how to tackle.

Could be a periodic foreground task, if threads aren't available? But yes, this would imply patching leveldb, there is no such API right now.

maflcko · 2024-12-05T13:35:28Z

ci/test/01_base_install.sh


+if [[ ${BARE_METAL_RISCV} == "true" ]]; then
+    ${CI_RETRY_EXE} git clone --depth=1 https://github.com/riscv-collab/riscv-gnu-toolchain -b 2024.11.22 /riscv/gcc
+    cd /riscv/gcc


style-wise, it would be good to not use cd in the CI scripts, because it affects the global state for all remaining lines.

It is better to wrap it into (), for example ( cd a ; foo(); ) (or similar).

maflcko · 2024-12-05T13:38:00Z

ci/test/01_base_install.sh

+        CFLAGS_FOR_TARGET="-march=rv32gc -mabi=ilp32 -mcmodel=medlow "\
+        CXXFLAGS_FOR_TARGET="-std=c++20 -march=rv32gc -mabi=ilp32 -mcmodel=medlow"
+    make -j "$MAKEJOBS"
+    make install


It would be good to rm -rf everything duplicate. Otherwise, you are using up space twice:

$ podman image ls | grep riscv localhost/ci_native_riscv_bare latest dfa9b8223b62 5 minutes ago 13.9 GB

See the llvm build on how to do this.

Should be better now:

docker image ls | grep riscv ci_native_riscv_bare latest 55627eefe6d7 3 minutes ago 2.8GB

sedited · 2024-12-06T15:39:07Z

Updated 4910ae5 -> f277600 (bare_metal_support_0 -> bare_metal_support_1, compare)

Addressed @maflcko's comment, removing sources after install step.
Addressed @maflcko's comment, wrapped cd step into a subshell
Addressed @laanwj's comment, use the installed lib
Addressed @laanwj's comment, removing a bunch of unneeded/superfluous flags
Added a start section to the binary for the global pointer and returning an exit code. Though not needed, since we are only checking that it links, I feel like this makes the example a bit clearer.

laanwj · 2024-12-10T12:46:12Z

ci/test/03_test_script.sh

+
+              call main
+
+              # Put Exit2 system call number into the a7 register


concept ACK on making it actually runnable somewhat
i was confused for a moment here, as to what syscall numbers mean for bare-metal
maybe add "Linux" to the comment

laanwj · 2024-12-10T12:58:09Z

Concept ACK, this looks like the minimum required to sanity check that libconsensus.a can be compiled for, and linked for bare-metal RISC-V.
It is not enough to test that it actually works (this would require a lot more, quite nasty low-level code), but maybe that's out of scope for this project. At the least it is for this PR.

hebasto · 2024-12-14T10:53:27Z

Concept ACK.

hebasto · 2025-02-03T14:36:09Z

CMakeLists.txt

  )
 endif()

+if(NOT CMAKE_SYSTEM_NAME STREQUAL "Generic")


I imagine that expanding this build scenario might enable building boost and libevent in depends, no?

If so, the Boost check should have another option to be disabled.

Additionally, mind considering this comment:

bitcoin/cmake/module/AddBoostIfNeeded.cmake

Lines 7 to 8 in 1172bc4

TODO: Not all targets, which will be added in the future, require

Boost. Therefore, a proper check will be appropriate here.

which, in turn, was based on this legacy code:

bitcoin/configure.ac

Lines 1247 to 1251 in 32efe85

if test "$build_bitcoin_wallet$build_bitcoin_cli$build_bitcoin_tx$build_bitcoin_util$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench$enable_fuzz_binary" = "nonononononononono"; then

use_boost=no

else

use_boost=yes

fi

DrahtBot · 2025-04-04T06:00:36Z

CI failure:

[12:50:33.743] gmake: *** No rule to make target 'install'.  Stop.

sedited · 2025-08-07T19:59:14Z

Rebased f277600 -> ec7c86c (bare_metal_support_1 -> bare_metal_support_2, compare)

Fixed a bunch of silent and proper merge conflicts.

sedited · 2025-09-04T16:08:33Z

Rebased ec7c86c -> a577bbe (bare_metal_support_2 -> bare_metal_support_3, compare)

Fixed conflict with ci: Migrate CI to hosted Cirrus Runners #32989

sedited · 2025-09-17T11:44:58Z

Rebased a577bbe -> e254de7 (bare_metal_support_3 -> bare_metal_support_4, compare)

A bare metal build is now supported by setting CMAKE_SYSTEM_NAME=Generic Skip the platform-dependent feature checks, such as threads and atomics, which are typically not available on bare metal. Also only make the boost headers mandatory if they exist for the target.

sedited · 2025-12-23T11:26:38Z

Rebased e254de7 -> dc53679 (bare_metal_support_4 -> bare_metal_support_5, compare)

Fixed conflict with ci: Add IWYU job #33810

sedited force-pushed the bare_metal_support branch from e75c002 to ece9dcd Compare December 5, 2024 09:12

DrahtBot added the CI failed label Dec 5, 2024

sedited force-pushed the bare_metal_support branch from ece9dcd to bc5adc3 Compare December 5, 2024 09:22

sedited marked this pull request as ready for review December 5, 2024 10:05

sedited force-pushed the bare_metal_support branch from bc5adc3 to 4910ae5 Compare December 5, 2024 10:33

laanwj reviewed Dec 5, 2024

View reviewed changes

laanwj added Build system Tests labels Dec 5, 2024

DrahtBot removed the CI failed label Dec 5, 2024

DrahtBot mentioned this pull request Dec 5, 2024

[POC] cmake: Introduce LLVM's Source-based Code Coverage reports #31394

Closed

maflcko reviewed Dec 5, 2024

View reviewed changes

DrahtBot mentioned this pull request Dec 5, 2024

ci: Test cross-built Windows executables on Windows natively #31176

Merged

sedited force-pushed the bare_metal_support branch from 4910ae5 to f277600 Compare December 6, 2024 15:39

laanwj reviewed Dec 10, 2024

View reviewed changes

hebasto reviewed Feb 3, 2025

View reviewed changes

DrahtBot added the Needs rebase label Mar 31, 2025

sedited force-pushed the bare_metal_support branch from f277600 to 6fbfb56 Compare April 3, 2025 15:32

DrahtBot added CI failed and removed Needs rebase labels Apr 3, 2025

maflcko mentioned this pull request May 21, 2025

fs: remove _POSIX_C_SOURCE defining #32460

Merged

sedited mentioned this pull request Jul 6, 2025

Bitcoin Kernel Library Project Tracking #27587

Open

2 tasks

DrahtBot mentioned this pull request Jul 16, 2025

ci: Migrate CI to hosted Cirrus Runners #32989

Merged

DrahtBot mentioned this pull request Jul 28, 2025

ci: limit max stack size to 512 KiB #33079

Merged

DrahtBot added the Needs rebase label Jul 29, 2025

sedited force-pushed the bare_metal_support branch from 6fbfb56 to ec7c86c Compare August 7, 2025 19:59

DrahtBot removed Needs rebase CI failed labels Aug 7, 2025

DrahtBot added the Needs rebase label Sep 3, 2025

sedited force-pushed the bare_metal_support branch from ec7c86c to a577bbe Compare September 4, 2025 16:08

DrahtBot removed the Needs rebase label Sep 4, 2025

sedited force-pushed the bare_metal_support branch from a577bbe to e254de7 Compare September 17, 2025 11:44

This was referenced Sep 18, 2025

ci: re-add Valgrind job to the CI #33411

Closed

fuzz: reduce iterations in slow targets #33429

Merged

ci: remove Clang build from msan fuzz job #33425

Merged

DrahtBot mentioned this pull request Oct 7, 2025

DRAFT: add a freebsd job using systemlibs #33562

Closed

DrahtBot mentioned this pull request Oct 17, 2025

scripted-diff: Type-safe settings retrieval #31260

Open

DrahtBot mentioned this pull request Nov 7, 2025

ci: Add IWYU job #33810

Merged

DrahtBot mentioned this pull request Dec 2, 2025

cmake: Check dependencies after build option interaction #33974

Open

sedited added this to The libbitcoinkernel Project Dec 14, 2025

sedited moved this to Platform Protability in The libbitcoinkernel Project Dec 14, 2025

DrahtBot added the Needs rebase label Dec 22, 2025

sedited added 3 commits December 22, 2025 22:47

Add CI job for producing a static bare metal binary

e82baf9

Add CI job for riscv bare metal

dc53679

sedited force-pushed the bare_metal_support branch from e254de7 to dc53679 Compare December 23, 2025 11:26

DrahtBot removed the Needs rebase label Dec 23, 2025


		call main

		# Put Exit2 system call number into the a7 register

	TODO: Not all targets, which will be added in the future, require
	Boost. Therefore, a proper check will be appropriate here.

	if test "$build_bitcoin_wallet$build_bitcoin_cli$build_bitcoin_tx$build_bitcoin_util$build_bitcoind$bitcoin_enable_qt$use_tests$use_bench$enable_fuzz_binary" = "nonononononononono"; then
	use_boost=no
	else
	use_boost=yes
	fi

RFC: Riscv bare metal CI job #31425

Are you sure you want to change the base?

RFC: Riscv bare metal CI job #31425

Uh oh!

Conversation

sedited commented Dec 5, 2024

Uh oh!

DrahtBot commented Dec 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

DrahtBot commented Dec 5, 2024

Uh oh!

maflcko commented Dec 5, 2024

Uh oh!

sedited commented Dec 5, 2024

Uh oh!

laanwj Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

laanwj Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

sedited Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

laanwj commented Dec 5, 2024

Uh oh!

maflcko Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

maflcko Dec 5, 2024

Choose a reason for hiding this comment

Uh oh!

sedited Dec 6, 2024

Choose a reason for hiding this comment

Uh oh!

sedited commented Dec 6, 2024

Uh oh!

laanwj Dec 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laanwj commented Dec 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hebasto commented Dec 14, 2024

Uh oh!

hebasto Feb 3, 2025

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Apr 4, 2025

Uh oh!

sedited commented Aug 7, 2025

Uh oh!

sedited commented Sep 4, 2025

Uh oh!

sedited commented Sep 17, 2025

Uh oh!

sedited commented Dec 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

DrahtBot commented Dec 5, 2024 •

edited

Loading

laanwj Dec 10, 2024 •

edited

Loading

laanwj commented Dec 10, 2024 •

edited

Loading