Sure. Let me know if you want me to resend.
A
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Ard Biesheuvel
via groups.io <ard.biesheuvel=arm....@groups.io>
Sent: Monday, June 1, 2020 4:05 AM
To: Andrei Warkentin <andrey.warken...@gmail.com>; devel@edk2.groups.io
<devel@edk2.groups.io>
Cc: l...@nuviainc.com <l...@nuviainc.com>; p...@akeo.ie <p...@akeo.ie>;
phi...@redhat.com <phi...@redhat.com>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi4: add descriptive
errors in PlatformRegisterOptionsAndKeys
On 6/1/20 9:32 AM, Andrei Warkentin wrote:
> I have reports of debug builds sitting with an ASSERT inside
> PlatformRegisterOptionsAndKeys, but have not been able to verify.
> Let's log errors to help diagnose this in the future.
>
> Signed-off-by: Andrei Warkentin <andrey.warken...@gmail.com>
> ---
> Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c | 25
> ++++++++++++++++----
> 1 file changed, 21 insertions(+), 4 deletions(-)
>
> diff --git a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> index 996ba8f3..f0a2fe1a 100644
> --- a/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> +++ b/Platform/RaspberryPi/Library/PlatformBootManagerLib/PlatformBm.c
> @@ -1,7 +1,7 @@
> /** @file
> *
> * Copyright (c) 2018, Pete Batard <p...@akeo.ie>
> - * Copyright (c) 2017-2018, Andrei Warkentin <andrey.warken...@gmail.com>
> + * Copyright (c) 2017-2020, Andrei Warkentin <andrey.warken...@gmail.com>
> * Copyright (c) 2016, Linaro Ltd. All rights reserved.
> * Copyright (c) 2015-2016, Red Hat, Inc.
> * Copyright (c) 2014, ARM Ltd. All rights reserved.
> @@ -468,7 +468,13 @@ PlatformRegisterOptionsAndKeys (
> F1.ScanCode = SCAN_F1;
> F1.UnicodeChar = CHAR_NULL;
> Status = EfiBootManagerAddKeyOptionVariable (NULL, (UINT16)ShellOption,
> 0, &F1, NULL);
> - ASSERT (Status == EFI_SUCCESS || Status == EFI_ALREADY_STARTED);
> + if (Status == EFI_ALREADY_STARTED) {
> + Status = EFI_SUCCESS;
> + }
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Failed to register F1 as UEFI Shell key: %r\n",
> Status));
> + }
> + ASSERT_EFI_ERROR (Status);
Mind if we invert this 'success handling' pattern?
if (EFI_ERROR (Status) && Status != EFI_ALREADY_STARTED) {
DEBUG ((DEBUG_ERROR, "Failed to register ...
ASSERT_EFI_ERROR (Status);
}
(same below)
> }
>
> //
> @@ -477,6 +483,9 @@ PlatformRegisterOptionsAndKeys (
> Enter.ScanCode = SCAN_NULL;
> Enter.UnicodeChar = CHAR_CARRIAGE_RETURN;
> Status = EfiBootManagerRegisterContinueKeyOption (0, &Enter, NULL);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Failed to register ENTER as CONTINUE key: %r\n",
> Status));
> + }
> ASSERT_EFI_ERROR (Status);
>
> //
> @@ -485,9 +494,17 @@ PlatformRegisterOptionsAndKeys (
> Esc.ScanCode = SCAN_ESC;
> Esc.UnicodeChar = CHAR_NULL;
> Status = EfiBootManagerGetBootManagerMenu (&BootOption);
> + if (!EFI_ERROR (Status)) {
> + Status = EfiBootManagerAddKeyOptionVariable (NULL,
> (UINT16)BootOption.OptionNumber, 0, &Esc, NULL);
> + if (Status == EFI_ALREADY_STARTED) {
> + Status = EFI_SUCCESS;
> + }
> + }
> +
> + if (EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_ERROR, "Failed to register ESC as Boot Manager key: %r\n",
> Status));
> + }
> ASSERT_EFI_ERROR (Status);
> - Status = EfiBootManagerAddKeyOptionVariable (NULL,
> (UINT16)BootOption.OptionNumber, 0, &Esc, NULL);
> - ASSERT (Status == EFI_SUCCESS || Status == EFI_ALREADY_STARTED);
> }
>
> STATIC VOID
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#60492): https://edk2.groups.io/g/devel/message/60492
Mute This Topic: https://groups.io/mt/74599060/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
