csr exception state with external debug request during illegal instruction entry

[strichmo]

Bug Title

CSR DECP and MECP miscompare when external debug request is granted during execution of an illegal instruction.

Component

Component:Verif
I suspect the cause of the miscompare is in the reference model.

The miscompares are:
UVM_ERROR @ 685665.300 ns : uvmt_cv32_step_compare.sv(96) reporter [Step-and-Compare] dpc expected=0x000056f6 and actual=0x00030900 PC=0x1a110800
UVM_ERROR @ 685665.300 ns : uvmt_cv32_step_compare.sv(96) reporter [Step-and-Compare] mepc expected=0x000054c8 and actual=0x000056f6 PC=0x1a110800
UVM_ERROR @ 685665.300 ns : uvmt_cv32_step_compare.sv(96) reporter [Step-and-Compare] mstatus expected=0x00001880 and actual=0x00001800 PC=0x1a110800

The sequence in the core appears to be:

1. The core is executing in non-debug mode.
2. The instruction at 0x56f6 is a dret, which takes an illegal insruction exception when not in debug mode. Therefore the next properly retired instruction (in normal operation) would be the mtvec handler instruction (0x30900).
3. An external debug request occurs during the illegal instruction.
4. The external debug request is granted so the next instruction is 0x1a110800 (the start of the debug handler).
5. However the Core (RTL) has switched dpc to 0x30900 and mepc to 0x56f6 (reflecting the "execution" of the illegal instruction). There is no way to signal this to the ISS, thus the ISS assumes that dpc should be 0x56f6 (the instruction after the last retired instruction).
6. When the debug rom executes a "dret" the ISS and RTL CSRs match up again as the retirement of the first mtvec instruction "realigns" the ISS and the RTL.

Steps to Reproduce

Use this core-v-verif:
https://github.com/silabs-oysteink/core-v-verif
Branch: silabs-oysteink_random-debug

Common.mk should be changed to point to the head of the CV32E40P branch referenced:
CV32E40P_REPO ?= https://github.com/strichmo/cv32e40p
CV32E40P_BRANCH ?= strichmo/temp/tracer_with_ebrk
#2020-10-08
-CV32E40P_HASH ?= 90c23eb
+CV32E40P_HASH ?= head

Testcase executed with Xcelium 20.03.009
% makecv32 comp_corev-dv gen_corev-dv test TEST=corev_rand_debug SEED=-2058259982 CFG=no_pulp

[silabs-oysteink]

I see that the riscv-dv can generate handshakes (writes to a specific memory address) to enable update of core internal state.

While the RISCV-DV Instruction Generator provides a debug_rom section as well as various interrupt and exception handlers, from a verification standpoint this is not enough to help ensure that a core’s internal state is being updated correctly upon receiving external stimulus such as interrupts or debug requests, such as the values of any relevant CSRs. To help with this issue, the instruction generator also provides a mechanism by which to communicate information to a RTL simulation environment via a handshaking protocol.

The full description is in the riscv-db documentation:
https://htmlpreview.github.io/?https://github.com/google/riscv-dv/blob/master/docs/build/singlehtml/index.html#document-cmd_line_reference

Maybe we need to add these handshakes to the virtual peripheral and use those events to update the ISS?

[Silabs-ArjanB]

I looked at this simulation and this seems like an ISS (or tracer) issue to me. The RTL is simply honoring the debug request after the dret completes (but dret needs to go through DECODE, FLUSH_EX, FLUSH_WB states, which opens up the door for the debug request to come in after DRET started its execution but before it completes).

[Silabs-ArjanB]

Hi @eroom1966 @strichmo Is this issue still being looked at or has it been solved?

[eroom1966]

still open, I have not had time to investigate a good way to schedule the haltreq signal, I will get some time today to re-investigate

[eroom1966]

is this problem still valid given the new core-v-verif setup with ImperasDV
If so can somebody describe how to reproduce

[MikeOpenHWGroup]

The steps to reproduce are provided at the top of the issue report. Having said that, they rely on GitHub repos and branches that may no longer exist. I'll give it a go...

[MikeOpenHWGroup]

As luck would have it, @silabs-oysteink's fork/branch of core-v-verif used to identify this issue does still exist. Alas, SteveR's fork/branch of the cv32e40p for this issue does not exist. I will try a few hashes of the cv32e40p that are of similar vintage, but if I do not reproduce the issue it will be an inconclusive result.

[silabs-oysteink]

As luck would have it, @silabs-oysteink's fork/branch of core-v-verif used to identify this issue does still exist. Alas, SteveR's fork/branch of the cv32e40p for this issue does not exist. I will try a few hashes of the cv32e40p that are of similar vintage, but if I do not reproduce the issue it will be an inconclusive result.

I have no clue if this ancient branch of my core-v-verif is in any useful state today, use at your own risk ;)

[MikeOpenHWGroup]

OK, I have had no luck recreating this in simulation, probably because of the issue's dependence on two "ancient branches", one of which no longer exists. However, I believe we can close this issue for the following reasons:

1. Arjan, agreed that the issue was with the ISS and/or testbench, not the RTL.
2. This issue was explicitly waived as part of the RTL Freeze signoff for CV32E40P v1.0.0 (although no reason was given).

@Silabs-ArjanB, I leave leave it to you to decide if this issue can be closed or not.

Having said that, I do not believe this specific scenario is explicitly called out in the Debug DVplan. Row 62 is close, but not exactly the same. The functional coverage for the row 62 is in core-v-verif/cv32e40p/env/uvme/cov/uvme_debug_covg.sv and according to the RTL Freeze report, this coverage was hit.

I think the condition we want is to assert the debug_request while a dret instruction is working its way through the pipeline. I will create an issue to get this into our DVplans and coverage.

[Silabs-ArjanB]

@Silabs-ArjanB, I leave leave it to you to decide if this issue can be closed or not.

It is up to the current maintainers of the CV32E40P. I would say if it has not been fixed, do not close it.

[davideschiavone]

There is a high change this is a TB issue, yet I agree with @Silabs-ArjanB this should be first tested.

Best
Davide