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

Debug halt causes illegal instructions #14

Closed
gmarkall opened this issue Jun 28, 2017 · 5 comments
Closed

Debug halt causes illegal instructions #14

gmarkall opened this issue Jun 28, 2017 · 5 comments

Comments

@gmarkall
Copy link
Contributor

Summary

We are working on a verilator model of RI5CY and are attempting to understand how to use the debug unit. Normal execution seems to work correctly as far as we can observe. Whenever we attempt to halt the processor by writing DBG_CTRL_HALT to DBG_CTRL, an illegal instruction exception seems to be generated.

Details

Our test bench does the following (the test bench file is at https://github.com/embecosm/ri5cy/blob/debug_halt/verilator-model/testbench.cpp), which is inspired by looking at tb.svh and spi_debug_test.svh in the PULPino repository:

  1. Sets irq_i, debug_req_i, fetch_enable_i and rstn_i to 0.
  2. Runs for 100 cycles (for the reset, probably this is more than needed)
  3. Set rstn_i to 1 and fetch_enable_i to 1.
  4. Runs for 20 clock cycles, to allow the CPU to do some "normal" execution. This appears to work without problems.
  5. Writes to the debug unit DBG_CTRL = DBG_CTRL | DBG_CTRL_HALT
  6. Writes DBG_IE = 0xF
  7. Waits for debug stall by reading DBG_CTRL until we observe that DBG_CTRL_HALT is set.
  8. Sets single step by writing DBG_CTRL = DBG_CTRL_HALT | DBG_CTRL_SSTE.

At this point we think we should be ready to single-step. For each single step, we:

  1. Clear DBG_HIT (I didn't see this in the test bench, but it's what the debug_bridge GDB server appeared to do for single steps).
  2. Writes DBG_CTRL = DBG_CTRL_SSTE.
  3. Waits for debug stall (as in step 7 above).

The first one or two single steps seem to work OK (depending on how many cycles we run for and how much we try and interact with the debug unit) but it seems that after about 8 cycles, an illegal instruction exception occurs.

Observations

A colleague of mine has suggested that it looks like something goes awry in the prefetch fifo - when executing normally, core/if_stage/prefetch_32/prefetch/fifo valid_Q[0] is never set, so we bypass the input of the fifo to the output without ever caching it in the fifo itself. In the debug case the core halts and the fifo fills up. The fifo then wipes one of the entries to 0, later it passes that to the decoder and it gives the undefined exception.

Reproducing

If it is helpful to try to reproduce the issue, then this can be done by:

  1. Cloning our RI5CY repo: https://github.com/embecosm/ri5cy
  2. Checking out the debug_halt branch: https://github.com/embecosm/ri5cy/tree/debug_halt
  3. In the verilator-model subdirectory, run make. Note that this requires the latest version of verilator installed (3.906), and for pkg-config to be able to find verilator.
  4. Execute ./testbench.

The output that it produces is reproduced here:

$ ./testbench 
DBG_CTRL  10001
DBG_HIT   0
DBG_CAUSE 1f
DBG_NPC   cc
DBG_PPC   c8
DBG_CTRL  10001
DBG_HIT   1
DBG_CAUSE 0
DBG_NPC   d0
DBG_PPC   cc
                1430: Entering exception routine.
                1430: Illegal instruction (core 0) at PC 0x000000d0:
DBG_CTRL  10001
DBG_HIT   1
DBG_CAUSE 0
DBG_NPC   84
DBG_PPC   d0
DBG_CTRL  10001
DBG_HIT   1
DBG_CAUSE 2
DBG_NPC   88
DBG_PPC   84
                1630: Entering exception routine.
                1630: Illegal instruction (core 0) at PC 0x00000088:
DBG_CTRL  10001
DBG_HIT   1
DBG_CAUSE 2
DBG_NPC   84
DBG_PPC   88

The output is produced by the stepSingle function - we read the registers each time we try to single step, to try and get some visibility into what is happening.

Questions

Looking at past issues it seems that other people are successfully making use of the debug unit, so I guess there might be something wrong with what we'e doing or how we're setting things up.

  • Are we doing something wrong in the setup or halting of the CPU?
  • If not, is there anything else we can look at to try and understand what is going on?
  • Any other thoughts?

Any assistance is greatly appreciated - many thanks in advance!

@gmarkall
Copy link
Contributor Author

gmarkall commented Jul 3, 2017

An update - we seem to get around the problem by reading the value of DBG_NPC and writing it back to cause a pipeline flush, as in: https://github.com/embecosm/ri5cy/blob/verilator-model/verilator-model/testbench.cpp#L279

Is this a bug somewhere, or is it always required to read/write DBG_NPC before resuming/single-stepping?

@gmarkall
Copy link
Contributor Author

gmarkall commented Jul 6, 2017

We also noticed that the problem isn't just limited to the debug unit - whilst executing some programs that stall the pipeline with a load, the contents of the prefetch fifo become corrupted.

It turns out that we observe these issues only when INSTR_RDATA_WIDTH is 32, and the prefetch_buffer (prefetch_32) is used. When INSTR_RDATA_WIDTH is 128 and the prefetch_L0_buffer (prefetch_128) is used, we can use the debug unit without flushing the pipeline by writing to DEBUG_NPC, and the programs that stalled the pipeline don't seem to result in the contents of the fifo getting corrupted.

Is it possible that there's an issue with prefetch_buffer? I notice that the last commit to it (cc82192) was intended to address issues like the one we are seeing - could we be hitting an edge case that somehow got through?

Or maybe there's something else that needs to be done that we're not doing when using the prefetch_buffer?

@davideschiavone
Copy link
Contributor

Hello,

thanks for reporting it!

I will try to replicate it, can you tell us more details to replicate such pipeline issue?

Maybe some waveforms or the piece of code with relative stalls would be also helpful for us.
Let us start with the example without debug :)

