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

[Ilias Apalodimas]

On Wed, 5 Feb 2025 at 21:25, Tom Rini <trini at konsulko.com> wrote:
>
> On Wed, Feb 05, 2025 at 09:18:50PM +0200, Ilias Apalodimas wrote:
> > 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.
>
> OK, so we don't end up with 8192 worth of alignment here.

We do, but that's because I added an ALIGN(4096) in .text. The change
above won't change that.

But looking at it again, I don't have to do that now. I can remove the
alignment from .text since some efi_runtime services are between that
and .text_start.
Since my current patch ignores efi_runtime, I can skip the .text
alignment, save ~4k and fix the rest when I deal with EFI.

>
> > 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.
>
> Yes, re-organizing things will help too, I agree.
>
[...]

Regards
/Ilias