Date   

Re: Managing GCC Assembly Code Size (AArch64)

Ard Biesheuvel
 

On 4 August 2016 at 20:08, Cohen, Eugene <eugene@hp.com> wrote:
Ard and Leif,

I've been too backlogged to provide a real patchset at this point but wanted to get your approval on this proposal...


As you know we have some code size sensitive uncompressed XIP stuff going on. For C code we get dead code stripping thanks to the "-ffunction-sections" switch which places each function in its own section so the linker can strip unreferenced sections.

For assembly there is not a solution that's as easy. For RVCT we handled this with an assembler macro that combined the procedure label definition, export of global symbols and placement of the procedure in its own section. For GCC I haven't found a way to fully do this because we rely on the C preprocessor for assembly which means you cannot expand to multi-line macros. (The label and assembler directives require their own lines but the preprocessor collapses stuff onto one line because in the C language newlines don't matter.)

So the solution I've settled on is to do this:

in MdePkg\Include\AArch64\ProcessorBind.h define:

/// Macro to place a function in its own section for dead code elimination
/// This must be placed directly before the corresponding code since the
/// .section directive applies to the code that follows it.
#define GCC_ASM_EXPORT_SECTION(func__) \
.global _CONCATENATE (__USER_LABEL_PREFIX__, func__) ;\
.section .text._CONCATENATE (__USER_LABEL_PREFIX__, func__) ;\
.type ASM_PFX(func__), %function; \

This has the effect of placing the function in a section called .text.<func__> so the linker can do its dead code stripping stuff. It also absorbs the making the symbol globally visible so the corresponding GCC_ASM_EXPORT statement can be removed.

then for every single assembly procedure change from this:

[top of file]
GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)

[lower down]
ASM_PFX(ArmInvalidateDataCacheEntryByMVA):
dc ivac, x0 // Invalidate single data cache line
ret

to this:

GCC_ASM_EXPORT_SECTION(ArmInvalidateDataCacheEntryByMVA)
ASM_PFX(ArmInvalidateDataCacheEntryByMVA):
dc ivac, x0 // Invalidate single data cache line
ret

Because the assembly label must appear in column 1 I couldn't find a way to use the C preprocessor to absorb it so hence the two lines. If you can find a way to improve on this it would be great.
What about GAS macros (.macro / .endm). I prefer those over cpp macros
in assembler anyway.

I'm not sure what impacts this might have to other toolchains - can this be translated to CLANG and ARM Compiler?
The asm dialect is 99% aligned between CLANG and GNU as, so this
shouldn't be a problem

I'd like to get your OK on this conceptually and then I could upstream some patches that modify the AArch64 *.S files to use this approach. Unfortunately it won't be complete because I only updated the libraries that we use. My hope is that long term all assembly (or at least assembly in libraries) adopt this approach so we are positioned for maximum dead code stripping.
I think this would be an improvement, so go for it. The only thing to
be wary of is routines that fall through into the subsequent one.
Those need to remain in the same section.


Re: [PATCH] BaseTools X64: fold PLT relocations into simple relative references

Ard Biesheuvel
 

On 4 aug. 2016, at 21:03, Nicolas Owens <mischief@offblast.org> wrote:

ard,

i think you need to have R_X86_64_PLT32 case in WriteRelocations64.
without that, i still hit the invalid relocation message.
Good point. I will send out a v2 tomorrow


On 08/04/2016 01:45 AM, Ard Biesheuvel wrote:
For X64/GCC, we use position independent code with hidden visibility
to inform the compiler that symbols references are never resolved at
runtime, which removes the need for PLTs and GOTs. However, in some
cases GCC has been reported to still emit PLT based relocations, which
we need to handle in the ELF to PE/COFF perform by GenFw.

Unlike GOT based relocations, which are non-trivial to handle since the
indirections in the code can not be fixed up easily (although relocation
types exist for X64 that annotate relocation targets as suitable for
relaxation), PLT relocations simply point to jump targets, and we can
relax such relocations by resolving them using the symbol directly rather
than via a PLT entry that does nothing more than tail call the function
we already know it is going to call (since all symbol references are
resolved in the same module).

So handle R_X86_64_PLT32 as a R_X86_64_PC32 relocation.

Suggested-by: Steven Shi <steven.shi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
BaseTools/Source/C/GenFw/Elf64Convert.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 944c94b8f8b4..7cbff0df0996 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -785,6 +785,17 @@ WriteSections64 (
*(INT32 *)Targ = (INT32)((INT64)(*(INT32 *)Targ) - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx]);
VerboseMsg ("Relocation: 0x%08X", *(UINT32*)Targ);
break;
+
+ case R_X86_64_PLT32:
+ //
+ // Treat R_X86_64_PLT32 relocations as R_X86_64_PC32: this is
+ // possible since we know all code symbol references resolve to
+ // definitions in the same module (UEFI has no shared libraries),
+ // and so there is never a reason to jump via a PLT entry,
+ // allowing us to resolve the reference using the symbol directly.
+ //
+ VerboseMsg ("Treating R_X86_64_PLT32 as R_X86_64_PC32 ...");
+ /* fall through */
case R_X86_64_PC32:
//
// Relative relocation: Symbol - Ip + Addend


Re: [PATCH] BaseTools X64: fold PLT relocations into simple relative references

Nicolas Owens <mischief@...>
 

ard,

i think you need to have R_X86_64_PLT32 case in WriteRelocations64.
without that, i still hit the invalid relocation message.

On 08/04/2016 01:45 AM, Ard Biesheuvel wrote:
For X64/GCC, we use position independent code with hidden visibility
to inform the compiler that symbols references are never resolved at
runtime, which removes the need for PLTs and GOTs. However, in some
cases GCC has been reported to still emit PLT based relocations, which
we need to handle in the ELF to PE/COFF perform by GenFw.

Unlike GOT based relocations, which are non-trivial to handle since the
indirections in the code can not be fixed up easily (although relocation
types exist for X64 that annotate relocation targets as suitable for
relaxation), PLT relocations simply point to jump targets, and we can
relax such relocations by resolving them using the symbol directly rather
than via a PLT entry that does nothing more than tail call the function
we already know it is going to call (since all symbol references are
resolved in the same module).

So handle R_X86_64_PLT32 as a R_X86_64_PC32 relocation.

Suggested-by: Steven Shi <steven.shi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
BaseTools/Source/C/GenFw/Elf64Convert.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 944c94b8f8b4..7cbff0df0996 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -785,6 +785,17 @@ WriteSections64 (
*(INT32 *)Targ = (INT32)((INT64)(*(INT32 *)Targ) - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx]);
VerboseMsg ("Relocation: 0x%08X", *(UINT32*)Targ);
break;
+
+ case R_X86_64_PLT32:
+ //
+ // Treat R_X86_64_PLT32 relocations as R_X86_64_PC32: this is
+ // possible since we know all code symbol references resolve to
+ // definitions in the same module (UEFI has no shared libraries),
+ // and so there is never a reason to jump via a PLT entry,
+ // allowing us to resolve the reference using the symbol directly.
+ //
+ VerboseMsg ("Treating R_X86_64_PLT32 as R_X86_64_PC32 ...");
+ /* fall through */
case R_X86_64_PC32:
//
// Relative relocation: Symbol - Ip + Addend


Managing GCC Assembly Code Size (AArch64)

Cohen, Eugene <eugene@...>
 

Ard and Leif,

I've been too backlogged to provide a real patchset at this point but wanted to get your approval on this proposal...


As you know we have some code size sensitive uncompressed XIP stuff going on. For C code we get dead code stripping thanks to the "-ffunction-sections" switch which places each function in its own section so the linker can strip unreferenced sections.

