Patchwork [1/2] Split out dump-guest-memory memory mapping code

login
register
mail settings
Submitter Jens Freimann
Date April 23, 2013, 3:30 p.m.
Message ID <1366731014-48790-2-git-send-email-jfrei@linux.vnet.ibm.com>
Download mbox | patch
Permalink /patch/238943/
State New
Headers show

Comments

Jens Freimann - April 23, 2013, 3:30 p.m.
Split out dump-guest-memory memory mapping code to allow dumping without
memory mapping

The qemu dump.c code currently requires CONFIG_HAVE_CORE_DUMP as well as
CONFIG_HAVE_GET_MEMORY_MAPPING. This allows for dumping with and without paging.
Some architectures will provide only the non-paging case. This patch allows an
architecture to provide dumping even when CONFIG_HAVE_GET_MEMORY_MAPPING is not
available. To do that, we split out the common code and provide stub functions
for the non-paging case. If -p is specified on a target that doesn't support it,
we will pass an error to the calling code.

Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 Makefile.target                 |   2 +-
 dump.c                          |  11 ++++-
 include/qapi/qmp/qerror.h       |   3 ++
 include/sysemu/dump.h           |   3 ++
 include/sysemu/memory_mapping.h |  12 +++++
 memory_mapping-stub.c           |  11 +----
 memory_mapping.c                |  84 +-------------------------------
 memory_mapping_common.c         | 104 ++++++++++++++++++++++++++++++++++++++++
 8 files changed, 137 insertions(+), 93 deletions(-)
 create mode 100644 memory_mapping_common.c
Eric Blake - April 23, 2013, 3:41 p.m.
On 04/23/2013 09:30 AM, Jens Freimann wrote:
> Split out dump-guest-memory memory mapping code to allow dumping without
> memory mapping
> 
> The qemu dump.c code currently requires CONFIG_HAVE_CORE_DUMP as well as
> CONFIG_HAVE_GET_MEMORY_MAPPING. This allows for dumping with and without paging.
> Some architectures will provide only the non-paging case. This patch allows an
> architecture to provide dumping even when CONFIG_HAVE_GET_MEMORY_MAPPING is not
> available. To do that, we split out the common code and provide stub functions
> for the non-paging case. If -p is specified on a target that doesn't support it,
> we will pass an error to the calling code.
> 
> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> ---

> +++ b/include/qapi/qmp/qerror.h
> @@ -249,4 +249,7 @@ void assert_no_error(Error *err);
>  #define QERR_SOCKET_CREATE_FAILED \
>      ERROR_CLASS_GENERIC_ERROR, "Failed to create socket"
>  
> +#define QERR_UNSUPPORTED_COMMAND_OPTION \
> +    ERROR_CLASS_GENERIC_ERROR, "Option(s) %s of %s command not supported for %s"

Rather than adding a new QERR_* constant here, just use error_setg() in
qmp_dump_guest_memory() in the first place.

This raises an interesting question about introspection - how will
management apps (such as libvirt) be able to determine whether the
paging command is supported for a given architecture?  Do we need to
expand the 'MachineInfo' QMP datatype so that 'query-machines' can tell
us whether a given machine will support or reject attempts to set
'paging':true during 'dump-guest-memory'?
Jens Freimann - April 23, 2013, 3:54 p.m.
On Tue, Apr 23, 2013 at 09:41:43AM -0600, Eric Blake wrote:
> On 04/23/2013 09:30 AM, Jens Freimann wrote:
> > Split out dump-guest-memory memory mapping code to allow dumping without
> > memory mapping
> > 
> > The qemu dump.c code currently requires CONFIG_HAVE_CORE_DUMP as well as
> > CONFIG_HAVE_GET_MEMORY_MAPPING. This allows for dumping with and without paging.
> > Some architectures will provide only the non-paging case. This patch allows an
> > architecture to provide dumping even when CONFIG_HAVE_GET_MEMORY_MAPPING is not
> > available. To do that, we split out the common code and provide stub functions
> > for the non-paging case. If -p is specified on a target that doesn't support it,
> > we will pass an error to the calling code.
> > 
> > Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
> > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> > ---
> 
> > +++ b/include/qapi/qmp/qerror.h
> > @@ -249,4 +249,7 @@ void assert_no_error(Error *err);
> >  #define QERR_SOCKET_CREATE_FAILED \
> >      ERROR_CLASS_GENERIC_ERROR, "Failed to create socket"
> >  
> > +#define QERR_UNSUPPORTED_COMMAND_OPTION \
> > +    ERROR_CLASS_GENERIC_ERROR, "Option(s) %s of %s command not supported for %s"
> 
> Rather than adding a new QERR_* constant here, just use error_setg() in
> qmp_dump_guest_memory() in the first place.

