[edk2-devel] [Patch v3 01/16] BaseTools/Python: Allow HOST_APPLICATION to use NULL libraries
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2797
Update HOST_APPLICATION module type to use NULL library instances.
Cc: Bob Feng
Cc: Liming Gao
Cc: Sean Brogan
Cc: Bret Barkelew
Cc: Jiewen Yao
Signed-off-by: Michael D Kinney
Reviewed-by: Bob Feng
Reviewed-by: Sean Brogan
---
BaseTools/Source/Python/Workspace/WorkspaceCommon.py | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
index 913e710fd9..53027a0e30 100644
--- a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
+++ b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py
@@ -1,7 +1,7 @@
## @file
# Common routines used by workspace
#
-# Copyright (c) 2012 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2012 - 2020, Intel Corporation. All rights reserved.
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -100,7 +100,7 @@ def GetModuleLibInstances(Module, Platform, BuildDatabase,
Arch, Target, Toolcha
# If a module has a MODULE_TYPE of USER_DEFINED,
# do not link in NULL library class instances from the global
[LibraryClasses.*] sections.
#
-if Module.ModuleType != SUP_MODULE_USER_DEFINED and Module.ModuleType !=
SUP_MODULE_HOST_APPLICATION:
+if Module.ModuleType != SUP_MODULE_USER_DEFINED:
for LibraryClass in Platform.LibraryClasses.GetKeys():
if LibraryClass.startswith("NULL") and
Platform.LibraryClasses[LibraryClass, Module.ModuleType]:
Module.LibraryClasses[LibraryClass] =
Platform.LibraryClasses[LibraryClass, Module.ModuleType]
--
2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#62352): https://edk2.groups.io/g/devel/message/62352
Mute This Topic: https://groups.io/mt/75432212/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch v3 13/16] MdePkg/Include: Hook DebugLib _ASSERT() for unit tests
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2801 Update DebugLib.h _ASSERT() macro to check if unit testing is enabled and call UnitTestDebugAssert() instead of DebugAssert() so the an ASSERT() condition that is triggered by a function under test can be handled by the Unit Test Framework. If EDKII_UNIT_TEST_FRAMEWORK_ENABLED is not defined, then the existing DebugLib behavior is preserved. Cc: Liming Gao Cc: Sean Brogan Cc: Bret Barkelew Cc: Jiewen Yao Signed-off-by: Michael D Kinney --- MdePkg/Include/Library/DebugLib.h | 28 +++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/MdePkg/Include/Library/DebugLib.h b/MdePkg/Include/Library/DebugLib.h index baab34bf05..4cacd4b8e2 100644 --- a/MdePkg/Include/Library/DebugLib.h +++ b/MdePkg/Include/Library/DebugLib.h @@ -289,12 +289,38 @@ DebugPrintLevelEnabled ( @param Expression Boolean expression that evaluated to FALSE **/ +#if defined (EDKII_UNIT_TEST_FRAMEWORK_ENABLED) +/** + Unit test library replacement for DebugAssert() in DebugLib. + + If FileName is NULL, then a string of "(NULL) Filename" is printed. + If Description is NULL, then a string of "(NULL) Description" is printed. + + @param FileName The pointer to the name of the source file that generated the assert condition. + @param LineNumber The line number in the source file that generated the assert condition + @param Description The pointer to the description of the assert condition. + +**/ +VOID +EFIAPI +UnitTestDebugAssert ( + IN CONST CHAR8 *FileName, + IN UINTNLineNumber, + IN CONST CHAR8 *Description + ); + +#if defined(__clang__) && defined(__FILE_NAME__) +#define _ASSERT(Expression) UnitTestDebugAssert (__FILE_NAME__, __LINE__, #Expression) +#else +#define _ASSERT(Expression) UnitTestDebugAssert (__FILE__, __LINE__, #Expression) +#endif +#else #if defined(__clang__) && defined(__FILE_NAME__) #define _ASSERT(Expression) DebugAssert (__FILE_NAME__, __LINE__, #Expression) #else #define _ASSERT(Expression) DebugAssert (__FILE__, __LINE__, #Expression) #endif - +#endif /** Internal worker macro that calls DebugPrint(). -- 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62365): https://edk2.groups.io/g/devel/message/62365 Mute This Topic: https://groups.io/mt/75432225/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch v3 07/16] UnitTestFrameworkPkg: Enable source level debug for host tests
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2804 Optionally enable a feature to support source level debug of a host based unit test. By default, this feature is disabled. Exceptions are caught by the unit test framework and are interpreted as a test failure. When a unit test is under development, bugs may generate exceptions or a unit test developer may want to trace the execution of unit tests to debug some unexpected behavior. Defining UNIT_TESTING_DEBUG in the DSC file or from the build command line allows exceptions to be caught by the host OS and allows the developer to debug their unit test under development or debug the Unit Test Framework itself. Cc: Sean Brogan Cc: Bret Barkelew Cc: Jiewen Yao Signed-off-by: Michael D Kinney --- UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc | 5 + 1 file changed, 5 insertions(+) diff --git a/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc b/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc index c22085fae1..c4e6e0e0a6 100644 --- a/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc +++ b/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc @@ -20,6 +20,11 @@ [LibraryClasses.common.HOST_APPLICATION] [BuildOptions] GCC:*_*_*_CC_FLAGS = -fno-pie +!ifdef $(UNIT_TESTING_DEBUG) + MSFT:*_*_*_CC_FLAGS = -D UNIT_TESTING_DEBUG=1 + GCC:*_*_*_CC_FLAGS = -D UNIT_TESTING_DEBUG=1 + XCODE:*_*_*_CC_FLAGS = -D UNIT_TESTING_DEBUG=1 +!endif [BuildOptions.common.EDKII.HOST_APPLICATION] # -- 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62358): https://edk2.groups.io/g/devel/message/62358 Mute This Topic: https://groups.io/mt/75432218/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch v3 08/16] UnitTestFrameworkPkg: Set host application stack size to 256KB
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2803 The UEFI Specification defines the minimum stack size before ExitBootServices() to be 128KB. When running a host based unit test, there may be additional stack overhead from the host application environment and cmocka. Update the build flags to set the size of the stack to 256KB which is double the UEFI Specification requirement. Cc: Sean Brogan Cc: Bret Barkelew Cc: Jiewen Yao Signed-off-by: Michael D Kinney --- UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc b/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc index c4e6e0e0a6..4dd8d4ac67 100644 --- a/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc +++ b/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc @@ -30,7 +30,7 @@ [BuildOptions.common.EDKII.HOST_APPLICATION] # # MSFT # - MSFT:*_*_*_DLINK_FLAGS== /out:"$(BIN_DIR)\$(BASE_NAME).exe" /pdb:"$(BIN_DIR)\$(BASE_NAME).pdb" /IGNORE:4001 /NOLOGO /SUBSYSTEM:CONSOLE /DEBUG /NODEFAULTLIB:libcmt.lib libcmtd.lib + MSFT:*_*_*_DLINK_FLAGS== /out:"$(BIN_DIR)\$(BASE_NAME).exe" /pdb:"$(BIN_DIR)\$(BASE_NAME).pdb" /IGNORE:4001 /NOLOGO /SUBSYSTEM:CONSOLE /DEBUG /STACK:0x4,0x4 /NODEFAULTLIB:libcmt.lib libcmtd.lib MSFT:*_*_IA32_DLINK_FLAGS = /MACHINE:I386 MSFT:*_*_X64_DLINK_FLAGS = /MACHINE:AMD64 -- 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62360): https://edk2.groups.io/g/devel/message/62360 Mute This Topic: https://groups.io/mt/75432220/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch v3 05/16] MdePkg/Library/BaseLib: Add BaseLib instance for host based unit tests
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2800
Add a new version of BaseLib that is safe for use from host based
unit test applications. Host based unit test applications may need
to provide implementations of some BaseLib functions that provide
simple emulation to exercise the code under test. The structure
UNIT_TEST_HOST_BASE_LIB is filled in with services that provide
default emulation for BaseLib APIs that would normally generate
exceptions in a host based unit test application. This structure
allows an individual unit test to replace the default emulation of
a BaseLib service with an alternate version that is required by a
specific unit test. A global variable of type UNIT_TEST_HOST_BASE_LIB
is provided through the new UnitTestHostBaseLib library class.
Normally cmocka would be used to mock services the code under
test calls. However, the BaseLib is used by the Unit Test
Framework itself, so using a mocked interface is not possible.
The use of a structure to provide hooks for unit test is not
expected to be a common feature. It should only be required
for libraries that are used by both the Unit Test Framework and
the code under test where the code under test requires a
different behavior than the Unit Test Framework.
Cc: Liming Gao
Cc: Sean Brogan
Cc: Bret Barkelew
Cc: Jiewen Yao
Signed-off-by: Michael D Kinney
---
MdePkg/Library/BaseLib/UnitTestHost.c | 140 +
MdePkg/Library/BaseLib/UnitTestHost.h | 66 +
.../Library/BaseLib/UnitTestHostBaseLib.inf | 217 ++
.../Library/BaseLib/UnitTestHostBaseLib.uni | 11 +
MdePkg/Library/BaseLib/X86UnitTestHost.c | 2977 +
MdePkg/MdePkg.dec |8 +-
MdePkg/Test/MdePkgHostTest.dsc|5 +
.../Include/Library/UnitTestHostBaseLib.h | 582
8 files changed, 4005 insertions(+), 1 deletion(-)
create mode 100644 MdePkg/Library/BaseLib/UnitTestHost.c
create mode 100644 MdePkg/Library/BaseLib/UnitTestHost.h
create mode 100644 MdePkg/Library/BaseLib/UnitTestHostBaseLib.inf
create mode 100644 MdePkg/Library/BaseLib/UnitTestHostBaseLib.uni
create mode 100644 MdePkg/Library/BaseLib/X86UnitTestHost.c
create mode 100644 MdePkg/Test/UnitTest/Include/Library/UnitTestHostBaseLib.h
diff --git a/MdePkg/Library/BaseLib/UnitTestHost.c
b/MdePkg/Library/BaseLib/UnitTestHost.c
new file mode 100644
index 00..79eec7caca
--- /dev/null
+++ b/MdePkg/Library/BaseLib/UnitTestHost.c
@@ -0,0 +1,140 @@
+/** @file
+ Common Unit Test Host functions.
+
+ Copyright (c) 2020, Intel Corporation. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "UnitTestHost.h"
+
+///
+/// Module global variable for simple system emulation of interrupt state
+///
+STATIC BOOLEAN mUnitTestHostBaseLibInterruptState;
+
+/**
+ Enables CPU interrupts.
+
+**/
+VOID
+EFIAPI
+UnitTestHostBaseLibEnableInterrupts (
+ VOID
+ )
+{
+ mUnitTestHostBaseLibInterruptState = TRUE;
+}
+
+/**
+ Disables CPU interrupts.
+
+**/
+VOID
+EFIAPI
+UnitTestHostBaseLibDisableInterrupts (
+ VOID
+ )
+{
+ mUnitTestHostBaseLibInterruptState = FALSE;
+}
+
+/**
+ Enables CPU interrupts for the smallest window required to capture any
+ pending interrupts.
+
+**/
+VOID
+EFIAPI
+UnitTestHostBaseLibEnableDisableInterrupts (
+ VOID
+ )
+{
+ mUnitTestHostBaseLibInterruptState = FALSE;
+}
+
+/**
+ Set the current CPU interrupt state.
+
+ Sets the current CPU interrupt state to the state specified by
+ InterruptState. If InterruptState is TRUE, then interrupts are enabled. If
+ InterruptState is FALSE, then interrupts are disabled. InterruptState is
+ returned.
+
+ @param InterruptState TRUE if interrupts should enabled. FALSE if
+ interrupts should be disabled.
+
+ @return InterruptState
+
+**/
+BOOLEAN
+EFIAPI
+UnitTestHostBaseLibGetInterruptState (
+ VOID
+ )
+{
+ return mUnitTestHostBaseLibInterruptState;
+}
+
+/**
+ Enables CPU interrupts.
+
+**/
+VOID
+EFIAPI
+EnableInterrupts (
+ VOID
+ )
+{
+ gUnitTestHostBaseLib.Common->EnableInterrupts ();
+}
+
+/**
+ Disables CPU interrupts.
+
+**/
+VOID
+EFIAPI
+DisableInterrupts (
+ VOID
+ )
+{
+ gUnitTestHostBaseLib.Common->DisableInterrupts ();
+}
+
+/**
+ Enables CPU interrupts for the smallest window required to capture any
+ pending interrupts.
+
+**/
+VOID
+EFIAPI
+EnableDisableInterrupts (
+ VOID
+ )
+{
+ gUnitTestHostBaseLib.Common->EnableDisableInterrupts ();
+}
+
+/**
+ Set the current CPU interrupt state.
+
+ Sets the current CPU interrupt state to the state specified by
+ InterruptState. If InterruptState is TRUE, then interrupts are enabled. If
+ InterruptState is FALSE, then interrupts are disabled. InterruptState is
+ returned.
+
+ @param InterruptState TRUE if interrupts should enabled. FALSE if
+ interrupts should be disabled.
+
+ @return InterruptState
+
+**/
+BOOLEAN
+EFIAPI
+GetInterruptState (
+ VOID
+ )
+{
[edk2-devel] [Patch v3 03/16] MdePkg/BaseCacheMaintenanceLibNull: Add Null instance for host testing
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2799
The services in CacheMaintenanceLib usually generate exceptions in a
unit test host application. Provide a Null instance that can be safely
used.
This Null instance can also be used as a template for implementing
new instances of CacheMaintenanceLib.
Cc: Liming Gao
Cc: Sean Brogan
Cc: Bret Barkelew
Cc: Jiewen Yao
Signed-off-by: Michael D Kinney
Reviewed-by: Liming Gao
---
.../BaseCacheMaintenanceLibNull.c | 225 ++
.../BaseCacheMaintenanceLibNull.inf | 29 +++
.../BaseCacheMaintenanceLibNull.uni | 12 +
MdePkg/MdePkg.dsc | 1 +
4 files changed, 267 insertions(+)
create mode 100644
MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.c
create mode 100644
MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.inf
create mode 100644
MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.uni
diff --git
a/MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.c
b/MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.c
new file mode 100644
index 00..fd5b9d4710
--- /dev/null
+++ b/MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.c
@@ -0,0 +1,225 @@
+/** @file
+ Null Cache Maintenance Librfary implementation.
+
+ Copyright (c) 2020, Intel Corporation. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include
+#include
+
+/**
+ Invalidates the entire instruction cache in cache coherency domain of the
+ calling CPU.
+
+**/
+VOID
+EFIAPI
+InvalidateInstructionCache (
+ VOID
+ )
+{
+}
+
+/**
+ Invalidates a range of instruction cache lines in the cache coherency domain
+ of the calling CPU.
+
+ Invalidates the instruction cache lines specified by Address and Length. If
+ Address is not aligned on a cache line boundary, then entire instruction
+ cache line containing Address is invalidated. If Address + Length is not
+ aligned on a cache line boundary, then the entire instruction cache line
+ containing Address + Length -1 is invalidated. This function may choose to
+ invalidate the entire instruction cache if that is more efficient than
+ invalidating the specified range. If Length is 0, then no instruction cache
+ lines are invalidated. Address is returned.
+
+ If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT().
+
+ @param Address The base address of the instruction cache lines to
+ invalidate. If the CPU is in a physical addressing mode, then
+ Address is a physical address. If the CPU is in a virtual
+ addressing mode, then Address is a virtual address.
+
+ @param Length The number of bytes to invalidate from the instruction cache.
+
+ @return Address.
+
+**/
+VOID *
+EFIAPI
+InvalidateInstructionCacheRange (
+ IN VOID *Address,
+ IN UINTN Length
+ )
+{
+ ASSERT (Length <= MAX_ADDRESS - (UINTN)Address + 1);
+ return Address;
+}
+
+/**
+ Writes back and invalidates the entire data cache in cache coherency domain
+ of the calling CPU.
+
+ Writes back and invalidates the entire data cache in cache coherency domain
+ of the calling CPU. This function guarantees that all dirty cache lines are
+ written back to system memory, and also invalidates all the data cache lines
+ in the cache coherency domain of the calling CPU.
+
+**/
+VOID
+EFIAPI
+WriteBackInvalidateDataCache (
+ VOID
+ )
+{
+}
+
+/**
+ Writes back and invalidates a range of data cache lines in the cache
+ coherency domain of the calling CPU.
+
+ Writes Back and Invalidate the data cache lines specified by Address and
+ Length. If Address is not aligned on a cache line boundary, then entire data
+ cache line containing Address is written back and invalidated. If Address +
+ Length is not aligned on a cache line boundary, then the entire data cache
+ line containing Address + Length -1 is written back and invalidated. This
+ function may choose to write back and invalidate the entire data cache if
+ that is more efficient than writing back and invalidating the specified
+ range. If Length is 0, then no data cache lines are written back and
+ invalidated. Address is returned.
+
+ If Length is greater than (MAX_ADDRESS - Address + 1), then ASSERT().
+
+ @param Address The base address of the data cache lines to write back and
+ invalidate. If the CPU is in a physical addressing mode, then
+ Address is a physical address. If the CPU is in a virtual
+ addressing mode, then Address is a virtual address.
+ @param Length The number of bytes to write back and invalidate from the
+ data cache.
+
+ @return Address of cache invalidation.
+
+**/
+VOID *
+EFIAPI
+WriteBackInvalidateDataCacheRange (
+ IN VOID
[edk2-devel] [Patch v3 16/16] UnitTestFramewokPkg/SampleUnitTest: Use UT_EXPECT_ASSERT_FAILURE()
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2801
Add samples for all UnitTestLib macros including using
UT_EXPECT_ASSERT_FAILURE() for positive test cases where an
ASSERT() is triggered and detected correctly.
Additional test cases are added that disable ASSERT()s and
verify that UT_EXPECT_ASSERT_FAILURE() macros are skipped.
Cc: Sean Brogan
Cc: Bret Barkelew
Cc: Jiewen Yao
Signed-off-by: Michael D Kinney
---
.../Sample/SampleUnitTest/SampleUnitTest.c| 510 ++
.../SampleUnitTest/SampleUnitTestDxe.inf | 3 +
.../SampleUnitTest/SampleUnitTestHost.inf | 3 +
.../SampleUnitTest/SampleUnitTestPei.inf | 3 +
.../SampleUnitTest/SampleUnitTestSmm.inf | 3 +
.../SampleUnitTestUefiShell.inf | 3 +
.../Test/UnitTestFrameworkPkgHostTest.dsc | 3 +
UnitTestFrameworkPkg/UnitTestFrameworkPkg.dsc | 3 +
8 files changed, 531 insertions(+)
diff --git
a/UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTest/SampleUnitTest.c
b/UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTest/SampleUnitTest.c
index 37d5747bca..cb50f45391 100644
--- a/UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTest/SampleUnitTest.c
+++ b/UnitTestFrameworkPkg/Test/UnitTest/Sample/SampleUnitTest/SampleUnitTest.c
@@ -181,6 +181,466 @@ GlobalPointerShouldBeChangeable (
return UNIT_TEST_PASSED;
}
+/**
+ Unit-Test Test Suite Setup (before) function that enables ASSERT() macros.
+**/
+VOID
+EFIAPI
+TestSuiteEnableAsserts (
+ VOID
+ )
+{
+ //
+ // Set BIT0 (DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED)
+ //
+ PatchPcdSet8 (PcdDebugPropertyMask, PcdGet8 (PcdDebugPropertyMask) | BIT0);
+}
+
+/**
+ Unit-Test Test Suite Setup (before) function that disables ASSERT() macros.
+**/
+VOID
+EFIAPI
+TestSuiteDisableAsserts (
+ VOID
+ )
+{
+ //
+ // Clear BIT0 (DEBUG_PROPERTY_DEBUG_ASSERT_ENABLED)
+ //
+ PatchPcdSet8 (PcdDebugPropertyMask, PcdGet8 (PcdDebugPropertyMask) &
(~BIT0));
+}
+
+/**
+ Sample unit test using the UT_ASSERT_TRUE() macro.
+
+ @param[in] Context[Optional] An optional parameter that enables:
+ 1) test-case reuse with varied parameters and
+ 2) test-case re-entry for Target tests that need a
+ reboot. This parameter is a VOID* and it is the
+ responsibility of the test author to ensure that the
+ contents are well understood by all test cases that
may
+ consume it.
+
+ @retval UNIT_TEST_PASSED The Unit test has completed and the
test
+case was successful.
+ @retval UNIT_TEST_ERROR_TEST_FAILED A test case assertion has failed.
+**/
+UNIT_TEST_STATUS
+EFIAPI
+MacroUtAssertTrue (
+ IN UNIT_TEST_CONTEXT Context
+ )
+{
+ UINT64 Result;
+
+ //
+ // This test passes because expression always evaluated to TRUE.
+ //
+ UT_ASSERT_TRUE (TRUE);
+
+ //
+ // This test passes because expression always evaluates to TRUE.
+ //
+ Result = LShiftU64 (BIT0, 1);
+ UT_ASSERT_TRUE (Result == BIT1);
+
+ return UNIT_TEST_PASSED;
+}
+
+/**
+ Sample unit test using the UT_ASSERT_FALSE() macro.
+
+ @param[in] Context[Optional] An optional parameter that enables:
+ 1) test-case reuse with varied parameters and
+ 2) test-case re-entry for Target tests that need a
+ reboot. This parameter is a VOID* and it is the
+ responsibility of the test author to ensure that the
+ contents are well understood by all test cases that
may
+ consume it.
+
+ @retval UNIT_TEST_PASSED The Unit test has completed and the
test
+case was successful.
+ @retval UNIT_TEST_ERROR_TEST_FAILED A test case assertion has failed.
+**/
+UNIT_TEST_STATUS
+EFIAPI
+MacroUtAssertFalse (
+ IN UNIT_TEST_CONTEXT Context
+ )
+{
+ UINT64 Result;
+
+ //
+ // This test passes because expression always evaluated to FALSE.
+ //
+ UT_ASSERT_FALSE (FALSE);
+
+ //
+ // This test passes because expression always evaluates to FALSE.
+ //
+ Result = LShiftU64 (BIT0, 1);
+ UT_ASSERT_FALSE (Result == BIT0);
+
+ return UNIT_TEST_PASSED;
+}
+
+/**
+ Sample unit test using the UT_ASSERT_EQUAL() macro.
+
+ @param[in] Context[Optional] An optional parameter that enables:
+ 1) test-case reuse with varied parameters and
+ 2) test-case re-entry for Target tests that need a
+ reboot. This parameter is a VOID* and it is the
+ responsibility of the test author to ensure that the
+ contents are well understood by all test cases that
may
+ consume it.
+
+ @retval UNIT_TEST_PASSED The Unit test has completed and the
test
+
[edk2-devel] [Patch v3 14/16] MdePkg/Include: Add UT_EXPECT_ASSERT_FAILURE() to UnitTestLib
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2801
Add the UT_EXPECT_ASSERT_FAILURE(FunctionCall, Status) macro
to the UnitTestLib that can be used to check if a function
under test triggers an ASSERT() condition. If an ASSERT()
condition is triggered, then the macro returns. If the
ASSERT() condition is not triggered, then the current unit
test fails with a status of UNIT_TEST_ERROR_TEST_FAILED.
If ASSERT()s are disabled, then this check for ASSERT()
behavior is not possible, and the check is skipped.
The global variable gUnitTestExpectAssertFailureJumpBuffer
is added to the UnitTestLib to save/restore context when
the UT_EXPECT_ASSERT_FAILURE(FunctionCall, Status) macro
is used. The UT_EXPECT_ASSERT_FAILURE() macro uses the
SetJump() service with this global variable. The UnitTestLib
service UnitTestDebugAssert() uses the LongJump() service
with this global to restore context if an ASSERT() is
triggered by the code under test.
Add UnitTestExpectAssertFailure() to the UnitTestLib class.
The UnitTestExpectAssertFailure() is called from the new
UT_EXPECT_ASSERT_FAILURE() macro after the status of this
macro check is known.
Add UnitTestDebugAssert() to the UnitTestLib class. The
UnitTestDebugAssert() service is the same as the DebugLib
DebugAssert() service and is invoked from the DebugLib
_ASSERT() macro if unit testing is enabled. This allows the
Unit Test Framework to know when code under test triggers an
ASSERT() condition.
Cc: Liming Gao
Cc: Sean Brogan
Cc: Bret Barkelew
Cc: Jiewen Yao
Signed-off-by: Michael D Kinney
---
MdePkg/Include/Library/UnitTestLib.h | 90
1 file changed, 90 insertions(+)
diff --git a/MdePkg/Include/Library/UnitTestLib.h
b/MdePkg/Include/Library/UnitTestLib.h
index a4374580a8..99175496c8 100644
--- a/MdePkg/Include/Library/UnitTestLib.h
+++ b/MdePkg/Include/Library/UnitTestLib.h
@@ -441,6 +441,56 @@ SaveFrameworkState (
return UNIT_TEST_ERROR_TEST_FAILED;
\
}
+/**
+ This macro uses the framework assertion logic to check whether a function
call
+ triggers an ASSERT() condition. The BaseLib SetJump()/LongJump() services
+ are used to establish a safe return point when an ASSERT() is triggered.
+ If an ASSERT() is triggered, unit test execution continues and Status is set
+ to UNIT_TEST_PASSED. Otherwise, a unit test case failure is raised and
+ Status is set to UNIT_TEST_ERROR_TEST_FAILED.
+
+ If ASSERT() macros are disabled, then the test case is skipped and a warning
+ message is added to the unit test log. Status is set to UNIT_TEST_SKIPPED.
+
+ @param[in] FunctionCall Function call that is expected to trigger ASSERT().
+ @param[out] StatusPointer to a UNIT_TEST_STATUS return value. This
+is an optional parameter that may be NULL.
+**/
+#if defined (EDKII_UNIT_TEST_FRAMEWORK_ENABLED)
+ #include
+
+ ///
+ /// Pointer to jump buffer used with SetJump()/LongJump() to test if a
+ /// function under test generates an expected ASSERT() condition.
+ ///
+ extern BASE_LIBRARY_JUMP_BUFFER *gUnitTestExpectAssertFailureJumpBuffer;
+
+ #define UT_EXPECT_ASSERT_FAILURE(FunctionCall, Status) \
+do { \
+ UNIT_TEST_STATUS UnitTestJumpStatus;\
+ BASE_LIBRARY_JUMP_BUFFER UnitTestJumpBuffer;\
+ UnitTestJumpStatus = UNIT_TEST_SKIPPED; \
+ if (DebugAssertEnabled ()) { \
+gUnitTestExpectAssertFailureJumpBuffer = \
+if (SetJump (gUnitTestExpectAssertFailureJumpBuffer) == 0) { \
+ FunctionCall;\
+ UnitTestJumpStatus = UNIT_TEST_ERROR_TEST_FAILED;\
+} else { \
+ UnitTestJumpStatus = UNIT_TEST_PASSED; \
+} \
+gUnitTestExpectAssertFailureJumpBuffer = NULL; \
+ }\
+ if (!UnitTestExpectAssertFailure ( \
+ UnitTestJumpStatus, \
+ __FUNCTION__, __LINE__, __FILE__, \
+ #FunctionCall, Status)) { \
+return UNIT_TEST_ERROR_TEST_FAILED;\
+ }\
+} while (FALSE)
+#else
+ #define UT_EXPECT_ASSERT_FAILURE(FunctionCall, Status) FunctionCall;
+#endif
+
/**
If Expression is TRUE, then TRUE is returned.
If Expression is FALSE, then an assert is triggered and the location of the
@@ -690,6 +740,46 @@ UnitTestAssertNotNull (
[edk2-devel] [Patch v3 15/16] MdePkg/Library/BaseStackCheckLib: Fix PCD type in INF
Update INF file to use a [Pcd] section instead of a [FixedPcd] section. [FixedPcd] should only be used in an INF file if the source code looks up the PCD value using the PcdLib FixedPcdGetxx() services. Using [FixedPcd] forces a platform to configure the PCD to type FixedAtBuild. In this case, PcdDebugPropertyMask supports PCD types FixedAtBuild and PatchableInModule. Without this change any platform that wants to use PcdDebugPropertyMask as type PatchableInModule breaks the build. Cc: Liming Gao Signed-off-by: Michael D Kinney Reviewed-by: Liming Gao --- MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf index 4ae3bd1a82..0dc3c4a83a 100644 --- a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf @@ -36,5 +36,5 @@ [LibraryClasses] BaseLib DebugLib -[FixedPcd] +[Pcd] gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask ## CONSUMES -- 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62367): https://edk2.groups.io/g/devel/message/62367 Mute This Topic: https://groups.io/mt/75432227/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch v3 00/16] UnitTestFrameworkPkg: Enhancements and bug fixes
Changes in V3
==
* Add UnitTestHostBaseLib class for the global variable that allows
host based unit tests to hook and emulate some BaseLib services
Changes in V2
==
* Add UnitTestExpectAssertFailure() to UnitTestLib to simplify the macro
UT_EXPECT_ASSERT_FAILURE() and provide better log messages.
* Expand UnitTestFrameworkPkg sample unit tests to cover test cases for the new
UT_EXPECT_ASSERT_FAILURE() macro and all other UnitTestLib macros.
* Add failure type FAILURETYPE_EXPECTASSERT when the macro
UT_EXPECT_ASSERT_FAILURE() does not detect an ASSERT().
* Move print of log messages to end of cleanup function to support log
messages generated in a cleanup function.
* Update running of target-based tests to use SetJump()/LongJump()
around prereq, cleanup, and unit test functions to match the behavior
of host based tests using cmocka. This also requires all UnitTestLib
Assert functions to generate error log message first and then call
UnitTestFailure() where the LongJump() is made to make sure all log
messages are added before the LongJump().
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2797
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2798
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2799
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2800
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2801
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2803
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2804
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2805
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2806
* Add Null base libraries for host based unit tests
* Add host based test version of BaseLib with hooks for services that use
privileged instructions.
* Add new UT_EXPECT_ASSERT_FAILURE() macro to UnitTestLib class
* Enable source level debug of unit tests
* Increase stack size to 256KB for host based tests on Windows
* Update BaseTools to support NULL libs for HOST_APPLICATION modules
* Guarantee print log works even if unit test generates an exception
* Use filename instead of function name in target mode logs
Cc: Liming Gao liming@intel.com
Cc: Bob Feng bob.c.f...@intel.com
Cc: Sean Brogan sean.bro...@microsoft.com
Cc: Bret Barkelew bret.barke...@microsoft.com
Cc: Jiewen Yao jiewen@intel.com
Signed-off-by: Michael D Kinney michael.d.kin...@intel.com
Michael D Kinney (16):
BaseTools/Python: Allow HOST_APPLICATION to use NULL libraries
MdePkg/BaseCpuLibNull: Add Null version of CpuLib for host testing
MdePkg/BaseCacheMaintenanceLibNull: Add Null instance for host testing
MdePkg/BaseLib: Break out IA32/X64 GCC inline privileged functions
MdePkg/Library/BaseLib: Add BaseLib instance for host based unit tests
UnitTestFrameworkPkg: Use host libraries from MdePkg
UnitTestFrameworkPkg: Enable source level debug for host tests
UnitTestFrameworkPkg: Set host application stack size to 256KB
UnitTestFrameworkPkg: Change target mode DebugLib mapping
UnitTestFrameworkPkg/UnitTestLib: Move print log into cleanup
UnitTestFrameworkPkg/UnitTestLib: Fix target mode log messages
UnitTestFrameworkPkg/UnitTestLib: Add checks for ASSERT()
MdePkg/Include: Hook DebugLib _ASSERT() for unit tests
MdePkg/Include: Add UT_EXPECT_ASSERT_FAILURE() to UnitTestLib
MdePkg/Library/BaseStackCheckLib: Fix PCD type in INF
UnitTestFramewokPkg/SampleUnitTest: Use UT_EXPECT_ASSERT_FAILURE()
.../Python/Workspace/WorkspaceCommon.py |4 +-
MdePkg/Include/Library/DebugLib.h | 28 +-
MdePkg/Include/Library/UnitTestLib.h | 90 +
.../BaseCacheMaintenanceLibNull.c | 225 ++
.../BaseCacheMaintenanceLibNull.inf | 29 +
.../BaseCacheMaintenanceLibNull.uni | 12 +
.../Library/BaseCpuLibNull/BaseCpuLibNull.c | 37 +
.../Library/BaseCpuLibNull/BaseCpuLibNull.inf | 26 +
.../Library/BaseCpuLibNull/BaseCpuLibNull.uni | 11 +
MdePkg/Library/BaseLib/BaseLib.inf|4 +-
MdePkg/Library/BaseLib/Ia32/GccInline.c | 1181 +--
.../Ia32/{GccInline.c => GccInlinePriv.c} | 601 +---
MdePkg/Library/BaseLib/UnitTestHost.c | 140 +
MdePkg/Library/BaseLib/UnitTestHost.h | 66 +
.../Library/BaseLib/UnitTestHostBaseLib.inf | 217 ++
.../Library/BaseLib/UnitTestHostBaseLib.uni | 11 +
MdePkg/Library/BaseLib/X64/GccInline.c| 1240 +--
.../X64/{GccInline.c => GccInlinePriv.c} | 572 +---
MdePkg/Library/BaseLib/X86UnitTestHost.c | 2977 +
.../BaseStackCheckLib/BaseStackCheckLib.inf |2 +-
MdePkg/MdePkg.dec |8 +-
MdePkg/MdePkg.dsc |4 +-
MdePkg/Test/MdePkgHostTest.dsc|5 +
.../Include/Library/UnitTestHostBaseLib.h | 582
.../UnitTestDebugAssertLib.c | 49 +
.../UnitTestDebugAssertLib.inf| 31 +
.../UnitTestDebugAssertLib.uni|
[edk2-devel] [Patch v3 12/16] UnitTestFrameworkPkg/UnitTestLib: Add checks for ASSERT()
REF: REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2801
Add UnitTestDebugAssertLib that provides the UnitTestDebugAssert()
service and the gUnitTestExpectAssertFailureJumpBuffer global
variable. This NULL library is linked against all host and target
unit test builds. This guarantees that the UnitTestDebugAssert()
service is available to link against all libraries and modules that
use the DebugLib class.
EDKII_UNIT_TEST_FRAMEWORK_ENABLED must always be defined when
building unit tests so the behavior of the DebugLib ASSERT()
macros can be adjusted to allow the unit test framework to
catch an ASSERT() if it is triggered by a function under test.
Cc: Sean Brogan
Cc: Bret Barkelew
Cc: Jiewen Yao
Signed-off-by: Michael D Kinney
---
.../UnitTestDebugAssertLib.c | 49 +
.../UnitTestDebugAssertLib.inf| 31 +++
.../UnitTestDebugAssertLib.uni| 11 +
.../Library/UnitTestLib/Assert.c | 203 --
.../Library/UnitTestLib/AssertCmocka.c| 68 ++
.../Library/UnitTestLib/RunTests.c| 23 +-
.../UnitTestResultReportLib.c | 3 +-
.../PrivateInclude/UnitTestFrameworkTypes.h | 1 +
.../Test/UnitTestFrameworkPkgHostTest.dsc | 2 +-
UnitTestFrameworkPkg/UnitTestFrameworkPkg.dsc | 1 +
.../UnitTestFrameworkPkgTarget.dsc.inc| 6 +-
11 files changed, 328 insertions(+), 70 deletions(-)
create mode 100644
UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLib.c
create mode 100644
UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLib.inf
create mode 100644
UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLib.uni
diff --git
a/UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLib.c
b/UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLib.c
new file mode 100644
index 00..0a4001e182
--- /dev/null
+++
b/UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLib.c
@@ -0,0 +1,49 @@
+/** @file
+ Unit Test Debug Assert Library
+
+ Copyright (c) 2020, Intel Corporation. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include
+#include
+#include
+
+///
+/// Point to jump buffer used with SetJump()/LongJump() to test if a function
+/// under test generates an expected ASSERT() condition.
+///
+BASE_LIBRARY_JUMP_BUFFER *gUnitTestExpectAssertFailureJumpBuffer = NULL;
+
+/**
+ Unit test library replacement for DebugAssert() in DebugLib.
+
+ If FileName is NULL, then a string of "(NULL) Filename" is
printed.
+ If Description is NULL, then a string of "(NULL) Description"
is printed.
+
+ @param FileName The pointer to the name of the source file that
generated the assert condition.
+ @param LineNumber The line number in the source file that generated the
assert condition
+ @param Description The pointer to the description of the assert condition.
+
+**/
+VOID
+EFIAPI
+UnitTestDebugAssert (
+ IN CONST CHAR8 *FileName,
+ IN UINTNLineNumber,
+ IN CONST CHAR8 *Description
+ )
+{
+ CHAR8 Message[256];
+
+ if (gUnitTestExpectAssertFailureJumpBuffer != NULL) {
+UT_LOG_INFO ("Detected expected ASSERT: %a(%d): %a\n", FileName,
LineNumber, Description);
+LongJump (gUnitTestExpectAssertFailureJumpBuffer, 1);
+ } else {
+AsciiStrCpyS (Message, sizeof(Message), "Detected unexpected ASSERT(");
+AsciiStrCatS (Message, sizeof(Message), Description);
+AsciiStrCatS (Message, sizeof(Message), ")");
+UnitTestAssertTrue (FALSE, "", LineNumber, FileName, Message);
+ }
+}
diff --git
a/UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLib.inf
b/UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLib.inf
new file mode 100644
index 00..e6ccab0dd9
--- /dev/null
+++
b/UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLib.inf
@@ -0,0 +1,31 @@
+## @file
+# Unit Test Debug Assert Library
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION= 0x00010005
+ BASE_NAME = UnitTestDebugAssertLib
+ MODULE_UNI_FILE= UnitTestDebugAssertLib.uni
+ FILE_GUID = 9D53AD0D-5416-451F-A5BF-E5420051A99B
+ MODULE_TYPE= BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = NULL
+
+#
+# VALID_ARCHITECTURES = ARM AARCH64
+#
+
+[Sources]
+ UnitTestDebugAssertLib.c
+
+[Packages]
+ MdePkg/MdePkg.dec
+ UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec
+
+[LibraryClasses]
+ BaseLib
+ UnitTestLib
diff --git
a/UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLib.uni
b/UnitTestFrameworkPkg/Library/UnitTestDebugAssertLib/UnitTestDebugAssertLib.uni
new file mode 100644
index
[edk2-devel] [Patch v3 04/16] MdePkg/BaseLib: Break out IA32/X64 GCC inline privileged functions
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2800
Break out the IA32/X64 GCC inline functions that can not be used
in a unit test host application into their own source file. This
does not make any changes to the BaseLib library instance. This
is in preparation for a new BaseLib instances that is safe to use
with host-based unit test applications.
Cc: Liming Gao
Cc: Sean Brogan
Cc: Bret Barkelew
Cc: Jiewen Yao
Signed-off-by: Michael D Kinney
---
MdePkg/Library/BaseLib/BaseLib.inf|4 +-
MdePkg/Library/BaseLib/Ia32/GccInline.c | 1181 +---
.../Ia32/{GccInline.c => GccInlinePriv.c} | 601 +---
MdePkg/Library/BaseLib/X64/GccInline.c| 1240 +
.../X64/{GccInline.c => GccInlinePriv.c} | 572 +---
5 files changed, 11 insertions(+), 3587 deletions(-)
copy MdePkg/Library/BaseLib/Ia32/{GccInline.c => GccInlinePriv.c} (62%)
copy MdePkg/Library/BaseLib/X64/{GccInline.c => GccInlinePriv.c} (65%)
diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
b/MdePkg/Library/BaseLib/BaseLib.inf
index a57ae2da31..c740a819ca 100644
--- a/MdePkg/Library/BaseLib/BaseLib.inf
+++ b/MdePkg/Library/BaseLib/BaseLib.inf
@@ -1,7 +1,7 @@
## @file
# Base Library implementation.
#
-# Copyright (c) 2007 - 2019, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.
# Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
# Portions copyright (c) 2011 - 2013, ARM Ltd. All rights reserved.
# Copyright (c) 2020, Hewlett Packard Enterprise Development LP. All rights
reserved.
@@ -156,6 +156,7 @@ [Sources.Ia32]
Ia32/GccInline.c | GCC
+ Ia32/GccInlinePriv.c | GCC
Ia32/Thunk16.nasm
Ia32/EnableDisableInterrupts.nasm| GCC
Ia32/EnablePaging64.nasm
@@ -310,6 +311,7 @@ [Sources.X64]
X86PatchInstruction.c
X86SpeculationBarrier.c
X64/GccInline.c | GCC
+ X64/GccInlinePriv.c | GCC
X64/EnableDisableInterrupts.nasm
X64/DisablePaging64.nasm
X64/RdRand.nasm
diff --git a/MdePkg/Library/BaseLib/Ia32/GccInline.c
b/MdePkg/Library/BaseLib/Ia32/GccInline.c
index 5287200f87..6ed938187a 100644
--- a/MdePkg/Library/BaseLib/Ia32/GccInline.c
+++ b/MdePkg/Library/BaseLib/Ia32/GccInline.c
@@ -1,7 +1,7 @@
/** @file
GCC inline implementation of BaseLib processor specific functions.
- Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+ Copyright (c) 2006 - 2020, Intel Corporation. All rights reserved.
Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -10,8 +10,6 @@
#include "BaseLibInternals.h"
-
-
/**
Used to serialize load and store operations.
@@ -31,41 +29,6 @@ MemoryFence (
__asm__ __volatile__ ("":::"memory");
}
-
-/**
- Enables CPU interrupts.
-
- Enables CPU interrupts.
-
-**/
-VOID
-EFIAPI
-EnableInterrupts (
- VOID
- )
-{
- __asm__ __volatile__ ("sti"::: "memory");
-}
-
-
-/**
- Disables CPU interrupts.
-
- Disables CPU interrupts.
-
-**/
-VOID
-EFIAPI
-DisableInterrupts (
- VOID
- )
-{
- __asm__ __volatile__ ("cli"::: "memory");
-}
-
-
-
-
/**
Requests CPU to pause for a short period of time.
@@ -82,7 +45,6 @@ CpuPause (
__asm__ __volatile__ ("pause");
}
-
/**
Generates a breakpoint on the CPU.
@@ -99,75 +61,6 @@ CpuBreakpoint (
__asm__ __volatile__ ("int $3");
}
-
-
-/**
- Returns a 64-bit Machine Specific Register(MSR).
-
- Reads and returns the 64-bit MSR specified by Index. No parameter checking is
- performed on Index, and some Index values may cause CPU exceptions. The
- caller must either guarantee that Index is valid, or the caller must set up
- exception handlers to catch the exceptions. This function is only available
- on IA-32 and X64.
-
- @param Index The 32-bit MSR index to read.
-
- @return The value of the MSR identified by Index.
-
-**/
-UINT64
-EFIAPI
-AsmReadMsr64 (
- IN UINT32Index
- )
-{
- UINT64 Data;
-
- __asm__ __volatile__ (
-"rdmsr"
-: "=A" (Data) // %0
-: "c" (Index) // %1
-);
-
- return Data;
-}
-
-/**
- Writes a 64-bit value to a Machine Specific Register(MSR), and returns the
- value.
-
- Writes the 64-bit value specified by Value to the MSR specified by Index. The
- 64-bit value written to the MSR is returned. No parameter checking is
- performed on Index or Value, and some of these may cause CPU exceptions. The
- caller must either guarantee that Index and Value are valid, or the caller
- must establish proper exception handlers. This function is only available on
- IA-32 and X64.
-
- @param Index The 32-bit MSR index to write.
- @param Value The 64-bit value to write to the MSR.
-
- @return Value
-
-**/
-UINT64
-EFIAPI
-AsmWriteMsr64 (
- IN UINT32Index,
- IN UINT64Value
- )
-{
- __asm__ __volatile__ (
-"wrmsr"
-:
-: "c" (Index),
- "A" (Value)
[edk2-devel] [Patch v3 06/16] UnitTestFrameworkPkg: Use host libraries from MdePkg
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2800 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2799 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2798 Update the default unit test library mappings to use the library instances from the MdePkg that are safe for host based unit tests. Cc: Sean Brogan Cc: Bret Barkelew Cc: Jiewen Yao Signed-off-by: Michael D Kinney --- UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc | 4 1 file changed, 4 insertions(+) diff --git a/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc b/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc index e954968efc..c22085fae1 100644 --- a/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc +++ b/UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc @@ -9,6 +9,10 @@ !include UnitTestFrameworkPkg/UnitTestFrameworkPkgTarget.dsc.inc [LibraryClasses.common.HOST_APPLICATION] + BaseLib|MdePkg/Library/BaseLib/UnitTestHostBaseLib.inf + UnitTestHostBaseLib|MdePkg/Library/BaseLib/UnitTestHostBaseLib.inf + CpuLib|MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf + CacheMaintenanceLib|MdePkg/Library/BaseCacheMaintenanceLibNull/BaseCacheMaintenanceLibNull.inf CmockaLib|UnitTestFrameworkPkg/Library/CmockaLib/CmockaLib.inf UnitTestLib|UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLibCmocka.inf DebugLib|UnitTestFrameworkPkg/Library/Posix/DebugLibPosix/DebugLibPosix.inf -- 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62357): https://edk2.groups.io/g/devel/message/62357 Mute This Topic: https://groups.io/mt/75432217/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch v3 11/16] UnitTestFrameworkPkg/UnitTestLib: Fix target mode log messages
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2806
Update the log messages generated in target mode to use
FileName instead of FunctionName. FunctionName is an empty
string so the log messages generated do not provide enough
information to know the source of a unit test failure.
Using FileName combined with LineNumber provides the
right information to identify the location of a unit test
failure.
Cc: Sean Brogan
Cc: Bret Barkelew
Cc: Jiewen Yao
Signed-off-by: Michael D Kinney
---
.../Library/UnitTestLib/Assert.c | 64 +--
1 file changed, 32 insertions(+), 32 deletions(-)
diff --git a/UnitTestFrameworkPkg/Library/UnitTestLib/Assert.c
b/UnitTestFrameworkPkg/Library/UnitTestLib/Assert.c
index c327ba88f1..8a131fab2b 100644
--- a/UnitTestFrameworkPkg/Library/UnitTestLib/Assert.c
+++ b/UnitTestFrameworkPkg/Library/UnitTestLib/Assert.c
@@ -105,14 +105,14 @@ UnitTestAssertTrue (
if (!Expression) {
UnitTestLogFailure (
FAILURETYPE_ASSERTTRUE,
- "%a::%d Expression (%a) is not TRUE!\n",
- FunctionName,
+ "%a:%d: Expression (%a) is not TRUE!\n",
+ FileName,
LineNumber,
Description
);
UT_LOG_ERROR (
- "[ASSERT FAIL] %a::%d Expression (%a) is not TRUE!\n",
- FunctionName,
+ "[ASSERT FAIL] %a:%d: Expression (%a) is not TRUE!\n",
+ FileName,
LineNumber,
Description
);
@@ -151,14 +151,14 @@ UnitTestAssertFalse (
if (Expression) {
UnitTestLogFailure (
FAILURETYPE_ASSERTFALSE,
- "%a::%d Expression(%a) is not FALSE!\n",
- FunctionName,
+ "%a:%d: Expression(%a) is not FALSE!\n",
+ FileName,
LineNumber,
Description
);
UT_LOG_ERROR (
- "[ASSERT FAIL] %a::%d Expression (%a) is not FALSE!\n",
- FunctionName,
+ "[ASSERT FAIL] %a:%d: Expression (%a) is not FALSE!\n",
+ FileName,
LineNumber,
Description
);
@@ -197,15 +197,15 @@ UnitTestAssertNotEfiError (
if (EFI_ERROR (Status)) {
UnitTestLogFailure (
FAILURETYPE_ASSERTNOTEFIERROR,
- "%a::%d Status '%a' is EFI_ERROR (%r)!\n",
- FunctionName,
+ "%a:%d: Status '%a' is EFI_ERROR (%r)!\n",
+ FileName,
LineNumber,
Description,
Status
);
UT_LOG_ERROR (
- "[ASSERT FAIL] %a::%d Status '%a' is EFI_ERROR (%r)!\n",
- FunctionName,
+ "[ASSERT FAIL] %a:%d: Status '%a' is EFI_ERROR (%r)!\n",
+ FileName,
LineNumber,
Description,
Status
@@ -250,8 +250,8 @@ UnitTestAssertEqual (
if (ValueA != ValueB) {
UnitTestLogFailure (
FAILURETYPE_ASSERTEQUAL,
- "%a::%d Value %a != %a (%d != %d)!\n",
- FunctionName,
+ "%a:%d: Value %a != %a (%d != %d)!\n",
+ FileName,
LineNumber,
DescriptionA,
DescriptionB,
@@ -259,8 +259,8 @@ UnitTestAssertEqual (
ValueB
);
UT_LOG_ERROR (
- "[ASSERT FAIL] %a::%d Value %a != %a (%d != %d)!\n",
- FunctionName,
+ "[ASSERT FAIL] %a:%d: Value %a != %a (%d != %d)!\n",
+ FileName,
LineNumber,
DescriptionA,
DescriptionB,
@@ -312,16 +312,16 @@ UnitTestAssertMemEqual (
if (CompareMem(BufferA, BufferB, Length) != 0) {
UnitTestLogFailure (
FAILURETYPE_ASSERTEQUAL,
- "%a::%d Memory at %a != %a for length %d bytes!\n",
- FunctionName,
+ "%a:%d: Memory at %a != %a for length %d bytes!\n",
+ FileName,
LineNumber,
DescriptionA,
DescriptionB,
Length
);
UT_LOG_ERROR (
- "[ASSERT FAIL] %a::%d Value %a != %a for length %d bytes!\n",
- FunctionName,
+ "[ASSERT FAIL] %a:%d: Value %a != %a for length %d bytes!\n",
+ FileName,
LineNumber,
DescriptionA,
DescriptionB,
@@ -368,8 +368,8 @@ UnitTestAssertNotEqual (
if (ValueA == ValueB) {
UnitTestLogFailure (
FAILURETYPE_ASSERTNOTEQUAL,
- "%a::%d Value %a == %a (%d == %d)!\n",
- FunctionName,
+ "%a:%d: Value %a == %a (%d == %d)!\n",
+ FileName,
LineNumber,
DescriptionA,
DescriptionB,
@@ -377,8 +377,8 @@ UnitTestAssertNotEqual (
ValueB
);
UT_LOG_ERROR (
- "[ASSERT FAIL] %a::%d Value %a == %a (%d == %d)!\n",
- FunctionName,
+ "[ASSERT FAIL] %a:%d: Value %a == %a (%d == %d)!\n",
+ FileName,
LineNumber,
DescriptionA,
DescriptionB,
@@ -423,16 +423,16 @@ UnitTestAssertStatusEqual (
if (Status != Expected) {
UnitTestLogFailure (
FAILURETYPE_ASSERTSTATUSEQUAL,
- "%a::%d Status '%a' is %r, should be %r!\n",
- FunctionName,
+ "%a:%d: Status '%a' is %r, should be %r!\n",
+ FileName,
LineNumber,
Description,
Status,
Expected
);
UT_LOG_ERROR (
- "[ASSERT FAIL] %a::%d Status '%a' is %r, should be %r!\n",
- FunctionName,
+ "[ASSERT FAIL] %a:%d: Status '%a' is %r, should be %r!\n",
+
[edk2-devel] [Patch v3 09/16] UnitTestFrameworkPkg: Change target mode DebugLib mapping
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2801 The default DebugLib for target mode was DebugLibNull. This library instance disables all ASSERT() and DEBUG() macros which removes the ability to write unit tests that check for ASSERT() behaviors. The DebugLib is changed to PeiDxeDebugLibReportStatusCode.inf that guarantees that DEBUG() and ASSERT() macros are active. The default ReportStatusCodeLib is set to BaseReportStatusCodeLibNull.inf so no messages are sent to any devices preserving the DebugLibNull behavior. A platform specific unit test can always override these mappings with a platform specific DebugLib. Cc: Sean Brogan Cc: Bret Barkelew Cc: Jiewen Yao Signed-off-by: Michael D Kinney --- UnitTestFrameworkPkg/UnitTestFrameworkPkgTarget.dsc.inc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/UnitTestFrameworkPkg/UnitTestFrameworkPkgTarget.dsc.inc b/UnitTestFrameworkPkg/UnitTestFrameworkPkgTarget.dsc.inc index c29b056fca..0881278ab0 100644 --- a/UnitTestFrameworkPkg/UnitTestFrameworkPkgTarget.dsc.inc +++ b/UnitTestFrameworkPkg/UnitTestFrameworkPkgTarget.dsc.inc @@ -16,7 +16,9 @@ [LibraryClasses] BaseLib|MdePkg/Library/BaseLib/BaseLib.inf BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf - DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf + DebugLib|MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf + ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf + DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf -- 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62361): https://edk2.groups.io/g/devel/message/62361 Mute This Topic: https://groups.io/mt/75432221/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch v3 10/16] UnitTestFrameworkPkg/UnitTestLib: Move print log into cleanup
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2805
If a unit test fails with an exception or an assert, then the
CmockaUnitTestFunctionRunner() is terminated and the logic
that follows the invocation of the unit test is skipped. This
currently skips the logic that prints log messages.
Move the print of log messages to the end of the function
CmockaUnitTestTeardownFunctionRunner() that is guaranteed to
be executed when a unit test completes normally or is
terminated with an exception or an assert.
Cc: Sean Brogan
Cc: Bret Barkelew
Cc: Jiewen Yao
Signed-off-by: Michael D Kinney
---
.../Library/UnitTestLib/RunTestsCmocka.c | 33 +--
1 file changed, 16 insertions(+), 17 deletions(-)
diff --git a/UnitTestFrameworkPkg/Library/UnitTestLib/RunTestsCmocka.c
b/UnitTestFrameworkPkg/Library/UnitTestLib/RunTestsCmocka.c
index fb81cc9658..96aa4d9b13 100644
--- a/UnitTestFrameworkPkg/Library/UnitTestLib/RunTestsCmocka.c
+++ b/UnitTestFrameworkPkg/Library/UnitTestLib/RunTestsCmocka.c
@@ -53,21 +53,9 @@ CmockaUnitTestFunctionRunner (
UnitTest->Result = UNIT_TEST_SKIPPED;
} else {
UnitTest->Result = UNIT_TEST_RUNNING;
-
Framework->CurrentTest = UnitTest;
UnitTest->Result = UnitTest->RunTest (UnitTest->Context);
Framework->CurrentTest = NULL;
-
-// Print out the log messages - This is a partial solution as it
-// does not get the log into the XML. Need cmocka changes to support
-// stdout and stderr in their xml format
-//
-if (UnitTest->Log != NULL) {
- print_message("UnitTest: %s - %s\n", UnitTest->Name,
UnitTest->Description);
- print_message("Log Output Start\n");
- print_message("%s", UnitTest->Log);
- print_message("Log Output End\n");
-}
}
}
@@ -112,13 +100,24 @@ CmockaUnitTestTeardownFunctionRunner (
Suite = (UNIT_TEST_SUITE *)(UnitTest->ParentSuite);
Framework = (UNIT_TEST_FRAMEWORK *)(Suite->ParentFramework);
- if (UnitTest->CleanUp == NULL) {
-return 0;
+ if (UnitTest->CleanUp != NULL) {
+Framework->CurrentTest = UnitTest;
+UnitTest->CleanUp (UnitTest->Context);
+Framework->CurrentTest = NULL;
+ }
+
+ //
+ // Print out the log messages - This is a partial solution as it
+ // does not get the log into the XML. Need cmocka changes to support
+ // stdout and stderr in their xml format
+ //
+ if (UnitTest->Log != NULL) {
+print_message("UnitTest: %s - %s\n", UnitTest->Name,
UnitTest->Description);
+print_message("Log Output Start\n");
+print_message("%s", UnitTest->Log);
+print_message("Log Output End\n");
}
- Framework->CurrentTest = UnitTest;
- UnitTest->CleanUp (UnitTest->Context);
- Framework->CurrentTest = NULL;
//
// Return 0 for success. Non-zero for error.
//
--
2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#62362): https://edk2.groups.io/g/devel/message/62362
Mute This Topic: https://groups.io/mt/7543/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [Patch v3 02/16] MdePkg/BaseCpuLibNull: Add Null version of CpuLib for host testing
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2798
The services in CpuLib usually generate exceptions in a unit test
host application. Provide a Null instance that can be safely used.
This Null instance can also be used as a template for implementing
new instances of CpuLib.
Cc: Liming Gao
Cc: Sean Brogan
Cc: Bret Barkelew
Cc: Jiewen Yao
Signed-off-by: Michael D Kinney
Reviewed-by: Liming Gao
Reviewed-by: Sean Brogan
---
.../Library/BaseCpuLibNull/BaseCpuLibNull.c | 37 +++
.../Library/BaseCpuLibNull/BaseCpuLibNull.inf | 26 +
.../Library/BaseCpuLibNull/BaseCpuLibNull.uni | 11 ++
MdePkg/MdePkg.dsc | 3 +-
4 files changed, 76 insertions(+), 1 deletion(-)
create mode 100644 MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c
create mode 100644 MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
create mode 100644 MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.uni
diff --git a/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c
b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c
new file mode 100644
index 00..3ba7a35096
--- /dev/null
+++ b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c
@@ -0,0 +1,37 @@
+/** @file
+ Null instance of CPU Library.
+
+ Copyright (c) 2020, Intel Corporation. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+/**
+ Places the CPU in a sleep state until an interrupt is received.
+
+ Places the CPU in a sleep state until an interrupt is received. If interrupts
+ are disabled prior to calling this function, then the CPU will be placed in a
+ sleep state indefinitely.
+
+**/
+VOID
+EFIAPI
+CpuSleep (
+ VOID
+ )
+{
+}
+
+/**
+ Flushes all the Translation Lookaside Buffers(TLB) entries in a CPU.
+
+ Flushes all the Translation Lookaside Buffers(TLB) entries in a CPU.
+
+**/
+VOID
+EFIAPI
+CpuFlushTlb (
+ VOID
+ )
+{
+}
diff --git a/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
new file mode 100644
index 00..a9e8399038
--- /dev/null
+++ b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
@@ -0,0 +1,26 @@
+## @file
+# Null instance of CPU Library.
+#
+# Copyright (c) 2020, Intel Corporation. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION= 0x00010005
+ BASE_NAME = BaseCpuLibNull
+ MODULE_UNI_FILE= BaseCpuLibNull.uni
+ FILE_GUID = 8A29AAA5-0FB7-44CC-8709-1344FE89B878
+ MODULE_TYPE= BASE
+ VERSION_STRING = 1.0
+ LIBRARY_CLASS = CpuLib
+
+#
+# VALID_ARCHITECTURES = IA32 X64 EBC ARM AARCH64 RISCV64
+#
+
+[Sources]
+ BaseCpuLibNull.c
+
+[Packages]
+ MdePkg/MdePkg.dec
diff --git a/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.uni
b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.uni
new file mode 100644
index 00..1030221d5c
--- /dev/null
+++ b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.uni
@@ -0,0 +1,11 @@
+// /** @file
+// Null instance of CPU Library.
+//
+// Copyright (c) 2020, Intel Corporation. All rights reserved.
+// SPDX-License-Identifier: BSD-2-Clause-Patent
+//
+// **/
+
+#string STR_MODULE_ABSTRACT #language en-US "Null Instance of CPU
Library"
+
+#string STR_MODULE_DESCRIPTION #language en-US "Null instance of CPU
Library."
diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc
index 6cd38e7ec3..3abe65ec7f 100644
--- a/MdePkg/MdePkg.dsc
+++ b/MdePkg/MdePkg.dsc
@@ -1,7 +1,7 @@
## @file
# EFI/PI MdePkg Package
#
-# Copyright (c) 2007 - 2018, Intel Corporation. All rights reserved.
+# Copyright (c) 2007 - 2020, Intel Corporation. All rights reserved.
# Portions copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
# (C) Copyright 2020 Hewlett Packard Enterprise Development LP
#
@@ -36,6 +36,7 @@ [Components]
MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
MdePkg/Library/BaseCacheMaintenanceLib/BaseCacheMaintenanceLib.inf
MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
+ MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.inf
MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
--
2.21.0.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#62354): https://edk2.groups.io/g/devel/message/62354
Mute This Topic: https://groups.io/mt/75432214/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v4 07/15] MdeModulePkg: Fix constructor invocation ordering
Hi Hao, Jian,
Can you let me know your feedback for this patch, please?
Previous discussion on this patch can be seen at
https://edk2.groups.io/g/devel/topic/75081484
Regards,
Sami Mujawar
-Original Message-
From: devel@edk2.groups.io On Behalf Of Sami Mujawar via
groups.io
Sent: 07 July 2020 01:48 PM
To: devel@edk2.groups.io
Cc: Sami Mujawar ; Ard Biesheuvel
; l...@nuviainc.com; ler...@redhat.com;
jian.j.w...@intel.com; hao.a...@intel.com; Alexandru Elisei
; Andre Przywara ; Matteo
Carlini ; Laura Moretta ; nd
Subject: [edk2-devel] [PATCH v4 07/15] MdeModulePkg: Fix constructor invocation
ordering
The BaseSerialPortLib16550 library does not implement a constructor. This
prevents the correct constructor invocation order for dependent libraries.
e.g. A PlatformHookLib (for the Serial Port) may have a dependency on
retrieving data from a Hob. A Hob library implementation may configure its
initial state in the HobLib constructor. Since BaseSerialPortLib16550 does not
implement a constructor, the Basetools do not resolve the correct order for
constructor invocation.
To fix this, add an empty constructor to the serial port library
BaseSerialPortLib16550.
Signed-off-by: Sami Mujawar
Acked-by: Ard Biesheuvel
---
Notes:
v4:
- No code change, resending patch with v4 series. [Sami]
Ref: https://edk2.groups.io/g/devel/topic/75081484
MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c | 17
+
MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf | 3 +++
2 files changed, 20 insertions(+)
diff --git
a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
index
9cb50dd80d5634ab2aa6d68bf5ca7fb891463eef..0fd1382ee83c9de09d8250830bd9569056fcee2f
100644
--- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
+++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550
+++ .c
@@ -4,6 +4,7 @@
(C) Copyright 2014 Hewlett-Packard Development Company, L.P.
Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
Copyright (c) 2018, AMD Incorporated. All rights reserved.
+ Copyright (c) 2020, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -1102,3 +1103,19 @@ SerialPortSetAttributes (
return RETURN_SUCCESS;
}
+/** Base Serial Port 16550 Library Constructor
+
+ @retval RETURN_SUCCESS Success.
+*/
+EFI_STATUS
+EFIAPI
+BaseSerialPortLib16550 (
+ VOID
+ )
+{
+ // Nothing to do here. This constructor is added to
+ // enable the chain of constructor invocation for
+ // dependent libraries.
+ return RETURN_SUCCESS;
+}
+
diff --git
a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
index
8b4ae3f1d4ee1e2e9a8b81eab4c900541ce8cfb6..92b7a8b7896a305d2ce22589f8a9593618d37bb7
100644
--- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550
+++ .inf
@@ -2,6 +2,8 @@
# SerialPortLib instance for 16550 UART.
#
# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
+# Copyright (c) 2020, ARM Limited. All rights reserved.
+#
# SPDX-License-Identifier: BSD-2-Clause-Patent # ## @@ -14,6 +16,7 @@
[Defines]
MODULE_TYPE= BASE
VERSION_STRING = 1.1
LIBRARY_CLASS = SerialPortLib
+ CONSTRUCTOR= BaseSerialPortLib16550
[Packages]
MdePkg/MdePkg.dec
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#62351): https://edk2.groups.io/g/devel/message/62351
Mute This Topic: https://groups.io/mt/75423440/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [Patch 04/15] MdePkg/BaseLib: Break out IA32/X64 GCC inline privileged functions
Liming,
GccInline.c can be used in UnitTestHostBaseLib instance.
GccInlinePriv.c is the file that cannot be used in the.
UnitTestHostBaseLib instance.
Here are the grep results in the branch:
baselib.inf: Ia32/GccInline.c | GCC
baselib.inf: Ia32/GccInlinePriv.c | GCC
baselib.inf: X64/GccInline.c | GCC
baselib.inf: X64/GccInlinePriv.c | GCC
unittesthostbaselib.inf: Ia32/GccInline.c | GCC
unittesthostbaselib.inf: X64/GccInline.c | GCC
Best regards,
Mike
> -Original Message-
> From: Gao, Liming
> Sent: Friday, July 10, 2020 1:06 AM
> To: Kinney, Michael D ;
> devel@edk2.groups.io
> Cc: Sean Brogan ; Bret
> Barkelew ; Yao, Jiewen
>
> Subject: RE: [Patch 04/15] MdePkg/BaseLib: Break out
> IA32/X64 GCC inline privileged functions
>
> Mike:
> So, GccInline.c file can't be used in the unit test
> BaseLib.
>
> Thanks
> Liming
> -Original Message-
> From: Kinney, Michael D
> Sent: 2020年7月10日 1:01
> To: Gao, Liming ;
> devel@edk2.groups.io; Kinney, Michael D
>
> Cc: Sean Brogan ; Bret
> Barkelew ; Yao, Jiewen
>
> Subject: RE: [Patch 04/15] MdePkg/BaseLib: Break out
> IA32/X64 GCC inline privileged functions
>
> Liming,
>
> In order to support host based unit tests, all the code
> has to be compatible with a user mode process. This
> means
> any functions that execute instructions that are not
> allowed
> in a user mode process need to be separated out.
>
> For the IA32/X64 specific GccInline.c files, the
> analysis
> was done for APIs that use instructions that would
> generate an exception in a user mode process.
>
> In addition, the BaseLib functions that return system
> state (e.g. CS/DS/ES/SS/FS/GS) are also split out. It
> does not make sense to return system state information
> from the currently executing user mode process.
> Instead,
> the FW code under test would expect system state that
> matches a FW execution environment. Each unit test
> implementation has the option of providing emulation
> of this system state if that is required to exercise the
> code under test.
>
> No changes to the MSFT specific BaseLib implementation
> were required because there those are already split out
> to one function per file.
>
> Best regards,
>
> Mike
>
> > -Original Message-
> > From: Gao, Liming
> > Sent: Thursday, July 9, 2020 7:07 AM
> > To: Kinney, Michael D ;
> > devel@edk2.groups.io
> > Cc: Sean Brogan ; Bret
> > Barkelew ; Yao, Jiewen
> >
> > Subject: RE: [Patch 04/15] MdePkg/BaseLib: Break out
> > IA32/X64 GCC inline privileged functions
> >
> > Mike:
> > Is there the rule to know which function can't be in
> > unit test? And, those functions will be in GccInline.c
> > or GccInlinePriv.c? There is no change in MSFT C
> source
> > file, because MSFT C source file has been separated as
> > the function level. Right?
> >
> > Thanks
> > Liming
> > > -Original Message-
> > > From: Kinney, Michael D
> > > Sent: Monday, June 15, 2020 8:19 AM
> > > To: devel@edk2.groups.io
> > > Cc: Gao, Liming ; Sean Brogan
> > ; Bret Barkelew
> > ;
> > > Yao, Jiewen
> > > Subject: [Patch 04/15] MdePkg/BaseLib: Break out
> > IA32/X64 GCC inline privileged functions
> > >
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=2800
> > >
> > > Break out the IA32/X64 GCC inline functions that can
> > not be used
> > > in a unit test host application into their own
> source
> > file. This
> > > does not make any changes to the BaseLib library
> > instance. This
> > > is in preparation for a new BaseLib instances that
> is
> > safe to use
> > > with host-based unit test applications.
> > >
> > > Cc: Liming Gao
> > > Cc: Sean Brogan
> > > Cc: Bret Barkelew
> > > Cc: Jiewen Yao
> > > Signed-off-by: Michael D Kinney
> >
> > > ---
> > > MdePkg/Library/BaseLib/BaseLib.inf|
> 4
> > +-
> > > MdePkg/Library/BaseLib/Ia32/GccInline.c |
> 1181
> > +---
> > > .../Ia32/{GccInline.c => GccInlinePriv.c} |
> 601
> > +---
> > > MdePkg/Library/BaseLib/X64/GccInline.c|
> 1240
> > +
> > > .../X64/{GccInline.c => GccInlinePriv.c} |
> 572
> > +---
> > > 5 files changed, 11 insertions(+), 3587 deletions(-
> )
> > > copy MdePkg/Library/BaseLib/Ia32/{GccInline.c =>
> > GccInlinePriv.c} (62%)
> > > copy MdePkg/Library/BaseLib/X64/{GccInline.c =>
> > GccInlinePriv.c} (65%)
> > >
> > > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> > b/MdePkg/Library/BaseLib/BaseLib.inf
> > > index a57ae2da31..c740a819ca 100644
> > > --- a/MdePkg/Library/BaseLib/BaseLib.inf
> > > +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> > > @@ -1,7 +1,7 @@
> > > ## @file
> > > # Base Library implementation.
> > > #
> > > -# Copyright (c) 2007 - 2019, Intel Corporation.
> All
> > rights reserved.
> > > +# Copyright (c) 2007 - 2020, Intel Corporation.
> All
> > rights reserved.
> > > # Portions copyright (c) 2008 - 2009, Apple Inc.
> All
> > rights reserved.
> > > # Portions copyright (c) 2011 - 2013, ARM Ltd. All
> >
Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix incorrect TCG VER comparision
On 7/10/20 9:53 AM, Stefan Berger wrote:
On 7/10/20 1:43 AM, Laszlo Ersek wrote:
(+Marc-André, Stefan)
On 07/10/20 02:44, Gao, Zhichao wrote:
This bug is not obeserved by me. But I view the code. The condition
is incorrect and it would affect the TCG operation:
if (!mIsTcg2PPVerLowerThan_1_3) {
if (OperationRequest <
TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
//
// TCG2 PP1.3 spec defined operations that are reserved
or un-implemented
//
return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
}
} else {
//
// TCG PP lower than 1.3. (1.0, 1.1, 1.2)
//
if (OperationRequest <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) {
RequestConfirmed = TRUE;
} else if (OperationRequest <
TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
}
}
I've found that code myself, but I'm not familiar enough with TPM PPI
stuff to understand immediately the effects of this change. I can see
that where we used to return
TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED before, we could now assign
"RequestConfirmed = TRUE", and vice versa, due to
"mIsTcg2PPVerLowerThan_1_3" being potentially inverted.
But what does that *mean*? What is the behavioral change that human
end-users, or software components, will experience?
The above code snipped is located in a default branch of a large
switch statement that handles most of the common PPI operations
independent of this change, so that at least is good.
I would say that in the worst case some of the operations not
otherwise handled may have mistakenly failed or could have been
executed without user confirmation/interaction. On Linux at least PPI
requests can only be sent by root.
I am running a somewhat dated version of edk2 (Fedora 31). The
operations advertised are: 0,5,14,21,22,23,24,33,96,97. All of these are
individually handled in the switch statement, so there should no be any
impact. I am currently not aware of whether this list can be extended
with some sort of module.
Thanks
Laszlo
So I think it should be fixed.
Thanks,
Zhichao
-Original Message-
From: devel@edk2.groups.io On Behalf Of
Laszlo Ersek
Sent: Thursday, July 9, 2020 6:02 PM
To: devel@edk2.groups.io; Gao, Zhichao
Cc: Terry Lee ; Yao, Jiewen
; Wang,
Jian J ; Zhang, Chao B
Subject: Re: [edk2-devel] [PATCH]
SecurityPkg/Tcg2PhysicalPresenceLib: Fix
incorrect TCG VER comparision
On 07/09/20 04:46, Gao, Zhichao wrote:
From: Terry Lee
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2697
Tcg2PhysicalPresenceLibConstructor set the module variable
mIsTcg2PPVerLowerThan_1_3 with incorrect TCG version comparision.
Cc: Jiewen Yao
Cc: Jian J Wang
Cc: Chao Zhang
Signed-off-by: Zhichao Gao
---
.../SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
ceLib.c
b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
ceLib.c
index 1c46d5e69d..8afaa0a785 100644
---
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
ceLib.c
+++
b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPr
+++ esenceLib.c
@@ -387,7 +387,7 @@ Tcg2PhysicalPresenceLibConstructor ( {
EFI_STATUS Status;
- if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8
*)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer),
sizeof(PP_INF_VERSION_1_2) - 1) <= 0) {
+ if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8
+ *)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer),
+ sizeof(PP_INF_VERSION_1_2) - 1) >= 0) {
mIsTcg2PPVerLowerThan_1_3 = TRUE;
}
What is the practical impact of this bug / fix?
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#62348): https://edk2.groups.io/g/devel/message/62348
Mute This Topic: https://groups.io/mt/75390754/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix incorrect TCG VER comparision
On 7/10/20 1:43 AM, Laszlo Ersek wrote:
(+Marc-André, Stefan)
On 07/10/20 02:44, Gao, Zhichao wrote:
This bug is not obeserved by me. But I view the code. The condition is
incorrect and it would affect the TCG operation:
if (!mIsTcg2PPVerLowerThan_1_3) {
if (OperationRequest <
TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
//
// TCG2 PP1.3 spec defined operations that are reserved or
un-implemented
//
return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
}
} else {
//
// TCG PP lower than 1.3. (1.0, 1.1, 1.2)
//
if (OperationRequest <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) {
RequestConfirmed = TRUE;
} else if (OperationRequest <
TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
}
}
I've found that code myself, but I'm not familiar enough with TPM PPI
stuff to understand immediately the effects of this change. I can see
that where we used to return
TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED before, we could now assign
"RequestConfirmed = TRUE", and vice versa, due to
"mIsTcg2PPVerLowerThan_1_3" being potentially inverted.
But what does that *mean*? What is the behavioral change that human
end-users, or software components, will experience?
The above code snipped is located in a default branch of a large switch
statement that handles most of the common PPI operations independent of
this change, so that at least is good.
I would say that in the worst case some of the operations not otherwise
handled may have mistakenly failed or could have been executed without
user confirmation/interaction. On Linux at least PPI requests can only
be sent by root.
Thanks
Laszlo
So I think it should be fixed.
Thanks,
Zhichao
-Original Message-
From: devel@edk2.groups.io On Behalf Of Laszlo Ersek
Sent: Thursday, July 9, 2020 6:02 PM
To: devel@edk2.groups.io; Gao, Zhichao
Cc: Terry Lee ; Yao, Jiewen ; Wang,
Jian J ; Zhang, Chao B
Subject: Re: [edk2-devel] [PATCH] SecurityPkg/Tcg2PhysicalPresenceLib: Fix
incorrect TCG VER comparision
On 07/09/20 04:46, Gao, Zhichao wrote:
From: Terry Lee
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2697
Tcg2PhysicalPresenceLibConstructor set the module variable
mIsTcg2PPVerLowerThan_1_3 with incorrect TCG version comparision.
Cc: Jiewen Yao
Cc: Jian J Wang
Cc: Chao Zhang
Signed-off-by: Zhichao Gao
---
.../SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
ceLib.c
b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
ceLib.c
index 1c46d5e69d..8afaa0a785 100644
---
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresen
ceLib.c
+++ b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPr
+++ esenceLib.c
@@ -387,7 +387,7 @@ Tcg2PhysicalPresenceLibConstructor ( {
EFI_STATUS Status;
- if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8
*)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer),
sizeof(PP_INF_VERSION_1_2) - 1) <= 0) {
+ if (AsciiStrnCmp(PP_INF_VERSION_1_2, (CHAR8
+ *)PcdGetPtr(PcdTcgPhysicalPresenceInterfaceVer),
+ sizeof(PP_INF_VERSION_1_2) - 1) >= 0) {
mIsTcg2PPVerLowerThan_1_3 = TRUE;
}
What is the practical impact of this bug / fix?
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#62347): https://edk2.groups.io/g/devel/message/62347
Mute This Topic: https://groups.io/mt/75390754/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] edk2-platform configure serial UART for debugging
the recommendations from: https://software.intel.com/content/dam/develop/external/us/en/documents/uefi-firmware-porting-guide-for-the-intel-atom-processor-e3900-series-820187.pdf to edit: gPlatformModuleTokenSpaceGuid.PcdSerialIoUartNumber|2|UINT8|0x10001011 But how can I set UART2 for serial output ? -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62346): https://edk2.groups.io/g/devel/message/62346 Mute This Topic: https://groups.io/mt/75416096/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [edk2-discuss] Need memory barriers in IoLib for AARCH64
Hello, MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf: IoLib library uses IoLibArm.c for AARCH64/ARM architecture and IoLib.c for other architectures. While IoLib.c already has memory barriers in MmioWrite functions, there barriers are missing in IoLibArm.c Is there any reason for **not** adding these memory barriers in IoLibArm.c to guarantee that all MMIO operations are serialized ? I am facing some issues and I need to add memory barriers in IoLibArm.c for AARCH64 also . Regards, Wasim -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62345): https://edk2.groups.io/g/devel/message/62345 Mute This Topic: https://groups.io/mt/75417094/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [edk2: PATCH] UefiCpuPkg: To enable X2Apic by default
On 07/10/20 07:38, Laszlo Ersek wrote:
> On 07/10/20 05:31, Ji-yunX Lu wrote:
>> BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2845
>> Platform shall enable X2APIC by default to meet requirements for interrupt
>> steering policy on Windows OS.
>>
>> Change-Id: Ia9e24bce79c91762c560fa3de6260716939f0b1b
>> Signed-off-by: Ji-yunX Lu
>> Cc: Eric Dong
>> Cc: Ray Ni
>> Cc: Laszlo Ersek
>> Cc: Rahul Kumar
>> ---
>> UefiCpuPkg/Library/MpInitLib/MpLib.c | 21 -
>> 1 file changed, 4 insertions(+), 17 deletions(-)
>>
>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> index ab7a8ed663..70bc5da195 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> @@ -488,8 +488,8 @@ CollectProcessorCount (
>>)
>> {
>>UINTN Index;
>> - CPU_INFO_IN_HOB*CpuInfoInHob;
>>BOOLEANX2Apic;
>> + CPUID_VERSION_INFO_ECX VersionInfoEcx;
>>
>>//
>>// Send 1st broadcast IPI to APs to wakeup APs
>> @@ -505,26 +505,13 @@ CollectProcessorCount (
>> CpuPause ();
>>}
>>
>> -
>> - //
>> - // Enable x2APIC mode if
>> - // 1. Number of CPU is greater than 255; or
>> - // 2. There are any logical processors reporting an Initial APIC ID of
>> 255 or greater.
>> - //
>>X2Apic = FALSE;
>> - if (CpuMpData->CpuCount > 255) {
>> + AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, , NULL);
>> + if (VersionInfoEcx.Bits.x2APIC == 1) {
>> //
>> -// If there are more than 255 processor found, force to enable X2APIC
>> +// Enable x2APIC mode if capable
>> //
>> X2Apic = TRUE;
>> - } else {
>> -CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob;
>> -for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
>> - if (CpuInfoInHob[Index].InitialApicId >= 0xFF) {
>> -X2Apic = TRUE;
>> -break;
>> - }
>> -}
>>}
>>
>>if (X2Apic) {
>>
>
> (1) I think this would break platforms that resolve the LocalApicLib
> class to the "BaseXApicLib.inf" instance.
>
> Based on the message of my earlier commit decb365b0016 ("OvmfPkg: select
> LocalApicLib instance with x2apic support", 2015-11-30), it seems like
> the BaseXApicLib instance notices and trips an assert when the LAPIC is
> in X2APIC mode, at the next time a LocalApicLib API is used.
>
> The BaseXApicLib instance contains many ASSERTs like this:
>
> ASSERT (GetApicMode () == LOCAL_APIC_MODE_XAPIC);
>
> and the GetApicMode() function itself has the following ASSERT:
>
> ASSERT (ApicBaseMsr.Bits.EXTD == 0);
>
> So, the change proposed in this patch needs to be gated by a boolean or
> Feature PCD, and the PCD should default to FALSE. If the platform uses
> the BaseXApicX2ApicLib instance, then it can set the PCD to TRUE.
>
> In turn, for such platform DSCs that already use BaseXApicX2ApicLib
> exclusively, in edk2 and in edk2-platforms, please post patches that set
> the PCD to TRUE. This includes the OvmfPkg platform DSC files, for example.
If a PCD is considered overkill for this, then a new API could be
declared in LocalApicLib.
UINTN
EFIAPI
GetMaxApicMode (
VOID
);
In BaseXApicLib, the function would return constant LOCAL_APIC_MODE_XAPIC.
In BaseXApicX2ApicLib, the function would call the AsmCpuid() seen above
in the patch, and return LOCAL_APIC_MODE_XAPIC or
LOCAL_APIC_MODE_X2APIC, dependent on "VersionInfoEcx.Bits.x2APIC".
And then this patch would call GetMaxApicMode(), rather than check CPUID.
Thanks
Laszlo
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#62344): https://edk2.groups.io/g/devel/message/62344
Mute This Topic: https://groups.io/mt/75413450/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [edk2: PATCH] UefiCpuPkg: To enable X2Apic by default
BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=2845
Platform shall enable X2APIC by default to meet requirements for interrupt
steering policy on Windows OS.
Change-Id: Ia9e24bce79c91762c560fa3de6260716939f0b1b
Signed-off-by: Ji-yunX Lu
Cc: Eric Dong
Cc: Ray Ni
Cc: Laszlo Ersek
Cc: Rahul Kumar
---
UefiCpuPkg/Library/MpInitLib/MpLib.c | 21 -
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index ab7a8ed663..70bc5da195 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
@@ -488,8 +488,8 @@ CollectProcessorCount (
)
{
UINTN Index;
- CPU_INFO_IN_HOB*CpuInfoInHob;
BOOLEANX2Apic;
+ CPUID_VERSION_INFO_ECX VersionInfoEcx;
//
// Send 1st broadcast IPI to APs to wakeup APs
@@ -505,26 +505,13 @@ CollectProcessorCount (
CpuPause ();
}
-
- //
- // Enable x2APIC mode if
- // 1. Number of CPU is greater than 255; or
- // 2. There are any logical processors reporting an Initial APIC ID of 255
or greater.
- //
X2Apic = FALSE;
- if (CpuMpData->CpuCount > 255) {
+ AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, , NULL);
+ if (VersionInfoEcx.Bits.x2APIC == 1) {
//
-// If there are more than 255 processor found, force to enable X2APIC
+// Enable x2APIC mode if capable
//
X2Apic = TRUE;
- } else {
-CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob;
-for (Index = 0; Index < CpuMpData->CpuCount; Index++) {
- if (CpuInfoInHob[Index].InitialApicId >= 0xFF) {
-X2Apic = TRUE;
-break;
- }
-}
}
if (X2Apic) {
--
2.26.2.windows.1
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#62342): https://edk2.groups.io/g/devel/message/62342
Mute This Topic: https://groups.io/mt/75413450/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] edk2-platform configure serial UART for debugging
Hi I maybe missing some steps, because mostly spent time to use SBL (slimbootloader) . I've managed to build Apollo Lake IFWI image for the LeafHill platform. However, I need to use it for our own board based LeafHill. How can I configure UART2 to get a serial message ? I can not see the settings like in coreboot setup or SBL(slim boot) Can someone help me with that? Regards, Andrey -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62341): https://edk2.groups.io/g/devel/message/62341 Mute This Topic: https://groups.io/mt/75416096/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] edk2-platform configure serial UART for debugging
Hi I maybe missing some steps, because mostly spent time to use SBL (slimbootloader) . I've managed to build Apollo Lake IFWI image for the LeafHill platform. However, I need to use it for our own board based LeafHill. How can I configure UART2 to get a serial message ? I can not see the settings like in coreboot setup or SBL(slim boot) Can someone help me with that? Regards, Andrey -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62343): https://edk2.groups.io/g/devel/message/62343 Mute This Topic: https://groups.io/mt/75416096/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [edk2-platforms][PATCH v2 0/1] Platform/RaspberryPi/Drivers: Add SD card detection
v2 to correct a couple "STATIC" -> "static" that were applied by an overzealous text editor. No other changes from v1. Pete Batard (1): Platform/RaspberryPi/Drivers: Add SD card detection Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c | 66 +--- Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c | 65 --- Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h | 6 ++ 3 files changed, 120 insertions(+), 17 deletions(-) -- 2.21.0.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62339): https://edk2.groups.io/g/devel/message/62339 Mute This Topic: https://groups.io/mt/75415098/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
[edk2-devel] [edk2-platforms][PATCH v2 1/1] Platform/RaspberryPi/Drivers: Add SD card detection
The Raspberry Pi 3 and Pi 4 platforms (with latest EEPROM) can boot
straight from USB, without the need for an SD card being present.
However, the IsCardPresent () calls from the ArasanMmcHost and SdHost
drivers are currently hardwired to return TRUE, which results in
straight to USB boot failing due to the SD drivers looping on
errors while trying to poke at a non-existent card...
Ideally, we would use the Card Detect signal from the uSD slot, to
report on the presence or absence of a card, but the Raspberry Pi
Foundation did not wire those signals in the Pi 2 and subsequent
models, leaving us with only potentially interfering SD commands
as means to perform card detection.
As a result of this, we are left with no other choice but limit
detection to occurring only once, prior to formal SD card init,
and then return the detected value for subsequent calls. This,
however, is more than good enough for the intended purpose, which
is to allow straight to USB boot.
Tested on Raspberry Pi 3 and 4, and for both SD controllers.
Addresses pftf/RPi3#13, pftf/RPi3#14, pftf/RPi4#37.
Signed-off-by: Pete Batard
Reviewed-by: Andrei Warkentin
Tested-by: Andrei Warkentin
---
Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c | 66
+---
Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c | 65
---
Platform/RaspberryPi/Include/Protocol/RpiMmcHost.h | 6 ++
3 files changed, 120 insertions(+), 17 deletions(-)
diff --git a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
index 6d706af6f276..08e5be1f015f 100644
--- a/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
+++ b/Platform/RaspberryPi/Drivers/ArasanMmcHostDxe/ArasanMmcHostDxe.c
@@ -11,7 +11,8 @@
#define DEBUG_MMCHOST_SD DEBUG_VERBOSE
-BOOLEAN PreviousIsCardPresent = FALSE;
+BOOLEAN CardIsPresent = FALSE;
+CARD_DETECT_STATE CardDetectState = CardDetectRequired;
UINT32 LastExecutedCommand = (UINT32) -1;
STATIC RASPBERRY_PI_FIRMWARE_PROTOCOL *mFwProtocol;
@@ -239,14 +240,6 @@ CalculateClockFrequencyDivisor (
return EFI_SUCCESS;
}
-BOOLEAN
-MMCIsCardPresent (
- IN EFI_MMC_HOST_PROTOCOL *This
-)
-{
- return TRUE;
-}
-
BOOLEAN
MMCIsReadOnly (
IN EFI_MMC_HOST_PROTOCOL *This
@@ -418,6 +411,10 @@ MMCNotifyState (
DEBUG ((DEBUG_MMCHOST_SD, "ArasanMMCHost: MMCNotifyState(State: %d)\n",
State));
+ // Stall all operations except init until card detection has occurred.
+ if (State != MmcHwInitializationState && CardDetectState !=
CardDetectCompleted)
+return EFI_NOT_READY;
+
switch (State) {
case MmcHwInitializationState:
{
@@ -489,6 +486,57 @@ MMCNotifyState (
return EFI_SUCCESS;
}
+BOOLEAN
+MMCIsCardPresent (
+ IN EFI_MMC_HOST_PROTOCOL *This
+)
+{
+ EFI_STATUS Status;
+
+ //
+ // If we are already in progress (we may get concurrent calls)
+ // or completed the detection, just return the current value.
+ //
+ if (CardDetectState != CardDetectRequired)
+return CardIsPresent;
+
+ CardDetectState = CardDetectInProgress;
+ CardIsPresent = FALSE;
+
+ //
+ // The two following commands should succeed even if no card is present.
+ //
+ Status = MMCNotifyState (This, MmcHwInitializationState);
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: Error MmcHwInitializationState,
Status=%r.\n", Status));
+// If we failed init, go back to requiring card detection
+CardDetectState = CardDetectRequired;
+return FALSE;
+ }
+
+ Status = MMCSendCommand (This, MMC_CMD0, 0);
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "MMCIsCardPresent: CMD0 Error, Status=%r.\n",
Status));
+goto out;
+ }
+
+ //
+ // CMD8 should tell us if a card is present.
+ //
+ Status = MMCSendCommand (This, MMC_CMD8, CMD8_SD_ARG);
+ if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_INFO, "MMCIsCardPresent: No card detected, Status=%r.\n",
Status));
+goto out;
+ }
+
+ DEBUG ((DEBUG_INFO, "MMCIsCardPresent: Card detected.\n"));
+ CardIsPresent = TRUE;
+
+out:
+ CardDetectState = CardDetectCompleted;
+ return CardIsPresent;
+}
+
EFI_STATUS
MMCReceiveResponse (
IN EFI_MMC_HOST_PROTOCOL*This,
diff --git a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
index 2f31c5eb8c46..964b2d3ac5c1 100644
--- a/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
+++ b/Platform/RaspberryPi/Drivers/SdHostDxe/SdHostDxe.c
@@ -64,6 +64,8 @@ STATIC CONST CHAR8 *mFsmState[] = { "identmode", "datamode",
"readdata",
"genpulses", "writewait2", "?",
"startpowdown" };
#endif /* NDEBUG */
+STATIC BOOLEAN CardIsPresent = FALSE;
+STATIC CARD_DETECT_STATE CardDetectState = CardDetectRequired;
STATIC UINT32 mLastGoodCmd = MMC_GET_INDX (MMC_CMD0);
STATIC inline BOOLEAN
@@ -264,14 +266,6 @@
Re: [edk2-devel] [Patch 04/15] MdePkg/BaseLib: Break out IA32/X64 GCC inline privileged functions
Mike:
So, GccInline.c file can't be used in the unit test BaseLib.
Thanks
Liming
-Original Message-
From: Kinney, Michael D
Sent: 2020年7月10日 1:01
To: Gao, Liming ; devel@edk2.groups.io; Kinney, Michael D
Cc: Sean Brogan ; Bret Barkelew
; Yao, Jiewen
Subject: RE: [Patch 04/15] MdePkg/BaseLib: Break out IA32/X64 GCC inline
privileged functions
Liming,
In order to support host based unit tests, all the code
has to be compatible with a user mode process. This means
any functions that execute instructions that are not allowed
in a user mode process need to be separated out.
For the IA32/X64 specific GccInline.c files, the analysis
was done for APIs that use instructions that would
generate an exception in a user mode process.
In addition, the BaseLib functions that return system
state (e.g. CS/DS/ES/SS/FS/GS) are also split out. It
does not make sense to return system state information
from the currently executing user mode process. Instead,
the FW code under test would expect system state that
matches a FW execution environment. Each unit test
implementation has the option of providing emulation
of this system state if that is required to exercise the
code under test.
No changes to the MSFT specific BaseLib implementation
were required because there those are already split out
to one function per file.
Best regards,
Mike
> -Original Message-
> From: Gao, Liming
> Sent: Thursday, July 9, 2020 7:07 AM
> To: Kinney, Michael D ;
> devel@edk2.groups.io
> Cc: Sean Brogan ; Bret
> Barkelew ; Yao, Jiewen
>
> Subject: RE: [Patch 04/15] MdePkg/BaseLib: Break out
> IA32/X64 GCC inline privileged functions
>
> Mike:
> Is there the rule to know which function can't be in
> unit test? And, those functions will be in GccInline.c
> or GccInlinePriv.c? There is no change in MSFT C source
> file, because MSFT C source file has been separated as
> the function level. Right?
>
> Thanks
> Liming
> > -Original Message-
> > From: Kinney, Michael D
> > Sent: Monday, June 15, 2020 8:19 AM
> > To: devel@edk2.groups.io
> > Cc: Gao, Liming ; Sean Brogan
> ; Bret Barkelew
> ;
> > Yao, Jiewen
> > Subject: [Patch 04/15] MdePkg/BaseLib: Break out
> IA32/X64 GCC inline privileged functions
> >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2800
> >
> > Break out the IA32/X64 GCC inline functions that can
> not be used
> > in a unit test host application into their own source
> file. This
> > does not make any changes to the BaseLib library
> instance. This
> > is in preparation for a new BaseLib instances that is
> safe to use
> > with host-based unit test applications.
> >
> > Cc: Liming Gao
> > Cc: Sean Brogan
> > Cc: Bret Barkelew
> > Cc: Jiewen Yao
> > Signed-off-by: Michael D Kinney
>
> > ---
> > MdePkg/Library/BaseLib/BaseLib.inf|4
> +-
> > MdePkg/Library/BaseLib/Ia32/GccInline.c | 1181
> +---
> > .../Ia32/{GccInline.c => GccInlinePriv.c} | 601
> +---
> > MdePkg/Library/BaseLib/X64/GccInline.c| 1240
> +
> > .../X64/{GccInline.c => GccInlinePriv.c} | 572
> +---
> > 5 files changed, 11 insertions(+), 3587 deletions(-)
> > copy MdePkg/Library/BaseLib/Ia32/{GccInline.c =>
> GccInlinePriv.c} (62%)
> > copy MdePkg/Library/BaseLib/X64/{GccInline.c =>
> GccInlinePriv.c} (65%)
> >
> > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> b/MdePkg/Library/BaseLib/BaseLib.inf
> > index a57ae2da31..c740a819ca 100644
> > --- a/MdePkg/Library/BaseLib/BaseLib.inf
> > +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> > @@ -1,7 +1,7 @@
> > ## @file
> > # Base Library implementation.
> > #
> > -# Copyright (c) 2007 - 2019, Intel Corporation. All
> rights reserved.
> > +# Copyright (c) 2007 - 2020, Intel Corporation. All
> rights reserved.
> > # Portions copyright (c) 2008 - 2009, Apple Inc. All
> rights reserved.
> > # Portions copyright (c) 2011 - 2013, ARM Ltd. All
> rights reserved.
> > # Copyright (c) 2020, Hewlett Packard Enterprise
> Development LP. All rights reserved.
> > @@ -156,6 +156,7 @@ [Sources.Ia32]
> >
> >
> >Ia32/GccInline.c | GCC
> > + Ia32/GccInlinePriv.c | GCC
> >Ia32/Thunk16.nasm
> >Ia32/EnableDisableInterrupts.nasm| GCC
> >Ia32/EnablePaging64.nasm
> > @@ -310,6 +311,7 @@ [Sources.X64]
> >X86PatchInstruction.c
> >X86SpeculationBarrier.c
> >X64/GccInline.c | GCC
> > + X64/GccInlinePriv.c | GCC
> >X64/EnableDisableInterrupts.nasm
> >X64/DisablePaging64.nasm
> >X64/RdRand.nasm
> > diff --git a/MdePkg/Library/BaseLib/Ia32/GccInline.c
> b/MdePkg/Library/BaseLib/Ia32/GccInline.c
> > index 5287200f87..6ed938187a 100644
> > --- a/MdePkg/Library/BaseLib/Ia32/GccInline.c
> > +++ b/MdePkg/Library/BaseLib/Ia32/GccInline.c
> > @@ -1,7 +1,7 @@
> > /** @file
> >GCC inline implementation of BaseLib processor
> specific functions.
> >
> > - Copyright (c) 2006 - 2018, Intel Corporation. All
> rights reserved.
> > + Copyright (c) 2006 - 2020, Intel
Re: [edk2-devel] [PATCH 01/13] SecurityPkg/Tcg2Pei: Add missing PCRIndex in FvBlob event.
Reviewed-by: Jian J Wang Regards, Jian > -Original Message- > From: devel@edk2.groups.io On Behalf Of Yao, Jiewen > Sent: Monday, July 06, 2020 11:03 AM > To: devel@edk2.groups.io > Cc: Yao, Jiewen ; Wang, Jian J > Subject: [edk2-devel] [PATCH 01/13] SecurityPkg/Tcg2Pei: Add missing PCRIndex > in FvBlob event. > > From: Jiewen Yao > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2840 > > > > Cc: Jian J Wang > Signed-off-by: Jiewen Yao > --- > SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > index 4852d86906..19b8e4b318 100644 > --- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > +++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c > @@ -633,6 +633,7 @@ MeasureFvImage ( > } > > FvBlob2.BlobBase = FvBase; > > FvBlob2.BlobLength= FvLength; > > +TcgEventHdr.PCRIndex = 0; > > TcgEventHdr.EventType = EV_EFI_PLATFORM_FIRMWARE_BLOB2; > > TcgEventHdr.EventSize = sizeof (FvBlob2); > > EventData = > > -- > 2.26.2.windows.1 > > > -=-=-=-=-=-= > Groups.io Links: You receive all messages sent to this group. > > View/Reply Online (#62062): https://edk2.groups.io/g/devel/message/62062 > Mute This Topic: https://groups.io/mt/75326457/1768734 > Group Owner: devel+ow...@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [jian.j.w...@intel.com] > -=-=-=-=-=-= -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62336): https://edk2.groups.io/g/devel/message/62336 Mute This Topic: https://groups.io/mt/75326457/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 01/14] SecurityPkg/Tcg2Dxe: Add PcdTcgPfpMeasurementRevision in SpecId event.
Reviewed-by: Jian J Wang Regards, Jian > -Original Message- > From: Yao, Jiewen > Sent: Monday, July 06, 2020 11:03 AM > To: devel@edk2.groups.io > Cc: Yao, Jiewen ; Wang, Jian J > Subject: [PATCH 01/14] SecurityPkg/Tcg2Dxe: Add > PcdTcgPfpMeasurementRevision in SpecId event. > > From: Jiewen Yao > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2839 > > > Cc: Jian J Wang > Signed-off-by: Jiewen Yao > --- > SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c | 2 +- > SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf | 1 + > 2 files changed, 2 insertions(+), 1 deletion(-) > > diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c > b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c > index 9a5f987e68..6d17616c1c 100644 > --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c > +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.c > @@ -1589,7 +1589,7 @@ SetupEventLog ( > TcgEfiSpecIdEventStruct->platformClass = PcdGet8 > (PcdTpmPlatformClass); > > TcgEfiSpecIdEventStruct->specVersionMajor = > TCG_EfiSpecIDEventStruct_SPEC_VERSION_MAJOR_TPM2; > > TcgEfiSpecIdEventStruct->specVersionMinor = > TCG_EfiSpecIDEventStruct_SPEC_VERSION_MINOR_TPM2; > > -TcgEfiSpecIdEventStruct->specErrata = > TCG_EfiSpecIDEventStruct_SPEC_ERRATA_TPM2; > > +TcgEfiSpecIdEventStruct->specErrata = > (UINT8)PcdGet32(PcdTcgPfpMeasurementRevision); > > TcgEfiSpecIdEventStruct->uintnSize = sizeof(UINTN)/sizeof(UINT32); > > NumberOfAlgorithms = 0; > > DigestSize = (TCG_EfiSpecIdEventAlgorithmSize *)((UINT8 > *)TcgEfiSpecIdEventStruct + sizeof(*TcgEfiSpecIdEventStruct) + > sizeof(NumberOfAlgorithms)); > > diff --git a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf > b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf > index 576cf80d06..7dc7a2683d 100644 > --- a/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf > +++ b/SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf > @@ -106,6 +106,7 @@ >gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev > ## > CONSUMES > >gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableLaml > ## > PRODUCES > >gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableLasa > ## > PRODUCES > > + gEfiMdeModulePkgTokenSpaceGuid.PcdTcgPfpMeasurementRevision > ## CONSUMES > > > > [Depex] > ># According to PcdTpm2AcpiTableRev definition in SecurityPkg.dec > > -- > 2.26.2.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62335): https://edk2.groups.io/g/devel/message/62335 Mute This Topic: https://groups.io/mt/75326453/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH v5 0/9] Add new feature that evacuate temporary to permanent memory (CVE-2019-11098)
I see it and will do it later. I remind that everyone should pay attention to it as well. Thanks. > -Original Message- > From: Laszlo Ersek > Sent: Friday, July 10, 2020 1:47 PM > To: devel@edk2.groups.io; Jiang, Guomin > Cc: Wang, Jian J ; Wu, Hao A > ; Bi, Dandan ; Gao, Liming > ; De, Debkumar ; Han, > Harry ; West, Catharine ; > Dong, Eric ; Ni, Ray ; Justen, > Jordan L ; Andrew Fish ; Ard > Biesheuvel ; Anthony Perard > ; Julien Grall ; Leif Lindholm > ; Kumar, Rahul1 ; Yao, > Jiewen ; Zhang, Chao B ; > Zhang, Qi1 > Subject: Re: [edk2-devel] [PATCH v5 0/9] Add new feature that evacuate > temporary to permanent memory (CVE-2019-11098) > > Guomin, > > On 07/09/20 03:56, Guomin Jiang wrote: > > The TOCTOU vulnerability allow that the physical present person to replace > the code with the normal BootGuard check and PCR0 value. > > The issue occur when BootGuard measure IBB and access flash code after > NEM disable. > > the reason why we access the flash code is that we have some pointer to > flash. > > To avoid this vulnerability, we need to convert those pointers, the patch > series do this work and make sure that no code will access flash address. > > > > v2: > > Create gEdkiiMigratedFvInfoGuid HOB and add > PcdMigrateTemporaryRamFirmwareVolumes to control whole feature. > > > > v3: > > Remove changes which is not related with the feature and disable the > feature in virtual platform. > > > > v4: > > Disable the feature as default, Copy the Tcg2Pei behavior to TcgPei > > > > v5: > > Initialize local variable Shadow and return EFI_ABORTED when > RepublishSecPpi not installed. > > When you post a new version of a patch set to the list, and there is an > associated BZ ticket, please *always* (not just for this BZ) capture the fact > of > posting the next version in a new BZ comment. Please record the version of > the patch series being posted, and also include a link to the series blurb > (patch 0), in the mailing list archive. > > I did that for you, covering the first four versions (v1 throuogh v4) of the > series in comment 16 on TianoCore#1614: > > https://bugzilla.tianocore.org/show_bug.cgi?id=1614#c16 > > Please do the same (in a new BZ comment) for the current version (v5), and > please repeat the same for any further versions. > > Again this applies to all BZs and all posted patches. > > Thanks > Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#62334): https://edk2.groups.io/g/devel/message/62334 Mute This Topic: https://groups.io/mt/75390172/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
Re: [edk2-devel] [PATCH 8/8] ShellPkg/AcpiView: Refactor table parsers
It is highly unrecommended to initializes the local variable at its declaration
like below. I didn't find the rule in the CCS 2.1 spec. But most of the edk2
code never do like this. There are serval places like below, I just point out
one example.
UINT16 NameSpaceStrLen = *(UINT16 *) Ptr;
For function ParseAcpiRsdp:
ProcessAcpiTable ((VOID *) *XsdtAddress);
Causing a warning and build failure with IA32 arch. I think the correct
statement should be:
ProcessAcpiTable ((VOID *)(UINTN)*XsdtAddress);
Thanks,
Zhichao
> -Original Message-
> From: devel@edk2.groups.io On Behalf Of Tomas Pilar
> (tpilar)
> Sent: Monday, June 29, 2020 11:20 PM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; n...@arm.com; Ni, Ray ; Gao,
> Zhichao
> Subject: [edk2-devel] [PATCH 8/8] ShellPkg/AcpiView: Refactor table parsers
>
> The tests for checking specific constraints and checking
> for buffer overflows have been simplified to use a standard
> set of templates defined in the logging facility.
>
> This regularises some of the error handling and makes
> it easier to write more tests like this in the future.
>
> Cc: Ray Ni
> Cc: Zhichao Gao
> Signed-off-by: Tomas Pilar
> ---
> .../UefiShellAcpiViewCommandLib/AcpiParser.c | 25 ---
> .../UefiShellAcpiViewCommandLib/AcpiParser.h | 18 --
> .../Arm/SbbrValidator.c | 65 +++---
> .../Parsers/Dbg2/Dbg2Parser.c | 118 +++---
> .../Parsers/Fadt/FadtParser.c | 48 ++--
> .../Parsers/Gtdt/GtdtParser.c | 84 ++-
> .../Parsers/Iort/IortParser.c | 207 +++---
> .../Parsers/Madt/MadtParser.c | 99 +++--
> .../Parsers/Mcfg/McfgParser.c | 11 +-
> .../Parsers/Pptt/PpttParser.c | 165 +++---
> .../Parsers/Rsdp/RsdpParser.c | 42 +---
> .../Parsers/Slit/SlitParser.c | 122 ---
> .../Parsers/Spcr/SpcrParser.c | 31 +--
> .../Parsers/Srat/SratParser.c | 188 +---
> .../Parsers/Xsdt/XsdtParser.c | 92 ++--
> 15 files changed, 375 insertions(+), 940 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index 9bbf724dfdfe..58599442d210 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -24,31 +24,6 @@ STATIC CONST ACPI_PARSER AcpiHeaderParser[] = {
>PARSE_ACPI_HEADER ()
> };
>
> -/**
> - This function increments the ACPI table error counter.
> -**/
> -VOID
> -EFIAPI
> -IncrementErrorCount (
> - VOID
> - )
> -{
> - mTableErrorCount++;
> -}
> -
> -/**
> - This function increments the ACPI table warning counter.
> -**/
> -VOID
> -EFIAPI
> -IncrementWarningCount (
> - VOID
> - )
> -{
> - mTableWarningCount++;
> -}
> -
> -
> /**
>This function verifies the ACPI table checksum.
>
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> index bd3cdb774fb5..cdae433fef3b 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.h
> @@ -18,24 +18,6 @@
> /// that allows us to process the log options.
> #define RSDP_TABLE_INFO SIGNATURE_32('R', 'S', 'D', 'P')
>
> -/**
> - This function increments the ACPI table error counter.
> -**/
> -VOID
> -EFIAPI
> -IncrementErrorCount (
> - VOID
> - );
> -
> -/**
> - This function increments the ACPI table warning counter.
> -**/
> -VOID
> -EFIAPI
> -IncrementWarningCount (
> - VOID
> - );
> -
> /**
>This function verifies the ACPI table checksum.
>
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
> index d3284417fa5f..ba80a5ab3b40 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/Arm/SbbrValidator.c
> @@ -18,15 +18,16 @@
> #include
> #include
> #include "AcpiParser.h"
> +#include "AcpiViewLog.h"
> #include "Arm/SbbrValidator.h"
>
> /**
>SBBR specification version strings
> **/
> -STATIC CONST CHAR8* ArmSbbrVersions[ArmSbbrVersionMax] = {
> - "1.0", // ArmSbbrVersion_1_0
> - "1.1", // ArmSbbrVersion_1_1
> - "1.2" // ArmSbbrVersion_1_2
> +STATIC CONST CHAR16* ArmSbbrVersions[ArmSbbrVersionMax] = {
> + L"SBBR-v1.0", // ArmSbbrVersion_1_0
> + L"SBBR-v1.1", // ArmSbbrVersion_1_1
> + L"SBBR-v1.2" // ArmSbbrVersion_1_2
> };
>
> /**
> @@ -96,6 +97,16 @@ STATIC ACPI_TABLE_COUNTER ArmSbbrTableCounts[] = {
>
> {EFI_ACPI_6_3_PROCESSOR_PROPERTIES_TOPOLOGY_TABLE_STRUCTURE_SIGN
> ATURE, 0}
> };
>
> +STATIC_ASSERT (
> + ARRAY_SIZE (ArmSbbr10Mandatory) <= ARRAY_SIZE
Re: [edk2-devel] [PATCH 4/8] ShellPkg/AcpiView: Create a logging facility
See below.
> -Original Message-
> From: Tomas Pilar
> Sent: Monday, June 29, 2020 11:20 PM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; n...@arm.com; Ni, Ray ; Gao,
> Zhichao
> Subject: [PATCH 4/8] ShellPkg/AcpiView: Create a logging facility
>
> Extract error and warning logging into separate methods. Fold highlighting and
> other output properties into the logging methods.
>
> Cc: Ray Ni
> Cc: Zhichao Gao
> Signed-off-by: Tomas Pilar
> ---
> .../UefiShellAcpiViewCommandLib/AcpiParser.c | 5 +-
> .../UefiShellAcpiViewCommandLib/AcpiViewLog.c | 230
> + .../UefiShellAcpiViewCommandLib/AcpiViewLog.h | 233
> ++
> .../UefiShellAcpiViewCommandLib.inf | 2 +
> 4 files changed, 466 insertions(+), 4 deletions(-) create mode 100644
> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c
> create mode 100644
> ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.h
>
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index 7017fa93efae..b88594cf3865 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -11,13 +11,10 @@
> #include "AcpiParser.h"
> #include "AcpiView.h"
> #include "AcpiViewConfig.h"
> +#include "AcpiViewLog.h"
>
> STATIC UINT32 gIndent;
>
> -// Publicly accessible error and warning counters.
> -UINT32 mTableErrorCount;
> -UINT32 mTableWarningCount;
> -
> STATIC ACPI_DESCRIPTION_HEADER_INFO AcpiHdrInfo;
>
> /**
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c
> new file mode 100644
> index ..9b9aaa855fdc
> --- /dev/null
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiViewLog.c
> @@ -0,0 +1,230 @@
> +/** @file
> + 'acpiview' logging and output facility
> +
> + Copyright (c) 2020, ARM Limited. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent **/
> +
> +#include "AcpiViewLog.h"
> +#include "AcpiViewConfig.h"
> +#include "AcpiParser.h"
> +#include
> +
> +static CHAR16 mOutputBuffer [MAX_OUTPUT_SIZE] = { 0 };
> +
> +// String descriptions of error types
> +static const CHAR16* mErrorTypeDesc [ACPI_ERROR_MAX] = {
> + L"Not an error",///< ACPI_ERROR_NONE
> + L"Generic", ///< ACPI_ERROR_GENERIC
> + L"Checksum",///< ACPI_ERROR_CSUM
> + L"Parsing", ///< ACPI_ERROR_PARSE
> + L"Length", ///< ACPI_ERROR_LENGTH
> + L"Value", ///< ACPI_ERROR_VALUE
> + L"Cross-check", ///< ACPI_ERROR_CROSS
> +};
> +
> +// Publicly accessible error and warning counters.
> +UINT32 mTableErrorCount;
> +UINT32 mTableWarningCount;
> +
> +/**
> + Change the attributes of the standard output console
> + to change the colour of the text according to the given
> + severity of a log message.
> +
> + @param[in] Severity The severity of the log message that is being
> + annotated with changed colour text.
> + @param[in] OriginalAttribute The current attributes of ConOut that will be
> modified.
> +**/
> +static
> +VOID
> +EFIAPI
> +ApplyColor (
> + IN ACPI_LOG_SEVERITY Severity,
> + IN UINTN OriginalAttribute
> + )
> +{
> + if (!mConfig.ColourHighlighting) {
> +return;
> + }
> +
> + // Strip the foreground colour
> + UINTN NewAttribute = OriginalAttribute & 0xF0;
> +
> + // Add specific foreground colour based on severity switch
> + (Severity) { case ACPI_DEBUG:
> +NewAttribute |= EFI_DARKGRAY;
> +break;
> + case ACPI_HIGHLIGHT:
> +NewAttribute |= EFI_LIGHTBLUE;
> +break;
> + case ACPI_GOOD:
> +NewAttribute |= EFI_GREEN;
> +break;
> + case ACPI_ITEM:
> + case ACPI_WARN:
> +NewAttribute |= EFI_YELLOW;
> +break;
> + case ACPI_BAD:
> + case ACPI_ERROR:
> + case ACPI_FATAL:
> +NewAttribute |= EFI_RED;
> +break;
> + case ACPI_INFO:
> + default:
> +NewAttribute |= OriginalAttribute;
> +break;
> + }
> +
> + gST->ConOut->SetAttribute (gST->ConOut, NewAttribute); }
> +
> +/**
> + Restore ConOut text attributes.
> +
> + @param[in] OriginalAttribute The attribute set that will be restored.
> +**/
> +static
> +VOID
> +EFIAPI
> +RestoreColor(
> + IN UINTN OriginalAttribute
> + )
> +{
> + gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute); }
> +
> +/**
> + Formats and prints an ascii string to screen.
> +
> + @param[in] Format String that will be formatted and printed.
> + @param[in] Marker The marker for variable parameters to be formatted.
> +**/
> +static
> +VOID
> +EFIAPI
> +AcpiViewVSOutput (
> + IN const CHAR16 *Format,
> + IN VA_LIST Marker
> + )
> +{
> + UnicodeVSPrint (mOutputBuffer, sizeof(mOutputBuffer), Format,
> +Marker);
> + gST->ConOut->OutputString(gST->ConOut, mOutputBuffer); }
> +
> +/**
> + Formats
Re: [edk2-devel] [PATCH 1/8] ShellPkg/AcpiView: Extract configuration struct
> -Original Message-
> From: Tomas Pilar
> Sent: Monday, June 29, 2020 11:20 PM
> To: devel@edk2.groups.io
> Cc: sami.muja...@arm.com; n...@arm.com; Ni, Ray ; Gao,
> Zhichao
> Subject: [PATCH 1/8] ShellPkg/AcpiView: Extract configuration struct
>
> Remove accessor method bloat by creating a configuration struct that is linked
> using an extern symbol in the config header file. Directly reference the
> config
> struct for all read and write accesses in the entire module.
>
> Rationalise the remaining methods in the config header and source.
>
> Cc: Ray Ni
> Cc: Zhichao Gao
> Signed-off-by: Tomas Pilar
> ---
> .../UefiShellAcpiViewCommandLib/AcpiParser.c | 18 +-
> .../AcpiTableParser.c | 4 +-
> .../UefiShellAcpiViewCommandLib/AcpiView.c| 67 +++
> .../AcpiViewConfig.c | 180 ++
> .../AcpiViewConfig.h | 138 ++
> .../UefiShellAcpiViewCommandLib.c | 18 +-
> 6 files changed, 71 insertions(+), 354 deletions(-)
>
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> index 02f6d771c7e1..3a029b01cc20 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c
> @@ -137,7 +137,7 @@ VerifyChecksum (
>if (Log) {
> OriginalAttribute = gST->ConOut->Mode->Attribute;
> if (Checksum == 0) {
> - if (GetColourHighlighting ()) {
> + if (mConfig.ColourHighlighting) {
> gST->ConOut->SetAttribute (
> gST->ConOut,
> EFI_TEXT_ATTR (EFI_GREEN, @@ -147,7 +147,7 @@
> VerifyChecksum (
>Print (L"Table Checksum : OK\n\n");
> } else {
>IncrementErrorCount ();
> - if (GetColourHighlighting ()) {
> + if (mConfig.ColourHighlighting) {
> gST->ConOut->SetAttribute (
> gST->ConOut,
> EFI_TEXT_ATTR (EFI_RED, @@ -156,7 +156,7 @@
> VerifyChecksum (
>}
>Print (L"Table Checksum : FAILED (0x%X)\n\n", Checksum);
> }
> -if (GetColourHighlighting ()) {
> +if (mConfig.ColourHighlighting) {
>gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
> }
>}
> @@ -507,7 +507,6 @@ ParseAcpi (
> {
>UINT32 Index;
>UINT32 Offset;
> - BOOLEAN HighLight;
>UINTN OriginalAttribute;
>
>//
> @@ -520,9 +519,8 @@ ParseAcpi (
>gIndent += Indent;
>
>if (Trace && (AsciiName != NULL)){
> -HighLight = GetColourHighlighting ();
>
> -if (HighLight) {
> +if (mConfig.ColourHighlighting) {
>OriginalAttribute = gST->ConOut->Mode->Attribute;
>gST->ConOut->SetAttribute (
> gST->ConOut,
> @@ -537,7 +535,7 @@ ParseAcpi (
>(OUTPUT_FIELD_COLUMN_WIDTH - gIndent),
>AsciiName
>);
> -if (HighLight) {
> +if (mConfig.ColourHighlighting) {
>gST->ConOut->SetAttribute (gST->ConOut, OriginalAttribute);
> }
>}
> @@ -555,8 +553,7 @@ ParseAcpi (
>continue;
> }
>
> -if (GetConsistencyChecking () &&
> -(Offset != Parser[Index].Offset)) {
> +if (mConfig.ConsistencyCheck && (Offset != Parser[Index].Offset)) {
>IncrementErrorCount ();
>Print (
> L"\nERROR: %a: Offset Mismatch for %s\n"
> @@ -599,8 +596,7 @@ ParseAcpi (
>
> // Validating only makes sense if we are tracing
> // the parsed table entries, to report by table name.
> -if (GetConsistencyChecking () &&
> -(Parser[Index].FieldValidator != NULL)) {
> +if (mConfig.ConsistencyCheck && (Parser[Index].FieldValidator
> + != NULL)) {
>Parser[Index].FieldValidator (Ptr, Parser[Index].Context);
> }
>}
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> index 4b618f131eac..526cb8cb7cad 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiTableParser.c
> @@ -222,13 +222,13 @@ ProcessAcpiTable (
>return;
> }
>
> -if (GetConsistencyChecking ()) {
> +if (mConfig.ConsistencyCheck) {
>VerifyChecksum (TRUE, Ptr, *AcpiTableLength);
> }
>}
>
> #if defined(MDE_CPU_ARM) || defined (MDE_CPU_AARCH64)
> - if (GetMandatoryTableValidate ()) {
> + if (mConfig.MandatoryTableValidate) {
> ArmSbbrIncrementTableCount (*AcpiTableSignature);
>}
> #endif
> diff --git a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> index 9a5b013fb234..d2c14d5b8f5a 100644
> --- a/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> +++ b/ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiView.c
> @@ -48,15 +48,14 @@
