I-MISALIGN_JMP-01.S outdated use of mbadaddr in trap handler?

[debs-sifive]

From line 280, the beginning of the trap handler has:

_trap_handler:
    # increment return address
    csrr    x30, mbadaddr
    addi    x30, x30, -2
    csrw    mepc, x30

The privileged spec states, "The mtval register replaces the mbadaddr register in the previous specification."

It also states, "When a hardware breakpoint is triggered, or an instruction-fetch, load, or store address-misaligned, access, or page-fault exception occurs, mtval is written with the faulting e�ffective address... For other exceptions, mtval is set to zero..."

So within the trap handler, mtval is used in place of mbadaddr, but the behavior of the two do not match up, since the spec does not actually define mtval behavior during misaligned jumps.

What I'm seeing is mtvec (i.e. mbadaddr in the test) being set to 0x0 as the spec says, so then mepc gets set to some large invalid address, which causes mret to return back into the trap handler, and then gets stuck in a loop.

[neelgala]

Agreed that mtval should replace mbadaddr not only here but in all other tests as well.
Will probably raise a pull-request for the same soon. Thanks for the catch.

I think the problem here is how you have interpreted the spec. I would interpret the following
... or an instruction-fetch, load, or store address-misaligned, access, or page-fault exception occurs, mtval is written with the faulting effective address ... as:

instruction-fetch-misaligned, load-address-misaligned, store-address-misaligned,
instruction-fetch-access, load-address-access, store-address-access,
instruction-fetch-pagefault, load-address-pagefault and store-address-pagefault exceptions.

Also there is no exception defined as: instruction-fetch-exception.

In which case a jal/jalr instruction which causes an instruction-fetch-misaligned exception should update mtval with the corresponding instruction-fetch-address.

Also you can find the same behaviour in spike and riscvOPVSim and also quite a few other implementations.

[debs-sifive]

Ah, that makes more sense! Thanks for your help!

[debs-sifive]

I missed this earlier, but in the paragraph above, the spec states:

"When a trap is taken into M-mode, mtval is either set to zero or written with exception-specific information to assist software in handling the trap. Otherwise, mtval is never written by the implementation, though it may be explicitly written by software. The hardware platform will specify which exceptions must set mtval informatively and which may unconditionally set it to zero."

So the behavior desired by the test isn't actually required by spec. This seems like it would render several tests that use mtval less useful for some targets. Thoughts?

[aswaterman]

I think the ISA-compliance tests should be revised to permit either zero or the expected nonzero value. These tests will still be useful. Implementations will choose to write the expected nonzero value for exceptions that are meant to be resumable, and we still want to test that they produce the right value if they do produce a value.

Platform-compliance tests should be responsible for checking whether mtval is populated with nonzero values for the exceptions that platform cares about (e.g., misaligned ld/st on platforms that emulate those operations; all page-fault exceptions on platforms that use virtual memory).

[allenjbaum]

The privileged compliance tests haven't really been written yet; but when they are, the framework should be presented with the platform configurations parameter as a platform specific function which returns the value of MTVAL for specific faults. That is a ways away, though there are hooks in the framework(s) to do this.

…

On Mon, Jan 28, 2019 at 12:06 PM Andrew Waterman ***@***.***> wrote: I think the ISA-compliance tests should be revised to permit either zero or the expected nonzero value. These tests will still be useful. Implementations will choose to write the expected nonzero value for exceptions that are meant to be resumable, and we still want to test that they produce the right value if they do produce a value. Platform-compliance tests should be responsible for checking whether mtval is populated with nonzero values for the exceptions that platform cares about (e.g., misaligned ld/st on platforms that emulate those operations; all page-fault exceptions on platforms that use virtual memory). — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#31 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/Ad96plCYMWOt7KgvFNwjRKnkupqww7I5ks5vH1hagaJpZM4aTkC5> .

[allenjbaum]

Note that the riscof framework will handle this correctly by dreclaring that MTVAL is unimplemented in the DUT YAML configuration file. The reference model will need to allow that kind of configuration (and many, many others) as well.

[allenjbaum]

So, I am unclear: is it possible to modify the test so it works for both MTVAL implemented or not?
I thought the current v.1 framework only allows a single assertion, which make it unfixable in v.1