Skip to content
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

Allow redirecting trace output to a file #275

Merged
merged 3 commits into from
Aug 28, 2023

Conversation

arichardson
Copy link
Collaborator

This is useful when booting an operating system with tracing enabled
as it allows showing the printed messages on the terminal and storing
the trace log to a separate file.

Example usage:
./c_emulator/riscv_sim_RV64 -b os-boot/rv64-64mb.dtb -t /dev/stderr os-boot/linux-rv64-64mb.bbl --trace-output /tmp/linux.log

This PR also includes a second commit to fix --help for options without a corresponding short option (rebase and merge preferred over squash and merge).

@github-actions
Copy link

github-actions bot commented Jun 16, 2023

Unit Test Results

712 tests  ±0   712 ✔️ ±0   0s ⏱️ ±0s
    6 suites ±0       0 💤 ±0 
    1 files   ±0       0 ±0 

Results for commit 4f3fd51. ± Comparison against base commit 58cac61.

♻️ This comment has been updated with latest results.

@arichardson
Copy link
Collaborator Author

ping?

@billmcspadden-riscv billmcspadden-riscv added the tgmm-agenda Tagged for the next Golden Model meeting agenda. label Jul 11, 2023
@arichardson
Copy link
Collaborator Author

ping?

@Alasdair
Copy link
Collaborator

Alasdair commented Jul 28, 2023

This looks good to me, I don't think there are any reasons not to merge this. The only problem I could foresee is if you don't use these printing functions and use the ones in the Sail standard library they won't be redirected.

This is useful when booting an operating system with tracing enabled
as it allows showing the printed messages on the terminal and storing
the trace log to a separate file. All categorized trace messages are
now redirected to the --trace-output file when passed. If this option is
not gived, the current behaviour of using stdout is retained.

Example usage:
`./c_emulator/riscv_sim_RV64 -b os-boot/rv64-64mb.dtb --trace-output /dev/stderr os-boot/linux-rv64-64mb.bbl --trace-output /tmp/linux.log`
Previously --help printed the following:
```
         -V      --no-trace
         -�      --trace-output
         -l      --inst-limit
```
With the new change it is:
```
         -V      --no-trace
                 --trace-output
         -l      --inst-limit
```
These empty lines don't add to the readability of the trace and in fact
when trace redirection is enabled they result in lots of empty lines being
printed to stderr which makes it impossible to read the OS boot messages.
@arichardson
Copy link
Collaborator Author

I believe I've addressed all outstanding comments, and I've updated the commit messages to explain which messages are redirected:

csim: Allow redirecting trace output to a file using command line flag

This is useful when booting an operating system with tracing enabled
as it allows showing the printed messages on the terminal and storing
the trace log to a separate file. All categorized trace messages are
now redirected to the --trace-output file when passed. If this option is
not gived, the current behaviour of using stdout is retained.

Example usage:
`./c_emulator/riscv_sim_RV64 -b os-boot/rv64-64mb.dtb --trace-output /dev/stderr os-boot/linux-rv64-64mb.bbl --trace-output /tmp/linux.log`

I've made these commits logically separate, so please rebase and merge rather than squashing the commits.

@arichardson
Copy link
Collaborator Author

ping?

@billmcspadden-riscv billmcspadden-riscv merged commit ec6a6a9 into riscv:master Aug 28, 2023
2 checks passed
@billmcspadden-riscv billmcspadden-riscv removed the tgmm-agenda Tagged for the next Golden Model meeting agenda. label Aug 28, 2023
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.

3 participants