* [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements
@ 2019-03-05 13:32 Ard Biesheuvel
2019-03-05 13:32 ` [PATCH 01/10] StandaloneMmPkg: drop redundant definition of gEfiMmConfigurationProtocolGuid Ard Biesheuvel
` (10 more replies)
0 siblings, 11 replies; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 13:32 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Achin Gupta, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja
This series is a further cleanup of the StandaloneMmPkg infrastructure
used to implement UEFI secure boot on ARM systems.
The first 5 patches are simple cleanups.
Patch #6 adds support for dispatching a compressed firmware volume in the
standalone MM context, so that all drivers except the core can be delivered
in an encapsulated compressed FV, which saves quite some space.
Patch #7 modifies the driver dispatch logic in the MM context so that the
dispatcher continues until all drivers are dispatched, rather than waiting
for a nudge from the non-secure side once the CPU driver has been loaded.
Patch #8 removes support for the FV dispatch MM call.
Patch #9 removes support for legacy boot handling.
Patch #10 implements relaying architected PI events from DXE into MM by
the MM communicate driver.
Cc: Achin Gupta <achin.gupta@arm.com>
Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
Ard Biesheuvel (10):
StandaloneMmPkg: drop redundant definition of
gEfiMmConfigurationProtocolGuid
StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable
StandaloneMmPkg: switch to NULL DebugLib resolution
StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver
StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit
SerialPortLib call
StandaloneMmPkg/Core: permit encapsulated firmware volumes
StandaloneMmPkg/Core: dispatch all drivers at init time
StandaloneMmPkg/Core: drop support for dispatching FVs into MM
StandaloneMmPkg/Core: remove legacy boot support
ArmPkg/MmCommunicationDxe: signal architected PI events into MM
context
StandaloneMmPkg/StandaloneMmPkg.dec | 6 -
StandaloneMmPkg/StandaloneMmPkg.dsc | 14 +-
ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5 +
StandaloneMmPkg/Core/StandaloneMmCore.inf | 1 +
StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf | 41 ------
StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf | 3 -
StandaloneMmPkg/Core/StandaloneMmCore.h | 44 -------
ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47 ++++++-
StandaloneMmPkg/Core/Dispatcher.c | 138 --------------------
StandaloneMmPkg/Core/FwVol.c | 99 ++++++++++++--
StandaloneMmPkg/Core/StandaloneMmCore.c | 126 +++++-------------
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c | 3 -
StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.c | 99 --------------
13 files changed, 175 insertions(+), 451 deletions(-)
delete mode 100644 StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
delete mode 100644 StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.c
--
2.20.1
^ permalink raw reply [flat|nested] 43+ messages in thread
* [PATCH 01/10] StandaloneMmPkg: drop redundant definition of gEfiMmConfigurationProtocolGuid
2019-03-05 13:32 [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
@ 2019-03-05 13:32 ` Ard Biesheuvel
2019-03-05 13:53 ` Yao, Jiewen
2019-03-05 13:32 ` [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable Ard Biesheuvel
` (9 subsequent siblings)
10 siblings, 1 reply; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 13:32 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Achin Gupta, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja
gEfiMmConfigurationProtocolGuid is already defined in MdePkg, so drop
the duplicate definition from StandaloneMmPkg.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
StandaloneMmPkg/StandaloneMmPkg.dec | 3 ---
1 file changed, 3 deletions(-)
diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec b/StandaloneMmPkg/StandaloneMmPkg.dec
index 34108376233d..0b5fbf9e1767 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dec
+++ b/StandaloneMmPkg/StandaloneMmPkg.dec
@@ -42,6 +42,3 @@ [Guids]
[PcdsFeatureFlag]
gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|FALSE|BOOLEAN|0x00000001
-[Protocols]
- gEfiMmConfigurationProtocolGuid = { 0xc109319, 0xc149, 0x450e, { 0xa3, 0xe3, 0xb9, 0xba, 0xdd, 0x9d, 0xc3, 0xa4 }}
-
--
2.20.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable
2019-03-05 13:32 [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
2019-03-05 13:32 ` [PATCH 01/10] StandaloneMmPkg: drop redundant definition of gEfiMmConfigurationProtocolGuid Ard Biesheuvel
@ 2019-03-05 13:32 ` Ard Biesheuvel
2019-03-05 13:55 ` Yao, Jiewen
2019-03-06 15:16 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution Ard Biesheuvel
` (8 subsequent siblings)
10 siblings, 2 replies; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 13:32 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Achin Gupta, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja
The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the
first place since the value is implied by the context (it is never
valid to set it to FALSE for standalone MM or TRUE for traditional
MM). So drop it.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
StandaloneMmPkg/StandaloneMmPkg.dec | 3 ---
StandaloneMmPkg/StandaloneMmPkg.dsc | 3 ---
StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf | 3 ---
3 files changed, 9 deletions(-)
diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec b/StandaloneMmPkg/StandaloneMmPkg.dec
index 0b5fbf9e1767..2d178c5e20a6 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dec
+++ b/StandaloneMmPkg/StandaloneMmPkg.dec
@@ -39,6 +39,3 @@ [Guids]
gEfiStandaloneMmNonSecureBufferGuid = { 0xf00497e3, 0xbfa2, 0x41a1, { 0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }}
gEfiArmTfCpuDriverEpDescriptorGuid = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}
-[PcdsFeatureFlag]
- gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|FALSE|BOOLEAN|0x00000001
-
diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc b/StandaloneMmPkg/StandaloneMmPkg.dsc
index e8d71ad56f52..f279d9e7e5c7 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dsc
+++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
@@ -78,9 +78,6 @@ [LibraryClasses.AARCH64]
# Pcd Section - list of all EDK II PCD Entries defined by this Platform
#
################################################################################
-[PcdsFeatureFlag]
- gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|TRUE
-
[PcdsFixedAtBuild]
gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x800000CF
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff
diff --git a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
index eef3d7c6e253..181c15b9345a 100644
--- a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
+++ b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
@@ -37,9 +37,6 @@ [Packages]
MdePkg/MdePkg.dec
StandaloneMmPkg/StandaloneMmPkg.dec
-[FeaturePcd]
- gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable
-
[LibraryClasses]
StandaloneMmMmuLib
PcdLib
--
2.20.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution
2019-03-05 13:32 [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
2019-03-05 13:32 ` [PATCH 01/10] StandaloneMmPkg: drop redundant definition of gEfiMmConfigurationProtocolGuid Ard Biesheuvel
2019-03-05 13:32 ` [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable Ard Biesheuvel
@ 2019-03-05 13:32 ` Ard Biesheuvel
2019-03-05 14:22 ` Yao, Jiewen
2019-03-06 15:38 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 04/10] StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver Ard Biesheuvel
` (7 subsequent siblings)
10 siblings, 2 replies; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 13:32 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Achin Gupta, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja
Building StandaloneMmPkg from its .DSC is mainly intended for build
coverage, and so platform specific configuration such as UART addresses
don't belong here.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
StandaloneMmPkg/StandaloneMmPkg.dsc | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc b/StandaloneMmPkg/StandaloneMmPkg.dsc
index f279d9e7e5c7..8def9688fe7d 100644
--- a/StandaloneMmPkg/StandaloneMmPkg.dsc
+++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
@@ -43,7 +43,7 @@ [LibraryClasses]
#
BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
- DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
+ DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
@@ -66,10 +66,6 @@ [LibraryClasses.AARCH64]
ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
- PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
- PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
- # ARM PL011 UART Driver
- SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
@@ -83,11 +79,6 @@ [PcdsFixedAtBuild]
gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff
gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x0f
-[PcdsFixedAtBuild.AARCH64]
- ## PL011 - Serial Terminal
- gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c0b0000
- gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
-
###################################################################################################
#
# Components Section - list of the modules and components that will be processed by compilation
--
2.20.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 04/10] StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver
2019-03-05 13:32 [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
` (2 preceding siblings ...)
2019-03-05 13:32 ` [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution Ard Biesheuvel
@ 2019-03-05 13:32 ` Ard Biesheuvel
2019-03-05 14:22 ` Yao, Jiewen
2019-03-05 13:32 ` [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call Ard Biesheuvel
` (6 subsequent siblings)
10 siblings, 1 reply; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 13:32 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Achin Gupta, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja
StandaloneMmDriverEntryPoint is implemented in MdePkg now, so let's
drop the redundant StandaloneMmPkg version.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf | 41 --------
StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.c | 99 --------------------
2 files changed, 140 deletions(-)
diff --git a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf b/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
deleted file mode 100644
index 4d1896db10ba..000000000000
--- a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
+++ /dev/null
@@ -1,41 +0,0 @@
-## @file
-# Module entry point library for Standalone MM driver.
-#
-# Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
-# Copyright (c) 2016-2018, ARM Ltd. All rights reserved.<BR>
-#
-# This program and the accompanying materials
-# are licensed and made available under the terms and conditions of the BSD License
-# which accompanies this distribution. The full text of the license may be found at
-# http://opensource.org/licenses/bsd-license.php.
-# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-#
-#
-##
-
-[Defines]
- INF_VERSION = 0x0001001A
- BASE_NAME = StandaloneMmDriverEntryPoint
- FILE_GUID = BBC33478-98F8-4B78-B29D-574D681B7E43
- MODULE_TYPE = MM_STANDALONE
- VERSION_STRING = 1.0
- PI_SPECIFICATION_VERSION = 0x00010032
- LIBRARY_CLASS = StandaloneMmDriverEntryPoint|MM_STANDALONE
-
-#
-# The following information is for reference only and not required by the build tools.
-#
-# VALID_ARCHITECTURES = IA32 X64 IPF EBC
-#
-
-[Sources]
- StandaloneMmDriverEntryPoint.c
-
-[Packages]
- MdePkg/MdePkg.dec
-
-[LibraryClasses]
- BaseLib
- DebugLib
-
diff --git a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.c
deleted file mode 100644
index 64bffcfccc8a..000000000000
--- a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.c
+++ /dev/null
@@ -1,99 +0,0 @@
-/** @file
- Entry point to a Standalone MM driver.
-
-Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
-Copyright (c) 2016 - 2018, ARM Ltd. All rights reserved.<BR>
-
-This program and the accompanying materials
-are licensed and made available under the terms and conditions of the BSD License
-which accompanies this distribution. The full text of the license may be found at
-http://opensource.org/licenses/bsd-license.php
-
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-#include <PiMm.h>
-
-#include <Library/BaseLib.h>
-#include <Library/DebugLib.h>
-
-VOID
-EFIAPI
-ProcessLibraryConstructorList (
- IN EFI_HANDLE ImageHandle,
- IN IN EFI_MM_SYSTEM_TABLE *MmSystemTable
- );
-
-EFI_STATUS
-EFIAPI
-ProcessModuleEntryPointList (
- IN EFI_HANDLE ImageHandle,
- IN IN EFI_MM_SYSTEM_TABLE *MmSystemTable
- );
-
-VOID
-EFIAPI
-ProcessLibraryDestructorList (
- IN EFI_HANDLE ImageHandle,
- IN IN EFI_MM_SYSTEM_TABLE *MmSystemTable
- );
-
-/**
- The entry point of PE/COFF Image for a Standalone MM Driver.
-
- This function is the entry point for a Standalone MM Driver.
- This function must call ProcessLibraryConstructorList() and
- ProcessModuleEntryPointList().
- If the return status from ProcessModuleEntryPointList()
- is an error status, then ProcessLibraryDestructorList() must be called.
- The return value from ProcessModuleEntryPointList() is returned.
- If _gDriverUnloadImageCount is greater than zero, then an unload
- handler must be registered for this image
- and the unload handler must invoke ProcessModuleUnloadList().
- If _gUefiDriverRevision is not zero and SystemTable->Hdr.Revision is less
- than _gUefiDriverRevison, then return EFI_INCOMPATIBLE_VERSION.
-
-
- @param ImageHandle The image handle of the Standalone MM Driver.
- @param SystemTable A pointer to the EFI System Table.
-
- @retval EFI_SUCCESS The Standalone MM Driver exited normally.
- @retval EFI_INCOMPATIBLE_VERSION _gUefiDriverRevision is greater than
- SystemTable->Hdr.Revision.
- @retval Other Return value from ProcessModuleEntryPointList().
-
-**/
-EFI_STATUS
-EFIAPI
-_ModuleEntryPoint (
- IN EFI_HANDLE ImageHandle,
- IN IN EFI_MM_SYSTEM_TABLE *MmSystemTable
- )
-{
- EFI_STATUS Status;
-
- //
- // Call constructor for all libraries
- //
- ProcessLibraryConstructorList (ImageHandle, MmSystemTable);
-
- //
- // Call the driver entry point
- //
- Status = ProcessModuleEntryPointList (ImageHandle, MmSystemTable);
-
- //
- // If all of the drivers returned errors, then invoke all of the library destructors
- //
- if (EFI_ERROR (Status)) {
- ProcessLibraryDestructorList (ImageHandle, MmSystemTable);
- }
-
- //
- // Return the cumulative return status code from all of the driver entry points
- //
- return Status;
-}
-
--
2.20.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call
2019-03-05 13:32 [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
` (3 preceding siblings ...)
2019-03-05 13:32 ` [PATCH 04/10] StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver Ard Biesheuvel
@ 2019-03-05 13:32 ` Ard Biesheuvel
2019-03-05 13:52 ` Yao, Jiewen
2019-03-06 16:35 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes Ard Biesheuvel
` (5 subsequent siblings)
10 siblings, 2 replies; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 13:32 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Achin Gupta, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja
Sending DEBUG output to the serial port should only be done via
DebugLib calls, which is in charge of initializing the serial
port when appropriate. So drop the explicit SerialPortInitialize ()
invocation, and rely on normal constructor ordering to get the
serial port into the appropriate state at the right time.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
index 5cca532456fd..c8e11a253d24 100644
--- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
+++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
@@ -232,9 +232,6 @@ _ModuleEntryPoint (
VOID *TeData;
UINTN TeDataSize;
- Status = SerialPortInitialize ();
- ASSERT_EFI_ERROR (Status);
-
// Get Secure Partition Manager Version Information
Status = GetSpmVersion ();
if (EFI_ERROR (Status)) {
--
2.20.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes
2019-03-05 13:32 [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
` (4 preceding siblings ...)
2019-03-05 13:32 ` [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call Ard Biesheuvel
@ 2019-03-05 13:32 ` Ard Biesheuvel
2019-03-05 15:50 ` Yao, Jiewen
2019-03-06 16:56 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time Ard Biesheuvel
` (4 subsequent siblings)
10 siblings, 2 replies; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 13:32 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Achin Gupta, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja
Standalone MM requires 4 KB section alignment for all images, so that
strict permissions can be applied. Unfortunately, this results in a
lot of wasted space, which is usually costly in the secure world
environment that standalone MM is expected to operate in.
So let's permit the standalone MM drivers (but not the core) to be
delivered in a compressed firmware volume.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
StandaloneMmPkg/Core/StandaloneMmCore.inf | 1 +
StandaloneMmPkg/Core/FwVol.c | 99 ++++++++++++++++++--
2 files changed, 91 insertions(+), 9 deletions(-)
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf b/StandaloneMmPkg/Core/StandaloneMmCore.inf
index ff2b8b9cef03..83d31e2d92c5 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
@@ -49,6 +49,7 @@ [LibraryClasses]
BaseMemoryLib
CacheMaintenanceLib
DebugLib
+ ExtractGuidedSectionLib
FvLib
HobLib
MemoryAllocationLib
diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
index 5abf98c24797..d95491f252f9 100644
--- a/StandaloneMmPkg/Core/FwVol.c
+++ b/StandaloneMmPkg/Core/FwVol.c
@@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
#include "StandaloneMmCore.h"
#include <Library/FvLib.h>
+#include <Library/ExtractGuidedSectionLib.h>
//
// List of file types supported by dispatcher
@@ -65,15 +66,25 @@ Returns:
--*/
{
- EFI_STATUS Status;
- EFI_STATUS DepexStatus;
- EFI_FFS_FILE_HEADER *FileHeader;
- EFI_FV_FILETYPE FileType;
- VOID *Pe32Data;
- UINTN Pe32DataSize;
- VOID *Depex;
- UINTN DepexSize;
- UINTN Index;
+ EFI_STATUS Status;
+ EFI_STATUS DepexStatus;
+ EFI_FFS_FILE_HEADER *FileHeader;
+ EFI_FV_FILETYPE FileType;
+ VOID *Pe32Data;
+ UINTN Pe32DataSize;
+ VOID *Depex;
+ UINTN DepexSize;
+ UINTN Index;
+ EFI_COMMON_SECTION_HEADER *Section;
+ VOID *SectionData;
+ UINTN SectionDataSize;
+ UINT32 DstBufferSize;
+ VOID *ScratchBuffer;
+ UINT32 ScratchBufferSize;
+ VOID *DstBuffer;
+ UINT16 SectionAttribute;
+ UINT32 AuthenticationStatus;
+ EFI_FIRMWARE_VOLUME_HEADER *InnerFvHeader;
DEBUG ((DEBUG_INFO, "MmCoreFfsFindMmDriver - 0x%x\n", FwVolHeader));
@@ -83,6 +94,71 @@ Returns:
FvIsBeingProcesssed (FwVolHeader);
+ //
+ // First check for encapsulated compressed firmware volumes
+ //
+ FileHeader = NULL;
+ do {
+ Status = FfsFindNextFile (EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE,
+ FwVolHeader, &FileHeader);
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+ Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED, FileHeader,
+ &SectionData, &SectionDataSize);
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+ Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
+ Status = ExtractGuidedSectionGetInfo (Section, &DstBufferSize,
+ &ScratchBufferSize, &SectionAttribute);
+ if (EFI_ERROR (Status)) {
+ break;
+ }
+
+ //
+ // Allocate scratch buffer
+ //
+ ScratchBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES (ScratchBufferSize));
+ if (ScratchBuffer == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ //
+ // Allocate destination buffer, extra one page for adjustment
+ //
+ DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize));
+ if (DstBuffer == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ //
+ // Call decompress function
+ //
+ Status = ExtractGuidedSectionDecode (Section, &DstBuffer, ScratchBuffer,
+ &AuthenticationStatus);
+ FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
+ if (EFI_ERROR (Status)) {
+ goto FreeDstBuffer;
+ }
+
+ DEBUG ((DEBUG_INFO,
+ "Processing compressed firmware volume (AuthenticationStatus == %x)\n",
+ AuthenticationStatus));
+
+ Status = FindFfsSectionInSections (DstBuffer, DstBufferSize,
+ EFI_SECTION_FIRMWARE_VOLUME_IMAGE, &Section);
+ if (EFI_ERROR (Status)) {
+ goto FreeDstBuffer;
+ }
+
+ InnerFvHeader = (VOID *)(Section + 1);
+ Status = MmCoreFfsFindMmDriver (InnerFvHeader);
+ if (EFI_ERROR (Status)) {
+ goto FreeDstBuffer;
+ }
+ } while (TRUE);
+
for (Index = 0; Index < sizeof (mMmFileTypes) / sizeof (mMmFileTypes[0]); Index++) {
DEBUG ((DEBUG_INFO, "Check MmFileTypes - 0x%x\n", mMmFileTypes[Index]));
FileType = mMmFileTypes[Index];
@@ -100,5 +176,10 @@ Returns:
} while (!EFI_ERROR (Status));
}
+ return EFI_SUCCESS;
+
+FreeDstBuffer:
+ FreePages (DstBuffer, EFI_SIZE_TO_PAGES (DstBufferSize));
+
return Status;
}
--
2.20.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time
2019-03-05 13:32 [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
` (5 preceding siblings ...)
2019-03-05 13:32 ` [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes Ard Biesheuvel
@ 2019-03-05 13:32 ` Ard Biesheuvel
2019-03-05 15:51 ` Yao, Jiewen
2019-03-06 16:56 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM Ard Biesheuvel
` (3 subsequent siblings)
10 siblings, 2 replies; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 13:32 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Achin Gupta, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja
Instead of deferring dispatch of the remaining MM drivers once the
CPU driver has been dispatched, proceed and dispatch all drivers.
This makes sense for standalone MM, since all dispatchable drivers
should be present in the initial firmware volume anyway: dispatch
of additional FVs originating in the non-secure side is not supported.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
StandaloneMmPkg/Core/Dispatcher.c | 92 --------------------
StandaloneMmPkg/Core/StandaloneMmCore.c | 1 -
2 files changed, 93 deletions(-)
diff --git a/StandaloneMmPkg/Core/Dispatcher.c b/StandaloneMmPkg/Core/Dispatcher.c
index 8a2ad5118d92..bede4832cfb7 100644
--- a/StandaloneMmPkg/Core/Dispatcher.c
+++ b/StandaloneMmPkg/Core/Dispatcher.c
@@ -575,7 +575,6 @@ MmDispatcher (
LIST_ENTRY *Link;
EFI_MM_DRIVER_ENTRY *DriverEntry;
BOOLEAN ReadyToRun;
- BOOLEAN PreviousMmEntryPointRegistered;
DEBUG ((DEBUG_INFO, "MmDispatcher\n"));
@@ -639,11 +638,6 @@ MmDispatcher (
DriverEntry->Initialized = TRUE;
RemoveEntryList (&DriverEntry->ScheduledLink);
- //
- // Cache state of MmEntryPointRegistered before calling entry point
- //
- PreviousMmEntryPointRegistered = gMmCorePrivate->MmEntryPointRegistered;
-
//
// For each MM driver, pass NULL as ImageHandle
//
@@ -661,20 +655,6 @@ MmDispatcher (
DEBUG ((DEBUG_INFO, "StartImage Status - %r\n", Status));
MmFreePages(DriverEntry->ImageBuffer, DriverEntry->NumberOfPage);
}
-
- if (!PreviousMmEntryPointRegistered && gMmCorePrivate->MmEntryPointRegistered) {
- //
- // Return immediately if the MM Entry Point was registered by the MM
- // Driver that was just dispatched. The MM IPL will reinvoke the MM
- // Core Dispatcher. This is required so MM Mode may be enabled as soon
- // as all the dependent MM Drivers for MM Mode have been dispatched.
- // Once the MM Entry Point has been registered, then MM Mode will be
- // used.
- //
- gRequestDispatch = TRUE;
- gDispatcherRunning = FALSE;
- return EFI_NOT_READY;
- }
}
//
@@ -903,78 +883,6 @@ MmAddToDriverList (
return EFI_SUCCESS;
}
-/**
- This function is the main entry point for an MM handler dispatch
- or communicate-based callback.
-
- Event notification that is fired every time a FV dispatch protocol is added.
- More than one protocol may have been added when this event is fired, so you
- must loop on MmLocateHandle () to see how many protocols were added and
- do the following to each FV:
- If the Fv has already been processed, skip it. If the Fv has not been
- processed then mark it as being processed, as we are about to process it.
- Read the Fv and add any driver in the Fv to the mDiscoveredList.The
- mDiscoveredList is never free'ed and contains variables that define
- the other states the MM driver transitions to..
- While you are at it read the A Priori file into memory.
- Place drivers in the A Priori list onto the mScheduledQueue.
-
- @param DispatchHandle The unique handle assigned to this handler by SmiHandlerRegister().
- @param Context Points to an optional handler context which was specified when the handler was registered.
- @param CommBuffer A pointer to a collection of data in memory that will
- be conveyed from a non-MM environment into an MM environment.
- @param CommBufferSize The size of the CommBuffer.
-
- @return Status Code
-
-**/
-EFI_STATUS
-EFIAPI
-MmDriverDispatchHandler (
- IN EFI_HANDLE DispatchHandle,
- IN CONST VOID *Context, OPTIONAL
- IN OUT VOID *CommBuffer, OPTIONAL
- IN OUT UINTN *CommBufferSize OPTIONAL
- )
-{
- EFI_STATUS Status;
-
- DEBUG ((DEBUG_INFO, "MmDriverDispatchHandler\n"));
-
- //
- // Execute the MM Dispatcher on any newly discovered FVs and previously
- // discovered MM drivers that have been discovered but not dispatched.
- //
- Status = MmDispatcher ();
-
- //
- // Check to see if CommBuffer and CommBufferSize are valid
- //
- if (CommBuffer != NULL && CommBufferSize != NULL) {
- if (*CommBufferSize > 0) {
- if (Status == EFI_NOT_READY) {
- //
- // If a the MM Core Entry Point was just registered, then set flag to
- // request the MM Dispatcher to be restarted.
- //
- *(UINT8 *)CommBuffer = COMM_BUFFER_MM_DISPATCH_RESTART;
- } else if (!EFI_ERROR (Status)) {
- //
- // Set the flag to show that the MM Dispatcher executed without errors
- //
- *(UINT8 *)CommBuffer = COMM_BUFFER_MM_DISPATCH_SUCCESS;
- } else {
- //
- // Set the flag to show that the MM Dispatcher encountered an error
- //
- *(UINT8 *)CommBuffer = COMM_BUFFER_MM_DISPATCH_ERROR;
- }
- }
- }
-
- return EFI_SUCCESS;
-}
-
/**
This function is the main entry point for an MM handler dispatch
or communicate-based callback.
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c b/StandaloneMmPkg/Core/StandaloneMmCore.c
index 74432320bfc7..ec53b8d8bec4 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.c
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
@@ -100,7 +100,6 @@ BOOLEAN mInLegacyBoot = FALSE;
//
MM_CORE_MMI_HANDLERS mMmCoreMmiHandlers[] = {
{ MmFvDispatchHandler, &gMmFvDispatchGuid, NULL, TRUE },
- { MmDriverDispatchHandler, &gEfiEventDxeDispatchGuid, NULL, TRUE },
{ MmReadyToLockHandler, &gEfiDxeMmReadyToLockProtocolGuid, NULL, TRUE },
{ MmEndOfDxeHandler, &gEfiEndOfDxeEventGroupGuid, NULL, FALSE },
{ MmLegacyBootHandler, &gEfiEventLegacyBootGuid, NULL, FALSE },
--
2.20.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM
2019-03-05 13:32 [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
` (6 preceding siblings ...)
2019-03-05 13:32 ` [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time Ard Biesheuvel
@ 2019-03-05 13:32 ` Ard Biesheuvel
2019-03-05 15:51 ` Yao, Jiewen
2019-03-06 16:58 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support Ard Biesheuvel
` (2 subsequent siblings)
10 siblings, 2 replies; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 13:32 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Achin Gupta, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja
Remove the support that permits calls into the MM context to dispatch
firmware volumes that are not part of the initial standalone MM firmware
volume.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
StandaloneMmPkg/Core/StandaloneMmCore.h | 22 ----------
StandaloneMmPkg/Core/Dispatcher.c | 46 --------------------
StandaloneMmPkg/Core/StandaloneMmCore.c | 1 -
3 files changed, 69 deletions(-)
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h b/StandaloneMmPkg/Core/StandaloneMmCore.h
index 0d20bcaa6be5..74338dc9da0d 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.h
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
@@ -635,28 +635,6 @@ MmDriverDispatchHandler (
@return Status Code
-**/
-EFI_STATUS
-EFIAPI
-MmFvDispatchHandler (
- IN EFI_HANDLE DispatchHandle,
- IN CONST VOID *Context, OPTIONAL
- IN OUT VOID *CommBuffer, OPTIONAL
- IN OUT UINTN *CommBufferSize OPTIONAL
- );
-
-/**
- This function is the main entry point for an MM handler dispatch
- or communicate-based callback.
-
- @param DispatchHandle The unique handle assigned to this handler by MmiHandlerRegister().
- @param Context Points to an optional handler context which was specified when the handler was registered.
- @param CommBuffer A pointer to a collection of data in memory that will
- be conveyed from a non-MM environment into an MM environment.
- @param CommBufferSize The size of the CommBuffer.
-
- @return Status Code
-
**/
EFI_STATUS
EFIAPI
diff --git a/StandaloneMmPkg/Core/Dispatcher.c b/StandaloneMmPkg/Core/Dispatcher.c
index bede4832cfb7..4b2f38f700a0 100644
--- a/StandaloneMmPkg/Core/Dispatcher.c
+++ b/StandaloneMmPkg/Core/Dispatcher.c
@@ -883,52 +883,6 @@ MmAddToDriverList (
return EFI_SUCCESS;
}
-/**
- This function is the main entry point for an MM handler dispatch
- or communicate-based callback.
-
- @param DispatchHandle The unique handle assigned to this handler by SmiHandlerRegister().
- @param Context Points to an optional handler context which was specified when the handler was registered.
- @param CommBuffer A pointer to a collection of data in memory that will
- be conveyed from a non-MM environment into an MM environment.
- @param CommBufferSize The size of the CommBuffer.
-
- @return Status Code
-
-**/
-EFI_STATUS
-EFIAPI
-MmFvDispatchHandler (
- IN EFI_HANDLE DispatchHandle,
- IN CONST VOID *Context, OPTIONAL
- IN OUT VOID *CommBuffer, OPTIONAL
- IN OUT UINTN *CommBufferSize OPTIONAL
- )
-{
- EFI_STATUS Status;
- EFI_MM_COMMUNICATE_FV_DISPATCH_DATA *CommunicationFvDispatchData;
- EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
-
- DEBUG ((DEBUG_INFO, "MmFvDispatchHandler\n"));
-
- CommunicationFvDispatchData = CommBuffer;
-
- DEBUG ((DEBUG_INFO, " Dispatch - 0x%016lx - 0x%016lx\n", CommunicationFvDispatchData->Address,
- CommunicationFvDispatchData->Size));
-
- FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)CommunicationFvDispatchData->Address;
-
- MmCoreFfsFindMmDriver (FwVolHeader);
-
- //
- // Execute the MM Dispatcher on any newly discovered FVs and previously
- // discovered MM drivers that have been discovered but not dispatched.
- //
- Status = MmDispatcher ();
-
- return Status;
-}
-
/**
Traverse the discovered list for any drivers that were discovered but not loaded
because the dependency experessions evaluated to false.
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c b/StandaloneMmPkg/Core/StandaloneMmCore.c
index ec53b8d8bec4..766cdb5c848c 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.c
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
@@ -99,7 +99,6 @@ BOOLEAN mInLegacyBoot = FALSE;
// Table of MMI Handlers that are registered by the MM Core when it is initialized
//
MM_CORE_MMI_HANDLERS mMmCoreMmiHandlers[] = {
- { MmFvDispatchHandler, &gMmFvDispatchGuid, NULL, TRUE },
{ MmReadyToLockHandler, &gEfiDxeMmReadyToLockProtocolGuid, NULL, TRUE },
{ MmEndOfDxeHandler, &gEfiEndOfDxeEventGroupGuid, NULL, FALSE },
{ MmLegacyBootHandler, &gEfiEventLegacyBootGuid, NULL, FALSE },
--
2.20.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support
2019-03-05 13:32 [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
` (7 preceding siblings ...)
2019-03-05 13:32 ` [PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM Ard Biesheuvel
@ 2019-03-05 13:32 ` Ard Biesheuvel
2019-03-05 13:52 ` Yao, Jiewen
2019-03-06 16:59 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context Ard Biesheuvel
2019-03-11 11:54 ` [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
10 siblings, 2 replies; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 13:32 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Achin Gupta, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja
Remove the support for booting 'legacy' (i.e., non-UEFI boot) OSes.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
StandaloneMmPkg/Core/StandaloneMmCore.h | 22 ----
StandaloneMmPkg/Core/StandaloneMmCore.c | 124 ++++++--------------
2 files changed, 33 insertions(+), 113 deletions(-)
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h b/StandaloneMmPkg/Core/StandaloneMmCore.h
index 74338dc9da0d..5d336b3dbbf6 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.h
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
@@ -635,28 +635,6 @@ MmDriverDispatchHandler (
@return Status Code
-**/
-EFI_STATUS
-EFIAPI
-MmLegacyBootHandler (
- IN EFI_HANDLE DispatchHandle,
- IN CONST VOID *Context, OPTIONAL
- IN OUT VOID *CommBuffer, OPTIONAL
- IN OUT UINTN *CommBufferSize OPTIONAL
- );
-
-/**
- This function is the main entry point for an MM handler dispatch
- or communicate-based callback.
-
- @param DispatchHandle The unique handle assigned to this handler by MmiHandlerRegister().
- @param Context Points to an optional handler context which was specified when the handler was registered.
- @param CommBuffer A pointer to a collection of data in memory that will
- be conveyed from a non-MM environment into an MM environment.
- @param CommBufferSize The size of the CommBuffer.
-
- @return Status Code
-
**/
EFI_STATUS
EFIAPI
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c b/StandaloneMmPkg/Core/StandaloneMmCore.c
index 766cdb5c848c..fb6b3055e9c6 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.c
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
@@ -87,21 +87,12 @@ EFI_MM_SYSTEM_TABLE gMmCoreMmst = {
MmiHandlerUnRegister
};
-//
-// Flag to determine if the platform has performed a legacy boot.
-// If this flag is TRUE, then the runtime code and runtime data associated with the
-// MM IPL are converted to free memory, so the MM Core must guarantee that is
-// does not touch of the code/data associated with the MM IPL if this flag is TRUE.
-//
-BOOLEAN mInLegacyBoot = FALSE;
-
//
// Table of MMI Handlers that are registered by the MM Core when it is initialized
//
MM_CORE_MMI_HANDLERS mMmCoreMmiHandlers[] = {
{ MmReadyToLockHandler, &gEfiDxeMmReadyToLockProtocolGuid, NULL, TRUE },
{ MmEndOfDxeHandler, &gEfiEndOfDxeEventGroupGuid, NULL, FALSE },
- { MmLegacyBootHandler, &gEfiEventLegacyBootGuid, NULL, FALSE },
{ MmExitBootServiceHandler,&gEfiEventExitBootServicesGuid, NULL, FALSE },
{ MmReadyToBootHandler, &gEfiEventReadyToBootGuid, NULL, FALSE },
{ NULL, NULL, NULL, FALSE },
@@ -142,47 +133,6 @@ MmEfiNotAvailableYetArg5 (
return EFI_NOT_AVAILABLE_YET;
}
-/**
- Software MMI handler that is called when a Legacy Boot event is signaled. The MM
- Core uses this signal to know that a Legacy Boot has been performed and that
- gMmCorePrivate that is shared between the UEFI and MM execution environments can
- not be accessed from MM anymore since that structure is considered free memory by
- a legacy OS.
-
- @param DispatchHandle The unique handle assigned to this handler by MmiHandlerRegister().
- @param Context Points to an optional handler context which was specified when the handler was registered.
- @param CommBuffer A pointer to a collection of data in memory that will
- be conveyed from a non-MM environment into an MM environment.
- @param CommBufferSize The size of the CommBuffer.
-
- @return Status Code
-
-**/
-EFI_STATUS
-EFIAPI
-MmLegacyBootHandler (
- IN EFI_HANDLE DispatchHandle,
- IN CONST VOID *Context, OPTIONAL
- IN OUT VOID *CommBuffer, OPTIONAL
- IN OUT UINTN *CommBufferSize OPTIONAL
- )
-{
- EFI_HANDLE MmHandle;
- EFI_STATUS Status = EFI_SUCCESS;
-
- if (!mInLegacyBoot) {
- MmHandle = NULL;
- Status = MmInstallProtocolInterface (
- &MmHandle,
- &gEfiEventLegacyBootGuid,
- EFI_NATIVE_INTERFACE,
- NULL
- );
- }
- mInLegacyBoot = TRUE;
- return Status;
-}
-
/**
Software MMI handler that is called when a ExitBoot Service event is signaled.
@@ -396,7 +346,6 @@ MmEntryPoint (
{
EFI_STATUS Status;
EFI_MM_COMMUNICATE_HEADER *CommunicateHeader;
- BOOLEAN InLegacyBoot;
DEBUG ((DEBUG_INFO, "MmEntryPoint ...\n"));
@@ -413,44 +362,42 @@ MmEntryPoint (
//
// If a legacy boot has occured, then make sure gMmCorePrivate is not accessed
//
- InLegacyBoot = mInLegacyBoot;
- if (!InLegacyBoot) {
- //
- // TBD: Mark the InMm flag as TRUE
- //
- gMmCorePrivate->InMm = TRUE;
+ //
+ // TBD: Mark the InMm flag as TRUE
+ //
+ gMmCorePrivate->InMm = TRUE;
+
+ //
+ // Check to see if this is a Synchronous MMI sent through the MM Communication
+ // Protocol or an Asynchronous MMI
+ //
+ if (gMmCorePrivate->CommunicationBuffer != 0) {
//
- // Check to see if this is a Synchronous MMI sent through the MM Communication
- // Protocol or an Asynchronous MMI
+ // Synchronous MMI for MM Core or request from Communicate protocol
//
- if (gMmCorePrivate->CommunicationBuffer != 0) {
+ if (!MmIsBufferOutsideMmValid ((UINTN)gMmCorePrivate->CommunicationBuffer, gMmCorePrivate->BufferSize)) {
+ //
+ // If CommunicationBuffer is not in valid address scope, return EFI_INVALID_PARAMETER
+ //
+ gMmCorePrivate->CommunicationBuffer = 0;
+ gMmCorePrivate->ReturnStatus = EFI_INVALID_PARAMETER;
+ } else {
+ CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER *)(UINTN)gMmCorePrivate->CommunicationBuffer;
+ gMmCorePrivate->BufferSize -= OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
+ Status = MmiManage (
+ &CommunicateHeader->HeaderGuid,
+ NULL,
+ CommunicateHeader->Data,
+ (UINTN *)&gMmCorePrivate->BufferSize
+ );
//
- // Synchronous MMI for MM Core or request from Communicate protocol
+ // Update CommunicationBuffer, BufferSize and ReturnStatus
+ // Communicate service finished, reset the pointer to CommBuffer to NULL
//
- if (!MmIsBufferOutsideMmValid ((UINTN)gMmCorePrivate->CommunicationBuffer, gMmCorePrivate->BufferSize)) {
- //
- // If CommunicationBuffer is not in valid address scope, return EFI_INVALID_PARAMETER
- //
- gMmCorePrivate->CommunicationBuffer = 0;
- gMmCorePrivate->ReturnStatus = EFI_INVALID_PARAMETER;
- } else {
- CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER *)(UINTN)gMmCorePrivate->CommunicationBuffer;
- gMmCorePrivate->BufferSize -= OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
- Status = MmiManage (
- &CommunicateHeader->HeaderGuid,
- NULL,
- CommunicateHeader->Data,
- (UINTN *)&gMmCorePrivate->BufferSize
- );
- //
- // Update CommunicationBuffer, BufferSize and ReturnStatus
- // Communicate service finished, reset the pointer to CommBuffer to NULL
- //
- gMmCorePrivate->BufferSize += OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
- gMmCorePrivate->CommunicationBuffer = 0;
- gMmCorePrivate->ReturnStatus = (Status == EFI_SUCCESS) ? EFI_SUCCESS : EFI_NOT_FOUND;
- }
+ gMmCorePrivate->BufferSize += OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
+ gMmCorePrivate->CommunicationBuffer = 0;
+ gMmCorePrivate->ReturnStatus = (Status == EFI_SUCCESS) ? EFI_SUCCESS : EFI_NOT_FOUND;
}
}
@@ -464,14 +411,9 @@ MmEntryPoint (
//
//
- // If a legacy boot has occured, then make sure gMmCorePrivate is not accessed
+ // Clear the InMm flag as we are going to leave MM
//
- if (!InLegacyBoot) {
- //
- // Clear the InMm flag as we are going to leave MM
- //
- gMmCorePrivate->InMm = FALSE;
- }
+ gMmCorePrivate->InMm = FALSE;
DEBUG ((DEBUG_INFO, "MmEntryPoint Done\n"));
}
--
2.20.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context
2019-03-05 13:32 [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
` (8 preceding siblings ...)
2019-03-05 13:32 ` [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support Ard Biesheuvel
@ 2019-03-05 13:32 ` Ard Biesheuvel
2019-03-05 15:55 ` Yao, Jiewen
2019-03-06 16:58 ` Achin Gupta
2019-03-11 11:54 ` [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
10 siblings, 2 replies; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 13:32 UTC (permalink / raw)
To: edk2-devel
Cc: Ard Biesheuvel, Achin Gupta, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja
PI defines a few architected events that have significance in the MM
context as well as in the non-secure DXE context. So register notify
handlers for these events, and relay them into the standalone MM world.
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5 +++
ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47 +++++++++++++++++++-
2 files changed, 50 insertions(+), 2 deletions(-)
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
index 88beafa39c05..8bf269270f9d 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
@@ -48,6 +48,11 @@ [LibraryClasses]
[Protocols]
gEfiMmCommunicationProtocolGuid ## PRODUCES
+[Guids]
+ gEfiEndOfDxeEventGroupGuid
+ gEfiEventExitBootServicesGuid
+ gEfiEventReadyToBootGuid
+
[Pcd.common]
gArmTokenSpaceGuid.PcdMmBufferBase
gArmTokenSpaceGuid.PcdMmBufferSize
diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
index feb9fa9f4ead..3203cf801a19 100644
--- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
+++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
@@ -265,6 +265,43 @@ GetMmCompatibility ()
return Status;
}
+STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
+ &gEfiEndOfDxeEventGroupGuid,
+ &gEfiEventExitBootServicesGuid,
+ &gEfiEventReadyToBootGuid,
+};
+
+STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)];
+
+/**
+ Event notification that is fired when GUIDed Event Group is signaled.
+
+ @param Event The Event that is being processed, not used.
+ @param Context Event Context, not used.
+
+**/
+STATIC
+VOID
+EFIAPI
+MmGuidedEventNotify (
+ IN EFI_EVENT Event,
+ IN VOID *Context
+ )
+{
+ EFI_MM_COMMUNICATE_HEADER Header;
+ UINTN Size;
+
+ //
+ // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure
+ //
+ CopyGuid (&Header.HeaderGuid, Context);
+ Header.MessageLength = 1;
+ Header.Data[0] = 0;
+
+ Size = sizeof (Header);
+ MmCommunicationCommunicate (&mMmCommunication, &Header, &Size);
+}
+
/**
The Entry Point for MM Communication
@@ -287,6 +324,7 @@ MmCommunicationInitialize (
)
{
EFI_STATUS Status;
+ UINTN Index;
// Check if we can make the MM call
Status = GetMmCompatibility ();
@@ -351,8 +389,13 @@ MmCommunicationInitialize (
NULL,
&mSetVirtualAddressMapEvent
);
- if (Status == EFI_SUCCESS) {
- return Status;
+ ASSERT_EFI_ERROR (Status);
+
+ for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) {
+ Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
+ MmGuidedEventNotify, mGuidedEventGuid[Index],
+ mGuidedEventGuid[Index], &mGuidedEvent[Index]);
+ ASSERT_EFI_ERROR (Status);
}
gBS->UninstallProtocolInterface (
--
2.20.1
^ permalink raw reply related [flat|nested] 43+ messages in thread
* Re: [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support
2019-03-05 13:32 ` [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support Ard Biesheuvel
@ 2019-03-05 13:52 ` Yao, Jiewen
2019-03-06 16:59 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Yao, Jiewen @ 2019-03-05 13:52 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Reviewed-by: Jiewen.yao@intel.com
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [edk2] [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot
> support
>
> Remove the support for booting 'legacy' (i.e., non-UEFI boot) OSes.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/Core/StandaloneMmCore.h | 22 ----
> StandaloneMmPkg/Core/StandaloneMmCore.c | 124 ++++++--------------
> 2 files changed, 33 insertions(+), 113 deletions(-)
>
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h
> b/StandaloneMmPkg/Core/StandaloneMmCore.h
> index 74338dc9da0d..5d336b3dbbf6 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.h
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
> @@ -635,28 +635,6 @@ MmDriverDispatchHandler (
>
> @return Status Code
>
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmLegacyBootHandler (
> - IN EFI_HANDLE DispatchHandle,
> - IN CONST VOID *Context, OPTIONAL
> - IN OUT VOID *CommBuffer, OPTIONAL
> - IN OUT UINTN *CommBufferSize OPTIONAL
> - );
> -
> -/**
> - This function is the main entry point for an MM handler dispatch
> - or communicate-based callback.
> -
> - @param DispatchHandle The unique handle assigned to this handler
> by MmiHandlerRegister().
> - @param Context Points to an optional handler context
> which was specified when the handler was registered.
> - @param CommBuffer A pointer to a collection of data in
> memory that will
> - be conveyed from a non-MM environment
> into an MM environment.
> - @param CommBufferSize The size of the CommBuffer.
> -
> - @return Status Code
> -
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c
> b/StandaloneMmPkg/Core/StandaloneMmCore.c
> index 766cdb5c848c..fb6b3055e9c6 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.c
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
> @@ -87,21 +87,12 @@ EFI_MM_SYSTEM_TABLE gMmCoreMmst = {
> MmiHandlerUnRegister
> };
>
> -//
> -// Flag to determine if the platform has performed a legacy boot.
> -// If this flag is TRUE, then the runtime code and runtime data associated
> with the
> -// MM IPL are converted to free memory, so the MM Core must guarantee
> that is
> -// does not touch of the code/data associated with the MM IPL if this flag is
> TRUE.
> -//
> -BOOLEAN mInLegacyBoot = FALSE;
> -
> //
> // Table of MMI Handlers that are registered by the MM Core when it is
> initialized
> //
> MM_CORE_MMI_HANDLERS mMmCoreMmiHandlers[] = {
> { MmReadyToLockHandler, &gEfiDxeMmReadyToLockProtocolGuid,
> NULL, TRUE },
> { MmEndOfDxeHandler, &gEfiEndOfDxeEventGroupGuid,
> NULL, FALSE },
> - { MmLegacyBootHandler, &gEfiEventLegacyBootGuid,
> NULL, FALSE },
> { MmExitBootServiceHandler,&gEfiEventExitBootServicesGuid,
> NULL, FALSE },
> { MmReadyToBootHandler, &gEfiEventReadyToBootGuid,
> NULL, FALSE },
> { NULL, NULL,
> NULL, FALSE },
> @@ -142,47 +133,6 @@ MmEfiNotAvailableYetArg5 (
> return EFI_NOT_AVAILABLE_YET;
> }
>
> -/**
> - Software MMI handler that is called when a Legacy Boot event is signaled.
> The MM
> - Core uses this signal to know that a Legacy Boot has been performed and
> that
> - gMmCorePrivate that is shared between the UEFI and MM execution
> environments can
> - not be accessed from MM anymore since that structure is considered
> free memory by
> - a legacy OS.
> -
> - @param DispatchHandle The unique handle assigned to this handler
> by MmiHandlerRegister().
> - @param Context Points to an optional handler context
> which was specified when the handler was registered.
> - @param CommBuffer A pointer to a collection of data in
> memory that will
> - be conveyed from a non-MM environment
> into an MM environment.
> - @param CommBufferSize The size of the CommBuffer.
> -
> - @return Status Code
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmLegacyBootHandler (
> - IN EFI_HANDLE DispatchHandle,
> - IN CONST VOID *Context, OPTIONAL
> - IN OUT VOID *CommBuffer, OPTIONAL
> - IN OUT UINTN *CommBufferSize OPTIONAL
> - )
> -{
> - EFI_HANDLE MmHandle;
> - EFI_STATUS Status = EFI_SUCCESS;
> -
> - if (!mInLegacyBoot) {
> - MmHandle = NULL;
> - Status = MmInstallProtocolInterface (
> - &MmHandle,
> - &gEfiEventLegacyBootGuid,
> - EFI_NATIVE_INTERFACE,
> - NULL
> - );
> - }
> - mInLegacyBoot = TRUE;
> - return Status;
> -}
> -
> /**
> Software MMI handler that is called when a ExitBoot Service event is
> signaled.
>
> @@ -396,7 +346,6 @@ MmEntryPoint (
> {
> EFI_STATUS Status;
> EFI_MM_COMMUNICATE_HEADER *CommunicateHeader;
> - BOOLEAN InLegacyBoot;
>
> DEBUG ((DEBUG_INFO, "MmEntryPoint ...\n"));
>
> @@ -413,44 +362,42 @@ MmEntryPoint (
> //
> // If a legacy boot has occured, then make sure gMmCorePrivate is not
> accessed
> //
> - InLegacyBoot = mInLegacyBoot;
> - if (!InLegacyBoot) {
> - //
> - // TBD: Mark the InMm flag as TRUE
> - //
> - gMmCorePrivate->InMm = TRUE;
>
> + //
> + // TBD: Mark the InMm flag as TRUE
> + //
> + gMmCorePrivate->InMm = TRUE;
> +
> + //
> + // Check to see if this is a Synchronous MMI sent through the MM
> Communication
> + // Protocol or an Asynchronous MMI
> + //
> + if (gMmCorePrivate->CommunicationBuffer != 0) {
> //
> - // Check to see if this is a Synchronous MMI sent through the MM
> Communication
> - // Protocol or an Asynchronous MMI
> + // Synchronous MMI for MM Core or request from Communicate
> protocol
> //
> - if (gMmCorePrivate->CommunicationBuffer != 0) {
> + if (!MmIsBufferOutsideMmValid
> ((UINTN)gMmCorePrivate->CommunicationBuffer,
> gMmCorePrivate->BufferSize)) {
> + //
> + // If CommunicationBuffer is not in valid address scope, return
> EFI_INVALID_PARAMETER
> + //
> + gMmCorePrivate->CommunicationBuffer = 0;
> + gMmCorePrivate->ReturnStatus = EFI_INVALID_PARAMETER;
> + } else {
> + CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER
> *)(UINTN)gMmCorePrivate->CommunicationBuffer;
> + gMmCorePrivate->BufferSize -= OFFSET_OF
> (EFI_MM_COMMUNICATE_HEADER, Data);
> + Status = MmiManage (
> + &CommunicateHeader->HeaderGuid,
> + NULL,
> + CommunicateHeader->Data,
> + (UINTN *)&gMmCorePrivate->BufferSize
> + );
> //
> - // Synchronous MMI for MM Core or request from Communicate
> protocol
> + // Update CommunicationBuffer, BufferSize and ReturnStatus
> + // Communicate service finished, reset the pointer to CommBuffer
> to NULL
> //
> - if (!MmIsBufferOutsideMmValid
> ((UINTN)gMmCorePrivate->CommunicationBuffer,
> gMmCorePrivate->BufferSize)) {
> - //
> - // If CommunicationBuffer is not in valid address scope, return
> EFI_INVALID_PARAMETER
> - //
> - gMmCorePrivate->CommunicationBuffer = 0;
> - gMmCorePrivate->ReturnStatus = EFI_INVALID_PARAMETER;
> - } else {
> - CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER
> *)(UINTN)gMmCorePrivate->CommunicationBuffer;
> - gMmCorePrivate->BufferSize -= OFFSET_OF
> (EFI_MM_COMMUNICATE_HEADER, Data);
> - Status = MmiManage (
> - &CommunicateHeader->HeaderGuid,
> - NULL,
> - CommunicateHeader->Data,
> - (UINTN *)&gMmCorePrivate->BufferSize
> - );
> - //
> - // Update CommunicationBuffer, BufferSize and ReturnStatus
> - // Communicate service finished, reset the pointer to
> CommBuffer to NULL
> - //
> - gMmCorePrivate->BufferSize += OFFSET_OF
> (EFI_MM_COMMUNICATE_HEADER, Data);
> - gMmCorePrivate->CommunicationBuffer = 0;
> - gMmCorePrivate->ReturnStatus = (Status == EFI_SUCCESS) ?
> EFI_SUCCESS : EFI_NOT_FOUND;
> - }
> + gMmCorePrivate->BufferSize += OFFSET_OF
> (EFI_MM_COMMUNICATE_HEADER, Data);
> + gMmCorePrivate->CommunicationBuffer = 0;
> + gMmCorePrivate->ReturnStatus = (Status == EFI_SUCCESS) ?
> EFI_SUCCESS : EFI_NOT_FOUND;
> }
> }
>
> @@ -464,14 +411,9 @@ MmEntryPoint (
> //
>
> //
> - // If a legacy boot has occured, then make sure gMmCorePrivate is not
> accessed
> + // Clear the InMm flag as we are going to leave MM
> //
> - if (!InLegacyBoot) {
> - //
> - // Clear the InMm flag as we are going to leave MM
> - //
> - gMmCorePrivate->InMm = FALSE;
> - }
> + gMmCorePrivate->InMm = FALSE;
>
> DEBUG ((DEBUG_INFO, "MmEntryPoint Done\n"));
> }
> --
> 2.20.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call
2019-03-05 13:32 ` [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call Ard Biesheuvel
@ 2019-03-05 13:52 ` Yao, Jiewen
2019-03-06 16:35 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Yao, Jiewen @ 2019-03-05 13:52 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Reviewed-by: Jiewen.yao@intel.com
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Supreeth Venkatesh
> <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> <jagadeesh.ujja@arm.com>
> Subject: [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint:
> drop explicit SerialPortLib call
>
> Sending DEBUG output to the serial port should only be done via
> DebugLib calls, which is in charge of initializing the serial
> port when appropriate. So drop the explicit SerialPortInitialize ()
> invocation, and rely on normal constructor ordering to get the
> serial port into the appropriate state at the right time.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standal
> oneMmCoreEntryPoint.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> index 5cca532456fd..c8e11a253d24 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Stand
> aloneMmCoreEntryPoint.c
> @@ -232,9 +232,6 @@ _ModuleEntryPoint (
> VOID *TeData;
> UINTN TeDataSize;
>
> - Status = SerialPortInitialize ();
> - ASSERT_EFI_ERROR (Status);
> -
> // Get Secure Partition Manager Version Information
> Status = GetSpmVersion ();
> if (EFI_ERROR (Status)) {
> --
> 2.20.1
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 01/10] StandaloneMmPkg: drop redundant definition of gEfiMmConfigurationProtocolGuid
2019-03-05 13:32 ` [PATCH 01/10] StandaloneMmPkg: drop redundant definition of gEfiMmConfigurationProtocolGuid Ard Biesheuvel
@ 2019-03-05 13:53 ` Yao, Jiewen
0 siblings, 0 replies; 43+ messages in thread
From: Yao, Jiewen @ 2019-03-05 13:53 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Reviewed-by: jiewen.yao@intel.com
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [edk2] [PATCH 01/10] StandaloneMmPkg: drop redundant definition
> of gEfiMmConfigurationProtocolGuid
>
> gEfiMmConfigurationProtocolGuid is already defined in MdePkg, so drop
> the duplicate definition from StandaloneMmPkg.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/StandaloneMmPkg.dec | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec
> b/StandaloneMmPkg/StandaloneMmPkg.dec
> index 34108376233d..0b5fbf9e1767 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dec
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dec
> @@ -42,6 +42,3 @@ [Guids]
> [PcdsFeatureFlag]
>
> gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|FALSE|BOOL
> EAN|0x00000001
>
> -[Protocols]
> - gEfiMmConfigurationProtocolGuid = { 0xc109319, 0xc149,
> 0x450e, { 0xa3, 0xe3, 0xb9, 0xba, 0xdd, 0x9d, 0xc3, 0xa4 }}
> -
> --
> 2.20.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable
2019-03-05 13:32 ` [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable Ard Biesheuvel
@ 2019-03-05 13:55 ` Yao, Jiewen
2019-03-06 15:16 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Yao, Jiewen @ 2019-03-05 13:55 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Reviewed-by: jiewen.yao@intel.com
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [edk2] [PATCH 02/10] StandaloneMmPkg: drop unused PCD
> PcdStandaloneMmEnable
>
> The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the
> first place since the value is implied by the context (it is never
> valid to set it to FALSE for standalone MM or TRUE for traditional
> MM). So drop it.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/StandaloneMmPkg.dec
> | 3 ---
> StandaloneMmPkg/StandaloneMmPkg.dsc
> | 3 ---
>
> StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalone
> MmPeCoffExtraActionLib.inf | 3 ---
> 3 files changed, 9 deletions(-)
>
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec
> b/StandaloneMmPkg/StandaloneMmPkg.dec
> index 0b5fbf9e1767..2d178c5e20a6 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dec
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dec
> @@ -39,6 +39,3 @@ [Guids]
> gEfiStandaloneMmNonSecureBufferGuid = { 0xf00497e3, 0xbfa2,
> 0x41a1, { 0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }}
> gEfiArmTfCpuDriverEpDescriptorGuid = { 0x6ecbd5a1, 0xc0f8,
> 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}
>
> -[PcdsFeatureFlag]
> -
> gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|FALSE|BOOL
> EAN|0x00000001
> -
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc
> b/StandaloneMmPkg/StandaloneMmPkg.dsc
> index e8d71ad56f52..f279d9e7e5c7 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dsc
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
> @@ -78,9 +78,6 @@ [LibraryClasses.AARCH64]
> # Pcd Section - list of all EDK II PCD Entries defined by this Platform
> #
>
> ##############################################################
> ##################
> -[PcdsFeatureFlag]
> - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|TRUE
> -
> [PcdsFixedAtBuild]
> gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x800000CF
> gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalo
> neMmPeCoffExtraActionLib.inf
> b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalo
> neMmPeCoffExtraActionLib.inf
> index eef3d7c6e253..181c15b9345a 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalo
> neMmPeCoffExtraActionLib.inf
> +++
> b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/Standalo
> neMmPeCoffExtraActionLib.inf
> @@ -37,9 +37,6 @@ [Packages]
> MdePkg/MdePkg.dec
> StandaloneMmPkg/StandaloneMmPkg.dec
>
> -[FeaturePcd]
> - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable
> -
> [LibraryClasses]
> StandaloneMmMmuLib
> PcdLib
> --
> 2.20.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution
2019-03-05 13:32 ` [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution Ard Biesheuvel
@ 2019-03-05 14:22 ` Yao, Jiewen
2019-03-06 15:38 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Yao, Jiewen @ 2019-03-05 14:22 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Reviewed-by: jiewen.yao@intel.com
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Supreeth Venkatesh
> <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> <jagadeesh.ujja@arm.com>
> Subject: [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib
> resolution
>
> Building StandaloneMmPkg from its .DSC is mainly intended for build
> coverage, and so platform specific configuration such as UART addresses
> don't belong here.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/StandaloneMmPkg.dsc | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc
> b/StandaloneMmPkg/StandaloneMmPkg.dsc
> index f279d9e7e5c7..8def9688fe7d 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dsc
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
> @@ -43,7 +43,7 @@ [LibraryClasses]
> #
> BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> -
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.
> inf
> + DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
>
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/Bas
> eDebugPrintErrorLevelLib.inf
> FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
>
> HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneM
> mCoreHobLib.inf
> @@ -66,10 +66,6 @@ [LibraryClasses.AARCH64]
> ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
>
> CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCach
> eMaintenanceLib.inf
>
> PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtr
> aActionLib/StandaloneMmPeCoffExtraActionLib.inf
> - PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
> -
> PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011Uart
> ClockLib.inf
> - # ARM PL011 UART Driver
> -
> SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLi
> b.inf
>
>
> StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmC
> oreEntryPoint/StandaloneMmCoreEntryPoint.inf
>
> @@ -83,11 +79,6 @@ [PcdsFixedAtBuild]
> gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff
> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x0f
>
> -[PcdsFixedAtBuild.AARCH64]
> - ## PL011 - Serial Terminal
> - gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c0b0000
> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> -
>
> ##############################################################
> #####################################
> #
> # Components Section - list of the modules and components that will be
> processed by compilation
> --
> 2.20.1
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 04/10] StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver
2019-03-05 13:32 ` [PATCH 04/10] StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver Ard Biesheuvel
@ 2019-03-05 14:22 ` Yao, Jiewen
0 siblings, 0 replies; 43+ messages in thread
From: Yao, Jiewen @ 2019-03-05 14:22 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Reviewed-by: jiewen.yao@intel.com
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Supreeth Venkatesh
> <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> <jagadeesh.ujja@arm.com>
> Subject: [PATCH 04/10] StandaloneMmPkg: remove redundant
> StandaloneMmDriverEntryPoint driver
>
> StandaloneMmDriverEntryPoint is implemented in MdePkg now, so let's
> drop the redundant StandaloneMmPkg version.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>
> StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmD
> riverEntryPoint.inf | 41 --------
>
> StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmD
> riverEntryPoint.c | 99 --------------------
> 2 files changed, 140 deletions(-)
>
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneM
> mDriverEntryPoint.inf
> b/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneM
> mDriverEntryPoint.inf
> deleted file mode 100644
> index 4d1896db10ba..000000000000
> ---
> a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneM
> mDriverEntryPoint.inf
> +++ /dev/null
> @@ -1,41 +0,0 @@
> -## @file
> -# Module entry point library for Standalone MM driver.
> -#
> -# Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> -# Copyright (c) 2016-2018, ARM Ltd. All rights reserved.<BR>
> -#
> -# This program and the accompanying materials
> -# are licensed and made available under the terms and conditions of the
> BSD License
> -# which accompanies this distribution. The full text of the license may be
> found at
> -# http://opensource.org/licenses/bsd-license.php.
> -# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> -# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> -#
> -#
> -##
> -
> -[Defines]
> - INF_VERSION = 0x0001001A
> - BASE_NAME = StandaloneMmDriverEntryPoint
> - FILE_GUID =
> BBC33478-98F8-4B78-B29D-574D681B7E43
> - MODULE_TYPE = MM_STANDALONE
> - VERSION_STRING = 1.0
> - PI_SPECIFICATION_VERSION = 0x00010032
> - LIBRARY_CLASS =
> StandaloneMmDriverEntryPoint|MM_STANDALONE
> -
> -#
> -# The following information is for reference only and not required by the
> build tools.
> -#
> -# VALID_ARCHITECTURES = IA32 X64 IPF EBC
> -#
> -
> -[Sources]
> - StandaloneMmDriverEntryPoint.c
> -
> -[Packages]
> - MdePkg/MdePkg.dec
> -
> -[LibraryClasses]
> - BaseLib
> - DebugLib
> -
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneM
> mDriverEntryPoint.c
> b/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneM
> mDriverEntryPoint.c
> deleted file mode 100644
> index 64bffcfccc8a..000000000000
> ---
> a/StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneM
> mDriverEntryPoint.c
> +++ /dev/null
> @@ -1,99 +0,0 @@
> -/** @file
> - Entry point to a Standalone MM driver.
> -
> -Copyright (c) 2015, Intel Corporation. All rights reserved.<BR>
> -Copyright (c) 2016 - 2018, ARM Ltd. All rights reserved.<BR>
> -
> -This program and the accompanying materials
> -are licensed and made available under the terms and conditions of the BSD
> License
> -which accompanies this distribution. The full text of the license may be
> found at
> -http://opensource.org/licenses/bsd-license.php
> -
> -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS"
> BASIS,
> -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> EXPRESS OR IMPLIED.
> -
> -**/
> -
> -#include <PiMm.h>
> -
> -#include <Library/BaseLib.h>
> -#include <Library/DebugLib.h>
> -
> -VOID
> -EFIAPI
> -ProcessLibraryConstructorList (
> - IN EFI_HANDLE ImageHandle,
> - IN IN EFI_MM_SYSTEM_TABLE *MmSystemTable
> - );
> -
> -EFI_STATUS
> -EFIAPI
> -ProcessModuleEntryPointList (
> - IN EFI_HANDLE ImageHandle,
> - IN IN EFI_MM_SYSTEM_TABLE *MmSystemTable
> - );
> -
> -VOID
> -EFIAPI
> -ProcessLibraryDestructorList (
> - IN EFI_HANDLE ImageHandle,
> - IN IN EFI_MM_SYSTEM_TABLE *MmSystemTable
> - );
> -
> -/**
> - The entry point of PE/COFF Image for a Standalone MM Driver.
> -
> - This function is the entry point for a Standalone MM Driver.
> - This function must call ProcessLibraryConstructorList() and
> - ProcessModuleEntryPointList().
> - If the return status from ProcessModuleEntryPointList()
> - is an error status, then ProcessLibraryDestructorList() must be called.
> - The return value from ProcessModuleEntryPointList() is returned.
> - If _gDriverUnloadImageCount is greater than zero, then an unload
> - handler must be registered for this image
> - and the unload handler must invoke ProcessModuleUnloadList().
> - If _gUefiDriverRevision is not zero and SystemTable->Hdr.Revision is less
> - than _gUefiDriverRevison, then return EFI_INCOMPATIBLE_VERSION.
> -
> -
> - @param ImageHandle The image handle of the Standalone MM
> Driver.
> - @param SystemTable A pointer to the EFI System Table.
> -
> - @retval EFI_SUCCESS The Standalone MM Driver
> exited normally.
> - @retval EFI_INCOMPATIBLE_VERSION _gUefiDriverRevision is
> greater than
> - SystemTable->Hdr.Revision.
> - @retval Other Return value from
> ProcessModuleEntryPointList().
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -_ModuleEntryPoint (
> - IN EFI_HANDLE ImageHandle,
> - IN IN EFI_MM_SYSTEM_TABLE *MmSystemTable
> - )
> -{
> - EFI_STATUS Status;
> -
> - //
> - // Call constructor for all libraries
> - //
> - ProcessLibraryConstructorList (ImageHandle, MmSystemTable);
> -
> - //
> - // Call the driver entry point
> - //
> - Status = ProcessModuleEntryPointList (ImageHandle, MmSystemTable);
> -
> - //
> - // If all of the drivers returned errors, then invoke all of the library
> destructors
> - //
> - if (EFI_ERROR (Status)) {
> - ProcessLibraryDestructorList (ImageHandle, MmSystemTable);
> - }
> -
> - //
> - // Return the cumulative return status code from all of the driver entry
> points
> - //
> - return Status;
> -}
> -
> --
> 2.20.1
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes
2019-03-05 13:32 ` [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes Ard Biesheuvel
@ 2019-03-05 15:50 ` Yao, Jiewen
2019-03-06 16:56 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Yao, Jiewen @ 2019-03-05 15:50 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Reviewed-by: jiewen.yao@intel.com
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Supreeth Venkatesh
> <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> <jagadeesh.ujja@arm.com>
> Subject: [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated
> firmware volumes
>
> Standalone MM requires 4 KB section alignment for all images, so that
> strict permissions can be applied. Unfortunately, this results in a
> lot of wasted space, which is usually costly in the secure world
> environment that standalone MM is expected to operate in.
>
> So let's permit the standalone MM drivers (but not the core) to be
> delivered in a compressed firmware volume.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/Core/StandaloneMmCore.inf | 1 +
> StandaloneMmPkg/Core/FwVol.c | 99
> ++++++++++++++++++--
> 2 files changed, 91 insertions(+), 9 deletions(-)
>
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> index ff2b8b9cef03..83d31e2d92c5 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> @@ -49,6 +49,7 @@ [LibraryClasses]
> BaseMemoryLib
> CacheMaintenanceLib
> DebugLib
> + ExtractGuidedSectionLib
> FvLib
> HobLib
> MemoryAllocationLib
> diff --git a/StandaloneMmPkg/Core/FwVol.c
> b/StandaloneMmPkg/Core/FwVol.c
> index 5abf98c24797..d95491f252f9 100644
> --- a/StandaloneMmPkg/Core/FwVol.c
> +++ b/StandaloneMmPkg/Core/FwVol.c
> @@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> #include "StandaloneMmCore.h"
> #include <Library/FvLib.h>
> +#include <Library/ExtractGuidedSectionLib.h>
>
> //
> // List of file types supported by dispatcher
> @@ -65,15 +66,25 @@ Returns:
>
> --*/
> {
> - EFI_STATUS Status;
> - EFI_STATUS DepexStatus;
> - EFI_FFS_FILE_HEADER *FileHeader;
> - EFI_FV_FILETYPE FileType;
> - VOID *Pe32Data;
> - UINTN Pe32DataSize;
> - VOID *Depex;
> - UINTN DepexSize;
> - UINTN Index;
> + EFI_STATUS Status;
> + EFI_STATUS DepexStatus;
> + EFI_FFS_FILE_HEADER *FileHeader;
> + EFI_FV_FILETYPE FileType;
> + VOID *Pe32Data;
> + UINTN Pe32DataSize;
> + VOID *Depex;
> + UINTN DepexSize;
> + UINTN Index;
> + EFI_COMMON_SECTION_HEADER *Section;
> + VOID *SectionData;
> + UINTN SectionDataSize;
> + UINT32 DstBufferSize;
> + VOID *ScratchBuffer;
> + UINT32 ScratchBufferSize;
> + VOID *DstBuffer;
> + UINT16 SectionAttribute;
> + UINT32 AuthenticationStatus;
> + EFI_FIRMWARE_VOLUME_HEADER *InnerFvHeader;
>
> DEBUG ((DEBUG_INFO, "MmCoreFfsFindMmDriver - 0x%x\n",
> FwVolHeader));
>
> @@ -83,6 +94,71 @@ Returns:
>
> FvIsBeingProcesssed (FwVolHeader);
>
> + //
> + // First check for encapsulated compressed firmware volumes
> + //
> + FileHeader = NULL;
> + do {
> + Status = FfsFindNextFile
> (EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE,
> + FwVolHeader, &FileHeader);
> + if (EFI_ERROR (Status)) {
> + break;
> + }
> + Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED,
> FileHeader,
> + &SectionData, &SectionDataSize);
> + if (EFI_ERROR (Status)) {
> + break;
> + }
> + Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
> + Status = ExtractGuidedSectionGetInfo (Section, &DstBufferSize,
> + &ScratchBufferSize, &SectionAttribute);
> + if (EFI_ERROR (Status)) {
> + break;
> + }
> +
> + //
> + // Allocate scratch buffer
> + //
> + ScratchBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES
> (ScratchBufferSize));
> + if (ScratchBuffer == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + //
> + // Allocate destination buffer, extra one page for adjustment
> + //
> + DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES
> (DstBufferSize));
> + if (DstBuffer == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + //
> + // Call decompress function
> + //
> + Status = ExtractGuidedSectionDecode (Section, &DstBuffer,
> ScratchBuffer,
> + &AuthenticationStatus);
> + FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
> + if (EFI_ERROR (Status)) {
> + goto FreeDstBuffer;
> + }
> +
> + DEBUG ((DEBUG_INFO,
> + "Processing compressed firmware volume (AuthenticationStatus
> == %x)\n",
> + AuthenticationStatus));
> +
> + Status = FindFfsSectionInSections (DstBuffer, DstBufferSize,
> + EFI_SECTION_FIRMWARE_VOLUME_IMAGE, &Section);
> + if (EFI_ERROR (Status)) {
> + goto FreeDstBuffer;
> + }
> +
> + InnerFvHeader = (VOID *)(Section + 1);
> + Status = MmCoreFfsFindMmDriver (InnerFvHeader);
> + if (EFI_ERROR (Status)) {
> + goto FreeDstBuffer;
> + }
> + } while (TRUE);
> +
> for (Index = 0; Index < sizeof (mMmFileTypes) / sizeof (mMmFileTypes[0]);
> Index++) {
> DEBUG ((DEBUG_INFO, "Check MmFileTypes - 0x%x\n",
> mMmFileTypes[Index]));
> FileType = mMmFileTypes[Index];
> @@ -100,5 +176,10 @@ Returns:
> } while (!EFI_ERROR (Status));
> }
>
> + return EFI_SUCCESS;
> +
> +FreeDstBuffer:
> + FreePages (DstBuffer, EFI_SIZE_TO_PAGES (DstBufferSize));
> +
> return Status;
> }
> --
> 2.20.1
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM
2019-03-05 13:32 ` [PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM Ard Biesheuvel
@ 2019-03-05 15:51 ` Yao, Jiewen
2019-03-06 16:58 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Yao, Jiewen @ 2019-03-05 15:51 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Reviewed-by: jiewen.yao@intel.com
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Supreeth Venkatesh
> <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> <jagadeesh.ujja@arm.com>
> Subject: [PATCH 08/10] StandaloneMmPkg/Core: drop support for
> dispatching FVs into MM
>
> Remove the support that permits calls into the MM context to dispatch
> firmware volumes that are not part of the initial standalone MM firmware
> volume.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/Core/StandaloneMmCore.h | 22 ----------
> StandaloneMmPkg/Core/Dispatcher.c | 46 --------------------
> StandaloneMmPkg/Core/StandaloneMmCore.c | 1 -
> 3 files changed, 69 deletions(-)
>
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h
> b/StandaloneMmPkg/Core/StandaloneMmCore.h
> index 0d20bcaa6be5..74338dc9da0d 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.h
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
> @@ -635,28 +635,6 @@ MmDriverDispatchHandler (
>
> @return Status Code
>
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmFvDispatchHandler (
> - IN EFI_HANDLE DispatchHandle,
> - IN CONST VOID *Context, OPTIONAL
> - IN OUT VOID *CommBuffer, OPTIONAL
> - IN OUT UINTN *CommBufferSize OPTIONAL
> - );
> -
> -/**
> - This function is the main entry point for an MM handler dispatch
> - or communicate-based callback.
> -
> - @param DispatchHandle The unique handle assigned to this handler
> by MmiHandlerRegister().
> - @param Context Points to an optional handler context
> which was specified when the handler was registered.
> - @param CommBuffer A pointer to a collection of data in
> memory that will
> - be conveyed from a non-MM environment
> into an MM environment.
> - @param CommBufferSize The size of the CommBuffer.
> -
> - @return Status Code
> -
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/StandaloneMmPkg/Core/Dispatcher.c
> b/StandaloneMmPkg/Core/Dispatcher.c
> index bede4832cfb7..4b2f38f700a0 100644
> --- a/StandaloneMmPkg/Core/Dispatcher.c
> +++ b/StandaloneMmPkg/Core/Dispatcher.c
> @@ -883,52 +883,6 @@ MmAddToDriverList (
> return EFI_SUCCESS;
> }
>
> -/**
> - This function is the main entry point for an MM handler dispatch
> - or communicate-based callback.
> -
> - @param DispatchHandle The unique handle assigned to this handler
> by SmiHandlerRegister().
> - @param Context Points to an optional handler context
> which was specified when the handler was registered.
> - @param CommBuffer A pointer to a collection of data in
> memory that will
> - be conveyed from a non-MM environment
> into an MM environment.
> - @param CommBufferSize The size of the CommBuffer.
> -
> - @return Status Code
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmFvDispatchHandler (
> - IN EFI_HANDLE DispatchHandle,
> - IN CONST VOID *Context, OPTIONAL
> - IN OUT VOID *CommBuffer, OPTIONAL
> - IN OUT UINTN *CommBufferSize OPTIONAL
> - )
> -{
> - EFI_STATUS Status;
> - EFI_MM_COMMUNICATE_FV_DISPATCH_DATA
> *CommunicationFvDispatchData;
> - EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
> -
> - DEBUG ((DEBUG_INFO, "MmFvDispatchHandler\n"));
> -
> - CommunicationFvDispatchData = CommBuffer;
> -
> - DEBUG ((DEBUG_INFO, " Dispatch - 0x%016lx - 0x%016lx\n",
> CommunicationFvDispatchData->Address,
> - CommunicationFvDispatchData->Size));
> -
> - FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER
> *)(UINTN)CommunicationFvDispatchData->Address;
> -
> - MmCoreFfsFindMmDriver (FwVolHeader);
> -
> - //
> - // Execute the MM Dispatcher on any newly discovered FVs and
> previously
> - // discovered MM drivers that have been discovered but not dispatched.
> - //
> - Status = MmDispatcher ();
> -
> - return Status;
> -}
> -
> /**
> Traverse the discovered list for any drivers that were discovered but not
> loaded
> because the dependency experessions evaluated to false.
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c
> b/StandaloneMmPkg/Core/StandaloneMmCore.c
> index ec53b8d8bec4..766cdb5c848c 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.c
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
> @@ -99,7 +99,6 @@ BOOLEAN mInLegacyBoot = FALSE;
> // Table of MMI Handlers that are registered by the MM Core when it is
> initialized
> //
> MM_CORE_MMI_HANDLERS mMmCoreMmiHandlers[] = {
> - { MmFvDispatchHandler, &gMmFvDispatchGuid,
> NULL, TRUE },
> { MmReadyToLockHandler, &gEfiDxeMmReadyToLockProtocolGuid,
> NULL, TRUE },
> { MmEndOfDxeHandler, &gEfiEndOfDxeEventGroupGuid,
> NULL, FALSE },
> { MmLegacyBootHandler, &gEfiEventLegacyBootGuid,
> NULL, FALSE },
> --
> 2.20.1
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time
2019-03-05 13:32 ` [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time Ard Biesheuvel
@ 2019-03-05 15:51 ` Yao, Jiewen
2019-03-06 16:56 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Yao, Jiewen @ 2019-03-05 15:51 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Reviewed-by: jiewen.yao@intel.com
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Supreeth Venkatesh
> <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> <jagadeesh.ujja@arm.com>
> Subject: [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init
> time
>
> Instead of deferring dispatch of the remaining MM drivers once the
> CPU driver has been dispatched, proceed and dispatch all drivers.
> This makes sense for standalone MM, since all dispatchable drivers
> should be present in the initial firmware volume anyway: dispatch
> of additional FVs originating in the non-secure side is not supported.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/Core/Dispatcher.c | 92 --------------------
> StandaloneMmPkg/Core/StandaloneMmCore.c | 1 -
> 2 files changed, 93 deletions(-)
>
> diff --git a/StandaloneMmPkg/Core/Dispatcher.c
> b/StandaloneMmPkg/Core/Dispatcher.c
> index 8a2ad5118d92..bede4832cfb7 100644
> --- a/StandaloneMmPkg/Core/Dispatcher.c
> +++ b/StandaloneMmPkg/Core/Dispatcher.c
> @@ -575,7 +575,6 @@ MmDispatcher (
> LIST_ENTRY *Link;
> EFI_MM_DRIVER_ENTRY *DriverEntry;
> BOOLEAN ReadyToRun;
> - BOOLEAN PreviousMmEntryPointRegistered;
>
> DEBUG ((DEBUG_INFO, "MmDispatcher\n"));
>
> @@ -639,11 +638,6 @@ MmDispatcher (
> DriverEntry->Initialized = TRUE;
> RemoveEntryList (&DriverEntry->ScheduledLink);
>
> - //
> - // Cache state of MmEntryPointRegistered before calling entry point
> - //
> - PreviousMmEntryPointRegistered =
> gMmCorePrivate->MmEntryPointRegistered;
> -
> //
> // For each MM driver, pass NULL as ImageHandle
> //
> @@ -661,20 +655,6 @@ MmDispatcher (
> DEBUG ((DEBUG_INFO, "StartImage Status - %r\n", Status));
> MmFreePages(DriverEntry->ImageBuffer,
> DriverEntry->NumberOfPage);
> }
> -
> - if (!PreviousMmEntryPointRegistered &&
> gMmCorePrivate->MmEntryPointRegistered) {
> - //
> - // Return immediately if the MM Entry Point was registered by the
> MM
> - // Driver that was just dispatched. The MM IPL will reinvoke the
> MM
> - // Core Dispatcher. This is required so MM Mode may be
> enabled as soon
> - // as all the dependent MM Drivers for MM Mode have been
> dispatched.
> - // Once the MM Entry Point has been registered, then MM Mode
> will be
> - // used.
> - //
> - gRequestDispatch = TRUE;
> - gDispatcherRunning = FALSE;
> - return EFI_NOT_READY;
> - }
> }
>
> //
> @@ -903,78 +883,6 @@ MmAddToDriverList (
> return EFI_SUCCESS;
> }
>
> -/**
> - This function is the main entry point for an MM handler dispatch
> - or communicate-based callback.
> -
> - Event notification that is fired every time a FV dispatch protocol is added.
> - More than one protocol may have been added when this event is fired, so
> you
> - must loop on MmLocateHandle () to see how many protocols were added
> and
> - do the following to each FV:
> - If the Fv has already been processed, skip it. If the Fv has not been
> - processed then mark it as being processed, as we are about to process it.
> - Read the Fv and add any driver in the Fv to the mDiscoveredList.The
> - mDiscoveredList is never free'ed and contains variables that define
> - the other states the MM driver transitions to..
> - While you are at it read the A Priori file into memory.
> - Place drivers in the A Priori list onto the mScheduledQueue.
> -
> - @param DispatchHandle The unique handle assigned to this handler
> by SmiHandlerRegister().
> - @param Context Points to an optional handler context
> which was specified when the handler was registered.
> - @param CommBuffer A pointer to a collection of data in
> memory that will
> - be conveyed from a non-MM environment
> into an MM environment.
> - @param CommBufferSize The size of the CommBuffer.
> -
> - @return Status Code
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmDriverDispatchHandler (
> - IN EFI_HANDLE DispatchHandle,
> - IN CONST VOID *Context, OPTIONAL
> - IN OUT VOID *CommBuffer, OPTIONAL
> - IN OUT UINTN *CommBufferSize OPTIONAL
> - )
> -{
> - EFI_STATUS Status;
> -
> - DEBUG ((DEBUG_INFO, "MmDriverDispatchHandler\n"));
> -
> - //
> - // Execute the MM Dispatcher on any newly discovered FVs and
> previously
> - // discovered MM drivers that have been discovered but not dispatched.
> - //
> - Status = MmDispatcher ();
> -
> - //
> - // Check to see if CommBuffer and CommBufferSize are valid
> - //
> - if (CommBuffer != NULL && CommBufferSize != NULL) {
> - if (*CommBufferSize > 0) {
> - if (Status == EFI_NOT_READY) {
> - //
> - // If a the MM Core Entry Point was just registered, then set flag
> to
> - // request the MM Dispatcher to be restarted.
> - //
> - *(UINT8 *)CommBuffer =
> COMM_BUFFER_MM_DISPATCH_RESTART;
> - } else if (!EFI_ERROR (Status)) {
> - //
> - // Set the flag to show that the MM Dispatcher executed without
> errors
> - //
> - *(UINT8 *)CommBuffer =
> COMM_BUFFER_MM_DISPATCH_SUCCESS;
> - } else {
> - //
> - // Set the flag to show that the MM Dispatcher encountered an
> error
> - //
> - *(UINT8 *)CommBuffer =
> COMM_BUFFER_MM_DISPATCH_ERROR;
> - }
> - }
> - }
> -
> - return EFI_SUCCESS;
> -}
> -
> /**
> This function is the main entry point for an MM handler dispatch
> or communicate-based callback.
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c
> b/StandaloneMmPkg/Core/StandaloneMmCore.c
> index 74432320bfc7..ec53b8d8bec4 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.c
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
> @@ -100,7 +100,6 @@ BOOLEAN mInLegacyBoot = FALSE;
> //
> MM_CORE_MMI_HANDLERS mMmCoreMmiHandlers[] = {
> { MmFvDispatchHandler, &gMmFvDispatchGuid,
> NULL, TRUE },
> - { MmDriverDispatchHandler, &gEfiEventDxeDispatchGuid,
> NULL, TRUE },
> { MmReadyToLockHandler, &gEfiDxeMmReadyToLockProtocolGuid,
> NULL, TRUE },
> { MmEndOfDxeHandler, &gEfiEndOfDxeEventGroupGuid,
> NULL, FALSE },
> { MmLegacyBootHandler, &gEfiEventLegacyBootGuid,
> NULL, FALSE },
> --
> 2.20.1
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context
2019-03-05 13:32 ` [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context Ard Biesheuvel
@ 2019-03-05 15:55 ` Yao, Jiewen
2019-03-05 15:58 ` Ard Biesheuvel
2019-03-06 16:58 ` Achin Gupta
1 sibling, 1 reply; 43+ messages in thread
From: Yao, Jiewen @ 2019-03-05 15:55 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Hi
In original SMM infrastructure, there are lots of interaction that SMM has to know the DXE status.
In StandaloneMm, I don't expect we have many interaction. Is there any specific example?
I am totally OK to add those. And I just want to know more usage.
Reviewed-by: Jiewen.yao@intel.com
Thank you
Yao Jiewen
> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, March 5, 2019 5:33 AM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> <achin.gupta@arm.com>; Supreeth Venkatesh
> <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> <jagadeesh.ujja@arm.com>
> Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected
> PI events into MM context
>
> PI defines a few architected events that have significance in the MM
> context as well as in the non-secure DXE context. So register notify
> handlers for these events, and relay them into the standalone MM world.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5 +++
> ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47
> +++++++++++++++++++-
> 2 files changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> index 88beafa39c05..8bf269270f9d 100644
> --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> @@ -48,6 +48,11 @@ [LibraryClasses]
> [Protocols]
> gEfiMmCommunicationProtocolGuid ## PRODUCES
>
> +[Guids]
> + gEfiEndOfDxeEventGroupGuid
> + gEfiEventExitBootServicesGuid
> + gEfiEventReadyToBootGuid
> +
> [Pcd.common]
> gArmTokenSpaceGuid.PcdMmBufferBase
> gArmTokenSpaceGuid.PcdMmBufferSize
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> index feb9fa9f4ead..3203cf801a19 100644
> --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> @@ -265,6 +265,43 @@ GetMmCompatibility ()
> return Status;
> }
>
> +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
> + &gEfiEndOfDxeEventGroupGuid,
> + &gEfiEventExitBootServicesGuid,
> + &gEfiEventReadyToBootGuid,
> +};
> +
> +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)];
> +
> +/**
> + Event notification that is fired when GUIDed Event Group is signaled.
> +
> + @param Event The Event that is being processed,
> not used.
> + @param Context Event Context, not used.
> +
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +MmGuidedEventNotify (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + EFI_MM_COMMUNICATE_HEADER Header;
> + UINTN Size;
> +
> + //
> + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure
> + //
> + CopyGuid (&Header.HeaderGuid, Context);
> + Header.MessageLength = 1;
> + Header.Data[0] = 0;
> +
> + Size = sizeof (Header);
> + MmCommunicationCommunicate (&mMmCommunication, &Header,
> &Size);
> +}
> +
> /**
> The Entry Point for MM Communication
>
> @@ -287,6 +324,7 @@ MmCommunicationInitialize (
> )
> {
> EFI_STATUS Status;
> + UINTN Index;
>
> // Check if we can make the MM call
> Status = GetMmCompatibility ();
> @@ -351,8 +389,13 @@ MmCommunicationInitialize (
> NULL,
> &mSetVirtualAddressMapEvent
> );
> - if (Status == EFI_SUCCESS) {
> - return Status;
> + ASSERT_EFI_ERROR (Status);
> +
> + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) {
> + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> + MmGuidedEventNotify, mGuidedEventGuid[Index],
> + mGuidedEventGuid[Index],
> &mGuidedEvent[Index]);
> + ASSERT_EFI_ERROR (Status);
> }
>
> gBS->UninstallProtocolInterface (
> --
> 2.20.1
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context
2019-03-05 15:55 ` Yao, Jiewen
@ 2019-03-05 15:58 ` Ard Biesheuvel
2019-03-05 16:04 ` Yao, Jiewen
0 siblings, 1 reply; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 15:58 UTC (permalink / raw)
To: Yao, Jiewen
Cc: edk2-devel@lists.01.org, Achin Gupta, Supreeth Venkatesh,
Leif Lindholm, Jagadeesh Ujja
On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> Hi
> In original SMM infrastructure, there are lots of interaction that SMM has to know the DXE status.
>
> In StandaloneMm, I don't expect we have many interaction. Is there any specific example?
>
> I am totally OK to add those. And I just want to know more usage.
>
> Reviewed-by: Jiewen.yao@intel.com
>
Jiewen,
Thanks for the review.
It is not 100% clear at the moment, but since existing third party
software designed to run in MM context may make assumptions about
security of the platform (e.g., before vs after end of dxe) based on
these events, we should at least signal the common ones added in this
patch.
> > -----Original Message-----
> > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > Sent: Tuesday, March 5, 2019 5:33 AM
> > To: edk2-devel@lists.01.org
> > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> > <achin.gupta@arm.com>; Supreeth Venkatesh
> > <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> > <jagadeesh.ujja@arm.com>
> > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected
> > PI events into MM context
> >
> > PI defines a few architected events that have significance in the MM
> > context as well as in the non-secure DXE context. So register notify
> > handlers for these events, and relay them into the standalone MM world.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5 +++
> > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47
> > +++++++++++++++++++-
> > 2 files changed, 50 insertions(+), 2 deletions(-)
> >
> > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > index 88beafa39c05..8bf269270f9d 100644
> > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > @@ -48,6 +48,11 @@ [LibraryClasses]
> > [Protocols]
> > gEfiMmCommunicationProtocolGuid ## PRODUCES
> >
> > +[Guids]
> > + gEfiEndOfDxeEventGroupGuid
> > + gEfiEventExitBootServicesGuid
> > + gEfiEventReadyToBootGuid
> > +
> > [Pcd.common]
> > gArmTokenSpaceGuid.PcdMmBufferBase
> > gArmTokenSpaceGuid.PcdMmBufferSize
> > diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > index feb9fa9f4ead..3203cf801a19 100644
> > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > @@ -265,6 +265,43 @@ GetMmCompatibility ()
> > return Status;
> > }
> >
> > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
> > + &gEfiEndOfDxeEventGroupGuid,
> > + &gEfiEventExitBootServicesGuid,
> > + &gEfiEventReadyToBootGuid,
> > +};
> > +
> > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)];
> > +
> > +/**
> > + Event notification that is fired when GUIDed Event Group is signaled.
> > +
> > + @param Event The Event that is being processed,
> > not used.
> > + @param Context Event Context, not used.
> > +
> > +**/
> > +STATIC
> > +VOID
> > +EFIAPI
> > +MmGuidedEventNotify (
> > + IN EFI_EVENT Event,
> > + IN VOID *Context
> > + )
> > +{
> > + EFI_MM_COMMUNICATE_HEADER Header;
> > + UINTN Size;
> > +
> > + //
> > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure
> > + //
> > + CopyGuid (&Header.HeaderGuid, Context);
> > + Header.MessageLength = 1;
> > + Header.Data[0] = 0;
> > +
> > + Size = sizeof (Header);
> > + MmCommunicationCommunicate (&mMmCommunication, &Header,
> > &Size);
> > +}
> > +
> > /**
> > The Entry Point for MM Communication
> >
> > @@ -287,6 +324,7 @@ MmCommunicationInitialize (
> > )
> > {
> > EFI_STATUS Status;
> > + UINTN Index;
> >
> > // Check if we can make the MM call
> > Status = GetMmCompatibility ();
> > @@ -351,8 +389,13 @@ MmCommunicationInitialize (
> > NULL,
> > &mSetVirtualAddressMapEvent
> > );
> > - if (Status == EFI_SUCCESS) {
> > - return Status;
> > + ASSERT_EFI_ERROR (Status);
> > +
> > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) {
> > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> > + MmGuidedEventNotify, mGuidedEventGuid[Index],
> > + mGuidedEventGuid[Index],
> > &mGuidedEvent[Index]);
> > + ASSERT_EFI_ERROR (Status);
> > }
> >
> > gBS->UninstallProtocolInterface (
> > --
> > 2.20.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context
2019-03-05 15:58 ` Ard Biesheuvel
@ 2019-03-05 16:04 ` Yao, Jiewen
2019-03-05 16:07 ` Ard Biesheuvel
0 siblings, 1 reply; 43+ messages in thread
From: Yao, Jiewen @ 2019-03-05 16:04 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org
OK. To keep the compatibility of existing MM driver. That makes sense.
If it is for security, I think EndOfDxe is the only point.
ReadyToBoot and ExitBootService cannot be used for security purpose.
Then do we need SmmReadyToLock ? :-)
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, March 5, 2019 7:58 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> architected PI events into MM context
>
> On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >
> > Hi
> > In original SMM infrastructure, there are lots of interaction that SMM has
> to know the DXE status.
> >
> > In StandaloneMm, I don't expect we have many interaction. Is there any
> specific example?
> >
> > I am totally OK to add those. And I just want to know more usage.
> >
> > Reviewed-by: Jiewen.yao@intel.com
> >
>
> Jiewen,
>
> Thanks for the review.
>
> It is not 100% clear at the moment, but since existing third party
> software designed to run in MM context may make assumptions about
> security of the platform (e.g., before vs after end of dxe) based on
> these events, we should at least signal the common ones added in this
> patch.
>
>
>
>
> > > -----Original Message-----
> > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > Sent: Tuesday, March 5, 2019 5:33 AM
> > > To: edk2-devel@lists.01.org
> > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> > > <achin.gupta@arm.com>; Supreeth Venkatesh
> > > <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> > > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> > > <jagadeesh.ujja@arm.com>
> > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> architected
> > > PI events into MM context
> > >
> > > PI defines a few architected events that have significance in the MM
> > > context as well as in the non-secure DXE context. So register notify
> > > handlers for these events, and relay them into the standalone MM world.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > ---
> > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5
> +++
> > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47
> > > +++++++++++++++++++-
> > > 2 files changed, 50 insertions(+), 2 deletions(-)
> > >
> > > diff --git
> a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > index 88beafa39c05..8bf269270f9d 100644
> > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > @@ -48,6 +48,11 @@ [LibraryClasses]
> > > [Protocols]
> > > gEfiMmCommunicationProtocolGuid ## PRODUCES
> > >
> > > +[Guids]
> > > + gEfiEndOfDxeEventGroupGuid
> > > + gEfiEventExitBootServicesGuid
> > > + gEfiEventReadyToBootGuid
> > > +
> > > [Pcd.common]
> > > gArmTokenSpaceGuid.PcdMmBufferBase
> > > gArmTokenSpaceGuid.PcdMmBufferSize
> > > diff --git
> a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > index feb9fa9f4ead..3203cf801a19 100644
> > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > @@ -265,6 +265,43 @@ GetMmCompatibility ()
> > > return Status;
> > > }
> > >
> > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
> > > + &gEfiEndOfDxeEventGroupGuid,
> > > + &gEfiEventExitBootServicesGuid,
> > > + &gEfiEventReadyToBootGuid,
> > > +};
> > > +
> > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)];
> > > +
> > > +/**
> > > + Event notification that is fired when GUIDed Event Group is signaled.
> > > +
> > > + @param Event The Event that is being
> processed,
> > > not used.
> > > + @param Context Event Context, not used.
> > > +
> > > +**/
> > > +STATIC
> > > +VOID
> > > +EFIAPI
> > > +MmGuidedEventNotify (
> > > + IN EFI_EVENT Event,
> > > + IN VOID *Context
> > > + )
> > > +{
> > > + EFI_MM_COMMUNICATE_HEADER Header;
> > > + UINTN Size;
> > > +
> > > + //
> > > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure
> > > + //
> > > + CopyGuid (&Header.HeaderGuid, Context);
> > > + Header.MessageLength = 1;
> > > + Header.Data[0] = 0;
> > > +
> > > + Size = sizeof (Header);
> > > + MmCommunicationCommunicate (&mMmCommunication, &Header,
> > > &Size);
> > > +}
> > > +
> > > /**
> > > The Entry Point for MM Communication
> > >
> > > @@ -287,6 +324,7 @@ MmCommunicationInitialize (
> > > )
> > > {
> > > EFI_STATUS Status;
> > > + UINTN Index;
> > >
> > > // Check if we can make the MM call
> > > Status = GetMmCompatibility ();
> > > @@ -351,8 +389,13 @@ MmCommunicationInitialize (
> > > NULL,
> > > &mSetVirtualAddressMapEvent
> > > );
> > > - if (Status == EFI_SUCCESS) {
> > > - return Status;
> > > + ASSERT_EFI_ERROR (Status);
> > > +
> > > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) {
> > > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL,
> TPL_CALLBACK,
> > > + MmGuidedEventNotify,
> mGuidedEventGuid[Index],
> > > + mGuidedEventGuid[Index],
> > > &mGuidedEvent[Index]);
> > > + ASSERT_EFI_ERROR (Status);
> > > }
> > >
> > > gBS->UninstallProtocolInterface (
> > > --
> > > 2.20.1
> >
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context
2019-03-05 16:04 ` Yao, Jiewen
@ 2019-03-05 16:07 ` Ard Biesheuvel
2019-03-05 16:19 ` Yao, Jiewen
0 siblings, 1 reply; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 16:07 UTC (permalink / raw)
To: Yao, Jiewen; +Cc: edk2-devel@lists.01.org
On Tue, 5 Mar 2019 at 17:04, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>
> OK. To keep the compatibility of existing MM driver. That makes sense.
>
> If it is for security, I think EndOfDxe is the only point.
> ReadyToBoot and ExitBootService cannot be used for security purpose.
>
OK, good to know. I will keep them for the time being - MM drivers may
be able to release resources or do other useful things when the
non-secure side enters runtime mode.
> Then do we need SmmReadyToLock ? :-)
>
Good point. It looked fairly x86 specific to me. Do you think it is
likely to be used in OEM code running in MM mode?
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Ard Biesheuvel
> > Sent: Tuesday, March 5, 2019 7:58 AM
> > To: Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> > architected PI events into MM context
> >
> > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> > >
> > > Hi
> > > In original SMM infrastructure, there are lots of interaction that SMM has
> > to know the DXE status.
> > >
> > > In StandaloneMm, I don't expect we have many interaction. Is there any
> > specific example?
> > >
> > > I am totally OK to add those. And I just want to know more usage.
> > >
> > > Reviewed-by: Jiewen.yao@intel.com
> > >
> >
> > Jiewen,
> >
> > Thanks for the review.
> >
> > It is not 100% clear at the moment, but since existing third party
> > software designed to run in MM context may make assumptions about
> > security of the platform (e.g., before vs after end of dxe) based on
> > these events, we should at least signal the common ones added in this
> > patch.
> >
> >
> >
> >
> > > > -----Original Message-----
> > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > Sent: Tuesday, March 5, 2019 5:33 AM
> > > > To: edk2-devel@lists.01.org
> > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> > > > <achin.gupta@arm.com>; Supreeth Venkatesh
> > > > <supreeth.venkatesh@arm.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> > > > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> > > > <jagadeesh.ujja@arm.com>
> > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> > architected
> > > > PI events into MM context
> > > >
> > > > PI defines a few architected events that have significance in the MM
> > > > context as well as in the non-secure DXE context. So register notify
> > > > handlers for these events, and relay them into the standalone MM world.
> > > >
> > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > ---
> > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5
> > +++
> > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47
> > > > +++++++++++++++++++-
> > > > 2 files changed, 50 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git
> > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > index 88beafa39c05..8bf269270f9d 100644
> > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > @@ -48,6 +48,11 @@ [LibraryClasses]
> > > > [Protocols]
> > > > gEfiMmCommunicationProtocolGuid ## PRODUCES
> > > >
> > > > +[Guids]
> > > > + gEfiEndOfDxeEventGroupGuid
> > > > + gEfiEventExitBootServicesGuid
> > > > + gEfiEventReadyToBootGuid
> > > > +
> > > > [Pcd.common]
> > > > gArmTokenSpaceGuid.PcdMmBufferBase
> > > > gArmTokenSpaceGuid.PcdMmBufferSize
> > > > diff --git
> > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > index feb9fa9f4ead..3203cf801a19 100644
> > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > @@ -265,6 +265,43 @@ GetMmCompatibility ()
> > > > return Status;
> > > > }
> > > >
> > > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
> > > > + &gEfiEndOfDxeEventGroupGuid,
> > > > + &gEfiEventExitBootServicesGuid,
> > > > + &gEfiEventReadyToBootGuid,
> > > > +};
> > > > +
> > > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)];
> > > > +
> > > > +/**
> > > > + Event notification that is fired when GUIDed Event Group is signaled.
> > > > +
> > > > + @param Event The Event that is being
> > processed,
> > > > not used.
> > > > + @param Context Event Context, not used.
> > > > +
> > > > +**/
> > > > +STATIC
> > > > +VOID
> > > > +EFIAPI
> > > > +MmGuidedEventNotify (
> > > > + IN EFI_EVENT Event,
> > > > + IN VOID *Context
> > > > + )
> > > > +{
> > > > + EFI_MM_COMMUNICATE_HEADER Header;
> > > > + UINTN Size;
> > > > +
> > > > + //
> > > > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure
> > > > + //
> > > > + CopyGuid (&Header.HeaderGuid, Context);
> > > > + Header.MessageLength = 1;
> > > > + Header.Data[0] = 0;
> > > > +
> > > > + Size = sizeof (Header);
> > > > + MmCommunicationCommunicate (&mMmCommunication, &Header,
> > > > &Size);
> > > > +}
> > > > +
> > > > /**
> > > > The Entry Point for MM Communication
> > > >
> > > > @@ -287,6 +324,7 @@ MmCommunicationInitialize (
> > > > )
> > > > {
> > > > EFI_STATUS Status;
> > > > + UINTN Index;
> > > >
> > > > // Check if we can make the MM call
> > > > Status = GetMmCompatibility ();
> > > > @@ -351,8 +389,13 @@ MmCommunicationInitialize (
> > > > NULL,
> > > > &mSetVirtualAddressMapEvent
> > > > );
> > > > - if (Status == EFI_SUCCESS) {
> > > > - return Status;
> > > > + ASSERT_EFI_ERROR (Status);
> > > > +
> > > > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) {
> > > > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL,
> > TPL_CALLBACK,
> > > > + MmGuidedEventNotify,
> > mGuidedEventGuid[Index],
> > > > + mGuidedEventGuid[Index],
> > > > &mGuidedEvent[Index]);
> > > > + ASSERT_EFI_ERROR (Status);
> > > > }
> > > >
> > > > gBS->UninstallProtocolInterface (
> > > > --
> > > > 2.20.1
> > >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context
2019-03-05 16:07 ` Ard Biesheuvel
@ 2019-03-05 16:19 ` Yao, Jiewen
2019-03-05 16:53 ` Felix Polyudov
0 siblings, 1 reply; 43+ messages in thread
From: Yao, Jiewen @ 2019-03-05 16:19 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org
For current X86 world, we do use both SmmReadyToLock and EndOfDxe, for almost all X86 platform.
So, let me clarify:
If we try to align with PI spec, we can add EndOfDxe/ReadyToBoot/ExitBootService.
If we try to align with security, we can add EndOfDxe/SmmReadyToLock.
It depends upon the what goal we want to achieve. That is why I ask if we have specific use case.
Anyway, I think we can add when it is really needed later.
So I am OK with current patch.
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, March 5, 2019 8:07 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> architected PI events into MM context
>
> On Tue, 5 Mar 2019 at 17:04, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >
> > OK. To keep the compatibility of existing MM driver. That makes sense.
> >
> > If it is for security, I think EndOfDxe is the only point.
> > ReadyToBoot and ExitBootService cannot be used for security purpose.
> >
>
> OK, good to know. I will keep them for the time being - MM drivers may
> be able to release resources or do other useful things when the
> non-secure side enters runtime mode.
>
> > Then do we need SmmReadyToLock ? :-)
> >
>
> Good point. It looked fairly x86 specific to me. Do you think it is
> likely to be used in OEM code running in MM mode?
>
>
>
>
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> Of
> > > Ard Biesheuvel
> > > Sent: Tuesday, March 5, 2019 7:58 AM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>
> > > Cc: edk2-devel@lists.01.org
> > > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe:
> signal
> > > architected PI events into MM context
> > >
> > > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen <jiewen.yao@intel.com>
> wrote:
> > > >
> > > > Hi
> > > > In original SMM infrastructure, there are lots of interaction that SMM
> has
> > > to know the DXE status.
> > > >
> > > > In StandaloneMm, I don't expect we have many interaction. Is there
> any
> > > specific example?
> > > >
> > > > I am totally OK to add those. And I just want to know more usage.
> > > >
> > > > Reviewed-by: Jiewen.yao@intel.com
> > > >
> > >
> > > Jiewen,
> > >
> > > Thanks for the review.
> > >
> > > It is not 100% clear at the moment, but since existing third party
> > > software designed to run in MM context may make assumptions about
> > > security of the platform (e.g., before vs after end of dxe) based on
> > > these events, we should at least signal the common ones added in this
> > > patch.
> > >
> > >
> > >
> > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > Sent: Tuesday, March 5, 2019 5:33 AM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> > > > > <achin.gupta@arm.com>; Supreeth Venkatesh
> > > > > <supreeth.venkatesh@arm.com>; Yao, Jiewen
> <jiewen.yao@intel.com>;
> > > > > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> > > > > <jagadeesh.ujja@arm.com>
> > > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> > > architected
> > > > > PI events into MM context
> > > > >
> > > > > PI defines a few architected events that have significance in the MM
> > > > > context as well as in the non-secure DXE context. So register notify
> > > > > handlers for these events, and relay them into the standalone MM
> world.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > ---
> > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |
> 5
> > > +++
> > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c |
> 47
> > > > > +++++++++++++++++++-
> > > > > 2 files changed, 50 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git
> > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > index 88beafa39c05..8bf269270f9d 100644
> > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > +++
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > @@ -48,6 +48,11 @@ [LibraryClasses]
> > > > > [Protocols]
> > > > > gEfiMmCommunicationProtocolGuid ##
> PRODUCES
> > > > >
> > > > > +[Guids]
> > > > > + gEfiEndOfDxeEventGroupGuid
> > > > > + gEfiEventExitBootServicesGuid
> > > > > + gEfiEventReadyToBootGuid
> > > > > +
> > > > > [Pcd.common]
> > > > > gArmTokenSpaceGuid.PcdMmBufferBase
> > > > > gArmTokenSpaceGuid.PcdMmBufferSize
> > > > > diff --git
> > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > index feb9fa9f4ead..3203cf801a19 100644
> > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > @@ -265,6 +265,43 @@ GetMmCompatibility ()
> > > > > return Status;
> > > > > }
> > > > >
> > > > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
> > > > > + &gEfiEndOfDxeEventGroupGuid,
> > > > > + &gEfiEventExitBootServicesGuid,
> > > > > + &gEfiEventReadyToBootGuid,
> > > > > +};
> > > > > +
> > > > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE
> (mGuidedEventGuid)];
> > > > > +
> > > > > +/**
> > > > > + Event notification that is fired when GUIDed Event Group is
> signaled.
> > > > > +
> > > > > + @param Event The Event that is being
> > > processed,
> > > > > not used.
> > > > > + @param Context Event Context, not used.
> > > > > +
> > > > > +**/
> > > > > +STATIC
> > > > > +VOID
> > > > > +EFIAPI
> > > > > +MmGuidedEventNotify (
> > > > > + IN EFI_EVENT Event,
> > > > > + IN VOID *Context
> > > > > + )
> > > > > +{
> > > > > + EFI_MM_COMMUNICATE_HEADER Header;
> > > > > + UINTN Size;
> > > > > +
> > > > > + //
> > > > > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER
> structure
> > > > > + //
> > > > > + CopyGuid (&Header.HeaderGuid, Context);
> > > > > + Header.MessageLength = 1;
> > > > > + Header.Data[0] = 0;
> > > > > +
> > > > > + Size = sizeof (Header);
> > > > > + MmCommunicationCommunicate (&mMmCommunication,
> &Header,
> > > > > &Size);
> > > > > +}
> > > > > +
> > > > > /**
> > > > > The Entry Point for MM Communication
> > > > >
> > > > > @@ -287,6 +324,7 @@ MmCommunicationInitialize (
> > > > > )
> > > > > {
> > > > > EFI_STATUS Status;
> > > > > + UINTN Index;
> > > > >
> > > > > // Check if we can make the MM call
> > > > > Status = GetMmCompatibility ();
> > > > > @@ -351,8 +389,13 @@ MmCommunicationInitialize (
> > > > > NULL,
> > > > > &mSetVirtualAddressMapEvent
> > > > > );
> > > > > - if (Status == EFI_SUCCESS) {
> > > > > - return Status;
> > > > > + ASSERT_EFI_ERROR (Status);
> > > > > +
> > > > > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++)
> {
> > > > > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL,
> > > TPL_CALLBACK,
> > > > > + MmGuidedEventNotify,
> > > mGuidedEventGuid[Index],
> > > > > + mGuidedEventGuid[Index],
> > > > > &mGuidedEvent[Index]);
> > > > > + ASSERT_EFI_ERROR (Status);
> > > > > }
> > > > >
> > > > > gBS->UninstallProtocolInterface (
> > > > > --
> > > > > 2.20.1
> > > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context
2019-03-05 16:19 ` Yao, Jiewen
@ 2019-03-05 16:53 ` Felix Polyudov
2019-03-05 17:29 ` Ard Biesheuvel
0 siblings, 1 reply; 43+ messages in thread
From: Felix Polyudov @ 2019-03-05 16:53 UTC (permalink / raw)
To: 'Yao, Jiewen', Ard Biesheuvel; +Cc: edk2-devel@lists.01.org
There is an architectural difference between EndOfDxe and SmmReadyToLock events.
It's important to have both of them.
Here is what PI specification says:
==
Transition from the environment where all of the components are under the authority of the platform manufacturer to the environment where third party modules are executed is a two-step process:
1.End of DXE Event is signaled. This event presents the last opportunity to use resources or interfaces that are going to be locked or disabled in anticipation of the invocation of 3rd party extensible modules.
2.DXE SMM Ready to Lock Protocol is installed. PI modules that need to lock or protect their resources in anticipation of the invocation of 3rd party extensible modules should register for notification on installation of this protocol and effect the appropriate protections in their notification handlers
==
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen
Sent: Tuesday, March 05, 2019 11:19 AM
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context
For current X86 world, we do use both SmmReadyToLock and EndOfDxe, for almost all X86 platform.
So, let me clarify:
If we try to align with PI spec, we can add EndOfDxe/ReadyToBoot/ExitBootService.
If we try to align with security, we can add EndOfDxe/SmmReadyToLock.
It depends upon the what goal we want to achieve. That is why I ask if we have specific use case.
Anyway, I think we can add when it is really needed later.
So I am OK with current patch.
Thank you
Yao Jiewen
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ard Biesheuvel
> Sent: Tuesday, March 5, 2019 8:07 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> architected PI events into MM context
>
> On Tue, 5 Mar 2019 at 17:04, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> >
> > OK. To keep the compatibility of existing MM driver. That makes sense.
> >
> > If it is for security, I think EndOfDxe is the only point.
> > ReadyToBoot and ExitBootService cannot be used for security purpose.
> >
>
> OK, good to know. I will keep them for the time being - MM drivers may
> be able to release resources or do other useful things when the
> non-secure side enters runtime mode.
>
> > Then do we need SmmReadyToLock ? :-)
> >
>
> Good point. It looked fairly x86 specific to me. Do you think it is
> likely to be used in OEM code running in MM mode?
>
>
>
>
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> Of
> > > Ard Biesheuvel
> > > Sent: Tuesday, March 5, 2019 7:58 AM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>
> > > Cc: edk2-devel@lists.01.org
> > > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe:
> signal
> > > architected PI events into MM context
> > >
> > > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen <jiewen.yao@intel.com>
> wrote:
> > > >
> > > > Hi
> > > > In original SMM infrastructure, there are lots of interaction that SMM
> has
> > > to know the DXE status.
> > > >
> > > > In StandaloneMm, I don't expect we have many interaction. Is there
> any
> > > specific example?
> > > >
> > > > I am totally OK to add those. And I just want to know more usage.
> > > >
> > > > Reviewed-by: Jiewen.yao@intel.com
> > > >
> > >
> > > Jiewen,
> > >
> > > Thanks for the review.
> > >
> > > It is not 100% clear at the moment, but since existing third party
> > > software designed to run in MM context may make assumptions about
> > > security of the platform (e.g., before vs after end of dxe) based on
> > > these events, we should at least signal the common ones added in this
> > > patch.
> > >
> > >
> > >
> > >
> > > > > -----Original Message-----
> > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > Sent: Tuesday, March 5, 2019 5:33 AM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> > > > > <achin.gupta@arm.com>; Supreeth Venkatesh
> > > > > <supreeth.venkatesh@arm.com>; Yao, Jiewen
> <jiewen.yao@intel.com>;
> > > > > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> > > > > <jagadeesh.ujja@arm.com>
> > > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> > > architected
> > > > > PI events into MM context
> > > > >
> > > > > PI defines a few architected events that have significance in the MM
> > > > > context as well as in the non-secure DXE context. So register notify
> > > > > handlers for these events, and relay them into the standalone MM
> world.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > ---
> > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |
> 5
> > > +++
> > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c |
> 47
> > > > > +++++++++++++++++++-
> > > > > 2 files changed, 50 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git
> > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > index 88beafa39c05..8bf269270f9d 100644
> > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > +++
> b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > @@ -48,6 +48,11 @@ [LibraryClasses]
> > > > > [Protocols]
> > > > > gEfiMmCommunicationProtocolGuid ##
> PRODUCES
> > > > >
> > > > > +[Guids]
> > > > > + gEfiEndOfDxeEventGroupGuid
> > > > > + gEfiEventExitBootServicesGuid
> > > > > + gEfiEventReadyToBootGuid
> > > > > +
> > > > > [Pcd.common]
> > > > > gArmTokenSpaceGuid.PcdMmBufferBase
> > > > > gArmTokenSpaceGuid.PcdMmBufferSize
> > > > > diff --git
> > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > index feb9fa9f4ead..3203cf801a19 100644
> > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > @@ -265,6 +265,43 @@ GetMmCompatibility ()
> > > > > return Status;
> > > > > }
> > > > >
> > > > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
> > > > > + &gEfiEndOfDxeEventGroupGuid,
> > > > > + &gEfiEventExitBootServicesGuid,
> > > > > + &gEfiEventReadyToBootGuid,
> > > > > +};
> > > > > +
> > > > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE
> (mGuidedEventGuid)];
> > > > > +
> > > > > +/**
> > > > > + Event notification that is fired when GUIDed Event Group is
> signaled.
> > > > > +
> > > > > + @param Event The Event that is being
> > > processed,
> > > > > not used.
> > > > > + @param Context Event Context, not used.
> > > > > +
> > > > > +**/
> > > > > +STATIC
> > > > > +VOID
> > > > > +EFIAPI
> > > > > +MmGuidedEventNotify (
> > > > > + IN EFI_EVENT Event,
> > > > > + IN VOID *Context
> > > > > + )
> > > > > +{
> > > > > + EFI_MM_COMMUNICATE_HEADER Header;
> > > > > + UINTN Size;
> > > > > +
> > > > > + //
> > > > > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER
> structure
> > > > > + //
> > > > > + CopyGuid (&Header.HeaderGuid, Context);
> > > > > + Header.MessageLength = 1;
> > > > > + Header.Data[0] = 0;
> > > > > +
> > > > > + Size = sizeof (Header);
> > > > > + MmCommunicationCommunicate (&mMmCommunication,
> &Header,
> > > > > &Size);
> > > > > +}
> > > > > +
> > > > > /**
> > > > > The Entry Point for MM Communication
> > > > >
> > > > > @@ -287,6 +324,7 @@ MmCommunicationInitialize (
> > > > > )
> > > > > {
> > > > > EFI_STATUS Status;
> > > > > + UINTN Index;
> > > > >
> > > > > // Check if we can make the MM call
> > > > > Status = GetMmCompatibility ();
> > > > > @@ -351,8 +389,13 @@ MmCommunicationInitialize (
> > > > > NULL,
> > > > > &mSetVirtualAddressMapEvent
> > > > > );
> > > > > - if (Status == EFI_SUCCESS) {
> > > > > - return Status;
> > > > > + ASSERT_EFI_ERROR (Status);
> > > > > +
> > > > > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++)
> {
> > > > > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL,
> > > TPL_CALLBACK,
> > > > > + MmGuidedEventNotify,
> > > mGuidedEventGuid[Index],
> > > > > + mGuidedEventGuid[Index],
> > > > > &mGuidedEvent[Index]);
> > > > > + ASSERT_EFI_ERROR (Status);
> > > > > }
> > > > >
> > > > > gBS->UninstallProtocolInterface (
> > > > > --
> > > > > 2.20.1
> > > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Please consider the environment before printing this email.
The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context
2019-03-05 16:53 ` Felix Polyudov
@ 2019-03-05 17:29 ` Ard Biesheuvel
0 siblings, 0 replies; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-05 17:29 UTC (permalink / raw)
To: Felix Polyudov; +Cc: Yao, Jiewen, edk2-devel@lists.01.org
On Tue, 5 Mar 2019 at 17:53, Felix Polyudov <Felixp@ami.com> wrote:
>
> There is an architectural difference between EndOfDxe and SmmReadyToLock events.
> It's important to have both of them.
> Here is what PI specification says:
> ==
> Transition from the environment where all of the components are under the authority of the platform manufacturer to the environment where third party modules are executed is a two-step process:
> 1.End of DXE Event is signaled. This event presents the last opportunity to use resources or interfaces that are going to be locked or disabled in anticipation of the invocation of 3rd party extensible modules.
> 2.DXE SMM Ready to Lock Protocol is installed. PI modules that need to lock or protect their resources in anticipation of the invocation of 3rd party extensible modules should register for notification on installation of this protocol and effect the appropriate protections in their notification handlers
Thanks for pointing that out, Felix. I will add SmmReadyToLock as well.
> ==
>
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao, Jiewen
> Sent: Tuesday, March 05, 2019 11:19 AM
> To: Ard Biesheuvel
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context
>
> For current X86 world, we do use both SmmReadyToLock and EndOfDxe, for almost all X86 platform.
>
> So, let me clarify:
> If we try to align with PI spec, we can add EndOfDxe/ReadyToBoot/ExitBootService.
> If we try to align with security, we can add EndOfDxe/SmmReadyToLock.
>
> It depends upon the what goal we want to achieve. That is why I ask if we have specific use case.
>
> Anyway, I think we can add when it is really needed later.
> So I am OK with current patch.
>
> Thank you
> Yao Jiewen
>
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > Ard Biesheuvel
> > Sent: Tuesday, March 5, 2019 8:07 AM
> > To: Yao, Jiewen <jiewen.yao@intel.com>
> > Cc: edk2-devel@lists.01.org
> > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> > architected PI events into MM context
> >
> > On Tue, 5 Mar 2019 at 17:04, Yao, Jiewen <jiewen.yao@intel.com> wrote:
> > >
> > > OK. To keep the compatibility of existing MM driver. That makes sense.
> > >
> > > If it is for security, I think EndOfDxe is the only point.
> > > ReadyToBoot and ExitBootService cannot be used for security purpose.
> > >
> >
> > OK, good to know. I will keep them for the time being - MM drivers may
> > be able to release resources or do other useful things when the
> > non-secure side enters runtime mode.
> >
> > > Then do we need SmmReadyToLock ? :-)
> > >
> >
> > Good point. It looked fairly x86 specific to me. Do you think it is
> > likely to be used in OEM code running in MM mode?
> >
> >
> >
> >
> > > > -----Original Message-----
> > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf
> > Of
> > > > Ard Biesheuvel
> > > > Sent: Tuesday, March 5, 2019 7:58 AM
> > > > To: Yao, Jiewen <jiewen.yao@intel.com>
> > > > Cc: edk2-devel@lists.01.org
> > > > Subject: Re: [edk2] [PATCH 10/10] ArmPkg/MmCommunicationDxe:
> > signal
> > > > architected PI events into MM context
> > > >
> > > > On Tue, 5 Mar 2019 at 16:56, Yao, Jiewen <jiewen.yao@intel.com>
> > wrote:
> > > > >
> > > > > Hi
> > > > > In original SMM infrastructure, there are lots of interaction that SMM
> > has
> > > > to know the DXE status.
> > > > >
> > > > > In StandaloneMm, I don't expect we have many interaction. Is there
> > any
> > > > specific example?
> > > > >
> > > > > I am totally OK to add those. And I just want to know more usage.
> > > > >
> > > > > Reviewed-by: Jiewen.yao@intel.com
> > > > >
> > > >
> > > > Jiewen,
> > > >
> > > > Thanks for the review.
> > > >
> > > > It is not 100% clear at the moment, but since existing third party
> > > > software designed to run in MM context may make assumptions about
> > > > security of the platform (e.g., before vs after end of dxe) based on
> > > > these events, we should at least signal the common ones added in this
> > > > patch.
> > > >
> > > >
> > > >
> > > >
> > > > > > -----Original Message-----
> > > > > > From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> > > > > > Sent: Tuesday, March 5, 2019 5:33 AM
> > > > > > To: edk2-devel@lists.01.org
> > > > > > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Achin Gupta
> > > > > > <achin.gupta@arm.com>; Supreeth Venkatesh
> > > > > > <supreeth.venkatesh@arm.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>;
> > > > > > Leif Lindholm <leif.lindholm@linaro.org>; Jagadeesh Ujja
> > > > > > <jagadeesh.ujja@arm.com>
> > > > > > Subject: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal
> > > > architected
> > > > > > PI events into MM context
> > > > > >
> > > > > > PI defines a few architected events that have significance in the MM
> > > > > > context as well as in the non-secure DXE context. So register notify
> > > > > > handlers for these events, and relay them into the standalone MM
> > world.
> > > > > >
> > > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > > > > ---
> > > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf |
> > 5
> > > > +++
> > > > > > ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c |
> > 47
> > > > > > +++++++++++++++++++-
> > > > > > 2 files changed, 50 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > diff --git
> > > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > > index 88beafa39c05..8bf269270f9d 100644
> > > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > > +++
> > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> > > > > > @@ -48,6 +48,11 @@ [LibraryClasses]
> > > > > > [Protocols]
> > > > > > gEfiMmCommunicationProtocolGuid ##
> > PRODUCES
> > > > > >
> > > > > > +[Guids]
> > > > > > + gEfiEndOfDxeEventGroupGuid
> > > > > > + gEfiEventExitBootServicesGuid
> > > > > > + gEfiEventReadyToBootGuid
> > > > > > +
> > > > > > [Pcd.common]
> > > > > > gArmTokenSpaceGuid.PcdMmBufferBase
> > > > > > gArmTokenSpaceGuid.PcdMmBufferSize
> > > > > > diff --git
> > > > a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > > b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > > index feb9fa9f4ead..3203cf801a19 100644
> > > > > > --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > > +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> > > > > > @@ -265,6 +265,43 @@ GetMmCompatibility ()
> > > > > > return Status;
> > > > > > }
> > > > > >
> > > > > > +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
> > > > > > + &gEfiEndOfDxeEventGroupGuid,
> > > > > > + &gEfiEventExitBootServicesGuid,
> > > > > > + &gEfiEventReadyToBootGuid,
> > > > > > +};
> > > > > > +
> > > > > > +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE
> > (mGuidedEventGuid)];
> > > > > > +
> > > > > > +/**
> > > > > > + Event notification that is fired when GUIDed Event Group is
> > signaled.
> > > > > > +
> > > > > > + @param Event The Event that is being
> > > > processed,
> > > > > > not used.
> > > > > > + @param Context Event Context, not used.
> > > > > > +
> > > > > > +**/
> > > > > > +STATIC
> > > > > > +VOID
> > > > > > +EFIAPI
> > > > > > +MmGuidedEventNotify (
> > > > > > + IN EFI_EVENT Event,
> > > > > > + IN VOID *Context
> > > > > > + )
> > > > > > +{
> > > > > > + EFI_MM_COMMUNICATE_HEADER Header;
> > > > > > + UINTN Size;
> > > > > > +
> > > > > > + //
> > > > > > + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER
> > structure
> > > > > > + //
> > > > > > + CopyGuid (&Header.HeaderGuid, Context);
> > > > > > + Header.MessageLength = 1;
> > > > > > + Header.Data[0] = 0;
> > > > > > +
> > > > > > + Size = sizeof (Header);
> > > > > > + MmCommunicationCommunicate (&mMmCommunication,
> > &Header,
> > > > > > &Size);
> > > > > > +}
> > > > > > +
> > > > > > /**
> > > > > > The Entry Point for MM Communication
> > > > > >
> > > > > > @@ -287,6 +324,7 @@ MmCommunicationInitialize (
> > > > > > )
> > > > > > {
> > > > > > EFI_STATUS Status;
> > > > > > + UINTN Index;
> > > > > >
> > > > > > // Check if we can make the MM call
> > > > > > Status = GetMmCompatibility ();
> > > > > > @@ -351,8 +389,13 @@ MmCommunicationInitialize (
> > > > > > NULL,
> > > > > > &mSetVirtualAddressMapEvent
> > > > > > );
> > > > > > - if (Status == EFI_SUCCESS) {
> > > > > > - return Status;
> > > > > > + ASSERT_EFI_ERROR (Status);
> > > > > > +
> > > > > > + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++)
> > {
> > > > > > + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL,
> > > > TPL_CALLBACK,
> > > > > > + MmGuidedEventNotify,
> > > > mGuidedEventGuid[Index],
> > > > > > + mGuidedEventGuid[Index],
> > > > > > &mGuidedEvent[Index]);
> > > > > > + ASSERT_EFI_ERROR (Status);
> > > > > > }
> > > > > >
> > > > > > gBS->UninstallProtocolInterface (
> > > > > > --
> > > > > > 2.20.1
> > > > >
> > > > _______________________________________________
> > > > edk2-devel mailing list
> > > > edk2-devel@lists.01.org
> > > > https://lists.01.org/mailman/listinfo/edk2-devel
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
>
> Please consider the environment before printing this email.
>
> The information contained in this message may be confidential and proprietary to American Megatrends, Inc. This communication is intended to be read only by the individual or entity to whom it is addressed or by their designee. If the reader of this message is not the intended recipient, you are on notice that any distribution of this message, in any form, is strictly prohibited. Please promptly notify the sender by reply e-mail or by telephone at 770-246-8600, and then delete or destroy all copies of the transmission.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable
2019-03-05 13:32 ` [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable Ard Biesheuvel
2019-03-05 13:55 ` Yao, Jiewen
@ 2019-03-06 15:16 ` Achin Gupta
2019-03-06 15:17 ` Ard Biesheuvel
1 sibling, 1 reply; 43+ messages in thread
From: Achin Gupta @ 2019-03-06 15:16 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
Hi Ard,
On Tue, Mar 05, 2019 at 02:32:40PM +0100, Ard Biesheuvel wrote:
> The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the
> first place since the value is implied by the context (it is never
> valid to set it to FALSE for standalone MM or TRUE for traditional
> MM). So drop it.
This is being used to determine if the ArmVExpressPkg should include support for
StMM comm. buffer or not [1] but it does look redundant now.
cheers,
Achin
[1] https://lists.01.org/pipermail/edk2-devel/2018-December/034432.html
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/StandaloneMmPkg.dec | 3 ---
> StandaloneMmPkg/StandaloneMmPkg.dsc | 3 ---
> StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf | 3 ---
> 3 files changed, 9 deletions(-)
>
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dec b/StandaloneMmPkg/StandaloneMmPkg.dec
> index 0b5fbf9e1767..2d178c5e20a6 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dec
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dec
> @@ -39,6 +39,3 @@ [Guids]
> gEfiStandaloneMmNonSecureBufferGuid = { 0xf00497e3, 0xbfa2, 0x41a1, { 0x9d, 0x29, 0x54, 0xc2, 0xe9, 0x37, 0x21, 0xc5 }}
> gEfiArmTfCpuDriverEpDescriptorGuid = { 0x6ecbd5a1, 0xc0f8, 0x4702, { 0x83, 0x01, 0x4f, 0xc2, 0xc5, 0x47, 0x0a, 0x51 }}
>
> -[PcdsFeatureFlag]
> - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|FALSE|BOOLEAN|0x00000001
> -
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc b/StandaloneMmPkg/StandaloneMmPkg.dsc
> index e8d71ad56f52..f279d9e7e5c7 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dsc
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
> @@ -78,9 +78,6 @@ [LibraryClasses.AARCH64]
> # Pcd Section - list of all EDK II PCD Entries defined by this Platform
> #
> ################################################################################
> -[PcdsFeatureFlag]
> - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable|TRUE
> -
> [PcdsFixedAtBuild]
> gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x800000CF
> gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff
> diff --git a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
> index eef3d7c6e253..181c15b9345a 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
> +++ b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
> @@ -37,9 +37,6 @@ [Packages]
> MdePkg/MdePkg.dec
> StandaloneMmPkg/StandaloneMmPkg.dec
>
> -[FeaturePcd]
> - gStandaloneMmPkgTokenSpaceGuid.PcdStandaloneMmEnable
> -
> [LibraryClasses]
> StandaloneMmMmuLib
> PcdLib
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable
2019-03-06 15:16 ` Achin Gupta
@ 2019-03-06 15:17 ` Ard Biesheuvel
2019-03-06 15:37 ` Achin Gupta
0 siblings, 1 reply; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-06 15:17 UTC (permalink / raw)
To: Achin Gupta
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
On Wed, 6 Mar 2019 at 16:16, Achin Gupta <Achin.Gupta@arm.com> wrote:
>
> Hi Ard,
>
> On Tue, Mar 05, 2019 at 02:32:40PM +0100, Ard Biesheuvel wrote:
> > The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the
> > first place since the value is implied by the context (it is never
> > valid to set it to FALSE for standalone MM or TRUE for traditional
> > MM). So drop it.
>
> This is being used to determine if the ArmVExpressPkg should include support for
> StMM comm. buffer or not [1] but it does look redundant now.
>
If that is the case, the PCD should be defined in that package.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable
2019-03-06 15:17 ` Ard Biesheuvel
@ 2019-03-06 15:37 ` Achin Gupta
2019-03-07 10:09 ` Ard Biesheuvel
0 siblings, 1 reply; 43+ messages in thread
From: Achin Gupta @ 2019-03-06 15:37 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
On Wed, Mar 06, 2019 at 04:17:51PM +0100, Ard Biesheuvel wrote:
> On Wed, 6 Mar 2019 at 16:16, Achin Gupta <Achin.Gupta@arm.com> wrote:
> >
> > Hi Ard,
> >
> > On Tue, Mar 05, 2019 at 02:32:40PM +0100, Ard Biesheuvel wrote:
> > > The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the
> > > first place since the value is implied by the context (it is never
> > > valid to set it to FALSE for standalone MM or TRUE for traditional
> > > MM). So drop it.
> >
> > This is being used to determine if the ArmVExpressPkg should include support for
> > StMM comm. buffer or not [1] but it does look redundant now.
> >
>
> If that is the case, the PCD should be defined in that package.
The Arm FVP port for StMM needs a rewrite on the lines of other platforms. This
change is fine.
Reviewed-by: achin.gupta@arm.com
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution
2019-03-05 13:32 ` [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution Ard Biesheuvel
2019-03-05 14:22 ` Yao, Jiewen
@ 2019-03-06 15:38 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Achin Gupta @ 2019-03-06 15:38 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
Reviewed-by: achin.gupta@arm.com
On Tue, Mar 05, 2019 at 02:32:41PM +0100, Ard Biesheuvel wrote:
> Building StandaloneMmPkg from its .DSC is mainly intended for build
> coverage, and so platform specific configuration such as UART addresses
> don't belong here.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/StandaloneMmPkg.dsc | 11 +----------
> 1 file changed, 1 insertion(+), 10 deletions(-)
>
> diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc b/StandaloneMmPkg/StandaloneMmPkg.dsc
> index f279d9e7e5c7..8def9688fe7d 100644
> --- a/StandaloneMmPkg/StandaloneMmPkg.dsc
> +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc
> @@ -43,7 +43,7 @@ [LibraryClasses]
> #
> BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
> - DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> + DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
> FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
> HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf
> @@ -66,10 +66,6 @@ [LibraryClasses.AARCH64]
> ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
> CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
> PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
> - PL011UartLib|ArmPlatformPkg/Library/PL011UartLib/PL011UartLib.inf
> - PL011UartClockLib|ArmPlatformPkg/Library/PL011UartClockLib/PL011UartClockLib.inf
> - # ARM PL011 UART Driver
> - SerialPortLib|ArmPlatformPkg/Library/PL011SerialPortLib/PL011SerialPortLib.inf
>
> StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
>
> @@ -83,11 +79,6 @@ [PcdsFixedAtBuild]
> gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xff
> gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x0f
>
> -[PcdsFixedAtBuild.AARCH64]
> - ## PL011 - Serial Terminal
> - gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x1c0b0000
> - gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
> -
> ###################################################################################################
> #
> # Components Section - list of the modules and components that will be processed by compilation
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call
2019-03-05 13:32 ` [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call Ard Biesheuvel
2019-03-05 13:52 ` Yao, Jiewen
@ 2019-03-06 16:35 ` Achin Gupta
2019-03-06 16:41 ` Ard Biesheuvel
1 sibling, 1 reply; 43+ messages in thread
From: Achin Gupta @ 2019-03-06 16:35 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
Hi Ard,
On Tue, Mar 05, 2019 at 02:32:43PM +0100, Ard Biesheuvel wrote:
> Sending DEBUG output to the serial port should only be done via
> DebugLib calls, which is in charge of initializing the serial
> port when appropriate. So drop the explicit SerialPortInitialize ()
> invocation, and rely on normal constructor ordering to get the
> serial port into the appropriate state at the right time.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> index 5cca532456fd..c8e11a253d24 100644
> --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> @@ -232,9 +232,6 @@ _ModuleEntryPoint (
> VOID *TeData;
> UINTN TeDataSize;
>
> - Status = SerialPortInitialize ();
> - ASSERT_EFI_ERROR (Status);
This is done in the first few instructions after EL3 ERETs into S-EL0 to
initialise the StMM partition. The constructors will be called a bit later. I
agree with the change but does EDK2 provide a mechanism for early prints to the
console in case we need this in future.
cheers,
Achin
> -
> // Get Secure Partition Manager Version Information
> Status = GetSpmVersion ();
> if (EFI_ERROR (Status)) {
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call
2019-03-06 16:35 ` Achin Gupta
@ 2019-03-06 16:41 ` Ard Biesheuvel
2019-03-06 16:55 ` Achin Gupta
0 siblings, 1 reply; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-06 16:41 UTC (permalink / raw)
To: Achin Gupta
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
On Wed, 6 Mar 2019 at 17:35, Achin Gupta <Achin.Gupta@arm.com> wrote:
>
> Hi Ard,
>
> On Tue, Mar 05, 2019 at 02:32:43PM +0100, Ard Biesheuvel wrote:
> > Sending DEBUG output to the serial port should only be done via
> > DebugLib calls, which is in charge of initializing the serial
> > port when appropriate. So drop the explicit SerialPortInitialize ()
> > invocation, and rely on normal constructor ordering to get the
> > serial port into the appropriate state at the right time.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> > StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c | 3 ---
> > 1 file changed, 3 deletions(-)
> >
> > diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > index 5cca532456fd..c8e11a253d24 100644
> > --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > @@ -232,9 +232,6 @@ _ModuleEntryPoint (
> > VOID *TeData;
> > UINTN TeDataSize;
> >
> > - Status = SerialPortInitialize ();
> > - ASSERT_EFI_ERROR (Status);
>
> This is done in the first few instructions after EL3 ERETs into S-EL0 to
> initialise the StMM partition. The constructors will be called a bit later. I
> agree with the change but does EDK2 provide a mechanism for early prints to the
> console in case we need this in future.
>
If so, the correct way to achieve this would be to call the DebugLib
constructor by hand, and that should call the SerialPortLib
constructor. Unfortunately, EDK2 is not put together like that, and
especially constructor ordering is slightly broken.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call
2019-03-06 16:41 ` Ard Biesheuvel
@ 2019-03-06 16:55 ` Achin Gupta
0 siblings, 0 replies; 43+ messages in thread
From: Achin Gupta @ 2019-03-06 16:55 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
On Wed, Mar 06, 2019 at 05:41:30PM +0100, Ard Biesheuvel wrote:
> On Wed, 6 Mar 2019 at 17:35, Achin Gupta <Achin.Gupta@arm.com> wrote:
> >
> > Hi Ard,
> >
> > On Tue, Mar 05, 2019 at 02:32:43PM +0100, Ard Biesheuvel wrote:
> > > Sending DEBUG output to the serial port should only be done via
> > > DebugLib calls, which is in charge of initializing the serial
> > > port when appropriate. So drop the explicit SerialPortInitialize ()
> > > invocation, and rely on normal constructor ordering to get the
> > > serial port into the appropriate state at the right time.
> > >
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > ---
> > > StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c | 3 ---
> > > 1 file changed, 3 deletions(-)
> > >
> > > diff --git a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > > index 5cca532456fd..c8e11a253d24 100644
> > > --- a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > > +++ b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
> > > @@ -232,9 +232,6 @@ _ModuleEntryPoint (
> > > VOID *TeData;
> > > UINTN TeDataSize;
> > >
> > > - Status = SerialPortInitialize ();
> > > - ASSERT_EFI_ERROR (Status);
> >
> > This is done in the first few instructions after EL3 ERETs into S-EL0 to
> > initialise the StMM partition. The constructors will be called a bit later. I
> > agree with the change but does EDK2 provide a mechanism for early prints to the
> > console in case we need this in future.
> >
>
> If so, the correct way to achieve this would be to call the DebugLib
> constructor by hand, and that should call the SerialPortLib
> constructor. Unfortunately, EDK2 is not put together like that, and
> especially constructor ordering is slightly broken.
Thanks!
Reviewed-by: achin.gupta@arm.com
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes
2019-03-05 13:32 ` [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes Ard Biesheuvel
2019-03-05 15:50 ` Yao, Jiewen
@ 2019-03-06 16:56 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Achin Gupta @ 2019-03-06 16:56 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
Reviewed-by: achin.gupta@arm.com
On Tue, Mar 05, 2019 at 02:32:44PM +0100, Ard Biesheuvel wrote:
> Standalone MM requires 4 KB section alignment for all images, so that
> strict permissions can be applied. Unfortunately, this results in a
> lot of wasted space, which is usually costly in the secure world
> environment that standalone MM is expected to operate in.
>
> So let's permit the standalone MM drivers (but not the core) to be
> delivered in a compressed firmware volume.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/Core/StandaloneMmCore.inf | 1 +
> StandaloneMmPkg/Core/FwVol.c | 99 ++++++++++++++++++--
> 2 files changed, 91 insertions(+), 9 deletions(-)
>
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> index ff2b8b9cef03..83d31e2d92c5 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
> @@ -49,6 +49,7 @@ [LibraryClasses]
> BaseMemoryLib
> CacheMaintenanceLib
> DebugLib
> + ExtractGuidedSectionLib
> FvLib
> HobLib
> MemoryAllocationLib
> diff --git a/StandaloneMmPkg/Core/FwVol.c b/StandaloneMmPkg/Core/FwVol.c
> index 5abf98c24797..d95491f252f9 100644
> --- a/StandaloneMmPkg/Core/FwVol.c
> +++ b/StandaloneMmPkg/Core/FwVol.c
> @@ -14,6 +14,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
>
> #include "StandaloneMmCore.h"
> #include <Library/FvLib.h>
> +#include <Library/ExtractGuidedSectionLib.h>
>
> //
> // List of file types supported by dispatcher
> @@ -65,15 +66,25 @@ Returns:
>
> --*/
> {
> - EFI_STATUS Status;
> - EFI_STATUS DepexStatus;
> - EFI_FFS_FILE_HEADER *FileHeader;
> - EFI_FV_FILETYPE FileType;
> - VOID *Pe32Data;
> - UINTN Pe32DataSize;
> - VOID *Depex;
> - UINTN DepexSize;
> - UINTN Index;
> + EFI_STATUS Status;
> + EFI_STATUS DepexStatus;
> + EFI_FFS_FILE_HEADER *FileHeader;
> + EFI_FV_FILETYPE FileType;
> + VOID *Pe32Data;
> + UINTN Pe32DataSize;
> + VOID *Depex;
> + UINTN DepexSize;
> + UINTN Index;
> + EFI_COMMON_SECTION_HEADER *Section;
> + VOID *SectionData;
> + UINTN SectionDataSize;
> + UINT32 DstBufferSize;
> + VOID *ScratchBuffer;
> + UINT32 ScratchBufferSize;
> + VOID *DstBuffer;
> + UINT16 SectionAttribute;
> + UINT32 AuthenticationStatus;
> + EFI_FIRMWARE_VOLUME_HEADER *InnerFvHeader;
>
> DEBUG ((DEBUG_INFO, "MmCoreFfsFindMmDriver - 0x%x\n", FwVolHeader));
>
> @@ -83,6 +94,71 @@ Returns:
>
> FvIsBeingProcesssed (FwVolHeader);
>
> + //
> + // First check for encapsulated compressed firmware volumes
> + //
> + FileHeader = NULL;
> + do {
> + Status = FfsFindNextFile (EFI_FV_FILETYPE_FIRMWARE_VOLUME_IMAGE,
> + FwVolHeader, &FileHeader);
> + if (EFI_ERROR (Status)) {
> + break;
> + }
> + Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED, FileHeader,
> + &SectionData, &SectionDataSize);
> + if (EFI_ERROR (Status)) {
> + break;
> + }
> + Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
> + Status = ExtractGuidedSectionGetInfo (Section, &DstBufferSize,
> + &ScratchBufferSize, &SectionAttribute);
> + if (EFI_ERROR (Status)) {
> + break;
> + }
> +
> + //
> + // Allocate scratch buffer
> + //
> + ScratchBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES (ScratchBufferSize));
> + if (ScratchBuffer == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + //
> + // Allocate destination buffer, extra one page for adjustment
> + //
> + DstBuffer = (VOID *)(UINTN)AllocatePages (EFI_SIZE_TO_PAGES (DstBufferSize));
> + if (DstBuffer == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + //
> + // Call decompress function
> + //
> + Status = ExtractGuidedSectionDecode (Section, &DstBuffer, ScratchBuffer,
> + &AuthenticationStatus);
> + FreePages (ScratchBuffer, EFI_SIZE_TO_PAGES (ScratchBufferSize));
> + if (EFI_ERROR (Status)) {
> + goto FreeDstBuffer;
> + }
> +
> + DEBUG ((DEBUG_INFO,
> + "Processing compressed firmware volume (AuthenticationStatus == %x)\n",
> + AuthenticationStatus));
> +
> + Status = FindFfsSectionInSections (DstBuffer, DstBufferSize,
> + EFI_SECTION_FIRMWARE_VOLUME_IMAGE, &Section);
> + if (EFI_ERROR (Status)) {
> + goto FreeDstBuffer;
> + }
> +
> + InnerFvHeader = (VOID *)(Section + 1);
> + Status = MmCoreFfsFindMmDriver (InnerFvHeader);
> + if (EFI_ERROR (Status)) {
> + goto FreeDstBuffer;
> + }
> + } while (TRUE);
> +
> for (Index = 0; Index < sizeof (mMmFileTypes) / sizeof (mMmFileTypes[0]); Index++) {
> DEBUG ((DEBUG_INFO, "Check MmFileTypes - 0x%x\n", mMmFileTypes[Index]));
> FileType = mMmFileTypes[Index];
> @@ -100,5 +176,10 @@ Returns:
> } while (!EFI_ERROR (Status));
> }
>
> + return EFI_SUCCESS;
> +
> +FreeDstBuffer:
> + FreePages (DstBuffer, EFI_SIZE_TO_PAGES (DstBufferSize));
> +
> return Status;
> }
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time
2019-03-05 13:32 ` [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time Ard Biesheuvel
2019-03-05 15:51 ` Yao, Jiewen
@ 2019-03-06 16:56 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Achin Gupta @ 2019-03-06 16:56 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
Reviewed-by: achin.gupta@arm.com
On Tue, Mar 05, 2019 at 02:32:45PM +0100, Ard Biesheuvel wrote:
> Instead of deferring dispatch of the remaining MM drivers once the
> CPU driver has been dispatched, proceed and dispatch all drivers.
> This makes sense for standalone MM, since all dispatchable drivers
> should be present in the initial firmware volume anyway: dispatch
> of additional FVs originating in the non-secure side is not supported.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/Core/Dispatcher.c | 92 --------------------
> StandaloneMmPkg/Core/StandaloneMmCore.c | 1 -
> 2 files changed, 93 deletions(-)
>
> diff --git a/StandaloneMmPkg/Core/Dispatcher.c b/StandaloneMmPkg/Core/Dispatcher.c
> index 8a2ad5118d92..bede4832cfb7 100644
> --- a/StandaloneMmPkg/Core/Dispatcher.c
> +++ b/StandaloneMmPkg/Core/Dispatcher.c
> @@ -575,7 +575,6 @@ MmDispatcher (
> LIST_ENTRY *Link;
> EFI_MM_DRIVER_ENTRY *DriverEntry;
> BOOLEAN ReadyToRun;
> - BOOLEAN PreviousMmEntryPointRegistered;
>
> DEBUG ((DEBUG_INFO, "MmDispatcher\n"));
>
> @@ -639,11 +638,6 @@ MmDispatcher (
> DriverEntry->Initialized = TRUE;
> RemoveEntryList (&DriverEntry->ScheduledLink);
>
> - //
> - // Cache state of MmEntryPointRegistered before calling entry point
> - //
> - PreviousMmEntryPointRegistered = gMmCorePrivate->MmEntryPointRegistered;
> -
> //
> // For each MM driver, pass NULL as ImageHandle
> //
> @@ -661,20 +655,6 @@ MmDispatcher (
> DEBUG ((DEBUG_INFO, "StartImage Status - %r\n", Status));
> MmFreePages(DriverEntry->ImageBuffer, DriverEntry->NumberOfPage);
> }
> -
> - if (!PreviousMmEntryPointRegistered && gMmCorePrivate->MmEntryPointRegistered) {
> - //
> - // Return immediately if the MM Entry Point was registered by the MM
> - // Driver that was just dispatched. The MM IPL will reinvoke the MM
> - // Core Dispatcher. This is required so MM Mode may be enabled as soon
> - // as all the dependent MM Drivers for MM Mode have been dispatched.
> - // Once the MM Entry Point has been registered, then MM Mode will be
> - // used.
> - //
> - gRequestDispatch = TRUE;
> - gDispatcherRunning = FALSE;
> - return EFI_NOT_READY;
> - }
> }
>
> //
> @@ -903,78 +883,6 @@ MmAddToDriverList (
> return EFI_SUCCESS;
> }
>
> -/**
> - This function is the main entry point for an MM handler dispatch
> - or communicate-based callback.
> -
> - Event notification that is fired every time a FV dispatch protocol is added.
> - More than one protocol may have been added when this event is fired, so you
> - must loop on MmLocateHandle () to see how many protocols were added and
> - do the following to each FV:
> - If the Fv has already been processed, skip it. If the Fv has not been
> - processed then mark it as being processed, as we are about to process it.
> - Read the Fv and add any driver in the Fv to the mDiscoveredList.The
> - mDiscoveredList is never free'ed and contains variables that define
> - the other states the MM driver transitions to..
> - While you are at it read the A Priori file into memory.
> - Place drivers in the A Priori list onto the mScheduledQueue.
> -
> - @param DispatchHandle The unique handle assigned to this handler by SmiHandlerRegister().
> - @param Context Points to an optional handler context which was specified when the handler was registered.
> - @param CommBuffer A pointer to a collection of data in memory that will
> - be conveyed from a non-MM environment into an MM environment.
> - @param CommBufferSize The size of the CommBuffer.
> -
> - @return Status Code
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmDriverDispatchHandler (
> - IN EFI_HANDLE DispatchHandle,
> - IN CONST VOID *Context, OPTIONAL
> - IN OUT VOID *CommBuffer, OPTIONAL
> - IN OUT UINTN *CommBufferSize OPTIONAL
> - )
> -{
> - EFI_STATUS Status;
> -
> - DEBUG ((DEBUG_INFO, "MmDriverDispatchHandler\n"));
> -
> - //
> - // Execute the MM Dispatcher on any newly discovered FVs and previously
> - // discovered MM drivers that have been discovered but not dispatched.
> - //
> - Status = MmDispatcher ();
> -
> - //
> - // Check to see if CommBuffer and CommBufferSize are valid
> - //
> - if (CommBuffer != NULL && CommBufferSize != NULL) {
> - if (*CommBufferSize > 0) {
> - if (Status == EFI_NOT_READY) {
> - //
> - // If a the MM Core Entry Point was just registered, then set flag to
> - // request the MM Dispatcher to be restarted.
> - //
> - *(UINT8 *)CommBuffer = COMM_BUFFER_MM_DISPATCH_RESTART;
> - } else if (!EFI_ERROR (Status)) {
> - //
> - // Set the flag to show that the MM Dispatcher executed without errors
> - //
> - *(UINT8 *)CommBuffer = COMM_BUFFER_MM_DISPATCH_SUCCESS;
> - } else {
> - //
> - // Set the flag to show that the MM Dispatcher encountered an error
> - //
> - *(UINT8 *)CommBuffer = COMM_BUFFER_MM_DISPATCH_ERROR;
> - }
> - }
> - }
> -
> - return EFI_SUCCESS;
> -}
> -
> /**
> This function is the main entry point for an MM handler dispatch
> or communicate-based callback.
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c b/StandaloneMmPkg/Core/StandaloneMmCore.c
> index 74432320bfc7..ec53b8d8bec4 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.c
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
> @@ -100,7 +100,6 @@ BOOLEAN mInLegacyBoot = FALSE;
> //
> MM_CORE_MMI_HANDLERS mMmCoreMmiHandlers[] = {
> { MmFvDispatchHandler, &gMmFvDispatchGuid, NULL, TRUE },
> - { MmDriverDispatchHandler, &gEfiEventDxeDispatchGuid, NULL, TRUE },
> { MmReadyToLockHandler, &gEfiDxeMmReadyToLockProtocolGuid, NULL, TRUE },
> { MmEndOfDxeHandler, &gEfiEndOfDxeEventGroupGuid, NULL, FALSE },
> { MmLegacyBootHandler, &gEfiEventLegacyBootGuid, NULL, FALSE },
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM
2019-03-05 13:32 ` [PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM Ard Biesheuvel
2019-03-05 15:51 ` Yao, Jiewen
@ 2019-03-06 16:58 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Achin Gupta @ 2019-03-06 16:58 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
Reviewed-by: achin.gupta@arm.com
On Tue, Mar 05, 2019 at 02:32:46PM +0100, Ard Biesheuvel wrote:
> Remove the support that permits calls into the MM context to dispatch
> firmware volumes that are not part of the initial standalone MM firmware
> volume.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/Core/StandaloneMmCore.h | 22 ----------
> StandaloneMmPkg/Core/Dispatcher.c | 46 --------------------
> StandaloneMmPkg/Core/StandaloneMmCore.c | 1 -
> 3 files changed, 69 deletions(-)
>
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h b/StandaloneMmPkg/Core/StandaloneMmCore.h
> index 0d20bcaa6be5..74338dc9da0d 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.h
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
> @@ -635,28 +635,6 @@ MmDriverDispatchHandler (
>
> @return Status Code
>
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmFvDispatchHandler (
> - IN EFI_HANDLE DispatchHandle,
> - IN CONST VOID *Context, OPTIONAL
> - IN OUT VOID *CommBuffer, OPTIONAL
> - IN OUT UINTN *CommBufferSize OPTIONAL
> - );
> -
> -/**
> - This function is the main entry point for an MM handler dispatch
> - or communicate-based callback.
> -
> - @param DispatchHandle The unique handle assigned to this handler by MmiHandlerRegister().
> - @param Context Points to an optional handler context which was specified when the handler was registered.
> - @param CommBuffer A pointer to a collection of data in memory that will
> - be conveyed from a non-MM environment into an MM environment.
> - @param CommBufferSize The size of the CommBuffer.
> -
> - @return Status Code
> -
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/StandaloneMmPkg/Core/Dispatcher.c b/StandaloneMmPkg/Core/Dispatcher.c
> index bede4832cfb7..4b2f38f700a0 100644
> --- a/StandaloneMmPkg/Core/Dispatcher.c
> +++ b/StandaloneMmPkg/Core/Dispatcher.c
> @@ -883,52 +883,6 @@ MmAddToDriverList (
> return EFI_SUCCESS;
> }
>
> -/**
> - This function is the main entry point for an MM handler dispatch
> - or communicate-based callback.
> -
> - @param DispatchHandle The unique handle assigned to this handler by SmiHandlerRegister().
> - @param Context Points to an optional handler context which was specified when the handler was registered.
> - @param CommBuffer A pointer to a collection of data in memory that will
> - be conveyed from a non-MM environment into an MM environment.
> - @param CommBufferSize The size of the CommBuffer.
> -
> - @return Status Code
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmFvDispatchHandler (
> - IN EFI_HANDLE DispatchHandle,
> - IN CONST VOID *Context, OPTIONAL
> - IN OUT VOID *CommBuffer, OPTIONAL
> - IN OUT UINTN *CommBufferSize OPTIONAL
> - )
> -{
> - EFI_STATUS Status;
> - EFI_MM_COMMUNICATE_FV_DISPATCH_DATA *CommunicationFvDispatchData;
> - EFI_FIRMWARE_VOLUME_HEADER *FwVolHeader;
> -
> - DEBUG ((DEBUG_INFO, "MmFvDispatchHandler\n"));
> -
> - CommunicationFvDispatchData = CommBuffer;
> -
> - DEBUG ((DEBUG_INFO, " Dispatch - 0x%016lx - 0x%016lx\n", CommunicationFvDispatchData->Address,
> - CommunicationFvDispatchData->Size));
> -
> - FwVolHeader = (EFI_FIRMWARE_VOLUME_HEADER *)(UINTN)CommunicationFvDispatchData->Address;
> -
> - MmCoreFfsFindMmDriver (FwVolHeader);
> -
> - //
> - // Execute the MM Dispatcher on any newly discovered FVs and previously
> - // discovered MM drivers that have been discovered but not dispatched.
> - //
> - Status = MmDispatcher ();
> -
> - return Status;
> -}
> -
> /**
> Traverse the discovered list for any drivers that were discovered but not loaded
> because the dependency experessions evaluated to false.
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c b/StandaloneMmPkg/Core/StandaloneMmCore.c
> index ec53b8d8bec4..766cdb5c848c 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.c
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
> @@ -99,7 +99,6 @@ BOOLEAN mInLegacyBoot = FALSE;
> // Table of MMI Handlers that are registered by the MM Core when it is initialized
> //
> MM_CORE_MMI_HANDLERS mMmCoreMmiHandlers[] = {
> - { MmFvDispatchHandler, &gMmFvDispatchGuid, NULL, TRUE },
> { MmReadyToLockHandler, &gEfiDxeMmReadyToLockProtocolGuid, NULL, TRUE },
> { MmEndOfDxeHandler, &gEfiEndOfDxeEventGroupGuid, NULL, FALSE },
> { MmLegacyBootHandler, &gEfiEventLegacyBootGuid, NULL, FALSE },
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context
2019-03-05 13:32 ` [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context Ard Biesheuvel
2019-03-05 15:55 ` Yao, Jiewen
@ 2019-03-06 16:58 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Achin Gupta @ 2019-03-06 16:58 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
Reviewed-by: achin.gupta@arm.com
On Tue, Mar 05, 2019 at 02:32:48PM +0100, Ard Biesheuvel wrote:
> PI defines a few architected events that have significance in the MM
> context as well as in the non-secure DXE context. So register notify
> handlers for these events, and relay them into the standalone MM world.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf | 5 +++
> ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c | 47 +++++++++++++++++++-
> 2 files changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> index 88beafa39c05..8bf269270f9d 100644
> --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> @@ -48,6 +48,11 @@ [LibraryClasses]
> [Protocols]
> gEfiMmCommunicationProtocolGuid ## PRODUCES
>
> +[Guids]
> + gEfiEndOfDxeEventGroupGuid
> + gEfiEventExitBootServicesGuid
> + gEfiEventReadyToBootGuid
> +
> [Pcd.common]
> gArmTokenSpaceGuid.PcdMmBufferBase
> gArmTokenSpaceGuid.PcdMmBufferSize
> diff --git a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> index feb9fa9f4ead..3203cf801a19 100644
> --- a/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> +++ b/ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.c
> @@ -265,6 +265,43 @@ GetMmCompatibility ()
> return Status;
> }
>
> +STATIC EFI_GUID* CONST mGuidedEventGuid[] = {
> + &gEfiEndOfDxeEventGroupGuid,
> + &gEfiEventExitBootServicesGuid,
> + &gEfiEventReadyToBootGuid,
> +};
> +
> +STATIC EFI_EVENT mGuidedEvent[ARRAY_SIZE (mGuidedEventGuid)];
> +
> +/**
> + Event notification that is fired when GUIDed Event Group is signaled.
> +
> + @param Event The Event that is being processed, not used.
> + @param Context Event Context, not used.
> +
> +**/
> +STATIC
> +VOID
> +EFIAPI
> +MmGuidedEventNotify (
> + IN EFI_EVENT Event,
> + IN VOID *Context
> + )
> +{
> + EFI_MM_COMMUNICATE_HEADER Header;
> + UINTN Size;
> +
> + //
> + // Use Guid to initialize EFI_SMM_COMMUNICATE_HEADER structure
> + //
> + CopyGuid (&Header.HeaderGuid, Context);
> + Header.MessageLength = 1;
> + Header.Data[0] = 0;
> +
> + Size = sizeof (Header);
> + MmCommunicationCommunicate (&mMmCommunication, &Header, &Size);
> +}
> +
> /**
> The Entry Point for MM Communication
>
> @@ -287,6 +324,7 @@ MmCommunicationInitialize (
> )
> {
> EFI_STATUS Status;
> + UINTN Index;
>
> // Check if we can make the MM call
> Status = GetMmCompatibility ();
> @@ -351,8 +389,13 @@ MmCommunicationInitialize (
> NULL,
> &mSetVirtualAddressMapEvent
> );
> - if (Status == EFI_SUCCESS) {
> - return Status;
> + ASSERT_EFI_ERROR (Status);
> +
> + for (Index = 0; Index < ARRAY_SIZE (mGuidedEventGuid); Index++) {
> + Status = gBS->CreateEventEx (EVT_NOTIFY_SIGNAL, TPL_CALLBACK,
> + MmGuidedEventNotify, mGuidedEventGuid[Index],
> + mGuidedEventGuid[Index], &mGuidedEvent[Index]);
> + ASSERT_EFI_ERROR (Status);
> }
>
> gBS->UninstallProtocolInterface (
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support
2019-03-05 13:32 ` [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support Ard Biesheuvel
2019-03-05 13:52 ` Yao, Jiewen
@ 2019-03-06 16:59 ` Achin Gupta
1 sibling, 0 replies; 43+ messages in thread
From: Achin Gupta @ 2019-03-06 16:59 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
Reviewed-by: achin.gupta@arm.com
On Tue, Mar 05, 2019 at 02:32:47PM +0100, Ard Biesheuvel wrote:
> Remove the support for booting 'legacy' (i.e., non-UEFI boot) OSes.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> StandaloneMmPkg/Core/StandaloneMmCore.h | 22 ----
> StandaloneMmPkg/Core/StandaloneMmCore.c | 124 ++++++--------------
> 2 files changed, 33 insertions(+), 113 deletions(-)
>
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.h b/StandaloneMmPkg/Core/StandaloneMmCore.h
> index 74338dc9da0d..5d336b3dbbf6 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.h
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.h
> @@ -635,28 +635,6 @@ MmDriverDispatchHandler (
>
> @return Status Code
>
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmLegacyBootHandler (
> - IN EFI_HANDLE DispatchHandle,
> - IN CONST VOID *Context, OPTIONAL
> - IN OUT VOID *CommBuffer, OPTIONAL
> - IN OUT UINTN *CommBufferSize OPTIONAL
> - );
> -
> -/**
> - This function is the main entry point for an MM handler dispatch
> - or communicate-based callback.
> -
> - @param DispatchHandle The unique handle assigned to this handler by MmiHandlerRegister().
> - @param Context Points to an optional handler context which was specified when the handler was registered.
> - @param CommBuffer A pointer to a collection of data in memory that will
> - be conveyed from a non-MM environment into an MM environment.
> - @param CommBufferSize The size of the CommBuffer.
> -
> - @return Status Code
> -
> **/
> EFI_STATUS
> EFIAPI
> diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c b/StandaloneMmPkg/Core/StandaloneMmCore.c
> index 766cdb5c848c..fb6b3055e9c6 100644
> --- a/StandaloneMmPkg/Core/StandaloneMmCore.c
> +++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
> @@ -87,21 +87,12 @@ EFI_MM_SYSTEM_TABLE gMmCoreMmst = {
> MmiHandlerUnRegister
> };
>
> -//
> -// Flag to determine if the platform has performed a legacy boot.
> -// If this flag is TRUE, then the runtime code and runtime data associated with the
> -// MM IPL are converted to free memory, so the MM Core must guarantee that is
> -// does not touch of the code/data associated with the MM IPL if this flag is TRUE.
> -//
> -BOOLEAN mInLegacyBoot = FALSE;
> -
> //
> // Table of MMI Handlers that are registered by the MM Core when it is initialized
> //
> MM_CORE_MMI_HANDLERS mMmCoreMmiHandlers[] = {
> { MmReadyToLockHandler, &gEfiDxeMmReadyToLockProtocolGuid, NULL, TRUE },
> { MmEndOfDxeHandler, &gEfiEndOfDxeEventGroupGuid, NULL, FALSE },
> - { MmLegacyBootHandler, &gEfiEventLegacyBootGuid, NULL, FALSE },
> { MmExitBootServiceHandler,&gEfiEventExitBootServicesGuid, NULL, FALSE },
> { MmReadyToBootHandler, &gEfiEventReadyToBootGuid, NULL, FALSE },
> { NULL, NULL, NULL, FALSE },
> @@ -142,47 +133,6 @@ MmEfiNotAvailableYetArg5 (
> return EFI_NOT_AVAILABLE_YET;
> }
>
> -/**
> - Software MMI handler that is called when a Legacy Boot event is signaled. The MM
> - Core uses this signal to know that a Legacy Boot has been performed and that
> - gMmCorePrivate that is shared between the UEFI and MM execution environments can
> - not be accessed from MM anymore since that structure is considered free memory by
> - a legacy OS.
> -
> - @param DispatchHandle The unique handle assigned to this handler by MmiHandlerRegister().
> - @param Context Points to an optional handler context which was specified when the handler was registered.
> - @param CommBuffer A pointer to a collection of data in memory that will
> - be conveyed from a non-MM environment into an MM environment.
> - @param CommBufferSize The size of the CommBuffer.
> -
> - @return Status Code
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -MmLegacyBootHandler (
> - IN EFI_HANDLE DispatchHandle,
> - IN CONST VOID *Context, OPTIONAL
> - IN OUT VOID *CommBuffer, OPTIONAL
> - IN OUT UINTN *CommBufferSize OPTIONAL
> - )
> -{
> - EFI_HANDLE MmHandle;
> - EFI_STATUS Status = EFI_SUCCESS;
> -
> - if (!mInLegacyBoot) {
> - MmHandle = NULL;
> - Status = MmInstallProtocolInterface (
> - &MmHandle,
> - &gEfiEventLegacyBootGuid,
> - EFI_NATIVE_INTERFACE,
> - NULL
> - );
> - }
> - mInLegacyBoot = TRUE;
> - return Status;
> -}
> -
> /**
> Software MMI handler that is called when a ExitBoot Service event is signaled.
>
> @@ -396,7 +346,6 @@ MmEntryPoint (
> {
> EFI_STATUS Status;
> EFI_MM_COMMUNICATE_HEADER *CommunicateHeader;
> - BOOLEAN InLegacyBoot;
>
> DEBUG ((DEBUG_INFO, "MmEntryPoint ...\n"));
>
> @@ -413,44 +362,42 @@ MmEntryPoint (
> //
> // If a legacy boot has occured, then make sure gMmCorePrivate is not accessed
> //
> - InLegacyBoot = mInLegacyBoot;
> - if (!InLegacyBoot) {
> - //
> - // TBD: Mark the InMm flag as TRUE
> - //
> - gMmCorePrivate->InMm = TRUE;
>
> + //
> + // TBD: Mark the InMm flag as TRUE
> + //
> + gMmCorePrivate->InMm = TRUE;
> +
> + //
> + // Check to see if this is a Synchronous MMI sent through the MM Communication
> + // Protocol or an Asynchronous MMI
> + //
> + if (gMmCorePrivate->CommunicationBuffer != 0) {
> //
> - // Check to see if this is a Synchronous MMI sent through the MM Communication
> - // Protocol or an Asynchronous MMI
> + // Synchronous MMI for MM Core or request from Communicate protocol
> //
> - if (gMmCorePrivate->CommunicationBuffer != 0) {
> + if (!MmIsBufferOutsideMmValid ((UINTN)gMmCorePrivate->CommunicationBuffer, gMmCorePrivate->BufferSize)) {
> + //
> + // If CommunicationBuffer is not in valid address scope, return EFI_INVALID_PARAMETER
> + //
> + gMmCorePrivate->CommunicationBuffer = 0;
> + gMmCorePrivate->ReturnStatus = EFI_INVALID_PARAMETER;
> + } else {
> + CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER *)(UINTN)gMmCorePrivate->CommunicationBuffer;
> + gMmCorePrivate->BufferSize -= OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
> + Status = MmiManage (
> + &CommunicateHeader->HeaderGuid,
> + NULL,
> + CommunicateHeader->Data,
> + (UINTN *)&gMmCorePrivate->BufferSize
> + );
> //
> - // Synchronous MMI for MM Core or request from Communicate protocol
> + // Update CommunicationBuffer, BufferSize and ReturnStatus
> + // Communicate service finished, reset the pointer to CommBuffer to NULL
> //
> - if (!MmIsBufferOutsideMmValid ((UINTN)gMmCorePrivate->CommunicationBuffer, gMmCorePrivate->BufferSize)) {
> - //
> - // If CommunicationBuffer is not in valid address scope, return EFI_INVALID_PARAMETER
> - //
> - gMmCorePrivate->CommunicationBuffer = 0;
> - gMmCorePrivate->ReturnStatus = EFI_INVALID_PARAMETER;
> - } else {
> - CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER *)(UINTN)gMmCorePrivate->CommunicationBuffer;
> - gMmCorePrivate->BufferSize -= OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
> - Status = MmiManage (
> - &CommunicateHeader->HeaderGuid,
> - NULL,
> - CommunicateHeader->Data,
> - (UINTN *)&gMmCorePrivate->BufferSize
> - );
> - //
> - // Update CommunicationBuffer, BufferSize and ReturnStatus
> - // Communicate service finished, reset the pointer to CommBuffer to NULL
> - //
> - gMmCorePrivate->BufferSize += OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
> - gMmCorePrivate->CommunicationBuffer = 0;
> - gMmCorePrivate->ReturnStatus = (Status == EFI_SUCCESS) ? EFI_SUCCESS : EFI_NOT_FOUND;
> - }
> + gMmCorePrivate->BufferSize += OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
> + gMmCorePrivate->CommunicationBuffer = 0;
> + gMmCorePrivate->ReturnStatus = (Status == EFI_SUCCESS) ? EFI_SUCCESS : EFI_NOT_FOUND;
> }
> }
>
> @@ -464,14 +411,9 @@ MmEntryPoint (
> //
>
> //
> - // If a legacy boot has occured, then make sure gMmCorePrivate is not accessed
> + // Clear the InMm flag as we are going to leave MM
> //
> - if (!InLegacyBoot) {
> - //
> - // Clear the InMm flag as we are going to leave MM
> - //
> - gMmCorePrivate->InMm = FALSE;
> - }
> + gMmCorePrivate->InMm = FALSE;
>
> DEBUG ((DEBUG_INFO, "MmEntryPoint Done\n"));
> }
> --
> 2.20.1
>
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable
2019-03-06 15:37 ` Achin Gupta
@ 2019-03-07 10:09 ` Ard Biesheuvel
2019-03-07 11:14 ` Achin Gupta
0 siblings, 1 reply; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-07 10:09 UTC (permalink / raw)
To: Achin Gupta
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
On Wed, 6 Mar 2019 at 16:37, Achin Gupta <Achin.Gupta@arm.com> wrote:
>
> On Wed, Mar 06, 2019 at 04:17:51PM +0100, Ard Biesheuvel wrote:
> > On Wed, 6 Mar 2019 at 16:16, Achin Gupta <Achin.Gupta@arm.com> wrote:
> > >
> > > Hi Ard,
> > >
> > > On Tue, Mar 05, 2019 at 02:32:40PM +0100, Ard Biesheuvel wrote:
> > > > The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the
> > > > first place since the value is implied by the context (it is never
> > > > valid to set it to FALSE for standalone MM or TRUE for traditional
> > > > MM). So drop it.
> > >
> > > This is being used to determine if the ArmVExpressPkg should include support for
> > > StMM comm. buffer or not [1] but it does look redundant now.
> > >
> >
> > If that is the case, the PCD should be defined in that package.
>
> The Arm FVP port for StMM needs a rewrite on the lines of other platforms. This
> change is fine.
>
Yes, you are right. SynQuacer also needs some tweaks to align with
these changes, but I will post those separately.
So with those changes merged, the only thing preventing us from
building the SynQuacer + MM platform from upstream sources is the
MmCommunicate VA vs PA issue. Is there any progress on that front?
Thanks,
Ard.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable
2019-03-07 10:09 ` Ard Biesheuvel
@ 2019-03-07 11:14 ` Achin Gupta
0 siblings, 0 replies; 43+ messages in thread
From: Achin Gupta @ 2019-03-07 11:14 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: edk2-devel@lists.01.org, Supreeth Venkatesh, Jiewen Yao,
Leif Lindholm, Jagadeesh Ujja, nd
On Thu, Mar 07, 2019 at 11:09:35AM +0100, Ard Biesheuvel wrote:
> On Wed, 6 Mar 2019 at 16:37, Achin Gupta <Achin.Gupta@arm.com> wrote:
> >
> > On Wed, Mar 06, 2019 at 04:17:51PM +0100, Ard Biesheuvel wrote:
> > > On Wed, 6 Mar 2019 at 16:16, Achin Gupta <Achin.Gupta@arm.com> wrote:
> > > >
> > > > Hi Ard,
> > > >
> > > > On Tue, Mar 05, 2019 at 02:32:40PM +0100, Ard Biesheuvel wrote:
> > > > > The PCD PcdStandaloneMmEnable is unused, and shouldn't exist in the
> > > > > first place since the value is implied by the context (it is never
> > > > > valid to set it to FALSE for standalone MM or TRUE for traditional
> > > > > MM). So drop it.
> > > >
> > > > This is being used to determine if the ArmVExpressPkg should include support for
> > > > StMM comm. buffer or not [1] but it does look redundant now.
> > > >
> > >
> > > If that is the case, the PCD should be defined in that package.
> >
> > The Arm FVP port for StMM needs a rewrite on the lines of other platforms. This
> > change is fine.
> >
>
> Yes, you are right. SynQuacer also needs some tweaks to align with
> these changes, but I will post those separately.
>
> So with those changes merged, the only thing preventing us from
> building the SynQuacer + MM platform from upstream sources is the
> MmCommunicate VA vs PA issue. Is there any progress on that front?
I am looking at this now after my holiday. I have some questions that I will
post separately.
cheers,
Achin
>
> Thanks,
> Ard.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements
2019-03-05 13:32 [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
` (9 preceding siblings ...)
2019-03-05 13:32 ` [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context Ard Biesheuvel
@ 2019-03-11 11:54 ` Ard Biesheuvel
2019-03-11 11:59 ` Ard Biesheuvel
10 siblings, 1 reply; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-11 11:54 UTC (permalink / raw)
To: edk2-devel@lists.01.org
Cc: Achin Gupta, Supreeth Venkatesh, Jiewen Yao, Leif Lindholm,
Jagadeesh Ujja
On Tue, 5 Mar 2019 at 14:32, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> This series is a further cleanup of the StandaloneMmPkg infrastructure
> used to implement UEFI secure boot on ARM systems.
>
> The first 5 patches are simple cleanups.
>
> Patch #6 adds support for dispatching a compressed firmware volume in the
> standalone MM context, so that all drivers except the core can be delivered
> in an encapsulated compressed FV, which saves quite some space.
>
> Patch #7 modifies the driver dispatch logic in the MM context so that the
> dispatcher continues until all drivers are dispatched, rather than waiting
> for a nudge from the non-secure side once the CPU driver has been loaded.
>
> Patch #8 removes support for the FV dispatch MM call.
>
> Patch #9 removes support for legacy boot handling.
>
> Patch #10 implements relaying architected PI events from DXE into MM by
> the MM communicate driver.
>
> Cc: Achin Gupta <achin.gupta@arm.com>
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
>
> Ard Biesheuvel (10):
> StandaloneMmPkg: drop redundant definition of
> gEfiMmConfigurationProtocolGuid
> StandaloneMmPkg: switch to NULL DebugLib resolution
> StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit
> SerialPortLib call
> StandaloneMmPkg/Core: permit encapsulated firmware volumes
> StandaloneMmPkg/Core: dispatch all drivers at init time
> StandaloneMmPkg/Core: drop support for dispatching FVs into MM
> StandaloneMmPkg/Core: remove legacy boot support
Pushed the 7 patches above as 326598e9b759..b2877855c7ec.
> StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable
> StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver
These 2 are ready to go, but are dependent on edk2-platforms patches
that are under review.
> ArmPkg/MmCommunicationDxe: signal architected PI events into MM
> context
This one is still under discussion, since we need to clarify which
events need to be signaled into the MM context.
^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements
2019-03-11 11:54 ` [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
@ 2019-03-11 11:59 ` Ard Biesheuvel
0 siblings, 0 replies; 43+ messages in thread
From: Ard Biesheuvel @ 2019-03-11 11:59 UTC (permalink / raw)
To: edk2-devel@lists.01.org
Cc: Achin Gupta, Supreeth Venkatesh, Jiewen Yao, Leif Lindholm,
Jagadeesh Ujja
On Mon, 11 Mar 2019 at 12:54, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> On Tue, 5 Mar 2019 at 14:32, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >
> > This series is a further cleanup of the StandaloneMmPkg infrastructure
> > used to implement UEFI secure boot on ARM systems.
> >
> > The first 5 patches are simple cleanups.
> >
> > Patch #6 adds support for dispatching a compressed firmware volume in the
> > standalone MM context, so that all drivers except the core can be delivered
> > in an encapsulated compressed FV, which saves quite some space.
> >
> > Patch #7 modifies the driver dispatch logic in the MM context so that the
> > dispatcher continues until all drivers are dispatched, rather than waiting
> > for a nudge from the non-secure side once the CPU driver has been loaded.
> >
> > Patch #8 removes support for the FV dispatch MM call.
> >
> > Patch #9 removes support for legacy boot handling.
> >
> > Patch #10 implements relaying architected PI events from DXE into MM by
> > the MM communicate driver.
> >
> > Cc: Achin Gupta <achin.gupta@arm.com>
> > Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Leif Lindholm <leif.lindholm@linaro.org>
> > Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
> >
> > Ard Biesheuvel (10):
> > StandaloneMmPkg: drop redundant definition of
> > gEfiMmConfigurationProtocolGuid
> > StandaloneMmPkg: switch to NULL DebugLib resolution
> > StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit
> > SerialPortLib call
> > StandaloneMmPkg/Core: permit encapsulated firmware volumes
> > StandaloneMmPkg/Core: dispatch all drivers at init time
> > StandaloneMmPkg/Core: drop support for dispatching FVs into MM
> > StandaloneMmPkg/Core: remove legacy boot support
>
> Pushed the 7 patches above as 326598e9b759..b2877855c7ec.
>
> > StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable
> > StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver
>
> These 2 are ready to go, but are dependent on edk2-platforms patches
> that are under review.
>
Unfortunately, I have already broken the SGI build by pushing
'StandaloneMmPkg/Core: permit encapsulated firmware volumes' above, so
no point in deferring these 2.
Pushed as b2877855c7ec..d6253d2f9a33
> > ArmPkg/MmCommunicationDxe: signal architected PI events into MM
> > context
>
> This one is still under discussion, since we need to clarify which
> events need to be signaled into the MM context.
^ permalink raw reply [flat|nested] 43+ messages in thread
end of thread, other threads:[~2019-03-11 11:59 UTC | newest]
Thread overview: 43+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-05 13:32 [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
2019-03-05 13:32 ` [PATCH 01/10] StandaloneMmPkg: drop redundant definition of gEfiMmConfigurationProtocolGuid Ard Biesheuvel
2019-03-05 13:53 ` Yao, Jiewen
2019-03-05 13:32 ` [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable Ard Biesheuvel
2019-03-05 13:55 ` Yao, Jiewen
2019-03-06 15:16 ` Achin Gupta
2019-03-06 15:17 ` Ard Biesheuvel
2019-03-06 15:37 ` Achin Gupta
2019-03-07 10:09 ` Ard Biesheuvel
2019-03-07 11:14 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution Ard Biesheuvel
2019-03-05 14:22 ` Yao, Jiewen
2019-03-06 15:38 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 04/10] StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver Ard Biesheuvel
2019-03-05 14:22 ` Yao, Jiewen
2019-03-05 13:32 ` [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call Ard Biesheuvel
2019-03-05 13:52 ` Yao, Jiewen
2019-03-06 16:35 ` Achin Gupta
2019-03-06 16:41 ` Ard Biesheuvel
2019-03-06 16:55 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes Ard Biesheuvel
2019-03-05 15:50 ` Yao, Jiewen
2019-03-06 16:56 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time Ard Biesheuvel
2019-03-05 15:51 ` Yao, Jiewen
2019-03-06 16:56 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM Ard Biesheuvel
2019-03-05 15:51 ` Yao, Jiewen
2019-03-06 16:58 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support Ard Biesheuvel
2019-03-05 13:52 ` Yao, Jiewen
2019-03-06 16:59 ` Achin Gupta
2019-03-05 13:32 ` [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context Ard Biesheuvel
2019-03-05 15:55 ` Yao, Jiewen
2019-03-05 15:58 ` Ard Biesheuvel
2019-03-05 16:04 ` Yao, Jiewen
2019-03-05 16:07 ` Ard Biesheuvel
2019-03-05 16:19 ` Yao, Jiewen
2019-03-05 16:53 ` Felix Polyudov
2019-03-05 17:29 ` Ard Biesheuvel
2019-03-06 16:58 ` Achin Gupta
2019-03-11 11:54 ` [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements Ard Biesheuvel
2019-03-11 11:59 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox