mirror of
https://github.com/riscv-software-src/opensbi.git
synced 2026-06-17 16:41:19 +01:00
Bo Gan
56c39d1f08
Rehaul instruction decoding to fix the following issues:
- We assume the XLEN of previous mode is the same as MXLEN. However,
RVC instructions decodes differently in RV32 and RV64, so shouldn't
have assumed that.
- We assume it's a misaligned fault and the load/store offset is 0,
i.e., base address == fault address, but access faults can have
non-0 offset (on HW supporting misaligned accesses), so platform
specific load/store fault handler gets the wrong base address.
- No checking of [63:32] of tinst in RV64, which is explicitly
required by Privileged ISA 19.6.3. Must reject tinst with non-0
high 32 bits.
Thus, fix all the above. For misaligned load/store fault, the address
offset should be 0, but we'll validate that on a DEBUG build. On an
optmized build, we kill the use of base address, and use trap address
instead (same as before), which lets the compiler optimize out imm
parsing and other calculations.
I also analyzed the behavior of misaligned fault handler before fix.
With the following conditions met, it can trigger data corruption:
- HW doesn't transform instruction into tinst.
- HW doesn't support misaligned load/store, and OS doesn't enable
misaligned delegation, thus OpenSBI handler is in effect
- HW supports mixed XLEN, and M mode is running RV64, and the trapping
mode (U/VS/VU) is running RV32.
- The trapping instruction is c.f{l|s}w(sp).
Due to the incorrect insn decoding, the trapping instruction would
mistakenly be decoded as c.{l|s}d(sp). With this fix, c.f{l|s}w(sp)
in RV32 is now emulated correctly.
Validation:
The patch is validated to have fixed the issue with test cases running
on a modified version of QEMU that exposes misaligned faults [1], and
a further modified version that removes tinst transformation [2]. The
S-mode OS is a local build of Debian Trixie 6.12 kernel that enables
COMPAT (RV32), and the U-mode test application exercises all integer
and floating-point load/store (RVIFD64/32+RVC64/32) instructions with
all possible imm values. The patch is also tested on real HW (Sifive
P550/ESWIN EIC7700), which only supports RV64. On P550, the same test
was validated both in U mode and VU mode, where the host runs a 6.12
ESWIN vendor kernel that has some ESWIN SoC device driver patches [3]
applied, and the guest runs the exact same Debian Trixie 6.12 kernel
mentioned above.
[1] https://github.com/ganboing/qemu/tree/ganboing-misalign
[2] https://github.com/ganboing/qemu/tree/ganboing-misalign-no-tinst
[3] https://github.com/sifiveinc/riscv-linux/tree/rel/kernel-6.12/hifive-premier-p550
Fixes: 7219477f7b ("lib: Use MTINST CSR in misaligned load/store emulation")
Fixes: b5ae8e8a65 ("lib: Add misaligned load/store trap handling")
Fixes: 4c112650bb ("lib: sbi: abstract out insn decoding to unify mem fault handlers")
Signed-off-by: Bo Gan <ganboing@gmail.com>
Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260605113214.242-8-ganboing@gmail.com
Signed-off-by: Anup Patel <anup@brainfault.org>