[Qemu-devel] [PATCH 0/2] s390: dump-guest-memory support

[Qemu-devel] [PATCH 0/2] s390: dump-guest-memory support

[Jens Freimann]

The first patch splits memory mapping code to allow architectures
that don't support/use paging to implement dump-guest-memory support.

Based on this, the second patch implements support for s390x.


Ekaterina Tumanova (2):
  Split out dump-guest-memory memory mapping code
  s390: dump guest memory implementation

 Makefile.target                 |   2 +-
 configure                       |   2 +-
 dump.c                          |  11 +-
 include/elf.h                   |   6 ++
 include/qapi/qmp/qerror.h       |   3 +
 include/sysemu/dump.h           |   3 +
 include/sysemu/memory_mapping.h |  12 +++
 memory_mapping-stub.c           |  11 +-
 memory_mapping.c                |  84 +--------------
 memory_mapping_common.c         | 104 ++++++++++++++++++
 target-s390x/Makefile.objs      |   2 +-
 target-s390x/arch_dump.c        | 231 ++++++++++++++++++++++++++++++++++++++++
 target-s390x/cpu-qom.h          |  21 ++++
 target-s390x/cpu.c              |   7 ++
 14 files changed, 404 insertions(+), 95 deletions(-)
 create mode 100644 memory_mapping_common.c
 create mode 100644 target-s390x/arch_dump.c

-- 
1.7.12.4

[Qemu-devel] [PATCH 1/2] Split out dump-guest-memory memory mapping code

[Jens Freimann]

Split out dump-guest-memory memory mapping code to allow dumping without
memory mapping

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

Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 Makefile.target                 |   2 +-
 dump.c                          |  11 ++++-
 include/qapi/qmp/qerror.h       |   3 ++
 include/sysemu/dump.h           |   3 ++
 include/sysemu/memory_mapping.h |  12 +++++
 memory_mapping-stub.c           |  11 +----
 memory_mapping.c                |  84 +-------------------------------
 memory_mapping_common.c         | 104 ++++++++++++++++++++++++++++++++++++++++
 8 files changed, 137 insertions(+), 93 deletions(-)
 create mode 100644 memory_mapping_common.c

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

Re: [Qemu-devel] [PATCH 1/2] Split out dump-guest-memory memory mapping code

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1771 bytes --]

On 04/23/2013 09:30 AM, Jens Freimann wrote:
> Split out dump-guest-memory memory mapping code to allow dumping without
> memory mapping
> 
> The qemu dump.c code currently requires CONFIG_HAVE_CORE_DUMP as well as
> CONFIG_HAVE_GET_MEMORY_MAPPING. This allows for dumping with and without paging.
> Some architectures will provide only the non-paging case. This patch allows an
> architecture to provide dumping even when CONFIG_HAVE_GET_MEMORY_MAPPING is not
> available. To do that, we split out the common code and provide stub functions
> for the non-paging case. If -p is specified on a target that doesn't support it,
> we will pass an error to the calling code.
> 
> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> ---

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

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

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

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH 1/2] Split out dump-guest-memory memory mapping code

[Jens Freimann]

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

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

sounds reasonable to me. 


regards
Jens 
> -- 
> Eric Blake   eblake redhat com    +1-919-301-3266
> Libvirt virtualization library http://libvirt.org
> 

Re: [Qemu-devel] [PATCH 1/2] Split out dump-guest-memory memory mapping code

[Andreas Färber]

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

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

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

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

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

Regards,
Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

Re: [Qemu-devel] [PATCH 1/2] Split out dump-guest-memory memory mapping code

[Luiz Capitulino]

On Tue, 23 Apr 2013 09:41:43 -0600
Eric Blake <eblake@redhat.com> wrote:

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

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

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

Re: [Qemu-devel] [PATCH 1/2] Split out dump-guest-memory memory mapping code

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2011 bytes --]

On 04/24/2013 09:50 AM, Luiz Capitulino wrote:
>> This raises an interesting question about introspection - how will
>> management apps (such as libvirt) be able to determine whether the
>> paging command is supported for a given architecture?  Do we need to
>> expand the 'MachineInfo' QMP datatype so that 'query-machines' can tell
>> us whether a given machine will support or reject attempts to set
>> 'paging':true during 'dump-guest-memory'?
> 
> Is libvirt going to query this for the automatic dump feature?

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

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

Introspection can't hurt, even if libvirt doesn't use it right away.  It
may be the sort of thing where we commit the initial capability with
try-and-fail handling, then down the road, decide whether the error
quality is good enough; it also seems like introspection is the sort of
thing that is easy to backport, even if the introspection does not go in
at the same time as the original feature, where improved error messages
becomes a quality-of-implementation value-added by distro packagers.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

Re: [Qemu-devel] [PATCH 1/2] Split out dump-guest-memory memory mapping code

[Luiz Capitulino]

On Wed, 24 Apr 2013 10:23:58 -0600
Eric Blake <eblake@redhat.com> wrote:

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

Fair enough.

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

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

I'd prefer query-dump-guest-memory-capabilities myself, although I'm unsure
if the proliferation of such commands is a good thing.

Re: [Qemu-devel] [PATCH 1/2] Split out dump-guest-memory memory mapping code

[Ekaterina Tumanova]

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

Re: [Qemu-devel] [PATCH 1/2] Split out dump-guest-memory memory mapping code

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2273 bytes --]

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

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

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

    qemu: drop unused arguments for dump-guest-memory

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

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

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

[Is it a bad sign when I can remember that libvirt USED to partially
support the paging flag, but not that _I_ was the one that ripped it out
because the public API never supported it?]

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]

[Qemu-devel] [RFC qom-cpu] dump: Unconditionally compile

[Andreas Färber]

qmp_dump_guest_memory() calls dump_init() and returns an Error when
cpu_get_dump_info() returns an error, as done by the stub.
So there is no need to have a stub for qmp_dump_guest_memory().

Enable the documentation of the always-present dump-guest-memory command.

That way we can drop CONFIG_HAVE_CORE_DUMP.

Signed-off-by: Andreas Färber <afaerber@suse.de>
---
 Based on Ekaterina's last patch and my stub cleanup.
 Next step will then be to clean up memory_mapping-stub.c.

 Makefile.target | 2 +-
 configure       | 4 ----
 hmp-commands.hx | 2 --
 stubs/dump.c    | 8 --------
 4 files changed, 1 insertion(+), 15 deletions(-)

diff --git a/Makefile.target b/Makefile.target
index 84ec344..5b02200 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -111,7 +111,7 @@ obj-$(CONFIG_FDT) += device_tree.o
 obj-$(CONFIG_KVM) += kvm-all.o
 obj-y += memory.o savevm.o cputlb.o
 obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
-obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o memory_mapping_common.o
+obj-y += dump.o memory_mapping_common.o
 obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
 LIBS+=$(libs_softmmu)
 
diff --git a/configure b/configure
index 5ae7e4a..8f987e8 100755
--- a/configure
+++ b/configure
@@ -4349,10 +4349,6 @@ if test "$target_bigendian" = "yes" ; then
 fi
 if test "$target_softmmu" = "yes" ; then
   echo "CONFIG_SOFTMMU=y" >> $config_target_mak
-  case "$target_arch2" in
-    i386|x86_64)
-      echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak
-  esac
 fi
 if test "$target_user_only" = "yes" ; then
   echo "CONFIG_USER_ONLY=y" >> $config_target_mak
diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9cea415..074dbe1 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -989,7 +989,6 @@ server will ask the spice/vnc client to automatically reconnect using the
 new parameters (if specified) once the vm migration finished successfully.
 ETEXI
 
