Hi Heinrich, On Thu, 30 Jan 2025 at 12:24, Heinrich Schuchardt <[email protected]> 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 <[email protected]> > > --- > > 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; >
