During boot, NextSTEP do memory allocation & deallocation: - Implement dumb_memfree()
- Unmap freed memory (during boot, all the virtual memory is probed, so freed memory must be properly unmapped)
- The pointer given to dumb_memfree() does not always match the pointer returned by dumb_memalloc(). In that case, try to find a memory area which includes the pointer address. Ugly, indeed...
Signed-off-by : Olivier Danet odanet@caramail.com
Index: arch/sparc32/lib.c =================================================================== --- arch/sparc32/lib.c (révision 1257) +++ arch/sparc32/lib.c (copie de travail) @@ -239,10 +245,18 @@ return obp_memalloc(va, size, align); }
-void obp_dumb_memfree(__attribute__((unused))char *va, - __attribute__((unused))unsigned sz) +void obp_dumb_memfree(char *va, unsigned size) { - DPRINTF("obp_dumb_memfree 0x%p (size %d)\n", va, sz); + phys_addr_t phys; + ucell cellmode; + + DPRINTF("obp_dumb_memfree: virta 0x%x, sz %d\n", (unsigned int)va, size); + + phys = ofmem_translate(pointer2cell(va), &cellmode); + + ofmem_unmap(pointer2cell(va), size); + ofmem_release_virt(pointer2cell(va), size); + ofmem_release_phys(phys, size); }
/* Data fault handling routines */ Index: arch/sparc32/ofmem_sparc32.c =================================================================== --- arch/sparc32/ofmem_sparc32.c (révision 1257) +++ arch/sparc32/ofmem_sparc32.c (copie de travail) @@ -133,17 +133,24 @@ /* Generate memory available property entry for Sparc32 */ void ofmem_arch_create_available_entry(phandle_t ph, ucell *availentry, phys_addr_t start, ucell size) { - int i = 0; + int i;
- i += ofmem_arch_encode_physaddr(availentry, start); + i = ofmem_arch_encode_physaddr(availentry, start); availentry[i] = size; }
/* Unmap a set of pages */ void ofmem_arch_unmap_pages(ucell virt, ucell size) { - /* OFMEM re-maps the pages for us, so just ensure the TLB is in sync */ - srmmu_flush_whole_tlb(); + unsigned i; + unsigned long pa; + + for (i = 0; i < size; i += PAGE_SIZE) { + pa = find_pte(virt, 0); + *(uint32_t *)pa = 0; + virt += PAGE_SIZE; + } + srmmu_flush_whole_tlb(); }
/* Map a set of pages */ Index: arch/sparc32/romvec.h =================================================================== --- arch/sparc32/romvec.h (révision 1257) +++ arch/sparc32/romvec.h (copie de travail) @@ -30,8 +30,8 @@ int obp_inst2pkg_handler(int dev_desc); char *obp_dumb_memalloc(char *va, unsigned int size); char *obp_dumb_memalloc_handler(char *va, unsigned int size); -void obp_dumb_memfree(__attribute__((unused))char *va, __attribute__((unused))unsigned sz); -void obp_dumb_memfree_handler(__attribute__((unused))char *va, __attribute__((unused))unsigned sz); +void obp_dumb_memfree(char *va, unsigned size); +void obp_dumb_memfree_handler(char *va, unsigned size); char *obp_dumb_mmap(char *va, int which_io, unsigned int pa, unsigned int size); char *obp_dumb_mmap_handler(char *va, int which_io, unsigned int pa, unsigned int size); void obp_dumb_munmap(__attribute__((unused)) char *va, __attribute__((unused)) unsigned int size); Index: libopenbios/ofmem_common.c =================================================================== --- libopenbios/ofmem_common.c (révision 1257) +++ libopenbios/ofmem_common.c (copie de travail) @@ -887,16 +887,25 @@ cr = *r; *r = (**r).next; free(cr); - return; } + cr = *r; + for ( ; *r && ((**r).next)->start < ea; r = &(**r).next) { + cr = *r; + }
- for( ; *r && ((**r).next)->start < ea; r=&(**r).next ) { + if (((**r).next)->start == ea) { + /* Found matching entry */ + cr = (**r).next; + (**r).next = cr->next; + free(cr); + + } else { + /* Not exactly matching, delete the previous entry, with start < ea */ + free(cr->next); + cr->next = (*r)->next; + return; } - - cr = (**r).next; - (**r).next = cr->next; - free(cr); }
static int remove_range( phys_addr_t ea, ucell size, range_t **r ) ===================================================================
On 09/02/14 22:59, Olivier Danet wrote:
During boot, NextSTEP do memory allocation & deallocation:
Implement dumb_memfree()
Unmap freed memory (during boot, all the virtual memory is probed,
so freed memory must be properly unmapped)
- The pointer given to dumb_memfree() does not always match the pointer
returned by dumb_memalloc(). In that case, try to find a memory area which includes the pointer address. Ugly, indeed...
Signed-off-by : Olivier Danet odanet@caramail.com
Index: arch/sparc32/lib.c
--- arch/sparc32/lib.c (révision 1257) +++ arch/sparc32/lib.c (copie de travail) @@ -239,10 +245,18 @@ return obp_memalloc(va, size, align); }
-void obp_dumb_memfree(__attribute__((unused))char *va,
- __attribute__((unused))unsigned sz)
+void obp_dumb_memfree(char *va, unsigned size) {
- DPRINTF("obp_dumb_memfree 0x%p (size %d)\n", va, sz);
- phys_addr_t phys;
- ucell cellmode;
- DPRINTF("obp_dumb_memfree: virta 0x%x, sz %d\n", (unsigned int)va, size);
- phys = ofmem_translate(pointer2cell(va), &cellmode);
- ofmem_unmap(pointer2cell(va), size);
- ofmem_release_virt(pointer2cell(va), size);
- ofmem_release_phys(phys, size);
}
/* Data fault handling routines */ Index: arch/sparc32/ofmem_sparc32.c =================================================================== --- arch/sparc32/ofmem_sparc32.c (révision 1257) +++ arch/sparc32/ofmem_sparc32.c (copie de travail) @@ -133,17 +133,24 @@ /* Generate memory available property entry for Sparc32 */ void ofmem_arch_create_available_entry(phandle_t ph, ucell *availentry, phys_addr_t start, ucell size) {
- int i = 0;
- int i;
- i += ofmem_arch_encode_physaddr(availentry, start);
- i = ofmem_arch_encode_physaddr(availentry, start);
availentry[i] = size; }
/* Unmap a set of pages */ void ofmem_arch_unmap_pages(ucell virt, ucell size) {
- /* OFMEM re-maps the pages for us, so just ensure the TLB is in sync */
- srmmu_flush_whole_tlb();
- unsigned i;
- unsigned long pa;
- for (i = 0; i < size; i += PAGE_SIZE) {
- pa = find_pte(virt, 0);
- *(uint32_t *)pa = 0;
- virt += PAGE_SIZE;
- }
- srmmu_flush_whole_tlb();
}
/* Map a set of pages */ Index: arch/sparc32/romvec.h =================================================================== --- arch/sparc32/romvec.h (révision 1257) +++ arch/sparc32/romvec.h (copie de travail) @@ -30,8 +30,8 @@ int obp_inst2pkg_handler(int dev_desc); char *obp_dumb_memalloc(char *va, unsigned int size); char *obp_dumb_memalloc_handler(char *va, unsigned int size); -void obp_dumb_memfree(__attribute__((unused))char *va, __attribute__((unused))unsigned sz); -void obp_dumb_memfree_handler(__attribute__((unused))char *va, __attribute__((unused))unsigned sz); +void obp_dumb_memfree(char *va, unsigned size); +void obp_dumb_memfree_handler(char *va, unsigned size); char *obp_dumb_mmap(char *va, int which_io, unsigned int pa, unsigned int size); char *obp_dumb_mmap_handler(char *va, int which_io, unsigned int pa, unsigned int size); void obp_dumb_munmap(__attribute__((unused)) char *va, __attribute__((unused)) unsigned int size); Index: libopenbios/ofmem_common.c =================================================================== --- libopenbios/ofmem_common.c (révision 1257) +++ libopenbios/ofmem_common.c (copie de travail) @@ -887,16 +887,25 @@ cr = *r; *r = (**r).next; free(cr);
return; }
- cr = *r;
- for ( ; *r && ((**r).next)->start < ea; r = &(**r).next) {
- cr = *r;
- }
- for( ; *r && ((**r).next)->start < ea; r=&(**r).next ) {
- if (((**r).next)->start == ea) {
- /* Found matching entry */
- cr = (**r).next;
- (**r).next = cr->next;
- free(cr);
- } else {
- /* Not exactly matching, delete the previous entry, with start < ea */
- free(cr->next);
- cr->next = (*r)->next;
- return;
}
- cr = (**r).next;
- (**r).next = cr->next;
- free(cr);
}
static int remove_range( phys_addr_t ea, ucell size, range_t **r )
The first 2 parts of this patch look fine; however the last part seems absolutely unbelievable(!). I wonder if this could be side-effect of your change to the memory allocation function? It would definitely be worth checking this again once you've had a look at my comments from the previous memory allocation patch.
ATB,
Mark.
On 10/02/2014 02:40, Mark Cave-Ayland wrote:
On 09/02/14 22:59, Olivier Danet wrote:
The first 2 parts of this patch look fine; however the last part seems absolutely unbelievable(!). I wonder if this could be side-effect of your change to the memory allocation function? It would definitely be worth checking this again once you've had a look at my comments from the previous memory allocation patch.
ATB, Mark.
I knew you would not like that ...
Allocation : - In the current code, 'arch/sparc32/lib.c:obp_memalloc(va,size,align)' always calls ofmem_claim_virt(va,size,align) with align=0. So allocating with va=0 does not work.
IIRC, there was also a problem ensuring that the physical and virtual areas had the same size and alignment.
Deallocation :
Here is a log of NextStep booting the ISO, with these crazy patches and this option in OpenBIOS : <option name="CONFIG_DEBUG_OFMEM" type="boolean" value="true"/> and "#define CONFIG_DEBUG_MEM" in arch/sparc32/lib.c ... and a few manual linefeeds
Welcome to OpenBIOS v1.1 built on Feb 10 2014 23:08 Type 'help' for detailed information Trying cdrom:d... Trying cdrom... Not a bootable ELF image Loading a.out image... Loaded 2772176 bytes entry point is 0x4000 bootpath: /iommu@0,10000000/sbus@0,10001000/espdma@5,8400000/esp@5,8800000/sd@2,0 start_adr: 0 num_bytes: 8000000
Jumping to entry point 00004000 for type 00000005... switching to new context:
obp_dumb_memalloc: virta 0x0, sz 7260 obp_memalloc: virta 0x0, sz 7260, align 7260 OFMEM: ofmem_claim phys=ffffffffffffffff size=00002000 align=00001c5c OFMEM: warning: bad alignment 00001c5c rounded up to 00002000 OFMEM: ofmem_claim_virt virt=00000000 size=00002000 align=00001c5c OFMEM: warning: bad alignment 00001c5c rounded up to 00002000 OFMEM: ofmem_claim_virt virt=00000000 size=00002000 align=00001000 OFMEM: ofmem_map_page_range fdffc000 -> 006fde000 00002000 mode 000000bc
obp_dumb_memfree: virta 0xfdffc000, sz 7260 OFMEM: ofmem_unmap fdffc000 00001c5c OFMEM: unmap_page_range found fdffc000 -> 006fde000 00002000 mode 000000bc OFMEM: ofmem_release_virt addr=fdffc000 size=00001c5c OFMEM: ofmem_release_phys addr=006fde000 size=00001c5c
obp_dumb_memalloc: virta 0x0, sz 8192 obp_memalloc: virta 0x0, sz 8192, align 8192 OFMEM: ofmem_claim phys=ffffffffffffffff size=00002000 align=00002000 OFMEM: ofmem_claim_virt virt=00000000 size=00002000 align=00002000 OFMEM: ofmem_claim_virt virt=00000000 size=00002000 align=00001000 OFMEM: ofmem_map_page_range fdffa000 -> 006fde000 00002000 mode 000000bc
obp_dumb_memalloc: virta 0x0, sz 8192 obp_memalloc: virta 0x0, sz 8192, align 8192 OFMEM: ofmem_claim phys=ffffffffffffffff size=00002000 align=00002000 OFMEM: ofmem_claim_virt virt=00000000 size=00002000 align=00002000 OFMEM: ofmem_claim_virt virt=00000000 size=00002000 align=00001000 OFMEM: ofmem_map_page_range fdff6000 -> 006fdc000 00002000 mode 000000bc
obp_dumb_memalloc: virta 0x0, sz 636 obp_memalloc: virta 0x0, sz 636, align 636 OFMEM: ofmem_claim phys=ffffffffffffffff size=00001000 align=0000027c OFMEM: warning: bad alignment 0000027c rounded up to 00001000 OFMEM: ofmem_claim_virt virt=00000000 size=00001000 align=0000027c OFMEM: warning: bad alignment 0000027c rounded up to 00001000 OFMEM: ofmem_claim_virt virt=00000000 size=00001000 align=00001000 OFMEM: ofmem_map_page_range fdff4000 -> 006fdb000 00001000 mode 000000bc
obp_dumb_memalloc: virta 0x80000, sz 49152 obp_memalloc: virta 0x80000, sz 49152, align 49152 OFMEM: ofmem_claim phys=ffffffffffffffff size=0000c000 align=0000c000 OFMEM: warning: bad alignment 0000c000 rounded up to 00010000 OFMEM: ofmem_claim_virt virt=00080000 size=0000c000 align=0000c000 OFMEM: warning: bad alignment 0000c000 rounded up to 00010000 OFMEM: ofmem_claim_virt virt=00080000 size=0000c000 align=00000000 OFMEM: ofmem_map_page_range 00080000 -> 006fc0000 0000c000 mode 000000bc OFMEM: mapping altered virt=00080000)
obp_dumb_memalloc: virta 0x8c000, sz 8192 obp_memalloc: virta 0x8c000, sz 8192, align 8192 OFMEM: ofmem_claim phys=ffffffffffffffff size=00002000 align=00002000 OFMEM: ofmem_claim_virt virt=0008c000 size=00002000 align=00002000 OFMEM: ofmem_claim_virt virt=0008c000 size=00002000 align=00000000 OFMEM: ofmem_map_page_range 0008c000 -> 006fd8000 00002000 mode 000000bc OFMEM: mapping altered virt=0008c000)
obp_dumb_memalloc: virta 0x0, sz 32 obp_memalloc: virta 0x0, sz 32, align 32 OFMEM: ofmem_claim phys=ffffffffffffffff size=00001000 align=00000020 OFMEM: ofmem_claim_virt virt=00000000 size=00001000 align=00000020 OFMEM: ofmem_claim_virt virt=00000000 size=00001000 align=00001000 OFMEM: ofmem_map_page_range fdff0000 -> 006fda000 00001000 mode 000000bc
initsyms: obp_dumb_memfree: virta 0xfdff4000, sz 636 OFMEM: ofmem_unmap fdff4000 0000027c OFMEM: unmap_page_range found fdff4000 -> 006fdb000 00001000 mode 000000bc OFMEM: ofmem_release_virt addr=fdff4000 size=0000027c OFMEM: ofmem_release_phys addr=006fdb000 size=0000027c
obp_dumb_memfree: virta 0xfdff6000, sz 8192 OFMEM: ofmem_unmap fdff6000 00002000 OFMEM: unmap_page_range found fdff6000 -> 006fdc000 00002000 mode 000000bc OFMEM: ofmem_release_virt addr=fdff6000 size=00002000 OFMEM: ofmem_release_phys addr=006fdc000 size=00002000 obp_dumb_memalloc: virta 0xc3000, sz 57344
obp_memalloc: virta 0xc3000, sz 57344, align 57344 OFMEM: ofmem_claim phys=ffffffffffffffff size=0000e000 align=0000e000 OFMEM: warning: bad alignment 0000e000 rounded up to 00010000 OFMEM: ofmem_claim_virt virt=000c3000 size=0000e000 align=0000e000 OFMEM: warning: bad alignment 0000e000 rounded up to 00010000 OFMEM: ofmem_claim_virt virt=000c3000 size=0000e000 align=00000000 OFMEM: ofmem_map_page_range 000c3000 -> 006fb0000 0000e000 mode 000000bc OFMEM: mapping altered virt=000c3000)
NEXTSTEP boot v3.3.4.17 114436 memory
NEXTSTEP will start up in 10 seconds, or you can: Type -v and press Return to start up NEXTSTEP with diagnostic messages Type ? and press Return to learn about advanced startup options Type any other character to stop NEXTSTEP from starting up automatically
boot:
Type 1 to use the English language and USA keyboard while installing NEXTSTEP. Tapez 2 pour installer NEXTSTEP avec un clavier et des messages francais. Eingabe 3 fuer NEXTSTEP-Installation mit deutscher Sprache und Tastatur. Premi 4 per installare NEXTSTEP usando lingua italiana e tastiera italiana. Pulse 5 para usar el idioma y el teclado espanol en la instalacion de NEXTSTEP. Skriv 6 for att installera NEXTSTEP pa svenska med svenskt tangentbord.
---> 1
This program is for installing NEXTSTEP on a hard disk. THIS IS NOT AN UPGRADE: ANY EXISTING NEXTSTEP FILES WILL BE DELETED.
If you have files on your hard disk that you want to keep, quit this program and copy what you want to keep onto another disk.
Type 1 to prepare to install NEXTSTEP. Type 2 to quit this installation program.
---> 1 Loading NEXTSTEP
obp_memalloc: virta 0xf0000000, sz 1490984, align 262144 OFMEM: ofmem_claim phys=ffffffffffffffff size=0016d000 align=00040000 OFMEM: ofmem_claim_virt virt=f0000000 size=0016d000 align=00040000 OFMEM: ofmem_claim_virt virt=f0000000 size=0016d000 align=00000000 OFMEM: ofmem_map_page_range f0000000 -> 006e40000 0016d000 mode 000000bc obp_dumb_memalloc: virta 0x0, sz 122952
obp_memalloc: virta 0x0, sz 122952, align 122952 OFMEM: ofmem_claim phys=ffffffffffffffff size=0001f000 align=0001e048 OFMEM: warning: bad alignment 0001e048 rounded up to 00020000 OFMEM: ofmem_claim_virt virt=00000000 size=0001f000 align=0001e048 OFMEM: warning: bad alignment 0001e048 rounded up to 00020000 OFMEM: ofmem_claim_virt virt=00000000 size=0001f000 align=00001000 OFMEM: ofmem_map_page_range fdfb1000 -> 006e20000 0001f000 mode 000000bc
initsyms: Reading NEXTSTEP configuration obp_dumb_memalloc: virta 0x30000, sz 163840
obp_memalloc: virta 0x30000, sz 163840, align 163840 OFMEM: ofmem_claim phys=ffffffffffffffff size=00028000 align=00028000 OFMEM: warning: bad alignment 00028000 rounded up to 00040000 OFMEM: ofmem_claim_virt virt=00030000 size=00028000 align=00028000 OFMEM: warning: bad alignment 00028000 rounded up to 00040000 OFMEM: ofmem_claim_virt virt=00030000 size=00028000 align=00000000 OFMEM: ofmem_map_page_range 00030000 -> 006dc0000 00028000 mode 000000bc OFMEM: mapping altered virt=00030000)
obp_memalloc: virta 0xf016e000, sz 4194304, align 4 OFMEM: ofmem_claim phys=ffffffffffffffff size=00400000 align=00000004 OFMEM: ofmem_claim_virt virt=f016e000 size=00400000 align=00000004 OFMEM: ofmem_claim_virt virt=f016e000 size=00400000 align=00000000 OFMEM: ofmem_map_page_range f016e000 -> 0069c0000 00400000 mode 000000bc
obp_memalloc: virta 0xf056e000, sz 1048576, align 4 OFMEM: ofmem_claim phys=ffffffffffffffff size=00100000 align=00000004 OFMEM: ofmem_claim_virt virt=f056e000 size=00100000 align=00000004 OFMEM: ofmem_claim_virt virt=f056e000 size=00100000 align=00000000 OFMEM: ofmem_map_page_range f056e000 -> 0068c0000 00100000 mode 000000bc Starting NEXTSTEP
obp_dumb_memfree: virta 0xf0180000, sz 4120576 OFMEM: ofmem_unmap f0180000 003ee000 OFMEM: unmap_page_range found f0180000 -> 0069d2000 003ee000 mode 000000bc OFMEM: ofmem_release_virt addr=f0180000 size=003ee000 OFMEM: ofmem_release_phys addr=0069d2000 size=003ee000
obp_dumb_memfree: virta 0xf056e000, sz 1048576 OFMEM: ofmem_unmap f056e000 00100000 OFMEM: unmap_page_range found f056e000 -> 0068c0000 00100000 mode 000000bc OFMEM: ofmem_release_virt addr=f056e000 size=00100000 OFMEM: ofmem_release_phys addr=0068c0000 size=00100000
obp_dumb_memfree: virta 0x30000, sz 163840 OFMEM: ofmem_unmap 00030000 00028000 OFMEM: unmap_page_range found 00030000 -> 006dc0000 00028000 mode 000000bc OFMEM: ofmem_release_virt addr=00030000 size=00028000 OFMEM: ofmem_release_phys addr=006dc0000 size=00028000
==================================================================================
The big problem is in : obp_dumb_memfree: virta 0xf0180000, sz 4120576 (=0x3EE000)
No memory starting at 0xf0180000 was ever allocated! There is instead something at 0xf016e000, sz 4194304 (0x400000)
There is certainly a relation, as the end match : f016e000+400000 = F056E000 f0180000+3EE000 = F056E000
I tried a bit to trace Sun's OFW with gdb, but with no luck. I don't know whether it is a bug in OpenBIOS, QEMU or NextSTEP. It certainly does not look reasonable...
Olivier
On 11/02/14 00:44, Olivier Danet wrote:
I knew you would not like that ...
Allocation :
- In the current code, 'arch/sparc32/lib.c:obp_memalloc(va,size,align)'
always calls ofmem_claim_virt(va,size,align) with align=0. So allocating with va=0 does not work.
IIRC, there was also a problem ensuring that the physical and virtual areas had the same size and alignment.
Deallocation :
Here is a log of NextStep booting the ISO, with these crazy patches and this option in OpenBIOS :
<option name="CONFIG_DEBUG_OFMEM" type="boolean" value="true"/> and "#define CONFIG_DEBUG_MEM" in arch/sparc32/lib.c ... and a few manual linefeeds
(big cut)
The big problem is in : obp_dumb_memfree: virta 0xf0180000, sz 4120576 (=0x3EE000)
No memory starting at 0xf0180000 was ever allocated! There is instead something at 0xf016e000, sz 4194304 (0x400000)
There is certainly a relation, as the end match : f016e000+400000 = F056E000 f0180000+3EE000 = F056E000
I tried a bit to trace Sun's OFW with gdb, but with no luck. I don't know whether it is a bug in OpenBIOS, QEMU or NextSTEP. It certainly does not look reasonable...
Thanks for the traces - these are really helpful! :) First thing to check is, are these traces from an unpatched SVN trunk? For example your output looks like this:
obp_memalloc: virta 0xf016e000, sz 4194304, align 4 OFMEM: ofmem_claim phys=ffffffffffffffff size=00400000 align=00000004 OFMEM: ofmem_claim_virt virt=f016e000 size=00400000 align=00000004 OFMEM: ofmem_claim_virt virt=f016e000 size=00400000 align=00000000 OFMEM: ofmem_map_page_range f016e000 -> 0069c0000 00400000 mode 000000bc
According to obp_memalloc(), ofmem_claim_virt() should only be called once whereas in these traces it is being called twice?
ATB,
Mark.
On 10/02/14 01:40, Mark Cave-Ayland wrote:
On 09/02/14 22:59, Olivier Danet wrote:
During boot, NextSTEP do memory allocation & deallocation:
Implement dumb_memfree()
Unmap freed memory (during boot, all the virtual memory is probed,
so freed memory must be properly unmapped)
- The pointer given to dumb_memfree() does not always match the pointer
returned by dumb_memalloc(). In that case, try to find a memory area which includes the pointer address. Ugly, indeed...
Signed-off-by : Olivier Danet odanet@caramail.com
Index: arch/sparc32/lib.c
--- arch/sparc32/lib.c (révision 1257) +++ arch/sparc32/lib.c (copie de travail) @@ -239,10 +245,18 @@ return obp_memalloc(va, size, align); }
-void obp_dumb_memfree(__attribute__((unused))char *va,
- __attribute__((unused))unsigned sz)
+void obp_dumb_memfree(char *va, unsigned size) {
- DPRINTF("obp_dumb_memfree 0x%p (size %d)\n", va, sz);
- phys_addr_t phys;
- ucell cellmode;
- DPRINTF("obp_dumb_memfree: virta 0x%x, sz %d\n", (unsigned int)va,
size);
- phys = ofmem_translate(pointer2cell(va), &cellmode);
- ofmem_unmap(pointer2cell(va), size);
- ofmem_release_virt(pointer2cell(va), size);
- ofmem_release_phys(phys, size);
}
/* Data fault handling routines */ Index: arch/sparc32/ofmem_sparc32.c =================================================================== --- arch/sparc32/ofmem_sparc32.c (révision 1257) +++ arch/sparc32/ofmem_sparc32.c (copie de travail) @@ -133,17 +133,24 @@ /* Generate memory available property entry for Sparc32 */ void ofmem_arch_create_available_entry(phandle_t ph, ucell *availentry, phys_addr_t start, ucell size) {
- int i = 0;
- int i;
- i += ofmem_arch_encode_physaddr(availentry, start);
- i = ofmem_arch_encode_physaddr(availentry, start);
availentry[i] = size; }
/* Unmap a set of pages */ void ofmem_arch_unmap_pages(ucell virt, ucell size) {
- /* OFMEM re-maps the pages for us, so just ensure the TLB is in sync */
- srmmu_flush_whole_tlb();
- unsigned i;
- unsigned long pa;
- for (i = 0; i < size; i += PAGE_SIZE) {
- pa = find_pte(virt, 0);
- *(uint32_t *)pa = 0;
- virt += PAGE_SIZE;
- }
- srmmu_flush_whole_tlb();
}
/* Map a set of pages */ Index: arch/sparc32/romvec.h =================================================================== --- arch/sparc32/romvec.h (révision 1257) +++ arch/sparc32/romvec.h (copie de travail) @@ -30,8 +30,8 @@ int obp_inst2pkg_handler(int dev_desc); char *obp_dumb_memalloc(char *va, unsigned int size); char *obp_dumb_memalloc_handler(char *va, unsigned int size); -void obp_dumb_memfree(__attribute__((unused))char *va, __attribute__((unused))unsigned sz); -void obp_dumb_memfree_handler(__attribute__((unused))char *va, __attribute__((unused))unsigned sz); +void obp_dumb_memfree(char *va, unsigned size); +void obp_dumb_memfree_handler(char *va, unsigned size); char *obp_dumb_mmap(char *va, int which_io, unsigned int pa, unsigned int size); char *obp_dumb_mmap_handler(char *va, int which_io, unsigned int pa, unsigned int size); void obp_dumb_munmap(__attribute__((unused)) char *va, __attribute__((unused)) unsigned int size); Index: libopenbios/ofmem_common.c =================================================================== --- libopenbios/ofmem_common.c (révision 1257) +++ libopenbios/ofmem_common.c (copie de travail) @@ -887,16 +887,25 @@ cr = *r; *r = (**r).next; free(cr);
return; }
- cr = *r;
- for ( ; *r && ((**r).next)->start < ea; r = &(**r).next) {
- cr = *r;
- }
- for( ; *r && ((**r).next)->start < ea; r=&(**r).next ) {
- if (((**r).next)->start == ea) {
- /* Found matching entry */
- cr = (**r).next;
- (**r).next = cr->next;
- free(cr);
- } else {
- /* Not exactly matching, delete the previous entry, with start < ea */
- free(cr->next);
- cr->next = (*r)->next;
- return;
}
- cr = (**r).next;
- (**r).next = cr->next;
- free(cr);
}
static int remove_range( phys_addr_t ea, ucell size, range_t **r )
The first 2 parts of this patch look fine; however the last part seems absolutely unbelievable(!). I wonder if this could be side-effect of your change to the memory allocation function? It would definitely be worth checking this again once you've had a look at my comments from the previous memory allocation patch.
I spent a bit of time looking at this over the weekend, and during some testing was surprised to see that OFMEM's unmap_pages() would do the right thing in that it would locate the "correct" memory region, even if the specified boundaries weren't exactly the same as those used during creation. This meant that the failure was purely caused by remove_range_() being unable to handle the subsequent release with the same parameters.
With some further work, I came up with the attached patch which uses the same algorithm as unmap_page_range() to match regions for removal. I'm more confident in this method since it was written by Igor (rather than me!), gives a consistent behaviour between the two sets of functions, and as an extra plus is smaller to boot.
A quick spin around my SPARC32/SPARC64 test suite shows no regressions, so please test and let me know what you think.
ATB,
Mark.
On 26/03/2014 16:18, Mark Cave-Ayland wrote:
On 10/02/14 01:40, Mark Cave-Ayland wrote:
On 09/02/14 22:59, Olivier Danet wrote:
During boot, NextSTEP do memory allocation & deallocation:
Implement dumb_memfree()
Unmap freed memory (during boot, all the virtual memory is probed,
so freed memory must be properly unmapped)
- The pointer given to dumb_memfree() does not always match the pointer
returned by dumb_memalloc(). In that case, try to find a memory area which includes the pointer address. Ugly, indeed...
Signed-off-by : Olivier Danet odanet@caramail.com
Index: arch/sparc32/lib.c
--- arch/sparc32/lib.c (révision 1257) +++ arch/sparc32/lib.c (copie de travail) @@ -239,10 +245,18 @@ return obp_memalloc(va, size, align); }
-void obp_dumb_memfree(__attribute__((unused))char *va,
- __attribute__((unused))unsigned sz)
+void obp_dumb_memfree(char *va, unsigned size) {
- DPRINTF("obp_dumb_memfree 0x%p (size %d)\n", va, sz);
- phys_addr_t phys;
- ucell cellmode;
- DPRINTF("obp_dumb_memfree: virta 0x%x, sz %d\n", (unsigned int)va,
size);
- phys = ofmem_translate(pointer2cell(va), &cellmode);
- ofmem_unmap(pointer2cell(va), size);
- ofmem_release_virt(pointer2cell(va), size);
- ofmem_release_phys(phys, size);
}
/* Data fault handling routines */ Index: arch/sparc32/ofmem_sparc32.c =================================================================== --- arch/sparc32/ofmem_sparc32.c (révision 1257) +++ arch/sparc32/ofmem_sparc32.c (copie de travail) @@ -133,17 +133,24 @@ /* Generate memory available property entry for Sparc32 */ void ofmem_arch_create_available_entry(phandle_t ph, ucell *availentry, phys_addr_t start, ucell size) {
- int i = 0;
- int i;
- i += ofmem_arch_encode_physaddr(availentry, start);
- i = ofmem_arch_encode_physaddr(availentry, start);
availentry[i] = size; }
/* Unmap a set of pages */ void ofmem_arch_unmap_pages(ucell virt, ucell size) {
- /* OFMEM re-maps the pages for us, so just ensure the TLB is in sync */
- srmmu_flush_whole_tlb();
- unsigned i;
- unsigned long pa;
- for (i = 0; i < size; i += PAGE_SIZE) {
- pa = find_pte(virt, 0);
- *(uint32_t *)pa = 0;
- virt += PAGE_SIZE;
- }
- srmmu_flush_whole_tlb();
}
/* Map a set of pages */ Index: arch/sparc32/romvec.h =================================================================== --- arch/sparc32/romvec.h (révision 1257) +++ arch/sparc32/romvec.h (copie de travail) @@ -30,8 +30,8 @@ int obp_inst2pkg_handler(int dev_desc); char *obp_dumb_memalloc(char *va, unsigned int size); char *obp_dumb_memalloc_handler(char *va, unsigned int size); -void obp_dumb_memfree(__attribute__((unused))char *va, __attribute__((unused))unsigned sz); -void obp_dumb_memfree_handler(__attribute__((unused))char *va, __attribute__((unused))unsigned sz); +void obp_dumb_memfree(char *va, unsigned size); +void obp_dumb_memfree_handler(char *va, unsigned size); char *obp_dumb_mmap(char *va, int which_io, unsigned int pa, unsigned int size); char *obp_dumb_mmap_handler(char *va, int which_io, unsigned int pa, unsigned int size); void obp_dumb_munmap(__attribute__((unused)) char *va, __attribute__((unused)) unsigned int size); Index: libopenbios/ofmem_common.c =================================================================== --- libopenbios/ofmem_common.c (révision 1257) +++ libopenbios/ofmem_common.c (copie de travail) @@ -887,16 +887,25 @@ cr = *r; *r = (**r).next; free(cr);
return; }
- cr = *r;
- for ( ; *r && ((**r).next)->start < ea; r = &(**r).next) {
- cr = *r;
- }
- for( ; *r && ((**r).next)->start < ea; r=&(**r).next ) {
- if (((**r).next)->start == ea) {
- /* Found matching entry */
- cr = (**r).next;
- (**r).next = cr->next;
- free(cr);
- } else {
- /* Not exactly matching, delete the previous entry, with start < ea */
- free(cr->next);
- cr->next = (*r)->next;
- return;
}
- cr = (**r).next;
- (**r).next = cr->next;
- free(cr);
}
static int remove_range( phys_addr_t ea, ucell size, range_t **r )
The first 2 parts of this patch look fine; however the last part seems absolutely unbelievable(!). I wonder if this could be side-effect of your change to the memory allocation function? It would definitely be worth checking this again once you've had a look at my comments from the previous memory allocation patch.
I spent a bit of time looking at this over the weekend, and during some testing was surprised to see that OFMEM's unmap_pages() would do the right thing in that it would locate the "correct" memory region, even if the specified boundaries weren't exactly the same as those used during creation. This meant that the failure was purely caused by remove_range_() being unable to handle the subsequent release with the same parameters.
With some further work, I came up with the attached patch which uses the same algorithm as unmap_page_range() to match regions for removal. I'm more confident in this method since it was written by Igor (rather than me!), gives a consistent behaviour between the two sets of functions, and as an extra plus is smaller to boot.
A quick spin around my SPARC32/SPARC64 test suite shows no regressions, so please test and let me know what you think.
ATB,
Mark.
Hello Mark I tested the patch and found no regression either,.. and it pleases NextStep.
Good !
Olivier
On 29/03/14 20:00, Olivier Danet wrote:
Hello Mark I tested the patch and found no regression either,.. and it pleases NextStep.
Good !
Olivier
Excellent! I've just run through my complete set of OpenBIOS boot tests for all architectures and there were no regressions there either, so I've just committed it to SVN trunk.
ATB,
Mark.
-
Mark Cave-Ayland -
Olivier Danet