public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [edk2-devel][edk2-platforms][PATCH v1 0/7]
@ 2022-09-06 17:42 Benjamin Doron
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 1/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Cleanup library includes Benjamin Doron
                   ` (6 more replies)
  0 siblings, 7 replies; 9+ messages in thread
From: Benjamin Doron @ 2022-09-06 17:42 UTC (permalink / raw)
  To: devel

Cleanup some unnecessary library #include's and LibraryClasses copied
from KabylakeRvp3 as part of the initial bring-up work. Some
quality-of-life improvements, such as working Secure Boot.

Enhancement to the EC support, preparing to implement the SMI handler.
Board detection data is now predictable, though more work is required
to finalise the desired output buffer correspondence with board ID.

Implement a HII form to control BIOS lock functionality. Other
configuration options planned.

Benjamin Doron (7):
  KabylakeOpenBoardPkg/AspireVn7Dash572G: Cleanup library includes
  KabylakeOpenBoardPkg/AspireVn7Dash572G: Enhance the build-logic
  KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi: Improvements for EC ACPI
  KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Resets notify
    EC
  KabylakeOpenBoardPkg/AspireVn7Dash572G: Use Setup to control security
  KabylakeOpenBoardPkg/AspireVn7Dash572G: Improve board detection
  KabylakeOpenBoardPkg/AspireVn7Dash572G: Align DEBUG() use

 .../Acpi/BoardAcpiTables.inf                  |   4 +
 .../AspireVn7Dash572G/Acpi/BoardSsdt.asl      |  29 +-
 .../AspireVn7Dash572G/Acpi/battery.asl        |  11 +-
 .../AspireVn7Dash572G/Acpi/ec.asl             | 130 +++---
 .../AspireVn7Dash572G/Acpi/eclib.asl          | 141 +++++++
 .../AspireVn7Dash572G/Acpi/mainboard.asl      |   6 +-
 .../AspireVn7Dash572G/Acpi/thermal.asl        |  16 +-
 .../PcieDeviceTable.c                         |   1 -
 .../PeiBoardPolicyUpdate.c                    |  62 ++-
 .../PeiPchPolicyUpdate.h                      |   3 +-
 .../PeiPchPolicyUpdatePreMem.c                |   1 -
 .../PeiSiliconPolicyUpdateLibFsp.inf          |  12 +-
 .../Include/BoardConfigNvData.h               |  37 ++
 .../Include/Library/BoardEcLib.h              |   5 +-
 .../DxeAspireVn7Dash572GAcpiTableLib.c        |   5 +
 .../BoardAcpiLib/DxeBoardAcpiTableLib.inf     |   6 +-
 .../SmmAspireVn7Dash572GAcpiEnableLib.c       |  17 +-
 .../BoardAcpiLib/SmmBoardAcpiEnableLib.inf    |   3 +-
 .../Library/BoardEcLib/BoardEcLib.inf         |   1 +
 .../Library/BoardEcLib/EcCommands.c           |  66 ++--
 .../AspireVn7Dash572GHdaVerbTables.c          |   3 +-
 .../Library/BoardInitLib/BoardConfigVfr.vfr   |  68 ++++
 .../BoardInitLib/BoardConfigVfrStrings.uni    |  29 ++
 .../Library/BoardInitLib/DxeBoardConfigHii.c  | 374 ++++++++++++++++++
 .../Library/BoardInitLib/DxeBoardInitLib.c    | 116 +++++-
 .../Library/BoardInitLib/DxeBoardInitLib.h    | 131 ++++++
 .../Library/BoardInitLib/DxeBoardInitLib.inf  |  14 +
 .../BoardInitLib/PeiAspireVn7Dash572GDetect.c |  47 ++-
 .../PeiAspireVn7Dash572GInitLib.h             |   3 +-
 .../PeiAspireVn7Dash572GInitPostMemLib.c      |  29 +-
 .../PeiAspireVn7Dash572GInitPreMemLib.c       |  38 +-
 .../BoardInitLib/PeiBoardInitPostMemLib.inf   |   4 +-
 .../BoardInitLib/PeiBoardInitPreMemLib.c      |   2 +
 .../BoardInitLib/PeiBoardInitPreMemLib.inf    |   5 +-
 .../AspireVn7Dash572G/OpenBoardPkg.dsc        |  68 ++--
 .../AspireVn7Dash572G/OpenBoardPkg.fdf        |   3 +-
 .../OpenBoardPkgBuildOption.dsc               |   4 +-
 .../AspireVn7Dash572G/OpenBoardPkgPcd.dsc     |  59 +--
 .../DxeGopPolicyInit.h                        |   3 -
 .../DxeSaPolicyInit.h                         |   3 -
 .../DxeSiliconPolicyUpdateLib.c               |   9 +-
 .../DxeSiliconPolicyUpdateLib.inf             |   2 +
 .../PeiBoardPolicyUpdate.c                    |   2 +-
 .../Include/PlatformBoardId.h                 |   5 +-
 44 files changed, 1309 insertions(+), 268 deletions(-)
 create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/eclib.asl
 create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/BoardConfigNvData.h
 create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/BoardConfigVfr.vfr
 create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/BoardConfigVfrStrings.uni
 create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardConfigHii.c
 create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.h

-- 
2.37.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [edk2-devel][edk2-platforms][PATCH v1 1/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Cleanup library includes
  2022-09-06 17:42 [edk2-devel][edk2-platforms][PATCH v1 0/7] Benjamin Doron
@ 2022-09-06 17:42 ` Benjamin Doron
  2022-09-09 21:41   ` Isaac Oram
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 2/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Enhance the build-logic Benjamin Doron
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 9+ messages in thread
From: Benjamin Doron @ 2022-09-06 17:42 UTC (permalink / raw)
  To: devel; +Cc: Sai Chaganty, Isaac Oram, Chasel Chiu, Nate DeSimone

Remove unused includes, LibraryClasses and update a comment or two.

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 .../PcieDeviceTable.c                          |  1 -
 .../PeiBoardPolicyUpdate.c                     |  6 ++++--
 .../PeiPchPolicyUpdate.h                       |  3 ++-
 .../PeiPchPolicyUpdatePreMem.c                 |  1 -
 .../PeiSiliconPolicyUpdateLibFsp.inf           |  5 ++---
 .../BoardAcpiLib/DxeBoardAcpiTableLib.inf      |  5 +----
 .../SmmAspireVn7Dash572GAcpiEnableLib.c        |  9 +++++----
 .../BoardAcpiLib/SmmBoardAcpiEnableLib.inf     |  3 ++-
 .../Library/BoardEcLib/EcCommands.c            | 14 ++++++++------
 .../AspireVn7Dash572GHdaVerbTables.c           |  3 ++-
 .../BoardInitLib/PeiAspireVn7Dash572GInitLib.h |  3 +--
 .../PeiAspireVn7Dash572GInitPreMemLib.c        | 18 +++++++++---------
 .../BoardInitLib/PeiBoardInitPostMemLib.inf    |  4 +---
 .../BoardInitLib/PeiBoardInitPreMemLib.inf     |  5 +----
 .../AspireVn7Dash572G/OpenBoardPkg.fdf         |  3 ++-
 .../OpenBoardPkgBuildOption.dsc                |  4 ++--
 .../DxeGopPolicyInit.h                         |  3 ---
 .../DxeSaPolicyInit.h                          |  3 ---
 .../DxeSiliconPolicyUpdateLib.c                |  3 +--
 .../DxeSiliconPolicyUpdateLib.inf              |  2 ++
 20 files changed, 45 insertions(+), 53 deletions(-)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PcieDeviceTable.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PcieDeviceTable.c
index 205ca581c6f3..537fb5c8e4f4 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PcieDeviceTable.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PcieDeviceTable.c
@@ -7,7 +7,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
 #include "PeiPchPolicyUpdate.h"
-#include <Library/PchPcieRpLib.h>
 
 #define PCI_CLASS_NETWORK             0x02
 #define PCI_CLASS_NETWORK_ETHERNET    0x00
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiBoardPolicyUpdate.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiBoardPolicyUpdate.c
index 81cd8b940f05..452c961b17ac 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiBoardPolicyUpdate.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiBoardPolicyUpdate.c
@@ -12,14 +12,16 @@
 #include <Library/PcdLib.h>
 #include <PchPolicyCommon.h>
 
-/* TODO:
+/*
+ * TODO:
  * - Validate PCH Sample policies: only SA one used by default.
  * - Remove likely fuse-disabled devices when reset handling is committed?
  * - Remove duplicate policy
  *   - Consider updating some policies, rather than overriding. This could be factored into
  *     BoardInitLib for deduplication
  * - Copy initialised array, where sane
- * - Set IgdDvmt50PreAlloc? */
+ * - Set IgdDvmt50PreAlloc?
+ */
 
 #define SA_VR           0
 #define IA_VR           1
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdate.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdate.h
index 5e720b0041e8..134188698077 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdate.h
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdate.h
@@ -17,10 +17,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/DebugLib.h>
 #include <Library/IoLib.h>
 #include <Library/MmPciLib.h>
-#include <Ppi/SiPolicy.h>
 
 #include <FspEas.h>
 #include <FspmUpd.h>
 #include <FspsUpd.h>
 
+#include <PchPolicyCommon.h>
+
 #endif
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdatePreMem.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdatePreMem.c
index 2bc142c0e5ff..28e4e45375c2 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdatePreMem.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdatePreMem.c
@@ -9,7 +9,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include "PeiPchPolicyUpdate.h"
 #include <Library/BaseMemoryLib.h>
 #include <Library/PchInfoLib.h>
-#include <Library/PchPcrLib.h>
 #include <Library/PchHsioLib.h>
 #include <Library/PchPcieRpLib.h>
 #include <PchHsioPtssTables.h>
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf
index eac9344b0aa2..0e1b42c20cd8 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf
@@ -62,19 +62,18 @@
 
 [LibraryClasses.IA32]
   FspWrapperApiLib
-  FspWrapperPlatformLib
   BaseMemoryLib
   DebugLib
-  HobLib
   IoLib
   PcdLib
   MmPciLib
-  ConfigBlockLib
+  PciLib
   PeiSaPolicyLib
   PchInfoLib
   PchHsioLib
   PchPcieRpLib
   SiPolicyLib
+  MemoryAllocationLib
   PeiLib
 
 [Pcd]
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
index 0d8264554734..660afe9292ec 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
@@ -22,10 +22,7 @@
 #
 
 [LibraryClasses]
-  BaseLib
-  IoLib
-  PciLib
-  AslUpdateLib
+  PcdLib
   EcLib
 
 [Packages]
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmAspireVn7Dash572GAcpiEnableLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmAspireVn7Dash572GAcpiEnableLib.c
index 69e9c928ff69..fa2ed9745ea6 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmAspireVn7Dash572GAcpiEnableLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmAspireVn7Dash572GAcpiEnableLib.c
@@ -2,6 +2,7 @@
   Acer Aspire VN7-572G SMM Board ACPI Enable library
 
 Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2021, Baruch Binyamin Doron<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -20,8 +21,8 @@ AspireVn7Dash572GBoardEnableAcpi (
   EFI_STATUS  Status;
 
   /* Tests at runtime show this re-enables charging and battery reporting
-   * - Obtained somewhere from somewhere in vendor's SmmKbcDriver (or RtKbcDriver).
-   *   Further reversing will be performed */
+   * - Obtained from somewhere in vendor's SmmKbcDriver.
+   *   Further information is needed */
   Status = SendEcCommand (0xE9);  /* Vendor implements using ACPI "CMDB" register" */
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a(): SendEcCommand(0xE9) failed!\n", __FUNCTION__));
@@ -48,8 +49,8 @@ AspireVn7Dash572GBoardDisableAcpi (
   EFI_STATUS  Status;
 
   /* Tests at runtime show this disables charging and battery reporting
-   * - Obtained somewhere from somewhere in vendor's SmmKbcDriver (or RtKbcDriver).
-   *   Further reversing will be performed */
+   * - Obtained from somewhere in vendor's SmmKbcDriver.
+   *   Further information is needed */
   Status = SendEcCommand (0xE9);  /* Vendor implements using ACPI "CMDB" register" */
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_ERROR, "%a(): SendEcCommand(0xE9) failed!\n", __FUNCTION__));
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
index 63a54e1830a5..5db00224dfce 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
@@ -23,9 +23,10 @@
 
 [LibraryClasses]
   BaseLib
+  DebugLib
   EcLib
   IoLib
-  PciLib
+  PcdLib
   MmPciLib
   PchCycleDecodingLib
 
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
index 6e752b4e227e..54cfaba47b1b 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
@@ -13,7 +13,8 @@
 #include <Library/EcLib.h>
 #include <Library/IoLib.h>
 
-/* Notes:
+/*
+ * Notes:
  * - ACPI "CMDB": Writing to this offset is equivalent to sending commands.
  *   The CMDx bytes contain the command parameters.
  *
@@ -21,9 +22,10 @@
  *   - Commands: 0x58, 0xE1 and 0xE2
  *     - 0x51, 0x52: EC flash write?
  *   - ACPI CMDB: 0x63 and 0x64, 0xC7
- *     - 0x0B: Flash write (Boolean argument? Set in offset 0x0B?)
+ *     - 0x0B: Flash lock/write (Set offset 0x0B?)
+ *   - Key/recovery detection?
  *
- * Reversing vendor's protocols:
+ * Vendor's protocols:
  * - Only read and write are used.
  * - Query, ACPI "CMDB" processing and command 58 are unused.
  * - Equivalent KbcPeim is an unused PPI.
@@ -32,9 +34,9 @@
  */
 
 #define EC_INDEX_IO_PORT            0x1200
-#define EC_INDEX_IO_HIGH_ADDR_PORT  EC_INDEX_IO_PORT+1
-#define EC_INDEX_IO_LOW_ADDR_PORT   EC_INDEX_IO_PORT+2
-#define EC_INDEX_IO_DATA_PORT       EC_INDEX_IO_PORT+3
+#define EC_INDEX_IO_HIGH_ADDR_PORT  (EC_INDEX_IO_PORT + 1)
+#define EC_INDEX_IO_LOW_ADDR_PORT   (EC_INDEX_IO_PORT + 2)
+#define EC_INDEX_IO_DATA_PORT       (EC_INDEX_IO_PORT + 3)
 
 /**
   Reads a byte of EC RAM.
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/AspireVn7Dash572GHdaVerbTables.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/AspireVn7Dash572GHdaVerbTables.c
index 0573736060fa..cc7369f3484c 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/AspireVn7Dash572GHdaVerbTables.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/AspireVn7Dash572GHdaVerbTables.c
@@ -2,6 +2,7 @@
   HDA Verb table for Acer Aspire VN7-572G
 
 Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2021, Baruch Binyamin Doron<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -9,7 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #ifndef _ASPIRE_VN7_572G_HDA_VERB_TABLES_H_
 #define _ASPIRE_VN7_572G_HDA_VERB_TABLES_H_
 
-#include <Ppi/SiPolicy.h>
+#include <PchPolicyCommon.h>
 
 HDAUDIO_VERB_TABLE HdaVerbTableAlc255AspireVn7Dash572G = HDAUDIO_VERB_TABLE_INIT (
   //
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitLib.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitLib.h
index 83789c90becf..51a7b714c463 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitLib.h
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitLib.h
@@ -8,10 +8,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #ifndef _PEI_ASPIRE_VN7_572G_BOARD_INIT_LIB_H_
 #define _PEI_ASPIRE_VN7_572G_BOARD_INIT_LIB_H_
 
-#include <Uefi.h>
+#include <PiPei.h>
 #include <Library/BaseLib.h>
 #include <Library/PcdLib.h>
-#include <Library/MemoryAllocationLib.h>
 #include <Library/DebugLib.h>
 #include <Library/GpioLib.h>
 #include <Ppi/SiPolicy.h>
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPreMemLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPreMemLib.c
index 1b4c6b484b43..d0125ebdbcb2 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPreMemLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPreMemLib.c
@@ -1,6 +1,7 @@
 /** @file
 
 Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2021, Baruch Binyamin Doron<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -8,7 +9,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <PiPei.h>
 #include <Library/DebugLib.h>
 #include <Library/IoLib.h>
-#include <Library/MemoryAllocationLib.h>
 #include <Library/PcdLib.h>
 #include <Library/PchCycleDecodingLib.h>
 #include <Library/PchPmcLib.h>
@@ -16,7 +16,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/PciLib.h>
 #include <Library/SiliconInitLib.h>
 #include <Library/TimerLib.h>
-#include <Library/PeiLib.h>
 
 #include <Library/GpioLib.h>
 #include <GpioPinsSklLp.h>
@@ -47,12 +46,6 @@ GLOBAL_REMOVE_IF_UNREFERENCED const UINT16 RcompTargetAspireVn7Dash572G[SA_MRC_M
 #define DGPU_HOLD_RST	GPIO_SKL_LP_GPP_B4	/* Active low */
 #define DGPU_PWR_EN	GPIO_SKL_LP_GPP_B21	/* Active low */
 
-EFI_STATUS
-EFIAPI
-AspireVn7Dash572GBoardDetect (
-  VOID
-  );
-
 /**
   Aspire VN7-572G board configuration init function for PEI pre-memory phase.
 
@@ -75,7 +68,7 @@ AspireVn7Dash572GInitPreMem (
   //
   PcdSet8S (PcdSaMiscUserBd, 5);     // ULT/ULX/Mobile Halo
   PcdSet8S (PcdMrcCaVrefConfig, 2);  // DDR4: "VREF_CA to CH_A and VREF_DQ_B to CH_B"
-  // TODO: Clear Dq/Dqs?
+  // TODO: Search vendor FW for Dq/Dqs. Unnecessary if FSP detects LPDDR
   PcdSetBoolS (PcdMrcDqPinsInterleaved, TRUE);
 
   PcdSet32S (PcdMrcRcompResistor, (UINTN) RcompResistorAspireVn7Dash572G);
@@ -241,9 +234,16 @@ AspireVn7Dash572GBoardInitAfterMemoryInit (
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_WARN, "Failed to enable LGMR. Were ACPI tables built for LGMR memory map?\n"));
   }
+
   return EFI_SUCCESS;
 }
 
+EFI_STATUS
+EFIAPI
+AspireVn7Dash572GBoardDetect (
+  VOID
+  );
+
 EFI_STATUS
 EFIAPI
 AspireVn7Dash572GBoardDebugInit (
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPostMemLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPostMemLib.inf
index c8c49fa20dcc..7b68f66ac78b 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPostMemLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPostMemLib.inf
@@ -10,7 +10,7 @@
 [Defines]
   INF_VERSION                    = 0x00010005
   BASE_NAME                      = PeiBoardPostMemInitLib
-  FILE_GUID                      = 7fcc3900-d38d-419f-826b-72481e8b5509
+  FILE_GUID                      = 7FCC3900-D38D-419F-826B-72481E8B5509
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = BoardInitLib
@@ -18,8 +18,6 @@
 [LibraryClasses]
   BaseLib
   DebugLib
-  BaseMemoryLib
-  MemoryAllocationLib
   PcdLib
   SiliconInitLib
   PchCycleDecodingLib
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPreMemLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPreMemLib.inf
index c53114e15450..a3164870ef9b 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPreMemLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPreMemLib.inf
@@ -10,7 +10,7 @@
 [Defines]
   INF_VERSION                    = 0x00010005
   BASE_NAME                      = PeiBoardInitPreMemLib
-  FILE_GUID                      = ec3675bc-1470-417d-826e-37378140213d
+  FILE_GUID                      = EC3675BC-1470-417D-826E-37378140213D
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = BoardInitLib
@@ -18,8 +18,6 @@
 [LibraryClasses]
   BaseLib
   DebugLib
-  BaseMemoryLib
-  MemoryAllocationLib
   PcdLib
   SiliconInitLib
   TimerLib
@@ -30,7 +28,6 @@
   EcLib
   BoardEcLib
   GpioLib
-  PeiLib
   PeiServicesLib
   PchPmcLib
 
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.fdf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.fdf
index 864d5561d7d8..b59d9a4f24e1 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.fdf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.fdf
@@ -279,7 +279,7 @@ INF IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
 INF $(PLATFORM_PACKAGE)/PlatformInit/SiliconPolicyPei/SiliconPolicyPeiPostMem.inf
 
 !if gSiPkgTokenSpaceGuid.PcdPeiDisplayEnable == TRUE
-FILE FREEFORM = 4ad46122-ffeb-4a52-bfb0-518cfca02db0 {
+FILE FREEFORM = 4AD46122-FFEB-4A52-BFB0-518CFCA02DB0 {
   SECTION RAW = $(BOARD)/Vbt.bin
   SECTION UI  = "Vbt"
 }
@@ -346,6 +346,7 @@ APRIORI DXE {
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf
 INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
 
+# TODO: Add NvmExpressDxe if supporting Newgate and RayleighSLS
 INF  MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf
 INF  MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
 INF  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkgBuildOption.dsc b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkgBuildOption.dsc
index b1a04c474845..6e2053d67734 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkgBuildOption.dsc
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkgBuildOption.dsc
@@ -84,6 +84,7 @@ DEFINE DSC_PLTPKG_FEATURE_BUILD_OPTIONS = $(DSC_PLTPKG_FEATURE_BUILD_OPTIONS) $(
 DEFINE DSC_PLTPKG_FEATURE_BUILD_OPTIONS = $(DSC_PLTPKG_FEATURE_BUILD_OPTIONS) $(DSC_ACPI_BUILD_OPTIONS) $(UP_SERVER_SUPPORT_BUILD_OPTIONS) $(USBTYPEC_BUILD_OPTION) $(SINITBIN_BUILD_OPTION) $(MINTREE_FLAG_BUILD_OPTION)
 
 # FIXME: $(DSC_PLTPKG_FEATURE_BUILD_OPTIONS) is passed multiple times
+# BUGBUG: `-Wl,--allow-multiple-definition` breaks CLANG build
 [BuildOptions.Common.EDKII]
 
 #
@@ -141,8 +142,7 @@ MSFT:  *_*_X64_ASLCC_FLAGS    = $(DSC_PLTPKG_FEATURE_BUILD_OPTIONS)
   MSFT:*_*_*_DLINK_FLAGS = /ALIGN:4096
   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
 
-# FIXME: Protection broken, but works on UefiPayload, and not related to
-# FspWrapperNotifyDxe. Cannot be related to SMM?
+# FIXME: Protection broken, yet works on UefiPayload. Consider diffing module/library include lists (unrelated to FspWrapperNotifyDxe).
 # Force PE/COFF sections to be aligned at 4KB boundaries to support NX protection
 [BuildOptions.common.EDKII.DXE_DRIVER, BuildOptions.common.EDKII.DXE_CORE, BuildOptions.common.EDKII.UEFI_DRIVER, BuildOptions.common.EDKII.UEFI_APPLICATION]
   #MSFT:*_*_*_DLINK_FLAGS = /ALIGN:4096
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeGopPolicyInit.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeGopPolicyInit.h
index 63cad5e3753f..56cab1df9b1d 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeGopPolicyInit.h
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeGopPolicyInit.h
@@ -9,11 +9,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define _GOP_POLICY_INIT_DXE_H_
 
 #include <Protocol/FirmwareVolume2.h>
-#include <Library/UefiLib.h>
 #include <Library/BaseLib.h>
-#include <Library/DxeServicesTableLib.h>
 #include <Library/UefiBootServicesTableLib.h>
-#include <Library/UefiRuntimeServicesTableLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/DebugLib.h>
 #include <Library/PcdLib.h>
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSaPolicyInit.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSaPolicyInit.h
index 801387b9476f..88a507547f69 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSaPolicyInit.h
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSaPolicyInit.h
@@ -8,12 +8,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #ifndef _SA_POLICY_INIT_DXE_H_
 #define _SA_POLICY_INIT_DXE_H_
 
-#include <Library/BaseMemoryLib.h>
-#include <Library/MemoryAllocationLib.h>
 #include <Library/DebugLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Protocol/SaPolicy.h>
-#include <Library/DxeSaPolicyLib.h>
 
 #include <SaAccess.h>
 
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.c
index 6298bb53e65d..6840531da986 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.c
@@ -1,14 +1,13 @@
 /** @file
 
 Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2021, Baruch Binyamin Doron<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
 #include <Library/ConfigBlockLib.h>
 #include <Library/SiliconPolicyUpdateLib.h>
-#include <Library/PcdLib.h>
-#include <Library/DebugLib.h>
 #include <Protocol/GopPolicy.h>
 #include <Protocol/SaPolicy.h>
 
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.inf
index 63ac194cd0d5..989796cf8244 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.inf
@@ -17,6 +17,8 @@
 
 [LibraryClasses]
   BaseLib
+  BaseMemoryLib
+  UefiBootServicesTableLib
   PcdLib
   DebugLib
   ConfigBlockLib
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [edk2-devel][edk2-platforms][PATCH v1 2/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Enhance the build-logic
  2022-09-06 17:42 [edk2-devel][edk2-platforms][PATCH v1 0/7] Benjamin Doron
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 1/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Cleanup library includes Benjamin Doron
@ 2022-09-06 17:42 ` Benjamin Doron
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 3/7] KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi: Improvements for EC ACPI Benjamin Doron
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Benjamin Doron @ 2022-09-06 17:42 UTC (permalink / raw)
  To: devel; +Cc: Sai Chaganty, Isaac Oram, Nate DeSimone, Chasel Chiu

Update the AspireVn7Dash572G DSC files with assorted enhancements since
the initial porting work. Some planned features, such as Secure Boot and
measuring the default FSP UPDs to a TPM (which does have security
relevance), are now fully working.

Enable the working advanced features in use on this board.

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 .../AspireVn7Dash572G/OpenBoardPkg.dsc        | 63 +++++++++++--------
 .../AspireVn7Dash572G/OpenBoardPkgPcd.dsc     | 59 ++++++++++-------
 2 files changed, 74 insertions(+), 48 deletions(-)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.dsc b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.dsc
index f4552ee83d6b..75c537f1253f 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.dsc
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.dsc
@@ -22,15 +22,6 @@
   #
   DEFINE BIOS_SIZE_OPTION = SIZE_60
 
-  #
-  # Debug logging
-  #
-  DEFINE USE_HDMI_DEBUG_PORT  = FALSE
-  DEFINE USE_PEI_SPI_LOGGING  = FALSE
-  DEFINE USE_MEMORY_LOGGING   = FALSE
-  DEFINE RELEASE_LOGGING      = ($(USE_HDMI_DEBUG_PORT) || $(USE_PEI_SPI_LOGGING) || $(USE_MEMORY_LOGGING))
-  DEFINE TESTING              = TRUE
-
   PLATFORM_NAME                               = $(PLATFORM_PACKAGE)
   PLATFORM_GUID                               = AEEEF17C-36B6-4B68-949A-1E54CB33492F
   PLATFORM_VERSION                            = 0.1
@@ -40,9 +31,17 @@
   BUILD_TARGETS                               = DEBUG|RELEASE
   SKUID_IDENTIFIER                            = ALL
   FLASH_DEFINITION                            = $(PROJECT)/OpenBoardPkg.fdf
-
   FIX_LOAD_TOP_MEMORY_ADDRESS                 = 0x0
 
+  #
+  # Debug logging
+  #
+  DEFINE USE_HDMI_DEBUG_PORT  = FALSE
+  DEFINE USE_PEI_SPI_LOGGING  = FALSE
+  DEFINE USE_MEMORY_LOGGING   = FALSE
+  DEFINE RELEASE_LOGGING      = ($(USE_HDMI_DEBUG_PORT) || $(USE_PEI_SPI_LOGGING) || $(USE_MEMORY_LOGGING))
+  DEFINE TESTING              = FALSE
+
   #
   # Include PCD configuration for this board.
   #
@@ -143,7 +142,7 @@
   #######################################
   FspWrapperApiLib|IntelFsp2WrapperPkg/Library/BaseFspWrapperApiLib/BaseFspWrapperApiLib.inf
   FspWrapperApiTestLib|IntelFsp2WrapperPkg/Library/PeiFspWrapperApiTestLib/PeiFspWrapperApiTestLib.inf
-  # This board will set debugging library instances; FIXME: UART2 not used
+  # Board DSC will select debug library instances; NOTE: UART2 not used
   SerialPortLib|MdePkg/Library/BaseSerialPortLibNull/BaseSerialPortLibNull.inf
 
   #######################################
@@ -198,12 +197,11 @@
   #######################################
   PlatformHookLib|MdeModulePkg/Library/BasePlatformHookLibNull/BasePlatformHookLibNull.inf
 
-# NB: MinPlatform sets a NULL DebugLib and only overrides it for DEBUG builds
-# TODO: Now that all debug logging is routed through RSC, correct the defines
 [LibraryClasses.IA32.SEC]
   #######################################
   # Edk2 Packages
   #######################################
+# NOTE: No way that RSC avoids PeiServices in SEC? Even if valid on re-entry...
   DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
 
 !if $(USE_HDMI_DEBUG_PORT) == TRUE
@@ -231,7 +229,8 @@
   # Edk2 Packages
   #######################################
 # SPI logging requires local patch: InitializeMemoryServices() before ProcessLibraryConstructorList()
-# In-memory logging may require too many services for early core debug output
+# Strongly suspect DebugLibSerialPort constructor presents PeiDxeSerialPortLibMem dependency on services as a bug
+# - While RSC calls Initialize after dependencies and constructors are satisfied
 !if $(RELEASE_LOGGING) == TRUE
   DebugLib|MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
 !endif
@@ -257,7 +256,7 @@
   FspWrapperPlatformLib|$(PLATFORM_PACKAGE)/FspWrapper/Library/PeiFspWrapperPlatformLib/PeiFspWrapperPlatformLib.inf
   MultiBoardInitSupportLib|$(PLATFORM_PACKAGE)/PlatformInit/Library/MultiBoardInitSupportLib/PeiMultiBoardInitSupportLib.inf
   TestPointLib|$(PLATFORM_PACKAGE)/Test/Library/TestPointLib/PeiTestPointLib.inf
-!if ($(TARGET) == DEBUG || $(TESTING) == TRUE)
+!if ($(TARGET) == DEBUG || $(RELEASE_LOGGING) == TRUE)
   TestPointCheckLib|$(PLATFORM_PACKAGE)/Test/Library/TestPointCheckLib/PeiTestPointCheckLib.inf
 !endif
   SetCacheMtrrLib|$(PLATFORM_PACKAGE)/Library/SetCacheMtrrLib/SetCacheMtrrLibNull.inf
@@ -287,7 +286,8 @@
   #######################################
   # Edk2 Packages
   #######################################
-# In-memory logging may require too many services for early core debug output
+# Strongly suspect DebugLibSerialPort constructor presents PeiDxeSerialPortLibMem dependency on services as a bug
+# - While RSC calls Initialize after dependencies and constructors are satisfied
 !if ($(USE_MEMORY_LOGGING) == TRUE || $(USE_HDMI_DEBUG_PORT) == TRUE)
   DebugLib|MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
 !endif
@@ -315,7 +315,7 @@
   MultiBoardInitSupportLib|$(PLATFORM_PACKAGE)/PlatformInit/Library/MultiBoardInitSupportLib/DxeMultiBoardInitSupportLib.inf
   TestPointLib|$(PLATFORM_PACKAGE)/Test/Library/TestPointLib/DxeTestPointLib.inf
 
-!if ($(TARGET) == DEBUG || $(TESTING) == TRUE)
+!if ($(TARGET) == DEBUG || $(RELEASE_LOGGING) == TRUE)
   TestPointCheckLib|$(PLATFORM_PACKAGE)/Test/Library/TestPointCheckLib/DxeTestPointCheckLib.inf
 !endif
   #######################################
@@ -346,7 +346,8 @@
   #######################################
   # Edk2 Packages
   #######################################
-# In-memory logging may require too many services for early core debug output
+# Strongly suspect DebugLibSerialPort constructor presents PeiDxeSerialPortLibMem dependency on services as a bug
+# - While RSC calls Initialize after dependencies and constructors are satisfied
 !if ($(USE_MEMORY_LOGGING) == TRUE || $(USE_HDMI_DEBUG_PORT) == TRUE)
   DebugLib|MdeModulePkg/Library/PeiDxeDebugLibReportStatusCode/PeiDxeDebugLibReportStatusCode.inf
 !endif
@@ -370,7 +371,7 @@
   BoardAcpiEnableLib|$(PLATFORM_PACKAGE)/Acpi/Library/MultiBoardAcpiSupportLib/SmmMultiBoardAcpiSupportLib.inf
   MultiBoardAcpiSupportLib|$(PLATFORM_PACKAGE)/Acpi/Library/MultiBoardAcpiSupportLib/SmmMultiBoardAcpiSupportLib.inf
   TestPointLib|$(PLATFORM_PACKAGE)/Test/Library/TestPointLib/SmmTestPointLib.inf
-!if ($(TARGET) == DEBUG || $(TESTING) == TRUE)
+!if ($(TARGET) == DEBUG || $(RELEASE_LOGGING) == TRUE)
   TestPointCheckLib|$(PLATFORM_PACKAGE)/Test/Library/TestPointCheckLib/SmmTestPointCheckLib.inf
 !endif
 
@@ -408,12 +409,12 @@
   MdeModulePkg/Universal/StatusCodeHandler/Pei/StatusCodeHandlerPei.inf {
     <LibraryClasses>
       DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+      # Reverse-ranked priority list
+!if $(USE_MEMORY_LOGGING) == TRUE
+      SerialPortLib|MdeModulePkg/Library/PeiDxeSerialPortLibMem/PeiSerialPortLibMem.inf
+!endif
 !if $(USE_PEI_SPI_LOGGING) == TRUE
       SerialPortLib|$(PLATFORM_BOARD_PACKAGE)/Library/PeiSerialPortLibSpiFlash/PeiSerialPortLibSpiFlash.inf
-!else
-!if $(USE_MEMORY_LOGGING) == TRUE
-      SerialPortLib|MdeModulePkg/Library/PeiDxeSerialPortLibMem/PeiSerialPortLibMem.inf
-!endif
 !endif
 !if $(USE_HDMI_DEBUG_PORT) == TRUE
       SerialPortLib|$(PLATFORM_BOARD_PACKAGE)/Library/I2cHdmiDebugSerialPortLib/PeiI2cHdmiDebugSerialPortLib.inf
@@ -431,10 +432,14 @@
   IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf {
     <LibraryClasses>
       SiliconPolicyInitLib|$(PLATFORM_SI_PACKAGE)/Library/PeiSiliconPolicyInitLibDependency/PeiPreMemSiliconPolicyInitLibDependency.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
   }
   IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf {
     <LibraryClasses>
       SiliconPolicyInitLib|$(PLATFORM_SI_PACKAGE)/Library/PeiSiliconPolicyInitLibDependency/PeiPostMemSiliconPolicyInitLibDependency.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
   }
 !else
   #
@@ -444,6 +449,8 @@
   IntelFsp2WrapperPkg/FspmWrapperPeim/FspmWrapperPeim.inf {
     <LibraryClasses>
       SiliconPolicyInitLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyInitLibNull/SiliconPolicyInitLibNull.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
   }
   #
   # In FSP Dispatch mode the policy will be installed after FSP-S dispatched (only PrePolicy silicon-init executed).
@@ -452,6 +459,8 @@
   IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf {
     <LibraryClasses>
       SiliconPolicyInitLib|MinPlatformPkg/PlatformInit/Library/SiliconPolicyInitLibNull/SiliconPolicyInitLibNull.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
   }
 !endif
 
@@ -555,6 +564,7 @@
   MdeModulePkg/Universal/StatusCodeHandler/RuntimeDxe/StatusCodeHandlerRuntimeDxe.inf {
     <LibraryClasses>
       DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+      # Reverse-ranked priority list
 !if $(USE_MEMORY_LOGGING) == TRUE
       SerialPortLib|MdeModulePkg/Library/PeiDxeSerialPortLibMem/DxeSerialPortLibMem.inf
 !endif
@@ -563,12 +573,12 @@
 !endif
     <PcdsFixedAtBuild>
       gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|($(USE_MEMORY_LOGGING) || $(USE_HDMI_DEBUG_PORT))
-      gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|512
+      gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1024
   }
-  # TODO: Still requires a little more thought
   MdeModulePkg/Universal/StatusCodeHandler/Smm/StatusCodeHandlerSmm.inf {
     <LibraryClasses>
       DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
+      # Reverse-ranked priority list
 !if $(USE_MEMORY_LOGGING) == TRUE
       SerialPortLib|MdeModulePkg/Library/PeiDxeSerialPortLibMem/SmmSerialPortLibMem.inf
 !endif
@@ -577,8 +587,9 @@
 !endif
     <PcdsFixedAtBuild>
       gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeUseSerial|($(USE_MEMORY_LOGGING) || $(USE_HDMI_DEBUG_PORT))
-      gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|512
+      gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1024
   }
+# TODO: Add NvmExpressDxe if supporting Newgate and RayleighSLS
   MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.inf
   MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf
   MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkgPcd.dsc b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkgPcd.dsc
index 490c3ee6bf76..3991c6f17c44 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkgPcd.dsc
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkgPcd.dsc
@@ -13,9 +13,10 @@
 #
 ################################################################################
 
-# TODO: Harden and tune platform by PCDs
-# TODO: Consider removing PCDs declared by build report to be unused (but confirm first)
-# - Also, consider more "fixed" and more "dynamic"/"patchable"
+# TODO:
+# - Harden and tune platform by PCDs
+# - Consider removing PCDs declared by build report to be unused (but confirm first)
+#   - Also, consider more "fixed" and more "dynamic"/"patchable"
 
 [PcdsFixedAtBuild.common]
   ######################################
@@ -118,13 +119,14 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdPs2MouseExtendedVerification|FALSE  # TODO/TEST
   gEfiMdeModulePkgTokenSpaceGuid.PcdHiiOsRuntimeSupport|FALSE
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmEnableBspElection|FALSE
+  # TODO: Hook-up memory, SMM and SMI handler profiling
   gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmProfileEnable|FALSE
 
 # TODO: Prune this list to relevant features only
 !if gMinPlatformPkgTokenSpaceGuid.PcdBootStage >= 6
-  # FIXME: SMM path also PatchAndLoadAcpiTable()
-  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureEnable            |FALSE
-  # PcdIpmiFeatureEnable will not be enabled (no BMC)
+  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureEnable            |TRUE
+  gAcpiDebugFeaturePkgTokenSpaceGuid.PcdUseSmmVersion                     |FALSE
+# NOTE: PcdIpmiFeatureEnable will not be enabled (no BMC)
   # TODO: Can be build-time (user) choice
   gNetworkFeaturePkgTokenSpaceGuid.PcdNetworkFeatureEnable                |FALSE
   gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnable                          |TRUE
@@ -132,12 +134,9 @@
   gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable                  |TRUE
   # Requires actual hook-up
   gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable            |FALSE
-  # FIXME: (Similar) DXE module is duplicate?
-  gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable    |FALSE
-  # FIXME: Must BootLogoEnableLogo() to turn platform logo into boot logo
-  # - BGRT must be BMP, but this duplicates FSP logo. Can GetSectionFromAnyFv()?
-  gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable                      |FALSE
-  gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable                              |FALSE
+  # FIXME: Version2 not working - doesn't challenge for password
+  gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable    |TRUE
+  gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable                      |TRUE
 !endif
 
   ######################################
@@ -209,7 +208,7 @@
   # Board Configuration
   ######################################
   gKabylakeOpenBoardPkgTokenSpaceGuid.PcdMultiBoardSupport|FALSE
-  gKabylakeOpenBoardPkgTokenSpaceGuid.PcdTbtEnable|FALSE  # TODO: Enable if supporting Newgate
+  gKabylakeOpenBoardPkgTokenSpaceGuid.PcdTbtEnable|FALSE  # TODO: Enable if supporting Newgate and RayleighSLS
 
 [PcdsFixedAtBuild.common]
   ######################################
@@ -245,16 +244,29 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdBrowserSubtitleTextColor|0x0
   gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdFastPS2Detection|TRUE  # TODO/TEST
+!if FALSE  # FIXME: Causes DxeTestPointCheck ASSERT
+  # Guard DXE phase in non-stop mode, preferred over UAF detection (mutually exclusive)
+  # NOTE: SMM phase requires disabling PcdCpuSmmRestrictedMemoryAccess, so only enable to test
+  # TODO/TEST: Also test with guarded pool-head and with UAF detection feature
+  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x43
+#!else
+  # Guard DXE phase preferred over UAF detection (mutually exclusive)
+  # TODO: Consider performance impact on release builds
+  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x03
+!endif
+  # Protects loader, BS and RT code and data. TODO: Should not protect code and also ACPI memory?
+  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType|0x7E
+  gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0x7E
   gEfiMdeModulePkgTokenSpaceGuid.PcdHwErrStorageSize|0x00000800
   gEfiMdeModulePkgTokenSpaceGuid.PcdLoadModuleAtFixAddressEnable|$(TOP_MEMORY_ADDRESS)
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxHardwareErrorVariableSize|0x400
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8000
-!if $(TESTING) == TRUE
-  # Test with non-stop mode, so not disabling for loader.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask|0x43
+!if $(RELEASE_LOGGING) == TRUE
+  # Using non-stop mode, so not disabling for loader. NOTE/TEST: Reconsider use with SMM, which causes SMM profiling to be enabled
+  gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask|0x41
 !else
-  # FIXME: Can be broken for CSM. At this time, be permissive for loader.
-  gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask|0x83
+  # FIXME: At this time, be permissive for loader
+  gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask|0x81
 !endif
   gEfiMdeModulePkgTokenSpaceGuid.PcdReclaimVariableSpaceAtEndOfDxe|TRUE
   gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
@@ -269,10 +281,10 @@
   gEfiMdeModulePkgTokenSpaceGuid.PcdSerialUseHardwareFlowControl|FALSE
 !endif
 
-  # UPDs are updated at runtime, don't bother measuring
+  # Measure default UPDs, code to update UPDs is measured as well
   # BUGBUG: FSP-S measurement returns DEVICE_ERROR from PtpCrbTpmCommand() - Step 0.
   # - Similarly, Tcg2Dxe.c:Tpm2GetCapabilityManufactureID() - first command - fails?
-  gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig|0x00000006
+  gIntelFsp2WrapperTokenSpaceGuid.PcdFspMeasurementConfig|0x80000006
 
   gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoBarEnableMask|0x80
   gPcAtChipsetPkgTokenSpaceGuid.PcdAcpiIoPciBarRegisterOffset|0x40
@@ -435,6 +447,9 @@
   # @ValidRange 0x80000001 | 0 - 4
   gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|3
 
+  # Hypothetically, remove all but the trusted console input, but there's no callback
+  gEfiSecurityPkgTokenSpaceGuid.PcdUserPhysicalPresence|TRUE
+
 [PcdsFixedAtBuild.IA32]
   ######################################
   # Edk2 Configuration
@@ -522,7 +537,7 @@
   gKabylakeOpenBoardPkgTokenSpaceGuid.PcdLowPowerS0Idle|1
   gKabylakeOpenBoardPkgTokenSpaceGuid.PcdPciExpNative|1
 
-  # Thunderbolt Configuration (FIXME: Remove if not supporting Newgate)
+  # Thunderbolt Configuration (FIXME: Remove if not supporting Newgate and RayleighSLS)
   gKabylakeOpenBoardPkgTokenSpaceGuid.PcdDTbtAcDcSwitch|0x0
   gKabylakeOpenBoardPkgTokenSpaceGuid.PcdDTbtAcpiGpeSignature|0
   gKabylakeOpenBoardPkgTokenSpaceGuid.PcdDTbtAcpiGpeSignaturePorting|0
@@ -567,5 +582,5 @@
 !endif
 !if gMinPlatformPkgTokenSpaceGuid.PcdTpm2Enable == TRUE
   gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
-  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|4|NV,BS
 !endif
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [edk2-devel][edk2-platforms][PATCH v1 3/7] KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi: Improvements for EC ACPI
  2022-09-06 17:42 [edk2-devel][edk2-platforms][PATCH v1 0/7] Benjamin Doron
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 1/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Cleanup library includes Benjamin Doron
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 2/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Enhance the build-logic Benjamin Doron
@ 2022-09-06 17:42 ` Benjamin Doron
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 4/7] KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Resets notify EC Benjamin Doron
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Benjamin Doron @ 2022-09-06 17:42 UTC (permalink / raw)
  To: devel; +Cc: Sai Chaganty, Isaac Oram, Nate DeSimone, Chasel Chiu

Perform EC read and write via SystemIO when EmbeddedControl is
unavailable. Not properly tested yet.

Re-sync against coreboot port.

Does not yet handle SMM traps. In-progress.

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 .../Acpi/BoardAcpiTables.inf                  |   4 +
 .../AspireVn7Dash572G/Acpi/BoardSsdt.asl      |  29 +++-
 .../AspireVn7Dash572G/Acpi/battery.asl        |  11 +-
 .../AspireVn7Dash572G/Acpi/ec.asl             | 130 ++++++++--------
 .../AspireVn7Dash572G/Acpi/eclib.asl          | 141 ++++++++++++++++++
 .../AspireVn7Dash572G/Acpi/mainboard.asl      |   6 +-
 .../AspireVn7Dash572G/Acpi/thermal.asl        |  16 +-
 7 files changed, 256 insertions(+), 81 deletions(-)
 create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/eclib.asl

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/BoardAcpiTables.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/BoardAcpiTables.inf
index 806c0d2575c8..9a31b400a35e 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/BoardAcpiTables.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/BoardAcpiTables.inf
@@ -13,5 +13,9 @@
   MODULE_TYPE          = USER_DEFINED
   VERSION_STRING       = 1.0
 
+[Packages]
+  KabylakeOpenBoardPkg/OpenBoardPkg.dec
+  MdePkg/MdePkg.dec
+
 [Sources]
   BoardSsdt.asl
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/BoardSsdt.asl b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/BoardSsdt.asl
index cdec0434883e..d1609131ecf7 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/BoardSsdt.asl
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/BoardSsdt.asl
@@ -6,17 +6,44 @@
 
 **/
 
+#include <IndustryStandard/Acpi63.h>
+
 DefinitionBlock (
   "Board.aml",
   "SSDT",
-  0x02,
+  EFI_ACPI_6_3_DIFFERENTIATED_SYSTEM_DESCRIPTION_TABLE_REVISION,
   "ACRSKL",
   "AcerSKL ",
   0x20141018
   )
 {
+  External (SSMP, IntObj)
+  External (SMIF, IntObj)
   External (\MDBG, MethodObj)
 
+  // SW SMI data port
+  OperationRegion (DPRT, SystemIO, 0xB3, 1)
+  Field (DPRT, ByteAcc, Lock, Preserve)
+  {
+    SSDP, 8
+  }
+
+  Name (ESMI, 0xDD)  // TODO: Patch at runtime
+  Method (TRPS, 3, Serialized)
+  {
+    \DBGH (Concatenate ("SMIF: ", ToHexString (Arg0)))
+    \DBGH (Concatenate ("Param0: ", ToHexString (Arg1)))
+    \DBGH (Concatenate ("Param1: ", ToHexString (Arg2)))
+
+    Local0 = Arg1
+    Local0 |= (Arg2 << 4)
+    \DBGH (Concatenate ("Local0: ", ToHexString (Local0)))
+
+    SMIF = Arg0
+    SSDP = Local0
+    SSMP = ESMI
+  }
+
   // Debug print helper
   Method (DBGH, 1)
   {
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/battery.asl b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/battery.asl
index 5ae4bdca89d5..1bf652c4a01e 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/battery.asl
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/battery.asl
@@ -184,11 +184,17 @@ Method (GBIF, 3, NotSerialized)
     Local7 = EBSN
 #endif
     Name (SERN, Buffer (0x06) { "     " })
+    /* Convert hex to decimal.
+     * - There appears to be a bug in the vendor's implementation:
+     *   The correct answer has, or can have, 5 digits, so Local6 = 5.
+     *   Also see "SERN" buffer.
+     * - Userspace prints reversed serial number?
+     */
     Local6 = 4
     While (Local7)
     {
       Divide (Local7, 10, Local5, Local7)
-      SERN[Local6] = (Local5 + 0x30)  // Add ASCII 0x30 to get character
+      SERN[Local6] = (Local5 + 0x30)  // Add 0x30 to get numeric character
       Local6--
     }
 
@@ -310,7 +316,7 @@ Method (GBST, 4, NotSerialized)  // All on one page
     If (Arg2)
     {
       Local1 *= Local3
-      Local1 /= 1000  /* Remainder ignored */
+      Local1 /= 1000  /* Remainder ignored by vendor */
     }
   }
   Else
@@ -382,6 +388,7 @@ Device (BAT0)
 
   Method (_BIF, 0, NotSerialized)  // _BIF: Battery Information
   {
+    /* Bitwise AND by vendor is lossy? */
     Local6 = B0ST
     Local7 = 20
     While (Local6 && Local7)
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/ec.asl b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/ec.asl
index df71dd69b491..b8f0dba1597f 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/ec.asl
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/ec.asl
@@ -5,18 +5,33 @@
 
 **/
 
-/* Global TODO: (externally: Optimus GC6 and GPS)
+/*
+ * Global TODO: (externally: Optimus GC6 and GPS)
  * - TRPS: This is SMI 0xDD, likely in SmmOemDriver. This SW SMI adds to and executes
  *         a table of function pointers produced throughout the OEM 'value-add' stack.
+ *         - Arg0 - "SFUN" - is index into "$FNC" pointer table? It's easier to
+ *           correlate *CommonService use: Offset 13 creates TRPS handlers.
+ *         - Known functions:
+ *           - 0x80 calls offset 0 in ACER_BOOT_DEVICE_SERVICE_PROTOCOL_GUID.
+ *             - NB: efiXplorer can miss InstallProtocolInterface() when Interface is local
+ *           - 0x81 toggles Intel Dynamic Acceleration in IA32_MISC_ENABLE MSR.
+ *           - 0x82 does switch on "OSYS" to set EC byte. Suspect this is for OS features.
+ *         (A CVE exists in the vendor code only if it never sets the offset in the buffer.)
+ * - RBEC/WBEC/MBEC: This is SMI 0xDD, "functions" 0x10, 0x11 and 0x12 in SmmKbcDriver,
+ *        added into SmmCommonService table at its protocol notify. Performs read, write
+ *        and read-modify-write from buffer. We will use ACPI instead.
  * - WMI: This is likely SMI 0xD0 in A01WMISmmCallback. This SW SMI likely uses the WMI
  *        object and consumes the OEM 'value-add' stack for EC and presumably the A01*
- *        OEM/ODM 'value-add' stack.
+ *        OEM/ODM 'value-add' stack. An SSDT contains the device and EC0 provides "GCMS"
+ *        and "GOTS" method helpers.
  *
- * Generally, more reversing is needed.
+ * Generally, more information is needed.
+ * TODO: Restore stripped features using reference code (except, check BoardAcpiDxe first)
+ *       and implement more board features: lid and touchpad trigger wake from S3,
+ *       Fn-Ctrl swap, sticky Fn keys and always-on USB charger.
  */
-/* TODO: Implement more features around reference code (except, check BoardAcpiDxe first) */
 
-// TODO: Enable and test
+// TODO/TEST
 #undef LGMR_ENABLED
 
 // "DIDX" - "DeviceIdX" is uninitialised, cannot use "BRTN" method yet
@@ -40,17 +55,7 @@ Device (\_SB.PCI0.LPCB.EC0)
     IO (Decode16, 0x66, 0x66, 0, 1)
   })
 
-  OperationRegion (ECO1, SystemIO, 0x62, 1)
-  Field (ECO1, ByteAcc, Lock, Preserve)
-  {
-    PX62, 8
-  }
-
-  OperationRegion (ECO2, SystemIO, 0x66, 1)
-  Field (ECO2, ByteAcc, Lock, Preserve)
-  {
-    PX66, 8
-  }
+  #include "eclib.asl"
 
 #ifdef LGMR_ENABLED
   OperationRegion (ECMB, SystemMemory, LGMR, 0x200)
@@ -84,10 +89,12 @@ Device (\_SB.PCI0.LPCB.EC0)
     EX3G, 1,        /* 3G */
         , 3,
     RFEX, 1,        /* RF present */
-#if 0  // Merely a guess
-    Offset (0x55),
-    BTH0, 8,        /* Battery threshold? TODO: Actually diff in modified vendor FW */
-#endif
+/*
+ * NOTE: Some reverse engineering, as well as corroborating vendor's hidden SetupUtility
+ * options with the EC's memory space, suggests that offset 0x55 might be the battery
+ * threshold
+ * - TODO: Actually diff changes in modified vendor FW
+ */
     Offset (0x57),
         , 7,
     AHKB, 1,        /* Hotkey triggered */
@@ -204,14 +211,19 @@ Device (\_SB.PCI0.LPCB.EC0)
       {
         TINI ()
         EOSS = 0x05
-//      OSIN ()
+        // OSYS retrieved by SMM, Arg3 is unused
+//      TRPS (0x82, 1, 0)
 
-        /* Other pages return valid data too, but this seems to be the page
+        /*
+         * Other pages return valid data too, but this seems to be the page
          * we are expecting - persistently in ectool dump with vendor firmware
-         * FIXME: Contents of other pages? */
+         * FIXME: Contents of other pages?
+         */
         ETID = 0x20
       }
     }
+
+    /* iGFX RC method call stripped */
   }
 
   Method (TINI, 0, NotSerialized)
@@ -223,10 +235,8 @@ Device (\_SB.PCI0.LPCB.EC0)
     }
     Else
     {
-      /* WBEC: Called SMI function 0x11 */
-//    EC_WRITE (0x92, 0)  // ETAF = 0
-      /* MBEC: Called SMI function 0x12 */
-//    MBEC (0x10, 0xFD, 0x02)  // ETEE = 1
+      WBEC (0x92, 0)  // ETAF = 0
+      MBEC (0x10, 0xFD, 0x02)  // ETEE = 1
     }
   }
 
@@ -234,10 +244,8 @@ Device (\_SB.PCI0.LPCB.EC0)
   Method (ECPS, 1, NotSerialized)  // _PTS: Prepare To Sleep
   {
     ECSS = Arg0
-//  COSI = OSYS
-//  SPR1 = Arg0
-    /* TRPS: Generic SMI trap handler */
-//  TRPS (0x82, 0x02)
+    // OSYS retrieved by SMM
+//  TRPS (0x82, 0x02, Arg0)
     If ((Arg0 == 3) || (Arg0 == 4))
     {
       RFST = RFEX
@@ -250,38 +258,23 @@ Device (\_SB.PCI0.LPCB.EC0)
     EOSS = Arg0
     TINI ()
     Notify (BAT0, 0x81) // Information Change
-//  COSI = OSYS
-//  SPR1 = Arg0
-    /* TRPS: Generic SMI trap handler */
-//  TRPS (0x82, 0x03)
+    // OSYS retrieved by SMM
+//  TRPS (0x82, 0x03, Arg0)
     If ((Arg0 == 3) || (Arg0 == 4))
     {
       RFEX = RFST
       Notify (SLPB, 0x02) // Device Wake
     }
+    /* iGFX RC method call stripped */
   }
 
-#if 0  // TODO: Figure out what this is for
-  Method (OSIN, 0, NotSerialized)
+  Method (MBEC, 3, Serialized)
   {
-    COSI = OSYS
-    /* TRPS: Generic SMI trap handler */
-    TRPS (0x82, 1)
-  }
-#endif
-
-#if 0  // TODO: Implement
-  Method (MBEC, 3, Serialized)  // Read-Modify-Write
-  {
-    /* Based on similar methods/tables at
-     * https://github.com/linuxhw/ACPI/blob/master/Notebook/Sony/SVE1713/SVE1713S1RW/506CDC50E671#L9359
-     * which use ASL instead of SMM calls */
-    Local0 = EC_READ (Arg0)
+    Local0 = RBEC (Arg0)
     Local0 &= Arg1
     Local0 |= Arg2
-    EC_WRITE (Arg0, Local0)
+    WBEC (Arg0, Local0)
   }
-#endif
 
   /* Graphical hotkey */
   Method (_Q19, 0, NotSerialized)
@@ -316,7 +309,8 @@ Device (\_SB.PCI0.LPCB.EC0)
         }
         ElseIf ((Local1 > 0x80) && (Local1 < 0xA0))
         {
-          TPEN ^= 1  /* TODO: Not working. What else does WMI do here? */
+          /* TODO: Not working when called by HID mode. What does WMI do here? */
+          TPEN ^= 1
         }
       }
     }
@@ -330,8 +324,7 @@ Device (\_SB.PCI0.LPCB.EC0)
     }
     Else
     {
-      /* MBEC: Called SMI function 0x12 */
-//    MBEC (0x92, 0xF7, 0x08)  // EOSD = 1
+      MBEC (0x92, 0xF7, 0x08)  // EOSD = 1
     }
 
     Sleep (500)
@@ -341,9 +334,8 @@ Device (\_SB.PCI0.LPCB.EC0)
 
   Method (_Q3F, 0, NotSerialized)
   {
-    \DBGH ("EC Query: 0x3F - TRPS")
-    /* TRPS: Generic SMI trap handler */
-//  TRPS (0x80, 0)
+    // Arg3 is unused
+//  TRPS (0x80, 0, 0)
   }
 
   Method (_Q40, 0, NotSerialized)
@@ -397,46 +389,44 @@ Device (\_SB.PCI0.LPCB.EC0)
 
   Method (_Q60, 0, NotSerialized)
   {
-    \DBGH ("EC Query: 0x60 -> WMI")
+    \DBGH ("EC Query (0x60): WMI")
   }
 
   Method (_Q61, 0, NotSerialized)
   {
-    \DBGH ("EC Query: 0x61 -> WMI")
+    \DBGH ("EC Query (0x61): WMI")
   }
 
   Method (_Q62, 0, NotSerialized)
   {
-    \DBGH ("EC Query: 0x62 -> Optimus GC6")
+    \DBGH ("EC Query (0x62): Optimus GC6 or NVIDIA GPS")
   }
 
   Method (_Q63, 0, NotSerialized)
   {
-    \DBGH ("EC Query: 0x63 -> Optimus GC6")
+    \DBGH ("EC Query (0x63): Optimus GC6 or NVIDIA GPS")
   }
 
   Method (_Q67, 0, NotSerialized)
   {
-    \DBGH ("EC Query: 0x67 -> NVIDIA GPS")
+    \DBGH ("EC Query (0x67): NVIDIA GPS")
   }
 
   Method (_Q68, 0, NotSerialized)
   {
-    \DBGH ("EC Query: 0x68 -> NVIDIA GPS")
+    \DBGH ("EC Query (0x68): NVIDIA GPS")
   }
 
   Method (_Q6C, 0, NotSerialized)
   {
-    \DBGH ("EC Query: 0x6C - TRPS")
-    /* TRPS: Generic SMI trap handler */
-//  TRPS (0x81, 0)
+    // Arg3 is unused
+//  TRPS (0x81, 0, 0)
   }
 
   Method (_Q6D, 0, NotSerialized)
   {
-    \DBGH ("EC Query: 0x6D - TRPS")
-    /* TRPS: Generic SMI trap handler */
-//  TRPS (0x81, 1)
+    // Arg3 is unused
+//  TRPS (0x81, 1, 0)
   }
 
   #include "ac.asl"
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/eclib.asl b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/eclib.asl
new file mode 100644
index 000000000000..57921eb09c8b
--- /dev/null
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/eclib.asl
@@ -0,0 +1,141 @@
+/** @file
+  This file contains the EC library for ACPI.
+
+  Copyright (c) 2021, Baruch Binyamin Doron
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <EcCommands.h>
+
+Mutex (EMTX, 0)
+
+OperationRegion (ECO1, SystemIO, EC_D_PORT, 1)
+Field (ECO1, ByteAcc, Lock, Preserve)
+{
+  ECDT, 8
+}
+
+OperationRegion (ECO2, SystemIO, EC_C_PORT, 1)
+Field (ECO2, ByteAcc, Lock, Preserve)
+{
+  ECSC, 8
+}
+
+// Send EC command
+// Return time elapsed on success, EC_TIME_OUT on failure
+Method (SECC, 1, Serialized)
+{
+  // Wait EC timeout for input buffer to be empty
+  Local0 = 0
+  While ((ECSC & EC_S_IBF) && Local0 < EC_TIME_OUT)
+  {
+    Stall (1)
+    Local0++
+  }
+  // Send command if timeout was not reached
+  If (Local0 < EC_TIME_OUT)
+  {
+    ECSC = Arg0
+  }
+
+  // Return status
+  Return (Local0)
+}
+
+// Send EC data
+// Return time elapsed on success, EC_TIME_OUT on failure
+Method (SECD, 1, Serialized)
+{
+  // Wait EC timeout for input buffer to be empty
+  Local0 = 0
+  While ((ECSC & EC_S_IBF) && Local0 < EC_TIME_OUT)
+  {
+    Stall (1)
+    Local0++
+  }
+  // Send data if timeout was not reached
+  If (Local0 < EC_TIME_OUT)
+  {
+    ECDT = Arg0
+  }
+
+  // Return status
+  Return (Local0)
+}
+
+// Receive EC data
+// Return data on success, EC_TIME_OUT on failure
+Method (RECD, 0, Serialized)
+{
+  // Wait EC timeout for input buffer to be empty
+  Local0 = 0
+  While ((ECSC & EC_S_OBF) && Local0 < EC_TIME_OUT)
+  {
+    Stall (1)
+    Local0++
+  }
+  // Return data
+  If (Local0 < EC_TIME_OUT)
+  {
+    Return (ECDT)
+  }
+
+  // Timeout exceeded, return failure
+  Return (Local0)
+}
+
+// Read EC byte
+// Return data on success, EC_TIME_OUT on failure
+Method (RBEC, 1, Serialized)
+{
+  // Check for mutex acquired to not run with another function
+  Local0 = Acquire (EMTX, 0xFFFF)
+  If (Local0 == 0)
+  {
+    Local0 = SECC (EC_C_ACPI_READ)
+  }
+
+  If (Local0 < EC_TIME_OUT)
+  {
+    // Send address
+    Local0 = SECD (Arg0)
+  }
+
+  If (Local0 < EC_TIME_OUT)
+  {
+    // Receive data
+    Local0 = RECD ()
+  }
+
+  Release (EMTX)
+  Return (Local0)
+}
+
+// Write EC byte
+// Return time elapsed on success, EC_TIME_OUT on failure
+Method (WBEC, 2, Serialized)
+{
+  // Check for mutex acquired to not run with another function
+  Local0 = Acquire (EMTX, 0xFFFF)
+  If (Local0 == 0)
+  {
+    Local0 = SECC (EC_C_ACPI_READ)
+  }
+
+  If (Local0 < EC_TIME_OUT)
+  {
+    // Send address
+    Local0 = SECD (Arg0)
+  }
+
+  If (Local0 < EC_TIME_OUT)
+  {
+    // Send data
+    Local0 = SECD (Arg1)
+  }
+
+  Release (EMTX)
+  // Return data
+  Return (Local0)
+}
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/mainboard.asl b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/mainboard.asl
index 7a73d37429d0..46edce92fb47 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/mainboard.asl
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/mainboard.asl
@@ -6,25 +6,27 @@
 **/
 
 // TODO: Add HID support for touchpad, etc.
+// - Does board actually support DPTF?
 #include "thermal.asl"
 
 External (\_SB.SLPB, DeviceObj)
 
-// TODO: Need hooks from BoardAcpiDxe
-
 Scope (_SB)
 {
   Method (MPTS, 1, NotSerialized)  // _PTS: Prepare To Sleep
   {
     ^PCI0.LPCB.EC0.ECPS (Arg0)
+    /* TBT and DTS not supported, TPM.PTS can be called elsewhere */
   }
 
   Method (MWAK, 1, Serialized)  // _WAK: Wake
   {
     ^PCI0.LPCB.EC0.ECWK (Arg0)
+    /* No GPIO expander, 8254 clock-gating and PCIe PME can be performed elsewhere */
 
     If ((Arg0 == 3) || (Arg0 == 4))
     {
+      /* DTS and TBT not supported, iGFX RC variable update stripped */
       Notify (LID0, 0x80) // Status Change
     }
   }
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/thermal.asl b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/thermal.asl
index 805ee0700cd0..5d6604b41a9f 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/thermal.asl
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi/thermal.asl
@@ -15,12 +15,16 @@ Scope (_TZ)
     {
 #ifdef LGMR_ENABLED
       Local0 = \_SB.PCI0.LPCB.EC0.MS0T
-//    Local1 = \_SB.PCI0.LPCB.EC0.MCSS
+      Local1 = \_SB.PCI0.LPCB.EC0.MCSS
+      /* Suppress warning over reading status flag by dummy OR */
+      Or (Local1, 1, Local1)
       Local2 = \_SB.PCI0.LPCB.EC0.MOSD
 #else
       Local0 = \_SB.PCI0.LPCB.EC0.ES0T
-//    Local1 = \_SB.PCI0.LPCB.EC0.ESSF  // "MCSS": Considering neighbouring bits, likely
-                                        // "ESSF" in thermals, not "ECSS" in notify
+      /* "MCSS": Considering neighbouring bits, likely
+       * "ESSF" in thermals, not "ECSS" in power notifications */
+      Local1 = \_SB.PCI0.LPCB.EC0.ESSF
+      Or (Local1, 1, Local1)
       Local2 = \_SB.PCI0.LPCB.EC0.EOSD
 #endif
       If (Local2)  // Thermal trip
@@ -59,7 +63,7 @@ Scope (_TZ)
       Else
       {
         /* MBEC: Called SMI function 0x12 */
-//      \_SB.PCI0.LPCB.EC0.MBEC (0x90, 0xFE, Arg0)  // SCPM = Arg0
+        \_SB.PCI0.LPCB.EC0.MBEC (0x90, 0xFE, Arg0)  // SCPM = Arg0
       }
     }
 
@@ -89,6 +93,7 @@ Scope (_TZ)
 #else
       Local0 = \_SB.PCI0.LPCB.EC0.ES1T
 #endif
+
       Return (C2K (Local0))
     }
 
@@ -116,7 +121,6 @@ Scope (_TZ)
       Local0 = 30
     }
 
-    Local0 = ((Local0 * 10) + 2732)  // Celsius to Kelvin
-    Return (Local0)
+    Return ((Local0 * 10) + 2732)  // Celsius to centi-Kelvin
   }
 }
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [edk2-devel][edk2-platforms][PATCH v1 4/7] KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Resets notify EC
  2022-09-06 17:42 [edk2-devel][edk2-platforms][PATCH v1 0/7] Benjamin Doron
                   ` (2 preceding siblings ...)
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 3/7] KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi: Improvements for EC ACPI Benjamin Doron
@ 2022-09-06 17:42 ` Benjamin Doron
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 5/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Use Setup to control security Benjamin Doron
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 9+ messages in thread
From: Benjamin Doron @ 2022-09-06 17:42 UTC (permalink / raw)
  To: devel; +Cc: Sai Chaganty, Isaac Oram, Chasel Chiu, Nate DeSimone

Add a callback to notify the EC of platform resets.

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 .../Library/BoardInitLib/DxeBoardInitLib.c    | 92 ++++++++++++++++++-
 .../Library/BoardInitLib/DxeBoardInitLib.inf  |  4 +
 2 files changed, 91 insertions(+), 5 deletions(-)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c
index 5c5c26d85c58..07278d956ddc 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c
@@ -8,17 +8,22 @@
 **/
 
 #include <PiDxe.h>
-#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Library/BoardEcLib.h>
 #include <Library/BoardInitLib.h>
 #include <Library/DebugLib.h>
 #include <Library/EcLib.h>
-#include <Library/BoardEcLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Protocol/ResetNotification.h>
+
+EFI_RESET_NOTIFICATION_PROTOCOL  *mResetNotify = NULL;
 
 /**
-  Update the EC's clock?
+  Update the EC's clock.
 
 **/
 VOID
+EFIAPI
 EcSendTime (
   VOID
   )
@@ -26,11 +31,13 @@ EcSendTime (
   EFI_STATUS  Status;
   EFI_TIME    EfiTime;
   // Time could be negative (before 2016)
-  INTN        EcTime;
+  INT32       EcTime;
   UINT8       EcTimeByte;
   INTN        Index;
   UINT8       EcResponse;
 
+  DEBUG ((DEBUG_INFO, "%a() Starts\n", __FUNCTION__));
+
   Status = gRT->GetTime (&EfiTime, NULL);
   if (EFI_ERROR (Status)) {
     DEBUG ((DEBUG_INFO, "Failed to retrieve current time\n"));
@@ -56,25 +63,72 @@ EcSendTime (
   if (!EFI_ERROR (Status)) {
     DEBUG ((DEBUG_INFO, "EC: response 0x%x\n", EcResponse));
   }
+
+  DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
 }
 
 /**
-  Configure EC
+  Process an EC time request.
 
 **/
 VOID
+EFIAPI
 EcRequestsTime (
   VOID
   )
 {
   UINT8           Dat;
 
+  DEBUG ((DEBUG_INFO, "%a() Starts\n", __FUNCTION__));
+
   /* This is executed as protocol notify in vendor's RtKbcDriver when *CommonService
    * protocol is installed. Effectively, this code could execute from the entrypoint */
   EcCmd90Read (0x79, &Dat);
   if (Dat & BIT0) {
     EcSendTime ();
   }
+
+  DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
+}
+
+/**
+  Notify EC of reset events.
+
+  @param[in] ResetType    The type of reset to perform.
+  @param[in] ResetStatus  The status code for the reset.
+  @param[in] DataSize     The size, in bytes, of ResetData.
+  @param[in] ResetData    For a ResetType of EfiResetCold, EfiResetWarm, or EfiResetShutdown
+                          the data buffer starts with a Null-terminated string, optionally
+                          followed by additional binary data. The string is a description
+                          that the caller may use to further indicate the reason for the
+                          system reset. For a ResetType of EfiResetPlatformSpecific the data
+                          buffer also starts with a Null-terminated string that is followed
+                          by an EFI_GUID that describes the specific type of reset to
+                          perform.
+
+**/
+VOID
+EFIAPI
+EcResetSystemHook (
+  IN EFI_RESET_TYPE           ResetType,
+  IN EFI_STATUS               ResetStatus,
+  IN UINTN                    DataSize,
+  IN VOID                     *ResetData OPTIONAL
+  )
+{
+  // If boolean PCD tokens 0xBD, 0xBE and 0xBF are set in vendor FW,
+  // OEM also sends command 0x5A with argument 0xAA via ACPI "CMDB" method and stalls for
+  // 100000, then sets ResetType to EfiResetShutdown.
+  // PCD token 0xBF may be set in a separate function of DxeOemDriver if
+  // some bits of EC RAM offset 0x5E are set.
+  // TODO: More information is needed
+  if (ResetType == EfiResetShutdown) {
+    EcCmd91Write (0x76, 7);  // "ECSS" register
+    // TODO: Write twice, like OEM?
+    EcCmd91Write (0x76, 7);  // "ECSS" register
+    // Now OEM calls function offset 2 in ACER_BOOT_DEVICE_SERVICE_PROTOCOL_GUID.
+    // TODO: What does this do?
+  }
 }
 
 /**
@@ -89,7 +143,23 @@ BoardInitAfterPciEnumeration (
   VOID
   )
 {
+  EFI_STATUS                       Status;
+
+  DEBUG ((DEBUG_INFO, "%a() Starts\n", __FUNCTION__));
+
+  // Send EC the present time, if requested
   EcRequestsTime ();
+
+  // Add a callback to gRT->ResetSystem() to notify EC, rather than hooking the table,
+  // (as vendor's DxeOemDriver does)
+  Status = gBS->LocateProtocol (&gEfiResetNotificationProtocolGuid, NULL, (VOID **) &mResetNotify);
+  if (!EFI_ERROR (Status)) {
+    Status = mResetNotify->RegisterResetNotify (mResetNotify, EcResetSystemHook);
+    ASSERT_EFI_ERROR (Status);
+    DEBUG ((DEBUG_INFO, "EC: Added callback to notify EC of resets\n"));
+  }
+
+  DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
 
@@ -120,5 +190,17 @@ BoardInitEndOfFirmware (
   VOID
   )
 {
+  EFI_STATUS                       Status;
+
+  DEBUG ((DEBUG_INFO, "%a() Starts\n", __FUNCTION__));
+
+  // Remove ResetSystem callback. ACPI will be notifying EC of events
+  if (mResetNotify != NULL) {
+    Status = mResetNotify->UnregisterResetNotify (mResetNotify, EcResetSystemHook);
+    ASSERT_EFI_ERROR (Status);
+    DEBUG ((DEBUG_INFO, "EC: Removed callback to notify EC of resets\n"));
+  }
+
+  DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.inf
index 9a868ee15fb2..24747fa7b224 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.inf
@@ -15,6 +15,7 @@
   LIBRARY_CLASS                  = BoardInitLib
 
 [LibraryClasses]
+  UefiBootServicesTableLib
   UefiRuntimeServicesTableLib
   DebugLib
   EcLib
@@ -27,3 +28,6 @@
 
 [Sources]
   DxeBoardInitLib.c
+
+[Protocols]
+  gEfiResetNotificationProtocolGuid  ## CONSUMES
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [edk2-devel][edk2-platforms][PATCH v1 5/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Use Setup to control security
  2022-09-06 17:42 [edk2-devel][edk2-platforms][PATCH v1 0/7] Benjamin Doron
                   ` (3 preceding siblings ...)
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 4/7] KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Resets notify EC Benjamin Doron
@ 2022-09-06 17:42 ` Benjamin Doron
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 6/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Improve board detection Benjamin Doron
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 7/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Align DEBUG() use Benjamin Doron
  6 siblings, 0 replies; 9+ messages in thread
From: Benjamin Doron @ 2022-09-06 17:42 UTC (permalink / raw)
  To: devel; +Cc: Sai Chaganty, Isaac Oram, Chasel Chiu, Nate DeSimone

Add a HII form to Setup for controlling lockdown UPDs. Default to
strict security, allowing it to be lifted for the user's convenience.

This is not board-specific, and could be ported to other boards. To add
more entries to the HII form, modify the VFR, VFR strings, variable
structure and consume the variable in the appropriate place.

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 .../PeiBoardPolicyUpdate.c                    |  56 ++-
 .../PeiSiliconPolicyUpdateLibFsp.inf          |   7 +-
 .../Include/BoardConfigNvData.h               |  37 ++
 .../Library/BoardInitLib/BoardConfigVfr.vfr   |  68 ++++
 .../BoardInitLib/BoardConfigVfrStrings.uni    |  29 ++
 .../Library/BoardInitLib/DxeBoardConfigHii.c  | 374 ++++++++++++++++++
 .../Library/BoardInitLib/DxeBoardInitLib.c    |  21 +-
 .../Library/BoardInitLib/DxeBoardInitLib.h    | 131 ++++++
 .../Library/BoardInitLib/DxeBoardInitLib.inf  |  10 +
 9 files changed, 712 insertions(+), 21 deletions(-)
 create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/BoardConfigNvData.h
 create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/BoardConfigVfr.vfr
 create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/BoardConfigVfrStrings.uni
 create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardConfigHii.c
 create mode 100644 Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.h

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiBoardPolicyUpdate.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiBoardPolicyUpdate.c
index 452c961b17ac..425deb4d16c0 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiBoardPolicyUpdate.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiBoardPolicyUpdate.c
@@ -6,11 +6,13 @@
 
 **/
 
+#include "PeiSaPolicyUpdate.h"
 #include "PeiPchPolicyUpdate.h"
 #include <Library/BaseMemoryLib.h>
-#include <Library/DebugLib.h>
 #include <Library/PcdLib.h>
-#include <PchPolicyCommon.h>
+#include <Library/PeiServicesLib.h>
+#include <Ppi/ReadOnlyVariable2.h>
+#include <BoardConfigNvData.h>
 
 /*
  * TODO:
@@ -54,8 +56,6 @@ PeiFspBoardPolicyUpdatePreMem (
   DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
 
   // BUGBUG: Preserve FSP defaults - PeiSiliconPolicyInitLibFsp ultimately overrides to 0.
-  // Drop when https://edk2.groups.io/g/devel/message/79391 is merged
-  FspmUpd->FspmConfig.PeciC10Reset = 1;
   FspmUpd->FspmConfig.RefClk = 1;  // Maybe "auto" is safe, but that isn't the FSP default
 
   // TODO: Why should this be here?
@@ -92,18 +92,43 @@ PeiFspBoardPolicyUpdate (
   IN OUT FSPS_UPD    *FspsUpd
   )
 {
-  INTN  Index;
+  EFI_STATUS                       Status;
+  EFI_PEI_READ_ONLY_VARIABLE2_PPI  *VariablePpi;
+  UINTN                            DataSize;
+  EFI_GUID                         BoardConfigFormsetGuid = BOARD_CONFIG_FORMSET_GUID;
+  BOARD_CONFIGURATION              BoardConfig;
+  INTN                             Index;
 
   DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
 
-  // FIXME/NB: This is insecure and not production-ready!
-  // TODO: Configure SPI lockdown by variable on FrontPage?
-  // - Later, also configure stronger protection: PRRs
-  FspsUpd->FspsConfig.PchLockDownBiosLock = 0;  // Default. Will enable, not remove
-  FspsUpd->FspsConfig.PchLockDownSpiEiss = 0;
-  // This may be PWRM+0x18[BIT22], causing HSTI "PCH Security Configuration -  Reserved Check failure"
-  // I think the intel_pmc_core kernel module requires this to populate debugfs?
-  FspsUpd->FspsTestConfig.PchPmPmcReadDisable = 0;
+  // Use variable services directly, to avoid casting reference to pointer into struct
+  // from PeiGetVariable()
+  Status = PeiServicesLocatePpi (&gEfiPeiReadOnlyVariable2PpiGuid, 0, NULL, (VOID **) &VariablePpi);
+  ASSERT_EFI_ERROR (Status);
+
+  DataSize = sizeof (BoardConfig);
+  Status = VariablePpi->GetVariable (
+                          VariablePpi,
+                          BOARD_CONFIG_NV_NAME,
+                          &BoardConfigFormsetGuid,
+                          NULL,
+                          &DataSize,
+                          &BoardConfig
+                          );
+  // TODO: Also configure stronger protection: PRRs
+  // - Also, we could lift lockdown here for BOOT_ON_FLASH_UPDATE.
+  //   User must do this themselves for flashrom
+  if (!EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_INFO, "BoardConfig: Set FSP UPDs from variable\n"));
+    FspsUpd->FspsConfig.PchLockDownBiosLock = BoardConfig.LockDownBiosLock;
+    FspsUpd->FspsConfig.PchLockDownSpiEiss = BoardConfig.LockDownBiosLock;
+    FspsUpd->FspsTestConfig.PchPmPmcReadDisable = BoardConfig.LockDownPmcReadDisable;
+  } else {
+    DEBUG ((DEBUG_INFO, "BoardConfig: Set FSP UPDs to secure default\n"));
+    FspsUpd->FspsConfig.PchLockDownBiosLock = 1;  // FSP default not secure
+    FspsUpd->FspsConfig.PchLockDownSpiEiss = 1;
+    FspsUpd->FspsTestConfig.PchPmPmcReadDisable = 1;
+  }
 
   // BUGBUG: Preserve FSP defaults - Pei*PolicyLib ultimately overrides
   // Requires HW support?
@@ -116,7 +141,7 @@ PeiFspBoardPolicyUpdate (
   FspsUpd->FspsConfig.SerialIoDevMode[0] = 2;
   FspsUpd->FspsConfig.SerialIoDevMode[1] = 2;
 
-  // Acer IDs (TODO: "Newgate" IDs)
+  // Acer IDs (TODO: "Newgate" and "RayleighSLS" IDs)
   FspsUpd->FspsConfig.DefaultSvid = 0x1025;
   FspsUpd->FspsConfig.DefaultSid = 0x1037;
   FspsUpd->FspsConfig.PchSubSystemVendorId = 0x1025;
@@ -265,7 +290,7 @@ PeiFspBoardPolicyUpdate (
   FspsUpd->FspsConfig.PcieRpAspm[9] = PchPcieAspmL1;
 
   /* SCS config */
-  // Although platform NVS area shows this enabled, the SD card reader is connected over USB, not SCS
+  // Although vendor's platform NVS area shows this is enabled, the SD card reader is connected over USB, not SCS
   FspsUpd->FspsConfig.ScsEmmcEnabled = 0;
   FspsUpd->FspsConfig.ScsSdCardEnabled = 0;
 
@@ -275,7 +300,6 @@ PeiFspBoardPolicyUpdate (
   FspsUpd->FspsConfig.PchSirqMode = PchContinuousMode;
 
   /* HDA config */
-  // FIXME: DspEnable is set, per PeiPchPolicyLib, however it is disabled in the HOB produced by FSP
   // Returned to DXE as HOB, used to select blob for NHLT
   // - FIXME: 1ch array DMIC may not be supported by the Linux driver
   FspsUpd->FspsConfig.PchHdaDspEndpointDmic = PchHdaDmic1chArray;
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf
index 0e1b42c20cd8..ac7e4f65f912 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf
@@ -75,6 +75,7 @@
   SiPolicyLib
   MemoryAllocationLib
   PeiLib
+  PeiServicesLib
 
 [Pcd]
   gSiPkgTokenSpaceGuid.PcdTsegSize                              ## CONSUMES
@@ -135,10 +136,14 @@
   gIntelFsp2WrapperTokenSpaceGuid.PcdFspmUpdDataAddress
   gKabylakeOpenBoardPkgTokenSpaceGuid.PcdRootPort4ClkInfo
 
+[Ppis]
+  gEfiPeiReadOnlyVariable2PpiGuid               ## CONSUMES
+
 [Guids]
   gFspNvsBufferVariableGuid                     ## CONSUMES
   gTianoLogoGuid                                ## CONSUMES
   gEfiMemoryOverwriteControlDataGuid
 
 [Depex]
-  gEdkiiVTdInfoPpiGuid
+  gEdkiiVTdInfoPpiGuid AND
+  gEfiPeiReadOnlyVariable2PpiGuid
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/BoardConfigNvData.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/BoardConfigNvData.h
new file mode 100644
index 000000000000..feaa324eaea4
--- /dev/null
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/BoardConfigNvData.h
@@ -0,0 +1,37 @@
+/** @file
+  Header file for NV data structure definition.
+
+Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2021, Baruch Binyamin Doron
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef __BOARD_CONFIG_NV_DATA_H__
+#define __BOARD_CONFIG_NV_DATA_H__
+
+#define BOARD_CONFIG_FORMSET_GUID \
+  { \
+    0x6E38A4A7, 0xB6B7, 0x41E0, { 0xA6, 0xF3, 0x41, 0x35, 0x72, 0xDF, 0x88, 0x2F } \
+  }
+
+#define BOARD_CONFIGURATION_VARSTORE_ID  0x0001
+#define BOARD_CONFIGURATION_FORM_ID      0x0001
+
+#define BOARD_LOCK_DOWN_BIOS_LOCK         0x2000
+#define BOARD_LOCK_DOWN_PMC_READ_DISABLE  0x2001
+
+#define QUESTION_SAVE_EXIT     0x2ffe
+#define QUESTION_DISCARD_EXIT  0x2fff
+
+//
+// NV data structure
+//
+typedef struct {
+  UINT8   LockDownBiosLock;
+  UINT8   LockDownPmcReadDisable;
+} BOARD_CONFIGURATION;
+
+#define BOARD_CONFIG_NV_NAME  L"BoardSetup"
+
+#endif
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/BoardConfigVfr.vfr b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/BoardConfigVfr.vfr
new file mode 100644
index 000000000000..c5af8d955de8
--- /dev/null
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/BoardConfigVfr.vfr
@@ -0,0 +1,68 @@
+/** @file
+  VFR file used by Aspire VN7-572G board configuration component.
+
+Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2021, Baruch Binyamin Doron
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Guid/HiiPlatformSetupFormset.h>
+#include <BoardConfigNvData.h>
+
+formset
+  guid       = BOARD_CONFIG_FORMSET_GUID,
+  title      = STRING_TOKEN(STR_BOARD_TITLE),
+  help       = STRING_TOKEN(STR_BOARD_HELP),
+  classguid  = EFI_HII_PLATFORM_SETUP_FORMSET_GUID,
+
+  efivarstore BOARD_CONFIGURATION,
+    varid      = BOARD_CONFIGURATION_VARSTORE_ID,
+    attribute  = 0x03,  // VARIABLE_ATTRIBUTE_NV_BS
+    name       = BoardSetup,
+    guid       = BOARD_CONFIG_FORMSET_GUID;
+
+  form formid = BOARD_CONFIGURATION_FORM_ID,
+    title = STRING_TOKEN(STR_BOARD_TITLE);
+
+    subtitle text = STRING_TOKEN(STR_NULL);
+
+    checkbox varid = BoardSetup.LockDownBiosLock,
+            questionid  = BOARD_LOCK_DOWN_BIOS_LOCK,
+            prompt      = STRING_TOKEN(STR_BOARD_LOCK_DOWN_BIOS_LOCK),
+            help        = STRING_TOKEN(STR_BOARD_LOCK_DOWN_BIOS_LOCK_HELP),
+            flags       = RESET_REQUIRED,
+            default     = 1,
+    endcheckbox;
+
+    checkbox varid = BoardSetup.LockDownPmcReadDisable,
+            questionid  = BOARD_LOCK_DOWN_PMC_READ_DISABLE,
+            prompt      = STRING_TOKEN(STR_BOARD_LOCK_DOWN_PMC_READ_DISABLE),
+            help        = STRING_TOKEN(STR_BOARD_LOCK_DOWN_PMC_READ_DISABLE_HELP),
+            flags       = RESET_REQUIRED,
+            default     = 1,
+    endcheckbox;
+
+#if 0
+    resetbutton
+            defaultstore  = BoardConfig,
+            prompt        = STRING_TOKEN(STR_RESET_DEFAULTS_PROMPT_RESET),
+            help          = STRING_TOKEN(STR_RESET_DEFAULTS_PROMPT_RESET_HELP),
+    endresetbutton;
+#endif
+
+    text
+            help    = STRING_TOKEN(STR_SAVE_EXIT),
+            text    = STRING_TOKEN(STR_SAVE_EXIT),
+            flags   = INTERACTIVE,
+            key     = QUESTION_SAVE_EXIT;
+
+    text
+            help    = STRING_TOKEN(STR_DISCARD_EXIT),
+            text    = STRING_TOKEN(STR_DISCARD_EXIT),
+            flags   = INTERACTIVE,
+            key     = QUESTION_DISCARD_EXIT;
+
+  endform;
+
+endformset;
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/BoardConfigVfrStrings.uni b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/BoardConfigVfrStrings.uni
new file mode 100644
index 000000000000..f3c7b66d0217
--- /dev/null
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/BoardConfigVfrStrings.uni
@@ -0,0 +1,29 @@
+/** @file
+  String definitions for Aspire VN7-572G board configuration form.
+
+Copyright (c) 2015 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2021, Baruch Binyamin Doron
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#langdef en-US "English"
+
+#string STR_BOARD_TITLE                            #language en-US "Board Configuration"
+#string STR_BOARD_HELP                             #language en-US "Press <Enter> to select board Setup options."
+
+#string STR_BOARD_LOCK_DOWN_BIOS_LOCK              #language en-US "BIOS Lock"
+#string STR_BOARD_LOCK_DOWN_BIOS_LOCK_HELP         #language en-US "Enable SPI flash lockdown\n"
+                                                               "Disable this option to flash the BIOS image.\n"
+                                                               "For security purposes, this option should be enabled."
+#string STR_BOARD_LOCK_DOWN_PMC_READ_DISABLE       #language en-US "PMC XRAM read disable"
+#string STR_BOARD_LOCK_DOWN_PMC_READ_DISABLE_HELP  #language en-US "Disable PMC XRAM read\n"
+                                                               "Disable this option to permit OS drivers to retrieve data from the PMC.\n"
+                                                               "This may have security impact."
+
+#string STR_RESET_DEFAULTS_PROMPT_RESET            #language en-US "Reset to defaults"
+#string STR_RESET_DEFAULTS_PROMPT_RESET_HELP       #language en-US "This will reset the configuration entries to their default values"
+#string STR_SAVE_EXIT                              #language en-US "Commit Changes and Exit"
+#string STR_DISCARD_EXIT                           #language en-US "Discard Changes and Exit"
+
+#string STR_NULL                                   #language en-US ""
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardConfigHii.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardConfigHii.c
new file mode 100644
index 000000000000..437d31698f7d
--- /dev/null
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardConfigHii.c
@@ -0,0 +1,374 @@
+/** @file
+  Installs Aspire VN7-572G board config and handles the HII callbacks.
+  NOTE: Variable structure is expected to change, so in-place updates are fragile.
+  - An updated structure may be larger than a present variable. Will this over-read,
+    or will HII validation mitigate this?
+
+  Copyright (c) 2021, Baruch Binyamin Doron
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include "DxeBoardInitLib.h"
+#include <Library/BaseMemoryLib.h>
+#include <Library/DevicePathLib.h>
+#include <Library/HiiLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiHiiServicesLib.h>
+#include <BoardConfigNvData.h>
+
+BOARD_CONFIG_CALLBACK_DATA  gBoardConfigPrivate = {
+  BOARD_CONFIG_CALLBACK_DATA_SIGNATURE,
+  NULL,
+  NULL,
+  {
+    BoardConfigExtractConfig,
+    BoardConfigRouteConfig,
+    BoardConfigCallback
+  }
+};
+
+EFI_GUID  mBoardConfigFormsetGuid = BOARD_CONFIG_FORMSET_GUID;
+
+HII_VENDOR_DEVICE_PATH  mBoardConfigHiiVendorDevicePath = {
+  {
+    {
+      HARDWARE_DEVICE_PATH,
+      HW_VENDOR_DP,
+      {
+        (UINT8) (sizeof (VENDOR_DEVICE_PATH)),
+        (UINT8) ((sizeof (VENDOR_DEVICE_PATH)) >> 8)
+      }
+    },
+    BOARD_CONFIG_FORMSET_GUID
+  },
+  {
+    END_DEVICE_PATH_TYPE,
+    END_ENTIRE_DEVICE_PATH_SUBTYPE,
+    {
+      (UINT8) (END_DEVICE_PATH_LENGTH),
+      (UINT8) ((END_DEVICE_PATH_LENGTH) >> 8)
+    }
+  }
+};
+
+/**
+  This function allows a caller to extract the current configuration for one
+  or more named elements from the target driver.
+
+
+  @param This            Points to the EFI_HII_CONFIG_ACCESS_PROTOCOL.
+  @param Request         A null-terminated Unicode string in <ConfigRequest> format.
+  @param Progress        On return, points to a character in the Request string.
+                         Points to the string's null terminator if request was successful.
+                         Points to the most recent '&' before the first failing name/value
+                         pair (or the beginning of the string if the failure is in the
+                         first name/value pair) if the request was not successful.
+  @param Results         A null-terminated Unicode string in <ConfigAltResp> format which
+                         has all values filled in for the names in the Request string.
+                         String to be allocated by the called function.
+
+  @retval  EFI_SUCCESS            The Results is filled with the requested values.
+  @retval  EFI_OUT_OF_RESOURCES   Not enough memory to store the results.
+  @retval  EFI_INVALID_PARAMETER  Request is illegal syntax, or unknown name.
+  @retval  EFI_NOT_FOUND          Routing data doesn't match any storage in this driver.
+
+**/
+EFI_STATUS
+EFIAPI
+BoardConfigExtractConfig (
+  IN  CONST EFI_HII_CONFIG_ACCESS_PROTOCOL   *This,
+  IN  CONST EFI_STRING                       Request,
+  OUT EFI_STRING                             *Progress,
+  OUT EFI_STRING                             *Results
+  )
+{
+  EFI_STATUS           Status;
+  UINTN                DataSize;
+  BOARD_CONFIGURATION  BoardConfig;
+
+  if (Progress == NULL || Results == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *Progress = Request;
+  if ((Request != NULL) &&
+    !HiiIsConfigHdrMatch (Request, &mBoardConfigFormsetGuid, BOARD_CONFIG_NV_NAME)) {
+    return EFI_NOT_FOUND;
+  }
+
+  // Get variable
+  DataSize = sizeof (BoardConfig);
+  Status = gRT->GetVariable (
+                  BOARD_CONFIG_NV_NAME,
+                  &mBoardConfigFormsetGuid,
+                  NULL,
+                  &DataSize,
+                  &BoardConfig
+                  );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  // Use HII helper to convert variable data to config
+  Status = gHiiConfigRouting->BlockToConfig (
+                                gHiiConfigRouting,
+                                Request,
+                                (VOID *) &BoardConfig,
+                                DataSize,
+                                Results,
+                                Progress
+                                );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a(): Failed to retrieve board config - %r!\n", Status));
+  }
+
+  return Status;
+}
+
+/**
+  This function processes the results of changes in configuration.
+
+
+  @param This            Points to the EFI_HII_CONFIG_ACCESS_PROTOCOL.
+  @param Configuration   A null-terminated Unicode string in <ConfigResp> format.
+  @param Progress        A pointer to a string filled in with the offset of the most
+                         recent '&' before the first failing name/value pair (or the
+                         beginning of the string if the failure is in the first
+                         name/value pair) or the terminating NULL if all was successful.
+
+  @retval  EFI_SUCCESS            The Results is processed successfully.
+  @retval  EFI_INVALID_PARAMETER  Configuration is NULL.
+  @retval  EFI_NOT_FOUND          Routing data doesn't match any storage in this driver.
+
+**/
+EFI_STATUS
+EFIAPI
+BoardConfigRouteConfig (
+  IN  CONST EFI_HII_CONFIG_ACCESS_PROTOCOL   *This,
+  IN  CONST EFI_STRING                       Configuration,
+  OUT EFI_STRING                             *Progress
+  )
+{
+  EFI_STATUS           Status;
+  UINTN                DataSize;
+  BOARD_CONFIGURATION  BoardConfig;
+
+  if (Configuration == NULL || Progress == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  *Progress  = Configuration;
+  if (!HiiIsConfigHdrMatch (Configuration, &mBoardConfigFormsetGuid, BOARD_CONFIG_NV_NAME)) {
+    return EFI_NOT_FOUND;
+  }
+
+  // Get variable
+  DataSize = sizeof (BoardConfig);
+  Status = gRT->GetVariable (
+                  BOARD_CONFIG_NV_NAME,
+                  &mBoardConfigFormsetGuid,
+                  NULL,
+                  &DataSize,
+                  &BoardConfig
+                  );
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  // Use HII helper to convert updated config to variable data
+  Status = gHiiConfigRouting->ConfigToBlock (
+                                gHiiConfigRouting,
+                                Configuration,
+                                (VOID *) &BoardConfig,
+                                &DataSize,
+                                Progress
+                                );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a(): Failed to convert board config - %r!\n", Status));
+  }
+
+  // Set variable
+  Status = gRT->SetVariable (
+                  BOARD_CONFIG_NV_NAME,
+                  &mBoardConfigFormsetGuid,
+                  EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+                  DataSize,
+                  &BoardConfig
+                  );
+
+  return Status;
+}
+
+/**
+  This callback function is registered with the formset. When user selects a configuration,
+  this call back function will be triggered.
+
+
+  @param This            Points to the EFI_HII_CONFIG_ACCESS_PROTOCOL.
+  @param Action          Specifies the type of action taken by the browser.
+  @param QuestionId      A unique value which is sent to the original exporting driver
+                         so that it can identify the type of data to expect.
+  @param Type            The type of value for the question.
+  @param Value           A pointer to the data being sent to the original exporting driver.
+  @param ActionRequest   On return, points to the action requested by the callback function.
+
+  @retval  EFI_SUCCESS           The callback successfully handled the action.
+  @retval  EFI_INVALID_PARAMETER The setup browser call this function with invalid parameters.
+
+**/
+EFI_STATUS
+EFIAPI
+BoardConfigCallback (
+  IN CONST EFI_HII_CONFIG_ACCESS_PROTOCOL      *This,
+  IN     EFI_BROWSER_ACTION                    Action,
+  IN     EFI_QUESTION_ID                       QuestionId,
+  IN     UINT8                                 Type,
+  IN     EFI_IFR_TYPE_VALUE                    *Value,
+     OUT EFI_BROWSER_ACTION_REQUEST            *ActionRequest
+  )
+{
+  if ((Value == NULL) || (ActionRequest == NULL)) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (Action != EFI_BROWSER_ACTION_CHANGED) {
+    return EFI_UNSUPPORTED;
+  }
+
+  if (QuestionId == QUESTION_SAVE_EXIT) {
+    *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_SUBMIT_EXIT;
+  } else if (QuestionId == QUESTION_DISCARD_EXIT) {
+    *ActionRequest = EFI_BROWSER_ACTION_REQUEST_FORM_DISCARD_EXIT;
+  }
+
+  return EFI_SUCCESS;
+}
+
+/**
+  This function installs the HII form.
+
+**/
+VOID
+EFIAPI
+InstallBoardConfigHiiForm (
+  VOID
+  )
+{
+  EFI_STATUS           Status;
+  BOARD_CONFIGURATION  BoardConfig;
+  EFI_STRING           ConfigRequestHdr;
+  UINTN                DataSize;
+  BOOLEAN              ActionFlag;
+
+  DEBUG ((DEBUG_INFO, "%a() Starts\n", __FUNCTION__));
+
+  //
+  // Install Device Path and Config Access protocols to driver handle
+  //
+  gBoardConfigPrivate.DriverHandle = NULL;
+  Status = gBS->InstallMultipleProtocolInterfaces (
+                  &gBoardConfigPrivate.DriverHandle,
+                  &gEfiDevicePathProtocolGuid,
+                  &mBoardConfigHiiVendorDevicePath,
+                  &gEfiHiiConfigAccessProtocolGuid,
+                  &gBoardConfigPrivate.ConfigAccess,
+                  NULL
+                  );
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Publish our HII data
+  //
+  gBoardConfigPrivate.HiiHandle = HiiAddPackages (
+                                    &mBoardConfigFormsetGuid,
+                                    gBoardConfigPrivate.DriverHandle,
+                                    BoardConfigVfrBin,
+                                    DxeBoardInitLibStrings,
+                                    NULL
+                                    );
+  ASSERT (gBoardConfigPrivate.HiiHandle != NULL);
+
+  //
+  // Initialise VarStore data.
+  //
+  ZeroMem (&BoardConfig, sizeof (BoardConfig));
+  ConfigRequestHdr = HiiConstructConfigHdr (
+                       &mBoardConfigFormsetGuid,
+                       BOARD_CONFIG_NV_NAME,
+                       gBoardConfigPrivate.DriverHandle
+                       );
+  ASSERT (ConfigRequestHdr != NULL);
+
+  // Attempt to retrieve variable
+  DataSize = sizeof (BoardConfig);
+  Status = gRT->GetVariable (
+                  BOARD_CONFIG_NV_NAME,
+                  &mBoardConfigFormsetGuid,
+                  NULL,
+                  &DataSize,
+                  &BoardConfig
+                  );
+  // HII helper functions will use ExtractConfig() and RouteConfig(),
+  // where we will set the variable as required
+  if (!EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_INFO, "Config variable exists, validate contents\n"));
+    ActionFlag = HiiValidateSettings (ConfigRequestHdr);
+    if (!ActionFlag) {
+      DEBUG ((DEBUG_INFO, "Variable is invalid, reset to defaults\n"));
+      ActionFlag = HiiSetToDefaults (ConfigRequestHdr, EFI_HII_DEFAULT_CLASS_STANDARD);
+      ASSERT (ActionFlag);
+    }
+  } else {
+    DEBUG ((DEBUG_INFO, "Config variable does not exist, create and set to defaults\n"));
+    Status = gRT->SetVariable (
+                    BOARD_CONFIG_NV_NAME,
+                    &mBoardConfigFormsetGuid,
+                    EFI_VARIABLE_NON_VOLATILE | EFI_VARIABLE_BOOTSERVICE_ACCESS,
+                    DataSize,
+                    &BoardConfig
+                    );
+    ASSERT_EFI_ERROR (Status);
+    ActionFlag = HiiSetToDefaults (ConfigRequestHdr, EFI_HII_DEFAULT_CLASS_STANDARD);
+    ASSERT (ActionFlag);
+  }
+
+  FreePool (ConfigRequestHdr);
+
+  DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
+}
+
+/**
+  This function uninstalls the HII form.
+
+**/
+VOID
+EFIAPI
+UninstallBoardConfigHiiForm (
+  VOID
+  )
+{
+  EFI_STATUS           Status;
+
+  DEBUG ((DEBUG_INFO, "%a() Starts\n", __FUNCTION__));
+
+  //
+  // Uninstall Device Path and Config Access protocols
+  //
+  Status = gBS->UninstallMultipleProtocolInterfaces (
+                  gBoardConfigPrivate.DriverHandle,
+                  &gEfiDevicePathProtocolGuid,
+                  &mBoardConfigHiiVendorDevicePath,
+                  &gEfiHiiConfigAccessProtocolGuid,
+                  &gBoardConfigPrivate.ConfigAccess,
+                  NULL
+                  );
+  ASSERT_EFI_ERROR (Status);
+
+  //
+  // Remove our HII data
+  //
+  HiiRemovePackages (gBoardConfigPrivate.HiiHandle);
+
+  DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
+}
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c
index 07278d956ddc..cc99240b5aaa 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c
@@ -7,13 +7,10 @@
 
 **/
 
-#include <PiDxe.h>
+#include "DxeBoardInitLib.h"
 #include <Library/BoardEcLib.h>
 #include <Library/BoardInitLib.h>
-#include <Library/DebugLib.h>
 #include <Library/EcLib.h>
-#include <Library/UefiBootServicesTableLib.h>
-#include <Library/UefiRuntimeServicesTableLib.h>
 #include <Protocol/ResetNotification.h>
 
 EFI_RESET_NOTIFICATION_PROTOCOL  *mResetNotify = NULL;
@@ -131,6 +128,12 @@ EcResetSystemHook (
   }
 }
 
+VOID
+EFIAPI
+InstallBoardConfigHiiForm (
+  VOID
+  );
+
 /**
   A hook for board-specific initialization after PCI enumeration.
 
@@ -159,6 +162,8 @@ BoardInitAfterPciEnumeration (
     DEBUG ((DEBUG_INFO, "EC: Added callback to notify EC of resets\n"));
   }
 
+  InstallBoardConfigHiiForm ();
+
   DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
@@ -178,6 +183,12 @@ BoardInitReadyToBoot (
   return EFI_SUCCESS;
 }
 
+VOID
+EFIAPI
+UninstallBoardConfigHiiForm (
+  VOID
+  );
+
 /**
   A hook for board-specific functionality for the ExitBootServices event.
 
@@ -201,6 +212,8 @@ BoardInitEndOfFirmware (
     DEBUG ((DEBUG_INFO, "EC: Removed callback to notify EC of resets\n"));
   }
 
+  UninstallBoardConfigHiiForm ();
+
   DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.h
new file mode 100644
index 000000000000..17383b71f7d9
--- /dev/null
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.h
@@ -0,0 +1,131 @@
+/** @file
+  Aspire VN7-572G Board Initialization DXE library
+
+  Copyright (c) 2021, Baruch Binyamin Doron
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _DXE_BOARD_INIT_LIB_H_
+#define _DXE_BOARD_INIT_LIB_H_
+
+#include <PiDxe.h>
+#include <Library/DebugLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+#include <Protocol/HiiConfigAccess.h>
+
+//
+// These are the VFR compiler generated data representing our VFR data.
+//
+extern UINT8 BoardConfigVfrBin[];
+
+#define BOARD_CONFIG_CALLBACK_DATA_SIGNATURE  SIGNATURE_32 ('B', 'C', 'C', 'B')
+
+typedef struct {
+  UINTN                           Signature;
+
+  //
+  // HII relative handles
+  //
+  EFI_HII_HANDLE                  HiiHandle;
+  EFI_HANDLE                      DriverHandle;
+
+  //
+  // Produced protocols
+  //
+  EFI_HII_CONFIG_ACCESS_PROTOCOL   ConfigAccess;
+} BOARD_CONFIG_CALLBACK_DATA;
+
+///
+/// HII specific Vendor Device Path definition.
+///
+typedef struct {
+  VENDOR_DEVICE_PATH                VendorDevicePath;
+  EFI_DEVICE_PATH_PROTOCOL          End;
+} HII_VENDOR_DEVICE_PATH;
+
+/**
+  This function allows a caller to extract the current configuration for one
+  or more named elements from the target driver.
+
+
+  @param This            Points to the EFI_HII_CONFIG_ACCESS_PROTOCOL.
+  @param Request         A null-terminated Unicode string in <ConfigRequest> format.
+  @param Progress        On return, points to a character in the Request string.
+                         Points to the string's null terminator if request was successful.
+                         Points to the most recent '&' before the first failing name/value
+                         pair (or the beginning of the string if the failure is in the
+                         first name/value pair) if the request was not successful.
+  @param Results         A null-terminated Unicode string in <ConfigAltResp> format which
+                         has all values filled in for the names in the Request string.
+                         String to be allocated by the called function.
+
+  @retval  EFI_SUCCESS            The Results is filled with the requested values.
+  @retval  EFI_OUT_OF_RESOURCES   Not enough memory to store the results.
+  @retval  EFI_INVALID_PARAMETER  Request is illegal syntax, or unknown name.
+  @retval  EFI_NOT_FOUND          Routing data doesn't match any storage in this driver.
+
+**/
+EFI_STATUS
+EFIAPI
+BoardConfigExtractConfig (
+  IN  CONST EFI_HII_CONFIG_ACCESS_PROTOCOL   *This,
+  IN  CONST EFI_STRING                       Request,
+  OUT EFI_STRING                             *Progress,
+  OUT EFI_STRING                             *Results
+  );
+
+/**
+  This function processes the results of changes in configuration.
+
+
+  @param This            Points to the EFI_HII_CONFIG_ACCESS_PROTOCOL.
+  @param Configuration   A null-terminated Unicode string in <ConfigResp> format.
+  @param Progress        A pointer to a string filled in with the offset of the most
+                         recent '&' before the first failing name/value pair (or the
+                         beginning of the string if the failure is in the first
+                         name/value pair) or the terminating NULL if all was successful.
+
+  @retval  EFI_SUCCESS            The Results is processed successfully.
+  @retval  EFI_INVALID_PARAMETER  Configuration is NULL.
+  @retval  EFI_NOT_FOUND          Routing data doesn't match any storage in this driver.
+
+**/
+EFI_STATUS
+EFIAPI
+BoardConfigRouteConfig (
+  IN  CONST EFI_HII_CONFIG_ACCESS_PROTOCOL   *This,
+  IN  CONST EFI_STRING                       Configuration,
+  OUT EFI_STRING                             *Progress
+  );
+
+/**
+  This callback function is registered with the formset. When user selects a configuration,
+  this call back function will be triggered.
+
+
+  @param This            Points to the EFI_HII_CONFIG_ACCESS_PROTOCOL.
+  @param Action          Specifies the type of action taken by the browser.
+  @param QuestionId      A unique value which is sent to the original exporting driver
+                         so that it can identify the type of data to expect.
+  @param Type            The type of value for the question.
+  @param Value           A pointer to the data being sent to the original exporting driver.
+  @param ActionRequest   On return, points to the action requested by the callback function.
+
+  @retval  EFI_SUCCESS           The callback successfully handled the action.
+  @retval  EFI_INVALID_PARAMETER The setup browser call this function with invalid parameters.
+
+**/
+EFI_STATUS
+EFIAPI
+BoardConfigCallback (
+  IN CONST EFI_HII_CONFIG_ACCESS_PROTOCOL      *This,
+  IN     EFI_BROWSER_ACTION                    Action,
+  IN     EFI_QUESTION_ID                       QuestionId,
+  IN     UINT8                                 Type,
+  IN     EFI_IFR_TYPE_VALUE                    *Value,
+     OUT EFI_BROWSER_ACTION_REQUEST            *ActionRequest
+  );
+
+#endif // _DXE_BOARD_INIT_LIB_H_
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.inf
index 24747fa7b224..cd74f957ce10 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.inf
@@ -17,17 +17,27 @@
 [LibraryClasses]
   UefiBootServicesTableLib
   UefiRuntimeServicesTableLib
+  BaseMemoryLib
   DebugLib
   EcLib
   BoardEcLib
+  HiiLib
+  MemoryAllocationLib
+  UefiHiiServicesLib
 
 [Packages]
   MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
   MinPlatformPkg/MinPlatformPkg.dec
   KabylakeOpenBoardPkg/OpenBoardPkg.dec
 
 [Sources]
   DxeBoardInitLib.c
+  DxeBoardConfigHii.c
+  BoardConfigVfr.vfr
+  BoardConfigVfrStrings.uni
 
 [Protocols]
   gEfiResetNotificationProtocolGuid  ## CONSUMES
+  gEfiDevicePathProtocolGuid         ## PRODUCES
+  gEfiHiiConfigAccessProtocolGuid    ## PRODUCES
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [edk2-devel][edk2-platforms][PATCH v1 6/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Improve board detection
  2022-09-06 17:42 [edk2-devel][edk2-platforms][PATCH v1 0/7] Benjamin Doron
                   ` (4 preceding siblings ...)
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 5/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Use Setup to control security Benjamin Doron
@ 2022-09-06 17:42 ` Benjamin Doron
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 7/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Align DEBUG() use Benjamin Doron
  6 siblings, 0 replies; 9+ messages in thread
From: Benjamin Doron @ 2022-09-06 17:42 UTC (permalink / raw)
  To: devel; +Cc: Sai Chaganty, Isaac Oram, Chasel Chiu, Nate DeSimone, Ankit Sinha

Improvements based on ENE KB9012Q datasheet. Values read from EC ADC are
much more consistent now. Some improvement still necessary before this
is reliable, as my Rayleigh-SL (PCH-LP) is now consistently detected as
a Newgate-SLS (PCH-H).

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Ankit Sinha <ankit.sinha@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 .../Include/Library/BoardEcLib.h              |  5 +-
 .../Library/BoardEcLib/BoardEcLib.inf         |  1 +
 .../Library/BoardEcLib/EcCommands.c           | 36 ++++++++++----
 .../BoardInitLib/PeiAspireVn7Dash572GDetect.c | 47 +++++++++++--------
 .../AspireVn7Dash572G/OpenBoardPkg.dsc        |  5 +-
 .../PeiBoardPolicyUpdate.c                    |  2 +-
 .../Include/PlatformBoardId.h                 |  5 +-
 7 files changed, 65 insertions(+), 36 deletions(-)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/Library/BoardEcLib.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/Library/BoardEcLib.h
index 8bb4cccb8f19..56fdd4ed756c 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/Library/BoardEcLib.h
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Include/Library/BoardEcLib.h
@@ -82,8 +82,9 @@ EcIdxWrite (
   );
 
 /**
-  Read EC analog-digital converter.
-  TODO: Check if ADC is valid.
+  Read an analog-digital converter from the EC.
+  TODO: There are actually 8 ADCs, but those can remain unused.
+  - Handling port enable bits and pin IE could get complicated.
 
   @param[in]  Adc
   @param[out] DataBuffer
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/BoardEcLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/BoardEcLib.inf
index 56527c3b9a3c..7287301583e0 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/BoardEcLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/BoardEcLib.inf
@@ -18,6 +18,7 @@
   DebugLib
   EcLib
   IoLib
+  TimerLib
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
index 54cfaba47b1b..182cda6f1933 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
@@ -9,9 +9,11 @@
 
 #include <Base.h>
 #include <Uefi.h>
+#include <Library/BoardEcLib.h>
 #include <Library/DebugLib.h>
 #include <Library/EcLib.h>
 #include <Library/IoLib.h>
+#include <Library/TimerLib.h>
 
 /*
  * Notes:
@@ -193,8 +195,9 @@ EcIdxWrite (
 }
 
 /**
-  Read EC analog-digital converter.
-  TODO: Check if ADC is valid.
+  Read an analog-digital converter from the EC.
+  TODO: There are actually 8 ADCs, but those can remain unused.
+  - Handling port enable bits and pin IE could get complicated.
 
   @param[in]  Adc
   @param[out] DataBuffer
@@ -205,23 +208,36 @@ ReadEcAdcConverter (
   OUT UINT16       *DataBuffer
   )
 {
-  UINT8            AdcConvertersEnabled;  // Contains some ADCs and some DACs
+  UINT8            LowAdcsEnabled;  // Contains some ADCs and some DACs
   UINT8            IdxData;
 
   if (DataBuffer == NULL) {
     return;
   }
 
+  if (Adc >= 4) {
+    DEBUG ((DEBUG_ERROR, "Handling ADC%d is unsupported!\n", Adc));
+    return;
+  }
+
   // Backup enabled ADCs
-  EcIdxRead (0xff15, &AdcConvertersEnabled);  // ADDAEN
+  EcIdxRead (0xff15, &LowAdcsEnabled);   // ADDAEN
 
-  // Enable desired ADC in bitmask (not enabled by EC FW, not used by vendor FW)
-  EcIdxWrite (0xff15, AdcConvertersEnabled | ((1 << Adc) & 0xf));  // ADDAEN
+  /* 1. Clear IE of the related pin - ADC0: "GPIOIE38[0] (0xFC67[0]) = 0b" */
+  EcIdxRead (0xfc67, &IdxData);
+  IdxData &= ~(1 << Adc);
+  EcIdxWrite (0xfc67, IdxData);
 
-  // Sample the desired ADC in binary field; OR the start bit
-  EcIdxWrite (0xff18, ((Adc << 1) & 0xf) | 1);  // ADCTRL
+  /* 2. Enable desired ADC function in bitmask */
+  EcIdxWrite (0xff15, (1 << Adc) & 0xf);  // ADDAEN
 
-  // Read the desired ADC
+  /* 3. Enable control of desired ADC in bit field, OR the start bit */
+  EcIdxWrite (0xff18, ((Adc << 1) & 7) | 1);  // ADCTRL
+
+  /* TODO: Await ADC interrupt */
+  MicroSecondDelay (256);  // Wait "Voltage Conversion Time"
+
+  /* 4. Read the desired ADC */
   EcIdxRead (0xff19, &IdxData);  // ADCDAT
   *DataBuffer = (IdxData << 2);
   // Lower 2-bits of 10-bit ADC are in high bits of next register
@@ -229,5 +245,5 @@ ReadEcAdcConverter (
   *DataBuffer |= ((IdxData & 0xc0) >> 6);
 
   // Restore enabled ADCs
-  EcIdxWrite (0xff15, AdcConvertersEnabled);  // ADDAEN
+  EcIdxWrite (0xff15, LowAdcsEnabled);   // ADDAEN
 }
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GDetect.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GDetect.c
index 344e06859e9b..0ce747bb67c6 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GDetect.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GDetect.c
@@ -1,6 +1,7 @@
 /** @file
 
 Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2021, Baruch Binyamin Doron<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -10,14 +11,14 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/BoardEcLib.h>
 #include <Library/DebugLib.h>
 
-#define ADC_3V_10BIT_GRANULARITY_MAX  (3005/1023)
+#define ADC_3V_10BIT_GRANULARITY_MAX  (3005 / 1023)
 #define PCB_VER_AD                    1
 #define MODEL_ID_AD                   3
 
 /**
-  Get Aspire V Nitro (Skylake) board ID.
-  There are 2 different boards having different ID.
-  This function will return board ID to caller.
+  Get Aspire V Nitro (Skylake) board ID. There are 3 different boards
+  having different PCH (therefore, ID). This function will return board ID to caller.
+  - TODO/NB: Detection is still unreliable. Likely must await interrupt.
 
   @param[out] DataBuffer
 
@@ -32,34 +33,42 @@ GetAspireVn7Dash572GBoardId (
   UINT16        DataBuffer;
 
   ReadEcAdcConverter (MODEL_ID_AD, &DataBuffer);
-  DEBUG ((DEBUG_INFO, "BoardId (raw) = 0x%X\n", DataBuffer));
+  DEBUG ((DEBUG_INFO, "BoardId (raw) = %d\n", DataBuffer));
   // Board by max millivoltage range (of 10-bit, 3.005 V ADC)
-  if (DataBuffer <= (1374/ADC_3V_10BIT_GRANULARITY_MAX)) {
+  if (DataBuffer <= (1374 / ADC_3V_10BIT_GRANULARITY_MAX)) {
     // Consider returning an error
     DEBUG ((DEBUG_ERROR, "BoardId is reserved?\n"));
-  } else if (DataBuffer <= (2017/ADC_3V_10BIT_GRANULARITY_MAX)) {
-    *BoardId = BoardIdNewgateSLx_dGPU;
+    *BoardId = 0;
+  } else if (DataBuffer <= (2017 / ADC_3V_10BIT_GRANULARITY_MAX)) {
+    *BoardId = BoardIdNewgateSLS_dGPU;
+  } else if (DataBuffer <= (2259 / ADC_3V_10BIT_GRANULARITY_MAX)) {
+    *BoardId = BoardIdRayleighSLS_960M;
   } else {
-    *BoardId = BoardIdRayleighSLx_dGPU;
+    *BoardId = BoardIdRayleighSL_dGPU;
   }
   DEBUG ((DEBUG_INFO, "BoardId = 0x%X\n", *BoardId));
 
   ReadEcAdcConverter (PCB_VER_AD, &DataBuffer);
-  DEBUG ((DEBUG_INFO, "PCB version (raw) = 0x%X\n", DataBuffer));
+  DEBUG ((DEBUG_INFO, "PCB version (raw) = %d\n", DataBuffer));
   DEBUG ((DEBUG_INFO, "PCB version: "));
   // PCB by max millivoltage range (of 10-bit, 3.005 V ADC)
-  if (DataBuffer <= (2017/ADC_3V_10BIT_GRANULARITY_MAX)) {
+  if (DataBuffer <= (2017 / ADC_3V_10BIT_GRANULARITY_MAX)) {
     // Consider returning an error
     DEBUG ((DEBUG_ERROR, "Reserved?\n"));
-  } else if (DataBuffer <= (2259/ADC_3V_10BIT_GRANULARITY_MAX)) {
-    DEBUG ((DEBUG_ERROR, "-1\n"));
-  } else if (DataBuffer <= (2493/ADC_3V_10BIT_GRANULARITY_MAX)) {
-    DEBUG ((DEBUG_ERROR, "SC\n"));
-  } else if (DataBuffer <= (2759/ADC_3V_10BIT_GRANULARITY_MAX)) {
-    DEBUG ((DEBUG_ERROR, "SB\n"));
+  } else if (DataBuffer <= (2259 / ADC_3V_10BIT_GRANULARITY_MAX)) {
+    DEBUG ((DEBUG_INFO, "-1\n"));
+  } else if (DataBuffer <= (2493 / ADC_3V_10BIT_GRANULARITY_MAX)) {
+    DEBUG ((DEBUG_INFO, "SC\n"));
+  } else if (DataBuffer <= (2759 / ADC_3V_10BIT_GRANULARITY_MAX)) {
+    DEBUG ((DEBUG_INFO, "SB\n"));
   } else {
-    DEBUG ((DEBUG_ERROR, "SA\n"));
+    DEBUG ((DEBUG_INFO, "SA\n"));
   }
+
+  // FIXME
+  DEBUG ((DEBUG_WARN, "OVERRIDE: Detection is unreliable and other boards unsupported!\n"));
+  DEBUG ((DEBUG_INFO, "Setting board SKU to Rayleigh-SL\n"));
+  *BoardId = BoardIdRayleighSL_dGPU;
 }
 
 EFI_STATUS
@@ -76,7 +85,7 @@ AspireVn7Dash572GBoardDetect (
 
   DEBUG ((DEBUG_INFO, "AspireVn7Dash572GDetectionCallback\n"));
   GetAspireVn7Dash572GBoardId (&BoardId);
-  if (BoardId == BoardIdNewgateSLx_dGPU || BoardId == BoardIdRayleighSLx_dGPU) {
+  if (BoardId == BoardIdNewgateSLS_dGPU || BoardId == BoardIdRayleighSLS_960M || BoardId == BoardIdRayleighSL_dGPU) {
     LibPcdSetSku (BoardId);
     ASSERT (LibPcdGetSku() == BoardId);
   } else {
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.dsc b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.dsc
index 75c537f1253f..4458e7b75118 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.dsc
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.dsc
@@ -88,8 +88,9 @@
 [SkuIds]
   0x00|DEFAULT                # 0|DEFAULT is reserved and always required.
   # For further details on specific SKUs (which dGPU installed), see EC page of schematics
-  0x41|RayleighSLx_dGPU       # Detect the UMA board by GPIO
-  0x42|NewgateSLx_dGPU
+  0x41|NewgateSLS_dGPU
+  0x42|RayleighSLS_960M
+  0x43|RayleighSL_dGPU        # Detect the UMA board by GPIO
 
 ################################################################################
 #
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/PeiSiliconPolicyUpdateLib/PeiBoardPolicyUpdate.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/PeiSiliconPolicyUpdateLib/PeiBoardPolicyUpdate.c
index 95b7c4ad5f77..54c742147b19 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/PeiSiliconPolicyUpdateLib/PeiBoardPolicyUpdate.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/PeiSiliconPolicyUpdateLib/PeiBoardPolicyUpdate.c
@@ -175,7 +175,7 @@ PeiFspBoardPolicyUpdate (
   //       that it does - this appears to be static text?) and is UART0 merely supporting
   //       the UART2 devfn?
 
-  // Acer IDs (TODO: "Newgate" IDs)
+  // Acer IDs (TODO: "Newgate" and "RayleighSLS" IDs)
 //FIXME  FspsUpd->FspsConfig.DefaultSvid = 0x1025;
 //FIXME  FspsUpd->FspsConfig.DefaultSid = 0x1037;
   PchGeneralConfig->SubSystemVendorId = 0x1025;
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/Include/PlatformBoardId.h b/Platform/Intel/KabylakeOpenBoardPkg/Include/PlatformBoardId.h
index 0db4fb23583e..78ea0dea88df 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/Include/PlatformBoardId.h
+++ b/Platform/Intel/KabylakeOpenBoardPkg/Include/PlatformBoardId.h
@@ -22,8 +22,9 @@ Kaby Lake Platform Board Identifiers
 
 #define BoardIdSkylakeRvp3                  0x4
 #define BoardIdGalagoPro3                   0x20
-#define BoardIdRayleighSLx_dGPU             0x41
-#define BoardIdNewgateSLx_dGPU              0x42
+#define BoardIdNewgateSLS_dGPU              0x41
+#define BoardIdRayleighSLS_960M             0x42
+#define BoardIdRayleighSL_dGPU              0x43
 #define BoardIdKabyLakeYLpddr3Rvp3          0x60
 
 #define BoardIdUnknown1                     0xffff
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [edk2-devel][edk2-platforms][PATCH v1 7/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Align DEBUG() use
  2022-09-06 17:42 [edk2-devel][edk2-platforms][PATCH v1 0/7] Benjamin Doron
                   ` (5 preceding siblings ...)
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 6/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Improve board detection Benjamin Doron
@ 2022-09-06 17:42 ` Benjamin Doron
  6 siblings, 0 replies; 9+ messages in thread
From: Benjamin Doron @ 2022-09-06 17:42 UTC (permalink / raw)
  To: devel; +Cc: Sai Chaganty, Isaac Oram, Nate DeSimone, Chasel Chiu

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 .../DxeAspireVn7Dash572GAcpiTableLib.c        |  5 ++++
 .../BoardAcpiLib/DxeBoardAcpiTableLib.inf     |  1 +
 .../SmmAspireVn7Dash572GAcpiEnableLib.c       |  8 ++++-
 .../Library/BoardEcLib/EcCommands.c           | 16 +++++-----
 .../Library/BoardInitLib/DxeBoardConfigHii.c  |  8 ++---
 .../Library/BoardInitLib/DxeBoardInitLib.c    | 23 +++++++++------
 .../PeiAspireVn7Dash572GInitPostMemLib.c      | 29 ++++++++++++++-----
 .../PeiAspireVn7Dash572GInitPreMemLib.c       | 20 ++++++++++---
 .../BoardInitLib/PeiBoardInitPreMemLib.c      |  2 ++
 .../DxeSiliconPolicyUpdateLib.c               |  6 ++--
 10 files changed, 80 insertions(+), 38 deletions(-)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeAspireVn7Dash572GAcpiTableLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeAspireVn7Dash572GAcpiTableLib.c
index 131e6460279a..994f18b2dc0e 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeAspireVn7Dash572GAcpiTableLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeAspireVn7Dash572GAcpiTableLib.c
@@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Base.h>
 #include <PiDxe.h>
 #include <Library/BoardAcpiTableLib.h>
+#include <Library/DebugLib.h>
 #include <Library/EcLib.h>
 #include <Library/PcdLib.h>
 #include <Protocol/GlobalNvsArea.h>
@@ -23,6 +24,8 @@ AspireVn7Dash572GUpdateGlobalNvs (
   EFI_STATUS  Status;
   UINT8       PowerRegister;
 
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
+
   //
   // Allocate and initialize the NVS area for SMM and ASL communication.
   //
@@ -57,6 +60,8 @@ AspireVn7Dash572GUpdateGlobalNvs (
   mGlobalNvsArea.Area->Ps2KbMsEnable      = PcdGet8 (PcdPs2KbMsEnable);
 
   mGlobalNvsArea.Area->BoardId = (UINT8) LibPcdGetSku ();
+
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
 }
 
 EFI_STATUS
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
index 660afe9292ec..dc5bc80d0380 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
@@ -23,6 +23,7 @@
 
 [LibraryClasses]
   PcdLib
+  DebugLib
   EcLib
 
 [Packages]
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmAspireVn7Dash572GAcpiEnableLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmAspireVn7Dash572GAcpiEnableLib.c
index fa2ed9745ea6..22c9c76a62e0 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmAspireVn7Dash572GAcpiEnableLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmAspireVn7Dash572GAcpiEnableLib.c
@@ -8,7 +8,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 **/
 
 #include <Base.h>
-#include <PiDxe.h>
+#include <PiSmm.h>
 #include <Library/DebugLib.h>
 #include <Library/EcLib.h>
 
@@ -20,6 +20,8 @@ AspireVn7Dash572GBoardEnableAcpi (
 {
   EFI_STATUS  Status;
 
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
+
   /* Tests at runtime show this re-enables charging and battery reporting
    * - Obtained from somewhere in vendor's SmmKbcDriver.
    *   Further information is needed */
@@ -36,6 +38,7 @@ AspireVn7Dash572GBoardEnableAcpi (
   }
 
   /* TODO: Set touchpad GPP owner to ACPI? */
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
 
   return EFI_SUCCESS;
 }
@@ -48,6 +51,8 @@ AspireVn7Dash572GBoardDisableAcpi (
 {
   EFI_STATUS  Status;
 
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
+
   /* Tests at runtime show this disables charging and battery reporting
    * - Obtained from somewhere in vendor's SmmKbcDriver.
    *   Further information is needed */
@@ -64,6 +69,7 @@ AspireVn7Dash572GBoardDisableAcpi (
   }
 
   /* TODO: Set touchpad GPP owner to GPIO? */
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
 
   return EFI_SUCCESS;
 }
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
index 182cda6f1933..24737d9ecb90 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
@@ -65,19 +65,19 @@ EcCmd90Read (
 
   Status = SendEcCommand (0x90);
   if (EFI_ERROR (Status)) {
-    DEBUG((DEBUG_ERROR, "%a(): SendEcCommand(0x90) failed!\n", __FUNCTION__));
+    DEBUG ((DEBUG_ERROR, "%a(): SendEcCommand(0x90) failed!\n", __FUNCTION__));
     return Status;
   }
 
   Status = SendEcData (Address);
   if (EFI_ERROR (Status)) {
-    DEBUG((DEBUG_ERROR, "%a(): SendEcData(Address) failed!\n", __FUNCTION__));
+    DEBUG ((DEBUG_ERROR, "%a(): SendEcData(Address) failed!\n", __FUNCTION__));
     return Status;
   }
 
   Status = ReceiveEcData (Data);
   if (EFI_ERROR (Status)) {
-    DEBUG((DEBUG_ERROR, "%a(): ReceiveEcData(Data) failed!\n", __FUNCTION__));
+    DEBUG ((DEBUG_ERROR, "%a(): ReceiveEcData(Data) failed!\n", __FUNCTION__));
     return Status;
   }
   return EFI_SUCCESS;
@@ -103,19 +103,19 @@ EcCmd91Write (
 
   Status = SendEcCommand (0x91);
   if (EFI_ERROR (Status)) {
-    DEBUG((DEBUG_ERROR, "%a(): SendEcCommand(0x91) failed!\n", __FUNCTION__));
+    DEBUG ((DEBUG_ERROR, "%a(): SendEcCommand(0x91) failed!\n", __FUNCTION__));
     return Status;
   }
 
   Status = SendEcData (Address);
   if (EFI_ERROR (Status)) {
-    DEBUG((DEBUG_ERROR, "%a(): SendEcData(Address) failed!\n", __FUNCTION__));
+    DEBUG ((DEBUG_ERROR, "%a(): SendEcData(Address) failed!\n", __FUNCTION__));
     return Status;
   }
 
   Status = SendEcData (Data);
   if (EFI_ERROR (Status)) {
-    DEBUG((DEBUG_ERROR, "%a(): SendEcData(Data) failed!\n", __FUNCTION__));
+    DEBUG ((DEBUG_ERROR, "%a(): SendEcData(Data) failed!\n", __FUNCTION__));
     return Status;
   }
   return EFI_SUCCESS;
@@ -144,13 +144,13 @@ EcCmd94Query (
 
   Status = SendEcCommand (0x94);
   if (EFI_ERROR (Status)) {
-    DEBUG((DEBUG_ERROR, "%a(): SendEcCommand(0x94) failed!\n", __FUNCTION__));
+    DEBUG ((DEBUG_ERROR, "%a(): SendEcCommand(0x94) failed!\n", __FUNCTION__));
     return Status;
   }
 
   Status = ReceiveEcData (Data);
   if (EFI_ERROR (Status)) {
-    DEBUG((DEBUG_ERROR, "%a(): ReceiveEcData(Data) failed!\n", __FUNCTION__));
+    DEBUG ((DEBUG_ERROR, "%a(): ReceiveEcData(Data) failed!\n", __FUNCTION__));
     return Status;
   }
   return EFI_SUCCESS;
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardConfigHii.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardConfigHii.c
index 437d31698f7d..2c302684913a 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardConfigHii.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardConfigHii.c
@@ -261,7 +261,7 @@ InstallBoardConfigHiiForm (
   UINTN                DataSize;
   BOOLEAN              ActionFlag;
 
-  DEBUG ((DEBUG_INFO, "%a() Starts\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
 
   //
   // Install Device Path and Config Access protocols to driver handle
@@ -335,7 +335,7 @@ InstallBoardConfigHiiForm (
 
   FreePool (ConfigRequestHdr);
 
-  DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
 }
 
 /**
@@ -350,7 +350,7 @@ UninstallBoardConfigHiiForm (
 {
   EFI_STATUS           Status;
 
-  DEBUG ((DEBUG_INFO, "%a() Starts\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
 
   //
   // Uninstall Device Path and Config Access protocols
@@ -370,5 +370,5 @@ UninstallBoardConfigHiiForm (
   //
   HiiRemovePackages (gBoardConfigPrivate.HiiHandle);
 
-  DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
 }
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c
index cc99240b5aaa..af91034d4701 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/DxeBoardInitLib.c
@@ -1,8 +1,8 @@
 /** @file
   Aspire VN7-572G Board Initialization DXE library
 
-  Copyright (c) 2021, Baruch Binyamin Doron
   Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2021, Baruch Binyamin Doron<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -33,7 +33,7 @@ EcSendTime (
   INTN        Index;
   UINT8       EcResponse;
 
-  DEBUG ((DEBUG_INFO, "%a() Starts\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
 
   Status = gRT->GetTime (&EfiTime, NULL);
   if (EFI_ERROR (Status)) {
@@ -61,7 +61,7 @@ EcSendTime (
     DEBUG ((DEBUG_INFO, "EC: response 0x%x\n", EcResponse));
   }
 
-  DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
 }
 
 /**
@@ -76,7 +76,7 @@ EcRequestsTime (
 {
   UINT8           Dat;
 
-  DEBUG ((DEBUG_INFO, "%a() Starts\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
 
   /* This is executed as protocol notify in vendor's RtKbcDriver when *CommonService
    * protocol is installed. Effectively, this code could execute from the entrypoint */
@@ -85,7 +85,7 @@ EcRequestsTime (
     EcSendTime ();
   }
 
-  DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
 }
 
 /**
@@ -113,6 +113,8 @@ EcResetSystemHook (
   IN VOID                     *ResetData OPTIONAL
   )
 {
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
+
   // If boolean PCD tokens 0xBD, 0xBE and 0xBF are set in vendor FW,
   // OEM also sends command 0x5A with argument 0xAA via ACPI "CMDB" method and stalls for
   // 100000, then sets ResetType to EfiResetShutdown.
@@ -126,6 +128,8 @@ EcResetSystemHook (
     // Now OEM calls function offset 2 in ACER_BOOT_DEVICE_SERVICE_PROTOCOL_GUID.
     // TODO: What does this do?
   }
+
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
 }
 
 VOID
@@ -148,7 +152,7 @@ BoardInitAfterPciEnumeration (
 {
   EFI_STATUS                       Status;
 
-  DEBUG ((DEBUG_INFO, "%a() Starts\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
 
   // Send EC the present time, if requested
   EcRequestsTime ();
@@ -164,7 +168,7 @@ BoardInitAfterPciEnumeration (
 
   InstallBoardConfigHiiForm ();
 
-  DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
 
@@ -180,6 +184,7 @@ BoardInitReadyToBoot (
   VOID
   )
 {
+  DEBUG ((DEBUG_INFO, "%a()\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
 
@@ -203,7 +208,7 @@ BoardInitEndOfFirmware (
 {
   EFI_STATUS                       Status;
 
-  DEBUG ((DEBUG_INFO, "%a() Starts\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
 
   // Remove ResetSystem callback. ACPI will be notifying EC of events
   if (mResetNotify != NULL) {
@@ -214,6 +219,6 @@ BoardInitEndOfFirmware (
 
   UninstallBoardConfigHiiForm ();
 
-  DEBUG ((DEBUG_INFO, "%a() Ends\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPostMemLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPostMemLib.c
index 77722f5d6062..2ad2dd866829 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPostMemLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPostMemLib.c
@@ -1,6 +1,7 @@
 /** @file
 
 Copyright (c) 2017 - 2021, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2021, Baruch Binyamin Doron<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -34,28 +35,32 @@ EcInit (
   VOID
   )
 {
+  EFI_STATUS     Status;
   EFI_BOOT_MODE  BootMode;
-  UINT8          PowerRegister;
+  UINT8          PowerState;
   UINT8          OutData;
   UINT16         ABase;
   UINT16         Pm1Sts;
   UINT32         GpeSts;
 
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
+
   /* This is called via a "$FNC" in a PeiOemModule pointer table, with "$DPX" on SiInit */
   IoWrite8 (0x6C, 0x5A);  // 6Ch is the EC sideband port
-  PeiServicesGetBootMode (&BootMode);
+  Status = PeiServicesGetBootMode (&BootMode);
+  ASSERT_EFI_ERROR (Status);
   if (BootMode == BOOT_ON_S3_RESUME) {
     /* "MLID" in LGMR-based memory map is equivalent to "ELID" in EC-based
      * memory map. Vendor firmware accesses through LGMR; remapped
      * - EcCmd* function calls will not remapped */
-    EcRead (0x70, &PowerRegister);
-    if (!(PowerRegister & BIT1)) {   // Lid is closed
+    EcRead (0x70, &PowerState);
+    if (!(PowerState & BIT1)) {   // Lid is closed
       EcCmd90Read (0x0A, &OutData);
       if (!(OutData & BIT1)) {
         EcCmd91Write (0x0A, OutData | BIT1);
       }
 
-      /* Clear events and go back to sleep */
+      /* Clear below events and go back to sleep */
       PchAcpiBaseGet (&ABase);
       /* Clear ABase PM1_STS - RW/1C set bits */
       Pm1Sts = IoRead16 (ABase + R_PCH_ACPI_PM1_STS);
@@ -78,6 +83,8 @@ EcInit (
       CpuDeadLoop ();
     }
   }
+
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
 }
 
 /**
@@ -105,15 +112,15 @@ GpioInitPostMem (
 {
   EFI_STATUS  Status;
 
-  DEBUG ((DEBUG_INFO, "GpioInitPostMem() Start\n"));
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
 
   Status = GpioConfigurePads (mGpioTableAspireVn7Dash572GSize, mGpioTableAspireVn7Dash572G);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "Failed to configure early GPIOs!\n"));
+    DEBUG ((DEBUG_ERROR, "Failed to configure GPIOs!\n"));
     return EFI_DEVICE_ERROR;
   }
 
-  DEBUG ((DEBUG_INFO, "GpioInitPostMem() End\n"));
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
 
@@ -128,6 +135,8 @@ AspireVn7Dash572GBoardInitBeforeSiliconInit (
   VOID
   )
 {
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
+
   GpioInitPostMem ();
   AspireVn7Dash572GInit ();
 
@@ -136,6 +145,7 @@ AspireVn7Dash572GBoardInitBeforeSiliconInit (
   ///
   LateSiliconInit ();
 
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
 
@@ -150,7 +160,10 @@ AspireVn7Dash572GBoardInitAfterSiliconInit (
   VOID
   )
 {
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
+
   EcInit ();
 
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPreMemLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPreMemLib.c
index d0125ebdbcb2..5be644fa72ae 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPreMemLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPreMemLib.c
@@ -106,7 +106,7 @@ GpioInitPreMem (
 {
   EFI_STATUS  Status;
 
-  DEBUG ((DEBUG_INFO, "GpioInitPreMem() Start\n"));
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
 
   Status = GpioConfigurePads (mGpioTableAspireVn7Dash572G_earlySize, mGpioTableAspireVn7Dash572G_early);
   if (EFI_ERROR (Status)) {
@@ -114,7 +114,7 @@ GpioInitPreMem (
     return EFI_DEVICE_ERROR;
   }
 
-  DEBUG ((DEBUG_INFO, "GpioInitPreMem() End\n"));
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
 
@@ -129,7 +129,7 @@ DgpuPowerOn (
 {
   UINT32         OutputVal;
 
-  DEBUG ((DEBUG_INFO, "DgpuPowerOn() Start\n"));
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
 
   GpioGetOutputValue (DGPU_PRESENT, &OutputVal);
   if (!OutputVal) {
@@ -146,7 +146,7 @@ DgpuPowerOn (
     GpioSetOutputValue (DGPU_PWR_EN, 1);    // Deassert dGPU_PWR_EN#
   }
 
-  DEBUG ((DEBUG_INFO, "DgpuPowerOn() End\n"));
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
 }
 
 /**
@@ -182,6 +182,8 @@ AspireVn7Dash572GBoardInitBeforeMemoryInit (
 {
   EFI_STATUS    Status;
 
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
+
   Status = GpioInitPreMem ();
   if (!EFI_ERROR (Status)) {
     DgpuPowerOn ();
@@ -206,6 +208,8 @@ AspireVn7Dash572GBoardInitBeforeMemoryInit (
   Status = PchInitializeReset ();
   ASSERT_EFI_ERROR (Status);
 
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
+
   return EFI_SUCCESS;
 }
 
@@ -222,6 +226,8 @@ AspireVn7Dash572GBoardInitAfterMemoryInit (
 {
   EFI_STATUS  Status;
 
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
+
   // BUGBUG: Workaround for a misbehaving system firmware not setting goIdle
   // - Based on prior investigation for coreboot, I suspect FSP
   if ((MmioRead32 (0xFED40044) & PTP_CRB_CONTROL_AREA_STATUS_TPM_IDLE) == 0) {
@@ -235,6 +241,8 @@ AspireVn7Dash572GBoardInitAfterMemoryInit (
     DEBUG ((DEBUG_WARN, "Failed to enable LGMR. Were ACPI tables built for LGMR memory map?\n"));
   }
 
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
+
   return EFI_SUCCESS;
 }
 
@@ -252,6 +260,8 @@ AspireVn7Dash572GBoardDebugInit (
 {
   UINT16  ABase;
 
+  DEBUG ((DEBUG_INFO, "%a() Start\n", __FUNCTION__));
+
   ///
   /// Do Early PCH init
   ///
@@ -272,6 +282,8 @@ AspireVn7Dash572GBoardDebugInit (
   DEBUG ((DEBUG_INFO, "ABase PM1_EN= 0x%x\n", IoRead16 (ABase + R_PCH_ACPI_PM1_EN)));
   DEBUG ((DEBUG_INFO, "ABase PM1_CNT= 0x%x\n", IoRead32 (ABase + R_PCH_ACPI_PM1_CNT)));
 
+  DEBUG ((DEBUG_INFO, "%a() End\n", __FUNCTION__));
+
   return EFI_SUCCESS;
 }
 
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPreMemLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPreMemLib.c
index 5f89d87e71f8..9a1b6bf47f0e 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPreMemLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPreMemLib.c
@@ -86,6 +86,7 @@ BoardInitBeforeTempRamExit (
   VOID
   )
 {
+  DEBUG ((DEBUG_INFO, "%a()\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
 
@@ -95,5 +96,6 @@ BoardInitAfterTempRamExit (
   VOID
   )
 {
+  DEBUG ((DEBUG_INFO, "%a()\n", __FUNCTION__));
   return EFI_SUCCESS;
 }
diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.c
index 6840531da986..ef04ea2feebf 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.c
@@ -60,12 +60,10 @@ SiliconPolicyUpdateLate (
   Status = gBS->LocateProtocol (&gGopPolicyProtocolGuid, NULL, (VOID **) &GopPolicy);
   if (!EFI_ERROR (Status)) {
     Status = GopPolicy->GetVbtData (&VbtAddress, &VbtSize);
-    if (!EFI_ERROR (Status) && GraphicsDxeConfig != NULL) {
+    if (!EFI_ERROR (Status)) {
+      DEBUG ((DEBUG_INFO, "Located VBT at 0x%x with size 0x%x\n", VbtAddress, VbtSize));
       GraphicsDxeConfig->VbtAddress = VbtAddress;
       GraphicsDxeConfig->Size = VbtSize;
-      DEBUG ((DEBUG_INFO, "Located VBT at 0x%x with size 0x%x\n", VbtAddress, VbtSize));
-    } else {
-      DEBUG ((DEBUG_ERROR, "No VBT found, or Policy == NULL; Status - %r\n", Status));
     }
   }
 
-- 
2.37.2


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [edk2-devel][edk2-platforms][PATCH v1 1/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Cleanup library includes
  2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 1/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Cleanup library includes Benjamin Doron
@ 2022-09-09 21:41   ` Isaac Oram
  0 siblings, 0 replies; 9+ messages in thread
