Re: [U-Boot] [PATCH v2] xilinx: zynq: Add support to secure images

On 7.6.2018 09:28, Siva Durga Prasad Paladugu wrote:

This patch basically adds two new commands for loadig secure images/bitstreams.

1. zynq rsa adds support to load secure image which can be both authenticated or encrypted or both authenticated and encrypted image in xilinx bootimage(BOOT.bin) format.
2. zynq aes command adds support to decrypted and load encrypted image either back to DDR or it can load an encrypted bitsream to PL directly by decrypting it. The image has to be encrypted using xilinx bootgen tool and to get only the encrypted image from tool use -split option while invoking bootgen.

Signed-off-by: Siva Durga Prasad Paladugu siva.durga.paladugu@xilinx.com

Changes from v1:

• Defined two config synbols for RSA and AES separately and used them wherever required.
• Used U_BOOT_CMD_KENT as per comment
• Cleared DEVCFG_CTRL_PCAP_RATE_EN_MASK once decryption is done.

Changes from RFC:

• Moved zynqaes to board/xilinx/zynq/cmds.c and renamed as "zynq aes".
• Moved boot image parsing code to a separate file.
• Squashed in to a single patch.
• Fixed coding style comments.

---

arch/arm/Kconfig | 1 + board/xilinx/zynq/Kconfig | 32 +++ board/xilinx/zynq/Makefile | 5 + board/xilinx/zynq/bootimg.c | 143 +++++++++++ board/xilinx/zynq/cmds.c | 556 +++++++++++++++++++++++++++++++++++++++++++ drivers/fpga/zynqpl.c | 67 ++++++ include/u-boot/rsa-mod-exp.h | 4 + include/zynq_bootimg.h | 33 +++ include/zynqpl.h | 5 + lib/rsa/rsa-mod-exp.c | 52 ++++ 10 files changed, 898 insertions(+) create mode 100644 board/xilinx/zynq/Kconfig create mode 100644 board/xilinx/zynq/bootimg.c create mode 100644 board/xilinx/zynq/cmds.c create mode 100644 include/zynq_bootimg.h

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 3e05f79..e78e1a4 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -1428,6 +1428,7 @@ source "board/toradex/colibri_pxa270/Kconfig" source "board/vscom/baltos/Kconfig" source "board/woodburn/Kconfig" source "board/work-microwave/work_92105/Kconfig" +source "board/xilinx/zynq/Kconfig" source "board/xilinx/zynqmp/Kconfig" source "board/zipitz2/Kconfig"

diff --git a/board/xilinx/zynq/Kconfig b/board/xilinx/zynq/Kconfig new file mode 100644 index 0000000..e665c8d --- /dev/null +++ b/board/xilinx/zynq/Kconfig @@ -0,0 +1,32 @@ +# Copyright (c) 2018, Xilinx, Inc. +# +# SPDX-License-Identifier: GPL-2.0

+if ARCH_ZYNQ

+config CMD_ZYNQ

• bool "Enable Zynq specific commands"
• default y
• help

•  Enables Zynq specific commands.
  

+config CMD_ZYNQ_AES

• bool "Enable zynq aes command for decryption of encrypted images"
• depends on CMD_ZYNQ
• help

•  Decrypts the encrypted image present in source address
  

•  and places the decrypted image at destination address.
  

+config CMD_ZYNQ_RSA

• bool "Enable zynq rsa command for loading secure images"
• default y
• depends on CMD_ZYNQ
• select CMD_ZYNQ_AES
• help

•  Enabling this will support zynq secure image verification.
  

•  The secure image is a xilinx specific BOOT.BIN with
  

•  either authentication or encryption or both encryption
  

•  and authentication feature enabled while generating
  

•  BOOT.BIN using Xilinx bootgen tool.
  

+endif diff --git a/board/xilinx/zynq/Makefile b/board/xilinx/zynq/Makefile index 5a76a26..f4996fa 100644 --- a/board/xilinx/zynq/Makefile +++ b/board/xilinx/zynq/Makefile @@ -18,6 +18,11 @@ $(warning Put custom ps7_init_gpl.c/h to board/xilinx/zynq/custom_hw_platform/)) endif endif

+ifndef CONFIG_SPL_BUILD +obj-$(CONFIG_CMD_ZYNQ) += cmds.o +obj-$(CONFIG_CMD_ZYNQ_RSA) += bootimg.o +endif

obj-$(CONFIG_SPL_BUILD) += $(init-objs)

# Suppress "warning: function declaration isn't a prototype" diff --git a/board/xilinx/zynq/bootimg.c b/board/xilinx/zynq/bootimg.c new file mode 100644 index 0000000..b069e2b --- /dev/null +++ b/board/xilinx/zynq/bootimg.c @@ -0,0 +1,143 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2018 Xilinx, Inc.
• */

+#include <common.h> +#include <asm/io.h> +#include <asm/arch/hardware.h> +#include <asm/arch/sys_proto.h> +#include <u-boot/md5.h> +#include <zynq_bootimg.h>

+DECLARE_GLOBAL_DATA_PTR;

+#define ZYNQ_IMAGE_PHDR_OFFSET 0x09C +#define ZYNQ_IMAGE_FSBL_LEN_OFFSET 0x040 +#define ZYNQ_PART_HDR_CHKSUM_WORD_COUNT 0x0F +#define ZYNQ_PART_HDR_WORD_COUNT 0x10 +#define ZYNQ_MAXIMUM_IMAGE_WORD_LEN 0x40000000 +#define MD5_CHECKSUM_SIZE 16

+struct headerarray {

• u32 fields[16];

+};

+/*

• • Check whether the given partition is last partition or not
• */

+static int zynq_islastpartition(struct headerarray *head) +{

• int index;
• debug("%s\n", __func__);
• if (head->fields[ZYNQ_PART_HDR_CHKSUM_WORD_COUNT] != 0xFFFFFFFF)

• return -1;
  

• for (index = 0; index < ZYNQ_PART_HDR_WORD_COUNT - 1; index++) {

• if (head->fields[index] != 0x0)
  

• 	return -1;
  

• }
• return 0;

+}

+/*

• • Get the partition count from the partition header
• */

+int zynq_get_part_count(struct partition_hdr *part_hdr_info) +{

• u32 count = 0;

no need to init it to 0; it is done 3 lines below already.

• struct headerarray *hap;
• debug("%s\n", __func__);
• for (count = 0; count < ZYNQ_MAX_PARTITION_NUMBER; count++) {

• hap = (struct headerarray *)&part_hdr_info[count];
  

• if (zynq_islastpartition(hap) != -1)
  

• 	break;
  

• }
• return count;

+}

+/*

• • Get the partition info of all the partitions available.
• */

+int zynq_get_partition_info(u32 image_base_addr, u32 *fsbl_len,

• 	    struct partition_hdr *part_hdr)
  

+{

• u32 parthdroffset;
• *fsbl_len = *((u32 *)(image_base_addr + ZYNQ_IMAGE_FSBL_LEN_OFFSET));
• parthdroffset = *((u32 *)(image_base_addr + ZYNQ_IMAGE_PHDR_OFFSET));
• parthdroffset += image_base_addr;
• memcpy(part_hdr, (u32 *)parthdroffset,

•       (sizeof(struct partition_hdr) * ZYNQ_MAX_PARTITION_NUMBER));
  

• return 0;

+}

+/*

• • Check whether the partition header is valid or not
• */

+int zynq_validate_hdr(struct partition_hdr *header) +{

• struct headerarray *hap;
• u32 index;
• u32 checksum;
• debug("%s\n", __func__);
• hap = (struct headerarray *)header;
• for (index = 0; index < ZYNQ_PART_HDR_WORD_COUNT; index++) {

• if (hap->fields[index] != 0x0)
  

if (hap->fields[index]) is enough.

• 	break;
  

• }
• if (index == ZYNQ_PART_HDR_WORD_COUNT)

^^ double space.

• return -1;
  

• checksum = 0;
• for (index = 0; index < ZYNQ_PART_HDR_CHKSUM_WORD_COUNT; index++)

• checksum += hap->fields[index];
  

• checksum ^= 0xFFFFFFFF;
• if (hap->fields[ZYNQ_PART_HDR_CHKSUM_WORD_COUNT] != checksum) {

• printf("Error: Checksum 0x%8.8x != 0x%8.8x\r\n",
  

Please use just \n here - fix in the whole patch.

•        checksum, hap->fields[ZYNQ_PART_HDR_CHKSUM_WORD_COUNT]);
  

• return -1;
  

• }
• if (header->imagewordlen > ZYNQ_MAXIMUM_IMAGE_WORD_LEN) {

• printf("INVALID_PARTITION_LENGTH\r\n");
  

• return -1;
  

• }
• return 0;

+}

+/*

• • Validate the partition by calculationg the md5 checksum for the
• • partition and compare with checksum present in checksum offset of
• • partition
• */

+int zynq_validate_partition(u32 start_addr, u32 len, u32 chksum_off) +{

• u8 checksum[MD5_CHECKSUM_SIZE];
• u8 calchecksum[MD5_CHECKSUM_SIZE];
• memcpy(&checksum[0], (u32 *)chksum_off, MD5_CHECKSUM_SIZE);
• md5_wd((u8 *)start_addr, len, &calchecksum[0], 0x10000);
• if ((memcmp(checksum, calchecksum, MD5_CHECKSUM_SIZE)) != 0) {

this is easier and shorter if (!memcmp...) return 0;

printf("Error: Partition DataChecksum\n"); return -1;

• printf("Error: Partition DataChecksum \r\n");
  

don't forget to remove that space before \n.

• return -1;
  

• }
• return 0;

+} diff --git a/board/xilinx/zynq/cmds.c b/board/xilinx/zynq/cmds.c new file mode 100644 index 0000000..f776031 --- /dev/null +++ b/board/xilinx/zynq/cmds.c @@ -0,0 +1,556 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*

• • Copyright (C) 2018 Xilinx, Inc.
• */

+#include <common.h> +#include <asm/io.h> +#include <asm/arch/hardware.h> +#include <asm/arch/sys_proto.h> +#include <u-boot/md5.h> +#include <u-boot/rsa.h> +#include <u-boot/rsa-mod-exp.h> +#include <u-boot/sha256.h> +#include <spi_flash.h>

this is quite suspicios header here. when this is removed there is missing only malloc which

+#include <zynqpl.h> +#include <fpga.h> +#include <zynq_bootimg.h>

+DECLARE_GLOBAL_DATA_PTR;

+#ifdef CONFIG_CMD_ZYNQ_RSA

empty line here.

+#define ZYNQ_EFUSE_RSA_ENABLE_MASK 0x400 +#define ZYNQ_ATTRIBUTE_PL_IMAGE_MASK 0x20 +#define ZYNQ_ATTRIBUTE_CHECKSUM_TYPE_MASK 0x7000 +#define ZYNQ_ATTRIBUTE_RSA_PRESENT_MASK 0x8000 +#define ZYNQ_ATTRIBUTE_RSA_PART_OWNER_MASK 0x30000

+#define ZYNQ_RSA_MODULAR_SIZE 256 +#define ZYNQ_RSA_MODULAR_EXT_SIZE 256 +#define ZYNQ_RSA_EXPO_SIZE 64 +#define ZYNQ_RSA_SPK_SIGNATURE_SIZE 256 +#define ZYNQ_RSA_PARTITION_SIGNATURE_SIZE 256 +#define ZYNQ_RSA_SIGNATURE_SIZE 0x6C0 +#define ZYNQ_RSA_HEADER_SIZE 4 +#define ZYNQ_RSA_MAGIC_WORD_SIZE 60 +#define ZYNQ_RSA_PART_OWNER_UBOOT 1 +#define ZYNQ_RSA_ALIGN_PPK_START 64

+#define WORD_LENGTH_SHIFT 2

+static u8 *ppkmodular; +static u8 *ppkmodularex; +static u32 ppkexp;

+struct zynq_rsa_public_key {

• uint len; /* Length of modulus[] in number of u32 */
• u32 n0inv; /* -1 / modulus[0] mod 2^32 */
• u32 *modulus; /* modulus as little endian array */
• u32 *rr; /* R^2 as little endian array */

+};

+struct zynq_rsa_public_key public_key;

+struct partition_hdr part_hdr[ZYNQ_MAX_PARTITION_NUMBER];

these two should be static.

+/*

• • Extract the primary public key components from already autheticated FSBL
• */

+static void zynq_extract_ppk(u32 fsbl_len) +{

• u32 padsize;
• u8 *ppkptr;
• debug("%s\n", __func__);
• /*

• * Extract the authenticated PPK from OCM i.e at end of the FSBL
  

• */
  

• ppkptr = (u8 *)(fsbl_len + 0xFFFC0000);

macro for this ocm start (in mach-zynq/.. hardware.h).

• padsize = ((u32)ppkptr % ZYNQ_RSA_ALIGN_PPK_START);
• if (padsize != 0)

if (padsize)

• ppkptr += (ZYNQ_RSA_ALIGN_PPK_START - padsize);
  

• ppkptr += ZYNQ_RSA_HEADER_SIZE;
• ppkptr += ZYNQ_RSA_MAGIC_WORD_SIZE;
• ppkmodular = (u8 *)ppkptr;
• ppkptr += ZYNQ_RSA_MODULAR_SIZE;
• ppkmodularex = (u8 *)ppkptr;
• ppkptr += ZYNQ_RSA_MODULAR_EXT_SIZE;
• ppkexp = *(u32 *)ppkptr;

+}

+/*

• • Calculate the inverse(-1 / modulus[0] mod 2^32 ) for the PPK
• */

+static u32 zynq_calc_inv(void) +{

• u32 modulus = public_key.modulus[0];
• u32 tmp = BIT(1);
• u32 inverse;
• inverse = modulus & BIT(0);
• while (tmp) {

• inverse *= 2 - modulus * inverse;
  

• tmp *= tmp;
  

• }
• return -inverse;

types are u32 this minus is weird.

+}

+/*

• • Recreate the signature by padding the bytes and verify with hash value
• */

+static int zynq_pad_and_check(u8 *signature, u8 *hash) +{

• u8 padding[] = {0x30, 0x31, 0x30, 0x0D, 0x06, 0x09, 0x60, 0x86, 0x48,

• 	0x01, 0x65, 0x03, 0x04, 0x02, 0x01, 0x05, 0x00, 0x04,
  

• 	0x20 };
  

• u8 *pad_ptr = signature + 256;
• u32 pad = 256 - 3 - 19 - 32;

A little bit clarity on these magic numbers will be good.

• u32 ii;
• /* Re-Create PKCS#1v1.5 Padding */
• if (*--pad_ptr != 0x00 || *--pad_ptr != 0x01)

0 and 1 here.

• return -1;
  

• for (ii = 0; ii < pad; ii++) {

• if (*--pad_ptr != 0xFF)
  

• 	return -1;
  

• }
• if (*--pad_ptr != 0x00)

0 here.

• return -1;
  

• for (ii = 0; ii < sizeof(padding); ii++) {

• if (*--pad_ptr != padding[ii])
  

• 	return -1;
  

• }
• for (ii = 0; ii < 32; ii++) {

• if (*--pad_ptr != hash[ii])
  

• 	return -1;
  

• }
• return 0;

+}

