[PATCH v5 15/29] boot: arm: riscv: sandbox: Add a format for the booti file

[Heinrich Schuchardt]

On 3/6/25 01:25, Simon Glass wrote:

There is no such thing as a booti file as booti takes pointers for three
different files: kernel, initrd, dtb. I guess you try to refer to the
Linux Image file format.

Please, rephrase the commit title.

> Arm invented a new format for arm64 and something similar is also used
> with RISC-V. Add this to the list of supported formats and provide a way
> for the format to be detected on both architectures.

In Linux `make help` provides this information:

Architecture-specific targets (arm64):
* Image.gz      - Compressed kernel image (arch/arm64/boot/Image.gz)
   Image         - Uncompressed kernel image (arch/arm64/boot/Image)
   image.fit     - Flat Image Tree (arch/arm64/boot/image.fit)
   install       - Install kernel (compressed if COMPRESSED_INSTALL set)
   zinstall      - Install compressed kernel
                   Install using (your) ~/bin/installkernel or
                   (distribution) /sbin/installkernel or
                   install to $(INSTALL_PATH) and run lilo

Architecture-specific targets (riscv):
   Image         - Uncompressed kernel image (arch/riscv/boot/Image)
   Image.gz      - Compressed kernel image (arch/riscv/boot/Image.gz)
   Image.bz2     - Compressed kernel image (arch/riscv/boot/Image.bz2)
   Image.lz4     - Compressed kernel image (arch/riscv/boot/Image.lz4)
   Image.lzma    - Compressed kernel image (arch/riscv/boot/Image.lzma)
   Image.lzo     - Compressed kernel image (arch/riscv/boot/Image.lzo)
   Image.zst     - Compressed kernel image (arch/riscv/boot/Image.zst)
   Image.xz      - Compressed kernel image (arch/riscv/boot/Image.xz)
   vmlinuz.efi   - Compressed EFI kernel image (arch/riscv/boot/vmlinuz.efi)
                   Default when CONFIG_EFI_ZBOOT=y
   xipImage      - Execute-in-place kernel image (arch/riscv/boot/xipImage)
                   Default when CONFIG_XIP_KERNEL=y
   install       - Install kernel using (your) ~/bin/installkernel or
                   (distribution) /sbin/installkernel or install to
                   $(INSTALL_PATH)

How about:

"With the booti command we support the "Image" format of the Linux
kernel in compressed and uncompressed form."

>
> Update the genimg_get_format() function to support this.

%s/this/this file format/

>
> Fix up switch() statements which don't currently mention this format.
> Booti does not support a ramdisk, so this can be ignored.

Booti takes three arguments the second one is for the RAM disk.

>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> (no changes since v3)
>
> Changes in v3:
> - Add new patch to add a format for the booti file
>
>   arch/arm/lib/image.c     | 9 ++++++++-
>   arch/riscv/lib/image.c   | 9 ++++++++-
>   arch/sandbox/lib/bootm.c | 5 +++++
>   boot/image-board.c       | 5 +++++
>   include/image.h          | 9 +++++++++
>   5 files changed, 35 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c
> index 1f672eee2c8..d78d704cb58 100644
> --- a/arch/arm/lib/image.c
> +++ b/arch/arm/lib/image.c
> @@ -28,6 +28,13 @@ struct Image_header {
>   	uint32_t	res5;
>   };
>
> +bool booti_is_valid(const void *img)
> +{
> +	const struct Image_header *ih = img;
> +
> +	return ih->magic == le32_to_cpu(LINUX_ARM64_IMAGE_MAGIC);
> +}
> +
>   int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
>   		bool force_reloc)
>   {
> @@ -39,7 +46,7 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
>
>   	ih = (struct Image_header *)map_sysmem(image, 0);
>
> -	if (ih->magic != le32_to_cpu(LINUX_ARM64_IMAGE_MAGIC)) {
> +	if (!booti_is_valid(ih)) {
>   		puts("Bad Linux ARM64 Image magic!\n");
>   		return 1;
>   	}
> diff --git a/arch/riscv/lib/image.c b/arch/riscv/lib/image.c
> index a82f48e9a50..859326cbac8 100644
> --- a/arch/riscv/lib/image.c
> +++ b/arch/riscv/lib/image.c
> @@ -32,6 +32,13 @@ struct linux_image_h {
>   	uint32_t	res4;		/* reserved */
>   };
>
> +bool booti_is_valid(const void *img)
> +{
> +	const struct linux_image_h *lhdr = img;
> +
> +	return lhdr->magic == LINUX_RISCV_IMAGE_MAGIC;

The Linux header has more fields that we should check, cf.
arch/riscv/kernel/head.S

#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

Please, check that the bitness matches the U-Boot architecture.

         .ascii RISCV_IMAGE_MAGIC
         .balign 4
         .ascii RISCV_IMAGE_MAGIC2

#define RISCV_IMAGE_MAGIC       "RISCV\0\0\0"
#define RISCV_IMAGE_MAGIC2      "RSC\x05"

Consider checking both magic fields.

> +}
> +
>   int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
>   		bool force_reloc)
>   {
> @@ -39,7 +46,7 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
>
>   	lhdr = (struct linux_image_h *)map_sysmem(image, 0);
>
> -	if (lhdr->magic != LINUX_RISCV_IMAGE_MAGIC) {
> +	if (!booti_is_valid(lhdr)) {
>   		puts("Bad Linux RISCV Image magic!\n");

As described above we should check more than the magic.

>   		return -EINVAL;
>   	}
> diff --git a/arch/sandbox/lib/bootm.c b/arch/sandbox/lib/bootm.c
> index 44ba8b52e13..8ed923750f4 100644
> --- a/arch/sandbox/lib/bootm.c
> +++ b/arch/sandbox/lib/bootm.c
> @@ -89,3 +89,8 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
>
>   	return 1;
>   }
> +
> +bool booti_is_valid(const void *img)
> +{
> +	return false;
> +}
> diff --git a/boot/image-board.c b/boot/image-board.c
> index addccf87c80..07931c64198 100644
> --- a/boot/image-board.c
> +++ b/boot/image-board.c
> @@ -250,6 +250,9 @@ enum image_fmt_t genimg_get_format(const void *img_addr)
>   	if (IS_ENABLED(CONFIG_ANDROID_BOOT_IMAGE) &&
>   	    is_android_boot_image_header(img_addr))
>   		return IMAGE_FORMAT_ANDROID;
> +	if (IS_ENABLED(CONFIG_CMD_BOOTI) &&
> +	    booti_is_valid(img_addr))
> +		return IMAGE_FORMAT_BOOTI;
>
>   	return IMAGE_FORMAT_INVALID;
>   }
> @@ -420,6 +423,8 @@ static int select_ramdisk(struct bootm_headers *images, const char *select, u8 a
>   			done = true;
>   		}
>   		break;
> +	case IMAGE_FORMAT_BOOTI:
> +		break;
>   	case IMAGE_FORMAT_INVALID:
>   		break;
>   	}
> diff --git a/include/image.h b/include/image.h
> index dc1a7c307cc..f8f2c887a4b 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -603,6 +603,7 @@ enum image_fmt_t {
>   	IMAGE_FORMAT_LEGACY,		/* legacy image_header based format */
>   	IMAGE_FORMAT_FIT,		/* new, libfdt based format */
>   	IMAGE_FORMAT_ANDROID,		/* Android boot image */
> +	IMAGE_FORMAT_BOOTI,		/* Arm64/RISC-V boot image */

Please, describe the enum in Sphinx style, see
https://docs.kernel.org/doc-guide/kernel-doc.html#structure-union-and-enumeration-documentation

Please, avoid self-referential naming (booti supports the booti format):

%s/IMAGE_FORMAT_BOOTI/IMAGE_FORMAT_LINUX_IMAGE/

>   };
>
>   /**
> @@ -649,6 +650,14 @@ enum image_fmt_t genimg_get_format(const void *img_addr);
>
>   int genimg_has_config(struct bootm_headers *images);
>
> +/**
> + * booti_is_valid() - Check if an image appears to be an Arm64 image

Above you implement the function for RISC-V.

"Check if an image is in a compatible Linux Image format"

> + *
> + * @img: Pointer to image
> + * Return: true if the image has the Arm64 magic

For RISC-V we should be checking more than the magic. See above.

Best regards

Heinrich

> + */
> +bool booti_is_valid(const void *img);
> +
>   /**
>    * boot_get_fpga() - Locate the FPGA image
>    *