| Message ID | 1407752887-6351-1-git-send-email-m.ilin@samsung.com |
|---|---|
| State | New |
| Headers | show |
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
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
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 > >
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
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; }
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(-)