| Message ID | 20220411180046.1505209-7-adrian.fiergolski@fastree3d.com |
|---|---|
| State | Deferred |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | fpga: zynqmp: Adding support of loading authenticated images | expand |
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
Hi Michal, On Tue, May 3, 2022 at 10:56 AM Michal Simek <michal.simek@xilinx.com> wrote: > > > > 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. 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
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")) { + 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");