[edk2-devel] [edk2-redfish-client][PATCH 1/6] RedfishClientPkg/RedfishETagDxe: fix issue and enhancement.

["Nickle Wang via groups.io" <nicklew=nvidia.com@groups.io>]

-Fix typo (replace Listheader to ListHeader).
-Replace "%a," to "%a:".
-Add more debug message.
-Remove the exit-boot-service event and use after-provisioning
event to save ETag data. Variable may not be writable during
exit-boot-service callback.

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>
---
 .../RedfishETagDxe/RedfishETagDxe.inf         |  5 +-
 .../RedfishETagDxe/RedfishETagDxe.h           |  4 +-
 .../RedfishETagDxe/RedfishETagDxe.c           | 84 ++++++++++---------
 3 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.inf b/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.inf
index 4aecdb41..3626f775 100644
--- a/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.inf
+++ b/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.inf
@@ -1,7 +1,7 @@
 ## @file
 #
 #  (C) Copyright 2021 Hewlett Packard Enterprise Development LP<BR>
-#  Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+#  Copyright (c) 2022-2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -35,6 +35,7 @@
   UefiBootServicesTableLib
   UefiRuntimeServicesTableLib
   UefiDriverEntryPoint
+  RedfishEventLib
 
 [Protocols]
   gEdkIIRedfishETagProtocolGuid           ## PRODUCED ##
@@ -44,4 +45,4 @@
   gEfiRedfishClientVariableGuid           ## CONSUMED ##
 
 [Depex]
-  TRUE
+  gEfiVariableArchProtocolGuid
diff --git a/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.h b/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.h
index 3542f289..c8302830 100644
--- a/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.h
+++ b/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.h
@@ -2,6 +2,7 @@
   Common header file for RedfishETagDxe driver.
 
   (C) Copyright 2021-2022 Hewlett Packard Enterprise Development LP<BR>
+  Copyright (c) 2023, NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -25,6 +26,7 @@
 #include <Library/UefiDriverEntryPoint.h>
 #include <Library/UefiLib.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
+#include <Library/RedfishEventLib.h>
 
 #include <Protocol/EdkIIRedfishETagProtocol.h>
 