-#if defined(CONFIG_HAVE_CORE_DUMP)
     {
         .name       = "dump-guest-memory",
         .args_type  = "paging:-p,filename:F,begin:i?,length:i?",
@@ -1013,7 +1012,6 @@ gdb.
     length: the memory size, in bytes. It's optional, and should be specified
             with begin together.
 ETEXI
-#endif
 
     {
         .name       = "snapshot_blkdev",
diff --git a/stubs/dump.c b/stubs/dump.c
index b3f42cb..43c9a3f 100644
--- a/stubs/dump.c
+++ b/stubs/dump.c
@@ -16,14 +16,6 @@
 #include "qapi/qmp/qerror.h"
 #include "qmp-commands.h"
 
-/* we need this function in hmp.c */
-void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
-                           int64_t begin, bool has_length, int64_t length,
-                           Error **errp)
-{
-    error_set(errp, QERR_UNSUPPORTED);
-}
-
 int cpu_get_dump_info(ArchDumpInfo *info)
 {
     return -1;
-- 
1.8.1.4

Re: [Qemu-devel] [RFC qom-cpu] dump: Unconditionally compile

[Paolo Bonzini]

Il 17/05/2013 12:58, Andreas Färber ha scritto:
> qmp_dump_guest_memory() calls dump_init() and returns an Error when
> cpu_get_dump_info() returns an error, as done by the stub.
> So there is no need to have a stub for qmp_dump_guest_memory().
> 
> Enable the documentation of the always-present dump-guest-memory command.
> 
> That way we can drop CONFIG_HAVE_CORE_DUMP.

Nicer. :)

Paolo


> Signed-off-by: Andreas Färber <afaerber@suse.de>
> ---
>  Based on Ekaterina's last patch and my stub cleanup.
>  Next step will then be to clean up memory_mapping-stub.c.
> 
>  Makefile.target | 2 +-
>  configure       | 4 ----
>  hmp-commands.hx | 2 --
>  stubs/dump.c    | 8 --------
>  4 files changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/Makefile.target b/Makefile.target
> index 84ec344..5b02200 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -111,7 +111,7 @@ obj-$(CONFIG_FDT) += device_tree.o
>  obj-$(CONFIG_KVM) += kvm-all.o
>  obj-y += memory.o savevm.o cputlb.o
>  obj-$(CONFIG_HAVE_GET_MEMORY_MAPPING) += memory_mapping.o
> -obj-$(CONFIG_HAVE_CORE_DUMP) += dump.o memory_mapping_common.o
> +obj-y += dump.o memory_mapping_common.o
>  obj-$(CONFIG_NO_GET_MEMORY_MAPPING) += memory_mapping-stub.o
>  LIBS+=$(libs_softmmu)
>  
> diff --git a/configure b/configure
> index 5ae7e4a..8f987e8 100755
> --- a/configure
> +++ b/configure
> @@ -4349,10 +4349,6 @@ if test "$target_bigendian" = "yes" ; then
>  fi
>  if test "$target_softmmu" = "yes" ; then
>    echo "CONFIG_SOFTMMU=y" >> $config_target_mak
> -  case "$target_arch2" in
> -    i386|x86_64)
> -      echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak
> -  esac
>  fi
>  if test "$target_user_only" = "yes" ; then
>    echo "CONFIG_USER_ONLY=y" >> $config_target_mak
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 9cea415..074dbe1 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -989,7 +989,6 @@ server will ask the spice/vnc client to automatically reconnect using the
>  new parameters (if specified) once the vm migration finished successfully.
>  ETEXI
>  
> -#if defined(CONFIG_HAVE_CORE_DUMP)
>      {
>          .name       = "dump-guest-memory",
>          .args_type  = "paging:-p,filename:F,begin:i?,length:i?",
> @@ -1013,7 +1012,6 @@ gdb.
>      length: the memory size, in bytes. It's optional, and should be specified
>              with begin together.
>  ETEXI
> -#endif
>  
>      {
>          .name       = "snapshot_blkdev",
> diff --git a/stubs/dump.c b/stubs/dump.c
> index b3f42cb..43c9a3f 100644
> --- a/stubs/dump.c
> +++ b/stubs/dump.c
> @@ -16,14 +16,6 @@
>  #include "qapi/qmp/qerror.h"
>  #include "qmp-commands.h"
>  
> -/* we need this function in hmp.c */
> -void qmp_dump_guest_memory(bool paging, const char *file, bool has_begin,
> -                           int64_t begin, bool has_length, int64_t length,
> -                           Error **errp)
> -{
> -    error_set(errp, QERR_UNSUPPORTED);
> -}
> -
>  int cpu_get_dump_info(ArchDumpInfo *info)
>  {
>      return -1;
> 

Re: [Qemu-devel] [RFC qom-cpu] dump: Unconditionally compile

[Andreas Färber]

Am 17.05.2013 13:12, schrieb Paolo Bonzini:
> Il 17/05/2013 12:58, Andreas Färber ha scritto:
>> qmp_dump_guest_memory() calls dump_init() and returns an Error when
>> cpu_get_dump_info() returns an error, as done by the stub.
>> So there is no need to have a stub for qmp_dump_guest_memory().
>>
>> Enable the documentation of the always-present dump-guest-memory command.
>>
>> That way we can drop CONFIG_HAVE_CORE_DUMP.
> 
> Nicer. :)

Thanks, applied to qom-cpu:
https://github.com/afaerber/qemu-cpu/commits/qom-cpu

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg

[Qemu-devel] [PATCH 2/2] s390: dump guest memory implementation

[Jens Freimann]

Implement dump-guest-memory support for target s390x.

dump-guest-memory QEMU monitor command didn't work for s390 architecture.
The result of the command is supposed to be ELF format crash-readable
dump.
In order to implement this, the arch-specific part of dump-guest-memory
was added:
target-s390x/arch_dump.c contains the whole set of function for writing
Elf note sections of all types for s390x.

Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
---
 configure                  |   2 +-
 include/elf.h              |   6 ++
 target-s390x/Makefile.objs |   2 +-
 target-s390x/arch_dump.c   | 231 +++++++++++++++++++++++++++++++++++++++++++++
 target-s390x/cpu-qom.h     |  21 +++++
 target-s390x/cpu.c         |   7 ++
 6 files changed, 267 insertions(+), 2 deletions(-)
 create mode 100644 target-s390x/arch_dump.c

diff --git a/configure b/configure
index ed49f91..90dc58b 100755
--- a/configure
+++ b/configure
@@ -4326,7 +4326,7 @@ fi
 if test "$target_softmmu" = "yes" ; then
   echo "CONFIG_SOFTMMU=y" >> $config_target_mak
   case "$target_arch2" in
-    i386|x86_64)
+    i386|x86_64|s390x)
       echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak
   esac
 fi
diff --git a/include/elf.h b/include/elf.h
index a21ea53..ba4b3a7 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -1219,11 +1219,17 @@ typedef struct elf64_shdr {
 
 /* Notes used in ET_CORE */
 #define NT_PRSTATUS	1
+#define NT_FPREGSET     2
 #define NT_PRFPREG	2
 #define NT_PRPSINFO	3
 #define NT_TASKSTRUCT	4
 #define NT_AUXV		6
 #define NT_PRXFPREG     0x46e62b7f      /* copied from gdb5.1/include/elf/common.h */
+#define NT_S390_PREFIX  0x305           /* s390 prefix register */
+#define NT_S390_CTRS    0x304           /* s390 control registers */
+#define NT_S390_TODPREG 0x303           /* s390 TOD programmable register */
+#define NT_S390_TODCMP  0x302           /* s390 TOD clock comparator register */
+#define NT_S390_TIMER   0x301           /* s390 timer register */
 
 
 /* Note header in a PT_NOTE section */
diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
index 4e63417..c34f654 100644
--- a/target-s390x/Makefile.objs
+++ b/target-s390x/Makefile.objs
@@ -1,4 +1,4 @@
 obj-y += translate.o helper.o cpu.o interrupt.o
 obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
-obj-$(CONFIG_SOFTMMU) += ioinst.o
+obj-$(CONFIG_SOFTMMU) += ioinst.o arch_dump.o
 obj-$(CONFIG_KVM) += kvm.o
diff --git a/target-s390x/arch_dump.c b/target-s390x/arch_dump.c
new file mode 100644
index 0000000..f908257
--- /dev/null
+++ b/target-s390x/arch_dump.c
@@ -0,0 +1,231 @@
+/*
+ * writing ELF notes for s390x arch
+ *
+ *
+ * Copyright IBM Corp. 2012
+ *
+ *     Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#include "cpu.h"
+#include "elf.h"
+#include "exec/cpu-all.h"
+#include "sysemu/dump.h"
+#include "sysemu/kvm.h"
+
+
+struct s390x_user_regs_struct {
+    uint64_t        psw[2];
+    uint64_t        gprs[16];
+    uint32_t        acrs[16];
+} QEMU_PACKED;
+
+typedef struct s390x_user_regs_struct s390x_user_regs;
+
+struct s390x_elf_prstatus_struct {
+    uint8_t pad1[32];
+    uint32_t pid;
+    uint8_t pad2[76];
+    s390x_user_regs regs;
+    uint8_t pad3[16];
+} QEMU_PACKED;
+
+typedef struct s390x_elf_prstatus_struct s390x_elf_prstatus;
+
+struct s390x_elf_fpregset_struct {
+        uint32_t        fpc;
+        uint32_t        pad;
+        uint64_t        fprs[16];
+} QEMU_PACKED;
+
+typedef struct s390x_elf_fpregset_struct s390x_elf_fpregset;
+
+    typedef struct note_struct {
+        Elf64_Nhdr hdr;
+        char name[5];
+        char pad3[3];
+        union {
+            s390x_elf_prstatus prstatus;
+            s390x_elf_fpregset fpregset;
+            uint32_t prefix;
+            uint64_t timer;
+            uint64_t todcmp;
+            uint32_t todpreg;
+            uint64_t ctrs[16];
+        } contents;
+    } QEMU_PACKED note_t;
+
+static int s390x_write_elf64_prstatus(note_t *note, CPUArchState *env)
+{
+    int i;
+    s390x_user_regs *regs;
+
+    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
+
+    regs = &(note->contents.prstatus.regs);
+    regs->psw[0] =  cpu_to_be32(env->psw.mask);
+    regs->psw[1] =  cpu_to_be32(env->psw.addr);
+    for (i = 0; i <= 15; i++) {
+        regs->acrs[i] = cpu_to_be32(env->aregs[i]);
+        regs->gprs[i] = cpu_to_be32(env->regs[i]);
+    }
+
+    return 0;
+}
+
+static int s390x_write_elf64_fpregset(note_t *note, CPUArchState *env)
+{
+    int i;
+
+    note->hdr.n_type = cpu_to_be32(NT_FPREGSET);
+
+    note->contents.fpregset.fpc = cpu_to_be32(env->fpc);
+    for (i = 0; i <= 15; i++) {
+        note->contents.fpregset.fprs[i] =  cpu_to_be64(env->fregs[i].ll);
+    }
+
+    return 0;
+}
+
+
+static int s390x_write_elf64_timer(note_t *note, CPUArchState *env)
+{
+    note->hdr.n_type = cpu_to_be32(NT_S390_TIMER);
+
+    note->contents.timer = cpu_to_be64((uint64_t)(env->cputm));
+
+    return 0;
+}
+
+static int s390x_write_elf64_todcmp(note_t *note, CPUArchState *env)
+{
+    note->hdr.n_type = cpu_to_be32(NT_S390_TODCMP);
+
+    note->contents.todcmp = cpu_to_be64((uint64_t)(env->ckc));
+
+    return 0;
+}
+
+static int s390x_write_elf64_todpreg(note_t *note, CPUArchState *env)
+{
+    note->hdr.n_type = cpu_to_be32(NT_S390_TODPREG);
+
+    note->contents.todpreg = cpu_to_be32((uint32_t)(env->todpr));
+
+    return 0;
+}
+
+static int s390x_write_elf64_ctrs(note_t *note, CPUArchState *env)
+{
+    int i;
+
+    note->hdr.n_type = cpu_to_be32(NT_S390_CTRS);
+
+    for (i = 0; i <= 15; i++) {
+        note->contents.ctrs[i] =  cpu_to_be32(env->cregs[i]);
+    }
+
+    return 0;
+}
+
+static int s390x_write_elf64_prefix(note_t *note, CPUArchState *env)
+{
+    note->hdr.n_type = cpu_to_be32(NT_S390_PREFIX);
+
+    note->contents.prefix = cpu_to_be32((uint32_t)(env->psa));
+
+    return 0;
+}
+
+
+struct note_func_desc_struct {
+    int contents_size;
+    int (*note_contents_func)(note_t *note, CPUArchState *env);
+} note_func[] = {
+    { sizeof(((note_t *)0)->contents.prstatus), s390x_write_elf64_prstatus },
+    { sizeof(((note_t *)0)->contents.prefix),   s390x_write_elf64_prefix },
+    { sizeof(((note_t *)0)->contents.fpregset), s390x_write_elf64_fpregset },
+    { sizeof(((note_t *)0)->contents.ctrs),     s390x_write_elf64_ctrs },
+    { sizeof(((note_t *)0)->contents.timer),    s390x_write_elf64_timer },
+    { sizeof(((note_t *)0)->contents.todcmp),   s390x_write_elf64_todcmp },
+    { sizeof(((note_t *)0)->contents.todpreg),  s390x_write_elf64_todpreg },
+
+    { 0, NULL}
+};
+
+
+static int s390x_write_all_elf64_notes(const char *note_name,
+            WriteCoreDumpFunction f,
+            CPUArchState *env,
+            int id,
+            void *opaque)
+{
+    note_t note;
+    struct note_func_desc_struct *nf;
+    int note_size;
+    int ret = -1;
+
+    for (nf = note_func; nf->note_contents_func; nf++) {
+        note.hdr.n_namesz = cpu_to_be32(sizeof(note.name));
+        note.hdr.n_descsz = cpu_to_be32(nf->contents_size);
+        strncpy(note.name, note_name, sizeof(note.name));
+        ret = (*nf->note_contents_func)(&note, env);
+
+        note_size = cpu_to_be32(sizeof(note) -
+                sizeof(note.contents) +
+                nf->contents_size);
+        ret = f(&note, note_size, opaque);
+
+        if (ret < 0) {
+            return -1;
+        }
+
+    }
+
+    return 0;
+}
+
+
+int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
+                         int cpuid, void *opaque)
+{
+    S390CPU *cpu = S390_CPU(cs);
+    return s390x_write_all_elf64_notes("CORE", f, &cpu->env, cpuid, opaque);
+}
+
+int cpu_get_dump_info(ArchDumpInfo *info)
+{
+    info->d_machine = EM_S390;
+    info->d_endian = ELFDATA2MSB;
+    info->d_class = ELFCLASS64;
+
+    return 0;
+}
+
+ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
+{
+    int name_size = 8; /* "CORE" or "QEMU" rounded*/
+    size_t elf_note_size = 0;
+    int note_head_size;
+    struct note_func_desc_struct *nf;
+
+    assert(class == ELFCLASS64);
+    note_head_size = sizeof(Elf64_Nhdr);
+
+    assert(machine == EM_S390);
+
+    for (nf = note_func; nf->note_contents_func; nf++) {
+        elf_note_size = elf_note_size +
+            note_head_size +
+            name_size +
+            nf->contents_size;
+    }
+
+    return (elf_note_size) * nr_cpus;
+
+}
+
diff --git a/target-s390x/cpu-qom.h b/target-s390x/cpu-qom.h
index 34d45c2..3370fb9 100644
--- a/target-s390x/cpu-qom.h
+++ b/target-s390x/cpu-qom.h
@@ -73,4 +73,25 @@ static inline S390CPU *s390_env_get_cpu(CPUS390XState *env)
 
 void s390_cpu_do_interrupt(CPUState *cpu);
 
+int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cpu,
+        int cpuid, void *opaque);
+
+static inline int s390_cpu_write_elf64_qemunote(WriteCoreDumpFunction f,
+        CPUState *env, void *opaque)
+{
+    return 0;
+}
+
+static inline int s390_cpu_write_elf32_qemunote(WriteCoreDumpFunction f,
+        CPUState *env, void *opaque)
+{
+    return 0;
+}
+
+static inline int s390_cpu_write_elf32_note(WriteCoreDumpFunction f,
+        CPUState *env, int cpuid, void *opaque)
+{
+    return 0;
+}
+
 #endif
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index 23fe51f..27cfbc0 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -27,6 +27,7 @@
 #include "qemu-common.h"
 #include "qemu/timer.h"
 #include "hw/hw.h"
+#include "cpu-qom.h"
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/arch_init.h"
 #endif
@@ -168,6 +169,12 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
 
     scc->parent_reset = cc->reset;
     cc->reset = s390_cpu_reset;
+#if !defined(CONFIG_USER_ONLY)
+    cc->write_elf64_note = s390_cpu_write_elf64_note;
+    cc->write_elf64_qemunote = s390_cpu_write_elf64_qemunote;
+    cc->write_elf32_note = s390_cpu_write_elf32_note;
+    cc->write_elf32_qemunote = s390_cpu_write_elf32_qemunote;
+#endif
 
     cc->do_interrupt = s390_cpu_do_interrupt;
     dc->vmsd = &vmstate_s390_cpu;
-- 
1.7.12.4

Re: [Qemu-devel] [PATCH 2/2] s390: dump guest memory implementation

[Alexander Graf]

On 23.04.2013, at 17:30, Jens Freimann wrote:

> Implement dump-guest-memory support for target s390x.
> 
> dump-guest-memory QEMU monitor command didn't work for s390 architecture.
> The result of the command is supposed to be ELF format crash-readable
> dump.
> In order to implement this, the arch-specific part of dump-guest-memory
> was added:
> target-s390x/arch_dump.c contains the whole set of function for writing
> Elf note sections of all types for s390x.
> 
> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
> ---
> configure                  |   2 +-
> include/elf.h              |   6 ++
> target-s390x/Makefile.objs |   2 +-
> target-s390x/arch_dump.c   | 231 +++++++++++++++++++++++++++++++++++++++++++++
> target-s390x/cpu-qom.h     |  21 +++++
> target-s390x/cpu.c         |   7 ++
> 6 files changed, 267 insertions(+), 2 deletions(-)
> create mode 100644 target-s390x/arch_dump.c
> 
> diff --git a/configure b/configure
> index ed49f91..90dc58b 100755
> --- a/configure
> +++ b/configure
> @@ -4326,7 +4326,7 @@ fi
> if test "$target_softmmu" = "yes" ; then
>   echo "CONFIG_SOFTMMU=y" >> $config_target_mak
>   case "$target_arch2" in
> -    i386|x86_64)
> +    i386|x86_64|s390x)
>       echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak
>   esac
> fi
> diff --git a/include/elf.h b/include/elf.h
> index a21ea53..ba4b3a7 100644
> --- a/include/elf.h
> +++ b/include/elf.h
> @@ -1219,11 +1219,17 @@ typedef struct elf64_shdr {
> 
> /* Notes used in ET_CORE */
> #define NT_PRSTATUS	1
> +#define NT_FPREGSET     2
> #define NT_PRFPREG	2
> #define NT_PRPSINFO	3
> #define NT_TASKSTRUCT	4
> #define NT_AUXV		6
> #define NT_PRXFPREG     0x46e62b7f      /* copied from gdb5.1/include/elf/common.h */
> +#define NT_S390_PREFIX  0x305           /* s390 prefix register */
> +#define NT_S390_CTRS    0x304           /* s390 control registers */
> +#define NT_S390_TODPREG 0x303           /* s390 TOD programmable register */
> +#define NT_S390_TODCMP  0x302           /* s390 TOD clock comparator register */
> +#define NT_S390_TIMER   0x301           /* s390 timer register */
> 
> 
> /* Note header in a PT_NOTE section */
> diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
> index 4e63417..c34f654 100644
> --- a/target-s390x/Makefile.objs
> +++ b/target-s390x/Makefile.objs
> @@ -1,4 +1,4 @@
> obj-y += translate.o helper.o cpu.o interrupt.o
> obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
> -obj-$(CONFIG_SOFTMMU) += ioinst.o
> +obj-$(CONFIG_SOFTMMU) += ioinst.o arch_dump.o
> obj-$(CONFIG_KVM) += kvm.o
> diff --git a/target-s390x/arch_dump.c b/target-s390x/arch_dump.c
> new file mode 100644
> index 0000000..f908257
> --- /dev/null
> +++ b/target-s390x/arch_dump.c
> @@ -0,0 +1,231 @@
> +/*
> + * writing ELF notes for s390x arch
> + *
> + *
> + * Copyright IBM Corp. 2012
> + *
> + *     Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
> + * See the COPYING file in the top-level directory.
> + *
> + */
> +
> +#include "cpu.h"
> +#include "elf.h"
> +#include "exec/cpu-all.h"
> +#include "sysemu/dump.h"
> +#include "sysemu/kvm.h"
> +
> +
> +struct s390x_user_regs_struct {
> +    uint64_t        psw[2];
> +    uint64_t        gprs[16];
> +    uint32_t        acrs[16];
> +} QEMU_PACKED;
> +
> +typedef struct s390x_user_regs_struct s390x_user_regs;
> +
> +struct s390x_elf_prstatus_struct {
> +    uint8_t pad1[32];
> +    uint32_t pid;
> +    uint8_t pad2[76];
> +    s390x_user_regs regs;
> +    uint8_t pad3[16];
> +} QEMU_PACKED;
> +
> +typedef struct s390x_elf_prstatus_struct s390x_elf_prstatus;
> +
> +struct s390x_elf_fpregset_struct {
> +        uint32_t        fpc;
> +        uint32_t        pad;
> +        uint64_t        fprs[16];
> +} QEMU_PACKED;
> +
> +typedef struct s390x_elf_fpregset_struct s390x_elf_fpregset;
> +
> +    typedef struct note_struct {
> +        Elf64_Nhdr hdr;
> +        char name[5];
> +        char pad3[3];
> +        union {
> +            s390x_elf_prstatus prstatus;
> +            s390x_elf_fpregset fpregset;
> +            uint32_t prefix;
> +            uint64_t timer;
> +            uint64_t todcmp;
> +            uint32_t todpreg;
> +            uint64_t ctrs[16];
> +        } contents;
> +    } QEMU_PACKED note_t;
> +
> +static int s390x_write_elf64_prstatus(note_t *note, CPUArchState *env)
> +{
> +    int i;
> +    s390x_user_regs *regs;
> +
> +    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
> +
> +    regs = &(note->contents.prstatus.regs);
> +    regs->psw[0] =  cpu_to_be32(env->psw.mask);
> +    regs->psw[1] =  cpu_to_be32(env->psw.addr);
> +    for (i = 0; i <= 15; i++) {
> +        regs->acrs[i] = cpu_to_be32(env->aregs[i]);
> +        regs->gprs[i] = cpu_to_be32(env->regs[i]);

be32? Please verify whether you produce proper dumps on a little endian host.


Alex

> +    }
> +
> +    return 0;
> +}
> +
> +static int s390x_write_elf64_fpregset(note_t *note, CPUArchState *env)
> +{
> +    int i;
> +
> +    note->hdr.n_type = cpu_to_be32(NT_FPREGSET);
> +
> +    note->contents.fpregset.fpc = cpu_to_be32(env->fpc);
> +    for (i = 0; i <= 15; i++) {
> +        note->contents.fpregset.fprs[i] =  cpu_to_be64(env->fregs[i].ll);
> +    }
> +
> +    return 0;
> +}
> +
> +
> +static int s390x_write_elf64_timer(note_t *note, CPUArchState *env)
> +{
> +    note->hdr.n_type = cpu_to_be32(NT_S390_TIMER);
> +
> +    note->contents.timer = cpu_to_be64((uint64_t)(env->cputm));
> +
> +    return 0;
> +}
> +
> +static int s390x_write_elf64_todcmp(note_t *note, CPUArchState *env)
> +{
> +    note->hdr.n_type = cpu_to_be32(NT_S390_TODCMP);
> +
> +    note->contents.todcmp = cpu_to_be64((uint64_t)(env->ckc));
> +
> +    return 0;
> +}
> +
> +static int s390x_write_elf64_todpreg(note_t *note, CPUArchState *env)
> +{
> +    note->hdr.n_type = cpu_to_be32(NT_S390_TODPREG);
> +
> +    note->contents.todpreg = cpu_to_be32((uint32_t)(env->todpr));
> +
> +    return 0;
> +}
> +
> +static int s390x_write_elf64_ctrs(note_t *note, CPUArchState *env)
> +{
> +    int i;
> +
> +    note->hdr.n_type = cpu_to_be32(NT_S390_CTRS);
> +
> +    for (i = 0; i <= 15; i++) {
> +        note->contents.ctrs[i] =  cpu_to_be32(env->cregs[i]);
> +    }
> +
> +    return 0;
> +}
> +
> +static int s390x_write_elf64_prefix(note_t *note, CPUArchState *env)
> +{
> +    note->hdr.n_type = cpu_to_be32(NT_S390_PREFIX);
> +
> +    note->contents.prefix = cpu_to_be32((uint32_t)(env->psa));
> +
> +    return 0;
> +}
> +
> +
> +struct note_func_desc_struct {
> +    int contents_size;
> +    int (*note_contents_func)(note_t *note, CPUArchState *env);
> +} note_func[] = {
> +    { sizeof(((note_t *)0)->contents.prstatus), s390x_write_elf64_prstatus },
> +    { sizeof(((note_t *)0)->contents.prefix),   s390x_write_elf64_prefix },
> +    { sizeof(((note_t *)0)->contents.fpregset), s390x_write_elf64_fpregset },
> +    { sizeof(((note_t *)0)->contents.ctrs),     s390x_write_elf64_ctrs },
> +    { sizeof(((note_t *)0)->contents.timer),    s390x_write_elf64_timer },
> +    { sizeof(((note_t *)0)->contents.todcmp),   s390x_write_elf64_todcmp },
> +    { sizeof(((note_t *)0)->contents.todpreg),  s390x_write_elf64_todpreg },
> +
> +    { 0, NULL}
> +};
> +
> +
> +static int s390x_write_all_elf64_notes(const char *note_name,
> +            WriteCoreDumpFunction f,
> +            CPUArchState *env,
> +            int id,
> +            void *opaque)
> +{
> +    note_t note;
> +    struct note_func_desc_struct *nf;
> +    int note_size;
> +    int ret = -1;
> +
> +    for (nf = note_func; nf->note_contents_func; nf++) {
> +        note.hdr.n_namesz = cpu_to_be32(sizeof(note.name));
> +        note.hdr.n_descsz = cpu_to_be32(nf->contents_size);
> +        strncpy(note.name, note_name, sizeof(note.name));
> +        ret = (*nf->note_contents_func)(&note, env);
> +
> +        note_size = cpu_to_be32(sizeof(note) -
> +                sizeof(note.contents) +
> +                nf->contents_size);
> +        ret = f(&note, note_size, opaque);
> +
> +        if (ret < 0) {
> +            return -1;
> +        }
> +
> +    }
> +
> +    return 0;
> +}
> +
> +
> +int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
> +                         int cpuid, void *opaque)
> +{
> +    S390CPU *cpu = S390_CPU(cs);
> +    return s390x_write_all_elf64_notes("CORE", f, &cpu->env, cpuid, opaque);
> +}
> +
> +int cpu_get_dump_info(ArchDumpInfo *info)
> +{
> +    info->d_machine = EM_S390;
> +    info->d_endian = ELFDATA2MSB;
> +    info->d_class = ELFCLASS64;
> +
> +    return 0;
> +}
> +
> +ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
> +{
> +    int name_size = 8; /* "CORE" or "QEMU" rounded*/
> +    size_t elf_note_size = 0;
> +    int note_head_size;
> +    struct note_func_desc_struct *nf;
> +
> +    assert(class == ELFCLASS64);
> +    note_head_size = sizeof(Elf64_Nhdr);
> +
> +    assert(machine == EM_S390);
> +
> +    for (nf = note_func; nf->note_contents_func; nf++) {
> +        elf_note_size = elf_note_size +
> +            note_head_size +
> +            name_size +
> +            nf->contents_size;
> +    }
> +
> +    return (elf_note_size) * nr_cpus;
> +
> +}
> +
> diff --git a/target-s390x/cpu-qom.h b/target-s390x/cpu-qom.h
> index 34d45c2..3370fb9 100644
> --- a/target-s390x/cpu-qom.h
> +++ b/target-s390x/cpu-qom.h
> @@ -73,4 +73,25 @@ static inline S390CPU *s390_env_get_cpu(CPUS390XState *env)
> 
> void s390_cpu_do_interrupt(CPUState *cpu);
> 
> +int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cpu,
> +        int cpuid, void *opaque);
> +
> +static inline int s390_cpu_write_elf64_qemunote(WriteCoreDumpFunction f,
> +        CPUState *env, void *opaque)
> +{
> +    return 0;
> +}
> +
> +static inline int s390_cpu_write_elf32_qemunote(WriteCoreDumpFunction f,
> +        CPUState *env, void *opaque)
> +{
> +    return 0;
> +}
> +
> +static inline int s390_cpu_write_elf32_note(WriteCoreDumpFunction f,
> +        CPUState *env, int cpuid, void *opaque)
> +{
> +    return 0;
> +}
> +
> #endif
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index 23fe51f..27cfbc0 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -27,6 +27,7 @@
> #include "qemu-common.h"
> #include "qemu/timer.h"
> #include "hw/hw.h"
> +#include "cpu-qom.h"
> #ifndef CONFIG_USER_ONLY
> #include "sysemu/arch_init.h"
> #endif
> @@ -168,6 +169,12 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
> 
>     scc->parent_reset = cc->reset;
>     cc->reset = s390_cpu_reset;
> +#if !defined(CONFIG_USER_ONLY)
> +    cc->write_elf64_note = s390_cpu_write_elf64_note;
> +    cc->write_elf64_qemunote = s390_cpu_write_elf64_qemunote;
> +    cc->write_elf32_note = s390_cpu_write_elf32_note;
> +    cc->write_elf32_qemunote = s390_cpu_write_elf32_qemunote;
> +#endif
> 
>     cc->do_interrupt = s390_cpu_do_interrupt;
>     dc->vmsd = &vmstate_s390_cpu;
> -- 
> 1.7.12.4
> 

