Re: [edk2-platforms][PATCH 1/2] ManageabilityPkg: Add Manageability IPMI helper Library

["Chang, Abner" <abner.chang@amd.com>]

[AMD Official Use Only - General]

Sure, I will.

Thanks for catching this.
Abner

> -----Original Message-----
> From: Oram, Isaac W <isaac.w.oram@intel.com>
> Sent: Wednesday, May 10, 2023 7:51 AM
> To: Chang, Abner <Abner.Chang@amd.com>; devel@edk2.groups.io
> Cc: Attar, AbdulLateef (Abdul Lateef) <AbdulLateef.Attar@amd.com>; Nickle
> Wang <nicklew@nvidia.com>; Tinh Nguyen
> <tinhnguyen@os.amperecomputing.com>
> Subject: RE: [edk2-platforms][PATCH 1/2] ManageabilityPkg: Add
> Manageability IPMI helper Library
> 
> Caution: This message originated from an External Source. Use proper
> caution when opening attachments, clicking links, or responding.
> 
> 
> Reviewed-by: Isaac Oram <isaac.w.oram@intel.com>
> 
> I noted that the BaseManageabilityTransportIpmiHelper.c file header
> comment indicates that this is a NULL library implementation which seems
> incorrect/misleading.  I think that this can be fixed before pushing.
> 
> -----Original Message-----
> From: abner.chang@amd.com <abner.chang@amd.com>
> Sent: Tuesday, May 9, 2023 12:56 AM
> To: devel@edk2.groups.io
> Cc: Oram, Isaac W <isaac.w.oram@intel.com>; Abdul Lateef Attar
> <abdattar@amd.com>; Nickle Wang <nicklew@nvidia.com>; Tinh Nguyen
> <tinhnguyen@os.amperecomputing.com>
> Subject: [edk2-platforms][PATCH 1/2] ManageabilityPkg: Add Manageability
> IPMI helper Library
> 
> From: Abner Chang <abner.chang@amd.com>
> 
> Add IPMI helper library to print debug message of IPMI Completion Code in
> human readable string and return the transport interface additional status.
> 
> Signed-off-by: Abner Chang <abner.chang@amd.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Abdul Lateef Attar <abdattar@amd.com>
> Cc: Nickle Wang <nicklew@nvidia.com>
> Cc: Tinh Nguyen <tinhnguyen@os.amperecomputing.com>
> ---
>  .../BaseManageabilityTransportHelper.inf      |  1 +
>  .../Library/ManageabilityTransportHelperLib.h | 24 +++++++
>  .../Library/ManageabilityTransportIpmiLib.h   | 13 +++-
>  .../Library/ManageabilityTransportLib.h       | 11 +--
>  .../BaseManageabilityTransportIpmiHelper.c    | 70 +++++++++++++++++++
>  5 files changed, 112 insertions(+), 7 deletions(-)  create mode 100644
> Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib/B
> aseManageabilityTransportIpmiHelper.c
> 
> diff --git
> a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib
> /BaseManageabilityTransportHelper.inf
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib
> /BaseManageabilityTransportHelper.inf
> index c9e5eaef60..0936449fda 100644
> ---
> a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib
> /BaseManageabilityTransportHelper.inf
> +++
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelper
> +++ Lib/BaseManageabilityTransportHelper.inf
> @@ -21,6 +21,7 @@
> 
>  [Sources]
>    BaseManageabilityTransportHelper.c
> +  BaseManageabilityTransportIpmiHelper.c
> 
>  [LibraryClasses]
>    BaseMemoryLib
> diff --git
> a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelpe
> rLib.h
> b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelpe
> rLib.h
> index c2c98d6c2d..11a1bd0521 100644
> ---
> a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelpe
> rLib.h
> +++
> b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHe
> +++ lperLib.h
> @@ -187,4 +187,28 @@ HelperManageabilityDebugPrint (
>    ...
>    );
> 
> +///
> +/// IPMI Helper Functions.
> +///
> +
> +/**
> +  This function returns a human readable string of IPMI KCS Completion
> +Code
> +  and returns the corresponding additional status of transport interface.
> +
> +  @param [in]  CompletionCode     The Completion Code returned from KCS.
> +  @param [out] CompletionCodeStr  Human readable string of IPMI
> Completion Code.
> +  @param [out] AdditionalStatus   Return the addtional status.
> +
> +  @retval  EFI_SUCCESS            The information of Completion Code is
> returned.
> +  @retval  EFI_NOT_FOUND          No information of Completion Code is
> returned.
> +  @retval  EFI_INVALID_PARAMETER  The given parameter is incorrect.
> +
> +**/
> +EFI_STATUS
> +IpmiHelperCheckCompletionCode (
> +  IN   UINT8                                      CompletionCode,
> +  OUT  CHAR16                                     **CompletionCodeStr,
> +  OUT  MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS
> *AdditionalStatus
> +  );
> +
>  #endif
> diff --git
> a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportIpmiLi
> b.h
> b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportIpmiLi
> b.h
> index 1628255a6a..6d136e460f 100644
> ---
> a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportIpmiLi
> b.h
> +++ b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportIp
> +++ miLib.h
> @@ -16,9 +16,18 @@
>  /// the payload.
>  ///
>  typedef struct {
> -  UINT8    Lun:2;
> -  UINT8    NetFn:6;
> +  UINT8    Lun   : 2;
> +  UINT8    NetFn : 6;
>    UINT8    Command;
>  } MANAGEABILITY_IPMI_TRANSPORT_HEADER;
> 
> +///
> +/// The IPMI Completion Code mapping.
> +///
> +typedef struct {
> +  UINT8                                        CompletionCode;
> +  CHAR16                                       *CompletionCodeString;
> +  MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS    AdditionalStatus;
> +} MANAGEABILITY_IPMI_COMPLETTION_CODE_MAPPING;
> +
>  #endif
> diff --git
> a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportLib.h
> b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportLib.h
> index 04072aee89..f423a1ed44 100644
> ---
> a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportLib.h
> +++ b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportLi
> +++ b.h
> @@ -61,11 +61,12 @@ typedef union {
>  /// Additional transport interface status.
>  ///
>  typedef UINT32 MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS;
> -#define MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_NO_ERRORS
> 0x00000000
> -#define MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_ERROR
> 0x00000001
> -#define
> MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_BUSY_IN_READ
> 0x00000002
> -#define
> MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_BUSY_IN_WRITE
> 0x00000004 -#define
> MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_NOT_AVAILABLE
> 0xffffffff
> +#define MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_NO_ERRORS
> 0x00000000
> +#define MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_ERROR
> 0x00000001
> +#define
> MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_BUSY_IN_READ
> 0x00000002
> +#define
> MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_BUSY_IN_WRITE
> 0x00000004
> +#define
> MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_INVALID_COMMAND
> 0x00000008
> +#define
> MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_NOT_AVAILABLE
> 0xffffffff
> 
>  ///
>  /// Additional transport interface features.
> diff --git
> a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib
> /BaseManageabilityTransportIpmiHelper.c
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib
> /BaseManageabilityTransportIpmiHelper.c
> new file mode 100644
> index 0000000000..8710cafc99
> --- /dev/null
> +++
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelper
> +++ Lib/BaseManageabilityTransportIpmiHelper.c
> @@ -0,0 +1,70 @@
> +/** @file
> +  Null instance of Manageability IPMI Helper Library
> +
> +  Copyright (C) 2023 Advanced Micro Devices, Inc. All rights
> +reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent **/
> +
> +#include <Uefi.h>
> +#include <Library/DebugLib.h>
> +#include <Library/ManageabilityTransportIpmiLib.h>
> +
> +#include <IndustryStandard/Ipmi.h>
> +
> +//
> +// BaseManageabilityTransportHelper is used by PEI, DXE and SMM.
> +// Make sure the global variables added here should be unchangeable.
> +//
> +MANAGEABILITY_IPMI_COMPLETTION_CODE_MAPPING
> IpmiCompletionCodeMapping[] = {
> +  { IPMI_COMP_CODE_NORMAL,          L"IPMI Completion Code - Normal",
> MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_NO_ERRORS       },
> +  { IPMI_COMP_CODE_NODE_BUSY,       L"IPMI Completion Code - Busy",
> MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_BUSY_IN_READ    },
> +  { IPMI_COMP_CODE_INVALID_COMMAND, L"IPMI Completion Code -
> Invalid
> +command",
> MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS_INVALID_COMMAND }
> +};
> +
> +UINT8  IpmiCompletionCodeMappingEntries = sizeof
> +(IpmiCompletionCodeMapping) / sizeof
> +(MANAGEABILITY_IPMI_COMPLETTION_CODE_MAPPING);
> +
> +///
> +/// IPMI Helper Functions.
> +///
> +
> +/**
> +  This function returns a human readable string of IPMI KCS Completion
> +Code
> +  and returns the corresponding additional status of transport interface.
> +
> +  @param [in]  CompletionCode     The Completion Code returned from KCS.
> +  @param [out] CompletionCodeStr  Human readable string of IPMI
> Completion Code.
> +  @param [out] AdditionalStatus   Return the addtional status.
> +
> +  @retval  EFI_SUCCESS            The information of Completion Code is
> returned.
> +  @retval  EFI_NOT_FOUND          No information of Completion Code is
> returned.
> +  @retval  EFI_INVALID_PARAMETER  The given parameter is incorrect.
> +
> +**/
> +EFI_STATUS
> +IpmiHelperCheckCompletionCode (
> +  IN   UINT8                                      CompletionCode,
> +  OUT  CHAR16                                     **CompletionCodeStr,
> +  OUT  MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS
> *AdditionalStatus
> +  )
> +{
> +  UINT8                                        Index;
> +  MANAGEABILITY_IPMI_COMPLETTION_CODE_MAPPING  *ThisCcMapping;
> +
> +  if ((CompletionCodeStr == NULL) || (AdditionalStatus == NULL)) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  *AdditionalStatus = 0;
> +  ThisCcMapping     = IpmiCompletionCodeMapping;
> +  for (Index = 0; Index < IpmiCompletionCodeMappingEntries; Index++) {
> +    if (ThisCcMapping->CompletionCode == CompletionCode) {
> +      *CompletionCodeStr = ThisCcMapping->CompletionCodeString;
> +      *AdditionalStatus  = ThisCcMapping->AdditionalStatus;
> +      return EFI_SUCCESS;
> +    }
> +
> +    ThisCcMapping++;
> +  }
> +
> +  return EFI_NOT_FOUND;
> +}
> --
> 2.37.1.windows.1