[edk2] [PATCH 6/6] MdeModulePkg/DxeCore: Not update RtCode in MemAttrTable after EndOfDxe

[Hao Wu]

From: Jiewen Yao 

We want to provide precise info in MemAttribTable
to both OS and SMM, and SMM only gets the info at EndOfDxe.
So we do not update RtCode entry in EndOfDxe.

The impact is that if 3rd part OPROM is runtime, it cannot be executed
at UEFI runtime phase.
Currently, we do not see compatibility issue, because the only runtime
OPROM we found before in UNDI, and UEFI OS will not use UNDI interface
in OS.

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 13 +
 1 file changed, 13 insertions(+)

diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c 
b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
index a84507df95..a96d442fbc 100644
--- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c
@@ -62,6 +62,8 @@ EFI_LOCK   mPropertiesTableLock = 
EFI_INITIALIZE_LOCK_VARIABLE (TPL_NOTI
 
 BOOLEANmPropertiesTableEnable;
 
+BOOLEANmPropertiesTableEndOfDxe = FALSE;
+
 //
 // Below functions are for MemoryMap
 //
@@ -1079,6 +1081,11 @@ InsertImageRecord (
   DEBUG ((EFI_D_VERBOSE, "InsertImageRecord - 0x%x\n", RuntimeImage));
   DEBUG ((EFI_D_VERBOSE, "InsertImageRecord - 0x%016lx - 0x%016lx\n", 
(EFI_PHYSICAL_ADDRESS)(UINTN)RuntimeImage->ImageBase, RuntimeImage->ImageSize));
 
+  if (mPropertiesTableEndOfDxe) {
+DEBUG ((DEBUG_INFO, "Do not insert runtime image record after 
EndOfDxe\n"));
+return ;
+  }
+
   ImageRecord = AllocatePool (sizeof(*ImageRecord));
   if (ImageRecord == NULL) {
 return ;
@@ -1296,6 +1303,11 @@ RemoveImageRecord (
   DEBUG ((EFI_D_VERBOSE, "RemoveImageRecord - 0x%x\n", RuntimeImage));
   DEBUG ((EFI_D_VERBOSE, "RemoveImageRecord - 0x%016lx - 0x%016lx\n", 
(EFI_PHYSICAL_ADDRESS)(UINTN)RuntimeImage->ImageBase, RuntimeImage->ImageSize));
 
+  if (mPropertiesTableEndOfDxe) {
+DEBUG ((DEBUG_INFO, "Do not remove runtime image record after 
EndOfDxe\n"));
+return ;
+  }
+
   ImageRecord = FindImageRecord 
((EFI_PHYSICAL_ADDRESS)(UINTN)RuntimeImage->ImageBase, RuntimeImage->ImageSize);
   if (ImageRecord == NULL) {
 DEBUG ((EFI_D_ERROR, " ImageRecord not found \n"));
@@ -1333,6 +1345,7 @@ InstallPropertiesTable (
   VOID*Context
   )
 {
+  mPropertiesTableEndOfDxe = TRUE;
   if (PcdGetBool (PcdPropertiesTableEnable)) {
 EFI_STATUS  Status;
 
-- 
2.16.2.windows.1

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

[edk2] [PATCH 3/6] MdeModulePkg/DxeCore: Install UEFI mem attrib table at EndOfDxe.

[Hao Wu]

From: Jiewen Yao 

So that the SMM can consume it to set page protection for
the UEFI runtime page with EFI_MEMORY_RO attribute.

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao 
---
 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c | 36 +++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c 
b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
index 43e5be8b54..60557faa1c 100644
--- a/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
+++ b/MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c
@@ -240,6 +240,23 @@ InstallMemoryAttributesTableOnReadyToBoot (
   mMemoryAttributesTableReadyToBoot = TRUE;
 }
 
+/**
+  Install initial MemoryAttributesTable on EndOfDxe.
+  Then SMM can consume this information.
+
+  @param[in] Event  The Event this notify function registered to.
+  @param[in] ContextPointer to the context data registered to the Event.
+**/
+VOID
+EFIAPI
+InstallMemoryAttributesTableOnEndOfDxe (
+  IN EFI_EVENT  Event,
+  IN VOID   *Context
+  )
+{
+  InstallMemoryAttributesTable ();
+}
+
 /**
   Initialize MemoryAttrubutesTable support.
 **/
@@ -251,18 +268,35 @@ CoreInitializeMemoryAttributesTable (
 {
   EFI_STATUS  Status;
   EFI_EVENT   ReadyToBootEvent;
+  EFI_EVENT   EndOfDxeEvent;
 
   //
   // Construct the table at ReadyToBoot.
   //
   Status = CoreCreateEventInternal (
  EVT_NOTIFY_SIGNAL,
- TPL_CALLBACK - 1,
+ TPL_CALLBACK,
  InstallMemoryAttributesTableOnReadyToBoot,
  NULL,
  ,
  
  );
   ASSERT_EFI_ERROR (Status);
+
+  //
+  // Construct the initial table at EndOfDxe,
+  // then SMM can consume this information.
+  // Use TPL_NOTIFY here, as such SMM code (TPL_CALLBACK)
+  // can run after it.
+  //
+  Status = CoreCreateEventInternal (
+ EVT_NOTIFY_SIGNAL,
+ TPL_NOTIFY,
+ InstallMemoryAttributesTableOnEndOfDxe,
+ NULL,
+ ,
+ 
+ );
+  ASSERT_EFI_ERROR (Status);
   return ;
 }
-- 
2.16.2.windows.1

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

[edk2] [PATCH 5/6] UefiCpuPkg/PiSmmCpu: Check EFI_RUNTIME_RO in UEFI mem attrib table.

[Hao Wu]

From: Jiewen Yao 

It treats the UEFI runtime page with EFI_MEMORY_RO attribute as
invalid SMM communication buffer.

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |  2 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |  1 +
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 75 +++-
 3 files changed, 75 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
index 1d016594e0..e3c7cff81c 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
@@ -28,6 +28,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 
 #include 
+#include 
 #include 
 
 #include 
@@ -45,6 +46,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf 
b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
index 52d8c55075..a7fb7b0b14 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
@@ -117,6 +117,7 @@
   gEfiAcpi20TableGuid  ## SOMETIMES_CONSUMES ## SystemTable
   gEfiAcpi10TableGuid  ## SOMETIMES_CONSUMES ## SystemTable
   gEdkiiPiSmmMemoryAttributesTableGuid ## CONSUMES ## SystemTable
+  gEfiMemoryAttributesTableGuid## CONSUMES ## SystemTable
 
 [FeaturePcd]
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmDebug ## CONSUMES
diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index b2ace6334e..7d99f23426 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -20,9 +20,6 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #define EFI_MEMORY_INITIALIZED  0x0200ULL
 #define EFI_MEMORY_TESTED   0x0400ULL
 
-#define NEXT_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \
-  ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) + (Size)))
-
 #define PREVIOUS_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \
   ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) - (Size)))
 
@@ -33,6 +30,8 @@ UINTN mUefiDescriptorSize;
 EFI_GCD_MEMORY_SPACE_DESCRIPTOR   *mGcdMemSpace   = NULL;
 UINTN mGcdMemNumberOfDesc = 0;
 
+EFI_MEMORY_ATTRIBUTES_TABLE  *mUefiMemoryAttributesTable = NULL;
+
 PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = {
   {Page4K,  SIZE_4KB, PAGING_4K_ADDRESS_MASK_64},
   {Page2M,  SIZE_2MB, PAGING_2M_ADDRESS_MASK_64},
@@ -1086,6 +1085,26 @@ GetGcdMemoryMap (
   gBS->FreePool (MemSpaceMap);
 }
 
+/**
+  Get UEFI MemoryAttributesTable.
+**/
+VOID
+GetUefiMemoryAttributesTable (
+  VOID
+  )
+{
+  EFI_STATUS   Status;
+  EFI_MEMORY_ATTRIBUTES_TABLE  *MemoryAttributesTable;
+  UINTNMemoryAttributesTableSize;
+
+  Status = EfiGetSystemConfigurationTable (, 
(VOID **));
+  if (!EFI_ERROR (Status)) {
+MemoryAttributesTableSize = sizeof(EFI_MEMORY_ATTRIBUTES_TABLE) + 
MemoryAttributesTable->DescriptorSize * MemoryAttributesTable->NumberOfEntries;
+mUefiMemoryAttributesTable = AllocateCopyPool (MemoryAttributesTableSize, 
MemoryAttributesTable);
+ASSERT (mUefiMemoryAttributesTable != NULL);
+  }
+}
+
 /**
   This function caches the UEFI memory map information.
 **/
@@ -1150,6 +1169,11 @@ GetUefiMemoryMap (
   // Get additional information from GCD memory map.
   //
   GetGcdMemoryMap ();
+
+  //
+  // Get UEFI memory attributes table.
+  //
+  GetUefiMemoryAttributesTable ();
 }
 
 /**
@@ -1168,6 +1192,7 @@ SetUefiMemMapAttributes (
   EFI_MEMORY_DESCRIPTOR *MemoryMap;
   UINTN MemoryMapEntryCount;
   UINTN Index;
+  EFI_MEMORY_DESCRIPTOR *Entry;
 
   DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n"));
 
@@ -1218,6 +1243,35 @@ SetUefiMemMapAttributes (
   //
   // Do not free mGcdMemSpace, it will be checked in 
IsSmmCommBufferForbiddenAddress().
   //
+
+  //
+  // Set UEFI runtime memory with EFI_MEMORY_RO as not present.
+  //
+  if (mUefiMemoryAttributesTable != NULL) {
+Entry = (EFI_MEMORY_DESCRIPTOR *)(mUefiMemoryAttributesTable + 1);
+for (Index = 0; Index < mUefiMemoryAttributesTable->NumberOfEntries; 
Index++) {
+  if (Entry->Type == EfiRuntimeServicesCode || Entry->Type == 
EfiRuntimeServicesData) {
+if ((Entry->Attribute & EFI_MEMORY_RO) != 0) {
+  Status = SmmSetMemoryAttributes (
+ Entry->PhysicalStart,
+ EFI_PAGES_TO_SIZE((UINTN)Entry->NumberOfPages),
+ EFI_MEMORY_RP
+ );
+  DEBUG ((
+DEBUG_INFO,
+

[edk2] [PATCH 4/6] MdePkg/SmmMemLib: Check EFI_MEMORY_RO in UEFI mem attrib table.

[Hao Wu]

From: Jiewen Yao 

It treats the UEFI runtime page with EFI_MEMORY_RO attribute as
invalid SMM communication buffer.

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao 
---
 MdePkg/Library/SmmMemLib/SmmMemLib.c   | 58 +++-
 MdePkg/Library/SmmMemLib/SmmMemLib.inf |  4 ++
 2 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/MdePkg/Library/SmmMemLib/SmmMemLib.c 
b/MdePkg/Library/SmmMemLib/SmmMemLib.c
index 3f79e46d46..3409ddf87c 100644
--- a/MdePkg/Library/SmmMemLib/SmmMemLib.c
+++ b/MdePkg/Library/SmmMemLib/SmmMemLib.c
@@ -27,10 +27,12 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 //
 // attributes for reserved memory before it is promoted to system memory
@@ -39,9 +41,6 @@
 #define EFI_MEMORY_INITIALIZED  0x0200ULL
 #define EFI_MEMORY_TESTED   0x0400ULL
 
-#define NEXT_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \
-  ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) + (Size)))
-
 EFI_SMRAM_DESCRIPTOR *mSmmMemLibInternalSmramRanges;
 UINTNmSmmMemLibInternalSmramCount;
 
@@ -57,6 +56,8 @@ UINTN mDescriptorSize;
 EFI_GCD_MEMORY_SPACE_DESCRIPTOR   *mSmmMemLibGcdMemSpace   = NULL;
 UINTN mSmmMemLibGcdMemNumberOfDesc = 0;
 
+EFI_MEMORY_ATTRIBUTES_TABLE  *mSmmMemLibMemoryAttributesTable = NULL;
+
 VOID  *mRegistrationEndOfDxe;
 VOID  *mRegistrationReadyToLock;
 
@@ -204,6 +205,32 @@ SmmIsBufferOutsideSmmValid (
 return FALSE;
   }
 }
+
+//
+// Check UEFI runtime memory with EFI_MEMORY_RO as invalid communication 
buffer.
+//
+if (mSmmMemLibMemoryAttributesTable != NULL) {
+  EFI_MEMORY_DESCRIPTOR *Entry;
+
+  Entry = (EFI_MEMORY_DESCRIPTOR *)(mSmmMemLibMemoryAttributesTable + 1);
+  for (Index = 0; Index < 
mSmmMemLibMemoryAttributesTable->NumberOfEntries; Index++) {
+if (Entry->Type == EfiRuntimeServicesCode || Entry->Type == 
EfiRuntimeServicesData) {
+  if ((Entry->Attribute & EFI_MEMORY_RO) != 0) {
+if (((Buffer >= Entry->PhysicalStart) && (Buffer < 
Entry->PhysicalStart + LShiftU64 (Entry->NumberOfPages, EFI_PAGE_SHIFT))) ||
+((Entry->PhysicalStart >= Buffer) && (Entry->PhysicalStart < 
Buffer + Length))) {
+  DEBUG ((
+EFI_D_ERROR,
+"SmmIsBufferOutsideSmmValid: In RuntimeCode Region: Buffer 
(0x%lx) - Length (0x%lx)\n",
+Buffer,
+Length
+));
+  return FALSE;
+}
+  }
+}
+Entry = NEXT_MEMORY_DESCRIPTOR (Entry, 
mSmmMemLibMemoryAttributesTable->DescriptorSize);
+  }
+}
   }
   return TRUE;
 }
@@ -399,6 +426,26 @@ SmmMemLibInternalGetGcdMemoryMap (
   gBS->FreePool (MemSpaceMap);
 }
 
+/**
+  Get UEFI MemoryAttributesTable.
+**/
+VOID
+SmmMemLibInternalGetUefiMemoryAttributesTable (
+  VOID
+  )
+{
+  EFI_STATUS   Status;
+  EFI_MEMORY_ATTRIBUTES_TABLE  *MemoryAttributesTable;
+  UINTNMemoryAttributesTableSize;
+
+  Status = EfiGetSystemConfigurationTable (, 
(VOID **));
+  if (!EFI_ERROR (Status)) {
+MemoryAttributesTableSize = sizeof(EFI_MEMORY_ATTRIBUTES_TABLE) + 
MemoryAttributesTable->DescriptorSize * MemoryAttributesTable->NumberOfEntries;
+mSmmMemLibMemoryAttributesTable = AllocateCopyPool 
(MemoryAttributesTableSize, MemoryAttributesTable);
+ASSERT (mSmmMemLibMemoryAttributesTable != NULL);
+  }
+}
+
 /**
   Notification for SMM EndOfDxe protocol.
 
@@ -502,6 +549,11 @@ SmmLibInternalEndOfDxeNotify (
   //
   SmmMemLibInternalGetGcdMemoryMap ();
 
+  //
+  // Get UEFI memory attributes table.
+  //
+  SmmMemLibInternalGetUefiMemoryAttributesTable ();
+
   return EFI_SUCCESS;
 }
 
diff --git a/MdePkg/Library/SmmMemLib/SmmMemLib.inf 
b/MdePkg/Library/SmmMemLib/SmmMemLib.inf
index 36576a4f2f..525449c25c 100644
--- a/MdePkg/Library/SmmMemLib/SmmMemLib.inf
+++ b/MdePkg/Library/SmmMemLib/SmmMemLib.inf
@@ -48,11 +48,15 @@
   BaseMemoryLib
   HobLib
   MemoryAllocationLib
+  UefiLib
 
 [Protocols]
   gEfiSmmAccess2ProtocolGuid ## CONSUMES
   gEfiSmmReadyToLockProtocolGuid ## CONSUMES
   gEfiSmmEndOfDxeProtocolGuid## CONSUMES
 
+[Guids]
+  gEfiMemoryAttributesTableGuid  ## CONSUMES ## SystemTable
+
 [Depex]
   gEfiSmmAccess2ProtocolGuid
-- 
2.16.2.windows.1

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

[edk2] [PATCH 1/6] MdePkg/SmmMemLib: Check for untested memory in GCD

[Hao Wu]

From: Jiewen Yao 

It treats GCD untested memory as invalid SMM
communication buffer.

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao 
---
 MdePkg/Library/SmmMemLib/SmmMemLib.c   | 96 +++-
 MdePkg/Library/SmmMemLib/SmmMemLib.inf |  1 +
 2 files changed, 92 insertions(+), 5 deletions(-)

diff --git a/MdePkg/Library/SmmMemLib/SmmMemLib.c 
b/MdePkg/Library/SmmMemLib/SmmMemLib.c
index 8c78a0b426..3f79e46d46 100644
--- a/MdePkg/Library/SmmMemLib/SmmMemLib.c
+++ b/MdePkg/Library/SmmMemLib/SmmMemLib.c
@@ -25,12 +25,20 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
 #include 
 #include 
 
+//
+// attributes for reserved memory before it is promoted to system memory
+//
+#define EFI_MEMORY_PRESENT  0x0100ULL
+#define EFI_MEMORY_INITIALIZED  0x0200ULL
+#define EFI_MEMORY_TESTED   0x0400ULL
+
 #define NEXT_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \
   ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) + (Size)))
 
@@ -46,10 +54,13 @@ UINTN mMemoryMapEntryCount;
 EFI_MEMORY_DESCRIPTOR *mMemoryMap;
 UINTN mDescriptorSize;
 
+EFI_GCD_MEMORY_SPACE_DESCRIPTOR   *mSmmMemLibGcdMemSpace   = NULL;
+UINTN mSmmMemLibGcdMemNumberOfDesc = 0;
+
 VOID  *mRegistrationEndOfDxe;
 VOID  *mRegistrationReadyToLock;
 
-BOOLEAN   mSmmReadyToLock = FALSE;
+BOOLEAN   mSmmMemLibSmmReadyToLock = FALSE;
 
 /**
   Calculate and save the maximum support address.
@@ -154,7 +165,7 @@ SmmIsBufferOutsideSmmValid (
   //
   // Check override for Valid Communication Region
   //
-  if (mSmmReadyToLock) {
+  if (mSmmMemLibSmmReadyToLock) {
 EFI_MEMORY_DESCRIPTOR  *MemoryMap;
 BOOLEANInValidCommunicationRegion;
 
@@ -171,12 +182,28 @@ SmmIsBufferOutsideSmmValid (
 if (!InValidCommunicationRegion) {
   DEBUG ((
 EFI_D_ERROR,
-"SmmIsBufferOutsideSmmValid: Not in ValidCommunicationRegion: Buffer 
(0x%lx) - Length (0x%lx), ",
+"SmmIsBufferOutsideSmmValid: Not in ValidCommunicationRegion: Buffer 
(0x%lx) - Length (0x%lx)\n",
 Buffer,
 Length
 ));
   return FALSE;
 }
+
+//
+// Check untested memory as invalid communication buffer.
+//
+for (Index = 0; Index < mSmmMemLibGcdMemNumberOfDesc; Index++) {
+  if (((Buffer >= mSmmMemLibGcdMemSpace[Index].BaseAddress) && (Buffer < 
mSmmMemLibGcdMemSpace[Index].BaseAddress + 
mSmmMemLibGcdMemSpace[Index].Length)) ||
+  ((mSmmMemLibGcdMemSpace[Index].BaseAddress >= Buffer) && 
(mSmmMemLibGcdMemSpace[Index].BaseAddress < Buffer + Length))) {
+DEBUG ((
+  EFI_D_ERROR,
+  "SmmIsBufferOutsideSmmValid: In Untested Memory Region: Buffer 
(0x%lx) - Length (0x%lx)\n",
+  Buffer,
+  Length
+  ));
+return FALSE;
+  }
+}
   }
   return TRUE;
 }
@@ -317,6 +344,61 @@ SmmSetMem (
   return EFI_SUCCESS;
 }
 
+/**
+  Get GCD memory map.
+  Only record untested memory as invalid communication buffer.
+**/
+VOID
+SmmMemLibInternalGetGcdMemoryMap (
+  VOID
+  )
+{
+  UINTNNumberOfDescriptors;
+  EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *MemSpaceMap;
+  EFI_STATUS   Status;
+  UINTNIndex;
+
+  Status = gDS->GetMemorySpaceMap (, );
+  if (EFI_ERROR (Status)) {
+return ;
+  }
+
+  mSmmMemLibGcdMemNumberOfDesc = 0;
+  for (Index = 0; Index < NumberOfDescriptors; Index++) {
+if (MemSpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeReserved &&
+(MemSpaceMap[Index].Capabilities & (EFI_MEMORY_PRESENT | 
EFI_MEMORY_INITIALIZED | EFI_MEMORY_TESTED)) ==
+  (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED)
+  ) {
+  mSmmMemLibGcdMemNumberOfDesc++;
+}
+  }
+
+  mSmmMemLibGcdMemSpace = AllocateZeroPool (mSmmMemLibGcdMemNumberOfDesc * 
sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR));
+  ASSERT (mSmmMemLibGcdMemSpace != NULL);
+  if (mSmmMemLibGcdMemSpace == NULL) {
+mSmmMemLibGcdMemNumberOfDesc = 0;
+gBS->FreePool (MemSpaceMap);
+return ;
+  }
+
+  mSmmMemLibGcdMemNumberOfDesc = 0;
+  for (Index = 0; Index < NumberOfDescriptors; Index++) {
+if (MemSpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeReserved &&
+(MemSpaceMap[Index].Capabilities & (EFI_MEMORY_PRESENT | 
EFI_MEMORY_INITIALIZED | EFI_MEMORY_TESTED)) ==
+  (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED)
+  ) {
+  CopyMem (
+[mSmmMemLibGcdMemNumberOfDesc],
+[Index],
+sizeof(EFI_GCD_MEMORY_SPACE_DESCRIPTOR)
+);
+  mSmmMemLibGcdMemNumberOfDesc++;
+}
+  }
+
+  gBS->FreePool (MemSpaceMap);
+}
+
 /**
   Notification for SMM EndOfDxe protocol.
 
@@ -415,10 +497,14 @@ SmmLibInternalEndOfDxeNotify (
 
   gBS->FreePool (MemoryMap);
 
+  //
+  // Get additional information 

[edk2] [PATCH 0/6] Check untested memory and EFI_MEMORY_RO

[Hao Wu]

From: Jiewen Yao 

This patch series adds check for untested memory in GCD
and check EFI_RUNTIME_RO in UEFI mem attrib table.

The final result is:
1) untested memory is not present in SMM page table.
2) the PE code section of runtime service is not present
in SMM page table.

Jiewen Yao (6):
  MdePkg/SmmMemLib: Check for untested memory in GCD
  UefiCpuPkg/PiSmmCpu: Check for untested memory in GCD
  MdeModulePkg/DxeCore: Install UEFI mem attrib table at EndOfDxe.
  MdePkg/SmmMemLib: Check EFI_MEMORY_RO in UEFI mem attrib table.
  UefiCpuPkg/PiSmmCpu: Check EFI_RUNTIME_RO in UEFI mem attrib table.
  MdeModulePkg/DxeCore: Not update RtCode in MemAttrTable after EndOfDxe

 MdeModulePkg/Core/Dxe/Misc/MemoryAttributesTable.c |  36 +++-
 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c   |  13 ++
 MdePkg/Library/SmmMemLib/SmmMemLib.c   | 152 +-
 MdePkg/Library/SmmMemLib/SmmMemLib.inf |   5 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h |   2 +
 UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf   |   1 +
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 217 +---
 7 files changed, 392 insertions(+), 34 deletions(-)

-- 
2.16.2.windows.1

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

[edk2] [PATCH 2/6] UefiCpuPkg/PiSmmCpu: Check for untested memory in GCD

[Hao Wu]

From: Jiewen Yao 

It treats GCD untested memory as invalid SMM
communication buffer.

Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jiewen Yao 
---
 UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 144 
 1 file changed, 120 insertions(+), 24 deletions(-)

diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c 
b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
index 85e06b2e6a..b2ace6334e 100644
--- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
+++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
@@ -13,6 +13,13 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #include "PiSmmCpuDxeSmm.h"
 
+//
+// attributes for reserved memory before it is promoted to system memory
+//
+#define EFI_MEMORY_PRESENT  0x0100ULL
+#define EFI_MEMORY_INITIALIZED  0x0200ULL
+#define EFI_MEMORY_TESTED   0x0400ULL
+
 #define NEXT_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \
   ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) + (Size)))
 
@@ -23,6 +30,9 @@ EFI_MEMORY_DESCRIPTOR *mUefiMemoryMap;
 UINTN mUefiMemoryMapSize;
 UINTN mUefiDescriptorSize;
 
+EFI_GCD_MEMORY_SPACE_DESCRIPTOR   *mGcdMemSpace   = NULL;
+UINTN mGcdMemNumberOfDesc = 0;
+
 PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = {
   {Page4K,  SIZE_4KB, PAGING_4K_ADDRESS_MASK_64},
   {Page2M,  SIZE_2MB, PAGING_2M_ADDRESS_MASK_64},
@@ -1022,6 +1032,60 @@ MergeMemoryMapForNotPresentEntry (
   return ;
 }
 
+/**
+  This function caches the GCD memory map information.
+**/
+VOID
+GetGcdMemoryMap (
+  VOID
+  )
+{
+  UINTNNumberOfDescriptors;
+  EFI_GCD_MEMORY_SPACE_DESCRIPTOR  *MemSpaceMap;
+  EFI_STATUS   Status;
+  UINTNIndex;
+
+  Status = gDS->GetMemorySpaceMap (, );
+  if (EFI_ERROR (Status)) {
+return ;
+  }
+
+  mGcdMemNumberOfDesc = 0;
+  for (Index = 0; Index < NumberOfDescriptors; Index++) {
+if (MemSpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeReserved &&
+(MemSpaceMap[Index].Capabilities & (EFI_MEMORY_PRESENT | 
EFI_MEMORY_INITIALIZED | EFI_MEMORY_TESTED)) ==
+  (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED)
+  ) {
+  mGcdMemNumberOfDesc++;
+}
+  }
+
+  mGcdMemSpace = AllocateZeroPool (mGcdMemNumberOfDesc * sizeof 
(EFI_GCD_MEMORY_SPACE_DESCRIPTOR));
+  ASSERT (mGcdMemSpace != NULL);
+  if (mGcdMemSpace == NULL) {
+mGcdMemNumberOfDesc = 0;
+gBS->FreePool (MemSpaceMap);
+return ;
+  }
+
+  mGcdMemNumberOfDesc = 0;
+  for (Index = 0; Index < NumberOfDescriptors; Index++) {
+if (MemSpaceMap[Index].GcdMemoryType == EfiGcdMemoryTypeReserved &&
+(MemSpaceMap[Index].Capabilities & (EFI_MEMORY_PRESENT | 
EFI_MEMORY_INITIALIZED | EFI_MEMORY_TESTED)) ==
+  (EFI_MEMORY_PRESENT | EFI_MEMORY_INITIALIZED)
+  ) {
+  CopyMem (
+[mGcdMemNumberOfDesc],
+[Index],
+sizeof(EFI_GCD_MEMORY_SPACE_DESCRIPTOR)
+);
+  mGcdMemNumberOfDesc++;
+}
+  }
+
+  gBS->FreePool (MemSpaceMap);
+}
+
 /**
   This function caches the UEFI memory map information.
 **/
@@ -1081,6 +1145,11 @@ GetUefiMemoryMap (
   ASSERT (mUefiMemoryMap != NULL);
 
   gBS->FreePool (MemoryMap);
+
+  //
+  // Get additional information from GCD memory map.
+  //
+  GetGcdMemoryMap ();
 }
 
 /**
@@ -1102,33 +1171,52 @@ SetUefiMemMapAttributes (
 
   DEBUG ((DEBUG_INFO, "SetUefiMemMapAttributes\n"));
 
-  if (mUefiMemoryMap == NULL) {
-DEBUG ((DEBUG_INFO, "UefiMemoryMap - NULL\n"));
-return ;
+  if (mUefiMemoryMap != NULL) {
+MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;
+MemoryMap = mUefiMemoryMap;
+for (Index = 0; Index < MemoryMapEntryCount; Index++) {
+  if (IsUefiPageNotPresent(MemoryMap)) {
+Status = SmmSetMemoryAttributes (
+   MemoryMap->PhysicalStart,
+   EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),
+   EFI_MEMORY_RP
+   );
+DEBUG ((
+  DEBUG_INFO,
+  "UefiMemory protection: 0x%lx - 0x%lx %r\n",
+  MemoryMap->PhysicalStart,
+  MemoryMap->PhysicalStart + 
(UINT64)EFI_PAGES_TO_SIZE((UINTN)MemoryMap->NumberOfPages),
+  Status
+  ));
+  }
+  MemoryMap = NEXT_MEMORY_DESCRIPTOR(MemoryMap, mUefiDescriptorSize);
+}
   }
+  //
+  // Do not free mUefiMemoryMap, it will be checked in 
IsSmmCommBufferForbiddenAddress().
+  //
 
-  MemoryMapEntryCount = mUefiMemoryMapSize/mUefiDescriptorSize;
-  MemoryMap = mUefiMemoryMap;
-  for (Index = 0; Index < MemoryMapEntryCount; Index++) {
-if (IsUefiPageNotPresent(MemoryMap)) {
+  //
+  // Set untested memory as not present.
+  //
+  if (mGcdMemSpace != NULL) {
+for (Index = 0; Index < mGcdMemNumberOfDesc; Index++) {
   Status = SmmSetMemoryAttributes (
- 

Re: [edk2] Boot Logo Questions

[Sven Auhagen]

Andrew,

Great, thank you.
That helps a lot.

Best
Sven

From:  on behalf of Andrew Fish 
Date: Wednesday, 18. July 2018 at 10:52 PM
To: Sven Auhagen 
Cc: "edk2-devel@lists.01.org" 
Subject: Re: [edk2] Boot Logo Questions



On Jul 17, 2018, at 11:01 PM, Sven Auhagen 
mailto:sven.auha...@voleatech.de>> wrote:
Hi,
Is there a way to add a text/ascii boot logo to the splash screen?
Is there any example for this,

Sven,

UEFI Spec "Protocols -- Console Support"  chapter - "Simple Text Out Protocol" 
section is the Console abstraction for text (terminal emulation) and graphics. 
It supports 16-bit Unicode, attributes, and curser position.  Basically 
anything you could do with a PCANSI terminal you should be able to do with EFI. 
Note the terminal emulation you use could limit some of the features [1]. This 
is the same API the EFI Shell uses, so it can go to Graphics and serial too.

The edk2 definition is here: 
https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Protocol/SimpleTextOut.h

The UefiLib has CreatePopUp(). That is probably a little more focused on a 
traditional BIOS Setup UI, but might give you some ideas, and it is trying to 
be pedantic from a Unicode point of view.
https://github.com/tianocore/edk2/blob/master/MdePkg/Library/UefiLib/Console.c#L397

The CustomDisplayLib has a PrintAt() function that may also be useful.
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLibInternal.c#L968

[1] - As you can see there is some box draw support in the terminal driver, 
assuming your terminal supports it:  
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c#L24
 This is done by sending a BOXDRAW Unicode character to 
SimpleTextOut->OutputString().

Thanks,

Andrew Fish

I am on an aarch64 platform and only have a serial console on my device.
Best and thank you
Sven
Beste Grüße/Best Regards
Sven Auhagen
Dipl. Math. oec., M.Sc.
Voleatech GmbH
HRB: B 754643
USTID: DE303643180
Grathwohlstr. 5
72762 Reutlingen
Tel: +49 7121539550
Fax: +49 7121539551
E-Mail: sven.auha...@voleatech.de
www.voleatech.de
Diese Information ist ausschließlich für den Adressaten bestimmt und kann 
vertraulich oder gesetzlich geschützte Informationen enthalten. Wenn Sie nicht 
der bestimmungsgemäße Adressat sind, unterrichten Sie bitte den Absender und 
vernichten Sie diese Mail. Anderen als dem bestimmungsgemäßen Adressaten ist es 
untersagt, diese E-Mail zu lesen, zu speichern, weiterzuleiten oder ihren 
Inhalt auf welche Weise auch immer zu verwenden. Für den Adressaten sind die 
Informationen in dieser Mail nur zum persönlichen Gebrauch. Eine Weiterleitung 
darf nur nach Rücksprache mit dem Absender erfolgen. Wir verwenden aktuelle 
Virenschutzprogramme. Für Schäden, die dem Empfänger gleichwohl durch von uns 
zugesandte mit Viren befallene E-Mails entstehen, schließen wir jede Haftung 
aus.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel



Beste Grüße/Best Regards

Sven Auhagen
Dipl. Math. oec., M.Sc.

Voleatech GmbH
HRB: B 754643
USTID: DE303643180
Grathwohlstr. 5
72762 Reutlingen
Tel: +49 7121539550
Fax: +49 7121539551
E-Mail: sven.auha...@voleatech.de

www.voleatech.de
Diese Information ist ausschließlich für den Adressaten bestimmt und kann 
vertraulich oder gesetzlich geschützte Informationen enthalten. Wenn Sie nicht 
der bestimmungsgemäße Adressat sind, unterrichten Sie bitte den Absender und 
vernichten Sie diese Mail. Anderen als dem bestimmungsgemäßen Adressaten ist es 
untersagt, diese E-Mail zu lesen, zu speichern, weiterzuleiten oder ihren 
Inhalt auf welche Weise auch immer zu verwenden. Für den Adressaten sind die 
Informationen in dieser Mail nur zum persönlichen Gebrauch. Eine Weiterleitung 
darf nur nach Rücksprache mit dem Absender erfolgen. Wir verwenden aktuelle 
Virenschutzprogramme. Für Schäden, die dem Empfänger gleichwohl durch von uns 
zugesandte mit Viren befallene E-Mails entstehen, schließen wir jede Haftung 
aus.

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

Re: [edk2] [Patch V3] BaseTools: enable FixedAtBuild (VOID*) PCD use in the [DEPEX] section

[Zhu, Yonghong]

Reviewed-by: Yonghong Zhu  

Best Regards,
Zhu Yonghong


-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Yonghong 
Zhu
Sent: Tuesday, July 17, 2018 10:29 AM
To: edk2-devel@lists.01.org
Cc: Gao, Liming 
Subject: [edk2] [Patch V3] BaseTools: enable FixedAtBuild (VOID*) PCD use in 
the [DEPEX] section

From: Yunhua Feng 

V3: Add some invalid type and datum check

V2: limit the PCD used in the [Depex] section should be used in the module

The PCD item used in INF [Depex] section must be defined as FixedAtBuild type 
and VOID* datum type, and the size of the PCD must be 16 bytes.

Fixes: https://bugzilla.tianocore.org/show_bug.cgi?id=443
Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Yunhua Feng 
---
 BaseTools/Source/Python/AutoGen/AutoGen.py| 32 +--
 BaseTools/Source/Python/AutoGen/GenDepex.py   |  6 +
 BaseTools/Source/Python/Workspace/InfBuildData.py | 20 ++
 BaseTools/Source/Python/build/BuildReport.py  |  7 -
 4 files changed, 57 insertions(+), 8 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/AutoGen.py 
b/BaseTools/Source/Python/AutoGen/AutoGen.py
index c5ab334..9bbf6e1 100644
--- a/BaseTools/Source/Python/AutoGen/AutoGen.py
+++ b/BaseTools/Source/Python/AutoGen/AutoGen.py
@@ -2750,10 +2750,14 @@ class ModuleAutoGen(AutoGen):
 ## Store the FixedAtBuild Pcds
 #
 self._FixedAtBuildPcds = []
 self.ConstPcd  = {}
 
+##Store the VOID* type FixedAtBuild Pcds
+#
+self._FixedPcdVoidTypeDict = {}
+
 def __repr__(self):
 return "%s [%s]" % (self.MetaFile, self.Arch)
 
 # Get FixedAtBuild Pcds of this Module
 def _GetFixedAtBuildPcds(self):
@@ -2765,10 +2769,19 @@ class ModuleAutoGen(AutoGen):
 if Pcd not in self._FixedAtBuildPcds:
 self._FixedAtBuildPcds.append(Pcd)
 
 return self._FixedAtBuildPcds
 
+def _GetFixedAtBuildVoidTypePcds(self):
+if self._FixedPcdVoidTypeDict:
+return self._FixedPcdVoidTypeDict
+for Pcd in self.ModulePcdList:
+if Pcd.Type == TAB_PCDS_FIXED_AT_BUILD and Pcd.DatumType == 
TAB_VOID:
+if '{}.{}'.format(Pcd.TokenSpaceGuidCName, Pcd.TokenCName) not 
in self._FixedPcdVoidTypeDict:
+
self._FixedPcdVoidTypeDict['{}.{}'.format(Pcd.TokenSpaceGuidCName, 
Pcd.TokenCName)] = Pcd.DefaultValue
+return self._FixedPcdVoidTypeDict
+
 def _GetUniqueBaseName(self):
 BaseName = self.Name
 for Module in self.PlatformInfo.ModuleAutoGenList:
 if Module.MetaFile == self.MetaFile:
 continue
@@ -3034,11 +3047,11 @@ class ModuleAutoGen(AutoGen):
 self._DepexDict = {}
 if self.DxsFile or self.IsLibrary or 
TAB_DEPENDENCY_EXPRESSION_FILE in self.FileTypes:
 return self._DepexDict
 
 self._DepexDict[self.ModuleType] = []
-
+self._GetFixedAtBuildVoidTypePcds()
 for ModuleType in self._DepexDict:
 DepexList = self._DepexDict[ModuleType]
 #
 # Append depex from dependent libraries, if not "BEFORE", 
"AFTER" expresion
 #
@@ -3046,11 +3059,25 @@ class ModuleAutoGen(AutoGen):
 Inherited = False
 for D in M.Depex[self.Arch, ModuleType]:
 if DepexList != []:
 DepexList.append('AND')
 DepexList.append('(')
-DepexList.extend(D)
+#replace D with value if D is FixedAtBuild PCD
+NewList = []
+for item in D:
+if '.' not in item:
+NewList.append(item)
+else:
+if item not in self._FixedPcdVoidTypeDict:
+EdkLogger.error("build", FORMAT_INVALID, 
"{} used in [Depex] section should be used as FixedAtBuild type and VOID* datum 
type in the module.".format(item))
+else:
+Value = self._FixedPcdVoidTypeDict[item]
+if len(Value.split(',')) != 16:
+EdkLogger.error("build", 
FORMAT_INVALID,
+"{} used in [Depex] 
section should be used as FixedAtBuild type and VOID* datum type and 16 bytes 
in the module.".format(item))
+NewList.append(Value)
+DepexList.extend(NewList)
 if DepexList[-1] == 'END':  # no need of a END at this 
time
 DepexList.pop()
 DepexList.append(')')
 

Re: [edk2] Help on AutoGen Files

[Udit Kumar]

Thanks Andrew and Marvin,

> You cannot explicitly influence the order of the calls, but implicitly via the
> dependency tree, which means you need to make SerialPortLib depend on your
> LibraryClass instance.

Looks this is difficult that I can force order on already available code. 

> Worst case you can demote FpgaInterfaceInit () from being a constructor to 
> just

Currently I am forcing the clock lib to call FpgaInterfaceInit first, this 
clock lib 
gives input clock to PL011 serial driver. 

Regards
Udit

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Andrew Fish
> Sent: Thursday, July 19, 2018 11:56 PM
> To: Marvin H?user 
> Cc: edk2-devel@lists.01.org
> Subject: Re: [edk2] Help on AutoGen Files
> 
> Udit,
> 
> As Marvin points out the [LibraryClasses] section of the INF file are going to
> imply the order of the library constructor calls in the AutoGen
> 
> Worst case you can demote FpgaInterfaceInit () from being a constructor to 
> just
> being a public library function that the other lib can call explicitly from 
> its
> constructor. Maybe that is too drastic and you could must move a function out
> of FpgaInterfaceInit () and make that function part of the Public library
> interface?
> 
> Thanks,
> 
> Andrew Fish
> 
> > On Jul 19, 2018, at 11:14 AM, Marvin H?user 
> wrote:
> >
> > Hey Udit,
> >
> > You cannot explicitly influence the order of the calls, but implicitly via 
> > the
> dependency tree, which means you need to make SerialPortLib depend on your
> LibraryClass instance.
> > You did not mention which SerialPortLib instance you use, but probably you
> need to execute FpgaInterfaceInit() earlier in platform code or fork 
> SerialPortLib
> for now.
> >
> > Regards,
> > Marvin
> >
> >> -Original Message-
> >> From: edk2-devel  On Behalf Of Udit
> >> Kumar
> >> Sent: Thursday, July 19, 2018 9:33 AM
> >> To: edk2-devel@lists.01.org
> >> Subject: [edk2] Help on AutoGen Files
> >>
> >> Hi Experts,
> >> How I can change the order of initialization in Constructor list of 
> >> autogen file.
> >> In my build system, if I look at
> >> MdeModulePkg/Universal/PCD/Pei/Pcd/DEBUG/AutoGen.c
> >> Below is function of Library Constructor List
> >>
> >> VOID
> >> EFIAPI
> >> ProcessLibraryConstructorList (
> >>  IN   EFI_PEI_FILE_HANDLE   FileHandle,
> >>  IN CONST EFI_PEI_SERVICES  **PeiServices
> >>  )
> >> {
> >>  EFI_STATUS  Status;
> >>
> >>  Status = BaseDebugLibSerialPortConstructor ();  ASSERT_EFI_ERROR
> >> (Status);
> >>
> >>  Status = PeiServicesTablePointerLibConstructor (FileHandle,
> >> PeiServices);  ASSERT_EFI_ERROR (Status);
> >>
> >>  Status = TimerConstructor ();
> >>  ASSERT_EFI_ERROR (Status);
> >>
> >>  Status = FpgaInterfaceInit ();
> >>  ASSERT_EFI_ERROR (Status);
> >>
> >> }
> >>
> >>
> >> My problem is SerialPortConstructor needs frequency, which can be
> >> retrieved after  FpgaInterfaceInit() Therefore, my preferred way for
> >> this constructor list will be
> >> FpgaInterfaceInit() followed by  BaseDebugLibSerialPortConstructor()
> >>
> >> how I can achieve this.
> >>
> >>
> >> Many Thanks
> >> Udit
> >> ___
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fli
> >> sts.01.org%2Fmailman%2Flistinfo%2Fedk2-
> develdata=02%7C01%7Cudit.
> >>
> kumar%40nxp.com%7C5df5baccbdde4daa481808d5eda51660%7C686ea1d3bc2
> b4c6f
> >>
> a92cd99c5c301635%7C0%7C0%7C636676215665542230sdata=6CnFNG5
> t05yH%
> >> 2FOSYpcbp%2F1gWQenUyWcJ%2Fb7C0Yt1n5Y%3Dreserved=0
> > ___
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
> > ts.01.org%2Fmailman%2Flistinfo%2Fedk2-
> develdata=02%7C01%7Cudit.ku
> >
> mar%40nxp.com%7C5df5baccbdde4daa481808d5eda51660%7C686ea1d3bc2b4
> c6fa92
> >
> cd99c5c301635%7C0%7C0%7C636676215665542230sdata=6CnFNG5t05y
> H%2FOS
> > Ypcbp%2F1gWQenUyWcJ%2Fb7C0Yt1n5Y%3Dreserved=0
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.01
> .org%2Fmailman%2Flistinfo%2Fedk2-
> develdata=02%7C01%7Cudit.kumar%40nxp.com%7C5df5baccbdde4daa4
> 81808d5eda51660%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636
> 676215665542230sdata=6CnFNG5t05yH%2FOSYpcbp%2F1gWQenUyWcJ
> %2Fb7C0Yt1n5Y%3Dreserved=0
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 00/10] Standalone Management Mode Core Interface for AARCH64 Platforms

[Yao, Jiewen]

Thank you all for the great effort.

I have pushed this StandaloneMmPkg series with Achin's reviewed-by.

GIT HASH: 2cde4bbadc09a6822e1e653fc4ef46a7106bfe79.. 
da417eb8ed4bbaf149c31d197eb56fc8a5abbf68

Thank you
Yao Jiewen

> -Original Message-
> From: Sughosh Ganu [mailto:sughosh.g...@arm.com]
> Sent: Friday, July 13, 2018 11:05 PM
> To: edk2-devel@lists.01.org
> Cc: Achin Gupta ; Yao, Jiewen 
> Subject: [PATCH v2 00/10] Standalone Management Mode Core Interface for
> AARCH64 Platforms
> 
> The following patch series adds StandaloneMM core interface for
> aarch64 platforms. Following earlier comments from Jeiwen [1], the
> patch series has been split into three parts and this series handles
> StandaloneMMPkg related changes.
> 
> [1] - https://lists.01.org/pipermail/edk2-devel/2018-June/026211.html
> 
> 
> Changes since V1:
>  - Handle review comments from Achin
>- Coding style fixes
> 
> Supreeth Venkatesh (10):
>   StandaloneMmPkg: Delete StandaloneMmPkg file.
>   StandaloneMmPkg/FvLib: Add a common FV Library for management
> mode.
>   StandaloneMmPkg/MemLib: Add Standalone MM instance of memory
> check
> library.
>   StandaloneMmPkg/MemoryAllocationLib: Add MM memory allocation
> library.
>   StandaloneMmPkg/HobLib: Add HOB Library for management mode.
>   StandaloneMmPkg: MM driver entry point library.
>   StandaloneMmPkg/Core: Implementation of Standalone MM Core Module.
>   StandaloneMmPkg: Add an AArch64 specific entry point library.
>   StandaloneMmPkg: Add CPU driver suitable for ARM Platforms.
>   StandaloneMmPkg: Describe the declaration and definition files.
> 
>  StandaloneMmPkg|0
>  StandaloneMmPkg/Core/Dependency.c  |  389 +++
>  StandaloneMmPkg/Core/Dispatcher.c  | 1071
> 
>  StandaloneMmPkg/Core/FwVol.c   |  104 ++
>  StandaloneMmPkg/Core/Handle.c  |  533
> ++
>  StandaloneMmPkg/Core/InstallConfigurationTable.c   |  178 
>  StandaloneMmPkg/Core/Locate.c  |  496
> +
>  StandaloneMmPkg/Core/Mmi.c |  337 ++
>  StandaloneMmPkg/Core/Notify.c  |  203 
>  StandaloneMmPkg/Core/Page.c|  384 +++
>  StandaloneMmPkg/Core/Pool.c|  293 ++
>  StandaloneMmPkg/Core/StandaloneMmCore.c|  712
> +
>  StandaloneMmPkg/Core/StandaloneMmCore.h|  903
> +
>  StandaloneMmPkg/Core/StandaloneMmCore.inf  |   80 ++
>  StandaloneMmPkg/Core/StandaloneMmCorePrivateData.h |   66 ++
>  .../Drivers/StandaloneMmCpu/AArch64/EventHandle.c  |  220 
>  .../StandaloneMmCpu/AArch64/StandaloneMmCpu.c  |  232 +
>  .../StandaloneMmCpu/AArch64/StandaloneMmCpu.h  |   64 ++
>  .../StandaloneMmCpu/AArch64/StandaloneMmCpu.inf|   59 ++
>  StandaloneMmPkg/Include/Guid/MmCoreData.h  |  133 +++
>  StandaloneMmPkg/Include/Guid/MmFvDispatch.h|   39 +
>  StandaloneMmPkg/Include/Guid/MmramMemoryReserve.h  |   62 ++
>  StandaloneMmPkg/Include/Guid/MpInformation.h   |   41 +
>  .../Library/AArch64/StandaloneMmCoreEntryPoint.h   |  215 
>  StandaloneMmPkg/Include/Library/FvLib.h|  109 ++
>  .../Include/Library/StandaloneMmCoreEntryPoint.h   |  101 ++
>  .../Include/Library/StandaloneMmDriverEntryPoint.h |  148 +++
>  .../Include/Library/StandaloneMmMemLib.h   |  140 +++
>  StandaloneMmPkg/Include/StandaloneMm.h |   36 +
>  StandaloneMmPkg/Library/FvLib/FvLib.c  |  385 +++
>  StandaloneMmPkg/Library/FvLib/FvLib.inf|   57 ++
>  .../AArch64/CreateHobList.c|  209 
>  .../AArch64/SetPermissions.c   |  289 ++
>  .../AArch64/StandaloneMmCoreEntryPoint.c   |  306 ++
>  .../StandaloneMmCoreEntryPoint.inf |   55 +
>  .../AArch64/StandaloneMmCoreHobLibInternal.c   |   64 ++
>  .../StandaloneMmCoreHobLib.c   |  608
> +++
>  .../StandaloneMmCoreHobLib.inf |   47 +
>  .../StandaloneMmCoreMemoryAllocationLib.c  |  908
> +
>  .../StandaloneMmCoreMemoryAllocationLib.inf|   49 +
>  .../StandaloneMmCoreMemoryAllocationServices.h |   38 +
>  .../StandaloneMmDriverEntryPoint.c |   99 ++
>  .../StandaloneMmDriverEntryPoint.inf   |   41 +
>  .../AArch64/StandaloneMmMemLibInternal.c   |   49 +
>  .../StandaloneMmMemLib/StandaloneMmMemLib.c|  269 +
>  .../StandaloneMmMemLib/StandaloneMmMemLib.inf  |   50 +
>  StandaloneMmPkg/StandaloneMmPkg.dec|   47 +
>  StandaloneMmPkg/StandaloneMmPkg.dsc|  130 +++
>  48 files changed, 11048 insertions(+)
>  delete mode 100644 StandaloneMmPkg
>  create mode 100644 StandaloneMmPkg/Core/Dependency.c
>  create 

Re: [edk2] [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode

[Wang, Jian J]

Hi Lazlo,

Thank you very much for the effort. We do have an emergency situation.

(1) Agree. Filed a bug (https://bugzilla.tianocore.org/show_bug.cgi?id=1039)
 for it.
(2) I went through the code you mentioned. I think it won't cause problem
 but it might miss protecting certain images (very rare case like calling
 gBS->LoadImage() from SMM driver entry point). I'll consult more relevant
experts before filing a bug.
(3) I'll update the commit message to reflect this for sure.

Regards,
Jian


> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, July 19, 2018 10:46 PM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Yao, Jiewen ; Dong, Eric ;
> Zeng, Star 
> Subject: Re: [edk2] [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM
> mode
> 
> On 07/18/18 04:35, Wang, Jian J wrote:
> > Hi Laszlo,
> >
> >
> > Regards,
> > Jian
> >
> >
> >> -Original Message-
> >> From: Laszlo Ersek [mailto:ler...@redhat.com]
> >> Sent: Tuesday, July 17, 2018 10:37 PM
> >> To: Wang, Jian J ; edk2-devel@lists.01.org
> >> Cc: Dong, Eric ; Yao, Jiewen ;
> >> Zeng, Star 
> >> Subject: Re: [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode
> >>
> >> On 07/13/18 07:53, Jian J Wang wrote:
> >>> Current IsInSmm() method makes use of
> gEfiSmmBase2ProtocolGuid.InSmm()
> >> to
> >>> check if current processor is in SMM mode or not. But this is not correct
> >>> because gEfiSmmBase2ProtocolGuid.InSmm() can only detect if the caller is
> >>> running in SMRAM or from SMM driver. It cannot guarantee if the caller is
> >>> running in SMM mode.
> >>
> >> Wow. This is the exact difference which I asked about, in my question
> >> (9b), and I was assured that InSmm() actually determined whether we were
> >> executing in Management Mode (meaning the processor operating mode).
> >>
> >> http://mid.mail-
> >>
> archive.com/0c09afa07dd0434d9e2a0c6aeb0483103bb55...@shsmsx102.cc
> >> r.corp.intel.com
> >>
> >> (Scroll down in that message to see my original question (9b).)
> >>
> >> So why doesn't Star's explanation, from the original thread, apply
> >> ultimately?
> >>
> >
> > We did many tests for this and didn't found any issue. So I took a risk. (I
> thought
> > a very precise check of SMM mode is hard and time consuming.)
> >
> >>> Because SMM mode will load its own page table, adding
> >>> an extra check of saved DXE page table base address against current CR3
> >>> register value can help to get the correct answer for sure (in SMM mode or
> >>> not in SMM mode).
> >>
> >> So, apparently, the PI spec offers no standard way for a platform module
> >> to determine whether it runs in Management Mode, despite protocol
> member
> >> being called "InSmm". Do we need a PI spec ECR for introducing the
> >> needed facility?
> >>
> >> Alternatively, the PI spec might already intend to specify that, but the
> >> edk2 implementation doesn't do what the PI spec requires.
> >>
> >> Which one is the case?
> >>
> >
> > The implementation conforms to the spec. It's my misunderstanding. But it's
> true
> > that there's no specific protocol API to determine if it's in SMM mode or 
> > not.
> >
> >>>
> >>> This is an issue caused by check-in at
> >>>
> >>>   d106cf71eabaacff63c14626a4a87346b93074dd
> >>
> >> I disagree; I think the issue was introduced in commit 2a1408d1d739
> >> ("UefiCpuPkg/CpuDxe: allow accessing (DXE) page table in SMM mode",
> >> 2018-06-19).
> >>
> >
> > You're right. Thanks for pointing this out.
> >
> >>
> >> How did you encounter / find this issue?
> >>
> >
> > I didn't find it. The issue came to me. In other words, I think it's random 
> > and
> hard
> > to reproduce it. Maybe a subtle change in boot sequence will hide it away.
> >
> >>>
> >>> Cc: Eric Dong 
> >>> Cc: Laszlo Ersek 
> >>> Cc: Jiewen Yao 
> >>> Cc: Star Zeng 
> >>> Contributed-under: TianoCore Contribution Agreement 1.1
> >>> Signed-off-by: Jian J Wang 
> >>> ---
> >>>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 9 -
> >>>  1 file changed, 8 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> >> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> >>> index 850eed60e7..df021798c0 100644
> >>> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> >>> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> >>> @@ -136,7 +136,14 @@ IsInSmm (
> >>>  mSmmBase2->InSmm (mSmmBase2, );
> >>>}
> >>>
> >>> -  return InSmm;
> >>> +  //
> >>> +  // mSmmBase2->InSmm() can only detect if the caller is running in
> SMRAM
> >>> +  // or from SMM driver. It cannot tell if the caller is running in SMM 
> >>> mode.
> >>> +  // Check page table base address to guarantee that because SMM mode
> >> willl
> >>> +  // load its own page table.
> >>> +  //
> >>> +  return (InSmm &&
> >>> +  mPagingContext.ContextData.X64.PageTableBase !=
> >> (UINT64)AsmReadCr3());
> >>>  }
> >>>
> >>>  /**
> >>>
> >>
> >> Shouldn't we consider Ia32.PageTableBase when that's appropriate? From
> >> "UefiCpuPkg/CpuDxe/CpuPageTable.h":

Re: [edk2] OVMF build fail

[Chen, Farrah]

OK, thanks!


Thanks,
Fan



-Original Message-
From: Dong, Eric 
Sent: Friday, July 20, 2018 9:39 AM
To: Chen, Farrah ; edk2-devel@lists.01.org
Cc: Wei, Danmei 
Subject: RE: OVMF build fail

Hi Fan,

Below change fixed this issue, please sync to the latest code and build.


SHA-1: 9b7242f5dec7649b878a4058f893103885520bf7

* UefiCpuPkg/MpInitLib: Fix VS2012 build failure

Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 


Thanks,
Eric

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Chen, Farrah
> Sent: Friday, July 20, 2018 9:19 AM
> To: edk2-devel@lists.01.org
> Cc: Wei, Danmei 
> Subject: [edk2] OVMF build fail
> 
> Hi,
> 
> When I build OVMF with the latest commit:
> a3fffb4f5e1535e8e542669925eed489fdce6b62
> 
> git clone https://github.com/tianocore/edk2.git
> cd edk2
> OvmfPkg/build.sh -a X64
> 
> Then the following error occurred:
> .
> /home/build/kvm_build/nightly/kvm_qemu/kvm-next-20180720013551-
> 6f0d349d-e1ea5566/edk2/MdePkg/Include/Library/DebugLib.h:268:20: error:
> 'ProcessorFlags' may be used uninitialized in this function 
> [-Werror=maybe- uninitialized]
>  DebugPrint (PrintLevel, ##__VA_ARGS__);  \
> ^
> /home/build/kvm_build/nightly/kvm_qemu/kvm-next-20180720013551-
> 6f0d349d-e1ea5566/edk2/UefiCpuPkg/Library/MpInitLib/Microcode.c:63:43:
> note: 'ProcessorFlags' was declared here
>UINT32  ProcessorFlags;
>^
> cc1: all warnings being treated as errors
> make: *** [/home/build/kvm_build/nightly/kvm_qemu/kvm-next-
> 20180720013551-6f0d349d-
> e1ea5566/edk2/Build/OvmfX64/DEBUG_GCC48/X64/UefiCpuPkg/Library/MpI
> nitLib/DxeMpInitLib/OUTPUT/Microcode.obj] Error 1
> 
> 
> build.py...
> : error 7000: Failed to execute command
> make tbuild [/home/build/kvm_build/nightly/kvm_qemu/kvm-next-
> 20180720013551-6f0d349d-
> e1ea5566/edk2/Build/OvmfX64/DEBUG_GCC48/X64/UefiCpuPkg/Library/MpI
> nitLib/DxeMpInitLib]
> 
> 
> build.py...
> : error F002: Failed to build module
> 
> /home/build/kvm_build/nightly/kvm_qemu/kvm-next-20180720013551-
> 6f0d349d-e1ea5566/edk2/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> [X64, GCC48, DEBUG]
> 
> - Failed -
> 
> 
> Best Regards,
> Fan Chen
> 
> ___
> 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] OVMF build fail

[Dong, Eric]

Hi Fan,

Below change fixed this issue, please sync to the latest code and build.


SHA-1: 9b7242f5dec7649b878a4058f893103885520bf7

* UefiCpuPkg/MpInitLib: Fix VS2012 build failure

Cc: Eric Dong 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
Reviewed-by: Eric Dong 


Thanks,
Eric

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Chen, Farrah
> Sent: Friday, July 20, 2018 9:19 AM
> To: edk2-devel@lists.01.org
> Cc: Wei, Danmei 
> Subject: [edk2] OVMF build fail
> 
> Hi,
> 
> When I build OVMF with the latest commit:
> a3fffb4f5e1535e8e542669925eed489fdce6b62
> 
> git clone https://github.com/tianocore/edk2.git
> cd edk2
> OvmfPkg/build.sh -a X64
> 
> Then the following error occurred:
> .
> /home/build/kvm_build/nightly/kvm_qemu/kvm-next-20180720013551-
> 6f0d349d-e1ea5566/edk2/MdePkg/Include/Library/DebugLib.h:268:20: error:
> 'ProcessorFlags' may be used uninitialized in this function [-Werror=maybe-
> uninitialized]
>  DebugPrint (PrintLevel, ##__VA_ARGS__);  \
> ^
> /home/build/kvm_build/nightly/kvm_qemu/kvm-next-20180720013551-
> 6f0d349d-e1ea5566/edk2/UefiCpuPkg/Library/MpInitLib/Microcode.c:63:43:
> note: 'ProcessorFlags' was declared here
>UINT32  ProcessorFlags;
>^
> cc1: all warnings being treated as errors
> make: *** [/home/build/kvm_build/nightly/kvm_qemu/kvm-next-
> 20180720013551-6f0d349d-
> e1ea5566/edk2/Build/OvmfX64/DEBUG_GCC48/X64/UefiCpuPkg/Library/MpI
> nitLib/DxeMpInitLib/OUTPUT/Microcode.obj] Error 1
> 
> 
> build.py...
> : error 7000: Failed to execute command
> make tbuild [/home/build/kvm_build/nightly/kvm_qemu/kvm-next-
> 20180720013551-6f0d349d-
> e1ea5566/edk2/Build/OvmfX64/DEBUG_GCC48/X64/UefiCpuPkg/Library/MpI
> nitLib/DxeMpInitLib]
> 
> 
> build.py...
> : error F002: Failed to build module
> /home/build/kvm_build/nightly/kvm_qemu/kvm-next-20180720013551-
> 6f0d349d-e1ea5566/edk2/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> [X64, GCC48, DEBUG]
> 
> - Failed -
> 
> 
> Best Regards,
> Fan Chen
> 
> ___
> 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] OVMF build fail

[Chen, Farrah]

Hi,

When I build OVMF with the latest commit: 
a3fffb4f5e1535e8e542669925eed489fdce6b62

git clone https://github.com/tianocore/edk2.git
cd edk2
OvmfPkg/build.sh -a X64

Then the following error occurred:
.
/home/build/kvm_build/nightly/kvm_qemu/kvm-next-20180720013551-6f0d349d-e1ea5566/edk2/MdePkg/Include/Library/DebugLib.h:268:20:
 error: 'ProcessorFlags' may be used uninitialized in this function 
[-Werror=maybe-uninitialized]
 DebugPrint (PrintLevel, ##__VA_ARGS__);  \
^
/home/build/kvm_build/nightly/kvm_qemu/kvm-next-20180720013551-6f0d349d-e1ea5566/edk2/UefiCpuPkg/Library/MpInitLib/Microcode.c:63:43:
 note: 'ProcessorFlags' was declared here
   UINT32  ProcessorFlags;
   ^
cc1: all warnings being treated as errors
make: *** 
[/home/build/kvm_build/nightly/kvm_qemu/kvm-next-20180720013551-6f0d349d-e1ea5566/edk2/Build/OvmfX64/DEBUG_GCC48/X64/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib/OUTPUT/Microcode.obj]
 Error 1


build.py...
: error 7000: Failed to execute command
make tbuild 
[/home/build/kvm_build/nightly/kvm_qemu/kvm-next-20180720013551-6f0d349d-e1ea5566/edk2/Build/OvmfX64/DEBUG_GCC48/X64/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib]


build.py...
: error F002: Failed to build module

/home/build/kvm_build/nightly/kvm_qemu/kvm-next-20180720013551-6f0d349d-e1ea5566/edk2/UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
 [X64, GCC48, DEBUG]

- Failed -


Best Regards,
Fan Chen

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

Re: [edk2] [PATCH v2 09/10] StandaloneMmPkg: Add CPU driver suitable for ARM Platforms.

[Achin Gupta]

Thanks Sughosh.

Reviewed-by: Achin Gupta 


On Fri, Jul 13, 2018 at 08:35:29PM +0530, Sughosh Ganu wrote:
> From: Supreeth Venkatesh 
> 
> This patch adds a simple CPU driver that exports the
> EFI_MM_CONFIGURATION_PROTOCOL to allow registration of the Standalone
> MM Foundation entry point. It preserves the existing notification
> mechanism for the configuration protocol.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sughosh Ganu 
> Signed-off-by: Achin Gupta 
> Signed-off-by: Supreeth Venkatesh 
> ---
>  .../Drivers/StandaloneMmCpu/AArch64/EventHandle.c  | 220 +++
>  .../StandaloneMmCpu/AArch64/StandaloneMmCpu.c  | 232 
> +
>  .../StandaloneMmCpu/AArch64/StandaloneMmCpu.h  |  64 ++
>  .../StandaloneMmCpu/AArch64/StandaloneMmCpu.inf|  59 ++
>  StandaloneMmPkg/Include/Guid/MpInformation.h   |  41 
>  5 files changed, 616 insertions(+)
>  create mode 100644 
> StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
>  create mode 100644 
> StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.c
>  create mode 100644 
> StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.h
>  create mode 100644 
> StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/StandaloneMmCpu.inf
>  create mode 100644 StandaloneMmPkg/Include/Guid/MpInformation.h
> 
> diff --git a/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c 
> b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
> new file mode 100644
> index ..2814577b3fcc
> --- /dev/null
> +++ b/StandaloneMmPkg/Drivers/StandaloneMmCpu/AArch64/EventHandle.c
> @@ -0,0 +1,220 @@
> +/** @file
> +
> +  Copyright (c) 2016 HP Development Company, L.P.
> +  Copyright (c) 2016 - 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
> +  which accompanies this distribution.  The full text of the license may be 
> found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR 
> IMPLIED.
> +
> +**/
> +
> +#include 
> +#include 
> +
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include  // for EFI_SYSTEM_CONTEXT
> +
> +#include 
> +#include 
> +
> +#include 
> +
> +#include "StandaloneMmCpu.h"
> +
> +EFI_STATUS
> +EFIAPI
> +MmFoundationEntryRegister (
> +  IN CONST EFI_MM_CONFIGURATION_PROTOCOL  *This,
> +  IN EFI_MM_ENTRY_POINTMmEntryPoint
> +  );
> +
> +//
> +// On ARM platforms every event is expected to have a GUID associated with
> +// it. It will be used by the MM Entry point to find the handler for the
> +// event. It will either be populated in a EFI_MM_COMMUNICATE_HEADER by the
> +// caller of the event (e.g. MM_COMMUNICATE SMC) or by the CPU driver
> +// (e.g. during an asynchronous event). In either case, this context is
> +// maintained in an array which has an entry for each CPU. The pointer to 
> this
> +// array is held in PerCpuGuidedEventContext. Memory is allocated once the
> +// number of CPUs in the system are made known through the
> +// MP_INFORMATION_HOB_DATA.
> +//
> +EFI_MM_COMMUNICATE_HEADER **PerCpuGuidedEventContext = NULL;
> +
> +// Descriptor with whereabouts of memory used for communication with the 
> normal world
> +EFI_MMRAM_DESCRIPTOR  mNsCommBuffer;
> +
> +MP_INFORMATION_HOB_DATA *mMpInformationHobData;
> +
> +EFI_MM_CONFIGURATION_PROTOCOL mMmConfig = {
> +  0,
> +  MmFoundationEntryRegister
> +};
> +
> +STATIC EFI_MM_ENTRY_POINT mMmEntryPoint = NULL;
> +
> +EFI_STATUS
> +PiMmStandloneArmTfCpuDriverEntry (
> +  IN UINTN EventId,
> +  IN UINTN CpuNumber,
> +  IN UINTN NsCommBufferAddr
> +  )
> +{
> +  EFI_MM_COMMUNICATE_HEADER *GuidedEventContext = NULL;
> +  EFI_MM_ENTRY_CONTEXTMmEntryPointContext = {0};
> +  EFI_STATUS  Status;
> +  UINTN   NsCommBufferSize;
> +
> +  DEBUG ((DEBUG_INFO, "Received event - 0x%x on cpu %d\n", EventId, 
> CpuNumber));
> +
> +  Status = EFI_SUCCESS;
> +  //
> +  // ARM TF passes SMC FID of the MM_COMMUNICATE interface as the Event ID 
> upon
> +  // receipt of a synchronous MM request. Use the Event ID to distinguish
> +  // between synchronous and asynchronous events.
> +  //
> +  if (ARM_SMC_ID_MM_COMMUNICATE_AARCH64 != EventId) {
> +DEBUG ((DEBUG_INFO, "UnRecognized Event - 0x%x\n", EventId));
> +return EFI_INVALID_PARAMETER;
> +  }
> +
> +  // Perform parameter validation of NsCommBufferAddr
> +  if (NsCommBufferAddr && (NsCommBufferAddr < mNsCommBuffer.PhysicalStart))
> +return EFI_ACCESS_DENIED;
> +
> +  if ((NsCommBufferAddr + sizeof (EFI_MM_COMMUNICATE_HEADER)) >=
> +  (mNsCommBuffer.PhysicalStart + mNsCommBuffer.PhysicalSize))
> +return EFI_INVALID_PARAMETER;
> +
> +  // 

Re: [edk2] [PATCH v2 08/10] StandaloneMmPkg: Add an AArch64 specific entry point library.

[Achin Gupta]

Thanks Sughosh.

Reviewed-by: Achin Gupta 

On Fri, Jul 13, 2018 at 08:35:28PM +0530, Sughosh Ganu wrote:
> From: Supreeth Venkatesh 
> 
> The Standalone MM environment runs in S-EL0 in AArch64 on ARM Standard
> Platforms and is initialised during the SEC phase. ARM Trusted firmware
> in EL3 is responsible for initialising the architectural context for
> S-EL0 and loading the Standalone MM image. The memory allocated to this
> image is marked as RO+X. Heap memory is marked as RW+XN.
> 
> Certain actions have to be completed prior to executing the generic code
> in the Standalone MM Core module. These are:
> 
> 1. Memory permission attributes for each section of the Standalone MM
>Core module need to be changed prior to accessing any RW data.
> 
> 2. A Hob list has to be created with information that allows the MM
>environment to initialise and dispatch drivers.
> 
> Furthermore, this module is responsible for handing over runtime MM
> events to the Standalone MM CPU driver and returning control to ARM
> Trusted Firmware upon event completion. Hence it needs to know the CPU
> driver entry point.
> 
> This patch implements an entry point module that ARM Trusted Firmware
> jumps to in S-EL0. It then performs the above actions before calling the
> Standalone MM Foundation entry point and handling subsequent MM events.
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Sughosh Ganu 
> Signed-off-by: Achin Gupta 
> Signed-off-by: Supreeth Venkatesh 
> ---
>  .../Library/AArch64/StandaloneMmCoreEntryPoint.h   | 215 +++
>  .../AArch64/CreateHobList.c| 209 ++
>  .../AArch64/SetPermissions.c   | 289 +++
>  .../AArch64/StandaloneMmCoreEntryPoint.c   | 306 
> +
>  .../StandaloneMmCoreEntryPoint.inf |  55 
>  5 files changed, 1074 insertions(+)
>  create mode 100644 
> StandaloneMmPkg/Include/Library/AArch64/StandaloneMmCoreEntryPoint.h
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/CreateHobList.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/SetPermissions.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/AArch64/StandaloneMmCoreEntryPoint.c
>  create mode 100644 
> StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf
> 
> diff --git 
> a/StandaloneMmPkg/Include/Library/AArch64/StandaloneMmCoreEntryPoint.h 
> b/StandaloneMmPkg/Include/Library/AArch64/StandaloneMmCoreEntryPoint.h
> new file mode 100644
> index ..e4e5875b5d22
> --- /dev/null
> +++ b/StandaloneMmPkg/Include/Library/AArch64/StandaloneMmCoreEntryPoint.h
> @@ -0,0 +1,215 @@
> +/** @file
> +  Entry point to the Standalone MM Foundation when initialized during the SEC
> +  phase on ARM platforms
> +
> +Copyright (c) 2017 - 2018, ARM Ltd. All rights reserved.
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD 
> License
> +which accompanies this distribution.  The full text of the license may be 
> found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#ifndef __STANDALONEMMCORE_ENTRY_POINT_H__
> +#define __STANDALONEMMCORE_ENTRY_POINT_H__
> +
> +#include 
> +#include 
> +
> +#define CPU_INFO_FLAG_PRIMARY_CPU  0x0001
> +
> +typedef struct {
> +  UINT8  Type;   /* type of the structure */
> +  UINT8  Version;/* version of this structure */
> +  UINT16 Size;  /* size of this structure in bytes */
> +  UINT32 Attr;  /* attributes: unused bits SBZ */
> +} EFI_PARAM_HEADER;
> +
> +typedef struct {
> +  UINT64 Mpidr;
> +  UINT32 LinearId;
> +  UINT32 Flags;
> +} EFI_SECURE_PARTITION_CPU_INFO;
> +
> +typedef struct {
> +  EFI_PARAM_HEADER  Header;
> +  UINT64SpMemBase;
> +  UINT64SpMemLimit;
> +  UINT64SpImageBase;
> +  UINT64SpStackBase;
> +  UINT64SpHeapBase;
> +  UINT64SpNsCommBufBase;
> +  UINT64SpSharedBufBase;
> +  UINT64SpImageSize;
> +  UINT64SpPcpuStackSize;
> +  UINT64SpHeapSize;
> +  UINT64SpNsCommBufSize;
> +  UINT64SpPcpuSharedBufSize;
> +  UINT32NumSpMemRegions;
> +  UINT32NumCpus;
> +  EFI_SECURE_PARTITION_CPU_INFO *CpuInfo;
> +} EFI_SECURE_PARTITION_BOOT_INFO;
> +
> +typedef
> +EFI_STATUS
> +(*PI_MM_ARM_TF_CPU_DRIVER_ENTRYPOINT) (
> +  IN UINTN EventId,
> +  IN UINTN CpuNumber,
> +  IN UINTN NsCommBufferAddr
> +  );
> +
> +typedef 

Re: [edk2] [PATCH] OvmfPkg/XenPvBlkDxe: remove gEfiDevicePathProtocolGuid from [Protocols]

[Jordan Justen]

Reviewed-by: Jordan Justen 

On 2018-07-19 11:01:43, Laszlo Ersek wrote:
> XenPvBlkDxe doesn't reference gEfiDevicePathProtocolGuid; remove it from
> the INF file.
> 
> Cc: Anthony Perard 
> Cc: Ard Biesheuvel 
> Cc: Jordan Justen 
> Cc: Julien Grall 
> Cc: Steven Shi 
> Reported-by: Steven Shi 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1034
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 
> ---
> 
> Notes:
> Repo:   https://github.com/lersek/edk2.git
> Branch: xenpvblk_redundant_proto_tcbz1034
> 
>  OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf 
> b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> index b3dd7c70294a..4b4dd1310f71 100644
> --- a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> +++ b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
> @@ -57,7 +57,6 @@ [Protocols]
>gEfiComponentName2ProtocolGuid
>gEfiComponentNameProtocolGuid
>gXenBusProtocolGuid
> -  gEfiDevicePathProtocolGuid## TO_START
>  
>  
>  [Guids]
> -- 
> 2.14.1.3.gb7cf6e02401b
> 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2] UefiCpuPkg/MpInitLib: Fix build of Microcode

[Laszlo Ersek]

Hi Anthony,

(I'm seeing your v1 and v2 postings at the same time now:)

On 07/19/18 17:52, Anthony PERARD wrote:
> On Debian Jessie, this fail to build with:
> 
> /build/UefiCpuPkg/Library/MpInitLib/Microcode.c: In function 
> 'MicrocodeDetect':
> /build/UefiCpuPkg/Library/MpInitLib/Microcode.c:248:37: error: 
> 'ProcessorFlags' may be used uninitialized in this function 
> [-Werror=maybe-uninitialized]
>  CpuMpData->ProcessorFlags   = ProcessorFlags;
>  ^
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Anthony PERARD 
> ---
> 
> Notes:
> v2:
> Fix coding style.
> 
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index efda143e67..cea1d34c0a 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -87,6 +87,7 @@ MicrocodeDetect (
>}
>  
>ExtendedTableLength = 0;
> +  ProcessorFlags = 0;
>//
>// Here data of CPUID leafs have not been collected into context buffer, so
>// GetProcessorCpuid() cannot be used here to retrieve CPUID data.
> 

a patch for fixing this issue is on its way to being committed (Dandan
posted it just ~10 hours before your v1). Please see the thread

  [edk2] [patch] UefiCpuPkg/MpInitLib: Fix VS2012 build failure
  20180719045059.55868-1-dandan.bi@intel.com">http://mid.mail-archive.com/20180719045059.55868-1-dandan.bi@intel.com

I think when Liming commented on your v1, he was unaware of Dandan's
patch. And, I'm seeing your v1+v2 just now, at the same time. Sorry
about the inconvenience!

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

Re: [edk2] Help on AutoGen Files

[Andrew Fish]

Udit,

As Marvin points out the [LibraryClasses] section of the INF file are going to 
imply the order of the library constructor calls in the AutoGen

Worst case you can demote FpgaInterfaceInit () from being a constructor to just 
being a public library function that the other lib can call explicitly from its 
constructor. Maybe that is too drastic and you could must move a function out 
of FpgaInterfaceInit () and make that function part of the Public library 
interface?

Thanks,

Andrew Fish

> On Jul 19, 2018, at 11:14 AM, Marvin H?user  
> wrote:
> 
> Hey Udit,
> 
> You cannot explicitly influence the order of the calls, but implicitly via 
> the dependency tree, which means you need to make SerialPortLib depend on 
> your LibraryClass instance.
> You did not mention which SerialPortLib instance you use, but probably you 
> need to execute FpgaInterfaceInit() earlier in platform code or fork 
> SerialPortLib for now.
> 
> Regards,
> Marvin
> 
>> -Original Message-
>> From: edk2-devel  On Behalf Of Udit
>> Kumar
>> Sent: Thursday, July 19, 2018 9:33 AM
>> To: edk2-devel@lists.01.org
>> Subject: [edk2] Help on AutoGen Files
>> 
>> Hi Experts,
>> How I can change the order of initialization in Constructor list of autogen 
>> file.
>> In my build system, if I look at
>> MdeModulePkg/Universal/PCD/Pei/Pcd/DEBUG/AutoGen.c
>> Below is function of Library Constructor List
>> 
>> VOID
>> EFIAPI
>> ProcessLibraryConstructorList (
>>  IN   EFI_PEI_FILE_HANDLE   FileHandle,
>>  IN CONST EFI_PEI_SERVICES  **PeiServices
>>  )
>> {
>>  EFI_STATUS  Status;
>> 
>>  Status = BaseDebugLibSerialPortConstructor ();
>>  ASSERT_EFI_ERROR (Status);
>> 
>>  Status = PeiServicesTablePointerLibConstructor (FileHandle, PeiServices);
>>  ASSERT_EFI_ERROR (Status);
>> 
>>  Status = TimerConstructor ();
>>  ASSERT_EFI_ERROR (Status);
>> 
>>  Status = FpgaInterfaceInit ();
>>  ASSERT_EFI_ERROR (Status);
>> 
>> }
>> 
>> 
>> My problem is SerialPortConstructor needs frequency, which can be
>> retrieved after  FpgaInterfaceInit() Therefore, my preferred way for this
>> constructor list will be
>> FpgaInterfaceInit() followed by  BaseDebugLibSerialPortConstructor()
>> 
>> how I can achieve this.
>> 
>> 
>> Many Thanks
>> Udit
>> ___
>> 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] Help on AutoGen Files

[Marvin H?user]

Hey Udit,

You cannot explicitly influence the order of the calls, but implicitly via the 
dependency tree, which means you need to make SerialPortLib depend on your 
LibraryClass instance.
You did not mention which SerialPortLib instance you use, but probably you need 
to execute FpgaInterfaceInit() earlier in platform code or fork SerialPortLib 
for now.

Regards,
Marvin

> -Original Message-
> From: edk2-devel  On Behalf Of Udit
> Kumar
> Sent: Thursday, July 19, 2018 9:33 AM
> To: edk2-devel@lists.01.org
> Subject: [edk2] Help on AutoGen Files
> 
> Hi Experts,
> How I can change the order of initialization in Constructor list of autogen 
> file.
> In my build system, if I look at
> MdeModulePkg/Universal/PCD/Pei/Pcd/DEBUG/AutoGen.c
> Below is function of Library Constructor List
> 
> VOID
> EFIAPI
> ProcessLibraryConstructorList (
>   IN   EFI_PEI_FILE_HANDLE   FileHandle,
>   IN CONST EFI_PEI_SERVICES  **PeiServices
>   )
> {
>   EFI_STATUS  Status;
> 
>   Status = BaseDebugLibSerialPortConstructor ();
>   ASSERT_EFI_ERROR (Status);
> 
>   Status = PeiServicesTablePointerLibConstructor (FileHandle, PeiServices);
>   ASSERT_EFI_ERROR (Status);
> 
>   Status = TimerConstructor ();
>   ASSERT_EFI_ERROR (Status);
> 
>   Status = FpgaInterfaceInit ();
>   ASSERT_EFI_ERROR (Status);
> 
> }
> 
> 
> My problem is SerialPortConstructor needs frequency, which can be
> retrieved after  FpgaInterfaceInit() Therefore, my preferred way for this
> constructor list will be
> FpgaInterfaceInit() followed by  BaseDebugLibSerialPortConstructor()
> 
> how I can achieve this.
> 
> 
> Many Thanks
> Udit
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] OvmfPkg/XenPvBlkDxe: remove gEfiDevicePathProtocolGuid from [Protocols]

[Laszlo Ersek]

XenPvBlkDxe doesn't reference gEfiDevicePathProtocolGuid; remove it from
the INF file.

Cc: Anthony Perard 
Cc: Ard Biesheuvel 
Cc: Jordan Justen 
Cc: Julien Grall 
Cc: Steven Shi 
Reported-by: Steven Shi 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1034
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---

Notes:
Repo:   https://github.com/lersek/edk2.git
Branch: xenpvblk_redundant_proto_tcbz1034

 OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf | 1 -
 1 file changed, 1 deletion(-)

diff --git a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf 
b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
index b3dd7c70294a..4b4dd1310f71 100644
--- a/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
+++ b/OvmfPkg/XenPvBlkDxe/XenPvBlkDxe.inf
@@ -57,7 +57,6 @@ [Protocols]
   gEfiComponentName2ProtocolGuid
   gEfiComponentNameProtocolGuid
   gXenBusProtocolGuid
-  gEfiDevicePathProtocolGuid## TO_START
 
 
 [Guids]
-- 
2.14.1.3.gb7cf6e02401b

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

[edk2] [PATCH v1 1/1] BaseTools: AutoGen - change class variable to funciton variable

[Jaben Carsey]

This variable is only used in one function, make it local there.
Also when iterating on the variable, use dict.items() to get value
instead of re-looking up the value multiple times.

Cc: Liming Gao 
Cc: Yonghong Zhu 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jaben Carsey 
---
 BaseTools/Source/Python/AutoGen/GenMake.py | 25 ++--
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/BaseTools/Source/Python/AutoGen/GenMake.py 
b/BaseTools/Source/Python/AutoGen/GenMake.py
index 992de5490dff..f1fe5514f3f2 100644
--- a/BaseTools/Source/Python/AutoGen/GenMake.py
+++ b/BaseTools/Source/Python/AutoGen/GenMake.py
@@ -435,7 +435,6 @@ cleanlib:
 self.ListFileMacros = {}
 
 self.FileCache = {}
-self.FileDependency = []
 self.LibraryBuildCommandList = []
 self.LibraryFileList = []
 self.LibraryMakefileList = []
@@ -890,26 +889,26 @@ cleanlib:
 if Item in SourceFileList:
 SourceFileList.remove(Item)
 
-self.FileDependency = self.GetFileDependency(
+FileDependencyDict = self.GetFileDependency(
 SourceFileList,
 ForceIncludedFile,
 self._AutoGenObject.IncludePathList + 
self._AutoGenObject.BuildOptionIncPathList
 )
 DepSet = None
-for File in self.FileDependency:
-if not self.FileDependency[File]:
-self.FileDependency[File] = ['$(FORCE_REBUILD)']
+for File,Dependency in FileDependencyDict.items():
+if not Dependency:
+FileDependencyDict[File] = ['$(FORCE_REBUILD)']
 continue
 
-self._AutoGenObject.AutoGenDepSet |= set(self.FileDependency[File])
+self._AutoGenObject.AutoGenDepSet |= set(Dependency)
 
 # skip non-C files
 if File.Ext not in [".c", ".C"] or File.Name == "AutoGen.c":
 continue
 elif DepSet is None:
-DepSet = set(self.FileDependency[File])
+DepSet = set(Dependency)
 else:
-DepSet &= set(self.FileDependency[File])
+DepSet &= set(Dependency)
 # in case nothing in SourceFileList
 if DepSet is None:
 DepSet = set()
@@ -919,13 +918,13 @@ cleanlib:
 for File in DepSet:
 self.CommonFileDependency.append(self.PlaceMacro(File.Path, 
self.Macros))
 
-for File in self.FileDependency:
+for File in FileDependencyDict:
 # skip non-C files
 if File.Ext not in [".c", ".C"] or File.Name == "AutoGen.c":
 continue
-NewDepSet = set(self.FileDependency[File])
+NewDepSet = set(FileDependencyDict[File])
 NewDepSet -= DepSet
-self.FileDependency[File] = ["$(COMMON_DEPS)"] + list(NewDepSet)
+FileDependencyDict[File] = ["$(COMMON_DEPS)"] + list(NewDepSet)
 
 # Convert target description object to target string in makefile
 for Type in self._AutoGenObject.Targets:
@@ -943,8 +942,8 @@ cleanlib:
 for Dep in T.Dependencies:
 Deps.append(self.PlaceMacro(str(Dep), self.Macros))
 # Add inclusion-dependencies
-if len(T.Inputs) == 1 and T.Inputs[0] in self.FileDependency:
-for F in self.FileDependency[T.Inputs[0]]:
+if len(T.Inputs) == 1 and T.Inputs[0] in FileDependencyDict:
+for F in FileDependencyDict[T.Inputs[0]]:
 Deps.append(self.PlaceMacro(str(F), self.Macros))
 # Add source-dependencies
 for F in T.Inputs:
-- 
2.16.2.windows.1

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

Re: [edk2] [Patch V2] UefiCpuPkg/MpInitLib: Remove redundant parameter.

[Laszlo Ersek]

Hi Eric,

apologies about the delay.

On 07/18/18 14:59, Dong, Eric wrote:
> Hi Laszlo,
>
> I finally succeed to setup the OVMF platform which can verify the boot
> failure issue.  But on my platform, if I use image build with below
> command (I assume it is used to enable SMM), the system can't boot to
> OS (host OS is fedora 25 and guest OS is Ubuntu 18.04). It hang at OS
> boot phase after ExitBootService point (I can see the console log
> which should been printed at ExitBootService point, so I think hang
> should after this point).
>   build -a IA32 -a X64 -p OvmfPkg/OvmfPkgIa32X64.dsc -t VS2015x86 -b 
> NOOPT -D SMM_REQUIRE -D SECURE_BOOT_ENABLE -D TLS_ENABLE
>
> If I use below command to build the image, the system can boot to OS.
>   build -a IA32 -a X64 -p OvmfPkg\OvmfPkgIa32X64.dsc -t VS2015x86 -b NOOPT
>
> Does my OVMF environment still has problem?
>
>
> When do the above test, I don't include my two patches.

Yes, I think this host environment is still problematic. Namely, the
latest QEMU version shipped in Fedora 25 is QEMU-2.7:

  https://koji.fedoraproject.org/koji/buildinfo?buildID=918114

and QEMU-2.7 does not have a feature that is important for SMM
stability. This feature is called "SMI broadcast".

In OVMF, the "OvmfPkg/SmmControl2Dxe" runtime driver implements
EFI_SMM_CONTROL2_PROTOCOL (which is a runtime protocol). The Trigger()
member function raises an SMI, by writing to IO port 0xB2
(ICH9_APM_CNT).

Originally, QEMU would raise the SMI synchronously only on the sole VCPU
that called Trigger(). Then, the edk2 SMM driver stack would have to
pull the other processors explicitly into SMM (via APIC accesses, if I
remember correctly). This was extremely slow (the processor first
raising the SMI would wait for a long time for the other processors to
show up in SMM, before it would decide to pull them in with APIC
writes). Also when we switched the edk2 SMM sync mode to "relaxed", the
results remained very unstable. We decided that edk2 supported the
"traditional" SMM sync mode much better, and so we implemented "SMI
broadcast" in QEMU, to satisfy that sync mode.

(My memories are a bit fuzzy at this point; you can read more in the
following RH Bugzilla entries:

  https://bugzilla.redhat.com/show_bug.cgi?id=1412327 [QEMU]
  https://bugzilla.redhat.com/show_bug.cgi?id=1412313 [OVMF])

The idea of "SMI broadcast" is that, regardless of which VCPU triggers
the SMI, QEMU raises the SMI immediately on all VCPUs. This made a
*huge* difference for the performance and the stability of the edk2 SMM
driver stack, used in OVMF and on QEMU/KVM.

Now, in order to be able to use old OVMF on new QEMU and vice versa,
this feature is runtime-negotiated between "OvmfPkg/SmmControl2Dxe" and
QEMU. (The feature is not enabled by default, and without "SMI
broadcast", the "relaxed" sync method is slightly less broken than the
"tradiational" method, so OVMF defaults to that. With the feature
enabled, the "traditional" mode is better -- that config is the absolute
best of all four possible combinations.)

More precisely, on the QEMU side, the feature is not tied to a QEMU
release, but to Q35 *machine type versions*. Therefore, in order to
benefit from the feature, you need all of the following:

- a recent enough OVMF,
- a recent enough QEMU release,
- a recent enough Q35 machine type, specified on the QEMU command line.

The particular minimum machine type is "pc-q35-2.9" (which is clearly
only provided by QEMU-2.9 and later). The machine type requirement is
automatically satisfied if you use QEMU-2.9+, and just request the "q35"
machine type. (Without an explicit machtype version number, the highest
one supported by the QEMU release will be picked.)

The lack of this feature in your environment is confirmed by your OVMF
log:

> NegotiateSmiFeatures: SMI feature negotiation unavailable

If the feature is available, you will see the following two messages
instead:

  NegotiateSmiFeatures: using SMI broadcast
  [...]
  AppendFwCfgBootScript: SMI feature negotiation boot script saved

(The second message only appears if you have S3 enabled -- at S3 resume,
the feature has to be re-enabled, so SmmControl2Dxe saves a boot script
fragment for that.)

Therefore, please upgrade the host to Fedora 26. In Fedora 26, QEMU 2.9
is shipped:

  https://koji.fedoraproject.org/koji/buildinfo?buildID=986762

... It's even better if you can upgrade to Fedora 27, as Fedora 27 is
the oldest Fedora release still supported at this point. The following
article describes the recommended upgrade method:

  https://fedoraproject.org/wiki/DNF_system_upgrade

> Then I include my patches and build the image with SMM enabled, I
> found I can't reproduce the issue you met. I can find the
> "MpInitChangeApLoopCallback done!" message in the console log.
> Attached the console log.

Yes, I can see "MpInitChangeApLoopCallback() done" in the log.

> Can you help to verify the OVMF image build from my side?

Your firmware image (SHA1: 

[edk2] [PATCH v2] UefiCpuPkg/MpInitLib: Fix build of Microcode

[Anthony PERARD]

On Debian Jessie, this fail to build with:

/build/UefiCpuPkg/Library/MpInitLib/Microcode.c: In function 'MicrocodeDetect':
/build/UefiCpuPkg/Library/MpInitLib/Microcode.c:248:37: error: 'ProcessorFlags' 
may be used uninitialized in this function [-Werror=maybe-uninitialized]
 CpuMpData->ProcessorFlags   = ProcessorFlags;
 ^

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Anthony PERARD 
---

Notes:
v2:
Fix coding style.

 UefiCpuPkg/Library/MpInitLib/Microcode.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index efda143e67..cea1d34c0a 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -87,6 +87,7 @@ MicrocodeDetect (
   }
 
   ExtendedTableLength = 0;
+  ProcessorFlags = 0;
   //
   // Here data of CPUID leafs have not been collected into context buffer, so
   // GetProcessorCpuid() cannot be used here to retrieve CPUID data.
-- 
Anthony PERARD

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

Re: [edk2] Inquiry regarding early DxeIplPeim loading.

[Cohen, Eugene]

Marvin,

> Though to be honest, I am not sure why these PPIs are exposed by
> DxeIplPeim instead of a dedicated module or maybe even PeiCore itself, if
> it's the de facto standard.

When we were originally facing the issue one of the ideas was in fact to move 
this to PeiCore.  I don't recall why that path wasn't pursued.

> Also, if the change was done to avoid disabling CAR prematurely, why
> wasn't SEC code adapted?

This was an ARM SoC not X86 so the modules involved were different than what 
you're seeing.  I'm trying to remember the specifics but it was long enough ago 
that it escapes me and my time machine is broken.

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

Re: [edk2] [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Return 0 when LSV check is not required

[Kinney, Michael D]

Reviewed-by: Michael D Kinney  

Mike

> -Original Message-
> From: Zeng, Star
> Sent: Thursday, July 19, 2018 3:32 AM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star ; Kinney, Michael D
> 
> Subject: [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg
> FmpDxe: Return 0 when LSV check is not required
> 
> Current code return 1 when LSV check is not required,
> but 1 LSV will make 0 Version capsule image update failed.
> 
> 0 LSV is valid, this patch updates the code to return 0
> when
> LSV check is not required
> We can see even the DEFAULT_LOWESTSUPPORTEDVERSION is 0.
> 
> Cc: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  FmpDevicePkg/FmpDxe/FmpDxe.c | 9 ++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
> b/FmpDevicePkg/FmpDxe/FmpDxe.c
> index c0c1383723fb..d6b599ee1a56 100644
> --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> @@ -210,12 +210,15 @@ GetLowestSupportedVersion (
>// Get the LowestSupportedVersion.
>//
> 
> -  DeviceLibLowestSupportedVersion =
> DEFAULT_LOWESTSUPPORTEDVERSION;
> -  ReturnLsv = PcdGet32
> (PcdFmpDeviceBuildTimeLowestSupportedVersion);
>if (!IsLowestSupportedVersionCheckRequired ()) {
> -return 1;
> +//
> +// Any Versioon can pass the 0 LowestSupportedVersion
> check.
> +//
> +return 0;
>}
> 
> +  ReturnLsv = PcdGet32
> (PcdFmpDeviceBuildTimeLowestSupportedVersion);
> +
>//
>// Check the FmpDeviceLib
>//
> --
> 2.7.0.windows.1

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

Re: [edk2] [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Update function comment for FmpDxeEntryPoint

[Kinney, Michael D]

Reviewed-by: Michael D Kinney  

Mike
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org]
> On Behalf Of Star Zeng
> Sent: Thursday, July 19, 2018 3:19 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Zeng,
> Star 
> Subject: [edk2] [staging/FmpDevicePkg-master][PATCH]
> FmpDevicePkg FmpDxe: Update function comment for
> FmpDxeEntryPoint
> 
> FmpDxeEntryPoint is used by both FmpDxe and FmpDxeLib.
> 
> Cc: Michael D Kinney 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Star Zeng 
> ---
>  FmpDevicePkg/FmpDxe/FmpDxe.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
> b/FmpDevicePkg/FmpDxe/FmpDxe.c
> index b709bc282a6c..c0c1383723fb 100644
> --- a/FmpDevicePkg/FmpDxe/FmpDxe.c
> +++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
> @@ -1360,7 +1360,7 @@ cleanup:
>  }
> 
>  /**
> -  Main entry for this library.
> +  Main entry for this driver/library.
> 
>@param[in] ImageHandle  Image handle this driver.
>@param[in] SystemTable  Pointer to SystemTable.
> --
> 2.7.0.windows.1
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Fix build of Microcode

[Gao, Liming]

Edk2 style doesn't initialize the local variable value in its declaration. 
Could you update this patch to separately set ProcessorFlags value?

Thanks
Liming
> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of 
> Anthony PERARD
> Sent: Thursday, July 19, 2018 10:58 PM
> To: edk2-devel@lists.01.org
> Cc: Anthony PERARD ; Laszlo Ersek 
> ; Dong, Eric 
> Subject: [edk2] [PATCH] UefiCpuPkg/MpInitLib: Fix build of Microcode
> 
> On Debian Jessie, this fail to build with:
> 
> /build/UefiCpuPkg/Library/MpInitLib/Microcode.c: In function 
> 'MicrocodeDetect':
> /build/UefiCpuPkg/Library/MpInitLib/Microcode.c:248:37: error: 
> 'ProcessorFlags' may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
>  CpuMpData->ProcessorFlags   = ProcessorFlags;
>  ^
> 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Anthony PERARD 
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index efda143e67..60cf8bf409 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -60,7 +60,7 @@ MicrocodeDetect (
>BOOLEAN CorrectMicrocode;
>VOID*MicrocodeData;
>MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
> -  UINT32  ProcessorFlags;
> +  UINT32  ProcessorFlags = 0;
>UINT32  ThreadId;
> 
>if (CpuMpData->MicrocodePatchRegionSize == 0) {
> --
> Anthony PERARD
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

[edk2] [PATCH] UefiCpuPkg/MpInitLib: Fix build of Microcode

[Anthony PERARD]

On Debian Jessie, this fail to build with:

/build/UefiCpuPkg/Library/MpInitLib/Microcode.c: In function 'MicrocodeDetect':
/build/UefiCpuPkg/Library/MpInitLib/Microcode.c:248:37: error: 'ProcessorFlags' 
may be used uninitialized in this function [-Werror=maybe-uninitialized]
 CpuMpData->ProcessorFlags   = ProcessorFlags;
 ^

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Anthony PERARD 
---
 UefiCpuPkg/Library/MpInitLib/Microcode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
b/UefiCpuPkg/Library/MpInitLib/Microcode.c
index efda143e67..60cf8bf409 100644
--- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
+++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
@@ -60,7 +60,7 @@ MicrocodeDetect (
   BOOLEAN CorrectMicrocode;
   VOID*MicrocodeData;
   MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
-  UINT32  ProcessorFlags;
+  UINT32  ProcessorFlags = 0;
   UINT32  ThreadId;
 
   if (CpuMpData->MicrocodePatchRegionSize == 0) {
-- 
Anthony PERARD

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

Re: [edk2] [patch] ShellPkg/Dp: Cumulate the perf data of "DB:Stop"

[Carsey, Jaben]

Reviewed-by: Jaben Carsey 

> -Original Message-
> From: Bi, Dandan
> Sent: Thursday, July 19, 2018 12:08 AM
> To: edk2-devel@lists.01.org
> Cc: Gao, Liming ; Carsey, Jaben
> ; Ni, Ruiyu 
> Subject: [patch] ShellPkg/Dp: Cumulate the perf data of "DB:Stop"
> Importance: High
> 
> Currently DP tool will cumulate the performance data for
> Driver Binding Support/Start, will count the number they
> appears, and record the maximum/minimum time value...
> Now add Driver Binding Stop performance data to the
> cumulative data to keep consistency.
> 
> Cc: Liming Gao 
> Cc: Jaben Carsey 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi 
> ---
>  ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> index 213f6057c9..2c094b63c9 100644
> --- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> +++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
> @@ -71,11 +71,12 @@ UINTN   mMeasurementNum= 0;
>  /// Items for which to gather cumulative statistics.
>  PERF_CUM_DATA CumData[] = {
>PERF_INIT_CUM_DATA (LOAD_IMAGE_TOK),
>PERF_INIT_CUM_DATA (START_IMAGE_TOK),
>PERF_INIT_CUM_DATA (DRIVERBINDING_START_TOK),
> -  PERF_INIT_CUM_DATA (DRIVERBINDING_SUPPORT_TOK)
> +  PERF_INIT_CUM_DATA (DRIVERBINDING_SUPPORT_TOK),
> +  PERF_INIT_CUM_DATA (DRIVERBINDING_STOP_TOK)
>  };
> 
>  /// Number of items for which we are gathering cumulative statistics.
>  UINT32 const  NumCum = sizeof(CumData) / sizeof(PERF_CUM_DATA);
> 
> --
> 2.14.3.windows.1

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

Re: [edk2] [PATCH] OvmfPkg/AcpiPlatformDxe: clean up libs and protos in "AcpiPlatformDxe.inf"

[Laszlo Ersek]

On 07/18/18 16:02, Ard Biesheuvel wrote:
> On 18 July 2018 at 21:52, Laszlo Ersek  wrote:
>> None of the source files referenced by "AcpiPlatformDxe.inf" #include
>> "MdePkg/Include/Library/DxeServicesLib.h" or use interfaces declared
>> therein, so drop DxeServicesLib from [LibraryClasses].
>>
>> "AcpiPlatformDxe.inf" references "AcpiPlatform.c", which installs ACPI
>> tables built into the firmware image from under "OvmfPkg/AcpiTables/", in
>> case dynamically generated ACPI tables are not available from Xen or QEMU.
>> For this, the driver consumes gEfiFirmwareVolume2ProtocolGuid. Account for
>> that in [Protocols].
>>
>> Cc: Ard Biesheuvel 
>> Cc: Jordan Justen 
>> Cc: Steven Shi 
>> Reported-by: Steven Shi 
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1014
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek 
> 
> Reviewed-by: Ard Biesheuvel 

Thank you, commit a3fffb4f5e15.

Laszlo

> 
>> ---
>>
>> Notes:
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: acpiplatform_redundant_tiano_1014
>>
>>  OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf 
>> b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> index edc192e03d73..8440e7b343d8 100644
>> --- a/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> +++ b/OvmfPkg/AcpiPlatformDxe/AcpiPlatformDxe.inf
>> @@ -46,7 +46,6 @@ [Packages]
>>
>>  [LibraryClasses]
>>UefiLib
>> -  DxeServicesLib
>>PcdLib
>>BaseMemoryLib
>>DebugLib
>> @@ -62,6 +61,7 @@ [LibraryClasses]
>>
>>  [Protocols]
>>gEfiAcpiTableProtocolGuid # PROTOCOL ALWAYS_CONSUMED
>> +  gEfiFirmwareVolume2ProtocolGuid   # PROTOCOL 
>> SOMETIMES_CONSUMED
>>gEfiPciIoProtocolGuid # PROTOCOL 
>> SOMETIMES_CONSUMED
>>
>>  [Guids]
>> --
>> 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 v2] UefiCpuPkg/MpInitLib: Optimize get processor number performance.

[Laszlo Ersek]

On 07/18/18 07:19, Eric Dong wrote:
> Current function has low performance because it calls GetApicId
> many times.
> 
> New logic call GetApicId once and base on this value to search
> the processor.
> 
> V2 changes:
> Rollback V1 change which base on stack info to get AP index because
> this solution may return error AP index if stack buffer overflow.
> 
> Cc: Ruiyu Ni 
> Cc: Jeff Fan 
> Cc: Laszlo Ersek 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 722db2a01f..0bb0582985 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -435,16 +435,19 @@ GetProcessorNumber (
>UINTN   TotalProcessorNumber;
>UINTN   Index;
>CPU_INFO_IN_HOB *CpuInfoInHob;
> +  UINT32  CurrentApicId;
>  
>CpuInfoInHob = (CPU_INFO_IN_HOB *) (UINTN) CpuMpData->CpuInfoInHob;
>  
>TotalProcessorNumber = CpuMpData->CpuCount;
> +  CurrentApicId = GetApicId ();
>for (Index = 0; Index < TotalProcessorNumber; Index ++) {
> -if (CpuInfoInHob[Index].ApicId == GetApicId ()) {
> +if (CpuInfoInHob[Index].ApicId == CurrentApicId) {
>*ProcessorNumber = Index;
>return EFI_SUCCESS;
>  }
>}
> +
>return EFI_NOT_FOUND;
>  }
>  
> 

Nice and clean.

Reviewed-by: Laszlo Ersek 

Sorry about the delay in my getting to this patch.
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode

[Laszlo Ersek]

On 07/18/18 04:35, Wang, Jian J wrote:
> Hi Laszlo,
> 
> 
> Regards,
> Jian
> 
> 
>> -Original Message-
>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>> Sent: Tuesday, July 17, 2018 10:37 PM
>> To: Wang, Jian J ; edk2-devel@lists.01.org
>> Cc: Dong, Eric ; Yao, Jiewen ;
>> Zeng, Star 
>> Subject: Re: [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode
>>
>> On 07/13/18 07:53, Jian J Wang wrote:
>>> Current IsInSmm() method makes use of gEfiSmmBase2ProtocolGuid.InSmm()
>> to
>>> check if current processor is in SMM mode or not. But this is not correct
>>> because gEfiSmmBase2ProtocolGuid.InSmm() can only detect if the caller is
>>> running in SMRAM or from SMM driver. It cannot guarantee if the caller is
>>> running in SMM mode.
>>
>> Wow. This is the exact difference which I asked about, in my question
>> (9b), and I was assured that InSmm() actually determined whether we were
>> executing in Management Mode (meaning the processor operating mode).
>>
>> http://mid.mail-
>> archive.com/0c09afa07dd0434d9e2a0c6aeb0483103bb55...@shsmsx102.cc
>> r.corp.intel.com
>>
>> (Scroll down in that message to see my original question (9b).)
>>
>> So why doesn't Star's explanation, from the original thread, apply
>> ultimately?
>>
> 
> We did many tests for this and didn't found any issue. So I took a risk. (I 
> thought
> a very precise check of SMM mode is hard and time consuming.) 
> 
>>> Because SMM mode will load its own page table, adding
>>> an extra check of saved DXE page table base address against current CR3
>>> register value can help to get the correct answer for sure (in SMM mode or
>>> not in SMM mode).
>>
>> So, apparently, the PI spec offers no standard way for a platform module
>> to determine whether it runs in Management Mode, despite protocol member
>> being called "InSmm". Do we need a PI spec ECR for introducing the
>> needed facility?
>>
>> Alternatively, the PI spec might already intend to specify that, but the
>> edk2 implementation doesn't do what the PI spec requires.
>>
>> Which one is the case?
>>
> 
> The implementation conforms to the spec. It's my misunderstanding. But it's 
> true
> that there's no specific protocol API to determine if it's in SMM mode or not.
> 
>>>
>>> This is an issue caused by check-in at
>>>
>>>   d106cf71eabaacff63c14626a4a87346b93074dd
>>
>> I disagree; I think the issue was introduced in commit 2a1408d1d739
>> ("UefiCpuPkg/CpuDxe: allow accessing (DXE) page table in SMM mode",
>> 2018-06-19).
>>
> 
> You're right. Thanks for pointing this out.
> 
>>
>> How did you encounter / find this issue?
>>
> 
> I didn't find it. The issue came to me. In other words, I think it's random 
> and hard
> to reproduce it. Maybe a subtle change in boot sequence will hide it away.
> 
>>>
>>> Cc: Eric Dong 
>>> Cc: Laszlo Ersek 
>>> Cc: Jiewen Yao 
>>> Cc: Star Zeng 
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Jian J Wang 
>>> ---
>>>  UefiCpuPkg/CpuDxe/CpuPageTable.c | 9 -
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
>> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
>>> index 850eed60e7..df021798c0 100644
>>> --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
>>> +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
>>> @@ -136,7 +136,14 @@ IsInSmm (
>>>  mSmmBase2->InSmm (mSmmBase2, );
>>>}
>>>
>>> -  return InSmm;
>>> +  //
>>> +  // mSmmBase2->InSmm() can only detect if the caller is running in SMRAM
>>> +  // or from SMM driver. It cannot tell if the caller is running in SMM 
>>> mode.
>>> +  // Check page table base address to guarantee that because SMM mode
>> willl
>>> +  // load its own page table.
>>> +  //
>>> +  return (InSmm &&
>>> +  mPagingContext.ContextData.X64.PageTableBase !=
>> (UINT64)AsmReadCr3());
>>>  }
>>>
>>>  /**
>>>
>>
>> Shouldn't we consider Ia32.PageTableBase when that's appropriate? From
>> "UefiCpuPkg/CpuDxe/CpuPageTable.h":
>>
>> typedef struct {
>>   UINT32  PageTableBase;
>>   UINT32  Reserved;
>>   UINT32  Attributes;
>> } PAGE_TABLE_LIB_PAGING_CONTEXT_IA32;
>>
>> typedef struct {
>>   UINT64  PageTableBase;
>>   UINT32  Attributes;
>> } PAGE_TABLE_LIB_PAGING_CONTEXT_X64;
>>
>> typedef union {
>>   PAGE_TABLE_LIB_PAGING_CONTEXT_IA32  Ia32;
>>   PAGE_TABLE_LIB_PAGING_CONTEXT_X64   X64;
>> } PAGE_TABLE_LIB_PAGING_CONTEXT_DATA;
>>
>> The Ia32/X64 structure types are not packed, and even if they were, I
>> wouldn't think we should rely on "Reserved" being zero.
>>
> 
> mPagingContext is zero-ed at each update in GetCurrentPagingContext().
> I think it should be no problem to use X64.
> 
>>
>> All in all, I think that determining whether the processor is operating
>> in Management Mode (regardless of where in RAM the processor is
>> executing code from) is a core functionality that should be offered as a
>> central service, not just an internal CpuDxe function. I think we need
>> either a PI spec addition, or at least an EDKII extension protocol. It's
>> 

Re: [edk2] [patch] UefiCpuPkg/MpInitLib: Fix VS2012 build failure

[Bi, Dandan]

Sure.
I will add the comments before commit it.

Thanks,
Dandan

-Original Message-
From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo 
Ersek
Sent: Thursday, July 19, 2018 8:04 PM
To: Bi, Dandan ; edk2-devel@lists.01.org
Cc: Dong, Eric 
Subject: Re: [edk2] [patch] UefiCpuPkg/MpInitLib: Fix VS2012 build failure

Hi Dandan,

On 07/19/18 06:50, Dandan Bi wrote:
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi 
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index efda143e67..9726172fbd 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -61,10 +61,12 @@ MicrocodeDetect (
>VOID*MicrocodeData;
>MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
>UINT32  ProcessorFlags;
>UINT32  ThreadId;
>  
> +  ProcessorFlags = 0;
> +
>if (CpuMpData->MicrocodePatchRegionSize == 0) {
>  //
>  // There is no microcode patches
>  //
>  return;
> 

can you please add the comment from

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

namely:

  //
  // set ProcessorFlags to suppress incorrect compiler/analyzer warnings
  //
  ProcessorFlags = 0;

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

Re: [edk2] [PATCH 2/6] MdeModulePkg/RamDiskDxe: replace OpenFileByDevicePath() with UefiLib API

[Laszlo Ersek]

Hi Star,

On 07/19/18 12:36, Zeng, Star wrote:
> Hi Laszlo,
> 
> Have you evaluated whether " #include  " can be 
> removed from *.h file or not since gEfiSimpleFileSystemProtocolGuid is 
> removed from *.inf?

Yes, I did check for that.

I did not remove the #include directive of the SimpleFileSystem protocol
header because the header defines a number of structure types, beyond
declaring "gEfiSimpleFileSystemProtocolGuid", and beyond #defining the
EFI_SIMPLE_FILE_SYSTEM_PROTOCOL_GUID.

In particular, it defines the (UEFI-standard) type EFI_FILE_PROTOCOL,
and the (non-standard) type EFI_FILE_HANDLE. RamDiskDxe uses the
EFI_FILE_HANDLE type liberally -- after the application of this patch,
four (4) uses of EFI_FILE_HANDLE remain.

Now, in patch #1, I do #include  in
UefiLib.h. However, that is because all APIs declared by a lib class
header should be usable at once, without having to hunt down
dependencies manually. This implies that types used in function
prototypes should be declared automatically for the client C file.

Therefore, the #include directive added to UefiLib.h, in patch #1, is
not a substitute for the existent #include directive in RamDiskDxe.
UefiLib.h needs the #include directive for staying self-contained, and
RamDiskDxe needs the #include directive because it uses the
EFI_FILE_HANDLE type for its own purposes.

(Same for the rest of the modules, and same for the device path protocol
header / GUID, wherever appropriate.)

Thanks,
Laszlo

> 
> Thanks,
> Star
> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com] 
> Sent: Thursday, July 19, 2018 4:51 AM
> To: edk2-devel-01 
> Cc: Dong, Eric ; Wu, Jiaxin ; Ni, 
> Ruiyu ; Fu, Siyuan ; Zeng, Star 
> 
> Subject: [PATCH 2/6] MdeModulePkg/RamDiskDxe: replace OpenFileByDevicePath() 
> with UefiLib API
> 
> Replace the OpenFileByDevicePath() function with EfiOpenFileByDevicePath() 
> from UefiLib, correcting the following issues:
> 
> - imprecise comments on OpenFileByDevicePath(),
> - code duplication between this module and other modules,
> - local variable name "EfiSimpleFileSystemProtocol" starting with "Efi"
>   prefix,
> - bogus "FileHandle = NULL" assignments,
> - passing a potentially unaligned "FILEPATH_DEVICE_PATH.PathName" field to
>   a protocol member function (forbidden by the UEFI spec),
> - leaking "Handle1" when the device path type/subtype check fails in the
>   loop,
> - stale SHELL_FILE_HANDLE reference in a comment.
> 
> Cc: Eric Dong 
> Cc: Jiaxin Wu 
> Cc: Ruiyu Ni 
> Cc: Siyuan Fu 
> Cc: Star Zeng 
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek 
> ---
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf|   1 -
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h |  39 --
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c | 140 
> 
>  MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c |   2 +-
>  4 files changed, 1 insertion(+), 181 deletions(-)
> 
> diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf 
> b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
> index cdd2da690411..da00e4a420e7 100644
> --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
> +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
> @@ -75,7 +75,6 @@ [Protocols]
>gEfiDevicePathProtocolGuid ## PRODUCES
>gEfiBlockIoProtocolGuid## PRODUCES
>gEfiBlockIo2ProtocolGuid   ## PRODUCES
> -  gEfiSimpleFileSystemProtocolGuid   ## SOMETIMES_CONSUMES
>gEfiAcpiTableProtocolGuid  ## SOMETIMES_CONSUMES
>gEfiAcpiSdtProtocolGuid## SOMETIMES_CONSUMES
>  
> diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h 
> b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
> index 077bb77b25bf..08a8ca94c9db 100644
> --- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
> +++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
> @@ -605,45 +605,6 @@ FileInfo (
>);
>  
>  
> -/**
> -  This function will open a file or directory referenced by DevicePath.
> -
> -  This function opens a file with the open mode according to the file path. 
> The
> -  Attributes is valid only for EFI_FILE_MODE_CREATE.
> -
> -  @param[in, out] FilePath   On input, the device path to the file.
> - On output, the remaining device path.
> -  @param[out] FileHandle Pointer to the file handle.
> -  @param[in]  OpenMode   The mode to open the file with.
> -  @param[in]  Attributes The file's file attributes.
> -
> -  @retval EFI_SUCCESS The information was set.
> -  @retval EFI_INVALID_PARAMETER   One of the parameters has an invalid value.
> -  @retval EFI_UNSUPPORTED Could not open the file path.
> -  @retval EFI_NOT_FOUND   The specified file could not be 

Re: [edk2] [Patch] UefiCpuPkg/MpInitLib: Remove useless code.

[Laszlo Ersek]

On 07/19/18 14:12, Eric Dong wrote:
> Remove the useless code error added by change
> 58942277bcbf41abda5f6e3a1c89d571105d5983.
> 
> Cc: Laszlo Ersek 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 15 ---
>  1 file changed, 15 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index 06d966b227..e6e1b7c57d 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> @@ -44,21 +44,6 @@ EFI_PEI_NOTIFY_DESCRIPTORmS3SmmInitDoneNotifyDesc 
> = {
>NotifyOnS3SmmInitDonePpi
>  };
>  
> -/**
> -  The function prototype for invoking a function on an Application Processor.
> -
> -  This definition is used by the UEFI MP Serices Protocol, and the
> -  PI SMM System Table.
> -
> -  @param[in,out] Buffer  The pointer to private data buffer.
> -**/
> -VOID
> -EmptyApProcedure (
> -  IN OUT VOID * Buffer
> -  )
> -{
> -}
> -
>  /**
>S3 SMM Init Done notification function.
>  
> 


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

Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()

[Yao, Jiewen]

Cool. Thanks!


> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Thursday, July 19, 2018 6:47 PM
> To: Yao, Jiewen 
> Cc: edk2-devel-01 ; Zhang, Chao B
> ; Dong, Eric ; Carsey, Jaben
> ; Wu, Jiaxin ; Gao, Liming
> ; Kinney, Michael D ;
> Roman Bacik ; Ni, Ruiyu ;
> Fu, Siyuan ; Zeng, Star 
> Subject: Re: [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()
> 
> On 07/19/18 01:10, Yao, Jiewen wrote:
> > Thanks Laszlo.
> >
> > Would you please add one line comment on the FilePath, to describe if the
> FilePath is internal input or external input? As such the API consumer can
> know if caller’s responsibility to verify it or callee’s responsibility.
> 
> Good point. The caller is responsible for ensuring the well-formedness
> of the device path pointed-to by (*FilePath). The function uses
> DevicePathLib APIs such as NextDevicePathNode(), and if e.g. some device
> path node headers are invalid (containing bogus lengths, for example),
> then the function won't work as expected.
> 
> I'll add such a comment.
> 
> > For example, if the caller gets path from a read write variable, and input 
> > it
> directly, the this API need validate before use.
> > If the caller already does the verification, then this API can use it 
> > directly.
> 
> Right.
> 
> (A separate question would be if edk2 offers facilities for verifying
> the well-formedness of any given device path -- it looks like a complex
> task, to implement everywhere.)
> 
> In this series I'd just like to extract the duplicated code to a common
> location, and fix its bugs. I didn't try to change the interface
> contract (just to document it more precisely). If we'd like to "armor"
> this function against maliciously formatted device paths, IMO that
> should be done separately.
> 
> So I agree that the comment you suggest should be added.
> 
> > Sanity check is just for the format, not the content.
> > The question I have is: Where should the sanity check be?
> 
> At the moment: in the caller.
> 
> Thanks!
> Laszlo
> 
> >
> > thank you!
> > Yao, Jiewen
> >
> >
> >> 在 2018年7月19日，上午4:50，Laszlo Ersek  写
> 道：
> >>
> >> The EfiOpenFileByDevicePath() function centralizes functionality from
> >>
> >> - MdeModulePkg/Universal/Disk/RamDiskDxe
> >> - NetworkPkg/TlsAuthConfigDxe
> >> - SecurityPkg/VariableAuthenticated/SecureBootConfigDxe
> >> - ShellPkg/Library/UefiShellLib
> >>
> >> unifying the implementation and fixing various bugs.
> >>
> >> Cc: Chao Zhang 
> >> Cc: Eric Dong 
> >> Cc: Jaben Carsey 
> >> Cc: Jiaxin Wu 
> >> Cc: Jiewen Yao 
> >> Cc: Liming Gao 
> >> Cc: Michael D Kinney 
> >> Cc: Roman Bacik 
> >> Cc: Ruiyu Ni 
> >> Cc: Siyuan Fu 
> >> Cc: Star Zeng 
> >> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Laszlo Ersek 
> >> ---
> >> MdePkg/Library/UefiLib/UefiLib.inf |   1 +
> >> MdePkg/Include/Library/UefiLib.h   |  86 
> >> MdePkg/Library/UefiLib/UefiLib.c   | 226 
> >> 3 files changed, 313 insertions(+)
> >>
> >> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf
> b/MdePkg/Library/UefiLib/UefiLib.inf
> >> index f69f0a43b576..a6c739ef3d6d 100644
> >> --- a/MdePkg/Library/UefiLib/UefiLib.inf
> >> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
> >> @@ -68,6 +68,7 @@ [Protocols]
> >>   gEfiSimpleTextOutProtocolGuid   ##
> SOMETIMES_CONSUMES
> >>   gEfiGraphicsOutputProtocolGuid  ##
> SOMETIMES_CONSUMES
> >>   gEfiHiiFontProtocolGuid ##
> SOMETIMES_CONSUMES
> >> +  gEfiSimpleFileSystemProtocolGuid##
> SOMETIMES_CONSUMES
> >>   gEfiUgaDrawProtocolGuid |
> gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport ##
> SOMETIMES_CONSUMES # Consumes if gEfiGraphicsOutputProtocolGuid
> uninstalled
> >>   gEfiComponentNameProtocolGuid  | NOT
> gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable   ##
> SOMETIMES_PRODUCES # User chooses to produce it
> >>   gEfiComponentName2ProtocolGuid | NOT
> gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable  ##
> SOMETIMES_PRODUCES # User chooses to produce it
> >> diff --git a/MdePkg/Include/Library/UefiLib.h
> b/MdePkg/Include/Library/UefiLib.h
> >> index 7c6fde620c74..2468bf2aee80 100644
> >> --- a/MdePkg/Include/Library/UefiLib.h
> >> +++ b/MdePkg/Include/Library/UefiLib.h
> >> @@ -33,6 +33,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF
> ANY KIND, EITHER EXPRESS OR IMPLIED.
> >> #include 
> >> #include 
> >> #include 
> >> +#include 
> >> +#include 
> >>
> >> #include 
> >>
> >> @@ -1520,4 +1522,88 @@ EfiLocateProtocolBuffer (
> >>   OUT UINTN *NoProtocols,
> >>   OUT VOID  ***Buffer
> >>   );
> >> +
> >> +/**
> >> +  Open or create a file or directory, possibly creating the chain of
> >> +  directories leading up to the directory.
> >> +
> >> +  EfiOpenFileByDevicePath() first locates
> EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on
> >> +  FilePath, and opens the root directory of that 

Re: [edk2] [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode

[Laszlo Ersek]

On 07/19/18 11:07, Wang, Jian J wrote:
> Hi Laszlo,
> 
> Do you have more comments? Or can you give a r-b?

Struggling with my workload, will try to come back to this soon.

Thanks
Laszlo

>> -Original Message-
>> From: Wang, Jian J
>> Sent: Wednesday, July 18, 2018 10:36 AM
>> To: Laszlo Ersek ; edk2-devel@lists.01.org
>> Cc: Dong, Eric ; Yao, Jiewen ;
>> Zeng, Star 
>> Subject: RE: [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode
>>
>> Hi Laszlo,
>>
>>
>> Regards,
>> Jian
>>
>>
>>> -Original Message-
>>> From: Laszlo Ersek [mailto:ler...@redhat.com]
>>> Sent: Tuesday, July 17, 2018 10:37 PM
>>> To: Wang, Jian J ; edk2-devel@lists.01.org
>>> Cc: Dong, Eric ; Yao, Jiewen ;
>>> Zeng, Star 
>>> Subject: Re: [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode
>>>
>>> On 07/13/18 07:53, Jian J Wang wrote:
 Current IsInSmm() method makes use of gEfiSmmBase2ProtocolGuid.InSmm()
>>> to
 check if current processor is in SMM mode or not. But this is not correct
 because gEfiSmmBase2ProtocolGuid.InSmm() can only detect if the caller is
 running in SMRAM or from SMM driver. It cannot guarantee if the caller is
 running in SMM mode.
>>>
>>> Wow. This is the exact difference which I asked about, in my question
>>> (9b), and I was assured that InSmm() actually determined whether we were
>>> executing in Management Mode (meaning the processor operating mode).
>>>
>>> http://mid.mail-
>>>
>> archive.com/0c09afa07dd0434d9e2a0c6aeb0483103bb55...@shsmsx102.cc
>>> r.corp.intel.com
>>>
>>> (Scroll down in that message to see my original question (9b).)
>>>
>>> So why doesn't Star's explanation, from the original thread, apply
>>> ultimately?
>>>
>>
>> We did many tests for this and didn't found any issue. So I took a risk. (I 
>> thought
>> a very precise check of SMM mode is hard and time consuming.)
>>
 Because SMM mode will load its own page table, adding
 an extra check of saved DXE page table base address against current CR3
 register value can help to get the correct answer for sure (in SMM mode or
 not in SMM mode).
>>>
>>> So, apparently, the PI spec offers no standard way for a platform module
>>> to determine whether it runs in Management Mode, despite protocol member
>>> being called "InSmm". Do we need a PI spec ECR for introducing the
>>> needed facility?
>>>
>>> Alternatively, the PI spec might already intend to specify that, but the
>>> edk2 implementation doesn't do what the PI spec requires.
>>>
>>> Which one is the case?
>>>
>>
>> The implementation conforms to the spec. It's my misunderstanding. But it's
>> true
>> that there's no specific protocol API to determine if it's in SMM mode or 
>> not.
>>

 This is an issue caused by check-in at

   d106cf71eabaacff63c14626a4a87346b93074dd
>>>
>>> I disagree; I think the issue was introduced in commit 2a1408d1d739
>>> ("UefiCpuPkg/CpuDxe: allow accessing (DXE) page table in SMM mode",
>>> 2018-06-19).
>>>
>>
>> You're right. Thanks for pointing this out.
>>
>>>
>>> How did you encounter / find this issue?
>>>
>>
>> I didn't find it. The issue came to me. In other words, I think it's random 
>> and hard
>> to reproduce it. Maybe a subtle change in boot sequence will hide it away.
>>

 Cc: Eric Dong 
 Cc: Laszlo Ersek 
 Cc: Jiewen Yao 
 Cc: Star Zeng 
 Contributed-under: TianoCore Contribution Agreement 1.1
 Signed-off-by: Jian J Wang 
 ---
  UefiCpuPkg/CpuDxe/CpuPageTable.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

 diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
>>> b/UefiCpuPkg/CpuDxe/CpuPageTable.c
 index 850eed60e7..df021798c0 100644
 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
 +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
 @@ -136,7 +136,14 @@ IsInSmm (
  mSmmBase2->InSmm (mSmmBase2, );
}

 -  return InSmm;
 +  //
 +  // mSmmBase2->InSmm() can only detect if the caller is running in SMRAM
 +  // or from SMM driver. It cannot tell if the caller is running in SMM 
 mode.
 +  // Check page table base address to guarantee that because SMM mode
>>> willl
 +  // load its own page table.
 +  //
 +  return (InSmm &&
 +  mPagingContext.ContextData.X64.PageTableBase !=
>>> (UINT64)AsmReadCr3());
  }

  /**

>>>
>>> Shouldn't we consider Ia32.PageTableBase when that's appropriate? From
>>> "UefiCpuPkg/CpuDxe/CpuPageTable.h":
>>>
>>> typedef struct {
>>>   UINT32  PageTableBase;
>>>   UINT32  Reserved;
>>>   UINT32  Attributes;
>>> } PAGE_TABLE_LIB_PAGING_CONTEXT_IA32;
>>>
>>> typedef struct {
>>>   UINT64  PageTableBase;
>>>   UINT32  Attributes;
>>> } PAGE_TABLE_LIB_PAGING_CONTEXT_X64;
>>>
>>> typedef union {
>>>   PAGE_TABLE_LIB_PAGING_CONTEXT_IA32  Ia32;
>>>   PAGE_TABLE_LIB_PAGING_CONTEXT_X64   X64;
>>> } PAGE_TABLE_LIB_PAGING_CONTEXT_DATA;
>>>
>>> The Ia32/X64 structure types are not packed, and even if they were, I
>>> 

[edk2] [Patch] UefiCpuPkg/MpInitLib: Remove useless code.

[Eric Dong]

Remove the useless code error added by change
58942277bcbf41abda5f6e3a1c89d571105d5983.

Cc: Laszlo Ersek 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 UefiCpuPkg/Library/MpInitLib/PeiMpLib.c | 15 ---
 1 file changed, 15 deletions(-)

diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c 
b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
index 06d966b227..e6e1b7c57d 100644
--- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
+++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
@@ -44,21 +44,6 @@ EFI_PEI_NOTIFY_DESCRIPTORmS3SmmInitDoneNotifyDesc = {
   NotifyOnS3SmmInitDonePpi
 };
 
-/**
-  The function prototype for invoking a function on an Application Processor.
-
-  This definition is used by the UEFI MP Serices Protocol, and the
-  PI SMM System Table.
-
-  @param[in,out] Buffer  The pointer to private data buffer.
-**/
-VOID
-EmptyApProcedure (
-  IN OUT VOID * Buffer
-  )
-{
-}
-
 /**
   S3 SMM Init Done notification function.
 
-- 
2.15.0.windows.1

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

Re: [edk2] UefiCpuPkg/MpInitLib: Fix S3 resume hang issue.

[Dong, Eric]

Hi Laszlo,

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Thursday, July 19, 2018 7:57 PM
> To: Dong, Eric ; edk2-devel@lists.01.org
> Cc: Ni, Ruiyu 
> Subject: Re: [edk2] UefiCpuPkg/MpInitLib: Fix S3 resume hang issue.
> 
> Hi,
> 
> On 07/18/18 07:10, Eric Dong wrote:
> > When resume from S3 and CPU loop mode is MWait mode, if driver calls
> > APs to do task at EndOfPei point, the APs can't been wake up and bios
> > hang at that point.
> >
> > The root cause is PiSmmCpuDxeSmm driver wakes up APs with HLT mode
> > during S3 resume phase to do SMM relocation.
> > After this task, PiSmmCpuDxeSmm driver not restore APs context which
> > make CpuMpPei driver saved wake up buffer not works.
> >
> > The solution for this issue is let CpuMpPei driver hook S3SmmInitDone
> > ppi notification. In this notify function, it check whether Cpu Loop
> > mode is not HLT mode. If yes, CpuMpPei driver will set a flag to force
> > BSP use INIT-SIPI -SIPI command to wake up the APs.
> >
> > Cc: Laszlo Ersek 
> > Cc: Ruiyu Ni 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Eric Dong 
> > ---
> >  UefiCpuPkg/Library/MpInitLib/MpLib.c  | 16 -
> >  UefiCpuPkg/Library/MpInitLib/MpLib.h  |  9 +++
> >  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |  4 ++
> >  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c   | 89
> +++
> >  4 files changed, 116 insertions(+), 2 deletions(-)
> 
> I see this patch is already committed (58942277bcbf); I have two comments in
> retrospect:
> 
> > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > index 722db2a01f..e5c701ddeb 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> > @@ -985,13 +985,15 @@ WakeUpAP (
> >CpuMpData->FinishedCount = 0;
> >ResetVectorRequired = FALSE;
> >
> > -  if (CpuMpData->ApLoopMode == ApInHltLoop ||
> > +  if (CpuMpData->WakeUpByInitSipiSipi ||
> >CpuMpData->InitFlag   != ApInitDone) {
> >  ResetVectorRequired = TRUE;
> >  AllocateResetVector (CpuMpData);
> >  FillExchangeInfoData (CpuMpData);
> >  SaveLocalApicTimerSetting (CpuMpData);
> > -  } else if (CpuMpData->ApLoopMode == ApInMwaitLoop) {
> > +  }
> > +
> > +  if (CpuMpData->ApLoopMode == ApInMwaitLoop) {
> >  //
> >  // Get AP target C-state each time when waking up AP,
> >  // for it maybe updated by platform again @@ -1076,6 +1078,13 @@
> > WakeUpAP (
> >if (ResetVectorRequired) {
> >  FreeResetVector (CpuMpData);
> >}
> > +
> > +  //
> > +  // After one round of Wakeup Ap actions, need to re-sync ApLoopMode
> > + with  // WakeUpByInitSipiSipi flag. WakeUpByInitSipiSipi flag maybe
> > + changed by  // S3SmmInitDone Ppi.
> > +  //
> > +  CpuMpData->WakeUpByInitSipiSipi = (CpuMpData->ApLoopMode ==
> > + ApInHltLoop);
> >  }
> >
> >  /**
> > @@ -1648,6 +1657,9 @@ MpInitLibInitialize (
> >//
> >CpuMpData->ApLoopMode = ApLoopMode;
> >DEBUG ((DEBUG_INFO, "AP Loop Mode is %d\n",
> > CpuMpData->ApLoopMode));
> > +
> > +  CpuMpData->WakeUpByInitSipiSipi = (CpuMpData->ApLoopMode ==
> > + ApInHltLoop);
> > +
> >//
> >// Set up APs wakeup signal buffer
> >//
> > diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > index 6958080ac1..9d0b866d09 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> > @@ -250,6 +250,15 @@ struct _CPU_MP_DATA {
> >UINT32 ProcessorFlags;
> >UINT64 MicrocodeDataAddress;
> >UINT32 MicrocodeRevision;
> > +
> > +  //
> > +  // Whether need to use Init-Sipi-Sipi to wake up the APs.
> > +  // Two cases need to set this value to TRUE. One is in HLT  // loop
> > + mode, the other is resume from S3 which loop mode  // will be
> > + hardcode change to HLT mode by PiSmmCpuDxeSmm  // driver.
> > +  //
> > +  BOOLEANWakeUpByInitSipiSipi;
> >  };
> >
> >  extern EFI_GUID mCpuInitMpLibHobGuid;
> > diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> > index fa84e392af..43a3b3b036 100644
> > --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> > +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> > @@ -44,6 +44,7 @@
> >  [Packages]
> >MdePkg/MdePkg.dec
> >UefiCpuPkg/UefiCpuPkg.dec
> > +  MdeModulePkg/MdeModulePkg.dec
> >
> >  [LibraryClasses]
> >BaseLib
> > @@ -54,6 +55,7 @@
> >CpuLib
> >UefiCpuLib
> >SynchronizationLib
> > +  PeiServicesLib
> >
> >  [Pcd]
> >gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber##
> CONSUMES
> > @@ -64,3 +66,5 @@
> >gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode   ##
> CONSUMES
> >gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate   

Re: [edk2] [patch] UefiCpuPkg/MpInitLib: Fix VS2012 build failure

[Laszlo Ersek]

Hi Dandan,

On 07/19/18 06:50, Dandan Bi wrote:
> Cc: Eric Dong 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi 
> ---
>  UefiCpuPkg/Library/MpInitLib/Microcode.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/UefiCpuPkg/Library/MpInitLib/Microcode.c 
> b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> index efda143e67..9726172fbd 100644
> --- a/UefiCpuPkg/Library/MpInitLib/Microcode.c
> +++ b/UefiCpuPkg/Library/MpInitLib/Microcode.c
> @@ -61,10 +61,12 @@ MicrocodeDetect (
>VOID*MicrocodeData;
>MSR_IA32_PLATFORM_ID_REGISTER   PlatformIdMsr;
>UINT32  ProcessorFlags;
>UINT32  ThreadId;
>  
> +  ProcessorFlags = 0;
> +
>if (CpuMpData->MicrocodePatchRegionSize == 0) {
>  //
>  // There is no microcode patches
>  //
>  return;
> 

can you please add the comment from

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

namely:

  //
  // set ProcessorFlags to suppress incorrect compiler/analyzer warnings
  //
  ProcessorFlags = 0;

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

Re: [edk2] UefiCpuPkg/MpInitLib: Fix S3 resume hang issue.

[Laszlo Ersek]

Hi,

On 07/18/18 07:10, Eric Dong wrote:
> When resume from S3 and CPU loop mode is MWait mode,
> if driver calls APs to do task at EndOfPei point, the
> APs can't been wake up and bios hang at that point.
>
> The root cause is PiSmmCpuDxeSmm driver wakes up APs
> with HLT mode during S3 resume phase to do SMM relocation.
> After this task, PiSmmCpuDxeSmm driver not restore APs
> context which make CpuMpPei driver saved wake up buffer
> not works.
>
> The solution for this issue is let CpuMpPei driver hook
> S3SmmInitDone ppi notification. In this notify function,
> it check whether Cpu Loop mode is not HLT mode. If yes,
> CpuMpPei driver will set a flag to force BSP use INIT-SIPI
> -SIPI command to wake up the APs.
>
> Cc: Laszlo Ersek 
> Cc: Ruiyu Ni 
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong 
> ---
>  UefiCpuPkg/Library/MpInitLib/MpLib.c  | 16 -
>  UefiCpuPkg/Library/MpInitLib/MpLib.h  |  9 +++
>  UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf |  4 ++
>  UefiCpuPkg/Library/MpInitLib/PeiMpLib.c   | 89 
> +++
>  4 files changed, 116 insertions(+), 2 deletions(-)

I see this patch is already committed (58942277bcbf); I have two
comments in retrospect:

> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> index 722db2a01f..e5c701ddeb 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c
> @@ -985,13 +985,15 @@ WakeUpAP (
>CpuMpData->FinishedCount = 0;
>ResetVectorRequired = FALSE;
>
> -  if (CpuMpData->ApLoopMode == ApInHltLoop ||
> +  if (CpuMpData->WakeUpByInitSipiSipi ||
>CpuMpData->InitFlag   != ApInitDone) {
>  ResetVectorRequired = TRUE;
>  AllocateResetVector (CpuMpData);
>  FillExchangeInfoData (CpuMpData);
>  SaveLocalApicTimerSetting (CpuMpData);
> -  } else if (CpuMpData->ApLoopMode == ApInMwaitLoop) {
> +  }
> +
> +  if (CpuMpData->ApLoopMode == ApInMwaitLoop) {
>  //
>  // Get AP target C-state each time when waking up AP,
>  // for it maybe updated by platform again
> @@ -1076,6 +1078,13 @@ WakeUpAP (
>if (ResetVectorRequired) {
>  FreeResetVector (CpuMpData);
>}
> +
> +  //
> +  // After one round of Wakeup Ap actions, need to re-sync ApLoopMode with
> +  // WakeUpByInitSipiSipi flag. WakeUpByInitSipiSipi flag maybe changed by
> +  // S3SmmInitDone Ppi.
> +  //
> +  CpuMpData->WakeUpByInitSipiSipi = (CpuMpData->ApLoopMode == ApInHltLoop);
>  }
>
>  /**
> @@ -1648,6 +1657,9 @@ MpInitLibInitialize (
>//
>CpuMpData->ApLoopMode = ApLoopMode;
>DEBUG ((DEBUG_INFO, "AP Loop Mode is %d\n", CpuMpData->ApLoopMode));
> +
> +  CpuMpData->WakeUpByInitSipiSipi = (CpuMpData->ApLoopMode == ApInHltLoop);
> +
>//
>// Set up APs wakeup signal buffer
>//
> diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.h 
> b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> index 6958080ac1..9d0b866d09 100644
> --- a/UefiCpuPkg/Library/MpInitLib/MpLib.h
> +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.h
> @@ -250,6 +250,15 @@ struct _CPU_MP_DATA {
>UINT32 ProcessorFlags;
>UINT64 MicrocodeDataAddress;
>UINT32 MicrocodeRevision;
> +
> +  //
> +  // Whether need to use Init-Sipi-Sipi to wake up the APs.
> +  // Two cases need to set this value to TRUE. One is in HLT
> +  // loop mode, the other is resume from S3 which loop mode
> +  // will be hardcode change to HLT mode by PiSmmCpuDxeSmm
> +  // driver.
> +  //
> +  BOOLEANWakeUpByInitSipiSipi;
>  };
>
>  extern EFI_GUID mCpuInitMpLibHobGuid;
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf 
> b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> index fa84e392af..43a3b3b036 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf
> @@ -44,6 +44,7 @@
>  [Packages]
>MdePkg/MdePkg.dec
>UefiCpuPkg/UefiCpuPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
>
>  [LibraryClasses]
>BaseLib
> @@ -54,6 +55,7 @@
>CpuLib
>UefiCpuLib
>SynchronizationLib
> +  PeiServicesLib
>
>  [Pcd]
>gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber## 
> CONSUMES
> @@ -64,3 +66,5 @@
>gUefiCpuPkgTokenSpaceGuid.PcdCpuApLoopMode   ## 
> CONSUMES
>gUefiCpuPkgTokenSpaceGuid.PcdCpuApTargetCstate   ## 
> SOMETIMES_CONSUMES
>
> +[Guids]
> +  gEdkiiS3SmmInitDoneGuid
> \ No newline at end of file
> diff --git a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c 
> b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> index 92f28681e4..06d966b227 100644
> --- a/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> +++ b/UefiCpuPkg/Library/MpInitLib/PeiMpLib.c
> @@ -13,6 +13,87 @@
>  **/
>
>  #include "MpLib.h"
> +#include 
> +#include 
> +
> +/**
> +  S3 SMM Init Done notification function.
> +
> +  @param  PeiServices  Indirect reference to the PEI Services 

Re: [edk2] [PATCH 1/6] MdePkg/UefiLib: introduce EfiOpenFileByDevicePath()

[Laszlo Ersek]

On 07/19/18 01:10, Yao, Jiewen wrote:
> Thanks Laszlo.
> 
> Would you please add one line comment on the FilePath, to describe if the 
> FilePath is internal input or external input? As such the API consumer can 
> know if caller’s responsibility to verify it or callee’s responsibility.

Good point. The caller is responsible for ensuring the well-formedness
of the device path pointed-to by (*FilePath). The function uses
DevicePathLib APIs such as NextDevicePathNode(), and if e.g. some device
path node headers are invalid (containing bogus lengths, for example),
then the function won't work as expected.

I'll add such a comment.

> For example, if the caller gets path from a read write variable, and input it 
> directly, the this API need validate before use. 
> If the caller already does the verification, then this API can use it 
> directly. 

Right.

(A separate question would be if edk2 offers facilities for verifying
the well-formedness of any given device path -- it looks like a complex
task, to implement everywhere.)

In this series I'd just like to extract the duplicated code to a common
location, and fix its bugs. I didn't try to change the interface
contract (just to document it more precisely). If we'd like to "armor"
this function against maliciously formatted device paths, IMO that
should be done separately.

So I agree that the comment you suggest should be added.

> Sanity check is just for the format, not the content. 
> The question I have is: Where should the sanity check be?

At the moment: in the caller.

Thanks!
Laszlo

> 
> thank you!
> Yao, Jiewen
> 
> 
>> 在 2018年7月19日，上午4:50，Laszlo Ersek  写道：
>>
>> The EfiOpenFileByDevicePath() function centralizes functionality from
>>
>> - MdeModulePkg/Universal/Disk/RamDiskDxe
>> - NetworkPkg/TlsAuthConfigDxe
>> - SecurityPkg/VariableAuthenticated/SecureBootConfigDxe
>> - ShellPkg/Library/UefiShellLib
>>
>> unifying the implementation and fixing various bugs.
>>
>> Cc: Chao Zhang 
>> Cc: Eric Dong 
>> Cc: Jaben Carsey 
>> Cc: Jiaxin Wu 
>> Cc: Jiewen Yao 
>> Cc: Liming Gao 
>> Cc: Michael D Kinney 
>> Cc: Roman Bacik 
>> Cc: Ruiyu Ni 
>> Cc: Siyuan Fu 
>> Cc: Star Zeng 
>> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Laszlo Ersek 
>> ---
>> MdePkg/Library/UefiLib/UefiLib.inf |   1 +
>> MdePkg/Include/Library/UefiLib.h   |  86 
>> MdePkg/Library/UefiLib/UefiLib.c   | 226 
>> 3 files changed, 313 insertions(+)
>>
>> diff --git a/MdePkg/Library/UefiLib/UefiLib.inf 
>> b/MdePkg/Library/UefiLib/UefiLib.inf
>> index f69f0a43b576..a6c739ef3d6d 100644
>> --- a/MdePkg/Library/UefiLib/UefiLib.inf
>> +++ b/MdePkg/Library/UefiLib/UefiLib.inf
>> @@ -68,6 +68,7 @@ [Protocols]
>>   gEfiSimpleTextOutProtocolGuid   ## SOMETIMES_CONSUMES
>>   gEfiGraphicsOutputProtocolGuid  ## SOMETIMES_CONSUMES
>>   gEfiHiiFontProtocolGuid ## SOMETIMES_CONSUMES
>> +  gEfiSimpleFileSystemProtocolGuid## SOMETIMES_CONSUMES
>>   gEfiUgaDrawProtocolGuid | gEfiMdePkgTokenSpaceGuid.PcdUgaConsumeSupport
>>  ## SOMETIMES_CONSUMES # Consumes if 
>> gEfiGraphicsOutputProtocolGuid uninstalled
>>   gEfiComponentNameProtocolGuid  | NOT 
>> gEfiMdePkgTokenSpaceGuid.PcdComponentNameDisable   ## SOMETIMES_PRODUCES # 
>> User chooses to produce it
>>   gEfiComponentName2ProtocolGuid | NOT 
>> gEfiMdePkgTokenSpaceGuid.PcdComponentName2Disable  ## SOMETIMES_PRODUCES # 
>> User chooses to produce it
>> diff --git a/MdePkg/Include/Library/UefiLib.h 
>> b/MdePkg/Include/Library/UefiLib.h
>> index 7c6fde620c74..2468bf2aee80 100644
>> --- a/MdePkg/Include/Library/UefiLib.h
>> +++ b/MdePkg/Include/Library/UefiLib.h
>> @@ -33,6 +33,8 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
>> EXPRESS OR IMPLIED.
>> #include 
>> #include 
>> #include 
>> +#include 
>> +#include 
>>
>> #include 
>>
>> @@ -1520,4 +1522,88 @@ EfiLocateProtocolBuffer (
>>   OUT UINTN *NoProtocols,
>>   OUT VOID  ***Buffer
>>   );
>> +
>> +/**
>> +  Open or create a file or directory, possibly creating the chain of
>> +  directories leading up to the directory.
>> +
>> +  EfiOpenFileByDevicePath() first locates EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on
>> +  FilePath, and opens the root directory of that filesystem with
>> +  EFI_SIMPLE_FILE_SYSTEM_PROTOCOL.OpenVolume().
>> +
>> +  On the remaining device path, the longest initial sequence of
>> +  FILEPATH_DEVICE_PATH nodes is node-wise traversed with
>> +  EFI_FILE_PROTOCOL.Open(). For the pathname fragment specified by each
>> +  traversed FILEPATH_DEVICE_PATH node, EfiOpenFileByDevicePath() first masks
>> +  EFI_FILE_MODE_CREATE out of OpenMode, and passes 0 for Attributes. If
>> +  EFI_FILE_PROTOCOL.Open() fails, and OpenMode includes 
>> EFI_FILE_MODE_CREATE,
>> +  then the operation is retried with the caller's OpenMode and Attributes
>> +  unmodified.
>> +
>> +  

Re: [edk2] [PATCH 0/6] UefiLib: centralize OpenFileByDevicePath() and fix its bugs

[Laszlo Ersek]

On 07/19/18 02:07, Ard Biesheuvel wrote:
> On 19 July 2018 at 06:15, Carsey, Jaben  wrote:
>> Reviewed-by: Jaben Carsey 
>>
>> One question (do hold up push). Is there a reason to use the former over the 
>> latter? I use latter and I see you use the former.
>>
>> ASSERT(EFI_ERROR (Status));
>> ASSERT_EFI_ERROR (Status);
>>
> 
> The former asserts that an error has occurred, the latter does the opposite.

Right -- the latter is actually a misnomer; that macro should have
always been called "ASSERT_NO_EFI_ERROR" :)

> It seems Laszlo is using this to ensure we don't end up in the error
> handling path if no error occurred.

Sort of -- the way I put it for myself was, ensure that we don't exit on
the error path without returning an error status to the caller. So it's
less about control flow, and more about setting (or recognizing) error
statuses at all locations that jump to the error path.

But I guess that's just both sides of the same coin. :)

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

Re: [edk2] [PATCH 2/6] MdeModulePkg/RamDiskDxe: replace OpenFileByDevicePath() with UefiLib API

[Zeng, Star]

Hi Laszlo,

Have you evaluated whether " #include  " can be 
removed from *.h file or not since gEfiSimpleFileSystemProtocolGuid is removed 
from *.inf?

Thanks,
Star
-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com] 
Sent: Thursday, July 19, 2018 4:51 AM
To: edk2-devel-01 
Cc: Dong, Eric ; Wu, Jiaxin ; Ni, 
Ruiyu ; Fu, Siyuan ; Zeng, Star 

Subject: [PATCH 2/6] MdeModulePkg/RamDiskDxe: replace OpenFileByDevicePath() 
with UefiLib API

Replace the OpenFileByDevicePath() function with EfiOpenFileByDevicePath() from 
UefiLib, correcting the following issues:

- imprecise comments on OpenFileByDevicePath(),
- code duplication between this module and other modules,
- local variable name "EfiSimpleFileSystemProtocol" starting with "Efi"
  prefix,
- bogus "FileHandle = NULL" assignments,
- passing a potentially unaligned "FILEPATH_DEVICE_PATH.PathName" field to
  a protocol member function (forbidden by the UEFI spec),
- leaking "Handle1" when the device path type/subtype check fails in the
  loop,
- stale SHELL_FILE_HANDLE reference in a comment.

Cc: Eric Dong 
Cc: Jiaxin Wu 
Cc: Ruiyu Ni 
Cc: Siyuan Fu 
Cc: Star Zeng 
Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1008
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek 
---
 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf|   1 -
 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h |  39 --
 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c | 140 

 MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.c |   2 +-
 4 files changed, 1 insertion(+), 181 deletions(-)

diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf 
b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
index cdd2da690411..da00e4a420e7 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
@@ -75,7 +75,6 @@ [Protocols]
   gEfiDevicePathProtocolGuid ## PRODUCES
   gEfiBlockIoProtocolGuid## PRODUCES
   gEfiBlockIo2ProtocolGuid   ## PRODUCES
-  gEfiSimpleFileSystemProtocolGuid   ## SOMETIMES_CONSUMES
   gEfiAcpiTableProtocolGuid  ## SOMETIMES_CONSUMES
   gEfiAcpiSdtProtocolGuid## SOMETIMES_CONSUMES
 
diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h 
b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
index 077bb77b25bf..08a8ca94c9db 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
+++ b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskImpl.h
@@ -605,45 +605,6 @@ FileInfo (
   );
 
 
-/**
-  This function will open a file or directory referenced by DevicePath.
-
-  This function opens a file with the open mode according to the file path. The
-  Attributes is valid only for EFI_FILE_MODE_CREATE.
-
-  @param[in, out] FilePath   On input, the device path to the file.
- On output, the remaining device path.
-  @param[out] FileHandle Pointer to the file handle.
-  @param[in]  OpenMode   The mode to open the file with.
-  @param[in]  Attributes The file's file attributes.
-
-  @retval EFI_SUCCESS The information was set.
-  @retval EFI_INVALID_PARAMETER   One of the parameters has an invalid value.
-  @retval EFI_UNSUPPORTED Could not open the file path.
-  @retval EFI_NOT_FOUND   The specified file could not be found on the
-  device or the file system could not be found
-  on the device.
-  @retval EFI_NO_MEDIAThe device has no medium.
-  @retval EFI_MEDIA_CHANGED   The device has a different medium in it or
-  the medium is no longer supported.
-  @retval EFI_DEVICE_ERRORThe device reported an error.
-  @retval EFI_VOLUME_CORRUPTEDThe file system structures are corrupted.
-  @retval EFI_WRITE_PROTECTED The file or medium is write protected.
-  @retval EFI_ACCESS_DENIED   The file was opened read only.
-  @retval EFI_OUT_OF_RESOURCESNot enough resources were available to open
-  the file.
-  @retval EFI_VOLUME_FULL The volume is full.
-**/
-EFI_STATUS
-EFIAPI
-OpenFileByDevicePath(
-  IN OUT EFI_DEVICE_PATH_PROTOCOL   **FilePath,
-  OUT EFI_FILE_HANDLE   *FileHandle,
-  IN UINT64 OpenMode,
-  IN UINT64 Attributes
-  );
-
-
 /**
   Publish the RAM disk NVDIMM Firmware Interface Table (NFIT) to the ACPI
   table.
diff --git a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c 
b/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c
index 2cfd4bbf6ce8..95d676fc3939 100644
--- a/MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskFileExplorer.c
+++ 

[edk2] [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Return 0 when LSV check is not required

[Star Zeng]

Current code return 1 when LSV check is not required,
but 1 LSV will make 0 Version capsule image update failed.

0 LSV is valid, this patch updates the code to return 0 when
LSV check is not required
We can see even the DEFAULT_LOWESTSUPPORTEDVERSION is 0.

Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 FmpDevicePkg/FmpDxe/FmpDxe.c | 9 ++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index c0c1383723fb..d6b599ee1a56 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -210,12 +210,15 @@ GetLowestSupportedVersion (
   // Get the LowestSupportedVersion.
   //
 
-  DeviceLibLowestSupportedVersion = DEFAULT_LOWESTSUPPORTEDVERSION;
-  ReturnLsv = PcdGet32 (PcdFmpDeviceBuildTimeLowestSupportedVersion);
   if (!IsLowestSupportedVersionCheckRequired ()) {
-return 1;
+//
+// Any Versioon can pass the 0 LowestSupportedVersion check.
+//
+return 0;
   }
 
+  ReturnLsv = PcdGet32 (PcdFmpDeviceBuildTimeLowestSupportedVersion);
+
   //
   // Check the FmpDeviceLib
   //
-- 
2.7.0.windows.1

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

[edk2] [staging/FmpDevicePkg-master][PATCH] FmpDevicePkg FmpDxe: Update function comment for FmpDxeEntryPoint

[Star Zeng]

FmpDxeEntryPoint is used by both FmpDxe and FmpDxeLib.

Cc: Michael D Kinney 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Star Zeng 
---
 FmpDevicePkg/FmpDxe/FmpDxe.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c b/FmpDevicePkg/FmpDxe/FmpDxe.c
index b709bc282a6c..c0c1383723fb 100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -1360,7 +1360,7 @@ cleanup:
 }
 
 /**
-  Main entry for this library.
+  Main entry for this driver/library.
 
   @param[in] ImageHandle  Image handle this driver.
   @param[in] SystemTable  Pointer to SystemTable.
-- 
2.7.0.windows.1

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

[edk2] [Patch] SecurityPkg: TcgSmm: Handle invalid parameter in MOR SMI handler

[Zhang, Chao B]

Add more logic to filter invalid function parameter in MOR Control SMI handler

Cc: Long Qin 
Cc: Yao Jiewen 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Chao Zhang 
Signed-off-by: Zhang, Chao B 
---
 SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 4 
 SecurityPkg/Tcg/TcgSmm/TcgSmm.c   | 4 
 2 files changed, 8 insertions(+)

diff --git a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c 
b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
index 21b1014a3b..4a1a293bfc 100644
--- a/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
+++ b/SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c
@@ -151,10 +151,14 @@ MemoryClearCallback (
 
 if (MOR_CLEAR_MEMORY_VALUE (MorControl) == 0x0) {
   return EFI_SUCCESS;
 }
 MorControl &= ~MOR_CLEAR_MEMORY_BIT_MASK;
+  } else {
+mTcgNvs->MemoryClear.ReturnCode = MOR_REQUEST_GENERAL_FAILURE;
+DEBUG ((EFI_D_ERROR, "[TPM] MOR Parameter error! Parameter = %x\n", 
mTcgNvs->MemoryClear.Parameter));
+return EFI_SUCCESS;
   }
 
   DataSize = sizeof (UINT8);
   Status = mSmmVariable->SmmSetVariable (
MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME,
diff --git a/SecurityPkg/Tcg/TcgSmm/TcgSmm.c b/SecurityPkg/Tcg/TcgSmm/TcgSmm.c
index 0b8a002a4d..d3ddae6886 100644
--- a/SecurityPkg/Tcg/TcgSmm/TcgSmm.c
+++ b/SecurityPkg/Tcg/TcgSmm/TcgSmm.c
@@ -269,10 +269,14 @@ MemoryClearCallback (
 
 if (MOR_CLEAR_MEMORY_VALUE (MorControl) == 0x0) {
   return EFI_SUCCESS;
 }
 MorControl &= ~MOR_CLEAR_MEMORY_BIT_MASK;
+  } else {
+mTcgNvs->MemoryClear.ReturnCode = MOR_REQUEST_GENERAL_FAILURE;
+DEBUG ((EFI_D_ERROR, "[TPM] MOR Parameter error! Parameter = %x\n", 
mTcgNvs->MemoryClear.Parameter));
+return EFI_SUCCESS;
   }
 
   DataSize = sizeof (UINT8);
   Status = mSmmVariable->SmmSetVariable (
MEMORY_OVERWRITE_REQUEST_VARIABLE_NAME,
-- 
2.16.2.windows.1

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

Re: [edk2] [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode

[Wang, Jian J]

Hi Laszlo,

Do you have more comments? Or can you give a r-b?

Regards,
Jian

> -Original Message-
> From: Wang, Jian J
> Sent: Wednesday, July 18, 2018 10:36 AM
> To: Laszlo Ersek ; edk2-devel@lists.01.org
> Cc: Dong, Eric ; Yao, Jiewen ;
> Zeng, Star 
> Subject: RE: [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode
> 
> Hi Laszlo,
> 
> 
> Regards,
> Jian
> 
> 
> > -Original Message-
> > From: Laszlo Ersek [mailto:ler...@redhat.com]
> > Sent: Tuesday, July 17, 2018 10:37 PM
> > To: Wang, Jian J ; edk2-devel@lists.01.org
> > Cc: Dong, Eric ; Yao, Jiewen ;
> > Zeng, Star 
> > Subject: Re: [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode
> >
> > On 07/13/18 07:53, Jian J Wang wrote:
> > > Current IsInSmm() method makes use of gEfiSmmBase2ProtocolGuid.InSmm()
> > to
> > > check if current processor is in SMM mode or not. But this is not correct
> > > because gEfiSmmBase2ProtocolGuid.InSmm() can only detect if the caller is
> > > running in SMRAM or from SMM driver. It cannot guarantee if the caller is
> > > running in SMM mode.
> >
> > Wow. This is the exact difference which I asked about, in my question
> > (9b), and I was assured that InSmm() actually determined whether we were
> > executing in Management Mode (meaning the processor operating mode).
> >
> > http://mid.mail-
> >
> archive.com/0c09afa07dd0434d9e2a0c6aeb0483103bb55...@shsmsx102.cc
> > r.corp.intel.com
> >
> > (Scroll down in that message to see my original question (9b).)
> >
> > So why doesn't Star's explanation, from the original thread, apply
> > ultimately?
> >
> 
> We did many tests for this and didn't found any issue. So I took a risk. (I 
> thought
> a very precise check of SMM mode is hard and time consuming.)
> 
> > > Because SMM mode will load its own page table, adding
> > > an extra check of saved DXE page table base address against current CR3
> > > register value can help to get the correct answer for sure (in SMM mode or
> > > not in SMM mode).
> >
> > So, apparently, the PI spec offers no standard way for a platform module
> > to determine whether it runs in Management Mode, despite protocol member
> > being called "InSmm". Do we need a PI spec ECR for introducing the
> > needed facility?
> >
> > Alternatively, the PI spec might already intend to specify that, but the
> > edk2 implementation doesn't do what the PI spec requires.
> >
> > Which one is the case?
> >
> 
> The implementation conforms to the spec. It's my misunderstanding. But it's
> true
> that there's no specific protocol API to determine if it's in SMM mode or not.
> 
> > >
> > > This is an issue caused by check-in at
> > >
> > >   d106cf71eabaacff63c14626a4a87346b93074dd
> >
> > I disagree; I think the issue was introduced in commit 2a1408d1d739
> > ("UefiCpuPkg/CpuDxe: allow accessing (DXE) page table in SMM mode",
> > 2018-06-19).
> >
> 
> You're right. Thanks for pointing this out.
> 
> >
> > How did you encounter / find this issue?
> >
> 
> I didn't find it. The issue came to me. In other words, I think it's random 
> and hard
> to reproduce it. Maybe a subtle change in boot sequence will hide it away.
> 
> > >
> > > Cc: Eric Dong 
> > > Cc: Laszlo Ersek 
> > > Cc: Jiewen Yao 
> > > Cc: Star Zeng 
> > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > Signed-off-by: Jian J Wang 
> > > ---
> > >  UefiCpuPkg/CpuDxe/CpuPageTable.c | 9 -
> > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > > index 850eed60e7..df021798c0 100644
> > > --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > > +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c
> > > @@ -136,7 +136,14 @@ IsInSmm (
> > >  mSmmBase2->InSmm (mSmmBase2, );
> > >}
> > >
> > > -  return InSmm;
> > > +  //
> > > +  // mSmmBase2->InSmm() can only detect if the caller is running in SMRAM
> > > +  // or from SMM driver. It cannot tell if the caller is running in SMM 
> > > mode.
> > > +  // Check page table base address to guarantee that because SMM mode
> > willl
> > > +  // load its own page table.
> > > +  //
> > > +  return (InSmm &&
> > > +  mPagingContext.ContextData.X64.PageTableBase !=
> > (UINT64)AsmReadCr3());
> > >  }
> > >
> > >  /**
> > >
> >
> > Shouldn't we consider Ia32.PageTableBase when that's appropriate? From
> > "UefiCpuPkg/CpuDxe/CpuPageTable.h":
> >
> > typedef struct {
> >   UINT32  PageTableBase;
> >   UINT32  Reserved;
> >   UINT32  Attributes;
> > } PAGE_TABLE_LIB_PAGING_CONTEXT_IA32;
> >
> > typedef struct {
> >   UINT64  PageTableBase;
> >   UINT32  Attributes;
> > } PAGE_TABLE_LIB_PAGING_CONTEXT_X64;
> >
> > typedef union {
> >   PAGE_TABLE_LIB_PAGING_CONTEXT_IA32  Ia32;
> >   PAGE_TABLE_LIB_PAGING_CONTEXT_X64   X64;
> > } PAGE_TABLE_LIB_PAGING_CONTEXT_DATA;
> >
> > The Ia32/X64 structure types are not packed, and even if they were, I
> > wouldn't think we should rely on "Reserved" being zero.
> >
> 
> mPagingContext is zero-ed at 

[edk2] Help on AutoGen Files

[Udit Kumar]

Hi Experts,
How I can change the order of initialization in Constructor list of autogen 
file.
In my build system, if I look at
MdeModulePkg/Universal/PCD/Pei/Pcd/DEBUG/AutoGen.c
Below is function of Library Constructor List

VOID
EFIAPI
ProcessLibraryConstructorList (
  IN   EFI_PEI_FILE_HANDLE   FileHandle,
  IN CONST EFI_PEI_SERVICES  **PeiServices
  )
{
  EFI_STATUS  Status;

  Status = BaseDebugLibSerialPortConstructor ();
  ASSERT_EFI_ERROR (Status);

  Status = PeiServicesTablePointerLibConstructor (FileHandle, PeiServices);
  ASSERT_EFI_ERROR (Status);

  Status = TimerConstructor ();
  ASSERT_EFI_ERROR (Status);

  Status = FpgaInterfaceInit ();
  ASSERT_EFI_ERROR (Status);

}


My problem is SerialPortConstructor needs frequency, which can be retrieved 
after  FpgaInterfaceInit()
Therefore, my preferred way for this constructor list will be
FpgaInterfaceInit() followed by  BaseDebugLibSerialPortConstructor()

how I can achieve this.


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

[edk2] [patch] ShellPkg/Dp: Cumulate the perf data of "DB:Stop"

[Dandan Bi]

Currently DP tool will cumulate the performance data for
Driver Binding Support/Start, will count the number they
appears, and record the maximum/minimum time value...
Now add Driver Binding Stop performance data to the
cumulative data to keep consistency.

Cc: Liming Gao 
Cc: Jaben Carsey 
Cc: Ruiyu Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c 
b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
index 213f6057c9..2c094b63c9 100644
--- a/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
+++ b/ShellPkg/DynamicCommand/DpDynamicCommand/Dp.c
@@ -71,11 +71,12 @@ UINTN   mMeasurementNum= 0;
 /// Items for which to gather cumulative statistics.
 PERF_CUM_DATA CumData[] = {
   PERF_INIT_CUM_DATA (LOAD_IMAGE_TOK),
   PERF_INIT_CUM_DATA (START_IMAGE_TOK),
   PERF_INIT_CUM_DATA (DRIVERBINDING_START_TOK),
-  PERF_INIT_CUM_DATA (DRIVERBINDING_SUPPORT_TOK)
+  PERF_INIT_CUM_DATA (DRIVERBINDING_SUPPORT_TOK),
+  PERF_INIT_CUM_DATA (DRIVERBINDING_STOP_TOK)
 };
 
 /// Number of items for which we are gathering cumulative statistics.
 UINT32 const  NumCum = sizeof(CumData) / sizeof(PERF_CUM_DATA);
 
-- 
2.14.3.windows.1

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

[edk2] [patch 2/2] SecurityPkg/Tcg: Add use case for new Perf macro

[Dandan Bi]

Add an example case for the usage of
PERF_CALLBACK_BEGIN/PERF_CALLBACK_END

Cc: Liming Gao 
Cc: Chao Zhang 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c | 4 
 SecurityPkg/Tcg/TcgPei/TcgPei.c   | 5 +
 SecurityPkg/Tcg/TcgPei/TcgPei.inf | 1 +
 3 files changed, 10 insertions(+)

diff --git a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c 
b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
index 74cdd1fa88..09ef0c70a5 100644
--- a/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
+++ b/SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.c
@@ -159,10 +159,12 @@ EndofPeiSignalNotifyCallBack (
 {
   MEASURED_HOB_DATA *MeasuredHobData;
 
   MeasuredHobData = NULL;
 
+  PERF_CALLBACK_BEGIN ();
+
   //
   // Create a Guid hob to save all measured Fv
   //
   MeasuredHobData = BuildGuidHob(
   ,
@@ -184,10 +186,12 @@ EndofPeiSignalNotifyCallBack (
 // Save measured child Fv info
 //
 CopyMem (>MeasuredFvBuf[mMeasuredBaseFvIndex] , 
mMeasuredChildFvInfo, sizeof(EFI_PLATFORM_FIRMWARE_BLOB) * 
(mMeasuredChildFvIndex));
   }
 
+  PERF_CALLBACK_END ();
+
   return EFI_SUCCESS;
 }
 
 /**
   Make sure that the current PCR allocations, the TPM supported PCRs,
diff --git a/SecurityPkg/Tcg/TcgPei/TcgPei.c b/SecurityPkg/Tcg/TcgPei/TcgPei.c
index 1ed11a1b29..d07047580c 100644
--- a/SecurityPkg/Tcg/TcgPei/TcgPei.c
+++ b/SecurityPkg/Tcg/TcgPei/TcgPei.c
@@ -39,10 +39,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 BOOLEAN mImageInMemory  = FALSE;
 
 EFI_PEI_PPI_DESCRIPTOR  mTpmInitializedPpiList = {
   EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
@@ -168,10 +169,12 @@ EndofPeiSignalNotifyCallBack (
 {
   MEASURED_HOB_DATA *MeasuredHobData;
 
   MeasuredHobData = NULL;
 
+  PERF_CALLBACK_BEGIN ();
+
   //
   // Create a Guid hob to save all measured Fv
   //
   MeasuredHobData = BuildGuidHob(
   ,
@@ -193,10 +196,12 @@ EndofPeiSignalNotifyCallBack (
 // Save measured child Fv info
 //
 CopyMem (>MeasuredFvBuf[mMeasuredBaseFvIndex] , 
mMeasuredChildFvInfo, sizeof(EFI_PLATFORM_FIRMWARE_BLOB) * 
(mMeasuredChildFvIndex));
   }
 
+  PERF_CALLBACK_END ();
+
   return EFI_SUCCESS;
 }
 
 /**
 Single function calculates SHA1 digest value for all raw data. It
diff --git a/SecurityPkg/Tcg/TcgPei/TcgPei.inf 
b/SecurityPkg/Tcg/TcgPei/TcgPei.inf
index 0252511391..4c8a055c6c 100644
--- a/SecurityPkg/Tcg/TcgPei/TcgPei.inf
+++ b/SecurityPkg/Tcg/TcgPei/TcgPei.inf
@@ -54,10 +54,11 @@
   BaseLib
   PcdLib
   MemoryAllocationLib
   ReportStatusCodeLib
   Tpm12CommandLib
+  PerformanceLib
 
 [Guids]
   gTcgEventEntryHobGuid   ## 
PRODUCES   ## HOB
   gTpmErrorHobGuid## 
SOMETIMES_PRODUCES ## HOB
   gMeasuredFvHobGuid  ## 
PRODUCES   ## HOB
-- 
2.14.3.windows.1

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

[edk2] [patch 1/2] MdeModulePkg/DxeLoadFunc: Add use case for new Perf macro

[Dandan Bi]

Add an example case for the usage of
PERF_EVENT_SIGNAL_BEGIN/PERF_EVENT_SIGNAL_END

Cc: Liming Gao 
Cc: Star Zeng 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi 
---
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.h   | 3 ++-
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 1 +
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4 
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h 
b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
index 6f8e13d213..9ea88a399b 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.h
@@ -1,10 +1,10 @@
 /** @file
   Master header file for DxeIpl PEIM. All source files in this module should
   include this file for common definitions.
 
-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
 http://opensource.org/licenses/bsd-license.php
 
@@ -46,10 +46,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 #include 
 #include 
 #include 
 #include 
 #include 
+#include 
 
 #define STACK_SIZE  0x2
 #define BSP_STORE_SIZE  0x4000
 
 
diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf 
b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
index 7deeb8f270..302934283a 100644
--- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
+++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
@@ -77,10 +77,11 @@
   BaseLib
   PeimEntryPoint
   DebugLib
   DebugAgentLib
   PeiServicesTablePointerLib
+  PerformanceLib
 
 [LibraryClasses.ARM, LibraryClasses.AARCH64]
   ArmMmuLib
 
 [Ppis]
diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c 
b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
index d5aa0474b0..8a939b6c24 100644
--- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
+++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c
@@ -318,11 +318,13 @@ HandOffToDxeCore (
 PageTables = CreateIdentityMappingPageTables (BaseOfStack, STACK_SIZE);
 
 //
 // End of PEI phase signal
 //
+PERF_EVENT_SIGNAL_BEGIN (gEndOfPeiSignalPpi.Guid);
 Status = PeiServicesInstallPpi ();
+PERF_EVENT_SIGNAL_END (gEndOfPeiSignalPpi.Guid);
 ASSERT_EFI_ERROR (Status);
 
 AsmWriteCr3 (PageTables);
 
 //
@@ -435,11 +437,13 @@ HandOffToDxeCore (
 }
 
 //
 // End of PEI phase signal
 //
+PERF_EVENT_SIGNAL_BEGIN (gEndOfPeiSignalPpi.Guid);
 Status = PeiServicesInstallPpi ();
+PERF_EVENT_SIGNAL_END (gEndOfPeiSignalPpi.Guid);
 ASSERT_EFI_ERROR (Status);
 
 if (BuildPageTablesIa32Pae) {
   AsmWriteCr3 (PageTables);
   //
-- 
2.14.3.windows.1

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