* [Qemu-devel] [PATCH v1] dump: Set correct vaddr for ELF dump @ 2018-12-25 12:53 Jon Doron 2019-01-07 12:14 ` Marc-André Lureau 0 siblings, 1 reply; 5+ messages in thread From: Jon Doron @ 2018-12-25 12:53 UTC (permalink / raw) To: qemu-devel; +Cc: qemu-trivial, marcandre.lureau, Jon Doron 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 <arilou@gmail.com> --- 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; 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 phdr.p_filesz = p_size phdr.p_memsz = p_size self.segments.append(phdr) -- 2.19.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v1] dump: Set correct vaddr for ELF dump 2018-12-25 12:53 [Qemu-devel] [PATCH v1] dump: Set correct vaddr for ELF dump Jon Doron @ 2019-01-07 12:14 ` Marc-André Lureau 2019-01-07 18:04 ` Laszlo Ersek 0 siblings, 1 reply; 5+ messages in thread From: Marc-André Lureau @ 2019-01-07 12:14 UTC (permalink / raw) To: Jon Doron Cc: QEMU, qemu trival, Laszlo Ersek, Wen Congyang, Luiz Capitulino, Paolo Bonzini Hi On Tue, Dec 25, 2018 at 5:52 PM Jon Doron <arilou@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 <arilou@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 > > -- Marc-André Lureau ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v1] dump: Set correct vaddr for ELF dump 2019-01-07 12:14 ` Marc-André Lureau @ 2019-01-07 18:04 ` Laszlo Ersek 2019-01-08 6:31 ` Jon Doron 0 siblings, 1 reply; 5+ messages in thread From: Laszlo Ersek @ 2019-01-07 18:04 UTC (permalink / raw) To: Marc-André Lureau, Jon Doron Cc: QEMU, qemu trival, Wen Congyang, Luiz Capitulino, Paolo Bonzini On 01/07/19 13:14, Marc-André Lureau wrote: > Hi > > On Tue, Dec 25, 2018 at 5:52 PM Jon Doron <arilou@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 <arilou@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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v1] dump: Set correct vaddr for ELF dump 2019-01-07 18:04 ` Laszlo Ersek @ 2019-01-08 6:31 ` Jon Doron 2019-01-08 11:30 ` Laszlo Ersek 0 siblings, 1 reply; 5+ messages in thread From: Jon Doron @ 2019-01-08 6:31 UTC (permalink / raw) To: Laszlo Ersek Cc: Marc-André Lureau, QEMU, Wen Congyang, Luiz Capitulino, Paolo Bonzini Thank you for looking into this, perhaps I could change the patch (at least in the C part not the python script) to something like: - phdr.p_vaddr = cpu_to_dumpXX(s, memory_mapping->virt_addr); + phdr.p_vaddr = cpu_to_dumpXX(s, memory_mapping->virt_addr) ? cpu_to_dumpXX(s, memory_mapping->virt_addr) : phdr.p_paddr; So in the case of paging where virt_addr is available we will use it Thanks, -- Jon. On Mon, Jan 7, 2019 at 8:04 PM Laszlo Ersek <lersek@redhat.com> wrote: > > On 01/07/19 13:14, Marc-André Lureau wrote: > > Hi > > > > On Tue, Dec 25, 2018 at 5:52 PM Jon Doron <arilou@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 <arilou@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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Qemu-devel] [PATCH v1] dump: Set correct vaddr for ELF dump 2019-01-08 6:31 ` Jon Doron @ 2019-01-08 11:30 ` Laszlo Ersek 0 siblings, 0 replies; 5+ messages in thread From: Laszlo Ersek @ 2019-01-08 11:30 UTC (permalink / raw) To: Jon Doron Cc: Marc-André Lureau, QEMU, Wen Congyang, Luiz Capitulino, Paolo Bonzini On 01/08/19 07:31, Jon Doron wrote: > Thank you for looking into this, perhaps I could change the patch (at > least in the C part not the python script) to something like: > - phdr.p_vaddr = cpu_to_dumpXX(s, memory_mapping->virt_addr); > + phdr.p_vaddr = cpu_to_dumpXX(s, memory_mapping->virt_addr) ? > cpu_to_dumpXX(s, memory_mapping->virt_addr) : phdr.p_paddr; > > So in the case of paging where virt_addr is available we will use it I guess that would be an improvement, yes. Regarding style: although I'm personally opposed to most GNUisms, QEMU liberally uses, for example, the GNU extension: https://gcc.gnu.org/onlinedocs/gcc/Conditionals.html And, the "?:" operator would apply here. Thanks, Laszlo > On Mon, Jan 7, 2019 at 8:04 PM Laszlo Ersek <lersek@redhat.com> wrote: >> >> On 01/07/19 13:14, Marc-André Lureau wrote: >>> Hi >>> >>> On Tue, Dec 25, 2018 at 5:52 PM Jon Doron <arilou@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 <arilou@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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-01-08 11:30 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-25 12:53 [Qemu-devel] [PATCH v1] dump: Set correct vaddr for ELF dump Jon Doron 2019-01-07 12:14 ` Marc-André Lureau 2019-01-07 18:04 ` Laszlo Ersek 2019-01-08 6:31 ` Jon Doron 2019-01-08 11:30 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).