[U-Boot] [PATCH] xilinx: zynq: Add support to secure images
Siva Durga Prasad Paladugu
sivadur at xilinx.com
Thu May 31 10:37:48 UTC 2018
Hi Stefan,
> -----Original Message-----
> From: Stefan Herbrechtsmeier [mailto:stefan at herbrechtsmeier.net]
> Sent: Thursday, May 31, 2018 3:43 PM
> To: Siva Durga Prasad Paladugu <sivadur at xilinx.com>; u-
> boot at lists.denx.de
> Cc: michal.simek at xilinx.com
> Subject: Re: [U-Boot] [PATCH] xilinx: zynq: Add support to secure images
>
> Hi Siva,
>
> 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.paladugu at 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.
>
> > 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.
>
> Additionally you doesn't clears the DEVCFG_CTRL_PCAP_RATE_EN_MASK.
Yes, I think, it needs to be cleared otherwise it may slow down next pcap transfers.
Will fix this.
Thanks,
Siva
>
> Regards
> Stefan
More information about the U-Boot
mailing list