Re: [edk2-devel] [PATCH v3 0/5] UefiCpuPkg/StandaloneMmCpuFeaturesLib: Add Standalone MM support

["Laszlo Ersek" <lersek@redhat.com>]

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%)
>