* [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings
@ 2020-06-11 22:35 Ard Biesheuvel
2020-06-12 14:33 ` [edk2-devel] " Liming Gao
0 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2020-06-11 22:35 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Kinney, Michael D, Gao, Liming
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>
Cc: "Gao, Liming" <liming.gao@intel.com>
Signed-off-by: Ard Biesheuvel <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
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings
2020-06-11 22:35 [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings Ard Biesheuvel
@ 2020-06-12 14:33 ` Liming Gao
2020-06-12 15:11 ` Ard Biesheuvel
0 siblings, 1 reply; 8+ messages in thread
From: Liming Gao @ 2020-06-12 14:33 UTC (permalink / raw)
To: devel@edk2.groups.io, ard.biesheuvel@arm.com
Cc: Kinney, Michael D, Liu, Zhiguang
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.biesheuvel@arm.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <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>
> Cc: "Gao, Liming" <liming.gao@intel.com>
> Signed-off-by: Ard Biesheuvel <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
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [liming.gao@intel.com]
> -=-=-=-=-=-=
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings
2020-06-12 14:33 ` [edk2-devel] " Liming Gao
@ 2020-06-12 15:11 ` Ard Biesheuvel
2020-06-15 7:33 ` Zhiguang Liu
0 siblings, 1 reply; 8+ messages in thread
From: Ard Biesheuvel @ 2020-06-12 15:11 UTC (permalink / raw)
To: Gao, Liming, devel@edk2.groups.io; +Cc: Kinney, Michael D, Liu, Zhiguang
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 <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.biesheuvel@arm.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <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>
>> Cc: "Gao, Liming" <liming.gao@intel.com>
>> Signed-off-by: Ard Biesheuvel <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
>> Unsubscribe: https://edk2.groups.io/g/devel/unsub [liming.gao@intel.com]
>> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings
2020-06-12 15:11 ` Ard Biesheuvel
@ 2020-06-15 7:33 ` Zhiguang Liu
2020-06-16 1:57 ` Liming Gao
0 siblings, 1 reply; 8+ messages in thread
From: Zhiguang Liu @ 2020-06-15 7:33 UTC (permalink / raw)
To: Ard Biesheuvel, Gao, Liming, devel@edk2.groups.io; +Cc: Kinney, Michael D
[-- 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 --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings
2020-06-15 7:33 ` Zhiguang Liu
@ 2020-06-16 1:57 ` Liming Gao
2020-06-16 9:14 ` Ard Biesheuvel
2020-06-16 21:12 ` Andrew Fish
0 siblings, 2 replies; 8+ messages in thread
From: Liming Gao @ 2020-06-16 1:57 UTC (permalink / raw)
To: Liu, Zhiguang, Ard Biesheuvel, devel@edk2.groups.io; +Cc: Kinney, Michael D
[-- Attachment #1: Type: text/plain, Size: 9263 bytes --]
This data is great. The change is good. Reviewed-by: Liming Gao <liming.gao@intel.com>
From: Liu, Zhiguang <zhiguang.liu@intel.com>
Sent: 2020年6月15日 15:34
To: Ard Biesheuvel <ard.biesheuvel@arm.com>; Gao, Liming <liming.gao@intel.com>; 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
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<mailto:ard.biesheuvel@arm.com>>
> Sent: Friday, June 12, 2020 11:11 PM
> To: Gao, Liming <liming.gao@intel.com<mailto:liming.gao@intel.com>>; devel@edk2.groups.io<mailto:devel@edk2.groups.io>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com<mailto:michael.d.kinney@intel.com>>; Liu, Zhiguang
> <zhiguang.liu@intel.com<mailto: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: 43600 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings
2020-06-16 1:57 ` Liming Gao
@ 2020-06-16 9:14 ` Ard Biesheuvel
2020-06-16 21:12 ` Andrew Fish
1 sibling, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2020-06-16 9:14 UTC (permalink / raw)
To: Gao, Liming, Liu, Zhiguang, devel@edk2.groups.io; +Cc: Kinney, Michael D
On 6/16/20 3:57 AM, Gao, Liming wrote:
> This data is great. The change is good. Reviewed-by: Liming Gao
> <liming.gao@intel.com>
>
Submitted as #699 and merged,
Thanks all
> *From:*Liu, Zhiguang <zhiguang.liu@intel.com>
> *Sent:* 2020年6月15日15:34
> *To:* Ard Biesheuvel <ard.biesheuvel@arm.com>; Gao, Liming
> <liming.gao@intel.com>; 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
>
> 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 <mailto:ard.biesheuvel@arm.com>>
>
>> Sent: Friday, June 12, 2020 11:11 PM
>
>> To: Gao, Liming <liming.gao@intel.com <mailto:liming.gao@intel.com>>;
> devel@edk2.groups.io <mailto:devel@edk2.groups.io>
>
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>; Liu,
> Zhiguang
>
>> <zhiguang.liu@intel.com <mailto: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]
>
>> >> -=-=-=-=-=-=
>
>> >
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings
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
1 sibling, 1 reply; 8+ messages in thread
From: Andrew Fish @ 2020-06-16 21:12 UTC (permalink / raw)
To: edk2-devel-groups-io, Ard Biesheuvel
Cc: Liu, Zhiguang, Mike Kinney, Gao, Liming
[-- Attachment #1: Type: text/plain, Size: 19189 bytes --]
I was wondering what would happen if we converted the CHAR8 arrays in to strings with multiple nulls [1]. Looks like it saves space in uncompressed FVs, but not in compressed FVs.
Here are the Xcode numbers for X64 DEBUG Ovmf:
With this Patch:
SECFV [14%Full] 212992 total, 30128 used, 182864 free
PEIFV [29%Full] 917504 total, 273192 used, 644312 free
DXEFV [39%Full] 12582912 total, 4997120 used, 7585792 free
FVMAIN_COMPACT [36%Full] 3440640 total, 1271264 used, 2169376 free
Vs my patch:
SECFV [14%Full] 212992 total, 29872 used, 183120 free
PEIFV [29%Full] 917504 total, 271048 used, 646456 free
DXEFV [39%Full] 12582912 total, 4979552 used, 7603360 free
FVMAIN_COMPACT [36%Full] 3440640 total, 1271824 used, 2168816 free
SEC: -256
PEI: -2,144
Dxe: -17,568
Compact: +560
[1] $ git diff
diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
index 50c6e8559c43..db2533e7affb 100644
--- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c
+++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c
@@ -30,53 +30,73 @@ GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 mHexStr[] = {'0','1','2','3','4','5','
//
// 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
-};
+GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 *mWarningString = \
+ "Success\0" /* RETURN_SUCCESS = 0 */ \
+ "Warning Unknown Glyph\0" /* RETURN_WARN_UNKNOWN_GLYPH = 1 */ \
+ "Warning Delete Failure\0" /* RETURN_WARN_DELETE_FAILURE = 2 */ \
+ "Warning Write Failure\0" /* RETURN_WARN_WRITE_FAILURE = 3 */ \
+ "Warning Buffer Too Small\0" /* 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
- "Bad Buffer Size", // RETURN_BAD_BUFFER_SIZE = 4 | MAX_BIT
- "Buffer Too Small", // RETURN_BUFFER_TOO_SMALL, = 5 | MAX_BIT
- "Not Ready", // RETURN_NOT_READY = 6 | MAX_BIT
- "Device Error", // RETURN_DEVICE_ERROR = 7 | MAX_BIT
- "Write Protected", // RETURN_WRITE_PROTECTED = 8 | MAX_BIT
- "Out of Resources", // RETURN_OUT_OF_RESOURCES = 9 | MAX_BIT
- "Volume Corrupt", // RETURN_VOLUME_CORRUPTED = 10 | MAX_BIT
- "Volume Full", // RETURN_VOLUME_FULL = 11 | MAX_BIT
- "No Media", // RETURN_NO_MEDIA = 12 | MAX_BIT
- "Media changed", // RETURN_MEDIA_CHANGED = 13 | MAX_BIT
- "Not Found", // RETURN_NOT_FOUND = 14 | MAX_BIT
- "Access Denied", // RETURN_ACCESS_DENIED = 15 | MAX_BIT
- "No Response", // RETURN_NO_RESPONSE = 16 | MAX_BIT
- "No mapping", // RETURN_NO_MAPPING = 17 | MAX_BIT
- "Time out", // RETURN_TIMEOUT = 18 | MAX_BIT
- "Not started", // RETURN_NOT_STARTED = 19 | MAX_BIT
- "Already started", // RETURN_ALREADY_STARTED = 20 | MAX_BIT
- "Aborted", // RETURN_ABORTED = 21 | MAX_BIT
- "ICMP Error", // RETURN_ICMP_ERROR = 22 | MAX_BIT
- "TFTP Error", // RETURN_TFTP_ERROR = 23 | MAX_BIT
- "Protocol Error", // RETURN_PROTOCOL_ERROR = 24 | MAX_BIT
- "Incompatible Version", // RETURN_INCOMPATIBLE_VERSION = 25 | MAX_BIT
- "Security Violation", // RETURN_SECURITY_VIOLATION = 26 | MAX_BIT
- "CRC Error", // RETURN_CRC_ERROR = 27 | MAX_BIT
- "End of Media", // RETURN_END_OF_MEDIA = 28 | MAX_BIT
- "Reserved (29)", // RESERVED = 29 | MAX_BIT
- "Reserved (30)", // RESERVED = 30 | MAX_BIT
- "End of File", // RETURN_END_OF_FILE = 31 | MAX_BIT
- "Invalid Language", // RETURN_INVALID_LANGUAGE = 32 | MAX_BIT
- "Compromised Data" // RETURN_COMPROMISED_DATA = 33 | MAX_BIT
-};
+GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 *mErrorString = \
+ "Load Error\0" /* RETURN_LOAD_ERROR = 1 | MAX_BIT */ \
+ "Invalid Parameter\0" /* RETURN_INVALID_PARAMETER = 2 | MAX_BIT */ \
+ "Unsupported\0" /* RETURN_UNSUPPORTED = 3 | MAX_BIT */ \
+ "Bad Buffer Size\0" /* RETURN_BAD_BUFFER_SIZE = 4 | MAX_BIT */ \
+ "Buffer Too Small\0" /* RETURN_BUFFER_TOO_SMALL, = 5 | MAX_BIT */ \
+ "Not Ready\0" /* RETURN_NOT_READY = 6 | MAX_BIT */ \
+ "Device Error\0" /* RETURN_DEVICE_ERROR = 7 | MAX_BIT */ \
+ "Write Protected\0" /* RETURN_WRITE_PROTECTED = 8 | MAX_BIT */ \
+ "Out of Resources\0" /* RETURN_OUT_OF_RESOURCES = 9 | MAX_BIT */ \
+ "Volume Corrupt\0" /* RETURN_VOLUME_CORRUPTED = 10 | MAX_BIT */ \
+ "Volume Full\0" /* RETURN_VOLUME_FULL = 11 | MAX_BIT */ \
+ "No Media\0" /* RETURN_NO_MEDIA = 12 | MAX_BIT */ \
+ "Media changed\0" /* RETURN_MEDIA_CHANGED = 13 | MAX_BIT */ \
+ "Not Found\0" /* RETURN_NOT_FOUND = 14 | MAX_BIT */ \
+ "Access Denied\0" /* RETURN_ACCESS_DENIED = 15 | MAX_BIT */ \
+ "No Response\0" /* RETURN_NO_RESPONSE = 16 | MAX_BIT */ \
+ "No mapping\0" /* RETURN_NO_MAPPING = 17 | MAX_BIT */ \
+ "Time out\0" /* RETURN_TIMEOUT = 18 | MAX_BIT */ \
+ "Not started\0" /* RETURN_NOT_STARTED = 19 | MAX_BIT */ \
+ "Already started\0" /* RETURN_ALREADY_STARTED = 20 | MAX_BIT */ \
+ "Aborted\0" /* RETURN_ABORTED = 21 | MAX_BIT */ \
+ "ICMP Error\0" /* RETURN_ICMP_ERROR = 22 | MAX_BIT */ \
+ "TFTP Error\0" /* RETURN_TFTP_ERROR = 23 | MAX_BIT */ \
+ "Protocol Error\0" /* RETURN_PROTOCOL_ERROR = 24 | MAX_BIT */ \
+ "Incompatible Version\0" /* RETURN_INCOMPATIBLE_VERSION = 25 | MAX_BIT */ \
+ "Security Violation\0" /* RETURN_SECURITY_VIOLATION = 26 | MAX_BIT */ \
+ "CRC Error\0" /* RETURN_CRC_ERROR = 27 | MAX_BIT */ \
+ "End of Media\0" /* RETURN_END_OF_MEDIA = 28 | MAX_BIT */ \
+ "Reserved (29)\0" /* RESERVED = 29 | MAX_BIT */ \
+ "Reserved (30)\0" /* RESERVED = 30 | MAX_BIT */ \
+ "End of File\0" /* RETURN_END_OF_FILE = 31 | MAX_BIT */ \
+ "Invalid Language\0" /* RETURN_INVALID_LANGUAGE = 32 | MAX_BIT */ \
+ "Compromised Data"; /* RETURN_COMPROMISED_DATA = 33 | MAX_BIT */
+
+
+CONST CHAR8 *
+FindNthStr (
+ IN CONST CHAR8 *Start,
+ IN UINTN Index
+ )
+{
+ CONST CHAR8 *str;
+
+ for (str = Start; Index > 0; str++) {
+ if (*str == '\0') {
+ Index--;
+ if (Index == 0) {
+ str++;
+ break;
+ }
+ }
+ }
+
+ return str;
+}
/**
@@ -1005,12 +1025,12 @@ BasePrintLibSPrintMarker (
//
Index = Status & ~MAX_BIT;
if (Index > 0 && Index <= ERROR_STATUS_NUMBER) {
- ArgumentString = mErrorString [Index - 1];
+ ArgumentString = FindNthStr (mErrorString, Index - 1);
}
} else {
Index = Status;
if (Index <= WARNING_STATUS_NUMBER) {
- ArgumentString = mWarningString [Index];
+ ArgumentString = FindNthStr (mWarningString, Index);
}
}
if (ArgumentString == ValueBuffer) {
Thanks,
Andrew Fish
> On Jun 15, 2020, at 6:57 PM, Liming Gao <liming.gao@intel.com> wrote:
>
> This data is great. The change is good. Reviewed-by: Liming Gao <liming.gao@intel.com <mailto:liming.gao@intel.com>>
>
> From: Liu, Zhiguang <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com>>
> Sent: 2020年6月15日 15:34
> To: Ard Biesheuvel <ard.biesheuvel@arm.com <mailto:ard.biesheuvel@arm.com>>; Gao, Liming <liming.gao@intel.com <mailto:liming.gao@intel.com>>; devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> Cc: Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>
> 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 <zhiguang.liu@intel.com <mailto:zhiguang.liu@intel.com>>
>
> > -----Original Message-----
> > From: Ard Biesheuvel <ard.biesheuvel@arm.com <mailto:ard.biesheuvel@arm.com>>
> > Sent: Friday, June 12, 2020 11:11 PM
> > To: Gao, Liming <liming.gao@intel.com <mailto:liming.gao@intel.com>>; devel@edk2.groups.io <mailto:devel@edk2.groups.io>
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>; Liu, Zhiguang
> > <zhiguang.liu@intel.com <mailto: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 <http://groups.io/> Links: You receive all messages sent to this group.
> > >>
> > >> View/Reply Online (#61170):
> > https://edk2.groups.io/g/devel/message/61170 <https://edk2.groups.io/g/devel/message/61170>
> > >> Mute This Topic: https://groups.io/mt/74829004/1759384 <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 <https://edk2.groups.io/g/devel/unsub>
> > [liming.gao@intel.com <mailto:liming.gao@intel.com>]
> > >> -=-=-=-=-=-=
> > >
>
>
[-- Attachment #2: Type: text/html, Size: 120197 bytes --]
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [edk2-devel] [PATCH v3] MdePkg/BasePrintLib: avoid absolute addresses for error strings
2020-06-16 21:12 ` Andrew Fish
@ 2020-06-16 22:12 ` Ard Biesheuvel
0 siblings, 0 replies; 8+ messages in thread
From: Ard Biesheuvel @ 2020-06-16 22:12 UTC (permalink / raw)
To: Andrew Fish, edk2-devel-groups-io; +Cc: Liu, Zhiguang, Mike Kinney, Gao, Liming
On 6/16/20 11:12 PM, Andrew Fish wrote:
> I was wondering what would happen if we converted the CHAR8 arrays in to
> strings with multiple nulls [1]. Looks like it saves space in
> uncompressed FVs, but not in compressed FVs.
>
Yeah, those NUL bytes compress really well, so the overhead of the extra
code you need to unpack your strings stands out in the compressed FV.
> Here are the Xcode numbers for X64 DEBUG Ovmf:
>
> With this Patch:
> SECFV [14%Full] 212992 total, 30128 used, 182864 free
> PEIFV [29%Full] 917504 total, 273192 used, 644312 free
> DXEFV [39%Full] 12582912 total, 4997120 used, 7585792 free
> FVMAIN_COMPACT [36%Full] 3440640 total, 1271264 used, 2169376 free
>
> Vs my patch:
> SECFV [14%Full] 212992 total, 29872 used, 183120 free
> PEIFV [29%Full] 917504 total, 271048 used, 646456 free
> DXEFV [39%Full] 12582912 total, 4979552 used, 7603360 free
> FVMAIN_COMPACT [36%Full] 3440640 total, 1271824 used, 2168816 free
>
> SEC: -256
> PEI: -2,144
> Dxe: -17,568
> Compact: +560
>
> [1] $ git diff
> *diff --git a/MdePkg/Library/BasePrintLib/PrintLibInternal.c
> b/MdePkg/Library/BasePrintLib/PrintLibInternal.c*
> *index 50c6e8559c43..db2533e7affb 100644*
> *--- a/MdePkg/Library/BasePrintLib/PrintLibInternal.c*
> *+++ b/MdePkg/Library/BasePrintLib/PrintLibInternal.c*
> @@ -30,53 +30,73 @@GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 mHexStr[] =
> {'0','1','2','3','4','5','
> //
> // 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
> -};
> +GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 *mWarningString =
> \
> + "Success\0" /* RETURN_SUCCESS =
> 0 */ \
> + "Warning Unknown Glyph\0" /* RETURN_WARN_UNKNOWN_GLYPH =
> 1 */ \
> + "Warning Delete Failure\0" /* RETURN_WARN_DELETE_FAILURE =
> 2 */ \
> + "Warning Write Failure\0" /* RETURN_WARN_WRITE_FAILURE =
> 3 */ \
> + "Warning Buffer Too Small\0" /* 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
> - "Bad Buffer Size", // RETURN_BAD_BUFFER_SIZE =
> 4 | MAX_BIT
> - "Buffer Too Small", // RETURN_BUFFER_TOO_SMALL, =
> 5 | MAX_BIT
> - "Not Ready", // RETURN_NOT_READY =
> 6 | MAX_BIT
> - "Device Error", // RETURN_DEVICE_ERROR =
> 7 | MAX_BIT
> - "Write Protected", // RETURN_WRITE_PROTECTED =
> 8 | MAX_BIT
> - "Out of Resources", // RETURN_OUT_OF_RESOURCES =
> 9 | MAX_BIT
> - "Volume Corrupt", // RETURN_VOLUME_CORRUPTED =
> 10 | MAX_BIT
> - "Volume Full", // RETURN_VOLUME_FULL =
> 11 | MAX_BIT
> - "No Media", // RETURN_NO_MEDIA =
> 12 | MAX_BIT
> - "Media changed", // RETURN_MEDIA_CHANGED =
> 13 | MAX_BIT
> - "Not Found", // RETURN_NOT_FOUND =
> 14 | MAX_BIT
> - "Access Denied", // RETURN_ACCESS_DENIED =
> 15 | MAX_BIT
> - "No Response", // RETURN_NO_RESPONSE =
> 16 | MAX_BIT
> - "No mapping", // RETURN_NO_MAPPING =
> 17 | MAX_BIT
> - "Time out", // RETURN_TIMEOUT =
> 18 | MAX_BIT
> - "Not started", // RETURN_NOT_STARTED =
> 19 | MAX_BIT
> - "Already started", // RETURN_ALREADY_STARTED =
> 20 | MAX_BIT
> - "Aborted", // RETURN_ABORTED =
> 21 | MAX_BIT
> - "ICMP Error", // RETURN_ICMP_ERROR =
> 22 | MAX_BIT
> - "TFTP Error", // RETURN_TFTP_ERROR =
> 23 | MAX_BIT
> - "Protocol Error", // RETURN_PROTOCOL_ERROR =
> 24 | MAX_BIT
> - "Incompatible Version", // RETURN_INCOMPATIBLE_VERSION =
> 25 | MAX_BIT
> - "Security Violation", // RETURN_SECURITY_VIOLATION =
> 26 | MAX_BIT
> - "CRC Error", // RETURN_CRC_ERROR =
> 27 | MAX_BIT
> - "End of Media", // RETURN_END_OF_MEDIA =
> 28 | MAX_BIT
> - "Reserved (29)", // RESERVED =
> 29 | MAX_BIT
> - "Reserved (30)", // RESERVED =
> 30 | MAX_BIT
> - "End of File", // RETURN_END_OF_FILE =
> 31 | MAX_BIT
> - "Invalid Language", // RETURN_INVALID_LANGUAGE =
> 32 | MAX_BIT
> - "Compromised Data" // RETURN_COMPROMISED_DATA =
> 33 | MAX_BIT
> -};
> +GLOBAL_REMOVE_IF_UNREFERENCED CONST CHAR8 *mErrorString =
> \
> + "Load Error\0" /* RETURN_LOAD_ERROR =
> 1 | MAX_BIT */ \
> + "Invalid Parameter\0" /* RETURN_INVALID_PARAMETER =
> 2 | MAX_BIT */ \
> + "Unsupported\0" /* RETURN_UNSUPPORTED =
> 3 | MAX_BIT */ \
> + "Bad Buffer Size\0" /* RETURN_BAD_BUFFER_SIZE =
> 4 | MAX_BIT */ \
> + "Buffer Too Small\0" /* RETURN_BUFFER_TOO_SMALL, =
> 5 | MAX_BIT */ \
> + "Not Ready\0" /* RETURN_NOT_READY =
> 6 | MAX_BIT */ \
> + "Device Error\0" /* RETURN_DEVICE_ERROR =
> 7 | MAX_BIT */ \
> + "Write Protected\0" /* RETURN_WRITE_PROTECTED =
> 8 | MAX_BIT */ \
> + "Out of Resources\0" /* RETURN_OUT_OF_RESOURCES =
> 9 | MAX_BIT */ \
> + "Volume Corrupt\0" /* RETURN_VOLUME_CORRUPTED =
> 10 | MAX_BIT */ \
> + "Volume Full\0" /* RETURN_VOLUME_FULL =
> 11 | MAX_BIT */ \
> + "No Media\0" /* RETURN_NO_MEDIA =
> 12 | MAX_BIT */ \
> + "Media changed\0" /* RETURN_MEDIA_CHANGED =
> 13 | MAX_BIT */ \
> + "Not Found\0" /* RETURN_NOT_FOUND =
> 14 | MAX_BIT */ \
> + "Access Denied\0" /* RETURN_ACCESS_DENIED =
> 15 | MAX_BIT */ \
> + "No Response\0" /* RETURN_NO_RESPONSE =
> 16 | MAX_BIT */ \
> + "No mapping\0" /* RETURN_NO_MAPPING =
> 17 | MAX_BIT */ \
> + "Time out\0" /* RETURN_TIMEOUT =
> 18 | MAX_BIT */ \
> + "Not started\0" /* RETURN_NOT_STARTED =
> 19 | MAX_BIT */ \
> + "Already started\0" /* RETURN_ALREADY_STARTED =
> 20 | MAX_BIT */ \
> + "Aborted\0" /* RETURN_ABORTED =
> 21 | MAX_BIT */ \
> + "ICMP Error\0" /* RETURN_ICMP_ERROR =
> 22 | MAX_BIT */ \
> + "TFTP Error\0" /* RETURN_TFTP_ERROR =
> 23 | MAX_BIT */ \
> + "Protocol Error\0" /* RETURN_PROTOCOL_ERROR =
> 24 | MAX_BIT */ \
> + "Incompatible Version\0" /* RETURN_INCOMPATIBLE_VERSION =
> 25 | MAX_BIT */ \
> + "Security Violation\0" /* RETURN_SECURITY_VIOLATION =
> 26 | MAX_BIT */ \
> + "CRC Error\0" /* RETURN_CRC_ERROR =
> 27 | MAX_BIT */ \
> + "End of Media\0" /* RETURN_END_OF_MEDIA =
> 28 | MAX_BIT */ \
> + "Reserved (29)\0" /* RESERVED =
> 29 | MAX_BIT */ \
> + "Reserved (30)\0" /* RESERVED =
> 30 | MAX_BIT */ \
> + "End of File\0" /* RETURN_END_OF_FILE =
> 31 | MAX_BIT */ \
> + "Invalid Language\0" /* RETURN_INVALID_LANGUAGE =
> 32 | MAX_BIT */ \
> + "Compromised Data"; /* RETURN_COMPROMISED_DATA =
> 33 | MAX_BIT */
> +
> +
> +CONST CHAR8 *
> +FindNthStr (
> + IN CONST CHAR8 *Start,
> + IN UINTN Index
> + )
> +{
> + CONST CHAR8 *str;
> +
> + for (str = Start; Index > 0; str++) {
> + if (*str == '\0') {
> + Index--;
> + if (Index == 0) {
> + str++;
> + break;
> + }
> + }
> + }
> +
> + return str;
> +}
>
>
>
> /**
> @@ -1005,12 +1025,12 @@BasePrintLibSPrintMarker (
> //
> Index = Status & ~MAX_BIT;
> if (Index > 0 && Index <= ERROR_STATUS_NUMBER) {
> - ArgumentString = mErrorString [Index - 1];
> + ArgumentString = FindNthStr (mErrorString, Index - 1);
> }
> } else {
> Index = Status;
> if (Index <= WARNING_STATUS_NUMBER) {
> - ArgumentString = mWarningString [Index];
> + ArgumentString = FindNthStr (mWarningString, Index);
> }
> }
> if (ArgumentString == ValueBuffer) {
>
> Thanks,
>
> Andrew Fish
>
>> On Jun 15, 2020, at 6:57 PM, Liming Gao <liming.gao@intel.com
>> <mailto:liming.gao@intel.com>> wrote:
>>
>> This data is great. The change is good. Reviewed-by: Liming Gao
>> <liming.gao@intel.com <mailto:liming.gao@intel.com>>
>> *From:*Liu, Zhiguang <zhiguang.liu@intel.com
>> <mailto:zhiguang.liu@intel.com>>
>> *Sent:*2020年6月15日15:34
>> *To:*Ard Biesheuvel <ard.biesheuvel@arm.com
>> <mailto:ard.biesheuvel@arm.com>>; Gao, Liming <liming.gao@intel.com
>> <mailto:liming.gao@intel.com>>;devel@edk2.groups.io
>> <mailto:devel@edk2.groups.io>
>> *Cc:*Kinney, Michael D <michael.d.kinney@intel.com
>> <mailto:michael.d.kinney@intel.com>>
>> *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 <zhiguang.liu@intel.com
>> <mailto:zhiguang.liu@intel.com>>
>> > -----Original Message-----
>> > From: Ard Biesheuvel <ard.biesheuvel@arm.com <mailto:ard.biesheuvel@arm.com>>
>> > Sent: Friday, June 12, 2020 11:11 PM
>> > To: Gao, Liming <liming.gao@intel.com
>> <mailto:liming.gao@intel.com>>;devel@edk2.groups.io
>> <mailto:devel@edk2.groups.io>
>> > Cc: Kinney, Michael D <michael.d.kinney@intel.com <mailto:michael.d.kinney@intel.com>>; Liu,
>> Zhiguang
>> > <zhiguang.liu@intel.com <mailto: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 <http://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 <mailto:liming.gao@intel.com>]
>> > >> -=-=-=-=-=-=
>> > >
>>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-06-16 22:12 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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
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
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox