[RFC PATCH v2 9/9] tools: spkgimage: add Renesas SPKG format

Sean Anderson seanga2 at gmail.com
Sat Aug 13 16:47:27 CEST 2022


On 8/12/22 1:03 PM, Ralph Siemsen wrote:
> Renesas RZ/N1 devices contain BootROM code that loads a custom SPKG
> image from QSPI, NAND or USB DFU. Support this format in mkimage tool.
> 
> SPKGs can optionally be signed, however creation of signed SPKG is not
> currently supported.
> 
> Example of how to use it:
> 
> tools/mkimage -n board/schneider/lces/spkgimage.cfg \
> 	-T spkgimage -a 0x20040000 -e 0x20040000 \
> 	-d u-boot.bin u-boot.bin.spkg
> 
> The config file (spkgimage.cfg in this example) contains additional
> parameters such as NAND ECC settings.
> 
> Signed-off-by: Ralph Siemsen <ralph.siemsen at linaro.org>
> ---
> 
> Changes in v2:
> - rewrote the stand-alone spkg_utility to integrate into mkimage
> 
>   board/schneider/lces/spkgimage.cfg |  26 +++
>   boot/image.c                       |   1 +
>   include/image.h                    |   1 +
>   tools/Makefile                     |   1 +
>   tools/spkgimage.c                  | 303 +++++++++++++++++++++++++++++
>   tools/spkgimage.h                  |  39 ++++

Please document your format in doc/mkimage.1

>   6 files changed, 371 insertions(+)
>   create mode 100644 board/schneider/lces/spkgimage.cfg
>   create mode 100644 tools/spkgimage.c
>   create mode 100644 tools/spkgimage.h
> 
> diff --git a/board/schneider/lces/spkgimage.cfg b/board/schneider/lces/spkgimage.cfg
> new file mode 100644
> index 0000000000..b5faf96b00
> --- /dev/null
> +++ b/board/schneider/lces/spkgimage.cfg
> @@ -0,0 +1,26 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +#
> +# (C) Copyright 2022 Schneider Electric
> +#
> +# SPKG image header, for booting on RZ/N1
> +
> +# b[35:32] SPKG version
> +VERSION			1
> +
> +# b[42:41]  ECC Block size: 0=256 bytes, 1=512 bytes, 2=1024 bytes
> +NAND_ECC_BLOCK_SIZE	1
> +
> +# b[45]     NAND enable (boolean)
> +NAND_ECC_ENABLE		1
> +
> +# b[50:48]  ECC Scheme: 0=BCH2 1=BCH4 2=BCH8 3=BCH16 4=BCH24 5=BCH32
> +NAND_ECC_SCHEME		3
> +
> +# b[63:56]  ECC bytes per block
> +NAND_BYTES_PER_ECC_BLOCK 28
> +
> +# Provide dummy BLp header (boolean)
> +ADD_DUMMY_BLP		1
> +
> +# Pad the image to a multiple of
> +PADDING			64K
> diff --git a/boot/image.c b/boot/image.c
> index 5dcb55ba46..7d50d708ab 100644
> --- a/boot/image.c
> +++ b/boot/image.c
> @@ -179,6 +179,7 @@ static const table_entry_t uimage_type[] = {
>   	{	IH_TYPE_COPRO, "copro", "Coprocessor Image"},
>   	{	IH_TYPE_SUNXI_EGON, "sunxi_egon",  "Allwinner eGON Boot Image" },
>   	{	IH_TYPE_SUNXI_TOC0, "sunxi_toc0",  "Allwinner TOC0 Boot Image" },
> +	{	IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" },
>   	{	-1,		    "",		  "",			},
>   };
>   
> diff --git a/include/image.h b/include/image.h
> index e4c6a50b88..98eaa384f4 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -229,6 +229,7 @@ enum {
>   	IH_TYPE_COPRO,			/* Coprocessor Image for remoteproc*/
>   	IH_TYPE_SUNXI_EGON,		/* Allwinner eGON Boot Image */
>   	IH_TYPE_SUNXI_TOC0,		/* Allwinner TOC0 Boot Image */
> +	IH_TYPE_RENESAS_SPKG,		/* Renesas SPKG image */
>   
>   	IH_TYPE_COUNT,			/* Number of image types */
>   };
> diff --git a/tools/Makefile b/tools/Makefile
> index 005e7362a3..7e24f3ecb9 100644
> --- a/tools/Makefile
> +++ b/tools/Makefile
> @@ -131,6 +131,7 @@ dumpimage-mkimage-objs := aisimage.o \
>   			stm32image.o \
>   			$(ROCKCHIP_OBS) \
>   			socfpgaimage.o \
> +			spkgimage.o \
>   			sunxi_egon.o \
>   			lib/crc16-ccitt.o \
>   			lib/hash-checksum.o \
> diff --git a/tools/spkgimage.c b/tools/spkgimage.c
> new file mode 100644
> index 0000000000..2e8c17d94a
> --- /dev/null
> +++ b/tools/spkgimage.c
> @@ -0,0 +1,303 @@
> +// SPDX-License-Identifier: BSD-2-Clause
> +/*
> + * Generate Renesas RZ/N1 BootROM header (SPKG)
> + * (C) Copyright 2022 Schneider Electric
> + *
> + * Based on spkg_utility.c
> + * (C) Copyright 2016 Renesas Electronics Europe Ltd
> + */
> +
> +#include "imagetool.h"
> +#include <limits.h>
> +#include <image.h>
> +#include <stdarg.h>
> +#include <stdint.h>
> +#include <u-boot/crc.h>
> +#include "spkgimage.h"
> +
> +static struct spkg_file out_buf;
> +
> +static uint32_t padding;
> +
> +/* Note: the ordering of the bitfields does not matter */
> +static struct config_file {
> +	unsigned int version:1;
> +	unsigned int ecc_block_size:2;
> +	unsigned int ecc_enable:1;
> +	unsigned int ecc_scheme:3;
> +	unsigned int ecc_bytes:8;
> +	unsigned int blp_len;
> +	unsigned int padding;
> +} conf;

I wonder if you could just fill in the header directly. This is
for a userspace tool, and this struct will be created at most
once. It's OK to use 10 bytes :)

> +static int spkgimage_parse_config_line(char *line)
> +{
> +	char *saveptr;
> +	char *delim = "\t ";
> +	char *name = strtok_r(line, delim, &saveptr);
> +	char *val_str = strtok_r(NULL, delim, &saveptr);
> +	int value = atoi(val_str);
> +
> +	if (!strcmp("VERSION", name)) {
> +		conf.version = value;
> +	} else if (!strcmp("NAND_ECC_ENABLE", name)) {
> +		conf.ecc_enable = value;

Can you add some checks for the valid range of values? E.g.
NAND_ECC_SCHEME should be 0 <= value <= 5

> +	} else if (!strcmp("NAND_ECC_BLOCK_SIZE", name)) {
> +		conf.ecc_block_size = value;
> +	} else if (!strcmp("NAND_ECC_SCHEME", name)) {
> +		conf.ecc_scheme = value;
> +	} else if (!strcmp("NAND_BYTES_PER_ECC_BLOCK", name)) {
> +		conf.ecc_bytes = value;
> +	} else if (!strcmp("ADD_DUMMY_BLP", name)) {
> +		conf.blp_len = value ? SPKG_BLP_SIZE : 0;
> +	} else if (!strcmp("PADDING", name)) {
> +		if (strrchr(val_str, 'K'))
> +			conf.padding = value * 1024;
> +		else if (strrchr(val_str, 'M'))
> +			conf.padding = value * 1024 * 1024;
> +		else
> +			conf.padding = value;
> +	} else {
> +		fprintf(stderr, "Error: unknown keyword '%s' in config\n",
> +			name);

perhaps print the line number?

> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int spkgimage_parse_config_file(char *filename)
> +{
> +	FILE *fcfg;
> +	char line[256];
> +	size_t len;
> +
> +	fcfg = fopen(filename, "r");
> +	if (!fcfg)
> +		return -EINVAL;
> +
> +	while (fgets(line, sizeof(line), fcfg)) {
> +		/* Skip blank lines and comments */
> +		if (line[0] == '\n' || line[0] == '#')
> +			continue;
> +
> +		/* Strip the trailing newline */
> +		len = strlen(line);
> +		if (line[len - 1] == '\n')
> +			line[--len] = 0;
> +
> +		/* Parse the line */
> +		if (spkgimage_parse_config_line(line))
> +			return -EINVAL;
> +	}
> +
> +	fclose(fcfg);
> +
> +	/* Avoid divide-by-zero later on */
> +	if (conf.padding == 0)

if (!conf.padding)

> +		conf.padding = 1;
> +
> +	return 0;
> +}
> +
> +static int spkgimage_check_params(struct image_tool_params *params)
> +{
> +	if (!params->addr) {
> +		fprintf(stderr, "Error: Load Address must be set.\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!params->imagename || !params->imagename[0]) {
> +		fprintf(stderr, "Error: Image name must be set.\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!params->datafile) {
> +		fprintf(stderr, "Error: Data filename must be set.\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int spkgimage_verify_header(unsigned char *ptr, int size,
> +				   struct image_tool_params *param)
> +{
> +	struct spkg_file *file = (struct spkg_file *)ptr;
> +	struct spkg_hdr *header = (struct spkg_hdr *)ptr;
> +	char signature[4] = SPKG_HEADER_SIGNATURE;

If this naming does not come from documentation, I would suggest
something like SPKG_HEADER_MAGIC, since this is not a signature,
or even a CRC.

> +	uint32_t payload_length;
> +	uint32_t crc;
> +	uint8_t *crc_buf;
> +
> +	/* Check the signature */
> +	if (memcmp(header->signature, signature, 4)) {
> +		fprintf(stderr, "Error: invalid signature bytes\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Check the CRC */
> +	crc = crc32(0, ptr, SPKG_HEADER_SIZE - SPKG_CRC_SIZE);
> +	if (crc != header->crc) {
> +		fprintf(stderr, "Error: invalid header CRC=\n");
> +		return -EINVAL;
> +	}
> +
> +	/* Check all copies of header are the same */
> +	for (int i = 1; i < SPKG_HEADER_COUNT; i++) {
> +		if (memcmp(&header[0], &header[i], SPKG_HEADER_SIZE)) {
> +			fprintf(stderr, "Error: header %d mismatch\n", i);
> +			return -EINVAL;
> +		}
> +	}
> +
> +	/* Check the payload CRC */
> +	payload_length = le32_to_cpu(header->payload_length) >> 8;
> +	crc_buf = file->payload + payload_length - SPKG_CRC_SIZE;
> +	crc = crc32(0, file->payload, payload_length - SPKG_CRC_SIZE);
> +	if (crc_buf[0] != (crc & 0xff) ||
> +	    crc_buf[1] != (crc >> 8 & 0xff) ||
> +	    crc_buf[2] != (crc >> 16 & 0xff) ||
> +	    crc_buf[3] != (crc >> 24 & 0xff)) {
> +		fprintf(stderr, "Error: invalid payload CRC\n");
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static void spkgimage_print_header(const void *ptr)
> +{
> +	const struct spkg_hdr *h = ptr;
> +	uint32_t offset = le32_to_cpu(h->execution_offset);
> +
> +	printf("Image type\t: Renesas SPKG Image\n");
> +	printf("Marker\t\t: %c%c%c%c\n", h->signature[0], h->signature[1],
> +					 h->signature[2], h->signature[3]);
> +	printf("Version\t\t: %d\n", h->version);
> +	printf("ECC\t\t: ");
> +	if (h->ecc & 0x20)
> +		printf("Scheme %d, Block size %d, Strength %d\n",
> +		       h->ecc_scheme, (h->ecc >> 1) & 3, h->ecc_bytes);
> +	else
> +		printf("Not enabled\n");
> +	printf("Payload length\t: %d\n", le32_to_cpu(h->payload_length) >> 8);
> +	printf("Load address\t: 0x%08x\n", le32_to_cpu(h->load_address));
> +	printf("Execution offset: 0x%08x (%s mode)\n", offset & ~1,
> +	       offset & 1 ? "THUMB" : "ARM");
> +	printf("Header checksum\t: 0x%08x\n", le32_to_cpu(h->crc));
> +}
> +
> +static inline uint32_t roundup(uint32_t x, uint32_t y)
> +{
> +	return ((x + y - 1) / y) * y;
> +}
> +
> +static int spkgimage_vrec_header(struct image_tool_params *params,
> +				 struct image_type_params *tparams)
> +{
> +	struct stat s;
> +
> +	/* Parse the config file */
> +	if (spkgimage_parse_config_file(params->imagename)) {
> +		fprintf(stderr, "Error parsing config file\n");
> +		exit(EXIT_FAILURE);
> +	}
> +
> +	/* Get size of input data file */
> +	if (stat(params->datafile, &s)) {
> +		fprintf(stderr, "Could not stat data file: %s: %s\n",
> +			params->datafile, strerror(errno));
> +		exit(EXIT_FAILURE);
> +	}
> +	params->orig_file_size = s.st_size;
> +
> +	/* Determine size of resulting SPKG file */
> +	uint32_t header_len = SPKG_HEADER_SIZE * SPKG_HEADER_COUNT;
> +	uint32_t payload_len = conf.blp_len + s.st_size + SPKG_CRC_SIZE;
> +	uint32_t total_len = header_len + payload_len;
> +
> +	/* Round up to next multiple of padding size */
> +	uint32_t padded_len = roundup(total_len, conf.padding);
> +
> +	/* Number of padding bytes to add */
> +	padding = padded_len - total_len;
> +
> +	/* Fixup payload_len to include padding bytes */
> +	payload_len += padding;
> +
> +	/* Prepare the header */
> +	struct spkg_hdr header = {
> +		.signature = SPKG_HEADER_SIGNATURE,
> +		.version = conf.version,
> +		.ecc = (conf.ecc_enable << 5) | (conf.ecc_block_size << 1),
> +		.ecc_scheme = conf.ecc_scheme,
> +		.ecc_bytes = conf.ecc_bytes,
> +		.payload_length = cpu_to_le32(payload_len << 8),
> +		.load_address = cpu_to_le32(params->addr),
> +		.execution_offset = cpu_to_le32(params->ep - params->addr),
> +	};
> +	header.crc = crc32(0, (uint8_t *)&header,
> +			   sizeof(header) - SPKG_CRC_SIZE);
> +
> +	/* Fill the SPKG with the headers */
> +	for (int i = 0; i < SPKG_HEADER_COUNT; i++)
> +		memcpy(&out_buf.header[i], &header, sizeof(header));
> +
> +	/* Extra bytes to allocate in the output file */
> +	return conf.blp_len + padding + 4;
> +}
> +
> +static void spkgimage_set_header(void *ptr, struct stat *sbuf, int ifd,
> +				 struct image_tool_params *params)
> +{
> +	uint8_t *payload = ptr + SPKG_HEADER_SIZE * SPKG_HEADER_COUNT;
> +	uint8_t *file_end = payload + conf.blp_len + params->orig_file_size;
> +	uint8_t *crc_buf = file_end + padding;
> +	uint32_t crc;
> +
> +	/* Make room for the Dummy BLp header */
> +	memmove(payload + conf.blp_len, payload, params->orig_file_size);
> +
> +	/* Fill the SPKG with the Dummy BLp */
> +	memset(payload, 0x88, conf.blp_len);
> +
> +	/*
> +	 * mkimage copy_file() pads the input file with zeros.
> +	 * Replace those zeros with flash friendly one bits.
> +	 * The original version skipped the fist 4 bytes,

nit: first

> +	 * probably an oversight, but for consistency we
> +	 * keep the same behaviour.
> +	 */
> +	memset(file_end + 4, 0xff, padding - 4);
> +
> +	/* Add Payload CRC */
> +	crc = crc32(0, payload, crc_buf - payload);
> +	crc_buf[0] = crc;
> +	crc_buf[1] = crc >> 8;
> +	crc_buf[2] = crc >> 16;
> +	crc_buf[3] = crc >> 24;
> +}
> +
> +static int spkgimage_check_image_types(uint8_t type)
> +{
> +	return type == IH_TYPE_RENESAS_SPKG ? 0 : 1;

This function is not necessary if you only support one type.

> +}
> +
> +/*
> + * spkgimage type parameter definition
> + */
> +U_BOOT_IMAGE_TYPE(
> +	spkgimage,
> +	"Renesas SPKG Image",
> +	sizeof(out_buf),
> +	&out_buf,
> +	spkgimage_check_params,
> +	spkgimage_verify_header,
> +	spkgimage_print_header,
> +	spkgimage_set_header,
> +	NULL,
> +	spkgimage_check_image_types,
> +	NULL,
> +	spkgimage_vrec_header
> +);
> diff --git a/tools/spkgimage.h b/tools/spkgimage.h
> new file mode 100644
> index 0000000000..db153bfc3f
> --- /dev/null
> +++ b/tools/spkgimage.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: BSD-2-Clause */
> +/*
> + * Renesas RZ/N1 Package Table format
> + * (C) 2015-2016 Renesas Electronics Europe, LTD
> + * All rights reserved.
> + *
> + * Converted to mkimage plug-in
> + * (C) Copyright 2022 Schneider Electric
> + */
> +
> +#ifndef _SPKGIMAGE_H_
> +#define _SPKGIMAGE_H_
> +
> +#define SPKG_HEADER_SIGNATURE	{'R', 'Z', 'N', '1'}
> +#define SPKG_HEADER_SIZE	24
> +#define SPKG_HEADER_COUNT	8

What are the other 7 headers for? Should you print them out above?

> +#define SPKG_BLP_SIZE		264
> +#define SPKG_CRC_SIZE		4
> +
> +/* SPKG header */
> +struct spkg_hdr {
> +	uint8_t		signature[4];
> +	uint8_t		version;
> +	uint8_t		ecc;
> +	uint8_t		ecc_scheme;
> +	uint8_t		ecc_bytes;
> +	uint32_t	payload_length; /* only HIGHER 24 bits */
> +	uint32_t	load_address;
> +	uint32_t	execution_offset;
> +	uint32_t	crc; /* of this header */
> +} __packed;
> +
> +struct spkg_file {
> +	struct spkg_hdr	header[SPKG_HEADER_COUNT];
> +	uint8_t		payload[0];
> +	/* then the CRC */
> +} __packed;
> +
> +#endif
> 

--Sean


More information about the U-Boot mailing list