[PATCH 0/6] net: fm: Verify Fman microcode

[PATCH 0/6] net: fm: Verify Fman microcode

[Sean Anderson]

Surprisingly, Fman microcode does not seem to be verified. This series
aims to rectify this by introducing an optional FIT wrapper. This
wrapper is made mandatory if FIT_SIGNATURE is enabled. NXP boards do not
use this config, so the microcode will remain unverified for them. This
is OK, since we do not want to break existing systems.


Sean Anderson (6):
  ARMv8/sec_firmware: Remove SEC_FIRMWARE_FIT_CNF_NAME
  image: fit: Add some helpers for getting data
  misc: fs_loader: Add function to get the chosen loader
  net: fm: Add firmware name parameter
  net: fm: Support loading firmware from a filesystem
  net: fm: Add support for FIT firmware

 arch/arm/cpu/armv8/sec_firmware.c  | 52 +++------------------------
 arch/arm/mach-k3/common.c          |  2 +-
 arch/arm/mach-omap2/boot-common.c  |  2 +-
 boot/image-fit.c                   | 37 +++++++++++++++++++
 cmd/fpga.c                         | 24 ++++---------
 drivers/fpga/socfpga_arria10.c     | 24 ++-----------
 drivers/misc/fs_loader.c           | 27 ++++++++++++++
 drivers/net/fm/fm.c                | 58 +++++++++++++++++++++++++++---
 drivers/net/fm/fm.h                |  2 +-
 drivers/net/fm/init.c              |  4 +--
 drivers/net/fsl-mc/mc.c            | 30 ++--------------
 drivers/net/pfe_eth/pfe_firmware.c | 40 +--------------------
 drivers/qe/Kconfig                 |  4 +++
 include/fs_loader.h                | 12 +++++++
 include/image.h                    |  4 +++
 15 files changed, 159 insertions(+), 163 deletions(-)

-- 
2.25.1

[PATCH 1/6] ARMv8/sec_firmware: Remove SEC_FIRMWARE_FIT_CNF_NAME

[Sean Anderson]

The config to use for FIT images can be better specified by enabling
CONFIG_MULTI_DTB_FIT and implementing board_fit_config_name_match.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 arch/arm/cpu/armv8/sec_firmware.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
index 267894fbcb..41525a10d5 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -35,9 +35,6 @@ phys_addr_t sec_firmware_addr;
 #ifndef SEC_FIRMWARE_FIT_IMAGE
 #define SEC_FIRMWARE_FIT_IMAGE		"firmware"
 #endif
-#ifndef SEC_FIRMWARE_FIT_CNF_NAME
-#define SEC_FIRMWARE_FIT_CNF_NAME	"config-1"
-#endif
 #ifndef SEC_FIRMWARE_TARGET_EL
 #define SEC_FIRMWARE_TARGET_EL		2
 #endif
@@ -46,15 +43,12 @@ static int sec_firmware_get_data(const void *sec_firmware_img,
 				const void **data, size_t *size)
 {
 	int conf_node_off, fw_node_off;
-	char *conf_node_name = NULL;
 	char *desc;
 	int ret;
 
-	conf_node_name = SEC_FIRMWARE_FIT_CNF_NAME;
-
-	conf_node_off = fit_conf_get_node(sec_firmware_img, conf_node_name);
+	conf_node_off = fit_conf_get_node(sec_firmware_img, NULL);
 	if (conf_node_off < 0) {
-		printf("SEC Firmware: %s: no such config\n", conf_node_name);
+		puts("SEC Firmware: no config\n");
 		return -ENOENT;
 	}
 
@@ -123,18 +117,15 @@ static int sec_firmware_check_copy_loadable(const void *sec_firmware_img,
 {
 	phys_addr_t sec_firmware_loadable_addr = 0;
 	int conf_node_off, ld_node_off, images;
-	char *conf_node_name = NULL;
 	const void *data;
 	size_t size;
 	ulong load;
 	const char *name, *str, *type;
 	int len;
 
-	conf_node_name = SEC_FIRMWARE_FIT_CNF_NAME;
-
-	conf_node_off = fit_conf_get_node(sec_firmware_img, conf_node_name);
+	conf_node_off = fit_conf_get_node(sec_firmware_img, NULL);
 	if (conf_node_off < 0) {
-		printf("SEC Firmware: %s: no such config\n", conf_node_name);
+		puts("SEC Firmware: no config\n");
 		return -ENOENT;
 	}
 
-- 
2.25.1

[PATCH 2/6] image: fit: Add some helpers for getting data

[Sean Anderson]

Several different firmware users have repetitive code to extract the
firmware data from a FIT. Add some helper functions to reduce the amount
of repetition. fit_conf_get_prop_node (eventually) calls
fdt_check_node_offset_, so we can avoid an explicit if. In general, this
version avoids printing on error because the callers are typically
library functions, and because the FIT code generally has (debug)
prints of its own. One difference in these helpers is that they use
fit_image_get_data_and_size instead of fit_image_get_data, as the former
handles external data correctly.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 arch/arm/cpu/armv8/sec_firmware.c  | 39 ++---------------------------
 boot/image-fit.c                   | 37 +++++++++++++++++++++++++++
 cmd/fpga.c                         | 24 +++++-------------
 drivers/net/fsl-mc/mc.c            | 30 +++-------------------
 drivers/net/pfe_eth/pfe_firmware.c | 40 +-----------------------------
 include/image.h                    |  4 +++
 6 files changed, 53 insertions(+), 121 deletions(-)

diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
index 41525a10d5..879eb6ec81 100644
--- a/arch/arm/cpu/armv8/sec_firmware.c
+++ b/arch/arm/cpu/armv8/sec_firmware.c
@@ -42,43 +42,8 @@ phys_addr_t sec_firmware_addr;
 static int sec_firmware_get_data(const void *sec_firmware_img,
 				const void **data, size_t *size)
 {
-	int conf_node_off, fw_node_off;
-	char *desc;
-	int ret;
-
-	conf_node_off = fit_conf_get_node(sec_firmware_img, NULL);
-	if (conf_node_off < 0) {
-		puts("SEC Firmware: no config\n");
-		return -ENOENT;
-	}
-
-	fw_node_off = fit_conf_get_prop_node(sec_firmware_img, conf_node_off,
-			SEC_FIRMWARE_FIT_IMAGE);
-	if (fw_node_off < 0) {
-		printf("SEC Firmware: No '%s' in config\n",
-		       SEC_FIRMWARE_FIT_IMAGE);
-		return -ENOLINK;
-	}
-
-	/* Verify secure firmware image */
-	if (!(fit_image_verify(sec_firmware_img, fw_node_off))) {
-		printf("SEC Firmware: Bad firmware image (bad CRC)\n");
-		return -EINVAL;
-	}
-
-	if (fit_image_get_data(sec_firmware_img, fw_node_off, data, size)) {
-		printf("SEC Firmware: Can't get %s subimage data/size",
-		       SEC_FIRMWARE_FIT_IMAGE);
-		return -ENOENT;
-	}
-
-	ret = fit_get_desc(sec_firmware_img, fw_node_off, &desc);
-	if (ret)
-		printf("SEC Firmware: Can't get description\n");
-	else
-		printf("%s\n", desc);
-
-	return ret;
+	return fit_get_data_conf_prop(sec_firmware_img, SEC_FIRMWARE_FIT_IMAGE,
+				      data, size);
 }
 
 /*
diff --git a/boot/image-fit.c b/boot/image-fit.c
index 6610035d0a..b87378cbf6 100644
--- a/boot/image-fit.c
+++ b/boot/image-fit.c
@@ -1919,6 +1919,43 @@ int fit_conf_get_prop_node(const void *fit, int noffset,
 	return fit_conf_get_prop_node_index(fit, noffset, prop_name, 0);
 }
 
+static int fit_get_data_tail(const void *fit, int noffset,
+			     const void **data, size_t *size)
+{
+	char *desc;
+
+	if (noffset < 0)
+		return noffset;
+
+	if (!fit_image_verify(fit, noffset))
+		return -EINVAL;
+
+	if (fit_image_get_data_and_size(fit, noffset, data, size))
+		return -ENOENT;
+
+	if (!fit_get_desc(fit, noffset, &desc))
+		printf("%s\n", desc);
+
+	return 0;
+}
+
+int fit_get_data_node(const void *fit, const char *image_uname,
+		      const void **data, size_t *size)
+{
+	int noffset = fit_image_get_node(fit, image_uname);
+
+	return fit_get_data_tail(fit, noffset, data, size);
+}
+
+int fit_get_data_conf_prop(const void *fit, const char *prop_name,
+			   const void **data, size_t *size)
+{
+	int noffset = fit_conf_get_node(fit, NULL);
+
+	noffset = fit_conf_get_prop_node(fit, noffset, prop_name);
+	return fit_get_data_tail(fit, noffset, data, size);
+}
+
 static int fit_image_select(const void *fit, int rd_noffset, int verify)
 {
 	fit_image_print(fit, rd_noffset, "   ");
diff --git a/cmd/fpga.c b/cmd/fpga.c
index 3fdd0b35e8..1102a84d76 100644
--- a/cmd/fpga.c
+++ b/cmd/fpga.c
@@ -322,7 +322,7 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc,
 	case IMAGE_FORMAT_FIT:
 	{
 		const void *fit_hdr = (const void *)fpga_data;
-		int noffset;
+		int err;
 		const void *fit_data;
 
 		if (!fit_uname) {
@@ -335,23 +335,11 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc,
 			return CMD_RET_FAILURE;
 		}
 
-		/* get fpga component image node offset */
-		noffset = fit_image_get_node(fit_hdr, fit_uname);
-		if (noffset < 0) {
-			printf("Can't find '%s' FIT subimage\n", fit_uname);
-			return CMD_RET_FAILURE;
-		}
-
-		/* verify integrity */
-		if (!fit_image_verify(fit_hdr, noffset)) {
-			puts("Bad Data Hash\n");
-			return CMD_RET_FAILURE;
-		}
-
-		/* get fpga subimage/external data address and length */
-		if (fit_image_get_data_and_size(fit_hdr, noffset,
-					       &fit_data, &data_size)) {
-			puts("Fpga subimage data not found\n");
+		err = fit_get_data_node(fit_hdr, fit_uname, &fit_data,
+					&data_size);
+		if (err) {
+			printf("Could not load '%s' subimage (err %d)\n",
+			       fit_uname, err);
 			return CMD_RET_FAILURE;
 		}
 
diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c
index bc1c31d467..68833f9ddd 100644
--- a/drivers/net/fsl-mc/mc.c
+++ b/drivers/net/fsl-mc/mc.c
@@ -137,13 +137,7 @@ int parse_mc_firmware_fit_image(u64 mc_fw_addr,
 				size_t *raw_image_size)
 {
 	int format;
-	void *fit_hdr;
-	int node_offset;
-	const void *data;
-	size_t size;
-	const char *uname = "firmware";
-
-	fit_hdr = (void *)mc_fw_addr;
+	void *fit_hdr = (void *)mc_fw_addr;
 
 	/* Check if Image is in FIT format */
 	format = genimg_get_format(fit_hdr);
@@ -158,26 +152,8 @@ int parse_mc_firmware_fit_image(u64 mc_fw_addr,
 		return -EINVAL;
 	}
 
-	node_offset = fit_image_get_node(fit_hdr, uname);
-
-	if (node_offset < 0) {
-		printf("fsl-mc: ERR: Bad firmware image (missing subimage)\n");
-		return -ENOENT;
-	}
-
-	/* Verify MC firmware image */
-	if (!(fit_image_verify(fit_hdr, node_offset))) {
-		printf("fsl-mc: ERR: Bad firmware image (bad CRC)\n");
-		return -EINVAL;
-	}
-
-	/* Get address and size of raw image */
-	fit_image_get_data(fit_hdr, node_offset, &data, &size);
-
-	*raw_image_addr = data;
-	*raw_image_size = size;
-
-	return 0;
+	return fit_get_data_node(fit_hdr, "firmware", raw_image_addr,
+				 raw_image_size);
 }
 #endif
 
diff --git a/drivers/net/pfe_eth/pfe_firmware.c b/drivers/net/pfe_eth/pfe_firmware.c
index 6669048181..adaa139219 100644
--- a/drivers/net/pfe_eth/pfe_firmware.c
+++ b/drivers/net/pfe_eth/pfe_firmware.c
@@ -104,45 +104,7 @@ err:
 static int pfe_get_fw(const void **data,
 		      size_t *size, char *fw_name)
 {
-	int conf_node_off, fw_node_off;
-	char *conf_node_name = NULL;
-	char *desc;
-	int ret = 0;
-
-	conf_node_name = PFE_FIRMWARE_FIT_CNF_NAME;
-
-	conf_node_off = fit_conf_get_node(pfe_fit_addr, conf_node_name);
-	if (conf_node_off < 0) {
-		printf("PFE Firmware: %s: no such config\n", conf_node_name);
-		return -ENOENT;
-	}
-
-	fw_node_off = fit_conf_get_prop_node(pfe_fit_addr, conf_node_off,
-					     fw_name);
-	if (fw_node_off < 0) {
-		printf("PFE Firmware: No '%s' in config\n",
-		       fw_name);
-		return -ENOLINK;
-	}
-
-	if (!(fit_image_verify(pfe_fit_addr, fw_node_off))) {
-		printf("PFE Firmware: Bad firmware image (bad CRC)\n");
-		return -EINVAL;
-	}
-
-	if (fit_image_get_data(pfe_fit_addr, fw_node_off, data, size)) {
-		printf("PFE Firmware: Can't get %s subimage data/size",
-		       fw_name);
-		return -ENOENT;
-	}
-
-	ret = fit_get_desc(pfe_fit_addr, fw_node_off, &desc);
-	if (ret)
-		printf("PFE Firmware: Can't get description\n");
-	else
-		printf("%s\n", desc);
-
-	return ret;
+	return fit_get_data_conf_prop(pfe_fit_addr, fw_name, data, size);
 }
 
 /*
diff --git a/include/image.h b/include/image.h
index 97e5f2eb24..ae1f015896 100644
--- a/include/image.h
+++ b/include/image.h
@@ -1010,6 +1010,10 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
 				       size_t *data_size);
 int fit_image_get_data_and_size(const void *fit, int noffset,
 				const void **data, size_t *size);
+int fit_get_data_node(const void *fit, const char *image_uname,
+		      const void **data, size_t *size);
+int fit_get_data_conf_prop(const void *fit, const char *prop_name,
+			   const void **data, size_t *size);
 
 int fit_image_hash_get_algo(const void *fit, int noffset, const char **algo);
 int fit_image_hash_get_value(const void *fit, int noffset, uint8_t **value,
-- 
2.25.1

[PATCH 3/6] misc: fs_loader: Add function to get the chosen loader

[Sean Anderson]

The fs_loader device is used to pull in settings via the chosen node.
However, there was no library function for this, so arria10 was doing it
explicitly. This function subsumes that, and uses ofnode_get_chosen_node
instead of navigating the device tree directly. Because fs_loader pulls
its config from the environment by default, it's fine to create a device
with nothing backing it at all. Doing this allows enabling
CONFIG_FS_LOADER without needing to modify the device tree.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 arch/arm/mach-k3/common.c         |  2 +-
 arch/arm/mach-omap2/boot-common.c |  2 +-
 drivers/fpga/socfpga_arria10.c    | 24 ++----------------------
 drivers/misc/fs_loader.c          | 27 +++++++++++++++++++++++++++
 include/fs_loader.h               | 12 ++++++++++++
 5 files changed, 43 insertions(+), 24 deletions(-)

diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
index b4b75f4e6c..ec236d5a2e 100644
--- a/arch/arm/mach-k3/common.c
+++ b/arch/arm/mach-k3/common.c
@@ -181,7 +181,7 @@ int load_firmware(char *name_fw, char *name_loadaddr, u32 *loadaddr)
 	if (!*loadaddr)
 		return 0;
 
-	if (!uclass_get_device(UCLASS_FS_FIRMWARE_LOADER, 0, &fsdev)) {
+	if (!get_fs_loader(&fsdev)) {
 		size = request_firmware_into_buf(fsdev, name, (void *)*loadaddr,
 						 0, 0);
 	}
diff --git a/arch/arm/mach-omap2/boot-common.c b/arch/arm/mach-omap2/boot-common.c
index afc3585641..c82aadc7cc 100644
--- a/arch/arm/mach-omap2/boot-common.c
+++ b/arch/arm/mach-omap2/boot-common.c
@@ -212,7 +212,7 @@ int load_firmware(char *name_fw, u32 *loadaddr)
 	if (!*loadaddr)
 		return 0;
 
-	if (!uclass_get_device(UCLASS_FS_FIRMWARE_LOADER, 0, &fsdev)) {
+	if (!get_fs_loader(&fsdev)) {
 		size = request_firmware_into_buf(fsdev, name_fw,
 						 (void *)*loadaddr, 0, 0);
 	}
diff --git a/drivers/fpga/socfpga_arria10.c b/drivers/fpga/socfpga_arria10.c
index 798e3a3f90..65bebd8997 100644
--- a/drivers/fpga/socfpga_arria10.c
+++ b/drivers/fpga/socfpga_arria10.c
@@ -765,32 +765,12 @@ int socfpga_loadfs(fpga_fs_info *fpga_fsinfo, const void *buf, size_t bsize,
 	u32 phandle;
 
 	node = get_fpga_mgr_ofnode(ofnode_null());
-
-	if (ofnode_valid(node)) {
-		phandle_p = ofnode_get_property(node, "firmware-loader", &size);
-		if (!phandle_p) {
-			node = ofnode_path("/chosen");
-			if (!ofnode_valid(node)) {
-				debug("FPGA: /chosen node was not found.\n");
-				return -ENOENT;
-			}
-
-			phandle_p = ofnode_get_property(node, "firmware-loader",
-						       &size);
-			if (!phandle_p) {
-				debug("FPGA: firmware-loader property was not");
-				debug(" found.\n");
-				return -ENOENT;
-			}
-		}
-	} else {
+	if (!ofnode_valid(node)) {
 		debug("FPGA: FPGA manager node was not found.\n");
 		return -ENOENT;
 	}
 
-	phandle = fdt32_to_cpu(*phandle_p);
-	ret = uclass_get_device_by_phandle_id(UCLASS_FS_FIRMWARE_LOADER,
-					     phandle, &dev);
+	ret = get_fs_loader(&dev);
 	if (ret)
 		return ret;
 
diff --git a/drivers/misc/fs_loader.c b/drivers/misc/fs_loader.c
index 0139bd66ba..0018c930ec 100644
--- a/drivers/misc/fs_loader.c
+++ b/drivers/misc/fs_loader.c
@@ -15,6 +15,8 @@
 #include <fs_loader.h>
 #include <log.h>
 #include <asm/global_data.h>
+#include <dm/device-internal.h>
+#include <dm/root.h>
 #include <linux/string.h>
 #include <mapmem.h>
 #include <malloc.h>
@@ -293,6 +295,31 @@ U_BOOT_DRIVER(fs_loader) = {
 	.priv_auto	= sizeof(struct firmware),
 };
 
+static struct device_plat default_plat = { 0 };
+
+int get_fs_loader(struct udevice **dev)
+{
+	int ret;
+	ofnode node = ofnode_get_chosen_node("firmware-loader");
+
+	if (ofnode_valid(node))
+		return uclass_get_device_by_ofnode(UCLASS_FS_FIRMWARE_LOADER,
+						   node, dev);
+
+	/* Try the first device if none was chosen */
+	ret = uclass_first_device_err(UCLASS_FS_FIRMWARE_LOADER, dev);
+	if (ret != -ENODEV)
+		return ret;
+
+	/* Just create a new device */
+	ret = device_bind(dm_root(), DM_DRIVER_GET(fs_loader), "default-loader",
+			  &default_plat, ofnode_null(), dev);
+	if (ret)
+		return ret;
+
+	return device_probe(*dev);
+}
+
 UCLASS_DRIVER(fs_loader) = {
 	.id		= UCLASS_FS_FIRMWARE_LOADER,
 	.name		= "fs-loader",
diff --git a/include/fs_loader.h b/include/fs_loader.h
index 8de7cb18dc..5eb5b7ab4a 100644
--- a/include/fs_loader.h
+++ b/include/fs_loader.h
@@ -52,4 +52,16 @@ struct device_plat {
 int request_firmware_into_buf(struct udevice *dev,
 			      const char *name,
 			      void *buf, size_t size, u32 offset);
+
+/**
+ * get_fs_loader() - Get the chosen filesystem loader
+ * @dev: Where to store the device
+ *
+ * This gets a filesystem loader device based on the value of
+ * /chosen/firmware-loader. If no such property exists, it returns a
+ * firmware loader which is configured by environmental variables.
+ *
+ * Return: 0 on success, negative value on error
+ */
+int get_fs_loader(struct udevice **dev);
 #endif
-- 
2.25.1

[PATCH 4/6] net: fm: Add firmware name parameter

[Sean Anderson]

In order to read the firmware from the filesystem, we need a file name.
Read the firmware name from the device tree, using the firmware-name
property. This property is commonly used in Linux to determine the
correct name to use (and can be seen in several device trees in U-Boot).

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 drivers/net/fm/fm.c   | 15 ++++++++++++---
 drivers/net/fm/fm.h   |  2 +-
 drivers/net/fm/init.c |  4 ++--
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c
index f825612640..aa0cc69232 100644
--- a/drivers/net/fm/fm.c
+++ b/drivers/net/fm/fm.c
@@ -7,6 +7,7 @@
 #include <env.h>
 #include <malloc.h>
 #include <asm/io.h>
+#include <dm/device_compat.h>
 #include <linux/errno.h>
 #include <u-boot/crc.h>
 #ifdef CONFIG_DM_ETH
@@ -354,7 +355,7 @@ static void fm_init_qmi(struct fm_qmi_common *qmi)
 
 /* Init common part of FM, index is fm num# like fm as above */
 #ifdef CONFIG_TFABOOT
-int fm_init_common(int index, struct ccsr_fman *reg)
+int fm_init_common(int index, struct ccsr_fman *reg, const char *firmware_name)
 {
 	int rc;
 	void *addr = NULL;
@@ -449,7 +450,7 @@ int fm_init_common(int index, struct ccsr_fman *reg)
 	return fm_init_bmi(index, &reg->fm_bmi_common);
 }
 #else
-int fm_init_common(int index, struct ccsr_fman *reg)
+int fm_init_common(int index, struct ccsr_fman *reg, const char *firmware_name)
 {
 	int rc;
 #if defined(CONFIG_SYS_QE_FMAN_FW_IN_NOR)
@@ -546,6 +547,8 @@ static const struct udevice_id fman_ids[] = {
 
 static int fman_probe(struct udevice *dev)
 {
+	const char *firmware_name = NULL;
+	int ret;
 	struct fman_priv *priv = dev_get_priv(dev);
 
 	priv->reg = (struct ccsr_fman *)(uintptr_t)dev_read_addr(dev);
@@ -555,7 +558,13 @@ static int fman_probe(struct udevice *dev)
 		return -EINVAL;
 	}
 
-	return fm_init_common(priv->fman_id, priv->reg);
+	ret = dev_read_string_index(dev, "firmware-name", 0, &firmware_name);
+	if (ret && ret != -EINVAL) {
+		dev_dbg(dev, "Could not read firmware-name\n");
+		return ret;
+	}
+
+	return fm_init_common(priv->fman_id, priv->reg, firmware_name);
 }
 
 static int fman_remove(struct udevice *dev)
diff --git a/drivers/net/fm/fm.h b/drivers/net/fm/fm.h
index 2379b3a11c..32de5cf2b6 100644
--- a/drivers/net/fm/fm.h
+++ b/drivers/net/fm/fm.h
@@ -108,7 +108,7 @@ struct fm_port_global_pram {
 
 void *fm_muram_alloc(int fm_idx, size_t size, ulong align);
 void *fm_muram_base(int fm_idx);
-int fm_init_common(int index, struct ccsr_fman *reg);
+int fm_init_common(int index, struct ccsr_fman *reg, const char *firmware_name);
 int fm_eth_initialize(struct ccsr_fman *reg, struct fm_eth_info *info);
 phy_interface_t fman_port_enet_if(enum fm_port port);
 void fman_disable_port(enum fm_port port);
diff --git a/drivers/net/fm/init.c b/drivers/net/fm/init.c
index 2fed64205c..a9a20931a1 100644
--- a/drivers/net/fm/init.c
+++ b/drivers/net/fm/init.c
@@ -93,7 +93,7 @@ int fm_standard_init(struct bd_info *bis)
 	struct ccsr_fman *reg;
 
 	reg = (void *)CONFIG_SYS_FSL_FM1_ADDR;
-	if (fm_init_common(0, reg))
+	if (fm_init_common(0, reg, NULL))
 		return 0;
 
 	for (i = 0; i < ARRAY_SIZE(fm_info); i++) {
@@ -103,7 +103,7 @@ int fm_standard_init(struct bd_info *bis)
 
 #if (CONFIG_SYS_NUM_FMAN == 2)
 	reg = (void *)CONFIG_SYS_FSL_FM2_ADDR;
-	if (fm_init_common(1, reg))
+	if (fm_init_common(1, reg, NULL))
 		return 0;
 
 	for (i = 0; i < ARRAY_SIZE(fm_info); i++) {
-- 
2.25.1

[PATCH 5/6] net: fm: Support loading firmware from a filesystem

[Sean Anderson]

This adds a new method to load Fman firmware from a filesystem. This
allows users to use regular files instead of hard-coded offsets for the
firmware.

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 drivers/net/fm/fm.c | 25 ++++++++++++++++++++++++-
 drivers/qe/Kconfig  |  4 ++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c
index aa0cc69232..39b939cb97 100644
--- a/drivers/net/fm/fm.c
+++ b/drivers/net/fm/fm.c
@@ -5,6 +5,7 @@
  */
 #include <common.h>
 #include <env.h>
+#include <fs_loader.h>
 #include <malloc.h>
 #include <asm/io.h>
 #include <dm/device_compat.h>
@@ -453,7 +454,29 @@ int fm_init_common(int index, struct ccsr_fman *reg, const char *firmware_name)
 int fm_init_common(int index, struct ccsr_fman *reg, const char *firmware_name)
 {
 	int rc;
-#if defined(CONFIG_SYS_QE_FMAN_FW_IN_NOR)
+#if defined(CONFIG_SYS_QE_FMAN_FW_IN_FS)
+	struct udevice *fs_loader;
+	void *addr = malloc(CONFIG_SYS_QE_FMAN_FW_LENGTH);
+
+	if (!addr)
+		return -ENOMEM;
+
+	rc = get_fs_loader(&fs_loader);
+	if (rc) {
+		debug("could not get fs loader: %d\n", rc);
+		return rc;
+	}
+
+	if (!firmware_name)
+		firmware_name = "fman.itb";
+
+	rc = request_firmware_into_buf(fs_loader, firmware_name, addr,
+				       CONFIG_SYS_QE_FMAN_FW_LENGTH, 0);
+	if (rc < 0) {
+		debug("could not request %s: %d\n", firmware_name, rc);
+		return rc;
+	}
+#elif defined(CONFIG_SYS_QE_FMAN_FW_IN_NOR)
 	void *addr = (void *)CONFIG_SYS_FMAN_FW_ADDR;
 #elif defined(CONFIG_SYS_QE_FMAN_FW_IN_NAND)
 	size_t fw_length = CONFIG_SYS_QE_FMAN_FW_LENGTH;
diff --git a/drivers/qe/Kconfig b/drivers/qe/Kconfig
index c44a81f69a..89a75c175b 100644
--- a/drivers/qe/Kconfig
+++ b/drivers/qe/Kconfig
@@ -27,6 +27,10 @@ choice
 	depends on FMAN_ENET || QE
 	default SYS_QE_FMAN_FW_IN_ROM
 
+config SYS_QE_FMAN_FW_IN_FS
+	depends on FS_LOADER && FMAN_ENET
+	bool "Filesystem"
+
 config SYS_QE_FMAN_FW_IN_NOR
 	bool "NOR flash"
 
-- 
2.25.1

[PATCH 6/6] net: fm: Add support for FIT firmware

[Sean Anderson]

Fman microcode is executable code (AFAICT) loaded into a
coprocessor. As such, if verified boot is enabled, it must be verified
like other executable code. However, this is not currently done.

This commit adds verified boot functionality by encapsulating the
microcode in a FIT, which can then be signed/verified as normal. By
default we allow fallback to unencapsulated firmware, but if
CONFIG_FIT_SIGNATURE is enabled, then we make it mandatory. Because
existing Layerscape do not use this config (instead enabling
CONFIG_CHAIN_OF_TRUST), this should not break any existing boards.

An example (mildly-abbreviated) its is provided below:

/ {
    #address-cells = <1>;

    images {
        firmware {
            data = /incbin/(/path/to/firmware);
            type = "firmware";
            arch = "arm64";
            compression = "none";
	    signature {
                algo = "sha256,rsa2048";
                key-name-hint = "your key name";
            };
        };
    };

    configurations {
        default = "conf";
        conf {
            description = "Load FMAN microcode";
            fman = "firmware";
        };
    };
};

Signed-off-by: Sean Anderson <sean.anderson@seco.com>
---

 drivers/net/fm/fm.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/net/fm/fm.c b/drivers/net/fm/fm.c
index 39b939cb97..09e13506bf 100644
--- a/drivers/net/fm/fm.c
+++ b/drivers/net/fm/fm.c
@@ -6,6 +6,7 @@
 #include <common.h>
 #include <env.h>
 #include <fs_loader.h>
+#include <image.h>
 #include <malloc.h>
 #include <asm/io.h>
 #include <dm/device_compat.h>
@@ -537,6 +538,23 @@ int fm_init_common(int index, struct ccsr_fman *reg, const char *firmware_name)
 	void *addr = NULL;
 #endif
 
+	rc = fit_check_format(addr, CONFIG_SYS_QE_FMAN_FW_LENGTH);
+	if (!rc) {
+		size_t unused;
+		const void *new_addr;
+
+		rc = fit_get_data_conf_prop(addr, "fman", &new_addr, &unused);
+		if (rc)
+			return rc;
+		addr = (void *)new_addr;
+	} else if (CONFIG_IS_ENABLED(FIT_SIGNATURE)) {
+		/*
+		 * Using a (signed) FIT wrapper is mandatory if we are
+		 * doing verified boot.
+		 */
+		return rc;
+	}
+
 	/* Upload the Fman microcode if it's present */
 	rc = fman_upload_firmware(index, &reg->fm_imem, addr);
 	if (rc)
-- 
2.25.1

Re: [PATCH 2/6] image: fit: Add some helpers for getting data

[Simon Glass]

Hi Sean,

On Thu, 24 Mar 2022 at 12:23, Sean Anderson <sean.anderson@seco.com> wrote:
>
> Several different firmware users have repetitive code to extract the
> firmware data from a FIT. Add some helper functions to reduce the amount
> of repetition. fit_conf_get_prop_node (eventually) calls
> fdt_check_node_offset_, so we can avoid an explicit if. In general, this
> version avoids printing on error because the callers are typically
> library functions, and because the FIT code generally has (debug)
> prints of its own. One difference in these helpers is that they use
> fit_image_get_data_and_size instead of fit_image_get_data, as the former
> handles external data correctly.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
>  arch/arm/cpu/armv8/sec_firmware.c  | 39 ++---------------------------
>  boot/image-fit.c                   | 37 +++++++++++++++++++++++++++
>  cmd/fpga.c                         | 24 +++++-------------
>  drivers/net/fsl-mc/mc.c            | 30 +++-------------------
>  drivers/net/pfe_eth/pfe_firmware.c | 40 +-----------------------------
>  include/image.h                    |  4 +++
>  6 files changed, 53 insertions(+), 121 deletions(-)

It feels like this patch should be split up into generic / armv8 / fsp things.

>
> diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
> index 41525a10d5..879eb6ec81 100644
> --- a/arch/arm/cpu/armv8/sec_firmware.c
> +++ b/arch/arm/cpu/armv8/sec_firmware.c
> @@ -42,43 +42,8 @@ phys_addr_t sec_firmware_addr;
>  static int sec_firmware_get_data(const void *sec_firmware_img,
>                                 const void **data, size_t *size)
>  {
> -       int conf_node_off, fw_node_off;
> -       char *desc;
> -       int ret;
> -
> -       conf_node_off = fit_conf_get_node(sec_firmware_img, NULL);
> -       if (conf_node_off < 0) {
> -               puts("SEC Firmware: no config\n");
> -               return -ENOENT;
> -       }
> -
> -       fw_node_off = fit_conf_get_prop_node(sec_firmware_img, conf_node_off,
> -                       SEC_FIRMWARE_FIT_IMAGE);
> -       if (fw_node_off < 0) {
> -               printf("SEC Firmware: No '%s' in config\n",
> -                      SEC_FIRMWARE_FIT_IMAGE);
> -               return -ENOLINK;
> -       }
> -
> -       /* Verify secure firmware image */
> -       if (!(fit_image_verify(sec_firmware_img, fw_node_off))) {
> -               printf("SEC Firmware: Bad firmware image (bad CRC)\n");
> -               return -EINVAL;
> -       }
> -
> -       if (fit_image_get_data(sec_firmware_img, fw_node_off, data, size)) {
> -               printf("SEC Firmware: Can't get %s subimage data/size",
> -                      SEC_FIRMWARE_FIT_IMAGE);
> -               return -ENOENT;
> -       }
> -
> -       ret = fit_get_desc(sec_firmware_img, fw_node_off, &desc);
> -       if (ret)
> -               printf("SEC Firmware: Can't get description\n");
> -       else
> -               printf("%s\n", desc);
> -
> -       return ret;
> +       return fit_get_data_conf_prop(sec_firmware_img, SEC_FIRMWARE_FIT_IMAGE,
> +                                     data, size);
>  }
>
>  /*
> diff --git a/boot/image-fit.c b/boot/image-fit.c
> index 6610035d0a..b87378cbf6 100644
> --- a/boot/image-fit.c
> +++ b/boot/image-fit.c
> @@ -1919,6 +1919,43 @@ int fit_conf_get_prop_node(const void *fit, int noffset,
>         return fit_conf_get_prop_node_index(fit, noffset, prop_name, 0);
>  }
>
> +static int fit_get_data_tail(const void *fit, int noffset,
> +                            const void **data, size_t *size)
> +{
> +       char *desc;
> +
> +       if (noffset < 0)
> +               return noffset;
> +
> +       if (!fit_image_verify(fit, noffset))
> +               return -EINVAL;
> +
> +       if (fit_image_get_data_and_size(fit, noffset, data, size))
> +               return -ENOENT;
> +
> +       if (!fit_get_desc(fit, noffset, &desc))
> +               printf("%s\n", desc);
> +
> +       return 0;
> +}
> +
> +int fit_get_data_node(const void *fit, const char *image_uname,
> +                     const void **data, size_t *size)
> +{
> +       int noffset = fit_image_get_node(fit, image_uname);
> +
> +       return fit_get_data_tail(fit, noffset, data, size);
> +}
> +
> +int fit_get_data_conf_prop(const void *fit, const char *prop_name,
> +                          const void **data, size_t *size)
> +{
> +       int noffset = fit_conf_get_node(fit, NULL);
> +
> +       noffset = fit_conf_get_prop_node(fit, noffset, prop_name);
> +       return fit_get_data_tail(fit, noffset, data, size);
> +}
> +
>  static int fit_image_select(const void *fit, int rd_noffset, int verify)
>  {
>         fit_image_print(fit, rd_noffset, "   ");
> diff --git a/cmd/fpga.c b/cmd/fpga.c
> index 3fdd0b35e8..1102a84d76 100644
> --- a/cmd/fpga.c
> +++ b/cmd/fpga.c
> @@ -322,7 +322,7 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc,
>         case IMAGE_FORMAT_FIT:
>         {
>                 const void *fit_hdr = (const void *)fpga_data;
> -               int noffset;
> +               int err;
>                 const void *fit_data;
>
>                 if (!fit_uname) {
> @@ -335,23 +335,11 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc,
>                         return CMD_RET_FAILURE;
>                 }
>
> -               /* get fpga component image node offset */
> -               noffset = fit_image_get_node(fit_hdr, fit_uname);
> -               if (noffset < 0) {
> -                       printf("Can't find '%s' FIT subimage\n", fit_uname);
> -                       return CMD_RET_FAILURE;
> -               }
> -
> -               /* verify integrity */
> -               if (!fit_image_verify(fit_hdr, noffset)) {
> -                       puts("Bad Data Hash\n");
> -                       return CMD_RET_FAILURE;
> -               }
> -
> -               /* get fpga subimage/external data address and length */
> -               if (fit_image_get_data_and_size(fit_hdr, noffset,
> -                                              &fit_data, &data_size)) {
> -                       puts("Fpga subimage data not found\n");
> +               err = fit_get_data_node(fit_hdr, fit_uname, &fit_data,
> +                                       &data_size);
> +               if (err) {
> +                       printf("Could not load '%s' subimage (err %d)\n",
> +                              fit_uname, err);
>                         return CMD_RET_FAILURE;
>                 }
>
> diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c
> index bc1c31d467..68833f9ddd 100644
> --- a/drivers/net/fsl-mc/mc.c
> +++ b/drivers/net/fsl-mc/mc.c
> @@ -137,13 +137,7 @@ int parse_mc_firmware_fit_image(u64 mc_fw_addr,
>                                 size_t *raw_image_size)
>  {
>         int format;
> -       void *fit_hdr;
> -       int node_offset;
> -       const void *data;
> -       size_t size;
> -       const char *uname = "firmware";
> -
> -       fit_hdr = (void *)mc_fw_addr;
> +       void *fit_hdr = (void *)mc_fw_addr;
>
>         /* Check if Image is in FIT format */
>         format = genimg_get_format(fit_hdr);
> @@ -158,26 +152,8 @@ int parse_mc_firmware_fit_image(u64 mc_fw_addr,
>                 return -EINVAL;
>         }
>
> -       node_offset = fit_image_get_node(fit_hdr, uname);
> -
> -       if (node_offset < 0) {
> -               printf("fsl-mc: ERR: Bad firmware image (missing subimage)\n");
> -               return -ENOENT;
> -       }
> -
> -       /* Verify MC firmware image */
> -       if (!(fit_image_verify(fit_hdr, node_offset))) {
> -               printf("fsl-mc: ERR: Bad firmware image (bad CRC)\n");
> -               return -EINVAL;
> -       }
> -
> -       /* Get address and size of raw image */
> -       fit_image_get_data(fit_hdr, node_offset, &data, &size);
> -
> -       *raw_image_addr = data;
> -       *raw_image_size = size;
> -
> -       return 0;
> +       return fit_get_data_node(fit_hdr, "firmware", raw_image_addr,
> +                                raw_image_size);
>  }
>  #endif
>
> diff --git a/drivers/net/pfe_eth/pfe_firmware.c b/drivers/net/pfe_eth/pfe_firmware.c
> index 6669048181..adaa139219 100644
> --- a/drivers/net/pfe_eth/pfe_firmware.c
> +++ b/drivers/net/pfe_eth/pfe_firmware.c
> @@ -104,45 +104,7 @@ err:
>  static int pfe_get_fw(const void **data,
>                       size_t *size, char *fw_name)
>  {
> -       int conf_node_off, fw_node_off;
> -       char *conf_node_name = NULL;
> -       char *desc;
> -       int ret = 0;
> -
> -       conf_node_name = PFE_FIRMWARE_FIT_CNF_NAME;
> -
> -       conf_node_off = fit_conf_get_node(pfe_fit_addr, conf_node_name);
> -       if (conf_node_off < 0) {
> -               printf("PFE Firmware: %s: no such config\n", conf_node_name);
> -               return -ENOENT;
> -       }
> -
> -       fw_node_off = fit_conf_get_prop_node(pfe_fit_addr, conf_node_off,
> -                                            fw_name);
> -       if (fw_node_off < 0) {
> -               printf("PFE Firmware: No '%s' in config\n",
> -                      fw_name);
> -               return -ENOLINK;
> -       }
> -
> -       if (!(fit_image_verify(pfe_fit_addr, fw_node_off))) {
> -               printf("PFE Firmware: Bad firmware image (bad CRC)\n");
> -               return -EINVAL;
> -       }
> -
> -       if (fit_image_get_data(pfe_fit_addr, fw_node_off, data, size)) {
> -               printf("PFE Firmware: Can't get %s subimage data/size",
> -                      fw_name);
> -               return -ENOENT;
> -       }
> -
> -       ret = fit_get_desc(pfe_fit_addr, fw_node_off, &desc);
> -       if (ret)
> -               printf("PFE Firmware: Can't get description\n");
> -       else
> -               printf("%s\n", desc);
> -
> -       return ret;
> +       return fit_get_data_conf_prop(pfe_fit_addr, fw_name, data, size);
>  }
>
>  /*
> diff --git a/include/image.h b/include/image.h
> index 97e5f2eb24..ae1f015896 100644
> --- a/include/image.h
> +++ b/include/image.h
> @@ -1010,6 +1010,10 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
>                                        size_t *data_size);
>  int fit_image_get_data_and_size(const void *fit, int noffset,
>                                 const void **data, size_t *size);
> +int fit_get_data_node(const void *fit, const char *image_uname,
> +                     const void **data, size_t *size);
> +int fit_get_data_conf_prop(const void *fit, const char *prop_name,
> +                          const void **data, size_t *size);

Please comment the functions.

>
>  int fit_image_hash_get_algo(const void *fit, int noffset, const char **algo);
>  int fit_image_hash_get_value(const void *fit, int noffset, uint8_t **value,
> --
> 2.25.1
>

Regards,
SImon

Re: [PATCH 3/6] misc: fs_loader: Add function to get the chosen loader

[Simon Glass]

On Thu, 24 Mar 2022 at 12:23, Sean Anderson <sean.anderson@seco.com> wrote:
>
> The fs_loader device is used to pull in settings via the chosen node.
> However, there was no library function for this, so arria10 was doing it
> explicitly. This function subsumes that, and uses ofnode_get_chosen_node
> instead of navigating the device tree directly. Because fs_loader pulls
> its config from the environment by default, it's fine to create a device
> with nothing backing it at all. Doing this allows enabling
> CONFIG_FS_LOADER without needing to modify the device tree.
>
> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
> ---
>
>  arch/arm/mach-k3/common.c         |  2 +-
>  arch/arm/mach-omap2/boot-common.c |  2 +-
>  drivers/fpga/socfpga_arria10.c    | 24 ++----------------------
>  drivers/misc/fs_loader.c          | 27 +++++++++++++++++++++++++++
>  include/fs_loader.h               | 12 ++++++++++++
>  5 files changed, 43 insertions(+), 24 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Re: [PATCH 2/6] image: fit: Add some helpers for getting data

[Sean Anderson]

Hi Simon,

On 3/28/22 2:35 AM, Simon Glass wrote:
> Hi Sean,
> 
> On Thu, 24 Mar 2022 at 12:23, Sean Anderson <sean.anderson@seco.com> wrote:
>>
>> Several different firmware users have repetitive code to extract the
>> firmware data from a FIT. Add some helper functions to reduce the amount
>> of repetition. fit_conf_get_prop_node (eventually) calls
>> fdt_check_node_offset_, so we can avoid an explicit if. In general, this
>> version avoids printing on error because the callers are typically
>> library functions, and because the FIT code generally has (debug)
>> prints of its own. One difference in these helpers is that they use
>> fit_image_get_data_and_size instead of fit_image_get_data, as the former
>> handles external data correctly.
>>
>> Signed-off-by: Sean Anderson <sean.anderson@seco.com>
>> ---
>>
>>  arch/arm/cpu/armv8/sec_firmware.c  | 39 ++---------------------------
>>  boot/image-fit.c                   | 37 +++++++++++++++++++++++++++
>>  cmd/fpga.c                         | 24 +++++-------------
>>  drivers/net/fsl-mc/mc.c            | 30 +++-------------------
>>  drivers/net/pfe_eth/pfe_firmware.c | 40 +-----------------------------
>>  include/image.h                    |  4 +++
>>  6 files changed, 53 insertions(+), 121 deletions(-)
> 
> It feels like this patch should be split up into generic / armv8 / fsp things.

OK, I will do that for V2.

(Actually, I think this series can be split into fs-loader and verified-boot).

>>
>> diff --git a/arch/arm/cpu/armv8/sec_firmware.c b/arch/arm/cpu/armv8/sec_firmware.c
>> index 41525a10d5..879eb6ec81 100644
>> --- a/arch/arm/cpu/armv8/sec_firmware.c
>> +++ b/arch/arm/cpu/armv8/sec_firmware.c
>> @@ -42,43 +42,8 @@ phys_addr_t sec_firmware_addr;
>>  static int sec_firmware_get_data(const void *sec_firmware_img,
>>                                 const void **data, size_t *size)
>>  {
>> -       int conf_node_off, fw_node_off;
>> -       char *desc;
>> -       int ret;
>> -
>> -       conf_node_off = fit_conf_get_node(sec_firmware_img, NULL);
>> -       if (conf_node_off < 0) {
>> -               puts("SEC Firmware: no config\n");
>> -               return -ENOENT;
>> -       }
>> -
>> -       fw_node_off = fit_conf_get_prop_node(sec_firmware_img, conf_node_off,
>> -                       SEC_FIRMWARE_FIT_IMAGE);
>> -       if (fw_node_off < 0) {
>> -               printf("SEC Firmware: No '%s' in config\n",
>> -                      SEC_FIRMWARE_FIT_IMAGE);
>> -               return -ENOLINK;
>> -       }
>> -
>> -       /* Verify secure firmware image */
>> -       if (!(fit_image_verify(sec_firmware_img, fw_node_off))) {
>> -               printf("SEC Firmware: Bad firmware image (bad CRC)\n");
>> -               return -EINVAL;
>> -       }
>> -
>> -       if (fit_image_get_data(sec_firmware_img, fw_node_off, data, size)) {
>> -               printf("SEC Firmware: Can't get %s subimage data/size",
>> -                      SEC_FIRMWARE_FIT_IMAGE);
>> -               return -ENOENT;
>> -       }
>> -
>> -       ret = fit_get_desc(sec_firmware_img, fw_node_off, &desc);
>> -       if (ret)
>> -               printf("SEC Firmware: Can't get description\n");
>> -       else
>> -               printf("%s\n", desc);
>> -
>> -       return ret;
>> +       return fit_get_data_conf_prop(sec_firmware_img, SEC_FIRMWARE_FIT_IMAGE,
>> +                                     data, size);
>>  }
>>
>>  /*
>> diff --git a/boot/image-fit.c b/boot/image-fit.c
>> index 6610035d0a..b87378cbf6 100644
>> --- a/boot/image-fit.c
>> +++ b/boot/image-fit.c
>> @@ -1919,6 +1919,43 @@ int fit_conf_get_prop_node(const void *fit, int noffset,
>>         return fit_conf_get_prop_node_index(fit, noffset, prop_name, 0);
>>  }
>>
>> +static int fit_get_data_tail(const void *fit, int noffset,
>> +                            const void **data, size_t *size)
>> +{
>> +       char *desc;
>> +
>> +       if (noffset < 0)
>> +               return noffset;
>> +
>> +       if (!fit_image_verify(fit, noffset))
>> +               return -EINVAL;
>> +
>> +       if (fit_image_get_data_and_size(fit, noffset, data, size))
>> +               return -ENOENT;
>> +
>> +       if (!fit_get_desc(fit, noffset, &desc))
>> +               printf("%s\n", desc);
>> +
>> +       return 0;
>> +}
>> +
>> +int fit_get_data_node(const void *fit, const char *image_uname,
>> +                     const void **data, size_t *size)
>> +{
>> +       int noffset = fit_image_get_node(fit, image_uname);
>> +
>> +       return fit_get_data_tail(fit, noffset, data, size);
>> +}
>> +
>> +int fit_get_data_conf_prop(const void *fit, const char *prop_name,
>> +                          const void **data, size_t *size)
>> +{
>> +       int noffset = fit_conf_get_node(fit, NULL);
>> +
>> +       noffset = fit_conf_get_prop_node(fit, noffset, prop_name);
>> +       return fit_get_data_tail(fit, noffset, data, size);
>> +}
>> +
>>  static int fit_image_select(const void *fit, int rd_noffset, int verify)
>>  {
>>         fit_image_print(fit, rd_noffset, "   ");
>> diff --git a/cmd/fpga.c b/cmd/fpga.c
>> index 3fdd0b35e8..1102a84d76 100644
>> --- a/cmd/fpga.c
>> +++ b/cmd/fpga.c
>> @@ -322,7 +322,7 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc,
>>         case IMAGE_FORMAT_FIT:
>>         {
>>                 const void *fit_hdr = (const void *)fpga_data;
>> -               int noffset;
>> +               int err;
>>                 const void *fit_data;
>>
>>                 if (!fit_uname) {
>> @@ -335,23 +335,11 @@ static int do_fpga_loadmk(struct cmd_tbl *cmdtp, int flag, int argc,
>>                         return CMD_RET_FAILURE;
>>                 }
>>
>> -               /* get fpga component image node offset */
>> -               noffset = fit_image_get_node(fit_hdr, fit_uname);
>> -               if (noffset < 0) {
>> -                       printf("Can't find '%s' FIT subimage\n", fit_uname);
>> -                       return CMD_RET_FAILURE;
>> -               }
>> -
>> -               /* verify integrity */
>> -               if (!fit_image_verify(fit_hdr, noffset)) {
>> -                       puts("Bad Data Hash\n");
>> -                       return CMD_RET_FAILURE;
>> -               }
>> -
>> -               /* get fpga subimage/external data address and length */
>> -               if (fit_image_get_data_and_size(fit_hdr, noffset,
>> -                                              &fit_data, &data_size)) {
>> -                       puts("Fpga subimage data not found\n");
>> +               err = fit_get_data_node(fit_hdr, fit_uname, &fit_data,
>> +                                       &data_size);
>> +               if (err) {
>> +                       printf("Could not load '%s' subimage (err %d)\n",
>> +                              fit_uname, err);
>>                         return CMD_RET_FAILURE;
>>                 }
>>
>> diff --git a/drivers/net/fsl-mc/mc.c b/drivers/net/fsl-mc/mc.c
>> index bc1c31d467..68833f9ddd 100644
>> --- a/drivers/net/fsl-mc/mc.c
>> +++ b/drivers/net/fsl-mc/mc.c
>> @@ -137,13 +137,7 @@ int parse_mc_firmware_fit_image(u64 mc_fw_addr,
>>                                 size_t *raw_image_size)
>>  {
>>         int format;
>> -       void *fit_hdr;
>> -       int node_offset;
>> -       const void *data;
>> -       size_t size;
>> -       const char *uname = "firmware";
>> -
>> -       fit_hdr = (void *)mc_fw_addr;
>> +       void *fit_hdr = (void *)mc_fw_addr;
>>
>>         /* Check if Image is in FIT format */
>>         format = genimg_get_format(fit_hdr);
>> @@ -158,26 +152,8 @@ int parse_mc_firmware_fit_image(u64 mc_fw_addr,
>>                 return -EINVAL;
>>         }
>>
>> -       node_offset = fit_image_get_node(fit_hdr, uname);
>> -
>> -       if (node_offset < 0) {
>> -               printf("fsl-mc: ERR: Bad firmware image (missing subimage)\n");
>> -               return -ENOENT;
>> -       }
>> -
>> -       /* Verify MC firmware image */
>> -       if (!(fit_image_verify(fit_hdr, node_offset))) {
>> -               printf("fsl-mc: ERR: Bad firmware image (bad CRC)\n");
>> -               return -EINVAL;
>> -       }
>> -
>> -       /* Get address and size of raw image */
>> -       fit_image_get_data(fit_hdr, node_offset, &data, &size);
>> -
>> -       *raw_image_addr = data;
>> -       *raw_image_size = size;
>> -
>> -       return 0;
>> +       return fit_get_data_node(fit_hdr, "firmware", raw_image_addr,
>> +                                raw_image_size);
>>  }
>>  #endif
>>
>> diff --git a/drivers/net/pfe_eth/pfe_firmware.c b/drivers/net/pfe_eth/pfe_firmware.c
>> index 6669048181..adaa139219 100644
>> --- a/drivers/net/pfe_eth/pfe_firmware.c
>> +++ b/drivers/net/pfe_eth/pfe_firmware.c
>> @@ -104,45 +104,7 @@ err:
>>  static int pfe_get_fw(const void **data,
>>                       size_t *size, char *fw_name)
>>  {
>> -       int conf_node_off, fw_node_off;
>> -       char *conf_node_name = NULL;
>> -       char *desc;
>> -       int ret = 0;
>> -
>> -       conf_node_name = PFE_FIRMWARE_FIT_CNF_NAME;
>> -
>> -       conf_node_off = fit_conf_get_node(pfe_fit_addr, conf_node_name);
>> -       if (conf_node_off < 0) {
>> -               printf("PFE Firmware: %s: no such config\n", conf_node_name);
>> -               return -ENOENT;
>> -       }
>> -
>> -       fw_node_off = fit_conf_get_prop_node(pfe_fit_addr, conf_node_off,
>> -                                            fw_name);
>> -       if (fw_node_off < 0) {
>> -               printf("PFE Firmware: No '%s' in config\n",
>> -                      fw_name);
>> -               return -ENOLINK;
>> -       }
>> -
>> -       if (!(fit_image_verify(pfe_fit_addr, fw_node_off))) {
>> -               printf("PFE Firmware: Bad firmware image (bad CRC)\n");
>> -               return -EINVAL;
>> -       }
>> -
>> -       if (fit_image_get_data(pfe_fit_addr, fw_node_off, data, size)) {
>> -               printf("PFE Firmware: Can't get %s subimage data/size",
>> -                      fw_name);
>> -               return -ENOENT;
>> -       }
>> -
>> -       ret = fit_get_desc(pfe_fit_addr, fw_node_off, &desc);
>> -       if (ret)
>> -               printf("PFE Firmware: Can't get description\n");
>> -       else
>> -               printf("%s\n", desc);
>> -
>> -       return ret;
>> +       return fit_get_data_conf_prop(pfe_fit_addr, fw_name, data, size);
>>  }
>>
>>  /*
>> diff --git a/include/image.h b/include/image.h
>> index 97e5f2eb24..ae1f015896 100644
>> --- a/include/image.h
>> +++ b/include/image.h
>> @@ -1010,6 +1010,10 @@ int fit_image_get_data_size_unciphered(const void *fit, int noffset,
>>                                        size_t *data_size);
>>  int fit_image_get_data_and_size(const void *fit, int noffset,
>>                                 const void **data, size_t *size);
>> +int fit_get_data_node(const void *fit, const char *image_uname,
>> +                     const void **data, size_t *size);
>> +int fit_get_data_conf_prop(const void *fit, const char *prop_name,
>> +                          const void **data, size_t *size);
> 
> Please comment the functions.

Sure.

>>
>>  int fit_image_hash_get_algo(const void *fit, int noffset, const char **algo);
>>  int fit_image_hash_get_value(const void *fit, int noffset, uint8_t **value,
>> --
>> 2.25.1
>>
> 
> Regards,
> SImon
>