From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.120; helo=mga04.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 27E6C211E00F0 for ; Thu, 21 Mar 2019 18:50:42 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Mar 2019 18:50:42 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,255,1549958400"; d="scan'208";a="154613143" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by fmsmga004.fm.intel.com with ESMTP; 21 Mar 2019 18:50:42 -0700 Received: from FMSMSX110.amr.corp.intel.com (10.18.116.10) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 21 Mar 2019 18:50:42 -0700 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx110.amr.corp.intel.com (10.18.116.10) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 21 Mar 2019 18:50:41 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.74]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.158]) with mapi id 14.03.0415.000; Fri, 22 Mar 2019 09:50:38 +0800 From: "Wu, Hao A" To: "Gao, Zhichao" , "edk2-devel@lists.01.org" CC: "Wang, Jian J" , "Ni, Ray" , "Zeng, Star" , "Gao, Liming" , Sean Brogan , Michael Turner , Bret Barkelew Thread-Topic: [PATCH V4 13/17] MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add new APIs Thread-Index: AQHU3+9MmDnUNosxFE6JK92l2k4XMKYW4muw Date: Fri, 22 Mar 2019 01:50:38 +0000 Message-ID: References: <20190321140459.18304-1-zhichao.gao@intel.com> <20190321140459.18304-14-zhichao.gao@intel.com> In-Reply-To: <20190321140459.18304-14-zhichao.gao@intel.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH V4 13/17] MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add new APIs X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 22 Mar 2019 01:50:43 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Gao, Zhichao > Sent: Thursday, March 21, 2019 10:05 PM > To: edk2-devel@lists.01.org > Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Zeng, Star; Gao, Liming; Sean Broga= n; > Michael Turner; Bret Barkelew > Subject: [PATCH V4 13/17] MdeModulePkg/PeiDxeDebugLibReportStatusCode: > Add new APIs >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1395 >=20 > Add new APIs' implementation (DebugVPrint, DebugBPrint) > in the DebugLib instance. These APIs would expose print > routines with VaList parameter and BaseList parameter. >=20 > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Zhichao Gao > Cc: Jian J Wang > Cc: Hao Wu > Cc: Ray Ni > Cc: Star Zeng > Cc: Liming Gao > Cc: Sean Brogan > Cc: Michael Turner > Cc: Bret Barkelew > --- > .../PeiDxeDebugLibReportStatusCode/DebugLib.c | 171 > +++++++++++++++++---- > 1 file changed, 144 insertions(+), 27 deletions(-) >=20 > diff --git > a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c > b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c > index 6f0f416273..d593752050 100644 > --- a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c > +++ b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c > @@ -4,7 +4,7 @@ > Note that if the debug message length is larger than the maximum allow= able > record length, then the debug message will be ignored directly. >=20 > - Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
> + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the = BSD > License > which accompanies this distribution. The full text of the license may= be found > at > @@ -27,6 +27,12 @@ > #include > #include >=20 > +// > +// VA_LIST can not initialize to NULL for all compiler, so we use this t= o > +// indicate a null VA_LIST > +// > +VA_LIST mVaListNull; > + > /** > Prints a debug message to the debug output device if the specified err= or level > is enabled. >=20 > @@ -52,12 +58,48 @@ DebugPrint ( > IN CONST CHAR8 *Format, > ... > ) > +{ > + VA_LIST Marker; > + > + VA_START (Marker, Format); > + DebugVPrint (ErrorLevel, Format, Marker); > + VA_END (Marker); > +} > + > +/** > + Prints a debug message to the debug output device if the specified > + error level is enabled base on Null-terminated format string and a > + VA_LIST argument list or a BASE_LIST argument list. > + > + If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib functi= on > + GetDebugPrintErrorLevel (), then print the message specified by Format= and > + the associated variable argument list to the debug output device. > + > + Only one list type is used. > + If BaseListMarker =3D=3D NULL, then use VaListMarker. > + Otherwise use BaseListMarker and the VaListMarker should be initilized= as > + mVaListNull. > + > + If Format is NULL, then ASSERT(). > + > + @param ErrorLevel The error level of the debug message. > + @param Format Format string for the debug message to print. > + @param VaListMarker VA_LIST marker for the variable argument list. > + @param BaseListMarker BASE_LIST marker for the variable argument lis= t. > + > +**/ > +VOID > +DebugPrintMarker ( > + IN UINTN ErrorLevel, > + IN CONST CHAR8 *Format, > + IN VA_LIST VaListMarker, > + IN BASE_LIST BaseListMarker > + ) > { > UINT64 Buffer[(EFI_STATUS_CODE_DATA_MAX_SIZE / sizeof (UINT64= )) > + 1]; > EFI_DEBUG_INFO *DebugInfo; > UINTN TotalSize; > - VA_LIST VaListMarker; > - BASE_LIST BaseListMarker; > + BASE_LIST BaseListMarkerPointer; > CHAR8 *FormatString; > BOOLEAN Long; >=20 > @@ -78,22 +120,22 @@ DebugPrint ( > // Note that the passing-in format string and variable parameters will= be > constructed to > // the following layout: > // > - // Buffer->|------------------------| > - // | Padding | 4 bytes > - // DebugInfo->|------------------------| > - // | EFI_DEBUG_INFO | sizeof(EFI_DEBUG_INFO) > - // BaseListMarker->|------------------------| > - // | ... | > - // | variable arguments | 12 * sizeof (UINT64) > - // | ... | > - // |------------------------| > - // | Format String | > - // |------------------------|<- (UINT8 *)Buffer + size= of(Buffer) > + // Buffer->|------------------------| > + // | Padding | 4 bytes > + // DebugInfo->|------------------------| > + // | EFI_DEBUG_INFO | sizeof(EFI_DEBUG_= INFO) > + // BaseListMarkerPointer->|------------------------| > + // | ... | > + // | variable arguments | 12 * sizeof (UINT= 64) > + // | ... | > + // |------------------------| > + // | Format String | > + // |------------------------|<- (UINT8 *)Buffer= + sizeof(Buffer) > // > TotalSize =3D 4 + sizeof (EFI_DEBUG_INFO) + 12 * sizeof (UINT64) + Asc= iiStrSize > (Format); >=20 > // > - // If the TotalSize is larger than the maximum record size, then retur= n > + // If the TotalSize is larger than the maximum record size, then trunc= ate it. > // > if (TotalSize > sizeof (Buffer)) { > TotalSize =3D sizeof (Buffer); It seems to me that the below chunk of comment needs to be updated for the rename to the 'BaseListMarkerPointer' as well: // // Fill in EFI_DEBUG_INFO // // Here we skip the first 4 bytes of Buffer, because we must ensure BaseL= istMarker is // 64-bit aligned, otherwise retrieving 64-bit parameter from BaseListMar= ker will cause // exception on IPF. Buffer starts at 64-bit aligned address, so skipping= 4 types (sizeof(EFI_DEBUG_INFO)) // just makes address of BaseListMarker, which follows DebugInfo, 64-bit = aligned. // There is no need to re-send the patch again if there is no other major change in the patch series. And you can keep my previous 'Reviewed-by' tag given in V3 of the series. Best Regards, Hao Wu > @@ -109,7 +151,7 @@ DebugPrint ( > // > DebugInfo =3D (EFI_DEBUG_INFO *)(Buffer) + 1; > DebugInfo->ErrorLevel =3D (UINT32)ErrorLevel; > - BaseListMarker =3D (BASE_LIST)(DebugInfo + 1); > + BaseListMarkerPointer =3D (BASE_LIST)(DebugInfo + 1); > FormatString =3D (CHAR8 *)((UINT64 *)(DebugInfo + 1) + 12); >=20 > // > @@ -125,7 +167,6 @@ DebugPrint ( > // of format in DEBUG string, which is followed by the DEBUG format st= ring. > // Here we will process the variable arguments and pack them in this a= rea. > // > - VA_START (VaListMarker, Format); >=20 > // > // Use the actual format string. > @@ -167,7 +208,11 @@ DebugPrint ( > // '*' in format field means the precision of the field is speci= fied by > // a UINTN argument in the argument list. > // > - BASE_ARG (BaseListMarker, UINTN) =3D VA_ARG (VaListMarker, UINTN= ); > + if (BaseListMarker =3D=3D NULL) { > + BASE_ARG (BaseListMarkerPointer, UINTN) =3D VA_ARG (VaListMark= er, > UINTN); > + } else { > + BASE_ARG (BaseListMarkerPointer, UINTN) =3D BASE_ARG > (BaseListMarker, UINTN); > + } > continue; > } > if (*Format =3D=3D '\0') { > @@ -192,16 +237,36 @@ DebugPrint ( > } > if (*Format =3D=3D 'p' || *Format =3D=3D 'X' || *Format =3D=3D 'x' |= | *Format =3D=3D 'd' || > *Format =3D=3D 'u') { > if (Long) { > - BASE_ARG (BaseListMarker, INT64) =3D VA_ARG (VaListMarker, INT64= ); > + if (BaseListMarker =3D=3D NULL) { > + BASE_ARG (BaseListMarkerPointer, INT64) =3D VA_ARG (VaListMark= er, > INT64); > + } else { > + BASE_ARG (BaseListMarkerPointer, INT64) =3D BASE_ARG > (BaseListMarker, INT64); > + } > } else { > - BASE_ARG (BaseListMarker, int) =3D VA_ARG (VaListMarker, int); > + if (BaseListMarker =3D=3D NULL) { > + BASE_ARG (BaseListMarkerPointer, int) =3D VA_ARG (VaListMarker= , int); > + } else { > + BASE_ARG (BaseListMarkerPointer, int) =3D BASE_ARG (BaseListMa= rker, > int); > + } > } > } else if (*Format =3D=3D 's' || *Format =3D=3D 'S' || *Format =3D= =3D 'a' || *Format =3D=3D > 'g' || *Format =3D=3D 't') { > - BASE_ARG (BaseListMarker, VOID *) =3D VA_ARG (VaListMarker, VOID *= ); > + if (BaseListMarker =3D=3D NULL) { > + BASE_ARG (BaseListMarkerPointer, VOID *) =3D VA_ARG (VaListMarke= r, > VOID *); > + } else { > + BASE_ARG (BaseListMarkerPointer, VOID *) =3D BASE_ARG > (BaseListMarker, VOID *); > + } > } else if (*Format =3D=3D 'c') { > - BASE_ARG (BaseListMarker, UINTN) =3D VA_ARG (VaListMarker, UINTN); > + if (BaseListMarker =3D=3D NULL) { > + BASE_ARG (BaseListMarkerPointer, UINTN) =3D VA_ARG (VaListMarker= , > UINTN); > + } else { > + BASE_ARG (BaseListMarkerPointer, UINTN) =3D BASE_ARG (BaseListMa= rker, > UINTN); > + } > } else if (*Format =3D=3D 'r') { > - BASE_ARG (BaseListMarker, RETURN_STATUS) =3D VA_ARG (VaListMarker, > RETURN_STATUS); > + if (BaseListMarker =3D=3D NULL) { > + BASE_ARG (BaseListMarkerPointer, RETURN_STATUS) =3D VA_ARG > (VaListMarker, RETURN_STATUS); > + } else { > + BASE_ARG (BaseListMarkerPointer, RETURN_STATUS) =3D BASE_ARG > (BaseListMarker, RETURN_STATUS); > + } > } >=20 > // > @@ -209,17 +274,15 @@ DebugPrint ( > // This indicates that the DEBUG() macro is passing in more argument= than > can be handled by > // the EFI_DEBUG_INFO record > // > - ASSERT ((CHAR8 *)BaseListMarker <=3D FormatString); > + ASSERT ((CHAR8 *)BaseListMarkerPointer <=3D FormatString); >=20 > // > // If the converted BASE_LIST is larger than the 12 * sizeof (UINT64= ) > allocated bytes, then return > // > - if ((CHAR8 *)BaseListMarker > FormatString) { > - VA_END (VaListMarker); > + if ((CHAR8 *)BaseListMarkerPointer > FormatString) { > return; > } > } > - VA_END (VaListMarker); >=20 > // > // Send the DebugInfo record > @@ -235,6 +298,60 @@ DebugPrint ( > ); > } >=20 > +/** > + Prints a debug message to the debug output device if the specified > + error level is enabled. > + > + If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib functi= on > + GetDebugPrintErrorLevel (), then print the message specified by Format= and > + the associated variable argument list to the debug output device. > + > + If Format is NULL, then ASSERT(). > + > + @param ErrorLevel The error level of the debug message. > + @param Format Format string for the debug message to print. > + @param VaListMarker VA_LIST marker for the variable argument list. > + > +**/ > +VOID > +EFIAPI > +DebugVPrint ( > + IN UINTN ErrorLevel, > + IN CONST CHAR8 *Format, > + IN VA_LIST VaListMarker > + ) > +{ > + DebugPrintMarker (ErrorLevel, Format, VaListMarker, NULL); > +} > + > +/** > + Prints a debug message to the debug output device if the specified > + error level is enabled. > + This function use BASE_LIST which would provide a more compatible > + service than VA_LIST. > + > + If any bit in ErrorLevel is also set in DebugPrintErrorLevelLib functi= on > + GetDebugPrintErrorLevel (), then print the message specified by Format= and > + the associated variable argument list to the debug output device. > + > + If Format is NULL, then ASSERT(). > + > + @param ErrorLevel The error level of the debug message. > + @param Format Format string for the debug message to print. > + @param BaseListMarker BASE_LIST marker for the variable argument lis= t. > + > +**/ > +VOID > +EFIAPI > +DebugBPrint ( > + IN UINTN ErrorLevel, > + IN CONST CHAR8 *Format, > + IN BASE_LIST BaseListMarker > + ) > +{ > + DebugPrintMarker (ErrorLevel, Format, mVaListNull, BaseListMarker); > +} > + > /** > Prints an assert message containing a filename, line number, and descr= iption. > This may be followed by a breakpoint or a dead loop. > -- > 2.16.2.windows.1