Hi Michal,
On Tue, May 3, 2022 at 10:56 AM Michal Simek <michal.si...@xilinx.com> wrote:
>
>
>
> On 4/11/22 20:00, Adrian Fiergolski wrote:
> > From: Oleksandr Suvorov <oleksandr.suvo...@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 <jo...@foundries.io>
> > Signed-off-by: Oleksandr Suvorov <oleksandr.suvo...@foundries.io>
> > Co-developed-by: Ricardo Salveti <rica...@foundries.io>
> > Signed-off-by: Ricardo Salveti <rica...@foundries.io>
> > Tested-by: Ricardo Salveti <rica...@foundries.io>
> > Co-developed-by: Adrian Fiergolski <adrian.fiergol...@fastree3d.com>
> > Signed-off-by: Adrian Fiergolski <adrian.fiergol...@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.
Thanks, fixed. The fix will be introduced in the next patchset version.
>
> > + 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.
Michal, this code block (for earlier PMUFW) does:
1. Set a flag ZYNQMP_FPGA_BIT_NS.
According to a description of the commit
19ed4b697b9732e0a5097bd233fba7e24dfe9146, ZYNQMP_FPGA_BIT_NS
bit should be set only for nonsecure bitstream. So not needed for
zynqmp_loads().
2. Prepare a pointer to a bitstream size (bsizeptr) instead of value (bsize).
For secure bitstream, a key address is already used instead.
3. Validate a bitstream data and placement with zynqmp_validate_bitstream().
I've not found enough information about validating non-secure/secure
bitstreams. Could you confirm the function zynqmp_validate_bitstream()
can be used to validate both legacy and secure bitstreams?
Thanks,
Oleksandr.
>
> Thanks,
> Michal
--
Best regards
Oleksandr
Oleksandr Suvorov
cryo...@gmail.com
