Date
1 - 3 of 3
[edk2-platforms][PATCH 1/1] RPi4: add descriptive errors in PlatformRegisterOptionsAndKeys
Andrei Warkentin
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@...>
---
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@...>
- * Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@...>
+ * Copyright (c) 2017-2020, Andrei Warkentin <andrey.warkentin@...>
* 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
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@...>
---
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@...>
- * Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@...>
+ * Copyright (c) 2017-2020, Andrei Warkentin <andrey.warkentin@...>
* 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
Ard Biesheuvel
On 6/1/20 9:32 AM, Andrei Warkentin wrote:
if (EFI_ERROR (Status) && Status != EFI_ALREADY_STARTED) {
DEBUG ((DEBUG_ERROR, "Failed to register ...
ASSERT_EFI_ERROR (Status);
}
(same below)
I have reports of debug builds sitting with an ASSERT insideMind if we invert this 'success handling' pattern?
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@...>
---
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@...>
- * Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@...>
+ * Copyright (c) 2017-2020, Andrei Warkentin <andrey.warkentin@...>
* 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);
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
Andrei Warkentin
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@...>
Sent: Monday, June 1, 2020 4:05 AM
To: Andrei Warkentin <andrey.warkentin@...>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: leif@... <leif@...>; pete@... <pete@...>; philmd@... <philmd@...>
Subject: Re: [edk2-devel] [edk2-platforms][PATCH 1/1] RPi4: add descriptive errors in PlatformRegisterOptionsAndKeys
Sent: Monday, June 1, 2020 4:05 AM
To: Andrei Warkentin <andrey.warkentin@...>; devel@edk2.groups.io <devel@edk2.groups.io>
Cc: leif@... <leif@...>; pete@... <pete@...>; philmd@... <philmd@...>
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@...>
> ---
> 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@...>
> - * Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@...>
> + * Copyright (c) 2017-2020, Andrei Warkentin <andrey.warkentin@...>
> * 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
>
> 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@...>
> ---
> 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@...>
> - * Copyright (c) 2017-2018, Andrei Warkentin <andrey.warkentin@...>
> + * Copyright (c) 2017-2020, Andrei Warkentin <andrey.warkentin@...>
> * 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
>