diff mbox series

[RFC,v1,9/9] tools: Add tool to create Renesas SPKG images

Message ID 20220809125959.217333-10-ralph.siemsen@linaro.org
State New
Headers show
Series Renesas RZ/N1 SoC initial support | expand

Commit Message

Ralph Siemsen Aug. 9, 2022, 12:59 p.m. UTC
From: Michel Pollet <michel.pollet@bp.renesas.com>

Renesas RZ/N1 devices contain BootROM code that loads a custom SPKG
image from QSPI, NAND or USB DFU. This tool converts a binary image into
an SPKG.

SPKGs can optionally be signed, however this tool does not currently
support signed SPKGs.

Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
Signed-off-by: Ralph Siemsen <ralph.siemsen@linaro.org>
---
This tool could possibly be incorporated into mkimage / imagetools.
However it is unclear how to handle the extra commandline parameters
(NAND ECC settings, etc). So for now it is stand-alone tool.

 tools/Makefile       |   2 +
 tools/spkg_header.h  |  49 +++++++
 tools/spkg_utility.c | 306 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 357 insertions(+)
 create mode 100644 tools/spkg_header.h
 create mode 100644 tools/spkg_utility.c

Comments

Pali Rohár Aug. 9, 2022, 1:03 p.m. UTC | #1
On Tuesday 09 August 2022 08:59:59 Ralph Siemsen wrote:
> From: Michel Pollet <michel.pollet@bp.renesas.com>
> 
> Renesas RZ/N1 devices contain BootROM code that loads a custom SPKG
> image from QSPI, NAND or USB DFU. This tool converts a binary image into
> an SPKG.
> 
> SPKGs can optionally be signed, however this tool does not currently
> support signed SPKGs.
> 
> Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
> Signed-off-by: Ralph Siemsen <ralph.siemsen@linaro.org>
> ---
> This tool could possibly be incorporated into mkimage / imagetools.
> However it is unclear how to handle the extra commandline parameters
> (NAND ECC settings, etc). So for now it is stand-alone tool.

Hello! You can use for example config file, like it has kwbimage.c which
is integrated into mkimage and has support for NAND ECC settings.

It is really better if BootROM specific image format is included in
standard mkimage and dumpimage tools instead of new custom vendor tools.
Pali Rohár Aug. 9, 2022, 1:07 p.m. UTC | #2
On Tuesday 09 August 2022 15:03:48 Pali Rohár wrote:
> On Tuesday 09 August 2022 08:59:59 Ralph Siemsen wrote:
> > From: Michel Pollet <michel.pollet@bp.renesas.com>
> > 
> > Renesas RZ/N1 devices contain BootROM code that loads a custom SPKG
> > image from QSPI, NAND or USB DFU. This tool converts a binary image into
> > an SPKG.
> > 
> > SPKGs can optionally be signed, however this tool does not currently
> > support signed SPKGs.
> > 
> > Signed-off-by: Michel Pollet <michel.pollet@bp.renesas.com>
> > Signed-off-by: Ralph Siemsen <ralph.siemsen@linaro.org>
> > ---
> > This tool could possibly be incorporated into mkimage / imagetools.
> > However it is unclear how to handle the extra commandline parameters
> > (NAND ECC settings, etc). So for now it is stand-alone tool.
> 
> Hello! You can use for example config file, like it has kwbimage.c which
> is integrated into mkimage and has support for NAND ECC settings.

Or another option could be to extend mkimage tool to accept new argument
for specifying NAND settings. As we can see more image formats have
support for it, so some abstraction in mkimage makes sense here.

> It is really better if BootROM specific image format is included in
> standard mkimage and dumpimage tools instead of new custom vendor tools.
Ralph Siemsen Aug. 9, 2022, 3:54 p.m. UTC | #3
Hi Pali,

On Tuesday 09 August 2022 15:03:48 Pali Rohár wrote:
>
> Hello! You can use for example config file, like it has kwbimage.c which
> is integrated into mkimage and has support for NAND ECC settings.

Thank you, I was unaware of this config file approach. From a quick
look at doc/README.kwbimage it seems quite reasonable for
device-specific parameters.

On Tue, Aug 9, 2022 at 9:07 AM Pali Rohár <pali@kernel.org> wrote:
>
> Or another option could be to extend mkimage tool to accept new argument
> for specifying NAND settings. As we can see more image formats have
> support for it, so some abstraction in mkimage makes sense here.

In principle I agree. But practically speaking I see two problems:
1) mkimage already has far too many options
2) covering all possible vendor-specific values/encodings of NAND
parameters would be quite a task

So I am inclined to keep it simple for now, and try using a config file.

Ralph
Pali Rohár Aug. 9, 2022, 4:06 p.m. UTC | #4
Hello!

On Tuesday 09 August 2022 11:54:30 Ralph Siemsen wrote:
> Hi Pali,
> 
> On Tuesday 09 August 2022 15:03:48 Pali Rohár wrote:
> >
> > Hello! You can use for example config file, like it has kwbimage.c which
> > is integrated into mkimage and has support for NAND ECC settings.
> 
> Thank you, I was unaware of this config file approach. From a quick
> look at doc/README.kwbimage it seems quite reasonable for
> device-specific parameters.

This documentation is not probably up-to-date. List of all kwbimage
config options can be visible in kwbimage_generate_config() function.

> On Tue, Aug 9, 2022 at 9:07 AM Pali Rohár <pali@kernel.org> wrote:
> >
> > Or another option could be to extend mkimage tool to accept new argument
> > for specifying NAND settings. As we can see more image formats have
> > support for it, so some abstraction in mkimage makes sense here.
> 
> In principle I agree. But practically speaking I see two problems:
> 1) mkimage already has far too many options

I know. But for nand there can be just --nand suboption1,suboption2,...
format. For other non-nand vendor specific option it would be an issue.
Maybe something like --vendor option or --image-options ... could be
used?

> 2) covering all possible vendor-specific values/encodings of NAND
> parameters would be quite a task

Yea, it is problematic. But... is not everything related to NAND just
common to what can be specified in device tree properties for nand node?
And de-facto already known and well-defined?

Because I cannot imagine what else for what there is not already device
tree binding, could be required for vendor bootrom. (But maybe I just do
not see it...)

> So I am inclined to keep it simple for now, and try using a config file.
> 
> Ralph

Keep it simple is the best option!

In the worst case, in the future, some of the options introduced by your
config file format, would be possible to specify also via command line
arguments. So I do not see any issue with this config file approach.


Just one suggestion: It is a good idea to also implement "verify_header"
mkimage callback. Build process then use it to verify that generated
image is really correct.
Ralph Siemsen Aug. 9, 2022, 5:02 p.m. UTC | #5
On Tue, Aug 9, 2022 at 12:07 PM Pali Rohár <pali@kernel.org> wrote:
>
> This documentation is not probably up-to-date. List of all kwbimage
> config options can be visible in kwbimage_generate_config() function.

I will check the code as well.

> > 1) mkimage already has far too many options
>
> I know. But for nand there can be just --nand suboption1,suboption2,...
> format. For other non-nand vendor specific option it would be an issue.
> Maybe something like --vendor option or --image-options ... could be
> used?

In my case this would work, as there are only 3 parameters for NAND.
However in general, it could get pretty messy, I see at least ten
different NAND parameters described in the NAND DT binding.

> > 2) covering all possible vendor-specific values/encodings of NAND
> > parameters would be quite a task
>
> Yea, it is problematic. But... is not everything related to NAND just
> common to what can be specified in device tree properties for nand node?
> And de-facto already known and well-defined?

I did have the thought of using device tree. It does have the
advantage that it is a known format with defined parameters.

> Because I cannot imagine what else for what there is not already device
> tree binding, could be required for vendor bootrom. (But maybe I just do
> not see it...)

In quite a few cases, the DT parameters are incomplete, or just hints
(see "nand-ecc-maximize"), that trigger various run-time decisions
about the actual parameters.

Duplicating this logic in mkimage seems difficult, bordering on
impossible if it depends on run-time identification of the flash chip,
for example.

> Just one suggestion: It is a good idea to also implement "verify_header"
> mkimage callback. Build process then use it to verify that generated
> image is really correct.

I'll check it out, thanks!

Ralph
Sean Anderson Aug. 9, 2022, 5:15 p.m. UTC | #6
Hi Ralph,

On 8/9/22 11:54 AM, Ralph Siemsen wrote:
> Hi Pali,
> 
> On Tuesday 09 August 2022 15:03:48 Pali Rohár wrote:
>>
>> Hello! You can use for example config file, like it has kwbimage.c which
>> is integrated into mkimage and has support for NAND ECC settings.
> 
> Thank you, I was unaware of this config file approach. From a quick
> look at doc/README.kwbimage it seems quite reasonable for
> device-specific parameters.
> 
> On Tue, Aug 9, 2022 at 9:07 AM Pali Rohár <pali@kernel.org> wrote:
>>
>> Or another option could be to extend mkimage tool to accept new argument
>> for specifying NAND settings. As we can see more image formats have
>> support for it, so some abstraction in mkimage makes sense here.
> 
> In principle I agree. But practically speaking I see two problems:
> 1) mkimage already has far too many options
> 2) covering all possible vendor-specific values/encodings of NAND
> parameters would be quite a task
> 
> So I am inclined to keep it simple for now, and try using a config file.

The traditional way to handle this is to specify a config file with -n.
See e.g. mtk_image

--Sean
Ralph Siemsen Aug. 12, 2022, 5 p.m. UTC | #7
On Tue, Aug 9, 2022 at 1:15 PM Sean Anderson <sean.anderson@seco.com> wrote:
>
> The traditional way to handle this is to specify a config file with -n.
> See e.g. mtk_image

Thanks Pali and Sean. I have converted this tool to work as part of
mkimage, with a config file for the extra parameters. Patch v2 to
follow.

Ralph
diff mbox series

Patch

diff --git a/tools/Makefile b/tools/Makefile
index 005e7362a3..c5dc92a13f 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -68,6 +68,8 @@  HOSTCFLAGS_img2srec.o := -pedantic
 hostprogs-$(CONFIG_XWAY_SWAP_BYTES) += xway-swap-bytes
 HOSTCFLAGS_xway-swap-bytes.o := -pedantic
 
+hostprogs-$(CONFIG_ARCH_RZN1) += spkg_utility
+
 hostprogs-y += mkenvimage
 mkenvimage-objs := mkenvimage.o os_support.o lib/crc32.o
 
diff --git a/tools/spkg_header.h b/tools/spkg_header.h
new file mode 100644
index 0000000000..029930cbf8
--- /dev/null
+++ b/tools/spkg_header.h
@@ -0,0 +1,49 @@ 
+/* SPDX-License-Identifier: BSD-2-Clause */
+/*
+ * Renesas RZ/N1 Linux tools: Package Table format
+ * (C) 2015-2016 Renesas Electronics Europe, LTD
+ * All rights reserved.
+ */
+
+#ifndef _SKGT_HEADER_H_
+#define _SKGT_HEADER_H_
+
+#define SPKG_HEADER_SIGNATURE (('R'<<0)|('Z'<<8)|('N'<<16)|('1'<<24))
+#define SPKG_HEADER_COUNT	8
+#define SPKG_BLP_SIZE		264
+
+#define SPKG_HEADER_SIZE	24
+#define SPKG_HEADER_SIZE_ALL	(SPKG_HEADER_SIZE * SPKG_HEADER_COUNT)
+#define SPKG_HEADER_CRC_SIZE	4
+
+/* Index into SPKG */
+#define INDEX_BLP_START		SPKG_HEADER_SIZE_ALL
+#define INDEX_IMAGE_START	(INDEX_BLP_START + SPKG_BLP_SIZE)
+
+/* Flags, not supported by ROM code, only used for U-Boot SPL */
+enum {
+	SPKG_CODE_NONSEC_BIT = 0,
+	SPKG_CODE_HYP_BIT,
+};
+
+/* SPKG header */
+struct spkg_hdr {
+	uint32_t	signature;
+	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 */
+} __attribute__((packed));
+
+struct spkg_file {
+	struct spkg_hdr		header[SPKG_HEADER_COUNT];
+	uint8_t			blp[SPKG_BLP_SIZE];
+	uint8_t			data[0];
+	/* then the CRC */
+} __attribute__((packed));
+
+#endif
diff --git a/tools/spkg_utility.c b/tools/spkg_utility.c
new file mode 100644
index 0000000000..235e23e0f1
--- /dev/null
+++ b/tools/spkg_utility.c
@@ -0,0 +1,306 @@ 
+// SPDX-License-Identifier: BSD-2-Clause
+/*
+ * This is a utility to create a SPKG file.
+ * It packages the binary code into the SPKG.
+ *
+ * (C) Copyright 2016 Renesas Electronics Europe Ltd
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <string.h>
+
+#include "spkg_header.h"
+
+/* For Windows compatibility */
+#ifndef htole32
+#if __BYTE_ORDER == __LITTLE_ENDIAN
+ #define htole32(x)	(x)
+#elif __BYTE_ORDER == __BIG_ENDIAN
+ #define htole32(x)	__builtin_bswap32((uint32_t)(x))
+#endif
+#endif
+
+#define MAX_PATH		300
+
+// Note: Order of bit fields is not used, this is purely just holding the SPKG information.
+struct spkg_header {
+	char input[MAX_PATH];
+	char output[MAX_PATH + 5];
+	unsigned int version:4;
+	unsigned int ecc_enable:1;
+	unsigned int ecc_block_size:2;
+	unsigned int ecc_scheme:3;
+	unsigned int ecc_bytes:8;
+	unsigned int dummy_blp_length:10;
+	unsigned int payload_length:24;
+	unsigned int spl_nonsec:1;
+	unsigned int spl_hyp:1;
+	unsigned int load_address;
+	unsigned int execution_offset;
+	uint32_t padding;
+};
+
+struct spkg_header g_header = {
+	.version = 1,
+	.padding = 256,
+};
+
+int verbose;
+
+static uint32_t crc32(const uint8_t *message, uint32_t l)
+{
+	uint32_t crc = ~0;
+
+	while (l--) {
+		uint32_t byte = *message++;		// Get next byte.
+
+		crc = crc ^ byte;
+		for (int8_t j = 7; j >= 0; j--) {	// Do eight times.
+			uint32_t mask = -(crc & 1);
+
+			crc = (crc >> 1) ^ (0xEDB88320 & mask);
+		}
+	}
+	return ~crc;
+}
+
+static int spkg_write(struct spkg_header *h, FILE *file_input, FILE *file_SPKG)
+{
+	int i;
+	uint32_t length_inputfile;
+	uint32_t length_read;
+	uint32_t length_written;
+	uint32_t length_total;
+	uint32_t padding = 0;
+	uint8_t *data, *start;
+	uint32_t crc;
+
+	/* Calculate length of input file */
+	fseek(file_input, 0, SEEK_END);	// seek to end of file
+	length_inputfile = ftell(file_input);	// get current file pointer
+	fseek(file_input, 0, SEEK_SET);	// seek back to beginning of file
+
+	/* Set payload_length field. */
+	h->payload_length =
+	    length_inputfile + h->dummy_blp_length + SPKG_HEADER_CRC_SIZE;
+
+	/* Calculate total length of SPKG */
+	length_total =
+	    (SPKG_HEADER_SIZE * SPKG_HEADER_COUNT) + h->dummy_blp_length +
+	    length_inputfile + SPKG_HEADER_CRC_SIZE;
+	padding = h->padding ? h->padding - (length_total % h->padding) : 0;
+	length_total += padding;
+	/* Padding needs to be part of the payload size, otherwise the ROM DFU
+	 * refuses to accept the extra bytes and return and error.
+	 */
+	h->payload_length += padding;
+
+	printf("Addr: 0x%08x ", h->load_address);
+	printf("In: %8d ", length_inputfile);
+	printf("padding to %3dKB: %6d ", h->padding / 1024, padding);
+	printf("Total: 0x%08x ", length_total);
+	printf("%s\n", h->output);
+
+	/* Create and zero array for SPKG */
+	data = malloc(length_total);
+	memset(data, 0, length_total);
+
+	/* Fill the SPKG with the headers */
+	{
+		struct spkg_hdr head = {
+			.signature = SPKG_HEADER_SIGNATURE,
+			.version = h->version,
+			.ecc = (h->ecc_enable << 5) | (h->ecc_block_size << 1),
+			.ecc_scheme = h->ecc_scheme,
+			.ecc_bytes = h->ecc_bytes,
+			.payload_length = htole32((h->payload_length << 8) |
+				(h->spl_nonsec << SPKG_CODE_NONSEC_BIT) |
+				(h->spl_hyp << SPKG_CODE_HYP_BIT)),
+			.load_address = htole32(h->load_address),
+			.execution_offset = htole32(h->execution_offset),
+		};
+
+		head.crc = crc32((uint8_t *)&head, sizeof(head) - SPKG_HEADER_CRC_SIZE);
+		for (i = 0; i < SPKG_HEADER_COUNT; i++)
+			((struct spkg_hdr *)data)[i] = head;
+	}
+
+	start = data + INDEX_BLP_START;
+
+	/* Fill the SPKG with the Dummy BLp */
+	for (i = 0; i < h->dummy_blp_length; i++)
+		*start++ = 0x88;
+	/* Fill the SPKG with the data from the code file. */
+	length_read =
+	    fread(start, sizeof(char), length_inputfile,
+		  file_input);
+
+	if (length_read != length_inputfile) {
+		fprintf(stderr, "Error reading %s: ferror=%d, feof=%d\n",
+			h->input, ferror(file_input), feof(file_input));
+
+		return -1;
+	}
+	/* fill padding with flash friendly one bits */
+	memset(start + length_inputfile + SPKG_HEADER_CRC_SIZE, 0xff, padding);
+
+	/* Add Payload CRC */
+	crc = crc32(&data[INDEX_BLP_START],
+		    h->dummy_blp_length + length_inputfile + padding);
+
+	start += length_inputfile + padding;
+	start[0] = crc;
+	start[1] = crc >> 8;
+	start[2] = crc >> 16;
+	start[3] = crc >> 24;
+
+	/* Write the completed SKPG to file */
+	length_written = fwrite(data, sizeof(char), length_total, file_SPKG);
+
+	if (length_written != length_total) {
+		fprintf(stderr, "Error writing to %s\n", h->output);
+		return -1;
+	}
+
+	return 0;
+}
+
+const char *usage =
+	"%s\n"
+	"  [-i <filename>]	: input file\n"
+	"  [-o <filename>]	: output file\n"
+	"  [--load_address <hex constant>] : code load address\n"
+	"  [--execution_offset <hex constant>] : starting offset\n"
+	"  [--nand_ecc_enable] : Enable nand ECC\n"
+	"  [--nand_ecc_blksize <hex constant>] : Block size code\n"
+	"        0=256 bytes, 1=512 bytes, 2=1024 bytes\n"
+	"  [--nand_ecc_scheme <hex constant>] : ECC scheme code\n"
+	"        0=BCH2 1=BCH4 2=BCH8 3=BCH16 4=BCH24 5=BCH32\n"
+	"  [--add_dummy_blp] : Add a passthru BLP\n"
+	"  [--spl_nonsec] : Code package run in NONSEC\n"
+	"  [--spl_hyp] : Code package run in HYP (and NONSEC)\n"
+	"  [--padding <value>[K|M]] : Pass SPKG to <value> size block\n"
+	;
+
+static int spkg_parse_option(struct spkg_header *h, const char *name,
+			     const char *sz, uint32_t value)
+{
+//	printf("%s %s=%s %08x\n", __func__, name, sz, value);
+	if (!strcmp("file_input", name) || !strcmp("i", name)) {
+		strncpy(h->input, sz, sizeof(h->input) - 1);
+		h->input[sizeof(h->input) - 1] = 0;
+	} else if (!strcmp("file_output", name) || !strcmp("o", name)) {
+		strncpy(h->output, sz, sizeof(h->output) - 1);
+		h->output[sizeof(h->output) - 1] = 0;
+	} else if (!strcmp("version", name)) {
+		h->version = value;
+	} else if (!strcmp("load_address", name)) {
+		h->load_address = value;
+	} else if (!strcmp("execution_offset", name)) {
+		h->execution_offset = value;
+	} else if (!strcmp("nand_ecc_enable", name)) {
+		h->ecc_enable = value;
+	} else if (!strcmp("nand_ecc_blksize", name)) {
+		h->ecc_block_size = value;
+	} else if (!strcmp("nand_ecc_scheme", name)) {
+		h->ecc_scheme = value;
+	} else if (!strcmp("nand_bytes_per_ecc_block", name)) {
+		h->ecc_bytes = value;
+	} else if (!strcmp("add_dummy_blp", name)) {
+		h->dummy_blp_length = value ? SPKG_BLP_SIZE : 0;
+	} else if (!strcmp("spl_nonsec", name)) {
+		h->spl_nonsec = !!value;
+	} else if (!strcmp("spl_hyp", name)) {
+		h->spl_hyp = !!value;
+	} else if (!strcmp("help", name) || !strcmp("h", name)) {
+		fprintf(stderr, usage, "spkg_utility");
+		exit(0);
+	} else if (!strcmp("padding", name) && sz && value) {
+		if (strchr(sz, 'K'))
+			h->padding = value * 1024;
+		else if (strchr(sz, 'M'))
+			h->padding = value * 1024 * 1024;
+		else
+			h->padding = value;
+	} else {
+		return -1;
+	}
+
+	return 0;
+}
+
+int main(int argc, char *argv[])
+{
+	FILE *file_SPKG = NULL;
+	FILE *file_input = NULL;
+	int result = -1;
+
+	for (int i = 1; i < argc; i++) {
+		unsigned long value = 0;
+		char *name = argv[i];
+		char *sz = NULL;
+
+		if (!name || name[0] != '-') {
+			fprintf(stderr, "%s invalid argument '%s'\n",
+				argv[0], argv[i]);
+			return -1;
+		}
+		name++;
+		if (name[0] == '-')
+			name++;
+
+		if (i < argc - 1 && argv[i + 1][0] != '-')
+			sz = argv[++i];
+
+		if (sz) {
+			if (!sscanf(sz, "0x%lx", &value))
+				sscanf(sz, "%lu", &value);
+		} else {
+			value = 1;
+		}
+		if (spkg_parse_option(&g_header, name, sz, value)) {
+			fprintf(stderr, "%s Error invalid '%s'\n",
+				argv[0],  argv[i]);
+			return -1;
+		}
+	}
+
+	if (!g_header.input[0]) {
+		fprintf(stderr, usage, argv[0]);
+		exit(1);
+	}
+	if (!g_header.output[0])
+		snprintf(g_header.output, sizeof(g_header.output),
+			 "%s.spkg", g_header.input);
+	if (verbose)
+		printf("%s -> %s\n", g_header.input, g_header.output);
+
+	/*NOTE: Using binary mode as this seems necessary if running in Windows */
+	file_SPKG = fopen(g_header.output, "wb");
+	file_input = fopen(g_header.input, "rb");
+
+	if (!file_SPKG)
+		perror(g_header.output);
+	if (!file_input)
+		perror(g_header.input);
+
+	if (file_input && file_SPKG)
+		result = spkg_write(&g_header, file_input, file_SPKG);
+
+	if (file_SPKG)
+		fclose(file_SPKG);
+	if (file_input)
+		fclose(file_input);
+
+	if (result >= 0) {
+		if (verbose)
+			printf("%s created\n", g_header.output);
+	} else {
+		fprintf(stderr, "ERROR creating %s\n", g_header.output);
+	}
+
+	return result;
+}