Skip to content

Add qemu CI#372

Closed
KushalMeghani1644 wants to merge 57 commits into
rust-embedded:masterfrom
KushalMeghani1644:add-qemu-ci
Closed

Add qemu CI#372
KushalMeghani1644 wants to merge 57 commits into
rust-embedded:masterfrom
KushalMeghani1644:add-qemu-ci

Conversation

@KushalMeghani1644

@KushalMeghani1644 KushalMeghani1644 commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

This PR adds a Github Actions workflow to build and execute RISC-V examples on QEMU, enabling detection of runtime bugs that static compilation checks may miss.

This PR addresses issue #311

@KushalMeghani1644 KushalMeghani1644 requested a review from a team as a code owner November 14, 2025 11:39
@KushalMeghani1644

Copy link
Copy Markdown
Contributor Author

ugh, I shall work on this more... thus I am making it a draft.

@KushalMeghani1644 KushalMeghani1644 marked this pull request as draft November 14, 2025 12:19
@KushalMeghani1644

Copy link
Copy Markdown
Contributor Author

Hola @romancardenas I tried to add QEMU CI here but things are getting way harder, I have done multiple fixes but I just want to make sure the fact that am I even going in the right direction? (sorry for the noise in this PR btw)

@romancardenas

Copy link
Copy Markdown
Contributor

Adding QEMU to the CI is not that easy. You must create working examples that produce an output that you can then check with something like xtasks. Check how rtic does this.

You must find a set of "illustrative" targets (e.g., e310x for riscv32imc, something else for riscv64gc...) and run examples there.

Again, it is quite a complex task, and I suggest you to leave it until you get more expertise on these subjects.

@KushalMeghani1644

Copy link
Copy Markdown
Contributor Author

I see! Thanks for helping me out!! And I do agree/understand that setting up QEMU ci is very complex, but I feel it will be valuable challenge to give a shot. Even if it takes time, I think it would be fun learning experience! Appreciate your guidance!

@romancardenas

romancardenas commented Nov 14, 2025

Copy link
Copy Markdown
Contributor

Good luck with it! RTIC has a great CI that runs tests on Arm and RISC-V. You can learn a lot from their project. Also, you can check how the esp-rs folks do it.

@KushalMeghani1644

Copy link
Copy Markdown
Contributor Author

Thanks alot for your guidance! I appreciate it alot.

@KushalMeghani1644

Copy link
Copy Markdown
Contributor Author

HI @romancardenas I am sorry to ping you again & again... I tried a-lot to mirror the RTIC QEMU CI here and seeing the mixed results here... I need your help for further fixes and guidance for what I shall do next?

@KushalMeghani1644

Copy link
Copy Markdown
Contributor Author

well @romancardenas I did a re-check of the RTIC crate's QEMU CI... and seems like it was easily fixable... now I just need your guidance upon what shall I change/update or what's wrong and what not! Thanks for your review and sorry for the disturbances.

@KushalMeghani1644 KushalMeghani1644 marked this pull request as ready for review November 15, 2025 08:54
@KushalMeghani1644

Copy link
Copy Markdown
Contributor Author

@romancardenas sorry for the ping... but can you please help me figure out what to do here next... the QEMU CI passes... but I have a few doubts if the results are fine! Could you please review this once?

@romancardenas

Copy link
Copy Markdown
Contributor

Hey @KushalMeghani1644

Sorry, but I currently have no time for advising and such activities. I can review contributions, but not helping people figure out how to add new functionalities. From what I can see, your QEMU tests are not useful, as the output is blank. You must set up a proper scenario with at least a working UART that allows us to print characters and check if the logic works.

Please, work out this locally, and once it works on your machine, push it here and check that it behaves exactly as it works on your machine.

@KushalMeghani1644

Copy link
Copy Markdown
Contributor Author

Alright! sorry to disturb you... I will work on this ASAP!

@romancardenas

Copy link
Copy Markdown
Contributor

Forget about the UART!

Use riscv-semihosting instead. It should work out of the box en qemu if semihisting enabled

@KushalMeghani1644

KushalMeghani1644 commented Nov 28, 2025

Copy link
Copy Markdown
Contributor Author

OK, thanks for the suggestion @romancardenas ! I will use riscv-semihosting instead! and btw in my local test I was able to get this!

