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 <[email protected]> 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 <[email protected]> > > > Signed-off-by: Ilias Apalodimas <[email protected]> > > > --- > > > 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
signature.asc
Description: PGP signature
