[AMD Official Use Only - General]
Hi Abdul,
I don't use EFIAPI is because this is a library linked with the module, so
there is no calling convention problem.
However, I should have a consistent coding style for this file. I will update
this file and remove EFIAPI for other functions.
Thanks
Abner
> -----Original Message-----
> From: Attar, AbdulLateef (Abdul Lateef) <abdullateef.at...@amd.com>
> Sent: Wednesday, April 19, 2023 1:30 PM
> To: Chang, Abner <abner.ch...@amd.com>; devel@edk2.groups.io
> Cc: Isaac Oram <isaac.w.o...@intel.com>; Nickle Wang
> <nick...@nvidia.com>; Igor Kulchytskyy <ig...@ami.com>
> Subject: RE: [edk2-platforms][PATCH V2 01/14] ManageabilityPkg: Add more
> helper functions
>
> [AMD Official Use Only - General]
>
> Comments inline.
>
> -----Original Message-----
> From: Chang, Abner <abner.ch...@amd.com>
> Sent: 18 April 2023 12:46
> To: devel@edk2.groups.io
> Cc: Isaac Oram <isaac.w.o...@intel.com>; Attar, AbdulLateef (Abdul Lateef)
> <abdullateef.at...@amd.com>; Nickle Wang <nick...@nvidia.com>; Igor
> Kulchytskyy <ig...@ami.com>
> Subject: [edk2-platforms][PATCH V2 01/14] ManageabilityPkg: Add more
> helper functions
>
> From: Abner Chang <abner.ch...@amd.com>
>
> 1. Add a helper function to output payload binary
> to debug output device.
> 2. Add a helper function to split payload into
> packages according to maximum transfer unit
> of transport interface.
> 3. Add a helper function to generate CRC8.
>
> Signed-off-by: Abner Chang <abner.ch...@amd.com>
> Cc: Isaac Oram <isaac.w.o...@intel.com>
> Cc: Abdul Lateef Attar <abdat...@amd.com>
> Cc: Nickle Wang <nick...@nvidia.com>
> Cc: Igor Kulchytskyy <ig...@ami.com>
> ---
> .../BaseManageabilityTransportHelper.inf | 1 +
> .../Library/ManageabilityTransportHelperLib.h | 98 ++++++++
> .../BaseManageabilityTransportHelper.c | 225 +++++++++++++++++-
> 3 files changed, 314 insertions(+), 10 deletions(-)
>
> diff --git
> a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib
> /BaseManageabilityTransportHelper.inf
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib
> /BaseManageabilityTransportHelper.inf
> index 5447954144..c9e5eaef60 100644
> ---
> a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib
> /BaseManageabilityTransportHelper.inf
> +++
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelper
> +++ Lib/BaseManageabilityTransportHelper.inf
> @@ -25,6 +25,7 @@
> [LibraryClasses]
> BaseMemoryLib
> DebugLib
> + MemoryAllocationLib
>
> [Packages]
> ManageabilityPkg/ManageabilityPkg.dec
> diff --git
> a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelpe
> rLib.h
> b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelpe
> rLib.h
> index 718ac34a1f..0dbf5ccb3c 100644
> ---
> a/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHelpe
> rLib.h
> +++
> b/Features/ManageabilityPkg/Include/Library/ManageabilityTransportHe
> +++ lperLib.h
> @@ -11,8 +11,24 @@
>
> #include <Library/ManageabilityTransportLib.h>
>
> +#define DEBUG_MANAGEABILITY_INFO DEBUG_INFO
> +
> typedef struct _MANAGEABILITY_PROTOCOL_NAME
> MANAGEABILITY_PROTOCOL_NAME;
>
> +typedef struct {
> + UINT8 *PayloadPointer;
> + UINT32 PayloadSize;
> +} MANAGEABILITY_TRANSMISSION_PACKAGE_ATTR;
> +
> +//
> +// The information of multi portions of payload it is // splitted
> +according to transport interface Maximum // Transfer Unit.
> +typedef struct {
> + UINT16 NumberOfPackages; ///< Number
> of packages in
> MultiPackages.
> + MANAGEABILITY_TRANSMISSION_PACKAGE_ATTR MultiPackages[];
> +} MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES;
> +
> /**
> Helper function returns the human readable name of Manageability
> specification.
>
> @@ -90,4 +106,86 @@ HelperInitManageabilityTransport (
> OUT MANAGEABILITY_TRANSPORT_ADDITIONAL_STATUS
> *TransportAdditionalStatus OPTIONAL
> );
>
> +/**
> + This function splits payload into multiple packages according to
> + the given transport interface Maximum Transfer Unit (MTU).
> +
> + @param[in] PreambleSize The additional data size precedes
> + each package.
> + @param[in] PostambleSize The additional data size succeeds
> + each package.
> + @param[in] Payload Pointer to payload.
> + @param[in] PayloadSize Payload size in byte.
> + @param[in] MaximumTransferUnit MTU of transport interface.
> + @param[out] MultiplePackages Pointer to receive
> + MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES
> + structure. Caller has to free the memory
> + allocated for
> MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES.
> +
> + @retval EFI_SUCCESS
> MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES structure
> + is returned successfully.
> + @retval EFI_OUT_OF_RESOURCE Not enough resource to create
> + MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES
> structure.
> +**/
> +EFI_STATUS
> EFIAPI missing, is it intentional?
> +HelperManageabilitySplitPayload (
> + IN UINT16 PreambleSize,
> + IN UINT16 PostambleSize,
> + IN UINT8 *Payload,
> + IN UINT32 PayloadSize,
> + IN UINT32 MaximumTransferUnit,
> + OUT MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES
> **MultiplePackages
> + );
> +
> +/**
> + This function generates CRC8 with given polynomial.
> +
> + @param[in] Polynomial Polynomial in 8-bit.
> + @param[in] CrcInitialValue CRC initial value.
> + @param[in] BufferStart Pointer to buffer starts the CRC calculation.
> + @param[in] BufferSize Size of buffer.
> +
> + @retval UINT8 CRC value.
> +**/
> +UINT8
> EFIAPI is missing, its common header file.
> +HelperManageabilityGenerateCrc8 (
> + IN UINT8 Polynomial,
> + IN UINT8 CrcInitialValue,
> + IN UINT8 *BufferStart,
> + IN UINT32 BufferSize
> + );
> +
> +/**
> + Print out manageability transmit payload to the debug output device.
> +
> + @param[in] Payload Payload to print.
> + @param[in] PayloadSize Payload size.
> +
> +**/
> +VOID
> +EFIAPI
> +HelperManageabilityPayLoadDebugPrint (
> + IN VOID *Payload,
> + IN UINT32 PayloadSize
> + );
> +
> +/**
> + Prints a debug message and manageability payload to the debug output
> device.
> +
> + @param[in] Payload Payload to print.
> + @param[in] PayloadSize Payload size.
> + @param[in] Format The format string for the debug message to print.
> + @param[in] ... The variable argument list whose contents are
> accessed
> + based on the format string specified by Format.
> +
> +**/
> +VOID
> +EFIAPI
> +HelperManageabilityDebugPrint (
> + IN VOID *Payload,
> + IN UINT32 PayloadSize,
> + IN CONST CHAR8 *Format,
> + ...
> + );
> +
> #endif
> diff --git
> a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib
> /BaseManageabilityTransportHelper.c
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib
> /BaseManageabilityTransportHelper.c
> index c3f35b7beb..0e241ca3fd 100644
> ---
> a/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelperLib
> /BaseManageabilityTransportHelper.c
> +++
> b/Features/ManageabilityPkg/Library/BaseManageabilityTransportHelper
> +++ Lib/BaseManageabilityTransportHelper.c
> @@ -8,11 +8,12 @@
> #include <Uefi.h>
> #include <Library/BaseMemoryLib.h>
> #include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> #include <Library/ManageabilityTransportHelperLib.h>
>
> //
> // BaseManageabilityTransportHelper is used by PEI, DXE and SMM.
> -// Make sure the global variables added here should be unchangable.
> +// Make sure the global variables added here should be unchangeable.
> //
> MANAGEABILITY_SPECIFICATION_NAME ManageabilitySpecNameTable[] =
> {
> { &gManageabilityTransportKcsGuid, L"KCS" },
> @@ -47,8 +48,8 @@ HelperManageabilitySpecName (
> return NULL;
> }
>
> - if (SpecificationGuid == NULL || IsZeroGuid (SpecificationGuid)) {
> - DEBUG((DEBUG_ERROR, "%a: Improper input GUIDs, could be NULL or
> zero GUID.\n", __FUNCTION__));
> + if ((SpecificationGuid == NULL) || IsZeroGuid (SpecificationGuid)) {
> + DEBUG ((DEBUG_ERROR, "%a: Improper input GUIDs, could be NULL or
> + zero GUID.\n", __FUNCTION__));
> return NULL;
> }
>
> @@ -99,12 +100,13 @@ HelperManageabilityCheckSupportedSpec (
> return EFI_INVALID_PARAMETER;
> }
>
> - if (TransportGuid == NULL ||
> + if ((TransportGuid == NULL) ||
> IsZeroGuid (TransportGuid) ||
> - ManageabilityProtocolToCheck == NULL ||
> + (ManageabilityProtocolToCheck == NULL) ||
> IsZeroGuid (ManageabilityProtocolToCheck)
> - ) {
> - DEBUG((DEBUG_ERROR, "%a: Improper input GUIDs, could be NULL or
> zero GUID.\n", __FUNCTION__));
> + )
> + {
> + DEBUG ((DEBUG_ERROR, "%a: Improper input GUIDs, could be NULL or
> + zero GUID.\n", __FUNCTION__));
> return EFI_INVALID_PARAMETER;
> }
>
> @@ -116,7 +118,7 @@ HelperManageabilityCheckSupportedSpec (
> ))
> {
> DEBUG ((
> - DEBUG_VERBOSE,
> + DEBUG_MANAGEABILITY_INFO,
> "%a: Transport interface %s supports %s manageability
> specification.\n",
> __FUNCTION__,
> HelperManageabilitySpecName (TransportGuid), @@ -174,7 +176,7 @@
> HelperAcquireManageabilityTransport (
> return EFI_UNSUPPORTED;
> }
>
> - DEBUG ((DEBUG_INFO, " Manageability protocol %s is going to acquire
> transport interface token...\n", ManageabilityProtocolName));
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, " Manageability protocol %s is
> + going to acquire transport interface token...\n",
> + ManageabilityProtocolName));
>
> Status = AcquireTransportSession (ManageabilityProtocolSpec,
> TransportToken);
> if (Status == EFI_UNSUPPORTED) {
> @@ -199,7 +201,7 @@ HelperAcquireManageabilityTransport (
> return EFI_UNSUPPORTED;
> }
>
> - DEBUG ((DEBUG_INFO, "%a: This is the transfer session for %s over %s\n",
> __FUNCTION__, ManageabilityProtocolName,
> ManageabilityTransportName));
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%a: This is the transfer session
> + for %s over %s\n", __FUNCTION__, ManageabilityProtocolName,
> + ManageabilityTransportName));
> return Status;
> }
>
> @@ -259,3 +261,206 @@ HelperInitManageabilityTransport (
>
> return Status;
> }
> +
> +/**
> + This function generates CRC8 with given polynomial.
> +
> + @param[in] Polynomial Polynomial in 8-bit.
> + @param[in] CrcInitialValue CRC initial value.
> + @param[in] BufferStart Pointer to buffer starts the CRC calculation.
> + @param[in] BufferSize Size of buffer.
> +
> + @retval UINT8 CRC value.
> +**/
> +UINT8
> EFIAPI missing, is it intentional?
> +HelperManageabilityGenerateCrc8 (
> + IN UINT8 Polynomial,
> + IN UINT8 CrcInitialValue,
> + IN UINT8 *BufferStart,
> + IN UINT32 BufferSize
> + )
> +{
> + UINT8 BitIndex;
> + UINT32 BufferIndex;
> +
> + BufferIndex = 0;
> + while (BufferIndex < BufferSize) {
> + CrcInitialValue = CrcInitialValue ^ *(BufferStart + BufferIndex);
> + BufferIndex++;
> +
> + for (BitIndex = 0; BitIndex < 8; BitIndex++) {
> + if ((CrcInitialValue & 0x80) != 0) {
> + CrcInitialValue = (CrcInitialValue << 1) ^ Polynomial;
> + } else {
> + CrcInitialValue <<= 1;
> + }
> + }
> + }
> +
> + return CrcInitialValue;
> +}
> +
> +/**
> + This function splits payload into multiple packages according to
> + the given transport interface Maximum Transfer Unit (MTU).
> +
> +
> + @param[in] PreambleSize The additional data size precedes
> + each package.
> + @param[in] PostambleSize The additional data size succeeds
> + each package.
> + @param[in] Payload Pointer to payload.
> + @param[in] PayloadSize Payload size in byte.
> + @param[in] MaximumTransferUnit MTU of transport interface.
> + @param[out] MultiplePackages Pointer to receive
> + MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES
> + structure. Caller has to free the memory
> + allocated for
> MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES.
> +
> + @retval EFI_SUCCESS
> MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES structure
> + is returned successfully.
> + @retval EFI_OUT_OF_RESOURCE Not enough resource to create
> + MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES
> structure.
> This function also returns EFI_INVALID_PARAMETER.
> +**/
> +EFI_STATUS
> EFIAPI missing, Is it intentional ?
> +HelperManageabilitySplitPayload (
> + IN UINT16 PreambleSize,
> + IN UINT16 PostambleSize,
> + IN UINT8 *Payload,
> + IN UINT32 PayloadSize,
> + IN UINT32 MaximumTransferUnit,
> + OUT MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES
> **MultiplePackages
> + )
> +{
> + UINT16 NumberOfPackages;
> + UINT16 IndexOfPackage;
> + UINT32 PackagePayloadSize;
> + UINT32 TotalPayloadRemaining;
> + MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES
> *ThisMultiplePackages;
> + MANAGEABILITY_TRANSMISSION_PACKAGE_ATTR *ThisPackage;
> +
> + if ((INT16)(MaximumTransferUnit - PreambleSize - PostambleSize) < 0) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: (Preamble 0x%x + PostambleSize 0x%x) is greater than
> MaximumTransferUnit 0x%x.\n",
> + __FUNCTION__,
> + PreambleSize,
> + PostambleSize,
> + MaximumTransferUnit
> + ));
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + PackagePayloadSize = MaximumTransferUnit -PreambleSize -
> PostambleSize;
> + NumberOfPackages = (UINT16)((PayloadSize + (PackagePayloadSize - 1))
> / PackagePayloadSize);
> + ThisMultiplePackages =
> (MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES *)AllocateZeroPool (
> +
> sizeof
> (MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES) +
> +
> + sizeof (MANAGEABILITY_TRANSMISSION_PACKAGE_ATTR) *
> NumberOfPackages
> +
> + ); if (ThisMultiplePackages == NULL) {
> + DEBUG ((DEBUG_ERROR, "%a: Not enough memory for
> MANAGEABILITY_TRANSMISSION_MULTI_PACKAGES\n"));
> + return EFI_INVALID_PARAMETER;
> return EFI_OUT_OF_RESOURCE.
> + }
> +
> + ThisMultiplePackages->NumberOfPackages = NumberOfPackages;
> + ThisPackage =
> (MANAGEABILITY_TRANSMISSION_PACKAGE_ATTR *)(ThisMultiplePackages
> + 1);
> + TotalPayloadRemaining = PayloadSize;
> + for (IndexOfPackage = 0; IndexOfPackage < NumberOfPackages;
> IndexOfPackage++) {
> + ThisPackage->PayloadPointer = Payload + (IndexOfPackage *
> PackagePayloadSize);
> + ThisPackage->PayloadSize = MIN (TotalPayloadRemaining,
> PackagePayloadSize);
> + TotalPayloadRemaining -= ThisPackage->PayloadSize;
> + ThisPackage++;
> + }
> +
> + if (TotalPayloadRemaining != 0) {
> + DEBUG ((DEBUG_ERROR, "%a: Error processing multiple packages
> + (TotalPayloadRemaining != 0)\n", __FUNCTION__));
> Does ThisMultiplePackages requires to free the space before returning?
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + *MultiplePackages = ThisMultiplePackages;
> + return EFI_SUCCESS;
> +}
> +
> +/**
> + Print out manageability transmit payload to the debug output device.
> +
> + @param[in] Payload Payload to print.
> + @param[in] PayloadSize Payload size.
> +
> +**/
> +VOID
> +EFIAPI
> +HelperManageabilityPayLoadDebugPrint (
> + IN VOID *Payload,
> + IN UINT32 PayloadSize
> + )
> +{
> + UINT16 Page256;
> + UINT16 Row16;
> + UINT16 Column16;
> + UINT32 RemainingBytes;
> + UINT32 TotalBytePrinted;
> +
> + RemainingBytes = PayloadSize;
> + TotalBytePrinted = 0;
> + while (TRUE) {
> + if (TotalBytePrinted % 256 == 0) {
> + Page256 = (UINT16)TotalBytePrinted / 256;
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, "======== Manageability
> Payload %04xH - %04xH =========\n", Page256 * 256, Page256 * 256 + MIN
> (RemainingBytes, 256) - 1));
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, " "));
> + for (Column16 = 0; Column16 < 16; Column16++) {
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%02x ", Column16));
> + }
> +
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, "\n
> -----------------------------
> ------------------\n"));
> + }
> +
> + for (Row16 = 0; Row16 < 16; Row16++) {
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%04x | ", Page256 * 256 +
> Row16 * 16));
> + for (Column16 = 0; Column16 < MIN (RemainingBytes, 16); Column16++)
> {
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, "%02x ", *((UINT8 *)Payload
> + Page256 * 256 + Row16 * 16 + Column16)));
> + }
> +
> + RemainingBytes -= Column16;
> + TotalBytePrinted += Column16;
> + if (RemainingBytes == 0) {
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, "\n\n"));
> + return;
> + }
> +
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, "\n"));
> + }
> +
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, "\n")); }
> +
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, "\n\n")); }
> +
> +/**
> + Prints a debug message and manageability payload to the debug output
> device.
> +
> + @param[in] Payload Payload to print.
> + @param[in] PayloadSize Payload size.
> + @param[in] Format The format string for the debug message to print.
> + @param[in] ... The variable argument list whose contents are
> accessed
> + based on the format string specified by Format.
> +
> +**/
> +VOID
> +EFIAPI
> +HelperManageabilityDebugPrint (
> + IN VOID *Payload,
> + IN UINT32 PayloadSize,
> + IN CONST CHAR8 *Format,
> + ...
> + )
> +{
> + VA_LIST Marker;
> +
> + VA_START (Marker, Format);
> + DEBUG ((DEBUG_MANAGEABILITY_INFO, "Manageability Transmission: "));
> + DebugVPrint ((UINTN)DEBUG_MANAGEABILITY_INFO, Format, Marker);
> + HelperManageabilityPayLoadDebugPrint (Payload, PayloadSize);
> + VA_END (Marker);
> +}
> --
> 2.37.1.windows.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103310): https://edk2.groups.io/g/devel/message/103310
Mute This Topic: https://groups.io/mt/98339112/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
