Re: [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and improvements

["Yao, Jiewen" <jiewen.yao@intel.com>]

Thanks to remind me.
I took a look at all patches.

I skipped all ARM specific patches, and reviewed rest of them.
All my feedback is provided in the individual patch.

Thank you
Yao Jiewen

> -----Original Message-----
> From: Achin Gupta [mailto:Achin.Gupta@arm.com]
> Sent: Friday, January 18, 2019 1:27 AM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-devel@lists.01.org;
> Yao, Jiewen <jiewen.yao@intel.com>
> Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com>; Leif Lindholm
> <leif.lindholm@linaro.org>; Jagadeesh Ujja <Jagadeesh.Ujja@arm.com>;
> Thomas Abraham <thomas.abraham@arm.com>; Sami Mujawar
> <Sami.Mujawar@arm.com>; nd <nd@arm.com>
> Subject: Re: [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and
> improvements
> 
> Hi Ard,
> 
> For all the patches...
> 
> Reviewed-by: Achin Gupta <achin.gupta@arm.com>
> 
> Jiewen. There are changes to the generic Standalone MM code in this series.
> Do you want to have a look as well?
> 
> cheers,
> Achin
> ________________________________________
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Sent: 16 January 2019 20:22
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel; Achin Gupta; Jiewen Yao; Supreeth Venkatesh; Leif
> Lindholm; Jagadeesh Ujja; Thomas Abraham; Sami Mujawar
> Subject: [PATCH v2 00/11] StandaloneMmPkg: assorted fixes and
> improvements
> 
> This series addresses a number of issues I ran into while bringing up
> the standalone MM based authenticated variable store on the SynQuacer
> (AArch64) platform.
> 
> Patches #1 - #3 are based on Jagadeesh's patch that imports some staging
> code into StandaloneMmPkg, with the following changes:
> - drop unused source files, GUID references are other unused bit,
> - clean up comments referring to the MM core implementation.
> 
> Patches #4 - #9 are obvious fixes/improvements.
> 
> Patch #10 adds support for TE formatted MM_CORE_STANDALONE binaries.
> This is useful given that the 4 KB section alignment we require in
> AArch64 implementations of standalone MM (due to the strict separation
> between code and date) results in 8 KB of wasted space at the start of
> the firmware volume. This can be reduced to 4 KB when using a TE image
> and the FIXED attribute in the associated [Rule] section, by leveraging
> an existing optimization in the FFS generation code that aligns TE images
> by reducing FFS padding rather than adding more.
> 
> Patch #11 is another space optimization: it reuses the existing support
> for encapsulated compressed firmware volumes in FFS files to shrink the
> size of the primary standalone MM FV considerably. Again, due to
> alignment requirements, there is significant bloat in the uncompressed
> images (4 KB for the PE/COFF header, and up to 4 KB per section for the
> .text, .data and .reloc sections), making the absolute minimum size of
> any trivial MM_STANDALONE module 16 KB.
> 
> Changes since v1:
> - add patches #1 - #3
> - add Supreeth's ack to patches #4 - #7
> 
> Cc: Achin Gupta <achin.gupta@arm.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Supreeth Venkatesh <supreeth.venkatesh@arm.com>
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Cc: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
> Cc: Thomas Panakamattam Abraham <thomas.abraham@arm.com>
> Cc: Sami Mujawar <Sami.Mujawar@arm.com>
> 
> Ard Biesheuvel (11):
>   StandaloneMmPkg: add HobLib implementation for MM_STANDALONE
> modules
>   StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib
> implementation
>   StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to
> MM_CORE_STANDALONE
>   StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone
>   StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing
> SerialPortLib
>     ref
>   StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for
> ASCII
>     strings
>   StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus
>     ASSERT_EFI_ERROR()s
>   StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore runtime
>     attribute
>   StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice
>   StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the use of TE
>     images
>   StandaloneMmPkg/Core: permit encapsulated firmware volumes
> 
>  StandaloneMmPkg/Core/Dispatcher.c             |  30 +-
>  StandaloneMmPkg/Core/FwVol.c                  |  99 ++-
>  StandaloneMmPkg/Core/StandaloneMmCore.inf     |   1 +
>  .../StandaloneMmCpu/AArch64/EventHandle.c     |   2 +-
>  .../StandaloneMmCpu/AArch64/StandaloneMmCpu.c |   6 +-
>  .../StandaloneMmCpu/AArch64/StandaloneMmCpu.h |   8 +-
>  .../AArch64/StandaloneMmCpu.inf               |   4 +-
>  .../AArch64/SetPermissions.c                  | 109 +--
>  .../AArch64/StandaloneMmCoreEntryPoint.c      |   7 +-
>  .../StandaloneMmCoreEntryPoint.inf            |   4 +
>  .../StandaloneMmCoreHobLib.inf                |   2 +-
>  .../StandaloneMmHobLib/StandaloneMmHobLib.c   | 649
> ++++++++++++++
>  .../StandaloneMmHobLib/StandaloneMmHobLib.inf |  45 +
>  .../StandaloneMmMemoryAllocationLib.c         | 822
> ++++++++++++++++++
>  .../StandaloneMmMemoryAllocationLib.inf       |  42 +
>  .../StandaloneMmPeCoffExtraActionLib.c        |   9 +-
>  16 files changed, 1716 insertions(+), 123 deletions(-)
>  create mode 100644
> StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.c
>  create mode 100644
> StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
>  create mode 100644
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalone
> MmMemoryAllocationLib.c
>  create mode 100644
> StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/Standalone
> MmMemoryAllocationLib.inf
> 
> --
> 2.17.1