qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Alexander Graf
Subject: Re: [Qemu-devel] [PATCH 2/2] s390: dump guest memory implementation
Date: Tue, 30 Apr 2013 12:25:11 +0200

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 <address@hidden>
>>>>> Signed-off-by: Jens Freimann <address@hidden>
>>>>> ---
>>>>> 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 <address@hidden>
>>>>> + *
>>>>> + * 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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]