For assembly there is not a solution that's as easy. For RVCT we handled this with an assembler macro that combined the procedure label definition, export of global symbols and placement of the procedure in its own section. For GCC I haven't found a way to fully do this because we rely on the C preprocessor for assembly which means you cannot expand to multi-line macros. (The label and assembler directives require their own lines but the preprocessor collapses stuff onto one line because in the C language newlines don't matter.)

So the solution I've settled on is to do this:

in MdePkg\Include\AArch64\ProcessorBind.h define:

/// Macro to place a function in its own section for dead code elimination
/// This must be placed directly before the corresponding code since the
/// .section directive applies to the code that follows it.
#define GCC_ASM_EXPORT_SECTION(func__) \
.global _CONCATENATE (__USER_LABEL_PREFIX__, func__) ;\
.section .text._CONCATENATE (__USER_LABEL_PREFIX__, func__) ;\
.type ASM_PFX(func__), %function; \

This has the effect of placing the function in a section called .text.<func__> so the linker can do its dead code stripping stuff. It also absorbs the making the symbol globally visible so the corresponding GCC_ASM_EXPORT statement can be removed.

then for every single assembly procedure change from this:

[top of file]
GCC_ASM_EXPORT (ArmInvalidateDataCacheEntryByMVA)

[lower down]
ASM_PFX(ArmInvalidateDataCacheEntryByMVA):
dc ivac, x0 // Invalidate single data cache line
ret

to this:

GCC_ASM_EXPORT_SECTION(ArmInvalidateDataCacheEntryByMVA)
ASM_PFX(ArmInvalidateDataCacheEntryByMVA):
dc ivac, x0 // Invalidate single data cache line
ret

Because the assembly label must appear in column 1 I couldn't find a way to use the C preprocessor to absorb it so hence the two lines. If you can find a way to improve on this it would be great.

I'm not sure what impacts this might have to other toolchains - can this be translated to CLANG and ARM Compiler?

I'd like to get your OK on this conceptually and then I could upstream some patches that modify the AArch64 *.S files to use this approach. Unfortunately it won't be complete because I only updated the libraries that we use. My hope is that long term all assembly (or at least assembly in libraries) adopt this approach so we are positioned for maximum dead code stripping.

Thanks,

Eugene


Re: [PATCH 3/3] BaseTools GCC/ARM: add -fno-builtin to CC flags

Michael Zimmermann
 

not directly related but could we add nostdinc too? At least for my
platform that works perfectly and it prevents you from accidentally
including any libc/libgcc/whatever headers from your toolchain.(nostdlib is
already enabled)

Thanks
Michael

On Thu, Aug 4, 2016 at 4:42 PM, Ard Biesheuvel <ard.biesheuvel@linaro.org>
wrote:

Avoid build errors when including OpensslLib, which may throw
undefined reference errors for builtin functions if -fno-builtin
is not specified (and it is already set for IA32, X64 and AARCH64)
So set it for ARM as well.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
BaseTools/Conf/tools_def.template | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.
template
index 88af82a683d9..4f1dd4be378e 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4334,7 +4334,7 @@ DEFINE GCC_ALL_CC_FLAGS = -g -Os
-fshort-wchar -fno-strict-aliasing -
DEFINE GCC_IA32_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -m32
-malign-double -freorder-blocks -freorder-blocks-and-partition -O2
-mno-stack-arg-probe
DEFINE GCC_X64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mno-red-zone
-Wno-address -mno-stack-arg-probe
DEFINE GCC_IPF_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS)
-minline-int-divide-min-latency
-DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS)
-mlittle-endian -mabi=aapcs -fno-short-enums -funsigned-char
-ffunction-sections -fdata-sections -fomit-frame-pointer -Wno-address
-mthumb -mfloat-abi=soft
+DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS)
-mlittle-endian -mabi=aapcs -fno-short-enums -funsigned-char
-ffunction-sections -fdata-sections -fomit-frame-pointer -fno-builtin
-Wno-address -mthumb -mfloat-abi=soft
DEFINE GCC_AARCH64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS)
-mlittle-endian -fno-short-enums -fverbose-asm -funsigned-char
-ffunction-sections -fdata-sections -fomit-frame-pointer -fno-builtin
-Wno-address -fno-asynchronous-unwind-tables
DEFINE GCC_AARCH64_CC_XIPFLAGS = -mstrict-align
DEFINE GCC_DLINK_FLAGS_COMMON = -nostdlib --pie
--
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[PATCH 3/3] BaseTools GCC/ARM: add -fno-builtin to CC flags

Ard Biesheuvel
 

Avoid build errors when including OpensslLib, which may throw
undefined reference errors for builtin functions if -fno-builtin
is not specified (and it is already set for IA32, X64 and AARCH64)
So set it for ARM as well.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
BaseTools/Conf/tools_def.template | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template
index 88af82a683d9..4f1dd4be378e 100644
--- a/BaseTools/Conf/tools_def.template
+++ b/BaseTools/Conf/tools_def.template
@@ -4334,7 +4334,7 @@ DEFINE GCC_ALL_CC_FLAGS = -g -Os -fshort-wchar -fno-strict-aliasing -
DEFINE GCC_IA32_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -m32 -malign-double -freorder-blocks -freorder-blocks-and-partition -O2 -mno-stack-arg-probe
DEFINE GCC_X64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mno-red-zone -Wno-address -mno-stack-arg-probe
DEFINE GCC_IPF_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -minline-int-divide-min-latency
-DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mlittle-endian -mabi=aapcs -fno-short-enums -funsigned-char -ffunction-sections -fdata-sections -fomit-frame-pointer -Wno-address -mthumb -mfloat-abi=soft
+DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mlittle-endian -mabi=aapcs -fno-short-enums -funsigned-char -ffunction-sections -fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address -mthumb -mfloat-abi=soft
DEFINE GCC_AARCH64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mlittle-endian -fno-short-enums -fverbose-asm -funsigned-char -ffunction-sections -fdata-sections -fomit-frame-pointer -fno-builtin -Wno-address -fno-asynchronous-unwind-tables
DEFINE GCC_AARCH64_CC_XIPFLAGS = -mstrict-align
DEFINE GCC_DLINK_FLAGS_COMMON = -nostdlib --pie
--
2.7.4


[PATCH 2/3] ArmPkg/CompilerIntrinsicsLib: make the default memset() weak

Ard Biesheuvel
 

The ARM compiler intrinsics library defines __aeabi_memset() and
memset() in the same object, which means that both will be pulled
in if either is referenced.

The IntrinsicLib in CryptoPkg defines its own, preferred memset(),
which may clash with our memset(). So make our version weak.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Library/CompilerIntrinsicsLib/Arm/memset.S | 8 ++++++++
1 file changed, 8 insertions(+)

diff --git a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/memset.S b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/memset.S
index bb75d7a70b80..65f6289b410b 100644
--- a/ArmPkg/Library/CompilerIntrinsicsLib/Arm/memset.S
+++ b/ArmPkg/Library/CompilerIntrinsicsLib/Arm/memset.S
@@ -40,6 +40,14 @@ ASM_PFX(__aeabi_memset):
# IN UINT32 Character,
# IN UINT32 Size
# );
+ //
+ // This object may be pulled in to satisfy an undefined reference to
+ // __aeabi_memset above, but in some cases, memset() is already provided
+ // by another library (i.e., CryptoPkg/IntrinsicLib), in which case we
+ // prefer the other version. So allow this one to be overridden by
+ // giving it weak linkage.
+ //
+ .weak memset
ASM_PFX(memset):
subs ip, r2, #0
bxeq lr
--
2.7.4


[PATCH 1/3] ArmPkg/ArmSoftFloatLib: disable LTO build for GCC

Ard Biesheuvel
 

Building ArmSoftFloatLib with LTO results in errors like

.../bin/ld: softfloat.obj: plugin needed to handle lto object
.../bin/ld: __aeabi_dcmpge.obj: plugin needed to handle lto object
.../bin/ld: __aeabi_dcmplt.obj: plugin needed to handle lto object
.../bin/ld: internal error ../../ld/ldlang.c 6299

This library is only linked by OpensslLib at the moment, and only
marginally used at runtime, so just disable LTO for it.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf b/ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
index f090b3f288ce..3c76381b25dc 100644
--- a/ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
+++ b/ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf
@@ -48,7 +48,7 @@ [Packages]
MdePkg/MdePkg.dec

[BuildOptions]
- GCC:*_*_*_CC_FLAGS = -DSOFTFLOAT_FOR_GCC -Wno-enum-compare
+ GCC:*_*_*_CC_FLAGS = -DSOFTFLOAT_FOR_GCC -Wno-enum-compare -fno-lto
*_GCC46_*_CC_FLAGS = -fno-tree-vrp
*_GCC47_*_CC_FLAGS = -fno-tree-vrp
RVCT:*_*_*_CC_FLAGS = -DSOFTFLOAT_FOR_GCC
--
2.7.4


[PATCH 0/3] Build fixes for ARM/OpenSSL

Ard Biesheuvel
 

This series addresses some issues that cause the build to be broken
for ARM GCC5 at the moment when including OpensslLib in the build.

Ard Biesheuvel (3):
ArmPkg/ArmSoftFloatLib: disable LTO build for GCC
ArmPkg/CompilerIntrinsicsLib: make the default memset() weak
BaseTools GCC/ARM: add -fno-builtin to CC flags

ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf | 2 +-
ArmPkg/Library/CompilerIntrinsicsLib/Arm/memset.S | 8 ++++++++
BaseTools/Conf/tools_def.template | 2 +-
3 files changed, 10 insertions(+), 2 deletions(-)

--
2.7.4


Re: [PATCH] PcdBdsLinuxSupport default value

Leif Lindholm <leif.lindholm@...>
 

On Wed, Aug 03, 2016 at 06:10:48PM -0500, Daniil Egranov wrote:
The built-in Linux Loader is obsolete and not included in most builds.
The patch sets the PcdBdsLinuxSupport default value to FALSE and prevents
ArmBds from looking for built-in Linux Loader by default and ASSERTing
when it cannot be found. Platforms which still using built-in loader have
to set this PCD in their platform description file.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Daniil Egranov <daniil.egranov@arm.com>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
Pushed (with a tweak to the subject line), thanks!