From: Isaac Oram @ 2022-09-09 21:41 UTC (permalink / raw)
  To: Benjamin Doron, devel@edk2.groups.io
  Cc: Chaganty, Rangasai V, Chiu, Chasel, Desimone, Nathaniel L

Reviewed-by: Isaac Oram <isaac.w.oram@intel.com>

-----Original Message-----
From: Benjamin Doron <benjamin.doron00@gmail.com> 
Sent: Tuesday, September 6, 2022 10:43 AM
To: devel@edk2.groups.io
Cc: Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>; Chiu, Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>
Subject: [edk2-devel][edk2-platforms][PATCH v1 1/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Cleanup library includes

Remove unused includes, LibraryClasses and update a comment or two.

Cc: Sai Chaganty <rangasai.v.chaganty@intel.com>
Cc: Isaac Oram <isaac.w.oram@intel.com>
Cc: Chasel Chiu <chasel.chiu@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Benjamin Doron <benjamin.doron00@gmail.com>
---
 .../PcieDeviceTable.c                          |  1 -
 .../PeiBoardPolicyUpdate.c                     |  6 ++++--
 .../PeiPchPolicyUpdate.h                       |  3 ++-
 .../PeiPchPolicyUpdatePreMem.c                 |  1 -
 .../PeiSiliconPolicyUpdateLibFsp.inf           |  5 ++---
 .../BoardAcpiLib/DxeBoardAcpiTableLib.inf      |  5 +----
 .../SmmAspireVn7Dash572GAcpiEnableLib.c        |  9 +++++----
 .../BoardAcpiLib/SmmBoardAcpiEnableLib.inf     |  3 ++-
 .../Library/BoardEcLib/EcCommands.c            | 14 ++++++++------
 .../AspireVn7Dash572GHdaVerbTables.c           |  3 ++-
 .../BoardInitLib/PeiAspireVn7Dash572GInitLib.h |  3 +--
 .../PeiAspireVn7Dash572GInitPreMemLib.c        | 18 +++++++++---------
 .../BoardInitLib/PeiBoardInitPostMemLib.inf    |  4 +---
 .../BoardInitLib/PeiBoardInitPreMemLib.inf     |  5 +----
 .../AspireVn7Dash572G/OpenBoardPkg.fdf         |  3 ++-
 .../OpenBoardPkgBuildOption.dsc                |  4 ++--
 .../DxeGopPolicyInit.h                         |  3 ---
 .../DxeSaPolicyInit.h                          |  3 ---
 .../DxeSiliconPolicyUpdateLib.c                |  3 +--
 .../DxeSiliconPolicyUpdateLib.inf              |  2 ++
 20 files changed, 45 insertions(+), 53 deletions(-)

diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PcieDeviceTable.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PcieDeviceTable.c
index 205ca581c6f3..537fb5c8e4f4 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PcieDeviceTable.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/L
+++ ibrary/PeiSiliconPolicyUpdateLibFsp/PcieDeviceTable.c
@@ -7,7 +7,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 **/  #include "PeiPchPolicyUpdate.h"-#include <Library/PchPcieRpLib.h>  #define PCI_CLASS_NETWORK             0x02 #define PCI_CLASS_NETWORK_ETHERNET    0x00diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiBoardPolicyUpdate.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiBoardPolicyUpdate.c
index 81cd8b940f05..452c961b17ac 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiBoardPolicyUpdate.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/L
+++ ibrary/PeiSiliconPolicyUpdateLibFsp/PeiBoardPolicyUpdate.c
@@ -12,14 +12,16 @@
 #include <Library/PcdLib.h> #include <PchPolicyCommon.h> -/* TODO:+/*+ * TODO:  * - Validate PCH Sample policies: only SA one used by default.  * - Remove likely fuse-disabled devices when reset handling is committed?  * - Remove duplicate policy  *   - Consider updating some policies, rather than overriding. This could be factored into  *     BoardInitLib for deduplication  * - Copy initialised array, where sane- * - Set IgdDvmt50PreAlloc? */+ * - Set IgdDvmt50PreAlloc?+ */  #define SA_VR           0 #define IA_VR           1diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdate.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdate.h
index 5e720b0041e8..134188698077 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdate.h
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/L
+++ ibrary/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdate.h
@@ -17,10 +17,11 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include <Library/DebugLib.h> #include <Library/IoLib.h> #include <Library/MmPciLib.h>-#include <Ppi/SiPolicy.h>  #include <FspEas.h> #include <FspmUpd.h> #include <FspsUpd.h> +#include <PchPolicyCommon.h>+ #endifdiff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdatePreMem.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdatePreMem.c
index 2bc142c0e5ff..28e4e45375c2 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdatePreMem.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/L
+++ ibrary/PeiSiliconPolicyUpdateLibFsp/PeiPchPolicyUpdatePreMem.c
@@ -9,7 +9,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include "PeiPchPolicyUpdate.h" #include <Library/BaseMemoryLib.h> #include <Library/PchInfoLib.h>-#include <Library/PchPcrLib.h> #include <Library/PchHsioLib.h> #include <Library/PchPcieRpLib.h> #include <PchHsioPtssTables.h>diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf
index eac9344b0aa2..0e1b42c20cd8 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/Library/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/FspWrapper/L
+++ ibrary/PeiSiliconPolicyUpdateLibFsp/PeiSiliconPolicyUpdateLibFsp.inf
@@ -62,19 +62,18 @@
  [LibraryClasses.IA32]   FspWrapperApiLib-  FspWrapperPlatformLib   BaseMemoryLib   DebugLib-  HobLib   IoLib   PcdLib   MmPciLib-  ConfigBlockLib+  PciLib   PeiSaPolicyLib   PchInfoLib   PchHsioLib   PchPcieRpLib   SiPolicyLib+  MemoryAllocationLib   PeiLib  [Pcd]diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
index 0d8264554734..660afe9292ec 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/DxeBoardAcpiTableLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/Boar
+++ dAcpiLib/DxeBoardAcpiTableLib.inf
@@ -22,10 +22,7 @@
 #  [LibraryClasses]-  BaseLib-  IoLib-  PciLib-  AslUpdateLib+  PcdLib   EcLib  [Packages]diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmAspireVn7Dash572GAcpiEnableLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmAspireVn7Dash572GAcpiEnableLib.c
index 69e9c928ff69..fa2ed9745ea6 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmAspireVn7Dash572GAcpiEnableLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/Boar
+++ dAcpiLib/SmmAspireVn7Dash572GAcpiEnableLib.c
@@ -2,6 +2,7 @@
   Acer Aspire VN7-572G SMM Board ACPI Enable library  Copyright (c) 2017 - 2019, Intel Corporation. All rights reserved.<BR>+Copyright (c) 2021, Baruch Binyamin Doron<BR> SPDX-License-Identifier: BSD-2-Clause-Patent  **/@@ -20,8 +21,8 @@ AspireVn7Dash572GBoardEnableAcpi (
   EFI_STATUS  Status;    /* Tests at runtime show this re-enables charging and battery reporting-   * - Obtained somewhere from somewhere in vendor's SmmKbcDriver (or RtKbcDriver).-   *   Further reversing will be performed */+   * - Obtained from somewhere in vendor's SmmKbcDriver.+   *   Further information is needed */   Status = SendEcCommand (0xE9);  /* Vendor implements using ACPI "CMDB" register" */   if (EFI_ERROR (Status)) {     DEBUG ((DEBUG_ERROR, "%a(): SendEcCommand(0xE9) failed!\n", __FUNCTION__));@@ -48,8 +49,8 @@ AspireVn7Dash572GBoardDisableAcpi (
   EFI_STATUS  Status;    /* Tests at runtime show this disables charging and battery reporting-   * - Obtained somewhere from somewhere in vendor's SmmKbcDriver (or RtKbcDriver).-   *   Further reversing will be performed */+   * - Obtained from somewhere in vendor's SmmKbcDriver.+   *   Further information is needed */   Status = SendEcCommand (0xE9);  /* Vendor implements using ACPI "CMDB" register" */   if (EFI_ERROR (Status)) {     DEBUG ((DEBUG_ERROR, "%a(): SendEcCommand(0xE9) failed!\n", __FUNCTION__));diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
index 63a54e1830a5..5db00224dfce 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardAcpiLib/SmmBoardAcpiEnableLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/Boar
+++ dAcpiLib/SmmBoardAcpiEnableLib.inf
@@ -23,9 +23,10 @@
  [LibraryClasses]   BaseLib+  DebugLib   EcLib   IoLib-  PciLib+  PcdLib   MmPciLib   PchCycleDecodingLib diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
index 6e752b4e227e..54cfaba47b1b 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardEcLib/EcCommands.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/Boar
+++ dEcLib/EcCommands.c
@@ -13,7 +13,8 @@
 #include <Library/EcLib.h> #include <Library/IoLib.h> -/* Notes:+/*+ * Notes:  * - ACPI "CMDB": Writing to this offset is equivalent to sending commands.  *   The CMDx bytes contain the command parameters.  *@@ -21,9 +22,10 @@
  *   - Commands: 0x58, 0xE1 and 0xE2  *     - 0x51, 0x52: EC flash write?  *   - ACPI CMDB: 0x63 and 0x64, 0xC7- *     - 0x0B: Flash write (Boolean argument? Set in offset 0x0B?)+ *     - 0x0B: Flash lock/write (Set offset 0x0B?)+ *   - Key/recovery detection?  *- * Reversing vendor's protocols:+ * Vendor's protocols:  * - Only read and write are used.  * - Query, ACPI "CMDB" processing and command 58 are unused.  * - Equivalent KbcPeim is an unused PPI.@@ -32,9 +34,9 @@
  */  #define EC_INDEX_IO_PORT            0x1200-#define EC_INDEX_IO_HIGH_ADDR_PORT  EC_INDEX_IO_PORT+1-#define EC_INDEX_IO_LOW_ADDR_PORT   EC_INDEX_IO_PORT+2-#define EC_INDEX_IO_DATA_PORT       EC_INDEX_IO_PORT+3+#define EC_INDEX_IO_HIGH_ADDR_PORT  (EC_INDEX_IO_PORT + 1)+#define EC_INDEX_IO_LOW_ADDR_PORT   (EC_INDEX_IO_PORT + 2)+#define EC_INDEX_IO_DATA_PORT       (EC_INDEX_IO_PORT + 3)  /**   Reads a byte of EC RAM.diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/AspireVn7Dash572GHdaVerbTables.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/AspireVn7Dash572GHdaVerbTables.c
index 0573736060fa..cc7369f3484c 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/AspireVn7Dash572GHdaVerbTables.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/Boar
+++ dInitLib/AspireVn7Dash572GHdaVerbTables.c
@@ -2,6 +2,7 @@
   HDA Verb table for Acer Aspire VN7-572G  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>+Copyright (c) 2021, Baruch Binyamin Doron<BR> SPDX-License-Identifier: BSD-2-Clause-Patent  **/@@ -9,7 +10,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #ifndef _ASPIRE_VN7_572G_HDA_VERB_TABLES_H_ #define _ASPIRE_VN7_572G_HDA_VERB_TABLES_H_ -#include <Ppi/SiPolicy.h>+#include <PchPolicyCommon.h>  HDAUDIO_VERB_TABLE HdaVerbTableAlc255AspireVn7Dash572G = HDAUDIO_VERB_TABLE_INIT (   //diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitLib.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitLib.h
index 83789c90becf..51a7b714c463 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitLib.h
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/Boar
+++ dInitLib/PeiAspireVn7Dash572GInitLib.h
@@ -8,10 +8,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #ifndef _PEI_ASPIRE_VN7_572G_BOARD_INIT_LIB_H_ #define _PEI_ASPIRE_VN7_572G_BOARD_INIT_LIB_H_ -#include <Uefi.h>+#include <PiPei.h> #include <Library/BaseLib.h> #include <Library/PcdLib.h>-#include <Library/MemoryAllocationLib.h> #include <Library/DebugLib.h> #include <Library/GpioLib.h> #include <Ppi/SiPolicy.h>diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPreMemLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPreMemLib.c
index 1b4c6b484b43..d0125ebdbcb2 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiAspireVn7Dash572GInitPreMemLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/Boar
+++ dInitLib/PeiAspireVn7Dash572GInitPreMemLib.c
@@ -1,6 +1,7 @@
 /** @file  Copyright (c) 2017 - 2022, Intel Corporation. All rights reserved.<BR>+Copyright (c) 2021, Baruch Binyamin Doron<BR> SPDX-License-Identifier: BSD-2-Clause-Patent  **/@@ -8,7 +9,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include <PiPei.h> #include <Library/DebugLib.h> #include <Library/IoLib.h>-#include <Library/MemoryAllocationLib.h> #include <Library/PcdLib.h> #include <Library/PchCycleDecodingLib.h> #include <Library/PchPmcLib.h>@@ -16,7 +16,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #include <Library/PciLib.h> #include <Library/SiliconInitLib.h> #include <Library/TimerLib.h>-#include <Library/PeiLib.h>  #include <Library/GpioLib.h> #include <GpioPinsSklLp.h>@@ -47,12 +46,6 @@ GLOBAL_REMOVE_IF_UNREFERENCED const UINT16 RcompTargetAspireVn7Dash572G[SA_MRC_M
 #define DGPU_HOLD_RST	GPIO_SKL_LP_GPP_B4	/* Active low */ #define DGPU_PWR_EN	GPIO_SKL_LP_GPP_B21	/* Active low */ -EFI_STATUS-EFIAPI-AspireVn7Dash572GBoardDetect (-  VOID-  );- /**   Aspire VN7-572G board configuration init function for PEI pre-memory phase. @@ -75,7 +68,7 @@ AspireVn7Dash572GInitPreMem (
   //   PcdSet8S (PcdSaMiscUserBd, 5);     // ULT/ULX/Mobile Halo   PcdSet8S (PcdMrcCaVrefConfig, 2);  // DDR4: "VREF_CA to CH_A and VREF_DQ_B to CH_B"-  // TODO: Clear Dq/Dqs?+  // TODO: Search vendor FW for Dq/Dqs. Unnecessary if FSP detects LPDDR   PcdSetBoolS (PcdMrcDqPinsInterleaved, TRUE);    PcdSet32S (PcdMrcRcompResistor, (UINTN) RcompResistorAspireVn7Dash572G);@@ -241,9 +234,16 @@ AspireVn7Dash572GBoardInitAfterMemoryInit (
   if (EFI_ERROR (Status)) {     DEBUG ((DEBUG_WARN, "Failed to enable LGMR. Were ACPI tables built for LGMR memory map?\n"));   }+   return EFI_SUCCESS; } +EFI_STATUS+EFIAPI+AspireVn7Dash572GBoardDetect (+  VOID+  );+ EFI_STATUS EFIAPI AspireVn7Dash572GBoardDebugInit (diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPostMemLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPostMemLib.inf
index c8c49fa20dcc..7b68f66ac78b 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPostMemLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/Boar
+++ dInitLib/PeiBoardInitPostMemLib.inf
@@ -10,7 +10,7 @@
 [Defines]   INF_VERSION                    = 0x00010005   BASE_NAME                      = PeiBoardPostMemInitLib-  FILE_GUID                      = 7fcc3900-d38d-419f-826b-72481e8b5509+  FILE_GUID                      = 7FCC3900-D38D-419F-826B-72481E8B5509   MODULE_TYPE                    = BASE   VERSION_STRING                 = 1.0   LIBRARY_CLASS                  = BoardInitLib@@ -18,8 +18,6 @@
 [LibraryClasses]   BaseLib   DebugLib-  BaseMemoryLib-  MemoryAllocationLib   PcdLib   SiliconInitLib   PchCycleDecodingLibdiff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPreMemLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPreMemLib.inf
index c53114e15450..a3164870ef9b 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/BoardInitLib/PeiBoardInitPreMemLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Library/Boar
+++ dInitLib/PeiBoardInitPreMemLib.inf
@@ -10,7 +10,7 @@
 [Defines]   INF_VERSION                    = 0x00010005   BASE_NAME                      = PeiBoardInitPreMemLib-  FILE_GUID                      = ec3675bc-1470-417d-826e-37378140213d+  FILE_GUID                      = EC3675BC-1470-417D-826E-37378140213D   MODULE_TYPE                    = BASE   VERSION_STRING                 = 1.0   LIBRARY_CLASS                  = BoardInitLib@@ -18,8 +18,6 @@
 [LibraryClasses]   BaseLib   DebugLib-  BaseMemoryLib-  MemoryAllocationLib   PcdLib   SiliconInitLib   TimerLib@@ -30,7 +28,6 @@
   EcLib   BoardEcLib   GpioLib-  PeiLib   PeiServicesLib   PchPmcLib diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.fdf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.fdf
index 864d5561d7d8..b59d9a4f24e1 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg.fdf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg
+++ .fdf
@@ -279,7 +279,7 @@ INF IntelFsp2WrapperPkg/FspsWrapperPeim/FspsWrapperPeim.inf
 INF $(PLATFORM_PACKAGE)/PlatformInit/SiliconPolicyPei/SiliconPolicyPeiPostMem.inf  !if gSiPkgTokenSpaceGuid.PcdPeiDisplayEnable == TRUE-FILE FREEFORM = 4ad46122-ffeb-4a52-bfb0-518cfca02db0 {+FILE FREEFORM = 4AD46122-FFEB-4A52-BFB0-518CFCA02DB0 {   SECTION RAW = $(BOARD)/Vbt.bin   SECTION UI  = "Vbt" }@@ -346,6 +346,7 @@ APRIORI DXE {
 INF  UefiCpuPkg/CpuDxe/CpuDxe.inf INF  MdeModulePkg/Bus/Pci/PciHostBridgeDxe/PciHostBridgeDxe.inf +# TODO: Add NvmExpressDxe if supporting Newgate and RayleighSLS INF  MdeModulePkg/Bus/Pci/SataControllerDxe/SataControllerDxe.inf INF  MdeModulePkg/Bus/Ata/AtaBusDxe/AtaBusDxe.inf INF  MdeModulePkg/Bus/Ata/AtaAtapiPassThru/AtaAtapiPassThru.infdiff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkgBuildOption.dsc b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkgBuildOption.dsc
index b1a04c474845..6e2053d67734 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkgBuildOption.dsc
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/OpenBoardPkg
+++ BuildOption.dsc
@@ -84,6 +84,7 @@ DEFINE DSC_PLTPKG_FEATURE_BUILD_OPTIONS = $(DSC_PLTPKG_FEATURE_BUILD_OPTIONS) $(
 DEFINE DSC_PLTPKG_FEATURE_BUILD_OPTIONS = $(DSC_PLTPKG_FEATURE_BUILD_OPTIONS) $(DSC_ACPI_BUILD_OPTIONS) $(UP_SERVER_SUPPORT_BUILD_OPTIONS) $(USBTYPEC_BUILD_OPTION) $(SINITBIN_BUILD_OPTION) $(MINTREE_FLAG_BUILD_OPTION)  # FIXME: $(DSC_PLTPKG_FEATURE_BUILD_OPTIONS) is passed multiple times+# BUGBUG: `-Wl,--allow-multiple-definition` breaks CLANG build [BuildOptions.Common.EDKII]  #@@ -141,8 +142,7 @@ MSFT:  *_*_X64_ASLCC_FLAGS    = $(DSC_PLTPKG_FEATURE_BUILD_OPTIONS)
   MSFT:*_*_*_DLINK_FLAGS = /ALIGN:4096   GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000 -# FIXME: Protection broken, but works on UefiPayload, and not related to-# FspWrapperNotifyDxe. Cannot be related to SMM?+# FIXME: Protection broken, yet works on UefiPayload. Consider diffing module/library include lists (unrelated to FspWrapperNotifyDxe). # Force PE/COFF sections to be aligned at 4KB boundaries to support NX protection [BuildOptions.common.EDKII.DXE_DRIVER, BuildOptions.common.EDKII.DXE_CORE, BuildOptions.common.EDKII.UEFI_DRIVER, BuildOptions.common.EDKII.UEFI_APPLICATION]   #MSFT:*_*_*_DLINK_FLAGS = /ALIGN:4096diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeGopPolicyInit.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeGopPolicyInit.h
index 63cad5e3753f..56cab1df9b1d 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeGopPolicyInit.h
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Libra
+++ ry/DxeSiliconPolicyUpdateLib/DxeGopPolicyInit.h
@@ -9,11 +9,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #define _GOP_POLICY_INIT_DXE_H_  #include <Protocol/FirmwareVolume2.h>-#include <Library/UefiLib.h> #include <Library/BaseLib.h>-#include <Library/DxeServicesTableLib.h> #include <Library/UefiBootServicesTableLib.h>-#include <Library/UefiRuntimeServicesTableLib.h> #include <Library/BaseMemoryLib.h> #include <Library/DebugLib.h> #include <Library/PcdLib.h>diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSaPolicyInit.h b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSaPolicyInit.h
index 801387b9476f..88a507547f69 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSaPolicyInit.h
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Libra
+++ ry/DxeSiliconPolicyUpdateLib/DxeSaPolicyInit.h
@@ -8,12 +8,9 @@ SPDX-License-Identifier: BSD-2-Clause-Patent  #ifndef _SA_POLICY_INIT_DXE_H_ #define _SA_POLICY_INIT_DXE_H_ -#include <Library/BaseMemoryLib.h>-#include <Library/MemoryAllocationLib.h> #include <Library/DebugLib.h> #include <Library/UefiBootServicesTableLib.h> #include <Protocol/SaPolicy.h>-#include <Library/DxeSaPolicyLib.h>  #include <SaAccess.h> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.c b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.c
index 6298bb53e65d..6840531da986 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.c
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Libra
+++ ry/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.c
@@ -1,14 +1,13 @@
 /** @file  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>+Copyright (c) 2021, Baruch Binyamin Doron<BR> SPDX-License-Identifier: BSD-2-Clause-Patent  **/  #include <Library/ConfigBlockLib.h> #include <Library/SiliconPolicyUpdateLib.h>-#include <Library/PcdLib.h>-#include <Library/DebugLib.h> #include <Protocol/GopPolicy.h> #include <Protocol/SaPolicy.h> diff --git a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.inf b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.inf
index 63ac194cd0d5..989796cf8244 100644
--- a/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Library/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.inf
+++ b/Platform/Intel/KabylakeOpenBoardPkg/AspireVn7Dash572G/Policy/Libra
+++ ry/DxeSiliconPolicyUpdateLib/DxeSiliconPolicyUpdateLib.inf
@@ -17,6 +17,8 @@
  [LibraryClasses]   BaseLib+  BaseMemoryLib+  UefiBootServicesTableLib   PcdLib   DebugLib   ConfigBlockLib-- 
2.37.2


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-09-09 21:41 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-09-06 17:42 [edk2-devel][edk2-platforms][PATCH v1 0/7] Benjamin Doron
2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 1/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Cleanup library includes Benjamin Doron
2022-09-09 21:41   ` Isaac Oram
2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 2/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Enhance the build-logic Benjamin Doron
2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 3/7] KabylakeOpenBoardPkg/AspireVn7Dash572G/Acpi: Improvements for EC ACPI Benjamin Doron
2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 4/7] KabylakeOpenBoardPkg/AspireVn7Dash572G/DxeBoardInitLib: Resets notify EC Benjamin Doron
2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 5/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Use Setup to control security Benjamin Doron
2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 6/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Improve board detection Benjamin Doron
2022-09-06 17:42 ` [edk2-devel][edk2-platforms][PATCH v1 7/7] KabylakeOpenBoardPkg/AspireVn7Dash572G: Align DEBUG() use Benjamin Doron

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox