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

[Ilias Apalodimas]

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.

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