ok, will fix
 
> This raises an interesting question about introspection - how will
> management apps (such as libvirt) be able to determine whether the
> paging command is supported for a given architecture?  Do we need to
> expand the 'MachineInfo' QMP datatype so that 'query-machines' can tell
> us whether a given machine will support or reject attempts to set
> 'paging':true during 'dump-guest-memory'?

sounds reasonable to me. 


regards
Jens 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
>
Luiz Capitulino - April 24, 2013, 3:50 p.m.
On Tue, 23 Apr 2013 09:41:43 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 04/23/2013 09:30 AM, Jens Freimann wrote:
> > Split out dump-guest-memory memory mapping code to allow dumping without
> > memory mapping
> > 
> > The qemu dump.c code currently requires CONFIG_HAVE_CORE_DUMP as well as
> > CONFIG_HAVE_GET_MEMORY_MAPPING. This allows for dumping with and without paging.
> > Some architectures will provide only the non-paging case. This patch allows an
> > architecture to provide dumping even when CONFIG_HAVE_GET_MEMORY_MAPPING is not
> > available. To do that, we split out the common code and provide stub functions
> > for the non-paging case. If -p is specified on a target that doesn't support it,
> > we will pass an error to the calling code.
> > 
> > Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
> > Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> > ---
> 
> > +++ b/include/qapi/qmp/qerror.h
> > @@ -249,4 +249,7 @@ void assert_no_error(Error *err);
> >  #define QERR_SOCKET_CREATE_FAILED \
> >      ERROR_CLASS_GENERIC_ERROR, "Failed to create socket"
> >  
> > +#define QERR_UNSUPPORTED_COMMAND_OPTION \
> > +    ERROR_CLASS_GENERIC_ERROR, "Option(s) %s of %s command not supported for %s"
> 
> Rather than adding a new QERR_* constant here, just use error_setg() in
> qmp_dump_guest_memory() in the first place.
> 
> This raises an interesting question about introspection - how will
> management apps (such as libvirt) be able to determine whether the
> paging command is supported for a given architecture?  Do we need to
> expand the 'MachineInfo' QMP datatype so that 'query-machines' can tell
> us whether a given machine will support or reject attempts to set
> 'paging':true during 'dump-guest-memory'?

Is libvirt going to query this for the automatic dump feature?

I'm asking this because if the fact that an arch doesn't support memory
dump is only visible to human users, then try & fail doesn't seem bad.
Eric Blake - April 24, 2013, 4:23 p.m.
On 04/24/2013 09:50 AM, Luiz Capitulino wrote:
>> This raises an interesting question about introspection - how will
>> management apps (such as libvirt) be able to determine whether the
>> paging command is supported for a given architecture?  Do we need to
>> expand the 'MachineInfo' QMP datatype so that 'query-machines' can tell
>> us whether a given machine will support or reject attempts to set
>> 'paging':true during 'dump-guest-memory'?
> 
> Is libvirt going to query this for the automatic dump feature?

Probably.  Right now, libvirt has already exposed the paging option to
users, and uses the try-and-fail approach of reporting back any error
message from QMP if the dump command fails.  But we've had error reports
in the past against libvirt that the error reported by qemu isn't always
the sanest, and that sometimes it is much nicer to have libvirt detect
in advance that a qemu command cannot succeed than it is to do a
try-and-fail approach.  There's also a matter of clean rollbacks;
libvirt has to set up some state when starting a dump command, and has
to undo that state if try-and-fail reported an error; whereas a
capability detection can avoid having to set up any state in the first
place.

> 
> I'm asking this because if the fact that an arch doesn't support memory
> dump is only visible to human users, then try & fail doesn't seem bad.

