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

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

--
2.7.4

This email and any attachments are intended for the sole use of the named recipient(s) and contain(s) confidential information that may be proprietary, privileged or copyrighted under applicable law. If you are not the intended recipient, do not read, copy, or forward this email message or any attachments. Delete this email message and any attachments immediately.

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

Hi Stefan,

> -----Original Message-----
> From: Stefan Herbrechtsmeier [mailto:stefan@herbrechtsmeier.net]
> Sent: Thursday, May 31, 2018 3:43 PM
> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>; u-
> boot@lists.denx.de
> Cc: michal.simek@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@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

On 31.5.2018 11:25, 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 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
> 
> 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..54d71dc
> --- /dev/null
> +++ b/board/xilinx/zynq/Kconfig
> @@ -0,0 +1,26 @@
> +# Copyright (c) 2018, Xilinx, Inc.
> +#
> +# SPDX-License-Identifier: GPL-2.0
> +
> +if ARCH_ZYNQ
> +
> +config CMD_ZYNQ_AES
> +	bool "Zynq AES"
> +	default y
> +	help
> +	  Decrypts the encrypted image present in source address
> +	  and places the decrypted image at destination address.
> +
> +config CMD_ZYNQ

This should be the first entry in this file.

> +	bool "Enable ZynqMP specific commands"

zynq

> +	default y
> +	depends on CMD_ZYNQ_AES

And CMD_ZYNQ_AES should depends on CMD_ZYNQ not vice-versa.


Also in code below I see that RSA and AES are different functionalities
and they should have own Kconfig symbol.

Keep in your mind that there could be an intention to extend zynq
specific commands to something else and there should be an option to
enable only it not AES and RSA.

It means create own symbols and separate them properly.


> +	help
> +	  Enable Zynq specific commands like "zynq rsa"
> +	  which is used for 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..9b63dd7 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) += bootimg.o

This should be only for rsa not for aes. Please use proper symbols here.

> +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;
> +	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)
> +			break;
> +	}
> +	if (index  == ZYNQ_PART_HDR_WORD_COUNT)
> +		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",
> +		       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) {
> +		printf("Error: Partition DataChecksum \r\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..6aebec1
> --- /dev/null
> +++ b/board/xilinx/zynq/cmds.c
> @@ -0,0 +1,527 @@
> +// 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>
> +#include <zynqpl.h>
> +#include <fpga.h>
> +#include <zynq_bootimg.h>
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#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];
> +
> +/*
> + * 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);
> +	padsize = ((u32)ppkptr % ZYNQ_RSA_ALIGN_PPK_START);
> +	if (padsize != 0)
> +		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;
> +}
> +
> +/*
> + * 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;
> +	u32 ii;
> +
> +	/* Re-Create PKCS#1v1.5 Padding */
> +	if (*--pad_ptr != 0x00 || *--pad_ptr != 0x01)
> +		return -1;
> +
> +	for (ii = 0; ii < pad; ii++) {
> +		if (*--pad_ptr != 0xFF)
> +			return -1;
> +	}
> +
> +	if (*--pad_ptr != 0x00)
> +		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);
> +
> +	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;
> +}
> +
> +/*
> + * 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;
> +	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 ||
> +	    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);
> +
> +		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 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)
> +		goto usage;
> +
> +	if (argc == 5) {
> +		if (!strcmp("load", argv[i]))
> +			imgtype = BIT_FULL;
> +		else if (!strcmp("loadp", argv[i]))
> +			imgtype = BIT_PARTIAL;
> +		else
> +			goto usage;
> +		i++;
> +	}
> +
> +	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");
> +		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;
> +
> +	return 0;
> +
> +usage:
> +	return CMD_RET_USAGE;
> +}

Don't be scared for all functions above to move them to their own files
to make it simple.

> +
> +static int do_zynq(cmd_tbl_t *cmdtp, int flag, int argc,
> +		   char * const argv[])
> +{
> +	u32 src_ptr;
> +	int ret;
> +	char *endp;
> +
> +	if (argc < 2 || (strncmp(argv[1], "rsa", 3) &&
> +			 strncmp(argv[1], "aes", 3)))
> +		goto usage;
> +
> +	if (!strncmp(argv[1], "rsa", 3)) {
> +		src_ptr = simple_strtoul(argv[2], &endp, 16);
> +		if (*argv[2] == 0 || *endp != 0)
> +			goto usage;
> +
> +		ret = zynq_verify_image(src_ptr);
> +		if (!ret)
> +			return 0;
> +	}
> +
> +	if (!strncmp(argv[1], "aes", 3)) {
> +		ret = zynq_decrypt_image(cmdtp, flag, argc, argv);
> +		if (!ret)
> +			return 0;
> +	}

First of all this is not good style how different subcommands should be
used.

Look at test/dm/cmd_dm.c and look at U_BOOT_CMD_MKENT and handle rsa and
aes.


> +
> +usage:
> +	return CMD_RET_USAGE;
> +}
> +
> +#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"

it should be zynq aes

> +	"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

remove RSA/AES from 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

this will become AES || RSA


> +/*
> + * 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)

as we discussed internally you have if BIT_FULL it means this if is true
all the time.

> +			zynq_slcr_devcfg_enable();
> +	}
> +
> +	return FPGA_SUCCESS;
> +}
> +#endif
> diff --git a/include/u-boot/rsa-mod-exp.h b/include/u-boot/rsa-mod-exp.h
> index 3253614..9dc8de1 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)

just for 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..703250b 100644
> --- a/lib/rsa/rsa-mod-exp.c
> +++ b/lib/rsa/rsa-mod-exp.c
> @@ -300,3 +300,54 @@ int rsa_mod_exp_sw(const uint8_t *sig, uint32_t sig_len,
>  
>  	return 0;
>  }
> +
> +#if defined(CONFIG_CMD_ZYNQ)
> +/**
> + * zynq_pow_mod() - in-place public exponentiation

remove ()

> + *
> + * @keyptr:	RSA key
> + * @inout:	Big-endian word array containing value and result
> + *
> + * 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.

Please describe return value too.

> + */
> +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];
> +
> +	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

Hi Siva,

Am 31.05.2018 um 12:37 schrieb Siva Durga Prasad Paladugu:
>> -----Original Message-----
>> From: Stefan Herbrechtsmeier [mailto:stefan@herbrechtsmeier.net]
>> Sent: Thursday, May 31, 2018 3:43 PM
>> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>; u-
>> boot@lists.denx.de
>> Cc: michal.simek@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.paladugu@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

Hi Stefan,

> -----Original Message-----
> From: Stefan Herbrechtsmeier [mailto:stefan@herbrechtsmeier.net]
> Sent: Thursday, May 31, 2018 10:57 PM
> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>; u-
> boot@lists.denx.de
> Cc: michal.simek@xilinx.com
> Subject: Re: [U-Boot] [PATCH] xilinx: zynq: Add support to secure images
> 
> Hi Siva,
> 
> Am 31.05.2018 um 12:37 schrieb Siva Durga Prasad Paladugu:
> >> -----Original Message-----
> >> From: Stefan Herbrechtsmeier [mailto:stefan@herbrechtsmeier.net]
> >> Sent: Thursday, May 31, 2018 3:43 PM
> >> To: Siva Durga Prasad Paladugu <sivadur@xilinx.com>; u-
> >> boot@lists.denx.de
> >> Cc: michal.simek@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.paladugu@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. 

I got your point, Please point me to such marker inside the fpga image if you are aware of. 
Till now, I thought we don’t have such mechanism and hence like this.
Meanwhile, I will also try to find out on this internally.

>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?
We can move that code also to a routine and invoke that routine here.
Anyway, lets first sort out the first comment as its more related to functionality
and will come back to this later.

Thanks,
Siva
> 
> Best regards
>    Stefan