Fixes for load/store misaligned and access faults
Status from this message: Applied all patches except last one to the riscv/opensbi repo.
Get series with b4
b4 am [email protected]Versions newest first · linked by author and title
- v2 Latest8 patches 8 repliesApplied
- v17 patches 10 repliesSuperseded
Changes in v2 from the cover letter
- Addressed Anup's comment for PATCH 5 in v1
- Validate load/store offset is 0 in misaligned faults w/ DEBUG build
Patches in v2 Applied
| # | Subject and files | Size | Replies | Applied |
|---|---|---|---|---|
| 1/8 | include: sbi: Add more mstatus and instruction encodinginclude/ | +18 | 0 replies | |
| 2/8 | include: sbi: Add sbi_regs_prev_xleninclude/ | +58 | 0 replies | |
| 3/8 | include: sbi: Add GET_RDS_NUM/SET(_FP32/_FP64)_RDS macrosinclude/ include/ include/ | +20 | 0 replies | |
| 4/8 | include: sbi: set FS dirty in vsstatus when V=1include/ | +5 | 0 replies | |
| 5/8 | lib: sbi: Do not override emulator callback for vector load/storeinclude/ include/ lib/ +1 more | +139 | 1 reply | |
| 6/8 | Makefile: define OPENSBI_DEBUG if DEBUG buildsMakefile | +1 | 1 reply | |
| 7/8 | lib: sbi: Rework load/store emulator instruction decodinglib/ | +308 | 1 reply | |
| 8/8 | Test program for misaligned load/storetests/ tests/ tests/ | +458 | 1 reply |
Reviews trailers per patch
7 of 8 patches have a Reviewed-by or Acked-by.
| Patch | Anup Patel | Anirudh Srinivasan |
|---|---|---|
| Series | T-b | |
| 1/8 | R-b | |
| 2/8 | R-b | |
| 3/8 | R-b | |
| 4/8 | R-b | |
| 5/8 | R-b | |
| 6/8 | R-b | |
| 7/8 | R-b | |
| 8/8 |
Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.
Cover letter [PATCH v2 0/8] Fixes for load/store misaligned and access faults
Re-visit the load/store misaligned and access fault handlers to fix issues related to coding patterns, floating-point state, and instruction decoding:
a. Vector misaligned load/store emulator is invoked improperly b. vsstatus.FS is not set dirty when V=1
c. No checking of previous XLEN, resulting in wrong insn decoding d. Load/Store base address is wrongly assumed to be trap address e. High 32-bits of tinst needs to be checked against 0
The pathset is validated on a modified QEMU[1] that exposed misaligned faults, and QEMU[2], which further disabled the insn transformation.
[2] covers more insn decoding branches, including the particular ones which can trigger the wrong decoding (c)
The patchset is also validated on Sifive P550 core (ESWIN EIC7700), which is RV64-only in M/(V)S/(V)U, no HW misaligned support, no vector, and mtinst always 0. Refer to [3] for the git repo/branch that has all 6 patches applied along with the test case that exercises all integer and floating-point load/store instructions.
Test case used is available in PATCH 8.
There's no change to the behavior of the vector misaligned load/store handler. However, I've found additional issues with them:
- uint8_t mask[VLEN_MAX / 8] in sbi_trap_v_ldst.c is 8KB, which can overflow the default 4KB stack.
- tinst should be zero'ed out to not confuse previous mode when redirecting faults, otherwise the vector insn can be mistaken as a regular load/store.
- VS in previous mode must be set dirty for loads.
These will be addressed in follow-up patches.
[1] https://github.com/ganboing/qemu/tree/ganboing-misalign
[2] https://github.com/ganboing/qemu/tree/ganboing-misalign-no-tinst
[3] https://github.com/ganboing/opensbi/tree/fix-ldst-v2
---
Changes in v2:
- Addressed Anup's comment for PATCH 5 in v1
- Validate load/store offset is 0 in misaligned faults w/ DEBUG build
---
Bo Gan (8):
include: sbi: Add more mstatus and instruction encoding
include: sbi: Add sbi_regs_prev_xlen
include: sbi: Add GET_RDS_NUM/SET(_FP32/_FP64)_RDS macros
include: sbi: set FS dirty in vsstatus when V=1
lib: sbi: Do not override emulator callback for vector load/store
Makefile: define OPENSBI_DEBUG if DEBUG builds
lib: sbi: Rework load/store emulator instruction decoding [NOT-FOR-UPSTREAM] Test program for misaligned load/store
Makefile | 1 + include/sbi/riscv_encoding.h | 21 +- include/sbi/riscv_fp.h | 30 ++- include/sbi/sbi_platform.h | 92 +++++-- include/sbi/sbi_trap.h | 59 ++++ include/sbi/sbi_trap_ldst.h | 4 +- lib/sbi/sbi_trap_ldst.c | 510 ++++++++++++++++++++++++----------- lib/sbi/sbi_trap_v_ldst.c | 25 +- tests/ldst.S | 134 +++++++++ tests/ldst.h | 170 ++++++++++++ tests/test-misaligned-ldst.c | 154 +++++++++++ 11 files changed, 994 insertions(+), 206 deletions(-) create mode 100644 tests/ldst.S create mode 100644 tests/ldst.h create mode 100644 tests/test-misaligned-ldst.c
Signature · 7 lines
2.34.1 -- opensbi mailing list [email protected] http://lists.infradead.org/mailman/listinfo/opensbi