Hi Heinrich, On Wed, Oct 3, 2018 at 2:19 AM Heinrich Schuchardt <xypron.g...@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...@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.g...@gmx.de> > > > optional_header: > > .short 0x20b /* PE32+ format */ > > .byte 0x02 /* MajorLinkerVersion */ > > Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