---
ArmPlatformPkg/ArmPlatformPkg.dec | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index 5864d98..d756fd2 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -55,7 +55,7 @@
gArmPlatformTokenSpaceGuid.PcdGopDisableOnExitBootServices|FALSE|BOOLEAN|0x0000003D

# Enable Legacy Linux support in the BDS
- gArmPlatformTokenSpaceGuid.PcdBdsLinuxSupport|TRUE|BOOLEAN|0x0000002E
+ gArmPlatformTokenSpaceGuid.PcdBdsLinuxSupport|FALSE|BOOLEAN|0x0000002E

[PcdsFixedAtBuild.common]
gArmPlatformTokenSpaceGuid.PcdCoreCount|1|UINT32|0x00000039
--
2.7.4


Re: [PATCH 3/3] SecurityPkg Tcg2: Remove internal implementation of IsZeroBuffer()

Zhang, Chao B <chao.b.zhang@...>
 

Reviewed-by: Chao Zhang<chao.b.zhang@intel.com>





Thanks & Best regards
Chao Zhang

-----Original Message-----
From: Wu, Hao A
Sent: Thursday, August 04, 2016 9:24 AM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A; Yao, Jiewen; Zhang, Chao B
Subject: [PATCH 3/3] SecurityPkg Tcg2: Remove internal implementation of IsZeroBuffer()

This commit removes the internal implementation of the function
IsZeroBuffer(). Instead, it will use the one from BaseMemoryLib.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c | 27 ---------------------------
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 27 ---------------------------
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 27 ---------------------------
3 files changed, 81 deletions(-)

diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
index db38bd4..5f4420c 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
@@ -617,33 +617,6 @@ FillBufferWithTCG2EventLogFormat (
}

/**
- Check if buffer is all zero.
-
- @param[in] Buffer Buffer to be checked.
- @param[in] BufferSize Size of buffer to be checked.
-
- @retval TRUE Buffer is all zero.
- @retval FALSE Buffer is not all zero.
-**/
-BOOLEAN
-IsZeroBuffer (
- IN VOID *Buffer,
- IN UINTN BufferSize
- )
-{
- UINT8 *BufferData;
- UINTN Index;
-
- BufferData = Buffer;
- for (Index = 0; Index < BufferSize; Index++) {
- if (BufferData[Index] != 0) {
- return FALSE;
- }
- }
- return TRUE;
-}
-
-/**
This function publish the TCG2 configuration Form for TPM device.

@param[in, out] PrivateData Points to TCG2 configuration private data.
diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index 95219c0..319f245 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -202,33 +202,6 @@ InternalDumpHex (
}

/**
- Check if buffer is all zero.
-
- @param[in] Buffer Buffer to be checked.
- @param[in] BufferSize Size of buffer to be checked.
-
- @retval TRUE Buffer is all zero.
- @retval FALSE Buffer is not all zero.
-**/
-BOOLEAN
-IsZeroBuffer (
- IN VOID *Buffer,
- IN UINTN BufferSize
- )
-{
- UINT8 *BufferData;
- UINTN Index;
-
- BufferData = Buffer;
- for (Index = 0; Index < BufferSize; Index++) {
- if (BufferData[Index] != 0) {
- return FALSE;
- }
- }
- return TRUE;
-}
-
-/**
Get All processors EFI_CPU_LOCATION in system. LocationBuf is allocated inside the function
Caller is responsible to free LocationBuf.

diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
index a830ba8..0d779f1 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
@@ -225,33 +225,6 @@ EndofPeiSignalNotifyCallBack (
}

/**
- Check if buffer is all zero.
-
- @param[in] Buffer Buffer to be checked.
- @param[in] BufferSize Size of buffer to be checked.
-
- @retval TRUE Buffer is all zero.
- @retval FALSE Buffer is not all zero.
-**/
-BOOLEAN
-IsZeroBuffer (
- IN VOID *Buffer,
- IN UINTN BufferSize
- )
-{
- UINT8 *BufferData;
- UINTN Index;
-
- BufferData = Buffer;
- for (Index = 0; Index < BufferSize; Index++) {
- if (BufferData[Index] != 0) {
- return FALSE;
- }
- }
- return TRUE;
-}
-
-/**
Get TPML_DIGEST_VALUES data size.

@param[in] DigestList TPML_DIGEST_VALUES data.
--
1.9.5.msysgit.0


Re: [PATCH] BaseTools X64: fold PLT relocations into simple relative references

Ard Biesheuvel
 

On 4 August 2016 at 10:58, Shi, Steven <steven.shi@intel.com> wrote:
OK, it is. But it is a bit not very clear.
Did you read the elaborate comment block explaining that (and why) it
is appropriate to treat R_X86_64_PLT32 as a R_X86_64_PC32 relocation?
This is not generally true, but it is true for UEFI since we don't
support shared libraries.

So I think it is incorrect to simply duplicate the code for
R_X86_64_PC32 without mentioning that, and suggesting that the PLT
relocation receive some kind of treatment that is different.

Thanks,
Ard.


Re: [PATCH] BaseTools X64: fold PLT relocations into simple relative references

Shi, Steven <steven.shi@...>
 

OK, it is. But it is a bit not very clear.





Steven Shi

Intel\SSG\STO\UEFI Firmware



Tel: +86 021-61166522

iNet: 821-6522

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Thursday, August 04, 2016 4:55 PM
To: Shi, Steven <steven.shi@intel.com>
Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; Gao, Liming
<liming.gao@intel.com>; Justen, Jordan L <jordan.l.justen@intel.com>;
edk2-devel@lists.01.org; mischief@offblast.org
Subject: Re: [PATCH] BaseTools X64: fold PLT relocations into simple relative
references
On 4 August 2016 at 10:54, Shi, Steven <steven.shi@intel.com<mailto:steven.shi@intel.com>> wrote:
Hi Ard,
I don't see you add below code for case R_X86_64_PLT32. Is it right?
*(UINT32 *)Targ = (UINT32) (*(UINT32 *)Targ
+ (mCoffSectionsOffset[Sym->st_shndx] - SymShdr->sh_addr)
- (SecOffset - SecShdr->sh_addr));
Isn't it identical to the code for R_X86_64_PC32?


Re: [PATCH] BaseTools X64: fold PLT relocations into simple relative references

Ard Biesheuvel
 

On 4 August 2016 at 10:54, Shi, Steven <steven.shi@intel.com> wrote:
Hi Ard,
I don't see you add below code for case R_X86_64_PLT32. Is it right?

*(UINT32 *)Targ = (UINT32) (*(UINT32 *)Targ
+ (mCoffSectionsOffset[Sym->st_shndx] - SymShdr->sh_addr)
- (SecOffset - SecShdr->sh_addr));
Isn't it identical to the code for R_X86_64_PC32?


Re: [PATCH] BaseTools X64: fold PLT relocations into simple relative references

Shi, Steven <steven.shi@...>
 

Hi Ard,
I don't see you add below code for case R_X86_64_PLT32. Is it right?

*(UINT32 *)Targ = (UINT32) (*(UINT32 *)Targ
+ (mCoffSectionsOffset[Sym->st_shndx] - SymShdr->sh_addr)
- (SecOffset - SecShdr->sh_addr));


Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522

-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Thursday, August 04, 2016 4:46 PM
To: Shi, Steven <steven.shi@intel.com>; Zhu, Yonghong
<yonghong.zhu@intel.com>; Gao, Liming <liming.gao@intel.com>; Justen,
Jordan L <jordan.l.justen@intel.com>; edk2-devel@lists.01.org
Cc: mischief@offblast.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH] BaseTools X64: fold PLT relocations into simple relative
references

For X64/GCC, we use position independent code with hidden visibility
to inform the compiler that symbols references are never resolved at
runtime, which removes the need for PLTs and GOTs. However, in some
cases GCC has been reported to still emit PLT based relocations, which
we need to handle in the ELF to PE/COFF perform by GenFw.

Unlike GOT based relocations, which are non-trivial to handle since the
indirections in the code can not be fixed up easily (although relocation
types exist for X64 that annotate relocation targets as suitable for
relaxation), PLT relocations simply point to jump targets, and we can
relax such relocations by resolving them using the symbol directly rather
than via a PLT entry that does nothing more than tail call the function
we already know it is going to call (since all symbol references are
resolved in the same module).

So handle R_X86_64_PLT32 as a R_X86_64_PC32 relocation.

Suggested-by: Steven Shi <steven.shi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
BaseTools/Source/C/GenFw/Elf64Convert.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c
b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 944c94b8f8b4..7cbff0df0996 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -785,6 +785,17 @@ WriteSections64 (
*(INT32 *)Targ = (INT32)((INT64)(*(INT32 *)Targ) - SymShdr->sh_addr
+ mCoffSectionsOffset[Sym->st_shndx]);
VerboseMsg ("Relocation: 0x%08X", *(UINT32*)Targ);
break;
+
+ case R_X86_64_PLT32:
+ //
+ // Treat R_X86_64_PLT32 relocations as R_X86_64_PC32: this is
+ // possible since we know all code symbol references resolve to
+ // definitions in the same module (UEFI has no shared libraries),
+ // and so there is never a reason to jump via a PLT entry,
+ // allowing us to resolve the reference using the symbol directly.
+ //
+ VerboseMsg ("Treating R_X86_64_PLT32 as R_X86_64_PC32 ...");
+ /* fall through */
case R_X86_64_PC32:
//
// Relative relocation: Symbol - Ip + Addend
--
2.7.4


[PATCH] BaseTools X64: fold PLT relocations into simple relative references

Ard Biesheuvel
 

For X64/GCC, we use position independent code with hidden visibility
to inform the compiler that symbols references are never resolved at
runtime, which removes the need for PLTs and GOTs. However, in some
cases GCC has been reported to still emit PLT based relocations, which
we need to handle in the ELF to PE/COFF perform by GenFw.

Unlike GOT based relocations, which are non-trivial to handle since the
indirections in the code can not be fixed up easily (although relocation
types exist for X64 that annotate relocation targets as suitable for
relaxation), PLT relocations simply point to jump targets, and we can
relax such relocations by resolving them using the symbol directly rather
than via a PLT entry that does nothing more than tail call the function
we already know it is going to call (since all symbol references are
resolved in the same module).

So handle R_X86_64_PLT32 as a R_X86_64_PC32 relocation.

Suggested-by: Steven Shi <steven.shi@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
BaseTools/Source/C/GenFw/Elf64Convert.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/BaseTools/Source/C/GenFw/Elf64Convert.c b/BaseTools/Source/C/GenFw/Elf64Convert.c
index 944c94b8f8b4..7cbff0df0996 100644
--- a/BaseTools/Source/C/GenFw/Elf64Convert.c
+++ b/BaseTools/Source/C/GenFw/Elf64Convert.c
@@ -785,6 +785,17 @@ WriteSections64 (
*(INT32 *)Targ = (INT32)((INT64)(*(INT32 *)Targ) - SymShdr->sh_addr + mCoffSectionsOffset[Sym->st_shndx]);
VerboseMsg ("Relocation: 0x%08X", *(UINT32*)Targ);
break;
+
+ case R_X86_64_PLT32:
+ //
+ // Treat R_X86_64_PLT32 relocations as R_X86_64_PC32: this is
+ // possible since we know all code symbol references resolve to
+ // definitions in the same module (UEFI has no shared libraries),
+ // and so there is never a reason to jump via a PLT entry,
+ // allowing us to resolve the reference using the symbol directly.
+ //
+ VerboseMsg ("Treating R_X86_64_PLT32 as R_X86_64_PC32 ...");
+ /* fall through */
case R_X86_64_PC32:
//
// Relative relocation: Symbol - Ip + Addend
--
2.7.4


Re: [PATCH 0/3] Add APIs IsZeroBuffer and IsZeroGuid in BaseMemoryLib

Wu, Hao A
 

-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo
Ersek
Sent: Thursday, August 04, 2016 4:07 PM
To: Wu, Hao A
Cc: edk2-devel@ml01.01.org; Leif Lindholm (Linaro address); Ard Biesheuvel
Subject: Re: [edk2] [PATCH 0/3] Add APIs IsZeroBuffer and IsZeroGuid in
BaseMemoryLib

Hello Hao,

On 08/04/16 03:24, Hao Wu wrote:
The patch series will add two APIs in BaseMemoryLib:
1. IsZeroBuffer()
The API is used to check if the contents of a buffer are all zeros.

2. IsZeroGuid()
The API is used to check if the given GUID is a zero GUID.

In order to resolve build issues in SecurityPkg, the series will also
remove the internal implementation of IsZeroBuffer() in modules within
SecurityPkg\Tcg and use the one in BaseMemoryLib instead.
(1) Do you plan to add optimized implementations of IsZeroBuffer() later
on?

For example, in QEMU, the buffer_is_zero() function has optimized
implementations for:
- AVX2
- SSE2
- AArch64

I see one of the library instances is called BaseMemoryLibSse2, so an
SSE2 optimized implementation could be possible.

Also, as far as I understand the example in the QEMU code, for AArch64,
the optimized implementation could go in all of the library instances
that build on AArch64. (QEMU calls the vgetq_lane_u64() function -- I'm
unsure how it would be available in edk2. Perhaps AArch64 assembly would
be necessary.)

Just an idea, of course.
I will hold this patch series and do more research on the optimized
implementations of IsZeroBuffer() for SSE2 and other library instances.


(2) The edk2 tree contains a number of zero GUID comparisons:

$ git grep -i -e zeroguid --and -e compareguid

BaseTools/Source/C/GenFfs/GenFfs.c:749: if (CompareGuid (&FileGuid,
&mZeroGuid) == 0) {
BaseTools/Source/C/GenFv/GenFvInternalLib.c:4134: if (CompareGuid
(&mCapDataInfo.CapGuid, &mZeroGuid) == 0) {
BaseTools/Source/C/GenSec/GenSec.c:854: if (CompareGuid (VendorGuid,
&mZeroGuid) == 0) {
BaseTools/Source/C/GenSec/GenSec.c:900: if (CompareGuid (VendorGuid,
&mZeroGuid) == 0) {
BaseTools/Source/C/GenSec/GenSec.c:1368: if ((SectType !=
EFI_SECTION_GUID_DEFINED) && (CompareGuid (&VendorGuid,
&mZeroGuid) != 0)) {
BaseTools/Source/C/GenSec/GenSec.c:1421: if (InputFileAlign != NULL &&
(CompareGuid (&VendorGuid, &mZeroGuid) != 0)) {
EdkCompatibilityPkg/Compatibility/FrameworkHiiOnUefiHiiThunk/Utility.c:717:
if (FormSetGuid == NULL || CompareGuid (FormSetGuid, &gZeroGuid)) {
EdkCompatibilityPkg/Compatibility/PiSmbiosRecordOnDataHubSmbiosRecordT
hunk/Translate.c:85: for (; !CompareGuid
(&(mConversionTable[Index].SubClass), &gZeroGuid); Index++) {
EdkCompatibilityPkg/Compatibility/PiSmbiosRecordOnDataHubSmbiosRecordT
hunk/Translate.c:96: if (CompareGuid (&(mConversionTable[Index].SubClass),
&gZeroGuid)) {
EdkCompatibilityPkg/Sample/Tools/Source/GenAprioriFile/GenAprioriFile.c:196:
if (CompareGuid (&GuidIn, &ZeroGuid) != 0) {
EdkCompatibilityPkg/Sample/Tools/Source/GenFfsFile/GenFfsFile.c:1328:
if (CompareGuid (&SignGuid, &mZeroGuid) != 0) {
EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c:1348: if (CompareGuid (&File-
FvNameGuid, &gZeroGuid)) {
IntelFrameworkModulePkg/Universal/DataHubDxe/DataHub.c:142:
(CompareGuid (&FilterEntry->FilterDataRecordGuid, &gZeroGuid) ||
MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c:258: if
(!CompareGuid (&DriverInfo->FileName, &gZeroGuid)) {
MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c:449: if
(CompareGuid (PcdGetPtr (PcdDriverHealthConfigureForm), &gZeroGuid)) {
MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGenFromFv.c:375: for
(Index = 0; !CompareGuid (&DriverGuidArray[Index], &gZeroGuid); Index++) {
MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGenFromFv.c:424: if
(CompareGuid (&DriverGuidArray[0], &gZeroGuid)) {
MdeModulePkg/Universal/SetupBrowserDxe/Expression.c:2832: } else if
(CompareGuid (&OpCode->Guid, &gZeroGuid) != 0) {
MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c:361: if
(!CompareGuid (&Statement->RefreshGuid, &gZeroGuid)) {
MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c:376: if
((!CompareGuid (&Statement->RefreshGuid, &gZeroGuid)) || (Statement-
RefreshInterval != 0)) {
MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c:631: if
(!CompareGuid (&gCurrentSelection->Form->RefreshGuid, &gZeroGuid)) {
MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c:1413: } else if
(!CompareGuid (&Statement->HiiValue.Value.ref.FormSetGuid, &gZeroGuid)) {
MdeModulePkg/Universal/SetupBrowserDxe/Setup.c:184: if
(CompareGuid (&MenuList->FormSetGuid, &gZeroGuid)) {
MdeModulePkg/Universal/SetupBrowserDxe/Setup.c:5659: if
(CompareGuid (FormSetGuid, &gZeroGuid) ||
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c:376: if
(CompareGuid (&VendorGuid, &gZeroGuid)) {
SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c:205:
if (!CompareGuid (&PartitionEntry->PartitionTypeGUID, &gZeroGuid)) {
SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c:241:
if (!CompareGuid (&PartitionEntry->PartitionTypeGUID, &gZeroGuid)) {
SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c:205:
if (!CompareGuid (&PartitionEntry->PartitionTypeGUID, &gZeroGuid)) {
SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c:239:
if (!CompareGuid (&PartitionEntry->PartitionTypeGUID, &gZeroGuid)) {

Do you plan to migrate these source files to the new IsZeroGuid()
function?

(There might be more -- "zeroguid" and "compareguid" could be on
different lines, so perhaps it's better to search for just "zeroguid",
and audit each location separately.)
Yes, we plan to replace the usages of "compareguid with zeroguid" with
the new API. It will be done independently by another patch later.

For the above-listed results, I think we won't handle the cases used in
tools. Since these codes won't be able to use the BaseMemoryLib.


(3) I think patch #3 should be implemented differently -- I believe the
current series can break bisection between patch #2 and patch #3.

I suggest to first rename the IsZeroBuffer() functions in
SecurityPkg/Tcg to IsZeroBufferInternal(), as patch #1.

Then add IsZeroBuffer() and IsZeroGuid() to the BaseMemoryLib instances,
as patch #2 and patch #3.

Finally, switch SecurityPkg/Tcg from IsZeroBufferInternal() to
IsZeroBuffer() in patch #4.

What do you think?
Yes, I will follow this approach when sending out the next version of patch.

Best Regards,
Hao Wu


Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [PATCH 0/3] Add APIs IsZeroBuffer and IsZeroGuid in BaseMemoryLib

Laszlo Ersek
 

Hello Hao,

On 08/04/16 03:24, Hao Wu wrote:
The patch series will add two APIs in BaseMemoryLib:
1. IsZeroBuffer()
The API is used to check if the contents of a buffer are all zeros.

2. IsZeroGuid()
The API is used to check if the given GUID is a zero GUID.

In order to resolve build issues in SecurityPkg, the series will also
remove the internal implementation of IsZeroBuffer() in modules within
SecurityPkg\Tcg and use the one in BaseMemoryLib instead.
(1) Do you plan to add optimized implementations of IsZeroBuffer() later
on?

For example, in QEMU, the buffer_is_zero() function has optimized
implementations for:
- AVX2
- SSE2
- AArch64

I see one of the library instances is called BaseMemoryLibSse2, so an
SSE2 optimized implementation could be possible.

Also, as far as I understand the example in the QEMU code, for AArch64,
the optimized implementation could go in all of the library instances
that build on AArch64. (QEMU calls the vgetq_lane_u64() function -- I'm
unsure how it would be available in edk2. Perhaps AArch64 assembly would
be necessary.)

Just an idea, of course.

(2) The edk2 tree contains a number of zero GUID comparisons:

$ git grep -i -e zeroguid --and -e compareguid

BaseTools/Source/C/GenFfs/GenFfs.c:749: if (CompareGuid (&FileGuid, &mZeroGuid) == 0) {
BaseTools/Source/C/GenFv/GenFvInternalLib.c:4134: if (CompareGuid (&mCapDataInfo.CapGuid, &mZeroGuid) == 0) {
BaseTools/Source/C/GenSec/GenSec.c:854: if (CompareGuid (VendorGuid, &mZeroGuid) == 0) {
BaseTools/Source/C/GenSec/GenSec.c:900: if (CompareGuid (VendorGuid, &mZeroGuid) == 0) {
BaseTools/Source/C/GenSec/GenSec.c:1368: if ((SectType != EFI_SECTION_GUID_DEFINED) && (CompareGuid (&VendorGuid, &mZeroGuid) != 0)) {
BaseTools/Source/C/GenSec/GenSec.c:1421: if (InputFileAlign != NULL && (CompareGuid (&VendorGuid, &mZeroGuid) != 0)) {
EdkCompatibilityPkg/Compatibility/FrameworkHiiOnUefiHiiThunk/Utility.c:717: if (FormSetGuid == NULL || CompareGuid (FormSetGuid, &gZeroGuid)) {
EdkCompatibilityPkg/Compatibility/PiSmbiosRecordOnDataHubSmbiosRecordThunk/Translate.c:85: for (; !CompareGuid (&(mConversionTable[Index].SubClass), &gZeroGuid); Index++) {
EdkCompatibilityPkg/Compatibility/PiSmbiosRecordOnDataHubSmbiosRecordThunk/Translate.c:96: if (CompareGuid (&(mConversionTable[Index].SubClass), &gZeroGuid)) {
EdkCompatibilityPkg/Sample/Tools/Source/GenAprioriFile/GenAprioriFile.c:196: if (CompareGuid (&GuidIn, &ZeroGuid) != 0) {
EdkCompatibilityPkg/Sample/Tools/Source/GenFfsFile/GenFfsFile.c:1328: if (CompareGuid (&SignGuid, &mZeroGuid) != 0) {
EmbeddedPkg/Library/EfiFileLib/EfiFileLib.c:1348: if (CompareGuid (&File->FvNameGuid, &gZeroGuid)) {
IntelFrameworkModulePkg/Universal/DataHubDxe/DataHub.c:142: (CompareGuid (&FilterEntry->FilterDataRecordGuid, &gZeroGuid) ||
MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c:258: if (!CompareGuid (&DriverInfo->FileName, &gZeroGuid)) {
MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c:449: if (CompareGuid (PcdGetPtr (PcdDriverHealthConfigureForm), &gZeroGuid)) {
MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGenFromFv.c:375: for (Index = 0; !CompareGuid (&DriverGuidArray[Index], &gZeroGuid); Index++) {
MdeModulePkg/Library/VarCheckHiiLib/VarCheckHiiGenFromFv.c:424: if (CompareGuid (&DriverGuidArray[0], &gZeroGuid)) {
MdeModulePkg/Universal/SetupBrowserDxe/Expression.c:2832: } else if (CompareGuid (&OpCode->Guid, &gZeroGuid) != 0) {
MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c:361: if (!CompareGuid (&Statement->RefreshGuid, &gZeroGuid)) {
MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c:376: if ((!CompareGuid (&Statement->RefreshGuid, &gZeroGuid)) || (Statement->RefreshInterval != 0)) {
MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c:631: if (!CompareGuid (&gCurrentSelection->Form->RefreshGuid, &gZeroGuid)) {
MdeModulePkg/Universal/SetupBrowserDxe/Presentation.c:1413: } else if (!CompareGuid (&Statement->HiiValue.Value.ref.FormSetGuid, &gZeroGuid)) {
MdeModulePkg/Universal/SetupBrowserDxe/Setup.c:184: if (CompareGuid (&MenuList->FormSetGuid, &gZeroGuid)) {
MdeModulePkg/Universal/SetupBrowserDxe/Setup.c:5659: if (CompareGuid (FormSetGuid, &gZeroGuid) ||
MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c:376: if (CompareGuid (&VendorGuid, &gZeroGuid)) {
SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c:205: if (!CompareGuid (&PartitionEntry->PartitionTypeGUID, &gZeroGuid)) {
SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.c:241: if (!CompareGuid (&PartitionEntry->PartitionTypeGUID, &gZeroGuid)) {
SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c:205: if (!CompareGuid (&PartitionEntry->PartitionTypeGUID, &gZeroGuid)) {
SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c:239: if (!CompareGuid (&PartitionEntry->PartitionTypeGUID, &gZeroGuid)) {
Do you plan to migrate these source files to the new IsZeroGuid()
function?

(There might be more -- "zeroguid" and "compareguid" could be on
different lines, so perhaps it's better to search for just "zeroguid",
and audit each location separately.)

(3) I think patch #3 should be implemented differently -- I believe the
current series can break bisection between patch #2 and patch #3.

I suggest to first rename the IsZeroBuffer() functions in
SecurityPkg/Tcg to IsZeroBufferInternal(), as patch #1.

Then add IsZeroBuffer() and IsZeroGuid() to the BaseMemoryLib instances,
as patch #2 and patch #3.

Finally, switch SecurityPkg/Tcg from IsZeroBufferInternal() to
IsZeroBuffer() in patch #4.

What do you think?

Thanks
Laszlo


[PATCH 3/3] SecurityPkg Tcg2: Remove internal implementation of IsZeroBuffer()

Wu, Hao A
 

This commit removes the internal implementation of the function
IsZeroBuffer(). Instead, it will use the one from BaseMemoryLib.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Chao Zhang <chao.b.zhang@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c | 27 ---------------------------
SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 27 ---------------------------
SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 27 ---------------------------
3 files changed, 81 deletions(-)

diff --git a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
index db38bd4..5f4420c 100644
--- a/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
+++ b/SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.c
@@ -617,33 +617,6 @@ FillBufferWithTCG2EventLogFormat (
}

/**
- Check if buffer is all zero.
-
- @param[in] Buffer Buffer to be checked.
- @param[in] BufferSize Size of buffer to be checked.
-
- @retval TRUE Buffer is all zero.
- @retval FALSE Buffer is not all zero.
-**/
-BOOLEAN
-IsZeroBuffer (
- IN VOID *Buffer,
- IN UINTN BufferSize
- )
-{
- UINT8 *BufferData;
- UINTN Index;
-
- BufferData = Buffer;
- for (Index = 0; Index < BufferSize; Index++) {
- if (BufferData[Index] != 0) {
- return FALSE;
- }
- }
- return TRUE;
-}
-
-/**
This function publish the TCG2 configuration Form for TPM device.

@param[in, out] PrivateData Points to TCG2 configuration private data.
diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
index 95219c0..319f245 100644
--- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
+++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c
@@ -202,33 +202,6 @@ InternalDumpHex (
}

/**
- Check if buffer is all zero.
-
- @param[in] Buffer Buffer to be checked.
- @param[in] BufferSize Size of buffer to be checked.
-
- @retval TRUE Buffer is all zero.
- @retval FALSE Buffer is not all zero.
-**/
-BOOLEAN
-IsZeroBuffer (
- IN VOID *Buffer,
- IN UINTN BufferSize
- )
-{
- UINT8 *BufferData;
- UINTN Index;
-
- BufferData = Buffer;
- for (Index = 0; Index < BufferSize; Index++) {
- if (BufferData[Index] != 0) {
- return FALSE;
- }
- }
- return TRUE;
-}
-
-/**
Get All processors EFI_CPU_LOCATION in system. LocationBuf is allocated inside the function
Caller is responsible to free LocationBuf.

diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
index a830ba8..0d779f1 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
@@ -225,33 +225,6 @@ EndofPeiSignalNotifyCallBack (
}

/**
- Check if buffer is all zero.
-
- @param[in] Buffer Buffer to be checked.
- @param[in] BufferSize Size of buffer to be checked.
-
- @retval TRUE Buffer is all zero.
- @retval FALSE Buffer is not all zero.
-**/
-BOOLEAN
-IsZeroBuffer (
- IN VOID *Buffer,
- IN UINTN BufferSize
- )
-{
- UINT8 *BufferData;
- UINTN Index;
-
- BufferData = Buffer;
- for (Index = 0; Index < BufferSize; Index++) {
- if (BufferData[Index] != 0) {
- return FALSE;
- }
- }
- return TRUE;
-}
-
-/**
Get TPML_DIGEST_VALUES data size.

@param[in] DigestList TPML_DIGEST_VALUES data.
--
1.9.5.msysgit.0


[PATCH 2/3] MdePkg BaseMemoryLib: Add implementation of API IsZeroBuffer()

Wu, Hao A
 

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Hao Wu <hao.a.wu@intel.com>
---
MdePkg/Include/Library/BaseMemoryLib.h | 23 ++++++++
MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf | 1 +
MdePkg/Library/BaseMemoryLib/IsZeroBuffer.c | 65 ++++++++++++++++++++++
.../Library/BaseMemoryLibMmx/BaseMemoryLibMmx.inf | 1 +
MdePkg/Library/BaseMemoryLibMmx/IsZeroBuffer.c | 65 ++++++++++++++++++++++
.../BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf | 2 +
MdePkg/Library/BaseMemoryLibOptDxe/IsZeroBuffer.c | 65 ++++++++++++++++++++++
.../BaseMemoryLibOptPei/BaseMemoryLibOptPei.inf | 2 +
MdePkg/Library/BaseMemoryLibOptPei/IsZeroBuffer.c | 65 ++++++++++++++++++++++
.../BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf | 1 +
MdePkg/Library/BaseMemoryLibRepStr/IsZeroBuffer.c | 65 ++++++++++++++++++++++
.../BaseMemoryLibSse2/BaseMemoryLibSse2.inf | 1 +
MdePkg/Library/BaseMemoryLibSse2/IsZeroBuffer.c | 65 ++++++++++++++++++++++
MdePkg/Library/PeiMemoryLib/IsZeroBuffer.c | 65 ++++++++++++++++++++++
MdePkg/Library/PeiMemoryLib/PeiMemoryLib.inf | 1 +
MdePkg/Library/UefiMemoryLib/IsZeroBuffer.c | 65 ++++++++++++++++++++++
MdePkg/Library/UefiMemoryLib/UefiMemoryLib.inf | 1 +
17 files changed, 553 insertions(+)
create mode 100644 MdePkg/Library/BaseMemoryLib/IsZeroBuffer.c
create mode 100644 MdePkg/Library/BaseMemoryLibMmx/IsZeroBuffer.c
create mode 100644 MdePkg/Library/BaseMemoryLibOptDxe/IsZeroBuffer.c
create mode 100644 MdePkg/Library/BaseMemoryLibOptPei/IsZeroBuffer.c
create mode 100644 MdePkg/Library/BaseMemoryLibRepStr/IsZeroBuffer.c
create mode 100644 MdePkg/Library/BaseMemoryLibSse2/IsZeroBuffer.c
create mode 100644 MdePkg/Library/PeiMemoryLib/IsZeroBuffer.c
create mode 100644 MdePkg/Library/UefiMemoryLib/IsZeroBuffer.c

diff --git a/MdePkg/Include/Library/BaseMemoryLib.h b/MdePkg/Include/Library/BaseMemoryLib.h
index 5a57dee..1338c27 100644
--- a/MdePkg/Include/Library/BaseMemoryLib.h
+++ b/MdePkg/Include/Library/BaseMemoryLib.h
@@ -463,4 +463,27 @@ IsZeroGuid (
IN CONST GUID *Guid
);

+/**
+ Checks if the contents of a buffer are all zeros.
+
+ This function checks whether the contents of a buffer are all zeros. If the
+ contents are all zeros, return TRUE. Otherwise, return FALSE.
+
+ If Length > 0 and Buffer is NULL, then ASSERT().
+ If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+ @param Buffer The pointer to the buffer to be checked.
+ @param Length The size of the buffer (in bytes) to be checked.
+
+ @retval TRUE Contents of the buffer are all zeros.
+ @retval FALSE Contents of the buffer are not all zeros.
+
+**/
+BOOLEAN
+EFIAPI
+IsZeroBuffer (
+ IN CONST VOID *Buffer,
+ IN UINTN Length
+ );
+
#endif
diff --git a/MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf b/MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
index 5dfab35..51b4d16 100644
--- a/MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+++ b/MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
@@ -45,6 +45,7 @@
MemLibGeneric.c
MemLibGuid.c
CopyMem.c
+ IsZeroBuffer.c
MemLibInternals.h

[Packages]
diff --git a/MdePkg/Library/BaseMemoryLib/IsZeroBuffer.c b/MdePkg/Library/BaseMemoryLib/IsZeroBuffer.c
new file mode 100644
index 0000000..fda0c7c
--- /dev/null
+++ b/MdePkg/Library/BaseMemoryLib/IsZeroBuffer.c
@@ -0,0 +1,65 @@
+/** @file
+ Implementation of IsZeroBuffer function.
+
+ The following BaseMemoryLib instances contain the same copy of this file:
+
+ BaseMemoryLib
+ BaseMemoryLibMmx
+ BaseMemoryLibSse2
+ BaseMemoryLibRepStr
+ BaseMemoryLibOptDxe
+ BaseMemoryLibOptPei
+ PeiMemoryLib
+ UefiMemoryLib
+
+ Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Base.h>
+#include <Library/DebugLib.h>
+
+/**
+ Checks if the contents of a buffer are all zeros.
+
+ This function checks whether the contents of a buffer are all zeros. If the
+ contents are all zeros, return TRUE. Otherwise, return FALSE.
+
+ If Length > 0 and Buffer is NULL, then ASSERT().
+ If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+ @param Buffer The pointer to the buffer to be checked.
+ @param Length The size of the buffer (in bytes) to be checked.
+
+ @retval TRUE Contents of the buffer are all zeros.
+ @retval FALSE Contents of the buffer are not all zeros.
+
+**/
+BOOLEAN
+EFIAPI
+IsZeroBuffer (
+ IN CONST VOID *Buffer,
+ IN UINTN Length
+ )
+{
+ CONST UINT8 *BufferData;
+ UINTN Index;
+
+ ASSERT (!(Buffer == NULL && Length > 0));
+ ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Buffer));
+
+ BufferData = Buffer;
+ for (Index = 0; Index < Length; Index++) {
+ if (BufferData[Index] != 0) {
+ return FALSE;
+ }
+ }
+ return TRUE;
+}
diff --git a/MdePkg/Library/BaseMemoryLibMmx/BaseMemoryLibMmx.inf b/MdePkg/Library/BaseMemoryLibMmx/BaseMemoryLibMmx.inf
index a609073..59261f9 100644
--- a/MdePkg/Library/BaseMemoryLibMmx/BaseMemoryLibMmx.inf
+++ b/MdePkg/Library/BaseMemoryLibMmx/BaseMemoryLibMmx.inf
@@ -47,6 +47,7 @@
SetMemWrapper.c
CopyMemWrapper.c
MemLibGuid.c
+ IsZeroBuffer.c
MemLibInternals.h

[Sources.Ia32]
diff --git a/MdePkg/Library/BaseMemoryLibMmx/IsZeroBuffer.c b/MdePkg/Library/BaseMemoryLibMmx/IsZeroBuffer.c
new file mode 100644
index 0000000..fda0c7c
--- /dev/null
+++ b/MdePkg/Library/BaseMemoryLibMmx/IsZeroBuffer.c
@@ -0,0 +1,65 @@
+/** @file
+ Implementation of IsZeroBuffer function.
+
+ The following BaseMemoryLib instances contain the same copy of this file:
+
+ BaseMemoryLib
+ BaseMemoryLibMmx
+ BaseMemoryLibSse2
+ BaseMemoryLibRepStr
+ BaseMemoryLibOptDxe
+ BaseMemoryLibOptPei
+ PeiMemoryLib
+ UefiMemoryLib
+
+ Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Base.h>
+#include <Library/DebugLib.h>
+
+/**
+ Checks if the contents of a buffer are all zeros.
+
+ This function checks whether the contents of a buffer are all zeros. If the
+ contents are all zeros, return TRUE. Otherwise, return FALSE.
+
+ If Length > 0 and Buffer is NULL, then ASSERT().
+ If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+ @param Buffer The pointer to the buffer to be checked.
+ @param Length The size of the buffer (in bytes) to be checked.
+
+ @retval TRUE Contents of the buffer are all zeros.
+ @retval FALSE Contents of the buffer are not all zeros.
+
+**/
+BOOLEAN
+EFIAPI
+IsZeroBuffer (
+ IN CONST VOID *Buffer,
+ IN UINTN Length
+ )
+{
+ CONST UINT8 *BufferData;
+ UINTN Index;
+
+ ASSERT (!(Buffer == NULL && Length > 0));
+ ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Buffer));
+
+ BufferData = Buffer;
+ for (Index = 0; Index < Length; Index++) {
+ if (BufferData[Index] != 0) {
+ return FALSE;
+ }
+ }
+ return TRUE;
+}
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
index e637034..a75270b 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
@@ -90,6 +90,7 @@
SetMemWrapper.c
CopyMemWrapper.c
MemLibGuid.c
+ IsZeroBuffer.c

[Sources.X64]
X64/ScanMem64.nasm
@@ -137,6 +138,7 @@
SetMemWrapper.c
CopyMemWrapper.c
MemLibGuid.c
+ IsZeroBuffer.c

[Packages]
MdePkg/MdePkg.dec
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/IsZeroBuffer.c b/MdePkg/Library/BaseMemoryLibOptDxe/IsZeroBuffer.c
new file mode 100644
index 0000000..fda0c7c
--- /dev/null
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/IsZeroBuffer.c
@@ -0,0 +1,65 @@
+/** @file
+ Implementation of IsZeroBuffer function.
+
+ The following BaseMemoryLib instances contain the same copy of this file:
+
+ BaseMemoryLib
+ BaseMemoryLibMmx
+ BaseMemoryLibSse2
+ BaseMemoryLibRepStr
+ BaseMemoryLibOptDxe
+ BaseMemoryLibOptPei
+ PeiMemoryLib
+ UefiMemoryLib
+
+ Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Base.h>
+#include <Library/DebugLib.h>
+
+/**
+ Checks if the contents of a buffer are all zeros.
+
+ This function checks whether the contents of a buffer are all zeros. If the
+ contents are all zeros, return TRUE. Otherwise, return FALSE.
+
+ If Length > 0 and Buffer is NULL, then ASSERT().
+ If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+ @param Buffer The pointer to the buffer to be checked.
+ @param Length The size of the buffer (in bytes) to be checked.
+
+ @retval TRUE Contents of the buffer are all zeros.
+ @retval FALSE Contents of the buffer are not all zeros.
+
+**/
+BOOLEAN
+EFIAPI
+IsZeroBuffer (
+ IN CONST VOID *Buffer,
+ IN UINTN Length
+ )
+{
+ CONST UINT8 *BufferData;
+ UINTN Index;
+
+ ASSERT (!(Buffer == NULL && Length > 0));
+ ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Buffer));
+
+ BufferData = Buffer;
+ for (Index = 0; Index < Length; Index++) {
+ if (BufferData[Index] != 0) {
+ return FALSE;
+ }
+ }
+ return TRUE;
+}
diff --git a/MdePkg/Library/BaseMemoryLibOptPei/BaseMemoryLibOptPei.inf b/MdePkg/Library/BaseMemoryLibOptPei/BaseMemoryLibOptPei.inf
index ff60e9e..bd915eb 100644
--- a/MdePkg/Library/BaseMemoryLibOptPei/BaseMemoryLibOptPei.inf
+++ b/MdePkg/Library/BaseMemoryLibOptPei/BaseMemoryLibOptPei.inf
@@ -90,6 +90,7 @@
SetMemWrapper.c
CopyMemWrapper.c
MemLibGuid.c
+ IsZeroBuffer.c

[Sources.X64]
X64/ScanMem64.nasm
@@ -137,6 +138,7 @@
SetMemWrapper.c
CopyMemWrapper.c
MemLibGuid.c
+ IsZeroBuffer.c


[Packages]
diff --git a/MdePkg/Library/BaseMemoryLibOptPei/IsZeroBuffer.c b/MdePkg/Library/BaseMemoryLibOptPei/IsZeroBuffer.c
new file mode 100644
index 0000000..fda0c7c
--- /dev/null
+++ b/MdePkg/Library/BaseMemoryLibOptPei/IsZeroBuffer.c
@@ -0,0 +1,65 @@
+/** @file
+ Implementation of IsZeroBuffer function.
+
+ The following BaseMemoryLib instances contain the same copy of this file:
+
+ BaseMemoryLib
+ BaseMemoryLibMmx
+ BaseMemoryLibSse2
+ BaseMemoryLibRepStr
+ BaseMemoryLibOptDxe
+ BaseMemoryLibOptPei
+ PeiMemoryLib
+ UefiMemoryLib
+
+ Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Base.h>
+#include <Library/DebugLib.h>
+
+/**
+ Checks if the contents of a buffer are all zeros.
+
+ This function checks whether the contents of a buffer are all zeros. If the
+ contents are all zeros, return TRUE. Otherwise, return FALSE.
+
+ If Length > 0 and Buffer is NULL, then ASSERT().
+ If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+ @param Buffer The pointer to the buffer to be checked.
+ @param Length The size of the buffer (in bytes) to be checked.
+
+ @retval TRUE Contents of the buffer are all zeros.
+ @retval FALSE Contents of the buffer are not all zeros.
+
+**/
+BOOLEAN
+EFIAPI
+IsZeroBuffer (
+ IN CONST VOID *Buffer,
+ IN UINTN Length
+ )
+{
+ CONST UINT8 *BufferData;
+ UINTN Index;
+
+ ASSERT (!(Buffer == NULL && Length > 0));
+ ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Buffer));
+
+ BufferData = Buffer;
+ for (Index = 0; Index < Length; Index++) {
+ if (BufferData[Index] != 0) {
+ return FALSE;
+ }
+ }
+ return TRUE;
+}
diff --git a/MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf b/MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
index 9d7ce4b..aac7865 100644
--- a/MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
+++ b/MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf
@@ -44,6 +44,7 @@
SetMemWrapper.c
CopyMemWrapper.c
MemLibGuid.c
+ IsZeroBuffer.c

[Sources.Ia32]
Ia32/ScanMem64.nasm
diff --git a/MdePkg/Library/BaseMemoryLibRepStr/IsZeroBuffer.c b/MdePkg/Library/BaseMemoryLibRepStr/IsZeroBuffer.c
new file mode 100644
index 0000000..fda0c7c
--- /dev/null
+++ b/MdePkg/Library/BaseMemoryLibRepStr/IsZeroBuffer.c
@@ -0,0 +1,65 @@
+/** @file
+ Implementation of IsZeroBuffer function.
+
+ The following BaseMemoryLib instances contain the same copy of this file:
+
+ BaseMemoryLib
+ BaseMemoryLibMmx
+ BaseMemoryLibSse2
+ BaseMemoryLibRepStr
+ BaseMemoryLibOptDxe
+ BaseMemoryLibOptPei
+ PeiMemoryLib
+ UefiMemoryLib
+
+ Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Base.h>
+#include <Library/DebugLib.h>
+
+/**
+ Checks if the contents of a buffer are all zeros.
+
+ This function checks whether the contents of a buffer are all zeros. If the
+ contents are all zeros, return TRUE. Otherwise, return FALSE.
+
+ If Length > 0 and Buffer is NULL, then ASSERT().
+ If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+ @param Buffer The pointer to the buffer to be checked.
+ @param Length The size of the buffer (in bytes) to be checked.
+
+ @retval TRUE Contents of the buffer are all zeros.
+ @retval FALSE Contents of the buffer are not all zeros.
+
+**/
+BOOLEAN
+EFIAPI
+IsZeroBuffer (
+ IN CONST VOID *Buffer,
+ IN UINTN Length
+ )
+{
+ CONST UINT8 *BufferData;
+ UINTN Index;
+
+ ASSERT (!(Buffer == NULL && Length > 0));
+ ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Buffer));
+
+ BufferData = Buffer;
+ for (Index = 0; Index < Length; Index++) {
+ if (BufferData[Index] != 0) {
+ return FALSE;
+ }
+ }
+ return TRUE;
+}
diff --git a/MdePkg/Library/BaseMemoryLibSse2/BaseMemoryLibSse2.inf b/MdePkg/Library/BaseMemoryLibSse2/BaseMemoryLibSse2.inf
index a78d823..2919827 100644
--- a/MdePkg/Library/BaseMemoryLibSse2/BaseMemoryLibSse2.inf
+++ b/MdePkg/Library/BaseMemoryLibSse2/BaseMemoryLibSse2.inf
@@ -43,6 +43,7 @@
SetMemWrapper.c
CopyMemWrapper.c
MemLibGuid.c
+ IsZeroBuffer.c

[Sources.Ia32]
Ia32/ScanMem64.nasm
diff --git a/MdePkg/Library/BaseMemoryLibSse2/IsZeroBuffer.c b/MdePkg/Library/BaseMemoryLibSse2/IsZeroBuffer.c
new file mode 100644
index 0000000..6a3a2c1
--- /dev/null
+++ b/MdePkg/Library/BaseMemoryLibSse2/IsZeroBuffer.c
@@ -0,0 +1,65 @@
+/** @file
+ Implementation of IsZeroBuffer function.
+
+ The following BaseMemoryLib instances contain the same copy of this file:
+
+ BaseMemoryLib
+ BaseMemoryLibMmx
+ BaseMemoryLibSse2
+ BaseMemoryLibRepStr
+ BaseMemoryLibOptDxe
+ BaseMemoryLibOptPei
+ PeiMemoryLib
+ UefiMemoryLib
+
+ Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Base.h>
+#include <Library/DebugLib.h>
+
+/**
+ Checks if the contents of a buffer are all zeros.
+
+ This function checks whether the contents of a buffer are all zeros. If the
+ contents are all zeros, return TRUE. Otherwise, return FALSE.
+
+ If Length > 0 and Buffer is NULL, then ASSERT().
+ If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+ @param Buffer The pointer to the buffer to be checked.
+ @param Length The size of the buffer (in bytes) to be checked.
+
+ @retval TRUE Contents of the buffer are all zeros.
+ @retval FALSE Contents of the buffer are not all zeros.
+
+**/
+BOOLEAN
+EFIAPI
+IsZeroBuffer (
+ IN CONST VOID *Buffer,
+ IN UINTN Length
+ )
+{
+ CONST UINT8 *BufferData;
+ UINTN Index;
+
+ ASSERT (!(Buffer == NULL && Length > 0));
+ ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Buffer));
+
+ BufferData = Buffer;
+ for (Index = 0; Index < Length; Index++) {
+ if (BufferData[Index] != 0) {
+ return FALSE;
+ }
+ }
+ return TRUE;
+}
diff --git a/MdePkg/Library/PeiMemoryLib/IsZeroBuffer.c b/MdePkg/Library/PeiMemoryLib/IsZeroBuffer.c
new file mode 100644
index 0000000..fda0c7c
--- /dev/null
+++ b/MdePkg/Library/PeiMemoryLib/IsZeroBuffer.c
@@ -0,0 +1,65 @@
+/** @file
+ Implementation of IsZeroBuffer function.
+
+ The following BaseMemoryLib instances contain the same copy of this file:
+
+ BaseMemoryLib
+ BaseMemoryLibMmx
+ BaseMemoryLibSse2
+ BaseMemoryLibRepStr
+ BaseMemoryLibOptDxe
+ BaseMemoryLibOptPei
+ PeiMemoryLib
+ UefiMemoryLib
+
+ Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Base.h>
+#include <Library/DebugLib.h>
+
+/**
+ Checks if the contents of a buffer are all zeros.
+
+ This function checks whether the contents of a buffer are all zeros. If the
+ contents are all zeros, return TRUE. Otherwise, return FALSE.
+
+ If Length > 0 and Buffer is NULL, then ASSERT().
+ If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+ @param Buffer The pointer to the buffer to be checked.
+ @param Length The size of the buffer (in bytes) to be checked.
+
+ @retval TRUE Contents of the buffer are all zeros.
+ @retval FALSE Contents of the buffer are not all zeros.
+
+**/
+BOOLEAN
+EFIAPI
+IsZeroBuffer (
+ IN CONST VOID *Buffer,
+ IN UINTN Length
+ )
+{
+ CONST UINT8 *BufferData;
+ UINTN Index;
+
+ ASSERT (!(Buffer == NULL && Length > 0));
+ ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Buffer));
+
+ BufferData = Buffer;
+ for (Index = 0; Index < Length; Index++) {
+ if (BufferData[Index] != 0) {
+ return FALSE;
+ }
+ }
+ return TRUE;
+}
diff --git a/MdePkg/Library/PeiMemoryLib/PeiMemoryLib.inf b/MdePkg/Library/PeiMemoryLib/PeiMemoryLib.inf
index 58af9d0..9760639 100644
--- a/MdePkg/Library/PeiMemoryLib/PeiMemoryLib.inf
+++ b/MdePkg/Library/PeiMemoryLib/PeiMemoryLib.inf
@@ -45,6 +45,7 @@
MemLibGeneric.c
MemLibGuid.c
MemLib.c
+ IsZeroBuffer.c
MemLibInternals.h


diff --git a/MdePkg/Library/UefiMemoryLib/IsZeroBuffer.c b/MdePkg/Library/UefiMemoryLib/IsZeroBuffer.c
new file mode 100644
index 0000000..fda0c7c
--- /dev/null
+++ b/MdePkg/Library/UefiMemoryLib/IsZeroBuffer.c
@@ -0,0 +1,65 @@
+/** @file
+ Implementation of IsZeroBuffer function.
+
+ The following BaseMemoryLib instances contain the same copy of this file:
+
+ BaseMemoryLib
+ BaseMemoryLibMmx
+ BaseMemoryLibSse2
+ BaseMemoryLibRepStr
+ BaseMemoryLibOptDxe
+ BaseMemoryLibOptPei
+ PeiMemoryLib
+ UefiMemoryLib
+
+ Copyright (c) 2016, Intel Corporation. All rights reserved.<BR>
+ This program and the accompanying materials
+ are licensed and made available under the terms and conditions of the BSD License
+ which accompanies this distribution. The full text of the license may be found at
+ http://opensource.org/licenses/bsd-license.php
+
+ THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <Base.h>
+#include <Library/DebugLib.h>
+
+/**
+ Checks if the contents of a buffer are all zeros.
+
+ This function checks whether the contents of a buffer are all zeros. If the
+ contents are all zeros, return TRUE. Otherwise, return FALSE.
+
+ If Length > 0 and Buffer is NULL, then ASSERT().
+ If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+ @param Buffer The pointer to the buffer to be checked.
+ @param Length The size of the buffer (in bytes) to be checked.
+
+ @retval TRUE Contents of the buffer are all zeros.
+ @retval FALSE Contents of the buffer are not all zeros.
+
+**/
+BOOLEAN
+EFIAPI
+IsZeroBuffer (
+ IN CONST VOID *Buffer,
+ IN UINTN Length
+ )
+{
+ CONST UINT8 *BufferData;
+ UINTN Index;
+
+ ASSERT (!(Buffer == NULL && Length > 0));
+ ASSERT ((Length - 1) <= (MAX_ADDRESS - (UINTN)Buffer));
+
+ BufferData = Buffer;
+ for (Index = 0; Index < Length; Index++) {
+ if (BufferData[Index] != 0) {
+ return FALSE;
+ }
+ }
+ return TRUE;
+}
diff --git a/MdePkg/Library/UefiMemoryLib/UefiMemoryLib.inf b/MdePkg/Library/UefiMemoryLib/UefiMemoryLib.inf
index e82732f..2ea7875 100644
--- a/MdePkg/Library/UefiMemoryLib/UefiMemoryLib.inf
+++ b/MdePkg/Library/UefiMemoryLib/UefiMemoryLib.inf
@@ -45,6 +45,7 @@
MemLibGeneric.c
MemLibGuid.c
MemLib.c
+ IsZeroBuffer.c
MemLibInternals.h


--
1.9.5.msysgit.0