ADD: a basic BTM N-trace spec compliant trace encoder model #1824
Conversation
|
@YenHaoChen can you review this one? |
|
Hi, thanks for the review. I have fixed (I think) the memory leak regarding |
riscv/trace_encoder_n.cc
Outdated
| #include "trace_encoder_n.h" | ||
|
|
||
| trace_encoder_n::trace_encoder_n() { | ||
| this->trace_sink= fopen("trace_n.bin", "wb"); |
There was a problem hiding this comment.
This file name should be different for each hartId .. a simple way to avoid clobbering the same file in a multi-core with-trace simulation.
| .i_type = _get_insn_type(&fetch.insn, npc != p->get_state()->pc + insn_length(fetch.insn.bits())), | ||
| .exc_cause = 0, | ||
| .tval = 0, | ||
| .priv = P_M, // TODO: check for processor privilege level |
There was a problem hiding this comment.
This priv should be fixed.
Should this trace_encoder_push_commit also be added to the take_trap and take_interrupt methods?
| public: | ||
|
|
||
| trace_encoder_n() { | ||
| this->trace_sink= fopen("trace_n.bin", "wb"); |
There was a problem hiding this comment.
Perhaps the constructor should include hartId as a parameter, and then trace_n.bin can be hart<id>_trace_n.bin here?
There was a problem hiding this comment.
You should implement a destructor for this class that fcloses the file.
There was a problem hiding this comment.
Feels like multi-core needs to be handled more carefully. According to the spec there needs to be a spec funnel, and the decoder also needs to spawn multiple workers and discern sources. Maybe we should add the multi-core support later.
This PR adds a trace encoder model to spike, making it capable of dumping encoded n-trace packets in the slow path, which a separate decoder can later decode. It is enabled by a trace controller device (see: ucb-bar/spike-devices#8) and implements the basic trace encoding mechanisms. Adding this to spike will make the software development of the decoder easier, and a reference for RTL encoders to check upon.