[Qemu-devel] [PULL 0/3] KVM patches for 2013-12-06

[Qemu-devel] [PULL 0/3] KVM patches for 2013-12-06

[Paolo Bonzini]

The following changes since commit 607bb022f2a44797cbf40e85e84da4134e2f0e01:

  Update version for 1.7.0-rc1 release (2013-11-21 08:11:47 -0800)

are available in the git repository at:
  git://git.kernel.org/pub/scm/virt/kvm/qemu-kvm.git uq/master

Alex Williamson (1):
      kvm: Query KVM for available memory slots

Liu Jinsong (1):
      target-i386: fix cpuid leaf 0x0d

Marcelo Tosatti (1):
      qemu: mempath: prefault pages manually (v4)

 exec.c            |   59 ++++++++++++++++++++++++++++++++++++++++++----------
 kvm-all.c         |   30 ++++++++++++++++++--------
 qemu-options.hx   |    2 -
 target-i386/cpu.c |    6 ++--
 vl.c              |    4 ---
 5 files changed, 71 insertions(+), 30 deletions(-)
---
 kvm-all.c |   30 +++++++++++++++++++++---------
 1 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/kvm-all.c b/kvm-all.c
index 4478969..3937754 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -72,7 +72,8 @@ typedef struct kvm_dirty_log KVMDirtyLog;
 
 struct KVMState
 {
-    KVMSlot slots[32];
+    KVMSlot *slots;
+    int nr_slots;
     int fd;
     int vmfd;
     int coalesced_mmio;
@@ -125,7 +126,7 @@ static KVMSlot *kvm_alloc_slot(KVMState *s)
 {
     int i;
 
-    for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
+    for (i = 0; i < s->nr_slots; i++) {
         if (s->slots[i].memory_size == 0) {
             return &s->slots[i];
         }
@@ -141,7 +142,7 @@ static KVMSlot *kvm_lookup_matching_slot(KVMState *s,
 {
     int i;
 
-    for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
+    for (i = 0; i < s->nr_slots; i++) {
         KVMSlot *mem = &s->slots[i];
 
         if (start_addr == mem->start_addr &&
@@ -163,7 +164,7 @@ static KVMSlot *kvm_lookup_overlapping_slot(KVMState *s,
     KVMSlot *found = NULL;
     int i;
 
-    for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
+    for (i = 0; i < s->nr_slots; i++) {
         KVMSlot *mem = &s->slots[i];
 
         if (mem->memory_size == 0 ||
@@ -185,7 +186,7 @@ int kvm_physical_memory_addr_from_host(KVMState *s, void *ram,
 {
     int i;
 
-    for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
+    for (i = 0; i < s->nr_slots; i++) {
         KVMSlot *mem = &s->slots[i];
 
         if (ram >= mem->ram && ram < mem->ram + mem->memory_size) {
@@ -357,7 +358,7 @@ static int kvm_set_migration_log(int enable)
 
     s->migration_log = enable;
 
-    for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
+    for (i = 0; i < s->nr_slots; i++) {
         mem = &s->slots[i];
 
         if (!mem->memory_size) {
@@ -1383,9 +1384,6 @@ int kvm_init(void)
 #ifdef KVM_CAP_SET_GUEST_DEBUG
     QTAILQ_INIT(&s->kvm_sw_breakpoints);
 #endif
-    for (i = 0; i < ARRAY_SIZE(s->slots); i++) {
-        s->slots[i].slot = i;
-    }
     s->vmfd = -1;
     s->fd = qemu_open("/dev/kvm", O_RDWR);
     if (s->fd == -1) {
@@ -1409,6 +1407,19 @@ int kvm_init(void)
         goto err;
     }
 
+    s->nr_slots = kvm_check_extension(s, KVM_CAP_NR_MEMSLOTS);
+
+    /* If unspecified, use the default value */
+    if (!s->nr_slots) {
+        s->nr_slots = 32;
+    }
+
+    s->slots = g_malloc0(s->nr_slots * sizeof(KVMSlot));
+
+    for (i = 0; i < s->nr_slots; i++) {
+        s->slots[i].slot = i;
+    }
+
     /* check the vcpu limits */
     soft_vcpus_limit = kvm_recommended_vcpus(s);
     hard_vcpus_limit = kvm_max_vcpus(s);
@@ -1527,6 +1538,7 @@ err:
     if (s->fd != -1) {
         close(s->fd);
     }
+    g_free(s->slots);
     g_free(s);
 
     return ret;
-- 
1.7.1

[Qemu-devel] [PULL 2/3] qemu: mempath: prefault pages manually (v4)

[Paolo Bonzini]

From: Marcelo Tosatti <mtosatti@redhat.com>

v4: s/fail/failed/  (Peter Maydell)

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
---
 exec.c          |   59 +++++++++++++++++++++++++++++++++++++++++++-----------
 qemu-options.hx |    2 -
 vl.c            |    4 ---
 3 files changed, 47 insertions(+), 18 deletions(-)

diff --git a/exec.c b/exec.c
index 95c4356..f4b9ef2 100644
--- a/exec.c
+++ b/exec.c
@@ -904,6 +904,13 @@ static long gethugepagesize(const char *path)
     return fs.f_bsize;
 }
 
+static sigjmp_buf sigjump;
+
+static void sigbus_handler(int signal)
+{
+    siglongjmp(sigjump, 1);
+}
+
 static void *file_ram_alloc(RAMBlock *block,
                             ram_addr_t memory,
                             const char *path)
@@ -913,9 +920,6 @@ static void *file_ram_alloc(RAMBlock *block,
     char *c;
     void *area;
     int fd;
-#ifdef MAP_POPULATE
-    int flags;
-#endif
     unsigned long hpagesize;
 
     hpagesize = gethugepagesize(path);
@@ -963,21 +967,52 @@ static void *file_ram_alloc(RAMBlock *block,
     if (ftruncate(fd, memory))
         perror("ftruncate");
 
-#ifdef MAP_POPULATE
-    /* NB: MAP_POPULATE won't exhaustively alloc all phys pages in the case
-     * MAP_PRIVATE is requested.  For mem_prealloc we mmap as MAP_SHARED
-     * to sidestep this quirk.
-     */
-    flags = mem_prealloc ? MAP_POPULATE | MAP_SHARED : MAP_PRIVATE;
-    area = mmap(0, memory, PROT_READ | PROT_WRITE, flags, fd, 0);
-#else
     area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
-#endif
     if (area == MAP_FAILED) {
         perror("file_ram_alloc: can't mmap RAM pages");
         close(fd);
         return (NULL);
     }
+
+    if (mem_prealloc) {
+        int ret, i;
+        struct sigaction act, oldact;
+        sigset_t set, oldset;
+
+        memset(&act, 0, sizeof(act));
+        act.sa_handler = &sigbus_handler;
+        act.sa_flags = 0;
+
+        ret = sigaction(SIGBUS, &act, &oldact);
+        if (ret) {
+            perror("file_ram_alloc: failed to install signal handler");
+            exit(1);
+        }
+
+        /* unblock SIGBUS */
+        sigemptyset(&set);
+        sigaddset(&set, SIGBUS);
+        pthread_sigmask(SIG_UNBLOCK, &set, &oldset);
+
+        if (sigsetjmp(sigjump, 1)) {
+            fprintf(stderr, "file_ram_alloc: failed to preallocate pages\n");
+            exit(1);
+        }
+
+        /* MAP_POPULATE silently ignores failures */
+        for (i = 0; i < (memory/hpagesize)-1; i++) {
+            memset(area + (hpagesize*i), 0, 1);
+        }
+
+        ret = sigaction(SIGBUS, &oldact, NULL);
+        if (ret) {
+            perror("file_ram_alloc: failed to reinstall signal handler");
+            exit(1);
+        }
+
+        pthread_sigmask(SIG_SETMASK, &oldset, NULL);
+    }
+
     block->fd = fd;
     return area;
 }
diff --git a/qemu-options.hx b/qemu-options.hx
index 8b94264..eafc022 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -228,7 +228,6 @@ STEXI
 Allocate guest RAM from a temporarily created file in @var{path}.
 ETEXI
 
-#ifdef MAP_POPULATE
 DEF("mem-prealloc", 0, QEMU_OPTION_mem_prealloc,
     "-mem-prealloc   preallocate guest memory (use with -mem-path)\n",
     QEMU_ARCH_ALL)
@@ -237,7 +236,6 @@ STEXI
 @findex -mem-prealloc
 Preallocate memory when using -mem-path.
 ETEXI
-#endif
 
 DEF("k", HAS_ARG, QEMU_OPTION_k,
     "-k language     use keyboard layout (for example 'fr' for French)\n",
diff --git a/vl.c b/vl.c
index 8d5d874..d191f51 100644
--- a/vl.c
+++ b/vl.c
@@ -188,9 +188,7 @@ static int display_remote;
 const char* keyboard_layout = NULL;
 ram_addr_t ram_size;
 const char *mem_path = NULL;
-#ifdef MAP_POPULATE
 int mem_prealloc = 0; /* force preallocation of physical target memory */
-#endif
 int nb_nics;
 NICInfo nd_table[MAX_NICS];
 int autostart;
@@ -3206,11 +3204,9 @@ int main(int argc, char **argv, char **envp)
             case QEMU_OPTION_mempath:
                 mem_path = optarg;
                 break;
-#ifdef MAP_POPULATE
             case QEMU_OPTION_mem_prealloc:
                 mem_prealloc = 1;
                 break;
-#endif
             case QEMU_OPTION_d:
                 log_mask = optarg;
                 break;
-- 
1.7.1

[Qemu-devel] [PULL 3/3] target-i386: fix cpuid leaf 0x0d

[Paolo Bonzini]

From: Liu Jinsong <jinsong.liu@intel.com>

Fix cpuid leaf 0x0d which incorrectly parsed eax and ebx.

However, before this patch the CPUID worked fine -- the .offset
field contained the size _and_ was stored in the register that
is supposed to hold the size (eax), and likewise the .size field
contained the offset _and_ was stored in the register trhat is
supposed to hold the offset (ebx).

Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 target-i386/cpu.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index 47af9a8..bb98f6d 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -335,7 +335,7 @@ typedef struct ExtSaveArea {
 
 static const ExtSaveArea ext_save_areas[] = {
     [2] = { .feature = FEAT_1_ECX, .bits = CPUID_EXT_AVX,
-            .offset = 0x100, .size = 0x240 },
+            .offset = 0x240, .size = 0x100 },
 };
 
 const char *get_register_name_32(unsigned int reg)
@@ -2227,8 +2227,8 @@ void cpu_x86_cpuid(CPUX86State *env, uint32_t index, uint32_t count,
             const ExtSaveArea *esa = &ext_save_areas[count];
             if ((env->features[esa->feature] & esa->bits) == esa->bits &&
                 (kvm_mask & (1 << count)) != 0) {
-                *eax = esa->offset;
-                *ebx = esa->size;
+                *eax = esa->size;
+                *ebx = esa->offset;
             }
         }
         break;
-- 
1.7.1

Re: [Qemu-devel] [PULL 2/3] qemu: mempath: prefault pages manually (v4)

[Stefan Weil]

Hi,

this patch was recently committed to git master.

It now causes problems with gcc-4.7 -Werror=clobbered. See more comments
below.

Am 06.12.2013 16:54, schrieb Paolo Bonzini:
> From: Marcelo Tosatti <mtosatti@redhat.com>
>
> v4: s/fail/failed/  (Peter Maydell)
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> ---
>  exec.c          |   59 +++++++++++++++++++++++++++++++++++++++++++-----------
>  qemu-options.hx |    2 -
>  vl.c            |    4 ---
>  3 files changed, 47 insertions(+), 18 deletions(-)
>
> diff --git a/exec.c b/exec.c
> index 95c4356..f4b9ef2 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -904,6 +904,13 @@ static long gethugepagesize(const char *path)
>      return fs.f_bsize;
>  }
>  
> +static sigjmp_buf sigjump;
> +
> +static void sigbus_handler(int signal)
> +{
> +    siglongjmp(sigjump, 1);
> +}
> +
>  static void *file_ram_alloc(RAMBlock *block,
>                              ram_addr_t memory,
>                              const char *path)
> @@ -913,9 +920,6 @@ static void *file_ram_alloc(RAMBlock *block,
>      char *c;
>      void *area;
>      int fd;
> -#ifdef MAP_POPULATE
> -    int flags;
> -#endif
>      unsigned long hpagesize;
>  
>      hpagesize = gethugepagesize(path);
> @@ -963,21 +967,52 @@ static void *file_ram_alloc(RAMBlock *block,
>      if (ftruncate(fd, memory))
>          perror("ftruncate");
>  
> -#ifdef MAP_POPULATE
> -    /* NB: MAP_POPULATE won't exhaustively alloc all phys pages in the case
> -     * MAP_PRIVATE is requested.  For mem_prealloc we mmap as MAP_SHARED
> -     * to sidestep this quirk.
> -     */
> -    flags = mem_prealloc ? MAP_POPULATE | MAP_SHARED : MAP_PRIVATE;
> -    area = mmap(0, memory, PROT_READ | PROT_WRITE, flags, fd, 0);
> -#else
>      area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
> -#endif
>      if (area == MAP_FAILED) {
>          perror("file_ram_alloc: can't mmap RAM pages");
>          close(fd);
>          return (NULL);
>      }
> +
> +    if (mem_prealloc) {
> +        int ret, i;
> +        struct sigaction act, oldact;
> +        sigset_t set, oldset;
> +
> +        memset(&act, 0, sizeof(act));
> +        act.sa_handler = &sigbus_handler;
> +        act.sa_flags = 0;
> +
> +        ret = sigaction(SIGBUS, &act, &oldact);
> +        if (ret) {
> +            perror("file_ram_alloc: failed to install signal handler");
> +            exit(1);
> +        }
> +
> +        /* unblock SIGBUS */
> +        sigemptyset(&set);
> +        sigaddset(&set, SIGBUS);
> +        pthread_sigmask(SIG_UNBLOCK, &set, &oldset);
> +


The sigsetjmp instruction causes the compiler to assume that 'area'
might be clobbered (sigsetjmp is declared to return twice, and gcc does
not now anything about its semantics).

> +        if (sigsetjmp(sigjump, 1)) {
> +            fprintf(stderr, "file_ram_alloc: failed to preallocate pages\n");
> +            exit(1);
> +        }
> +
> +        /* MAP_POPULATE silently ignores failures */

Yes, but why this comment in this context? Here we don't use
MAP_POPULATE and can get SIGBUS (which is not silent). Or am I wrong?


> +        for (i = 0; i < (memory/hpagesize)-1; i++) {
> +            memset(area + (hpagesize*i), 0, 1);
> +        }
> +
> +        ret = sigaction(SIGBUS, &oldact, NULL);
> +        if (ret) {
> +            perror("file_ram_alloc: failed to reinstall signal handler");
> +            exit(1);
> +        }
> +
> +        pthread_sigmask(SIG_SETMASK, &oldset, NULL);
> +    }
> +
>      block->fd = fd;
>      return area;
>  }


This is the warning shown by newer gcc versions:

qemu/exec.c:921:11: error:
 variable ‘area’ might be clobbered by ‘longjmp’ or ‘vfork’
[-Werror=clobbered]

I want to get support for -Wextra, and -Wclobbered is part of -Wextra.

Of course we could disable -Wclobbered and still use the other
advantages of -Wextra,
but this might hide real problems with clobbered variables in the future.

Declaring the local variable 'area' to be volatile also fixes the warning.

Any opinion how this problem should be solved?

Thanks,
Stefan

Re: [Qemu-devel] [PULL 2/3] qemu: mempath: prefault pages manually (v4)

[Marcelo Tosatti]

On Mon, Dec 09, 2013 at 08:05:15PM +0100, Stefan Weil wrote:
> Hi,
> 
> this patch was recently committed to git master.
> 
> It now causes problems with gcc-4.7 -Werror=clobbered. See more comments
> below.
> 
> Am 06.12.2013 16:54, schrieb Paolo Bonzini:
> > From: Marcelo Tosatti <mtosatti@redhat.com>
> >
> > v4: s/fail/failed/  (Peter Maydell)
> >
> > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
> > ---
> >  exec.c          |   59 +++++++++++++++++++++++++++++++++++++++++++-----------
> >  qemu-options.hx |    2 -
> >  vl.c            |    4 ---
> >  3 files changed, 47 insertions(+), 18 deletions(-)
> >
> > diff --git a/exec.c b/exec.c
> > index 95c4356..f4b9ef2 100644
> > --- a/exec.c
> > +++ b/exec.c
> > @@ -904,6 +904,13 @@ static long gethugepagesize(const char *path)
> >      return fs.f_bsize;
> >  }
> >  
> > +static sigjmp_buf sigjump;
> > +
> > +static void sigbus_handler(int signal)
> > +{
> > +    siglongjmp(sigjump, 1);
> > +}
> > +
> >  static void *file_ram_alloc(RAMBlock *block,
> >                              ram_addr_t memory,
> >                              const char *path)
> > @@ -913,9 +920,6 @@ static void *file_ram_alloc(RAMBlock *block,
> >      char *c;
> >      void *area;
> >      int fd;
> > -#ifdef MAP_POPULATE
> > -    int flags;
> > -#endif
> >      unsigned long hpagesize;
> >  
> >      hpagesize = gethugepagesize(path);
> > @@ -963,21 +967,52 @@ static void *file_ram_alloc(RAMBlock *block,
> >      if (ftruncate(fd, memory))
> >          perror("ftruncate");
> >  
> > -#ifdef MAP_POPULATE
> > -    /* NB: MAP_POPULATE won't exhaustively alloc all phys pages in the case
> > -     * MAP_PRIVATE is requested.  For mem_prealloc we mmap as MAP_SHARED
> > -     * to sidestep this quirk.
> > -     */
> > -    flags = mem_prealloc ? MAP_POPULATE | MAP_SHARED : MAP_PRIVATE;
> > -    area = mmap(0, memory, PROT_READ | PROT_WRITE, flags, fd, 0);
> > -#else
> >      area = mmap(0, memory, PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
> > -#endif
> >      if (area == MAP_FAILED) {
> >          perror("file_ram_alloc: can't mmap RAM pages");
> >          close(fd);
> >          return (NULL);
> >      }
> > +
> > +    if (mem_prealloc) {
> > +        int ret, i;
> > +        struct sigaction act, oldact;
> > +        sigset_t set, oldset;
> > +
> > +        memset(&act, 0, sizeof(act));
> > +        act.sa_handler = &sigbus_handler;
> > +        act.sa_flags = 0;
> > +
> > +        ret = sigaction(SIGBUS, &act, &oldact);
> > +        if (ret) {
> > +            perror("file_ram_alloc: failed to install signal handler");
> > +            exit(1);
> > +        }
> > +
> > +        /* unblock SIGBUS */
> > +        sigemptyset(&set);
> > +        sigaddset(&set, SIGBUS);
> > +        pthread_sigmask(SIG_UNBLOCK, &set, &oldset);
> > +
> 
> 
> The sigsetjmp instruction causes the compiler to assume that 'area'
> might be clobbered (sigsetjmp is declared to return twice, and gcc does
> not now anything about its semantics).
> 
> > +        if (sigsetjmp(sigjump, 1)) {
> > +            fprintf(stderr, "file_ram_alloc: failed to preallocate pages\n");
> > +            exit(1);
> > +        }
> > +
> > +        /* MAP_POPULATE silently ignores failures */
> 
> Yes, but why this comment in this context? Here we don't use
> MAP_POPULATE and can get SIGBUS (which is not silent). Or am I wrong?

It explains why MAP_POPULATE cannot be used.

> > +        for (i = 0; i < (memory/hpagesize)-1; i++) {
> > +            memset(area + (hpagesize*i), 0, 1);
> > +        }
> > +
> > +        ret = sigaction(SIGBUS, &oldact, NULL);
> > +        if (ret) {
> > +            perror("file_ram_alloc: failed to reinstall signal handler");
> > +            exit(1);
> > +        }
> > +
> > +        pthread_sigmask(SIG_SETMASK, &oldset, NULL);
> > +    }
> > +
> >      block->fd = fd;
> >      return area;
> >  }
> 
> 
> This is the warning shown by newer gcc versions:
> 
> qemu/exec.c:921:11: error:
>  variable ‘area’ might be clobbered by ‘longjmp’ or ‘vfork’
> [-Werror=clobbered]
> 
> I want to get support for -Wextra, and -Wclobbered is part of -Wextra.
> 
> Of course we could disable -Wclobbered and still use the other
> advantages of -Wextra,
> but this might hide real problems with clobbered variables in the future.
> 
> Declaring the local variable 'area' to be volatile also fixes the warning.
> 
> Any opinion how this problem should be solved?
> 
> Thanks,
> Stefan

The warning is spurious, obviously. Don't see a problem with 'volatile' for area pointer.