[edk2] [PATCH] UefiCpuPkg/Cpuid: Dump leaf 1FH information correctly

2019-03-29 Thread Ray Ni 
Leaf 1FH is very similar to leaf 0BH. Both return the CPU topology
information.
Leaf 0BH returns 3-level (Package/Core/Thread) CPU topology info.
Leaf 1FH returns 6-level (Package/Die/Tile/Module/Core/Thread) CPU
topology info.
The logic to enumerate the topology info is the same.

But today's logic to handle 1FH is completely wrong.
The patch combines them together to fix the 1FH issue.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni 
Cc: Eric Dong 
---
 UefiCpuPkg/Application/Cpuid/Cpuid.c | 83 ++--
 1 file changed, 28 insertions(+), 55 deletions(-)

diff --git a/UefiCpuPkg/Application/Cpuid/Cpuid.c 
b/UefiCpuPkg/Application/Cpuid/Cpuid.c
index 67cacf2714..3d242a0cbf 100644
--- a/UefiCpuPkg/Application/Cpuid/Cpuid.c
+++ b/UefiCpuPkg/Application/Cpuid/Cpuid.c
@@ -1,7 +1,7 @@
 /** @file
   UEFI Application to display CPUID leaf information.
 
-  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2016 - 2019, 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
@@ -717,36 +717,42 @@ CpuidArchitecturalPerformanceMonitoring (
 **/
 VOID
 CpuidExtendedTopology (
-  VOID
+  UINT32   LeafFunction
   )
 {
-  CPUID_EXTENDED_TOPOLOGY_EAX  Eax;
-  CPUID_EXTENDED_TOPOLOGY_EBX  Ebx;
-  CPUID_EXTENDED_TOPOLOGY_ECX  Ecx;
-  UINT32   Edx;
-  UINT32   LevelNumber;
+  CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_EAX  Eax;
+  CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_EBX  Ebx;
+  CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_ECX  Ecx;
+  UINT32  Edx;
+  UINT32  LevelNumber;
 
-  if (CPUID_EXTENDED_TOPOLOGY > gMaximumBasicFunction) {
+  if (LeafFunction > gMaximumBasicFunction) {
+return;
+  }
+  if ((LeafFunction != CPUID_EXTENDED_TOPOLOGY) && (LeafFunction != 
CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION)) {
 return;
   }
 
   LevelNumber = 0;
-  do {
+  for (LevelNumber = 0; ; LevelNumber++) {
 AsmCpuidEx (
-  CPUID_EXTENDED_TOPOLOGY, LevelNumber,
+  LeafFunction, LevelNumber,
   &Eax.Uint32, &Ebx.Uint32, &Ecx.Uint32, &Edx
   );
-if (Eax.Bits.ApicIdShift != 0) {
-  Print (L"CPUID_EXTENDED_TOPOLOGY (Leaf %08x, Sub-Leaf %08x)\n", 
CPUID_EXTENDED_TOPOLOGY, LevelNumber);
-  Print (L"  EAX:%08x  EBX:%08x  ECX:%08x  EDX:%08x\n", Eax.Uint32, 
Ebx.Uint32, Ecx.Uint32, Edx);
-  PRINT_BIT_FIELD (Eax, ApicIdShift);
-  PRINT_BIT_FIELD (Ebx, LogicalProcessors);
-  PRINT_BIT_FIELD (Ecx, LevelNumber);
-  PRINT_BIT_FIELD (Ecx, LevelType);
-  PRINT_VALUE (Edx, x2APIC_ID);
+if (Ecx.Bits.LevelType == 
CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_LEVEL_TYPE_INVALID) {
+  break;
 }
-LevelNumber++;
-  } while (Eax.Bits.ApicIdShift != 0);
+Print (
+  L"%a (Leaf %08x, Sub-Leaf %08x)\n",
+  LeafFunction == CPUID_EXTENDED_TOPOLOGY ? "CPUID_EXTENDED_TOPOLOGY" : 
"CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION-2",
+  LeafFunction, LevelNumber);
+Print (L"  EAX:%08x  EBX:%08x  ECX:%08x  EDX:%08x\n", Eax.Uint32, 
Ebx.Uint32, Ecx.Uint32, Edx);
+PRINT_BIT_FIELD (Eax, BitsNum);
+PRINT_BIT_FIELD (Ebx, ProcessorsNum);
+PRINT_BIT_FIELD (Ecx, LevelNum);
+PRINT_BIT_FIELD (Ecx, LevelType);
+PRINT_VALUE (Edx, x2APIC_ID);
+  }
 }
 
 /**
@@ -1385,39 +1391,6 @@ CpuidDeterministicAddressTranslationParameters (
   PRINT_BIT_FIELD (Edx, MaximumNum);
 }
 
-/**
-  Display CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION main leaf and sub-leafs.
-
-**/
-VOID
-CpuidV2ExtendedTopologyEnumeration (
-  VOID
-  )
-{
-  CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_EAX  Eax;
-  CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_EBX  Ebx;
-  CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_ECX  Ecx;
-  UINT32  Edx;
-
-  if (CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION > gMaximumBasicFunction) {
-return;
-  }
-
-  AsmCpuidEx (
-CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION,
-CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_MAIN_LEAF,
-&Eax.Uint32, &Ebx.Uint32, &Ecx.Uint32, &Edx
-);
-  Print (L"CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION (Leaf %08x, Sub-Leaf 
%08x)\n", CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION, 
CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_MAIN_LEAF);
-  Print (L"  EAX:%08x  EBX:%08x  ECX:%08x  EDX:%08x\n", Eax.Uint32, 
Ebx.Uint32, Ecx.Uint32, Edx);
-
-  PRINT_BIT_FIELD (Eax, BitsNum);
-  PRINT_BIT_FIELD (Ebx, ProcessorsNum);
-  PRINT_BIT_FIELD (Ecx, LevelNum);
-  PRINT_BIT_FIELD (Ecx, LevelType);
-  PRINT_VALUE (Edx, x2APICID);
-}
-
 /**
   Display CPUID_EXTENDED_FUNCTION leaf.
 
@@ -1619,7 +1592,7 @@ UefiMain (
   CpuidStruc

[edk2] [PATCH] UefiCpuPkg/LocalApicLib: Add GetProcessorLocation2ByApicId() API

2019-03-29 Thread Ray Ni 
GetProcessorLocation2ByApicId() extracts the
package/die/tile/module/core/thread ID from the initial APIC ID.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni 
Cc: Eric Dong 
---
 UefiCpuPkg/Include/Library/LocalApicLib.h |  29 +++-
 .../Library/BaseXApicLib/BaseXApicLib.c   | 125 +-
 .../BaseXApicX2ApicLib/BaseXApicX2ApicLib.c   | 125 +-
 3 files changed, 276 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Include/Library/LocalApicLib.h 
b/UefiCpuPkg/Include/Library/LocalApicLib.h
index ad1c26df60..ffe60c56fc 100644
--- a/UefiCpuPkg/Include/Library/LocalApicLib.h
+++ b/UefiCpuPkg/Include/Library/LocalApicLib.h
@@ -4,7 +4,7 @@
   Local APIC library assumes local APIC is enabled. It does not
   handles cases where local APIC is disabled.
 
-  Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2019, 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
@@ -432,5 +432,32 @@ GetProcessorLocationByApicId (
   OUT UINT32  *Thread  OPTIONAL
   );
 
+/**
+  Get Package ID/Module ID/Tile ID/Die ID/Core ID/Thread ID of a processor.
+
+  The algorithm assumes the target system has symmetry across physical
+  package boundaries with respect to the number of threads per core, number of
+  cores per module, number of modules per tile, number of tiles per die, number
+  of dies per package.
+
+  @param[in]   InitialApicId Initial APIC ID of the target logical processor.
+  @param[out]  Package   Returns the processor package ID.
+  @param[out]  Die   Returns the processor die ID.
+  @param[out]  Tile  Returns the processor tile ID.
+  @param[out]  ModuleReturns the processor module ID.
+  @param[out]  Core  Returns the processor core ID.
+  @param[out]  ThreadReturns the processor thread ID.
+**/
+VOID
+EFIAPI
+GetProcessorLocation2ByApicId (
+  IN  UINT32  InitialApicId,
+  OUT UINT32  *Package  OPTIONAL,
+  OUT UINT32  *Die  OPTIONAL,
+  OUT UINT32  *Tile OPTIONAL,
+  OUT UINT32  *Module   OPTIONAL,
+  OUT UINT32  *Core OPTIONAL,
+  OUT UINT32  *Thread   OPTIONAL
+  );
 #endif
 
diff --git a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c 
b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
index 7d66d89dfd..1f994c81cf 100644
--- a/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
+++ b/UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.c
@@ -3,7 +3,7 @@
 
   This local APIC library instance supports xAPIC mode only.
 
-  Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.
+  Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.
   Copyright (c) 2017, AMD Inc. All rights reserved.
 
   This program and the accompanying materials
@@ -1156,3 +1156,126 @@ GetProcessorLocationByApicId (
 *Package = (InitialApicId >> (ThreadBits + CoreBits));
   }
 }
