-
Notifications
You must be signed in to change notification settings - Fork 46
feat(esp32c3): QEMU emulator support #1536
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(esp32c3): QEMU emulator support #1536
Conversation
Summary of ChangesHello @luoliwoshang, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces support for the ESP32-C3 QEMU emulator by addressing an issue with the firmware image offset. Previously, a universal 4KB padding was applied to all ESP chip images, which caused the ESP32-C3's QEMU instance to fail due to an incorrect bootloader start address. The changes ensure that only ESP32 (Xtensa) chips receive this padding, allowing ESP32-C3 (RISC-V) images to boot correctly from offset 0x0 as required by its ROM bootloader. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request correctly adjusts the firmware image creation for ESP32-C3 to support QEMU emulation by removing the 4KB padding that was causing boot failures. The change is specific to the esp32 chip, leaving other chips like esp32c3 unaffected, which is the correct approach. I have one suggestion regarding code maintainability to replace magic values with named constants.
| if chip == "esp32" { | ||
| outf.Write(make([]byte, 4096)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To improve readability and maintainability, it's good practice to avoid magic values. Consider defining "esp32" and 4096 as named constants at a suitable scope (e.g., package-level). This makes the code's intent clearer and centralizes these values for easier updates in the future.
For example:
const (
esp32ChipName = "esp32"
esp32BootloaderPadding = 4096
)Then you can use these constants here:
if chip == esp32ChipName {
outf.Write(make([]byte, esp32BootloaderPadding))
}
Code Review SummaryThe fix correctly addresses the ESP32-C3 QEMU boot issue. The implementation is functionally correct with clear documentation. Three noteworthy observations: 1. Testing Gap: No test coverage exists for firmware image generation. Consider adding tests to verify padding behavior for both chip variants. 2. Forward Compatibility: The hardcoded 3. Silent Failure Risk: The chip_id map lookup (line 97-100) silently returns 0 for unknown chips, which could generate incorrect firmware. Recommend adding validation after chip extraction. Overall assessment: Ready to merge with these considerations noted for future improvements. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1536 +/- ##
=======================================
Coverage 91.00% 91.00%
=======================================
Files 45 45
Lines 11955 11955
=======================================
Hits 10880 10880
Misses 899 899
Partials 176 176 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
ESP32-C3 bootloader starts at flash offset 0x0, unlike ESP32 which starts at 0x1000. The previous code added 4KB padding for all ESP chips, causing QEMU to fail with "invalid header: 0x00000000" when trying to boot ESP32-C3 images. This fix only adds the 4KB padding for ESP32 (Xtensa), while ESP32-C3 (RISC-V) images now start at offset 0x0 as expected by its ROM bootloader. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add emulator command configuration to esp32c3.json target file, enabling ESP32-C3 programs to run in QEMU emulator with: llgo run -target=esp32c3 -emulator . 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Add missing newlines at end of esp32c3.json and esp32c3-basic.json to follow standard file formatting conventions. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
66ef1c9 to
9d6314c
Compare
Summary
Enable ESP32-C3 programs to exit cleanly in QEMU emulator by using semihosting
_exitimplementation.Background
This PR builds on top of #1536 (feat/esp32c3-emulator-support) which added QEMU emulator configuration for ESP32-C3.
Previously, ESP32-C3 programs would crash in QEMU with "Illegal instruction" error because the default
_exitimplementation usesasm("unimp")which is invalid for QEMU emulator. The semihosting version usesebreakinstruction which QEMU can properly handle with the-semihostingflag.Solution
This PR implements a three-part solution:
1. Upgrade to newlib-esp32 patch5
Update from
esp-4.3.0_20250211-patch4toesp-4.3.0_20250211-patch5which includes the separation of_exitfromsyscalls.ctosyscalls_exit.c(from goplus/newlib#10).2. Remove conflicting
_exitimplementationsRemove the following stub implementations from the compile list:
libgloss/libnosys/_exit.c- generic stub for Xtensa targetslibgloss/riscv/sys_exit.c- RISC-V specific stubAfter removing these,
libgloss/riscv/semihost-sys_exit.cbecomes the only_exitimplementation and will be automatically linked from the existing static library (.a).3. Enable semihosting in QEMU config
Add
-semihostingflag to the QEMU command intargets/esp32c3-basic.json.Changes
internal/crosscompile/compile/libc/newlibesp.go:esp-4.3.0_20250211-patch5libgloss/libnosys/_exit.cfrom Xtensa compile listlibgloss/riscv/sys_exit.cfrom RISC-V compile listinternal/crosscompile/compile/libc/libc_test.go:targets/esp32c3-basic.json:-semihostingflag to QEMU emulator commandResult
ESP32-C3 programs now run and exit cleanly in QEMU:
Dependencies
Test Plan
-emulatorflag🤖 Generated with Claude Code