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 8AB86AC147F for ; Wed, 2 Aug 2023 18:59:35 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=yV8KAsNwYQOBX/v2HYQmNHsdTomNqKCzYTwZmzEjFis=; c=relaxed/simple; d=groups.io; h=DKIM-Filter:From:To:Cc:Subject:Date:Message-Id:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20140610; t=1691002774; v=1; b=e7ti2YDZEXINy4lCBl+p68nJSnPJGnP6eYs+M0Uampq/MjkLvjNUK8J81/DcZ+KPEu16PyhZ Zw6m8roWRLaVxMnKHN/jslAjvxx+FC9O27MpW2Vzez5UIxJR9lMHOCWWJ5zwkbv7IiolOFzDWne 1e+BfvWgYpE6NdqAQq16R3Cg= X-Received: by 127.0.0.2 with SMTP id wpKQYY7687511xXJhrftVqNa; Wed, 02 Aug 2023 11:59:34 -0700 X-Received: from linux.microsoft.com (linux.microsoft.com [13.77.154.182]) by mx.groups.io with SMTP id smtpd.web10.4628.1691002773541618533 for ; Wed, 02 Aug 2023 11:59:33 -0700 X-Received: from OSD-Desktop.redmond.corp.microsoft.com (unknown [131.107.1.171]) by linux.microsoft.com (Postfix) with ESMTPSA id E75E8238C439; Wed, 2 Aug 2023 11:59:32 -0700 (PDT) DKIM-Filter: OpenDKIM Filter v2.11.0 linux.microsoft.com E75E8238C439 From: "Oliver Smith-Denny" To: devel@edk2.groups.io Cc: Leif Lindholm , Ard Biesheuvel , Sami Mujawar , Gerd Hoffmann Subject: [edk2-devel][PATCH v2 1/1] ArmPkg: DefaultExceptionHandlerLib: Do Not Allocate Memory Date: Wed, 2 Aug 2023 11:59:28 -0700 Message-Id: <20230802185928.18773-1-osde@linux.microsoft.com> MIME-Version: 1.0 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,osde@linux.microsoft.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: 5pqWBaYNEYBBFra0ONvUapKSx7686176AA= 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=e7ti2YDZ; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=linux.microsoft.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 Changes since v1:=0D - Move OutputString write to after serial logging=0D - Drop sending "Recursive Exception" to ConOut=0D =0D If gST->ConOut is available when Arm's DefaultExceptionHandler=0D is running, AsciiPrint will get called to attempt to print to=0D ConOut, in addition to the serial output.=0D =0D AsciiPrint calls AsciiInternalPrint in UefiLibPrint.c which=0D in turn calls AllocatePool to allocate a buffer to convert=0D the Ascii input string to a Unicode string to pass to=0D ConOut->OutputString.=0D =0D Per the comment on DefaultExceptionHandler, we should not be=0D allocating memory in the exception handler, as this can cause=0D the exception handler to fail if we had a memory exception or=0D the system state is such that we cannot allocate memory.=0D =0D It has been observed on ArmVirtQemu that exceptions generated=0D in the memory handling code will fail to output the stack dump=0D and CPU state that is critical to debugging because the=0D AllocatePool will fail.=0D =0D This patch fixes the Arm and AARCH64 DefaultExceptionHandlers to=0D not allocate memory when ConOut is available and instead use=0D stack memory to convert the Ascii string needed for SerialPortWrite=0D to the Unicode string needed for ConOut->OutputString. Correspondingly,=0D ArmVirtQemu can now output the stack dump and CPU state when hitting=0D an exception in memory code.=0D =0D GitHub PR: https://github.com/tianocore/edk2/pull/4713=0D =0D Cc: Leif Lindholm =0D Cc: Ard Biesheuvel =0D Cc: Sami Mujawar =0D Cc: Gerd Hoffmann =0D =0D Signed-off-by: Oliver Smith-Denny =0D ---=0D ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.= c | 30 +++++++++++++-------=0D ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler.c = | 22 +++++++++++---=0D 2 files changed, 37 insertions(+), 15 deletions(-)=0D =0D diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExcep= tionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultEx= ceptionHandler.c=0D index f2bca5d74005..f4967dabed14 100644=0D --- a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHan= dler.c=0D +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHan= dler.c=0D @@ -22,6 +22,11 @@=0D #include =0D #include =0D =0D +//=0D +// Maximum number of characters to print to serial (UINT8s) and to console= if available (as UINT16s)=0D +//=0D +#define MAX_PRINT_CHARS 100=0D +=0D STATIC CHAR8 *gExceptionTypeString[] =3D {=0D "Synchronous",=0D "IRQ",=0D @@ -188,18 +193,14 @@ DefaultExceptionHandler (=0D IN OUT EFI_SYSTEM_CONTEXT SystemContext=0D )=0D {=0D - CHAR8 Buffer[100];=0D - UINTN CharCount;=0D - INT32 Offset;=0D + CHAR8 Buffer[MAX_PRINT_CHARS];=0D + CHAR16 UnicodeBuffer[MAX_PRINT_CHARS];=0D + UINTN CharCount;=0D + INT32 Offset;=0D =0D if (mRecursiveException) {=0D STATIC CHAR8 CONST Message[] =3D "\nRecursive exception occurred whil= e dumping the CPU state\n";=0D -=0D SerialPortWrite ((UINT8 *)Message, sizeof Message - 1);=0D - if (gST->ConOut !=3D NULL) {=0D - AsciiPrint (Message);=0D - }=0D -=0D CpuDeadLoop ();=0D }=0D =0D @@ -207,9 +208,9 @@ DefaultExceptionHandler (=0D =0D CharCount =3D AsciiSPrint (Buffer, sizeof (Buffer), "\n\n%a Exception at= 0x%016lx\n", gExceptionTypeString[ExceptionType], SystemContext.SystemCont= extAArch64->ELR);=0D SerialPortWrite ((UINT8 *)Buffer, CharCount);=0D - if (gST->ConOut !=3D NULL) {=0D - AsciiPrint (Buffer);=0D - }=0D +=0D + // prepare a unicode buffer for ConOut, if applicable, in case the buffe= r gets reused=0D + UnicodeSPrintAsciiFormat (UnicodeBuffer, MAX_PRINT_CHARS, Buffer);=0D =0D DEBUG_CODE_BEGIN ();=0D CHAR8 *Pdb, *PrevPdb;=0D @@ -330,6 +331,13 @@ DefaultExceptionHandler (=0D ));=0D }=0D =0D + // attempt to print that we had a synchronous exception to ConOut=0D + // we do this after the serial logging as ConOut's logging is more=0D + // complex and we aren't guaranteed to succeed=0D + if (gST->ConOut !=3D NULL) {=0D + gST->ConOut->OutputString (gST->ConOut, UnicodeBuffer);=0D + }=0D +=0D ASSERT (FALSE);=0D CpuDeadLoop ();=0D }=0D diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultException= Handler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionH= andler.c=0D index 13b321e45615..f5fce4c4b516 100644=0D --- a/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler= .c=0D +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/Arm/DefaultExceptionHandler= .c=0D @@ -23,6 +23,11 @@=0D #include =0D #include =0D =0D +//=0D +// Maximum number of characters to print to serial (UINT8s) and to console= if available (as UINT16s)=0D +//=0D +#define MAX_PRINT_CHARS 100=0D +=0D //=0D // The number of elements in a CHAR8 array, including the terminating NUL,= that=0D // is meant to hold the string rendering of the CPSR.=0D @@ -198,7 +203,8 @@ DefaultExceptionHandler (=0D IN OUT EFI_SYSTEM_CONTEXT SystemContext=0D )=0D {=0D - CHAR8 Buffer[100];=0D + CHAR8 Buffer[MAX_PRINT_CHARS];=0D + CHAR16 UnicodeBuffer[MAX_PRINT_CHARS];=0D UINTN CharCount;=0D UINT32 DfsrStatus;=0D UINT32 IfsrStatus;=0D @@ -216,9 +222,9 @@ DefaultExceptionHandler (=0D SystemContext.SystemContextArm->CPSR=0D );=0D SerialPortWrite ((UINT8 *)Buffer, CharCount);=0D - if (gST->ConOut !=3D NULL) {=0D - AsciiPrint (Buffer);=0D - }=0D +=0D + // prepare a unicode buffer for ConOut, if applicable, as Buffer is used= below=0D + UnicodeSPrintAsciiFormat (UnicodeBuffer, MAX_PRINT_CHARS, Buffer);=0D =0D DEBUG_CODE_BEGIN ();=0D CHAR8 *Pdb;=0D @@ -289,6 +295,14 @@ DefaultExceptionHandler (=0D }=0D =0D DEBUG ((DEBUG_ERROR, "\n"));=0D +=0D + // attempt to print that we had a synchronous exception to ConOut=0D + // we do this after the serial logging as ConOut's logging is more=0D + // complex and we aren't guaranteed to succeed=0D + if (gST->ConOut !=3D NULL) {=0D + gST->ConOut->OutputString (gST->ConOut, UnicodeBuffer);=0D + }=0D +=0D ASSERT (FALSE);=0D =0D CpuDeadLoop (); // may return if executing under a debugger=0D -- =0D 2.40.1=0D =0D -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#107469): https://edk2.groups.io/g/devel/message/107469 Mute This Topic: https://groups.io/mt/100512229/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-