+
+/**
+  Get Package ID/Die ID/Tile ID/Module ID/Core ID/Thread ID of a processor.
+
+  The algorithm assumes the target system has symmetry across physical
+  package boundaries with respect to the number of threads per core, number of
+  cores per module, number of modules per tile, number of tiles per die, number
+  of dies per package.
+
+  @param[in]   InitialApicId Initial APIC ID of the target logical processor.
+  @param[out]  Package   Returns the processor package ID.
+  @param[out]  Die   Returns the processor die ID.
+  @param[out]  Tile  Returns the processor tile ID.
+  @param[out]  ModuleReturns the processor module ID.
+  @param[out]  Core  Returns the processor core ID.
+  @param[out]  ThreadReturns the processor thread ID.
+**/
+VOID
+EFIAPI
+GetProcessorLocation2ByApicId (
+  IN  UINT32  InitialApicId,
+  OUT UINT32  *Package  OPTIONAL,
+  OUT UINT32  *Die  OPTIONAL,
+  OUT UINT32  *Tile OPTIONAL,
+  OUT UINT32  *Module   OPTIONAL,
+  OUT UINT32  *Core OPTIONAL,
+  OUT UINT32  *Thread   OPTIONAL
+  )
+{
+  CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_EAX V2ExtendedTopologyEax;
+  CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_ECX V2ExtendedTopologyEcx;
+  UINT32 MaxStandardCpuIdIndex;
+  UINT32 Index;
+  UINTN  LevelType;
+  UINT32 
Bits[CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_LEVEL_TYPE_DIE + 2];
+  UINT32 
*Location[CPUID_V2_EXTENDED_TOPOLOGY_ENUMERATION_LEVEL_TYPE_DIE + 2];
+
+  for (LevelType = 0; LevelType < ARRAY_SIZE (Bits); LevelType++) {
+Bits[LevelType] = 0;
+  }
+
+  //
+  // Get max index of CPUID
+  //
+  AsmCpuid (CPUID_SIGNATURE, &MaxStandardCpuIdIndex, NULL, NULL, NULL);
+  if (MaxStand

[edk2] [PATCH v2 0/2] Fix bugs in HiiDatabase driver

2019-03-07 Thread Ray Ni 
v2: put the CVE number in patch title.

Ray Ni (2):
  MdeModulePkg/HiiDatabase: Fix potential integer overflow
(CVE-2018-12181)
  MdeModulePkg/HiiImage: Fix stack overflow when corrupted BMP is parsed
(CVE-2018-12181)

 MdeModulePkg/Universal/HiiDatabaseDxe/Image.c | 130 ++
 1 file changed, 105 insertions(+), 25 deletions(-)

-- 
2.20.1.windows.1

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

[edk2] [PATCH v2 2/2] MdeModulePkg/HiiImage: Fix stack overflow when corrupted BMP is parsed (CVE-2018-12181)

2019-03-07 Thread Ray Ni 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1135

For 4bit BMP, there are only 2^4 = 16 colors in the palette.
But when a corrupted BMP contains more than 16 colors in the palette,
today's implementation wrongly copies all colors to the local
PaletteValue[16] array which causes stack overflow.

The similar issue also exists in the logic to handle 8bit BMP.

The patch fixes the issue by only copies the first 16 or 256 colors
in the palette depending on the BMP type.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni 
Cc: Liming Gao 
Cc: Jiewen Yao 
---
 MdeModulePkg/Universal/HiiDatabaseDxe/Image.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
index 80a4ec1114..8532f272eb 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
@@ -370,7 +370,7 @@ Output4bitPixel (
   PaletteNum = (UINT16)(Palette->PaletteSize / sizeof (EFI_HII_RGB_PIXEL));
 
   ZeroMem (PaletteValue, sizeof (PaletteValue));
-  CopyRgbToGopPixel (PaletteValue, Palette->PaletteValue, PaletteNum);
+  CopyRgbToGopPixel (PaletteValue, Palette->PaletteValue, MIN (PaletteNum, 
ARRAY_SIZE (PaletteValue)));
   FreePool (Palette);
 
   //
@@ -447,7 +447,7 @@ Output8bitPixel (
   CopyMem (Palette, PaletteInfo, PaletteSize);
   PaletteNum = (UINT16)(Palette->PaletteSize / sizeof (EFI_HII_RGB_PIXEL));
   ZeroMem (PaletteValue, sizeof (PaletteValue));
-  CopyRgbToGopPixel (PaletteValue, Palette->PaletteValue, PaletteNum);
+  CopyRgbToGopPixel (PaletteValue, Palette->PaletteValue, MIN (PaletteNum, 
ARRAY_SIZE (PaletteValue)));
   FreePool (Palette);
 
   //
-- 
2.20.1.windows.1

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

[edk2] [PATCH v2 1/2] MdeModulePkg/HiiDatabase: Fix potential integer overflow (CVE-2018-12181)

2019-03-07 Thread Ray Ni 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1135

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni 
Cc: Dandan Bi 
Cc: Hao A Wu 
---
 MdeModulePkg/Universal/HiiDatabaseDxe/Image.c | 126 ++
 1 file changed, 103 insertions(+), 23 deletions(-)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
index 71ebc559c0..80a4ec1114 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
@@ -16,6 +16,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #include "HiiDatabase.h"
 
+#define MAX_UINT240xFF
 
 /**
   Get the imageid of last image block: EFI_HII_IIBT_END_BLOCK when input
@@ -651,8 +652,16 @@ HiiNewImage (
 
   EfiAcquireLock (&mHiiDatabaseLock);
 
-  NewBlockSize = sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof 
(EFI_HII_RGB_PIXEL) +
- BITMAP_LEN_24_BIT ((UINT32) Image->Width, Image->Height);
+  //
+  // Calcuate the size of new image.
+  // Make sure the size doesn't overflow UINT32.
+  // Note: 24Bit BMP occpuies 3 bytes per pixel.
+  //
+  NewBlockSize = (UINT32)Image->Width * Image->Height;
+  if (NewBlockSize > (MAX_UINT32 - (sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - 
sizeof (EFI_HII_RGB_PIXEL))) / 3) {
+return EFI_OUT_OF_RESOURCES;
+  }
+  NewBlockSize = NewBlockSize * 3 + (sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - 
sizeof (EFI_HII_RGB_PIXEL));
 
   //
   // Get the image package in the package list,
@@ -671,6 +680,18 @@ HiiNewImage (
 //
 // Update the package's image block by appending the new block to the end.
 //
+
+//
+// Make sure the final package length doesn't overflow.
+// Length of the package header is represented using 24 bits. So MAX 
length is MAX_UINT24.
+//
+if (NewBlockSize > MAX_UINT24 - ImagePackage->ImagePkgHdr.Header.Length) {
+  return EFI_OUT_OF_RESOURCES;
+}
+//
+// Because ImagePackage->ImageBlockSize < 
ImagePackage->ImagePkgHdr.Header.Length,
+// So (ImagePackage->ImageBlockSize + NewBlockSize) <= MAX_UINT24
+//
 ImageBlocks = AllocatePool (ImagePackage->ImageBlockSize + NewBlockSize);
 if (ImageBlocks == NULL) {
   EfiReleaseLock (&mHiiDatabaseLock);
@@ -701,6 +722,13 @@ HiiNewImage (
 PackageListNode->PackageListHdr.PackageLength += NewBlockSize;
 
   } else {
+//
+// Make sure the final package length doesn't overflow.
+// Length of the package header is represented using 24 bits. So MAX 
length is MAX_UINT24.
+//
+if (NewBlockSize > MAX_UINT24 - (sizeof (EFI_HII_IMAGE_PACKAGE_HDR) + 
sizeof (EFI_HII_IIBT_END_BLOCK))) {
+  return EFI_OUT_OF_RESOURCES;
+}
 //
 // The specified package list does not contain image package.
 // Create one to add this image block.
@@ -902,8 +930,11 @@ IGetImage (
 // Use the common block code since the definition of these structures is 
the same.
 //
 CopyMem (&Iibt1bit, CurrentImageBlock, sizeof 
(EFI_HII_IIBT_IMAGE_1BIT_BLOCK));
-ImageLength = sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) *
-  ((UINT32) Iibt1bit.Bitmap.Width * Iibt1bit.Bitmap.Height);
+ImageLength = (UINTN) Iibt1bit.Bitmap.Width * Iibt1bit.Bitmap.Height;
+if (ImageLength > MAX_UINTN / sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)) {
+  return EFI_OUT_OF_RESOURCES;
+}
+ImageLength  *= sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL);
 Image->Bitmap = AllocateZeroPool (ImageLength);
 if (Image->Bitmap == NULL) {
   return EFI_OUT_OF_RESOURCES;
@@ -952,9 +983,13 @@ IGetImage (
 // fall through
 //
   case EFI_HII_IIBT_IMAGE_24BIT:
-Width = ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) 
CurrentImageBlock)->Bitmap.Width);
+Width  = ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) 
CurrentImageBlock)->Bitmap.Width);
 Height = ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) 
CurrentImageBlock)->Bitmap.Height);
-ImageLength = sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) * ((UINT32) Width * 
Height);
+ImageLength = (UINTN)Width * Height;
+if (ImageLength > MAX_UINTN / sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)) {
+  return EFI_OUT_OF_RESOURCES;
+}
+ImageLength  *= sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL);
 Image->Bitmap = AllocateZeroPool (ImageLength);
 if (Image->Bitmap == NULL) {
   return EFI_OUT_OF_RESOURCES;
@@ -1124,8 +1159,23 @@ HiiSetImage (
   //
   // Create the new image block according to input image.
   //
-  NewBlockSize = sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof 
(EFI_HII_RGB_PIXEL) +
- BITMAP_LEN_24_BIT ((UINT32) Image->Width, Image->Height);
+
+  //
+  // Make sure the final package length doesn't overflow.
+  // Length of the package heade

[edk2] [PATCH 0/2] Fix bugs in HiiDatabase driver

2019-03-07 Thread Ray Ni 
Ray Ni (2):
  MdeModulePkg/HiiDatabase: Fix potential integer overflow
  MdeModulePkg/HiiImage: Fix stack overflow when corrupted BMP is parsed

 MdeModulePkg/Universal/HiiDatabaseDxe/Image.c | 130 ++
 1 file changed, 105 insertions(+), 25 deletions(-)

-- 
2.20.1.windows.1

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

[edk2] [PATCH 1/2] MdeModulePkg/HiiDatabase: Fix potential integer overflow

2019-03-07 Thread Ray Ni 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1135
CVE number: CVE-2018-12181

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni 
Cc: Dandan Bi 
Cc: Hao A Wu 
---
 MdeModulePkg/Universal/HiiDatabaseDxe/Image.c | 126 ++
 1 file changed, 103 insertions(+), 23 deletions(-)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
index 71ebc559c0..80a4ec1114 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
@@ -16,6 +16,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER 
EXPRESS OR IMPLIED.
 
 #include "HiiDatabase.h"
 
+#define MAX_UINT240xFF
 
 /**
   Get the imageid of last image block: EFI_HII_IIBT_END_BLOCK when input
@@ -651,8 +652,16 @@ HiiNewImage (
 
   EfiAcquireLock (&mHiiDatabaseLock);
 
-  NewBlockSize = sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof 
(EFI_HII_RGB_PIXEL) +
- BITMAP_LEN_24_BIT ((UINT32) Image->Width, Image->Height);
+  //
+  // Calcuate the size of new image.
+  // Make sure the size doesn't overflow UINT32.
+  // Note: 24Bit BMP occpuies 3 bytes per pixel.
+  //
+  NewBlockSize = (UINT32)Image->Width * Image->Height;
+  if (NewBlockSize > (MAX_UINT32 - (sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - 
sizeof (EFI_HII_RGB_PIXEL))) / 3) {
+return EFI_OUT_OF_RESOURCES;
+  }
+  NewBlockSize = NewBlockSize * 3 + (sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - 
sizeof (EFI_HII_RGB_PIXEL));
 
   //
   // Get the image package in the package list,
@@ -671,6 +680,18 @@ HiiNewImage (
 //
 // Update the package's image block by appending the new block to the end.
 //
+
+//
+// Make sure the final package length doesn't overflow.
+// Length of the package header is represented using 24 bits. So MAX 
length is MAX_UINT24.
+//
+if (NewBlockSize > MAX_UINT24 - ImagePackage->ImagePkgHdr.Header.Length) {
+  return EFI_OUT_OF_RESOURCES;
+}
+//
+// Because ImagePackage->ImageBlockSize < 
ImagePackage->ImagePkgHdr.Header.Length,
+// So (ImagePackage->ImageBlockSize + NewBlockSize) <= MAX_UINT24
+//
 ImageBlocks = AllocatePool (ImagePackage->ImageBlockSize + NewBlockSize);
 if (ImageBlocks == NULL) {
   EfiReleaseLock (&mHiiDatabaseLock);
@@ -701,6 +722,13 @@ HiiNewImage (
 PackageListNode->PackageListHdr.PackageLength += NewBlockSize;
 
   } else {
+//
+// Make sure the final package length doesn't overflow.
+// Length of the package header is represented using 24 bits. So MAX 
length is MAX_UINT24.
+//
+if (NewBlockSize > MAX_UINT24 - (sizeof (EFI_HII_IMAGE_PACKAGE_HDR) + 
sizeof (EFI_HII_IIBT_END_BLOCK))) {
+  return EFI_OUT_OF_RESOURCES;
+}
 //
 // The specified package list does not contain image package.
 // Create one to add this image block.
@@ -902,8 +930,11 @@ IGetImage (
 // Use the common block code since the definition of these structures is 
the same.
 //
 CopyMem (&Iibt1bit, CurrentImageBlock, sizeof 
(EFI_HII_IIBT_IMAGE_1BIT_BLOCK));
-ImageLength = sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) *
-  ((UINT32) Iibt1bit.Bitmap.Width * Iibt1bit.Bitmap.Height);
+ImageLength = (UINTN) Iibt1bit.Bitmap.Width * Iibt1bit.Bitmap.Height;
+if (ImageLength > MAX_UINTN / sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)) {
+  return EFI_OUT_OF_RESOURCES;
+}
+ImageLength  *= sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL);
 Image->Bitmap = AllocateZeroPool (ImageLength);
 if (Image->Bitmap == NULL) {
   return EFI_OUT_OF_RESOURCES;
@@ -952,9 +983,13 @@ IGetImage (
 // fall through
 //
   case EFI_HII_IIBT_IMAGE_24BIT:
-Width = ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) 
CurrentImageBlock)->Bitmap.Width);
+Width  = ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) 
CurrentImageBlock)->Bitmap.Width);
 Height = ReadUnaligned16 ((VOID *) &((EFI_HII_IIBT_IMAGE_24BIT_BLOCK *) 
CurrentImageBlock)->Bitmap.Height);
-ImageLength = sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL) * ((UINT32) Width * 
Height);
+ImageLength = (UINTN)Width * Height;
+if (ImageLength > MAX_UINTN / sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL)) {
+  return EFI_OUT_OF_RESOURCES;
+}
+ImageLength  *= sizeof (EFI_GRAPHICS_OUTPUT_BLT_PIXEL);
 Image->Bitmap = AllocateZeroPool (ImageLength);
 if (Image->Bitmap == NULL) {
   return EFI_OUT_OF_RESOURCES;
@@ -1124,8 +1159,23 @@ HiiSetImage (
   //
   // Create the new image block according to input image.
   //
-  NewBlockSize = sizeof (EFI_HII_IIBT_IMAGE_24BIT_BLOCK) - sizeof 
(EFI_HII_RGB_PIXEL) +
- BITMAP_LEN_24_BIT ((UINT32) Image->Width, Image->Height);
+
+  //
+  // Make sure the final package length doesn't overflow.

[edk2] [PATCH 2/2] MdeModulePkg/HiiImage: Fix stack overflow when corrupted BMP is parsed

2019-03-07 Thread Ray Ni 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1135
CVE number: CVE-2018-12181

For 4bit BMP, there are only 2^4 = 16 colors in the palette.
But when a corrupted BMP contains more than 16 colors in the palette,
today's implementation wrongly copies all colors to the local
PaletteValue[16] array which causes stack overflow.

The similar issue also exists in the logic to handle 8bit BMP.

The patch fixes the issue by only copies the first 16 or 256 colors
in the palette depending on the BMP type.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni 
Cc: Liming Gao 
Cc: Jiewen Yao 
---
 MdeModulePkg/Universal/HiiDatabaseDxe/Image.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c 
b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
index 80a4ec1114..8532f272eb 100644
--- a/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
+++ b/MdeModulePkg/Universal/HiiDatabaseDxe/Image.c
@@ -370,7 +370,7 @@ Output4bitPixel (
   PaletteNum = (UINT16)(Palette->PaletteSize / sizeof (EFI_HII_RGB_PIXEL));
 
   ZeroMem (PaletteValue, sizeof (PaletteValue));
-  CopyRgbToGopPixel (PaletteValue, Palette->PaletteValue, PaletteNum);
+  CopyRgbToGopPixel (PaletteValue, Palette->PaletteValue, MIN (PaletteNum, 
ARRAY_SIZE (PaletteValue)));
   FreePool (Palette);
 
   //
@@ -447,7 +447,7 @@ Output8bitPixel (
   CopyMem (Palette, PaletteInfo, PaletteSize);
   PaletteNum = (UINT16)(Palette->PaletteSize / sizeof (EFI_HII_RGB_PIXEL));
   ZeroMem (PaletteValue, sizeof (PaletteValue));
-  CopyRgbToGopPixel (PaletteValue, Palette->PaletteValue, PaletteNum);
+  CopyRgbToGopPixel (PaletteValue, Palette->PaletteValue, MIN (PaletteNum, 
ARRAY_SIZE (PaletteValue)));
   FreePool (Palette);
 
   //
-- 
2.20.1.windows.1

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

[edk2] [PATCH] EmulatorPkg/Sec: Don't install TemporaryRamSupport PPI

2019-03-02 Thread Ray Ni 
TemporaryRamSupport PPI is called by PeiCore to migrate temporary
memory from permanent memory. But the implementation assumes
ebp/rbp contains the original esp/rsp value when migrating which
is not always true the compiler optimization is turned on.
A real boot failure is seen using GCC5.
In fact, below commit in year 2013 enhanced PeiCore to migrate
the memory without calling TemporaryRamSupport PPI.
SHA-1: 0f9ebb321638e9142ab3bdcc19000c49bb83b9ba
* Add support for PI1.2.1 TempRam Done PPI.

So this patch removes TemporaryRamSupport PPI implementation from
EmulatorPkg/Sec module to fix the boot failure when using GCC5.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni 
Cc: Jordan Justen 
Cc: Andrew Fish 
---
 EmulatorPkg/Sec/Ia32/SwitchRam.S   | 95 --
 EmulatorPkg/Sec/Ia32/SwitchRam.asm | 94 -
 EmulatorPkg/Sec/Ia32/TempRam.c | 65 
 EmulatorPkg/Sec/Sec.c  | 61 +--
 EmulatorPkg/Sec/Sec.h  | 14 +
 EmulatorPkg/Sec/Sec.inf| 15 +
 EmulatorPkg/Sec/X64/SwitchRam.S| 72 --
 EmulatorPkg/Sec/X64/SwitchRam.asm  | 76 
 8 files changed, 5 insertions(+), 487 deletions(-)
 delete mode 100644 EmulatorPkg/Sec/Ia32/SwitchRam.S
 delete mode 100644 EmulatorPkg/Sec/Ia32/SwitchRam.asm
 delete mode 100644 EmulatorPkg/Sec/Ia32/TempRam.c
 delete mode 100644 EmulatorPkg/Sec/X64/SwitchRam.S
 delete mode 100644 EmulatorPkg/Sec/X64/SwitchRam.asm

diff --git a/EmulatorPkg/Sec/Ia32/SwitchRam.S b/EmulatorPkg/Sec/Ia32/SwitchRam.S
deleted file mode 100644
index 39304daef1..00
--- a/EmulatorPkg/Sec/Ia32/SwitchRam.S
+++ /dev/null
@@ -1,95 +0,0 @@
-#--
-#
-# Copyright (c) 2007, Intel Corporation. All rights reserved.
-# This program and the accompanying materials
-# are licensed and made available under the terms and conditions of the BSD 
License
-# which accompanies this distribution.  The full text of the license may be 
found at
-# http:#opensource.org/licenses/bsd-license.php
-#
-# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
-# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
-#
-# Module Name:
-#
-#   Stack.asm
-#
-# Abstract:
-#
-#   Switch the stack from temporary memory to permenent memory.
-#
-#--
-
- .text
-
-
-//--
-// VOID
-// EFIAPI
-// SecSwitchStack (
-//   UINT32   TemporaryMemoryBase,
-//   UINT32   PermenentMemoryBase
-//   )//
-//--
-ASM_GLOBAL ASM_PFX(SecSwitchStack)
-ASM_PFX(SecSwitchStack):
-#
-# Save three register: eax, ebx, ecx
-#
-push  %eax
-push  %ebx
-push  %ecx
-push  %edx
-
-#
-# !!CAUTION!! this function address's is pushed into stack after
-# migration of whole temporary memory, so need save it to permenent
-# memory at first!
-#
-
-movl  20(%esp), %ebx# Save the first parameter
-movl  24(%esp), %ecx# Save the second parameter
-
-#
-# Save this function's return address into permenent memory at first.
-# Then, Fixup the esp point to permenent memory
-#
-
-movl  %esp, %eax
-subl  %ebx, %eax
-addl  %ecx, %eax
-movl  (%esp), %edx # copy pushed register's value to 
permenent memory
-movl  %edx, (%eax)
-movl  4(%esp), %edx
-movl  %edx, 4(%eax)
-movl  8(%esp), %edx
-movl  %edx, 8(%eax)
-movl  12(%esp), %edx
-movl  %edx, 12(%eax)
-movl  16(%esp), %edx
-movl  %edx, 16(%eax)
-movl  %eax, %esp   # From now, esp is pointed to permenent 
memory
-
-#
-# Fixup the ebp point to permenent memory
-#
-#ifndef __APPLE__
-movl   %ebp, %eax
-subl   %ebx, %eax
-addl   %ecx, %eax
-movl   %eax, %ebp  # From now, ebp is pointed to permenent 
memory
-
-#
-# Fixup callee's ebp point for PeiDispatch
-#
-movl   (%ebp), %eax
-subl   %ebx, %eax
-addl   %ecx, %eax
-movl   %eax, (%ebp)# From now, Temporary's PPI caller's 
stack is in permenent memory
-#endif
-
-pop   %edx
-pop   %ecx
-pop   %ebx
-pop   %eax
-ret
-
diff --git a/EmulatorPkg/Sec/Ia32/SwitchRam.asm 
b/EmulatorPkg/Sec/Ia32/SwitchRam.asm
deleted file mode 100644
index 731ee0ffdb..00
--- a/EmulatorPkg/Sec/Ia32/SwitchRam.asm
+++ /dev/null
@@ -1,94 +0,0 @@
-;--
-;
-; Copyright (c) 2007 - 2012, 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

[edk2] [PATCH v2 1/3] MdeModulePkg/PciBus: Change PCI_IO_DEVICE.RomSize to UINT32 type

2019-02-12 Thread Ray Ni 
Per PCI Spec, the option ROM BAR is 32bit so the maximum option ROM
size can be hold by UINT32 type.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni 
Cc: Hao Wu 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h   | 4 ++--
 MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c | 6 +++---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c| 8 
 MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.h| 4 ++--
 MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c   | 4 ++--
 5 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
index 6938802857..18d76ea965 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h
@@ -1,7 +1,7 @@
 /** @file
   Header files and data structures needed by PCI Bus module.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, 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
@@ -237,7 +237,7 @@ struct _PCI_IO_DEVICE {
   //
   // The OptionRom Size
   //
-  UINT64RomSize;
+  UINT32RomSize;
 
   //
   // TRUE if all OpROM (in device or in platform specific position) have been 
processed
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
index ab791541c3..7fb8e596f5 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciDeviceSupport.c
@@ -1,7 +1,7 @@
 /** @file
   Supporting functions implementaion for PCI devices management.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 (C) Copyright 2018 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
@@ -259,7 +259,7 @@ RegisterPciDevice (
);
   if (!EFI_ERROR (Status)) {
 PciIoDevice->EmbeddedRom= FALSE;
-PciIoDevice->RomSize= PlatformOpRomSize;
+PciIoDevice->RomSize= (UINT32) PlatformOpRomSize;
 PciIoDevice->PciIo.RomSize  = PlatformOpRomSize;
 PciIoDevice->PciIo.RomImage = PlatformOpRomBuffer;
 //
@@ -285,7 +285,7 @@ RegisterPciDevice (
);
   if (!EFI_ERROR (Status)) {
 PciIoDevice->EmbeddedRom= FALSE;
-PciIoDevice->RomSize= PlatformOpRomSize;
+PciIoDevice->RomSize= (UINT32) PlatformOpRomSize;
 PciIoDevice->PciIo.RomSize  = PlatformOpRomSize;
 PciIoDevice->PciIo.RomImage = PlatformOpRomBuffer;
 //
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c
index f44db01a9d..f8ba342681 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.c
@@ -1,7 +1,7 @@
 /** @file
   PCI eunmeration implementation on entire PCI bus system for PCI Bus module.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.
 (C) Copyright 2015 Hewlett Packard Enterprise Development LP
 This program and the accompanying materials
 are licensed and made available under the terms and conditions of the BSD 
License
@@ -528,15 +528,15 @@ DetermineRootBridgeAttributes (
   @return Max size of option rom needed.
 
 **/
-UINT64
+UINT32
 GetMaxOptionRomSize (
   IN PCI_IO_DEVICE   *Bridge
   )
 {
   LIST_ENTRY  *CurrentLink;
   PCI_IO_DEVICE   *Temp;
-  UINT64  MaxOptionRomSize;
-  UINT64  TempOptionRomSize;
+  UINT32  MaxOptionRomSize;
+  UINT32  TempOptionRomSize;
 
   MaxOptionRomSize = 0;
 
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.h 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.h
index 2df1fb0b94..ed4c875d36 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciEnumerator.h
@@ -1,7 +1,7 @@
 /** @file
   PCI bus enumeration logic function declaration for PCI bus module.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, 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
@@ -109,7 +109,7 @@ DetermineRootBridgeAttributes (
   @return Max size of option rom needed.
 
 **/
-UINT64
+UINT32
 GetMaxOptio

[edk2] [PATCH v2 0/3] MdeModulePkg/PciBus: Fix a bug PPB MEM32 BAR isn't restored sometimes

2019-02-12 Thread Ray Ni 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1505

v2: fixed all typos in PciBus driver.
changed RomSize to UINT32 and added type cast to PPB MEM32 BAR
Base/Length to avoid using RShiftU64().


Ray Ni (3):
  MdeModulePkg/PciBus: Change PCI_IO_DEVICE.RomSize to UINT32 type
  MdeModulePkg/PciBus: Correct typos
  MdeModulePkg/PciBus: Fix a bug PPB MEM32 BAR isn't restored sometimes

 MdeModulePkg/Bus/Pci/PciBusDxe/PciBus.h   |   4 +-
 MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.c   |  14 +--
 MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.h   |  16 +--
 .../Bus/Pci/PciBusDxe/PciDeviceSupport.c  |  20 ++--
 .../Bus/Pci/PciBusDxe/PciDeviceSupport.h  |  14 +--
 .../Bus/Pci/PciBusDxe/PciDriverOverride.c |   4 +-
 .../Bus/Pci/PciBusDxe/PciDriverOverride.h |   4 +-
 .../Bus/Pci/PciBusDxe/PciEnumerator.c |   8 +-
 .../Bus/Pci/PciBusDxe/PciEnumerator.h |   8 +-
 .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c  |  42 +++
 .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.h  |  16 +--
 .../Bus/Pci/PciBusDxe/PciHotPlugSupport.c |  16 +--
 .../Bus/Pci/PciBusDxe/PciHotPlugSupport.h |  18 +--
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c|  16 +--
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h|   4 +-
 MdeModulePkg/Bus/Pci/PciBusDxe/PciLib.c   |   4 +-
 .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   |   6 +-
 .../Bus/Pci/PciBusDxe/PciOptionRomSupport.h   |   4 +-
 .../Bus/Pci/PciBusDxe/PciPowerManagement.c|   4 +-
 .../Bus/Pci/PciBusDxe/PciPowerManagement.h|   4 +-
 .../Bus/Pci/PciBusDxe/PciResourceSupport.c| 113 +-
 .../Bus/Pci/PciBusDxe/PciResourceSupport.h|  41 ---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.h  |   7 +-
 23 files changed, 190 insertions(+), 197 deletions(-)

-- 
2.20.1.windows.1

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

[edk2] [PATCH v2 2/3] MdeModulePkg/PciBus: Correct typos

2019-02-12 Thread Ray Ni 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni 
Cc: Hao Wu 
---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.c   | 14 ++---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.h   | 16 ++---
 .../Bus/Pci/PciBusDxe/PciDeviceSupport.c  | 14 ++---
 .../Bus/Pci/PciBusDxe/PciDeviceSupport.h  | 14 ++---
 .../Bus/Pci/PciBusDxe/PciDriverOverride.c |  4 +-
 .../Bus/Pci/PciBusDxe/PciDriverOverride.h |  4 +-
 .../Bus/Pci/PciBusDxe/PciEnumerator.h |  4 +-
 .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.c  | 42 ++---
 .../Bus/Pci/PciBusDxe/PciEnumeratorSupport.h  | 16 ++---
 .../Bus/Pci/PciBusDxe/PciHotPlugSupport.c | 16 ++---
 .../Bus/Pci/PciBusDxe/PciHotPlugSupport.h | 18 +++---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.c| 16 ++---
 MdeModulePkg/Bus/Pci/PciBusDxe/PciIo.h|  4 +-
 .../Bus/Pci/PciBusDxe/PciOptionRomSupport.c   |  6 +-
 .../Bus/Pci/PciBusDxe/PciOptionRomSupport.h   |  4 +-
 .../Bus/Pci/PciBusDxe/PciPowerManagement.c|  4 +-
 .../Bus/Pci/PciBusDxe/PciPowerManagement.h|  4 +-
 .../Bus/Pci/PciBusDxe/PciResourceSupport.c| 62 +--
 .../Bus/Pci/PciBusDxe/PciResourceSupport.h| 41 ++--
 MdeModulePkg/Bus/Pci/PciBusDxe/PciRomTable.h  |  7 +--
 20 files changed, 154 insertions(+), 156 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.c
index 0bc1fbfeff..a71868cbf8 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.c
@@ -1,7 +1,7 @@
 /** @file
   PCI command register operations supporting functions implementation for PCI 
Bus module.
 
-Copyright (c) 2006 - 2015, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, 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
@@ -75,12 +75,12 @@ PciOperateRegister (
 }
 
 /**
-  Check the cpability supporting by given device.
+  Check the capability supporting by given device.
 
   @param PciIoDevice   Pointer to instance of PCI_IO_DEVICE.
 
-  @retval TRUE Cpability supportted.
-  @retval FALSECpability not supportted.
+  @retval TRUE Capability supported.
+  @retval FALSECapability not supported.
 
 **/
 BOOLEAN
@@ -103,7 +103,7 @@ PciCapabilitySupport (
   @param OffsetA pointer to the offset returned.
   @param NextRegBlock  A pointer to the next block returned.
 
-  @retval EFI_SUCCESS  Successfuly located capability register block.
+  @retval EFI_SUCCESS  Successfully located capability register block.
   @retval EFI_UNSUPPORTED  Pci device does not support capability.
   @retval EFI_NOT_FOUNDPci device support but can not find register block.
 
@@ -121,7 +121,7 @@ LocateCapabilityRegBlock (
   UINT8   CapabilityID;
 
   //
-  // To check the cpability of this device supports
+  // To check the capability of this device supports
   //
   if (!PciCapabilitySupport (PciIoDevice)) {
 return EFI_UNSUPPORTED;
@@ -195,7 +195,7 @@ LocateCapabilityRegBlock (
   @param OffsetA pointer to the offset returned.
   @param NextRegBlock  A pointer to the next block returned.
 
-  @retval EFI_SUCCESS  Successfuly located capability register block.
+  @retval EFI_SUCCESS  Successfully located capability register block.
   @retval EFI_UNSUPPORTED  Pci device does not support capability.
   @retval EFI_NOT_FOUNDPci device support but can not find register block.
 
diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.h 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.h
index cc942d0d42..3e1746b969 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.h
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciCommand.h
@@ -1,7 +1,7 @@
 /** @file
   PCI command register operations supporting functions declaration for PCI Bus 
module.
 
-Copyright (c) 2006 - 2009, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, 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
@@ -84,12 +84,12 @@ PciOperateRegister (
   );
 
 /**
-  Check the cpability supporting by given device.
+  Check the capability supporting by given device.
 
   @param PciIoDevice   Pointer to instance of PCI_IO_DEVICE.
 
-  @retval TRUE Cpability supportted.
-  @retval FALSECpability not supportted.
+  @retval TRUE Capability supported.
+  @retval FALSECapability not supported.
 
 **/
 BOOLEAN
@@ -105,7 +105,7 @@ PciCapabilitySupport (
   @param OffsetA pointer to the offset returned.
   @param NextRegBlock  A pointer to the next block returned.
 
-  @retval

[edk2] [PATCH v2 3/3] MdeModulePkg/PciBus: Fix a bug PPB MEM32 BAR isn't restored sometimes

2019-02-12 Thread Ray Ni 
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1505

When a device under PPB contains option ROM but doesn't require 32bit
MMIO, ProgrameUpstreamBridgeForRom() cannot correctly restore the
PPB MEM32 RANGE BAR. It causes the 32bit MMIO conflict which may
cause system hangs in boot.

The root cause is when ProgrameUpstreamBridgeForRom() calls
ProgramPpbApperture() to restore the PPB MEM32 RANGE BAR, the
ProgramPpbApperture() skips to program the BAR when the resource
length is 0.

This patch fixes this issue by not calling ProgramPpbApperture().
Instead, it directly programs the PPB MEM32 RANGE BAR.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni 
Cc: Hao Wu 
Cc: Dandan Bi 
---
 .../Bus/Pci/PciBusDxe/PciResourceSupport.c| 51 +--
 1 file changed, 23 insertions(+), 28 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
index f5ae3d857b..70e45040e2 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
@@ -1661,57 +1661,52 @@ ProgramUpstreamBridgeForRom (
   IN BOOLEAN Enable
   )
 {
-  PCI_IO_DEVICE *Parent;
-  PCI_RESOURCE_NODE Node;
-  UINT64Base;
-  UINT64Length;
+  PCI_IO_DEVICE   *Parent;
+  EFI_PCI_IO_PROTOCOL *PciIo;
+  UINT16  Base;
+  UINT16  Limit;
   //
   // For root bridge, just return.
   //
   Parent = PciDevice->Parent;
-  ZeroMem (&Node, sizeof (Node));
   while (Parent != NULL) {
 if (!IS_PCI_BRIDGE (&Parent->Pci)) {
   break;
 }
 
-Node.PciDev = Parent;
-Node.Alignment  = 0;
-Node.Bar= PPB_MEM32_RANGE;
-Node.ResType= PciBarTypeMem32;
-Node.Offset = 0;
+PciIo = &Parent->PciIo;
 
 //
 // Program PPB to only open a single <= 16MB aperture
 //
 if (Enable) {
-  //
-  // Save the original PPB_MEM32_RANGE BAR.
-  // The values will be changed by ProgramPpbApperture().
-  //
-  Base   = Parent->PciBar[Node.Bar].BaseAddress;
-  Length = Parent->PciBar[Node.Bar].Length;
-
   //
   // Only cover MMIO for Option ROM.
   //
-  Node.Length = PciDevice->RomSize;
-  ProgramPpbApperture (OptionRomBase, &Node);
-
-  //
-  // Restore the original PPB_MEM32_RANGE BAR.
-  // So the MEM32 RANGE BAR register can be restored when disable the 
decoding.
-  //
-  Parent->PciBar[Node.Bar].BaseAddress = Base;
-  Parent->PciBar[Node.Bar].Length  = Length;
+  Base  = (UINT16) (OptionRomBase >> 16);
+  Limit = (UINT16) ((OptionRomBase + PciDevice->RomSize - 1) >> 16);
+  PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, OFFSET_OF (PCI_TYPE01, 
Bridge.MemoryBase),  1, &Base);
+  PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, OFFSET_OF (PCI_TYPE01, 
Bridge.MemoryLimit), 1, &Limit);
 
   PCI_ENABLE_COMMAND_REGISTER (Parent, EFI_PCI_COMMAND_MEMORY_SPACE);
 } else {
   //
   // Cover 32bit MMIO for devices below the bridge.
   //
-  Node.Length = Parent->PciBar[Node.Bar].Length;
-  ProgramPpbApperture (Parent->PciBar[Node.Bar].BaseAddress, &Node);
+  if (Parent->PciBar[PPB_MEM32_RANGE].Length == 0) {
+//
+// When devices under the bridge contains Option ROM and doesn't 
require 32bit MMIO.
+//
+Base  = (UINT16) gAllOne;
+Limit = (UINT16) gAllZero;
+  } else {
+Base  = (UINT16) ((UINT32) Parent->PciBar[PPB_MEM32_RANGE].BaseAddress 
>> 16);
+Limit = (UINT16) ((UINT32) (Parent->PciBar[PPB_MEM32_RANGE].BaseAddress
++ Parent->PciBar[PPB_MEM32_RANGE].Length - 
1) >> 16);
+  }
+  PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, OFFSET_OF (PCI_TYPE01, 
Bridge.MemoryBase),  1, &Base);
+  PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, OFFSET_OF (PCI_TYPE01, 
Bridge.MemoryLimit), 1, &Limit);
+
   PCI_DISABLE_COMMAND_REGISTER (Parent, EFI_PCI_COMMAND_MEMORY_SPACE);
 }
 
-- 
2.20.1.windows.1

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

[edk2] [PATCH] MdeModulePkg/PciBus: Fix a bug PPB MEM32 BAR isn't restored sometimes

2019-02-01 Thread Ray Ni 
From: Ruiyu Ni 

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

When a device under PPB contains option ROM but doesn't require 32bit
MMIO, ProgrameUpstreamBridgeForRom() cannot correctly restore the
PPB MEM32 RANGE BAR. It causes the 32bit MMIO conflict which may
cause system hangs in boot.

The root cause is when ProgrameUpstreamBridgeForRom() calls
ProgramPpbApperture() to restore the PPB MEM32 RANGE BAR, the
ProgramPpbApperture() skips to program the BAR when the resource
length is 0.

This patch fixes this issue by not calling ProgramPpbApperture().
Instead, it directly programs the PPB MEM32 RANGE BAR.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ray Ni 
Cc: Hao Wu 
Cc: Dandan Bi 
---
 .../Bus/Pci/PciBusDxe/PciResourceSupport.c| 53 +--
 1 file changed, 24 insertions(+), 29 deletions(-)

diff --git a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c 
b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
index d3cbefbadf..77cdc3e844 100644
--- a/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
+++ b/MdeModulePkg/Bus/Pci/PciBusDxe/PciResourceSupport.c
@@ -1,7 +1,7 @@
 /** @file
   PCI resouces support functions implemntation for PCI Bus module.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.
+Copyright (c) 2006 - 2019, 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
@@ -1661,57 +1661,52 @@ ProgrameUpstreamBridgeForRom (
   IN BOOLEAN Enable
   )
 {
-  PCI_IO_DEVICE *Parent;
-  PCI_RESOURCE_NODE Node;
-  UINT64Base;
-  UINT64Length;
+  PCI_IO_DEVICE   *Parent;
+  EFI_PCI_IO_PROTOCOL *PciIo;
+  UINT16  Base;
+  UINT16  Limit;
   //
   // For root bridge, just return.
   //
   Parent = PciDevice->Parent;
-  ZeroMem (&Node, sizeof (Node));
   while (Parent != NULL) {
 if (!IS_PCI_BRIDGE (&Parent->Pci)) {
   break;
 }
 
-Node.PciDev = Parent;
-Node.Alignment  = 0;
-Node.Bar= PPB_MEM32_RANGE;
-Node.ResType= PciBarTypeMem32;
-Node.Offset = 0;
+PciIo = &Parent->PciIo;
 
 //
 // Program PPB to only open a single <= 16MB apperture
 //
 if (Enable) {
-  //
-  // Save the original PPB_MEM32_RANGE BAR.
-  // The values will be changed by ProgramPpbApperture().
-  //
-  Base   = Parent->PciBar[Node.Bar].BaseAddress;
-  Length = Parent->PciBar[Node.Bar].Length;
-
   //
   // Only cover MMIO for Option ROM.
   //
-  Node.Length = PciDevice->RomSize;
-  ProgramPpbApperture (OptionRomBase, &Node);
-
-  //
-  // Restore the original PPB_MEM32_RANGE BAR.
-  // So the MEM32 RANGE BAR register can be restored when disable the 
decoding.
-  //
-  Parent->PciBar[Node.Bar].BaseAddress = Base;
-  Parent->PciBar[Node.Bar].Length  = Length;
+  Base  = (UINT16) (OptionRomBase >> 16);
+  Limit = (UINT16) ((OptionRomBase + PciDevice->RomSize - 1) >> 16);
+  PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, OFFSET_OF (PCI_TYPE01, 
Bridge.MemoryBase),  1, &Base);
+  PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, OFFSET_OF (PCI_TYPE01, 
Bridge.MemoryLimit), 1, &Limit);
 
   PCI_ENABLE_COMMAND_REGISTER (Parent, EFI_PCI_COMMAND_MEMORY_SPACE);
 } else {
   //
   // Cover 32bit MMIO for devices below the bridge.
   //
-  Node.Length = Parent->PciBar[Node.Bar].Length;
-  ProgramPpbApperture (Parent->PciBar[Node.Bar].BaseAddress, &Node);
+  if (Parent->PciBar[PPB_MEM32_RANGE].Length == 0) {
+//
+// When devices under the bridge contains Option ROM and doesn't 
require 32bit MMIO.
+//
+Base  = (UINT16) gAllOne;
+Limit = (UINT16) gAllZero;
+  } else {
+Base  = (UINT16) (Parent->PciBar[PPB_MEM32_RANGE].BaseAddress >> 16);
+Limit = (UINT16)
+((Parent->PciBar[PPB_MEM32_RANGE].BaseAddress + 
Parent->PciBar[PPB_MEM32_RANGE].Length - 1) >> 16);
+  }
+  PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, OFFSET_OF (PCI_TYPE01, 
Bridge.MemoryBase),  1, &Base);
+  PciIo->Pci.Write (PciIo, EfiPciIoWidthUint16, OFFSET_OF (PCI_TYPE01, 
Bridge.MemoryLimit), 1, &Limit);
+
   PCI_DISABLE_COMMAND_REGISTER (Parent, EFI_PCI_COMMAND_MEMORY_SPACE);
 }
 
-- 
2.20.1.windows.1

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