* [PATCH] target/riscv: Set pc_succ_insn for !rvc illegal insn
@ 2022-12-03 17:57 Richard Henderson
2022-12-07 5:36 ` Alistair Francis
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Richard Henderson @ 2022-12-03 17:57 UTC (permalink / raw)
To: qemu-devel; +Cc: alistair.francis, bin.meng, qemu-riscv, qemu-stable
Failure to set pc_succ_insn may result in a TB covering zero bytes,
which triggers an assert within the code generator.
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1224
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/riscv/translate.c | 12 ++++--------
tests/tcg/Makefile.target | 2 ++
tests/tcg/riscv64/Makefile.target | 5 +++++
tests/tcg/riscv64/test-noc.S | 32 +++++++++++++++++++++++++++++++
4 files changed, 43 insertions(+), 8 deletions(-)
create mode 100644 tests/tcg/riscv64/test-noc.S
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index db123da5ec..1ed4bb5ec3 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -1064,14 +1064,10 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
/* Check for compressed insn */
if (insn_len(opcode) == 2) {
- 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)) {
- return;
- }
+ ctx->opcode = opcode;
+ ctx->pc_succ_insn = ctx->base.pc_next + 2;
+ if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
+ return;
}
} else {
uint32_t opcode32 = opcode;
diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
index 75257f2b29..14bc013181 100644
--- a/tests/tcg/Makefile.target
+++ b/tests/tcg/Makefile.target
@@ -117,6 +117,8 @@ endif
%: %.c
$(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
+%: %.S
+ $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
else
# For softmmu targets we include a different Makefile fragement as the
# build options for bare programs are usually pretty different. They
diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
index b5b89dfb0e..9973ba3b5f 100644
--- a/tests/tcg/riscv64/Makefile.target
+++ b/tests/tcg/riscv64/Makefile.target
@@ -4,3 +4,8 @@
VPATH += $(SRC_PATH)/tests/tcg/riscv64
TESTS += test-div
TESTS += noexec
+
+# Disable compressed instructions for test-noc
+TESTS += test-noc
+test-noc: LDFLAGS = -nostdlib -static
+run-test-noc: QEMU_OPTS += -cpu rv64,c=false
diff --git a/tests/tcg/riscv64/test-noc.S b/tests/tcg/riscv64/test-noc.S
new file mode 100644
index 0000000000..e29d60c8b3
--- /dev/null
+++ b/tests/tcg/riscv64/test-noc.S
@@ -0,0 +1,32 @@
+#include <asm/unistd.h>
+
+ .text
+ .globl _start
+_start:
+ .option norvc
+ li a0, 4 /* SIGILL */
+ la a1, sa
+ li a2, 0
+ li a3, 8
+ li a7, __NR_rt_sigaction
+ scall
+
+ .option rvc
+ li a0, 1
+ j exit
+ .option norvc
+
+pass:
+ li a0, 0
+exit:
+ li a7, __NR_exit
+ scall
+
+ .data
+ /* struct kernel_sigaction sa = { .sa_handler = pass }; */
+ .type sa, @object
+ .size sa, 32
+sa:
+ .dword pass
+ .zero 24
+
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: Set pc_succ_insn for !rvc illegal insn
2022-12-03 17:57 [PATCH] target/riscv: Set pc_succ_insn for !rvc illegal insn Richard Henderson
@ 2022-12-07 5:36 ` Alistair Francis
2022-12-07 6:45 ` Alistair Francis
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2022-12-07 5:36 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel, alistair.francis, bin.meng, qemu-riscv, qemu-stable
On Sun, Dec 4, 2022 at 3:58 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Failure to set pc_succ_insn may result in a TB covering zero bytes,
> which triggers an assert within the code generator.
>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1224
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Alistair
> ---
> target/riscv/translate.c | 12 ++++--------
> tests/tcg/Makefile.target | 2 ++
> tests/tcg/riscv64/Makefile.target | 5 +++++
> tests/tcg/riscv64/test-noc.S | 32 +++++++++++++++++++++++++++++++
> 4 files changed, 43 insertions(+), 8 deletions(-)
> create mode 100644 tests/tcg/riscv64/test-noc.S
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index db123da5ec..1ed4bb5ec3 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1064,14 +1064,10 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
>
> /* Check for compressed insn */
> if (insn_len(opcode) == 2) {
> - 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)) {
> - return;
> - }
> + ctx->opcode = opcode;
> + ctx->pc_succ_insn = ctx->base.pc_next + 2;
> + if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
> + return;
> }
> } else {
> uint32_t opcode32 = opcode;
> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
> index 75257f2b29..14bc013181 100644
> --- a/tests/tcg/Makefile.target
> +++ b/tests/tcg/Makefile.target
> @@ -117,6 +117,8 @@ endif
>
> %: %.c
> $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
> +%: %.S
> + $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
> else
> # For softmmu targets we include a different Makefile fragement as the
> # build options for bare programs are usually pretty different. They
> diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
> index b5b89dfb0e..9973ba3b5f 100644
> --- a/tests/tcg/riscv64/Makefile.target
> +++ b/tests/tcg/riscv64/Makefile.target
> @@ -4,3 +4,8 @@
> VPATH += $(SRC_PATH)/tests/tcg/riscv64
> TESTS += test-div
> TESTS += noexec
> +
> +# Disable compressed instructions for test-noc
> +TESTS += test-noc
> +test-noc: LDFLAGS = -nostdlib -static
> +run-test-noc: QEMU_OPTS += -cpu rv64,c=false
> diff --git a/tests/tcg/riscv64/test-noc.S b/tests/tcg/riscv64/test-noc.S
> new file mode 100644
> index 0000000000..e29d60c8b3
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-noc.S
> @@ -0,0 +1,32 @@
> +#include <asm/unistd.h>
> +
> + .text
> + .globl _start
> +_start:
> + .option norvc
> + li a0, 4 /* SIGILL */
> + la a1, sa
> + li a2, 0
> + li a3, 8
> + li a7, __NR_rt_sigaction
> + scall
> +
> + .option rvc
> + li a0, 1
> + j exit
> + .option norvc
> +
> +pass:
> + li a0, 0
> +exit:
> + li a7, __NR_exit
> + scall
> +
> + .data
> + /* struct kernel_sigaction sa = { .sa_handler = pass }; */
> + .type sa, @object
> + .size sa, 32
> +sa:
> + .dword pass
> + .zero 24
> +
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: Set pc_succ_insn for !rvc illegal insn
2022-12-03 17:57 [PATCH] target/riscv: Set pc_succ_insn for !rvc illegal insn Richard Henderson
2022-12-07 5:36 ` Alistair Francis
@ 2022-12-07 6:45 ` Alistair Francis
2022-12-07 7:00 ` Philippe Mathieu-Daudé
2022-12-21 6:20 ` Alistair Francis
3 siblings, 0 replies; 6+ messages in thread
From: Alistair Francis @ 2022-12-07 6:45 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel, alistair.francis, bin.meng, qemu-riscv, qemu-stable
On Sun, Dec 4, 2022 at 3:58 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Failure to set pc_succ_insn may result in a TB covering zero bytes,
> which triggers an assert within the code generator.
>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1224
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Thanks!
Applied to riscv-to-apply.next
Alistair
> ---
> target/riscv/translate.c | 12 ++++--------
> tests/tcg/Makefile.target | 2 ++
> tests/tcg/riscv64/Makefile.target | 5 +++++
> tests/tcg/riscv64/test-noc.S | 32 +++++++++++++++++++++++++++++++
> 4 files changed, 43 insertions(+), 8 deletions(-)
> create mode 100644 tests/tcg/riscv64/test-noc.S
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index db123da5ec..1ed4bb5ec3 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1064,14 +1064,10 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
>
> /* Check for compressed insn */
> if (insn_len(opcode) == 2) {
> - 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)) {
> - return;
> - }
> + ctx->opcode = opcode;
> + ctx->pc_succ_insn = ctx->base.pc_next + 2;
> + if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
> + return;
> }
> } else {
> uint32_t opcode32 = opcode;
> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
> index 75257f2b29..14bc013181 100644
> --- a/tests/tcg/Makefile.target
> +++ b/tests/tcg/Makefile.target
> @@ -117,6 +117,8 @@ endif
>
> %: %.c
> $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
> +%: %.S
> + $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
> else
> # For softmmu targets we include a different Makefile fragement as the
> # build options for bare programs are usually pretty different. They
> diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
> index b5b89dfb0e..9973ba3b5f 100644
> --- a/tests/tcg/riscv64/Makefile.target
> +++ b/tests/tcg/riscv64/Makefile.target
> @@ -4,3 +4,8 @@
> VPATH += $(SRC_PATH)/tests/tcg/riscv64
> TESTS += test-div
> TESTS += noexec
> +
> +# Disable compressed instructions for test-noc
> +TESTS += test-noc
> +test-noc: LDFLAGS = -nostdlib -static
> +run-test-noc: QEMU_OPTS += -cpu rv64,c=false
> diff --git a/tests/tcg/riscv64/test-noc.S b/tests/tcg/riscv64/test-noc.S
> new file mode 100644
> index 0000000000..e29d60c8b3
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-noc.S
> @@ -0,0 +1,32 @@
> +#include <asm/unistd.h>
> +
> + .text
> + .globl _start
> +_start:
> + .option norvc
> + li a0, 4 /* SIGILL */
> + la a1, sa
> + li a2, 0
> + li a3, 8
> + li a7, __NR_rt_sigaction
> + scall
> +
> + .option rvc
> + li a0, 1
> + j exit
> + .option norvc
> +
> +pass:
> + li a0, 0
> +exit:
> + li a7, __NR_exit
> + scall
> +
> + .data
> + /* struct kernel_sigaction sa = { .sa_handler = pass }; */
> + .type sa, @object
> + .size sa, 32
> +sa:
> + .dword pass
> + .zero 24
> +
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: Set pc_succ_insn for !rvc illegal insn
2022-12-03 17:57 [PATCH] target/riscv: Set pc_succ_insn for !rvc illegal insn Richard Henderson
2022-12-07 5:36 ` Alistair Francis
2022-12-07 6:45 ` Alistair Francis
@ 2022-12-07 7:00 ` Philippe Mathieu-Daudé
2022-12-21 6:20 ` Alistair Francis
3 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-07 7:00 UTC (permalink / raw)
To: Richard Henderson, qemu-devel
Cc: alistair.francis, bin.meng, qemu-riscv, qemu-stable
On 3/12/22 18:57, Richard Henderson wrote:
> Failure to set pc_succ_insn may result in a TB covering zero bytes,
> which triggers an assert within the code generator.
>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1224
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/riscv/translate.c | 12 ++++--------
> tests/tcg/Makefile.target | 2 ++
> tests/tcg/riscv64/Makefile.target | 5 +++++
> tests/tcg/riscv64/test-noc.S | 32 +++++++++++++++++++++++++++++++
> 4 files changed, 43 insertions(+), 8 deletions(-)
> create mode 100644 tests/tcg/riscv64/test-noc.S
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: Set pc_succ_insn for !rvc illegal insn
2022-12-03 17:57 [PATCH] target/riscv: Set pc_succ_insn for !rvc illegal insn Richard Henderson
` (2 preceding siblings ...)
2022-12-07 7:00 ` Philippe Mathieu-Daudé
@ 2022-12-21 6:20 ` Alistair Francis
2022-12-21 17:08 ` Richard Henderson
3 siblings, 1 reply; 6+ messages in thread
From: Alistair Francis @ 2022-12-21 6:20 UTC (permalink / raw)
To: Richard Henderson
Cc: qemu-devel, alistair.francis, bin.meng, qemu-riscv, qemu-stable
On Sun, Dec 4, 2022 at 3:58 AM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Failure to set pc_succ_insn may result in a TB covering zero bytes,
> which triggers an assert within the code generator.
>
> Cc: qemu-stable@nongnu.org
> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1224
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/riscv/translate.c | 12 ++++--------
> tests/tcg/Makefile.target | 2 ++
> tests/tcg/riscv64/Makefile.target | 5 +++++
> tests/tcg/riscv64/test-noc.S | 32 +++++++++++++++++++++++++++++++
> 4 files changed, 43 insertions(+), 8 deletions(-)
> create mode 100644 tests/tcg/riscv64/test-noc.S
>
> diff --git a/target/riscv/translate.c b/target/riscv/translate.c
> index db123da5ec..1ed4bb5ec3 100644
> --- a/target/riscv/translate.c
> +++ b/target/riscv/translate.c
> @@ -1064,14 +1064,10 @@ static void decode_opc(CPURISCVState *env, DisasContext *ctx, uint16_t opcode)
>
> /* Check for compressed insn */
> if (insn_len(opcode) == 2) {
> - 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)) {
> - return;
> - }
> + ctx->opcode = opcode;
> + ctx->pc_succ_insn = ctx->base.pc_next + 2;
> + if (has_ext(ctx, RVC) && decode_insn16(ctx, opcode)) {
> + return;
> }
> } else {
> uint32_t opcode32 = opcode;
> diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target
> index 75257f2b29..14bc013181 100644
> --- a/tests/tcg/Makefile.target
> +++ b/tests/tcg/Makefile.target
> @@ -117,6 +117,8 @@ endif
>
> %: %.c
> $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
> +%: %.S
> + $(CC) $(CFLAGS) $(EXTRA_CFLAGS) $< -o $@ $(LDFLAGS)
> else
> # For softmmu targets we include a different Makefile fragement as the
> # build options for bare programs are usually pretty different. They
> diff --git a/tests/tcg/riscv64/Makefile.target b/tests/tcg/riscv64/Makefile.target
> index b5b89dfb0e..9973ba3b5f 100644
> --- a/tests/tcg/riscv64/Makefile.target
> +++ b/tests/tcg/riscv64/Makefile.target
> @@ -4,3 +4,8 @@
> VPATH += $(SRC_PATH)/tests/tcg/riscv64
> TESTS += test-div
> TESTS += noexec
> +
> +# Disable compressed instructions for test-noc
> +TESTS += test-noc
> +test-noc: LDFLAGS = -nostdlib -static
> +run-test-noc: QEMU_OPTS += -cpu rv64,c=false
This fails the `make check-tcg` for Linux user mode when testing plugins.
This diff is required to get it working. I have applied the change myself
diff --git a/tests/tcg/riscv64/Makefile.target
b/tests/tcg/riscv64/Makefile.target
index 9973ba3b5f..cc3ed65ffd 100644
--- a/tests/tcg/riscv64/Makefile.target
+++ b/tests/tcg/riscv64/Makefile.target
@@ -9,3 +9,4 @@ TESTS += noexec
TESTS += test-noc
test-noc: LDFLAGS = -nostdlib -static
run-test-noc: QEMU_OPTS += -cpu rv64,c=false
+run-plugin-test-noc-%: QEMU_OPTS += -cpu rv64,c=false
Alistair
> diff --git a/tests/tcg/riscv64/test-noc.S b/tests/tcg/riscv64/test-noc.S
> new file mode 100644
> index 0000000000..e29d60c8b3
> --- /dev/null
> +++ b/tests/tcg/riscv64/test-noc.S
> @@ -0,0 +1,32 @@
> +#include <asm/unistd.h>
> +
> + .text
> + .globl _start
> +_start:
> + .option norvc
> + li a0, 4 /* SIGILL */
> + la a1, sa
> + li a2, 0
> + li a3, 8
> + li a7, __NR_rt_sigaction
> + scall
> +
> + .option rvc
> + li a0, 1
> + j exit
> + .option norvc
> +
> +pass:
> + li a0, 0
> +exit:
> + li a7, __NR_exit
> + scall
> +
> + .data
> + /* struct kernel_sigaction sa = { .sa_handler = pass }; */
> + .type sa, @object
> + .size sa, 32
> +sa:
> + .dword pass
> + .zero 24
> +
> --
> 2.34.1
>
>
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] target/riscv: Set pc_succ_insn for !rvc illegal insn
2022-12-21 6:20 ` Alistair Francis
@ 2022-12-21 17:08 ` Richard Henderson
0 siblings, 0 replies; 6+ messages in thread
From: Richard Henderson @ 2022-12-21 17:08 UTC (permalink / raw)
To: Alistair Francis
Cc: qemu-devel, alistair.francis, bin.meng, qemu-riscv, qemu-stable
On 12/20/22 22:20, Alistair Francis wrote:
>> +# Disable compressed instructions for test-noc
>> +TESTS += test-noc
>> +test-noc: LDFLAGS = -nostdlib -static
>> +run-test-noc: QEMU_OPTS += -cpu rv64,c=false
>
> This fails the `make check-tcg` for Linux user mode when testing plugins.
>
> This diff is required to get it working. I have applied the change myself
>
> diff --git a/tests/tcg/riscv64/Makefile.target
> b/tests/tcg/riscv64/Makefile.target
> index 9973ba3b5f..cc3ed65ffd 100644
> --- a/tests/tcg/riscv64/Makefile.target
> +++ b/tests/tcg/riscv64/Makefile.target
> @@ -9,3 +9,4 @@ TESTS += noexec
> TESTS += test-noc
> test-noc: LDFLAGS = -nostdlib -static
> run-test-noc: QEMU_OPTS += -cpu rv64,c=false
> +run-plugin-test-noc-%: QEMU_OPTS += -cpu rv64,c=false
Oops, thanks.
r~
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-21 17:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-03 17:57 [PATCH] target/riscv: Set pc_succ_insn for !rvc illegal insn Richard Henderson
2022-12-07 5:36 ` Alistair Francis
2022-12-07 6:45 ` Alistair Francis
2022-12-07 7:00 ` Philippe Mathieu-Daudé
2022-12-21 6:20 ` Alistair Francis
2022-12-21 17:08 ` 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).