Skip to content

Commit

Permalink
Replace PlatformPowerRestorePolicyConfigurationLib with PlatformPower…
Browse files Browse the repository at this point in the history
…RestorePolicy (#182)

## Description

In IpmiPowerRestorePolicy module, replace
PlatformPowerRestorePolicyConfigurationLib with
PlatformPowerRestorePolicy policy. Now, the platform specific power
restore configuration is read via.
PlatformPowerRestorePolicy.PolicyValue instead of GetPowerRestorePolicy
API. Default implementation of PlatformPowerRestorePolicy is provided
which always returns "no change".

This simplifies interface to provide platform specific configuration for
power restore and reduce the number of libraires.

- [ ] Impacts functionality?
- **Functionality** - Does the change ultimately impact how firmware
functions?
- Examples: Add a new library, publish a new PPI, update an algorithm,
...
- [ ] Impacts security?
- **Security** - Does the change have a direct security impact on an
application,
    flow, or firmware?
  - Examples: Crypto algorithm change, buffer overflow fix, parameter
    validation improvement, ...
- [x] Breaking change?
- **Breaking change** - Will anyone consuming this change experience a
break
    in build or boot behavior?
- Examples: Add a new library class, move a module to a different repo,
call
    a function in a new library class in a pre-existing module, ...
- [x] Includes tests?
  - **Tests** - Does the change include any explicit test code?
  - Examples: Unit tests, integration tests, robot tests, ...
- [ ] Includes documentation?
- **Documentation** - Does the change contain explicit documentation
additions
    outside direct code modifications (and comments)?
- Examples: Update readme file, add feature readme file, link to
documentation
    on an a separate Web page, ...

## How This Was Tested

This was tested with consumer build pipeline on proprietary platforms.

## Integration Instructions

1. Remove PlatformPowerRestorePolicyConfigurationLib library
implementation in the platform code.
2. Use PlatformPowerRestorePolicyDefault.inf or introduce platform code
to produce PlatformPowerRestorePolicy policy.
  • Loading branch information
shrugupt authored Nov 22, 2023
1 parent 5b35751 commit 197d948
Show file tree
Hide file tree
Showing 13 changed files with 222 additions and 163 deletions.
32 changes: 32 additions & 0 deletions IpmiFeaturePkg/Include/Guid/PlatformPowerRestorePolicy.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/** @file PlatformPowerRestorePolicy.h
Platform Power Restore Policy Definition
Copyright (c) Microsoft Corporation.
SPDX-License-Identifier: BSD-2-Clause-Patent
**/

#ifndef PLATFORM_POWER_RESTORE_POLICY_H_
#define PLATFORM_POWER_RESTORE_POLICY_H_

#define PLATFORM_POWER_RESTORE_POLICY_GUID {0x85bcbff7, 0x8f9d, 0x4997, {0xac, 0x46, 0x5b, 0x36, 0x70, 0x0b, 0x0b, 0x85}}

//
// Power Restore Policy definition from IPMI Spec.
//
typedef enum {
PowerRestorePolicyPowerOff = 0x00,
PowerRestorePolicyLastState = 0x01,
PowerRestorePolicyPowerOn = 0x02,
PowerRestorePolicyNoChange = 0x03,
} PLATFORM_POWER_RESTORE_POLICY_TYPE;

#pragma pack(1)

typedef struct _PLATFORM_POWER_RESTORE_POLICY {
PLATFORM_POWER_RESTORE_POLICY_TYPE PolicyValue;
} PLATFORM_POWER_RESTORE_POLICY;

#pragma pack()

#endif //#ifdef PLATFORM_POWER_RESTORE_POLICY_H_

This file was deleted.

2 changes: 1 addition & 1 deletion IpmiFeaturePkg/IpmiFeaturePkg.dec
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,12 @@
IpmiWatchdogLib|Include/Library/IpmiWatchdogLib.h
IpmiBootOptionLib|Include/Library/IpmiBootOptionLib.h
PlatformCmosClearLib|Include/Library/PlatformCmosClearLib.h
PlatformPowerRestorePolicyConfigurationLib|Include/Library/PlatformPowerRestorePolicyConfigurationLib.h

[Guids]
gIpmiFeaturePkgTokenSpaceGuid = {0xc05283f6, 0xd6a8, 0x48f3, {0x9b, 0x59, 0xfb, 0xca, 0x71, 0x32, 0x0f, 0x12}}
gIpmiBmcHobGuid = {0x3d133ac1, 0x8565, 0x4a08, {0xac, 0x50, 0x9d, 0xff, 0x55, 0x29, 0xac, 0x12}}
gIpmiWatchdogPolicyGuid = {0x6b53a598, 0x4ff5, 0x43c5, {0x81, 0x83, 0x74, 0xd0, 0xe6, 0x34, 0xcd, 0x66}}
gPlatformPowerRestorePolicyGuid = {0x85bcbff7, 0x8f9d, 0x4997, {0xac, 0x46, 0x5b, 0x36, 0x70, 0x0b, 0x0b, 0x85}}

[Ppis]
gPeiIpmiTransportPpiGuid = {0x7bf5fecc, 0xc5b5, 0x4b25, {0x81, 0x1b, 0xb4, 0xb5, 0xb, 0x28, 0x79, 0xf7}}
Expand Down
3 changes: 1 addition & 2 deletions IpmiFeaturePkg/IpmiFeaturePkg.dsc
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@
IpmiTransportLib|IpmiFeaturePkg/Library/IpmiTransportLibNull/IpmiTransportLibNull.inf
IpmiPlatformLib|IpmiFeaturePkg/Library/IpmiPlatformLibNull/IpmiPlatformLibNull.inf
PlatformCmosClearLib|IpmiFeaturePkg/Library/PlatformCmosClearLibNull/PlatformCmosClearLibNull.inf
PlatformPowerRestorePolicyConfigurationLib|IpmiFeaturePkg/Library/PlatformPowerRestorePolicyConfigurationLibNull/PlatformPowerRestorePolicyConfigurationLibNull.inf

[LibraryClasses.common.PEI_CORE,LibraryClasses.common.PEIM]
#######################################
Expand Down Expand Up @@ -112,7 +111,7 @@
IpmiFeaturePkg/Library/IpmiBootOptionLib/IpmiBootOptionLib.inf
IpmiFeaturePkg/IpmiCmosClear/IpmiCmosClear.inf
IpmiFeaturePkg/Library/PlatformCmosClearLibNull/PlatformCmosClearLibNull.inf
IpmiFeaturePkg/Library/PlatformPowerRestorePolicyConfigurationLibNull/PlatformPowerRestorePolicyConfigurationLibNull.inf
IpmiFeaturePkg/PlatformPowerRestorePolicyDefault/PlatformPowerRestorePolicyDefault.inf

# Transport Libraries
IpmiFeaturePkg/Library/IpmiTransportLibNull/IpmiTransportLibNull.inf
Expand Down
19 changes: 11 additions & 8 deletions IpmiFeaturePkg/IpmiPowerRestorePolicy/IpmiPowerRestorePolicy.c
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
#include <PiPei.h>
#include <Library/DebugLib.h>
#include <Library/IpmiCommandLib.h>
#include <Library/PlatformPowerRestorePolicyConfigurationLib.h>
#include <Guid/PlatformPowerRestorePolicy.h>
#include <Library/PolicyLib.h>

#define POWER_RESTORE_POLICY_UNKNOWN 0x03 // "Get Chassis Status"

/**
Configure Power Restore Policy via IPMI
Expand All @@ -31,9 +34,10 @@ IpmiPowerRestorePolicyEntry (
EFI_STATUS Status;
IPMI_GET_CHASSIS_STATUS_RESPONSE ChassisStatusRespData;
UINT8 CurrentPowerRestorePolicy;
UINT8 PlatformPowerRestorePolicy;
PLATFORM_POWER_RESTORE_POLICY PlatformPowerRestorePolicy;
IPMI_SET_POWER_RESTORE_POLICY_REQUEST SetRestorePolicyRequest;
IPMI_SET_POWER_RESTORE_POLICY_RESPONSE SetRestorePolicyResponse;
UINT16 PolicySize = sizeof (PLATFORM_POWER_RESTORE_POLICY);

//
// Get current power restore policy setting
Expand All @@ -52,16 +56,15 @@ IpmiPowerRestorePolicyEntry (
//
// Get platform power restore policy setting
//
Status = GetPowerRestorePolicy (&PlatformPowerRestorePolicy);

Status = GetPolicy (&gPlatformPowerRestorePolicyGuid, NULL, &PlatformPowerRestorePolicy, &PolicySize);
if (EFI_ERROR (Status)) {
DEBUG ((DEBUG_ERROR, "[%a] - GetPowerRestorePolicy: %r\n", __FUNCTION__, Status));
DEBUG ((DEBUG_ERROR, "%a: Failed to get platform power restore policy! %r\n", __FUNCTION__, Status));
return Status;
}

DEBUG ((DEBUG_VERBOSE, "[%a] - PlatformPowerRestorePolicy: 0x%x\n", __FUNCTION__, PlatformPowerRestorePolicy));

if ( PlatformPowerRestorePolicy == CurrentPowerRestorePolicy) {
if ( PlatformPowerRestorePolicy.PolicyValue == CurrentPowerRestorePolicy) {
// Just return if CurrentPowerRestorePolicy the same with PlatformPowerRestorePolicy
DEBUG ((DEBUG_VERBOSE, "[%a] - Current Power Restore Policy is consistent with platform setting\n", __FUNCTION__));
return EFI_SUCCESS;
Expand All @@ -70,8 +73,8 @@ IpmiPowerRestorePolicyEntry (
//
// If platform setting is not POWER_RESTORE_POLICY_NO_CHANGE, then configure the power restore policy based on PlatformPowerRestorePolicy
//
if (PlatformPowerRestorePolicy != POWER_RESTORE_POLICY_NO_CHANGE) {
SetRestorePolicyRequest.PowerRestorePolicy.Bits.PowerRestorePolicy = PlatformPowerRestorePolicy;
if (PlatformPowerRestorePolicy.PolicyValue != PowerRestorePolicyNoChange) {
SetRestorePolicyRequest.PowerRestorePolicy.Bits.PowerRestorePolicy = (UINT8)PlatformPowerRestorePolicy.PolicyValue;
SetRestorePolicyRequest.PowerRestorePolicy.Bits.Reserved = 0;
Status = IpmiSetPowerRestorePolicy (&SetRestorePolicyRequest, &SetRestorePolicyResponse);
if (EFI_ERROR (Status)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,19 @@
MdePkg/MdePkg.dec
MdeModulePkg/MdeModulePkg.dec
IpmiFeaturePkg/IpmiFeaturePkg.dec
PolicyServicePkg/PolicyServicePkg.dec

[LibraryClasses]
PeimEntryPoint
DebugLib
BaseLib
IpmiCommandLib
PlatformPowerRestorePolicyConfigurationLib
PolicyLib

[Guids]
gPlatformPowerRestorePolicyGuid

[Depex]
gPeiIpmiTransportPpiGuid
gPeiIpmiTransportPpiGuid AND
gPlatformPowerRestorePolicyGuid

36 changes: 26 additions & 10 deletions IpmiFeaturePkg/IpmiPowerRestorePolicy/UnitTest/MockApi.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,38 @@
#include <PiPei.h>
#include <Library/DebugLib.h>
#include <Library/IpmiCommandLib.h>
#include <Library/PlatformPowerRestorePolicyConfigurationLib.h>
#include <Library/PolicyLib.h>
#include <Library/BaseMemoryLib.h>

/**
@brief Mock this api locally in the test Get the Platform Power Restore Policy Setting Value object
@param[out] PowerRestorePolicy
@retval EFI_STATUS
**/
* @brief Mock version of GetPolicy
*
*/
EFI_STATUS
EFIAPI
GetPowerRestorePolicy (
OUT UINT8 *PowerRestorePolicy
GetPolicy (
IN CONST EFI_GUID *PolicyGuid,
OUT UINT64 *Attributes OPTIONAL,
OUT VOID *Policy,
IN OUT UINT16 *PolicySize
)
{
*PowerRestorePolicy = mock_type (UINT8);
return mock_type (UINTN);
EFI_STATUS Status;

// Check that this is the right guid being used
check_expected_ptr (PolicyGuid);

Status = mock_type (EFI_STATUS);

if (!EFI_ERROR (Status)) {
// Set Attributes, Policy and PolicySize
*PolicySize = (UINT16)mock ();
if (Policy != NULL) {
CopyMem ((VOID *)Policy, (VOID *)mock (), *PolicySize);
}
}

return Status;
}

EFI_STATUS
Expand Down
Loading

0 comments on commit 197d948

Please sign in to comment.