[PATCH 4/4] efi_loader: use include in ARM EFI linker script

[Ilias Apalodimas]

On Wed, 15 Jan 2025 at 10:53, Heinrich Schuchardt
<heinrich.schuchardt at canonical.com> wrote:
>
> On 15.01.25 09:06, Ilias Apalodimas wrote:
> > Hi Heinrich
> >
> >
> > On Tue, 14 Jan 2025 at 12:30, Heinrich Schuchardt
> > <heinrich.schuchardt at canonical.com> wrote:
> >>
> >> Use the same include as arm64 for the linker script.
> >>
> >> Adjust the 32-bit ARM PE-COFF header accordingly and harmonize it with the
> >> 64-bit ARM header.
> >>
> >> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt at canonical.com>
> >> ---
> >>   arch/arm/lib/crt0_arm_efi.S  | 37 ++++++++++---------
> >>   arch/arm/lib/elf_arm_efi.lds | 71 +-----------------------------------
> >>   2 files changed, 20 insertions(+), 88 deletions(-)
> >>
> >> diff --git a/arch/arm/lib/crt0_arm_efi.S b/arch/arm/lib/crt0_arm_efi.S
> >> index b5dfd4e3819..590fcf515da 100644
> >> --- a/arch/arm/lib/crt0_arm_efi.S
> >> +++ b/arch/arm/lib/crt0_arm_efi.S
> >> @@ -14,11 +14,11 @@
> >>          /*
> >>           * Magic "MZ" signature for PE/COFF
> >>           */
> >> -       .globl  image_base
> >> -image_base:
> >> +       .globl  ImageBase
> >> +ImageBase:
> >>          .short  IMAGE_DOS_SIGNATURE             /* 'MZ' */
> >>          .skip   58                              /* 'MZ' + pad + offset == 64 */
> >> -       .long   pe_header - image_base          /* Offset to the PE header */
> >> +       .long   pe_header - ImageBase           /* Offset to the PE header */
> >>   pe_header:
> >>          .long   IMAGE_NT_SIGNATURE              /* 'PE' */
> >>   coff_header:
> >> @@ -38,16 +38,16 @@ optional_header:
> >>          .short  IMAGE_NT_OPTIONAL_HDR32_MAGIC   /* PE32 format */
> >>          .byte   0x02                            /* MajorLinkerVersion */
> >>          .byte   0x14                            /* MinorLinkerVersion */
> >> -       .long   _edata - _start                 /* SizeOfCode */
> >> +       .long   _etext - _start                 /* SizeOfCode */
> >
> > Was that an error all along? Or the boundaries changed by using the
> > include file?
>
> According to https://learn.microsoft.com/en-us/windows/win32/debug/pe-format
>
> SizeOfCode:
> "The size of the code (text) section, or the sum of all code sections if
> there are multiple sections."
>
> We only have one code section.
>
> Looking at EDK2's Shell.efi SizeOfCode equals the VirtualSize of the
> .text section. It does not include the .data section.
>

Hmm that's interesting. gnu-efi includes .data AFAICT.
Looking at the pe/coff spec there's SizeOfCode and
SizeOfInitializedData (for .data) and SizeOfUninitializedData (for
.bss).
But in both gnu-efi and U-Boot we define those two as 0 though. So I
think we should either keep SizeOfData as is, or define it as you
propose and also define SizeOfInitializedData and
SizeOfUninitializedData correctly -- and I don't know if the latter
will have any implication on loaders.

Cheers
/Ilias
> Best regards
>
> Heinrich
>
> >
> >>          .long   0                               /* SizeOfInitializedData */
> >>          .long   0                               /* SizeOfUninitializedData */
> >> -       .long   _start - image_base             /* AddressOfEntryPoint */
> >> -       .long   _start - image_base             /* BaseOfCode */
> >> +       .long   _start - ImageBase              /* AddressOfEntryPoint */
> >> +       .long   _start - ImageBase              /* BaseOfCode */
> >>          .long   0                               /* BaseOfData */
> > [...]
> >
> > Other than that it looks ok
> >
> > Thanks for cleaning this up!
> > /Ilias
>