Re: [edk2] [PATCH 27/37] PcAtChipsetPkg: Removing ipf which is no longer supported from edk2.

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: edk2-devel  On Behalf Of chenc2
> Sent: Wednesday, June 13, 2018 11:45 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ruiyu ; Kinney, Michael D
> 
> Subject: [edk2] [PATCH 27/37] PcAtChipsetPkg: Removing ipf which is no
> longer supported from edk2.
> 
> Removing rules for Ipf sources file:
> * Remove the source file which path with "ipf" and also listed in
>   [Sources.IPF] section of INF file.
> * Remove the source file which listed in [Components.IPF] section
>   of DSC file and not listed in any other [Components] section.
> * Remove the embedded Ipf code for MDE_CPU_IPF.
> 
> Removing rules for Inf file:
> * Remove IPF from VALID_ARCHITECTURES comments.
> * Remove DXE_SAL_DRIVER from LIBRARY_CLASS in [Defines] section.
> * Remove the INF which only listed in [Components.IPF] section in DSC.
> * Remove statements from [BuildOptions] that provide IPF specific flags.
> * Remove any IPF sepcific sections.
> 
> Removing rules for Dec file:
> * Remove [Includes.IPF] section from Dec.
> 
> Removing rules for Dsc file:
> * Remove IPF from SUPPORTED_ARCHITECTURES in [Defines] section of DSC.
> * Remove any IPF specific sections.
> * Remove statements from [BuildOptions] that provide IPF specific flags.
> 
> Cc: Ruiyu Ni 
> Cc: Michael D Kinney 
> Signed-off-by: chenc2 
> Contributed-under: TianoCore Contribution Agreement 1.1
> ---
>  PcAtChipsetPkg/Bus/Pci/IdeControllerDxe/IdeControllerDxe.inf | 2 +-
>  PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf | 2 +-
>  PcAtChipsetPkg/Library/ResetSystemLib/ResetSystemLib.inf | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/PcAtChipsetPkg/Bus/Pci/IdeControllerDxe/IdeControllerDxe.inf
> b/PcAtChipsetPkg/Bus/Pci/IdeControllerDxe/IdeControllerDxe.inf
> index 0184bf40f6..2b4f7516b1 100644
> --- a/PcAtChipsetPkg/Bus/Pci/IdeControllerDxe/IdeControllerDxe.inf
> +++ b/PcAtChipsetPkg/Bus/Pci/IdeControllerDxe/IdeControllerDxe.inf
> @@ -25,7 +25,7 @@
>  #
>  # The following information is for reference only and not required by the
> build tools.
>  #
> -#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
> +#  VALID_ARCHITECTURES   = IA32 X64 EBC
>  #
> 
>  [Sources]
> diff --git a/PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
> b/PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
> index 8cc4c4d87b..42891ca64d 100644
> --- a/PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
> +++ b/PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf
> @@ -24,7 +24,7 @@
>  #
>  # The following information is for reference only and not required by the
> build tools.
>  #
> -#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
> +#  VALID_ARCHITECTURES   = IA32 X64 EBC
>  #
>  #
> 
> diff --git a/PcAtChipsetPkg/Library/ResetSystemLib/ResetSystemLib.inf
> b/PcAtChipsetPkg/Library/ResetSystemLib/ResetSystemLib.inf
> index 2384efaae2..7d658b1fdb 100644
> --- a/PcAtChipsetPkg/Library/ResetSystemLib/ResetSystemLib.inf
> +++ b/PcAtChipsetPkg/Library/ResetSystemLib/ResetSystemLib.inf
> @@ -24,7 +24,7 @@
>  #
>  # The following information is for reference only and not required by the
> build tools.
>  #
> -#  VALID_ARCHITECTURES   = IA32 X64 IPF
> +#  VALID_ARCHITECTURES   = IA32 X64
>  #
> 
>  [Sources]
> --
> 2.16.2.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 19/37] IntelSiliconPkg: Removing ipf which is no longer supported from edk2.

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Chen,
> Chen A
> Sent: Wednesday, June 13, 2018 11:45 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Yao, Jiewen
> 
> Subject: [edk2] [PATCH 19/37] IntelSiliconPkg: Removing ipf which is no longer
> supported from edk2.
> 
> Removing rules for Ipf sources file:
> * Remove the source file which path with "ipf" and also listed in
>   [Sources.IPF] section of INF file.
> * Remove the source file which listed in [Components.IPF] section
>   of DSC file and not listed in any other [Components] section.
> * Remove the embedded Ipf code for MDE_CPU_IPF.
> 
> Removing rules for Inf file:
> * Remove IPF from VALID_ARCHITECTURES comments.
> * Remove DXE_SAL_DRIVER from LIBRARY_CLASS in [Defines] section.
> * Remove the INF which only listed in [Components.IPF] section in DSC.
> * Remove statements from [BuildOptions] that provide IPF specific flags.
> * Remove any IPF sepcific sections.
> 
> Removing rules for Dec file:
> * Remove [Includes.IPF] section from Dec.
> 
> Removing rules for Dsc file:
> * Remove IPF from SUPPORTED_ARCHITECTURES in [Defines] section of DSC.
> * Remove any IPF specific sections.
> * Remove statements from [BuildOptions] that provide IPF specific flags.
> 
> Cc: Jiewen Yao 
> Cc: Rangasai V Chaganty 
> Cc: Michael D Kinney 
> Signed-off-by: chenc2 
> Contributed-under: TianoCore Contribution Agreement 1.1
> ---
>  IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf |
> 2 +-
>  .../Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSamplePei.inf   |
> 2 +-
>  .../Feature/VTd/PlatformVTdSampleDxe/PlatformVTdSampleDxe.inf
> | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
> b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
> index bfb6777040..9d55acab8c 100644
> --- a/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
> +++ b/IntelSiliconPkg/Feature/VTd/IntelVTdDxe/IntelVTdDxe.inf
> @@ -27,7 +27,7 @@
>  #
>  # The following information is for reference only and not required by the 
> build
> tools.
>  #
> -#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
> +#  VALID_ARCHITECTURES   = IA32 X64 EBC
>  #
>  #
> 
> diff --git
> a/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSam
> plePei.inf
> b/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSam
> plePei.inf
> index a4ffe51e2f..213e96fa8e 100644
> ---
> a/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSam
> plePei.inf
> +++
> b/IntelSiliconPkg/Feature/VTd/PlatformVTdInfoSamplePei/PlatformVTdInfoSam
> plePei.inf
> @@ -24,7 +24,7 @@
>  #
>  # The following information is for reference only and not required by the 
> build
> tools.
>  #
> -#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
> +#  VALID_ARCHITECTURES   = IA32 X64 EBC
>  #
>  #
> 
> diff --git
> a/IntelSiliconPkg/Feature/VTd/PlatformVTdSampleDxe/PlatformVTdSampleDxe.i
> nf
> b/IntelSiliconPkg/Feature/VTd/PlatformVTdSampleDxe/PlatformVTdSampleDxe.i
> nf
> index 0200b932d8..e49920704f 100644
> ---
> a/IntelSiliconPkg/Feature/VTd/PlatformVTdSampleDxe/PlatformVTdSampleDxe.i
> nf
> +++
> b/IntelSiliconPkg/Feature/VTd/PlatformVTdSampleDxe/PlatformVTdSampleDxe.i
> nf
> @@ -27,7 +27,7 @@
>  #
>  # The following information is for reference only and not required by the 
> build
> tools.
>  #
> -#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
> +#  VALID_ARCHITECTURES   = IA32 X64 EBC
>  #
>  #
> 
> --
> 2.16.2.windows.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][edk2-platforms/devel-IntelAtomProcessorE3900] Generate UUID.

[Guo, Mang]

Generate UUID for MinnowBoard Max. User can configure their UUID in setup UI. 
If user don't configure it in setup, MAC address will be used as UUID.

Contributed-under: TianoCore Contribution Agreement 1.1

Signed-off-by: Guo Mang 
---
 Vlv2TbltDevicePkg/Include/Guid/SetupVariable.h |   1 +
 .../PlatformSetupDxe/SystemComponent.vfi   |   9 +-
 Vlv2TbltDevicePkg/PlatformSetupDxe/VfrStrings.uni  | Bin 216550 -> 217008 bytes
 .../SmBiosMiscDxe/MiscSystemManufacturerFunction.c | 353 ++---
 Vlv2TbltDevicePkg/SmBiosMiscDxe/SmBiosMiscDxe.inf  |   5 +-
 Vlv2TbltDevicePkg/Stitch/Gcc/NvStorageVariable.bin | Bin 253952 -> 253952 bytes
 6 files changed, 247 insertions(+), 121 deletions(-)

diff --git a/Vlv2TbltDevicePkg/Include/Guid/SetupVariable.h 
b/Vlv2TbltDevicePkg/Include/Guid/SetupVariable.h
index f0319b6..1490f76 100644
--- a/Vlv2TbltDevicePkg/Include/Guid/SetupVariable.h
+++ b/Vlv2TbltDevicePkg/Include/Guid/SetupVariable.h
@@ -1311,6 +1311,7 @@ typedef struct {
   UINT8   RtcBattery;
   UINT8   LpeAudioReportedByDSDT;
   UINT8   Uart1Int3511Com; // Report UART1 as COM with _HID INT3511
+  CHAR16  SystemUuid[37];
 
 } SYSTEM_CONFIGURATION;
 #pragma pack()
diff --git a/Vlv2TbltDevicePkg/PlatformSetupDxe/SystemComponent.vfi 
b/Vlv2TbltDevicePkg/PlatformSetupDxe/SystemComponent.vfi
index e21f880..097d324 100644
--- a/Vlv2TbltDevicePkg/PlatformSetupDxe/SystemComponent.vfi
+++ b/Vlv2TbltDevicePkg/PlatformSetupDxe/SystemComponent.vfi
@@ -1,6 +1,6 @@
 //
 //
-// Copyright (c) 2004  - 2014, Intel Corporation. All rights reserved.
+// Copyright (c) 2004  - 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.  

@@ -81,6 +81,11 @@ form formid = SYSTEM_COMPONENT_FORM_ID,
 option text = STRING_TOKEN(STR_ON), value = 1, flags = RESET_REQUIRED;
   endoneof;
 
-
+  string varid  = Setup.SystemUuid,
+prompt   = STRING_TOKEN (STR_SYSTEM_UUID_TITLE),
+help = STRING_TOKEN (STR_SYSTEM_UUID_HELP),
+minsize = 0,
+maxsize = 36,
+  endstring;
 
 endform;
diff --git a/Vlv2TbltDevicePkg/PlatformSetupDxe/VfrStrings.uni 
b/Vlv2TbltDevicePkg/PlatformSetupDxe/VfrStrings.uni
index 
ad4cf64405e100f8f554d26badb01eb2d38a3128..20fcde03c71597489ac0fa635e6a3d48f8a8debd
 100644
GIT binary patch
delta 241
zcmaDhn|H%}-U$wj78@OtrKaC%V3e7hpvcD_$`H!n$>1`5p$4P#v}Q(?$@7-;FarfA
z_ZKRI6oA=`d9KqJa4;!NU!cv%R$stS%22|fz);DM4`dYqSx`fOtW1VHAZdsObs3DH
zP?y0N3UwJwpimd6+6X9W!eGi^#$e81!C=W?z>vt01T-lPXh#u4E|6Ekpv1t-z{S8j
pz1x^sdUAub-sA~lS|BG+Pq@m&(d;1I?jX$w#7x^Aq?wl)0RWDpGe`gc

