A patch for RTEMS4.10.0 PowerPC heap space initialization

[Sebastian Huber]

Hello,

I have some questions about this patch.

On 05/07/2011 02:44 PM, Kate Feng wrote:
> Hi Joel,
> 
>    There are bugs in RTEMS4.10.0 and its CVS for the PowerPC heap space
> allocation.  All PPC boards are limited to 32M bytes of memory space due
> to the change in RTEMS4.10 for its work space. The attached patch fixed
> this.
> I consider that it is critical.  Thus, I will submit a PR.
> 
>   Cheers,
>   Kate Feng
> 
> 
> rtems-4.10.0-powerpc-heapspace.diff
> 
> 
> diff -Naur rtems-4.10.0.orig/c/src/lib/libbsp/powerpc/ChangeLog rtems-4.10.0/c/src/lib/libbsp/powerpc/ChangeLog
> --- rtems-4.10.0.orig/c/src/lib/libbsp/powerpc/ChangeLog	2011-03-04 11:56:23.000000000 -0500
> +++ rtems-4.10.0/c/src/lib/libbsp/powerpc/ChangeLog	2011-05-07 08:15:17.000000000 -0400
> @@ -1,3 +1,8 @@
> +2011-05-07      Kate Feng <feng at bnl.gov>
> +
> +	* shared/startup/bspgetworkarea.c : Moved _bsp_sbrk_init to 
> +	c/src/lib/libbsp/shared/bootcard.c
> +	
>  2011-03-04	Joel Sherrill <joel.sherrilL at OARcorp.com>
>  
>  	* shared/start/start.S: Remove conflict markers in comment.
> diff -Naur rtems-4.10.0.orig/c/src/lib/libbsp/powerpc/shared/startup/bspgetworkarea.c rtems-4.10.0/c/src/lib/libbsp/powerpc/shared/startup/bspgetworkarea.c
> --- rtems-4.10.0.orig/c/src/lib/libbsp/powerpc/shared/startup/bspgetworkarea.c	2009-11-29 23:29:47.000000000 -0500
> +++ rtems-4.10.0/c/src/lib/libbsp/powerpc/shared/startup/bspgetworkarea.c	2011-05-07 08:21:08.000000000 -0400
> @@ -31,15 +31,12 @@
>  )
>  {
>    uintptr_t work_size;
> -  uintptr_t spared;
>    uintptr_t work_area;
>  
>    work_area = (uintptr_t)&__rtems_end +
>                rtems_configuration_get_interrupt_stack_size();
>    work_size = (uintptr_t)BSP_mem_size - work_area;
>  
> -  spared = _bsp_sbrk_init( work_area, &work_size );
> -
>    *work_area_start = (void *)work_area,
>    *work_area_size  = work_size;
>    *heap_start      = BSP_BOOTCARD_HEAP_USES_WORK_AREA;
> @@ -54,7 +51,7 @@
>        void *sp = __builtin_frame_address(0);
>        void *end = *work_area_start + *work_area_size;
>        printk(
> -        "work_area_start = 0x%p\n"
> +        "work_area_start = 0x%08\n"

What about 64-bit architectures?

>          "work_area_size = %d 0x%08x\n"
>          "end = 0x%p\n"
>          "heap_start = 0x%p\n"
> diff -Naur rtems-4.10.0.orig/c/src/lib/libbsp/shared/bootcard.c rtems-4.10.0/c/src/lib/libbsp/shared/bootcard.c
> --- rtems-4.10.0.orig/c/src/lib/libbsp/shared/bootcard.c	2009-10-12 16:22:18.000000000 -0400
> +++ rtems-4.10.0/c/src/lib/libbsp/shared/bootcard.c	2011-05-07 07:58:07.000000000 -0400
> @@ -77,8 +77,10 @@
>    uintptr_t  heap_size
>  )
>  {
> -  if ( !rtems_unified_work_area &&
> -       heap_start == BSP_BOOTCARD_HEAP_USES_WORK_AREA) {
> +  uintptr_t spared;
> +
> +  if ( (!rtems_unified_work_area) &&
> +       (heap_start == BSP_BOOTCARD_HEAP_USES_WORK_AREA)) {

Why the extra parentheses?

>      uintptr_t work_space_size = rtems_configuration_get_work_space_size();
>  
>      heap_start = (char *) work_area_start + work_space_size;
> @@ -89,8 +91,13 @@
>        heap_size = heap_size_default;
>      }
>    }
> +  else  /* Kate Feng: if (rtems_unified_work_area) */ {

What is the purpose of this comment?

> +    heap_start = (char *) work_area_start ;

What is the purpose of this cast?  Why the extra ' ' before the ';'?

> +    heap_size = work_area_size;
> +  }
> +  spared = _bsp_sbrk_init( (uintptr_t) heap_start , &heap_size );

Here we force every BSP to provide this sbrk() mechanic.  I don't think this is
good.  This _bsp_sbrk_init() only exists for the PowerPC at the moment.

>  
> -  bsp_libc_init(heap_start, heap_size, 0);
> +  bsp_libc_init((void *) heap_start, heap_size, spared);

What is the purpose of this cast?

>  }
>  
>  /*

-- 
Sebastian Huber, embedded brains GmbH

Address : Obere Lagerstr. 30, D-82178 Puchheim, Germany
Phone   : +49 89 18 90 80 79-6
Fax     : +49 89 18 90 80 79-9
E-Mail  : sebastian.huber at embedded-brains.de
PGP     : Public key available on request.

Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.