[PATCH] riscv: Fix efi header for RV32
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@wdc.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 */
On Wed, Oct 14, 2020 at 3:23 AM Atish Patra atish.patra@wdc.com wrote:
RV32 should use PE32 format instead of PE32+ as the efi header format. This requires following changes
- A different header magic value
- An additional parameter known as BaseOfData. Currently, it is set to zero in absence of any usage.
Signed-off-by: Atish Patra atish.patra@wdc.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
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
extra_header_fields: .quad 0 /* ImageBase */
Looks good otherwise Reviewed-by: Bin Meng bin.meng@windriver.com
Am 14. Oktober 2020 03:03:28 MESZ schrieb Bin Meng bmeng.cn@gmail.com:
On Wed, Oct 14, 2020 at 3:23 AM Atish Patra atish.patra@wdc.com wrote:
RV32 should use PE32 format instead of PE32+ as the efi header
format.
This requires following changes
- A different header magic value
- An additional parameter known as BaseOfData. Currently, it is set
to
zero in absence of any usage.
Signed-off-by: Atish Patra atish.patra@wdc.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
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
Best regards
Heinrich
extra_header_fields: .quad 0 /* ImageBase */
Looks good otherwise Reviewed-by: Bin Meng bin.meng@windriver.com
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
- A different header magic value
- An additional parameter known as BaseOfData. Currently, it is set to zero in absence of any usage.
Signed-off-by: Atish Patra atish.patra@wdc.com Reviewed-by: Bin Meng bin.meng@windriver.com Reviewed-by: Rick Chen rick@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 */
Based on Atish's patch, would the above modification fit more closely to the specification that Heinrich provided ?
Best regards, Leo
On Wed, Nov 11, 2020 at 04:25:53PM +0800, Leo Liang 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
- A different header magic value
- An additional parameter known as BaseOfData. Currently, it is set to zero in absence of any usage.
Signed-off-by: Atish Patra atish.patra@wdc.com Reviewed-by: Bin Meng bin.meng@windriver.com Reviewed-by: Rick Chen rick@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 */
Based on Atish's patch, would the above modification fit more closely to the specification that Heinrich provided ?
Best regards, Leo
Sending a new patch seems more reasonable. I will send a new patch later based on this patch.
Best regards, Leo
On Wed, Nov 11, 2020 at 12:26 AM Leo Liang ycliang@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
- A different header magic value
- An additional parameter known as BaseOfData. Currently, it is set to zero in absence of any usage.
Signed-off-by: Atish Patra atish.patra@wdc.com Reviewed-by: Bin Meng bin.meng@windriver.com Reviewed-by: Rick Chen rick@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
participants (5)
-
Atish Patra -
Atish Patra -
Bin Meng -
Heinrich Schuchardt -
Leo Liang