CLIC or vectored mode leads to misaligned load

[marcfedorow]

This line

riscv-arch-test/riscv-test-suite/env/arch_test.h

Line 250 in da638ba

lw t6, 0(t2) // get original mtvec target

suggests that mtvec (thus t2) contains a correct address.
This is not always true, e.g. if MODE (MTVEC[1:0]) == 3 (CLIC).
andi t2, t2, -4 should be added before this line.

[allenjbaum]

This should be closed when new trap handler is merged

[pawks]

Closing this as PR #277 has addressed this issue. Reopen if problem persists.

[marcfedorow]

This obviously does not work.
https://github.com/riscv-non-isa/riscv-arch-test/blob/main/riscv-test-suite/env/arch_test.h#L1018
Here MCAUSE is checked to be in range [8, 11], which is not true if CLIC is enabled.

[allenjbaum]

Has CLIC been ratified yet? If it hasn’t, then this will get fixed after it is. Please file a PR with the changes that are required- I’m a bit unclear what exactly the problem is. CLIC is an interrupt extension, and mcause MSB is also ways a one for interrupts- this code Is specifically checking for *exception* codes 8..11. Also: if this is a problem only when CLIC is implemented, than a YAML variable or March string must be defined to identify its presence, and conditional code must be added to handle It two different ways

…

On Wednesday, May 17, 2023, marcfedorow ***@***.***> wrote: This obviously does not work. https://github.com/riscv-non-isa/riscv-arch-test/blob/main/r iscv-test-suite/env/arch_test.h#L1018 Here MCAUSE is checked to be equal zero, which is not true if CLIC is enabled. — Reply to this email directly, view it on GitHub <#285 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJWVFW3SDH46CK5ZLQTXGUBYZANCNFSM6AAAAAAQRJLILE> . You are receiving this because you commented.Message ID: ***@***.***>

[marcfedorow]

Is specifically checking for exception codes 8..11.

Let us say that the exception code is 0xb (ecall), XLEN=32, CLIC is enabled

  csrr t5, CSR_XCAUSE // t5 contains 0x3000000b
  addi t4, t5, -8     // t4 contains 0x30000003
  andi t4, t4, -4     // t4 contains 0x30000000
  bnez t4, ...        // jump will be performed
                      // t5 still contains 0x3000000b

I've patched it via

diff --git a/riscv-test-suite/env/arch_test.h b/riscv-test-suite/env/arch_test.h
index aaaaaaa..bbbbbbb 100644
--- a/riscv-test-suite/env/arch_test.h
+++ b/riscv-test-suite/env/arch_test.h
@@ -1013,6 +1013,8 @@ common_\__MODE__\()entry:
 //spcl case handling for ECALL in GOTO_MMODE mode,)  ****tests can't use ECALL t2=0****
 spcl_\__MODE__\()2mmode_test:
         csrr    t5, CSR_XCAUSE
+        li      t4, ~0x7fff0000                 // bits reserved for mpp, mpie, etc
+        and     t5, t5, t4                      // clear such bits
         addi    t4, t5, -8                      // is cause 8..11? Mmode should avoid ECALL 0
         andi    t4, t4, -4                      // NOTE: cause 10 is RSVD.  Sail will diverge, but buggy anyway  
         bnez    t4, \__MODE__\()trapsig_ptr_upd // no, not in special mode, just continue
@@ -1068,6 +1070,7 @@ rvtest_\__MODE__\()endtest:                     // target may be too far away, s
 //------end atomic------------------------------------------------
 
         LREG    t3, xtvec_new_off(sp)           // get pointer to actual tramp table
+        andi    t3, t3, -4                      // clear CLIC-related bits
 //----------------------------------------------------------------
 
   /*************************************************************************/

, because it is simple and it works. Of course this does not test if CLIC is enabled (which can not be checked via isa-string) or even implemented.

I'm good with what I've done, but if one needs to run arch-tests with CLIC enabled, this should be adressed.

[allenjbaum]

Correct- if bits 29:28 are used, this code needs to change. That should happen when CLIC is ratified, (and we’ll remember that now, thanks)

…

On Thursday, May 18, 2023, marcfedorow ***@***.***> wrote: Is specifically checking for *exception* codes 8..11. Let us say that the *exception* code is 0xb (ecall), XLEN=32, CLIC is enabled csrr t5, CSR_XCAUSE // t5 contains 0x3000000b addi t4, t5, -8 // t4 contains 0x30000003 andi t4, t4, -4 // t4 contains 0x30000000 bnez t4, ... // jump will be performed // t5 still contains 0x3000000b I've patched it via diff --git a/riscv-test-suite/env/arch_test.h b/riscv-test-suite/env/arch_test.h index aaaaaaa..bbbbbbb 100644 --- a/riscv-test-suite/env/arch_test.h +++ b/riscv-test-suite/env/arch_test.h @@ -1013,6 +1013,8 @@ common_\__MODE__\()entry: //spcl case handling for ECALL in GOTO_MMODE mode,) ****tests can't use ECALL t2=0**** spcl_\__MODE__\()2mmode_test: csrr t5, CSR_XCAUSE + li t4, ~0x7fff0000 // bits reserved for mpp, mpie, etc + and t5, t5, t4 // clear such bits addi t4, t5, -8 // is cause 8..11? Mmode should avoid ECALL 0 andi t4, t4, -4 // NOTE: cause 10 is RSVD. Sail will diverge, but buggy anyway bnez t4, \__MODE__\()trapsig_ptr_upd // no, not in special mode, just continue @@ -1068,6 +1070,7 @@ rvtest_\__MODE__\()endtest: // target may be too far away, s //------end atomic------------------------------------------------ LREG t3, xtvec_new_off(sp) // get pointer to actual tramp table + andi t3, t3, -4 // clear CLIC-related bits //---------------------------------------------------------------- /*************************************************************************/ , because it is simple and it works. Of course this does not test if CLIC is enabled (which can not be checked via isa-string) or even implemented. I'm good with what I've done, but if one needs to run arch-tests with CLIC enabled, this should be adressed. — Reply to this email directly, view it on GitHub <#285 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJXAEFKZFKMBXFZVHX3XGX7Z5ANCNFSM6AAAAAAQRJLILE> . You are receiving this because you commented.Message ID: ***@***.***>