Hi Jan,

>> In an update scenario, if ustate was just switched from
>> INSTALLED=1 to TESTING=2 but the kernel image to be tested
>> is not present, reboot to trigger a fallback into the
>> original boot path.
> 
> And why do we want or even need this?
> 
> Looks to me like an optimization that will accelerate the reset which
> would otherwise have to wait for the watchdog to fire.

In a perfect world with perfect firmware shipped, this will just spare you a 
few seconds waiting for the watchdog to kick in, so a small optimization, 
you're totally right. However, the assumption of perfectness unfortunately 
doesn't hold :)


> But your vague description may suggest you are actually fixing a critical bug.

It's a bug, namely on aarch64 + U-Boot with flawed code+configuration where the 
watchdog is indefinitely fed by U-Boot, so never kicking in, and EFI Boot 
Guard's error_exit() calling BS→Exit() after having waited a bit results in 
being dropped to back to U-Boot.
This patch is a fix to not get dropped back to U-Boot in the particular case 
described in the commit message.
While it's not strictly needed, see your argument above, it does help in 
exactly this situation and spares you a few seconds on top.


>> 
>> Signed-off-by: Christian Storm <[email protected]>
>> ---
>> env/fatvars.c       |  1 +
>> include/bootguard.h |  1 +
>> main.c              | 14 ++++++++++++++
>> 3 files changed, 16 insertions(+)
>> 
>> diff --git a/env/fatvars.c b/env/fatvars.c
>> index 675f6cc..07c9e85 100644
>> --- a/env/fatvars.c
>> +++ b/env/fatvars.c
>> @@ -198,6 +198,7 @@ BG_STATUS load_config(BG_LOADER_PARAMS *bglp)
>> save_current_config(config_volumes, numHandles);
>> }
>> 
>> + bglp->ustate = env[latest_idx].ustate;
>> bglp->payload_path = StrDuplicate(env[current_partition].kernelfile);
>> bglp->payload_options =
>>    StrDuplicate(env[current_partition].kernelparams);
>> diff --git a/include/bootguard.h b/include/bootguard.h
>> index 93d415c..af0f54a 100644
>> --- a/include/bootguard.h
>> +++ b/include/bootguard.h
>> @@ -37,4 +37,5 @@ typedef struct _BG_LOADER_PARAMS {
>> CHAR16 *payload_path;
>> CHAR16 *payload_options;
>> UINTN timeout;
>> + UINT8 ustate;
>> } BG_LOADER_PARAMS;
>> diff --git a/main.c b/main.c
>> index 3885754..8046ac1 100644
>> --- a/main.c
>> +++ b/main.c
>> @@ -184,6 +184,20 @@ EFI_STATUS efi_main(EFI_HANDLE image_handle, 
>> EFI_SYSTEM_TABLE *system_table)
>> status = BS->LoadImage(TRUE, this_image, payload_dev_path, NULL, 0,
>>       &payload_handle);
>> if (EFI_ERROR(status)) {
>> + if (bg_loader_params.ustate == USTATE_TESTING) {
>> + /*
>> + * `ustate` was just switched from `1` (INSTALLED)
>> + * to `2` (TESTING), but the kernel image to be
>> + * tested is not present. Reboot to trigger a
>> + * fallback into the original boot path.
>> + */
>> + ERROR(L"Failed to load kernel image %s (%r).\n",
>> +      bg_loader_params.payload_path, status);
>> + ERROR(L"Triggering Rollback as ustate==2 (TESTING).\n");
>> + (VOID) BS->Stall(3 * 1000 * 1000);
>> + ST->RuntimeServices->ResetSystem(EfiResetCold,
>> + EFI_SUCCESS, 0, NULL);
>> + }
>> error_exit(L"Cannot load specified kernel image", status);
>> }
>> 
> 
> -- 
> Siemens AG, Foundational Technologies
> Linux Expert Center


Best regards,
  Christian

-- 
Dr. Christian Storm
Siemens AG, FT RPD CED
Friedrich-Ludwig-Bauer-Str. 3, 85748 Garching, Germany

-- 
You received this message because you are subscribed to the Google Groups "EFI 
Boot Guard" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion visit 
https://groups.google.com/d/msgid/efibootguard-dev/5574F1D3-C351-4282-B8E1-A79EEDC09EF8%40siemens.com.

Reply via email to