Skip to content
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

Added Tests and Coverage for Exception Handling(Illegal instruction, environment calls and misaligned address exceptions) and Trap State Restoration Exceptions #605

Open
wants to merge 22 commits into
base: dev
Choose a base branch
from

Conversation

maria-rehman
Copy link

Description

Includes tests and coverage of illegal instructions, environment calls, address misaligned exceptions, trap entry/exit state handling, and validation of mcause, scause, mtval, and stval values.

Related Issues

N/A

Ratified/Unratified Extensions

  • Ratified

List Extensions

RV32I: Base integer instruction set
S: Supervisor mode extension
U: User mode Extension
F: Single-precision floating-point extension
D: Double-Precision Floating-Point
Zicsr: Control and status register instructions

Reference Model Used

  • SAIL

Mandatory Checklist:

All tests are compliant with the test-format spec present in this repo
Ran the new tests on RISCOF with SAIL/Spike as reference model successfully
Ran the new tests on RISCOF in coverage mode
Link to Google-Drive folder containing the new coverage reports: > https://drive.google.com/drive/folders/1XwiTjZUGTCR7KKmygZ9NpDvy9ydp-lGH?usp=drive_link

Optional Checklist:

  • Tests are hand-written
  • Ran these tests on DUT model(Spike)
  • Modified test_macros.h file, added one macro to off mstatus.FS bits (RVTEST_FP_DISABLE)

Copy link
Collaborator

@allenjbaum allenjbaum left a 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must ensure that x2=0x1111_1111 because the trap handler will treat that as a parameter to the handler.
(if X2==0, this is GOTO_MMODE, =above value, 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
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, T2 must ==0x1111_1111 else it will be treated as a test failure (note that if we ever change the existing ECALL test, this will have to change also)

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
Copy link
Collaborator

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)
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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
Copy link
Collaborator

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



// user ecall
ecall
Copy link
Collaborator

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=0x1111_1111



LI (a4, 3)
csrrc t1,0xFFF,a4 // accessing non existing CSR will generate the illegal instruction exception
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no guaratnee this will cause an excpetion

nop
nop

ecall // This exception will be handled in the Supervisor mode
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

set x2 correctly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants