riscv / riscv-j-extension

Working Draft of the RISC-V J Extension Specification
https://jira.riscv.org/browse/RVG-128
Creative Commons Attribution 4.0 International
167 stars 17 forks source link

MXR in effect at the effective privilege level #83

Closed kdockser closed 1 month ago

kdockser commented 2 months ago

When MXR is in effect at the effective privilege mode where explicit memory access is performed, pointer masking does not apply.

Originally posted by @martinmaas in https://github.com/riscv/riscv-j-extension/issues/70#issuecomment-2123898322

This specification text is contradicted by other changes such as the non-normative statement recently made in https://github.com/riscv/riscv-j-extension/pull/82

"For the same reason, pointer masking does not apply when MXR is set."

It is also contradicted by https://github.com/riscv-software-src/riscv-isa-sim/pull/1784

if (!proc || proc->get_xlen() != 64 || (proc->state.sstatus->read() & MSTATUS_MXR) || flags.hlvx)

If these contradictory statements were true, the specification text could be simplified to

When mstatus.MXR is true, pointer masking does not apply.

Which is correct?

kdockser commented 2 months ago

To further clarify my question, what is the intention of the phrase: at the effective privilege mode where explicit memory access is performed In When MXR is in effect at the effective privilege mode where explicit memory access is performed, pointer masking does not apply.

Is it implying that the implementation needs to look at the MXR for the effective privilege mode. One of:

Furthermore, does this MXR disabling of pointer masking extend down to U and VU mode?

martinmaas commented 2 months ago

Apologies for the slightly delayed response. This was a detail where there was quite a bit of iteration with the Architecture Review Committee. The intention is that whichever MXR field governs a particular memory access (if there is such a field), that MXR field is used to determine whether pointer masking applies (if MXR is set, pointer masking does not apply; if it is not set or there is no governing MXR field, pointer masking does apply).

I believe the two quotes you mentioned in your first comment are consistent and correct in this regard. However, I think the Spike implementation might be slightly incorrect – here is how MXR is determined for regular memory accesses, which I think is the same logic that pointer masking would use as well:

https://github.com/riscv-software-src/riscv-isa-sim/blob/3c5b1bb09ef6daf4146e311d1303cb8dd67c5ff3/riscv/mmu.cc#L407

I think the reason this is correct is that (IIUC) the MXR bit exposed via sstatus is the same as the one in mstatus (the privileged spec states: "The mstatus bit MXR has been exposed to S-mode via sstatus"). I therefore think it is probably correct to only look at sstatus. The case where the current Spike implementation might be incorrect is 2-stage translation, since it is not taken into account here when it should be.

Is it implying that the implementation needs to look at the MXR for the effective privilege mode.

I believe this is the case. Let me add @aswaterman @Adam-pi3 to also take a look and confirm.

aswaterman commented 2 months ago

This was fixed in Spike over the last few days: https://github.com/riscv-software-src/riscv-isa-sim/pull/1791

Adam-pi3 commented 1 month ago

I'm aligned with @martinmaas analysis.

aswaterman commented 1 month ago

Since the Spike fix is consistent with Martin's analysis, I think we can probably close this one.