Skip to contentSkip to conversation

little-endian dump for ppc64

PATCH Superseded Greg Kurz 6 participants 27 messages May 5, 2014 – May 9, 2014

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Greg Kurz

author
#0 lore raw

v3 cover little-endian dump for ppc64

Hi,

Please find the latest patch set for dump-guest-memory to support little-endian ppc64 guest kernels. The only change since the last post is a better naming of the dump endian helpers. Since it affects patches 1 & 2, I resend the full serie, with Alex's Reviewed-by tags from the last review.

Cheers.

---

Bharata B Rao (3):
dump: Make DumpState and endian conversion routines available for arch-specific dump code
ppc64-dump: Support dump for little endian ppc64
ppc64 dump: Set the correct endianness in ELF dump header

Greg Kurz (1):
target-ppc: ppc can be either endian

 dump.c                      |  228 ++++++++++++++++---------------------------
 include/sysemu/dump-arch.h  |   28 +++++
 include/sysemu/dump.h       |   48 +++++++--
 stubs/dump.c                |    2 
 target-ppc/arch_dump.c      |   94 +++++++++++-------
 target-ppc/cpu-qom.h        |    1 
 target-ppc/translate_init.c |   16 +++
 7 files changed, 228 insertions(+), 189 deletions(-)
 create mode 100644 include/sysemu/dump-arch.h
Signature · 1 line
Greg
author
#1 lore raw

PATCH v3 1/4 dump: Make DumpState and endian conversion routines available for arch-specific dump code

From: Bharata B Rao <[email protected]>

Make DumpState and endian conversion routines available for arch-specific dump code by moving into dump.h. DumpState will be needed by arch-specific dump code to access target endian information from DumpState->ArchDumpInfo. Also break the dependency of dump.h from stubs/dump.c by creating a separate dump-arch.h.

This patch doesn't change any functionality.

[ rebased on top of current master branch,
renamed endian helpers to cpu_to_dump{16,32,64}, pass a DumpState * argument to endian helpers, Greg Kurz <[email protected]> ]

---

Changes in v3:
- better taste with the endian helpers naming

 dump.c                     |  228 +++++++++++++++++---------------------------
 include/sysemu/dump-arch.h |   28 +++++
 include/sysemu/dump.h      |   48 +++++++--
 stubs/dump.c               |    2 
 4 files changed, 154 insertions(+), 152 deletions(-)
 create mode 100644 include/sysemu/dump-arch.h
dump.c +87 -141
index 14b3d1d..d24b865 100644
@@ -36,9 +36,9 @@
 #define ELF_MACHINE_UNAME "Unknown"
 #endif
 
