search

riscv-non-isa / riscv-arch-test

https://jira.riscv.org/browse/RVG-141?src=confmacro
Apache License 2.0
514 stars 199 forks source link

Test I-EBREAK-01.S #145

Closed Imperas closed 3 years ago

Imperas commented 3 years ago

This test writes values to the MTVEC register and assumes that all bits are writable. this is not a requirement of the RISCV spec, and some bits of the MTVEC register can be omitted in the case where the exception handlers are expected to be aligned to a certain byte boundary

Additionally, this test assumes that the DEBUG mode is not implemented on the target device, and that an EBREAK instruction will cause an exception to occur, rather than a mode switch to DEBUG mode

I suggest this test is deprecated, until it can be enhanced to (at least) cover the above.

pdonahue-ventana commented 3 years ago

Additionally, this test assumes that the DEBUG mode is not implemented on the target device, and that an EBREAK instruction will cause an exception to occur, rather than a mode switch to DEBUG mode

Even if debug mode is implemented, an ebreak instruction will always cause an exception to occur unless the appropriate dcsr.ebreak* bit is set. Those dcsr bits reset to 0 and essentially can only be set by an external debugger. Therefore, this sentence is not applicable.

The mtvec assumption is a valid concern.

allenjbaum commented 3 years ago

I believe the MTVEC problem will go away when the RFQ tests are pulled in. The new standard trap handler prolog will attempt to point MTVEC to the handler entr. If that fails, it will leave the current value unchanged, but replace the code that it points to with trampoline code. If MTVEC is not writable, and the code it points to is not writable, then the test will fail. IF this is inadequate, then we have two fallback plans: force trampoline alignment to 4K, which will probably fix most problems, or allow the model to supply a writable trampoline address that MTVEC can be set to.

Imperas commented 3 years ago

So until resolved at some point in the future...

I suggest this test is deprecated, until it can be enhanced to (at least) cover the above.

Do you not agree ?

allenjbaum commented 3 years ago

No, I do not. These tests appear to wrk for the majority of implementations (based on the fact that only two have reported that they don't pass) and there is no reason to weaken the already weak testing to accommodate those implementations. The consensus is that we will add a note into the README that indicates that the tests in the repo assume specific architectural options (specifically: MTVEC is completely writable, and that HW misalign is not supported) and that implementations that fail tests that assume the opposite should apply for a waiver when applying for the trademark.

allenjbaum commented 3 years ago

The README now explicitly lists the conditions under which this test will succeed, so I'm closing this. As above, implementations that cannot support either of those conditions can apply for a waiver.