Adding policy check for advanced file logger (#384)

# Preface Please ensure you have read the [contribution docs](https://github.com/microsoft/mu/blob/master/CONTRIBUTING.md) prior to submitting the pull request. In particular, [pull request guidelines](https://github.com/microsoft/mu/blob/master/CONTRIBUTING.md#pull-request-best-practices). ## Description This change added support for enablement/disablement of advanced file logger through policy services. Platforms intending to support this functionality should produce policy prior to the driver load. The default configuration is set to file logger enabled. For each item, place an "x" in between `[` and `]` if true. Example: `[x]`. _(you can also check items in the GitHub UI)_ - [x] 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, ... - [ ] 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, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [x] 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 is tested on QEMU Q35 and verified that not creating policy will remain producing log files, and disabling the policy will prevent the driver from logging any files to ESP. ## Integration Instructions In platform code, use `SetPolicy` with `gAdvancedFileLoggerPolicyGuid` to produce a policy entry for file logger driver to consume.
microsoft · Dec 12, 2023 · c67a13e · c67a13e
1 parent 045991a
commit c67a13e
Show file tree

Hide file tree

Showing 8 changed files with 134 additions and 22 deletions.
diff --git a/AdvLoggerPkg/AdvLoggerPkg.ci.yaml b/AdvLoggerPkg/AdvLoggerPkg.ci.yaml
@@ -26,7 +26,7 @@
             "MdeModulePkg/MdeModulePkg.dec",
             "AdvLoggerPkg/AdvLoggerPkg.dec",
             "MsWheaPkg/MsWheaPkg.dec",
-            "UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec",
+            "PolicyServicePkg/PolicyServicePkg.dec",
             "ShellPkg/ShellPkg.dec"
         ],
         "AcceptableDependencies-HOST_APPLICATION":[ # for host based unit tests

diff --git a/AdvLoggerPkg/AdvLoggerPkg.dec b/AdvLoggerPkg/AdvLoggerPkg.dec
@@ -46,6 +46,9 @@
   #
   gAdvancedFileLoggerWriteLogFiles = { 0xc60e894d, 0x29a4, 0x4e0b, {0xb7, 0x44, 0x77, 0x84, 0x22, 0x6e, 0x81, 0x80 }}
 
+  ## GUID for Advanced file logger policy data
+  #
+  gAdvancedFileLoggerPolicyGuid = { 0x6c3fd4f1, 0xb596, 0x438e, { 0x8a, 0xb8, 0x53, 0xf1, 0xc, 0x9c, 0x27, 0x74 } }
 
 [Ppis]
   ## Advanced Logger Ppi - Communication from PEIM to PEI_CORE library implementation

diff --git a/AdvLoggerPkg/AdvLoggerPkg.dsc b/AdvLoggerPkg/AdvLoggerPkg.dsc
@@ -87,6 +87,9 @@
 
 [LibraryClasses.common.DXE_CORE]
 
+[LibraryClasses.common.DXE_DRIVER]
+  PolicyLib|PolicyServicePkg/Library/DxePolicyLib/DxePolicyLib.inf
+
 [LibraryClasses.common.DXE_SMM_DRIVER]
   AdvancedLoggerLib|AdvLoggerPkg/Library/AdvancedLoggerLib/Smm/AdvancedLoggerLib.inf
   SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTableLib.inf

diff --git a/AdvLoggerPkg/AdvancedFileLogger/AdvancedFileLogger.c b/AdvLoggerPkg/AdvancedFileLogger/AdvancedFileLogger.c
@@ -142,17 +142,17 @@ OnResetNotificationProtocolInstalled (
     //
     // Register our reset notification request
     //
-    DEBUG ((DEBUG_INFO, "%a: Located Reset notification protocol. Registering handler\n", __FUNCTION__));
+    DEBUG ((DEBUG_INFO, "%a: Located Reset notification protocol. Registering handler\n", __func__));
     Status = ResetNotificationProtocol->RegisterResetNotify (ResetNotificationProtocol, OnResetNotification);
     if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "%a: failed to register Reset Notification handler (%r)\n", __FUNCTION__, Status));
+      DEBUG ((DEBUG_ERROR, "%a: failed to register Reset Notification handler (%r)\n", __func__, Status));
     }
 
     if (Event != NULL) {
       gBS->CloseEvent (Event);
     }
   } else {
-    DEBUG ((DEBUG_ERROR, "%a: Unable to locate Reset Notification Protocol.\n", __FUNCTION__));
+    DEBUG ((DEBUG_ERROR, "%a: Unable to locate Reset Notification Protocol.\n", __func__));
   }
 
   return;
