[RFC PATCH v4 6/6] fpga: zynqmp: support loading authenticated images

[Simon Glass]

Hi Oleksandr,

On Thu, 11 Nov 2021 at 02:59, Oleksandr Suvorov
<oleksandr.suvorov at foundries.io> 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 at foundries.io>
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov at foundries.io>
> Co-developed-by: Ricardo Salveti <ricardo at foundries.io>
> Signed-off-by: Ricardo Salveti <ricardo at foundries.io>
> Tested-by: Ricardo Salveti <ricardo at foundries.io>
> ---
>
> Changes in v4:
> - change interface to xilinx_desc->operations->open() callback.
> - fix a bug from previous version of the patchset in dereferencing
>   of a parent fpga_desc structure.
>
> Changes in v3:
> - remove the patch which introduced CMD_SPL_FPGA_LOAD_SECURE.
> - fix mixing definitions/declarations.
> - replace strcmp() calls with more secure strncmp().
> - document the "u-boot,zynqmp-fpga-ddrauth" compatible string.
> - fix code style by check-patch recommendations.
>
> Changes in v2:
> - add function fit_fpga_load() to simplify calls of fpga_load()
>   from contexts without a compatible attribute.
> - move all ZynqMP-specific logic to drivers/fpga/zynqmppl.c
> - prepare for passing a "compatible" FDT property to any fpga driver.
>
>  common/Kconfig.boot                   |  4 ++--
>  doc/uImage.FIT/source_file_format.txt |  5 ++++-
>  drivers/fpga/zynqmppl.c               | 21 +++++++++++++++++++++
>  3 files changed, 27 insertions(+), 3 deletions(-)
>
> diff --git a/common/Kconfig.boot b/common/Kconfig.boot
> index a8d4be23a97..f8796541746 100644
> --- a/common/Kconfig.boot
> +++ b/common/Kconfig.boot
> @@ -198,8 +198,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 c7f9f4ae846..59948afa3ee 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")) {
> +#if CONFIG_IS_ENABLED(FPGA_LOAD_SECURE)

Can you use if() ?

> +               struct fpga_secure_info info = { 0 };
> +
> +               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);
> +#else
> +               printf("No support for %s\n", fdesc->compatible);
> +               return FPGA_FAIL;
> +#endif
> +       }
>
>         if (zynqmp_firmware_version() <= PMUFW_V1_0) {
>                 puts("WARN: PMUFW v1.0 or less is detected\n");
> --
> 2.31.1
>

We do need an FPGA emulator to provide test coverage for this. All
uclasses should have one. It just needs to implement the uclass
methods in sandbox. At present all tests are skipped!

Regards,
SImon