Re: [U-Boot] [PATCH v5] socfpga: Add socfpga preloader signing to mkimage

Wed, 26 Feb 2014 12:07:49 -0800 

Hi Charles,

On 02/26/2014 01:42 AM, Charles Manning wrote:

On Wednesday 26 February 2014 19:16:37 Michal Simek wrote:

On 02/26/2014 02:17 AM, Charles Manning wrote:

Like many platforms, the Altera socfpga platform requires that the
preloader be "signed" in a certain way or the built-in boot ROM will
not boot the code.

This change automatically creates an appropriately signed preloader
from an SPL image.

The signed image includes a CRC which must, of course, be generated
with a CRC generator that the SoCFPGA boot ROM agrees with otherwise
the boot ROM will reject the image.

Unfortunately the CRC used in this boot ROM is not the same as the
Adler CRC in lib/crc32.c. Indeed the Adler code is not technically a
CRC but is more correctly described as a checksum.

Thus, the appropriate CRC generator is added to lib/ as crc32_alt.c.

Signed-off-by: Charles Manning <cdhmann...@gmail.com>
---

Changes for v3:
  - Fix some coding style issues.
  - Move from a standalone tool to the mkimgae framework.

Changes for v4:
  - Fix more coding style issues.
  - Fix typos in Makefile.
  - Rebase on master (previous version was not on master, but on a
    working socfpga branch).

Changes for v5:
  - Fix more coding style issues.
  - Add some more comments.
  - Remove some unused defines.
  - Move the local CRC32 code into lib/crc32_alt.c.

Note: Building a SOCFPGA preloader will currently not produe a working
image if built in master, but that is due to issues in building SPL,
not in this signer.


  common/image.c       |    1 +
  include/crc32_alt.h  |   17 ++++
  include/image.h      |    1 +
  lib/Makefile         |    1 +
  lib/crc32_alt.c      |   94 +++++++++++++++++
  spl/Makefile         |    5 +
  tools/Makefile       |    2 +
  tools/crc32_alt.c    |    1 +
  tools/imagetool.c    |    2 +
  tools/imagetool.h    |    1 +
  tools/socfpgaimage.c |  276
++++++++++++++++++++++++++++++++++++++++++++++++++ 11 files changed, 401
insertions(+)
  create mode 100644 include/crc32_alt.h
  create mode 100644 lib/crc32_alt.c
  create mode 100644 tools/crc32_alt.c
  create mode 100644 tools/socfpgaimage.c

