A patch for RTEMS4.10.0 PowerPC heap space initialization

[Kate Feng]

Sebastian Huber wrote:
> Hello,
>
> I have some questions about this patch.
>   
Can you or anyone see if the attached new patch acceptable, please ?
I am thinking to submit a PR by tomorrow afternoon if there is
no further question, feedback or suggestion.  It seems a critical bug.

Cheers,
Kate
> 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?
>
>   
>>  }
>>  
>>  /*
>>     
>
>   

-------------- next part --------------
A non-text attachment was scrubbed...
Name: rtems-4.10.0-heapspace_05102011.diff
Type: text/x-patch
Size: 6317 bytes
Desc: not available
URL: <http://lists.rtems.org/pipermail/users/attachments/20110510/71b83cb2/attachment-0001.bin>