[Qemu-devel] [PATCH for-1.5 0/2] Fix hot-unplug of devices with ROM or RAM

[Qemu-devel] [PATCH for-1.5 0/2] Fix hot-unplug of devices with ROM or RAM

[Paolo Bonzini]

Reported by Amos.

Paolo Bonzini (2):
  osdep: rename qemu_vmalloc to qemu_anon_ram_alloc
  osdep: introduce qemu_anon_ram_free to free qemu_anon_ram_alloc-ed memory

 HACKING              |  7 +++----
 exec.c               | 14 +++++---------
 include/qemu/osdep.h |  3 ++-
 include/sysemu/kvm.h |  4 ++--
 kvm-all.c            |  6 +++---
 target-s390x/kvm.c   |  2 +-
 trace-events         |  3 ++-
 util/oslib-posix.c   | 12 ++++++++++--
 util/oslib-win32.c   | 12 ++++++++++--
 9 files changed, 38 insertions(+), 25 deletions(-)

-- 
1.8.2.1

[Qemu-devel] [PATCH for-1.5 1/2] osdep, kvm: rename low-level RAM allocation functions

[Paolo Bonzini]

This is preparatory to the introduction of a separate freeing API.

Reported-by: Amos Kong <akong@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 HACKING              | 7 +++----
 exec.c               | 6 +++---
 include/qemu/osdep.h | 2 +-
 include/sysemu/kvm.h | 4 ++--
 kvm-all.c            | 6 +++---
 target-s390x/kvm.c   | 2 +-
 trace-events         | 2 +-
 util/oslib-posix.c   | 4 ++--
 util/oslib-win32.c   | 4 ++--
 9 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/HACKING b/HACKING
index 6654d33..e73ac79 100644
--- a/HACKING
+++ b/HACKING
@@ -78,16 +78,15 @@ avoided.
 Use of the malloc/free/realloc/calloc/valloc/memalign/posix_memalign
 APIs is not allowed in the QEMU codebase. Instead of these routines,
 use the GLib memory allocation routines g_malloc/g_malloc0/g_new/
-g_new0/g_realloc/g_free or QEMU's qemu_vmalloc/qemu_memalign/qemu_vfree
+g_new0/g_realloc/g_free or QEMU's qemu_memalign/qemu_blockalign/qemu_vfree
 APIs.
 
 Please note that g_malloc will exit on allocation failure, so there
 is no need to test for failure (as you would have to with malloc).
 Calling g_malloc with a zero size is valid and will return NULL.
 
-Memory allocated by qemu_vmalloc or qemu_memalign must be freed with
-qemu_vfree, since breaking this will cause problems on Win32 and user
-emulators.
+Memory allocated by qemu_memalign or qemu_blockalign must be freed with
+qemu_vfree, since breaking this will cause problems on Win32.
 
 4. String manipulation
 
diff --git a/exec.c b/exec.c
index 19725db..b7e7494 100644
--- a/exec.c
+++ b/exec.c
@@ -1062,7 +1062,7 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
 #if defined (__linux__) && !defined(TARGET_S390X)
             new_block->host = file_ram_alloc(new_block, size, mem_path);
             if (!new_block->host) {
-                new_block->host = qemu_vmalloc(size);
+                new_block->host = qemu_anon_ram_alloc(size);
                 memory_try_enable_merging(new_block->host, size);
             }
 #else
@@ -1074,9 +1074,9 @@ ram_addr_t qemu_ram_alloc_from_ptr(ram_addr_t size, void *host,
                 xen_ram_alloc(new_block->offset, size, mr);
             } else if (kvm_enabled()) {
                 /* some s390/kvm configurations have special constraints */
-                new_block->host = kvm_vmalloc(size);
+                new_block->host = kvm_ram_alloc(size);
             } else {
-                new_block->host = qemu_vmalloc(size);
+                new_block->host = qemu_anon_ram_alloc(size);
             }
             memory_try_enable_merging(new_block->host, size);
         }
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index 42545bc..ae71042 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -96,7 +96,7 @@ typedef signed int              int_fast16_t;
 
 int qemu_daemon(int nochdir, int noclose);
 void *qemu_memalign(size_t alignment, size_t size);
-void *qemu_vmalloc(size_t size);
+void *qemu_anon_ram_alloc(size_t size);
 void qemu_vfree(void *ptr);
 
 #define QEMU_MADV_INVALID -1
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 9735c1d..95c0dc9 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -142,8 +142,8 @@ int kvm_init_vcpu(CPUState *cpu);
 int kvm_cpu_exec(CPUArchState *env);
 
 #if !defined(CONFIG_USER_ONLY)