Introspection can't hurt, even if libvirt doesn't use it right away.  It
may be the sort of thing where we commit the initial capability with
try-and-fail handling, then down the road, decide whether the error
quality is good enough; it also seems like introspection is the sort of
thing that is easy to backport, even if the introspection does not go in
at the same time as the original feature, where improved error messages
becomes a quality-of-implementation value-added by distro packagers.
Ekaterina Tumanova - April 24, 2013, 5:07 p.m.
On 04/23/2013 07:41 PM, Eric Blake wrote:
> On 04/23/2013 09:30 AM, Jens Freimann wrote:
>> Split out dump-guest-memory memory mapping code to allow dumping without
>> memory mapping
>>
>> The qemu dump.c code currently requires CONFIG_HAVE_CORE_DUMP as well as
>> CONFIG_HAVE_GET_MEMORY_MAPPING. This allows for dumping with and without paging.
>> Some architectures will provide only the non-paging case. This patch allows an
>> architecture to provide dumping even when CONFIG_HAVE_GET_MEMORY_MAPPING is not
>> available. To do that, we split out the common code and provide stub functions
>> for the non-paging case. If -p is specified on a target that doesn't support it,
>> we will pass an error to the calling code.
>>
>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>> ---
>> +++ b/include/qapi/qmp/qerror.h
>> @@ -249,4 +249,7 @@ void assert_no_error(Error *err);
>>   #define QERR_SOCKET_CREATE_FAILED \
>>       ERROR_CLASS_GENERIC_ERROR, "Failed to create socket"
>>   
>> +#define QERR_UNSUPPORTED_COMMAND_OPTION \
>> +    ERROR_CLASS_GENERIC_ERROR, "Option(s) %s of %s command not supported for %s"
> Rather than adding a new QERR_* constant here, just use error_setg() in
> qmp_dump_guest_memory() in the first place.
>
> This raises an interesting question about introspection - how will
> management apps (such as libvirt) be able to determine whether the
> paging command is supported for a given architecture?  Do we need to
> expand the 'MachineInfo' QMP datatype so that 'query-machines' can tell
> us whether a given machine will support or reject attempts to set
> 'paging':true during 'dump-guest-memory'?
>
as far as I understand libvirt doesn't actually use -p dump-guest-memory 
parameter.
and virsh dump doesn't have paging param
Luiz Capitulino - April 24, 2013, 5:25 p.m.
On Wed, 24 Apr 2013 10:23:58 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 04/24/2013 09:50 AM, Luiz Capitulino wrote:
> >> This raises an interesting question about introspection - how will
> >> management apps (such as libvirt) be able to determine whether the
> >> paging command is supported for a given architecture?  Do we need to
> >> expand the 'MachineInfo' QMP datatype so that 'query-machines' can tell
> >> us whether a given machine will support or reject attempts to set
> >> 'paging':true during 'dump-guest-memory'?
> > 
> > Is libvirt going to query this for the automatic dump feature?
> 
> Probably.  Right now, libvirt has already exposed the paging option to
> users, and uses the try-and-fail approach of reporting back any error
> message from QMP if the dump command fails.  But we've had error reports
> in the past against libvirt that the error reported by qemu isn't always
> the sanest, and that sometimes it is much nicer to have libvirt detect
> in advance that a qemu command cannot succeed than it is to do a
> try-and-fail approach.  There's also a matter of clean rollbacks;
> libvirt has to set up some state when starting a dump command, and has
> to undo that state if try-and-fail reported an error; whereas a
> capability detection can avoid having to set up any state in the first
> place.

Fair enough.

So, we have to choose a good and consistent method for reporting
arch-dependent capabilities.

I like what you suggest, but there are two issues with it. First, we're
adding query-<command>-capabilities commands for some commands, so I feel
that reporting this capability through MachineInfo is inconsistent. The other
issue is that, if we do add this capability to MachineInfo, then we'll
have to add future arch-dependent capabilities to MachineInfo as well.

