public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael Kubacki" <mikuback@linux.microsoft.com>
To: devel@edk2.groups.io
Cc: Andrew Fish <afish@apple.com>, Kang Gao <kang.gao@intel.com>,
	Michael D Kinney <michael.d.kinney@intel.com>,
	Michael Kubacki <michael.kubacki@microsoft.com>,
	Leif Lindholm <leif@nuviainc.com>,
	Benjamin You <benjamin.you@intel.com>,
	Liu Yun <yun.y.liu@intel.com>,
	Ankit Sinha <ankit.sinha@intel.com>,
	Nate DeSimone <nathaniel.l.desimone@intel.com>
Subject: [PATCH v1 11/41] PrmPkg: Add ALLOCATE_CONTEXT_BUFFER_IN_FW build option
Date: Tue, 22 Mar 2022 12:19:17 -0400	[thread overview]
Message-ID: <20220322161947.9319-12-mikuback@linux.microsoft.com> (raw)
In-Reply-To: <20220322161947.9319-1-mikuback@linux.microsoft.com>

From: Michael Kubacki <michael.kubacki@microsoft.com>

There's currently two approaches being considered for how to allocate the
context buffer passed to PRM handlers:

1. The context buffer is allocated and populated in firmware. As such, the
   FW converts all pointers internal to the buffer to virtual memory
   addresses at the virtual address change event. A single context buffer
   pointer is given to the OS via the PRM ACPI table and the OS converts
   this single physical address to a virtual address when it passes the
   context buffer as a pointer to PRM handlers.

2. The context buffer is allocated and populated in the OS. The OS gets
   all the information needed to populate the context buffer from other
   pre-existing resources (mainly physical addresses in the PRM ACPI
   table). The OS converts all the physical addresses to virtual addresses,
   allocates the context buffer instances, and fills in the information.
   The OS passes the context buffer virtual address to PRM handlers.

The prior behavior was (1). The current POR behavior has moved to (2).
Until (2) is used more widely, it can be kept around with fairly minimal
overhead via a build flag in a few places.

So the default behavior is now (2) (the expected permanent behavior) with
(1) easily enabled by defining "ALLOCATE_CONTEXT_BUFFER_IN_FW" in the
compiler defined macros. A DSC define was added in PrmPkg.dsc to set this
compiler macro in the package  build.

At some point in the future, all code (and some peripheral code)
surrounded with this build flag can be removed if (2) is fully
decided upon.

Cc: Andrew Fish <afish@apple.com>
Cc: Kang Gao <kang.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Michael Kubacki <michael.kubacki@microsoft.com>
Cc: Leif Lindholm <leif@nuviainc.com>
Cc: Benjamin You <benjamin.you@intel.com>
Cc: Liu Yun <yun.y.liu@intel.com>
Cc: Ankit Sinha <ankit.sinha@intel.com>
Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>
---
 PrmPkg/PrmConfigDxe/PrmConfigDxe.c                                                                                    | 51 +++++++++++++-------
 PrmPkg/PrmLoaderDxe/PrmLoaderDxe.c                                                                                    |  6 ++-
 PrmPkg/Samples/PrmSampleContextBufferModule/Library/DxeContextBufferModuleConfigLib/DxeContextBufferModuleConfigLib.c | 16 ++++++
 PrmPkg/PrmLoaderDxe/PrmAcpiTable.h                                                                                    |  4 ++
 PrmPkg/PrmPkg.dsc                                                                                                     |  5 ++
 PrmPkg/Readme.md                                                                                                      | 12 +++++
 6 files changed, 76 insertions(+), 18 deletions(-)

diff --git a/PrmPkg/PrmConfigDxe/PrmConfigDxe.c b/PrmPkg/PrmConfigDxe/PrmConfigDxe.c
index cb38146bc9e0..7215c2e1dc6f 100644
--- a/PrmPkg/PrmConfigDxe/PrmConfigDxe.c
+++ b/PrmPkg/PrmConfigDxe/PrmConfigDxe.c
@@ -28,8 +28,8 @@
 STATIC  UINTN                     mMaxRuntimeMmioRangeCount;
 STATIC  UINTN                     mMaxStaticDataBufferCount;
 