delta 40
vcmdlmpZD2p-U$wj<{KT8r6#YtDlvV-H72g+2{P>yWEg>%Y5N2jW(8vaR67p}

diff --git a/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c 
b/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
index 1d47e4a..26da5ab 100644
--- a/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
+++ b/Vlv2TbltDevicePkg/SmBiosMiscDxe/MiscSystemManufacturerFunction.c
@@ -1,6 +1,6 @@
 /*++
 
-Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2009 - 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.
@@ -31,147 +31,208 @@ Abstract:
 #include "Library/DebugLib.h"
 #include 
 #include 
+#include 
+#include 
+#include 
 
 
 extern EFI_PLATFORM_INFO_HOB *mPlatformInfo;
-static EFI_SMBIOS_HANDLE mSmbiosHandleType1;
+EFI_GUID  mSystemConfigurationGuid = SYSTEM_CONFIGURATION_GUID;
 
 
-EFI_STATUS
-EFIAPI
-UpdateSmbiosManuCallback (
-  IN EFI_EVENT  Event,
-  IN VOID   *Context
-  )
-{
-  EFI_STATUSStatus;
-  EFI_HANDLE*Handles;
-  UINTN BufferSize;
-  CHAR16*MacStr;
-  EFI_SMBIOS_PROTOCOL   *Smbios;
-  UINTN SerialNumberOffset;
-  CHAR8 AsciiData[SMBIOS_STRING_MAX_LENGTH];
-
-  gBS->CloseEvent (Event);// Unload this event.
+/**
+  Return the description for network boot device.
 
-  DEBUG ((EFI_D_INFO, "Executing UpdateSmbiosManuCallback.\n"));
+  @param HandleController handle.
 
-  //
-  //Get handle infomation
-  //
-  BufferSize = 0;
-  Handles = NULL;
-  Status = gBS->LocateHandle (
-  ByProtocol,
-  &gEfiSimpleNetworkProtocolGuid,
+  @return  The description string.
+**/
+CHAR16 *
+GetNetworkDescription (
+  IN EFI_HANDLE  Handle
+  )
+{
+  EFI_STATUS Status;
+  EFI_DEVICE_PATH_PROTOCOL   *DevicePath;
+  MAC_ADDR_DEVICE_PATH   *Mac;
+  CHAR16 *Description;
+  UINTN  DescriptionSize;
+
+  Status = gBS->OpenProtocol (
+  

Re: [edk2] [PATCH 32/37] SignedCapsulePkg: Removing ipf which is no longer supported from edk2.

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Chen,
> Chen A
> Sent: Wednesday, June 13, 2018 11:46 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Yao, Jiewen
> 
> Subject: [edk2] [PATCH 32/37] SignedCapsulePkg: Removing ipf which is no
> longer supported from edk2.
> 
> Removing rules for Ipf sources file:
> * Remove the source file which path with "ipf" and also listed in
>   [Sources.IPF] section of INF file.
> * Remove the source file which listed in [Components.IPF] section
>   of DSC file and not listed in any other [Components] section.
> * Remove the embedded Ipf code for MDE_CPU_IPF.
> 
> Removing rules for Inf file:
> * Remove IPF from VALID_ARCHITECTURES comments.
> * Remove DXE_SAL_DRIVER from LIBRARY_CLASS in [Defines] section.
> * Remove the INF which only listed in [Components.IPF] section in DSC.
> * Remove statements from [BuildOptions] that provide IPF specific flags.
> * Remove any IPF sepcific sections.
> 
> Removing rules for Dec file:
> * Remove [Includes.IPF] section from Dec.
> 
> Removing rules for Dsc file:
> * Remove IPF from SUPPORTED_ARCHITECTURES in [Defines] section of DSC.
> * Remove any IPF specific sections.
> * Remove statements from [BuildOptions] that provide IPF specific flags.
> 
> Cc: Jiewen Yao 
> Cc: Michael D Kinney 
> Signed-off-by: chenc2 
> Contributed-under: TianoCore Contribution Agreement 1.1
> ---
>  .../Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf | 2
> +-
>  SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.inf| 2
> +-
>  .../Library/PlatformFlashAccessLibNull/PlatformFlashAccessLibNull.inf   | 2 
> +-
>  .../Universal/RecoveryModuleLoadPei/RecoveryModuleLoadPei.inf
> | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git
> a/SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf
> b/SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf
> index a721619a67..16cf8f6ba9 100644
> ---
> a/SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf
> +++
> b/SignedCapsulePkg/Library/EdkiiSystemCapsuleLib/EdkiiSystemCapsuleLib.inf
> @@ -27,7 +27,7 @@
>  #
>  # The following information is for reference only and not required by the 
> build
> tools.
>  #
> -#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
> +#  VALID_ARCHITECTURES   = IA32 X64 EBC
>  #
> 
>  [Sources]
> diff --git a/SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.inf
> b/SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.inf
> index 00f1d40858..0f2692c156 100644
> --- a/SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.inf
> +++ b/SignedCapsulePkg/Library/IniParsingLib/IniParsingLib.inf
> @@ -26,7 +26,7 @@
>  #
>  # The following information is for reference only and not required by the 
> build
> tools.
>  #
> -#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
> +#  VALID_ARCHITECTURES   = IA32 X64 EBC
>  #
> 
>  [Sources]
> diff --git
> a/SignedCapsulePkg/Library/PlatformFlashAccessLibNull/PlatformFlashAccessLib
> Null.inf
> b/SignedCapsulePkg/Library/PlatformFlashAccessLibNull/PlatformFlashAccessLib
> Null.inf
> index f3a7a6c663..e73eb713ec 100644
> ---
> a/SignedCapsulePkg/Library/PlatformFlashAccessLibNull/PlatformFlashAccessLib
> Null.inf
> +++
> b/SignedCapsulePkg/Library/PlatformFlashAccessLibNull/PlatformFlashAccessLib
> Null.inf
> @@ -26,7 +26,7 @@
>  #
>  # The following information is for reference only and not required by the 
> build
> tools.
>  #
> -#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
> +#  VALID_ARCHITECTURES   = IA32 X64 EBC
>  #
> 
>  [Sources]
> diff --git
> a/SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadP
> ei.inf
> b/SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadP
> ei.inf
> index 96a0cdd770..949ad298cf 100644
> ---
> a/SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadP
> ei.inf
> +++
> b/SignedCapsulePkg/Universal/RecoveryModuleLoadPei/RecoveryModuleLoadP
> ei.inf
> @@ -27,7 +27,7 @@
>  #
>  # The following information is for reference only and not required by the 
> build
> tools.
>  #
> -#  VALID_ARCHITECTURES   = IA32 X64 IPF EBC
> +#  VALID_ARCHITECTURES   = IA32 X64 EBC
>  #
> 
>  [Sources]
> --
> 2.16.2.windows.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 v2] Emulator/Win: Fix build failure using VS2015x86 or old WinSDK

[Ruiyu Ni]

When build with WinSDK <= Win10 TH2, the terminal over CMD.exe
doesn't work. Because Win10 later than TH2 starts to support VT
terminal.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Michael D Kinney 
Cc: Hao A Wu 
---
 EmulatorPkg/Win/Host/WinHost.c  |  2 +-
 EmulatorPkg/Win/Host/WinThunk.c | 23 ---
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/EmulatorPkg/Win/Host/WinHost.c b/EmulatorPkg/Win/Host/WinHost.c
index 9b98d5330f..65e8960eff 100644
--- a/EmulatorPkg/Win/Host/WinHost.c
+++ b/EmulatorPkg/Win/Host/WinHost.c
@@ -673,7 +673,7 @@ Returns:
   // Transfer control to the SEC Core
   //
   SwitchStack (
-(SWITCH_STACK_ENTRY_POINT)SecCoreEntryPoint,
+(SWITCH_STACK_ENTRY_POINT)(UINTN)SecCoreEntryPoint,
 SecCoreData,
 GetThunkPpiList (),
 TopOfStack
diff --git a/EmulatorPkg/Win/Host/WinThunk.c b/EmulatorPkg/Win/Host/WinThunk.c
index 306fe75ecd..6007db73b5 100644
--- a/EmulatorPkg/Win/Host/WinThunk.c
+++ b/EmulatorPkg/Win/Host/WinThunk.c
@@ -71,15 +71,23 @@ SecConfigStdIn (
 //
 // Disable buffer (line input), echo, mouse, window
 //
-Success = SetConsoleMode (
-GetStdHandle (STD_INPUT_HANDLE),
-Mode | ENABLE_VIRTUAL_TERMINAL_INPUT & ~(ENABLE_LINE_INPUT | 
ENABLE_ECHO_INPUT | ENABLE_MOUSE_INPUT | ENABLE_WINDOW_INPUT)
-);
-  }
-  if (Success) {
+Mode &= ~(ENABLE_LINE_INPUT | ENABLE_ECHO_INPUT | ENABLE_MOUSE_INPUT | 
ENABLE_WINDOW_INPUT);
+
+#if defined(NTDDI_VERSION) && defined (NTDDI_WIN10_TH2) && (NTDDI_VERSION > 
NTDDI_WIN10_TH2)
 //
-// Enable terminal mode
+// Enable virtual terminal input for Win10 above TH2
 //
+Mode |= ENABLE_VIRTUAL_TERMINAL_INPUT;
+#endif
+
+Success = SetConsoleMode (GetStdHandle (STD_INPUT_HANDLE), Mode);
+  }
+
+#if defined(NTDDI_VERSION) && defined (NTDDI_WIN10_TH2) && (NTDDI_VERSION > 
NTDDI_WIN10_TH2)
+  //
+  // Enable terminal mode for Win10 above TH2
+  //
+  if (Success) {
 Success = GetConsoleMode (GetStdHandle (STD_OUTPUT_HANDLE), &Mode);
 if (Success) {
   Success = SetConsoleMode (
@@ -88,6 +96,7 @@ SecConfigStdIn (
   );
 }
   }
+#endif
   return Success ? EFI_SUCCESS : EFI_DEVICE_ERROR;
 }
 
-- 
2.16.1.windows.1

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

[edk2] [PATCH v2 2/3] MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on IPF

[Ard Biesheuvel]

Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Jiewen Yao 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Star Zeng 
Reviewed-by: Liming Gao 
---
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++-
 1 file changed, 9 insertions(+), 52 deletions(-)

diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c 
b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 32eca0ad2ef4..c57816a80887 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -46,36 +46,6 @@ PeCoffLoaderAdjustOffsetForTeImage (
   SectionHeader->PointerToRawData -= TeStrippedOffset;
 }
 
-/**
-  Retrieves the magic value from the PE/COFF header.
-
-  @param  Hdr The buffer in which to return the PE32, PE32+, or TE 
header.
-
-  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
-  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-PeCoffLoaderGetPeHeaderMagicValue (
-  IN  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr
-  )
-{
-  //
-  // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
value
-  //   in the PE/COFF Header.  If the MachineType is Itanium(IA64) and the
-  //   Magic value in the OptionalHeader is  
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
-  //   then override the returned value to 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
-  //
-  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
-  }
-  //
-  // Return the magic value from the PC/COFF Optional Header
-  //
-  return Hdr.Pe32->OptionalHeader.Magic;
-}
-
-
 /**
   Retrieves the PE or TE Header from a PE/COFF or TE image.
 
@@ -101,7 +71,6 @@ PeCoffLoaderGetPeHeader (
   EFI_IMAGE_DOS_HEADER  DosHdr;
   UINTN Size;
   UINTN ReadSize;
-  UINT16Magic;
   UINT32SectionHeaderOffset;
   UINT32Index;
   UINT32HeaderWithoutDataDir;
@@ -222,9 +191,7 @@ PeCoffLoaderGetPeHeader (
 ImageContext->IsTeImage = FALSE;
 ImageContext->Machine = Hdr.Pe32->FileHeader.Machine;
 
-Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // 1. Check OptionalHeader.NumberOfRvaAndSizes filed.
   //
@@ -339,7 +306,7 @@ PeCoffLoaderGetPeHeader (
   ImageContext->SectionAlignment  = 
Hdr.Pe32->OptionalHeader.SectionAlignment;
   ImageContext->SizeOfHeaders = Hdr.Pe32->OptionalHeader.SizeOfHeaders;
 
-} else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+} else if (Hdr.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
   //
   // 1. Check FileHeader.NumberOfRvaAndSizes filed.
   //
@@ -605,7 +572,6 @@ PeCoffLoaderGetImageInfo (
   EFI_IMAGE_SECTION_HEADER  SectionHeader;
   EFI_IMAGE_DEBUG_DIRECTORY_ENTRY   DebugEntry;
   UINT32NumberOfRvaAndSizes;
-  UINT16Magic;
   UINT32TeStrippedOffset;
 
   if (ImageContext == NULL) {
@@ -622,14 +588,12 @@ PeCoffLoaderGetImageInfo (
 return Status;
   }
 
-  Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
   //
   // Retrieve the base address of the image
   //
   if (!(ImageContext->IsTeImage)) {
 TeStrippedOffset = 0;
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -678,7 +642,7 @@ PeCoffLoaderGetImageInfo (
   }
 
   if (!(ImageContext->IsTeImage)) {
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -952,7 +916,6 @@ PeCoffLoaderRelocateImage (
   CHAR8 *FixupData;
   PHYSICAL_ADDRESS  BaseAddress;
   UINT32NumberOfRvaAndSizes;
-  UINT16Magic;
   UINT32TeStrippedOffset;
 
   ASSERT (ImageContext != NULL);
@@ -985,9 +948,8 @@ PeCoffLoaderRelocateImage (
   if (!(ImageContext->IsTeImage)) {
 Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageContext->ImageAddress + 
ImageContext->PeCoffHeaderOffset);
 TeStrippedOffset = 0;
-Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
 
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
 

[edk2] [PATCH v2 3/3] SecurityPkg: remove PE/COFF header workaround for ELILO on IPF

[Ard Biesheuvel]

Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c| 
47 
 SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c  | 
27 +++
 SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c  | 
27 +++
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c | 
25 +++
 4 files changed, 25 insertions(+), 101 deletions(-)

diff --git 
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 0f795c0af125..66d96a9396b9 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -295,7 +295,6 @@ HashPeImage (
   )
 {
   BOOLEAN   Status;
-  UINT16Magic;
   EFI_IMAGE_SECTION_HEADER  *Section;
   VOID  *HashCtx;
   UINTN CtxSize;
@@ -367,33 +366,19 @@ HashPeImage (
   // Measuring PE/COFF Image Header;
   // But CheckSum field and SECURITY data directory (certificate) are excluded
   //
-  if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-//
-// NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
value
-//   in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
-//   Magic value in the OptionalHeader is 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
-//   then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
-//
-Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
-  } else {
-//
-// Get the magic value from the PE/COFF Optional Header
-//
-Magic =  mNtHeader.Pe32->OptionalHeader.Magic;
-  }
 
   //
   // 3.  Calculate the distance from the base of the image header to the image 
checksum address.
   // 4.  Hash the image header from its base to beginning of the image 
checksum.
   //
   HashBase = mImageBase;
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+  if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
 //
 // Use PE32 offset.
 //
 HashSize = (UINTN) (&mNtHeader.Pe32->OptionalHeader.CheckSum) - (UINTN) 
HashBase;
 NumberOfRvaAndSizes = mNtHeader.Pe32->OptionalHeader.NumberOfRvaAndSizes;
-  } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+  } else if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
 //
 // Use PE32+ offset.
 //
@@ -420,7 +405,7 @@ HashPeImage (
 // 6.  Since there is no Cert Directory in optional header, hash everything
 // from the end of the checksum to the end of image header.
 //
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset.
   //
@@ -444,7 +429,7 @@ HashPeImage (
 //
 // 7.  Hash everything from the end of the checksum to the start of the 
Cert Directory.
 //
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset.
   //
@@ -469,7 +454,7 @@ HashPeImage (
 // 8.  Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) 
bytes.)
 // 9.  Hash everything from the end of the Cert Directory to the end of 
image header.
 //
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -494,7 +479,7 @@ HashPeImage (
   //
   // 10. Set the SUM_OF_BYTES_HASHED to the size of the header.
   //
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+  if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
 //
 // Use PE32 offset.
 //
@@ -577,7 +562,7 @@ HashPeImage (
 if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
   CertSize = 0;
 } else {
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+  if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
 //
 // Use PE32 offset.
 //
@@ -1583,7 +1568,6 @@ DxeImageVerificationHandler (
   )
 {
   EFI_STATUS   Status;
-  UINT16   Magic;
   EFI_IMAGE_DOS_HEADER *DosHdr;
   EFI_STATUS   VerifyStatus;
   EFI_SIGNATURE_LIST   *SignatureList;
@@ -1723,2

[edk2] [PATCH v2 1/3] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF

[Ard Biesheuvel]

Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
Reviewed-by: Jiewen Yao 
Reviewed-by: Laszlo Ersek 
Reviewed-by: Star Zeng 
---
 MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c | 31 +---
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c   | 17 +--
 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c| 17 +--
 MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 17 +--
 MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c| 31 +---
 5 files changed, 5 insertions(+), 108 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c 
b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
index 83ed43a16e95..ff1940431c2f 100644
--- a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
+++ b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
@@ -254,35 +254,6 @@ GetMemoryProfileContext (
   return mMemoryProfileContextPtr;
 }
 
-/**
-  Retrieves the magic value from the PE/COFF header.
-
-  @param HdrThe buffer in which to return the PE32, PE32+, or TE header.
-
-  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
-  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-InternalPeCoffGetPeHeaderMagicValue (
-  IN  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr
-  )
-{
-  //
-  // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
value
-  //   in the PE/COFF Header.  If the MachineType is Itanium(IA64) and the
-  //   Magic value in the OptionalHeader is  
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
-  //   then override the returned value to 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
-  //
-  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
-  }
-  //
-  // Return the magic value from the PC/COFF Optional Header
-  //
-  return Hdr.Pe32->OptionalHeader.Magic;
-}
-
 /**
   Retrieves and returns the Subsystem of a PE/COFF image that has been loaded 
into system memory.
   If Pe32Data is NULL, then ASSERT().
@@ -319,7 +290,7 @@ InternalPeCoffGetSubsystem (
   if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
 return Hdr.Te->Subsystem;
   } else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE)  {
-Magic = InternalPeCoffGetPeHeaderMagicValue (Hdr);
+Magic = Hdr.Pe32->OptionalHeader.Magic;
 if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   return Hdr.Pe32->OptionalHeader.Subsystem;
 } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 4cd219c88efc..fa8f8fe91ac7 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -406,7 +406,6 @@ ProtectUefiImage (
   IMAGE_PROPERTIES_RECORD  *ImageRecord;
   CHAR8*PdbPointer;
   IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
-  UINT16   Magic;
   BOOLEAN  IsAligned;
   UINT32   ProtectionPolicy;
 
@@ -466,21 +465,7 @@ ProtectUefiImage (
   //
   // Get SectionAlignment
   //
-  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-//
-// NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
value
-//   in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
-//   Magic value in the OptionalHeader is 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
-//   then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
-//
-Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
-  } else {
-//
-// Get the magic value from the PE/COFF Optional Header
-//
-Magic = Hdr.Pe32->OptionalHeader.Magic;
-  }
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+  if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
 SectionAlignment  = Hdr.Pe32->OptionalHeader.SectionAlignment;
   } else {
 SectionAlignment  = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c 
b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index a96d442fbc64..05eb4f422b2f 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
@@ -1076,7 +1076,6 @@ InsertImageRecord (
   IMAGE_PROPERTIES_RECORD  *ImageRecord;
   CHAR8*PdbPointer;
   IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
-  UINT16   Magic;
 
   DEBUG ((EFI_D_VERBOSE, "InsertImageRecord - 0x%x\n", Ru

[edk2] [PATCH v2 0/3] remove most occurrences of ELILO on IPF PE/COFF header hack

[Ard Biesheuvel]

Now that Itanium support has been dropped from EDK2, we can remove most
occurrences of the ELILO PE/COFF loader hack from the code base.

Note that SecurityPkg appears to have four mostly identical implementations
of the PE/COFF measuring routine, so this may be another area for cleanup
later.

Changes since v1:
- fix copy/paste error in patch #3 which went unnoticed due to the fact that
  SecurityPkg.dsc does not cover the module in question for AARCH64
- drop EdkCompatibilityPkg patch, it is likely to go away soon anyway
- add Reviewed-by tags

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816

Cc: Star Zeng 
Cc: Jian J Wang 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Chao Zhang 
Cc: Jiewen Yao 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 

Ard Biesheuvel (3):
  MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF
  MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on
IPF
  SecurityPkg: remove PE/COFF header workaround for ELILO on IPF

 .../Core/Dxe/Mem/MemoryProfileRecord.c| 31 +-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 17 +-
 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c  | 17 +-
 .../Core/PiSmmCore/MemoryAttributesTable.c| 17 +-
 .../Core/PiSmmCore/SmramProfileRecord.c   | 31 +-
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++
 .../DxeImageVerificationLib.c | 47 +++---
 .../DxeTpmMeasureBootLib.c| 27 ++--
 SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c   | 27 ++--
 .../SecureBootConfigImpl.c| 25 ++--
 10 files changed, 39 insertions(+), 261 deletions(-)

-- 
2.17.1

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

Re: [edk2] [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on IPF

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: Zeng, Star
>Sent: Friday, September 07, 2018 1:15 PM
>To: Ard Biesheuvel ; edk2-devel@lists.01.org
>Cc: Wang, Jian J ; Kinney, Michael D
>; Gao, Liming ; Zhang,
>Chao B ; Yao, Jiewen ;
>Laszlo Ersek ; Leif Lindholm ;
>Zeng, Star 
>Subject: RE: [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header
>workaround for ELILO on IPF
>
>Reviewed-by: Star Zeng 
>
>-Original Message-
>From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>Sent: Thursday, September 6, 2018 9:45 PM
>To: edk2-devel@lists.01.org
>Cc: Ard Biesheuvel ; Zeng, Star
>; Wang, Jian J ; Kinney,
>Michael D ; Gao, Liming
>; Zhang, Chao B ; Yao,
>Jiewen ; Laszlo Ersek ; Leif
>Lindholm 
>Subject: [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header
>workaround for ELILO on IPF
>
>Now that Itanium support has been dropped, we can remove the various
>occurrences of the ELILO on Itanium PE/COFF header workaround.
>
>Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
>Contributed-under: TianoCore Contribution Agreement 1.1
>Signed-off-by: Ard Biesheuvel 
>---
> MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++-
> 1 file changed, 9 insertions(+), 52 deletions(-)
>
>diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>index 32eca0ad2ef4..c57816a80887 100644
>--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
>@@ -46,36 +46,6 @@ PeCoffLoaderAdjustOffsetForTeImage (
>   SectionHeader->PointerToRawData -= TeStrippedOffset;  }
>
>-/**
>-  Retrieves the magic value from the PE/COFF header.
>-
>-  @param  Hdr The buffer in which to return the PE32, PE32+, or TE
>header.
>-
>-  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
>-  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
>-
>-**/
>-UINT16
>-PeCoffLoaderGetPeHeaderMagicValue (
>-  IN  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr
>-  )
>-{
>-  //
>-  // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic
>value
>-  //   in the PE/COFF Header.  If the MachineType is Itanium(IA64) and the
>-  //   Magic value in the OptionalHeader is
>EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
>-  //   then override the returned value to
>EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
>-  //
>-  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 &&
>Hdr.Pe32->OptionalHeader.Magic ==
>EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>-return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
>-  }
>-  //
>-  // Return the magic value from the PC/COFF Optional Header
>-  //
>-  return Hdr.Pe32->OptionalHeader.Magic; -}
>-
>-
> /**
>   Retrieves the PE or TE Header from a PE/COFF or TE image.
>
>@@ -101,7 +71,6 @@ PeCoffLoaderGetPeHeader (
>   EFI_IMAGE_DOS_HEADER  DosHdr;
>   UINTN Size;
>   UINTN ReadSize;
>-  UINT16Magic;
>   UINT32SectionHeaderOffset;
>   UINT32Index;
>   UINT32HeaderWithoutDataDir;
>@@ -222,9 +191,7 @@ PeCoffLoaderGetPeHeader (
> ImageContext->IsTeImage = FALSE;
> ImageContext->Machine = Hdr.Pe32->FileHeader.Machine;
>
>-Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
>-
>-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>+if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>   //
>   // 1. Check OptionalHeader.NumberOfRvaAndSizes filed.
>   //
>@@ -339,7 +306,7 @@ PeCoffLoaderGetPeHeader (
>   ImageContext->SectionAlignment  = Hdr.Pe32-
>>OptionalHeader.SectionAlignment;
>   ImageContext->SizeOfHeaders = Hdr.Pe32-
>>OptionalHeader.SizeOfHeaders;
>
>-} else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
>+} else if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
>   //
>   // 1. Check FileHeader.NumberOfRvaAndSizes filed.
>   //
>@@ -605,7 +572,6 @@ PeCoffLoaderGetImageInfo (
>   EFI_IMAGE_SECTION_HEADER  SectionHeader;
>   EFI_IMAGE_DEBUG_DIRECTORY_ENTRY   DebugEntry;
>   UINT32NumberOfRvaAndSizes;
>-  UINT16Magic;
>   UINT32TeStrippedOffset;
>
>   if (ImageContext == NULL) {
>@@ -622,14 +588,12 @@ PeCoffLoaderGetImageInfo (
> return Status;
>   }
>
>-  Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
>-
>   //
>   // Retrieve the base address of the image
>   //
>   if (!(ImageContext->IsTeImage)) {
> TeStrippedOffset = 0;
>-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>+if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>   //
>   // Use PE32 offset
>   //
>@@ -678,7 +642,7 @@ PeCoffLoaderGetImageInfo (
>   }
>
>   if (!(ImageContext->IsTeImage)) {
>-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>+if (Hdr.Pe32->OptionalHeader.Magic ==
>+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>   //
>   // Use 

Re: [edk2] [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on IPF

[Zeng, Star]

Reviewed-by: Star Zeng 

-Original Message-
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] 
Sent: Thursday, September 6, 2018 9:45 PM
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel ; Zeng, Star 
; Wang, Jian J ; Kinney, Michael D 
; Gao, Liming ; Zhang, Chao B 
; Yao, Jiewen ; Laszlo Ersek 
; Leif Lindholm 
Subject: [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header workaround for 
ELILO on IPF

Now that Itanium support has been dropped, we can remove the various 
occurrences of the ELILO on Itanium PE/COFF header workaround.

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++-
 1 file changed, 9 insertions(+), 52 deletions(-)

diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c 
b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 32eca0ad2ef4..c57816a80887 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -46,36 +46,6 @@ PeCoffLoaderAdjustOffsetForTeImage (
   SectionHeader->PointerToRawData -= TeStrippedOffset;  }
 
-/**
-  Retrieves the magic value from the PE/COFF header.
-
-  @param  Hdr The buffer in which to return the PE32, PE32+, or TE 
header.
-
-  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
-  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-PeCoffLoaderGetPeHeaderMagicValue (
-  IN  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr
-  )
-{
-  //
-  // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
value
-  //   in the PE/COFF Header.  If the MachineType is Itanium(IA64) and the
-  //   Magic value in the OptionalHeader is  
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
-  //   then override the returned value to 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
-  //
-  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
-  }
-  //
-  // Return the magic value from the PC/COFF Optional Header
-  //
-  return Hdr.Pe32->OptionalHeader.Magic; -}
-
-
 /**
   Retrieves the PE or TE Header from a PE/COFF or TE image.
 
@@ -101,7 +71,6 @@ PeCoffLoaderGetPeHeader (
   EFI_IMAGE_DOS_HEADER  DosHdr;
   UINTN Size;
   UINTN ReadSize;
-  UINT16Magic;
   UINT32SectionHeaderOffset;
   UINT32Index;
   UINT32HeaderWithoutDataDir;
@@ -222,9 +191,7 @@ PeCoffLoaderGetPeHeader (
 ImageContext->IsTeImage = FALSE;
 ImageContext->Machine = Hdr.Pe32->FileHeader.Machine;
 
-Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == 
+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // 1. Check OptionalHeader.NumberOfRvaAndSizes filed.
   //
@@ -339,7 +306,7 @@ PeCoffLoaderGetPeHeader (
   ImageContext->SectionAlignment  = 
Hdr.Pe32->OptionalHeader.SectionAlignment;
   ImageContext->SizeOfHeaders = Hdr.Pe32->OptionalHeader.SizeOfHeaders;
 
-} else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+} else if (Hdr.Pe32->OptionalHeader.Magic == 
+ EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
   //
   // 1. Check FileHeader.NumberOfRvaAndSizes filed.
   //
@@ -605,7 +572,6 @@ PeCoffLoaderGetImageInfo (
   EFI_IMAGE_SECTION_HEADER  SectionHeader;
   EFI_IMAGE_DEBUG_DIRECTORY_ENTRY   DebugEntry;
   UINT32NumberOfRvaAndSizes;
-  UINT16Magic;
   UINT32TeStrippedOffset;
 
   if (ImageContext == NULL) {
@@ -622,14 +588,12 @@ PeCoffLoaderGetImageInfo (
 return Status;
   }
 
-  Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
   //
   // Retrieve the base address of the image
   //
   if (!(ImageContext->IsTeImage)) {
 TeStrippedOffset = 0;
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == 
+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -678,7 +642,7 @@ PeCoffLoaderGetImageInfo (
   }
 
   if (!(ImageContext->IsTeImage)) {
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == 
+ EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -952,7 +916,6 @@ PeCoffLoaderRelocateImage (
   CHAR8 *FixupData;
   PHYSICAL_ADDRESS  BaseAddress;
   UINT32NumberOfRvaAndSizes;
-  UINT16Magic;
   UINT32TeStrippedOffset;
 
   ASSERT (ImageContext != NULL);
@@ -985,9 +948,8 @@ PeCoffLoaderRelocateImage (
   if (!(ImageContext->IsTeImage)) {
 Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS3

Re: [edk2] [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF

[Zeng, Star]

Reviewed-by: Star Zeng 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
Biesheuvel
Sent: Thursday, September 6, 2018 9:45 PM
To: edk2-devel@lists.01.org
Cc: Gao, Liming ; Yao, Jiewen ; 
Zeng, Star ; Kinney, Michael D 
; Laszlo Ersek ; Zhang, Chao B 

Subject: [edk2] [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for 
ELILO on IPF

Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c | 31 +---
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c   | 17 +--
 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c| 17 +--
 MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 17 +--
 MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c| 31 +---
 5 files changed, 5 insertions(+), 108 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c 
b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
index 83ed43a16e95..ff1940431c2f 100644
--- a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
+++ b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
@@ -254,35 +254,6 @@ GetMemoryProfileContext (
   return mMemoryProfileContextPtr;
 }
 
-/**
-  Retrieves the magic value from the PE/COFF header.
-
-  @param HdrThe buffer in which to return the PE32, PE32+, or TE header.
-
-  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
-  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-InternalPeCoffGetPeHeaderMagicValue (
-  IN  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr
-  )
-{
-  //
-  // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
value
-  //   in the PE/COFF Header.  If the MachineType is Itanium(IA64) and the
-  //   Magic value in the OptionalHeader is  
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
-  //   then override the returned value to 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
-  //
-  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
-  }
-  //
-  // Return the magic value from the PC/COFF Optional Header
-  //
-  return Hdr.Pe32->OptionalHeader.Magic;
-}
-
 /**
   Retrieves and returns the Subsystem of a PE/COFF image that has been loaded 
into system memory.
   If Pe32Data is NULL, then ASSERT().
@@ -319,7 +290,7 @@ InternalPeCoffGetSubsystem (
   if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
 return Hdr.Te->Subsystem;
   } else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE)  {
-Magic = InternalPeCoffGetPeHeaderMagicValue (Hdr);
+Magic = Hdr.Pe32->OptionalHeader.Magic;
 if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   return Hdr.Pe32->OptionalHeader.Subsystem;
 } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 4cd219c88efc..fa8f8fe91ac7 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -406,7 +406,6 @@ ProtectUefiImage (
   IMAGE_PROPERTIES_RECORD  *ImageRecord;
   CHAR8*PdbPointer;
   IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
-  UINT16   Magic;
   BOOLEAN  IsAligned;
   UINT32   ProtectionPolicy;
 
@@ -466,21 +465,7 @@ ProtectUefiImage (
   //
   // Get SectionAlignment
   //
-  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-//
-// NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
value
-//   in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
-//   Magic value in the OptionalHeader is 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
-//   then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
-//
-Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
-  } else {
-//
-// Get the magic value from the PE/COFF Optional Header
-//
-Magic = Hdr.Pe32->OptionalHeader.Magic;
-  }
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+  if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
 SectionAlignment  = Hdr.Pe32->OptionalHeader.SectionAlignment;
   } else {
 SectionAlignment  = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c 
b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index a96d442fbc64..05eb4f422b2f 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTabl

Re: [edk2] [PATCH] ShellPkg Shell: Remove an unused global variable

[Ni, Ruiyu]

Reviewed-by: Ruiyu Ni 

Thanks/Ray

> -Original Message-
> From: edk2-devel  On Behalf Of shenglei
> Sent: Friday, September 7, 2018 9:52 AM
> To: edk2-devel@lists.01.org
> Cc: Carsey, Jaben ; Ni, Ruiyu 
> Subject: [edk2] [PATCH] ShellPkg Shell: Remove an unused global variable
> 
> The unused global variable InvalidChars is removed.
> It is only used in the function IsValidCommandName which was removed
> previously.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1066
> 
> Cc: Jaben Carsey 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: shenglei 
> ---
>  ShellPkg/Application/Shell/Shell.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/ShellPkg/Application/Shell/Shell.c
> b/ShellPkg/Application/Shell/Shell.c
> index 397cfd1994..3f3bcbb4b0 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -2750,9 +2750,6 @@ RunCommand(
>return (RunShellCommand(CmdLine, NULL));  }
> 
> -
> -STATIC CONST UINT16 InvalidChars[] = {L'*', L'?', L'<', L'>', L'\\', L'/', 
> L'\"',
> 0x0001, 0x0002};
> -
>  /**
>Function to process a NSH script file via SHELL_FILE_HANDLE.
> 
> --
> 2.18.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] MdeModulePkg PeiCore: Always use PeiImageRead() function to load PEI image

[Zeng, Star]

Reviewed-by: Star Zeng 

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Liming 
Gao
Sent: Wednesday, September 5, 2018 10:18 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH v2] MdeModulePkg PeiCore: Always use PeiImageRead() 
function to load PEI image

In V2, Remove GetImageReadFunction(), directly use PeiImageRead().

The copy PeiImageReadForShadow function doesn't improve the boot performance.
This patch removes this copy logic to simplify the code logic.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Liming Gao 
---
 MdeModulePkg/Core/Pei/Image/Image.c | 85 +
 1 file changed, 1 insertion(+), 84 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/Image/Image.c 
b/MdeModulePkg/Core/Pei/Image/Image.c
index f84f2c7..315b551 100644
--- a/MdeModulePkg/Core/Pei/Image/Image.c
+++ b/MdeModulePkg/Core/Pei/Image/Image.c
@@ -61,87 +61,6 @@ PeiImageRead (
 }
 
 /**
-
-  Support routine for the PE/COFF Loader that reads a buffer from a PE/COFF 
file.
-  The function is implemented as PIC so as to support shadowing.
-
-  @param FileHandle  - The handle to the PE/COFF file
-  @param FileOffset  - The offset, in bytes, into the file to read
-  @param ReadSize- The number of bytes to read from the file starting 
at FileOffset
-  @param Buffer  - A pointer to the buffer to read the data into.
-
-  @return EFI_SUCCESS - ReadSize bytes of data were read into Buffer from the 
PE/COFF file starting at FileOffset
-
-**/
-EFI_STATUS
-EFIAPI
-PeiImageReadForShadow (
-  IN VOID*FileHandle,
-  IN UINTN   FileOffset,
-  IN UINTN   *ReadSize,
-  OUTVOID*Buffer
-  )
-{
-  volatile CHAR8  *Destination8;
-  CHAR8   *Source8;
-  UINTN   Length;
-
-  Destination8  = Buffer;
-  Source8   = (CHAR8 *) ((UINTN) FileHandle + FileOffset);
-  if (Destination8 != Source8) {
-Length= *ReadSize;
-while ((Length--) > 0) {
-  *(Destination8++) = *(Source8++);
-}
-  }
-
-  return EFI_SUCCESS;
-}
-
-/**
-
-  Support routine to get the Image read file function.
-
-  @param ImageContext- The context of the image being loaded
-
-  @retval EFI_SUCCESS - If Image function location is found
-
-**/
-EFI_STATUS
-GetImageReadFunction (
-  IN  PE_COFF_LOADER_IMAGE_CONTEXT  *ImageContext
-  )
-{
-#if defined (MDE_CPU_IA32) || defined (MDE_CPU_X64)
-  PEI_CORE_INSTANCE *Private;
-  EFI_PHYSICAL_ADDRESS  MemoryBuffer;
-
-  Private = PEI_CORE_INSTANCE_FROM_PS_THIS (GetPeiServicesTablePointer ());
-  MemoryBuffer = 0;
-
-  if (Private->PeiMemoryInstalled  && 
(((Private->HobList.HandoffInformationTable->BootMode != BOOT_ON_S3_RESUME) && 
PcdGetBool (PcdShadowPeimOnBoot)) ||
-  ((Private->HobList.HandoffInformationTable->BootMode == 
BOOT_ON_S3_RESUME) && PcdGetBool (PcdShadowPeimOnS3Boot {
-//
-// Shadow algorithm makes lots of non ANSI C assumptions and only works 
for IA32 and X64
-//  compilers that have been tested
-//
-if (Private->ShadowedImageRead == NULL) {
-  PeiServicesAllocatePages (EfiBootServicesCode, 0x400 / EFI_PAGE_SIZE + 
1, &MemoryBuffer);
-  ASSERT (MemoryBuffer != 0);
-  CopyMem ((VOID *)(UINTN)MemoryBuffer, (CONST VOID *) (UINTN) 
PeiImageReadForShadow, 0x400);
-  Private->ShadowedImageRead = (PE_COFF_LOADER_READ_FILE) (UINTN) 
MemoryBuffer;
-}
-
-ImageContext->ImageRead = Private->ShadowedImageRead;
-  } else {
-ImageContext->ImageRead = PeiImageRead;
-  }
-#else
-  ImageContext->ImageRead = PeiImageRead; -#endif
-  return EFI_SUCCESS;
-}
-/**
   To check memory usage bit map array to figure out if the memory range the 
image will be loaded in is available or not. If
   memory range is available, the function will mark the corresponding bits to 
1 which indicates the memory range is used.
   The function is only invoked when load modules at fixed address feature is 
enabled.
@@ -369,9 +288,7 @@ LoadAndRelocatePeCoffImage (
   IsXipImage   = FALSE;
   ZeroMem (&ImageContext, sizeof (ImageContext));
   ImageContext.Handle = Pe32Data;
-  Status  = GetImageReadFunction (&ImageContext);
-
-  ASSERT_EFI_ERROR (Status);
+  ImageContext.ImageRead = PeiImageRead;
 
   Status = PeCoffLoaderGetImageInfo (&ImageContext);
   if (EFI_ERROR (Status)) {
--
2.10.0.windows.1

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

Re: [edk2] [PATCH 1/1] FatPkg/EnhancedFatDxe Fix Double Cluster Allocation

[Robinson, Herbie]

I forgot to mention this is entered as Bug 1157 in Bugzilla.

Herbie Robinson
Software Architect
Stratus Technologies | www.stratus.com
5 Mill and Main Place, Suite 500 | Maynard, MA 01754
T: +1-978-461-7531 | E: herbie.robin...@stratus.com
[Stratus Technologies]

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

Re: [edk2] Performance enabling of Event handler

[Bi, Dandan]

Hi Prabin,

The Performance logging for the normal functions handlers and event handlers 
should be the same.
Are the normal function calls and the event handler function calls you tested 
in the same module?
If not, please double check to make sure the performance library instance used 
correctly for each module.


Thanks,
Dandan

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of prabin ca
Sent: Friday, September 7, 2018 10:30 AM
To: Laszlo Ersek 
Cc: Bi, Dandan ; edk2-devel@lists.01.org; af...@apple.com
Subject: Re: [edk2] Performance enabling of Event handler

Hi,
PerformancePkg is not working with event handlers, but it’s working with normal 
functions handlers. 

> On 06-Sep-2018, at 3:28 PM, Laszlo Ersek  wrote:
> 
>> On 09/06/18 08:10, prabin ca wrote:
>> Hi Team,
>> 
>> I’m used edk2 PerformancePkg for profiling cpu execution time taken by a 
>> event handler. Event is created successfully and event handler is also 
>> called successfully, but I can capture the performance of this event handler 
>> with PerformancePkg (by using perf_start and perf_end check points). This 
>> PerformancePkg is working fine with normal function calls.
> 
> Do you mean "can not", instead of "can"? (Sorry, I don't understand.)
> 
>> 
>> Please help me to enable PerformancePkg action on event handler also.
>> 
> 
> Hmmm, even with the suggested typo correction, I wouldn't know what to 
> suggest. Sorry!
> 
> Laszlo
___
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] Maintainer.txt: Add Ray to be co-maintainer of EmulatorPkg

[Jordan Justen]

Reviewed-by: Jordan Justen 

On 2018-09-06 06:27:17, Ruiyu Ni wrote:
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni 
> Cc: Jordan Justen 
> Cc: Andrew Fish 
> ---
>  Maintainers.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Maintainers.txt b/Maintainers.txt
> index 61df6e198b..7ebd53f662 100644
> --- a/Maintainers.txt
> +++ b/Maintainers.txt
> @@ -124,6 +124,7 @@ EmulatorPkg
>  W: https://github.com/tianocore/tianocore.github.io/wiki/EmulatorPkg
>  M: Jordan Justen 
>  M: Andrew Fish 
> +M: Ruiyu Ni 
>  S: Maintained
>  
>  FatPkg, FatBinPkg
> -- 
> 2.16.1.windows.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] Drop the support of BaseTools Python run from the freeze binary in Windows OS

[Gao, Liming]

Hi, all
  Now, we are working to migrate BaseTools Python tool from Python27 to 
Python36. To reduce the migration effort, we will only verify BaseTools Python 
tool run from source. That means we will stop to support the freeze python tool 
as windows exe. In fact, we get the report that edk2-BaseTools-win32 doesn't 
work. We don't plan to fix it. If you still use the freeze python tool exe, 
please run BaseTools Python from source in Windows. Here is the step wiki 
https://github.com/tianocore/tianocore.github.io/wiki/Windows-systems Compile 
Tools section. After migrate to Python36, we will update wiki page to document 
Python36 step.

>From now, we will stop to freeze python tool, remove 
>Edk2\BaseTools\Source\Python\Makefile, and stop update edk2-BaseTools-win32 
>github repo. And, I will reset edk2-BaseTools-win32 binary to match edk2 
>vUDK2018 tag. If you require the freeze python tool, you can maintain your 
>step to freeze the python tool. Here is the link 
>https://docs.python-guide.org/shipping/freezing/ to introduce the different 
>freezing solutions.

Thanks
Liming

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

Re: [edk2] Performance enabling of Event handler

[Andrew Fish]

Prabin,

Last time I looked every call to PERF_START()/PERF_END() creates a new 
performance record. So it does not work well for a function that is called a 
large number of times. 

Thanks,

Andrew Fish

> On Sep 6, 2018, at 7:30 PM, prabin ca  wrote:
> 
> Hi,
> PerformancePkg is not working with event handlers, but it’s working with 
> normal functions handlers. 
> 
>> On 06-Sep-2018, at 3:28 PM, Laszlo Ersek  wrote:
>> 
>>> On 09/06/18 08:10, prabin ca wrote:
>>> Hi Team,
>>> 
>>> I’m used edk2 PerformancePkg for profiling cpu execution time taken by a 
>>> event handler. Event is created successfully and event handler is also 
>>> called successfully, but I can capture the performance of this event 
>>> handler with PerformancePkg (by using perf_start and perf_end check 
>>> points). This PerformancePkg is working fine with normal function calls.
>> 
>> Do you mean "can not", instead of "can"? (Sorry, I don't understand.)
>> 
>>> 
>>> Please help me to enable PerformancePkg action on event handler also.
>>> 
>> 
>> Hmmm, even with the suggested typo correction, I wouldn't know what to
>> suggest. Sorry!
>> 
>> Laszlo

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

Re: [edk2] Performance enabling of Event handler

[prabin ca]

Hi,
PerformancePkg is not working with event handlers, but it’s working with normal 
functions handlers. 

> On 06-Sep-2018, at 3:28 PM, Laszlo Ersek  wrote:
> 
>> On 09/06/18 08:10, prabin ca wrote:
>> Hi Team,
>> 
>> I’m used edk2 PerformancePkg for profiling cpu execution time taken by a 
>> event handler. Event is created successfully and event handler is also 
>> called successfully, but I can capture the performance of this event handler 
>> with PerformancePkg (by using perf_start and perf_end check points). This 
>> PerformancePkg is working fine with normal function calls.
> 
> Do you mean "can not", instead of "can"? (Sorry, I don't understand.)
> 
>> 
>> Please help me to enable PerformancePkg action on event handler also.
>> 
> 
> Hmmm, even with the suggested typo correction, I wouldn't know what to
> suggest. Sorry!
> 
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] ShellPkg Shell: Remove an unused global variable

[shenglei]

The unused global variable InvalidChars is removed.
It is only used in the function IsValidCommandName which
was removed previously.
https://bugzilla.tianocore.org/show_bug.cgi?id=1066

Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: shenglei 
---
 ShellPkg/Application/Shell/Shell.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/ShellPkg/Application/Shell/Shell.c 
b/ShellPkg/Application/Shell/Shell.c
index 397cfd1994..3f3bcbb4b0 100644
--- a/ShellPkg/Application/Shell/Shell.c
+++ b/ShellPkg/Application/Shell/Shell.c
@@ -2750,9 +2750,6 @@ RunCommand(
   return (RunShellCommand(CmdLine, NULL));
 }
 
-
-STATIC CONST UINT16 InvalidChars[] = {L'*', L'?', L'<', L'>', L'\\', L'/', 
L'\"', 0x0001, 0x0002};
-
 /**
   Function to process a NSH script file via SHELL_FILE_HANDLE.
 
-- 
2.18.0.windows.1

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

[edk2] [Patch] BaseTool: Variable Merge.

[BobCF]

If Structure PCD and Normal Pcd refer to the
same variable, do variable merge.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Bob Feng 
Cc: Liming Gao 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py |  3 +-
 BaseTools/Source/Python/AutoGen/GenVar.py  | 71 --
 .../Source/Python/Workspace/BuildClassObject.py|  2 +
 BaseTools/Source/Python/Workspace/DscBuildData.py  |  3 +-
 4 files changed, 60 insertions(+), 19 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index 95370d1821..d1bd1b1d4c 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -1211,11 +1211,11 @@ class PlatformAutoGen(AutoGen):
 continue
 if len(Sku.VariableName) > 0:
 VariableGuidStructure = Sku.VariableGuidValue
 VariableGuid = 
GuidStructureStringToGuidString(VariableGuidStructure)
 for StorageName in Sku.DefaultStoreDict:
-VariableInfo.append_variable(var_info(Index, pcdname, 
StorageName, SkuName, StringToArray(Sku.VariableName), VariableGuid, 
Sku.VariableOffset, Sku.VariableAttribute, Sku.HiiDefaultValue, 
Sku.DefaultStoreDict[StorageName], Pcd.DatumType))
+VariableInfo.append_variable(var_info(Index, pcdname, 
StorageName, SkuName, StringToArray(Sku.VariableName), VariableGuid, 
Sku.VariableOffset, Sku.VariableAttribute, Sku.HiiDefaultValue, 
Sku.DefaultStoreDict[StorageName], Pcd.DatumType, 
Pcd.CustomAttribute['DscPosition'], Pcd.CustomAttribute.get('IsStru',False)))
 Index += 1
 return VariableInfo
 
 def UpdateNVStoreMaxSize(self, OrgVpdFile):
 if self.VariableInfo:
@@ -2104,10 +2104,11 @@ class PlatformAutoGen(AutoGen):
 EdkLogger.error('build', FORMAT_INVALID, Cause, 
File=self.MetaFile,
 ExtraData="%s.%s" % 
(ToPcd.TokenSpaceGuidCName, TokenCName))
 ToPcd.validateranges = FromPcd.validateranges
 ToPcd.validlists = FromPcd.validlists
 ToPcd.expressions = FromPcd.expressions
+ToPcd.CustomAttribute = FromPcd.CustomAttribute
 
 if FromPcd is not None and ToPcd.DatumType == TAB_VOID and not 
ToPcd.MaxDatumSize:
 EdkLogger.debug(EdkLogger.DEBUG_9, "No MaxDatumSize specified for 
PCD %s.%s" \
 % (ToPcd.TokenSpaceGuidCName, TokenCName))
 Value = ToPcd.DefaultValue
diff --git a/BaseTools/Source/Python/AutoGen/GenVar.py 
b/BaseTools/Source/Python/AutoGen/GenVar.py
index 75d455b407..654ff7b14e 100644
--- a/BaseTools/Source/Python/AutoGen/GenVar.py
+++ b/BaseTools/Source/Python/AutoGen/GenVar.py
@@ -20,11 +20,11 @@ import copy
 from Common.VariableAttributes import VariableAttributes
 from Common.Misc import *
 import collections
 import Common.DataType as DataType
 
-var_info = collections.namedtuple("uefi_var", 
"pcdindex,pcdname,defaultstoragename,skuname,var_name, var_guid, 
var_offset,var_attribute,pcd_default_value, default_value, data_type")
+var_info = collections.namedtuple("uefi_var", 
"pcdindex,pcdname,defaultstoragename,skuname,var_name, var_guid, 
var_offset,var_attribute,pcd_default_value, default_value, 
data_type,PcdDscLine,StructurePcd")
 NvStorageHeaderSize = 28
 VariableHeaderSize = 32
 
 class VariableMgr(object):
 def __init__(self, DefaultStoreMap, SkuIdMap):
@@ -54,37 +54,74 @@ class VariableMgr(object):
 value_str = "{"
 default_var_bin_strip = [ data.strip("""'""") for data in 
default_var_bin]
 value_str += ",".join(default_var_bin_strip)
 value_str += "}"
 return value_str
+def Do_combine(self,sku_var_info_offset_list):
+newvalue = {}
+for item in sku_var_info_offset_list:
+data_type = item.data_type
+value_list = item.default_value.strip("{").strip("}").split(",")
+if data_type in DataType.TAB_PCD_NUMERIC_TYPES:
+data_flag = 
DataType.PACK_CODE_BY_SIZE[MAX_SIZE_TYPE[data_type]]
+data = value_list[0]
+value_list = []
+for data_byte in pack(data_flag, int(data, 16) if 
data.upper().startswith('0X') else int(data)):
+value_list.append(hex(unpack("B", data_byte)[0]))
+newvalue[int(item.var_offset, 16) if 
item.var_offset.upper().startswith("0X") else int(item.var_offset)] = value_list
+try:
+newvaluestr = "{" + 
",".join(VariableMgr.assemble_variable(newvalue)) +"}"
+except:
+EdkLogger.error("build", AUTOGEN_ERROR, "Variable offset conflict 
in PCDs: %s \n" % (" and ".join(item.pcdname for item in 
sku_var_info_offset_list)))
+return newvaluestr
+def Do_Merge(self,sku_var_info_offset_list):
+StructrurePcds = sorted([item for item in sku_var_info_offset_list

Re: [edk2] [PATCH] ShellPkg Shell: Remove redundant functions

[Gao, Liming]

Laszlo:
  I ask Shenglei to provide the patch to fix this failure. Today, I meet with 
the failure in XCODE5 tool chain. 

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
> Ersek
> Sent: Friday, September 7, 2018 12:19 AM
> To: Gao, Liming ; Ard Biesheuvel 
> 
> Cc: Carsey, Jaben ; Ni, Ruiyu ; 
> edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] ShellPkg Shell: Remove redundant functions
> 
> On 09/06/18 17:22, Gao, Liming wrote:
> > The build log shows the error message. I agree to remove this unused global 
> > variable InvalidChars in this patch.
> >
> > /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/ShellPkg/Application/Shell/Shell.c:2754:21:
> >  error: unused
> variable 'InvalidChars' [-Werror,-Wunused-const-variable]
> > 12:23:26 STATIC CONST UINT16 InvalidChars[] = {L'*', L'?', L'<', L'>', 
> > L'\\', L'/', L'\"', 0x0001, 0x0002};
> 
> OK... should we file a TianoCore BZ about this and wait until it gets
> assigned and fixed, should we ask Shenglei just on the list to post a
> follow-up (like Ard already did), or should one of us that actually
> encounters the error post the one-liner patch?
> 
> (I would have already posted the patch if I could actually reproduce the
> error.)
> 
> Thanks
> Laszlo
> 
> >> -Original Message-
> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
> >> Biesheuvel
> >> Sent: Thursday, September 6, 2018 8:31 PM
> >> To: Laszlo Ersek 
> >> Cc: Carsey, Jaben ; Ni, Ruiyu 
> >> ; edk2-devel@lists.01.org
> >> Subject: Re: [edk2] [PATCH] ShellPkg Shell: Remove redundant functions
> >>
> >> On 6 September 2018 at 14:24, Laszlo Ersek  wrote:
> >>> On 09/06/18 12:28, Ard Biesheuvel wrote:
>  On 9 August 2018 at 17:41,   wrote:
> > The InvalidChars[] array is only used in function IsValidCommandName().
> > The array should be deleted also, I think.
> >
> 
>  Indeed, and for this reason this patch has now broken the build for
>  clang. Please fix.
> >>>
> >>> I agree about InvalidChars being unused, post-22cf747fcf75. I just can't
> >>> reproduce the build error, with CLANG38 (3.8.1). The command
> >>>
> >>>   build -a X64 -p ShellPkg/ShellPkg.dsc -b RELEASE -t CLANG38 \
> >>> -m ShellPkg/Application/Shell/Shell.inf
> >>>
> >>> seems to work for me, also with "-b DEBUG".
> >>>
> >>> What differs on your end?
> >>>
> >>
> >> Not sure what the exact difference is, but I am using CLANG35 profile:
> >>
> >> https://ci.linaro.org/job/leg-virt-tianocore-edk2-upstream/3176/consoleFull
> >> ___
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v1 1/1] BaseTools: refactor to remove duplicate functions

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: Carsey, Jaben 
Sent: Thursday, September 06, 2018 5:51 AM
To: edk2-devel@lists.01.org
Cc: Zhu, Yonghong ; Gao, Liming ; 
Feng, Bob C 
Subject: [PATCH v1 1/1] BaseTools: refactor to remove duplicate functions

Update GenFdsGlobalVariable GetAlignment to support G.
replace use of local function in Region with updated shared function.

Cc: Yonghong Zhu 
Cc: Liming Gao 
Cc: Bob C Feng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py | 11 +-
 BaseTools/Source/Python/GenFds/Region.py   | 21 
+---
 2 files changed, 7 insertions(+), 25 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py 
b/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py
index eb106b574420..77873d36b98a 100644
--- a/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py
+++ b/BaseTools/Source/Python/GenFds/GenFdsGlobalVariable.py
@@ -512,14 +512,15 @@ class GenFdsGlobalVariable:
 
 @staticmethod
 def GetAlignment (AlignString):
-if AlignString is None:
+if not AlignString:
 return 0
-if AlignString in ("1K", "2K", "4K", "8K", "16K", "32K", "64K", 
"128K", "256K", "512K"):
+if AlignString.endswith('K'):
 return int (AlignString.rstrip('K')) * 1024
-elif AlignString in ("1M", "2M", "4M", "8M", "16M"):
+if AlignString.endswith('M'):
 return int (AlignString.rstrip('M')) * 1024 * 1024
-else:
-return int (AlignString)
+if AlignString.endswith('G'):
+return int (AlignString.rstrip('G')) * 1024 * 1024 * 1024
+return int (AlignString)
 
 @staticmethod
 def GenerateFfs(Output, Input, Type, Guid, Fixed=False, CheckSum=False, 
Align=None, diff --git a/BaseTools/Source/Python/GenFds/Region.py 
b/BaseTools/Source/Python/GenFds/Region.py
index 7f94b3d66b55..5242b74c9e70 100644
--- a/BaseTools/Source/Python/GenFds/Region.py
+++ b/BaseTools/Source/Python/GenFds/Region.py
@@ -124,7 +124,7 @@ class Region(RegionClassObject):
 #
 self.BlockInfoOfRegion(BlockSizeList, FvObj)
 self.FvAddress = self.FvAddress + FvOffset
-FvAlignValue = self.GetFvAlignValue(FvObj.FvAlignment)
+FvAlignValue = 
+ GenFdsGlobalVariable.GetAlignment(FvObj.FvAlignment)
 if self.FvAddress % FvAlignValue != 0:
 EdkLogger.error("GenFds", GENFDS_ERROR,
 "FV (%s) is NOT %s Aligned!" % 
(FvObj.UiFvName, FvObj.FvAlignment)) @@ -277,25 +277,6 @@ class 
Region(RegionClassObject):
 GenFdsGlobalVariable.InfLogger('   Region Name = None')
 self.PadBuffer(Buffer, ErasePolarity, Size)
 
-def GetFvAlignValue(self, Str):
-AlignValue = 1
-Granu = 1
-Str = Str.strip().upper()
-if Str.endswith('K'):
-Granu = 1024
-Str = Str[:-1]
-elif Str.endswith('M'):
-Granu = 1024 * 1024
-Str = Str[:-1]
-elif Str.endswith('G'):
-Granu = 1024 * 1024 * 1024
-Str = Str[:-1]
-else:
-pass
-
-AlignValue = int(Str) * Granu
-return AlignValue
-
 ## BlockSizeOfRegion()
 #
 #   @param  BlockSizeListList of block information
--
2.16.2.windows.1

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

Re: [edk2] PciBusDxe: PCI-Express bug with dynamic PcdPciExpressBaseAddress

[Ni, Ruiyu]

Mike,
Do you think that maybe just raising to TPL_NOTIFY is enough for PCI BAR 
probing?
Though the timer interrupt still triggers, all callbacks are suspended in 
TPL_NOTIFY.

Thanks,
Ray

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Nikita
> Leshenko
> Sent: Friday, September 7, 2018 3:09 AM
> To: edk2-devel@lists.01.org
> Cc: Liran Alon 
> Subject: [edk2] PciBusDxe: PCI-Express bug with dynamic
> PcdPciExpressBaseAddress
> 
> Hi,
> 
> We ran into a bug in EDK2 relating to PCI-Express in PciBusDxe. Here's the 
> flow
> of the bug:
> 
> 1. PciBusDxe/PciEnumeratorSupport.c: Function BarExisted probes a BAR. It
> raises
>TPL to TPL_HIGH_LEVEL to avoid timer interrupts while probing the BAR and
>calls PciIo->Pci.Write.
> 2. BasePciExpressLib/PciExpressLib.c: The write reaches PciExpressWrite32,
> which
>calls GetPciExpressBaseAddress.
> 3. GetPciExpressBaseAddress retrieves the address from
> PcdPciExpressBaseAddress.
> 4. Reading the PCD calls DxePcdGet64 -> GetWorker ->
>EfiAcquireLock(&mPcdDatabaseLock), which is at TPL_NOTIFY level. This
> crashes
>the firmware because step 1 raised the TPL to TPL_HIGH_LEVEL.
> 
> This doesn't happen when PcdPciExpressBaseAddress is fixed at build (because
> then the read is optimized to a static global variable), but when the PCD is
> dynamic PCI-Express is broken.
> 
> Does anybody have a suggestion for fixing it?
> 
> Options we thought about:
> - Change mPcdDatabaseLock.Tpl to TPL_HIGH_LEVEL
> - Don't use a PCD for the base address, put it in a static global variable and
>   create functions to set and retrieve it.
> 
> Thanks,
> Nikita
> ___
> 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 1/1] FatPkg/EnhancedFatDxe Fix Double Cluster Allocation

[Robinson, Herbie]

This is a fix for a double cluster allocation when the disk is full.  The 
double allocation happens because FatGrowEof calls FatAllocateCluster without 
immediately marking the each returned cluster as allocated.  The fix is to move 
the FatSetFatEntry call inside the loop.  I've also include some improvements 
to the sanity checks that I added while tracking this down.  They are optional.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by:Herbie Robinson 
---
diff --git a/FatPkg/EnhancedFatDxe/FileSpace.c 
b/FatPkg/EnhancedFatDxe/FileSpace.c
index 1254cd6..e17d3b6 100644
--- a/FatPkg/EnhancedFatDxe/FileSpace.c
+++ b/FatPkg/EnhancedFatDxe/FileSpace.c
@@ -467,7 +467,7 @@ FatGrowEof (
   ClusterCount  = 0;

   while (!FAT_END_OF_FAT_CHAIN (Cluster)) {
-if (Cluster == FAT_CLUSTER_FREE || Cluster >= FAT_CLUSTER_SPECIAL) {
+if (Cluster < FAT_MIN_CLUSTER || Cluster > Volume->MaxCluster + 1) {

   DEBUG (
 (EFI_D_INIT | EFI_D_ERROR,
@@ -509,6 +509,11 @@ FatGrowEof (
 goto Done;
   }

+  if (NewCluster < FAT_MIN_CLUSTER || NewCluster > Volume->MaxCluster + 1) 
{
+Status = EFI_VOLUME_CORRUPTED;
+goto Done;
+  }
+
   if (LastCluster != 0) {
 FatSetFatEntry (Volume, LastCluster, NewCluster);
   } else {
@@ -518,12 +523,21 @@ FatGrowEof (

   LastCluster = NewCluster;
   CurSize += 1;
+
+  //
+  // Terminate the cluster list
+  //
+  // Note that we must do this EVERY time we allocate a cluster, because
+  // FatAllocateCluster scans the FAT looking for a free cluster and
+  // "LastCluster" is no longer free!  Usually, FatAllocateCluster will
+  // start looking with the cluster after "LastCluster"; however, when
+  // there is only one free cluster left, it will find "LastCluster"
+  // a second time.  There are other, less predictable scenarios
+  // where this could happen, as well.
+  //
+  FatSetFatEntry (Volume, LastCluster, (UINTN) FAT_CLUSTER_LAST);
+  OFile->FileLastCluster = LastCluster;
 }
-//
-// Terminate the cluster list
-//
-FatSetFatEntry (Volume, LastCluster, (UINTN) FAT_CLUSTER_LAST);
-OFile->FileLastCluster = LastCluster;
   }

   OFile->FileSize = (UINTN) NewSizeInBytes;
@@ -603,7 +617,7 @@ FatOFilePosition (
   Cluster = FatGetFatEntry (Volume, Cluster);
 }

-if (Cluster < FAT_MIN_CLUSTER) {
+if (Cluster < FAT_MIN_CLUSTER || Cluster > Volume->MaxCluster + 1) {
   return EFI_VOLUME_CORRUPTED;
 }

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

Re: [edk2] portability of ShellPkg

[Andrew Fish]

Mike,

More possible ideas

I think from a 10,000 foot level the issue is the Public definition of the HOB 
lib calls out the ASSERT behavior 

5) Make the ASSERT() conditional via a PCD and update the HOB lib definition. 
For platforms that make HOBs options GetHobList() must be called 1st, and non 
of the other APIs can be called on platforms that don't support HOBs. That has 
the upside of only needing to make the Lib constructor and GetHobList() ASSERTs 
conditional onPCDs. 

6) Split the existing UefiBootManagerLib in a backwards compatible way by 
making a new UefiBootLib. UefiBootManagerLib.h can include UefiBootLib.h and 
UefiBootManagerLib.inf could depend on UefiBootLib. This would only require 
adding UefiBootLib to the library class of existing DSC files. 

Thanks,

Andrew Fish

> On Sep 6, 2018, at 10:17 AM, Kinney, Michael D  
> wrote:
> 
> Ray,
> 
> I have a few ideas here.
> 
> 1) Add a new UefiHobLib instance that is only for 
>   use by UEFI Drivers and UEFI Applications.  It fails
>   gracefully if the HOB List is not in the UEFI System
>   Configuration Table.  BootMode if HobList is not present
>   can be BOOT_WITH_FULL_CONFIGURATION.
> 
> 2) Remove ASSERT() from the CONSTRUCTOR and a return
>   NULL from the Get*() functions if mHobList is NULL.
>   BootMode can be BOOT_WITH_FULL_CONFIGURATION if
>   mHobList is NULL.
> 
> 3) Update UEFI Shell to not use the UefiBootManagerLib.
>   This would add duplicate code to the UEFI Shell.
> 
> 4) Update UefiBootManagerLib to not use the HobLib.
>   Instead look for HobList in UEFI System Configuration
>   Table and fail gracefully if it is not present.
>   This would add some duplicate code to the 
>   UefiBootManagerLib to parse the Hob List.
> 
> Best regards,
> 
> Mike
> 
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Thursday, September 6, 2018 2:57 AM
>> To: Ni, Ruiyu ; Andrew Fish
>> 
>> Cc: Leif Lindholm ; edk2-
>> de...@lists.01.org; Carsey, Jaben
>> ; Alexander Graf
>> ; Heinrich Schuchardt
>> ; AKASHI Takahiro
>> ; Kinney, Michael D
>> 
>> Subject: Re: portability of ShellPkg
>> 
>> On 09/06/18 04:34, Ni, Ruiyu wrote:
>>> On 9/6/2018 3:47 AM, Andrew Fish wrote:
 
 Laszlo,
 
 gEfiMemoryTypeInformationGuid is an edk2/MdeModulePkg
>> concept used to
 give the DXE Core hints on how to reduce
>> fragmentation in the memory
 map. Typically there is code in PEI that creates a
>> HOB and may consume
 a variable written by the BDS. This library seems to
>> be the generic
 way to do it on an edk2 platform.
 
 Thus this library is not just PI but edk2
>> MdeModulePkg specific in
 some of its assumptions. In general this
>> UefiBootManagerLib seems
 focused on construction and edk2 BDS. It would
>> probably make more
 sense to break out the UEFI Spec related bits so they
>> could be used in
 generic UEFI Applications.
 
 Thanks,
 
 Andrew Fish
 
 
>>> 
>>> Andrew,
>>> I agree refactoring UefiBootManagerLib to separate the
>> pure UEFI, pure
>>> PI and EDKII specific looks much more cleaner.
>>> So far the PI related bits in UefiBootManagerLib may
>> include:
>>> 1. Hob access for S4 support (memory type information
>> HOB).
>>> 2. Boot from Firmware Volume support.
>>> 
>>> But that requires introducing two or more library
>> classes so affecting
>>> all existing platforms. EfiCreateEventLegacyBootEx()
>> in UefiLib also
>>> touches PI event gEfiEventLegacyBootGuid.
>>> 
>>> And I think the value of refactor might be small.
>>> 
>>> I think root cause of this problem is not
>> UefiBootManagerLib includes
>>> PI, it's the assertion in DxeHobLib.
>>> So I am thinking maybe a very light fix is to remove
>> the constructor of
>>> DxeHobLib.
>>> 
>>> I talked with Liming about this and he suggested that
>> instead of
>>> removing the constructor, it's safer to just remove
>> the assertion in the
>>> constructor. Because removing the constructor of
>> HobLib may cause
>>> AutoGen process generates a different order of library
>> constructors
>>> calling sequence, which may break the platform.
>>> 
>>> So I propose to just remove the assertion in DxeHobLib
>> constructor.
>>> Thoughts?
>> 
>> I think keeping the constructor itself is important and
>> a good idea.
>> 
>> I also think that we should *perhaps* keep the assertion
>> *somewhere*,
>> just not in the constructor. Because, at least some of
>> the HobLib APIs
>> cannot return errors (and their callers expect them to
>> succeed at all
>> times). This suggests we should still trip assertions
>> when a HobLib API
>> is called *in practice* on a non-PI UEFI platform.
>> 
>> Thanks
>> Laszlo

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

[edk2] PciBusDxe: PCI-Express bug with dynamic PcdPciExpressBaseAddress

[Nikita Leshenko]

Hi,

We ran into a bug in EDK2 relating to PCI-Express in PciBusDxe. Here's the flow
of the bug:

1. PciBusDxe/PciEnumeratorSupport.c: Function BarExisted probes a BAR. It raises
   TPL to TPL_HIGH_LEVEL to avoid timer interrupts while probing the BAR and
   calls PciIo->Pci.Write.
2. BasePciExpressLib/PciExpressLib.c: The write reaches PciExpressWrite32, which
   calls GetPciExpressBaseAddress.
3. GetPciExpressBaseAddress retrieves the address from PcdPciExpressBaseAddress.
4. Reading the PCD calls DxePcdGet64 -> GetWorker ->
   EfiAcquireLock(&mPcdDatabaseLock), which is at TPL_NOTIFY level. This crashes
   the firmware because step 1 raised the TPL to TPL_HIGH_LEVEL.

This doesn't happen when PcdPciExpressBaseAddress is fixed at build (because
then the read is optimized to a static global variable), but when the PCD is
dynamic PCI-Express is broken.

Does anybody have a suggestion for fixing it?

Options we thought about:
- Change mPcdDatabaseLock.Tpl to TPL_HIGH_LEVEL
- Don't use a PCD for the base address, put it in a static global variable and
  create functions to set and retrieve it.

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

[edk2] [PATCH 2/3] EmbeddedPkg/CoherentDmaLib: Add missing checks to DmaMap

[Vladimir Olovyannikov]

UEFI Sct validates Dma mapping. For CoherentDmaLib it always failed
because there were no required checks present in DmaMap.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Vladimir Olovyannikov 
---
 EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c 
b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
index 8ca9e6aa5b1b..eb88fa288a99 100644
--- a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
+++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
@@ -58,6 +58,12 @@ DmaMap (
   OUTVOID   **Mapping
   )
 {
+  if (HostAddress == NULL ||
+  NumberOfBytes == NULL ||
+  DeviceAddress == NULL ||
+  Mapping == NULL ) {
+return EFI_INVALID_PARAMETER;
+  }
   *DeviceAddress = HostToDeviceAddress (HostAddress);
   *Mapping = NULL;
   return EFI_SUCCESS;
-- 
2.18.0

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

[edk2] [PATCH 3/3] MdeModulePkg/NonDiscoverablePciDeviceDxe: add missing validation

[Vladimir Olovyannikov]

UEFI SCT crashed and failed in NonDiscoverablePciDeviceDxe becase
required checks were not performed. Perform parameters validation in
NonDiscoverablePciDeviceDxe.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Vladimir Olovyannikov 
---
 .../NonDiscoverablePciDeviceIo.c  | 50 ++-
 1 file changed, 49 insertions(+), 1 deletion(-)

diff --git 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
index 0e42ae4bf6ec..07118d59fd68 100644
--- 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
+++ 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
@@ -52,6 +52,10 @@ GetBarResource (
 
   BarIndex -= (UINT8)Dev->BarOffset;
 
+  if (BarIndex >= Dev->BarCount) {
+return EFI_UNSUPPORTED;
+  }
+
   for (Desc = Dev->Device->Resources;
Desc->Desc != ACPI_END_TAG_DESCRIPTOR;
Desc = (VOID *)((UINT8 *)Desc + Desc->Len + 3)) {
@@ -597,6 +601,19 @@ CoherentPciIoMap (
   EFI_STATUSStatus;
   NON_DISCOVERABLE_PCI_DEVICE_MAP_INFO  *MapInfo;
 
+  if (Operation != EfiPciIoOperationBusMasterRead &&
+  Operation != EfiPciIoOperationBusMasterWrite &&
+  Operation != EfiPciIoOperationBusMasterCommonBuffer) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (HostAddress   == NULL ||
+  NumberOfBytes == NULL ||
+  DeviceAddress == NULL ||
+  Mapping   == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
   //
   // If HostAddress exceeds 4 GB, and this device does not support 64-bit DMA
   // addressing, we need to allocate a bounce buffer and copy over the data.
@@ -720,6 +737,11 @@ CoherentPciIoAllocateBuffer (
 return EFI_UNSUPPORTED;
   }
 
+  if ((MemoryType != EfiBootServicesData) &&
+  (MemoryType != EfiRuntimeServicesData)) {
+return EFI_INVALID_PARAMETER;
+  }
+
   //
   // Allocate below 4 GB if the dual address cycle attribute has not
   // been set. If the system has no memory available below 4 GB, there
@@ -877,6 +899,10 @@ NonCoherentPciIoAllocateBuffer (
   NON_DISCOVERABLE_DEVICE_UNCACHED_ALLOCATION *Alloc;
   VOID*AllocAddress;
 
+  if (HostAddress == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
   Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
 
   Status = CoherentPciIoAllocateBuffer (This, Type, MemoryType, Pages,
@@ -995,6 +1021,19 @@ NonCoherentPciIoMap (
   EFI_GCD_MEMORY_SPACE_DESCRIPTOR   GcdDescriptor;
   BOOLEAN   Bounce;
 
+  if (HostAddress   == NULL ||
+  NumberOfBytes == NULL ||
+  DeviceAddress == NULL ||
+  Mapping   == NULL) {
+return EFI_INVALID_PARAMETER;
+  }
+
+  if (Operation != EfiPciIoOperationBusMasterRead &&
+  Operation != EfiPciIoOperationBusMasterWrite &&
+  Operation != EfiPciIoOperationBusMasterCommonBuffer) {
+return EFI_INVALID_PARAMETER;
+  }
+
   MapInfo = AllocatePool (sizeof *MapInfo);
   if (MapInfo == NULL) {
 return EFI_OUT_OF_RESOURCES;
@@ -1228,8 +1267,17 @@ PciIoAttributes (
   NON_DISCOVERABLE_PCI_DEVICE   *Dev;
   BOOLEAN   Enable;
 
+  #define DEV_SUPPORTED_ATTRIBUTES \
+(EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE)
+
   Dev = NON_DISCOVERABLE_PCI_DEVICE_FROM_PCI_IO(This);
 
+  if (Attributes) {
+  if ((Attributes & (~(DEV_SUPPORTED_ATTRIBUTES))) != 0) {
+return EFI_UNSUPPORTED;
+  }
+}
+
   Enable = FALSE;
   switch (Operation) {
   case EfiPciIoAttributeOperationGet:
@@ -1243,7 +1291,7 @@ PciIoAttributes (
 if (Result == NULL) {
   return EFI_INVALID_PARAMETER;
 }
-*Result = EFI_PCI_DEVICE_ENABLE | EFI_PCI_IO_ATTRIBUTE_DUAL_ADDRESS_CYCLE;
+*Result = DEV_SUPPORTED_ATTRIBUTES;
 break;
 
   case EfiPciIoAttributeOperationEnable:
-- 
2.18.0

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

[edk2] [PATCH 1/3] EmbeddedPkg/CoherentDmaLib: Fix typo in DmaAlignedBuffer

[Vladimir Olovyannikov]

The only valid memory types for DmaAlignedBuffer should be
EfiBootServicesData and EfiRuntimeServicesData. However due to the typo,
there is no way to allocate runtime pages, and INVALID_PARAMETER is
always returned. Fix the typo.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Vladimir Olovyannikov 
---
 EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c 
b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
index 564db83c901c..8ca9e6aa5b1b 100644
--- a/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
+++ b/EmbeddedPkg/Library/CoherentDmaLib/CoherentDmaLib.c
@@ -154,7 +154,7 @@ DmaAllocateAlignedBuffer (
   //
   if (MemoryType == EfiBootServicesData) {
 *HostAddress = AllocateAlignedPages (Pages, Alignment);
-  } else if (MemoryType != EfiRuntimeServicesData) {
+  } else if (MemoryType == EfiRuntimeServicesData) {
 *HostAddress = AllocateAlignedRuntimePages (Pages, Alignment);
   } else {
 return EFI_INVALID_PARAMETER;
-- 
2.18.0

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

Re: [edk2] [PATCH v1 1/1] BaseTools/GenFds: Verify binaries all the time

[Carsey, Jaben]

I have found a related issue.  Please ignore this patch.

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Jaben Carsey
> Sent: Thursday, September 06, 2018 11:31 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming 
> Subject: [edk2] [PATCH v1 1/1] BaseTools/GenFds: Verify binaries all the time
> 
> Currently, Basetools only verifies Binary file list existance for default 
> ARCH,
> but it should verify for a specified ARCH the same.
> Also, dont save the list to a unused variable.
> 
> Cc: Yonghong Zhu 
> Cc: Liming Gao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jaben Carsey 
> ---
>  BaseTools/Source/Python/GenFds/AprioriSection.py | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/BaseTools/Source/Python/GenFds/AprioriSection.py
> b/BaseTools/Source/Python/GenFds/AprioriSection.py
> index 7196f7f2c753..70aa150799a7 100644
> --- a/BaseTools/Source/Python/GenFds/AprioriSection.py
> +++ b/BaseTools/Source/Python/GenFds/AprioriSection.py
> @@ -89,12 +89,10 @@ class AprioriSection (AprioriSectionClassObject):
>  Inf =
> GenFdsGlobalVariable.WorkSpace.BuildObject[PathClass(InfFileName,
> GenFdsGlobalVariable.WorkSpaceDir), TAB_COMMON,
> GenFdsGlobalVariable.TargetName, GenFdsGlobalVariable.ToolChainTag]
>  Guid = Inf.Guid
> 
> -self.BinFileList = Inf.Module.Binaries
> -if self.BinFileList == []:
> -EdkLogger.error("GenFds", RESOURCE_NOT_AVAILABLE,
> -"INF %s not found in build ARCH %s!" 
> \
> -% (InfFileName, 
> GenFdsGlobalVariable.ArchList))
> -
> +if not Inf.Module.Binaries:
> +EdkLoggerError("GenFds", RESOURCE_NOT_AVAILABLE,
> +"INF %s not found in build ARCH %s!" \
> +% (InfFileName, 
> GenFdsGlobalVariable.ArchList))
> 
>  GuidPart = Guid.split('-')
>  Buffer.write(pack('I', long(GuidPart[0], 16)))
> --
> 2.16.2.windows.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 v1 1/1] BaseTools/GenFds: Verify binaries all the time

[Jaben Carsey]

Currently, Basetools only verifies Binary file list existance for default ARCH,
but it should verify for a specified ARCH the same.
Also, dont save the list to a unused variable.

Cc: Yonghong Zhu 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/GenFds/AprioriSection.py | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/BaseTools/Source/Python/GenFds/AprioriSection.py 
b/BaseTools/Source/Python/GenFds/AprioriSection.py
index 7196f7f2c753..70aa150799a7 100644
--- a/BaseTools/Source/Python/GenFds/AprioriSection.py
+++ b/BaseTools/Source/Python/GenFds/AprioriSection.py
@@ -89,12 +89,10 @@ class AprioriSection (AprioriSectionClassObject):
 Inf = 
GenFdsGlobalVariable.WorkSpace.BuildObject[PathClass(InfFileName, 
GenFdsGlobalVariable.WorkSpaceDir), TAB_COMMON, 
GenFdsGlobalVariable.TargetName, GenFdsGlobalVariable.ToolChainTag]
 Guid = Inf.Guid
 
-self.BinFileList = Inf.Module.Binaries
-if self.BinFileList == []:
-EdkLogger.error("GenFds", RESOURCE_NOT_AVAILABLE,
-"INF %s not found in build ARCH %s!" \
-% (InfFileName, 
GenFdsGlobalVariable.ArchList))
-
+if not Inf.Module.Binaries:
+EdkLoggerError("GenFds", RESOURCE_NOT_AVAILABLE,
+"INF %s not found in build ARCH %s!" \
+% (InfFileName, 
GenFdsGlobalVariable.ArchList))
 
 GuidPart = Guid.split('-')
 Buffer.write(pack('I', long(GuidPart[0], 16)))
-- 
2.16.2.windows.1

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

Re: [edk2] [PATCH 3/4] SecurityPkg: remove PE/COFF header workaround for ELILO on IPF

[Laszlo Ersek]

On 09/06/18 19:25, Ard Biesheuvel wrote:
> On 6 September 2018 at 18:47, Laszlo Ersek  wrote:

>> For build-testing, I suggest
>>
>>   build -p SecurityPkg/SecurityPkg.dsc ...
>>
>> It will build all SecurityPkg modules, without putting them in a flash 
>> device (FD).
>>
> 
> That is what I used to perform the build test, which completed without
> error. [...]

That's spooky! In "SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c", no
declaration of "mNtHeader" should be visible. Hm...

Arrgh, if you check "SecurityPkg/SecurityPkg.dsc", the
"SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf" file is listed *only* under

[Components.IA32, Components.X64]

Thus, assuming you built natively on AARCH64, this module was not built. :/

Jiewen, is there any particular reason for not building Tcg2Dxe on
AARCH64? Is it just "historical lack of AARCH64 test environment
providing TPM2 hardware", or something more "architectural"?

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

Re: [edk2] [PATCH 4/4] EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF

[Laszlo Ersek]

On 09/06/18 20:01, Carsey, Jaben wrote:
> We have a BZ to remove the compatibility package... I’d call updating it 
> redundant and not worth your time.

Ah, very good point!

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

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

Re: [edk2] [PATCH 4/4] EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF

[Carsey, Jaben]

We have a BZ to remove the compatibility package... I’d call updating it 
redundant and not worth your time.

Jaben

> On Sep 6, 2018, at 10:27 AM, Ard Biesheuvel  wrote:
> 
>> On 6 September 2018 at 18:53, Laszlo Ersek  wrote:
>>> On 09/06/18 15:45, Ard Biesheuvel wrote:
>>> Now that Itanium support has been dropped, we can remove the various
>>> occurrences of the ELILO on Itanium PE/COFF header workaround.
>>> 
>>> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ard Biesheuvel 
>>> ---
>>> EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c
>>>  | 60 +++-
>>> 1 file changed, 8 insertions(+), 52 deletions(-)
>> 
>> Should we care about EdkCompatibilityPkg at all? Because:
>> 
>> * IPF removal seems not to have occurred to EdkCompatibilityPkg:
>> 
>>  $ git grep -w IPF -- 'EdkCompatibilityPkg/*inf'
>>  [bunch of hits]
>> 
>> * In , you wrote:
>> 
>>> [...] there is a big difference between IPF drivers that are never
>>> referenced by modern platforms, and workarounds in generic code that
>>> are present in every modern build for every platform, and are only
>>> intended for a specific build of ELILO.
>> 
>>> The former is essentially dead code. The latter gets executed many
>>> times on every boot of every modern UEFI platform in existence.
>> 
>> Under that distinction, I would classify EdkCompatibilityPkg as the
>> first category, i.e., essentially dead code.
>> 
> 
> OK, fair enough. I don't care about EdkCompatibilityPkg at all, I just
> wanted to be thorough, but if others don't care either, I'll drop this
> from v2.
> ___
> 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 4/4] EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF

[Ard Biesheuvel]

On 6 September 2018 at 18:53, Laszlo Ersek  wrote:
> On 09/06/18 15:45, Ard Biesheuvel wrote:
>> Now that Itanium support has been dropped, we can remove the various
>> occurrences of the ELILO on Itanium PE/COFF header workaround.
>>
>> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  
>> EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c
>>  | 60 +++-
>>  1 file changed, 8 insertions(+), 52 deletions(-)
>
> Should we care about EdkCompatibilityPkg at all? Because:
>
> * IPF removal seems not to have occurred to EdkCompatibilityPkg:
>
>   $ git grep -w IPF -- 'EdkCompatibilityPkg/*inf'
>   [bunch of hits]
>
> * In , you wrote:
>
>> [...] there is a big difference between IPF drivers that are never
>> referenced by modern platforms, and workarounds in generic code that
>> are present in every modern build for every platform, and are only
>> intended for a specific build of ELILO.
>
>> The former is essentially dead code. The latter gets executed many
>> times on every boot of every modern UEFI platform in existence.
>
> Under that distinction, I would classify EdkCompatibilityPkg as the
> first category, i.e., essentially dead code.
>

OK, fair enough. I don't care about EdkCompatibilityPkg at all, I just
wanted to be thorough, but if others don't care either, I'll drop this
from v2.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/4] SecurityPkg: remove PE/COFF header workaround for ELILO on IPF

[Ard Biesheuvel]

On 6 September 2018 at 18:47, Laszlo Ersek  wrote:
> On 09/06/18 15:45, Ard Biesheuvel wrote:
>> Now that Itanium support has been dropped, we can remove the various
>> occurrences of the ELILO on Itanium PE/COFF header workaround.
>>
>> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ard Biesheuvel 
>> ---
>>  SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c   
>>  | 47 
>>  SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c 
>>  | 27 +++
>>  SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c 
>>  | 27 +++
>>  
>> SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c 
>> | 25 +++
>>  4 files changed, 25 insertions(+), 101 deletions(-)
>>
>> diff --git 
>> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
>> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> index 0f795c0af125..66d96a9396b9 100644
>> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
>> @@ -295,7 +295,6 @@ HashPeImage (
>>)
>>  {
>>BOOLEAN   Status;
>> -  UINT16Magic;
>>EFI_IMAGE_SECTION_HEADER  *Section;
>>VOID  *HashCtx;
>>UINTN CtxSize;
>> @@ -367,33 +366,19 @@ HashPeImage (
>>// Measuring PE/COFF Image Header;
>>// But CheckSum field and SECURITY data directory (certificate) are 
>> excluded
>>//
>> -  if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
>> mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> -//
>> -// NOTE: Some versions of Linux ELILO for Itanium have an incorrect 
>> magic value
>> -//   in the PE/COFF Header. If the MachineType is Itanium(IA64) and 
>> the
>> -//   Magic value in the OptionalHeader is 
>> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
>> -//   then override the magic value to 
>> EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
>> -//
>> -Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
>> -  } else {
>> -//
>> -// Get the magic value from the PE/COFF Optional Header
>> -//
>> -Magic =  mNtHeader.Pe32->OptionalHeader.Magic;
>> -  }
>>
>>//
>>// 3.  Calculate the distance from the base of the image header to the 
>> image checksum address.
>>// 4.  Hash the image header from its base to beginning of the image 
>> checksum.
>>//
>>HashBase = mImageBase;
>> -  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> +  if (mNtHeader.Pe32->OptionalHeader.Magic == 
>> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>>  //
>>  // Use PE32 offset.
>>  //
>>  HashSize = (UINTN) (&mNtHeader.Pe32->OptionalHeader.CheckSum) - (UINTN) 
>> HashBase;
>>  NumberOfRvaAndSizes = 
>> mNtHeader.Pe32->OptionalHeader.NumberOfRvaAndSizes;
>> -  } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
>> +  } else if (mNtHeader.Pe32->OptionalHeader.Magic == 
>> EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
>>  //
>>  // Use PE32+ offset.
>>  //
>> @@ -420,7 +405,7 @@ HashPeImage (
>>  // 6.  Since there is no Cert Directory in optional header, hash 
>> everything
>>  // from the end of the checksum to the end of image header.
>>  //
>> -if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> +if (mNtHeader.Pe32->OptionalHeader.Magic == 
>> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>>//
>>// Use PE32 offset.
>>//
>> @@ -444,7 +429,7 @@ HashPeImage (
>>  //
>>  // 7.  Hash everything from the end of the checksum to the start of the 
>> Cert Directory.
>>  //
>> -if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> +if (mNtHeader.Pe32->OptionalHeader.Magic == 
>> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>>//
>>// Use PE32 offset.
>>//
>> @@ -469,7 +454,7 @@ HashPeImage (
>>  // 8.  Skip over the Cert Directory. (It is 
>> sizeof(IMAGE_DATA_DIRECTORY) bytes.)
>>  // 9.  Hash everything from the end of the Cert Directory to the end of 
>> image header.
>>  //
>> -if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> +if (mNtHeader.Pe32->OptionalHeader.Magic == 
>> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>>//
>>// Use PE32 offset
>>//
>> @@ -494,7 +479,7 @@ HashPeImage (
>>//
>>// 10. Set the SUM_OF_BYTES_HASHED to the size of the header.
>>//
>> -  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>> +  if (mNtHeader.Pe32->OptionalHeader.Magic == 
>> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>>  //
>>  // Use PE32 offset.
>>  //
>> @@ -577,7 +562,7 @@ HashPeImage (
>>  if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
>>CertSize = 0;
>>  } else {
>> -  if (Magic == EFI_IMAGE_NT_OPTIONA

Re: [edk2] portability of ShellPkg

[Kinney, Michael D]

Ray,

I have a few ideas here.

1) Add a new UefiHobLib instance that is only for 
   use by UEFI Drivers and UEFI Applications.  It fails
   gracefully if the HOB List is not in the UEFI System
   Configuration Table.  BootMode if HobList is not present
   can be BOOT_WITH_FULL_CONFIGURATION.

2) Remove ASSERT() from the CONSTRUCTOR and a return
   NULL from the Get*() functions if mHobList is NULL.
   BootMode can be BOOT_WITH_FULL_CONFIGURATION if
   mHobList is NULL.

3) Update UEFI Shell to not use the UefiBootManagerLib.
   This would add duplicate code to the UEFI Shell.

4) Update UefiBootManagerLib to not use the HobLib.
   Instead look for HobList in UEFI System Configuration
   Table and fail gracefully if it is not present.
   This would add some duplicate code to the 
   UefiBootManagerLib to parse the Hob List.

Best regards,

Mike

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, September 6, 2018 2:57 AM
> To: Ni, Ruiyu ; Andrew Fish
> 
> Cc: Leif Lindholm ; edk2-
> de...@lists.01.org; Carsey, Jaben
> ; Alexander Graf
> ; Heinrich Schuchardt
> ; AKASHI Takahiro
> ; Kinney, Michael D
> 
> Subject: Re: portability of ShellPkg
> 
> On 09/06/18 04:34, Ni, Ruiyu wrote:
> > On 9/6/2018 3:47 AM, Andrew Fish wrote:
> >>
> >> Laszlo,
> >>
> >> gEfiMemoryTypeInformationGuid is an edk2/MdeModulePkg
> concept used to
> >> give the DXE Core hints on how to reduce
> fragmentation in the memory
> >> map. Typically there is code in PEI that creates a
> HOB and may consume
> >> a variable written by the BDS. This library seems to
> be the generic
> >> way to do it on an edk2 platform.
> >>
> >> Thus this library is not just PI but edk2
> MdeModulePkg specific in
> >> some of its assumptions. In general this
> UefiBootManagerLib seems
> >> focused on construction and edk2 BDS. It would
> probably make more
> >> sense to break out the UEFI Spec related bits so they
> could be used in
> >> generic UEFI Applications.
> >>
> >> Thanks,
> >>
> >> Andrew Fish
> >>
> >>
> >
> > Andrew,
> > I agree refactoring UefiBootManagerLib to separate the
> pure UEFI, pure
> > PI and EDKII specific looks much more cleaner.
> > So far the PI related bits in UefiBootManagerLib may
> include:
> > 1. Hob access for S4 support (memory type information
> HOB).
> > 2. Boot from Firmware Volume support.
> >
> > But that requires introducing two or more library
> classes so affecting
> > all existing platforms. EfiCreateEventLegacyBootEx()
> in UefiLib also
> > touches PI event gEfiEventLegacyBootGuid.
> >
> > And I think the value of refactor might be small.
> >
> > I think root cause of this problem is not
> UefiBootManagerLib includes
> > PI, it's the assertion in DxeHobLib.
> > So I am thinking maybe a very light fix is to remove
> the constructor of
> > DxeHobLib.
> >
> > I talked with Liming about this and he suggested that
> instead of
> > removing the constructor, it's safer to just remove
> the assertion in the
> > constructor. Because removing the constructor of
> HobLib may cause
> > AutoGen process generates a different order of library
> constructors
> > calling sequence, which may break the platform.
> >
> > So I propose to just remove the assertion in DxeHobLib
> constructor.
> > Thoughts?
> 
> I think keeping the constructor itself is important and
> a good idea.
> 
> I also think that we should *perhaps* keep the assertion
> *somewhere*,
> just not in the constructor. Because, at least some of
> the HobLib APIs
> cannot return errors (and their callers expect them to
> succeed at all
> times). This suggests we should still trip assertions
> when a HobLib API
> is called *in practice* on a non-PI UEFI platform.
> 
> Thanks
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 4/4] EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF

[Laszlo Ersek]

On 09/06/18 15:45, Ard Biesheuvel wrote:
> Now that Itanium support has been dropped, we can remove the various
> occurrences of the ELILO on Itanium PE/COFF header workaround.
>
> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  
> EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c
>  | 60 +++-
>  1 file changed, 8 insertions(+), 52 deletions(-)

Should we care about EdkCompatibilityPkg at all? Because:

* IPF removal seems not to have occurred to EdkCompatibilityPkg:

  $ git grep -w IPF -- 'EdkCompatibilityPkg/*inf'
  [bunch of hits]

* In , you wrote:

> [...] there is a big difference between IPF drivers that are never
> referenced by modern platforms, and workarounds in generic code that
> are present in every modern build for every platform, and are only
> intended for a specific build of ELILO.

> The former is essentially dead code. The latter gets executed many
> times on every boot of every modern UEFI platform in existence.

Under that distinction, I would classify EdkCompatibilityPkg as the
first category, i.e., essentially dead code.

(I'm pointing this out in the hope that it'll save me the review of this
patch! :) )

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

Re: [edk2] [PATCH 3/4] SecurityPkg: remove PE/COFF header workaround for ELILO on IPF

[Laszlo Ersek]

On 09/06/18 15:45, Ard Biesheuvel wrote:
> Now that Itanium support has been dropped, we can remove the various
> occurrences of the ELILO on Itanium PE/COFF header workaround.
> 
> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> | 47 
>  SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c  
> | 27 +++
>  SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c  
> | 27 +++
>  SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c 
> | 25 +++
>  4 files changed, 25 insertions(+), 101 deletions(-)
> 
> diff --git 
> a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
> b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> index 0f795c0af125..66d96a9396b9 100644
> --- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> +++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
> @@ -295,7 +295,6 @@ HashPeImage (
>)
>  {
>BOOLEAN   Status;
> -  UINT16Magic;
>EFI_IMAGE_SECTION_HEADER  *Section;
>VOID  *HashCtx;
>UINTN CtxSize;
> @@ -367,33 +366,19 @@ HashPeImage (
>// Measuring PE/COFF Image Header;
>// But CheckSum field and SECURITY data directory (certificate) are 
> excluded
>//
> -  if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
> mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> -//
> -// NOTE: Some versions of Linux ELILO for Itanium have an incorrect 
> magic value
> -//   in the PE/COFF Header. If the MachineType is Itanium(IA64) and 
> the
> -//   Magic value in the OptionalHeader is 
> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> -//   then override the magic value to 
> EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> -//
> -Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> -  } else {
> -//
> -// Get the magic value from the PE/COFF Optional Header
> -//
> -Magic =  mNtHeader.Pe32->OptionalHeader.Magic;
> -  }
>  
>//
>// 3.  Calculate the distance from the base of the image header to the 
> image checksum address.
>// 4.  Hash the image header from its base to beginning of the image 
> checksum.
>//
>HashBase = mImageBase;
> -  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +  if (mNtHeader.Pe32->OptionalHeader.Magic == 
> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>  //
>  // Use PE32 offset.
>  //
>  HashSize = (UINTN) (&mNtHeader.Pe32->OptionalHeader.CheckSum) - (UINTN) 
> HashBase;
>  NumberOfRvaAndSizes = mNtHeader.Pe32->OptionalHeader.NumberOfRvaAndSizes;
> -  } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> +  } else if (mNtHeader.Pe32->OptionalHeader.Magic == 
> EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
>  //
>  // Use PE32+ offset.
>  //
> @@ -420,7 +405,7 @@ HashPeImage (
>  // 6.  Since there is no Cert Directory in optional header, hash 
> everything
>  // from the end of the checksum to the end of image header.
>  //
> -if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +if (mNtHeader.Pe32->OptionalHeader.Magic == 
> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>//
>// Use PE32 offset.
>//
> @@ -444,7 +429,7 @@ HashPeImage (
>  //
>  // 7.  Hash everything from the end of the checksum to the start of the 
> Cert Directory.
>  //
> -if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +if (mNtHeader.Pe32->OptionalHeader.Magic == 
> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>//
>// Use PE32 offset.
>//
> @@ -469,7 +454,7 @@ HashPeImage (
>  // 8.  Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) 
> bytes.)
>  // 9.  Hash everything from the end of the Cert Directory to the end of 
> image header.
>  //
> -if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +if (mNtHeader.Pe32->OptionalHeader.Magic == 
> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>//
>// Use PE32 offset
>//
> @@ -494,7 +479,7 @@ HashPeImage (
>//
>// 10. Set the SUM_OF_BYTES_HASHED to the size of the header.
>//
> -  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +  if (mNtHeader.Pe32->OptionalHeader.Magic == 
> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>  //
>  // Use PE32 offset.
>  //
> @@ -577,7 +562,7 @@ HashPeImage (
>  if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
>CertSize = 0;
>  } else {
> -  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +  if (mNtHeader.Pe32->OptionalHeader.Magic == 
> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>  //
>  // Use PE32 offset.
>  //
> @@ -1583,7 +1

Re: [edk2] [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on IPF

[Laszlo Ersek]

On 09/06/18 15:45, Ard Biesheuvel wrote:
> Now that Itanium support has been dropped, we can remove the various
> occurrences of the ELILO on Itanium PE/COFF header workaround.
> 
> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++-
>  1 file changed, 9 insertions(+), 52 deletions(-)
> 
> diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c 
> b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> index 32eca0ad2ef4..c57816a80887 100644
> --- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> +++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
> @@ -46,36 +46,6 @@ PeCoffLoaderAdjustOffsetForTeImage (
>SectionHeader->PointerToRawData -= TeStrippedOffset;
>  }
>  
> -/**
> -  Retrieves the magic value from the PE/COFF header.
> -
> -  @param  Hdr The buffer in which to return the PE32, PE32+, or 
> TE header.
> -
> -  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
> -  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
> -
> -**/
> -UINT16
> -PeCoffLoaderGetPeHeaderMagicValue (
> -  IN  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr
> -  )
> -{
> -  //
> -  // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
> value
> -  //   in the PE/COFF Header.  If the MachineType is Itanium(IA64) and 
> the
> -  //   Magic value in the OptionalHeader is  
> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> -  //   then override the returned value to 
> EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> -  //
> -  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
> Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> -return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> -  }
> -  //
> -  // Return the magic value from the PC/COFF Optional Header
> -  //
> -  return Hdr.Pe32->OptionalHeader.Magic;
> -}
> -
> -
>  /**
>Retrieves the PE or TE Header from a PE/COFF or TE image.
>  
> @@ -101,7 +71,6 @@ PeCoffLoaderGetPeHeader (
>EFI_IMAGE_DOS_HEADER  DosHdr;
>UINTN Size;
>UINTN ReadSize;
> -  UINT16Magic;
>UINT32SectionHeaderOffset;
>UINT32Index;
>UINT32HeaderWithoutDataDir;
> @@ -222,9 +191,7 @@ PeCoffLoaderGetPeHeader (
>  ImageContext->IsTeImage = FALSE;
>  ImageContext->Machine = Hdr.Pe32->FileHeader.Machine;
>  
> -Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
> -
> -if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) 
> {
>//
>// 1. Check OptionalHeader.NumberOfRvaAndSizes filed.
>//
> @@ -339,7 +306,7 @@ PeCoffLoaderGetPeHeader (
>ImageContext->SectionAlignment  = 
> Hdr.Pe32->OptionalHeader.SectionAlignment;
>ImageContext->SizeOfHeaders = 
> Hdr.Pe32->OptionalHeader.SizeOfHeaders;
>  
> -} else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> +} else if (Hdr.Pe32->OptionalHeader.Magic == 
> EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
>//
>// 1. Check FileHeader.NumberOfRvaAndSizes filed.
>//
> @@ -605,7 +572,6 @@ PeCoffLoaderGetImageInfo (
>EFI_IMAGE_SECTION_HEADER  SectionHeader;
>EFI_IMAGE_DEBUG_DIRECTORY_ENTRY   DebugEntry;
>UINT32NumberOfRvaAndSizes;
> -  UINT16Magic;
>UINT32TeStrippedOffset;
>  
>if (ImageContext == NULL) {
> @@ -622,14 +588,12 @@ PeCoffLoaderGetImageInfo (
>  return Status;
>}
>  
> -  Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
> -
>//
>// Retrieve the base address of the image
>//
>if (!(ImageContext->IsTeImage)) {
>  TeStrippedOffset = 0;
> -if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) 
> {
>//
>// Use PE32 offset
>//
> @@ -678,7 +642,7 @@ PeCoffLoaderGetImageInfo (
>}
>  
>if (!(ImageContext->IsTeImage)) {
> -if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) 
> {
>//
>// Use PE32 offset
>//
> @@ -952,7 +916,6 @@ PeCoffLoaderRelocateImage (
>CHAR8 *FixupData;
>PHYSICAL_ADDRESS  BaseAddress;
>UINT32NumberOfRvaAndSizes;
> -  UINT16Magic;
>UINT32TeStrippedOffset;
>  
>ASSERT (ImageContext != NULL);
> @@ -985,9 +948,8 @@ PeCoffLoaderRelocateImage (
>if (!(ImageContext->IsTeImage)) {
>  Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageContext->ImageAddress 
> + ImageContext->PeCoffHeaderOffset);
>  TeStrippedOffset = 0;
> -

Re: [edk2] [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF

[Laszlo Ersek]

On 09/06/18 15:45, Ard Biesheuvel wrote:
> Now that Itanium support has been dropped, we can remove the various
> occurrences of the ELILO on Itanium PE/COFF header workaround.
> 
> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ard Biesheuvel 
> ---
>  MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c | 31 +---
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c   | 17 +--
>  MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c| 17 +--
>  MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 17 +--
>  MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c| 31 +---
>  5 files changed, 5 insertions(+), 108 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c 
> b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
> index 83ed43a16e95..ff1940431c2f 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
> @@ -254,35 +254,6 @@ GetMemoryProfileContext (
>return mMemoryProfileContextPtr;
>  }
>  
> -/**
> -  Retrieves the magic value from the PE/COFF header.
> -
> -  @param HdrThe buffer in which to return the PE32, PE32+, or TE header.
> -
> -  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
> -  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
> -
> -**/
> -UINT16
> -InternalPeCoffGetPeHeaderMagicValue (
> -  IN  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr
> -  )
> -{
> -  //
> -  // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
> value
> -  //   in the PE/COFF Header.  If the MachineType is Itanium(IA64) and 
> the
> -  //   Magic value in the OptionalHeader is  
> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> -  //   then override the returned value to 
> EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> -  //
> -  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
> Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> -return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> -  }
> -  //
> -  // Return the magic value from the PC/COFF Optional Header
> -  //
> -  return Hdr.Pe32->OptionalHeader.Magic;
> -}
> -
>  /**
>Retrieves and returns the Subsystem of a PE/COFF image that has been 
> loaded into system memory.
>If Pe32Data is NULL, then ASSERT().
> @@ -319,7 +290,7 @@ InternalPeCoffGetSubsystem (
>if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
>  return Hdr.Te->Subsystem;
>} else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE)  {
> -Magic = InternalPeCoffGetPeHeaderMagicValue (Hdr);
> +Magic = Hdr.Pe32->OptionalHeader.Magic;
>  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>return Hdr.Pe32->OptionalHeader.Subsystem;
>  } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 
> b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> index 4cd219c88efc..fa8f8fe91ac7 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> @@ -406,7 +406,6 @@ ProtectUefiImage (
>IMAGE_PROPERTIES_RECORD  *ImageRecord;
>CHAR8*PdbPointer;
>IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
> -  UINT16   Magic;
>BOOLEAN  IsAligned;
>UINT32   ProtectionPolicy;
>  
> @@ -466,21 +465,7 @@ ProtectUefiImage (
>//
>// Get SectionAlignment
>//
> -  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
> Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> -//
> -// NOTE: Some versions of Linux ELILO for Itanium have an incorrect 
> magic value
> -//   in the PE/COFF Header. If the MachineType is Itanium(IA64) and 
> the
> -//   Magic value in the OptionalHeader is 
> EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
> -//   then override the magic value to 
> EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
> -//
> -Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
> -  } else {
> -//
> -// Get the magic value from the PE/COFF Optional Header
> -//
> -Magic = Hdr.Pe32->OptionalHeader.Magic;
> -  }
> -  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
> +  if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
>  SectionAlignment  = Hdr.Pe32->OptionalHeader.SectionAlignment;
>} else {
>  SectionAlignment  = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
> diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c 
> b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> index a96d442fbc64..05eb4f422b2f 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
> @@ -1076,7 +1076,6 @@ InsertImageRecord (
>IMAGE_PROPERTIES_RECORD  *ImageRecord;
>CHAR8   

Re: [edk2] [PATCH] ShellPkg Shell: Remove redundant functions

[Laszlo Ersek]

On 09/06/18 17:22, Gao, Liming wrote:
> The build log shows the error message. I agree to remove this unused global 
> variable InvalidChars in this patch. 
> 
> /home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/ShellPkg/Application/Shell/Shell.c:2754:21:
>  error: unused variable 'InvalidChars' [-Werror,-Wunused-const-variable]
> 12:23:26 STATIC CONST UINT16 InvalidChars[] = {L'*', L'?', L'<', L'>', L'\\', 
> L'/', L'\"', 0x0001, 0x0002};

OK... should we file a TianoCore BZ about this and wait until it gets
assigned and fixed, should we ask Shenglei just on the list to post a
follow-up (like Ard already did), or should one of us that actually
encounters the error post the one-liner patch?

(I would have already posted the patch if I could actually reproduce the
error.)

Thanks
Laszlo

>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
>> Biesheuvel
>> Sent: Thursday, September 6, 2018 8:31 PM
>> To: Laszlo Ersek 
>> Cc: Carsey, Jaben ; Ni, Ruiyu ; 
>> edk2-devel@lists.01.org
>> Subject: Re: [edk2] [PATCH] ShellPkg Shell: Remove redundant functions
>>
>> On 6 September 2018 at 14:24, Laszlo Ersek  wrote:
>>> On 09/06/18 12:28, Ard Biesheuvel wrote:
 On 9 August 2018 at 17:41,   wrote:
> The InvalidChars[] array is only used in function IsValidCommandName().
> The array should be deleted also, I think.
>

 Indeed, and for this reason this patch has now broken the build for
 clang. Please fix.
>>>
>>> I agree about InvalidChars being unused, post-22cf747fcf75. I just can't
>>> reproduce the build error, with CLANG38 (3.8.1). The command
>>>
>>>   build -a X64 -p ShellPkg/ShellPkg.dsc -b RELEASE -t CLANG38 \
>>> -m ShellPkg/Application/Shell/Shell.inf
>>>
>>> seems to work for me, also with "-b DEBUG".
>>>
>>> What differs on your end?
>>>
>>
>> Not sure what the exact difference is, but I am using CLANG35 profile:
>>
>> https://ci.linaro.org/job/leg-virt-tianocore-edk2-upstream/3176/consoleFull
>> ___
>> 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] ShellPkg Shell: Remove redundant functions

[Gao, Liming]

The build log shows the error message. I agree to remove this unused global 
variable InvalidChars in this patch. 

/home/buildslave/workspace/leg-virt-tianocore-edk2-upstream/edk2/ShellPkg/Application/Shell/Shell.c:2754:21:
 error: unused variable 'InvalidChars' [-Werror,-Wunused-const-variable]
12:23:26 STATIC CONST UINT16 InvalidChars[] = {L'*', L'?', L'<', L'>', L'\\', 
L'/', L'\"', 0x0001, 0x0002};

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Ard 
> Biesheuvel
> Sent: Thursday, September 6, 2018 8:31 PM
> To: Laszlo Ersek 
> Cc: Carsey, Jaben ; Ni, Ruiyu ; 
> edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] ShellPkg Shell: Remove redundant functions
> 
> On 6 September 2018 at 14:24, Laszlo Ersek  wrote:
> > On 09/06/18 12:28, Ard Biesheuvel wrote:
> >> On 9 August 2018 at 17:41,   wrote:
> >>> The InvalidChars[] array is only used in function IsValidCommandName().
> >>> The array should be deleted also, I think.
> >>>
> >>
> >> Indeed, and for this reason this patch has now broken the build for
> >> clang. Please fix.
> >
> > I agree about InvalidChars being unused, post-22cf747fcf75. I just can't
> > reproduce the build error, with CLANG38 (3.8.1). The command
> >
> >   build -a X64 -p ShellPkg/ShellPkg.dsc -b RELEASE -t CLANG38 \
> > -m ShellPkg/Application/Shell/Shell.inf
> >
> > seems to work for me, also with "-b DEBUG".
> >
> > What differs on your end?
> >
> 
> Not sure what the exact difference is, but I am using CLANG35 profile:
> 
> https://ci.linaro.org/job/leg-virt-tianocore-edk2-upstream/3176/consoleFull
> ___
> 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] [platforms: PATCH 1/7] Silicon/SynQuacer/PlatformDxe: Modify initialization of SdMmcOverride

[Ard Biesheuvel]

On 6 September 2018 at 16:38, Marcin Wojtas  wrote:
> czw., 6 wrz 2018 o 16:31 Ard Biesheuvel  
> napisał(a):
>>
>> On 6 September 2018 at 16:26, Marcin Wojtas  wrote:
>> > czw., 6 wrz 2018 o 16:04 Ard Biesheuvel  
>> > napisał(a):
>> >>
>> >> On 3 September 2018 at 06:53, Marcin Wojtas  wrote:
>> >> > From: Tomasz Michalec 
>> >> >
>> >> > This patch changes way the EDKII_SD_MMC_OVERRIDE protocol
>> >> > sturcture is allocated. Using AllocateZeroPool and then
>> >> > seting callbacks in the structure allow driver to be immune to
>> >> > adding new callbacks in SdMmcOveride protocol in future.
>> >> >
>> >>
>> >> What is the point of this patch?
>> >>
>> >> Statically allocating the structure will zero initialize the members
>> >> that are not initialized explicitly, but only the members that are
>> >> known to exist at compile time.
>> >>
>> >
>> > In such case this patch is really not needed.
>> >
>> >> I guess the idea of this patch is to work around the latter
>> >> limitation, but unfortunately, using sizeof(EDKII_SD_MMC_OVERRIDE)
>> >> puts you in the exact same situation.
>> >
>> > If the newly added callback are zero-initialized, the situation is
>> > fine as they won't be executed.
>> >
>>
>> Yes, but this patch does not change that situation at all.
>>
>> So please, explain which problem is fixed by this patch?
>
> None, we only forgot, the static initializer will zero non-declared
> fields by default.
>
>>
>> >>
>> >> This is the reason I added the version field. New hooks should only be
>> >> added after incrementing the version, and calling the new hooks should
>> >> only occur if the runtime version of the protocol implementation is
>> >> greater than or equal to the version where those hooks were first
>> >> introduced.
>> >>
>> >
>> > So even if the given SdMmcOverride protocol callback will be NULL for
>> > Synquacer controller, is there still a risk that anything could be
>> > broken without the version check?
>> >
>>
>> Yes. In EDK2, you can combine binary drivers with drivers build from
>> source. If a binary driver was built against an older version of the
>> SdMmcOverride header, it may have non-NULL values in the locations of
>> the new methods. This patch does not help against that scenario.
>
> Indeed, this is why it will disappear from v2. So, when adding the new
> callbacks, the version should be increased and checked in relevant
> places of the main EDK2 driver, right?
>
> Because a couple of the new callbacks are introduced, would it be ok,
> to increment the version only once, i.e. v2 of the SdMmcOverride will
> support 4 new routines?
>

Yes, that is preferred in my opinion.

Also, perhaps add some helper macros, e.g.,

#define EDKII_SD_MMC_OVERRIDE_HAVE_POST_CLOCK_FREQ_SWITCH(p) \
  ((p)->Version >= 0x2 && (p)->SwitchClockFreqPost != NULL)

so that the version handling is completely contained in the header file.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms: PATCH 1/7] Silicon/SynQuacer/PlatformDxe: Modify initialization of SdMmcOverride

[Marcin Wojtas]

czw., 6 wrz 2018 o 16:31 Ard Biesheuvel  napisał(a):
>
> On 6 September 2018 at 16:26, Marcin Wojtas  wrote:
> > czw., 6 wrz 2018 o 16:04 Ard Biesheuvel  
> > napisał(a):
> >>
> >> On 3 September 2018 at 06:53, Marcin Wojtas  wrote:
> >> > From: Tomasz Michalec 
> >> >
> >> > This patch changes way the EDKII_SD_MMC_OVERRIDE protocol
> >> > sturcture is allocated. Using AllocateZeroPool and then
> >> > seting callbacks in the structure allow driver to be immune to
> >> > adding new callbacks in SdMmcOveride protocol in future.
> >> >
> >>
> >> What is the point of this patch?
> >>
> >> Statically allocating the structure will zero initialize the members
> >> that are not initialized explicitly, but only the members that are
> >> known to exist at compile time.
> >>
> >
> > In such case this patch is really not needed.
> >
> >> I guess the idea of this patch is to work around the latter
> >> limitation, but unfortunately, using sizeof(EDKII_SD_MMC_OVERRIDE)
> >> puts you in the exact same situation.
> >
> > If the newly added callback are zero-initialized, the situation is
> > fine as they won't be executed.
> >
>
> Yes, but this patch does not change that situation at all.
>
> So please, explain which problem is fixed by this patch?

None, we only forgot, the static initializer will zero non-declared
fields by default.

>
> >>
> >> This is the reason I added the version field. New hooks should only be
> >> added after incrementing the version, and calling the new hooks should
> >> only occur if the runtime version of the protocol implementation is
> >> greater than or equal to the version where those hooks were first
> >> introduced.
> >>
> >
> > So even if the given SdMmcOverride protocol callback will be NULL for
> > Synquacer controller, is there still a risk that anything could be
> > broken without the version check?
> >
>
> Yes. In EDK2, you can combine binary drivers with drivers build from
> source. If a binary driver was built against an older version of the
> SdMmcOverride header, it may have non-NULL values in the locations of
> the new methods. This patch does not help against that scenario.

Indeed, this is why it will disappear from v2. So, when adding the new
callbacks, the version should be increased and checked in relevant
places of the main EDK2 driver, right?

Because a couple of the new callbacks are introduced, would it be ok,
to increment the version only once, i.e. v2 of the SdMmcOverride will
support 4 new routines?

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

Re: [edk2] [platforms: PATCH 1/7] Silicon/SynQuacer/PlatformDxe: Modify initialization of SdMmcOverride

[Ard Biesheuvel]

On 6 September 2018 at 16:26, Marcin Wojtas  wrote:
> czw., 6 wrz 2018 o 16:04 Ard Biesheuvel  
> napisał(a):
>>
>> On 3 September 2018 at 06:53, Marcin Wojtas  wrote:
>> > From: Tomasz Michalec 
>> >
>> > This patch changes way the EDKII_SD_MMC_OVERRIDE protocol
>> > sturcture is allocated. Using AllocateZeroPool and then
>> > seting callbacks in the structure allow driver to be immune to
>> > adding new callbacks in SdMmcOveride protocol in future.
>> >
>>
>> What is the point of this patch?
>>
>> Statically allocating the structure will zero initialize the members
>> that are not initialized explicitly, but only the members that are
>> known to exist at compile time.
>>
>
> In such case this patch is really not needed.
>
>> I guess the idea of this patch is to work around the latter
>> limitation, but unfortunately, using sizeof(EDKII_SD_MMC_OVERRIDE)
>> puts you in the exact same situation.
>
> If the newly added callback are zero-initialized, the situation is
> fine as they won't be executed.
>

Yes, but this patch does not change that situation at all.

So please, explain which problem is fixed by this patch?

>>
>> This is the reason I added the version field. New hooks should only be
>> added after incrementing the version, and calling the new hooks should
>> only occur if the runtime version of the protocol implementation is
>> greater than or equal to the version where those hooks were first
>> introduced.
>>
>
> So even if the given SdMmcOverride protocol callback will be NULL for
> Synquacer controller, is there still a risk that anything could be
> broken without the version check?
>

Yes. In EDK2, you can combine binary drivers with drivers build from
source. If a binary driver was built against an older version of the
SdMmcOverride header, it may have non-NULL values in the locations of
the new methods. This patch does not help against that scenario.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [platforms: PATCH 1/7] Silicon/SynQuacer/PlatformDxe: Modify initialization of SdMmcOverride

[Marcin Wojtas]

czw., 6 wrz 2018 o 16:04 Ard Biesheuvel  napisał(a):
>
> On 3 September 2018 at 06:53, Marcin Wojtas  wrote:
> > From: Tomasz Michalec 
> >
> > This patch changes way the EDKII_SD_MMC_OVERRIDE protocol
> > sturcture is allocated. Using AllocateZeroPool and then
> > seting callbacks in the structure allow driver to be immune to
> > adding new callbacks in SdMmcOveride protocol in future.
> >
>
> What is the point of this patch?
>
> Statically allocating the structure will zero initialize the members
> that are not initialized explicitly, but only the members that are
> known to exist at compile time.
>

In such case this patch is really not needed.

> I guess the idea of this patch is to work around the latter
> limitation, but unfortunately, using sizeof(EDKII_SD_MMC_OVERRIDE)
> puts you in the exact same situation.

If the newly added callback are zero-initialized, the situation is
fine as they won't be executed.

>
> This is the reason I added the version field. New hooks should only be
> added after incrementing the version, and calling the new hooks should
> only occur if the runtime version of the protocol implementation is
> greater than or equal to the version where those hooks were first
> introduced.
>

So even if the given SdMmcOverride protocol callback will be NULL for
Synquacer controller, is there still a risk that anything could be
broken without the version check?

Best regards,
Marcin

>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Marcin Wojtas 
> > ---
> >  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c | 20 
> > +---
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c 
> > b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> > index e0987c9..1ad8b88 100644
> > --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> > +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> > @@ -59,6 +59,8 @@
> >
> >  STATIC EFI_HANDLE mSdMmcControllerHandle;
> >
> > +STATIC EDKII_SD_MMC_OVERRIDE   *mSdMmcOverride;
> > +
> >  STATIC EFI_ACPI_DESCRIPTION_HEADER  *mSsdt;
> >  STATIC UINTNmSsdtSize;
> >  STATIC VOID *mEventRegistration;
> > @@ -180,12 +182,6 @@ SynQuacerSdMmcNotifyPhase (
> >return EFI_SUCCESS;
> >  }
> >
> > -STATIC EDKII_SD_MMC_OVERRIDE mSdMmcOverride = {
> > -  EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION,
> > -  SynQuacerSdMmcCapability,
> > -  SynQuacerSdMmcNotifyPhase,
> > -};
> > -
> >  STATIC
> >  VOID
> >  EFIAPI
> > @@ -255,10 +251,20 @@ RegisterEmmc (
> >   SYNQUACER_EMMC_BASE, SYNQUACER_EMMC_BASE_SZ);
> >ASSERT_EFI_ERROR (Status);
> >
> > +  mSdMmcOverride = AllocateZeroPool (sizeof (EDKII_SD_MMC_OVERRIDE));
> > +  if (mSdMmcOverride == NULL) {
> > +DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
> > +return EFI_OUT_OF_RESOURCES;
> > +  }
> > +
> > +  mSdMmcOverride->Version = EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION;
> > +  mSdMmcOverride->Capability = SynQuacerSdMmcCapability;
> > +  mSdMmcOverride->NotifyPhase = SynQuacerSdMmcNotifyPhase;
> > +
> >Handle = NULL;
> >Status = gBS->InstallProtocolInterface (&Handle,
> >&gEdkiiSdMmcOverrideProtocolGuid,
> > -  EFI_NATIVE_INTERFACE, (VOID **)&mSdMmcOverride);
> > +  EFI_NATIVE_INTERFACE, mSdMmcOverride);
> >ASSERT_EFI_ERROR (Status);
> >
> >return EFI_SUCCESS;
> > --
> > 2.7.4
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/7] SdMmc fixes and SdMmcOverride extension

[Marcin Wojtas]

Hi Ard,

czw., 6 wrz 2018 o 16:12 Ard Biesheuvel  napisał(a):
>
> On 3 September 2018 at 06:54, Marcin Wojtas  wrote:
> > Hi,
> >
> > This patchset extends SdMmcOverride protocol with new callbacks:
> > * UhsSignaling - allow writing custom values to HostControl2 register
> > * SwitchClockFreqPost - perform additional opperations after clock switch
> > * BaseClockFreq - allow overriding base clock frequency
> > Also a couple of fixes for MMC, card detection and reset are submitted.
> > More details can be found in the commit messages.
> >
> > Patches are available in the github:
> > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/sdmmc-override-upstream-r20180902
> >
> > Please note that extending SdMmcOverride protocol was impacting
> > so far the only user of it (Synquacer controller). In paralel
> > edk2-platforms patchset, a patch can be found:
> > ("Silicon/SynQuacer/PlatformDxe: Modify initialization of SdMmcOverride")
> > which immunizes for above and future extensions of the protocol:
> > https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/xenon-upstream-r20180902
> >
> > I'm looking forward to the comments and remarks.
> >
>
> Could we split this into a series of fixes/compliance tweaks, and a
> set of changes to the SD/MMC override protocol hooks that need to be
> added to accommodate Xenon?
>
> I suppose the former can be merged without much hassle, but the latter
> may provoke some fierce discussion, I'm afraid.
>

Ok. will split into two separate series. Anyway, I hope the override
protocol won't be that problematic - no functional change for
non-users, and the callbacks resemble the hooks that are provided by
drivers/mmc in Linux.

Marcin

>
>
> > Marcin Wojtas (3):
> >   MdeModulePkg/SdMmcPciHcDxe: Fix HS200 operation
> >   MdeModulePkg/SdMmcPciHcDxe: Adjust eMMC clock and bus width sequence
> >   MdeModulePkg/SdMmcPciHcDxe: Execute card detect only for RemovableSlot
> >
> > Tomasz Michalec (4):
> >   MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol
> >   MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride
> >   MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
> >   MdeModulePkg/SdMmcPciHcDxe: Fix SdMmcHcReset to set only necesery bits
> >
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   6 +
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  67 +-
> >  MdeModulePkg/Include/Protocol/SdMmcOverride.h  |  83 ++-
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c| 246 
> > +---
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c  |  55 -
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  37 ++-
> >  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 102 ++--
> >  7 files changed, 467 insertions(+), 129 deletions(-)
> >
> > --
> > 2.7.4
> >
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/7] SdMmc fixes and SdMmcOverride extension

[Ard Biesheuvel]

On 3 September 2018 at 06:54, Marcin Wojtas  wrote:
> Hi,
>
> This patchset extends SdMmcOverride protocol with new callbacks:
> * UhsSignaling - allow writing custom values to HostControl2 register
> * SwitchClockFreqPost - perform additional opperations after clock switch
> * BaseClockFreq - allow overriding base clock frequency
> Also a couple of fixes for MMC, card detection and reset are submitted.
> More details can be found in the commit messages.
>
> Patches are available in the github:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/sdmmc-override-upstream-r20180902
>
> Please note that extending SdMmcOverride protocol was impacting
> so far the only user of it (Synquacer controller). In paralel
> edk2-platforms patchset, a patch can be found:
> ("Silicon/SynQuacer/PlatformDxe: Modify initialization of SdMmcOverride")
> which immunizes for above and future extensions of the protocol:
> https://github.com/MarvellEmbeddedProcessors/edk2-open-platform/commits/xenon-upstream-r20180902
>
> I'm looking forward to the comments and remarks.
>

Could we split this into a series of fixes/compliance tweaks, and a
set of changes to the SD/MMC override protocol hooks that need to be
added to accommodate Xenon?

I suppose the former can be merged without much hassle, but the latter
may provoke some fierce discussion, I'm afraid.



> Marcin Wojtas (3):
>   MdeModulePkg/SdMmcPciHcDxe: Fix HS200 operation
>   MdeModulePkg/SdMmcPciHcDxe: Adjust eMMC clock and bus width sequence
>   MdeModulePkg/SdMmcPciHcDxe: Execute card detect only for RemovableSlot
>
> Tomasz Michalec (4):
>   MdeModulePkg/SdMmcPciHcDxe: Add UhsSignaling to SdMmcOverride protocol
>   MdeModulePkg/SdMmcPciHcDxe: Add SwitchClockFreqPost to SdMmcOverride
>   MdeModulePkg/SdMmcPciHcDxe: Allow overriding base clock frequency
>   MdeModulePkg/SdMmcPciHcDxe: Fix SdMmcHcReset to set only necesery bits
>
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.h |   6 +
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.h   |  67 +-
>  MdeModulePkg/Include/Protocol/SdMmcOverride.h  |  83 ++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/EmmcDevice.c| 246 +---
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdDevice.c  |  55 -
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHcDxe.c |  37 ++-
>  MdeModulePkg/Bus/Pci/SdMmcPciHcDxe/SdMmcPciHci.c   | 102 ++--
>  7 files changed, 467 insertions(+), 129 deletions(-)
>
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack

[Yao, Jiewen]

Thanks for the clean up.

Reviewed-by: Jiewen Yao 

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Thursday, September 6, 2018 9:45 PM
> To: edk2-devel@lists.01.org
> Cc: Ard Biesheuvel ; Zeng, Star
> ; Wang, Jian J ; Kinney, Michael
> D ; Gao, Liming ; Zhang,
> Chao B ; Yao, Jiewen ; Laszlo
> Ersek ; Leif Lindholm 
> Subject: [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header
> hack
> 
> Now that Itanium support has been dropped from EDK2, we can remove all 11
> (!)
> occurrences of the ELILO PE/COFF loader hack from the code base.
> 
> Note that SecurityPkg appears to have four mostly identical implementations
> of the PE/COFF measuring routine, so this may be another area for cleanup
> later.
> 
> Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
> 
> Cc: Star Zeng 
> Cc: Jian J Wang 
> Cc: Michael D Kinney 
> Cc: Liming Gao 
> Cc: Chao Zhang 
> Cc: Jiewen Yao 
> Cc: Laszlo Ersek 
> Cc: Leif Lindholm 
> 
> Ard Biesheuvel (4):
>   MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF
>   MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on
> IPF
>   SecurityPkg: remove PE/COFF header workaround for ELILO on IPF
>   EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF
> 
>  .../Library/BasePeCoffLib/BasePeCoff.c| 60 +++---
>  .../Core/Dxe/Mem/MemoryProfileRecord.c| 31 +-
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 17 +-
>  MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c  | 17 +-
>  .../Core/PiSmmCore/MemoryAttributesTable.c| 17 +-
>  .../Core/PiSmmCore/SmramProfileRecord.c   | 31 +-
>  MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++
>  .../DxeImageVerificationLib.c | 47 +++---
>  .../DxeTpmMeasureBootLib.c| 27 ++--
>  SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c   | 27 ++--
>  .../SecureBootConfigImpl.c| 25 ++--
>  11 files changed, 47 insertions(+), 313 deletions(-)
> 
> --
> 2.18.0

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

Re: [edk2] [platforms: PATCH 1/7] Silicon/SynQuacer/PlatformDxe: Modify initialization of SdMmcOverride

[Ard Biesheuvel]

On 3 September 2018 at 06:53, Marcin Wojtas  wrote:
> From: Tomasz Michalec 
>
> This patch changes way the EDKII_SD_MMC_OVERRIDE protocol
> sturcture is allocated. Using AllocateZeroPool and then
> seting callbacks in the structure allow driver to be immune to
> adding new callbacks in SdMmcOveride protocol in future.
>

What is the point of this patch?

Statically allocating the structure will zero initialize the members
that are not initialized explicitly, but only the members that are
known to exist at compile time.

I guess the idea of this patch is to work around the latter
limitation, but unfortunately, using sizeof(EDKII_SD_MMC_OVERRIDE)
puts you in the exact same situation.

This is the reason I added the version field. New hooks should only be
added after incrementing the version, and calling the new hooks should
only occur if the runtime version of the protocol implementation is
greater than or equal to the version where those hooks were first
introduced.


> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marcin Wojtas 
> ---
>  Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c | 20 
> +---
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c 
> b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> index e0987c9..1ad8b88 100644
> --- a/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> +++ b/Silicon/Socionext/SynQuacer/Drivers/PlatformDxe/Emmc.c
> @@ -59,6 +59,8 @@
>
>  STATIC EFI_HANDLE mSdMmcControllerHandle;
>
> +STATIC EDKII_SD_MMC_OVERRIDE   *mSdMmcOverride;
> +
>  STATIC EFI_ACPI_DESCRIPTION_HEADER  *mSsdt;
>  STATIC UINTNmSsdtSize;
>  STATIC VOID *mEventRegistration;
> @@ -180,12 +182,6 @@ SynQuacerSdMmcNotifyPhase (
>return EFI_SUCCESS;
>  }
>
> -STATIC EDKII_SD_MMC_OVERRIDE mSdMmcOverride = {
> -  EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION,
> -  SynQuacerSdMmcCapability,
> -  SynQuacerSdMmcNotifyPhase,
> -};
> -
>  STATIC
>  VOID
>  EFIAPI
> @@ -255,10 +251,20 @@ RegisterEmmc (
>   SYNQUACER_EMMC_BASE, SYNQUACER_EMMC_BASE_SZ);
>ASSERT_EFI_ERROR (Status);
>
> +  mSdMmcOverride = AllocateZeroPool (sizeof (EDKII_SD_MMC_OVERRIDE));
> +  if (mSdMmcOverride == NULL) {
> +DEBUG ((DEBUG_ERROR, "%a: Cannot allocate memory\n", __FUNCTION__));
> +return EFI_OUT_OF_RESOURCES;
> +  }
> +
> +  mSdMmcOverride->Version = EDKII_SD_MMC_OVERRIDE_PROTOCOL_VERSION;
> +  mSdMmcOverride->Capability = SynQuacerSdMmcCapability;
> +  mSdMmcOverride->NotifyPhase = SynQuacerSdMmcNotifyPhase;
> +
>Handle = NULL;
>Status = gBS->InstallProtocolInterface (&Handle,
>&gEdkiiSdMmcOverrideProtocolGuid,
> -  EFI_NATIVE_INTERFACE, (VOID **)&mSdMmcOverride);
> +  EFI_NATIVE_INTERFACE, mSdMmcOverride);
>ASSERT_EFI_ERROR (Status);
>
>return EFI_SUCCESS;
> --
> 2.7.4
>
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Fix ECC issues.

[Dong, Eric]

Hi Laszlo,

Thanks for your advice. I truly received the r-b from Dandan, but I just found 
she just reply to me.  But I truly forgot to include you in the r-b list. I 
will follow your suggestion next time.

Thanks,
Eric
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Thursday, September 6, 2018 8:04 PM
To: Dong, Eric 
Cc: edk2-devel@lists.01.org; Bi, Dandan 
Subject: Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Fix ECC issues.

Hi Eric,

On 09/05/18 09:57, Laszlo Ersek wrote:
> On 09/05/18 08:22, Eric Dong wrote:
>> Fix trailing white spaces and invalid line ending issue.
>>
>> Cc: Dandan Bi 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Eric Dong 
>> ---
>>  UefiCpuPkg/Library/MpInitLib/MpLib.c   | 2 +-
>>  UefiCpuPkg/Library/MpInitLib/MpLib.h   | 2 +-
>>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf  | 2 +-
>>  UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c | 
>> 2 +-
>>  4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
>> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> index 377876643f..5c562d4759 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> @@ -1014,7 +1014,7 @@ WakeUpAP (
>>  CpuData = &CpuMpData->CpuData[Index];
>>  //
>>  // All AP(include disabled AP) will be woke up by INIT-SIPI-SIPI, 
>> but
>> -// the AP procedure will be skipped for disabled AP because AP 
>> state 
>> +// the AP procedure will be skipped for disabled AP because 
>> + AP state
>>  // is not CpuStateReady.
>>  //
>>  if (GetApState (CpuData) == CpuStateDisabled && 
>> !WakeUpDisabledAps) { diff --git 
>> a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
>> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
>> index 75f3fdda1d..773db76b61 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
>> @@ -257,7 +257,7 @@ struct _CPU_MP_DATA {
>>// Whether need to use Init-Sipi-Sipi to wake up the APs.
>>// Two cases need to set this value to TRUE. One is in HLT
>>// loop mode, the other is resume from S3 which loop mode
>> -  // will be hardcode change to HLT mode by PiSmmCpuDxeSmm
>> +  // will be hardcode change to HLT mode by PiSmmCpuDxeSmm
>>// driver.
>>//
>>BOOLEANWakeUpByInitSipiSipi;
>> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf 
>> b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>> index 43a3b3b036..81036f0b12 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>> @@ -67,4 +67,4 @@
>>gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate   ## 
>> SOMETIMES_CONSUMES
>>  
>>  [Guids]
>> -  gEdkiiS3SmmInitDoneGuid
>> \ No newline at end of file
>> +  gEdkiiS3SmmInitDoneGuid
>> diff --git 
>> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c 
>> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> index 4143ee4bb1..fa7e107e39 100644
>> --- 
>> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLi
>> +++ b.c
>> @@ -516,7 +516,7 @@ AllocateAcpiCpuData (
>>  
>>//
>>// Allocate buffer for empty RegisterTable and 
>> PreSmmInitRegisterTable for all CPUs
>> -  //
>> +  //
>>TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
>>RegisterTable  = AllocatePages (EFI_SIZE_TO_PAGES (TableSize));
>>ASSERT (RegisterTable != NULL);
>>
> 
> Reviewed-by: Laszlo Ersek 

I think you may have made a mistake when committing this patch (commit 
e23d9c3ed86c); you added Dandan's R-b, but on the list, the one R-b you got was 
from me, not Dandan.

Not a big issue, but as a best practice for the future, I recommend copying the 
feedback tags verbatim from the reply emails, using the clipboard.

Thanks
Laszlo
--- Begin Message ---
Reviewed-by: Dandan Bi 

Thanks,
Dandan

-Original Message-
From: Dong, Eric
Sent: Wednesday, September 5, 2018 2:22 PM
To: edk2-devel@lists.01.org
Cc: Bi, Dandan 
Subject: [Patch] UefiCpuPkg/MpInitLib: Fix ECC issues.

Fix trailing white spaces and invalid line ending issue.

Cc: Dandan Bi 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Library/MpInitLib/MpLib.c   | 2 +-
 UefiCpuPkg/Library/MpInitLib/MpLib.h   | 2 +-
 UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf  | 2 +-
 UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
b/UefiCpuPkg/Library/MpInitLib/MpLib.c
index 377876643f..5c562d4759 100644
--- a/UefiCpuPkg/Library/MpInitLib/MpLib.c

[edk2] [PATCH 4/4] EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF

[Ard Biesheuvel]

Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 
EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c
 | 60 +++-
 1 file changed, 8 insertions(+), 52 deletions(-)

diff --git 
a/EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c
 
b/EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c
index f99c23e5ee4c..28d39b342afd 100644
--- 
a/EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c
+++ 
b/EdkCompatibilityPkg/Foundation/Library/EdkIIGlueLib/Library/BasePeCoffLib/BasePeCoff.c
@@ -22,36 +22,6 @@ Abstract:
 
 #include "BasePeCoffLibInternals.h"
 
-/**
-  Retrieves the magic value from the PE/COFF header.
-
-  @param  Hdr The buffer in which to return the PE32, PE32+, or TE 
header.
-
-  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
-  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-PeCoffLoaderGetPeHeaderMagicValue (
-  IN  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr
-  )
-{
-  //
-  // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
value 
-  //   in the PE/COFF Header.  If the MachineType is Itanium(IA64) and the 
-  //   Magic value in the OptionalHeader is  
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
-  //   then override the returned value to 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
-  //
-  if (Hdr.Pe32->FileHeader.Machine == EFI_IMAGE_MACHINE_IA64 && 
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
-  }
-  //
-  // Return the magic value from the PC/COFF Optional Header
-  //
-  return Hdr.Pe32->OptionalHeader.Magic;
-}
-
-
 /**
   Retrieves the PE or TE Header from a PE/COFF or TE image.
 
@@ -71,7 +41,6 @@ GluePeCoffLoaderGetPeHeader (
   RETURN_STATUS Status;
   EFI_IMAGE_DOS_HEADER  DosHdr;
   UINTN Size;
-  UINT16Magic;
 
   //
   // Read the DOS image header to check for it's existance
@@ -130,9 +99,7 @@ GluePeCoffLoaderGetPeHeader (
 ImageContext->IsTeImage = FALSE;
 ImageContext->Machine = Hdr.Pe32->FileHeader.Machine;
 
-Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -141,7 +108,7 @@ GluePeCoffLoaderGetPeHeader (
   ImageContext->SectionAlignment  = 
Hdr.Pe32->OptionalHeader.SectionAlignment;
   ImageContext->SizeOfHeaders = Hdr.Pe32->OptionalHeader.SizeOfHeaders;
 
-} else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+} else if (Hdr.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
   //
   // Use PE32+ offset
   //
@@ -209,7 +176,6 @@ GluePeCoffLoaderGetImageInfo (
   EFI_IMAGE_SECTION_HEADER  SectionHeader;
   EFI_IMAGE_DEBUG_DIRECTORY_ENTRY   DebugEntry;
   UINT32NumberOfRvaAndSizes;
-  UINT16Magic;
 
   if (NULL == ImageContext) {
 return RETURN_INVALID_PARAMETER;
@@ -225,13 +191,11 @@ GluePeCoffLoaderGetImageInfo (
 return Status;
   }
 
-  Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
   //
   // Retrieve the base address of the image
   //
   if (!(ImageContext->IsTeImage)) {
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -276,7 +240,7 @@ GluePeCoffLoaderGetImageInfo (
   }
 
   if (!(ImageContext->IsTeImage)) {
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -520,7 +484,6 @@ GluePeCoffLoaderRelocateImage (
   CHAR8 *FixupData;
   PHYSICAL_ADDRESS  BaseAddress;
   UINT32NumberOfRvaAndSizes;
-  UINT16Magic;
 
   ASSERT (ImageContext != NULL);
 
@@ -552,9 +515,7 @@ GluePeCoffLoaderRelocateImage (
   if (!(ImageContext->IsTeImage)) {
 Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageContext->ImageAddress + 
ImageContext->PeCoffHeaderOffset);
 
-Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -777,7 +738,6 @@ GluePeCoffLoaderLoadImage (
   UINTN Size;
   UINT32   

[edk2] [PATCH 0/4] remove all 11 occurrences of ELILO on IPF PE/COFF header hack

[Ard Biesheuvel]

Now that Itanium support has been dropped from EDK2, we can remove all 11 (!)
occurrences of the ELILO PE/COFF loader hack from the code base.

Note that SecurityPkg appears to have four mostly identical implementations
of the PE/COFF measuring routine, so this may be another area for cleanup
later.

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816

Cc: Star Zeng 
Cc: Jian J Wang 
Cc: Michael D Kinney 
Cc: Liming Gao 
Cc: Chao Zhang 
Cc: Jiewen Yao 
Cc: Laszlo Ersek 
Cc: Leif Lindholm 

Ard Biesheuvel (4):
  MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF
  MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on
IPF
  SecurityPkg: remove PE/COFF header workaround for ELILO on IPF
  EdkCompatibilityPkg: remove PE/COFF header workaround for ELILO on IPF

 .../Library/BasePeCoffLib/BasePeCoff.c| 60 +++---
 .../Core/Dxe/Mem/MemoryProfileRecord.c| 31 +-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 17 +-
 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c  | 17 +-
 .../Core/PiSmmCore/MemoryAttributesTable.c| 17 +-
 .../Core/PiSmmCore/SmramProfileRecord.c   | 31 +-
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++
 .../DxeImageVerificationLib.c | 47 +++---
 .../DxeTpmMeasureBootLib.c| 27 ++--
 SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c   | 27 ++--
 .../SecureBootConfigImpl.c| 25 ++--
 11 files changed, 47 insertions(+), 313 deletions(-)

-- 
2.18.0

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

[edk2] [PATCH 1/4] MdeModulePkg: remove PE/COFF header workaround for ELILO on IPF

[Ard Biesheuvel]

Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c | 31 +---
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c   | 17 +--
 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c| 17 +--
 MdeModulePkg/Core/PiSmmCore/MemoryAttributesTable.c | 17 +--
 MdeModulePkg/Core/PiSmmCore/SmramProfileRecord.c| 31 +---
 5 files changed, 5 insertions(+), 108 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c 
b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
index 83ed43a16e95..ff1940431c2f 100644
--- a/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
+++ b/MdeModulePkg/Core/Dxe/Mem/MemoryProfileRecord.c
@@ -254,35 +254,6 @@ GetMemoryProfileContext (
   return mMemoryProfileContextPtr;
 }
 
-/**
-  Retrieves the magic value from the PE/COFF header.
-
-  @param HdrThe buffer in which to return the PE32, PE32+, or TE header.
-
-  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
-  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-InternalPeCoffGetPeHeaderMagicValue (
-  IN  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr
-  )
-{
-  //
-  // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
value
-  //   in the PE/COFF Header.  If the MachineType is Itanium(IA64) and the
-  //   Magic value in the OptionalHeader is  
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
-  //   then override the returned value to 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
-  //
-  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
-  }
-  //
-  // Return the magic value from the PC/COFF Optional Header
-  //
-  return Hdr.Pe32->OptionalHeader.Magic;
-}
-
 /**
   Retrieves and returns the Subsystem of a PE/COFF image that has been loaded 
into system memory.
   If Pe32Data is NULL, then ASSERT().
@@ -319,7 +290,7 @@ InternalPeCoffGetSubsystem (
   if (Hdr.Te->Signature == EFI_TE_IMAGE_HEADER_SIGNATURE) {
 return Hdr.Te->Subsystem;
   } else if (Hdr.Pe32->Signature == EFI_IMAGE_NT_SIGNATURE)  {
-Magic = InternalPeCoffGetPeHeaderMagicValue (Hdr);
+Magic = Hdr.Pe32->OptionalHeader.Magic;
 if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   return Hdr.Pe32->OptionalHeader.Subsystem;
 } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 4cd219c88efc..fa8f8fe91ac7 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -406,7 +406,6 @@ ProtectUefiImage (
   IMAGE_PROPERTIES_RECORD  *ImageRecord;
   CHAR8*PdbPointer;
   IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
-  UINT16   Magic;
   BOOLEAN  IsAligned;
   UINT32   ProtectionPolicy;
 
@@ -466,21 +465,7 @@ ProtectUefiImage (
   //
   // Get SectionAlignment
   //
-  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-//
-// NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
value
-//   in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
-//   Magic value in the OptionalHeader is 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
-//   then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
-//
-Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
-  } else {
-//
-// Get the magic value from the PE/COFF Optional Header
-//
-Magic = Hdr.Pe32->OptionalHeader.Magic;
-  }
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+  if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
 SectionAlignment  = Hdr.Pe32->OptionalHeader.SectionAlignment;
   } else {
 SectionAlignment  = Hdr.Pe32Plus->OptionalHeader.SectionAlignment;
diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c 
b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index a96d442fbc64..05eb4f422b2f 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
@@ -1076,7 +1076,6 @@ InsertImageRecord (
   IMAGE_PROPERTIES_RECORD  *ImageRecord;
   CHAR8*PdbPointer;
   IMAGE_PROPERTIES_RECORD_CODE_SECTION *ImageRecordCodeSection;
-  UINT16   Magic;
 
   DEBUG ((EFI_D_VERBOSE, "InsertImageRecord - 0x%x\n", RuntimeImage));
   DEBUG ((EFI_D_VERBOSE, "InsertImageRecord - 0x%016lx - 0x%0

[edk2] [PATCH 3/4] SecurityPkg: remove PE/COFF header workaround for ELILO on IPF

[Ard Biesheuvel]

Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c| 
47 
 SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.c  | 
27 +++
 SecurityPkg/Tcg/Tcg2Dxe/MeasureBootPeCoff.c  | 
27 +++
 SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigImpl.c | 
25 +++
 4 files changed, 25 insertions(+), 101 deletions(-)

diff --git 
a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c 
b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
index 0f795c0af125..66d96a9396b9 100644
--- a/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
+++ b/SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.c
@@ -295,7 +295,6 @@ HashPeImage (
   )
 {
   BOOLEAN   Status;
-  UINT16Magic;
   EFI_IMAGE_SECTION_HEADER  *Section;
   VOID  *HashCtx;
   UINTN CtxSize;
@@ -367,33 +366,19 @@ HashPeImage (
   // Measuring PE/COFF Image Header;
   // But CheckSum field and SECURITY data directory (certificate) are excluded
   //
-  if (mNtHeader.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
mNtHeader.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-//
-// NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
value
-//   in the PE/COFF Header. If the MachineType is Itanium(IA64) and the
-//   Magic value in the OptionalHeader is 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
-//   then override the magic value to EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
-//
-Magic = EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
-  } else {
-//
-// Get the magic value from the PE/COFF Optional Header
-//
-Magic =  mNtHeader.Pe32->OptionalHeader.Magic;
-  }
 
   //
   // 3.  Calculate the distance from the base of the image header to the image 
checksum address.
   // 4.  Hash the image header from its base to beginning of the image 
checksum.
   //
   HashBase = mImageBase;
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+  if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
 //
 // Use PE32 offset.
 //
 HashSize = (UINTN) (&mNtHeader.Pe32->OptionalHeader.CheckSum) - (UINTN) 
HashBase;
 NumberOfRvaAndSizes = mNtHeader.Pe32->OptionalHeader.NumberOfRvaAndSizes;
-  } else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+  } else if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
 //
 // Use PE32+ offset.
 //
@@ -420,7 +405,7 @@ HashPeImage (
 // 6.  Since there is no Cert Directory in optional header, hash everything
 // from the end of the checksum to the end of image header.
 //
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset.
   //
@@ -444,7 +429,7 @@ HashPeImage (
 //
 // 7.  Hash everything from the end of the checksum to the start of the 
Cert Directory.
 //
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset.
   //
@@ -469,7 +454,7 @@ HashPeImage (
 // 8.  Skip over the Cert Directory. (It is sizeof(IMAGE_DATA_DIRECTORY) 
bytes.)
 // 9.  Hash everything from the end of the Cert Directory to the end of 
image header.
 //
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -494,7 +479,7 @@ HashPeImage (
   //
   // 10. Set the SUM_OF_BYTES_HASHED to the size of the header.
   //
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+  if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
 //
 // Use PE32 offset.
 //
@@ -577,7 +562,7 @@ HashPeImage (
 if (NumberOfRvaAndSizes <= EFI_IMAGE_DIRECTORY_ENTRY_SECURITY) {
   CertSize = 0;
 } else {
-  if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+  if (mNtHeader.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
 //
 // Use PE32 offset.
 //
@@ -1583,7 +1568,6 @@ DxeImageVerificationHandler (
   )
 {
   EFI_STATUS   Status;
-  UINT16   Magic;
   EFI_IMAGE_DOS_HEADER *DosHdr;
   EFI_STATUS   VerifyStatus;
   EFI_SIGNATURE_LIST   *SignatureList;
@@ -1723,2

[edk2] [PATCH 2/4] MdePkg/BasePeCoffLib: remove PE/COFF header workaround for ELILO on IPF

[Ard Biesheuvel]

Now that Itanium support has been dropped, we can remove the various
occurrences of the ELILO on Itanium PE/COFF header workaround.

Link: https://bugzilla.tianocore.org/show_bug.cgi?id=816
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel 
---
 MdePkg/Library/BasePeCoffLib/BasePeCoff.c | 61 +++-
 1 file changed, 9 insertions(+), 52 deletions(-)

diff --git a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c 
b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
index 32eca0ad2ef4..c57816a80887 100644
--- a/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
+++ b/MdePkg/Library/BasePeCoffLib/BasePeCoff.c
@@ -46,36 +46,6 @@ PeCoffLoaderAdjustOffsetForTeImage (
   SectionHeader->PointerToRawData -= TeStrippedOffset;
 }
 
-/**
-  Retrieves the magic value from the PE/COFF header.
-
-  @param  Hdr The buffer in which to return the PE32, PE32+, or TE 
header.
-
-  @return EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC - Image is PE32
-  @return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC - Image is PE32+
-
-**/
-UINT16
-PeCoffLoaderGetPeHeaderMagicValue (
-  IN  EFI_IMAGE_OPTIONAL_HEADER_PTR_UNION  Hdr
-  )
-{
-  //
-  // NOTE: Some versions of Linux ELILO for Itanium have an incorrect magic 
value
-  //   in the PE/COFF Header.  If the MachineType is Itanium(IA64) and the
-  //   Magic value in the OptionalHeader is  
EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC
-  //   then override the returned value to 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC
-  //
-  if (Hdr.Pe32->FileHeader.Machine == IMAGE_FILE_MACHINE_IA64 && 
Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
-return EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC;
-  }
-  //
-  // Return the magic value from the PC/COFF Optional Header
-  //
-  return Hdr.Pe32->OptionalHeader.Magic;
-}
-
-
 /**
   Retrieves the PE or TE Header from a PE/COFF or TE image.
 
@@ -101,7 +71,6 @@ PeCoffLoaderGetPeHeader (
   EFI_IMAGE_DOS_HEADER  DosHdr;
   UINTN Size;
   UINTN ReadSize;
-  UINT16Magic;
   UINT32SectionHeaderOffset;
   UINT32Index;
   UINT32HeaderWithoutDataDir;
@@ -222,9 +191,7 @@ PeCoffLoaderGetPeHeader (
 ImageContext->IsTeImage = FALSE;
 ImageContext->Machine = Hdr.Pe32->FileHeader.Machine;
 
-Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // 1. Check OptionalHeader.NumberOfRvaAndSizes filed.
   //
@@ -339,7 +306,7 @@ PeCoffLoaderGetPeHeader (
   ImageContext->SectionAlignment  = 
Hdr.Pe32->OptionalHeader.SectionAlignment;
   ImageContext->SizeOfHeaders = Hdr.Pe32->OptionalHeader.SizeOfHeaders;
 
-} else if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
+} else if (Hdr.Pe32->OptionalHeader.Magic == 
EFI_IMAGE_NT_OPTIONAL_HDR64_MAGIC) {
   //
   // 1. Check FileHeader.NumberOfRvaAndSizes filed.
   //
@@ -605,7 +572,6 @@ PeCoffLoaderGetImageInfo (
   EFI_IMAGE_SECTION_HEADER  SectionHeader;
   EFI_IMAGE_DEBUG_DIRECTORY_ENTRY   DebugEntry;
   UINT32NumberOfRvaAndSizes;
-  UINT16Magic;
   UINT32TeStrippedOffset;
 
   if (ImageContext == NULL) {
@@ -622,14 +588,12 @@ PeCoffLoaderGetImageInfo (
 return Status;
   }
 
-  Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
-
   //
   // Retrieve the base address of the image
   //
   if (!(ImageContext->IsTeImage)) {
 TeStrippedOffset = 0;
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -678,7 +642,7 @@ PeCoffLoaderGetImageInfo (
   }
 
   if (!(ImageContext->IsTeImage)) {
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -952,7 +916,6 @@ PeCoffLoaderRelocateImage (
   CHAR8 *FixupData;
   PHYSICAL_ADDRESS  BaseAddress;
   UINT32NumberOfRvaAndSizes;
-  UINT16Magic;
   UINT32TeStrippedOffset;
 
   ASSERT (ImageContext != NULL);
@@ -985,9 +948,8 @@ PeCoffLoaderRelocateImage (
   if (!(ImageContext->IsTeImage)) {
 Hdr.Pe32 = (EFI_IMAGE_NT_HEADERS32 *)((UINTN)ImageContext->ImageAddress + 
ImageContext->PeCoffHeaderOffset);
 TeStrippedOffset = 0;
-Magic = PeCoffLoaderGetPeHeaderMagicValue (Hdr);
 
-if (Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
+if (Hdr.Pe32->OptionalHeader.Magic == EFI_IMAGE_NT_OPTIONAL_HDR32_MAGIC) {
   //
   // Use PE32 offset
   //
@@ -1230,7 +1192,6 @@ PeCoffLoaderLoadImage (
   UINTN Size;

[edk2] [PATCH] Maintainer.txt: Add Ray to be co-maintainer of EmulatorPkg

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni 
Cc: Jordan Justen 
Cc: Andrew Fish 
---
 Maintainers.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Maintainers.txt b/Maintainers.txt
index 61df6e198b..7ebd53f662 100644
--- a/Maintainers.txt
+++ b/Maintainers.txt
@@ -124,6 +124,7 @@ EmulatorPkg
 W: https://github.com/tianocore/tianocore.github.io/wiki/EmulatorPkg
 M: Jordan Justen 
 M: Andrew Fish 
+M: Ruiyu Ni 
 S: Maintained
 
 FatPkg, FatBinPkg
-- 
2.16.1.windows.1

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

Re: [edk2] [PATCH] ShellPkg Shell: Remove redundant functions

[Ard Biesheuvel]

On 6 September 2018 at 14:24, Laszlo Ersek  wrote:
> On 09/06/18 12:28, Ard Biesheuvel wrote:
>> On 9 August 2018 at 17:41,   wrote:
>>> The InvalidChars[] array is only used in function IsValidCommandName().
>>> The array should be deleted also, I think.
>>>
>>
>> Indeed, and for this reason this patch has now broken the build for
>> clang. Please fix.
>
> I agree about InvalidChars being unused, post-22cf747fcf75. I just can't
> reproduce the build error, with CLANG38 (3.8.1). The command
>
>   build -a X64 -p ShellPkg/ShellPkg.dsc -b RELEASE -t CLANG38 \
> -m ShellPkg/Application/Shell/Shell.inf
>
> seems to work for me, also with "-b DEBUG".
>
> What differs on your end?
>

Not sure what the exact difference is, but I am using CLANG35 profile:

https://ci.linaro.org/job/leg-virt-tianocore-edk2-upstream/3176/consoleFull
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ShellPkg Shell: Remove redundant functions

[Laszlo Ersek]

On 09/06/18 12:28, Ard Biesheuvel wrote:
> On 9 August 2018 at 17:41,   wrote:
>> The InvalidChars[] array is only used in function IsValidCommandName().
>> The array should be deleted also, I think.
>>
> 
> Indeed, and for this reason this patch has now broken the build for
> clang. Please fix.

I agree about InvalidChars being unused, post-22cf747fcf75. I just can't
reproduce the build error, with CLANG38 (3.8.1). The command

  build -a X64 -p ShellPkg/ShellPkg.dsc -b RELEASE -t CLANG38 \
-m ShellPkg/Application/Shell/Shell.inf

seems to work for me, also with "-b DEBUG".

What differs on your end?

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

Re: [edk2] [PATCH] MdeModulePkg/EhciDxe: factor out EhcIsDebugPortInUse()

[Laszlo Ersek]

On 09/06/18 03:35, Zeng, Star wrote:
> Thanks for the quick following up. :)
> 
> Reviewed-by: Star Zeng 

Thanks! Commit b48ec0e8ab1c.

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

Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Fix ECC issues.

[Laszlo Ersek]

Hi Eric,

On 09/05/18 09:57, Laszlo Ersek wrote:
> On 09/05/18 08:22, Eric Dong wrote:
>> Fix trailing white spaces and invalid line ending issue.
>>
>> Cc: Dandan Bi 
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Eric Dong 
>> ---
>>  UefiCpuPkg/Library/MpInitLib/MpLib.c   | 2 +-
>>  UefiCpuPkg/Library/MpInitLib/MpLib.h   | 2 +-
>>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf  | 2 +-
>>  UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c | 2 +-
>>  4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
>> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> index 377876643f..5c562d4759 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
>> @@ -1014,7 +1014,7 @@ WakeUpAP (
>>  CpuData = &CpuMpData->CpuData[Index];
>>  //
>>  // All AP(include disabled AP) will be woke up by INIT-SIPI-SIPI, 
>> but
>> -// the AP procedure will be skipped for disabled AP because AP 
>> state 
>> +// the AP procedure will be skipped for disabled AP because AP state
>>  // is not CpuStateReady.
>>  //
>>  if (GetApState (CpuData) == CpuStateDisabled && !WakeUpDisabledAps) 
>> {
>> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
>> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
>> index 75f3fdda1d..773db76b61 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
>> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
>> @@ -257,7 +257,7 @@ struct _CPU_MP_DATA {
>>// Whether need to use Init-Sipi-Sipi to wake up the APs.
>>// Two cases need to set this value to TRUE. One is in HLT
>>// loop mode, the other is resume from S3 which loop mode
>> -  // will be hardcode change to HLT mode by PiSmmCpuDxeSmm 
>> +  // will be hardcode change to HLT mode by PiSmmCpuDxeSmm
>>// driver.
>>//
>>BOOLEANWakeUpByInitSipiSipi;
>> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf 
>> b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>> index 43a3b3b036..81036f0b12 100644
>> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
>> @@ -67,4 +67,4 @@
>>gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate   ## 
>> SOMETIMES_CONSUMES
>>  
>>  [Guids]
>> -  gEdkiiS3SmmInitDoneGuid
>> \ No newline at end of file
>> +  gEdkiiS3SmmInitDoneGuid
>> diff --git 
>> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c 
>> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> index 4143ee4bb1..fa7e107e39 100644
>> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeaturesLib.c
>> @@ -516,7 +516,7 @@ AllocateAcpiCpuData (
>>  
>>//
>>// Allocate buffer for empty RegisterTable and PreSmmInitRegisterTable 
>> for all CPUs
>> -  //  
>> +  //
>>TableSize = 2 * NumberOfCpus * sizeof (CPU_REGISTER_TABLE);
>>RegisterTable  = AllocatePages (EFI_SIZE_TO_PAGES (TableSize));
>>ASSERT (RegisterTable != NULL);
>>
> 
> Reviewed-by: Laszlo Ersek 

I think you may have made a mistake when committing this patch (commit
e23d9c3ed86c); you added Dandan's R-b, but on the list, the one R-b you
got was from me, not Dandan.

Not a big issue, but as a best practice for the future, I recommend
copying the feedback tags verbatim from the reply emails, using the
clipboard.

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

Re: [edk2] [PATCH] ShellPkg Shell: Remove redundant functions

[Ard Biesheuvel]

On 9 August 2018 at 17:41,   wrote:
> The InvalidChars[] array is only used in function IsValidCommandName().
> The array should be deleted also, I think.
>

Indeed, and for this reason this patch has now broken the build for
clang. Please fix.


>
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> shenglei
> Sent: Thursday, August 9, 2018 12:42 AM
> To: edk2-devel@lists.01.org
> Cc: Jaben Carsey; Ruiyu Ni
> Subject: [edk2] [PATCH] ShellPkg Shell: Remove redundant functions
>
> The redundant functions which are never called have been
> removed. They are InternalShellProtocolDebugPrintMessage,
> UpdateFileName,RemoveFileTag and IsValidCommandName.
> https://bugzilla.tianocore.org/show_bug.cgi?id=1066
>
> Cc: Jaben Carsey 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: shenglei 
> ---
>  ShellPkg/Application/Shell/Shell.c| 29 ---
>  ShellPkg/Application/Shell/Shell.h| 13 ---
>  .../Shell/ShellParametersProtocol.c   | 24 --
>  ShellPkg/Application/Shell/ShellProtocol.c| 81 +--
>  4 files changed, 1 insertion(+), 146 deletions(-)
>
> diff --git a/ShellPkg/Application/Shell/Shell.c 
> b/ShellPkg/Application/Shell/Shell.c
> index 47ae3c373c..397cfd1994 100644
> --- a/ShellPkg/Application/Shell/Shell.c
> +++ b/ShellPkg/Application/Shell/Shell.c
> @@ -2752,35 +2752,6 @@ RunCommand(
>
>
>  STATIC CONST UINT16 InvalidChars[] = {L'*', L'?', L'<', L'>', L'\\', L'/', 
> L'\"', 0x0001, 0x0002};
> -/**
> -  Function determines if the CommandName COULD be a valid command.  It does 
> not determine whether
> -  this is a valid command.  It only checks for invalid characters.
> -
> -  @param[in] CommandNameThe name to check
> -
> -  @retval TRUE  CommandName could be a command name
> -  @retval FALSE CommandName could not be a valid command name
> -**/
> -BOOLEAN
> -IsValidCommandName(
> -  IN CONST CHAR16 *CommandName
> -  )
> -{
> -  UINTN Count;
> -  if (CommandName == NULL) {
> -ASSERT(FALSE);
> -return (FALSE);
> -  }
> -  for ( Count = 0
> -  ; Count < sizeof(InvalidChars) / sizeof(InvalidChars[0])
> -  ; Count++
> - ){
> -if (ScanMem16(CommandName, StrSize(CommandName), InvalidChars[Count]) != 
> NULL) {
> -  return (FALSE);
> -}
> -  }
> -  return (TRUE);
> -}
>
>  /**
>Function to process a NSH script file via SHELL_FILE_HANDLE.
> diff --git a/ShellPkg/Application/Shell/Shell.h 
> b/ShellPkg/Application/Shell/Shell.h
> index 69b19c6a2d..bad8f08d47 100644
> --- a/ShellPkg/Application/Shell/Shell.h
> +++ b/ShellPkg/Application/Shell/Shell.h
> @@ -309,19 +309,6 @@ RunShellCommand(
>OUT EFI_STATUS*CommandStatus
>);
>
> -/**
> -  Function determines if the CommandName COULD be a valid command.  It does 
> not determine whether
> -  this is a valid command.  It only checks for invalid characters.
> -
> -  @param[in] CommandNameThe name to check
> -
> -  @retval TRUE  CommandName could be a command name
> -  @retval FALSE CommandName could not be a valid command name
> -**/
> -BOOLEAN
> -IsValidCommandName(
> -  IN CONST CHAR16 *CommandName
> -  );
>
>  /**
>Function to process a NSH script file via SHELL_FILE_HANDLE.
> diff --git a/ShellPkg/Application/Shell/ShellParametersProtocol.c 
> b/ShellPkg/Application/Shell/ShellParametersProtocol.c
> index 90889a3725..a21c690518 100644
> --- a/ShellPkg/Application/Shell/ShellParametersProtocol.c
> +++ b/ShellPkg/Application/Shell/ShellParametersProtocol.c
> @@ -626,30 +626,6 @@ FixVarName (
>return (FixFileName(Copy));
>  }
>
> -/**
> -  Remove the unicode file tag from the begining of the file buffer since 
> that will not be
> -  used by StdIn.
> -
> -  @param[in]  HandlePointer to the handle of the file to be processed.
> -
> -  @retval EFI_SUCCESS   The unicode file tag has been moved successfully.
> -**/
> -EFI_STATUS
> -RemoveFileTag(
> -  IN SHELL_FILE_HANDLE *Handle
> -  )
> -{
> -  UINTN CharSize;
> -  CHAR16CharBuffer;
> -
> -  CharSize= sizeof(CHAR16);
> -  CharBuffer  = 0;
> -  gEfiShellProtocol->ReadFile(*Handle, &CharSize, &CharBuffer);
> -  if (CharBuffer != gUnicodeFileTag) {
> -gEfiShellProtocol->SetFilePosition(*Handle, 0);
> -  }
> -  return (EFI_SUCCESS);
> -}
>
>  /**
>Write the unicode file tag to the specified file.
> diff --git a/ShellPkg/Application/Shell/ShellProtocol.c 
> b/ShellPkg/Application/Shell/ShellProtocol.c
> index f2ca2029e3..8cf924b384 100644
> --- a/ShellPkg/Application/Shell/ShellProtocol.c
> +++ b/ShellPkg/Application/Shell/ShellProtocol.c
> @@ -98,40 +98,6 @@ InternalShellProtocolIsSimpleFileSystemPresent(
>return (FALSE);
>  }
>
> -/**
> -  Internal worker debug helper function to print out maps as they are added.
> -
> -  @param[in] Mappingstring mapping that has been added
> -  @param[in] DevicePath pointer

Re: [edk2] Performance enabling of Event handler

[Laszlo Ersek]

On 09/06/18 08:10, prabin ca wrote:
> Hi Team,
> 
> I’m used edk2 PerformancePkg for profiling cpu execution time taken by a 
> event handler. Event is created successfully and event handler is also called 
> successfully, but I can capture the performance of this event handler with 
> PerformancePkg (by using perf_start and perf_end check points). This 
> PerformancePkg is working fine with normal function calls.

Do you mean "can not", instead of "can"? (Sorry, I don't understand.)

> 
> Please help me to enable PerformancePkg action on event handler also.
> 

Hmmm, even with the suggested typo correction, I wouldn't know what to
suggest. Sorry!

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

Re: [edk2] portability of ShellPkg

[Laszlo Ersek]

On 09/06/18 04:34, Ni, Ruiyu wrote:
> On 9/6/2018 3:47 AM, Andrew Fish wrote:
>>
>> Laszlo,
>>
>> gEfiMemoryTypeInformationGuid is an edk2/MdeModulePkg concept used to
>> give the DXE Core hints on how to reduce fragmentation in the memory
>> map. Typically there is code in PEI that creates a HOB and may consume
>> a variable written by the BDS. This library seems to be the generic
>> way to do it on an edk2 platform.
>>
>> Thus this library is not just PI but edk2 MdeModulePkg specific in
>> some of its assumptions. In general this UefiBootManagerLib seems
>> focused on construction and edk2 BDS. It would probably make more
>> sense to break out the UEFI Spec related bits so they could be used in
>> generic UEFI Applications.
>>
>> Thanks,
>>
>> Andrew Fish
>>
>>
> 
> Andrew,
> I agree refactoring UefiBootManagerLib to separate the pure UEFI, pure
> PI and EDKII specific looks much more cleaner.
> So far the PI related bits in UefiBootManagerLib may include:
> 1. Hob access for S4 support (memory type information HOB).
> 2. Boot from Firmware Volume support.
> 
> But that requires introducing two or more library classes so affecting
> all existing platforms. EfiCreateEventLegacyBootEx() in UefiLib also
> touches PI event gEfiEventLegacyBootGuid.
> 
> And I think the value of refactor might be small.
> 
> I think root cause of this problem is not UefiBootManagerLib includes
> PI, it's the assertion in DxeHobLib.
> So I am thinking maybe a very light fix is to remove the constructor of
> DxeHobLib.
> 
> I talked with Liming about this and he suggested that instead of
> removing the constructor, it's safer to just remove the assertion in the
> constructor. Because removing the constructor of HobLib may cause
> AutoGen process generates a different order of library constructors
> calling sequence, which may break the platform.
> 
> So I propose to just remove the assertion in DxeHobLib constructor.
> Thoughts?

I think keeping the constructor itself is important and a good idea.

I also think that we should *perhaps* keep the assertion *somewhere*,
just not in the constructor. Because, at least some of the HobLib APIs
cannot return errors (and their callers expect them to succeed at all
times). This suggests we should still trip assertions when a HobLib API
is called *in practice* on a non-PI UEFI platform.

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