* [PATCH v2 1/8] include: sbi: Add more mstatus and instruction encoding
2026-06-05 11:32 [PATCH v2 0/8] Fixes for load/store misaligned and access faults Bo Gan
@ 2026-06-05 11:32 ` Bo Gan
2026-06-05 11:32 ` [PATCH v2 2/8] include: sbi: Add sbi_regs_prev_xlen Bo Gan
` (8 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Bo Gan @ 2026-06-05 11:32 UTC (permalink / raw)
To: opensbi, wangruikang, dramforever, andrew.jones
Cc: cleger, pjw, asrinivasan, Anup Patel
- Add MXL encoding for calculating XLEN.
- Add instruction encoding for c.lbu/c.sb,
and imm encoding for multiple RVC insn.
Signed-off-by: Bo Gan <ganboing@gmail.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
include/sbi/riscv_encoding.h | 20 ++++++++++++++++++--
1 file changed, 18 insertions(+), 2 deletions(-)
diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
index 18f7b4a7..535aed66 100644
--- a/include/sbi/riscv_encoding.h
+++ b/include/sbi/riscv_encoding.h
@@ -36,8 +36,10 @@
#define MSTATUS_SDT _UL(0x01000000)
#define MSTATUS32_SD _UL(0x80000000)
#if __riscv_xlen == 64
-#define MSTATUS_UXL _ULL(0x0000000300000000)
-#define MSTATUS_SXL _ULL(0x0000000C00000000)
+#define MSTATUS_UXL_SHIFT 32
+#define MSTATUS_UXL (_ULL(3) << MSTATUS_UXL_SHIFT)
+#define MSTATUS_SXL_SHIFT 34
+#define MSTATUS_SXL (_ULL(3) << MSTATUS_SXL_SHIFT)
#define MSTATUS_SBE _ULL(0x0000001000000000)
#define MSTATUS_MBE _ULL(0x0000002000000000)
#define MSTATUS_GVA _ULL(0x0000004000000000)
@@ -56,6 +58,9 @@
#endif
#define MSTATUS32_SD _UL(0x80000000)
#define MSTATUS64_SD _ULL(0x8000000000000000)
+#define MXL_XLEN_32 1
+#define MXL_XLEN_64 2
+#define MXL_TO_XLEN(x) (1U << (x + 4))
#define SSTATUS_SIE MSTATUS_SIE
#define SSTATUS_SPIE_SHIFT MSTATUS_SPIE_SHIFT
@@ -959,6 +964,10 @@
#define INSN_MATCH_C_FSWSP 0xe002
#define INSN_MASK_C_FSWSP 0xe003
+#define INSN_MATCH_C_LBU 0x8000
+#define INSN_MASK_C_LBU 0xfc03
+#define INSN_MATCH_C_SB 0x8800
+#define INSN_MASK_C_SB 0xfc03
#define INSN_MATCH_C_LHU 0x8400
#define INSN_MASK_C_LHU 0xfc43
#define INSN_MATCH_C_LH 0x8440
@@ -1388,6 +1397,9 @@
#define SH_RS2C 2
#define RV_X(x, s, n) (((x) >> (s)) & ((1 << (n)) - 1))
+#define RVC_LB_IMM(x) ((RV_X(x, 6, 1) << 0) | \
+ (RV_X(x, 5, 1) << 1))
+#define RVC_LH_IMM(x) (RV_X(x, 5, 1) << 1)
#define RVC_LW_IMM(x) ((RV_X(x, 6, 1) << 2) | \
(RV_X(x, 10, 3) << 3) | \
(RV_X(x, 5, 1) << 6))
@@ -1399,6 +1411,10 @@
#define RVC_LDSP_IMM(x) ((RV_X(x, 5, 2) << 3) | \
(RV_X(x, 12, 1) << 5) | \
(RV_X(x, 2, 3) << 6))
+#define RVC_SB_IMM(x) RVC_LB_IMM(x)
+#define RVC_SH_IMM(x) RVC_LH_IMM(x)
+#define RVC_SW_IMM(x) RVC_LW_IMM(x)
+#define RVC_SD_IMM(x) RVC_LD_IMM(x)
#define RVC_SWSP_IMM(x) ((RV_X(x, 9, 4) << 2) | \
(RV_X(x, 7, 2) << 6))
#define RVC_SDSP_IMM(x) ((RV_X(x, 10, 3) << 3) | \
--
2.34.1
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 2/8] include: sbi: Add sbi_regs_prev_xlen
2026-06-05 11:32 [PATCH v2 0/8] Fixes for load/store misaligned and access faults Bo Gan
2026-06-05 11:32 ` [PATCH v2 1/8] include: sbi: Add more mstatus and instruction encoding Bo Gan
@ 2026-06-05 11:32 ` Bo Gan
2026-06-05 11:32 ` [PATCH v2 3/8] include: sbi: Add GET_RDS_NUM/SET(_FP32/_FP64)_RDS macros Bo Gan
` (7 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Bo Gan @ 2026-06-05 11:32 UTC (permalink / raw)
To: opensbi, wangruikang, dramforever, andrew.jones
Cc: cleger, pjw, asrinivasan, Anup Patel
sbi_regs_prev_xlen reports the xlen of previous mode by decoding
from multiple CSRs including mstatus/hstatus/vsstatus
Signed-off-by: Bo Gan <ganboing@gmail.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
include/sbi/sbi_trap.h | 58 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 58 insertions(+)
diff --git a/include/sbi/sbi_trap.h b/include/sbi/sbi_trap.h
index 091a2446..3be418a3 100644
--- a/include/sbi/sbi_trap.h
+++ b/include/sbi/sbi_trap.h
@@ -273,6 +273,64 @@ static inline int sbi_mstatus_prev_mode(unsigned long mstatus)
return (mstatus & MSTATUS_MPP) >> MSTATUS_MPP_SHIFT;
}
+#if __riscv_xlen == 32
+static inline int sbi_regs_prev_xlen(const struct sbi_trap_regs *regs)
+{
+ return __riscv_xlen;
+}
+#else
+static inline int sbi_mstatus_sxl(unsigned long mstatus)
+{
+ return (mstatus & MSTATUS_SXL) >> MSTATUS_SXL_SHIFT;
+}
+
+static inline int sbi_mstatus_uxl(unsigned long mstatus)
+{
+ return (mstatus & MSTATUS_UXL) >> MSTATUS_UXL_SHIFT;
+}
+
+static inline int sbi_hstatus_vsxl(unsigned long hstatus)
+{
+ return (hstatus & HSTATUS_VSXL) >> HSTATUS_VSXL_SHIFT;
+}
+
+static inline int sbi_regs_prev_xlen(const struct sbi_trap_regs *regs)
+{
+ unsigned long hstatus, vsstatus;
+
+ if (!sbi_regs_from_virt(regs)) {
+ switch (sbi_mstatus_prev_mode(regs->mstatus)) {
+ case PRV_M:
+ return __riscv_xlen;
+ case PRV_S:
+ return MXL_TO_XLEN(sbi_mstatus_sxl(regs->mstatus));
+ case PRV_U:
+ return MXL_TO_XLEN(sbi_mstatus_uxl(regs->mstatus));
+ default:
+ __builtin_unreachable();
+ }
+ }
+
+ /* V=1, Check HSXLEN first */
+ if (sbi_mstatus_sxl(regs->mstatus) < MXL_XLEN_64)
+ return 32;
+
+ hstatus = csr_read(CSR_HSTATUS);
+ /* Check VSXLEN */
+ if (sbi_hstatus_vsxl(hstatus) < MXL_XLEN_64)
+ return 32;
+
+ vsstatus = csr_read(CSR_VSSTATUS);
+ switch (sbi_mstatus_prev_mode(regs->mstatus)) {
+ case PRV_S:
+ return MXL_TO_XLEN(sbi_hstatus_vsxl(hstatus));
+ case PRV_U:
+ return MXL_TO_XLEN(sbi_mstatus_uxl(vsstatus));
+ }
+ __builtin_unreachable();
+}
+#endif
+
int sbi_trap_redirect(struct sbi_trap_regs *regs,
const struct sbi_trap_info *trap);
--
2.34.1
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 3/8] include: sbi: Add GET_RDS_NUM/SET(_FP32/_FP64)_RDS macros
2026-06-05 11:32 [PATCH v2 0/8] Fixes for load/store misaligned and access faults Bo Gan
2026-06-05 11:32 ` [PATCH v2 1/8] include: sbi: Add more mstatus and instruction encoding Bo Gan
2026-06-05 11:32 ` [PATCH v2 2/8] include: sbi: Add sbi_regs_prev_xlen Bo Gan
@ 2026-06-05 11:32 ` Bo Gan
2026-06-05 11:32 ` [PATCH v2 4/8] include: sbi: set FS dirty in vsstatus when V=1 Bo Gan
` (6 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Bo Gan @ 2026-06-05 11:32 UTC (permalink / raw)
To: opensbi, wangruikang, dramforever, andrew.jones
Cc: cleger, pjw, asrinivasan, Anup Patel
These macros can be used to decode rd' and set rd' in RVC instructions
Signed-off-by: Bo Gan <ganboing@gmail.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
include/sbi/riscv_encoding.h | 1 +
include/sbi/riscv_fp.h | 24 ++++++++++++++++++------
include/sbi/sbi_trap.h | 1 +
3 files changed, 20 insertions(+), 6 deletions(-)
diff --git a/include/sbi/riscv_encoding.h b/include/sbi/riscv_encoding.h
index 535aed66..ffe7666c 100644
--- a/include/sbi/riscv_encoding.h
+++ b/include/sbi/riscv_encoding.h
@@ -1434,6 +1434,7 @@
#define GET_RS2S_NUM(insn) RVC_RS2S(insn)
#define GET_RS2C_NUM(insn) RVC_RS2(insn)
#define GET_RD_NUM(insn) ((insn & MASK_RD) >> SH_RD)
+#define GET_RDS_NUM(insn) RVC_RS2S(insn)
#define GET_CSR_NUM(insn) ((insn & MASK_CSR) >> SHIFT_CSR)
#define GET_AQRL(insn) ((insn & MASK_AQRL) >> SHIFT_AQRL)
diff --git a/include/sbi/riscv_fp.h b/include/sbi/riscv_fp.h
index f523c56e..03011af9 100644
--- a/include/sbi/riscv_fp.h
+++ b/include/sbi/riscv_fp.h
@@ -91,15 +91,27 @@
#define GET_F64_RS1(insn, regs) (GET_F64_REG(insn, 15, regs))
#define GET_F64_RS2(insn, regs) (GET_F64_REG(insn, 20, regs))
#define GET_F64_RS3(insn, regs) (GET_F64_REG(insn, 27, regs))
-#define SET_F32_RD(insn, regs, val) \
- (SET_F32_REG(insn, 7, regs, val), SET_FS_DIRTY(regs))
-#define SET_F64_RD(insn, regs, val) \
- (SET_F64_REG(insn, 7, regs, val), SET_FS_DIRTY(regs))
+#define SET_F32_RD(insn, regs, val) do { \
+ SET_F32_REG(insn, 7, regs, val); \
+ SET_FS_DIRTY(regs); \
+} while(0)
+#define SET_F64_RD(insn, regs, val) do { \
+ SET_F64_REG(insn, 7, regs, val); \
+ SET_FS_DIRTY(regs); \
+} while(0)
#define GET_F32_RS2C(insn, regs) (GET_F32_REG(insn, 2, regs))
-#define GET_F32_RS2S(insn, regs) (GET_F32_REG(RVC_RS2S(insn), 0, regs))
+#define GET_F32_RS2S(insn, regs) (GET_F32_REG(GET_RS2S_NUM(insn), 0, regs))
#define GET_F64_RS2C(insn, regs) (GET_F64_REG(insn, 2, regs))
-#define GET_F64_RS2S(insn, regs) (GET_F64_REG(RVC_RS2S(insn), 0, regs))
+#define GET_F64_RS2S(insn, regs) (GET_F64_REG(GET_RS2S_NUM(insn), 0, regs))
+#define SET_F32_RDS(insn, regs, val) do { \
+ SET_F32_REG(GET_RDS_NUM(insn), 0, regs, val); \
+ SET_FS_DIRTY(regs); \
+} while(0)
+#define SET_F64_RDS(insn, regs, val) do { \
+ SET_F64_REG(GET_RDS_NUM(insn), 0, regs, val); \
+ SET_FS_DIRTY(regs); \
+} while(0)
#endif
diff --git a/include/sbi/sbi_trap.h b/include/sbi/sbi_trap.h
index 3be418a3..281366e7 100644
--- a/include/sbi/sbi_trap.h
+++ b/include/sbi/sbi_trap.h
@@ -218,6 +218,7 @@ _Static_assert(
#define GET_RS2S(insn, regs) REG_VAL(GET_RS2S_NUM(insn), regs)
#define GET_RS2C(insn, regs) REG_VAL(GET_RS2C_NUM(insn), regs)
#define SET_RD(insn, regs, val) (REG_VAL(GET_RD_NUM(insn), regs) = (val))
+#define SET_RDS(insn, regs, val) (REG_VAL(GET_RDS_NUM(insn), regs) = (val))
/** Representation of trap details */
struct sbi_trap_info {
--
2.34.1
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 4/8] include: sbi: set FS dirty in vsstatus when V=1
2026-06-05 11:32 [PATCH v2 0/8] Fixes for load/store misaligned and access faults Bo Gan
` (2 preceding siblings ...)
2026-06-05 11:32 ` [PATCH v2 3/8] include: sbi: Add GET_RDS_NUM/SET(_FP32/_FP64)_RDS macros Bo Gan
@ 2026-06-05 11:32 ` Bo Gan
2026-06-05 11:32 ` [PATCH v2 5/8] lib: sbi: Do not override emulator callback for vector load/store Bo Gan
` (5 subsequent siblings)
9 siblings, 0 replies; 17+ messages in thread
From: Bo Gan @ 2026-06-05 11:32 UTC (permalink / raw)
To: opensbi, wangruikang, dramforever, andrew.jones
Cc: cleger, pjw, asrinivasan, Anup Patel
According to Privileged ISA 19.2.11:
Modifying the floating-point state when V=1 causes both fields
(vsstatus.FS and the HS-level sstatus.FS) to be set to 3 (Dirty)
Fixes: 130e65dd9d44 ("lib: sbi: Implement SET_FS_DIRTY() to make sure the mstatus FS dirty is set")
Signed-off-by: Bo Gan <ganboing@gmail.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
include/sbi/riscv_fp.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/include/sbi/riscv_fp.h b/include/sbi/riscv_fp.h
index 03011af9..ff1dbc87 100644
--- a/include/sbi/riscv_fp.h
+++ b/include/sbi/riscv_fp.h
@@ -83,7 +83,11 @@
#define GET_FFLAGS() csr_read(CSR_FFLAGS)
#define SET_FFLAGS(value) csr_write(CSR_FFLAGS, (value))
-#define SET_FS_DIRTY(regs) (regs->mstatus |= MSTATUS_FS)
+#define SET_FS_DIRTY(regs) do { \
+ if (sbi_regs_from_virt(regs)) \
+ csr_set(CSR_VSSTATUS, MSTATUS_FS); \
+ regs->mstatus |= MSTATUS_FS; \
+} while(0)
#define GET_F32_RS1(insn, regs) (GET_F32_REG(insn, 15, regs))
#define GET_F32_RS2(insn, regs) (GET_F32_REG(insn, 20, regs))
--
2.34.1
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCH v2 5/8] lib: sbi: Do not override emulator callback for vector load/store
2026-06-05 11:32 [PATCH v2 0/8] Fixes for load/store misaligned and access faults Bo Gan
` (3 preceding siblings ...)
2026-06-05 11:32 ` [PATCH v2 4/8] include: sbi: set FS dirty in vsstatus when V=1 Bo Gan
@ 2026-06-05 11:32 ` Bo Gan
2026-06-16 8:17 ` Anup Patel
2026-06-05 11:32 ` [PATCH v2 6/8] Makefile: define OPENSBI_DEBUG if DEBUG builds Bo Gan
` (4 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Bo Gan @ 2026-06-05 11:32 UTC (permalink / raw)
To: opensbi, wangruikang, dramforever, andrew.jones; +Cc: cleger, pjw, asrinivasan
It's wrong to override the emulator callback in sbi_trap_emulate_load/
store. The function must respect the callback function passed in the
parameter. Hence, let the misaligned emulator callback decide when to
use sbi_misaligned_v_ld/st_emulator. To clean up things, also make the
following changes:
- Add the `insn` parameter to the callback. The trapping insn has been
fetched by the caller already, whether transformed or directly loaded,
thus saving the trouble in the callback. Note that you must not rely
on the length of the `insn`, as it can be a transformed one from tinst
- Also the `tcntx` is added, providing the callback with register values
to handle vector insn or other customized insns.
- Clarify that the read/write length (rlen/wlen) can be 0, in which
case it could be a vector load/store or some customized instruction.
The callback is responsible to handle it accordingly.
Also fixed issues in the sbi_misaligned_v_ld/st_emulator:
a. Redirect the trap when OPENSBI_CC_SUPPORT_VECTOR is not available.
b. Ensure the return code is >0 when no faults are redirected.
Fixes: c2acc5e5b0d8 ("lib: sbi_misaligned_ldst: Add handling of vector load/store")
Signed-off-by: Bo Gan <ganboing@gmail.com>
---
include/sbi/sbi_platform.h | 92 +++++++++++++++++++++++----------
include/sbi/sbi_trap_ldst.h | 4 +-
lib/sbi/sbi_trap_ldst.c | 100 ++++++++++++++++++++++--------------
lib/sbi/sbi_trap_v_ldst.c | 25 ++++-----
4 files changed, 139 insertions(+), 82 deletions(-)
diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
index fe382b56..525cde51 100644
--- a/include/sbi/sbi_platform.h
+++ b/include/sbi/sbi_platform.h
@@ -136,12 +136,17 @@ struct sbi_platform_operations {
struct sbi_trap_regs *regs,
struct sbi_ecall_return *out);
- /** platform specific handler to fixup load fault */
- int (*emulate_load)(int rlen, unsigned long addr,
- union sbi_ldst_data *out_val);
- /** platform specific handler to fixup store fault */
- int (*emulate_store)(int wlen, unsigned long addr,
- union sbi_ldst_data in_val);
+ /** platform specific handler to fixup load fault
+ * Refer to comments below at sbi_platform_emulate_load */
+ int (*emulate_load)(ulong insn, int rlen, ulong addr,
+ union sbi_ldst_data *out_val,
+ struct sbi_trap_context *tcntx);
+
+ /** platform specific handler to fixup store fault
+ * Refer to comments below at sbi_platform_emulate_store */
+ int (*emulate_store)(ulong insn, int wlen, ulong addr,
+ union sbi_ldst_data in_val,
+ struct sbi_trap_context *tcntx);
/** platform specific pmp setup on current HART */
void (*pmp_set)(unsigned int n, unsigned long flags,
@@ -620,45 +625,76 @@ static inline int sbi_platform_vendor_ext_provider(
}
/**
- * Ask platform to emulate the trapped load
+ * Ask platform to emulate the trapped load:
*
- * @param plat pointer to struct sbi_platform
- * @param rlen length of the load: 1/2/4/8...
- * @param addr virtual address of the load. Platform needs to page-walk and
- * find the physical address if necessary
- * @param out_val value loaded
+ * @param insn the instruction that caused the load fault.
+ * It could be a transformed instruction from tinst, thus do
+ * not rely on the length of insn, and use appropriate return
+ * code, so the caller can advance mepc properly.
+ * @param rlen read length in [0, 1, 2, 4, 8]. If 0, it's a special load.
+ * In that case, it could be a vector load or customized insn,
+ * which may read/gather a block of memory. The emulator should
+ * further parse the @insn (fetch if 0), and act accordingly.
+ * @param raddr read address. If @rlen is not 0, it's the base address of
+ * the load. It doesn't necessarily match tcntx->trap->tval,
+ * in case of unaligned load triggering access fault.
+ * If @rlen is 0, @raddr should be ignored.
+ * @param out_val the buffer to hold data loaded by the emulator.
+ * If @rlen == 0, @out_val is ignored by caller.
+ * @param tcntx trap context saved on load fault entry.
*
- * @return 0 on success and negative error code on failure
+ * @return >0 success: register will be updated by caller if @rlen != 0,
+ * and mepc will be advanced by caller.
+ * 0 success: no register modification; no mepc advancement.
+ * <0 failure
+ *
+ * It's expected that if @rlen != 0, and the emulator returns >0, the
+ * caller will set the corresponding registers with @out_val to simplify
+ * things. Otherwise, no register manipulation is done by the caller.
*/
static inline int sbi_platform_emulate_load(const struct sbi_platform *plat,
- int rlen, unsigned long addr,
- union sbi_ldst_data *out_val)
+ ulong insn, int rlen, ulong raddr,
+ union sbi_ldst_data *out_val,
+ struct sbi_trap_context *tcntx)
{
if (plat && sbi_platform_ops(plat)->emulate_load) {
- return sbi_platform_ops(plat)->emulate_load(rlen, addr,
- out_val);
+ return sbi_platform_ops(plat)->emulate_load(insn, rlen, raddr,
+ out_val, tcntx);
}
return SBI_ENOTSUPP;
}
/**
- * Ask platform to emulate the trapped store
+ * Ask platform to emulate the trapped store:
*
- * @param plat pointer to struct sbi_platform
- * @param wlen length of the store: 1/2/4/8...
- * @param addr virtual address of the store. Platform needs to page-walk and
- * find the physical address if necessary
- * @param in_val value to store
+ * @param insn the instruction that caused the store fault.
+ * It could be a transformed instruction from tinst, thus do
+ * not rely on the length of insn, and use appropriate return
+ * code, so the caller can advance mepc properly.
+ * @param wlen write length in [0, 1, 2, 4, 8]. If 0, it's a special store.
+ * In that case, it could be a vector store or customized insn,
+ * which may write/scatter a block of memory. The emulator should
+ * further parse the @insn (fetch if 0), and act accordingly.
+ * @param waddr write address. If @wlen is not 0, it's the base address of
+ * the store. It doesn't necessarily match tcntx->trap->tval,
+ * in case of unaligned store triggering access fault.
+ * If @wlen is 0, @waddr should be ignored.
+ * @param in_val the buffer to hold data about to be stored by the emulator.
+ * If @wlen == 0, @in_val should be ignored.
+ * @param tcntx trap context saved on store fault entry.
*
- * @return 0 on success and negative error code on failure
+ * @return >0 success: mepc will be advanced by caller.
+ * 0 success: no mepc advancement.
+ * <0 failure
*/
static inline int sbi_platform_emulate_store(const struct sbi_platform *plat,
- int wlen, unsigned long addr,
- union sbi_ldst_data in_val)
+ ulong insn, int wlen, ulong waddr,
+ union sbi_ldst_data in_val,
+ struct sbi_trap_context *tcntx)
{
if (plat && sbi_platform_ops(plat)->emulate_store) {
- return sbi_platform_ops(plat)->emulate_store(wlen, addr,
- in_val);
+ return sbi_platform_ops(plat)->emulate_store(insn, wlen, waddr,
+ in_val, tcntx);
}
return SBI_ENOTSUPP;
}
diff --git a/include/sbi/sbi_trap_ldst.h b/include/sbi/sbi_trap_ldst.h
index a6a6c75b..33c348c5 100644
--- a/include/sbi/sbi_trap_ldst.h
+++ b/include/sbi/sbi_trap_ldst.h
@@ -31,10 +31,10 @@ int sbi_store_access_handler(struct sbi_trap_context *tcntx);
ulong sbi_misaligned_tinst_fixup(ulong orig_tinst, ulong new_tinst,
ulong addr_offset);
-int sbi_misaligned_v_ld_emulator(int rlen, union sbi_ldst_data *out_val,
+int sbi_misaligned_v_ld_emulator(ulong insn,
struct sbi_trap_context *tcntx);
-int sbi_misaligned_v_st_emulator(int wlen, union sbi_ldst_data in_val,
+int sbi_misaligned_v_st_emulator(ulong insn,
struct sbi_trap_context *tcntx);
#endif
diff --git a/lib/sbi/sbi_trap_ldst.c b/lib/sbi/sbi_trap_ldst.c
index 448406b1..ad19926a 100644
--- a/lib/sbi/sbi_trap_ldst.c
+++ b/lib/sbi/sbi_trap_ldst.c
@@ -18,18 +18,18 @@
/**
* Load emulator callback:
- *
- * @return rlen=success, 0=success w/o regs modification, or negative error
+ * Refer to comments of `sbi_platform_emulate_load`.
*/
-typedef int (*sbi_trap_ld_emulator)(int rlen, union sbi_ldst_data *out_val,
+typedef int (*sbi_trap_ld_emulator)(ulong insn, int rlen, ulong raddr,
+ union sbi_ldst_data *out_val,
struct sbi_trap_context *tcntx);
/**
* Store emulator callback:
- *
- * @return wlen=success, 0=success w/o regs modification, or negative error
+ * Refer to comments of `sbi_platform_emulate_store`.
*/
-typedef int (*sbi_trap_st_emulator)(int wlen, union sbi_ldst_data in_val,
+typedef int (*sbi_trap_st_emulator)(ulong insn, int wlen, ulong waddr,
+ union sbi_ldst_data in_val,
struct sbi_trap_context *tcntx);
ulong sbi_misaligned_tinst_fixup(ulong orig_tinst, ulong new_tinst,
@@ -52,7 +52,7 @@ static int sbi_trap_emulate_load(struct sbi_trap_context *tcntx,
ulong insn, insn_len;
union sbi_ldst_data val = { 0 };
struct sbi_trap_info uptrap;
- int rc, fp = 0, shift = 0, len = 0, vector = 0;
+ int rc, fp = 0, shift = 0, len = 0;
if (orig_trap->tinst & 0x1) {
/*
@@ -144,28 +144,24 @@ static int sbi_trap_emulate_load(struct sbi_trap_context *tcntx,
len = 2;
shift = 8 * (sizeof(ulong) - len);
insn = RVC_RS2S(insn) << SH_RD;
- } else if (IS_VECTOR_LOAD_STORE(insn)) {
- vector = 1;
- emu = sbi_misaligned_v_ld_emulator;
- } else {
- return sbi_trap_redirect(regs, orig_trap);
}
- rc = emu(len, &val, tcntx);
+ rc = emu(insn, len, orig_trap->tval, &val, tcntx);
if (rc <= 0)
return rc;
+ if (!len)
+ goto epc_fixup;
- if (!vector) {
- if (!fp)
- SET_RD(insn, regs, ((long)(val.data_ulong << shift)) >> shift);
+ if (!fp)
+ SET_RD(insn, regs, ((long)(val.data_ulong << shift)) >> shift);
#ifdef __riscv_flen
- else if (len == 8)
- SET_F64_RD(insn, regs, val.data_u64);
- else
- SET_F32_RD(insn, regs, val.data_ulong);
+ else if (len == 8)
+ SET_F64_RD(insn, regs, val.data_u64);
+ else
+ SET_F32_RD(insn, regs, val.data_ulong);
#endif
- }
+epc_fixup:
regs->mepc += insn_len;
return 0;
@@ -253,13 +249,9 @@ static int sbi_trap_emulate_store(struct sbi_trap_context *tcntx,
} else if ((insn & INSN_MASK_C_SH) == INSN_MATCH_C_SH) {
len = 2;
val.data_ulong = GET_RS2S(insn, regs);
- } else if (IS_VECTOR_LOAD_STORE(insn)) {
- emu = sbi_misaligned_v_st_emulator;
- } else {
- return sbi_trap_redirect(regs, orig_trap);
}
- rc = emu(len, val, tcntx);
+ rc = emu(insn, len, orig_trap->tval, val, tcntx);
if (rc <= 0)
return rc;
@@ -268,7 +260,8 @@ static int sbi_trap_emulate_store(struct sbi_trap_context *tcntx,
return 0;
}
-static int sbi_misaligned_ld_emulator(int rlen, union sbi_ldst_data *out_val,
+static int sbi_misaligned_ld_emulator(ulong insn, int rlen, ulong addr,
+ union sbi_ldst_data *out_val,
struct sbi_trap_context *tcntx)
{
const struct sbi_trap_info *orig_trap = &tcntx->trap;
@@ -276,9 +269,20 @@ static int sbi_misaligned_ld_emulator(int rlen, union sbi_ldst_data *out_val,
struct sbi_trap_info uptrap;
int i;
+ if (!rlen) {
+ if (IS_VECTOR_LOAD_STORE(insn))
+ return sbi_misaligned_v_ld_emulator(insn, tcntx);
+ else
+ /* Unrecognized instruction. Can't emulate it. */
+ return sbi_trap_redirect(regs, orig_trap);
+ }
+ /* For misaligned fault, addr must be the same as orig_trap->tval */
+ if (addr != orig_trap->tval)
+ return SBI_EFAIL;
+
for (i = 0; i < rlen; i++) {
out_val->data_bytes[i] =
- sbi_load_u8((void *)(orig_trap->tval + i), &uptrap);
+ sbi_load_u8((void *)(addr + i), &uptrap);
if (uptrap.cause) {
uptrap.tinst = sbi_misaligned_tinst_fixup(
orig_trap->tinst, uptrap.tinst, i);
@@ -293,7 +297,8 @@ int sbi_misaligned_load_handler(struct sbi_trap_context *tcntx)
return sbi_trap_emulate_load(tcntx, sbi_misaligned_ld_emulator);
}
-static int sbi_misaligned_st_emulator(int wlen, union sbi_ldst_data in_val,
+static int sbi_misaligned_st_emulator(ulong insn, int wlen, ulong addr,
+ union sbi_ldst_data in_val,
struct sbi_trap_context *tcntx)
{
const struct sbi_trap_info *orig_trap = &tcntx->trap;
@@ -301,8 +306,19 @@ static int sbi_misaligned_st_emulator(int wlen, union sbi_ldst_data in_val,
struct sbi_trap_info uptrap;
int i;
+ if (!wlen) {
+ if (IS_VECTOR_LOAD_STORE(insn))
+ return sbi_misaligned_v_st_emulator(insn, tcntx);
+ else
+ /* Unrecognized instruction. Can't emulate it. */
+ return sbi_trap_redirect(regs, orig_trap);
+ }
+ /* For misaligned fault, addr must be the same as orig_trap->tval */
+ if (addr != orig_trap->tval)
+ return SBI_EFAIL;
+
for (i = 0; i < wlen; i++) {
- sbi_store_u8((void *)(orig_trap->tval + i),
+ sbi_store_u8((void *)(addr + i),
in_val.data_bytes[i], &uptrap);
if (uptrap.cause) {
uptrap.tinst = sbi_misaligned_tinst_fixup(
@@ -318,22 +334,26 @@ int sbi_misaligned_store_handler(struct sbi_trap_context *tcntx)
return sbi_trap_emulate_store(tcntx, sbi_misaligned_st_emulator);
}
-static int sbi_ld_access_emulator(int rlen, union sbi_ldst_data *out_val,
+static int sbi_ld_access_emulator(ulong insn, int rlen, ulong addr,
+ union sbi_ldst_data *out_val,
struct sbi_trap_context *tcntx)
{
const struct sbi_trap_info *orig_trap = &tcntx->trap;
struct sbi_trap_regs *regs = &tcntx->regs;
+ int rc;
/* If fault came from M mode, just fail */
if (sbi_mstatus_prev_mode(regs->mstatus) == PRV_M)
return SBI_EINVAL;
+ rc = sbi_platform_emulate_load(sbi_platform_thishart_ptr(),
+ insn, rlen, addr, out_val, tcntx);
+
/* If platform emulator failed, we redirect instead of fail */
- if (sbi_platform_emulate_load(sbi_platform_thishart_ptr(), rlen,
- orig_trap->tval, out_val))
+ if (rc < 0)
return sbi_trap_redirect(regs, orig_trap);
- return rlen;
+ return rc;
}
int sbi_load_access_handler(struct sbi_trap_context *tcntx)
@@ -341,22 +361,26 @@ int sbi_load_access_handler(struct sbi_trap_context *tcntx)
return sbi_trap_emulate_load(tcntx, sbi_ld_access_emulator);
}
-static int sbi_st_access_emulator(int wlen, union sbi_ldst_data in_val,
+static int sbi_st_access_emulator(ulong insn, int wlen, ulong addr,
+ union sbi_ldst_data in_val,
struct sbi_trap_context *tcntx)
{
const struct sbi_trap_info *orig_trap = &tcntx->trap;
struct sbi_trap_regs *regs = &tcntx->regs;
+ int rc;
/* If fault came from M mode, just fail */
if (sbi_mstatus_prev_mode(regs->mstatus) == PRV_M)
return SBI_EINVAL;
+ rc = sbi_platform_emulate_store(sbi_platform_thishart_ptr(),
+ insn, wlen, addr, in_val, tcntx);
+
/* If platform emulator failed, we redirect instead of fail */
- if (sbi_platform_emulate_store(sbi_platform_thishart_ptr(), wlen,
- orig_trap->tval, in_val))
+ if (rc < 0)
return sbi_trap_redirect(regs, orig_trap);
- return wlen;
+ return rc;
}
int sbi_store_access_handler(struct sbi_trap_context *tcntx)
diff --git a/lib/sbi/sbi_trap_v_ldst.c b/lib/sbi/sbi_trap_v_ldst.c
index f4d469dc..57f12b83 100644
--- a/lib/sbi/sbi_trap_v_ldst.c
+++ b/lib/sbi/sbi_trap_v_ldst.c
@@ -137,13 +137,11 @@ static inline void vsetvl(ulong vl, ulong vtype)
:: "r" (vl), "r" (vtype));
}
-int sbi_misaligned_v_ld_emulator(int rlen, union sbi_ldst_data *out_val,
- struct sbi_trap_context *tcntx)
+int sbi_misaligned_v_ld_emulator(ulong insn, struct sbi_trap_context *tcntx)
{
const struct sbi_trap_info *orig_trap = &tcntx->trap;
struct sbi_trap_regs *regs = &tcntx->regs;
struct sbi_trap_info uptrap;
- ulong insn = sbi_get_insn(regs->mepc, &uptrap);
ulong vl = csr_read(CSR_VL);
ulong vtype = csr_read(CSR_VTYPE);
ulong vlenb = csr_read(CSR_VLENB);
@@ -234,16 +232,15 @@ int sbi_misaligned_v_ld_emulator(int rlen, union sbi_ldst_data *out_val,
/* restore clobbered vl/vtype */
vsetvl(vl, vtype);
- return vl;
+ /* Return a >0 value for the caller to advance mepc */
+ return 1;
}
-int sbi_misaligned_v_st_emulator(int wlen, union sbi_ldst_data in_val,
- struct sbi_trap_context *tcntx)
+int sbi_misaligned_v_st_emulator(ulong insn, struct sbi_trap_context *tcntx)
{
const struct sbi_trap_info *orig_trap = &tcntx->trap;
struct sbi_trap_regs *regs = &tcntx->regs;
struct sbi_trap_info uptrap;
- ulong insn = sbi_get_insn(regs->mepc, &uptrap);
ulong vl = csr_read(CSR_VL);
ulong vtype = csr_read(CSR_VTYPE);
ulong vlenb = csr_read(CSR_VLENB);
@@ -328,17 +325,17 @@ int sbi_misaligned_v_st_emulator(int wlen, union sbi_ldst_data in_val,
/* restore clobbered vl/vtype */
vsetvl(vl, vtype);
- return vl;
+ /* Return a >0 value for the caller to advance mepc */
+ return 1;
}
#else
-int sbi_misaligned_v_ld_emulator(int rlen, union sbi_ldst_data *out_val,
- struct sbi_trap_context *tcntx)
+int sbi_misaligned_v_ld_emulator(ulong insn, struct sbi_trap_context *tcntx)
{
- return 0;
+ return sbi_trap_redirect(&tcntx->regs, &tcntx->trap);
}
-int sbi_misaligned_v_st_emulator(int wlen, union sbi_ldst_data in_val,
- struct sbi_trap_context *tcntx)
+
+int sbi_misaligned_v_st_emulator(ulong insn, struct sbi_trap_context *tcntx)
{
- return 0;
+ return sbi_trap_redirect(&tcntx->regs, &tcntx->trap);
}
#endif /* OPENSBI_CC_SUPPORT_VECTOR */
--
2.34.1
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 5/8] lib: sbi: Do not override emulator callback for vector load/store
2026-06-05 11:32 ` [PATCH v2 5/8] lib: sbi: Do not override emulator callback for vector load/store Bo Gan
@ 2026-06-16 8:17 ` Anup Patel
0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2026-06-16 8:17 UTC (permalink / raw)
To: Bo Gan
Cc: opensbi, wangruikang, dramforever, andrew.jones, cleger, pjw,
asrinivasan
On Fri, Jun 5, 2026 at 5:04 PM Bo Gan <ganboing@gmail.com> wrote:
>
> It's wrong to override the emulator callback in sbi_trap_emulate_load/
> store. The function must respect the callback function passed in the
> parameter. Hence, let the misaligned emulator callback decide when to
> use sbi_misaligned_v_ld/st_emulator. To clean up things, also make the
> following changes:
>
> - Add the `insn` parameter to the callback. The trapping insn has been
> fetched by the caller already, whether transformed or directly loaded,
> thus saving the trouble in the callback. Note that you must not rely
> on the length of the `insn`, as it can be a transformed one from tinst
>
> - Also the `tcntx` is added, providing the callback with register values
> to handle vector insn or other customized insns.
>
> - Clarify that the read/write length (rlen/wlen) can be 0, in which
> case it could be a vector load/store or some customized instruction.
> The callback is responsible to handle it accordingly.
>
> Also fixed issues in the sbi_misaligned_v_ld/st_emulator:
> a. Redirect the trap when OPENSBI_CC_SUPPORT_VECTOR is not available.
> b. Ensure the return code is >0 when no faults are redirected.
>
> Fixes: c2acc5e5b0d8 ("lib: sbi_misaligned_ldst: Add handling of vector load/store")
> Signed-off-by: Bo Gan <ganboing@gmail.com>
LGTM.
Reviewed-by: Anup Patel <anup@brainfault.org>
Thanks,
Anup
> ---
> include/sbi/sbi_platform.h | 92 +++++++++++++++++++++++----------
> include/sbi/sbi_trap_ldst.h | 4 +-
> lib/sbi/sbi_trap_ldst.c | 100 ++++++++++++++++++++++--------------
> lib/sbi/sbi_trap_v_ldst.c | 25 ++++-----
> 4 files changed, 139 insertions(+), 82 deletions(-)
>
> diff --git a/include/sbi/sbi_platform.h b/include/sbi/sbi_platform.h
> index fe382b56..525cde51 100644
> --- a/include/sbi/sbi_platform.h
> +++ b/include/sbi/sbi_platform.h
> @@ -136,12 +136,17 @@ struct sbi_platform_operations {
> struct sbi_trap_regs *regs,
> struct sbi_ecall_return *out);
>
> - /** platform specific handler to fixup load fault */
> - int (*emulate_load)(int rlen, unsigned long addr,
> - union sbi_ldst_data *out_val);
> - /** platform specific handler to fixup store fault */
> - int (*emulate_store)(int wlen, unsigned long addr,
> - union sbi_ldst_data in_val);
> + /** platform specific handler to fixup load fault
> + * Refer to comments below at sbi_platform_emulate_load */
> + int (*emulate_load)(ulong insn, int rlen, ulong addr,
> + union sbi_ldst_data *out_val,
> + struct sbi_trap_context *tcntx);
> +
> + /** platform specific handler to fixup store fault
> + * Refer to comments below at sbi_platform_emulate_store */
> + int (*emulate_store)(ulong insn, int wlen, ulong addr,
> + union sbi_ldst_data in_val,
> + struct sbi_trap_context *tcntx);
>
> /** platform specific pmp setup on current HART */
> void (*pmp_set)(unsigned int n, unsigned long flags,
> @@ -620,45 +625,76 @@ static inline int sbi_platform_vendor_ext_provider(
> }
>
> /**
> - * Ask platform to emulate the trapped load
> + * Ask platform to emulate the trapped load:
> *
> - * @param plat pointer to struct sbi_platform
> - * @param rlen length of the load: 1/2/4/8...
> - * @param addr virtual address of the load. Platform needs to page-walk and
> - * find the physical address if necessary
> - * @param out_val value loaded
> + * @param insn the instruction that caused the load fault.
> + * It could be a transformed instruction from tinst, thus do
> + * not rely on the length of insn, and use appropriate return
> + * code, so the caller can advance mepc properly.
> + * @param rlen read length in [0, 1, 2, 4, 8]. If 0, it's a special load.
> + * In that case, it could be a vector load or customized insn,
> + * which may read/gather a block of memory. The emulator should
> + * further parse the @insn (fetch if 0), and act accordingly.
> + * @param raddr read address. If @rlen is not 0, it's the base address of
> + * the load. It doesn't necessarily match tcntx->trap->tval,
> + * in case of unaligned load triggering access fault.
> + * If @rlen is 0, @raddr should be ignored.
> + * @param out_val the buffer to hold data loaded by the emulator.
> + * If @rlen == 0, @out_val is ignored by caller.
> + * @param tcntx trap context saved on load fault entry.
> *
> - * @return 0 on success and negative error code on failure
> + * @return >0 success: register will be updated by caller if @rlen != 0,
> + * and mepc will be advanced by caller.
> + * 0 success: no register modification; no mepc advancement.
> + * <0 failure
> + *
> + * It's expected that if @rlen != 0, and the emulator returns >0, the
> + * caller will set the corresponding registers with @out_val to simplify
> + * things. Otherwise, no register manipulation is done by the caller.
> */
> static inline int sbi_platform_emulate_load(const struct sbi_platform *plat,
> - int rlen, unsigned long addr,
> - union sbi_ldst_data *out_val)
> + ulong insn, int rlen, ulong raddr,
> + union sbi_ldst_data *out_val,
> + struct sbi_trap_context *tcntx)
> {
> if (plat && sbi_platform_ops(plat)->emulate_load) {
> - return sbi_platform_ops(plat)->emulate_load(rlen, addr,
> - out_val);
> + return sbi_platform_ops(plat)->emulate_load(insn, rlen, raddr,
> + out_val, tcntx);
> }
> return SBI_ENOTSUPP;
> }
>
> /**
> - * Ask platform to emulate the trapped store
> + * Ask platform to emulate the trapped store:
> *
> - * @param plat pointer to struct sbi_platform
> - * @param wlen length of the store: 1/2/4/8...
> - * @param addr virtual address of the store. Platform needs to page-walk and
> - * find the physical address if necessary
> - * @param in_val value to store
> + * @param insn the instruction that caused the store fault.
> + * It could be a transformed instruction from tinst, thus do
> + * not rely on the length of insn, and use appropriate return
> + * code, so the caller can advance mepc properly.
> + * @param wlen write length in [0, 1, 2, 4, 8]. If 0, it's a special store.
> + * In that case, it could be a vector store or customized insn,
> + * which may write/scatter a block of memory. The emulator should
> + * further parse the @insn (fetch if 0), and act accordingly.
> + * @param waddr write address. If @wlen is not 0, it's the base address of
> + * the store. It doesn't necessarily match tcntx->trap->tval,
> + * in case of unaligned store triggering access fault.
> + * If @wlen is 0, @waddr should be ignored.
> + * @param in_val the buffer to hold data about to be stored by the emulator.
> + * If @wlen == 0, @in_val should be ignored.
> + * @param tcntx trap context saved on store fault entry.
> *
> - * @return 0 on success and negative error code on failure
> + * @return >0 success: mepc will be advanced by caller.
> + * 0 success: no mepc advancement.
> + * <0 failure
> */
> static inline int sbi_platform_emulate_store(const struct sbi_platform *plat,
> - int wlen, unsigned long addr,
> - union sbi_ldst_data in_val)
> + ulong insn, int wlen, ulong waddr,
> + union sbi_ldst_data in_val,
> + struct sbi_trap_context *tcntx)
> {
> if (plat && sbi_platform_ops(plat)->emulate_store) {
> - return sbi_platform_ops(plat)->emulate_store(wlen, addr,
> - in_val);
> + return sbi_platform_ops(plat)->emulate_store(insn, wlen, waddr,
> + in_val, tcntx);
> }
> return SBI_ENOTSUPP;
> }
> diff --git a/include/sbi/sbi_trap_ldst.h b/include/sbi/sbi_trap_ldst.h
> index a6a6c75b..33c348c5 100644
> --- a/include/sbi/sbi_trap_ldst.h
> +++ b/include/sbi/sbi_trap_ldst.h
> @@ -31,10 +31,10 @@ int sbi_store_access_handler(struct sbi_trap_context *tcntx);
> ulong sbi_misaligned_tinst_fixup(ulong orig_tinst, ulong new_tinst,
> ulong addr_offset);
>
> -int sbi_misaligned_v_ld_emulator(int rlen, union sbi_ldst_data *out_val,
> +int sbi_misaligned_v_ld_emulator(ulong insn,
> struct sbi_trap_context *tcntx);
>
> -int sbi_misaligned_v_st_emulator(int wlen, union sbi_ldst_data in_val,
> +int sbi_misaligned_v_st_emulator(ulong insn,
> struct sbi_trap_context *tcntx);
>
> #endif
> diff --git a/lib/sbi/sbi_trap_ldst.c b/lib/sbi/sbi_trap_ldst.c
> index 448406b1..ad19926a 100644
> --- a/lib/sbi/sbi_trap_ldst.c
> +++ b/lib/sbi/sbi_trap_ldst.c
> @@ -18,18 +18,18 @@
>
> /**
> * Load emulator callback:
> - *
> - * @return rlen=success, 0=success w/o regs modification, or negative error
> + * Refer to comments of `sbi_platform_emulate_load`.
> */
> -typedef int (*sbi_trap_ld_emulator)(int rlen, union sbi_ldst_data *out_val,
> +typedef int (*sbi_trap_ld_emulator)(ulong insn, int rlen, ulong raddr,
> + union sbi_ldst_data *out_val,
> struct sbi_trap_context *tcntx);
>
> /**
> * Store emulator callback:
> - *
> - * @return wlen=success, 0=success w/o regs modification, or negative error
> + * Refer to comments of `sbi_platform_emulate_store`.
> */
> -typedef int (*sbi_trap_st_emulator)(int wlen, union sbi_ldst_data in_val,
> +typedef int (*sbi_trap_st_emulator)(ulong insn, int wlen, ulong waddr,
> + union sbi_ldst_data in_val,
> struct sbi_trap_context *tcntx);
>
> ulong sbi_misaligned_tinst_fixup(ulong orig_tinst, ulong new_tinst,
> @@ -52,7 +52,7 @@ static int sbi_trap_emulate_load(struct sbi_trap_context *tcntx,
> ulong insn, insn_len;
> union sbi_ldst_data val = { 0 };
> struct sbi_trap_info uptrap;
> - int rc, fp = 0, shift = 0, len = 0, vector = 0;
> + int rc, fp = 0, shift = 0, len = 0;
>
> if (orig_trap->tinst & 0x1) {
> /*
> @@ -144,28 +144,24 @@ static int sbi_trap_emulate_load(struct sbi_trap_context *tcntx,
> len = 2;
> shift = 8 * (sizeof(ulong) - len);
> insn = RVC_RS2S(insn) << SH_RD;
> - } else if (IS_VECTOR_LOAD_STORE(insn)) {
> - vector = 1;
> - emu = sbi_misaligned_v_ld_emulator;
> - } else {
> - return sbi_trap_redirect(regs, orig_trap);
> }
>
> - rc = emu(len, &val, tcntx);
> + rc = emu(insn, len, orig_trap->tval, &val, tcntx);
> if (rc <= 0)
> return rc;
> + if (!len)
> + goto epc_fixup;
>
> - if (!vector) {
> - if (!fp)
> - SET_RD(insn, regs, ((long)(val.data_ulong << shift)) >> shift);
> + if (!fp)
> + SET_RD(insn, regs, ((long)(val.data_ulong << shift)) >> shift);
> #ifdef __riscv_flen
> - else if (len == 8)
> - SET_F64_RD(insn, regs, val.data_u64);
> - else
> - SET_F32_RD(insn, regs, val.data_ulong);
> + else if (len == 8)
> + SET_F64_RD(insn, regs, val.data_u64);
> + else
> + SET_F32_RD(insn, regs, val.data_ulong);
> #endif
> - }
>
> +epc_fixup:
> regs->mepc += insn_len;
>
> return 0;
> @@ -253,13 +249,9 @@ static int sbi_trap_emulate_store(struct sbi_trap_context *tcntx,
> } else if ((insn & INSN_MASK_C_SH) == INSN_MATCH_C_SH) {
> len = 2;
> val.data_ulong = GET_RS2S(insn, regs);
> - } else if (IS_VECTOR_LOAD_STORE(insn)) {
> - emu = sbi_misaligned_v_st_emulator;
> - } else {
> - return sbi_trap_redirect(regs, orig_trap);
> }
>
> - rc = emu(len, val, tcntx);
> + rc = emu(insn, len, orig_trap->tval, val, tcntx);
> if (rc <= 0)
> return rc;
>
> @@ -268,7 +260,8 @@ static int sbi_trap_emulate_store(struct sbi_trap_context *tcntx,
> return 0;
> }
>
> -static int sbi_misaligned_ld_emulator(int rlen, union sbi_ldst_data *out_val,
> +static int sbi_misaligned_ld_emulator(ulong insn, int rlen, ulong addr,
> + union sbi_ldst_data *out_val,
> struct sbi_trap_context *tcntx)
> {
> const struct sbi_trap_info *orig_trap = &tcntx->trap;
> @@ -276,9 +269,20 @@ static int sbi_misaligned_ld_emulator(int rlen, union sbi_ldst_data *out_val,
> struct sbi_trap_info uptrap;
> int i;
>
> + if (!rlen) {
> + if (IS_VECTOR_LOAD_STORE(insn))
> + return sbi_misaligned_v_ld_emulator(insn, tcntx);
> + else
> + /* Unrecognized instruction. Can't emulate it. */
> + return sbi_trap_redirect(regs, orig_trap);
> + }
> + /* For misaligned fault, addr must be the same as orig_trap->tval */
> + if (addr != orig_trap->tval)
> + return SBI_EFAIL;
> +
> for (i = 0; i < rlen; i++) {
> out_val->data_bytes[i] =
> - sbi_load_u8((void *)(orig_trap->tval + i), &uptrap);
> + sbi_load_u8((void *)(addr + i), &uptrap);
> if (uptrap.cause) {
> uptrap.tinst = sbi_misaligned_tinst_fixup(
> orig_trap->tinst, uptrap.tinst, i);
> @@ -293,7 +297,8 @@ int sbi_misaligned_load_handler(struct sbi_trap_context *tcntx)
> return sbi_trap_emulate_load(tcntx, sbi_misaligned_ld_emulator);
> }
>
> -static int sbi_misaligned_st_emulator(int wlen, union sbi_ldst_data in_val,
> +static int sbi_misaligned_st_emulator(ulong insn, int wlen, ulong addr,
> + union sbi_ldst_data in_val,
> struct sbi_trap_context *tcntx)
> {
> const struct sbi_trap_info *orig_trap = &tcntx->trap;
> @@ -301,8 +306,19 @@ static int sbi_misaligned_st_emulator(int wlen, union sbi_ldst_data in_val,
> struct sbi_trap_info uptrap;
> int i;
>
> + if (!wlen) {
> + if (IS_VECTOR_LOAD_STORE(insn))
> + return sbi_misaligned_v_st_emulator(insn, tcntx);
> + else
> + /* Unrecognized instruction. Can't emulate it. */
> + return sbi_trap_redirect(regs, orig_trap);
> + }
> + /* For misaligned fault, addr must be the same as orig_trap->tval */
> + if (addr != orig_trap->tval)
> + return SBI_EFAIL;
> +
> for (i = 0; i < wlen; i++) {
> - sbi_store_u8((void *)(orig_trap->tval + i),
> + sbi_store_u8((void *)(addr + i),
> in_val.data_bytes[i], &uptrap);
> if (uptrap.cause) {
> uptrap.tinst = sbi_misaligned_tinst_fixup(
> @@ -318,22 +334,26 @@ int sbi_misaligned_store_handler(struct sbi_trap_context *tcntx)
> return sbi_trap_emulate_store(tcntx, sbi_misaligned_st_emulator);
> }
>
> -static int sbi_ld_access_emulator(int rlen, union sbi_ldst_data *out_val,
> +static int sbi_ld_access_emulator(ulong insn, int rlen, ulong addr,
> + union sbi_ldst_data *out_val,
> struct sbi_trap_context *tcntx)
> {
> const struct sbi_trap_info *orig_trap = &tcntx->trap;
> struct sbi_trap_regs *regs = &tcntx->regs;
> + int rc;
>
> /* If fault came from M mode, just fail */
> if (sbi_mstatus_prev_mode(regs->mstatus) == PRV_M)
> return SBI_EINVAL;
>
> + rc = sbi_platform_emulate_load(sbi_platform_thishart_ptr(),
> + insn, rlen, addr, out_val, tcntx);
> +
> /* If platform emulator failed, we redirect instead of fail */
> - if (sbi_platform_emulate_load(sbi_platform_thishart_ptr(), rlen,
> - orig_trap->tval, out_val))
> + if (rc < 0)
> return sbi_trap_redirect(regs, orig_trap);
>
> - return rlen;
> + return rc;
> }
>
> int sbi_load_access_handler(struct sbi_trap_context *tcntx)
> @@ -341,22 +361,26 @@ int sbi_load_access_handler(struct sbi_trap_context *tcntx)
> return sbi_trap_emulate_load(tcntx, sbi_ld_access_emulator);
> }
>
> -static int sbi_st_access_emulator(int wlen, union sbi_ldst_data in_val,
> +static int sbi_st_access_emulator(ulong insn, int wlen, ulong addr,
> + union sbi_ldst_data in_val,
> struct sbi_trap_context *tcntx)
> {
> const struct sbi_trap_info *orig_trap = &tcntx->trap;
> struct sbi_trap_regs *regs = &tcntx->regs;
> + int rc;
>
> /* If fault came from M mode, just fail */
> if (sbi_mstatus_prev_mode(regs->mstatus) == PRV_M)
> return SBI_EINVAL;
>
> + rc = sbi_platform_emulate_store(sbi_platform_thishart_ptr(),
> + insn, wlen, addr, in_val, tcntx);
> +
> /* If platform emulator failed, we redirect instead of fail */
> - if (sbi_platform_emulate_store(sbi_platform_thishart_ptr(), wlen,
> - orig_trap->tval, in_val))
> + if (rc < 0)
> return sbi_trap_redirect(regs, orig_trap);
>
> - return wlen;
> + return rc;
> }
>
> int sbi_store_access_handler(struct sbi_trap_context *tcntx)
> diff --git a/lib/sbi/sbi_trap_v_ldst.c b/lib/sbi/sbi_trap_v_ldst.c
> index f4d469dc..57f12b83 100644
> --- a/lib/sbi/sbi_trap_v_ldst.c
> +++ b/lib/sbi/sbi_trap_v_ldst.c
> @@ -137,13 +137,11 @@ static inline void vsetvl(ulong vl, ulong vtype)
> :: "r" (vl), "r" (vtype));
> }
>
> -int sbi_misaligned_v_ld_emulator(int rlen, union sbi_ldst_data *out_val,
> - struct sbi_trap_context *tcntx)
> +int sbi_misaligned_v_ld_emulator(ulong insn, struct sbi_trap_context *tcntx)
> {
> const struct sbi_trap_info *orig_trap = &tcntx->trap;
> struct sbi_trap_regs *regs = &tcntx->regs;
> struct sbi_trap_info uptrap;
> - ulong insn = sbi_get_insn(regs->mepc, &uptrap);
> ulong vl = csr_read(CSR_VL);
> ulong vtype = csr_read(CSR_VTYPE);
> ulong vlenb = csr_read(CSR_VLENB);
> @@ -234,16 +232,15 @@ int sbi_misaligned_v_ld_emulator(int rlen, union sbi_ldst_data *out_val,
> /* restore clobbered vl/vtype */
> vsetvl(vl, vtype);
>
> - return vl;
> + /* Return a >0 value for the caller to advance mepc */
> + return 1;
> }
>
> -int sbi_misaligned_v_st_emulator(int wlen, union sbi_ldst_data in_val,
> - struct sbi_trap_context *tcntx)
> +int sbi_misaligned_v_st_emulator(ulong insn, struct sbi_trap_context *tcntx)
> {
> const struct sbi_trap_info *orig_trap = &tcntx->trap;
> struct sbi_trap_regs *regs = &tcntx->regs;
> struct sbi_trap_info uptrap;
> - ulong insn = sbi_get_insn(regs->mepc, &uptrap);
> ulong vl = csr_read(CSR_VL);
> ulong vtype = csr_read(CSR_VTYPE);
> ulong vlenb = csr_read(CSR_VLENB);
> @@ -328,17 +325,17 @@ int sbi_misaligned_v_st_emulator(int wlen, union sbi_ldst_data in_val,
> /* restore clobbered vl/vtype */
> vsetvl(vl, vtype);
>
> - return vl;
> + /* Return a >0 value for the caller to advance mepc */
> + return 1;
> }
> #else
> -int sbi_misaligned_v_ld_emulator(int rlen, union sbi_ldst_data *out_val,
> - struct sbi_trap_context *tcntx)
> +int sbi_misaligned_v_ld_emulator(ulong insn, struct sbi_trap_context *tcntx)
> {
> - return 0;
> + return sbi_trap_redirect(&tcntx->regs, &tcntx->trap);
> }
> -int sbi_misaligned_v_st_emulator(int wlen, union sbi_ldst_data in_val,
> - struct sbi_trap_context *tcntx)
> +
> +int sbi_misaligned_v_st_emulator(ulong insn, struct sbi_trap_context *tcntx)
> {
> - return 0;
> + return sbi_trap_redirect(&tcntx->regs, &tcntx->trap);
> }
> #endif /* OPENSBI_CC_SUPPORT_VECTOR */
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 6/8] Makefile: define OPENSBI_DEBUG if DEBUG builds
2026-06-05 11:32 [PATCH v2 0/8] Fixes for load/store misaligned and access faults Bo Gan
` (4 preceding siblings ...)
2026-06-05 11:32 ` [PATCH v2 5/8] lib: sbi: Do not override emulator callback for vector load/store Bo Gan
@ 2026-06-05 11:32 ` Bo Gan
2026-06-16 8:19 ` Anup Patel
2026-06-05 11:32 ` [PATCH v2 7/8] lib: sbi: Rework load/store emulator instruction decoding Bo Gan
` (3 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Bo Gan @ 2026-06-05 11:32 UTC (permalink / raw)
To: opensbi, wangruikang, dramforever, andrew.jones; +Cc: cleger, pjw, asrinivasan
Signed-off-by: Bo Gan <ganboing@gmail.com>
---
Makefile | 1 +
1 file changed, 1 insertion(+)
diff --git a/Makefile b/Makefile
index 46541063..6ce3c11e 100644
--- a/Makefile
+++ b/Makefile
@@ -451,6 +451,7 @@ DTSCPPFLAGS = $(CPPFLAGS) -nostdinc -nostdlib -fno-builtin -D__DTS__ -x assemble
ifneq ($(DEBUG),)
CFLAGS += -O0
+CPPFLAGS += -DOPENSBI_DEBUG
else
CFLAGS += -O2
endif
--
2.34.1
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 6/8] Makefile: define OPENSBI_DEBUG if DEBUG builds
2026-06-05 11:32 ` [PATCH v2 6/8] Makefile: define OPENSBI_DEBUG if DEBUG builds Bo Gan
@ 2026-06-16 8:19 ` Anup Patel
0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2026-06-16 8:19 UTC (permalink / raw)
To: Bo Gan
Cc: opensbi, wangruikang, dramforever, andrew.jones, cleger, pjw,
asrinivasan
Generally good to have at least one sentence of commit description.
I will take care of it at the time of merging.
On Fri, Jun 5, 2026 at 5:04 PM Bo Gan <ganboing@gmail.com> wrote:
>
> Signed-off-by: Bo Gan <ganboing@gmail.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Thanks,
Anup
> ---
> Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/Makefile b/Makefile
> index 46541063..6ce3c11e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -451,6 +451,7 @@ DTSCPPFLAGS = $(CPPFLAGS) -nostdinc -nostdlib -fno-builtin -D__DTS__ -x assemble
>
> ifneq ($(DEBUG),)
> CFLAGS += -O0
> +CPPFLAGS += -DOPENSBI_DEBUG
> else
> CFLAGS += -O2
> endif
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 7/8] lib: sbi: Rework load/store emulator instruction decoding
2026-06-05 11:32 [PATCH v2 0/8] Fixes for load/store misaligned and access faults Bo Gan
` (5 preceding siblings ...)
2026-06-05 11:32 ` [PATCH v2 6/8] Makefile: define OPENSBI_DEBUG if DEBUG builds Bo Gan
@ 2026-06-05 11:32 ` Bo Gan
2026-06-16 8:21 ` Anup Patel
2026-06-05 11:32 ` [PATCH v2 8/8] [NOT-FOR-UPSTREAM] Test program for misaligned load/store Bo Gan
` (2 subsequent siblings)
9 siblings, 1 reply; 17+ messages in thread
From: Bo Gan @ 2026-06-05 11:32 UTC (permalink / raw)
To: opensbi, wangruikang, dramforever, andrew.jones; +Cc: cleger, pjw, asrinivasan
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: 7219477f7b40 ("lib: Use MTINST CSR in misaligned load/store emulation")
Fixes: b5ae8e8a650d ("lib: Add misaligned load/store trap handling")
Fixes: 4c112650bbb0 ("lib: sbi: abstract out insn decoding to unify mem fault handlers")
Signed-off-by: Bo Gan <ganboing@gmail.com>
---
lib/sbi/sbi_trap_ldst.c | 436 ++++++++++++++++++++++++++++------------
1 file changed, 308 insertions(+), 128 deletions(-)
diff --git a/lib/sbi/sbi_trap_ldst.c b/lib/sbi/sbi_trap_ldst.c
index ad19926a..c1392251 100644
--- a/lib/sbi/sbi_trap_ldst.c
+++ b/lib/sbi/sbi_trap_ldst.c
@@ -44,28 +44,34 @@ ulong sbi_misaligned_tinst_fixup(ulong orig_tinst, ulong new_tinst,
return orig_tinst | (addr_offset << SH_RS1);
}
+static inline bool sbi_trap_tinst_valid(ulong tinst)
+{
+ /*
+ * Bit[0] == 1 implies trapped instruction value is
+ * transformed instruction or custom instruction.
+ * Also do proper checking per Privileged ISA 19.6.3,
+ * and make sure high 32 bits of tinst is 0
+ */
+ return tinst == (uint32_t)tinst && (tinst & 0x1);
+}
+
static int sbi_trap_emulate_load(struct sbi_trap_context *tcntx,
sbi_trap_ld_emulator emu)
{
const struct sbi_trap_info *orig_trap = &tcntx->trap;
struct sbi_trap_regs *regs = &tcntx->regs;
- ulong insn, insn_len;
+ ulong insn, insn_len, imm = 0, shift = 0, off = 0;
union sbi_ldst_data val = { 0 };
struct sbi_trap_info uptrap;
- int rc, fp = 0, shift = 0, len = 0;
+ bool xform = false, fp = false, c_load = false, c_ldsp = false;
+ int rc, len = 0, prev_xlen = 0;
- if (orig_trap->tinst & 0x1) {
- /*
- * Bit[0] == 1 implies trapped instruction value is
- * transformed instruction or custom instruction.
- */
+ if (sbi_trap_tinst_valid(orig_trap->tinst)) {
+ xform = true;
insn = orig_trap->tinst | INSN_16BIT_MASK;
insn_len = (orig_trap->tinst & 0x2) ? INSN_LEN(insn) : 2;
} else {
- /*
- * Bit[0] == 0 implies trapped instruction value is
- * zero or special value.
- */
+ /* trapped instruction value is zero or special value */
insn = sbi_get_insn(regs->mepc, &uptrap);
if (uptrap.cause) {
return sbi_trap_redirect(regs, &uptrap);
@@ -73,93 +79,176 @@ static int sbi_trap_emulate_load(struct sbi_trap_context *tcntx,
insn_len = INSN_LEN(insn);
}
+ /**
+ * Common for RV32/RV64:
+ * lb, lbu, lh, lhu, lw, flw, flw
+ * c.lbu, c.lh, c.lhu, c.lw, c.lwsp, c.fld, c.fldsp
+ */
if ((insn & INSN_MASK_LB) == INSN_MATCH_LB) {
- len = 1;
- shift = 8 * (sizeof(ulong) - len);
+ len = -1;
} else if ((insn & INSN_MASK_LBU) == INSN_MATCH_LBU) {
len = 1;
- } else if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
- len = 4;
- shift = 8 * (sizeof(ulong) - len);
-#if __riscv_xlen == 64
- } else if ((insn & INSN_MASK_LD) == INSN_MATCH_LD) {
- len = 8;
- shift = 8 * (sizeof(ulong) - len);
- } else if ((insn & INSN_MASK_LWU) == INSN_MATCH_LWU) {
- len = 4;
-#endif
-#ifdef __riscv_flen
- } else if ((insn & INSN_MASK_FLD) == INSN_MATCH_FLD) {
- fp = 1;
- len = 8;
- } else if ((insn & INSN_MASK_FLW) == INSN_MATCH_FLW) {
- fp = 1;
- len = 4;
-#endif
+ } else if ((insn & INSN_MASK_C_LBU) == INSN_MATCH_C_LBU) {
+ /* Zcb */
+ len = 1;
+ imm = RVC_LB_IMM(insn);
+ c_load = true;
} else if ((insn & INSN_MASK_LH) == INSN_MATCH_LH) {
- len = 2;
- shift = 8 * (sizeof(ulong) - len);
+ len = -2;
+ } else if ((insn & INSN_MASK_C_LH) == INSN_MATCH_C_LH) {
+ /* Zcb */
+ len = -2;
+ imm = RVC_LH_IMM(insn);
+ c_load = true;
} else if ((insn & INSN_MASK_LHU) == INSN_MATCH_LHU) {
len = 2;
-#if __riscv_xlen >= 64
- } else if ((insn & INSN_MASK_C_LD) == INSN_MATCH_C_LD) {
- len = 8;
- shift = 8 * (sizeof(ulong) - len);
- insn = RVC_RS2S(insn) << SH_RD;
- } else if ((insn & INSN_MASK_C_LDSP) == INSN_MATCH_C_LDSP &&
- ((insn >> SH_RD) & 0x1f)) {
- len = 8;
- shift = 8 * (sizeof(ulong) - len);
-#endif
+ } else if ((insn & INSN_MASK_C_LHU) == INSN_MATCH_C_LHU) {
+ /* Zcb */
+ len = 2;
+ imm = RVC_LH_IMM(insn);
+ c_load = true;
+ } else if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
+ len = -4;
} else if ((insn & INSN_MASK_C_LW) == INSN_MATCH_C_LW) {
- len = 4;
- shift = 8 * (sizeof(ulong) - len);
- insn = RVC_RS2S(insn) << SH_RD;
+ /* Zca */
+ len = -4;
+ imm = RVC_LW_IMM(insn);
+ c_load = true;
} else if ((insn & INSN_MASK_C_LWSP) == INSN_MATCH_C_LWSP &&
- ((insn >> SH_RD) & 0x1f)) {
- len = 4;
- shift = 8 * (sizeof(ulong) - len);
+ GET_RD_NUM(insn)) {
+ /* Zca */
+ len = -4;
+ imm = RVC_LWSP_IMM(insn);
+ c_ldsp = true;
#ifdef __riscv_flen
+ } else if ((insn & INSN_MASK_FLW) == INSN_MATCH_FLW) {
+ len = 4;
+ fp = true;
+ } else if ((insn & INSN_MASK_FLD) == INSN_MATCH_FLD) {
+ len = 8;
+ fp = true;
} else if ((insn & INSN_MASK_C_FLD) == INSN_MATCH_C_FLD) {
- fp = 1;
- len = 8;
- insn = RVC_RS2S(insn) << SH_RD;
+ /* Zcd */
+ len = 8;
+ imm = RVC_LD_IMM(insn);
+ c_load = true;
+ fp = true;
} else if ((insn & INSN_MASK_C_FLDSP) == INSN_MATCH_C_FLDSP) {
- fp = 1;
+ /* Zcd */
len = 8;
-#if __riscv_xlen == 32
- } else if ((insn & INSN_MASK_C_FLW) == INSN_MATCH_C_FLW) {
- fp = 1;
- len = 4;
- insn = RVC_RS2S(insn) << SH_RD;
- } else if ((insn & INSN_MASK_C_FLWSP) == INSN_MATCH_C_FLWSP) {
- fp = 1;
- len = 4;
+ imm = RVC_LDSP_IMM(insn);
+ c_ldsp = true;
+ fp = true;
#endif
+ } else {
+ prev_xlen = sbi_regs_prev_xlen(regs);
+ }
+
+ /**
+ * Must distinguish between rv64 and rv32, RVC instructions have
+ * overlapping encoding:
+ * c.ld in rv64 == c.flw in rv32
+ * c.ldsp in rv64 == c.flwsp in rv32
+ */
+ if (prev_xlen == 64) {
+ /* RV64 Only: lwu, ld, c.ld, c.ldsp */
+ if ((insn & INSN_MASK_LWU) == INSN_MATCH_LWU) {
+ len = 4;
+ } else if ((insn & INSN_MASK_LD) == INSN_MATCH_LD) {
+ len = 8;
+ } else if ((insn & INSN_MASK_C_LD) == INSN_MATCH_C_LD) {
+ /* Zca */
+ len = 8;
+ imm = RVC_LD_IMM(insn);
+ c_load = true;
+ } else if ((insn & INSN_MASK_C_LDSP) == INSN_MATCH_C_LDSP &&
+ GET_RD_NUM(insn)) {
+ /* Zca */
+ len = 8;
+ imm = RVC_LDSP_IMM(insn);
+ c_ldsp = true;
+ }
+#ifdef __riscv_flen
+ } else if (prev_xlen == 32) {
+ /* RV32 Only: c.flw, c.flwsp */
+ if ((insn & INSN_MASK_C_FLW) == INSN_MATCH_C_FLW) {
+ /* Zcf */
+ len = 4;
+ imm = RVC_LW_IMM(insn);
+ c_load = true;
+ fp = true;
+ } else if ((insn & INSN_MASK_C_FLWSP) == INSN_MATCH_C_FLWSP) {
+ /* Zcf */
+ len = 4;
+ imm = RVC_LWSP_IMM(insn);
+ c_ldsp = true;
+ fp = true;
+ }
#endif
- } else if ((insn & INSN_MASK_C_LHU) == INSN_MATCH_C_LHU) {
- len = 2;
- insn = RVC_RS2S(insn) << SH_RD;
- } else if ((insn & INSN_MASK_C_LH) == INSN_MATCH_C_LH) {
- len = 2;
+ }
+
+ if (len < 0) {
+ len = -len;
shift = 8 * (sizeof(ulong) - len);
- insn = RVC_RS2S(insn) << SH_RD;
}
- rc = emu(insn, len, orig_trap->tval, &val, tcntx);
+ if (!len) // Unknown instruction
+ goto do_emu;
+
+#if !defined(OPENSBI_DEBUG)
+ /**
+ * For misaligned faults. Skip offset calculation unless DEBUG
+ * builds. It helps validating OpenSBI and HW.
+ */
+ if (orig_trap->cause == CAUSE_MISALIGNED_LOAD)
+ goto do_emu;
+#endif
+
+ if (xform)
+ /* Transformed insn */
+ off = GET_RS1_NUM(insn);
+ else if (c_load)
+ /* non SP-based compressed load */
+ off = orig_trap->tval - GET_RS1S(insn, regs) - imm;
+ else if (c_ldsp)
+ /* SP-based compressed load */
+ off = orig_trap->tval - REG_VAL(2, regs) - imm;
+ else
+ /* I-type non-compressed load */
+ off = orig_trap->tval - GET_RS1(insn, regs) - (ulong)IMM_I(insn);
+ /**
+ * Normalize offset, in case the XLEN of unpriv mode is smaller,
+ * and/or pointer masking is in effect
+ */
+ off &= (len - 1);
+
+do_emu:
+ rc = emu(insn, len, orig_trap->tval - off, &val, tcntx);
if (rc <= 0)
return rc;
if (!len)
goto epc_fixup;
- if (!fp)
- SET_RD(insn, regs, ((long)(val.data_ulong << shift)) >> shift);
+ if (!fp) {
+ ulong v = ((long)(val.data_ulong << shift)) >> shift;
+
+ if (c_load)
+ SET_RDS(insn, regs, v);
+ else
+ SET_RD(insn, regs, v);
#ifdef __riscv_flen
- else if (len == 8)
- SET_F64_RD(insn, regs, val.data_u64);
- else
- SET_F32_RD(insn, regs, val.data_ulong);
+ } else if (len == 8) {
+ if (c_load)
+ SET_F64_RDS(insn, regs, val.data_u64);
+ else
+ SET_F64_RD(insn, regs, val.data_u64);
+ } else {
+ if (c_load)
+ SET_F32_RDS(insn, regs, val.data_ulong);
+ else
+ SET_F32_RD(insn, regs, val.data_ulong);
#endif
+ }
epc_fixup:
regs->mepc += insn_len;
@@ -172,23 +261,18 @@ static int sbi_trap_emulate_store(struct sbi_trap_context *tcntx,
{
const struct sbi_trap_info *orig_trap = &tcntx->trap;
struct sbi_trap_regs *regs = &tcntx->regs;
- ulong insn, insn_len;
+ ulong insn, insn_len, imm = 0, off = 0;
union sbi_ldst_data val;
struct sbi_trap_info uptrap;
- int rc, len = 0;
+ bool xform = false, fp = false, c_store = false, c_stsp = false;
+ int rc, len = 0, prev_xlen = 0;
- if (orig_trap->tinst & 0x1) {
- /*
- * Bit[0] == 1 implies trapped instruction value is
- * transformed instruction or custom instruction.
- */
+ if (sbi_trap_tinst_valid(orig_trap->tinst)) {
+ xform = true;
insn = orig_trap->tinst | INSN_16BIT_MASK;
insn_len = (orig_trap->tinst & 0x2) ? INSN_LEN(insn) : 2;
} else {
- /*
- * Bit[0] == 0 implies trapped instruction value is
- * zero or special value.
- */
+ /* trapped instruction value is zero or special value */
insn = sbi_get_insn(regs->mepc, &uptrap);
if (uptrap.cause) {
return sbi_trap_redirect(regs, &uptrap);
@@ -196,62 +280,158 @@ static int sbi_trap_emulate_store(struct sbi_trap_context *tcntx,
insn_len = INSN_LEN(insn);
}
- val.data_ulong = GET_RS2(insn, regs);
-
+ /**
+ * Common for RV32/RV64:
+ * sb, sh, sw, fsw, fsd
+ * c.sb, c.sh, c.sw, c.swsp, c.fsd, c.fsdsp
+ */
if ((insn & INSN_MASK_SB) == INSN_MATCH_SB) {
len = 1;
- } else if ((insn & INSN_MASK_SW) == INSN_MATCH_SW) {
- len = 4;
-#if __riscv_xlen == 64
- } else if ((insn & INSN_MASK_SD) == INSN_MATCH_SD) {
- len = 8;
-#endif
-#ifdef __riscv_flen
- } else if ((insn & INSN_MASK_FSD) == INSN_MATCH_FSD) {
- len = 8;
- val.data_u64 = GET_F64_RS2(insn, regs);
- } else if ((insn & INSN_MASK_FSW) == INSN_MATCH_FSW) {
- len = 4;
- val.data_ulong = GET_F32_RS2(insn, regs);
-#endif
+ } else if ((insn & INSN_MASK_C_SB) == INSN_MATCH_C_SB) {
+ /* Zcb */
+ len = 1;
+ imm = RVC_SB_IMM(insn);
+ c_store = true;
} else if ((insn & INSN_MASK_SH) == INSN_MATCH_SH) {
len = 2;
-#if __riscv_xlen >= 64
- } else if ((insn & INSN_MASK_C_SD) == INSN_MATCH_C_SD) {
- len = 8;
- val.data_ulong = GET_RS2S(insn, regs);
- } else if ((insn & INSN_MASK_C_SDSP) == INSN_MATCH_C_SDSP) {
- len = 8;
- val.data_ulong = GET_RS2C(insn, regs);
-#endif
+ } else if ((insn & INSN_MASK_C_SH) == INSN_MATCH_C_SH) {
+ /* Zcb */
+ len = 2;
+ imm = RVC_SH_IMM(insn);
+ c_store = true;
+ } else if ((insn & INSN_MASK_SW) == INSN_MATCH_SW) {
+ len = 4;
} else if ((insn & INSN_MASK_C_SW) == INSN_MATCH_C_SW) {
- len = 4;
- val.data_ulong = GET_RS2S(insn, regs);
+ /* Zca */
+ len = 4;
+ imm = RVC_SW_IMM(insn);
+ c_store = true;
} else if ((insn & INSN_MASK_C_SWSP) == INSN_MATCH_C_SWSP) {
- len = 4;
- val.data_ulong = GET_RS2C(insn, regs);
+ /* Zca */
+ len = 4;
+ imm = RVC_SWSP_IMM(insn);
+ c_stsp = true;
#ifdef __riscv_flen
+ } else if ((insn & INSN_MASK_FSW) == INSN_MATCH_FSW) {
+ len = 4;
+ fp = true;
+ } else if ((insn & INSN_MASK_FSD) == INSN_MATCH_FSD) {
+ len = 8;
+ fp = true;
} else if ((insn & INSN_MASK_C_FSD) == INSN_MATCH_C_FSD) {
- len = 8;
- val.data_u64 = GET_F64_RS2S(insn, regs);
+ /* Zcd */
+ len = 8;
+ imm = RVC_SD_IMM(insn);
+ c_store = true;
+ fp = true;
} else if ((insn & INSN_MASK_C_FSDSP) == INSN_MATCH_C_FSDSP) {
- len = 8;
- val.data_u64 = GET_F64_RS2C(insn, regs);
-#if __riscv_xlen == 32
- } else if ((insn & INSN_MASK_C_FSW) == INSN_MATCH_C_FSW) {
- len = 4;
- val.data_ulong = GET_F32_RS2S(insn, regs);
- } else if ((insn & INSN_MASK_C_FSWSP) == INSN_MATCH_C_FSWSP) {
- len = 4;
- val.data_ulong = GET_F32_RS2C(insn, regs);
+ /* Zcd */
+ len = 8;
+ imm = RVC_SDSP_IMM(insn);
+ c_stsp = true;
+ fp = true;
#endif
+ } else {
+ prev_xlen = sbi_regs_prev_xlen(regs);
+ }
+
+ /**
+ * Must distinguish between rv64 and rv32, RVC instructions have
+ * overlapping encoding:
+ * c.sd in rv64 == c.fsw in rv32
+ * c.sdsp in rv64 == c.fswsp in rv32
+ */
+ if (prev_xlen == 64) {
+ /* RV64 Only: sd, c.sd, c.sdsp */
+ if ((insn & INSN_MASK_SD) == INSN_MATCH_SD) {
+ len = 8;
+ } else if ((insn & INSN_MASK_C_SD) == INSN_MATCH_C_SD) {
+ /* Zca */
+ len = 8;
+ imm = RVC_SD_IMM(insn);
+ c_store = true;
+ } else if ((insn & INSN_MASK_C_SDSP) == INSN_MATCH_C_SDSP) {
+ /* Zca */
+ len = 8;
+ imm = RVC_SDSP_IMM(insn);
+ c_stsp = true;
+ }
+#ifdef __riscv_flen
+ } else if (prev_xlen == 32) {
+ /* RV32 Only: c.fsw, c.fswsp */
+ if ((insn & INSN_MASK_C_FSW) == INSN_MATCH_C_FSW) {
+ /* Zcf */
+ len = 4;
+ imm = RVC_SW_IMM(insn);
+ c_store = true;
+ fp = true;
+ } else if ((insn & INSN_MASK_C_FSWSP) == INSN_MATCH_C_FSWSP) {
+ /* Zcf */
+ len = 4;
+ imm = RVC_SWSP_IMM(insn);
+ c_stsp = true;
+ fp = true;
+ }
+#endif
+ }
+
+ if (!fp) {
+ if (c_store)
+ val.data_ulong = GET_RS2S(insn, regs);
+ else if (c_stsp)
+ val.data_ulong = GET_RS2C(insn, regs);
+ else
+ val.data_ulong = GET_RS2(insn, regs);
+#ifdef __riscv_flen
+ } else if (len == 8) {
+ if (c_store)
+ val.data_u64 = GET_F64_RS2S(insn, regs);
+ else if (c_stsp)
+ val.data_u64 = GET_F64_RS2C(insn, regs);
+ else
+ val.data_u64 = GET_F64_RS2(insn, regs);
+ } else {
+ if (c_store)
+ val.data_ulong = GET_F32_RS2S(insn, regs);
+ else if (c_stsp)
+ val.data_ulong = GET_F32_RS2C(insn, regs);
+ else
+ val.data_ulong = GET_F32_RS2(insn, regs);
#endif
- } else if ((insn & INSN_MASK_C_SH) == INSN_MATCH_C_SH) {
- len = 2;
- val.data_ulong = GET_RS2S(insn, regs);
}
- rc = emu(insn, len, orig_trap->tval, val, tcntx);
+ if (!len) // Unknown instruction
+ goto do_emu;
+
+#if !defined(OPENSBI_DEBUG)
+ /**
+ * For misaligned faults. Skip offset calculation unless DEBUG
+ * builds. It helps validating OpenSBI and HW.
+ */
+ if (orig_trap->cause == CAUSE_MISALIGNED_STORE)
+ goto do_emu;
+#endif
+
+ if (xform)
+ /* Transformed insn */
+ off = GET_RS1_NUM(insn);
+ else if (c_store)
+ /* non SP-based compressed store */
+ off = orig_trap->tval - GET_RS1S(insn, regs) - imm;
+ else if (c_stsp)
+ /* SP-based compressed store */
+ off = orig_trap->tval - REG_VAL(2, regs) - imm;
+ else
+ /* S-type non-compressed store */
+ off = orig_trap->tval - GET_RS1(insn, regs) - (ulong)IMM_S(insn);
+ /**
+ * Normalize offset, in case the XLEN of unpriv mode is smaller,
+ * and/or pointer masking is in effect
+ */
+ off &= (len - 1);
+
+do_emu:
+ rc = emu(insn, len, orig_trap->tval - off, val, tcntx);
if (rc <= 0)
return rc;
--
2.34.1
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 7/8] lib: sbi: Rework load/store emulator instruction decoding
2026-06-05 11:32 ` [PATCH v2 7/8] lib: sbi: Rework load/store emulator instruction decoding Bo Gan
@ 2026-06-16 8:21 ` Anup Patel
0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2026-06-16 8:21 UTC (permalink / raw)
To: Bo Gan
Cc: opensbi, wangruikang, dramforever, andrew.jones, cleger, pjw,
asrinivasan
On Fri, Jun 5, 2026 at 8:22 PM Bo Gan <ganboing@gmail.com> wrote:
>
> 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: 7219477f7b40 ("lib: Use MTINST CSR in misaligned load/store emulation")
> Fixes: b5ae8e8a650d ("lib: Add misaligned load/store trap handling")
> Fixes: 4c112650bbb0 ("lib: sbi: abstract out insn decoding to unify mem fault handlers")
> Signed-off-by: Bo Gan <ganboing@gmail.com>
LGTM.
Reviewed-by: Anup Patel <anup@brainfault.org>
Thanks,
Anup
> ---
> lib/sbi/sbi_trap_ldst.c | 436 ++++++++++++++++++++++++++++------------
> 1 file changed, 308 insertions(+), 128 deletions(-)
>
> diff --git a/lib/sbi/sbi_trap_ldst.c b/lib/sbi/sbi_trap_ldst.c
> index ad19926a..c1392251 100644
> --- a/lib/sbi/sbi_trap_ldst.c
> +++ b/lib/sbi/sbi_trap_ldst.c
> @@ -44,28 +44,34 @@ ulong sbi_misaligned_tinst_fixup(ulong orig_tinst, ulong new_tinst,
> return orig_tinst | (addr_offset << SH_RS1);
> }
>
> +static inline bool sbi_trap_tinst_valid(ulong tinst)
> +{
> + /*
> + * Bit[0] == 1 implies trapped instruction value is
> + * transformed instruction or custom instruction.
> + * Also do proper checking per Privileged ISA 19.6.3,
> + * and make sure high 32 bits of tinst is 0
> + */
> + return tinst == (uint32_t)tinst && (tinst & 0x1);
> +}
> +
> static int sbi_trap_emulate_load(struct sbi_trap_context *tcntx,
> sbi_trap_ld_emulator emu)
> {
> const struct sbi_trap_info *orig_trap = &tcntx->trap;
> struct sbi_trap_regs *regs = &tcntx->regs;
> - ulong insn, insn_len;
> + ulong insn, insn_len, imm = 0, shift = 0, off = 0;
> union sbi_ldst_data val = { 0 };
> struct sbi_trap_info uptrap;
> - int rc, fp = 0, shift = 0, len = 0;
> + bool xform = false, fp = false, c_load = false, c_ldsp = false;
> + int rc, len = 0, prev_xlen = 0;
>
> - if (orig_trap->tinst & 0x1) {
> - /*
> - * Bit[0] == 1 implies trapped instruction value is
> - * transformed instruction or custom instruction.
> - */
> + if (sbi_trap_tinst_valid(orig_trap->tinst)) {
> + xform = true;
> insn = orig_trap->tinst | INSN_16BIT_MASK;
> insn_len = (orig_trap->tinst & 0x2) ? INSN_LEN(insn) : 2;
> } else {
> - /*
> - * Bit[0] == 0 implies trapped instruction value is
> - * zero or special value.
> - */
> + /* trapped instruction value is zero or special value */
> insn = sbi_get_insn(regs->mepc, &uptrap);
> if (uptrap.cause) {
> return sbi_trap_redirect(regs, &uptrap);
> @@ -73,93 +79,176 @@ static int sbi_trap_emulate_load(struct sbi_trap_context *tcntx,
> insn_len = INSN_LEN(insn);
> }
>
> + /**
> + * Common for RV32/RV64:
> + * lb, lbu, lh, lhu, lw, flw, flw
> + * c.lbu, c.lh, c.lhu, c.lw, c.lwsp, c.fld, c.fldsp
> + */
> if ((insn & INSN_MASK_LB) == INSN_MATCH_LB) {
> - len = 1;
> - shift = 8 * (sizeof(ulong) - len);
> + len = -1;
> } else if ((insn & INSN_MASK_LBU) == INSN_MATCH_LBU) {
> len = 1;
> - } else if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
> - len = 4;
> - shift = 8 * (sizeof(ulong) - len);
> -#if __riscv_xlen == 64
> - } else if ((insn & INSN_MASK_LD) == INSN_MATCH_LD) {
> - len = 8;
> - shift = 8 * (sizeof(ulong) - len);
> - } else if ((insn & INSN_MASK_LWU) == INSN_MATCH_LWU) {
> - len = 4;
> -#endif
> -#ifdef __riscv_flen
> - } else if ((insn & INSN_MASK_FLD) == INSN_MATCH_FLD) {
> - fp = 1;
> - len = 8;
> - } else if ((insn & INSN_MASK_FLW) == INSN_MATCH_FLW) {
> - fp = 1;
> - len = 4;
> -#endif
> + } else if ((insn & INSN_MASK_C_LBU) == INSN_MATCH_C_LBU) {
> + /* Zcb */
> + len = 1;
> + imm = RVC_LB_IMM(insn);
> + c_load = true;
> } else if ((insn & INSN_MASK_LH) == INSN_MATCH_LH) {
> - len = 2;
> - shift = 8 * (sizeof(ulong) - len);
> + len = -2;
> + } else if ((insn & INSN_MASK_C_LH) == INSN_MATCH_C_LH) {
> + /* Zcb */
> + len = -2;
> + imm = RVC_LH_IMM(insn);
> + c_load = true;
> } else if ((insn & INSN_MASK_LHU) == INSN_MATCH_LHU) {
> len = 2;
> -#if __riscv_xlen >= 64
> - } else if ((insn & INSN_MASK_C_LD) == INSN_MATCH_C_LD) {
> - len = 8;
> - shift = 8 * (sizeof(ulong) - len);
> - insn = RVC_RS2S(insn) << SH_RD;
> - } else if ((insn & INSN_MASK_C_LDSP) == INSN_MATCH_C_LDSP &&
> - ((insn >> SH_RD) & 0x1f)) {
> - len = 8;
> - shift = 8 * (sizeof(ulong) - len);
> -#endif
> + } else if ((insn & INSN_MASK_C_LHU) == INSN_MATCH_C_LHU) {
> + /* Zcb */
> + len = 2;
> + imm = RVC_LH_IMM(insn);
> + c_load = true;
> + } else if ((insn & INSN_MASK_LW) == INSN_MATCH_LW) {
> + len = -4;
> } else if ((insn & INSN_MASK_C_LW) == INSN_MATCH_C_LW) {
> - len = 4;
> - shift = 8 * (sizeof(ulong) - len);
> - insn = RVC_RS2S(insn) << SH_RD;
> + /* Zca */
> + len = -4;
> + imm = RVC_LW_IMM(insn);
> + c_load = true;
> } else if ((insn & INSN_MASK_C_LWSP) == INSN_MATCH_C_LWSP &&
> - ((insn >> SH_RD) & 0x1f)) {
> - len = 4;
> - shift = 8 * (sizeof(ulong) - len);
> + GET_RD_NUM(insn)) {
> + /* Zca */
> + len = -4;
> + imm = RVC_LWSP_IMM(insn);
> + c_ldsp = true;
> #ifdef __riscv_flen
> + } else if ((insn & INSN_MASK_FLW) == INSN_MATCH_FLW) {
> + len = 4;
> + fp = true;
> + } else if ((insn & INSN_MASK_FLD) == INSN_MATCH_FLD) {
> + len = 8;
> + fp = true;
> } else if ((insn & INSN_MASK_C_FLD) == INSN_MATCH_C_FLD) {
> - fp = 1;
> - len = 8;
> - insn = RVC_RS2S(insn) << SH_RD;
> + /* Zcd */
> + len = 8;
> + imm = RVC_LD_IMM(insn);
> + c_load = true;
> + fp = true;
> } else if ((insn & INSN_MASK_C_FLDSP) == INSN_MATCH_C_FLDSP) {
> - fp = 1;
> + /* Zcd */
> len = 8;
> -#if __riscv_xlen == 32
> - } else if ((insn & INSN_MASK_C_FLW) == INSN_MATCH_C_FLW) {
> - fp = 1;
> - len = 4;
> - insn = RVC_RS2S(insn) << SH_RD;
> - } else if ((insn & INSN_MASK_C_FLWSP) == INSN_MATCH_C_FLWSP) {
> - fp = 1;
> - len = 4;
> + imm = RVC_LDSP_IMM(insn);
> + c_ldsp = true;
> + fp = true;
> #endif
> + } else {
> + prev_xlen = sbi_regs_prev_xlen(regs);
> + }
> +
> + /**
> + * Must distinguish between rv64 and rv32, RVC instructions have
> + * overlapping encoding:
> + * c.ld in rv64 == c.flw in rv32
> + * c.ldsp in rv64 == c.flwsp in rv32
> + */
> + if (prev_xlen == 64) {
> + /* RV64 Only: lwu, ld, c.ld, c.ldsp */
> + if ((insn & INSN_MASK_LWU) == INSN_MATCH_LWU) {
> + len = 4;
> + } else if ((insn & INSN_MASK_LD) == INSN_MATCH_LD) {
> + len = 8;
> + } else if ((insn & INSN_MASK_C_LD) == INSN_MATCH_C_LD) {
> + /* Zca */
> + len = 8;
> + imm = RVC_LD_IMM(insn);
> + c_load = true;
> + } else if ((insn & INSN_MASK_C_LDSP) == INSN_MATCH_C_LDSP &&
> + GET_RD_NUM(insn)) {
> + /* Zca */
> + len = 8;
> + imm = RVC_LDSP_IMM(insn);
> + c_ldsp = true;
> + }
> +#ifdef __riscv_flen
> + } else if (prev_xlen == 32) {
> + /* RV32 Only: c.flw, c.flwsp */
> + if ((insn & INSN_MASK_C_FLW) == INSN_MATCH_C_FLW) {
> + /* Zcf */
> + len = 4;
> + imm = RVC_LW_IMM(insn);
> + c_load = true;
> + fp = true;
> + } else if ((insn & INSN_MASK_C_FLWSP) == INSN_MATCH_C_FLWSP) {
> + /* Zcf */
> + len = 4;
> + imm = RVC_LWSP_IMM(insn);
> + c_ldsp = true;
> + fp = true;
> + }
> #endif
> - } else if ((insn & INSN_MASK_C_LHU) == INSN_MATCH_C_LHU) {
> - len = 2;
> - insn = RVC_RS2S(insn) << SH_RD;
> - } else if ((insn & INSN_MASK_C_LH) == INSN_MATCH_C_LH) {
> - len = 2;
> + }
> +
> + if (len < 0) {
> + len = -len;
> shift = 8 * (sizeof(ulong) - len);
> - insn = RVC_RS2S(insn) << SH_RD;
> }
>
> - rc = emu(insn, len, orig_trap->tval, &val, tcntx);
> + if (!len) // Unknown instruction
> + goto do_emu;
> +
> +#if !defined(OPENSBI_DEBUG)
> + /**
> + * For misaligned faults. Skip offset calculation unless DEBUG
> + * builds. It helps validating OpenSBI and HW.
> + */
> + if (orig_trap->cause == CAUSE_MISALIGNED_LOAD)
> + goto do_emu;
> +#endif
> +
> + if (xform)
> + /* Transformed insn */
> + off = GET_RS1_NUM(insn);
> + else if (c_load)
> + /* non SP-based compressed load */
> + off = orig_trap->tval - GET_RS1S(insn, regs) - imm;
> + else if (c_ldsp)
> + /* SP-based compressed load */
> + off = orig_trap->tval - REG_VAL(2, regs) - imm;
> + else
> + /* I-type non-compressed load */
> + off = orig_trap->tval - GET_RS1(insn, regs) - (ulong)IMM_I(insn);
> + /**
> + * Normalize offset, in case the XLEN of unpriv mode is smaller,
> + * and/or pointer masking is in effect
> + */
> + off &= (len - 1);
> +
> +do_emu:
> + rc = emu(insn, len, orig_trap->tval - off, &val, tcntx);
> if (rc <= 0)
> return rc;
> if (!len)
> goto epc_fixup;
>
> - if (!fp)
> - SET_RD(insn, regs, ((long)(val.data_ulong << shift)) >> shift);
> + if (!fp) {
> + ulong v = ((long)(val.data_ulong << shift)) >> shift;
> +
> + if (c_load)
> + SET_RDS(insn, regs, v);
> + else
> + SET_RD(insn, regs, v);
> #ifdef __riscv_flen
> - else if (len == 8)
> - SET_F64_RD(insn, regs, val.data_u64);
> - else
> - SET_F32_RD(insn, regs, val.data_ulong);
> + } else if (len == 8) {
> + if (c_load)
> + SET_F64_RDS(insn, regs, val.data_u64);
> + else
> + SET_F64_RD(insn, regs, val.data_u64);
> + } else {
> + if (c_load)
> + SET_F32_RDS(insn, regs, val.data_ulong);
> + else
> + SET_F32_RD(insn, regs, val.data_ulong);
> #endif
> + }
>
> epc_fixup:
> regs->mepc += insn_len;
> @@ -172,23 +261,18 @@ static int sbi_trap_emulate_store(struct sbi_trap_context *tcntx,
> {
> const struct sbi_trap_info *orig_trap = &tcntx->trap;
> struct sbi_trap_regs *regs = &tcntx->regs;
> - ulong insn, insn_len;
> + ulong insn, insn_len, imm = 0, off = 0;
> union sbi_ldst_data val;
> struct sbi_trap_info uptrap;
> - int rc, len = 0;
> + bool xform = false, fp = false, c_store = false, c_stsp = false;
> + int rc, len = 0, prev_xlen = 0;
>
> - if (orig_trap->tinst & 0x1) {
> - /*
> - * Bit[0] == 1 implies trapped instruction value is
> - * transformed instruction or custom instruction.
> - */
> + if (sbi_trap_tinst_valid(orig_trap->tinst)) {
> + xform = true;
> insn = orig_trap->tinst | INSN_16BIT_MASK;
> insn_len = (orig_trap->tinst & 0x2) ? INSN_LEN(insn) : 2;
> } else {
> - /*
> - * Bit[0] == 0 implies trapped instruction value is
> - * zero or special value.
> - */
> + /* trapped instruction value is zero or special value */
> insn = sbi_get_insn(regs->mepc, &uptrap);
> if (uptrap.cause) {
> return sbi_trap_redirect(regs, &uptrap);
> @@ -196,62 +280,158 @@ static int sbi_trap_emulate_store(struct sbi_trap_context *tcntx,
> insn_len = INSN_LEN(insn);
> }
>
> - val.data_ulong = GET_RS2(insn, regs);
> -
> + /**
> + * Common for RV32/RV64:
> + * sb, sh, sw, fsw, fsd
> + * c.sb, c.sh, c.sw, c.swsp, c.fsd, c.fsdsp
> + */
> if ((insn & INSN_MASK_SB) == INSN_MATCH_SB) {
> len = 1;
> - } else if ((insn & INSN_MASK_SW) == INSN_MATCH_SW) {
> - len = 4;
> -#if __riscv_xlen == 64
> - } else if ((insn & INSN_MASK_SD) == INSN_MATCH_SD) {
> - len = 8;
> -#endif
> -#ifdef __riscv_flen
> - } else if ((insn & INSN_MASK_FSD) == INSN_MATCH_FSD) {
> - len = 8;
> - val.data_u64 = GET_F64_RS2(insn, regs);
> - } else if ((insn & INSN_MASK_FSW) == INSN_MATCH_FSW) {
> - len = 4;
> - val.data_ulong = GET_F32_RS2(insn, regs);
> -#endif
> + } else if ((insn & INSN_MASK_C_SB) == INSN_MATCH_C_SB) {
> + /* Zcb */
> + len = 1;
> + imm = RVC_SB_IMM(insn);
> + c_store = true;
> } else if ((insn & INSN_MASK_SH) == INSN_MATCH_SH) {
> len = 2;
> -#if __riscv_xlen >= 64
> - } else if ((insn & INSN_MASK_C_SD) == INSN_MATCH_C_SD) {
> - len = 8;
> - val.data_ulong = GET_RS2S(insn, regs);
> - } else if ((insn & INSN_MASK_C_SDSP) == INSN_MATCH_C_SDSP) {
> - len = 8;
> - val.data_ulong = GET_RS2C(insn, regs);
> -#endif
> + } else if ((insn & INSN_MASK_C_SH) == INSN_MATCH_C_SH) {
> + /* Zcb */
> + len = 2;
> + imm = RVC_SH_IMM(insn);
> + c_store = true;
> + } else if ((insn & INSN_MASK_SW) == INSN_MATCH_SW) {
> + len = 4;
> } else if ((insn & INSN_MASK_C_SW) == INSN_MATCH_C_SW) {
> - len = 4;
> - val.data_ulong = GET_RS2S(insn, regs);
> + /* Zca */
> + len = 4;
> + imm = RVC_SW_IMM(insn);
> + c_store = true;
> } else if ((insn & INSN_MASK_C_SWSP) == INSN_MATCH_C_SWSP) {
> - len = 4;
> - val.data_ulong = GET_RS2C(insn, regs);
> + /* Zca */
> + len = 4;
> + imm = RVC_SWSP_IMM(insn);
> + c_stsp = true;
> #ifdef __riscv_flen
> + } else if ((insn & INSN_MASK_FSW) == INSN_MATCH_FSW) {
> + len = 4;
> + fp = true;
> + } else if ((insn & INSN_MASK_FSD) == INSN_MATCH_FSD) {
> + len = 8;
> + fp = true;
> } else if ((insn & INSN_MASK_C_FSD) == INSN_MATCH_C_FSD) {
> - len = 8;
> - val.data_u64 = GET_F64_RS2S(insn, regs);
> + /* Zcd */
> + len = 8;
> + imm = RVC_SD_IMM(insn);
> + c_store = true;
> + fp = true;
> } else if ((insn & INSN_MASK_C_FSDSP) == INSN_MATCH_C_FSDSP) {
> - len = 8;
> - val.data_u64 = GET_F64_RS2C(insn, regs);
> -#if __riscv_xlen == 32
> - } else if ((insn & INSN_MASK_C_FSW) == INSN_MATCH_C_FSW) {
> - len = 4;
> - val.data_ulong = GET_F32_RS2S(insn, regs);
> - } else if ((insn & INSN_MASK_C_FSWSP) == INSN_MATCH_C_FSWSP) {
> - len = 4;
> - val.data_ulong = GET_F32_RS2C(insn, regs);
> + /* Zcd */
> + len = 8;
> + imm = RVC_SDSP_IMM(insn);
> + c_stsp = true;
> + fp = true;
> #endif
> + } else {
> + prev_xlen = sbi_regs_prev_xlen(regs);
> + }
> +
> + /**
> + * Must distinguish between rv64 and rv32, RVC instructions have
> + * overlapping encoding:
> + * c.sd in rv64 == c.fsw in rv32
> + * c.sdsp in rv64 == c.fswsp in rv32
> + */
> + if (prev_xlen == 64) {
> + /* RV64 Only: sd, c.sd, c.sdsp */
> + if ((insn & INSN_MASK_SD) == INSN_MATCH_SD) {
> + len = 8;
> + } else if ((insn & INSN_MASK_C_SD) == INSN_MATCH_C_SD) {
> + /* Zca */
> + len = 8;
> + imm = RVC_SD_IMM(insn);
> + c_store = true;
> + } else if ((insn & INSN_MASK_C_SDSP) == INSN_MATCH_C_SDSP) {
> + /* Zca */
> + len = 8;
> + imm = RVC_SDSP_IMM(insn);
> + c_stsp = true;
> + }
> +#ifdef __riscv_flen
> + } else if (prev_xlen == 32) {
> + /* RV32 Only: c.fsw, c.fswsp */
> + if ((insn & INSN_MASK_C_FSW) == INSN_MATCH_C_FSW) {
> + /* Zcf */
> + len = 4;
> + imm = RVC_SW_IMM(insn);
> + c_store = true;
> + fp = true;
> + } else if ((insn & INSN_MASK_C_FSWSP) == INSN_MATCH_C_FSWSP) {
> + /* Zcf */
> + len = 4;
> + imm = RVC_SWSP_IMM(insn);
> + c_stsp = true;
> + fp = true;
> + }
> +#endif
> + }
> +
> + if (!fp) {
> + if (c_store)
> + val.data_ulong = GET_RS2S(insn, regs);
> + else if (c_stsp)
> + val.data_ulong = GET_RS2C(insn, regs);
> + else
> + val.data_ulong = GET_RS2(insn, regs);
> +#ifdef __riscv_flen
> + } else if (len == 8) {
> + if (c_store)
> + val.data_u64 = GET_F64_RS2S(insn, regs);
> + else if (c_stsp)
> + val.data_u64 = GET_F64_RS2C(insn, regs);
> + else
> + val.data_u64 = GET_F64_RS2(insn, regs);
> + } else {
> + if (c_store)
> + val.data_ulong = GET_F32_RS2S(insn, regs);
> + else if (c_stsp)
> + val.data_ulong = GET_F32_RS2C(insn, regs);
> + else
> + val.data_ulong = GET_F32_RS2(insn, regs);
> #endif
> - } else if ((insn & INSN_MASK_C_SH) == INSN_MATCH_C_SH) {
> - len = 2;
> - val.data_ulong = GET_RS2S(insn, regs);
> }
>
> - rc = emu(insn, len, orig_trap->tval, val, tcntx);
> + if (!len) // Unknown instruction
> + goto do_emu;
> +
> +#if !defined(OPENSBI_DEBUG)
> + /**
> + * For misaligned faults. Skip offset calculation unless DEBUG
> + * builds. It helps validating OpenSBI and HW.
> + */
> + if (orig_trap->cause == CAUSE_MISALIGNED_STORE)
> + goto do_emu;
> +#endif
> +
> + if (xform)
> + /* Transformed insn */
> + off = GET_RS1_NUM(insn);
> + else if (c_store)
> + /* non SP-based compressed store */
> + off = orig_trap->tval - GET_RS1S(insn, regs) - imm;
> + else if (c_stsp)
> + /* SP-based compressed store */
> + off = orig_trap->tval - REG_VAL(2, regs) - imm;
> + else
> + /* S-type non-compressed store */
> + off = orig_trap->tval - GET_RS1(insn, regs) - (ulong)IMM_S(insn);
> + /**
> + * Normalize offset, in case the XLEN of unpriv mode is smaller,
> + * and/or pointer masking is in effect
> + */
> + off &= (len - 1);
> +
> +do_emu:
> + rc = emu(insn, len, orig_trap->tval - off, val, tcntx);
> if (rc <= 0)
> return rc;
>
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v2 8/8] [NOT-FOR-UPSTREAM] Test program for misaligned load/store
2026-06-05 11:32 [PATCH v2 0/8] Fixes for load/store misaligned and access faults Bo Gan
` (6 preceding siblings ...)
2026-06-05 11:32 ` [PATCH v2 7/8] lib: sbi: Rework load/store emulator instruction decoding Bo Gan
@ 2026-06-05 11:32 ` Bo Gan
2026-06-16 8:23 ` Anup Patel
2026-06-08 18:18 ` [PATCH v2 0/8] Fixes for load/store misaligned and access faults Anirudh Srinivasan
2026-06-16 8:25 ` Anup Patel
9 siblings, 1 reply; 17+ messages in thread
From: Bo Gan @ 2026-06-05 11:32 UTC (permalink / raw)
To: opensbi, wangruikang, dramforever, andrew.jones; +Cc: cleger, pjw, asrinivasan
Build: gcc -o test-misaligned-ldst test-misaligned-ldst.c ldst.S
Failure observed before the fix on QEMU with 64-bit Linux and 32-bit test
...
loadfp 64, insn cfldsp, n=64, off=1, cmp=788796a5b4c3d2e1
loadfp 64, insn cfldsp, n=64, off=2, cmp=69788796a5b4c3d2
loadfp 64, insn cfldsp, n=64, off=3, cmp=5a69788796a5b4c3
loadfp 64, insn cfldsp, n=64, off=4, cmp=4b5a69788796a5b4
loadfp 64, insn cfldsp, n=64, off=5, cmp=3c4b5a69788796a5
loadfp 64, insn cfldsp, n=64, off=6, cmp=2d3c4b5a69788796
loadfp 64, insn cfldsp, n=64, off=7, cmp=1e2d3c4b5a697887
loadfp 32, insn cflw, n=32, off=1, cmp=ffffffffb4c3d2e1
./test32: failed at load_f 1 4 0: 0
With the patch series:
...
loadfp 64, insn cfldsp, n=64, off=1, cmp=788796a5b4c3d2e1
loadfp 64, insn cfldsp, n=64, off=2, cmp=69788796a5b4c3d2
loadfp 64, insn cfldsp, n=64, off=3, cmp=5a69788796a5b4c3
loadfp 64, insn cfldsp, n=64, off=4, cmp=4b5a69788796a5b4
loadfp 64, insn cfldsp, n=64, off=5, cmp=3c4b5a69788796a5
loadfp 64, insn cfldsp, n=64, off=6, cmp=2d3c4b5a69788796
loadfp 64, insn cfldsp, n=64, off=7, cmp=1e2d3c4b5a697887
loadfp 32, insn cflw, n=32, off=1, cmp=ffffffffb4c3d2e1
loadfp 32, insn cflw, n=32, off=2, cmp=ffffffffa5b4c3d2
loadfp 32, insn cflw, n=32, off=3, cmp=ffffffff96a5b4c3
loadfp 32, insn cflw, n=32, off=5, cmp=ffffffff788796a5
loadfp 32, insn cflw, n=32, off=6, cmp=ffffffff69788796
loadfp 32, insn cflw, n=32, off=7, cmp=ffffffff5a697887
loadfp 32, insn cflw, n=32, off=9, cmp=ffffffff3c4b5a69
loadfp 32, insn cflw, n=32, off=10, cmp=ffffffff2d3c4b5a
loadfp 32, insn cflw, n=32, off=11, cmp=ffffffff1e2d3c4b
loadfp 32, insn cflwsp, n=64, off=1, cmp=ffffffffb4c3d2e1
loadfp 32, insn cflwsp, n=64, off=2, cmp=ffffffffa5b4c3d2
loadfp 32, insn cflwsp, n=64, off=3, cmp=ffffffff96a5b4c3
loadfp 32, insn cflwsp, n=64, off=5, cmp=ffffffff788796a5
loadfp 32, insn cflwsp, n=64, off=6, cmp=ffffffff69788796
loadfp 32, insn cflwsp, n=64, off=7, cmp=ffffffff5a697887
loadfp 32, insn cflwsp, n=64, off=9, cmp=ffffffff3c4b5a69
loadfp 32, insn cflwsp, n=64, off=10, cmp=ffffffff2d3c4b5a
loadfp 32, insn cflwsp, n=64, off=11, cmp=ffffffff1e2d3c4b
<no failure>
---
tests/ldst.S | 134 +++++++++++++++++++++++++++
tests/ldst.h | 170 +++++++++++++++++++++++++++++++++++
tests/test-misaligned-ldst.c | 154 +++++++++++++++++++++++++++++++
3 files changed, 458 insertions(+)
create mode 100644 tests/ldst.S
create mode 100644 tests/ldst.h
create mode 100644 tests/test-misaligned-ldst.c
diff --git a/tests/ldst.S b/tests/ldst.S
new file mode 100644
index 00000000..66f88e42
--- /dev/null
+++ b/tests/ldst.S
@@ -0,0 +1,134 @@
+.altmacro
+
+.macro mem_repeat name:req, ldst:req, op:req, rx:req, ry:req, len:req, signext=0, step=1, n=4096, max=2048
+.globl mem_\name
+.type mem_\name, @function
+mem_\name\():
+.set i, 0
+.rept \n
+.set j, \max - \n * \step + i * \step
+1:
+ mem_\ldst \op \rx \ry j
+ ret
+.ifne . - mem_\name - (. - 1b) * (i + 1)
+.error "mem_\name is not aligned"
+.endif
+.set i, i + 1
+.endr
+.size mem_\name, . - mem_\name
+
+.align 3
+.globl mem_\name\()_desc
+.type mem_\name\()_desc @object
+mem_\name\()_desc:
+ .byte \len, \signext, \step, (. - mem_\name) / \n
+ .half \max - \n * \step, \n
+#if __riscv_xlen == 32
+ .word mem_\name
+#elif __riscv_xlen == 64
+ .dword mem_\name
+#endif
+.size mem_\name\()_desc, . - mem_\name\()_desc
+.endm
+
+.macro mem_load op rs rd imm
+ mv \rs, a0
+ \op \rd, \imm(\rs)
+ mv a0, \rd
+.endm
+
+.macro mem_fpld op rs rd imm
+ mv \rs, a0
+ \op \rd, \imm(\rs)
+ fmv.d fa0, \rd
+.endm
+
+.macro mem_store op rs1 rs2 imm
+ mv \rs2, a1
+ mv \rs1, a0
+ \op \rs2, \imm(\rs1)
+.endm
+
+.macro mem_fpst op rs1 rs2 imm
+ fmv.d \rs2, fa0
+ mv \rs1, a0
+ \op \rs2, \imm(\rs1)
+.endm
+
+.macro mem_ldsp op tmp rd imm
+ mv \tmp, sp
+ mv sp, a0
+ \op \rd, \imm(sp)
+ mv sp, \tmp
+ mv a0, \rd
+.endm
+
+.macro mem_fpldsp op tmp rd imm
+ mv \tmp, sp
+ mv sp, a0
+ \op \rd, \imm(sp)
+ mv sp, \tmp
+ fmv.d fa0, \rd
+.endm
+
+.macro mem_stsp op tmp rs2 imm
+ mv \tmp, sp
+ mv sp, a0
+ mv \rs2, a1
+ \op \rs2, \imm(sp)
+ mv sp, \tmp
+.endm
+
+.macro mem_fpstsp op tmp rs2 imm
+ mv \tmp, sp
+ mv sp, a0
+ fmv.d \rs2, fa0
+ \op \rs2, \imm(sp)
+ mv sp, \tmp
+.endm
+
+mem_repeat lb load lb t0 a1 1 1
+mem_repeat lbu load lbu t1 a2 1
+mem_repeat sb store sb t2 a3 1
+mem_repeat lh load lh t4 a5 2 1
+mem_repeat lhu load lhu t5 a6 2
+mem_repeat sh store sh t6 a7 2
+mem_repeat lw load lw a7 t6 4 1
+#if __riscv_xlen == 64
+mem_repeat lwu load lwu a6 t5 4
+#endif
+mem_repeat sw store sw a5 t4 4
+#if __riscv_xlen == 64
+mem_repeat ld load ld a4 t3 8 1
+mem_repeat sd store sd a3 t2 8
+#endif
+mem_repeat flw fpld flw t3 ft8 4
+mem_repeat fsw fpst fsw t4 ft9 4
+mem_repeat fld fpld fld t5 ft10 8
+mem_repeat fsd fpst fsd t6 ft11 8
+#ifdef __riscv_zcb
+mem_repeat clbu load c.lbu a5 a1 1 0 1 4 4
+mem_repeat csb store c.sb a4 a2 1 0 1 4 4
+mem_repeat clh load c.lh a3 a3 2 1 2 2 4
+mem_repeat clhu load c.lhu a2 a4 2 0 2 2 4
+mem_repeat csh store c.sh a1 a5 2 0 2 2 4
+#endif
+mem_repeat clw load c.lw a5 a1 4 1 4 32 128
+mem_repeat csw store c.sw a4 a2 4 0 4 32 128
+mem_repeat clwsp ldsp lw t0 t6 4 1 4 64 256
+mem_repeat cswsp stsp sw t1 t5 4 0 4 64 256
+mem_repeat cfld fpld c.fld a3 fa2 8 0 8 32 256
+mem_repeat cfsd fpst c.fsd a2 fa3 8 0 8 32 256
+mem_repeat cfldsp fpldsp fld t2 ft10 8 0 8 64 512
+mem_repeat cfsdsp fpstsp fsd t3 ft11 8 0 8 64 512
+#if __riscv_xlen == 32
+mem_repeat cflw fpld c.flw a5 fa4 4 0 4 32 128
+mem_repeat cfsw fpst c.fsw a4 fa5 4 0 4 32 128
+mem_repeat cflwsp fpldsp flw t3 ft11 4 0 4 64 256
+mem_repeat cfswsp fpstsp fsw t4 ft10 4 0 4 64 256
+#elif __riscv_xlen == 64
+mem_repeat cld load c.ld a5 a4 8 1 8 32 256
+mem_repeat csd store c.sd a4 a5 8 0 8 32 256
+mem_repeat cldsp ldsp ld t3 t6 8 1 8 64 512
+mem_repeat csdsp stsp sd t4 t5 8 0 8 64 512
+#endif
diff --git a/tests/ldst.h b/tests/ldst.h
new file mode 100644
index 00000000..3d4b6062
--- /dev/null
+++ b/tests/ldst.h
@@ -0,0 +1,170 @@
+#include <inttypes.h>
+
+typedef struct {
+ uint8_t len;
+ uint8_t signext;
+ uint8_t imm_step;
+ uint8_t isize;
+ int16_t imm_base;
+ uint16_t n;
+ void *fp;
+} mem_op_desc;
+
+typedef union {
+ unsigned long u_long;
+ long i_long;
+ uint32_t u32[2];
+ int32_t i32[2];
+ uint64_t u64;
+ int64_t i64;
+ float f32[2];
+ double f64;
+ uint8_t bytes[8];
+} ldst_val;
+
+extern mem_op_desc mem_lb_desc;
+extern mem_op_desc mem_lbu_desc;
+extern mem_op_desc mem_sb_desc;
+extern mem_op_desc mem_lh_desc;
+extern mem_op_desc mem_lhu_desc;
+extern mem_op_desc mem_sh_desc;
+extern mem_op_desc mem_lw_desc;
+extern mem_op_desc mem_lwu_desc;
+extern mem_op_desc mem_sw_desc;
+extern mem_op_desc mem_ld_desc;
+extern mem_op_desc mem_sd_desc;
+extern mem_op_desc mem_flw_desc;
+extern mem_op_desc mem_fsw_desc;
+extern mem_op_desc mem_fld_desc;
+extern mem_op_desc mem_fsd_desc;
+extern mem_op_desc mem_clbu_desc;
+extern mem_op_desc mem_csb_desc;
+extern mem_op_desc mem_clh_desc;
+extern mem_op_desc mem_clhu_desc;
+extern mem_op_desc mem_csh_desc;
+extern mem_op_desc mem_clw_desc;
+extern mem_op_desc mem_csw_desc;
+extern mem_op_desc mem_clwsp_desc;
+extern mem_op_desc mem_cswsp_desc;
+extern mem_op_desc mem_cfld_desc;
+extern mem_op_desc mem_cfsd_desc;
+extern mem_op_desc mem_cfldsp_desc;
+extern mem_op_desc mem_cfsdsp_desc;
+extern mem_op_desc mem_cflw_desc;
+extern mem_op_desc mem_cfsw_desc;
+extern mem_op_desc mem_cflwsp_desc;
+extern mem_op_desc mem_cfswsp_desc;
+extern mem_op_desc mem_cld_desc;
+extern mem_op_desc mem_csd_desc;
+extern mem_op_desc mem_cldsp_desc;
+extern mem_op_desc mem_csdsp_desc;
+
+typedef struct ldst_func {
+ const char *name;
+ const mem_op_desc *desc;
+} ldst_func;
+
+#define DEF_LDST_FUNC(x) { #x, &mem_ ## x ## _desc }
+
+static const ldst_func load_funcs[] = {
+ DEF_LDST_FUNC(lb),
+ DEF_LDST_FUNC(lbu),
+ DEF_LDST_FUNC(lh),
+ DEF_LDST_FUNC(lhu),
+ DEF_LDST_FUNC(lw),
+#ifdef __riscv_zcb
+ DEF_LDST_FUNC(clbu),
+ DEF_LDST_FUNC(clh),
+ DEF_LDST_FUNC(clhu),
+#endif
+ DEF_LDST_FUNC(clw),
+ DEF_LDST_FUNC(clwsp),
+#if __riscv_xlen == 64
+ DEF_LDST_FUNC(lwu),
+ DEF_LDST_FUNC(ld),
+ DEF_LDST_FUNC(cld),
+ DEF_LDST_FUNC(cldsp),
+#endif
+};
+
+static const ldst_func store_funcs[] = {
+ DEF_LDST_FUNC(sb),
+ DEF_LDST_FUNC(sh),
+ DEF_LDST_FUNC(sw),
+#ifdef __riscv_zcb
+ DEF_LDST_FUNC(csb),
+ DEF_LDST_FUNC(csh),
+#endif
+ DEF_LDST_FUNC(csw),
+ DEF_LDST_FUNC(cswsp),
+#if __riscv_xlen == 64
+ DEF_LDST_FUNC(sd),
+ DEF_LDST_FUNC(csd),
+ DEF_LDST_FUNC(csdsp),
+#endif
+};
+
+static const ldst_func loadfp_funcs[] = {
+#if __riscv_xlen == 32
+ DEF_LDST_FUNC(cflw),
+ DEF_LDST_FUNC(cflwsp),
+#endif
+ DEF_LDST_FUNC(flw),
+ DEF_LDST_FUNC(fld),
+ DEF_LDST_FUNC(cfld),
+ DEF_LDST_FUNC(cfldsp),
+};
+
+static const ldst_func storefp_funcs[] = {
+#if __riscv_xlen == 32
+ DEF_LDST_FUNC(cfsw),
+ DEF_LDST_FUNC(cfswsp),
+#endif
+ DEF_LDST_FUNC(fsw),
+ DEF_LDST_FUNC(fsd),
+ DEF_LDST_FUNC(cfsd),
+ DEF_LDST_FUNC(cfsdsp),
+};
+
+static inline unsigned long load_i(const void *p, unsigned func, unsigned sel)
+{
+ typedef unsigned long (*func_i)(const void *p);
+ const mem_op_desc *desc = load_funcs[func].desc;
+
+ long imm = (long)desc->imm_base + desc->imm_step * sel;
+ func_i f = desc->fp + desc->isize * sel;
+
+ return f(p - imm);
+}
+
+static inline void store_i(void *p, unsigned long val, unsigned func, unsigned sel)
+{
+ typedef void (*func_i)(void *p, unsigned long val);
+ const mem_op_desc *desc = store_funcs[func].desc;
+
+ long imm = (long)desc->imm_base + desc->imm_step * sel;
+ func_i f = desc->fp + desc->isize * sel;
+ f(p - imm, val);
+}
+
+static inline double load_f(const void *p, unsigned func, unsigned sel)
+{
+ typedef double (*func_f)(const void *p);
+ const mem_op_desc *desc = loadfp_funcs[func].desc;
+
+ long imm = (long)desc->imm_base + desc->imm_step * sel;
+ func_f f = desc->fp + desc->isize * sel;
+
+ return f(p - imm);
+}
+
+static inline void store_f(void *p, double val, unsigned func, unsigned sel)
+{
+ typedef void (*func_f)(void *p, double val);
+ const mem_op_desc *desc = storefp_funcs[func].desc;
+
+ long imm = (long)desc->imm_base + desc->imm_step * sel;
+ func_f f = desc->fp + desc->isize * sel;
+
+ f(p - imm, val);
+}
diff --git a/tests/test-misaligned-ldst.c b/tests/test-misaligned-ldst.c
new file mode 100644
index 00000000..ccbd0d36
--- /dev/null
+++ b/tests/test-misaligned-ldst.c
@@ -0,0 +1,154 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+#include <error.h>
+#include "ldst.h"
+
+#define ARR_SIZE(arr) (sizeof(arr) / sizeof(arr[0]))
+
+static const uint8_t patt[16] = {
+ 0xf0, 0xe1, 0xd2, 0xc3, 0xb4, 0xa5, 0x96, 0x87,
+ 0x78, 0x69, 0x5a, 0x4b, 0x3c, 0x2d, 0x1e, 0x0f,
+};
+
+static const uint64_t load_val[16] = {
+ 0x8796a5b4c3d2e1f0ULL,
+ 0x788796a5b4c3d2e1ULL,
+ 0x69788796a5b4c3d2ULL,
+ 0x5a69788796a5b4c3ULL,
+ 0x4b5a69788796a5b4ULL,
+ 0x3c4b5a69788796a5ULL,
+ 0x2d3c4b5a69788796ULL,
+ 0x1e2d3c4b5a697887ULL,
+ 0x0f1e2d3c4b5a6978ULL,
+ 0x0f1e2d3c4b5a69ULL,
+ 0x0f1e2d3c4b5aULL,
+ 0x0f1e2d3c4bULL,
+ 0x0f1e2d3cULL,
+ 0x0f1e2dULL,
+ 0x0f1eULL,
+ 0x0fULL,
+};
+
+int main()
+{
+ unsigned i, j, k;
+ //ldst_val val;
+
+ // Test int read
+ for (j = 0; j < ARR_SIZE(load_funcs); ++j) {
+ const mem_op_desc *desc = load_funcs[j].desc;
+ unsigned shift = 8 * (sizeof(long) - desc->len);
+
+ for (i = 0; i < ARR_SIZE(patt) - desc->len; ++i) {
+ unsigned long expected;
+ if (desc->len != 1 && i % desc->len == 0)
+ continue;
+
+ expected = load_val[i];
+ if (desc->signext)
+ expected = (long)(expected << shift) >> shift;
+ else
+ expected = (expected << shift) >> shift;
+
+ printf("load %c%u, insn %s, n=%u, off=%u, cmp=%lx\n",
+ desc->signext ? 'i' : 'u',
+ desc->len * 8, load_funcs[j].name, desc->n, i, expected);
+ fflush(stdout);
+ for (k = 0; k < desc->n; ++k) {
+ unsigned long read = load_i(&patt[i], j, k);
+ if (read != expected)
+ error(1, 0, "failed at load_i %u %u %u: %lx",
+ i, j, k, read);
+ }
+ }
+ }
+ // Test fp load
+ for (j = 0; j < ARR_SIZE(loadfp_funcs); ++j) {
+ const mem_op_desc *desc = loadfp_funcs[j].desc;
+
+ for (i = 0; i < ARR_SIZE(patt) - desc->len; ++i) {
+ ldst_val expected, read;
+
+ expected.u64 = load_val[i];
+ if (desc->len == 4) // float
+ expected.i32[1] = -1;
+
+ if (i % desc->len == 0)
+ continue;
+
+ printf("loadfp %u, insn %s, n=%u, off=%u, cmp=%" PRIx64 "\n",
+ desc->len * 8, loadfp_funcs[j].name, desc->n, i, expected.u64);
+ fflush(stdout);
+ for (k = 0; k < desc->n; ++k) {
+ read.f64 = load_f(&patt[i], j, k);
+ if (read.u64 != expected.u64)
+ error(1, 0, "failed at load_f %u %u %u: %" PRIx64,
+ i, j, k, read.u64);
+ }
+ }
+ }
+ // Test int store
+ for (j = 0; j < ARR_SIZE(store_funcs); ++j) {
+ const mem_op_desc *desc = store_funcs[j].desc;
+
+ for (i = 0; i < ARR_SIZE(patt) - desc->len; ++i) {
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Woverflow"
+ ldst_val val = { 0x8899aabbccddeeffULL };
+#pragma GCC diagnostic pop
+ unsigned end = i + desc->len;
+
+ if (desc->len != 1 && i % desc->len == 0)
+ continue;
+
+ memcpy(val.bytes, &patt[i], desc->len);
+ printf("store %u, insn %s, n=%u, off=%u, src=%lx\n",
+ desc->len * 8, store_funcs[j].name, desc->n, i, val.u_long);
+ fflush(stdout);
+
+ for (k = 0; k < desc->n; ++k) {
+ uint8_t buff[ARR_SIZE(patt)] = {};
+
+ memcpy(buff, patt, i);
+ memcpy(&buff[end], &patt[end], ARR_SIZE(patt) - end);
+ store_i(&buff[i], val.u_long, j, k);
+ if (memcmp(buff, patt, sizeof(buff)))
+ error(1, 0, "faild at store_i %u %u %u", i, j, k);
+ }
+
+ }
+ }
+ // Test fp store
+ for (j = 0; j < ARR_SIZE(storefp_funcs); ++j) {
+ const mem_op_desc *desc = storefp_funcs[j].desc;
+
+ for (i = 0; i < ARR_SIZE(patt) - desc->len; ++i) {
+ ldst_val val;
+ unsigned end = i + desc->len;
+
+ val.u32[0] = 0xccddeeffUL;
+ val.u32[1] = 0x8899aabbUL;
+
+ if (i % desc->len == 0)
+ continue;
+
+ memcpy(val.bytes, &patt[i], desc->len);
+ printf("storefp %u, insn %s, n=%u, off=%u, src=%" PRIx64 "\n",
+ desc->len * 8, storefp_funcs[j].name, desc->n, i, val.u64);
+ fflush(stdout);
+
+ for (k = 0; k < desc->n; ++k) {
+ uint8_t buff[ARR_SIZE(patt)] = {};
+
+ memcpy(buff, patt, i);
+ memcpy(&buff[end], &patt[end], ARR_SIZE(patt) - end);
+ store_f(&buff[i], val.f64, j, k);
+ if (memcmp(buff, patt, sizeof(buff)))
+ error(1, 0, "faild at store_f %u %u %u", i, j, k);
+ }
+ }
+ }
+ return 0;
+}
--
2.34.1
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 8/8] [NOT-FOR-UPSTREAM] Test program for misaligned load/store
2026-06-05 11:32 ` [PATCH v2 8/8] [NOT-FOR-UPSTREAM] Test program for misaligned load/store Bo Gan
@ 2026-06-16 8:23 ` Anup Patel
0 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2026-06-16 8:23 UTC (permalink / raw)
To: Bo Gan
Cc: opensbi, wangruikang, dramforever, andrew.jones, cleger, pjw,
asrinivasan
On Fri, Jun 5, 2026 at 8:43 PM Bo Gan <ganboing@gmail.com> wrote:
>
> Build: gcc -o test-misaligned-ldst test-misaligned-ldst.c ldst.S
>
> Failure observed before the fix on QEMU with 64-bit Linux and 32-bit test
>
> ...
> loadfp 64, insn cfldsp, n=64, off=1, cmp=788796a5b4c3d2e1
> loadfp 64, insn cfldsp, n=64, off=2, cmp=69788796a5b4c3d2
> loadfp 64, insn cfldsp, n=64, off=3, cmp=5a69788796a5b4c3
> loadfp 64, insn cfldsp, n=64, off=4, cmp=4b5a69788796a5b4
> loadfp 64, insn cfldsp, n=64, off=5, cmp=3c4b5a69788796a5
> loadfp 64, insn cfldsp, n=64, off=6, cmp=2d3c4b5a69788796
> loadfp 64, insn cfldsp, n=64, off=7, cmp=1e2d3c4b5a697887
> loadfp 32, insn cflw, n=32, off=1, cmp=ffffffffb4c3d2e1
> ./test32: failed at load_f 1 4 0: 0
>
> With the patch series:
>
> ...
> loadfp 64, insn cfldsp, n=64, off=1, cmp=788796a5b4c3d2e1
> loadfp 64, insn cfldsp, n=64, off=2, cmp=69788796a5b4c3d2
> loadfp 64, insn cfldsp, n=64, off=3, cmp=5a69788796a5b4c3
> loadfp 64, insn cfldsp, n=64, off=4, cmp=4b5a69788796a5b4
> loadfp 64, insn cfldsp, n=64, off=5, cmp=3c4b5a69788796a5
> loadfp 64, insn cfldsp, n=64, off=6, cmp=2d3c4b5a69788796
> loadfp 64, insn cfldsp, n=64, off=7, cmp=1e2d3c4b5a697887
> loadfp 32, insn cflw, n=32, off=1, cmp=ffffffffb4c3d2e1
> loadfp 32, insn cflw, n=32, off=2, cmp=ffffffffa5b4c3d2
> loadfp 32, insn cflw, n=32, off=3, cmp=ffffffff96a5b4c3
> loadfp 32, insn cflw, n=32, off=5, cmp=ffffffff788796a5
> loadfp 32, insn cflw, n=32, off=6, cmp=ffffffff69788796
> loadfp 32, insn cflw, n=32, off=7, cmp=ffffffff5a697887
> loadfp 32, insn cflw, n=32, off=9, cmp=ffffffff3c4b5a69
> loadfp 32, insn cflw, n=32, off=10, cmp=ffffffff2d3c4b5a
> loadfp 32, insn cflw, n=32, off=11, cmp=ffffffff1e2d3c4b
> loadfp 32, insn cflwsp, n=64, off=1, cmp=ffffffffb4c3d2e1
> loadfp 32, insn cflwsp, n=64, off=2, cmp=ffffffffa5b4c3d2
> loadfp 32, insn cflwsp, n=64, off=3, cmp=ffffffff96a5b4c3
> loadfp 32, insn cflwsp, n=64, off=5, cmp=ffffffff788796a5
> loadfp 32, insn cflwsp, n=64, off=6, cmp=ffffffff69788796
> loadfp 32, insn cflwsp, n=64, off=7, cmp=ffffffff5a697887
> loadfp 32, insn cflwsp, n=64, off=9, cmp=ffffffff3c4b5a69
> loadfp 32, insn cflwsp, n=64, off=10, cmp=ffffffff2d3c4b5a
> loadfp 32, insn cflwsp, n=64, off=11, cmp=ffffffff1e2d3c4b
>
> <no failure>
> ---
> tests/ldst.S | 134 +++++++++++++++++++++++++++
> tests/ldst.h | 170 +++++++++++++++++++++++++++++++++++
> tests/test-misaligned-ldst.c | 154 +++++++++++++++++++++++++++++++
> 3 files changed, 458 insertions(+)
> create mode 100644 tests/ldst.S
> create mode 100644 tests/ldst.h
> create mode 100644 tests/test-misaligned-ldst.c
Instead of test-misaligned-ldst being user-space test, I
suggest adding this as another payload bare-metal app
under firmware/payloads and we can use FW_PAYLOAD_PATH
to select this bare-metal app instead of default test.bin.
Regards,
Anup
>
> diff --git a/tests/ldst.S b/tests/ldst.S
> new file mode 100644
> index 00000000..66f88e42
> --- /dev/null
> +++ b/tests/ldst.S
> @@ -0,0 +1,134 @@
> +.altmacro
> +
> +.macro mem_repeat name:req, ldst:req, op:req, rx:req, ry:req, len:req, signext=0, step=1, n=4096, max=2048
> +.globl mem_\name
> +.type mem_\name, @function
> +mem_\name\():
> +.set i, 0
> +.rept \n
> +.set j, \max - \n * \step + i * \step
> +1:
> + mem_\ldst \op \rx \ry j
> + ret
> +.ifne . - mem_\name - (. - 1b) * (i + 1)
> +.error "mem_\name is not aligned"
> +.endif
> +.set i, i + 1
> +.endr
> +.size mem_\name, . - mem_\name
> +
> +.align 3
> +.globl mem_\name\()_desc
> +.type mem_\name\()_desc @object
> +mem_\name\()_desc:
> + .byte \len, \signext, \step, (. - mem_\name) / \n
> + .half \max - \n * \step, \n
> +#if __riscv_xlen == 32
> + .word mem_\name
> +#elif __riscv_xlen == 64
> + .dword mem_\name
> +#endif
> +.size mem_\name\()_desc, . - mem_\name\()_desc
> +.endm
> +
> +.macro mem_load op rs rd imm
> + mv \rs, a0
> + \op \rd, \imm(\rs)
> + mv a0, \rd
> +.endm
> +
> +.macro mem_fpld op rs rd imm
> + mv \rs, a0
> + \op \rd, \imm(\rs)
> + fmv.d fa0, \rd
> +.endm
> +
> +.macro mem_store op rs1 rs2 imm
> + mv \rs2, a1
> + mv \rs1, a0
> + \op \rs2, \imm(\rs1)
> +.endm
> +
> +.macro mem_fpst op rs1 rs2 imm
> + fmv.d \rs2, fa0
> + mv \rs1, a0
> + \op \rs2, \imm(\rs1)
> +.endm
> +
> +.macro mem_ldsp op tmp rd imm
> + mv \tmp, sp
> + mv sp, a0
> + \op \rd, \imm(sp)
> + mv sp, \tmp
> + mv a0, \rd
> +.endm
> +
> +.macro mem_fpldsp op tmp rd imm
> + mv \tmp, sp
> + mv sp, a0
> + \op \rd, \imm(sp)
> + mv sp, \tmp
> + fmv.d fa0, \rd
> +.endm
> +
> +.macro mem_stsp op tmp rs2 imm
> + mv \tmp, sp
> + mv sp, a0
> + mv \rs2, a1
> + \op \rs2, \imm(sp)
> + mv sp, \tmp
> +.endm
> +
> +.macro mem_fpstsp op tmp rs2 imm
> + mv \tmp, sp
> + mv sp, a0
> + fmv.d \rs2, fa0
> + \op \rs2, \imm(sp)
> + mv sp, \tmp
> +.endm
> +
> +mem_repeat lb load lb t0 a1 1 1
> +mem_repeat lbu load lbu t1 a2 1
> +mem_repeat sb store sb t2 a3 1
> +mem_repeat lh load lh t4 a5 2 1
> +mem_repeat lhu load lhu t5 a6 2
> +mem_repeat sh store sh t6 a7 2
> +mem_repeat lw load lw a7 t6 4 1
> +#if __riscv_xlen == 64
> +mem_repeat lwu load lwu a6 t5 4
> +#endif
> +mem_repeat sw store sw a5 t4 4
> +#if __riscv_xlen == 64
> +mem_repeat ld load ld a4 t3 8 1
> +mem_repeat sd store sd a3 t2 8
> +#endif
> +mem_repeat flw fpld flw t3 ft8 4
> +mem_repeat fsw fpst fsw t4 ft9 4
> +mem_repeat fld fpld fld t5 ft10 8
> +mem_repeat fsd fpst fsd t6 ft11 8
> +#ifdef __riscv_zcb
> +mem_repeat clbu load c.lbu a5 a1 1 0 1 4 4
> +mem_repeat csb store c.sb a4 a2 1 0 1 4 4
> +mem_repeat clh load c.lh a3 a3 2 1 2 2 4
> +mem_repeat clhu load c.lhu a2 a4 2 0 2 2 4
> +mem_repeat csh store c.sh a1 a5 2 0 2 2 4
> +#endif
> +mem_repeat clw load c.lw a5 a1 4 1 4 32 128
> +mem_repeat csw store c.sw a4 a2 4 0 4 32 128
> +mem_repeat clwsp ldsp lw t0 t6 4 1 4 64 256
> +mem_repeat cswsp stsp sw t1 t5 4 0 4 64 256
> +mem_repeat cfld fpld c.fld a3 fa2 8 0 8 32 256
> +mem_repeat cfsd fpst c.fsd a2 fa3 8 0 8 32 256
> +mem_repeat cfldsp fpldsp fld t2 ft10 8 0 8 64 512
> +mem_repeat cfsdsp fpstsp fsd t3 ft11 8 0 8 64 512
> +#if __riscv_xlen == 32
> +mem_repeat cflw fpld c.flw a5 fa4 4 0 4 32 128
> +mem_repeat cfsw fpst c.fsw a4 fa5 4 0 4 32 128
> +mem_repeat cflwsp fpldsp flw t3 ft11 4 0 4 64 256
> +mem_repeat cfswsp fpstsp fsw t4 ft10 4 0 4 64 256
> +#elif __riscv_xlen == 64
> +mem_repeat cld load c.ld a5 a4 8 1 8 32 256
> +mem_repeat csd store c.sd a4 a5 8 0 8 32 256
> +mem_repeat cldsp ldsp ld t3 t6 8 1 8 64 512
> +mem_repeat csdsp stsp sd t4 t5 8 0 8 64 512
> +#endif
> diff --git a/tests/ldst.h b/tests/ldst.h
> new file mode 100644
> index 00000000..3d4b6062
> --- /dev/null
> +++ b/tests/ldst.h
> @@ -0,0 +1,170 @@
> +#include <inttypes.h>
> +
> +typedef struct {
> + uint8_t len;
> + uint8_t signext;
> + uint8_t imm_step;
> + uint8_t isize;
> + int16_t imm_base;
> + uint16_t n;
> + void *fp;
> +} mem_op_desc;
> +
> +typedef union {
> + unsigned long u_long;
> + long i_long;
> + uint32_t u32[2];
> + int32_t i32[2];
> + uint64_t u64;
> + int64_t i64;
> + float f32[2];
> + double f64;
> + uint8_t bytes[8];
> +} ldst_val;
> +
> +extern mem_op_desc mem_lb_desc;
> +extern mem_op_desc mem_lbu_desc;
> +extern mem_op_desc mem_sb_desc;
> +extern mem_op_desc mem_lh_desc;
> +extern mem_op_desc mem_lhu_desc;
> +extern mem_op_desc mem_sh_desc;
> +extern mem_op_desc mem_lw_desc;
> +extern mem_op_desc mem_lwu_desc;
> +extern mem_op_desc mem_sw_desc;
> +extern mem_op_desc mem_ld_desc;
> +extern mem_op_desc mem_sd_desc;
> +extern mem_op_desc mem_flw_desc;
> +extern mem_op_desc mem_fsw_desc;
> +extern mem_op_desc mem_fld_desc;
> +extern mem_op_desc mem_fsd_desc;
> +extern mem_op_desc mem_clbu_desc;
> +extern mem_op_desc mem_csb_desc;
> +extern mem_op_desc mem_clh_desc;
> +extern mem_op_desc mem_clhu_desc;
> +extern mem_op_desc mem_csh_desc;
> +extern mem_op_desc mem_clw_desc;
> +extern mem_op_desc mem_csw_desc;
> +extern mem_op_desc mem_clwsp_desc;
> +extern mem_op_desc mem_cswsp_desc;
> +extern mem_op_desc mem_cfld_desc;
> +extern mem_op_desc mem_cfsd_desc;
> +extern mem_op_desc mem_cfldsp_desc;
> +extern mem_op_desc mem_cfsdsp_desc;
> +extern mem_op_desc mem_cflw_desc;
> +extern mem_op_desc mem_cfsw_desc;
> +extern mem_op_desc mem_cflwsp_desc;
> +extern mem_op_desc mem_cfswsp_desc;
> +extern mem_op_desc mem_cld_desc;
> +extern mem_op_desc mem_csd_desc;
> +extern mem_op_desc mem_cldsp_desc;
> +extern mem_op_desc mem_csdsp_desc;
> +
> +typedef struct ldst_func {
> + const char *name;
> + const mem_op_desc *desc;
> +} ldst_func;
> +
> +#define DEF_LDST_FUNC(x) { #x, &mem_ ## x ## _desc }
> +
> +static const ldst_func load_funcs[] = {
> + DEF_LDST_FUNC(lb),
> + DEF_LDST_FUNC(lbu),
> + DEF_LDST_FUNC(lh),
> + DEF_LDST_FUNC(lhu),
> + DEF_LDST_FUNC(lw),
> +#ifdef __riscv_zcb
> + DEF_LDST_FUNC(clbu),
> + DEF_LDST_FUNC(clh),
> + DEF_LDST_FUNC(clhu),
> +#endif
> + DEF_LDST_FUNC(clw),
> + DEF_LDST_FUNC(clwsp),
> +#if __riscv_xlen == 64
> + DEF_LDST_FUNC(lwu),
> + DEF_LDST_FUNC(ld),
> + DEF_LDST_FUNC(cld),
> + DEF_LDST_FUNC(cldsp),
> +#endif
> +};
> +
> +static const ldst_func store_funcs[] = {
> + DEF_LDST_FUNC(sb),
> + DEF_LDST_FUNC(sh),
> + DEF_LDST_FUNC(sw),
> +#ifdef __riscv_zcb
> + DEF_LDST_FUNC(csb),
> + DEF_LDST_FUNC(csh),
> +#endif
> + DEF_LDST_FUNC(csw),
> + DEF_LDST_FUNC(cswsp),
> +#if __riscv_xlen == 64
> + DEF_LDST_FUNC(sd),
> + DEF_LDST_FUNC(csd),
> + DEF_LDST_FUNC(csdsp),
> +#endif
> +};
> +
> +static const ldst_func loadfp_funcs[] = {
> +#if __riscv_xlen == 32
> + DEF_LDST_FUNC(cflw),
> + DEF_LDST_FUNC(cflwsp),
> +#endif
> + DEF_LDST_FUNC(flw),
> + DEF_LDST_FUNC(fld),
> + DEF_LDST_FUNC(cfld),
> + DEF_LDST_FUNC(cfldsp),
> +};
> +
> +static const ldst_func storefp_funcs[] = {
> +#if __riscv_xlen == 32
> + DEF_LDST_FUNC(cfsw),
> + DEF_LDST_FUNC(cfswsp),
> +#endif
> + DEF_LDST_FUNC(fsw),
> + DEF_LDST_FUNC(fsd),
> + DEF_LDST_FUNC(cfsd),
> + DEF_LDST_FUNC(cfsdsp),
> +};
> +
> +static inline unsigned long load_i(const void *p, unsigned func, unsigned sel)
> +{
> + typedef unsigned long (*func_i)(const void *p);
> + const mem_op_desc *desc = load_funcs[func].desc;
> +
> + long imm = (long)desc->imm_base + desc->imm_step * sel;
> + func_i f = desc->fp + desc->isize * sel;
> +
> + return f(p - imm);
> +}
> +
> +static inline void store_i(void *p, unsigned long val, unsigned func, unsigned sel)
> +{
> + typedef void (*func_i)(void *p, unsigned long val);
> + const mem_op_desc *desc = store_funcs[func].desc;
> +
> + long imm = (long)desc->imm_base + desc->imm_step * sel;
> + func_i f = desc->fp + desc->isize * sel;
> + f(p - imm, val);
> +}
> +
> +static inline double load_f(const void *p, unsigned func, unsigned sel)
> +{
> + typedef double (*func_f)(const void *p);
> + const mem_op_desc *desc = loadfp_funcs[func].desc;
> +
> + long imm = (long)desc->imm_base + desc->imm_step * sel;
> + func_f f = desc->fp + desc->isize * sel;
> +
> + return f(p - imm);
> +}
> +
> +static inline void store_f(void *p, double val, unsigned func, unsigned sel)
> +{
> + typedef void (*func_f)(void *p, double val);
> + const mem_op_desc *desc = storefp_funcs[func].desc;
> +
> + long imm = (long)desc->imm_base + desc->imm_step * sel;
> + func_f f = desc->fp + desc->isize * sel;
> +
> + f(p - imm, val);
> +}
> diff --git a/tests/test-misaligned-ldst.c b/tests/test-misaligned-ldst.c
> new file mode 100644
> index 00000000..ccbd0d36
> --- /dev/null
> +++ b/tests/test-misaligned-ldst.c
> @@ -0,0 +1,154 @@
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <stdint.h>
> +#include <error.h>
> +#include "ldst.h"
> +
> +#define ARR_SIZE(arr) (sizeof(arr) / sizeof(arr[0]))
> +
> +static const uint8_t patt[16] = {
> + 0xf0, 0xe1, 0xd2, 0xc3, 0xb4, 0xa5, 0x96, 0x87,
> + 0x78, 0x69, 0x5a, 0x4b, 0x3c, 0x2d, 0x1e, 0x0f,
> +};
> +
> +static const uint64_t load_val[16] = {
> + 0x8796a5b4c3d2e1f0ULL,
> + 0x788796a5b4c3d2e1ULL,
> + 0x69788796a5b4c3d2ULL,
> + 0x5a69788796a5b4c3ULL,
> + 0x4b5a69788796a5b4ULL,
> + 0x3c4b5a69788796a5ULL,
> + 0x2d3c4b5a69788796ULL,
> + 0x1e2d3c4b5a697887ULL,
> + 0x0f1e2d3c4b5a6978ULL,
> + 0x0f1e2d3c4b5a69ULL,
> + 0x0f1e2d3c4b5aULL,
> + 0x0f1e2d3c4bULL,
> + 0x0f1e2d3cULL,
> + 0x0f1e2dULL,
> + 0x0f1eULL,
> + 0x0fULL,
> +};
> +
> +int main()
> +{
> + unsigned i, j, k;
> + //ldst_val val;
> +
> + // Test int read
> + for (j = 0; j < ARR_SIZE(load_funcs); ++j) {
> + const mem_op_desc *desc = load_funcs[j].desc;
> + unsigned shift = 8 * (sizeof(long) - desc->len);
> +
> + for (i = 0; i < ARR_SIZE(patt) - desc->len; ++i) {
> + unsigned long expected;
> + if (desc->len != 1 && i % desc->len == 0)
> + continue;
> +
> + expected = load_val[i];
> + if (desc->signext)
> + expected = (long)(expected << shift) >> shift;
> + else
> + expected = (expected << shift) >> shift;
> +
> + printf("load %c%u, insn %s, n=%u, off=%u, cmp=%lx\n",
> + desc->signext ? 'i' : 'u',
> + desc->len * 8, load_funcs[j].name, desc->n, i, expected);
> + fflush(stdout);
> + for (k = 0; k < desc->n; ++k) {
> + unsigned long read = load_i(&patt[i], j, k);
> + if (read != expected)
> + error(1, 0, "failed at load_i %u %u %u: %lx",
> + i, j, k, read);
> + }
> + }
> + }
> + // Test fp load
> + for (j = 0; j < ARR_SIZE(loadfp_funcs); ++j) {
> + const mem_op_desc *desc = loadfp_funcs[j].desc;
> +
> + for (i = 0; i < ARR_SIZE(patt) - desc->len; ++i) {
> + ldst_val expected, read;
> +
> + expected.u64 = load_val[i];
> + if (desc->len == 4) // float
> + expected.i32[1] = -1;
> +
> + if (i % desc->len == 0)
> + continue;
> +
> + printf("loadfp %u, insn %s, n=%u, off=%u, cmp=%" PRIx64 "\n",
> + desc->len * 8, loadfp_funcs[j].name, desc->n, i, expected.u64);
> + fflush(stdout);
> + for (k = 0; k < desc->n; ++k) {
> + read.f64 = load_f(&patt[i], j, k);
> + if (read.u64 != expected.u64)
> + error(1, 0, "failed at load_f %u %u %u: %" PRIx64,
> + i, j, k, read.u64);
> + }
> + }
> + }
> + // Test int store
> + for (j = 0; j < ARR_SIZE(store_funcs); ++j) {
> + const mem_op_desc *desc = store_funcs[j].desc;
> +
> + for (i = 0; i < ARR_SIZE(patt) - desc->len; ++i) {
> +#pragma GCC diagnostic push
> +#pragma GCC diagnostic ignored "-Woverflow"
> + ldst_val val = { 0x8899aabbccddeeffULL };
> +#pragma GCC diagnostic pop
> + unsigned end = i + desc->len;
> +
> + if (desc->len != 1 && i % desc->len == 0)
> + continue;
> +
> + memcpy(val.bytes, &patt[i], desc->len);
> + printf("store %u, insn %s, n=%u, off=%u, src=%lx\n",
> + desc->len * 8, store_funcs[j].name, desc->n, i, val.u_long);
> + fflush(stdout);
> +
> + for (k = 0; k < desc->n; ++k) {
> + uint8_t buff[ARR_SIZE(patt)] = {};
> +
> + memcpy(buff, patt, i);
> + memcpy(&buff[end], &patt[end], ARR_SIZE(patt) - end);
> + store_i(&buff[i], val.u_long, j, k);
> + if (memcmp(buff, patt, sizeof(buff)))
> + error(1, 0, "faild at store_i %u %u %u", i, j, k);
> + }
> +
> + }
> + }
> + // Test fp store
> + for (j = 0; j < ARR_SIZE(storefp_funcs); ++j) {
> + const mem_op_desc *desc = storefp_funcs[j].desc;
> +
> + for (i = 0; i < ARR_SIZE(patt) - desc->len; ++i) {
> + ldst_val val;
> + unsigned end = i + desc->len;
> +
> + val.u32[0] = 0xccddeeffUL;
> + val.u32[1] = 0x8899aabbUL;
> +
> + if (i % desc->len == 0)
> + continue;
> +
> + memcpy(val.bytes, &patt[i], desc->len);
> + printf("storefp %u, insn %s, n=%u, off=%u, src=%" PRIx64 "\n",
> + desc->len * 8, storefp_funcs[j].name, desc->n, i, val.u64);
> + fflush(stdout);
> +
> + for (k = 0; k < desc->n; ++k) {
> + uint8_t buff[ARR_SIZE(patt)] = {};
> +
> + memcpy(buff, patt, i);
> + memcpy(&buff[end], &patt[end], ARR_SIZE(patt) - end);
> + store_f(&buff[i], val.f64, j, k);
> + if (memcmp(buff, patt, sizeof(buff)))
> + error(1, 0, "faild at store_f %u %u %u", i, j, k);
> + }
> + }
> + }
> + return 0;
> +}
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/8] Fixes for load/store misaligned and access faults
2026-06-05 11:32 [PATCH v2 0/8] Fixes for load/store misaligned and access faults Bo Gan
` (7 preceding siblings ...)
2026-06-05 11:32 ` [PATCH v2 8/8] [NOT-FOR-UPSTREAM] Test program for misaligned load/store Bo Gan
@ 2026-06-08 18:18 ` Anirudh Srinivasan
2026-06-08 21:42 ` Bo Gan
2026-06-16 8:25 ` Anup Patel
9 siblings, 1 reply; 17+ messages in thread
From: Anirudh Srinivasan @ 2026-06-08 18:18 UTC (permalink / raw)
To: Bo Gan
Cc: opensbi, wangruikang, dramforever, andrew.jones, cleger, pjw,
Michael Ellerman
Hello Bo,
On Fri, Jun 5, 2026 at 6:34 AM Bo Gan <ganboing@gmail.com> wrote:
>
> Re-visit the load/store misaligned and access fault handlers to fix
> issues related to coding patterns, floating-point state, and instruction
> decoding:
I had previously reported here [1] that there were issues booting into
linux after enabling misaligned trap delegation to linux on Sifive
X280. In the discussion over there, we concluding that bumping up the
per hart stack size in opensbi fixed the issue.
This series (without the stack size bump) also seems to fix the issues
that prevented linux from booting. Particularly it was this patch
"lib: sbi: Do not override emulator callback for vector load/store".
But as you say, I still think the stack size bump is needed as I was
able to break the boot by adding some debug prints like this along the
way. I guess this must have somehow caused the stack to overflow?
diff --git a/lib/sbi/sbi_trap_v_ldst.c b/lib/sbi/sbi_trap_v_ldst.c
index 57f12b83..5e596664 100644
--- a/lib/sbi/sbi_trap_v_ldst.c
+++ b/lib/sbi/sbi_trap_v_ldst.c
@@ -16,6 +16,7 @@
#include <sbi/sbi_trap.h>
#include <sbi/sbi_unpriv.h>
#include <sbi/sbi_trap.h>
+#include <sbi/sbi_console.h>
#ifdef OPENSBI_CC_SUPPORT_VECTOR
@@ -139,6 +140,8 @@ static inline void vsetvl(ulong vl, ulong vtype)
int sbi_misaligned_v_ld_emulator(ulong insn, struct sbi_trap_context *tcntx)
{
+ sbi_printf("%s: insn=0x%lx mepc=0x%lx mtval=0x%lx\n",
+ __func__, insn, tcntx->regs.mepc, tcntx->trap.tval);
const struct sbi_trap_info *orig_trap = &tcntx->trap;
struct sbi_trap_regs *regs = &tcntx->regs;
struct sbi_trap_info uptrap;
@@ -238,6 +241,8 @@ int sbi_misaligned_v_ld_emulator(ulong insn,
struct sbi_trap_context *tcntx)
int sbi_misaligned_v_st_emulator(ulong insn, struct sbi_trap_context *tcntx)
{
+ sbi_printf("%s: insn=0x%lx mepc=0x%lx mtval=0x%lx\n",
+ __func__, insn, tcntx->regs.mepc, tcntx->trap.tval);
const struct sbi_trap_info *orig_trap = &tcntx->trap;
struct sbi_trap_regs *regs = &tcntx->regs;
struct sbi_trap_info uptrap;
[ 0.075576] clocksource: jiffies: mask: 0xffffffff max_cycles:
0xffffffff, max_idle_ns: 7645041785100000 @Z0@Υ0@1] posix`Z0@Υ0@s:
2048 T-0[0@�[0@A0@%�Z0@��0@%�Z0@Υ0@B,
linea[0@Υ0@�Z0@ڤ0@T-0[0@ڤ0@T-0@([0@�[0A0@^^�[0@��0@2 KiB
GF�0@�[0@fA0@8.Q������~0@'`fR0@�0@�[0@(�0@`}0@�[0@dit_enab�~0@p}0@�l0@c`�����/0@'`�o�����a@����
@�\0@`����[ 0.155689] cpu1:
Ratio of byte access time to unaligned word access is 0.01, unaligned
accesses are slow
[ 0.155689] cpu3: Ratio of byte access time to unaligned word
access is 0.01, unaligned accesses are slow
[ 0.183733] cpu0: Ratio of byte access time to unaligned word
access is 0.01, unaligned accesses are slow
sbi_misaligned_v_ld_emulator: insn=0x207d007 mepc=0xffffffff80015efc
mtval=0xffff8f8000073d51
sbi_misaligned_v_ld_emulator: insn=0x207d007 mepc=0xffffffff80015efc
mtval=0xffff8f800013bd51
sbi_misaligned_v_ld_emulator: insn=0x207d007 mepc=0xffffffff80015efc
mtval=0xffff8f80000ebd51
sbi_misaligned_v_ld_emulator: insn=0x207d007 mepc=0xffffffff80015efc
mtval=0xffff8f8000113d51
sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
mtval=0xffffaf8001b72003
sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
mtval=0xffffaf8002112003
sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
mtval=0xffffaf800214e003
sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
mtval=0xffffaf8002026003
sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
mtval=0xffffaf8001b70001
sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
mtval=0xffffaf8002110001
sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
mtval=0xffffaf800214c001
sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
mtval=0xffffaf8002024001
sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
mtval=0xffffaf8001b72023
sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
mtval=0xffffaf8002112023
sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
mtval=0xffffaf800214e023
sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
mtval=0xffffaf8002026023
sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
mtval=0xffffaf8001b70021
sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
mtval=0xffffaf8002110021
sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
mtval=0xffffaf800214c021
sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
mtval=0xffffaf8002024021
sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
mtval=0xffffaf8001b72043
sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
mtval=0xffffaf8002112043
sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
mtval=0xffffaf800214e043
sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
mtval=0xffffaf8002026043
sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
mtval=0xffffaf8001b70041
sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
mtval=0xffffaf8002110041
sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
mtval=0xffffaf800214c041
sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
mtval=0xffffaf8002024041
sbi_misaligned_v_l�X0@lS0@�X0@�0@f8001b72Y0@Y0@M0@
�0@��������80021120�0@��������emulatoB0@^b�Z0@
�0@b�0@`Z0@h�M������
������0@�6�Z0@bb^
HL0@B0@
�([0@�Z0@ a��0@"
�b�~0@��0@ator: insn=0x205[0@fA0@
B0@^
A0@[0@sb
Bumping the stack size seems to fix this. If you want me to test
anything else, let me know. Not sure if the tests in PATCH 8 are
appropriate to test on this hw (I think they are for KVM guests
only?).
[1] - https://lore.kernel.org/linux-riscv/nrvt74qnojaubiwjo37ums4lnclu466hovwrhmtbag6f5uhrql@q6msoe2oto4b/
> - 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
>
> --
> 2.34.1
>
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCH v2 0/8] Fixes for load/store misaligned and access faults
2026-06-08 18:18 ` [PATCH v2 0/8] Fixes for load/store misaligned and access faults Anirudh Srinivasan
@ 2026-06-08 21:42 ` Bo Gan
2026-06-10 0:02 ` Anirudh Srinivasan
0 siblings, 1 reply; 17+ messages in thread
From: Bo Gan @ 2026-06-08 21:42 UTC (permalink / raw)
To: Anirudh Srinivasan
Cc: opensbi, wangruikang, dramforever, andrew.jones, cleger, pjw,
Michael Ellerman
Hi Anirudh,
Glad you asked. The stack overflow issue you saw is exactly the same as
Vivian reported. I just sent another series to fix the issue among others:
https://lore.kernel.org/opensbi/20260608211703.571-1-ganboing@gmail.com/T/#t
It optimizes stack usage, so we'll never overflow the stack again, also
being vlen-agnostic. If you could, please help review and validate, as I
don't have any real HW that supports rvv 1.0, except perhaps the quite old
K230, which in itself is a pain to test with. Thanks. I'm hesitant to go
the default stack-size bump route, as it may cause memory space issues on
machines with huge number of cores or tiny embedded ones with limited ram.
BTW, PATCH 8 can be run anywhere, but it's for scalar misaligned ld/st
only. It doesn't test vector load/store.
Bo
On 6/8/26 11:18, Anirudh Srinivasan wrote:
> Hello Bo,
>
>
> On Fri, Jun 5, 2026 at 6:34 AM Bo Gan <ganboing@gmail.com> wrote:
>>
>> Re-visit the load/store misaligned and access fault handlers to fix
>> issues related to coding patterns, floating-point state, and instruction
>> decoding:
>
> I had previously reported here [1] that there were issues booting into
> linux after enabling misaligned trap delegation to linux on Sifive
> X280. In the discussion over there, we concluding that bumping up the
> per hart stack size in opensbi fixed the issue.
>
> This series (without the stack size bump) also seems to fix the issues
> that prevented linux from booting. Particularly it was this patch
> "lib: sbi: Do not override emulator callback for vector load/store".
>
> But as you say, I still think the stack size bump is needed as I was
> able to break the boot by adding some debug prints like this along the
> way. I guess this must have somehow caused the stack to overflow?
>
> diff --git a/lib/sbi/sbi_trap_v_ldst.c b/lib/sbi/sbi_trap_v_ldst.c
> index 57f12b83..5e596664 100644
> --- a/lib/sbi/sbi_trap_v_ldst.c
> +++ b/lib/sbi/sbi_trap_v_ldst.c
> @@ -16,6 +16,7 @@
> #include <sbi/sbi_trap.h>
> #include <sbi/sbi_unpriv.h>
> #include <sbi/sbi_trap.h>
> +#include <sbi/sbi_console.h>
>
> #ifdef OPENSBI_CC_SUPPORT_VECTOR
>
> @@ -139,6 +140,8 @@ static inline void vsetvl(ulong vl, ulong vtype)
>
> int sbi_misaligned_v_ld_emulator(ulong insn, struct sbi_trap_context *tcntx)
> {
> + sbi_printf("%s: insn=0x%lx mepc=0x%lx mtval=0x%lx\n",
> + __func__, insn, tcntx->regs.mepc, tcntx->trap.tval);
> const struct sbi_trap_info *orig_trap = &tcntx->trap;
> struct sbi_trap_regs *regs = &tcntx->regs;
> struct sbi_trap_info uptrap;
> @@ -238,6 +241,8 @@ int sbi_misaligned_v_ld_emulator(ulong insn,
> struct sbi_trap_context *tcntx)
>
> int sbi_misaligned_v_st_emulator(ulong insn, struct sbi_trap_context *tcntx)
> {
> + sbi_printf("%s: insn=0x%lx mepc=0x%lx mtval=0x%lx\n",
> + __func__, insn, tcntx->regs.mepc, tcntx->trap.tval);
> const struct sbi_trap_info *orig_trap = &tcntx->trap;
> struct sbi_trap_regs *regs = &tcntx->regs;
> struct sbi_trap_info uptrap;
>
>
> [ 0.075576] clocksource: jiffies: mask: 0xffffffff max_cycles:
> 0xffffffff, max_idle_ns: 7645041785100000 @Z0@Υ0@1] posix`Z0@Υ0@s:
> 2048 T-0[0@�[0@A0@%�Z0@��0@%�Z0@Υ0@B,
> linea[0@Υ0@�Z0@ڤ0@T-0[0@ڤ0@T-0@([0@�[0A0@^^�[0@��0@2 KiB
> GF�0@�[0@fA0@8.Q������~0@'`fR0@�0@�[0@(�0@`}0@�[0@dit_enab�~0@p}0@�l0@c`�����/0@'`�o�����a@����
>
>
>
>
> @�\0@`����[ 0.155689] cpu1:
> Ratio of byte access time to unaligned word access is 0.01, unaligned
> accesses are slow
> [ 0.155689] cpu3: Ratio of byte access time to unaligned word
> access is 0.01, unaligned accesses are slow
> [ 0.183733] cpu0: Ratio of byte access time to unaligned word
> access is 0.01, unaligned accesses are slow
> sbi_misaligned_v_ld_emulator: insn=0x207d007 mepc=0xffffffff80015efc
> mtval=0xffff8f8000073d51
> sbi_misaligned_v_ld_emulator: insn=0x207d007 mepc=0xffffffff80015efc
> mtval=0xffff8f800013bd51
> sbi_misaligned_v_ld_emulator: insn=0x207d007 mepc=0xffffffff80015efc
> mtval=0xffff8f80000ebd51
> sbi_misaligned_v_ld_emulator: insn=0x207d007 mepc=0xffffffff80015efc
> mtval=0xffff8f8000113d51
> sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> mtval=0xffffaf8001b72003
> sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> mtval=0xffffaf8002112003
> sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> mtval=0xffffaf800214e003
> sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> mtval=0xffffaf8002026003
> sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> mtval=0xffffaf8001b70001
> sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> mtval=0xffffaf8002110001
> sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> mtval=0xffffaf800214c001
> sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> mtval=0xffffaf8002024001
> sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> mtval=0xffffaf8001b72023
> sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> mtval=0xffffaf8002112023
> sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> mtval=0xffffaf800214e023
> sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> mtval=0xffffaf8002026023
> sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> mtval=0xffffaf8001b70021
> sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> mtval=0xffffaf8002110021
> sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> mtval=0xffffaf800214c021
> sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> mtval=0xffffaf8002024021
> sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> mtval=0xffffaf8001b72043
> sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> mtval=0xffffaf8002112043
> sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> mtval=0xffffaf800214e043
> sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> mtval=0xffffaf8002026043
> sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> mtval=0xffffaf8001b70041
> sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> mtval=0xffffaf8002110041
> sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> mtval=0xffffaf800214c041
> sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> mtval=0xffffaf8002024041
> sbi_misaligned_v_l�X0@lS0@�X0@�0@f8001b72Y0@Y0@M0@
>
> �0@��������80021120�0@��������emulatoB0@^b�Z0@
>
> �0@b�0@`Z0@h�M������
>
> ������0@�6�Z0@bb^
>
> HL0@B0@
>
>
> �([0@�Z0@ a��0@"
>
>
> �b�~0@��0@ator: insn=0x205[0@fA0@
>
>
> B0@^
>
>
> A0@[0@sb
>
>
>
> Bumping the stack size seems to fix this. If you want me to test
> anything else, let me know. Not sure if the tests in PATCH 8 are
> appropriate to test on this hw (I think they are for KVM guests
> only?).
>
> [1] - https://lore.kernel.org/linux-riscv/nrvt74qnojaubiwjo37ums4lnclu466hovwrhmtbag6f5uhrql@q6msoe2oto4b/
>
>
>
>
>
>> - 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
>>
>> --
>> 2.34.1
>>
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCH v2 0/8] Fixes for load/store misaligned and access faults
2026-06-08 21:42 ` Bo Gan
@ 2026-06-10 0:02 ` Anirudh Srinivasan
0 siblings, 0 replies; 17+ messages in thread
From: Anirudh Srinivasan @ 2026-06-10 0:02 UTC (permalink / raw)
To: Bo Gan
Cc: opensbi, wangruikang, dramforever, andrew.jones, cleger, pjw,
Michael Ellerman
On Mon, Jun 8, 2026 at 4:36 PM Bo Gan <ganboing@gmail.com> wrote:
>
> Hi Anirudh,
>
> Glad you asked. The stack overflow issue you saw is exactly the same as
> Vivian reported. I just sent another series to fix the issue among others:
>
> https://lore.kernel.org/opensbi/20260608211703.571-1-ganboing@gmail.com/T/#t
>
> It optimizes stack usage, so we'll never overflow the stack again, also
> being vlen-agnostic. If you could, please help review and validate, as I
> don't have any real HW that supports rvv 1.0, except perhaps the quite old
> K230, which in itself is a pain to test with. Thanks. I'm hesitant to go
> the default stack-size bump route, as it may cause memory space issues on
> machines with huge number of cores or tiny embedded ones with limited ram.
I sent some feedback on the other patch. These patches seem to let the
X280 cores boot up fine.
Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com>
> BTW, PATCH 8 can be run anywhere, but it's for scalar misaligned ld/st
> only. It doesn't test vector load/store.
>
> Bo
>
> On 6/8/26 11:18, Anirudh Srinivasan wrote:
> > Hello Bo,
> >
> >
> > On Fri, Jun 5, 2026 at 6:34 AM Bo Gan <ganboing@gmail.com> wrote:
> >>
> >> Re-visit the load/store misaligned and access fault handlers to fix
> >> issues related to coding patterns, floating-point state, and instruction
> >> decoding:
> >
> > I had previously reported here [1] that there were issues booting into
> > linux after enabling misaligned trap delegation to linux on Sifive
> > X280. In the discussion over there, we concluding that bumping up the
> > per hart stack size in opensbi fixed the issue.
> >
> > This series (without the stack size bump) also seems to fix the issues
> > that prevented linux from booting. Particularly it was this patch
> > "lib: sbi: Do not override emulator callback for vector load/store".
> >
> > But as you say, I still think the stack size bump is needed as I was
> > able to break the boot by adding some debug prints like this along the
> > way. I guess this must have somehow caused the stack to overflow?
> >
> > diff --git a/lib/sbi/sbi_trap_v_ldst.c b/lib/sbi/sbi_trap_v_ldst.c
> > index 57f12b83..5e596664 100644
> > --- a/lib/sbi/sbi_trap_v_ldst.c
> > +++ b/lib/sbi/sbi_trap_v_ldst.c
> > @@ -16,6 +16,7 @@
> > #include <sbi/sbi_trap.h>
> > #include <sbi/sbi_unpriv.h>
> > #include <sbi/sbi_trap.h>
> > +#include <sbi/sbi_console.h>
> >
> > #ifdef OPENSBI_CC_SUPPORT_VECTOR
> >
> > @@ -139,6 +140,8 @@ static inline void vsetvl(ulong vl, ulong vtype)
> >
> > int sbi_misaligned_v_ld_emulator(ulong insn, struct sbi_trap_context *tcntx)
> > {
> > + sbi_printf("%s: insn=0x%lx mepc=0x%lx mtval=0x%lx\n",
> > + __func__, insn, tcntx->regs.mepc, tcntx->trap.tval);
> > const struct sbi_trap_info *orig_trap = &tcntx->trap;
> > struct sbi_trap_regs *regs = &tcntx->regs;
> > struct sbi_trap_info uptrap;
> > @@ -238,6 +241,8 @@ int sbi_misaligned_v_ld_emulator(ulong insn,
> > struct sbi_trap_context *tcntx)
> >
> > int sbi_misaligned_v_st_emulator(ulong insn, struct sbi_trap_context *tcntx)
> > {
> > + sbi_printf("%s: insn=0x%lx mepc=0x%lx mtval=0x%lx\n",
> > + __func__, insn, tcntx->regs.mepc, tcntx->trap.tval);
> > const struct sbi_trap_info *orig_trap = &tcntx->trap;
> > struct sbi_trap_regs *regs = &tcntx->regs;
> > struct sbi_trap_info uptrap;
> >
> >
> > [ 0.075576] clocksource: jiffies: mask: 0xffffffff max_cycles:
> > 0xffffffff, max_idle_ns: 7645041785100000 @Z0@Υ0@1] posix`Z0@Υ0@s:
> > 2048 T-0[0@�[0@A0@%�Z0@��0@%�Z0@Υ0@B,
> > linea[0@Υ0@�Z0@ڤ0@T-0[0@ڤ0@T-0@([0@�[0A0@^^�[0@��0@2 KiB
> > GF�0@�[0@fA0@8.Q������~0@'`fR0@�0@�[0@(�0@`}0@�[0@dit_enab�~0@p}0@�l0@c`�����/0@'`�o�����a@����
> >
> >
> >
> >
> > @�\0@`����[ 0.155689] cpu1:
> > Ratio of byte access time to unaligned word access is 0.01, unaligned
> > accesses are slow
> > [ 0.155689] cpu3: Ratio of byte access time to unaligned word
> > access is 0.01, unaligned accesses are slow
> > [ 0.183733] cpu0: Ratio of byte access time to unaligned word
> > access is 0.01, unaligned accesses are slow
> > sbi_misaligned_v_ld_emulator: insn=0x207d007 mepc=0xffffffff80015efc
> > mtval=0xffff8f8000073d51
> > sbi_misaligned_v_ld_emulator: insn=0x207d007 mepc=0xffffffff80015efc
> > mtval=0xffff8f800013bd51
> > sbi_misaligned_v_ld_emulator: insn=0x207d007 mepc=0xffffffff80015efc
> > mtval=0xffff8f80000ebd51
> > sbi_misaligned_v_ld_emulator: insn=0x207d007 mepc=0xffffffff80015efc
> > mtval=0xffff8f8000113d51
> > sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> > mtval=0xffffaf8001b72003
> > sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> > mtval=0xffffaf8002112003
> > sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> > mtval=0xffffaf800214e003
> > sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> > mtval=0xffffaf8002026003
> > sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> > mtval=0xffffaf8001b70001
> > sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> > mtval=0xffffaf8002110001
> > sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> > mtval=0xffffaf800214c001
> > sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> > mtval=0xffffaf8002024001
> > sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> > mtval=0xffffaf8001b72023
> > sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> > mtval=0xffffaf8002112023
> > sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> > mtval=0xffffaf800214e023
> > sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> > mtval=0xffffaf8002026023
> > sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> > mtval=0xffffaf8001b70021
> > sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> > mtval=0xffffaf8002110021
> > sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> > mtval=0xffffaf800214c021
> > sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> > mtval=0xffffaf8002024021
> > sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> > mtval=0xffffaf8001b72043
> > sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> > mtval=0xffffaf8002112043
> > sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> > mtval=0xffffaf800214e043
> > sbi_misaligned_v_ld_emulator: insn=0x205e007 mepc=0xffffffff80016fee
> > mtval=0xffffaf8002026043
> > sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> > mtval=0xffffaf8001b70041
> > sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> > mtval=0xffffaf8002110041
> > sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> > mtval=0xffffaf800214c041
> > sbi_misaligned_v_st_emulator: insn=0x2056027 mepc=0xffffffff80016ff2
> > mtval=0xffffaf8002024041
> > sbi_misaligned_v_l�X0@lS0@�X0@�0@f8001b72Y0@Y0@M0@
> >
> > �0@��������80021120�0@��������emulatoB0@^b�Z0@
> >
> > �0@b�0@`Z0@h�M������
> >
> > ������0@�6�Z0@bb^
> >
> > HL0@B0@
> >
> >
> > �([0@�Z0@ a��0@"
> >
> >
> > �b�~0@��0@ator: insn=0x205[0@fA0@
> >
> >
> > B0@^
> >
> >
> > A0@[0@sb
> >
> >
> >
> > Bumping the stack size seems to fix this. If you want me to test
> > anything else, let me know. Not sure if the tests in PATCH 8 are
> > appropriate to test on this hw (I think they are for KVM guests
> > only?).
> >
> > [1] - https://lore.kernel.org/linux-riscv/nrvt74qnojaubiwjo37ums4lnclu466hovwrhmtbag6f5uhrql@q6msoe2oto4b/
> >
> >
> >
> >
> >
> >> - 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
> >>
> >> --
> >> 2.34.1
> >>
>
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v2 0/8] Fixes for load/store misaligned and access faults
2026-06-05 11:32 [PATCH v2 0/8] Fixes for load/store misaligned and access faults Bo Gan
` (8 preceding siblings ...)
2026-06-08 18:18 ` [PATCH v2 0/8] Fixes for load/store misaligned and access faults Anirudh Srinivasan
@ 2026-06-16 8:25 ` Anup Patel
9 siblings, 0 replies; 17+ messages in thread
From: Anup Patel @ 2026-06-16 8:25 UTC (permalink / raw)
To: Bo Gan
Cc: opensbi, wangruikang, dramforever, andrew.jones, cleger, pjw,
asrinivasan
On Fri, Jun 5, 2026 at 7:15 PM Bo Gan <ganboing@gmail.com> wrote:
>
> 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
Applied all patches except last one to the riscv/opensbi repo.
Thanks,
Anup
>
> 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
>
> --
> 2.34.1
>
>
> --
> opensbi mailing list
> opensbi@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/opensbi
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 17+ messages in thread