Re: [PATCH v9 12/13] fpga: zynqmp: support loading authenticated images

On 6/1/22 10:46, Oleksandr Suvorov wrote:

Add supporting new compatible string "u-boot,zynqmp-fpga-ddrauth" to handle loading authenticated images (DDR).

Based on solution by Jorge Ramirez-Ortiz jorge@foundries.io Signed-off-by: Oleksandr Suvorov oleksandr.suvorov@foundries.io

---

(no changes since v1)

boot/Kconfig | 4 ++-- doc/uImage.FIT/source_file_format.txt | 5 ++++- drivers/fpga/zynqmppl.c | 18 ++++++++++++++++++ include/xilinx.h | 1 + include/zynqmppl.h | 3 ++- 5 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/boot/Kconfig b/boot/Kconfig index dff4d23b887..4a06d35acc0 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -210,8 +210,8 @@ config SPL_LOAD_FIT 1. "loadables" images, other than FDTs, which do not have a "load" property will not be loaded. This limitation also applies to FPGA images with the correct "compatible" string.

•  2. For FPGA images, only the "compatible" = "u-boot,fpga-legacy"
  

•     loading method is supported.
  

•  2. For FPGA images, the supported "compatible" list is in the
  

•     doc/uImage.FIT/source_file_format.txt.
  

  3. FDTs are only loaded for images with an "os" property of "u-boot". "linux" images are also supported with Falcon boot mode.

diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt index f93ac6d1c7b..461e2af2a84 100644 --- a/doc/uImage.FIT/source_file_format.txt +++ b/doc/uImage.FIT/source_file_format.txt @@ -184,7 +184,10 @@ the '/images' node should have the following layout: Mandatory for types: "firmware", and "kernel". - compatible : compatible method for loading image. Mandatory for types: "fpga", and images that do not specify a load address.

• To use the generic fpga loading routine, use "u-boot,fpga-legacy".

• Supported compatible methods:

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

  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 3dacb10e11f..76efc4b4a90 100644 --- a/drivers/fpga/zynqmppl.c +++ b/drivers/fpga/zynqmppl.c @@ -9,6 +9,7 @@ #include <common.h> #include <compiler.h> #include <cpu_func.h> +#include <fpga.h> #include <log.h> #include <zynqmppl.h> #include <zynqmp_firmware.h> @@ -232,6 +233,7 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf, u32 buf_lo, buf_hi; u32 bsize_req = (u32)bsize; u32 ret_payload[PAYLOAD_ARG_CNT];

• struct fpga_secure_info info = { 0 };

  debug("%s called!\n", __func__);

@@ -240,6 +242,19 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf, return FPGA_FAIL; }

• switch (flags) {
• case FPGA_LEGACY:

• break;	/* Handle the legacy image later in this function */
  

• case FPGA_XILINX_ZYNQMP_DDRAUTH:

• /* DDR authentication */
  

• info.authflag = ZYNQMP_FPGA_AUTH_DDR;
  

• info.encflag = FPGA_NO_ENC_OR_NO_AUTH;
  

• return desc->operations->loads(desc, buf, bsize, &info);
  

In case of FPGA_LOAD_SECURE is not enabled I would expect this code won't be even here. The same for 13/13 for another type.

• default:

• puts("Unsupported bitstream type\n");
  

• return FPGA_FAIL;
  

• }
• if (zynqmp_firmware_version() <= PMUFW_V1_0) { puts("WARN: PMUFW v1.0 or less is detected\n"); puts("WARN: Not all bitstream formats are supported\n");

@@ -335,6 +350,9 @@ static int zynqmp_str2flag(xilinx_desc *desc, const char *str) if (!strncmp(str, "u-boot,fpga-legacy", 18)) return FPGA_LEGACY;

• if (!strncmp(str, "u-boot,zynqmp-fpga-ddrauth", 26))

• return FPGA_XILINX_ZYNQMP_DDRAUTH;
  

The same here. When FPGA_LOAD_SECURE is disabled u-boot shouldn't bother/understand these secure compatible strings.

return 0; }

diff --git a/include/xilinx.h b/include/xilinx.h index 89a12818311..ffd95ad7225 100644 --- a/include/xilinx.h +++ b/include/xilinx.h @@ -39,6 +39,7 @@ typedef enum { /* typedef xilinx_family */

/* FPGA bitstream supported types */ #define FPGA_LEGACY BIT(0) +#define FPGA_XILINX_ZYNQMP_DDRAUTH BIT(1)

typedef struct { /* typedef xilinx_desc */ xilinx_family family; /* part type */ diff --git a/include/zynqmppl.h b/include/zynqmppl.h index a6e171dcb49..c4d7a41220d 100644 --- a/include/zynqmppl.h +++ b/include/zynqmppl.h @@ -26,6 +26,7 @@ extern struct xilinx_fpga_op zynqmp_op;

#define XILINX_ZYNQMP_DESC \ -{ xilinx_zynqmp, csu_dma, 1, &zynqmp_op, 0, &zynqmp_op, NULL, FPGA_LEGACY } +{ xilinx_zynqmp, csu_dma, 1, &zynqmp_op, 0, &zynqmp_op, NULL, \

• (FPGA_LEGACY | FPGA_XILINX_ZYNQMP_DDRAUTH) }

The same here. You can move this macro directly to code and put there some macros around. When secure options is disabled you shouldn't publish that support is there via flags.

M

#endif /* _ZYNQMPPL_H_ */