Allow for non-word aligned mtvec

[vogelpi]

I just wanted to raise awareness that the RISC-V compliance suite currently seems not respect coarser alignment restrictions on the BASE field for mtvec of a given target implementation.

The minimum alignment mandated by the RISC-V Privileged Spec v.1.11 is 4-byte but it explicitly allows for coarser restrictions:

If mtvec is writable, the set of values the register may hold can vary by implementation. The value in the BASE field must always be aligned on a 4-byte boundary, and the MODE setting may impose additional alignment constraints on the value in the BASE field.

An implementation may have different alignment constraints for different modes. In particular, MODE=Vectored may have stricter alignment constraints than MODE=Direct.

Allowing coarser alignments in Vectored mode enables vectoring to be implemented without a hardware adder circuit.

Ibex requires mtvec to be 256-byte aligned. Since the RISC-V compliance suite currently requires a finer alignment, the I-EBREAK and I-ECALL tests keep failing.

We are not sure what your view on this is, but we thought this should maybe be tracked here. For further information, please also have a look here lowRISC/ibex#428, lowRISC/ibex#99, lowRISC/ibex#100.

[vogelpi]

I am also pinging @eroom1966 as we have discussed this a while ago.

[allenjbaum]

The riscof compliance framework (currently under review) does have a method of specifying restrictions on WARL fields, and the example you give is a prime example of why. However, the current compliance suite has poor coverage of any privileged mode operations (and interrupts are an example of that, as are any privileged mode CSRS), And, the existing privileged mode tests are self-checking; they flag mismatched expected result as a literal in the test, and the were written to expect a Rocket core (or so I understand) , which doesn't have those restrictions. The upcoming compliance framework will instead compare an implementation (Ibex in this example) with a formal model. The implications of this is that the - the framework must be supplied with a description of the implementation options - the formal model must use that description to restrict what it models (in this case, it would ignore writes to MTVEC[7:0] and keep them as 0) So, when the formal model is released with those capabilities (I believe it is being worked on) then tests which compare instead of self-check should run correctly. More details are available here: Targets and framework (incl. tests ) are now in separate repositories. RISCOF: https://gitlab.com/incoresemi/riscof . YAML based framework, test-pool and relevant docs. RISCOF-Targets: https://gitlab.com/incoresemi/riscof-plugins plugins for various targets YAML description:https://github.com/riscv/riscv-config

…

On Fri, Oct 25, 2019 at 8:26 AM Pirmin Vogel ***@***.***> wrote: I am also pinging @eroom1966 <https://github.com/eroom1966> as we have discussed this a while ago. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#72>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJXRU626VY2L7BR4XZDQQMFZLANCNFSM4JFEYYDQ> .

[vogelpi]

Thanks @allenjbaum for your quick response. I am happy to learn that this problem is being addressed.

[allenjbaum]

Well, it will be addressed in the future. It isn't clear to me that it is possible to fix this in v.1, though.

[neelgala]

Therer are currently 4 tests: I-EBREAK, I-ECALL, I-MISALIGN_JMP and I-MISALIGN_LDST which require access and update of mtvec. While riscof accounts for capturing the platform specifc constraints for mtvec, these tests would also have to be updated along with a linker script changes as well to account for this issue.

[allenjbaum]

Is this possible to fix in the current framework to handle arbitrary alignments?
if we only need to modify linker scripts which are already implementation specific, that is doable
If we need to modify tests, that will be implementation specific, and until we can import YAML parameters, we can't have a general test, in which case this should be deferred until we get to v.2.
I don't think we have a way of "deferring" issues; should this get a "fixed in riscof" tag?

[eroom1966]

I think the issue here is that the idiom adopted by Andrew Waterman was used early in the test development, so that the mtvec address is set to be the address of an instruction after the faulting address, in other words (pseudo code)

l1: some_instruction
l2: set mtvec = &l4
l3: faulting_instruction
l4: some_instruction

if the intention is to use a READONLY MTVEC, then a trap handler must be written and located at the fixed MTVEC address, this traphandler could read the EPC and perform a jump to EPC+2 (compressed) or EPC+4 (uncompressed) thus resuming execution at l4

I think it is a test issue, not a framework issue

Thx
Lee

[allenjbaum]

Not only do these tests need to be different depending on whether the DUT supports unaligned ops or not, but also requires different trap handling depending on the WARL characteristics of mtvec. The latter might be able to be handled with model specific linker script (and test changes), but the former requires test changes or new tests (plus a way to select different tests, plus different reference signatures) and that can't be done in v.1. Unless someone volunteers to write more tests and linker scripts, I'd rather defer this to v.2. A better approach would be to replace the instruction at the MTVEC location with a branch to the handler (so we don't care about mtvec alignment or writability constraints), and restore it when the test ends. (that breaks if the MTVEC location isn't RW - but then we can't test any of this anyway) (It's actually more complicated than that, because for fixed MTVEC we can't guarantee that the branch offset is long enough to get to the handler, which means we need to either save a register first using mscratch, or mscratch points to the actual handler) I've been talking with Neel about a standard interrupt handling macro that would alleviate this in general. In addition to saving a trap signature (at the end of signature area, running backwards) it always returns to a 4B aligned instruction past EPC, regardless of the length or alignment of the op itself. Those tests will need NOP padding after an op that is expected to trap. A standard test prolog, included by the RVTEST_CODE_BEGIN macro can perform the MTVEC op save and replacement, and RVMODEL_HALT can restore it. Not for v.1, though.

…

On Wed, Apr 1, 2020 at 8:02 AM eroom ***@***.***> wrote: I think the issue here is that the idiom adopted by Andrew Waterman was used early in the test development, so that the mtvec address is set to be the address of an instruction after the faulting address, in other words (pseudo code) l1: some_instruction l2: set mtvec = &l4 l3: faulting_instruction l4: some_instruction if the intention is to use a READONLY MTVEC, then a trap handler must be written and located at the fixed MTVEC address, this traphandler could read the EPC and perform a jump to EPC+2 (compressed) or EPC+4 (uncompressed) thus resuming execution at l4 I think it is a test issue, not a framework issue Thx Lee — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#72 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AHPXVJQCLOC4NXLRGPWX6MLRKNJR5ANCNFSM4JFEYYDQ> .

[allenjbaum]

OK, the tests that have just merged have a standard trap handler that any test can use.
The prolog/setup sets mtvec to a trampoline in the handler that enables saving trap state.
If it is unable to set mtvec to that handler, it instead replaces whatever mtvec points to with the trampoline code.
Only if that fails does it give up & the test fails
A future version may allow implementors to point to a RWX area to place the trampoline table. So:

• An implementation that has a fixed MTVEC that vectors to ROM will always fail
• An implementation that has partially writable MTVEC that is initialized point to ROM when the initialization routine is entered will fail.
  Otherwise, it should work.

[allenjbaum]

Has this been retested with the updated RFQ tests?
If not, I suggest you reclone the entire riscv-arch-test (the new name for the old riscv-compliance repo) repo and try again
The new tests should save and overwrite the region that MTVEC points to with a trap vector table, and restore it after the test. Any traps will then get vectored to ther standard trap handler, save trap state, and return past the trap point.

[allenjbaum]

Seeing no comments, I believe this has been fixed in the latest version of the tests; that is, the tests that were failing because of mtvec alignment shouldn't be failing now. I am closing this, but please reopen if you see further failures.