summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLaszlo Ersek <lersek@redhat.com>2024-01-22 09:31:41 +0700
committermergify[bot] <37929162+mergify[bot]@users.noreply.github.com>2024-01-23 18:26:25 +0000
commit2ddae5df31789853040f4c5261bb85e2f010c4a7 (patch)
tree7e222fcb44cbdbf8e925ac87e4fba4499b92adc3
parentd97f3a1d80fc4880da9726d9a5d7504d3c31da70 (diff)
downloadedk2-2ddae5df31789853040f4c5261bb85e2f010c4a7.tar.gz
edk2-2ddae5df31789853040f4c5261bb85e2f010c4a7.tar.bz2
edk2-2ddae5df31789853040f4c5261bb85e2f010c4a7.zip
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 <nhi@os.amperecomputing.com> Tested-by: levi.yun <yeoreum.yun@arm.com> Reviewed-by: levi.yun <yeoreum.yun@arm.com> Reviewed-by: Ray Ni <ray.ni@intel.com>
-rw-r--r--StandaloneMmPkg/Core/Dependency.c37
1 files 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
@@ -14,16 +14,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
///
#define DEPEX_STACK_SIZE_INCREMENT 0x1000
@@ -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;