* OVMF is crashing for me in master
@ 2019-10-10 21:12 Andrew Fish
2019-10-11 1:19 ` [edk2-devel] " Liming Gao
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Fish @ 2019-10-10 21:12 UTC (permalink / raw)
To: devel, Pete Batard
[-- Attachment #1: Type: text/plain, Size: 7568 bytes --]
This is my flavor of OVMF: build -p OvmfPkg/OvmfPkgX64.dsc -a X64 -t XCODE5
It looks like I took an exception? Is it expected that an unhandled exception just hang in a dead loop? I would have expected some serial output about the failure?
Looks like a divide by zero exception. The exception context has PC and FP so I can manually walk the stack. Yikes I see PlatformBootManagerWaitCallback() will fault if PcdPlatformBootTimeOut is zero?
/Volumes/Case/UDK2018(master)>git grep PcdPlatformBootTimeOut -- *.dsc
ArmVirtPkg/ArmVirtQemu.dsc:194: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
ArmVirtPkg/ArmVirtQemuKernel.dsc:191: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
ArmVirtPkg/ArmVirtXen.dsc:122: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
EmulatorPkg/EmulatorPkg.dsc:236: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|10
OvmfPkg/OvmfPkgIa32.dsc:541: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
OvmfPkg/OvmfPkgIa32X64.dsc:553: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
OvmfPkg/OvmfPkgX64.dsc:552: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
OvmfPkg/OvmfXen.dsc:470: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
UefiPayloadPkg/UefiPayloadPkgIa32.dsc:344: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc:345: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
OK PcdPlatformBootTimeOut is zero on Ovmf, so how did this ever work?
Ahhh gotom....
/Volumes/Case/UDK2018(master)>git blame -L344,344 /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
2de1f611be0 (Pete Batard 2019-09-25 23:50:05 +0800 344) PlatformBootManagerWaitCallback (0);
This call causes a divide by zero if PcdPlatformBootTimeOut == 0.
This fixes my crash:
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index 70df6b841a..d6ae43e900 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -1634,6 +1634,11 @@ PlatformBootManagerWaitCallback (
UINT16 Timeout;
Timeout = PcdGet16 (PcdPlatformBootTimeOut);
+ if (Timeout == 0) {
+ Timeout = 100;
+ } else {
+ Timeout = (Timeout - TimeoutRemain) * 100 / Timeout;
+ }
Black.Raw = 0x00000000;
White.Raw = 0x00FFFFFF;
@@ -1643,7 +1648,7 @@ PlatformBootManagerWaitCallback (
Black.Pixel,
L"Start boot option",
White.Pixel,
- (Timeout - TimeoutRemain) * 100 / Timeout,
+ Timeout,
0
);
}
lldb debugger output:
(lldb) bt
* thread #1, stop reason = signal SIGTRAP
* frame #0: 0x0000000007b58a70 CpuDxe.dll:CpuDeadLoop() + 13 at /Volumes/Case/UDK2018/MdePkg/Library/BaseLib/CpuDeadLoop.c:31
frame #1: 0x0000000007b61222 CpuDxe.dll:CommonExceptionHandlerWorker() + 674 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:115
frame #2: 0x0000000007b61624 CpuDxe.dll:CommonExceptionHandler() + 36 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c:40
frame #3: 0x0000000007b5ff26 CpuDxe.dll:HasErrorCode() + 230
(lldb) fr sel 1
frame #1: 0x0000000007b61222 CpuDxe.dll:CommonExceptionHandlerWorker() + 674 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:115
91 (ExternalInterruptHandler[ExceptionType]) (ExceptionType, SystemContext);
92 } else if (ExceptionType < CPU_EXCEPTION_NUM) {
93 //
94 // Get Spinlock to display CPU information
95 //
96 while (!AcquireSpinLockOrFail (&ExceptionHandlerData->DisplayMessageSpinLock)) {
97 CpuPause ();
98 }
99 //
100 // Initialize the serial port before dumping.
101 //
102 SerialPortInitialize ();
103 //
104 // Display ExceptionType, CPU information and Image information
105 //
106 DumpImageAndCpuContent (ExceptionType, SystemContext);
107 //
108 // Release Spinlock of output message
109 //
110 ReleaseSpinLock (&ExceptionHandlerData->DisplayMessageSpinLock);
111 //
112 // Enter a dead loop if needn't to execute old IDT handler further
113 //
114 if (ReservedVectors[ExceptionType].Attribute != EFI_VECTOR_HANDOFF_HOOK_BEFORE) {
-> 115 CpuDeadLoop ();
116 }
117 }
118 }
119
(lldb) p ExceptionType
(EFI_EXCEPTION_TYPE) $0 = 0
(lldb) p SystemContext.SystemContextX64->Rip
(UINT64) $1 = 0x0000000007a9cc38
(lldb) p SystemContext.SystemContextX64->Rbp
(UINT64) $2 = 0x0000000007e8fc20
(lldb) efi_backtrace --pc 0x0000000007a9cc38 --frame 0x0000000007e8fc20 --symbols
frame 0: 0x07a9cc38 BdsDxe:PlatformBootManagerWaitCallback + 35 at /Volumes/Case/UDK2018/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c:1646:37
frame 1: 0x07a9e744 BdsDxe:BdsWait + 269 at /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:344:3
frame 2: 0x07a9dff9 BdsDxe:BdsEntry + 2620 at /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:1002:5
frame 3: 0x07eb367d DxeCore:DxeMain + 2680 at /Volumes/Case/UDK2018/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c:544:3
frame 4: 0x07e943cb DxeCore:_ModuleEntryPoint + 20 at /Volumes/Case/UDK2018/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.c:48:3
frame 5: 0x07edc947 DxeIpl.dll`AsmEnableCache
frame 6: 0x07ee1e4e DxeIpl:HandOffToDxeCore + 509 at /Volumes/Case/UDK2018/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c:113:3
frame 7: 0x07ee0604 DxeIpl:DxeLoadCore + 1354 at /Volumes/Case/UDK2018/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c:449:3
frame 8: 0x07eeff2f PeiCore.dll`PeiCore.cold.3 + 847
frame 9: 0x07ee9d04 PeiCore:PeiCore + 163 at /Volumes/Case/UDK2018/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c:502:3
frame 10: 0x0082c387 0x0082c387
frame 11: 0x00825dd7 0x00825dd7
frame 12: 0x0082ad27 0x0082ad27
frame 13: 0x0082b3a8 0x0082b3a8
frame 14: 0x0082bf23 0x0082bf23
frame 15: 0x00825e24 0x00825e24
frame 16: 0x00823af2 0x00823af2
frame 17: 0xfffd1db8 SecMain:SecStartupPhase2 + 67 at /Volumes/Case/UDK2018/OvmfPkg/Sec/SecMain.c:858:3
frame 18: 0xfffd1d67 SecMain:SecCoreStartupWithStack + 420 at /Volumes/Case/UDK2018/OvmfPkg/Sec/SecMain.c:821:3
frame 19: 0xfffd1e14 SecMain:ProcessLibraryConstructorList + 0 at /Volumes/Case/UDK2018/Build/OvmfX64/DEBUG_XCODE5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.c:201
(lldb) l /Volumes/Case/UDK2018/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c:1646
1646 (Timeout - TimeoutRemain) * 100 / Timeout,
1647 0
1648 );
1649 }
1650
1651 /**
1652 The function is called when no boot option could be launched,
1653 including platform recovery options and options pointing to applications
1654 built into firmware volumes.
1655
(lldb) l /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:344
344 PlatformBootManagerWaitCallback (0);
345 DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
346 }
347
348 /**
349 Attempt to boot each boot option in the BootOptions array.
350
351 @param BootOptions Input boot option array.
352 @param BootOptionCount Input boot option count.
353 @param BootManagerMenu Input boot manager menu.
354
Thanks,
Andrew Fish
[-- Attachment #2: Type: text/html, Size: 49849 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [edk2-devel] OVMF is crashing for me in master
2019-10-10 21:12 OVMF is crashing for me in master Andrew Fish
@ 2019-10-11 1:19 ` Liming Gao
2019-10-11 4:59 ` Andrew Fish
2019-10-11 11:50 ` Laszlo Ersek
0 siblings, 2 replies; 10+ messages in thread
From: Liming Gao @ 2019-10-11 1:19 UTC (permalink / raw)
To: devel@edk2.groups.io, afish@apple.com, Pete Batard
[-- Attachment #1: Type: text/plain, Size: 8587 bytes --]
Andrew:
I verify the change (2de1f611be06ded3a59726a4052a9039be7d459b MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0) in Emulator.
It works, because PCD value is set to 10 in Emulator.
Before this change, if TimeOut PCD is zero, BdsEntry doesn't call PlatformBootManagerWaitCallback().
After this change, if TimeOut PCD is zero, BdsEntry still call PlatformBootManagerWaitCallback(). So, it trigs this issue. I agree your fix.
Pete:
Will you contribute the patch to fix this hang issue in OVMF?
Thanks
Liming
From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Andrew Fish via Groups.Io
Sent: Friday, October 11, 2019 5:12 AM
To: devel@edk2.groups.io; Pete Batard <pete@akeo.ie>
Subject: [edk2-devel] OVMF is crashing for me in master
This is my flavor of OVMF: build -p OvmfPkg/OvmfPkgX64.dsc -a X64 -t XCODE5
It looks like I took an exception? Is it expected that an unhandled exception just hang in a dead loop? I would have expected some serial output about the failure?
Looks like a divide by zero exception. The exception context has PC and FP so I can manually walk the stack. Yikes I see PlatformBootManagerWaitCallback() will fault if PcdPlatformBootTimeOut is zero?
/Volumes/Case/UDK2018(master)>git grep PcdPlatformBootTimeOut -- *.dsc
ArmVirtPkg/ArmVirtQemu.dsc:194: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
ArmVirtPkg/ArmVirtQemuKernel.dsc:191: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
ArmVirtPkg/ArmVirtXen.dsc:122: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
EmulatorPkg/EmulatorPkg.dsc:236: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|10
OvmfPkg/OvmfPkgIa32.dsc:541: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
OvmfPkg/OvmfPkgIa32X64.dsc:553: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
OvmfPkg/OvmfPkgX64.dsc:552: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
OvmfPkg/OvmfXen.dsc:470: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
UefiPayloadPkg/UefiPayloadPkgIa32.dsc:344: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc:345: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
OK PcdPlatformBootTimeOut is zero on Ovmf, so how did this ever work?
Ahhh gotom....
/Volumes/Case/UDK2018(master)>git blame -L344,344 /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
2de1f611be0 (Pete Batard 2019-09-25 23:50:05 +0800 344) PlatformBootManagerWaitCallback (0);
This call causes a divide by zero if PcdPlatformBootTimeOut == 0.
This fixes my crash:
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index 70df6b841a..d6ae43e900 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -1634,6 +1634,11 @@ PlatformBootManagerWaitCallback (
UINT16 Timeout;
Timeout = PcdGet16 (PcdPlatformBootTimeOut);
+ if (Timeout == 0) {
+ Timeout = 100;
+ } else {
+ Timeout = (Timeout - TimeoutRemain) * 100 / Timeout;
+ }
Black.Raw = 0x00000000;
White.Raw = 0x00FFFFFF;
@@ -1643,7 +1648,7 @@ PlatformBootManagerWaitCallback (
Black.Pixel,
L"Start boot option",
White.Pixel,
- (Timeout - TimeoutRemain) * 100 / Timeout,
+ Timeout,
0
);
}
lldb debugger output:
(lldb) bt
* thread #1, stop reason = signal SIGTRAP
* frame #0: 0x0000000007b58a70 CpuDxe.dll:CpuDeadLoop() + 13 at /Volumes/Case/UDK2018/MdePkg/Library/BaseLib/CpuDeadLoop.c:31
frame #1: 0x0000000007b61222 CpuDxe.dll:CommonExceptionHandlerWorker() + 674 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:115
frame #2: 0x0000000007b61624 CpuDxe.dll:CommonExceptionHandler() + 36 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c:40
frame #3: 0x0000000007b5ff26 CpuDxe.dll:HasErrorCode() + 230
(lldb) fr sel 1
frame #1: 0x0000000007b61222 CpuDxe.dll:CommonExceptionHandlerWorker() + 674 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:115
91 (ExternalInterruptHandler[ExceptionType]) (ExceptionType, SystemContext);
92 } else if (ExceptionType < CPU_EXCEPTION_NUM) {
93 //
94 // Get Spinlock to display CPU information
95 //
96 while (!AcquireSpinLockOrFail (&ExceptionHandlerData->DisplayMessageSpinLock)) {
97 CpuPause ();
98 }
99 //
100 // Initialize the serial port before dumping.
101 //
102 SerialPortInitialize ();
103 //
104 // Display ExceptionType, CPU information and Image information
105 //
106 DumpImageAndCpuContent (ExceptionType, SystemContext);
107 //
108 // Release Spinlock of output message
109 //
110 ReleaseSpinLock (&ExceptionHandlerData->DisplayMessageSpinLock);
111 //
112 // Enter a dead loop if needn't to execute old IDT handler further
113 //
114 if (ReservedVectors[ExceptionType].Attribute != EFI_VECTOR_HANDOFF_HOOK_BEFORE) {
-> 115 CpuDeadLoop ();
116 }
117 }
118 }
119
(lldb) p ExceptionType
(EFI_EXCEPTION_TYPE) $0 = 0
(lldb) p SystemContext.SystemContextX64->Rip
(UINT64) $1 = 0x0000000007a9cc38
(lldb) p SystemContext.SystemContextX64->Rbp
(UINT64) $2 = 0x0000000007e8fc20
(lldb) efi_backtrace --pc 0x0000000007a9cc38 --frame 0x0000000007e8fc20 --symbols
frame 0: 0x07a9cc38 BdsDxe:PlatformBootManagerWaitCallback + 35 at /Volumes/Case/UDK2018/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c:1646:37
frame 1: 0x07a9e744 BdsDxe:BdsWait + 269 at /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:344:3
frame 2: 0x07a9dff9 BdsDxe:BdsEntry + 2620 at /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:1002:5
frame 3: 0x07eb367d DxeCore:DxeMain + 2680 at /Volumes/Case/UDK2018/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c:544:3
frame 4: 0x07e943cb DxeCore:_ModuleEntryPoint + 20 at /Volumes/Case/UDK2018/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.c:48:3
frame 5: 0x07edc947 DxeIpl.dll`AsmEnableCache
frame 6: 0x07ee1e4e DxeIpl:HandOffToDxeCore + 509 at /Volumes/Case/UDK2018/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c:113:3
frame 7: 0x07ee0604 DxeIpl:DxeLoadCore + 1354 at /Volumes/Case/UDK2018/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c:449:3
frame 8: 0x07eeff2f PeiCore.dll`PeiCore.cold.3 + 847
frame 9: 0x07ee9d04 PeiCore:PeiCore + 163 at /Volumes/Case/UDK2018/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c:502:3
frame 10: 0x0082c387 0x0082c387
frame 11: 0x00825dd7 0x00825dd7
frame 12: 0x0082ad27 0x0082ad27
frame 13: 0x0082b3a8 0x0082b3a8
frame 14: 0x0082bf23 0x0082bf23
frame 15: 0x00825e24 0x00825e24
frame 16: 0x00823af2 0x00823af2
frame 17: 0xfffd1db8 SecMain:SecStartupPhase2 + 67 at /Volumes/Case/UDK2018/OvmfPkg/Sec/SecMain.c:858:3
frame 18: 0xfffd1d67 SecMain:SecCoreStartupWithStack + 420 at /Volumes/Case/UDK2018/OvmfPkg/Sec/SecMain.c:821:3
frame 19: 0xfffd1e14 SecMain:ProcessLibraryConstructorList + 0 at /Volumes/Case/UDK2018/Build/OvmfX64/DEBUG_XCODE5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.c:201
(lldb) l /Volumes/Case/UDK2018/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c:1646
1646 (Timeout - TimeoutRemain) * 100 / Timeout,
1647 0
1648 );
1649 }
1650
1651 /**
1652 The function is called when no boot option could be launched,
1653 including platform recovery options and options pointing to applications
1654 built into firmware volumes.
1655
(lldb) l /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:344
344 PlatformBootManagerWaitCallback (0);
345 DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
346 }
347
348 /**
349 Attempt to boot each boot option in the BootOptions array.
350
351 @param BootOptions Input boot option array.
352 @param BootOptionCount Input boot option count.
353 @param BootManagerMenu Input boot manager menu.
354
Thanks,
Andrew Fish
[-- Attachment #2: Type: text/html, Size: 46769 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [edk2-devel] OVMF is crashing for me in master
2019-10-11 1:19 ` [edk2-devel] " Liming Gao
@ 2019-10-11 4:59 ` Andrew Fish
2019-10-11 11:53 ` Laszlo Ersek
2019-10-11 11:50 ` Laszlo Ersek
1 sibling, 1 reply; 10+ messages in thread
From: Andrew Fish @ 2019-10-11 4:59 UTC (permalink / raw)
To: Gao, Liming; +Cc: devel@edk2.groups.io, Pete Batard
[-- Attachment #1: Type: text/plain, Size: 9202 bytes --]
Liming,
Thanks for looking into this!
Can someone also answer my question about the expected behavior of taking an exception in OVMF? Is the CpuDeadloop() expected?
Thanks,
Andrew Fish
> On Oct 10, 2019, at 6:19 PM, Gao, Liming <liming.gao@intel.com> wrote:
>
> Andrew:
> I verify the change (2de1f611be06ded3a59726a4052a9039be7d459b MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0) in Emulator.
> It works, because PCD value is set to 10 in Emulator.
>
> Before this change, if TimeOut PCD is zero, BdsEntry doesn’t call PlatformBootManagerWaitCallback().
> After this change, if TimeOut PCD is zero, BdsEntry still call PlatformBootManagerWaitCallback(). So, it trigs this issue. I agree your fix.
>
> Pete:
> Will you contribute the patch to fix this hang issue in OVMF?
>
> Thanks
> Liming
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Andrew Fish via Groups.Io
> Sent: Friday, October 11, 2019 5:12 AM
> To: devel@edk2.groups.io; Pete Batard <pete@akeo.ie>
> Subject: [edk2-devel] OVMF is crashing for me in master
>
> This is my flavor of OVMF: build -p OvmfPkg/OvmfPkgX64.dsc -a X64 -t XCODE5
>
> It looks like I took an exception? Is it expected that an unhandled exception just hang in a dead loop? I would have expected some serial output about the failure?
>
> Looks like a divide by zero exception. The exception context has PC and FP so I can manually walk the stack. Yikes I see PlatformBootManagerWaitCallback() will fault if PcdPlatformBootTimeOut is zero?
> /Volumes/Case/UDK2018(master)>git grep PcdPlatformBootTimeOut -- *.dsc
> ArmVirtPkg/ArmVirtQemu.dsc:194: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
> ArmVirtPkg/ArmVirtQemuKernel.dsc:191: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
> ArmVirtPkg/ArmVirtXen.dsc:122: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
> EmulatorPkg/EmulatorPkg.dsc:236: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|10
> OvmfPkg/OvmfPkgIa32.dsc:541: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
> OvmfPkg/OvmfPkgIa32X64.dsc:553: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
> OvmfPkg/OvmfPkgX64.dsc:552: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
> OvmfPkg/OvmfXen.dsc:470: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
> UefiPayloadPkg/UefiPayloadPkgIa32.dsc:344: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
> UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc:345: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
>
>
> OK PcdPlatformBootTimeOut is zero on Ovmf, so how did this ever work?
>
>
> Ahhh gotom....
> /Volumes/Case/UDK2018(master)>git blame -L344,344 /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> 2de1f611be0 (Pete Batard 2019-09-25 23:50:05 +0800 344) PlatformBootManagerWaitCallback (0);
>
>
> This call causes a divide by zero if PcdPlatformBootTimeOut == 0.
>
> This fixes my crash:
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 70df6b841a..d6ae43e900 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -1634,6 +1634,11 @@ PlatformBootManagerWaitCallback (
> UINT16 Timeout;
>
> Timeout = PcdGet16 (PcdPlatformBootTimeOut);
> + if (Timeout == 0) {
> + Timeout = 100;
> + } else {
> + Timeout = (Timeout - TimeoutRemain) * 100 / Timeout;
> + }
>
> Black.Raw = 0x00000000;
> White.Raw = 0x00FFFFFF;
> @@ -1643,7 +1648,7 @@ PlatformBootManagerWaitCallback (
> Black.Pixel,
> L"Start boot option",
> White.Pixel,
> - (Timeout - TimeoutRemain) * 100 / Timeout,
> + Timeout,
> 0
> );
> }
>
>
>
> lldb debugger output:
>
> (lldb) bt
> * thread #1, stop reason = signal SIGTRAP
> * frame #0: 0x0000000007b58a70 CpuDxe.dll:CpuDeadLoop() + 13 at /Volumes/Case/UDK2018/MdePkg/Library/BaseLib/CpuDeadLoop.c:31
> frame #1: 0x0000000007b61222 CpuDxe.dll:CommonExceptionHandlerWorker() + 674 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:115
> frame #2: 0x0000000007b61624 CpuDxe.dll:CommonExceptionHandler() + 36 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c:40
> frame #3: 0x0000000007b5ff26 CpuDxe.dll:HasErrorCode() + 230
> (lldb) fr sel 1
> frame #1: 0x0000000007b61222 CpuDxe.dll:CommonExceptionHandlerWorker() + 674 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:115
> 91 (ExternalInterruptHandler[ExceptionType]) (ExceptionType, SystemContext);
> 92 } else if (ExceptionType < CPU_EXCEPTION_NUM) {
> 93 //
> 94 // Get Spinlock to display CPU information
> 95 //
> 96 while (!AcquireSpinLockOrFail (&ExceptionHandlerData->DisplayMessageSpinLock)) {
> 97 CpuPause ();
> 98 }
> 99 //
> 100 // Initialize the serial port before dumping.
> 101 //
> 102 SerialPortInitialize ();
> 103 //
> 104 // Display ExceptionType, CPU information and Image information
> 105 //
> 106 DumpImageAndCpuContent (ExceptionType, SystemContext);
> 107 //
> 108 // Release Spinlock of output message
> 109 //
> 110 ReleaseSpinLock (&ExceptionHandlerData->DisplayMessageSpinLock);
> 111 //
> 112 // Enter a dead loop if needn't to execute old IDT handler further
> 113 //
> 114 if (ReservedVectors[ExceptionType].Attribute != EFI_VECTOR_HANDOFF_HOOK_BEFORE) {
> -> 115 CpuDeadLoop ();
> 116 }
> 117 }
> 118 }
> 119
> (lldb) p ExceptionType
> (EFI_EXCEPTION_TYPE) $0 = 0
> (lldb) p SystemContext.SystemContextX64->Rip
> (UINT64) $1 = 0x0000000007a9cc38
> (lldb) p SystemContext.SystemContextX64->Rbp
> (UINT64) $2 = 0x0000000007e8fc20
> (lldb) efi_backtrace --pc 0x0000000007a9cc38 --frame 0x0000000007e8fc20 --symbols
> frame 0: 0x07a9cc38 BdsDxe:PlatformBootManagerWaitCallback + 35 at /Volumes/Case/UDK2018/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c:1646:37
> frame 1: 0x07a9e744 BdsDxe:BdsWait + 269 at /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:344:3
> frame 2: 0x07a9dff9 BdsDxe:BdsEntry + 2620 at /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:1002:5
> frame 3: 0x07eb367d DxeCore:DxeMain + 2680 at /Volumes/Case/UDK2018/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c:544:3
> frame 4: 0x07e943cb DxeCore:_ModuleEntryPoint + 20 at /Volumes/Case/UDK2018/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.c:48:3
> frame 5: 0x07edc947 DxeIpl.dll`AsmEnableCache
> frame 6: 0x07ee1e4e DxeIpl:HandOffToDxeCore + 509 at /Volumes/Case/UDK2018/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c:113:3
> frame 7: 0x07ee0604 DxeIpl:DxeLoadCore + 1354 at /Volumes/Case/UDK2018/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c:449:3
> frame 8: 0x07eeff2f PeiCore.dll`PeiCore.cold.3 + 847
> frame 9: 0x07ee9d04 PeiCore:PeiCore + 163 at /Volumes/Case/UDK2018/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c:502:3
> frame 10: 0x0082c387 0x0082c387
> frame 11: 0x00825dd7 0x00825dd7
> frame 12: 0x0082ad27 0x0082ad27
> frame 13: 0x0082b3a8 0x0082b3a8
> frame 14: 0x0082bf23 0x0082bf23
> frame 15: 0x00825e24 0x00825e24
> frame 16: 0x00823af2 0x00823af2
> frame 17: 0xfffd1db8 SecMain:SecStartupPhase2 + 67 at /Volumes/Case/UDK2018/OvmfPkg/Sec/SecMain.c:858:3
> frame 18: 0xfffd1d67 SecMain:SecCoreStartupWithStack + 420 at /Volumes/Case/UDK2018/OvmfPkg/Sec/SecMain.c:821:3
> frame 19: 0xfffd1e14 SecMain:ProcessLibraryConstructorList + 0 at /Volumes/Case/UDK2018/Build/OvmfX64/DEBUG_XCODE5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.c:201
>
> (lldb) l /Volumes/Case/UDK2018/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c:1646
> 1646 (Timeout - TimeoutRemain) * 100 / Timeout,
> 1647 0
> 1648 );
> 1649 }
> 1650
> 1651 /**
> 1652 The function is called when no boot option could be launched,
> 1653 including platform recovery options and options pointing to applications
> 1654 built into firmware volumes.
> 1655
> (lldb) l /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:344
> 344 PlatformBootManagerWaitCallback (0);
> 345 DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
> 346 }
> 347
> 348 /**
> 349 Attempt to boot each boot option in the BootOptions array.
> 350
> 351 @param BootOptions Input boot option array.
> 352 @param BootOptionCount Input boot option count.
> 353 @param BootManagerMenu Input boot manager menu.
> 354
>
>
> Thanks,
>
> Andrew Fish
>
>
[-- Attachment #2: Type: text/html, Size: 68953 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] OVMF is crashing for me in master
2019-10-11 1:19 ` [edk2-devel] " Liming Gao
2019-10-11 4:59 ` Andrew Fish
@ 2019-10-11 11:50 ` Laszlo Ersek
2019-10-11 12:16 ` Pete Batard
2019-10-11 13:13 ` Andrew Fish
1 sibling, 2 replies; 10+ messages in thread
From: Laszlo Ersek @ 2019-10-11 11:50 UTC (permalink / raw)
To: devel, liming.gao, afish@apple.com, Pete Batard
Hi,
On 10/11/19 03:19, Liming Gao wrote:
> Andrew:
> I verify the change (2de1f611be06ded3a59726a4052a9039be7d459b
> MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0)
> in Emulator.
> It works, because PCD value is set to 10 in Emulator.
>
> Before this change, if TimeOut PCD is zero, BdsEntry doesn't call
> PlatformBootManagerWaitCallback().
> After this change, if TimeOut PCD is zero, BdsEntry still call
> PlatformBootManagerWaitCallback(). So, it trigs this issue. I agree
> your fix.
>
> Pete:
> Will you contribute the patch to fix this hang issue in OVMF?
So, when Pete submitted the BDS patch, I didn't test it, I only looked
at the patch email. See my feedback here:
http://mid.mail-archive.com/8bacbb53-98df-724e-b704-b5059d11e378@redhat.com
https://edk2.groups.io/g/devel/message/48133
It seemed OK to me. In particular, I reasoned about (TimeoutRemain==0),
and the patch seemed to do the right thing for that.
Unfortunately, what I missed was the following case:
- if PcdPlatformBootTimeOut is zero, then the patch by Pete does not
simply make one more call to PlatformBootManagerWaitCallback(), with
argument zero, but it makes the *ONLY* call to
PlatformBootManagerWaitCallback().
I missed this because (like normal) the patch was sent with a 3-line
context, and I didn't think it necessary to review the full function
context.
Here is the commit, with full function context:
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> index f3d5e5ac0615..7968a58f3454 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> @@ -310,47 +310,48 @@ VOID
> BdsWait (
> IN EFI_EVENT HotkeyTriggered
> )
> {
> EFI_STATUS Status;
> UINT16 TimeoutRemain;
>
> DEBUG ((EFI_D_INFO, "[Bds]BdsWait ...Zzzzzzzzzzzz...\n"));
>
> TimeoutRemain = PcdGet16 (PcdPlatformBootTimeOut);
> while (TimeoutRemain != 0) {
> DEBUG ((EFI_D_INFO, "[Bds]BdsWait(%d)..Zzzz...\n", (UINTN) TimeoutRemain));
> PlatformBootManagerWaitCallback (TimeoutRemain);
>
> BdsReadKeys (); // BUGBUG: Only reading can signal HotkeyTriggered
> // Can be removed after all keyboard drivers invoke callback in timer callback.
>
> if (HotkeyTriggered != NULL) {
> Status = BdsWaitForSingleEvent (HotkeyTriggered, EFI_TIMER_PERIOD_SECONDS (1));
> if (!EFI_ERROR (Status)) {
> break;
> }
> } else {
> gBS->Stall (1000000);
> }
>
> //
> // 0xffff means waiting forever
> // BDS with no hotkey provided and 0xffff as timeout will "hang" in the loop
> //
> if (TimeoutRemain != 0xffff) {
> TimeoutRemain--;
> }
> }
> + PlatformBootManagerWaitCallback (0);
> DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
> }
We can see that the function used to handle the
PcdPlatformBootTimeOut==0 situation well, and make *no* call to
PlatformBootManagerWaitCallback(). The patch broke that behavior.
Therefore, I disagree with the idea to fix this in OvmfPkg. Any number
of platforms may exist that rely on PlatformBootManagerWaitCallback()
not being called when PcdPlatformBootTimeOut is zero.
(To name just one other example, ArmVirtPkg too is affected.)
And for OVMF and ArmVirtPkg, it is definitely correct to *not* display
any progress bar if PcdPlatformBootTimeOut is 0.
Please consult the definition of the PCD in "MdePkg/MdePkg.dec":
> ## The number of seconds that the firmware will wait before initiating the original default boot selection.
> # A value of 0 indicates that the default boot selection is to be initiated immediately on boot.
> # The value of 0xFFFF then firmware will wait for user input before booting.
> # @Prompt Boot Timeout (s)
> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0xffff|UINT16|0x0000002c
Also see the specification of PlatformBootManagerWaitCallback(), in
"MdeModulePkg/Include/Library/PlatformBootManagerLib.h":
> /**
> This function is called each second during the boot manager waits the timeout.
>
> @param TimeoutRemain The remaining timeout.
> **/
> VOID
> EFIAPI
> PlatformBootManagerWaitCallback (
> UINT16 TimeoutRemain
> );
According to the DEC file, there is no waiting if the PCD is zero.
Therefore, if the PCD is zero, the function should not be called at all.
There is another complication, namely, if the PCD is 0xFFFF. In this
case, the loop goes on until a hotkey is pressed. In every iteration
(that is, every second), PlatformBootManagerWaitCallback() is called
with argument 0xFFFF, invariably. Clearly, this *cannot* cause a
platform to display any progress *changes*. In the most common callback
implementation, namely in the formula
(Timeout - TimeoutRemain) * 100 / Timeout,
it will mean
(0xFFFF - 0xFFFF) * 100 / 0xFFFF,
that is, "zero percent", in every iteration.
Based on that, it looks wrong to suddenly call
PlatformBootManagerWaitCallback() with a zero argument ("hundred percent
completion"), after it's been called, let's say for a minute or more,
with a constant 0xFFFF argument ("zero percent completion"), until the
user finally presses a hotkey.
Jumping from zero to 100% is discontiguous and doesn't appear helpful to
me.
Further yet, what if there is an actual progress bar and timeout (like
10 seconds), and the user presses a hotkey after 5 seconds? Should the
progress bar jump to 100% at once? I wouldn't think so.
Therefore, I claim that the right fix is:
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> index 7968a58f3454..d12829afeb8b 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> @@ -341,7 +341,16 @@ BdsWait (
> TimeoutRemain--;
> }
> }
> - PlatformBootManagerWaitCallback (0);
> + //
> + // If the platform configured a nonzero and finite time-out, and we have
> + // actually reached that, report 100% completion to the platform.
> + //
> + // Note that the (TimeoutRemain == 0) condition excludes
> + // PcdPlatformBootTimeOut=0xFFFF, and that's deliberate.
> + //
> + if (PcdGet16 (PcdPlatformBootTimeOut) != 0 && TimeoutRemain == 0) {
> + PlatformBootManagerWaitCallback (0);
> + }
> DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
> }
>
Thanks!
Laszlo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] OVMF is crashing for me in master
2019-10-11 4:59 ` Andrew Fish
@ 2019-10-11 11:53 ` Laszlo Ersek
2019-10-11 12:49 ` Andrew Fish
0 siblings, 1 reply; 10+ messages in thread
From: Laszlo Ersek @ 2019-10-11 11:53 UTC (permalink / raw)
To: devel, afish, Gao, Liming; +Cc: Pete Batard
On 10/11/19 06:59, Andrew Fish via Groups.Io wrote:
> Liming,
>
> Thanks for looking into this!
>
> Can someone also answer my question about the expected behavior of taking an exception in OVMF? Is the CpuDeadloop() expected?
Yes, it is.
The exception handler dumps the register file and the stack to the
emulated serial port, not to the QEMU debug port.
When looking for OVMF debug messages, people usually (and rightly) check
wherever the QEMU debug port has been redirected. (Unless they built
OVMF with -D DEBUG_ON_SERIAL_PORT.) However, the exception handler
always dumps the state to the serial port, regardless of
DEBUG_ON_SERIAL_PORT. This can be confusing, as the normally consulted
debug log has no indication of the issue.
Thanks
Laszlo
>
> Thanks,
>
> Andrew Fish
>
>> On Oct 10, 2019, at 6:19 PM, Gao, Liming <liming.gao@intel.com> wrote:
>>
>> Andrew:
>> I verify the change (2de1f611be06ded3a59726a4052a9039be7d459b MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0) in Emulator.
>> It works, because PCD value is set to 10 in Emulator.
>>
>> Before this change, if TimeOut PCD is zero, BdsEntry doesn’t call PlatformBootManagerWaitCallback().
>> After this change, if TimeOut PCD is zero, BdsEntry still call PlatformBootManagerWaitCallback(). So, it trigs this issue. I agree your fix.
>>
>> Pete:
>> Will you contribute the patch to fix this hang issue in OVMF?
>>
>> Thanks
>> Liming
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Andrew Fish via Groups.Io
>> Sent: Friday, October 11, 2019 5:12 AM
>> To: devel@edk2.groups.io; Pete Batard <pete@akeo.ie>
>> Subject: [edk2-devel] OVMF is crashing for me in master
>>
>> This is my flavor of OVMF: build -p OvmfPkg/OvmfPkgX64.dsc -a X64 -t XCODE5
>>
>> It looks like I took an exception? Is it expected that an unhandled exception just hang in a dead loop? I would have expected some serial output about the failure?
>>
>> Looks like a divide by zero exception. The exception context has PC and FP so I can manually walk the stack. Yikes I see PlatformBootManagerWaitCallback() will fault if PcdPlatformBootTimeOut is zero?
>> /Volumes/Case/UDK2018(master)>git grep PcdPlatformBootTimeOut -- *.dsc
>> ArmVirtPkg/ArmVirtQemu.dsc:194: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
>> ArmVirtPkg/ArmVirtQemuKernel.dsc:191: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
>> ArmVirtPkg/ArmVirtXen.dsc:122: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
>> EmulatorPkg/EmulatorPkg.dsc:236: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|10
>> OvmfPkg/OvmfPkgIa32.dsc:541: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
>> OvmfPkg/OvmfPkgIa32X64.dsc:553: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
>> OvmfPkg/OvmfPkgX64.dsc:552: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
>> OvmfPkg/OvmfXen.dsc:470: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
>> UefiPayloadPkg/UefiPayloadPkgIa32.dsc:344: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
>> UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc:345: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
>>
>>
>> OK PcdPlatformBootTimeOut is zero on Ovmf, so how did this ever work?
>>
>>
>> Ahhh gotom....
>> /Volumes/Case/UDK2018(master)>git blame -L344,344 /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> 2de1f611be0 (Pete Batard 2019-09-25 23:50:05 +0800 344) PlatformBootManagerWaitCallback (0);
>>
>>
>> This call causes a divide by zero if PcdPlatformBootTimeOut == 0.
>>
>> This fixes my crash:
>> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>> index 70df6b841a..d6ae43e900 100644
>> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>> @@ -1634,6 +1634,11 @@ PlatformBootManagerWaitCallback (
>> UINT16 Timeout;
>>
>> Timeout = PcdGet16 (PcdPlatformBootTimeOut);
>> + if (Timeout == 0) {
>> + Timeout = 100;
>> + } else {
>> + Timeout = (Timeout - TimeoutRemain) * 100 / Timeout;
>> + }
>>
>> Black.Raw = 0x00000000;
>> White.Raw = 0x00FFFFFF;
>> @@ -1643,7 +1648,7 @@ PlatformBootManagerWaitCallback (
>> Black.Pixel,
>> L"Start boot option",
>> White.Pixel,
>> - (Timeout - TimeoutRemain) * 100 / Timeout,
>> + Timeout,
>> 0
>> );
>> }
>>
>>
>>
>> lldb debugger output:
>>
>> (lldb) bt
>> * thread #1, stop reason = signal SIGTRAP
>> * frame #0: 0x0000000007b58a70 CpuDxe.dll:CpuDeadLoop() + 13 at /Volumes/Case/UDK2018/MdePkg/Library/BaseLib/CpuDeadLoop.c:31
>> frame #1: 0x0000000007b61222 CpuDxe.dll:CommonExceptionHandlerWorker() + 674 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:115
>> frame #2: 0x0000000007b61624 CpuDxe.dll:CommonExceptionHandler() + 36 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c:40
>> frame #3: 0x0000000007b5ff26 CpuDxe.dll:HasErrorCode() + 230
>> (lldb) fr sel 1
>> frame #1: 0x0000000007b61222 CpuDxe.dll:CommonExceptionHandlerWorker() + 674 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:115
>> 91 (ExternalInterruptHandler[ExceptionType]) (ExceptionType, SystemContext);
>> 92 } else if (ExceptionType < CPU_EXCEPTION_NUM) {
>> 93 //
>> 94 // Get Spinlock to display CPU information
>> 95 //
>> 96 while (!AcquireSpinLockOrFail (&ExceptionHandlerData->DisplayMessageSpinLock)) {
>> 97 CpuPause ();
>> 98 }
>> 99 //
>> 100 // Initialize the serial port before dumping.
>> 101 //
>> 102 SerialPortInitialize ();
>> 103 //
>> 104 // Display ExceptionType, CPU information and Image information
>> 105 //
>> 106 DumpImageAndCpuContent (ExceptionType, SystemContext);
>> 107 //
>> 108 // Release Spinlock of output message
>> 109 //
>> 110 ReleaseSpinLock (&ExceptionHandlerData->DisplayMessageSpinLock);
>> 111 //
>> 112 // Enter a dead loop if needn't to execute old IDT handler further
>> 113 //
>> 114 if (ReservedVectors[ExceptionType].Attribute != EFI_VECTOR_HANDOFF_HOOK_BEFORE) {
>> -> 115 CpuDeadLoop ();
>> 116 }
>> 117 }
>> 118 }
>> 119
>> (lldb) p ExceptionType
>> (EFI_EXCEPTION_TYPE) $0 = 0
>> (lldb) p SystemContext.SystemContextX64->Rip
>> (UINT64) $1 = 0x0000000007a9cc38
>> (lldb) p SystemContext.SystemContextX64->Rbp
>> (UINT64) $2 = 0x0000000007e8fc20
>> (lldb) efi_backtrace --pc 0x0000000007a9cc38 --frame 0x0000000007e8fc20 --symbols
>> frame 0: 0x07a9cc38 BdsDxe:PlatformBootManagerWaitCallback + 35 at /Volumes/Case/UDK2018/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c:1646:37
>> frame 1: 0x07a9e744 BdsDxe:BdsWait + 269 at /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:344:3
>> frame 2: 0x07a9dff9 BdsDxe:BdsEntry + 2620 at /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:1002:5
>> frame 3: 0x07eb367d DxeCore:DxeMain + 2680 at /Volumes/Case/UDK2018/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c:544:3
>> frame 4: 0x07e943cb DxeCore:_ModuleEntryPoint + 20 at /Volumes/Case/UDK2018/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.c:48:3
>> frame 5: 0x07edc947 DxeIpl.dll`AsmEnableCache
>> frame 6: 0x07ee1e4e DxeIpl:HandOffToDxeCore + 509 at /Volumes/Case/UDK2018/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c:113:3
>> frame 7: 0x07ee0604 DxeIpl:DxeLoadCore + 1354 at /Volumes/Case/UDK2018/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c:449:3
>> frame 8: 0x07eeff2f PeiCore.dll`PeiCore.cold.3 + 847
>> frame 9: 0x07ee9d04 PeiCore:PeiCore + 163 at /Volumes/Case/UDK2018/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c:502:3
>> frame 10: 0x0082c387 0x0082c387
>> frame 11: 0x00825dd7 0x00825dd7
>> frame 12: 0x0082ad27 0x0082ad27
>> frame 13: 0x0082b3a8 0x0082b3a8
>> frame 14: 0x0082bf23 0x0082bf23
>> frame 15: 0x00825e24 0x00825e24
>> frame 16: 0x00823af2 0x00823af2
>> frame 17: 0xfffd1db8 SecMain:SecStartupPhase2 + 67 at /Volumes/Case/UDK2018/OvmfPkg/Sec/SecMain.c:858:3
>> frame 18: 0xfffd1d67 SecMain:SecCoreStartupWithStack + 420 at /Volumes/Case/UDK2018/OvmfPkg/Sec/SecMain.c:821:3
>> frame 19: 0xfffd1e14 SecMain:ProcessLibraryConstructorList + 0 at /Volumes/Case/UDK2018/Build/OvmfX64/DEBUG_XCODE5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.c:201
>>
>> (lldb) l /Volumes/Case/UDK2018/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c:1646
>> 1646 (Timeout - TimeoutRemain) * 100 / Timeout,
>> 1647 0
>> 1648 );
>> 1649 }
>> 1650
>> 1651 /**
>> 1652 The function is called when no boot option could be launched,
>> 1653 including platform recovery options and options pointing to applications
>> 1654 built into firmware volumes.
>> 1655
>> (lldb) l /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:344
>> 344 PlatformBootManagerWaitCallback (0);
>> 345 DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
>> 346 }
>> 347
>> 348 /**
>> 349 Attempt to boot each boot option in the BootOptions array.
>> 350
>> 351 @param BootOptions Input boot option array.
>> 352 @param BootOptionCount Input boot option count.
>> 353 @param BootManagerMenu Input boot manager menu.
>> 354
>>
>>
>> Thanks,
>>
>> Andrew Fish
>>
>>
>
>
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] OVMF is crashing for me in master
2019-10-11 11:50 ` Laszlo Ersek
@ 2019-10-11 12:16 ` Pete Batard
2019-10-11 13:13 ` Andrew Fish
1 sibling, 0 replies; 10+ messages in thread
From: Pete Batard @ 2019-10-11 12:16 UTC (permalink / raw)
To: Laszlo Ersek, devel, liming.gao, afish@apple.com
Hi Lazlo,
On 2019.10.11 12:50, Laszlo Ersek wrote:
> Hi,
>
> On 10/11/19 03:19, Liming Gao wrote:
>> Andrew:
>> I verify the change (2de1f611be06ded3a59726a4052a9039be7d459b
>> MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0)
>> in Emulator.
>> It works, because PCD value is set to 10 in Emulator.
>>
>> Before this change, if TimeOut PCD is zero, BdsEntry doesn't call
>> PlatformBootManagerWaitCallback().
>> After this change, if TimeOut PCD is zero, BdsEntry still call
>> PlatformBootManagerWaitCallback(). So, it trigs this issue. I agree
>> your fix.
>>
>> Pete:
>> Will you contribute the patch to fix this hang issue in OVMF?
>
> So, when Pete submitted the BDS patch, I didn't test it, I only looked
> at the patch email. See my feedback here:
>
> http://mid.mail-archive.com/8bacbb53-98df-724e-b704-b5059d11e378@redhat.com
> https://edk2.groups.io/g/devel/message/48133
>
> It seemed OK to me. In particular, I reasoned about (TimeoutRemain==0),
> and the patch seemed to do the right thing for that.
>
> Unfortunately, what I missed was the following case:
>
> - if PcdPlatformBootTimeOut is zero, then the patch by Pete does not
> simply make one more call to PlatformBootManagerWaitCallback(), with
> argument zero, but it makes the *ONLY* call to
> PlatformBootManagerWaitCallback().
>
> I missed this because (like normal) the patch was sent with a 3-line
> context, and I didn't think it necessary to review the full function
> context.
>
> Here is the commit, with full function context:
>
>> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> index f3d5e5ac0615..7968a58f3454 100644
>> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> @@ -310,47 +310,48 @@ VOID
>> BdsWait (
>> IN EFI_EVENT HotkeyTriggered
>> )
>> {
>> EFI_STATUS Status;
>> UINT16 TimeoutRemain;
>>
>> DEBUG ((EFI_D_INFO, "[Bds]BdsWait ...Zzzzzzzzzzzz...\n"));
>>
>> TimeoutRemain = PcdGet16 (PcdPlatformBootTimeOut);
>> while (TimeoutRemain != 0) {
>> DEBUG ((EFI_D_INFO, "[Bds]BdsWait(%d)..Zzzz...\n", (UINTN) TimeoutRemain));
>> PlatformBootManagerWaitCallback (TimeoutRemain);
>>
>> BdsReadKeys (); // BUGBUG: Only reading can signal HotkeyTriggered
>> // Can be removed after all keyboard drivers invoke callback in timer callback.
>>
>> if (HotkeyTriggered != NULL) {
>> Status = BdsWaitForSingleEvent (HotkeyTriggered, EFI_TIMER_PERIOD_SECONDS (1));
>> if (!EFI_ERROR (Status)) {
>> break;
>> }
>> } else {
>> gBS->Stall (1000000);
>> }
>>
>> //
>> // 0xffff means waiting forever
>> // BDS with no hotkey provided and 0xffff as timeout will "hang" in the loop
>> //
>> if (TimeoutRemain != 0xffff) {
>> TimeoutRemain--;
>> }
>> }
>> + PlatformBootManagerWaitCallback (0);
>> DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
>> }
>
> We can see that the function used to handle the
> PcdPlatformBootTimeOut==0 situation well, and make *no* call to
> PlatformBootManagerWaitCallback(). The patch broke that behavior.
>
> Therefore, I disagree with the idea to fix this in OvmfPkg. Any number
> of platforms may exist that rely on PlatformBootManagerWaitCallback()
> not being called when PcdPlatformBootTimeOut is zero.
>
> (To name just one other example, ArmVirtPkg too is affected.)
>
> And for OVMF and ArmVirtPkg, it is definitely correct to *not* display
> any progress bar if PcdPlatformBootTimeOut is 0.
>
> Please consult the definition of the PCD in "MdePkg/MdePkg.dec":
>
>> ## The number of seconds that the firmware will wait before initiating the original default boot selection.
>> # A value of 0 indicates that the default boot selection is to be initiated immediately on boot.
>> # The value of 0xFFFF then firmware will wait for user input before booting.
>> # @Prompt Boot Timeout (s)
>> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0xffff|UINT16|0x0000002c
>
> Also see the specification of PlatformBootManagerWaitCallback(), in
> "MdeModulePkg/Include/Library/PlatformBootManagerLib.h":
>
>> /**
>> This function is called each second during the boot manager waits the timeout.
>>
>> @param TimeoutRemain The remaining timeout.
>> **/
>> VOID
>> EFIAPI
>> PlatformBootManagerWaitCallback (
>> UINT16 TimeoutRemain
>> );
>
> According to the DEC file, there is no waiting if the PCD is zero.
> Therefore, if the PCD is zero, the function should not be called at all.
>
>
> There is another complication, namely, if the PCD is 0xFFFF. In this
> case, the loop goes on until a hotkey is pressed. In every iteration
> (that is, every second), PlatformBootManagerWaitCallback() is called
> with argument 0xFFFF, invariably. Clearly, this *cannot* cause a
> platform to display any progress *changes*. In the most common callback
> implementation, namely in the formula
>
> (Timeout - TimeoutRemain) * 100 / Timeout,
>
> it will mean
>
> (0xFFFF - 0xFFFF) * 100 / 0xFFFF,
>
> that is, "zero percent", in every iteration.
>
> Based on that, it looks wrong to suddenly call
> PlatformBootManagerWaitCallback() with a zero argument ("hundred percent
> completion"), after it's been called, let's say for a minute or more,
> with a constant 0xFFFF argument ("zero percent completion"), until the
> user finally presses a hotkey.
>
> Jumping from zero to 100% is discontiguous and doesn't appear helpful to
> me.
>
>
> Further yet, what if there is an actual progress bar and timeout (like
> 10 seconds), and the user presses a hotkey after 5 seconds? Should the
> progress bar jump to 100% at once? I wouldn't think so.
>
>
> Therefore, I claim that the right fix is:
>
>> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> index 7968a58f3454..d12829afeb8b 100644
>> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> @@ -341,7 +341,16 @@ BdsWait (
>> TimeoutRemain--;
>> }
>> }
>> - PlatformBootManagerWaitCallback (0);
>> + //
>> + // If the platform configured a nonzero and finite time-out, and we have
>> + // actually reached that, report 100% completion to the platform.
>> + //
>> + // Note that the (TimeoutRemain == 0) condition excludes
>> + // PcdPlatformBootTimeOut=0xFFFF, and that's deliberate.
>> + //
>> + if (PcdGet16 (PcdPlatformBootTimeOut) != 0 && TimeoutRemain == 0) {
>> + PlatformBootManagerWaitCallback (0);
>> + }
>> DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
>> }
>>
I agree with the analysis and the proposed fix. My earlier patch was
indeed altering existing behaviour when PcdPlatformBootTimeOut was 0,
and I completely missed that.
Also, apologies for not replying to the direct request you sent to me
earlier. The e-mail client I use does not currently deduplicate messages
that are sent both to the list as well as directly to me as CC, so I
only get one copy in the list which makes these requests easy to miss.
I'll try to fix that.
Many thanks for looking into this!
Regards,
/Pete
>
> Thanks!
> Laszlo
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] OVMF is crashing for me in master
2019-10-11 11:53 ` Laszlo Ersek
@ 2019-10-11 12:49 ` Andrew Fish
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Fish @ 2019-10-11 12:49 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: devel, Gao, Liming, Pete Batard
[-- Attachment #1: Type: text/plain, Size: 10374 bytes --]
> On Oct 11, 2019, at 4:53 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 10/11/19 06:59, Andrew Fish via Groups.Io wrote:
>> Liming,
>>
>> Thanks for looking into this!
>>
>> Can someone also answer my question about the expected behavior of taking an exception in OVMF? Is the CpuDeadloop() expected?
>
> Yes, it is.
>
> The exception handler dumps the register file and the stack to the
> emulated serial port, not to the QEMU debug port.
>
> When looking for OVMF debug messages, people usually (and rightly) check
> wherever the QEMU debug port has been redirected. (Unless they built
> OVMF with -D DEBUG_ON_SERIAL_PORT.) However, the exception handler
> always dumps the state to the serial port, regardless of
> DEBUG_ON_SERIAL_PORT. This can be confusing, as the normally consulted
> debug log has no indication of the issue.
>
Laszlo,
Thanks if I add -serial file:serial.log to the QEMU command line I see the exception in serial.log.
Thanks,
Andrew Fish
> Thanks
> Laszlo
>
>>
>> Thanks,
>>
>> Andrew Fish
>>
>>> On Oct 10, 2019, at 6:19 PM, Gao, Liming <liming.gao@intel.com> wrote:
>>>
>>> Andrew:
>>> I verify the change (2de1f611be06ded3a59726a4052a9039be7d459b MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0) in Emulator.
>>> It works, because PCD value is set to 10 in Emulator.
>>>
>>> Before this change, if TimeOut PCD is zero, BdsEntry doesn’t call PlatformBootManagerWaitCallback().
>>> After this change, if TimeOut PCD is zero, BdsEntry still call PlatformBootManagerWaitCallback(). So, it trigs this issue. I agree your fix.
>>>
>>> Pete:
>>> Will you contribute the patch to fix this hang issue in OVMF?
>>>
>>> Thanks
>>> Liming
>>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of Andrew Fish via Groups.Io
>>> Sent: Friday, October 11, 2019 5:12 AM
>>> To: devel@edk2.groups.io; Pete Batard <pete@akeo.ie>
>>> Subject: [edk2-devel] OVMF is crashing for me in master
>>>
>>> This is my flavor of OVMF: build -p OvmfPkg/OvmfPkgX64.dsc -a X64 -t XCODE5
>>>
>>> It looks like I took an exception? Is it expected that an unhandled exception just hang in a dead loop? I would have expected some serial output about the failure?
>>>
>>> Looks like a divide by zero exception. The exception context has PC and FP so I can manually walk the stack. Yikes I see PlatformBootManagerWaitCallback() will fault if PcdPlatformBootTimeOut is zero?
>>> /Volumes/Case/UDK2018(master)>git grep PcdPlatformBootTimeOut -- *.dsc
>>> ArmVirtPkg/ArmVirtQemu.dsc:194: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
>>> ArmVirtPkg/ArmVirtQemuKernel.dsc:191: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
>>> ArmVirtPkg/ArmVirtXen.dsc:122: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
>>> EmulatorPkg/EmulatorPkg.dsc:236: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|L"Timeout"|gEfiGlobalVariableGuid|0x0|10
>>> OvmfPkg/OvmfPkgIa32.dsc:541: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
>>> OvmfPkg/OvmfPkgIa32X64.dsc:553: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
>>> OvmfPkg/OvmfPkgX64.dsc:552: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
>>> OvmfPkg/OvmfXen.dsc:470: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0
>>> UefiPayloadPkg/UefiPayloadPkgIa32.dsc:344: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
>>> UefiPayloadPkg/UefiPayloadPkgIa32X64.dsc:345: gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|3
>>>
>>>
>>> OK PcdPlatformBootTimeOut is zero on Ovmf, so how did this ever work?
>>>
>>>
>>> Ahhh gotom....
>>> /Volumes/Case/UDK2018(master)>git blame -L344,344 /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>>> 2de1f611be0 (Pete Batard 2019-09-25 23:50:05 +0800 344) PlatformBootManagerWaitCallback (0);
>>>
>>>
>>> This call causes a divide by zero if PcdPlatformBootTimeOut == 0.
>>>
>>> This fixes my crash:
>>> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>>> index 70df6b841a..d6ae43e900 100644
>>> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>>> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
>>> @@ -1634,6 +1634,11 @@ PlatformBootManagerWaitCallback (
>>> UINT16 Timeout;
>>>
>>> Timeout = PcdGet16 (PcdPlatformBootTimeOut);
>>> + if (Timeout == 0) {
>>> + Timeout = 100;
>>> + } else {
>>> + Timeout = (Timeout - TimeoutRemain) * 100 / Timeout;
>>> + }
>>>
>>> Black.Raw = 0x00000000;
>>> White.Raw = 0x00FFFFFF;
>>> @@ -1643,7 +1648,7 @@ PlatformBootManagerWaitCallback (
>>> Black.Pixel,
>>> L"Start boot option",
>>> White.Pixel,
>>> - (Timeout - TimeoutRemain) * 100 / Timeout,
>>> + Timeout,
>>> 0
>>> );
>>> }
>>>
>>>
>>>
>>> lldb debugger output:
>>>
>>> (lldb) bt
>>> * thread #1, stop reason = signal SIGTRAP
>>> * frame #0: 0x0000000007b58a70 CpuDxe.dll:CpuDeadLoop() + 13 at /Volumes/Case/UDK2018/MdePkg/Library/BaseLib/CpuDeadLoop.c:31
>>> frame #1: 0x0000000007b61222 CpuDxe.dll:CommonExceptionHandlerWorker() + 674 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:115
>>> frame #2: 0x0000000007b61624 CpuDxe.dll:CommonExceptionHandler() + 36 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeException.c:40
>>> frame #3: 0x0000000007b5ff26 CpuDxe.dll:HasErrorCode() + 230
>>> (lldb) fr sel 1
>>> frame #1: 0x0000000007b61222 CpuDxe.dll:CommonExceptionHandlerWorker() + 674 at /Volumes/Case/UDK2018/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiDxeSmmCpuException.c:115
>>> 91 (ExternalInterruptHandler[ExceptionType]) (ExceptionType, SystemContext);
>>> 92 } else if (ExceptionType < CPU_EXCEPTION_NUM) {
>>> 93 //
>>> 94 // Get Spinlock to display CPU information
>>> 95 //
>>> 96 while (!AcquireSpinLockOrFail (&ExceptionHandlerData->DisplayMessageSpinLock)) {
>>> 97 CpuPause ();
>>> 98 }
>>> 99 //
>>> 100 // Initialize the serial port before dumping.
>>> 101 //
>>> 102 SerialPortInitialize ();
>>> 103 //
>>> 104 // Display ExceptionType, CPU information and Image information
>>> 105 //
>>> 106 DumpImageAndCpuContent (ExceptionType, SystemContext);
>>> 107 //
>>> 108 // Release Spinlock of output message
>>> 109 //
>>> 110 ReleaseSpinLock (&ExceptionHandlerData->DisplayMessageSpinLock);
>>> 111 //
>>> 112 // Enter a dead loop if needn't to execute old IDT handler further
>>> 113 //
>>> 114 if (ReservedVectors[ExceptionType].Attribute != EFI_VECTOR_HANDOFF_HOOK_BEFORE) {
>>> -> 115 CpuDeadLoop ();
>>> 116 }
>>> 117 }
>>> 118 }
>>> 119
>>> (lldb) p ExceptionType
>>> (EFI_EXCEPTION_TYPE) $0 = 0
>>> (lldb) p SystemContext.SystemContextX64->Rip
>>> (UINT64) $1 = 0x0000000007a9cc38
>>> (lldb) p SystemContext.SystemContextX64->Rbp
>>> (UINT64) $2 = 0x0000000007e8fc20
>>> (lldb) efi_backtrace --pc 0x0000000007a9cc38 --frame 0x0000000007e8fc20 --symbols
>>> frame 0: 0x07a9cc38 BdsDxe:PlatformBootManagerWaitCallback + 35 at /Volumes/Case/UDK2018/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c:1646:37
>>> frame 1: 0x07a9e744 BdsDxe:BdsWait + 269 at /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:344:3
>>> frame 2: 0x07a9dff9 BdsDxe:BdsEntry + 2620 at /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:1002:5
>>> frame 3: 0x07eb367d DxeCore:DxeMain + 2680 at /Volumes/Case/UDK2018/MdeModulePkg/Core/Dxe/DxeMain/DxeMain.c:544:3
>>> frame 4: 0x07e943cb DxeCore:_ModuleEntryPoint + 20 at /Volumes/Case/UDK2018/MdePkg/Library/DxeCoreEntryPoint/DxeCoreEntryPoint.c:48:3
>>> frame 5: 0x07edc947 DxeIpl.dll`AsmEnableCache
>>> frame 6: 0x07ee1e4e DxeIpl:HandOffToDxeCore + 509 at /Volumes/Case/UDK2018/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c:113:3
>>> frame 7: 0x07ee0604 DxeIpl:DxeLoadCore + 1354 at /Volumes/Case/UDK2018/MdeModulePkg/Core/DxeIplPeim/DxeLoad.c:449:3
>>> frame 8: 0x07eeff2f PeiCore.dll`PeiCore.cold.3 + 847
>>> frame 9: 0x07ee9d04 PeiCore:PeiCore + 163 at /Volumes/Case/UDK2018/MdeModulePkg/Core/Pei/PeiMain/PeiMain.c:502:3
>>> frame 10: 0x0082c387 0x0082c387
>>> frame 11: 0x00825dd7 0x00825dd7
>>> frame 12: 0x0082ad27 0x0082ad27
>>> frame 13: 0x0082b3a8 0x0082b3a8
>>> frame 14: 0x0082bf23 0x0082bf23
>>> frame 15: 0x00825e24 0x00825e24
>>> frame 16: 0x00823af2 0x00823af2
>>> frame 17: 0xfffd1db8 SecMain:SecStartupPhase2 + 67 at /Volumes/Case/UDK2018/OvmfPkg/Sec/SecMain.c:858:3
>>> frame 18: 0xfffd1d67 SecMain:SecCoreStartupWithStack + 420 at /Volumes/Case/UDK2018/OvmfPkg/Sec/SecMain.c:821:3
>>> frame 19: 0xfffd1e14 SecMain:ProcessLibraryConstructorList + 0 at /Volumes/Case/UDK2018/Build/OvmfX64/DEBUG_XCODE5/X64/OvmfPkg/Sec/SecMain/DEBUG/AutoGen.c:201
>>>
>>> (lldb) l /Volumes/Case/UDK2018/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c:1646
>>> 1646 (Timeout - TimeoutRemain) * 100 / Timeout,
>>> 1647 0
>>> 1648 );
>>> 1649 }
>>> 1650
>>> 1651 /**
>>> 1652 The function is called when no boot option could be launched,
>>> 1653 including platform recovery options and options pointing to applications
>>> 1654 built into firmware volumes.
>>> 1655
>>> (lldb) l /Volumes/Case/UDK2018/MdeModulePkg/Universal/BdsDxe/BdsEntry.c:344
>>> 344 PlatformBootManagerWaitCallback (0);
>>> 345 DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
>>> 346 }
>>> 347
>>> 348 /**
>>> 349 Attempt to boot each boot option in the BootOptions array.
>>> 350
>>> 351 @param BootOptions Input boot option array.
>>> 352 @param BootOptionCount Input boot option count.
>>> 353 @param BootManagerMenu Input boot manager menu.
>>> 354
>>>
>>>
>>> Thanks,
>>>
>>> Andrew Fish
>>>
>>>
>>
>>
>>
[-- Attachment #2: Type: text/html, Size: 27212 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] OVMF is crashing for me in master
2019-10-11 11:50 ` Laszlo Ersek
2019-10-11 12:16 ` Pete Batard
@ 2019-10-11 13:13 ` Andrew Fish
2019-10-11 16:33 ` Laszlo Ersek
1 sibling, 1 reply; 10+ messages in thread
From: Andrew Fish @ 2019-10-11 13:13 UTC (permalink / raw)
To: devel, lersek; +Cc: liming.gao, Pete Batard
[-- Attachment #1: Type: text/plain, Size: 10066 bytes --]
> On Oct 11, 2019, at 4:50 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>
> Hi,
>
> On 10/11/19 03:19, Liming Gao wrote:
>> Andrew:
>> I verify the change (2de1f611be06ded3a59726a4052a9039be7d459b
>> MdeModulePkg/BdsDxe: Also call PlatformBootManagerWaitCallback on 0)
>> in Emulator.
>> It works, because PCD value is set to 10 in Emulator.
>>
>> Before this change, if TimeOut PCD is zero, BdsEntry doesn't call
>> PlatformBootManagerWaitCallback().
>> After this change, if TimeOut PCD is zero, BdsEntry still call
>> PlatformBootManagerWaitCallback(). So, it trigs this issue. I agree
>> your fix.
>>
>> Pete:
>> Will you contribute the patch to fix this hang issue in OVMF?
>
> So, when Pete submitted the BDS patch, I didn't test it, I only looked
> at the patch email. See my feedback here:
>
> http://mid.mail-archive.com/8bacbb53-98df-724e-b704-b5059d11e378@redhat.com <http://mid.mail-archive.com/8bacbb53-98df-724e-b704-b5059d11e378@redhat.com>
> https://edk2.groups.io/g/devel/message/48133 <https://edk2.groups.io/g/devel/message/48133>
>
> It seemed OK to me. In particular, I reasoned about (TimeoutRemain==0),
> and the patch seemed to do the right thing for that.
>
> Unfortunately, what I missed was the following case:
>
> - if PcdPlatformBootTimeOut is zero, then the patch by Pete does not
> simply make one more call to PlatformBootManagerWaitCallback(), with
> argument zero, but it makes the *ONLY* call to
> PlatformBootManagerWaitCallback().
>
> I missed this because (like normal) the patch was sent with a 3-line
> context, and I didn't think it necessary to review the full function
> context.
>
> Here is the commit, with full function context:
>
>> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> index f3d5e5ac0615..7968a58f3454 100644
>> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> @@ -310,47 +310,48 @@ VOID
>> BdsWait (
>> IN EFI_EVENT HotkeyTriggered
>> )
>> {
>> EFI_STATUS Status;
>> UINT16 TimeoutRemain;
>>
>> DEBUG ((EFI_D_INFO, "[Bds]BdsWait ...Zzzzzzzzzzzz...\n"));
>>
>> TimeoutRemain = PcdGet16 (PcdPlatformBootTimeOut);
>> while (TimeoutRemain != 0) {
>> DEBUG ((EFI_D_INFO, "[Bds]BdsWait(%d)..Zzzz...\n", (UINTN) TimeoutRemain));
>> PlatformBootManagerWaitCallback (TimeoutRemain);
>>
>> BdsReadKeys (); // BUGBUG: Only reading can signal HotkeyTriggered
>> // Can be removed after all keyboard drivers invoke callback in timer callback.
>>
>> if (HotkeyTriggered != NULL) {
>> Status = BdsWaitForSingleEvent (HotkeyTriggered, EFI_TIMER_PERIOD_SECONDS (1));
>> if (!EFI_ERROR (Status)) {
>> break;
>> }
>> } else {
>> gBS->Stall (1000000);
>> }
>>
>> //
>> // 0xffff means waiting forever
>> // BDS with no hotkey provided and 0xffff as timeout will "hang" in the loop
>> //
>> if (TimeoutRemain != 0xffff) {
>> TimeoutRemain--;
>> }
>> }
>> + PlatformBootManagerWaitCallback (0);
>> DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
>> }
>
> We can see that the function used to handle the
> PcdPlatformBootTimeOut==0 situation well, and make *no* call to
> PlatformBootManagerWaitCallback(). The patch broke that behavior.
>
> Therefore, I disagree with the idea to fix this in OvmfPkg. Any number
> of platforms may exist that rely on PlatformBootManagerWaitCallback()
> not being called when PcdPlatformBootTimeOut is zero.
>
> (To name just one other example, ArmVirtPkg too is affected.)
>
> And for OVMF and ArmVirtPkg, it is definitely correct to *not* display
> any progress bar if PcdPlatformBootTimeOut is 0.
>
> Please consult the definition of the PCD in "MdePkg/MdePkg.dec":
>
>> ## The number of seconds that the firmware will wait before initiating the original default boot selection.
>> # A value of 0 indicates that the default boot selection is to be initiated immediately on boot.
>> # The value of 0xFFFF then firmware will wait for user input before booting.
>> # @Prompt Boot Timeout (s)
>> gEfiMdePkgTokenSpaceGuid.PcdPlatformBootTimeOut|0xffff|UINT16|0x0000002c
>
> Also see the specification of PlatformBootManagerWaitCallback(), in
> "MdeModulePkg/Include/Library/PlatformBootManagerLib.h":
>
>> /**
>> This function is called each second during the boot manager waits the timeout.
>>
>> @param TimeoutRemain The remaining timeout.
>> **/
>> VOID
>> EFIAPI
>> PlatformBootManagerWaitCallback (
>> UINT16 TimeoutRemain
>> );
>
> According to the DEC file, there is no waiting if the PCD is zero.
> Therefore, if the PCD is zero, the function should not be called at all.
>
>
> There is another complication, namely, if the PCD is 0xFFFF. In this
> case, the loop goes on until a hotkey is pressed. In every iteration
> (that is, every second), PlatformBootManagerWaitCallback() is called
> with argument 0xFFFF, invariably. Clearly, this *cannot* cause a
> platform to display any progress *changes*. In the most common callback
> implementation, namely in the formula
>
> (Timeout - TimeoutRemain) * 100 / Timeout,
>
> it will mean
>
> (0xFFFF - 0xFFFF) * 100 / 0xFFFF,
>
> that is, "zero percent", in every iteration.
>
> Based on that, it looks wrong to suddenly call
> PlatformBootManagerWaitCallback() with a zero argument ("hundred percent
> completion"), after it's been called, let's say for a minute or more,
> with a constant 0xFFFF argument ("zero percent completion"), until the
> user finally presses a hotkey.
>
> Jumping from zero to 100% is discontiguous and doesn't appear helpful to
> me.
>
>
> Further yet, what if there is an actual progress bar and timeout (like
> 10 seconds), and the user presses a hotkey after 5 seconds? Should the
> progress bar jump to 100% at once? I wouldn't think so.
>
Laszlo,
I'm with Pete on this as my expectation would be the progress bar completes when you are done. I'd also point out that a common progress bar UI implementation is to show the area that is going to be updated. I don't think our UI does that, but it could get updated at some point. Thus I would posit that if PlatformBootManagerWaitCallback() is called by passing in PcdPlatformBootTimeOut you should always call PlatformBootManagerWaitCallback() with zero to terminate the progress bar.
What makes sense to me is to skip the UI if PcdPlatformBootTimeOut is zero. The PlatformBootManagerWaitCallback (0) is only really needed if PlatformBootManagerWaitCallback (0) has not been called (TimeoutRemain != 0).
diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
index 7968a58f34..fd551d17e1 100644
--- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
+++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
@@ -317,31 +317,35 @@ BdsWait (
DEBUG ((EFI_D_INFO, "[Bds]BdsWait ...Zzzzzzzzzzzz...\n"));
TimeoutRemain = PcdGet16 (PcdPlatformBootTimeOut);
- while (TimeoutRemain != 0) {
- DEBUG ((EFI_D_INFO, "[Bds]BdsWait(%d)..Zzzz...\n", (UINTN) TimeoutRemain));
- PlatformBootManagerWaitCallback (TimeoutRemain);
+ if (TimeoutRemain != 0) {
+ while (TimeoutRemain != 0) {
+ DEBUG ((EFI_D_INFO, "[Bds]BdsWait(%d)..Zzzz...\n", (UINTN) TimeoutRemain));
+ PlatformBootManagerWaitCallback (TimeoutRemain);
- BdsReadKeys (); // BUGBUG: Only reading can signal HotkeyTriggered
- // Can be removed after all keyboard drivers invoke callback in timer callback.
+ BdsReadKeys (); // BUGBUG: Only reading can signal HotkeyTriggered
+ // Can be removed after all keyboard drivers invoke callback in timer callback.
- if (HotkeyTriggered != NULL) {
- Status = BdsWaitForSingleEvent (HotkeyTriggered, EFI_TIMER_PERIOD_SECONDS (1));
- if (!EFI_ERROR (Status)) {
- break;
+ if (HotkeyTriggered != NULL) {
+ Status = BdsWaitForSingleEvent (HotkeyTriggered, EFI_TIMER_PERIOD_SECONDS (1));
+ if (!EFI_ERROR (Status)) {
+ break;
+ }
+ } else {
+ gBS->Stall (1000000);
}
- } else {
- gBS->Stall (1000000);
- }
- //
- // 0xffff means waiting forever
- // BDS with no hotkey provided and 0xffff as timeout will "hang" in the loop
- //
- if (TimeoutRemain != 0xffff) {
- TimeoutRemain--;
+ //
+ // 0xffff means waiting forever
+ // BDS with no hotkey provided and 0xffff as timeout will "hang" in the loop
+ //
+ if (TimeoutRemain != 0xffff) {
+ TimeoutRemain--;
+ }
+ }
+ if (TimeoutRemain != 0) {
+ PlatformBootManagerWaitCallback (0);
}
}
- PlatformBootManagerWaitCallback (0);
DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
}
Thanks,
Andrew Fish
>
> Therefore, I claim that the right fix is:
>
>> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> index 7968a58f3454..d12829afeb8b 100644
>> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> @@ -341,7 +341,16 @@ BdsWait (
>> TimeoutRemain--;
>> }
>> }
>> - PlatformBootManagerWaitCallback (0);
>> + //
>> + // If the platform configured a nonzero and finite time-out, and we have
>> + // actually reached that, report 100% completion to the platform.
>> + //
>> + // Note that the (TimeoutRemain == 0) condition excludes
>> + // PcdPlatformBootTimeOut=0xFFFF, and that's deliberate.
>> + //
>> + if (PcdGet16 (PcdPlatformBootTimeOut) != 0 && TimeoutRemain == 0) {
>> + PlatformBootManagerWaitCallback (0);
>> + }
>> DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
>> }
>>
>
> Thanks!
> Laszlo
>
>
[-- Attachment #2: Type: text/html, Size: 79693 bytes --]
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [edk2-devel] OVMF is crashing for me in master
2019-10-11 13:13 ` Andrew Fish
@ 2019-10-11 16:33 ` Laszlo Ersek
2019-10-11 19:07 ` Andrew Fish
0 siblings, 1 reply; 10+ messages in thread
From: Laszlo Ersek @ 2019-10-11 16:33 UTC (permalink / raw)
To: Andrew Fish, devel; +Cc: liming.gao, Pete Batard
Hi Andrew,
On 10/11/19 15:13, Andrew Fish wrote:
> I'm with Pete on this as my expectation would be the progress bar
> completes when you are done. I'd also point out that a common progress
> bar UI implementation is to show the area that is going to be updated.
> I don't think our UI does that, but it could get updated at some
> point. Thus I would posit that if PlatformBootManagerWaitCallback() is
> called by passing in PcdPlatformBootTimeOut you should always call
> PlatformBootManagerWaitCallback() with zero to terminate the progress
> bar.
OK.
> What makes sense to me is to skip the UI if PcdPlatformBootTimeOut is
> zero. The PlatformBootManagerWaitCallback (0) is only really needed if
> PlatformBootManagerWaitCallback (0) has not been called (TimeoutRemain
> != 0).
Let me paste your patch below, formatted with the
"--ignore-space-change" option (it hides changes due to simple
re-indentation of code):
> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> index 7968a58f3454..fd551d17e15d 100644
> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
> @@ -310,48 +310,52 @@ VOID
> BdsWait (
> IN EFI_EVENT HotkeyTriggered
> )
> {
> EFI_STATUS Status;
> UINT16 TimeoutRemain;
>
> DEBUG ((EFI_D_INFO, "[Bds]BdsWait ...Zzzzzzzzzzzz...\n"));
>
> TimeoutRemain = PcdGet16 (PcdPlatformBootTimeOut);
> + if (TimeoutRemain != 0) {
> while (TimeoutRemain != 0) {
> DEBUG ((EFI_D_INFO, "[Bds]BdsWait(%d)..Zzzz...\n", (UINTN) TimeoutRemain));
> PlatformBootManagerWaitCallback (TimeoutRemain);
>
> BdsReadKeys (); // BUGBUG: Only reading can signal HotkeyTriggered
> // Can be removed after all keyboard drivers invoke callback in timer callback.
>
> if (HotkeyTriggered != NULL) {
> Status = BdsWaitForSingleEvent (HotkeyTriggered, EFI_TIMER_PERIOD_SECONDS (1));
> if (!EFI_ERROR (Status)) {
> break;
> }
> } else {
> gBS->Stall (1000000);
> }
>
> //
> // 0xffff means waiting forever
> // BDS with no hotkey provided and 0xffff as timeout will "hang" in the loop
> //
> if (TimeoutRemain != 0xffff) {
> TimeoutRemain--;
> }
> }
> + if (TimeoutRemain != 0) {
> PlatformBootManagerWaitCallback (0);
> + }
> + }
> DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
> }
This would solve the (PcdPlatformBootTimeOut==0) case well.
However, it would regress Pete's use case. Namely, if you allow the loop
to time out (the "while" statement sees a zero "TimeoutRemain"), then
the final PlatformBootManagerWaitCallback (0) is skipped.
And that's the problem Pete intended to fix in the first place: when the
loop times out, PlatformBootManagerWaitCallback (0) is *never* called,
inside the loop. So we start booting e.g. an OS, without the progress
bar filling the screen to the right edge.
I think if we eliminate the *inner* "if" statement -- call
PlatformBootManagerWaitCallback (0) unconditionally --, and preserve the
outer "if" statement, it should work.
Thanks
Laszlo
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [edk2-devel] OVMF is crashing for me in master
2019-10-11 16:33 ` Laszlo Ersek
@ 2019-10-11 19:07 ` Andrew Fish
0 siblings, 0 replies; 10+ messages in thread
From: Andrew Fish @ 2019-10-11 19:07 UTC (permalink / raw)
To: devel, lersek; +Cc: liming.gao, Pete Batard
> On Oct 11, 2019, at 9:33 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>
> Hi Andrew,
>
> On 10/11/19 15:13, Andrew Fish wrote:
>
>> I'm with Pete on this as my expectation would be the progress bar
>> completes when you are done. I'd also point out that a common progress
>> bar UI implementation is to show the area that is going to be updated.
>> I don't think our UI does that, but it could get updated at some
>> point. Thus I would posit that if PlatformBootManagerWaitCallback() is
>> called by passing in PcdPlatformBootTimeOut you should always call
>> PlatformBootManagerWaitCallback() with zero to terminate the progress
>> bar.
>
> OK.
>
>> What makes sense to me is to skip the UI if PcdPlatformBootTimeOut is
>> zero. The PlatformBootManagerWaitCallback (0) is only really needed if
>> PlatformBootManagerWaitCallback (0) has not been called (TimeoutRemain
>> != 0).
>
> Let me paste your patch below, formatted with the
> "--ignore-space-change" option (it hides changes due to simple
> re-indentation of code):
>
>> diff --git a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> index 7968a58f3454..fd551d17e15d 100644
>> --- a/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> +++ b/MdeModulePkg/Universal/BdsDxe/BdsEntry.c
>> @@ -310,48 +310,52 @@ VOID
>> BdsWait (
>> IN EFI_EVENT HotkeyTriggered
>> )
>> {
>> EFI_STATUS Status;
>> UINT16 TimeoutRemain;
>>
>> DEBUG ((EFI_D_INFO, "[Bds]BdsWait ...Zzzzzzzzzzzz...\n"));
>>
>> TimeoutRemain = PcdGet16 (PcdPlatformBootTimeOut);
>> + if (TimeoutRemain != 0) {
>> while (TimeoutRemain != 0) {
>> DEBUG ((EFI_D_INFO, "[Bds]BdsWait(%d)..Zzzz...\n", (UINTN) TimeoutRemain));
>> PlatformBootManagerWaitCallback (TimeoutRemain);
>>
>> BdsReadKeys (); // BUGBUG: Only reading can signal HotkeyTriggered
>> // Can be removed after all keyboard drivers invoke callback in timer callback.
>>
>> if (HotkeyTriggered != NULL) {
>> Status = BdsWaitForSingleEvent (HotkeyTriggered, EFI_TIMER_PERIOD_SECONDS (1));
>> if (!EFI_ERROR (Status)) {
>> break;
>> }
>> } else {
>> gBS->Stall (1000000);
>> }
>>
>> //
>> // 0xffff means waiting forever
>> // BDS with no hotkey provided and 0xffff as timeout will "hang" in the loop
>> //
>> if (TimeoutRemain != 0xffff) {
>> TimeoutRemain--;
>> }
>> }
>> + if (TimeoutRemain != 0) {
>> PlatformBootManagerWaitCallback (0);
>> + }
>> + }
>> DEBUG ((EFI_D_INFO, "[Bds]Exit the waiting!\n"));
>> }
>
> This would solve the (PcdPlatformBootTimeOut==0) case well.
>
> However, it would regress Pete's use case. Namely, if you allow the loop
> to time out (the "while" statement sees a zero "TimeoutRemain"), then
> the final PlatformBootManagerWaitCallback (0) is skipped.
>
> And that's the problem Pete intended to fix in the first place: when the
> loop times out, PlatformBootManagerWaitCallback (0) is *never* called,
> inside the loop. So we start booting e.g. an OS, without the progress
> bar filling the screen to the right edge.
>
> I think if we eliminate the *inner* "if" statement -- call
> PlatformBootManagerWaitCallback (0) unconditionally --, and preserve the
> outer "if" statement, it should work.
>
Laszlo,
Good point. I was trying to prevent 2 calls with 0 but now I notice that is not possible in the while loop. So I agree with your suggestion. I also like Pete's idea of adding the error check in PlatformBootManagerWaitCallback().
Thanks,
Andrew Fish
> Thanks
> Laszlo
>
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-10-11 19:07 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-10-10 21:12 OVMF is crashing for me in master Andrew Fish
2019-10-11 1:19 ` [edk2-devel] " Liming Gao
2019-10-11 4:59 ` Andrew Fish
2019-10-11 11:53 ` Laszlo Ersek
2019-10-11 12:49 ` Andrew Fish
2019-10-11 11:50 ` Laszlo Ersek
2019-10-11 12:16 ` Pete Batard
2019-10-11 13:13 ` Andrew Fish
2019-10-11 16:33 ` Laszlo Ersek
2019-10-11 19:07 ` Andrew Fish
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox