[PATCH] riscv: Fix efi header for RV32

[Atish Patra]

On Wed, Nov 11, 2020 at 12:26 AM Leo Liang <ycliang at andestech.com> wrote:
>
> Hi Atish and Heinrich,
>
> On Tue, Oct 13, 2020 at 12:23:31PM -0700, Atish Patra wrote:
> > RV32 should use PE32 format instead of PE32+ as the efi header format.
> > This requires following changes
> > 1. A different header magic value
> > 2. An additional parameter known as BaseOfData. Currently, it is set to
> >    zero in absence of any usage.
> >
> > Signed-off-by: Atish Patra <atish.patra at wdc.com>
> > Reviewed-by: Bin Meng <bin.meng at windriver.com>
> > Reviewed-by: Rick Chen <rick at andestech.com>
> > ---
> >  arch/riscv/lib/crt0_riscv_efi.S | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/lib/crt0_riscv_efi.S b/arch/riscv/lib/crt0_riscv_efi.S
> > index 87fe1e56f906..4aaa49ad0777 100644
> > --- a/arch/riscv/lib/crt0_riscv_efi.S
> > +++ b/arch/riscv/lib/crt0_riscv_efi.S
> > @@ -15,11 +15,13 @@
> >  #define SAVE_LONG(reg, idx)  sd      reg, (idx*SIZE_LONG)(sp)
> >  #define LOAD_LONG(reg, idx)  ld      reg, (idx*SIZE_LONG)(sp)
> >  #define PE_MACHINE   IMAGE_FILE_MACHINE_RISCV64
> > +#define PE_MAGIC        IMAGE_NT_OPTIONAL_HDR64_MAGIC
> >  #else
> >  #define SIZE_LONG    4
> >  #define SAVE_LONG(reg, idx)  sw      reg, (idx*SIZE_LONG)(sp)
> >  #define LOAD_LONG(reg, idx)  lw      reg, (idx*SIZE_LONG)(sp)
> >  #define PE_MACHINE   IMAGE_FILE_MACHINE_RISCV32
> > +#define PE_MAGIC        IMAGE_NT_OPTIONAL_HDR32_MAGIC
> >  #endif
> >
> >
> > @@ -48,7 +50,7 @@ coff_header:
> >                IMAGE_FILE_LOCAL_SYMS_STRIPPED | \
> >                IMAGE_FILE_DEBUG_STRIPPED)
> >  optional_header:
> > -     .short  IMAGE_NT_OPTIONAL_HDR64_MAGIC   /* PE32+ format */
> > +     .short  PE_MAGIC                        /* PE32+ format */
> >       .byte   0x02                            /* MajorLinkerVersion */
> >       .byte   0x14                            /* MinorLinkerVersion */
> >       .long   _edata - _start                 /* SizeOfCode */
> > @@ -56,6 +58,9 @@ optional_header:
> >       .long   0                               /* SizeOfUninitializedData */
> >       .long   _start - ImageBase              /* AddressOfEntryPoint */
> >       .long   _start - ImageBase              /* BaseOfCode */
> > +#if __riscv_xlen == 32
> > +     .long   0                               /* BaseOfData */
> > +#endif
> >
> >  extra_header_fields:
> >       .quad   0                               /* ImageBase */
>
> diff --git a/arch/riscv/lib/crt0_riscv_efi.S b/arch/riscv/lib/crt0_riscv_efi.S
> index 4aaa49ad07..f47c4a6d82 100644
> --- a/arch/riscv/lib/crt0_riscv_efi.S
> +++ b/arch/riscv/lib/crt0_riscv_efi.S
> @@ -63,7 +63,11 @@ optional_header:
>  #endif
>
>  extra_header_fields:
> +#if __riscv_xlen == 32
> +       .long   0                               /* ImageBase */
> +#else if
>         .quad   0                               /* ImageBase */
> +#endif
>         .long   0x20                            /* SectionAlignment */
>         .long   0x8                             /* FileAlignment */
>         .short  0                               /* MajorOperatingSystemVersion */
> @@ -83,10 +87,17 @@ extra_header_fields:
>         .long   0                               /* CheckSum */
>         .short  IMAGE_SUBSYSTEM_EFI_APPLICATION /* Subsystem */
>         .short  0                               /* DllCharacteristics */
> +#if __riscv_xlen == 32
> +       .long   0                               /* SizeOfStackReserve */
> +       .long   0                               /* SizeOfStackCommit */
> +       .long   0                               /* SizeOfHeapReserve */
> +       .long   0                               /* SizeOfHeapCommit */
> +#else if
>         .quad   0                               /* SizeOfStackReserve */
>         .quad   0                               /* SizeOfStackCommit */
>         .quad   0                               /* SizeOfHeapReserve */
>         .quad   0                               /* SizeOfHeapCommit */
> +#endif
>         .long   0                               /* LoaderFlags */
>         .long   0x6                             /* NumberOfRvaAndSizes */
>

Sorry for not responding to this thread earlier. It went to a
different filter due to a filtering mistake.

> Based on Atish's patch,
> would the above modification fit more closely to the specification that Heinrich provided ?
>

The above changes look good to me and matches the PE32 spec. Thanks for the fix.

> Best regards,
> Leo


-- 
Regards,
Atish