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 4339D7803DF for ; Thu, 7 Sep 2023 13:25:03 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=CjlTz+ybY67ckvkc4w5HdGtBxBcJf/rbQqjcBtUqbYg=; c=relaxed/simple; d=groups.io; h=MIME-Version:In-Reply-To:References:From:User-Agent: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=1694093102; v=1; b=MqWt3IbbrVxNL9tmAooLAFP83V/f8rEnATgwQXyQAulph8BFEPSGQfCTc/3S9E7KjKrreG0f cApXJl9KMebCm4aY4Gn6bxrUvS2ALw/rsNrbLKab9TuCJJS+zBsbLqq51+AhZ/HmtcnpspD9r9u XRncRRZE74abeRa26TKJjDh4= X-Received: by 127.0.0.2 with SMTP id Qp4yYY7687511xOJJFeV8SFM; Thu, 07 Sep 2023 06:25:02 -0700 X-Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web10.12734.1694093101146920788 for ; Thu, 07 Sep 2023 06:25:01 -0700 X-Received: from mail-lf1-f70.google.com (mail-lf1-f70.google.com [209.85.167.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-538-P_HStOBGMiOUJjFCjOJC4w-1; Thu, 07 Sep 2023 09:24:59 -0400 X-MC-Unique: P_HStOBGMiOUJjFCjOJC4w-1 X-Received: by mail-lf1-f70.google.com with SMTP id 2adb3069b0e04-500af1b3b20so1011481e87.3 for ; Thu, 07 Sep 2023 06:24:58 -0700 (PDT) X-Gm-Message-State: leC6b4ZudWKZGKIbk4JHA4Ubx7686176AA= X-Received: by 2002:a05:6512:239d:b0:500:7e64:cff1 with SMTP id c29-20020a056512239d00b005007e64cff1mr6209925lfv.14.1694093097605; Thu, 07 Sep 2023 06:24:57 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFZhmyUCHnEtyvCQV3B2ejgd/gWTe14J1KhsAn7cgAF4ncq3ZwQVU4QK2xOUQ12RhoK4FfvKpO7N9tBT9KLs9s= X-Received: by 2002:a05:6512:239d:b0:500:7e64:cff1 with SMTP id c29-20020a056512239d00b005007e64cff1mr6209906lfv.14.1694093097224; Thu, 07 Sep 2023 06:24:57 -0700 (PDT) X-Received: from 567203818698 named unknown by gmailapi.google.com with HTTPREST; Thu, 7 Sep 2023 06:24:56 -0700 MIME-Version: 1.0 In-Reply-To: References: <20230907111117.3559823-1-zhenyzha@redhat.com> From: "Oliver Steffen" User-Agent: alot/0.8.1 Date: Thu, 7 Sep 2023 06:24:56 -0700 Message-ID: Subject: Re: [edk2-devel] [PATCH v2 1/1] OvmfPkg:Add variable store is full debug info To: "Yao, Jiewen" , "devel@edk2.groups.io" , "zhenyzha@redhat.com" Cc: "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,osteffen@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=MqWt3Ibb; 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 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_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. > 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 full= 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 0x3FF98 > > > > 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_UI= NTN); > > + 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 (#108387): https://edk2.groups.io/g/devel/message/108387 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-