hi Masami, On Fri, 24 Dec 2021 at 15:29, Masami Hiramatsu <masami.hirama...@linaro.org> wrote: > > Hi Sughosh, > > 2021年12月19日(日) 16:06 Sughosh Ganu <sughosh.g...@linaro.org>: > > > +static int gpt_get_image_alt_num(struct blk_desc *desc, > > + efi_guid_t image_type_id, > > + u32 update_bank, int *alt_no) > > +{ > > + int ret, i; > > + u32 part; > > + struct fwu_mdata *mdata; > > + struct fwu_image_entry *img_entry; > > + struct fwu_image_bank_info *img_bank_info; > > + struct disk_partition info; > > + efi_guid_t unique_part_guid; > > + efi_guid_t image_guid = NULL_GUID; > > + > > + ret = gpt_get_mdata(&mdata); > > + if (ret < 0) { > > + log_err("Unable to read valid FWU metadata\n"); > > + goto out; > > + } > > + > > + /* > > + * The FWU metadata has been read. Now get the image_uuid for the > > + * image with the update_bank. > > + */ > > + for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) { > > + if (!guidcmp(&image_type_id, > > + &mdata->img_entry[i].image_type_uuid)) { > > + img_entry = &mdata->img_entry[i]; > > + img_bank_info = > > &img_entry->img_bank_info[update_bank]; > > + guidcpy(&image_guid, &img_bank_info->image_uuid); > > + break; > > + } > > + } > > + > > + /* > > + * Now read the GPT Partition Table Entries to find a matching > > + * partition with UniquePartitionGuid value. We need to iterate > > + * through all the GPT partitions since they might be in any > > + * order > > + */ > > + for (i = 1; i < MAX_SEARCH_PARTITIONS; i++) { > > + if (part_get_info(desc, i, &info)) > > + continue; > > + uuid_str_to_bin(info.uuid, unique_part_guid.b, > > + UUID_STR_FORMAT_GUID); > > + > > + if (!guidcmp(&unique_part_guid, &image_guid)) { > > + /* Found the partition */ > > + part = i; > > + *alt_no = fwu_plat_get_alt_num(desc->devnum, &part); > > This is still GPT depending interface. In my use case (no GPT but raw SPI > flash) > I don't have devnum. Moreover, this one is used only from the > fwu_mdata_gpt_blk.c.
You are right. I will change this api to not have the first parameter. The function will then pass only the identifier which is a void *. > Please ensure (and comment) that this is only for GPT user. > > > + if (*alt_no != -1) > > + log_info("alt_num %d for partition %pUl\n", > > + *alt_no, &image_guid); > > + ret = 0; > > + break; > > + } > > + } > > + > > + if (*alt_no == -1) { > > + log_err("alt_num not found for partition with GUID %pUl\n", > > + &image_guid); > > + ret = -EINVAL; > > + } > > + > > + if (i == MAX_SEARCH_PARTITIONS) { > > + log_err("Partition with the image guid not found\n"); > > + ret = -EINVAL; > > + } > > + > > +out: > > + free(mdata); > > + > > + return ret; > > +} > > + > > +int fwu_gpt_update_active_index(u32 active_idx) > > +{ > > + int ret; > > + void *buf; > > + struct fwu_mdata *mdata; > > + > > + if (active_idx > CONFIG_FWU_NUM_BANKS) { > > + printf("Active index value to be updated is incorrect\n"); > > + return -1; > > + } > > + > > + ret = gpt_get_mdata(&mdata); > > + if (ret < 0) { > > + log_err("Unable to get valid FWU metadata\n"); > > + goto out; > > + } > > + > > + /* > > + * Update the active index and previous_active_index fields > > + * in the FWU metadata > > + */ > > + mdata->previous_active_index = mdata->active_index; > > + mdata->active_index = active_idx; > > + > > + > > + /* > > + * Calculate the crc32 for the updated FWU metadata > > + * and put the updated value in the FWU metadata crc32 > > + * field > > + */ > > + buf = &mdata->version; > > + mdata->crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32)); > > + > > + /* > > + * Now write this updated FWU metadata to both the > > + * FWU metadata partitions > > + */ > > + ret = gpt_update_mdata(mdata); > > + if (ret < 0) { > > + log_err("Failed to update FWU metadata partitions\n"); > > + ret = -EIO; > > + } > > + > > +out: > > + free(mdata); > > + > > + return ret; > > +} > > + > > +int fwu_gpt_get_image_alt_num(efi_guid_t image_type_id, u32 update_bank, > > + int *alt_no) > > +{ > > + int ret; > > + struct blk_desc *desc; > > + > > + ret = fwu_plat_get_blk_desc(&desc); > > + if (ret < 0) { > > + log_err("Block device not found\n"); > > + return -ENODEV; > > + } > > + > > + return gpt_get_image_alt_num(desc, image_type_id, update_bank, > > alt_no); > > +} > > + > > +int fwu_gpt_mdata_check(void) > > +{ > > + /* > > + * Check if both the copies of the FWU metadata are > > + * valid. If one has gone bad, restore it from the > > + * other good copy. > > + */ > > + return gpt_check_mdata_validity(); > > +} > > + > > +int fwu_gpt_get_mdata(struct fwu_mdata **mdata) > > +{ > > + return gpt_get_mdata(mdata); > > +} > > + > > +int fwu_gpt_revert_boot_index(void) > > +{ > > + int ret; > > + void *buf; > > + u32 cur_active_index; > > + struct fwu_mdata *mdata; > > + > > + ret = gpt_get_mdata(&mdata); > > + if (ret < 0) { > > + log_err("Unable to get valid FWU metadata\n"); > > + goto out; > > + } > > + > > + /* > > + * Swap the active index and previous_active_index fields > > + * in the FWU metadata > > + */ > > + cur_active_index = mdata->active_index; > > + mdata->active_index = mdata->previous_active_index; > > + mdata->previous_active_index = cur_active_index; > > + > > + /* > > + * Calculate the crc32 for the updated FWU metadata > > + * and put the updated value in the FWU metadata crc32 > > + * field > > + */ > > + buf = &mdata->version; > > + mdata->crc32 = crc32(0, buf, sizeof(*mdata) - sizeof(u32)); > > + > > + /* > > + * Now write this updated FWU metadata to both the > > + * FWU metadata partitions > > + */ > > + ret = gpt_update_mdata(mdata); > > If we have the fwu_update_mdata(), you can make this > fwu_gpt_revert_boot_index() > and above fwu_gpt_update_boot_index() platform agnostic. (in that case > "gpt" must > be removed), because there is no dependency to GPT. Okay. I believe you are suggesting having a GPT specific implementation only for fwu_update_mdata, and move fwu_gpt_revert_boot_index and fwu_gpt_update_active_index as common functions with appropriate renaming. I can do that. -sughosh > > > + if (ret < 0) { > > + log_err("Failed to update FWU metadata partitions\n"); > > + ret = -EIO; > > + } > > + > > +out: > > + free(mdata); > > + > > + return ret; > > +} > > + > > +static int fwu_gpt_set_clear_image_accept(efi_guid_t *img_type_id, > > + u32 bank, u8 action) > > +{ > > + void *buf; > > + int ret, i; > > + u32 nimages; > > + struct fwu_mdata *mdata; > > + struct fwu_image_entry *img_entry; > > + struct fwu_image_bank_info *img_bank_info; > > + > > + ret = gpt_get_mdata(&mdata); > > + if (ret < 0) { > > + log_err("Unable to get valid FWU metadata\n"); > > + goto out; > > + } > > + > > + if (action == IMAGE_ACCEPT_SET) > > + bank = mdata->active_index; > > + > > + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK; > > + img_entry = &mdata->img_entry[0]; > > + for (i = 0; i < nimages; i++) { > > + if (!guidcmp(&img_entry[i].image_type_uuid, img_type_id)) { > > + img_bank_info = &img_entry[i].img_bank_info[bank]; > > + if (action == IMAGE_ACCEPT_SET) > > + img_bank_info->accepted |= > > FWU_IMAGE_ACCEPTED; > > + else > > + img_bank_info->accepted = 0; > > + > > + buf = &mdata->version; > > + mdata->crc32 = crc32(0, buf, sizeof(*mdata) - > > + sizeof(u32)); > > + > > + ret = gpt_update_mdata(mdata); > > This function is also doing the generic things. Only gpt_update_mdata() is > the barrier. I would like to suggest you to add ".update_mdata" member > to the fwu_mdata_ops. > > > Thank you, > > > + goto out; > > + } > > + } > > + > > + /* Image not found */ > > + ret = -EINVAL; > > + > > +out: > > + free(mdata); > > + > > + return ret; > > +} > > + > > +int fwu_gpt_accept_image(efi_guid_t *img_type_id) > > +{ > > + return fwu_gpt_set_clear_image_accept(img_type_id, 0, > > + IMAGE_ACCEPT_SET); > > +} > > + > > +int fwu_gpt_clear_accept_image(efi_guid_t *img_type_id, u32 bank) > > +{ > > + return fwu_gpt_set_clear_image_accept(img_type_id, bank, > > + IMAGE_ACCEPT_CLEAR); > > +} > > + > > +struct fwu_mdata_ops fwu_gpt_blk_ops = { > > + .get_active_index = fwu_gpt_get_active_index, > > + .update_active_index = fwu_gpt_update_active_index, > > + .get_image_alt_num = fwu_gpt_get_image_alt_num, > > + .mdata_check = fwu_gpt_mdata_check, > > + .revert_boot_index = fwu_gpt_revert_boot_index, > > + .set_accept_image = fwu_gpt_accept_image, > > + .clear_accept_image = fwu_gpt_clear_accept_image, > > + .get_mdata = fwu_gpt_get_mdata, > > +}; > > -- > > 2.17.1 > > > > > -- > Masami Hiramatsu