[PATCH v9 00/13] fpga: zynqmp: Adding support of loading authenticated images

[PATCH v9 00/13] fpga: zynqmp: Adding support of loading authenticated images

[Oleksandr Suvorov]

This patchset introduces support for the authenticated and encrypted
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 v9:
- remove an alien commit from a patchset :)

Changes in v8:
- Michal Simek's suggestions addressed:
-- introduce the compatible flags in xilinx_desc;
-- pass a binary compatible flag instead of "compatible" property to
   an FPGA driver.
- Optimize a zynqmp_load() function.

Changes in v7:
- apply Michal Simek's suggestions
  As I applied changes on Oleksandr's patches, I indicated it by
  specifying myself as co-author in the commits logs. I am not sure
  if that is the convention of marking it.

Changes in v6:
- add support for the encrypted bitfiles.

Changes in v5:
- replace ifdef with if() where it's possible.

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.

Adrian Fiergolski (1):
  fpga: zynqmp: support loading encrypted bitfiles

Oleksandr Suvorov (12):
  fpga: add option for loading FPGA secure bitstreams
  fpga: xilinx: add missed identifier names
  fpga: xilinx: add bitstream flags to driver desc
  fpga: zynqmp: add str2flags call
  fpga: add fpga_compatible2flag
  fpga: xilinx: pass compatible flags to xilinx_load()
  fpga: pass compatible flags to fpga_load()
  spl: fit: pass real compatible flags to fpga_load()
  fpga: xilinx: pass compatible flags to load() callback
  fpga: zynqmp: optimize zynqmppl_load() code
  fpga: zynqmp: add bitstream compatible checking
  fpga: zynqmp: support loading authenticated images

 boot/Kconfig                          |  4 +-
 boot/image-board.c                    |  4 +-
 cmd/Kconfig                           |  3 +-
 cmd/fpga.c                            |  8 +--
 common/spl/spl_fit.c                  | 16 +++--
 doc/uImage.FIT/source_file_format.txt |  7 +-
 drivers/fpga/Kconfig                  | 14 ++++
 drivers/fpga/fpga.c                   | 33 +++++++++-
 drivers/fpga/spartan2.c               |  2 +-
 drivers/fpga/spartan3.c               |  2 +-
 drivers/fpga/versalpl.c               |  2 +-
 drivers/fpga/virtex2.c                |  2 +-
 drivers/fpga/xilinx.c                 |  8 +--
 drivers/fpga/zynqmppl.c               | 93 ++++++++++++++++++++++-----
 drivers/fpga/zynqpl.c                 |  2 +-
 include/fpga.h                        |  4 +-
 include/versalpl.h                    |  2 +-
 include/xilinx.h                      | 19 ++++--
 include/zynqmppl.h                    |  3 +-
 19 files changed, 177 insertions(+), 51 deletions(-)

-- 
2.36.1

[PATCH v9 01/13] 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>
Co-developed-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com>
Signed-off-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com>
---

(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 69c1814d24a..76578dcb247 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -1019,8 +1019,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..6f8ef7b8dba 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 SPL_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.36.1

[PATCH v9 02/13] 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 ab4537becfa..362943bc717 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.36.1

[PATCH v9 03/13] fpga: xilinx: add bitstream flags to driver desc

[Oleksandr Suvorov]

Store a set of supported bitstream types in xilinx_desc structure.
It will be used to determine whether an FPGA image is able to be
loaded with a given driver.

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

(no changes since v1)

 include/versalpl.h | 2 +-
 include/xilinx.h   | 5 +++++
 include/zynqmppl.h | 2 +-
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/versalpl.h b/include/versalpl.h
index b94c82e6e66..26f04a2f649 100644
--- a/include/versalpl.h
+++ b/include/versalpl.h
@@ -15,6 +15,6 @@
 extern struct xilinx_fpga_op versal_op;
 
 #define XILINX_VERSAL_DESC \
-{ xilinx_versal, csu_dma, 1, &versal_op, 0, &versal_op }
+{ xilinx_versal, csu_dma, 1, &versal_op, 0, &versal_op, NULL, FPGA_LEGACY }
 
 #endif /* _VERSALPL_H_ */
diff --git a/include/xilinx.h b/include/xilinx.h
index 362943bc717..91179abe31f 100644
--- a/include/xilinx.h
+++ b/include/xilinx.h
@@ -37,6 +37,9 @@ typedef enum {			/* typedef xilinx_family */
 	max_xilinx_type		/* insert all new types before this */
 } xilinx_family;		/* end, typedef xilinx_family */
 
+/* FPGA bitstream supported types */
+#define FPGA_LEGACY			BIT(0)
+
 typedef struct {		/* typedef xilinx_desc */
 	xilinx_family family;	/* part type */
 	xilinx_iface iface;	/* interface type */
@@ -45,6 +48,7 @@ typedef struct {		/* typedef xilinx_desc */
 	int cookie;		/* implementation specific cookie */
 	struct xilinx_fpga_op *operations; /* operations */
 	char *name;		/* device name in bitstream */
+	int flags;		/* compatible flags */
 } xilinx_desc;			/* end, typedef xilinx_desc */
 
 struct xilinx_fpga_op {
@@ -56,6 +60,7 @@ struct xilinx_fpga_op {
 		     struct fpga_secure_info *fpga_sec_info);
 	int (*dump)(xilinx_desc *desc, const void *buf, size_t bsize);
 	int (*info)(xilinx_desc *desc);
+	int (*str2flag)(xilinx_desc *desc, const char *string);
 };
 
 /* Generic Xilinx Functions
diff --git a/include/zynqmppl.h b/include/zynqmppl.h
index 35cfe17d444..a6e171dcb49 100644
--- a/include/zynqmppl.h
+++ b/include/zynqmppl.h
@@ -26,6 +26,6 @@
 extern struct xilinx_fpga_op zynqmp_op;
 
 #define XILINX_ZYNQMP_DESC \
-{ xilinx_zynqmp, csu_dma, 1, &zynqmp_op, 0, &zynqmp_op }
+{ xilinx_zynqmp, csu_dma, 1, &zynqmp_op, 0, &zynqmp_op, NULL, FPGA_LEGACY }
 
 #endif /* _ZYNQMPPL_H_ */
-- 
2.36.1

[PATCH v9 04/13] fpga: zynqmp: add str2flags call

[Oleksandr Suvorov]

Add a call to convert FPGA "compatible" string to a binary flag.

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

(no changes since v1)

 drivers/fpga/zynqmppl.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c
index 8ff12bf50a0..0ffcff0c148 100644
--- a/drivers/fpga/zynqmppl.c
+++ b/drivers/fpga/zynqmppl.c
@@ -304,10 +304,19 @@ static int zynqmp_pcap_info(xilinx_desc *desc)
 	return ret;
 }
 
+static int zynqmp_str2flag(xilinx_desc *desc, const char *str)
+{
+	if (!strncmp(str, "u-boot,fpga-legacy", 18))
+		return FPGA_LEGACY;
+
+	return 0;
+}
+
 struct xilinx_fpga_op zynqmp_op = {
 	.load = zynqmp_load,
 #if CONFIG_IS_ENABLED(FPGA_LOAD_SECURE)
 	.loads = zynqmp_loads,
 #endif
 	.info = zynqmp_pcap_info,
+	.str2flag = zynqmp_str2flag,
 };
-- 
2.36.1

[PATCH v9 05/13] fpga: add fpga_compatible2flag

[Oleksandr Suvorov]

Add a "compatible" string to binary flag converter, which uses
a callback str2flag() of given FPGA driver if available.

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

(no changes since v1)

 drivers/fpga/fpga.c | 26 ++++++++++++++++++++++++++
 include/fpga.h      |  1 +
 2 files changed, 27 insertions(+)

diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c
index 3b0a44b2420..fbfdd406e3b 100644
--- a/drivers/fpga/fpga.c
+++ b/drivers/fpga/fpga.c
@@ -356,3 +356,29 @@ int fpga_info(int devnum)
 
 	return fpga_dev_info(devnum);
 }
+
+int fpga_compatible2flag(int devnum, const char *compatible)
+{
+	const fpga_desc * const desc = fpga_get_desc(devnum);
+
+	if (!desc)
+		return FPGA_FAIL;
+
+	switch (desc->devtype) {
+	case fpga_xilinx:
+#if defined(CONFIG_FPGA_XILINX)
+	{
+		xilinx_desc *xdesc = (xilinx_desc *)desc->devdesc;
+
+		if (xdesc->operations->str2flag)
+			return xdesc->operations->str2flag(xdesc, compatible);
+	}
+#else
+		return FPGA_FAIL;
+#endif
+	default:
+		break;
+	}
+
+	return 0;
+}
diff --git a/include/fpga.h b/include/fpga.h
index ec5144334df..2172b0d015e 100644
--- a/include/fpga.h
+++ b/include/fpga.h
@@ -75,5 +75,6 @@ int fpga_dump(int devnum, const void *buf, size_t bsize);
 int fpga_info(int devnum);
 const fpga_desc *const fpga_validate(int devnum, const void *buf,
 				     size_t bsize, char *fn);
+int fpga_compatible2flag(int devnum, const char *compatible);
 
 #endif	/* _FPGA_H_ */
-- 
2.36.1

[PATCH v9 06/13] fpga: xilinx: pass compatible flags to xilinx_load()

[Oleksandr Suvorov]

This flag is used to check whether a Xilinx FPGA driver is able to
load a particular FPGA bitstream image.

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

(no changes since v1)

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

diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c
index fbfdd406e3b..f12c985b831 100644
--- a/drivers/fpga/fpga.c
+++ b/drivers/fpga/fpga.c
@@ -263,7 +263,7 @@ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype)
 		case fpga_xilinx:
 #if defined(CONFIG_FPGA_XILINX)
 			ret_val = xilinx_load(desc->devdesc, buf, bsize,
-					      bstype);
+					      bstype, 0);
 #else
 			fpga_no_sup((char *)__func__, "Xilinx devices");
 #endif
diff --git a/drivers/fpga/xilinx.c b/drivers/fpga/xilinx.c
index 6bc1bc491fb..5dd721575ec 100644
--- a/drivers/fpga/xilinx.c
+++ b/drivers/fpga/xilinx.c
@@ -139,7 +139,7 @@ int fpga_loadbitstream(int devnum, char *fpgadata, size_t size,
 }
 
 int xilinx_load(xilinx_desc *desc, const void *buf, size_t bsize,
-		bitstream_type bstype)
+		bitstream_type bstype, int flags)
 {
 	if (!xilinx_validate (desc, (char *)__FUNCTION__)) {
 		printf ("%s: Invalid device descriptor\n", __FUNCTION__);
diff --git a/include/xilinx.h b/include/xilinx.h
index 91179abe31f..a9e68138169 100644
--- a/include/xilinx.h
+++ b/include/xilinx.h
@@ -66,7 +66,7 @@ struct xilinx_fpga_op {
 /* Generic Xilinx Functions
  *********************************************************************/
 int xilinx_load(xilinx_desc *desc, const void *image, size_t size,
-		bitstream_type bstype);
+		bitstream_type bstype, int flags);
 int xilinx_dump(xilinx_desc *desc, const void *buf, size_t bsize);
 int xilinx_info(xilinx_desc *desc);
 int xilinx_loadfs(xilinx_desc *desc, const void *buf, size_t bsize,
-- 
2.36.1

[PATCH v9 07/13] fpga: pass compatible flags to fpga_load()

[Oleksandr Suvorov]

These flags may be used to check whether an FPGA driver is able to
load a particular FPGA bitstream image.

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

(no changes since v1)

 boot/image-board.c    | 4 ++--
 cmd/fpga.c            | 8 ++++----
 common/spl/spl_fit.c  | 6 ++++--
 drivers/fpga/fpga.c   | 5 +++--
 drivers/fpga/xilinx.c | 2 +-
 include/fpga.h        | 2 +-
 6 files changed, 15 insertions(+), 12 deletions(-)

diff --git a/boot/image-board.c b/boot/image-board.c
index 0d2e0fc9692..fbd95e4a77c 100644
--- a/boot/image-board.c
+++ b/boot/image-board.c
@@ -707,14 +707,14 @@ int boot_get_fpga(int argc, char *const argv[], bootm_headers_t *images,
 						 img_len, BIT_FULL);
 			if (err)
 				err = fpga_load(devnum, (const void *)img_data,
-						img_len, BIT_FULL);
+						img_len, BIT_FULL, 0);
 		} else {
 			name = "partial";
 			err = fpga_loadbitstream(devnum, (char *)img_data,
 						 img_len, BIT_PARTIAL);
 			if (err)
 				err = fpga_load(devnum, (const void *)img_data,
-						img_len, BIT_PARTIAL);
+						img_len, BIT_PARTIAL, 0);
 		}
 
 		if (err)
diff --git a/cmd/fpga.c b/cmd/fpga.c
index 3fdd0b35e80..c4651dd403e 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -178,7 +178,7 @@ static int do_fpga_load(struct cmd_tbl *cmdtp, int flag, int argc,
 	if (ret)
 		return ret;
 
-	return fpga_load(dev, (void *)fpga_data, data_size, BIT_FULL);
+	return fpga_load(dev, (void *)fpga_data, data_size, BIT_FULL, 0);
 }
 
 static int do_fpga_loadb(struct cmd_tbl *cmdtp, int flag, int argc,
@@ -209,7 +209,7 @@ static int do_fpga_loadp(struct cmd_tbl *cmdtp, int flag, int argc,
 	if (ret)
 		return ret;
 
-	return fpga_load(dev, (void *)fpga_data, data_size, BIT_PARTIAL);
+	return fpga_load(dev, (void *)fpga_data, data_size, BIT_PARTIAL, 0);
 }
 #endif
 
@@ -315,7 +315,7 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc,
 			data_size = image_get_data_size(hdr);
 		}
 		return fpga_load(dev, (void *)data, data_size,
-				  BIT_FULL);
+				  BIT_FULL, 0);
 	}
 #endif
 #if defined(CONFIG_FIT)
@@ -355,7 +355,7 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc,
 			return CMD_RET_FAILURE;
 		}
 
-		return fpga_load(dev, fit_data, data_size, BIT_FULL);
+		return fpga_load(dev, fit_data, data_size, BIT_FULL, 0);
 	}
 #endif
 	default:
diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 1bbf824684a..3c5a91916cc 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -581,6 +581,8 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
 {
 	const char *compatible;
 	int ret;
+	int devnum = 0;
+	int flags = 0;
 
 	debug("FPGA bitstream at: %x, size: %x\n",
 	      (u32)fpga_image->load_addr, fpga_image->size);
@@ -591,8 +593,8 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
 	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 = fpga_load(devnum, (void *)fpga_image->load_addr,
+			fpga_image->size, BIT_FULL, flags);
 	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 f12c985b831..11c5acab642 100644
--- a/drivers/fpga/fpga.c
+++ b/drivers/fpga/fpga.c
@@ -252,7 +252,8 @@ int fpga_loads(int devnum, const void *buf, size_t size,
 /*
  * Generic multiplexing code
  */
-int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype)
+int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype,
+	      int flags)
 {
 	int ret_val = FPGA_FAIL;           /* assume failure */
 	const fpga_desc *desc = fpga_validate(devnum, buf, bsize,
@@ -263,7 +264,7 @@ int fpga_load(int devnum, const void *buf, size_t bsize, bitstream_type bstype)
 		case fpga_xilinx:
 #if defined(CONFIG_FPGA_XILINX)
 			ret_val = xilinx_load(desc->devdesc, buf, bsize,
-					      bstype, 0);
+					      bstype, flags);
 #else
 			fpga_no_sup((char *)__func__, "Xilinx devices");
 #endif
diff --git a/drivers/fpga/xilinx.c b/drivers/fpga/xilinx.c
index 5dd721575ec..d9951ca3ecf 100644
--- a/drivers/fpga/xilinx.c
+++ b/drivers/fpga/xilinx.c
@@ -135,7 +135,7 @@ int fpga_loadbitstream(int devnum, char *fpgadata, size_t size,
 	dataptr += 4;
 	printf("  bytes in bitstream = %d\n", swapsize);
 
-	return fpga_load(devnum, dataptr, swapsize, bstype);
+	return fpga_load(devnum, dataptr, swapsize, bstype, 0);
 }
 
 int xilinx_load(xilinx_desc *desc, const void *buf, size_t bsize,
diff --git a/include/fpga.h b/include/fpga.h
index 2172b0d015e..13b1bbee3ca 100644
--- a/include/fpga.h
+++ b/include/fpga.h
@@ -64,7 +64,7 @@ int fpga_count(void);
 const fpga_desc *const fpga_get_desc(int devnum);
 int fpga_is_partial_data(int devnum, size_t img_len);
 int fpga_load(int devnum, const void *buf, size_t bsize,
-	      bitstream_type bstype);
+	      bitstream_type bstype, int flags);
 int fpga_fsload(int devnum, const void *buf, size_t size,
 		fpga_fs_info *fpga_fsinfo);
 int fpga_loads(int devnum, const void *buf, size_t size,
-- 
2.36.1

[PATCH v9 08/13] spl: fit: pass real compatible flags to fpga_load()

[Oleksandr Suvorov]

Convert taken FPGA image "compatible" string to a binary compatible
flag and pass it to an FPGA driver.

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

(no changes since v1)

 common/spl/spl_fit.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 3c5a91916cc..4769b6f29bd 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -588,10 +588,14 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
 	      (u32)fpga_image->load_addr, fpga_image->size);
 
 	compatible = fdt_getprop(ctx->fit, node, "compatible", NULL);
-	if (!compatible)
+	if (!compatible) {
 		warn_deprecated("'fpga' image without 'compatible' property");
-	else if (strcmp(compatible, "u-boot,fpga-legacy"))
-		printf("Ignoring compatible = %s property\n", compatible);
+	} else {
+		flags = fpga_compatible2flag(devnum, compatible);
+		if (strcmp(compatible, "u-boot,fpga-legacy"))
+			printf("Ignoring compatible = %s property\n",
+			       compatible);
+	}
 
 	ret = fpga_load(devnum, (void *)fpga_image->load_addr,
 			fpga_image->size, BIT_FULL, flags);
-- 
2.36.1

[PATCH v9 09/13] fpga: xilinx: pass compatible flags to load() callback

[Oleksandr Suvorov]

These flags may be used to check whether an FPGA driver is able to
load a particular FPGA bitstream image.

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

(no changes since v1)

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

diff --git a/drivers/fpga/spartan2.c b/drivers/fpga/spartan2.c
index 3435400e58b..328740f3f35 100644
--- a/drivers/fpga/spartan2.c
+++ b/drivers/fpga/spartan2.c
@@ -41,7 +41,7 @@ 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,
-			 bitstream_type bstype)
+			 bitstream_type bstype, int flags)
 {
 	int ret_val = FPGA_FAIL;
 
diff --git a/drivers/fpga/spartan3.c b/drivers/fpga/spartan3.c
index 4850c99352d..918f6db5065 100644
--- a/drivers/fpga/spartan3.c
+++ b/drivers/fpga/spartan3.c
@@ -45,7 +45,7 @@ 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,
-			 bitstream_type bstype)
+			 bitstream_type bstype, int flags)
 {
 	int ret_val = FPGA_FAIL;
 
diff --git a/drivers/fpga/versalpl.c b/drivers/fpga/versalpl.c
index c44a7d34557..d3876a8f541 100644
--- a/drivers/fpga/versalpl.c
+++ b/drivers/fpga/versalpl.c
@@ -27,7 +27,7 @@ static ulong versal_align_dma_buffer(ulong *buf, u32 len)
 }
 
 static int versal_load(xilinx_desc *desc, const void *buf, size_t bsize,
-		       bitstream_type bstype)
+		       bitstream_type bstype, int flags)
 {
 	ulong bin_buf;
 	int ret;
diff --git a/drivers/fpga/virtex2.c b/drivers/fpga/virtex2.c
index b3e0537bab0..83b90298cad 100644
--- a/drivers/fpga/virtex2.c
+++ b/drivers/fpga/virtex2.c
@@ -94,7 +94,7 @@ 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,
-			bitstream_type bstype)
+			bitstream_type bstype, int flags)
 {
 	int ret_val = FPGA_FAIL;
 
diff --git a/drivers/fpga/xilinx.c b/drivers/fpga/xilinx.c
index d9951ca3ecf..8170c3368ef 100644
--- a/drivers/fpga/xilinx.c
+++ b/drivers/fpga/xilinx.c
@@ -151,7 +151,7 @@ int xilinx_load(xilinx_desc *desc, const void *buf, size_t bsize,
 		return FPGA_FAIL;
 	}
 
-	return desc->operations->load(desc, buf, bsize, bstype);
+	return desc->operations->load(desc, buf, bsize, bstype, flags);
 }
 
 #if defined(CONFIG_CMD_FPGA_LOADFS)
diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c
index 0ffcff0c148..239c498f7b5 100644
--- a/drivers/fpga/zynqmppl.c
+++ b/drivers/fpga/zynqmppl.c
@@ -200,7 +200,7 @@ static int zynqmp_validate_bitstream(xilinx_desc *desc, const void *buf,
 }
 
 static int zynqmp_load(xilinx_desc *desc, const void *buf, size_t bsize,
-		     bitstream_type bstype)
+		     bitstream_type bstype, int flags)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(u32, bsizeptr, 1);
 	u32 swap = 0;
diff --git a/drivers/fpga/zynqpl.c b/drivers/fpga/zynqpl.c
index 2de40109a81..d8ebd542abd 100644
--- a/drivers/fpga/zynqpl.c
+++ b/drivers/fpga/zynqpl.c
@@ -371,7 +371,7 @@ static int zynq_validate_bitstream(xilinx_desc *desc, const void *buf,
 }
 
 static int zynq_load(xilinx_desc *desc, const void *buf, size_t bsize,
-		     bitstream_type bstype)
+		     bitstream_type bstype, int flags)
 {
 	unsigned long ts; /* Timestamp */
 	u32 isr_status, swap;
diff --git a/include/xilinx.h b/include/xilinx.h
index a9e68138169..89a12818311 100644
--- a/include/xilinx.h
+++ b/include/xilinx.h
@@ -53,7 +53,7 @@ typedef struct {		/* typedef xilinx_desc */
 
 struct xilinx_fpga_op {
 	int (*load)(xilinx_desc *desc, const void *buf, size_t bsize,
-		    bitstream_type bstype);
+		    bitstream_type bstype, int flags);
 	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,
-- 
2.36.1

[PATCH v9 10/13] fpga: zynqmp: optimize zynqmppl_load() code

[Oleksandr Suvorov]

Optimize function code preparing to add secure bitstream types
support.

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

(no changes since v1)

 drivers/fpga/zynqmppl.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c
index 239c498f7b5..6959b8ae97e 100644
--- a/drivers/fpga/zynqmppl.c
+++ b/drivers/fpga/zynqmppl.c
@@ -199,46 +199,45 @@ 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, int flags)
+static int zynqmp_load(xilinx_desc *desc, const void *buf,
+		       size_t bsize, bitstream_type bstype,
+		       int flags)
 {
 	ALLOC_CACHE_ALIGN_BUFFER(u32, bsizeptr, 1);
 	u32 swap = 0;
 	ulong bin_buf;
 	int ret;
 	u32 buf_lo, buf_hi;
+	u32 bsize_req = (u32)bsize;
 	u32 ret_payload[PAYLOAD_ARG_CNT];
-	bool xilfpga_old = false;
+
+	debug("%s called!\n", __func__);
 
 	if (zynqmp_firmware_version() <= PMUFW_V1_0) {
 		puts("WARN: PMUFW v1.0 or less is detected\n");
 		puts("WARN: Not all bitstream formats are supported\n");
 		puts("WARN: Please upgrade PMUFW\n");
-		xilfpga_old = true;
-		if (zynqmp_validate_bitstream(desc, buf, bsize, bsize, &swap))
+		if (zynqmp_validate_bitstream(desc, buf, bsize,
+					      bsize, &swap))
 			return FPGA_FAIL;
 		bsizeptr = (u32 *)&bsize;
 		flush_dcache_range((ulong)bsizeptr,
 				   (ulong)bsizeptr + sizeof(size_t));
+		bsize_req = (u32)(uintptr_t)bsizeptr;
 		bstype |= BIT(ZYNQMP_FPGA_BIT_NS);
+	} else {
+		bstype = 0;
 	}
 
 	bin_buf = zynqmp_align_dma_buffer((u32 *)buf, bsize, swap);
 
-	debug("%s called!\n", __func__);
 	flush_dcache_range(bin_buf, bin_buf + bsize);
 
 	buf_lo = (u32)bin_buf;
 	buf_hi = upper_32_bits(bin_buf);
 
-	if (xilfpga_old)
-		ret = xilinx_pm_request(PM_FPGA_LOAD, buf_lo,
-					buf_hi, (u32)(uintptr_t)bsizeptr,
-					bstype, ret_payload);
-	else
-		ret = xilinx_pm_request(PM_FPGA_LOAD, buf_lo,
-					buf_hi, (u32)bsize, 0, ret_payload);
-
+	ret = xilinx_pm_request(PM_FPGA_LOAD, buf_lo, buf_hi,
+				bsize_req, bstype, ret_payload);
 	if (ret)
 		printf("PL FPGA LOAD failed with err: 0x%08x\n", ret);
 
-- 
2.36.1

[PATCH v9 11/13] fpga: zynqmp: add bitstream compatible checking

[Oleksandr Suvorov]

Check whether the FPGA ZynqMP driver supports the given bitstream
image type.

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

(no changes since v1)

 drivers/fpga/zynqmppl.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/fpga/zynqmppl.c b/drivers/fpga/zynqmppl.c
index 6959b8ae97e..3dacb10e11f 100644
--- a/drivers/fpga/zynqmppl.c
+++ b/drivers/fpga/zynqmppl.c
@@ -199,6 +199,28 @@ static int zynqmp_validate_bitstream(xilinx_desc *desc, const void *buf,
 	return 0;
 }
 
+static int zynqmp_check_compatible(xilinx_desc *desc, int flags)
+{
+	/* If no flags set, the image is legacy */
+	if (!flags)
+		return 0;
+
+	/* For legacy bitstream images no need other methods exist */
+	if ((flags & desc->flags) && flags == FPGA_LEGACY)
+		return 0;
+
+	/*
+	 * Other images are handled in secure callback loads(). Check
+	 * callback existence besides image type support.
+	 */
+	if (CONFIG_IS_ENABLED(FPGA_LOAD_SECURE) &&
+	    desc->operations->loads &&
+	    (flags & desc->flags))
+		return 0;
+
+	return FPGA_FAIL;
+}
+
 static int zynqmp_load(xilinx_desc *desc, const void *buf,
 		       size_t bsize, bitstream_type bstype,
 		       int flags)
@@ -213,6 +235,11 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf,
 
 	debug("%s called!\n", __func__);
 
+	if (zynqmp_check_compatible(desc, flags)) {
+		puts("Missing loads operation or unsupported bitstream type\n");
+		return FPGA_FAIL;
+	}
+
 	if (zynqmp_firmware_version() <= PMUFW_V1_0) {
 		puts("WARN: PMUFW v1.0 or less is detected\n");
 		puts("WARN: Not all bitstream formats are supported\n");
-- 
2.36.1

[PATCH v9 12/13] 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>
---

(no changes since v1)

 boot/Kconfig                          |  4 ++--
 doc/uImage.FIT/source_file_format.txt |  5 ++++-
 drivers/fpga/zynqmppl.c               | 18 ++++++++++++++++++
 include/xilinx.h                      |  1 +
 include/zynqmppl.h                    |  3 ++-
 5 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/boot/Kconfig b/boot/Kconfig
index dff4d23b887..4a06d35acc0 100644
--- a/boot/Kconfig
+++ b/boot/Kconfig
@@ -210,8 +210,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 3dacb10e11f..76efc4b4a90 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>
@@ -232,6 +233,7 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf,
 	u32 buf_lo, buf_hi;
 	u32 bsize_req = (u32)bsize;
 	u32 ret_payload[PAYLOAD_ARG_CNT];
+	struct fpga_secure_info info = { 0 };
 
 	debug("%s called!\n", __func__);
 
@@ -240,6 +242,19 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf,
 		return FPGA_FAIL;
 	}
 
+	switch (flags) {
+	case FPGA_LEGACY:
+		break;	/* Handle the legacy image later in this function */
+	case FPGA_XILINX_ZYNQMP_DDRAUTH:
+		/* DDR authentication */
+		info.authflag = ZYNQMP_FPGA_AUTH_DDR;
+		info.encflag = FPGA_NO_ENC_OR_NO_AUTH;
+		return desc->operations->loads(desc, buf, bsize, &info);
+	default:
+		puts("Unsupported bitstream type\n");
+		return FPGA_FAIL;
+	}
+
 	if (zynqmp_firmware_version() <= PMUFW_V1_0) {
 		puts("WARN: PMUFW v1.0 or less is detected\n");
 		puts("WARN: Not all bitstream formats are supported\n");
@@ -335,6 +350,9 @@ static int zynqmp_str2flag(xilinx_desc *desc, const char *str)
 	if (!strncmp(str, "u-boot,fpga-legacy", 18))
 		return FPGA_LEGACY;
 
+	if (!strncmp(str, "u-boot,zynqmp-fpga-ddrauth", 26))
+		return FPGA_XILINX_ZYNQMP_DDRAUTH;
+
 	return 0;
 }
 
diff --git a/include/xilinx.h b/include/xilinx.h
index 89a12818311..ffd95ad7225 100644
--- a/include/xilinx.h
+++ b/include/xilinx.h
@@ -39,6 +39,7 @@ typedef enum {			/* typedef xilinx_family */
 
 /* FPGA bitstream supported types */
 #define FPGA_LEGACY			BIT(0)
+#define FPGA_XILINX_ZYNQMP_DDRAUTH	BIT(1)
 
 typedef struct {		/* typedef xilinx_desc */
 	xilinx_family family;	/* part type */
diff --git a/include/zynqmppl.h b/include/zynqmppl.h
index a6e171dcb49..c4d7a41220d 100644
--- a/include/zynqmppl.h
+++ b/include/zynqmppl.h
@@ -26,6 +26,7 @@
 extern struct xilinx_fpga_op zynqmp_op;
 
 #define XILINX_ZYNQMP_DESC \
-{ xilinx_zynqmp, csu_dma, 1, &zynqmp_op, 0, &zynqmp_op, NULL, FPGA_LEGACY }
+{ xilinx_zynqmp, csu_dma, 1, &zynqmp_op, 0, &zynqmp_op, NULL, \
+	(FPGA_LEGACY | FPGA_XILINX_ZYNQMP_DDRAUTH) }
 
 #endif /* _ZYNQMPPL_H_ */
-- 
2.36.1

[PATCH v9 13/13] fpga: zynqmp: support loading encrypted bitfiles

[Oleksandr Suvorov]

From: Adrian Fiergolski <adrian.fiergolski@fastree3d.com>

Add supporting new compatible string "u-boot,zynqmp-fpga-enc" to handle
loading encrypted bitfiles.

This feature requires encrypted FSBL,as according to UG1085:
"The CSU automatically locks out the AES key, stored in either BBRAM or eFUSEs,
 as a key source to the AES engine if the FSBL is not encrypted. This prevents
 using the BBRAM or eFUSE as the key source to the AES engine during run-time
 applications."

Signed-off-by: Adrian Fiergolski <adrian.fiergolski@fastree3d.com>
Co-developed-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
---

Changes in v9:
- remove an alien commit from a patchset :)

Changes in v8:
- Michal Simek's suggestions addressed:
-- introduce the compatible flags in xilinx_desc;
-- pass a binary compatible flag instead of "compatible" property to
   an FPGA driver.
- Optimize a zynqmp_load() function.

Changes in v7:
- apply Michal Simek's suggestions
  As I applied changes on Oleksandr's patches, I indicated it by
  specifying myself as co-author in the commits logs. I am not sure
  if that is the convention of marking it.

Changes in v6:
- add support for the encrypted bitfiles.

Changes in v5:
- replace ifdef with if() where it's possible.

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.

 doc/uImage.FIT/source_file_format.txt | 2 ++
 drivers/fpga/zynqmppl.c               | 8 ++++++++
 include/fpga.h                        | 1 +
 include/xilinx.h                      | 1 +
 include/zynqmppl.h                    | 2 +-
 5 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/doc/uImage.FIT/source_file_format.txt b/doc/uImage.FIT/source_file_format.txt
index 461e2af2a84..68701118409 100644
--- a/doc/uImage.FIT/source_file_format.txt
+++ b/doc/uImage.FIT/source_file_format.txt
@@ -188,6 +188,8 @@ the '/images' node should have the following layout:
     "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.
+    "u-boot,zynqmp-fpga-enc" - encrypted FPGA bitstream for Xilinx Zynq
+    UltraScale+ (ZynqMP) 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 76efc4b4a90..9087909dfe5 100644
--- a/drivers/fpga/zynqmppl.c
+++ b/drivers/fpga/zynqmppl.c
@@ -250,6 +250,11 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf,
 		info.authflag = ZYNQMP_FPGA_AUTH_DDR;
 		info.encflag = FPGA_NO_ENC_OR_NO_AUTH;
 		return desc->operations->loads(desc, buf, bsize, &info);
+	case FPGA_XILINX_ZYNQMP_ENC:
+		/* Encryption using device key */
+		info.authflag = FPGA_NO_ENC_OR_NO_AUTH;
+		info.encflag = FPGA_ENC_DEV_KEY;
+		return desc->operations->loads(desc, buf, bsize, &info);
 	default:
 		puts("Unsupported bitstream type\n");
 		return FPGA_FAIL;
@@ -353,6 +358,9 @@ static int zynqmp_str2flag(xilinx_desc *desc, const char *str)
 	if (!strncmp(str, "u-boot,zynqmp-fpga-ddrauth", 26))
 		return FPGA_XILINX_ZYNQMP_DDRAUTH;
 
+	if (!strncmp(str, "u-boot,zynqmp-fpga-enc", 22))
+		return FPGA_XILINX_ZYNQMP_ENC;
+
 	return 0;
 }
 
diff --git a/include/fpga.h b/include/fpga.h
index 13b1bbee3ca..a4e16401da7 100644
--- a/include/fpga.h
+++ b/include/fpga.h
@@ -20,6 +20,7 @@
 /* device numbers must be non-negative */
 #define FPGA_INVALID_DEVICE	-1
 
+#define FPGA_ENC_DEV_KEY	0
 #define FPGA_ENC_USR_KEY	1
 #define FPGA_NO_ENC_OR_NO_AUTH	2
 
diff --git a/include/xilinx.h b/include/xilinx.h
index ffd95ad7225..a62f6fd074f 100644
--- a/include/xilinx.h
+++ b/include/xilinx.h
@@ -40,6 +40,7 @@ typedef enum {			/* typedef xilinx_family */
 /* FPGA bitstream supported types */
 #define FPGA_LEGACY			BIT(0)
 #define FPGA_XILINX_ZYNQMP_DDRAUTH	BIT(1)
+#define FPGA_XILINX_ZYNQMP_ENC		BIT(2)
 
 typedef struct {		/* typedef xilinx_desc */
 	xilinx_family family;	/* part type */
diff --git a/include/zynqmppl.h b/include/zynqmppl.h
index c4d7a41220d..44d6933f82b 100644
--- a/include/zynqmppl.h
+++ b/include/zynqmppl.h
@@ -27,6 +27,6 @@ extern struct xilinx_fpga_op zynqmp_op;
 
 #define XILINX_ZYNQMP_DESC \
 { xilinx_zynqmp, csu_dma, 1, &zynqmp_op, 0, &zynqmp_op, NULL, \
-	(FPGA_LEGACY | FPGA_XILINX_ZYNQMP_DDRAUTH) }
+	(FPGA_LEGACY | FPGA_XILINX_ZYNQMP_DDRAUTH | FPGA_XILINX_ZYNQMP_ENC) }
 
 #endif /* _ZYNQMPPL_H_ */
-- 
2.36.1

Re: [PATCH v9 00/13] fpga: zynqmp: Adding support of loading authenticated images

[Oleksandr Suvorov]

Adrian, I don't have access to the ZynqMP hardware for now, so could
you please test this patchset?

On Wed, Jun 1, 2022 at 11:46 AM Oleksandr Suvorov
<oleksandr.suvorov@foundries.io> wrote:
>
>
> This patchset introduces support for the authenticated and encrypted
> 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 v9:
> - remove an alien commit from a patchset :)
>
> Changes in v8:
> - Michal Simek's suggestions addressed:
> -- introduce the compatible flags in xilinx_desc;
> -- pass a binary compatible flag instead of "compatible" property to
>    an FPGA driver.
> - Optimize a zynqmp_load() function.
>
> Changes in v7:
> - apply Michal Simek's suggestions
>   As I applied changes on Oleksandr's patches, I indicated it by
>   specifying myself as co-author in the commits logs. I am not sure
>   if that is the convention of marking it.
>
> Changes in v6:
> - add support for the encrypted bitfiles.
>
> Changes in v5:
> - replace ifdef with if() where it's possible.
>
> 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.
>
> Adrian Fiergolski (1):
>   fpga: zynqmp: support loading encrypted bitfiles
>
> Oleksandr Suvorov (12):
>   fpga: add option for loading FPGA secure bitstreams
>   fpga: xilinx: add missed identifier names
>   fpga: xilinx: add bitstream flags to driver desc
>   fpga: zynqmp: add str2flags call
>   fpga: add fpga_compatible2flag
>   fpga: xilinx: pass compatible flags to xilinx_load()
>   fpga: pass compatible flags to fpga_load()
>   spl: fit: pass real compatible flags to fpga_load()
>   fpga: xilinx: pass compatible flags to load() callback
>   fpga: zynqmp: optimize zynqmppl_load() code
>   fpga: zynqmp: add bitstream compatible checking
>   fpga: zynqmp: support loading authenticated images
>
>  boot/Kconfig                          |  4 +-
>  boot/image-board.c                    |  4 +-
>  cmd/Kconfig                           |  3 +-
>  cmd/fpga.c                            |  8 +--
>  common/spl/spl_fit.c                  | 16 +++--
>  doc/uImage.FIT/source_file_format.txt |  7 +-
>  drivers/fpga/Kconfig                  | 14 ++++
>  drivers/fpga/fpga.c                   | 33 +++++++++-
>  drivers/fpga/spartan2.c               |  2 +-
>  drivers/fpga/spartan3.c               |  2 +-
>  drivers/fpga/versalpl.c               |  2 +-
>  drivers/fpga/virtex2.c                |  2 +-
>  drivers/fpga/xilinx.c                 |  8 +--
>  drivers/fpga/zynqmppl.c               | 93 ++++++++++++++++++++++-----
>  drivers/fpga/zynqpl.c                 |  2 +-
>  include/fpga.h                        |  4 +-
>  include/versalpl.h                    |  2 +-
>  include/xilinx.h                      | 19 ++++--
>  include/zynqmppl.h                    |  3 +-
>  19 files changed, 177 insertions(+), 51 deletions(-)
>
> --
> 2.36.1
>


-- 
Best regards
Oleksandr

Oleksandr Suvorov
cryosay@gmail.com

Re: [PATCH v9 03/13] fpga: xilinx: add bitstream flags to driver desc

[Michal Simek]

On 6/1/22 10:46, Oleksandr Suvorov wrote:
> Store a set of supported bitstream types in xilinx_desc structure.
> It will be used to determine whether an FPGA image is able to be
> loaded with a given driver.
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> ---
> 
> (no changes since v1)
> 
>   include/versalpl.h | 2 +-
>   include/xilinx.h   | 5 +++++
>   include/zynqmppl.h | 2 +-
>   3 files changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/include/versalpl.h b/include/versalpl.h
> index b94c82e6e66..26f04a2f649 100644
> --- a/include/versalpl.h
> +++ b/include/versalpl.h
> @@ -15,6 +15,6 @@
>   extern struct xilinx_fpga_op versal_op;
>   
>   #define XILINX_VERSAL_DESC \
> -{ xilinx_versal, csu_dma, 1, &versal_op, 0, &versal_op }
> +{ xilinx_versal, csu_dma, 1, &versal_op, 0, &versal_op, NULL, FPGA_LEGACY }
>   
>   #endif /* _VERSALPL_H_ */
> diff --git a/include/xilinx.h b/include/xilinx.h
> index 362943bc717..91179abe31f 100644
> --- a/include/xilinx.h
> +++ b/include/xilinx.h
> @@ -37,6 +37,9 @@ typedef enum {			/* typedef xilinx_family */
>   	max_xilinx_type		/* insert all new types before this */
>   } xilinx_family;		/* end, typedef xilinx_family */
>   
> +/* FPGA bitstream supported types */
> +#define FPGA_LEGACY			BIT(0)
> +
>   typedef struct {		/* typedef xilinx_desc */
>   	xilinx_family family;	/* part type */
>   	xilinx_iface iface;	/* interface type */
> @@ -45,6 +48,7 @@ typedef struct {		/* typedef xilinx_desc */
>   	int cookie;		/* implementation specific cookie */
>   	struct xilinx_fpga_op *operations; /* operations */
>   	char *name;		/* device name in bitstream */
> +	int flags;		/* compatible flags */
>   } xilinx_desc;			/* end, typedef xilinx_desc */
>   
>   struct xilinx_fpga_op {
> @@ -56,6 +60,7 @@ struct xilinx_fpga_op {
>   		     struct fpga_secure_info *fpga_sec_info);
>   	int (*dump)(xilinx_desc *desc, const void *buf, size_t bsize);
>   	int (*info)(xilinx_desc *desc);
> +	int (*str2flag)(xilinx_desc *desc, const char *string);

This should be in 4/13.

M

Re: [PATCH v9 03/13] fpga: xilinx: add bitstream flags to driver desc

[Oleksandr Suvorov]

Hi Michal,

On Tue, Jun 7, 2022 at 2:32 PM Michal Simek <michal.simek@xilinx.com> wrote:
>
>
>
> On 6/1/22 10:46, Oleksandr Suvorov wrote:
> > Store a set of supported bitstream types in xilinx_desc structure.
> > It will be used to determine whether an FPGA image is able to be
> > loaded with a given driver.
> >
> > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> > ---
> >
> > (no changes since v1)
> >
> >   include/versalpl.h | 2 +-
> >   include/xilinx.h   | 5 +++++
> >   include/zynqmppl.h | 2 +-
> >   3 files changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/versalpl.h b/include/versalpl.h
> > index b94c82e6e66..26f04a2f649 100644
> > --- a/include/versalpl.h
> > +++ b/include/versalpl.h
> > @@ -15,6 +15,6 @@
> >   extern struct xilinx_fpga_op versal_op;
> >
> >   #define XILINX_VERSAL_DESC \
> > -{ xilinx_versal, csu_dma, 1, &versal_op, 0, &versal_op }
> > +{ xilinx_versal, csu_dma, 1, &versal_op, 0, &versal_op, NULL, FPGA_LEGACY }
> >
> >   #endif /* _VERSALPL_H_ */
> > diff --git a/include/xilinx.h b/include/xilinx.h
> > index 362943bc717..91179abe31f 100644
> > --- a/include/xilinx.h
> > +++ b/include/xilinx.h
> > @@ -37,6 +37,9 @@ typedef enum {                      /* typedef xilinx_family */
> >       max_xilinx_type         /* insert all new types before this */
> >   } xilinx_family;            /* end, typedef xilinx_family */
> >
> > +/* FPGA bitstream supported types */
> > +#define FPGA_LEGACY                  BIT(0)
> > +
> >   typedef struct {            /* typedef xilinx_desc */
> >       xilinx_family family;   /* part type */
> >       xilinx_iface iface;     /* interface type */
> > @@ -45,6 +48,7 @@ typedef struct {            /* typedef xilinx_desc */
> >       int cookie;             /* implementation specific cookie */
> >       struct xilinx_fpga_op *operations; /* operations */
> >       char *name;             /* device name in bitstream */
> > +     int flags;              /* compatible flags */
> >   } xilinx_desc;                      /* end, typedef xilinx_desc */
> >
> >   struct xilinx_fpga_op {
> > @@ -56,6 +60,7 @@ struct xilinx_fpga_op {
> >                    struct fpga_secure_info *fpga_sec_info);
> >       int (*dump)(xilinx_desc *desc, const void *buf, size_t bsize);
> >       int (*info)(xilinx_desc *desc);
> > +     int (*str2flag)(xilinx_desc *desc, const char *string);
>
> This should be in 4/13.
>
> M

Thanks, Michal! Agree. I'll post the fixed patchset after receiving
the testing feedback from Ricardo and Adrian.

-- 
Best regards
Oleksandr

Oleksandr Suvorov
cryosay@gmail.com

Re: [PATCH v9 03/13] fpga: xilinx: add bitstream flags to driver desc

[Michal Simek]

On 6/1/22 10:46, Oleksandr Suvorov wrote:
> Store a set of supported bitstream types in xilinx_desc structure.
> It will be used to determine whether an FPGA image is able to be
> loaded with a given driver.
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> ---
> 
> (no changes since v1)
> 
>   include/versalpl.h | 2 +-
>   include/xilinx.h   | 5 +++++
>   include/zynqmppl.h | 2 +-

And to be more accurate you should also extend configuration for zynq.
FPGA_LEGACY is bit 1 and you are missing it here.


  20 #if CONFIG_IS_ENABLED(FPGA)
  21 xilinx_desc fpga = {
  22         .family = xilinx_zynq,
  23         .iface = devcfg,
  24         .operations = &zynq_op,
  25 };
  26 #endif

Thanks,
Michal

Re: [PATCH v9 05/13] fpga: add fpga_compatible2flag

[Michal Simek]

On 6/1/22 10:46, Oleksandr Suvorov wrote:
> Add a "compatible" string to binary flag converter, which uses
> a callback str2flag() of given FPGA driver if available.
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> ---
> 
> (no changes since v1)
> 
>   drivers/fpga/fpga.c | 26 ++++++++++++++++++++++++++
>   include/fpga.h      |  1 +
>   2 files changed, 27 insertions(+)
> 
> diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c
> index 3b0a44b2420..fbfdd406e3b 100644
> --- a/drivers/fpga/fpga.c
> +++ b/drivers/fpga/fpga.c
> @@ -356,3 +356,29 @@ int fpga_info(int devnum)
>   
>   	return fpga_dev_info(devnum);
>   }
> +
> +int fpga_compatible2flag(int devnum, const char *compatible)
> +{
> +	const fpga_desc * const desc = fpga_get_desc(devnum);
> +
> +	if (!desc)
> +		return FPGA_FAIL;
> +
> +	switch (desc->devtype) {
> +	case fpga_xilinx:
> +#if defined(CONFIG_FPGA_XILINX)
> +	{
> +		xilinx_desc *xdesc = (xilinx_desc *)desc->devdesc;
> +
> +		if (xdesc->operations->str2flag)
> +			return xdesc->operations->str2flag(xdesc, compatible);

This function is returning FPGA_FAIL or 0 (FPGA_SUCCESS) but str2flag is 
returning based on 4/14 flags.

It means you are mixing two things here together.

Thanks,
Michal

Re: [PATCH v9 08/13] spl: fit: pass real compatible flags to fpga_load()

[Michal Simek]

On 6/1/22 10:46, Oleksandr Suvorov wrote:
> Convert taken FPGA image "compatible" string to a binary compatible
> flag and pass it to an FPGA driver.
> 
> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> ---
> 
> (no changes since v1)
> 
>   common/spl/spl_fit.c | 10 +++++++---
>   1 file changed, 7 insertions(+), 3 deletions(-)
> 
> diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> index 3c5a91916cc..4769b6f29bd 100644
> --- a/common/spl/spl_fit.c
> +++ b/common/spl/spl_fit.c
> @@ -588,10 +588,14 @@ static int spl_fit_upload_fpga(struct spl_fit_info *ctx, int node,
>   	      (u32)fpga_image->load_addr, fpga_image->size);
>   
>   	compatible = fdt_getprop(ctx->fit, node, "compatible", NULL);
> -	if (!compatible)
> +	if (!compatible) {
>   		warn_deprecated("'fpga' image without 'compatible' property");
> -	else if (strcmp(compatible, "u-boot,fpga-legacy"))
> -		printf("Ignoring compatible = %s property\n", compatible);
> +	} else {
> +		flags = fpga_compatible2flag(devnum, compatible);
> +		if (strcmp(compatible, "u-boot,fpga-legacy"))
> +			printf("Ignoring compatible = %s property\n",
> +			       compatible);

Please make this message as debug. It just consumes space and likely hit only 
when you develop things. And message itself is just informative anyway

M

Re: [PATCH v9 12/13] fpga: zynqmp: support loading authenticated images

[Michal Simek]

On 6/1/22 10:46, Oleksandr Suvorov 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>
> ---
> 
> (no changes since v1)
> 
>   boot/Kconfig                          |  4 ++--
>   doc/uImage.FIT/source_file_format.txt |  5 ++++-
>   drivers/fpga/zynqmppl.c               | 18 ++++++++++++++++++
>   include/xilinx.h                      |  1 +
>   include/zynqmppl.h                    |  3 ++-
>   5 files changed, 27 insertions(+), 4 deletions(-)
> 
> diff --git a/boot/Kconfig b/boot/Kconfig
> index dff4d23b887..4a06d35acc0 100644
> --- a/boot/Kconfig
> +++ b/boot/Kconfig
> @@ -210,8 +210,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 3dacb10e11f..76efc4b4a90 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>
> @@ -232,6 +233,7 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf,
>   	u32 buf_lo, buf_hi;
>   	u32 bsize_req = (u32)bsize;
>   	u32 ret_payload[PAYLOAD_ARG_CNT];
> +	struct fpga_secure_info info = { 0 };
>   
>   	debug("%s called!\n", __func__);
>   
> @@ -240,6 +242,19 @@ static int zynqmp_load(xilinx_desc *desc, const void *buf,
>   		return FPGA_FAIL;
>   	}
>   
> +	switch (flags) {
> +	case FPGA_LEGACY:
> +		break;	/* Handle the legacy image later in this function */
> +	case FPGA_XILINX_ZYNQMP_DDRAUTH:
> +		/* DDR authentication */
> +		info.authflag = ZYNQMP_FPGA_AUTH_DDR;
> +		info.encflag = FPGA_NO_ENC_OR_NO_AUTH;
> +		return desc->operations->loads(desc, buf, bsize, &info);

In case of FPGA_LOAD_SECURE is not enabled I would expect this code won't be 
even here. The same for 13/13 for another type.




> +	default:
> +		puts("Unsupported bitstream type\n");
> +		return FPGA_FAIL;
> +	}
> +
>   	if (zynqmp_firmware_version() <= PMUFW_V1_0) {
>   		puts("WARN: PMUFW v1.0 or less is detected\n");
>   		puts("WARN: Not all bitstream formats are supported\n");
> @@ -335,6 +350,9 @@ static int zynqmp_str2flag(xilinx_desc *desc, const char *str)
>   	if (!strncmp(str, "u-boot,fpga-legacy", 18))
>   		return FPGA_LEGACY;
>   
> +	if (!strncmp(str, "u-boot,zynqmp-fpga-ddrauth", 26))
> +		return FPGA_XILINX_ZYNQMP_DDRAUTH;
> +

The same here. When FPGA_LOAD_SECURE is disabled u-boot shouldn't 
bother/understand these secure compatible strings.

>   	return 0;
>   }
>   
> diff --git a/include/xilinx.h b/include/xilinx.h
> index 89a12818311..ffd95ad7225 100644
> --- a/include/xilinx.h
> +++ b/include/xilinx.h
> @@ -39,6 +39,7 @@ typedef enum {			/* typedef xilinx_family */
>   
>   /* FPGA bitstream supported types */
>   #define FPGA_LEGACY			BIT(0)
> +#define FPGA_XILINX_ZYNQMP_DDRAUTH	BIT(1)
>   
>   typedef struct {		/* typedef xilinx_desc */
>   	xilinx_family family;	/* part type */
> diff --git a/include/zynqmppl.h b/include/zynqmppl.h
> index a6e171dcb49..c4d7a41220d 100644
> --- a/include/zynqmppl.h
> +++ b/include/zynqmppl.h
> @@ -26,6 +26,7 @@
>   extern struct xilinx_fpga_op zynqmp_op;
>   
>   #define XILINX_ZYNQMP_DESC \
> -{ xilinx_zynqmp, csu_dma, 1, &zynqmp_op, 0, &zynqmp_op, NULL, FPGA_LEGACY }
> +{ xilinx_zynqmp, csu_dma, 1, &zynqmp_op, 0, &zynqmp_op, NULL, \
> +	(FPGA_LEGACY | FPGA_XILINX_ZYNQMP_DDRAUTH) }

The same here. You can move this macro directly to code and put there some 
macros around. When secure options is disabled you shouldn't publish that 
support is there via flags.

M


>   
>   #endif /* _ZYNQMPPL_H_ */

Re: [PATCH v9 03/13] fpga: xilinx: add bitstream flags to driver desc

[Michal Simek]

Hi,

On 6/7/22 13:37, Oleksandr Suvorov wrote:
> Hi Michal,
> 
> On Tue, Jun 7, 2022 at 2:32 PM Michal Simek <michal.simek@xilinx.com> wrote:
>>
>>
>>
>> On 6/1/22 10:46, Oleksandr Suvorov wrote:
>>> Store a set of supported bitstream types in xilinx_desc structure.
>>> It will be used to determine whether an FPGA image is able to be
>>> loaded with a given driver.
>>>
>>> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
>>> ---
>>>
>>> (no changes since v1)
>>>
>>>    include/versalpl.h | 2 +-
>>>    include/xilinx.h   | 5 +++++
>>>    include/zynqmppl.h | 2 +-
>>>    3 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/versalpl.h b/include/versalpl.h
>>> index b94c82e6e66..26f04a2f649 100644
>>> --- a/include/versalpl.h
>>> +++ b/include/versalpl.h
>>> @@ -15,6 +15,6 @@
>>>    extern struct xilinx_fpga_op versal_op;
>>>
>>>    #define XILINX_VERSAL_DESC \
>>> -{ xilinx_versal, csu_dma, 1, &versal_op, 0, &versal_op }
>>> +{ xilinx_versal, csu_dma, 1, &versal_op, 0, &versal_op, NULL, FPGA_LEGACY }
>>>
>>>    #endif /* _VERSALPL_H_ */
>>> diff --git a/include/xilinx.h b/include/xilinx.h
>>> index 362943bc717..91179abe31f 100644
>>> --- a/include/xilinx.h
>>> +++ b/include/xilinx.h
>>> @@ -37,6 +37,9 @@ typedef enum {                      /* typedef xilinx_family */
>>>        max_xilinx_type         /* insert all new types before this */
>>>    } xilinx_family;            /* end, typedef xilinx_family */
>>>
>>> +/* FPGA bitstream supported types */
>>> +#define FPGA_LEGACY                  BIT(0)
>>> +
>>>    typedef struct {            /* typedef xilinx_desc */
>>>        xilinx_family family;   /* part type */
>>>        xilinx_iface iface;     /* interface type */
>>> @@ -45,6 +48,7 @@ typedef struct {            /* typedef xilinx_desc */
>>>        int cookie;             /* implementation specific cookie */
>>>        struct xilinx_fpga_op *operations; /* operations */
>>>        char *name;             /* device name in bitstream */
>>> +     int flags;              /* compatible flags */
>>>    } xilinx_desc;                      /* end, typedef xilinx_desc */
>>>
>>>    struct xilinx_fpga_op {
>>> @@ -56,6 +60,7 @@ struct xilinx_fpga_op {
>>>                     struct fpga_secure_info *fpga_sec_info);
>>>        int (*dump)(xilinx_desc *desc, const void *buf, size_t bsize);
>>>        int (*info)(xilinx_desc *desc);
>>> +     int (*str2flag)(xilinx_desc *desc, const char *string);
>>
>> This should be in 4/13.
>>
>> M
> 
> Thanks, Michal! Agree. I'll post the fixed patchset after receiving
> the testing feedback from Ricardo and Adrian.

ok. I think series in general going in right direction based on current fpga 
framework style. Would be better to move it to DM but none is pushing you to do it.
What needs to be fixed are that return values where flags and fpga status values 
are mixed together. And making sure that secure options are available only when 
secure configuration is enabled.

Thanks,
Michal

Re: [PATCH v9 00/13] fpga: zynqmp: Adding support of loading authenticated images

[Adrian Fiergolski]

Oleksandr,

yes, I will try to find some time slot to test them. However, don't you 
want to first implement Michael's suggestions, so I test the final version?

Regards,
Adrian

On 02.06.2022 17:11, Oleksandr Suvorov wrote:
> Adrian, I don't have access to the ZynqMP hardware for now, so could
> you please test this patchset?
>
> On Wed, Jun 1, 2022 at 11:46 AM Oleksandr Suvorov
> <oleksandr.suvorov@foundries.io> wrote:
>>
>> This patchset introduces support for the authenticated and encrypted
>> 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 v9:
>> - remove an alien commit from a patchset :)
>>
>> Changes in v8:
>> - Michal Simek's suggestions addressed:
>> -- introduce the compatible flags in xilinx_desc;
>> -- pass a binary compatible flag instead of "compatible" property to
>>     an FPGA driver.
>> - Optimize a zynqmp_load() function.
>>
>> Changes in v7:
>> - apply Michal Simek's suggestions
>>    As I applied changes on Oleksandr's patches, I indicated it by
>>    specifying myself as co-author in the commits logs. I am not sure
>>    if that is the convention of marking it.
>>
>> Changes in v6:
>> - add support for the encrypted bitfiles.
>>
>> Changes in v5:
>> - replace ifdef with if() where it's possible.
>>
>> 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.
>>
>> Adrian Fiergolski (1):
>>    fpga: zynqmp: support loading encrypted bitfiles
>>
>> Oleksandr Suvorov (12):
>>    fpga: add option for loading FPGA secure bitstreams
>>    fpga: xilinx: add missed identifier names
>>    fpga: xilinx: add bitstream flags to driver desc
>>    fpga: zynqmp: add str2flags call
>>    fpga: add fpga_compatible2flag
>>    fpga: xilinx: pass compatible flags to xilinx_load()
>>    fpga: pass compatible flags to fpga_load()
>>    spl: fit: pass real compatible flags to fpga_load()
>>    fpga: xilinx: pass compatible flags to load() callback
>>    fpga: zynqmp: optimize zynqmppl_load() code
>>    fpga: zynqmp: add bitstream compatible checking
>>    fpga: zynqmp: support loading authenticated images
>>
>>   boot/Kconfig                          |  4 +-
>>   boot/image-board.c                    |  4 +-
>>   cmd/Kconfig                           |  3 +-
>>   cmd/fpga.c                            |  8 +--
>>   common/spl/spl_fit.c                  | 16 +++--
>>   doc/uImage.FIT/source_file_format.txt |  7 +-
>>   drivers/fpga/Kconfig                  | 14 ++++
>>   drivers/fpga/fpga.c                   | 33 +++++++++-
>>   drivers/fpga/spartan2.c               |  2 +-
>>   drivers/fpga/spartan3.c               |  2 +-
>>   drivers/fpga/versalpl.c               |  2 +-
>>   drivers/fpga/virtex2.c                |  2 +-
>>   drivers/fpga/xilinx.c                 |  8 +--
>>   drivers/fpga/zynqmppl.c               | 93 ++++++++++++++++++++++-----
>>   drivers/fpga/zynqpl.c                 |  2 +-
>>   include/fpga.h                        |  4 +-
>>   include/versalpl.h                    |  2 +-
>>   include/xilinx.h                      | 19 ++++--
>>   include/zynqmppl.h                    |  3 +-
>>   19 files changed, 177 insertions(+), 51 deletions(-)
>>
>> --
>> 2.36.1
>>
>

Re: [PATCH v9 03/13] fpga: xilinx: add bitstream flags to driver desc

[Oleksandr Suvorov]

Hi Michal,

On Tue, Jun 7, 2022 at 3:47 PM Michal Simek <michal.simek@xilinx.com> wrote:
>
> Hi,
>
> On 6/7/22 13:37, Oleksandr Suvorov wrote:
> > Hi Michal,
> >
> > On Tue, Jun 7, 2022 at 2:32 PM Michal Simek <michal.simek@xilinx.com> wrote:
> >>
> >>
> >>
> >> On 6/1/22 10:46, Oleksandr Suvorov wrote:
> >>> Store a set of supported bitstream types in xilinx_desc structure.
> >>> It will be used to determine whether an FPGA image is able to be
> >>> loaded with a given driver.
> >>>
> >>> Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> >>> ---
> >>>
> >>> (no changes since v1)
> >>>
> >>>    include/versalpl.h | 2 +-
> >>>    include/xilinx.h   | 5 +++++
> >>>    include/zynqmppl.h | 2 +-
> >>>    3 files changed, 7 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/include/versalpl.h b/include/versalpl.h
> >>> index b94c82e6e66..26f04a2f649 100644
> >>> --- a/include/versalpl.h
> >>> +++ b/include/versalpl.h
> >>> @@ -15,6 +15,6 @@
> >>>    extern struct xilinx_fpga_op versal_op;
> >>>
> >>>    #define XILINX_VERSAL_DESC \
> >>> -{ xilinx_versal, csu_dma, 1, &versal_op, 0, &versal_op }
> >>> +{ xilinx_versal, csu_dma, 1, &versal_op, 0, &versal_op, NULL, FPGA_LEGACY }
> >>>
> >>>    #endif /* _VERSALPL_H_ */
> >>> diff --git a/include/xilinx.h b/include/xilinx.h
> >>> index 362943bc717..91179abe31f 100644
> >>> --- a/include/xilinx.h
> >>> +++ b/include/xilinx.h
> >>> @@ -37,6 +37,9 @@ typedef enum {                      /* typedef xilinx_family */
> >>>        max_xilinx_type         /* insert all new types before this */
> >>>    } xilinx_family;            /* end, typedef xilinx_family */
> >>>
> >>> +/* FPGA bitstream supported types */
> >>> +#define FPGA_LEGACY                  BIT(0)
> >>> +
> >>>    typedef struct {            /* typedef xilinx_desc */
> >>>        xilinx_family family;   /* part type */
> >>>        xilinx_iface iface;     /* interface type */
> >>> @@ -45,6 +48,7 @@ typedef struct {            /* typedef xilinx_desc */
> >>>        int cookie;             /* implementation specific cookie */
> >>>        struct xilinx_fpga_op *operations; /* operations */
> >>>        char *name;             /* device name in bitstream */
> >>> +     int flags;              /* compatible flags */
> >>>    } xilinx_desc;                      /* end, typedef xilinx_desc */
> >>>
> >>>    struct xilinx_fpga_op {
> >>> @@ -56,6 +60,7 @@ struct xilinx_fpga_op {
> >>>                     struct fpga_secure_info *fpga_sec_info);
> >>>        int (*dump)(xilinx_desc *desc, const void *buf, size_t bsize);
> >>>        int (*info)(xilinx_desc *desc);
> >>> +     int (*str2flag)(xilinx_desc *desc, const char *string);
> >>
> >> This should be in 4/13.
> >>
> >> M
> >
> > Thanks, Michal! Agree. I'll post the fixed patchset after receiving
> > the testing feedback from Ricardo and Adrian.
>
> ok. I think series in general going in right direction based on current fpga
> framework style. Would be better to move it to DM but none is pushing you to do it.

Yeah, I started adding the DM version, but it needs way extra time,
I'm not sure I'll be
able to complete it even in a month (not enough free time for that).

> What needs to be fixed are that return values where flags and fpga status values
> are mixed together. And making sure that secure options are available only when
> secure configuration is enabled.

Ok, thanks, I'll revise these points.

>
> Thanks,
> Michal



-- 
Best regards
Oleksandr

Oleksandr Suvorov
cryosay@gmail.com

Re: [PATCH v9 00/13] fpga: zynqmp: Adding support of loading authenticated images

[Oleksandr Suvorov]

Hi Adrian,

On Tue, Jun 7, 2022 at 4:01 PM Adrian Fiergolski
<adrian.fiergolski@fastree3d.com> wrote:
>
> Oleksandr,
>
> yes, I will try to find some time slot to test them. However, don't you
> want to first implement Michael's suggestions, so I test the final version?

Sure, no problem, Adrian.

>
> Regards,
> Adrian
>
> On 02.06.2022 17:11, Oleksandr Suvorov wrote:
> > Adrian, I don't have access to the ZynqMP hardware for now, so could
> > you please test this patchset?
> >
> > On Wed, Jun 1, 2022 at 11:46 AM Oleksandr Suvorov
> > <oleksandr.suvorov@foundries.io> wrote:
> >>
> >> This patchset introduces support for the authenticated and encrypted
> >> 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 v9:
> >> - remove an alien commit from a patchset :)
> >>
> >> Changes in v8:
> >> - Michal Simek's suggestions addressed:
> >> -- introduce the compatible flags in xilinx_desc;
> >> -- pass a binary compatible flag instead of "compatible" property to
> >>     an FPGA driver.
> >> - Optimize a zynqmp_load() function.
> >>
> >> Changes in v7:
> >> - apply Michal Simek's suggestions
> >>    As I applied changes on Oleksandr's patches, I indicated it by
> >>    specifying myself as co-author in the commits logs. I am not sure
> >>    if that is the convention of marking it.
> >>
> >> Changes in v6:
> >> - add support for the encrypted bitfiles.
> >>
> >> Changes in v5:
> >> - replace ifdef with if() where it's possible.
> >>
> >> 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.
> >>
> >> Adrian Fiergolski (1):
> >>    fpga: zynqmp: support loading encrypted bitfiles
> >>
> >> Oleksandr Suvorov (12):
> >>    fpga: add option for loading FPGA secure bitstreams
> >>    fpga: xilinx: add missed identifier names
> >>    fpga: xilinx: add bitstream flags to driver desc
> >>    fpga: zynqmp: add str2flags call
> >>    fpga: add fpga_compatible2flag
> >>    fpga: xilinx: pass compatible flags to xilinx_load()
> >>    fpga: pass compatible flags to fpga_load()
> >>    spl: fit: pass real compatible flags to fpga_load()
> >>    fpga: xilinx: pass compatible flags to load() callback
> >>    fpga: zynqmp: optimize zynqmppl_load() code
> >>    fpga: zynqmp: add bitstream compatible checking
> >>    fpga: zynqmp: support loading authenticated images
> >>
> >>   boot/Kconfig                          |  4 +-
> >>   boot/image-board.c                    |  4 +-
> >>   cmd/Kconfig                           |  3 +-
> >>   cmd/fpga.c                            |  8 +--
> >>   common/spl/spl_fit.c                  | 16 +++--
> >>   doc/uImage.FIT/source_file_format.txt |  7 +-
> >>   drivers/fpga/Kconfig                  | 14 ++++
> >>   drivers/fpga/fpga.c                   | 33 +++++++++-
> >>   drivers/fpga/spartan2.c               |  2 +-
> >>   drivers/fpga/spartan3.c               |  2 +-
> >>   drivers/fpga/versalpl.c               |  2 +-
> >>   drivers/fpga/virtex2.c                |  2 +-
> >>   drivers/fpga/xilinx.c                 |  8 +--
> >>   drivers/fpga/zynqmppl.c               | 93 ++++++++++++++++++++++-----
> >>   drivers/fpga/zynqpl.c                 |  2 +-
> >>   include/fpga.h                        |  4 +-
> >>   include/versalpl.h                    |  2 +-
> >>   include/xilinx.h                      | 19 ++++--
> >>   include/zynqmppl.h                    |  3 +-
> >>   19 files changed, 177 insertions(+), 51 deletions(-)
> >>
> >> --
> >> 2.36.1
> >>Ok, thanks, I'll revise these points.

> >



-- 
Best regards
Oleksandr

Oleksandr Suvorov
cryosay@gmail.com

Re: [PATCH v9 05/13] fpga: add fpga_compatible2flag

[Oleksandr Suvorov]

On Tue, Jun 7, 2022 at 3:11 PM Michal Simek <michal.simek@xilinx.com> wrote:
>
>
>
> On 6/1/22 10:46, Oleksandr Suvorov wrote:
> > Add a "compatible" string to binary flag converter, which uses
> > a callback str2flag() of given FPGA driver if available.
> >
> > Signed-off-by: Oleksandr Suvorov <oleksandr.suvorov@foundries.io>
> > ---
> >
> > (no changes since v1)
> >
> >   drivers/fpga/fpga.c | 26 ++++++++++++++++++++++++++
> >   include/fpga.h      |  1 +
> >   2 files changed, 27 insertions(+)
> >
> > diff --git a/drivers/fpga/fpga.c b/drivers/fpga/fpga.c
> > index 3b0a44b2420..fbfdd406e3b 100644
> > --- a/drivers/fpga/fpga.c
> > +++ b/drivers/fpga/fpga.c
> > @@ -356,3 +356,29 @@ int fpga_info(int devnum)
> >
> >       return fpga_dev_info(devnum);
> >   }
> > +
> > +int fpga_compatible2flag(int devnum, const char *compatible)
> > +{
> > +     const fpga_desc * const desc = fpga_get_desc(devnum);
> > +
> > +     if (!desc)
> > +             return FPGA_FAIL;
> > +
> > +     switch (desc->devtype) {
> > +     case fpga_xilinx:
> > +#if defined(CONFIG_FPGA_XILINX)
> > +     {
> > +             xilinx_desc *xdesc = (xilinx_desc *)desc->devdesc;
> > +
> > +             if (xdesc->operations->str2flag)
> > +                     return xdesc->operations->str2flag(xdesc, compatible);
>
> This function is returning FPGA_FAIL or 0 (FPGA_SUCCESS) but str2flag is
> returning based on 4/14 flags.
>
> It means you are mixing two things here together.

Omg, thanks, missed that.

>
> Thanks,
> Michal



-- 
Best regards
Oleksandr

Oleksandr Suvorov
cryosay@gmail.com