diff mbox series

[v15,09/15] FWU: Add boot time checks as highlighted by the FWU specification

Message ID 20221021124608.681387-10-sughosh.ganu@linaro.org
State Accepted
Commit 7e9814cc6c1dcda8cb8028e1d34483387889e149
Headers show
Series FWU: Add FWU Multi Bank Update feature support | expand

Commit Message

Sughosh Ganu Oct. 21, 2022, 12:46 p.m. UTC 
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.

Call these checks through the main loop event at the time of platform
boot.

Signed-off-by: Sughosh Ganu <sughosh.ganu@linaro.org>
Reviewed-by: Etienne Carriere <etienne.carriere@linaro.org>
---
Changes since V14:
* s/trial_state/in_trial/g as suggested by Ilias
* Move the call to check for trial state in fwu_boottime_checks() as
  suggested by Ilias
* Put the checks done earlier in fwu_trial_state_check() in
  fwu_trial_count_update() with the deletion of the variable called
  from a single place as suggested by Ilias
* Add a function fwu_empty_capsule_checks_pass() to check if the empty
  capsules can be applied

 include/fwu.h         |  26 ++++++
 lib/fwu_updates/fwu.c | 192 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 217 insertions(+), 1 deletion(-)

Comments

Ilias Apalodimas Oct. 21, 2022, 3:08 p.m. UTC | #1 
Hi Sughosh,

>  
>  #include <linux/errno.h>
>  #include <linux/types.h>
> @@ -44,6 +53,96 @@ static int fwu_get_dev_mdata(struct udevice **dev, struct fwu_mdata *mdata)
>  	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 trial_counter_read(u16 *trial_state_ctr)
> +{
> +	efi_status_t status;
> +	efi_uintn_t var_size;
> +
> +	var_size = (efi_uintn_t)sizeof(trial_state_ctr);
> +	status = efi_get_variable_int(u"TrialStateCtr",
> +				      &efi_global_variable_guid,
> +				      NULL,
> +				      &var_size, trial_state_ctr,
> +				      NULL);
> +	if (status != EFI_SUCCESS) {
> +		log_err("Unable to read TrialStateCtr variable\n");
> +		return -1;
> +	}
> +
> +	return 0;
> +}
> +
> +static int fwu_trial_count_update(void)
> +{
> +	int ret;
> +	u16 trial_state_ctr;
> +
> +	ret = trial_counter_read(&trial_state_ctr);
> +	if (ret) {
> +		log_debug("Unable to read trial_state_ctr\n");
> +		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");
> +		ret = fwu_revert_boot_index();
> +		if (ret)
> +			log_err("Unable to revert active_index\n");
> +		ret = 1;
> +	} else {
> +		ret = trial_counter_update(&trial_state_ctr);
> +		if (ret)
> +			log_err("Unable to increment TrialStateCtr variable\n");
> +	}
> +
> +out:
> +	return ret;
> +}
> +
> +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) {
> +			log_info("System booting in Trial State\n");
> +			return 1;
> +		}
> +	}
> +
> +	return 0;
> +}
> +
>  static int fwu_get_image_type_id(u8 *image_index, efi_guid_t *image_type_id)
>  {
>  	u8 index;
> @@ -499,3 +598,94 @@ __weak int fwu_plat_get_update_index(uint *update_idx)
>  
>  	return ret;
>  }
> +
> +/**
> + * fwu_update_checks_pass() - Check if FWU update can be done
> + *
> + * Check if the FWU update can be executed. The updates are
> + * allowed only when the platform is not in Trial State and
> + * the boot time checks have passed
> + *
> + * Return: 1 if OK, 0 if checks do not pass
> + *
> + */
> +u8 fwu_update_checks_pass(void)
> +{
> +	return !in_trial && boottime_check;
> +}
> +
> +/**
> + * fwu_empty_capsule_checks_pass() - Check if empty capsule can be processed
> + *
> + * Check if the empty capsule can be processed to either accept or revert
> + * an earlier executed update. The empty capsules need to be processed
> + * only when the platform is in Trial State and the boot time checks have
> + * passed
> + *
> + * Return: 1 if OK, 0 if not to be allowed
> + *
> + */
> +u8 fwu_empty_capsule_checks_pass(void)
> +{
> +	return in_trial && boottime_check;
> +}
> +
> +static int fwu_boottime_checks(void *ctx, struct event *event)
> +{
> +	int ret;
> +	u32 boot_idx, active_idx;
> +	struct udevice *dev;
> +	struct fwu_mdata mdata = { 0 };
> +
> +	ret = fwu_check_mdata_validity();
> +	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_set_active_index(boot_idx);
> +		if (!ret)
> +			boottime_check = 1;
> +
> +		return 0;
> +	}
> +
> +	if (efi_init_obj_list() != EFI_SUCCESS)
> +		return 0;
> +
> +	ret = fwu_get_dev_mdata(&dev, &mdata);
> +	if (ret)
> +		return ret;
> +
> +	in_trial = in_trial_state(&mdata);
> +	if (!in_trial || (ret = fwu_trial_count_update()) > 0)

Why do we need to assign ret here?
if (!in_trial || !fwu_trial_count_update()) should be enough

> +		ret = trial_counter_update(NULL);
> +
> +	if (!ret)
> +		boottime_check = 1;
> +
> +	return 0;
> +}
> +EVENT_SPY(EVT_MAIN_LOOP, fwu_boottime_checks);
> -- 
> 2.34.1
> 

With that
Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Sughosh Ganu Oct. 21, 2022, 4:30 p.m. UTC | #2 
hi Ilias,

On Fri, 21 Oct 2022 at 20:38, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Sughosh,
>
> >
> >  #include <linux/errno.h>
> >  #include <linux/types.h>
> > @@ -44,6 +53,96 @@ static int fwu_get_dev_mdata(struct udevice **dev, struct fwu_mdata *mdata)
> >       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 trial_counter_read(u16 *trial_state_ctr)
> > +{
> > +     efi_status_t status;
> > +     efi_uintn_t var_size;
> > +
> > +     var_size = (efi_uintn_t)sizeof(trial_state_ctr);
> > +     status = efi_get_variable_int(u"TrialStateCtr",
> > +                                   &efi_global_variable_guid,
> > +                                   NULL,
> > +                                   &var_size, trial_state_ctr,
> > +                                   NULL);
> > +     if (status != EFI_SUCCESS) {
> > +             log_err("Unable to read TrialStateCtr variable\n");
> > +             return -1;
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> > +static int fwu_trial_count_update(void)
> > +{
> > +     int ret;
> > +     u16 trial_state_ctr;
> > +
> > +     ret = trial_counter_read(&trial_state_ctr);
> > +     if (ret) {
> > +             log_debug("Unable to read trial_state_ctr\n");
> > +             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");
> > +             ret = fwu_revert_boot_index();
> > +             if (ret)
> > +                     log_err("Unable to revert active_index\n");
> > +             ret = 1;
> > +     } else {
> > +             ret = trial_counter_update(&trial_state_ctr);
> > +             if (ret)
> > +                     log_err("Unable to increment TrialStateCtr variable\n");
> > +     }
> > +
> > +out:
> > +     return ret;
> > +}
> > +
> > +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) {
> > +                     log_info("System booting in Trial State\n");
> > +                     return 1;
> > +             }
> > +     }
> > +
> > +     return 0;
> > +}
> > +
> >  static int fwu_get_image_type_id(u8 *image_index, efi_guid_t *image_type_id)
> >  {
> >       u8 index;
> > @@ -499,3 +598,94 @@ __weak int fwu_plat_get_update_index(uint *update_idx)
> >
> >       return ret;
> >  }
> > +
> > +/**
> > + * fwu_update_checks_pass() - Check if FWU update can be done
> > + *
> > + * Check if the FWU update can be executed. The updates are
> > + * allowed only when the platform is not in Trial State and
> > + * the boot time checks have passed
> > + *
> > + * Return: 1 if OK, 0 if checks do not pass
> > + *
> > + */
> > +u8 fwu_update_checks_pass(void)
> > +{
> > +     return !in_trial && boottime_check;
> > +}
> > +
> > +/**
> > + * fwu_empty_capsule_checks_pass() - Check if empty capsule can be processed
> > + *
> > + * Check if the empty capsule can be processed to either accept or revert
> > + * an earlier executed update. The empty capsules need to be processed
> > + * only when the platform is in Trial State and the boot time checks have
> > + * passed
> > + *
> > + * Return: 1 if OK, 0 if not to be allowed
> > + *
> > + */
> > +u8 fwu_empty_capsule_checks_pass(void)
> > +{
> > +     return in_trial && boottime_check;
> > +}
> > +
> > +static int fwu_boottime_checks(void *ctx, struct event *event)
> > +{
> > +     int ret;
> > +     u32 boot_idx, active_idx;
> > +     struct udevice *dev;
> > +     struct fwu_mdata mdata = { 0 };
> > +
> > +     ret = fwu_check_mdata_validity();
> > +     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_set_active_index(boot_idx);
> > +             if (!ret)
> > +                     boottime_check = 1;
> > +
> > +             return 0;
> > +     }
> > +
> > +     if (efi_init_obj_list() != EFI_SUCCESS)
> > +             return 0;
> > +
> > +     ret = fwu_get_dev_mdata(&dev, &mdata);
> > +     if (ret)
> > +             return ret;
> > +
> > +     in_trial = in_trial_state(&mdata);
> > +     if (!in_trial || (ret = fwu_trial_count_update()) > 0)
>
> Why do we need to assign ret here?
> if (!in_trial || !fwu_trial_count_update()) should be enough

