Quoting Yao, Jiewen (2023-09-07 14:26:31) > I don't think using ASSERT is a good idea here. >
Some context: We observed that EDK2 hits an ASSERT (Out of Resources) when booting with a full variable store. The message provided in this case is not helpful for non-experts. I think the goal here is to emit a more user friendly message. My suggestion is to wrap the newly added DEBUG statement with an if: if (OptionIndex == -1) { Status = EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_UINTN); + if (Status == EFI_OUT_OF_RESOURCES) { + DEBUG ((DEBUG_ERROR, "ERROR: Variable store is full.\n")); + } ASSERT_EFI_ERROR (Status); } The ASSERT_EFI_ERROR was already there. It will halt the execution in case Status contains an error. Aparently the firmware is not able to continue with a full variable store. > Why not return ERROR? > This is in VOID PlatformRegisterFvBootOption (), returning ERROR is not possible. Thanks, Oliver > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zhenyu > > Zhang > > Sent: Thursday, September 7, 2023 7:11 PM > > To: devel@edk2.groups.io > > Cc: zheny...@redhat.com; ostef...@redhat.com; kra...@redhat.com; > > marcandre.lur...@redhat.com; stef...@linux.ibm.com; > > anthony.per...@citrix.com; jul...@xen.org > > Subject: [edk2-devel] [PATCH v2 1/1] OvmfPkg:Add variable store is full > > debug > > info > > > > From: "Zhenyu Zhang" <zheny...@redhat.com> > > > > When the variable store is full, edk2 will directly assert. > > Add debug information to help users understand what caused > > the assertion. > > > > Actual results: > > RecordVarErrorFlag (0xEF) 9A144FE2A47E:937FE521-95AE-4D1A-8929- > > 48BCD90AD31A - 0x00000003 - 0x7E > > CommonVariableSpace = 0x3FF9C - CommonVariableTotalSize = > > 0x3FF98 > > RecordVarErrorFlag (0xEF) 9A144FE2A47E:937FE521-95AE-4D1A-8929- > > 48BCD90AD31A - 0x00000003 - 0x92 > > CommonVariableSpace = 0x3FF9C - CommonVariableTotalSize = 0x3FF98 > > > > Synchronous Exception at 0x000000023CA60374 > > ...... > > ASSERT_EFI_ERROR (Status = Out of Resources) > > ASSERT /builddir/build/BUILD/edk2-ba91d0292e59/OvmfPkg/Library/ > > PlatformBootManagerLib/BdsPlatform.c(142): !(((INTN)(RETURN_ > > STATUS)(Status)) < 0) > > > > Cc: Oliver Steffen <ostef...@redhat.com> > > Cc: Gerd Hoffmann <kra...@redhat.com> > > Cc: Marc-André Lureau <marcandre.lur...@redhat.com> > > Cc: Stefan Berger <stef...@linux.ibm.com> > > Cc: Anthony Perard <anthony.per...@citrix.com> > > Cc: Julien Grall <jul...@xen.org> > > Signed-off-by: Zhenyu Zhang <zheny...@redhat.com> > > --- > > OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > > b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > > index 8dc2bbf97371..c95c7931a3f5 100644 > > --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > > +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c > > @@ -139,6 +139,7 @@ PlatformRegisterFvBootOption ( > > > > if (OptionIndex == -1) { > > Status = EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_UINTN); > > + DEBUG ((DEBUG_ERROR, "ERROR: Variable store is full.\n")); > > ASSERT_EFI_ERROR (Status); > > } > > > > -- > > 2.39.3 > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108387): https://edk2.groups.io/g/devel/message/108387 Mute This Topic: https://groups.io/mt/101211889/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-