Skip to content

Preliminary support for trap handling during block emulation #463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Oct 19, 2024
Merged

Preliminary support for trap handling during block emulation #463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fc0d878
Preliminary support for trap handling during block emulation
ChinYikMing Jun 24, 2024
e6d12a8
Add misaligned access test suite
ChinYikMing Jun 25, 2024
a4f6dab
CI: Add misaligned access handling test suite during block emulation
ChinYikMing Oct 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .github/workflows/main.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ jobs:
make distclean && make ENABLE_EXT_F=0 check -j$(nproc)
make distclean && make ENABLE_EXT_C=0 check -j$(nproc)
make distclean && make ENABLE_SDL=0 check -j$(nproc)
- name: misalignment test in block emulation
run: |
make -C tests/system/alignment/
make distclean && make ENABLE_EXT_C=0 ENABLE_SYSTEM=1 misalign-in-blk-emu -j$(nproc)
- name: gdbstub test
run: |
make distclean && make ENABLE_GDBSTUB=1 gdbstub-test -j$(nproc)
Expand Down
15 changes: 14 additions & 1 deletion Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,9 @@ CFLAGS = -std=gnu99 -O2 -Wall -Wextra
CFLAGS += -Wno-unused-label
CFLAGS += -include src/common.h

ENABLE_SYSTEM ?= 0
$(call set-feature, SYSTEM)

# Enable link-time optimization (LTO)
ENABLE_LTO ?= 1
ifeq ($(call has, LTO), 1)
Expand Down Expand Up @@ -134,7 +137,7 @@ endif
ENABLE_JIT ?= 0
$(call set-feature, JIT)
ifeq ($(call has, JIT), 1)
OBJS_EXT += jit.o
OBJS_EXT += jit.o
# tier-2 JIT compiler powered LLVM
LLVM_CONFIG = llvm-config-17
LLVM_CONFIG := $(shell which $(LLVM_CONFIG))
Expand Down Expand Up @@ -286,6 +289,16 @@ misalign: $(BIN) artifact
$(PRINTF) "Failed.\n"; \
fi

EXPECTED_misalign = MISALIGNED INSTRUCTION FETCH TEST PASSED!
misalign-in-blk-emu: $(BIN)
$(Q)$(PRINTF) "Running misalign.elf ... "; \
if [ "$(shell $(BIN) tests/system/alignment/misalign.elf | tail -n 2)" = "$(strip $(EXPECTED_misalign)) inferior exit code 0" ]; then \
$(call notice, [OK]); \
else \
$(PRINTF) "Failed.\n"; \
exit 1; \
fi;

# Non-trivial demonstration programs
ifeq ($(call has, SDL), 1)
doom_action := (cd $(OUT); ../$(BIN) riscv32/doom)
Expand Down
2 changes: 2 additions & 0 deletions src/common.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

#define ARRAYS_SIZE(arr) (sizeof(arr) / sizeof(arr[0]))

#define MASK(n) (~((~0U << (n))))

