-
Notifications
You must be signed in to change notification settings - Fork 969
Feature: Complete RISC-V baremetal support with automated qemu emulation #3254
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?
Feature: Complete RISC-V baremetal support with automated qemu emulation #3254
Conversation
…under qemu emulation. This update introduces a complete bare-metal support for TensorFlow Lite Micro on riscv32 baremetal toolchain using system mode qemu, including: - A minimal bootloader: start.s initializes stack and frame pointers for program entry. - Linker script: linker.ld defines memory layout and section mapping. - Stub handling: stubs.cc provides minimal syscall stubs for bare-metal builds. - UART driver: debug_log.cc implements UART-based logging for TFLM. - Lightweight printf: lightweight_snprintf supports %s, %d, and %p without libc. - Documentation: README.md describes build and run steps for the riscv32_baremetal target.
… to final baremetal executable.
…m emulation for any example.
veblush
left a comment
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.
Thank you for the PR!
| @@ -0,0 +1,38 @@ | |||
| ### **RISC-V 32-Bit Bare-Metal Support** | |||
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.
The current setup appears to be highly specific to QEMU. If this is the case, perhaps we should rename riscv32_baremetal to riscv32_qemu for better clarity.
| @@ -0,0 +1,72 @@ | |||
| /* Copyright 2023 The TensorFlow Authors. All Rights Reserved. | |||
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.
2025?
| #include <stdio.h> | ||
| #include "tensorflow/lite/micro/riscv32_baremetal/lightweight_snprintf.cc" | ||
|
|
||
| // UART definitions- Datasheet: https://pdos.csail.mit.edu/6.S081/2024/lec/16550.pdf |
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.
How about using Semihosting instead of having UART support here?
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.
@veblush I dont think we should have any specific UART support here, since this PR is for generic riscv32. Any specific UART support should be part of the application, not TFLM. This file should look identical to tensorflow/lite/micro/riscv32_generic/debug_log.cc with the std::fputs replaced with an external uart_puts which is supplied by the application. Also there should be documentation explaining the application requirements for using riscv32-baremetal (stub methods, uart, etc.)
| @@ -0,0 +1,40 @@ | |||
| extern "C" { | |||
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.
This might not be necessary if Semihosting is used?
| -o $@ $< \ | ||
| $(MICROLITE_LIB_PATH) $(LDFLAGS) $(MICROLITE_LIBS) | ||
|
|
||
| # Build rules and object setup for the RISC-V 32-bit bare-metal target, |
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.
The primary Makefile should only contain common definitions. All platform-specific configurations must be moved into their respective target makefiles.
| # List of available examples | ||
| EXAMPLES := person_detection hello_world micro_speech person_detection_test hello_world_test micro_speech_test dtln_test | ||
|
|
||
| # Run a given example in QEMU using test_with_qemu.sh | ||
| $(EXAMPLES): | ||
| @$(TEST_SCRIPT) system-riscv32 - gen/riscv32_baremetal_x86_64_default_gcc/bin/$@ non_test_binary riscv32_baremetal |
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.
This doesn't seem like the best place for the examples instructions. Why do we need this?
| TARGET_TOOLCHAIN_PREFIX := riscv32-unknown-elf- | ||
|
|
||
| # Toolchain Path | ||
| TARGET_TOOLCHAIN_ROOT := /home/mips/riscv32/bin/ |
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.
I think this is the test code?
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.
@veblush The toolchain download should occur here as per tensorflow/lite/micro/tools/make/targets/riscv32_generic_makefile.inc
| char buffer[256]; // Temporary buffer for formatted string | ||
| int len = mini_vsnprintf(buffer, sizeof(buffer), format, args); | ||
| if (len > 0) { | ||
| UART0_FCR = UARTFCR_FFENA; |
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.
Should be part of uart_puts method.
| #include <stdio.h> | ||
| #include "tensorflow/lite/micro/riscv32_baremetal/lightweight_snprintf.cc" | ||
|
|
||
| // UART definitions- Datasheet: https://pdos.csail.mit.edu/6.S081/2024/lec/16550.pdf |
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.
@veblush I dont think we should have any specific UART support here, since this PR is for generic riscv32. Any specific UART support should be part of the application, not TFLM. This file should look identical to tensorflow/lite/micro/riscv32_generic/debug_log.cc with the std::fputs replaced with an external uart_puts which is supplied by the application. Also there should be documentation explaining the application requirements for using riscv32-baremetal (stub methods, uart, etc.)
| extern "C" void DebugLog(const char* format, va_list args) { | ||
| #ifndef TF_LITE_STRIP_ERROR_STRINGS | ||
| char buffer[256]; // Temporary buffer for formatted string | ||
| int len = mini_vsnprintf(buffer, sizeof(buffer), format, args); |
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.
Why are we using this mini_vsnprintf? The eyalroz_printf code should be used here (as per tensorflow/lite/micro/riscv32_generic/debug_log.cc). If the application developer wants a different printf library, they can define their own DebugLog and DebugVsnprintf. Slapping a weak attribute on these two methods in debug_log.cc for bare-metal should be sufficient for that to happen.
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.
@veblush I'm not seeing why this should be part of TFLM.
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.
@veblush Don't see how any of the stubs should be part of TFLM (except _exit). No example application or unit test should require any of these stubs, nor should they currently be calling any C or C++ standard library code that would use these stubs. The exception I think would be _exit as a return from main may trigger _exit as part of the C/C++ runtime.
| qemu-${1} -cpu ${2} ${3} 2>&1 | tee ${MICRO_LOG_FILENAME} | ||
| if [[ "${5}" == "riscv32_baremetal" ]]; then | ||
| # unified call | ||
| qemu-${1} -nographic -M virt -bios none -kernel ${3} 2>&1 | tee ${MICRO_LOG_FILENAME} |
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.
@veblush If something beyond apt install qemu-system is involved in running this command or some other Linux OS configuration is required, this should be documented.
| TARGET_TOOLCHAIN_PREFIX := riscv32-unknown-elf- | ||
|
|
||
| # Toolchain Path | ||
| TARGET_TOOLCHAIN_ROOT := /home/mips/riscv32/bin/ |
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.
@veblush The toolchain download should occur here as per tensorflow/lite/micro/tools/make/targets/riscv32_generic_makefile.inc
Solves issue #3253
Adds full bare-metal RISC-V 32-bit support for TensorFlow Lite Micro, including a minimal bootloader, linker script, UART driver logging, stubs (all under tensorflow/lite/micro/riscv32_baremetal/), and a new target (riscv32_baremetal_makefile.inc) with QEMU full-system emulation. This enables running TFLM examples without any OS dependencies and improves embedded portability and testability.