[RFC PATCH 4/4] arm64: Change mapping for data/rodata/text

[Heinrich Schuchardt]

On 1/30/25 10:29, Ilias Apalodimas wrote:
> Hi Heinrich,
>
> On Thu, 30 Jan 2025 at 12:24, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> On 1/30/25 07:20, Ilias Apalodimas wrote:
>>> Now that we have everything in place switch the page permissions for
>>> .rodata, .text and .data just after we relocate everything in top of the
>>> RAM.
>>>
>>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>>> ---
>>>    common/board_r.c | 7 +++++++
>>>    1 file changed, 7 insertions(+)
>>>
>>> diff --git a/common/board_r.c b/common/board_r.c
>>> index 179259b00de8..38944f600fd6 100644
>>> --- a/common/board_r.c
>>> +++ b/common/board_r.c
>>> @@ -170,6 +170,13 @@ static int initr_reloc_global_data(void)
>>>        efi_save_gd();
>>>
>>>        efi_runtime_relocate(gd->relocaddr, NULL);
>>> +
>>> +#if (IS_ENABLED(CONFIG_ARM64))
>>
>> '#if' should be replaced by 'if (CONFIG_IS_ENABLED())'.
>>
>> Shall we make mmu_set_attrs() a weak function which each architecture
>> can override?
>
> In order to do the IS_ENABLED as you asked, we need to define a weak
> function, otherwise the linker will complain.
> I can add a generic mmu_set_attrs() in the next version. The right way
> to do this in the future is to have a proper MMU API.

For IS_ENABLED you only need the definition in the header file and the
implementation where it is configured.

With the weak function no 'if' is needed.

Best regards

Heinrich

>
>>
>>> +     mmu_set_attrs((u64)(__start_rodata), (u64)(__end_rodata - __start_rodata), 1);
>>> +     mmu_set_attrs((u64)(__start_data), (u64)(__end_data - __start_data), 3);
>>> +     mmu_set_attrs((u64)(__text_start), (u64)(__text_end - __text_start), 2);
>>
>> Please, replace 1, 2, 3 by constants both here and in mmu_set_attrs, e.g.
>>
>> 1 -> MMU_ATTR_RO
>> 2 -> MMU_ATTR_RX
>> 3 -> MMU_ATTR_RW
>
> Yea I am explaining the same thing in the cover letter
>
> Thanks
> /Ilias
>>
>> Best regards
>>
>> Heinrich
>>
>>> +#endif
>>> +
>>>    #endif
>>>
>>>        return 0;
>>