-void *kvm_vmalloc(ram_addr_t size);
-void *kvm_arch_vmalloc(ram_addr_t size);
+void *kvm_ram_alloc(ram_addr_t size);
+void *kvm_arch_ram_alloc(ram_addr_t size);
 #endif
 
 void kvm_setup_guest_memory(void *start, size_t size);
diff --git a/kvm-all.c b/kvm-all.c
index 3a31602..92709a2 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1790,17 +1790,17 @@ int kvm_has_intx_set_mask(void)
     return kvm_state->intx_set_mask;
 }
 
-void *kvm_vmalloc(ram_addr_t size)
+void *kvm_ram_alloc(ram_addr_t size)
 {
 #ifdef TARGET_S390X
     void *mem;
 
-    mem = kvm_arch_vmalloc(size);
+    mem = kvm_arch_ram_alloc(size);
     if (mem) {
         return mem;
     }
 #endif
-    return qemu_vmalloc(size);
+    return qemu_anon_ram_alloc(size);
 }
 
 void kvm_setup_guest_memory(void *start, size_t size)
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index a585392..7d49d73 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -332,7 +332,7 @@ static void *legacy_s390_alloc(ram_addr_t size)
     return mem;
 }
 
-void *kvm_arch_vmalloc(ram_addr_t size)
+void *kvm_arch_ram_alloc(ram_addr_t size)
 {
     /* Can we use the standard allocation ? */
     if (kvm_check_extension(kvm_state, KVM_CAP_S390_GMAP) &&
diff --git a/trace-events b/trace-events
index 17d75ab..acf0f5c 100644
--- a/trace-events
+++ b/trace-events
@@ -32,7 +32,7 @@ g_free(void *ptr) "ptr %p"
 
 # osdep.c
 qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size %zu ptr %p"
-qemu_vmalloc(size_t size, void *ptr) "size %zu ptr %p"
+qemu_anon_ram_alloc(size_t size, void *ptr) "size %zu ptr %p"
 qemu_vfree(void *ptr) "ptr %p"
 
 # hw/virtio.c
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index 3efc763..e7e9c01 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -101,7 +101,7 @@ void *qemu_memalign(size_t alignment, size_t size)
 }
 
 /* alloc shared memory pages */
-void *qemu_vmalloc(size_t size)
+void *qemu_anon_ram_alloc(size_t size)
 {
     size_t align = QEMU_VMALLOC_ALIGN;
     size_t total = size + align - getpagesize();
@@ -125,7 +125,7 @@ void *qemu_vmalloc(size_t size)
         munmap(ptr + size, total - size);
     }
 
-    trace_qemu_vmalloc(size, ptr);
+    trace_qemu_anon_ram_alloc(size, ptr);
     return ptr;
 }
 
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index dcfa0c2..e83c22b 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -53,7 +53,7 @@ void *qemu_memalign(size_t alignment, size_t size)
     return ptr;
 }
 
-void *qemu_vmalloc(size_t size)
+void *qemu_anon_ram_alloc(size_t size)
 {
     void *ptr;
 
@@ -64,7 +64,7 @@ void *qemu_vmalloc(size_t size)
         abort();
     }
     ptr = qemu_oom_check(VirtualAlloc(NULL, size, MEM_COMMIT, PAGE_READWRITE));
-    trace_qemu_vmalloc(size, ptr);
+    trace_qemu_anon_ram_alloc(size, ptr);
     return ptr;
 }
 
-- 
1.8.2.1

[Qemu-devel] [PATCH for-1.5 2/2] osdep: introduce qemu_anon_ram_free to free qemu_anon_ram_alloc-ed memory

[Paolo Bonzini]

We switched from qemu_memalign to mmap() but then we don't modify
qemu_vfree() to do a munmap() over free().  Which we cannot do
because qemu_vfree() frees memory allocated by qemu_{mem,block}align.

Introduce a new function that does the munmap(), luckily the size is
available in the RAMBlock.

Reported-by: Amos Kong <akong@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 exec.c               | 8 ++------
 include/qemu/osdep.h | 1 +
 trace-events         | 1 +
 util/oslib-posix.c   | 8 ++++++++
 util/oslib-win32.c   | 8 ++++++++
 5 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/exec.c b/exec.c
