[v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Atish Patra]

Currently, last stage boot loaders such as U-Boot can accept only
uImage which is an unnecessary additional step in automating boot flows.

Add a PE/COFF compliant image header that boot loaders can parse and
directly load kernel flat Image. The existing booting methods will continue
to work as it is.

Another goal of this header is to support EFI stub for RISC-V in future.
EFI specification needs PE/COFF image header in the beginning of the kernel
image in order to load it as an EFI application. In order to support
EFI stub, code0 should be replaced with "MZ" magic string and res5(at
offset 0x3c) should point to the rest of the PE/COFF header (which will
be added during EFI support).

Tested on both QEMU and HiFive Unleashed using OpenSBI + U-Boot + Linux.

Signed-off-by: Atish Patra <atish.patra@wdc.com>
---
Changes from v1-v2:
1. Added additional reserved elements to make it fully PE compatible.
---
 arch/riscv/include/asm/image.h | 37 ++++++++++++++++++++++++++++++++++
 arch/riscv/kernel/head.S       | 30 +++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)
 create mode 100644 arch/riscv/include/asm/image.h

diff --git a/arch/riscv/include/asm/image.h b/arch/riscv/include/asm/image.h
new file mode 100644
index 000000000000..927333ededee
--- /dev/null
+++ b/arch/riscv/include/asm/image.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef __ASM_IMAGE_H
+#define __ASM_IMAGE_H
+
+#define RISCV_IMAGE_MAGIC	"RISCV"
+
+#ifndef __ASSEMBLY__
+/*
+ * struct riscv_image_header - riscv kernel image header
+ *
+ * @code0:		Executable code
+ * @code1:		Executable code
+ * @text_offset:	Image load offset
+ * @image_size:		Effective Image size
+ * @reserved:		reserved
+ * @reserved:		reserved
+ * @reserved:		reserved
+ * @magic:		Magic number
+ * @reserved:		reserved
+ * @reserved:		reserved (will be used for PE COFF offset)
+ */
+
+struct riscv_image_header {
+	u32 code0;
+	u32 code1;
+	u64 text_offset;
+	u64 image_size;
+	u64 res1;
+	u64 res2;
+	u64 res3;
+	u64 magic;
+	u32 res4;
+	u32 res5;
+};
+#endif /* __ASSEMBLY__ */
+#endif /* __ASM_IMAGE_H */
diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
index fe884cd69abd..12d660d929ba 100644
--- a/arch/riscv/kernel/head.S
+++ b/arch/riscv/kernel/head.S
@@ -19,9 +19,39 @@
 #include <asm/thread_info.h>
 #include <asm/page.h>
 #include <asm/csr.h>
+#include <asm/image.h>
 
 __INIT
 ENTRY(_start)
+	/*
+	 * Image header expected by Linux boot-loaders. The image header data
+	 * structure is described in asm/image.h.
+	 * Do not modify it without modifying the structure and all bootloaders
+	 * that expects this header format!!
+	 */
+	/* jump to start kernel */
+	j _start_kernel
+	/* reserved */
+	.word 0
+	.balign 8
+#if __riscv_xlen == 64
+	/* Image load offset(2MB) from start of RAM */
+	.dword 0x200000
+#else
+	/* Image load offset(4MB) from start of RAM */
+	.dword 0x400000
+#endif
+	/* Effective size of kernel image */
+	.dword _end - _start
+	.dword 0
+	.dword 0
+	.dword 0
+	.asciz RISCV_IMAGE_MAGIC
+	.word 0
+	.word 0
+
+.global _start_kernel
+_start_kernel:
 	/* Mask all interrupts */
 	csrw sie, zero
 
-- 
2.21.0

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Karsten Merker]

On Wed, May 01, 2019 at 12:56:07PM -0700, Atish Patra wrote:

> Currently, last stage boot loaders such as U-Boot can accept only
> uImage which is an unnecessary additional step in automating boot flows.
> 
> Add a PE/COFF compliant image header that boot loaders can parse and
> directly load kernel flat Image. The existing booting methods will continue
> to work as it is.
> 
> Another goal of this header is to support EFI stub for RISC-V in future.
> EFI specification needs PE/COFF image header in the beginning of the kernel
> image in order to load it as an EFI application. In order to support
> EFI stub, code0 should be replaced with "MZ" magic string and res5(at
> offset 0x3c) should point to the rest of the PE/COFF header (which will
> be added during EFI support).
> 
> Tested on both QEMU and HiFive Unleashed using OpenSBI + U-Boot + Linux.
> 
> Signed-off-by: Atish Patra <atish.patra@wdc.com>

Looks good to me.

Reviewed-by: Karsten Merker <merker@debian.org>
Tested-by: Karsten Merker <merker@debian.org> (QEMU+OpenSBI+U-Boot)

Regards,
Karsten
-- 
Ich widerspreche hiermit ausdrücklich der Nutzung sowie der
Weitergabe meiner personenbezogenen Daten für Zwecke der Werbung
sowie der Markt- oder Meinungsforschung.

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Karsten Merker]

On Wed, May 01, 2019 at 12:56:07PM -0700, Atish Patra wrote:

> Currently, last stage boot loaders such as U-Boot can accept only
> uImage which is an unnecessary additional step in automating boot flows.
> 
> Add a PE/COFF compliant image header that boot loaders can parse and
> directly load kernel flat Image. The existing booting methods will continue
> to work as it is.
> 
> Another goal of this header is to support EFI stub for RISC-V in future.
> EFI specification needs PE/COFF image header in the beginning of the kernel
> image in order to load it as an EFI application. In order to support
> EFI stub, code0 should be replaced with "MZ" magic string and res5(at
> offset 0x3c) should point to the rest of the PE/COFF header (which will
> be added during EFI support).
> 
> Tested on both QEMU and HiFive Unleashed using OpenSBI + U-Boot + Linux.

Hello Palmer,

it would be great if this patch could go in with the 5.2 merge
window. Is there anything particular blocking its acceptance?

Regards,
Karsten
-- 
Ich widerspreche hiermit ausdrücklich der Nutzung sowie der
Weitergabe meiner personenbezogenen Daten für Zwecke der Werbung
sowie der Markt- oder Meinungsforschung.

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Paul Walmsley]

On Wed, 1 May 2019, Atish Patra wrote:

> Currently, last stage boot loaders such as U-Boot can accept only
> uImage which is an unnecessary additional step in automating boot flows.
> 
> Add a PE/COFF compliant image header that boot loaders can parse and
> directly load kernel flat Image. The existing booting methods will continue
> to work as it is.
> 
> Another goal of this header is to support EFI stub for RISC-V in future.
> EFI specification needs PE/COFF image header in the beginning of the kernel
> image in order to load it as an EFI application. In order to support
> EFI stub, code0 should be replaced with "MZ" magic string and res5(at
> offset 0x3c) should point to the rest of the PE/COFF header (which will
> be added during EFI support).
> 
> Tested on both QEMU and HiFive Unleashed using OpenSBI + U-Boot + Linux.
> 
> Signed-off-by: Atish Patra <atish.patra@wdc.com>

Seems like we're stuck with this basic format for EFI, etc.  Even though 
we may be stuck with it, I think we should take the opportunity to add the 
possibility to extending this header format by adding fields after the 
basic PE/COFF header ends.

In particular, at the very least, I'd suggest adding a u32 after the 
PE/COFF header ends, to represent a "RISC-V header format version number".  
Then if we add more fields that follow the PE/COFF header -- for example, 
to represent the RISC-V instruction set extensions that are required to 
run this binary -- we can just bump that RISC-V-specific version number 
field to indicate to bootloaders that there's more there.

One other observation - if what's here was copied from some other 
architecture, like ARM, please acknowledge the source in the patch 
description.

thanks

- Paul

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Atish Patra]

