LKML Archive on lore.kernel.org
 help / color / Atom feed
From: Atish Patra <Atish.Patra@wdc.com>
To: "paul.walmsley@sifive.com" <paul.walmsley@sifive.com>,
	"linux-riscv@lists.infradead.org"
	<linux-riscv@lists.infradead.org>
Cc: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"palmer@sifive.com" <palmer@sifive.com>,
	"merker@debian.org" <merker@debian.org>
Subject: Re: [PATCH] riscv: modify the Image header to improve compatibility with the ARM64 header
Date: Sat, 14 Sep 2019 04:03:07 +0000
Message-ID: <2e697e9c7966cf1a6cac415b6a247a8ba9f4de29.camel@wdc.com> (raw)
In-Reply-To: <alpine.DEB.2.21.9999.1909132005200.24255@viisi.sifive.com>

On Fri, 2019-09-13 at 20:08 -0700, Paul Walmsley wrote:
> Part of the intention during the definition of the RISC-V kernel
> image
> header was to lay the groundwork for a future merge with the ARM64
> image header.  One error during my original review was not noticing
> that the RISC-V header's "magic" field was at a different size and
> position than the ARM64's "magic" field.  If the existing ARM64 Image
> header parsing code were to attempt to parse an existing RISC-V
> kernel
> image header format, it would see a magic number 0.  This is
> undesirable, since it's our intention to align as closely as possible
> with the ARM64 header format.  Another problem was that the original
> "res3" field was not being initialized correctly to zero.
> 
> Address these issues by creating a 32-bit "magic2" field in the RISC-
> V
> header which matches the ARM64 "magic" field.  RISC-V binaries will
> store "RSC\x05" in this field.  The intention is that the use of the
> existing 64-bit "magic" field in the RISC-V header will be deprecated
> over time.  Increment the minor version number of the file format to
> indicate this change, and update the documentation accordingly.  Fix
> the assembler directives in head.S to ensure that reserved fields are
> properly zero-initialized.
> 

Thanks for the quick fix. Is there a planned timeline when we can
remove the deprecated magic ?

I was planning to send a U-boot header documentation patch to match
Linux one anyways. Do you want me that to rebase based on this patch or
are you planning to send a U-boot patch as well ?

> Signed-off-by: Paul Walmsley <paul.walmsley@sifive.com>
> Reported-by: Palmer Dabbelt <palmer@sifive.com>
> Cc: Atish Patra <atish.patra@wdc.com>
> Cc: Karsten Merker <merker@debian.org>
> ---
> Will try to get this merged before v5.3, to minimize the delta with
> the 
> ARM64 header in the final release.
> 
>  Documentation/riscv/boot-image-header.txt | 13 +++++++------
>  arch/riscv/include/asm/image.h            | 12 ++++++------
>  arch/riscv/kernel/head.S                  |  4 ++--
>  3 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/Documentation/riscv/boot-image-header.txt
> b/Documentation/riscv/boot-image-header.txt
> index 1b73fea23b39..14b1492f689b 100644
> --- a/Documentation/riscv/boot-image-header.txt
> +++ b/Documentation/riscv/boot-image-header.txt

There is a patch already queued that changed the format to ReST. You
need to rebase the patch accordingly

https://lkml.org/lkml/2019/7/26/1321

