On 01/07/19 13:14, Marc-André Lureau wrote: > Hi > > On Tue, Dec 25, 2018 at 5:52 PM Jon Doron <ari...@gmail.com> wrote: >> >> vaddr needs to be equal to the paddr since the dump file represents the >> physical memory image. >> >> Without setting vaddr correctly, GDB would load all the different memory >> regions on top of each other to vaddr 0, thus making GDB showing the wrong >> memory data for a given address. >> >> Signed-off-by: Jon Doron <ari...@gmail.com> > > This is a non-trivial patch! (qemu-trivial, please ignore). > >> --- >> dump.c | 4 ++-- >> scripts/dump-guest-memory.py | 1 + >> 2 files changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/dump.c b/dump.c >> index 4ec94c5e25..bf77a119ea 100644 >> --- a/dump.c >> +++ b/dump.c >> @@ -192,7 +192,7 @@ static void write_elf64_load(DumpState *s, MemoryMapping >> *memory_mapping, >> 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); >> + phdr.p_vaddr = phdr.p_paddr; > > This is likely breaking paging=true somehow, which sets > memory_mapping->virt_addr to non-0. > > According to doc "If you want to use gdb to process the core, please > set @paging to true." > > Although I am not able to (gdb) x/10bx 0xa0000 for example on a core > produced with paging. Not sure why, anybody could help? > >> assert(memory_mapping->length >= filesz); >> >> @@ -216,7 +216,7 @@ static void write_elf32_load(DumpState *s, MemoryMapping >> *memory_mapping, >> 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); >> + phdr.p_vaddr = phdr.p_paddr; >> >> assert(memory_mapping->length >= filesz); >> >> diff --git a/scripts/dump-guest-memory.py b/scripts/dump-guest-memory.py >> index 198cd0fe40..2c587cbefc 100644 >> --- a/scripts/dump-guest-memory.py >> +++ b/scripts/dump-guest-memory.py >> @@ -163,6 +163,7 @@ class ELF(object): >> phdr = get_arch_phdr(self.endianness, self.elfclass) >> phdr.p_type = p_type >> phdr.p_paddr = p_paddr >> + phdr.p_vaddr = p_paddr > > With your proposed change though, I can dump memory with gdb... > >> phdr.p_filesz = p_size >> phdr.p_memsz = p_size >> self.segments.append(phdr) >> -- >> 2.19.2 >> >> > >
I've never used paging-enabled dumps. First, because doing so requires QEMU to trust guest memory contents (see original commit 783e9b4826b9; or more recently/generally, the @dump-guest-memory docs in "qapi/misc.json"). Second, because whenever I had to deal with guest memory dumps, I always used "crash" (which needs no paging), and the subject guests were all Linux. I can't comment on paging-enabled patches for dump, except that they shouldn't regress the paging-disabled functionality. :) If the patches satisfy that, I'm fine. (I *am* surprised that GDB insists on p_vaddr equaling p_paddr; after all, in the guest, the virtual address is "memory_mapping->virt_addr". But, I would never claim to understand most of the ELF intricacies, and/or what GDB requires on top of those.) Thanks Laszlo