Open allenjbaum opened 7 months ago
I'm adding some comments by @Timmmm here as part of the original discussion:
The proper fix is to refactor out all of the rvfi_
calls (e.g. rvfi_read
) into ext_
calls (ext_on_read
), and fill in the gaps for things that RVFI doesn't currently track. Then those ext_
functions can:
Ideally the code that is compiled by default would have no mention of RVFI in it at all.
Our internal fork has a system similar to RVFI but more comprehensive. For now I have just added my_read()
calls right next to rvfi_read()
etc. but the ext_on_read()
refactoring is on my todo list (but pretty low down I'm afraid; especially given how long reviews take).
The additional state changes we record that are missing from RVFI are:
mstatus
in dirty_fd_context()
fcsr
in accrue_fflags()
We also record traps and branches, though those aren't strictly state changes. Some things like PC and the current privilege mode we just read directly from the model between steps.
See also #449 which is about using callbacks to log state changes. I think that's probably the best approach.
The current implementation of logging exposes a lot of information:
But: it doesn't identify which level of the page table is being accessed, and any implicit updates to CSR values that are a result of executing an op (with some exceptions: FCSR is exposed, xCAUSE is exposed, but xStatus and EPC are not, and possibly if a CSR is explicitly written, the data that will be read if different than what is written (WARL behavior)
This a request to ensure that all state changes be available in the execution log.