+/*

• • Verify and extract the hash value from signature using the public key
• • and compare it with calculated hash value.
• */

+static int zynq_rsa_verify_key(const struct zynq_rsa_public_key *key,

• 	       const u8 *sig, const u32 sig_len, const u8 *hash)
  

+{

• int status;
• u32 *buf;
• if (!key || !sig || !hash)

• return -1;
  

• if (sig_len != (key->len * sizeof(u32))) {

• printf("Signature is of incorrect length %d\n", sig_len);
  

• return -1;
  

• }
• /* Sanity check for stack size */
• if (sig_len > ZYNQ_RSA_SPK_SIGNATURE_SIZE) {

• printf("Signature length %u exceeds maximum %d\n", sig_len,
  

•        ZYNQ_RSA_SPK_SIGNATURE_SIZE);
  

• return -1;
  

• }
• buf = malloc(sig_len);
• memcpy(buf, sig, sig_len);
• status = zynq_pow_mod((u32 *)key, buf);
• if (status == -1)

• return status;
  

• status = zynq_pad_and_check((u8 *)buf, (u8 *)hash);
• return status;

+}

+/*

• • Authenticate the partition
• */

+static int zynq_authenticate_part(u8 *buffer, u32 size) +{

• u8 hash_signature[32];
• u8 *spk_modular;
• u8 *spk_modular_ex;
• u8 *signature_ptr;
• u32 status;
• debug("%s\n", __func__);
• signature_ptr = (u8 *)(buffer + size - ZYNQ_RSA_SIGNATURE_SIZE);
• signature_ptr += ZYNQ_RSA_HEADER_SIZE;
• signature_ptr += ZYNQ_RSA_MAGIC_WORD_SIZE;
• ppkmodular = (u8 *)signature_ptr;
• signature_ptr += ZYNQ_RSA_MODULAR_SIZE;
• ppkmodularex = signature_ptr;
• signature_ptr += ZYNQ_RSA_MODULAR_EXT_SIZE;
• signature_ptr += ZYNQ_RSA_EXPO_SIZE;
• sha256_csum_wd((const unsigned char *)signature_ptr,

•        (ZYNQ_RSA_MODULAR_EXT_SIZE + ZYNQ_RSA_EXPO_SIZE +
  

•        ZYNQ_RSA_MODULAR_SIZE),
  

•        (unsigned char *)hash_signature, 0x1000);
  

• spk_modular = (u8 *)signature_ptr;
• signature_ptr += ZYNQ_RSA_MODULAR_SIZE;
• spk_modular_ex = (u8 *)signature_ptr;
• signature_ptr += ZYNQ_RSA_MODULAR_EXT_SIZE;
• signature_ptr += ZYNQ_RSA_EXPO_SIZE;
• public_key.len = ZYNQ_RSA_MODULAR_SIZE / sizeof(u32);
• public_key.modulus = (u32 *)ppkmodular;
• public_key.rr = (u32 *)ppkmodularex;
• public_key.n0inv = zynq_calc_inv();
• status = zynq_rsa_verify_key(&public_key, signature_ptr,

• 		     ZYNQ_RSA_SPK_SIGNATURE_SIZE,
  

• 		     hash_signature);
  

remove this empty line.

• if (status)

• return status;
  

• signature_ptr += ZYNQ_RSA_SPK_SIGNATURE_SIZE;
• sha256_csum_wd((const unsigned char *)buffer,

•        (size - ZYNQ_RSA_PARTITION_SIGNATURE_SIZE),
  

•        (unsigned char *)hash_signature, 0x1000);
  

• public_key.len = ZYNQ_RSA_MODULAR_SIZE / sizeof(u32);
• public_key.modulus = (u32 *)spk_modular;
• public_key.rr = (u32 *)spk_modular_ex;
• public_key.n0inv = zynq_calc_inv();
• status = zynq_rsa_verify_key(&public_key, (u8 *)signature_ptr,

• 		     ZYNQ_RSA_PARTITION_SIGNATURE_SIZE,
  

• 		     (u8 *)hash_signature);
  

• return status;

you can simply just return zynq_rsa_verify_key...

+}

+/*

• • Parses the partition header and verfies the authenticated and
• • encrypted image.
• */

+static int zynq_verify_image(u32 src_ptr) +{

• u32 silicon_ver;
• u32 image_base_addr;
• u32 status;
• u32 partition_num = 0;
• u32 efuseval;
• u32 srcaddr;
• u32 size;
• u32 fsbl_len;
• struct partition_hdr *hdr_ptr;
• u32 part_data_len;
• u32 part_img_len;
• u32 part_attr;
• u32 part_load_addr;
• u32 part_dst_addr;
• u32 part_chksum_offset;
• u32 part_start_addr;
• u32 part_total_size;
• u32 partitioncount;

this is quite huge, please add more vars to one line.

• bool encrypt_part_flag = false;
• bool part_chksum_flag = false;
• bool signed_part_flag = false;
• image_base_addr = src_ptr;
• silicon_ver = zynq_get_silicon_version();
• /* RSA not supported in silicon versions 1.0 and 2.0 */
• if (silicon_ver == 0 || silicon_ver == 1)

• return -1;
  

• /* Extract ppk if efuse was blown Otherwise return error */
• efuseval = readl(&efuse_base->status);
• if (!(efuseval & ZYNQ_EFUSE_RSA_ENABLE_MASK))

• return -1;
  

• zynq_extract_ppk(fsbl_len);
• zynq_get_partition_info(image_base_addr, &fsbl_len,

• 		&part_hdr[0]);
  

• partitioncount = zynq_get_part_count(&part_hdr[0]);
• if (partitioncount <= 2 ||

more clarity on this 2 will be helpful.

•    partitioncount > ZYNQ_MAX_PARTITION_NUMBER)
  

• return -1;
  

• while (partition_num < partitioncount) {

• if (((part_hdr[partition_num].partitionattr &
  

•    ZYNQ_ATTRIBUTE_RSA_PART_OWNER_MASK) >> 16) !=
  

•    ZYNQ_RSA_PART_OWNER_UBOOT) {
  

• 	printf("UBOOT is not Owner for partition %d\r\n",
  

• 	       partition_num);
  

• 	partition_num++;
  

• 	continue;
  

• }
  

• hdr_ptr = &part_hdr[partition_num];
  

• status = zynq_validate_hdr(hdr_ptr);
  

• if (status)
  

• 	return status;
  

• part_data_len = hdr_ptr->datawordlen;
  

• part_img_len = hdr_ptr->imagewordlen;
  

• part_attr = hdr_ptr->partitionattr;
  

• part_load_addr = hdr_ptr->loadaddr;
  

• part_chksum_offset = hdr_ptr->checksumoffset;
  

• part_start_addr = hdr_ptr->partitionstart;
  

• part_total_size = hdr_ptr->partitionwordlen;
  

• if (part_data_len != part_img_len) {
  

• 	debug("Encrypted\r\n");
  

• 	encrypt_part_flag = true;
  

• }
  

• if (part_attr & ZYNQ_ATTRIBUTE_CHECKSUM_TYPE_MASK)
  

• 	part_chksum_flag = true;
  

• if (part_attr & ZYNQ_ATTRIBUTE_RSA_PRESENT_MASK) {
  

• 	debug("RSA Signed\r\n");
  

• 	signed_part_flag = true;
  

• 	size = part_total_size << WORD_LENGTH_SHIFT;
  

• } else {
  

• 	size = part_img_len;
  

• }
  

• if (!signed_part_flag && !part_chksum_flag) {
  

• 	printf("Partition not signed & no chksum\n");
  

• 	continue;
  

• }
  

• srcaddr = image_base_addr +
  

• 	  (part_start_addr << WORD_LENGTH_SHIFT);
  

Please put TODO here that this is just limititation for PS DDR. There could be a gap between PS and PL DDR which will end up that this condition is not valid. It means in general this should be loop over all banks but TODO here should be fine.

• if (part_load_addr < gd->bd->bi_dram[0].start &&
  

•     ((part_load_addr + part_data_len) >
  

•     (gd->bd->bi_dram[0].start +
  

•      gd->bd->bi_dram[0].size))) {
  

• 	printf("INVALID_LOAD_ADDRESS_FAIL\r\n");
  

• 	return -1;
  

• }
  

• if (part_attr & ZYNQ_ATTRIBUTE_PL_IMAGE_MASK)
  

• 	part_load_addr = srcaddr;
  

• else
  

• 	memcpy((u32 *)part_load_addr, (u32 *)srcaddr,
  

• 	       size);
  

• if (part_chksum_flag) {
  

• 	part_chksum_offset = image_base_addr +
  

• 			     (part_chksum_offset <<
  

• 			     WORD_LENGTH_SHIFT);
  

• 	status = zynq_validate_partition(part_load_addr,
  

• 					 (part_total_size <<
  

• 					  WORD_LENGTH_SHIFT),
  

• 					 part_chksum_offset);
  

• 	if (status != 0) {
  

• 		printf("PART_CHKSUM_FAIL\r\n");
  

• 		return -1;
  

• 	}
  

• 	debug("Partition Validation Done\r\n");
  

• }
  

• if (signed_part_flag) {
  

• 	status = zynq_authenticate_part((u8 *)part_load_addr,
  

• 					size);
  

• 	if (status != 0) {
  

• 		printf("AUTHENTICATION_FAIL\r\n");
  

• 		return -1;
  

• 	}
  

• 	debug("Authentication Done\r\n");
  

• }
  

• if (encrypt_part_flag) {
  

• 	debug("DECRYPTION \r\n");
  

• 	part_dst_addr = part_load_addr;
  

• 	if (part_attr & ZYNQ_ATTRIBUTE_PL_IMAGE_MASK)
  

• 		part_dst_addr = 0xFFFFFFFF;
  

• 	status = zynq_decrypt_load(part_load_addr,
  

• 				   part_img_len,
  

• 				   part_dst_addr,
  

• 				   part_data_len,
  

• 				   BIT_NONE);
  

• 	if (status != 0) {
  

• 		printf("DECRYPTION_FAIL\r\n");
  

• 		return -1;
  

• 	}
  

• }
  

• partition_num++;
  

• }
• return 0;

+}

+static int do_zynq_rsa(cmd_tbl_t *cmdtp, int flag, int argc,

•        char * const argv[])
  

+{

• u32 src_ptr;
• char *endp;
• if (argc < 3)

• goto usage;
  

just return CMD_RET_USAGE;

• src_ptr = simple_strtoul(argv[2], &endp, 16);
• if (*argv[2] == 0 || *endp != 0)

• goto usage;
  

just return CMD_RET_USAGE;

• if (!zynq_verify_image(src_ptr))

• return 0;
  

+usage:

• return CMD_RET_USAGE;

+} +#endif

+#ifdef CONFIG_CMD_ZYNQ_AES +static int zynq_decrypt_image(cmd_tbl_t *cmdtp, int flag, int argc,

• 	      char * const argv[])
  

+{

• char *endp;
• u32 srcaddr;
• u32 srclen;
• u32 dstaddr;
• u32 dstlen;
• u8 imgtype = BIT_NONE;
• int status;
• u8 i = 2;
• if (argc < 5 && argc > 6)

6 is already handled by do_zynq.

• goto usage;
  

just return CMD_RET_USAGE instead of these gotos. The same below.

• if (argc == 5) {

• if (!strcmp("load", argv[i]))
  

• 	imgtype = BIT_FULL;
  

• else if (!strcmp("loadp", argv[i]))
  

• 	imgtype = BIT_PARTIAL;
  

• else
  

• 	goto usage;
  

• i++;
  

this i++ doesn't look good. Really we can get rid of this by simply using zynq aes with 6 parameters and then simply

zynq aesload and zynq aesloadb and they can point to the same function where you will simply find out if this full or partial.

With making this change the whole code below can be pretty much simplified and will be much easier to read.

Also then you can check exact number of args like this. if (argc != cmdtp->maxargs) return CMD_RET_USAGE;

• }
• srcaddr = simple_strtoul(argv[i], &endp, 16);
• if (*argv[i++] == 0 || *endp != 0)

• goto usage;
  

• srclen = simple_strtoul(argv[i], &endp, 16);
• if (*argv[i++] == 0 || *endp != 0)

• goto usage;
  

• if (argc == 5) {

• dstaddr = 0xFFFFFFFF;
  

• dstlen = srclen;
  

• } else {

• dstaddr = simple_strtoul(argv[i], &endp, 16);
  

• if (*argv[i++] == 0 || *endp != 0)
  

• 	goto usage;
  

• dstlen = simple_strtoul(argv[i], &endp, 16);
  

• if (*argv[i++] == 0 || *endp != 0)
  

• 	goto usage;
  

• }
• /*

• * If the image is not bitstream but destination address is
  

• * 0xFFFFFFFF
  

• */
  

• if (imgtype == BIT_NONE && dstaddr == 0xFFFFFFFF) {

• printf("ERR:use zynqaes load/loadp encrypted bitstream\n");
  

this will go away with zynqload/loadb subcommand.

• goto usage;
  

• }
• /*

• * Roundup source and destination lengths to
  

• * word size
  

• */
  

• if (srclen % 4)

• srclen = roundup(srclen, 4);
  

• if (dstlen % 4)

• dstlen = roundup(dstlen, 4);
  

• status = zynq_decrypt_load(srcaddr, srclen >> 2, dstaddr, dstlen >> 2,

• 		   imgtype);
  

• if (status != 0)

• return -1;
  

Use CMD_RET_FAILURE here.

• return 0;

Use CMD_RET_SUCCESS

+usage:

• return CMD_RET_USAGE;

+}

+static int do_zynq_aes(cmd_tbl_t *cmdtp, int flag, int argc,

•        char * const argv[])
  

+{

• if (!zynq_decrypt_image(cmdtp, flag, argc, argv))

• return 0;
  

• else

• return CMD_RET_USAGE;
  

I can't see any reason for having this function completely. zynq_decrypt_image can be used directly without this function.

+} +#endif

+static cmd_tbl_t zynq_commands[] = { +#ifdef CONFIG_CMD_ZYNQ_RSA

• U_BOOT_CMD_MKENT(rsa, 3, 1, do_zynq_rsa, "", ""),

+#endif +#ifdef CONFIG_CMD_ZYNQ_AES

• U_BOOT_CMD_MKENT(aes, 6, 1, do_zynq_aes, "", ""),

+#endif +};

+static int do_zynq(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) +{

• cmd_tbl_t *zynq_cmd;
• int ret;
• if (!ARRAY_SIZE(zynq_commands)) {

• puts("No zynq specific command enabled\n");
  

• return CMD_RET_USAGE;
  

• }
• if (argc < 2)

• return CMD_RET_USAGE;
  

empty line here please

• zynq_cmd = find_cmd_tbl(argv[1], zynq_commands,

• 		ARRAY_SIZE(zynq_commands));
  

• if (!zynq_cmd || argc > zynq_cmd->maxargs)

• return CMD_RET_USAGE;
  

• ret = zynq_cmd->cmd(zynq_cmd, flag, argc, argv);
• return cmd_process_error(zynq_cmd, ret);

+}

+static char zynq_help_text[] =

#if defined(CONFIG_CMD_ZYNQ_RSA)

• "rsa <baseaddr> - Verifies the authenticated and encrypted\n"
• " zynq images and loads them back to load\n"
• " addresses as specified in BOOT image(BOOT.BIN)\n"

#endif + the same for AES here.

• "aes [operation type] <srcaddr> <srclen> <dstaddr> <dstlen>\n"
• "Decrypts the encrypted image present in source address\n"
• "and places the decrypted image at destination address\n"
• "zynq aes operations:\n"
• " zynq aes <srcaddr> <srclen> <dstaddr> <dstlen>\n"
• " zynq aes load <srcaddr> <srclen>\n"
• " zynq aes loadp <srcaddr> <srclen>\n"
• "if operation type is load or loadp, it loads the encrypted\n"
• "full or partial bitstream on to PL respectively. If no valid\n"
• "operation type specified then it loads decrypted image back\n"
• "to memory and it doesn't support loading PL bistsream\n";

+U_BOOT_CMD(zynq, 6, 0, do_zynq,

•   "Zynq specific commands", zynq_help_text
  

+); diff --git a/drivers/fpga/zynqpl.c b/drivers/fpga/zynqpl.c index fd37d18..99b7930 100644 --- a/drivers/fpga/zynqpl.c +++ b/drivers/fpga/zynqpl.c @@ -17,6 +17,7 @@

#define DEVCFG_CTRL_PCFG_PROG_B 0x40000000 #define DEVCFG_CTRL_PCFG_AES_EFUSE_MASK 0x00001000 +#define DEVCFG_CTRL_PCAP_RATE_EN_MASK 0x02000000 #define DEVCFG_ISR_FATAL_ERROR_MASK 0x00740040 #define DEVCFG_ISR_ERROR_FLAGS_MASK 0x00340840 #define DEVCFG_ISR_RX_FIFO_OV 0x00040000 @@ -497,3 +498,69 @@ struct xilinx_fpga_op zynq_op = { .loadfs = zynq_loadfs, #endif };

+#ifdef CONFIG_CMD_ZYNQ_AES +/*

• • Load the encrypted image from src addr and decrypt the image and
• • place it back the decrypted image into dstaddr.
• */

+int zynq_decrypt_load(u32 srcaddr, u32 srclen, u32 dstaddr, u32 dstlen,

•       u8 bstype)
  

+{

• u32 isr_status, ts;
• if (srcaddr < SZ_1M || dstaddr < SZ_1M) {

• printf("%s: src and dst addr should be > 1M\n",
  

•        __func__);
  

• return FPGA_FAIL;
  

• }
• if (zynq_dma_xfer_init(bstype)) {

• printf("%s: zynq_dma_xfer_init FAIL\n", __func__);
  

• return FPGA_FAIL;
  

• }
• writel((readl(&devcfg_base->ctrl) | DEVCFG_CTRL_PCAP_RATE_EN_MASK),

•       &devcfg_base->ctrl);
  

• debug("%s: Source = 0x%08X\n", __func__, (u32)srcaddr);
• debug("%s: Size = %zu\n", __func__, srclen);
• /* flush(clean & invalidate) d-cache range buf */
• flush_dcache_range((u32)srcaddr, (u32)srcaddr +

• 	roundup(srclen << 2, ARCH_DMA_MINALIGN));
  

• /*

• * Flush destination address range only if image is not
  

• * bitstream.
  

• */
  

• if (bstype == BIT_NONE)

• flush_dcache_range((u32)dstaddr, (u32)dstaddr +
  

• 		roundup(dstlen << 2, ARCH_DMA_MINALIGN));
  

• if (zynq_dma_transfer(srcaddr | 1, srclen, dstaddr | 1, dstlen))

• return FPGA_FAIL;
  

• if (bstype == BIT_FULL) {

• isr_status = readl(&devcfg_base->int_sts);
  

• /* Check FPGA configuration completion */
  

• ts = get_timer(0);
  

• while (!(isr_status & DEVCFG_ISR_PCFG_DONE)) {
  

• 	if (get_timer(ts) > CONFIG_SYS_FPGA_WAIT) {
  

• 		printf("%s: Timeout wait for FPGA to config\n",
  

• 		       __func__);
  

• 		return FPGA_FAIL;
  

• 	}
  

• 	isr_status = readl(&devcfg_base->int_sts);
  

• }
  

• printf("%s: FPGA config done\n", __func__);
  

• zynq_slcr_devcfg_enable();
  

• }
• writel((readl(&devcfg_base->ctrl) & ~DEVCFG_CTRL_PCAP_RATE_EN_MASK),

•       &devcfg_base->ctrl);
  

• return FPGA_SUCCESS;

+} +#endif diff --git a/include/u-boot/rsa-mod-exp.h b/include/u-boot/rsa-mod-exp.h index 3253614..8a428c4 100644 --- a/include/u-boot/rsa-mod-exp.h +++ b/include/u-boot/rsa-mod-exp.h @@ -42,6 +42,10 @@ int rsa_mod_exp_sw(const uint8_t *sig, uint32_t sig_len, int rsa_mod_exp(struct udevice *dev, const uint8_t *sig, uint32_t sig_len, struct key_prop *node, uint8_t *out);

+#if defined(CONFIG_CMD_ZYNQ_RSA) +int zynq_pow_mod(u32 *keyptr, u32 *inout); +#endif

/**

• struct struct mod_exp_ops - Driver model for RSA Modular Exponentiation

• 		operations
  

diff --git a/include/zynq_bootimg.h b/include/zynq_bootimg.h new file mode 100644 index 0000000..c39c0bf --- /dev/null +++ b/include/zynq_bootimg.h @@ -0,0 +1,33 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/*

• • Copyright (C) 2018 Xilinx, Inc.
• */

+#ifndef _ZYNQ_BOOTIMG_H_ +#define _ZYNQ_BOOTIMG_H_

+#define ZYNQ_MAX_PARTITION_NUMBER 0xE

+struct partition_hdr {

• u32 imagewordlen; /* 0x0 */
• u32 datawordlen; /* 0x4 */
• u32 partitionwordlen; /* 0x8 */
• u32 loadaddr; /* 0xC */
• u32 execaddr; /* 0x10 */
• u32 partitionstart; /* 0x14 */
• u32 partitionattr; /* 0x18 */
• u32 sectioncount; /* 0x1C */
• u32 checksumoffset; /* 0x20 */
• u32 pads1[1];
• u32 acoffset; /* 0x28 */
• u32 pads2[4];
• u32 checksum; /* 0x3C */

+};

+int zynq_get_part_count(struct partition_hdr *part_hdr_info); +int zynq_get_partition_info(u32 image_base_addr, u32 *fsbl_len,

• 	    struct partition_hdr *part_hdr);
  

+int zynq_validate_hdr(struct partition_hdr *header); +int zynq_validate_partition(u32 start_addr, u32 len, u32 chksum_off);

+#endif /* _ZYNQ_BOOTIMG_H_ */ diff --git a/include/zynqpl.h b/include/zynqpl.h index cdfd8a2..d7dc064 100644 --- a/include/zynqpl.h +++ b/include/zynqpl.h @@ -11,6 +11,11 @@

#include <xilinx.h>

+#ifdef CONFIG_CMD_ZYNQ_AES +int zynq_decrypt_load(u32 srcaddr, u32 dstaddr, u32 srclen, u32 dstlen,

•       u8 bstype);
  

+#endif

extern struct xilinx_fpga_op zynq_op;

#define XILINX_ZYNQ_XC7Z007S 0x3 diff --git a/lib/rsa/rsa-mod-exp.c b/lib/rsa/rsa-mod-exp.c index 031c710..9834fd0 100644 --- a/lib/rsa/rsa-mod-exp.c +++ b/lib/rsa/rsa-mod-exp.c @@ -300,3 +300,55 @@ int rsa_mod_exp_sw(const uint8_t *sig, uint32_t sig_len,

return 0; }

+#if defined(CONFIG_CMD_ZYNQ_RSA) +/**

• • zynq_pow_mod - in-place public exponentiation
• • @keyptr: RSA key
• • @inout: Big-endian word array containing value and result
• • @return 0 on successful calculation, otherwise failure error code
• • FIXME: Use pow_mod() instead of zynq_pow_mod()

• •    pow_mod calculation required for zynq is bit different from
    

• •    pw_mod above here, hence defined zynq specific routine.
    

• */

+int zynq_pow_mod(u32 *keyptr, u32 *inout) +{

• u32 *result, *ptr;
• uint i;
• struct rsa_public_key *key;
• key = (struct rsa_public_key *)keyptr;
• /* Sanity check for stack size - key->len is in 32-bit words */
• if (key->len > RSA_MAX_KEY_BITS / 32) {

• debug("RSA key words %u exceeds maximum %d\n", key->len,
  

•       RSA_MAX_KEY_BITS / 32);
  

• return -EINVAL;
  

• }
• u32 val[key->len], acc[key->len], tmp[key->len];

This is interesting c&p from above which generate this sparse warning. I am not quite sure if this is even something what we should use - it is the part of ISO c99.

lib/rsa/rsa-mod-exp.c:331:20: warning: Variable length array is used. lib/rsa/rsa-mod-exp.c:331:35: warning: Variable length array is used. lib/rsa/rsa-mod-exp.c:331:50: warning: Variable length array is used.

• result = tmp; /* Re-use location. */
• for (i = 0, ptr = inout; i < key->len; i++, ptr++)

• val[i] = *(ptr);
  

• montgomery_mul(key, acc, val, key->rr); /* axx = a * RR / R mod M */
• for (i = 0; i < 16; i += 2) {

• montgomery_mul(key, tmp, acc, acc); /* tmp = acc^2 / R mod M */
  

• montgomery_mul(key, acc, tmp, tmp); /* acc = tmp^2 / R mod M */
  

• }
• montgomery_mul(key, result, acc, val); /* result = XX * a / R mod M */
• /* Make sure result < mod; result is at most 1x mod too large. */
• if (greater_equal_modulus(key, result))

• subtract_modulus(key, result);
  

• for (i = 0, ptr = inout; i < key->len; i++, ptr++)

• *ptr = result[i];
  

• return 0;

+} +#endif

Thanks, Michal