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 <tuman...@linux.vnet.ibm.com>
>>>>> Signed-off-by: Jens Freimann <jf...@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 <tuman...@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
