From 5b1aebf31905dd00e8cf2064ce8584e55edb66da Mon Sep 17 00:00:00 2001 From: Emmanuel Blot Date: Fri, 16 Apr 2021 16:17:11 +0200 Subject: [PATCH 1/3] target/riscv: fix exception index on instruction access fault When no MMU is used and the guest code attempts to fetch an instruction from an invalid memory location, the exception index defaults to a data load access fault, rather an instruction access fault. Signed-off-by: Emmanuel Blot Reviewed-by: Alistair Francis Message-id: FB9EA197-B018-4879-AB0F-922C2047A08B@sifive.com Signed-off-by: Alistair Francis (cherry picked from commit f9e580c13ae0d42cf8989063254300c59166ffed) --- target/riscv/cpu_helper.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 4b2518543b..f971c81cc4 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -892,8 +892,10 @@ void riscv_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr, if (access_type == MMU_DATA_STORE) { cs->exception_index = RISCV_EXCP_STORE_AMO_ACCESS_FAULT; - } else { + } else if (access_type == MMU_DATA_LOAD) { cs->exception_index = RISCV_EXCP_LOAD_ACCESS_FAULT; + } else { + cs->exception_index = RISCV_EXCP_INST_ACCESS_FAULT; } env->badaddr = addr; From 0077028fed11d3c4c644f3630ee2b01557b3e025 Mon Sep 17 00:00:00 2001 From: Alex Richardson Date: Tue, 14 Dec 2021 19:16:24 +0000 Subject: [PATCH 2/3] [instr logging] Log the faulting instruction before fault side-effects Previously, we would not log any exception output if we get stuck in an infinite loop due to an ifetch fault after an exception because the log_instr_commit only happens when we execute an instruction. To work around this, we can call qemu_log_instr_commit() before handling the exception/interrupt and inject a fake instruction to the log buffer (with size==0 and address -1). A nice side-effect of this change is that the faulting exception is now decoded according to the mode that the CPU was in before installing the trap vector. Previously a trap from capmode to a non-capmode handler would disassemble a capability-relative load as the integer one since the exception handler PCC was already installed when the logging performs the disassembly. Instead of no output, we now print an infinite stream of the following if we get a trap while trying to jump to the exception handler: ``` [0:0] 0x0000000080000078: 0002c023 csc cnull,0(ct0) [0:0] logging exception side effects -> Switch to Machine mode -> Exception #6 vector 0x0000000000000000 fault-addr 0x0000000080065aa8 Write mstatus = 0000000a00001800 Write mcause = 0000000000000006 Write MEPCC|v:1 s:0 p:00078fff f:1 b:0000000000000000 l:ffffffffffffffff |o:0000000080000078 t:000000000003ffff Write mbadaddr = 0000000080065aa8 Write mtval2 = 0000000000000000 Write PCC|v:1 s:0 p:00078fff f:0 b:0000000000000000 l:ffffffffffffffff |o:0000000000000000 t:000000000003ffff [0:0] logging exception side effects -> Switch to Machine mode -> Exception #1 vector 0x0000000000000000 fault-addr 0x0000000000000000 Write mstatus = 0000000a00001800 Write mcause = 0000000000000001 Write MEPCC|v:1 s:0 p:00078fff f:0 b:0000000000000000 l:ffffffffffffffff |o:0000000000000000 t:000000000003ffff Write mbadaddr = 0000000000000000 Write mtval2 = 0000000000000000 Write PCC|v:1 s:0 p:00078fff f:0 b:0000000000000000 l:ffffffffffffffff |o:0000000000000000 t:000000000003ffff [0:0] logging exception side effects -> Switch to Machine mode -> Exception #1 vector 0x0000000000000000 fault-addr 0x0000000000000000 Write mstatus = 0000000a00001800 Write mcause = 0000000000000001 Write MEPCC|v:1 s:0 p:00078fff f:0 b:0000000000000000 l:ffffffffffffffff |o:0000000000000000 t:000000000003ffff Write mbadaddr = 0000000000000000 Write mtval2 = 0000000000000000 Write PCC|v:1 s:0 p:00078fff f:0 b:0000000000000000 l:ffffffffffffffff |o:0000000000000000 t:000000000003ffff ``` --- accel/tcg/cpu-exec.c | 14 +++++++++++++- accel/tcg/log_instr.c | 8 ++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index b834006e8a..5d34744bf2 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -32,6 +32,7 @@ #include "exec/tb-hash.h" #include "exec/tb-lookup.h" #include "exec/log.h" +#include "exec/log_instr.h" #include "qemu/main-loop.h" #if defined(TARGET_I386) && !defined(CONFIG_USER_ONLY) #include "hw/i386/apic.h" @@ -488,7 +489,12 @@ static inline void cpu_handle_debug_exception(CPUState *cpu) wp->flags &= ~BP_WATCHPOINT_HIT; } } - + /* Print the current (aborted) instruction. */ + if (qemu_log_instr_enabled(cpu->env_ptr)) { + qemu_log_instr_commit(cpu->env_ptr); + /* Add a "fake" instruction for the exception side effects. */ + qemu_log_instr(cpu->env_ptr, ~(target_ulong)0, "", 0); + } cc->debug_excp_handler(cpu); } @@ -532,6 +538,12 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret) if (replay_exception()) { CPUClass *cc = CPU_GET_CLASS(cpu); qemu_mutex_lock_iothread(); + if (qemu_log_instr_enabled(cpu->env_ptr)) { + /* Print the current (aborted) instruction. */ + qemu_log_instr_commit(cpu->env_ptr); + /* Add a "fake" instruction for the exception side effects. */ + qemu_log_instr(cpu->env_ptr, ~(target_ulong)0, "", 0); + } cc->do_interrupt(cpu); qemu_mutex_unlock_iothread(); cpu->exception_index = -1; diff --git a/accel/tcg/log_instr.c b/accel/tcg/log_instr.c index d454eeda7b..835aae1222 100644 --- a/accel/tcg/log_instr.c +++ b/accel/tcg/log_instr.c @@ -349,8 +349,12 @@ static void emit_text_entry(CPUArchState *env, cpu_log_instr_info_t *iinfo) rcu_read_lock(); logfile = qatomic_rcu_read(&qemu_logfile); if (logfile) { - target_disas_buf(logfile->fd, env_cpu(env), iinfo->insn_bytes, - sizeof(iinfo->insn_bytes), iinfo->pc, 1); + if (iinfo->insn_size < 1) { + fprintf(logfile->fd, "logging exception side effects\n"); + } else { + target_disas_buf(logfile->fd, env_cpu(env), iinfo->insn_bytes, + sizeof(iinfo->insn_bytes), iinfo->pc, 1); + } } rcu_read_unlock(); From 524ff9676f67d22e13108e3e37a14f9eba2a22fc Mon Sep 17 00:00:00 2001 From: Alex Richardson Date: Tue, 14 Dec 2021 19:19:51 +0000 Subject: [PATCH 3/3] [RISC-V] Don't attempt to log an instruction on ifetch faults Right now we will just print the following useless information: Trap (fault_fetch) was probably caused by: 0x0000000000000000: Address 0x0 is out of bounds. --- target/riscv/cpu_helper.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index f971c81cc4..35966951a7 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -1190,7 +1190,6 @@ bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size, void riscv_cpu_do_interrupt(CPUState *cs) { #if !defined(CONFIG_USER_ONLY) - RISCVCPU *cpu = RISCV_CPU(cs); CPURISCVState *env = &cpu->env; tcg_debug_assert(pc_is_current(env)); @@ -1224,10 +1223,10 @@ void riscv_cpu_do_interrupt(CPUState *cs) case RISCV_EXCP_LOAD_CAP_PAGE_FAULT: case RISCV_EXCP_STORE_AMO_CAP_PAGE_FAULT: #endif - log_inst = false; - /* fallthrough */ case RISCV_EXCP_INST_ADDR_MIS: case RISCV_EXCP_INST_ACCESS_FAULT: + log_inst = false; + /* fallthrough */ case RISCV_EXCP_LOAD_ADDR_MIS: case RISCV_EXCP_STORE_AMO_ADDR_MIS: case RISCV_EXCP_LOAD_ACCESS_FAULT: @@ -1279,7 +1278,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) if (unlikely(log_inst && qemu_loglevel_mask(CPU_LOG_INT))) { FILE* logf = qemu_log_lock(); - qemu_log("Trap (%s) was probably caused by: ", riscv_cpu_get_trap_name(cause, async)); + fprintf(logf, "Trap (%s) was probably caused by: ", riscv_cpu_get_trap_name(cause, async)); target_disas(logf, cs, PC_ADDR(env), /* Only one instr*/-1); qemu_log_unlock(logf); }