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

Sean Anderson sean.anderson at seco.com
Thu Mar 24 19:23:01 CET 2022


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



More information about the U-Boot mailing list