[OpenBIOS] [PATCH] OBP Memory deallocation

Mark Cave-Ayland mark.cave-ayland at ilande.co.uk
Mon Feb 10 02:40:00 CET 2014


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 at 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.



More information about the OpenBIOS mailing list