Re: [edk2] [PATCH] MdeModulePkg/Core: fix too many available pages between BS_Data

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Wang, Jian J
> Sent: Wednesday, March 7, 2018 12:41 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Dong, Eric ; Yao,
> Jiewen 
> Subject: [PATCH] MdeModulePkg/Core: fix too many available pages between
> BS_Data
> 
> The root cause is an unnecessary check to Size parameter in function
> AdjustMemoryS(). It will cause one standalone free page (happen to have
> Guard page around) in the free memory list cannot be allocated, even if
> the requested memory size is less than a page.
> 
>   //
>   // At least one more page needed for Guard page.
>   //
>   if (Size < (SizeRequested + EFI_PAGES_TO_SIZE (1))) {
> return 0;
>   }
> 
> The following code in the same function actually covers above check
> implicitly. So the fix is simply removing above check.
> 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang 
> ---
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c   | 9 +
>  MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 9 +
>  2 files changed, 2 insertions(+), 16 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> index d7906e08c5..19245049c2 100644
> --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> @@ -1,7 +1,7 @@
>  /** @file
>UEFI Heap Guard functions.
> 
> -Copyright (c) 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2017-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
>  which accompanies this distribution.  The full text of the license may be 
> found
> at
> @@ -905,13 +905,6 @@ AdjustMemoryS (
> 
>Target = Start + Size - SizeRequested;
> 
> -  //
> -  // At least one more page needed for Guard page.
> -  //
> -  if (Size < (SizeRequested + EFI_PAGES_TO_SIZE (1))) {
> -return 0;
> -  }
> -
>if (!IsGuardPage (Start + Size)) {
>  // No Guard at tail to share. One more page is needed.
>  Target -= EFI_PAGES_TO_SIZE (1);
> diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> index c5ffb26342..aa9c25d102 100644
> --- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> +++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> @@ -1,7 +1,7 @@
>  /** @file
>UEFI Heap Guard functions.
> 
> -Copyright (c) 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2017-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
>  which accompanies this distribution.  The full text of the license may be 
> found
> at
> @@ -888,13 +888,6 @@ AdjustMemoryS (
> 
>Target = Start + Size - SizeRequested;
> 
> -  //
> -  // At least one more page needed for Guard page.
> -  //
> -  if (Size < (SizeRequested + EFI_PAGES_TO_SIZE (1))) {
> -return 0;
> -  }
> -
>if (!IsGuardPage (Start + Size)) {
>  // No Guard at tail to share. One more page is needed.
>  Target -= EFI_PAGES_TO_SIZE (1);
> --
> 2.14.1.windows.1

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

Re: [edk2] [patch] SecurityPkg/SmmTcg2PhysicalPresenceLib: Fix coding style issue

[Zhang, Chao B]

Reviewed-by: Chao Zhang 

-Original Message-
From: Bi, Dandan 
Sent: Wednesday, March 7, 2018 1:54 PM
To: edk2-devel@lists.01.org
Cc: Zhang, Chao B 
Subject: [patch] SecurityPkg/SmmTcg2PhysicalPresenceLib: Fix coding style issue

Boolean values do not need to use explicit comparisons to TRUE or FALSE.

Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c 
b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
index dfef6c8..6a4dce9 100644
--- 
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
+++ b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPres
+++ enceLib.c
@@ -339,11 +339,11 @@ Tcg2PhysicalPresenceLibGetUserConfirmationStatusFunction (
 case 
TCG2_PHYSICAL_PRESENCE_SET_PP_REQUIRED_FOR_ENABLE_BLOCK_SID_FUNC_FALSE:
 case 
TCG2_PHYSICAL_PRESENCE_SET_PP_REQUIRED_FOR_DISABLE_BLOCK_SID_FUNC_FALSE:
   break;
 
 default:
-  if (mIsTcg2PPVerLowerThan_1_3 == FALSE) {
+  if (!mIsTcg2PPVerLowerThan_1_3) {
 if (OperationRequest < 
TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
   //
   // TCG2 PP1.3 spec defined operations that are reserved or 
un-implemented
   //
   return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
--
1.9.5.msysgit.1

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

[edk2] [PATCH 1/3] Hisilicon/D0x: Set ACPI GTDT always-on flag

[Heyi Guo]

From: Jason Zhang 

Timer is always working on Hisilicon D0x, even system enters WFI/WFE,
and there is no other low power status, so we set "always-on" flag in
ACPI GTDT.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jason Zhang 
Signed-off-by: Heyi Guo 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Graeme Gregory 
---
 Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Gtdt.aslc | 3 ++-
 Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc| 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Gtdt.aslc 
b/Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Gtdt.aslc
index 4c1050ae83b9..3feb99e88c88 100644
--- a/Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Gtdt.aslc
+++ b/Silicon/Hisilicon/Hi1610/Hi1610AcpiTables/Gtdt.aslc
@@ -40,8 +40,9 @@
 #define GTDT_TIMER_LEVEL_TRIGGERED  0
 #define GTDT_TIMER_ACTIVE_LOW   
EFI_ACPI_6_1_GTDT_TIMER_FLAG_TIMER_INTERRUPT_POLARITY
 #define GTDT_TIMER_ACTIVE_HIGH  0
+#define GTDT_TIMER_ALWAYS_ON_CAPABILITY 
EFI_ACPI_6_1_GTDT_TIMER_FLAG_ALWAYS_ON_CAPABILITY
 
-#define GTDT_GTIMER_FLAGS   (GTDT_TIMER_ACTIVE_LOW | 
GTDT_TIMER_LEVEL_TRIGGERED)
+#define GTDT_GTIMER_FLAGS   (GTDT_TIMER_ALWAYS_ON_CAPABILITY | 
GTDT_TIMER_ACTIVE_LOW | GTDT_TIMER_LEVEL_TRIGGERED)
 
 #pragma pack (1)
 
diff --git a/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc 
b/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
index 16e2c6a972ba..2a9d209c00f0 100644
--- a/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
+++ b/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
@@ -28,7 +28,7 @@
 #define GTDT_TIMER_ACTIVE_HIGH  0
 #define GTDT_TIMER_ALWAYS_ON_CAPABILITY 
EFI_ACPI_6_1_GTDT_TIMER_FLAG_ALWAYS_ON_CAPABILITY
 
-#define GTDT_GTIMER_FLAGS   (GTDT_TIMER_ACTIVE_LOW | 
GTDT_TIMER_LEVEL_TRIGGERED)
+#define GTDT_GTIMER_FLAGS   (GTDT_TIMER_ALWAYS_ON_CAPABILITY | 
GTDT_TIMER_ACTIVE_LOW | GTDT_TIMER_LEVEL_TRIGGERED)
 
 #pragma pack (1)
 
-- 
2.7.4

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

[edk2] [PATCH 3/3] Hisilicon/D05: Support SBSA watchdog

[Heyi Guo]

From: Chenhui Sun 

Add description of SBSA watchdogs to ACPI GTDT on D05.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chenhui Sun 
Signed-off-by: Heyi Guo 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
Cc: Graeme Gregory 
---
 Platform/Hisilicon/D05/D05.dsc  |  4 
 Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf |  2 ++
 Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc| 19 
+++
 3 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
index 0792b0814ea1..22eaf356224d 100644
--- a/Platform/Hisilicon/D05/D05.dsc
+++ b/Platform/Hisilicon/D05/D05.dsc
@@ -418,6 +418,10 @@ [PcdsFixedAtBuild.common]
 
   gHisiTokenSpaceGuid.Pcdsoctype|0x1610
 
+  # SBSA watchdog on Hi1616
+  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase|0x4050
+  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase|0x4060
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform
diff --git a/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf 
b/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
index bb279c8e428e..6955e6145c30 100644
--- a/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
+++ b/Silicon/Hisilicon/Hi1616/D05AcpiTables/AcpiTablesHi1616.inf
@@ -55,5 +55,7 @@ [FixedPcd]
   gArmTokenSpaceGuid.PcdArmArchTimerIntrNum
   gArmTokenSpaceGuid.PcdArmArchTimerHypIntrNum
   gArmTokenSpaceGuid.PcdArmArchTimerVirtIntrNum
+  gArmTokenSpaceGuid.PcdGenericWatchdogControlBase
+  gArmTokenSpaceGuid.PcdGenericWatchdogRefreshBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialRegisterBase
 
diff --git a/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc 
b/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
index 2a9d209c00f0..6bc1bde2a490 100644
--- a/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
+++ b/Silicon/Hisilicon/Hi1616/D05AcpiTables/Gtdt.aslc
@@ -29,6 +29,7 @@
 #define GTDT_TIMER_ALWAYS_ON_CAPABILITY 
EFI_ACPI_6_1_GTDT_TIMER_FLAG_ALWAYS_ON_CAPABILITY
 
 #define GTDT_GTIMER_FLAGS   (GTDT_TIMER_ALWAYS_ON_CAPABILITY | 
GTDT_TIMER_ACTIVE_LOW | GTDT_TIMER_LEVEL_TRIGGERED)
+#define WATCHDOG_SPAN  0x2000
 
 #pragma pack (1)
 
@@ -57,22 +58,16 @@ EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLES Gtdt = {
 FixedPcdGet32 (PcdArmArchTimerHypIntrNum),// UINT32  
NonSecurePL2TimerGSIV
 GTDT_GTIMER_FLAGS,// UINT32  
NonSecurePL2TimerFlags
 0x,   // UINT64  
CntReadBasePhysicalAddress
-#ifdef notyet
-PV660_WATCHDOG_COUNT,  // UINT32  
PlatformTimerCount
+HI1616_WATCHDOG_COUNT,// UINT32  PlatformTimerCount
 sizeof (EFI_ACPI_6_1_GENERIC_TIMER_DESCRIPTION_TABLE) // UINT32 
PlatfromTimerOffset
   },
   {
-EFI_ACPI_6_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
-//FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 
(PcdGenericWatchdogControlBase), 93, 0),
-0, 0, 0, 0),
-EFI_ACPI_6_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
-//FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 
(PcdGenericWatchdogControlBase), 94, 
EFI_ACPI_6_1_GTDT_SBSA_GENERIC_WATCHDOG_FLAG_SECURE_TIMER)
-0, 0, 0, 0)
+EFI_ACPI_5_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
+  FixedPcdGet32 (PcdGenericWatchdogRefreshBase), FixedPcdGet32 
(PcdGenericWatchdogControlBase), 400, 0),
+EFI_ACPI_5_1_SBSA_GENERIC_WATCHDOG_STRUCTURE_INIT(
+  FixedPcdGet32 (PcdGenericWatchdogRefreshBase) + WATCHDOG_SPAN, 
FixedPcdGet32 (PcdGenericWatchdogControlBase) + WATCHDOG_SPAN, 496, 0)
+
   }
-#else /* !notyet */
-  0, 0
-  }
-#endif
   };
 
 //
-- 
2.7.4

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

Re: [edk2] [PATCH 6/7] SecurityPkg OpalPasswordSupportLib: Remove it

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, March 6, 2018 10:28 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen ;
> Dong, Eric ; Zhang, Chao B 
> Subject: [PATCH 6/7] SecurityPkg OpalPasswordSupportLib: Remove it
> 
> Remove OpalPasswordSupportLib as it is not been used
> anymore.
> 
> Cc: Jiewen Yao 
> Cc: Eric Dong 
> Cc: Chao Zhang 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  .../Include/Library/OpalPasswordSupportLib.h   | 289 
>  .../OpalPasswordSupportLib.c   | 781
> -
>  .../OpalPasswordSupportLib.inf |  55 --
>  .../OpalPasswordSupportNotify.h|  55 --
>  SecurityPkg/SecurityPkg.dec|   4 -
>  SecurityPkg/SecurityPkg.dsc|   2 -
>  6 files changed, 1186 deletions(-)
>  delete mode 100644 SecurityPkg/Include/Library/OpalPasswordSupportLib.h
>  delete mode 100644
> SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.c
>  delete mode 100644
> SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.inf
>  delete mode 100644
> SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportNotify.h
> 
> diff --git a/SecurityPkg/Include/Library/OpalPasswordSupportLib.h
> b/SecurityPkg/Include/Library/OpalPasswordSupportLib.h
> deleted file mode 100644
> index e616c763f05c..
> --- a/SecurityPkg/Include/Library/OpalPasswordSupportLib.h
> +++ /dev/null
> @@ -1,289 +0,0 @@
> -/** @file
> -  Header file of Opal password support library.
> -
> -Copyright (c) 2016, 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
> -which accompanies this distribution.  The full text of the license may be 
> found
> at
> -http://opensource.org/licenses/bsd-license.php
> -
> -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
> -
> -**/
> -
> -
> -#ifndef _OPAL_PASSWORD_SUPPORT_LIB_H_
> -#define _OPAL_PASSWORD_SUPPORT_LIB_H_
> -
> -#include 
> -#include 
> -
> -
> -#pragma pack(1)
> -
> -//
> -// Structure that is used to represent the available actions for an OpalDisk.
> -// The data can then be utilized to expose/hide certain actions available to 
> an
> end user
> -// by the consumer of this library.
> -//
> -typedef struct {
> -//
> -// Indicates if the disk can support PSID Revert action.  should verify 
> disk
> supports PSID authority
> -//
> -UINT16 PsidRevert : 1;
> -
> -//
> -// Indicates if the disk can support Revert action
> -//
> -UINT16 Revert : 1;
> -
> -//
> -// Indicates if the user must keep data for revert action.  It is true 
> if no
> media encryption is supported.
> -//
> -UINT16 RevertKeepDataForced : 1;
> -
> -//
> -// Indicates if the disk can support set Admin password
> -//
> -UINT16 AdminPass : 1;
> -
> -//
> -// Indicates if the disk can support set User password.  This action 
> requires
> that a user
> -// password is first enabled.
> -//
> -UINT16 UserPass : 1;
> -
> -//
> -// Indicates if unlock action is available.  Requires disk to be 
> currently
> locked.
> -//
> -UINT16 Unlock : 1;
> -
> -//
> -// Indicates if Secure Erase action is available.  Action requires admin
> credentials and media encryption support.
> -//
> -UINT16 SecureErase : 1;
> -
> -//
> -// Indicates if Disable User action is available.  Action requires admin
> credentials.
> -//
> -UINT16 DisableUser : 1;
> -} OPAL_DISK_ACTIONS;
> -
> -//
> -// Structure that is used to represent the Opal device with password info.
> -//
> -typedef struct {
> -  LIST_ENTRY Link;
> -
> -  UINT8  Password[32];
> -  UINT8  PasswordLength;
> -
> -  EFI_DEVICE_PATH_PROTOCOL   OpalDevicePath;
> -} OPAL_DISK_AND_PASSWORD_INFO;
> -
> -#pragma pack()
> -
> -/**
> -
> -  The function performs determines the available actions for the OPAL_DISK
> provided.
> -
> -  @param[in]   SupportedAttributes   The support attribute for the device.
> -  @param[in]   LockingFeatureThe locking status for the device.
> -  @param[in]   OwnerShip The ownership for the device.
> -  @param[out]  AvalDiskActions   Pointer to fill-out with appropriate
> disk actions.
> -
> -**/
> -TCG_RESULT
> -EFIAPI
> -OpalSupportGetAvailableActions(
> -  IN  OPAL_DISK_SUPPORT_ATTRIBUTE  *SupportedAttributes,
> -  IN  TCG_LOCKING_FEATURE_DESCRIPTOR   *LockingFeature,
> -  IN  UINT16 

Re: [edk2] [PATCH 7/7] SecurityPkg OpalPasswordExtraInfoVariable.h: Remove it

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, March 6, 2018 10:28 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen ;
> Dong, Eric ; Zhang, Chao B 
> Subject: [PATCH 7/7] SecurityPkg OpalPasswordExtraInfoVariable.h: Remove it
> 
> Remove OpalPasswordExtraInfoVariable.h as it is not been used
> anymore.
> 
> Cc: Jiewen Yao 
> Cc: Eric Dong 
> Cc: Chao Zhang 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  .../Include/Guid/OpalPasswordExtraInfoVariable.h   | 27 
> --
>  1 file changed, 27 deletions(-)
>  delete mode 100644
> SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h
> 
> diff --git a/SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h
> b/SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h
> deleted file mode 100644
> index f16d0a4ac360..
> --- a/SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h
> +++ /dev/null
> @@ -1,27 +0,0 @@
> -/** @file
> -  Defines Name GUIDs to represent an Opal device variable guid for Opal
> Security Feature.
> -
> -Copyright (c) 2016, 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
> -which accompanies this distribution.  The full text of the license may be 
> found
> at
> -http://opensource.org/licenses/bsd-license.php
> -
> -THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> -WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS
> OR IMPLIED.
> -
> -**/
> -
> -#ifndef _OPAL_PASSWORD_EXTRA_INFO_VARIABLE_H_
> -#define _OPAL_PASSWORD_EXTRA_INFO_VARIABLE_H_
> -
> -#define OPAL_EXTRA_INFO_VAR_NAME L"OpalExtraInfo"
> -
> -typedef struct {
> -  UINT8   EnableBlockSid;
> -} OPAL_EXTRA_INFO_VAR;
> -
> -extern EFI_GUID gOpalExtraInfoVariableGuid;
> -
> -#endif // _OPAL_PASSWORD_SECURITY_VARIABLE_H_
> -
> --
> 2.7.0.windows.1

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

Re: [edk2] [PATCH 3/7] SecurityPkg TcgStorageCoreLib: Make it be base type really

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, March 6, 2018 10:28 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen ;
> Dong, Eric ; Zhang, Chao B 
> Subject: [PATCH 3/7] SecurityPkg TcgStorageCoreLib: Make it be base type 
> really
> 
> Cc: Jiewen Yao 
> Cc: Eric Dong 
> Cc: Chao Zhang 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf
> b/SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf
> index a80cb00a9ce3..78a0557bfe42 100644
> --- a/SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf
> +++ b/SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf
> @@ -3,7 +3,7 @@
>  #
>  #  This module is used to provide API used by Opal password solution.
>  #
> -# Copyright (c) 2016, Intel Corporation. All rights reserved.
> +# Copyright (c) 2016 - 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
>  # which accompanies this distribution. The full text of the license may be 
> found
> at
> @@ -18,7 +18,7 @@ [Defines]
>FILE_GUID  =
> ad63b09b-1fc9-4789-af0c-5af8a3fb1f9c
>VERSION_STRING = 1.0
>MODULE_TYPE= BASE
> -  LIBRARY_CLASS  = TcgStorageCoreLib|DXE_DRIVER
> DXE_CORE DXE_SMM_DRIVER
> +  LIBRARY_CLASS  = TcgStorageCoreLib
>  #
>  # The following information is for reference only and not required by the 
> build
> tools.
>  #
> --
> 2.7.0.windows.1

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

Re: [edk2] [PATCH 2/7] SecurityPkg TcgStorageOpalLib: Make it be base type really

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, March 6, 2018 10:28 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Yao, Jiewen ;
> Dong, Eric ; Zhang, Chao B 
> Subject: [PATCH 2/7] SecurityPkg TcgStorageOpalLib: Make it be base type 
> really
> 
> Cc: Jiewen Yao 
> Cc: Eric Dong 
> Cc: Chao Zhang 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf | 8 ++--
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
> b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
> index ec26111ee2c3..78e47387a981 100644
> --- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
> +++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
> @@ -3,7 +3,7 @@
>  #
>  #  This module is used to provide API used by Opal password solution.
>  #
> -# Copyright (c) 2016, Intel Corporation. All rights reserved.
> +# Copyright (c) 2016 - 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
>  # which accompanies this distribution. The full text of the license may be 
> found
> at
> @@ -18,7 +18,7 @@ [Defines]
>FILE_GUID  =
> F8B56221-FD5D-4215-B578-C3574AD1E253
>VERSION_STRING = 1.0
>MODULE_TYPE= BASE
> -  LIBRARY_CLASS  = TcgStorageOpalLib|DXE_DRIVER
> DXE_CORE DXE_SMM_DRIVER
> +  LIBRARY_CLASS  = TcgStorageOpalLib
> 
>  #
>  # The following information is for reference only and not required by the 
> build
> tools.
> @@ -37,11 +37,7 @@ [LibraryClasses]
>DebugLib
>TimerLib
>TcgStorageCoreLib
> -  UefiLib
> 
>  [Packages]
>MdePkg/MdePkg.dec
>SecurityPkg/SecurityPkg.dec
> -
> -[Protocols]
> -  gEfiStorageSecurityCommandProtocolGuid ##
> CONSUMES
> --
> 2.7.0.windows.1

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

Re: [edk2] [PATCH 0/3] BaseTools: let the C-language build utils compile with gcc-8

[Gao, Liming]

Laszlo:
  We just find this change causes XCODE5 build failure, because XCODE compiler 
doesn't know these two options. So, could you provide the patch to remove the 
change in BUILD_CFLAGS for MAC?

ifeq ($(DARWIN),Darwin)
# assume clang or clang compatible flags on OS X
BUILD_CFLAGS =
else

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Gao, 
> Liming
> Sent: Monday, March 5, 2018 10:14 PM
> To: Laszlo Ersek ; edk2-devel-01 
> Cc: Paolo Bonzini ; Cole Robinson ; 
> Ard Biesheuvel 
> Subject: Re: [edk2] [PATCH 0/3] BaseTools: let the C-language build utils 
> compile with gcc-8
> 
> Reviewed-by: Liming Gao 
> 
> >-Original Message-
> >From: Laszlo Ersek [mailto:ler...@redhat.com]
> >Sent: Saturday, March 03, 2018 2:09 AM
> >To: edk2-devel-01 
> >Cc: Ard Biesheuvel ; Cole Robinson
> >; Gao, Liming ; Paolo Bonzini
> >; Zhu, Yonghong 
> >Subject: [PATCH 0/3] BaseTools: let the C-language build utils compile with
> >gcc-8
> >
> >Repo:   https://github.com/lersek/edk2.git
> >Branch: basetools_gcc8
> >
> >Once these patches are applied to the build flags and the source code of
> >the build utilities themselves, OVMF builds fine with gcc-8, using the
> >GCC5 toolchain settings without any changes.
> >
> >Regression-tested with gcc-4.8 / x86_64.
> >
> >Cc: Ard Biesheuvel 
> >Cc: Cole Robinson 
> >Cc: Liming Gao 
> >Cc: Paolo Bonzini 
> >Cc: Yonghong Zhu 
> >
> >Thanks
> >Laszlo
> >
> >Laszlo Ersek (3):
> >  BaseTools/header.makefile: add "-Wno-stringop-truncation"
> >  BaseTools/header.makefile: add "-Wno-restrict"
> >  BaseTools/GenVtf: silence false "stringop-overflow" warning with
> >memcpy()
> >
> > BaseTools/Source/C/Makefiles/header.makefile | 4 ++--
> > BaseTools/Source/C/GenVtf/GenVtf.c   | 4 ++--
> > 2 files changed, 4 insertions(+), 4 deletions(-)
> >
> >--
> >2.14.1.3.gb7cf6e02401b
> 
> ___
> 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/7] MdeModulePkg LockBoxLib: Support LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY

[Yao, Jiewen]

Reviewed-by: jiewen@intel.com

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Star
> Zeng
> Sent: Tuesday, March 6, 2018 10:28 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Zeng, Star 
> Subject: [edk2] [PATCH 1/7] MdeModulePkg LockBoxLib: Support
> LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY
> 
> With this flag, the LockBox can be restored in S3 resume only.
> The LockBox can not be restored after SmmReadyToLock in normal boot
> and after EndOfS3Resume in S3 resume.
> It can not be set together with LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE.
> 
> Cc: Jiewen Yao 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  MdeModulePkg/Include/Library/LockBoxLib.h  |  14 +-
>  .../Library/SmmLockBoxLib/SmmLockBoxDxeLib.c   |   4 +-
>  .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.c   | 227
> -
>  .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf |  10 +-
>  4 files changed, 247 insertions(+), 8 deletions(-)
> 
> diff --git a/MdeModulePkg/Include/Library/LockBoxLib.h
> b/MdeModulePkg/Include/Library/LockBoxLib.h
> index db7fd05def58..80beb4d0f880 100644
> --- a/MdeModulePkg/Include/Library/LockBoxLib.h
> +++ b/MdeModulePkg/Include/Library/LockBoxLib.h
> @@ -2,7 +2,7 @@
>This library is only intended to be used by DXE modules that need save
>confidential information to LockBox and get it by PEI modules in S3 phase.
> 
> -Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved.
> +Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
> 
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions
> @@ -62,9 +62,17 @@ SetLockBoxAttributes (
>);
> 
>  //
> -// With this flag, this LockBox can be restored to this Buffer with
> RestoreAllLockBoxInPlace()
> +// With this flag, this LockBox can be restored to this Buffer
> +// with RestoreAllLockBoxInPlace()
>  //
> -#define LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE  BIT0
> +#define LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE BIT0
> +//
> +// With this flag, this LockBox can be restored in S3 resume only.
> +// This LockBox can not be restored after SmmReadyToLock in normal boot
> +// and after EndOfS3Resume in S3 resume.
> +// It can not be set together with LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE.
> +//
> +#define LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY   BIT1
> 
>  /**
>This function will update confidential information to lockbox.
> diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
> b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
> index b75f81e69e04..9b6f0bedbd4f 100644
> --- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
> +++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
> +Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
> 
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions
> @@ -241,7 +241,7 @@ SetLockBoxAttributes (
>// Basic check
>//
>if ((Guid == NULL) ||
> -  ((Attributes & ~LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE) != 0)) {
> +  ((Attributes & ~(LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE |
> LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY)) != 0)) {
>  return EFI_INVALID_PARAMETER;
>}
> 
> diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> index 4960df755534..af75a4cb9cd1 100644
> --- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> +++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
> @@ -1,6 +1,6 @@
>  /** @file
> 
> -Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
> 
>  This program and the accompanying materials
>  are licensed and made available under the terms and conditions
> @@ -20,6 +20,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
>  #include 
>  #include 
>  #include 
> +#include 
> +#include 
> +#include 
> +#include 
> 
>  #include "SmmLockBoxLibPrivate.h"
> 
> @@ -31,6 +35,11 @@ SMM_LOCK_BOX_CONTEXT mSmmLockBoxContext;
>  LIST_ENTRY   mLockBoxQueue = INITIALIZE_LIST_HEAD_VARIABLE
> (mLockBoxQueue);
> 
>  BOOLEAN  mSmmConfigurationTableInstalled = FALSE;
> +VOID *mRegistrationSmmEndOfDxe = NULL;
> +VOID *mRegistrationSmmReadyToLock = NULL;
> +VOID *mRegistrationEndOfS3Resume = NULL;
> +BOOLEAN  mSmmLockBoxSmmReadyToLock = FALSE;
> +BOOLEAN  mSmmLockBoxDuringS3Resume = FALSE;
> 
>  /**
>This function return SmmLockBox context from SMST.
> @@ -64,6 +73,128 @@ InternalGetSmmLockBoxContext (
>  }
> 
>  /**
> +  

Re: [edk2] [PATCH v5 4/6] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

[Guo Heyi]

Thanks. Please let me know if any further changes are needed.

Regards,

Heyi

On Wed, Mar 07, 2018 at 12:30:59PM +0800, Ni, Ruiyu wrote:
> On 3/6/2018 10:44 AM, Guo Heyi wrote:
> >Hi Ray,
> >
> >Any comments for v5?
> 
> Heyi,
> Some backward compatibility concerns were received from internal production
> teams. Current change will cause silent failure on old platforms because
> TranslationOffset might be random if uninitialized.
> I will solve the concern and then send out updates to you, hopefully by end
> of next week.
> 
> >
> >Regards,
> >
> >Heyi
> >
> >On Thu, Mar 01, 2018 at 02:57:22PM +0800, Heyi Guo wrote:
> >>PCI address translation is necessary for some non-x86 platforms. On
> >>such platforms, address value (denoted as "device address" or "address
> >>in PCI view") set to PCI BAR registers in configuration space might be
> >>different from the address which is used by CPU to access the
> >>registers in memory BAR or IO BAR spaces (denoted as "host address" or
> >>"address in CPU view"). The difference between the two addresses is
> >>called "Address Translation Offset" or simply "translation", and can
> >>be represented by "Address Translation Offset" in ACPI QWORD Address
> >>Space Descriptor (Offset 0x1E). However UEFI and ACPI differs on the
> >>definitions of QWORD Address Space Descriptor, and we will follow UEFI
> >>definition on UEFI protocols, such as PCI root bridge IO protocol and
> >>PCI IO protocol. In UEFI 2.7, "Address Translation Offset" is "Offset
> >>to apply to the Starting address to convert it to a PCI address". This
> >>means:
> >>
> >>1. Translation = device address - host address.
> >>
> >>2. PciRootBridgeIo->Configuration should return CPU view address, as
> >>well as PciIo->GetBarAttributes.
> >>
> >>Summary of addresses used in protocol interfaces and internal
> >>implementations:
> >>
> >>1. *Only* the following protocol interfaces assume Address is Device
> >>Address:
> >>(1). PciHostBridgeResourceAllocation.GetProposedResources()
> >>  Otherwise PCI bus driver cannot set correct address into PCI
> >>  BARs.
> >>(2). PciRootBridgeIo.Mem.Read() and PciRootBridgeIo.Mem.Write()
> >>(3). PciRootBridgeIo.CopyMem()
> >>UEFI and PI spec have clear statements for all other protocol
> >>interfaces about the address type.
> >>
> >>2. Library interfaces and internal implementation:
> >>(1). Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address.
> >>  It is easy to check whether the address is below 4G or above 4G.
> >>(2). Addresses in PCI_ROOT_BRIDGE_INSTANCE.ResAllocNode are host
> >>  address, for they are allocated from GCD.
> >>(3). Address passed to PciHostBridgeResourceConflict is host address,
> >>  for it comes from PCI_ROOT_BRIDGE_INSTANCE.ResAllocNode.
> >>
> >>RESTRICTION: to simplify the situation, we require the alignment of
> >>Translation must be larger than any BAR alignment in the same root
> >>bridge, so that resource allocation alignment can be applied to both
> >>device address and host address.
> >>
> >>Contributed-under: TianoCore Contribution Agreement 1.1
> >>Signed-off-by: Heyi Guo 
> >>Cc: Ruiyu Ni 
> >>Cc: Ard Biesheuvel 
> >>Cc: Star Zeng 
> >>Cc: Eric Dong 
> >>Cc: Laszlo Ersek 
> >>Cc: Michael D Kinney 
> >>---
> >>
> >>Notes:
> >> v5:
> >> - Add check for the alignment of Translation against the BAR alignment
> >>   [Ray]
> >> - Keep coding style of comments consistent with the context [Ray]
> >> - Comment change for Base in PCI_RES_NODE [Ray]
> >> - Add macros of TO_HOST_ADDRESS and TO_DEVICE_ADDRESS for address type
> >>   conversion (After that we can also simply the comments about the
> >>   calculation [Ray]
> >> - Add check for bus translation offset in CreateRootBridge(), making
> >>   sure it is zero, and unify code logic for all types of resource
> >>   after that [Ray]
> >> - Use GetTranslationByResourceType() to simplify the code in
> >>   RootBridgeIoConfiguration() (also fix a bug in previous patch
> >>   version of missing a break after case TypePMem64) [Ray]
> >> - Commit message refinement [Ray]
> >> v4:
> >> - Add ASSERT (FALSE) to default branch in GetTranslationByResourceType
> >>   [Laszlo]
> >> - Fix bug when passing BaseAddress to gDS->AllocateIoSpace and
> >>   gDS->AllocateMemorySpace [Laszlo]
> >> - Add comment for applying alignment to both device address and host
> >>   address, and add NOTE for the alignment requirement of Translation,
> >>   as well as in commit message [Laszlo][Ray]
> >> - Improve indention for the code in CreateRootBridge [Laszlo]
> >> - Improve comment for Translation in PCI_ROOT_BRIDGE_APERTURE
> >>   definition [Laszlo]
> >> - Ignore translation of bus in CreateRootBridge
> >> v4:
> >> - Add 

Re: [edk2] [patch] UefiCpuPkg/CpuCommonFeaturesLib: Fix coding style issue

[Ni, Ruiyu]

On 3/7/2018 1:53 PM, Dandan Bi wrote:

Boolean values do not need to use explicit comparisons
to TRUE or FALSE.

Cc: Eric Dong 
Cc: Laszlo Ersek 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
  UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c 
b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
index cc64dbb..27ca911 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
@@ -138,11 +138,11 @@ McaInitialize (
)
  {
MSR_IA32_MCG_CAP_REGISTER  McgCap;
UINT32 BankIndex;
  
-  if (State == TRUE) {

+  if (State) {
  McgCap.Uint64 = AsmReadMsr64 (MSR_IA32_MCG_CAP);
  for (BankIndex = 0; BankIndex < (UINT32) McgCap.Bits.Count; BankIndex++) {
CPU_REGISTER_TABLE_WRITE64 (
  ProcessorNumber,
  Msr,


Reviewed-by: Ruiyu Ni 

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

[edk2] [patch] SecurityPkg/SmmTcg2PhysicalPresenceLib: Fix coding style issue

[Dandan Bi]

Boolean values do not need to use explicit comparisons
to TRUE or FALSE.

Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 .../Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git 
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c 
b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
index dfef6c8..6a4dce9 100644
--- 
a/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
+++ 
b/SecurityPkg/Library/SmmTcg2PhysicalPresenceLib/SmmTcg2PhysicalPresenceLib.c
@@ -339,11 +339,11 @@ Tcg2PhysicalPresenceLibGetUserConfirmationStatusFunction (
 case 
TCG2_PHYSICAL_PRESENCE_SET_PP_REQUIRED_FOR_ENABLE_BLOCK_SID_FUNC_FALSE:
 case 
TCG2_PHYSICAL_PRESENCE_SET_PP_REQUIRED_FOR_DISABLE_BLOCK_SID_FUNC_FALSE:
   break;
 
 default:
-  if (mIsTcg2PPVerLowerThan_1_3 == FALSE) {
+  if (!mIsTcg2PPVerLowerThan_1_3) {
 if (OperationRequest < 
TCG2_PHYSICAL_PRESENCE_VENDOR_SPECIFIC_OPERATION) {
   //
   // TCG2 PP1.3 spec defined operations that are reserved or 
un-implemented
   //
   return TCG_PP_GET_USER_CONFIRMATION_NOT_IMPLEMENTED;
-- 
1.9.5.msysgit.1

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

[edk2] [patch] UefiCpuPkg/CpuCommonFeaturesLib: Fix coding style issue

[Dandan Bi]

Boolean values do not need to use explicit comparisons
to TRUE or FALSE.

Cc: Eric Dong 
Cc: Laszlo Ersek 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c 
b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
index cc64dbb..27ca911 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/MachineCheck.c
@@ -138,11 +138,11 @@ McaInitialize (
   )
 {
   MSR_IA32_MCG_CAP_REGISTER  McgCap;
   UINT32 BankIndex;
 
-  if (State == TRUE) {
+  if (State) {
 McgCap.Uint64 = AsmReadMsr64 (MSR_IA32_MCG_CAP);
 for (BankIndex = 0; BankIndex < (UINT32) McgCap.Bits.Count; BankIndex++) {
   CPU_REGISTER_TABLE_WRITE64 (
 ProcessorNumber,
 Msr,
-- 
1.9.5.msysgit.1

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

[edk2] [PATCH] MdeModulePkg/Core: fix too many available pages between BS_Data

[Jian J Wang]

The root cause is an unnecessary check to Size parameter in function
AdjustMemoryS(). It will cause one standalone free page (happen to have
Guard page around) in the free memory list cannot be allocated, even if
the requested memory size is less than a page.

  //
  // At least one more page needed for Guard page.
  //
  if (Size < (SizeRequested + EFI_PAGES_TO_SIZE (1))) {
return 0;
  }

The following code in the same function actually covers above check
implicitly. So the fix is simply removing above check.

Cc: Star Zeng 
Cc: Eric Dong 
Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c   | 9 +
 MdeModulePkg/Core/PiSmmCore/HeapGuard.c | 9 +
 2 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c 
b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
index d7906e08c5..19245049c2 100644
--- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
+++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
@@ -1,7 +1,7 @@
 /** @file
   UEFI Heap Guard functions.
 
-Copyright (c) 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2017-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
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -905,13 +905,6 @@ AdjustMemoryS (
 
   Target = Start + Size - SizeRequested;
 
-  //
-  // At least one more page needed for Guard page.
-  //
-  if (Size < (SizeRequested + EFI_PAGES_TO_SIZE (1))) {
-return 0;
-  }
-
   if (!IsGuardPage (Start + Size)) {
 // No Guard at tail to share. One more page is needed.
 Target -= EFI_PAGES_TO_SIZE (1);
diff --git a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c 
b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
index c5ffb26342..aa9c25d102 100644
--- a/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
+++ b/MdeModulePkg/Core/PiSmmCore/HeapGuard.c
@@ -1,7 +1,7 @@
 /** @file
   UEFI Heap Guard functions.
 
-Copyright (c) 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2017-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
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -888,13 +888,6 @@ AdjustMemoryS (
 
   Target = Start + Size - SizeRequested;
 
-  //
-  // At least one more page needed for Guard page.
-  //
-  if (Size < (SizeRequested + EFI_PAGES_TO_SIZE (1))) {
-return 0;
-  }
-
   if (!IsGuardPage (Start + Size)) {
 // No Guard at tail to share. One more page is needed.
 Target -= EFI_PAGES_TO_SIZE (1);
-- 
2.14.1.windows.1

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

Re: [edk2] [PATCH v5 4/6] MdeModulePkg/PciHostBridgeDxe: Add support for address translation

[Ni, Ruiyu]

On 3/6/2018 10:44 AM, Guo Heyi wrote:

Hi Ray,

Any comments for v5?


Heyi,
Some backward compatibility concerns were received from internal 
production teams. Current change will cause silent failure on old 
platforms because TranslationOffset might be random if uninitialized.
I will solve the concern and then send out updates to you, hopefully by 
end of next week.




Regards,

Heyi

On Thu, Mar 01, 2018 at 02:57:22PM +0800, Heyi Guo wrote:

PCI address translation is necessary for some non-x86 platforms. On
such platforms, address value (denoted as "device address" or "address
in PCI view") set to PCI BAR registers in configuration space might be
different from the address which is used by CPU to access the
registers in memory BAR or IO BAR spaces (denoted as "host address" or
"address in CPU view"). The difference between the two addresses is
called "Address Translation Offset" or simply "translation", and can
be represented by "Address Translation Offset" in ACPI QWORD Address
Space Descriptor (Offset 0x1E). However UEFI and ACPI differs on the
definitions of QWORD Address Space Descriptor, and we will follow UEFI
definition on UEFI protocols, such as PCI root bridge IO protocol and
PCI IO protocol. In UEFI 2.7, "Address Translation Offset" is "Offset
to apply to the Starting address to convert it to a PCI address". This
means:

1. Translation = device address - host address.

2. PciRootBridgeIo->Configuration should return CPU view address, as
well as PciIo->GetBarAttributes.

Summary of addresses used in protocol interfaces and internal
implementations:

1. *Only* the following protocol interfaces assume Address is Device
Address:
(1). PciHostBridgeResourceAllocation.GetProposedResources()
  Otherwise PCI bus driver cannot set correct address into PCI
  BARs.
(2). PciRootBridgeIo.Mem.Read() and PciRootBridgeIo.Mem.Write()
(3). PciRootBridgeIo.CopyMem()
UEFI and PI spec have clear statements for all other protocol
interfaces about the address type.

2. Library interfaces and internal implementation:
(1). Base and Limit in PCI_ROOT_BRIDGE_APERTURE are device address.
  It is easy to check whether the address is below 4G or above 4G.
(2). Addresses in PCI_ROOT_BRIDGE_INSTANCE.ResAllocNode are host
  address, for they are allocated from GCD.
(3). Address passed to PciHostBridgeResourceConflict is host address,
  for it comes from PCI_ROOT_BRIDGE_INSTANCE.ResAllocNode.

RESTRICTION: to simplify the situation, we require the alignment of
Translation must be larger than any BAR alignment in the same root
bridge, so that resource allocation alignment can be applied to both
device address and host address.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Ruiyu Ni 
Cc: Ard Biesheuvel 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Laszlo Ersek 
Cc: Michael D Kinney 
---

Notes:
 v5:
 - Add check for the alignment of Translation against the BAR alignment
   [Ray]
 - Keep coding style of comments consistent with the context [Ray]
 - Comment change for Base in PCI_RES_NODE [Ray]
 - Add macros of TO_HOST_ADDRESS and TO_DEVICE_ADDRESS for address type
   conversion (After that we can also simply the comments about the
   calculation [Ray]
 - Add check for bus translation offset in CreateRootBridge(), making
   sure it is zero, and unify code logic for all types of resource
   after that [Ray]
 - Use GetTranslationByResourceType() to simplify the code in
   RootBridgeIoConfiguration() (also fix a bug in previous patch
   version of missing a break after case TypePMem64) [Ray]
 - Commit message refinement [Ray]
 
 v4:

 - Add ASSERT (FALSE) to default branch in GetTranslationByResourceType
   [Laszlo]
 - Fix bug when passing BaseAddress to gDS->AllocateIoSpace and
   gDS->AllocateMemorySpace [Laszlo]
 - Add comment for applying alignment to both device address and host
   address, and add NOTE for the alignment requirement of Translation,
   as well as in commit message [Laszlo][Ray]
 - Improve indention for the code in CreateRootBridge [Laszlo]
 - Improve comment for Translation in PCI_ROOT_BRIDGE_APERTURE
   definition [Laszlo]
 - Ignore translation of bus in CreateRootBridge
 
 v4:

 - Add ASSERT (FALSE) to default branch in GetTranslationByResourceType
   [Laszlo]
 - Fix bug when passing BaseAddress to gDS->AllocateIoSpace and
   gDS->AllocateMemorySpace [Laszlo]
 - Add comment for applying alignment to both device address and host
   address, and add NOTE for the alignment requirement of Translation,
   as well as in commit message [Laszlo][Ray]
 - Improve indention for the code in CreateRootBridge [Laszlo]
 - Improve comment for Translation in 

Re: [edk2] [PATCH v2 0/2] Resolve feature conflict between NX and Stack guard

[Wu, Hao A]

Hi Ray,

Below are the answers to your feedbacks:

> -Original Message-
> From: Yao, Jiewen
> Sent: Wednesday, March 07, 2018 12:14 PM
> To: Wang, Jian J; Ni, Ruiyu; Wu, Hao A; edk2-devel@lists.01.org
> Cc: Zeng, Star; Dong, Eric
> Subject: RE: [PATCH v2 0/2] Resolve feature conflict between NX and Stack
> guard
> 
> I think the original patch is fine.
> 
> StackBase is already checked by using ASSERT before.
> > +ASSERT ((StackBase & EFI_PAGE_MASK) == 0);
> 
> MemMap entry must be page aligned.
> 
> No additional check is required here.
> 
> Thank you
> Yao Jiewen
> 
> 
> > -Original Message-
> > From: Wang, Jian J
> > Sent: Wednesday, March 7, 2018 11:40 AM
> > To: Ni, Ruiyu ; Wu, Hao A ;
> > edk2-devel@lists.01.org
> > Cc: Zeng, Star ; Dong, Eric ; Yao,
> > Jiewen 
> > Subject: RE: [PATCH v2 0/2] Resolve feature conflict between NX and Stack
> guard
> >
> >
> >
> > Regards,
> > Jian
> >
> >
> > > -Original Message-
> > > From: Ni, Ruiyu
> > > Sent: Wednesday, March 07, 2018 11:17 AM
> > > To: Wu, Hao A ; edk2-devel@lists.01.org
> > > Cc: Wang, Jian J ; Zeng, Star
> ;
> > > Dong, Eric ; Yao, Jiewen 
> > > Subject: Re: [PATCH v2 0/2] Resolve feature conflict between NX and Stack
> > > guard
> > >
> > > On 3/6/2018 9:33 PM, Hao Wu wrote:
> > > > V2 changes:
> > > >
> > > > A. Use Hoblib APIs to get the base of stack from Hob.
> > > > B. Remove unnecessary local variable used in function
> > > > InitializeDxeNxMemoryProtectionPolicy().
> > > >
> > > > V1 history:
> > > >
> > > > If enabled, NX memory protection feature will mark some types of active
> > > > memory as NX (non-executable), which includes the first page of the 
> > > > stack.
> > > > This will overwrite the attributes of the first page of the stack if the
> > > > stack guard feature is also enabled.
> > > >
> > > > The series will override the attributes setting to the first page of the
> > > > stack by adding back the 'EFI_MEMORY_RP' attribute when the stack
> guard
> > > > feature is enabled.
> > > >
> > > > Cc: Jian J Wang 
> > > > Cc: Star Zeng 
> > > > Cc: Eric Dong 
> > > > Cc: Jiewen Yao 
> > > > Cc: Ruiyu Ni 
> > > >
> > > > Hao Wu (2):
> > > >MdeModulePkg/Core: Refine handling NULL detection in NX setting
> > > >MdeModulePkg/Core: Fix feature conflict between NX and Stack guard
> > > >
> > > >   MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
> > > >   MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 74
> > > +++
> > > >   2 files changed, 67 insertions(+), 11 deletions(-)
> > > >
> > >
> > >if (MemoryMapEntry->PhysicalStart == 0 &&
> > >PcdGet8 (PcdNullPointerDetectionPropertyMask) != 0) {
> > >
> > >  ASSERT (MemoryMapEntry->NumberOfPages > 0);
> > >  //
> > >  // Add EFI_MEMORY_RP attribute for page 0 if NULL pointer
> > > detection is
> > >  // enabled.
> > >  //
> > > [Ray] 1. I prefer to move the above comments before the "if (...)".

Yes. I agree that moving the comments block out of the 'if' statement is
more readable. I will update according to your suggestion when I pushing
the changes.

> > >
> > >  SetUefiImageMemoryAttributes (
> > >0,
> > >EFI_PAGES_TO_SIZE (1),
> > >EFI_MEMORY_RP | Attributes);
> > >}
> > >
> > >if (StackBase != 0 &&
> > >(StackBase >= MemoryMapEntry->PhysicalStart &&
> > > StackBase <  MemoryMapEntry->PhysicalStart +
> > >  LShiftU64
> > (MemoryMapEntry->NumberOfPages,
> > > EFI_PAGE_SHIFT)) &&
> > >PcdGetBool (PcdCpuStackGuard)) {
> > >
> > >  //
> > >  // Add EFI_MEMORY_RP attribute for the first page of the stack
> > > if stack
> > >  // guard is enabled.
> > >  //
> > >  SetUefiImageMemoryAttributes (
> > >StackBase,
> > >EFI_PAGES_TO_SIZE (1),
> > >EFI_MEMORY_RP | Attributes);
> > > [Ray] 2. The StackBase is directly used here. So do we need to check
> > > whether it's page aligned? Do we need to check whether the range
> > > [StackBase, StackBase + 4KB) is inside the MemoryMapEntry?
> > >}
> >
> > If PcdCpuStackGuard is TRUE, I think the owner who allocates memory for
> > StackBase
> > should make sure all the conditions you mentioned, but not here.
> >

Just as Jiewen mentioned in the previous reply. An ASSERT:
ASSERT ((StackBase & EFI_PAGE_MASK) == 0);

is added to ensure the stack base fetched from Hob is page-size aligned.

And the below check:
(StackBase >= MemoryMapEntry->PhysicalStart &&
 StackBase <  MemoryMapEntry->PhysicalStart +
  

Re: [edk2] [PATCH 0/2] Fix bug in CompatibleRangeTest

[Gao, Liming]

Reviewed-by: Liming Gao 

>-Original Message-
>From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
>Kinney, Michael D
>Sent: Wednesday, March 07, 2018 12:09 PM
>To: Ni, Ruiyu ; edk2-devel@lists.01.org; Kinney, Michael
>D 
>Subject: Re: [edk2] [PATCH 0/2] Fix bug in CompatibleRangeTest
>
>Reviewed-by: Michael D Kinney 
>
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-
>> boun...@lists.01.org] On Behalf Of Ruiyu Ni
>> Sent: Monday, March 5, 2018 7:40 PM
>> To: edk2-devel@lists.01.org
>> Subject: [edk2] [PATCH 0/2] Fix bug in
>> CompatibleRangeTest
>>
>>
>> Ruiyu Ni (2):
>>   MdeModulePkg/NullMemoryTest: Change prototype of
>> ConvertToTestedMemory
>>   MdeModulePkg/NullMemoryTest: Fix bug in
>> CompatibleRangeTest
>>
>>  .../MemoryTest/NullMemoryTestDxe/NullMemoryTest.c  | 82
>> +-
>>  1 file changed, 65 insertions(+), 17 deletions(-)
>>
>> --
>> 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-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 0/2] Resolve feature conflict between NX and Stack guard

[Yao, Jiewen]

I think the original patch is fine.

StackBase is already checked by using ASSERT before.
> +ASSERT ((StackBase & EFI_PAGE_MASK) == 0);

MemMap entry must be page aligned.

No additional check is required here.

Thank you
Yao Jiewen


> -Original Message-
> From: Wang, Jian J
> Sent: Wednesday, March 7, 2018 11:40 AM
> To: Ni, Ruiyu ; Wu, Hao A ;
> edk2-devel@lists.01.org
> Cc: Zeng, Star ; Dong, Eric ; Yao,
> Jiewen 
> Subject: RE: [PATCH v2 0/2] Resolve feature conflict between NX and Stack 
> guard
> 
> 
> 
> Regards,
> Jian
> 
> 
> > -Original Message-
> > From: Ni, Ruiyu
> > Sent: Wednesday, March 07, 2018 11:17 AM
> > To: Wu, Hao A ; edk2-devel@lists.01.org
> > Cc: Wang, Jian J ; Zeng, Star ;
> > Dong, Eric ; Yao, Jiewen 
> > Subject: Re: [PATCH v2 0/2] Resolve feature conflict between NX and Stack
> > guard
> >
> > On 3/6/2018 9:33 PM, Hao Wu wrote:
> > > V2 changes:
> > >
> > > A. Use Hoblib APIs to get the base of stack from Hob.
> > > B. Remove unnecessary local variable used in function
> > > InitializeDxeNxMemoryProtectionPolicy().
> > >
> > > V1 history:
> > >
> > > If enabled, NX memory protection feature will mark some types of active
> > > memory as NX (non-executable), which includes the first page of the stack.
> > > This will overwrite the attributes of the first page of the stack if the
> > > stack guard feature is also enabled.
> > >
> > > The series will override the attributes setting to the first page of the
> > > stack by adding back the 'EFI_MEMORY_RP' attribute when the stack guard
> > > feature is enabled.
> > >
> > > Cc: Jian J Wang 
> > > Cc: Star Zeng 
> > > Cc: Eric Dong 
> > > Cc: Jiewen Yao 
> > > Cc: Ruiyu Ni 
> > >
> > > Hao Wu (2):
> > >MdeModulePkg/Core: Refine handling NULL detection in NX setting
> > >MdeModulePkg/Core: Fix feature conflict between NX and Stack guard
> > >
> > >   MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
> > >   MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 74
> > +++
> > >   2 files changed, 67 insertions(+), 11 deletions(-)
> > >
> >
> >if (MemoryMapEntry->PhysicalStart == 0 &&
> >PcdGet8 (PcdNullPointerDetectionPropertyMask) != 0) {
> >
> >  ASSERT (MemoryMapEntry->NumberOfPages > 0);
> >  //
> >  // Add EFI_MEMORY_RP attribute for page 0 if NULL pointer
> > detection is
> >  // enabled.
> >  //
> > [Ray] 1. I prefer to move the above comments before the "if (...)".
> >
> >  SetUefiImageMemoryAttributes (
> >0,
> >EFI_PAGES_TO_SIZE (1),
> >EFI_MEMORY_RP | Attributes);
> >}
> >
> >if (StackBase != 0 &&
> >(StackBase >= MemoryMapEntry->PhysicalStart &&
> > StackBase <  MemoryMapEntry->PhysicalStart +
> >  LShiftU64
> (MemoryMapEntry->NumberOfPages,
> > EFI_PAGE_SHIFT)) &&
> >PcdGetBool (PcdCpuStackGuard)) {
> >
> >  //
> >  // Add EFI_MEMORY_RP attribute for the first page of the stack
> > if stack
> >  // guard is enabled.
> >  //
> >  SetUefiImageMemoryAttributes (
> >StackBase,
> >EFI_PAGES_TO_SIZE (1),
> >EFI_MEMORY_RP | Attributes);
> > [Ray] 2. The StackBase is directly used here. So do we need to check
> > whether it's page aligned? Do we need to check whether the range
> > [StackBase, StackBase + 4KB) is inside the MemoryMapEntry?
> >}
> 
> If PcdCpuStackGuard is TRUE, I think the owner who allocates memory for
> StackBase
> should make sure all the conditions you mentioned, but not here.
> 
> >
> > --
> > Thanks,
> > Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/2] Fix bug in CompatibleRangeTest

[Kinney, Michael D]

Reviewed-by: Michael D Kinney 

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-
> boun...@lists.01.org] On Behalf Of Ruiyu Ni
> Sent: Monday, March 5, 2018 7:40 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH 0/2] Fix bug in
> CompatibleRangeTest
> 
> 
> Ruiyu Ni (2):
>   MdeModulePkg/NullMemoryTest: Change prototype of
> ConvertToTestedMemory
>   MdeModulePkg/NullMemoryTest: Fix bug in
> CompatibleRangeTest
> 
>  .../MemoryTest/NullMemoryTestDxe/NullMemoryTest.c  | 82
> +-
>  1 file changed, 65 insertions(+), 17 deletions(-)
> 
> --
> 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

Re: [edk2] [PATCH v2 0/2] Resolve feature conflict between NX and Stack guard

[Wang, Jian J]

Regards,
Jian


> -Original Message-
> From: Ni, Ruiyu
> Sent: Wednesday, March 07, 2018 11:17 AM
> To: Wu, Hao A ; edk2-devel@lists.01.org
> Cc: Wang, Jian J ; Zeng, Star ;
> Dong, Eric ; Yao, Jiewen 
> Subject: Re: [PATCH v2 0/2] Resolve feature conflict between NX and Stack
> guard
> 
> On 3/6/2018 9:33 PM, Hao Wu wrote:
> > V2 changes:
> >
> > A. Use Hoblib APIs to get the base of stack from Hob.
> > B. Remove unnecessary local variable used in function
> > InitializeDxeNxMemoryProtectionPolicy().
> >
> > V1 history:
> >
> > If enabled, NX memory protection feature will mark some types of active
> > memory as NX (non-executable), which includes the first page of the stack.
> > This will overwrite the attributes of the first page of the stack if the
> > stack guard feature is also enabled.
> >
> > The series will override the attributes setting to the first page of the
> > stack by adding back the 'EFI_MEMORY_RP' attribute when the stack guard
> > feature is enabled.
> >
> > Cc: Jian J Wang 
> > Cc: Star Zeng 
> > Cc: Eric Dong 
> > Cc: Jiewen Yao 
> > Cc: Ruiyu Ni 
> >
> > Hao Wu (2):
> >MdeModulePkg/Core: Refine handling NULL detection in NX setting
> >MdeModulePkg/Core: Fix feature conflict between NX and Stack guard
> >
> >   MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
> >   MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 74
> +++
> >   2 files changed, 67 insertions(+), 11 deletions(-)
> >
> 
>if (MemoryMapEntry->PhysicalStart == 0 &&
>PcdGet8 (PcdNullPointerDetectionPropertyMask) != 0) {
> 
>  ASSERT (MemoryMapEntry->NumberOfPages > 0);
>  //
>  // Add EFI_MEMORY_RP attribute for page 0 if NULL pointer
> detection is
>  // enabled.
>  //
> [Ray] 1. I prefer to move the above comments before the "if (...)".
> 
>  SetUefiImageMemoryAttributes (
>0,
>EFI_PAGES_TO_SIZE (1),
>EFI_MEMORY_RP | Attributes);
>}
> 
>if (StackBase != 0 &&
>(StackBase >= MemoryMapEntry->PhysicalStart &&
> StackBase <  MemoryMapEntry->PhysicalStart +
>  LShiftU64 (MemoryMapEntry->NumberOfPages,
> EFI_PAGE_SHIFT)) &&
>PcdGetBool (PcdCpuStackGuard)) {
> 
>  //
>  // Add EFI_MEMORY_RP attribute for the first page of the stack
> if stack
>  // guard is enabled.
>  //
>  SetUefiImageMemoryAttributes (
>StackBase,
>EFI_PAGES_TO_SIZE (1),
>EFI_MEMORY_RP | Attributes);
> [Ray] 2. The StackBase is directly used here. So do we need to check
> whether it's page aligned? Do we need to check whether the range
> [StackBase, StackBase + 4KB) is inside the MemoryMapEntry?
>}

If PcdCpuStackGuard is TRUE, I think the owner who allocates memory for 
StackBase
should make sure all the conditions you mentioned, but not here.

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

Re: [edk2] [PATCH v2 0/2] Resolve feature conflict between NX and Stack guard

[Ni, Ruiyu]

On 3/6/2018 9:33 PM, Hao Wu wrote:

V2 changes:

A. Use Hoblib APIs to get the base of stack from Hob.
B. Remove unnecessary local variable used in function
InitializeDxeNxMemoryProtectionPolicy().

V1 history:

If enabled, NX memory protection feature will mark some types of active
memory as NX (non-executable), which includes the first page of the stack.
This will overwrite the attributes of the first page of the stack if the
stack guard feature is also enabled.

The series will override the attributes setting to the first page of the
stack by adding back the 'EFI_MEMORY_RP' attribute when the stack guard
feature is enabled.

Cc: Jian J Wang 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 

Hao Wu (2):
   MdeModulePkg/Core: Refine handling NULL detection in NX setting
   MdeModulePkg/Core: Fix feature conflict between NX and Stack guard

  MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 74 +++
  2 files changed, 67 insertions(+), 11 deletions(-)



  if (MemoryMapEntry->PhysicalStart == 0 &&
  PcdGet8 (PcdNullPointerDetectionPropertyMask) != 0) {

ASSERT (MemoryMapEntry->NumberOfPages > 0);
//
// Add EFI_MEMORY_RP attribute for page 0 if NULL pointer 
detection is

// enabled.
//
[Ray] 1. I prefer to move the above comments before the "if (...)".

SetUefiImageMemoryAttributes (
  0,
  EFI_PAGES_TO_SIZE (1),
  EFI_MEMORY_RP | Attributes);
  }

  if (StackBase != 0 &&
  (StackBase >= MemoryMapEntry->PhysicalStart &&
   StackBase <  MemoryMapEntry->PhysicalStart +
LShiftU64 (MemoryMapEntry->NumberOfPages, 
EFI_PAGE_SHIFT)) &&

  PcdGetBool (PcdCpuStackGuard)) {

//
// Add EFI_MEMORY_RP attribute for the first page of the stack 
if stack

// guard is enabled.
//
SetUefiImageMemoryAttributes (
  StackBase,
  EFI_PAGES_TO_SIZE (1),
  EFI_MEMORY_RP | Attributes);
[Ray] 2. The StackBase is directly used here. So do we need to check
whether it's page aligned? Do we need to check whether the range
[StackBase, StackBase + 4KB) is inside the MemoryMapEntry?
  }

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

[edk2] [patch] MdeModulePkg/DriverSampleDxe: Refine the sample case for UNION type

[Dandan Bi]

The example of UNION storage is not good, now update it.

Cc: Eric Dong 
Cc: Liming Gao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h  |  6 +++---
 MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr| 14 ++
 MdeModulePkg/Universal/DriverSampleDxe/VfrStrings.uni |  6 +-
 3 files changed, 6 insertions(+), 20 deletions(-)

diff --git a/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h 
b/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
index 6f092de..208a4c6 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
+++ b/MdeModulePkg/Universal/DriverSampleDxe/NVDataStruc.h
@@ -1,8 +1,8 @@
 /** @file
 
-Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2007 - 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
 which accompanies this distribution.  The full text of the license may be 
found at
 http://opensource.org/licenses/bsd-license.php
 
@@ -51,12 +51,12 @@ typedef struct {
   UINT8: 0;  // Special width 0 can be used to force 
alignment at the next word boundary
   UINT8NestBitNumeric  : 4;
 } MY_BITS_DATA;
 
 typedef union {
-  UINT16   BitField : 10;
-  UINT8ByteField;
+  UINT8UnionNumeric;
+  UINT8UnionNumericAlias;
 } MY_EFI_UNION_DATA;
 
 typedef struct {
   UINT16  MyStringData[40];
   UINT16  SomethingHiddenForHtml;
diff --git a/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr 
b/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr
index b1017d9..9d99dcf 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr
+++ b/MdeModulePkg/Universal/DriverSampleDxe/Vfr.vfr
@@ -1,10 +1,10 @@
 ///** @file
 //
 //Sample Setup formset.
 //
-//  Copyright (c) 2004 - 2017, 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
 //  which accompanies this distribution.  The full text of the license may be 
found at
 //  http://opensource.org/licenses/bsd-license.php
 //
@@ -913,30 +913,20 @@ formset
 endnumeric;
 
 subtitle text = STRING_TOKEN(STR_SUBTITLE_TEXT2);
 subtitle text = STRING_TOKEN(STR_UNION_EFI_VARSTORE);
 
-numeric varid   = MyEfiUnionVar.ByteField,
+numeric varid   = MyEfiUnionVar.UnionNumeric,
 prompt  = STRING_TOKEN(STR_UNION_BYTE_NUMERIC_PROMPT),
 help= STRING_TOKEN(STR_UNION_BYTE_NUMERIC_HELP),
 minimum = 0,
 maximum = 20,
 step= 0,
 default = 7, defaultstore = MyStandardDefault,
 default = 8, defaultstore = MyManufactureDefault,
 endnumeric;
 
-numeric varid   = MyEfiUnionVar.BitField,
-prompt  = STRING_TOKEN(STR_UNION_BIT_NUMERIC_PROMPT),
-help= STRING_TOKEN(STR_UNION_BIT_NUMERIC_HELP),
-minimum = 0,
-maximum = 20,
-step= 0,
-default = 7, defaultstore = MyStandardDefault,
-default = 8, defaultstore = MyManufactureDefault,
-endnumeric;
-
 guidop
   guid = DRIVER_SAMPLE_FORMSET_GUID,
   datatype = MY_EFI_BITS_VARSTORE_DATA,
 data.EfiBitNumeric  = 1,
 data.EfiBitOneof = 1,
diff --git a/MdeModulePkg/Universal/DriverSampleDxe/VfrStrings.uni 
b/MdeModulePkg/Universal/DriverSampleDxe/VfrStrings.uni
index 7cc6a19..2215c08 100644
--- a/MdeModulePkg/Universal/DriverSampleDxe/VfrStrings.uni
+++ b/MdeModulePkg/Universal/DriverSampleDxe/VfrStrings.uni
@@ -1,8 +1,8 @@
 // *++
//
-// Copyright (c) 2007 - 2017, Intel Corporation. All rights reserved.
+// Copyright (c) 2007 - 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 
 // which accompanies this distribution.  The full text of the license may be 
found at
 // http://opensource.org/licenses/bsd-license.php  
  
 // 
  
@@ -330,14 +330,10 @@
#language fr-FR "The question refer to 
byte field in BIT structure"
 #string STR_UNION_BYTE_NUMERIC_PROMPT  #language en-US "UNION EfiVarStore byte 
numeric"
#language fr-FR "UNION EfiVarStore byte 
numeric"
 #string STR_UNION_BYTE_NUMERIC_HELP#language en-US "Question refer to byte 
field in UNION type efivastore, the Standard default is 7 Manufacture default 
is 8"
#language fr-FR 

[edk2] [PATCH 1/2] Hisilicon/D0x: fix tftp command init failure

[Heyi Guo]

We need to set PcdShellLibAutoInitialize to FALSE for
TftpDynamicCommand, or else we will get initialization failure when
loading TftpDynamicCommand module, for EFI Shell has not been started
at this moment.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
---
 Platform/Hisilicon/D03/D03.dsc | 5 -
 Platform/Hisilicon/D05/D05.dsc | 5 -
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
index c4963063794b..cb0669d639d1 100644
--- a/Platform/Hisilicon/D03/D03.dsc
+++ b/Platform/Hisilicon/D03/D03.dsc
@@ -517,5 +517,8 @@ [Components.common]
   gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
   }
 !ifdef $(INCLUDE_TFTP_COMMAND)
-  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
+  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
+
+  gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
+  }
 !endif #$(INCLUDE_TFTP_COMMAND)
diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
index 0792b0814ea1..8373a821a496 100644
--- a/Platform/Hisilicon/D05/D05.dsc
+++ b/Platform/Hisilicon/D05/D05.dsc
@@ -675,5 +675,8 @@ [Components.common]
   gEfiMdePkgTokenSpaceGuid.PcdUefiLibMaxPrintBufferSize|8000
   }
 !ifdef $(INCLUDE_TFTP_COMMAND)
-  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf
+  ShellPkg/DynamicCommand/TftpDynamicCommand/TftpDynamicCommand.inf {
+
+  gEfiShellPkgTokenSpaceGuid.PcdShellLibAutoInitialize|FALSE
+  }
 !endif #$(INCLUDE_TFTP_COMMAND)
-- 
2.7.4

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

[edk2] [PATCH 2/2] Hisilicon/D0x: Enable tftp command by default

[Heyi Guo]

Since D0x platforms always have network enabled, we would like to
enable tftp command by default so that we can download something in
EFI Shell.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Heyi Guo 
Cc: Ard Biesheuvel 
Cc: Leif Lindholm 
---
 Platform/Hisilicon/D03/D03.dsc | 2 ++
 Platform/Hisilicon/D05/D05.dsc | 1 +
 2 files changed, 3 insertions(+)

diff --git a/Platform/Hisilicon/D03/D03.dsc b/Platform/Hisilicon/D03/D03.dsc
index cb0669d639d1..fce1e60b1275 100644
--- a/Platform/Hisilicon/D03/D03.dsc
+++ b/Platform/Hisilicon/D03/D03.dsc
@@ -29,6 +29,8 @@ [Defines]
   SKUID_IDENTIFIER   = DEFAULT
   FLASH_DEFINITION   = 
Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
 
+  DEFINE INCLUDE_TFTP_COMMAND= TRUE
+
 !include Silicon/Hisilicon/Hisilicon.dsc.inc
 
 [LibraryClasses.common]
diff --git a/Platform/Hisilicon/D05/D05.dsc b/Platform/Hisilicon/D05/D05.dsc
index 8373a821a496..f007f3d2d7e8 100644
--- a/Platform/Hisilicon/D05/D05.dsc
+++ b/Platform/Hisilicon/D05/D05.dsc
@@ -29,6 +29,7 @@ [Defines]
   SKUID_IDENTIFIER   = DEFAULT
   FLASH_DEFINITION   = 
Platform/Hisilicon/$(PLATFORM_NAME)/$(PLATFORM_NAME).fdf
   DEFINE EDK2_SKIP_PEICORE=0
+  DEFINE INCLUDE_TFTP_COMMAND= TRUE
   DEFINE NETWORK_IP6_ENABLE  = FALSE
   DEFINE HTTP_BOOT_ENABLE= FALSE
 
-- 
2.7.4

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

Re: [edk2] [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface informations

[Zhang, Chao B]

Reviewed-by: Chao Zhang 

-Original Message-
From: marcandre.lur...@redhat.com [mailto:marcandre.lur...@redhat.com] 
Sent: Wednesday, March 7, 2018 4:27 AM
To: edk2-devel@lists.01.org
Cc: Marc-André Lureau ; Yao, Jiewen 
; Zhang, Chao B ; Zeng, Star 
; Laszlo Ersek 
Subject: [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface informations

From: Marc-André Lureau 

The ZeroMem() call goes beyond the HashInterfaceHob structure, causing HOB list 
corruption. Instead, just clear the HashInterface fields, as I suppose was 
originally intended.

Cc: Jiewen Yao 
Cc: Chao Zhang 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau 
---
 .../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git 
a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
index dbee0f2531bc..361a4f6508a0 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRoute
+++ rPei.c
@@ -424,7 +424,8 @@ HashLibBaseCryptoRouterPeiConstructor (
 // This is the second execution of this module, clear the hash interface
 // information registered at its first execution.
 //
-ZeroMem (>HashInterface, sizeof (*HashInterfaceHob) - 
sizeof (EFI_GUID));
+ZeroMem (>HashInterface, sizeof 
(HashInterfaceHob->HashInterface));
+HashInterfaceHob->HashInterfaceCount = 0;
   }
 
   //
--
2.16.2.346.g9779355e34

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

Re: [edk2] [PATCH 1/1] MdeModulePkg/UefiBootManagerLib: limit recursive call depth

[Guo Heyi]

Hi Ray,

Sorry to disturb, but I didn't find the patch committed. Could you help to do
that?

Thanks,
Heyi


On Thu, Mar 01, 2018 at 12:46:32PM +0800, Ni, Ruiyu wrote:
> On 3/1/2018 10:39 AM, Heyi Guo wrote:
> >Function BmRepairAllControllers may recursively call itself if some
> >driver health protocol returns EfiDriverHealthStatusReconnectRequired.
> >However, driver health protocol of some buggy third party driver may
> >always return such status even after one and another reconnect. The
> >endless iteration will cause stack overflow and then system exception,
> >and it may be not easy to find that the exception is actually caused
> >by stack overflow.
> >
> >So we limit the number of reconnect retry to 10 to improve code
> >robustness, and DEBUG_CODE is moved ahead before recursive repair to
> >track the repair result.
> >
> >We also remove a duplicated declaration of BmRepairAllControllers() in
> >InternalBm.h in this patch, for it is only a trivial change.
> >
> >Contributed-under: TianoCore Contribution Agreement 1.1
> >Signed-off-by: Heyi Guo 
> >Cc: Star Zeng 
> >Cc: Eric Dong 
> >Cc: Ruiyu Ni 
> >Cc: Laszlo Ersek 
> >---
> >
> >Notes:
> > v2
> > - Use argument instead of global variable to record the recursive
> >   count [Ray]
> > - Move DEBUG_CODE before recursively calling BmRepairAllControllers()
> >   to track the change of each reconnect [Ray]
> > - Remove a duplicated declaration of BmRepairAllControllers() in
> >   InternalBm.h.
> >
> >  MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h | 19 
> > ++-
> >  MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c |  2 +-
> >  MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c | 18 
> > +-
> >  3 files changed, 24 insertions(+), 15 deletions(-)
> >
> >diff --git a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h 
> >b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> >index 25a1d522fe84..21ecd8584d24 100644
> >--- a/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> >+++ b/MdeModulePkg/Library/UefiBootManagerLib/InternalBm.h
> >@@ -108,6 +108,12 @@ CHAR16 *
> >  #define BM_OPTION_NAME_LEN  sizeof 
> > ("PlatformRecovery")
> >  extern CHAR16  *mBmLoadOptionName[];
> >+//
> >+// Maximum number of reconnect retry to repair controller; it is to limit 
> >the
> >+// number of recursive call of BmRepairAllControllers.
> >+//
> >+#define MAX_RECONNECT_REPAIR10
> >+
> >  /**
> >Visitor function to be called by BmForEachVariable for each variable
> >in variable storage.
> >@@ -145,10 +151,13 @@ typedef struct {
> >  /**
> >Repair all the controllers according to the Driver Health status queried.
> >+
> >+  @param ReconnectRepairCount To record the number of recursive call of
> >+  this function itself.
> >  **/
> >  VOID
> >  BmRepairAllControllers (
> >-  VOID
> >+  UINTN   ReconnectRepairCount
> >);
> >  #define BM_HOTKEY_SIGNATURE SIGNATURE_32 ('b', 'm', 'h', 'k')
> >@@ -328,14 +337,6 @@ BmDelPartMatchInstance (
> >);
> >  /**
> >-  Repair all the controllers according to the Driver Health status queried.
> >-**/
> >-VOID
> >-BmRepairAllControllers (
> >-  VOID
> >-  );
> >-
> >-/**
> >Print the device path info.
> >@param DevicePath   The device path need to print.
> >diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c 
> >b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> >index ce19ae400660..b842d5824aed 100644
> >--- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> >+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c
> >@@ -1767,7 +1767,7 @@ EfiBootManagerBoot (
> >  //
> >  // 4. Repair system through DriverHealth protocol
> >  //
> >-BmRepairAllControllers ();
> >+BmRepairAllControllers (0);
> >}
> >PERF_START_EX (gImageHandle, "BdsAttempt", NULL, 0, (UINT32) 
> > OptionNumber);
> >diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c 
> >b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> >index ddcee8b0676f..db2f859ae73d 100644
> >--- a/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> >+++ b/MdeModulePkg/Library/UefiBootManagerLib/BmDriverHealth.c
> >@@ -423,10 +423,13 @@ EfiBootManagerFreeDriverHealthInfo (
> >  /**
> >Repair all the controllers according to the Driver Health status queried.
> >+
> >+  @param ReconnectRepairCount To record the number of recursive call of
> >+  this function itself.
> >  **/
> >  VOID
> >  BmRepairAllControllers (
> >-  VOID
> >+  UINTN   ReconnectRepairCount
> >)
> >  {
> >EFI_STATUS  Status;
> >@@ -548,10 +551,6 @@ BmRepairAllControllers (
> >EfiBootManagerFreeDriverHealthInfo (DriverHealthInfo, Count);
> >-  if (ReconnectRequired) {

Re: [edk2] [PATCH v2 0/2] Resolve feature conflict between NX and Stack guard

[Wang, Jian J]

Thanks for fixing the issue.

For this patch series:

Reviewed-by: Jian J Wang 

> -Original Message-
> From: Wu, Hao A
> Sent: Tuesday, March 06, 2018 9:33 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Wang, Jian J ;
> Zeng, Star ; Dong, Eric ; Yao,
> Jiewen ; Ni, Ruiyu 
> Subject: [PATCH v2 0/2] Resolve feature conflict between NX and Stack guard
> 
> V2 changes:
> 
> A. Use Hoblib APIs to get the base of stack from Hob.
> B. Remove unnecessary local variable used in function
>InitializeDxeNxMemoryProtectionPolicy().
> 
> V1 history:
> 
> If enabled, NX memory protection feature will mark some types of active
> memory as NX (non-executable), which includes the first page of the stack.
> This will overwrite the attributes of the first page of the stack if the
> stack guard feature is also enabled.
> 
> The series will override the attributes setting to the first page of the
> stack by adding back the 'EFI_MEMORY_RP' attribute when the stack guard
> feature is enabled.
> 
> Cc: Jian J Wang 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Jiewen Yao 
> Cc: Ruiyu Ni 
> 
> Hao Wu (2):
>   MdeModulePkg/Core: Refine handling NULL detection in NX setting
>   MdeModulePkg/Core: Fix feature conflict between NX and Stack guard
> 
>  MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 74
> +++
>  2 files changed, 67 insertions(+), 11 deletions(-)
> 
> --
> 2.12.0.windows.1

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

[edk2] [PATCH 1/4] MdeModulePkg: Added new Virtio non-discoverable type and GUID

[Daniil Egranov]

Added Virtio type and GUID to the list of supported
non-discoverable devices.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Daniil Egranov 
---
 MdeModulePkg/Include/Guid/NonDiscoverableDevice.h   | 3 +++
 MdeModulePkg/Include/Library/NonDiscoverableDeviceRegistrationLib.h | 1 +
 MdeModulePkg/MdeModulePkg.dec   | 1 +
 3 files changed, 5 insertions(+)

diff --git a/MdeModulePkg/Include/Guid/NonDiscoverableDevice.h 
b/MdeModulePkg/Include/Guid/NonDiscoverableDevice.h
index d182e4b9d2..9d598d0487 100644
--- a/MdeModulePkg/Include/Guid/NonDiscoverableDevice.h
+++ b/MdeModulePkg/Include/Guid/NonDiscoverableDevice.h
@@ -44,6 +44,8 @@
 #define EDKII_NON_DISCOVERABLE_XHCI_DEVICE_GUID \
   { 0xB1BE0BC5, 0x6C28, 0x442D, {0xAA, 0x37, 0x15, 0x1B, 0x42, 0x57, 0xBD, 
0x78 } }
 
+#define EDKII_NON_DISCOVERABLE_VIRTIO_DEVICE_GUID \
+  { 0xA72A1646, 0x1A4D, 0x4A84, {0xB6, 0xAC, 0xEF, 0x18, 0x13, 0x62, 0xC5, 
0x85 } }
 
 extern EFI_GUID gEdkiiNonDiscoverableAhciDeviceGuid;
 extern EFI_GUID gEdkiiNonDiscoverableAmbaDeviceGuid;
@@ -54,5 +56,6 @@ extern EFI_GUID gEdkiiNonDiscoverableSdhciDeviceGuid;
 extern EFI_GUID gEdkiiNonDiscoverableUfsDeviceGuid;
 extern EFI_GUID gEdkiiNonDiscoverableUhciDeviceGuid;
 extern EFI_GUID gEdkiiNonDiscoverableXhciDeviceGuid;
+extern EFI_GUID gEdkiiNonDiscoverableVirtioDeviceGuid;
 
 #endif
diff --git 
a/MdeModulePkg/Include/Library/NonDiscoverableDeviceRegistrationLib.h 
b/MdeModulePkg/Include/Library/NonDiscoverableDeviceRegistrationLib.h
index c2d9e48353..a3d35af275 100644
--- a/MdeModulePkg/Include/Library/NonDiscoverableDeviceRegistrationLib.h
+++ b/MdeModulePkg/Include/Library/NonDiscoverableDeviceRegistrationLib.h
@@ -26,6 +26,7 @@ typedef enum {
   NonDiscoverableDeviceTypeUfs,
   NonDiscoverableDeviceTypeUhci,
   NonDiscoverableDeviceTypeXhci,
+  NonDiscoverableDeviceTypeVirtio,
   NonDiscoverableDeviceTypeMax,
 } NON_DISCOVERABLE_DEVICE_TYPE;
 
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 5d561ff484..cd18bbfbd2 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -396,6 +396,7 @@
   gEdkiiNonDiscoverableUfsDeviceGuid = { 0x2EA77912, 0x80A8, 0x4947, {0xBE, 
0x69, 0xCD, 0xD0, 0x0A, 0xFB, 0xE5, 0x56 } }
   gEdkiiNonDiscoverableUhciDeviceGuid = { 0xA8CDA0A2, 0x4F37, 0x4A1B, {0x8E, 
0x10, 0x8E, 0xF3, 0xCC, 0x3B, 0xF3, 0xA8 } }
   gEdkiiNonDiscoverableXhciDeviceGuid = { 0xB1BE0BC5, 0x6C28, 0x442D, {0xAA, 
0x37, 0x15, 0x1B, 0x42, 0x57, 0xBD, 0x78 } }
+  gEdkiiNonDiscoverableVirtioDeviceGuid = { 0xA72A1646, 0x1A4D, 0x4A84, {0xB6, 
0xAC, 0xEF, 0x18, 0x13, 0x62, 0xC5, 0x85 } }
 
   ## Include/Guid/PlatformHasAcpi.h
   gEdkiiPlatformHasAcpiGuid = { 0xf0966b41, 0xc23f, 0x41b9, { 0x96, 0x04, 
0x0f, 0xf7, 0xe1, 0x11, 0x96, 0x5a } }
-- 
2.11.0

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

[edk2] [PATCH 2/4] NonDiscoverableDeviceRegistrationLib: Added Virtio support

[Daniil Egranov]

Added Virtio non-discoverable device case.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Daniil Egranov 
---
 .../NonDiscoverableDeviceRegistrationLib.c | 3 +++
 .../NonDiscoverableDeviceRegistrationLib.inf   | 1 +
 2 files changed, 4 insertions(+)

diff --git 
a/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.c
 
b/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.c
index 536dfc7297..2cc3da7772 100644
--- 
a/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.c
+++ 
b/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.c
@@ -67,6 +67,9 @@ GetGuidFromType (
   case NonDiscoverableDeviceTypeXhci:
 return 
 
+  case NonDiscoverableDeviceTypeVirtio:
+return 
+
   default:
 return NULL;
   }
diff --git 
a/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
 
b/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
index 763f9a0a6f..8ebf869d1d 100644
--- 
a/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
+++ 
b/MdeModulePkg/Library/NonDiscoverableDeviceRegistrationLib/NonDiscoverableDeviceRegistrationLib.inf
@@ -46,3 +46,4 @@
   gEdkiiNonDiscoverableUfsDeviceGuid ## CONSUMES   ## GUID
   gEdkiiNonDiscoverableUhciDeviceGuid## CONSUMES   ## GUID
   gEdkiiNonDiscoverableXhciDeviceGuid## CONSUMES   ## GUID
+  gEdkiiNonDiscoverableVirtioDeviceGuid  ## CONSUMES   ## GUID
-- 
2.11.0

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

[edk2] [PATCH 0/4] Virtio non-discoverable devices

[Daniil Egranov]

This is an attempt to add MMIO Virtio devices into the
non-discoverable device registration procedure and allow
Virtio PCI drivers to recognize and program such devices 
correctly.
The main issue is that the set of MMIO registers is different
from PCI, plus the width of similar registers are not 
always the same. The code implements the translation of 
the PCI IO registers to MMIO registers. 
Another difference between PCI and MMIO Virtio devices found 
during the testing is that MMIO devices may require more 
registers to be programmed compared to PCI. The VirtioPciDeviceDxe
was patched to detect non-discoverable MMIO devices and allow
calling a PCI MemIo protocol function.

This set of patches was tested with MMIO Virtio Block and
Virtio Net devices.

Daniil Egranov (4):
  MdeModulePkg: Added new Virtio non-discoverable type and GUID
  NonDiscoverableDeviceRegistrationLib: Added Virtio support
  NonDiscoverablePciDeviceDxe: Added MMIO Virtio support
  VirtioPciDeviceDxe: Added non-discoverable Virtio support

 .../NonDiscoverablePciDeviceDxe.c  |   3 +-
 .../NonDiscoverablePciDeviceDxe.inf|   5 +-
 .../NonDiscoverablePciDeviceIo.c   | 240 -
 MdeModulePkg/Include/Guid/NonDiscoverableDevice.h  |   3 +
 .../Library/NonDiscoverableDeviceRegistrationLib.h |   1 +
 .../NonDiscoverableDeviceRegistrationLib.c |   3 +
 .../NonDiscoverableDeviceRegistrationLib.inf   |   1 +
 MdeModulePkg/MdeModulePkg.dec  |   1 +
 OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c   | 143 +++-
 OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h   |  21 +-
 OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf  |   4 +-
 OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c| 117 +-
 12 files changed, 528 insertions(+), 14 deletions(-)

-- 
2.11.0

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

[edk2] [PATCH 4/4] VirtioPciDeviceDxe: Added non-discoverable Virtio support

[Daniil Egranov]

The VirtioPciDeviceDxe was extended to support a non-discoverable
MMIO Virtio case.
The Virtio spec defines both PCI and MMIO device types with the 
set of registers that are not the same between these two types
of devices. All PCI registers have corresponding MMIO registers
but the number of registers is less then MMIO. The width of MMIO
registers and PCI registers is not always the same.
Compared to PCI, MMIO Virtio devices required more registers to be
programmed in some cases. Added detection that a PCI device is 
based on a non-discoverable device and allows MMIO transactions.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Daniil Egranov 
---
 OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c  | 143 +-
 OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h  |  21 +++-
 OvmfPkg/VirtioPciDeviceDxe/VirtioPciDeviceDxe.inf |   4 +-
 OvmfPkg/VirtioPciDeviceDxe/VirtioPciFunctions.c   | 117 +-
 4 files changed, 278 insertions(+), 7 deletions(-)

diff --git a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c 
b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c
index d4b4ec21c3..df2a4ea116 100644
--- a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c
+++ b/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.c
@@ -4,7 +4,7 @@
 
   Copyright (C) 2012, Red Hat, Inc.
   Copyright (c) 2012 - 2016, Intel Corporation. All rights reserved.
-  Copyright (C) 2013, ARM Ltd.
+  Copyright (C) 2013 - 2018, ARM Ltd.
   Copyright (C) 2017, AMD Inc, All rights reserved.
 
   This program and the accompanying materials are licensed and made available
@@ -215,6 +215,147 @@ VirtioPciIoWrite (
 
 /**
 
+  Read a word from Region 0 of the device specified by PciMemIo.
+
+  Region 0 must be an iomem region. This is an internal function for the PCI
+  implementation of the protocol.
+
+  @param[in] Dev  Virtio PCI device.
+
+  @param[in] FieldOffset  Source offset.
+
+  @param[in] FieldSizeSource field size, must be in { 1, 2, 4, 8 }.
+
+  @param[in] BufferSize   Number of bytes available in the target buffer. Must
+  equal FieldSize.
+
+  @param[out] Buffer  Target buffer.
+
+
+  @return  Status code returned by PciIo->Io.Read().
+
+**/
+EFI_STATUS
+EFIAPI
+VirtioPciMemIoRead (
+  IN  VIRTIO_PCI_DEVICE *Dev,
+  IN  UINTN FieldOffset,
+  IN  UINTN FieldSize,
+  IN  UINTN BufferSize,
+  OUT VOID  *Buffer
+  )
+{
+  UINTN Count;
+  EFI_PCI_IO_PROTOCOL_WIDTH Width;
+  EFI_PCI_IO_PROTOCOL   *PciIo;
+
+  ASSERT (FieldSize == BufferSize);
+
+  PciIo = Dev->PciIo;
+  Count = 1;
+
+  switch (FieldSize) {
+  case 1:
+Width = EfiPciIoWidthUint8;
+break;
+
+  case 2:
+Width = EfiPciIoWidthUint16;
+break;
+
+  case 4:
+Width = EfiPciIoWidthUint32;
+break;
+
+  case 8:
+Width = EfiPciIoWidthUint64;
+break;
+
+  default:
+ASSERT (FALSE);
+return EFI_INVALID_PARAMETER;
+  }
+
+  return PciIo->Mem.Read (
+ PciIo,
+ Width,
+ PCI_BAR_IDX0,
+ FieldOffset,
+ Count,
+ Buffer
+ );
+}
+
+/**
+
+  Write a word into Region 0 of the device specified by PciMemIo.
+
+  Region 0 must be an iomem region. This is an internal function for the PCI
+  implementation of the protocol.
+
+  @param[in] Dev  Virtio PCI device.
+
+  @param[in] FieldOffset  Destination offset.
+
+  @param[in] FieldSizeDestination field size, must be in { 1, 2, 4, 8 }.
+
+  @param[in] ValueLittle endian value to write, converted to UINT64.
+  The least significant FieldSize bytes will be used.
+
+
+  @return  Status code returned by PciIo->Io.Write().
+
+**/
+EFI_STATUS
+EFIAPI
+VirtioPciMemIoWrite (
+  IN  VIRTIO_PCI_DEVICE *Dev,
+  IN UINTN  FieldOffset,
+  IN UINTN  FieldSize,
+  IN UINT64 Value
+  )
+{
+  UINTN Count;
+  EFI_PCI_IO_PROTOCOL_WIDTH Width;
+  EFI_PCI_IO_PROTOCOL   *PciIo;
+
+  PciIo = Dev->PciIo;
+  Count = 1;
+
+  switch (FieldSize) {
+  case 1:
+Width = EfiPciIoWidthUint8;
+break;
+
+  case 2:
+Width = EfiPciIoWidthUint16;
+break;
+
+  case 4:
+Width = EfiPciIoWidthUint32;
+break;
+
+  case 8:
+Width = EfiPciIoWidthUint64;
+break;
+
+  default:
+ASSERT (FALSE);
+return EFI_INVALID_PARAMETER;
+  }
+
+  return PciIo->Mem.Write (
+ PciIo,
+ Width,
+ PCI_BAR_IDX0,
+ FieldOffset,
+ Count,
+ 
+ );
+}
+
+/**
+
   Device probe function for this driver.
 
   The DXE core calls this function for any given device in order to see if the
diff --git a/OvmfPkg/VirtioPciDeviceDxe/VirtioPciDevice.h 

[edk2] [PATCH 3/4] NonDiscoverablePciDeviceDxe: Added MMIO Virtio support

[Daniil Egranov]

Added PCI IO to MMIO translation for Virtio case into the PCI IO 
protocol functions.
Added Virtio device type into the PCI IO protocol initialization
procedure.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Daniil Egranov 
---
 .../NonDiscoverablePciDeviceDxe.c  |   3 +-
 .../NonDiscoverablePciDeviceDxe.inf|   5 +-
 .../NonDiscoverablePciDeviceIo.c   | 240 -
 3 files changed, 241 insertions(+), 7 deletions(-)

diff --git 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
index 3e9ff6620d..2c3eeca914 100644
--- 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
+++ 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.c
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (C) 2016, Linaro Ltd. All rights reserved.
+  Copyright (C) 2016-2018, Linaro Ltd. All rights reserved.
 
   This program and the accompanying materials are licensed and made available
   under the terms and conditions of the BSD License which accompanies this
@@ -32,6 +32,7 @@ SupportedNonDiscoverableDevices[] = {
   ,
   ,
   ,
+  
 };
 
 //
diff --git 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
index ac551a82ab..a7bf5a5fc1 100644
--- 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
+++ 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceDxe.inf
@@ -1,7 +1,7 @@
 ## @file
 # PCI I/O driver for non-discoverable devices.
 #
-# Copyright (C) 2016, Linaro Ltd.
+# Copyright (C) 2016-2018, Linaro Ltd.
 #
 # This program and the accompanying materials are licensed and made available
 # under the terms and conditions of the BSD License which accompanies this
@@ -30,6 +30,7 @@
 [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
+  OvmfPkg/OvmfPkg.dec
 
 [LibraryClasses]
   BaseMemoryLib
@@ -54,3 +55,5 @@
   gEdkiiNonDiscoverableUfsDeviceGuid## CONSUMES ## GUID
   gEdkiiNonDiscoverableUhciDeviceGuid   ## CONSUMES ## GUID
   gEdkiiNonDiscoverableXhciDeviceGuid   ## CONSUMES ## GUID
+  gEdkiiNonDiscoverableVirtioDeviceGuid ## CONSUMES ## GUID
+
diff --git 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
index 0e42ae4bf6..cb99bf0ba6 100644
--- 
a/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
+++ 
b/MdeModulePkg/Bus/Pci/NonDiscoverablePciDeviceDxe/NonDiscoverablePciDeviceIo.c
@@ -1,7 +1,7 @@
 /** @file
 
   Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.
-  Copyright (c) 2016, Linaro, Ltd. All rights reserved.
+  Copyright (c) 2016-2018, Linaro, Ltd. All rights reserved.
 
   This program and the accompanying materials
   are licensed and made available under the terms and conditions of the BSD 
License
@@ -17,8 +17,12 @@
 
 #include 
 
+#include 
+
 #include 
 
+#include 
+
 #include 
 
 typedef struct {
@@ -374,6 +378,82 @@ PciIoMemWrite (
 }
 
 /**
+  Translate PCI IO to MMIO for VirtIo Non-Discoverable devices.
+
+  @param  BaseAddress   A base MMIO memory address of a VirtIo device.
+  @param  PciIoOffset   PCI IO offset.
+  @param  DevConfigAddress  The address is a device specific config space.
+
+  @retval EFI_SUCCESS   The address translated successfully.
+  @retval EFI_UNSUPPORTED   The PCI IO address can not be translated.
+
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+VirtioPciIoToMemIoTranslation (
+  IN UINTNBaseAddress,
+  IN UINTNPciIoOffset,
+  IN OUT UINTN*Address,
+  IN OUT BOOLEAN  *DevConfigAddress
+  )
+{
+  EFI_STATUS  Status;
+  UINTN   Offset;
+  INTNDevConfigSpaceOffset;
+
+  Status   = EFI_SUCCESS;
+  Offset   = PciIoOffset;
+  *DevConfigAddress= FALSE;
+  DevConfigSpaceOffset = 0;
+
+  //
+  //Check if it's device specific config space
+  //
+  if (PciIoOffset >= VIRTIO_DEVICE_SPECIFIC_CONFIGURATION_OFFSET_PCI ) {
+Offset = VIRTIO_DEVICE_SPECIFIC_CONFIGURATION_OFFSET_PCI;
+DevConfigSpaceOffset = PciIoOffset - 
VIRTIO_DEVICE_SPECIFIC_CONFIGURATION_OFFSET_PCI;
+*DevConfigAddress = TRUE;
+  }
+
+  switch (Offset) {
+  case VIRTIO_PCI_OFFSET_DEVICE_FEATURES:
+*Address = BaseAddress + VIRTIO_MMIO_OFFSET_HOST_FEATURES;
+break;
+  case VIRTIO_PCI_OFFSET_GUEST_FEATURES:
+*Address = BaseAddress + VIRTIO_MMIO_OFFSET_GUEST_FEATURES;
+break;
+  case VIRTIO_PCI_OFFSET_QUEUE_ADDRESS:
+*Address = BaseAddress + VIRTIO_MMIO_OFFSET_QUEUE_PFN;
+break;
+  case VIRTIO_PCI_OFFSET_QUEUE_SIZE:
+*Address = 

Re: [edk2] [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface informations

[Zeng, Star]

Reviewed-by: Star Zeng 

Thanks,
Star
-Original Message-
From: marcandre.lur...@redhat.com [mailto:marcandre.lur...@redhat.com] 
Sent: Wednesday, March 7, 2018 4:27 AM
To: edk2-devel@lists.01.org
Cc: Marc-André Lureau ; Yao, Jiewen 
; Zhang, Chao B ; Zeng, Star 
; Laszlo Ersek 
Subject: [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface informations

From: Marc-André Lureau 

The ZeroMem() call goes beyond the HashInterfaceHob structure, causing HOB list 
corruption. Instead, just clear the HashInterface fields, as I suppose was 
originally intended.

Cc: Jiewen Yao 
Cc: Chao Zhang 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau 
---
 .../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git 
a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
index dbee0f2531bc..361a4f6508a0 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRoute
+++ rPei.c
@@ -424,7 +424,8 @@ HashLibBaseCryptoRouterPeiConstructor (
 // This is the second execution of this module, clear the hash interface
 // information registered at its first execution.
 //
-ZeroMem (>HashInterface, sizeof (*HashInterfaceHob) - 
sizeof (EFI_GUID));
+ZeroMem (>HashInterface, sizeof 
(HashInterfaceHob->HashInterface));
+HashInterfaceHob->HashInterfaceCount = 0;
   }
 
   //
--
2.16.2.346.g9779355e34

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

Re: [edk2] [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface informations

[Yao, Jiewen]

Excellent. Thanks to catch that.

Reviewed-by: jiewen@intel.com



> -Original Message-
> From: marcandre.lur...@redhat.com [mailto:marcandre.lur...@redhat.com]
> Sent: Wednesday, March 7, 2018 4:27 AM
> To: edk2-devel@lists.01.org
> Cc: Marc-André Lureau ; Yao, Jiewen
> ; Zhang, Chao B ; Zeng, Star
> ; Laszlo Ersek 
> Subject: [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface informations
> 
> From: Marc-André Lureau 
> 
> The ZeroMem() call goes beyond the HashInterfaceHob structure, causing
> HOB list corruption. Instead, just clear the HashInterface fields, as
> I suppose was originally intended.
> 
> Cc: Jiewen Yao 
> Cc: Chao Zhang 
> Cc: Star Zeng 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc-André Lureau 
> ---
>  .../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c   | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> index dbee0f2531bc..361a4f6508a0 100644
> ---
> a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> +++
> b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
> @@ -424,7 +424,8 @@ HashLibBaseCryptoRouterPeiConstructor (
>  // This is the second execution of this module, clear the hash interface
>  // information registered at its first execution.
>  //
> -ZeroMem (>HashInterface, sizeof (*HashInterfaceHob)
> - sizeof (EFI_GUID));
> +ZeroMem (>HashInterface, sizeof
> (HashInterfaceHob->HashInterface));
> +HashInterfaceHob->HashInterfaceCount = 0;
>}
> 
>//
> --
> 2.16.2.346.g9779355e34

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

[edk2] [PATCH 1/1] RFC: SecurityPkg: only clear HashInterface informations

[marcandre . lureau]

From: Marc-André Lureau 

The ZeroMem() call goes beyond the HashInterfaceHob structure, causing
HOB list corruption. Instead, just clear the HashInterface fields, as
I suppose was originally intended.

Cc: Jiewen Yao 
Cc: Chao Zhang 
Cc: Star Zeng 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc-André Lureau 
---
 .../HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c   | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git 
a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c 
b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
index dbee0f2531bc..361a4f6508a0 100644
--- a/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
+++ b/SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.c
@@ -424,7 +424,8 @@ HashLibBaseCryptoRouterPeiConstructor (
 // This is the second execution of this module, clear the hash interface
 // information registered at its first execution.
 //
-ZeroMem (>HashInterface, sizeof (*HashInterfaceHob) - 
sizeof (EFI_GUID));
+ZeroMem (>HashInterface, sizeof 
(HashInterfaceHob->HashInterface));
+HashInterfaceHob->HashInterfaceCount = 0;
   }
 
   //
-- 
2.16.2.346.g9779355e34

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

Re: [edk2] how do I use RamDiskDxe?

[Rick Warner]

On 02/13/18 19:48, Wu, Hao A wrote:
>> -Original Message-
>> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Rick
>> Warner
>> Sent: Wednesday, February 14, 2018 12:33 AM
>> To: Wu, Hao A; Andrew Fish
>> Cc: edk2-devel@lists.01.org
>> Subject: Re: [edk2] how do I use RamDiskDxe?
>>
>> On 02/12/2018 10:05 PM, Wu, Hao A wrote:
 -Original Message-
 From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
 Andrew Fish
 Sent: Tuesday, February 13, 2018 8:40 AM
 To: Rick Warner
 Cc: edk2-devel@lists.01.org
 Subject: Re: [edk2] how do I use RamDiskDxe?



> On Feb 12, 2018, at 1:07 PM, Rick Warner  wrote:
>
> Hi All,
>
> I'm trying to develop a tool for automated BIOS flashing from the EFI 
> shell
 using IPMI serial over lan to drive it.  To accomplish this, my plan is to 
 use a
 network booted EFI shell with a tftp client and a ramdisk to download the
>> flash
 tools and ROM into as a workspace and then run the flash commands from
>> that
 ram disk.
> I've been able to successfully build and boot an efi shell from the edk 2
 sources including the tftp client.  I've also been able to separately 
 build the
 RamDiskDxe driver from the MdeModulePkg. Loading the RamDiskDxe.efi
 driver file results in a successful message, but I do not get any usable 
 ram
>> disk
 made available to me?
> How do I get a usable drive letter/name (ie blk0: or any other name) from
>> the
 RamDiskDxe driver?
 Looks like when you load the RamDiskDxe it registers
>> gEfiRamDiskProtocolGuid
>> https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Protocol/Ra
 mDisk.h

>>  which notes this API was added in UEFI 2.6 so you can read up on 
 it
>> in
 that version of the EFI Spec.

  From a quick site read of the code it looks like the driver just produces 
 a
 Protocol that lets you create a RAM disk and VRF (Setup Pages) that let you
 configure one. Seems like you still need to call it and give the raw disk
>> image.
>>>
> Once I've done that, will it be possible to include the RamDiskDxe driver 
> as
 part of the network bootable efi shell image?  I assume I'll need to modify
 the .dsc file for the ShellPkg to include the MdeModulePkg pieces.
> If anyone has any other suggestions for this, I'm open to ideas. The only
>> way
 I've been successful in network booting EFI shell is using just that file 
 directly.
>> I
 tried creating a dos filesystem image with the efi shell file in it just 
 like
>> would
 work with a USB key, but that would not boot over the network stack. If
>> there is
 some way of creating a network bootable EFI shell filesystem image (similar
>> to
 how Linux can network boot a separate vmlinuz kernel and initrd image)?
> Are there any docs that I've missed (I've tried looking in the user docs 
> and
>> on
 the wiki) regarding setting up RamDiskDxe?
 Traditionally network booting has involved loading a single file over the
 network (like an OS Loader) and that file uses the UEFI networking stack to
 download (TFTP read) more images (like the Kernel) from a location implied
>> by
 the location used to download the single file (OS Loader).

>>> Hi,
>>>
>>> For the RamDiskDxe part:
>>>
>>> Just as the previous reply, the RamDiskDxe driver provides:
>>> a). A protocol to (un)register a range of memory to a Ram disk
>>> b). A setup page for user to create:
>>>  a raw Ram disk (no data on the created one) or,
>>>  from a selected file (the file will be the content in the Ram disk,
>>>  no file system though)
>>>
>>> If you want to place the flash tools in the Ram disk and use it, I think
>>> you also need to format the Ram disk and create file system on it first
>>> (e.g. FAT).
>>>
>>> Best Regards,
>>> Hao Wu
>>>
 Thanks,

 Andrew Fish
>> Thank you both very much!
>>
>> How do I use the setup page to create either a raw (empty) or filled w/
>> a file ramdisk?  How do I call that?  I assume the file to create it
>> from would need to already be local to the system.  Is that correct? Or
>> would I be able to directly pull a filesystem image from tftp into the
>> ramdisk upon creation?
> For example, on OVMF, you can enter the setup page by executing 'exit'
> under shell. Then navigate through:
> 'Device Manager' ->
> 'RAM Disk Configuration'
>
> If you create a Ram disk from a file, the whole content of the file will
> be copied to the memory range when creating the Ram disk. The file can be
> an ISO image ,for example. In such case, after creating the Ram disk, a
> file system can be accessed on the Ram disk.
>
> Best Regards,
> Hao Wu
>
>> Thanks,
>> Rick Warner
>>
>>
>> 

[edk2] How load Duet Floppy.IMG File in Bochs or Quemu?

[David Moheban]

Hi,

My Duet floppy image that i compiled with Gcc 5 gets stuck when I try to run it 
in Bochs or Qemu as it does not get any further than 'Welcome to Efi World'. Is 
that normal or are there specific setting needed in the Bochs config to get it 
emulated?

Thank you

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

Re: [edk2] [PATCH] ArmPkg/TimerDxe: Always perform an EOI, even for spurious interrupts

[Marc Zyngier]

On 06/03/18 14:25, Laszlo Ersek wrote:
> On 03/06/18 14:24, Ard Biesheuvel wrote:
>> From: Marc Zyngier 
>>
>> The generic timer driver only EOIs the timer interrupt if
>> the ISTATUS bit is set. This is completely fine if you pretend
>> that spurious interrupts do not exist. But as a matter of fact,
>> they do, and the first one will leave the interrupt activated
>> at the GIC level, making sure that no other interrupt can make
>> it anymore.
>>
>> Making sure that each interrupt Ack is paired with an EOI is the
>> way to go. Oh, and enabling the interrupt each time it is taken
>> is completely pointless. We entered this function for a good
>> reason...
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marc Zyngier 
>> Reviewed-by: Ard Biesheuvel 
>> ---
>>  ArmPkg/Drivers/TimerDxe/TimerDxe.c | 10 --
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c 
>> b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
>> index 2416c90f5545..33d7c91f 100644
>> --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c
>> +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
>> @@ -306,12 +306,13 @@ TimerInterruptHandler (
>>//
>>OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
>>
>> +  // Signal end of interrupt early to help avoid losing subsequent ticks
>> +  // from long duration handlers
>> +  gInterrupt->EndOfInterrupt (gInterrupt, Source);
>> +
>>// Check if the timer interrupt is active
>>if ((ArmGenericTimerGetTimerCtrlReg () ) & ARM_ARCH_TIMER_ISTATUS) {
>>
>> -// Signal end of interrupt early to help avoid losing subsequent ticks 
>> from long duration handlers
>> -gInterrupt->EndOfInterrupt (gInterrupt, Source);
>> -
>>  if (mTimerNotifyFunction) {
>>mTimerNotifyFunction (mTimerPeriod * mElapsedPeriod);
>>  }
>> @@ -339,9 +340,6 @@ TimerInterruptHandler (
>>  ArmGenericTimerEnableTimer ();
>>}
>>
>> -  // Enable timer interrupts
>> -  gInterrupt->EnableInterruptSource (gInterrupt, Source);
>> -
>>gBS->RestoreTPL (OriginalTPL);
>>  }
>>
>>
> 
> It is spooky to see this patch on edk2-devel :)
> 
> In RHEL-7 ArmVirtQemu (aka "AAVMF"), we had carried a somewhat similar
> patch, for a long time.
> 
> I originally added it in Feb 2015, after we rebased our aarch64 host
> kernel from 3.18. to 3.19.. That host kernel
> (incl. KVM) rebase broke guest UEFI *reboot*. We figured it was a KVM
> regression -- but, because we couldn't figure out the exact KVM problem,
> we plastered it over in the guest firmware, a bit similarly to the
> above. This was done for RHBZ#1188054.
> 
> We reported the issue to several KVM people (off-list -- maybe that
> wasn't a great idea, sorry!); you might find the thread under msgid

Yup. In general, not reporting bugs on the ML ends up in missing fixes
and duplicated work. Not the best use of an already limited resource.

> <54d3796e.1040...@redhat.com>, subject "virtual timer interrupt stuck
> across guest firmware reboot on KVM", in your 2015 archives :)

I only have a single trace of this as Christoffer cc'd me on on reply. I
wasn't cc'd on the previous emails, nor on the follow-up if it ever existed.

> At the same time, we filed RHBZ#1189429 for tracking the KVM problem, so
> that once the host kernel got fixed, we could backport those patches,
> and revert the guest firmware kludge.
> 
> The host kernel (KVM) fix was
> :
> 
>   [PATCH 0/9] Rework architected timer and fix UEFI reset
> 
> And, indeed we dropped the "AAVMF" kludge (which revert was separately
> tracked under RHBZ#1259395).
> 
> So it looks like spurious timer interrupts exists on phys hardware as
> well -- I guess I should have attempted to upstream the guest fw patch

Not only timer interrupts. Any interrupt can be spurious, depending on
how quickly your interrupt controller can retire an interrupt that
hasn't been acknowledged already. Propagation delays and all that, and
assuming a level interrupt. For an edge interrupt, you cannot retire it,
full stop (because you cannot "unwrite" something).

> back then, after all. We might have ended up with an improved version of
> it that could have now covered this recent symptom too, perhaps.
> 
> Quoting my downstream patch under my sig for reference.
> 
> For this patch:
> 
> Reviewed-by: Laszlo Ersek 

Thanks,

M.
-- 
Jazz is not dead. It just smells funny...
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] ArmPkg/TimerDxe: Always perform an EOI, even for spurious interrupts

[Ard Biesheuvel]

On 6 March 2018 at 14:25, Laszlo Ersek  wrote:
> On 03/06/18 14:24, Ard Biesheuvel wrote:
>> From: Marc Zyngier 
>>
>> The generic timer driver only EOIs the timer interrupt if
>> the ISTATUS bit is set. This is completely fine if you pretend
>> that spurious interrupts do not exist. But as a matter of fact,
>> they do, and the first one will leave the interrupt activated
>> at the GIC level, making sure that no other interrupt can make
>> it anymore.
>>
>> Making sure that each interrupt Ack is paired with an EOI is the
>> way to go. Oh, and enabling the interrupt each time it is taken
>> is completely pointless. We entered this function for a good
>> reason...
>>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Marc Zyngier 
>> Reviewed-by: Ard Biesheuvel 
>> ---
>>  ArmPkg/Drivers/TimerDxe/TimerDxe.c | 10 --
>>  1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c 
>> b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
>> index 2416c90f5545..33d7c91f 100644
>> --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c
>> +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
>> @@ -306,12 +306,13 @@ TimerInterruptHandler (
>>//
>>OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
>>
>> +  // Signal end of interrupt early to help avoid losing subsequent ticks
>> +  // from long duration handlers
>> +  gInterrupt->EndOfInterrupt (gInterrupt, Source);
>> +
>>// Check if the timer interrupt is active
>>if ((ArmGenericTimerGetTimerCtrlReg () ) & ARM_ARCH_TIMER_ISTATUS) {
>>
>> -// Signal end of interrupt early to help avoid losing subsequent ticks 
>> from long duration handlers
>> -gInterrupt->EndOfInterrupt (gInterrupt, Source);
>> -
>>  if (mTimerNotifyFunction) {
>>mTimerNotifyFunction (mTimerPeriod * mElapsedPeriod);
>>  }
>> @@ -339,9 +340,6 @@ TimerInterruptHandler (
>>  ArmGenericTimerEnableTimer ();
>>}
>>
>> -  // Enable timer interrupts
>> -  gInterrupt->EnableInterruptSource (gInterrupt, Source);
>> -
>>gBS->RestoreTPL (OriginalTPL);
>>  }
>>
>>
>
> It is spooky to see this patch on edk2-devel :)
>

:-)

> In RHEL-7 ArmVirtQemu (aka "AAVMF"), we had carried a somewhat similar
> patch, for a long time.
>
> I originally added it in Feb 2015, after we rebased our aarch64 host
> kernel from 3.18. to 3.19.. That host kernel
> (incl. KVM) rebase broke guest UEFI *reboot*. We figured it was a KVM
> regression -- but, because we couldn't figure out the exact KVM problem,
> we plastered it over in the guest firmware, a bit similarly to the
> above. This was done for RHBZ#1188054.
>
> We reported the issue to several KVM people (off-list -- maybe that
> wasn't a great idea, sorry!); you might find the thread under msgid
> <54d3796e.1040...@redhat.com>, subject "virtual timer interrupt stuck
> across guest firmware reboot on KVM", in your 2015 archives :)
>
> At the same time, we filed RHBZ#1189429 for tracking the KVM problem, so
> that once the host kernel got fixed, we could backport those patches,
> and revert the guest firmware kludge.
>
> The host kernel (KVM) fix was
> :
>
>   [PATCH 0/9] Rework architected timer and fix UEFI reset
>
> And, indeed we dropped the "AAVMF" kludge (which revert was separately
> tracked under RHBZ#1259395).
>
> So it looks like spurious timer interrupts exists on phys hardware as
> well -- I guess I should have attempted to upstream the guest fw patch
> back then, after all. We might have ended up with an improved version of
> it that could have now covered this recent symptom too, perhaps.
>

I was never aware of any such issues at any point, so a head's up
would have been nice, yes :-)

> Quoting my downstream patch under my sig for reference.
>
> For this patch:
>
> Reviewed-by: Laszlo Ersek 
>

Thanks all

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

[edk2] [PATCH 7/7] SecurityPkg OpalPasswordExtraInfoVariable.h: Remove it

[Star Zeng]

Remove OpalPasswordExtraInfoVariable.h as it is not been used
anymore.

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 .../Include/Guid/OpalPasswordExtraInfoVariable.h   | 27 --
 1 file changed, 27 deletions(-)
 delete mode 100644 SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h

diff --git a/SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h 
b/SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h
deleted file mode 100644
index f16d0a4ac360..
--- a/SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h
+++ /dev/null
@@ -1,27 +0,0 @@
-/** @file
-  Defines Name GUIDs to represent an Opal device variable guid for Opal 
Security Feature.
-
-Copyright (c) 2016, 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
-which accompanies this distribution.  The full text of the license may be 
found at
-http://opensource.org/licenses/bsd-license.php
-
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-#ifndef _OPAL_PASSWORD_EXTRA_INFO_VARIABLE_H_
-#define _OPAL_PASSWORD_EXTRA_INFO_VARIABLE_H_
-
-#define OPAL_EXTRA_INFO_VAR_NAME L"OpalExtraInfo"
-
-typedef struct {
-  UINT8   EnableBlockSid;
-} OPAL_EXTRA_INFO_VAR;
-
-extern EFI_GUID gOpalExtraInfoVariableGuid;
-
-#endif // _OPAL_PASSWORD_SECURITY_VARIABLE_H_
-
-- 
2.7.0.windows.1

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

[edk2] [PATCH 3/7] SecurityPkg TcgStorageCoreLib: Make it be base type really

[Star Zeng]

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf 
b/SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf
index a80cb00a9ce3..78a0557bfe42 100644
--- a/SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf
+++ b/SecurityPkg/Library/TcgStorageCoreLib/TcgStorageCoreLib.inf
@@ -3,7 +3,7 @@
 #
 #  This module is used to provide API used by Opal password solution.
 #
-# Copyright (c) 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2016 - 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
 # which accompanies this distribution. The full text of the license may be 
found at
@@ -18,7 +18,7 @@ [Defines]
   FILE_GUID  = ad63b09b-1fc9-4789-af0c-5af8a3fb1f9c
   VERSION_STRING = 1.0
   MODULE_TYPE= BASE
-  LIBRARY_CLASS  = TcgStorageCoreLib|DXE_DRIVER DXE_CORE 
DXE_SMM_DRIVER
+  LIBRARY_CLASS  = TcgStorageCoreLib
 #
 # The following information is for reference only and not required by the 
build tools.
 #
-- 
2.7.0.windows.1

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

[edk2] [PATCH 6/7] SecurityPkg OpalPasswordSupportLib: Remove it

[Star Zeng]

Remove OpalPasswordSupportLib as it is not been used
anymore.

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 .../Include/Library/OpalPasswordSupportLib.h   | 289 
 .../OpalPasswordSupportLib.c   | 781 -
 .../OpalPasswordSupportLib.inf |  55 --
 .../OpalPasswordSupportNotify.h|  55 --
 SecurityPkg/SecurityPkg.dec|   4 -
 SecurityPkg/SecurityPkg.dsc|   2 -
 6 files changed, 1186 deletions(-)
 delete mode 100644 SecurityPkg/Include/Library/OpalPasswordSupportLib.h
 delete mode 100644 
SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.c
 delete mode 100644 
SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.inf
 delete mode 100644 
SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportNotify.h

diff --git a/SecurityPkg/Include/Library/OpalPasswordSupportLib.h 
b/SecurityPkg/Include/Library/OpalPasswordSupportLib.h
deleted file mode 100644
index e616c763f05c..
--- a/SecurityPkg/Include/Library/OpalPasswordSupportLib.h
+++ /dev/null
@@ -1,289 +0,0 @@
-/** @file
-  Header file of Opal password support library.
-
-Copyright (c) 2016, 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
-which accompanies this distribution.  The full text of the license may be 
found at
-http://opensource.org/licenses/bsd-license.php
-
-THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-
-**/
-
-
-#ifndef _OPAL_PASSWORD_SUPPORT_LIB_H_
-#define _OPAL_PASSWORD_SUPPORT_LIB_H_
-
-#include 
-#include 
-
-
-#pragma pack(1)
-
-//
-// Structure that is used to represent the available actions for an OpalDisk.
-// The data can then be utilized to expose/hide certain actions available to 
an end user
-// by the consumer of this library.
-//
-typedef struct {
-//
-// Indicates if the disk can support PSID Revert action.  should verify 
disk supports PSID authority
-//
-UINT16 PsidRevert : 1;
-
-//
-// Indicates if the disk can support Revert action
-//
-UINT16 Revert : 1;
-
-//
-// Indicates if the user must keep data for revert action.  It is true if 
no media encryption is supported.
-//
-UINT16 RevertKeepDataForced : 1;
-
-//
-// Indicates if the disk can support set Admin password
-//
-UINT16 AdminPass : 1;
-
-//
-// Indicates if the disk can support set User password.  This action 
requires that a user
-// password is first enabled.
-//
-UINT16 UserPass : 1;
-
-//
-// Indicates if unlock action is available.  Requires disk to be currently 
locked.
-//
-UINT16 Unlock : 1;
-
-//
-// Indicates if Secure Erase action is available.  Action requires admin 
credentials and media encryption support.
-//
-UINT16 SecureErase : 1;
-
-//
-// Indicates if Disable User action is available.  Action requires admin 
credentials.
-//
-UINT16 DisableUser : 1;
-} OPAL_DISK_ACTIONS;
-
-//
-// Structure that is used to represent the Opal device with password info.
-//
-typedef struct {
-  LIST_ENTRY Link;
-
-  UINT8  Password[32];
-  UINT8  PasswordLength;
-
-  EFI_DEVICE_PATH_PROTOCOL   OpalDevicePath;
-} OPAL_DISK_AND_PASSWORD_INFO;
-
-#pragma pack()
-
-/**
-
-  The function performs determines the available actions for the OPAL_DISK 
provided.
-
-  @param[in]   SupportedAttributes   The support attribute for the device.
-  @param[in]   LockingFeatureThe locking status for the device.
-  @param[in]   OwnerShip The ownership for the device.
-  @param[out]  AvalDiskActions   Pointer to fill-out with appropriate disk 
actions.
-
-**/
-TCG_RESULT
-EFIAPI
-OpalSupportGetAvailableActions(
-  IN  OPAL_DISK_SUPPORT_ATTRIBUTE  *SupportedAttributes,
-  IN  TCG_LOCKING_FEATURE_DESCRIPTOR   *LockingFeature,
-  IN  UINT16   OwnerShip,
-  OUT OPAL_DISK_ACTIONS*AvalDiskActions
-  );
-
-/**
-  Enable Opal Feature for the input device.
-
-  @param[in]  SessionThe opal session for the opal device.
-  @param[in]  Msid   Msid
-  @param[in]  MsidLength Msid Length
-  @param[in]  Password   Admin password
-  @param[in]  PassLength Length of password in bytes
-  @param[in]  DevicePath The device path for the opal devcie.
-
-**/
-TCG_RESULT
-EFIAPI
-OpalSupportEnableOpalFeature(
-  IN OPAL_SESSION  *Session,
-  IN VOID  *Msid,
-  IN UINT32   

[edk2] [PATCH 2/7] SecurityPkg TcgStorageOpalLib: Make it be base type really

[Star Zeng]

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf 
b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
index ec26111ee2c3..78e47387a981 100644
--- a/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
+++ b/SecurityPkg/Library/TcgStorageOpalLib/TcgStorageOpalLib.inf
@@ -3,7 +3,7 @@
 #
 #  This module is used to provide API used by Opal password solution.
 #
-# Copyright (c) 2016, Intel Corporation. All rights reserved.
+# Copyright (c) 2016 - 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
 # which accompanies this distribution. The full text of the license may be 
found at
@@ -18,7 +18,7 @@ [Defines]
   FILE_GUID  = F8B56221-FD5D-4215-B578-C3574AD1E253
   VERSION_STRING = 1.0
   MODULE_TYPE= BASE
-  LIBRARY_CLASS  = TcgStorageOpalLib|DXE_DRIVER DXE_CORE 
DXE_SMM_DRIVER
+  LIBRARY_CLASS  = TcgStorageOpalLib
 
 #
 # The following information is for reference only and not required by the 
build tools.
@@ -37,11 +37,7 @@ [LibraryClasses]
   DebugLib
   TimerLib
   TcgStorageCoreLib
-  UefiLib
 
 [Packages]
   MdePkg/MdePkg.dec
   SecurityPkg/SecurityPkg.dec
-
-[Protocols]
-  gEfiStorageSecurityCommandProtocolGuid ## CONSUMES
-- 
2.7.0.windows.1

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

[edk2] [PATCH 0/7] OpalPassword: New solution without SMM device code

[Star Zeng]

The patch series is also at
https://github.com/lzeng14/edk2 OpalPasswordNew branch.

After IOMMU is enabled in S3, original solution with SMM device
code (OpalPasswordSmm) to unlock OPAL device for S3 will not work
as the DMA operation will be aborted without granted DMA buffer.
Instead, this solution is to add OpalPasswordPei to eliminate
SMM device code, and OPAL setup UI produced by OpalPasswordDxe
will be updated to send requests (set password, update password,
and etc), and then the requests will be processed in next boot
before SmmReadyToLock, password and device info will be saved to
lock box used by OpalPasswordPei to unlock OPAL device for S3.

The old solution related codes are also removed.

Cc: Jiewen Yao 
Cc: Eric Dong 
Cc: Chao Zhang 

Star Zeng (7):
  MdeModulePkg LockBoxLib: Support LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY
  SecurityPkg TcgStorageOpalLib: Make it be base type really
  SecurityPkg TcgStorageCoreLib: Make it be base type really
  SecurityPkg OpalPassword: Add solution without SMM device code
  SecurityPkg OpalPassword: Remove old solution
  SecurityPkg OpalPasswordSupportLib: Remove it
  SecurityPkg OpalPasswordExtraInfoVariable.h: Remove it

 MdeModulePkg/Include/Library/LockBoxLib.h  |   14 +-
 .../Library/SmmLockBoxLib/SmmLockBoxDxeLib.c   |4 +-
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.c   |  227 +-
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf |   10 +-
 .../Include/Guid/OpalPasswordExtraInfoVariable.h   |   27 -
 .../Include/Library/OpalPasswordSupportLib.h   |  289 --
 .../OpalPasswordSupportLib.c   |  781 -
 .../OpalPasswordSupportLib.inf |   55 -
 .../OpalPasswordSupportNotify.h|   55 -
 .../TcgStorageCoreLib/TcgStorageCoreLib.inf|4 +-
 .../TcgStorageOpalLib/TcgStorageOpalLib.inf|8 +-
 SecurityPkg/SecurityPkg.dec|4 -
 SecurityPkg/SecurityPkg.dsc|6 +-
 .../ComponentName.c|0
 .../OpalAhciMode.c |  492 ++--
 .../OpalAhciMode.h |   93 +-
 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c | 2988 
 .../{OpalPasswordDxe => OpalPassword}/OpalDriver.h |  202 +-
 .../{OpalPasswordDxe => OpalPassword}/OpalHii.c|  825 ++
 .../OpalHiiPrivate.h => OpalPassword/OpalHii.h}|  150 +-
 .../OpalHiiCallbacks.c |6 +-
 .../OpalHiiFormStrings.uni |   49 +-
 .../OpalHiiFormValues.h|   74 +-
 .../OpalNvmeMode.c |   95 +-
 .../OpalNvmeMode.h |   19 +-
 .../OpalNvmeReg.h  |5 +-
 .../Tcg/Opal/OpalPassword/OpalPasswordCommon.h |   65 +
 .../OpalPasswordDxe.inf|   25 +-
 .../OpalPasswordForm.vfr   |  250 +-
 .../Tcg/Opal/OpalPassword/OpalPasswordPei.c|  940 ++
 .../Tcg/Opal/OpalPassword/OpalPasswordPei.h|  133 +
 .../Tcg/Opal/OpalPassword/OpalPasswordPei.inf  |   63 +
 SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalDriver.c  | 1091 ---
 .../Tcg/Opal/OpalPasswordDxe/OpalDriverPrivate.h   |  102 -
 SecurityPkg/Tcg/Opal/OpalPasswordDxe/OpalHii.h |  146 -
 SecurityPkg/Tcg/Opal/OpalPasswordSmm/OpalIdeMode.c |  767 -
 SecurityPkg/Tcg/Opal/OpalPasswordSmm/OpalIdeMode.h |  173 --
 .../Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.c | 1088 ---
 .../Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.h |  299 --
 .../Tcg/Opal/OpalPasswordSmm/OpalPasswordSmm.inf   |   77 -
 40 files changed, 5536 insertions(+), 6165 deletions(-)
 delete mode 100644 SecurityPkg/Include/Guid/OpalPasswordExtraInfoVariable.h
 delete mode 100644 SecurityPkg/Include/Library/OpalPasswordSupportLib.h
 delete mode 100644 
SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.c
 delete mode 100644 
SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportLib.inf
 delete mode 100644 
SecurityPkg/Library/OpalPasswordSupportLib/OpalPasswordSupportNotify.h
 rename SecurityPkg/Tcg/Opal/{OpalPasswordDxe => OpalPassword}/ComponentName.c 
(100%)
 rename SecurityPkg/Tcg/Opal/{OpalPasswordSmm => OpalPassword}/OpalAhciMode.c 
(68%)
 rename SecurityPkg/Tcg/Opal/{OpalPasswordSmm => OpalPassword}/OpalAhciMode.h 
(85%)
 create mode 100644 SecurityPkg/Tcg/Opal/OpalPassword/OpalDriver.c
 rename SecurityPkg/Tcg/Opal/{OpalPasswordDxe => OpalPassword}/OpalDriver.h 
(72%)
 rename SecurityPkg/Tcg/Opal/{OpalPasswordDxe => OpalPassword}/OpalHii.c (57%)
 rename SecurityPkg/Tcg/Opal/{OpalPasswordDxe/OpalHiiPrivate.h => 
OpalPassword/OpalHii.h} (72%)
 rename SecurityPkg/Tcg/Opal/{OpalPasswordDxe => 
OpalPassword}/OpalHiiCallbacks.c (91%)
 rename SecurityPkg/Tcg/Opal/{OpalPasswordDxe => 

[edk2] [PATCH 1/7] MdeModulePkg LockBoxLib: Support LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY

[Star Zeng]

With this flag, the LockBox can be restored in S3 resume only.
The LockBox can not be restored after SmmReadyToLock in normal boot
and after EndOfS3Resume in S3 resume.
It can not be set together with LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE.

Cc: Jiewen Yao 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 MdeModulePkg/Include/Library/LockBoxLib.h  |  14 +-
 .../Library/SmmLockBoxLib/SmmLockBoxDxeLib.c   |   4 +-
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.c   | 227 -
 .../Library/SmmLockBoxLib/SmmLockBoxSmmLib.inf |  10 +-
 4 files changed, 247 insertions(+), 8 deletions(-)

diff --git a/MdeModulePkg/Include/Library/LockBoxLib.h 
b/MdeModulePkg/Include/Library/LockBoxLib.h
index db7fd05def58..80beb4d0f880 100644
--- a/MdeModulePkg/Include/Library/LockBoxLib.h
+++ b/MdeModulePkg/Include/Library/LockBoxLib.h
@@ -2,7 +2,7 @@
   This library is only intended to be used by DXE modules that need save
   confidential information to LockBox and get it by PEI modules in S3 phase.
 
-Copyright (c) 2010 - 2011, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -62,9 +62,17 @@ SetLockBoxAttributes (
   );
 
 //
-// With this flag, this LockBox can be restored to this Buffer with 
RestoreAllLockBoxInPlace()
+// With this flag, this LockBox can be restored to this Buffer
+// with RestoreAllLockBoxInPlace()
 //
-#define LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE  BIT0
+#define LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE BIT0
+//
+// With this flag, this LockBox can be restored in S3 resume only.
+// This LockBox can not be restored after SmmReadyToLock in normal boot
+// and after EndOfS3Resume in S3 resume.
+// It can not be set together with LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE.
+//
+#define LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY   BIT1
 
 /**
   This function will update confidential information to lockbox.
diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
index b75f81e69e04..9b6f0bedbd4f 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxDxeLib.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2010 - 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -241,7 +241,7 @@ SetLockBoxAttributes (
   // Basic check
   //
   if ((Guid == NULL) ||
-  ((Attributes & ~LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE) != 0)) {
+  ((Attributes & ~(LOCK_BOX_ATTRIBUTE_RESTORE_IN_PLACE | 
LOCK_BOX_ATTRIBUTE_RESTORE_IN_S3_ONLY)) != 0)) {
 return EFI_INVALID_PARAMETER;
   }
 
diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c 
b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
index 4960df755534..af75a4cb9cd1 100644
--- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
+++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxSmmLib.c
@@ -1,6 +1,6 @@
 /** @file
 
-Copyright (c) 2010 - 2016, Intel Corporation. All rights reserved.
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
 
 This program and the accompanying materials
 are licensed and made available under the terms and conditions
@@ -20,6 +20,10 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
+#include 
+#include 
+#include 
 
 #include "SmmLockBoxLibPrivate.h"
 
@@ -31,6 +35,11 @@ SMM_LOCK_BOX_CONTEXT mSmmLockBoxContext;
 LIST_ENTRY   mLockBoxQueue = INITIALIZE_LIST_HEAD_VARIABLE 
(mLockBoxQueue);
 
 BOOLEAN  mSmmConfigurationTableInstalled = FALSE;
+VOID *mRegistrationSmmEndOfDxe = NULL;
+VOID *mRegistrationSmmReadyToLock = NULL;
+VOID *mRegistrationEndOfS3Resume = NULL;
+BOOLEAN  mSmmLockBoxSmmReadyToLock = FALSE;
+BOOLEAN  mSmmLockBoxDuringS3Resume = FALSE;
 
 /**
   This function return SmmLockBox context from SMST.
@@ -64,6 +73,128 @@ InternalGetSmmLockBoxContext (
 }
 
 /**
+  Notification for SMM ReadyToLock protocol.
+
+  @param[in] Protocol   Points to the protocol's unique identifier.
+  @param[in] Interface  Points to the interface instance.
+  @param[in] Handle The handle on which the interface was installed.
+
+  @retval EFI_SUCCESS   Notification runs successfully.
+**/
+EFI_STATUS
+EFIAPI
+SmmLockBoxSmmReadyToLockNotify (
+  IN CONST EFI_GUID  *Protocol,
+  IN VOID*Interface,
+  IN EFI_HANDLE  Handle
+  )
+{
+  mSmmLockBoxSmmReadyToLock = TRUE;
+  return EFI_SUCCESS;
+}
+
+/**
+  Main entry point for an SMM handler dispatch or communicate-based callback.
+
+  

Re: [edk2] [PATCH] ArmPkg/TimerDxe: Always perform an EOI, even for spurious interrupts

[Laszlo Ersek]

On 03/06/18 14:24, Ard Biesheuvel wrote:
> From: Marc Zyngier 
>
> The generic timer driver only EOIs the timer interrupt if
> the ISTATUS bit is set. This is completely fine if you pretend
> that spurious interrupts do not exist. But as a matter of fact,
> they do, and the first one will leave the interrupt activated
> at the GIC level, making sure that no other interrupt can make
> it anymore.
>
> Making sure that each interrupt Ack is paired with an EOI is the
> way to go. Oh, and enabling the interrupt each time it is taken
> is completely pointless. We entered this function for a good
> reason...
>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marc Zyngier 
> Reviewed-by: Ard Biesheuvel 
> ---
>  ArmPkg/Drivers/TimerDxe/TimerDxe.c | 10 --
>  1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c 
> b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> index 2416c90f5545..33d7c91f 100644
> --- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> +++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
> @@ -306,12 +306,13 @@ TimerInterruptHandler (
>//
>OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
>
> +  // Signal end of interrupt early to help avoid losing subsequent ticks
> +  // from long duration handlers
> +  gInterrupt->EndOfInterrupt (gInterrupt, Source);
> +
>// Check if the timer interrupt is active
>if ((ArmGenericTimerGetTimerCtrlReg () ) & ARM_ARCH_TIMER_ISTATUS) {
>
> -// Signal end of interrupt early to help avoid losing subsequent ticks 
> from long duration handlers
> -gInterrupt->EndOfInterrupt (gInterrupt, Source);
> -
>  if (mTimerNotifyFunction) {
>mTimerNotifyFunction (mTimerPeriod * mElapsedPeriod);
>  }
> @@ -339,9 +340,6 @@ TimerInterruptHandler (
>  ArmGenericTimerEnableTimer ();
>}
>
> -  // Enable timer interrupts
> -  gInterrupt->EnableInterruptSource (gInterrupt, Source);
> -
>gBS->RestoreTPL (OriginalTPL);
>  }
>
>

It is spooky to see this patch on edk2-devel :)

In RHEL-7 ArmVirtQemu (aka "AAVMF"), we had carried a somewhat similar
patch, for a long time.

I originally added it in Feb 2015, after we rebased our aarch64 host
kernel from 3.18. to 3.19.. That host kernel
(incl. KVM) rebase broke guest UEFI *reboot*. We figured it was a KVM
regression -- but, because we couldn't figure out the exact KVM problem,
we plastered it over in the guest firmware, a bit similarly to the
above. This was done for RHBZ#1188054.

We reported the issue to several KVM people (off-list -- maybe that
wasn't a great idea, sorry!); you might find the thread under msgid
<54d3796e.1040...@redhat.com>, subject "virtual timer interrupt stuck
across guest firmware reboot on KVM", in your 2015 archives :)

At the same time, we filed RHBZ#1189429 for tracking the KVM problem, so
that once the host kernel got fixed, we could backport those patches,
and revert the guest firmware kludge.

The host kernel (KVM) fix was
:

  [PATCH 0/9] Rework architected timer and fix UEFI reset

And, indeed we dropped the "AAVMF" kludge (which revert was separately
tracked under RHBZ#1259395).

So it looks like spurious timer interrupts exists on phys hardware as
well -- I guess I should have attempted to upstream the guest fw patch
back then, after all. We might have ended up with an improved version of
it that could have now covered this recent symptom too, perhaps.

Quoting my downstream patch under my sig for reference.

For this patch:

Reviewed-by: Laszlo Ersek 

Thanks
Laszlo

PS: some of the RHBZs listed above are "private" -- sorry about that,
the rules are arcane, and for mere developers like yours truly, there's
a certain element of "better safe than sorry" to it.

> From 58351b83d3f8badb8f0e44f70b06cfbba9644d20 Mon Sep 17 00:00:00 2001
> From: Laszlo Ersek 
> Date: Thu, 5 Feb 2015 17:53:53 +0100
> Subject: [RHELSA AAVMF PATCH 1/1] ArmPkg: TimerDxe: smack down spurious timer
>  interrupt (RHELSA hack)
>
> Message-id: <1423155233-22668-2-git-send-email-ler...@redhat.com>
> Patchwork-id: 63732
> O-Subject:  [RHELSA AAVMF PATCH 1/1] ArmPkg: TimerDxe: smack down spurious 
> timer
>   interrupt (RHELSA hack)
> Bugzilla: 1188054
> Acked-by: Miroslav Rezanina 
> Acked-by: w...@redhat.com
> Acked-by: Andrew Jones 
>
> When an aarch64 KVM guest is rebooted with the UEFI shell command "reset
> -c", the instance of AAVMF that runs after the reboot hangs in the timer
> driver. We tracked this issue unto a virtual timer interrupt that remains
> stuck / asserted across the reboot for some reason, and is delivered to
> the handler function in TimerDxe *before* TimerDxe is done setting up data
> for the handler and unmasks the interrupt. This happens to cause an
> infinite loop in the handler.
>
> 

[edk2] [PATCH v1] Platform/ARM/VExpressPkg: Fix RSDT pointer in RSDP

[Sami Mujawar]

According to the SBBR Specification (ARM DEN 0044B), Section 4.2.1.1
  "Within the RSDP, the RsdtAddress field must be null (zero)
   and the XsdtAddresss MUST be a valid, non-null, 64-bit value."

The PcdAcpiExposedTableVersions is used to indicate the ACPI versions
that are supported. The default value for PcdAcpiExposedTableVersions
is 0x3E which indicates that the ACPI versions 1.0B and above are
supported.

For ACPI 1.0B the RSDT pointer is set in the RSDP table. However for
ACPI versions greater than ACPI 1.0B the AcpiTableDxe populates
the RSDP with the RSDT address set to NULL.

Therefore set the PcdAcpiExposedTableVersions to 0x20 indicating
support for ACPI 5.0 and above.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Sami Mujawar 
Signed-off-by: Evan Lloyd 
---

The changes can be seen at 
https://github.com/samimujawar/edk2-platforms/tree/212_fvp_rsdt_fix_v1

Notes:
v1:
- Setup PcdAcpiExposedTableVersions correctly so that the
  AcpiTableDxe populates the RSDP with the RSDT address set
  to NULL. [SAMI]

 Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc 
b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
index 
295e9a126b0ebaa4653d7e25e2f7d8c396403764..cdf9e2d49784d542701dc84eb511f592e77ec106
 100644
--- a/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
+++ b/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
@@ -1,5 +1,5 @@
 #
-#  Copyright (c) 2011-2017, ARM Limited. All rights reserved.
+#  Copyright (c) 2011-2018, ARM Limited. All rights reserved.
 #
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
@@ -162,6 +162,11 @@ [PcdsFixedAtBuild.common]
   # the entire FVP address space can be covered by 36 bit VAs
   gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|36
 
+  #
+  # ACPI Table Version
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiExposedTableVersions|0x20
+
 

 #
 # Components Section - list of all EDK II Modules needed by this Platform
-- 
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'


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

Re: [edk2] [PATCH v2 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard

[Yao, Jiewen]

Both patches are reviewed-by: jiewen@intel.com

> -Original Message-
> From: Wu, Hao A
> Sent: Tuesday, March 6, 2018 9:33 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Wang, Jian J ;
> Zeng, Star ; Dong, Eric ; Yao,
> Jiewen ; Ni, Ruiyu 
> Subject: [PATCH v2 2/2] MdeModulePkg/Core: Fix feature conflict between NX
> and Stack guard
> 
> If enabled, NX memory protection feature will mark some types of active
> memory as NX (non-executable), which includes the first page of the stack.
> This will overwrite the attributes of the first page of the stack if the
> stack guard feature is also enabled.
> 
> The solution is to override the attributes setting to the first page of
> the stack by adding back the 'EFI_MEMORY_RP' attribute when the stack
> guard feature is enabled.
> 
> Cc: Jian J Wang 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Jiewen Yao 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu 
> ---
>  MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 54
> +++
>  2 files changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index 7334780326..d2e7360ed4 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -3,7 +3,7 @@
>  #
>  #  It provides an implementation of DXE Core that is compliant with DXE CIS.
>  #
> -#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
>  #  This program and the accompanying materials
>  #  are licensed and made available under the terms and conditions of the BSD
> License
>  #  which accompanies this distribution.  The full text of the license may be
> found at
> @@ -130,6 +130,7 @@
>gEfiPropertiesTableGuid   ##
> SOMETIMES_PRODUCES   ## SystemTable
>gEfiMemoryAttributesTableGuid ##
> SOMETIMES_PRODUCES   ## SystemTable
>gEfiEndOfDxeEventGroupGuid##
> SOMETIMES_CONSUMES   ## Event
> +  gEfiHobMemoryAllocStackGuid   ##
> SOMETIMES_CONSUMES   ## SystemTable
> 
>  [Ppis]
>gEfiVectorHandoffInfoPpiGuid  ## UNDEFINED # HOB
> @@ -198,6 +199,7 @@
>gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType
> ## CONSUMES
>gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType
> ## CONSUMES
>gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
> ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
> ## CONSUMES
> 
>  # [Hob]
>  # RESOURCE_DESCRIPTOR   ## CONSUMES
> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> index a2ea445eef..f3e62dd2c5 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> @@ -801,6 +801,9 @@ InitializeDxeNxMemoryProtectionPolicy (
>UINT64Attributes;
>LIST_ENTRY*Link;
>EFI_GCD_MAP_ENTRY *Entry;
> +  EFI_PEI_HOB_POINTERS  Hob;
> +  EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
> +  EFI_PHYSICAL_ADDRESS  StackBase;
> 
>//
>// Get the EFI memory map.
> @@ -832,6 +835,40 @@ InitializeDxeNxMemoryProtectionPolicy (
>} while (Status == EFI_BUFFER_TOO_SMALL);
>ASSERT_EFI_ERROR (Status);
> 
> +  StackBase = 0;
> +  if (PcdGetBool (PcdCpuStackGuard)) {
> +//
> +// Get the base of stack from Hob.
> +//
> +Hob.Raw = GetHobList ();
> +while ((Hob.Raw = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION,
> Hob.Raw)) != NULL) {
> +  MemoryHob = Hob.MemoryAllocation;
> +  if (CompareGuid(,
> >AllocDescriptor.Name)) {
> +DEBUG ((
> +  DEBUG_INFO,
> +  "%a: StackBase = 0x%016lx  StackSize = 0x%016lx\n",
> +  __FUNCTION__,
> +  MemoryHob->AllocDescriptor.MemoryBaseAddress,
> +  MemoryHob->AllocDescriptor.MemoryLength
> +  ));
> +
> +StackBase = MemoryHob->AllocDescriptor.MemoryBaseAddress;
> +//
> +// Ensure the base of the stack is page-size aligned.
> +//
> +ASSERT ((StackBase & EFI_PAGE_MASK) == 0);
> +break;
> +  }
> +  Hob.Raw = GET_NEXT_HOB (Hob);
> +}
> +
> +//
> +// Ensure the base of stack can be found from Hob when stack guard is
> +// enabled.
> +//
> +ASSERT (StackBase != 0);
> +  }
> +
>DEBUG ((
>  DEBUG_INFO,
>  "%a: applying strict permissions to active memory regions\n",
> @@ -864,6 +901,23 @@ InitializeDxeNxMemoryProtectionPolicy (
>

[edk2] [PATCH v2 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard

[Hao Wu]

If enabled, NX memory protection feature will mark some types of active
memory as NX (non-executable), which includes the first page of the stack.
This will overwrite the attributes of the first page of the stack if the
stack guard feature is also enabled.

The solution is to override the attributes setting to the first page of
the stack by adding back the 'EFI_MEMORY_RP' attribute when the stack
guard feature is enabled.

Cc: Jian J Wang 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 54 +++
 2 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf 
b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 7334780326..d2e7360ed4 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -3,7 +3,7 @@
 #
 #  It provides an implementation of DXE Core that is compliant with DXE CIS.
 #  
-#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
 #  which accompanies this distribution.  The full text of the license may be 
found at
@@ -130,6 +130,7 @@
   gEfiPropertiesTableGuid   ## SOMETIMES_PRODUCES   ## 
SystemTable
   gEfiMemoryAttributesTableGuid ## SOMETIMES_PRODUCES   ## 
SystemTable
   gEfiEndOfDxeEventGroupGuid## SOMETIMES_CONSUMES   ## 
Event
+  gEfiHobMemoryAllocStackGuid   ## SOMETIMES_CONSUMES   ## 
SystemTable
 
 [Ppis]
   gEfiVectorHandoffInfoPpiGuid  ## UNDEFINED # HOB
@@ -198,6 +199,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType   ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType   ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask   ## 
CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard   ## 
CONSUMES
 
 # [Hob]
 # RESOURCE_DESCRIPTOR   ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index a2ea445eef..f3e62dd2c5 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -801,6 +801,9 @@ InitializeDxeNxMemoryProtectionPolicy (
   UINT64Attributes;
   LIST_ENTRY*Link;
   EFI_GCD_MAP_ENTRY *Entry;
+  EFI_PEI_HOB_POINTERS  Hob;
+  EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
+  EFI_PHYSICAL_ADDRESS  StackBase;
 
   //
   // Get the EFI memory map.
@@ -832,6 +835,40 @@ InitializeDxeNxMemoryProtectionPolicy (
   } while (Status == EFI_BUFFER_TOO_SMALL);
   ASSERT_EFI_ERROR (Status);
 
+  StackBase = 0;
+  if (PcdGetBool (PcdCpuStackGuard)) {
+//
+// Get the base of stack from Hob.
+//
+Hob.Raw = GetHobList ();
+while ((Hob.Raw = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, Hob.Raw)) != 
NULL) {
+  MemoryHob = Hob.MemoryAllocation;
+  if (CompareGuid(, 
>AllocDescriptor.Name)) {
+DEBUG ((
+  DEBUG_INFO,
+  "%a: StackBase = 0x%016lx  StackSize = 0x%016lx\n",
+  __FUNCTION__,
+  MemoryHob->AllocDescriptor.MemoryBaseAddress,
+  MemoryHob->AllocDescriptor.MemoryLength
+  ));
+
+StackBase = MemoryHob->AllocDescriptor.MemoryBaseAddress;
+//
+// Ensure the base of the stack is page-size aligned.
+//
+ASSERT ((StackBase & EFI_PAGE_MASK) == 0);
+break;
+  }
+  Hob.Raw = GET_NEXT_HOB (Hob);
+}
+
+//
+// Ensure the base of stack can be found from Hob when stack guard is
+// enabled.
+//
+ASSERT (StackBase != 0);
+  }
+
   DEBUG ((
 DEBUG_INFO,
 "%a: applying strict permissions to active memory regions\n",
@@ -864,6 +901,23 @@ InitializeDxeNxMemoryProtectionPolicy (
   EFI_PAGES_TO_SIZE (1),
   EFI_MEMORY_RP | Attributes);
   }
+
+  if (StackBase != 0 &&
+  (StackBase >= MemoryMapEntry->PhysicalStart &&
+   StackBase <  MemoryMapEntry->PhysicalStart +
+LShiftU64 (MemoryMapEntry->NumberOfPages, 
EFI_PAGE_SHIFT)) &&
+  PcdGetBool (PcdCpuStackGuard)) {
+
+//
+// Add EFI_MEMORY_RP attribute for the first page of the stack if stack
+// guard is enabled.
+//
+SetUefiImageMemoryAttributes (
+  StackBase,
+  EFI_PAGES_TO_SIZE (1),
+  EFI_MEMORY_RP | Attributes);
+ 

[edk2] [PATCH v2 0/2] Resolve feature conflict between NX and Stack guard

[Hao Wu]

V2 changes:

A. Use Hoblib APIs to get the base of stack from Hob.
B. Remove unnecessary local variable used in function
   InitializeDxeNxMemoryProtectionPolicy().

V1 history:

If enabled, NX memory protection feature will mark some types of active
memory as NX (non-executable), which includes the first page of the stack.
This will overwrite the attributes of the first page of the stack if the
stack guard feature is also enabled.

The series will override the attributes setting to the first page of the
stack by adding back the 'EFI_MEMORY_RP' attribute when the stack guard
feature is enabled.

Cc: Jian J Wang 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 

Hao Wu (2):
  MdeModulePkg/Core: Refine handling NULL detection in NX setting
  MdeModulePkg/Core: Fix feature conflict between NX and Stack guard

 MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 74 +++
 2 files changed, 67 insertions(+), 11 deletions(-)

-- 
2.12.0.windows.1

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

[edk2] [PATCH v2 1/2] MdeModulePkg/Core: Refine handling NULL detection in NX setting

[Hao Wu]

The commit rewrites the logic in function
InitializeDxeNxMemoryProtectionPolicy() for handling the first page
(page 0) when NULL pointer detection feature is enabled.

Instead of skip setting the page 0, the codes will now override the
attribute setting of page 0 by adding the 'EFI_MEMORY_RP' attribute.

The purpose is to make it easy for other special handlings of pages
(e.g. the first page of the stack when stack guard feature is enabled).

Cc: Jian J Wang 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 455ed35f9a..a2ea445eef 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -19,7 +19,7 @@
 
   Once the image is unloaded, the protection is removed automatically.
 
-Copyright (c) 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2017 - 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
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -846,23 +846,23 @@ InitializeDxeNxMemoryProtectionPolicy (
 
 Attributes = GetPermissionAttributeForMemoryType (MemoryMapEntry->Type);
 if (Attributes != 0) {
+  SetUefiImageMemoryAttributes (
+MemoryMapEntry->PhysicalStart,
+LShiftU64 (MemoryMapEntry->NumberOfPages, EFI_PAGE_SHIFT),
+Attributes);
+
   if (MemoryMapEntry->PhysicalStart == 0 &&
   PcdGet8 (PcdNullPointerDetectionPropertyMask) != 0) {
 
 ASSERT (MemoryMapEntry->NumberOfPages > 0);
 //
-// Skip page 0 if NULL pointer detection is enabled to avoid attributes
-// overwritten.
+// Add EFI_MEMORY_RP attribute for page 0 if NULL pointer detection is
+// enabled.
 //
 SetUefiImageMemoryAttributes (
-  MemoryMapEntry->PhysicalStart + EFI_PAGE_SIZE,
-  LShiftU64 (MemoryMapEntry->NumberOfPages - 1, EFI_PAGE_SHIFT),
-  Attributes);
-  } else {
-SetUefiImageMemoryAttributes (
-  MemoryMapEntry->PhysicalStart,
-  LShiftU64 (MemoryMapEntry->NumberOfPages, EFI_PAGE_SHIFT),
-  Attributes);
+  0,
+  EFI_PAGES_TO_SIZE (1),
+  EFI_MEMORY_RP | Attributes);
   }
 }
 MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);
-- 
2.12.0.windows.1

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

Re: [edk2] [PATCH 1/5] MdePkg/SPI: Change function definitions to match their descriptions.

[Marvin Häuser]

Hey Dandan,

Thanks for your reply.
I issued updates for the current headers because I saw activity in 
edk2-platforms to implement these incomplete protocols and because I haven't 
heard back for discussed changes in the specification for some time.
Sure I could help submitting the ECR, however I'm not sure how. I'm not an UEFI 
Contributor and neither did I find any design flaws, just typos and missing 
definitions, which obviously must be bit values due to their non-exclusive 
nature.
If you need me to do something concrete, I will try to do my best.

Regards,
Marvin.

> -Original Message-
> From: Bi, Dandan 
> Sent: Monday, March 5, 2018 3:15 AM
> To: Marvin Häuser ; edk2-
> de...@lists.01.org
> Cc: Kinney, Michael D ; Gao, Liming
> ; Bi, Dandan 
> Subject: RE: [PATCH 1/5] MdePkg/SPI: Change function definitions to match
> their descriptions.
> 
> Hi Marvin,
> 
> Thank you very much for your contribution. Could you hold this patch series?
> Since current SPI header files follow PI1.6 spec.
> For this case, we should submit an ECR to update the PI spec firstly. After 
> the
> ECR has been approved by PIWG, then we can send patch to update them.
> Since you have found a lot of missing/incorrect parts in the SPI part of PI
> Spec. Could you help to submit an ECR to update them?
> 
> 
> Thanks,
> Dandan
> 
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Marvin Häuser
> Sent: Wednesday, February 28, 2018 12:49 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Gao, Liming
> 
> Subject: [edk2] [PATCH 1/5] MdePkg/SPI: Change function definitions to
> match their descriptions.
> 
> The PI specification is not continuous with function headers and descriptions
> for the SPI protocols. Correct and comment these mistakes.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Marvin Haeuser 
> ---
>  MdePkg/Include/Protocol/SpiConfiguration.h | 12 
>  MdePkg/Include/Protocol/SpiHc.h| 14 +-
>  MdePkg/Include/Protocol/SpiIo.h| 15 ++-
>  3 files changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/MdePkg/Include/Protocol/SpiConfiguration.h
> b/MdePkg/Include/Protocol/SpiConfiguration.h
> index c0df183ec7f0..c36a809f4232 100644
> --- a/MdePkg/Include/Protocol/SpiConfiguration.h
> +++ b/MdePkg/Include/Protocol/SpiConfiguration.h
> @@ -1,7 +1,7 @@
>  /** @file
>This file defines the SPI Configuration Protocol.
> 
> -  Copyright (c) 2017, Intel Corporation. All rights reserved.
> +  Copyright (c) 2017 - 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 which accompanies this distribution. The full text of the license 
> may
> @@ -65,6 +65,10 @@ EFI_STATUS
>IN BOOLEAN   PinValue
>);
> 
> +//
> +// Note: In the PI specification, ClockHz is decorated as only 'IN', which is
> +//   not conforming to the parameter description.
> +//
>  /**
>Set up the clock generator to produce the correct clock frequency, phase
> and
>polarity for a SPI chip.
> @@ -78,7 +82,7 @@ EFI_STATUS
>  ClockPhase and ClockPolarity fields. The routine
>  also has access to the names for the SPI bus and
>  chip which can be used during debugging.
> -  @param[in] ClockHzPointer to the requested clock frequency. The
> clock
> +  @param[in,out] ClockHzPointer to the requested clock frequency. The
> clock
>  generator will choose a supported clock frequency
>  which is less then or equal to this value.
>  Specify zero to turn the clock generator off.
> @@ -92,8 +96,8 @@ EFI_STATUS
>  **/
>  typedef EFI_STATUS
>  (EFIAPI *EFI_SPI_CLOCK) (
> -  IN CONST EFI_SPI_PERIPHERAL  *SpiPeripheral,
> -  IN UINT32*ClockHz
> +  IN CONST EFI_SPI_PERIPHERAL  *SpiPeripheral,
> +  IN OUT UINT32*ClockHz
>);
> 
>  ///
> diff --git a/MdePkg/Include/Protocol/SpiHc.h
> b/MdePkg/Include/Protocol/SpiHc.h index 12fe5d2dce0a..71c75431e4e8
> 100644
> --- a/MdePkg/Include/Protocol/SpiHc.h
> +++ b/MdePkg/Include/Protocol/SpiHc.h
> @@ -1,7 +1,7 @@
>  /** @file
>This file defines the SPI Host Controller Protocol.
> 
> -  Copyright (c) 2017, Intel Corporation. All rights reserved.
> +  Copyright (c) 2017 - 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 which accompanies this distribution. 

[edk2] [PATCH] ArmPkg/TimerDxe: Always perform an EOI, even for spurious interrupts

[Ard Biesheuvel]

From: Marc Zyngier 

The generic timer driver only EOIs the timer interrupt if
the ISTATUS bit is set. This is completely fine if you pretend
that spurious interrupts do not exist. But as a matter of fact,
they do, and the first one will leave the interrupt activated
at the GIC level, making sure that no other interrupt can make
it anymore.

Making sure that each interrupt Ack is paired with an EOI is the
way to go. Oh, and enabling the interrupt each time it is taken
is completely pointless. We entered this function for a good
reason...

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Marc Zyngier 
Reviewed-by: Ard Biesheuvel 
---
 ArmPkg/Drivers/TimerDxe/TimerDxe.c | 10 --
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/ArmPkg/Drivers/TimerDxe/TimerDxe.c 
b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
index 2416c90f5545..33d7c91f 100644
--- a/ArmPkg/Drivers/TimerDxe/TimerDxe.c
+++ b/ArmPkg/Drivers/TimerDxe/TimerDxe.c
@@ -306,12 +306,13 @@ TimerInterruptHandler (
   //
   OriginalTPL = gBS->RaiseTPL (TPL_HIGH_LEVEL);
 
+  // Signal end of interrupt early to help avoid losing subsequent ticks
+  // from long duration handlers
+  gInterrupt->EndOfInterrupt (gInterrupt, Source);
+
   // Check if the timer interrupt is active
   if ((ArmGenericTimerGetTimerCtrlReg () ) & ARM_ARCH_TIMER_ISTATUS) {
 
-// Signal end of interrupt early to help avoid losing subsequent ticks 
from long duration handlers
-gInterrupt->EndOfInterrupt (gInterrupt, Source);
-
 if (mTimerNotifyFunction) {
   mTimerNotifyFunction (mTimerPeriod * mElapsedPeriod);
 }
@@ -339,9 +340,6 @@ TimerInterruptHandler (
 ArmGenericTimerEnableTimer ();
   }
 
-  // Enable timer interrupts
-  gInterrupt->EnableInterruptSource (gInterrupt, Source);
-
   gBS->RestoreTPL (OriginalTPL);
 }
 
-- 
2.11.0

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

Re: [edk2] [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard

[Wu, Hao A]

Got it, I will send out a V2 of the series according to the comments.

Best Regards,
Hao Wu


> -Original Message-
> From: Yao, Jiewen
> Sent: Tuesday, March 06, 2018 9:11 PM
> To: Yao, Jiewen; Zeng, Star; Wu, Hao A; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu; Dong, Eric
> Subject: RE: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX
> and Stack guard
> 
> BTW: I don't think "StackBaseFound" is really needed.
> 
> We can use ASSERT (StackBase != 0); directly. :-)
> 
> Thank you
> Yao Jiewen
> 
> > -Original Message-
> > From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yao,
> > Jiewen
> > Sent: Tuesday, March 6, 2018 9:05 PM
> > To: Zeng, Star ; Wu, Hao A ;
> > edk2-devel@lists.01.org
> > Cc: Ni, Ruiyu ; Dong, Eric 
> > Subject: Re: [edk2] [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict
> > between NX and Stack guard
> >
> > Agree.
> >
> > With this update, reviewed-by: jiewen@intel.com
> >
> >
> > > -Original Message-
> > > From: Zeng, Star
> > > Sent: Tuesday, March 6, 2018 8:16 PM
> > > To: Wu, Hao A ; edk2-devel@lists.01.org
> > > Cc: Wang, Jian J ; Dong, Eric
> ;
> > > Yao, Jiewen ; Ni, Ruiyu ; Zeng,
> > Star
> > > 
> > > Subject: RE: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between
> > NX
> > > and Stack guard
> > >
> > > A quick minor comment.
> > > GetHobList() could be used instead of EfiGetSystemConfigurationTable
> > > (, ).
> > >
> > > Thanks,
> > > Star
> > > -Original Message-
> > > From: Wu, Hao A
> > > Sent: Tuesday, March 6, 2018 8:11 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Wu, Hao A ; Wang, Jian J
> ;
> > > Zeng, Star ; Dong, Eric ; Yao,
> > > Jiewen ; Ni, Ruiyu 
> > > Subject: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX
> > and
> > > Stack guard
> > >
> > > If enabled, NX memory protection feature will mark some types of active
> > > memory as NX (non-executable), which includes the first page of the stack.
> > > This will overwrite the attributes of the first page of the stack if the 
> > > stack
> guard
> > > feature is also enabled.
> > >
> > > The solution is to override the attributes setting to the first page of 
> > > the
> stack by
> > > adding back the 'EFI_MEMORY_RP' attribute when the stack guard feature
> is
> > > enabled.
> > >
> > > Cc: Jian J Wang 
> > > Cc: Star Zeng 
> > > Cc: Eric Dong 
> > > Cc: Jiewen Yao 
> > > Cc: Ruiyu Ni 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Hao Wu 
> > > ---
> > >  MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
> > >  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 61
> > > +++
> > >  2 files changed, 64 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > index 7334780326..d2e7360ed4 100644
> > > --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> > > @@ -3,7 +3,7 @@
> > >  #
> > >  #  It provides an implementation of DXE Core that is compliant with DXE
> CIS.
> > >  #
> > > -#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
> > > +#  Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > > +reserved.
> > >  #  This program and the accompanying materials  #  are licensed and
> > made
> > > available under the terms and conditions of the BSD License  #  which
> > > accompanies this distribution.  The full text of the license may be found 
> > > at
> > @@
> > > -130,6 +130,7 @@
> > >gEfiPropertiesTableGuid   ##
> > > SOMETIMES_PRODUCES   ## SystemTable
> > >gEfiMemoryAttributesTableGuid ##
> > > SOMETIMES_PRODUCES   ## SystemTable
> > >gEfiEndOfDxeEventGroupGuid##
> > > SOMETIMES_CONSUMES   ## Event
> > > +  gEfiHobMemoryAllocStackGuid   ##
> > > SOMETIMES_CONSUMES   ## SystemTable
> > >
> > >  [Ppis]
> > >gEfiVectorHandoffInfoPpiGuid  ## UNDEFINED # HOB
> > > @@ -198,6 +199,7 @@
> > >gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType
> > > ## CONSUMES
> > >gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType
> > > ## CONSUMES
> > >gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
> > > ## CONSUMES
> > > +  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
> > > ## CONSUMES
> > >
> > >  # [Hob]
> > >  # RESOURCE_DESCRIPTOR   ## CONSUMES
> > > diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> > > 

Re: [edk2] [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard

[Yao, Jiewen]

Agree.

With this update, reviewed-by: jiewen@intel.com


> -Original Message-
> From: Zeng, Star
> Sent: Tuesday, March 6, 2018 8:16 PM
> To: Wu, Hao A ; edk2-devel@lists.01.org
> Cc: Wang, Jian J ; Dong, Eric ;
> Yao, Jiewen ; Ni, Ruiyu ; Zeng, Star
> 
> Subject: RE: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX
> and Stack guard
> 
> A quick minor comment.
> GetHobList() could be used instead of EfiGetSystemConfigurationTable
> (, ).
> 
> Thanks,
> Star
> -Original Message-
> From: Wu, Hao A
> Sent: Tuesday, March 6, 2018 8:11 PM
> To: edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Wang, Jian J ;
> Zeng, Star ; Dong, Eric ; Yao,
> Jiewen ; Ni, Ruiyu 
> Subject: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and
> Stack guard
> 
> If enabled, NX memory protection feature will mark some types of active
> memory as NX (non-executable), which includes the first page of the stack.
> This will overwrite the attributes of the first page of the stack if the 
> stack guard
> feature is also enabled.
> 
> The solution is to override the attributes setting to the first page of the 
> stack by
> adding back the 'EFI_MEMORY_RP' attribute when the stack guard feature is
> enabled.
> 
> Cc: Jian J Wang 
> Cc: Star Zeng 
> Cc: Eric Dong 
> Cc: Jiewen Yao 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Hao Wu 
> ---
>  MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
>  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 61
> +++
>  2 files changed, 64 insertions(+), 1 deletion(-)
> 
> diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf
> b/MdeModulePkg/Core/Dxe/DxeMain.inf
> index 7334780326..d2e7360ed4 100644
> --- a/MdeModulePkg/Core/Dxe/DxeMain.inf
> +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
> @@ -3,7 +3,7 @@
>  #
>  #  It provides an implementation of DXE Core that is compliant with DXE CIS.
>  #
> -#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
> +#  Copyright (c) 2006 - 2018, Intel Corporation. All rights
> +reserved.
>  #  This program and the accompanying materials  #  are licensed and made
> available under the terms and conditions of the BSD License  #  which
> accompanies this distribution.  The full text of the license may be found at 
> @@
> -130,6 +130,7 @@
>gEfiPropertiesTableGuid   ##
> SOMETIMES_PRODUCES   ## SystemTable
>gEfiMemoryAttributesTableGuid ##
> SOMETIMES_PRODUCES   ## SystemTable
>gEfiEndOfDxeEventGroupGuid##
> SOMETIMES_CONSUMES   ## Event
> +  gEfiHobMemoryAllocStackGuid   ##
> SOMETIMES_CONSUMES   ## SystemTable
> 
>  [Ppis]
>gEfiVectorHandoffInfoPpiGuid  ## UNDEFINED # HOB
> @@ -198,6 +199,7 @@
>gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType
> ## CONSUMES
>gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType
> ## CONSUMES
>gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask
> ## CONSUMES
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard
> ## CONSUMES
> 
>  # [Hob]
>  # RESOURCE_DESCRIPTOR   ## CONSUMES
> diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> index a2ea445eef..a6de22d3af 100644
> --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
> @@ -801,6 +801,11 @@ InitializeDxeNxMemoryProtectionPolicy (
>UINT64Attributes;
>LIST_ENTRY*Link;
>EFI_GCD_MAP_ENTRY *Entry;
> +  VOID  *HobList;
> +  EFI_PEI_HOB_POINTERS  Hob;
> +  EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
> +  EFI_PHYSICAL_ADDRESS  StackBase;
> +  BOOLEAN   StackBaseFound;
> 
>//
>// Get the EFI memory map.
> @@ -832,6 +837,45 @@ InitializeDxeNxMemoryProtectionPolicy (
>} while (Status == EFI_BUFFER_TOO_SMALL);
>ASSERT_EFI_ERROR (Status);
> 
> +  StackBase = 0;
> +  StackBaseFound = FALSE;
> +  if (PcdGetBool (PcdCpuStackGuard)) {
> +//
> +// Get the base of stack from Hob.
> +//
> +Status = EfiGetSystemConfigurationTable (, );
> +if (!EFI_ERROR (Status)) {
> +  for (Hob.Raw = HobList; !END_OF_HOB_LIST (Hob); Hob.Raw =
> GET_NEXT_HOB (Hob)) {
> +if (GET_HOB_TYPE(Hob) == EFI_HOB_TYPE_MEMORY_ALLOCATION)
> {
> +  MemoryHob = Hob.MemoryAllocation;
> +  if (CompareGuid(,
> >AllocDescriptor.Name)) {
> +DEBUG ((
> +  DEBUG_INFO,
> +  

Re: [edk2] [PATCH 00/20] OvmfPkg: SEV: decrypt the initial SMRAM save state map for SMBASE relocation

[Laszlo Ersek]

On 03/02/18 16:21, Brijesh Singh wrote:
> 
> 
> On 03/01/2018 06:03 PM, Laszlo Ersek wrote:
> ...
> 
>>
>> I regression-tested my usual non-SEV guests (with Brijesh's v2 2/2 patch
>> applied on top, from the above-referenced series), which covers i440fx
>> (no SMM, X64), q35 (SMM, IA32 and IA32X64), Fedora and Windows, normal
>> boot and S3.
>>
>> I also tried to test the series with SEV guests (again with Brijesh's v2
>> 2/2 patch applied on top). Unfortunately, I didn't get good results with
>> or without SMM. Without SMM, the guest OS boots to a point, but then it
>> gets stuck with the CPU spinning. With SMM, OVMF gets stuck in SMBASE
>> relocation.
>>
>> I should mention however that my SEV host setup dates back to November
>> 2017, including host kernel, QEMU and guest OS. I suppose all those
>> components have seen many changes since, on the respective upstream
>> lists. I'll have to work with Brijesh to update my SEV host to the
>> latest bits.
>>
>> Until then, Brijesh, can you please test this series? Thank you!
>>
> 
> 
> I tested the series on SEV and non SEV guest (with my v2 2/2 patch
> applied on top of this series). The result is positive,  I am able boot
> both SMM and non SMM enabled BIOS.
> 
> Once again, thank you so much for cleanup.
> 
> I will submit by v2 2/2 patch after your series is pushed to the tree.
> 
> Tested-by: Brijesh Singh 
> Reviewed-by: Brijesh Singh 

Series pushed: 66f2329446db..5e2e5647b9fb.

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

Re: [edk2] [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard

[Zeng, Star]

A quick minor comment.
GetHobList() could be used instead of EfiGetSystemConfigurationTable 
(, ).

Thanks,
Star
-Original Message-
From: Wu, Hao A 
Sent: Tuesday, March 6, 2018 8:11 PM
To: edk2-devel@lists.01.org
Cc: Wu, Hao A ; Wang, Jian J ; Zeng, 
Star ; Dong, Eric ; Yao, Jiewen 
; Ni, Ruiyu 
Subject: [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and 
Stack guard

If enabled, NX memory protection feature will mark some types of active memory 
as NX (non-executable), which includes the first page of the stack.
This will overwrite the attributes of the first page of the stack if the stack 
guard feature is also enabled.

The solution is to override the attributes setting to the first page of the 
stack by adding back the 'EFI_MEMORY_RP' attribute when the stack guard feature 
is enabled.

Cc: Jian J Wang 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 61 +++
 2 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf 
b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 7334780326..d2e7360ed4 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -3,7 +3,7 @@
 #
 #  It provides an implementation of DXE Core that is compliant with DXE CIS.
 #
-#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2018, Intel Corporation. All rights 
+reserved.
 #  This program and the accompanying materials  #  are licensed and made 
available under the terms and conditions of the BSD License  #  which 
accompanies this distribution.  The full text of the license may be found at @@ 
-130,6 +130,7 @@
   gEfiPropertiesTableGuid   ## SOMETIMES_PRODUCES   ## 
SystemTable
   gEfiMemoryAttributesTableGuid ## SOMETIMES_PRODUCES   ## 
SystemTable
   gEfiEndOfDxeEventGroupGuid## SOMETIMES_CONSUMES   ## 
Event
+  gEfiHobMemoryAllocStackGuid   ## SOMETIMES_CONSUMES   ## 
SystemTable
 
 [Ppis]
   gEfiVectorHandoffInfoPpiGuid  ## UNDEFINED # HOB
@@ -198,6 +199,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType   ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType   ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask   ## 
CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard   ## 
CONSUMES
 
 # [Hob]
 # RESOURCE_DESCRIPTOR   ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index a2ea445eef..a6de22d3af 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -801,6 +801,11 @@ InitializeDxeNxMemoryProtectionPolicy (
   UINT64Attributes;
   LIST_ENTRY*Link;
   EFI_GCD_MAP_ENTRY *Entry;
+  VOID  *HobList;
+  EFI_PEI_HOB_POINTERS  Hob;
+  EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
+  EFI_PHYSICAL_ADDRESS  StackBase;
+  BOOLEAN   StackBaseFound;
 
   //
   // Get the EFI memory map.
@@ -832,6 +837,45 @@ InitializeDxeNxMemoryProtectionPolicy (
   } while (Status == EFI_BUFFER_TOO_SMALL);
   ASSERT_EFI_ERROR (Status);
 
+  StackBase = 0;
+  StackBaseFound = FALSE;
+  if (PcdGetBool (PcdCpuStackGuard)) {
+//
+// Get the base of stack from Hob.
+//
+Status = EfiGetSystemConfigurationTable (, );
+if (!EFI_ERROR (Status)) {
+  for (Hob.Raw = HobList; !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB 
(Hob)) {
+if (GET_HOB_TYPE(Hob) == EFI_HOB_TYPE_MEMORY_ALLOCATION) {
+  MemoryHob = Hob.MemoryAllocation;
+  if (CompareGuid(, 
>AllocDescriptor.Name)) {
+DEBUG ((
+  DEBUG_INFO,
+  "%a: StackBase = 0x%016lx  StackSize = 0x%016lx\n",
+  __FUNCTION__,
+  MemoryHob->AllocDescriptor.MemoryBaseAddress,
+  MemoryHob->AllocDescriptor.MemoryLength
+  ));
+
+StackBase = MemoryHob->AllocDescriptor.MemoryBaseAddress;
+//
+// Ensure the base of the stack is page-size aligned.
+//
+ASSERT ((StackBase & EFI_PAGE_MASK) == 0);
+StackBaseFound = TRUE;
+break;
+  }
+}
+  }
+}
+
+//
+// Ensure the base of stack can be found from Hob when stack 

[edk2] [PATCH 1/2] MdeModulePkg/Core: Refine handling NULL detection in NX setting

[Hao Wu]

The commit rewrites the logic in function
InitializeDxeNxMemoryProtectionPolicy() for handling the first page
(page 0) when NULL pointer detection feature is enabled.

Instead of skip setting the page 0, the codes will now override the
attribute setting of page 0 by adding the 'EFI_MEMORY_RP' attribute.

The purpose is to make it easy for other special handlings of pages
(e.g. the first page of the stack when stack guard feature is enabled).

Cc: Jian J Wang 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index 455ed35f9a..a2ea445eef 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -19,7 +19,7 @@
 
   Once the image is unloaded, the protection is removed automatically.
 
-Copyright (c) 2017, Intel Corporation. All rights reserved.
+Copyright (c) 2017 - 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
 which accompanies this distribution.  The full text of the license may be 
found at
@@ -846,23 +846,23 @@ InitializeDxeNxMemoryProtectionPolicy (
 
 Attributes = GetPermissionAttributeForMemoryType (MemoryMapEntry->Type);
 if (Attributes != 0) {
+  SetUefiImageMemoryAttributes (
+MemoryMapEntry->PhysicalStart,
+LShiftU64 (MemoryMapEntry->NumberOfPages, EFI_PAGE_SHIFT),
+Attributes);
+
   if (MemoryMapEntry->PhysicalStart == 0 &&
   PcdGet8 (PcdNullPointerDetectionPropertyMask) != 0) {
 
 ASSERT (MemoryMapEntry->NumberOfPages > 0);
 //
-// Skip page 0 if NULL pointer detection is enabled to avoid attributes
-// overwritten.
+// Add EFI_MEMORY_RP attribute for page 0 if NULL pointer detection is
+// enabled.
 //
 SetUefiImageMemoryAttributes (
-  MemoryMapEntry->PhysicalStart + EFI_PAGE_SIZE,
-  LShiftU64 (MemoryMapEntry->NumberOfPages - 1, EFI_PAGE_SHIFT),
-  Attributes);
-  } else {
-SetUefiImageMemoryAttributes (
-  MemoryMapEntry->PhysicalStart,
-  LShiftU64 (MemoryMapEntry->NumberOfPages, EFI_PAGE_SHIFT),
-  Attributes);
+  0,
+  EFI_PAGES_TO_SIZE (1),
+  EFI_MEMORY_RP | Attributes);
   }
 }
 MemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize);
-- 
2.12.0.windows.1

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

[edk2] [PATCH 0/2] Resolve feature conflict between NX and Stack guard

[Hao Wu]

If enabled, NX memory protection feature will mark some types of active
memory as NX (non-executable), which includes the first page of the stack.
This will overwrite the attributes of the first page of the stack if the
stack guard feature is also enabled.

The series will override the attributes setting to the first page of the
stack by adding back the 'EFI_MEMORY_RP' attribute when the stack guard
feature is enabled.

Cc: Jian J Wang 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 

Hao Wu (2):
  MdeModulePkg/Core: Refine handling NULL detection in NX setting
  MdeModulePkg/Core: Fix feature conflict between NX and Stack guard

 MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 81 +++
 2 files changed, 74 insertions(+), 11 deletions(-)

-- 
2.12.0.windows.1

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

[edk2] [PATCH 2/2] MdeModulePkg/Core: Fix feature conflict between NX and Stack guard

[Hao Wu]

If enabled, NX memory protection feature will mark some types of active
memory as NX (non-executable), which includes the first page of the stack.
This will overwrite the attributes of the first page of the stack if the
stack guard feature is also enabled.

The solution is to override the attributes setting to the first page of
the stack by adding back the 'EFI_MEMORY_RP' attribute when the stack
guard feature is enabled.

Cc: Jian J Wang 
Cc: Star Zeng 
Cc: Eric Dong 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Hao Wu 
---
 MdeModulePkg/Core/Dxe/DxeMain.inf |  4 +-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 61 +++
 2 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf 
b/MdeModulePkg/Core/Dxe/DxeMain.inf
index 7334780326..d2e7360ed4 100644
--- a/MdeModulePkg/Core/Dxe/DxeMain.inf
+++ b/MdeModulePkg/Core/Dxe/DxeMain.inf
@@ -3,7 +3,7 @@
 #
 #  It provides an implementation of DXE Core that is compliant with DXE CIS.
 #  
-#  Copyright (c) 2006 - 2017, Intel Corporation. All rights reserved.
+#  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
 #  This program and the accompanying materials
 #  are licensed and made available under the terms and conditions of the BSD 
License
 #  which accompanies this distribution.  The full text of the license may be 
found at
@@ -130,6 +130,7 @@
   gEfiPropertiesTableGuid   ## SOMETIMES_PRODUCES   ## 
SystemTable
   gEfiMemoryAttributesTableGuid ## SOMETIMES_PRODUCES   ## 
SystemTable
   gEfiEndOfDxeEventGroupGuid## SOMETIMES_CONSUMES   ## 
Event
+  gEfiHobMemoryAllocStackGuid   ## SOMETIMES_CONSUMES   ## 
SystemTable
 
 [Ppis]
   gEfiVectorHandoffInfoPpiGuid  ## UNDEFINED # HOB
@@ -198,6 +199,7 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType   ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType   ## 
CONSUMES
   gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask   ## 
CONSUMES
+  gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard   ## 
CONSUMES
 
 # [Hob]
 # RESOURCE_DESCRIPTOR   ## CONSUMES
diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
index a2ea445eef..a6de22d3af 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c
@@ -801,6 +801,11 @@ InitializeDxeNxMemoryProtectionPolicy (
   UINT64Attributes;
   LIST_ENTRY*Link;
   EFI_GCD_MAP_ENTRY *Entry;
+  VOID  *HobList;
+  EFI_PEI_HOB_POINTERS  Hob;
+  EFI_HOB_MEMORY_ALLOCATION *MemoryHob;
+  EFI_PHYSICAL_ADDRESS  StackBase;
+  BOOLEAN   StackBaseFound;
 
   //
   // Get the EFI memory map.
@@ -832,6 +837,45 @@ InitializeDxeNxMemoryProtectionPolicy (
   } while (Status == EFI_BUFFER_TOO_SMALL);
   ASSERT_EFI_ERROR (Status);
 
+  StackBase = 0;
+  StackBaseFound = FALSE;
+  if (PcdGetBool (PcdCpuStackGuard)) {
+//
+// Get the base of stack from Hob.
+//
+Status = EfiGetSystemConfigurationTable (, );
+if (!EFI_ERROR (Status)) {
+  for (Hob.Raw = HobList; !END_OF_HOB_LIST (Hob); Hob.Raw = GET_NEXT_HOB 
(Hob)) {
+if (GET_HOB_TYPE(Hob) == EFI_HOB_TYPE_MEMORY_ALLOCATION) {
+  MemoryHob = Hob.MemoryAllocation;
+  if (CompareGuid(, 
>AllocDescriptor.Name)) {
+DEBUG ((
+  DEBUG_INFO,
+  "%a: StackBase = 0x%016lx  StackSize = 0x%016lx\n",
+  __FUNCTION__,
+  MemoryHob->AllocDescriptor.MemoryBaseAddress,
+  MemoryHob->AllocDescriptor.MemoryLength
+  ));
+
+StackBase = MemoryHob->AllocDescriptor.MemoryBaseAddress;
+//
+// Ensure the base of the stack is page-size aligned.
+//
+ASSERT ((StackBase & EFI_PAGE_MASK) == 0);
+StackBaseFound = TRUE;
+break;
+  }
+}
+  }
+}
+
+//
+// Ensure the base of stack can be found from Hob when stack guard is
+// enabled.
+//
+ASSERT (StackBaseFound);
+  }
+
   DEBUG ((
 DEBUG_INFO,
 "%a: applying strict permissions to active memory regions\n",
@@ -864,6 +908,23 @@ InitializeDxeNxMemoryProtectionPolicy (
   EFI_PAGES_TO_SIZE (1),
   EFI_MEMORY_RP | Attributes);
   }
+
+  if (StackBaseFound &&
+  (StackBase >= MemoryMapEntry->PhysicalStart &&
+   StackBase <  MemoryMapEntry->PhysicalStart +
+LShiftU64 (MemoryMapEntry->NumberOfPages, 

Re: [edk2] [PATCH v4 0/2] quirks handling for SDHCI controllers

[Meenakshi Aggarwal]

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Tuesday, March 06, 2018 4:41 PM
> To: Meenakshi Aggarwal 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH v4 0/2] quirks handling for SDHCI controllers
> 
> On 6 March 2018 at 05:14, Meenakshi Aggarwal
>  wrote:
> > Hi,
> >
> > I am using Mmc Driver implemented in "
> EmbeddedPkg/Universal/MmcDxe/" for my SD/MMC controller and my
> controller is not on PCI bus.
> >
> > I am a bit confused if i should move to SD implementation available in
> 'MdeModulePkg\Bus\Pci\SdMmcPciHcDxe".
> >
> > Please suggest.
> >
> 
> It really depends on whether the IP is SDHCI compatible or not.

Thanks for reply Ard, its not SDHCI compatible, so i guess no additional 
advantage in moving to 'MdeModulePkg\Bus\Pci\SdMmcPciHcDxe' for my IP.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH edk2-platforms v2 06/18] ARM/VExpressPkg: Add and update debug ASSERTS

[Ard Biesheuvel]

On 5 March 2018 at 15:08, Evan Lloyd  wrote:
> In that case, would you be happy to take Girish's patches with the ASSERTs 
> done your (our) way?
> Leif can fulminate about it when he gets back, if he really feels that 
> strongly.

Why? Because it is almost the end of the quarter? Can't it wait?

> I suspect, though, that Leif is capable of being reasonable if pressed (and 
> offered beer at Plugfest).
>

I am not going to push something I know Leif disapproves of in its
current form in his absence, sorry.

>
>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: 02 March 2018 19:07
>> To: Evan Lloyd 
>> Cc: leif.lindh...@linaro.org; Girish Pathak ;
>> Matteo Carlini ; nd ; edk2-
>> de...@lists.01.org
>> Subject: Re: [edk2] [PATCH edk2-platforms v2 06/18] ARM/VExpressPkg:
>> Add and update debug ASSERTS
>>
>> On 28 February 2018 at 20:27, Evan Lloyd  wrote:
>> > Hi Leif, Ard.
>> > Can I get you two argue out the pros and cons of the "ASSERT(FALSE)"
>> debate, please.
>>
>> I can argue the cons if you like. For the pros, you'll have to wait for Leif 
>> to
>> return from holiday (in a week or two AFAIK)
>>
>> > (see
>> > https://lists.01.org/pipermail/edk2-devel/2018-January/019788.html)
>> > For what it is worth, our (surprisingly unanimous) opinion is that, since
>> the ASSERT is only there to help spot a problem, then the more information
>> reported the better.  The only benefits of ASSERT(FALSE) would be a smaller
>> debug image and minor efficiency improvement on the path to the crash.
>> >
>> > Regards,
>> > Evan
>> >
>> >> -Original Message-
>> >> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> >> Sent: 04 January 2018 19:55
>> >> To: Evan Lloyd 
>> >> Cc: Girish Pathak ; Matteo Carlini
>> >> ; nd ; edk2-
>> de...@lists.01.org;
>> >> Thomas Abraham ; Arvind Chauhan
>> >> ; leif.lindh...@linaro.org
>> >> Subject: Re: [edk2] [PATCH edk2-platforms v2 06/18] ARM/VExpressPkg:
>> >> Add and update debug ASSERTS
>> >>
>> >> On 4 January 2018 at 19:51, Evan Lloyd  wrote:
>> >> >
>> >> >
>> >> >> -Original Message-
>> >> >> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> >> >> Sent: 04 January 2018 19:24
>> >> >> To: Girish Pathak 
>> >> >> Cc: Evan Lloyd ; Matteo Carlini
>> >> >> ; nd ; edk2-
>> >> de...@lists.01.org;
>> >> >> Thomas Abraham ; Arvind Chauhan
>> >> >> ; leif.lindh...@linaro.org
>> >> >> Subject: Re: [edk2] [PATCH edk2-platforms v2 06/18]
>> ARM/VExpressPkg:
>> >> >> Add and update debug ASSERTS
>> >> >>
>> >> >> On 4 January 2018 at 18:55, Girish Pathak 
>> >> >> wrote:
>> >> >> > Hi Ard,
>> >> >> >
>> >> >> >> -Original Message-
>> >> >> >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On
>> >> >> >> Behalf Of Ard Biesheuvel
>> >> >> >> Sent: 23 December 2017 14:12
>> >> >> >> To: Evan Lloyd 
>> >> >> >> Cc: "matteo.carl...@arm.com"@arm.com;
>> >> >> >> "leif.lindh...@linaro.org"@arm.com; "n...@arm.com"@arm.com;
>> >> edk2-
>> >> >> >> de...@lists.01.org; Thomas Abraham
>> ;
>> >> >> Arvind
>> >> >> >> Chauhan ;
>> >> >> "ard.biesheu...@linaro.org"@arm.com
>> >> >> >> Subject: Re: [edk2] [PATCH edk2-platforms v2 06/18]
>> >> ARM/VExpressPkg:
>> >> >> >> Add and update debug ASSERTS
>> >> >> >>
>> >> >> >> On 22 December 2017 at 19:08,   wrote:
>> >> >> >> > From: Girish Pathak 
>> >> >> >> >
>> >> >> >> > This change adds some debug assertions e.g to catch NULL
>> >> >> >> > pointer errors missing in PL11Lcd and HdLcd platform libraries.
>> >> >> >> >
>> >> >> >> > Contributed-under: TianoCore Contribution Agreement 1.1
>> >> >> >> > Signed-off-by: Girish Pathak 
>> >> >> >> > Signed-off-by: Evan Lloyd 
>> >> >> >> > ---
>> >> >> >> >
>> >> >> >>
>> >> >>
>> >>
>> Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVExp
>> >> >> r
>> >> >> >> ess.c   | 22 +-
>> >> >> >> >
>> >> >> >> >
>> >> >> >>
>> >> >>
>> >>
>> Platform/ARM/VExpressPkg/Library/PL111LcdArmVExpressLib/PL111LcdAr
>> >> >> m
>> >> >> >> VEx
>> >> >> >> > press.c | 24 +++-
>> >> >> >> >  2 files changed, 44 insertions(+), 2 deletions(-)
>> >> >> >> >
>> >> >> >> > diff --git
>> >> >> >> >
>> >> >> >>
>> >> >>
>> >>
>> a/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVE
>> >> >> x
>> >> >> >> pres
>> >> >> >> > s.c
>> >> >> >> >
>> >> >> >>
>> >> >>
>> >>
>> b/Platform/ARM/VExpressPkg/Library/HdLcdArmVExpressLib/HdLcdArmVE

Re: [edk2] [PATCH v4 0/2] quirks handling for SDHCI controllers

[Ard Biesheuvel]

On 6 March 2018 at 05:14, Meenakshi Aggarwal  wrote:
> Hi,
>
> I am using Mmc Driver implemented in " EmbeddedPkg/Universal/MmcDxe/" for my 
> SD/MMC controller and my controller is not on PCI bus.
>
> I am a bit confused if i should move to SD implementation available in 
> 'MdeModulePkg\Bus\Pci\SdMmcPciHcDxe".
>
> Please suggest.
>

It really depends on whether the IP is SDHCI compatible or not.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 3/7] HACK: HobLib: workaround infinite loop

[Laszlo Ersek]

On 03/06/18 01:45, Brian J. Johnson wrote:
> On 03/05/2018 12:22 PM, Laszlo Ersek wrote:
>> PEIMs generally "execute in place" (XIP), i.e. they run from flash, not
>> RAM. In this status they use "temporary RAM" (e.g. CPU caches configured
>> like RAM) for stack & heap; whatever HOBs they produce are stored in
>> "temp RAM" as well. Then one of the PEIMs "discovers permanent RAM"
>> (basically it programs the memory controller and publishes the RAM
>> ranges). In turn the PEI core "migrates" PEIMs from temporary to
>> permanent RAM, including the HOB list.
>>
>> Before the temporary RAM migration (when still executing in place from
>> flash), PEIMs cannot have writeable global variables. For example,
>> dynamic PCDs are also maintained in a HOB (the PCD HOB).
>>
>> A PEIM normally cannot (and shouldn't) tell whether it is dispatched
>> before or after permanent RAM is published. If needed, a PEIM can
>> advertise that it depends on permanent RAM (for example because it needs
>> a lot of heap memory) by including "gEfiPeiMemoryDiscoveredPpiGuid" in
>> its DEPEX.
>>
>> Finally, it seems like a PEIM can also express, "I'm fine with being
>> dispatched from both flash (XIP) vs. permanent RAM, just the PEI core
>> tell me whichever it is". Apparently, if the PEIM is dispatched from
>> flash (before permanent RAM is available), its call to
>> RegisterForShadow() returns EFI_SUCCESS (and then its entry point
>> function will be invoked for a 2nd time, after the temp RAM migration).
>> And when a PEIM is dispatched from RAM (either for the very first time,
>> or for the second time, after being dispatched from flash first), the
>> same call returns EFI_ALREADY_STARTED.
>>
>> Honestly, I'm unsure what this is good for (both in general, and
>> specifically for Tcg2Pei). Apparently, Tcg2Pei needs permanent RAM for
>> doing the measurements (which makes sense); I just wonder what exactly
>> it does so that it cannot simply specify
>> "gEfiPeiMemoryDiscoveredPpiGuid" in its DEPEX.
> 
> I haven't looked at this particular PEIM.  But one case where
> registering for shadowing is useful is for improving performance when
> running from permanent RAM, where writable global variables are
> available.  For instance, when running from flash, a ReportStatusCode
> PEIM may need to go through a slow process to locate an output hardware
> device on every PPI call.  This may involve traversing the HOB list,
> consulting other PPIs, even probing hardware addresses.  But once it's
> shadowed to RAM, it can locate the device once, then cache its address
> in a global.  Not to mention that the code itself is far, far faster
> when run from RAM vs. flash.  (That's probably a key difference between
> a real machine and a VM.)

Ah, this sounds like a great example. Status code reporting is useful /
important for debugging even before permanent RAM is installed, so the
PEIM providing that PPI should not have a depex on
gEfiPeiMemoryDiscoveredPpiGuid. However, once the permanent RAM is
published, it makes sense to speed up the implementation. Thanks for
this example!

> Also, I've personally written a PEIM which saves a bunch of internal
> state in a HOB, since that's the main writable storage when running from
> flash.  That state includes pointers to other data (in flash.)  Once the
> data is all shadowed to RAM, it updates the HOB to point to the data in
> RAM.  That way it's a lot faster to access the data.

Another good example. (I think I've been generally blind to this perf
difference between flash and RAM, specifically concerning execution -- I
must have been spoiled by virt, as you say :) )

> I also have other PEIMs which are constrained (via DEPEX) to run *only*
> from RAM, since they have larger memory requirements than can be
> satisfied from temporary cache-as-RAM.  That's certainly a valid
> technique as well.

Right, that's quite known to OVMF too, due to CpuMpPei being "hungry"
like this.

> RegisterForShadow() is a useful tool for making the most of the
> restricted PEI environment.  And having it standardized like this is, as
> Andrew said, a lot better than the hacks people had to use beforehand.

I agree. I'm happy to have learned about this service!

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