[Patch 0/3] Fix Quark platform ASSERT() on GCC tip

[Patch 0/3] Fix Quark platform ASSERT() on GCC tip

[Jeff Fan]

Flat32.S should set ESP to top of eSRAM range that aligns with Flat32.asm.
It cause ASSERT() reported at https://tianocore.acgmultimedia.com/show_bug.cgi?id=123

Jeff Fan (3):
  QuarkPlatformPkg/PlatformSecLib: Fix stack pointer issue in Flat32.S
  UefiCpuPkg/SecCore: Fix comment typo
  UefiCpuPkg/SecCore: SecPlatformInformation(2) are optional PPIs

 QuarkPlatformPkg/Library/PlatformSecLib/Ia32/Flat32.S |  4 ++--
 UefiCpuPkg/SecCore/SecBist.c                          | 14 ++++++++------
 2 files changed, 10 insertions(+), 8 deletions(-)

-- 
2.9.3.windows.2

[Patch 1/3] QuarkPlatformPkg/PlatformSecLib: Fix stack pointer issue in Flat32.S

[Jeff Fan]

ESP should be set to top of eSRAM range that aligns with Flat32.asm. Because CPU
BIST data will be located at top of STACK, this issue leads Platform Sec Lib
cannot get the correct CPU BIST information.

This fix is to address below issue:
  https://tianocore.acgmultimedia.com/show_bug.cgi?id=123

Cc: Steven Shi <Steven.shi@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Kelly Steele <kelly.steele@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 QuarkPlatformPkg/Library/PlatformSecLib/Ia32/Flat32.S | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/QuarkPlatformPkg/Library/PlatformSecLib/Ia32/Flat32.S b/QuarkPlatformPkg/Library/PlatformSecLib/Ia32/Flat32.S
index 2bb503f..f35dbcf 100644
--- a/QuarkPlatformPkg/Library/PlatformSecLib/Ia32/Flat32.S
+++ b/QuarkPlatformPkg/Library/PlatformSecLib/Ia32/Flat32.S
@@ -1,6 +1,6 @@
 #------------------------------------------------------------------------------
 #
-# Copyright (c) 2013 - 2015 Intel Corporation.
+# Copyright (c) 2013 - 2016 Intel Corporation.
 #
 # This program and the accompanying materials
 # are licensed and made available under the terms and conditions of the BSD License
@@ -263,7 +263,7 @@ L0:
   # Set up stack pointer
   #
   movl    ASM_PFX(PcdGet32(PcdEsramStage1Base)), %esp
-  movl    $QUARK_STACK_SIZE_BYTES, %esi
+  movl    $QUARK_ESRAM_MEM_SIZE_BYTES, %esi
   addl    %esi, %esp                          # ESP = top of stack (stack grows downwards).
 
   #
-- 
2.9.3.windows.2

[Patch 2/3] UefiCpuPkg/SecCore: Fix comment typo

[Jeff Fan]

Revert SecPlatformInformation2 and SecPlatformInformation in two comment blocks.
And correct the words.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 UefiCpuPkg/SecCore/SecBist.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/SecCore/SecBist.c b/UefiCpuPkg/SecCore/SecBist.c
index dd5c5e5..19f3492 100644
--- a/UefiCpuPkg/SecCore/SecBist.c
+++ b/UefiCpuPkg/SecCore/SecBist.c
@@ -230,9 +230,9 @@ RepublishSecPlatformInformationPpi (
       (UINTN) BistInformationSize
       );
     //
-    // The old SecPlatformInformation data is on CAR.
-    // After memory discovered, we should never get it from CAR, or the data will be crashed.
-    // So, we reinstall SecPlatformInformation PPI here.
+    // The old SecPlatformInformation2 data is on temporary memory.
+    // After memory discovered, we should never get it from temporary memory,
+    // or the data will be crashed. So, we reinstall SecPlatformInformation2 PPI here.
     //
     Status = PeiServicesReInstallPpi (
                SecInformationDescriptor,
@@ -253,9 +253,9 @@ RepublishSecPlatformInformationPpi (
         (UINTN) BistInformationSize
         );
       //
-      // The old SecPlatformInformation2 data is on CAR.
-      // After memory discovered, we should never get it from CAR, or the data will be crashed.
-      // So, we reinstall SecPlatformInformation2 PPI here.
+      // The old SecPlatformInformation data is on temporary memory.
+      // After memory discovered, we should never get it from temporary memory,
+      // or the data will be crashed. So, we reinstall SecPlatformInformation PPI here.
       //
       Status = PeiServicesReInstallPpi (
                  SecInformationDescriptor,
-- 
2.9.3.windows.2

[Patch 3/3] UefiCpuPkg/SecCore: SecPlatformInformation(2) are optional PPIs

[Jeff Fan]

Currently, this is ASSERT() if neither SecPlatformInformation2 nor
SecPlatformInformation PPIs are found. This is not correct. Per PI specification
both of them are optional PPI. Platform may not install them.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Jeff Fan <jeff.fan@intel.com>
---
 UefiCpuPkg/SecCore/SecBist.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/UefiCpuPkg/SecCore/SecBist.c b/UefiCpuPkg/SecCore/SecBist.c
index 19f3492..ba7d7ca 100644
--- a/UefiCpuPkg/SecCore/SecBist.c
+++ b/UefiCpuPkg/SecCore/SecBist.c
@@ -261,6 +261,8 @@ RepublishSecPlatformInformationPpi (
                  SecInformationDescriptor,
                  &mPeiSecPlatformInformation
                  );
+    } else if (Status == EFI_NOT_FOUND) {
+      return;
     }
   }
 
-- 
2.9.3.windows.2

Re: [Patch 0/3] Fix Quark platform ASSERT() on GCC tip

[Kinney, Michael D]

Jeff,

These changes look good to me and I have verified Galileo boots with these changes.

Series Reviewed-by: Michael Kinney <michael.d.kinney@intel.com>

Mike

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jeff Fan
> Sent: Tuesday, September 20, 2016 1:29 AM
> To: edk2-devel@ml01.01.org
> Subject: [edk2] [Patch 0/3] Fix Quark platform ASSERT() on GCC tip
> 
> Flat32.S should set ESP to top of eSRAM range that aligns with Flat32.asm.
> It cause ASSERT() reported at https://tianocore.acgmultimedia.com/show_bug.cgi?id=123
> 
> Jeff Fan (3):
>   QuarkPlatformPkg/PlatformSecLib: Fix stack pointer issue in Flat32.S
>   UefiCpuPkg/SecCore: Fix comment typo
>   UefiCpuPkg/SecCore: SecPlatformInformation(2) are optional PPIs
> 
>  QuarkPlatformPkg/Library/PlatformSecLib/Ia32/Flat32.S |  4 ++--
>  UefiCpuPkg/SecCore/SecBist.c                          | 14 ++++++++------
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> --
> 2.9.3.windows.2
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel