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

[Tom Rini]

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.

> 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.

> > 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.

And I had mis-read the context. Yes, we need to make sure the end is
aligned so that we don't have incorrect permissions for the next
section. And with re-organization of the areas we'll have less extra
padding too.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20250205/fb740f93/attachment.sig>