The fwu_trial_count_update() would return a 0 in case the platform is
in trial state, and the TrialStateCtr variable gets updated
successfully. In that case, the if condition would evaluate to 0, and
the trial_counter_update(NULL) will not get called. So ret will have
to be set for that case, since we set the value of boottime_check flag
based on the value of ret below. Thanks.

-sughosh

>
> > +             ret = trial_counter_update(NULL);
> > +
> > +     if (!ret)
> > +             boottime_check = 1;
> > +
> > +     return 0;
> > +}
> > +EVENT_SPY(EVT_MAIN_LOOP, fwu_boottime_checks);
> > --
> > 2.34.1
> >
>
> With that
> Acked-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>
>
diff mbox series

Patch

diff --git a/include/fwu.h b/include/fwu.h
index 73628c59f4..a8ed67b0e0 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -353,4 +353,30 @@  int fwu_plat_get_update_index(uint *update_idx);
  *
  */
 void fwu_plat_get_bootidx(uint *boot_idx);
+
+/**
+ * fwu_update_checks_pass() - Check if FWU update can be done
+ *
+ * Check if the FWU update can be executed. The updates are
+ * allowed only when the platform is not in Trial State and
+ * the boot time checks have passed
+ *
+ * Return: 1 if OK, 0 if checks do not pass
+ *
+ */
+u8 fwu_update_checks_pass(void);
+
+/**
+ * fwu_empty_capsule_checks_pass() - Check if empty capsule can be processed
+ *
+ * Check if the empty capsule can be processed to either accept or revert
+ * an earlier executed update. The empty capsules need to be processed
+ * only when the platform is in Trial State and the boot time checks have
+ * passed
+ *
+ * Return: 1 if OK, 0 if not to be allowed
+ *
+ */
+u8 fwu_empty_capsule_checks_pass(void);
+
 #endif /* _FWU_H_ */
diff --git a/lib/fwu_updates/fwu.c b/lib/fwu_updates/fwu.c
index e73d5f11ff..c9a9022a59 100644
--- a/lib/fwu_updates/fwu.c
+++ b/lib/fwu_updates/fwu.c
@@ -4,10 +4,19 @@ 
  */
 
 #include <dm.h>
+#include <efi.h>
 #include <efi_loader.h>
