Re: [PATCH v7 6/7] fpga: zynqmp: support loading authenticated images

[Michal Simek <michal.simek@xilinx.com>]

On 4/11/22 20:00, Adrian Fiergolski wrote:
> From: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> 
> 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>
> Co-developed-by: Ricardo Salveti <ricardo@foundries.io>
> Signed-off-by: Ricardo Salveti <ricardo@foundries.io>
> Tested-by: Ricardo Salveti <ricardo@foundries.io>
> Co-developed-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com>
> Signed-off-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com>
> ---
>   boot/Kconfig                          |  4 ++--
>   doc/uImage.FIT/source_file_format.txt |  5 ++++-
>   drivers/fpga/zynqmppl.c               | 21 +++++++++++++++++++++
>   3 files changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/boot/Kconfig b/boot/Kconfig
> index b83a4e8400..f7faafb29f 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -209,8 +209,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 f93ac6d1c7..461e2af2a8 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 c7f9f4ae84..0ce641e495 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>
> @@ -210,6 +211,26 @@ static int zynqmp_load(xilinx_desc **desc_ptr, const void *buf, size_t bsize,
>   	u32 ret_payload[PAYLOAD_ARG_CNT];
>   	bool xilfpga_old = false;
>   	xilinx_desc *desc = *desc_ptr;
> +	fpga_desc *fdesc = container_of((void *)desc_ptr, fpga_desc, devdesc);
> +
> +	if (fdesc && fdesc->compatible &&
> +	    !strcmp(fdesc->compatible, "u-boot,zynqmp-fpga-ddrauth")) {

I think you should use directly here what you have in 7/7. It means to check 
that it is not fpga-legacy.

> +		struct fpga_secure_info info = { 0 };
> +
> +		if (!CONFIG_IS_ENABLED(FPGA_LOAD_SECURE)) {
> +			printf("No support for %s\n", fdesc->compatible);
> +			return FPGA_FAIL;
> +		}
> +
> +		if (!desc->operations->loads) {
> +			printf("%s: Missing load operation\n", __func__);
> +			return FPGA_FAIL;
> +		}
> +		/* DDR authentication */
> +		info.authflag = 1;
> +		info.encflag = 2;
> +		return desc->operations->loads(desc, buf, bsize, &info);
> +	}
>   
>   	if (zynqmp_firmware_version() <= PMUFW_V1_0) {
>   		puts("WARN: PMUFW v1.0 or less is detected\n");

Before you start to deal with secure bitstreams you should also likely check 
this PMUFW checking before you call loads.

Thanks,
Michal