| Message ID | 20220817124323.375968-9-sughosh.ganu@linaro.org |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
| Series | FWU: Add FWU Multi Bank Update feature support | expand |
Hi Sughosh, On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > The FWU Multi Bank Update specification requires the Update Agent to > carry out certain checks at the time of platform boot. The Update > Agent is the component which is responsible for updating the firmware > components and maintaining and keeping the metadata in sync. > > The spec requires that the Update Agent perform the following checks > at the time of boot > * Sanity check of both the metadata copies maintained by the platform. > * Get the boot index passed to U-Boot by the prior stage bootloader > and use this value for metadata bookkeeping. > * Check if the system is booting in Trial State. If the system boots > in the Trial State for more than a specified number of boot counts, > change the Active Bank to be booting the platform from. > > Add these checks in the board initialisation sequence, invoked after > relocation. > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > --- > Changes since V7: > * Introduce trial_counter_update() to increment and delete the > TrialStateCtr variable. > * Introduce in_trial_state() to check if the platform is booting in > Trial State. > > common/board_r.c | 5 ++ > include/fwu.h | 3 + > lib/fwu_updates/fwu.c | 175 ++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 183 insertions(+) We should use the event system instead of adding another hook. We are trying to get rid of them! Also, please separate out the fwu change from touching common code (although you may not need to touch comment code with events). Regards, SImon
hi Simon, On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote: > > Hi Sughosh, > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > The FWU Multi Bank Update specification requires the Update Agent to > > carry out certain checks at the time of platform boot. The Update > > Agent is the component which is responsible for updating the firmware > > components and maintaining and keeping the metadata in sync. > > > > The spec requires that the Update Agent perform the following checks > > at the time of boot > > * Sanity check of both the metadata copies maintained by the platform. > > * Get the boot index passed to U-Boot by the prior stage bootloader > > and use this value for metadata bookkeeping. > > * Check if the system is booting in Trial State. If the system boots > > in the Trial State for more than a specified number of boot counts, > > change the Active Bank to be booting the platform from. > > > > Add these checks in the board initialisation sequence, invoked after > > relocation. > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > --- > > Changes since V7: > > * Introduce trial_counter_update() to increment and delete the > > TrialStateCtr variable. > > * Introduce in_trial_state() to check if the platform is booting in > > Trial State. > > > > common/board_r.c | 5 ++ > > include/fwu.h | 3 + > > lib/fwu_updates/fwu.c | 175 ++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 183 insertions(+) > > We should use the event system instead of adding another hook. We are > trying to get rid of them! Okay, I will check about this. > > Also, please separate out the fwu change from touching common code > (although you may not need to touch comment code with events). The FWU specification requires certain checks to be run at the platform boot time. Not sure how else will that be done(although this might not be relevant if I can use events). -sughosh
Hi Sughosh, On Thu, 18 Aug 2022 at 05:42, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > hi Simon, > > On Thu, 18 Aug 2022 at 08:51, Simon Glass <sjg@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Wed, 17 Aug 2022 at 06:44, Sughosh Ganu <sughosh.ganu@linaro.org> wrote: > > > > > > The FWU Multi Bank Update specification requires the Update Agent to > > > carry out certain checks at the time of platform boot. The Update > > > Agent is the component which is responsible for updating the firmware > > > components and maintaining and keeping the metadata in sync. > > > > > > The spec requires that the Update Agent perform the following checks > > > at the time of boot > > > * Sanity check of both the metadata copies maintained by the platform. > > > * Get the boot index passed to U-Boot by the prior stage bootloader > > > and use this value for metadata bookkeeping. > > > * Check if the system is booting in Trial State. If the system boots > > > in the Trial State for more than a specified number of boot counts, > > > change the Active Bank to be booting the platform from. > > > > > > Add these checks in the board initialisation sequence, invoked after > > > relocation. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> > > > --- > > > Changes since V7: > > > * Introduce trial_counter_update() to increment and delete the > > > TrialStateCtr variable. > > > * Introduce in_trial_state() to check if the platform is booting in > > > Trial State. > > > > > > common/board_r.c | 5 ++ > > > include/fwu.h | 3 + > > > lib/fwu_updates/fwu.c | 175 ++++++++++++++++++++++++++++++++++++++++++ > > > 3 files changed, 183 insertions(+) > > > > We should use the event system instead of adding another hook. We are > > trying to get rid of them! > > Okay, I will check about this. > > > > > Also, please separate out the fwu change from touching common code > > (although you may not need to touch comment code with events). > > The FWU specification requires certain checks to be run at the > platform boot time. Not sure how else will that be done(although this > might not be relevant if I can use events). > You can have an 'fwu' command to handle this. Regards, Simon > -sughosh
diff --git a/common/board_r.c b/common/board_r.c index 56eb60fa27..425644dda7 100644 --- a/common/board_r.c +++ b/common/board_r.c @@ -18,6 +18,7 @@ #ifdef CONFIG_MTD_NOR_FLASH #include <flash.h> #endif +#include <fwu.h> #include <hang.h> #include <image.h> #include <irq_func.h> @@ -788,6 +789,10 @@ static init_fnc_t init_sequence_r[] = { #if defined(CONFIG_PRAM) initr_mem, #endif + +#ifdef CONFIG_FWU_MULTI_BANK_UPDATE + fwu_boottime_checks, +#endif run_main_loop, }; diff --git a/include/fwu.h b/include/fwu.h index cfc9aafc7d..d863b23a5a 100644 --- a/include/fwu.h +++ b/include/fwu.h @@ -37,6 +37,9 @@ struct fwu_mdata_ops { EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \ 0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23) +u8 fwu_update_checks_pass(void); +int fwu_boottime_checks(void); + int fwu_get_mdata(struct fwu_mdata **mdata); int fwu_update_mdata(struct fwu_mdata *mdata); int fwu_get_active_index(u32 *active_idx); diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c index 9808036eec..4871e0ccd6 100644 --- a/lib/fwu_updates/fwu.c +++ b/lib/fwu_updates/fwu.c @@ -3,8 +3,20 @@ * Copyright (c) 2022, Linaro Limited */ + +#include <dm.h> +#include <efi.h> +#include <efi_loader.h> +#include <efi_variable.h> #include <fwu.h> #include <fwu_mdata.h> +#include <malloc.h> + +#include <linux/errno.h> +#include <linux/types.h> + +static u8 trial_state; +static u8 boottime_check; __weak int fwu_plat_get_update_index(u32 *update_idx) { @@ -20,3 +32,166 @@ __weak int fwu_plat_get_update_index(u32 *update_idx) return ret; } + +static int trial_counter_update(u16 *trial_state_ctr) +{ + bool delete; + u32 var_attr; + efi_status_t status; + efi_uintn_t var_size; + + delete = !trial_state_ctr ? true : false; + var_size = !trial_state_ctr ? 0 : (efi_uintn_t)sizeof(*trial_state_ctr); + var_attr = !trial_state_ctr ? 0 : EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS; + status = efi_set_variable_int(u"TrialStateCtr", + &efi_global_variable_guid, + var_attr, + var_size, trial_state_ctr, false); + + if ((delete && (status != EFI_NOT_FOUND && + status != EFI_SUCCESS)) || + (!delete && status != EFI_SUCCESS)) + return -1; + + return 0; +} + +static int in_trial_state(struct fwu_mdata *mdata) +{ + u32 i, active_bank; + struct fwu_image_entry *img_entry; + struct fwu_image_bank_info *img_bank_info; + + active_bank = mdata->active_index; + img_entry = &mdata->img_entry[0]; + for (i = 0; i < CONFIG_FWU_NUM_IMAGES_PER_BANK; i++) { + img_bank_info = &img_entry[i].img_bank_info[active_bank]; + if (!img_bank_info->accepted) { + return 1; + } + } + + return 0; +} + +static int fwu_trial_state_check(void) +{ + int ret; + efi_status_t status; + efi_uintn_t var_size; + u16 trial_state_ctr; + u32 var_attributes, active_idx; + struct fwu_mdata *mdata = NULL; + + ret = fwu_get_mdata(&mdata); + if (ret) + return ret; + + if ((trial_state = in_trial_state(mdata))) { + var_size = (efi_uintn_t)sizeof(trial_state_ctr); + log_info("System booting in Trial State\n"); + var_attributes = EFI_VARIABLE_NON_VOLATILE | + EFI_VARIABLE_BOOTSERVICE_ACCESS; + status = efi_get_variable_int(u"TrialStateCtr", + &efi_global_variable_guid, + &var_attributes, + &var_size, &trial_state_ctr, + NULL); + if (status != EFI_SUCCESS) { + log_err("Unable to read TrialStateCtr variable\n"); + ret = -1; + goto out; + } + + ++trial_state_ctr; + if (trial_state_ctr > CONFIG_FWU_TRIAL_STATE_CNT) { + log_info("Trial State count exceeded. Revert back to previous_active_index\n"); + active_idx = mdata->active_index; + ret = fwu_revert_boot_index(); + if (ret) { + log_err("Unable to revert active_index\n"); + goto out; + } + + /* Delete the TrialStateCtr variable */ + ret = trial_counter_update(NULL); + if (ret) { + log_err("Unable to delete TrialStateCtr variable\n"); + goto out; + } + } else { + ret = trial_counter_update(&trial_state_ctr); + if (ret) { + log_err("Unable to increment TrialStateCtr variable\n"); + goto out; + } + } + } else { + /* Delete the variable */ + ret = trial_counter_update(NULL); + if (ret) { + log_err("Unable to delete TrialStateCtr variable\n"); + } + } + +out: + free(mdata); + return ret; +} + +u8 fwu_update_checks_pass(void) +{ + return !trial_state && boottime_check; +} + +int fwu_boottime_checks(void) +{ + int ret; + u32 boot_idx, active_idx; + + ret = fwu_mdata_check(); + if (ret) { + return 0; + } + + /* + * Get the Boot Index, i.e. the bank from + * which the platform has booted. This value + * gets passed from the ealier stage bootloader + * which booted u-boot, e.g. tf-a. If the + * boot index is not the same as the + * active_index read from the FWU metadata, + * update the active_index. + */ + fwu_plat_get_bootidx(&boot_idx); + if (boot_idx >= CONFIG_FWU_NUM_BANKS) { + log_err("Received incorrect value of boot_index\n"); + return 0; + } + + ret = fwu_get_active_index(&active_idx); + if (ret) { + log_err("Unable to read active_index\n"); + return 0; + } + + if (boot_idx != active_idx) { + log_info("Boot idx %u is not matching active idx %u, changing active_idx\n", + boot_idx, active_idx); + ret = fwu_update_active_index(boot_idx); + if (!ret) + boottime_check = 1; + + return 0; + } + + if (efi_init_obj_list() != EFI_SUCCESS) + return 0; + + ret = fwu_trial_state_check(); + if (!ret) + boottime_check = 1; + + return 0; +}
The FWU Multi Bank Update specification requires the Update Agent to carry out certain checks at the time of platform boot. The Update Agent is the component which is responsible for updating the firmware components and maintaining and keeping the metadata in sync. The spec requires that the Update Agent perform the following checks at the time of boot * Sanity check of both the metadata copies maintained by the platform. * Get the boot index passed to U-Boot by the prior stage bootloader and use this value for metadata bookkeeping. * Check if the system is booting in Trial State. If the system boots in the Trial State for more than a specified number of boot counts, change the Active Bank to be booting the platform from. Add these checks in the board initialisation sequence, invoked after relocation. Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org> --- Changes since V7: * Introduce trial_counter_update() to increment and delete the TrialStateCtr variable. * Introduce in_trial_state() to check if the platform is booting in Trial State. common/board_r.c | 5 ++ include/fwu.h | 3 + lib/fwu_updates/fwu.c | 175 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 183 insertions(+)