/* Alignment macro */
#if defined(__GNUC__) || defined(__clang__)
#define __ALIGNED(x) __attribute__((aligned(x)))
Expand Down
2 changes: 2 additions & 0 deletions src/decode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -841,9 +841,11 @@ static inline bool op_system(rv_insn_t *ir, const uint32_t insn)
case 0x202: /* HRET: return from traps in H-mode */
/* illegal instruction */
return false;
#if RV32_HAS(SYSTEM)
case 0x102: /* SRET: return from traps in S-mode */
ir->opcode = rv_insn_sret;
break;
#endif
case 0x302: /* MRET */
ir->opcode = rv_insn_mret;
break;
Expand Down
4 changes: 3 additions & 1 deletion src/decode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ enum op_field {
/* RISC-V Privileged Instruction */ \
_(wfi, 0, 4, 0, ENC(rs1, rd)) \
_(uret, 0, 4, 0, ENC(rs1, rd)) \
_(sret, 1, 4, 0, ENC(rs1, rd)) \
IIF(RV32_HAS(SYSTEM))( \
_(sret, 1, 4, 0, ENC(rs1, rd)) \
) \
_(hret, 0, 4, 0, ENC(rs1, rd)) \
_(mret, 1, 4, 0, ENC(rs1, rd)) \
_(sfencevma, 1, 4, 0, ENC(rs1, rs2, rd)) \
Expand Down
202 changes: 150 additions & 52 deletions src/emulate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,82 +43,127 @@ extern struct target_ops gdbstub_ops;

/* RISC-V exception code list */
/* clang-format off */
#define RV_EXCEPTION_LIST \
#define RV_TRAP_LIST \
IIF(RV32_HAS(EXT_C))(, \
_(insn_misaligned, 0) /* Instruction address misaligned */ \
) \
_(illegal_insn, 2) /* Illegal instruction */ \
_(breakpoint, 3) /* Breakpoint */ \
_(load_misaligned, 4) /* Load address misaligned */ \
_(store_misaligned, 6) /* Store/AMO address misaligned */ \
_(ecall_M, 11) /* Environment call from M-mode */
IIF(RV32_HAS(SYSTEM))(, \
_(ecall_M, 11) /* Environment call from M-mode */ \
)
/* clang-format on */

enum {
#define _(type, code) rv_exception_code##type = code,
RV_EXCEPTION_LIST
#define _(type, code) rv_trap_code_##type = code,
RV_TRAP_LIST
#undef _
};

static void rv_exception_default_handler(riscv_t *rv)
static void rv_trap_default_handler(riscv_t *rv)
{
rv->csr_mepc += rv->compressed ? 2 : 4;
rv->PC = rv->csr_mepc; /* mret */
}

/* When a trap occurs in M-mode, mtval is either initialized to zero or
/*
* Trap might occurs during block emulation. For instance, page fault.
* In order to handle trap, we have to escape from block and execute
* registered trap handler. This trap_handler function helps to execute
* the registered trap handler, PC by PC. Once the trap is handled,
* resume the previous execution flow where cause the trap.
*
* Since the system emulation has not yet included in rv32emu, the page
* fault is not practical in current test suite. Instead, we try to
* emulate the misaligned handling in the test suite.
*/
#if RV32_HAS(SYSTEM)
static void trap_handler(riscv_t *rv);
#endif

/* When a trap occurs in M-mode/S-mode, m/stval is either initialized to zero or
* populated with exception-specific details to assist software in managing
* the trap. Otherwise, the implementation never modifies mtval, although
* the trap. Otherwise, the implementation never modifies m/stval, although
* software can explicitly write to it. The hardware platform will define
* which exceptions are required to informatively set mtval and which may
* consistently set it to zero.
*
* When a hardware breakpoint is triggered or an exception like address
* misalignment, access fault, or page fault occurs during an instruction
* fetch, load, or store operation, mtval is updated with the virtual address
* that caused the fault. In the case of an illegal instruction trap, mtval
* fetch, load, or store operation, m/stval is updated with the virtual address
* that caused the fault. In the case of an illegal instruction trap, m/stval
* might be updated with the first XLEN or ILEN bits of the offending
* instruction. For all other traps, mtval is simply set to zero. However,
* it is worth noting that a future standard could redefine how mtval is
* instruction. For all other traps, m/stval is simply set to zero. However,
* it is worth noting that a future standard could redefine how m/stval is
* handled for different types of traps.
*
* For simplicity and clarity, abstracting stval and mtval into a single
* identifier called tval, as both are handled by TRAP_HANDLER_IMPL.
*/
#define EXCEPTION_HANDLER_IMPL(type, code) \
static void rv_except_##type(riscv_t *rv, uint32_t mtval) \
{ \
/* mtvec (Machine Trap-Vector Base Address Register) \
* mtvec[MXLEN-1:2]: vector base address \
* mtvec[1:0] : vector mode \
*/ \
const uint32_t base = rv->csr_mtvec & ~0x3; \
const uint32_t mode = rv->csr_mtvec & 0x3; \
/* mepc (Machine Exception Program Counter) \
* mtval (Machine Trap Value Register) \
* mcause (Machine Cause Register): store exception code \
* mstatus (Machine Status Register): keep track of and controls the \
* hart’s current operating state \
*/ \
rv->csr_mepc = rv->PC; \
rv->csr_mtval = mtval; \
rv->csr_mcause = code; \
rv->csr_mstatus = MSTATUS_MPP; /* set privilege mode */ \
if (!rv->csr_mtvec) { /* in case CSR is not configured */ \
rv_exception_default_handler(rv); \
return; \
} \
switch (mode) { \
case 0: /* DIRECT: All exceptions set PC to base */ \
rv->PC = base; \
break; \
/* VECTORED: Asynchronous interrupts set PC to base + 4 * code */ \
case 1: \
rv->PC = base + 4 * code; \
break; \
} \
#define TRAP_HANDLER_IMPL(type, code) \
static void rv_trap_##type(riscv_t *rv, uint32_t tval) \
{ \
/* m/stvec (Machine/Supervisor Trap-Vector Base Address Register) \
* m/stvec[MXLEN-1:2]: vector base address \
* m/stvec[1:0] : vector mode \
* m/sepc (Machine/Supervisor Exception Program Counter) \
* m/stval (Machine/Supervisor Trap Value Register) \
* m/scause (Machine/Supervisor Cause Register): store exception code \
* m/sstatus (Machine/Supervisor Status Register): keep track of and \
* controls the hart’s current operating state \
*/ \
uint32_t base; \
uint32_t mode; \
/* user or supervisor */ \
if (RV_PRIV_IS_U_OR_S_MODE()) { \
const uint32_t sstatus_sie = \
(rv->csr_sstatus & SSTATUS_SIE) >> SSTATUS_SIE_SHIFT; \
rv->csr_sstatus |= (sstatus_sie << SSTATUS_SPIE_SHIFT); \
rv->csr_sstatus &= ~(SSTATUS_SIE); \
rv->csr_sstatus |= (rv->priv_mode << SSTATUS_SPP_SHIFT); \
rv->priv_mode = RV_PRIV_S_MODE; \
base = rv->csr_stvec & ~0x3; \
mode = rv->csr_stvec & 0x3; \
rv->csr_sepc = rv->PC; \
rv->csr_stval = tval; \
rv->csr_scause = code; \
} else { /* machine */ \
const uint32_t mstatus_mie = \
(rv->csr_mstatus & MSTATUS_MIE) >> MSTATUS_MIE_SHIFT; \
rv->csr_mstatus |= (mstatus_mie << MSTATUS_MPIE_SHIFT); \
rv->csr_mstatus &= ~(MSTATUS_MIE); \
rv->csr_mstatus |= (rv->priv_mode << MSTATUS_MPP_SHIFT); \
rv->priv_mode = RV_PRIV_M_MODE; \
base = rv->csr_mtvec & ~0x3; \
mode = rv->csr_mtvec & 0x3; \
rv->csr_mepc = rv->PC; \
rv->csr_mtval = tval; \
rv->csr_mcause = code; \
if (!rv->csr_mtvec) { /* in case CSR is not configured */ \
rv_trap_default_handler(rv); \
return; \
} \
} \
switch (mode) { \
/* DIRECT: All traps set PC to base */ \
case 0: \
rv->PC = base; \
break; \
/* VECTORED: Asynchronous traps set PC to base + 4 * code */ \
case 1: \
/* MSB of code is used to indicate whether the trap is interrupt \
* or exception, so it is not considered as the 'real' code */ \
rv->PC = base + 4 * (code & MASK(31)); \
break; \
} \
IIF(RV32_HAS(SYSTEM))(if (rv->is_trapped) trap_handler(rv);, ) \
}

/* RISC-V exception handlers */
#define _(type, code) EXCEPTION_HANDLER_IMPL(type, code)
RV_EXCEPTION_LIST
#define _(type, code) TRAP_HANDLER_IMPL(type, code)
RV_TRAP_LIST
#undef _

/* wrap load/store and insn misaligned handler
Expand All @@ -135,7 +180,8 @@ RV_EXCEPTION_LIST
rv->compressed = compress; \
rv->csr_cycle = cycle; \
rv->PC = PC; \
rv_except_##type##_misaligned(rv, IIF(IO)(addr, mask_or_pc)); \
IIF(RV32_HAS(SYSTEM))(rv->is_trapped = true, ); \
rv_trap_##type##_misaligned(rv, IIF(IO)(addr, mask_or_pc)); \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shall we regard misalignment as a special case? Or, can it be generalized?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I know what does the special case refer to?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May I know what does the special case refer to?

I meant to ask whether misalignment exceptions should be treated as one of the supported traps. If so, checks for misalignment exceptions should be implemented indirectly.

Copy link
Collaborator Author

@ChinYikMing ChinYikMing Oct 5, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the entry.S of RISCV/Linux v6.1.112, it does support do_trap_load_misaligned and do_trap_store_misaligned. Thus, any misaligned load/store fault should be trapped into same path. In other words, trap_handler should be capable to handle the trap.

return false; \
}

Expand Down Expand Up @@ -164,6 +210,10 @@ static uint32_t *csr_get_ptr(riscv_t *rv, uint32_t csr)
return (uint32_t *) (&rv->csr_misa);

/* Machine Trap Handling */
case CSR_MEDELEG: /* Machine Exception Delegation Register */
return (uint32_t *) (&rv->csr_medeleg);
case CSR_MIDELEG: /* Machine Interrupt Delegation Register */
return (uint32_t *) (&rv->csr_mideleg);
case CSR_MSCRATCH: /* Machine Scratch Register */
return (uint32_t *) (&rv->csr_mscratch);
case CSR_MEPC: /* Machine Exception Program Counter */
Expand Down Expand Up @@ -196,6 +246,26 @@ static uint32_t *csr_get_ptr(riscv_t *rv, uint32_t csr)
case CSR_FCSR:
return (uint32_t *) (&rv->csr_fcsr);
#endif
case CSR_SSTATUS:
return (uint32_t *) (&rv->csr_sstatus);
case CSR_SIE:
return (uint32_t *) (&rv->csr_sie);
case CSR_STVEC:
return (uint32_t *) (&rv->csr_stvec);
case CSR_SCOUNTEREN:
return (uint32_t *) (&rv->csr_scounteren);
case CSR_SSCRATCH:
return (uint32_t *) (&rv->csr_sscratch);
case CSR_SEPC:
return (uint32_t *) (&rv->csr_sepc);
case CSR_SCAUSE:
return (uint32_t *) (&rv->csr_scause);
case CSR_STVAL:
return (uint32_t *) (&rv->csr_stval);
case CSR_SIP:
return (uint32_t *) (&rv->csr_sip);
case CSR_SATP:
return (uint32_t *) (&rv->csr_satp);
default:
return NULL;
}
Expand Down Expand Up @@ -377,9 +447,10 @@ enum {
};

#if RV32_HAS(GDBSTUB)
#define RVOP_NO_NEXT(ir) (!ir->next | rv->debug_mode)
#define RVOP_NO_NEXT(ir) \
(!ir->next | rv->debug_mode IIF(RV32_HAS(SYSTEM))(| rv->is_trapped, ))
#else
#define RVOP_NO_NEXT(ir) (!ir->next)
#define RVOP_NO_NEXT(ir) (!ir->next IIF(RV32_HAS(SYSTEM))(| rv->is_trapped, ))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be (!ir->next IIF(RV32_HAS(SYSTEM))(|| rv->is_trapped, )) instead?

Copy link
Collaborator Author

@ChinYikMing ChinYikMing Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be (!ir->next IIF(RV32_HAS(SYSTEM))(|| rv->is_trapped, )) instead?

Single | has same effect than double || since the all condition are boolean.

Copy link
Collaborator

@vacantron vacantron Oct 10, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current semantics is equivalent, but I'm worrying about the support of nested interrupt handling in the future. The is-trapped might become a integer as a counter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current semantics is equivalent, but I'm worrying about the support of nested interrupt handling in the future. The is-trapped might become a integer as a counter.

If used as a counter, the implementation of sret should decrement is_trapped rather than simply setting it to false? Also, other places using is_trapped should increment it?

However, I'm curious in the current emulation, have you observed nested traps occurring, and do these changes need to be made to handle them?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, I'm curious in the current emulation, have you observed nested traps occurring, and do these changes need to be made to handle them?

In current, no (if I'm not mistaken). But we need it if we want to handle the page fault during handling the external interrupt, right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

However, I'm curious in the current emulation, have you observed nested traps occurring, and do these changes need to be made to handle them?

In current, no (if I'm not mistaken). But we need it if we want to handle the page fault during handling the external interrupt, right?

Indeed. However, we currently don't have a test suite for this situation. The best test suite would be the Linux kernel itself, and we might need an asynchronous mechanism (e.g., a signal thread) to periodically intercept the external interrupt.

For simplicity, I would prefer to keep track of this issue and address it only when it becomes necessary.

#endif

/* record whether the branch is taken or not during emulation */
Expand Down Expand Up @@ -565,8 +636,10 @@ FORCE_INLINE bool insn_is_unconditional_branch(uint8_t opcode)
case rv_insn_ebreak:
case rv_insn_jal:
case rv_insn_jalr:
case rv_insn_sret:
case rv_insn_mret:
#if RV32_HAS(SYSTEM)
case rv_insn_sret:
#endif
#if RV32_HAS(EXT_C)
case rv_insn_cj:
case rv_insn_cjalr:
Expand Down Expand Up @@ -598,7 +671,7 @@ static void block_translate(riscv_t *rv, block_t *block)
/* decode the instruction */
if (!rv_decode(ir, insn)) {
rv->compressed = is_compressed(insn);
rv_except_illegal_insn(rv, insn);
rv_trap_illegal_insn(rv, insn);
break;
}
ir->impl = dispatch_table[ir->opcode];
Expand Down Expand Up @@ -1048,17 +1121,42 @@ void rv_step(void *arg)
#endif
}

#if RV32_HAS(SYSTEM)
static void trap_handler(riscv_t *rv)
{
rv_insn_t *ir = mpool_alloc(rv->block_ir_mp);
assert(ir);

/* set to false by sret/mret implementation */
uint32_t insn;
while (rv->is_trapped && !rv_has_halted(rv)) {
insn = rv->io.mem_ifetch(rv->PC);
assert(insn);

rv_decode(ir, insn);
ir->impl = dispatch_table[ir->opcode];
rv->compressed = is_compressed(insn);
ir->impl(rv, ir, rv->csr_cycle, rv->PC);
}
}
#endif

void ebreak_handler(riscv_t *rv)
{
assert(rv);
rv_except_breakpoint(rv, rv->PC);
rv_trap_breakpoint(rv, rv->PC);
}

void ecall_handler(riscv_t *rv)
{
assert(rv);
rv_except_ecall_M(rv, 0);
#if RV32_HAS(SYSTEM)
syscall_handler(rv);
rv->PC += 4;
#else
rv_trap_ecall_M(rv, 0);
syscall_handler(rv);
#endif
}

void memset_handler(riscv_t *rv)
Expand Down
14 changes: 14 additions & 0 deletions src/riscv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,20 @@ riscv_t *rv_create(riscv_user_t rv_attr)
#endif
#endif

#if RV32_HAS(SYSTEM)
/*
* System simulation defaults to S-mode as
* it does not rely on M-mode software like OpenSBI.
*/
rv->priv_mode = RV_PRIV_S_MODE;

/* not being trapped */
rv->is_trapped = false;
#else
/* ISA simulation defaults to M-mode */
rv->priv_mode = RV_PRIV_M_MODE;
#endif

return rv;
}

Expand Down
Loading