[PATCH v5 09/10] tools: spkgimage: add Renesas SPKG format
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/rzn1-snarc/spkgimage.cfg \
-T spkgimage -a 0x2004 -e 0x2004 \
-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
Reviewed-by: Simon Glass
Reviewed-by: Marek Vasut
---
(no changes since v5)
Changes in v5:
- use strcspn() instead of open-coded loop for \n removal
- rename source files to include vendor name
- replace static globals with dynamically allocated structure
- update print_header function signature
Changes in v4:
- added tags
- add RZ/N1 board documentation
- added binman support
Changes in v3:
- provide definition of __packed (as done in kwbimage.h)
- explain why a local copy of roundup() is needed
- document spkgimage in doc/mkimage.1
- add range checks when parsing config file values
- add line numbers for reporting errors in config file
- rename SPKG_HEADER_SIGNATURE to SPKG_HEADER_MARKER
- fix segfault when image is padded by less than 4 bytes
- minor style and typo fixes
Changes in v2:
- rewrote the stand-alone spkg_utility to integrate into mkimage
board/schneider/rzn1-snarc/spkgimage.cfg | 26 ++
boot/image.c | 1 +
doc/mkimage.1| 45 +++
include/image.h | 1 +
tools/Makefile | 1 +
tools/renesas_spkgimage.c| 338 +++
tools/renesas_spkgimage.h| 87 ++
7 files changed, 499 insertions(+)
create mode 100644 board/schneider/rzn1-snarc/spkgimage.cfg
create mode 100644 tools/renesas_spkgimage.c
create mode 100644 tools/renesas_spkgimage.h
diff --git a/board/schneider/rzn1-snarc/spkgimage.cfg
b/board/schneider/rzn1-snarc/spkgimage.cfg
new file mode 100644
index 00..b5faf96b00
--- /dev/null
+++ b/board/schneider/rzn1-snarc/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
+VERSION1
+
+# b[42:41] ECC Block size: 0=256 bytes, 1=512 bytes, 2=1024 bytes
+NAND_ECC_BLOCK_SIZE1
+
+# b[45] NAND enable (boolean)
+NAND_ECC_ENABLE1
+
+# b[50:48] ECC Scheme: 0=BCH2 1=BCH4 2=BCH8 3=BCH16 4=BCH24 5=BCH32
+NAND_ECC_SCHEME3
+
+# 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
+PADDING64K
diff --git a/boot/image.c b/boot/image.c
index 958dbf8534..5c4f9b807d 100644
--- a/boot/image.c
+++ b/boot/image.c
@@ -181,6 +181,7 @@ static const table_entry_t uimage_type[] = {
{ IH_TYPE_SUNXI_EGON, "sunxi_egon", "Allwinner eGON Boot Image"
},
{ IH_TYPE_SUNXI_TOC0, "sunxi_toc0", "Allwinner TOC0 Boot Image"
},
{ IH_TYPE_FDT_LEGACY, "fdt_legacy", "legacy Image with Flat
Device Tree ", },
+ { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" },
{ -1, "", "", },
};
diff --git a/doc/mkimage.1 b/doc/mkimage.1
index d8727ec73c..76c7859bb0 100644
--- a/doc/mkimage.1
+++ b/doc/mkimage.1
@@ -662,6 +662,51 @@ rk3568
.TE
.RE
.
+.SS spkgimage
+The primary configuration file consists of lines containing key/value pairs
+delimited by whitespace. An example follows.
+.PP
+.RS
+.EX
+# Comments and blank lines may be used
+.I key1 value1
+.I key2 value2
+.EE
+.RE
+.P
+The supported
+.I key
+types are as follows.
+.TP
+.B VERSION
+.TQ
+.B NAND_ECC_BLOCK_SIZE
+.TQ
+.B NAND_ECC_ENABLE
+.TQ
+.B NAND_ECC_SCHEME
+.TQ
+.B NAND_BYTES_PER_ECC_BLOCK
+These all take a positive integer value as their argument.
+The value will be copied directly into the respective field
+of the SPKG header structure. For details on these values,
+refer to Section 7.4 of the Renesas RZ/N1 User's Manual.
+.
+.TP
+.B ADD_DUMMY_BLP
+Takes a numeric argument, which is treated as a boolean. Any nonzero
+value will cause a fake BLp security header to be included in the SPKG
+output.
+.
+.TP
+.B PADDING
+Takes a positive integer value, with an optional
+.B K
+or
+.B M
+suffix, indicating KiB / MiB respectively.
+The output SPKG file will be padded to a multiple of this value.
+.
.SS sunxi_egon
The primary configuration is the name to use for the device tree.
.
diff --git a/include/image.h b/include/image.h
index 456197d6fd..01a6787d21 100644
--- a/include/image.h
+++ b/include/image.h
@@ -230,6 +230,7 @@ enum image_type_t {
IH_TYPE_SUNXI_EGON, /* Allwinner eGON Boot Image */
Re: [PATCH v5 09/10] tools: spkgimage: add Renesas SPKG format
On Tue, May 09, 2023 at 11:42:30AM -0400, Ralph Siemsen wrote: On Tue, May 09, 2023 at 04:52:45PM +0200, Marek Vasut wrote: Do we have some sort of global (?) state structure which exists during the whole work cycle of the tool ? If so, add a link list into there. There is struct image_tool_params which is passed to the callbacks and holds most of the state. And in fact it is a global, but declared static in mkimage.c, without any accessor function. If we really want to worry about the lifecycle of these dynamic allocations, then we'd probably need to add some kind of "cleanup" method to the API, and call it right before exiting from main(). I can do an example implementation, but there are 9 other image formats already in the tree, which also do dynamic allocation. I don't want to start touching each one of those, or I'll never get this RZ/N1 in... I'll keep the current logic (which leaks) and send a separate RFC patch to discuss a strategy for fixing this in all the tools/*image.c drivers. Ralph
Re: [PATCH v5 09/10] tools: spkgimage: add Renesas SPKG format
On Tue, May 09, 2023 at 04:52:45PM +0200, Marek Vasut wrote: Do we have some sort of global (?) state structure which exists during the whole work cycle of the tool ? If so, add a link list into there. There is struct image_tool_params which is passed to the callbacks and holds most of the state. And in fact it is a global, but declared static in mkimage.c, without any accessor function. If we really want to worry about the lifecycle of these dynamic allocations, then we'd probably need to add some kind of "cleanup" method to the API, and call it right before exiting from main(). I can do an example implementation, but there are 9 other image formats already in the tree, which also do dynamic allocation. I don't want to start touching each one of those, or I'll never get this RZ/N1 in... Ralph
Re: [PATCH v5 09/10] tools: spkgimage: add Renesas SPKG format
On 5/9/23 15:07, Ralph Siemsen wrote: On Tue, May 09, 2023 at 04:25:06AM +0200, Marek Vasut wrote: The usual fail path handling like: " if (there is an error) goto exit; ... exit: free(data); return ret; " does not work here ? Yes, this would handle de-allocation in the failing case. However in the normal case (no error), there is no corresponding call to free(). And there is no good place to put such a call, given the API of the callbacks. It would be possible call free() from print_header, however this is brittle since it relies on mkimage core calling print_header as the last step (and only once). Do we have some sort of global (?) state structure which exists during the whole work cycle of the tool ? If so, add a link list into there.
Re: [PATCH v5 09/10] tools: spkgimage: add Renesas SPKG format
On Tue, May 09, 2023 at 04:25:06AM +0200, Marek Vasut wrote: The usual fail path handling like: " if (there is an error) goto exit; ... exit: free(data); return ret; " does not work here ? Yes, this would handle de-allocation in the failing case. However in the normal case (no error), there is no corresponding call to free(). And there is no good place to put such a call, given the API of the callbacks. It would be possible call free() from print_header, however this is brittle since it relies on mkimage core calling print_header as the last step (and only once). Ralph
Re: [PATCH v5 09/10] tools: spkgimage: add Renesas SPKG format
On 5/8/23 19:50, Ralph Siemsen wrote:
On Sun, May 07, 2023 at 06:08:33PM +0200, Marek Vasut wrote:
[...]
+static int spkgimage_parse_config_file(char *filename)
+{
+ FILE *fcfg;
+ char line[256];
+ size_t line_num = 0;
+
+ fcfg = fopen(filename, "r");
+ if (!fcfg)
+ return -EINVAL;
+
+ conf = calloc(1, sizeof(struct config_file));
+ if (!conf)
+ return -ENOMEM;
+
+ while (fgets(line, sizeof(line), fcfg)) {
+ line_num += 1;
+
+ /* Skip blank lines and comments */
+ if (line[0] == '\n' || line[0] == '#')
+ continue;
+
+ /* Strip any trailing newline */
+ line[strcspn(line, "\n")] = 0;
+
+ /* Parse the line */
+ if (spkgimage_parse_config_line(line, line_num))
+ return -EINVAL;
Wouldn't this return -EINVAL; leak memory allocated by the calloc()
above?
You are correct. But note that in the normal (non-error) code path, the
structure remains allocated as well, and there is no good place to
free() it, given the available callbacks in struct image_type_params.
So I am relying on the OS to free all memory upon program exit, both in
the error and non-error case. I would think this is reasonable for a
small one-shot utility program, keeps things simple.
If this is not acceptable, I can rework it, but there are quite a few
other spots which would also need to free resources before bailing out.
The usual fail path handling like:
"
if (there is an error)
goto exit;
...
exit:
free(data);
return ret;
"
does not work here ?
Re: [PATCH v5 09/10] tools: spkgimage: add Renesas SPKG format
On Sun, May 07, 2023 at 06:08:33PM +0200, Marek Vasut wrote:
[...]
+static int spkgimage_parse_config_file(char *filename)
+{
+ FILE *fcfg;
+ char line[256];
+ size_t line_num = 0;
+
+ fcfg = fopen(filename, "r");
+ if (!fcfg)
+ return -EINVAL;
+
+ conf = calloc(1, sizeof(struct config_file));
+ if (!conf)
+ return -ENOMEM;
+
+ while (fgets(line, sizeof(line), fcfg)) {
+ line_num += 1;
+
+ /* Skip blank lines and comments */
+ if (line[0] == '\n' || line[0] == '#')
+ continue;
+
+ /* Strip any trailing newline */
+ line[strcspn(line, "\n")] = 0;
+
+ /* Parse the line */
+ if (spkgimage_parse_config_line(line, line_num))
+ return -EINVAL;
Wouldn't this return -EINVAL; leak memory allocated by the calloc() above?
You are correct. But note that in the normal (non-error) code path, the
structure remains allocated as well, and there is no good place to
free() it, given the available callbacks in struct image_type_params.
So I am relying on the OS to free all memory upon program exit, both in
the error and non-error case. I would think this is reasonable for a
small one-shot utility program, keeps things simple.
If this is not acceptable, I can rework it, but there are quite a few
other spots which would also need to free resources before bailing out.
[...]
With that fixed:
Reviewed-by: Marek Vasut
I'll wait to hear back from you before applying this tag.
Regards,
Ralph
Re: [PATCH v5 09/10] tools: spkgimage: add Renesas SPKG format
On 4/24/23 03:15, 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/rzn1-snarc/spkgimage.cfg \
-T spkgimage -a 0x2004 -e 0x2004 \
-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
Reviewed-by: Simon Glass
squash! tools: spkgimage: add Renesas SPKG format
[...]
+static int spkgimage_parse_config_file(char *filename)
+{
+ FILE *fcfg;
+ char line[256];
+ size_t line_num = 0;
+
+ fcfg = fopen(filename, "r");
+ if (!fcfg)
+ return -EINVAL;
+
+ conf = calloc(1, sizeof(struct config_file));
+ if (!conf)
+ return -ENOMEM;
+
+ while (fgets(line, sizeof(line), fcfg)) {
+ line_num += 1;
+
+ /* Skip blank lines and comments */
+ if (line[0] == '\n' || line[0] == '#')
+ continue;
+
+ /* Strip any trailing newline */
+ line[strcspn(line, "\n")] = 0;
+
+ /* Parse the line */
+ if (spkgimage_parse_config_line(line, line_num))
+ return -EINVAL;
Wouldn't this return -EINVAL; leak memory allocated by the calloc() above?
[...]
With that fixed:
Reviewed-by: Marek Vasut
[PATCH v5 09/10] tools: spkgimage: add Renesas SPKG format
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/rzn1-snarc/spkgimage.cfg \
-T spkgimage -a 0x2004 -e 0x2004 \
-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
Reviewed-by: Simon Glass
squash! tools: spkgimage: add Renesas SPKG format
---
Changes in v5:
- use strcspn() instead of open-coded loop for \n removal
- rename source files to include vendor name
- replace static globals with dynamically allocated structure
- update print_header function signature
Changes in v4:
- added tags
- add RZ/N1 board documentation
- added binman support
Changes in v3:
- provide definition of __packed (as done in kwbimage.h)
- explain why a local copy of roundup() is needed
- document spkgimage in doc/mkimage.1
- add range checks when parsing config file values
- add line numbers for reporting errors in config file
- rename SPKG_HEADER_SIGNATURE to SPKG_HEADER_MARKER
- fix segfault when image is padded by less than 4 bytes
- minor style and typo fixes
Changes in v2:
- rewrote the stand-alone spkg_utility to integrate into mkimage
board/schneider/rzn1-snarc/spkgimage.cfg | 26 ++
boot/image.c | 1 +
doc/mkimage.1| 45 +++
include/image.h | 1 +
tools/Makefile | 1 +
tools/renesas_spkgimage.c| 338 +++
tools/renesas_spkgimage.h| 87 ++
7 files changed, 499 insertions(+)
create mode 100644 board/schneider/rzn1-snarc/spkgimage.cfg
create mode 100644 tools/renesas_spkgimage.c
create mode 100644 tools/renesas_spkgimage.h
diff --git a/board/schneider/rzn1-snarc/spkgimage.cfg
b/board/schneider/rzn1-snarc/spkgimage.cfg
new file mode 100644
index 00..b5faf96b00
--- /dev/null
+++ b/board/schneider/rzn1-snarc/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
+VERSION1
+
+# b[42:41] ECC Block size: 0=256 bytes, 1=512 bytes, 2=1024 bytes
+NAND_ECC_BLOCK_SIZE1
+
+# b[45] NAND enable (boolean)
+NAND_ECC_ENABLE1
+
+# b[50:48] ECC Scheme: 0=BCH2 1=BCH4 2=BCH8 3=BCH16 4=BCH24 5=BCH32
+NAND_ECC_SCHEME3
+
+# 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
+PADDING64K
diff --git a/boot/image.c b/boot/image.c
index 958dbf8534..5c4f9b807d 100644
--- a/boot/image.c
+++ b/boot/image.c
@@ -181,6 +181,7 @@ static const table_entry_t uimage_type[] = {
{ IH_TYPE_SUNXI_EGON, "sunxi_egon", "Allwinner eGON Boot Image"
},
{ IH_TYPE_SUNXI_TOC0, "sunxi_toc0", "Allwinner TOC0 Boot Image"
},
{ IH_TYPE_FDT_LEGACY, "fdt_legacy", "legacy Image with Flat
Device Tree ", },
+ { IH_TYPE_RENESAS_SPKG, "spkgimage", "Renesas SPKG Image" },
{ -1, "", "", },
};
diff --git a/doc/mkimage.1 b/doc/mkimage.1
index d8727ec73c..76c7859bb0 100644
--- a/doc/mkimage.1
+++ b/doc/mkimage.1
@@ -662,6 +662,51 @@ rk3568
.TE
.RE
.
+.SS spkgimage
+The primary configuration file consists of lines containing key/value pairs
+delimited by whitespace. An example follows.
+.PP
+.RS
+.EX
+# Comments and blank lines may be used
+.I key1 value1
+.I key2 value2
+.EE
+.RE
+.P
+The supported
+.I key
+types are as follows.
+.TP
+.B VERSION
+.TQ
+.B NAND_ECC_BLOCK_SIZE
+.TQ
+.B NAND_ECC_ENABLE
+.TQ
+.B NAND_ECC_SCHEME
+.TQ
+.B NAND_BYTES_PER_ECC_BLOCK
+These all take a positive integer value as their argument.
+The value will be copied directly into the respective field
+of the SPKG header structure. For details on these values,
+refer to Section 7.4 of the Renesas RZ/N1 User's Manual.
+.
+.TP
+.B ADD_DUMMY_BLP
+Takes a numeric argument, which is treated as a boolean. Any nonzero
+value will cause a fake BLp security header to be included in the SPKG
+output.
+.
+.TP
+.B PADDING
+Takes a positive integer value, with an optional
+.B K
+or
+.B M
+suffix, indicating KiB / MiB respectively.
+The output SPKG file will be padded to a multiple of this value.
+.
.SS sunxi_egon
The primary configuration is the name to use for the device tree.
.
diff --git a/include/image.h b/include/image.h
index 456197d6fd..01a6787d21 100644
--- a/include/image.h
+++ b/include/image.h
@@ -230,6 +230,7 @@ enum image_type_t {
IH_TYPE_SUNXI_EGON, /* Allwinner eGON Boot Image */
