public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support
@ 2021-02-17 21:32 Michael Kubacki
  2021-02-17 21:32 ` [PATCH v3 1/5] UefiCpuPkg/SmmCpuFeaturesLib: Move multi-instance function decl to header Michael Kubacki
                   ` (7 more replies)
  0 siblings, 8 replies; 13+ messages in thread
From: Michael Kubacki @ 2021-02-17 21:32 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek, Rahul Kumar

From: Michael Kubacki <michael.kubacki@microsoft.com>

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

The present SmmCpuFeaturesLib implementation in UefiCpuPkg can be
useful for IA32/X64 platforms that need a library instance for
a Standalone MM environment. Much of the logic can be reused and a
new INF can isolate the differences unique to Standalone MM.

This patch series contains an initial set of changes for cleaning
up pre-existing design issues in the library. The final two patches
contain changes needed for Standalone MM support.

Here's an overview of how the three library instances are organized
that may be a useful reference (provided by Laszlo):

                    Traditional,  Traditional,  Standalone,
                    no STM        STM           no STM

Entry point type   DXE           DXE           MM

Lib inst. init.    basic         STM           basic

Processor init.    basic         STM           basic

PCD access         any           any           fixed

* Traditional no STM = SmmCpuFeaturesLib.inf
* Traditional STM = SmmCpuFeaturesLibStm.inf
* Standalone no STM = StandaloneMmCpuFeaturesLib.inf

V3 changes:

  PATCH v3 2/5 is a new patch in the series that renames the file
  SmmCpuFeaturesLib.c to SmmCpuFeaturesLibCommon.c to more clearly
  identify implementation in the file as shared between all library
  instances.

  PATCH v3 3/5 adds a new source file SmmCpuFeaturesLib.c that
  contains the constructor specific to the Traditional MM no
  STM library instance. This was previously implemented in a
  file built by the Standalone MM instance and while not
  harmful, it was not clean.

  PATCH v3 4/5 updates "@retval" to "@return" in the documentation
  for GetCpuMaxLogicalProcessorNumber() since it is not a constant
  return value.
  
  PATCH v3 5/5 contains a commit message update to note that all
  instances of "PiSmm.h" in the library source files have been
  updated to "PiMm.h" for consistency throughout the library.

V2 changes:

  Due to some pre-existing design issues in the library that
  affected a single v1 patch that add Standalone MM support,
  it was suggested to first address those issues and then add the
  new INF StandaloneMmCpuFeaturesLib.inf.

  To address these concerns, the following v1 patch was converted
  into a v2 patch series:
  https://edk2.groups.io/g/devel/message/71626

  The first two patches in v2 primarily addressed those concerns.

  PATCH v2 1/4 and PATCH v2 2/4 focused on fixing pre-existing
  design issues.

  PATCH v2 3/4 and PATCH v2 4/4 focused on the changes needed to add
  Standalone MM support.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

Michael Kubacki (5):
  UefiCpuPkg/SmmCpuFeaturesLib: Move multi-instance function decl to
    header
  UefiCpuPkg/SmmCpuFeaturesLib: Rename SmmCpuFeaturesLib.c
  UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors
  UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber
  UefiCpuPkg/SmmCpuFeaturesLib: Add Standalone MM support

 UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c                                      |   2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c                                       | 608 +-------------------
 UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => SmmCpuFeaturesLibCommon.c}        |  36 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c                                  |   3 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c                                                  |  26 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c                              |  50 ++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c                             |  28 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c                                       |   2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h                                          |  48 ++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf                                     |   3 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf                                  |   4 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.inf => StandaloneMmCpuFeaturesLib.inf} |  22 +-
 UefiCpuPkg/UefiCpuPkg.dsc                                                                      |   1 +
 13 files changed, 172 insertions(+), 661 deletions(-)
 copy UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => SmmCpuFeaturesLibCommon.c} (93%)
 create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c
 create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c
 create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
 copy UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.inf => StandaloneMmCpuFeaturesLib.inf} (53%)

-- 
2.28.0.windows.1


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