On 5/13/19 3:31 PM, Paul Walmsley wrote:
> On Wed, 1 May 2019, Atish Patra wrote:
> 
>> Currently, last stage boot loaders such as U-Boot can accept only
>> uImage which is an unnecessary additional step in automating boot flows.
>>
>> Add a PE/COFF compliant image header that boot loaders can parse and
>> directly load kernel flat Image. The existing booting methods will continue
>> to work as it is.
>>
>> Another goal of this header is to support EFI stub for RISC-V in future.
>> EFI specification needs PE/COFF image header in the beginning of the kernel
>> image in order to load it as an EFI application. In order to support
>> EFI stub, code0 should be replaced with "MZ" magic string and res5(at
>> offset 0x3c) should point to the rest of the PE/COFF header (which will
>> be added during EFI support).
>>
>> Tested on both QEMU and HiFive Unleashed using OpenSBI + U-Boot + Linux.
>>
>> Signed-off-by: Atish Patra <atish.patra@wdc.com>
> 
> Seems like we're stuck with this basic format for EFI, etc.  Even though
> we may be stuck with it, I think we should take the opportunity to add the
> possibility to extending this header format by adding fields after the
> basic PE/COFF header ends.
> 
> In particular, at the very least, I'd suggest adding a u32 after the
> PE/COFF header ends, to represent a "RISC-V header format version number".
> Then if we add more fields that follow the PE/COFF header -- for example,
> to represent the RISC-V instruction set extensions that are required to
> run this binary -- we can just bump that RISC-V-specific version number
> field to indicate to bootloaders that there's more there.
> 
That would be inventing our RISC-V specific header format. However, we 
can always use the one of the reserved fields in proposed header (res4) 
for this purpose.

Do we need to add it now or add it later when we actually need a version 
number. My preference is to add it later based on requirement.

> One other observation - if what's here was copied from some other
> architecture, like ARM, please acknowledge the source in the patch
> description.
> 

Sure. I will update the patch.

Regards,
Atish
> thanks
> 
> - Paul
> 

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Paul Walmsley]

On Mon, 13 May 2019, Atish Patra wrote:

> On 5/13/19 3:31 PM, Paul Walmsley wrote:
> > On Wed, 1 May 2019, Atish Patra wrote:
> > 
> > > Currently, last stage boot loaders such as U-Boot can accept only
> > > uImage which is an unnecessary additional step in automating boot flows.
> > > 
> > > Add a PE/COFF compliant image header that boot loaders can parse and
> > > directly load kernel flat Image. The existing booting methods will
> > > continue
> > > to work as it is.
> > > 
> > > Another goal of this header is to support EFI stub for RISC-V in future.
> > > EFI specification needs PE/COFF image header in the beginning of the
> > > kernel
> > > image in order to load it as an EFI application. In order to support
> > > EFI stub, code0 should be replaced with "MZ" magic string and res5(at
> > > offset 0x3c) should point to the rest of the PE/COFF header (which will
> > > be added during EFI support).
> > > 
> > > Tested on both QEMU and HiFive Unleashed using OpenSBI + U-Boot + Linux.
> > > 
> > > Signed-off-by: Atish Patra <atish.patra@wdc.com>
> > 
> > Seems like we're stuck with this basic format for EFI, etc.  Even though
> > we may be stuck with it, I think we should take the opportunity to add the
> > possibility to extending this header format by adding fields after the
> > basic PE/COFF header ends.
> > 
> > In particular, at the very least, I'd suggest adding a u32 after the
> > PE/COFF header ends, to represent a "RISC-V header format version number".
> > Then if we add more fields that follow the PE/COFF header -- for example,
> > to represent the RISC-V instruction set extensions that are required to
> > run this binary -- we can just bump that RISC-V-specific version number
> > field to indicate to bootloaders that there's more there.
> > 
> That would be inventing our RISC-V specific header format.  However, we 
> can always use the one of the reserved fields in proposed header (res4) 
> for this purpose.

What are the semantics of those reserved fields?

> Do we need to add it now or add it later when we actually need a version
> number. My preference is to add it later based on requirement.

If it isn't added now, how would bootloaders know whether it was there or 
not?


- Paul

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Atish Patra]