-STATIC  PRM_RUNTIME_MMIO_RANGES   **mRuntimeMmioRanges;
-STATIC  PRM_DATA_BUFFER           ***mStaticDataBuffers;
+GLOBAL_REMOVE_IF_UNREFERENCED STATIC  PRM_RUNTIME_MMIO_RANGES   **mRuntimeMmioRanges;
+GLOBAL_REMOVE_IF_UNREFERENCED STATIC  PRM_DATA_BUFFER           ***mStaticDataBuffers;
 
 /**
   Converts the runtime memory range physical addresses to virtual addresses.
@@ -178,32 +178,23 @@ StoreVirtualMemoryAddressChangePointers (
   )
 {
   EFI_STATUS                  Status;
-  UINTN                       BufferIndex;
   UINTN                       HandleCount;
   UINTN                       HandleIndex;
   UINTN                       RangeIndex;
+#ifdef ALLOCATE_CONTEXT_BUFFER_IN_FW
+  UINTN                       BufferIndex;
   UINTN                       StaticDataBufferIndex;
+  PRM_CONTEXT_BUFFER          *CurrentContextBuffer;
+#endif
   EFI_HANDLE                  *HandleBuffer;
   PRM_CONFIG_PROTOCOL         *PrmConfigProtocol;
-  PRM_CONTEXT_BUFFER          *CurrentContextBuffer;
 
   DEBUG ((DEBUG_INFO, "%a %a - Entry.\n", _DBGMSGID_, __FUNCTION__));
 
   RangeIndex = 0;
+#ifdef ALLOCATE_CONTEXT_BUFFER_IN_FW
   StaticDataBufferIndex = 0;
 
-  mRuntimeMmioRanges = AllocateRuntimeZeroPool (sizeof (*mRuntimeMmioRanges) * mMaxRuntimeMmioRangeCount);
-  if (mRuntimeMmioRanges == NULL && mMaxRuntimeMmioRangeCount > 0) {
-    DEBUG ((
-      DEBUG_ERROR,
-      "  %a %a: Memory allocation for runtime MMIO pointer array failed.\n",
-      _DBGMSGID_,
-      __FUNCTION__
-      ));
-    ASSERT (FALSE);
-    return;
-  }
-
   mStaticDataBuffers = AllocateRuntimeZeroPool (sizeof (*mStaticDataBuffers) * mMaxStaticDataBufferCount);
   if (mStaticDataBuffers == NULL && mMaxStaticDataBufferCount > 0) {
     DEBUG ((
@@ -215,6 +206,19 @@ StoreVirtualMemoryAddressChangePointers (
     ASSERT (FALSE);
     return;
   }
+#endif
+
+  mRuntimeMmioRanges = AllocateRuntimeZeroPool (sizeof (*mRuntimeMmioRanges) * mMaxRuntimeMmioRangeCount);
+  if (mRuntimeMmioRanges == NULL && mMaxRuntimeMmioRangeCount > 0) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "  %a %a: Memory allocation for runtime MMIO pointer array failed.\n",
+      _DBGMSGID_,
+      __FUNCTION__
+      ));
+    ASSERT (FALSE);
+    return;
+  }
 
   HandleBuffer = NULL;
   Status = gBS->LocateHandleBuffer (
@@ -236,6 +240,7 @@ StoreVirtualMemoryAddressChangePointers (
         continue;
       }
 
+#ifdef ALLOCATE_CONTEXT_BUFFER_IN_FW
       for (BufferIndex = 0; BufferIndex < PrmConfigProtocol->ModuleContextBuffers.BufferCount; BufferIndex++) {
         CurrentContextBuffer = &(PrmConfigProtocol->ModuleContextBuffers.Buffer[BufferIndex]);
 
@@ -256,6 +261,7 @@ StoreVirtualMemoryAddressChangePointers (
           mStaticDataBuffers[StaticDataBufferIndex++] = &CurrentContextBuffer->StaticDataBuffer;
         }
       }
+#endif
       if (PrmConfigProtocol->ModuleContextBuffers.RuntimeMmioRanges != NULL) {
         if (RangeIndex >= mMaxRuntimeMmioRangeCount) {
           Status = EFI_BUFFER_TOO_SMALL;
@@ -280,6 +286,7 @@ StoreVirtualMemoryAddressChangePointers (
       __FUNCTION__,
       RangeIndex
       ));
+#ifdef ALLOCATE_CONTEXT_BUFFER_IN_FW
     DEBUG ((
       DEBUG_INFO,
       "  %a %a: %d static buffers saved for future virtual memory conversion.\n",
@@ -287,6 +294,7 @@ StoreVirtualMemoryAddressChangePointers (
       __FUNCTION__,
       StaticDataBufferIndex
       ));
+#endif
   }
 }
 
@@ -388,12 +396,14 @@ PrmConfigVirtualAddressChangeEvent (
 {
   UINTN   Index;
 
+#ifdef ALLOCATE_CONTEXT_BUFFER_IN_FW
   //
   // Convert static data buffer pointers
   //
   for (Index = 0; Index < mMaxStaticDataBufferCount; Index++) {
     gRT->ConvertPointer (0x0, (VOID **) mStaticDataBuffers[Index]);
   }
+#endif
 
   //
   // Convert runtime MMIO ranges
@@ -484,7 +494,7 @@ PrmConfigEndOfDxeNotification (
       if (PrmConfigProtocol->ModuleContextBuffers.RuntimeMmioRanges != NULL) {
         DEBUG ((
           DEBUG_INFO,
-          "    %a %a: Found %d PRM runtime MMIO ranges to convert.\n",
+          "    %a %a: Found %d PRM runtime MMIO ranges.\n",
            _DBGMSGID_,
            __FUNCTION__,
            PrmConfigProtocol->ModuleContextBuffers.RuntimeMmioRanges->Count
@@ -541,6 +551,13 @@ PrmConfigEntryPoint (
                   );
   ASSERT_EFI_ERROR (Status);
 
+  DEBUG ((DEBUG_INFO, "  %a %a: Context buffers will be allocated in ", _DBGMSGID_, __FUNCTION__));
+#ifdef ALLOCATE_CONTEXT_BUFFER_IN_FW
+  DEBUG ((DEBUG_INFO, "firmware.\n"));
+#else
+  DEBUG ((DEBUG_INFO, "the operating system.\n"));
+#endif
+
   //
   // Register a notification function for virtual address change
   //
diff --git a/PrmPkg/PrmLoaderDxe/PrmLoaderDxe.c b/PrmPkg/PrmLoaderDxe/PrmLoaderDxe.c
index b43e6d6bf078..85fffdcbd9f1 100644
--- a/PrmPkg/PrmLoaderDxe/PrmLoaderDxe.c
+++ b/PrmPkg/PrmLoaderDxe/PrmLoaderDxe.c
@@ -755,7 +755,11 @@ ProcessPrmModules (
                   &CurrentContextBuffer
                   );
       if (!EFI_ERROR (Status)) {
-        CurrentHandlerInfoStruct->PrmContextBuffer = (UINT64) CurrentContextBuffer;
+#ifdef ALLOCATE_CONTEXT_BUFFER_IN_FW
+        CurrentHandlerInfoStruct->PrmContextBuffer = (UINT64) (UINTN) CurrentContextBuffer;
+#else
+        CurrentHandlerInfoStruct->StaticDataBuffer = (UINT64) (UINTN) CurrentContextBuffer->StaticDataBuffer;
+#endif
       }
 
       Status =  GetExportEntryAddress (
diff --git a/PrmPkg/Samples/PrmSampleContextBufferModule/Library/DxeContextBufferModuleConfigLib/DxeContextBufferModuleConfigLib.c b/PrmPkg/Samples/PrmSampleContextBufferModule/Library/DxeContextBufferModuleConfigLib/DxeContextBufferModuleConfigLib.c
index 3bf5beba7d4a..b0b12c012a41 100644
--- a/PrmPkg/Samples/PrmSampleContextBufferModule/Library/DxeContextBufferModuleConfigLib/DxeContextBufferModuleConfigLib.c
+++ b/PrmPkg/Samples/PrmSampleContextBufferModule/Library/DxeContextBufferModuleConfigLib/DxeContextBufferModuleConfigLib.c
@@ -151,10 +151,26 @@ ContextBufferModuleConfigLibConstructor (
   //
   // Allocate and populate the context buffer
   //
+#ifdef ALLOCATE_CONTEXT_BUFFER_IN_FW
+  //
+  // The context buffer allocated in FW will continue being used at OS runtime so it must
+  // be a runtime services data buffer.
+  //
   // This sample module uses a single context buffer for all the handlers
   // Todo: This can be done more elegantly in the future. Likely though a library service.
   //
   PrmContextBuffer = AllocateRuntimeZeroPool (sizeof (*PrmContextBuffer));
+#else
+  //
+  // This context buffer is not actually used by PRM handler at OS runtime. The OS will allocate
+  // the actual context buffer passed to the PRM handler.
+  //
+  // This context buffer is used internally in the firmware to associate a PRM handler with a
+  // a static data buffer and a runtime MMIO ranges array so those can be placed into the
+  // PRM_HANDLER_INFORMATION_STRUCT and PRM_MODULE_INFORMATION_STRUCT respectively for the PRM handler.
+  //
+  PrmContextBuffer = AllocateZeroPool (sizeof (*PrmContextBuffer));
+#endif
   ASSERT (PrmContextBuffer != NULL);
   if (PrmContextBuffer == NULL) {
     Status = EFI_OUT_OF_RESOURCES;
diff --git a/PrmPkg/PrmLoaderDxe/PrmAcpiTable.h b/PrmPkg/PrmLoaderDxe/PrmAcpiTable.h
index 6b9099ca7ba7..8a9c82347d93 100644
--- a/PrmPkg/PrmLoaderDxe/PrmAcpiTable.h
+++ b/PrmPkg/PrmLoaderDxe/PrmAcpiTable.h
@@ -30,8 +30,10 @@ typedef struct {
   UINT16                              StructureLength;            ///< Length in bytes of this structure
   GUID                                Identifier;                 ///< GUID of the PRM handler for this structure
   UINT64                              PhysicalAddress;            ///< Physical address of this PRM handler
+#ifdef ALLOCATE_CONTEXT_BUFFER_IN_FW
   UINT64                              PrmContextBuffer;           ///< Physical address of the context buffer for this
                                                                   ///< PRM handler (PRM_CONTEXT_BUFFER *)
+#else
   UINT64                              StaticDataBuffer;           ///< Physical address of the static data buffer for
                                                                   ///< this PRM handler (PRM_DATA_BUFFER *)
   UINT64                              AcpiParameterBuffer;        ///< Physical address of the parameter buffer
@@ -39,6 +41,8 @@ typedef struct {
                                                                   ///< that is only used in the case of _DSM invocation.
                                                                   ///< If _DSM invocation is not used, this value is
                                                                   ///< ignored.
+#endif
+
 } PRM_HANDLER_INFORMATION_STRUCT;
 
 typedef struct {
diff --git a/PrmPkg/PrmPkg.dsc b/PrmPkg/PrmPkg.dsc
index a0237d1fe059..1a0ddc6ac181 100644
--- a/PrmPkg/PrmPkg.dsc
+++ b/PrmPkg/PrmPkg.dsc
@@ -105,3 +105,8 @@ [Components]
 [BuildOptions]
 # Force deprecated interfaces off
   *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
+
+# Append package-specific build settings
+!ifdef $(ALLOCATE_CONTEXT_BUFFER_IN_FW)
+  *_*_*_CC_FLAGS = -D ALLOCATE_CONTEXT_BUFFER_IN_FW
+!endif
diff --git a/PrmPkg/Readme.md b/PrmPkg/Readme.md
index b67b3a391e37..00ef41bc5f42 100644
--- a/PrmPkg/Readme.md
+++ b/PrmPkg/Readme.md
@@ -68,6 +68,18 @@ record (POR) configuration.
 The following list are the currently defined build flags (if any) that may be passed to the `build` command
 (e.g. -D FLAG=VALUE).
 
+* `ALLOCATE_CONTEXT_BUFFER_IN_FW` - Allocates the context buffer for each PRM handler in the firmware instead of
+   the operating system (OS).
+
+   Additional detail: The context buffer structure is defined in [PrmContextBuffer.h](PrmPkg/Include/PrmContextBuffer.h).
+   This structure can be instantiated by either firmware with a physical pointer to the buffer placed in the
+   `PRM_HANDLER_INFORMATION_STRUCT` for each handler wherein the OS would convert that physical pointer and pass it
+   as a virtual address pointer to each PRM handler. Alternatively, the context buffer can be allocated and populated
+   by the OS where it would get all the information to populate the context buffer from other structures.
+
+   The default is for the OS to allocate and populate the buffer. The alternative option of the firmware doing this
+   work is kept in the source code until broader OS testing is completed.
+
 ## Overview
 At a high-level, PRM can be viewed from three levels of granularity:
 
-- 
2.28.0.windows.1


  parent reply	other threads:[~2022-03-22 16:21 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-22 16:19 [PATCH v1 00/41] Add PrmPkg Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 01/41] PrmPkg: Add package and include headers Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 02/41] PrmPkg: Add PrmConfig protocol interface Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 03/41] PrmPkg/PrmContextBufferLib: Add initial library instance Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 04/41] PrmPkg/PrmConfigDxe: Add initial driver Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 05/41] PrmPkg: Add initial PrmSamplePrintModule Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 06/41] PrmPkg: Add initial PrmSampleMemoryAllocationModule Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 07/41] PrmPkg: Add initial PrmSampleHardwareAccessModule Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 08/41] PrmPkg: Add initial PrmSampleContextBufferModule Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 09/41] PrmPkg: Add initial package DSC file Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 10/41] Readme.md: Add initial content Michael Kubacki
2022-03-22 16:19 ` Michael Kubacki [this message]
2022-03-22 16:19 ` [PATCH v1 12/41] PrmPkg: Enable variable growth for the PRM_MODULE_EXPORT macro Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 13/41] PrmPkg: Publish PRM operation region to support PRM ACPI _DSM invocation Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 14/41] PrmPkg: Export major/minor version in PRM module PE COFF header Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 15/41] PrmPkg: Add initial PrmSsdtInstallDxe module Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 16/41] PrmPkg: Remove PRM Module Update Lock Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 17/41] PrmPkg: Remove ALLOCATE_CONTEXT_BUFFER_IN_FW build flag Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 18/41] PrmPkg/PrmContextBuffer.h: Add ACPI parameter support structures Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 19/41] PrmPkg/PrmLoaderDxe: Add ACPI parameter buffer support Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 20/41] PrmPkg/PrmSampleContextBufferModule: Remove OS debug print requirement Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 21/41] PrmPkg/PrmSampleHardwareAccessModule: Add non-print PRM handlers Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 22/41] PrmPkg/SampleAcpiParameterBufferModule: Add initial module Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 23/41] PrmPkg/HardwareAccessModuleConfigLib: Add initial library Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 24/41] PrmPkg/Samples/Readme.md: Add initial file Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 25/41] PrmPkg: Refactor some PrmLoaderDxe functionality into libraries Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 26/41] PrmPkg/Application/PrmInfo: Add initial application Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 27/41] PrmPkg: Enforce stricter types Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 28/41] PrmPkg/Test/PrmPkgHostTest.dsc: Add initial file Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 29/41] PrmPkg/Test/UnitTest/Library: Add initial UEFI Boot Services test lib Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 30/41] PrmPkg/Library/DxePrmContextBufferLib: Add host-based unit tests Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 31/41] PrmPkg/DxePrmModuleDiscoveryLib: Add initial " Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 32/41] PrmPkg: Add PlatformGuid Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 33/41] PrmPkg: Update PRM OpRegion Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 34/41] Readme.md: Add iASL note and QEMU sample link Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 35/41] PrmPkg: Replace PcdPrmPlatformGuid with EDKII_DSC_PLATFORM_GUID Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 36/41] PrmPkg/Samples: Remove PrmSampleMemoryAllocationModule Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 37/41] PrmPkg/Samples: Remove PrmSamplePrintModule Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 38/41] PrmPkg: Remove the concept of OS services Michael Kubacki
2022-03-22 16:19 ` [PATCH v1 39/41] Readme.md: Add a link to PRM Specification Michael Kubacki
2022-03-28  8:48 ` 回复: [edk2-devel] [PATCH v1 00/41] Add PrmPkg gaoliming
2022-03-28 14:05   ` Michael Kubacki
2022-03-29 16:28 ` Sinha, Ankit
2022-03-31  0:52   ` 回复: " gaoliming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220322161947.9319-12-mikuback@linux.microsoft.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox