Draft: Add m-mode, s-mode CLIC interrupt testcases

[dansmathers]

Description

Provide a detailed description of the changes performed by the PR.

This is a draft version of the m-mode (Smclic) and s-mode (Ssclic) CLIC interrupt testcases using clint MSW and MTIMER macros.

Note: pulls are not yet available for spike or sail that support CLIC but these testcases should help enable their development.

This pull requires:
riscv-software-src/riscv-config#169,
riscv-software-src/riscof#106
riscv-software-src/riscv-isa-sim#1596

To include m-mode CLIC interrupt tests in riscof testlist flow, add Smclic to riscof yaml file, e.g.:
spike/spike_isa.yaml:
ISA: RV32IMCZicsr_Zifencei_Smclic

To include s-mode CLIC interrupt tests in riscof testlist flow, add Ssclic to riscof yaml file, e.g.:
spike/spike_isa.yaml:
ISA: RV32IMCZicsr_Zifencei_Ssclic

Related Issues

Please list all the issues related to this PR. Use NA if no issues exist

Ratified/Unratified Extensions

• Ratified
• [x ] Unratified

List Extensions

List the extensions that your PR affects. In case of unratified extensions, please provide a link to the spec draft that was referred to make this PR.

Smclic, Ssclic

Reference Model Used

• [x ] SAIL - No CLIC implementation available yet. Tests complete but signatures will not match with actual implementations.
• [x ] Spike - No CLIC implementation available yet. Tests complete but signatures will not match with actual implementations.
• [x ] Other - < Dut with CLIC implemented >

Mandatory Checklist:

• [ x] All tests are compliant with the test-format spec present in this repo ?
• [ x] Ran the new tests on RISCOF with SAIL/Spike as reference model successfully ?
• [ x] Ran the new tests on RISCOF in coverage mode
• Link to Google-Drive folder containing the new coverage reports (See this for more info): N/A, CLIC not implemented on SAIL or spike yet.
• Link to PR in RISCV-ISAC from which the reports were generated : N/A
• Changelog entry created with a minor patch

Optional Checklist:

• RISCV-V CTG PR link if tests were generated using it : < SPECIFY HERE >
• Were the tests hand-written/modified ?
• [X ] Have you run these on any hard DUT model ? Please specify name and provide link if possible in the description
• If you have modified arch_test.h Please provide a detailed description of the changes in the Description section above.

[silabs-kjetil]

I notice that this PR includes specific test cases for CLIC, do you use CLINT mode when running the other non-CLIC related test cases that include a trap handler like ebreak and ecall?

In my case I am looking for a way to run the arch-test suite on a device that only supports CLIC mode. It looks like this scenario is not yet supported in the arch-test trap handling code.

Do you know of any effort to include CLIC support in the trap handling code inside the arch_test.h file?

[dansmathers]

Can you point me and @allenjbaum to the line you are having problems with in arch_test.h? arch_test.h is not supposed to be changing mtvec.mode (briefly looking at it, I think it looks like it is trying to do read/modify/write to mtvec). Allen developed arch_test.h and I'm sure would be willing to update it to fix any issues.

[silabs-kjetil]

Sure @dansmathers and @allenjbaum. It looks like the init_Mtvec function is preserving the mode bits, so that part should be fine. So far I have identified 3 potential issues in arch_test.h that makes it hard to use a pure CLIC device for the test cases that includes a trap handler.

ecall check fails
https://github.com/riscv-non-isa/riscv-arch-test/blob/main/riscv-test-suite/env/arch_test.h#L1160

This code is supposed to check for ecall, and exit early when it is detected. However the way the code is doing this does not work correctly when mcause contains extra bits like in the case when CLIC is used. Typically bits 29:28 will contain 0b11 so mcause will somtimes have the value 0x3000000B when read by the handler and the code will not detect that this is actually an ecall with exception code = 0xB

mcause is directly stored in the signature
https://github.com/riscv-non-isa/riscv-arch-test/blob/main/riscv-test-suite/env/arch_test.h#L1243

This means that any DUT using CLIC will never match the signature of a non-CLIC reference. This might be by design, however if the signature could filter out the exception/interrupt code then the signature would be equal between a CLIC DUT and a non-CLIC reference.

mode bits inside "vector" entry in signature
https://github.com/riscv-non-isa/riscv-arch-test/blob/main/riscv-test-suite/env/arch_test.h#L1222

It looks like the signature should contain the trampoline table (mtvec) offset (excluding mode bits), however the code brings in all the bits from mtvec including bits 1:0 containing the mode. These mode bits which are 0b11 in the CLIC case will cause artifacts in the signature. It also looks like the vector offset signature is supposed to be placed at bits 15:6 so shifted 5 bits to the left, however the code only shifts 4 bits left.

[allenjbaum]

1st point: arc_test.h will need to get updated for the new cause format. 2nd point: If the reference signature is not configured for CLIC, and the DUT is, you will get a mismatch - intentionally. They need to be configured the same way. 3rd point: I’ll look into that; arch test.h will need to be updated to save all relevant bits. Currently it compresses cause, the vector number, and the current mode into a single word, but this probably need to handle a larger vector as well as different cause bits. Thanks for pointing out exactly what needs to change. I’m more concerned that the trampoline is totally broken; that’s a harder problem to fix- as is re-entrancy which will be very hard to retrofit ( and test). We don’t have a way to introduce interrupts into tests, and even if we did, the timing won’t be reproducible between a reference model and DUT, so this will be tricky. I have a plan for getting close, but we need some programming resources to do it.

…

On Friday, March 8, 2024, Kjetil Østerås ***@***.***> wrote: Sure @dansmathers <https://github.com/dansmathers> and @allenjbaum <https://github.com/allenjbaum>. It looks like the init_Mtvec function is preserving the mode bits, so that part should be fine. So far I have identified 3 potential issues in arch_test.h that makes it hard to use a pure CLIC device for the test cases that includes a trap handler. *ecall check fails* https://github.com/riscv-non-isa/riscv-arch-test/blob/main/ riscv-test-suite/env/arch_test.h#L1160 This code is supposed to check for ecall, and exit early when it is detected. However the way the code is doing this does not work correctly when mcause contains extra bits like in the case when CLIC is used. Typically bits 29:28 will contain 0b11 so mcause will somtimes have the value 0x3000000B when read by the handler and the code will not detect that this is actually an ecall with exception code = 0xB *mcause is directly stored in the signature* https://github.com/riscv-non-isa/riscv-arch-test/blob/main/ riscv-test-suite/env/arch_test.h#L1243 This means that any DUT using CLIC will never match the signature of a non-CLIC reference. This might be by design, however if the signature could filter out the exception/interrupt code then the signature would be equal between a CLIC DUT and a non-CLIC reference. *mode bits inside "vector" entry in signature* https://github.com/riscv-non-isa/riscv-arch-test/blob/main/ riscv-test-suite/env/arch_test.h#L1222 It looks like the signature should contain the trampoline table (mtvec) offset (excluding mode bits), however the code brings in all the bits from mtvec including bits 1:0 containing the mode. These mode bits which are 0b11 in the CLIC case will cause artifacts in the signature. It also looks like the vector offset signature is supposed to be placed at bits 15:6 so shifted 5 bits to the left, however the code only shifts 4 bits left. — Reply to this email directly, view it on GitHub <#436 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJVEBTE3XZEMLBIDSKDYXGZPJAVCNFSM6AAAAABDA3JGHOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSOBVGY3DCNRUGM> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[silabs-kjetil]

Thank you @allenjbaum, if there are some changes that I can help out with then please reach out. I can start by creating issues in this project for the 1st and 3rd point so we can track it.

[silabs-kjetil]

Here are the two new issues created:

#440
#441

[xiaoweish]

Line 9 mentions "enable clicintie (default)".
However, will this default setting on Line 33 result in lines 211-213, which program interrupt1 CLICINTIE, being set to 0x0 (i.e., disabling clicintie)?

[smatherd]

In this testcase, there is no active interrupt. This testcase tests the behavior of the mcause.inhv bit. The inhv bit indicates that the hardware vectoring had an exception during the read of the hardware vector (e.g. page fault). after software clears the reason for the exception, mret sees inhv bit set and treats mepc as an address of an address in the hardware vector table instead of as an address of an instruction. It looks like I had a cut and paste error with the comments. Only this line is accurate: set mepc to shv table entry, set inhv, mret

[xiaoweish]

Could you please confirm whether the test logic for set mepc to SHV table entry, set inhv, mret is located between lines 246 and 256? If that is the case, it's worth noting that RVMODEL_WFI will result in the hart being halted in the WFI state, as interrupts are disabled by lines 32 to 37, meaning the test logic will not be triggered. Please correct me if I’m mistaken.

location_1:

    RVMODEL_WFI

    LI(     t0, RVMODEL_TEST_INHV)
    beqz t0, finish
    LA(     t0,mtvtval)
    csrw    CSR_MEPC, t0
    LI(     t0,0x40000000)
    csrrs   x0, CSR_MCAUSE, t0
    mret