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

2022-08-12 Thread Ralph Siemsen
On Tue, Aug 9, 2022 at 1:15 PM Sean Anderson  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


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

2022-08-09 Thread Sean Anderson
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  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


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

2022-08-09 Thread Ralph Siemsen
On Tue, Aug 9, 2022 at 12:07 PM Pali Rohár  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


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

2022-08-09 Thread Pali Rohár
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  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.


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

2022-08-09 Thread Ralph Siemsen
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  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


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

2022-08-09 Thread Pali Rohár
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 
> > 
> > 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 
> > Signed-off-by: Ralph Siemsen 
> > ---
> > 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.


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

2022-08-09 Thread Pali Rohár
On Tuesday 09 August 2022 08:59:59 Ralph Siemsen wrote:
> From: Michel Pollet 
> 
> 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 
> Signed-off-by: Ralph Siemsen 
> ---
> 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.


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

2022-08-09 Thread Ralph Siemsen
From: Michel Pollet 

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 
Signed-off-by: Ralph Siemsen 
---
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

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 00..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_STARTSPKG_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_tsignature;
+   uint8_t version;
+   uint8_t ecc;
+   uint8_t ecc_scheme;
+   uint8_t ecc_bytes;
+   uint32_tpayload_length; /* only HIGHER 24 bits */
+   uint32_tload_address;
+   uint32_texecution_offset;
+   uint32_tcrc; /* 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 00..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 
+#include 
+#include 
+#include 
+#include 
+
+#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;