Hi Heinrich,
On Wed, Oct 3, 2018 at 2:19 AM Heinrich Schuchardt xypron.glpk@gmx.de wrote:
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@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?
You observation is correct. We should use separate files to support 32-bit and 64-bit.
.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.
Sure, will do it in coming patches.
Reviewed-by: Heinrich Schuchardt xypron.glpk@gmx.de
optional_header: .short 0x20b /* PE32+ format */ .byte 0x02 /* MajorLinkerVersion */
Regards, Bin