introducing a new, dedicated memory dump mechanism

Sync command needs these two APIs to suspend/resume monitor.

Signed-off-by Wen Congyang <[email protected]>

---
 monitor.c |   27 +++++++++++++++++++++++++++
 monitor.h |    2 ++
 2 files changed, 29 insertions(+), 0 deletions(-)

monitor.c +27 -0

index 11639b1..7e72739 100644
@@ -4442,6 +4442,26 @@ static void monitor_command_cb(Monitor *mon, const char *cmdline, void *opaque)
     monitor_resume(mon);
 }
 
+int qemu_suspend_monitor(const char *fmt, ...)
+{
+    int ret;
+
+    if (cur_mon) {
+        ret = monitor_suspend(cur_mon);
+    } else {
+        ret = -ENOTTY;
+    }
+
+    if (ret < 0 && fmt) {
+        va_list ap;
+        va_start(ap, fmt);
+        monitor_vprintf(cur_mon, fmt, ap);
+        va_end(ap);
+    }
+
+    return ret;
+}
+
 int monitor_suspend(Monitor *mon)
 {
     if (!mon->rs)

@@ -4450,6 +4470,13 @@ int monitor_suspend(Monitor *mon)
     return 0;
 }
 
+void qemu_resume_monitor(void)
+{
+    if (cur_mon) {
+        monitor_resume(cur_mon);
+    }
+}
+
 void monitor_resume(Monitor *mon)
 {
     if (!mon->rs)

monitor.h +0 -0

index 58109af..60a1e17 100644

Quoted 66 lines

Sync command needs these two APIs to suspend/resume monitor.

Signed-off-by Wen Congyang <[email protected]>

---
 monitor.c |   27 +++++++++++++++++++++++++++
 monitor.h |    2 ++
 2 files changed, 29 insertions(+), 0 deletions(-)

monitor.c +27 -0

index 11639b1..7e72739 100644
@@ -4442,6 +4442,26 @@ static void monitor_command_cb(Monitor *mon, const char *cmdline, void *opaque)
     monitor_resume(mon);
 }
 
+int qemu_suspend_monitor(const char *fmt, ...)
+{
+    int ret;
+
+    if (cur_mon) {
+        ret = monitor_suspend(cur_mon);
+    } else {
+        ret = -ENOTTY;
+    }
+
+    if (ret < 0 && fmt) {
+        va_list ap;
+        va_start(ap, fmt);
+        monitor_vprintf(cur_mon, fmt, ap);
+        va_end(ap);
+    }
+
+    return ret;
+}
+
 int monitor_suspend(Monitor *mon)
 {
     if (!mon->rs)

@@ -4450,6 +4470,13 @@ int monitor_suspend(Monitor *mon)
     return 0;
 }
 
+void qemu_resume_monitor(void)
+{
+    if (cur_mon) {
+        monitor_resume(cur_mon);
+    }
+}
+
 void monitor_resume(Monitor *mon)
 {
     if (!mon->rs)

monitor.h +2 -0

index 58109af..60a1e17 100644
@@ -46,7 +46,9 @@ int monitor_cur_is_qmp(void);
 void monitor_protocol_event(MonitorEvent event, QObject *data);
 void monitor_init(CharDriverState *chr, int flags);
 
+int qemu_suspend_monitor(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 int monitor_suspend(Monitor *mon);
+void qemu_resume_monitor(void);
 void monitor_resume(Monitor *mon);
 

monitor.h int monitor_cur_is_qmp(void); · line 54

Quoted 1 line

monitor.h +0 -0

 int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,

I don't see any added value in this API, specifically as it is built on top of cur_mon. Just use the existing services like the migration code does. If you properly pass down the monitor reference from the command to the suspend and store what monitor you suspended, all should be fine.

Jan

At 02/15/2012 12:19 AM, Jan Kiszka Wrote:

Quoted 67 lines · Wen Congyang

Quoted 66 lines

Sync command needs these two APIs to suspend/resume monitor.

Signed-off-by Wen Congyang <[email protected]>

---
 monitor.c |   27 +++++++++++++++++++++++++++
 monitor.h |    2 ++
 2 files changed, 29 insertions(+), 0 deletions(-)

monitor.c +27 -0

index 11639b1..7e72739 100644
@@ -4442,6 +4442,26 @@ static void monitor_command_cb(Monitor *mon, const char *cmdline, void *opaque)
     monitor_resume(mon);
 }
 
+int qemu_suspend_monitor(const char *fmt, ...)
+{
+    int ret;
+
+    if (cur_mon) {
+        ret = monitor_suspend(cur_mon);
+    } else {
+        ret = -ENOTTY;
+    }
+
+    if (ret < 0 && fmt) {
+        va_list ap;
+        va_start(ap, fmt);
+        monitor_vprintf(cur_mon, fmt, ap);
+        va_end(ap);
+    }
+
+    return ret;
+}
+
 int monitor_suspend(Monitor *mon)
 {
     if (!mon->rs)

@@ -4450,6 +4470,13 @@ int monitor_suspend(Monitor *mon)
     return 0;
 }
 
+void qemu_resume_monitor(void)
+{
+    if (cur_mon) {
+        monitor_resume(cur_mon);
+    }
+}
+
 void monitor_resume(Monitor *mon)
 {
     if (!mon->rs)

monitor.h +2 -0

index 58109af..60a1e17 100644
@@ -46,7 +46,9 @@ int monitor_cur_is_qmp(void);
 void monitor_protocol_event(MonitorEvent event, QObject *data);
 void monitor_init(CharDriverState *chr, int flags);
 
+int qemu_suspend_monitor(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 int monitor_suspend(Monitor *mon);
+void qemu_resume_monitor(void);
 void monitor_resume(Monitor *mon);
 

Quoted 6 lines

monitor.h int monitor_cur_is_qmp(void); · line 54

Quoted 1 line

monitor.h +0 -0

 int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,

I don't see any added value in this API, specifically as it is built on top of cur_mon. Just use the existing services like the migration code does. If you properly pass down the monitor reference from the command to the suspend and store what monitor you suspended, all should be fine.

This API is like qemu_get_fd() which is not merged into upstream qemu. I need this API because I cannot use monitor in qapi command.

Quoted 75 lines · Wen Congyang

At 02/15/2012 12:19 AM, Jan Kiszka Wrote:

Quoted 73 lines · Wen Congyang

Quoted 58 lines

Sync command needs these two APIs to suspend/resume monitor.

Signed-off-by Wen Congyang <[email protected]>

---
 monitor.c |   27 +++++++++++++++++++++++++++
 monitor.h |    2 ++
 2 files changed, 29 insertions(+), 0 deletions(-)

monitor.c +27 -0

index 11639b1..7e72739 100644
@@ -4442,6 +4442,26 @@ static void monitor_command_cb(Monitor *mon, const char *cmdline, void *opaque)
     monitor_resume(mon);
 }
 
+int qemu_suspend_monitor(const char *fmt, ...)
+{
+    int ret;
+
+    if (cur_mon) {
+        ret = monitor_suspend(cur_mon);
+    } else {
+        ret = -ENOTTY;
+    }
+
+    if (ret < 0 && fmt) {
+        va_list ap;
+        va_start(ap, fmt);
+        monitor_vprintf(cur_mon, fmt, ap);
+        va_end(ap);
+    }
+
+    return ret;
+}
+
 int monitor_suspend(Monitor *mon)
 {
     if (!mon->rs)

@@ -4450,6 +4470,13 @@ int monitor_suspend(Monitor *mon)
     return 0;
 }
 
+void qemu_resume_monitor(void)
+{
+    if (cur_mon) {
+        monitor_resume(cur_mon);
+    }
+}
+
 void monitor_resume(Monitor *mon)
 {
     if (!mon->rs)

monitor.h +0 -0

index 58109af..60a1e17 100644

monitor.h int monitor_cur_is_qmp(void); · line 54

Quoted 9 lines

monitor.h +2 -0

@@ -46,7 +46,9 @@ int monitor_cur_is_qmp(void);
 void monitor_protocol_event(MonitorEvent event, QObject *data);
 void monitor_init(CharDriverState *chr, int flags);
 
+int qemu_suspend_monitor(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 int monitor_suspend(Monitor *mon);
+void qemu_resume_monitor(void);
 void monitor_resume(Monitor *mon);
 
 int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,

I don't see any added value in this API, specifically as it is built on top of cur_mon. Just use the existing services like the migration code does. If you properly pass down the monitor reference from the command to the suspend and store what monitor you suspended, all should be fine.

Quoted 2 lines

This API is like qemu_get_fd() which is not merged into upstream qemu. I need this API because I cannot use monitor in qapi command.

OK, then I need to comment on that approach. QMP looks flawed here. Either you have a need for a Monitor object (or a generic HMP/QMP context), then you also have a handle. Or your don't, then you do not need monitor suspend/resume or get_fd as well.

Quoted 79 lines · On Wed, 15 Feb 2012 09:51:04 +0100 Jan Kiszka <[email protected]> wrote:

Quoted 77 lines · Wen Congyang

At 02/15/2012 12:19 AM, Jan Kiszka Wrote:

Quoted 67 lines · Wen Congyang

Quoted 66 lines

Sync command needs these two APIs to suspend/resume monitor.

Signed-off-by Wen Congyang <[email protected]>

---
 monitor.c |   27 +++++++++++++++++++++++++++
 monitor.h |    2 ++
 2 files changed, 29 insertions(+), 0 deletions(-)

monitor.c +27 -0

index 11639b1..7e72739 100644
@@ -4442,6 +4442,26 @@ static void monitor_command_cb(Monitor *mon, const char *cmdline, void *opaque)
     monitor_resume(mon);
 }
 
+int qemu_suspend_monitor(const char *fmt, ...)
+{
+    int ret;
+
+    if (cur_mon) {
+        ret = monitor_suspend(cur_mon);
+    } else {
+        ret = -ENOTTY;
+    }
+
+    if (ret < 0 && fmt) {
+        va_list ap;
+        va_start(ap, fmt);
+        monitor_vprintf(cur_mon, fmt, ap);
+        va_end(ap);
+    }
+
+    return ret;
+}
+
 int monitor_suspend(Monitor *mon)
 {
     if (!mon->rs)

@@ -4450,6 +4470,13 @@ int monitor_suspend(Monitor *mon)
     return 0;
 }
 
+void qemu_resume_monitor(void)
+{
+    if (cur_mon) {
+        monitor_resume(cur_mon);
+    }
+}
+
 void monitor_resume(Monitor *mon)
 {
     if (!mon->rs)

monitor.h +2 -0

index 58109af..60a1e17 100644
@@ -46,7 +46,9 @@ int monitor_cur_is_qmp(void);
 void monitor_protocol_event(MonitorEvent event, QObject *data);
 void monitor_init(CharDriverState *chr, int flags);
 
+int qemu_suspend_monitor(const char *fmt, ...) GCC_FMT_ATTR(1, 2);
 int monitor_suspend(Monitor *mon);
+void qemu_resume_monitor(void);
 void monitor_resume(Monitor *mon);
 

Quoted 6 lines

monitor.h int monitor_cur_is_qmp(void); · line 54

Quoted 1 line

monitor.h +0 -0

 int monitor_read_bdrv_key_start(Monitor *mon, BlockDriverState *bs,

I don't see any added value in this API, specifically as it is built on top of cur_mon. Just use the existing services like the migration code does. If you properly pass down the monitor reference from the command to the suspend and store what monitor you suspended, all should be fine.

This API is like qemu_get_fd() which is not merged into upstream qemu. I need this API because I cannot use monitor in qapi command.

Quoted 4 lines

OK, then I need to comment on that approach. QMP looks flawed here. Either you have a need for a Monitor object (or a generic HMP/QMP context), then you also have a handle. Or your don't, then you do not need monitor suspend/resume or get_fd as well.

The getfd one is explained in the other thread, but suspend/resume should be done from HMP only.

PS: Haven't reviewed this series yet.

The memory mapping list stores virtual address and physical address mapping. The folloing patch will use this information to create PT_LOAD in the vmcore.

Signed-off-by Wen Congyang <[email protected]>

---
 Makefile.target  |    1 +
 memory_mapping.c |  130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory_mapping.h |   38 ++++++++++++++++
 3 files changed, 169 insertions(+), 0 deletions(-)
 create mode 100644 memory_mapping.c
 create mode 100644 memory_mapping.h

Makefile.target +1 -0

index 68481a3..e35e464 100644
@@ -200,6 +200,7 @@ obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-$(CONFIG_VGA) += vga.o
 obj-y += memory.o savevm.o
+obj-y += memory_mapping.o
 LIBS+=-lz
 
 obj-i386-$(CONFIG_KVM) += hyperv.o

memory_mapping.c new file +130 -0

new file mode 100644
index 0000000..d83b7d7
@@ -0,0 +1,130 @@
+/*
+ * QEMU memory mapping
+ *
+ * Copyright Fujitsu, Corp. 2011
+ *
+ * Authors:
+ *     Wen Congyang <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "cpu.h"
+#include "cpu-all.h"
+#include "memory_mapping.h"
+
+static MemoryMapping *last_mapping;
+
+static void create_new_memory_mapping(MemoryMappingList *list,
+                                      target_phys_addr_t phys_addr,
+                                      target_phys_addr_t virt_addr,
+                                      ram_addr_t length)
+{
+    MemoryMapping *memory_mapping, *p;
+
+    memory_mapping = g_malloc(sizeof(MemoryMapping));
+    memory_mapping->phys_addr = phys_addr;
+    memory_mapping->virt_addr = virt_addr;
+    memory_mapping->length = length;
+    last_mapping = memory_mapping;
+    list->num++;
+    QTAILQ_FOREACH(p, &list->head, next) {
+        if (p->phys_addr >= memory_mapping->phys_addr) {
+            QTAILQ_INSERT_BEFORE(p, memory_mapping, next);
+            return;
+        }
+    }
+    QTAILQ_INSERT_TAIL(&list->head, memory_mapping, next);
+    return;
+}
+
+void create_new_memory_mapping_head(MemoryMappingList *list,
+                                    target_phys_addr_t phys_addr,
+                                    target_phys_addr_t 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;
+    last_mapping = memory_mapping;
+    list->num++;
+    QTAILQ_INSERT_HEAD(&list->head, memory_mapping, next);
+    return;
+}
+
+void add_to_memory_mapping(MemoryMappingList *list,
+                           target_phys_addr_t phys_addr,
+                           target_phys_addr_t virt_addr,
+                           ram_addr_t length)
+{
+    MemoryMapping *memory_mapping;
+
+    if (QTAILQ_EMPTY(&list->head)) {
+        create_new_memory_mapping(list, phys_addr, virt_addr, length);
+        return;
+    }
+
+    if (last_mapping) {
+        if ((phys_addr == (last_mapping->phys_addr + last_mapping->length)) &&
+            (virt_addr == (last_mapping->virt_addr + last_mapping->length))) {
+            last_mapping->length += length;
+            return;
+        }
+    }
+
+    QTAILQ_FOREACH(memory_mapping, &list->head, next) {
+        last_mapping = memory_mapping;
+        if ((phys_addr == (last_mapping->phys_addr + last_mapping->length)) &&
+            (virt_addr == (last_mapping->virt_addr + last_mapping->length))) {
+            last_mapping->length += length;
+            return;
+        }
+
+        if (!(phys_addr >= (last_mapping->phys_addr)) ||
+            !(phys_addr < (last_mapping->phys_addr + last_mapping->length))) {
+            /* last_mapping does not contain this region */
+            continue;
+        }
+        if (!(virt_addr >= (last_mapping->virt_addr)) ||
+            !(virt_addr < (last_mapping->virt_addr + last_mapping->length))) {
+            /* last_mapping does not contain this region */
+            continue;
+        }
+        if ((virt_addr - last_mapping->virt_addr) !=
+            (phys_addr - last_mapping->phys_addr)) {
+            /*
+             * last_mapping contains this region, but we should create another
+             * mapping region.
+             */
+            break;
+        }
+
+        /* merge this region into last_mapping */
+        if ((virt_addr + length) >
+            (last_mapping->virt_addr + last_mapping->length)) {
+            last_mapping->length = virt_addr + length - last_mapping->virt_addr;
+        }
+        return;
+    }
+
+    /* this region can not be merged into any existed memory mapping. */
+    create_new_memory_mapping(list, phys_addr, virt_addr, length);
+    return;
+}
+
+void free_memory_mapping_list(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;
+}

memory_mapping.h new file +4 -0

new file mode 100644
index 0000000..a4b1532
@@ -0,0 +1,38 @@
+#ifndef MEMORY_MAPPING_H
+#define MEMORY_MAPPING_H
+
+#include "qemu-queue.h"

memory_mapping.h new file +14 -0

+    QTAILQ_HEAD(, MemoryMapping) head;
+} MemoryMappingList;
+
+/*
+ * crash needs some memory mapping should be at the head of the list. It will
+ * cause the list is not sorted. So the caller must add the special memory
+ * mapping after adding all the normal memory mapping into list.
+ */
+void create_new_memory_mapping_head(MemoryMappingList *list,
+                                    target_phys_addr_t phys_addr,
+                                    target_phys_addr_t virt_addr,
+                                    ram_addr_t length);
+/*
+ * add or merge the memory region into the memory mapping's list. The list is

Quoted 175 lines

The memory mapping list stores virtual address and physical address mapping. The folloing patch will use this information to create PT_LOAD in the vmcore.

Signed-off-by Wen Congyang <[email protected]>

---
 Makefile.target  |    1 +
 memory_mapping.c |  130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory_mapping.h |   38 ++++++++++++++++
 3 files changed, 169 insertions(+), 0 deletions(-)
 create mode 100644 memory_mapping.c
 create mode 100644 memory_mapping.h

Makefile.target +1 -0

index 68481a3..e35e464 100644
@@ -200,6 +200,7 @@ obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 obj-$(CONFIG_VGA) += vga.o
 obj-y += memory.o savevm.o
+obj-y += memory_mapping.o
 LIBS+=-lz
 
 obj-i386-$(CONFIG_KVM) += hyperv.o

memory_mapping.c new file +130 -0

new file mode 100644
index 0000000..d83b7d7
@@ -0,0 +1,130 @@
+/*
+ * QEMU memory mapping
+ *
+ * Copyright Fujitsu, Corp. 2011
+ *
+ * Authors:
+ *     Wen Congyang <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "cpu.h"
+#include "cpu-all.h"
+#include "memory_mapping.h"
+
+static MemoryMapping *last_mapping;
+
+static void create_new_memory_mapping(MemoryMappingList *list,
+                                      target_phys_addr_t phys_addr,
+                                      target_phys_addr_t virt_addr,
+                                      ram_addr_t length)
+{
+    MemoryMapping *memory_mapping, *p;
+
+    memory_mapping = g_malloc(sizeof(MemoryMapping));
+    memory_mapping->phys_addr = phys_addr;
+    memory_mapping->virt_addr = virt_addr;
+    memory_mapping->length = length;
+    last_mapping = memory_mapping;
+    list->num++;
+    QTAILQ_FOREACH(p, &list->head, next) {
+        if (p->phys_addr >= memory_mapping->phys_addr) {
+            QTAILQ_INSERT_BEFORE(p, memory_mapping, next);
+            return;
+        }
+    }
+    QTAILQ_INSERT_TAIL(&list->head, memory_mapping, next);
+    return;
+}
+
+void create_new_memory_mapping_head(MemoryMappingList *list,
+                                    target_phys_addr_t phys_addr,
+                                    target_phys_addr_t 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;
+    last_mapping = memory_mapping;
+    list->num++;
+    QTAILQ_INSERT_HEAD(&list->head, memory_mapping, next);
+    return;
+}
+
+void add_to_memory_mapping(MemoryMappingList *list,
+                           target_phys_addr_t phys_addr,
+                           target_phys_addr_t virt_addr,
+                           ram_addr_t length)
+{
+    MemoryMapping *memory_mapping;
+
+    if (QTAILQ_EMPTY(&list->head)) {
+        create_new_memory_mapping(list, phys_addr, virt_addr, length);
+        return;
+    }
+
+    if (last_mapping) {
+        if ((phys_addr == (last_mapping->phys_addr + last_mapping->length)) &&
+            (virt_addr == (last_mapping->virt_addr + last_mapping->length))) {
+            last_mapping->length += length;
+            return;
+        }
+    }
+
+    QTAILQ_FOREACH(memory_mapping, &list->head, next) {
+        last_mapping = memory_mapping;
+        if ((phys_addr == (last_mapping->phys_addr + last_mapping->length)) &&
+            (virt_addr == (last_mapping->virt_addr + last_mapping->length))) {
+            last_mapping->length += length;
+            return;
+        }
+
+        if (!(phys_addr >= (last_mapping->phys_addr)) ||
+            !(phys_addr < (last_mapping->phys_addr + last_mapping->length))) {
+            /* last_mapping does not contain this region */
+            continue;
+        }
+        if (!(virt_addr >= (last_mapping->virt_addr)) ||
+            !(virt_addr < (last_mapping->virt_addr + last_mapping->length))) {
+            /* last_mapping does not contain this region */
+            continue;
+        }
+        if ((virt_addr - last_mapping->virt_addr) !=
+            (phys_addr - last_mapping->phys_addr)) {
+            /*
+             * last_mapping contains this region, but we should create another
+             * mapping region.
+             */
+            break;
+        }
+
+        /* merge this region into last_mapping */
+        if ((virt_addr + length) >
+            (last_mapping->virt_addr + last_mapping->length)) {
+            last_mapping->length = virt_addr + length - last_mapping->virt_addr;
+        }
+        return;
+    }
+
+    /* this region can not be merged into any existed memory mapping. */
+    create_new_memory_mapping(list, phys_addr, virt_addr, length);
+    return;
+}
+
+void free_memory_mapping_list(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;
+}

memory_mapping.h new file +9 -0

new file mode 100644
index 0000000..a4b1532
@@ -0,0 +1,38 @@
+#ifndef MEMORY_MAPPING_H
+#define MEMORY_MAPPING_H
+
+#include "qemu-queue.h"
+
+typedef struct MemoryMapping {
+    target_phys_addr_t phys_addr;
+    target_ulong virt_addr;
+    ram_addr_t length;

memory_mapping.h line 14

Quoted 5 lines

memory_mapping.h new file +5 -0

+    QTAILQ_ENTRY(MemoryMapping) next;
+} MemoryMapping;
+
+typedef struct MemoryMappingList {
+    unsigned int num;

This field looks unused by this series. Unless I miss something, you probably want to drop it.

Quoted 19 lines

memory_mapping.h new file +18 -0

+    QTAILQ_HEAD(, MemoryMapping) head;
+} MemoryMappingList;
+
+/*
+ * crash needs some memory mapping should be at the head of the list. It will
+ * cause the list is not sorted. So the caller must add the special memory
+ * mapping after adding all the normal memory mapping into list.
+ */
+void create_new_memory_mapping_head(MemoryMappingList *list,
+                                    target_phys_addr_t phys_addr,
+                                    target_phys_addr_t virt_addr,
+                                    ram_addr_t length);
+/*
+ * add or merge the memory region into the memory mapping's list. The list is
+ * sorted by phys_addr.
+ */
+void add_to_memory_mapping(MemoryMappingList *list,
+                           target_phys_addr_t phys_addr,

memory_mapping.h line 38

Quoted 6 lines

memory_mapping.h new file +6 -0

+                           target_phys_addr_t virt_addr,
+                           ram_addr_t length);
+
+void free_memory_mapping_list(MemoryMappingList *list);
+
+#endif

A bit hard to understand and use the API. I would suggest:

memory_mapping_list_add_sorted(MemoryMappingList *list, ...); memory_mapping_list_add_head(MemoryMappingList *list, ...); memory_mapping_list_free(MemoryMappingList *list);

memory_mapping_list_add_head should set a flag in the MemoryMapping appended to the list or let the MemoryMappingList point to the firs sorted entry. That way, the adding order becomes irrelevant.

Moreover, you are lacking some memory_mapping_list_init(MemoryMappingList *list). Cleaner than open-coding this.

Jan

This API will be used in the following patch.

Signed-off-by Wen Congyang <[email protected]>

---
 cpu-common.h |    2 ++
 exec.c       |   16 ++++++++++++++++
 2 files changed, 18 insertions(+), 0 deletions(-)

cpu-common.h +0 -0

index a40c57d..d047137 100644

cpu-common.h +1 -0

+
 /* Coalesced MMIO regions are areas where write operations can be reordered.
  * This usually implies that write operations are side-effect free.  This allows
  * batching which can make a major impact on performance when using

exec.c +0 -0

index b81677a..edc5684 100644
@@ -4435,3 +4435,19 @@ bool virtio_is_big_endian(void)
 #undef env
 
 #endif

Quoted 16 lines

This API will be used in the following patch.

Signed-off-by Wen Congyang <[email protected]>

---
 cpu-common.h |    2 ++
 exec.c       |   16 ++++++++++++++++
 2 files changed, 18 insertions(+), 0 deletions(-)

cpu-common.h +0 -0

index a40c57d..d047137 100644
@@ -71,6 +71,8 @@ void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len,
 void *cpu_register_map_client(void *opaque, void (*callback)(void *opaque));
 void cpu_unregister_map_client(void *cookie);
 

cpu-common.h void cpu_physical_memory_unmap(void *buffer, target_phys_addr_t len, · line 74

Quoted 1 line

cpu-common.h +1 -0

+bool is_io_addr(target_phys_addr_t phys_addr);

Something like cpu_physical_memory_is_io would be more consistent with other APIs around.

Quoted 12 lines

cpu-common.h +1 -0

+
 /* Coalesced MMIO regions are areas where write operations can be reordered.
  * This usually implies that write operations are side-effect free.  This allows
  * batching which can make a major impact on performance when using

exec.c +0 -0

index b81677a..edc5684 100644
@@ -4435,3 +4435,19 @@ bool virtio_is_big_endian(void)
 #undef env
 

exec.c bool virtio_is_big_endian(void) · line 4447

Quoted 11 lines

exec.c +10 -0

 #endif
+
+bool is_io_addr(target_phys_addr_t phys_addr)
+{
+    ram_addr_t pd;
+    PhysPageDesc p;
+
+    p = phys_page_find(phys_addr >> TARGET_PAGE_BITS);
+    pd = p.phys_offset;
+
+    if (!is_ram_rom_romd(pd)) {

return !is_ram_rom_romd(pd); ?

exec.c bool virtio_is_big_endian(void) · line 4453

Quoted 7 lines

exec.c +6 -0

+        /* I/O region */
+        return true;
+    }
+
+    return false;
+}

Jan

Walk cpu's page table and collect all virtual address and physical address mapping. Then, add these mapping into memory mapping list.

Signed-off-by Wen Congyang <[email protected]>

---
 Makefile.target         |    2 +-
 cpu-all.h               |    7 ++
 target-i386/arch-dump.c |  254 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 262 insertions(+), 1 deletions(-)
 create mode 100644 target-i386/arch-dump.c

Makefile.target +1 -1

index e35e464..d6e5684 100644
@@ -75,7 +75,7 @@ libobj-$(CONFIG_TCG_INTERPRETER) += tci.o
 libobj-y += fpu/softfloat.o
 libobj-y += op_helper.o helper.o
 ifeq ($(TARGET_BASE_ARCH), i386)
-libobj-y += cpuid.o
+libobj-y += cpuid.o arch-dump.o
 endif
 libobj-$(TARGET_SPARC64) += vis_helper.o
 libobj-$(CONFIG_NEED_MMU) += mmu.o

cpu-all.h +1 -0

index e2c3c49..4cd7fbb 100644
@@ -22,6 +22,7 @@
 #include "qemu-common.h"
 #include "qemu-tls.h"
 #include "cpu-common.h"
+#include "memory_mapping.h"
 
 /* some important defines:
  *

cpu-all.h +2 -0

+#endif
+
 #endif /* CPU_ALL_H */

target-i386/arch-dump.c new file +244 -0

new file mode 100644
index 0000000..2e921c7
@@ -0,0 +1,254 @@
+/*
+ * i386 dump
+ *
+ * Copyright Fujitsu, Corp. 2011
+ *
+ * Authors:
+ *     Wen Congyang <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "cpu.h"
+#include "cpu-all.h"
+
+/* PAE Paging or IA-32e Paging */
+static void walk_pte(MemoryMappingList *list, target_phys_addr_t pte_start_addr,
+                     int32_t a20_mask, target_ulong start_line_addr)
+{
+    target_phys_addr_t pte_addr, start_paddr;
+    uint64_t pte;
+    target_ulong start_vaddr;
+    int i;
+
+    for (i = 0; i < 512; i++) {
+        pte_addr = (pte_start_addr + i * 8) & a20_mask;
+        pte = ldq_phys(pte_addr);
+        if (!(pte & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        start_paddr = (pte & ~0xfff) & ~(0x1ULL << 63);
+        if (is_io_addr(start_paddr)) {
+            /* I/O region */
+            continue;
+        }
+
+        start_vaddr = start_line_addr | ((i & 0x1fff) << 12);
+        add_to_memory_mapping(list, start_paddr, start_vaddr, 1 << 12);
+    }
+}
+
+/* 32-bit Paging */
+static void walk_pte2(MemoryMappingList *list,
+                      target_phys_addr_t pte_start_addr, int32_t a20_mask,
+                      target_ulong start_line_addr)
+{
+    target_phys_addr_t pte_addr, start_paddr;
+    uint32_t pte;
+    target_ulong start_vaddr;
+    int i;
+
+    for (i = 0; i < 1024; i++) {
+        pte_addr = (pte_start_addr + i * 4) & a20_mask;
+        pte = ldl_phys(pte_addr);
+        if (!(pte & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        start_paddr = pte & ~0xfff;
+        if (is_io_addr(start_paddr)) {
+            /* I/O region */
+            continue;
+        }
+
+        start_vaddr = start_line_addr | ((i & 0x3ff) << 12);
+        add_to_memory_mapping(list, start_paddr, start_vaddr, 1 << 12);
+    }
+}
+
+/* PAE Paging or IA-32e Paging */
+static void walk_pde(MemoryMappingList *list, target_phys_addr_t pde_start_addr,
+                     int32_t a20_mask, target_ulong start_line_addr)
+{
+    target_phys_addr_t pde_addr, pte_start_addr, start_paddr;
+    uint64_t pde;
+    target_ulong line_addr, start_vaddr;
+    int i;
+
+    for (i = 0; i < 512; i++) {
+        pde_addr = (pde_start_addr + i * 8) & a20_mask;
+        pde = ldq_phys(pde_addr);
+        if (!(pde & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        line_addr = start_line_addr | ((i & 0x1ff) << 21);
+        if (pde & PG_PSE_MASK) {
+            /* 2 MB page */
+            start_paddr = (pde & ~0x1fffff) & ~(0x1ULL << 63);
+            if (is_io_addr(start_paddr)) {
+                /* I/O region */
+                continue;
+            }
+            start_vaddr = line_addr;
+            add_to_memory_mapping(list, start_paddr, start_vaddr, 1 << 21);
+            continue;
+        }
+
+        pte_start_addr = (pde & ~0xfff) & a20_mask;
+        walk_pte(list, pte_start_addr, a20_mask, line_addr);
+    }
+}
+
+/* 32-bit Paging */
+static void walk_pde2(MemoryMappingList *list,
+                      target_phys_addr_t pde_start_addr, int32_t a20_mask,
+                      bool pse)
+{
+    target_phys_addr_t pde_addr, pte_start_addr, start_paddr;
+    uint32_t pde;
+    target_ulong line_addr, start_vaddr;
+    int i;
+
+    for (i = 0; i < 1024; i++) {
+        pde_addr = (pde_start_addr + i * 4) & a20_mask;
+        pde = ldl_phys(pde_addr);
+        if (!(pde & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        line_addr = (((unsigned int)i & 0x3ff) << 22);
+        if ((pde & PG_PSE_MASK) && pse) {
+            /* 4 MB page */
+            start_paddr = (pde & ~0x3fffff) | ((pde & 0x1fe000) << 19);
+            if (is_io_addr(start_paddr)) {
+                /* I/O region */
+                continue;
+            }
+            start_vaddr = line_addr;
+            add_to_memory_mapping(list, start_paddr, start_vaddr, 1 << 22);
+            continue;
+        }
+
+        pte_start_addr = (pde & ~0xfff) & a20_mask;
+        walk_pte2(list, pte_start_addr, a20_mask, line_addr);
+    }
+}
+
+/* PAE Paging */
+static void walk_pdpe2(MemoryMappingList *list,
+                       target_phys_addr_t pdpe_start_addr, int32_t a20_mask)
+{
+    target_phys_addr_t pdpe_addr, pde_start_addr;
+    uint64_t pdpe;
+    target_ulong line_addr;
+    int i;
+
+    for (i = 0; i < 4; i++) {
+        pdpe_addr = (pdpe_start_addr + i * 8) & a20_mask;
+        pdpe = ldq_phys(pdpe_addr);
+        if (!(pdpe & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        line_addr = (((unsigned int)i & 0x3) << 30);
+        pde_start_addr = (pdpe & ~0xfff) & a20_mask;
+        walk_pde(list, pde_start_addr, a20_mask, line_addr);
+    }
+}
+
+#ifdef TARGET_X86_64
+/* IA-32e Paging */
+static void walk_pdpe(MemoryMappingList *list,
+                      target_phys_addr_t pdpe_start_addr, int32_t a20_mask,
+                      target_ulong start_line_addr)
+{
+    target_phys_addr_t pdpe_addr, pde_start_addr, start_paddr;
+    uint64_t pdpe;
+    target_ulong line_addr, start_vaddr;
+    int i;
+
+    for (i = 0; i < 512; i++) {
+        pdpe_addr = (pdpe_start_addr + i * 8) & a20_mask;
+        pdpe = ldq_phys(pdpe_addr);
+        if (!(pdpe & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        line_addr = start_line_addr | ((i & 0x1ffULL) << 30);
+        if (pdpe & PG_PSE_MASK) {
+            /* 1 GB page */
+            start_paddr = (pdpe & ~0x3fffffff) & ~(0x1ULL << 63);
+            if (is_io_addr(start_paddr)) {
+                /* I/O region */
+                continue;
+            }
+            start_vaddr = line_addr;
+            add_to_memory_mapping(list, start_paddr, start_vaddr, 1 << 30);
+            continue;
+        }
+
+        pde_start_addr = (pdpe & ~0xfff) & a20_mask;
+        walk_pde(list, pde_start_addr, a20_mask, line_addr);
+    }
+}
+
+/* IA-32e Paging */
+static void walk_pml4e(MemoryMappingList *list,
+                       target_phys_addr_t pml4e_start_addr, int32_t a20_mask)
+{
+    target_phys_addr_t pml4e_addr, pdpe_start_addr;
+    uint64_t pml4e;
+    target_ulong line_addr;
+    int i;
+
+    for (i = 0; i < 512; i++) {
+        pml4e_addr = (pml4e_start_addr + i * 8) & a20_mask;
+        pml4e = ldq_phys(pml4e_addr);
+        if (!(pml4e & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        line_addr = ((i & 0x1ffULL) << 39) | (0xffffULL << 48);
+        pdpe_start_addr = (pml4e & ~0xfff) & a20_mask;
+        walk_pdpe(list, pdpe_start_addr, a20_mask, line_addr);
+    }
+}
+#endif
+
+void cpu_get_memory_mapping(MemoryMappingList *list, CPUState *env)
+{
+    if (env->cr[4] & CR4_PAE_MASK) {
+#ifdef TARGET_X86_64
+        if (env->hflags & HF_LMA_MASK) {
+            target_phys_addr_t pml4e_addr;
+
+            pml4e_addr = (env->cr[3] & ~0xfff) & env->a20_mask;
+            walk_pml4e(list, pml4e_addr, env->a20_mask);
+        } else
+#endif
+        {
+            target_phys_addr_t pdpe_addr;
+
+            pdpe_addr = (env->cr[3] & ~0x1f) & env->a20_mask;
+            walk_pdpe2(list, pdpe_addr, env->a20_mask);

Quoted 40 lines

Walk cpu's page table and collect all virtual address and physical address mapping. Then, add these mapping into memory mapping list.

Signed-off-by Wen Congyang <[email protected]>

---
 Makefile.target         |    2 +-
 cpu-all.h               |    7 ++
 target-i386/arch-dump.c |  254 +++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 262 insertions(+), 1 deletions(-)
 create mode 100644 target-i386/arch-dump.c

Makefile.target +1 -1

index e35e464..d6e5684 100644
@@ -75,7 +75,7 @@ libobj-$(CONFIG_TCG_INTERPRETER) += tci.o
 libobj-y += fpu/softfloat.o
 libobj-y += op_helper.o helper.o
 ifeq ($(TARGET_BASE_ARCH), i386)
-libobj-y += cpuid.o
+libobj-y += cpuid.o arch-dump.o
 endif
 libobj-$(TARGET_SPARC64) += vis_helper.o
 libobj-$(CONFIG_NEED_MMU) += mmu.o

cpu-all.h +1 -0

index e2c3c49..4cd7fbb 100644
@@ -22,6 +22,7 @@
 #include "qemu-common.h"
 #include "qemu-tls.h"
 #include "cpu-common.h"
+#include "memory_mapping.h"
 
 /* some important defines:
  *

@@ -523,4 +524,10 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf);
 int cpu_memory_rw_debug(CPUState *env, target_ulong addr,
                         uint8_t *buf, int len, int is_write);
 

cpu-all.h void dump_exec_info(FILE *f, fprintf_function cpu_fprintf); · line 527

Quoted 1 line

cpu-all.h +1 -0

+#if defined(TARGET_I386)

Instead of collecting archs here, you could introduce some HAVE_GET_MEMORY_MAPPING and let the targets that support that define it.

cpu-all.h void dump_exec_info(FILE *f, fprintf_function cpu_fprintf); · line 530

Quoted 4 lines

cpu-all.h +3 -0

+void cpu_get_memory_mapping(MemoryMappingList *list, CPUState *env);
+#else
+#define cpu_get_memory_mapping(list, env)

Better return an error from cpu_get_memory_mapping (and use static inline) so that the caller can find out and report that dumping is not supported for the current target.

Quoted 254 lines

cpu-all.h +2 -0

+#endif
+
 #endif /* CPU_ALL_H */

target-i386/arch-dump.c new file +244 -0

new file mode 100644
index 0000000..2e921c7
@@ -0,0 +1,254 @@
+/*
+ * i386 dump
+ *
+ * Copyright Fujitsu, Corp. 2011
+ *
+ * Authors:
+ *     Wen Congyang <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ */
+
+#include "cpu.h"
+#include "cpu-all.h"
+
+/* PAE Paging or IA-32e Paging */
+static void walk_pte(MemoryMappingList *list, target_phys_addr_t pte_start_addr,
+                     int32_t a20_mask, target_ulong start_line_addr)
+{
+    target_phys_addr_t pte_addr, start_paddr;
+    uint64_t pte;
+    target_ulong start_vaddr;
+    int i;
+
+    for (i = 0; i < 512; i++) {
+        pte_addr = (pte_start_addr + i * 8) & a20_mask;
+        pte = ldq_phys(pte_addr);
+        if (!(pte & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        start_paddr = (pte & ~0xfff) & ~(0x1ULL << 63);
+        if (is_io_addr(start_paddr)) {
+            /* I/O region */
+            continue;
+        }
+
+        start_vaddr = start_line_addr | ((i & 0x1fff) << 12);
+        add_to_memory_mapping(list, start_paddr, start_vaddr, 1 << 12);
+    }
+}
+
+/* 32-bit Paging */
+static void walk_pte2(MemoryMappingList *list,
+                      target_phys_addr_t pte_start_addr, int32_t a20_mask,
+                      target_ulong start_line_addr)
+{
+    target_phys_addr_t pte_addr, start_paddr;
+    uint32_t pte;
+    target_ulong start_vaddr;
+    int i;
+
+    for (i = 0; i < 1024; i++) {
+        pte_addr = (pte_start_addr + i * 4) & a20_mask;
+        pte = ldl_phys(pte_addr);
+        if (!(pte & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        start_paddr = pte & ~0xfff;
+        if (is_io_addr(start_paddr)) {
+            /* I/O region */
+            continue;
+        }
+
+        start_vaddr = start_line_addr | ((i & 0x3ff) << 12);
+        add_to_memory_mapping(list, start_paddr, start_vaddr, 1 << 12);
+    }
+}
+
+/* PAE Paging or IA-32e Paging */
+static void walk_pde(MemoryMappingList *list, target_phys_addr_t pde_start_addr,
+                     int32_t a20_mask, target_ulong start_line_addr)
+{
+    target_phys_addr_t pde_addr, pte_start_addr, start_paddr;
+    uint64_t pde;
+    target_ulong line_addr, start_vaddr;
+    int i;
+
+    for (i = 0; i < 512; i++) {
+        pde_addr = (pde_start_addr + i * 8) & a20_mask;
+        pde = ldq_phys(pde_addr);
+        if (!(pde & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        line_addr = start_line_addr | ((i & 0x1ff) << 21);
+        if (pde & PG_PSE_MASK) {
+            /* 2 MB page */
+            start_paddr = (pde & ~0x1fffff) & ~(0x1ULL << 63);
+            if (is_io_addr(start_paddr)) {
+                /* I/O region */
+                continue;
+            }
+            start_vaddr = line_addr;
+            add_to_memory_mapping(list, start_paddr, start_vaddr, 1 << 21);
+            continue;
+        }
+
+        pte_start_addr = (pde & ~0xfff) & a20_mask;
+        walk_pte(list, pte_start_addr, a20_mask, line_addr);
+    }
+}
+
+/* 32-bit Paging */
+static void walk_pde2(MemoryMappingList *list,
+                      target_phys_addr_t pde_start_addr, int32_t a20_mask,
+                      bool pse)
+{
+    target_phys_addr_t pde_addr, pte_start_addr, start_paddr;
+    uint32_t pde;
+    target_ulong line_addr, start_vaddr;
+    int i;
+
+    for (i = 0; i < 1024; i++) {
+        pde_addr = (pde_start_addr + i * 4) & a20_mask;
+        pde = ldl_phys(pde_addr);
+        if (!(pde & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        line_addr = (((unsigned int)i & 0x3ff) << 22);
+        if ((pde & PG_PSE_MASK) && pse) {
+            /* 4 MB page */
+            start_paddr = (pde & ~0x3fffff) | ((pde & 0x1fe000) << 19);
+            if (is_io_addr(start_paddr)) {
+                /* I/O region */
+                continue;
+            }
+            start_vaddr = line_addr;
+            add_to_memory_mapping(list, start_paddr, start_vaddr, 1 << 22);
+            continue;
+        }
+
+        pte_start_addr = (pde & ~0xfff) & a20_mask;
+        walk_pte2(list, pte_start_addr, a20_mask, line_addr);
+    }
+}
+
+/* PAE Paging */
+static void walk_pdpe2(MemoryMappingList *list,
+                       target_phys_addr_t pdpe_start_addr, int32_t a20_mask)
+{
+    target_phys_addr_t pdpe_addr, pde_start_addr;
+    uint64_t pdpe;
+    target_ulong line_addr;
+    int i;
+
+    for (i = 0; i < 4; i++) {
+        pdpe_addr = (pdpe_start_addr + i * 8) & a20_mask;
+        pdpe = ldq_phys(pdpe_addr);
+        if (!(pdpe & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        line_addr = (((unsigned int)i & 0x3) << 30);
+        pde_start_addr = (pdpe & ~0xfff) & a20_mask;
+        walk_pde(list, pde_start_addr, a20_mask, line_addr);
+    }
+}
+
+#ifdef TARGET_X86_64
+/* IA-32e Paging */
+static void walk_pdpe(MemoryMappingList *list,
+                      target_phys_addr_t pdpe_start_addr, int32_t a20_mask,
+                      target_ulong start_line_addr)
+{
+    target_phys_addr_t pdpe_addr, pde_start_addr, start_paddr;
+    uint64_t pdpe;
+    target_ulong line_addr, start_vaddr;
+    int i;
+
+    for (i = 0; i < 512; i++) {
+        pdpe_addr = (pdpe_start_addr + i * 8) & a20_mask;
+        pdpe = ldq_phys(pdpe_addr);
+        if (!(pdpe & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        line_addr = start_line_addr | ((i & 0x1ffULL) << 30);
+        if (pdpe & PG_PSE_MASK) {
+            /* 1 GB page */
+            start_paddr = (pdpe & ~0x3fffffff) & ~(0x1ULL << 63);
+            if (is_io_addr(start_paddr)) {
+                /* I/O region */
+                continue;
+            }
+            start_vaddr = line_addr;
+            add_to_memory_mapping(list, start_paddr, start_vaddr, 1 << 30);
+            continue;
+        }
+
+        pde_start_addr = (pdpe & ~0xfff) & a20_mask;
+        walk_pde(list, pde_start_addr, a20_mask, line_addr);
+    }
+}
+
+/* IA-32e Paging */
+static void walk_pml4e(MemoryMappingList *list,
+                       target_phys_addr_t pml4e_start_addr, int32_t a20_mask)
+{
+    target_phys_addr_t pml4e_addr, pdpe_start_addr;
+    uint64_t pml4e;
+    target_ulong line_addr;
+    int i;
+
+    for (i = 0; i < 512; i++) {
+        pml4e_addr = (pml4e_start_addr + i * 8) & a20_mask;
+        pml4e = ldq_phys(pml4e_addr);
+        if (!(pml4e & PG_PRESENT_MASK)) {
+            /* not present */
+            continue;
+        }
+
+        line_addr = ((i & 0x1ffULL) << 39) | (0xffffULL << 48);
+        pdpe_start_addr = (pml4e & ~0xfff) & a20_mask;
+        walk_pdpe(list, pdpe_start_addr, a20_mask, line_addr);
+    }
+}
+#endif
+
+void cpu_get_memory_mapping(MemoryMappingList *list, CPUState *env)
+{
+    if (env->cr[4] & CR4_PAE_MASK) {
+#ifdef TARGET_X86_64
+        if (env->hflags & HF_LMA_MASK) {
+            target_phys_addr_t pml4e_addr;
+
+            pml4e_addr = (env->cr[3] & ~0xfff) & env->a20_mask;
+            walk_pml4e(list, pml4e_addr, env->a20_mask);
+        } else
+#endif
+        {
+            target_phys_addr_t pdpe_addr;
+
+            pdpe_addr = (env->cr[3] & ~0x1f) & env->a20_mask;
+            walk_pdpe2(list, pdpe_addr, env->a20_mask);

target-i386/arch-dump.c line 254

Quoted 10 lines

target-i386/arch-dump.c new file +10 -0

+        }
+    } else {
+        target_phys_addr_t pde_addr;
+        bool pse;
+
+        pde_addr = (env->cr[3] & ~0xfff) & env->a20_mask;
+        pse = !!(env->cr[4] & CR4_PSE_MASK);
+        walk_pde2(list, pde_addr, env->a20_mask, pse);
+    }
+}

I haven't checked all paging details, but it looks good otherwise.

Jan

Add API to get all virtual address and physical address mapping. If there is no virtual address for some physical address, the virtual address is 0.

Signed-off-by Wen Congyang <[email protected]>

---
 memory_mapping.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory_mapping.h |    1 +
 2 files changed, 66 insertions(+), 0 deletions(-)

memory_mapping.c +2 -0

index d83b7d7..fc0ddee 100644
@@ -128,3 +128,68 @@ void free_memory_mapping_list(MemoryMappingList *list)
 
     list->num = 0;
 }
+
+void get_memory_mapping(MemoryMappingList *list)

memory_mapping.c +42 -0

+    }
+
+    /* some memory may be not mapped, add them into memory mapping's list */
+    QLIST_FOREACH(block, &ram_list.blocks, next) {
+        offset = block->offset;
+        length = block->length;
+
+        QTAILQ_FOREACH(memory_mapping, &list->head, next) {
+            if (memory_mapping->phys_addr >= (offset + length)) {
+                /*
+                 * memory_mapping's list does not conatin the region
+                 * [offset, offset+length)
+                 */
+                create_new_memory_mapping(list, offset, 0, length);
+                length = 0;
+                break;
+            }
+
+            if ((memory_mapping->phys_addr + memory_mapping->length) <=
+                offset) {
+                continue;
+            }
+
+            if (memory_mapping->phys_addr > offset) {
+                /*
+                 * memory_mapping's list does not conatin the region
+                 * [offset, memory_mapping->phys_addr)
+                 */
+                create_new_memory_mapping(list, offset, 0,
+                                          memory_mapping->phys_addr - offset);
+            }
+
+            if ((offset + length) <=
+                (memory_mapping->phys_addr + memory_mapping->length)) {
+                length = 0;
+                break;
+            }
+            length -= memory_mapping->phys_addr + memory_mapping->length -
+                      offset;
+            offset = memory_mapping->phys_addr + memory_mapping->length;
+        }
+

Quoted 20 lines

Add API to get all virtual address and physical address mapping. If there is no virtual address for some physical address, the virtual address is 0.

Signed-off-by Wen Congyang <[email protected]>

---
 memory_mapping.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory_mapping.h |    1 +
 2 files changed, 66 insertions(+), 0 deletions(-)

memory_mapping.c +2 -0

index d83b7d7..fc0ddee 100644
@@ -128,3 +128,68 @@ void free_memory_mapping_list(MemoryMappingList *list)
 
     list->num = 0;
 }
+
+void get_memory_mapping(MemoryMappingList *list)

memory_mapping.c void free_memory_mapping_list(MemoryMappingList *list) · line 142

Quoted 10 lines

memory_mapping.c +10 -0

+{
+    CPUState *env;
+    MemoryMapping *memory_mapping;
+    RAMBlock *block;
+    ram_addr_t offset, length;
+
+    last_mapping = NULL;
+
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+        cpu_get_memory_mapping(list, env);

Hmm, is the CPU number recorded along with the mappings? I mean, how could crash tell them apart afterward if they are contradictory? This way, they are just thrown in the same bucket, correct?

Even if crash or gdb aren't prepared for cpu/thread-specific mappings, could we already record that information for later use? Or would it break compatibility with current versions?

Quoted 47 lines

memory_mapping.c +46 -0

+    }
+
+    /* some memory may be not mapped, add them into memory mapping's list */
+    QLIST_FOREACH(block, &ram_list.blocks, next) {
+        offset = block->offset;
+        length = block->length;
+
+        QTAILQ_FOREACH(memory_mapping, &list->head, next) {
+            if (memory_mapping->phys_addr >= (offset + length)) {
+                /*
+                 * memory_mapping's list does not conatin the region
+                 * [offset, offset+length)
+                 */
+                create_new_memory_mapping(list, offset, 0, length);
+                length = 0;
+                break;
+            }
+
+            if ((memory_mapping->phys_addr + memory_mapping->length) <=
+                offset) {
+                continue;
+            }
+
+            if (memory_mapping->phys_addr > offset) {
+                /*
+                 * memory_mapping's list does not conatin the region
+                 * [offset, memory_mapping->phys_addr)
+                 */
+                create_new_memory_mapping(list, offset, 0,
+                                          memory_mapping->phys_addr - offset);
+            }
+
+            if ((offset + length) <=
+                (memory_mapping->phys_addr + memory_mapping->length)) {
+                length = 0;
+                break;
+            }
+            length -= memory_mapping->phys_addr + memory_mapping->length -
+                      offset;
+            offset = memory_mapping->phys_addr + memory_mapping->length;
+        }
+
+        if (length > 0) {
+            /*
+             * memory_mapping's list does not conatin the region
+             * [offset, memory_mapping->phys_addr)

memory_mapping.c void free_memory_mapping_list(MemoryMappingList *list) · line 194

Quoted 6 lines

memory_mapping.c +6 -0

+             */
+            create_new_memory_mapping(list, offset, 0, length);
+        }
+    }
+
+    return;

Please avoid redundant returns.

Quoted 9 lines

memory_mapping.c +1 -0

+}

memory_mapping.h +0 -0

index a4b1532..679f9ef 100644
@@ -34,5 +34,6 @@ void add_to_memory_mapping(MemoryMappingList *list,
                            ram_addr_t length);
 

memory_mapping.h void add_to_memory_mapping(MemoryMappingList *list, · line 39

Quoted 4 lines

memory_mapping.h +1 -0

 void free_memory_mapping_list(MemoryMappingList *list);
+void get_memory_mapping(MemoryMappingList *list);
 
 #endif

Maybe [qemu_]get_guest_memory_mapping. Just get_memory_mapping sounds a bit to generic to me. Could be any mapping.

Jan

At 02/15/2012 01:21 AM, Jan Kiszka Wrote:

Quoted 36 lines · Wen Congyang

Quoted 20 lines

Add API to get all virtual address and physical address mapping. If there is no virtual address for some physical address, the virtual address is 0.

Signed-off-by Wen Congyang <[email protected]>

---
 memory_mapping.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory_mapping.h |    1 +
 2 files changed, 66 insertions(+), 0 deletions(-)

memory_mapping.c +2 -0

index d83b7d7..fc0ddee 100644
@@ -128,3 +128,68 @@ void free_memory_mapping_list(MemoryMappingList *list)
 
     list->num = 0;
 }
+
+void get_memory_mapping(MemoryMappingList *list)

memory_mapping.c void free_memory_mapping_list(MemoryMappingList *list) · line 142

Quoted 10 lines

memory_mapping.c +10 -0

+{
+    CPUState *env;
+    MemoryMapping *memory_mapping;
+    RAMBlock *block;
+    ram_addr_t offset, length;
+
+    last_mapping = NULL;
+
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+        cpu_get_memory_mapping(list, env);

Hmm, is the CPU number recorded along with the mappings? I mean, how could crash tell them apart afterward if they are contradictory? This way, they are just thrown in the same bucket, correct?

Quoted 3 lines

Even if crash or gdb aren't prepared for cpu/thread-specific mappings, could we already record that information for later use? Or would it break compatibility with current versions?

crash does not need this information. It only needs the physical address stored in PT_LOAD.

gdb needs the virtual address and physical address stored in PT_LOAD.

If the address is in the kernel space, the virtual address and physical address mapping should be the same. I collect the mapping information from all vcpus, because the OS may enter the second kernel. In this case, IIRC(according to my test result, but I don't remeber clearly), gdb's bt can output the backtrace in the first kernel if the OS does not use the first vcpu to do kdump. otherwise gdb's bt can output the backtrace in the second kernel.

Quoted 37 lines

At 02/15/2012 01:21 AM, Jan Kiszka Wrote:

Quoted 36 lines · Wen Congyang

Quoted 20 lines

Add API to get all virtual address and physical address mapping. If there is no virtual address for some physical address, the virtual address is 0.

Signed-off-by Wen Congyang <[email protected]>

---
 memory_mapping.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory_mapping.h |    1 +
 2 files changed, 66 insertions(+), 0 deletions(-)

memory_mapping.c +2 -0

index d83b7d7..fc0ddee 100644
@@ -128,3 +128,68 @@ void free_memory_mapping_list(MemoryMappingList *list)
 
     list->num = 0;
 }
+
+void get_memory_mapping(MemoryMappingList *list)

memory_mapping.c void free_memory_mapping_list(MemoryMappingList *list) · line 142

Quoted 10 lines

memory_mapping.c +10 -0

+{
+    CPUState *env;
+    MemoryMapping *memory_mapping;
+    RAMBlock *block;
+    ram_addr_t offset, length;
+
+    last_mapping = NULL;
+
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+        cpu_get_memory_mapping(list, env);

Hmm, is the CPU number recorded along with the mappings? I mean, how could crash tell them apart afterward if they are contradictory? This way, they are just thrown in the same bucket, correct?

Quoted 6 lines

Quoted 3 lines

Even if crash or gdb aren't prepared for cpu/thread-specific mappings, could we already record that information for later use? Or would it break compatibility with current versions?

crash does not need this information. It only needs the physical address stored in PT_LOAD.

So crash does not support viewing memory through the eyes of different CPUs? OK.

Quoted 10 lines

gdb needs the virtual address and physical address stored in PT_LOAD.

If the address is in the kernel space, the virtual address and physical address mapping should be the same. I collect the mapping information from all vcpus, because the OS may enter the second kernel. In this case, IIRC(according to my test result, but I don't remeber clearly), gdb's bt can output the backtrace in the first kernel if the OS does not use the first vcpu to do kdump. otherwise gdb's bt can output the backtrace in the second kernel.

gdb could only make proper use of the additional mappings if they are not contradictory (which can easily happen with user space processes) or the cpu context is additionally provided so that views can be switched via the "thread N" command. So far, QEMU's gdbstub does this for gdb when it requests some memory over the remote connection. I bet gdb requires some extension to exploit such information offline from a core file, but I'm also sure that this will come as the importance of gdb for system level debugging will rise.

Therefore my question: is there room to encode the mapping relation to a CPU/thread context?

Jan

At 02/15/2012 05:17 PM, Jan Kiszka Wrote:

Quoted 68 lines · Wen Congyang

Quoted 37 lines

At 02/15/2012 01:21 AM, Jan Kiszka Wrote:

Quoted 36 lines · Wen Congyang

Quoted 20 lines

Add API to get all virtual address and physical address mapping. If there is no virtual address for some physical address, the virtual address is 0.

Signed-off-by Wen Congyang <[email protected]>

---
 memory_mapping.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory_mapping.h |    1 +
 2 files changed, 66 insertions(+), 0 deletions(-)

memory_mapping.c +2 -0

index d83b7d7..fc0ddee 100644
@@ -128,3 +128,68 @@ void free_memory_mapping_list(MemoryMappingList *list)
 
     list->num = 0;
 }
+
+void get_memory_mapping(MemoryMappingList *list)

memory_mapping.c void free_memory_mapping_list(MemoryMappingList *list) · line 142

Quoted 10 lines

memory_mapping.c +10 -0

+{
+    CPUState *env;
+    MemoryMapping *memory_mapping;
+    RAMBlock *block;
+    ram_addr_t offset, length;
+
+    last_mapping = NULL;
+
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+        cpu_get_memory_mapping(list, env);

Hmm, is the CPU number recorded along with the mappings? I mean, how could crash tell them apart afterward if they are contradictory? This way, they are just thrown in the same bucket, correct?

Quoted 6 lines

Quoted 3 lines

Even if crash or gdb aren't prepared for cpu/thread-specific mappings, could we already record that information for later use? Or would it break compatibility with current versions?

crash does not need this information. It only needs the physical address stored in PT_LOAD.

So crash does not support viewing memory through the eyes of different CPUs? OK.

Quoted 10 lines

gdb needs the virtual address and physical address stored in PT_LOAD.

If the address is in the kernel space, the virtual address and physical address mapping should be the same. I collect the mapping information from all vcpus, because the OS may enter the second kernel. In this case, IIRC(according to my test result, but I don't remeber clearly), gdb's bt can output the backtrace in the first kernel if the OS does not use the first vcpu to do kdump. otherwise gdb's bt can output the backtrace in the second kernel.

gdb could only make proper use of the additional mappings if they are not contradictory (which can easily happen with user space processes) or the cpu context is additionally provided so that views can be switched via the "thread N" command. So far, QEMU's gdbstub does this for gdb when it requests some memory over the remote connection. I bet gdb requires some extension to exploit such information offline from a core file, but I'm also sure that this will come as the importance of gdb for system level debugging will rise.

Quoted 2 lines

Therefore my question: is there room to encode the mapping relation to a CPU/thread context?

I donot know. But I think the answer is no, because there is no filed in the struct Elf32_Phdr/Elf64_Phdr to store the CPU/thread id.

Thanks
Wen Congyang

Quoted 3 lines

Jan

From: Wen Congyang <[email protected]>
Subject: Re: [RFC][PATCH 05/16 v6] Add API to get memory mapping
Date: Wed, 15 Feb 2012 17:41:15 +0800

Quoted 69 lines

At 02/15/2012 05:17 PM, Jan Kiszka Wrote:

Quoted 68 lines · Wen Congyang

Quoted 37 lines

At 02/15/2012 01:21 AM, Jan Kiszka Wrote:

Quoted 36 lines · Wen Congyang

Quoted 20 lines

Add API to get all virtual address and physical address mapping. If there is no virtual address for some physical address, the virtual address is 0.

Signed-off-by Wen Congyang <[email protected]>

---
 memory_mapping.c |   65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 memory_mapping.h |    1 +
 2 files changed, 66 insertions(+), 0 deletions(-)

memory_mapping.c +2 -0

index d83b7d7..fc0ddee 100644
@@ -128,3 +128,68 @@ void free_memory_mapping_list(MemoryMappingList *list)
 
     list->num = 0;
 }
+
+void get_memory_mapping(MemoryMappingList *list)

memory_mapping.c void free_memory_mapping_list(MemoryMappingList *list) · line 142

Quoted 10 lines

memory_mapping.c +10 -0

+{
+    CPUState *env;
+    MemoryMapping *memory_mapping;
+    RAMBlock *block;
+    ram_addr_t offset, length;
+
+    last_mapping = NULL;
+
+    for (env = first_cpu; env != NULL; env = env->next_cpu) {
+        cpu_get_memory_mapping(list, env);

Hmm, is the CPU number recorded along with the mappings? I mean, how could crash tell them apart afterward if they are contradictory? This way, they are just thrown in the same bucket, correct?

Quoted 6 lines

Quoted 3 lines

Even if crash or gdb aren't prepared for cpu/thread-specific mappings, could we already record that information for later use? Or would it break compatibility with current versions?

crash does not need this information. It only needs the physical address stored in PT_LOAD.

So crash does not support viewing memory through the eyes of different CPUs? OK.

Quoted 10 lines

gdb needs the virtual address and physical address stored in PT_LOAD.

If the address is in the kernel space, the virtual address and physical address mapping should be the same. I collect the mapping information from all vcpus, because the OS may enter the second kernel. In this case, IIRC(according to my test result, but I don't remeber clearly), gdb's bt can output the backtrace in the first kernel if the OS does not use the first vcpu to do kdump. otherwise gdb's bt can output the backtrace in the second kernel.

gdb could only make proper use of the additional mappings if they are not contradictory (which can easily happen with user space processes) or the cpu context is additionally provided so that views can be switched via the "thread N" command. So far, QEMU's gdbstub does this for gdb when it requests some memory over the remote connection. I bet gdb requires some extension to exploit such information offline from a core file, but I'm also sure that this will come as the importance of gdb for system level debugging will rise.

Quoted 6 lines

Quoted 2 lines

Therefore my question: is there room to encode the mapping relation to a CPU/thread context?

I donot know. But I think the answer is no, because there is no filed in the struct Elf32_Phdr/Elf64_Phdr to store the CPU/thread id.

See NT_PRSTATUS note, from which gdb knows what CPUs is related to what thread.

For vmcore generated by kdump, NT_PRSTATUS notes is contained in the order corresponding to online cpus.

If crash reads the vmcore generated by this command just as by kdump and not considering this, crash might be understanding each CPU information wrongly because qemu dump generated all possible CPUs.

The core file contains register's value. These APIs write registers to core file, and them will be called in the following patch.

Signed-off-by Wen Congyang <[email protected]>

---
 cpu-all.h               |    6 +
 target-i386/arch-dump.c |  243 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 249 insertions(+), 0 deletions(-)

cpu-all.h +0 -0

index 4cd7fbb..efb5ba3 100644

cpu-all.h +2 -0

 #else
 #define cpu_get_memory_mapping(list, env)
+#define cpu_write_elf64_note(fd, env, cpuid, offset) ({ -1; })
+#define cpu_write_elf32_note(fd, env, cpuid, offset) ({ -1; })
 #endif
 
 #endif /* CPU_ALL_H */

target-i386/arch-dump.c +0 -0

index 2e921c7..4c0ff77 100644

target-i386/arch-dump.c +232 -0

+
 #include "cpu.h"
 #include "cpu-all.h"
+#include "monitor.h"
 
 /* PAE Paging or IA-32e Paging */
 static void walk_pte(MemoryMappingList *list, target_phys_addr_t pte_start_addr,

@@ -252,3 +255,243 @@ void cpu_get_memory_mapping(MemoryMappingList *list, CPUState *env)
         walk_pde2(list, pde_addr, env->a20_mask, pse);
     }
 }
+
+#ifdef TARGET_X86_64
+typedef struct {
+    target_ulong r15, r14, r13, r12, rbp, rbx, r11, r10;
+    target_ulong r9, r8, rax, rcx, rdx, rsi, rdi, orig_rax;
+    target_ulong rip, cs, eflags;
+    target_ulong rsp, ss;
+    target_ulong fs_base, gs_base;
+    target_ulong ds, es, fs, gs;
+} x86_64_user_regs_struct;
+
+static int x86_64_write_elf64_note(int fd, CPUState *env, int id,
+                                   target_phys_addr_t *offset)
+{
+    x86_64_user_regs_struct regs;
+    Elf64_Nhdr *note;
+    char *buf;
+    int descsz, note_size, name_size = 5;
+    const char *name = "CORE";
+    int ret;
+
+    regs.r15 = env->regs[15];
+    regs.r14 = env->regs[14];
+    regs.r13 = env->regs[13];
+    regs.r12 = env->regs[12];
+    regs.r11 = env->regs[11];
+    regs.r10 = env->regs[10];
+    regs.r9  = env->regs[9];
+    regs.r8  = env->regs[8];
+    regs.rbp = env->regs[R_EBP];
+    regs.rsp = env->regs[R_ESP];
+    regs.rdi = env->regs[R_EDI];
+    regs.rsi = env->regs[R_ESI];
+    regs.rdx = env->regs[R_EDX];
+    regs.rcx = env->regs[R_ECX];
+    regs.rbx = env->regs[R_EBX];
+    regs.rax = env->regs[R_EAX];
+    regs.rip = env->eip;
+    regs.eflags = env->eflags;
+
+    regs.orig_rax = 0; /* FIXME */
+    regs.cs = env->segs[R_CS].selector;
+    regs.ss = env->segs[R_SS].selector;
+    regs.fs_base = env->segs[R_FS].base;
+    regs.gs_base = env->segs[R_GS].base;
+    regs.ds = env->segs[R_DS].selector;
+    regs.es = env->segs[R_ES].selector;
+    regs.fs = env->segs[R_FS].selector;
+    regs.gs = env->segs[R_GS].selector;
+
+    descsz = 336; /* sizeof(prstatus_t) is 336 on x86_64 box */
+    note_size = ((sizeof(Elf64_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
+                (descsz + 3) / 4) * 4;
+    note = g_malloc(note_size);
+
+    memset(note, 0, note_size);
+    note->n_namesz = cpu_to_le32(name_size);
+    note->n_descsz = cpu_to_le32(descsz);
+    note->n_type = cpu_to_le32(NT_PRSTATUS);
+    buf = (char *)note;
+    buf += ((sizeof(Elf64_Nhdr) + 3) / 4) * 4;
+    memcpy(buf, name, name_size);
+    buf += ((name_size + 3) / 4) * 4;
+    memcpy(buf + 32, &id, 4); /* pr_pid */
+    buf += descsz - sizeof(x86_64_user_regs_struct)-sizeof(target_ulong);
+    memcpy(buf, &regs, sizeof(x86_64_user_regs_struct));
+
+    lseek(fd, *offset, SEEK_SET);
+    ret = write(fd, note, note_size);
+    g_free(note);
+    if (ret < 0) {
+        return -1;
+    }
+
+    *offset += note_size;
+
+    return 0;
+}
+#endif
+
+typedef struct {
+    uint32_t ebx, ecx, edx, esi, edi, ebp, eax;
+    unsigned short ds, __ds, es, __es;
+    unsigned short fs, __fs, gs, __gs;
+    uint32_t orig_eax, eip;
+    unsigned short cs, __cs;
+    uint32_t eflags, esp;
+    unsigned short ss, __ss;
+} x86_user_regs_struct;
+
+static int x86_write_elf64_note(int fd, CPUState *env, int id,
+                                target_phys_addr_t *offset)
+{
+    x86_user_regs_struct regs;
+    Elf64_Nhdr *note;
+    char *buf;
+    int descsz, note_size, name_size = 5;
+    const char *name = "CORE";
+    int ret;
+
+    regs.ebp = env->regs[R_EBP] & 0xffffffff;
+    regs.esp = env->regs[R_ESP] & 0xffffffff;
+    regs.edi = env->regs[R_EDI] & 0xffffffff;
+    regs.esi = env->regs[R_ESI] & 0xffffffff;
+    regs.edx = env->regs[R_EDX] & 0xffffffff;
+    regs.ecx = env->regs[R_ECX] & 0xffffffff;
+    regs.ebx = env->regs[R_EBX] & 0xffffffff;
+    regs.eax = env->regs[R_EAX] & 0xffffffff;
+    regs.eip = env->eip & 0xffffffff;
+    regs.eflags = env->eflags & 0xffffffff;
+
+    regs.cs = env->segs[R_CS].selector;
+    regs.__cs = 0;
+    regs.ss = env->segs[R_SS].selector;
+    regs.__ss = 0;
+    regs.ds = env->segs[R_DS].selector;
+    regs.__ds = 0;
+    regs.es = env->segs[R_ES].selector;
+    regs.__es = 0;
+    regs.fs = env->segs[R_FS].selector;
+    regs.__fs = 0;
+    regs.gs = env->segs[R_GS].selector;
+    regs.__gs = 0;
+
+    descsz = 144; /* sizeof(prstatus_t) is 144 on x86 box */
+    note_size = ((sizeof(Elf64_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
+                (descsz + 3) / 4) * 4;
+    note = g_malloc(note_size);
+
+    memset(note, 0, note_size);
+    note->n_namesz = cpu_to_le32(name_size);
+    note->n_descsz = cpu_to_le32(descsz);
+    note->n_type = cpu_to_le32(NT_PRSTATUS);
+    buf = (char *)note;
+    buf += ((sizeof(Elf64_Nhdr) + 3) / 4) * 4;
+    memcpy(buf, name, name_size);
+    buf += ((name_size + 3) / 4) * 4;
+    memcpy(buf + 24, &id, 4); /* pr_pid */
+    buf += descsz - sizeof(x86_user_regs_struct)-4;
+    memcpy(buf, &regs, sizeof(x86_user_regs_struct));
+
+    lseek(fd, *offset, SEEK_SET);
+    ret = write(fd, note, note_size);
+    g_free(note);
+    if (ret < 0) {
+        return -1;
+    }
+
+    *offset += note_size;
+
+    return 0;
+}
+
+static int x86_write_elf32_note(int fd, CPUState *env, int id,
+                                target_phys_addr_t *offset)
+{
+    x86_user_regs_struct regs;
+    Elf32_Nhdr *note;
+    char *buf;
+    int descsz, note_size, name_size = 5;
+    const char *name = "CORE";
+    int ret;
+
+    regs.ebp = env->regs[R_EBP] & 0xffffffff;
+    regs.esp = env->regs[R_ESP] & 0xffffffff;
+    regs.edi = env->regs[R_EDI] & 0xffffffff;
+    regs.esi = env->regs[R_ESI] & 0xffffffff;
+    regs.edx = env->regs[R_EDX] & 0xffffffff;
+    regs.ecx = env->regs[R_ECX] & 0xffffffff;
+    regs.ebx = env->regs[R_EBX] & 0xffffffff;
+    regs.eax = env->regs[R_EAX] & 0xffffffff;
+    regs.eip = env->eip & 0xffffffff;
+    regs.eflags = env->eflags & 0xffffffff;
+
+    regs.cs = env->segs[R_CS].selector;
+    regs.__cs = 0;
+    regs.ss = env->segs[R_SS].selector;
+    regs.__ss = 0;
+    regs.ds = env->segs[R_DS].selector;
+    regs.__ds = 0;
+    regs.es = env->segs[R_ES].selector;
+    regs.__es = 0;
+    regs.fs = env->segs[R_FS].selector;
+    regs.__fs = 0;
+    regs.gs = env->segs[R_GS].selector;
+    regs.__gs = 0;
+
+    descsz = 144; /* sizeof(prstatus_t) is 144 on x86 box */
+    note_size = ((sizeof(Elf32_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
+                (descsz + 3) / 4) * 4;
+    note = g_malloc(note_size);
+
+    memset(note, 0, note_size);
+    note->n_namesz = cpu_to_le32(name_size);
+    note->n_descsz = cpu_to_le32(descsz);
+    note->n_type = cpu_to_le32(NT_PRSTATUS);
+    buf = (char *)note;
+    buf += ((sizeof(Elf32_Nhdr) + 3) / 4) * 4;
+    memcpy(buf, name, name_size);
+    buf += ((name_size + 3) / 4) * 4;
+    memcpy(buf + 24, &id, 4); /* pr_pid */
+    buf += descsz - sizeof(x86_user_regs_struct)-4;
+    memcpy(buf, &regs, sizeof(x86_user_regs_struct));
+
+    lseek(fd, *offset, SEEK_SET);
+    ret = write(fd, note, note_size);
+    g_free(note);
+    if (ret < 0) {
+        return -1;
+    }
+
+    *offset += note_size;
+
+    return 0;
+}
+
+int cpu_write_elf64_note(int fd, CPUState *env, int cpuid,
+                         target_phys_addr_t *offset)
+{
+    int ret;
+#ifdef TARGET_X86_64
+    bool lma = !!(first_cpu->hflags & HF_LMA_MASK);
+
+    if (lma) {
+        ret = x86_64_write_elf64_note(fd, env, cpuid, offset);
+    } else {
+#endif
+        ret = x86_write_elf64_note(fd, env, cpuid, offset);
+#ifdef TARGET_X86_64
+    }

Quoted 15 lines

The core file contains register's value. These APIs write registers to core file, and them will be called in the following patch.

Signed-off-by Wen Congyang <[email protected]>

---
 cpu-all.h               |    6 +
 target-i386/arch-dump.c |  243 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 249 insertions(+), 0 deletions(-)

cpu-all.h +0 -0

index 4cd7fbb..efb5ba3 100644
@@ -526,8 +526,14 @@ int cpu_memory_rw_debug(CPUState *env, target_ulong addr,
 

cpu-all.h int cpu_memory_rw_debug(CPUState *env, target_ulong addr, · line 532

Quoted 6 lines

cpu-all.h +4 -0

 #if defined(TARGET_I386)
 void cpu_get_memory_mapping(MemoryMappingList *list, CPUState *env);
+int cpu_write_elf64_note(int fd, CPUState *env, int cpuid,
+                         target_phys_addr_t *offset);
+int cpu_write_elf32_note(int fd, CPUState *env, int cpuid,
+                         target_phys_addr_t *offset);

Again, some HAVE_XXX would be nicer. Maybe you put the whole block under HAVE_GUEST_CORE_DUMP or so.

Is writing to file descriptor generic enough? What if we want to dump via QMP, letting the receiver side decide about where to write it?

Quoted 16 lines

cpu-all.h +2 -0

 #else
 #define cpu_get_memory_mapping(list, env)
+#define cpu_write_elf64_note(fd, env, cpuid, offset) ({ -1; })
+#define cpu_write_elf32_note(fd, env, cpuid, offset) ({ -1; })
 #endif
 
 #endif /* CPU_ALL_H */

target-i386/arch-dump.c +0 -0

index 2e921c7..4c0ff77 100644
@@ -11,8 +11,11 @@
  *
  */
 

target-i386/arch-dump.c line 14

Quoted 1 line

target-i386/arch-dump.c +1 -0

+#include <elf.h>

Does this create a new dependency and break non-Linux hosts? Can you pull the required bits into qemu's elf.h then?

Quoted 247 lines

target-i386/arch-dump.c +237 -0

+
 #include "cpu.h"
 #include "cpu-all.h"
+#include "monitor.h"
 
 /* PAE Paging or IA-32e Paging */
 static void walk_pte(MemoryMappingList *list, target_phys_addr_t pte_start_addr,

@@ -252,3 +255,243 @@ void cpu_get_memory_mapping(MemoryMappingList *list, CPUState *env)
         walk_pde2(list, pde_addr, env->a20_mask, pse);
     }
 }
+
+#ifdef TARGET_X86_64
+typedef struct {
+    target_ulong r15, r14, r13, r12, rbp, rbx, r11, r10;
+    target_ulong r9, r8, rax, rcx, rdx, rsi, rdi, orig_rax;
+    target_ulong rip, cs, eflags;
+    target_ulong rsp, ss;
+    target_ulong fs_base, gs_base;
+    target_ulong ds, es, fs, gs;
+} x86_64_user_regs_struct;
+
+static int x86_64_write_elf64_note(int fd, CPUState *env, int id,
+                                   target_phys_addr_t *offset)
+{
+    x86_64_user_regs_struct regs;
+    Elf64_Nhdr *note;
+    char *buf;
+    int descsz, note_size, name_size = 5;
+    const char *name = "CORE";
+    int ret;
+
+    regs.r15 = env->regs[15];
+    regs.r14 = env->regs[14];
+    regs.r13 = env->regs[13];
+    regs.r12 = env->regs[12];
+    regs.r11 = env->regs[11];
+    regs.r10 = env->regs[10];
+    regs.r9  = env->regs[9];
+    regs.r8  = env->regs[8];
+    regs.rbp = env->regs[R_EBP];
+    regs.rsp = env->regs[R_ESP];
+    regs.rdi = env->regs[R_EDI];
+    regs.rsi = env->regs[R_ESI];
+    regs.rdx = env->regs[R_EDX];
+    regs.rcx = env->regs[R_ECX];
+    regs.rbx = env->regs[R_EBX];
+    regs.rax = env->regs[R_EAX];
+    regs.rip = env->eip;
+    regs.eflags = env->eflags;
+
+    regs.orig_rax = 0; /* FIXME */
+    regs.cs = env->segs[R_CS].selector;
+    regs.ss = env->segs[R_SS].selector;
+    regs.fs_base = env->segs[R_FS].base;
+    regs.gs_base = env->segs[R_GS].base;
+    regs.ds = env->segs[R_DS].selector;
+    regs.es = env->segs[R_ES].selector;
+    regs.fs = env->segs[R_FS].selector;
+    regs.gs = env->segs[R_GS].selector;
+
+    descsz = 336; /* sizeof(prstatus_t) is 336 on x86_64 box */
+    note_size = ((sizeof(Elf64_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
+                (descsz + 3) / 4) * 4;
+    note = g_malloc(note_size);
+
+    memset(note, 0, note_size);
+    note->n_namesz = cpu_to_le32(name_size);
+    note->n_descsz = cpu_to_le32(descsz);
+    note->n_type = cpu_to_le32(NT_PRSTATUS);
+    buf = (char *)note;
+    buf += ((sizeof(Elf64_Nhdr) + 3) / 4) * 4;
+    memcpy(buf, name, name_size);
+    buf += ((name_size + 3) / 4) * 4;
+    memcpy(buf + 32, &id, 4); /* pr_pid */
+    buf += descsz - sizeof(x86_64_user_regs_struct)-sizeof(target_ulong);
+    memcpy(buf, &regs, sizeof(x86_64_user_regs_struct));
+
+    lseek(fd, *offset, SEEK_SET);
+    ret = write(fd, note, note_size);
+    g_free(note);
+    if (ret < 0) {
+        return -1;
+    }
+
+    *offset += note_size;
+
+    return 0;
+}
+#endif
+
+typedef struct {
+    uint32_t ebx, ecx, edx, esi, edi, ebp, eax;
+    unsigned short ds, __ds, es, __es;
+    unsigned short fs, __fs, gs, __gs;
+    uint32_t orig_eax, eip;
+    unsigned short cs, __cs;
+    uint32_t eflags, esp;
+    unsigned short ss, __ss;
+} x86_user_regs_struct;
+
+static int x86_write_elf64_note(int fd, CPUState *env, int id,
+                                target_phys_addr_t *offset)
+{
+    x86_user_regs_struct regs;
+    Elf64_Nhdr *note;
+    char *buf;
+    int descsz, note_size, name_size = 5;
+    const char *name = "CORE";
+    int ret;
+
+    regs.ebp = env->regs[R_EBP] & 0xffffffff;
+    regs.esp = env->regs[R_ESP] & 0xffffffff;
+    regs.edi = env->regs[R_EDI] & 0xffffffff;
+    regs.esi = env->regs[R_ESI] & 0xffffffff;
+    regs.edx = env->regs[R_EDX] & 0xffffffff;
+    regs.ecx = env->regs[R_ECX] & 0xffffffff;
+    regs.ebx = env->regs[R_EBX] & 0xffffffff;
+    regs.eax = env->regs[R_EAX] & 0xffffffff;
+    regs.eip = env->eip & 0xffffffff;
+    regs.eflags = env->eflags & 0xffffffff;
+
+    regs.cs = env->segs[R_CS].selector;
+    regs.__cs = 0;
+    regs.ss = env->segs[R_SS].selector;
+    regs.__ss = 0;
+    regs.ds = env->segs[R_DS].selector;
+    regs.__ds = 0;
+    regs.es = env->segs[R_ES].selector;
+    regs.__es = 0;
+    regs.fs = env->segs[R_FS].selector;
+    regs.__fs = 0;
+    regs.gs = env->segs[R_GS].selector;
+    regs.__gs = 0;
+
+    descsz = 144; /* sizeof(prstatus_t) is 144 on x86 box */
+    note_size = ((sizeof(Elf64_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
+                (descsz + 3) / 4) * 4;
+    note = g_malloc(note_size);
+
+    memset(note, 0, note_size);
+    note->n_namesz = cpu_to_le32(name_size);
+    note->n_descsz = cpu_to_le32(descsz);
+    note->n_type = cpu_to_le32(NT_PRSTATUS);
+    buf = (char *)note;
+    buf += ((sizeof(Elf64_Nhdr) + 3) / 4) * 4;
+    memcpy(buf, name, name_size);
+    buf += ((name_size + 3) / 4) * 4;
+    memcpy(buf + 24, &id, 4); /* pr_pid */
+    buf += descsz - sizeof(x86_user_regs_struct)-4;
+    memcpy(buf, &regs, sizeof(x86_user_regs_struct));
+
+    lseek(fd, *offset, SEEK_SET);
+    ret = write(fd, note, note_size);
+    g_free(note);
+    if (ret < 0) {
+        return -1;
+    }
+
+    *offset += note_size;
+
+    return 0;
+}
+
+static int x86_write_elf32_note(int fd, CPUState *env, int id,
+                                target_phys_addr_t *offset)
+{
+    x86_user_regs_struct regs;
+    Elf32_Nhdr *note;
+    char *buf;
+    int descsz, note_size, name_size = 5;
+    const char *name = "CORE";
+    int ret;
+
+    regs.ebp = env->regs[R_EBP] & 0xffffffff;
+    regs.esp = env->regs[R_ESP] & 0xffffffff;
+    regs.edi = env->regs[R_EDI] & 0xffffffff;
+    regs.esi = env->regs[R_ESI] & 0xffffffff;
+    regs.edx = env->regs[R_EDX] & 0xffffffff;
+    regs.ecx = env->regs[R_ECX] & 0xffffffff;
+    regs.ebx = env->regs[R_EBX] & 0xffffffff;
+    regs.eax = env->regs[R_EAX] & 0xffffffff;
+    regs.eip = env->eip & 0xffffffff;
+    regs.eflags = env->eflags & 0xffffffff;
+
+    regs.cs = env->segs[R_CS].selector;
+    regs.__cs = 0;
+    regs.ss = env->segs[R_SS].selector;
+    regs.__ss = 0;
+    regs.ds = env->segs[R_DS].selector;
+    regs.__ds = 0;
+    regs.es = env->segs[R_ES].selector;
+    regs.__es = 0;
+    regs.fs = env->segs[R_FS].selector;
+    regs.__fs = 0;
+    regs.gs = env->segs[R_GS].selector;
+    regs.__gs = 0;
+
+    descsz = 144; /* sizeof(prstatus_t) is 144 on x86 box */
+    note_size = ((sizeof(Elf32_Nhdr) + 3) / 4 + (name_size + 3) / 4 +
+                (descsz + 3) / 4) * 4;
+    note = g_malloc(note_size);
+
+    memset(note, 0, note_size);
+    note->n_namesz = cpu_to_le32(name_size);
+    note->n_descsz = cpu_to_le32(descsz);
+    note->n_type = cpu_to_le32(NT_PRSTATUS);
+    buf = (char *)note;
+    buf += ((sizeof(Elf32_Nhdr) + 3) / 4) * 4;
+    memcpy(buf, name, name_size);
+    buf += ((name_size + 3) / 4) * 4;
+    memcpy(buf + 24, &id, 4); /* pr_pid */
+    buf += descsz - sizeof(x86_user_regs_struct)-4;
+    memcpy(buf, &regs, sizeof(x86_user_regs_struct));
+
+    lseek(fd, *offset, SEEK_SET);
+    ret = write(fd, note, note_size);
+    g_free(note);
+    if (ret < 0) {
+        return -1;
+    }
+
+    *offset += note_size;
+
+    return 0;
+}
+
+int cpu_write_elf64_note(int fd, CPUState *env, int cpuid,
+                         target_phys_addr_t *offset)
+{
+    int ret;
+#ifdef TARGET_X86_64
+    bool lma = !!(first_cpu->hflags & HF_LMA_MASK);
+
+    if (lma) {
+        ret = x86_64_write_elf64_note(fd, env, cpuid, offset);
+    } else {
+#endif
+        ret = x86_write_elf64_note(fd, env, cpuid, offset);
+#ifdef TARGET_X86_64
+    }
+#endif
+
+    return ret;
+}
+

target-i386/arch-dump.c void cpu_get_memory_mapping(MemoryMappingList *list, CPUState *env) · line 497

Quoted 5 lines

target-i386/arch-dump.c +5 -0

+int cpu_write_elf32_note(int fd, CPUState *env, int cpuid,
+                         target_phys_addr_t *offset)
+{
+    return x86_write_elf32_note(fd, env, cpuid, offset);
+}

Minor nit: I think this wrapping is not needed, just fold x86_write_elf32_note into this function.

Jan

Crash needs extra memory mapping to determine phys_base.

Signed-off-by Wen Congyang <[email protected]>

---
 cpu-all.h               |    2 ++
 target-i386/arch-dump.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 0 deletions(-)

cpu-all.h +2 -0

index efb5ba3..290c43a 100644
@@ -530,10 +530,12 @@ int cpu_write_elf64_note(int fd, CPUState *env, int cpuid,
                          target_phys_addr_t *offset);
 int cpu_write_elf32_note(int fd, CPUState *env, int cpuid,
                          target_phys_addr_t *offset);
+int cpu_add_extra_memory_mapping(MemoryMappingList *list);
 #else
 #define cpu_get_memory_mapping(list, env)
 #define cpu_write_elf64_note(fd, env, cpuid, offset) ({ -1; })
 #define cpu_write_elf32_note(fd, env, cpuid, offset) ({ -1; })
+#define cpu_add_extra_memory_mapping(list) ({ 0; })
 #endif
 
 #endif /* CPU_ALL_H */

target-i386/arch-dump.c +0 -0

index 4c0ff77..d96f6ae 100644

target-i386/arch-dump.c +11 -0

+static target_ulong get_phys_base_addr(CPUState *env, target_ulong *base_vaddr)
+{
+    int i;
+    target_ulong kernel_base = -1;
+    target_ulong last, mask;
+
+    for (i = 30, last = -1; (kernel_base == -1) && (i >= 20); i--) {
+        mask = ~((1LL << i) - 1);
+        *base_vaddr = env->idt.base & mask;
+        if (*base_vaddr == last) {
+            continue;

target-i386/arch-dump.c +10 -0

+{
+#ifdef TARGET_X86_64
+    target_phys_addr_t kernel_base = -1;
+    target_ulong base_vaddr;
+    bool lma = !!(first_cpu->hflags & HF_LMA_MASK);
+
+    if (!lma) {
+        return 0;
+    }
+

Quoted 30 lines

Crash needs extra memory mapping to determine phys_base.

Signed-off-by Wen Congyang <[email protected]>

---
 cpu-all.h               |    2 ++
 target-i386/arch-dump.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 0 deletions(-)

cpu-all.h +2 -0

index efb5ba3..290c43a 100644
@@ -530,10 +530,12 @@ int cpu_write_elf64_note(int fd, CPUState *env, int cpuid,
                          target_phys_addr_t *offset);
 int cpu_write_elf32_note(int fd, CPUState *env, int cpuid,
                          target_phys_addr_t *offset);
+int cpu_add_extra_memory_mapping(MemoryMappingList *list);
 #else
 #define cpu_get_memory_mapping(list, env)
 #define cpu_write_elf64_note(fd, env, cpuid, offset) ({ -1; })
 #define cpu_write_elf32_note(fd, env, cpuid, offset) ({ -1; })
+#define cpu_add_extra_memory_mapping(list) ({ 0; })
 #endif
 
 #endif /* CPU_ALL_H */

target-i386/arch-dump.c +0 -0

index 4c0ff77..d96f6ae 100644

target-i386/arch-dump.c int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, · line 499

Quoted 5 lines

target-i386/arch-dump.c +2 -0

@@ -495,3 +495,46 @@ int cpu_write_elf32_note(int fd, CPUState *env, int cpuid,
 {
     return x86_write_elf32_note(fd, env, cpuid, offset);
 }
+
+/* This function is copied from crash */

And what does it do there and here? I suppose it is Linux-specific - any version? This should be documented and encoded in the function name.

Quoted 15 lines

target-i386/arch-dump.c +15 -0

+static target_ulong get_phys_base_addr(CPUState *env, target_ulong *base_vaddr)
+{
+    int i;
+    target_ulong kernel_base = -1;
+    target_ulong last, mask;
+
+    for (i = 30, last = -1; (kernel_base == -1) && (i >= 20); i--) {
+        mask = ~((1LL << i) - 1);
+        *base_vaddr = env->idt.base & mask;
+        if (*base_vaddr == last) {
+            continue;
+        }
+
+        kernel_base = cpu_get_phys_page_debug(env, *base_vaddr);
+        last = *base_vaddr;

target-i386/arch-dump.c int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, · line 520

Quoted 6 lines

target-i386/arch-dump.c +6 -0

+    }
+
+    return kernel_base;
+}
+
+int cpu_add_extra_memory_mapping(MemoryMappingList *list)

Again, what does "extra" mean? Probably guest-specific, no?

Quoted 11 lines

target-i386/arch-dump.c +10 -0

+{
+#ifdef TARGET_X86_64
+    target_phys_addr_t kernel_base = -1;
+    target_ulong base_vaddr;
+    bool lma = !!(first_cpu->hflags & HF_LMA_MASK);
+
+    if (!lma) {
+        return 0;
+    }
+

target-i386/arch-dump.c int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, · line 540

Quoted 10 lines

target-i386/arch-dump.c +10 -0

+    kernel_base = get_phys_base_addr(first_cpu, &base_vaddr);
+    if (kernel_base == -1) {
+        return -1;
+    }
+
+    create_new_memory_mapping_head(list, kernel_base, base_vaddr,
+                                   TARGET_PAGE_SIZE);
+#endif
+    return 0;
+}

Jan

At 02/15/2012 01:35 AM, Jan Kiszka Wrote:

Quoted 55 lines · Wen Congyang

Quoted 30 lines

Crash needs extra memory mapping to determine phys_base.

Signed-off-by Wen Congyang <[email protected]>

---
 cpu-all.h               |    2 ++
 target-i386/arch-dump.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 0 deletions(-)

cpu-all.h +2 -0

index efb5ba3..290c43a 100644
@@ -530,10 +530,12 @@ int cpu_write_elf64_note(int fd, CPUState *env, int cpuid,
                          target_phys_addr_t *offset);
 int cpu_write_elf32_note(int fd, CPUState *env, int cpuid,
                          target_phys_addr_t *offset);
+int cpu_add_extra_memory_mapping(MemoryMappingList *list);
 #else
 #define cpu_get_memory_mapping(list, env)
 #define cpu_write_elf64_note(fd, env, cpuid, offset) ({ -1; })
 #define cpu_write_elf32_note(fd, env, cpuid, offset) ({ -1; })
+#define cpu_add_extra_memory_mapping(list) ({ 0; })
 #endif
 
 #endif /* CPU_ALL_H */

target-i386/arch-dump.c +0 -0

index 4c0ff77..d96f6ae 100644

target-i386/arch-dump.c int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, · line 499

Quoted 5 lines

target-i386/arch-dump.c +2 -0

@@ -495,3 +495,46 @@ int cpu_write_elf32_note(int fd, CPUState *env, int cpuid,
 {
     return x86_write_elf32_note(fd, env, cpuid, offset);
 }
+
+/* This function is copied from crash */

And what does it do there and here? I suppose it is Linux-specific - any version? This should be documented and encoded in the function name.

Quoted 15 lines

target-i386/arch-dump.c +15 -0

+static target_ulong get_phys_base_addr(CPUState *env, target_ulong *base_vaddr)
+{
+    int i;
+    target_ulong kernel_base = -1;
+    target_ulong last, mask;
+
+    for (i = 30, last = -1; (kernel_base == -1) && (i >= 20); i--) {
+        mask = ~((1LL << i) - 1);
+        *base_vaddr = env->idt.base & mask;
+        if (*base_vaddr == last) {
+            continue;
+        }
+
+        kernel_base = cpu_get_phys_page_debug(env, *base_vaddr);
+        last = *base_vaddr;

Quoted 8 lines

target-i386/arch-dump.c int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, · line 520

Quoted 6 lines

target-i386/arch-dump.c +6 -0

+    }
+
+    return kernel_base;
+}
+
+int cpu_add_extra_memory_mapping(MemoryMappingList *list)

Again, what does "extra" mean? Probably guest-specific, no?

crash will calculate the phys_base according to the virtual address and physical address stored in the PT_LOAD.

If the vmcore is generated by 'virsh dump'(use migration to implement dumping), crash calculates the phys_base according to idt.base. The function get_phys_base_addr() uses the same way to calculates the phys_base.

I think crash may work without this. I will verify it.

Quoted 65 lines

At 02/15/2012 01:35 AM, Jan Kiszka Wrote:

Quoted 55 lines · Wen Congyang

Quoted 30 lines

Crash needs extra memory mapping to determine phys_base.

Signed-off-by Wen Congyang <[email protected]>

---
 cpu-all.h               |    2 ++
 target-i386/arch-dump.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 0 deletions(-)

cpu-all.h +2 -0

index efb5ba3..290c43a 100644
@@ -530,10 +530,12 @@ int cpu_write_elf64_note(int fd, CPUState *env, int cpuid,
                          target_phys_addr_t *offset);
 int cpu_write_elf32_note(int fd, CPUState *env, int cpuid,
                          target_phys_addr_t *offset);
+int cpu_add_extra_memory_mapping(MemoryMappingList *list);
 #else
 #define cpu_get_memory_mapping(list, env)
 #define cpu_write_elf64_note(fd, env, cpuid, offset) ({ -1; })
 #define cpu_write_elf32_note(fd, env, cpuid, offset) ({ -1; })
+#define cpu_add_extra_memory_mapping(list) ({ 0; })
 #endif
 
 #endif /* CPU_ALL_H */

target-i386/arch-dump.c +0 -0

index 4c0ff77..d96f6ae 100644

target-i386/arch-dump.c int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, · line 499

Quoted 5 lines

target-i386/arch-dump.c +2 -0

@@ -495,3 +495,46 @@ int cpu_write_elf32_note(int fd, CPUState *env, int cpuid,
 {
     return x86_write_elf32_note(fd, env, cpuid, offset);
 }
+
+/* This function is copied from crash */

And what does it do there and here? I suppose it is Linux-specific - any version? This should be documented and encoded in the function name.

Quoted 15 lines

target-i386/arch-dump.c +15 -0

+static target_ulong get_phys_base_addr(CPUState *env, target_ulong *base_vaddr)
+{
+    int i;
+    target_ulong kernel_base = -1;
+    target_ulong last, mask;
+
+    for (i = 30, last = -1; (kernel_base == -1) && (i >= 20); i--) {
+        mask = ~((1LL << i) - 1);
+        *base_vaddr = env->idt.base & mask;
+        if (*base_vaddr == last) {
+            continue;
+        }
+
+        kernel_base = cpu_get_phys_page_debug(env, *base_vaddr);
+        last = *base_vaddr;

Quoted 8 lines

target-i386/arch-dump.c int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, · line 520

Quoted 6 lines

target-i386/arch-dump.c +6 -0

+    }
+
+    return kernel_base;
+}
+
+int cpu_add_extra_memory_mapping(MemoryMappingList *list)

Again, what does "extra" mean? Probably guest-specific, no?

Quoted 2 lines

crash will calculate the phys_base according to the virtual address and physical address stored in the PT_LOAD.

Crash is a Linux-only tool, dump must not be restricted to that guest - but could contain transparent extensions of the file format if needed.

Quoted 1 line

Quoted 3 lines

If the vmcore is generated by 'virsh dump'(use migration to implement dumping), crash calculates the phys_base according to idt.base. The function get_phys_base_addr() uses the same way to calculates the phys_base.

Hmm, where are those special registers (idt, gdt, tr etc.) stored in the vmcore file, BTW?

Quoted 3 lines

I think crash may work without this. I will verify it.

Does gdb require this?

Jan

At 02/15/2012 05:21 PM, Jan Kiszka Wrote:

Quoted 77 lines · Wen Congyang

Quoted 65 lines

At 02/15/2012 01:35 AM, Jan Kiszka Wrote:

Quoted 55 lines · Wen Congyang

Quoted 30 lines

Crash needs extra memory mapping to determine phys_base.

Signed-off-by Wen Congyang <[email protected]>

---
 cpu-all.h               |    2 ++
 target-i386/arch-dump.c |   43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 0 deletions(-)

cpu-all.h +2 -0

index efb5ba3..290c43a 100644
@@ -530,10 +530,12 @@ int cpu_write_elf64_note(int fd, CPUState *env, int cpuid,
                          target_phys_addr_t *offset);
 int cpu_write_elf32_note(int fd, CPUState *env, int cpuid,
                          target_phys_addr_t *offset);
+int cpu_add_extra_memory_mapping(MemoryMappingList *list);
 #else
 #define cpu_get_memory_mapping(list, env)
 #define cpu_write_elf64_note(fd, env, cpuid, offset) ({ -1; })
 #define cpu_write_elf32_note(fd, env, cpuid, offset) ({ -1; })
+#define cpu_add_extra_memory_mapping(list) ({ 0; })
 #endif
 
 #endif /* CPU_ALL_H */

target-i386/arch-dump.c +0 -0

index 4c0ff77..d96f6ae 100644

target-i386/arch-dump.c int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, · line 499

Quoted 5 lines

target-i386/arch-dump.c +2 -0

@@ -495,3 +495,46 @@ int cpu_write_elf32_note(int fd, CPUState *env, int cpuid,
 {
     return x86_write_elf32_note(fd, env, cpuid, offset);
 }
+
+/* This function is copied from crash */

And what does it do there and here? I suppose it is Linux-specific - any version? This should be documented and encoded in the function name.

Quoted 15 lines

target-i386/arch-dump.c +15 -0

+static target_ulong get_phys_base_addr(CPUState *env, target_ulong *base_vaddr)
+{
+    int i;
+    target_ulong kernel_base = -1;
+    target_ulong last, mask;
+
+    for (i = 30, last = -1; (kernel_base == -1) && (i >= 20); i--) {
+        mask = ~((1LL << i) - 1);
+        *base_vaddr = env->idt.base & mask;
+        if (*base_vaddr == last) {
+            continue;
+        }
+
+        kernel_base = cpu_get_phys_page_debug(env, *base_vaddr);
+        last = *base_vaddr;

Quoted 8 lines

target-i386/arch-dump.c int cpu_write_elf32_note(int fd, CPUState *env, int cpuid, · line 520

Quoted 6 lines

target-i386/arch-dump.c +6 -0

+    }
+
+    return kernel_base;
+}
+
+int cpu_add_extra_memory_mapping(MemoryMappingList *list)

Again, what does "extra" mean? Probably guest-specific, no?

Quoted 2 lines

crash will calculate the phys_base according to the virtual address and physical address stored in the PT_LOAD.

Crash is a Linux-only tool, dump must not be restricted to that guest - but could contain transparent extensions of the file format if needed.

Quoted 4 lines

If the vmcore is generated by 'virsh dump'(use migration to implement dumping), crash calculates the phys_base according to idt.base. The function get_phys_base_addr() uses the same way to calculates the phys_base.