* [PATCH v3 1/5] UefiCpuPkg/SmmCpuFeaturesLib: Move multi-instance function decl to header
  2021-02-17 21:32 [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
@ 2021-02-17 21:32 ` Michael Kubacki
  2021-02-17 21:32 ` [PATCH v3 2/5] UefiCpuPkg/SmmCpuFeaturesLib: Rename SmmCpuFeaturesLib.c Michael Kubacki
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 13+ messages in thread
From: Michael Kubacki @ 2021-02-17 21:32 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek, Rahul Kumar

From: Michael Kubacki <michael.kubacki@microsoft.com>

FinishSmmCpuFeaturesInitializeProcessor() is a multi-instance
internal library function that is currently not declared in a
header file but embedded in "SmmCpuFeaturesLib.c".

This change cleans up the declaration moving it to a new header
file "CpuFeaturesLib.h" and removing the local declaration in
"SmmCpuFeaturesLib.c".

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c      | 11 +---------
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c |  1 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c                 |  1 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h         | 22 ++++++++++++++++++++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf    |  1 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf |  1 +
 6 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 8fed18cf0e17..75bde752785a 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -15,6 +15,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #include <Library/DebugLib.h>
 #include <Register/Intel/Cpuid.h>
 #include <Register/Intel/SmramSaveStateMap.h>
+#include "CpuFeaturesLib.h"
 
 //
 // Machine Specific Registers (MSRs)
@@ -35,16 +36,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 #define SMM_FEATURES_LIB_IA32_MCA_CAP              0x17D
 #define   SMM_CODE_ACCESS_CHK_BIT                  BIT58
 
-/**
-  Internal worker function that is called to complete CPU initialization at the
-  end of SmmCpuFeaturesInitializeProcessor().
-
-**/
-VOID
-FinishSmmCpuFeaturesInitializeProcessor (
-  VOID
-  );
-
 //
 // Set default value to assume SMRR is not supported
 //
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c
index 3e63c5e27f98..c562582ccee0 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c
@@ -9,6 +9,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 #include <PiSmm.h>
 #include <Library/SmmCpuFeaturesLib.h>
+#include "CpuFeaturesLib.h"
 
 /**
   Internal worker function that is called to complete CPU initialization at the
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
index f7f8afacffb5..b5aad41fdb64 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
@@ -21,6 +21,7 @@
 
 #include <Protocol/MpService.h>
 
+#include "CpuFeaturesLib.h"
 #include "SmmStm.h"
 
 #define TXT_EVTYPE_BASE                  0x400
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
new file mode 100644
index 000000000000..4645bbb066c9
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
@@ -0,0 +1,22 @@
+/** @file
+  Internal library function definitions.
+
+  Copyright (c) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef _CPU_FEATURES_LIB_H_
+#define _CPU_FEATURES_LIB_H_
+
+/**
+  Internal worker function that is called to complete CPU initialization at the
+  end of SmmCpuFeaturesInitializeProcessor().
+
+**/
+VOID
+FinishSmmCpuFeaturesInitializeProcessor (
+  VOID
+  );
+
+#endif
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index dd828baf69cb..a6d8467d26aa 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -17,6 +17,7 @@ [Defines]
   CONSTRUCTOR                    = SmmCpuFeaturesLibConstructor
 
 [Sources]
+  CpuFeaturesLib.h
   SmmCpuFeaturesLib.c
   SmmCpuFeaturesLibNoStm.c
 
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
index 50b9cc871302..89cd252ef44e 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
@@ -18,6 +18,7 @@ [Defines]
   CONSTRUCTOR                    = SmmCpuFeaturesLibStmConstructor
 
 [Sources]
+  CpuFeaturesLib.h
   SmmCpuFeaturesLib.c
   SmmStm.c
   SmmStm.h
-- 
2.28.0.windows.1


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

* [PATCH v3 2/5] UefiCpuPkg/SmmCpuFeaturesLib: Rename SmmCpuFeaturesLib.c
  2021-02-17 21:32 [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
  2021-02-17 21:32 ` [PATCH v3 1/5] UefiCpuPkg/SmmCpuFeaturesLib: Move multi-instance function decl to header Michael Kubacki
@ 2021-02-17 21:32 ` Michael Kubacki
  2021-02-19 20:31   ` Laszlo Ersek
  2021-02-17 21:32 ` [PATCH v3 3/5] UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors Michael Kubacki
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Michael Kubacki @ 2021-02-17 21:32 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek, Rahul Kumar

From: Michael Kubacki <michael.kubacki@microsoft.com>

This change renames SmmCpuFeaturesLib.c to SmmCpuFeaturesLibCommon.c
to better convey that this file contains library implementation
common to all library instances.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => SmmCpuFeaturesLibCommon.c} | 2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf                              | 2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf                           | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
similarity index 96%
rename from UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
rename to UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
index 75bde752785a..36c48310c31e 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
@@ -1,5 +1,5 @@
 /** @file
-The CPU specific programming for PiSmmCpuDxeSmm module.
+Implementation shared across all library instances.
 
 Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index a6d8467d26aa..7ebb0b0ef011 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -18,7 +18,7 @@ [Defines]
 
 [Sources]
   CpuFeaturesLib.h
-  SmmCpuFeaturesLib.c
+  SmmCpuFeaturesLibCommon.c
   SmmCpuFeaturesLibNoStm.c
 
 [Packages]
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
index 89cd252ef44e..78f8e4b42c44 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
@@ -19,7 +19,7 @@ [Defines]
 
 [Sources]
   CpuFeaturesLib.h
-  SmmCpuFeaturesLib.c
+  SmmCpuFeaturesLibCommon.c
   SmmStm.c
   SmmStm.h
 
-- 
2.28.0.windows.1


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

* [PATCH v3 3/5] UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors
  2021-02-17 21:32 [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
  2021-02-17 21:32 ` [PATCH v3 1/5] UefiCpuPkg/SmmCpuFeaturesLib: Move multi-instance function decl to header Michael Kubacki
  2021-02-17 21:32 ` [PATCH v3 2/5] UefiCpuPkg/SmmCpuFeaturesLib: Rename SmmCpuFeaturesLib.c Michael Kubacki
@ 2021-02-17 21:32 ` Michael Kubacki
  2021-02-19 20:39   ` Laszlo Ersek
  2021-02-17 21:32 ` [PATCH v3 4/5] UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber Michael Kubacki
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Michael Kubacki @ 2021-02-17 21:32 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek, Rahul Kumar

From: Michael Kubacki <michael.kubacki@microsoft.com>

There's currently two library instances:
  1. SmmCpuFeaturesLib
  2. SmmCpuFeaturesLibStm

There's two constructor functions:
  1. SmmCpuFeaturesLibConstructor()
  2. SmmCpuFeaturesLibStmConstructor()

SmmCpuFeaturesLibConstructor() is called by
SmmCpuFeaturesLibStmConstructor() since the functionality in that
function is required by both library instances.

The declaration for SmmCpuFeaturesLibConstructor() is embedded in
"SmmStm.c" instead of being declared in a header file. Further,
that constructor function is called by the STM specific constructor.

This change moves the common code to a function called
CpuFeaturesLibInitialization() which is declared in an internal
library header file "CpuFeaturesLib.h". Each constructor simply
calls this function to perform the common functionality.

Additionally, SmmCpuFeaturesLibConstructor() is moved from
SmmCpuFeaturesLibNoStm.c into a instance-specific file allowing
SmmCpuFeaturesLibNoStm.c to contain no STM implementation agnostic
to a particular library instance.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c       | 31 ++++++++++++++++++++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c | 19 +++++-------
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c                  | 23 ++-------------
 UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h          | 12 ++++++++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf     |  1 +
 5 files changed, 54 insertions(+), 32 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
new file mode 100644
index 000000000000..00948a191fad
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -0,0 +1,31 @@
+/** @file
+Implementation specific to the SmmCpuFeatureLib library instance.
+
+Copyright (c) Microsoft Corporation.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiSmm.h>
+#include "CpuFeaturesLib.h"
+
+/**
+  The constructor function for the Traditional MM library instance without STM.
+
+  @param[in]  ImageHandle  The firmware allocated handle for the EFI image.
+  @param[in]  SystemTable  A pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS      The constructor always returns EFI_SUCCESS.
+
+**/
+EFI_STATUS
+EFIAPI
+SmmCpuFeaturesLibConstructor (
+  IN EFI_HANDLE        ImageHandle,
+  IN EFI_SYSTEM_TABLE  *SystemTable
+  )
+{
+  CpuFeaturesLibInitialization ();
+
+  return EFI_SUCCESS;
+}
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
index 36c48310c31e..7a919c5ee70f 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
@@ -2,6 +2,7 @@
 Implementation shared across all library instances.
 
 Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -63,19 +64,15 @@ BOOLEAN  mNeedConfigureMtrrs = TRUE;
 BOOLEAN  *mSmrrEnabled;
 
 /**
-  The constructor function
+  Performs library initialization.
 
-  @param[in]  ImageHandle  The firmware allocated handle for the EFI image.
-  @param[in]  SystemTable  A pointer to the EFI System Table.
-
-  @retval EFI_SUCCESS      The constructor always returns EFI_SUCCESS.
+  This initialization function contains common functionality shared betwen all
+  library instance constructors.
 
 **/
-EFI_STATUS
-EFIAPI
-SmmCpuFeaturesLibConstructor (
-  IN EFI_HANDLE        ImageHandle,
-  IN EFI_SYSTEM_TABLE  *SystemTable
+VOID
+CpuFeaturesLibInitialization (
+  VOID
   )
 {
   UINT32  RegEax;
@@ -162,8 +159,6 @@ SmmCpuFeaturesLibConstructor (
   //
   mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
   ASSERT (mSmrrEnabled != NULL);
-
-  return EFI_SUCCESS;
 }
 
 /**
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
index b5aad41fdb64..dcc2e9f9a09a 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
@@ -30,22 +30,6 @@
 #define RDWR_ACCS             3
 #define FULL_ACCS             7
 
-/**
-  The constructor function
-
-  @param[in]  ImageHandle  The firmware allocated handle for the EFI image.
-  @param[in]  SystemTable  A pointer to the EFI System Table.
-
-  @retval EFI_SUCCESS      The constructor always returns EFI_SUCCESS.
-
-**/
-EFI_STATUS
-EFIAPI
-SmmCpuFeaturesLibConstructor (
-  IN EFI_HANDLE        ImageHandle,
-  IN EFI_SYSTEM_TABLE  *SystemTable
-  );
-
 EFI_HANDLE  mStmSmmCpuHandle = NULL;
 
 BOOLEAN mLockLoadMonitor = FALSE;
@@ -112,7 +96,7 @@ UINTN  mMsegSize = 0;
 BOOLEAN  mStmConfigurationTableInitialized = FALSE;
 
 /**
-  The constructor function
+  The constructor function for the Traditional MM library instance with STM.
 
   @param[in]  ImageHandle  The firmware allocated handle for the EFI image.
   @param[in]  SystemTable  A pointer to the EFI System Table.
@@ -138,10 +122,9 @@ SmmCpuFeaturesLibStmConstructor (
   SmmCpuFeaturesLibStmSmiEntryFixupAddress ();
 
   //
-  // Call the common constructor function
+  // Perform library initialization common across all instances
   //
-  Status = SmmCpuFeaturesLibConstructor (ImageHandle, SystemTable);
-  ASSERT_EFI_ERROR (Status);
+  CpuFeaturesLibInitialization ();
 
   //
   // Lookup the MP Services Protocol
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
index 4645bbb066c9..f9a758e82558 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
@@ -9,6 +9,18 @@
 #ifndef _CPU_FEATURES_LIB_H_
 #define _CPU_FEATURES_LIB_H_
 
+/**
+  Performs library initialization.
+
+  This initialization function contains common functionality shared betwen all
+  library instance constructors.
+
+**/
+VOID
+CpuFeaturesLibInitialization (
+  VOID
+  );
+
 /**
   Internal worker function that is called to complete CPU initialization at the
   end of SmmCpuFeaturesInitializeProcessor().
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index 7ebb0b0ef011..ddd00eeceb84 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -18,6 +18,7 @@ [Defines]
 
 [Sources]
   CpuFeaturesLib.h
+  SmmCpuFeaturesLib.c
   SmmCpuFeaturesLibCommon.c
   SmmCpuFeaturesLibNoStm.c
 
-- 
2.28.0.windows.1


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

* [PATCH v3 4/5] UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber
  2021-02-17 21:32 [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
                   ` (2 preceding siblings ...)
  2021-02-17 21:32 ` [PATCH v3 3/5] UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors Michael Kubacki
@ 2021-02-17 21:32 ` Michael Kubacki
  2021-02-19 20:43   ` Laszlo Ersek
  2021-02-17 21:32 ` [PATCH v3 5/5] UefiCpuPkg/SmmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Michael Kubacki @ 2021-02-17 21:32 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek, Rahul Kumar

From: Michael Kubacki <michael.kubacki@microsoft.com>

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

Adds a new function called GetCpuMaxLogicalProcessorNumber() to
return the number of maximum CPU logical processors (currently
gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber).

This allows the the mechanism used to retrieve the CPU maximum
logical processor number to be abstracted from the logic that
needs the value.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
---
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c     |  2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c | 28 ++++++++++++++++++++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h              | 14 ++++++++++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf         |  1 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf      |  1 +
 5 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
index 7a919c5ee70f..50379f3aea19 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
@@ -157,7 +157,7 @@ CpuFeaturesLibInitialization (
   //
   // Allocate array for state of SMRR enable on all CPUs
   //
-  mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
+  mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * GetCpuMaxLogicalProcessorNumber ());
   ASSERT (mSmrrEnabled != NULL);
 }
 
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c
new file mode 100644
index 000000000000..b7a5c1926e4d
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c
@@ -0,0 +1,28 @@
+/** @file
+  Traditional MM CPU specific programming.
+
+  Copyright (c) Microsoft Corporation.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <Base.h>
+#include <Library/PcdLib.h>
+#include "CpuFeaturesLib.h"
+
+/**
+  Gets the maximum number of logical processors from the PCD PcdCpuMaxLogicalProcessorNumber.
+
+  This access is abstracted from the PCD services to enforce that the PCD be
+  FixedAtBuild in the Standalone MM build of this driver.
+
+  @return  The value of PcdCpuMaxLogicalProcessorNumber.
+
+**/
+UINT32
+GetCpuMaxLogicalProcessorNumber (
+  VOID
+  )
+{
+  return PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
+}
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
index f9a758e82558..2b6bfa899a48 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
@@ -31,4 +31,18 @@ FinishSmmCpuFeaturesInitializeProcessor (
   VOID
   );
 
+/**
+  Gets the maximum number of logical processors from the PCD PcdCpuMaxLogicalProcessorNumber.
+
+  This access is abstracted from the PCD services to enforce that the PCD be
+  FixedAtBuild in the Standalone MM build of this driver.
+
+  @return  The value of PcdCpuMaxLogicalProcessorNumber.
+
+**/
+UINT32
+GetCpuMaxLogicalProcessorNumber (
+  VOID
+  );
+
 #endif
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
index ddd00eeceb84..35292dac31ba 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
@@ -21,6 +21,7 @@ [Sources]
   SmmCpuFeaturesLib.c
   SmmCpuFeaturesLibCommon.c
   SmmCpuFeaturesLibNoStm.c
+  TraditionalMmCpuFeaturesLib.c
 
 [Packages]
   MdePkg/MdePkg.dec
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
index 78f8e4b42c44..022351f59320 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
@@ -22,6 +22,7 @@ [Sources]
   SmmCpuFeaturesLibCommon.c
   SmmStm.c
   SmmStm.h
+  TraditionalMmCpuFeaturesLib.c
 
 [Sources.Ia32]
   Ia32/SmmStmSupport.c
-- 
2.28.0.windows.1


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

* [PATCH v3 5/5] UefiCpuPkg/SmmCpuFeaturesLib: Add Standalone MM support
  2021-02-17 21:32 [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
                   ` (3 preceding siblings ...)
  2021-02-17 21:32 ` [PATCH v3 4/5] UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber Michael Kubacki
@ 2021-02-17 21:32 ` Michael Kubacki
  2021-02-19 20:43   ` Laszlo Ersek
  2021-03-05 14:39 ` [edk2-devel] [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: " Laszlo Ersek
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 13+ messages in thread
From: Michael Kubacki @ 2021-02-17 21:32 UTC (permalink / raw)
  To: devel; +Cc: Eric Dong, Ray Ni, Laszlo Ersek, Rahul Kumar

From: Michael Kubacki <michael.kubacki@microsoft.com>

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

Adds an INF for StandaloneMmCpuFeaturesLib, which supports building
the SmmCpuFeaturesLib code for Standalone MM. Minimal code changes
are made to allow reuse of existing code for Standalone MM.

The original INF file names are left intact (continue to use SMM
terminology) to retain backward compatibility with platforms that
use those INFs. Similarly, the pre-existing C file names are
unchanged to be consistent with the INF file names.

Note that all references in library source files to PiSmm.h have
been changed to PiMm.h for consistency.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c           |  2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c            |  2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c      |  2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c       |  2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c                       |  2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c   | 50 ++++++++++++++++++++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c            |  2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf | 38 +++++++++++++++
 UefiCpuPkg/UefiCpuPkg.dsc                                           |  1 +
 9 files changed, 95 insertions(+), 6 deletions(-)

diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c
index 399ddd742dd7..24bd6d0a946d 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c
@@ -6,7 +6,7 @@
 
 **/
 
-#include <PiSmm.h>
+#include <PiMm.h>
 #include <Library/DebugLib.h>
 
 #include "SmmStm.h"
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
index 00948a191fad..29e3b650acee 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
@@ -6,7 +6,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include <PiSmm.h>
+#include <PiMm.h>
 #include "CpuFeaturesLib.h"
 
 /**
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
index 50379f3aea19..fa3a866ab6f6 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
@@ -7,7 +7,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include <PiSmm.h>
+#include <PiMm.h>
 #include <Library/SmmCpuFeaturesLib.h>
 #include <Library/BaseLib.h>
 #include <Library/MtrrLib.h>
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c
index c562582ccee0..823dbe906cb8 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c
@@ -7,7 +7,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
 
-#include <PiSmm.h>
+#include <PiMm.h>
 #include <Library/SmmCpuFeaturesLib.h>
 #include "CpuFeaturesLib.h"
 
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
index dcc2e9f9a09a..c92387625ddc 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
@@ -6,7 +6,7 @@
 
 **/
 
-#include <PiSmm.h>
+#include <PiMm.h>
 #include <Library/BaseLib.h>
 #include <Library/BaseMemoryLib.h>
 #include <Library/MemoryAllocationLib.h>
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c
new file mode 100644
index 000000000000..dec6e53e5b55
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c
@@ -0,0 +1,50 @@
+/** @file
+Standalone MM CPU specific programming.
+
+Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.<BR>
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiMm.h>
+#include <Library/PcdLib.h>
+#include "CpuFeaturesLib.h"
+
+/**
+  Gets the maximum number of logical processors from the PCD PcdCpuMaxLogicalProcessorNumber.
+
+  This access is abstracted from the PCD services to enforce that the PCD be
+  FixedAtBuild in the Standalone MM build of this driver.
+
+  @return  The value of PcdCpuMaxLogicalProcessorNumber.
+
+**/
+UINT32
+GetCpuMaxLogicalProcessorNumber (
+  VOID
+  )
+{
+  return FixedPcdGet32 (PcdCpuMaxLogicalProcessorNumber);
+}
+
+/**
+  The Standalone MM library constructor.
+
+  @param[in] ImageHandle  Image handle of this driver.
+  @param[in] SystemTable  A Pointer to the EFI System Table.
+
+  @retval EFI_SUCCESS     This constructor always returns success.
+
+**/
+EFI_STATUS
+EFIAPI
+StandaloneMmCpuFeaturesLibConstructor (
+  IN EFI_HANDLE           ImageHandle,
+  IN EFI_MM_SYSTEM_TABLE  *SystemTable
+  )
+{
+  CpuFeaturesLibInitialization ();
+
+  return EFI_SUCCESS;
+}
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c
index aacc1455a90c..c8a3f58e0ca5 100644
--- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c
@@ -6,7 +6,7 @@
 
 **/
 
-#include <PiSmm.h>
+#include <PiMm.h>
 #include <Library/DebugLib.h>
 
 #include "SmmStm.h"
diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
new file mode 100644
index 000000000000..ec97041d8b75
--- /dev/null
+++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
@@ -0,0 +1,38 @@
+## @file
+#  Standalone MM CPU specific programming.
+#
+#  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) Microsoft Corporation.<BR>
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = StandaloneMmCpuFeaturesLib
+  MODULE_UNI_FILE                = SmmCpuFeaturesLib.uni
+  FILE_GUID                      = BB554A2D-F5DF-41D3-8C62-46476A2B2B18
+  MODULE_TYPE                    = MM_STANDALONE
+  VERSION_STRING                 = 1.0
+  PI_SPECIFICATION_VERSION       = 0x00010032
+  LIBRARY_CLASS                  = SmmCpuFeaturesLib
+  CONSTRUCTOR                    = StandaloneMmCpuFeaturesLibConstructor
+
+[Sources]
+  CpuFeaturesLib.h
+  StandaloneMmCpuFeaturesLib.c
+  SmmCpuFeaturesLibCommon.c
+  SmmCpuFeaturesLibNoStm.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  UefiCpuPkg/UefiCpuPkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  MemoryAllocationLib
+  PcdLib
+
+[FixedPcd]
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## SOMETIMES_CONSUMES
diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
index 9128cef076dd..7db419471deb 100644
--- a/UefiCpuPkg/UefiCpuPkg.dsc
+++ b/UefiCpuPkg/UefiCpuPkg.dsc
@@ -154,6 +154,7 @@ [Components.IA32, Components.X64]
   UefiCpuPkg/Library/SmmCpuPlatformHookLibNull/SmmCpuPlatformHookLibNull.inf
   UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
   UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
+  UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
   UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
   UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
   UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
-- 
2.28.0.windows.1


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

* Re: [PATCH v3 2/5] UefiCpuPkg/SmmCpuFeaturesLib: Rename SmmCpuFeaturesLib.c
  2021-02-17 21:32 ` [PATCH v3 2/5] UefiCpuPkg/SmmCpuFeaturesLib: Rename SmmCpuFeaturesLib.c Michael Kubacki
@ 2021-02-19 20:31   ` Laszlo Ersek
  0 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2021-02-19 20:31 UTC (permalink / raw)
  To: mikuback, devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar

On 02/17/21 22:32, mikuback@linux.microsoft.com wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> This change renames SmmCpuFeaturesLib.c to SmmCpuFeaturesLibCommon.c
> to better convey that this file contains library implementation
> common to all library instances.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => SmmCpuFeaturesLibCommon.c} | 2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf                              | 2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf                           | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> similarity index 96%
> rename from UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> rename to UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> index 75bde752785a..36c48310c31e 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> @@ -1,5 +1,5 @@
>  /** @file
> -The CPU specific programming for PiSmmCpuDxeSmm module.
> +Implementation shared across all library instances.
>  
>  Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> index a6d8467d26aa..7ebb0b0ef011 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> @@ -18,7 +18,7 @@ [Defines]
>  
>  [Sources]
>    CpuFeaturesLib.h
> -  SmmCpuFeaturesLib.c
> +  SmmCpuFeaturesLibCommon.c
>    SmmCpuFeaturesLibNoStm.c
>  
>  [Packages]
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> index 89cd252ef44e..78f8e4b42c44 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> @@ -19,7 +19,7 @@ [Defines]
>  
>  [Sources]
>    CpuFeaturesLib.h
> -  SmmCpuFeaturesLib.c
> +  SmmCpuFeaturesLibCommon.c
>    SmmStm.c
>    SmmStm.h
>  
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


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

* Re: [PATCH v3 3/5] UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors
  2021-02-17 21:32 ` [PATCH v3 3/5] UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors Michael Kubacki
@ 2021-02-19 20:39   ` Laszlo Ersek
  0 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2021-02-19 20:39 UTC (permalink / raw)
  To: mikuback, devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar

On 02/17/21 22:32, mikuback@linux.microsoft.com wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> There's currently two library instances:
>   1. SmmCpuFeaturesLib
>   2. SmmCpuFeaturesLibStm
> 
> There's two constructor functions:
>   1. SmmCpuFeaturesLibConstructor()
>   2. SmmCpuFeaturesLibStmConstructor()
> 
> SmmCpuFeaturesLibConstructor() is called by
> SmmCpuFeaturesLibStmConstructor() since the functionality in that
> function is required by both library instances.
> 
> The declaration for SmmCpuFeaturesLibConstructor() is embedded in
> "SmmStm.c" instead of being declared in a header file. Further,
> that constructor function is called by the STM specific constructor.
> 
> This change moves the common code to a function called
> CpuFeaturesLibInitialization() which is declared in an internal
> library header file "CpuFeaturesLib.h". Each constructor simply
> calls this function to perform the common functionality.
> 
> Additionally, SmmCpuFeaturesLibConstructor() is moved from
> SmmCpuFeaturesLibNoStm.c into a instance-specific file allowing
> SmmCpuFeaturesLibNoStm.c to contain no STM implementation agnostic
> to a particular library instance.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c       | 31 ++++++++++++++++++++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c | 19 +++++-------
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c                  | 23 ++-------------
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h          | 12 ++++++++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf     |  1 +
>  5 files changed, 54 insertions(+), 32 deletions(-)

The update looks OK, thanks.
Laszlo

> 
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> new file mode 100644
> index 000000000000..00948a191fad
> --- /dev/null
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> @@ -0,0 +1,31 @@
> +/** @file
> +Implementation specific to the SmmCpuFeatureLib library instance.
> +
> +Copyright (c) Microsoft Corporation.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <PiSmm.h>
> +#include "CpuFeaturesLib.h"
> +
> +/**
> +  The constructor function for the Traditional MM library instance without STM.
> +
> +  @param[in]  ImageHandle  The firmware allocated handle for the EFI image.
> +  @param[in]  SystemTable  A pointer to the EFI System Table.
> +
> +  @retval EFI_SUCCESS      The constructor always returns EFI_SUCCESS.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +SmmCpuFeaturesLibConstructor (
> +  IN EFI_HANDLE        ImageHandle,
> +  IN EFI_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  CpuFeaturesLibInitialization ();
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> index 36c48310c31e..7a919c5ee70f 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> @@ -2,6 +2,7 @@
>  Implementation shared across all library instances.
>  
>  Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
>  SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
> @@ -63,19 +64,15 @@ BOOLEAN  mNeedConfigureMtrrs = TRUE;
>  BOOLEAN  *mSmrrEnabled;
>  
>  /**
> -  The constructor function
> +  Performs library initialization.
>  
> -  @param[in]  ImageHandle  The firmware allocated handle for the EFI image.
> -  @param[in]  SystemTable  A pointer to the EFI System Table.
> -
> -  @retval EFI_SUCCESS      The constructor always returns EFI_SUCCESS.
> +  This initialization function contains common functionality shared betwen all
> +  library instance constructors.
>  
>  **/
> -EFI_STATUS
> -EFIAPI
> -SmmCpuFeaturesLibConstructor (
> -  IN EFI_HANDLE        ImageHandle,
> -  IN EFI_SYSTEM_TABLE  *SystemTable
> +VOID
> +CpuFeaturesLibInitialization (
> +  VOID
>    )
>  {
>    UINT32  RegEax;
> @@ -162,8 +159,6 @@ SmmCpuFeaturesLibConstructor (
>    //
>    mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
>    ASSERT (mSmrrEnabled != NULL);
> -
> -  return EFI_SUCCESS;
>  }
>  
>  /**
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
> index b5aad41fdb64..dcc2e9f9a09a 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
> @@ -30,22 +30,6 @@
>  #define RDWR_ACCS             3
>  #define FULL_ACCS             7
>  
> -/**
> -  The constructor function
> -
> -  @param[in]  ImageHandle  The firmware allocated handle for the EFI image.
> -  @param[in]  SystemTable  A pointer to the EFI System Table.
> -
> -  @retval EFI_SUCCESS      The constructor always returns EFI_SUCCESS.
> -
> -**/
> -EFI_STATUS
> -EFIAPI
> -SmmCpuFeaturesLibConstructor (
> -  IN EFI_HANDLE        ImageHandle,
> -  IN EFI_SYSTEM_TABLE  *SystemTable
> -  );
> -
>  EFI_HANDLE  mStmSmmCpuHandle = NULL;
>  
>  BOOLEAN mLockLoadMonitor = FALSE;
> @@ -112,7 +96,7 @@ UINTN  mMsegSize = 0;
>  BOOLEAN  mStmConfigurationTableInitialized = FALSE;
>  
>  /**
> -  The constructor function
> +  The constructor function for the Traditional MM library instance with STM.
>  
>    @param[in]  ImageHandle  The firmware allocated handle for the EFI image.
>    @param[in]  SystemTable  A pointer to the EFI System Table.
> @@ -138,10 +122,9 @@ SmmCpuFeaturesLibStmConstructor (
>    SmmCpuFeaturesLibStmSmiEntryFixupAddress ();
>  
>    //
> -  // Call the common constructor function
> +  // Perform library initialization common across all instances
>    //
> -  Status = SmmCpuFeaturesLibConstructor (ImageHandle, SystemTable);
> -  ASSERT_EFI_ERROR (Status);
> +  CpuFeaturesLibInitialization ();
>  
>    //
>    // Lookup the MP Services Protocol
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
> index 4645bbb066c9..f9a758e82558 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
> @@ -9,6 +9,18 @@
>  #ifndef _CPU_FEATURES_LIB_H_
>  #define _CPU_FEATURES_LIB_H_
>  
> +/**
> +  Performs library initialization.
> +
> +  This initialization function contains common functionality shared betwen all
> +  library instance constructors.
> +
> +**/
> +VOID
> +CpuFeaturesLibInitialization (
> +  VOID
> +  );
> +
>  /**
>    Internal worker function that is called to complete CPU initialization at the
>    end of SmmCpuFeaturesInitializeProcessor().
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> index 7ebb0b0ef011..ddd00eeceb84 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> @@ -18,6 +18,7 @@ [Defines]
>  
>  [Sources]
>    CpuFeaturesLib.h
> +  SmmCpuFeaturesLib.c
>    SmmCpuFeaturesLibCommon.c
>    SmmCpuFeaturesLibNoStm.c
>  
> 


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

* Re: [PATCH v3 4/5] UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber
  2021-02-17 21:32 ` [PATCH v3 4/5] UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber Michael Kubacki
@ 2021-02-19 20:43   ` Laszlo Ersek
  0 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2021-02-19 20:43 UTC (permalink / raw)
  To: mikuback, devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar

On 02/17/21 22:32, mikuback@linux.microsoft.com wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3218
> 
> Adds a new function called GetCpuMaxLogicalProcessorNumber() to
> return the number of maximum CPU logical processors (currently
> gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber).
> 
> This allows the the mechanism used to retrieve the CPU maximum
> logical processor number to be abstracted from the logic that
> needs the value.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> ---
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c     |  2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c | 28 ++++++++++++++++++++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h              | 14 ++++++++++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf         |  1 +
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf      |  1 +
>  5 files changed, 45 insertions(+), 1 deletion(-)

Looks good, thanks.
Laszlo

> 
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> index 7a919c5ee70f..50379f3aea19 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> @@ -157,7 +157,7 @@ CpuFeaturesLibInitialization (
>    //
>    // Allocate array for state of SMRR enable on all CPUs
>    //
> -  mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * PcdGet32 (PcdCpuMaxLogicalProcessorNumber));
> +  mSmrrEnabled = (BOOLEAN *)AllocatePool (sizeof (BOOLEAN) * GetCpuMaxLogicalProcessorNumber ());
>    ASSERT (mSmrrEnabled != NULL);
>  }
>  
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c
> new file mode 100644
> index 000000000000..b7a5c1926e4d
> --- /dev/null
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c
> @@ -0,0 +1,28 @@
> +/** @file
> +  Traditional MM CPU specific programming.
> +
> +  Copyright (c) Microsoft Corporation.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <Base.h>
> +#include <Library/PcdLib.h>
> +#include "CpuFeaturesLib.h"
> +
> +/**
> +  Gets the maximum number of logical processors from the PCD PcdCpuMaxLogicalProcessorNumber.
> +
> +  This access is abstracted from the PCD services to enforce that the PCD be
> +  FixedAtBuild in the Standalone MM build of this driver.
> +
> +  @return  The value of PcdCpuMaxLogicalProcessorNumber.
> +
> +**/
> +UINT32
> +GetCpuMaxLogicalProcessorNumber (
> +  VOID
> +  )
> +{
> +  return PcdGet32 (PcdCpuMaxLogicalProcessorNumber);
> +}
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
> index f9a758e82558..2b6bfa899a48 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
> @@ -31,4 +31,18 @@ FinishSmmCpuFeaturesInitializeProcessor (
>    VOID
>    );
>  
> +/**
> +  Gets the maximum number of logical processors from the PCD PcdCpuMaxLogicalProcessorNumber.
> +
> +  This access is abstracted from the PCD services to enforce that the PCD be
> +  FixedAtBuild in the Standalone MM build of this driver.
> +
> +  @return  The value of PcdCpuMaxLogicalProcessorNumber.
> +
> +**/
> +UINT32
> +GetCpuMaxLogicalProcessorNumber (
> +  VOID
> +  );
> +
>  #endif
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> index ddd00eeceb84..35292dac31ba 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
> @@ -21,6 +21,7 @@ [Sources]
>    SmmCpuFeaturesLib.c
>    SmmCpuFeaturesLibCommon.c
>    SmmCpuFeaturesLibNoStm.c
> +  TraditionalMmCpuFeaturesLib.c
>  
>  [Packages]
>    MdePkg/MdePkg.dec
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> index 78f8e4b42c44..022351f59320 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> @@ -22,6 +22,7 @@ [Sources]
>    SmmCpuFeaturesLibCommon.c
>    SmmStm.c
>    SmmStm.h
> +  TraditionalMmCpuFeaturesLib.c
>  
>  [Sources.Ia32]
>    Ia32/SmmStmSupport.c
> 


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

* Re: [PATCH v3 5/5] UefiCpuPkg/SmmCpuFeaturesLib: Add Standalone MM support
  2021-02-17 21:32 ` [PATCH v3 5/5] UefiCpuPkg/SmmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
@ 2021-02-19 20:43   ` Laszlo Ersek
  0 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2021-02-19 20:43 UTC (permalink / raw)
  To: mikuback, devel; +Cc: Eric Dong, Ray Ni, Rahul Kumar

On 02/17/21 22:32, mikuback@linux.microsoft.com wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3218
> 
> Adds an INF for StandaloneMmCpuFeaturesLib, which supports building
> the SmmCpuFeaturesLib code for Standalone MM. Minimal code changes
> are made to allow reuse of existing code for Standalone MM.
> 
> The original INF file names are left intact (continue to use SMM
> terminology) to retain backward compatibility with platforms that
> use those INFs. Similarly, the pre-existing C file names are
> unchanged to be consistent with the INF file names.
> 
> Note that all references in library source files to PiSmm.h have
> been changed to PiMm.h for consistency.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> ---
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c           |  2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c            |  2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c      |  2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c       |  2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c                       |  2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c   | 50 ++++++++++++++++++++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c            |  2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf | 38 +++++++++++++++
>  UefiCpuPkg/UefiCpuPkg.dsc                                           |  1 +
>  9 files changed, 95 insertions(+), 6 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c
> index 399ddd742dd7..24bd6d0a946d 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c
> @@ -6,7 +6,7 @@
>  
>  **/
>  
> -#include <PiSmm.h>
> +#include <PiMm.h>
>  #include <Library/DebugLib.h>
>  
>  #include "SmmStm.h"
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> index 00948a191fad..29e3b650acee 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c
> @@ -6,7 +6,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
>  
> -#include <PiSmm.h>
> +#include <PiMm.h>
>  #include "CpuFeaturesLib.h"
>  
>  /**
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> index 50379f3aea19..fa3a866ab6f6 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibCommon.c
> @@ -7,7 +7,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
>  
> -#include <PiSmm.h>
> +#include <PiMm.h>
>  #include <Library/SmmCpuFeaturesLib.h>
>  #include <Library/BaseLib.h>
>  #include <Library/MtrrLib.h>
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c
> index c562582ccee0..823dbe906cb8 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c
> @@ -7,7 +7,7 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
>  
>  **/
>  
> -#include <PiSmm.h>
> +#include <PiMm.h>
>  #include <Library/SmmCpuFeaturesLib.h>
>  #include "CpuFeaturesLib.h"
>  
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
> index dcc2e9f9a09a..c92387625ddc 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c
> @@ -6,7 +6,7 @@
>  
>  **/
>  
> -#include <PiSmm.h>
> +#include <PiMm.h>
>  #include <Library/BaseLib.h>
>  #include <Library/BaseMemoryLib.h>
>  #include <Library/MemoryAllocationLib.h>
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c
> new file mode 100644
> index 000000000000..dec6e53e5b55
> --- /dev/null
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c
> @@ -0,0 +1,50 @@
> +/** @file
> +Standalone MM CPU specific programming.
> +
> +Copyright (c) 2010 - 2019, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) Microsoft Corporation.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <PiMm.h>
> +#include <Library/PcdLib.h>
> +#include "CpuFeaturesLib.h"
> +
> +/**
> +  Gets the maximum number of logical processors from the PCD PcdCpuMaxLogicalProcessorNumber.
> +
> +  This access is abstracted from the PCD services to enforce that the PCD be
> +  FixedAtBuild in the Standalone MM build of this driver.
> +
> +  @return  The value of PcdCpuMaxLogicalProcessorNumber.
> +
> +**/
> +UINT32
> +GetCpuMaxLogicalProcessorNumber (
> +  VOID
> +  )
> +{
> +  return FixedPcdGet32 (PcdCpuMaxLogicalProcessorNumber);
> +}
> +
> +/**
> +  The Standalone MM library constructor.
> +
> +  @param[in] ImageHandle  Image handle of this driver.
> +  @param[in] SystemTable  A Pointer to the EFI System Table.
> +
> +  @retval EFI_SUCCESS     This constructor always returns success.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +StandaloneMmCpuFeaturesLibConstructor (
> +  IN EFI_HANDLE           ImageHandle,
> +  IN EFI_MM_SYSTEM_TABLE  *SystemTable
> +  )
> +{
> +  CpuFeaturesLibInitialization ();
> +
> +  return EFI_SUCCESS;
> +}
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c b/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c
> index aacc1455a90c..c8a3f58e0ca5 100644
> --- a/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c
> @@ -6,7 +6,7 @@
>  
>  **/
>  
> -#include <PiSmm.h>
> +#include <PiMm.h>
>  #include <Library/DebugLib.h>
>  
>  #include "SmmStm.h"
> diff --git a/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
> new file mode 100644
> index 000000000000..ec97041d8b75
> --- /dev/null
> +++ b/UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
> @@ -0,0 +1,38 @@
> +## @file
> +#  Standalone MM CPU specific programming.
> +#
> +#  Copyright (c) 2009 - 2016, Intel Corporation. All rights reserved.<BR>
> +#  Copyright (c) Microsoft Corporation.<BR>
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = StandaloneMmCpuFeaturesLib
> +  MODULE_UNI_FILE                = SmmCpuFeaturesLib.uni
> +  FILE_GUID                      = BB554A2D-F5DF-41D3-8C62-46476A2B2B18
> +  MODULE_TYPE                    = MM_STANDALONE
> +  VERSION_STRING                 = 1.0
> +  PI_SPECIFICATION_VERSION       = 0x00010032
> +  LIBRARY_CLASS                  = SmmCpuFeaturesLib
> +  CONSTRUCTOR                    = StandaloneMmCpuFeaturesLibConstructor
> +
> +[Sources]
> +  CpuFeaturesLib.h
> +  StandaloneMmCpuFeaturesLib.c
> +  SmmCpuFeaturesLibCommon.c
> +  SmmCpuFeaturesLibNoStm.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  UefiCpuPkg/UefiCpuPkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> +  DebugLib
> +  MemoryAllocationLib
> +  PcdLib
> +
> +[FixedPcd]
> +  gUefiCpuPkgTokenSpaceGuid.PcdCpuMaxLogicalProcessorNumber        ## SOMETIMES_CONSUMES
> diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc
> index 9128cef076dd..7db419471deb 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dsc
> +++ b/UefiCpuPkg/UefiCpuPkg.dsc
> @@ -154,6 +154,7 @@ [Components.IA32, Components.X64]
>    UefiCpuPkg/Library/SmmCpuPlatformHookLibNull/SmmCpuPlatformHookLibNull.inf
>    UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf
>    UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf
> +  UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.inf
>    UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
>    UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf
>    UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationSmm.inf
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>


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

* Re: [edk2-devel] [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support
  2021-02-17 21:32 [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
                   ` (4 preceding siblings ...)
  2021-02-17 21:32 ` [PATCH v3 5/5] UefiCpuPkg/SmmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
@ 2021-03-05 14:39 ` Laszlo Ersek
  2021-03-08  1:45 ` Dong, Eric
  2021-03-08 18:16 ` Laszlo Ersek
  7 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2021-03-05 14:39 UTC (permalink / raw)
  To: Eric Dong, Ray Ni; +Cc: devel, mikuback, Rahul Kumar

Eric, Ray,

On 02/17/21 22:32, Michael Kubacki wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3218
> 
> The present SmmCpuFeaturesLib implementation in UefiCpuPkg can be
> useful for IA32/X64 platforms that need a library instance for
> a Standalone MM environment. Much of the logic can be reused and a
> new INF can isolate the differences unique to Standalone MM.
> 
> This patch series contains an initial set of changes for cleaning
> up pre-existing design issues in the library. The final two patches
> contain changes needed for Standalone MM support.
> 
> Here's an overview of how the three library instances are organized
> that may be a useful reference (provided by Laszlo):
> 
>                     Traditional,  Traditional,  Standalone,
>                     no STM        STM           no STM
> 
> Entry point type   DXE           DXE           MM
> 
> Lib inst. init.    basic         STM           basic
> 
> Processor init.    basic         STM           basic
> 
> PCD access         any           any           fixed
> 
> * Traditional no STM = SmmCpuFeaturesLib.inf
> * Traditional STM = SmmCpuFeaturesLibStm.inf
> * Standalone no STM = StandaloneMmCpuFeaturesLib.inf

do you have any comments please?

Thanks,
Laszlo


> 
> V3 changes:
> 
>   PATCH v3 2/5 is a new patch in the series that renames the file
>   SmmCpuFeaturesLib.c to SmmCpuFeaturesLibCommon.c to more clearly
>   identify implementation in the file as shared between all library
>   instances.
> 
>   PATCH v3 3/5 adds a new source file SmmCpuFeaturesLib.c that
>   contains the constructor specific to the Traditional MM no
>   STM library instance. This was previously implemented in a
>   file built by the Standalone MM instance and while not
>   harmful, it was not clean.
> 
>   PATCH v3 4/5 updates "@retval" to "@return" in the documentation
>   for GetCpuMaxLogicalProcessorNumber() since it is not a constant
>   return value.
>   
>   PATCH v3 5/5 contains a commit message update to note that all
>   instances of "PiSmm.h" in the library source files have been
>   updated to "PiMm.h" for consistency throughout the library.
> 
> V2 changes:
> 
>   Due to some pre-existing design issues in the library that
>   affected a single v1 patch that add Standalone MM support,
>   it was suggested to first address those issues and then add the
>   new INF StandaloneMmCpuFeaturesLib.inf.
> 
>   To address these concerns, the following v1 patch was converted
>   into a v2 patch series:
>   https://edk2.groups.io/g/devel/message/71626
> 
>   The first two patches in v2 primarily addressed those concerns.
> 
>   PATCH v2 1/4 and PATCH v2 2/4 focused on fixing pre-existing
>   design issues.
> 
>   PATCH v2 3/4 and PATCH v2 4/4 focused on the changes needed to add
>   Standalone MM support.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> Michael Kubacki (5):
>   UefiCpuPkg/SmmCpuFeaturesLib: Move multi-instance function decl to
>     header
>   UefiCpuPkg/SmmCpuFeaturesLib: Rename SmmCpuFeaturesLib.c
>   UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors
>   UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber
>   UefiCpuPkg/SmmCpuFeaturesLib: Add Standalone MM support
> 
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c                                      |   2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c                                       | 608 +-------------------
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => SmmCpuFeaturesLibCommon.c}        |  36 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c                                  |   3 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c                                                  |  26 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c                              |  50 ++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c                             |  28 +
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c                                       |   2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h                                          |  48 ++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf                                     |   3 +
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf                                  |   4 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.inf => StandaloneMmCpuFeaturesLib.inf} |  22 +-
>  UefiCpuPkg/UefiCpuPkg.dsc                                                                      |   1 +
>  13 files changed, 172 insertions(+), 661 deletions(-)
>  copy UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => SmmCpuFeaturesLibCommon.c} (93%)
>  create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c
>  create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c
>  create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
>  copy UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.inf => StandaloneMmCpuFeaturesLib.inf} (53%)
> 


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

* Re: [edk2-devel] [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support
  2021-02-17 21:32 [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
                   ` (5 preceding siblings ...)
  2021-03-05 14:39 ` [edk2-devel] [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: " Laszlo Ersek
@ 2021-03-08  1:45 ` Dong, Eric
  2021-03-08 18:16 ` Laszlo Ersek
  7 siblings, 0 replies; 13+ messages in thread
From: Dong, Eric @ 2021-03-08  1:45 UTC (permalink / raw)
  To: devel@edk2.groups.io, mikuback@linux.microsoft.com
  Cc: Ni, Ray, Laszlo Ersek, Kumar, Rahul1

Reviewed-by: Eric Dong <eric.dong@intel.com>  for this serial.

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael Kubacki
Sent: Thursday, February 18, 2021 5:32 AM
To: devel@edk2.groups.io
Cc: Dong, Eric <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Laszlo Ersek <lersek@redhat.com>; Kumar, Rahul1 <rahul1.kumar@intel.com>
Subject: [edk2-devel] [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support

From: Michael Kubacki <michael.kubacki@microsoft.com>

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

The present SmmCpuFeaturesLib implementation in UefiCpuPkg can be useful for IA32/X64 platforms that need a library instance for a Standalone MM environment. Much of the logic can be reused and a new INF can isolate the differences unique to Standalone MM.

This patch series contains an initial set of changes for cleaning up pre-existing design issues in the library. The final two patches contain changes needed for Standalone MM support.

Here's an overview of how the three library instances are organized that may be a useful reference (provided by Laszlo):

                    Traditional,  Traditional,  Standalone,
                    no STM        STM           no STM

Entry point type   DXE           DXE           MM

Lib inst. init.    basic         STM           basic

Processor init.    basic         STM           basic

PCD access         any           any           fixed

* Traditional no STM = SmmCpuFeaturesLib.inf
* Traditional STM = SmmCpuFeaturesLibStm.inf
* Standalone no STM = StandaloneMmCpuFeaturesLib.inf

V3 changes:

  PATCH v3 2/5 is a new patch in the series that renames the file
  SmmCpuFeaturesLib.c to SmmCpuFeaturesLibCommon.c to more clearly
  identify implementation in the file as shared between all library
  instances.

  PATCH v3 3/5 adds a new source file SmmCpuFeaturesLib.c that
  contains the constructor specific to the Traditional MM no
  STM library instance. This was previously implemented in a
  file built by the Standalone MM instance and while not
  harmful, it was not clean.

  PATCH v3 4/5 updates "@retval" to "@return" in the documentation
  for GetCpuMaxLogicalProcessorNumber() since it is not a constant
  return value.
  
  PATCH v3 5/5 contains a commit message update to note that all
  instances of "PiSmm.h" in the library source files have been
  updated to "PiMm.h" for consistency throughout the library.

V2 changes:

  Due to some pre-existing design issues in the library that
  affected a single v1 patch that add Standalone MM support,
  it was suggested to first address those issues and then add the
  new INF StandaloneMmCpuFeaturesLib.inf.

  To address these concerns, the following v1 patch was converted
  into a v2 patch series:
  https://edk2.groups.io/g/devel/message/71626

  The first two patches in v2 primarily addressed those concerns.

  PATCH v2 1/4 and PATCH v2 2/4 focused on fixing pre-existing
  design issues.

  PATCH v2 3/4 and PATCH v2 4/4 focused on the changes needed to add
  Standalone MM support.

Cc: Eric Dong <eric.dong@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Rahul Kumar <rahul1.kumar@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

Michael Kubacki (5):
  UefiCpuPkg/SmmCpuFeaturesLib: Move multi-instance function decl to
    header
  UefiCpuPkg/SmmCpuFeaturesLib: Rename SmmCpuFeaturesLib.c
  UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors
  UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber
  UefiCpuPkg/SmmCpuFeaturesLib: Add Standalone MM support

 UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c                                      |   2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c                                       | 608 +-------------------
 UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => SmmCpuFeaturesLibCommon.c}        |  36 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c                                  |   3 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c                                                  |  26 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c                              |  50 ++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c                             |  28 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c                                       |   2 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h                                          |  48 ++
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf                                     |   3 +
 UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf                                  |   4 +-
 UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.inf => StandaloneMmCpuFeaturesLib.inf} |  22 +-
 UefiCpuPkg/UefiCpuPkg.dsc                                                                      |   1 +
 13 files changed, 172 insertions(+), 661 deletions(-)  copy UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => SmmCpuFeaturesLibCommon.c} (93%)  create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c
 create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c
 create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
 copy UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.inf => StandaloneMmCpuFeaturesLib.inf} (53%)

