diff mbox series

[v4,2/6] FWU: mtd: Add helper functions for accessing FWU metadata

Message ID 20230327211603.498936-1-jaswinder.singh@linaro.org
State New
Headers show
Series FWU: Add support for mtd backed feature on DeveloperBox | expand

Commit Message

Jassi Brar March 27, 2023, 9:16 p.m. UTC
From: Masami Hiramatsu <masami.hiramatsu@linaro.org>

Add helper functions needed for accessing the FWU metadata which
contains information on the updatable images.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---
 include/fwu.h             |  34 ++++++++
 lib/fwu_updates/Makefile  |   1 +
 lib/fwu_updates/fwu_mtd.c | 164 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 199 insertions(+)
 create mode 100644 lib/fwu_updates/fwu_mtd.c

Comments

Michal Simek March 29, 2023, 11:55 a.m. UTC | #1
On 3/27/23 23:16, jassisinghbrar@gmail.com wrote:
> From: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> 
> Add helper functions needed for accessing the FWU metadata which
> contains information on the updatable images.
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu@linaro.org>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
> ---
>   include/fwu.h             |  34 ++++++++
>   lib/fwu_updates/Makefile  |   1 +
>   lib/fwu_updates/fwu_mtd.c | 164 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 199 insertions(+)
>   create mode 100644 lib/fwu_updates/fwu_mtd.c
> 
> diff --git a/include/fwu.h b/include/fwu.h
> index 33b4d0b3db..117f51c4f5 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -8,6 +8,8 @@
>   
>   #include <blk.h>
>   #include <efi.h>
> +#include <mtd.h>
> +#include <uuid.h>
>   
>   #include <linux/types.h>
>   
> @@ -18,6 +20,12 @@ struct fwu_mdata_gpt_blk_priv {
>   	struct udevice *blk_dev;
>   };
>   
> +struct fwu_mtd_image_info {
> +	u32 start, size;
> +	int bank_num, image_num;
> +	char uuidbuf[UUID_STR_LEN + 1];
> +};
> +
>   struct fwu_mdata_ops {
>   	/**
>   	 * read_mdata() - Populate the asked FWU metadata copy
> @@ -251,4 +259,30 @@ u8 fwu_empty_capsule_checks_pass(void);
>    */
>   int fwu_trial_state_ctr_start(void);
>   
> +/**
> + * fwu_gen_alt_info_from_mtd() - Parse dfu_alt_info from metadata in mtd
> + * @buf: Buffer into which the dfu_alt_info is filled
> + * @len: Maximum characters that can be written in buf
> + * @mtd: Pointer to underlying MTD device
> + *
> + * Parse dfu_alt_info from metadata in mtd. Used for setting the env.
> + *
> + * Return: 0 if OK, -ve on error
> + *

nit: remove this line above.

> + */
> +int fwu_gen_alt_info_from_mtd(char *buf, size_t len, struct mtd_info *mtd);
> +
> +/**
> + * fwu_mtd_get_alt_num() - Mapping of fwu_plat_get_alt_num for MTD device
> + * @image_guid: Image GUID for which DFU alt number needs to be retrieved

it doesn't match with parameter below.

> + * @alt_num: Pointer to the alt_num
> + * @mtd_dev: Name of mtd device instance
> + *
> + * To map fwu_plat_get_alt_num onto mtd based metadata implementation.
> + *
> + * Return: 0 if OK, -ve on error
> + *

nit: remove this line above

> + */
> +int fwu_mtd_get_alt_num(efi_guid_t *image_id, u8 *alt_num, const char *mtd_dev);


./scripts/kernel-doc -v -man include/fwu.h 1>/dev/null

include/fwu.h:276: info: Scanning doc for fwu_mtd_get_alt_num
include/fwu.h:286: warning: Function parameter or member 'image_id' not 
described in 'fwu_mtd_get_alt_num'
include/fwu.h:286: warning: Excess function parameter 'image_guid' description 
in 'fwu_mtd_get_alt_num'

(In general this file has other kernel-doc issues which should be fixed too)


> +
>   #endif /* _FWU_H_ */
> diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
> index 1993088e5b..c9e3c06b48 100644
> --- a/lib/fwu_updates/Makefile
> +++ b/lib/fwu_updates/Makefile
> @@ -5,3 +5,4 @@
>   
>   obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
>   obj-$(CONFIG_FWU_MDATA_GPT_BLK) += fwu_gpt.o
> +obj-$(CONFIG_FWU_MDATA_MTD) += fwu_mtd.o
> diff --git a/lib/fwu_updates/fwu_mtd.c b/lib/fwu_updates/fwu_mtd.c
> new file mode 100644
> index 0000000000..c1d04e574b
> --- /dev/null
> +++ b/lib/fwu_updates/fwu_mtd.c
> @@ -0,0 +1,164 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2023, Linaro Limited
> + */
> +
> +#include <dm.h>
> +#include <dfu.h>
> +#include <fwu.h>
> +#include <fwu_mdata.h>
> +#include <log.h>
> +#include <malloc.h>
> +#include <mtd.h>
> +#include <uuid.h>
> +#include <vsprintf.h>
> +
> +#include <dm/ofnode.h>
> +
> +struct fwu_mtd_image_info
> +fwu_mtd_images[CONFIG_FWU_NUM_BANKS * CONFIG_FWU_NUM_IMAGES_PER_BANK];
> +
> +static struct fwu_mtd_image_info *mtd_img_by_uuid(const char *uuidbuf)
> +{
> +	int num_images = sizeof(fwu_mtd_images) / sizeof(struct fwu_mtd_image_info);

include/linux/kernel.h:55:#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))


> +
> +	for (int i = 0; i < num_images; i++)
> +		if (!strcmp(uuidbuf, fwu_mtd_images[i].uuidbuf))
> +			return &fwu_mtd_images[i];
> +
> +	return NULL;
> +}
> +
> +int fwu_mtd_get_alt_num(efi_guid_t *image_id, u8 *alt_num,
> +			const char *mtd_dev)
> +{
> +	int i, nalt;
> +	int ret = -1;

nit: can be on the same line.

> +	struct mtd_info *mtd;
> +	struct dfu_entity *dfu;
> +	fdt_addr_t offset, size = 0;
> +	char uuidbuf[UUID_STR_LEN + 1];
> +	struct fwu_mtd_image_info *mtd_img_info;

nit: reverse christmas tree order

> +
> +	mtd_probe_devices();
> +	mtd = get_mtd_device_nm(mtd_dev);

you are getting link but you are not using it anywhere.
You should check return value or remove this call completely.


> +
> +	uuid_bin_to_str(image_id->b, uuidbuf, UUID_STR_FORMAT_STD);
> +
> +	mtd_img_info = mtd_img_by_uuid(uuidbuf);
> +	if (!mtd_img_info) {
> +		log_err("%s: Not found partition for image %s\n", __func__, uuidbuf);
> +		return -ENOENT;
> +	}

nit: newline

> +	offset = mtd_img_info->start;
> +	size = mtd_img_info->size;
> +
> +	dfu_init_env_entities(NULL, NULL);

worth to check return value here.

M
Jassi Brar April 10, 2023, 4:02 a.m. UTC | #2
On Wed, 29 Mar 2023 at 06:55, Michal Simek <michal.simek@amd.com> wrote:
> On 3/27/23 23:16, jassisinghbrar@gmail.com wrote:


> > +/**
> > + * fwu_gen_alt_info_from_mtd() - Parse dfu_alt_info from metadata in mtd
> > + * @buf: Buffer into which the dfu_alt_info is filled
> > + * @len: Maximum characters that can be written in buf
> > + * @mtd: Pointer to underlying MTD device
> > + *
> > + * Parse dfu_alt_info from metadata in mtd. Used for setting the env.
> > + *
> > + * Return: 0 if OK, -ve on error
> > + *
>
> nit: remove this line above.
>
ok

> > + */
> > +int fwu_gen_alt_info_from_mtd(char *buf, size_t len, struct mtd_info *mtd);
> > +
> > +/**
> > + * fwu_mtd_get_alt_num() - Mapping of fwu_plat_get_alt_num for MTD device
> > + * @image_guid: Image GUID for which DFU alt number needs to be retrieved
>
> it doesn't match with parameter below.
>
ok

> > + * @alt_num: Pointer to the alt_num
> > + * @mtd_dev: Name of mtd device instance
> > + *
> > + * To map fwu_plat_get_alt_num onto mtd based metadata implementation.
> > + *
> > + * Return: 0 if OK, -ve on error
> > + *
>
> nit: remove this line above
>
ok

> > + */
> > +int fwu_mtd_get_alt_num(efi_guid_t *image_id, u8 *alt_num, const char *mtd_dev);
>
>
> ./scripts/kernel-doc -v -man include/fwu.h 1>/dev/null
>
> include/fwu.h:276: info: Scanning doc for fwu_mtd_get_alt_num
> include/fwu.h:286: warning: Function parameter or member 'image_id' not
> described in 'fwu_mtd_get_alt_num'
> include/fwu.h:286: warning: Excess function parameter 'image_guid' description
> in 'fwu_mtd_get_alt_num'
>
ok


> > +
> > +static struct fwu_mtd_image_info *mtd_img_by_uuid(const char *uuidbuf)
> > +{
> > +     int num_images = sizeof(fwu_mtd_images) / sizeof(struct fwu_mtd_image_info);
>
> include/linux/kernel.h:55:#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>
of course. thanks.


> > +int fwu_mtd_get_alt_num(efi_guid_t *image_id, u8 *alt_num,
> > +                     const char *mtd_dev)
> > +{
> > +     int i, nalt;
> > +     int ret = -1;
>
> nit: can be on the same line.
>
ok

> > +     struct mtd_info *mtd;
> > +     struct dfu_entity *dfu;
> > +     fdt_addr_t offset, size = 0;
> > +     char uuidbuf[UUID_STR_LEN + 1];
> > +     struct fwu_mtd_image_info *mtd_img_info;
>
> nit: reverse christmas tree order
>
ok

> > +
> > +     mtd_probe_devices();
> > +     mtd = get_mtd_device_nm(mtd_dev);
>
> you are getting link but you are not using it anywhere.
> You should check return value or remove this call completely.
>
This should go. thanks.


> > +
> > +     uuid_bin_to_str(image_id->b, uuidbuf, UUID_STR_FORMAT_STD);
> > +
> > +     mtd_img_info = mtd_img_by_uuid(uuidbuf);
> > +     if (!mtd_img_info) {
> > +             log_err("%s: Not found partition for image %s\n", __func__, uuidbuf);
> > +             return -ENOENT;
> > +     }
>
> nit: newline
>
ok

> > +     offset = mtd_img_info->start;
> > +     size = mtd_img_info->size;
> > +
> > +     dfu_init_env_entities(NULL, NULL);
>
> worth to check return value here.
>
ok, though it would also cleanly fail immediately after this call when
it find empty dfu_list.

Thanks.
diff mbox series

Patch

diff --git a/include/fwu.h b/include/fwu.h
index 33b4d0b3db..117f51c4f5 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -8,6 +8,8 @@ 
 
 #include <blk.h>
 #include <efi.h>
+#include <mtd.h>
+#include <uuid.h>
 
 #include <linux/types.h>
 
@@ -18,6 +20,12 @@  struct fwu_mdata_gpt_blk_priv {
 	struct udevice *blk_dev;
 };
 
+struct fwu_mtd_image_info {
+	u32 start, size;
+	int bank_num, image_num;
+	char uuidbuf[UUID_STR_LEN + 1];
+};
+
 struct fwu_mdata_ops {
 	/**
 	 * read_mdata() - Populate the asked FWU metadata copy
@@ -251,4 +259,30 @@  u8 fwu_empty_capsule_checks_pass(void);
  */
 int fwu_trial_state_ctr_start(void);
 
+/**
+ * fwu_gen_alt_info_from_mtd() - Parse dfu_alt_info from metadata in mtd
+ * @buf: Buffer into which the dfu_alt_info is filled
+ * @len: Maximum characters that can be written in buf
+ * @mtd: Pointer to underlying MTD device
+ *
+ * Parse dfu_alt_info from metadata in mtd. Used for setting the env.
+ *
+ * Return: 0 if OK, -ve on error
+ *
+ */
+int fwu_gen_alt_info_from_mtd(char *buf, size_t len, struct mtd_info *mtd);
+
+/**
+ * fwu_mtd_get_alt_num() - Mapping of fwu_plat_get_alt_num for MTD device
+ * @image_guid: Image GUID for which DFU alt number needs to be retrieved
+ * @alt_num: Pointer to the alt_num
+ * @mtd_dev: Name of mtd device instance
+ *
+ * To map fwu_plat_get_alt_num onto mtd based metadata implementation.
+ *
+ * Return: 0 if OK, -ve on error
+ *
+ */
+int fwu_mtd_get_alt_num(efi_guid_t *image_id, u8 *alt_num, const char *mtd_dev);
+
 #endif /* _FWU_H_ */
diff --git a/lib/fwu_updates/Makefile b/lib/fwu_updates/Makefile
index 1993088e5b..c9e3c06b48 100644
--- a/lib/fwu_updates/Makefile
+++ b/lib/fwu_updates/Makefile
@@ -5,3 +5,4 @@ 
 
 obj-$(CONFIG_FWU_MULTI_BANK_UPDATE) += fwu.o
 obj-$(CONFIG_FWU_MDATA_GPT_BLK) += fwu_gpt.o
+obj-$(CONFIG_FWU_MDATA_MTD) += fwu_mtd.o
diff --git a/lib/fwu_updates/fwu_mtd.c b/lib/fwu_updates/fwu_mtd.c
new file mode 100644
index 0000000000..c1d04e574b
--- /dev/null
+++ b/lib/fwu_updates/fwu_mtd.c
@@ -0,0 +1,164 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2023, Linaro Limited
+ */
+
+#include <dm.h>
+#include <dfu.h>
+#include <fwu.h>
+#include <fwu_mdata.h>
+#include <log.h>
+#include <malloc.h>
+#include <mtd.h>
+#include <uuid.h>
+#include <vsprintf.h>
+
+#include <dm/ofnode.h>
+
+struct fwu_mtd_image_info
+fwu_mtd_images[CONFIG_FWU_NUM_BANKS * CONFIG_FWU_NUM_IMAGES_PER_BANK];
+
+static struct fwu_mtd_image_info *mtd_img_by_uuid(const char *uuidbuf)
+{
+	int num_images = sizeof(fwu_mtd_images) / sizeof(struct fwu_mtd_image_info);
+
+	for (int i = 0; i < num_images; i++)
+		if (!strcmp(uuidbuf, fwu_mtd_images[i].uuidbuf))
+			return &fwu_mtd_images[i];
+
+	return NULL;
+}
+
+int fwu_mtd_get_alt_num(efi_guid_t *image_id, u8 *alt_num,
+			const char *mtd_dev)
+{
+	int i, nalt;
+	int ret = -1;
+	struct mtd_info *mtd;
+	struct dfu_entity *dfu;
+	fdt_addr_t offset, size = 0;
+	char uuidbuf[UUID_STR_LEN + 1];
+	struct fwu_mtd_image_info *mtd_img_info;
+
+	mtd_probe_devices();
+	mtd = get_mtd_device_nm(mtd_dev);
+
+	uuid_bin_to_str(image_id->b, uuidbuf, UUID_STR_FORMAT_STD);
+
+	mtd_img_info = mtd_img_by_uuid(uuidbuf);
+	if (!mtd_img_info) {
+		log_err("%s: Not found partition for image %s\n", __func__, uuidbuf);
+		return -ENOENT;
+	}
+	offset = mtd_img_info->start;
+	size = mtd_img_info->size;
+
+	dfu_init_env_entities(NULL, NULL);
+
+	nalt = 0;
+	list_for_each_entry(dfu, &dfu_list, list)
+		nalt++;
+
+	if (!nalt) {
+		log_warning("No entities in dfu_alt_info\n");
+		dfu_free_entities();
+		return -ENOENT;
+	}
+
+	for (i = 0; i < nalt; i++) {
+		dfu = dfu_get_entity(i);
+
+		/* Only MTD RAW access */
+		if (!dfu || dfu->dev_type != DFU_DEV_MTD ||
+		    dfu->layout != DFU_RAW_ADDR ||
+			dfu->data.mtd.start != offset ||
+			dfu->data.mtd.size != size)
+			continue;
+
+		*alt_num = dfu->alt;
+		ret = 0;
+		break;
+	}
+
+	dfu_free_entities();
+
+	log_debug("%s: %s -> %d\n", __func__, uuidbuf, *alt_num);
+	return ret;
+}
+
+static int gen_image_alt_info(char *buf, size_t len, int sidx,
+			      struct fwu_image_entry *img, struct mtd_info *mtd)
+{
+	int i;
+	char *p = buf, *end = buf + len;
+
+	p += snprintf(p, end - p, "mtd %s", mtd->name);
+	if (end < p) {
+		log_err("%s:%d Run out of buffer\n", __func__, __LINE__);
+		return -E2BIG;
+	}
+
+	/*
+	 * List the image banks in the FWU mdata and search the corresponding
+	 * partition based on partition's uuid.
+	 */
+	for (i = 0; i < CONFIG_FWU_NUM_BANKS; i++) {
+		struct fwu_mtd_image_info *mtd_img_info;
+		struct fwu_image_bank_info *bank;
+		char uuidbuf[UUID_STR_LEN + 1];
+		u32 offset, size;
+
+		/* Query a partition by image UUID */
+		bank = &img->img_bank_info[i];
+		uuid_bin_to_str(bank->image_uuid.b, uuidbuf, UUID_STR_FORMAT_STD);
+
+		mtd_img_info = mtd_img_by_uuid(uuidbuf);
+		if (!mtd_img_info) {
+			log_err("%s: Not found partition for image %s\n", __func__, uuidbuf);
+			break;
+		}
+
+		offset = mtd_img_info->start;
+		size = mtd_img_info->size;
+
+		p += snprintf(p, end - p, "%sbank%d raw %x %x",
+			      i == 0 ? "=" : ";", i, offset, size);
+		if (end < p) {
+			log_err("%s:%d Run out of buffer\n", __func__, __LINE__);
+			return -E2BIG;
+		}
+	}
+
+	if (i == CONFIG_FWU_NUM_BANKS)
+		return 0;
+
+	return -ENOENT;
+}
+
+int fwu_gen_alt_info_from_mtd(char *buf, size_t len, struct mtd_info *mtd)
+{
+	struct fwu_mdata mdata;
+	int i, l, ret;
+
+	ret = fwu_get_mdata(&mdata);
+	if (ret < 0) {
+		log_err("Failed to get the FWU mdata.\n");
+		return ret;
+	}
+
+	for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) {
+		ret = gen_image_alt_info(buf, len, i * CONFIG_FWU_NUM_BANKS,
+					 &mdata.img_entry[i], mtd);
+		if (ret)
+			break;
+
+		l = strlen(buf);
+		/* Replace the last ';' with '&' if there is another image. */
+		if (i != CONFIG_FWU_NUM_IMAGES_PER_BANK - 1 && l)
+			buf[l - 1] = '&';
+		len -= l;
+		buf += l;
+	}
+
+	return ret;
+}