From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by mx.groups.io with SMTP id smtpd.web11.22459.1591974683401494401 for ; Fri, 12 Jun 2020 08:11:23 -0700 Authentication-Results: mx.groups.io; dkim=missing; spf=pass (domain: arm.com, ip: 217.140.110.172, mailfrom: ard.biesheuvel@arm.com) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id D33BE31B; Fri, 12 Jun 2020 08:11:21 -0700 (PDT) Received: from [192.168.1.69] (unknown [10.37.8.184]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id DD4763F66F; Fri, 12 Jun 2020 08:11:20 -0700 (PDT) Subject: Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings To: "Gao, Liming" , "devel@edk2.groups.io" Cc: "Kinney, Michael D" , "Liu, Zhiguang" References: <20200611223509.344290-1-ard.biesheuvel@arm.com> From: "Ard Biesheuvel" Message-ID: Date: Fri, 12 Jun 2020 17:11:15 +0200 User-Agent: Mozilla/5.0 (X11; Linux aarch64; rv:68.0) Gecko/20100101 Thunderbird/68.9.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit 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] >> -=-=-=-=-=-= >