Report the faulting virtual address in tval

[arichardson]

In some cases we were reporting the translated value instead, but the privileged spec text requires the virtual address:

If mtval is written with a nonzero value when a breakpoint,
address-misaligned, access-fault, or page-fault exception occurs on an
instruction fetch, load, or store, then mtval will contain the faulting
virtual address.

[jrtc27]

Somewhat concerning this hadn't yet been found by anything using the Sail model for testing, like RISCOF

[github-actions]

Unit Test Results

712 tests  ±0   712 ✔️ ±0   0s ⏱️ ±0s
    6 suites ±0       0 💤 ±0 
    1 files   ±0       0 ❌ ±0 

Results for commit 3a46146. ± Comparison against base commit 861bd6f.

[allenjbaum]

We don't have a lot of tests for TVAL yet (especially with translation enabled), and it would depend on what the "some cases" cause was - when does this happen? It looks like store memory exceptions, but precisely which cases?

…

On Fri, Sep 8, 2023 at 2:00 PM Jessica Clarke ***@***.***> wrote: Somewhat concerning this hadn't yet been found by anything using the Sail model for testing, like RISCOF — Reply to this email directly, view it on GitHub <#304 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJQEP2QBPOZECCM67G3XZOBQFANCNFSM6AAAAAA4Q4JV7E> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[jrtc27]

We don't have a lot of tests for TVAL yet (especially with translation enabled), and it would depend on what the "some cases" cause was - when does this happen? It looks like store memory exceptions, but precisely which cases?
…

Floating-point stores, store conditional and AMOs (for both the loading and storing part), unconditionally.

[allenjbaum]

Yup, all tests that haven't been written yet, though some are in process. We have tests for FSD, but not ones that trap with MMU enabled.

…

On Fri, Sep 8, 2023 at 2:30 PM Jessica Clarke ***@***.***> wrote: We don't have a lot of tests for TVAL yet (especially with translation enabled), and it would depend on what the "some cases" cause was - when does this happen? It looks like store memory exceptions, but precisely which cases? … <#m_7076590006193333384_> Floating-point stores, store conditional and AMOs (for both the loading and storing part), unconditionally. — Reply to this email directly, view it on GitHub <#304 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJVXVWVTM3PAG3DVYQ3XZOFAHANCNFSM6AAAAAA4Q4JV7E> . You are receiving this because you commented.Message ID: ***@***.***>

[arichardson]

Was this discussed at the meeting? Can it be merged?

[allenjbaum]

I think it is ready to be merged... but I don't think it is complete. Floating Load has the same problem, I think,and I think LR/SC does also It wasn't discussed because there was another long discussion about other issues, particularly the process for declaring when an extension's test are done for the purposes of freeze

…

On Thu, Sep 14, 2023 at 3:03 PM Alexander Richardson < ***@***.***> wrote: Was this discussed at the meeting? Can it be merged? — Reply to this email directly, view it on GitHub <#304 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJS2BOBKFLHCNAO2UW3X2N5K3ANCNFSM6AAAAAA4Q4JV7E> . You are receiving this because you commented.Message ID: ***@***.***>

[arichardson]

I think it is ready to be merged... but I don't think it is complete. Floating Load has the same problem, I think,and I think LR/SC does also It wasn't discussed because there was another long discussion about other issues, particularly the process for declaring when an extension's test are done for the purposes of freeze
…
On Thu, Sep 14, 2023 at 3:03 PM Alexander Richardson < @.> wrote: Was this discussed at the meeting? Can it be merged? — Reply to this email directly, view it on GitHub <#304 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJS2BOBKFLHCNAO2UW3X2N5K3ANCNFSM6AAAAAA4Q4JV7E . You are receiving this because you commented.Message ID: @.>

LR uses the vaddr, and this commit fixes SC, amo* and the floating point loads stores. I did search for all calls to handle_mem_exception and updated all the ones that did not report the vaddr.

[allenjbaum]

IT's possible (well true)that I didn't follow the code all the way back but:
in model/riscv_insts_aext.sail
·
in val process_loadres
122 MemException(e) => { handle_mem_exception(addr, e); RETIRE_FAIL }

and n model/riscv_insts_fext.sail
function process_fload64(rd, addr, value)
353 MemException(e) => { handle_mem_exception(addr, e); RETIRE_FAIL }
}
function process_fload32(rd, addr, value)
365 MemException(e) => { handle_mem_exception(addr, e); RETIRE_FAIL }

 function process_fload16(rd, addr, value) 

373 MemException(e) => { handle_mem_exception(addr, e); RETIRE_FAIL }
}

in addition to the one that was fixed in function clause execute (STORE_FP(imm, rs2, rs1, width)) = {
461 MemException(e) => { handle_mem_exception(addr, e); RETIRE_FAIL },

[allenjbaum]

OK, I checked a bit deeper. The "Addr" I see is a function parameter, and it is called with "vaddr" in all the cases I found.
So this is ready to go. My apologies.