On 5/13/19 5:09 PM, Paul Walmsley wrote:
> On Mon, 13 May 2019, Atish Patra wrote:
> 
>> On 5/13/19 3:31 PM, Paul Walmsley wrote:
>>> On Wed, 1 May 2019, Atish Patra wrote:
>>>
>>>> Currently, last stage boot loaders such as U-Boot can accept only
>>>> uImage which is an unnecessary additional step in automating boot flows.
>>>>
>>>> Add a PE/COFF compliant image header that boot loaders can parse and
>>>> directly load kernel flat Image. The existing booting methods will
>>>> continue
>>>> to work as it is.
>>>>
>>>> Another goal of this header is to support EFI stub for RISC-V in future.
>>>> EFI specification needs PE/COFF image header in the beginning of the
>>>> kernel
>>>> image in order to load it as an EFI application. In order to support
>>>> EFI stub, code0 should be replaced with "MZ" magic string and res5(at
>>>> offset 0x3c) should point to the rest of the PE/COFF header (which will
>>>> be added during EFI support).
>>>>
>>>> Tested on both QEMU and HiFive Unleashed using OpenSBI + U-Boot + Linux.
>>>>
>>>> Signed-off-by: Atish Patra <atish.patra@wdc.com>
>>>
>>> Seems like we're stuck with this basic format for EFI, etc.  Even though
>>> we may be stuck with it, I think we should take the opportunity to add the
>>> possibility to extending this header format by adding fields after the
>>> basic PE/COFF header ends.
>>>
>>> In particular, at the very least, I'd suggest adding a u32 after the
>>> PE/COFF header ends, to represent a "RISC-V header format version number".
>>> Then if we add more fields that follow the PE/COFF header -- for example,
>>> to represent the RISC-V instruction set extensions that are required to
>>> run this binary -- we can just bump that RISC-V-specific version number
>>> field to indicate to bootloaders that there's more there.
>>>
>> That would be inventing our RISC-V specific header format.  However, we
>> can always use the one of the reserved fields in proposed header (res4)
>> for this purpose.
> 
> What are the semantics of those reserved fields?
> 

+struct riscv_image_header {
+	u32 code0;
+	u32 code1;
+	u64 text_offset;
+	u64 image_size;
+	u64 res1;
+	u64 res2;
+	u64 res3;
+	u64 magic;
+	u32 res4; ---> We can use this for versioning when required
+	u32 res5; ---> This is reserved for PE/COFF header
+};

>> Do we need to add it now or add it later when we actually need a version
>> number. My preference is to add it later based on requirement.
> 
> If it isn't added now, how would bootloaders know whether it was there or
> not?
> 
> 
Here is the corresponding U-Boot Patch
https://patchwork.ozlabs.org/patch/1096087/

Currently, boot loader doesn't care about versioning. Since we are 
updating a reserved field, offsets will not change. If a boot loader 
want to use the versioning, it should be patched along with the kernel 
patch.

Any other boot loader that doesn't care about the version, it can 
continue to do so without any change.

My idea is to enable the minimum required fields in this patch and keep 
everything else as reserved so that it can be amended in future as required.

Regards,
Atish

> - Paul
> 

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Paul Walmsley]

On Mon, 13 May 2019, Atish Patra wrote:

> On 5/13/19 5:09 PM, Paul Walmsley wrote:
> 
> > What are the semantics of those reserved fields?
> 
> +struct riscv_image_header {
> +	u32 code0;
> +	u32 code1;
> +	u64 text_offset;
> +	u64 image_size;
> +	u64 res1;
> +	u64 res2;
> +	u64 res3;
> +	u64 magic;
> +	u32 res4; ---> We can use this for versioning when required
> +	u32 res5; ---> This is reserved for PE/COFF header
> +};

I saw that in your patch.  The problem is that this doesn't describe what 
other software might expect in those fields.  Can anything at all be 
placed in those reserved fields?

> > > Do we need to add it now or add it later when we actually need a version
> > > number. My preference is to add it later based on requirement.
> > 
> > If it isn't added now, how would bootloaders know whether it was there or
> > not?
> > 
> > 
> Here is the corresponding U-Boot Patch
> https://patchwork.ozlabs.org/patch/1096087/
> 
> Currently, boot loader doesn't care about versioning. Since we are updating a
> reserved field, offsets will not change. If a boot loader want to use the
> versioning, it should be patched along with the kernel patch.
> 
> Any other boot loader that doesn't care about the version, it can continue to
> do so without any change.
> 
> My idea is to enable the minimum required fields in this patch and keep
> everything else as reserved so that it can be amended in future as required.

If those fields really are reserved for implementors to do whatever they 
want with them, then that might be a reasonable approach.  That seems 
unlikely, however, since specification authors usually reserve the right 
to use reserved fields for their own purposes in later versions.


- Paul

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Atish Patra]

On 5/13/19 5:40 PM, Paul Walmsley wrote:
> On Mon, 13 May 2019, Atish Patra wrote:
> 
>> On 5/13/19 5:09 PM, Paul Walmsley wrote:
>>
>>> What are the semantics of those reserved fields?
>>
>> +struct riscv_image_header {
>> +	u32 code0;
>> +	u32 code1;
>> +	u64 text_offset;
>> +	u64 image_size;
>> +	u64 res1;
>> +	u64 res2;
>> +	u64 res3;
>> +	u64 magic;
>> +	u32 res4; ---> We can use this for versioning when required
>> +	u32 res5; ---> This is reserved for PE/COFF header
>> +};
> 
> I saw that in your patch.  The problem is that this doesn't describe what
> other software might expect in those fields.  Can anything at all be
> placed in those reserved fields?
> 

Yes. The reserved fields can be used for anything that boot loaders and 
Linux kernel can agree with each other. If you look at the ARM64, they 
have "Informative flags" in place of res1.

>>>> Do we need to add it now or add it later when we actually need a version
>>>> number. My preference is to add it later based on requirement.
>>>
>>> If it isn't added now, how would bootloaders know whether it was there or
>>> not?
>>>
>>>
>> Here is the corresponding U-Boot Patch
>> https://patchwork.ozlabs.org/patch/1096087/
>>
>> Currently, boot loader doesn't care about versioning. Since we are updating a
>> reserved field, offsets will not change. If a boot loader want to use the
>> versioning, it should be patched along with the kernel patch.
>>
>> Any other boot loader that doesn't care about the version, it can continue to
>> do so without any change.
>>
>> My idea is to enable the minimum required fields in this patch and keep
>> everything else as reserved so that it can be amended in future as required.
> 
> If those fields really are reserved for implementors to do whatever they
> want with them, then that might be a reasonable approach.  That seems
> unlikely, however, since specification authors usually reserve the right
> to use reserved fields for their own purposes in later versions.
> 
Technically, we are just implementing the "DOS" header part of PE/COFF 
format for now. It only mandates a magic string "MZ" at the top and a 
32bit value at offset 0x3c tells us offset of PE/COFF header in image.
Anything in between is implementation specific.

For example, it will be updated to support EFI stub as described in the 
commit text,
"In order to support EFI stub, code0 should be replaced with "MZ" magic 
string and res5(at offset 0x3c) should point to the rest of the PE/COFF 
header (which will be added during EFI support)."

Regards,
Atish
> 
> - Paul
> 

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Paul Walmsley]

+ ARM64 maintainers, Tom, Marek

Hi Atish,

On Mon, 13 May 2019, Atish Patra wrote:

> On 5/13/19 5:40 PM, Paul Walmsley wrote:
> > On Mon, 13 May 2019, Atish Patra wrote:
> > > On 5/13/19 5:09 PM, Paul Walmsley wrote:
> > > 
> > > > What are the semantics of those reserved fields?
> > > 
> > > +struct riscv_image_header {
> > > +	u32 code0;
> > > +	u32 code1;
> > > +	u64 text_offset;
> > > +	u64 image_size;
> > > +	u64 res1;
> > > +	u64 res2;
> > > +	u64 res3;
> > > +	u64 magic;
> > > +	u32 res4; ---> We can use this for versioning when required
> > > +	u32 res5; ---> This is reserved for PE/COFF header
> > > +};
> > 
> > I saw that in your patch.  The problem is that this doesn't describe what
> > other software might expect in those fields.  Can anything at all be
> > placed in those reserved fields?
> 
> Yes. The reserved fields can be used for anything that boot loaders and Linux
> kernel can agree with each other. If you look at the ARM64, they have
> "Informative flags" in place of res1.
> 
> > > > > Do we need to add it now or add it later when we actually need a
> > > > > version
> > > > > number. My preference is to add it later based on requirement.
> > > > 
> > > > If it isn't added now, how would bootloaders know whether it was there
> > > > or
> > > > not?
> > > > 
> > > > 
> > > Here is the corresponding U-Boot Patch
> > > https://patchwork.ozlabs.org/patch/1096087/
> > > 
> > > Currently, boot loader doesn't care about versioning. Since we are
> > > updating a
> > > reserved field, offsets will not change. If a boot loader want to use the
> > > versioning, it should be patched along with the kernel patch.
> > > 
> > > Any other boot loader that doesn't care about the version, it can continue
> > > to
> > > do so without any change.
> > > 
> > > My idea is to enable the minimum required fields in this patch and keep
> > > everything else as reserved so that it can be amended in future as
> > > required.
> > 
> > If those fields really are reserved for implementors to do whatever they
> > want with them, then that might be a reasonable approach.  That seems
> > unlikely, however, since specification authors usually reserve the right
> > to use reserved fields for their own purposes in later versions.
> > 
> Technically, we are just implementing the "DOS" header part of PE/COFF format
> for now. It only mandates a magic string "MZ" at the top and a 32bit value at
> offset 0x3c tells us offset of PE/COFF header in image.
> Anything in between is implementation specific.
> 
> For example, it will be updated to support EFI stub as described in the commit
> text,
> "In order to support EFI stub, code0 should be replaced with "MZ" magic string
> and res5(at offset 0x3c) should point to the rest of the PE/COFF header (which
> will be added during EFI support)."

OK.  I think we should try to share this header format with other 
architectures.  This one after all is copied from ARM64, and some of the 
core fields will be the same across multiple architectures.  That way we 
can try to avoid proliferating different boot header formats for each 
architecture, which should be better for both the kernel and the 
bootloaders.  ARM64 folks, would you be interested in working together on 
this?

Meanwhile, to unblock RISC-V, and to make this header durable for future 
extensions and to match the existing ARM64 usage, I think we should make 
the following technical changes to what you proposed:

1. Reserve all of the existing ARM64 fields in the same way ARM64 does 
   now.  This keeps open the possibility that we can merge this format 
   with the one used with ARM64, and reuse the same bootloader code.  
   Based on our discussions, it sounds like the primary difference between 
   what you're proposing and the ARM64 format involves the flags/res1 
   field.  Let's keep that as a flag field, reuse ARM64's endianness bit 
   as architecture-independent, then define the rest of the flags in that 
   field as architecture-defined.

2. Allocate another set of reserved bits for a format version number.
   Probably 16 bits is sufficient.  This tells bootloaders how to 
   interpret the header fields in future extensions.  The goal is to 
   preserve compatibility across newer and older versions of the header.  
   The existing ARM64 header would be version 0.  This format that 
   incorporates these changes would be version 1.  The thought here is to 
   preserve all of the semantics of existing fields in newer versions 
   (except for any remaining reserved fields), since many people often do 
   not replace their bootloaders.

3. Define a way to point to additional fields outside this existing
   header.  Another 32 bits of previously reserved data can be defined as 
   a file offset to additional fields (defined as 32-bit words from the 
   beginning of the header).  This should make it technically simple to 
   add additional fields in the future.  For example, RISC-V, and probably 
   other architectures, will want to add some way to indicate which ISA 
   extensions are necessary to run the kernel image.  Right now there 
   won't be any fields defined, so we can leave the format undefined for 
   the moment also.  Let's stipulate for version 1 that this field 
   should be fixed at 0, indicating no additional fields.

4. Document all of this, in this patch, in a file such as
   Documentation/riscv/boot-image-header.txt.  If
   we're able to reach agreement with other maintainers, then we
   can move this file out into a common, non-architecture-specific 
   documentation location.


thanks

- Paul

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Paul Walmsley]

On Wed, 1 May 2019, Atish Patra wrote:

> Currently, last stage boot loaders such as U-Boot can accept only
> uImage which is an unnecessary additional step in automating boot flows.
> 
> Add a PE/COFF compliant image header that boot loaders can parse and
> directly load kernel flat Image. The existing booting methods will continue
> to work as it is.

One other thought: as I think you or someone else may have pointed out, 
this isn't the PE/COFF header itself, but rather an ersatz DOS stub 
header, that is apparently quite close to what some EFI bootloaders 
require.  So from that point of view, it's probably best to just write in 
the patch description that the idea is to add something that resembles an 
MS-DOS stub header, and that the motivations are that:

1. it resembles what ARM64 is doing, and there's not much point in 
inventing another boot header format that's completely different

2. it can be easily converted into an MS-DOS header that some EFI 
bootloaders apparently require, by tweaking a few bytes at the beginning


- Paul

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Atish Patra]

On 5/17/19 10:39 AM, Paul Walmsley wrote:
> 
> On Wed, 1 May 2019, Atish Patra wrote:
> 
>> Currently, last stage boot loaders such as U-Boot can accept only
>> uImage which is an unnecessary additional step in automating boot flows.
>>
>> Add a PE/COFF compliant image header that boot loaders can parse and
>> directly load kernel flat Image. The existing booting methods will continue
>> to work as it is.
> 
> One other thought: as I think you or someone else may have pointed out,
> this isn't the PE/COFF header itself, but rather an ersatz DOS stub
> header, that is apparently quite close to what some EFI bootloaders
> require.  So from that point of view, it's probably best to just write in
> the patch description that the idea is to add something that resembles an
> MS-DOS stub header, and that the motivations are that:
> 
> 1. it resembles what ARM64 is doing, and there's not much point in
> inventing another boot header format that's completely different
> 

Yup. I will add this in the next version.

> 2. it can be easily converted into an MS-DOS header that some EFI
> bootloaders apparently require, by tweaking a few bytes at the beginning
> 

I mentioned all the required changes in the proposed header to so that 
EFI bootloaders can load it directly. Probably, I will clarify it more 
to avoid confusion.


> 
> - Paul
> 


-- 
Regards,
Atish

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Atish Patra]

On 5/16/19 9:20 AM, Paul Walmsley wrote:
> + ARM64 maintainers, Tom, Marek
> 
> Hi Atish,
> 
> On Mon, 13 May 2019, Atish Patra wrote:
> 
>> On 5/13/19 5:40 PM, Paul Walmsley wrote:
>>> On Mon, 13 May 2019, Atish Patra wrote:
>>>> On 5/13/19 5:09 PM, Paul Walmsley wrote:
>>>>
>>>>> What are the semantics of those reserved fields?
>>>>
>>>> +struct riscv_image_header {
>>>> +	u32 code0;
>>>> +	u32 code1;
>>>> +	u64 text_offset;
>>>> +	u64 image_size;
>>>> +	u64 res1;
>>>> +	u64 res2;
>>>> +	u64 res3;
>>>> +	u64 magic;
>>>> +	u32 res4; ---> We can use this for versioning when required
>>>> +	u32 res5; ---> This is reserved for PE/COFF header
>>>> +};
>>>
>>> I saw that in your patch.  The problem is that this doesn't describe what
>>> other software might expect in those fields.  Can anything at all be
>>> placed in those reserved fields?
>>
>> Yes. The reserved fields can be used for anything that boot loaders and Linux
>> kernel can agree with each other. If you look at the ARM64, they have
>> "Informative flags" in place of res1.
>>
>>>>>> Do we need to add it now or add it later when we actually need a
>>>>>> version
>>>>>> number. My preference is to add it later based on requirement.
>>>>>
>>>>> If it isn't added now, how would bootloaders know whether it was there
>>>>> or
>>>>> not?
>>>>>
>>>>>
>>>> Here is the corresponding U-Boot Patch
>>>> https://patchwork.ozlabs.org/patch/1096087/
>>>>
>>>> Currently, boot loader doesn't care about versioning. Since we are
>>>> updating a
>>>> reserved field, offsets will not change. If a boot loader want to use the
>>>> versioning, it should be patched along with the kernel patch.
>>>>
>>>> Any other boot loader that doesn't care about the version, it can continue
>>>> to
>>>> do so without any change.
>>>>
>>>> My idea is to enable the minimum required fields in this patch and keep
>>>> everything else as reserved so that it can be amended in future as
>>>> required.
>>>
>>> If those fields really are reserved for implementors to do whatever they
>>> want with them, then that might be a reasonable approach.  That seems
>>> unlikely, however, since specification authors usually reserve the right
>>> to use reserved fields for their own purposes in later versions.
>>>
>> Technically, we are just implementing the "DOS" header part of PE/COFF format
>> for now. It only mandates a magic string "MZ" at the top and a 32bit value at
>> offset 0x3c tells us offset of PE/COFF header in image.
>> Anything in between is implementation specific.
>>
>> For example, it will be updated to support EFI stub as described in the commit
>> text,
>> "In order to support EFI stub, code0 should be replaced with "MZ" magic string
>> and res5(at offset 0x3c) should point to the rest of the PE/COFF header (which
>> will be added during EFI support)."
> 
> OK.  I think we should try to share this header format with other
> architectures.  This one after all is copied from ARM64, and some of the
> core fields will be the same across multiple architectures.  That way we
> can try to avoid proliferating different boot header formats for each
> architecture, which should be better for both the kernel and the
> bootloaders.  ARM64 folks, would you be interested in working together on
> this?
> 
> Meanwhile, to unblock RISC-V, and to make this header durable for future
> extensions and to match the existing ARM64 usage, I think we should make
> the following technical changes to what you proposed:
> 
> 1. Reserve all of the existing ARM64 fields in the same way ARM64 does
>     now.  This keeps open the possibility that we can merge this format
>     with the one used with ARM64, and reuse the same bootloader code.
>     Based on our discussions, it sounds like the primary difference between
>     what you're proposing and the ARM64 format involves the flags/res1
>     field.  Let's keep that as a flag field, reuse ARM64's endianness bit
>     as architecture-independent, then define the rest of the flags in that
>     field as architecture-defined.
> 
> 2. Allocate another set of reserved bits for a format version number.
>     Probably 16 bits is sufficient.  This tells bootloaders how to
>     interpret the header fields in future extensions.  The goal is to
>     preserve compatibility across newer and older versions of the header.
>     The existing ARM64 header would be version 0.  This format that
>     incorporates these changes would be version 1.  The thought here is to
>     preserve all of the semantics of existing fields in newer versions
>     (except for any remaining reserved fields), since many people often do
>     not replace their bootloaders.
> 
> 3. Define a way to point to additional fields outside this existing
>     header.  Another 32 bits of previously reserved data can be defined as
>     a file offset to additional fields (defined as 32-bit words from the
>     beginning of the header).  This should make it technically simple to
>     add additional fields in the future.  For example, RISC-V, and probably
>     other architectures, will want to add some way to indicate which ISA
>     extensions are necessary to run the kernel image.  Right now there
>     won't be any fields defined, so we can leave the format undefined for
>     the moment also.  Let's stipulate for version 1 that this field
>     should be fixed at 0, indicating no additional fields.
> 
> 4. Document all of this, in this patch, in a file such as
>     Documentation/riscv/boot-image-header.txt.  If
>     we're able to reach agreement with other maintainers, then we
>     can move this file out into a common, non-architecture-specific
>     documentation location.
> 

I have sent out a v3 incorporating most of your suggestions. If ARM 
maintainers agree, we can move both the headers to a common place.

Just FYI: Marek also suggested to add unified support Image.gz for both 
U-Boot & RISC-V in U-Boot. I am working on that as well.

> 
> thanks
> 
> - Paul
> 


-- 
Regards,
Atish

Re: [v2 PATCH] RISC-V: Add a PE/COFF compliant Image header.

[Paul Walmsley]

On Thu, 23 May 2019, Atish Patra wrote:

> I have sent out a v3 incorporating most of your suggestions. If ARM
> maintainers agree, we can move both the headers to a common place.
> 
> Just FYI: Marek also suggested to add unified support Image.gz for both U-Boot
> & RISC-V in U-Boot. I am working on that as well.

Great - thanks very much.

- Paul