index b7e7494..7697be7 100644
--- a/exec.c
+++ b/exec.c
@@ -1156,21 +1156,17 @@ void qemu_ram_free(ram_addr_t addr)
                     munmap(block->host, block->length);
                     close(block->fd);
                 } else {
-                    qemu_vfree(block->host);
+                    qemu_anon_ram_free(block->host, block->length);
                 }
 #else
                 abort();
 #endif
             } else {
-#if defined(TARGET_S390X) && defined(CONFIG_KVM)
-                munmap(block->host, block->length);
-#else
                 if (xen_enabled()) {
                     xen_invalidate_map_cache_entry(block->host);
                 } else {
-                    qemu_vfree(block->host);
+                    qemu_anon_ram_free(block->host, block->length);
                 }
-#endif
             }
             g_free(block);
             break;
diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h
index ae71042..d8e8500 100644
--- a/include/qemu/osdep.h
+++ b/include/qemu/osdep.h
@@ -98,6 +98,7 @@ int qemu_daemon(int nochdir, int noclose);
 void *qemu_memalign(size_t alignment, size_t size);
 void *qemu_anon_ram_alloc(size_t size);
 void qemu_vfree(void *ptr);
+void qemu_anon_ram_free(void *ptr, size_t size);
 
 #define QEMU_MADV_INVALID -1
 
diff --git a/trace-events b/trace-events
index acf0f5c..c7dbcf0 100644
--- a/trace-events
+++ b/trace-events
@@ -34,6 +34,7 @@ g_free(void *ptr) "ptr %p"
 qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size %zu ptr %p"
 qemu_anon_ram_alloc(size_t size, void *ptr) "size %zu ptr %p"
 qemu_vfree(void *ptr) "ptr %p"
+qemu_anon_ram_free(void *ptr, size_t size) "size %zu ptr %p"
 
 # hw/virtio.c
 virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int idx) "vq %p elem %p len %u idx %u"
diff --git a/util/oslib-posix.c b/util/oslib-posix.c
index e7e9c01..087edc9 100644
--- a/util/oslib-posix.c
+++ b/util/oslib-posix.c
@@ -135,6 +135,14 @@ void qemu_vfree(void *ptr)
     free(ptr);
 }
 
+void qemu_anon_ram_free(void *ptr, size_t size)
+{
+    trace_qemu_anon_ram_free(ptr, size);
+    if (ptr) {
+        munmap(ptr, size);
+    }
+}
+
 void qemu_set_block(int fd)
 {
     int f;
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index e83c22b..adcf514 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -76,6 +76,14 @@ void qemu_vfree(void *ptr)
     }
 }
 
