This data is great. The change is good. Reviewed-by: Liming Gao From: Liu, Zhiguang Sent: 2020年6月15日 15:34 To: Ard Biesheuvel ; Gao, Liming ; devel@edk2.groups.io Cc: Kinney, Michael D Subject: RE: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings Hi Ard, I also collected the image size of OVMFX64 size building with VS2015x86. Debug Release before after After-before before after after-before sec 27664 27409 -255 13968 13968 0 pei 223016 221000 -2016 107208 106984 -224 dxe 4507000 4481336 -25664 2987064 2979384 -7680 compact 1179776 1172528 -7248 922664 920304 -2360 It can reduce the image size in X64. Reviewed-by: Zhiguang Liu > > -----Original Message----- > From: Ard Biesheuvel > > Sent: Friday, June 12, 2020 11:11 PM > To: Gao, Liming >; devel@edk2.groups.io > Cc: Kinney, Michael D >; Liu, Zhiguang > > > Subject: Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute > addresses for error strings > > On 6/12/20 4:33 PM, Gao, Liming wrote: > > Ard: > > I will collect the image size on OVMF X64 platform with this patch. > > > > Building OvmfPkgX64.dsc in RELEASE mode using GCC5 profile gives me > > > Before: > SECFV [5%Full] 212992 total, 11760 used, 201232 free PEIFV [9%Full] 917504 > total, 89384 used, 828120 free DXEFV [22%Full] 12582912 total, 2806320 used, > 9776592 free FVMAIN_COMPACT [26%Full] 3440640 total, 918760 used, > 2521880 free > > After: > SECFV [5%Full] 212992 total, 11760 used, 201232 free PEIFV [9%Full] 917504 > total, 89192 used, 828312 free DXEFV [22%Full] 12582912 total, 2802928 used, > 9779984 free FVMAIN_COMPACT [26%Full] 3440640 total, 916784 used, > 2523856 free > > So no change for SECFV, -192 bytes for PEIFV, -3392 bytes for DXEFV and > -1976 bytes for the resulting outer FV image. > > > > >> -----Original Message----- > >> From: devel@edk2.groups.io > On Behalf Of Ard > Biesheuvel > >> Sent: Friday, June 12, 2020 6:35 AM > >> To: devel@edk2.groups.io > >> Cc: Ard Biesheuvel >; Kinney, Michael D > >; Gao, Liming > > >> Subject: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute > addresses for error strings > >> > >> The mStatusString[] array is constructed as an array of pointer-to-char, > >> which means that on X64 or AARCH64, it is emitted as a single linear list > >> of 64-bit quantities, each containing the absolute address of one of the > >> string literals in memory. > >> > >> This means that each string takes up 8 bytes of additional space, along > >> with 2 bytes of relocation data. It also means that extra work needs to > >> be done at runtime to process these relocations, every time a module is > >> loaded that incorporates this library. > >> > >> So fix both issues, by splitting mStatusString into two arrays of char > >> arrays. The memory footprint decreases from 955 to 843 bytes, and given > >> that in the latter case, the overhead consists of 278 NUL characters rather > >> than 390 bytes worth of absolute addresses and relocation records, the > size > >> of a compressed image is reduced even further. For example, when > building > >> ArmVirtQemu.dsc in RELEASE mode for AARCH64 with the GCC5 profile, I > get: > >> > >> Before > >> > >> FV Space Information > >> FVMAIN [100%Full] 5329920 total, 5329920 used, 0 free > >> FVMAIN_COMPACT [38%Full] 2093056 total, 811840 used, 1281216 free > >> > >> After > >> > >> FV Space Information > >> FVMAIN [100%Full] 5321728 total, 5321728 used, 0 free > >> FVMAIN_COMPACT [38%Full] 2093056 total, 809696 used, 1283360 free > >> > >> So the uncompressed contents of the compressed image are 8 KB smaller, > >> whereas the resulting flash image (consisting of the compressed image > >> along with SEC, PEI_CORE and a set of PEIMs that execute in place) is > >> 2 KB smaller. > >> > >> Cc: "Kinney, Michael D" > > >> Cc: "Gao, Liming" > > >> Signed-off-by: Ard Biesheuvel > > >> --- > >> v3: > >> - add code comments to explain what the inner dimension of each array is > >> based on > >> > >> v2: > >> - split off this patch from the StandaloneMmPkg series, since they are not > >> interdependent anyway, and so they can be discussed separately > >> - remove mention of StandaloneMmPkg from the commit log - the space > savings > >> by themselves are sufficient justification > >> - update the before/after numbers with the current results > >> - split the warnings and errors into a separate array, so that the latter > >> can use smaller entries > >> - clarify the commit log to explain the effect on compressed as well as > >> XIP images (which both get smaller) > >> > >> MdePkg/Library/BasePrintLib/PrintLibInternal.c | 15 ++++++++++++--- > >> 1 file changed, 12 insertions(+), 3 deletions(-) > >> > >> diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c > b/MdePkg/Library/BasePrintLib/PrintLibInternal.c > >> index b6ec5ac4fbb9..50c6e8559c43 100644 > >> --- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c > >> +++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c > >> @@ -27,13 +27,22 @@ > >> > >> > >> GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 mHexStr[] = > {'0','1','2','3','4','5','6','7','8','9','A','B','C','D','E','F'}; > >> > >> > >> > >> -GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 * CONST > mStatusString[] = { > >> > >> +// > >> > >> +// Longest string: RETURN_WARN_BUFFER_TOO_SMALL => 24 > characters plus NUL byte > >> > >> +// > >> > >> +GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 > mWarningString[][24+1] = { > >> > >> "Success", // RETURN_SUCCESS = 0 > >> > >> "Warning Unknown Glyph", // RETURN_WARN_UNKNOWN_GLYPH > = 1 > >> > >> "Warning Delete Failure", // RETURN_WARN_DELETE_FAILURE = 2 > >> > >> "Warning Write Failure", // RETURN_WARN_WRITE_FAILURE = 3 > >> > >> "Warning Buffer Too Small", // > RETURN_WARN_BUFFER_TOO_SMALL = 4 > >> > >> "Warning Stale Data", // RETURN_WARN_STALE_DATA = 5 > >> > >> +}; > >> > >> + > >> > >> +// > >> > >> +// Longest string: RETURN_INCOMPATIBLE_VERSION => 20 characters > plus NUL byte > >> > >> +// > >> > >> +GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 > mErrorString[][20+1] = { > >> > >> "Load Error", // RETURN_LOAD_ERROR = 1 | MAX_BIT > >> > >> "Invalid Parameter", // RETURN_INVALID_PARAMETER = 2 | > MAX_BIT > >> > >> "Unsupported", // RETURN_UNSUPPORTED = 3 | > MAX_BIT > >> > >> @@ -996,12 +1005,12 @@ BasePrintLibSPrintMarker ( > >> // > >> > >> Index = Status & ~MAX_BIT; > >> > >> if (Index > 0 && Index <= ERROR_STATUS_NUMBER) { > >> > >> - ArgumentString = mStatusString [Index + > WARNING_STATUS_NUMBER]; > >> > >> + ArgumentString = mErrorString [Index - 1]; > >> > >> } > >> > >> } else { > >> > >> Index = Status; > >> > >> if (Index <= WARNING_STATUS_NUMBER) { > >> > >> - ArgumentString = mStatusString [Index]; > >> > >> + ArgumentString = mWarningString [Index]; > >> > >> } > >> > >> } > >> > >> if (ArgumentString == ValueBuffer) { > >> > >> -- > >> 2.26.2 > >> > >> > >> -=-=-=-=-=-= > >> Groups.io Links: You receive all messages sent to this group. > >> > >> View/Reply Online (#61170): > https://edk2.groups.io/g/devel/message/61170 > >> Mute This Topic: https://groups.io/mt/74829004/1759384 > >> Group Owner: devel+owner@edk2.groups.io > >> Unsubscribe: https://edk2.groups.io/g/devel/unsub > [liming.gao@intel.com] > >> -=-=-=-=-=-= > >