[PATCH edk2-platforms 0/3] Platform/ARM/SgiPkg: Implement StandaloneMm based secure boot

[PATCH edk2-platforms 0/3] Platform/ARM/SgiPkg: Implement StandaloneMm based secure boot

[Jagadeesh Ujja]

Integrating various pieces together so that the authenticated variable store
runs entirely in standalone MM context residing in a secure partition.
This primarily involves adding all required library and drivers to platform
specific .DSC and .FDF files. This creates separate Nor flash region which
is visible to only StandaoneMm drivers, this Nor Flash will co-exist along
with general Nor flash region.

Jagadeesh Ujja (3):
  Platform/ARM/Sgi: define nor2 flash controller memory map
  Platform/ARM/Sgi: allow MM_STANDALONE modules to use
    NorFlashPlatformLib
  Platform/ARM/SgiPkg: add MM based UEFI secure boot support

 Platform/ARM/SgiPkg/Include/SgiPlatform.h                           |  4 ++
 Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c   | 63 ++++++++++++++++++++
 Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf | 35 +++++++++++
 Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc                        | 33 +++++++++-
 Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf                        |  5 ++
 Platform/ARM/SgiPkg/SgiPlatform.dsc                                 | 18 +++++-
 Platform/ARM/SgiPkg/SgiPlatform.fdf                                 |  9 ++-
 7 files changed, 164 insertions(+), 3 deletions(-)
 create mode 100644 Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c
 create mode 100644 Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf

-- 
2.7.4

In-Reply-To: 

[PATCH edk2-platforms 1/3] Platform/ARM/Sgi: define nor2 flash controller memory map

[Jagadeesh Ujja]

Add the definitions of NOR2 flash controller memory map. The NO2 flash
can be used as an additional non-volatile storage by non-secure code or
used as a non-volatile storage for secure variables by the StandaloneMM.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
---
 Platform/ARM/SgiPkg/Include/SgiPlatform.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
index b9a662a..2a7b79d 100644
--- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
+++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
@@ -27,6 +27,10 @@
 #define SGI_EXP_SMC_CS1_BASE                      0x0C000000
 #define SGI_EXP_SMC_CS1_SZ                        SIZE_64MB
 
+// Expansion AXI - SMC Chip Select 2
+#define SGI_EXP_SMC_CS2_BASE                      0x10000000
+#define SGI_EXP_SMC_CS2_SZ                        SIZE_64MB
+
 // Expansion AXI - SMSC 91C111 (Ethernet)
 #define SGI_EXP_SMSC91X_BASE                      0x18000000
 #define SGI_EXP_SMSC91X_SZ                        SIZE_64MB
-- 
2.7.4

[PATCH edk2-platforms 2/3] Platform/ARM/Sgi: allow MM_STANDALONE modules to use NorFlashPlatformLib

[Jagadeesh Ujja]

“NorFlashPlatformLib” library can be used by MM_STANDALONE drivers as
well. When used in MM mode, the third instance of the NOR flash is used as
the non-volatile storage. This NOR flash instance is partitioned into
two regions - first 4MB space is used for secure boot and next 3MB for
secure variable storage

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
---
 Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c   | 63 ++++++++++++++++++++
 Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf | 35 +++++++++++
 2 files changed, 98 insertions(+)

diff --git a/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c b/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c
new file mode 100644
index 0000000..06e3f97
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c
@@ -0,0 +1,63 @@
+/** @file
+
+  Copyright (c) 2019, ARM Ltd. All rights reserved.
+
+  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/DebugLib.h>
+#include <Library/IoLib.h>
+#include <Library/NorFlashPlatformLib.h>
+#include <SgiPlatform.h>
+
+STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = {
+  {
+    // Secure Boot storage space of 4MB
+    SGI_EXP_SMC_CS2_BASE,
+    SGI_EXP_SMC_CS2_BASE,
+    SIZE_256KB * 16,
+    SIZE_256KB,
+  },
+  {
+    //Secure variable storage space of 1MB*3
+    SGI_EXP_SMC_CS2_BASE,
+    SGI_EXP_SMC_CS2_BASE + SIZE_256KB * 16,
+    SIZE_256KB * 12,
+    SIZE_256KB,
+  },
+};
+
+EFI_STATUS
+NorFlashPlatformInitialization (
+  VOID
+  )
+{
+  UINT64 SysRegFlash;
+
+  SysRegFlash = SGI_EXP_SYSPH_SYSTEM_REGISTERS + SGI_SYSPH_SYS_REG_FLASH;
+  MmioOr32 (SysRegFlash, SGI_SYSPH_SYS_REG_FLASH_RWEN);
+  return EFI_SUCCESS;
+}
+
+EFI_STATUS
+NorFlashPlatformGetDevices (
+  OUT NOR_FLASH_DESCRIPTION   **NorFlashDevices,
+  OUT UINT32                  *Count
+  )
+{
+  if ((NorFlashDevices == NULL) || (Count == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *NorFlashDevices = mNorFlashDevices;
+  *Count = ARRAY_SIZE (mNorFlashDevices);
+  return EFI_SUCCESS;
+}
diff --git a/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf b/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
new file mode 100644
index 0000000..eedfacc
--- /dev/null
+++ b/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
@@ -0,0 +1,35 @@
+#/** @file
+#
+#  Copyright (c) 2019, ARM Ltd. All rights reserved.
+
+#  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                      = NorFlashSgiLib
+  FILE_GUID                      = 2ce22190-b933-4d1e-99ba-8bf1f0768255
+  MODULE_TYPE                    = MM_STANDALONE
+  VERSION_STRING                 = 1.0
+  PI_SPECIFICATION_VERSION       = 0x00010032
+  LIBRARY_CLASS                  = NorFlashPlatformLib
+
+[Sources.common]
+  StandaloneMmNorFlashLib.c
+
+[Packages]
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  MdePkg/MdePkg.dec
+  Platform/ARM/SgiPkg/SgiPlatform.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  IoLib
-- 
2.7.4

[PATCH edk2-platforms 3/3] Platform/ARM/SgiPkg: add MM based UEFI secure boot support

[Jagadeesh Ujja]

This implements support for UEFI secure boot on SGI platforms using
the standalone MM framework. This moves all of the software handling
of the UEFI authenticated variable store into the standalone MM
context residing in a secure partition.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
---
 Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc | 33 +++++++++++++++++++-
 Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf |  5 +++
 Platform/ARM/SgiPkg/SgiPlatform.dsc          | 18 ++++++++++-
 Platform/ARM/SgiPkg/SgiPlatform.fdf          |  9 +++++-
 4 files changed, 62 insertions(+), 3 deletions(-)

diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
index 65dd6ac..889be2f 100644
--- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
+++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
@@ -75,7 +75,17 @@
   HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
   MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
   MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
-
+!if $(MM_SECURE_STORAGE_ENABLE) == TRUE
+  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
+  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+  NorFlashPlatformLib|Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
+  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
+  PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
+  SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
+  TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
+  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
+!endif
 ################################################################################
 #
 # Pcd Section - list of all EDK II PCD Entries defined by this Platform
@@ -93,6 +103,21 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x7FF70000
   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
 
+!if $(MM_SECURE_STORAGE_ENABLE) == TRUE
+  #Secure Storage
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
+  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
+
+  ## NV Storage - 1MB*3 in NOR2 Flash
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x10400000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00100000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x10500000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00100000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x10600000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00100000
+!endif
+
 ###################################################################################################
 #
 # Components Section - list of the modules and components that will be processed by compilation
@@ -118,6 +143,12 @@
   StandaloneMmPkg/Core/StandaloneMmCore.inf
 
 [Components.AARCH64]
+!if $(MM_SECURE_STORAGE_ENABLE) == TRUE
+  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
+  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
+  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
+!endif
+
   StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
 
 ###################################################################################################
diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf b/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
index 810460c..04aa5cd 100644
--- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
+++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
@@ -55,6 +55,11 @@ READ_LOCK_CAP      = TRUE
 READ_LOCK_STATUS   = TRUE
 
   INF StandaloneMmPkg/Core/StandaloneMmCore.inf
+!if $(MM_SECURE_STORAGE_ENABLE) == TRUE
+  INF ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
+  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
+  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
+!endif
   INF StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
 
 ################################################################################
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc
index bdb4ecb..cba2e1f 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.dsc
+++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
@@ -260,7 +260,15 @@
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
   MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
   MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
+    <LibraryClasses>
+      NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
+  }
+  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+!else
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
+!endif
   OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
 
   MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
@@ -268,6 +276,9 @@
   MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
   MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
   MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
+!if $(SECURE_STORAGE_ENABLE) == TRUE
+  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+!else
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
     <LibraryClasses>
       NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
@@ -275,6 +286,7 @@
       BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
   }
   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
+!endif
 
   #
   # ACPI Support
@@ -344,4 +356,8 @@
   #
   MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
 
-  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
+  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf {
+    <LibraryClasses>
+      NULL|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
+  }
+  ArmPkg/Drivers/ApeiDxe/Apei.inf
diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/SgiPlatform.fdf
index 7916a52..a438940 100644
--- a/Platform/ARM/SgiPkg/SgiPlatform.fdf
+++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf
@@ -96,10 +96,17 @@ READ_LOCK_STATUS   = TRUE
   INF EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf
   INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
   INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
-  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
   INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
   INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
+!if $(SECURE_BOOT_ENABLE) == TRUE
+  INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+!endif
+!if $(SECURE_STORAGE_ENABLE) == TRUE
+  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+!else
+  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
   INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+!endif
 
   #
   # ACPI Support
-- 
2.7.4

Re: [PATCH edk2-platforms 2/3] Platform/ARM/Sgi: allow MM_STANDALONE modules to use NorFlashPlatformLib

[Ard Biesheuvel]

On Mon, 4 Mar 2019 at 11:12, Jagadeesh Ujja <jagadeesh.ujja@arm.com> wrote:
>
> “NorFlashPlatformLib” library can be used by MM_STANDALONE drivers as
> well. When used in MM mode, the third instance of the NOR flash is used as
> the non-volatile storage. This NOR flash instance is partitioned into
> two regions - first 4MB space is used for secure boot and next 3MB for
> secure variable storage
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
> ---
>  Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c   | 63 ++++++++++++++++++++
>  Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf | 35 +++++++++++
>  2 files changed, 98 insertions(+)
>
> diff --git a/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c b/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c
> new file mode 100644
> index 0000000..06e3f97
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.c
> @@ -0,0 +1,63 @@
> +/** @file
> +
> +  Copyright (c) 2019, ARM Ltd. All rights reserved.
> +
> +  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/DebugLib.h>
> +#include <Library/IoLib.h>
> +#include <Library/NorFlashPlatformLib.h>
> +#include <SgiPlatform.h>
> +
> +STATIC NOR_FLASH_DESCRIPTION mNorFlashDevices[] = {
> +  {
> +    // Secure Boot storage space of 4MB
> +    SGI_EXP_SMC_CS2_BASE,
> +    SGI_EXP_SMC_CS2_BASE,
> +    SIZE_256KB * 16,
> +    SIZE_256KB,
> +  },
> +  {
> +    //Secure variable storage space of 1MB*3
> +    SGI_EXP_SMC_CS2_BASE,
> +    SGI_EXP_SMC_CS2_BASE + SIZE_256KB * 16,
> +    SIZE_256KB * 12,
> +    SIZE_256KB,
> +  },
> +};
> +
> +EFI_STATUS
> +NorFlashPlatformInitialization (
> +  VOID
> +  )
> +{
> +  UINT64 SysRegFlash;
> +
> +  SysRegFlash = SGI_EXP_SYSPH_SYSTEM_REGISTERS + SGI_SYSPH_SYS_REG_FLASH;
> +  MmioOr32 (SysRegFlash, SGI_SYSPH_SYS_REG_FLASH_RWEN);
> +  return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +NorFlashPlatformGetDevices (
> +  OUT NOR_FLASH_DESCRIPTION   **NorFlashDevices,
> +  OUT UINT32                  *Count
> +  )
> +{
> +  if ((NorFlashDevices == NULL) || (Count == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *NorFlashDevices = mNorFlashDevices;
> +  *Count = ARRAY_SIZE (mNorFlashDevices);
> +  return EFI_SUCCESS;
> +}
> diff --git a/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf b/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
> new file mode 100644
> index 0000000..eedfacc
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
> @@ -0,0 +1,35 @@
> +#/** @file
> +#
> +#  Copyright (c) 2019, ARM Ltd. All rights reserved.
> +
> +#  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                      = NorFlashSgiLib
> +  FILE_GUID                      = 2ce22190-b933-4d1e-99ba-8bf1f0768255
> +  MODULE_TYPE                    = MM_STANDALONE

Could this be BASE ?

> +  VERSION_STRING                 = 1.0
> +  PI_SPECIFICATION_VERSION       = 0x00010032

Can we drop this?

> +  LIBRARY_CLASS                  = NorFlashPlatformLib
> +
> +[Sources.common]
> +  StandaloneMmNorFlashLib.c
> +
> +[Packages]
> +  ArmPlatformPkg/ArmPlatformPkg.dec
> +  MdePkg/MdePkg.dec
> +  Platform/ARM/SgiPkg/SgiPlatform.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  IoLib
> --
> 2.7.4
>

Re: [PATCH edk2-platforms 1/3] Platform/ARM/Sgi: define nor2 flash controller memory map

[Ard Biesheuvel]

On Mon, 4 Mar 2019 at 11:12, Jagadeesh Ujja <jagadeesh.ujja@arm.com> wrote:
>
> Add the definitions of NOR2 flash controller memory map. The NO2 flash
> can be used as an additional non-volatile storage by non-secure code or
> used as a non-volatile storage for secure variables by the StandaloneMM.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---
>  Platform/ARM/SgiPkg/Include/SgiPlatform.h | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/Platform/ARM/SgiPkg/Include/SgiPlatform.h b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> index b9a662a..2a7b79d 100644
> --- a/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> +++ b/Platform/ARM/SgiPkg/Include/SgiPlatform.h
> @@ -27,6 +27,10 @@
>  #define SGI_EXP_SMC_CS1_BASE                      0x0C000000
>  #define SGI_EXP_SMC_CS1_SZ                        SIZE_64MB
>
> +// Expansion AXI - SMC Chip Select 2
> +#define SGI_EXP_SMC_CS2_BASE                      0x10000000
> +#define SGI_EXP_SMC_CS2_SZ                        SIZE_64MB
> +
>  // Expansion AXI - SMSC 91C111 (Ethernet)
>  #define SGI_EXP_SMSC91X_BASE                      0x18000000
>  #define SGI_EXP_SMSC91X_SZ                        SIZE_64MB
> --
> 2.7.4
>

Re: [PATCH edk2-platforms 3/3] Platform/ARM/SgiPkg: add MM based UEFI secure boot support

[Ard Biesheuvel]

On Mon, 4 Mar 2019 at 11:12, Jagadeesh Ujja <jagadeesh.ujja@arm.com> wrote:
>
> This implements support for UEFI secure boot on SGI platforms using
> the standalone MM framework. This moves all of the software handling
> of the UEFI authenticated variable store into the standalone MM
> context residing in a secure partition.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jagadeesh Ujja <jagadeesh.ujja@arm.com>
> ---
>  Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc | 33 +++++++++++++++++++-
>  Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf |  5 +++
>  Platform/ARM/SgiPkg/SgiPlatform.dsc          | 18 ++++++++++-
>  Platform/ARM/SgiPkg/SgiPlatform.fdf          |  9 +++++-
>  4 files changed, 62 insertions(+), 3 deletions(-)
>
> diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> index 65dd6ac..889be2f 100644
> --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.dsc
> @@ -75,7 +75,17 @@
>    HobLib|StandaloneMmPkg/Library/StandaloneMmHobLib/StandaloneMmHobLib.inf
>    MmServicesTableLib|MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
>    MemoryAllocationLib|StandaloneMmPkg/Library/StandaloneMmMemoryAllocationLib/StandaloneMmMemoryAllocationLib.inf
> -
> +!if $(MM_SECURE_STORAGE_ENABLE) == TRUE

Does it make sense to make this conditional? Would you build
PlatformStandaloneMm.dsc if MM_SECURE_STORAGE_ENABLE is FALSE?

> +  AuthVariableLib|SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> +  IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> +  NorFlashPlatformLib|Platform/ARM/SgiPkg/Library/NorFlashLib/StandaloneMmNorFlashLib.inf
> +  OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +  PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
> +  SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
> +  TimerLib|MdePkg/Library/BaseTimerLibNullTemplate/BaseTimerLibNullTemplate.inf
> +  VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> +!endif
>  ################################################################################
>  #
>  # Pcd Section - list of all EDK II PCD Entries defined by this Platform
> @@ -93,6 +103,21 @@
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase|0x7FF70000
>    gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate|115200
>
> +!if $(MM_SECURE_STORAGE_ENABLE) == TRUE

Same here

> +  #Secure Storage
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
> +  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
> +
> +  ## NV Storage - 1MB*3 in NOR2 Flash
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase|0x10400000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize|0x00100000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase|0x10500000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize|0x00100000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase|0x10600000
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize|0x00100000
> +!endif
> +
>  ###################################################################################################
>  #
>  # Components Section - list of the modules and components that will be processed by compilation
> @@ -118,6 +143,12 @@
>    StandaloneMmPkg/Core/StandaloneMmCore.inf
>
>  [Components.AARCH64]
> +!if $(MM_SECURE_STORAGE_ENABLE) == TRUE

Same here

> +  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> +  MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
> +  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> +!endif
> +
>    StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
>
>  ###################################################################################################
> diff --git a/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf b/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> index 810460c..04aa5cd 100644
> --- a/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> +++ b/Platform/ARM/SgiPkg/PlatformStandaloneMm.fdf
> @@ -55,6 +55,11 @@ READ_LOCK_CAP      = TRUE
>  READ_LOCK_STATUS   = TRUE
>
>    INF StandaloneMmPkg/Core/StandaloneMmCore.inf
> +!if $(MM_SECURE_STORAGE_ENABLE) == TRUE

and here

> +  INF ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> +  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteStandaloneMm.inf
> +  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableStandaloneMm.inf
> +!endif
>    INF StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
>
>  ################################################################################
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> index bdb4ecb..cba2e1f 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dsc
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc
> @@ -260,7 +260,15 @@
>    MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>    MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
>    MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
> +!if $(SECURE_BOOT_ENABLE) == TRUE

Please declare this symbol in the [Defines] section using 'DEFINE'

> +  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
> +    <LibraryClasses>
> +      NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> +  }
> +  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> +!else
>    MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> +!endif
>    OvmfPkg/VirtioBlkDxe/VirtioBlk.inf
>
>    MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
> @@ -268,6 +276,9 @@
>    MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf
>    MdeModulePkg/Universal/Console/TerminalDxe/TerminalDxe.inf
>    MdeModulePkg/Universal/SerialDxe/SerialDxe.inf
> +!if $(SECURE_STORAGE_ENABLE) == TRUE
> +  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
> +!else
>    MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
>      <LibraryClasses>
>        NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
> @@ -275,6 +286,7 @@
>        BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
>    }
>    MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
> +!endif
>
>    #
>    # ACPI Support
> @@ -344,4 +356,8 @@
>    #
>    MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
>
> -  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
> +  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf {
> +    <LibraryClasses>
> +      NULL|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf
> +  }
> +  ArmPkg/Drivers/ApeiDxe/Apei.inf

This driver does not exist

> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> index 7916a52..a438940 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.fdf
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> @@ -96,10 +96,17 @@ READ_LOCK_STATUS   = TRUE
>    INF EmbeddedPkg/ResetRuntimeDxe/ResetRuntimeDxe.inf
>    INF MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
>    INF MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> -  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>    INF MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
>    INF MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
> +!if $(SECURE_BOOT_ENABLE) == TRUE
> +  INF SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
> +!endif
> +!if $(SECURE_STORAGE_ENABLE) == TRUE

Why would you support secure boot without secure storage? Can you
create a single configuration item for this?

> +  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
> +!else
> +  INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
>    INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
> +!endif
>
>    #
>    # ACPI Support
> --
> 2.7.4
>