diff --git a/common/image.c b/common/image.c
index 9c6bec5..e7dc8cc 100644
--- a/common/image.c
+++ b/common/image.c
@@ -135,6 +135,7 @@ static const table_entry_t uimage_type[] = {
        {       IH_TYPE_PBLIMAGE,   "pblimage",   "Freescale PBL Boot Image",},
        {       IH_TYPE_RAMDISK,    "ramdisk",          "RAMDisk Image",    },
        {       IH_TYPE_SCRIPT,     "script",   "Script",           },
+       {       IH_TYPE_SOCFPGAIMAGE,  "socfpgaimage",  "Altera SOCFPGA 
preloader",},
        {       IH_TYPE_STANDALONE, "standalone", "Standalone Program", },
        {       IH_TYPE_UBLIMAGE,   "ublimage",   "Davinci UBL image",},
        {       IH_TYPE_MXSIMAGE,   "mxsimage",   "Freescale MXS Boot Image",},
diff --git a/include/crc32_alt.h b/include/crc32_alt.h
new file mode 100644
index 0000000..813d55d
--- /dev/null
+++ b/include/crc32_alt.h
@@ -0,0 +1,17 @@
+/*
+ * Copyright (C) 2014 Charles Manning <cdhmann...@gmail.com>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ *
+ * Note that the CRC is **not** the zlib/Adler crc32 in crc32.c.
+ * It is the CRC-32 used in bzip2, ethernet and elsewhere.
+ */
+
+#ifndef __CRC32_ALT_H__
+#define __CRC32_ALT_H__
+
+#include <stdint.h>
+
+uint32_t crc32_alt(uint32_t crc, const void *_buf, int length);
+
+#endif
diff --git a/include/image.h b/include/image.h
index 6afd57b..bde31d9 100644
--- a/include/image.h
+++ b/include/image.h
@@ -215,6 +215,7 @@ struct lmb;
  #define IH_TYPE_KERNEL_NOLOAD 14      /* OS Kernel Image, can run from any
load address */ #define IH_TYPE_PBLIMAGE        15      /* Freescale PBL Boot
Image   */
  #define IH_TYPE_MXSIMAGE      16      /* Freescale MXSBoot Image      */
+#define IH_TYPE_SOCFPGAIMAGE   17      /* Altera SOCFPGA Preloader     */

  /*
   * Compression Types
diff --git a/lib/Makefile b/lib/Makefile
index 8c483c9..7ee07a5 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -52,6 +52,7 @@ obj-y += errno.o
  obj-y += display_options.o
  obj-$(CONFIG_BCH) += bch.o
  obj-y += crc32.o
+obj-y += crc32_alt.o
  obj-y += ctype.o
  obj-y += div64.o
  obj-y += hang.o
diff --git a/lib/crc32_alt.c b/lib/crc32_alt.c
new file mode 100644
index 0000000..e0db335
--- /dev/null
+++ b/lib/crc32_alt.c
@@ -0,0 +1,94 @@
+/*
+ * Copyright (C) 2014 Charles Manning <cdhmann...@gmail.com>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ *
+ * Note that the CRC is **not** the zlib/Adler crc32 in crc32.c.
+ * It is the CRC-32 used in bzip2, ethernet and elsewhere.
+ */
+
+#include <crc32_alt.h>
+#include <stdint.h>
+
+static uint32_t crc_table[256] = {
+       0x00000000, 0x04c11db7, 0x09823b6e, 0x0d4326d9,
+       0x130476dc, 0x17c56b6b, 0x1a864db2, 0x1e475005,
+       0x2608edb8, 0x22c9f00f, 0x2f8ad6d6, 0x2b4bcb61,
+       0x350c9b64, 0x31cd86d3, 0x3c8ea00a, 0x384fbdbd,
+       0x4c11db70, 0x48d0c6c7, 0x4593e01e, 0x4152fda9,
+       0x5f15adac, 0x5bd4b01b, 0x569796c2, 0x52568b75,
+       0x6a1936c8, 0x6ed82b7f, 0x639b0da6, 0x675a1011,
+       0x791d4014, 0x7ddc5da3, 0x709f7b7a, 0x745e66cd,
+       0x9823b6e0, 0x9ce2ab57, 0x91a18d8e, 0x95609039,
+       0x8b27c03c, 0x8fe6dd8b, 0x82a5fb52, 0x8664e6e5,
+       0xbe2b5b58, 0xbaea46ef, 0xb7a96036, 0xb3687d81,
+       0xad2f2d84, 0xa9ee3033, 0xa4ad16ea, 0xa06c0b5d,
+       0xd4326d90, 0xd0f37027, 0xddb056fe, 0xd9714b49,
+       0xc7361b4c, 0xc3f706fb, 0xceb42022, 0xca753d95,
+       0xf23a8028, 0xf6fb9d9f, 0xfbb8bb46, 0xff79a6f1,
+       0xe13ef6f4, 0xe5ffeb43, 0xe8bccd9a, 0xec7dd02d,
+       0x34867077, 0x30476dc0, 0x3d044b19, 0x39c556ae,
+       0x278206ab, 0x23431b1c, 0x2e003dc5, 0x2ac12072,
+       0x128e9dcf, 0x164f8078, 0x1b0ca6a1, 0x1fcdbb16,
+       0x018aeb13, 0x054bf6a4, 0x0808d07d, 0x0cc9cdca,
+       0x7897ab07, 0x7c56b6b0, 0x71159069, 0x75d48dde,
+       0x6b93dddb, 0x6f52c06c, 0x6211e6b5, 0x66d0fb02,
+       0x5e9f46bf, 0x5a5e5b08, 0x571d7dd1, 0x53dc6066,
+       0x4d9b3063, 0x495a2dd4, 0x44190b0d, 0x40d816ba,
+       0xaca5c697, 0xa864db20, 0xa527fdf9, 0xa1e6e04e,
+       0xbfa1b04b, 0xbb60adfc, 0xb6238b25, 0xb2e29692,
+       0x8aad2b2f, 0x8e6c3698, 0x832f1041, 0x87ee0df6,
+       0x99a95df3, 0x9d684044, 0x902b669d, 0x94ea7b2a,
+       0xe0b41de7, 0xe4750050, 0xe9362689, 0xedf73b3e,
+       0xf3b06b3b, 0xf771768c, 0xfa325055, 0xfef34de2,
+       0xc6bcf05f, 0xc27dede8, 0xcf3ecb31, 0xcbffd686,
+       0xd5b88683, 0xd1799b34, 0xdc3abded, 0xd8fba05a,
+       0x690ce0ee, 0x6dcdfd59, 0x608edb80, 0x644fc637,
+       0x7a089632, 0x7ec98b85, 0x738aad5c, 0x774bb0eb,
+       0x4f040d56, 0x4bc510e1, 0x46863638, 0x42472b8f,
+       0x5c007b8a, 0x58c1663d, 0x558240e4, 0x51435d53,
+       0x251d3b9e, 0x21dc2629, 0x2c9f00f0, 0x285e1d47,
+       0x36194d42, 0x32d850f5, 0x3f9b762c, 0x3b5a6b9b,
+       0x0315d626, 0x07d4cb91, 0x0a97ed48, 0x0e56f0ff,
+       0x1011a0fa, 0x14d0bd4d, 0x19939b94, 0x1d528623,
+       0xf12f560e, 0xf5ee4bb9, 0xf8ad6d60, 0xfc6c70d7,
+       0xe22b20d2, 0xe6ea3d65, 0xeba91bbc, 0xef68060b,
+       0xd727bbb6, 0xd3e6a601, 0xdea580d8, 0xda649d6f,
+       0xc423cd6a, 0xc0e2d0dd, 0xcda1f604, 0xc960ebb3,
+       0xbd3e8d7e, 0xb9ff90c9, 0xb4bcb610, 0xb07daba7,
+       0xae3afba2, 0xaafbe615, 0xa7b8c0cc, 0xa379dd7b,
+       0x9b3660c6, 0x9ff77d71, 0x92b45ba8, 0x9675461f,
+       0x8832161a, 0x8cf30bad, 0x81b02d74, 0x857130c3,
+       0x5d8a9099, 0x594b8d2e, 0x5408abf7, 0x50c9b640,
+       0x4e8ee645, 0x4a4ffbf2, 0x470cdd2b, 0x43cdc09c,
+       0x7b827d21, 0x7f436096, 0x7200464f, 0x76c15bf8,
+       0x68860bfd, 0x6c47164a, 0x61043093, 0x65c52d24,
+       0x119b4be9, 0x155a565e, 0x18197087, 0x1cd86d30,
+       0x029f3d35, 0x065e2082, 0x0b1d065b, 0x0fdc1bec,
+       0x3793a651, 0x3352bbe6, 0x3e119d3f, 0x3ad08088,
+       0x2497d08d, 0x2056cd3a, 0x2d15ebe3, 0x29d4f654,
+       0xc5a92679, 0xc1683bce, 0xcc2b1d17, 0xc8ea00a0,
+       0xd6ad50a5, 0xd26c4d12, 0xdf2f6bcb, 0xdbee767c,
+       0xe3a1cbc1, 0xe760d676, 0xea23f0af, 0xeee2ed18,
+       0xf0a5bd1d, 0xf464a0aa, 0xf9278673, 0xfde69bc4,
+       0x89b8fd09, 0x8d79e0be, 0x803ac667, 0x84fbdbd0,
+       0x9abc8bd5, 0x9e7d9662, 0x933eb0bb, 0x97ffad0c,
+       0xafb010b1, 0xab710d06, 0xa6322bdf, 0xa2f33668,
+       0xbcb4666d, 0xb8757bda, 0xb5365d03, 0xb1f740b4,
+};
+
+uint32_t crc32_alt(uint32_t crc, const void *_buf, int length)
+{
+       const uint8_t *buf = _buf;
+
+       crc ^= ~0;
+
+       while (length--) {
+               crc = (crc << 8) ^ crc_table[((crc >> 24) ^ *buf) & 0xff];
+               buf++;
+       }
+
+       crc ^= ~0;
+
+       return crc;
+}
diff --git a/spl/Makefile b/spl/Makefile
index bf98024..bce9055 100644
--- a/spl/Makefile
+++ b/spl/Makefile
@@ -181,6 +181,11 @@ $(objtree)/SPL : $(obj)/u-boot-spl.bin

  ALL-y += $(obj)/$(SPL_BIN).bin

+$(OBJTREE)/socfpga-signed-preloader.bin: $(obj)/u-boot-spl.bin
+       $(OBJTREE)/tools/mkimage -T socfpgaimage -d $< $@
+
+ALL-$(CONFIG_SOCFPGA) += $(OBJTREE)/socfpga-signed-preloader.bin
+
  ifdef CONFIG_SAMSUNG
  ALL-y += $(obj)/$(BOARD)-spl.bin
  endif
diff --git a/tools/Makefile b/tools/Makefile
index dcd49f8..46af0b1 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -71,6 +71,7 @@ RSA_OBJS-$(CONFIG_FIT_SIGNATURE) := rsa-sign.o
  dumpimage-mkimage-objs := aisimage.o \
                        $(FIT_SIG_OBJS-y) \
                        crc32.o \
+                       crc32_alt.o \
                        default_image.o \
                        fit_image.o \
                        image-fit.o \
@@ -85,6 +86,7 @@ dumpimage-mkimage-objs := aisimage.o \
                        os_support.o \
                        pblimage.o \
                        sha1.o \
+                       socfpgaimage.o \
                        ublimage.o \
                        $(LIBFDT_OBJS) \
                        $(RSA_OBJS-y)
diff --git a/tools/crc32_alt.c b/tools/crc32_alt.c
new file mode 100644
index 0000000..3faa222
--- /dev/null
+++ b/tools/crc32_alt.c
@@ -0,0 +1 @@
+#include "../lib/crc32_alt.c"
diff --git a/tools/imagetool.c b/tools/imagetool.c
index 29d2189..1ef20b1 100644
--- a/tools/imagetool.c
+++ b/tools/imagetool.c
@@ -45,6 +45,8 @@ void register_image_tool(imagetool_register_t
image_register) init_ubl_image_type();
        /* Init Davinci AIS support */
        init_ais_image_type();
+       /* Init Altera SOCFPGA support */
+       init_socfpga_image_type();
  }

  /*
diff --git a/tools/imagetool.h b/tools/imagetool.h
index c2c9aea..c4833b1 100644
--- a/tools/imagetool.h
+++ b/tools/imagetool.h
@@ -167,6 +167,7 @@ void init_mxs_image_type(void);
  void init_fit_image_type(void);
  void init_ubl_image_type(void);
  void init_omap_image_type(void);
+void init_socfpga_image_type(void);

  void pbl_load_uboot(int fd, struct image_tool_params *mparams);

diff --git a/tools/socfpgaimage.c b/tools/socfpgaimage.c
new file mode 100644
index 0000000..f9df9ae
--- /dev/null
+++ b/tools/socfpgaimage.c
@@ -0,0 +1,276 @@
+/*
+ * Copyright (C) 2014 Charles Manning <cdhmann...@gmail.com>
+ *
+ * SPDX-License-Identifier:    GPL-2.0+
+ *
+ * Reference doc
http://www.altera.com.cn/literature/hb/cyclone-v/cv_5400A.pdf + * Note
this doc is not entirely accurate. Of particular interest to us is the +
* "header" length field being in U32s and not bytes.
+ *
+ * "Header" is a structure of the following format.
+ * this is positioned at 0x40.
+ *
+ * Endian is LSB.
+ *
+ * Offset   Length   Usage
+ * -----------------------
+ *   0x40        4   Validation word 0x31305341
+ *   0x44        1   Version (whatever, zero is fine)
+ *   0x45        1   Flags   (unused, zero is fine)
+ *   0x46        2   Length  (in units of u32, including the end
checksum). + *   0x48        2   Zero
+ *   0x0A        2   Checksum over the heder. NB Not CRC32
+ *
+ * At the end of the code we have a 32-bit CRC checksum over whole
binary + * excluding the CRC.
+ *
+ * Note that the CRC used here is **not** the zlib/Adler crc32. It is
the + * CRC-32 used in bzip2, ethernet and elsewhere.
+ *
+ * The image is padded out to 64k, because that is what is
+ * typically used to write the image to the boot medium.
+ */
+
+#include <crc32_alt.h>
+#include "imagetool.h"
+#include <image.h>
+
+#define HEADER_OFFSET  0x40
+#define HEADER_SIZE    0x0C

just 0xC here


+#define VALIDATION_WORD        0x31305341
+#define PADDED_SIZE    0x10000
+
+static uint8_t buffer[PADDED_SIZE];
+
+/*
+ * The header checksum is just a very simple checksum over
+ * the header area.
+ * There is still a crc32 over the whole lot.
+ */
+static uint16_t hdr_checksum(const uint8_t *buf, int len)
+{
+       uint16_t ret = 0;
+       int i;
+
+       for (i = 0; i < len; i++) {
+               ret += (((uint16_t) *buf) & 0xff);
+               buf++;
+       }
+       return ret;
+}
+
+/*
+ * Some byte marshalling functions...
+ * These load or get little endian values to/from the
+ * buffer.
+ */
+static void load_le16(uint8_t *buf, uint16_t v)
+{
+       buf[0] = (v >> 0) & 0xff;
+       buf[1] = (v >> 8) & 0xff;
+}
+
+static void load_le32(uint8_t *buf, uint32_t v)
+{
+       buf[0] = (v >>  0) & 0xff;
+       buf[1] = (v >>  8) & 0xff;
+       buf[2] = (v >> 16) & 0xff;
+       buf[3] = (v >> 24) & 0xff;
+}
+
+static uint16_t get_le16(const uint8_t *buf)
+{
+       uint16_t retval;
+
+       retval = (((uint16_t) buf[0]) << 0) |
+                (((uint16_t) buf[1]) << 8);
+       return retval;
+}
+
+static uint32_t get_le32(const uint8_t *buf)
+{
+       uint32_t retval;
+
+       retval = (((uint32_t) buf[0]) <<  0) |
+                (((uint32_t) buf[1]) <<  8) |
+                (((uint32_t) buf[2]) << 16) |
+                (((uint32_t) buf[3]) << 24);
+       return retval;
+}
+
+static int align4(int v)
+{
+       return ((v + 3) / 4) * 4;
+}
+
+static void build_header(uint8_t *buf,
+                         uint8_t version,
+                         uint8_t flags,
+                         uint16_t length_bytes)
+{
+       memset(buf, 0, HEADER_SIZE);
+
+       load_le32(buf + 0, VALIDATION_WORD);
+       buf[4] = version;
+       buf[5] = flags;
+       load_le16(buf + 6, length_bytes/4);
+       load_le16(buf + 10, hdr_checksum(buf, 10));
+}
+
+/*
+ * Perform a rudimentary verification of header and return
+ * size of image.
+ */
+static int verify_header(const uint8_t *buf)
+{
+       if (get_le32(buf) != VALIDATION_WORD)
+               return -1;
+       if (get_le16(buf + 10) != hdr_checksum(buf, 10))
+               return -1;
+
+       return get_le16(buf+6) * 4;

buf + 6


+}
+
+/* Sign the buffer and return the signed buffer size */
+static int sign_buffer(uint8_t *buf,
+                       uint8_t version, uint8_t flags,
+                       int len, int pad_64k)
+{
+       uint32_t crcval;
+
+       /* Align the length up */
+       len = align4(len);
+
+       /* Build header, adding 4 bytes to length to hold the CRC32. */
+       build_header(buf + HEADER_OFFSET,  version, flags, len + 4);
+
+       crcval = crc32_alt(0, buf, len);
+
+       load_le32(buf + len, crcval);
+
+       if (!pad_64k)
+               return len + 4;
+
+       return PADDED_SIZE;
+}
+
+/* Verify that the buffer looks sane */
+static int verify_buffer(const uint8_t *buf)
+{
+       int len; /* Including 32bit CRC */
+       uint32_t crccalc;
+
+       len = verify_header(buf + HEADER_OFFSET);
+       if (len < 0)
+               return -1;
+       if (len < HEADER_OFFSET || len > PADDED_SIZE)
+               return -1;
+
+       /* Adjust length, removing CRC */
+       len -= 4;
+
+       crccalc = crc32_alt(0, buf, len);
+
+       if (get_le32(buf + len) != crccalc)
+               return -1;
+
+       return 0;
+}
+
+/* mkimage glue functions */
+static int socfpgaimage_verify_header(unsigned char *ptr, int
image_size, +                   struct image_tool_params *params)
+{
+       if (image_size != PADDED_SIZE)
+               return -1;
+
+       return verify_buffer(ptr);
+}
+
+static void socfpgaimage_print_header(const void *ptr)
+{
+       if (verify_buffer(ptr) == 0)
+               printf("Looks like a sane SOCFPGA preloader\n");
+       else
+               printf("Not a sane SOCFPGA preloader\n");
+}
+
+static int socfpgaimage_check_params(struct image_tool_params *params)
+{
+       /* Not sure if we should be accepting fflags */
+       return  (params->dflag && (params->fflag || params->lflag)) ||
+               (params->fflag && (params->dflag || params->lflag)) ||
+               (params->lflag && (params->dflag || params->fflag));
+}
+
+static int socfpgaimage_check_image_types(uint8_t type)
+{
+       if (type == IH_TYPE_SOCFPGAIMAGE)
+               return EXIT_SUCCESS;
+       else

this else is not needed here.


+               return EXIT_FAILURE;
+}
+
+/*
+ * To work in with the mkimage framework, we do some ugly stuff...
+ *
+ * First,  socfpgaimage_vrec_header() is called.

on space here.


+ * We prepend a fake header big enough to make the file PADDED_SIZE.
+ * This gives us enough space to do what we want later.
+ *
+ * Next, socfpgaimage_set_header() is called.
+ * We fix up the buffer by moving the image to the start of the buffer.
+ * We now have some room to do what we need (add CRC and padding).
+ */
+
+static int data_size;
+#define FAKE_HEADER_SIZE (PADDED_SIZE - data_size)
+
+static int socfpgaimage_vrec_header(struct image_tool_params *params,
+                               struct image_type_params *tparams)
+{
+       struct stat sbuf;
+
+       if (!params->datafile || stat(params->datafile, &sbuf) < 0)
+               return 0;
+
+       data_size = sbuf.st_size;
+       tparams->header_size = FAKE_HEADER_SIZE;
+
+       return 0;

That's quite weird that you are returning 0 for both cases.

The history for that is that the usage of vrec_header by mkimage.c changed.

In the branch where this code was developed, I returned FAKE_HEADER_SIZE, but
mkimage.c was ignoring the returned value.

On master, the returned value is used for the post padding size or something,
but is limited to using a 4k. mkimage.c cannot tolerate return values of more
than 4k and will abort the mkimage

I agree the code does look weird and I will rework that to something like:
Thanks for this. Can you please CC myself and Chin Liang on your next series? 

c...@altera.com
dingu...@altera.com

Thanks,
Dinh


if (...) {
   data_size = ..
   tparams->header_size ...
}
return 0;

Thank you for your feedback, most appreciated.

-- CHarles

_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot


_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to