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

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

[Oleksandr Suvorov]

Note: this version of the patchset changes the interface to the
load() callback of xilinx_fpga_op.

This patchset introduces support for the authenticated FPGA images
on ZynqMP boards, besides that introducing common way to pass the
compatible property to any fpga driver.

It bases on the initial work by Jorge Ramirez-Ortiz <jorge@foundries.io>
https://patchwork.ozlabs.org/project/uboot/patch/20211015091506.2602-1-jorge@foundries.io/
https://patchwork.ozlabs.org/project/uboot/patch/20211005111324.19749-3-jorge@foundries.io/

Changes in v4:
- change interface to xilinx_desc->operations->load() 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.

Oleksandr Suvorov (6):
  fpga: add option for loading FPGA secure bitstreams
  fpga: add fit_fpga_load function
  fpga: xilinx: pass an address of xilinx_desc in fpga_desc
  fpga: xilinx: add missed identifier names
  fpga: xilinx: pass xilinx_desc pointer address into load() ops
  fpga: zynqmp: support loading authenticated images

 cmd/Kconfig                           |  3 +-
 common/Kconfig.boot                   |  4 +--
 common/spl/spl_fit.c                  |  6 ++--
 doc/uImage.FIT/source_file_format.txt |  5 +++-
 drivers/fpga/Kconfig                  | 14 +++++++++
 drivers/fpga/fpga.c                   | 41 ++++++++++++++++++++-------
 drivers/fpga/spartan2.c               |  3 +-
 drivers/fpga/spartan3.c               |  3 +-
 drivers/fpga/versalpl.c               |  2 +-
 drivers/fpga/virtex2.c                |  3 +-
 drivers/fpga/xilinx.c                 |  8 ++++--
 drivers/fpga/zynqmppl.c               | 30 +++++++++++++++++---
 drivers/fpga/zynqpl.c                 |  3 +-
 include/fpga.h                        |  4 +++
 include/xilinx.h                      | 12 ++++----
 15 files changed, 106 insertions(+), 35 deletions(-)

-- 
2.31.1

[RFC PATCH v4 1/6] fpga: add option for loading FPGA secure bitstreams

[Oleksandr Suvorov]

It allows using this feature without enabling the "fpga loads"
command.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
Tested-by: Ricardo Salveti <ricardo@foundries.io>
---

(no changes since v1)

 cmd/Kconfig             |  3 ++-
 drivers/fpga/Kconfig    | 14 ++++++++++++++
 drivers/fpga/fpga.c     |  2 +-
 drivers/fpga/xilinx.c   |  2 +-
 drivers/fpga/zynqmppl.c |  4 ++--
 5 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 5b30b13e438..270d1765d35 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -949,8 +949,9 @@ config CMD_FPGA_LOADP
 	  a partial bitstream.
 
 config CMD_FPGA_LOAD_SECURE
-	bool "fpga loads - loads secure bitstreams (Xilinx only)"
+	bool "fpga loads - loads secure bitstreams"
 	depends on CMD_FPGA
+	select FPGA_LOAD_SECURE
 	help
 	  Enables the fpga loads command which is used to load secure
 	  (authenticated or encrypted or both) bitstreams on to FPGA.
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index dc0b3dd31b7..262f95a252a 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -85,4 +85,18 @@ config FPGA_ZYNQPL
 	  Enable FPGA driver for loading bitstream in BIT and BIN format
 	  on Xilinx Zynq devices.
 
+config FPGA_LOAD_SECURE
+	bool "Enable loading secure bitstreams"
+	depends on FPGA
+	help
+	  Enables the fpga loads() functions that are used to load secure
+	  (authenticated or encrypted or both) bitstreams on to FPGA.
+
+config SPL_FPGA_LOAD_SECURE
+	bool "Enable loading secure bitstreams for SPL"
+	depends on FPGA
+	help
+	  Enables the fpga loads() functions that are used to load secure
+	  (authenticated or encrypted or both) bitstreams on to FPGA.
+
 endmenu
diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c
index fe3dfa12335..3b0a44b2420 100644
--- a/drivers/fpga/fpga.c
+++ b/drivers/fpga/fpga.c
@@ -220,7 +220,7 @@ int fpga_fsload(int devnum, const void *buf, size_t size,
 }
 #endif
 
-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
+#if CONFIG_IS_ENABLED(FPGA_LOAD_SECURE)
 int fpga_loads(int devnum, const void *buf, size_t size,
 	       struct fpga_secure_info *fpga_sec_info)
 {
diff --git a/drivers/fpga/xilinx.c b/drivers/fpga/xilinx.c
index cbebefb55fe..6bc1bc491fb 100644
--- a/drivers/fpga/xilinx.c
+++ b/drivers/fpga/xilinx.c
@@ -172,7 +172,7 @@ int xilinx_loadfs(xilinx_desc *desc, const void *buf, size_t bsize,
 }
 #endif
 
-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE)
+#if CONFIG_IS_ENABLED(FPGA_LOAD_SECURE)
 int xilinx_loads(xilinx_desc *desc, const void *buf, size_t bsize,
 		 struct fpga_secure_info *fpga_sec_info)
 {
diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c
index 6b394869dbf..8ff12bf50a0 100644
--- a/drivers/fpga/zynqmppl.c
+++ b/drivers/fpga/zynqmppl.c
@@ -245,7 +245,7 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize,
 	return ret;
 }
 
-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE) && !defined(CONFIG_SPL_BUILD)
+#if CONFIG_IS_ENABLED(FPGA_LOAD_SECURE)
 static int zynqmp_loads(xilinx_desc *desc, const void *buf, size_t bsize,
 			struct fpga_secure_info *fpga_sec_info)
 {
@@ -306,7 +306,7 @@ static int zynqmp_pcap_info(xilinx_desc *desc)
 
 struct xilinx_fpga_op zynqmp_op = {
 	.load = zynqmp_load,
-#if defined(CONFIG_CMD_FPGA_LOAD_SECURE) && !defined(CONFIG_SPL_BUILD)
+#if CONFIG_IS_ENABLED(FPGA_LOAD_SECURE)
 	.loads = zynqmp_loads,
 #endif
 	.info = zynqmp_pcap_info,
-- 
2.31.1

[RFC PATCH v4 2/6] fpga: add fit_fpga_load function

[Oleksandr Suvorov]

Introduce a function which passes an fpga compatible string from
FIT images to FPGA drivers. This lets the different implementations
decide how to handle it.

Some code of Jorge Ramirez-Ortiz <jorge@foundries.io> is reused.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
Tested-by: Ricardo Salveti <ricardo@foundries.io>
---

(no changes since v1)

 common/spl/spl_fit.c |  6 ++----
 drivers/fpga/fpga.c  | 35 ++++++++++++++++++++++++++++-------
 include/fpga.h       |  4 ++++
 3 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 5fe0273d66d..bd29e8c59d4 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -580,11 +580,9 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
 	compatible = fdt_getprop(ctx->fit, node, "compatible", NULL);
 	if (!compatible)
 		warn_deprecated("'fpga' image without 'compatible' property");
-	else if (strcmp(compatible, "u-boot,fpga-legacy"))
-		printf("Ignoring compatible = %s property\n", compatible);
 
-	ret = fpga_load(0, (void *)fpga_image->load_addr, fpga_image->size,
-			BIT_FULL);
+	ret = fit_fpga_load(0, (void *)fpga_image->load_addr, fpga_image->size,
+			    BIT_FULL, compatible);
 	if (ret) {
 		printf("%s: Cannot load the image to the FPGA\n", __func__);
 		return ret;
diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c
index 3b0a44b2420..2266c7d83a4 100644
--- a/drivers/fpga/fpga.c
+++ b/drivers/fpga/fpga.c
@@ -197,9 +197,9 @@ int fpga_fsload(int devnum, const void *buf, size_t size,
 		 fpga_fs_info *fpga_fsinfo)
 {
 	int ret_val = FPGA_FAIL;           /* assume failure */
-	const fpga_desc *desc = fpga_validate(devnum, buf, size,
-					      (char *)__func__);
+	const fpga_desc *desc;
 
+	desc = fpga_validate(devnum, buf, size, (char *)__func__);
 	if (desc) {
 		switch (desc->devtype) {
 		case fpga_xilinx:
@@ -225,10 +225,9 @@ int fpga_loads(int devnum, const void *buf, size_t size,
 	       struct fpga_secure_info *fpga_sec_info)
 {
 	int ret_val = FPGA_FAIL;
+	const fpga_desc *desc;
 
-	const fpga_desc *desc = fpga_validate(devnum, buf, size,
-					      (char *)__func__);
-
+	desc = fpga_validate(devnum, buf, size, (char *)__func__);
 	if (desc) {
 		switch (desc->devtype) {
 		case fpga_xilinx:
@@ -249,15 +248,31 @@ int fpga_loads(int devnum, const void *buf, size_t size,
 }
 #endif
 
+int fit_fpga_load(int devnum, const void *buf, size_t bsize,
+		  bitstream_type bstype, const char *compatible)
+{
+	fpga_desc *desc = (fpga_desc *)fpga_validate(devnum, buf, bsize,
+					      (char *)__func__);
+
+	if (!desc)
+		return FPGA_FAIL;
+	/*
+	 * Store the compatible string to proceed it in underlying
+	 * functions
+	 */
+	desc->compatible = (char *)compatible;
+
+	return fpga_load(devnum, buf, bsize, bstype);
+}
 /*
- * Generic multiplexing code
+ * Generic multiplexing code:
+ * Each architecture must handle the mandatory FPGA DT compatible property.
  */
 int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype)
 {
 	int ret_val = FPGA_FAIL;           /* assume failure */
 	const fpga_desc *desc = fpga_validate(devnum, buf, bsize,
 					      (char *)__func__);
-
 	if (desc) {
 		switch (desc->devtype) {
 		case fpga_xilinx:
@@ -270,6 +285,9 @@ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype)
 			break;
 		case fpga_altera:
 #if defined(CONFIG_FPGA_ALTERA)
+			if (strncmp(desc->compatible, "u-boot,fpga-legacy", 18))
+				printf("Ignoring compatible = %s property\n",
+				       desc->compatible);
 			ret_val = altera_load(desc->devdesc, buf, bsize);
 #else
 			fpga_no_sup((char *)__func__, "Altera devices");
@@ -277,6 +295,9 @@ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype)
 			break;
 		case fpga_lattice:
 #if defined(CONFIG_FPGA_LATTICE)
+			if (strncmp(desc->compatible, "u-boot,fpga-legacy", 18))
+				printf("Ignoring compatible = %s property\n",
+				       desc->compatible);
 			ret_val = lattice_load(desc->devdesc, buf, bsize);
 #else
 			fpga_no_sup((char *)__func__, "Lattice devices");
diff --git a/include/fpga.h b/include/fpga.h
index ec5144334df..2891f321061 100644
--- a/include/fpga.h
+++ b/include/fpga.h
@@ -35,6 +35,7 @@ typedef enum {			/* typedef fpga_type */
 typedef struct {		/* typedef fpga_desc */
 	fpga_type devtype;	/* switch value to select sub-functions */
 	void *devdesc;		/* real device descriptor */
+	char *compatible;	/* device compatible string */
 } fpga_desc;			/* end, typedef fpga_desc */
 
 typedef struct {                /* typedef fpga_desc */
@@ -63,6 +64,9 @@ int fpga_add(fpga_type devtype, void *desc);
 int fpga_count(void);
 const fpga_desc *const fpga_get_desc(int devnum);
 int fpga_is_partial_data(int devnum, size_t img_len);
+/* the DT compatible property must be handled by the different FPGA archs */
+int fit_fpga_load(int devnum, const void *buf, size_t bsize,
+		  bitstream_type bstype, const char *compatible);
 int fpga_load(int devnum, const void *buf, size_t bsize,
 	      bitstream_type bstype);
 int fpga_fsload(int devnum, const void *buf, size_t size,
-- 
2.31.1

[RFC PATCH v4 3/6] fpga: xilinx: pass an address of xilinx_desc in fpga_desc

[Oleksandr Suvorov]

Pass an address of xilinx_desc pointer in an fpga_desc  to use parent
fpga_desc structure members inside a xilinx fpga driver.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
Tested-by: Ricardo Salveti <ricardo@foundries.io>
---

(no changes since v1)

 drivers/fpga/fpga.c   | 4 ++--
 drivers/fpga/xilinx.c | 4 +++-
 include/xilinx.h      | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c
index 2266c7d83a4..781ab8bbef0 100644
--- a/drivers/fpga/fpga.c
+++ b/drivers/fpga/fpga.c
@@ -277,8 +277,8 @@ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype)
 		switch (desc->devtype) {
 		case fpga_xilinx:
 #if defined(CONFIG_FPGA_XILINX)
-			ret_val = xilinx_load(desc->devdesc, buf, bsize,
-					      bstype);
+			ret_val = xilinx_load((xilinx_desc **)&desc->devdesc,
+					      buf, bsize, bstype);
 #else
 			fpga_no_sup((char *)__func__, "Xilinx devices");
 #endif
diff --git a/drivers/fpga/xilinx.c b/drivers/fpga/xilinx.c
index 6bc1bc491fb..640baac66e6 100644
--- a/drivers/fpga/xilinx.c
+++ b/drivers/fpga/xilinx.c
@@ -138,9 +138,11 @@ int fpga_loadbitstream(int devnum, char *fpgadata, size_t size,
 	return fpga_load(devnum, dataptr, swapsize, bstype);
 }
 
-int xilinx_load(xilinx_desc *desc, const void *buf, size_t bsize,
+int xilinx_load(xilinx_desc **desc_ptr, const void *buf, size_t bsize,
 		bitstream_type bstype)
 {
+	xilinx_desc *desc = *desc_ptr;
+
 	if (!xilinx_validate (desc, (char *)__FUNCTION__)) {
 		printf ("%s: Invalid device descriptor\n", __FUNCTION__);
 		return FPGA_FAIL;
diff --git a/include/xilinx.h b/include/xilinx.h
index ab4537becfa..57b0e7be113 100644
--- a/include/xilinx.h
+++ b/include/xilinx.h
@@ -58,7 +58,7 @@ struct xilinx_fpga_op {
 
 /* Generic Xilinx Functions
  *********************************************************************/
-int xilinx_load(xilinx_desc *desc, const void *image, size_t size,
+int xilinx_load(xilinx_desc **desc_ptr, const void *image, size_t size,
 		bitstream_type bstype);
 int xilinx_dump(xilinx_desc *desc, const void *buf, size_t bsize);
 int xilinx_info(xilinx_desc *desc);
-- 
2.31.1

[RFC PATCH v4 4/6] fpga: xilinx: add missed identifier names

[Oleksandr Suvorov]

Function definition arguments should also have identifier names.
Add missed ones to struct xilinx_fpga_op callbacks, unifying code.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
---

(no changes since v1)

 include/xilinx.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/xilinx.h b/include/xilinx.h
index 57b0e7be113..06ecc9a842c 100644
--- a/include/xilinx.h
+++ b/include/xilinx.h
@@ -48,12 +48,14 @@ typedef struct {		/* typedef xilinx_desc */
 } xilinx_desc;			/* end, typedef xilinx_desc */
 
 struct xilinx_fpga_op {
-	int (*load)(xilinx_desc *, const void *, size_t, bitstream_type);
-	int (*loadfs)(xilinx_desc *, const void *, size_t, fpga_fs_info *);
+	int (*load)(xilinx_desc *desc, const void *buf, size_t bsize,
+		    bitstream_type bstype);
+	int (*loadfs)(xilinx_desc *desc, const void *buf, size_t bsize,
+		      fpga_fs_info *fpga_fsinfo);
 	int (*loads)(xilinx_desc *desc, const void *buf, size_t bsize,
 		     struct fpga_secure_info *fpga_sec_info);
-	int (*dump)(xilinx_desc *, const void *, size_t);
-	int (*info)(xilinx_desc *);
+	int (*dump)(xilinx_desc *desc, const void *buf, size_t bsize);
+	int (*info)(xilinx_desc *desc);
 };
 
 /* Generic Xilinx Functions
-- 
2.31.1

[RFC PATCH v4 5/6] fpga: xilinx: pass xilinx_desc pointer address into load() ops

[Oleksandr Suvorov]

Pass an address of xilinx_desc pointer in an fpga_desc into a load()
callback of struct xilinx_fpga_op.
It allows getting parent fpga_desc structure members inside xilinx
fpga drivers.

Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
Tested-by: Ricardo Salveti <ricardo@foundries.io>
---

(no changes since v1)

 drivers/fpga/spartan2.c | 3 ++-
 drivers/fpga/spartan3.c | 3 ++-
 drivers/fpga/versalpl.c | 2 +-
 drivers/fpga/virtex2.c  | 3 ++-
 drivers/fpga/xilinx.c   | 2 +-
 drivers/fpga/zynqmppl.c | 5 +++--
 drivers/fpga/zynqpl.c   | 3 ++-
 include/xilinx.h        | 2 +-
 8 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/fpga/spartan2.c b/drivers/fpga/spartan2.c
index 3435400e58b..f40edb1747b 100644
--- a/drivers/fpga/spartan2.c
+++ b/drivers/fpga/spartan2.c
@@ -40,10 +40,11 @@ static int spartan2_ss_dump(xilinx_desc *desc, const void *buf, size_t bsize);
 
 /* ------------------------------------------------------------------------- */
 /* Spartan-II Generic Implementation */
-static int spartan2_load(xilinx_desc *desc, const void *buf, size_t bsize,
+static int spartan2_load(xilinx_desc **desc_ptr, const void *buf, size_t bsize,
 			 bitstream_type bstype)
 {
 	int ret_val = FPGA_FAIL;
+	xilinx_desc *desc = *desc_ptr;
 
 	switch (desc->iface) {
 	case slave_serial:
diff --git a/drivers/fpga/spartan3.c b/drivers/fpga/spartan3.c
index 4850c99352d..b7c1ddd40f6 100644
--- a/drivers/fpga/spartan3.c
+++ b/drivers/fpga/spartan3.c
@@ -44,10 +44,11 @@ static int spartan3_ss_dump(xilinx_desc *desc, const void *buf, size_t bsize);
 
 /* ------------------------------------------------------------------------- */
 /* Spartan-II Generic Implementation */
-static int spartan3_load(xilinx_desc *desc, const void *buf, size_t bsize,
+static int spartan3_load(xilinx_desc **desc_ptr, const void *buf, size_t bsize,
 			 bitstream_type bstype)
 {
 	int ret_val = FPGA_FAIL;
+	xilinx_desc *desc = *desc_ptr;
 
 	switch (desc->iface) {
 	case slave_serial:
diff --git a/drivers/fpga/versalpl.c b/drivers/fpga/versalpl.c
index c44a7d34557..9464cae22ee 100644
--- a/drivers/fpga/versalpl.c
+++ b/drivers/fpga/versalpl.c
@@ -26,7 +26,7 @@ static ulong versal_align_dma_buffer(ulong *buf, u32 len)
 	return (ulong)buf;
 }
 
-static int versal_load(xilinx_desc *desc, const void *buf, size_t bsize,
+static int versal_load(xilinx_desc **desc_ptr, const void *buf, size_t bsize,
 		       bitstream_type bstype)
 {
 	ulong bin_buf;
diff --git a/drivers/fpga/virtex2.c b/drivers/fpga/virtex2.c
index b3e0537bab0..d0369d320b1 100644
--- a/drivers/fpga/virtex2.c
+++ b/drivers/fpga/virtex2.c
@@ -93,10 +93,11 @@ static int virtex2_ssm_dump(xilinx_desc *desc, const void *buf, size_t bsize);
 static int virtex2_ss_load(xilinx_desc *desc, const void *buf, size_t bsize);
 static int virtex2_ss_dump(xilinx_desc *desc, const void *buf, size_t bsize);
 
-static int virtex2_load(xilinx_desc *desc, const void *buf, size_t bsize,
+static int virtex2_load(xilinx_desc **desc_ptr, const void *buf, size_t bsize,
 			bitstream_type bstype)
 {
 	int ret_val = FPGA_FAIL;
+	xilinx_desc *desc = *desc_ptr;
 
 	switch (desc->iface) {
 	case slave_serial:
diff --git a/drivers/fpga/xilinx.c b/drivers/fpga/xilinx.c
index 640baac66e6..f89ae8fe103 100644
--- a/drivers/fpga/xilinx.c
+++ b/drivers/fpga/xilinx.c
@@ -153,7 +153,7 @@ int xilinx_load(xilinx_desc **desc_ptr, const void *buf, size_t bsize,
 		return FPGA_FAIL;
 	}
 
-	return desc->operations->load(desc, buf, bsize, bstype);
+	return desc->operations->load(desc_ptr, buf, bsize, bstype);
 }
 
 #if defined(CONFIG_CMD_FPGA_LOADFS)
diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c
index 8ff12bf50a0..c7f9f4ae846 100644
--- a/drivers/fpga/zynqmppl.c
+++ b/drivers/fpga/zynqmppl.c
@@ -199,8 +199,8 @@ static int zynqmp_validate_bitstream(xilinx_desc *desc, const void *buf,
 	return 0;
 }
 
-static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize,
-		     bitstream_type bstype)
+static int zynqmp_load(xilinx_desc **desc_ptr, const void *buf, size_t bsize,
+		       bitstream_type bstype)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(u32, bsizeptr, 1);
 	u32 swap = 0;
@@ -209,6 +209,7 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize,
 	u32 buf_lo, buf_hi;
 	u32 ret_payload[PAYLOAD_ARG_CNT];
 	bool xilfpga_old = false;
+	xilinx_desc *desc = *desc_ptr;
 
 	if (zynqmp_firmware_version() <= PMUFW_V1_0) {
 		puts("WARN: PMUFW v1.0 or less is detected\n");
diff --git a/drivers/fpga/zynqpl.c b/drivers/fpga/zynqpl.c
index 2de40109a81..c5d9dbcedfd 100644
--- a/drivers/fpga/zynqpl.c
+++ b/drivers/fpga/zynqpl.c
@@ -370,11 +370,12 @@ static int zynq_validate_bitstream(xilinx_desc *desc, const void *buf,
 	return 0;
 }
 
-static int zynq_load(xilinx_desc *desc, const void *buf, size_t bsize,
+static int zynq_load(xilinx_desc **desc_ptr, const void *buf, size_t bsize,
 		     bitstream_type bstype)
 {
 	unsigned long ts; /* Timestamp */
 	u32 isr_status, swap;
+	xilinx_desc *desc = *desc_ptr;
 
 	/*
 	 * send bsize inplace of blocksize as it was not a bitstream
diff --git a/include/xilinx.h b/include/xilinx.h
index 06ecc9a842c..a7efe0e876c 100644
--- a/include/xilinx.h
+++ b/include/xilinx.h
@@ -48,7 +48,7 @@ typedef struct {		/* typedef xilinx_desc */
 } xilinx_desc;			/* end, typedef xilinx_desc */
 
 struct xilinx_fpga_op {
-	int (*load)(xilinx_desc *desc, const void *buf, size_t bsize,
+	int (*load)(xilinx_desc **desc_ptr, const void *buf, size_t bsize,
 		    bitstream_type bstype);
 	int (*loadfs)(xilinx_desc *desc, const void *buf, size_t bsize,
 		      fpga_fs_info *fpga_fsinfo);
-- 
2.31.1

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

[Oleksandr Suvorov]

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>
---

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)
+		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

Re: [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@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@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>
> ---
>
> 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

Re: [RFC PATCH v4 0/6] fpga: zynqmp: Adding support of loading authenticated images

[Michal Simek]

On 11/11/21 10:58, Oleksandr Suvorov wrote:
> 
> Note: this version of the patchset changes the interface to the
> load() callback of xilinx_fpga_op.
> 
> This patchset introduces support for the authenticated FPGA images
> on ZynqMP boards, besides that introducing common way to pass the
> compatible property to any fpga driver.
> 
> It bases on the initial work by Jorge Ramirez-Ortiz <jorge@foundries.io>
> https://patchwork.ozlabs.org/project/uboot/patch/20211015091506.2602-1-jorge@foundries.io/
> https://patchwork.ozlabs.org/project/uboot/patch/20211005111324.19749-3-jorge@foundries.io/
> 
> Changes in v4:
> - change interface to xilinx_desc->operations->load() 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.
> 
> Oleksandr Suvorov (6):
>    fpga: add option for loading FPGA secure bitstreams
>    fpga: add fit_fpga_load function
>    fpga: xilinx: pass an address of xilinx_desc in fpga_desc
>    fpga: xilinx: add missed identifier names
>    fpga: xilinx: pass xilinx_desc pointer address into load() ops
>    fpga: zynqmp: support loading authenticated images
> 
>   cmd/Kconfig                           |  3 +-
>   common/Kconfig.boot                   |  4 +--
>   common/spl/spl_fit.c                  |  6 ++--
>   doc/uImage.FIT/source_file_format.txt |  5 +++-
>   drivers/fpga/Kconfig                  | 14 +++++++++
>   drivers/fpga/fpga.c                   | 41 ++++++++++++++++++++-------
>   drivers/fpga/spartan2.c               |  3 +-
>   drivers/fpga/spartan3.c               |  3 +-
>   drivers/fpga/versalpl.c               |  2 +-
>   drivers/fpga/virtex2.c                |  3 +-
>   drivers/fpga/xilinx.c                 |  8 ++++--
>   drivers/fpga/zynqmppl.c               | 30 +++++++++++++++++---
>   drivers/fpga/zynqpl.c                 |  3 +-
>   include/fpga.h                        |  4 +++
>   include/xilinx.h                      | 12 ++++----
>   15 files changed, 106 insertions(+), 35 deletions(-)
> 

Not sure why this series was RFC. I see comment from Simon which hasn't been 
addressed yet. Can you please take a look at it and send new version?

Thanks,
Michal

Re: [RFC PATCH v4 0/6] fpga: zynqmp: Adding support of loading authenticated images

[Oleksandr Suvorov]

Hi Michal,

On Tue, Dec 14, 2021 at 3:11 PM Michal Simek <michal.simek@xilinx.com> wrote:
>
>
>
> On 11/11/21 10:58, Oleksandr Suvorov wrote:
> >
> > Note: this version of the patchset changes the interface to the
> > load() callback of xilinx_fpga_op.
> >
> > This patchset introduces support for the authenticated FPGA images
> > on ZynqMP boards, besides that introducing common way to pass the
> > compatible property to any fpga driver.
> >
> > It bases on the initial work by Jorge Ramirez-Ortiz <jorge@foundries.io>
> > https://patchwork.ozlabs.org/project/uboot/patch/20211015091506.2602-1-jorge@foundries.io/
> > https://patchwork.ozlabs.org/project/uboot/patch/20211005111324.19749-3-jorge@foundries.io/
> >
> > Changes in v4:
> > - change interface to xilinx_desc->operations->load() 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.
> >
> > Oleksandr Suvorov (6):
> >    fpga: add option for loading FPGA secure bitstreams
> >    fpga: add fit_fpga_load function
> >    fpga: xilinx: pass an address of xilinx_desc in fpga_desc
> >    fpga: xilinx: add missed identifier names
> >    fpga: xilinx: pass xilinx_desc pointer address into load() ops
> >    fpga: zynqmp: support loading authenticated images
> >
> >   cmd/Kconfig                           |  3 +-
> >   common/Kconfig.boot                   |  4 +--
> >   common/spl/spl_fit.c                  |  6 ++--
> >   doc/uImage.FIT/source_file_format.txt |  5 +++-
> >   drivers/fpga/Kconfig                  | 14 +++++++++
> >   drivers/fpga/fpga.c                   | 41 ++++++++++++++++++++-------
> >   drivers/fpga/spartan2.c               |  3 +-
> >   drivers/fpga/spartan3.c               |  3 +-
> >   drivers/fpga/versalpl.c               |  2 +-
> >   drivers/fpga/virtex2.c                |  3 +-
> >   drivers/fpga/xilinx.c                 |  8 ++++--
> >   drivers/fpga/zynqmppl.c               | 30 +++++++++++++++++---
> >   drivers/fpga/zynqpl.c                 |  3 +-
> >   include/fpga.h                        |  4 +++
> >   include/xilinx.h                      | 12 ++++----
> >   15 files changed, 106 insertions(+), 35 deletions(-)
> >
>
> Not sure why this series was RFC. I see comment from Simon which hasn't been
> addressed yet. Can you please take a look at it and send new version?

I addressed only the 1st comment from Simon, the 2nd one about making sandbox
FPGA tests work again needs more time than I have for now.

I'll send the patchset with the 1st addressed comment today.

> Thanks,
> Michal
-- 
Best regards,

Oleksandr Suvorov
Software Engineer
T: +380 63 8489656
E: oleksandr.suvorov@foundries.io
W: www.foundries.io