--
2.28.0.windows.1



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#71751): https://edk2.groups.io/g/devel/message/71751
Mute This Topic: https://groups.io/mt/80715262/1768733
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [eric.dong@intel.com]
-=-=-=-=-=-=



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

* Re: [edk2-devel] [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support
  2021-02-17 21:32 [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
                   ` (6 preceding siblings ...)
  2021-03-08  1:45 ` Dong, Eric
@ 2021-03-08 18:16 ` Laszlo Ersek
  7 siblings, 0 replies; 13+ messages in thread
From: Laszlo Ersek @ 2021-03-08 18:16 UTC (permalink / raw)
  To: devel, mikuback; +Cc: Eric Dong, Ray Ni, Rahul Kumar

On 02/17/21 22:32, Michael Kubacki wrote:
> From: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> REF:https://bugzilla.tianocore.org/show_bug.cgi?id=3218
> 
> The present SmmCpuFeaturesLib implementation in UefiCpuPkg can be
> useful for IA32/X64 platforms that need a library instance for
> a Standalone MM environment. Much of the logic can be reused and a
> new INF can isolate the differences unique to Standalone MM.
> 
> This patch series contains an initial set of changes for cleaning
> up pre-existing design issues in the library. The final two patches
> contain changes needed for Standalone MM support.
> 
> Here's an overview of how the three library instances are organized
> that may be a useful reference (provided by Laszlo):
> 
>                     Traditional,  Traditional,  Standalone,
>                     no STM        STM           no STM
> 
> Entry point type   DXE           DXE           MM
> 
> Lib inst. init.    basic         STM           basic
> 
> Processor init.    basic         STM           basic
> 
> PCD access         any           any           fixed
> 
> * Traditional no STM = SmmCpuFeaturesLib.inf
> * Traditional STM = SmmCpuFeaturesLibStm.inf
> * Standalone no STM = StandaloneMmCpuFeaturesLib.inf
> 
> V3 changes:
> 
>   PATCH v3 2/5 is a new patch in the series that renames the file
>   SmmCpuFeaturesLib.c to SmmCpuFeaturesLibCommon.c to more clearly
>   identify implementation in the file as shared between all library
>   instances.
> 
>   PATCH v3 3/5 adds a new source file SmmCpuFeaturesLib.c that
>   contains the constructor specific to the Traditional MM no
>   STM library instance. This was previously implemented in a
>   file built by the Standalone MM instance and while not
>   harmful, it was not clean.
> 
>   PATCH v3 4/5 updates "@retval" to "@return" in the documentation
>   for GetCpuMaxLogicalProcessorNumber() since it is not a constant
>   return value.
>   
>   PATCH v3 5/5 contains a commit message update to note that all
>   instances of "PiSmm.h" in the library source files have been
>   updated to "PiMm.h" for consistency throughout the library.
> 
> V2 changes:
> 
>   Due to some pre-existing design issues in the library that
>   affected a single v1 patch that add Standalone MM support,
>   it was suggested to first address those issues and then add the
>   new INF StandaloneMmCpuFeaturesLib.inf.
> 
>   To address these concerns, the following v1 patch was converted
>   into a v2 patch series:
>   https://edk2.groups.io/g/devel/message/71626
> 
>   The first two patches in v2 primarily addressed those concerns.
> 
>   PATCH v2 1/4 and PATCH v2 2/4 focused on fixing pre-existing
>   design issues.
> 
>   PATCH v2 3/4 and PATCH v2 4/4 focused on the changes needed to add
>   Standalone MM support.
> 
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Rahul Kumar <rahul1.kumar@intel.com>
> Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
> 
> Michael Kubacki (5):
>   UefiCpuPkg/SmmCpuFeaturesLib: Move multi-instance function decl to
>     header
>   UefiCpuPkg/SmmCpuFeaturesLib: Rename SmmCpuFeaturesLib.c
>   UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors
>   UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber
>   UefiCpuPkg/SmmCpuFeaturesLib: Add Standalone MM support
> 
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/Ia32/SmmStmSupport.c                                      |   2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.c                                       | 608 +-------------------
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => SmmCpuFeaturesLibCommon.c}        |  36 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibNoStm.c                                  |   3 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmStm.c                                                  |  26 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c                              |  50 ++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c                             |  28 +
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/X64/SmmStmSupport.c                                       |   2 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h                                          |  48 ++
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLib.inf                                     |   3 +
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/SmmCpuFeaturesLibStm.inf                                  |   4 +-
>  UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.inf => StandaloneMmCpuFeaturesLib.inf} |  22 +-
>  UefiCpuPkg/UefiCpuPkg.dsc                                                                      |   1 +
>  13 files changed, 172 insertions(+), 661 deletions(-)
>  copy UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.c => SmmCpuFeaturesLibCommon.c} (93%)
>  create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/StandaloneMmCpuFeaturesLib.c
>  create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/TraditionalMmCpuFeaturesLib.c
>  create mode 100644 UefiCpuPkg/Library/SmmCpuFeaturesLib/CpuFeaturesLib.h
>  copy UefiCpuPkg/Library/SmmCpuFeaturesLib/{SmmCpuFeaturesLib.inf => StandaloneMmCpuFeaturesLib.inf} (53%)
> 

Merged as commit range 94fa95c8746c..edd46cd407ea, via
<https://github.com/tianocore/edk2/pull/1482>.

I had to replace the guard macro "_CPU_FEATURES_LIB_H_" with
"CPU_FEATURES_LIB_H_" (removing the underscore prefix) in the first patch.

That's because commit 6ffbb3581ab7 ("BaseTools: Align include guards
policy", 2021-02-26) was merged after this series had been posted, and
now "_CPU_FEATURES_LIB_H_" triggered ECC error 8003.

Thanks,
Laszlo


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

end of thread, other threads:[~2021-03-08 18:16 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-02-17 21:32 [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
2021-02-17 21:32 ` [PATCH v3 1/5] UefiCpuPkg/SmmCpuFeaturesLib: Move multi-instance function decl to header Michael Kubacki
2021-02-17 21:32 ` [PATCH v3 2/5] UefiCpuPkg/SmmCpuFeaturesLib: Rename SmmCpuFeaturesLib.c Michael Kubacki
2021-02-19 20:31   ` Laszlo Ersek
2021-02-17 21:32 ` [PATCH v3 3/5] UefiCpuPkg/SmmCpuFeaturesLib: Cleanup library constructors Michael Kubacki
2021-02-19 20:39   ` Laszlo Ersek
2021-02-17 21:32 ` [PATCH v3 4/5] UefiCpuPkg/SmmCpuFeaturesLib: Abstract PcdCpuMaxLogicalProcessorNumber Michael Kubacki
2021-02-19 20:43   ` Laszlo Ersek
2021-02-17 21:32 ` [PATCH v3 5/5] UefiCpuPkg/SmmCpuFeaturesLib: Add Standalone MM support Michael Kubacki
2021-02-19 20:43   ` Laszlo Ersek
2021-03-05 14:39 ` [edk2-devel] [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: " Laszlo Ersek
2021-03-08  1:45 ` Dong, Eric
2021-03-08 18:16 ` Laszlo Ersek

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