Except, 8bit transfer encoding.
Tested-by: levi.yun <yeoreum....@arm.com>
Reviewed-by: levi.yun <yeoreum....@arm.com>
On 19/01/2024 10:39, Ard Biesheuvel via groups.io wrote:
On Fri, 19 Jan 2024 at 05:58, Nhi Pham via groups.io
<nhi=os.amperecomputing....@groups.io> wrote:
From: Laszlo Ersek <ler...@redhat.com>
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 <n...@os.amperecomputing.com>
Thanks for the patch. This looks good to me in principle, only the
patch got mangled by your MTA
Please resend using 8bit transfer encoding. (You can use
BaseTools/Scripts/SetupGit.py to configure Git for you)
---
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 440fe3e45238..2bcb07d34666 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;
--
2.25.1
IMPORTANT NOTICE: The contents of this email and any attachments are
confidential and may also be privileged. If you are not the intended recipient,
please notify the sender immediately and do not disclose the contents to any
other person, use it for any purpose, or store or copy the information in any
medium. Thank you.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114044): https://edk2.groups.io/g/devel/message/114044
Mute This Topic: https://groups.io/mt/103824815/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