@@ -183,7 +183,7 @@ RegisterLogDevice (
   LogDevice = (LOG_DEVICE *)AllocateZeroPool (sizeof (LOG_DEVICE));
   ASSERT (LogDevice);
   if (NULL == LogDevice) {
-    DEBUG ((DEBUG_ERROR, "%a: Out of memory:\n", __FUNCTION__));
+    DEBUG ((DEBUG_ERROR, "%a: Out of memory:\n", __func__));
     return;
   }
 
@@ -233,7 +233,7 @@ OnFileSystemNotification (
   EFI_HANDLE  *HandleBuffer;
   EFI_STATUS  Status;
 
-  DEBUG ((DEBUG_INFO, "%a: Entry...\n", __FUNCTION__));
+  DEBUG ((DEBUG_INFO, "%a: Entry...\n", __func__));
 
   for ( ; ;) {
     //
@@ -256,7 +256,7 @@ OnFileSystemNotification (
     // Spec says we only get one at a time using ByRegisterNotify
     ASSERT (HandleCount == 1);
 
-    DEBUG ((DEBUG_INFO, "%a: processing a potential log device on handle %p\n", __FUNCTION__, HandleBuffer[0]));
+    DEBUG ((DEBUG_INFO, "%a: processing a potential log device on handle %p\n", __func__, HandleBuffer[0]));
 
     RegisterLogDevice (HandleBuffer[0]);
 
@@ -363,7 +363,7 @@ ProcessFileSystemRegistration (
                   );
 
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: failed to create callback event (%r)\n", __FUNCTION__, Status));
+    DEBUG ((DEBUG_ERROR, "%a: failed to create callback event (%r)\n", __func__, Status));
     goto Cleanup;
   }
 
@@ -374,7 +374,7 @@ ProcessFileSystemRegistration (
                   );
 
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: failed to register for file system notifications (%r)\n", __FUNCTION__, Status));
+    DEBUG ((DEBUG_ERROR, "%a: failed to register for file system notifications (%r)\n", __func__, Status));
     gBS->CloseEvent (FileSystemCallBackEvent);
     goto Cleanup;
   }
@@ -424,13 +424,13 @@ ProcessResetEventRegistration (
     //
     // Register our reset notification request
     //
-    DEBUG ((DEBUG_INFO, "%a: Located Reset notification protocol. Registering handler\n", __FUNCTION__));
+    DEBUG ((DEBUG_INFO, "%a: Located Reset notification protocol. Registering handler\n", __func__));
     Status = ResetNotificationProtocol->RegisterResetNotify (ResetNotificationProtocol, OnResetNotification);
     if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "%a: failed to register Reset Notification handler (%r)\n", __FUNCTION__, Status));
+      DEBUG ((DEBUG_ERROR, "%a: failed to register Reset Notification handler (%r)\n", __func__, Status));
     }
   } else {
-    DEBUG ((DEBUG_INFO, "%a: Reset Notification protocol not installed. Registering for notification\n", __FUNCTION__));
+    DEBUG ((DEBUG_INFO, "%a: Reset Notification protocol not installed. Registering for notification\n", __func__));
     Status = gBS->CreateEvent (
                     EVT_NOTIFY_SIGNAL,
                     TPL_CALLBACK,
@@ -440,7 +440,7 @@ ProcessResetEventRegistration (
                     );
 
     if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "%a: failed to create Reset Protocol protocol callback event (%r)\n", __FUNCTION__, Status));
+      DEBUG ((DEBUG_ERROR, "%a: failed to create Reset Protocol protocol callback event (%r)\n", __func__, Status));
     } else {
       Status = gBS->RegisterProtocolNotify (
                       &gEdkiiPlatformSpecificResetFilterProtocolGuid,
@@ -449,7 +449,7 @@ ProcessResetEventRegistration (
                       );
 
       if (EFI_ERROR (Status)) {
-        DEBUG ((DEBUG_ERROR, "%a: failed to register for Reset Protocol notification (%r)\n", __FUNCTION__, Status));
+        DEBUG ((DEBUG_ERROR, "%a: failed to register for Reset Protocol notification (%r)\n", __func__, Status));
         gBS->CloseEvent (ResetNotificationEvent);
       }
     }
@@ -491,7 +491,7 @@ ProcessSyncRequestRegistration (
                   );
 
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a - Create Event Ex for file logger write. Code = %r\n", __FUNCTION__, Status));
+    DEBUG ((DEBUG_ERROR, "%a - Create Event Ex for file logger write. Code = %r\n", __func__, Status));
   }
 
   return Status;
@@ -534,7 +534,7 @@ ProcessReadyToBootRegistration (
                     );
 
     if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "%a - Create Event Ex for ReadyToBoot. Code = %r\n", __FUNCTION__, Status));
+      DEBUG ((DEBUG_ERROR, "%a - Create Event Ex for ReadyToBoot. Code = %r\n", __func__, Status));
     }
   }
 
@@ -577,7 +577,7 @@ ProcessPreExitBootServicesRegistration (
                     );
 
     if (EFI_ERROR (Status)) {
-      DEBUG ((DEBUG_ERROR, "%a - Create Event Ex for ExitBootServices. Code = %r\n", __FUNCTION__, Status));
+      DEBUG ((DEBUG_ERROR, "%a - Create Event Ex for ExitBootServices. Code = %r\n", __func__, Status));
     }
   }
 
@@ -601,9 +601,24 @@ AdvancedFileLoggerEntry (
   IN EFI_SYSTEM_TABLE  *SystemTable
   )
 {
-  EFI_STATUS  Status;
+  EFI_STATUS                   Status;
+  ADVANCED_FILE_LOGGER_POLICY  AdvFileLoggerPolicy;
+  UINT16                       PolicySize = ADVANCED_FILE_LOGGER_POLICY_SIZE;
+
+  DEBUG ((DEBUG_INFO, "%a: enter...\n", __func__));
+
+  // Step 0. Check advanced file logger policy, default to enabled.
 
-  DEBUG ((DEBUG_INFO, "%a: enter...\n", __FUNCTION__));
+  Status = GetPolicy (&gAdvancedFileLoggerPolicyGuid, NULL, (VOID *)&AdvFileLoggerPolicy, &PolicySize);
+  if (EFI_ERROR (Status)) {
+    // If we fail to get policy, default to enabled.
+    DEBUG ((DEBUG_WARN, "%a: Unable to get file logger - %r defaulting to enabled!\n", __func__, Status));
+  } else if (AdvFileLoggerPolicy.FileLoggerEnable == FALSE) {
+    DEBUG ((DEBUG_INFO, "%a: File logger disabled in policy, exiting.\n", __func__));
+    return EFI_SUCCESS;
+  } else {
+    DEBUG ((DEBUG_INFO, "%a: File logger enabled in policy.\n", __func__));
+  }
 
   //
   // Step 1. Register for file system notifications
@@ -645,9 +660,9 @@ AdvancedFileLoggerEntry (
 Exit:
 
   if (EFI_ERROR (Status)) {
-    DEBUG ((DEBUG_ERROR, "%a: Leaving, code = %r\n", __FUNCTION__, Status));
+    DEBUG ((DEBUG_ERROR, "%a: Leaving, code = %r\n", __func__, Status));
   } else {
-    DEBUG ((DEBUG_INFO, "%a: Leaving, code = %r\n", __FUNCTION__, Status));
+    DEBUG ((DEBUG_INFO, "%a: Leaving, code = %r\n", __func__, Status));
   }
 
   // Always return EFI_SUCCESS.  This means any partial registration of functions

diff --git a/AdvLoggerPkg/AdvancedFileLogger/AdvancedFileLogger.h b/AdvLoggerPkg/AdvancedFileLogger/AdvancedFileLogger.h
@@ -15,6 +15,7 @@
 #include <AdvancedLoggerInternal.h>
 
 #include <Guid/EventGroup.h>
+#include <Guid/AdvancedFileLoggerPolicy.h>
 
 #include <Protocol/AdvancedLogger.h>
 #include <Protocol/DevicePath.h>
@@ -34,6 +35,7 @@
 #include <Library/TimerLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 #include <Library/UefiRuntimeServicesTableLib.h>
+#include <Library/PolicyLib.h>
 
 #define LOG_DEVICE_SIGNATURE  SIGNATURE_32('D','L','o','g')
 

diff --git a/AdvLoggerPkg/AdvancedFileLogger/AdvancedFileLogger.inf b/AdvLoggerPkg/AdvancedFileLogger/AdvancedFileLogger.inf
@@ -30,6 +30,7 @@
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
   AdvLoggerPkg/AdvLoggerPkg.dec
+  PolicyServicePkg/PolicyServicePkg.dec
 
 [LibraryClasses]
   AdvancedLoggerAccessLib
@@ -46,11 +47,13 @@
   TimerLib
   UefiDriverEntryPoint
   UefiRuntimeServicesTableLib
+  PolicyLib
 
 [Guids]
   gAdvancedFileLoggerWriteLogFiles
   gEfiEventReadyToBootGuid
   gMuEventPreExitBootServicesGuid
+  gAdvancedFileLoggerPolicyGuid
 
 [Protocols]
   gEdkiiPlatformSpecificResetFilterProtocolGuid                 ## CONSUMES

diff --git a/AdvLoggerPkg/AdvancedFileLogger/ReadMe.md b/AdvLoggerPkg/AdvancedFileLogger/ReadMe.md
@@ -6,7 +6,7 @@ The Advanced File Logger monitors for file systems mounted during boot.
 When an eligible file system is detected, the log is flushed to the file system.
 The log is flushed if the system is reset during POST, and at Exit Boot Services.
 
-An eligible file system is one with a Logs directory in the root of the file system.
+An eligible file system is one with a `UefiLogs` directory in the root of the file system.
 If no log files are present, the Advanced File Logger will create a log index file which
 contains the index of the last log file written, and nine log files each PcdAdvancedLoggerPages in size.
 These files are pre allocated at one time to reduce interference with other users of the filesystem.
@@ -25,6 +25,63 @@ and the follow change is needed in the .fdf:
   INF AdvLoggerPkg/AdvancedFileLogger/AdvancedFileLogger.inf
 ```
 
+## Driver Usage Configuration
+
+This section describes the configuration options of the Advanced File Logger driver.
+
+### Advanced file logger enforcement
+
+The Advanced File Logger can be configured to enforce the storing of memory logs when setting
+`PcdAdvancedFileLoggerForceEnable` to `TRUE`.
+
+### Advanced file logger enlightenment
+
+The Advanced File Logger can be set to store memory logs with a `UefiLogs` directory in the ESP partition.
+Without such a directory **AND** the enforcement is not enabled, the Advanced File Logger will not store
+memory logs.
+
+### Advanced file logger enablement through Policy
+
+The Advanced File Logger can be configured to be enabled through policy service by producing a policy under[`gAdvancedFileLoggerPolicyGuid`](../Include/Guid/AdvancedFileLoggerPolicy.h)
+and setting the `FileLoggerEnable` field to `TRUE`. For the sake of backwards compatibility, if a platform does not
+produce such policy or not support policy services at all, the Advanced File Logger will default to be enabled.
+
+Note: The above enablement, enforcement and enlightenment are in serial, the general pseudo code is as follows:
+
+```c
+  if (PolicyNotFound) {
+    Enabled = TRUE;
+  } else {
+    Enabled = Policy.FileLoggerEnable;
+  }
+
+  if (Enable) {
+    if (PcdGet (PcdAdvancedFileLoggerForceEnable)) {
+      StoreLogs = TRUE;
+    } else if (DirectoryExists (L"UefiLogs")) {
+      StoreLogs = TRUE;
+    } else {
+      StoreLogs = FALSE;
+    }
+  }
+
+  if (StoreLogs) {
+    // Store logs
+    // The UEFI_Index.txt file will indicate the last log file written
+    StoreLogs ("UefiLogs/Log#.txt");
+  }
+```
+
+### Advanced file logger Flush events
+
+The advanced file logger can be flushed or configured to flush on the following events:
+
+| Event | Description |
+| --- | --- |
+| `gEfiEventReadyToBootGuid` | By setting `BIT0` of `PcdAdvancedFileLoggerFlush`, logger will flushed to ESP at "Ready To Boot" event |
+| `gEfiEventExitBootServicesGuid` | By setting `BIT1` of `PcdAdvancedFileLoggerFlush`, logger will flushed to ESP at "Exit Boot Services" event |
+| System Reset | This will always be enabled as long as the system supports `gEdkiiPlatformSpecificResetFilterProtocolGuid` |
+
 ---
 
 ## Copyright

diff --git a/AdvLoggerPkg/Include/Guid/AdvancedFileLoggerPolicy.h b/AdvLoggerPkg/Include/Guid/AdvancedFileLoggerPolicy.h
@@ -0,0 +1,29 @@
+/** @file
+  This file defines GUIDs and data structure used for Advanced file logger policy.
+
+  Copyright (C) Microsoft Corporation.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef  ADVANCED_FILE_LOGGER_POLICY_H_
+#define  ADVANCED_FILE_LOGGER_POLICY_H_
+
+#define ADVANCED_FILE_LOGGER_POLICY_GUID  \
+  { \
+    0x6c3fd4f1, 0xb596, 0x438e, { 0x8a, 0xb8, 0x53, 0xf1, 0xc, 0x9c, 0x27, 0x74 } \
+  }
+
+#define ADVANCED_FILE_LOGGER_POLICY_SIZE  sizeof(ADVANCED_FILE_LOGGER_POLICY)
+
+#pragma pack(1)
+
+typedef struct {
+  BOOLEAN    FileLoggerEnable;
+} ADVANCED_FILE_LOGGER_POLICY;
+
+#pragma pack()
+
+extern EFI_GUID  gAdvancedFileLoggerPolicyGuid;
+
+#endif //ADVANCED_FILE_LOGGER_POLICY_H_