* [PATCH 0/2] RISC-V: Populate mtval and stval @ 2021-12-10 6:26 Alistair Francis 2021-12-10 6:26 ` [PATCH 1/2] target/riscv: Set the opcode in DisasContext Alistair Francis 2021-12-10 6:26 ` [PATCH 2/2] target/riscv: Implement the stval/mtval illegal instruction Alistair Francis 0 siblings, 2 replies; 5+ messages in thread From: Alistair Francis @ 2021-12-10 6:26 UTC (permalink / raw) To: qemu-devel, qemu-riscv Cc: Alistair Francis, palmer, alistair23, Bin Meng, bmeng.cn From: Alistair Francis <alistair.francis@wdc.com> Populate mtval and stval when taking an illegal instruction exception. The RISC-V spec states that "The stval register can optionally also be used to return the faulting instruction bits on an illegal instruction exception...". In this case we are always writing the value on an illegal instruction. This doesn't match all CPUs (some CPUs won't write the data), but in QEMU let's just populate the value on illegal instructions. This won't break any guest software, but will provide more information to guests. *** BLURB HERE *** Alistair Francis (2): target/riscv: Set the opcode in DisasContext target/riscv: Implement the stval/mtval illegal instruction target/riscv/cpu.h | 2 ++ target/riscv/cpu_helper.c | 25 +++++++++++-------------- target/riscv/translate.c | 5 +++++ 3 files changed, 18 insertions(+), 14 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] target/riscv: Set the opcode in DisasContext 2021-12-10 6:26 [PATCH 0/2] RISC-V: Populate mtval and stval Alistair Francis @ 2021-12-10 6:26 ` Alistair Francis 2021-12-10 15:28 ` Richard Henderson 2021-12-10 6:26 ` [PATCH 2/2] target/riscv: Implement the stval/mtval illegal instruction Alistair Francis 1 sibling, 1 reply; 5+ messages in thread From: Alistair Francis @ 2021-12-10 6:26 UTC (permalink / raw) To: qemu-devel, qemu-riscv Cc: Alistair Francis, palmer, alistair23, Bin Meng, bmeng.cn From: Alistair Francis <alistair.francis@wdc.com> Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- target/riscv/translate.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 1d57bc97b5..24251bc8cc 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -586,6 +586,7 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) if (!has_ext(ctx, RVC)) { gen_exception_illegal(ctx); } else { + ctx->opcode = opcode; ctx->pc_succ_insn = ctx->base.pc_next + 2; if (!decode_insn16(ctx, opcode)) { gen_exception_illegal(ctx); @@ -596,6 +597,7 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode) opcode32 = deposit32(opcode32, 16, 16, translator_lduw(env, &ctx->base, ctx->base.pc_next + 2)); + ctx->opcode = opcode32; ctx->pc_succ_insn = ctx->base.pc_next + 4; if (!decode_insn32(ctx, opcode32)) { gen_exception_illegal(ctx); -- 2.31.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] target/riscv: Set the opcode in DisasContext 2021-12-10 6:26 ` [PATCH 1/2] target/riscv: Set the opcode in DisasContext Alistair Francis @ 2021-12-10 15:28 ` Richard Henderson 0 siblings, 0 replies; 5+ messages in thread From: Richard Henderson @ 2021-12-10 15:28 UTC (permalink / raw) To: Alistair Francis, qemu-devel, qemu-riscv Cc: palmer, Bin Meng, Alistair Francis, bmeng.cn, alistair23 On 12/9/21 10:26 PM, Alistair Francis wrote: > From: Alistair Francis <alistair.francis@wdc.com> > > Signed-off-by: Alistair Francis <alistair.francis@wdc.com> > --- > target/riscv/translate.c | 2 ++ > 1 file changed, 2 insertions(+) > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> r~ ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 2/2] target/riscv: Implement the stval/mtval illegal instruction 2021-12-10 6:26 [PATCH 0/2] RISC-V: Populate mtval and stval Alistair Francis 2021-12-10 6:26 ` [PATCH 1/2] target/riscv: Set the opcode in DisasContext Alistair Francis @ 2021-12-10 6:26 ` Alistair Francis 2021-12-10 15:11 ` Richard Henderson 1 sibling, 1 reply; 5+ messages in thread From: Alistair Francis @ 2021-12-10 6:26 UTC (permalink / raw) To: qemu-devel, qemu-riscv Cc: Alistair Francis, palmer, alistair23, Bin Meng, bmeng.cn From: Alistair Francis <alistair.francis@wdc.com> The stval and mtval registers can optionally contain the faulting instruction on an illegal instruction exception. This patch adds support for setting the stval and mtval registers based on the CPU feature. Signed-off-by: Alistair Francis <alistair.francis@wdc.com> --- target/riscv/cpu.h | 2 ++ target/riscv/cpu_helper.c | 25 +++++++++++-------------- target/riscv/translate.c | 3 +++ 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 0760c0af93..3a163b57ed 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -127,6 +127,8 @@ struct CPURISCVState { target_ulong frm; target_ulong badaddr; + uint32_t bins; + target_ulong guest_phys_fault_addr; target_ulong priv_ver; diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 9eeed38c7e..cb5833a6d2 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -975,7 +975,6 @@ void riscv_cpu_do_interrupt(CPUState *cs) bool async = !!(cs->exception_index & RISCV_EXCP_INT_FLAG); target_ulong cause = cs->exception_index & RISCV_EXCP_INT_MASK; target_ulong deleg = async ? env->mideleg : env->medeleg; - bool write_tval = false; target_ulong tval = 0; target_ulong htval = 0; target_ulong mtval2 = 0; @@ -1004,9 +1003,11 @@ void riscv_cpu_do_interrupt(CPUState *cs) case RISCV_EXCP_INST_PAGE_FAULT: case RISCV_EXCP_LOAD_PAGE_FAULT: case RISCV_EXCP_STORE_PAGE_FAULT: - write_tval = true; tval = env->badaddr; break; + case RISCV_EXCP_ILLEGAL_INST: + tval = env->bins; + break; default: break; } @@ -1041,17 +1042,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) if (riscv_has_ext(env, RVH)) { target_ulong hdeleg = async ? env->hideleg : env->hedeleg; - if (env->two_stage_lookup && write_tval) { - /* - * If we are writing a guest virtual address to stval, set - * this to 1. If we are trapping to VS we will set this to 0 - * later. - */ - env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 1); - } else { - /* For other HS-mode traps, we set this to 0. */ - env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 0); - } + env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 0); if (riscv_cpu_virt_enabled(env) && ((hdeleg >> cause) & 1)) { /* Trap to VS mode */ @@ -1063,7 +1054,6 @@ void riscv_cpu_do_interrupt(CPUState *cs) cause == IRQ_VS_EXT) { cause = cause - 1; } - env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 0); } else if (riscv_cpu_virt_enabled(env)) { /* Trap into HS mode, from virt */ riscv_cpu_swap_hypervisor_regs(env); @@ -1071,6 +1061,13 @@ void riscv_cpu_do_interrupt(CPUState *cs) env->priv); env->hstatus = set_field(env->hstatus, HSTATUS_SPV, riscv_cpu_virt_enabled(env)); + if (tval) { + /* + * If we are writing a guest virtual address to stval, set + * this to 1. + */ + env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 1); + } htval = env->guest_phys_fault_addr; diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 24251bc8cc..921ca06bf9 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -167,6 +167,9 @@ static void generate_exception_mtval(DisasContext *ctx, int excp) static void gen_exception_illegal(DisasContext *ctx) { + tcg_gen_st_i32(tcg_constant_i32(ctx->opcode), cpu_env, + offsetof(CPURISCVState, bins)); + generate_exception(ctx, RISCV_EXCP_ILLEGAL_INST); } -- 2.31.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] target/riscv: Implement the stval/mtval illegal instruction 2021-12-10 6:26 ` [PATCH 2/2] target/riscv: Implement the stval/mtval illegal instruction Alistair Francis @ 2021-12-10 15:11 ` Richard Henderson 0 siblings, 0 replies; 5+ messages in thread From: Richard Henderson @ 2021-12-10 15:11 UTC (permalink / raw) To: Alistair Francis, qemu-devel, qemu-riscv Cc: palmer, Bin Meng, Alistair Francis, bmeng.cn, alistair23 On 12/9/21 10:26 PM, Alistair Francis wrote: > @@ -975,7 +975,6 @@ void riscv_cpu_do_interrupt(CPUState *cs) > bool async = !!(cs->exception_index & RISCV_EXCP_INT_FLAG); > target_ulong cause = cs->exception_index & RISCV_EXCP_INT_MASK; > target_ulong deleg = async ? env->mideleg : env->medeleg; > - bool write_tval = false; > target_ulong tval = 0; > target_ulong htval = 0; > target_ulong mtval2 = 0; > @@ -1004,9 +1003,11 @@ void riscv_cpu_do_interrupt(CPUState *cs) > case RISCV_EXCP_INST_PAGE_FAULT: > case RISCV_EXCP_LOAD_PAGE_FAULT: > case RISCV_EXCP_STORE_PAGE_FAULT: > - write_tval = true; > tval = env->badaddr; > break; ... > @@ -1041,17 +1042,7 @@ void riscv_cpu_do_interrupt(CPUState *cs) > if (riscv_has_ext(env, RVH)) { > target_ulong hdeleg = async ? env->hideleg : env->hedeleg; > > - if (env->two_stage_lookup && write_tval) { > - /* > - * If we are writing a guest virtual address to stval, set > - * this to 1. If we are trapping to VS we will set this to 0 > - * later. > - */ > - env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 1); > - } else { > - /* For other HS-mode traps, we set this to 0. */ > - env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 0); > - } > + env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 0); > > if (riscv_cpu_virt_enabled(env) && ((hdeleg >> cause) & 1)) { > /* Trap to VS mode */ > @@ -1063,7 +1054,6 @@ void riscv_cpu_do_interrupt(CPUState *cs) > cause == IRQ_VS_EXT) { > cause = cause - 1; > } > - env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 0); > } else if (riscv_cpu_virt_enabled(env)) { > /* Trap into HS mode, from virt */ > riscv_cpu_swap_hypervisor_regs(env); > @@ -1071,6 +1061,13 @@ void riscv_cpu_do_interrupt(CPUState *cs) > env->priv); > env->hstatus = set_field(env->hstatus, HSTATUS_SPV, > riscv_cpu_virt_enabled(env)); > + if (tval) { > + /* > + * If we are writing a guest virtual address to stval, set > + * this to 1. > + */ > + env->hstatus = set_field(env->hstatus, HSTATUS_GVA, 1); > + } The thing that concerns me here is the very legitimate badaddr of NULL. I think you need to keep some out-of-band flag for that. In addition, the out-of-band flag should probably be called write_gva, because tval from ILLEGAL_INST is not a Guest Virtual Address, and so should not set GVA. You could even push the setting of the bit down so that it's only done once, e.g. hstatus_gva = true; tval = env->badaddr; if (trap to vs) { ... hstatus_gva = false; } else if (virt enabled) { ... } else { trap into hs hstatus_gva = false; } env->hstatus(set_field(env->hstatus, HSTATUS_GVA, hstatus_gva); The actual handling of ILLEGAL_INST looks fine. You may well want to split the GVA handling to a separate patch. r~ ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-12-10 15:32 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-12-10 6:26 [PATCH 0/2] RISC-V: Populate mtval and stval Alistair Francis 2021-12-10 6:26 ` [PATCH 1/2] target/riscv: Set the opcode in DisasContext Alistair Francis 2021-12-10 15:28 ` Richard Henderson 2021-12-10 6:26 ` [PATCH 2/2] target/riscv: Implement the stval/mtval illegal instruction Alistair Francis 2021-12-10 15:11 ` Richard Henderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).