Ard:
I will collect the image size on OVMF X64 platform with this patch.
Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io <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 <ard.biesheu...@arm.com>; Kinney, Michael D
> <michael.d.kin...@intel.com>; Gao, Liming <liming....@intel.com>
> 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" <michael.d.kin...@intel.com>
> Cc: "Gao, Liming" <liming....@intel.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheu...@arm.com>
> ---
> 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+ow...@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [liming....@intel.com]
> -=-=-=-=-=-=
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61213): https://edk2.groups.io/g/devel/message/61213
Mute This Topic: https://groups.io/mt/74829004/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
