Re: [PATCH v6 7/7] fpga: zynqmp: support loading encrypted bitfiles

[Michal Simek]

On 2/7/22 12:18, Adrian Fiergolski wrote:
Add supporting new compatible string "u-boot,zynqmp-fpga-enc" to handle
loading encrypted bitfiles.

This feature requires encrypted FSBL,as according to UG1085:
"The CSU automatically locks out the AES key, stored in either BBRAM or eFUSEs,
  as a key source to the AES engine if the FSBL is not encrypted. This prevents
  using the BBRAM or eFUSE as the key source to the AES engine during run-time
  applications."

Signed-off-and-tested-by: Adrian Fiergolski <adrian.fiergol...@fastree3d.com>
---
  doc/uImage.FIT/source_file_format.txt |  2 ++
  drivers/fpga/zynqmppl.c               | 16 ++++++++++++----
  2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/doc/uImage.FIT/source_file_format.txt 
b/doc/uImage.FIT/source_file_format.txt
index 461e2af2a8..2cf77ba3e9 100644
--- a/doc/uImage.FIT/source_file_format.txt
+++ b/doc/uImage.FIT/source_file_format.txt
@@ -188,6 +188,8 @@ the '/images' node should have the following layout:
      "u-boot,fpga-legacy" - the generic fpga loading routine.
      "u-boot,zynqmp-fpga-ddrauth" - signed non-encrypted FPGA bitstream for
      Xilinx Zynq UltraScale+ (ZymqMP) device.
+    "u-boot,zynqmp-fpga-enc" - encrypted FPGA bitstream for Xilinx Zynq
+    UltraScale+ (ZymqMP) device.

ZynqMP

  
    Optional nodes:
    - hash-1 : Each hash sub-node represents separate hash or checksum
diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c
index bf6f56e1c4..5fcca8d1b8 100644
--- a/drivers/fpga/zynqmppl.c
+++ b/drivers/fpga/zynqmppl.c
@@ -214,7 +214,9 @@ static int zynqmp_load(xilinx_desc **desc_ptr, const void 
*buf, size_t bsize,
        fpga_desc *fdesc = container_of((void *)desc_ptr, fpga_desc, devdesc);
  
  	if (fdesc && fdesc->compatible &&
-           !strcmp(fdesc->compatible, "u-boot,zynqmp-fpga-ddrauth")) {
+           ( !strcmp(fdesc->compatible, "u-boot,zynqmp-fpga-ddrauth") ||
+             !strcmp(fdesc->compatible, "u-boot,zynqmp-fpga-enc") )
+             ) {

coding style and I think you should revert the logic here. You should check 
u-boot-fpga-legacy and use inverted logic if possible which should save some bytes.

And strncmp


                if (CONFIG_IS_ENABLED(FPGA_LOAD_SECURE)) {
                        struct fpga_secure_info info = { 0 };
  
@@ -222,9 +224,15 @@ static int zynqmp_load(xilinx_desc **desc_ptr, const void *buf, size_t bsize,
                                printf("%s: Missing load operation\n", 
__func__);
                                return FPGA_FAIL;
                        }
-                       /* DDR authentication */
-                       info.authflag = 1;
-                       info.encflag = 2;
+                       if(!strcmp(fdesc->compatible+19, "enc")){

coding style issues and use strncmp.

+                         /* Encryption using device key*/

coding style issues.

+                         info.authflag = 2;
+                         info.encflag = 0;

You should use macros for it.


+                       } else {
+                         /* DDR authentication */
+                         info.authflag = 1;
+                         info.encflag = 2;

ditto.

+                       }
                        return desc->operations->loads(desc, buf, bsize, &info);
                } else {
                        printf("No support for %s\n", fdesc->compatible);

M