| Message ID | 20190108130219.18550-1-arilou@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] dump: Set correct vaddr for ELF dump | expand |
On Tue, Jan 8, 2019 at 5:02 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> Not sure we are doing everything correctly, but solves the non-paging GDB case, so: Tested-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> > --- > dump.c | 5 +++-- > scripts/dump-guest-memory.py | 1 + > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/dump.c b/dump.c > index 4ec94c5e25..7a56e254d4 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 = cpu_to_dump64(s, memory_mapping->virt_addr) ? : phdr.p_paddr; > > assert(memory_mapping->length >= filesz); > > @@ -216,7 +216,8 @@ 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 = > + cpu_to_dump32(s, memory_mapping->virt_addr) ? : 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 >
Patchew URL: https://patchew.org/QEMU/20190108130219.18550-1-arilou@gmail.com/
Hi,
This series seems to have some coding style problems. See output below for
more information:
Type: series
Subject: [Qemu-devel] [PATCH v2] dump: Set correct vaddr for ELF dump
Message-id: 20190108130219.18550-1-arilou@gmail.com
=== TEST SCRIPT BEGIN ===
#!/bin/bash
BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
failed=1
echo
fi
n=$((n+1))
done
exit $failed
=== TEST SCRIPT END ===
Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
bbcb775 dump: Set correct vaddr for ELF dump
=== OUTPUT BEGIN ===
Checking PATCH 1/1: dump: Set correct vaddr for ELF dump...
ERROR: code indent should never use tabs
#40: FILE: dump.c:220:
+^I^Icpu_to_dump32(s, memory_mapping->virt_addr) ? : phdr.p_paddr;$
total: 1 errors, 0 warnings, 24 lines checked
Your patch has style problems, please review. If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
=== OUTPUT END ===
Test command exited with code: 1
The full log is available at
http://patchew.org/logs/20190108130219.18550-1-arilou@gmail.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
On 01/08/19 14:05, Marc-André Lureau wrote: > On Tue, Jan 8, 2019 at 5:02 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> > > Not sure we are doing everything correctly, but solves the non-paging > GDB case, so: > > Tested-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Same thoughts here. Acked-by: Laszlo Ersek <lersek@redhat.com> Thanks Laszlo >> --- >> dump.c | 5 +++-- >> scripts/dump-guest-memory.py | 1 + >> 2 files changed, 4 insertions(+), 2 deletions(-) >> >> diff --git a/dump.c b/dump.c >> index 4ec94c5e25..7a56e254d4 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 = cpu_to_dump64(s, memory_mapping->virt_addr) ? : phdr.p_paddr; >> >> assert(memory_mapping->length >= filesz); >> >> @@ -216,7 +216,8 @@ 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 = >> + cpu_to_dump32(s, memory_mapping->virt_addr) ? : 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 >>
diff --git a/dump.c b/dump.c index 4ec94c5e25..7a56e254d4 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 = cpu_to_dump64(s, memory_mapping->virt_addr) ? : phdr.p_paddr; assert(memory_mapping->length >= filesz); @@ -216,7 +216,8 @@ 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 = + cpu_to_dump32(s, memory_mapping->virt_addr) ? : 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)
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 | 5 +++-- scripts/dump-guest-memory.py | 1 + 2 files changed, 4 insertions(+), 2 deletions(-)