[PATCH v1 3/6] arm: Prepare linker scripts for memory permissions

[Ilias Apalodimas]

Hi Tom,

On Wed, 5 Feb 2025 at 19:33, Tom Rini <trini at konsulko.com> wrote:
>
> On Wed, Feb 05, 2025 at 09:16:47AM +0200, Ilias Apalodimas wrote:
>
> > Upcoming patches are switching the memory mappings to RW, RO, RX
> > after the U-Boot binary and its data are relocated. Add
> > annotations in the linker scripts to and mark text, data, rodata
> > sections and align them to a page boundary.
> >
> > It's worth noting that efi_runtime relocations are left untouched for
> > now. The efi runtime regions can be relocated by the OS when the latter
> > is calling SetVirtualAddressMap. Which means we have to configure the
> > pages as RX for U-Boot but convert them to RWX just before
> > ExitBootServices. It also needs extra code in efi_tuntime relocation
> > code since R_AARCH64_NONE are emitted as well if we page align the
> > section. Keep it out for now and we can fix it in future patches.
> >
> > Acked-by: Jerome Forissier <jerome.forissier at linaro.org>
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > ---
> >  arch/arm/cpu/armv8/u-boot.lds  | 29 +++++++++++++++++------------
> >  include/asm-generic/sections.h |  2 ++
> >  2 files changed, 19 insertions(+), 12 deletions(-)
> >
> > diff --git a/arch/arm/cpu/armv8/u-boot.lds b/arch/arm/cpu/armv8/u-boot.lds
> > index 857f44412e07..35afc3cbe7ec 100644
> > --- a/arch/arm/cpu/armv8/u-boot.lds
> > +++ b/arch/arm/cpu/armv8/u-boot.lds
> > @@ -22,7 +22,7 @@ SECTIONS
> >
> >       . = ALIGN(8);
> >       __image_copy_start = ADDR(.text);
> > -     .text :
> > +     .text ALIGN(4096):
> >       {
> >               CPUDIR/start.o (.text*)
> >       }
>
> Shouldn't this be:
> -       . = ALIGN(8);
> -       __image_copy_start = ADDR(.text);
> -       .text :
> +       .text ALIGN(4096):
>         {
> +               __image_copy_start = ADDR(.text); // Or even just '= .;' ?
>                 CPUDIR/start.o (.text*)

IIRC this will produce the exact same output.
Given Richards's idea, I'll move the sections around a bit and define
rw_start_end, ro_start/end and rx_start/end symbols.
It would make other architectures life easier if they ever want to
replicate this. So with those new symbols, adding ifdefs around the
alignment should be more readable.

>         }
> Or so? Something like that would also make it easier in v2 to have a
> Kconfig about page align / min align for sections in the linker script
> and then ALIGN(CONFIG_FOO) or #ifdef CONFIG_FOO #define MINALIGN 8 #else
> 4096 #endif, or so. Also I guess today we're not going to worry about
> bigger than 4KiB pages?
>
> > @@ -36,9 +36,12 @@ SECTIONS
> >                  __efi_runtime_stop = .;
> >       }
> >
> > -     .text_rest :
> > +     .text_rest ALIGN(4096) :
> >       {
> > +             __text_start = .;
> >               *(.text*)
> > +             . = ALIGN(4096);
> > +             __text_end = .;
> >       }
>
> Here and elsewhere in the patch, does that really need two ALIGN(4096)
> lines? Or am I totally misremembering how these symbols work in a linker
> script?

Yes it does, the first one guarantees the start of the section and the
latter the end. IOW if I commit the second align, there's no guarantee
we'll end up page aliged. Our mmu code assumes that it's operating in
page boundaries.

Thanks
/Ilias


>
> --
> Tom