[PATCH] riscv: Fix efi header for RV32

[Rick Chen]

Hi Atish

> From: U-Boot [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Heinrich Schuchardt
> Sent: Wednesday, October 14, 2020 9:16 AM
> To: Bin Meng; Atish Patra
> Cc: U-Boot Mailing List; Anup Patel; Jagan Teki; Marek Vasut; Simon Goldschmidt; David Abdurachmanov; Tom Rini
> Subject: Re: [PATCH] riscv: Fix efi header for RV32
>
> Am 14. Oktober 2020 03:03:28 MESZ schrieb Bin Meng <bmeng.cn at gmail.com>:
> >On Wed, Oct 14, 2020 at 3:23 AM Atish Patra <atish.patra at wdc.com>
> >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>
> >> ---
> >>  arch/riscv/lib/crt0_riscv_efi.S | 7 ++++++-
> >>  1 file changed, 6 insertions(+), 1 deletion(-)
> >>

Reviewed-by: Rick Chen <rick at andestech.com>

> >> 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
> >
> >nits: the indentation seems to be incorrect
> >
> >>  #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 */
> >
> >nits: PE32(+) ?
> >
> >> +       .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
> >>
>
> The PE32 header has to be 96 bytes long, the PE32+ header 112 bytes long.
>
> See https://docs.microsoft.com/en-us/windows/win32/debug/pe-format
>

Do you have any comments ?

The other headers seem for Windows-Specific Fields. If no other
comments furthermore.
I will pull it into riscv tree later.

Thanks,
Rick

> Best regards
>
> Heinrich
>
>
> >>  extra_header_fields:
> >>         .quad   0                               /* ImageBase */
> >
> >Looks good otherwise
> >Reviewed-by: Bin Meng <bin.meng at windriver.com>