Looked over the series at a high-level and the feature structure looks fine. For the series:
Acked-by: Michael Kubacki <michael.kuba...@microsoft.com>

I didn't look closely at implementation but there's a few things I noticed.

There's a few typos. I'd suggest running a spell check to clean those up. Some quick examples:

ServerManagement.h:

 "Generic Definations for Server Management Header File."

  typedef struct {
    LINERIZATION_TYPE Linearization;              // L

IpmiTransportPei.h:

  "IPMI Ttransport PPI Header File."

There was a mix of function documentation styles though that might be something to clean up later.

I saw some globals that did not appear necessary. For example, mIpmiTransport in SmmIpmiBaseLib.c seems to be located before every access.

Thanks,
Michael

On 3/1/2021 6:27 PM, Nate DeSimone wrote:
From: Isaac Oram <isaac.w.o...@intel.com>

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

Implement an open source generic IPMI transport driver.
Provides the ability to communicate with a BMC over IPMI
in MinPlatform board packages.

New changes:
   1. Added GenericIpmi
   2. Added IPMI base libs
   3. Added transport PPI and protocol
   4. Updated IPMI command request and response data size from
      UINT8 to UINT32 in IPMI transport layer to be compatible
      with EDK2 industry standard IPMI commands.
   6. Added the completion code in the first byte of all IPMI
      response data to be compatible with EDK2 industry standard
      IPMI commands.

Cc: Sai Chaganty <rangasai.v.chaga...@intel.com>
Cc: Liming Gao <gaolim...@byosoft.com.cn>
Cc: Michael Kubacki <michael.kuba...@microsoft.com>
Signed-off-by: Isaac Oram <isaac.w.o...@intel.com>
Co-authored-by: Nate DeSimone <nathaniel.l.desim...@intel.com>

Isaac Oram (9):
   IpmiFeaturePkg: Add IPMI driver Include headers
   IpmiFeaturePkg: Add IpmiBaseLib and IpmiCommandLib
   IpmiFeaturePkg: Add IpmiInit drivers
   IpmiFeaturePkg: Add GenericIpmi driver common code
   IpmiFeaturePkg: Add GenericIpmi PEIM
   IpmiFeaturePkg: Add GenericIpmi DXE Driver
   IpmiFeaturePkg: Add GenericIpmi SMM Driver
   IpmiFeaturePkg: Add IPMI driver build files
   Maintainers.txt: Add IpmiFeaturePkg maintainers

  .../GenericIpmi/Common/IpmiBmc.c              | 297 +++++++++++
  .../GenericIpmi/Common/IpmiBmc.h              |  44 ++
  .../GenericIpmi/Common/IpmiBmcCommon.h        | 179 +++++++
  .../GenericIpmi/Common/IpmiHooks.c            |  96 ++++
  .../GenericIpmi/Common/IpmiHooks.h            |  81 +++
  .../GenericIpmi/Common/IpmiPhysicalLayer.h    |  29 ++
  .../GenericIpmi/Common/KcsBmc.c               | 483 ++++++++++++++++++
  .../GenericIpmi/Common/KcsBmc.h               | 236 +++++++++
  .../GenericIpmi/Dxe/GenericIpmi.c             |  46 ++
  .../GenericIpmi/Dxe/GenericIpmi.inf           |  66 +++
  .../IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c | 452 ++++++++++++++++
  .../GenericIpmi/Pei/PeiGenericIpmi.c          | 362 +++++++++++++
  .../GenericIpmi/Pei/PeiGenericIpmi.h          | 138 +++++
  .../GenericIpmi/Pei/PeiGenericIpmi.inf        |  58 +++
  .../GenericIpmi/Pei/PeiIpmiBmc.c              | 277 ++++++++++
  .../GenericIpmi/Pei/PeiIpmiBmc.h              |  38 ++
  .../GenericIpmi/Pei/PeiIpmiBmcDef.h           | 156 ++++++
  .../GenericIpmi/Smm/SmmGenericIpmi.c          | 208 ++++++++
  .../GenericIpmi/Smm/SmmGenericIpmi.inf        |  53 ++
  .../IpmiFeaturePkg/Include/IpmiFeature.dsc    |   9 +-
  .../Include/Library/IpmiBaseLib.h             |  50 ++
  .../Include/Library/IpmiCommandLib.h          |  19 +-
  .../Include/Ppi/IpmiTransportPpi.h            |  68 +++
  .../Include/Protocol/IpmiTransportProtocol.h  |  75 +++
  .../IpmiFeaturePkg/Include/ServerManagement.h | 337 ++++++++++++
  .../IpmiFeaturePkg/Include/SmStatusCodes.h    |  98 ++++
  .../IpmiFeaturePkg/IpmiFeaturePkg.dec         |  22 +-
  .../IpmiFeaturePkg/IpmiInit/DxeIpmiInit.c     |   4 +-
  .../IpmiFeaturePkg/IpmiInit/DxeIpmiInit.inf   |   6 +-
  .../IpmiFeaturePkg/IpmiInit/PeiIpmiInit.c     |   4 +-
  .../IpmiFeaturePkg/IpmiInit/PeiIpmiInit.inf   |   4 +-
  .../Library/IpmiBaseLib/IpmiBaseLib.c         | 155 ++++++
  .../Library/IpmiBaseLib/IpmiBaseLib.inf       |  28 +
  .../Library/IpmiBaseLibNull/IpmiBaseLibNull.c |  76 +++
  .../IpmiBaseLibNull/IpmiBaseLibNull.inf       |  36 ++
  .../Library/IpmiCommandLib/IpmiCommandLib.inf |   4 +-
  .../IpmiCommandLib/IpmiCommandLibNetFnApp.c   |   7 +-
  .../IpmiCommandLibNetFnChassis.c              |  51 +-
  .../IpmiCommandLibNetFnStorage.c              |   7 +-
  .../IpmiCommandLibNetFnTransport.c            |   7 +-
  .../Library/PeiIpmiBaseLib/PeiIpmiBaseLib.c   | 111 ++++
  .../Library/PeiIpmiBaseLib/PeiIpmiBaseLib.inf |  30 ++
  .../Library/SmmIpmiBaseLib/SmmIpmiBaseLib.c   | 180 +++++++
  .../Library/SmmIpmiBaseLib/SmmIpmiBaseLib.inf |  29 ++
  .../IpmiFeaturePkg/Readme.md                  |   4 +-
  Maintainers.txt                               |   6 +
  46 files changed, 4694 insertions(+), 32 deletions(-)
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiBmc.c
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiBmc.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiBmcCommon.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiHooks.c
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiHooks.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/IpmiPhysicalLayer.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/KcsBmc.c
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Common/KcsBmc.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/GenericIpmi.c
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/GenericIpmi.inf
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Dxe/IpmiInit.c
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.c
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiGenericIpmi.inf
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiBmc.c
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiBmc.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Pei/PeiIpmiBmcDef.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Smm/SmmGenericIpmi.c
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/GenericIpmi/Smm/SmmGenericIpmi.inf
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/Library/IpmiBaseLib.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/Ppi/IpmiTransportPpi.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/Protocol/IpmiTransportProtocol.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/ServerManagement.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Include/SmStatusCodes.h
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiBaseLib/IpmiBaseLib.c
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiBaseLib/IpmiBaseLib.inf
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiBaseLibNull/IpmiBaseLibNull.c
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/IpmiBaseLibNull/IpmiBaseLibNull.inf
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/PeiIpmiBaseLib/PeiIpmiBaseLib.c
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/PeiIpmiBaseLib/PeiIpmiBaseLib.inf
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/SmmIpmiBaseLib/SmmIpmiBaseLib.c
  create mode 100644 
Features/Intel/OutOfBandManagement/IpmiFeaturePkg/Library/SmmIpmiBaseLib/SmmIpmiBaseLib.inf



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#72401): https://edk2.groups.io/g/devel/message/72401
Mute This Topic: https://groups.io/mt/81016650/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to