[edk2] [PATCH v2 3/4] MdePkg/UefiLib: Simplify protocol un/installation abstraction

2019-01-04 Thread Ashish Singhal
Add a helper function to operate upon protocol installation and
uninstallation instead of every function doing it by itself.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 MdePkg/Include/Library/UefiLib.h |   26 +-
 MdePkg/Library/UefiLib/UefiDriverModel.c | 1980 --
 2 files changed, 270 insertions(+), 1736 deletions(-)

diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index 08222d4..fbc9739 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -1323,7 +1323,10 @@ EfiLibInstallDriverBinding (
   If DriverBinding is NULL, then ASSERT().
   If DriverBinding can not be uninstalled, then ASSERT().
 
+  @param  ImageHandle  The image handle of the driver.
+  @param  SystemTable  The EFI System Table that was passed to the 
driver's entry point.
   @param  DriverBindingA Driver Binding Protocol instance that this 
driver produced.
+  @param  DriverBindingHandle  The handle that DriverBinding is to be 
installed onto.
 
   @retval EFI_SUCCESS   The protocol uninstallation successfully 
completed.
   @retval OthersStatus from 
gBS->UninstallMultipleProtocolInterfaces().
@@ -1332,7 +1335,10 @@ EfiLibInstallDriverBinding (
 EFI_STATUS
 EFIAPI
 EfiLibUninstallDriverBinding (
-  IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding
+  IN CONST EFI_HANDLE ImageHandle,
+  IN CONST EFI_SYSTEM_TABLE   *SystemTable,
+  IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding,
+  IN EFI_HANDLE   DriverBindingHandle
   );
 
 
@@ -1382,7 +1388,10 @@ EfiLibInstallAllDriverProtocols (
   If DriverBinding is NULL, then ASSERT().
   If the uninstallation fails, then ASSERT().
 
+  @param  ImageHandle  The image handle of the driver.
+  @param  SystemTable  The EFI System Table that was passed to the 
driver's entry point.
   @param  DriverBindingA Driver Binding Protocol instance that this 
driver produced.
+  @param  DriverBindingHandle  The handle that DriverBinding is to be 
installed onto.
   @param  ComponentNameA Component Name Protocol instance that this 
driver produced.
   @param  DriverConfiguration  A Driver Configuration Protocol instance that 
this driver produced.
   @param  DriverDiagnosticsA Driver Diagnostics Protocol instance that 
this driver produced.
@@ -1394,7 +1403,10 @@ EfiLibInstallAllDriverProtocols (
 EFI_STATUS
 EFIAPI
 EfiLibUninstallAllDriverProtocols (
+  IN CONST EFI_HANDLE ImageHandle,
+  IN CONST EFI_SYSTEM_TABLE   *SystemTable,
   IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding,
+  IN EFI_HANDLE   DriverBindingHandle,
   IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,   OPTIONAL
   IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL  *DriverConfiguration, OPTIONAL
   IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnosticsOPTIONAL
@@ -1442,7 +1454,10 @@ EfiLibInstallDriverBindingComponentName2 (
   If DriverBinding is NULL, then ASSERT().
   If the uninstallation fails, then ASSERT().
 
+  @param  ImageHandle  The image handle of the driver.
+  @param  SystemTable  The EFI System Table that was passed to the 
driver's entry point.
   @param  DriverBindingA Driver Binding Protocol instance that this 
driver produced.
+  @param  DriverBindingHandle  The handle that DriverBinding is to be 
installed onto.
   @param  ComponentNameA Component Name Protocol instance that this 
driver produced.
   @param  ComponentName2   A Component Name 2 Protocol instance that this 
driver produced.
 
@@ -1453,7 +1468,10 @@ EfiLibInstallDriverBindingComponentName2 (
 EFI_STATUS
 EFIAPI
 EfiLibUninstallDriverBindingComponentName2 (
+  IN CONST EFI_HANDLE ImageHandle,
+  IN CONST EFI_SYSTEM_TABLE   *SystemTable,
   IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding,
+  IN EFI_HANDLE   DriverBindingHandle,
   IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,   OPTIONAL
   IN CONST EFI_COMPONENT_NAME2_PROTOCOL   *ComponentName2   OPTIONAL
   );
@@ -1512,7 +1530,10 @@ EfiLibInstallAllDriverProtocols2 (
   If the installation fails, then ASSERT().
 
 
+  @param  ImageHandle   The image handle of the driver.
+  @param  SystemTable   The EFI System Table that was passed to the 
driver's entry point.
   @param  DriverBinding A Driver Binding Protocol instance that this 
driver produced.
+  @param  DriverBindingHandle   The handle that DriverBinding is to be 
installed onto.
   @param  ComponentName A Component Name Protocol instance that this 
driver produced.
   @param  ComponentName2A Component Name 2 Protocol instance that this 
driver produced.
   @param  DriverConfiguration   A Driver Configuration Protocol 

[edk2] [PATCH v2 2/4] NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.

2019-01-04 Thread Ashish Singhal
During cleanup in case of initialization failure, some driver
bindings are not installed. Using abstractions in UEFILib takes
care of it.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1428

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 NetworkPkg/IScsiDxe/IScsiDriver.c | 31 +++
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c 
b/NetworkPkg/IScsiDxe/IScsiDriver.c
index 91176e6..8747de7 100644
--- a/NetworkPkg/IScsiDxe/IScsiDriver.c
+++ b/NetworkPkg/IScsiDxe/IScsiDriver.c
@@ -1,6 +1,7 @@
 /** @file
   The entry point of IScsi driver.
 
+Copyright (c) 2019, NVIDIA Corporation. All rights reserved.
 Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.
 (C) Copyright 2017 Hewlett Packard Enterprise Development LP
 
@@ -1861,28 +1862,18 @@ Error3:
  );
 
 Error2:
-  gBS->UninstallMultipleProtocolInterfaces (
- gIScsiIp6DriverBinding.DriverBindingHandle,
- ,
- ,
- ,
- ,
- ,
- ,
- NULL
- );
+  EfiLibUninstallDriverBindingComponentName2 (
+,
+,
+
+);
 
 Error1:
-  gBS->UninstallMultipleProtocolInterfaces (
- ImageHandle,
- ,
- ,
- ,
- ,
- ,
- ,
- NULL
- );
+  EfiLibUninstallDriverBindingComponentName2 (
+,
+,
+
+);
 
   return Status;
 }
-- 
2.7.4

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


[edk2] [PATCH v2 4/4] NetworkPkg/IScsiDxe: Update UEFILib Usage

2019-01-04 Thread Ashish Singhal
Update interfaces as exposed by UEFILib for protocol
installation and uninstallation abstraction.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 NetworkPkg/IScsiDxe/IScsiDriver.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c 
b/NetworkPkg/IScsiDxe/IScsiDriver.c
index 8747de7..fa0ea00 100644
--- a/NetworkPkg/IScsiDxe/IScsiDriver.c
+++ b/NetworkPkg/IScsiDxe/IScsiDriver.c
@@ -1863,14 +1863,20 @@ Error3:
 
 Error2:
   EfiLibUninstallDriverBindingComponentName2 (
+ImageHandle,
+SystemTable,
 ,
+NULL,
 ,
 
 );
 
 Error1:
   EfiLibUninstallDriverBindingComponentName2 (
+ImageHandle,
+SystemTable,
 ,
+NULL,
 ,
 
 );
-- 
2.7.4

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


[edk2] [PATCH v2 0/4] Provide UEFILib functions for protocol uninstallation.

2019-01-04 Thread Ashish Singhal
An issue was seen in IScsiDxe in NetworkPkg where driver cleanup after
initialization failure was not done right. Bug 1428 was filed in this regard.
As per discussions with Mike, it was also discussed that having UEFILib
provide protocol uninstallation abstraction would help to avoid these
issues in the future. Bug 1429 was found to track this. The first 2 patches
take care of this.

Patch number 3 simplifies the UEFILib protocol installation and
uninstallation abstraction by adding a helper function doing operations
instead of every public function.

Patch set 4 uses the updated uninstallation interfaces as a result of patch 3.


Ashish Singhal (4):
  MdePkg/UefiLib: Abstract driver model protocol uninstallation
  NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.
  MdePkg/UefiLib: Simplify protocol un/installation abstraction
  NetworkPkg/IScsiDxe: Update UEFILib Usage

 MdePkg/Include/Library/UefiLib.h |  127 
 MdePkg/Library/UefiLib/UefiDriverModel.c | 1210 +-
 NetworkPkg/IScsiDxe/IScsiDriver.c|   37 +-
 3 files changed, 489 insertions(+), 885 deletions(-)

-- 
2.7.4

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


[edk2] [PATCH v2 1/4] MdePkg/UefiLib: Abstract driver model protocol uninstallation

2019-01-04 Thread Ashish Singhal
Provided functions in UEFILib that abstract driver model protocol
uninstallation. This helps drivers to install and uninstall protocols
using a library to keep things seemless.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1429

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 MdePkg/Include/Library/UefiLib.h | 103 
 MdePkg/Library/UefiLib/UefiDriverModel.c | 972 ++-
 2 files changed, 1074 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index 468bffc..08222d4 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -12,6 +12,7 @@
   of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is
   defined, then debug and assert related macros wrapped by it are the NULL 
implementations.
 
+Copyright (c) 2019, NVIDIA Corporation. All rights reserved.
 Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials are licensed and made available 
under
 the terms and conditions of the BSD License that accompanies this distribution.
@@ -1283,6 +1284,7 @@ AsciiPrintXY (
   ...
   );
 
+
 /**
   Installs and completes the initialization of a Driver Binding Protocol 
instance.
 
@@ -1316,6 +1318,25 @@ EfiLibInstallDriverBinding (
 
 
 /**
+  Uninstalls a Driver Binding Protocol instance.
+
+  If DriverBinding is NULL, then ASSERT().
+  If DriverBinding can not be uninstalled, then ASSERT().
+
+  @param  DriverBindingA Driver Binding Protocol instance that this 
driver produced.
+
+  @retval EFI_SUCCESS   The protocol uninstallation successfully 
completed.
+  @retval OthersStatus from 
gBS->UninstallMultipleProtocolInterfaces().
+
+**/
+EFI_STATUS
+EFIAPI
+EfiLibUninstallDriverBinding (
+  IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding
+  );
+
+
+/**
   Installs and completes the initialization of a Driver Binding Protocol 
instance and
   optionally installs the Component Name, Driver Configuration and Driver 
Diagnostics Protocols.
 
@@ -1354,6 +1375,31 @@ EfiLibInstallAllDriverProtocols (
   );
 
 
+/**
+  Uninstalls a Driver Binding Protocol instance and optionally uninstalls the
+  Component Name, Driver Configuration and Driver Diagnostics Protocols.
+
+  If DriverBinding is NULL, then ASSERT().
+  If the uninstallation fails, then ASSERT().
+
+  @param  DriverBindingA Driver Binding Protocol instance that this 
driver produced.
+  @param  ComponentNameA Component Name Protocol instance that this 
driver produced.
+  @param  DriverConfiguration  A Driver Configuration Protocol instance that 
this driver produced.
+  @param  DriverDiagnosticsA Driver Diagnostics Protocol instance that 
this driver produced.
+
+  @retval EFI_SUCCESS   The protocol uninstallation successfully 
completed.
+  @retval OthersStatus from 
gBS->UninstallMultipleProtocolInterfaces().
+
+**/
+EFI_STATUS
+EFIAPI
+EfiLibUninstallAllDriverProtocols (
+  IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding,
+  IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,   OPTIONAL
+  IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL  *DriverConfiguration, OPTIONAL
+  IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnosticsOPTIONAL
+  );
+
 
 /**
   Installs Driver Binding Protocol with optional Component Name and Component 
Name 2 Protocols.
@@ -1391,6 +1437,29 @@ EfiLibInstallDriverBindingComponentName2 (
 
 
 /**
+  Uninstalls Driver Binding Protocol with optional Component Name and 
Component Name 2 Protocols.
+
+  If DriverBinding is NULL, then ASSERT().
+  If the uninstallation fails, then ASSERT().
+
+  @param  DriverBindingA Driver Binding Protocol instance that this 
driver produced.
+  @param  ComponentNameA Component Name Protocol instance that this 
driver produced.
+  @param  ComponentName2   A Component Name 2 Protocol instance that this 
driver produced.
+
+  @retval EFI_SUCCESS   The protocol installation successfully 
completed.
+  @retval OthersStatus from 
gBS->UninstallMultipleProtocolInterfaces().
+
+**/
+EFI_STATUS
+EFIAPI
+EfiLibUninstallDriverBindingComponentName2 (
+  IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding,
+  IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,   OPTIONAL
+  IN CONST EFI_COMPONENT_NAME2_PROTOCOL   *ComponentName2   OPTIONAL
+  );
+
+
+/**
   Installs Driver Binding Protocol with optional Component Name, Component 
Name 2, Driver
   Configuration, Driver Configuration 2, Driver Diagnostics, and Driver 
Diagnostics 2 Protocols.
 
@@ -1434,6 +1503,40 @@ EfiLibInstallAllDriverProtocols2 (
   IN CONST EFI_DRIVER_DIAGNOSTICS2_PROTOCOL   *DriverDiagnostics2OPTIONAL
   );
 
+
+/**
+  Uninstalls Driver Binding Protocol with optional Component Name, Component 
Name 2, Driver
+  

Re: [edk2] [PATCH v2 08/11] MdeModulePkg/VarCheckLib: allow MM_STANDALONE drivers to use this library

2019-01-04 Thread Ard Biesheuvel
On Wed, 2 Jan 2019 at 14:14, Jagadeesh Ujja  wrote:
>
> “VarCheckLib” library can be used by MM_STANDALONE drivers as well.
> So add MM_STANDALONE as the module type this library supports.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jagadeesh Ujja 

Reviewed-by: Ard Biesheuvel 

> ---
>  MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf 
> b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> index 099f83d..c8cf810 100644
> --- a/MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> +++ b/MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
> @@ -2,6 +2,7 @@
>  #  Provides variable check services and database management.
>  #
>  #  Copyright (c) 2015, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2018, ARM Limited. All rights reserved.
>  #
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions
> @@ -21,12 +22,12 @@
>FILE_GUID  = 63E12D08-0C5D-47F8-95E4-09F89D7506C5
>MODULE_TYPE= DXE_RUNTIME_DRIVER
>VERSION_STRING = 1.0
> -  LIBRARY_CLASS  = VarCheckLib|DXE_RUNTIME_DRIVER 
> DXE_SMM_DRIVER
> +  LIBRARY_CLASS  = VarCheckLib|DXE_RUNTIME_DRIVER 
> DXE_SMM_DRIVER MM_STANDALONE
>
>  #
>  # The following information is for reference only and not required by the 
> build tools.
>  #
> -#  VALID_ARCHITECTURES   = IA32 X64
> +#  VALID_ARCHITECTURES   = IA32 X64 AARCH64
>  #
>
>  [Sources]
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v2 11/11] CryptoPkg/BaseCryptLib: allow MM_STANDALONE drivers to use this library

2019-01-04 Thread Ard Biesheuvel
(add the CryptoPkg maintainer)

On Wed, 2 Jan 2019 at 14:14, Jagadeesh Ujja  wrote:
>
> “SmmCryptLib” library can be used by MM_STANDALONE drivers as well.
> So add MM_STANDALONE as the module type this library supports.
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jagadeesh Ujja 

The subject is a bit misleading, please change it to

CryptoPkg/BaseCryptLib: allow MM_STANDALONE drivers to use SmmCryptLib

With that,

Reviewed-by: Ard Biesheuvel 


> ---
>  CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf 
> b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> index 4829669..9a63419 100644
> --- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> +++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> @@ -30,7 +30,7 @@
>MODULE_TYPE= DXE_SMM_DRIVER
>VERSION_STRING = 1.0
>PI_SPECIFICATION_VERSION   = 0x0001000A
> -  LIBRARY_CLASS  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE
> +  LIBRARY_CLASS  = BaseCryptLib|DXE_SMM_DRIVER SMM_CORE 
> MM_STANDALONE
>
>  #
>  # The following information is for reference only and not required by the 
> build tools.
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [PATCH 8/8] StandaloneMmPkg/Core: permit encapsulated firmware volumes

2019-01-04 Thread 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 
---
 StandaloneMmPkg/Core/FwVol.c   
 | 99 ++--
 StandaloneMmPkg/Core/StandaloneMmCore.inf  
 |  1 +
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
 |  5 +
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
   |  3 +
 4 files changed, 99 insertions(+), 9 deletions(-)

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 
+#include 
 
 //
 // 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, );
+if (EFI_ERROR (Status)) {
+  break;
+}
+Status = FfsFindSectionData (EFI_SECTION_GUID_DEFINED, FileHeader,
+   , );
+if (EFI_ERROR (Status)) {
+  break;
+}
+Section = (EFI_COMMON_SECTION_HEADER *)(FileHeader + 1);
+Status = ExtractGuidedSectionGetInfo (Section, ,
+   , );
+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, , ScratchBuffer,
+);
+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, );
+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 

[edk2] [PATCH 5/8] StandaloneMmPkg/StandaloneMmPeCoffExtraActionLib: ignore runtime attribute

2019-01-04 Thread Ard Biesheuvel
The special handling of the EFI_IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER
attribute is only necessary for images that are relocated twice, i.e.,
in the context of SetVirtualAddressMap (). This does not apply to
standalone MM modules, so drop the check.

Drop some redundant DEBUG output while at it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 
StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c
 | 9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c
 
b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c
index 1c9fec201916..f6bfcc875751 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/AArch64/StandaloneMmPeCoffExtraActionLib.c
@@ -145,8 +145,7 @@ UpdatePeCoffPermissions (
 
 if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_MEM_EXECUTE) == 0) {
 
-  if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_MEM_WRITE) == 0 &&
-  TmpContext.ImageType != EFI_IMAGE_SUBSYSTEM_EFI_RUNTIME_DRIVER) {
+  if ((SectionHeader.Characteristics & EFI_IMAGE_SCN_MEM_WRITE) == 0) {
 
 DEBUG ((DEBUG_INFO,
   "%a: Mapping section %d of image at 0x%lx with RO-XN permissions and 
size 0x%x\n",
@@ -158,14 +157,10 @@ UpdatePeCoffPermissions (
   __FUNCTION__, Index, Base, SectionHeader.Misc.VirtualSize));
   }
 } else {
-DEBUG ((DEBUG_INFO,
-  "%a: Mapping section %d of image at 0x%lx with RO-XN permissions and 
size 0x%x\n",
-   __FUNCTION__, Index, Base, SectionHeader.Misc.VirtualSize));
-ReadOnlyUpdater (Base, SectionHeader.Misc.VirtualSize);
-
 DEBUG ((DEBUG_INFO,
   "%a: Mapping section %d of image at 0x%lx with RO-X permissions and 
size 0x%x\n",
   __FUNCTION__, Index, Base, SectionHeader.Misc.VirtualSize));
+ReadOnlyUpdater (Base, SectionHeader.Misc.VirtualSize);
 NoExecUpdater (Base, SectionHeader.Misc.VirtualSize);
 }
 
-- 
2.17.1

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


[edk2] [PATCH 2/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib ref

2019-01-04 Thread Ard Biesheuvel
StandaloneMmCoreEntryPoint calls SerialPortInitialize() explicitly,
so add SerialPortLib to its list of LibraryClasses.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
 | 1 +
 1 file changed, 1 insertion(+)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
index 3222cd359f3e..769eaeeefbea 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
@@ -43,6 +43,7 @@ [Packages.AARCH64]
 [LibraryClasses]
   BaseLib
   DebugLib
+  SerialPortLib
 
 [LibraryClasses.AARCH64]
   StandaloneMmMmuLib
-- 
2.17.1

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


[edk2] [PATCH 6/8] StandaloneMmPkg/Core/Dispatcher: don't copy dispatched image twice

2019-01-04 Thread Ard Biesheuvel
The dispatcher uses the PE/COFF loader to load images into the heap,
but only does so after copying the entire image first, leading to
two copies being made for no good reason.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 StandaloneMmPkg/Core/Dispatcher.c | 30 +---
 1 file changed, 1 insertion(+), 29 deletions(-)

diff --git a/StandaloneMmPkg/Core/Dispatcher.c 
b/StandaloneMmPkg/Core/Dispatcher.c
index 8d009b4f80c1..8a2ad5118d92 100644
--- a/StandaloneMmPkg/Core/Dispatcher.c
+++ b/StandaloneMmPkg/Core/Dispatcher.c
@@ -294,7 +294,6 @@ MmLoadImage (
   IN OUT EFI_MM_DRIVER_ENTRY  *DriverEntry
   )
 {
-  VOID   *Buffer;
   UINTN  PageCount;
   EFI_STATUS Status;
   EFI_PHYSICAL_ADDRESS   DstBuffer;
@@ -302,17 +301,12 @@ MmLoadImage (
 
   DEBUG ((DEBUG_INFO, "MmLoadImage - %g\n", >FileName));
 
-  Buffer = AllocateCopyPool (DriverEntry->Pe32DataSize, DriverEntry->Pe32Data);
-  if (Buffer == NULL) {
-return EFI_OUT_OF_RESOURCES;
-  }
-
   Status   = EFI_SUCCESS;
 
   //
   // Initialize ImageContext
   //
-  ImageContext.Handle = Buffer;
+  ImageContext.Handle = DriverEntry->Pe32Data;
   ImageContext.ImageRead = PeCoffLoaderImageReadFromMemory;
 
   //
@@ -320,9 +314,6 @@ MmLoadImage (
   //
   Status = PeCoffLoaderGetImageInfo ();
   if (EFI_ERROR (Status)) {
-if (Buffer != NULL) {
-  MmFreePool (Buffer);
-}
 return Status;
   }
 
@@ -336,9 +327,6 @@ MmLoadImage (
  
  );
   if (EFI_ERROR (Status)) {
-if (Buffer != NULL) {
-  MmFreePool (Buffer);
-}
 return Status;
   }
 
@@ -355,9 +343,6 @@ MmLoadImage (
   //
   Status = PeCoffLoaderLoadImage ();
   if (EFI_ERROR (Status)) {
-if (Buffer != NULL) {
-  MmFreePool (Buffer);
-}
 MmFreePages (DstBuffer, PageCount);
 return Status;
   }
@@ -367,9 +352,6 @@ MmLoadImage (
   //
   Status = PeCoffLoaderRelocateImage ();
   if (EFI_ERROR (Status)) {
-if (Buffer != NULL) {
-  MmFreePool (Buffer);
-}
 MmFreePages (DstBuffer, PageCount);
 return Status;
   }
@@ -393,9 +375,6 @@ MmLoadImage (
   (VOID 
**)>LoadedImage
   );
 if (EFI_ERROR (Status)) {
-  if (Buffer != NULL) {
-MmFreePool (Buffer);
-  }
   MmFreePages (DstBuffer, PageCount);
   return Status;
 }
@@ -482,13 +461,6 @@ MmLoadImage (
 
   DEBUG_CODE_END ();
 
-  //
-  // Free buffer allocated by Fv->ReadSection.
-  //
-  // The UEFI Boot Services FreePool() function must be used because 
Fv->ReadSection
-  // used the UEFI Boot Services AllocatePool() function
-  //
-  MmFreePool (Buffer);
   return Status;
 }
 
-- 
2.17.1

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


[edk2] [PATCH 1/8] StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone

2019-01-04 Thread Ard Biesheuvel
Fix a couple of occurrences of typo Standlone -> Standalone. Since
_PiMmStandloneArmTfCpuDriverEntry() is never referenced, drop it
altogether.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c   | 2 +-
 StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c   | 6 +++---
 StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h   | 8 
+---
 StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf | 4 ++--
 4 files changed, 7 insertions(+), 13 deletions(-)

diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c 
b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
index 2814577b3fcc..25114821448a 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
@@ -65,7 +65,7 @@ EFI_MM_CONFIGURATION_PROTOCOL mMmConfig = {
 STATIC EFI_MM_ENTRY_POINT mMmEntryPoint = NULL;
 
 EFI_STATUS
-PiMmStandloneArmTfCpuDriverEntry (
+PiMmStandaloneArmTfCpuDriverEntry (
   IN UINTN EventId,
   IN UINTN CpuNumber,
   IN UINTN NsCommBufferAddr
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c 
b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
index 85a9c108aea4..203a32baaaf9 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
@@ -74,7 +74,7 @@ GetGuidedHobData (
 }
 
 EFI_STATUS
-StandloneMmCpuInitialize (
+StandaloneMmCpuInitialize (
   IN EFI_HANDLE ImageHandle,  // not actual imagehandle
   IN EFI_MM_SYSTEM_TABLE   *SystemTable  // not actual systemtable
   )
@@ -147,8 +147,8 @@ StandloneMmCpuInitialize (
   // Share the entry point of the CPU driver
   DEBUG ((DEBUG_INFO, "Sharing Cpu Driver EP *0x%lx = 0x%lx\n",
   (UINT64) CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
-  (UINT64) PiMmStandloneArmTfCpuDriverEntry));
-  *(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) = 
PiMmStandloneArmTfCpuDriverEntry;
+  (UINT64) PiMmStandaloneArmTfCpuDriverEntry));
+  *(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) = 
PiMmStandaloneArmTfCpuDriverEntry;
 
   // Find the descriptor that contains the whereabouts of the buffer for
   // communication with the Normal world.
diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h 
b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h
index 7b38b65e1242..543467f67576 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h
@@ -40,7 +40,7 @@ extern MP_INFORMATION_HOB_DATA   *mMpInformationHobData;
 extern EFI_MM_CONFIGURATION_PROTOCOL mMmConfig;
 
 EFI_STATUS
-PiMmStandloneArmTfCpuDriverEntry (
+PiMmStandaloneArmTfCpuDriverEntry (
   IN UINTN EventId,
   IN UINTN CpuNumber,
   IN UINTN NsCommBufferAddr
@@ -55,10 +55,4 @@ PiMmCpuTpFwRootMmiHandler (
   IN OUT UINTN*CommBufferSize  OPTIONAL
   );
 
-EFI_STATUS _PiMmStandloneArmTfCpuDriverEntry (
-  IN UINTN EventId,
-  IN UINTN CpuNumber,
-  IN UINTN NsCommBufferAddr
-  );
-
 #endif
diff --git 
a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf 
b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
index 9e6bbabdb103..d261e51ebc75 100644
--- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
+++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
@@ -18,12 +18,12 @@
 
 [Defines]
   INF_VERSION= 0x0001001A
-  BASE_NAME  = StandloneMmCpu
+  BASE_NAME  = StandaloneMmCpu
   FILE_GUID  = 58F7A62B-6280-42A7-BC38-10535A64A92C
   MODULE_TYPE= MM_STANDALONE
   VERSION_STRING = 1.0
   PI_SPECIFICATION_VERSION   = 0x00010032
-  ENTRY_POINT= StandloneMmCpuInitialize
+  ENTRY_POINT= StandaloneMmCpuInitialize
 
 [Sources]
   StandaloneMmCpu.c
-- 
2.17.1

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


[edk2] [PATCH 4/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus ASSERT_EFI_ERROR()s

2019-01-04 Thread Ard Biesheuvel
ASSERT_EFI_ERROR (x) is a shorthand for ASSERT(!EFI_ERROR(x)), and so
it should only be used with EFI_STATUS type expressions.

So drop two instances that operate on other types, since neither looks
particularly useful.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 
StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
 | 2 --
 1 file changed, 2 deletions(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
index 05ed6c8dd0b5..5cca532456fd 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
@@ -295,7 +295,6 @@ _ModuleEntryPoint (
   //
   ProcessModuleEntryPointList (HobStart);
 
-  ASSERT_EFI_ERROR (CpuDriverEntryPoint);
   DEBUG ((DEBUG_INFO, "Shared Cpu Driver EP 0x%lx\n", (UINT64) 
CpuDriverEntryPoint));
 
 finish:
@@ -303,5 +302,4 @@ finish:
   InitMmFoundationSvcArgs.Arg0 = ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64;
   InitMmFoundationSvcArgs.Arg1 = Status;
   DelegatedEventLoop ();
-  ASSERT_EFI_ERROR (0);
 }
-- 
2.17.1

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


[edk2] [PATCH 7/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: permit the use of TE images

2019-01-04 Thread Ard Biesheuvel
TE images take up less space when using 4 KB section alignment, since
the FFS/FV generation code optimizes away the redundant, nested padding.
This saves 4 KB of space, which is a worthwhile improvement for code
that executes in place in secure context.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c | 
107 +---
 1 file changed, 46 insertions(+), 61 deletions(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
index 3ca7f6660f47..90299ebbafb6 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
@@ -143,9 +143,12 @@ LocateStandaloneMmCorePeCoffData (
 
   Status = FfsFindSectionData (EFI_SECTION_PE32, FileHeader, TeData, 
TeDataSize);
   if (EFI_ERROR (Status)) {
-  DEBUG ((DEBUG_ERROR, "Unable to locate Standalone MM Section data - 
0x%x\n",
-  Status));
-return Status;
+Status = FfsFindSectionData (EFI_SECTION_TE, FileHeader, TeData, 
TeDataSize);
+if (EFI_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR, "Unable to locate Standalone MM Section data - 
%r\n",
+Status));
+  return Status;
+}
   }
 
   DEBUG ((DEBUG_INFO, "Found Standalone MM PE data - 0x%x\n", *TeData));
@@ -155,10 +158,9 @@ LocateStandaloneMmCorePeCoffData (
 STATIC
 EFI_STATUS
 GetPeCoffSectionInformation (
-  IN  CONST PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext,
-  IN  OUT   PE_COFF_LOADER_IMAGE_CONTEXT  *TmpContext,
-  IN  OUT   UINT32*SectionHeaderOffset,
-  IN  OUT   UINT16*NumberOfSections
+  IN  OUT   PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext,
+  OUT   UINT32*SectionHeaderOffset,
+  OUT   UINT16*NumberOfSections
   )
 {
   RETURN_STATUS Status;
@@ -168,44 +170,29 @@ GetPeCoffSectionInformation (
   UINTN ReadSize;
 
   ASSERT (ImageContext != NULL);
-  ASSERT (TmpContext != NULL);
   ASSERT (SectionHeaderOffset != NULL);
   ASSERT (NumberOfSections != NULL);
 
-  //
-  // We need to copy ImageContext since PeCoffLoaderGetImageInfo ()
-  // will mangle the ImageAddress field
-  //
-  CopyMem (TmpContext, ImageContext, sizeof (*TmpContext));
-
-  if (TmpContext->PeCoffHeaderOffset == 0) {
-Status = PeCoffLoaderGetImageInfo (TmpContext);
-if (RETURN_ERROR (Status)) {
-  DEBUG ((DEBUG_ERROR,
-  "%a: PeCoffLoaderGetImageInfo () failed (Status = %r)\n",
-  __FUNCTION__, Status));
-  return Status;
-}
-  }
-
-  if (TmpContext->IsTeImage &&
-  TmpContext->ImageAddress == ImageContext->ImageAddress) {
-DEBUG ((DEBUG_INFO, "%a: ignoring XIP TE image at 0x%lx\n", __FUNCTION__,
-ImageContext->ImageAddress));
-return RETURN_UNSUPPORTED;
+  Status = PeCoffLoaderGetImageInfo (ImageContext);
+  if (RETURN_ERROR (Status)) {
+DEBUG ((DEBUG_ERROR,
+"%a: PeCoffLoaderGetImageInfo () failed (Status == %r)\n",
+__FUNCTION__, Status));
+return Status;
   }
 
-  if (TmpContext->SectionAlignment < EFI_PAGE_SIZE) {
+  if (ImageContext->SectionAlignment < EFI_PAGE_SIZE) {
 //
 // The sections need to be at least 4 KB aligned, since that is the
 // granularity at which we can tighten permissions.
 //
-if (!TmpContext->IsTeImage) {
+if (!ImageContext->IsTeImage) {
   DEBUG ((DEBUG_WARN,
   "%a: non-TE Image at 0x%lx has SectionAlignment < 4 KB (%lu)\n",
-  __FUNCTION__, ImageContext->ImageAddress, 
TmpContext->SectionAlignment));
+  __FUNCTION__, ImageContext->ImageAddress, 
ImageContext->SectionAlignment));
+  return RETURN_UNSUPPORTED;
 }
-return RETURN_UNSUPPORTED;
+ImageContext->SectionAlignment = EFI_PAGE_SIZE;
   }
 
   //
@@ -217,9 +204,9 @@ GetPeCoffSectionInformation (
   Hdr.Union = 
   Size = sizeof (EFI_IMAGE_OPTIONAL_HEADER_UNION);
   ReadSize = Size;
-  Status = TmpContext->ImageRead (
- TmpContext->Handle,
- TmpContext->PeCoffHeaderOffset,
+  Status = ImageContext->ImageRead (
+ ImageContext->Handle,
+ ImageContext->PeCoffHeaderOffset,
  ,
  Hdr.Pe32
  );
@@ -231,23 +218,28 @@ GetPeCoffSectionInformation (
 return Status;
   }
 
-  ASSERT (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE);
-
-  *SectionHeaderOffset = TmpContext->PeCoffHeaderOffset + sizeof (UINT32) +
-sizeof (EFI_IMAGE_FILE_HEADER);
-  *NumberOfSections= Hdr.Pe32->FileHeader.NumberOfSections;
-
-  

[edk2] [PATCH 3/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for ASCII strings

2019-01-04 Thread Ard Biesheuvel
PE/COFF section names are ASCII strings so use %a not %s.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c | 
2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
index 60c1f66b83fa..3ca7f6660f47 100644
--- 
a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
+++ 
b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
@@ -78,7 +78,7 @@ UpdateMmFoundationPeCoffPermissions (
 "%a: Section %d of image at 0x%lx has 0x%x permissions\n",
 __FUNCTION__, Index, ImageContext->ImageAddress, 
SectionHeader.Characteristics));
 DEBUG ((DEBUG_INFO,
-"%a: Section %d of image at 0x%lx has %s name\n",
+"%a: Section %d of image at 0x%lx has %a name\n",
 __FUNCTION__, Index, ImageContext->ImageAddress, 
SectionHeader.Name));
 DEBUG ((DEBUG_INFO,
 "%a: Section %d of image at 0x%lx has 0x%x address\n",
-- 
2.17.1

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


Re: [edk2] [PATCH v2 04/11] MdePkg/Include: Add StandaloneMmServicesTableLib library

2019-01-04 Thread Ard Biesheuvel
On Thu, 3 Jan 2019 at 17:14, Laszlo Ersek  wrote:
>
> On 01/03/19 12:03, Ard Biesheuvel wrote:
> > On Wed, 2 Jan 2019 at 14:14, Jagadeesh Ujja  wrote:
> >>
> >> Some of the existing DXE drivers can be refactored to execute within
> >> the Standalone MM execution environment as well. Allow such drivers to
> >> get access to the Standalone MM services tables.
> >>
> >> Add a mechanism to determine the execution mode is required.
> >> i.e, in MM or non-MM
> >>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Jagadeesh Ujja 
> >> ---
> >>  MdePkg/Include/Library/StandaloneMmServicesTableLib.h 
> >>| 43 
> >>  
> >> MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.c 
> >>   | 39 ++
> >>  
> >> MdePkg/Library/StandaloneMmServicesTableLib/StandaloneMmServicesTableLib.inf
> >>  | 36 
> >>  MdePkg/MdePkg.dec 
> >>|  4 ++
> >>  4 files changed, 122 insertions(+)
> >>
> >
> > OK, so since the PI spec only refers to MM mode now, this library
> > class should be
> >
> > MmServicesTableLib|Include/Library/MmServicesTableLib.h
> >
> > with an implementation in MdeModulePkg that exposes the deprecated SMM
> > system table as the MM system table.
> >
> > In StandaloneMmPkg, we can add an implementation that exposes the
> > standalone MM system table.
> >
> > (They are binary compatible, so it is just a matter of casting one
> > pointer to the other)
> >
> > With this in place, we can go ahead and update FaultTolerantWrite and
> > Variable SMM driver to switch from SmmServicesTableLib to
> > MmServicesTableLib. This will require existing x86 platforms to define
> > a new library class resolution for MmServicesTableLib, referring to
> > the implementation in MdeModulePkg. This is unfortunate, but it is an
> > unavoidable consequence of the PI spec changes.
>
> It shouldn't be too intrusive or hard to review, I expect.
>
> >
> > Remaining question is what to do with InSmm() ...
>
> I'm lacking the context on this; on the other hand, I can refer back to
> at least one earlier discussion -- there had been multiple -- of the
> discrepancy between the PI spec and the edk2 code. See:
>
> - bullet (9) in
> ,
> - and
> .
>
> Not sure how that can be applied to Arm.
>

The code I posted yesterday does not use InMm() at all. For standalone
MM, it should always return TRUE anyway, and any code that a driver
would execute if it returned FALSE needs to be factored out anyway,
since it should not end up in standalone MM binaries as dead code.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [edk2-announce][RFC] Collaboration Software: Microsoft Teams

2019-01-04 Thread Knop, Ryszard
I think any solution like that should be publicly available for an
unlimited number of users. As it stands now, mailing lists and IRC are
open to all - if software like Microsoft Teams is to be used, it's
pretty likely most communication would quickly become invitation-only
to avoid paying $8 per user. This often happens with many projects
moving from mailing lists/IRC to something like Slack and having
essentially zero public or searchable communications. Perhaps someone
admining the edk2-devel list could look up the amount of subscribers
and chime in?

Thanks, Richard.

On Thu, 2019-01-03 at 20:37 +, Jeremiah Cox via edk2-devel wrote:
> On the topic of Microsoft Teams, it has a free option, limited to 300
> users.  More than 300 users requires an enterprise license, starting
> at $8/user/month.  There is a steep discount for some types of non-
> profits, but I don't know if we would qualify.
> 
> More details on the offering here:  
> https://products.office.com/en-us/microsoft-teams/free
> 
> Highlights:
> * Unlimited chat messages and search.
> * Built-in audio and video calling for individuals, groups, and full
> team meetups.
> * 10 GB of team file storage plus additional 2 GB per person for
> personal storage.
> * Integrated, real-time content creation with Office Online apps,
> including built-in Word, Excel, PowerPoint, and OneNote.
> * Unlimited app integrations with 140+ business apps to choose from-
> including Adobe, * Evernote, and Trello.
> * Ability to communicate and collaborate with anyone inside or
> outside your organization, backed by Microsoft's secure, global
> infrastructure.
> 
> If this sounds like a viable option to the community, please email me
> and I'll add you as a guest on our Project Mu Teams channel for
> evaluation purposes.
> 
> Thanks,
> Jeremiah
> 
> -Original Message-
> From: edk2-devel  On Behalf Of
> stephano
> Sent: Friday, November 16, 2018 8:59 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [edk2-announce][RFC] Collaboration Software
> 
> We are looking to augment our current communication methods (mailing
> list / IRC) with a modern solution for group collaboration. The goal
> is to allow folks to communicate effectively without interrupting the
> current patch review system, as well as enabling any future systems
> with more robust options.
> 
> Specific features we are looking for include attachments (currently
> blocked by the list), robust logging, modern chat, and integration
> with tools like bug trackers and source repositories (APIs, or better
> yet, pre-rolled plugins).
> 
> Our current contenders are Google Groups and Groups.io. This RFC is
> in hopes of finding other options to evaluate.
> 
> Cheers,
> Stephano
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01.org%2Fmailman%2Flistinfo%2Fedk2-develdata=02%7C01%7Cjerecox%40microsoft.com%7Ccced1ed304fa4bb2912508d64be60e56%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636779848794152121sdata=i3HtNOPFa010PWb2qSe4TKnTpsTwzPbQzN3S48uG9Po%3Dreserved=0
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial 
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | 
Kapital zakladowy 200.000 PLN.

Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
jakiekolwiek
przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). If you are not the intended recipient, please 
contact the sender and delete all copies; any review or distribution by
others is strictly prohibited.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH v2 10/11] SecurityPkg/AuthVariableLib: allow MM_STANDALONE drivers to use this library

2019-01-04 Thread Ard Biesheuvel
On Thu, 3 Jan 2019 at 07:15, Jagadeesh Ujja  wrote:
>
> On Thu, Jan 3, 2019 at 6:45 AM Zhang, Chao B  wrote:
> >
> > Reviewed-by : Chao Zhang 
>
> Hi Chao Zhang,
>
> Thanks for the review, I will not be having any new changes with this
> specific patch.
> Can you please merge this patch, so that I will not resubmit with
> other patch set
>

Reviewed-by: Ard Biesheuvel 

Pushed as 8ef653aa5aad..38f3c1b0d274

> >
> > -Original Message-
> > From: Jagadeesh Ujja [mailto:jagadeesh.u...@arm.com]
> > Sent: Wednesday, January 2, 2019 9:14 PM
> > To: edk2-devel@lists.01.org; Gao, Liming ; Zhang, 
> > Chao B ; leif.lindh...@linaro.org; 
> > ard.biesheu...@linaro.org; achin.gu...@arm.com; supreeth.venkat...@arm.com; 
> > Wang, Jian J 
> > Subject: [PATCH v2 10/11] SecurityPkg/AuthVariableLib: allow MM_STANDALONE 
> > drivers to use this library
> >
> > “AuthVariableLib” library can be used by MM_STANDALONE drivers as well.
> > So add MM_STANDALONE as the module type this library supports.
> >
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jagadeesh Ujja 
> > Reviewed-by: Chao Zhang 
> > ---
> >  SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf 
> > b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > index 572ba4e..4294d3b 100644
> > --- a/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > +++ b/SecurityPkg/Library/AuthVariableLib/AuthVariableLib.inf
> > @@ -2,6 +2,7 @@
> >  #  Provides authenticated variable services.
> >  #
> >  #  Copyright (c) 2015 - 2016, Intel Corporation. All rights reserved.
> > +#  Copyright (c) 2018, ARM Limited. All rights reserved.
> >  #
> >  #  This program and the accompanying materials  #  are licensed and made 
> > available under the terms and conditions @@ -21,12 +22,12 @@
> >FILE_GUID  = B23CF5FB-6FCC-4422-B145-D855DBC05457
> >MODULE_TYPE= DXE_RUNTIME_DRIVER
> >VERSION_STRING = 1.0
> > -  LIBRARY_CLASS  = AuthVariableLib|DXE_RUNTIME_DRIVER 
> > DXE_SMM_DRIVER
> > +  LIBRARY_CLASS  = AuthVariableLib|DXE_RUNTIME_DRIVER 
> > DXE_SMM_DRIVER MM_STANDALONE
> >
> >  #
> >  # The following information is for reference only and not required by the 
> > build tools.
> >  #
> > -#  VALID_ARCHITECTURES   = IA32 X64
> > +#  VALID_ARCHITECTURES   = IA32 X64 AARCH64
> >  #
> >
> >  [Sources]
> > --
> > 2.7.4
> >
> > ___
> > 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: [edk2] [PATCH] BaseTools/GenFds: permit stripped MM_CORE_STANDALONE binaries

2019-01-04 Thread Ard Biesheuvel
On Thu, 3 Jan 2019 at 17:05, Carsey, Jaben  wrote:
>
> Reviewed-by: Jaben Carsey 
>
>

Thanks all

Pushed as 672601cfcc6f..8ef653aa5aad

> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> > Ard Biesheuvel
> > Sent: Thursday, January 03, 2019 4:13 AM
> > To: edk2-devel@lists.01.org
> > Cc: Gao, Liming 
> > Subject: [edk2] [PATCH] BaseTools/GenFds: permit stripped
> > MM_CORE_STANDALONE binaries
> >
> > The standalone MM core is executed in place, and resides in a
> > separate execution context which may be space constrained.
> > Since code and data may be mapped with different attributes for
> > security reasons, the PE/COFF binary could have a section
> > alignment of 4 KB.
> >
> > This means that any relocation data is not only useless, but it
> > will also take up 4 KB of valuable space.
> >
> > So add support for the RELOCS_STRIPPED attribute on FFS files of
> > this type, so that we can get rid of the .reloc section altogether.
> > Combined with the FIXED attribute (which enables an optimization
> > in GenFfs that strips redundant padding) and a TE type binary, this
> > gets rid of all the needless padding around the standalone MM core
> > binary.
> >
> > Cc: Bob Feng 
> > Cc: Liming Gao 
> > Cc: Jagadeesh Ujja 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel 
> > ---
> >  BaseTools/Source/Python/GenFds/EfiSection.py  | 2 +-
> >  BaseTools/Source/Python/GenFds/FdfParser.py   | 2 +-
> >  BaseTools/Source/Python/GenFds/FfsInfStatement.py | 2 +-
> >  3 files changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/BaseTools/Source/Python/GenFds/EfiSection.py
> > b/BaseTools/Source/Python/GenFds/EfiSection.py
> > index f8573b5c7d1b..0be176ec8ae1 100644
> > --- a/BaseTools/Source/Python/GenFds/EfiSection.py
> > +++ b/BaseTools/Source/Python/GenFds/EfiSection.py
> > @@ -68,7 +68,7 @@ class EfiSection (EfiSectionClassObject):
> >  StringData = FfsInf.__ExtendMacro__(self.StringData)
> >  ModuleNameStr = FfsInf.__ExtendMacro__('$(MODULE_NAME)')
> >  NoStrip = True
> > -if FfsInf.ModuleType in (SUP_MODULE_SEC,
> > SUP_MODULE_PEI_CORE, SUP_MODULE_PEIM) and SectionType in
> > (BINARY_FILE_TYPE_TE, BINARY_FILE_TYPE_PE32):
> > +if FfsInf.ModuleType in (SUP_MODULE_SEC,
> > SUP_MODULE_PEI_CORE, SUP_MODULE_PEIM,
> > SUP_MODULE_MM_CORE_STANDALONE) and SectionType in
> > (BINARY_FILE_TYPE_TE, BINARY_FILE_TYPE_PE32):
> >  if FfsInf.KeepReloc is not None:
> >  NoStrip = FfsInf.KeepReloc
> >  elif FfsInf.KeepRelocFromRule is not None:
> > diff --git a/BaseTools/Source/Python/GenFds/FdfParser.py
> > b/BaseTools/Source/Python/GenFds/FdfParser.py
> > index e000228d2f6c..de0b166030e7 100644
> > --- a/BaseTools/Source/Python/GenFds/FdfParser.py
> > +++ b/BaseTools/Source/Python/GenFds/FdfParser.py
> > @@ -2589,7 +2589,7 @@ class FdfParser:
> >  #
> >  @staticmethod
> >  def _FileCouldHaveRelocFlag (FileType):
> > -if FileType in {SUP_MODULE_SEC, SUP_MODULE_PEI_CORE,
> > SUP_MODULE_PEIM, 'PEI_DXE_COMBO'}:
> > +if FileType in {SUP_MODULE_SEC, SUP_MODULE_PEI_CORE,
> > SUP_MODULE_PEIM, SUP_MODULE_MM_CORE_STANDALONE,
> > 'PEI_DXE_COMBO'}:
> >  return True
> >  else:
> >  return False
> > diff --git a/BaseTools/Source/Python/GenFds/FfsInfStatement.py
> > b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
> > index c1073c96e9aa..d4c61c074963 100644
> > --- a/BaseTools/Source/Python/GenFds/FfsInfStatement.py
> > +++ b/BaseTools/Source/Python/GenFds/FfsInfStatement.py
> > @@ -901,7 +901,7 @@ class FfsInfStatement(FfsInfStatementClassObject):
> >  #   @retval string   File name of the generated section file
> >  #
> >  def __GenComplexFileSection__(self, Rule, FvChildAddr, FvParentAddr,
> > IsMakefile = False):
> > -if self.ModuleType in (SUP_MODULE_SEC, SUP_MODULE_PEI_CORE,
> > SUP_MODULE_PEIM):
> > +if self.ModuleType in (SUP_MODULE_SEC, SUP_MODULE_PEI_CORE,
> > SUP_MODULE_PEIM, SUP_MODULE_MM_CORE_STANDALONE):
> >  if Rule.KeepReloc is not None:
> >  self.KeepRelocFromRule = Rule.KeepReloc
> >  SectFiles = []
> > --
> > 2.17.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] [patch] MdePkg/BasePeCoffLib: Add more check for relocation data

2019-01-04 Thread Dandan Bi
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1426

In function PeCoffLoaderRelocateImageForRuntime, it doesn't
do much check when applies relocation fixups. For API level
consideration, it's not safe enough.
This patch is to replace the same code logic with calling
function PeCoffLoaderImageAddress which will cover more
check and validation.

Cc: Michael D Kinney 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 29 +++
 1 file changed, 24 insertions(+), 5 deletions(-)

diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c 
b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index c57816a808..ae64705d7c 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -13,11 +13,11 @@
   This library will also do some additional check for PE header fields.
 
   PeCoffLoaderGetPeHeader() routine will do basic check for PE/COFF header.
   PeCoffLoaderGetImageInfo() routine will do basic check for whole PE/COFF 
image.
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
   Portions copyright (c) 2008 - 2009, Apple Inc. 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.
@@ -1669,25 +1669,30 @@ PeCoffLoaderRelocateImageForRuntime (
   UINT32  NumberOfRvaAndSizes;
   EFI_IMAGE_DATA_DIRECTORY*DataDirectory;
   EFI_IMAGE_DATA_DIRECTORY*RelocDir;
   EFI_IMAGE_BASE_RELOCATION   *RelocBase;
   EFI_IMAGE_BASE_RELOCATION   *RelocBaseEnd;
+  EFI_IMAGE_BASE_RELOCATION   *RelocBaseOrig;
   UINT16  *Reloc;
   UINT16  *RelocEnd;
   CHAR8   *Fixup;
   CHAR8   *FixupBase;
   UINT16  *Fixup16;
   UINT32  *Fixup32;
   UINT64  *Fixup64;
   CHAR8   *FixupData;
   UINTN   Adjust;
   RETURN_STATUS   Status;
+  PE_COFF_LOADER_IMAGE_CONTEXTImageContext;
 
   OldBase = (CHAR8 *)((UINTN)ImageBase);
   NewBase = (CHAR8 *)((UINTN)VirtImageBase);
   Adjust = (UINTN) NewBase - (UINTN) OldBase;
 
+  ImageContext.ImageAddress = ImageBase;
+  ImageContext.ImageSize = ImageSize;
+
   //
   // Find the image's relocate dir info
   //
   DosHdr = (EFI_IMAGE_DOS_HEADER *)OldBase;
   if (DosHdr->e_magic == EFI_IMAGE_DOS_SIGNATURE) {
@@ -1730,12 +1735,15 @@ PeCoffLoaderRelocateImageForRuntime (
   // is present in the image. You have to check the NumberOfRvaAndSizes in
   // the optional header to verify a desired directory entry is there.
   //
   if (NumberOfRvaAndSizes > EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC) {
 RelocDir  = DataDirectory + EFI_IMAGE_DIRECTORY_ENTRY_BASERELOC;
-RelocBase = (EFI_IMAGE_BASE_RELOCATION *)(UINTN)(ImageBase + 
RelocDir->VirtualAddress);
-RelocBaseEnd  = (EFI_IMAGE_BASE_RELOCATION *)(UINTN)(ImageBase + 
RelocDir->VirtualAddress + RelocDir->Size);
+RelocBase = (EFI_IMAGE_BASE_RELOCATION *) PeCoffLoaderImageAddress 
(, RelocDir->VirtualAddress, 0);
+RelocBaseEnd  = (EFI_IMAGE_BASE_RELOCATION *) PeCoffLoaderImageAddress 
(,
+
RelocDir->VirtualAddress + RelocDir->Size,
+0
+);
   } else {
 //
 // Cannot find relocations, cannot continue to relocate the image, ASSERT 
for this invalid image.
 //
 ASSERT (FALSE);
@@ -1753,10 +1761,11 @@ PeCoffLoaderRelocateImageForRuntime (
   // since it was relocated. This is so data sections that have been updated
   // by code will not be fixed up, since that would set them back to
   // defaults.
   //
   FixupData = RelocationData;
+  RelocBaseOrig = RelocBase;
   while (RelocBase < RelocBaseEnd) {
 //
 // Add check for RelocBase->SizeOfBlock field.
 //
 if ((RelocBase->SizeOfBlock == 0) || (RelocBase->SizeOfBlock > 
RelocDir->Size)) {
@@ -1766,18 +1775,28 @@ PeCoffLoaderRelocateImageForRuntime (
   return;
 }
 
 Reloc = (UINT16 *) ((UINT8 *) RelocBase + sizeof 
(EFI_IMAGE_BASE_RELOCATION));
 RelocEnd  = (UINT16 *) ((UINT8 *) RelocBase + RelocBase->SizeOfBlock);
-FixupBase = (CHAR8 *) ((UINTN)ImageBase) + RelocBase->VirtualAddress;
+if ((UINTN)RelocEnd > (UINTN)RelocBaseOrig + RelocDir->Size) {
+  return;
+}
+
+FixupBase = 

[edk2] [PATCH edk2-platforms 4/7] Silicon/SynQuacer/Fip006Dxe: use proper accessor for unaligned access

2019-01-04 Thread Ard Biesheuvel
This code may execute in SMM context, where unaligned accesses are
not permitted. So use ReadUnaligned32() instead of performing a
direct UINT32* cast.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c 
b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
index 2134739bfba9..d45c8d9b35d2 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
@@ -841,7 +841,7 @@ NorFlashWriteSingleBlock (
BlockSize);
 
   // The word of data that is to be written.
-  TmpBuf = *((UINT32*)(Buffer + (*NumBytes - BytesToWrite)));
+  TmpBuf = ReadUnaligned32 ((UINT32 *)(Buffer + (*NumBytes - 
BytesToWrite)));
 
   // First do word aligned chunks.
   if ((CurOffset & 0x3) == 0) {
-- 
2.17.1

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


[edk2] [PATCH edk2-platforms 2/7] Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces

2019-01-04 Thread Ard Biesheuvel
In preparation of creating a SMM version of the FIP006 NOR flash
driver, refactor the existing pieces into a core driver, the FVB
methods and the DXE instantiation code.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
   |6 +-
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c   
   | 1006 +
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashDxe.h => NorFlash.h}
   |   52 +-
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c
   | 1150 +++-
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashFvbDxe.c => 
NorFlashFvb.c} |  161 +--
 5 files changed, 1194 insertions(+), 1181 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf 
b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
index b939aa689eef..603641e0a68f 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
@@ -2,7 +2,7 @@
 #  Socionext FIP006 High-Speed SPI Controller with NOR Flash Driver
 #
 #  Copyright (c) 2017, Socionext Inc. All rights reserved.
-#  Copyright (c) 2017, Linaro, Ltd. All rights reserved.
+#  Copyright (c) 2017-2018, Linaro, Ltd. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -25,7 +25,9 @@ [Defines]
 
 [Sources]
   NorFlashDxe.c
-  NorFlashFvbDxe.c
+  NorFlash.c
+  NorFlash.h
+  NorFlashFvb.c
 
 [Packages]
   ArmPlatformPkg/ArmPlatformPkg.dec
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c 
b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
new file mode 100644
index ..2134739bfba9
--- /dev/null
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
@@ -0,0 +1,1006 @@
+/** @file  NorFlashDxe.c
+
+  Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Socionext Inc. All rights reserved.
+  Copyright (c) 2017, Linaro, 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 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include "NorFlash.h"
+
+STATIC CONST UINT16 mFip006NullCmdSeq[] = {
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+  CSDC (CSDC_END, CSDC_CONT_NON_CONTINUOUS, CSDC_TRP_MBM, CSDC_DEC_DECODE),
+};
+
+STATIC CONST CSDC_DEFINITION mN25qCSDCDefTable[] = {
+  // Identification Operations
+  { SPINOR_OP_RDID, FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+CSDC_TRP_SINGLE },
+  // Register Operations
+  { SPINOR_OP_RDSR, FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+CSDC_TRP_SINGLE },
+  { SPINOR_OP_WRSR, FALSE, FALSE, FALSE, TRUE,  CS_CFG_MBM_SINGLE,
+CSDC_TRP_SINGLE },
+  { SPINOR_OP_RD_ARRAY, TRUE,  FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+CSDC_TRP_SINGLE },
+  { SPINOR_OP_RDFSR,FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+CSDC_TRP_SINGLE },
+  { SPINOR_OP_RD_NVCFG, FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+CSDC_TRP_SINGLE },
+  { SPINOR_OP_RD_VCR,   FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+CSDC_TRP_SINGLE },
+  { SPINOR_OP_RD_EVCR,  FALSE, FALSE, FALSE, FALSE, CS_CFG_MBM_SINGLE,
+CSDC_TRP_SINGLE },
+  // Read Operations
+  { SPINOR_OP_READ_4B,  TRUE,  TRUE,  FALSE, FALSE, CS_CFG_MBM_SINGLE,
+CSDC_TRP_SINGLE },
+  // Write Operations
+  { SPINOR_OP_PP,   TRUE,  FALSE, FALSE, TRUE,  CS_CFG_MBM_SINGLE,
+CSDC_TRP_SINGLE },
+  { SPINOR_OP_PP_1_1_4, TRUE,  FALSE, FALSE, TRUE,  CS_CFG_MBM_QUAD,
+CSDC_TRP_SINGLE },
+  // Erase Operations
+  { SPINOR_OP_SE,   FALSE, FALSE, FALSE, TRUE,  CS_CFG_MBM_SINGLE,
+CSDC_TRP_SINGLE },
+};
+
+STATIC CONST 

[edk2] [PATCH edk2-platforms 7/7] Platform/DeveloperBox: add MM based UEFI secure boot support

2019-01-04 Thread Ard Biesheuvel
This implements support for UEFI secure boot on DeveloperBox 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.

Note that SynQuacer as configured today is not a truly secure
platform, since the NOR flash registers are accessible to the
non-secure world. However, from a software point of view, all
of the required pieces are in place. (In particular, it is no
longer possible for the OS to stub out authentication checks
in the validation code residing in RuntimeServicesCode regions)

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 23 +++-
 Platform/Socionext/DeveloperBox/DeveloperBox.fdf | 13 +++
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc 
b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index 666bd2716336..d244048c5a6b 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -28,6 +28,8 @@ [Defines]
   FLASH_DEFINITION   = 
Platform/Socionext/DeveloperBox/DeveloperBox.fdf
   BUILD_NUMBER   = 1
 
+  DEFINE SECURE_BOOT_ENABLE  = FALSE
+
 !include Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
 
 
[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
@@ -165,6 +167,13 @@ [PcdsFixedAtBuild]
   g96BoardsTokenSpaceGuid.PcdGpioPinK|24
   g96BoardsTokenSpaceGuid.PcdGpioPinL|25
 
+  gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC0
+  gArmTokenSpaceGuid.PcdMmBufferSize|0x0020
+
+  gEfiSecurityPkgTokenSpaceGuid.PcdOptionRomImageVerificationPolicy|0x04
+  gEfiSecurityPkgTokenSpaceGuid.PcdFixedMediaImageVerificationPolicy|0x04
+  gEfiSecurityPkgTokenSpaceGuid.PcdRemovableMediaImageVerificationPolicy|0x04
+
 [PcdsDynamicExDefault.common.DEFAULT]
   
gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareImageDescriptor|{0x0}|VOID*|0x100
   gEfiSignedCapsulePkgTokenSpaceGuid.PcdEdkiiSystemFirmwareFileGuid|{0xf7, 
0x89, 0x9b, 0xe9, 0x20, 0xc1, 0x25, 0x4b, 0x4d, 0xb1, 0x83, 0x94, 0xed, 0xb0, 
0xb4, 0xf5}
@@ -223,7 +232,13 @@ [Components.common]
   }
   MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
-  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf
+  MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
+!if $(SECURE_BOOT_ENABLE) == TRUE
+
+  
NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
+!endif
+  }
+
   ArmPkg/Drivers/TimerDxe/TimerDxe.inf
   ArmPkg/Drivers/GenericWatchdogDxe/GenericWatchdogDxe.inf
   MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
@@ -251,6 +266,7 @@ [Components.common]
   # Variable services
   #
   Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
+!if $(SECURE_BOOT_ENABLE) == FALSE
   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
 
@@ -260,6 +276,11 @@ [Components.common]
   
TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
   VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf
   }
+!else
+  ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
+  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
+!endif
 
   #
   # UEFI application (Shell Embedded Boot Loader)
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf 
b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf
index 4a234a36525e..7be40380efb4 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.fdf
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.fdf
@@ -51,7 +51,11 @@ [FD.SPI_NOR_IMAGE]
 

 
 0x|0x00078000
+!if $(SECURE_BOOT_ENABLE) == FALSE
 FILE = Platform/Socionext/DeveloperBox/fip_all_arm_tf.bin
+!else
+FILE = Platform/Socionext/DeveloperBox/fip_all_arm_tf_mm.bin
+!endif
 
 0x00078000|0x8000
 FILE = 
$(OUTPUT_DIRECTORY)/$(TARGET)_$(TOOL_CHAIN_TAG)/$(ARCH)/Silicon/Socionext/SynQuacer/Stage2Tables/Stage2Tables/OUTPUT/Stage2Tables.bin
@@ -122,9 +126,15 @@ [FV.FvMain]
   #
   # Variable services
   #
+!if $(SECURE_BOOT_ENABLE) == FALSE
   INF Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
   INF MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
   INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+!else
+  INF ArmPkg/Drivers/MmCommunicationDxe/MmCommunication.inf
+  INF MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmmRuntimeDxe.inf
+  INF 

[edk2] [PATCH edk2-platforms 6/7] Platform/DeveloperBox: add .DSC/.FDF description of MM components

2019-01-04 Thread Ard Biesheuvel
Create a pair of .DSC/.FDF files that describe the components and
the firmware volumes and flash device that will be dispatched into
a secure partition in the secure world to control the UEFI secure
variable store.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc |   5 +-
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc |   7 +-
 Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc   | 103 +
 Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf   | 161 
 4 files changed, 270 insertions(+), 6 deletions(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc 
b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index 56787a744157..666bd2716336 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -250,10 +250,7 @@ [Components.common]
   #
   # Variable services
   #
-  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf {
-
-  
NorFlashPlatformLib|Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacerLib.inf
-  }
+  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
   MdeModulePkg/Universal/FaultTolerantWriteDxe/FaultTolerantWriteDxe.inf
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
 
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc 
b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
index 56adc21d5caf..87b2094cb356 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
@@ -125,6 +125,7 @@ [LibraryClasses.common]
   
PlatformSecureLib|SecurityPkg/Library/PlatformSecureLibNull/PlatformSecureLibNull.inf
 
   NorFlashInfoLib|EmbeddedPkg/Library/NorFlashInfoLib/NorFlashInfoLib.inf
+  
NorFlashPlatformLib|Silicon/Socionext/SynQuacer/Library/NorFlashSynQuacerLib/NorFlashSynQuacerLib.inf
 
 

 #
@@ -294,8 +295,10 @@ [PcdsFixedAtBuild.common]
 !endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdFirmwareRevision|$(BUILD_NUMBER)
 
-  gArmTokenSpaceGuid.PcdMmBufferBase|0xFFC0
-  gArmTokenSpaceGuid.PcdMmBufferSize|0x0020
+  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
+
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800
 
 [PcdsPatchableInModule]
   gEfiMdeModulePkgTokenSpaceGuid.PcdVideoHorizontalResolution|0
diff --git a/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc 
b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc
new file mode 100644
index ..45cfd5645d9b
--- /dev/null
+++ b/Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc
@@ -0,0 +1,103 @@
+#
+#  Copyright (c) 2013-2014, ARM Limited. All rights reserved.
+#  Copyright (c) 2017-2018, Linaro Limited. 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 Section - statements that will be processed to create a Makefile.
+#
+
+[Defines]
+  PLATFORM_NAME  = DeveloperBoxMm
+  PLATFORM_GUID  = cedcc3f9-fb42-45e6-b134-e7ca97abbffd
+  PLATFORM_VERSION   = 0.1
+  DSC_SPECIFICATION  = 0x0001001B
+  OUTPUT_DIRECTORY   = Build/$(PLATFORM_NAME)
+  SUPPORTED_ARCHITECTURES= AARCH64
+  BUILD_TARGETS  = DEBUG|RELEASE|NOOPT
+  SKUID_IDENTIFIER   = DEFAULT
+  FLASH_DEFINITION   = 
Platform/Socionext/DeveloperBox/$(PLATFORM_NAME).fdf
+  BUILD_NUMBER   = 1
+
+!include Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
+
+[LibraryClasses.common.MM_STANDALONE, LibraryClasses.common.MM_CORE_STANDALONE]
+  ArmSvcLib|ArmPkg/Library/ArmSvcLib/ArmSvcLib.inf
+  BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
+  FvLib|StandaloneMmPkg/Library/FvLib/FvLib.inf
+  MemLib|StandaloneMmPkg/Library/StandaloneMmMemLib/StandaloneMmMemLib.inf
+  
PeCoffExtraActionLib|StandaloneMmPkg/Library/StandaloneMmPeCoffExtraActionLib/StandaloneMmPeCoffExtraActionLib.inf
+  
StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
+  
StandaloneMmDriverEntryPoint|StandaloneMmPkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf
+  

[edk2] [PATCH edk2-platforms 3/7] Silicon/SynQuacer/Fip006Dxe: implement standalone MM variant

2019-01-04 Thread Ard Biesheuvel
Implement a variant of the FIP006 NOR flash driver that can execute
in standalone MM context. This is the foundation for hosting the
EFI authenticated variable store in the secure world.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf |  71 

 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c  | 182 

 2 files changed, 253 insertions(+)

diff --git 
a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf 
b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf
new file mode 100644
index ..2dcbfd7db892
--- /dev/null
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf
@@ -0,0 +1,71 @@
+## @file
+#  Socionext FIP006 High-Speed SPI Controller with NOR Flash Driver
+#
+#  Copyright (c) 2017, Socionext Inc. All rights reserved.
+#  Copyright (c) 2017-2018, Linaro, 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  = Fip006Dxe
+  FILE_GUID  = 1b041d85-9b44-442b-a583-5cf008ef9060
+  MODULE_TYPE= MM_STANDALONE
+  VERSION_STRING = 0.1
+  PI_SPECIFICATION_VERSION   = 0x00010032
+  ENTRY_POINT= NorFlashInitialise
+
+[Sources]
+  NorFlashSmm.c
+  NorFlash.c
+  NorFlash.h
+  NorFlashFvb.c
+
+[Packages]
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.dec
+  StandaloneMmPkg/StandaloneMmPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  IoLib
+  MemoryAllocationLib
+  MmServicesTableLib
+  NorFlashInfoLib
+  NorFlashPlatformLib
+  StandaloneMmDriverEntryPoint
+
+[Guids]
+  gEfiAuthenticatedVariableGuid
+  gEfiSystemNvDataFvGuid
+  gEfiVariableGuid
+
+[Protocols]
+  gEfiSmmFirmwareVolumeBlockProtocolGuid
+
+[FixedPcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
+  gFip006DxeTokenSpaceGuid.PcdFip006DxeRegBaseAddress
+  gFip006DxeTokenSpaceGuid.PcdFip006DxeMemBaseAddress
+
+[Depex]
+  TRUE
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c 
b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c
new file mode 100644
index ..bab3d9f4cd14
--- /dev/null
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c
@@ -0,0 +1,182 @@
+/** @file  NorFlashSmm.c
+
+  Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.
+  Copyright (c) 2017, Socionext Inc. All rights reserved.
+  Copyright (c) 2017, Linaro, 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 
+
+#include 
+
+#include "NorFlash.h"
+
+//
+// Global variable declarations
+//
+STATIC NOR_FLASH_INSTANCE   **mNorFlashInstances;
+STATIC UINT32   mNorFlashDeviceCount;
+
+EFI_STATUS
+EFIAPI
+NorFlashFvbInitialize (
+  IN NOR_FLASH_INSTANCE* Instance
+  )
+{
+  EFI_STATUS  Status;
+  UINT32  FvbNumLba;
+  UINTN   BlockSize;
+
+  DEBUG ((DEBUG_BLKIO,"NorFlashFvbInitialize\n"));
+
+  BlockSize = Instance->BlockSize;
+
+  // FirmwareVolumeHeader->FvLength is declared to have the Variable area
+  // AND the FTW working area AND the FTW Spare contiguous.
+  ASSERT (PcdGet32 (PcdFlashNvStorageVariableBase) +
+  PcdGet32 (PcdFlashNvStorageVariableSize) ==
+  PcdGet32 (PcdFlashNvStorageFtwWorkingBase));
+  ASSERT (PcdGet32 (PcdFlashNvStorageFtwWorkingBase) +
+  PcdGet32 (PcdFlashNvStorageFtwWorkingSize) ==
+  PcdGet32 (PcdFlashNvStorageFtwSpareBase));
+
+  // Check if the size of the area is at least one block size
+  ASSERT ((PcdGet32 (PcdFlashNvStorageVariableSize) > 0) 

[edk2] [PATCH edk2-platforms 0/7] Silicon/SynQuacer: implement SMM based secure boot

2019-01-04 Thread Ard Biesheuvel
Wire up the various pieces so that the authenticated variable store
runs entirely in standalone MM context residing in a secure partition.

This primarily involves refactoring the platform's NOR flash driver so
we can build a version that can work in the standalone MM context.
Beyond that, it is just a matter of enabling all the boilerplate in
the .DSC and .FDF files.

Note that the resulting standalone MM firmware volume needs to be
wrapped in a FIP, which is not part of the build sequence.

Cc: Leif Lindholm 
Cc: Masahisa Kojima 

Ard Biesheuvel (7):
  Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines
  Silicon/SynQuacer/Fip006Dxe: factor out DXE specific pieces
  Silicon/SynQuacer/Fip006Dxe: implement standalone MM variant
  Silicon/SynQuacer/Fip006Dxe: use proper accessor for unaligned access
  Platform/DeveloperBox: create shared .DSC include file
  Platform/DeveloperBox: add .DSC/.FDF description of MM components
  Platform/DeveloperBox: add MM based UEFI secure boot support

 .../Socionext/DeveloperBox/DeveloperBox.dsc   |  304 +---
 .../DeveloperBox/DeveloperBox.dsc.inc |  315 
 .../Socionext/DeveloperBox/DeveloperBox.fdf   |   13 +
 .../Socionext/DeveloperBox/DeveloperBoxMm.dsc |  103 ++
 .../Socionext/DeveloperBox/DeveloperBoxMm.fdf |  161 ++
 .../SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf |9 +-
 .../Drivers/Fip006Dxe/Fip006StandaloneMm.inf  |   71 +
 .../SynQuacer/Drivers/Fip006Dxe/NorFlash.c| 1006 +
 .../Fip006Dxe/{NorFlashDxe.h => NorFlash.h}   |   93 +-
 .../Drivers/Fip006Dxe/NorFlashBlockIoDxe.c|  138 --
 .../SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c | 1341 ++---
 .../{NorFlashFvbDxe.c => NorFlashFvb.c}   |  197 +--
 .../SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c |  182 +++
 13 files changed, 2076 insertions(+), 1857 deletions(-)
 create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
 create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBoxMm.dsc
 create mode 100644 Platform/Socionext/DeveloperBox/DeveloperBoxMm.fdf
 create mode 100644 
Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006StandaloneMm.inf
 create mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlash.c
 rename Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashDxe.h => 
NorFlash.h} (85%)
 delete mode 100644 
Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c
 rename Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/{NorFlashFvbDxe.c => 
NorFlashFvb.c} (76%)
 create mode 100644 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashSmm.c

-- 
2.17.1

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


[edk2] [PATCH edk2-platforms 1/7] Silicon/SynQuacer/Fip006Dxe: drop block I/O and disk I/O routines

2019-01-04 Thread Ard Biesheuvel
The FIP006 NOR flash driver contains implementations of the block I/O
and disk I/O protocols, but never exposes them to other drivers (i.e.,
it never installs the protocol interfaces). So let's drop this code
altogether: the NOR flash is for code and variables, not for arbitrary
files.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf|   3 -
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c | 138 

 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.c| 229 
++--
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashDxe.h|  51 +
 Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashFvbDxe.c |  40 ++--
 5 files changed, 37 insertions(+), 424 deletions(-)

diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf 
b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
index bddb052c2dcc..b939aa689eef 100644
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
+++ b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/Fip006Dxe.inf
@@ -24,7 +24,6 @@ [Defines]
   ENTRY_POINT= NorFlashInitialise
 
 [Sources]
-  NorFlashBlockIoDxe.c
   NorFlashDxe.c
   NorFlashFvbDxe.c
 
@@ -60,9 +59,7 @@ [Guids]
   gEfiVariableGuid
 
 [Protocols]
-  gEfiBlockIoProtocolGuid
   gEfiDevicePathProtocolGuid
-  gEfiDiskIoProtocolGuid
   gEfiFirmwareVolumeBlockProtocolGuid
 
 [FixedPcd]
diff --git a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c 
b/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c
deleted file mode 100644
index b41f5003217c..
--- a/Silicon/Socionext/SynQuacer/Drivers/Fip006Dxe/NorFlashBlockIoDxe.c
+++ /dev/null
@@ -1,138 +0,0 @@
-/** @file  NorFlashBlockIoDxe.c
-
-  Copyright (c) 2011-2013, ARM Ltd. All rights reserved.
-  Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.
-  Copyright (c) 2017, Linaro, 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 
-#include 
-
-#include "NorFlashDxe.h"
-
-//
-// BlockIO Protocol function EFI_BLOCK_IO_PROTOCOL.Reset
-//
-EFI_STATUS
-EFIAPI
-NorFlashBlockIoReset (
-  IN EFI_BLOCK_IO_PROTOCOL  *This,
-  IN BOOLEANExtendedVerification
-  )
-{
-  NOR_FLASH_INSTANCE *Instance;
-
-  Instance = INSTANCE_FROM_BLKIO_THIS(This);
-
-  DEBUG ((DEBUG_BLKIO, "NorFlashBlockIoReset(MediaId=0x%x)\n",
-This->Media->MediaId));
-
-  return NorFlashReset (Instance);
-}
-
-//
-// BlockIO Protocol function EFI_BLOCK_IO_PROTOCOL.ReadBlocks
-//
-EFI_STATUS
-EFIAPI
-NorFlashBlockIoReadBlocks (
-  IN  EFI_BLOCK_IO_PROTOCOL   *This,
-  IN  UINT32  MediaId,
-  IN  EFI_LBA Lba,
-  IN  UINTN   BufferSizeInBytes,
-  OUT VOID*Buffer
-  )
-{
-  NOR_FLASH_INSTANCE  *Instance;
-  EFI_STATUS  Status;
-  EFI_BLOCK_IO_MEDIA  *Media;
-
-  if (This == NULL) {
-return EFI_INVALID_PARAMETER;
-  }
-
-  Instance = INSTANCE_FROM_BLKIO_THIS(This);
-  Media = This->Media;
-
-  DEBUG ((DEBUG_BLKIO,
-"NorFlashBlockIoReadBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes "
-"(%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, 
Buffer));
-
-  if (!Media) {
-Status = EFI_INVALID_PARAMETER;
-  } else if (!Media->MediaPresent) {
-Status = EFI_NO_MEDIA;
-  } else if (Media->MediaId != MediaId) {
-Status = EFI_MEDIA_CHANGED;
-  } else if ((Media->IoAlign > 2) &&
- (((UINTN)Buffer & (Media->IoAlign - 1)) != 0)) {
-Status = EFI_INVALID_PARAMETER;
-  } else {
-Status = NorFlashReadBlocks (Instance, Lba, BufferSizeInBytes, Buffer);
-  }
-
-  return Status;
-}
-
-//
-// BlockIO Protocol function EFI_BLOCK_IO_PROTOCOL.WriteBlocks
-//
-EFI_STATUS
-EFIAPI
-NorFlashBlockIoWriteBlocks (
-  IN  EFI_BLOCK_IO_PROTOCOL   *This,
-  IN  UINT32  MediaId,
-  IN  EFI_LBA Lba,
-  IN  UINTN   BufferSizeInBytes,
-  IN  VOID*Buffer
-  )
-{
-  NOR_FLASH_INSTANCE  *Instance;
-  EFI_STATUS  Status;
-
-  Instance = INSTANCE_FROM_BLKIO_THIS(This);
-
-  DEBUG ((DEBUG_BLKIO,
-"NorFlashBlockIoWriteBlocks(MediaId=0x%x, Lba=%ld, BufferSize=0x%x bytes "
-"(%d kB), BufferPtr @ 0x%08x)\n", MediaId, Lba, BufferSizeInBytes, 
Buffer));
-
-  if( !This->Media->MediaPresent ) {
-Status = EFI_NO_MEDIA;
-  } else if( This->Media->MediaId != MediaId ) {
-Status = EFI_MEDIA_CHANGED;
-  } else if( This->Media->ReadOnly ) {
-Status = 

[edk2] [PATCH edk2-platforms 5/7] Platform/DeveloperBox: create shared .DSC include file

2019-01-04 Thread Ard Biesheuvel
We are going to add a separate .DSC/.FDF combo for the standalone
MM components. So put all the pieces we will share in an include
file that both .DSC files can include.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc | 280 +-
 Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc | 312 
 2 files changed, 317 insertions(+), 275 deletions(-)

diff --git a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc 
b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
index ed11aed798b7..56787a744157 100644
--- a/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
+++ b/Platform/Socionext/DeveloperBox/DeveloperBox.dsc
@@ -28,11 +28,7 @@ [Defines]
   FLASH_DEFINITION   = 
Platform/Socionext/DeveloperBox/DeveloperBox.fdf
   BUILD_NUMBER   = 1
 
-[BuildOptions]
-  RELEASE_*_*_CC_FLAGS  = -DMDEPKG_NDEBUG -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=0
-
-  # add ample padding to the DTC so we can apply 96boards mezzanine overlays
-  *_*_*_DTC_FLAGS = -p 1024
+!include Platform/Socionext/DeveloperBox/DeveloperBox.dsc.inc
 
 
[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
@@ -41,101 +37,6 @@ [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
   GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
   GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x1
 
-[LibraryClasses.common]
-  
ArmPlatformLib|Silicon/Socionext/SynQuacer/Library/SynQuacerLib/SynQuacerLib.inf
-  ArmHvcLib|ArmPkg/Library/ArmHvcLib/ArmHvcLib.inf
-  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
-
-  TimerLib|ArmPkg/Library/ArmArchTimerLib/ArmArchTimerLib.inf
-  FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
-
-!if $(TARGET) == RELEASE
-  DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
-!else
-  DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
-!endif
-  
DebugPrintErrorLevelLib|MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf
-
-  BaseMemoryLib|MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
-
-  # Networking Requirements
-  NetLib|MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
-  DpcLib|MdeModulePkg/Library/DxeDpcLib/DxeDpcLib.inf
-  UdpIoLib|MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
-  IpIoLib|MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
-
-  # ARM Architectural Libraries
-  
CacheMaintenanceLib|ArmPkg/Library/ArmCacheMaintenanceLib/ArmCacheMaintenanceLib.inf
-  
DefaultExceptionHandlerLib|ArmPkg/Library/DefaultExceptionHandlerLib/DefaultExceptionHandlerLib.inf
-  CpuExceptionHandlerLib|ArmPkg/Library/ArmExceptionLib/ArmExceptionLib.inf
-  ArmDisassemblerLib|ArmPkg/Library/ArmDisassemblerLib/ArmDisassemblerLib.inf
-  ArmLib|ArmPkg/Library/ArmLib/ArmBaseLib.inf
-  ArmMmuLib|ArmPkg/Library/ArmMmuLib/ArmMmuBaseLib.inf
-  ArmGicLib|ArmPkg/Drivers/ArmGic/ArmGicLib.inf
-  ArmGicArchLib|ArmPkg/Library/ArmGicArchLib/ArmGicArchLib.inf
-  
ArmPlatformStackLib|ArmPlatformPkg/Library/ArmPlatformStackLib/ArmPlatformStackLib.inf
-  ArmSmcLib|ArmPkg/Library/ArmSmcLib/ArmSmcLib.inf
-  
ArmGenericTimerCounterLib|ArmPkg/Library/ArmGenericTimerPhyCounterLib/ArmGenericTimerPhyCounterLib.inf
-  OpteeLib|ArmPkg/Library/OpteeLib/OpteeLib.inf
-
-  BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
-  BmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf
-  SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
-  
SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf
-  
PerformanceLib|MdePkg/Library/BasePerformanceLibNull/BasePerformanceLibNull.inf
-  PrintLib|MdePkg/Library/BasePrintLib/BasePrintLib.inf
-  
PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
-  PeCoffLib|MdePkg/Library/BasePeCoffLib/BasePeCoffLib.inf
-  IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf
-  
UefiDecompressLib|MdePkg/Library/BaseUefiDecompressLib/BaseUefiDecompressLib.inf
-  CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf
-  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
-
-  UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
-  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
-  
UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
-  
DevicePathLib|MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLibDevicePathProtocol.inf
-  
UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
-  
DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
-  
UefiDriverEntryPoint|MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf
-  
UefiApplicationEntryPoint|MdePkg/Library/UefiApplicationEntryPoint/UefiApplicationEntryPoint.inf
-  

[edk2] EDK II Network Stack Issue

2019-01-04 Thread Karin Willers

G'Day!

I'm trying to get networking under edk2 up and running. I tried 
AppPkg/Applications/Sockets/RawIp4Tx
under OVMF. The raw packet is sent out on the network, but the 
application never returns from the

socket close routine.

I'm currently using UDK2017 with the latest security patches (downloaded 
December 18 2018). The network

driver under OVMF is the e1000 driver.

The effect that the socket close never returns is also visible when 
running RawIp4Tx on real hardware,

so I think the behavior has nothing to do with OVMF or the UEFI itself.

Does anyone see similar effects? Any hints on setting up networking 
under edk2 correctly?


Thanks,  Karin
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] Uninstalling Invalid Protocol Interfaces

2019-01-04 Thread Kinney, Michael D
Ashish,

Thanks for the pointer.  I agree there is an issue here.

Please enter a Bugzilla against the IScsiDxe module for this issue so we can 
fix this failure.

You are also welcome to enter a Bugzilla for a feature request to add UefiLib 
APIs that can be used to safely uninstall all the driver model related protocol 
that can be used in error paths in the entry point and in unload handlers.

Thanks,

Mike

From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
Sent: Thursday, January 3, 2019 4:16 PM
To: Kinney, Michael D ; edk2-devel@lists.01.org
Cc: Gao, Liming ; Fu, Siyuan ; Wu, 
Jiaxin 
Subject: RE: Uninstalling Invalid Protocol Interfaces

Hi Mike,

Call to UninstallMultipleProtocolInterfaces at 
https://github.com/tianocore/edk2/blob/master/NetworkPkg/IScsiDxe/IScsiDriver.c#L1864
 fails because the component name interface may not be installed depending on 
the state of PCD.

Thanks
Ashish

From: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Sent: Thursday, January 3, 2019 5:09 PM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>; 
edk2-devel@lists.01.org; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Cc: Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan 
mailto:siyuan...@intel.com>>; Wu, Jiaxin 
mailto:jiaxin...@intel.com>>
Subject: RE: Uninstalling Invalid Protocol Interfaces

Hi Ashish,

Can you provide a pointer to the UninstallMultipleProtocolInterfaces() call 
that is causing this failure?

I agree that the UefiLib could provide additional services to simplify the 
implementation of the unload handlers.  Matching the UefiLib APIs that install 
the Uefi Driver Model protocol would be useful, so the driver entry point and 
the driver unload functions can use matching APIs.

Mike

From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
Sent: Thursday, January 3, 2019 3:39 PM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>; Gao, Liming 
mailto:liming@intel.com>>; Fu, Siyuan 
mailto:siyuan...@intel.com>>; Wu, Jiaxin 
mailto:jiaxin...@intel.com>>
Subject: Uninstalling Invalid Protocol Interfaces

Hello,

As part of moving from MdeModulePkg implementation of IScsiDxe to the 
implementation in NetworkPkg, I started hitting exception in the driver loaded 
after IScsiDxe if IScsiDxe's installation fails for some reason. Upon 
debugging, I found out that calls to UninstallMultipleProtocolInterfaces as 
part of Error 2 as well Error 1 fail while trying to uninstall component name 
protocol-interface pair and return error code EFI_INVALID_PARAMETER. As per 
UninstallMultipleProtocolInterfaces's documentation, if uninstallation of any 
of the input protocol-interface pair fails, it will reinstall any just 
uninstalled protocol and return EFI_INVALID_PARAMETER which causes the cleanup 
of this driver corrupt leading to failure in next driver getting loaded. The 
reason of failure in UninstallMultipleProtocolInterfaces is because the driver 
uses EfiLibInstallDriverBindingComponentName2 to install interfaces which may 
not install component name interfaces depending on the value of PCDs 
PcdComponentNameDi
 sable and PcdComponentName2Disable. I have the following proposals to get 
around this issue.


  1.  Instead of calling UninstallMultipleProtocolInterfaces once and list all 
protocol-interface pairs, do it sequentially for every pair so that the once 
which was installed correctly, gets uninstalled instead of getting reinstalled 
because of a failure uninstalling another pair. This would however make us not 
really use use UninstallMultipleProtocolInterfaces API to its full caliber.
  2.  In UEFILib, add a function EfiLibUninstallDriverBindingComponentName2 for 
uninstalling protocol interfaces taking into consideration the state of PCDs 
PcdComponentNameDisable and PcdComponentName2Disable.
  3.  In UEFILib, add uninstall functions for all corresponding install APIs to 
get coverage for all scenarios.

I would certainly prefer option 2 or 3 as they seem to be more correct and 
would provide a for all drivers which may hit the issue. I am happy to make the 
code changes as needed and suggested by the maintainers.

Thanks
Ashish

This email message is for the sole use of the intended recipient(s) and may 
contain confidential information.  Any unauthorized review, use, disclosure or 
distribution is prohibited.  If you are not the intended recipient, please 
contact the sender by reply email and destroy all copies of the original 
message.

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


[edk2] [PATCH 2/2] ArmPkg/ArmMmuLib ARM: fix thinko in second level page table handling

2019-01-04 Thread Ard Biesheuvel
PopulateLevel2PageTable () is invoked for [parts of] mappings that
start or end on a non-1 MB aligned address (or both). The size of
the mapping depends on both the start address modulo 1 MB and the
length of the mapping, but the logic that calculates this size is
flawed: subtracting 'start address modulo 1 MB' could result in a
negative value for the remaining length, which is obviously wrong.

So instead, take either RemainLength, or the rest of the 1 MB
block, whichever is smaller.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c 
b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index b237321a8d8b..3b3b20aa9b78 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -294,8 +294,8 @@ FillTranslationTable (
   PhysicalBase += TT_DESCRIPTOR_SECTION_SIZE;
   RemainLength -= TT_DESCRIPTOR_SECTION_SIZE;
 } else {
-  PageMapLength = MIN (RemainLength, TT_DESCRIPTOR_SECTION_SIZE) -
-  (PhysicalBase % TT_DESCRIPTOR_SECTION_SIZE);
+  PageMapLength = MIN (RemainLength, TT_DESCRIPTOR_SECTION_SIZE -
+ (PhysicalBase % 
TT_DESCRIPTOR_SECTION_SIZE));
 
   // Case: Physical address aligned on the Section Size (1MB) && the length
   //   does not fill a section
-- 
2.17.1

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


Re: [edk2] [PATCH 4/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: remove bogus ASSERT_EFI_ERROR()s

2019-01-04 Thread Supreeth Venkatesh
On Fri, 2019-01-04 at 12:03 +0100, Ard Biesheuvel wrote:
> ASSERT_EFI_ERROR (x) is a shorthand for ASSERT(!EFI_ERROR(x)), and so
> it should only be used with EFI_STATUS type expressions.
> 
> So drop two instances that operate on other types, since neither
> looks
> particularly useful.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
Reviewed-by: Supreeth Venkatesh 

> ---
>  StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalon
> eMmCoreEntryPoint.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalo
> neMmCoreEntryPoint.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalo
> neMmCoreEntryPoint.c
> index 05ed6c8dd0b5..5cca532456fd 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalo
> neMmCoreEntryPoint.c
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/Standalo
> neMmCoreEntryPoint.c
> @@ -295,7 +295,6 @@ _ModuleEntryPoint (
>//
>ProcessModuleEntryPointList (HobStart);
>  
> -  ASSERT_EFI_ERROR (CpuDriverEntryPoint);
>DEBUG ((DEBUG_INFO, "Shared Cpu Driver EP 0x%lx\n", (UINT64)
> CpuDriverEntryPoint));
>  
>  finish:
> @@ -303,5 +302,4 @@ finish:
>InitMmFoundationSvcArgs.Arg0 =
> ARM_SVC_ID_SP_EVENT_COMPLETE_AARCH64;
>InitMmFoundationSvcArgs.Arg1 = Status;
>DelegatedEventLoop ();
> -  ASSERT_EFI_ERROR (0);
>  }

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


Re: [edk2] [PATCH 2/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: add missing SerialPortLib ref

2019-01-04 Thread Supreeth Venkatesh
On Fri, 2019-01-04 at 12:03 +0100, Ard Biesheuvel wrote:
> StandaloneMmCoreEntryPoint calls SerialPortInitialize() explicitly,
> so add SerialPortLib to its list of LibraryClasses.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
Reviewed-by: Supreeth Venkatesh 

> ---
>  StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreE
> ntryPoint.inf | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCore
> EntryPoint.inf
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCore
> EntryPoint.inf
> index 3222cd359f3e..769eaeeefbea 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCore
> EntryPoint.inf
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCore
> EntryPoint.inf
> @@ -43,6 +43,7 @@ [Packages.AARCH64]
>  [LibraryClasses]
>BaseLib
>DebugLib
> +  SerialPortLib
>  
>  [LibraryClasses.AARCH64]
>StandaloneMmMmuLib

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


[edk2] [PATCH 1/2] ArmPkg/ArmMmuLib ARM: add missing support for non-shareable cached mappings

2019-01-04 Thread Ard Biesheuvel
We introduced support for non-shareable cached mappings to the AArch64
version of ArmMmuLib a while ago, but the ARM version was left behind,
so fix it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c 
b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
index 889b22867dc7..b237321a8d8b 100644
--- a/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
+++ b/ArmPkg/Library/ArmMmuLib/Arm/ArmMmuLibCore.c
@@ -135,6 +135,11 @@ PopulateLevel2PageTable (
 case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
   PageAttributes = TT_DESCRIPTOR_PAGE_WRITE_BACK;
   break;
+case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE:
+case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE:
+  PageAttributes = TT_DESCRIPTOR_PAGE_WRITE_BACK;
+  PageAttributes &= ~TT_DESCRIPTOR_PAGE_S_SHARED;
+  break;
 case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH:
 case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH:
   PageAttributes = TT_DESCRIPTOR_PAGE_WRITE_THROUGH;
@@ -239,6 +244,10 @@ FillTranslationTable (
 case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK:
   Attributes = TT_DESCRIPTOR_SECTION_WRITE_BACK(0);
   break;
+case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_BACK_NONSHAREABLE:
+  Attributes = TT_DESCRIPTOR_SECTION_WRITE_BACK(0);
+  Attributes &= ~TT_DESCRIPTOR_SECTION_S_SHARED;
+  break;
 case ARM_MEMORY_REGION_ATTRIBUTE_WRITE_THROUGH:
   Attributes = TT_DESCRIPTOR_SECTION_WRITE_THROUGH(0);
   break;
@@ -251,6 +260,10 @@ FillTranslationTable (
 case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK:
   Attributes = TT_DESCRIPTOR_SECTION_WRITE_BACK(1);
   break;
+case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_BACK_NONSHAREABLE:
+  Attributes = TT_DESCRIPTOR_SECTION_WRITE_BACK(1);
+  Attributes &= ~TT_DESCRIPTOR_SECTION_S_SHARED;
+  break;
 case ARM_MEMORY_REGION_ATTRIBUTE_NONSECURE_WRITE_THROUGH:
   Attributes = TT_DESCRIPTOR_SECTION_WRITE_THROUGH(1);
   break;
-- 
2.17.1

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


Re: [edk2] Uninstalling Invalid Protocol Interfaces

2019-01-04 Thread Ashish Singhal
Mike,

I have filed https://bugzilla.tianocore.org/show_bug.cgi?id=1428 and 
https://bugzilla.tianocore.org/show_bug.cgi?id=1429 to address this. I have 
assigned these to me as I already have fix for these.

Also, how do we ensure all components which may have this issue (may not have 
exposed it yet) also absorb the change?

Thanks
Ashish

From: Kinney, Michael D 
Sent: Friday, January 4, 2019 10:15 AM
To: Ashish Singhal ; edk2-devel@lists.01.org; Kinney, 
Michael D 
Cc: Gao, Liming ; Fu, Siyuan ; Wu, 
Jiaxin 
Subject: RE: Uninstalling Invalid Protocol Interfaces

Ashish,

Thanks for the pointer.  I agree there is an issue here.

Please enter a Bugzilla against the IScsiDxe module for this issue so we can 
fix this failure.

You are also welcome to enter a Bugzilla for a feature request to add UefiLib 
APIs that can be used to safely uninstall all the driver model related protocol 
that can be used in error paths in the entry point and in unload handlers.

Thanks,

Mike

From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
Sent: Thursday, January 3, 2019 4:16 PM
To: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>; 
edk2-devel@lists.01.org
Cc: Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan 
mailto:siyuan...@intel.com>>; Wu, Jiaxin 
mailto:jiaxin...@intel.com>>
Subject: RE: Uninstalling Invalid Protocol Interfaces

Hi Mike,

Call to UninstallMultipleProtocolInterfaces at 
https://github.com/tianocore/edk2/blob/master/NetworkPkg/IScsiDxe/IScsiDriver.c#L1864
 fails because the component name interface may not be installed depending on 
the state of PCD.

Thanks
Ashish

From: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Sent: Thursday, January 3, 2019 5:09 PM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>; 
edk2-devel@lists.01.org; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Cc: Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan 
mailto:siyuan...@intel.com>>; Wu, Jiaxin 
mailto:jiaxin...@intel.com>>
Subject: RE: Uninstalling Invalid Protocol Interfaces

Hi Ashish,

Can you provide a pointer to the UninstallMultipleProtocolInterfaces() call 
that is causing this failure?

I agree that the UefiLib could provide additional services to simplify the 
implementation of the unload handlers.  Matching the UefiLib APIs that install 
the Uefi Driver Model protocol would be useful, so the driver entry point and 
the driver unload functions can use matching APIs.

Mike

From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
Sent: Thursday, January 3, 2019 3:39 PM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>; Gao, Liming 
mailto:liming@intel.com>>; Fu, Siyuan 
mailto:siyuan...@intel.com>>; Wu, Jiaxin 
mailto:jiaxin...@intel.com>>
Subject: Uninstalling Invalid Protocol Interfaces

Hello,

As part of moving from MdeModulePkg implementation of IScsiDxe to the 
implementation in NetworkPkg, I started hitting exception in the driver loaded 
after IScsiDxe if IScsiDxe's installation fails for some reason. Upon 
debugging, I found out that calls to UninstallMultipleProtocolInterfaces as 
part of Error 2 as well Error 1 fail while trying to uninstall component name 
protocol-interface pair and return error code EFI_INVALID_PARAMETER. As per 
UninstallMultipleProtocolInterfaces's documentation, if uninstallation of any 
of the input protocol-interface pair fails, it will reinstall any just 
uninstalled protocol and return EFI_INVALID_PARAMETER which causes the cleanup 
of this driver corrupt leading to failure in next driver getting loaded. The 
reason of failure in UninstallMultipleProtocolInterfaces is because the driver 
uses EfiLibInstallDriverBindingComponentName2 to install interfaces which may 
not install component name interfaces depending on the value of PCDs 
PcdComponentNameDi
 sable and PcdComponentName2Disable. I have the following proposals to get 
around this issue.


  1.  Instead of calling UninstallMultipleProtocolInterfaces once and list all 
protocol-interface pairs, do it sequentially for every pair so that the once 
which was installed correctly, gets uninstalled instead of getting reinstalled 
because of a failure uninstalling another pair. This would however make us not 
really use use UninstallMultipleProtocolInterfaces API to its full caliber.
  2.  In UEFILib, add a function EfiLibUninstallDriverBindingComponentName2 for 
uninstalling protocol interfaces taking into consideration the state of PCDs 
PcdComponentNameDisable and PcdComponentName2Disable.
  3.  In UEFILib, add uninstall functions for all corresponding install APIs to 
get coverage for all scenarios.

I would certainly prefer option 2 or 3 as they seem to be more correct and 
would provide a for all drivers which may hit the issue. I am happy to make the 
code changes as needed and suggested by the maintainers.

Thanks
Ashish


Re: [edk2] [PATCH 1/8] StandaloneMmPkg/StandaloneMmCpu: fix typo Standlone -> Standalone

2019-01-04 Thread Supreeth Venkatesh
On Fri, 2019-01-04 at 12:03 +0100, Ard Biesheuvel wrote:
> Fix a couple of occurrences of typo Standlone -> Standalone. Since
> _PiMmStandloneArmTfCpuDriverEntry() is never referenced, drop it
> altogether.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
Reviewed-by: Supreeth Venkatesh 

> ---
>  StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c   
> | 2 +-
>  StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c   
> | 6 +++---
>  StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h   
> | 8 +---
>  StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
> | 4 ++--
>  4 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git
> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
> index 2814577b3fcc..25114821448a 100644
> --- a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
> +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
> @@ -65,7 +65,7 @@ EFI_MM_CONFIGURATION_PROTOCOL mMmConfig = {
>  STATIC EFI_MM_ENTRY_POINT mMmEntryPoint = NULL;
>  
>  EFI_STATUS
> -PiMmStandloneArmTfCpuDriverEntry (
> +PiMmStandaloneArmTfCpuDriverEntry (
>IN UINTN EventId,
>IN UINTN CpuNumber,
>IN UINTN NsCommBufferAddr
> diff --git
> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
> index 85a9c108aea4..203a32baaaf9 100644
> ---
> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
> +++
> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
> @@ -74,7 +74,7 @@ GetGuidedHobData (
>  }
>  
>  EFI_STATUS
> -StandloneMmCpuInitialize (
> +StandaloneMmCpuInitialize (
>IN EFI_HANDLE ImageHandle,  // not actual imagehandle
>IN EFI_MM_SYSTEM_TABLE   *SystemTable  // not actual systemtable
>)
> @@ -147,8 +147,8 @@ StandloneMmCpuInitialize (
>// Share the entry point of the CPU driver
>DEBUG ((DEBUG_INFO, "Sharing Cpu Driver EP *0x%lx = 0x%lx\n",
>(UINT64) CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr,
> -  (UINT64) PiMmStandloneArmTfCpuDriverEntry));
> -  *(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) =
> PiMmStandloneArmTfCpuDriverEntry;
> +  (UINT64) PiMmStandaloneArmTfCpuDriverEntry));
> +  *(CpuDriverEntryPointDesc->ArmTfCpuDriverEpPtr) =
> PiMmStandaloneArmTfCpuDriverEntry;
>  
>// Find the descriptor that contains the whereabouts of the buffer
> for
>// communication with the Normal world.
> diff --git
> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h
> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h
> index 7b38b65e1242..543467f67576 100644
> ---
> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h
> +++
> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h
> @@ -40,7 +40,7 @@ extern
> MP_INFORMATION_HOB_DATA   *mMpInformationHobData;
>  extern EFI_MM_CONFIGURATION_PROTOCOL mMmConfig;
>  
>  EFI_STATUS
> -PiMmStandloneArmTfCpuDriverEntry (
> +PiMmStandaloneArmTfCpuDriverEntry (
>IN UINTN EventId,
>IN UINTN CpuNumber,
>IN UINTN NsCommBufferAddr
> @@ -55,10 +55,4 @@ PiMmCpuTpFwRootMmiHandler (
>IN OUT UINTN*CommBufferSize  OPTIONAL
>);
>  
> -EFI_STATUS _PiMmStandloneArmTfCpuDriverEntry (
> -  IN UINTN EventId,
> -  IN UINTN CpuNumber,
> -  IN UINTN NsCommBufferAddr
> -  );
> -
>  #endif
> diff --git
> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
> index 9e6bbabdb103..d261e51ebc75 100644
> ---
> a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
> +++
> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
> @@ -18,12 +18,12 @@
>  
>  [Defines]
>INF_VERSION= 0x0001001A
> -  BASE_NAME  = StandloneMmCpu
> +  BASE_NAME  = StandaloneMmCpu
>FILE_GUID  = 58F7A62B-6280-42A7-BC38-
> 10535A64A92C
>MODULE_TYPE= MM_STANDALONE
>VERSION_STRING = 1.0
>PI_SPECIFICATION_VERSION   = 0x00010032
> -  ENTRY_POINT= StandloneMmCpuInitialize
> +  ENTRY_POINT= StandaloneMmCpuInitialize
>  
>  [Sources]
>StandaloneMmCpu.c

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


Re: [edk2] [PATCH 3/8] StandaloneMmPkg/StandaloneMmCoreEntryPoint: use %a modifier for ASCII strings

2019-01-04 Thread Supreeth Venkatesh
On Fri, 2019-01-04 at 12:03 +0100, Ard Biesheuvel wrote:
> PE/COFF section names are ASCII strings so use %a not %s.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
Reviewed-by: Supreeth Venkatesh 

> ---
>  StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermis
> sions.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermi
> ssions.c
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermi
> ssions.c
> index 60c1f66b83fa..3ca7f6660f47 100644
> ---
> a/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermi
> ssions.c
> +++
> b/StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermi
> ssions.c
> @@ -78,7 +78,7 @@ UpdateMmFoundationPeCoffPermissions (
>  "%a: Section %d of image at 0x%lx has 0x%x
> permissions\n",
>  __FUNCTION__, Index, ImageContext->ImageAddress,
> SectionHeader.Characteristics));
>  DEBUG ((DEBUG_INFO,
> -"%a: Section %d of image at 0x%lx has %s name\n",
> +"%a: Section %d of image at 0x%lx has %a name\n",
>  __FUNCTION__, Index, ImageContext->ImageAddress,
> SectionHeader.Name));
>  DEBUG ((DEBUG_INFO,
>  "%a: Section %d of image at 0x%lx has 0x%x address\n",

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


[edk2] [PATCH 2/4] NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.

2019-01-04 Thread Ashish Singhal
During cleanup in case of initialization failure, some driver
bindings are not installed. Using abstractions in UEFILib takes
care of it.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1428

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 NetworkPkg/IScsiDxe/IScsiDriver.c | 31 +++
 1 file changed, 11 insertions(+), 20 deletions(-)

diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c 
b/NetworkPkg/IScsiDxe/IScsiDriver.c
index 91176e6..8747de7 100644
--- a/NetworkPkg/IScsiDxe/IScsiDriver.c
+++ b/NetworkPkg/IScsiDxe/IScsiDriver.c
@@ -1,6 +1,7 @@
 /** @file
   The entry point of IScsi driver.
 
+Copyright (c) 2019, NVIDIA Corporation. All rights reserved.
 Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.
 (C) Copyright 2017 Hewlett Packard Enterprise Development LP
 
@@ -1861,28 +1862,18 @@ Error3:
  );
 
 Error2:
-  gBS->UninstallMultipleProtocolInterfaces (
- gIScsiIp6DriverBinding.DriverBindingHandle,
- ,
- ,
- ,
- ,
- ,
- ,
- NULL
- );
+  EfiLibUninstallDriverBindingComponentName2 (
+,
+,
+
+);
 
 Error1:
-  gBS->UninstallMultipleProtocolInterfaces (
- ImageHandle,
- ,
- ,
- ,
- ,
- ,
- ,
- NULL
- );
+  EfiLibUninstallDriverBindingComponentName2 (
+,
+,
+
+);
 
   return Status;
 }
-- 
2.7.4

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


[edk2] [PATCH 0/4] Provide UEFILib functions for protocol uninstallation.

2019-01-04 Thread Ashish Singhal
An issue was seen in IScsiDxe in NetworkPkg where driver cleanup after
initialization failure was not done right. Bug 1428 was filed in this regard.
As per discussions with Mike, it was also discussed that having UEFILib
provide protocol uninstallation abstraction would help to avoid these
issues in the future. Bug 1429 was found to track this. The first 2 patches
take care of this.

Patch number 3 simplifies the UEFILib protocol installation and
uninstallation abstraction by adding a helper function doing operations
instead of every public function.

Patch set 4 uses the updated uninstallation interfaces as a result of patch 3.

Ashish Singhal (4):
  MdePkg/UefiLib: Abstract driver model protocol uninstallation
  NetworkPkg/IScsiDxe: Use UEFILib APIs to uninstall protocols.
  MdePkg/UefiLib: Simplify protocol un/installation abstraction
  NetworkPkg/IScsiDxe: Update UEFILib Usage

 MdePkg/Include/Library/UefiLib.h |  127 
 MdePkg/Library/UefiLib/UefiDriverModel.c | 1205 +-
 NetworkPkg/IScsiDxe/IScsiDriver.c|   37 +-
 3 files changed, 484 insertions(+), 885 deletions(-)

-- 
2.7.4

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


[edk2] [PATCH 3/4] MdePkg/UefiLib: Simplify protocol un/installation abstraction

2019-01-04 Thread Ashish Singhal
Add a helper function to operate upon protocol installation and
uninstallation instead of every function doing it by itself.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 MdePkg/Include/Library/UefiLib.h |   26 +-
 MdePkg/Library/UefiLib/UefiDriverModel.c | 1975 --
 2 files changed, 265 insertions(+), 1736 deletions(-)

diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index 08222d4..fbc9739 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -1323,7 +1323,10 @@ EfiLibInstallDriverBinding (
   If DriverBinding is NULL, then ASSERT().
   If DriverBinding can not be uninstalled, then ASSERT().
 
+  @param  ImageHandle  The image handle of the driver.
+  @param  SystemTable  The EFI System Table that was passed to the 
driver's entry point.
   @param  DriverBindingA Driver Binding Protocol instance that this 
driver produced.
+  @param  DriverBindingHandle  The handle that DriverBinding is to be 
installed onto.
 
   @retval EFI_SUCCESS   The protocol uninstallation successfully 
completed.
   @retval OthersStatus from 
gBS->UninstallMultipleProtocolInterfaces().
@@ -1332,7 +1335,10 @@ EfiLibInstallDriverBinding (
 EFI_STATUS
 EFIAPI
 EfiLibUninstallDriverBinding (
-  IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding
+  IN CONST EFI_HANDLE ImageHandle,
+  IN CONST EFI_SYSTEM_TABLE   *SystemTable,
+  IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding,
+  IN EFI_HANDLE   DriverBindingHandle
   );
 
 
@@ -1382,7 +1388,10 @@ EfiLibInstallAllDriverProtocols (
   If DriverBinding is NULL, then ASSERT().
   If the uninstallation fails, then ASSERT().
 
+  @param  ImageHandle  The image handle of the driver.
+  @param  SystemTable  The EFI System Table that was passed to the 
driver's entry point.
   @param  DriverBindingA Driver Binding Protocol instance that this 
driver produced.
+  @param  DriverBindingHandle  The handle that DriverBinding is to be 
installed onto.
   @param  ComponentNameA Component Name Protocol instance that this 
driver produced.
   @param  DriverConfiguration  A Driver Configuration Protocol instance that 
this driver produced.
   @param  DriverDiagnosticsA Driver Diagnostics Protocol instance that 
this driver produced.
@@ -1394,7 +1403,10 @@ EfiLibInstallAllDriverProtocols (
 EFI_STATUS
 EFIAPI
 EfiLibUninstallAllDriverProtocols (
+  IN CONST EFI_HANDLE ImageHandle,
+  IN CONST EFI_SYSTEM_TABLE   *SystemTable,
   IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding,
+  IN EFI_HANDLE   DriverBindingHandle,
   IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,   OPTIONAL
   IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL  *DriverConfiguration, OPTIONAL
   IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnosticsOPTIONAL
@@ -1442,7 +1454,10 @@ EfiLibInstallDriverBindingComponentName2 (
   If DriverBinding is NULL, then ASSERT().
   If the uninstallation fails, then ASSERT().
 
+  @param  ImageHandle  The image handle of the driver.
+  @param  SystemTable  The EFI System Table that was passed to the 
driver's entry point.
   @param  DriverBindingA Driver Binding Protocol instance that this 
driver produced.
+  @param  DriverBindingHandle  The handle that DriverBinding is to be 
installed onto.
   @param  ComponentNameA Component Name Protocol instance that this 
driver produced.
   @param  ComponentName2   A Component Name 2 Protocol instance that this 
driver produced.
 
@@ -1453,7 +1468,10 @@ EfiLibInstallDriverBindingComponentName2 (
 EFI_STATUS
 EFIAPI
 EfiLibUninstallDriverBindingComponentName2 (
+  IN CONST EFI_HANDLE ImageHandle,
+  IN CONST EFI_SYSTEM_TABLE   *SystemTable,
   IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding,
+  IN EFI_HANDLE   DriverBindingHandle,
   IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,   OPTIONAL
   IN CONST EFI_COMPONENT_NAME2_PROTOCOL   *ComponentName2   OPTIONAL
   );
@@ -1512,7 +1530,10 @@ EfiLibInstallAllDriverProtocols2 (
   If the installation fails, then ASSERT().
 
 
+  @param  ImageHandle   The image handle of the driver.
+  @param  SystemTable   The EFI System Table that was passed to the 
driver's entry point.
   @param  DriverBinding A Driver Binding Protocol instance that this 
driver produced.
+  @param  DriverBindingHandle   The handle that DriverBinding is to be 
installed onto.
   @param  ComponentName A Component Name Protocol instance that this 
driver produced.
   @param  ComponentName2A Component Name 2 Protocol instance that this 
driver produced.
   @param  DriverConfiguration   A Driver Configuration Protocol 

[edk2] [PATCH 1/4] MdePkg/UefiLib: Abstract driver model protocol uninstallation

2019-01-04 Thread Ashish Singhal
Provided functions in UEFILib that abstract driver model protocol
uninstallation. This helps drivers to install and uninstall protocols
using a library to keep things seemless.

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1429

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 MdePkg/Include/Library/UefiLib.h | 103 
 MdePkg/Library/UefiLib/UefiDriverModel.c | 972 ++-
 2 files changed, 1074 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Include/Library/UefiLib.h b/MdePkg/Include/Library/UefiLib.h
index 468bffc..08222d4 100644
--- a/MdePkg/Include/Library/UefiLib.h
+++ b/MdePkg/Include/Library/UefiLib.h
@@ -12,6 +12,7 @@
   of size reduction when compiler optimization is disabled. If MDEPKG_NDEBUG is
   defined, then debug and assert related macros wrapped by it are the NULL 
implementations.
 
+Copyright (c) 2019, NVIDIA Corporation. All rights reserved.
 Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
 This program and the accompanying materials are licensed and made available 
under
 the terms and conditions of the BSD License that accompanies this distribution.
@@ -1283,6 +1284,7 @@ AsciiPrintXY (
   ...
   );
 
+
 /**
   Installs and completes the initialization of a Driver Binding Protocol 
instance.
 
@@ -1316,6 +1318,25 @@ EfiLibInstallDriverBinding (
 
 
 /**
+  Uninstalls a Driver Binding Protocol instance.
+
+  If DriverBinding is NULL, then ASSERT().
+  If DriverBinding can not be uninstalled, then ASSERT().
+
+  @param  DriverBindingA Driver Binding Protocol instance that this 
driver produced.
+
+  @retval EFI_SUCCESS   The protocol uninstallation successfully 
completed.
+  @retval OthersStatus from 
gBS->UninstallMultipleProtocolInterfaces().
+
+**/
+EFI_STATUS
+EFIAPI
+EfiLibUninstallDriverBinding (
+  IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding
+  );
+
+
+/**
   Installs and completes the initialization of a Driver Binding Protocol 
instance and
   optionally installs the Component Name, Driver Configuration and Driver 
Diagnostics Protocols.
 
@@ -1354,6 +1375,31 @@ EfiLibInstallAllDriverProtocols (
   );
 
 
+/**
+  Uninstalls a Driver Binding Protocol instance and optionally uninstalls the
+  Component Name, Driver Configuration and Driver Diagnostics Protocols.
+
+  If DriverBinding is NULL, then ASSERT().
+  If the uninstallation fails, then ASSERT().
+
+  @param  DriverBindingA Driver Binding Protocol instance that this 
driver produced.
+  @param  ComponentNameA Component Name Protocol instance that this 
driver produced.
+  @param  DriverConfiguration  A Driver Configuration Protocol instance that 
this driver produced.
+  @param  DriverDiagnosticsA Driver Diagnostics Protocol instance that 
this driver produced.
+
+  @retval EFI_SUCCESS   The protocol uninstallation successfully 
completed.
+  @retval OthersStatus from 
gBS->UninstallMultipleProtocolInterfaces().
+
+**/
+EFI_STATUS
+EFIAPI
+EfiLibUninstallAllDriverProtocols (
+  IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding,
+  IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,   OPTIONAL
+  IN CONST EFI_DRIVER_CONFIGURATION_PROTOCOL  *DriverConfiguration, OPTIONAL
+  IN CONST EFI_DRIVER_DIAGNOSTICS_PROTOCOL*DriverDiagnosticsOPTIONAL
+  );
+
 
 /**
   Installs Driver Binding Protocol with optional Component Name and Component 
Name 2 Protocols.
@@ -1391,6 +1437,29 @@ EfiLibInstallDriverBindingComponentName2 (
 
 
 /**
+  Uninstalls Driver Binding Protocol with optional Component Name and 
Component Name 2 Protocols.
+
+  If DriverBinding is NULL, then ASSERT().
+  If the uninstallation fails, then ASSERT().
+
+  @param  DriverBindingA Driver Binding Protocol instance that this 
driver produced.
+  @param  ComponentNameA Component Name Protocol instance that this 
driver produced.
+  @param  ComponentName2   A Component Name 2 Protocol instance that this 
driver produced.
+
+  @retval EFI_SUCCESS   The protocol installation successfully 
completed.
+  @retval OthersStatus from 
gBS->UninstallMultipleProtocolInterfaces().
+
+**/
+EFI_STATUS
+EFIAPI
+EfiLibUninstallDriverBindingComponentName2 (
+  IN EFI_DRIVER_BINDING_PROTOCOL  *DriverBinding,
+  IN CONST EFI_COMPONENT_NAME_PROTOCOL*ComponentName,   OPTIONAL
+  IN CONST EFI_COMPONENT_NAME2_PROTOCOL   *ComponentName2   OPTIONAL
+  );
+
+
+/**
   Installs Driver Binding Protocol with optional Component Name, Component 
Name 2, Driver
   Configuration, Driver Configuration 2, Driver Diagnostics, and Driver 
Diagnostics 2 Protocols.
 
@@ -1434,6 +1503,40 @@ EfiLibInstallAllDriverProtocols2 (
   IN CONST EFI_DRIVER_DIAGNOSTICS2_PROTOCOL   *DriverDiagnostics2OPTIONAL
   );
 
+
+/**
+  Uninstalls Driver Binding Protocol with optional Component Name, Component 
Name 2, Driver
+  

[edk2] [PATCH 4/4] NetworkPkg/IScsiDxe: Update UEFILib Usage

2019-01-04 Thread Ashish Singhal
Update interfaces as exposed by UEFILib for protocol
installation and uninstallation abstraction.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ashish Singhal 
---
 NetworkPkg/IScsiDxe/IScsiDriver.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/NetworkPkg/IScsiDxe/IScsiDriver.c 
b/NetworkPkg/IScsiDxe/IScsiDriver.c
index 8747de7..fa0ea00 100644
--- a/NetworkPkg/IScsiDxe/IScsiDriver.c
+++ b/NetworkPkg/IScsiDxe/IScsiDriver.c
@@ -1863,14 +1863,20 @@ Error3:
 
 Error2:
   EfiLibUninstallDriverBindingComponentName2 (
+ImageHandle,
+SystemTable,
 ,
+NULL,
 ,
 
 );
 
 Error1:
   EfiLibUninstallDriverBindingComponentName2 (
+ImageHandle,
+SystemTable,
 ,
+NULL,
 ,
 
 );
-- 
2.7.4

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


Re: [edk2] Uninstalling Invalid Protocol Interfaces

2019-01-04 Thread Ashish Singhal
Mike,

I have addressed the issue along with some optimizations and have submitted 
patches for review.

Thanks
Ashish

From: Ashish Singhal
Sent: Friday, January 4, 2019 10:33 AM
To: 'Kinney, Michael D' ; edk2-devel@lists.01.org
Cc: Gao, Liming ; Fu, Siyuan ; Wu, 
Jiaxin 
Subject: RE: Uninstalling Invalid Protocol Interfaces

Mike,

I have filed https://bugzilla.tianocore.org/show_bug.cgi?id=1428 and 
https://bugzilla.tianocore.org/show_bug.cgi?id=1429 to address this. I have 
assigned these to me as I already have fix for these.

Also, how do we ensure all components which may have this issue (may not have 
exposed it yet) also absorb the change?

Thanks
Ashish

From: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Sent: Friday, January 4, 2019 10:15 AM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>; 
edk2-devel@lists.01.org; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Cc: Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan 
mailto:siyuan...@intel.com>>; Wu, Jiaxin 
mailto:jiaxin...@intel.com>>
Subject: RE: Uninstalling Invalid Protocol Interfaces

Ashish,

Thanks for the pointer.  I agree there is an issue here.

Please enter a Bugzilla against the IScsiDxe module for this issue so we can 
fix this failure.

You are also welcome to enter a Bugzilla for a feature request to add UefiLib 
APIs that can be used to safely uninstall all the driver model related protocol 
that can be used in error paths in the entry point and in unload handlers.

Thanks,

Mike

From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
Sent: Thursday, January 3, 2019 4:16 PM
To: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>; 
edk2-devel@lists.01.org
Cc: Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan 
mailto:siyuan...@intel.com>>; Wu, Jiaxin 
mailto:jiaxin...@intel.com>>
Subject: RE: Uninstalling Invalid Protocol Interfaces

Hi Mike,

Call to UninstallMultipleProtocolInterfaces at 
https://github.com/tianocore/edk2/blob/master/NetworkPkg/IScsiDxe/IScsiDriver.c#L1864
 fails because the component name interface may not be installed depending on 
the state of PCD.

Thanks
Ashish

From: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Sent: Thursday, January 3, 2019 5:09 PM
To: Ashish Singhal mailto:ashishsin...@nvidia.com>>; 
edk2-devel@lists.01.org; Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>
Cc: Gao, Liming mailto:liming@intel.com>>; Fu, Siyuan 
mailto:siyuan...@intel.com>>; Wu, Jiaxin 
mailto:jiaxin...@intel.com>>
Subject: RE: Uninstalling Invalid Protocol Interfaces

Hi Ashish,

Can you provide a pointer to the UninstallMultipleProtocolInterfaces() call 
that is causing this failure?

I agree that the UefiLib could provide additional services to simplify the 
implementation of the unload handlers.  Matching the UefiLib APIs that install 
the Uefi Driver Model protocol would be useful, so the driver entry point and 
the driver unload functions can use matching APIs.

Mike

From: Ashish Singhal [mailto:ashishsin...@nvidia.com]
Sent: Thursday, January 3, 2019 3:39 PM
To: edk2-devel@lists.01.org
Cc: Kinney, Michael D 
mailto:michael.d.kin...@intel.com>>; Gao, Liming 
mailto:liming@intel.com>>; Fu, Siyuan 
mailto:siyuan...@intel.com>>; Wu, Jiaxin 
mailto:jiaxin...@intel.com>>
Subject: Uninstalling Invalid Protocol Interfaces

Hello,

As part of moving from MdeModulePkg implementation of IScsiDxe to the 
implementation in NetworkPkg, I started hitting exception in the driver loaded 
after IScsiDxe if IScsiDxe's installation fails for some reason. Upon 
debugging, I found out that calls to UninstallMultipleProtocolInterfaces as 
part of Error 2 as well Error 1 fail while trying to uninstall component name 
protocol-interface pair and return error code EFI_INVALID_PARAMETER. As per 
UninstallMultipleProtocolInterfaces's documentation, if uninstallation of any 
of the input protocol-interface pair fails, it will reinstall any just 
uninstalled protocol and return EFI_INVALID_PARAMETER which causes the cleanup 
of this driver corrupt leading to failure in next driver getting loaded. The 
reason of failure in UninstallMultipleProtocolInterfaces is because the driver 
uses EfiLibInstallDriverBindingComponentName2 to install interfaces which may 
not install component name interfaces depending on the value of PCDs 
PcdComponentNameDi
 sable and PcdComponentName2Disable. I have the following proposals to get 
around this issue.


  1.  Instead of calling UninstallMultipleProtocolInterfaces once and list all 
protocol-interface pairs, do it sequentially for every pair so that the once 
which was installed correctly, gets uninstalled instead of getting reinstalled 
because of a failure uninstalling another pair. This would however make us not 
really use use UninstallMultipleProtocolInterfaces API to its full caliber.
  2.  In UEFILib, add a function