> @@ -18,7 +18,7 @@ The following 64-byte header is present in
> decompressed Linux kernel image.
>  	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 magic2 = 0x56534905;  /* Magic number 2, little endian,
> "RSC\x05" */
>  	u32 res4;		  /* Reserved for PE COFF offset */
>  
>  This header format is compliant with PE/COFF header and largely
> inspired from
> @@ -37,13 +37,14 @@ Notes:
>  	Bits 16:31 - Major version
>  
>    This preserves compatibility across newer and older version of the
> header.
> -  The current version is defined as 0.1.
> +  The current version is defined as 0.2.
>  
> -- 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.
> +- The "magic" field is deprecated as of version 0.2.  In a future
> +  release, it may be removed.  This originally should have matched
> up
> +  with the ARM64 header "magic" field, but unfortunately does not.
> +  The "magic2" field replaces it, matching up with the ARM64 header.
>  
> -- In current header, the flag field has only one field.
> +- In current header, the flags 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
> diff --git a/arch/riscv/include/asm/image.h
> b/arch/riscv/include/asm/image.h
> index ef28e106f247..344db5244547 100644
> --- a/arch/riscv/include/asm/image.h
> +++ b/arch/riscv/include/asm/image.h
> @@ -3,7 +3,8 @@
>  #ifndef __ASM_IMAGE_H
>  #define __ASM_IMAGE_H
>  
> -#define RISCV_IMAGE_MAGIC	"RISCV"
> +#define RISCV_IMAGE_MAGIC	"RISCV\0\0\0"
> +#define RISCV_IMAGE_MAGIC2	"RSC\x05"
>  
>  #define RISCV_IMAGE_FLAG_BE_SHIFT	0
>  #define RISCV_IMAGE_FLAG_BE_MASK	0x1
> @@ -23,7 +24,7 @@
>  #define __HEAD_FLAGS		(__HEAD_FLAG(BE))
>  
>  #define RISCV_HEADER_VERSION_MAJOR 0
> -#define RISCV_HEADER_VERSION_MINOR 1
> +#define RISCV_HEADER_VERSION_MINOR 2
>  
>  #define RISCV_HEADER_VERSION (RISCV_HEADER_VERSION_MAJOR << 16 | \
>  			      RISCV_HEADER_VERSION_MINOR)
> @@ -39,9 +40,8 @@
>   * @version:		version
>   * @res1:		reserved
>   * @res2:		reserved
> - * @magic:		Magic number
> - * @res3:		reserved (will be used for additional RISC-V
> specific
> - *			header)
> + * @magic:		Magic number (RISC-V specific; deprecated)
> + * @magic2:		Magic number 2 (to match the ARM64 'magic'
> field pos)
>   * @res4:		reserved (will be used for PE COFF offset)
>   *
>   * The intention is for this header format to be shared between
> multiple
> @@ -58,7 +58,7 @@ struct riscv_image_header {
>  	u32 res1;
>  	u64 res2;
>  	u64 magic;
> -	u32 res3;
> +	u32 magic2;
>  	u32 res4;
>  };
>  #endif /* __ASSEMBLY__ */
> diff --git a/arch/riscv/kernel/head.S b/arch/riscv/kernel/head.S
> index 0f1ba17e476f..52eec0c1bf30 100644
> --- a/arch/riscv/kernel/head.S
> +++ b/arch/riscv/kernel/head.S
> @@ -39,9 +39,9 @@ ENTRY(_start)
>  	.word RISCV_HEADER_VERSION
>  	.word 0
>  	.dword 0
> -	.asciz RISCV_IMAGE_MAGIC
> -	.word 0
> +	.ascii RISCV_IMAGE_MAGIC
>  	.balign 4
> +	.ascii RISCV_IMAGE_MAGIC2
>  	.word 0
>  
>  .global _start_kernel

-- 
Regards,
Atish

  reply index

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-14  3:08 Paul Walmsley
2019-09-14  4:03 ` Atish Patra [this message]
2019-09-14 13:58   ` Paul Walmsley
2019-09-14 12:18 ` Palmer Dabbelt

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2e697e9c7966cf1a6cac415b6a247a8ba9f4de29.camel@wdc.com \
    --to=atish.patra@wdc.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-riscv@lists.infradead.org \
    --cc=merker@debian.org \
    --cc=palmer@sifive.com \
    --cc=paul.walmsley@sifive.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

LKML Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lkml.kernel.org/lkml/0 lkml/git/0.git
	git clone --mirror https://lkml.kernel.org/lkml/1 lkml/git/1.git
	git clone --mirror https://lkml.kernel.org/lkml/2 lkml/git/2.git
	git clone --mirror https://lkml.kernel.org/lkml/3 lkml/git/3.git
	git clone --mirror https://lkml.kernel.org/lkml/4 lkml/git/4.git
	git clone --mirror https://lkml.kernel.org/lkml/5 lkml/git/5.git
	git clone --mirror https://lkml.kernel.org/lkml/6 lkml/git/6.git
	git clone --mirror https://lkml.kernel.org/lkml/7 lkml/git/7.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 lkml lkml/ https://lkml.kernel.org/lkml \
		linux-kernel@vger.kernel.org linux-kernel@archiver.kernel.org
	public-inbox-index lkml


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-kernel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox