> -----Original Message----- > From: Troy Benjegerdes <troy.benjeger...@sifive.com> > Sent: Tuesday, May 28, 2019 5:11 AM > To: Karsten Merker <mer...@debian.org> > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org>; Albert Ou > <a...@eecs.berkeley.edu>; Jonathan Corbet <cor...@lwn.net>; Anup Patel > <anup.pa...@wdc.com>; Zong Li <z...@andestech.com>; Atish Patra > <atish.pa...@wdc.com>; Nick Kossifidis <m...@ics.forth.gr>; Palmer Dabbelt > <pal...@sifive.com>; paul.walms...@sifive.com; linux- > ri...@lists.infradead.org; marek.va...@gmail.com > Subject: Re: [v4 PATCH] RISC-V: Add an Image header that boot loader can > parse. > > > > > On May 27, 2019, at 5:16 PM, Karsten Merker <mer...@debian.org> > wrote: > > > > On Mon, May 27, 2019 at 04:34:57PM +0200, Ard Biesheuvel wrote: > >> On Fri, 24 May 2019 at 06:18, Atish Patra <atish.pa...@wdc.com> wrote: > >>> Currently, the last stage boot loaders such as U-Boot can accept > >>> only uImage which is an unnecessary additional step in automating > >>> boot process. > >>> > >>> Add an image header that boot loader understands and boot Linux from > >>> flat Image directly. > >>> > >>> This header is based on ARM64 boot image header and provides an > >>> opportunity to combine both ARM64 & RISC-V image headers in future. > >>> > >>> Also make sure that PE/COFF header can co-exist in the same image so > >>> that EFI stub can be supported 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 res4(at offset 0x3c) should point to the rest of the PE/COFF > >>> header (which will be added during EFI support). > > [...] > >>> 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.pa...@wdc.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 */ > >> > >> Apologies for not mentioning this in my previous reply, but given > >> that you already know that you will need to put the magic string MZ > >> at offset 0x0, it makes more sense to not put any code there at all, > >> but educate the bootloader that the first executable instruction is > >> at offset 0x20, and put the spare fields right after it in case you > >> ever need more than 2 slots. (On arm64, we were lucky to be able to > >> find an opcode that happened to contain the MZ bit pattern and act > >> almost like a NOP, but it seems silly to rely on that for RISC-V as > >> well) > >> > >> So something like > >> > >> u16 pe_res1; /* MZ for EFI bootable images, don't care otherwise */ > >> u8 magic[6]; /* "RISCV\0" > >> > >> u64 text_offset; /* Image load offset, little endian */ > >> u64 image_size; /* Effective Image size, little endian */ > >> u64 flags; /* kernel flags, little endian */ > >> > >> u32 code0; /* Executable code */ > >> u32 code1; /* Executable code */ > >> > >> u64 reserved[2]; /* reserved for future use */ > >> > >> u32 version; /* Version of this header */ > >> u32 pe_res2; /* Reserved for PE COFF offset */ > > > > Hello, > > > > wouldn't that immediately break existing systems (including qemu when > > loading kernels with the "-kernel" option) that rely on the fact that > > the kernel entry point is always at the kernel load address? The > > ARM64 header and Atish's original RISC-V proposal based on the ARM64 > > header keep the property that jumping to the kernel load address > > always works, regardless of what the particular header looks like and > > which potential future extensions it includes, but the proposed change > > above wouldn't do that. > > > > Although I agree that having to integrate the "MZ" string as an > > instruction isn't particularly nice, I don't think that this is a > > sufficient justification for breaking compatibility with prior kernel > > releases and/or existing boot firmware. On RISC-V, the "MZ" string is > > a compressed load immediate to x20/s4, i.e. an instruction that should > > be "harmless" as far as the kernel boot flow is concerned as the > > x20/s4 register AFAIK doesn't contain any information that the kernel > > would use. > > > > Regards, > > Karsten > > > > Yes, that would break existing systems. Besides, the qemu -kernel option > uses the vmlinux elf file, and I think a better solution is make ‘loadelf’ > work, > and include a second method for EFI. > > (unfortunately, I had to drop some lists as I’m having trouble sending to > them via gmail, so the CC list on my response has been limited)
Nopes, it works perfectly fine on QEMU RISC-V. Just like ARM64, we are lucky for RISC-V as well. The "MZ" string is a harmless load instruction in RISC-V so we don't need any changes in QEMU. We should have "MZ" string in Image header only when Linux RISC-V kernel has EFI support enabled (just like Linux ARM64 kernel) so that bootloader trying to boot Linux RISC-V kernel as EFI application will certainly fail when EFI support is disabled/not-available in Linux RISC-V kernel. Regards, Anup