[PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements

[PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements

[Ard Biesheuvel]

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

[PATCH 01/10] StandaloneMmPkg: drop redundant definition of gEfiMmConfigurationProtocolGuid

[Ard Biesheuvel]

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

Re: [PATCH 01/10] StandaloneMmPkg: drop redundant definition of gEfiMmConfigurationProtocolGuid

[Yao, Jiewen]

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

[PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable

[Ard Biesheuvel]

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

Re: [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable

[Yao, Jiewen]

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

Re: [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable

[Achin Gupta]

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
>

Re: [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable

[Ard Biesheuvel]

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.

Re: [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable

[Achin Gupta]

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

Re: [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable

[Ard Biesheuvel]

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.

Re: [PATCH 02/10] StandaloneMmPkg: drop unused PCD PcdStandaloneMmEnable

[Achin Gupta]

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.

[PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution

[Ard Biesheuvel]

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

Re: [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution

[Yao, Jiewen]

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

Re: [PATCH 03/10] StandaloneMmPkg: switch to NULL DebugLib resolution

[Achin Gupta]

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
>

[PATCH 04/10] StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver

[Ard Biesheuvel]

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

Re: [PATCH 04/10] StandaloneMmPkg: remove redundant StandaloneMmDriverEntryPoint driver

[Yao, Jiewen]

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

[PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call

[Ard Biesheuvel]

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

Re: [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call

[Yao, Jiewen]

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

Re: [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call

[Achin Gupta]

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
>

Re: [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call

[Ard Biesheuvel]

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.

Re: [PATCH 05/10] StandaloneMmPkg/StandaloneMmCoreEntryPoint: drop explicit SerialPortLib call

[Achin Gupta]

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

[PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes

[Ard Biesheuvel]

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

Re: [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes

[Yao, Jiewen]

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

Re: [PATCH 06/10] StandaloneMmPkg/Core: permit encapsulated firmware volumes

[Achin Gupta]

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
>

[PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time

[Ard Biesheuvel]

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

Re: [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time

[Yao, Jiewen]

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

Re: [PATCH 07/10] StandaloneMmPkg/Core: dispatch all drivers at init time

[Achin Gupta]

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
>

[PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM

[Ard Biesheuvel]

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

Re: [PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM

[Yao, Jiewen]

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

Re: [PATCH 08/10] StandaloneMmPkg/Core: drop support for dispatching FVs into MM

[Achin Gupta]

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
>

[PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support

[Ard Biesheuvel]

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

Re: [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support

[Yao, Jiewen]

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

Re: [PATCH 09/10] StandaloneMmPkg/Core: remove legacy boot support

[Achin Gupta]

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
>

[PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Ard Biesheuvel]

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

Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Yao, Jiewen]

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

Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Ard Biesheuvel]

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
>

Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Yao, Jiewen]

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

Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Ard Biesheuvel]

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

Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Yao, Jiewen]

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

Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Felix Polyudov]

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.

Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Ard Biesheuvel]

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.

Re: [PATCH 10/10] ArmPkg/MmCommunicationDxe: signal architected PI events into MM context

[Achin Gupta]

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
>

Re: [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements

[Ard Biesheuvel]

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.

Re: [PATCH 00/10] StandaloneMmPkg, ArmPkg: cleanups and improvements

[Ard Biesheuvel]

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.