Re: [Qemu-devel] [PATCH 2/2] s390: dump guest memory implementation

[Ekaterina Tumanova]

On 04/26/2013 09:12 PM, Alexander Graf wrote:
> On 23.04.2013, at 17:30, Jens Freimann wrote:
>
>> Implement dump-guest-memory support for target s390x.
>>
>> dump-guest-memory QEMU monitor command didn't work for s390 architecture.
>> The result of the command is supposed to be ELF format crash-readable
>> dump.
>> In order to implement this, the arch-specific part of dump-guest-memory
>> was added:
>> target-s390x/arch_dump.c contains the whole set of function for writing
>> Elf note sections of all types for s390x.
>>
>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>> ---
>> configure                  |   2 +-
>> include/elf.h              |   6 ++
>> target-s390x/Makefile.objs |   2 +-
>> target-s390x/arch_dump.c   | 231 +++++++++++++++++++++++++++++++++++++++++++++
>> target-s390x/cpu-qom.h     |  21 +++++
>> target-s390x/cpu.c         |   7 ++
>> 6 files changed, 267 insertions(+), 2 deletions(-)
>> create mode 100644 target-s390x/arch_dump.c
>>
>> diff --git a/configure b/configure
>> index ed49f91..90dc58b 100755
>> --- a/configure
>> +++ b/configure
>> @@ -4326,7 +4326,7 @@ fi
>> if test "$target_softmmu" = "yes" ; then
>>    echo "CONFIG_SOFTMMU=y" >> $config_target_mak
>>    case "$target_arch2" in
>> -    i386|x86_64)
>> +    i386|x86_64|s390x)
>>        echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak
>>    esac
>> fi
>> diff --git a/include/elf.h b/include/elf.h
>> index a21ea53..ba4b3a7 100644
>> --- a/include/elf.h
>> +++ b/include/elf.h
>> @@ -1219,11 +1219,17 @@ typedef struct elf64_shdr {
>>
>> /* Notes used in ET_CORE */
>> #define NT_PRSTATUS	1
>> +#define NT_FPREGSET     2
>> #define NT_PRFPREG	2
>> #define NT_PRPSINFO	3
>> #define NT_TASKSTRUCT	4
>> #define NT_AUXV		6
>> #define NT_PRXFPREG     0x46e62b7f      /* copied from gdb5.1/include/elf/common.h */
>> +#define NT_S390_PREFIX  0x305           /* s390 prefix register */
>> +#define NT_S390_CTRS    0x304           /* s390 control registers */
>> +#define NT_S390_TODPREG 0x303           /* s390 TOD programmable register */
>> +#define NT_S390_TODCMP  0x302           /* s390 TOD clock comparator register */
>> +#define NT_S390_TIMER   0x301           /* s390 timer register */
>>
>>
>> /* Note header in a PT_NOTE section */
>> diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
>> index 4e63417..c34f654 100644
>> --- a/target-s390x/Makefile.objs
>> +++ b/target-s390x/Makefile.objs
>> @@ -1,4 +1,4 @@
>> obj-y += translate.o helper.o cpu.o interrupt.o
>> obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
>> -obj-$(CONFIG_SOFTMMU) += ioinst.o
>> +obj-$(CONFIG_SOFTMMU) += ioinst.o arch_dump.o
>> obj-$(CONFIG_KVM) += kvm.o
>> diff --git a/target-s390x/arch_dump.c b/target-s390x/arch_dump.c
>> new file mode 100644
>> index 0000000..f908257
>> --- /dev/null
>> +++ b/target-s390x/arch_dump.c
>> @@ -0,0 +1,231 @@
>> +/*
>> + * writing ELF notes for s390x arch
>> + *
>> + *
>> + * Copyright IBM Corp. 2012
>> + *
>> + *     Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>> + * See the COPYING file in the top-level directory.
>> + *
>> + */
>> +
>> +#include "cpu.h"
>> +#include "elf.h"
>> +#include "exec/cpu-all.h"
>> +#include "sysemu/dump.h"
>> +#include "sysemu/kvm.h"
>> +
>> +
>> +struct s390x_user_regs_struct {
>> +    uint64_t        psw[2];
>> +    uint64_t        gprs[16];
>> +    uint32_t        acrs[16];
>> +} QEMU_PACKED;
>> +
>> +typedef struct s390x_user_regs_struct s390x_user_regs;
>> +
>> +struct s390x_elf_prstatus_struct {
>> +    uint8_t pad1[32];
>> +    uint32_t pid;
>> +    uint8_t pad2[76];
>> +    s390x_user_regs regs;
>> +    uint8_t pad3[16];
>> +} QEMU_PACKED;
>> +
>> +typedef struct s390x_elf_prstatus_struct s390x_elf_prstatus;
>> +
>> +struct s390x_elf_fpregset_struct {
>> +        uint32_t        fpc;
>> +        uint32_t        pad;
>> +        uint64_t        fprs[16];
>> +} QEMU_PACKED;
>> +
>> +typedef struct s390x_elf_fpregset_struct s390x_elf_fpregset;
>> +
>> +    typedef struct note_struct {
>> +        Elf64_Nhdr hdr;
>> +        char name[5];
>> +        char pad3[3];
>> +        union {
>> +            s390x_elf_prstatus prstatus;
>> +            s390x_elf_fpregset fpregset;
>> +            uint32_t prefix;
>> +            uint64_t timer;
>> +            uint64_t todcmp;
>> +            uint32_t todpreg;
>> +            uint64_t ctrs[16];
>> +        } contents;
>> +    } QEMU_PACKED note_t;
>> +
>> +static int s390x_write_elf64_prstatus(note_t *note, CPUArchState *env)
>> +{
>> +    int i;
>> +    s390x_user_regs *regs;
>> +
>> +    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
>> +
>> +    regs = &(note->contents.prstatus.regs);
>> +    regs->psw[0] =  cpu_to_be32(env->psw.mask);
>> +    regs->psw[1] =  cpu_to_be32(env->psw.addr);
>> +    for (i = 0; i <= 15; i++) {
>> +        regs->acrs[i] = cpu_to_be32(env->aregs[i]);
>> +        regs->gprs[i] = cpu_to_be32(env->regs[i]);
> be32? Please verify whether you produce proper dumps on a little endian host.
>
>
> Alex
Hi,
I don't think I have an opportunity to test this on x86 host. But 
logically as far as I understand, the code is correct here, since we need
to produce big endian dump for s390 guest for any endian host had, and 
we use cpu_to_be32 in the arch-specific part of code.

Regards,
Kate.
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int s390x_write_elf64_fpregset(note_t *note, CPUArchState *env)
>> +{
>> +    int i;
>> +
>> +    note->hdr.n_type = cpu_to_be32(NT_FPREGSET);
>> +
>> +    note->contents.fpregset.fpc = cpu_to_be32(env->fpc);
>> +    for (i = 0; i <= 15; i++) {
>> +        note->contents.fpregset.fprs[i] =  cpu_to_be64(env->fregs[i].ll);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +
>> +static int s390x_write_elf64_timer(note_t *note, CPUArchState *env)
>> +{
>> +    note->hdr.n_type = cpu_to_be32(NT_S390_TIMER);
>> +
>> +    note->contents.timer = cpu_to_be64((uint64_t)(env->cputm));
>> +
>> +    return 0;
>> +}
>> +
>> +static int s390x_write_elf64_todcmp(note_t *note, CPUArchState *env)
>> +{
>> +    note->hdr.n_type = cpu_to_be32(NT_S390_TODCMP);
>> +
>> +    note->contents.todcmp = cpu_to_be64((uint64_t)(env->ckc));
>> +
>> +    return 0;
>> +}
>> +
>> +static int s390x_write_elf64_todpreg(note_t *note, CPUArchState *env)
>> +{
>> +    note->hdr.n_type = cpu_to_be32(NT_S390_TODPREG);
>> +
>> +    note->contents.todpreg = cpu_to_be32((uint32_t)(env->todpr));
>> +
>> +    return 0;
>> +}
>> +
>> +static int s390x_write_elf64_ctrs(note_t *note, CPUArchState *env)
>> +{
>> +    int i;
>> +
>> +    note->hdr.n_type = cpu_to_be32(NT_S390_CTRS);
>> +
>> +    for (i = 0; i <= 15; i++) {
>> +        note->contents.ctrs[i] =  cpu_to_be32(env->cregs[i]);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int s390x_write_elf64_prefix(note_t *note, CPUArchState *env)
>> +{
>> +    note->hdr.n_type = cpu_to_be32(NT_S390_PREFIX);
>> +
>> +    note->contents.prefix = cpu_to_be32((uint32_t)(env->psa));
>> +
>> +    return 0;
>> +}
>> +
>> +
>> +struct note_func_desc_struct {
>> +    int contents_size;
>> +    int (*note_contents_func)(note_t *note, CPUArchState *env);
>> +} note_func[] = {
>> +    { sizeof(((note_t *)0)->contents.prstatus), s390x_write_elf64_prstatus },
>> +    { sizeof(((note_t *)0)->contents.prefix),   s390x_write_elf64_prefix },
>> +    { sizeof(((note_t *)0)->contents.fpregset), s390x_write_elf64_fpregset },
>> +    { sizeof(((note_t *)0)->contents.ctrs),     s390x_write_elf64_ctrs },
>> +    { sizeof(((note_t *)0)->contents.timer),    s390x_write_elf64_timer },
>> +    { sizeof(((note_t *)0)->contents.todcmp),   s390x_write_elf64_todcmp },
>> +    { sizeof(((note_t *)0)->contents.todpreg),  s390x_write_elf64_todpreg },
>> +
>> +    { 0, NULL}
>> +};
>> +
>> +
>> +static int s390x_write_all_elf64_notes(const char *note_name,
>> +            WriteCoreDumpFunction f,
>> +            CPUArchState *env,
>> +            int id,
>> +            void *opaque)
>> +{
>> +    note_t note;
>> +    struct note_func_desc_struct *nf;
>> +    int note_size;
>> +    int ret = -1;
>> +
>> +    for (nf = note_func; nf->note_contents_func; nf++) {
>> +        note.hdr.n_namesz = cpu_to_be32(sizeof(note.name));
>> +        note.hdr.n_descsz = cpu_to_be32(nf->contents_size);
>> +        strncpy(note.name, note_name, sizeof(note.name));
>> +        ret = (*nf->note_contents_func)(&note, env);
>> +
>> +        note_size = cpu_to_be32(sizeof(note) -
>> +                sizeof(note.contents) +
>> +                nf->contents_size);
>> +        ret = f(&note, note_size, opaque);
>> +
>> +        if (ret < 0) {
>> +            return -1;
>> +        }
>> +
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +
>> +int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cs,
>> +                         int cpuid, void *opaque)
>> +{
>> +    S390CPU *cpu = S390_CPU(cs);
>> +    return s390x_write_all_elf64_notes("CORE", f, &cpu->env, cpuid, opaque);
>> +}
>> +
>> +int cpu_get_dump_info(ArchDumpInfo *info)
>> +{
>> +    info->d_machine = EM_S390;
>> +    info->d_endian = ELFDATA2MSB;
>> +    info->d_class = ELFCLASS64;
>> +
>> +    return 0;
>> +}
>> +
>> +ssize_t cpu_get_note_size(int class, int machine, int nr_cpus)
>> +{
>> +    int name_size = 8; /* "CORE" or "QEMU" rounded*/
>> +    size_t elf_note_size = 0;
>> +    int note_head_size;
>> +    struct note_func_desc_struct *nf;
>> +
>> +    assert(class == ELFCLASS64);
>> +    note_head_size = sizeof(Elf64_Nhdr);
>> +
>> +    assert(machine == EM_S390);
>> +
>> +    for (nf = note_func; nf->note_contents_func; nf++) {
>> +        elf_note_size = elf_note_size +
>> +            note_head_size +
>> +            name_size +
>> +            nf->contents_size;
>> +    }
>> +
>> +    return (elf_note_size) * nr_cpus;
>> +
>> +}
>> +
>> diff --git a/target-s390x/cpu-qom.h b/target-s390x/cpu-qom.h
>> index 34d45c2..3370fb9 100644
>> --- a/target-s390x/cpu-qom.h
>> +++ b/target-s390x/cpu-qom.h
>> @@ -73,4 +73,25 @@ static inline S390CPU *s390_env_get_cpu(CPUS390XState *env)
>>
>> void s390_cpu_do_interrupt(CPUState *cpu);
>>
>> +int s390_cpu_write_elf64_note(WriteCoreDumpFunction f, CPUState *cpu,
>> +        int cpuid, void *opaque);
>> +
>> +static inline int s390_cpu_write_elf64_qemunote(WriteCoreDumpFunction f,
>> +        CPUState *env, void *opaque)
>> +{
>> +    return 0;
>> +}
>> +
>> +static inline int s390_cpu_write_elf32_qemunote(WriteCoreDumpFunction f,
>> +        CPUState *env, void *opaque)
>> +{
>> +    return 0;
>> +}
>> +
>> +static inline int s390_cpu_write_elf32_note(WriteCoreDumpFunction f,
>> +        CPUState *env, int cpuid, void *opaque)
>> +{
>> +    return 0;
>> +}
>> +
>> #endif
>> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
>> index 23fe51f..27cfbc0 100644
>> --- a/target-s390x/cpu.c
>> +++ b/target-s390x/cpu.c
>> @@ -27,6 +27,7 @@
>> #include "qemu-common.h"
>> #include "qemu/timer.h"
>> #include "hw/hw.h"
>> +#include "cpu-qom.h"
>> #ifndef CONFIG_USER_ONLY
>> #include "sysemu/arch_init.h"
>> #endif
>> @@ -168,6 +169,12 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
>>
>>      scc->parent_reset = cc->reset;
>>      cc->reset = s390_cpu_reset;
>> +#if !defined(CONFIG_USER_ONLY)
>> +    cc->write_elf64_note = s390_cpu_write_elf64_note;
>> +    cc->write_elf64_qemunote = s390_cpu_write_elf64_qemunote;
>> +    cc->write_elf32_note = s390_cpu_write_elf32_note;
>> +    cc->write_elf32_qemunote = s390_cpu_write_elf32_qemunote;
>> +#endif
>>
>>      cc->do_interrupt = s390_cpu_do_interrupt;
>>      dc->vmsd = &vmstate_s390_cpu;
>> -- 
>> 1.7.12.4
>>

Re: [Qemu-devel] [PATCH 2/2] s390: dump guest memory implementation

[Alexander Graf]

On 29.04.2013, at 13:39, Ekaterina Tumanova wrote:

> On 04/26/2013 09:12 PM, Alexander Graf wrote:
>> On 23.04.2013, at 17:30, Jens Freimann wrote:
>> 
>>> Implement dump-guest-memory support for target s390x.
>>> 
>>> dump-guest-memory QEMU monitor command didn't work for s390 architecture.
>>> The result of the command is supposed to be ELF format crash-readable
>>> dump.
>>> In order to implement this, the arch-specific part of dump-guest-memory
>>> was added:
>>> target-s390x/arch_dump.c contains the whole set of function for writing
>>> Elf note sections of all types for s390x.
>>> 
>>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>>> ---
>>> configure                  |   2 +-
>>> include/elf.h              |   6 ++
>>> target-s390x/Makefile.objs |   2 +-
>>> target-s390x/arch_dump.c   | 231 +++++++++++++++++++++++++++++++++++++++++++++
>>> target-s390x/cpu-qom.h     |  21 +++++
>>> target-s390x/cpu.c         |   7 ++
>>> 6 files changed, 267 insertions(+), 2 deletions(-)
>>> create mode 100644 target-s390x/arch_dump.c
>>> 
>>> diff --git a/configure b/configure
>>> index ed49f91..90dc58b 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -4326,7 +4326,7 @@ fi
>>> if test "$target_softmmu" = "yes" ; then
>>>   echo "CONFIG_SOFTMMU=y" >> $config_target_mak
>>>   case "$target_arch2" in
>>> -    i386|x86_64)
>>> +    i386|x86_64|s390x)
>>>       echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak
>>>   esac
>>> fi
>>> diff --git a/include/elf.h b/include/elf.h
>>> index a21ea53..ba4b3a7 100644
>>> --- a/include/elf.h
>>> +++ b/include/elf.h
>>> @@ -1219,11 +1219,17 @@ typedef struct elf64_shdr {
>>> 
>>> /* Notes used in ET_CORE */
>>> #define NT_PRSTATUS	1
>>> +#define NT_FPREGSET     2
>>> #define NT_PRFPREG	2
>>> #define NT_PRPSINFO	3
>>> #define NT_TASKSTRUCT	4
>>> #define NT_AUXV		6
>>> #define NT_PRXFPREG     0x46e62b7f      /* copied from gdb5.1/include/elf/common.h */
>>> +#define NT_S390_PREFIX  0x305           /* s390 prefix register */
>>> +#define NT_S390_CTRS    0x304           /* s390 control registers */
>>> +#define NT_S390_TODPREG 0x303           /* s390 TOD programmable register */
>>> +#define NT_S390_TODCMP  0x302           /* s390 TOD clock comparator register */
>>> +#define NT_S390_TIMER   0x301           /* s390 timer register */
>>> 
>>> 
>>> /* Note header in a PT_NOTE section */
>>> diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
>>> index 4e63417..c34f654 100644
>>> --- a/target-s390x/Makefile.objs
>>> +++ b/target-s390x/Makefile.objs
>>> @@ -1,4 +1,4 @@
>>> obj-y += translate.o helper.o cpu.o interrupt.o
>>> obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
>>> -obj-$(CONFIG_SOFTMMU) += ioinst.o
>>> +obj-$(CONFIG_SOFTMMU) += ioinst.o arch_dump.o
>>> obj-$(CONFIG_KVM) += kvm.o
>>> diff --git a/target-s390x/arch_dump.c b/target-s390x/arch_dump.c
>>> new file mode 100644
>>> index 0000000..f908257
>>> --- /dev/null
>>> +++ b/target-s390x/arch_dump.c
>>> @@ -0,0 +1,231 @@
>>> +/*
>>> + * writing ELF notes for s390x arch
>>> + *
>>> + *
>>> + * Copyright IBM Corp. 2012
>>> + *
>>> + *     Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>>> + *
>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>> + * See the COPYING file in the top-level directory.
>>> + *
>>> + */
>>> +
>>> +#include "cpu.h"
>>> +#include "elf.h"
>>> +#include "exec/cpu-all.h"
>>> +#include "sysemu/dump.h"
>>> +#include "sysemu/kvm.h"
>>> +
>>> +
>>> +struct s390x_user_regs_struct {
>>> +    uint64_t        psw[2];
>>> +    uint64_t        gprs[16];
>>> +    uint32_t        acrs[16];
>>> +} QEMU_PACKED;
>>> +
>>> +typedef struct s390x_user_regs_struct s390x_user_regs;
>>> +
>>> +struct s390x_elf_prstatus_struct {
>>> +    uint8_t pad1[32];
>>> +    uint32_t pid;
>>> +    uint8_t pad2[76];
>>> +    s390x_user_regs regs;
>>> +    uint8_t pad3[16];
>>> +} QEMU_PACKED;
>>> +
>>> +typedef struct s390x_elf_prstatus_struct s390x_elf_prstatus;
>>> +
>>> +struct s390x_elf_fpregset_struct {
>>> +        uint32_t        fpc;
>>> +        uint32_t        pad;
>>> +        uint64_t        fprs[16];
>>> +} QEMU_PACKED;
>>> +
>>> +typedef struct s390x_elf_fpregset_struct s390x_elf_fpregset;
>>> +
>>> +    typedef struct note_struct {
>>> +        Elf64_Nhdr hdr;
>>> +        char name[5];
>>> +        char pad3[3];
>>> +        union {
>>> +            s390x_elf_prstatus prstatus;
>>> +            s390x_elf_fpregset fpregset;
>>> +            uint32_t prefix;
>>> +            uint64_t timer;
>>> +            uint64_t todcmp;
>>> +            uint32_t todpreg;
>>> +            uint64_t ctrs[16];
>>> +        } contents;
>>> +    } QEMU_PACKED note_t;
>>> +
>>> +static int s390x_write_elf64_prstatus(note_t *note, CPUArchState *env)
>>> +{
>>> +    int i;
>>> +    s390x_user_regs *regs;
>>> +
>>> +    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
>>> +
>>> +    regs = &(note->contents.prstatus.regs);
>>> +    regs->psw[0] =  cpu_to_be32(env->psw.mask);
>>> +    regs->psw[1] =  cpu_to_be32(env->psw.addr);
>>> +    for (i = 0; i <= 15; i++) {
>>> +        regs->acrs[i] = cpu_to_be32(env->aregs[i]);
>>> +        regs->gprs[i] = cpu_to_be32(env->regs[i]);
>> be32? Please verify whether you produce proper dumps on a little endian host.
>> 
>> 
>> Alex
> Hi,
> I don't think I have an opportunity to test this on x86 host.

Then ask someone else to do so.

> But logically as far as I understand, the code is correct here, since we need
> to produce big endian dump for s390 guest for any endian host had, and we use cpu_to_be32 in the arch-specific part of code.

It's completely broken, as you truncate 64-bit registers to 32 bit.


Alex

Re: [Qemu-devel] [PATCH 2/2] s390: dump guest memory implementation

[Ekaterina Tumanova]

On 04/30/2013 02:02 PM, Alexander Graf wrote:
> On 29.04.2013, at 13:39, Ekaterina Tumanova wrote:
>
>> On 04/26/2013 09:12 PM, Alexander Graf wrote:
>>> On 23.04.2013, at 17:30, Jens Freimann wrote:
>>>
>>>> Implement dump-guest-memory support for target s390x.
>>>>
>>>> dump-guest-memory QEMU monitor command didn't work for s390 architecture.
>>>> The result of the command is supposed to be ELF format crash-readable
>>>> dump.
>>>> In order to implement this, the arch-specific part of dump-guest-memory
>>>> was added:
>>>> target-s390x/arch_dump.c contains the whole set of function for writing
>>>> Elf note sections of all types for s390x.
>>>>
>>>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>>>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>>>> ---
>>>> configure                  |   2 +-
>>>> include/elf.h              |   6 ++
>>>> target-s390x/Makefile.objs |   2 +-
>>>> target-s390x/arch_dump.c   | 231 +++++++++++++++++++++++++++++++++++++++++++++
>>>> target-s390x/cpu-qom.h     |  21 +++++
>>>> target-s390x/cpu.c         |   7 ++
>>>> 6 files changed, 267 insertions(+), 2 deletions(-)
>>>> create mode 100644 target-s390x/arch_dump.c
>>>>
>>>> diff --git a/configure b/configure
>>>> index ed49f91..90dc58b 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -4326,7 +4326,7 @@ fi
>>>> if test "$target_softmmu" = "yes" ; then
>>>>    echo "CONFIG_SOFTMMU=y" >> $config_target_mak
>>>>    case "$target_arch2" in
>>>> -    i386|x86_64)
>>>> +    i386|x86_64|s390x)
>>>>        echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak
>>>>    esac
>>>> fi
>>>> diff --git a/include/elf.h b/include/elf.h
>>>> index a21ea53..ba4b3a7 100644
>>>> --- a/include/elf.h
>>>> +++ b/include/elf.h
>>>> @@ -1219,11 +1219,17 @@ typedef struct elf64_shdr {
>>>>
>>>> /* Notes used in ET_CORE */
>>>> #define NT_PRSTATUS	1
>>>> +#define NT_FPREGSET     2
>>>> #define NT_PRFPREG	2
>>>> #define NT_PRPSINFO	3
>>>> #define NT_TASKSTRUCT	4
>>>> #define NT_AUXV		6
>>>> #define NT_PRXFPREG     0x46e62b7f      /* copied from gdb5.1/include/elf/common.h */
>>>> +#define NT_S390_PREFIX  0x305           /* s390 prefix register */
>>>> +#define NT_S390_CTRS    0x304           /* s390 control registers */
>>>> +#define NT_S390_TODPREG 0x303           /* s390 TOD programmable register */
>>>> +#define NT_S390_TODCMP  0x302           /* s390 TOD clock comparator register */
>>>> +#define NT_S390_TIMER   0x301           /* s390 timer register */
>>>>
>>>>
>>>> /* Note header in a PT_NOTE section */
>>>> diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
>>>> index 4e63417..c34f654 100644
>>>> --- a/target-s390x/Makefile.objs
>>>> +++ b/target-s390x/Makefile.objs
>>>> @@ -1,4 +1,4 @@
>>>> obj-y += translate.o helper.o cpu.o interrupt.o
>>>> obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
>>>> -obj-$(CONFIG_SOFTMMU) += ioinst.o
>>>> +obj-$(CONFIG_SOFTMMU) += ioinst.o arch_dump.o
>>>> obj-$(CONFIG_KVM) += kvm.o
>>>> diff --git a/target-s390x/arch_dump.c b/target-s390x/arch_dump.c
>>>> new file mode 100644
>>>> index 0000000..f908257
>>>> --- /dev/null
>>>> +++ b/target-s390x/arch_dump.c
>>>> @@ -0,0 +1,231 @@
>>>> +/*
>>>> + * writing ELF notes for s390x arch
>>>> + *
>>>> + *
>>>> + * Copyright IBM Corp. 2012
>>>> + *
>>>> + *     Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>>>> + *
>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>>> + * See the COPYING file in the top-level directory.
>>>> + *
>>>> + */
>>>> +
>>>> +#include "cpu.h"
>>>> +#include "elf.h"
>>>> +#include "exec/cpu-all.h"
>>>> +#include "sysemu/dump.h"
>>>> +#include "sysemu/kvm.h"
>>>> +
>>>> +
>>>> +struct s390x_user_regs_struct {
>>>> +    uint64_t        psw[2];
>>>> +    uint64_t        gprs[16];
>>>> +    uint32_t        acrs[16];
>>>> +} QEMU_PACKED;
>>>> +
>>>> +typedef struct s390x_user_regs_struct s390x_user_regs;
>>>> +
>>>> +struct s390x_elf_prstatus_struct {
>>>> +    uint8_t pad1[32];
>>>> +    uint32_t pid;
>>>> +    uint8_t pad2[76];
>>>> +    s390x_user_regs regs;
>>>> +    uint8_t pad3[16];
>>>> +} QEMU_PACKED;
>>>> +
>>>> +typedef struct s390x_elf_prstatus_struct s390x_elf_prstatus;
>>>> +
>>>> +struct s390x_elf_fpregset_struct {
>>>> +        uint32_t        fpc;
>>>> +        uint32_t        pad;
>>>> +        uint64_t        fprs[16];
>>>> +} QEMU_PACKED;
>>>> +
>>>> +typedef struct s390x_elf_fpregset_struct s390x_elf_fpregset;
>>>> +
>>>> +    typedef struct note_struct {
>>>> +        Elf64_Nhdr hdr;
>>>> +        char name[5];
>>>> +        char pad3[3];
>>>> +        union {
>>>> +            s390x_elf_prstatus prstatus;
>>>> +            s390x_elf_fpregset fpregset;
>>>> +            uint32_t prefix;
>>>> +            uint64_t timer;
>>>> +            uint64_t todcmp;
>>>> +            uint32_t todpreg;
>>>> +            uint64_t ctrs[16];
>>>> +        } contents;
>>>> +    } QEMU_PACKED note_t;
>>>> +
>>>> +static int s390x_write_elf64_prstatus(note_t *note, CPUArchState *env)
>>>> +{
>>>> +    int i;
>>>> +    s390x_user_regs *regs;
>>>> +
>>>> +    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
>>>> +
>>>> +    regs = &(note->contents.prstatus.regs);
>>>> +    regs->psw[0] =  cpu_to_be32(env->psw.mask);
>>>> +    regs->psw[1] =  cpu_to_be32(env->psw.addr);
>>>> +    for (i = 0; i <= 15; i++) {
>>>> +        regs->acrs[i] = cpu_to_be32(env->aregs[i]);
>>>> +        regs->gprs[i] = cpu_to_be32(env->regs[i]);
>>> be32? Please verify whether you produce proper dumps on a little endian host.
>>>
>>>
>>> Alex
>> Hi,
>> I don't think I have an opportunity to test this on x86 host.
> Then ask someone else to do so.
>
>> But logically as far as I understand, the code is correct here, since we need
>> to produce big endian dump for s390 guest for any endian host had, and we use cpu_to_be32 in the arch-specific part of code.
> It's completely broken, as you truncate 64-bit registers to 32 bit.
>
>
> Alex
>
I thought you were talking about be/le. Agreed. Will fix this. Thanks!

Regards,
Kate.

Re: [Qemu-devel] [PATCH 2/2] s390: dump guest memory implementation

[Alexander Graf]

On 30.04.2013, at 12:20, Ekaterina Tumanova wrote:

> On 04/30/2013 02:02 PM, Alexander Graf wrote:
>> On 29.04.2013, at 13:39, Ekaterina Tumanova wrote:
>> 
>>> On 04/26/2013 09:12 PM, Alexander Graf wrote:
>>>> On 23.04.2013, at 17:30, Jens Freimann wrote:
>>>> 
>>>>> Implement dump-guest-memory support for target s390x.
>>>>> 
>>>>> dump-guest-memory QEMU monitor command didn't work for s390 architecture.
>>>>> The result of the command is supposed to be ELF format crash-readable
>>>>> dump.
>>>>> In order to implement this, the arch-specific part of dump-guest-memory
>>>>> was added:
>>>>> target-s390x/arch_dump.c contains the whole set of function for writing
>>>>> Elf note sections of all types for s390x.
>>>>> 
>>>>> Signed-off-by: Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>>>>> Signed-off-by: Jens Freimann <jfrei@linux.vnet.ibm.com>
>>>>> ---
>>>>> configure                  |   2 +-
>>>>> include/elf.h              |   6 ++
>>>>> target-s390x/Makefile.objs |   2 +-
>>>>> target-s390x/arch_dump.c   | 231 +++++++++++++++++++++++++++++++++++++++++++++
>>>>> target-s390x/cpu-qom.h     |  21 +++++
>>>>> target-s390x/cpu.c         |   7 ++
>>>>> 6 files changed, 267 insertions(+), 2 deletions(-)
>>>>> create mode 100644 target-s390x/arch_dump.c
>>>>> 
>>>>> diff --git a/configure b/configure
>>>>> index ed49f91..90dc58b 100755
>>>>> --- a/configure
>>>>> +++ b/configure
>>>>> @@ -4326,7 +4326,7 @@ fi
>>>>> if test "$target_softmmu" = "yes" ; then
>>>>>   echo "CONFIG_SOFTMMU=y" >> $config_target_mak
>>>>>   case "$target_arch2" in
>>>>> -    i386|x86_64)
>>>>> +    i386|x86_64|s390x)
>>>>>       echo "CONFIG_HAVE_CORE_DUMP=y" >> $config_target_mak
>>>>>   esac
>>>>> fi
>>>>> diff --git a/include/elf.h b/include/elf.h
>>>>> index a21ea53..ba4b3a7 100644
>>>>> --- a/include/elf.h
>>>>> +++ b/include/elf.h
>>>>> @@ -1219,11 +1219,17 @@ typedef struct elf64_shdr {
>>>>> 
>>>>> /* Notes used in ET_CORE */
>>>>> #define NT_PRSTATUS	1
>>>>> +#define NT_FPREGSET     2
>>>>> #define NT_PRFPREG	2
>>>>> #define NT_PRPSINFO	3
>>>>> #define NT_TASKSTRUCT	4
>>>>> #define NT_AUXV		6
>>>>> #define NT_PRXFPREG     0x46e62b7f      /* copied from gdb5.1/include/elf/common.h */
>>>>> +#define NT_S390_PREFIX  0x305           /* s390 prefix register */
>>>>> +#define NT_S390_CTRS    0x304           /* s390 control registers */
>>>>> +#define NT_S390_TODPREG 0x303           /* s390 TOD programmable register */
>>>>> +#define NT_S390_TODCMP  0x302           /* s390 TOD clock comparator register */
>>>>> +#define NT_S390_TIMER   0x301           /* s390 timer register */
>>>>> 
>>>>> 
>>>>> /* Note header in a PT_NOTE section */
>>>>> diff --git a/target-s390x/Makefile.objs b/target-s390x/Makefile.objs
>>>>> index 4e63417..c34f654 100644
>>>>> --- a/target-s390x/Makefile.objs
>>>>> +++ b/target-s390x/Makefile.objs
>>>>> @@ -1,4 +1,4 @@
>>>>> obj-y += translate.o helper.o cpu.o interrupt.o
>>>>> obj-y += int_helper.o fpu_helper.o cc_helper.o mem_helper.o misc_helper.o
>>>>> -obj-$(CONFIG_SOFTMMU) += ioinst.o
>>>>> +obj-$(CONFIG_SOFTMMU) += ioinst.o arch_dump.o
>>>>> obj-$(CONFIG_KVM) += kvm.o
>>>>> diff --git a/target-s390x/arch_dump.c b/target-s390x/arch_dump.c
>>>>> new file mode 100644
>>>>> index 0000000..f908257
>>>>> --- /dev/null
>>>>> +++ b/target-s390x/arch_dump.c
>>>>> @@ -0,0 +1,231 @@
>>>>> +/*
>>>>> + * writing ELF notes for s390x arch
>>>>> + *
>>>>> + *
>>>>> + * Copyright IBM Corp. 2012
>>>>> + *
>>>>> + *     Ekaterina Tumanova <tumanova@linux.vnet.ibm.com>
>>>>> + *
>>>>> + * This work is licensed under the terms of the GNU GPL, version 2 or later.
>>>>> + * See the COPYING file in the top-level directory.
>>>>> + *
>>>>> + */
>>>>> +
>>>>> +#include "cpu.h"
>>>>> +#include "elf.h"
>>>>> +#include "exec/cpu-all.h"
>>>>> +#include "sysemu/dump.h"
>>>>> +#include "sysemu/kvm.h"
>>>>> +
>>>>> +
>>>>> +struct s390x_user_regs_struct {
>>>>> +    uint64_t        psw[2];
>>>>> +    uint64_t        gprs[16];
>>>>> +    uint32_t        acrs[16];
>>>>> +} QEMU_PACKED;
>>>>> +
>>>>> +typedef struct s390x_user_regs_struct s390x_user_regs;
>>>>> +
>>>>> +struct s390x_elf_prstatus_struct {
>>>>> +    uint8_t pad1[32];
>>>>> +    uint32_t pid;
>>>>> +    uint8_t pad2[76];
>>>>> +    s390x_user_regs regs;
>>>>> +    uint8_t pad3[16];
>>>>> +} QEMU_PACKED;
>>>>> +
>>>>> +typedef struct s390x_elf_prstatus_struct s390x_elf_prstatus;
>>>>> +
>>>>> +struct s390x_elf_fpregset_struct {
>>>>> +        uint32_t        fpc;
>>>>> +        uint32_t        pad;
>>>>> +        uint64_t        fprs[16];
>>>>> +} QEMU_PACKED;
>>>>> +
>>>>> +typedef struct s390x_elf_fpregset_struct s390x_elf_fpregset;
>>>>> +
>>>>> +    typedef struct note_struct {
>>>>> +        Elf64_Nhdr hdr;
>>>>> +        char name[5];
>>>>> +        char pad3[3];
>>>>> +        union {
>>>>> +            s390x_elf_prstatus prstatus;
>>>>> +            s390x_elf_fpregset fpregset;
>>>>> +            uint32_t prefix;
>>>>> +            uint64_t timer;
>>>>> +            uint64_t todcmp;
>>>>> +            uint32_t todpreg;
>>>>> +            uint64_t ctrs[16];
>>>>> +        } contents;
>>>>> +    } QEMU_PACKED note_t;
>>>>> +
>>>>> +static int s390x_write_elf64_prstatus(note_t *note, CPUArchState *env)
>>>>> +{
>>>>> +    int i;
>>>>> +    s390x_user_regs *regs;
>>>>> +
>>>>> +    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
>>>>> +
>>>>> +    regs = &(note->contents.prstatus.regs);
>>>>> +    regs->psw[0] =  cpu_to_be32(env->psw.mask);
>>>>> +    regs->psw[1] =  cpu_to_be32(env->psw.addr);
>>>>> +    for (i = 0; i <= 15; i++) {
>>>>> +        regs->acrs[i] = cpu_to_be32(env->aregs[i]);
>>>>> +        regs->gprs[i] = cpu_to_be32(env->regs[i]);
>>>> be32? Please verify whether you produce proper dumps on a little endian host.
>>>> 
>>>> 
>>>> Alex
>>> Hi,
>>> I don't think I have an opportunity to test this on x86 host.
>> Then ask someone else to do so.
>> 
>>> But logically as far as I understand, the code is correct here, since we need
>>> to produce big endian dump for s390 guest for any endian host had, and we use cpu_to_be32 in the arch-specific part of code.
>> It's completely broken, as you truncate 64-bit registers to 32 bit.
>> 
>> 
>> Alex
>> 
> I thought you were talking about be/le. Agreed. Will fix this. Thanks!

The reason you didn't realize it is broken is that cpu_to_be.. functions are #defined to nops. On an LE machine, you would've seen that the values get truncated. Hence I asked you to run this code on an LE machine. I'm quite sure there are more issues like this lurking in the code somewhere ;).


Alex