Re: [PATCH v5 08/23] FWU: Add boot time checks as highlighted by the FWU specification
hi Etienne,
On Thu, 23 Jun 2022 at 18:02, Etienne Carriere
wrote:
>
> Hi Sughosh,
>
> On Thu, 23 Jun 2022 at 11:46, Sughosh Ganu wrote:
> >
> > hi Etienne,
> >
> > On Tue, 21 Jun 2022 at 16:26, Etienne Carriere
> > wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Thu, 9 Jun 2022 at 14:31, Sughosh Ganu 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
> > > > ---
> > > > common/board_r.c | 5 ++
> > > > include/fwu.h | 3 +
> > > > lib/fwu_updates/fwu.c | 170 ++
> > > > 3 files changed, 178 insertions(+)
> > > > create mode 100644 lib/fwu_updates/fwu.c
> >
> >
> >
> > > > --- /dev/null
> > > > +++ b/lib/fwu_updates/fwu.c
> > > > @@ -0,0 +1,170 @@
> > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > +/*
> > > > + * Copyright (c) 2022, Linaro Limited
> > > > + */
> > > > +
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +#include
> > > > +
> > > > +#include
> > > > +#include
> > > > +
> > > > +static u8 trial_state;
> > > > +static u8 boottime_check;
> > > > +
> > > > +static int fwu_trial_state_check(void)
> > > > +{
> > > > + int ret, i;
> > > > + efi_status_t status;
> > > > + efi_uintn_t var_size;
> > > > + u16 trial_state_ctr;
> > > > + u32 nimages, active_bank, var_attributes, active_idx;
> > > > + struct fwu_mdata *mdata = NULL;
> > > > + struct fwu_image_entry *img_entry;
> > > > + struct fwu_image_bank_info *img_bank_info;
> > > > +
> > > > + ret = fwu_get_mdata();
> > > > + if (ret)
> > > > + return ret;
> > > > +
> > > > + ret = 0;
> > > > + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK;
> > > > + active_bank = mdata->active_index;
> > > > + img_entry = >img_entry[0];
> > > > + for (i = 0; i < nimages; i++) {
> > > > + img_bank_info =
> > > > _entry[i].img_bank_info[active_bank];
> > > > + if (!img_bank_info->accepted) {
> > > > + trial_state = 1;
> > > > + break;
> > > > + }
> > > > + }
> > > > +
> > > > + if (trial_state) {
> > > > + 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(L"TrialStateCtr",
> > > > + _global_variable_guid,
> > > > + _attributes,
> > > > + _size,
> > > > _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;
> > > > + }
> > > > +
> > > > + trial_state_ctr = 0;
> > > > + status = efi_set_variable_int(L"TrialStateCtr",
> > > > +
> > > > _global_variable_guid,
> > > > + var_attributes,
> > > > +
Re: [PATCH v5 08/23] FWU: Add boot time checks as highlighted by the FWU specification
Hi Sughosh,
On Thu, 23 Jun 2022 at 11:46, Sughosh Ganu wrote:
>
> hi Etienne,
>
> On Tue, 21 Jun 2022 at 16:26, Etienne Carriere
> wrote:
> >
> > Hi Sughosh,
> >
> > On Thu, 9 Jun 2022 at 14:31, Sughosh Ganu 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
> > > ---
> > > common/board_r.c | 5 ++
> > > include/fwu.h | 3 +
> > > lib/fwu_updates/fwu.c | 170 ++
> > > 3 files changed, 178 insertions(+)
> > > create mode 100644 lib/fwu_updates/fwu.c
>
>
>
> > > --- /dev/null
> > > +++ b/lib/fwu_updates/fwu.c
> > > @@ -0,0 +1,170 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (c) 2022, Linaro Limited
> > > + */
> > > +
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +#include
> > > +
> > > +#include
> > > +#include
> > > +
> > > +static u8 trial_state;
> > > +static u8 boottime_check;
> > > +
> > > +static int fwu_trial_state_check(void)
> > > +{
> > > + int ret, i;
> > > + efi_status_t status;
> > > + efi_uintn_t var_size;
> > > + u16 trial_state_ctr;
> > > + u32 nimages, active_bank, var_attributes, active_idx;
> > > + struct fwu_mdata *mdata = NULL;
> > > + struct fwu_image_entry *img_entry;
> > > + struct fwu_image_bank_info *img_bank_info;
> > > +
> > > + ret = fwu_get_mdata();
> > > + if (ret)
> > > + return ret;
> > > +
> > > + ret = 0;
> > > + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK;
> > > + active_bank = mdata->active_index;
> > > + img_entry = >img_entry[0];
> > > + for (i = 0; i < nimages; i++) {
> > > + img_bank_info = _entry[i].img_bank_info[active_bank];
> > > + if (!img_bank_info->accepted) {
> > > + trial_state = 1;
> > > + break;
> > > + }
> > > + }
> > > +
> > > + if (trial_state) {
> > > + 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(L"TrialStateCtr",
> > > + _global_variable_guid,
> > > + _attributes,
> > > + _size, _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;
> > > + }
> > > +
> > > + trial_state_ctr = 0;
> > > + status = efi_set_variable_int(L"TrialStateCtr",
> > > +
> > > _global_variable_guid,
> > > + var_attributes,
> > > + 0,
> >
> > s/0/var_size/ ?
> > Ditto 24 lines below.
>
> The variable size is 0 since the variable is being deleted here and
> the other instance that you mention. Maybe I can put a comment in the
> two places.
The goal is to delete the variable or to reset it to 0?
If really deleting, this function
Re: [PATCH v5 08/23] FWU: Add boot time checks as highlighted by the FWU specification
hi Patrick,
On Tue, 21 Jun 2022 at 17:16, Patrick DELAUNAY
wrote:
>
> Hi,
>
>
> On 6/9/22 14:29, Sughosh Ganu 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
> > ---
> > common/board_r.c | 5 ++
> > include/fwu.h | 3 +
> > lib/fwu_updates/fwu.c | 170 ++
> > 3 files changed, 178 insertions(+)
> > create mode 100644 lib/fwu_updates/fwu.c
> >
> > diff --git a/common/board_r.c b/common/board_r.c
> > index 6f4aca2077..33a600715d 100644
> > --- a/common/board_r.c
> > +++ b/common/board_r.c
> > @@ -15,6 +15,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -797,6 +798,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 41774ff9e2..8fbd91b463 100644
> > --- a/include/fwu.h
> > +++ b/include/fwu.h
> > @@ -33,6 +33,9 @@ struct fwu_mdata_ops {
> > EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> >0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> >
> > +int fwu_boottime_checks(void);
> > +u8 fwu_update_checks_pass(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
> > new file mode 100644
> > index 00..af884439fb
> > --- /dev/null
> > +++ b/lib/fwu_updates/fwu.c
> > @@ -0,0 +1,170 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2022, Linaro Limited
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +#include
> > +
> > +static u8 trial_state;
> > +static u8 boottime_check;
> > +
> > +static int fwu_trial_state_check(void)
> > +{
> > + int ret, i;
> > + efi_status_t status;
> > + efi_uintn_t var_size;
> > + u16 trial_state_ctr;
> > + u32 nimages, active_bank, var_attributes, active_idx;
> > + struct fwu_mdata *mdata = NULL;
> > + struct fwu_image_entry *img_entry;
> > + struct fwu_image_bank_info *img_bank_info;
> > +
> > + ret = fwu_get_mdata();
> > + if (ret)
> > + return ret;
> > +
> > + ret = 0;
> > + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK;
> > + active_bank = mdata->active_index;
> > + img_entry = >img_entry[0];
> > + for (i = 0; i < nimages; i++) {
> > + img_bank_info = _entry[i].img_bank_info[active_bank];
> > + if (!img_bank_info->accepted) {
> > + trial_state = 1;
> > + break;
> > + }
> > + }
> > +
> > + if (trial_state) {
> > + 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(L"TrialStateCtr",
> > + _global_variable_guid,
> > + _attributes,
> > + _size, _state_ctr,
> > + NULL);
>
> for 'L"TrialStateCtr"' => wide characters for unicode
>
> L string is really needed here ?
>
> cf=
> http://patchwork.ozlabs.org/project/uboot/patch/20220123195514.3152022-5-...@chromium.org/
>
> same for all the other string L"TrialStateCtr" in the file...
Will change.
>
>
> > + 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) {
> > +
Re: [PATCH v5 08/23] FWU: Add boot time checks as highlighted by the FWU specification
hi Etienne,
On Tue, 21 Jun 2022 at 16:26, Etienne Carriere
wrote:
>
> Hi Sughosh,
>
> On Thu, 9 Jun 2022 at 14:31, Sughosh Ganu 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
> > ---
> > common/board_r.c | 5 ++
> > include/fwu.h | 3 +
> > lib/fwu_updates/fwu.c | 170 ++
> > 3 files changed, 178 insertions(+)
> > create mode 100644 lib/fwu_updates/fwu.c
> > --- /dev/null
> > +++ b/lib/fwu_updates/fwu.c
> > @@ -0,0 +1,170 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2022, Linaro Limited
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +#include
> > +
> > +static u8 trial_state;
> > +static u8 boottime_check;
> > +
> > +static int fwu_trial_state_check(void)
> > +{
> > + int ret, i;
> > + efi_status_t status;
> > + efi_uintn_t var_size;
> > + u16 trial_state_ctr;
> > + u32 nimages, active_bank, var_attributes, active_idx;
> > + struct fwu_mdata *mdata = NULL;
> > + struct fwu_image_entry *img_entry;
> > + struct fwu_image_bank_info *img_bank_info;
> > +
> > + ret = fwu_get_mdata();
> > + if (ret)
> > + return ret;
> > +
> > + ret = 0;
> > + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK;
> > + active_bank = mdata->active_index;
> > + img_entry = >img_entry[0];
> > + for (i = 0; i < nimages; i++) {
> > + img_bank_info = _entry[i].img_bank_info[active_bank];
> > + if (!img_bank_info->accepted) {
> > + trial_state = 1;
> > + break;
> > + }
> > + }
> > +
> > + if (trial_state) {
> > + 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(L"TrialStateCtr",
> > + _global_variable_guid,
> > + _attributes,
> > + _size, _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;
> > + }
> > +
> > + trial_state_ctr = 0;
> > + status = efi_set_variable_int(L"TrialStateCtr",
> > +
> > _global_variable_guid,
> > + var_attributes,
> > + 0,
>
> s/0/var_size/ ?
> Ditto 24 lines below.
The variable size is 0 since the variable is being deleted here and
the other instance that you mention. Maybe I can put a comment in the
two places.
>
> > + _state_ctr,
> > false);
> > + if (status != EFI_SUCCESS) {
> > + log_err("Unable to clear TrialStateCtr
> > variable\n");
> > + ret = -1;
> > + goto out;
> > + }
> > + } else {
> > +
Re: [PATCH v5 08/23] FWU: Add boot time checks as highlighted by the FWU specification
Hi,
On 6/9/22 14:29, Sughosh Ganu 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
---
common/board_r.c | 5 ++
include/fwu.h | 3 +
lib/fwu_updates/fwu.c | 170 ++
3 files changed, 178 insertions(+)
create mode 100644 lib/fwu_updates/fwu.c
diff --git a/common/board_r.c b/common/board_r.c
index 6f4aca2077..33a600715d 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -797,6 +798,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 41774ff9e2..8fbd91b463 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -33,6 +33,9 @@ struct fwu_mdata_ops {
EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
+int fwu_boottime_checks(void);
+u8 fwu_update_checks_pass(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
new file mode 100644
index 00..af884439fb
--- /dev/null
+++ b/lib/fwu_updates/fwu.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2022, Linaro Limited
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+static u8 trial_state;
+static u8 boottime_check;
+
+static int fwu_trial_state_check(void)
+{
+ int ret, i;
+ efi_status_t status;
+ efi_uintn_t var_size;
+ u16 trial_state_ctr;
+ u32 nimages, active_bank, var_attributes, active_idx;
+ struct fwu_mdata *mdata = NULL;
+ struct fwu_image_entry *img_entry;
+ struct fwu_image_bank_info *img_bank_info;
+
+ ret = fwu_get_mdata();
+ if (ret)
+ return ret;
+
+ ret = 0;
+ nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK;
+ active_bank = mdata->active_index;
+ img_entry = >img_entry[0];
+ for (i = 0; i < nimages; i++) {
+ img_bank_info = _entry[i].img_bank_info[active_bank];
+ if (!img_bank_info->accepted) {
+ trial_state = 1;
+ break;
+ }
+ }
+
+ if (trial_state) {
+ 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(L"TrialStateCtr",
+ _global_variable_guid,
+ _attributes,
+ _size, _state_ctr,
+ NULL);
for 'L"TrialStateCtr"' => wide characters for unicode
L string is really needed here ?
cf=
http://patchwork.ozlabs.org/project/uboot/patch/20220123195514.3152022-5-...@chromium.org/
same for all the other string L"TrialStateCtr" in the file...
+ 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;
+ }
+
+ trial_state_ctr = 0;
+ status = efi_set_variable_int(L"TrialStateCtr",
+
Re: [PATCH v5 08/23] FWU: Add boot time checks as highlighted by the FWU specification
Hi Sughosh,
On Thu, 9 Jun 2022 at 14:31, Sughosh Ganu 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
> ---
> common/board_r.c | 5 ++
> include/fwu.h | 3 +
> lib/fwu_updates/fwu.c | 170 ++
> 3 files changed, 178 insertions(+)
> create mode 100644 lib/fwu_updates/fwu.c
>
> diff --git a/common/board_r.c b/common/board_r.c
> index 6f4aca2077..33a600715d 100644
> --- a/common/board_r.c
> +++ b/common/board_r.c
> @@ -15,6 +15,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -797,6 +798,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 41774ff9e2..8fbd91b463 100644
> --- a/include/fwu.h
> +++ b/include/fwu.h
> @@ -33,6 +33,9 @@ struct fwu_mdata_ops {
> EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> 0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
>
> +int fwu_boottime_checks(void);
> +u8 fwu_update_checks_pass(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
> new file mode 100644
> index 00..af884439fb
> --- /dev/null
> +++ b/lib/fwu_updates/fwu.c
> @@ -0,0 +1,170 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (c) 2022, Linaro Limited
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +#include
> +
> +static u8 trial_state;
> +static u8 boottime_check;
> +
> +static int fwu_trial_state_check(void)
> +{
> + int ret, i;
> + efi_status_t status;
> + efi_uintn_t var_size;
> + u16 trial_state_ctr;
> + u32 nimages, active_bank, var_attributes, active_idx;
> + struct fwu_mdata *mdata = NULL;
> + struct fwu_image_entry *img_entry;
> + struct fwu_image_bank_info *img_bank_info;
> +
> + ret = fwu_get_mdata();
> + if (ret)
> + return ret;
> +
> + ret = 0;
> + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK;
> + active_bank = mdata->active_index;
> + img_entry = >img_entry[0];
> + for (i = 0; i < nimages; i++) {
> + img_bank_info = _entry[i].img_bank_info[active_bank];
> + if (!img_bank_info->accepted) {
> + trial_state = 1;
> + break;
> + }
> + }
> +
> + if (trial_state) {
> + 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(L"TrialStateCtr",
> + _global_variable_guid,
> + _attributes,
> + _size, _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;
> + }
> +
> + trial_state_ctr = 0;
> + status = efi_set_variable_int(L"TrialStateCtr",
> +
Re: [PATCH v5 08/23] FWU: Add boot time checks as highlighted by the FWU specification
On Wed, Jun 15, 2022 at 08:34:18AM +0200, Heinrich Schuchardt wrote:
> On 6/9/22 14:29, Sughosh Ganu 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
> > ---
> > common/board_r.c | 5 ++
> > include/fwu.h | 3 +
> > lib/fwu_updates/fwu.c | 170 ++
> > 3 files changed, 178 insertions(+)
> > create mode 100644 lib/fwu_updates/fwu.c
> >
> > diff --git a/common/board_r.c b/common/board_r.c
> > index 6f4aca2077..33a600715d 100644
> > --- a/common/board_r.c
> > +++ b/common/board_r.c
> > @@ -15,6 +15,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -797,6 +798,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 41774ff9e2..8fbd91b463 100644
> > --- a/include/fwu.h
> > +++ b/include/fwu.h
> > @@ -33,6 +33,9 @@ struct fwu_mdata_ops {
> > EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
> > 0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
> >
> > +int fwu_boottime_checks(void);
> > +u8 fwu_update_checks_pass(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
> > new file mode 100644
> > index 00..af884439fb
> > --- /dev/null
> > +++ b/lib/fwu_updates/fwu.c
> > @@ -0,0 +1,170 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (c) 2022, Linaro Limited
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +#include
> > +
> > +static u8 trial_state;
> > +static u8 boottime_check;
> > +
> > +static int fwu_trial_state_check(void)
> > +{
> > + int ret, i;
> > + efi_status_t status;
> > + efi_uintn_t var_size;
> > + u16 trial_state_ctr;
> > + u32 nimages, active_bank, var_attributes, active_idx;
> > + struct fwu_mdata *mdata = NULL;
> > + struct fwu_image_entry *img_entry;
> > + struct fwu_image_bank_info *img_bank_info;
> > +
> > + ret = fwu_get_mdata();
> > + if (ret)
> > + return ret;
> > +
> > + ret = 0;
> > + nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK;
> > + active_bank = mdata->active_index;
> > + img_entry = >img_entry[0];
> > + for (i = 0; i < nimages; i++) {
> > + img_bank_info = _entry[i].img_bank_info[active_bank];
> > + if (!img_bank_info->accepted) {
> > + trial_state = 1;
> > + break;
> > + }
> > + }
> > +
> > + if (trial_state) {
> > + 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(L"TrialStateCtr",
> > + _global_variable_guid,
> > + _attributes,
> > + _size, _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;
> > +
Re: [PATCH v5 08/23] FWU: Add boot time checks as highlighted by the FWU specification
On 6/9/22 14:29, Sughosh Ganu 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
---
common/board_r.c | 5 ++
include/fwu.h | 3 +
lib/fwu_updates/fwu.c | 170 ++
3 files changed, 178 insertions(+)
create mode 100644 lib/fwu_updates/fwu.c
diff --git a/common/board_r.c b/common/board_r.c
index 6f4aca2077..33a600715d 100644
--- a/common/board_r.c
+++ b/common/board_r.c
@@ -15,6 +15,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -797,6 +798,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 41774ff9e2..8fbd91b463 100644
--- a/include/fwu.h
+++ b/include/fwu.h
@@ -33,6 +33,9 @@ struct fwu_mdata_ops {
EFI_GUID(0x8a7a84a0, 0x8387, 0x40f6, 0xab, 0x41, \
0xa8, 0xb9, 0xa5, 0xa6, 0x0d, 0x23)
+int fwu_boottime_checks(void);
+u8 fwu_update_checks_pass(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
new file mode 100644
index 00..af884439fb
--- /dev/null
+++ b/lib/fwu_updates/fwu.c
@@ -0,0 +1,170 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (c) 2022, Linaro Limited
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+static u8 trial_state;
+static u8 boottime_check;
+
+static int fwu_trial_state_check(void)
+{
+ int ret, i;
+ efi_status_t status;
+ efi_uintn_t var_size;
+ u16 trial_state_ctr;
+ u32 nimages, active_bank, var_attributes, active_idx;
+ struct fwu_mdata *mdata = NULL;
+ struct fwu_image_entry *img_entry;
+ struct fwu_image_bank_info *img_bank_info;
+
+ ret = fwu_get_mdata();
+ if (ret)
+ return ret;
+
+ ret = 0;
+ nimages = CONFIG_FWU_NUM_IMAGES_PER_BANK;
+ active_bank = mdata->active_index;
+ img_entry = >img_entry[0];
+ for (i = 0; i < nimages; i++) {
+ img_bank_info = _entry[i].img_bank_info[active_bank];
+ if (!img_bank_info->accepted) {
+ trial_state = 1;
+ break;
+ }
+ }
+
+ if (trial_state) {
+ 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(L"TrialStateCtr",
+ _global_variable_guid,
+ _attributes,
+ _size, _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;
+ }
+
+ trial_state_ctr = 0;
+ status = efi_set_variable_int(L"TrialStateCtr",
+ _global_variable_guid,
+ var_attributes,
+ 0,
+ _state_ctr, false);
+