Thanks a lot,
Davide

@gmarkall
Copy link
Contributor Author

I've just tried changing INSTR_RDATA_WIDTH back to 32 in the most recent revision (was using 128 to workaround the issue as mentioned above), and I can no longer replicate the issue - I guess some of the changes recently that made fixes to the prefetcher have maybe resolved it?

@davideschiavone
Copy link
Contributor

Hello,

I have actually fixed the prefetcher buffer in these 2 commits:

7078fa8

and here

5fb7e42

I hope this also solved your issue :)

davideschiavone added a commit that referenced this issue Mar 7, 2023
* Restored correct syntax

* Automatic PR dev->master (#766)

* Correcting RFVI trace

* Some clarifications about immediate in scalar replication on SIMD instructions.

Signed-off-by: Pascal Gouedo <pascal.gouedo@dolphin.fr>

* Headers alignement.

Signed-off-by: Pascal Gouedo <pascal.gouedo@dolphin.fr>

* Changed uvm_error in uvm_info in old tracer to avoid errors and header alignement.

Signed-off-by: Pascal Gouedo <pascal.gouedo@dolphin.fr>

* Moved typedef definitions in rvfi_pkg and header alignement.

Signed-off-by: Pascal Gouedo <pascal.gouedo@dolphin.fr>

* launched verible

Signed-off-by: Pascal Gouedo <pascal.gouedo@dolphin.fr>

* Update documentation of instruction extensions for CV32E40P version 2. (#760)

* Restored correct syntax

* Update documentation of instruction extensions for CV32E40P version 2.

	These changes particularly relate to how tool chain support is
	invoked using -march.  We take the opportunity to remove caveats
	about ISA extensions not supported in the tool chain, since all
	are now supported in both GCC and Clang/LLVM.

	* docs/source/instruction_set_extensions.rst: Add -march tool
	chain invocations for each ISA extension and remove all caveats
	about compiler support.

Signed-off-by: Jeremy Bennett <jeremy.bennett@embecosm.com>

---------

Signed-off-by: Jeremy Bennett <jeremy.bennett@embecosm.com>
Co-authored-by: Davide Schiavone <davide@openhwgroup.org>
Co-authored-by: pascalgouedo <pascal.gouedo@dolphin.fr>

---------

Signed-off-by: Pascal Gouedo <pascal.gouedo@dolphin.fr>
Signed-off-by: Jeremy Bennett <jeremy.bennett@embecosm.com>
Co-authored-by: Yoann Pruvost <yoann.pruvost@dolphin.fr>
Co-authored-by: pascalgouedo <pascal.gouedo@dolphin.fr>
Co-authored-by: Jeremy Bennett <jeremy.bennett@embecosm.com>
Co-authored-by: Davide Schiavone <davide@openhwgroup.org>

* new block diagram (#14)

---------

Signed-off-by: Pascal Gouedo <pascal.gouedo@dolphin.fr>
Signed-off-by: Jeremy Bennett <jeremy.bennett@embecosm.com>
Co-authored-by: pascalgouedo <pascal.gouedo@dolphin.fr>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Yoann Pruvost <yoann.pruvost@dolphin.fr>
Co-authored-by: Jeremy Bennett <jeremy.bennett@embecosm.com>
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

No branches or pull requests

2 participants