From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.24; helo=mga09.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) (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 B5FA8211DF225 for ; Thu, 21 Mar 2019 00:06:13 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 21 Mar 2019 00:06:12 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.60,251,1549958400"; d="scan'208";a="133432582" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga008.fm.intel.com with ESMTP; 21 Mar 2019 00:06:12 -0700 Received: from fmsmsx152.amr.corp.intel.com (10.18.125.5) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 21 Mar 2019 00:06:12 -0700 Received: from shsmsx107.ccr.corp.intel.com (10.239.4.96) by FMSMSX152.amr.corp.intel.com (10.18.125.5) with Microsoft SMTP Server (TLS) id 14.3.408.0; Thu, 21 Mar 2019 00:06:11 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.74]) by SHSMSX107.ccr.corp.intel.com ([169.254.9.252]) with mapi id 14.03.0415.000; Thu, 21 Mar 2019 15:06:09 +0800 From: "Wu, Hao A" To: "Gao, Liming" , "Gao, Zhichao" , "edk2-devel@lists.01.org" CC: "Wang, Jian J" , "Ni, Ray" , "Zeng, Star" , Sean Brogan , Michael Turner , Bret Barkelew Thread-Topic: [PATCH V3 13/17] MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add new APIs Thread-Index: AQHU3mgx8OCi5bbzYkeeuIS8lCxa4KYVI9wAgACHjIA= Date: Thu, 21 Mar 2019 07:06:09 +0000 Message-ID: References: <20190319152549.16104-1-zhichao.gao@intel.com> <20190319152549.16104-14-zhichao.gao@intel.com> <4A89E2EF3DFEDB4C8BFDE51014F606A14E408772@SHSMSX104.ccr.corp.intel.com> In-Reply-To: <4A89E2EF3DFEDB4C8BFDE51014F606A14E408772@SHSMSX104.ccr.corp.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 V3 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: Thu, 21 Mar 2019 07:06:14 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable > -----Original Message----- > From: Gao, Liming > Sent: Thursday, March 21, 2019 2:58 PM > To: Gao, Zhichao; edk2-devel@lists.01.org > Cc: Wang, Jian J; Wu, Hao A; Ni, Ray; Zeng, Star; Sean Brogan; Michael Tu= rner; > Bret Barkelew > Subject: RE: [PATCH V3 13/17] > MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add new APIs >=20 > Zhichao: > Why do below change? Seemly, this change is not related to add new APIs= . In > fact, current log is added as the purpose by commit > 137ed15511e2045a7333e33ae7f1e873ce1961dd. I would suggest to add another separate commit to refine the below comments to state the message truncating behavior here: // // If the TotalSize is larger than the maximum record size, then return ^^^^^^^^^^^ // if (TotalSize > sizeof (Buffer)) { TotalSize =3D sizeof (Buffer); } Best Regards, Hao Wu >=20 > if (TotalSize > sizeof (Buffer)) { > TotalSize =3D sizeof (Buffer); =3D=3D> return; > } >=20 > Thanks > Liming > >-----Original Message----- > >From: Gao, Zhichao > >Sent: Tuesday, March 19, 2019 11:26 PM > >To: edk2-devel@lists.01.org > >Cc: Wang, Jian J ; Wu, Hao A = ; > >Ni, Ray ; Zeng, Star ; Gao, Limin= g > >; Sean Brogan ; > >Michael Turner ; Bret Barkelew > > > >Subject: [PATCH V3 13/17] > >MdeModulePkg/PeiDxeDebugLibReportStatusCode: Add new APIs > > > >REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1395 > > > >Add new APIs' implementation (DebugVPrint, DebugBPrint) > >in the DebugLib instance. These APIs would expose print > >routines with VaList parameter and BaseList parameter. > > > >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 | 144 > >++++++++++++++++++--- > > 1 file changed, 128 insertions(+), 16 deletions(-) > > > >diff --git > >a/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c > >b/MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/DebugLib.c > >index 6f0f416273..f1d31cb619 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 > allowable > > record length, then the debug message will be ignored directly. > > > >- 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 ma= y be > >found at > >@@ -27,6 +27,12 @@ > > #include > > #include > > > >+// > >+// VA_LIST can not initialize to NULL for all compiler, so we use this = to > >+// indicate a null VA_LIST > >+// > >+VA_LIST mVaListNull; > >+ > > /** > > Prints a debug message to the debug output device if the specified er= ror > >level is enabled. > > > >@@ -52,12 +58,43 @@ 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 funct= ion > >+ GetDebugPrintErrorLevel (), then print the message specified by Forma= t > >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= . > >+ @param BaseListMarker BASE_LIST marker for the variable argument li= st. > >+ > >+**/ > >+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 (UINT6= 4)) > >+ 1]; > > EFI_DEBUG_INFO *DebugInfo; > > UINTN TotalSize; > >- VA_LIST VaListMarker; > >- BASE_LIST BaseListMarker; > >+ BASE_LIST BaseListMarkerPointer; > > CHAR8 *FormatString; > > BOOLEAN Long; > > > >@@ -96,7 +133,7 @@ DebugPrint ( > > // If the TotalSize is larger than the maximum record size, then retu= rn > > // > > if (TotalSize > sizeof (Buffer)) { > >- TotalSize =3D sizeof (Buffer); > >+ return; > > } > > > > // > >@@ -109,7 +146,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); > > > > // > >@@ -125,7 +162,6 @@ DebugPrint ( > > // of format in DEBUG string, which is followed by the DEBUG format s= tring. > > // Here we will process the variable arguments and pack them in this = area. > > // > >- VA_START (VaListMarker, Format); > > > > // > > // Use the actual format string. > >@@ -167,7 +203,11 @@ DebugPrint ( > > // '*' in format field means the precision of the field is spec= ified by > > // a UINTN argument in the argument list. > > // > >- BASE_ARG (BaseListMarker, UINTN) =3D VA_ARG (VaListMarker, UINT= N); > >+ if (BaseListMarker =3D=3D NULL) { > >+ BASE_ARG (BaseListMarkerPointer, UINTN) =3D VA_ARG (VaListMar= ker, > >UINTN); > >+ } else { > >+ BASE_ARG (BaseListMarkerPointer, UINTN) =3D BASE_ARG > >(BaseListMarker, UINTN); > >+ } > > continue; > > } > > if (*Format =3D=3D '\0') { > >@@ -192,16 +232,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, INT6= 4); > >+ if (BaseListMarker =3D=3D NULL) { > >+ BASE_ARG (BaseListMarkerPointer, INT64) =3D VA_ARG (VaListMar= ker, > >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 (VaListMarke= r, int); > >+ } else { > >+ BASE_ARG (BaseListMarkerPointer, int) =3D BASE_ARG (BaseListM= arker, > >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 (VaListMark= er, > >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 (VaListMarke= r, > >UINTN); > >+ } else { > >+ BASE_ARG (BaseListMarkerPointer, UINTN) =3D BASE_ARG > >(BaseListMarker, 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); > >+ } > > } > > > > // > >@@ -209,17 +269,15 @@ DebugPrint ( > > // This indicates that the DEBUG() macro is passing in more argumen= t than > >can be handled by > > // the EFI_DEBUG_INFO record > > // > >- ASSERT ((CHAR8 *)BaseListMarker <=3D FormatString); > >+ ASSERT ((CHAR8 *)BaseListMarkerPointer <=3D FormatString); > > > > // > > // If the converted BASE_LIST is larger than the 12 * sizeof (UINT6= 4) > >allocated bytes, then return > > // > >- if ((CHAR8 *)BaseListMarker > FormatString) { > >- VA_END (VaListMarker); > >+ if ((CHAR8 *)BaseListMarkerPointer > FormatString) { > > return; > > } > > } > >- VA_END (VaListMarker); > > > > // > > // Send the DebugInfo record > >@@ -235,6 +293,60 @@ DebugPrint ( > > ); > > } > > > >+/** > >+ 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 funct= ion > >+ GetDebugPrintErrorLevel (), then print the message specified by Forma= t > >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 funct= ion > >+ GetDebugPrintErrorLevel (), then print the message specified by Forma= t > >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 li= st. > >+ > >+**/ > >+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 > description. > > This may be followed by a breakpoint or a dead loop. > >-- > >2.16.2.windows.1