From d97f3a1d80fc4880da9726d9a5d7504d3c31da70 Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Mon, 22 Jan 2024 14:41:15 -0800 Subject: [PATCH 1/5] .pytool/Plugin: UncrustifyCheck: use stat instead of os.stat The UncrustifyCheck plugin passes os.stat.S_IWRITE to os.chmod, when attempting to change file permissions. os.stat.S_IWRITE does not exist as os.stat is a function. The correct value is stat.S_IWRITE. Signed-off-by: Joey Vagedes Cc: Liming Gao Cc: Michael D Kinney Cc: Sean Brogan Reviewed-by: Michael D Kinney --- .pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py b/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py index 9aeef5a5a3..73dc03c0dc 100644 --- a/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py +++ b/.pytool/Plugin/UncrustifyCheck/UncrustifyCheck.py @@ -12,6 +12,7 @@ import os import pathlib import shutil +import stat import timeit from edk2toolext.environment import version_aggregator from edk2toolext.environment.plugin_manager import PluginManager @@ -628,7 +629,7 @@ def _remove_readonly(func, path, _): """ Private function to attempt to change permissions on file/folder being deleted. """ - os.chmod(path, os.stat.S_IWRITE) + os.chmod(path, stat.S_IWRITE) func(path) for _ in range(3): # retry up to 3 times From 2ddae5df31789853040f4c5261bb85e2f010c4a7 Mon Sep 17 00:00:00 2001 From: Laszlo Ersek Date: Mon, 22 Jan 2024 09:31:41 +0700 Subject: [PATCH 2/5] StandaloneMmPkg/Core: Remove optimization for depex evaluation The current dependency evaluator violates the memory access permission when patching depex grammar directly in the read-only depex memory area. Laszlo pointed out the optimization issue in the thread (1) "Memory Attribute for depex section" and provided suggested patch to remove the perf optimization. In my testing, removing the optimization does not make significant perf reduction. That makes sense that StandaloneMM dispatcher only searches in MM protocol database and does not depend on UEFI/DXE protocol database. Also, we don't have many protocols in StandaloneMM like UEFI/DXE. From Laszlo, "The patch removes the EFI_DEP_REPLACE_TRUE handling altogether, plus it CONST-ifies the Iterator pointer (which points into the DEPEX section), so that the compiler catch any possible accesses at *build time* that would write to the write-protected DEPEX memory area." (1) https://edk2.groups.io/g/devel/message/113531 Signed-off-by: Nhi Pham Tested-by: levi.yun Reviewed-by: levi.yun Reviewed-by: Ray Ni --- StandaloneMmPkg/Core/Dependency.c | 37 ++++++------------------------- 1 file changed, 7 insertions(+), 30 deletions(-) diff --git a/StandaloneMmPkg/Core/Dependency.c b/StandaloneMmPkg/Core/Dependency.c index 440fe3e452..2bcb07d346 100644 --- a/StandaloneMmPkg/Core/Dependency.c +++ b/StandaloneMmPkg/Core/Dependency.c @@ -13,16 +13,6 @@ #include "StandaloneMmCore.h" -/// -/// EFI_DEP_REPLACE_TRUE - Used to dynamically patch the dependency expression -/// to save time. A EFI_DEP_PUSH is evaluated one an -/// replaced with EFI_DEP_REPLACE_TRUE. If PI spec's Vol 2 -/// Driver Execution Environment Core Interface use 0xff -/// as new DEPEX opcode. EFI_DEP_REPLACE_TRUE should be -/// defined to a new value that is not conflicting with PI spec. -/// -#define EFI_DEP_REPLACE_TRUE 0xff - /// /// Define the initial size of the dependency expression evaluation stack /// @@ -170,12 +160,12 @@ MmIsSchedulable ( IN EFI_MM_DRIVER_ENTRY *DriverEntry ) { - EFI_STATUS Status; - UINT8 *Iterator; - BOOLEAN Operator; - BOOLEAN Operator2; - EFI_GUID DriverGuid; - VOID *Interface; + EFI_STATUS Status; + CONST UINT8 *Iterator; + BOOLEAN Operator; + BOOLEAN Operator2; + EFI_GUID DriverGuid; + VOID *Interface; Operator = FALSE; Operator2 = FALSE; @@ -253,8 +243,7 @@ MmIsSchedulable ( Status = PushBool (FALSE); } else { DEBUG ((DEBUG_DISPATCH, " PUSH GUID(%g) = TRUE\n", &DriverGuid)); - *Iterator = EFI_DEP_REPLACE_TRUE; - Status = PushBool (TRUE); + Status = PushBool (TRUE); } if (EFI_ERROR (Status)) { @@ -356,18 +345,6 @@ MmIsSchedulable ( DEBUG ((DEBUG_DISPATCH, " RESULT = %a\n", Operator ? "TRUE" : "FALSE")); return Operator; - case EFI_DEP_REPLACE_TRUE: - CopyMem (&DriverGuid, Iterator + 1, sizeof (EFI_GUID)); - DEBUG ((DEBUG_DISPATCH, " PUSH GUID(%g) = TRUE\n", &DriverGuid)); - Status = PushBool (TRUE); - if (EFI_ERROR (Status)) { - DEBUG ((DEBUG_DISPATCH, " RESULT = FALSE (Unexpected error)\n")); - return FALSE; - } - - Iterator += sizeof (EFI_GUID); - break; - default: DEBUG ((DEBUG_DISPATCH, " RESULT = FALSE (Unknown opcode)\n")); goto Done; From 417ebe6d1d6052b6cf023332da07558363d7fd08 Mon Sep 17 00:00:00 2001 From: Suqiang Ren Date: Mon, 22 Jan 2024 23:02:36 -0800 Subject: [PATCH 3/5] MdePkg/Include/Guid: Update the definition of FileName in EFI_FILE_INFO Add the description of EFI_FILE_INFO FileName[1] field to align with UEFI spec 2.10 Section 13.5.16. Cc: Michael D Kinney Cc: Liming Gao Cc: Zhiguang Liu Signed-off-by: Suqiang Ren Reviewed-by: Michael D Kinney --- MdePkg/Include/Guid/FileInfo.h | 1 + 1 file changed, 1 insertion(+) diff --git a/MdePkg/Include/Guid/FileInfo.h b/MdePkg/Include/Guid/FileInfo.h index 2b7edf36aa..71bb289e12 100644 --- a/MdePkg/Include/Guid/FileInfo.h +++ b/MdePkg/Include/Guid/FileInfo.h @@ -47,6 +47,7 @@ typedef struct { UINT64 Attribute; /// /// The Null-terminated name of the file. + /// For a root directory, the name is an empty string. /// CHAR16 FileName[1]; } EFI_FILE_INFO; From 7f72c2829fa29d2b4451c9a60e904df4c6a5df6c Mon Sep 17 00:00:00 2001 From: "devel@edk2.groups.io" Date: Tue, 23 Jan 2024 03:36:49 -0800 Subject: [PATCH 4/5] MdePkg/Library/BaseCpuLibNull: Add StandardSignatureIsAuthenticAMD() CpuLib.h exposes StandardSignatureIsAuthenticAMD() API and we require stub function in its BaseCpuLibNull library instance to avoid potential link issue. Cc: Michael D Kinney Cc: Liming Gao Cc: Zhiguang Liu Signed-off-by: Qing Huang Reviewed-by: Michael D Kinney --- MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c index 3ba7a35096..3542cf6921 100644 --- a/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c +++ b/MdePkg/Library/BaseCpuLibNull/BaseCpuLibNull.c @@ -35,3 +35,18 @@ CpuFlushTlb ( ) { } + +/** + Determine if the standard CPU signature is "AuthenticAMD". + + @retval TRUE The CPU signature matches. + @retval FALSE The CPU signature does not match. +**/ +BOOLEAN +EFIAPI +StandardSignatureIsAuthenticAMD ( + VOID + ) +{ + return FALSE; +} From 117b18420619e5b7065d678401063a6c24b2f52e Mon Sep 17 00:00:00 2001 From: Jeff Brasen Date: Mon, 20 Nov 2023 18:25:10 +0000 Subject: [PATCH 5/5] MdePkg/BaseFdtLib: Rename standard functions Rename the standard functions in the LibFdtSupport to remove conflicts with other libraries that define them. Jira TEGRAUEFI-3105 Signed-off-by: Jeff Brasen Change-Id: Ib0683a29f9aeb7e9c7fa179e9d495c3e979f1261 --- MdePkg/Library/BaseFdtLib/LibFdtSupport.h | 7 +++++-- MdePkg/Library/BaseFdtLib/LibFdtWrapper.c | 25 ++--------------------- 2 files changed, 7 insertions(+), 25 deletions(-) diff --git a/MdePkg/Library/BaseFdtLib/LibFdtSupport.h b/MdePkg/Library/BaseFdtLib/LibFdtSupport.h index 393019324b..8a26fbfc32 100644 --- a/MdePkg/Library/BaseFdtLib/LibFdtSupport.h +++ b/MdePkg/Library/BaseFdtLib/LibFdtSupport.h @@ -63,13 +63,13 @@ strchr ( ); char * -strrchr ( +fdt_strrchr ( const char *, int ); unsigned long -strtoul ( +fdt_strtoul ( const char *, char **, int @@ -93,7 +93,10 @@ strcpy ( #define strnlen(str, count) (size_t)(AsciiStrnLenS(str, count)) #define strncpy(strDest, strSource, count) AsciiStrnCpyS(strDest, MAX_STRING_SIZE, strSource, (UINTN)count) #define strcat(strDest, strSource) AsciiStrCatS(strDest, MAX_STRING_SIZE, strSource) +#define strchr(str, ch) ScanMem8(str, AsciiStrSize (str), (UINT8)ch) #define strcmp(string1, string2, count) (int)(AsciiStrCmp(string1, string2)) #define strncmp(string1, string2, count) (int)(AsciiStrnCmp(string1, string2, (UINTN)(count))) +#define strrchr(str, ch) fdt_strrchr(str, ch) +#define strtoul(ptr, end_ptr, base) fdt_strtoul(ptr, end_ptr, base) #endif /* FDT_LIB_SUPPORT_H_ */ diff --git a/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c b/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c index ef6452914f..1a4cd573fd 100644 --- a/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c +++ b/MdePkg/Library/BaseFdtLib/LibFdtWrapper.c @@ -18,28 +18,7 @@ // so the code gets a bit clunky to handle that case specifically. char * -strchr ( - const char *Str, - int Char - ) -{ - char *S; - - S = (char *)Str; - - for ( ; ; S++) { - if (*S == Char) { - return S; - } - - if (*S == '\0') { - return NULL; - } - } -} - -char * -strrchr ( +fdt_strrchr ( const char *Str, int Char ) @@ -71,7 +50,7 @@ __isspace ( } unsigned long -strtoul ( +fdt_strtoul ( const char *Nptr, char **EndPtr, int Base