From e25fe886b89a396bae5847520b70c148587d490a Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Thu, 4 Apr 2024 20:53:50 +0000 Subject: tcg/optimize: Do not attempt to constant fold neg_vec Split out the tail of fold_neg to fold_neg_no_const so that we can avoid attempting to constant fold vector negate. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2150 Signed-off-by: Richard Henderson --- tcg/optimize.c | 17 ++++++++--------- tests/tcg/aarch64/Makefile.target | 2 +- tests/tcg/aarch64/test-2150.c | 12 ++++++++++++ 3 files changed, 21 insertions(+), 10 deletions(-) create mode 100644 tests/tcg/aarch64/test-2150.c diff --git a/tcg/optimize.c b/tcg/optimize.c index 275db77b42..2e9e5725a9 100644 --- a/tcg/optimize.c +++ b/tcg/optimize.c @@ -1990,16 +1990,10 @@ static bool fold_nand(OptContext *ctx, TCGOp *op) return false; } -static bool fold_neg(OptContext *ctx, TCGOp *op) +static bool fold_neg_no_const(OptContext *ctx, TCGOp *op) { - uint64_t z_mask; - - if (fold_const1(ctx, op)) { - return true; - } - /* Set to 1 all bits to the left of the rightmost. */ - z_mask = arg_info(op->args[1])->z_mask; + uint64_t z_mask = arg_info(op->args[1])->z_mask; ctx->z_mask = -(z_mask & -z_mask); /* @@ -2010,6 +2004,11 @@ static bool fold_neg(OptContext *ctx, TCGOp *op) return true; } +static bool fold_neg(OptContext *ctx, TCGOp *op) +{ + return fold_const1(ctx, op) || fold_neg_no_const(ctx, op); +} + static bool fold_nor(OptContext *ctx, TCGOp *op) { if (fold_const2_commutative(ctx, op) || @@ -2418,7 +2417,7 @@ static bool fold_sub_to_neg(OptContext *ctx, TCGOp *op) if (have_neg) { op->opc = neg_op; op->args[1] = op->args[2]; - return fold_neg(ctx, op); + return fold_neg_no_const(ctx, op); } return false; } diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target index 0efd565f05..70d728ae9a 100644 --- a/tests/tcg/aarch64/Makefile.target +++ b/tests/tcg/aarch64/Makefile.target @@ -10,7 +10,7 @@ VPATH += $(AARCH64_SRC) # Base architecture tests AARCH64_TESTS=fcvt pcalign-a64 lse2-fault -AARCH64_TESTS += test-2248 +AARCH64_TESTS += test-2248 test-2150 fcvt: LDFLAGS+=-lm diff --git a/tests/tcg/aarch64/test-2150.c b/tests/tcg/aarch64/test-2150.c new file mode 100644 index 0000000000..fb86c11958 --- /dev/null +++ b/tests/tcg/aarch64/test-2150.c @@ -0,0 +1,12 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* See https://gitlab.com/qemu-project/qemu/-/issues/2150 */ + +int main() +{ + asm volatile( + "movi v6.4s, #1\n" + "movi v7.4s, #0\n" + "sub v6.2d, v7.2d, v6.2d\n" + : : : "v6", "v7"); + return 0; +} -- cgit v1.2.3 From f0907ff4cae743f1a4ef3d0a55a047029eed06ff Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Fri, 5 Apr 2024 11:58:14 -1000 Subject: linux-user: Fix waitid return of siginfo_t and rusage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The copy back to siginfo_t should be conditional only on arg3, not the specific values that might have been written. The copy back to rusage was missing entirely. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2262 Signed-off-by: Richard Henderson Tested-by: Alex Fan Reviewed-by: Philippe Mathieu-Daudé --- linux-user/syscall.c | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index e12d969c2e..3df2b94d9a 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -9272,14 +9272,24 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int num, abi_long arg1, #ifdef TARGET_NR_waitid case TARGET_NR_waitid: { + struct rusage ru; siginfo_t info; - info.si_pid = 0; - ret = get_errno(safe_waitid(arg1, arg2, &info, arg4, NULL)); - if (!is_error(ret) && arg3 && info.si_pid != 0) { - if (!(p = lock_user(VERIFY_WRITE, arg3, sizeof(target_siginfo_t), 0))) + + ret = get_errno(safe_waitid(arg1, arg2, (arg3 ? &info : NULL), + arg4, (arg5 ? &ru : NULL))); + if (!is_error(ret)) { + if (arg3) { + p = lock_user(VERIFY_WRITE, arg3, + sizeof(target_siginfo_t), 0); + if (!p) { + return -TARGET_EFAULT; + } + host_to_target_siginfo(p, &info); + unlock_user(p, arg3, sizeof(target_siginfo_t)); + } + if (arg5 && host_to_target_rusage(arg5, &ru)) { return -TARGET_EFAULT; - host_to_target_siginfo(p, &info); - unlock_user(p, arg3, sizeof(target_siginfo_t)); + } } } return ret; -- cgit v1.2.3 From 2ee80bce4f7cf2ef78f996ea608755d4e97c94de Mon Sep 17 00:00:00 2001 From: Nguyen Dinh Phi Date: Mon, 18 Mar 2024 01:17:47 +0800 Subject: linux-user: replace calloc() with g_new0() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use glib allocation as recommended by the coding convention Signed-off-by: Nguyen Dinh Phi Message-Id: <20240317171747.1642207-1-phind.uet@gmail.com> Reviewed-by: Alex Bennée Signed-off-by: Richard Henderson --- linux-user/main.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/linux-user/main.c b/linux-user/main.c index 9277df2e9d..149e35432e 100644 --- a/linux-user/main.c +++ b/linux-user/main.c @@ -928,11 +928,7 @@ int main(int argc, char **argv, char **envp) * Prepare copy of argv vector for target. */ target_argc = argc - optind; - target_argv = calloc(target_argc + 1, sizeof (char *)); - if (target_argv == NULL) { - (void) fprintf(stderr, "Unable to allocate memory for target_argv\n"); - exit(EXIT_FAILURE); - } + target_argv = g_new0(char *, target_argc + 1); /* * If argv0 is specified (using '-0' switch) we replace -- cgit v1.2.3 From 26d937237f363297e5f70efcb001b15b1cb0fc2b Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 1 Apr 2024 14:52:39 -1000 Subject: target/hppa: Fix IIAOQ, IIASQ for pa2.0 The contents of IIAOQ depend on PSW_W. Follow the text in "Interruption Instruction Address Queues", pages 2-13 through 2-15. Tested-by: Sven Schnelle Tested-by: Helge Deller Reported-by: Sven Schnelle Fixes: b10700d826c ("target/hppa: Update IIAOQ, IIASQ for pa2.0") Signed-off-by: Richard Henderson --- target/hppa/int_helper.c | 20 +++++++++++--------- target/hppa/sys_helper.c | 18 +++++++++--------- 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/target/hppa/int_helper.c b/target/hppa/int_helper.c index 90437a92cd..a667ee380d 100644 --- a/target/hppa/int_helper.c +++ b/target/hppa/int_helper.c @@ -107,14 +107,10 @@ void hppa_cpu_do_interrupt(CPUState *cs) /* step 3 */ /* - * For pa1.x, IIASQ is simply a copy of IASQ. - * For pa2.0, IIASQ is the top bits of the virtual address, - * or zero if translation is disabled. + * IIASQ is the top bits of the virtual address, or zero if translation + * is disabled -- with PSW_W == 0, this will reduce to the space. */ - if (!hppa_is_pa20(env)) { - env->cr[CR_IIASQ] = env->iasq_f >> 32; - env->cr_back[0] = env->iasq_b >> 32; - } else if (old_psw & PSW_C) { + if (old_psw & PSW_C) { env->cr[CR_IIASQ] = hppa_form_gva_psw(old_psw, env->iasq_f, env->iaoq_f) >> 32; env->cr_back[0] = @@ -123,8 +119,14 @@ void hppa_cpu_do_interrupt(CPUState *cs) env->cr[CR_IIASQ] = 0; env->cr_back[0] = 0; } - env->cr[CR_IIAOQ] = env->iaoq_f; - env->cr_back[1] = env->iaoq_b; + /* IIAOQ is the full offset for wide mode, or 32 bits for narrow mode. */ + if (old_psw & PSW_W) { + env->cr[CR_IIAOQ] = env->iaoq_f; + env->cr_back[1] = env->iaoq_b; + } else { + env->cr[CR_IIAOQ] = (uint32_t)env->iaoq_f; + env->cr_back[1] = (uint32_t)env->iaoq_b; + } if (old_psw & PSW_Q) { /* step 5 */ diff --git a/target/hppa/sys_helper.c b/target/hppa/sys_helper.c index 208e51c086..22d6c89964 100644 --- a/target/hppa/sys_helper.c +++ b/target/hppa/sys_helper.c @@ -78,21 +78,21 @@ target_ulong HELPER(swap_system_mask)(CPUHPPAState *env, target_ulong nsm) void HELPER(rfi)(CPUHPPAState *env) { - env->iasq_f = (uint64_t)env->cr[CR_IIASQ] << 32; - env->iasq_b = (uint64_t)env->cr_back[0] << 32; - env->iaoq_f = env->cr[CR_IIAOQ]; - env->iaoq_b = env->cr_back[1]; + uint64_t mask; + + cpu_hppa_put_psw(env, env->cr[CR_IPSW]); /* * For pa2.0, IIASQ is the top bits of the virtual address. * To recreate the space identifier, remove the offset bits. + * For pa1.x, the mask reduces to no change to space. */ - if (hppa_is_pa20(env)) { - env->iasq_f &= ~env->iaoq_f; - env->iasq_b &= ~env->iaoq_b; - } + mask = gva_offset_mask(env->psw); - cpu_hppa_put_psw(env, env->cr[CR_IPSW]); + env->iaoq_f = env->cr[CR_IIAOQ]; + env->iaoq_b = env->cr_back[1]; + env->iasq_f = (env->cr[CR_IIASQ] << 32) & ~(env->iaoq_f & mask); + env->iasq_b = (env->cr_back[0] << 32) & ~(env->iaoq_b & mask); } static void getshadowregs(CPUHPPAState *env) -- cgit v1.2.3 From b0f2f2976b4db05351117b0440b32bf0aac2c5c6 Mon Sep 17 00:00:00 2001 From: Zack Buhman Date: Tue, 2 Apr 2024 17:37:49 +0800 Subject: target/sh4: mac.w: memory accesses are 16-bit words MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this change, executing a code sequence such as: mova tblm,r0 mov r0,r1 mova tbln,r0 clrs clrmac mac.w @r0+,@r1+ mac.w @r0+,@r1+ .align 4 tblm: .word 0x1234 .word 0x5678 tbln: .word 0x9abc .word 0xdefg Does not result in correct behavior: Expected behavior: first macw : macl = 0x1234 * 0x9abc + 0x0 mach = 0x0 second macw: macl = 0x5678 * 0xdefg + 0xb00a630 mach = 0x0 Observed behavior (qemu-sh4eb, prior to this commit): first macw : macl = 0x5678 * 0xdefg + 0x0 mach = 0x0 second macw: (unaligned longword memory access, SIGBUS) Various SH-4 ISA manuals also confirm that `mac.w` is a 16-bit word memory access, not a 32-bit longword memory access. Signed-off-by: Zack Buhman Reviewed-by: Yoshinori Sato Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20240402093756.27466-1-zack@buhman.org> Signed-off-by: Richard Henderson --- target/sh4/translate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/sh4/translate.c b/target/sh4/translate.c index a9b1bc7524..6643c14dde 100644 --- a/target/sh4/translate.c +++ b/target/sh4/translate.c @@ -816,10 +816,10 @@ static void _decode_opc(DisasContext * ctx) TCGv arg0, arg1; arg0 = tcg_temp_new(); tcg_gen_qemu_ld_i32(arg0, REG(B7_4), ctx->memidx, - MO_TESL | MO_ALIGN); + MO_TESW | MO_ALIGN); arg1 = tcg_temp_new(); tcg_gen_qemu_ld_i32(arg1, REG(B11_8), ctx->memidx, - MO_TESL | MO_ALIGN); + MO_TESW | MO_ALIGN); gen_helper_macw(tcg_env, arg0, arg1); tcg_gen_addi_i32(REG(B11_8), REG(B11_8), 2); tcg_gen_addi_i32(REG(B7_4), REG(B7_4), 2); -- cgit v1.2.3 From 7d95db5e78a24d3315e3112d26909a7262355cb7 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Fri, 5 Apr 2024 17:31:05 -1000 Subject: target/sh4: Merge mach and macl into a union MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow host access to the entire 64-bit accumulator. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/sh4/cpu.h | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/target/sh4/cpu.h b/target/sh4/cpu.h index 9211da6bde..d928bcf006 100644 --- a/target/sh4/cpu.h +++ b/target/sh4/cpu.h @@ -155,12 +155,22 @@ typedef struct CPUArchState { uint32_t pc; /* program counter */ uint32_t delayed_pc; /* target of delayed branch */ uint32_t delayed_cond; /* condition of delayed branch */ - uint32_t mach; /* multiply and accumulate high */ - uint32_t macl; /* multiply and accumulate low */ uint32_t pr; /* procedure register */ uint32_t fpscr; /* floating point status/control register */ uint32_t fpul; /* floating point communication register */ + /* multiply and accumulate: high, low and combined. */ + union { + uint64_t mac; + struct { +#if HOST_BIG_ENDIAN + uint32_t mach, macl; +#else + uint32_t macl, mach; +#endif + }; + }; + /* float point status register */ float_status fp_status; -- cgit v1.2.3 From c97e8977dcacb3fa8362ee28bcee75ceb01fceaa Mon Sep 17 00:00:00 2001 From: Zack Buhman Date: Fri, 5 Apr 2024 15:17:39 -1000 Subject: target/sh4: Fix mac.l with saturation enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The saturation arithmetic logic in helper_macl is not correct. I tested and verified this behavior on a SH7091. Signed-off-by: Zack Buhman Message-Id: <20240404162641.27528-2-zack@buhman.org> [rth: Reformat helper_macl, add a test case.] Signed-off-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé --- target/sh4/helper.h | 2 +- target/sh4/op_helper.c | 23 ++++++++------- tests/tcg/sh4/Makefile.target | 5 ++++ tests/tcg/sh4/test-macl.c | 67 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 tests/tcg/sh4/test-macl.c diff --git a/target/sh4/helper.h b/target/sh4/helper.h index 8d792f6b55..64056e4a39 100644 --- a/target/sh4/helper.h +++ b/target/sh4/helper.h @@ -11,7 +11,7 @@ DEF_HELPER_3(movcal, void, env, i32, i32) DEF_HELPER_1(discard_movcal_backup, void, env) DEF_HELPER_2(ocbi, void, env, i32) -DEF_HELPER_3(macl, void, env, i32, i32) +DEF_HELPER_3(macl, void, env, s32, s32) DEF_HELPER_3(macw, void, env, i32, i32) DEF_HELPER_2(ld_fpscr, void, env, i32) diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c index 4559d0d376..d0bae0cc00 100644 --- a/target/sh4/op_helper.c +++ b/target/sh4/op_helper.c @@ -158,20 +158,23 @@ void helper_ocbi(CPUSH4State *env, uint32_t address) } } -void helper_macl(CPUSH4State *env, uint32_t arg0, uint32_t arg1) +void helper_macl(CPUSH4State *env, int32_t arg0, int32_t arg1) { + const int64_t min = -(1ll << 47); + const int64_t max = (1ll << 47) - 1; + int64_t mul = (int64_t)arg0 * arg1; + int64_t mac = env->mac; int64_t res; - res = ((uint64_t) env->mach << 32) | env->macl; - res += (int64_t) (int32_t) arg0 *(int64_t) (int32_t) arg1; - env->mach = (res >> 32) & 0xffffffff; - env->macl = res & 0xffffffff; - if (env->sr & (1u << SR_S)) { - if (res < 0) - env->mach |= 0xffff0000; - else - env->mach &= 0x00007fff; + if (!(env->sr & (1u << SR_S))) { + res = mac + mul; + } else if (sadd64_overflow(mac, mul, &res)) { + res = mac < 0 ? min : max; + } else { + res = MIN(MAX(res, min), max); } + + env->mac = res; } void helper_macw(CPUSH4State *env, uint32_t arg0, uint32_t arg1) diff --git a/tests/tcg/sh4/Makefile.target b/tests/tcg/sh4/Makefile.target index 16eaa850a8..9a11c10924 100644 --- a/tests/tcg/sh4/Makefile.target +++ b/tests/tcg/sh4/Makefile.target @@ -9,3 +9,8 @@ run-signals: signals $(call skip-test, $<, "BROKEN") run-plugin-signals-with-%: $(call skip-test, $<, "BROKEN") + +VPATH += $(SRC_PATH)/tests/tcg/sh4 + +test-macl: CFLAGS += -O -g +TESTS += test-macl diff --git a/tests/tcg/sh4/test-macl.c b/tests/tcg/sh4/test-macl.c new file mode 100644 index 0000000000..b66c854365 --- /dev/null +++ b/tests/tcg/sh4/test-macl.c @@ -0,0 +1,67 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include +#include +#include + +#define MACL_S_MIN (-(1ll << 47)) +#define MACL_S_MAX ((1ll << 47) - 1) + +int64_t mac_l(int64_t mac, const int32_t *a, const int32_t *b) +{ + register uint32_t macl __asm__("macl") = mac; + register uint32_t mach __asm__("mach") = mac >> 32; + + asm volatile("mac.l @%0+,@%1+" + : "+r"(a), "+r"(b), "+x"(macl), "+x"(mach)); + + return ((uint64_t)mach << 32) | macl; +} + +typedef struct { + int64_t mac; + int32_t a, b; + int64_t res[2]; +} Test; + +__attribute__((noinline)) +void test(const Test *t, int sat) +{ + int64_t res; + + if (sat) { + asm volatile("sets"); + } else { + asm volatile("clrs"); + } + res = mac_l(t->mac, &t->a, &t->b); + + if (res != t->res[sat]) { + fprintf(stderr, "%#llx + (%#x * %#x) = %#llx -- got %#llx\n", + t->mac, t->a, t->b, t->res[sat], res); + abort(); + } +} + +int main() +{ + static const Test tests[] = { + { 0x00007fff12345678ll, INT32_MAX, INT32_MAX, + { 0x40007ffe12345679ll, MACL_S_MAX } }, + { MACL_S_MIN, -1, 1, + { 0xffff7fffffffffffll, MACL_S_MIN } }, + { INT64_MIN, -1, 1, + { INT64_MAX, MACL_S_MIN } }, + { 0x00007fff00000000ll, INT32_MAX, INT32_MAX, + { 0x40007ffe00000001ll, MACL_S_MAX } }, + { 4, 1, 2, { 6, 6 } }, + { -4, -1, -2, { -2, -2 } }, + }; + + for (int i = 0; i < sizeof(tests) / sizeof(tests[0]); ++i) { + for (int j = 0; j < 2; ++j) { + test(&tests[i], j); + } + } + return 0; +} -- cgit v1.2.3 From 7227c0cd506eaab5b1d89d15832cac7e05ecb412 Mon Sep 17 00:00:00 2001 From: Zack Buhman Date: Fri, 5 Apr 2024 17:11:47 -1000 Subject: target/sh4: Fix mac.w with saturation enabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The saturation arithmetic logic in helper_macw is not correct. I tested and verified this behavior on a SH7091. Reviewd-by: Yoshinori Sato Signed-off-by: Zack Buhman Message-Id: <20240405233802.29128-3-zack@buhman.org> [rth: Reformat helper_macw, add a test case.] Signed-off-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé --- target/sh4/helper.h | 2 +- target/sh4/op_helper.c | 28 ++++++++++++-------- tests/tcg/sh4/Makefile.target | 3 +++ tests/tcg/sh4/test-macw.c | 61 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 82 insertions(+), 12 deletions(-) create mode 100644 tests/tcg/sh4/test-macw.c diff --git a/target/sh4/helper.h b/target/sh4/helper.h index 64056e4a39..29011d3dbb 100644 --- a/target/sh4/helper.h +++ b/target/sh4/helper.h @@ -12,7 +12,7 @@ DEF_HELPER_1(discard_movcal_backup, void, env) DEF_HELPER_2(ocbi, void, env, i32) DEF_HELPER_3(macl, void, env, s32, s32) -DEF_HELPER_3(macw, void, env, i32, i32) +DEF_HELPER_3(macw, void, env, s32, s32) DEF_HELPER_2(ld_fpscr, void, env, i32) diff --git a/target/sh4/op_helper.c b/target/sh4/op_helper.c index d0bae0cc00..99394b714c 100644 --- a/target/sh4/op_helper.c +++ b/target/sh4/op_helper.c @@ -177,22 +177,28 @@ void helper_macl(CPUSH4State *env, int32_t arg0, int32_t arg1) env->mac = res; } -void helper_macw(CPUSH4State *env, uint32_t arg0, uint32_t arg1) +void helper_macw(CPUSH4State *env, int32_t arg0, int32_t arg1) { - int64_t res; + /* Inputs are already sign-extended from 16 bits. */ + int32_t mul = arg0 * arg1; - res = ((uint64_t) env->mach << 32) | env->macl; - res += (int64_t) (int16_t) arg0 *(int64_t) (int16_t) arg1; - env->mach = (res >> 32) & 0xffffffff; - env->macl = res & 0xffffffff; if (env->sr & (1u << SR_S)) { - if (res < -0x80000000) { - env->mach = 1; - env->macl = 0x80000000; - } else if (res > 0x000000007fffffff) { + /* + * In saturation arithmetic mode, the accumulator is 32-bit + * with carry. MACH is not considered during the addition + * operation nor the 32-bit saturation logic. + */ + int32_t res, macl = env->macl; + + if (sadd32_overflow(macl, mul, &res)) { + res = macl < 0 ? INT32_MIN : INT32_MAX; + /* If overflow occurs, the MACH register is set to 1. */ env->mach = 1; - env->macl = 0x7fffffff; } + env->macl = res; + } else { + /* In non-saturation arithmetic mode, the accumulator is 64-bit */ + env->mac += mul; } } diff --git a/tests/tcg/sh4/Makefile.target b/tests/tcg/sh4/Makefile.target index 9a11c10924..4d09291c0c 100644 --- a/tests/tcg/sh4/Makefile.target +++ b/tests/tcg/sh4/Makefile.target @@ -14,3 +14,6 @@ VPATH += $(SRC_PATH)/tests/tcg/sh4 test-macl: CFLAGS += -O -g TESTS += test-macl + +test-macw: CFLAGS += -O -g +TESTS += test-macw diff --git a/tests/tcg/sh4/test-macw.c b/tests/tcg/sh4/test-macw.c new file mode 100644 index 0000000000..4eceec8634 --- /dev/null +++ b/tests/tcg/sh4/test-macw.c @@ -0,0 +1,61 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +#include +#include +#include + +int64_t mac_w(int64_t mac, const int16_t *a, const int16_t *b) +{ + register uint32_t macl __asm__("macl") = mac; + register uint32_t mach __asm__("mach") = mac >> 32; + + asm volatile("mac.w @%0+,@%1+" + : "+r"(a), "+r"(b), "+x"(macl), "+x"(mach)); + + return ((uint64_t)mach << 32) | macl; +} + +typedef struct { + int64_t mac; + int16_t a, b; + int64_t res[2]; +} Test; + +__attribute__((noinline)) +void test(const Test *t, int sat) +{ + int64_t res; + + if (sat) { + asm volatile("sets"); + } else { + asm volatile("clrs"); + } + res = mac_w(t->mac, &t->a, &t->b); + + if (res != t->res[sat]) { + fprintf(stderr, "%#llx + (%#x * %#x) = %#llx -- got %#llx\n", + t->mac, t->a, t->b, t->res[sat], res); + abort(); + } +} + +int main() +{ + static const Test tests[] = { + { 0, 2, 3, { 6, 6 } }, + { 0x123456787ffffffell, 2, -3, + { 0x123456787ffffff8ll, 0x123456787ffffff8ll } }, + { 0xabcdef127ffffffall, 2, 3, + { 0xabcdef1280000000ll, 0x000000017fffffffll } }, + { 0xfffffffffll, INT16_MAX, INT16_MAX, + { 0x103fff0000ll, 0xf3fff0000ll } }, + }; + + for (int i = 0; i < sizeof(tests) / sizeof(tests[0]); ++i) { + for (int j = 0; j < 2; ++j) { + test(&tests[i], j); + } + } + return 0; +} -- cgit v1.2.3 From b754cb2dcde26a7bc8a9d17bb6900a0ac0dd38e2 Mon Sep 17 00:00:00 2001 From: Zack Buhman Date: Sun, 7 Apr 2024 23:07:05 +0800 Subject: target/sh4: add missing CHECK_NOT_DELAY_SLOT CHECK_NOT_DELAY_SLOT is correctly applied to the branch-related instructions, but not to the PC-relative mov* instructions. I verified the existence of an illegal slot exception on a SH7091 when any of these instructions are attempted inside a delay slot. This also matches the behavior described in the SH-4 ISA manual. Signed-off-by: Zack Buhman Reviewed-by: Richard Henderson Message-Id: <20240407150705.5965-1-zack@buhman.org> Signed-off-by: Richard Henderson Reviewd-by: Yoshinori Sato --- target/sh4/translate.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/target/sh4/translate.c b/target/sh4/translate.c index 6643c14dde..ebb6c901bf 100644 --- a/target/sh4/translate.c +++ b/target/sh4/translate.c @@ -523,6 +523,7 @@ static void _decode_opc(DisasContext * ctx) tcg_gen_movi_i32(REG(B11_8), B7_0s); return; case 0x9000: /* mov.w @(disp,PC),Rn */ + CHECK_NOT_DELAY_SLOT { TCGv addr = tcg_constant_i32(ctx->base.pc_next + 4 + B7_0 * 2); tcg_gen_qemu_ld_i32(REG(B11_8), addr, ctx->memidx, @@ -530,6 +531,7 @@ static void _decode_opc(DisasContext * ctx) } return; case 0xd000: /* mov.l @(disp,PC),Rn */ + CHECK_NOT_DELAY_SLOT { TCGv addr = tcg_constant_i32((ctx->base.pc_next + 4 + B7_0 * 4) & ~3); tcg_gen_qemu_ld_i32(REG(B11_8), addr, ctx->memidx, @@ -1236,6 +1238,7 @@ static void _decode_opc(DisasContext * ctx) } return; case 0xc700: /* mova @(disp,PC),R0 */ + CHECK_NOT_DELAY_SLOT tcg_gen_movi_i32(REG(0), ((ctx->base.pc_next & 0xfffffffc) + 4 + B7_0 * 4) & ~3); return; -- cgit v1.2.3 From 5888357942da1fd5a50efb6e4a6af8b1a27a5af8 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Wed, 2 Aug 2023 20:52:31 -0700 Subject: target/m68k: Map FPU exceptions to FPSR register Add helpers for reading/writing the 68881 FPSR register so that changes in floating point exception state can be seen by the application. Call these helpers in pre_load/post_load hooks to synchronize exception state. Signed-off-by: Keith Packard Reviewed-by: Richard Henderson Message-Id: <20230803035231.429697-1-keithp@keithp.com> Signed-off-by: Richard Henderson --- target/m68k/cpu.c | 12 ++++++-- target/m68k/cpu.h | 3 +- target/m68k/fpu_helper.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++ target/m68k/helper.c | 4 +-- target/m68k/helper.h | 2 ++ target/m68k/translate.c | 4 +-- 6 files changed, 90 insertions(+), 7 deletions(-) diff --git a/target/m68k/cpu.c b/target/m68k/cpu.c index 7c8efbb42c..df49ff1880 100644 --- a/target/m68k/cpu.c +++ b/target/m68k/cpu.c @@ -390,12 +390,19 @@ static const VMStateDescription vmstate_freg = { } }; -static int fpu_post_load(void *opaque, int version) +static int fpu_pre_save(void *opaque) { M68kCPU *s = opaque; - cpu_m68k_restore_fp_status(&s->env); + s->env.fpsr = cpu_m68k_get_fpsr(&s->env); + return 0; +} + +static int fpu_post_load(void *opaque, int version) +{ + M68kCPU *s = opaque; + cpu_m68k_set_fpsr(&s->env, s->env.fpsr); return 0; } @@ -404,6 +411,7 @@ const VMStateDescription vmmstate_fpu = { .version_id = 1, .minimum_version_id = 1, .needed = fpu_needed, + .pre_save = fpu_pre_save, .post_load = fpu_post_load, .fields = (const VMStateField[]) { VMSTATE_UINT32(env.fpcr, M68kCPU), diff --git a/target/m68k/cpu.h b/target/m68k/cpu.h index 346427e144..e184239a81 100644 --- a/target/m68k/cpu.h +++ b/target/m68k/cpu.h @@ -199,7 +199,8 @@ void cpu_m68k_set_ccr(CPUM68KState *env, uint32_t); void cpu_m68k_set_sr(CPUM68KState *env, uint32_t); void cpu_m68k_restore_fp_status(CPUM68KState *env); void cpu_m68k_set_fpcr(CPUM68KState *env, uint32_t val); - +uint32_t cpu_m68k_get_fpsr(CPUM68KState *env); +void cpu_m68k_set_fpsr(CPUM68KState *env, uint32_t val); /* * Instead of computing the condition codes after each m68k instruction, diff --git a/target/m68k/fpu_helper.c b/target/m68k/fpu_helper.c index ab120b5f59..8314791f50 100644 --- a/target/m68k/fpu_helper.c +++ b/target/m68k/fpu_helper.c @@ -164,6 +164,78 @@ void HELPER(set_fpcr)(CPUM68KState *env, uint32_t val) cpu_m68k_set_fpcr(env, val); } +/* Convert host exception flags to cpu_m68k form. */ +static int cpu_m68k_exceptbits_from_host(int host_bits) +{ + int target_bits = 0; + + if (host_bits & float_flag_invalid) { + target_bits |= 0x80; + } + if (host_bits & float_flag_overflow) { + target_bits |= 0x40; + } + if (host_bits & (float_flag_underflow | float_flag_output_denormal)) { + target_bits |= 0x20; + } + if (host_bits & float_flag_divbyzero) { + target_bits |= 0x10; + } + if (host_bits & float_flag_inexact) { + target_bits |= 0x08; + } + return target_bits; +} + +/* Convert cpu_m68k exception flags to target form. */ +static int cpu_m68k_exceptbits_to_host(int target_bits) +{ + int host_bits = 0; + + if (target_bits & 0x80) { + host_bits |= float_flag_invalid; + } + if (target_bits & 0x40) { + host_bits |= float_flag_overflow; + } + if (target_bits & 0x20) { + host_bits |= float_flag_underflow; + } + if (target_bits & 0x10) { + host_bits |= float_flag_divbyzero; + } + if (target_bits & 0x08) { + host_bits |= float_flag_inexact; + } + return host_bits; +} + +uint32_t cpu_m68k_get_fpsr(CPUM68KState *env) +{ + int host_flags = get_float_exception_flags(&env->fp_status); + int target_flags = cpu_m68k_exceptbits_from_host(host_flags); + int except = (env->fpsr & ~(0xf8)) | target_flags; + return except; +} + +uint32_t HELPER(get_fpsr)(CPUM68KState *env) +{ + return cpu_m68k_get_fpsr(env); +} + +void cpu_m68k_set_fpsr(CPUM68KState *env, uint32_t val) +{ + env->fpsr = val; + + int host_flags = cpu_m68k_exceptbits_to_host((int) env->fpsr); + set_float_exception_flags(host_flags, &env->fp_status); +} + +void HELPER(set_fpsr)(CPUM68KState *env, uint32_t val) +{ + cpu_m68k_set_fpsr(env, val); +} + #define PREC_BEGIN(prec) \ do { \ FloatX80RoundPrec old = \ diff --git a/target/m68k/helper.c b/target/m68k/helper.c index 1a475f082a..7a91f33b17 100644 --- a/target/m68k/helper.c +++ b/target/m68k/helper.c @@ -87,7 +87,7 @@ static int m68k_fpu_gdb_get_reg(CPUState *cs, GByteArray *mem_buf, int n) case 8: /* fpcontrol */ return gdb_get_reg32(mem_buf, env->fpcr); case 9: /* fpstatus */ - return gdb_get_reg32(mem_buf, env->fpsr); + return gdb_get_reg32(mem_buf, cpu_m68k_get_fpsr(env)); case 10: /* fpiar, not implemented */ return gdb_get_reg32(mem_buf, 0); } @@ -109,7 +109,7 @@ static int m68k_fpu_gdb_set_reg(CPUState *cs, uint8_t *mem_buf, int n) cpu_m68k_set_fpcr(env, ldl_p(mem_buf)); return 4; case 9: /* fpstatus */ - env->fpsr = ldl_p(mem_buf); + cpu_m68k_set_fpsr(env, ldl_p(mem_buf)); return 4; case 10: /* fpiar, not implemented */ return 4; diff --git a/target/m68k/helper.h b/target/m68k/helper.h index 2bbe0dc032..95aa5e53bb 100644 --- a/target/m68k/helper.h +++ b/target/m68k/helper.h @@ -54,6 +54,8 @@ DEF_HELPER_4(fsdiv, void, env, fp, fp, fp) DEF_HELPER_4(fddiv, void, env, fp, fp, fp) DEF_HELPER_4(fsgldiv, void, env, fp, fp, fp) DEF_HELPER_FLAGS_3(fcmp, TCG_CALL_NO_RWG, void, env, fp, fp) +DEF_HELPER_2(set_fpsr, void, env, i32) +DEF_HELPER_1(get_fpsr, i32, env) DEF_HELPER_FLAGS_2(set_fpcr, TCG_CALL_NO_RWG, void, env, i32) DEF_HELPER_FLAGS_2(ftst, TCG_CALL_NO_RWG, void, env, fp) DEF_HELPER_3(fconst, void, env, fp, i32) diff --git a/target/m68k/translate.c b/target/m68k/translate.c index 6ae3df43bc..8a194f2f21 100644 --- a/target/m68k/translate.c +++ b/target/m68k/translate.c @@ -4686,7 +4686,7 @@ static void gen_load_fcr(DisasContext *s, TCGv res, int reg) tcg_gen_movi_i32(res, 0); break; case M68K_FPSR: - tcg_gen_ld_i32(res, tcg_env, offsetof(CPUM68KState, fpsr)); + gen_helper_get_fpsr(res, tcg_env); break; case M68K_FPCR: tcg_gen_ld_i32(res, tcg_env, offsetof(CPUM68KState, fpcr)); @@ -4700,7 +4700,7 @@ static void gen_store_fcr(DisasContext *s, TCGv val, int reg) case M68K_FPIAR: break; case M68K_FPSR: - tcg_gen_st_i32(val, tcg_env, offsetof(CPUM68KState, fpsr)); + gen_helper_set_fpsr(tcg_env, val); break; case M68K_FPCR: gen_helper_set_fpcr(tcg_env, val); -- cgit v1.2.3 From 07843f75fdaff26950e4ddfd5d03556915fee1a7 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Wed, 13 Mar 2024 13:32:29 -1000 Subject: tcg: Add TCGContext.emit_before_op MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Allow operations to be emitted via normal expanders into the middle of the opcode stream. Tested-by: Jørgen Hansen Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Pierrick Bouvier Signed-off-by: Richard Henderson --- include/tcg/tcg.h | 6 ++++++ tcg/tcg.c | 14 ++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h index 451f3fec41..05a1912f8a 100644 --- a/include/tcg/tcg.h +++ b/include/tcg/tcg.h @@ -553,6 +553,12 @@ struct TCGContext { QTAILQ_HEAD(, TCGOp) ops, free_ops; QSIMPLEQ_HEAD(, TCGLabel) labels; + /* + * When clear, new ops are added to the tail of @ops. + * When set, new ops are added in front of @emit_before_op. + */ + TCGOp *emit_before_op; + /* Tells which temporary holds a given register. It does not take into account fixed registers */ TCGTemp *reg_to_temp[TCG_TARGET_NB_REGS]; diff --git a/tcg/tcg.c b/tcg/tcg.c index d6670237fb..0c0bb9d169 100644 --- a/tcg/tcg.c +++ b/tcg/tcg.c @@ -1521,6 +1521,7 @@ void tcg_func_start(TCGContext *s) QTAILQ_INIT(&s->ops); QTAILQ_INIT(&s->free_ops); + s->emit_before_op = NULL; QSIMPLEQ_INIT(&s->labels); tcg_debug_assert(s->addr_type == TCG_TYPE_I32 || @@ -2332,7 +2333,11 @@ static void tcg_gen_callN(TCGHelperInfo *info, TCGTemp *ret, TCGTemp **args) op->args[pi++] = (uintptr_t)info; tcg_debug_assert(pi == total_args); - QTAILQ_INSERT_TAIL(&tcg_ctx->ops, op, link); + if (tcg_ctx->emit_before_op) { + QTAILQ_INSERT_BEFORE(tcg_ctx->emit_before_op, op, link); + } else { + QTAILQ_INSERT_TAIL(&tcg_ctx->ops, op, link); + } tcg_debug_assert(n_extend < ARRAY_SIZE(extend_free)); for (i = 0; i < n_extend; ++i) { @@ -3215,7 +3220,12 @@ static TCGOp *tcg_op_alloc(TCGOpcode opc, unsigned nargs) TCGOp *tcg_emit_op(TCGOpcode opc, unsigned nargs) { TCGOp *op = tcg_op_alloc(opc, nargs); - QTAILQ_INSERT_TAIL(&tcg_ctx->ops, op, link); + + if (tcg_ctx->emit_before_op) { + QTAILQ_INSERT_BEFORE(tcg_ctx->emit_before_op, op, link); + } else { + QTAILQ_INSERT_TAIL(&tcg_ctx->ops, op, link); + } return op; } -- cgit v1.2.3 From e7face702ac1e98109dbfde557e7974ea04a295d Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 6 Apr 2024 10:42:10 -1000 Subject: accel/tcg: Add insn_start to DisasContextBase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is currently target-specific for many; begin making it target independent. Tested-by: Jørgen Hansen Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- accel/tcg/translator.c | 2 ++ include/exec/translator.h | 3 +++ 2 files changed, 5 insertions(+) diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c index 38c34009a5..ae61c154c2 100644 --- a/accel/tcg/translator.c +++ b/accel/tcg/translator.c @@ -140,6 +140,7 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns, db->max_insns = *max_insns; db->singlestep_enabled = cflags & CF_SINGLE_STEP; db->saved_can_do_io = -1; + db->insn_start = NULL; db->host_addr[0] = host_pc; db->host_addr[1] = NULL; @@ -157,6 +158,7 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns, while (true) { *max_insns = ++db->num_insns; ops->insn_start(db, cpu); + db->insn_start = tcg_last_op(); tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */ if (plugin_enabled) { diff --git a/include/exec/translator.h b/include/exec/translator.h index 51624feb10..ceaeca8c91 100644 --- a/include/exec/translator.h +++ b/include/exec/translator.h @@ -74,6 +74,8 @@ typedef enum DisasJumpType { * @singlestep_enabled: "Hardware" single stepping enabled. * @saved_can_do_io: Known value of cpu->neg.can_do_io, or -1 for unknown. * @plugin_enabled: TCG plugin enabled in this TB. + * @insn_start: The last op emitted by the insn_start hook, + * which is expected to be INDEX_op_insn_start. * * Architecture-agnostic disassembly context. */ @@ -87,6 +89,7 @@ typedef struct DisasContextBase { bool singlestep_enabled; int8_t saved_can_do_io; bool plugin_enabled; + struct TCGOp *insn_start; void *host_addr[2]; } DisasContextBase; -- cgit v1.2.3 From 4642250e3c4c00495c0902ff8f32559ad99ce130 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 6 Apr 2024 10:52:33 -1000 Subject: target/arm: Use insn_start from DisasContextBase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To keep the multiple update check, replace insn_start with insn_start_updated. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/arm/tcg/translate-a64.c | 2 +- target/arm/tcg/translate.c | 2 +- target/arm/tcg/translate.h | 12 ++++++------ 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/target/arm/tcg/translate-a64.c b/target/arm/tcg/translate-a64.c index 340265beb0..2666d52711 100644 --- a/target/arm/tcg/translate-a64.c +++ b/target/arm/tcg/translate-a64.c @@ -14179,7 +14179,7 @@ static void aarch64_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) pc_arg &= ~TARGET_PAGE_MASK; } tcg_gen_insn_start(pc_arg, 0, 0); - dc->insn_start = tcg_last_op(); + dc->insn_start_updated = false; } static void aarch64_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) diff --git a/target/arm/tcg/translate.c b/target/arm/tcg/translate.c index 69585e6003..dc49a8d806 100644 --- a/target/arm/tcg/translate.c +++ b/target/arm/tcg/translate.c @@ -9273,7 +9273,7 @@ static void arm_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) condexec_bits = (dc->condexec_cond << 4) | (dc->condexec_mask >> 1); } tcg_gen_insn_start(pc_arg, condexec_bits, 0); - dc->insn_start = tcg_last_op(); + dc->insn_start_updated = false; } static bool arm_check_kernelpage(DisasContext *dc) diff --git a/target/arm/tcg/translate.h b/target/arm/tcg/translate.h index 93be745cf3..dc66ff2190 100644 --- a/target/arm/tcg/translate.h +++ b/target/arm/tcg/translate.h @@ -165,10 +165,10 @@ typedef struct DisasContext { uint8_t gm_blocksize; /* True if this page is guarded. */ bool guarded_page; + /* True if the current insn_start has been updated. */ + bool insn_start_updated; /* Bottom two bits of XScale c15_cpar coprocessor access control reg */ int c15_cpar; - /* TCG op of the current insn_start. */ - TCGOp *insn_start; /* Offset from VNCR_EL2 when FEAT_NV2 redirects this reg to memory */ uint32_t nv2_redirect_offset; } DisasContext; @@ -276,10 +276,10 @@ static inline void disas_set_insn_syndrome(DisasContext *s, uint32_t syn) syn &= ARM_INSN_START_WORD2_MASK; syn >>= ARM_INSN_START_WORD2_SHIFT; - /* We check and clear insn_start_idx to catch multiple updates. */ - assert(s->insn_start != NULL); - tcg_set_insn_start_param(s->insn_start, 2, syn); - s->insn_start = NULL; + /* Check for multiple updates. */ + assert(!s->insn_start_updated); + s->insn_start_updated = true; + tcg_set_insn_start_param(s->base.insn_start, 2, syn); } static inline int curr_insn_len(DisasContext *s) -- cgit v1.2.3 From 24638bd17df4bdd7d54634dd5f6a5e8ec1282dc8 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 6 Apr 2024 10:59:02 -1000 Subject: target/hppa: Use insn_start from DisasContextBase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To keep the multiple update check, replace insn_start with insn_start_updated. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/hppa/translate.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/target/hppa/translate.c b/target/hppa/translate.c index 8a1a8bc3aa..42fa480950 100644 --- a/target/hppa/translate.c +++ b/target/hppa/translate.c @@ -44,7 +44,6 @@ typedef struct DisasCond { typedef struct DisasContext { DisasContextBase base; CPUState *cs; - TCGOp *insn_start; uint64_t iaoq_f; uint64_t iaoq_b; @@ -62,6 +61,7 @@ typedef struct DisasContext { int privilege; bool psw_n_nonzero; bool is_pa20; + bool insn_start_updated; #ifdef CONFIG_USER_ONLY MemOp unalign; @@ -300,9 +300,9 @@ void hppa_translate_init(void) static void set_insn_breg(DisasContext *ctx, int breg) { - assert(ctx->insn_start != NULL); - tcg_set_insn_start_param(ctx->insn_start, 2, breg); - ctx->insn_start = NULL; + assert(!ctx->insn_start_updated); + ctx->insn_start_updated = true; + tcg_set_insn_start_param(ctx->base.insn_start, 2, breg); } static DisasCond cond_make_f(void) @@ -4694,7 +4694,7 @@ static void hppa_tr_insn_start(DisasContextBase *dcbase, CPUState *cs) DisasContext *ctx = container_of(dcbase, DisasContext, base); tcg_gen_insn_start(ctx->iaoq_f, ctx->iaoq_b, 0); - ctx->insn_start = tcg_last_op(); + ctx->insn_start_updated = false; } static void hppa_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) -- cgit v1.2.3 From 8df1ba49d76e30fa5c8889121f4ed206ad6c2098 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 6 Apr 2024 11:05:12 -1000 Subject: target/i386: Preserve DisasContextBase.insn_start across rewind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When aborting translation of the current insn, restore the previous value of insn_start. Acked-by: Paolo Bonzini Tested-by: Jørgen Hansen Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/i386/tcg/translate.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c index 07f642dc9e..76a42c679c 100644 --- a/target/i386/tcg/translate.c +++ b/target/i386/tcg/translate.c @@ -139,6 +139,7 @@ typedef struct DisasContext { TCGv_i64 tmp1_i64; sigjmp_buf jmpbuf; + TCGOp *prev_insn_start; TCGOp *prev_insn_end; } DisasContext; @@ -3123,6 +3124,7 @@ static bool disas_insn(DisasContext *s, CPUState *cpu) /* END TODO */ s->base.num_insns--; tcg_remove_ops_after(s->prev_insn_end); + s->base.insn_start = s->prev_insn_start; s->base.is_jmp = DISAS_TOO_MANY; return false; default: @@ -6995,6 +6997,7 @@ static void i386_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) DisasContext *dc = container_of(dcbase, DisasContext, base); target_ulong pc_arg = dc->base.pc_next; + dc->prev_insn_start = dc->base.insn_start; dc->prev_insn_end = tcg_last_op(); if (tb_cflags(dcbase->tb) & CF_PCREL) { pc_arg &= ~TARGET_PAGE_MASK; -- cgit v1.2.3 From e2313450274659a08d9791435766dc43ab388aa9 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 6 Apr 2024 11:06:56 -1000 Subject: target/microblaze: Use insn_start from DisasContextBase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/microblaze/translate.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/target/microblaze/translate.c b/target/microblaze/translate.c index 4e52ef32db..fc451befae 100644 --- a/target/microblaze/translate.c +++ b/target/microblaze/translate.c @@ -62,9 +62,6 @@ typedef struct DisasContext { DisasContextBase base; const MicroBlazeCPUConfig *cfg; - /* TCG op of the current insn_start. */ - TCGOp *insn_start; - TCGv_i32 r0; bool r0_set; @@ -699,14 +696,14 @@ static TCGv compute_ldst_addr_ea(DisasContext *dc, int ra, int rb) static void record_unaligned_ess(DisasContext *dc, int rd, MemOp size, bool store) { - uint32_t iflags = tcg_get_insn_start_param(dc->insn_start, 1); + uint32_t iflags = tcg_get_insn_start_param(dc->base.insn_start, 1); iflags |= ESR_ESS_FLAG; iflags |= rd << 5; iflags |= store * ESR_S; iflags |= (size == MO_32) * ESR_W; - tcg_set_insn_start_param(dc->insn_start, 1, iflags); + tcg_set_insn_start_param(dc->base.insn_start, 1, iflags); } #endif @@ -1624,7 +1621,6 @@ static void mb_tr_insn_start(DisasContextBase *dcb, CPUState *cs) DisasContext *dc = container_of(dcb, DisasContext, base); tcg_gen_insn_start(dc->base.pc_next, dc->tb_flags & ~MSR_TB_MASK); - dc->insn_start = tcg_last_op(); } static void mb_tr_translate_insn(DisasContextBase *dcb, CPUState *cs) -- cgit v1.2.3 From 401aa608d8d1ee7a4bbf88cae1a16854103324e6 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 6 Apr 2024 11:12:14 -1000 Subject: target/riscv: Use insn_start from DisasContextBase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To keep the multiple update check, replace insn_start with insn_start_updated. Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/riscv/translate.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/target/riscv/translate.c b/target/riscv/translate.c index 9d57089fcc..9ff09ebdb6 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -115,8 +115,7 @@ typedef struct DisasContext { bool itrigger; /* FRM is known to contain a valid value. */ bool frm_valid; - /* TCG of the current insn_start */ - TCGOp *insn_start; + bool insn_start_updated; } DisasContext; static inline bool has_ext(DisasContext *ctx, uint32_t ext) @@ -207,9 +206,9 @@ static void gen_check_nanbox_s(TCGv_i64 out, TCGv_i64 in) static void decode_save_opc(DisasContext *ctx) { - assert(ctx->insn_start != NULL); - tcg_set_insn_start_param(ctx->insn_start, 1, ctx->opcode); - ctx->insn_start = NULL; + assert(!ctx->insn_start_updated); + ctx->insn_start_updated = true; + tcg_set_insn_start_param(ctx->base.insn_start, 1, ctx->opcode); } static void gen_pc_plus_diff(TCGv target, DisasContext *ctx, @@ -1224,7 +1223,7 @@ static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) } tcg_gen_insn_start(pc_next, 0); - ctx->insn_start = tcg_last_op(); + ctx->insn_start_updated = false; } static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) -- cgit v1.2.3 From b338970f8cdcba114dad467e085f3ff13dfa9a43 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 6 Apr 2024 11:14:52 -1000 Subject: target/s390x: Use insn_start from DisasContextBase MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- target/s390x/tcg/translate.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index 57b7db1ee9..90a74ee795 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -141,7 +141,6 @@ struct DisasFields { struct DisasContext { DisasContextBase base; const DisasInsn *insn; - TCGOp *insn_start; DisasFields fields; uint64_t ex_value; /* @@ -6314,7 +6313,7 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s) insn = extract_insn(env, s); /* Update insn_start now that we know the ILEN. */ - tcg_set_insn_start_param(s->insn_start, 2, s->ilen); + tcg_set_insn_start_param(s->base.insn_start, 2, s->ilen); /* Not found means unimplemented/illegal opcode. */ if (insn == NULL) { @@ -6468,7 +6467,6 @@ static void s390x_tr_insn_start(DisasContextBase *dcbase, CPUState *cs) /* Delay the set of ilen until we've read the insn. */ tcg_gen_insn_start(dc->base.pc_next, dc->cc_op, 0); - dc->insn_start = tcg_last_op(); } static target_ulong get_next_pc(CPUS390XState *env, DisasContext *s, -- cgit v1.2.3 From dcd092a0636ec36e69e42a3dbbe447d97cb0d113 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 6 Apr 2024 12:17:57 -1000 Subject: accel/tcg: Improve can_do_io management MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We already attempted to set and clear can_do_io before the first and last insns, but only used the initial value of max_insns and the call to translator_io_start to find those insns. Now that we track insn_start in DisasContextBase, and now that we have emit_before_op, we can wait until we have finished translation to identify the true first and last insns and emit the sets of can_do_io at that time. This fixes the case of a translation block which crossed a page boundary, and for which the second page turned out to be mmio. In this case we truncate the block, and the previous logic for can_do_io could leave a block with a single insn with can_do_io set to false, which would fail an assertion in cpu_io_recompile. Reported-by: Jørgen Hansen Reviewed-by: Philippe Mathieu-Daudé Tested-by: Jørgen Hansen Signed-off-by: Richard Henderson --- accel/tcg/translator.c | 45 +++++++++++++++++++++++---------------------- include/exec/translator.h | 1 - 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c index ae61c154c2..9de0bc34c8 100644 --- a/accel/tcg/translator.c +++ b/accel/tcg/translator.c @@ -18,20 +18,14 @@ static void set_can_do_io(DisasContextBase *db, bool val) { - if (db->saved_can_do_io != val) { - db->saved_can_do_io = val; - - QEMU_BUILD_BUG_ON(sizeof_field(CPUState, neg.can_do_io) != 1); - tcg_gen_st8_i32(tcg_constant_i32(val), tcg_env, - offsetof(ArchCPU, parent_obj.neg.can_do_io) - - offsetof(ArchCPU, env)); - } + QEMU_BUILD_BUG_ON(sizeof_field(CPUState, neg.can_do_io) != 1); + tcg_gen_st8_i32(tcg_constant_i32(val), tcg_env, + offsetof(ArchCPU, parent_obj.neg.can_do_io) - + offsetof(ArchCPU, env)); } bool translator_io_start(DisasContextBase *db) { - set_can_do_io(db, true); - /* * Ensure that this instruction will be the last in the TB. * The target may override this to something more forceful. @@ -84,13 +78,6 @@ static TCGOp *gen_tb_start(DisasContextBase *db, uint32_t cflags) - offsetof(ArchCPU, env)); } - /* - * cpu->neg.can_do_io is set automatically here at the beginning of - * each translation block. The cost is minimal, plus it would be - * very easy to forget doing it in the translator. - */ - set_can_do_io(db, db->max_insns == 1); - return icount_start_insn; } @@ -129,6 +116,7 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns, { uint32_t cflags = tb_cflags(tb); TCGOp *icount_start_insn; + TCGOp *first_insn_start = NULL; bool plugin_enabled; /* Initialize DisasContext */ @@ -139,7 +127,6 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns, db->num_insns = 0; db->max_insns = *max_insns; db->singlestep_enabled = cflags & CF_SINGLE_STEP; - db->saved_can_do_io = -1; db->insn_start = NULL; db->host_addr[0] = host_pc; db->host_addr[1] = NULL; @@ -159,6 +146,9 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns, *max_insns = ++db->num_insns; ops->insn_start(db, cpu); db->insn_start = tcg_last_op(); + if (first_insn_start == NULL) { + first_insn_start = db->insn_start; + } tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */ if (plugin_enabled) { @@ -171,10 +161,6 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns, * done next -- either exiting this loop or locate the start of * the next instruction. */ - if (db->num_insns == db->max_insns) { - /* Accept I/O on the last instruction. */ - set_can_do_io(db, true); - } ops->translate_insn(db, cpu); /* @@ -207,6 +193,21 @@ void translator_loop(CPUState *cpu, TranslationBlock *tb, int *max_insns, ops->tb_stop(db, cpu); gen_tb_end(tb, cflags, icount_start_insn, db->num_insns); + /* + * Manage can_do_io for the translation block: set to false before + * the first insn and set to true before the last insn. + */ + if (db->num_insns == 1) { + tcg_debug_assert(first_insn_start == db->insn_start); + } else { + tcg_debug_assert(first_insn_start != db->insn_start); + tcg_ctx->emit_before_op = first_insn_start; + set_can_do_io(db, false); + } + tcg_ctx->emit_before_op = db->insn_start; + set_can_do_io(db, true); + tcg_ctx->emit_before_op = NULL; + if (plugin_enabled) { plugin_gen_tb_end(cpu, db->num_insns); } diff --git a/include/exec/translator.h b/include/exec/translator.h index ceaeca8c91..2c4fb818e7 100644 --- a/include/exec/translator.h +++ b/include/exec/translator.h @@ -87,7 +87,6 @@ typedef struct DisasContextBase { int num_insns; int max_insns; bool singlestep_enabled; - int8_t saved_can_do_io; bool plugin_enabled; struct TCGOp *insn_start; void *host_addr[2]; -- cgit v1.2.3 From 143bcc1d59f174b6c6743bd4ca8f99415ed1aba2 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 8 Apr 2024 14:33:35 -1000 Subject: linux-user: Preserve unswapped siginfo_t for strace MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Passing the tswapped structure to strace means that our internal si_type is also gone, which then aborts in print_siginfo. Fixes: 4d6d8a05a0a ("linux-user: Move tswap_siginfo out of target code") Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Richard Henderson --- linux-user/signal.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/linux-user/signal.c b/linux-user/signal.c index a93148a4cb..05dc4afb52 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -1173,6 +1173,7 @@ static void handle_pending_signal(CPUArchState *cpu_env, int sig, CPUState *cpu = env_cpu(cpu_env); abi_ulong handler; sigset_t set; + target_siginfo_t unswapped; target_sigset_t target_old_set; struct target_sigaction *sa; TaskState *ts = get_task_state(cpu); @@ -1182,9 +1183,14 @@ static void handle_pending_signal(CPUArchState *cpu_env, int sig, k->pending = 0; /* - * Writes out siginfo values byteswapped, accordingly to the target. It also - * cleans the si_type from si_code making it correct for the target. + * Writes out siginfo values byteswapped, accordingly to the target. + * It also cleans the si_type from si_code making it correct for + * the target. We must hold on to the original unswapped copy for + * strace below, because si_type is still required there. */ + if (unlikely(qemu_loglevel_mask(LOG_STRACE))) { + unswapped = k->info; + } tswap_siginfo(&k->info, &k->info); sig = gdb_handlesig(cpu, sig, NULL, &k->info, sizeof(k->info)); @@ -1197,7 +1203,7 @@ static void handle_pending_signal(CPUArchState *cpu_env, int sig, } if (unlikely(qemu_loglevel_mask(LOG_STRACE))) { - print_taken_signal(sig, &k->info); + print_taken_signal(sig, &unswapped); } if (handler == TARGET_SIG_DFL) { -- cgit v1.2.3