[PATCH] libmm patches after fixups

Tue Sep 3 00:27:59 UTC 2013

Hi all,

The attached patches includes :
- Removing hard-coded memory addresses at test cases.
- Some fixups for potential bugs.

Regards,
Hesham

On Sat, Aug 31, 2013 at 5:03 PM, Gedare Bloom <gedare at gwmail.gwu.edu> wrote:
> I'm wondering if the BSP files maybe are the right place for all of
> this mm infrastructure, rather than libcpu. Hm.
>
> Anyway, there are some proposals to eliminate libcpu in the future. It
> might be sensible to consider if all of the "_CPU_Memory_management"
> functions should be relocated into some "bsp_memory_management"
> implementations.
>
> On Sat, Aug 31, 2013 at 11:01 AM, Gedare Bloom <gedare at gwmail.gwu.edu> wrote:
>> if it becomes part of the "bsp" files, then there should be a
>> libbsp/shared/mminit.c file with a default implementation, and then
>> each BSP can override with its particular implementation. So you could
>> put libbsp/arm/shared/mminit.c (or maybe arm-cp15-mminit.c).
>>
>> -Gedare
>>
>> On Fri, Aug 30, 2013 at 5:39 PM, Hesham Moustafa
>> <heshamelmatary at gmail.com> wrote:
>>> On Fri, Aug 30, 2013 at 10:25 PM, Gedare Bloom <gedare at gwmail.gwu.edu> wrote:
>>>> OK, then maybe the MM initialization needs to become part of the
>>>> standard set of BSP functions, like bsp_memory_management_initialize()
>>>> or something similar.
>>>>
>>> Should I just change the name and of _CPU_Memory_management_initilaize to
>>> become bsp_memory_management_initialize ? Or this routine needs to be separated
>>> from libcpu/arm/shared/mm.c and goes to another BSP files domain ?
>>> Maybe shared/startup ?
>>>> On Fri, Aug 30, 2013 at 2:10 PM, Joel Sherrill
>>>> <Joel.Sherrill at oarcorp.com> wrote:
>>>>> :( we really have other violations to fix.
>>>>>
>>>>> I don't know the details of that routine but if it is called outside the implementation of POSIX, sapi, or classic, then we need to discuss it. From my perspective BSPs are just a special type of user code.
>>>>>
>>>>> Gedare Bloom <gedare at gwmail.gwu.edu> wrote:
>>>>>
>>>>>
>>>>> I should have been a tad more clear. The BSP support he adds for ARM
>>>>> call _CPU_Memory_management_Initialize() during startup. AFAIK that
>>>>> name is ok to call from the BSP, or we have other violations to fix
>>>>> elsewhere.
>>>>>
>>>>> On Fri, Aug 30, 2013 at 12:31 PM, Joel Sherrill
>>>>> <Joel.Sherrill at oarcorp.com> wrote:
>>>>>> I know you know the naming rules and the leading underscore means it is private. So if the bsp or app init is to perform it, it needs a public style name
>>>>>>
>>>>>> Gedare Bloom <gedare at gwmail.gwu.edu> wrote:
>>>>>>
>>>>>>
>>>>>> Does the high level score interface need to include
>>>>>> "_Memory_management_Initialize"? Is MMU/MPU initialization only going
>>>>>> to be invoked in low-level BSP startup code?
>>>>>>
>>>>>> The other primary issue I see (other than the hard-coded values in the
>>>>>> tests) is the use of a global SMP lock. I don't know if that is the
>>>>>> right way we want to use locks, or if the lock should be encapsulated
>>>>>> inside of some kind of MM structure (even if that structure is only a
>>>>>> wrapper for the lock).
>>>>>>
>>>>>> -Gedare
>>>>>>
>>>>>> On Wed, Aug 28, 2013 at 12:11 PM, Hesham Moustafa
>>>>>> <heshamelmatary at gmail.com> wrote:
>>>>>>> Hey all,
>>>>>>>
>>>>>>> Please review the new patches after fixups according
>>>>>>> to your reviews.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Hesham
>>>>>>>
>>>>>>> On Tue, Aug 27, 2013 at 6:39 PM, Gedare Bloom <gedare at gwmail.gwu.edu> wrote:
>>>>>>>> For 0003-libmm-libcpu-arm-shared.patch, you should not specify the
>>>>>>>> exception handler is inline, since you register a pointer for it. At
>>>>>>>> any rate can you use the ARM's default exception handler (Sebastian
>>>>>>>> pointed it out in another email). Also, you should probably get rid of
>>>>>>>> the arm_cp15_print_fsr.c file or make it part of your test code if you
>>>>>>>> want to keep it for debugging purposes. There are unused variables in
>>>>>>>> _CPU_Memory_management_Initialize(), and a lot of unused variables in
>>>>>>>> _CPU_Memory_management_Set_attributes(). Get rid of unused variables
>>>>>>>> and get rid of unnecessary initializations of variables.
>>>>>>>>
>>>>>>>> In the arm-cp15.h file it looks like you have a spurious paste:
>>>>>>>> +
>>>>>>>> +/** @} */
>>>>>>>> +
>>>>>>>> +/** @} */
>>>>>>>> +
>>>>>>>> I think you only want one of those.
>>>>>>>>
>>>>>>>> 0004-libmm-armbsps-realview-xilinx-raspberry.patch:
>>>>>>>> * mm_config_table.h: The global variables in these 3 header files
>>>>>>>> should not be given the static attribute. It does not make a lot of
>>>>>>>> sense to me for a variable to be static in a header file.
>>>>>>>>
>>>>>>>> The tests in patch 0005 still need some fixes to deal with workspace
>>>>>>>> allocation, and eventually to make them flexible and not use
>>>>>>>> hard-coded addresses.
>>>>>>>>
>>>>>>>> -Gedare
>>>>>>>>
>>>>>>>> On Mon, Aug 26, 2013 at 8:23 PM, Hesham Moustafa
>>>>>>>> <heshamelmatary at gmail.com> wrote:
>>>>>>>>> Hey all,
>>>>>>>>>
>>>>>>>>> I have fixed up some issues according to reviews on my
>>>>>>>>> latest set of patches. Fixups include:
>>>>>>>>>
>>>>>>>>> - Deleting translation table and created a new translation macro.
>>>>>>>>> (According to Sebastian Review) with introducing flags at high-level
>>>>>>>>> for attributes.
>>>>>>>>> - s/dummy_data_abort_excpetion_handler/default_data_abort_excpetion_handler/
>>>>>>>>> and made it call bsp_generic_fatal
>>>>>>>>> - Created new xxx_fsr_print_description C file (According to Gedare review)
>>>>>>>>> - Moved FSR Regiser Defines to arm-cp15.h (According to Gedare review)
>>>>>>>>> - Deleted hard coded values.
>>>>>>>>> - Remove blanks and white spaces.
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Hesham
>>>>>> _______________________________________________
>>>>>> rtems-devel mailing list
>>>>>> rtems-devel at rtems.org
>>>>>> http://www.rtems.org/mailman/listinfo/rtems-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0005-libmm-test-cases.patch
Type: application/octet-stream
Size: 11987 bytes
Desc: not available
URL: <http://lists.rtems.org/pipermail/devel/attachments/20130903/0f87b758/attachment.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-libmm-bsps-raspberrypi-realview-xilinx.patch
Type: application/octet-stream
Size: 29545 bytes
Desc: not available
URL: <http://lists.rtems.org/pipermail/devel/attachments/20130903/0f87b758/attachment-0001.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-libmm-libcpu-arm-shared.patch
Type: application/octet-stream
Size: 3099 bytes
Desc: not available
URL: <http://lists.rtems.org/pipermail/devel/attachments/20130903/0f87b758/attachment-0002.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-libmm-libcpu-shared.patch
Type: application/octet-stream
Size: 1950 bytes
Desc: not available
URL: <http://lists.rtems.org/pipermail/devel/attachments/20130903/0f87b758/attachment-0003.obj>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-libmm-score-api.patch
Type: application/octet-stream
Size: 5758 bytes
Desc: not available
URL: <http://lists.rtems.org/pipermail/devel/attachments/20130903/0f87b758/attachment-0004.obj>