[Qemu-devel] [PATCH v2] dump: Set correct vaddr for ELF dump

[Qemu-devel] [PATCH v2] dump: Set correct vaddr for ELF dump

[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                       | 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

Re: [Qemu-devel] [PATCH v2] dump: Set correct vaddr for ELF dump

[Marc-André Lureau]

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
>

Re: [Qemu-devel] [PATCH v2] dump: Set correct vaddr for ELF dump

[no-reply]

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

Re: [Qemu-devel] [PATCH v2] dump: Set correct vaddr for ELF dump

[Laszlo Ersek]

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
>>