tyler.durden
/
audk
mirror of https://github.com/acidanthera/audk.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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>
This commit is contained in:
Laszlo Ersek 2024-01-22 09:31:41 +07:00 committed by mergify[bot]
parent d97f3a1d80
commit 2ddae5df31
1 changed files with 7 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

37
StandaloneMmPkg/Core/Dependency.c
View File

@ -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;