Re: [edk2-devel] [edk2-redfish-client][PATCH 2/2] RedfishClientPkg/RedfishFeatureCoreDxe: notify platform before reboot.

["Chang, Abner via groups.io" <abner.chang=amd.com@groups.io>]

[AMD Official Use Only - General]

Looks good,  you can move EDKII_REDFISH_OVERRIDE_PROTOCOL  *RedfishOverride from local variable to global variable and locate this protocol only once if there are more than one use cases of Redfish override protocol in RedfishFeatureCore in the future.

Reviewed-by: Abner Chang <abner.chang@amd.com>

> -----Original Message-----
> From: Nickle Wang <nicklew@nvidia.com>
> Sent: Thursday, November 2, 2023 6:18 PM
> To: devel@edk2.groups.io
> Cc: Chang, Abner <Abner.Chang@amd.com>; Igor Kulchytskyy
> <igork@ami.com>; Nick Ramirez <nramirez@nvidia.com>
> Subject: [edk2-redfish-client][PATCH 2/2]
> RedfishClientPkg/RedfishFeatureCoreDxe: notify platform before reboot.
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Call Redfish override protocol to notify platform that the Redfish
> changes system settings and about to perform system reboot. Platform
> firmware can do platform specific operation before system rebooted.
> To follow platform policy, platform can reject system reboot by
> returning error status and Redfish feature core driver will abort system
> reboot.
>
> Signed-off-by: Nickle Wang <nicklew@nvidia.com>
> Cc: Abner Chang <abner.chang@amd.com>
> Cc: Igor Kulchytskyy <igork@ami.com>
> Cc: Nick Ramirez <nramirez@nvidia.com>
> ---
>  .../RedfishFeatureCoreDxe.inf                 |  1 +
>  .../RedfishFeatureCoreDxe.h                   |  1 +
>  .../RedfishFeatureCoreDxe.c                   | 25 +++++++++++++++++--
>  3 files changed, 25 insertions(+), 2 deletions(-)
>
> diff --git
> a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.inf
> b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.inf
> index c67525ee..320eddc9 100644
> --- a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.inf
> +++ b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.inf
> @@ -47,6 +47,7 @@
>
>  [Protocols]
>    gEdkIIRedfishFeatureProtocolGuid    ## BY_START
> +  gEdkiiRedfishOverrideProtocolGuid   ## CONSUMED
>
>  [Pcd]
>
> gEfiRedfishClientPkgTokenSpaceGuid.PcdEdkIIRedfishFeatureDriverStartupEve
> ntGuid
> diff --git
> a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.h
> b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.h
> index a94c7930..93139af2 100644
> --- a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.h
> +++ b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.h
> @@ -12,6 +12,7 @@
>  #define EDKII_REDFISH_FEATURE_CORE_DXE_H_
>
>  #include <Protocol/EdkIIRedfishFeature.h>
> +#include <Protocol/EdkIIRedfishOverrideProtocol.h>
>
>  #include <Base.h>
>  #include <Library/BaseLib.h>
> diff --git a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> index d9aa6e1a..e2ae125f 100644
> --- a/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> +++ b/RedfishClientPkg/RedfishFeatureCoreDxe/RedfishFeatureCoreDxe.c
> @@ -242,11 +242,14 @@ RedfishFeatureDriverStartup (
>    IN  VOID       *Context
>    )
>  {
> +  EFI_STATUS                       Status;
>    REDFISH_FEATURE_STARTUP_CONTEXT  *StartupContext;
>    UINT16                           RebootTimeout;
> +  EDKII_REDFISH_OVERRIDE_PROTOCOL  *RedfishOverride;
>
> -  StartupContext = (REDFISH_FEATURE_STARTUP_CONTEXT *)Context;
> -  RebootTimeout  = PcdGet16 (PcdRedfishSystemRebootTimeout);
> +  RedfishOverride = NULL;
> +  StartupContext  = (REDFISH_FEATURE_STARTUP_CONTEXT *)Context;
> +  RebootTimeout   = PcdGet16 (PcdRedfishSystemRebootTimeout);
>    //
>    // Invoke EDK2 Redfish feature driver callback to start up
>    // the Redfish operations.
> @@ -290,6 +293,24 @@ RedfishFeatureDriverStartup (
>    if (PcdGetBool (PcdRedfishSystemRebootRequired)) {
>      Print (L"System configuration is changed from RESTful interface. Reboot
> system in %d seconds...\n", RebootTimeout);
>      gBS->Stall (RebootTimeout * 1000000U);
> +
> +    //
> +    // Call override protocol to notify platform that Redfish is processed
> +    // and about to reboot system.
> +    //
> +    Status = gBS->LocateProtocol (
> +                    &gEdkiiRedfishOverrideProtocolGuid,
> +                    NULL,
> +                    (VOID **)&RedfishOverride
> +                    );
> +    if (!EFI_ERROR (Status)) {
> +      Status = RedfishOverride->NotifyPhase (RedfishOverride,
> EdkiiRedfishPhaseBeforeReboot);
> +      if (EFI_ERROR (Status)) {
> +        DEBUG ((DEBUG_ERROR, "%a: abort the reboot because NotifyPhase()
> returns failure: %r\n", __func__, Status));
> +        return;
> +      }
> +    }
> +
>      gRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
>      CpuDeadLoop ();
>    }
> --
> 2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#110687): https://edk2.groups.io/g/devel/message/110687
Mute This Topic: https://groups.io/mt/102338708/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-