Hi Siva,
Am 31.05.2018 um 12:37 schrieb Siva Durga Prasad Paladugu:
-----Original Message-----
From: Stefan Herbrechtsmeier [mailto:ste...@herbrechtsmeier.net]
Sent: Thursday, May 31, 2018 3:43 PM
To: Siva Durga Prasad Paladugu <siva...@xilinx.com>; u-
b...@lists.denx.de
Cc: michal.si...@xilinx.com
Subject: Re: [U-Boot] [PATCH] xilinx: zynq: Add support to secure images
Am 31.05.2018 um 11:25 schrieb Siva Durga Prasad Paladugu:
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.palad...@xilinx.com>
---
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 | 26 +++
board/xilinx/zynq/Makefile | 5 +
board/xilinx/zynq/bootimg.c | 143 ++++++++++++
board/xilinx/zynq/cmds.c | 527
+++++++++++++++++++++++++++++++++++++++++++
drivers/fpga/zynqpl.c | 65 ++++++
include/u-boot/rsa-mod-exp.h | 4 +
include/zynq_bootimg.h | 33 +++
include/zynqpl.h | 5 +
lib/rsa/rsa-mod-exp.c | 51 +++++
10 files changed, 860 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
[snip]
diff --git a/board/xilinx/zynq/cmds.c b/board/xilinx/zynq/cmds.c new
file mode 100644 index 0000000..6aebec1
--- /dev/null
+++ b/board/xilinx/zynq/cmds.c
@@ -0,0 +1,527 @@
[snip]
+#ifdef CONFIG_SYS_LONGHELP
+static char zynq_help_text[] =
+ "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"
+ "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"
+ "zynqaes operations:\n"
+ " zynqaes <srcaddr> <srclen> <dstaddr> <dstlen>\n"
+ " zynqaes load <srcaddr> <srclen>\n"
+ " zynqaes 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";
+#endif
+
+U_BOOT_CMD(zynq, 6, 0, do_zynq,
+ "Zynq specific commands RSA/AES verification ",
+zynq_help_text );
Why don't you integrate the encrypted fpga image support into the fpga
command?
The encrypted fpga image could be detected at run time and the only
difference is a reduced pcap rate.
Its not just handles encrypted fpga images, indeed it handles all encrypted
Images so, that’s why it is here.
But the encrypted fpga image is handled total different as it isn't
copied back to the memory. The encrypted fpga image command has more
similarity with the fpga command than the aes command. You introduce a
second command to program the fpga outside of the fpga framework.
Furthermore this command isn't supported by the fit image nor the spl.
I think the main different between the encrypted and not encrypted fpga
image is the lower pcap rate. Because the encrypted fpga image is marked
as encrypted inside the image the software could automatically enable
the lower pcap rate. This enables the spl to use an encrypted fpga image
inside a fit image.
diff --git a/drivers/fpga/zynqpl.c b/drivers/fpga/zynqpl.c index
fd37d18..bdac49e 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,67 @@ 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__);
+
+ if (bstype != BIT_PARTIAL)
+ zynq_slcr_devcfg_enable();
+ }
+
+ return FPGA_SUCCESS;
+}
+#endif
This function introduces a lot of duplicated code because it mostly copies
the zynq_load function.
Yeah, but not much. I felt its better to be a separate code than handling with
lot of if's just to be clean.
Also, most of functionality was anyway carried out in separate functions like
dma init and xfer. So, not a big
duplicate.
Don't you duplicate the whole "check fpga configuration completion" code?
Best regards
Stefan
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
