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

Dromajo STF Trace Support Improvements #130

Open
kathlenemagnus opened this issue Dec 11, 2023 · 12 comments
Open

Dromajo STF Trace Support Improvements #130

kathlenemagnus opened this issue Dec 11, 2023 · 12 comments
Assignees
Labels
enhancement New feature or request

Comments

@kathlenemagnus
Copy link
Collaborator

I have been reviewing the STF trace generation support in Dromajo and have some recommendations to improve the implementation.

  • Add register read and write records
  • Add memory read and write records
  • Refactor parameters to control enablement of tracepoint and privilege mode checks
  • Better handling of "no trace generated" scenarios (no trace created + a warning if no instructions are traced)

These are the current STF trace parameters in Dromajo:

--stf_trace <filename>  Dump an STF trace to the given file"
--stf_no_priv_check Turn off the privledge check in STF generation"

I would like to propose removing --stf_no_priv_check and adding these parameters:

--stf-tracepoint Enable tracepoint detection for STF trace generation"
--stf-priv-modes <MHSU|HSU|SU|U> Specify which privilege modes to include in STF trace generation"

These parameters would allow the user to specify whether tracepoints should be used to start/stop trace generation and specify which privilege modes to include in the trace. All conditions specified must be met for tracing to be enabled. These new options would also allow for noncontiguous traces, which is well handled by the STF library.

@kathlenemagnus kathlenemagnus added the enhancement New feature or request label Dec 11, 2023
@kathlenemagnus kathlenemagnus self-assigned this Dec 11, 2023
@danbone
Copy link
Contributor

danbone commented Dec 13, 2023

Sorry could you expand on what you mean by:

These parameters would allow the user to specify whether tracepoints should be used to start/stop trace generation

@jeffnye-gh
Copy link
Collaborator

Sounds good to me, my comments:

I like the generality of replacing no priv check.

I also would like to know more about --stf-tracepoint

It's pretty quick for me to run our process against a PR/draft PR when it's available.

@kathlenemagnus
Copy link
Collaborator Author

Tracepoints are just hint instructions with a special meaning.

xor x0, x0, x0 // tracepoint to start tracing
xor x0, x1, x1 // tracepoint to stop tracing

In the current Dromajo STF tracing implementation, tracepoints are required to start and stop tracing. Adding this new option would make them optional.

@jeffnye-gh
Copy link
Collaborator

jeffnye-gh commented Dec 15, 2023

I was asking if new behavior was intended with --stf-tracepoint since the existing trace macros are enabled with --stf_trace and disabled without.

@kathlenemagnus
Copy link
Collaborator Author

Yes, so --stf_trace would be required to enable tracing and --stf-tracepoint would enable tracepoint detection. By default, tracepoint detection would be disabled and the --stf-tracepoint parameter would be ignored if --stf_trace is not set.

BTW I intended to follow the style of the existing parameters so --stf-tracepoint will actually be --stf_tracepoint.

@kathlenemagnus
Copy link
Collaborator Author

@jeffnye-gh how would you like me to push my changes to the Condor Dromajo repo? I don't have write permissions so I can't push my branch.

@jeffnye-gh
Copy link
Collaborator

jeffnye-gh commented Dec 21, 2023 via email

@kathlenehurt-sifive
Copy link

@jeffnye-gh I'll do that. Thanks!

@klingaard
Copy link
Collaborator

@kathlenehurt-sifive, when you're done with this issue, pass it over to me (I'll hijack it) and I'll move Olympia (via documentation) to illustrate the new flow.

@kathlenemagnus
Copy link
Collaborator Author

@klingaard this is good to go. Olympia should point to the master branch of Condor's Dromajo fork: https://github.com/Condor-Performance-Modeling/dromajo/tree/master

The STF lib version also needs to be moved to the latest to pull in a required fix for the PC record.

@klingaard
Copy link
Collaborator

Thank you @kathlenemagnus!

@jeffnye-gh
Copy link
Collaborator

@klingaard @kathlenemagnus

The SHA with KM's changes is

commit ff13255

aka ff13255a50bd1b5e7597f3bf2ceaf24b782e8b72

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants