mbox series

[edk2,v2,00/11] StandaloneMmPkg: assorted fixes and improvements

Message ID 20190116202236.6977-1-ard.biesheuvel@linaro.org
Headers show
Series StandaloneMmPkg: assorted fixes and improvements | expand

Message

Ard Biesheuvel Jan. 16, 2019, 8:22 p.m. UTC 
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/StandaloneMmMemoryAllocationLib.c
 create mode 100644 StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf

-- 
2.17.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Achin Gupta Jan. 18, 2019, 9:26 a.m. UTC | #1 
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/StandaloneMmMemoryAllocationLib.c
 create mode 100644 StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf

--
2.17.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Yao, Jiewen Jan. 18, 2019, 3:41 p.m. UTC | #2 
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Jan. 21, 2019, 2:39 p.m. UTC | #3 
On Fri, 18 Jan 2019 at 16:42, Yao, Jiewen <jiewen.yao@intel.com> wrote:
>

> 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?

> >


Thanks all

I pushed most of this series

380148b685b7 StandaloneMmPkg: add HobLib implementation for
MM_STANDALONE modules
66dde0c7516b StandaloneMmPkg: add MM_STANDALONE MemoryAllocationLib
implementation
2cc186178bfd StandaloneMmPkg/StandaloneMmCoreHobLib: restrict to
MM_CORE_STANDALONE
c8102727edb0 StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone
41915a19a772 StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a
modifier for ASCII strings
d2f438bf6a51 StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus
ASSERT_EFI_ERROR()s
77746e70807d StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore
runtime attribute
877013d0a58f StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice
4b28452d9863 StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the
use of TE images

The remaining two patches will be included in the next batch of
StandaloneMmPkg work.
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel