From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 1E0CEC19F2D for ; Sat, 13 Aug 2022 14:47:38 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 5E342843FB; Sat, 13 Aug 2022 16:47:36 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="maup8Clr"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id BFCAA844AF; Sat, 13 Aug 2022 16:47:33 +0200 (CEST) Received: from mail-qv1-xf36.google.com (mail-qv1-xf36.google.com [IPv6:2607:f8b0:4864:20::f36]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 302CE84377 for ; Sat, 13 Aug 2022 16:47:30 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=seanga2@gmail.com Received: by mail-qv1-xf36.google.com with SMTP id l8so2563687qvr.5 for ; Sat, 13 Aug 2022 07:47:30 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject:from:to:cc; bh=nyj8dxWdRen0vGrECqFm02gg6GK4ElDKMIMgA80I8F4=; b=maup8ClrJrzbntdZ9xBMwUz49iktTVUBSlFNmuoy8DORd3/3QwZcNYp9qci85f5p1X AYexQ3DxX2t8eg74CX91+oXLHW5hBHghv85ByGkFtGE54VLt1Zlv6SB5TVHEFrLeshMp d1ut5g3n+Jpu2mJZeepNSKPYUzUuaEkD/fvhIpBP8wAL3Xmb6BnCfGHJsW2xP7qSPvaj CCLvZimEXLRam9Db4rRTxu/IczrXVnfgx7sr4scKS9IJvThIxnMB1Su9Lax9I1/he2rZ Gv5Avhsp8/3YboS1KhcXZMx47/Ve1RNU+kv229r2rEmVxx4+Q4693xAQ+Zg2w8N4Q5Y9 amEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:cc:to:subject :x-gm-message-state:from:to:cc; bh=nyj8dxWdRen0vGrECqFm02gg6GK4ElDKMIMgA80I8F4=; b=oAsu5Byh+xR8xnjoZntU2Wqp+ryjj8zMgghUrrvHov8CzihQeLsjqgsDLE+qeI/WGf jOZ1WC5jNlTUbnwzI75lLCPOgz5IPWilDLrkSOFMxkEMJtglbowV/H/j68phB5CcAC93 4fuETk+JX40AcUGt6/Bg3boNGOzi59N0vCet0BbKJA1g+dOp1L3brx6Zjgku4yV+u2gY yMSqCfLrx2ZBS2guwoMRR4AK+iKuFUg5uuK8VH4cc8pHpWIv54HttK9ohH9vMZY+Nrvp lvJc0FxjoVMZtWdEvON6Ep7bL3+0TLSrw5HNRXPqcX6PphwHXbDQS/TXPOWNc822cx7l uuNw== X-Gm-Message-State: ACgBeo2oOz6ughoexzxrtspyjs/G/OrJaiV+OQN/KewbbmWkbZZF3tkY FlN2dj8tLtJKEO3dCKEThnc= X-Google-Smtp-Source: AA6agR76vdahXaI4BUC0lEfFYxa1gZte2e0Wf4f4uhloWu+XfiCcvOy7kNNlbi2qsXhlhBs5IvI5Gg== X-Received: by 2002:a05:6214:2268:b0:476:c04e:65f6 with SMTP id gs8-20020a056214226800b00476c04e65f6mr7545771qvb.21.1660402048833; Sat, 13 Aug 2022 07:47:28 -0700 (PDT) Received: from [192.168.1.201] (pool-173-73-95-180.washdc.fios.verizon.net. [173.73.95.180]) by smtp.gmail.com with ESMTPSA id u6-20020a05622a14c600b0031f0b43629dsm4335157qtx.23.2022.08.13.07.47.27 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Sat, 13 Aug 2022 07:47:28 -0700 (PDT) Subject: Re: [RFC PATCH v2 9/9] tools: spkgimage: add Renesas SPKG format To: Ralph Siemsen Cc: andre.przywara@arm.com, heiko.thiery@gmail.com, pali@kernel.org, samuel@sholland.org, sjg@chromium.org, sr@denx.de, takahiro.akashi@linaro.org, u-boot@lists.denx.de, sean.anderson@seco.com References: <20220809125959.217333-10-ralph.siemsen@linaro.org> <20220812170325.485707-1-ralph.siemsen@linaro.org> From: Sean Anderson Message-ID: <7e4a3833-bc51-9b2c-aa0a-4af017e1fb1b@gmail.com> Date: Sat, 13 Aug 2022 10:47:27 -0400 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0 MIME-Version: 1.0 In-Reply-To: <20220812170325.485707-1-ralph.siemsen@linaro.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean 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 > --- > > 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 > +#include > +#include > +#include > +#include > +#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