[Qemu-devel] [PATCH] translate-all.c: fix debug memory maps printing

[Qemu-devel] [PATCH] translate-all.c: fix debug memory maps printing

[Mikhail Ilyin]

Fix memory maps textualizing function. The output was not correct because of
wrong base address calculation. The initial address has to be shifted also
for TARGET_PAGE_BITS.

Signed-off-by: Mikhail Ilyin <m.ilin@samsung.com>
---
 translate-all.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index 8f7e11b..cb7a33d 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1728,9 +1728,8 @@ int walk_memory_regions(void *priv, walk_memory_regions_fn fn)
     data.prot = 0;
 
     for (i = 0; i < V_L1_SIZE; i++) {
-        int rc = walk_memory_regions_1(&data, (abi_ulong)i << V_L1_SHIFT,
+        int rc = walk_memory_regions_1(&data, (abi_ulong)i << (V_L1_SHIFT + TARGET_PAGE_BITS),
                                        V_L1_SHIFT / V_L2_BITS - 1, l1_map + i);
-
         if (rc != 0) {
             return rc;
         }
-- 
1.9.1

Re: [Qemu-devel] [PATCH] translate-all.c: fix debug memory maps printing

[Paolo Bonzini]

Il 11/08/2014 12:28, Mikhail Ilyin ha scritto:
> Fix memory maps textualizing function. The output was not correct because of
> wrong base address calculation. The initial address has to be shifted also
> for TARGET_PAGE_BITS.
> 
> Signed-off-by: Mikhail Ilyin <m.ilin@samsung.com>
> ---
>  translate-all.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/translate-all.c b/translate-all.c
> index 8f7e11b..cb7a33d 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -1728,9 +1728,8 @@ int walk_memory_regions(void *priv, walk_memory_regions_fn fn)
>      data.prot = 0;
>  
>      for (i = 0; i < V_L1_SIZE; i++) {
> -        int rc = walk_memory_regions_1(&data, (abi_ulong)i << V_L1_SHIFT,
> +        int rc = walk_memory_regions_1(&data, (abi_ulong)i << (V_L1_SHIFT + TARGET_PAGE_BITS),
>                                         V_L1_SHIFT / V_L2_BITS - 1, l1_map + i);
> -
>          if (rc != 0) {
>              return rc;
>          }
> 

Thanks, this is simple enough that I've queued it.

Paolo

Re: [Qemu-devel] [PATCH] translate-all.c: fix debug memory maps printing

[Paolo Bonzini]

Il 25/08/2014 13:45, Paolo Bonzini ha scritto:
> Il 11/08/2014 12:28, Mikhail Ilyin ha scritto:
>> Fix memory maps textualizing function. The output was not correct because of
>> wrong base address calculation. The initial address has to be shifted also
>> for TARGET_PAGE_BITS.
>>
>> Signed-off-by: Mikhail Ilyin <m.ilin@samsung.com>
>> ---
>>  translate-all.c | 3 +--
>>  1 file changed, 1 insertion(+), 2 deletions(-)
>>
>> diff --git a/translate-all.c b/translate-all.c
>> index 8f7e11b..cb7a33d 100644
>> --- a/translate-all.c
>> +++ b/translate-all.c
>> @@ -1728,9 +1728,8 @@ int walk_memory_regions(void *priv, walk_memory_regions_fn fn)
>>      data.prot = 0;
>>  
>>      for (i = 0; i < V_L1_SIZE; i++) {
>> -        int rc = walk_memory_regions_1(&data, (abi_ulong)i << V_L1_SHIFT,
>> +        int rc = walk_memory_regions_1(&data, (abi_ulong)i << (V_L1_SHIFT + TARGET_PAGE_BITS),
>>                                         V_L1_SHIFT / V_L2_BITS - 1, l1_map + i);
>> -
>>          if (rc != 0) {
>>              return rc;
>>          }
>>
> 
> Thanks, this is simple enough that I've queued it.

Ouch, I spoke too soon.

This patch fails to compile for MIPS N32 (a 32-bit ABI with a 64-bit
virtual address space).  I'm not sure if there is a simple fix.
walk_memory_regions and its user should be changed to use target_ulong
instead of abi_ulong.  access_ok probably needs to be changed in the
same way too.  Riku, do you have any ideas (or free cycles to do the
change)?

Paolo

Re: [Qemu-devel] [PATCH] translate-all.c: fix debug memory maps printing

[Mikhail Ilin]

I've also found that this issue in walker API leads to creating
a misformed core file. elf_core_dump() uses walk_memory_regions()
to build memory mapping for a core file.

As a result the core file has a very small size and doesn't contain
page snapshots of mapped libraries.

I've compiled a simple helloworld prog (which dereference a NULL
pointer at the end to get a core file) and run the prog twice
without a workaround patch and with it.

$ gcc -m32 -g -Wall -o /tmp/prog /tmp/prog.c
$ ulimic -c unlimited

I use i386 but the same works for ARM and believe for other 32bits
arches.

$ qemu-i386 /tmp/prog
Hello world!
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Segmentation fault (core dumped)

Here is core files sizes without a workaround patch and with it.

$ ls -l
892      qemu_prog_20140905-121147_21636.core
8454144  qemu_prog_20140905-120737_21355.core

Also I've investigate them with gdb.

No workaround:

$ gdb /tmp/prog qemu_prog_20140905-121147_21636.core
Cannot access memory at address 0x407fffe4
#-1 0x08048406 in main () at /tmp/prog.c:8

Here is the fail because there is no mapping that contains address
0x407fffe4.

(gdb) info files
	0x00048000 - 0x00048000 is load1
	0x00049000 - 0x00049000 is load2
	0x00040000 - 0x00040000 is load3
	0x00041000 - 0x00041000 is load4
	0x00041800 - 0x00041800 is load5
	0x00061800 - 0x00061800 is load6
	0x00062800 - 0x00062800 is load7
	0x00045800 - 0x00045800 is load8
	0x001ee800 - 0x001ee800 is load9
	0x001ef800 - 0x001ef800 is load10
	0x001f1800 - 0x001f1800 is load11
	....
	
With workaround:

	0x08048000 - 0x08048000 is load1
	0x08049000 - 0x0804a000 is load2
	0x40000000 - 0x40000000 is load3
	0x40001000 - 0x40801000 is load4
	0x40801000 - 0x40801000 is load5
	0x40821000 - 0x40822000 is load6
	0x40822000 - 0x40827000 is load7
	0x40845000 - 0x40845000 is load8
	0x409ee000 - 0x409ee000 is load9
	0x409ef000 - 0x409f1000 is load10
	0x409f1000 - 0x409f7000 is load11

address 0x407fffe4 belongs to load4.

Also this core includes other library mappings that are not
presented in the core file without a workaround:

	0x40845174 - 0x40845198 is /lib/i386-linux-gnu/libc.so.6
	0x40845198 - 0x408451b8 is /lib/i386-linux-gnu/libc.so.6
	0x408451b8 - 0x40848ec8 is /lib/i386-linux-gnu/libc.so.6
	0x40848ec8 - 0x40852438 is /lib/i386-linux-gnu/libc.so.6
	0x40852438 - 0x4085815e is /lib/i386-linux-gnu/libc.so.6
	0x4085815e - 0x4085940c is /lib/i386-linux-gnu/libc.so.6
	0x4085940c - 0x40859898 is /lib/i386-linux-gnu/libc.so.6
	0x40859898 - 0x408598d8 is /lib/i386-linux-gnu/libc.so.6
	0x408598d8 - 0x4085c2e8 is /lib/i386-linux-gnu/libc.so.6
	0x4085c2e8 - 0x4085c348 is /lib/i386-linux-gnu/libc.so.6
	0x4085c350 - 0x4085c420 is /lib/i386-linux-gnu/libc.so.6
	0x4085c420 - 0x4098e44e is /lib/i386-linux-gnu/libc.so.6
	0x4098e450 - 0x4098f3db is /lib/i386-linux-gnu/libc.so.6
	0x4098f3e0 - 0x4098f5de is /lib/i386-linux-gnu/libc.so.6

 > This patch fails to compile for MIPS N32 (a 32-bit ABI with a 64-bit
 > virtual address space).

I also wonder we have separate linux-user emulators for i386 (32 bit
ABI + 32 bit address space) and amd64 binaries (64 bit ABI + 64 bit
address space). And we can not run 32 bits apps under qemu-x86_64 but
MIPS N32 looks in some other way and it supports 32 bit ABI apps with
64 bit address space. I really not sure but is it a right design or
not?

 > Riku, do you have any ideas (or free cycles to do the change)?
Please, give some clues.

Thanks.

On 25.08.2014 16:05, Paolo Bonzini wrote:
> Il 25/08/2014 13:45, Paolo Bonzini ha scritto:
>> Il 11/08/2014 12:28, Mikhail Ilyin ha scritto:
>>> Fix memory maps textualizing function. The output was not correct because of
>>> wrong base address calculation. The initial address has to be shifted also
>>> for TARGET_PAGE_BITS.
>>>
>>> Signed-off-by: Mikhail Ilyin <m.ilin@samsung.com>
>>> ---
>>>   translate-all.c | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/translate-all.c b/translate-all.c
>>> index 8f7e11b..cb7a33d 100644
>>> --- a/translate-all.c
>>> +++ b/translate-all.c
>>> @@ -1728,9 +1728,8 @@ int walk_memory_regions(void *priv, walk_memory_regions_fn fn)
>>>       data.prot = 0;
>>>
>>>       for (i = 0; i < V_L1_SIZE; i++) {
>>> -        int rc = walk_memory_regions_1(&data, (abi_ulong)i << V_L1_SHIFT,
>>> +        int rc = walk_memory_regions_1(&data, (abi_ulong)i << (V_L1_SHIFT + TARGET_PAGE_BITS),
>>>                                          V_L1_SHIFT / V_L2_BITS - 1, l1_map + i);
>>> -
>>>           if (rc != 0) {
>>>               return rc;
>>>           }
>>>
>>
>> Thanks, this is simple enough that I've queued it.
>
> Ouch, I spoke too soon.
>
> This patch fails to compile for MIPS N32 (a 32-bit ABI with a 64-bit
> virtual address space).  I'm not sure if there is a simple fix.
> walk_memory_regions and its user should be changed to use target_ulong
> instead of abi_ulong.  access_ok probably needs to be changed in the
> same way too.  Riku, do you have any ideas (or free cycles to do the
> change)?
>
> Paolo
>
>

Re: [Qemu-devel] [PATCH] translate-all.c: fix debug memory maps printing

[Peter Maydell]

On 5 September 2014 09:59, Mikhail Ilin <m.ilin@samsung.com> wrote:
> I also wonder we have separate linux-user emulators for i386 (32 bit
> ABI + 32 bit address space) and amd64 binaries (64 bit ABI + 64 bit
> address space). And we can not run 32 bits apps under qemu-x86_64 but
> MIPS N32 looks in some other way and it supports 32 bit ABI apps with
> 64 bit address space. I really not sure but is it a right design or
> not?

The design here is that we have one QEMU executable for each
Linux ABI we support. On MIPS there are a number of different
ABIs which may be in use even with the same CPU type, which
is why there are separate emulator binaries. On x86, if we ever
supported the x86_64 "x32" ABI, that would be an extra emulator
binary in addition to the current x86_64 and i386 ones.

This should all not matter much for core code, which simply
has to use the correct types for things. Quoting from HACKING:

vaddr is the best type to use to hold a CPU virtual address in
target-independent code. It is guaranteed to be large enough to hold a
virtual address for any target, and it does not change size from target
to target. It is always unsigned.
target_ulong is a type the size of a virtual address on the CPU; this means
it may be 32 or 64 bits depending on which target is being built. It should
therefore be used only in target-specific code, and in some
performance-critical built-per-target core code such as the TLB code.
There is also a signed version, target_long.
abi_ulong is for the *-user targets, and represents a type the size of
'void *' in that target's ABI. (This may not be the same as the size of a
full CPU virtual address in the case of target ABIs which use 32 bit pointers
on 64 bit CPUs, like sparc32plus.) Definitions of structures that must match
the target's ABI must use this type for anything that on the target is defined
to be an 'unsigned long' or a pointer type.
There is also a signed version, abi_long.

The problem you're running into here is with the ABIs where
abi_ulong and target_ulong are different sizes.

thanks
-- PMM