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

The ibex tracer rewritten to be synthesizable #806

Open
wants to merge 30 commits into
base: master
Choose a base branch
from

Conversation

jacekmw8
Copy link

I have rewritten the ibex_tracer module to be synthesizable and produce the trace when running on emulators.
The trace format has been adapted to be used by and offline debug tools, I got rid of tabs, cycle number in favor of $time (some tools still generate cycles as a result of $time).
The trace has been split in 3 different groups:

instruction trace with disassembly closely similar to objdump -Mnumeric format
register trace, logging register value changes
memory trace, logging memory reads and writes
All these can be enabled individually though +arguments, which allows user to trace only the changes he is interested in.
The synthesis requirements implied eliminating all dynamic data structures, all string types and $sformats had to go. They have been replaced by bit vectors and their concatenations. It is not as pretty as $sformat, as register names and values have to use the same, constant width, but it's clear and well readable (with a fixed width font)
310 ns PC=0x1a0003d4, 0x35c9, jal 0x1a000296
310 ns REG= x1, 0x1a0003d6
350 ns PC=0x1a000296, 0x1a104737, lui x14,0x1a104
350 ns REG=x14, 0x1a104000
410 ns PC=0x1a00029a, 0x5b7c, lw x15,0x74(x14)
410 ns MR=0x1a104074, 0x00000000
410 ns REG=x15, 0x00000000
I have made an effort to rewrite the tracer with compliance to the SV guidelines,

As a requirement to generate a trace on the hardware, the trace log needs to be opened before the run, as it includes the hartid it needed to be passed as a parameter to the ibex_core and ibex_tracer, not as an input signal. This is the only visible change at the core level (which goes down to crs registers). But I think it is actually beneficial overall, as the hartid is not supposed to change.

There is few other changes,

added optional CSRs, as per debug specifications, like TINFO
This register is read out by the Lauterbach Trace32 debugger during the connection.
I have integrated the latest ibex with the Pulpissimo platform:
https://github.com/pulp-platform/pulpissimo
and have it running with Verilator, Cadence Xcelium, Palladium and Protium emulators with Trace32 connected through a virtual debug interface.

I hope these changes are useful for the community and can be integrated in the project. Please let me know your comments.

@rswarbrick
Copy link
Contributor

Hi again! I replied on PR #803, which you closed earlier this morning. Could you have a look at that message and let us know what you think? I don't think we'll be able to merge this exact code, but maybe there's something we can do in the Ibex repository to make something customised to your FPGA setup more convenient for you.

@jacekmw8
Copy link
Author

Hi Rupert,

Thank you for your comment and suggestions.

I do realize the current changes are quite invasive. I have dropped the cycle number, as I thought it was redundant and can be actually devised from the timestamp. Some tools like verilator actually output cycle number as timestamp it seems. I could put this back it it was important.

Yes, I could just write a wrapper and dump few rvfi signals, then post process it and have the trace in the needed format. But the thought was - such a wrapper would be of little use for others, where this tracer produces a full trace with the following benefits:

  • synthesizability
  • I think the format is actually easier to read
  • splitting the trace into I R M groups that can be individually enabled. That impacts the log size and runtime performance.
  • HartId as a parameter eliminates the discrepancies with trace file name, depending on the way to assign a value to a signal.
    I just thought its worth sharing what I have done, for the reasons above. Perhaps there would be other who could use it in similar envs to mine...

Jacek

@imphil
Copy link
Contributor

imphil commented Apr 22, 2020

Thanks for this PR. I'll have a look next week to see if we can somehow converge on a solution that retains the design goals we had for this version (especially: strict equivalence of the decoded trace with objdump as a way to test the trace decoder), and gets closer to what you're looking for.

"Readability" and "beauty" of a trace are obviously somewhat in the eye of the beholder, so let's see if we can find compelling technical arguments first 😄

Out of interest: what prototyping platform did you try this on? And do you know if platforms from other vendors have the same/similar ways of getting data out? (So in other words: are we creating something for a single special use case, or is there a wider use for it.)

PS: You can update a GitHub Pull Request (PR) after you've opened it by pushing more commits to the same branch in your fork. The PR will then update automatically.

@jacekmw8
Copy link
Author

Thank you for looking into this PR and for the hint regarding the automatic updates.

I still need to figure out why the compliance test does not produce the needed signatures. I have noticed the code checks for the hartid and stalls if it's not 0, fixed that, but apparently there is something else... need to debug.

I have been using this kind of trace produced by ARM cores on Cadence emulators, to my best knowledge the same works on the Synopsys hardware platforms. Some tracers are based on DPI, which allows more flexibility, $fwrite-based tarcer are simpler and can work well for simpler ISAs.

@jacekmw8
Copy link
Author

The power of config! I did not know the fusesoc could override the SV parameters this way. Yes, hartid=0 is needed for the compliance tests to run...

I hope we can converge on a trace format that still meets the design acceptance criteria. if some part of the trace does not I will to adjust it.

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