[PATCH] libmm patches after fixups

Hesham Moustafa heshamelmatary at gmail.com
Fri Aug 30 21:32:42 UTC 2013


On Fri, Aug 30, 2013 at 6:23 PM, 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?
>
No. I think we should remove the _Memory_management_Initialize and
keep _CPU_Memory_management_Initialize to be invoked at 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).
When I was reviewing the POSIX code, I noticed it's already encapsulated
and manipulated through new interfaces. I think encapsulation  will be helpful
for POSIX API too. I will implement it.

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



More information about the devel mailing list