-static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
+uint16_t cpu_to_dump16(DumpState *s, uint16_t val)
 {
-    if (endian == ELFDATA2LSB) {
+    if (s->dump_info.d_endian == ELFDATA2LSB) {
         val = cpu_to_le16(val);
     } else {
         val = cpu_to_be16(val);
@@ -47,9 +47,9 @@ static uint16_t cpu_convert_to_target16(uint16_t val, int endian)
     return val;
 }
 
-static uint32_t cpu_convert_to_target32(uint32_t val, int endian)
+uint32_t cpu_to_dump32(DumpState *s, uint32_t val)
 {
-    if (endian == ELFDATA2LSB) {
+    if (s->dump_info.d_endian == ELFDATA2LSB) {
         val = cpu_to_le32(val);
     } else {
         val = cpu_to_be32(val);
@@ -58,9 +58,9 @@ static uint32_t cpu_convert_to_target32(uint32_t val, int endian)
     return val;
 }
 
-static uint64_t cpu_convert_to_target64(uint64_t val, int endian)
+uint64_t cpu_to_dump64(DumpState *s, uint64_t val)
 {
-    if (endian == ELFDATA2LSB) {
+    if (s->dump_info.d_endian == ELFDATA2LSB) {
         val = cpu_to_le64(val);
     } else {
         val = cpu_to_be64(val);
@@ -69,39 +69,6 @@ static uint64_t cpu_convert_to_target64(uint64_t val, int endian)
     return val;
 }
 
-typedef struct DumpState {
-    GuestPhysBlockList guest_phys_blocks;
-    ArchDumpInfo dump_info;
-    MemoryMappingList list;
-    uint16_t phdr_num;
-    uint32_t sh_info;
-    bool have_section;
-    bool resume;
-    ssize_t note_size;
-    hwaddr memory_offset;
-    int fd;
-
-    GuestPhysBlock *next_block;
-    ram_addr_t start;
-    bool has_filter;
-    int64_t begin;
-    int64_t length;
-    Error **errp;
-
-    uint8_t *note_buf;          /* buffer for notes */
-    size_t note_buf_offset;     /* the writing place in note_buf */
-    uint32_t nr_cpus;           /* number of guest's cpu */
-    size_t page_size;           /* guest's page size */
-    uint32_t page_shift;        /* guest's page shift */
-    uint64_t max_mapnr;         /* the biggest guest's phys-mem's number */
-    size_t len_dump_bitmap;     /* the size of the place used to store
-                                   dump_bitmap in vmcore */
-    off_t offset_dump_bitmap;   /* offset of dump_bitmap part in vmcore */
-    off_t offset_page;          /* offset of page part in vmcore */
-    size_t num_dumpable;        /* number of page that can be dumped */
-    uint32_t flag_compress;     /* indicate the compression format */
-} DumpState;
-
 static int dump_cleanup(DumpState *s)
 {
     int ret = 0;
@@ -140,29 +107,25 @@ static int write_elf64_header(DumpState *s)
 {
     Elf64_Ehdr elf_header;
     int ret;
-    int endian = s->dump_info.d_endian;
 
     memset(&elf_header, 0, sizeof(Elf64_Ehdr));
     memcpy(&elf_header, ELFMAG, SELFMAG);
     elf_header.e_ident[EI_CLASS] = ELFCLASS64;
     elf_header.e_ident[EI_DATA] = s->dump_info.d_endian;
     elf_header.e_ident[EI_VERSION] = EV_CURRENT;
-    elf_header.e_type = cpu_convert_to_target16(ET_CORE, endian);
-    elf_header.e_machine = cpu_convert_to_target16(s->dump_info.d_machine,
-                                                   endian);
-    elf_header.e_version = cpu_convert_to_target32(EV_CURRENT, endian);
-    elf_header.e_ehsize = cpu_convert_to_target16(sizeof(elf_header), endian);
-    elf_header.e_phoff = cpu_convert_to_target64(sizeof(Elf64_Ehdr), endian);
-    elf_header.e_phentsize = cpu_convert_to_target16(sizeof(Elf64_Phdr),
-                                                     endian);
-    elf_header.e_phnum = cpu_convert_to_target16(s->phdr_num, endian);
+    elf_header.e_type = cpu_to_dump16(s, ET_CORE);
+    elf_header.e_machine = cpu_to_dump16(s, s->dump_info.d_machine);
+    elf_header.e_version = cpu_to_dump32(s, EV_CURRENT);
+    elf_header.e_ehsize = cpu_to_dump16(s, sizeof(elf_header));
+    elf_header.e_phoff = cpu_to_dump64(s, sizeof(Elf64_Ehdr));
+    elf_header.e_phentsize = cpu_to_dump16(s, sizeof(Elf64_Phdr));
+    elf_header.e_phnum = cpu_to_dump16(s, s->phdr_num);
     if (s->have_section) {
         uint64_t shoff = sizeof(Elf64_Ehdr) + sizeof(Elf64_Phdr) * s->sh_info;
 
-        elf_header.e_shoff = cpu_convert_to_target64(shoff, endian);
-        elf_header.e_shentsize = cpu_convert_to_target16(sizeof(Elf64_Shdr),
-                                                         endian);
-        elf_header.e_shnum = cpu_convert_to_target16(1, endian);
+        elf_header.e_shoff = cpu_to_dump64(s, shoff);
+        elf_header.e_shentsize = cpu_to_dump16(s, sizeof(Elf64_Shdr));
+        elf_header.e_shnum = cpu_to_dump16(s, 1);
     }
 
     ret = fd_write_vmcore(&elf_header, sizeof(elf_header), s);
@@ -178,29 +141,25 @@ static int write_elf32_header(DumpState *s)
 {
     Elf32_Ehdr elf_header;
     int ret;
-    int endian = s->dump_info.d_endian;
 
     memset(&elf_header, 0, sizeof(Elf32_Ehdr));
     memcpy(&elf_header, ELFMAG, SELFMAG);
     elf_header.e_ident[EI_CLASS] = ELFCLASS32;
-    elf_header.e_ident[EI_DATA] = endian;
+    elf_header.e_ident[EI_DATA] = s->dump_info.d_endian;
     elf_header.e_ident[EI_VERSION] = EV_CURRENT;
-    elf_header.e_type = cpu_convert_to_target16(ET_CORE, endian);
-    elf_header.e_machine = cpu_convert_to_target16(s->dump_info.d_machine,
-                                                   endian);
-    elf_header.e_version = cpu_convert_to_target32(EV_CURRENT, endian);
-    elf_header.e_ehsize = cpu_convert_to_target16(sizeof(elf_header), endian);
-    elf_header.e_phoff = cpu_convert_to_target32(sizeof(Elf32_Ehdr), endian);
-    elf_header.e_phentsize = cpu_convert_to_target16(sizeof(Elf32_Phdr),
-                                                     endian);
-    elf_header.e_phnum = cpu_convert_to_target16(s->phdr_num, endian);
+    elf_header.e_type = cpu_to_dump16(s, ET_CORE);
+    elf_header.e_machine = cpu_to_dump16(s, s->dump_info.d_machine);
+    elf_header.e_version = cpu_to_dump32(s, EV_CURRENT);
+    elf_header.e_ehsize = cpu_to_dump16(s, sizeof(elf_header));
+    elf_header.e_phoff = cpu_to_dump32(s, sizeof(Elf32_Ehdr));
+    elf_header.e_phentsize = cpu_to_dump16(s, sizeof(Elf32_Phdr));
+    elf_header.e_phnum = cpu_to_dump16(s, s->phdr_num);
     if (s->have_section) {
         uint32_t shoff = sizeof(Elf32_Ehdr) + sizeof(Elf32_Phdr) * s->sh_info;
 
-        elf_header.e_shoff = cpu_convert_to_target32(shoff, endian);
-        elf_header.e_shentsize = cpu_convert_to_target16(sizeof(Elf32_Shdr),
-                                                         endian);
-        elf_header.e_shnum = cpu_convert_to_target16(1, endian);
+        elf_header.e_shoff = cpu_to_dump32(s, shoff);
+        elf_header.e_shentsize = cpu_to_dump16(s, sizeof(Elf32_Shdr));
+        elf_header.e_shnum = cpu_to_dump16(s, 1);
     }
 
     ret = fd_write_vmcore(&elf_header, sizeof(elf_header), s);
@@ -218,15 +177,14 @@ static int write_elf64_load(DumpState *s, MemoryMapping *memory_mapping,
 {
     Elf64_Phdr phdr;
     int ret;
-    int endian = s->dump_info.d_endian;
 
     memset(&phdr, 0, sizeof(Elf64_Phdr));
-    phdr.p_type = cpu_convert_to_target32(PT_LOAD, endian);
-    phdr.p_offset = cpu_convert_to_target64(offset, endian);
-    phdr.p_paddr = cpu_convert_to_target64(memory_mapping->phys_addr, endian);
-    phdr.p_filesz = cpu_convert_to_target64(filesz, endian);
-    phdr.p_memsz = cpu_convert_to_target64(memory_mapping->length, endian);
-    phdr.p_vaddr = cpu_convert_to_target64(memory_mapping->virt_addr, endian);
+    phdr.p_type = cpu_to_dump32(s, PT_LOAD);
+    phdr.p_offset = cpu_to_dump64(s, offset);
+    phdr.p_paddr = cpu_to_dump64(s, memory_mapping->phys_addr);
+    phdr.p_filesz = cpu_to_dump64(s, filesz);
+    phdr.p_memsz = cpu_to_dump64(s, memory_mapping->length);
+    phdr.p_vaddr = cpu_to_dump64(s, memory_mapping->virt_addr);
 
     assert(memory_mapping->length >= filesz);
 
@@ -245,15 +203,14 @@ static int write_elf32_load(DumpState *s, MemoryMapping *memory_mapping,
 {
     Elf32_Phdr phdr;
     int ret;
-    int endian = s->dump_info.d_endian;
 
     memset(&phdr, 0, sizeof(Elf32_Phdr));
-    phdr.p_type = cpu_convert_to_target32(PT_LOAD, endian);
-    phdr.p_offset = cpu_convert_to_target32(offset, endian);
-    phdr.p_paddr = cpu_convert_to_target32(memory_mapping->phys_addr, endian);
-    phdr.p_filesz = cpu_convert_to_target32(filesz, endian);
-    phdr.p_memsz = cpu_convert_to_target32(memory_mapping->length, endian);
-    phdr.p_vaddr = cpu_convert_to_target32(memory_mapping->virt_addr, endian);
+    phdr.p_type = cpu_to_dump32(s, PT_LOAD);
+    phdr.p_offset = cpu_to_dump32(s, offset);
+    phdr.p_paddr = cpu_to_dump32(s, memory_mapping->phys_addr);
+    phdr.p_filesz = cpu_to_dump32(s, filesz);
+    phdr.p_memsz = cpu_to_dump32(s, memory_mapping->length);
+    phdr.p_vaddr = cpu_to_dump32(s, memory_mapping->virt_addr);
 
     assert(memory_mapping->length >= filesz);
 
@@ -269,16 +226,15 @@ static int write_elf32_load(DumpState *s, MemoryMapping *memory_mapping,
 static int write_elf64_note(DumpState *s)
 {
     Elf64_Phdr phdr;
-    int endian = s->dump_info.d_endian;
     hwaddr begin = s->memory_offset - s->note_size;
     int ret;
 
     memset(&phdr, 0, sizeof(Elf64_Phdr));
-    phdr.p_type = cpu_convert_to_target32(PT_NOTE, endian);
-    phdr.p_offset = cpu_convert_to_target64(begin, endian);
+    phdr.p_type = cpu_to_dump32(s, PT_NOTE);
+    phdr.p_offset = cpu_to_dump64(s, begin);
     phdr.p_paddr = 0;
-    phdr.p_filesz = cpu_convert_to_target64(s->note_size, endian);
-    phdr.p_memsz = cpu_convert_to_target64(s->note_size, endian);
+    phdr.p_filesz = cpu_to_dump64(s, s->note_size);
+    phdr.p_memsz = cpu_to_dump64(s, s->note_size);
     phdr.p_vaddr = 0;
 
     ret = fd_write_vmcore(&phdr, sizeof(Elf64_Phdr), s);
@@ -325,15 +281,14 @@ static int write_elf32_note(DumpState *s)
 {
     hwaddr begin = s->memory_offset - s->note_size;
     Elf32_Phdr phdr;
-    int endian = s->dump_info.d_endian;
     int ret;
 
     memset(&phdr, 0, sizeof(Elf32_Phdr));
-    phdr.p_type = cpu_convert_to_target32(PT_NOTE, endian);
-    phdr.p_offset = cpu_convert_to_target32(begin, endian);
+    phdr.p_type = cpu_to_dump32(s, PT_NOTE);
+    phdr.p_offset = cpu_to_dump32(s, begin);
     phdr.p_paddr = 0;
-    phdr.p_filesz = cpu_convert_to_target32(s->note_size, endian);
-    phdr.p_memsz = cpu_convert_to_target32(s->note_size, endian);
+    phdr.p_filesz = cpu_to_dump32(s, s->note_size);
+    phdr.p_memsz = cpu_to_dump32(s, s->note_size);
     phdr.p_vaddr = 0;
 
     ret = fd_write_vmcore(&phdr, sizeof(Elf32_Phdr), s);
@@ -375,7 +330,6 @@ static int write_elf_section(DumpState *s, int type)
 {
     Elf32_Shdr shdr32;
     Elf64_Shdr shdr64;
-    int endian = s->dump_info.d_endian;
     int shdr_size;
     void *shdr;
     int ret;
@@ -383,12 +337,12 @@ static int write_elf_section(DumpState *s, int type)
     if (type == 0) {
         shdr_size = sizeof(Elf32_Shdr);
         memset(&shdr32, 0, shdr_size);
-        shdr32.sh_info = cpu_convert_to_target32(s->sh_info, endian);
+        shdr32.sh_info = cpu_to_dump32(s, s->sh_info);
         shdr = &shdr32;
     } else {
         shdr_size = sizeof(Elf64_Shdr);
         memset(&shdr64, 0, shdr_size);
-        shdr64.sh_info = cpu_convert_to_target32(s->sh_info, endian);
+        shdr64.sh_info = cpu_to_dump32(s, s->sh_info);
         shdr = &shdr64;
     }
 
@@ -796,7 +750,6 @@ static int create_header32(DumpState *s)
     DiskDumpHeader32 *dh = NULL;
     KdumpSubHeader32 *kh = NULL;
     size_t size;
-    int endian = s->dump_info.d_endian;
     uint32_t block_size;
     uint32_t sub_hdr_size;
     uint32_t bitmap_blocks;
@@ -808,18 +761,17 @@ static int create_header32(DumpState *s)
     dh = g_malloc0(size);
 
     strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
-    dh->header_version = cpu_convert_to_target32(6, endian);
+    dh->header_version = cpu_to_dump32(s, 6);
     block_size = s->page_size;
-    dh->block_size = cpu_convert_to_target32(block_size, endian);
+    dh->block_size = cpu_to_dump32(s, block_size);
     sub_hdr_size = sizeof(struct KdumpSubHeader32) + s->note_size;
     sub_hdr_size = DIV_ROUND_UP(sub_hdr_size, block_size);
-    dh->sub_hdr_size = cpu_convert_to_target32(sub_hdr_size, endian);
+    dh->sub_hdr_size = cpu_to_dump32(s, sub_hdr_size);
     /* dh->max_mapnr may be truncated, full 64bit is in kh.max_mapnr_64 */
-    dh->max_mapnr = cpu_convert_to_target32(MIN(s->max_mapnr, UINT_MAX),
-                                            endian);
-    dh->nr_cpus = cpu_convert_to_target32(s->nr_cpus, endian);
+    dh->max_mapnr = cpu_to_dump32(s, MIN(s->max_mapnr, UINT_MAX));
+    dh->nr_cpus = cpu_to_dump32(s, s->nr_cpus);
     bitmap_blocks = DIV_ROUND_UP(s->len_dump_bitmap, block_size) * 2;
-    dh->bitmap_blocks = cpu_convert_to_target32(bitmap_blocks, endian);
+    dh->bitmap_blocks = cpu_to_dump32(s, bitmap_blocks);
     strncpy(dh->utsname.machine, ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
 
     if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
@@ -835,7 +787,7 @@ static int create_header32(DumpState *s)
         status |= DUMP_DH_COMPRESSED_SNAPPY;
     }
 #endif
-    dh->status = cpu_convert_to_target32(status, endian);
+    dh->status = cpu_to_dump32(s, status);
 
     if (write_buffer(s->fd, 0, dh, size) < 0) {
         dump_error(s, "dump: failed to write disk dump header.\n");
@@ -848,13 +800,13 @@ static int create_header32(DumpState *s)
     kh = g_malloc0(size);
 
     /* 64bit max_mapnr_64 */
-    kh->max_mapnr_64 = cpu_convert_to_target64(s->max_mapnr, endian);
-    kh->phys_base = cpu_convert_to_target32(PHYS_BASE, endian);
-    kh->dump_level = cpu_convert_to_target32(DUMP_LEVEL, endian);
+    kh->max_mapnr_64 = cpu_to_dump64(s, s->max_mapnr);
+    kh->phys_base = cpu_to_dump32(s, PHYS_BASE);
+    kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
 
     offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
-    kh->offset_note = cpu_convert_to_target64(offset_note, endian);
-    kh->note_size = cpu_convert_to_target32(s->note_size, endian);
+    kh->offset_note = cpu_to_dump64(s, offset_note);
+    kh->note_size = cpu_to_dump32(s, s->note_size);
 
     if (write_buffer(s->fd, DISKDUMP_HEADER_BLOCKS *
                      block_size, kh, size) < 0) {
@@ -903,7 +855,6 @@ static int create_header64(DumpState *s)
     DiskDumpHeader64 *dh = NULL;
     KdumpSubHeader64 *kh = NULL;
     size_t size;
-    int endian = s->dump_info.d_endian;
     uint32_t block_size;
     uint32_t sub_hdr_size;
     uint32_t bitmap_blocks;
@@ -915,18 +866,17 @@ static int create_header64(DumpState *s)
     dh = g_malloc0(size);
 
     strncpy(dh->signature, KDUMP_SIGNATURE, strlen(KDUMP_SIGNATURE));
-    dh->header_version = cpu_convert_to_target32(6, endian);
+    dh->header_version = cpu_to_dump32(s, 6);
     block_size = s->page_size;
-    dh->block_size = cpu_convert_to_target32(block_size, endian);
+    dh->block_size = cpu_to_dump32(s, block_size);
     sub_hdr_size = sizeof(struct KdumpSubHeader64) + s->note_size;
     sub_hdr_size = DIV_ROUND_UP(sub_hdr_size, block_size);
-    dh->sub_hdr_size = cpu_convert_to_target32(sub_hdr_size, endian);
+    dh->sub_hdr_size = cpu_to_dump32(s, sub_hdr_size);
     /* dh->max_mapnr may be truncated, full 64bit is in kh.max_mapnr_64 */
-    dh->max_mapnr = cpu_convert_to_target32(MIN(s->max_mapnr, UINT_MAX),
-                                            endian);
-    dh->nr_cpus = cpu_convert_to_target32(s->nr_cpus, endian);
+    dh->max_mapnr = cpu_to_dump32(s, MIN(s->max_mapnr, UINT_MAX));
+    dh->nr_cpus = cpu_to_dump32(s, s->nr_cpus);
     bitmap_blocks = DIV_ROUND_UP(s->len_dump_bitmap, block_size) * 2;
-    dh->bitmap_blocks = cpu_convert_to_target32(bitmap_blocks, endian);
+    dh->bitmap_blocks = cpu_to_dump32(s, bitmap_blocks);
     strncpy(dh->utsname.machine, ELF_MACHINE_UNAME, sizeof(dh->utsname.machine));
 
     if (s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) {
@@ -942,7 +892,7 @@ static int create_header64(DumpState *s)
         status |= DUMP_DH_COMPRESSED_SNAPPY;
     }
 #endif
-    dh->status = cpu_convert_to_target32(status, endian);
+    dh->status = cpu_to_dump32(s, status);
 
     if (write_buffer(s->fd, 0, dh, size) < 0) {
         dump_error(s, "dump: failed to write disk dump header.\n");
@@ -955,13 +905,13 @@ static int create_header64(DumpState *s)
     kh = g_malloc0(size);
 
     /* 64bit max_mapnr_64 */
-    kh->max_mapnr_64 = cpu_convert_to_target64(s->max_mapnr, endian);
-    kh->phys_base = cpu_convert_to_target64(PHYS_BASE, endian);
-    kh->dump_level = cpu_convert_to_target32(DUMP_LEVEL, endian);
+    kh->max_mapnr_64 = cpu_to_dump64(s, s->max_mapnr);
+    kh->phys_base = cpu_to_dump64(s, PHYS_BASE);
+    kh->dump_level = cpu_to_dump32(s, DUMP_LEVEL);
 
     offset_note = DISKDUMP_HEADER_BLOCKS * block_size + size;
-    kh->offset_note = cpu_convert_to_target64(offset_note, endian);
-    kh->note_size = cpu_convert_to_target64(s->note_size, endian);
+    kh->offset_note = cpu_to_dump64(s, offset_note);
+    kh->note_size = cpu_to_dump64(s, s->note_size);
 
     if (write_buffer(s->fd, DISKDUMP_HEADER_BLOCKS *
                      block_size, kh, size) < 0) {
@@ -1283,7 +1233,6 @@ static int write_dump_pages(DumpState *s)
     off_t offset_desc, offset_data;
     PageDescriptor pd, pd_zero;
     uint8_t *buf;
-    int endian = s->dump_info.d_endian;
     GuestPhysBlock *block_iter = NULL;
     uint64_t pfn_iter;
 
@@ -1311,10 +1260,10 @@ static int write_dump_pages(DumpState *s)
      * init zero page's page_desc and page_data, because every zero page
      * uses the same page_data
      */
-    pd_zero.size = cpu_convert_to_target32(s->page_size, endian);
-    pd_zero.flags = cpu_convert_to_target32(0, endian);
-    pd_zero.offset = cpu_convert_to_target64(offset_data, endian);
-    pd_zero.page_flags = cpu_convert_to_target64(0, endian);
+    pd_zero.size = cpu_to_dump32(s, s->page_size);
+    pd_zero.flags = cpu_to_dump32(s, 0);
+    pd_zero.offset = cpu_to_dump64(s, offset_data);
+    pd_zero.page_flags = cpu_to_dump64(s, 0);
     buf = g_malloc0(s->page_size);
     ret = write_cache(&page_data, buf, s->page_size, false);
     g_free(buf);
@@ -1354,9 +1303,8 @@ static int write_dump_pages(DumpState *s)
              if ((s->flag_compress & DUMP_DH_COMPRESSED_ZLIB) &&
                     (compress2(buf_out, (uLongf *)&size_out, buf, s->page_size,
                     Z_BEST_SPEED) == Z_OK) && (size_out < s->page_size)) {
-                pd.flags = cpu_convert_to_target32(DUMP_DH_COMPRESSED_ZLIB,
-                                                   endian);
-                pd.size  = cpu_convert_to_target32(size_out, endian);
+                pd.flags = cpu_to_dump32(s, DUMP_DH_COMPRESSED_ZLIB);
+                pd.size  = cpu_to_dump32(s, size_out);
 
                 ret = write_cache(&page_data, buf_out, size_out, false);
                 if (ret < 0) {
@@ -1368,9 +1316,8 @@ static int write_dump_pages(DumpState *s)
                     (lzo1x_1_compress(buf, s->page_size, buf_out,
                     (lzo_uint *)&size_out, wrkmem) == LZO_E_OK) &&
                     (size_out < s->page_size)) {
-                pd.flags = cpu_convert_to_target32(DUMP_DH_COMPRESSED_LZO,
-                                                   endian);
-                pd.size  = cpu_convert_to_target32(size_out, endian);
+                pd.flags = cpu_to_dump32(s, DUMP_DH_COMPRESSED_LZO);
+                pd.size  = cpu_to_dump32(s, size_out);
 
                 ret = write_cache(&page_data, buf_out, size_out, false);
                 if (ret < 0) {
@@ -1383,9 +1330,8 @@ static int write_dump_pages(DumpState *s)
                     (snappy_compress((char *)buf, s->page_size,
                     (char *)buf_out, &size_out) == SNAPPY_OK) &&
                     (size_out < s->page_size)) {
-                pd.flags = cpu_convert_to_target32(
-                                        DUMP_DH_COMPRESSED_SNAPPY, endian);
-                pd.size  = cpu_convert_to_target32(size_out, endian);
+                pd.flags = cpu_to_dump32(s, DUMP_DH_COMPRESSED_SNAPPY);
+                pd.size  = cpu_to_dump32(s, size_out);
 
                 ret = write_cache(&page_data, buf_out, size_out, false);
                 if (ret < 0) {
@@ -1398,9 +1344,9 @@ static int write_dump_pages(DumpState *s)
                  * fall back to save in plaintext, size_out should be
                  * assigned to s->page_size
                  */
-                pd.flags = cpu_convert_to_target32(0, endian);
+                pd.flags = cpu_to_dump32(s, 0);
                 size_out = s->page_size;
-                pd.size = cpu_convert_to_target32(size_out, endian);
+                pd.size = cpu_to_dump32(s, size_out);
 
                 ret = write_cache(&page_data, buf, s->page_size, false);
                 if (ret < 0) {
@@ -1410,8 +1356,8 @@ static int write_dump_pages(DumpState *s)
             }
 
             /* get and write page desc here */
-            pd.page_flags = cpu_convert_to_target64(0, endian);
-            pd.offset = cpu_convert_to_target64(offset_data, endian);
+            pd.page_flags = cpu_to_dump64(s, 0);
+            pd.offset = cpu_to_dump64(s, offset_data);
             offset_data += size_out;
 
             ret = write_cache(&page_desc, &pd, sizeof(PageDescriptor), false);
include/sysemu/dump-arch.h new file +28 -0
new file mode 100644
index 0000000..9c95ced
@@ -0,0 +1,28 @@
+/*
+ * QEMU dump
+ *
+ * Copyright Fujitsu, Corp. 2011, 2012
+ *
+ * Authors:
+ *     Wen Congyang <[email protected]>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ *
+ */
+
+#ifndef DUMP_ARCH_H
+#define DUMP_ARCH_H
+
+typedef struct ArchDumpInfo {
+    int d_machine;  /* Architecture */
+    int d_endian;   /* ELFDATA2LSB or ELFDATA2MSB */
+    int d_class;    /* ELFCLASS32 or ELFCLASS64 */
+} ArchDumpInfo;
+
+struct GuestPhysBlockList; /* memory_mapping.h */
+int cpu_get_dump_info(ArchDumpInfo *info,
+                      const struct GuestPhysBlockList *guest_phys_blocks);
+ssize_t cpu_get_note_size(int class, int machine, int nr_cpus);
+
+#endif
include/sysemu/dump.h +38 -10

58 lines not on this page · raw message

stubs/dump.c +1 -1
index 370cd96..fac7019 100644
@@ -12,7 +12,7 @@
  */
 
 #include "qemu-common.h"
-#include "sysemu/dump.h"
+#include "sysemu/dump-arch.h"
 #include "qapi/qmp/qerror.h"
 #include "qmp-commands.h"
 
author
#2 lore raw

PATCH v3 2/4 ppc64-dump: Support dump for little endian ppc64

In reply to Greg Kurz

From: Bharata B Rao <[email protected]>

Fix ppc64 arch specific dump code to work correctly for little endian guests.

We introduce a NoteFuncArg type to avoid adding extra arguments to all note functions.

[ rebased on top of current master branch,
introduced NoteFuncArg,
use new cpu_to_dump{16,32,64} endian helpers,
Greg Kurz <[email protected]> ]

---

Changes in v3:
- better taste with the endian helpers naming

 target-ppc/arch_dump.c |   82 +++++++++++++++++++++++++++++-------------------
 1 file changed, 49 insertions(+), 33 deletions(-)
target-ppc/arch_dump.c +49 -33
index 9dccf1a..5487b61 100644
@@ -79,94 +79,109 @@ typedef struct noteStruct {
     } contents;
 } QEMU_PACKED Note;
 
+typedef struct NoteFuncArg {
+    Note note;
+    DumpState *state;
+} NoteFuncArg;
 
-static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
 {
     int i;
     uint64_t cr;
     struct PPC64ElfPrstatus *prstatus;
     struct PPC64UserRegStruct *reg;
+    Note *note = &arg->note;
+    DumpState *s = arg->state;
 
-    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
+    note->hdr.n_type = cpu_to_dump32(s, NT_PRSTATUS);
 
     prstatus = &note->contents.prstatus;
     memset(prstatus, 0, sizeof(*prstatus));
     reg = &prstatus->pr_reg;
 
     for (i = 0; i < 32; i++) {
-        reg->gpr[i] = cpu_to_be64(cpu->env.gpr[i]);
+        reg->gpr[i] = cpu_to_dump64(s, cpu->env.gpr[i]);
     }
-    reg->nip = cpu_to_be64(cpu->env.nip);
-    reg->msr = cpu_to_be64(cpu->env.msr);
-    reg->ctr = cpu_to_be64(cpu->env.ctr);
-    reg->link = cpu_to_be64(cpu->env.lr);
-    reg->xer = cpu_to_be64(cpu_read_xer(&cpu->env));
+    reg->nip = cpu_to_dump64(s, cpu->env.nip);
+    reg->msr = cpu_to_dump64(s, cpu->env.msr);
+    reg->ctr = cpu_to_dump64(s, cpu->env.ctr);
+    reg->link = cpu_to_dump64(s, cpu->env.lr);
+    reg->xer = cpu_to_dump64(s, cpu_read_xer(&cpu->env));
 
     cr = 0;
     for (i = 0; i < 8; i++) {
         cr |= (cpu->env.crf[i] & 15) << (4 * (7 - i));
     }
-    reg->ccr = cpu_to_be64(cr);
+    reg->ccr = cpu_to_dump64(s, cr);
 }
 
-static void ppc64_write_elf64_fpregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu)
 {
     int i;
     struct PPC64ElfFpregset  *fpregset;
+    Note *note = &arg->note;
+    DumpState *s = arg->state;
 
-    note->hdr.n_type = cpu_to_be32(NT_PRFPREG);
+    note->hdr.n_type = cpu_to_dump32(s, NT_PRFPREG);
 
     fpregset = &note->contents.fpregset;
     memset(fpregset, 0, sizeof(*fpregset));
 
     for (i = 0; i < 32; i++) {
-        fpregset->fpr[i] = cpu_to_be64(cpu->env.fpr[i]);
+        fpregset->fpr[i] = cpu_to_dump64(s, cpu->env.fpr[i]);
     }
-    fpregset->fpscr = cpu_to_be64(cpu->env.fpscr);
+    fpregset->fpscr = cpu_to_dump64(s, cpu->env.fpscr);
 }
 
-static void ppc64_write_elf64_vmxregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
 {
     int i;
     struct PPC64ElfVmxregset *vmxregset;
+    Note *note = &arg->note;
+    DumpState *s = arg->state;
 
-    note->hdr.n_type = cpu_to_be32(NT_PPC_VMX);
+    note->hdr.n_type = cpu_to_dump32(s, NT_PPC_VMX);
     vmxregset = &note->contents.vmxregset;
     memset(vmxregset, 0, sizeof(*vmxregset));
 
     for (i = 0; i < 32; i++) {
-        vmxregset->avr[i].u64[0] = cpu_to_be64(cpu->env.avr[i].u64[0]);
-        vmxregset->avr[i].u64[1] = cpu_to_be64(cpu->env.avr[i].u64[1]);
+        vmxregset->avr[i].u64[0] = cpu_to_dump64(s, cpu->env.avr[i].u64[0]);
+        vmxregset->avr[i].u64[1] = cpu_to_dump64(s, cpu->env.avr[i].u64[1]);
     }
-    vmxregset->vscr.u32[3] = cpu_to_be32(cpu->env.vscr);
+    vmxregset->vscr.u32[3] = cpu_to_dump32(s, cpu->env.vscr);
 }
-static void ppc64_write_elf64_vsxregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
 {
     int i;
     struct PPC64ElfVsxregset *vsxregset;
+    Note *note = &arg->note;
+    DumpState *s = arg->state;
 
-    note->hdr.n_type = cpu_to_be32(NT_PPC_VSX);
+    note->hdr.n_type = cpu_to_dump32(s, NT_PPC_VSX);
     vsxregset = &note->contents.vsxregset;
     memset(vsxregset, 0, sizeof(*vsxregset));
 
     for (i = 0; i < 32; i++) {
-        vsxregset->vsr[i] = cpu_to_be64(cpu->env.vsr[i]);
+        vsxregset->vsr[i] = cpu_to_dump64(s, cpu->env.vsr[i]);
     }
 }
-static void ppc64_write_elf64_speregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_speregset(NoteFuncArg *arg, PowerPCCPU *cpu)
 {
     struct PPC64ElfSperegset *speregset;
-    note->hdr.n_type = cpu_to_be32(NT_PPC_SPE);
+    Note *note = &arg->note;
+    DumpState *s = arg->state;
+
+    note->hdr.n_type = cpu_to_dump32(s, NT_PPC_SPE);
     speregset = &note->contents.speregset;
     memset(speregset, 0, sizeof(*speregset));
 
-    speregset->spe_acc = cpu_to_be64(cpu->env.spe_acc);
-    speregset->spe_fscr = cpu_to_be32(cpu->env.spe_fscr);
+    speregset->spe_acc = cpu_to_dump64(s, cpu->env.spe_acc);
+    speregset->spe_fscr = cpu_to_dump32(s, cpu->env.spe_fscr);
 }
 
 static const struct NoteFuncDescStruct {
     int contents_size;
-    void (*note_contents_func)(Note *note, PowerPCCPU *cpu);
+    void (*note_contents_func)(NoteFuncArg *arg, PowerPCCPU *cpu);
 } note_func[] = {
     {sizeof(((Note *)0)->contents.prstatus),  ppc64_write_elf64_prstatus},
     {sizeof(((Note *)0)->contents.fpregset),  ppc64_write_elf64_fpregset},
@@ -218,20 +233,21 @@ static int ppc64_write_all_elf64_notes(const char *note_name,
                                        PowerPCCPU *cpu, int id,
                                        void *opaque)
 {
-    Note note;
+    NoteFuncArg arg = { .state = opaque };
     int ret = -1;
     int note_size;
     const NoteFuncDesc *nf;
 
     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));
+        arg.note.hdr.n_namesz = cpu_to_dump32(opaque, sizeof(arg.note.name));
+        arg.note.hdr.n_descsz = cpu_to_dump32(opaque, nf->contents_size);
+        strncpy(arg.note.name, note_name, sizeof(arg.note.name));
 
-        (*nf->note_contents_func)(&note, cpu);
+        (*nf->note_contents_func)(&arg, cpu);
 
-        note_size = sizeof(note) - sizeof(note.contents) + nf->contents_size;
-        ret = f(&note, note_size, opaque);
+        note_size =
+            sizeof(arg.note) - sizeof(arg.note.contents) + nf->contents_size;
+        ret = f(&arg.note, note_size, opaque);
         if (ret < 0) {
             return -1;
         }
#3 lore raw

Re: [Qemu-devel] [PATCH v3 2/4] ppc64-dump: Support dump for little endian ppc64

Quoted 106 lines · Greg Kurz

From: Bharata B Rao <[email protected]>

Fix ppc64 arch specific dump code to work correctly for little endian guests.

We introduce a NoteFuncArg type to avoid adding extra arguments to all note functions.

[ rebased on top of current master branch,
introduced NoteFuncArg,
use new cpu_to_dump{16,32,64} endian helpers,
Greg Kurz <[email protected]> ]

---

Changes in v3:
- better taste with the endian helpers naming

  target-ppc/arch_dump.c |   82 +++++++++++++++++++++++++++++-------------------
  1 file changed, 49 insertions(+), 33 deletions(-)
target-ppc/arch_dump.c +24 -14
index 9dccf1a..5487b61 100644
@@ -79,94 +79,109 @@ typedef struct noteStruct {
      } contents;
  } QEMU_PACKED Note;
 
+typedef struct NoteFuncArg {
+    Note note;
+    DumpState *state;
+} NoteFuncArg;
 
-static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
  {
      int i;
      uint64_t cr;
      struct PPC64ElfPrstatus *prstatus;
      struct PPC64UserRegStruct *reg;
+    Note *note = &arg->note;
+    DumpState *s = arg->state;
 
-    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
+    note->hdr.n_type = cpu_to_dump32(s, NT_PRSTATUS);
 
      prstatus = &note->contents.prstatus;
      memset(prstatus, 0, sizeof(*prstatus));
      reg = &prstatus->pr_reg;
 
      for (i = 0; i < 32; i++) {
-        reg->gpr[i] = cpu_to_be64(cpu->env.gpr[i]);
+        reg->gpr[i] = cpu_to_dump64(s, cpu->env.gpr[i]);
      }
-    reg->nip = cpu_to_be64(cpu->env.nip);
-    reg->msr = cpu_to_be64(cpu->env.msr);
-    reg->ctr = cpu_to_be64(cpu->env.ctr);
-    reg->link = cpu_to_be64(cpu->env.lr);
-    reg->xer = cpu_to_be64(cpu_read_xer(&cpu->env));
+    reg->nip = cpu_to_dump64(s, cpu->env.nip);
+    reg->msr = cpu_to_dump64(s, cpu->env.msr);
+    reg->ctr = cpu_to_dump64(s, cpu->env.ctr);
+    reg->link = cpu_to_dump64(s, cpu->env.lr);
+    reg->xer = cpu_to_dump64(s, cpu_read_xer(&cpu->env));
 
      cr = 0;
      for (i = 0; i < 8; i++) {
          cr |= (cpu->env.crf[i] & 15) << (4 * (7 - i));
      }
-    reg->ccr = cpu_to_be64(cr);
+    reg->ccr = cpu_to_dump64(s, cr);
  }
 
-static void ppc64_write_elf64_fpregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu)
  {
      int i;
      struct PPC64ElfFpregset  *fpregset;
+    Note *note = &arg->note;
+    DumpState *s = arg->state;
 
-    note->hdr.n_type = cpu_to_be32(NT_PRFPREG);
+    note->hdr.n_type = cpu_to_dump32(s, NT_PRFPREG);
 
      fpregset = &note->contents.fpregset;
      memset(fpregset, 0, sizeof(*fpregset));
 
      for (i = 0; i < 32; i++) {
-        fpregset->fpr[i] = cpu_to_be64(cpu->env.fpr[i]);
+        fpregset->fpr[i] = cpu_to_dump64(s, cpu->env.fpr[i]);
      }
-    fpregset->fpscr = cpu_to_be64(cpu->env.fpscr);
+    fpregset->fpscr = cpu_to_dump64(s, cpu->env.fpscr);
  }
 
-static void ppc64_write_elf64_vmxregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
  {
      int i;
      struct PPC64ElfVmxregset *vmxregset;
+    Note *note = &arg->note;
+    DumpState *s = arg->state;
 

target-ppc/arch_dump.c typedef struct noteStruct { · line 149

Quoted 10 lines
target-ppc/arch_dump.c +3 -3
-    note->hdr.n_type = cpu_to_be32(NT_PPC_VMX);
+    note->hdr.n_type = cpu_to_dump32(s, NT_PPC_VMX);
      vmxregset = &note->contents.vmxregset;
      memset(vmxregset, 0, sizeof(*vmxregset));
 
      for (i = 0; i < 32; i++) {
-        vmxregset->avr[i].u64[0] = cpu_to_be64(cpu->env.avr[i].u64[0]);
-        vmxregset->avr[i].u64[1] = cpu_to_be64(cpu->env.avr[i].u64[1]);
+        vmxregset->avr[i].u64[0] = cpu_to_dump64(s, cpu->env.avr[i].u64[0]);
+        vmxregset->avr[i].u64[1] = cpu_to_dump64(s, cpu->env.avr[i].u64[1]);

Is this correct? Tom, could you please ack if it is?

Alex

author
#4 lore raw

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 2/4] ppc64-dump: Support dump for little endian ppc64

Quoted 121 lines · On Mon, 05 May 2014 13:04:35 +0200 Alexander Graf <[email protected]> wrote:
Quoted 106 lines · Greg Kurz

From: Bharata B Rao <[email protected]>

Fix ppc64 arch specific dump code to work correctly for little endian guests.

We introduce a NoteFuncArg type to avoid adding extra arguments to all note functions.

[ rebased on top of current master branch,
introduced NoteFuncArg,
use new cpu_to_dump{16,32,64} endian helpers,
Greg Kurz <[email protected]> ]

---

Changes in v3:
- better taste with the endian helpers naming

  target-ppc/arch_dump.c |   82 +++++++++++++++++++++++++++++-------------------
  1 file changed, 49 insertions(+), 33 deletions(-)
target-ppc/arch_dump.c +24 -14
index 9dccf1a..5487b61 100644
@@ -79,94 +79,109 @@ typedef struct noteStruct {
      } contents;
  } QEMU_PACKED Note;
 
+typedef struct NoteFuncArg {
+    Note note;
+    DumpState *state;
+} NoteFuncArg;
 
-static void ppc64_write_elf64_prstatus(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)
  {
      int i;
      uint64_t cr;
      struct PPC64ElfPrstatus *prstatus;
      struct PPC64UserRegStruct *reg;
+    Note *note = &arg->note;
+    DumpState *s = arg->state;
 
-    note->hdr.n_type = cpu_to_be32(NT_PRSTATUS);
+    note->hdr.n_type = cpu_to_dump32(s, NT_PRSTATUS);
 
      prstatus = &note->contents.prstatus;
      memset(prstatus, 0, sizeof(*prstatus));
      reg = &prstatus->pr_reg;
 
      for (i = 0; i < 32; i++) {
-        reg->gpr[i] = cpu_to_be64(cpu->env.gpr[i]);
+        reg->gpr[i] = cpu_to_dump64(s, cpu->env.gpr[i]);
      }
-    reg->nip = cpu_to_be64(cpu->env.nip);
-    reg->msr = cpu_to_be64(cpu->env.msr);
-    reg->ctr = cpu_to_be64(cpu->env.ctr);
-    reg->link = cpu_to_be64(cpu->env.lr);
-    reg->xer = cpu_to_be64(cpu_read_xer(&cpu->env));
+    reg->nip = cpu_to_dump64(s, cpu->env.nip);
+    reg->msr = cpu_to_dump64(s, cpu->env.msr);
+    reg->ctr = cpu_to_dump64(s, cpu->env.ctr);
+    reg->link = cpu_to_dump64(s, cpu->env.lr);
+    reg->xer = cpu_to_dump64(s, cpu_read_xer(&cpu->env));
 
      cr = 0;
      for (i = 0; i < 8; i++) {
          cr |= (cpu->env.crf[i] & 15) << (4 * (7 - i));
      }
-    reg->ccr = cpu_to_be64(cr);
+    reg->ccr = cpu_to_dump64(s, cr);
  }
 
-static void ppc64_write_elf64_fpregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu)
  {
      int i;
      struct PPC64ElfFpregset  *fpregset;
+    Note *note = &arg->note;
+    DumpState *s = arg->state;
 
-    note->hdr.n_type = cpu_to_be32(NT_PRFPREG);
+    note->hdr.n_type = cpu_to_dump32(s, NT_PRFPREG);
 
      fpregset = &note->contents.fpregset;
      memset(fpregset, 0, sizeof(*fpregset));
 
      for (i = 0; i < 32; i++) {
-        fpregset->fpr[i] = cpu_to_be64(cpu->env.fpr[i]);
+        fpregset->fpr[i] = cpu_to_dump64(s, cpu->env.fpr[i]);
      }
-    fpregset->fpscr = cpu_to_be64(cpu->env.fpscr);
+    fpregset->fpscr = cpu_to_dump64(s, cpu->env.fpscr);
  }
 
-static void ppc64_write_elf64_vmxregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
  {
      int i;
      struct PPC64ElfVmxregset *vmxregset;
+    Note *note = &arg->note;
+    DumpState *s = arg->state;
 

target-ppc/arch_dump.c typedef struct noteStruct { · line 149

Quoted 10 lines
target-ppc/arch_dump.c +3 -3
-    note->hdr.n_type = cpu_to_be32(NT_PPC_VMX);
+    note->hdr.n_type = cpu_to_dump32(s, NT_PPC_VMX);
      vmxregset = &note->contents.vmxregset;
      memset(vmxregset, 0, sizeof(*vmxregset));
 
      for (i = 0; i < 32; i++) {
-        vmxregset->avr[i].u64[0] = cpu_to_be64(cpu->env.avr[i].u64[0]);
-        vmxregset->avr[i].u64[1] = cpu_to_be64(cpu->env.avr[i].u64[1]);
+        vmxregset->avr[i].u64[0] = cpu_to_dump64(s, cpu->env.avr[i].u64[0]);
+        vmxregset->avr[i].u64[1] = cpu_to_dump64(s, cpu->env.avr[i].u64[1]);

Is this correct? Tom, could you please ack if it is?

Quoted 3 lines

Alex

Tom,

Can you comment plz ?

Thanks.

Signature · 7 lines
Gregory Kurz                                     [email protected]
                                                 [email protected]
Software Engineer @ IBM/Meiosys                  http://www.ibm.com
Tel +33 (0)562 165 496

"Anarchy is about taking complete responsibility for yourself."
        Alan Moore.
#5 lore raw

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 2/4] ppc64-dump: Support dump for little endian ppc64

Quoted 12 lines · Greg Kurz
Quoted 10 lines · On Mon, 05 May 2014 13:04:35 +0200 Alexander Graf <[email protected]> wrote:
Quoted 9 lines · Greg Kurz

From: Bharata B Rao <[email protected]>

Fix ppc64 arch specific dump code to work correctly for little endian guests.

We introduce a NoteFuncArg type to avoid adding extra arguments to all note functions.

[snip]

Quoted 26 lines
Quoted 22 lines
Quoted 8 lines
target-ppc/arch_dump.c +3 -1
-static void ppc64_write_elf64_vmxregset(Note *note, PowerPCCPU *cpu)
+static void ppc64_write_elf64_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)
  {
      int i;
      struct PPC64ElfVmxregset *vmxregset;
+    Note *note = &arg->note;
+    DumpState *s = arg->state;
 

target-ppc/arch_dump.c typedef struct noteStruct { · line 149

Quoted 10 lines
target-ppc/arch_dump.c +3 -3
-    note->hdr.n_type = cpu_to_be32(NT_PPC_VMX);
+    note->hdr.n_type = cpu_to_dump32(s, NT_PPC_VMX);
      vmxregset = &note->contents.vmxregset;
      memset(vmxregset, 0, sizeof(*vmxregset));
 
      for (i = 0; i < 32; i++) {
-        vmxregset->avr[i].u64[0] = cpu_to_be64(cpu->env.avr[i].u64[0]);
-        vmxregset->avr[i].u64[1] = cpu_to_be64(cpu->env.avr[i].u64[1]);
+        vmxregset->avr[i].u64[0] = cpu_to_dump64(s, cpu->env.avr[i].u64[0]);
+        vmxregset->avr[i].u64[1] = cpu_to_dump64(s, cpu->env.avr[i].u64[1]);

Is this correct? Tom, could you please ack if it is?

Quoted 3 lines

Alex

Quoted 6 lines

Tom,

Can you comment plz ?

Thanks.

Is the avr element in the dump supposed to represent a 128-bit value?

Or maybe I should ask a more fundamental question: should a dump of a BE guest be identical regardless of whether it is a BE host or LE host. Ditto for an LE guest.

It feels like there is an endianness issue here but I have not yet been able to put my finger on it.

#6 lore raw

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 2/4] ppc64-dump: Support dump for little endian ppc64

Quoted 2 lines · Tom Musta

It feels like there is an endianness issue here but I have not yet been able to put my finger on it.

OK ... after more thought and scribbling ... here is what I mean ....

Suppose I have a 64-bit value 0xa0a1a2a3a4a5a6a7 stored in guest memory and it gets loaded into a GPR. If I follow the dump code and view all four combinations of guest/host big/little endian, I convince myself that the big endian guest code writes the byte sequence 0xa0, 0xa1, 0xa2, ..., 0xa7 into the file. And the little endian guest dumps contain the sequence 0xa7, 0xa6, ..., 0xa0.

This make sense ... the endianness indicated in the dump header and the endianness of the dump file layout are consistent, irrespective of the host endianness.

If I take this a step further and consider a 128-bit value 0xa0a1a2a3a4a5a6a7a8a9aaabacadaeaf stored in guest memory and look at the AVR structure (via printf or debugger) after doing a 128-bit lvx load, I get the following:

  +------+-------+------------------+------------------+---------------------+
  | Host | Guest | avr.u64[0]       | avr.u64[1]       | file sequence       |
  +------+-------+------------------+------------------+---------------------+
  | BE   | BE    | a0a1a2a3a4a5a6a7 | a8a9aaabacadaeaf | a0,...,a7,a8,...,af |
  | LE   | BE    | a8a9aaabacadaeaf | a0a1a2a3a4a5a6a7 | a8,...,af,a0,...,a7 |
  | BE   | LE    | a0a1a2a3a4a5a6a7 | a8a9aaabacadaeaf | a7,...,a0,af,...,a8 |
  | LE   | LE    | a8a9aaabacadaeaf | a0a1a2a3a4a5a6a7 | af,...,a8,a7,...,a0 |
  +------+-------+------------------+------------------+---------------------+

The last column represents how I think the proposed dump code will write bytes to disk. Notice that if you invert the (64-bit) array elements, then the two BE dumps look alike and the two LE dumps look alike. If you swap array u64 elements on LE hosts, and also swap on LE guests, then you get a byte sequence that looks like a 128-bit integer in all cases.

#7 lore raw

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 2/4] ppc64-dump: Support dump for little endian ppc64

Quoted 28 lines · Tom Musta
Quoted 2 lines · Tom Musta

It feels like there is an endianness issue here but I have not yet been able to put my finger on it.

OK ... after more thought and scribbling ... here is what I mean ....

Suppose I have a 64-bit value 0xa0a1a2a3a4a5a6a7 stored in guest memory and it gets loaded into a GPR. If I follow the dump code and view all four combinations of guest/host big/little endian, I convince myself that the big endian guest code writes the byte sequence 0xa0, 0xa1, 0xa2, ..., 0xa7 into the file. And the little endian guest dumps contain the sequence 0xa7, 0xa6, ..., 0xa0.

This make sense ... the endianness indicated in the dump header and the endianness of the dump file layout are consistent, irrespective of the host endianness.

If I take this a step further and consider a 128-bit value 0xa0a1a2a3a4a5a6a7a8a9aaabacadaeaf stored in guest memory and look at the AVR structure (via printf or debugger) after doing a 128-bit lvx load, I get the following:

   +------+-------+------------------+------------------+---------------------+
   | Host | Guest | avr.u64[0]       | avr.u64[1]       | file sequence       |
   +------+-------+------------------+------------------+---------------------+
   | BE   | BE    | a0a1a2a3a4a5a6a7 | a8a9aaabacadaeaf | a0,...,a7,a8,...,af |
   | LE   | BE    | a8a9aaabacadaeaf | a0a1a2a3a4a5a6a7 | a8,...,af,a0,...,a7 |
   | BE   | LE    | a0a1a2a3a4a5a6a7 | a8a9aaabacadaeaf | a7,...,a0,af,...,a8 |
   | LE   | LE    | a8a9aaabacadaeaf | a0a1a2a3a4a5a6a7 | af,...,a8,a7,...,a0 |
   +------+-------+------------------+------------------+---------------------+
Quoted 5 lines

The last column represents how I think the proposed dump code will write bytes to disk. Notice that if you invert the (64-bit) array elements, then the two BE dumps look alike and the two LE dumps look alike. If you swap array u64 elements on LE hosts, and also swap on LE guests, then you get a byte sequence that looks like a 128-bit integer in all cases.

Since we're dumping an ELF core dump the big question is what gdb wants to load :)

Alex

author
#8 lore raw

Re: [Qemu-devel] [Qemu-ppc] [PATCH v3 2/4] ppc64-dump: Support dump for little endian ppc64

In reply to Tom Musta
Quoted 29 lines · On Wed, 07 May 2014 15:54:10 -0500 Tom Musta <[email protected]> wrote:
Quoted 2 lines · Tom Musta

It feels like there is an endianness issue here but I have not yet been able to put my finger on it.

OK ... after more thought and scribbling ... here is what I mean ....

Suppose I have a 64-bit value 0xa0a1a2a3a4a5a6a7 stored in guest memory and it gets loaded into a GPR. If I follow the dump code and view all four combinations of guest/host big/little endian, I convince myself that the big endian guest code writes the byte sequence 0xa0, 0xa1, 0xa2, ..., 0xa7 into the file. And the little endian guest dumps contain the sequence 0xa7, 0xa6, ..., 0xa0.

This make sense ... the endianness indicated in the dump header and the endianness of the dump file layout are consistent, irrespective of the host endianness.

If I take this a step further and consider a 128-bit value 0xa0a1a2a3a4a5a6a7a8a9aaabacadaeaf stored in guest memory and look at the AVR structure (via printf or debugger) after doing a 128-bit lvx load, I get the following:

  +------+-------+------------------+------------------+---------------------+
  | Host | Guest | avr.u64[0]       | avr.u64[1]       | file sequence       |
  +------+-------+------------------+------------------+---------------------+
  | BE   | BE    | a0a1a2a3a4a5a6a7 | a8a9aaabacadaeaf | a0,...,a7,a8,...,af |
  | LE   | BE    | a8a9aaabacadaeaf | a0a1a2a3a4a5a6a7 | a8,...,af,a0,...,a7 |
  | BE   | LE    | a0a1a2a3a4a5a6a7 | a8a9aaabacadaeaf | a7,...,a0,af,...,a8 |
  | LE   | LE    | a8a9aaabacadaeaf | a0a1a2a3a4a5a6a7 | af,...,a8,a7,...,a0 |
  +------+-------+------------------+------------------+---------------------+
Quoted 7 lines

The last column represents how I think the proposed dump code will write bytes to disk. Notice that if you invert the (64-bit) array elements, then the two BE dumps look alike and the two LE dumps look alike. If you swap array u64 elements on LE hosts, and also swap on LE guests, then you get a byte sequence that looks like a 128-bit integer in all cases.

Then we already have an issue with the current code...

Signature · 7 lines
Gregory Kurz                                     [email protected]
                                                 [email protected]
Software Engineer @ IBM/Meiosys                  http://www.ibm.com
Tel +33 (0)562 165 496

"Anarchy is about taking complete responsibility for yourself."
        Alan Moore.
author
#9 lore raw

PATCH v3 3/4 target-ppc: ppc can be either endian

In reply to Greg Kurz

POWER7, POWER7+ and POWER8 families use the ILE bit of the LPCR special purpose register to decide the endianness to use when entering interrupt handlers. When running a Linux guest, this provides a hint on the endianness used by the kernel. From a QEMU point of view, the information is needed for legacy virtio support and crash dump support as well.

---

No changes, resent in case it helps

 target-ppc/cpu-qom.h        |    1 +
 target-ppc/translate_init.c |   16 ++++++++++++++++
 2 files changed, 17 insertions(+)
target-ppc/cpu-qom.h +1 -0
index 47dc8e6..fdce638 100644
@@ -76,6 +76,7 @@ typedef struct PowerPCCPUClass {
     int (*handle_mmu_fault)(PowerPCCPU *cpu, target_ulong eaddr, int rwx,
                             int mmu_idx);
 #endif
+    bool (*interrupts_big_endian)(PowerPCCPU *cpu);
 } PowerPCCPUClass;
 
 /**
target-ppc/translate_init.c +16 -0
index 7f53c33..a27e5a7 100644
@@ -3102,6 +3102,18 @@ static int check_pow_hid0_74xx (CPUPPCState *env)
     return 0;
 }
 
+static bool ppc_cpu_interrupts_big_endian_always(PowerPCCPU *cpu)
+{
+    return true;
+}
+
+#ifdef TARGET_PPC64
+static bool ppc_cpu_interrupts_big_endian_lpcr(PowerPCCPU *cpu)
+{
+    return !(cpu->env.spr[SPR_LPCR] & LPCR_ILE);
+}
+#endif
+
 /*****************************************************************************/
 /* PowerPC implementations definitions                                       */
 
@@ -7089,6 +7101,7 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
                  POWERPC_FLAG_VSX;
     pcc->l1_dcache_size = 0x8000;
     pcc->l1_icache_size = 0x8000;
+    pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
 }
 
 POWERPC_FAMILY(POWER7P)(ObjectClass *oc, void *data)
@@ -7132,6 +7145,7 @@ POWERPC_FAMILY(POWER7P)(ObjectClass *oc, void *data)
                  POWERPC_FLAG_VSX;
     pcc->l1_dcache_size = 0x8000;
     pcc->l1_icache_size = 0x8000;
+    pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
 }
 
 static void init_proc_POWER8(CPUPPCState *env)
@@ -7189,6 +7203,7 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
                  POWERPC_FLAG_VSX;
     pcc->l1_dcache_size = 0x8000;
     pcc->l1_icache_size = 0x8000;
+    pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
 }
 #endif /* defined (TARGET_PPC64) */
 
@@ -8511,6 +8526,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     pcc->parent_realize = dc->realize;
     pcc->pvr = CPU_POWERPC_DEFAULT_MASK;
     pcc->pvr_mask = CPU_POWERPC_DEFAULT_MASK;
+    pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_always;
     dc->realize = ppc_cpu_realizefn;
     dc->unrealize = ppc_cpu_unrealizefn;
 
#10 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

Quoted 6 lines · Greg Kurz

POWER7, POWER7+ and POWER8 families use the ILE bit of the LPCR special purpose register to decide the endianness to use when entering interrupt handlers. When running a Linux guest, this provides a hint on the endianness used by the kernel. From a QEMU point of view, the information is needed for legacy virtio support and crash dump support as well.

Do you care about the case of:
* kernel bigendian
* userspace littleendian (or vice-versa)
* guest kernel passes virtio device through to guest userspace
* guest userspace is doing the manipulation of the device

?

(Will Deacon just suggested this as a possibility on the kvm-arm mailing list...)

Also, are we documenting what the process should be for a virtio implementation to decide the endianness for a particular architecture? I assume we'd like kvmtool and QEMU to do the same thing rather than subtly different things...

thanks
-- PMM

author
#11 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

Quoted 10 lines · On Tue, 6 May 2014 19:37:22 +0100 Peter Maydell <[email protected]>…
Quoted 6 lines · Greg Kurz

POWER7, POWER7+ and POWER8 families use the ILE bit of the LPCR special purpose register to decide the endianness to use when entering interrupt handlers. When running a Linux guest, this provides a hint on the endianness used by the kernel. From a QEMU point of view, the information is needed for legacy virtio support and crash dump support as well.

Do you care about the case of:
* kernel bigendian

Yes. FWIW, ppc64 is still widely used in big endian mode we don't want to break.

Quoted 1 line

* userspace littleendian (or vice-versa)

We don't care about userspace here. We assume that virtio structures are owned by the guest kernel.

Quoted 1 line

* guest kernel passes virtio device through to guest userspace

Not sure to understand... could you please point me to an example ?

Quoted 2 lines

* guest userspace is doing the manipulation of the device

Hmm... you mean we would have virtio drivers implemented in the guest userspace ? Does that exist ? Please elaborate.

Quoted 5 lines

?

(Will Deacon just suggested this as a possibility on the kvm-arm mailing list...)

Just discovered some virtio endian threads in the kvm-arm@ archives... I'll take some time to read.

Quoted 5 lines

Also, are we documenting what the process should be for a virtio implementation to decide the endianness for a particular architecture? I assume we'd like kvmtool and QEMU to do the same thing rather than subtly different things...

Sure !

Quoted 3 lines

thanks
-- PMM

Thanks.

Signature · 7 lines
Gregory Kurz                                     [email protected]
                                                 [email protected]
Software Engineer @ IBM/Meiosys                  http://www.ibm.com
Tel +33 (0)562 165 496

"Anarchy is about taking complete responsibility for yourself."
        Alan Moore.
#12 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

Quoted 16 lines · Greg Kurz
Quoted 10 lines · On Tue, 6 May 2014 19:37:22 +0100 Peter Maydell <[email protected]>…
Quoted 6 lines · Greg Kurz

POWER7, POWER7+ and POWER8 families use the ILE bit of the LPCR special purpose register to decide the endianness to use when entering interrupt handlers. When running a Linux guest, this provides a hint on the endianness used by the kernel. From a QEMU point of view, the information is needed for legacy virtio support and crash dump support as well.

Do you care about the case of:
* kernel bigendian

Yes. FWIW, ppc64 is still widely used in big endian mode we don't want to break.

Quoted 8 lines
Quoted 1 line

* userspace littleendian (or vice-versa)

We don't care about userspace here. We assume that virtio structures are owned by the guest kernel.

Quoted 1 line

* guest kernel passes virtio device through to guest userspace

Not sure to understand... could you please point me to an example ?

Consider PCI passthrough of a virtio device to userspace Linux or to a nested KVM/QEMU instance running inside the outermost KVM. It's a bit of an odd corner case, but we should either accommodate it or definitely say it's not expected to work consistently across architectures I think.

thanks
-- PMM

#13 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

In reply to Greg Kurz
Quoted 18 lines · Greg Kurz
Quoted 9 lines · On Tue, 6 May 2014 19:37:22 +0100 Peter Maydell <[email protected]>…
Quoted 6 lines · Greg Kurz

POWER7, POWER7+ and POWER8 families use the ILE bit of the LPCR special purpose register to decide the endianness to use when entering interrupt handlers. When running a Linux guest, this provides a hint on the endianness used by the kernel. From a QEMU point of view, the information is needed for legacy virtio support and crash dump support as well.

Do you care about the case of:
* kernel bigendian

Yes. FWIW, ppc64 is still widely used in big endian mode we don't want to break.

Quoted 1 line

* userspace littleendian (or vice-versa)

We don't care about userspace here. We assume that virtio structures are owned by the guest kernel.

Quoted 7 lines
Quoted 1 line

* guest kernel passes virtio device through to guest userspace

Not sure to understand... could you please point me to an example ?

Quoted 2 lines

* guest userspace is doing the manipulation of the device

Hmm... you mean we would have virtio drivers implemented in the guest userspace ? Does that exist ? Please elaborate.

Virtio bypasses the IOMMU by design, so user space drivers don't make sense here :).

I don't think we should overengineer hacks for legacy virtio.

Alex

#14 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

Quoted 1 line · Alexander Graf

I don't think we should overengineer hacks for legacy virtio.

Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes?

thanks
-- PMM

#15 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

Quoted 10 lines

Am 07.05.2014 um 11:26 schrieb Peter Maydell <[email protected]>:

Quoted 2 lines · Alexander Graf

I don't think we should overengineer hacks for legacy virtio.

Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes?

The interrupt endianness for book3s PPC. Since that's an arch specific thing I think we should just make the determination mechanism arch dependent and list it in the spec.

Booke for example would be vastly different since there is no global LE flag - it's a bit in the TLB entries.

Alex

#16 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

Quoted 11 lines · Alexander Graf
Quoted 10 lines

Am 07.05.2014 um 11:26 schrieb Peter Maydell <[email protected]>:

Quoted 2 lines · Alexander Graf

I don't think we should overengineer hacks for legacy virtio.

Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes?

Quoted 6 lines

The interrupt endianness for book3s PPC. Since that's an arch specific thing I think we should just make the determination mechanism arch dependent and list it in the spec.

Booke for example would be vastly different since there is no global LE flag - it's a bit in the TLB entries.

Sure, but I think we should also state the general principle we're aiming to implement with the arch specific detail, so that people figuring out what a new arch should do have a guide to follow.

thanks
-- PMM

#17 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

Quoted 20 lines

Am 07.05.2014 um 11:40 schrieb Peter Maydell <[email protected]>:

Quoted 16 lines · Alexander Graf
Quoted 10 lines
Quoted 6 lines

Am 07.05.2014 um 11:26 schrieb Peter Maydell <[email protected]>:

On 7 May 2014 10:09, Alexander Graf <[email protected]> wrote:
I don't think we should overengineer hacks for legacy virtio.

Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes?

The interrupt endianness for book3s PPC. Since that's an arch specific thing I think we should just make the determination mechanism arch dependent and list it in the spec.

Quoted 7 lines
Quoted 2 lines

Booke for example would be vastly different since there is no global LE flag - it's a bit in the TLB entries.

Sure, but I think we should also state the general principle we're aiming to implement with the arch specific detail, so that people figuring out what a new arch should do have a guide to follow.

The general principle is "Try to find the Linux guest compile time endianness" :).

Alex

#18 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

In reply to Peter Maydell
Quoted 10 lines

Am 07.05.2014 um 11:26 schrieb Peter Maydell <[email protected]>:

Quoted 2 lines · Alexander Graf

I don't think we should overengineer hacks for legacy virtio.

Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes?

I just realized we're talking about virtio in a non-virtio thread. This patch set is about core dump support which is different from virtio bi-endian support. While both may end up at the same logic, I don't like the idea to mix them. This function is PPC internal.

Alex

Quoted 3 lines

thanks
-- PMM

author
#19 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

Quoted 7 lines · On Wed, 7 May 2014 11:41:10 +0200 Alexander Graf <[email protected]> wrote:
Quoted 7 lines

Am 07.05.2014 um 11:26 schrieb Peter Maydell <[email protected]>:

Quoted 2 lines · Alexander Graf

I don't think we should overengineer hacks for legacy virtio.

Quoted 8 lines
Quoted 3 lines

Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes?

I just realized we're talking about virtio in a non-virtio thread. This patch set is about core dump support which is different from virtio bi-endian support. While both may end up at the same logic, I don't like the idea to mix them. This function is PPC internal.

Alex

Correct and now I have this feeling about using LPCR_ILE versus MSR_LE...

LPCR_ILE reflects the interrupt vector endianness. It is set during early boot by the guest kernel according to the desired endianness. MSR_LE gives the current endian mode for the cpu.

The idea is that you need to rely on LPCR_ILE when you peek from the host because you lack context and MSR_LE may be have been temporarily changed. This is clearly the case for dump support.

Now when it comes to virtio, we cache the endianness at device reset time: MSR_LE from current_cpu should reflect the guest kernel endianness, no ?

In this case we could end up like what's being currently discussed with ARM:

http://www.spinics.net/lists/kvm-arm/msg09099.html
http://www.spinics.net/lists/kvm-arm/msg09091.html

Alex,

If we agree that current_cpu->MSR_LE does the job when the guest kernel resets the device, then I guess we don't even need this patch...

Signature · 7 lines
Gregory Kurz                                     [email protected]
                                                 [email protected]
Software Engineer @ IBM/Meiosys                  http://www.ibm.com
Tel +33 (0)562 165 496

"Anarchy is about taking complete responsibility for yourself."
        Alan Moore.
#20 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

Quoted 30 lines · Greg Kurz
Quoted 12 lines · On Wed, 7 May 2014 11:41:10 +0200 Alexander Graf <[email protected]> wrote:
Quoted 8 lines

Am 07.05.2014 um 11:26 schrieb Peter Maydell <[email protected]>:

Quoted 2 lines · Alexander Graf

I don't think we should overengineer hacks for legacy virtio.

Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes?

I just realized we're talking about virtio in a non-virtio thread. This patch set is about core dump support which is different from virtio bi-endian support. While both may end up at the same logic, I don't like the idea to mix them. This function is PPC internal.

Alex

Correct and now I have this feeling about using LPCR_ILE versus MSR_LE...

LPCR_ILE reflects the interrupt vector endianness. It is set during early boot by the guest kernel according to the desired endianness. MSR_LE gives the current endian mode for the cpu.

The idea is that you need to rely on LPCR_ILE when you peek from the host because you lack context and MSR_LE may be have been temporarily changed. This is clearly the case for dump support.

Now when it comes to virtio, we cache the endianness at device reset time: MSR_LE from current_cpu should reflect the guest kernel endianness, no ?

In this case we could end up like what's being currently discussed with ARM:

Quoted 7 lines

http://www.spinics.net/lists/kvm-arm/msg09099.html
http://www.spinics.net/lists/kvm-arm/msg09091.html

Alex,

If we agree that current_cpu->MSR_LE does the job when the guest kernel resets the device, then I guess we don't even need this patch...

Either one works for me as long as we put it into a spec. No solution will be able to fulfill all cases.

The uglyness about the current_cpu bit is that devices are usually not supposed to know about the cpu accesses come from usually. But then again devices shouldn't know about the endianness of a cpu either so I guess it's ok to breach layers here.

Rusty, do you have strong feelings either way?

Alex

author
#21 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

Quoted 47 lines · On Wed, 07 May 2014 13:54:36 +0200 Alexander Graf <[email protected]> wrote:
Quoted 30 lines · Greg Kurz
Quoted 12 lines · On Wed, 7 May 2014 11:41:10 +0200 Alexander Graf <[email protected]> wrote:
Quoted 8 lines

Am 07.05.2014 um 11:26 schrieb Peter Maydell <[email protected]>:

Quoted 2 lines · Alexander Graf

I don't think we should overengineer hacks for legacy virtio.

Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes?

I just realized we're talking about virtio in a non-virtio thread. This patch set is about core dump support which is different from virtio bi-endian support. While both may end up at the same logic, I don't like the idea to mix them. This function is PPC internal.

Alex

Correct and now I have this feeling about using LPCR_ILE versus MSR_LE...

LPCR_ILE reflects the interrupt vector endianness. It is set during early boot by the guest kernel according to the desired endianness. MSR_LE gives the current endian mode for the cpu.

The idea is that you need to rely on LPCR_ILE when you peek from the host because you lack context and MSR_LE may be have been temporarily changed. This is clearly the case for dump support.

Now when it comes to virtio, we cache the endianness at device reset time: MSR_LE from current_cpu should reflect the guest kernel endianness, no ?

In this case we could end up like what's being currently discussed with ARM:

Quoted 7 lines

http://www.spinics.net/lists/kvm-arm/msg09099.html
http://www.spinics.net/lists/kvm-arm/msg09091.html

Alex,

If we agree that current_cpu->MSR_LE does the job when the guest kernel resets the device, then I guess we don't even need this patch...

Either one works for me as long as we put it into a spec. No solution will be able to fulfill all cases.

The uglyness about the current_cpu bit is that devices are usually not supposed to know about the cpu accesses come from usually. But then again devices shouldn't know about the endianness of a cpu either so I guess it's ok to breach layers here.

Quoted 5 lines

Rusty, do you have strong feelings either way?

Alex

Coming back to the initial subject. What about changing the last sentence in the commit message to the following ?

"And when it comes to dumping a guest, the information is needed to write
ELF headers using the kernel endianness."

This would make the patch a PPC64 dump only business and end the debate. :)

Tell me if you want me to repost.

Cheers.

Signature · 7 lines
Gregory Kurz                                     [email protected]
                                                 [email protected]
Software Engineer @ IBM/Meiosys                  http://www.ibm.com
Tel +33 (0)562 165 496

"Anarchy is about taking complete responsibility for yourself."
        Alan Moore.
#22 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

Quoted 55 lines · Greg Kurz
Quoted 45 lines · On Wed, 07 May 2014 13:54:36 +0200 Alexander Graf <[email protected]> wrote:
Quoted 29 lines · Greg Kurz
Quoted 11 lines · On Wed, 7 May 2014 11:41:10 +0200 Alexander Graf <[email protected]> wrote:
Quoted 7 lines

Am 07.05.2014 um 11:26 schrieb Peter Maydell <[email protected]>:

Quoted 2 lines · Alexander Graf

I don't think we should overengineer hacks for legacy virtio.

Agreed. So what's our final conclusion: virtio endianness is the endianness of the guest kernel at the point where it triggers a reset of the virtio device, yes?

I just realized we're talking about virtio in a non-virtio thread. This patch set is about core dump support which is different from virtio bi-endian support. While both may end up at the same logic, I don't like the idea to mix them. This function is PPC internal.

Alex

Correct and now I have this feeling about using LPCR_ILE versus MSR_LE...

LPCR_ILE reflects the interrupt vector endianness. It is set during early boot by the guest kernel according to the desired endianness. MSR_LE gives the current endian mode for the cpu.

The idea is that you need to rely on LPCR_ILE when you peek from the host because you lack context and MSR_LE may be have been temporarily changed. This is clearly the case for dump support.

Now when it comes to virtio, we cache the endianness at device reset time: MSR_LE from current_cpu should reflect the guest kernel endianness, no ?

In this case we could end up like what's being currently discussed with ARM:

Quoted 7 lines

http://www.spinics.net/lists/kvm-arm/msg09099.html
http://www.spinics.net/lists/kvm-arm/msg09091.html

Alex,

If we agree that current_cpu->MSR_LE does the job when the guest kernel resets the device, then I guess we don't even need this patch...

Either one works for me as long as we put it into a spec. No solution will be able to fulfill all cases.

The uglyness about the current_cpu bit is that devices are usually not supposed to know about the cpu accesses come from usually. But then again devices shouldn't know about the endianness of a cpu either so I guess it's ok to breach layers here.

Quoted 5 lines

Rusty, do you have strong feelings either way?

Alex

Coming back to the initial subject. What about changing the last sentence in the commit message to the following ?

Quoted 6 lines

"And when it comes to dumping a guest, the information is needed to write
ELF headers using the kernel endianness."

This would make the patch a PPC64 dump only business and end the debate. :)

Tell me if you want me to repost.

Let's wait for Tom's ack first - or if you can show me that you fully grasped that we're doing the "right thing" in all host/guest big/little combinations with AVX registers I'm happy too :)

Alex

#23 lore raw

Re: [Qemu-devel] [PATCH v3 3/4] target-ppc: ppc can be either endian

In reply to Alexander Graf
Quoted 7 lines · Alexander Graf

On 05/07/2014 12:19 PM, Greg Kurz wrote:
The uglyness about the current_cpu bit is that devices are usually not supposed to know about the cpu accesses come from usually. But then again devices shouldn't know about the endianness of a cpu either so I guess it's ok to breach layers here.

Rusty, do you have strong feelings either way?

My only strong feeling is that qemu is horrible to get patches into.

At this point, I really don't care :(

Rusty.

author
#24 lore raw

PATCH v2 4/4 ppc64 dump: Set the correct endianness in ELF dump header

In reply to Greg Kurz

From: Bharata B Rao <[email protected]>

[ use PowerPCCPUClass::interrupts_big_endian(),
Greg Kurz <[email protected]> ]

---

No changes, resent in case it helps

 target-ppc/arch_dump.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
target-ppc/arch_dump.c +8 -4
index 5487b61..53e2fe0 100644
@@ -196,12 +196,16 @@ typedef struct NoteFuncDescStruct NoteFuncDesc;
 int cpu_get_dump_info(ArchDumpInfo *info,
                       const struct GuestPhysBlockList *guest_phys_blocks)
 {
-    /*
-     * Currently only handling PPC64 big endian.
-     */
+    PowerPCCPU *cpu = POWERPC_CPU(first_cpu);
+    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+
     info->d_machine = EM_PPC64;
-    info->d_endian = ELFDATA2MSB;
     info->d_class = ELFCLASS64;
+    if ((*pcc->interrupts_big_endian)(cpu)) {
+        info->d_endian = ELFDATA2MSB;
+    } else {
+        info->d_endian = ELFDATA2LSB;
+    }
 
     return 0;
 }
#25 lore raw
In reply to Greg Kurz
Quoted 7 lines · Greg Kurz

Hi,

Please find the latest patch set for dump-guest-memory to support little-endian ppc64 guest kernels. The only change since the last post is a better naming of the dump endian helpers. Since it affects patches 1 & 2, I resend the full serie, with Alex's Reviewed-by tags from the last review.

I like the patch set. If I also get an ack from Tom on the data layout of AVR registers I'd be more than happy to apply it to my tree. I'd also welcome someone non-ppc to review the dump patches.

Alex

#26 lore raw
In reply to Greg Kurz

Am 05.05.2014 10:02, schrieb Greg Kurz:

Quoted 4 lines

Bharata B Rao (3):
dump: Make DumpState and endian conversion routines available for arch-specific dump code
ppc64-dump: Support dump for little endian ppc64
ppc64 dump: Set the correct endianness in ELF dump header

Both 2/4 and 4/4 touch target-ppc/arch_dump.c exclusively. Please don't invent random topics, in particular not inconsistently. Use target-ppc (QEMU scheme) or PPC (Alex' Linux'ish scheme), cf.
http://git.qemu-project.org/?p=qemu.git;a=history;f=target-ppc;hb=HEAD

I'm sure Alex can edit it if you're finally reaching consensus.

Regards,
Andreas

Quoted 3 lines

Greg Kurz (1):
target-ppc: ppc can be either endian

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