+void qemu_anon_ram_free(void *ptr, size_t size)
+{
+    trace_qemu_anon_ram_free(ptr, size);
+    if (ptr) {
+        VirtualFree(ptr, 0, MEM_RELEASE);
+    }
+}
+
 /* FIXME: add proper locking */
 struct tm *gmtime_r(const time_t *timep, struct tm *result)
 {
-- 
1.8.2.1

Re: [Qemu-devel] [PATCH for-1.5 2/2] osdep: introduce qemu_anon_ram_free to free qemu_anon_ram_alloc-ed memory

[Peter Maydell]

On 13 May 2013 15:19, Paolo Bonzini <pbonzini@redhat.com> wrote:
> --- a/include/qemu/osdep.h
> +++ b/include/qemu/osdep.h
> @@ -98,6 +98,7 @@ int qemu_daemon(int nochdir, int noclose);
>  void *qemu_memalign(size_t alignment, size_t size);
>  void *qemu_anon_ram_alloc(size_t size);
>  void qemu_vfree(void *ptr);
> +void qemu_anon_ram_free(void *ptr, size_t size);

No documentation comment?

thanks
-- PMM

Re: [Qemu-devel] [PATCH for-1.5 2/2] osdep: introduce qemu_anon_ram_free to free qemu_anon_ram_alloc-ed memory

[Paolo Bonzini]

> On 13 May 2013 15:19, Paolo Bonzini <pbonzini@redhat.com> wrote:
> > --- a/include/qemu/osdep.h
> > +++ b/include/qemu/osdep.h
> > @@ -98,6 +98,7 @@ int qemu_daemon(int nochdir, int noclose);
> >  void *qemu_memalign(size_t alignment, size_t size);
> >  void *qemu_anon_ram_alloc(size_t size);
> >  void qemu_vfree(void *ptr);
> > +void qemu_anon_ram_free(void *ptr, size_t size);
> 
> No documentation comment?

None in the proximity, either, and it's (a) pretty obvious that it
matches qemu_anon_ram_alloc (b) not really commonly used, so it
won't help a lot of readers.

Paolo

Re: [Qemu-devel] [PATCH for-1.5 0/2] Fix hot-unplug of devices with ROM or RAM

[Amos Kong]

On Mon, May 13, 2013 at 04:19:54PM +0200, Paolo Bonzini wrote:
> Reported by Amos.
> 
> Paolo Bonzini (2):
>   osdep: rename qemu_vmalloc to qemu_anon_ram_alloc
>   osdep: introduce qemu_anon_ram_free to free qemu_anon_ram_alloc-ed memory
 
Looks good to me, hotplug works now.

Reviewed-by: Amos Kong <akong@redhat.com>

>  HACKING              |  7 +++----
>  exec.c               | 14 +++++---------
>  include/qemu/osdep.h |  3 ++-
>  include/sysemu/kvm.h |  4 ++--
>  kvm-all.c            |  6 +++---
>  target-s390x/kvm.c   |  2 +-
>  trace-events         |  3 ++-
>  util/oslib-posix.c   | 12 ++++++++++--
>  util/oslib-win32.c   | 12 ++++++++++--
>  9 files changed, 38 insertions(+), 25 deletions(-)
> 
> -- 
> 1.8.2.1
> 

-- 
			Amos.

Re: [Qemu-devel] [PATCH for-1.5 0/2] Fix hot-unplug of devices with ROM or RAM

[Anthony Liguori]

Applied.  Thanks.

Regards,

Anthony Liguori

Re: [Qemu-devel] [PATCH for-1.5 2/2] osdep: introduce qemu_anon_ram_free to free qemu_anon_ram_alloc-ed memory

[Markus Armbruster]

This patch would break the build for me, if I didn't configure
--disable-werror.  Using --enable-trace-backend=stderr in case it makes
a difference.  Troublemaker flagged inline.

Paolo Bonzini <pbonzini@redhat.com> writes:

> We switched from qemu_memalign to mmap() but then we don't modify
> qemu_vfree() to do a munmap() over free().  Which we cannot do
> because qemu_vfree() frees memory allocated by qemu_{mem,block}align.
>
> Introduce a new function that does the munmap(), luckily the size is
> available in the RAMBlock.
[...]
> diff --git a/trace-events b/trace-events
> index acf0f5c..c7dbcf0 100644
> --- a/trace-events
> +++ b/trace-events
> @@ -34,6 +34,7 @@ g_free(void *ptr) "ptr %p"
>  qemu_memalign(size_t alignment, size_t size, void *ptr) "alignment %zu size %zu ptr %p"
>  qemu_anon_ram_alloc(size_t size, void *ptr) "size %zu ptr %p"
>  qemu_vfree(void *ptr) "ptr %p"
> +qemu_anon_ram_free(void *ptr, size_t size) "size %zu ptr %p"

In file included from /work/armbru/qemu/include/trace.h:4:0,
                 from /work/armbru/qemu/util/osdep.c:49:
./trace/generated-tracers.h: In function 'trace_qemu_anon_ram_free':
./trace/generated-tracers.h:64:9: warning: format '%zu' expects argument of type 'size_t', but argument 3 has type 'void *' [-Wformat]
./trace/generated-tracers.h:64:9: warning: format '%p' expects argument of type 'void *', but argument 4 has type 'size_t' [-Wformat]


>  
>  # hw/virtio.c
>  virtqueue_fill(void *vq, const void *elem, unsigned int len, unsigned int idx) "vq %p elem %p len %u idx %u"
[...]

Re: [Qemu-devel] [PATCH for-1.5 2/2] osdep: introduce qemu_anon_ram_free to free qemu_anon_ram_alloc-ed memory

[Paolo Bonzini]

Il 15/05/2013 16:36, Markus Armbruster ha scritto:
> This patch would break the build for me, if I didn't configure
> --disable-werror.  Using --enable-trace-backend=stderr in case it makes
> a difference.  Troublemaker flagged inline.

Yes, it does---and it makes it a bit less urgent.  Will fix tomorrow.

Paolo

Re: [Qemu-devel] [PATCH for-1.5 2/2] osdep: introduce qemu_anon_ram_free to free qemu_anon_ram_alloc-ed memory

[Stefan Weil]

Am 15.05.2013 16:44, schrieb Paolo Bonzini:
> Il 15/05/2013 16:36, Markus Armbruster ha scritto:
>> This patch would break the build for me, if I didn't configure
>> --disable-werror.  Using --enable-trace-backend=stderr in case it makes
>> a difference.  Troublemaker flagged inline.
> Yes, it does---and it makes it a bit less urgent.  Will fix tomorrow.
>
> Paolo

I just got this error with the latest QEMU release candidate
and have sent a patch to qemu-devel now.

Cheers, Stefan