tum-ei-eda / etiss

Extendable Translating Instruction Set Simulator
https://tum-ei-eda.github.io/etiss/
Other
29 stars 36 forks source link

Simulation Loop ReturnCodes aliasing with Architecture Exception Codes #142

Closed JoGei closed 9 months ago

JoGei commented 11 months ago

Observation

From Testing the new Architecture (PR #127) , minor concerns in Exception Handling:

Coroutine-Plugin exceptions, be it Interruptplugin or any others, are handled in the main simulation loop within etiss::CPUCore::execute: https://github.com/tum-ei-eda/etiss/blob/0d215c87a794a7979f4d728deeeeee0408b8f169/src/CPUCore.cpp#L712-L718

First, they are consumed by the ETISS exception handler to filter out ETISS-internal exceptions as defined per etiss::Returncodes.

Then, per default, all non-listed Return-Codes are forwarded to the architectural exception handler.

I see the problem here more with some Return-Codes being interpretable as legal Architecture Exception Causes, e.g., ... https://github.com/tum-ei-eda/etiss/blob/0d215c87a794a7979f4d728deeeeee0408b8f169/include_c/etiss/jit/ReturnCode.h#L189-L191 ... translating to a RISC-V 1 0 User software interrupt (USI p. 34)

The old (RISCV) Architecture seemed to handle etiss::CPUTERMINATED incorrectly-correctly, as it seems the USI just was not implemented correctly? The new one does consume the Code as it should - as a RISC-V interrupt - which makes the etiss::CPUTERMINATED unusable from Plugins.

Fix?

Split the two (Simulation-Loop ReturnCode) and (Architecture Exception Code). Maybe add a new "Simulation Event Type" that holds both, an Event(=Simloop-Returncode) and optionally an attached Value type. Something like that:

struct SimEvent{
  int32_t event_;
  int32_t value_;
};

SimEvent usip {etiss::ArchException, 0x80000001};

Then force all Plugins to use this struct to pass information. Could possibly pack the two into a 64-bit type?

wysiwyng commented 11 months ago

Exception handling in ETISS always assumes (and to my knowledge always has) that error codes are from ReturnCode.h, and therefore are always ETISS-specific, architecture-independent. The problem described arises from the fact that the new translation of ETISS-specific error codes to RISC-V-specific error codes of #127 does not have a good fallback for unknown ETISS-specific error codes. Currently, the fallback translates all unhandled error codes to an illegal instruction exception (see https://github.com/wysiwyng/etiss/blob/2a29d8e605d0031c960fbb59cf379de54ff614c3/ArchImpl/RV64IMACFD/RV64IMACFDFuncs.c#L236 or https://github.com/tum-ei-eda/etiss_arch_riscv/blob/11ae382dff6537b58cfb5031de556ee8a3a5b809/tum_mod.core_desc#L132). The old exception handling implementation simply returns the unhandled error code to its caller as a fallback (see https://github.com/wysiwyng/etiss/blob/2a29d8e605d0031c960fbb59cf379de54ff614c3/ArchImpl/RISCV/RISCVArchSpecificImp.h#L323).

My proposed fix would be to implement handling of ETISS-specific error codes that will never be thrown by the CPU simulation model itself in etiss_CPUCore_handleException and change the new fallback mechanism to terminate the simulator signaling a spurious error code instead of raising an illegal instruction exception.

JoGei commented 9 months ago

My proposed fix would be to implement handling of ETISS-specific error codes that will never be thrown by the CPU simulation model itself in etiss_CPUCore_handleException and change the new fallback mechanism to terminate the simulator signaling a spurious error code instead of raising an illegal instruction exception.

Agree. But this still begs the question of how we avoid the aliasing Returncode.h exception codes with possible ISA-defined Exception codes that we might want to pass through the simulation loop, e.g., InterruptPlugin.

wysiwyng commented 9 months ago

Normally this should never happen though. If CoroutinePlugins want to raise exceptions, they have to do so with the ETISS-specific codes. The current exception/interrupt implementation assumes that architecture-specific codes are only present within the architecture code itself, and therefore interprets all external error codes as being ETISS-specific.