I'd prefer query-dump-guest-memory-capabilities myself, although I'm unsure
if the proliferation of such commands is a good thing.
Eric Blake - April 24, 2013, 7:57 p.m.
On 04/24/2013 11:07 AM, Ekaterina Tumanova wrote:
>> This raises an interesting question about introspection - how will
>> management apps (such as libvirt) be able to determine whether the
>> paging command is supported for a given architecture?  Do we need to
>> expand the 'MachineInfo' QMP datatype so that 'query-machines' can tell
>> us whether a given machine will support or reject attempts to set
>> 'paging':true during 'dump-guest-memory'?
>>
> as far as I understand libvirt doesn't actually use -p dump-guest-memory
> parameter.
> and virsh dump doesn't have paging param

Hmm, you're right.  At the public libvirt API level,
virDomainCoreDumpFlags currently exposes VIR_DUMP_MEMORY_ONLY (request
to use dump-guest-memory instead of migration to file), but does not
have a flag for exposing the paging boolean.  At the internal C level,
qemuMonitorJSONDump hardcodes 'paging' to false in current libvirt.git.
 I did a bit more digging, and found this libvirt commit:

commit d239085e956ca6ca42480e877e98a4302e91b853
Author: Eric Blake <eblake@redhat.com>
Date:   Mon Sep 17 13:05:29 2012 -0600

    qemu: drop unused arguments for dump-guest-memory

    Upstream qemu has raised a concern about whether dumping guest
    memory by reading guest paging tables is a security hole:
    https://lists.gnu.org/archive/html/qemu-devel/2012-09/msg02607.html

    While auditing libvirt to see if we would be impacted, I noticed
    that we had some dead code.  It is simpler to nuke the dead code
    and limit our monitor code to just the subset we make use of.

    * src/qemu/qemu_monitor.h (QEMU_MONITOR_DUMP): Drop poorly named
    and mostly-unused enum.
    * src/qemu/qemu_monitor.c (qemuMonitorDumpToFd): Drop arguments.
    * src/qemu/qemu_monitor_json.h (qemuMonitorJSONDump): Likewise.
    * src/qemu/qemu_monitor_json.c (qemuMonitorJSONDump): Likewise.
    * src/qemu/qemu_driver.c (qemuDumpToFd): Update caller.

[Is it a bad sign when I can remember that libvirt USED to partially
support the paging flag, but not that _I_ was the one that ripped it out
because the public API never supported it?]
Andreas Färber - May 17, 2013, 10:18 a.m.
Am 23.04.2013 17:54, schrieb Jens Freimann:
> On Tue, Apr 23, 2013 at 09:41:43AM -0600, Eric Blake wrote:
>> On 04/23/2013 09:30 AM, Jens Freimann wrote:
>>> Split out dump-guest-memory memory mapping code to allow dumping without
>>> memory mapping
>>>
>>> The qemu dump.c code currently requires CONFIG_HAVE_CORE_DUMP as well as
>>> CONFIG_HAVE_GET_MEMORY_MAPPING. This allows for dumping with and without paging.
>>> Some architectures will provide only the non-paging case. This patch allows an
>>> architecture to provide dumping even when CONFIG_HAVE_GET_MEMORY_MAPPING is not
>>> available. To do that, we split out the common code and provide stub functions
>>> for the non-paging case. If -p is specified on a target that doesn't support it,
>>> we will pass an error to the calling code.
>>>
>>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>>> ---
>>
>>> +++ b/include/qapi/qmp/qerror.h
>>> @@ -249,4 +249,7 @@ void assert_no_error(Error *err);
>>>  #define QERR_SOCKET_CREATE_FAILED \
>>>      ERROR_CLASS_GENERIC_ERROR, "Failed to create socket"
>>>  
>>> +#define QERR_UNSUPPORTED_COMMAND_OPTION \
>>> +    ERROR_CLASS_GENERIC_ERROR, "Option(s) %s of %s command not supported for %s"
>>
>> Rather than adding a new QERR_* constant here, just use error_setg() in
>> qmp_dump_guest_memory() in the first place.
> 
> ok, will fix

Any progress on this? I'd be happy to take an updated version of this
generic patch and leave the s390x part for Alex et al. to review.

If you resend, note that Git complained about a trailing white line in
the new file; otherwise I'd be fine with a diff+Sob or suggestion for
the error_setg() text. Thanks.