+#include <efi_variable.h>
+#include <event.h>
 #include <fwu.h>
 #include <fwu_mdata.h>
-#include <log.h>
+#include <malloc.h>
+
+#include <linux/errno.h>
+#include <linux/types.h>
+
+static u8 in_trial;
+static u8 boottime_check;
 
 #include <linux/errno.h>
 #include <linux/types.h>
@@ -44,6 +53,96 @@  static int fwu_get_dev_mdata(struct udevice **dev, struct fwu_mdata *mdata)
 	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 trial_counter_read(u16 *trial_state_ctr)
+{
+	efi_status_t status;
+	efi_uintn_t var_size;
+
+	var_size = (efi_uintn_t)sizeof(trial_state_ctr);
+	status = efi_get_variable_int(u"TrialStateCtr",
+				      &efi_global_variable_guid,
+				      NULL,
+				      &var_size, trial_state_ctr,
+				      NULL);
+	if (status != EFI_SUCCESS) {
+		log_err("Unable to read TrialStateCtr variable\n");
+		return -1;
+	}
+
+	return 0;
+}
+
+static int fwu_trial_count_update(void)
+{
+	int ret;
+	u16 trial_state_ctr;
+
+	ret = trial_counter_read(&trial_state_ctr);
+	if (ret) {
+		log_debug("Unable to read trial_state_ctr\n");
+		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");
+		ret = fwu_revert_boot_index();
+		if (ret)
+			log_err("Unable to revert active_index\n");
+		ret = 1;
+	} else {
+		ret = trial_counter_update(&trial_state_ctr);
+		if (ret)
+			log_err("Unable to increment TrialStateCtr variable\n");
+	}
+
+out:
+	return ret;
+}
+
+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) {
+			log_info("System booting in Trial State\n");
+			return 1;
+		}
+	}
+
+	return 0;
+}
+
 static int fwu_get_image_type_id(u8 *image_index, efi_guid_t *image_type_id)
 {
 	u8 index;
@@ -499,3 +598,94 @@  __weak int fwu_plat_get_update_index(uint *update_idx)
 
 	return ret;
 }
+
+/**
+ * fwu_update_checks_pass() - Check if FWU update can be done
+ *
+ * Check if the FWU update can be executed. The updates are
+ * allowed only when the platform is not in Trial State and
+ * the boot time checks have passed
+ *
+ * Return: 1 if OK, 0 if checks do not pass
+ *
+ */
+u8 fwu_update_checks_pass(void)
+{
+	return !in_trial && boottime_check;
+}
+
+/**
+ * fwu_empty_capsule_checks_pass() - Check if empty capsule can be processed
+ *
+ * Check if the empty capsule can be processed to either accept or revert
+ * an earlier executed update. The empty capsules need to be processed
+ * only when the platform is in Trial State and the boot time checks have
+ * passed
+ *
+ * Return: 1 if OK, 0 if not to be allowed
+ *
+ */
+u8 fwu_empty_capsule_checks_pass(void)
+{
+	return in_trial && boottime_check;
+}
+
+static int fwu_boottime_checks(void *ctx, struct event *event)
+{
+	int ret;
+	u32 boot_idx, active_idx;
+	struct udevice *dev;
+	struct fwu_mdata mdata = { 0 };
+
+	ret = fwu_check_mdata_validity();
+	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_set_active_index(boot_idx);
+		if (!ret)
+			boottime_check = 1;
+
+		return 0;
+	}
+
+	if (efi_init_obj_list() != EFI_SUCCESS)
+		return 0;
+
+	ret = fwu_get_dev_mdata(&dev, &mdata);
+	if (ret)
+		return ret;
+
+	in_trial = in_trial_state(&mdata);
+	if (!in_trial || (ret = fwu_trial_count_update()) > 0)
+		ret = trial_counter_update(NULL);
+
+	if (!ret)
+		boottime_check = 1;
+
+	return 0;
+}
+EVENT_SPY(EVT_MAIN_LOOP, fwu_boottime_checks);