| Message ID | 20230327211603.498936-1-jaswinder.singh@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | FWU: Add support for mtd backed feature on DeveloperBox | expand |
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
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 --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; +}