[PATCH] efi_loader: access __efi_runtime_start/stop without &

[Ilias Apalodimas]

On Fri, 5 Apr 2024 at 10:12, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> On 4/4/24 08:35, Ilias Apalodimas wrote:
> > A symbol defined in a linker script (e.g. __efi_runtime_start = .;) is
> > only a symbol, not a variable and should not be dereferenced.
> > The common practice is either define it as
> > extern uint32_t __efi_runtime_start or
> > extern char __efi_runtime_start[] and access it as
> > &__efi_runtime_start or __efi_runtime_start respectively.
> >
> > So let's access it properly since we define it as an array
>
> Thanks for investigating this.
>
> Beyond this patch I guess we should eliminate these duplicate defintions:
>
> include/asm-generic/sections.h:38:extern char __efi_runtime_start[],
> __efi_runtime_stop[];
> include/efi_loader.h:348:extern char __efi_runtime_start[],
> __efi_runtime_stop[];

Yes, you've already sent a patch on this, thanks

>
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > ---
> >   lib/efi_loader/efi_memory.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c
> > index edfad2d95a1d..98f104390c8d 100644
> > --- a/lib/efi_loader/efi_memory.c
> > +++ b/lib/efi_loader/efi_memory.c
> > @@ -933,8 +933,8 @@ static void add_u_boot_and_runtime(void)
> >        * Add Runtime Services. We mark surrounding boottime code as runtime as
> >        * well to fulfill the runtime alignment constraints but avoid padding.
> >        */
> > -     runtime_start = (ulong)&__efi_runtime_start & ~runtime_mask;
> > -     runtime_end = (ulong)&__efi_runtime_stop;
> > +     runtime_start = (ulong)__efi_runtime_start & ~runtime_mask;
>
> Using (uintptr_t) would make it clearer that we are converting from a
> pointer to an integer type.
>

Sure, I would prefer this to be on a followup with a commit message of
its own, but I am fine with amending it, if you merge this

Thanks
/Ilias
> Best regards
>
> Heinrich
>
> > +     runtime_end = (ulong)__efi_runtime_stop;
> >       runtime_end = (runtime_end + runtime_mask) & ~runtime_mask;
> >       runtime_pages = (runtime_end - runtime_start) >> EFI_PAGE_SHIFT;
> >       efi_add_memory_map_pg(runtime_start, runtime_pages,
>