>> This raises an interesting question about introspection - how will
>> management apps (such as libvirt) be able to determine whether the
>> paging command is supported for a given architecture?  Do we need to
>> expand the 'MachineInfo' QMP datatype so that 'query-machines' can tell
>> us whether a given machine will support or reject attempts to set
>> 'paging':true during 'dump-guest-memory'?
> 
> sounds reasonable to me.

This discussion seems orthogonal since we are not changing behavior.
AFAICS we were only hiding the documentation of the dump-guest-memory
command but not shielding its implementation and the stub raised
QERR_UNSUPPORTED, so raising an error when the implementation doesn't
support paging doesn't seem a regression to me and can be revisited when
we actually add a second implementation, whether s390x or arm.

Regards,
Andreas

Patch

diff --git a/Makefile.target b/Makefile.target
index 2636103..ea733b1 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -112,7 +112,7 @@  obj-$(CONFIG_KVM) += kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-y += memory.o savevm.o cputlb.o
 obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
-obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o
+obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o memory_mapping_common.o
 obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
 obj-$(CONFIG_NO_CORE_DUMP) += dump-stub.o
 LIBS+=$(libs_softmmu)
diff --git a/dump.c b/dump.c
index c0d3da5..fc878bd 100644
--- a/dump.c
+++ b/dump.c
@@ -756,7 +756,10 @@  static int dump_init(DumpState *s, int fd, bool paging, bool has_filter,
     /* get memory mapping */
     memory_mapping_list_init(&s->list);
     if (paging) {
-        qemu_get_guest_memory_mapping(&s->list);
+        ret = qemu_get_guest_memory_mapping(&s->list);
+        if (ret < 0) {
+            goto cleanup;
+        }
     } else {
         qemu_get_guest_simple_memory_mapping(&s->list);
     }
@@ -826,6 +829,12 @@  void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
     DumpState *s;
     int ret;
 
+    if (paging && !memory_mapping_allowed()) {
+            error_set(errp, QERR_UNSUPPORTED_COMMAND_OPTION,
+                      "paging", "dump-guest-memory", "current architecture");
+            return;
+    }
+
     if (has_begin && !has_length) {
         error_set(errp, QERR_MISSING_PARAMETER, "length");
         return;
diff --git a/include/qapi/qmp/qerror.h b/include/qapi/qmp/qerror.h
index 6c0a18d..58b7f7a 100644
--- a/include/qapi/qmp/qerror.h
+++ b/include/qapi/qmp/qerror.h
@@ -249,4 +249,7 @@  void assert_no_error(Error *err);
 #define QERR_SOCKET_CREATE_FAILED \
     ERROR_CLASS_GENERIC_ERROR, "Failed to create socket"
 
+#define QERR_UNSUPPORTED_COMMAND_OPTION \
+    ERROR_CLASS_GENERIC_ERROR, "Option(s) %s of %s command not supported for %s"
+
 #endif /* QERROR_H */
diff --git a/include/sysemu/dump.h b/include/sysemu/dump.h
index 75823e5..557bf50 100644
--- a/include/sysemu/dump.h
+++ b/include/sysemu/dump.h
@@ -14,6 +14,9 @@ 
 #ifndef DUMP_H
 #define DUMP_H
 
+#include "qapi/error.h"
+#include "include/qapi/qmp/qerror.h"
+
 typedef struct ArchDumpInfo {
     int d_machine;  /* Architecture */
     int d_endian;   /* ELFDATA2LSB or ELFDATA2MSB */
diff --git a/include/sysemu/memory_mapping.h b/include/sysemu/memory_mapping.h
index 1256125..691b60a 100644
--- a/include/sysemu/memory_mapping.h
+++ b/include/sysemu/memory_mapping.h
@@ -15,6 +15,8 @@ 
 #define MEMORY_MAPPING_H
 
 #include "qemu/queue.h"
+#include "qapi/error.h"
+#include "include/qapi/qmp/qerror.h"
 
 /* The physical and virtual address in the memory mapping are contiguous. */
 typedef struct MemoryMapping {
@@ -38,6 +40,15 @@  bool cpu_paging_enabled(CPUArchState *env);
  * memory mapping's list. The region's virtual address starts with virt_addr,
  * and is contiguous. The list is sorted by phys_addr.
  */
+
+void memory_mapping_list_add_mapping_sorted(MemoryMappingList *list,
+                                            MemoryMapping *mapping);
+
+void create_new_memory_mapping(MemoryMappingList *list,
+                               hwaddr phys_addr,
+                               hwaddr virt_addr,
+                               ram_addr_t length);
+
 void memory_mapping_list_add_merge_sorted(MemoryMappingList *list,
                                           hwaddr phys_addr,
                                           hwaddr virt_addr,
@@ -60,5 +71,6 @@  void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list);
 
 void memory_mapping_filter(MemoryMappingList *list, int64_t begin,
                            int64_t length);
+bool memory_mapping_allowed(void);
 
 #endif
diff --git a/memory_mapping-stub.c b/memory_mapping-stub.c
index 24d5d67..c48ea44 100644
--- a/memory_mapping-stub.c
+++ b/memory_mapping-stub.c
@@ -20,14 +20,7 @@  int qemu_get_guest_memory_mapping(MemoryMappingList *list)
     return -2;
 }
 
-int cpu_get_memory_mapping(MemoryMappingList *list,
-					                                          CPUArchState *env)
+bool memory_mapping_allowed(void)
 {
-    return -1;
+    return false;
 }
-
-bool cpu_paging_enabled(CPUArchState *env)
-{
-    return true;
-}
-
diff --git a/memory_mapping.c b/memory_mapping.c
index ff45b3a..4867ae4 100644
--- a/memory_mapping.c
+++ b/memory_mapping.c
@@ -15,36 +15,6 @@ 
 #include "exec/cpu-all.h"
 #include "sysemu/memory_mapping.h"
 
-static void memory_mapping_list_add_mapping_sorted(MemoryMappingList *list,
-                                                   MemoryMapping *mapping)
-{
-    MemoryMapping *p;
-
-    QTAILQ_FOREACH(p, &list->head, next) {
-        if (p->phys_addr >= mapping->phys_addr) {
-            QTAILQ_INSERT_BEFORE(p, mapping, next);
-            return;
-        }
-    }
-    QTAILQ_INSERT_TAIL(&list->head, mapping, next);
-}
-
-static void create_new_memory_mapping(MemoryMappingList *list,
-                                      hwaddr phys_addr,
-                                      hwaddr virt_addr,
-                                      ram_addr_t length)
-{
-    MemoryMapping *memory_mapping;
-
-    memory_mapping = g_malloc(sizeof(MemoryMapping));
-    memory_mapping->phys_addr = phys_addr;
-    memory_mapping->virt_addr = virt_addr;
-    memory_mapping->length = length;
-    list->last_mapping = memory_mapping;
-    list->num++;
-    memory_mapping_list_add_mapping_sorted(list, memory_mapping);
-}
-
 static inline bool mapping_contiguous(MemoryMapping *map,
                                       hwaddr phys_addr,
                                       hwaddr virt_addr)
@@ -145,26 +115,6 @@  void memory_mapping_list_add_merge_sorted(MemoryMappingList *list,
     create_new_memory_mapping(list, phys_addr, virt_addr, length);
 }
 
-void memory_mapping_list_free(MemoryMappingList *list)
-{
-    MemoryMapping *p, *q;
-
-    QTAILQ_FOREACH_SAFE(p, &list->head, next, q) {
-        QTAILQ_REMOVE(&list->head, p, next);
-        g_free(p);
-    }
-
-    list->num = 0;
-    list->last_mapping = NULL;
-}
-
-void memory_mapping_list_init(MemoryMappingList *list)
-{
-    list->num = 0;
-    list->last_mapping = NULL;
-    QTAILQ_INIT(&list->head);
-}
-
 static CPUArchState *find_paging_enabled_cpu(CPUArchState *start_cpu)
 {
     CPUArchState *env;
@@ -209,38 +159,8 @@  int qemu_get_guest_memory_mapping(MemoryMappingList *list)
     return 0;
 }
 
-void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list)
+bool memory_mapping_allowed(void)
 {
-    RAMBlock *block;
-
-    QTAILQ_FOREACH(block, &ram_list.blocks, next) {
-        create_new_memory_mapping(list, block->offset, 0, block->length);
-    }
+    return true;
 }
 
-void memory_mapping_filter(MemoryMappingList *list, int64_t begin,
-                           int64_t length)
-{
-    MemoryMapping *cur, *next;
-
-    QTAILQ_FOREACH_SAFE(cur, &list->head, next, next) {
-        if (cur->phys_addr >= begin + length ||
-            cur->phys_addr + cur->length <= begin) {
-            QTAILQ_REMOVE(&list->head, cur, next);
-            list->num--;
-            continue;
-        }
-
-        if (cur->phys_addr < begin) {
-            cur->length -= begin - cur->phys_addr;
-            if (cur->virt_addr) {
-                cur->virt_addr += begin - cur->phys_addr;
-            }
-            cur->phys_addr = begin;
-        }
-
-        if (cur->phys_addr + cur->length > begin + length) {
-            cur->length -= cur->phys_addr + cur->length - begin - length;
-        }
-    }
-}
diff --git a/memory_mapping_common.c b/memory_mapping_common.c
new file mode 100644
index 0000000..f0f9947
--- /dev/null
+++ b/memory_mapping_common.c
@@ -0,0 +1,104 @@ 
+/*
+ * QEMU memory mapping
+ *
+ * Copyright Fujitsu, Corp. 2011, 2012
+ *
+ * Authors:
+ *     Wen Congyang <wency@cn.fujitsu.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "cpu.h"
+#include "exec/cpu-all.h"
+#include "sysemu/memory_mapping.h"
+
+void memory_mapping_list_add_mapping_sorted(MemoryMappingList *list,
+                                            MemoryMapping *mapping)
+{
+    MemoryMapping *p;
+
+    QTAILQ_FOREACH(p, &list->head, next) {
+        if (p->phys_addr >= mapping->phys_addr) {
+            QTAILQ_INSERT_BEFORE(p, mapping, next);
+            return;
+        }
+    }
+    QTAILQ_INSERT_TAIL(&list->head, mapping, next);
+}
+
+void create_new_memory_mapping(MemoryMappingList *list,
+                               hwaddr phys_addr,
+                               hwaddr virt_addr,
+                               ram_addr_t length)
+{
+    MemoryMapping *memory_mapping;
+
+    memory_mapping = g_malloc(sizeof(MemoryMapping));
+    memory_mapping->phys_addr = phys_addr;
+    memory_mapping->virt_addr = virt_addr;
+    memory_mapping->length = length;
+    list->last_mapping = memory_mapping;
+    list->num++;
+    memory_mapping_list_add_mapping_sorted(list, memory_mapping);
+}
+
+
+void memory_mapping_list_free(MemoryMappingList *list)
+{
+    MemoryMapping *p, *q;
+
+    QTAILQ_FOREACH_SAFE(p, &list->head, next, q) {
+        QTAILQ_REMOVE(&list->head, p, next);
+        g_free(p);
+    }
+
+    list->num = 0;
+    list->last_mapping = NULL;
+}
+
+void memory_mapping_list_init(MemoryMappingList *list)
+{
+    list->num = 0;
+    list->last_mapping = NULL;
+    QTAILQ_INIT(&list->head);
+}
+
+void qemu_get_guest_simple_memory_mapping(MemoryMappingList *list)
+{
+    RAMBlock *block;
+
+    QTAILQ_FOREACH(block, &ram_list.blocks, next) {
+        create_new_memory_mapping(list, block->offset, 0, block->length);
+    }
+}
+
+void memory_mapping_filter(MemoryMappingList *list, int64_t begin,
+                           int64_t length)
+{
+    MemoryMapping *cur, *next;
+
+    QTAILQ_FOREACH_SAFE(cur, &list->head, next, next) {
+        if (cur->phys_addr >= begin + length ||
+            cur->phys_addr + cur->length <= begin) {
+            QTAILQ_REMOVE(&list->head, cur, next);
+            list->num--;
+            continue;
+        }
+
+        if (cur->phys_addr < begin) {
+            cur->length -= begin - cur->phys_addr;
+            if (cur->virt_addr) {
+                cur->virt_addr += begin - cur->phys_addr;
+            }
+            cur->phys_addr = begin;
+        }
+
+        if (cur->phys_addr + cur->length > begin + length) {
+            cur->length -= cur->phys_addr + cur->length - begin - length;
+        }
+    }
+}
+