[edk2-devel] [PATCH v3 1/1] OvmfPkg:Add variable store is full debug info

[Zhenyu Zhang]

> I don't think a full variable store is the only reason we might end up
> with EFI_OUT_OF_RESOURCES here, so this message could be misleading.
>
> Given that this is a DEBUG build, I'd expect the developer to be able
> to infer from the context that this might be either an out of memory
> or an out of flash space condition, and the extra DEBUG message is
> kind of redundant.

Sorry for the late reply, you are right.
Yes, there may be different situations here.
It will be clearer if we can distinguish between out of memory
or out of flash space.

> OTOH, running out of flash space is a serious condition, so I wouldn't
> object to adding better diagnostics for this - they just belong
> somewhere else (i.e., in the SetVariable() code)

Sorry in SetVariable(), how can we tell that there is insufficient
flash storage?
I noticed there is a similar method in NorFlashKvmtool.c,
but I'm not sure it's what you're talking about.

  if (FlashRegion > (FlashDevice->DeviceBaseAddress + FlashDevice->Size)) {
    DEBUG ((DEBUG_ERROR, "Insufficient flash storage size\n"));
    return EFI_OUT_OF_RESOURCES;
  }


On Wed, Sep 13, 2023 at 4:54 PM Ard Biesheuvel <ardb at kernel.org> wrote:
>
> On Mon, 11 Sept 2023 at 04:47, Zhenyu Zhang <zhenyzha at redhat.com> wrote:
> >
> > From: "Zhenyu Zhang" <zhenyzha at redhat.com>
> >
> > 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.
> > Add debug information to help users understand what caused
> > the assertion.
> >
> >  Actual results:
> >  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 <osteffen at redhat.com>
> > Cc: Gerd Hoffmann <ghoffman at redhat.com>
> > Cc: Yao Jiewen <jiewen.yao at intel.com>
> > Cc: Marc-André Lureau <marcandre.lureau at redhat.com>
> > Cc: Stefan Berger <stefanb at linux.ibm.com>
> > Cc: Anthony Perard <anthony.perard at citrix.com>
> > Cc: Julien Grall <julien at xen.org>
> > Signed-off-by: Zhenyu Zhang <zhenyzha at redhat.com>
> > ---
> >  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> > index 8dc2bbf97371..fc2d64d69207 100644
> > --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> > +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> > @@ -139,6 +139,9 @@ PlatformRegisterFvBootOption (
> >
> >    if (OptionIndex == -1) {
> >      Status = EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_UINTN);
> > +    if (Status == EFI_OUT_OF_RESOURCES) {
> > +      DEBUG ((DEBUG_ERROR, "ERROR: Variable store is full.\n"));
>
> I don't think a full variable store is the only reason we might end up
> with EFI_OUT_OF_RESOURCES here, so this message could be misleading.
>
> Given that this is a DEBUG build, I'd expect the developer to be able
> to infer from the context that this might be either an out of memory
> or an out of flash space condition, and the extra DEBUG message is
> kind of redundant.
>
> OTOH, running out of flash space is a serious condition, so I wouldn't
> object to adding better diagnostics for this - they just belong
> somewhere else (i.e., in the SetVariable() code)
>
>
> > +    }
> >      ASSERT_EFI_ERROR (Status);
> >    }
> >
> > --
> > 2.39.3
> >
>
>
> 
>
>



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#108903): https://edk2.groups.io/g/devel/message/108903
Mute This Topic: https://groups.io/mt/101285762/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-