-
Couldn't load subscription status.
- Fork 239
Added Tests and Coverage for Exception Handling(Illegal instruction, environment calls and misaligned address exceptions) and Trap State Restoration Exceptions #605
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Conversation
…/s cause m/s tval csr after trap handling
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This has a misuse of ECALL in the test. IT should always set x2 to 0x1111_1111 to be handled by the trap handler properly. There also a misunderstanding of how unimplemented CSR are not handled in the spec (not guarnateed to trap), how misaligned occurs (may or may not trap), coments that don't seem to match the code.
The same procedue is used for all the test; it would make it more readable if you created a test macro for all the boilerplate to (greatly) reduce test size
| // get the address of the ecall instruction verify by checking both registers values | ||
|
|
||
| // machine ecall | ||
| ecall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless the instruction immediately previous to ecall is a beq x,x,.+4, this must ensure that x2!=0 because the trap handler will treat that as a parameter to the handler.
(if X2==0, this is GOTO_MMODE, !=0, it's the ECALL test, anything is is a test failure)
| LI (a4, CSR_MEDELEG) | ||
| csrw medeleg, a4 | ||
|
|
||
| ecall // This exception will be handled in the machine mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unless the instruction immediately previous to ecall is a beq x,x,.+4, this must ensure that x2!=0 because the trap handler will treat that as a parameter to the handler.
(if X2==0, this is GOTO_MMODE, !=0, it's the ECALL test, anything is is a test failure)
| li t0, 0x3FC00000 // Hex representation of 1.5 in IEEE-754 single-precision | ||
| li t1, 0x40200000 // Hex representation of 2.5 in IEEE-754 single-precision | ||
|
|
||
| // Move these values into floating-point registers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test should also try a csrrd x2, FCSR (and FRM and FFLAGS to ensure that they trap also
| // mcause exception code will be "4" in U mode | ||
| */ | ||
|
|
||
| LI (a4, 0x0008) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is already tested in the misalign_lw_01.S test quite extensively. Also note that it very well might not trap, depending on misalign granularity
|
|
||
| // This test will check that in machine mode, a store address misaligned exception is raised when we try | ||
| // to attempt to store data to a misaligned memory address. For this we will check the log file | ||
| // that either it is giving the expected exception at that point and value of the mcause is "6" in M mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment: this appears to be redundant with (much more extensive) tests that already exist.
| /* When we try to jump to the misaligned address then instead of instruction misaligned adddress it gives the illegal instruction exception */ | ||
|
|
||
|
|
||
| // This test will check that in Supervisor mode, a load address misaligned exception will be raised |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note that this may or may not trap, depending on the capabilities of the core
|
|
||
| LA(x17,rvtest_data) | ||
| sw t1,0(x17) // will not give any exception | ||
| sw t2,3(x17) // will cause the exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no guarantee that this will trap,. This should replace the first nop with li -1, and the 2nd nop with a SIGUPD.
|
|
||
|
|
||
| // user ecall | ||
| ecall |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
usuall ecall issue: needs x2!=0
|
|
||
|
|
||
| LI (a4, 3) | ||
| csrrc t1,0xFFF,a4 // accessing non existing CSR will generate the illegal instruction exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no guarantee this will cause an exception. FFF is a custom readonly register, so there is no guarnatee what it will do if you try to execute this. Note that the you are in Umode at this time, so if you simply want to generate an illegal op exception, any legal mmode CSR will be safer and will guarantee the trap. But if the purpose is to show that a non-existent CSR access will trap - it's not possible unless Ssstrict extension is implemented (I think). At some point there may be a way to configure Sail to determine which CSRs will result in a trap if accessed, but even when that comes, you'll need to define it with an RVMODEL_CSR_TRAP variable (and skip the test if it isn't defined), so each implementation will have a slightly different test (but will have the same signature)
| nop | ||
| nop | ||
|
|
||
| ecall // This exception will be handled in the Supervisor mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make sure to set x2 correctly (!=0) Also: this is a case where you can never get back to Mmode, so you need to install a special illegal_op handler (which should uninstall itself) to get back
| nop | ||
| nop | ||
|
|
||
| LI (a4, 0x0003) // this signature will give information that test for accessing another mode csr has successfully returned from the trap handler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the trap handler wasn't ever entered, this would make it seem like it did.
The proper way to do this would be to preload a4 with some value X before the possible trap, load it immediately after the CSR write with a different value Y, and then store it. If the trap is entirely skipped, you'll see Y If it does trap and properly return you should see X.
|
|
||
| // Try to write the supervisor mode "read only CSR" then exception in the S mode will occur | ||
| LI (a4, 3) | ||
| csrrw t1, 0xDC0, a4 // this will give an illegal instruction as trying to write the read only register of supervisor mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if CSR 0xDC0 has a name, it should be used. If it doesn't have a name, then it shouldn't be used in any test. Ditto for 0xFFF, unless that is known to always exist and always cause a trap.
| // after handling the trap to ensure it shows the expected values for different exceptions. | ||
| // Additionally, it confirms that stval contains the correct values for various exceptions. | ||
|
|
||
| LI (a4, CSR_MEDELEG) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the wrong immediate; this will put the address of MEDELEG into the mdeleg CSR. You want to put the mask bit for the ECALL from Smode, which is 1<<CAUSE_SUPERVISOR_ECALL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also: this is extremely dangerous. Once you've delegated ECALL to Smode, you can't goto Mmode or clear MEDELEG without installing a special illegal opcode handler and executing an illegal opcode.
The special handler will clear medeleg, then jump to the goto_mmode handler.
There is an array of pointers at excpt__MODE_()hndlr_tbl, and the offset CAUSE_SUPERVISOR_ECALL8 must be filled in with the address of this special handler, which will clear MEDELEG, restore the pointer to its default value of CAUSE_SUPERVISOR_ECALL2+1, and branch to rtn2mmode:. Note that if you're executing this in Umode, you need to do the same thing, except with value CAUSE_USER_ECALL
We probably should define a macro to set and clear this handler because I'm sure it will come up again...
|
|
||
|
|
||
|
|
||
| // This test checks that if an extension is turned off (e.g., FS bits [13 and 14] in mstatus |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment isn't quite correct. It isn't any extension, this is specifically FP extensions.
I believe that includes F, as well as D, Zfh, BF26, Zfa (though they all depend on F).
You may want to test it with ops other than just FADD, (especially CSR reads of FCSR, FRM, and FFLAGS)
| nop | ||
| nop | ||
|
|
||
| LI (a4, 0x0009) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be deleted since it is handled as above
| nop | ||
| nop | ||
|
|
||
| LI (a4, 0x0003) // this signature will give information that test for accessing another mode csr has successfully returned from the trap handler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this would be a more robust test (here, and in all other instances!) if the LI () was moved above the CSR access, and the 2 nops after was replaced with a different LI() value.
|
|
||
| LI (a4, 2) | ||
| csrw satp, a4 // writing satp exceptions will occur here | ||
| csrr t3,satp |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these should be separated by at least one instruction, that instruction should reload or increment t4 by 1
| csrw satp, a4 // writing satp exceptions will occur here | ||
| csrr t3,satp | ||
| nop | ||
| nop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these nops should be replace with an increment of t4 by 2
| nop | ||
|
|
||
| sfence.vma // using sfence.vma but no faults occur | ||
| nop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This noop should be replaced by an increment of t4 by 4, and the following LI() should be removed.
What this will do is produce a unique value in t4, where the lower 3 bits indicate which (if any) of the preceding ops failed to trap.
| // delegated the environment call in U mode to be handled in S mode | ||
|
|
||
|
|
||
| LI (a4, 0x100) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please replace constant with a mnemonic, e.g. 1<<CAUSE_USER_ECALL
| li t0, 0x3FC00000 // Hex representation of 1.5 in IEEE-754 single-precision | ||
| li t1, 0x40200000 // Hex representation of 2.5 in IEEE-754 single-precision | ||
|
|
||
| // Move these values into floating-point registers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should move the LI above each op that is supposed to trap, replace each noop pair with a increment by 1<<op# (this should be done everywhere you have a series of ops that trap . This ensures that if a trap is not taken in self_test mode, it will be caught .
| /* When we try to jump to the misaligned address then instead of instruction misaligned adddress it gives the illegal instruction exception */ | ||
|
|
||
|
|
||
| // This test will check that in user mode, a load address misaligned exception will be raised |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you expect an exception to be raised? It won't if there is HW support of misaligned accesses.
Note that regardless of wehther it does or not, you don't need to change the test, as it will compare with whatever Sail does. But: you have a vunch of ops that might or might not trap. You should remove the last LI(), remove the nops, and insert addi a4,a4,1<<op# after each op that might trap (even if you think it won't!), as mentioned above. This should be done in every test, so I won't mention i t again.
|
|
||
| // illegal instruction exception in M mode | ||
| LI (t1, 0xdead) | ||
| csrw 0xFFF, t1 // accessing non existing CSR will generate the illegal instruction exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NOT guaranteed to trap! Use a different method for causing an illegal op trap. Also: don't you have several tests that do this already? You don't need another.
|
|
||
| // illegal instruction exception in S mode | ||
| LI (a4, 0xabcd) | ||
| csrrc t1,0xFFF,a4 // accessing non existing CSR will generate the illegal instruction exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as above: this doesn't work
|
|
||
| // illegal instruction exception in U mode | ||
| LI (a4, 0xbeef) | ||
| csrrs t1,0xFFF,a4 // accessing non existing CSR will generate the illegal instruction exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As above - not guaranteed to trap. Does this need to be a CSR access, or will execution of an allzero opcode work just as well?
| nop | ||
| nop | ||
|
|
||
| LI (a4, 0x4) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
USe a mnemonic, not a constant here, e.g. 1<<CAUSE_ILL_OP or whatever
|
|
||
| // illegal instruction exception in M mode | ||
| LI (t1, 0xdead) | ||
| csrw 0xFFF, t1 // accessing non existing CSR will generate the illegal instruction exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as above
|
|
||
| // illegal instruction exception in U mode | ||
| LI (a4, 0xbeef) | ||
| csrrs t1,0xFFF,a4 // accessing non existing CSR will generate the illegal instruction exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pick a different illegal op
|
|
||
| LI (a4, 0xcccc) | ||
| LA(x17,rvtest_data) | ||
| lw x5,1(x17) // exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SIGUPD after load tests should store the register that was loaded
| @@ -0,0 +1,290 @@ | |||
| /* | |||
| /* | |||
| Note: Trap here in this file is taken load store address misaligned traps | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no guarantee that any of these will trap, so you have to ensure that the noop after every load/store loads a value different than the LI() before the load/store. If the op that traps is a load, then the way to do that is for SIGUPD to use the register that was loaded into. Also, make it clear that you're looking at different modes, and different widths being loaded or stored.
|
|
||
| LI (a4, 0xaaaa) | ||
| LA(x17,rvtest_data) | ||
| lh x5,3(x17) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use lh, x4... instead of x5, so SIGUPD gets the data that was loaded, to make sure it loaded the right thing (or didn't load anything, in the case a trap was taken.
| nop | ||
|
|
||
| // Delegate and then check the same valuse for scause and stval// | ||
| LI (a4, 0x50) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace constant with nmemonic
| // Here below are the some opcode combinations that are not defined for any instruction | ||
| // so when we use such opcodes they give the illegal instruction exception | ||
|
|
||
| // "110_0111" // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Undefined opcodes are not guaranteed to trap unless Zsstrict is implemented.
All zero ops are guaranteed, but not sure what else is.
| // instruction, mtval will contain the binary representation of illegal instruction | ||
|
|
||
| LI (t1, 0xdead) | ||
| csrw 0xFFF, t1 // accessing non existing CSR will generate the illegal instruction exception |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, this will not necessarily generate an illgal int exception !
| // are 00), trying to access it will cause an illegal instruction error. | ||
|
|
||
|
|
||
| RVTEST_FP_DISABLE() // this will set the FS bit in mstatsus to 00 means off |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this work if the F extension isn't implemented (and you're not filtering the test based on that).
How would you test this if it isn't? You can't depend on any particular extension being implemented, or, if it is, that you can disable it (except FP)
| /* When we try to jump to the misaligned address then instead of instruction misaligned adddress it gives the illegal instruction exception */ | ||
|
|
||
| /* | ||
| // This test will check that in machine mode, a load address misaligned exception will be raised |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
haven't you done this in several other tests already?
| @@ -0,0 +1,262 @@ | |||
| /* // These tests are for the exceptions in U mode | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if thse M/U/S mode tests should just loop 3 times, with different GO_TO_LOWER mode each pass through the loop (and a BASEUPD each time through). You'd got the number of tests by 3x.
|
|
||
| #define RVTEST_FP_DISABLE() ;\ | ||
| csrr a0, mstatus ;\ | ||
| csrr a0, mtvec ;\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this instruction here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are persistent problems in all the tests that need fixing.
It is not the case that misaligned accesses will trap.
They may or may not depending on whether HW misalignment is handled in HW,
and whether it crosses specific address boundaries.
This may not actually affect the tests too much - as long as whatever happens is
duplicated by the reference model. But we do have an entire set of much more
comprehensive misaligned tests (except they don't check [v]Umode or [v]Smode
- I would instead ask that those tests be upgraded as I suggested, by looping
- through in each mode, changing modes at each loop with BASEUPD in between
It is no longer the case that accesses to undefined CSRs will trap
Accesses to unimplemented CSRs are RSVD unless Zsstrict is implemented;
then they will trap; otherwise, they're unpredictable, so you can't use them in a test
It is no longer the case that FP ops will trap if mstatus.FS=0
This is only the case if F-extension is supported.
If it isn't then those ops are undefined, RSVD unless Zsstrict is implemented;
then they will trap; otherwise, they're unpredictable, so you can't use them in a test
It is not the case that undefined opcodes will trap (with few exceptions) .
Execution of undefined opcode are RSVD unless Zsstrict is implemented;
then they will trap; otherwise, they're unpredictable, so you can't use them in a test
you can't use them in a test. An allzeros opcode is guaranteed.
Tests use 2 noops after ops that trap, and none after ops that aren't supposed to
Only one noop is needed, but they should all be replaced with a addI of 1<<op#.
A series of ops will then set different bits depending on whether it didn't trap (or did)
and a single SIGUPD will enable SELF_TEST mode to catch cases where a trap didn't
happen the should have.
Note that this should be done even if you expect that the op will not trap
Tests delegate ECALL to Smode
When this happens, special measures are required in advance to get back to Mmode:
Installing a special illegal op handler that resets mdeleg & then uninstalls itself
Tests must execute an illegal op that goes to this handler before GOTO_MMODE
Constants are used instead of mnemonics
Specifically for values of mdeleg masks, but there are probably others
Many tests cases appear to be duplicated or redundant
e.g. there are many tests that write CSR 0xFFF to cause an illegal op trap.
There only needs to be one of them per test suite, not one per test case
There are a lot of tests that are identical except for the mode they are execute from
The number of tests can be cut down by just executing the loop 3 times,
with different GOTO_LOWER modes each time (so, keep loop count in a reg,
and branch to different ones depending on loop count,
with a BASE_UPD macro between each to make sure offset is updated correctly
SIGUPD after load should update the register that was (supposed to be) loaded
Explicit use of ECALL must guarantee that x2 is nonzero (and that the previous op is not a beq .+4), else it will alias to GOTO_MMODE or SELFCHK_MISCOMPARE
Almost all the comments I added in
Description
Related Issues
Ratified/Unratified Extensions
List Extensions
Reference Model Used
Mandatory Checklist:
Optional Checklist: