[RFC PATCH v2 2/8] FWU: Add FWU metadata access functions for GPT partitioned block devices
Masami Hiramatsu
masami.hiramatsu at linaro.org
Sat Dec 25 07:57:46 CET 2021
Hi Sughosh,
2021年12月25日(土) 2:08 Sughosh Ganu <sughosh.ganu at linaro.org>:
>
> hi Masami,
>
> On Fri, 24 Dec 2021 at 15:29, Masami Hiramatsu
> <masami.hiramatsu at linaro.org> wrote:
> >
> > Hi Sughosh,
> >
> > 2021年12月19日(日) 16:06 Sughosh Ganu <sughosh.ganu at 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
More information about the U-Boot
mailing list