Hi Sughosh, 2021年12月25日(土) 2:08 Sughosh Ganu <sughosh.g...@linaro.org>: > > 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 *.
What about passing image index and bank index ? :-) > > > 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. Yes, that's right. Then I don't need to repeat the same code :-) Thank you, > > -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 -- Masami Hiramatsu