PPC: QEMU breaks static variables in OpenBIOS
Hi all, Futher to John's report of next_grab_slot not changing value within the PPC MMU hash_page_32() function, I've done some more experimentation and determined that there is indeed a problem with all OpenBIOS static variables on PPC. The issue is that OpenBIOS uses static variables in several places to keep track of various bits of information, and at compile-time these variables are given addresses within the OpenBIOS image itself, e.g. (gdb) p &next_grab_slot $1 = (int *) 0xfffdd000 However as John points out, the MemoryRegion associated with OpenBIOS is set to read-only. Single-stepping using gdb shows that while the store to update the value is executed, it is discarded by QEMU and so therefore static variables never change from their initial value. Rather amusingly, it seems that setting the variable value via gdb bypasses the MemoryRegion read-only check which made this more interesting to debug. I've done a quick hack on QEMU which is to comment out the following line from hw/ppc_oldworld.c in order to make the region read/write again: memory_region_set_readonly(bios, true); I've confirmed in gdb that this fixes the issue with static variables being able to update their value, and now with the latest round of OS X patches applied, the Darwin kernel boots although panics fairly early on. At this point, it seems that we have a decision to make. Is the bug here that the OpenBIOS ROM is marked as read-only for PPC, or OpenBIOS's use of static variables? If it's the latter, I think there is going to have to be a fairly major patch to remove static variables completely from the target image codebase. Thoughts? Mark.
On Jan 2, 2013, at 7:18 AM, Mark Cave-Ayland wrote:
Hi all,
Futher to John's report of next_grab_slot not changing value within the PPC MMU hash_page_32() function, I've done some more experimentation and determined that there is indeed a problem with all OpenBIOS static variables on PPC.
The issue is that OpenBIOS uses static variables in several places to keep track of various bits of information, and at compile-time these variables are given addresses within the OpenBIOS image itself, e.g.
(gdb) p &next_grab_slot $1 = (int *) 0xfffdd000
However as John points out, the MemoryRegion associated with OpenBIOS is set to read-only. Single-stepping using gdb shows that while the store to update the value is executed, it is discarded by QEMU and so therefore static variables never change from their initial value.
Rather amusingly, it seems that setting the variable value via gdb bypasses the MemoryRegion read-only check which made this more interesting to debug.
I've done a quick hack on QEMU which is to comment out the following line from hw/ppc_oldworld.c in order to make the region read/write again:
memory_region_set_readonly(bios, true);
I've confirmed in gdb that this fixes the issue with static variables being able to update their value, and now with the latest round of OS X patches applied, the Darwin kernel boots although panics fairly early on.
At this point, it seems that we have a decision to make. Is the bug here that the OpenBIOS ROM is marked as read-only for PPC, or OpenBIOS's use of static variables? If it's the latter, I think there is going to have to be a fairly major patch to remove static variables completely from the target image codebase.
Thoughts?
Mark.
-- OpenBIOS http://openbios.org/ Mailinglist: http://lists.openbios.org/mailman/listinfo Free your System - May the Forth be with you
I have noticed that a single register (r9) appears to be used at a base register for all static variables. If we created our own memory region using malloc for static and global variables, and set register r9 to this value, we can make static variables work again. I know it is a hack, but I was able to make static variables work this way. Early in the boot process, register r9 is set to 0x7c631a15. This address is what I used to make next_grab_slot work. My guess is the address is a static variable memory region. Register r9 might have been intended to be some kind of base register, but I haven't found any documentation that states this. I do know all static variables in ofmem.c use it for address calculations.
On 02/01/13 13:23, Programmingkid wrote:
I have noticed that a single register (r9) appears to be used at a base register for all static variables. If we created our own memory region using malloc for static and global variables, and set register r9 to this value, we can make static variables work again. I know it is a hack, but I was able to make static variables work this way.
Early in the boot process, register r9 is set to 0x7c631a15. This address is what I used to make next_grab_slot work. My guess is the address is a static variable memory region. Register r9 might have been intended to be some kind of base register, but I haven't found any documentation that states this. I do know all static variables in ofmem.c use it for address calculations.
It would almost seem that either the QEMU ELF loader or something in arch/ppc/qemu/entry.S should be relocating the data section or entire ROM image into RAM before execution. Reading the comments for SPARC in arch/sparc32/entry.S, it looks as if the pages are marked writeable in the MMU and there is some kind of relocation into RAM occurring - Blue, can you comment on this? ATB, Mark.
On Wed, Jan 2, 2013 at 1:45 PM, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote:
On 02/01/13 13:23, Programmingkid wrote:
I have noticed that a single register (r9) appears to be used at a base register for all static variables. If we created our own memory region using malloc for static and global variables, and set register r9 to this value, we can make static variables work again. I know it is a hack, but I was able to make static variables work this way.
Early in the boot process, register r9 is set to 0x7c631a15. This address is what I used to make next_grab_slot work. My guess is the address is a static variable memory region. Register r9 might have been intended to be some kind of base register, but I haven't found any documentation that states this. I do know all static variables in ofmem.c use it for address calculations.
It would almost seem that either the QEMU ELF loader or something in arch/ppc/qemu/entry.S should be relocating the data section or entire ROM image into RAM before execution.
Reading the comments for SPARC in arch/sparc32/entry.S, it looks as if the pages are marked writeable in the MMU and there is some kind of relocation into RAM occurring - Blue, can you comment on this?
At first, I tried to use ROM for .text and .rodata, eliminate all .data and only put .bss in RAM, but because of some problems it was easier to copy the whole ROM to RAM. It wastes some guest RAM but IIRC Linux wanted to patch some areas. I'd copy the ROM to RAM for PPC also. On Sparc, MMU is always enabled and guests take care to preserve our mappings, but maybe this is not the case for PPC. Then fully relocatable code would make sense there.
ATB,
Mark.
-- OpenBIOS http://openbios.org/ Mailinglist: http://lists.openbios.org/mailman/listinfo Free your System - May the Forth be with you
Am 02.01.2013 um 13:18 schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
Hi all,
Futher to John's report of next_grab_slot not changing value within the PPC MMU hash_page_32() function, I've done some more experimentation and determined that there is indeed a problem with all OpenBIOS static variables on PPC.
The issue is that OpenBIOS uses static variables in several places to keep track of various bits of information, and at compile-time these variables are given addresses within the OpenBIOS image itself, e.g.
(gdb) p &next_grab_slot $1 = (int *) 0xfffdd000
However as John points out, the MemoryRegion associated with OpenBIOS is set to read-only. Single-stepping using gdb shows that while the store to update the value is executed, it is discarded by QEMU and so therefore static variables never change from their initial value.
Rather amusingly, it seems that setting the variable value via gdb bypasses the MemoryRegion read-only check which made this more interesting to debug.
I've done a quick hack on QEMU which is to comment out the following line from hw/ppc_oldworld.c in order to make the region read/write again:
memory_region_set_readonly(bios, true);
I've confirmed in gdb that this fixes the issue with static variables being able to update their value, and now with the latest round of OS X patches applied, the Darwin kernel boots although panics fairly early on.
At this point, it seems that we have a decision to make. Is the bug here that the OpenBIOS ROM is marked as read-only for PPC, or OpenBIOS's use of static variables?
I'd be inclined to say it's the latter. But we should ask Ben to make sure. Ben, is the firmware mapped r/w on Macs?
If it's the latter, I think there is going to have to be a fairly major patch to remove static variables completely from the target image codebase.
Thoughts?
Can't we manually relocate our OpenBIOS code when relocating it? Imagine we add an extra extraction step to the build process before omitting the final .elf file. In that step we copy the elf relocation info into a special data section that is accessible from within OpenBIOS. Now when we relocate our .text section down to offset 0x0 we also run a normal elf relocator to move all .data variable access down to offset 0x0. That way we could still keep globals around, right? Alex
On 02/01/13 14:33, Alexander Graf wrote:
Can't we manually relocate our OpenBIOS code when relocating it?
Imagine we add an extra extraction step to the build process before omitting the final .elf file. In that step we copy the elf relocation info into a special data section that is accessible from within OpenBIOS.
Now when we relocate our .text section down to offset 0x0 we also run a normal elf relocator to move all .data variable access down to offset 0x0.
That way we could still keep globals around, right?
Possibly, yes. I've been reading around how relocation works with ELF, and I've found something we could potentially use here: http://www.math.utah.edu/docs/info/ld_3.html#SEC18 See the part about the AT keyword and the corresponding C code. Could this be used to relocate the image to a fixed location in RAM without having to create a full-blown ELF relocator? Otherwise I guess we have to have multiple .text sections; one containing an ELF relocator and another containing the payload itself. I'm not sure whether that would affect non-QEMU use of OpenBIOS though. ATB, Mark.
On Wed, 2013-01-02 at 18:47 +0000, Mark Cave-Ayland wrote:
Possibly, yes. I've been reading around how relocation works with ELF, and I've found something we could potentially use here:
http://www.math.utah.edu/docs/info/ld_3.html#SEC18
See the part about the AT keyword and the corresponding C code. Could this be used to relocate the image to a fixed location in RAM without having to create a full-blown ELF relocator?
Otherwise I guess we have to have multiple .text sections; one containing an ELF relocator and another containing the payload itself. I'm not sure whether that would affect non-QEMU use of OpenBIOS though.
On ppc64 at least, with a bit of care on the use of global symbols in asm, it's fairly easy to generate an position independent executable that relocates itself. I've written such a relocator myself, it's about 20 lines of C (unfortunately it's some internal stuff I cannot post publicly as-is), or you can look at the asm variant in Linux written by paulus. The idea is that you can get the compiler to generate really only one type of relocs (R_PPC64_RELATIVE) which are easy to process. I assume ppc32 should be similarly easy. I don't know about sparc however. If you can do the relocation from qemu it's even easier. Cheers, Ben.
On Thu, 2013-01-03 at 08:00 +1100, Benjamin Herrenschmidt wrote:
On ppc64 at least, with a bit of care on the use of global symbols in asm, it's fairly easy to generate an position independent executable that relocates itself.
I've written such a relocator myself, it's about 20 lines of C (unfortunately it's some internal stuff I cannot post publicly as-is), or you can look at the asm variant in Linux written by paulus. The idea is that you can get the compiler to generate really only one type of relocs (R_PPC64_RELATIVE) which are easy to process.
I assume ppc32 should be similarly easy. I don't know about sparc however.
If you can do the relocation from qemu it's even easier.
Reading the original question, a relocator doesn't seem necessary, I'll reply separately to it. Cheers, Ben.
On Wed, 2013-01-02 at 15:33 +0100, Alexander Graf wrote:
I'd be inclined to say it's the latter. But we should ask Ben to make sure.
Ben, is the firmware mapped r/w on Macs?
Of course not :-)
If it's the latter, I think there is going to have to be a fairly major patch to remove static variables completely from the target image codebase.
No it's easy to fix
Can't we manually relocate our OpenBIOS code when relocating it?
Imagine we add an extra extraction step to the build process before omitting the final .elf file. In that step we copy the elf relocation info into a special data section that is accessible from within OpenBIOS.
Now when we relocate our .text section down to offset 0x0 we also run a normal elf relocator to move all .data variable access down to offset 0x0.
That way we could still keep globals around, right?
Just relocate .data, that's trivial. Use the .lds to specify a different load address from link address, put the link address in memory along with the bss and in your early startup code, copy them from the load address to the link address. No need to process ELF relocs or anything like that. I'm sure you can find examples of how to do that all over the place (uboot would be a place to look if you don't know the .lds syntax for this, off the top of my head you look for the AT() directive). Cheers, Ben.
On 02/01/13 21:07, Benjamin Herrenschmidt wrote:
Just relocate .data, that's trivial.
Use the .lds to specify a different load address from link address, put the link address in memory along with the bss and in your early startup code, copy them from the load address to the link address.
No need to process ELF relocs or anything like that.
I'm sure you can find examples of how to do that all over the place (uboot would be a place to look if you don't know the .lds syntax for this, off the top of my head you look for the AT() directive).
Cheers, Ben.
Hi Ben, Thanks for the response. The existing comments in arch/ppc/qemu/start.S suggest that we have a 1MB space at the top of (physical) RAM which we could use for this: /* Memory map: * * Top +-------------------------+ * | | * | ROM into RAM (1 MB) | * | | * +-------------------------+ * | | * | MMU Hash Table (64 kB) | * | | * +-------------------------+ * | | * | Exception Stack (32 kB) | * | | * +-------------------------+ * | | * | Stack (64 kB) | * | | * +-------------------------+ * | | * | Client Stack (64 kB) | * | | * +-------------------------+ * | | * | Malloc Zone (2 MiB) | * | | * +-------------------------+ * : : * Bottom */ The comments in arch/ppc/ofmem.c suggest that virtual memory looks like this: /**************************************************************** * Memory usage (before of_quiesce is called) * * Physical * * 0x00000000 Exception vectors * 0x00004000 Free space * 0x01e00000 Open Firmware (us) * 0x01f00000 OF allocations * 0x01ff0000 PTE Hash * 0x02000000- Free space * * Allocations grow downwards from 0x01e00000 * ****************************************************************/ So could we stash .data somewhere beneath the Open Firmware virtual mapping, e.g. at 0x01d00000? Ben: is this a suitable location for the data section? AFAICT the Open Firmware load base is set to 0x4000 so we shouldn't in theory clash with any boot loaders/kernels. (Disclaimer: I've mainly worked on the SPARC side of OpenBIOS to date, so a lot of the PPC stuff is new to me) Alex/Andreas: is this something that you would be able to help with as PPC maintainers? In particular, I have no way to test PPC64. I could probably make an attempt at something for PPC32, with the minor caveat that I would have to learn PPC assembler first... ATB, Mark.
On 2013-Jan-2 16:35 , Mark Cave-Ayland wrote:
Thanks for the response. The existing comments in arch/ppc/qemu/start.S suggest that we have a 1MB space at the top of (physical) RAM which we could use for this:
/* Memory map: * * Top +-------------------------+ * | | * | ROM into RAM (1 MB) | * | |
Ah. That suggests that qemu expects the PROM code to copy itself lock, stock and barrel (code, text, and data) from ROM into memory and then jump into the the copied code. That way you wouldn't need to separately relocate your .text section.
On 02.01.2013, at 22:35, Mark Cave-Ayland wrote:
On 02/01/13 21:07, Benjamin Herrenschmidt wrote:
Just relocate .data, that's trivial.
Use the .lds to specify a different load address from link address, put the link address in memory along with the bss and in your early startup code, copy them from the load address to the link address.
No need to process ELF relocs or anything like that.
I'm sure you can find examples of how to do that all over the place (uboot would be a place to look if you don't know the .lds syntax for this, off the top of my head you look for the AT() directive).
Cheers, Ben.
Hi Ben,
Thanks for the response. The existing comments in arch/ppc/qemu/start.S suggest that we have a 1MB space at the top of (physical) RAM which we could use for this:
/* Memory map: * * Top +-------------------------+ * | | * | ROM into RAM (1 MB) | * | | * +-------------------------+ * | | * | MMU Hash Table (64 kB) | * | | * +-------------------------+ * | | * | Exception Stack (32 kB) | * | | * +-------------------------+ * | | * | Stack (64 kB) | * | | * +-------------------------+ * | | * | Client Stack (64 kB) | * | | * +-------------------------+ * | | * | Malloc Zone (2 MiB) | * | | * +-------------------------+ * : : * Bottom */
The comments in arch/ppc/ofmem.c suggest that virtual memory looks like this:
/**************************************************************** * Memory usage (before of_quiesce is called) * * Physical * * 0x00000000 Exception vectors * 0x00004000 Free space * 0x01e00000 Open Firmware (us) * 0x01f00000 OF allocations * 0x01ff0000 PTE Hash * 0x02000000- Free space * * Allocations grow downwards from 0x01e00000 * ****************************************************************/
So could we stash .data somewhere beneath the Open Firmware virtual mapping, e.g. at 0x01d00000?
Ben: is this a suitable location for the data section? AFAICT the Open Firmware load base is set to 0x4000 so we shouldn't in theory clash with any boot loaders/kernels. (Disclaimer: I've mainly worked on the SPARC side of OpenBIOS to date, so a lot of the PPC stuff is new to me)
Alex/Andreas: is this something that you would be able to help with as PPC maintainers? In particular, I have no way to test PPC64. I could probably make an attempt at something for PPC32, with the minor caveat that I would have to learn PPC assembler first...
Well, .bss is easy as we can just move that down to a r/w memory location. .data is slightly more tricky, as we need to copy data early on. Do we need any global preassigned (.data) variables before we get into C? I highly doubt so :). Thus, I'd be very inclined to believe that we don't need any asm code at all, making this an all-arch solution. Alex
On 02/01/13 21:58, Alexander Graf wrote:
Well, .bss is easy as we can just move that down to a r/w memory location. .data is slightly more tricky, as we need to copy data early on. Do we need any global preassigned (.data) variables before we get into C? I highly doubt so :).
Thus, I'd be very inclined to believe that we don't need any asm code at all, making this an all-arch solution.
Okay, I spent some time looking at the PPC MMU code in detail last night and my current thinking is that this might be a QEMU bug. The initial physical memory map (before MMU is enabled on startup) looks like this: 0x0000000 - 0x8000000 RAM 0xfff00000 - 0xffffffff OpenBIOS image In setup_mmu(), just before the MMU is enabled, the entire image is memcpy()'d from the OpenBIOS image area to the top area of RAM so we now have this: 0x0000000 - 0x7efffff Free RAM 0x7f00000 - 0x7ffffff OpenBIOS RAM image 0xfff00000 - 0xffffffff OpenBIOS ROM image Now the reason that OpenBIOS actually executes from RAM is because of this section of code in the MMU fault handler code: if (ea >= OF_CODE_START && ea <= 0xffffffffUL) { /* ROM into RAM */ ea -= OF_CODE_START; phys = get_rom_base() + ea; *mode = 0x02; return phys; } Given that OF_CODE_START is 0xfff00000 and get_rom_base() returns 0x7f00000, it's fairly easy to see that accesses to the virtual address range from 0xfff00000 - 0xffffffff are mapped to the OpenBIOS RAM image physical addresses 0x7f00000 - 0x7ffffff. Hence I would expect OpenBIOS to be running from RAM. I wonder if what is happening is that QEMU is not only marking the *physical* range 0xfff00000 - 0xffffffff as read-only, but also the *virtual* range 0xfff00000 - 0xffffffff when the MMU is enabled? If so, I think this may be a QEMU bug since after these addresses have been remapped into RAM then they should be writeable. ATB, Mark.
On 05.01.2013, at 13:05, Mark Cave-Ayland wrote:
On 02/01/13 21:58, Alexander Graf wrote:
Well, .bss is easy as we can just move that down to a r/w memory location. .data is slightly more tricky, as we need to copy data early on. Do we need any global preassigned (.data) variables before we get into C? I highly doubt so :).
Thus, I'd be very inclined to believe that we don't need any asm code at all, making this an all-arch solution.
Okay, I spent some time looking at the PPC MMU code in detail last night and my current thinking is that this might be a QEMU bug.
The initial physical memory map (before MMU is enabled on startup) looks like this:
0x0000000 - 0x8000000 RAM 0xfff00000 - 0xffffffff OpenBIOS image
In setup_mmu(), just before the MMU is enabled, the entire image is memcpy()'d from the OpenBIOS image area to the top area of RAM so we now have this:
0x0000000 - 0x7efffff Free RAM 0x7f00000 - 0x7ffffff OpenBIOS RAM image 0xfff00000 - 0xffffffff OpenBIOS ROM image
Now the reason that OpenBIOS actually executes from RAM is because of this section of code in the MMU fault handler code:
if (ea >= OF_CODE_START && ea <= 0xffffffffUL) { /* ROM into RAM */ ea -= OF_CODE_START; phys = get_rom_base() + ea; *mode = 0x02; return phys; }
Given that OF_CODE_START is 0xfff00000 and get_rom_base() returns 0x7f00000, it's fairly easy to see that accesses to the virtual address range from 0xfff00000 - 0xffffffff are mapped to the OpenBIOS RAM image physical addresses 0x7f00000 - 0x7ffffff. Hence I would expect OpenBIOS to be running from RAM.
So far things make sense. Looking at the first MMU miss we get, QEMU seems to agree with this: Check segment v=00000000fff08614 15 000000002000040f nip=00000000fff08614 lr=00000000fff08608 ir=1 dr=1 pr=0 0 t=16 pte segment: key=0 ds 0 nx 0 vsid 000000000000040f htab_base 0000000007e00000 htab_mask 000000000000ffff hash 000000000000fb07 0 htab=0000000007e00000/000000000000ffff vsid=000000000000040f ptem=00000000000207bf hash=000000000000fb07 PTE access granted ! Load pte from 000000000000c1c0 => 00000000800207bf 0000000007f08002 1 0 0 00000000000207bf found PTE at addr 0000000007f08002 prot=7 ret=0
I wonder if what is happening is that QEMU is not only marking the *physical* range 0xfff00000 - 0xffffffff as read-only, but also the *virtual* range 0xfff00000 - 0xffffffff when the MMU is enabled? If so, I think this may be a QEMU bug since after these addresses have been remapped into RAM then they should be writeable.
The logic that marks virtual pages r/o and the logic that marks physical pages r/o are running in two completely different frameworks. Physical r/o marking happens through the memory api, while virtual r/o marking happens during virtual page lookup. Also, when a virtual address is mapped r/o, the obvious thing happening would be a page fault, not a silent write ignore. But as you can see from the log above, the prot mask is 0x7, meaning rwx: #define PAGE_READ 0x0001 #define PAGE_WRITE 0x0002 #define PAGE_EXEC 0x0004 Do you think you could narrow this down to a simple test case? Somewhere in openbios' init function, access a global variable, check that the write fails and if so, go into an endless loop? That way we might be able to track it down with all logging facilities enabled. Alex
On 05.01.2013, at 16:58, Alexander Graf wrote:
On 05.01.2013, at 13:05, Mark Cave-Ayland wrote:
On 02/01/13 21:58, Alexander Graf wrote:
Well, .bss is easy as we can just move that down to a r/w memory location. .data is slightly more tricky, as we need to copy data early on. Do we need any global preassigned (.data) variables before we get into C? I highly doubt so :).
Thus, I'd be very inclined to believe that we don't need any asm code at all, making this an all-arch solution.
Okay, I spent some time looking at the PPC MMU code in detail last night and my current thinking is that this might be a QEMU bug.
The initial physical memory map (before MMU is enabled on startup) looks like this:
0x0000000 - 0x8000000 RAM 0xfff00000 - 0xffffffff OpenBIOS image
In setup_mmu(), just before the MMU is enabled, the entire image is memcpy()'d from the OpenBIOS image area to the top area of RAM so we now have this:
0x0000000 - 0x7efffff Free RAM 0x7f00000 - 0x7ffffff OpenBIOS RAM image 0xfff00000 - 0xffffffff OpenBIOS ROM image
Now the reason that OpenBIOS actually executes from RAM is because of this section of code in the MMU fault handler code:
if (ea >= OF_CODE_START && ea <= 0xffffffffUL) { /* ROM into RAM */ ea -= OF_CODE_START; phys = get_rom_base() + ea; *mode = 0x02; return phys; }
Given that OF_CODE_START is 0xfff00000 and get_rom_base() returns 0x7f00000, it's fairly easy to see that accesses to the virtual address range from 0xfff00000 - 0xffffffff are mapped to the OpenBIOS RAM image physical addresses 0x7f00000 - 0x7ffffff. Hence I would expect OpenBIOS to be running from RAM.
So far things make sense. Looking at the first MMU miss we get, QEMU seems to agree with this:
Check segment v=00000000fff08614 15 000000002000040f nip=00000000fff08614 lr=00000000fff08608 ir=1 dr=1 pr=0 0 t=16 pte segment: key=0 ds 0 nx 0 vsid 000000000000040f htab_base 0000000007e00000 htab_mask 000000000000ffff hash 000000000000fb07 0 htab=0000000007e00000/000000000000ffff vsid=000000000000040f ptem=00000000000207bf hash=000000000000fb07 PTE access granted ! Load pte from 000000000000c1c0 => 00000000800207bf 0000000007f08002 1 0 0 00000000000207bf found PTE at addr 0000000007f08002 prot=7 ret=0
I wonder if what is happening is that QEMU is not only marking the *physical* range 0xfff00000 - 0xffffffff as read-only, but also the *virtual* range 0xfff00000 - 0xffffffff when the MMU is enabled? If so, I think this may be a QEMU bug since after these addresses have been remapped into RAM then they should be writeable.
The logic that marks virtual pages r/o and the logic that marks physical pages r/o are running in two completely different frameworks. Physical r/o marking happens through the memory api, while virtual r/o marking happens during virtual page lookup. Also, when a virtual address is mapped r/o, the obvious thing happening would be a page fault, not a silent write ignore.
But as you can see from the log above, the prot mask is 0x7, meaning rwx:
#define PAGE_READ 0x0001 #define PAGE_WRITE 0x0002 #define PAGE_EXEC 0x0004
Do you think you could narrow this down to a simple test case? Somewhere in openbios' init function, access a global variable, check that the write fails and if so, go into an endless loop? That way we might be able to track it down with all logging facilities enabled.
Ok, so I did just that with the below patch and got the expected result. Maybe for some reason we're in real mode at the point in time when the breakage occurs? Alex agraf@lychee:/home/agraf/release/qemu> ./ppc-softmmu/qemu-system-ppc -nographic -bios /home/agraf/git/openbios-devel/obj-ppc/openbios-qemu.elf
============================================================= OpenBIOS 1.0 [Oct 1 2012 21:06] Configuration device id QEMU version 1 machine id 2 CPUs: 1 global_var: 0 global_var: 123 Memory: 128M UUID: 00000000-0000-0000-0000-000000000000 CPU type PowerPC,750 Welcome to OpenBIOS v1.0 built on Oct 1 2012 21:06 Trying hd:,\\:tbxi... Trying hd:,\ppc\bootinfo.txt... No valid state has been set by load or init-program
0 > diff --git a/arch/ppc/qemu/init.c b/arch/ppc/qemu/init.c index 61374c3..2b526c3 100644 --- a/arch/ppc/qemu/init.c +++ b/arch/ppc/qemu/init.c @@ -628,6 +628,8 @@ static void ffilll(void) } } +volatile int global_var; + void arch_of_init(void) { @@ -665,6 +667,12 @@ arch_of_init(void) printk("CPUs: %x\n", temp); + printk("global_var: %d\n", global_var); + global_var = 123; + printk("global_var: %d\n", global_var); + if (!global_var) + while(1) { } + ram_size = ofmem->ramsize; printk("Memory: %lldM\n", ram_size / 1024 / 1024);
On 05/01/13 16:08, Alexander Graf wrote:
Do you think you could narrow this down to a simple test case? Somewhere in openbios' init function, access a global variable, check that the write fails and if so, go into an endless loop? That way we might be able to track it down with all logging facilities enabled.
Ok, so I did just that with the below patch and got the expected result. Maybe for some reason we're in real mode at the point in time when the breakage occurs?
Ah yes - wait a sec. The code in question is in arch/ppc/qemu/ofmem.c:hash_page_32() which can be called from both of the ISI/DSI exception handlers dsi_exception()/isi_exception(). And I'm sure I remember reading somewhere over the past day or so that PPC switches to real mode when handling TLB misses - could that be it? (The static that is not being updated is next_grab_slot in hash_page_32(), although I'm fairly sure the same bug would exist for PPC64 in hash_page_64()) ATB, Mark.
Am 05.01.2013 um 17:42 schrieb Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>:
On 05/01/13 16:08, Alexander Graf wrote:
Do you think you could narrow this down to a simple test case? Somewhere in openbios' init function, access a global variable, check that the write fails and if so, go into an endless loop? That way we might be able to track it down with all logging facilities enabled.
Ok, so I did just that with the below patch and got the expected result. Maybe for some reason we're in real mode at the point in time when the breakage occurs?
Ah yes - wait a sec. The code in question is in arch/ppc/qemu/ofmem.c:hash_page_32() which can be called from both of the ISI/DSI exception handlers dsi_exception()/isi_exception(). And I'm sure I remember reading somewhere over the past day or so that PPC switches to real mode when handling TLB misses - could that be it?
Right. HTAB miss handling happens in real mode :). So all we need to do is to manually resolve that global onto its actual location in ram. Since we're already in mmu helper code, that shouldn't be too hard to do, no? Alex
(The static that is not being updated is next_grab_slot in hash_page_32(), although I'm fairly sure the same bug would exist for PPC64 in hash_page_64())
ATB,
Mark.
On 05/01/13 16:52, Alexander Graf wrote:
Ah yes - wait a sec. The code in question is in arch/ppc/qemu/ofmem.c:hash_page_32() which can be called from both of the ISI/DSI exception handlers dsi_exception()/isi_exception(). And I'm sure I remember reading somewhere over the past day or so that PPC switches to real mode when handling TLB misses - could that be it?
Right. HTAB miss handling happens in real mode :).
So all we need to do is to manually resolve that global onto its actual location in ram. Since we're already in mmu helper code, that shouldn't be too hard to do, no?
Alex
Okay - I have the following patch that now works for me on PPC32 and enables BootX to start the Darwin kernel with the BIOS area set back to read-only in QEMU :) Since I couldn't use global variables, I decided to use a fixed offset in the image by reserving 8 bytes (in preparation for PPC64?) in start.S and then applying the offset to the physical memory base similar to the existing ea_to_phys() function. If you could review, and perhaps even enhance with the relevant change for PPC64 then that would be great! Many thanks, Mark.
On 05/01/13 19:43, Mark Cave-Ayland wrote:
If you could review, and perhaps even enhance with the relevant change for PPC64 then that would be great!
Actually here's a revised version that moves the __next_grab_slot back into C which is simpler still and continues to work for me. I guess in this form the same change could be applied as-is to hash_page_64()? ATB, Mark.
On Sat, Jan 5, 2013 at 8:01 PM, Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> wrote:
On 05/01/13 19:43, Mark Cave-Ayland wrote:
If you could review, and perhaps even enhance with the relevant change for PPC64 then that would be great!
Actually here's a revised version that moves the __next_grab_slot back into C which is simpler still and continues to work for me. I guess in this form the same change could be applied as-is to hash_page_64()?
The address arithmetic looks very similar to va2pa() or ea_to_phys(). Could one of those be used instead? Also since this is pretty hot code path, could you initialize the pointer and perform the calculations just once during init?
ATB,
Mark.
-- OpenBIOS http://openbios.org/ Mailinglist: http://lists.openbios.org/mailman/listinfo Free your System - May the Forth be with you
participants (7)
-
Alexander Graf -
Benjamin Herrenschmidt -
Benjamin Herrenschmidt -
Blue Swirl -
Mark Cave-Ayland -
Programmingkid -
Tarl Neustaedter