* [PATCH] target/riscv: Prevent lost illegal instruction exceptions
@ 2021-03-16 15:03 ` Georg Kotheimer
0 siblings, 0 replies; 5+ messages in thread
From: Georg Kotheimer @ 2021-03-16 15:03 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Alistair Francis, Richard Henderson, Georg Kotheimer
When decode_insn16() fails, we fall back to decode_RV32_64C() for
further compressed instruction decoding. However, prior to this change,
we did not raise an illegal instruction exception, if decode_RV32_64C()
fails to decode the instruction. This means that we skipped illegal
compressed instructions instead of raising an illegal instruction
exception.
Signed-off-by: Georg Kotheimer <georg.kotheimer@kernkonzept.com>
---
target/riscv/translate.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0f28b5f41e..8c00734252 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -537,7 +537,7 @@ static void gen_set_rm(DisasContext *ctx, int rm)
tcg_temp_free_i32(t0);
}
-static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
+static bool decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
{
uint8_t funct3 = extract16(opcode, 13, 3);
uint8_t rd_rs2 = GET_C_RS2S(opcode);
@@ -554,7 +554,7 @@ static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
gen_fp_load(ctx, OPC_RISC_FLW, rd_rs2, rs1s,
GET_C_LW_IMM(opcode));
#endif
- break;
+ return true;
case 7:
#if defined(TARGET_RISCV64)
/* C.SD (RV64/128) -> sd rs2', offset[7:3](rs1')*/
@@ -565,18 +565,21 @@ static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
gen_fp_store(ctx, OPC_RISC_FSW, rs1s, rd_rs2,
GET_C_LW_IMM(opcode));
#endif
- break;
+ return true;
+ default:
+ return false;
}
}
-static void decode_RV32_64C(DisasContext *ctx, uint16_t opcode)
+static bool decode_RV32_64C(DisasContext *ctx, uint16_t opcode)
{
uint8_t op = extract16(opcode, 0, 2);
switch (op) {
case 0:
- decode_RV32_64C0(ctx, opcode);
- break;
+ return decode_RV32_64C0(ctx, opcode);
+ default:
+ return false;
}
}
@@ -780,7 +783,9 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
ctx->pc_succ_insn = ctx->base.pc_next + 2;
if (!decode_insn16(ctx, opcode)) {
/* fall back to old decoder */
- decode_RV32_64C(ctx, opcode);
+ if (!decode_RV32_64C(ctx, opcode)) {
+ gen_exception_illegal(ctx);
+ }
}
}
} else {
--
2.30.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH] target/riscv: Prevent lost illegal instruction exceptions
@ 2021-03-16 15:03 ` Georg Kotheimer
0 siblings, 0 replies; 5+ messages in thread
From: Georg Kotheimer @ 2021-03-16 15:03 UTC (permalink / raw)
To: qemu-devel, qemu-riscv
Cc: Richard Henderson, Alistair Francis, Georg Kotheimer
When decode_insn16() fails, we fall back to decode_RV32_64C() for
further compressed instruction decoding. However, prior to this change,
we did not raise an illegal instruction exception, if decode_RV32_64C()
fails to decode the instruction. This means that we skipped illegal
compressed instructions instead of raising an illegal instruction
exception.
Signed-off-by: Georg Kotheimer <georg.kotheimer@kernkonzept.com>
---
target/riscv/translate.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 0f28b5f41e..8c00734252 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -537,7 +537,7 @@ static void gen_set_rm(DisasContext *ctx, int rm)
tcg_temp_free_i32(t0);
}
-static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
+static bool decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
{
uint8_t funct3 = extract16(opcode, 13, 3);
uint8_t rd_rs2 = GET_C_RS2S(opcode);
@@ -554,7 +554,7 @@ static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
gen_fp_load(ctx, OPC_RISC_FLW, rd_rs2, rs1s,
GET_C_LW_IMM(opcode));
#endif
- break;
+ return true;
case 7:
#if defined(TARGET_RISCV64)
/* C.SD (RV64/128) -> sd rs2', offset[7:3](rs1')*/
@@ -565,18 +565,21 @@ static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
gen_fp_store(ctx, OPC_RISC_FSW, rs1s, rd_rs2,
GET_C_LW_IMM(opcode));
#endif
- break;
+ return true;
+ default:
+ return false;
}
}
-static void decode_RV32_64C(DisasContext *ctx, uint16_t opcode)
+static bool decode_RV32_64C(DisasContext *ctx, uint16_t opcode)
{
uint8_t op = extract16(opcode, 0, 2);
switch (op) {
case 0:
- decode_RV32_64C0(ctx, opcode);
- break;
+ return decode_RV32_64C0(ctx, opcode);
+ default:
+ return false;
}
}
@@ -780,7 +783,9 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
ctx->pc_succ_insn = ctx->base.pc_next + 2;
if (!decode_insn16(ctx, opcode)) {
/* fall back to old decoder */
- decode_RV32_64C(ctx, opcode);
+ if (!decode_RV32_64C(ctx, opcode)) {
+ gen_exception_illegal(ctx);
+ }
}
}
} else {
--
2.30.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] target/riscv: Prevent lost illegal instruction exceptions
2021-03-16 15:03 ` Georg Kotheimer
@ 2021-03-19 13:52 ` Alistair Francis
-1 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2021-03-19 13:52 UTC (permalink / raw)
To: Georg Kotheimer
Cc: Alistair Francis, Richard Henderson, open list:RISC-V,
qemu-devel@nongnu.org Developers
On Tue, Mar 16, 2021 at 11:05 AM Georg Kotheimer
<georg.kotheimer@kernkonzept.com> wrote:
>
> When decode_insn16() fails, we fall back to decode_RV32_64C() for
> further compressed instruction decoding. However, prior to this change,
> we did not raise an illegal instruction exception, if decode_RV32_64C()
> fails to decode the instruction. This means that we skipped illegal
> compressed instructions instead of raising an illegal instruction
> exception.
>
> Signed-off-by: Georg Kotheimer <georg.kotheimer@kernkonzept.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/translate.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 0f28b5f41e..8c00734252 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -537,7 +537,7 @@ static void gen_set_rm(DisasContext *ctx, int rm)
> tcg_temp_free_i32(t0);
> }
>
> -static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
> +static bool decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
> {
> uint8_t funct3 = extract16(opcode, 13, 3);
> uint8_t rd_rs2 = GET_C_RS2S(opcode);
> @@ -554,7 +554,7 @@ static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
> gen_fp_load(ctx, OPC_RISC_FLW, rd_rs2, rs1s,
> GET_C_LW_IMM(opcode));
> #endif
> - break;
> + return true;
> case 7:
> #if defined(TARGET_RISCV64)
> /* C.SD (RV64/128) -> sd rs2', offset[7:3](rs1')*/
> @@ -565,18 +565,21 @@ static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
> gen_fp_store(ctx, OPC_RISC_FSW, rs1s, rd_rs2,
> GET_C_LW_IMM(opcode));
> #endif
> - break;
> + return true;
> + default:
> + return false;
> }
> }
>
> -static void decode_RV32_64C(DisasContext *ctx, uint16_t opcode)
> +static bool decode_RV32_64C(DisasContext *ctx, uint16_t opcode)
> {
> uint8_t op = extract16(opcode, 0, 2);
>
> switch (op) {
> case 0:
> - decode_RV32_64C0(ctx, opcode);
> - break;
> + return decode_RV32_64C0(ctx, opcode);
> + default:
> + return false;
> }
> }
>
> @@ -780,7 +783,9 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
> ctx->pc_succ_insn = ctx->base.pc_next + 2;
> if (!decode_insn16(ctx, opcode)) {
> /* fall back to old decoder */
> - decode_RV32_64C(ctx, opcode);
> + if (!decode_RV32_64C(ctx, opcode)) {
> + gen_exception_illegal(ctx);
> + }
> }
> }
> } else {
> --
> 2.30.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] target/riscv: Prevent lost illegal instruction exceptions
@ 2021-03-19 13:52 ` Alistair Francis
0 siblings, 0 replies; 5+ messages in thread
From: Alistair Francis @ 2021-03-19 13:52 UTC (permalink / raw)
To: Georg Kotheimer
Cc: qemu-devel@nongnu.org Developers, open list:RISC-V,
Alistair Francis, Richard Henderson
On Tue, Mar 16, 2021 at 11:05 AM Georg Kotheimer
<georg.kotheimer@kernkonzept.com> wrote:
>
> When decode_insn16() fails, we fall back to decode_RV32_64C() for
> further compressed instruction decoding. However, prior to this change,
> we did not raise an illegal instruction exception, if decode_RV32_64C()
> fails to decode the instruction. This means that we skipped illegal
> compressed instructions instead of raising an illegal instruction
> exception.
>
> Signed-off-by: Georg Kotheimer <georg.kotheimer@kernkonzept.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/translate.c | 19 ++++++++++++-------
> 1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index 0f28b5f41e..8c00734252 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -537,7 +537,7 @@ static void gen_set_rm(DisasContext *ctx, int rm)
> tcg_temp_free_i32(t0);
> }
>
> -static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
> +static bool decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
> {
> uint8_t funct3 = extract16(opcode, 13, 3);
> uint8_t rd_rs2 = GET_C_RS2S(opcode);
> @@ -554,7 +554,7 @@ static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
> gen_fp_load(ctx, OPC_RISC_FLW, rd_rs2, rs1s,
> GET_C_LW_IMM(opcode));
> #endif
> - break;
> + return true;
> case 7:
> #if defined(TARGET_RISCV64)
> /* C.SD (RV64/128) -> sd rs2', offset[7:3](rs1')*/
> @@ -565,18 +565,21 @@ static void decode_RV32_64C0(DisasContext *ctx, uint16_t opcode)
> gen_fp_store(ctx, OPC_RISC_FSW, rs1s, rd_rs2,
> GET_C_LW_IMM(opcode));
> #endif
> - break;
> + return true;
> + default:
> + return false;
> }
> }
>
> -static void decode_RV32_64C(DisasContext *ctx, uint16_t opcode)
> +static bool decode_RV32_64C(DisasContext *ctx, uint16_t opcode)
> {
> uint8_t op = extract16(opcode, 0, 2);
>
> switch (op) {
> case 0:
> - decode_RV32_64C0(ctx, opcode);
> - break;
> + return decode_RV32_64C0(ctx, opcode);
> + default:
> + return false;
> }
> }
>
> @@ -780,7 +783,9 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
> ctx->pc_succ_insn = ctx->base.pc_next + 2;
> if (!decode_insn16(ctx, opcode)) {
> /* fall back to old decoder */
> - decode_RV32_64C(ctx, opcode);
> + if (!decode_RV32_64C(ctx, opcode)) {
> + gen_exception_illegal(ctx);
> + }
> }
> }
> } else {
> --
> 2.30.1
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] target/riscv: Prevent lost illegal instruction exceptions
2021-03-16 15:03 ` Georg Kotheimer
(?)
(?)
@ 2021-03-19 15:22 ` Richard Henderson
-1 siblings, 0 replies; 5+ messages in thread
From: Richard Henderson @ 2021-03-19 15:22 UTC (permalink / raw)
To: Georg Kotheimer, qemu-devel, qemu-riscv; +Cc: Alistair Francis
On 3/16/21 9:03 AM, Georg Kotheimer wrote:
> When decode_insn16() fails, we fall back to decode_RV32_64C() for
> further compressed instruction decoding.
I think this is all dead code now. Certainly c.ld/c.sd are in insn16-64.decode
and c.flw/c.fsw are in insn16-32.decode.
Digging, we failed to remove these functions here: f330433b363.
You are absolutely right there's a missing
> if (!decode_insn16(ctx, opcode)) {
> /* fall back to old decoder */
> - decode_RV32_64C(ctx, opcode);
> + if (!decode_RV32_64C(ctx, opcode)) {
> + gen_exception_illegal(ctx);
exception here, but we can remove the last remnants of the old decoder instead
of patching them.
r~
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-03-19 15:23 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-16 15:03 [PATCH] target/riscv: Prevent lost illegal instruction exceptions Georg Kotheimer
2021-03-16 15:03 ` Georg Kotheimer
2021-03-19 13:52 ` Alistair Francis
2021-03-19 13:52 ` Alistair Francis
2021-03-19 15:22 ` Richard Henderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.