[PULL 0/9] Mini misc patches queue for 2020-02-12

[PULL 0/9] Mini misc patches queue for 2020-02-12

[Paolo Bonzini]

The following changes since commit 7bd9d0a9e26c7a3c67c0f174f0009ba19969b158:

  Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2020-02-04' into staging (2020-02-04 16:12:31 +0000)

are available in the Git repository at:

  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to be02cda3afde60d219786e23c3f8edb53aec8e17:

  target/i386: enable monitor and ucode revision with -cpu max (2020-02-12 16:29:51 +0100)

----------------------------------------------------------------
* various small fixes and cleanups
* fixes for the ucode revision patch from the previous pull request

----------------------------------------------------------------
Luc Michel (1):
      seqlock: fix seqlock_write_unlock_impl function

Marc-André Lureau (1):
      minikconf: accept alnum identifiers

Pan Nengyuan (1):
      vl: Don't mismatch g_strsplit()/g_free()

Paolo Bonzini (5):
      exec: do not define use_icount for user-mode emulation
      build: move TARGET_GPROF to config-host.mak
      target/i386: fix TCG UCODE_REV access
      target/i386: check for availability of MSR_IA32_UCODE_REV as an emulated MSR
      target/i386: enable monitor and ucode revision with -cpu max

Peter Maydell (1):
      Remove support for CLOCK_MONOTONIC not being defined

 bsd-user/syscall.c        |  6 +++---
 configure                 |  4 +++-
 exec.c                    |  8 ++++----
 include/qemu/seqlock.h    |  4 ++--
 include/qemu/timer.h      |  5 +----
 linux-user/exit.c         |  4 ++--
 linux-user/signal.c       |  2 +-
 scripts/minikconf.py      |  2 +-
 target/i386/cpu.c         |  2 ++
 target/i386/kvm.c         |  7 +++++--
 target/i386/misc_helper.c |  8 ++++----
 tests/check-block.sh      |  2 +-
 util/qemu-timer-common.c  | 11 ++++-------
 vl.c                      |  2 +-
 14 files changed, 34 insertions(+), 33 deletions(-)
-- 
2.21.0

[PULL 1/9] vl: Don't mismatch g_strsplit()/g_free()

[Paolo Bonzini]

From: Pan Nengyuan <pannengyuan@huawei.com>

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 <euler.robot@huawei.com>
Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com>
Message-Id: <20200110091710.53424-2-pannengyuan@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 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);
                     }
-- 
2.21.0

[PULL 2/9] seqlock: fix seqlock_write_unlock_impl function

[Paolo Bonzini]

From: Luc Michel <luc.michel@greensocs.com>

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 <luc.michel@greensocs.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200129144948.2161551-1-luc.michel@greensocs.com>
Fixes: 988fcafc73 ("seqlock: add QemuLockable support", 2018-08-23)
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 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))
-- 
2.21.0

[PULL 3/9] Remove support for CLOCK_MONOTONIC not being defined

[Paolo Bonzini]

From: Peter Maydell <peter.maydell@linaro.org>

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 <peter.maydell@linaro.org>
Message-Id: <20200201172252.6605-1-peter.maydell@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 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
-- 
2.21.0

[PULL 4/9] minikconf: accept alnum identifiers

[Paolo Bonzini]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 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
-- 
2.21.0

[PULL 5/9] exec: do not define use_icount for user-mode emulation

[Paolo Bonzini]

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 <pbonzini@redhat.com>
Message-id: <20200204161036.20889-1-pbonzini@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 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;
 
-- 
2.21.0

[PULL 6/9] build: move TARGET_GPROF to config-host.mak

[Paolo Bonzini]

TARGET_GPROF is the same for all targets, write it to
config-host.mak instead.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: <20200204161104.21077-1-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 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 <sys/gmon.h>
 #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
-- 
2.21.0

[PULL 7/9] target/i386: fix TCG UCODE_REV access

[Paolo Bonzini]

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 <pbonzini@redhat.com>
---
 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 +
-- 
2.21.0

[PULL 8/9] target/i386: check for availability of MSR_IA32_UCODE_REV as an emulated MSR

[Paolo Bonzini]

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 <alex.williamson@redhat.com>
Message-id: <20200211175516.10716-1-pbonzini@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 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);
     }
 
-- 
2.21.0

[PULL 9/9] target/i386: enable monitor and ucode revision with -cpu max

[Paolo Bonzini]

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 <pbonzini@redhat.com>
---
 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);
-- 
2.21.0

Re: [PULL 0/9] Mini misc patches queue for 2020-02-12

[Peter Maydell]

On Wed, 12 Feb 2020 at 16:42, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> The following changes since commit 7bd9d0a9e26c7a3c67c0f174f0009ba19969b158:
>
>   Merge remote-tracking branch 'remotes/huth-gitlab/tags/pull-request-2020-02-04' into staging (2020-02-04 16:12:31 +0000)
>
> are available in the Git repository at:
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to be02cda3afde60d219786e23c3f8edb53aec8e17:
>
>   target/i386: enable monitor and ucode revision with -cpu max (2020-02-12 16:29:51 +0100)
>
> ----------------------------------------------------------------
> * various small fixes and cleanups
> * fixes for the ucode revision patch from the previous pull request
>



Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/5.0
for any user-visible changes.

-- PMM