From 4903602eae82787b1ade04efd9bb5949c04571d6 Mon Sep 17 00:00:00 2001 From: Pan Nengyuan Date: Fri, 10 Jan 2020 17:17:09 +0800 Subject: [PATCH 1/9] vl: Don't mismatch g_strsplit()/g_free() It's a mismatch between g_strsplit and g_free, it will cause a memory leak as follow: [root@localhost]# ./aarch64-softmmu/qemu-system-aarch64 -accel help Accelerators supported in QEMU binary: tcg kvm ================================================================= ==1207900==ERROR: LeakSanitizer: detected memory leaks Direct leak of 8 byte(s) in 2 object(s) allocated from: #0 0xfffd700231cb in __interceptor_malloc (/lib64/libasan.so.4+0xd31cb) #1 0xfffd6ec57163 in g_malloc (/lib64/libglib-2.0.so.0+0x57163) #2 0xfffd6ec724d7 in g_strndup (/lib64/libglib-2.0.so.0+0x724d7) #3 0xfffd6ec73d3f in g_strsplit (/lib64/libglib-2.0.so.0+0x73d3f) #4 0xaaab66be5077 in main /mnt/sdc/qemu-master/qemu-4.2.0-rc0/vl.c:3517 #5 0xfffd6e140b9f in __libc_start_main (/lib64/libc.so.6+0x20b9f) #6 0xaaab66bf0f53 (./build/aarch64-softmmu/qemu-system-aarch64+0x8a0f53) Direct leak of 2 byte(s) in 2 object(s) allocated from: #0 0xfffd700231cb in __interceptor_malloc (/lib64/libasan.so.4+0xd31cb) #1 0xfffd6ec57163 in g_malloc (/lib64/libglib-2.0.so.0+0x57163) #2 0xfffd6ec7243b in g_strdup (/lib64/libglib-2.0.so.0+0x7243b) #3 0xfffd6ec73e6f in g_strsplit (/lib64/libglib-2.0.so.0+0x73e6f) #4 0xaaab66be5077 in main /mnt/sdc/qemu-master/qemu-4.2.0-rc0/vl.c:3517 #5 0xfffd6e140b9f in __libc_start_main (/lib64/libc.so.6+0x20b9f) #6 0xaaab66bf0f53 (./build/aarch64-softmmu/qemu-system-aarch64+0x8a0f53) Reported-by: Euler Robot Signed-off-by: Pan Nengyuan Message-Id: <20200110091710.53424-2-pannengyuan@huawei.com> Signed-off-by: Paolo Bonzini --- vl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vl.c b/vl.c index 7dcb0879c4..c5beda7d48 100644 --- a/vl.c +++ b/vl.c @@ -3501,7 +3501,7 @@ int main(int argc, char **argv, char **envp) gchar **optname = g_strsplit(typename, ACCEL_CLASS_SUFFIX, 0); printf("%s\n", optname[0]); - g_free(optname); + g_strfreev(optname); } g_free(typename); } From e261b36810de8dfd825b6b01fff43a6bd6f2bd8d Mon Sep 17 00:00:00 2001 From: Luc Michel Date: Wed, 29 Jan 2020 15:49:48 +0100 Subject: [PATCH 2/9] seqlock: fix seqlock_write_unlock_impl function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The seqlock write unlock function was incorrectly calling seqlock_write_begin() instead of seqlock_write_end(), and was releasing the lock before incrementing the sequence. This could lead to a race condition and a corrupted sequence number becoming odd even though the lock is not held. Signed-off-by: Luc Michel Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20200129144948.2161551-1-luc.michel@greensocs.com> Fixes: 988fcafc73 ("seqlock: add QemuLockable support", 2018-08-23) Signed-off-by: Paolo Bonzini --- include/qemu/seqlock.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/qemu/seqlock.h b/include/qemu/seqlock.h index fd408b7ec5..8b6b4ee4bb 100644 --- a/include/qemu/seqlock.h +++ b/include/qemu/seqlock.h @@ -55,11 +55,11 @@ static inline void seqlock_write_lock_impl(QemuSeqLock *sl, QemuLockable *lock) #define seqlock_write_lock(sl, lock) \ seqlock_write_lock_impl(sl, QEMU_MAKE_LOCKABLE(lock)) -/* Lock out other writers and update the count. */ +/* Update the count and release the lock. */ static inline void seqlock_write_unlock_impl(QemuSeqLock *sl, QemuLockable *lock) { + seqlock_write_end(sl); qemu_lockable_unlock(lock); - seqlock_write_begin(sl); } #define seqlock_write_unlock(sl, lock) \ seqlock_write_unlock_impl(sl, QEMU_MAKE_LOCKABLE(lock)) From a284f798f356ccb9e2c5c6dcae08c92da3b22114 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Sat, 1 Feb 2020 17:22:52 +0000 Subject: [PATCH 3/9] Remove support for CLOCK_MONOTONIC not being defined Some older parts of QEMU's codebase assume that CLOCK_MONOTONIC might not be defined by the host OS, and have workarounds to deal with this. However, more recently (notably in commit 50290c002c045280f8d for qemu-img in mid-2019, but also much earlier in 2011 in commit 22795174a37e0 for ui/spice-display.c) we've written code that assumes CLOCK_MONOTONIC is always defined. The only host OS anybody's ever noticed this on is OSX 10.11 and earlier, which we don't support. So we can assume that all our host OSes have the #define, and we can remove some now-unnecessary ifdefs. Signed-off-by: Peter Maydell Message-Id: <20200201172252.6605-1-peter.maydell@linaro.org> Signed-off-by: Paolo Bonzini --- include/qemu/timer.h | 5 +---- util/qemu-timer-common.c | 11 ++++------- 2 files changed, 5 insertions(+), 11 deletions(-) diff --git a/include/qemu/timer.h b/include/qemu/timer.h index 85bc6eb00b..6a8b48b5a9 100644 --- a/include/qemu/timer.h +++ b/include/qemu/timer.h @@ -838,14 +838,11 @@ extern int use_rt_clock; static inline int64_t get_clock(void) { -#ifdef CLOCK_MONOTONIC if (use_rt_clock) { struct timespec ts; clock_gettime(CLOCK_MONOTONIC, &ts); return ts.tv_sec * 1000000000LL + ts.tv_nsec; - } else -#endif - { + } else { /* XXX: using gettimeofday leads to problems if the date changes, so it should be avoided. */ return get_clock_realtime(); diff --git a/util/qemu-timer-common.c b/util/qemu-timer-common.c index 06d084d364..baf3317f74 100644 --- a/util/qemu-timer-common.c +++ b/util/qemu-timer-common.c @@ -49,14 +49,11 @@ int use_rt_clock; static void __attribute__((constructor)) init_get_clock(void) { + struct timespec ts; + use_rt_clock = 0; -#ifdef CLOCK_MONOTONIC - { - struct timespec ts; - if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) { - use_rt_clock = 1; - } + if (clock_gettime(CLOCK_MONOTONIC, &ts) == 0) { + use_rt_clock = 1; } -#endif } #endif From 1b29af2f41227aaa45f6331a993fe4afe45d53c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Sat, 17 Aug 2019 12:05:17 +0400 Subject: [PATCH 4/9] minikconf: accept alnum identifiers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Marc-André Lureau Signed-off-by: Paolo Bonzini --- scripts/minikconf.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/minikconf.py b/scripts/minikconf.py index 40ae1989e1..febd9a479f 100644 --- a/scripts/minikconf.py +++ b/scripts/minikconf.py @@ -645,7 +645,7 @@ class KconfigParser: self.cursor = self.src.find('\n', self.cursor) self.val = self.src[start:self.cursor] return TOK_SOURCE - elif self.tok.isalpha(): + elif self.tok.isalnum(): # identifier while self.src[self.cursor].isalnum() or self.src[self.cursor] == '_': self.cursor += 1 From fe3dada3172310c7d3db666b4223d89e6c6f7fa3 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 4 Feb 2020 17:10:36 +0100 Subject: [PATCH 5/9] exec: do not define use_icount for user-mode emulation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit use_icount is also defined by stubs/cpu-get-icount.c, we do not need to have a useless definition in exec.c. Signed-off-by: Paolo Bonzini Message-id: <20200204161036.20889-1-pbonzini@redhat.com> Reviewed-by: Alex Bennée --- exec.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/exec.c b/exec.c index 67e520d18e..5fc3746053 100644 --- a/exec.c +++ b/exec.c @@ -98,15 +98,15 @@ CPUTailQ cpus = QTAILQ_HEAD_INITIALIZER(cpus); /* current CPU in the current thread. It is only valid inside cpu_exec() */ __thread CPUState *current_cpu; -/* 0 = Do not count executed instructions. - 1 = Precise instruction counting. - 2 = Adaptive rate instruction counting. */ -int use_icount; uintptr_t qemu_host_page_size; intptr_t qemu_host_page_mask; #if !defined(CONFIG_USER_ONLY) +/* 0 = Do not count executed instructions. + 1 = Precise instruction counting. + 2 = Adaptive rate instruction counting. */ +int use_icount; typedef struct PhysPageEntry PhysPageEntry; From 4cc600d22906a839719116043dbc3760e02c756e Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 4 Feb 2020 17:11:04 +0100 Subject: [PATCH 6/9] build: move TARGET_GPROF to config-host.mak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TARGET_GPROF is the same for all targets, write it to config-host.mak instead. Reviewed-by: Alex Bennée Reviewed-by: Marc-André Lureau Reviewed-by: Philippe Mathieu-Daudé Message-id: <20200204161104.21077-1-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- bsd-user/syscall.c | 6 +++--- configure | 4 +++- linux-user/exit.c | 4 ++-- linux-user/signal.c | 2 +- tests/check-block.sh | 2 +- 5 files changed, 10 insertions(+), 8 deletions(-) diff --git a/bsd-user/syscall.c b/bsd-user/syscall.c index 0d45b654bb..d38ec7a162 100644 --- a/bsd-user/syscall.c +++ b/bsd-user/syscall.c @@ -330,7 +330,7 @@ abi_long do_freebsd_syscall(void *cpu_env, int num, abi_long arg1, switch(num) { case TARGET_FREEBSD_NR_exit: -#ifdef TARGET_GPROF +#ifdef CONFIG_GPROF _mcleanup(); #endif gdb_exit(cpu_env, arg1); @@ -432,7 +432,7 @@ abi_long do_netbsd_syscall(void *cpu_env, int num, abi_long arg1, switch(num) { case TARGET_NETBSD_NR_exit: -#ifdef TARGET_GPROF +#ifdef CONFIG_GPROF _mcleanup(); #endif gdb_exit(cpu_env, arg1); @@ -511,7 +511,7 @@ abi_long do_openbsd_syscall(void *cpu_env, int num, abi_long arg1, switch(num) { case TARGET_OPENBSD_NR_exit: -#ifdef TARGET_GPROF +#ifdef CONFIG_GPROF _mcleanup(); #endif gdb_exit(cpu_env, arg1); diff --git a/configure b/configure index 115dc38085..16f94cd96b 100755 --- a/configure +++ b/configure @@ -6771,6 +6771,9 @@ fi if test "$l2tpv3" = "yes" ; then echo "CONFIG_L2TPV3=y" >> $config_host_mak fi +if test "$gprof" = "yes" ; then + echo "CONFIG_GPROF=y" >> $config_host_mak +fi if test "$cap_ng" = "yes" ; then echo "CONFIG_LIBCAP_NG=y" >> $config_host_mak fi @@ -7951,7 +7954,6 @@ alpha) esac if test "$gprof" = "yes" ; then - echo "TARGET_GPROF=y" >> $config_target_mak if test "$target_linux_user" = "yes" ; then cflags="-p $cflags" ldflags="-p $ldflags" diff --git a/linux-user/exit.c b/linux-user/exit.c index a362ef67d2..1594015444 100644 --- a/linux-user/exit.c +++ b/linux-user/exit.c @@ -18,7 +18,7 @@ */ #include "qemu/osdep.h" #include "qemu.h" -#ifdef TARGET_GPROF +#ifdef CONFIG_GPROF #include #endif @@ -28,7 +28,7 @@ extern void __gcov_dump(void); void preexit_cleanup(CPUArchState *env, int code) { -#ifdef TARGET_GPROF +#ifdef CONFIG_GPROF _mcleanup(); #endif #ifdef CONFIG_GCOV diff --git a/linux-user/signal.c b/linux-user/signal.c index 5ca6d62b15..02f860ecb9 100644 --- a/linux-user/signal.c +++ b/linux-user/signal.c @@ -509,7 +509,7 @@ void signal_init(void) act.sa_flags = SA_SIGINFO; act.sa_sigaction = host_signal_handler; for(i = 1; i <= TARGET_NSIG; i++) { -#ifdef TARGET_GPROF +#ifdef CONFIG_GPROF if (i == SIGPROF) { continue; } diff --git a/tests/check-block.sh b/tests/check-block.sh index 679aedec50..ad320c21ba 100755 --- a/tests/check-block.sh +++ b/tests/check-block.sh @@ -16,7 +16,7 @@ if [ "$#" -ne 0 ]; then format_list="$@" fi -if grep -q "TARGET_GPROF=y" *-softmmu/config-target.mak 2>/dev/null ; then +if grep -q "CONFIG_GPROF=y" config-host.mak 2>/dev/null ; then echo "GPROF is enabled ==> Not running the qemu-iotests." exit 0 fi From 9028c75c9d08be303ccc425bfe3d3b23d8f4cac7 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 6 Feb 2020 18:10:22 +0100 Subject: [PATCH 7/9] target/i386: fix TCG UCODE_REV access This was a very interesting semantic conflict that caused git to move the MSR_IA32_UCODE_REV read to helper_wrmsr. Not a big deal, but still should be fixed... Fixes: 4e45aff398 ("target/i386: add a ucode-rev property", 2020-01-24) Message-id: <20200206171022.9289-1-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- target/i386/misc_helper.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/target/i386/misc_helper.c b/target/i386/misc_helper.c index aed16fe3f0..7d61221024 100644 --- a/target/i386/misc_helper.c +++ b/target/i386/misc_helper.c @@ -229,7 +229,6 @@ void helper_rdmsr(CPUX86State *env) #else void helper_wrmsr(CPUX86State *env) { - X86CPU *x86_cpu = env_archcpu(env); uint64_t val; cpu_svm_check_intercept_param(env, SVM_EXIT_MSR, 1, GETPC()); @@ -372,9 +371,6 @@ void helper_wrmsr(CPUX86State *env) env->msr_bndcfgs = val; cpu_sync_bndcs_hflags(env); break; - case MSR_IA32_UCODE_REV: - val = x86_cpu->ucode_rev; - break; default: if ((uint32_t)env->regs[R_ECX] >= MSR_MC0_CTL && (uint32_t)env->regs[R_ECX] < MSR_MC0_CTL + @@ -393,6 +389,7 @@ void helper_wrmsr(CPUX86State *env) void helper_rdmsr(CPUX86State *env) { + X86CPU *x86_cpu = env_archcpu(env); uint64_t val; cpu_svm_check_intercept_param(env, SVM_EXIT_MSR, 0, GETPC()); @@ -526,6 +523,9 @@ void helper_rdmsr(CPUX86State *env) case MSR_IA32_BNDCFGS: val = env->msr_bndcfgs; break; + case MSR_IA32_UCODE_REV: + val = x86_cpu->ucode_rev; + break; default: if ((uint32_t)env->regs[R_ECX] >= MSR_MC0_CTL && (uint32_t)env->regs[R_ECX] < MSR_MC0_CTL + From 6702514814c7e7b4cbf179624539b5f38c72740b Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 11 Feb 2020 18:55:16 +0100 Subject: [PATCH 8/9] target/i386: check for availability of MSR_IA32_UCODE_REV as an emulated MSR Even though MSR_IA32_UCODE_REV has been available long before Linux 5.6, which added it to the emulated MSR list, a bug caused the microcode version to revert to 0x100000000 on INIT. As a result, processors other than the bootstrap processor would not see the host microcode revision; some Windows version complain loudly about this and crash with a fairly explicit MICROCODE REVISION MISMATCH error. [If running 5.6 prereleases, the kernel fix "KVM: x86: do not reset microcode version on INIT or RESET" should also be applied.] Reported-by: Alex Williamson Message-id: <20200211175516.10716-1-pbonzini@redhat.com> Signed-off-by: Paolo Bonzini --- target/i386/kvm.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/target/i386/kvm.c b/target/i386/kvm.c index 6ef291d580..69eb43d796 100644 --- a/target/i386/kvm.c +++ b/target/i386/kvm.c @@ -105,6 +105,7 @@ static bool has_msr_smi_count; static bool has_msr_arch_capabs; static bool has_msr_core_capabs; static bool has_msr_vmx_vmfunc; +static bool has_msr_ucode_rev; static uint32_t has_architectural_pmu_version; static uint32_t num_architectural_pmu_gp_counters; @@ -2056,6 +2057,9 @@ static int kvm_get_supported_msrs(KVMState *s) case MSR_IA32_VMX_VMFUNC: has_msr_vmx_vmfunc = true; break; + case MSR_IA32_UCODE_REV: + has_msr_ucode_rev = true; + break; } } } @@ -2696,8 +2700,7 @@ static void kvm_init_msrs(X86CPU *cpu) env->features[FEAT_CORE_CAPABILITY]); } - if (kvm_arch_get_supported_msr_feature(kvm_state, - MSR_IA32_UCODE_REV)) { + if (has_msr_ucode_rev) { kvm_msr_entry_add(cpu, MSR_IA32_UCODE_REV, cpu->ucode_rev); } From be02cda3afde60d219786e23c3f8edb53aec8e17 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Tue, 11 Feb 2020 18:47:48 +0100 Subject: [PATCH 9/9] target/i386: enable monitor and ucode revision with -cpu max These two features were incorrectly tied to host_cpuid_required rather than cpu->max_features. As a result, -cpu max was not enabling either MONITOR features or ucode revision. Signed-off-by: Paolo Bonzini --- target/i386/cpu.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/target/i386/cpu.c b/target/i386/cpu.c index 32efa46852..92fafa2659 100644 --- a/target/i386/cpu.c +++ b/target/i386/cpu.c @@ -6414,7 +6414,9 @@ static void x86_cpu_realizefn(DeviceState *dev, Error **errp) error_setg(&local_err, "CPU model '%s' requires KVM", name); goto out; } + } + if (cpu->max_features && accel_uses_host_cpuid()) { if (enable_cpu_pm) { host_cpuid(5, 0, &cpu->mwait.eax, &cpu->mwait.ebx, &cpu->mwait.ecx, &cpu->mwait.edx);