riscv / riscv-test-env

https://jira.riscv.org/browse/RVG-141
Other
42 stars 107 forks source link

Assumptions about medeleg WARL behavior #17

Closed pdonahue-ventana closed 4 years ago

pdonahue-ventana commented 4 years ago

Of medeleg, the architecture says "An implementation can choose to subset the delegatable traps, with the supported delegatable bits found by writing one to every bit location, then reading back the value in medeleg or mideleg to see which bit positions hold a one." Presumably, an implementation can hard-wire medeleg to 0.

The spec also says "For exceptions that cannot occur in less privileged modes, the corresponding medeleg bits should be hardwired to zero." That means that in implementations that have misa.C hard-wired to 1, medeleg[0] is not only allowed to be hard-wired to 0 (as all bits are) but it is encouraged to be hard-wired to 0.

In riscv_test.h:

        li t0, (1 << CAUSE_LOAD_PAGE_FAULT) |                           \
               (1 << CAUSE_STORE_PAGE_FAULT) |                          \
               (1 << CAUSE_FETCH_PAGE_FAULT) |                          \
               (1 << CAUSE_MISALIGNED_FETCH) |                          \
               (1 << CAUSE_USER_ECALL) |                                \
               (1 << CAUSE_BREAKPOINT);                                 \
        csrw medeleg, t0;                                               \
        csrr t1, medeleg;                                               \
        bne t0, t1, other_exception;                                    \

This means that (if it's not an M-mode-only implementation) those 6 traps must be delegatable to S mode or else it goes to other_exception ("some unhandlable exception occurred"). This reset code should make no assumptions about the delegatability of traps in medeleg, especially misaligned fetches.

This reset code assumption will cause the riscv-tests csr.S test to fail if, for example, misaligned fetches are not delegatable due to misa.C=1 (although presence or absence of C support is not otherwise necessary to run that test).

pdonahue-ventana commented 4 years ago

This is a duplicate of https://github.com/riscv/riscv-test-env/issues/12 but since it has substantially more information about what the problem is, I'll leave it open.

aswaterman commented 4 years ago

I think it's OK to remove the bne, but of course some of the tests won't function on systems that don't support delegating those exceptions. I guess that's better than none of the tests functioning on such systems.

allenjbaum commented 4 years ago

The tests that fail are either buggy, or need to be conditioned on the implementation of the particular bit - which is something that the v.2 framework is supposed to deal with. But, the framework expects to filter tests, and the macros defined here are not part of any test, so that won't apply. We can remove the bne, but we will need write separate copies of any test that then fails (with uses all possible alternate WARL definitions), or be able to write a universal test that is passed a mask parameter (derived from the YAML). This will be an ongoing issue for every test that expects a specific WARL field implementation (which is almost all of them) and will be the major cost for getting adequate coverage of priv mode tests.