Hi all,
I've spent a few hours looking at converting SPARC32 over to OFMEM and have hit a fairly fundamental hurdle at the start - the OFMEM code doesn't handle physical addresses properly.
In terms of mapping I/O devices into address space, the first problem is with the ofmem_arch_early_map_pages() API which looks like this:
ofmem_arch_early_map_pages(ucell phys, ucell virt, ucell size, ucell mode);
The problem here is that when setting up the QEMU firmware configuration interface with an address of 0xd00000510, once we hit the page mapping functions we lose the top 8 bits of the physical address since sizeof(phys_addr_t) > size(ucell) on SPARC32. This causes incorrect physical address mappings.
My current thinking is that all physical addresses should be changed to phys_addr_t in the OFMEM APIs, but this is slightly more complicated because the same range_t structure is used for storing both physical and virtual range mappings.
I believe that if I change the OFMEM range_t structure to use phys_addr_t instead of ucell then this should work assuming that sizeof(phys_addr_t) is always >= sizeof(ucell). Looking at the OF spec it seems that all virtual addresses always fit within 1 Forth cell, and so virtual addresses should be fine in their present form.
Can anyone see a flaw in the above logic before I go ahead and code it up?
ATB,
Mark.
Hi Mark,
Am 06.11.2010 um 12:24 schrieb Mark Cave-Ayland:
[...] the OFMEM code doesn't handle physical addresses properly.
In terms of mapping I/O devices into address space, the first problem is with the ofmem_arch_early_map_pages() API which looks like this:
ofmem_arch_early_map_pages(ucell phys, ucell virt, ucell size, ucell mode);
The problem here is that when setting up the QEMU firmware configuration interface with an address of 0xd00000510, once we hit the page mapping functions we lose the top 8 bits of the physical address since sizeof(phys_addr_t) > size(ucell) on SPARC32. This causes incorrect physical address mappings.
My current thinking is that all physical addresses should be changed to phys_addr_t in the OFMEM APIs, but this is slightly more complicated because the same range_t structure is used for storing both physical and virtual range mappings.
I believe that if I change the OFMEM range_t structure to use phys_addr_t instead of ucell then this should work assuming that sizeof(phys_addr_t) is always >= sizeof(ucell). Looking at the OF spec it seems that all virtual addresses always fit within 1 Forth cell, and so virtual addresses should be fine in their present form.
Can anyone see a flaw in the above logic before I go ahead and code it up?
I was planning to do the same thing for ppc64, please go ahead.
The alternative would've been to create separate range structs - cleaner API-wise, but then the range logic would need to be duplicated, which I consider a big con. ;)
Regards, Andreas
Andreas Färber wrote:
I was planning to do the same thing for ppc64, please go ahead.
The alternative would've been to create separate range structs - cleaner API-wise, but then the range logic would need to be duplicated, which I consider a big con. ;)
Yeah, that's what I was thinking. I know that the old SPARC64 code used to reference addresses in the translation_t struct linked list directly in the MMU miss handlers, but that section has now been replaced with C code. Are there any similar gotchas on PPC?
I'm also thinking it would be nice to have a couple of functions for each arch such as POP_PHYS() and PUSH_PHYS() which will PUSH/POP stack items into phys_addr_t and vice-versa. I think this will also allow a nice tidy up of the areas where these conversions take place, e.g. the MMU handlers.
Looks like I'm busy for the rest of the day, so will try and have a stab at this tomorrow.
ATB,
Mark.
Am 06.11.2010 um 13:05 schrieb Mark Cave-Ayland:
Andreas Färber wrote:
I was planning to do the same thing for ppc64, please go ahead. The alternative would've been to create separate range structs - cleaner API-wise, but then the range logic would need to be duplicated, which I consider a big con. ;)
Yeah, that's what I was thinking. I know that the old SPARC64 code used to reference addresses in the translation_t struct linked list directly in the MMU miss handlers, but that section has now been replaced with C code. Are there any similar gotchas on PPC?
Not that I'm aware of. Alex?
We may need to tidy arch/ppc/qemu/ofmem.c though wrt ucell. I noticed there's a private get_ram/rom/sth_size() function defined in the header but it's only used further down in the same file, so I'll probably change the type to phys_addr_t and make it static.
I'm also thinking it would be nice to have a couple of functions for each arch such as POP_PHYS() and PUSH_PHYS() which will PUSH/POP stack items into phys_addr_t and vice-versa. I think this will also allow a nice tidy up of the areas where these conversions take place, e.g. the MMU handlers.
Sounds fine to me, and making them inline functions rather than macros will probably make Blue happy, too. :)
Looks like I'm busy for the rest of the day, so will try and have a stab at this tomorrow.
Great. I've been postponing this to not open another can of worms on ppc64.
Andreas
On 06.11.2010, at 08:24, Andreas Färber wrote:
Am 06.11.2010 um 13:05 schrieb Mark Cave-Ayland:
Andreas Färber wrote:
I was planning to do the same thing for ppc64, please go ahead. The alternative would've been to create separate range structs - cleaner API-wise, but then the range logic would need to be duplicated, which I consider a big con. ;)
Yeah, that's what I was thinking. I know that the old SPARC64 code used to reference addresses in the translation_t struct linked list directly in the MMU miss handlers, but that section has now been replaced with C code. Are there any similar gotchas on PPC?
Not that I'm aware of. Alex?
The MMU miss handler code is very simple:
static ucell ea_to_phys( ucell ea, ucell *mode ) { ucell phys;
if (ea >= OF_CODE_START) { /* ROM into RAM */ ea -= OF_CODE_START; phys = get_rom_base() + ea; *mode = 0x02; return phys; }
phys = ofmem_translate(ea, mode); if( phys == -1 ) { phys = ea; *mode = ofmem_arch_default_translation_mode( phys );
/* print_virt_range(); */ /* print_phys_range(); */ /* print_trans(); */ } return phys; }
[...]
phys = ea_to_phys(nip, &mode); hash_page( nip, phys, mode );
So as long as you're in openBIOS code or no ofmem map is available, it maps linearly, otherwise it uses normal ofmem handlers. I don't see any list involved here :).
Alex
Am 07.11.2010 um 01:38 schrieb Alexander Graf:
On 06.11.2010, at 08:24, Andreas Färber wrote:
Am 06.11.2010 um 13:05 schrieb Mark Cave-Ayland:
Andreas Färber wrote:
I was planning to do the same thing for ppc64, please go ahead. The alternative would've been to create separate range structs - cleaner API-wise, but then the range logic would need to be duplicated, which I consider a big con. ;)
Yeah, that's what I was thinking. I know that the old SPARC64 code used to reference addresses in the translation_t struct linked list directly in the MMU miss handlers, but that section has now been replaced with C code. Are there any similar gotchas on PPC?
Not that I'm aware of. Alex?
The MMU miss handler code is very simple:
[...]
phys = ea_to_phys(nip, &mode); hash_page( nip, phys, mode );
Ah! I played with that earlier today and tried to implement early mapping based on those two lines:
diff --git a/arch/ppc/qemu/ofmem.c b/arch/ppc/qemu/ofmem.c index a507009..594defe 100644 --- a/arch/ppc/qemu/ofmem.c +++ b/arch/ppc/qemu/ofmem.c @@ -128,9 +128,15 @@ void ofmem_arch_unmap_pages(ucell virt, ucell size) /* kill page mappings in provided range */ }
+//static ucell ea_to_phys( ucell ea, ucell *mode ); +static void hash_page( unsigned long ea, unsigned long phys, ucell mode ); + void ofmem_arch_early_map_pages(ucell phys, ucell virt, ucell size, ucell mode) { - /* none yet */ + ucell offset; + for (offset = 0; offset < size; offset += 0x1000) { + hash_page(virt + offset, phys + offset, mode); + } }
retain_t *ofmem_arch_get_retained(void)
No regressions on ppc, but no change on ppc64.
While it doesn't matter here, we might change unsigned long phys to phys_addr_t phys.
Andreas
So as long as you're in openBIOS code or no ofmem map is available, it maps linearly, otherwise it uses normal ofmem handlers. I don't see any list involved here :).
Alex
-
Alexander Graf -
Andreas Färber -
Mark Cave-Ayland