From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 23B757803CC for ; Fri, 8 Sep 2023 02:45:27 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=j7rrQVfnRS/o1vMSkkDt3XIW4u+c1csAvuwDMiaBdpo=; c=relaxed/simple; d=groups.io; h=MIME-Version:References:In-Reply-To:From:Date:Message-ID:Subject:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Transfer-Encoding; s=20140610; t=1694141126; v=1; b=RPfdSwAUCSofQJO30LxB+MLQF6QzveTjJnhOjFWLFBcuYmGuRSIqXOtIFBPTwpO+2uhhayex befjVH2u16jq6My37pyjtSYkUMRUmkZhOBWiefq5qJNfFKyQRDTG7pV2gZDzAPVmxaTWhxxY6tD PoCnCkcb9Nigzt99VBgZzlPo= X-Received: by 127.0.0.2 with SMTP id 8q9ZYY7687511x7gIqea3eHY; Thu, 07 Sep 2023 19:45:26 -0700 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web10.31427.1694141126000406894 for ; Thu, 07 Sep 2023 19:45:26 -0700 X-Received: from mail-ed1-f69.google.com (mail-ed1-f69.google.com [209.85.208.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-83-fZklJTj0MBySch1_vNU5jQ-1; Thu, 07 Sep 2023 22:45:21 -0400 X-MC-Unique: fZklJTj0MBySch1_vNU5jQ-1 X-Received: by mail-ed1-f69.google.com with SMTP id 4fb4d7f45d1cf-52c05552645so1106690a12.3 for ; Thu, 07 Sep 2023 19:45:21 -0700 (PDT) X-Gm-Message-State: XOOGGM05ZPBsv6cPxD6AQnn2x7686176AA= X-Received: by 2002:a17:906:1d8:b0:9a9:e393:8bd6 with SMTP id 24-20020a17090601d800b009a9e3938bd6mr894505ejj.38.1694141120552; Thu, 07 Sep 2023 19:45:20 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFlTXy40nMJPXEdsH09l5+ebSFgNcv1sue2T4zlKcVsmrcsEJq57Tgk42eUimqH2LMgpgbmBTgmKxorchwgk8w= X-Received: by 2002:a17:906:1d8:b0:9a9:e393:8bd6 with SMTP id 24-20020a17090601d800b009a9e3938bd6mr894490ejj.38.1694141120213; Thu, 07 Sep 2023 19:45:20 -0700 (PDT) MIME-Version: 1.0 References: <20230907111117.3559823-1-zhenyzha@redhat.com> In-Reply-To: From: "Zhenyu Zhang" Date: Fri, 8 Sep 2023 10:44:44 +0800 Message-ID: Subject: Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg:Add variable store is full debug info To: Oliver Steffen Cc: "Yao, Jiewen" , "devel@edk2.groups.io" , "kraxel@redhat.com" , "marcandre.lureau@redhat.com" , "stefanb@linux.ibm.com" , "anthony.perard@citrix.com" , "julien@xen.org" X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,zhenyzha@redhat.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=RPfdSwAU; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=redhat.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io > My suggestion is to wrap the newly added DEBUG statement with an if: > > if (OptionIndex =3D=3D -1) { > Status =3D EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_UINTN= ); > + if (Status =3D=3D 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. Agreed, I'll add it in the next patch. > > Why not return ERROR? > > > This is in VOID PlatformRegisterFvBootOption (), returning ERROR is not > possible. Indeed so. Best wishes, Zhenyu On Thu, Sep 7, 2023 at 9:25=E2=80=AFPM Oliver Steffen = wrote: > > 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 =3D=3D -1) { > Status =3D EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_UINT= N); > + if (Status =3D=3D 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 On Behalf Of Zhenyu > > > Zhang > > > Sent: Thursday, September 7, 2023 7:11 PM > > > To: devel@edk2.groups.io > > > Cc: zhenyzha@redhat.com; osteffen@redhat.com; kraxel@redhat.com; > > > marcandre.lureau@redhat.com; stefanb@linux.ibm.com; > > > anthony.perard@citrix.com; julien@xen.org > > > Subject: [edk2-devel] [PATCH v2 1/1] OvmfPkg:Add variable store is fu= ll debug > > > info > > > > > > From: "Zhenyu Zhang" > > > > > > 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 =3D 0x3FF9C - CommonVariableTotalSize =3D > > > 0x3FF98 > > > RecordVarErrorFlag (0xEF) 9A144FE2A47E:937FE521-95AE-4D1A-8929- > > > 48BCD90AD31A - 0x00000003 - 0x92 > > > CommonVariableSpace =3D 0x3FF9C - CommonVariableTotalSize =3D 0x3FF9= 8 > > > > > > Synchronous Exception at 0x000000023CA60374 > > > ...... > > > ASSERT_EFI_ERROR (Status =3D Out of Resources) > > > ASSERT /builddir/build/BUILD/edk2-ba91d0292e59/OvmfPkg/Library/ > > > PlatformBootManagerLib/BdsPlatform.c(142): !(((INTN)(RETURN_ > > > STATUS)(Status)) < 0) > > > > > > Cc: Oliver Steffen > > > Cc: Gerd Hoffmann > > > Cc: Marc-Andr=C3=A9 Lureau > > > Cc: Stefan Berger > > > Cc: Anthony Perard > > > Cc: Julien Grall > > > Signed-off-by: Zhenyu Zhang > > > --- > > > 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 =3D=3D -1) { > > > Status =3D EfiBootManagerAddLoadOptionVariable (&NewOption, MAX_= UINTN); > > > + DEBUG ((DEBUG_ERROR, "ERROR: Variable store is full.\n")); > > > ASSERT_EFI_ERROR (Status); > > > } > > > > > > -- > > > 2.39.3 > > > > -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#108438): https://edk2.groups.io/g/devel/message/108438 Mute This Topic: https://groups.io/mt/101211889/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-