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

[Stefan Herbrechtsmeier]

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.

> 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.

Additionally you doesn't clears the DEVCFG_CTRL_PCAP_RATE_EN_MASK.

Regards
   Stefan