[U-Boot] [PATCH v2 6/6] riscv: efi: Generate Microsoft PE format compliant images

[Heinrich Schuchardt]

On 10/02/2018 04:39 PM, Bin Meng wrote:
> Per Microsoft PE Format documentation [1], PointerToSymbolTable and
> NumberOfSymbols should be zero for an image in the COFF file header.
> Currently the COFF file header is hardcoded on RISC-V and these two
> members are not zero.
> 
> This updates the hardcoded structure to clear these two members, as
> well as setting the flag IMAGE_FILE_LOCAL_SYMS_STRIPPED so that we
> can generate compliant *.efi images.
> 
> [1] https://docs.microsoft.com/zh-cn/windows/desktop/Debug/pe-format
> 
> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
> 
> ---
> 
> Changes in v2:
> - use macros in pe.h for the characteristics field
> 
>  arch/riscv/lib/crt0_riscv_efi.S | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/riscv/lib/crt0_riscv_efi.S b/arch/riscv/lib/crt0_riscv_efi.S
> index 18f61f5..b7b5329 100644
> --- a/arch/riscv/lib/crt0_riscv_efi.S
> +++ b/arch/riscv/lib/crt0_riscv_efi.S
> @@ -41,13 +41,13 @@ coff_header:
>  	.short	2				/* nr_sections */
>  	.long	0				/* TimeDateStamp */
>  	.long	0				/* PointerToSymbolTable */
> -	.long	1				/* NumberOfSymbols */
> +	.long	0				/* NumberOfSymbols */

Looking at the top of the file it is meant both for 32bit and for 64bit
systems:

#if __riscv_xlen == 64
#define SIZE_LONG       8
#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      0x5064
#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      0x5032
#endif

But for 32bit and 64bit we cannot use the same values of characteristics
and we need different optional header formats.

For the ARM and the x86 architecture this has resulted in separate files
crt0*.S - one for 32bit, the other for 64bit.

Shouldn't we go the same way for RISC-V?

>  	.short	section_table - optional_header	/* SizeOfOptionalHeader */
> -	/*
> -	 * Characteristics: IMAGE_FILE_DEBUG_STRIPPED |
> -	 * IMAGE_FILE_EXECUTABLE_IMAGE | IMAGE_FILE_LINE_NUMS_STRIPPED
> -	 */
> -	.short	0x206
> +	/* Characteristics */
> +	.short	(IMAGE_FILE_EXECUTABLE_IMAGE | \
> +		 IMAGE_FILE_LINE_NUMS_STRIPPED | \
> +		 IMAGE_FILE_LOCAL_SYMS_STRIPPED | \
> +		 IMAGE_FILE_DEBUG_STRIPPED)

These values suit 64bit only.

As currently we only have a defconfig for a 64bit system -
ax25-ae350_defconfig - let's merge this patch.

@Bin:
Your "riscv: Add QEMU virt board support" patch series adds a 32bit
board. Please, consider adding a separate 32bit crt0*.S there.

Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>

>  optional_header:
>  	.short	0x20b				/* PE32+ format */
>  	.byte	0x02				/* MajorLinkerVersion */
>