cargo run -p xtask -- qemu --target riscv32imac-unknown-none-elf --example hello
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/xtask qemu --target riscv32imac-unknown-none-elf --example hello`
   Compiling riscv-rt v0.16.0 (/home/kushal/riscv/riscv-rt)
    Finished `release` profile [optimized] target(s) in 0.23s
HELLO_QEMU                                                                                                                      /2.5s
 cargo run -p xtask -- qemu --target riscv64gc-unknown-none-elf --example hello
    Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.03s
     Running `target/debug/xtask qemu --target riscv64gc-unknown-none-elf --example hello`
   Compiling riscv-rt v0.16.0 (/home/kushal/riscv/riscv-rt)
    Finished `release` profile [optimized] target(s) in 0.24s
HELLO_QEMU

though this is indeed through UART... I will try and make it use riscv-semihosting!

@romancardenas

Copy link
Copy Markdown
Contributor

Congrats! you are in the right track.

For this PR, two examples (one with UART, one with semihosting) could be enough. Using semihosting, you can make QEMU finish its execution so the CI can check the output with a baseline file to see if everything works as expected

@rust-embedded rust-embedded deleted a comment from Copilot AI Dec 3, 2025
@@ -0,0 +1 @@
Hello from semihosting!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of having multiple identical files, I think it would be better to only have a ci/expected/<example.run> file.

You can implement a smarter xtask process that:

  1. Checks if the ci/expected/<target>/<example>.run file exists. If so, use that file as golden file.
  2. If file does not exist, use ci/expected/<example>.run

Comment thread .github/workflows/qemu.yaml Outdated
Comment on lines +20 to +21
target:
- riscv32i-unknown-none-elf

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try something like this, I think it should work (not 100% sure about the correctness of the syntax):

Suggested change
target:
- riscv32i-unknown-none-elf
target-qemu:
- target: riscv32i-unknown-none-elf
qemu: riscv32
- target: ...

Then, in the examples, you can refer to the each element of the tuple as ${{ matrix.target-qemu.target }} and ${{ matrix.target-qemu.qemu }}

Comment thread .github/workflows/qemu.yaml Outdated
Comment on lines +31 to +45
include:
- target: riscv32i-unknown-none-elf
qemu-system: riscv32
- target: riscv32im-unknown-none-elf
qemu-system: riscv32
- target: riscv32imc-unknown-none-elf
qemu-system: riscv32
- target: riscv32imac-unknown-none-elf
qemu-system: riscv32
- target: riscv32imafc-unknown-none-elf
qemu-system: riscv32
- target: riscv64imac-unknown-none-elf
qemu-system: riscv64
- target: riscv64gc-unknown-none-elf
qemu-system: riscv64

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the previous suggestion, this part is not required

Suggested change
include:
- target: riscv32i-unknown-none-elf
qemu-system: riscv32
- target: riscv32im-unknown-none-elf
qemu-system: riscv32
- target: riscv32imc-unknown-none-elf
qemu-system: riscv32
- target: riscv32imac-unknown-none-elf
qemu-system: riscv32
- target: riscv32imafc-unknown-none-elf
qemu-system: riscv32
- target: riscv64imac-unknown-none-elf
qemu-system: riscv64
- target: riscv64gc-unknown-none-elf
qemu-system: riscv64

Comment thread xtask/src/main.rs Outdated
Comment on lines +112 to +116
if !expected_path.exists() {
fs::create_dir_all(expected_path.parent().unwrap())?;
fs::write(&expected_path, stdout.as_bytes())?;
bail!("expected output created; re-run CI");
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't like the idea of automatically creating golden files. I think it should fail the test and inform the user that the reason of the fail is that a golden file does not exist.

Also, maybe here is where we can add the additional logic of: if golden file does not exist for this target, then check for the generic golden file

Comment thread riscv-rt/examples/device_virt_s.x Outdated
@@ -0,0 +1,11 @@
MEMORY

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, this comment is not resolved! This file should be deleted. Only the device_virt.x file is required

Comment thread xtask/src/main.rs
fs,
path::PathBuf,
process::{Command, Stdio},
thread,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, make sure that the code does not have any warning in your PC. An unused reexport should trigger a warning

@KushalMeghani1644

Copy link
Copy Markdown
Contributor Author

Hey @romancardenas I have fixed/updated the codes and the CI as instructed and have created the golden files as-well! hope the changes are fine!
Thanks for putting time to my PR

Sorry for the terrible commit history xD

@romancardenas

Copy link
Copy Markdown
Contributor

The git history is all wrong. You are including changes from other PRs. Please, fix this and I will keep reviewing.

My suggestion is:

  • Start from a clean repo with the current master branch
  • Add only the changes related to QEMU CI
  • Open a new PR with clean changes so it makes my job easier.

@KushalMeghani1644

Copy link
Copy Markdown
Contributor Author

DONE! I have made the new PR @romancardenas with only 1 commit and the files that add the CI only, no spaghetti instead of commit history :)

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.

5 participants