From: "Zhiguang Liu" <zhiguang.liu@intel.com>
To: Ard Biesheuvel <ard.biesheuvel@arm.com>,
"Gao, Liming" <liming.gao@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Kinney, Michael D" <michael.d.kinney@intel.com>
Subject: Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings
Date: Mon, 15 Jun 2020 07:33:38 +0000 [thread overview]
Message-ID: <DM6PR11MB4012B18571DF21AC64F8BAA7909C0@DM6PR11MB4012.namprd11.prod.outlook.com> (raw)
In-Reply-To: <e86a5601-c4c2-0db5-5672-9b9dbb12238f@arm.com>
[-- Attachment #1: Type: text/plain, Size: 8681 bytes --]
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 <zhiguang.liu@intel.com<mailto:zhiguang.liu@intel.com>>
> -----Original Message-----
> From: Ard Biesheuvel <ard.biesheuvel@arm.com>
> Sent: Friday, June 12, 2020 11:11 PM
> To: Gao, Liming <liming.gao@intel.com>; devel@edk2.groups.io
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Liu, Zhiguang
> <zhiguang.liu@intel.com>
> 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<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Ard
> Biesheuvel
> >> Sent: Friday, June 12, 2020 6:35 AM
> >> To: devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> >> Cc: Ard Biesheuvel <ard.biesheuvel@arm.com<mailto:ard.biesheuvel@arm.com>>; Kinney, Michael D
> <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Gao, Liming <liming.gao@intel.com<mailto:liming.gao@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.kinney@intel.com<mailto:michael.d.kinney@intel.com>>
> >> Cc: "Gao, Liming" <liming.gao@intel.com<mailto:liming.gao@intel.com>>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com<mailto:ard.biesheuvel@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+owner@edk2.groups.io<mailto:devel+owner@edk2.groups.io>
> >> Unsubscribe: https://edk2.groups.io/g/devel/unsub
> [liming.gao@intel.com]
> >> -=-=-=-=-=-=
> >
[-- Attachment #2: Type: text/html, Size: 33593 bytes --]
next prev parent reply other threads:[~2020-06-15 7:33 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-11 22:35 [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings Ard Biesheuvel
2020-06-12 14:33 ` [edk2-devel] " Liming Gao
2020-06-12 15:11 ` Ard Biesheuvel
2020-06-15 7:33 ` Zhiguang Liu [this message]
2020-06-16 1:57 ` Liming Gao
2020-06-16 9:14 ` Ard Biesheuvel
2020-06-16 21:12 ` Andrew Fish
2020-06-16 22:12 ` Ard Biesheuvel
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=DM6PR11MB4012B18571DF21AC64F8BAA7909C0@DM6PR11MB4012.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox