Message-ID: <439bf2f0-785b-ac52-8116-63e2e82bc179@wdc.com>
Date:   Thu, 23 May 2019 17:16:40 -0700
From:   Atish Patra <atish.patra@....com>
To:     Ard Biesheuvel <ard.biesheuvel@...aro.org>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Mark Rutland <mark.rutland@....com>,
        Jonathan Corbet <corbet@....net>,
        Albert Ou <aou@...s.berkeley.edu>,
        Linux Doc Mailing List <linux-doc@...r.kernel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Anup Patel <Anup.Patel@....com>,
        Will Deacon <will.deacon@....com>,
        Zong Li <zong@...estech.com>,
        Palmer Dabbelt <palmer@...ive.com>,
        Tom Rini <trini@...sulko.com>,
        "paul.walmsley@...ive.com" <paul.walmsley@...ive.com>,
        Nick Kossifidis <mick@....forth.gr>,
        "linux-riscv@...ts.infradead.org" <linux-riscv@...ts.infradead.org>,
        "marek.vasut@...il.com" <marek.vasut@...il.com>,
        linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>
Subject: Re: [v3 PATCH] RISC-V: Add a PE/COFF compliant Image header.

On 5/23/19 2:09 PM, Ard Biesheuvel wrote:
> On Thu, 23 May 2019 at 19:35, Atish Patra <atish.patra@....com> 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.
>>
> 
> This statement does not make sense. This patch does not implement a
> single one of the various elements that make up a valid PE/COFF
> header.
> 

Probably "compliant" is not the correct word. I meant to say that
PE/COFF header can be implemented in future with this header.


> The arm64 Image header has been designed in a way so that it can
> co-exist with a PE/COFF header in the same image, and this is what

Correct. "co-exist" is much better than "compliant"

Sorry for the choosing ambiguous words. I will update the commit text in 
next version.

> this patch duplicates. The arm64 Image header has nothing to do with
> PE/COFF.
> 
> A PE/COFF executable header consists of
> - the letters MZ at offset 0x0 (the MS-DOS header)
> - the offset to the PE header at offset 0x3c
> - the characters PE\0\0 at the offset mentioned above, followed by the
> standard COFF header fields
> - a PE32 or PE32+ (depending on the bitness) optional* header,
> followed by a set of section headers.
> 
> 
> 
> 
>> 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).
>>
>> This patch is based on ARM64 boot image header and provides an opprtunity
>> to combine both ARM64 & RISC-V image headers.
>>
>> Tested on both QEMU and HiFive Unleashed using OpenSBI + U-Boot + Linux.
>>
>> Signed-off-by: Atish Patra <atish.patra@....com>
>>
>> ---
>> I have not sent out corresponding U-Boot patch as all the changes are
>> compatible with current u-boot support. Once, the kernel header format
>> is agreed upon, I will update the U-Boot patch.
>>
>> Changes from v2->v3
>> 1. Modified reserved fields to define a header version.
>> 2. Added header documentation.
>>
>> Changes from v1-v2:
>> 1. Added additional reserved elements to make it fully PE compatible.
>> ---
>>   Documentation/riscv/boot-image-header.txt | 50 ++++++++++++++++++
>>   arch/riscv/include/asm/image.h            | 64 +++++++++++++++++++++++
>>   arch/riscv/kernel/head.S                  | 32 ++++++++++++
>>   3 files changed, 146 insertions(+)
>>   create mode 100644 Documentation/riscv/boot-image-header.txt
>>   create mode 100644 arch/riscv/include/asm/image.h
>>
>> diff --git a/Documentation/riscv/boot-image-header.txt b/Documentation/riscv/boot-image-header.txt
>> new file mode 100644
>> index 000000000000..68abc2353cec
>> --- /dev/null
>> +++ b/Documentation/riscv/boot-image-header.txt
>> @@ -0,0 +1,50 @@
>> +                               Boot image header in RISC-V Linux
>> +                       =============================================
>> +
>> +Author: Atish Patra <atish.patra@....com>
>> +Date  : 20 May 2019
>> +
>> +This document only describes the boot image header details for RISC-V Linux.
>> +The complete booting guide will be available at Documentation/riscv/booting.txt.
>> +
>> +The following 64-byte header is present in decompressed Linux kernel image.
>> +
>> +       u32 code0;                /* Executable code */
>> +       u32 code1;                /* Executable code */
>> +       u64 text_offset;          /* Image load offset, little endian */
>> +       u64 image_size;           /* Effective Image size, little endian */
>> +       u64 flags;                /* kernel flags, little endian */
>> +       u32 version;              /* Version of this header */
>> +       u32 res1  = 0;            /* Reserved */
>> +       u64 res2  = 0;            /* Reserved */
>> +       u64 magic = 0x5643534952; /* Magic number, little endian, "RISCV" */
>> +       u32 res3;                 /* Reserved for additional RISC-V specific header */
>> +       u32 res4;                 /* Reserved for PE COFF offset */
>> +
>> +This header format is compliant with PE/COFF header and largely inspired from
>> +ARM64 header. Thus, both ARM64 & RISC-V header can be combined into one common
>> +header in future.
>> +
>> +Notes:
>> +- This header can also be reused 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.
>> +
>> +- version field indicate header version number.
>> +       Bits 0:15  - Minor version
>> +       Bits 16:31 - Major version
>> +
>> +  This preserves compatibility across newer and older version of the header.
>> +  The current version is defined as 0.1.
>> +
>> +- res3 is reserved for offset to any other additional fields. This makes the
>> +  header extendible in future. One example would be to accommodate ISA
>> +  extension for RISC-V in future. For current version, it is set to be zero.
>> +
>> +- In current header, the flag field has only one field.
>> +       Bit 0: Kernel endianness. 1 if BE, 0 if LE.
>> +
>> +- Image size is mandatory for boot loader to load kernel image. Booting will
>> +  fail otherwise.
>> diff --git a/arch/riscv/include/asm/image.h b/arch/riscv/include/asm/image.h
>> new file mode 100644
>> index 000000000000..61c9f20d2f19
>> --- /dev/null
>> +++ b/arch/riscv/include/asm/image.h
>> @@ -0,0 +1,64 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +
>> +#ifndef __ASM_IMAGE_H
>> +#define __ASM_IMAGE_H
>> +
>> +#define RISCV_IMAGE_MAGIC      "RISCV"
>> +
>> +
>> +#define RISCV_IMAGE_FLAG_BE_SHIFT      0
>> +#define RISCV_IMAGE_FLAG_BE_MASK       0x1
>> +
>> +#define RISCV_IMAGE_FLAG_LE            0
>> +#define RISCV_IMAGE_FLAG_BE            1
>> +
>> +
>> +#ifdef CONFIG_CPU_BIG_ENDIAN
>> +#define __HEAD_FLAG_BE         RISCV_IMAGE_FLAG_BE
>> +#else
>> +#define __HEAD_FLAG_BE         RISCV_IMAGE_FLAG_LE
>> +#endif
>> +
>> +#define __HEAD_FLAG(field)     (__HEAD_FLAG_##field << \
>> +                               RISCV_IMAGE_FLAG_##field##_SHIFT)
>> +
>> +#define __HEAD_FLAGS           (__HEAD_FLAG(BE))
>> +
>> +#define RISCV_HEADER_VERSION_MAJOR 0
>> +#define RISCV_HEADER_VERSION_MINOR 1
>> +
>> +#define RISCV_HEADER_VERSION (RISCV_HEADER_VERSION_MAJOR << 16 | \
>> +                             RISCV_HEADER_VERSION_MINOR)
>> +
>> +#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
>> + * @flags:             kernel flags
>> + * @version:           version
>> + * @reserved:          reserved
>> + * @reserved:          reserved
>> + * @magic:             Magic number
>> + * @reserved:          reserved (will be used for additional RISC-V specific header)
>> + * @reserved:          reserved (will be used for PE COFF offset)
>> + */
>> +
>> +struct riscv_image_header {
>> +       u32 code0;
>> +       u32 code1;
>> +       u64 text_offset;
>> +       u64 image_size;
>> +       u64 flags;
>> +       u32 version;
>> +       u32 res1;
>> +       u64 res2;
>> +       u64 magic;
>> +       u32 res3;
>> +       u32 res4;
>> +};
>> +#endif /* __ASSEMBLY__ */
>> +#endif /* __ASM_IMAGE_H */
>> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
>> index 370c66ce187a..577893bb150d 100644
>> --- a/arch/riscv/kernel/head.S
>> +++ b/arch/riscv/kernel/head.S
>> @@ -19,9 +19,41 @@
>>   #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 __HEAD_FLAGS
>> +       .word RISCV_HEADER_VERSION
>> +       .word 0
>> +       .dword 0
>> +       .asciz RISCV_IMAGE_MAGIC
>> +       .word 0
>> +       .balign 4
>> +       .word 0
>> +
>> +.global _start_kernel
>> +_start_kernel:
>>          /* Mask all interrupts */
>>          csrw CSR_SIE, zero
>>          csrw CSR_SIP, zero
>> --
>> 2.21.0
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@...ts.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 


-- 
Regards,
Atish

Powered by blists - more mailing lists