@@ -49,7 +51,7 @@ typedef struct {
 // Definition of REDFISH_ETAG_LIST
 //
 typedef struct {
-  LIST_ENTRY    Listheader;
+  LIST_ENTRY    ListHeader;
   UINTN         TotalSize;
   UINTN         Count;
 } REDFISH_ETAG_LIST;
diff --git a/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.c b/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.c
index f731d39d..f303604a 100644
--- a/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.c
+++ b/RedfishClientPkg/RedfishETagDxe/RedfishETagDxe.c
@@ -124,7 +124,7 @@ AddETagRecord (
     return EFI_OUT_OF_RESOURCES;
   }
 
-  InsertTailList (&List->Listheader, &NewRecord->List);
+  InsertTailList (&List->ListHeader, &NewRecord->List);
   ++List->Count;
   List->TotalSize += NewRecord->Size;
 
@@ -225,20 +225,20 @@ DumpETagList (
     DEBUG ((DEBUG_ERROR, "%s\n", Msg));
   }
 
-  if (IsListEmpty (&ETagList->Listheader)) {
+  if (IsListEmpty (&ETagList->ListHeader)) {
     DEBUG ((DEBUG_MANAGEABILITY, "ETag list is empty\n"));
     return EFI_NOT_FOUND;
   }
 
   DEBUG ((DEBUG_MANAGEABILITY, "Count: %d Total Size: %d\n", ETagList->Count, ETagList->TotalSize));
   Record = NULL;
-  List   = GetFirstNode (&ETagList->Listheader);
-  while (!IsNull (&ETagList->Listheader, List)) {
+  List   = GetFirstNode (&ETagList->ListHeader);
+  while (!IsNull (&ETagList->ListHeader, List)) {
     Record = REDFISH_ETAG_RECORD_FROM_LIST (List);
 
     DEBUG ((DEBUG_MANAGEABILITY, "ETag: %a Uri: %a Size: %d\n", Record->ETag, Record->Uri, Record->Size));
 
-    List = GetNextNode (&ETagList->Listheader, List);
+    List = GetNextNode (&ETagList->ListHeader, List);
   }
 
   return EFI_SUCCESS;
@@ -304,16 +304,16 @@ ReleaseETagList (
     return EFI_INVALID_PARAMETER;
   }
 
-  if (IsListEmpty (&ETagList->Listheader)) {
+  if (IsListEmpty (&ETagList->ListHeader)) {
     return EFI_SUCCESS;
   }
 
   Record = NULL;
   Next   = NULL;
-  List   = GetFirstNode (&ETagList->Listheader);
-  while (!IsNull (&ETagList->Listheader, List)) {
+  List   = GetFirstNode (&ETagList->ListHeader);
+  while (!IsNull (&ETagList->ListHeader, List)) {
     Record = REDFISH_ETAG_RECORD_FROM_LIST (List);
-    Next   = GetNextNode (&ETagList->Listheader, List);
+    Next   = GetNextNode (&ETagList->ListHeader, List);
 
     DeleteETagRecord (ETagList, Record);
 
@@ -352,14 +352,14 @@ SaveETagList (
     return EFI_INVALID_PARAMETER;
   }
 
-  if (IsListEmpty (&ETagList->Listheader)) {
+  if (IsListEmpty (&ETagList->ListHeader)) {
     return EFI_SUCCESS;
   }
 
   //
   // Calculate the total size we need to keep ETag list.
   //
-  VarSize = ETagList->TotalSize + 1; // terminator character
+  VarSize = ETagList->TotalSize + sizeof (CHAR8); // terminator character
   VarData = AllocateZeroPool (VarSize);
   if (VarData == NULL) {
     return EFI_OUT_OF_RESOURCES;
@@ -367,8 +367,8 @@ SaveETagList (
 
   Seeker = VarData;
   Record = NULL;
-  List   = GetFirstNode (&ETagList->Listheader);
-  while (!IsNull (&ETagList->Listheader, List)) {
+  List   = GetFirstNode (&ETagList->ListHeader);
+  while (!IsNull (&ETagList->ListHeader, List)) {
     Record = REDFISH_ETAG_RECORD_FROM_LIST (List);
 
     StrSize = AsciiStrSize (Record->Uri);
@@ -386,7 +386,7 @@ SaveETagList (
 
     ++Seeker;
 
-    List = GetNextNode (&ETagList->Listheader, List);
+    List = GetNextNode (&ETagList->ListHeader, List);
   }
 
   *Seeker = '\0';
@@ -468,7 +468,7 @@ InitialETagList (
     //
     Seeker = AsciiStrStr (UriPointer, "|");
     if (Seeker == NULL) {
-      DEBUG ((DEBUG_ERROR, "%a, data corrupted\n", __func__));
+      DEBUG ((DEBUG_ERROR, "%a: data corrupted\n", __func__));
       Status = EFI_DEVICE_ERROR;
       goto ON_ERROR;
     }
@@ -481,7 +481,7 @@ InitialETagList (
     //
     Seeker = AsciiStrStr (ETagPointer, "\n");
     if (Seeker == NULL) {
-      DEBUG ((DEBUG_ERROR, "%a, data corrupted\n", __func__));
+      DEBUG ((DEBUG_ERROR, "%a: data corrupted\n", __func__));
       Status = EFI_DEVICE_ERROR;
       goto ON_ERROR;
     }
@@ -532,11 +532,13 @@ RedfishETagGet (
     return EFI_INVALID_PARAMETER;
   }
 
+  DEBUG ((DEBUG_MANAGEABILITY, "%a: %a\n", __func__, Uri));
+
   Private = REDFISH_ETAG_PRIVATE_FROM_THIS (This);
 
   *ETag = NULL;
 
-  Target = FindETagRecord (&Private->ETagList.Listheader, Uri);
+  Target = FindETagRecord (&Private->ETagList.ListHeader, Uri);
   if (Target == NULL) {
     return EFI_NOT_FOUND;
   }
@@ -551,7 +553,8 @@ RedfishETagGet (
 
   @param[in]   This                Pointer to EDKII_REDFISH_ETAG_PROTOCOL instance.
   @param[in]   Uri                 The target Uri which related to ETag.
-  @param[in]   ETag                The ETag to add. If ETag is NULL, the record of corresponding URI will be removed.
+  @param[in]   ETag                The ETag to add. If ETag is NULL, the record of
+                                   corresponding URI will be removed.
 
   @retval EFI_SUCCESS              This handler has been stoped successfully.
   @retval Others                   Some error happened.
@@ -573,10 +576,12 @@ RedfishETagSet (
     return EFI_INVALID_PARAMETER;
   }
 
+  DEBUG ((DEBUG_MANAGEABILITY, "%a: %a -> %a\n", __func__, Uri, (ETag == NULL ? "NULL" : ETag)));
+
   Private = REDFISH_ETAG_PRIVATE_FROM_THIS (This);
 
   Status = EFI_NOT_FOUND;
-  Target = FindETagRecord (&Private->ETagList.Listheader, Uri);
+  Target = FindETagRecord (&Private->ETagList.ListHeader, Uri);
   if (Target != NULL) {
     //
     // Remove old one and create new one.
@@ -620,14 +625,16 @@ RedfishETagFlush (
 
   Status = SaveETagList (&Private->ETagList, Private->VariableName);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a, save ETag list to variable: %s failed: %r\n", __func__, Private->VariableName, Status));
+    DEBUG ((DEBUG_ERROR, "%a: save ETag list to variable: %s failed: %r\n", __func__, Private->VariableName, Status));
   }
 
+  DEBUG ((DEBUG_MANAGEABILITY, "%a: save ETag list to variable: %s\n", __func__, Private->VariableName));
+
   return Status;
 }
 
 /**
-  Callback function executed when the ExitBootService event group is signaled.
+  Callback function executed when the AfterProvisioning event group is signaled.
 
   @param[in]   Event    Event whose notification function is being invoked.
   @param[out]  Context  Pointer to the Context buffer
@@ -635,13 +642,13 @@ RedfishETagFlush (
 **/
 VOID
 EFIAPI
-RedfishETagOnExitBootService (
+RedfishETagOnRedfishAfterProvisioning (
   IN  EFI_EVENT  Event,
   OUT VOID       *Context
   )
 {
   //
-  // Memory is about to be released. Keep list into variable.
+  // Redfish provisioning is finished. Keep ETag into variable.
   //
   RedfishETagFlush (&mRedfishETagPrivate->Protocol);
 }
@@ -670,7 +677,7 @@ RedfishETagDriverUnload (
                     (VOID *)&mRedfishETagPrivate->Protocol
                     );
     if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "%a, can not uninstall gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
+      DEBUG ((DEBUG_ERROR, "%a: can not uninstall gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
       ASSERT (FALSE);
     }
 
@@ -725,7 +732,7 @@ RedfishETagDriverEntryPoint (
     return EFI_OUT_OF_RESOURCES;
   }
 
-  InitializeListHead (&mRedfishETagPrivate->ETagList.Listheader);
+  InitializeListHead (&mRedfishETagPrivate->ETagList.ListHeader);
   mRedfishETagPrivate->VariableName = AllocateCopyPool (StrSize (ETAG_VARIABLE_NAME), ETAG_VARIABLE_NAME);
   if (mRedfishETagPrivate->VariableName == NULL) {
     Status = EFI_OUT_OF_RESOURCES;
@@ -742,33 +749,30 @@ RedfishETagDriverEntryPoint (
                   (VOID *)&mRedfishETagPrivate->Protocol
                   );
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a, can not install gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
+    DEBUG ((DEBUG_ERROR, "%a: can not install gEdkIIRedfishETagProtocolGuid: %r\n", __func__, Status));
     ASSERT (FALSE);
     goto ON_ERROR;
   }
 
   //
-  // Create Exit Boot Service event.
+  // Read existing record from variable.
   //
-  Status = gBS->CreateEventEx (
-                  EVT_NOTIFY_SIGNAL,
-                  TPL_CALLBACK,
-                  RedfishETagOnExitBootService,
-                  NULL,
-                  &gEfiEventExitBootServicesGuid,
-                  &mRedfishETagPrivate->Event
-                  );
+  DEBUG ((DEBUG_MANAGEABILITY, "%a: Initial ETag List from variable: %s\n", __func__, mRedfishETagPrivate->VariableName));
+  Status = InitialETagList (&mRedfishETagPrivate->ETagList, mRedfishETagPrivate->VariableName);
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: Fail to register Exit Boot Service event.", __func__));
-    goto ON_ERROR;
+    DEBUG ((DEBUG_MANAGEABILITY, "%a: Initial ETag List: %r\n", __func__, Status));
   }
 
   //
-  // Read existing record from variable.
+  // Register after provisioning event
   //
-  Status = InitialETagList (&mRedfishETagPrivate->ETagList, mRedfishETagPrivate->VariableName);
+  Status = CreateAfterProvisioningEvent (
+             RedfishETagOnRedfishAfterProvisioning,
+             NULL,
+             &mRedfishETagPrivate->Event
+             );
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_MANAGEABILITY, "%a, Initial ETag List: %r\n", __func__, Status));
+    DEBUG ((DEBUG_ERROR, "%a: failed to register after-provisioning event: %r\n", __func__, Status));
   }
 
   return EFI_SUCCESS;
-- 
2.17.1



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