From 808d15b383fecb3ec540186f68767a211c756c5a Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Mon, 6 Feb 2023 13:32:32 +0100 Subject: build: make meson-buildoptions.sh stable The library directory can change depending on the multilib setup of the host. It would be even better to detect it in configure with the same algorithm that Meson uses, but the important thing to avoid confusing developers is to have identical contents of scripts/meson-buildoptions.sh, independent of the distro and architecture on which it was created. So, for now just give a custom default value to libdir. Signed-off-by: Paolo Bonzini --- scripts/meson-buildoptions.py | 7 +++++-- scripts/meson-buildoptions.sh | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/scripts/meson-buildoptions.py b/scripts/meson-buildoptions.py index 3e2b478538..a04dcc70a5 100755 --- a/scripts/meson-buildoptions.py +++ b/scripts/meson-buildoptions.py @@ -61,7 +61,10 @@ LINE_WIDTH = 76 # Convert the default value of an option to the string used in # the help message -def value_to_help(value): +def get_help(opt): + if opt["name"] == "libdir": + return 'system default' + value = opt["value"] if isinstance(value, list): return ",".join(value) if isinstance(value, bool): @@ -88,7 +91,7 @@ def sh_print(line=""): def help_line(left, opt, indent, long): right = f'{opt["description"]}' if long: - value = value_to_help(opt["value"]) + value = get_help(opt) if value != "auto" and value != "": right += f" [{value}]" if "choices" in opt and long: diff --git a/scripts/meson-buildoptions.sh b/scripts/meson-buildoptions.sh index 0f71e92dcb..d663c9cadf 100644 --- a/scripts/meson-buildoptions.sh +++ b/scripts/meson-buildoptions.sh @@ -49,7 +49,7 @@ meson_options_help() { printf "%s\n" ' --includedir=VALUE Header file directory [include]' printf "%s\n" ' --interp-prefix=VALUE where to find shared libraries etc., use %M for' printf "%s\n" ' cpu name [/usr/gnemul/qemu-%M]' - printf "%s\n" ' --libdir=VALUE Library directory [lib64]' + printf "%s\n" ' --libdir=VALUE Library directory [system default]' printf "%s\n" ' --libexecdir=VALUE Library executable directory [libexec]' printf "%s\n" ' --localedir=VALUE Locale data directory [share/locale]' printf "%s\n" ' --localstatedir=VALUE Localstate data directory [/var/local]' -- cgit v1.2.3 From d76aa73fad1f64c192856e1420ad0756f5e3b778 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 10 Jan 2023 09:19:31 +0100 Subject: remove unnecessary extern "C" blocks A handful of header files in QEMU are wrapped with extern "C" blocks. These are not necessary: there are C++ source files anymore in QEMU, and even where there were some, they did not include most of these files anyway. Remove them for consistency. Signed-off-by: Paolo Bonzini --- include/disas/dis-asm.h | 8 -------- include/qemu/bswap.h | 8 -------- include/qemu/envlist.h | 8 -------- include/qemu/rcu.h | 8 -------- include/qemu/rcu_queue.h | 8 -------- include/qemu/uri.h | 7 ------- 6 files changed, 47 deletions(-) diff --git a/include/disas/dis-asm.h b/include/disas/dis-asm.h index 64247ecb11..32cda9ef14 100644 --- a/include/disas/dis-asm.h +++ b/include/disas/dis-asm.h @@ -11,10 +11,6 @@ #include "qemu/bswap.h" -#ifdef __cplusplus -extern "C" { -#endif - typedef void *PTR; typedef uint64_t bfd_vma; typedef int64_t bfd_signed_vma; @@ -506,8 +502,4 @@ static inline bfd_vma bfd_getb16(const bfd_byte *addr) typedef bool bfd_boolean; -#ifdef __cplusplus -} -#endif - #endif /* DISAS_DIS_ASM_H */ diff --git a/include/qemu/bswap.h b/include/qemu/bswap.h index 3cbe52246b..b1650daedf 100644 --- a/include/qemu/bswap.h +++ b/include/qemu/bswap.h @@ -1,10 +1,6 @@ #ifndef BSWAP_H #define BSWAP_H -#ifdef __cplusplus -extern "C" { -#endif - #undef bswap16 #define bswap16(_x) __builtin_bswap16(_x) #undef bswap32 @@ -395,8 +391,4 @@ DO_STN_LDN_P(be) #undef le_bswaps #undef be_bswaps -#ifdef __cplusplus -} -#endif - #endif /* BSWAP_H */ diff --git a/include/qemu/envlist.h b/include/qemu/envlist.h index b9addcc11f..6006dfae44 100644 --- a/include/qemu/envlist.h +++ b/include/qemu/envlist.h @@ -1,10 +1,6 @@ #ifndef ENVLIST_H #define ENVLIST_H -#ifdef __cplusplus -extern "C" { -#endif - typedef struct envlist envlist_t; envlist_t *envlist_create(void); @@ -15,8 +11,4 @@ int envlist_parse_set(envlist_t *, const char *); int envlist_parse_unset(envlist_t *, const char *); char **envlist_to_environ(const envlist_t *, size_t *); -#ifdef __cplusplus -} -#endif - #endif /* ENVLIST_H */ diff --git a/include/qemu/rcu.h b/include/qemu/rcu.h index b063c6fde8..313fc414bc 100644 --- a/include/qemu/rcu.h +++ b/include/qemu/rcu.h @@ -31,10 +31,6 @@ #include "qemu/sys_membarrier.h" #include "qemu/coroutine-tls.h" -#ifdef __cplusplus -extern "C" { -#endif - /* * Important ! * @@ -196,8 +192,4 @@ G_DEFINE_AUTOPTR_CLEANUP_FUNC(RCUReadAuto, rcu_read_auto_unlock) void rcu_add_force_rcu_notifier(Notifier *n); void rcu_remove_force_rcu_notifier(Notifier *n); -#ifdef __cplusplus -} -#endif - #endif /* QEMU_RCU_H */ diff --git a/include/qemu/rcu_queue.h b/include/qemu/rcu_queue.h index 0e53ddd530..4e6298d473 100644 --- a/include/qemu/rcu_queue.h +++ b/include/qemu/rcu_queue.h @@ -28,11 +28,6 @@ #include "qemu/queue.h" #include "qemu/atomic.h" -#ifdef __cplusplus -extern "C" { -#endif - - /* * List access methods. */ @@ -311,7 +306,4 @@ extern "C" { (var) && ((next) = qatomic_rcu_read(&(var)->field.sle_next), 1); \ (var) = (next)) -#ifdef __cplusplus -} -#endif #endif /* QEMU_RCU_QUEUE_H */ diff --git a/include/qemu/uri.h b/include/qemu/uri.h index d201c61260..db5218c39e 100644 --- a/include/qemu/uri.h +++ b/include/qemu/uri.h @@ -53,10 +53,6 @@ #ifndef QEMU_URI_H #define QEMU_URI_H -#ifdef __cplusplus -extern "C" { -#endif - /** * URI: * @@ -105,7 +101,4 @@ struct QueryParams *query_params_new (int init_alloc); extern QueryParams *query_params_parse (const char *query); extern void query_params_free (QueryParams *ps); -#ifdef __cplusplus -} -#endif #endif /* QEMU_URI_H */ -- cgit v1.2.3 From 5080152e2ef6cde7aa692e29880c62bd54acb750 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 10 Jan 2023 17:36:33 +0100 Subject: block/iscsi: fix double-free on BUSY or similar statuses Commit 8c460269aa77 ("iscsi: base all handling of check condition on scsi_sense_to_errno", 2019-07-15) removed a "goto out" so that the same coroutine is re-entered twice; once from iscsi_co_generic_cb, once from the timer callback iscsi_retry_timer_expired. This can cause a crash. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1378 Reported-by: Grzegorz Zdanowski Signed-off-by: Paolo Bonzini --- block/iscsi.c | 1 + 1 file changed, 1 insertion(+) diff --git a/block/iscsi.c b/block/iscsi.c index b3e10f40b6..3aacd0709f 100644 --- a/block/iscsi.c +++ b/block/iscsi.c @@ -269,6 +269,7 @@ iscsi_co_generic_cb(struct iscsi_context *iscsi, int status, timer_mod(&iTask->retry_timer, qemu_clock_get_ms(QEMU_CLOCK_REALTIME) + retry_time); iTask->do_retry = 1; + return; } else if (status == SCSI_STATUS_CHECK_CONDITION) { int error = iscsi_translate_sense(&task->sense); if (error == EAGAIN) { -- cgit v1.2.3 From 78901b5047a2c27858f6c3e780ab3f2af5463631 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 13 Jan 2023 11:35:40 +0100 Subject: vl: catch [accel] entry without accelerator Avoid a SIGSEGV and return an error instead. Reported-by: Thomas Huth Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1439 Signed-off-by: Paolo Bonzini --- softmmu/vl.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/softmmu/vl.c b/softmmu/vl.c index b2ee3fee3f..459588aa7d 100644 --- a/softmmu/vl.c +++ b/softmmu/vl.c @@ -2204,14 +2204,18 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp) int ret; bool qtest_with_kvm; + if (!acc) { + error_setg(errp, QERR_MISSING_PARAMETER, "accel"); + goto bad; + } + qtest_with_kvm = g_str_equal(acc, "kvm") && qtest_chrdev != NULL; if (!ac) { - *p_init_failed = true; if (!qtest_with_kvm) { error_report("invalid accelerator %s", acc); } - return 0; + goto bad; } accel = ACCEL(object_new_with_class(OBJECT_CLASS(ac))); object_apply_compat_props(OBJECT(accel)); @@ -2221,14 +2225,17 @@ static int do_configure_accelerator(void *opaque, QemuOpts *opts, Error **errp) ret = accel_init_machine(accel, current_machine); if (ret < 0) { - *p_init_failed = true; if (!qtest_with_kvm || ret != -ENOENT) { error_report("failed to initialize %s: %s", acc, strerror(-ret)); } - return 0; + goto bad; } return 1; + +bad: + *p_init_failed = true; + return 0; } static void configure_accelerators(const char *progname) -- cgit v1.2.3 From 5d62d6649cd367b5b4a3676e7514d2f9ca86cb03 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 14 Jan 2023 13:05:41 -1000 Subject: tests/tcg/i386: Introduce and use reg_t consistently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Define reg_t based on the actual register width. Define the inlines using that type. This will allow input registers to 32-bit insns to be set to 64-bit values on x86-64, which allows testing various edge cases. Signed-off-by: Richard Henderson Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20230114230542.3116013-2-richard.henderson@linaro.org> Signed-off-by: Paolo Bonzini --- tests/tcg/i386/test-i386-bmi2.c | 182 ++++++++++++++++++++-------------------- 1 file changed, 93 insertions(+), 89 deletions(-) diff --git a/tests/tcg/i386/test-i386-bmi2.c b/tests/tcg/i386/test-i386-bmi2.c index 5fadf47510..3c3ef85513 100644 --- a/tests/tcg/i386/test-i386-bmi2.c +++ b/tests/tcg/i386/test-i386-bmi2.c @@ -3,34 +3,40 @@ #include #include +#ifdef __x86_64 +typedef uint64_t reg_t; +#else +typedef uint32_t reg_t; +#endif + #define insn1q(name, arg0) \ -static inline uint64_t name##q(uint64_t arg0) \ +static inline reg_t name##q(reg_t arg0) \ { \ - uint64_t result64; \ + reg_t result64; \ asm volatile (#name "q %1, %0" : "=r"(result64) : "rm"(arg0)); \ return result64; \ } #define insn1l(name, arg0) \ -static inline uint32_t name##l(uint32_t arg0) \ +static inline reg_t name##l(reg_t arg0) \ { \ - uint32_t result32; \ + reg_t result32; \ asm volatile (#name "l %k1, %k0" : "=r"(result32) : "rm"(arg0)); \ return result32; \ } #define insn2q(name, arg0, c0, arg1, c1) \ -static inline uint64_t name##q(uint64_t arg0, uint64_t arg1) \ +static inline reg_t name##q(reg_t arg0, reg_t arg1) \ { \ - uint64_t result64; \ + reg_t result64; \ asm volatile (#name "q %2, %1, %0" : "=r"(result64) : c0(arg0), c1(arg1)); \ return result64; \ } #define insn2l(name, arg0, c0, arg1, c1) \ -static inline uint32_t name##l(uint32_t arg0, uint32_t arg1) \ +static inline reg_t name##l(reg_t arg0, reg_t arg1) \ { \ - uint32_t result32; \ + reg_t result32; \ asm volatile (#name "l %k2, %k1, %k0" : "=r"(result32) : c0(arg0), c1(arg1)); \ return result32; \ } @@ -65,130 +71,128 @@ insn1l(blsr, src) int main(int argc, char *argv[]) { uint64_t ehlo = 0x202020204f4c4845ull; uint64_t mask = 0xa080800302020001ull; - uint32_t result32; + reg_t result; #ifdef __x86_64 - uint64_t result64; - /* 64 bits */ - result64 = andnq(mask, ehlo); - assert(result64 == 0x002020204d4c4844); + result = andnq(mask, ehlo); + assert(result == 0x002020204d4c4844); - result64 = pextq(ehlo, mask); - assert(result64 == 133); + result = pextq(ehlo, mask); + assert(result == 133); - result64 = pdepq(result64, mask); - assert(result64 == (ehlo & mask)); + result = pdepq(result, mask); + assert(result == (ehlo & mask)); - result64 = pextq(-1ull, mask); - assert(result64 == 511); /* mask has 9 bits set */ + result = pextq(-1ull, mask); + assert(result == 511); /* mask has 9 bits set */ - result64 = pdepq(-1ull, mask); - assert(result64 == mask); + result = pdepq(-1ull, mask); + assert(result == mask); - result64 = bextrq(mask, 0x3f00); - assert(result64 == (mask & ~INT64_MIN)); + result = bextrq(mask, 0x3f00); + assert(result == (mask & ~INT64_MIN)); - result64 = bextrq(mask, 0x1038); - assert(result64 == 0xa0); + result = bextrq(mask, 0x1038); + assert(result == 0xa0); - result64 = bextrq(mask, 0x10f8); - assert(result64 == 0); + result = bextrq(mask, 0x10f8); + assert(result == 0); - result64 = blsiq(0x30); - assert(result64 == 0x10); + result = blsiq(0x30); + assert(result == 0x10); - result64 = blsiq(0x30ull << 32); - assert(result64 == 0x10ull << 32); + result = blsiq(0x30ull << 32); + assert(result == 0x10ull << 32); - result64 = blsmskq(0x30); - assert(result64 == 0x1f); + result = blsmskq(0x30); + assert(result == 0x1f); - result64 = blsrq(0x30); - assert(result64 == 0x20); + result = blsrq(0x30); + assert(result == 0x20); - result64 = blsrq(0x30ull << 32); - assert(result64 == 0x20ull << 32); + result = blsrq(0x30ull << 32); + assert(result == 0x20ull << 32); - result64 = bzhiq(mask, 0x3f); - assert(result64 == (mask & ~INT64_MIN)); + result = bzhiq(mask, 0x3f); + assert(result == (mask & ~INT64_MIN)); - result64 = bzhiq(mask, 0x1f); - assert(result64 == (mask & ~(-1 << 30))); + result = bzhiq(mask, 0x1f); + assert(result == (mask & ~(-1 << 30))); - result64 = rorxq(0x2132435465768798, 8); - assert(result64 == 0x9821324354657687); + result = rorxq(0x2132435465768798, 8); + assert(result == 0x9821324354657687); - result64 = sarxq(0xffeeddccbbaa9988, 8); - assert(result64 == 0xffffeeddccbbaa99); + result = sarxq(0xffeeddccbbaa9988, 8); + assert(result == 0xffffeeddccbbaa99); - result64 = sarxq(0x77eeddccbbaa9988, 8 | 64); - assert(result64 == 0x0077eeddccbbaa99); + result = sarxq(0x77eeddccbbaa9988, 8 | 64); + assert(result == 0x0077eeddccbbaa99); - result64 = shrxq(0xffeeddccbbaa9988, 8); - assert(result64 == 0x00ffeeddccbbaa99); + result = shrxq(0xffeeddccbbaa9988, 8); + assert(result == 0x00ffeeddccbbaa99); - result64 = shrxq(0x77eeddccbbaa9988, 8 | 192); - assert(result64 == 0x0077eeddccbbaa99); + result = shrxq(0x77eeddccbbaa9988, 8 | 192); + assert(result == 0x0077eeddccbbaa99); - result64 = shlxq(0xffeeddccbbaa9988, 8); - assert(result64 == 0xeeddccbbaa998800); + result = shlxq(0xffeeddccbbaa9988, 8); + assert(result == 0xeeddccbbaa998800); #endif /* 32 bits */ - result32 = andnl(mask, ehlo); - assert(result32 == 0x04d4c4844); + result = andnl(mask, ehlo); + assert(result == 0x04d4c4844); - result32 = pextl((uint32_t) ehlo, mask); - assert(result32 == 5); + result = pextl((uint32_t) ehlo, mask); + assert(result == 5); - result32 = pdepl(result32, mask); - assert(result32 == (uint32_t)(ehlo & mask)); + result = pdepl(result, mask); + assert(result == (uint32_t)(ehlo & mask)); - result32 = pextl(-1u, mask); - assert(result32 == 7); /* mask has 3 bits set */ + result = pextl(-1u, mask); + assert(result == 7); /* mask has 3 bits set */ - result32 = pdepl(-1u, mask); - assert(result32 == (uint32_t)mask); + result = pdepl(-1u, mask); + assert(result == (uint32_t)mask); - result32 = bextrl(mask, 0x1f00); - assert(result32 == (mask & ~INT32_MIN)); + result = bextrl(mask, 0x1f00); + assert(result == (mask & ~INT32_MIN)); - result32 = bextrl(ehlo, 0x1018); - assert(result32 == 0x4f); + result = bextrl(ehlo, 0x1018); + assert(result == 0x4f); - result32 = bextrl(mask, 0x1038); - assert(result32 == 0); + result = bextrl(mask, 0x1038); + assert(result == 0); - result32 = blsil(0xffff); - assert(result32 == 1); + result = blsil(0xffff); + assert(result == 1); - result32 = blsmskl(0x300); - assert(result32 == 0x1ff); + result = blsmskl(0x300); + assert(result == 0x1ff); - result32 = blsrl(0xffc); - assert(result32 == 0xff8); + result = blsrl(0xffc); + assert(result == 0xff8); - result32 = bzhil(mask, 0xf); - assert(result32 == 1); + result = bzhil(mask, 0xf); + assert(result == 1); - result32 = rorxl(0x65768798, 8); - assert(result32 == 0x98657687); + result = rorxl(0x65768798, 8); + assert(result == 0x98657687); - result32 = sarxl(0xffeeddcc, 8); - assert(result32 == 0xffffeedd); + result = sarxl(0xffeeddcc, 8); + assert(result == 0xffffeedd); - result32 = sarxl(0x77eeddcc, 8 | 32); - assert(result32 == 0x0077eedd); + result = sarxl(0x77eeddcc, 8 | 32); + assert(result == 0x0077eedd); - result32 = shrxl(0xffeeddcc, 8); - assert(result32 == 0x00ffeedd); + result = shrxl(0xffeeddcc, 8); + assert(result == 0x00ffeedd); - result32 = shrxl(0x77eeddcc, 8 | 128); - assert(result32 == 0x0077eedd); + result = shrxl(0x77eeddcc, 8 | 128); + assert(result == 0x0077eedd); - result32 = shlxl(0xffeeddcc, 8); - assert(result32 == 0xeeddcc00); + result = shlxl(0xffeeddcc, 8); + assert(result == 0xeeddcc00); return 0; } -- cgit v1.2.3 From b14c0098975264ed03144f145bca0179a6763a07 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 14 Jan 2023 13:05:42 -1000 Subject: target/i386: Fix BEXTR instruction There were two problems here: not limiting the input to operand bits, and not correctly handling large extraction length. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1372 Signed-off-by: Richard Henderson Message-Id: <20230114230542.3116013-3-richard.henderson@linaro.org> Cc: qemu-stable@nongnu.org Fixes: 1d0b926150e5 ("target/i386: move scalar 0F 38 and 0F 3A instruction to new decoder", 2022-10-18) Signed-off-by: Paolo Bonzini --- target/i386/tcg/emit.c.inc | 22 +++++++++++----------- tests/tcg/i386/test-i386-bmi2.c | 12 ++++++++++++ 2 files changed, 23 insertions(+), 11 deletions(-) diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc index 7037ff91c6..99f6ba6e19 100644 --- a/target/i386/tcg/emit.c.inc +++ b/target/i386/tcg/emit.c.inc @@ -1078,30 +1078,30 @@ static void gen_ANDN(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode) static void gen_BEXTR(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode) { MemOp ot = decode->op[0].ot; - TCGv bound, zero; + TCGv bound = tcg_constant_tl(ot == MO_64 ? 63 : 31); + TCGv zero = tcg_constant_tl(0); + TCGv mone = tcg_constant_tl(-1); /* * Extract START, and shift the operand. * Shifts larger than operand size get zeros. */ tcg_gen_ext8u_tl(s->A0, s->T1); + if (TARGET_LONG_BITS == 64 && ot == MO_32) { + tcg_gen_ext32u_tl(s->T0, s->T0); + } tcg_gen_shr_tl(s->T0, s->T0, s->A0); - bound = tcg_constant_tl(ot == MO_64 ? 63 : 31); - zero = tcg_constant_tl(0); tcg_gen_movcond_tl(TCG_COND_LEU, s->T0, s->A0, bound, s->T0, zero); /* - * Extract the LEN into a mask. Lengths larger than - * operand size get all ones. + * Extract the LEN into an inverse mask. Lengths larger than + * operand size get all zeros, length 0 gets all ones. */ tcg_gen_extract_tl(s->A0, s->T1, 8, 8); - tcg_gen_movcond_tl(TCG_COND_LEU, s->A0, s->A0, bound, s->A0, bound); - - tcg_gen_movi_tl(s->T1, 1); - tcg_gen_shl_tl(s->T1, s->T1, s->A0); - tcg_gen_subi_tl(s->T1, s->T1, 1); - tcg_gen_and_tl(s->T0, s->T0, s->T1); + tcg_gen_shl_tl(s->T1, mone, s->A0); + tcg_gen_movcond_tl(TCG_COND_LEU, s->T1, s->A0, bound, s->T1, zero); + tcg_gen_andc_tl(s->T0, s->T0, s->T1); gen_op_update1_cc(s); set_cc_op(s, CC_OP_LOGICB + ot); diff --git a/tests/tcg/i386/test-i386-bmi2.c b/tests/tcg/i386/test-i386-bmi2.c index 3c3ef85513..982d4abda4 100644 --- a/tests/tcg/i386/test-i386-bmi2.c +++ b/tests/tcg/i386/test-i386-bmi2.c @@ -99,6 +99,9 @@ int main(int argc, char *argv[]) { result = bextrq(mask, 0x10f8); assert(result == 0); + result = bextrq(0xfedcba9876543210ull, 0x7f00); + assert(result == 0xfedcba9876543210ull); + result = blsiq(0x30); assert(result == 0x10); @@ -164,6 +167,15 @@ int main(int argc, char *argv[]) { result = bextrl(mask, 0x1038); assert(result == 0); + result = bextrl((reg_t)0x8f635a775ad3b9b4ull, 0x3018); + assert(result == 0x5a); + + result = bextrl((reg_t)0xfedcba9876543210ull, 0x7f00); + assert(result == 0x76543210u); + + result = bextrl(-1, 0); + assert(result == 0); + result = blsil(0xffff); assert(result == 1); -- cgit v1.2.3 From 99282098dc74c2055bde5652bde6cf0067d0c370 Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Sat, 14 Jan 2023 08:06:01 -1000 Subject: target/i386: Fix C flag for BLSI, BLSMSK, BLSR We forgot to set cc_src, which is used for computing C. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1370 Signed-off-by: Richard Henderson Message-Id: <20230114180601.2993644-1-richard.henderson@linaro.org> Cc: qemu-stable@nongnu.org Fixes: 1d0b926150e5 ("target/i386: move scalar 0F 38 and 0F 3A instruction to new decoder", 2022-10-18) Signed-off-by: Paolo Bonzini --- target/i386/tcg/emit.c.inc | 3 +++ 1 file changed, 3 insertions(+) diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc index 99f6ba6e19..4d7702c106 100644 --- a/target/i386/tcg/emit.c.inc +++ b/target/i386/tcg/emit.c.inc @@ -1111,6 +1111,7 @@ static void gen_BLSI(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode) { MemOp ot = decode->op[0].ot; + tcg_gen_mov_tl(cpu_cc_src, s->T0); tcg_gen_neg_tl(s->T1, s->T0); tcg_gen_and_tl(s->T0, s->T0, s->T1); tcg_gen_mov_tl(cpu_cc_dst, s->T0); @@ -1121,6 +1122,7 @@ static void gen_BLSMSK(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode { MemOp ot = decode->op[0].ot; + tcg_gen_mov_tl(cpu_cc_src, s->T0); tcg_gen_subi_tl(s->T1, s->T0, 1); tcg_gen_xor_tl(s->T0, s->T0, s->T1); tcg_gen_mov_tl(cpu_cc_dst, s->T0); @@ -1131,6 +1133,7 @@ static void gen_BLSR(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode) { MemOp ot = decode->op[0].ot; + tcg_gen_mov_tl(cpu_cc_src, s->T0); tcg_gen_subi_tl(s->T1, s->T0, 1); tcg_gen_and_tl(s->T0, s->T0, s->T1); tcg_gen_mov_tl(cpu_cc_dst, s->T0); -- cgit v1.2.3 From 60c7dd22e1383754d5f150bc9f7c2785c662a7b6 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 31 Jan 2023 09:48:03 +0100 Subject: target/i386: fix ADOX followed by ADCX When ADCX is followed by ADOX or vice versa, the second instruction's carry comes from EFLAGS and the condition codes use the CC_OP_ADCOX operation. Retrieving the carry from EFLAGS is handled by this bit of gen_ADCOX: tcg_gen_extract_tl(carry_in, cpu_cc_src, ctz32(cc_op == CC_OP_ADCX ? CC_C : CC_O), 1); Unfortunately, in this case cc_op has been overwritten by the previous "if" statement to CC_OP_ADCOX. This works by chance when the first instruction is ADCX; however, if the first instruction is ADOX, ADCX will incorrectly take its carry from OF instead of CF. Fix by moving the computation of the new cc_op at the end of the function. The included exhaustive test case fails without this patch and passes afterwards. Because ADCX/ADOX need not be invoked through the VEX prefix, this regression bisects to commit 16fc5726a6e2 ("target/i386: reimplement 0x0f 0x38, add AVX", 2022-10-18). However, the mistake happened a little earlier, when BMI instructions were rewritten using the new decoder framework. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1471 Reported-by: Paul Jolly Fixes: 1d0b926150e5 ("target/i386: move scalar 0F 38 and 0F 3A instruction to new decoder", 2022-10-18) Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini --- target/i386/tcg/emit.c.inc | 20 ++++++----- tests/tcg/i386/Makefile.target | 6 +++- tests/tcg/i386/test-i386-adcox.c | 75 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 91 insertions(+), 10 deletions(-) create mode 100644 tests/tcg/i386/test-i386-adcox.c diff --git a/target/i386/tcg/emit.c.inc b/target/i386/tcg/emit.c.inc index 4d7702c106..0d7c6e80ae 100644 --- a/target/i386/tcg/emit.c.inc +++ b/target/i386/tcg/emit.c.inc @@ -1015,6 +1015,7 @@ VSIB_AVX(VPGATHERQ, vpgatherq) static void gen_ADCOX(DisasContext *s, CPUX86State *env, MemOp ot, int cc_op) { + int opposite_cc_op; TCGv carry_in = NULL; TCGv carry_out = (cc_op == CC_OP_ADCX ? cpu_cc_dst : cpu_cc_src2); TCGv zero; @@ -1022,14 +1023,8 @@ static void gen_ADCOX(DisasContext *s, CPUX86State *env, MemOp ot, int cc_op) if (cc_op == s->cc_op || s->cc_op == CC_OP_ADCOX) { /* Re-use the carry-out from a previous round. */ carry_in = carry_out; - cc_op = s->cc_op; - } else if (s->cc_op == CC_OP_ADCX || s->cc_op == CC_OP_ADOX) { - /* Merge with the carry-out from the opposite instruction. */ - cc_op = CC_OP_ADCOX; - } - - /* If we don't have a carry-in, get it out of EFLAGS. */ - if (!carry_in) { + } else { + /* We don't have a carry-in, get it out of EFLAGS. */ if (s->cc_op != CC_OP_ADCX && s->cc_op != CC_OP_ADOX) { gen_compute_eflags(s); } @@ -1053,7 +1048,14 @@ static void gen_ADCOX(DisasContext *s, CPUX86State *env, MemOp ot, int cc_op) tcg_gen_add2_tl(s->T0, carry_out, s->T0, carry_out, s->T1, zero); break; } - set_cc_op(s, cc_op); + + opposite_cc_op = cc_op == CC_OP_ADCX ? CC_OP_ADOX : CC_OP_ADCX; + if (s->cc_op == CC_OP_ADCOX || s->cc_op == opposite_cc_op) { + /* Merge with the carry-out from the opposite instruction. */ + set_cc_op(s, CC_OP_ADCOX); + } else { + set_cc_op(s, cc_op); + } } static void gen_ADCX(DisasContext *s, CPUX86State *env, X86DecodedInsn *decode) diff --git a/tests/tcg/i386/Makefile.target b/tests/tcg/i386/Makefile.target index 81831cafbc..bafd8c2180 100644 --- a/tests/tcg/i386/Makefile.target +++ b/tests/tcg/i386/Makefile.target @@ -14,7 +14,7 @@ config-cc.mak: Makefile I386_SRCS=$(notdir $(wildcard $(I386_SRC)/*.c)) ALL_X86_TESTS=$(I386_SRCS:.c=) SKIP_I386_TESTS=test-i386-ssse3 test-avx test-3dnow test-mmx -X86_64_TESTS:=$(filter test-i386-bmi2 $(SKIP_I386_TESTS), $(ALL_X86_TESTS)) +X86_64_TESTS:=$(filter test-i386-adcox test-i386-bmi2 $(SKIP_I386_TESTS), $(ALL_X86_TESTS)) test-i386-sse-exceptions: CFLAGS += -msse4.1 -mfpmath=sse run-test-i386-sse-exceptions: QEMU_OPTS += -cpu max @@ -28,6 +28,10 @@ test-i386-bmi2: CFLAGS=-O2 run-test-i386-bmi2: QEMU_OPTS += -cpu max run-plugin-test-i386-bmi2-%: QEMU_OPTS += -cpu max +test-i386-adcox: CFLAGS=-O2 +run-test-i386-adcox: QEMU_OPTS += -cpu max +run-plugin-test-i386-adcox-%: QEMU_OPTS += -cpu max + # # hello-i386 is a barebones app # diff --git a/tests/tcg/i386/test-i386-adcox.c b/tests/tcg/i386/test-i386-adcox.c new file mode 100644 index 0000000000..16169efff8 --- /dev/null +++ b/tests/tcg/i386/test-i386-adcox.c @@ -0,0 +1,75 @@ +/* See if various BMI2 instructions give expected results */ +#include +#include +#include + +#define CC_C 1 +#define CC_O (1 << 11) + +#ifdef __x86_64__ +#define REG uint64_t +#else +#define REG uint32_t +#endif + +void test_adox_adcx(uint32_t in_c, uint32_t in_o, REG adcx_operand, REG adox_operand) +{ + REG flags; + REG out_adcx, out_adox; + + asm("pushf; pop %0" : "=r"(flags)); + flags &= ~(CC_C | CC_O); + flags |= (in_c ? CC_C : 0); + flags |= (in_o ? CC_O : 0); + + out_adcx = adcx_operand; + out_adox = adox_operand; + asm("push %0; popf;" + "adox %3, %2;" + "adcx %3, %1;" + "pushf; pop %0" + : "+r" (flags), "+r" (out_adcx), "+r" (out_adox) + : "r" ((REG)-1), "0" (flags), "1" (out_adcx), "2" (out_adox)); + + assert(out_adcx == in_c + adcx_operand - 1); + assert(out_adox == in_o + adox_operand - 1); + assert(!!(flags & CC_C) == (in_c || adcx_operand)); + assert(!!(flags & CC_O) == (in_o || adox_operand)); +} + +void test_adcx_adox(uint32_t in_c, uint32_t in_o, REG adcx_operand, REG adox_operand) +{ + REG flags; + REG out_adcx, out_adox; + + asm("pushf; pop %0" : "=r"(flags)); + flags &= ~(CC_C | CC_O); + flags |= (in_c ? CC_C : 0); + flags |= (in_o ? CC_O : 0); + + out_adcx = adcx_operand; + out_adox = adox_operand; + asm("push %0; popf;" + "adcx %3, %1;" + "adox %3, %2;" + "pushf; pop %0" + : "+r" (flags), "+r" (out_adcx), "+r" (out_adox) + : "r" ((REG)-1), "0" (flags), "1" (out_adcx), "2" (out_adox)); + + assert(out_adcx == in_c + adcx_operand - 1); + assert(out_adox == in_o + adox_operand - 1); + assert(!!(flags & CC_C) == (in_c || adcx_operand)); + assert(!!(flags & CC_O) == (in_o || adox_operand)); +} + +int main(int argc, char *argv[]) { + /* try all combinations of input CF, input OF, CF from op1+op2, OF from op2+op1 */ + int i; + for (i = 0; i <= 15; i++) { + printf("%d\n", i); + test_adcx_adox(!!(i & 1), !!(i & 2), !!(i & 4), !!(i & 8)); + test_adox_adcx(!!(i & 1), !!(i & 2), !!(i & 4), !!(i & 8)); + } + return 0; +} + -- cgit v1.2.3 From 786c5256d3262518d8805ac2a62eb1c4a3813b80 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 13 Jan 2023 11:27:14 +0100 Subject: libqtest: split qtest_spawn_qemu function In order to create a function that allows testing of invalid command lines, extract the parts of qtest_init_without_qmp_handshake that do not require any successful set up of sockets. Signed-off-by: Paolo Bonzini --- tests/qtest/libqtest.c | 105 ++++++++++++++++++++++++++----------------------- 1 file changed, 56 insertions(+), 49 deletions(-) diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index d658222a19..4e1f4fb91c 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -360,60 +360,25 @@ static pid_t qtest_create_process(char *cmd) } #endif /* _WIN32 */ -QTestState *qtest_init_without_qmp_handshake(const char *extra_args) +static QTestState *G_GNUC_PRINTF(1, 2) qtest_spawn_qemu(const char *fmt, ...) { - QTestState *s; - int sock, qmpsock, i; - gchar *socket_path; - gchar *qmp_socket_path; - gchar *command; - const char *qemu_binary = qtest_qemu_binary(); + va_list ap; + QTestState *s = g_new0(QTestState, 1); const char *trace = g_getenv("QTEST_TRACE"); g_autofree char *tracearg = trace ? g_strdup_printf("-trace %s ", trace) : g_strdup(""); + g_autoptr(GString) command = g_string_new(""); - s = g_new(QTestState, 1); - - socket_path = g_strdup_printf("%s/qtest-%d.sock", - g_get_tmp_dir(), getpid()); - qmp_socket_path = g_strdup_printf("%s/qtest-%d.qmp", - g_get_tmp_dir(), getpid()); - - /* It's possible that if an earlier test run crashed it might - * have left a stale unix socket lying around. Delete any - * stale old socket to avoid spurious test failures with - * tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1) - */ - unlink(socket_path); - unlink(qmp_socket_path); - - socket_init(); - sock = init_socket(socket_path); - qmpsock = init_socket(qmp_socket_path); - - qtest_client_set_rx_handler(s, qtest_client_socket_recv_line); - qtest_client_set_tx_handler(s, qtest_client_socket_send); + va_start(ap, fmt); + g_string_append_printf(command, CMD_EXEC "%s %s", + qtest_qemu_binary(), tracearg); + g_string_append_vprintf(command, fmt, ap); + va_end(ap); qtest_add_abrt_handler(kill_qemu_hook_func, s); - command = g_strdup_printf(CMD_EXEC "%s %s" - "-qtest unix:%s " - "-qtest-log %s " - "-chardev socket,path=%s,id=char0 " - "-mon chardev=char0,mode=control " - "-display none " - "%s" - " -accel qtest", - qemu_binary, tracearg, socket_path, - getenv("QTEST_LOG") ? DEV_STDERR : DEV_NULL, - qmp_socket_path, - extra_args ?: ""); - - g_test_message("starting QEMU: %s", command); - - s->pending_events = NULL; - s->wstatus = 0; - s->expected_status = 0; + g_test_message("starting QEMU: %s", command->str); + #ifndef _WIN32 s->qemu_pid = fork(); if (s->qemu_pid == 0) { @@ -434,14 +399,56 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args) if (!g_setenv("QEMU_AUDIO_DRV", "none", true)) { exit(1); } - execlp("/bin/sh", "sh", "-c", command, NULL); + execlp("/bin/sh", "sh", "-c", command->str, NULL); exit(1); } #else - s->qemu_pid = qtest_create_process(command); + s->qemu_pid = qtest_create_process(command->str); #endif /* _WIN32 */ - g_free(command); + return s; +} + +QTestState *qtest_init_without_qmp_handshake(const char *extra_args) +{ + QTestState *s; + int sock, qmpsock, i; + gchar *socket_path; + gchar *qmp_socket_path; + + socket_path = g_strdup_printf("%s/qtest-%d.sock", + g_get_tmp_dir(), getpid()); + qmp_socket_path = g_strdup_printf("%s/qtest-%d.qmp", + g_get_tmp_dir(), getpid()); + + /* + * It's possible that if an earlier test run crashed it might + * have left a stale unix socket lying around. Delete any + * stale old socket to avoid spurious test failures with + * tests/libqtest.c:70:init_socket: assertion failed (ret != -1): (-1 != -1) + */ + unlink(socket_path); + unlink(qmp_socket_path); + + socket_init(); + sock = init_socket(socket_path); + qmpsock = init_socket(qmp_socket_path); + + s = qtest_spawn_qemu("-qtest unix:%s " + "-qtest-log %s " + "-chardev socket,path=%s,id=char0 " + "-mon chardev=char0,mode=control " + "-display none " + "%s" + " -accel qtest", + socket_path, + getenv("QTEST_LOG") ? DEV_STDERR : DEV_NULL, + qmp_socket_path, + extra_args ?: ""); + + qtest_client_set_rx_handler(s, qtest_client_socket_recv_line); + qtest_client_set_tx_handler(s, qtest_client_socket_send); + s->fd = socket_accept(sock); if (s->fd >= 0) { s->qmp_fd = socket_accept(qmpsock); -- cgit v1.2.3 From 12008ff748d8cfb62fb937559c0fd844371bab5e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 13 Jan 2023 12:01:20 +0100 Subject: libqtest: ensure waitpid() is only called once If a test aborts after qtest_wait_qemu() is called, the SIGABRT hooks are still in place and waitpid() is called again. The second time it is called, the process does not exist anymore and the system call fails. Move the s->qemu_pid = -1 assignment to qtest_wait_qemu() to make it idempotent, and anyway remove the SIGABRT hook as well to avoid that qtest_check_status() is called twice. Because of the extra call, qtest_remove_abrt_handler() now has to be made idempotent as well. Signed-off-by: Paolo Bonzini --- tests/qtest/libqtest.c | 55 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 33 insertions(+), 22 deletions(-) diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c index 4e1f4fb91c..2bfd460531 100644 --- a/tests/qtest/libqtest.c +++ b/tests/qtest/libqtest.c @@ -158,6 +158,7 @@ bool qtest_probe_child(QTestState *s) CloseHandle((HANDLE)pid); #endif s->qemu_pid = -1; + qtest_remove_abrt_handler(s); } return false; } @@ -169,6 +170,8 @@ void qtest_set_expected_status(QTestState *s, int status) static void qtest_check_status(QTestState *s) { + assert(s->qemu_pid == -1); + /* * Check whether qemu exited with expected exit status; anything else is * fishy and should be logged with as much detail as possible. @@ -202,36 +205,40 @@ static void qtest_check_status(QTestState *s) void qtest_wait_qemu(QTestState *s) { + if (s->qemu_pid != -1) { #ifndef _WIN32 - pid_t pid; - uint64_t end; + pid_t pid; + uint64_t end; - /* poll for a while until sending SIGKILL */ - end = g_get_monotonic_time() + WAITPID_TIMEOUT * G_TIME_SPAN_SECOND; + /* poll for a while until sending SIGKILL */ + end = g_get_monotonic_time() + WAITPID_TIMEOUT * G_TIME_SPAN_SECOND; - do { - pid = waitpid(s->qemu_pid, &s->wstatus, WNOHANG); - if (pid != 0) { - break; - } - g_usleep(100 * 1000); - } while (g_get_monotonic_time() < end); + do { + pid = waitpid(s->qemu_pid, &s->wstatus, WNOHANG); + if (pid != 0) { + break; + } + g_usleep(100 * 1000); + } while (g_get_monotonic_time() < end); - if (pid == 0) { - kill(s->qemu_pid, SIGKILL); - pid = RETRY_ON_EINTR(waitpid(s->qemu_pid, &s->wstatus, 0)); - } + if (pid == 0) { + kill(s->qemu_pid, SIGKILL); + pid = RETRY_ON_EINTR(waitpid(s->qemu_pid, &s->wstatus, 0)); + } - assert(pid == s->qemu_pid); + assert(pid == s->qemu_pid); #else - DWORD ret; + DWORD ret; - ret = WaitForSingleObject((HANDLE)s->qemu_pid, INFINITE); - assert(ret == WAIT_OBJECT_0); - GetExitCodeProcess((HANDLE)s->qemu_pid, &s->exit_code); - CloseHandle((HANDLE)s->qemu_pid); + ret = WaitForSingleObject((HANDLE)s->qemu_pid, INFINITE); + assert(ret == WAIT_OBJECT_0); + GetExitCodeProcess((HANDLE)s->qemu_pid, &s->exit_code); + CloseHandle((HANDLE)s->qemu_pid); #endif + s->qemu_pid = -1; + qtest_remove_abrt_handler(s); + } qtest_check_status(s); } @@ -245,7 +252,6 @@ void qtest_kill_qemu(QTestState *s) TerminateProcess((HANDLE)s->qemu_pid, s->expected_status); #endif qtest_wait_qemu(s); - s->qemu_pid = -1; return; } @@ -307,6 +313,11 @@ void qtest_add_abrt_handler(GHookFunc fn, const void *data) void qtest_remove_abrt_handler(void *data) { GHook *hook = g_hook_find_data(&abrt_hooks, TRUE, data); + + if (!hook) { + return; + } + g_hook_destroy_link(&abrt_hooks, hook); /* Uninstall SIGABRT handler on last instance */ -- cgit v1.2.3