* [edk2-platforms][PATCH 1/1] RPi4: add descriptive errors in PlatformRegisterOptionsAndKeys
@ 2020-06-01 7:32 Andrei Warkentin
2020-06-01 9:05 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: Andrei Warkentin @ 2020-06-01 7:32 UTC (permalink / raw)
To: devel; +Cc: ard.biesheuvel, leif, pete, philmd
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.warkentin@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 <pete@akeo.ie>
- * Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@gmail.com>
+ * Copyright (c) 2017-2020, Andrei Warkentin <andrey.warkentin@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);
}
//
@@ -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
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [edk2-platforms][PATCH 1/1] RPi4: add descriptive errors in PlatformRegisterOptionsAndKeys
2020-06-01 7:32 [edk2-platforms][PATCH 1/1] RPi4: add descriptive errors in PlatformRegisterOptionsAndKeys Andrei Warkentin
@ 2020-06-01 9:05 ` Ard Biesheuvel
2020-06-01 9:21 ` [edk2-devel] " Andrei Warkentin
0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2020-06-01 9:05 UTC (permalink / raw)
To: Andrei Warkentin, devel; +Cc: leif, pete, philmd
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.warkentin@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 <pete@akeo.ie>
> - * Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> + * Copyright (c) 2017-2020, Andrei Warkentin <andrey.warkentin@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
>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi4: add descriptive errors in PlatformRegisterOptionsAndKeys
2020-06-01 9:05 ` Ard Biesheuvel
@ 2020-06-01 9:21 ` Andrei Warkentin
0 siblings, 0 replies; 3+ messages in thread
From: Andrei Warkentin @ 2020-06-01 9:21 UTC (permalink / raw)
To: Andrei Warkentin, devel@edk2.groups.io, ard.biesheuvel@arm.com
Cc: leif@nuviainc.com, pete@akeo.ie, philmd@redhat.com
[-- Attachment #1: Type: text/plain, Size: 3783 bytes --]
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.com@groups.io>
Sent: Monday, June 1, 2020 4:05 AM
To: Andrei Warkentin <andrey.warkentin@gmail.com>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: leif@nuviainc.com <leif@nuviainc.com>; pete@akeo.ie <pete@akeo.ie>; philmd@redhat.com <philmd@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.warkentin@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 <pete@akeo.ie>
> - * Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@gmail.com>
> + * Copyright (c) 2017-2020, Andrei Warkentin <andrey.warkentin@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
>
[-- Attachment #2: Type: text/html, Size: 6341 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-06-01 9:21 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-06-01 7:32 [edk2-platforms][PATCH 1/1] RPi4: add descriptive errors in PlatformRegisterOptionsAndKeys Andrei Warkentin
2020-06-01 9:05 ` Ard Biesheuvel
2020-06-01 9:21 ` [edk2-devel] " Andrei Warkentin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox