Customize bootargs for system emulation #534
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jserv
reviewed
Jan 5, 2025
There was a problem hiding this comment.
Benchmarks
| Benchmark suite | Current: 4c73b1a | Previous: 08146d9 | Ratio |
|---|---|---|---|
Dhrystone |
1336 Average DMIPS over 10 runs |
1329 Average DMIPS over 10 runs |
0.99 |
Coremark |
942.606 Average iterations/sec over 10 runs |
942.861 Average iterations/sec over 10 runs |
1.00 |
This comment was automatically generated by workflow using github-action-benchmark.
RinHizakura
commented
Jan 5, 2025
src/riscv.c
Outdated
| const char *name, | ||
| int newlen) | ||
| { | ||
| /* FIXME: assume RAM is enough */ |
There was a problem hiding this comment.
I'm unsure if we need to consider the case of the preallocated memory being smaller than the required size to extend the DTB. Ideally, we can assume this won't happen.
ChinYikMing
reviewed
Jan 5, 2025
| assert(node > 0); | ||
|
|
||
| len = strlen(bootargs) + 1; | ||
| buf = malloc(len); |
There was a problem hiding this comment.
Include an assertion or verify the return value of malloc, as it may fail.
ChinYikMing
reviewed
Jan 5, 2025
RinHizakura
force-pushed
the
embed-dtb
branch
2 times, most recently
from
31d4a5d to
bb84d6b
Compare
jserv
reviewed
Jan 6, 2025
tools/bin_to_c.c
Outdated
Comment on lines
1
to
10
| // | ||
| // bin_to_c - convert binary data into c-compatible data tables | ||
| // | ||
| // Written by Larry Bank | ||
| // Copyright (c) 2009 BitBank Software, Inc. | ||
| // Change history | ||
| // 2/1/09 - Started the project | ||
| // 11/6/15 - converted to Linux | ||
| // 7/29/20 - tailored to Arduino FLASH data output | ||
| // |
There was a problem hiding this comment.
- Use C-style comments.
- Put the original copyright notice: https://github.com/bitbank2/bin_to_c/blob/master/LICENSE and keep "Written by Larry Bank" and "Copyright (c) 2015 BitBank Software, Inc."
- Remove the change history for short.
jserv
reviewed
Jan 6, 2025
mk/system.mk
Outdated
Comment on lines
14
to
17
| BIN_TO_C := $(OUT)/bin_to_c | ||
| $(BIN_TO_C): tools/bin_to_c.c | ||
| $(Q)$(CC) -Wall -o $@ $^ |
There was a problem hiding this comment.
Rename from bin_to_c to bin2c for consistency.
RinHizakura
force-pushed
the
embed-dtb
branch
3 times, most recently
from
0be4792 to
c15ee4c
Compare
|
Thank @RinHizakura for contributing! |
vestata
pushed a commit
to vestata/rv32emu
that referenced
this pull request
Jan 24, 2025
Customize bootargs for system emulation
ChinYikMing
added a commit
to ChinYikMing/rv32emu
that referenced
this pull request
Jan 25, 2025
The submodules are built locally, which may lead to build failures. Specifically, the dtc submodule fails to build when using GCC versions 11.4.0 or 12.3.0 with optimization levels set to -O0 or with debug flag -g. This is a critical issue, as it can disable the debug mode. To address this, optimization levels and debug level build are introduced to stabilize the pull request changes. The compiled error: /usr/bin/ld: /tmp/cc2eaHdM.ltrans0.ltrans.o: in function fdt_del_node': <artificial>:(.text+0x279f0): undefined reference to fdt_node_end_offset_' collect2: error: ld returned 1 exit status Note: I have only used gcc 11.4.0 and gcc 12.3.0 to test this out. Github runners should test other compilers. After bisecting, the original cause was found after sysprog21#534.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As discussed in #527: The device tree is coupled with the hardware design of rv32emu itself, so if the user wants to customize the device tree, he/she still needs to be based on a template. Therefore, embedding the DT template in the emulator can be more appropriate, and we can allow the user to tweak parts of the device tree from specific options.
To accomplish this, we first use bin_to_c to include DTB in the emulator binary itself and use libfdt to modify the property.
You can verify the effect of this change by running the following command to see the difference.