public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Jordan Justen <jordan.l.justen@intel.com>,
	Tom Lendacky <thomas.lendacky@amd.com>
Subject: [PATCH 12/12] OvmfPkg/IoMmuDxe: Unmap(): recycle MAP_INFO after BusMasterCommonBuffer[64]
Date: Wed,  2 Aug 2017 23:24:53 +0200	[thread overview]
Message-ID: <20170802212453.19221-13-lersek@redhat.com> (raw)
In-Reply-To: <20170802212453.19221-1-lersek@redhat.com>

In order for Unmap() to be callable from ExitBootServices() event handler
context (for cleaning up a BusMasterCommonBuffer[64] operation), we have
to completely liberate the affected path in Unmap() from dynamic memory
management.

The last remaining piece is the release of the MAP_INFO structure. Rather
than freeing it with FreePool(), recycle it to an internal list. Elements
of this "free list" can be reused for any kind of Map() operation, and can
be freed later, or recycled again.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Brijesh Singh <brijesh.singh@amd.com>
Cc: Jordan Justen <jordan.l.justen@intel.com>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 48 ++++++++++++++++----
 1 file changed, 40 insertions(+), 8 deletions(-)

diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
index ee94cd4efbe2..b2c825123fbb 100644
--- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
+++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
@@ -20,22 +20,36 @@
 
 #include "AmdSevIoMmu.h"
 
+#define MAP_INFO_SIG SIGNATURE_64 ('M', 'A', 'P', '_', 'I', 'N', 'F', 'O')
+
 typedef struct {
+  UINT64                                    Signature;
+  LIST_ENTRY                                Link;
   EDKII_IOMMU_OPERATION                     Operation;
   UINTN                                     NumberOfBytes;
   UINTN                                     NumberOfPages;
   EFI_PHYSICAL_ADDRESS                      CryptedAddress;
   EFI_PHYSICAL_ADDRESS                      PlainTextAddress;
 } MAP_INFO;
 
+//
+// List of MAP_INFO structures recycled by Unmap().
+//
+// Recycled MAP_INFO structures are equally good for future recycling and
+// freeing.
+//
+STATIC LIST_ENTRY mRecycledMapInfos = INITIALIZE_LIST_HEAD_VARIABLE (
+                                        mRecycledMapInfos
+                                        );
+
 #define COMMON_BUFFER_SIG SIGNATURE_64 ('C', 'M', 'N', 'B', 'U', 'F', 'F', 'R')
 
 //
 // The following structure enables Map() and Unmap() to perform in-place
 // decryption and encryption, respectively, for BusMasterCommonBuffer[64]
 // operations, without dynamic memory allocation or release.
 //
 // Both COMMON_BUFFER_HEADER and COMMON_BUFFER_HEADER.StashBuffer are allocated
 // by AllocateBuffer() and released by FreeBuffer().
 //
 #pragma pack (1)
@@ -89,178 +103,190 @@ EFIAPI
 IoMmuMap (
   IN     EDKII_IOMMU_PROTOCOL                       *This,
   IN     EDKII_IOMMU_OPERATION                      Operation,
   IN     VOID                                       *HostAddress,
   IN OUT UINTN                                      *NumberOfBytes,
   OUT    EFI_PHYSICAL_ADDRESS                       *DeviceAddress,
   OUT    VOID                                       **Mapping
   )
 {
   EFI_STATUS                                        Status;
+  LIST_ENTRY                                        *RecycledMapInfo;
   MAP_INFO                                          *MapInfo;
   EFI_ALLOCATE_TYPE                                 AllocateType;
   COMMON_BUFFER_HEADER                              *CommonBufferHeader;
   VOID                                              *DecryptionSource;
 
   if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL ||
       Mapping == NULL) {
     return EFI_INVALID_PARAMETER;
   }
 
   //
   // Allocate a MAP_INFO structure to remember the mapping when Unmap() is
   // called later.
   //
-  MapInfo = AllocatePool (sizeof (MAP_INFO));
-  if (MapInfo == NULL) {
-    Status = EFI_OUT_OF_RESOURCES;
-    goto Failed;
+  RecycledMapInfo = GetFirstNode (&mRecycledMapInfos);
+  if (RecycledMapInfo == &mRecycledMapInfos) {
+    //
+    // No recycled MAP_INFO structure, allocate a new one.
+    //
+    MapInfo = AllocatePool (sizeof (MAP_INFO));
+    if (MapInfo == NULL) {
+      Status = EFI_OUT_OF_RESOURCES;
+      goto Failed;
+    }
+  } else {
+    MapInfo = CR (RecycledMapInfo, MAP_INFO, Link, MAP_INFO_SIG);
+    RemoveEntryList (RecycledMapInfo);
   }
 
   //
   // Initialize the MAP_INFO structure, except the PlainTextAddress field
   //
+  ZeroMem (&MapInfo->Link, sizeof MapInfo->Link);
+  MapInfo->Signature         = MAP_INFO_SIG;
   MapInfo->Operation         = Operation;
   MapInfo->NumberOfBytes     = *NumberOfBytes;
   MapInfo->NumberOfPages     = EFI_SIZE_TO_PAGES (MapInfo->NumberOfBytes);
   MapInfo->CryptedAddress    = (UINTN)HostAddress;
 
   //
   // In the switch statement below, we point "MapInfo->PlainTextAddress" to the
   // plaintext buffer, according to Operation. We also set "DecryptionSource".
   //
   MapInfo->PlainTextAddress = MAX_ADDRESS;
   AllocateType = AllocateAnyPages;
   DecryptionSource = (VOID *)(UINTN)MapInfo->CryptedAddress;
   switch (Operation) {
   //
   // For BusMasterRead[64] and BusMasterWrite[64] operations, a bounce buffer
   // is necessary regardless of whether the original (crypted) buffer crosses
   // the 4GB limit or not -- we have to allocate a separate plaintext buffer.
   // The only variable is whether the plaintext buffer should be under 4GB.
   //
   case EdkiiIoMmuOperationBusMasterRead:
   case EdkiiIoMmuOperationBusMasterWrite:
     MapInfo->PlainTextAddress = BASE_4GB - 1;
     AllocateType = AllocateMaxAddress;
     //
     // fall through
     //
   case EdkiiIoMmuOperationBusMasterRead64:
   case EdkiiIoMmuOperationBusMasterWrite64:
     //
     // Allocate the implicit plaintext bounce buffer.
     //
     Status = gBS->AllocatePages (
                     AllocateType,
                     EfiBootServicesData,
                     MapInfo->NumberOfPages,
                     &MapInfo->PlainTextAddress
                     );
     if (EFI_ERROR (Status)) {
       goto FreeMapInfo;
     }
     break;
 
   //
   // For BusMasterCommonBuffer[64] operations, a to-be-plaintext buffer and a
   // stash buffer (for in-place decryption) have been allocated already, with
   // AllocateBuffer(). We only check whether the address of the to-be-plaintext
   // buffer is low enough for the requested operation.
   //
   case EdkiiIoMmuOperationBusMasterCommonBuffer:
     if ((MapInfo->CryptedAddress > BASE_4GB) ||
         (EFI_PAGES_TO_SIZE (MapInfo->NumberOfPages) >
          BASE_4GB - MapInfo->CryptedAddress)) {
       //
       // CommonBuffer operations cannot be remapped. If the common buffer is
       // above 4GB, then it is not possible to generate a mapping, so return an
       // error.
       //
       Status = EFI_UNSUPPORTED;
       goto FreeMapInfo;
     }
     //
     // fall through
     //
   case EdkiiIoMmuOperationBusMasterCommonBuffer64:
     //
     // The buffer at MapInfo->CryptedAddress comes from AllocateBuffer().
     //
     MapInfo->PlainTextAddress = MapInfo->CryptedAddress;
     //
     // Stash the crypted data.
     //
     CommonBufferHeader = (COMMON_BUFFER_HEADER *)(
                            (UINTN)MapInfo->CryptedAddress - EFI_PAGE_SIZE
                            );
     ASSERT (CommonBufferHeader->Signature == COMMON_BUFFER_SIG);
     CopyMem (
       CommonBufferHeader->StashBuffer,
       (VOID *)(UINTN)MapInfo->CryptedAddress,
       MapInfo->NumberOfBytes
       );
     //
     // Point "DecryptionSource" to the stash buffer so that we decrypt
     // it to the original location, after the switch statement.
     //
     DecryptionSource = CommonBufferHeader->StashBuffer;
     break;
 
   default:
     //
     // Operation is invalid
     //
     Status = EFI_INVALID_PARAMETER;
     goto FreeMapInfo;
   }
 
   //
   // Clear the memory encryption mask on the plaintext buffer.
   //
   Status = MemEncryptSevClearPageEncMask (
              0,
              MapInfo->PlainTextAddress,
              MapInfo->NumberOfPages,
              TRUE
              );
   ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
     CpuDeadLoop ();
   }
 
   //
   // If this is a read operation from the Bus Master's point of view,
   // then copy the contents of the real buffer into the mapped buffer
   // so the Bus Master can read the contents of the real buffer.
   //
   // For BusMasterCommonBuffer[64] operations, the CopyMem() below will decrypt
   // the original data (from the stash buffer) back to the original location.
   //
   if (Operation == EdkiiIoMmuOperationBusMasterRead ||
       Operation == EdkiiIoMmuOperationBusMasterRead64 ||
       Operation == EdkiiIoMmuOperationBusMasterCommonBuffer ||
       Operation == EdkiiIoMmuOperationBusMasterCommonBuffer64) {
     CopyMem (
       (VOID *) (UINTN) MapInfo->PlainTextAddress,
       DecryptionSource,
       MapInfo->NumberOfBytes
       );
   }
 
   //
   // Populate output parameters.
   //
   *DeviceAddress = MapInfo->PlainTextAddress;
   *Mapping       = MapInfo;
 
   DEBUG ((
     DEBUG_VERBOSE,
     "%a PlainText 0x%Lx Crypted 0x%Lx Pages 0x%Lx Bytes 0x%Lx\n",
     __FUNCTION__,
     MapInfo->PlainTextAddress,
     MapInfo->CryptedAddress,
     (UINT64)MapInfo->NumberOfPages,
     (UINT64)MapInfo->NumberOfBytes
     ));
 
   return EFI_SUCCESS;
@@ -290,132 +316,138 @@ EFIAPI
 IoMmuUnmap (
   IN  EDKII_IOMMU_PROTOCOL                     *This,
   IN  VOID                                     *Mapping
   )
 {
   MAP_INFO                 *MapInfo;
   EFI_STATUS               Status;
   COMMON_BUFFER_HEADER     *CommonBufferHeader;
   VOID                     *EncryptionTarget;
 
   if (Mapping == NULL) {
     return EFI_INVALID_PARAMETER;
   }
 
   MapInfo = (MAP_INFO *)Mapping;
 
   //
   // For BusMasterWrite[64] operations and BusMasterCommonBuffer[64] operations
   // we have to encrypt the results, ultimately to the original place (i.e.,
   // "MapInfo->CryptedAddress").
   //
   // For BusMasterCommonBuffer[64] operations however, this encryption has to
   // land in-place, so divert the encryption to the stash buffer first.
   //
   EncryptionTarget = (VOID *)(UINTN)MapInfo->CryptedAddress;
 
   switch (MapInfo->Operation) {
   case EdkiiIoMmuOperationBusMasterCommonBuffer:
   case EdkiiIoMmuOperationBusMasterCommonBuffer64:
     ASSERT (MapInfo->PlainTextAddress == MapInfo->CryptedAddress);
 
     CommonBufferHeader = (COMMON_BUFFER_HEADER *)(
                            (UINTN)MapInfo->PlainTextAddress - EFI_PAGE_SIZE
                            );
     ASSERT (CommonBufferHeader->Signature == COMMON_BUFFER_SIG);
     EncryptionTarget = CommonBufferHeader->StashBuffer;
     //
     // fall through
     //
 
   case EdkiiIoMmuOperationBusMasterWrite:
   case EdkiiIoMmuOperationBusMasterWrite64:
     CopyMem (
       EncryptionTarget,
       (VOID *) (UINTN) MapInfo->PlainTextAddress,
       MapInfo->NumberOfBytes
       );
     break;
 
   default:
     //
     // nothing to encrypt after BusMasterRead[64] operations
     //
     break;
   }
 
   DEBUG ((
     DEBUG_VERBOSE,
     "%a PlainText 0x%Lx Crypted 0x%Lx Pages 0x%Lx Bytes 0x%Lx\n",
     __FUNCTION__,
     MapInfo->PlainTextAddress,
     MapInfo->CryptedAddress,
     (UINT64)MapInfo->NumberOfPages,
     (UINT64)MapInfo->NumberOfBytes
     ));
 
   //
   // Restore the memory encryption mask on the area we used to hold the
   // plaintext.
   //
   Status = MemEncryptSevSetPageEncMask (
              0,
              MapInfo->PlainTextAddress,
              MapInfo->NumberOfPages,
              TRUE
              );
   ASSERT_EFI_ERROR (Status);
   if (EFI_ERROR (Status)) {
     CpuDeadLoop ();
   }
 
   //
   // For BusMasterCommonBuffer[64] operations, copy the stashed data to the
   // original (now encrypted) location.
   //
   // For all other operations, fill the late bounce buffer (which existed as
   // plaintext at some point) with zeros, and then release it.
   //
   if (MapInfo->Operation == EdkiiIoMmuOperationBusMasterCommonBuffer ||
       MapInfo->Operation == EdkiiIoMmuOperationBusMasterCommonBuffer64) {
     CopyMem (
       (VOID *)(UINTN)MapInfo->CryptedAddress,
       CommonBufferHeader->StashBuffer,
       MapInfo->NumberOfBytes
       );
+
+    //
+    // Recycle the MAP_INFO structure.
+    //
+    InsertTailList (&mRecycledMapInfos, &MapInfo->Link);
   } else {
     ZeroMem (
       (VOID *)(UINTN)MapInfo->PlainTextAddress,
       EFI_PAGES_TO_SIZE (MapInfo->NumberOfPages)
       );
     gBS->FreePages (MapInfo->PlainTextAddress, MapInfo->NumberOfPages);
+
+    //
+    // Free the MAP_INFO structure.
+    //
+    FreePool (MapInfo);
   }
 
-  //
-  // Free the MAP_INFO structure.
-  //
-  FreePool (Mapping);
   return EFI_SUCCESS;
 }
 
 /**
   Allocates pages that are suitable for an OperationBusMasterCommonBuffer or
   OperationBusMasterCommonBuffer64 mapping.
 
   @param  This                  The protocol instance pointer.
   @param  Type                  This parameter is not used and must be ignored.
   @param  MemoryType            The type of memory to allocate,
                                 EfiBootServicesData or EfiRuntimeServicesData.
   @param  Pages                 The number of pages to allocate.
   @param  HostAddress           A pointer to store the base system memory
                                 address of the allocated range.
   @param  Attributes            The requested bit mask of attributes for the
                                 allocated range.
 
   @retval EFI_SUCCESS           The requested memory pages were allocated.
   @retval EFI_UNSUPPORTED       Attributes is unsupported. The only legal
                                 attribute bits are MEMORY_WRITE_COMBINE and
                                 MEMORY_CACHED.
   @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
   @retval EFI_OUT_OF_RESOURCES  The memory pages could not be allocated.
 
 **/
-- 
2.13.1.3.g8be5a757fa67



  parent reply	other threads:[~2017-08-02 21:23 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-02 21:24 [PATCH 00/12] OvmfPkg/IoMmuDxe: cleanups and fixes Laszlo Ersek
2017-08-02 21:24 ` [PATCH 01/12] OvmfPkg/IoMmuDxe: rewrap source code to 79 characters Laszlo Ersek
2017-08-02 21:24 ` [PATCH 02/12] OvmfPkg/IoMmuDxe: rename DeviceAddress to PlainTextAddress in MAP_INFO Laszlo Ersek
2017-08-02 21:24 ` [PATCH 03/12] OvmfPkg/IoMmuDxe: rename HostAddress to CryptedAddress " Laszlo Ersek
2017-08-02 21:24 ` [PATCH 04/12] OvmfPkg/IoMmuDxe: convert UINTN arguments to UINT64 for the %Lx fmt spec Laszlo Ersek
2017-08-02 21:24 ` [PATCH 05/12] OvmfPkg/IoMmuDxe: don't initialize local variables Laszlo Ersek
2017-08-02 21:24 ` [PATCH 06/12] OvmfPkg/IoMmuDxe: propagate errors from AmdSevInstallIoMmuProtocol() Laszlo Ersek
2017-08-02 21:24 ` [PATCH 07/12] OvmfPkg/IoMmuDxe: clean up used library classes Laszlo Ersek
2017-08-02 21:24 ` [PATCH 08/12] OvmfPkg/IoMmuDxe: zero out pages before releasing them Laszlo Ersek
2017-08-02 21:24 ` [PATCH 09/12] OvmfPkg/IoMmuDxe: rework setup of "MapInfo->PlainTextAddress" in Map() Laszlo Ersek
2017-08-02 21:24 ` [PATCH 10/12] OvmfPkg/IoMmuDxe: implement in-place decryption/encryption for Map/Unmap Laszlo Ersek
2017-08-02 23:01   ` Brijesh Singh
2017-08-03  0:13     ` Laszlo Ersek
2017-08-03  1:09       ` Brijesh Singh
2017-08-03 14:35         ` Brijesh Singh
2017-08-03 14:40           ` Laszlo Ersek
2017-08-02 21:24 ` [PATCH 11/12] OvmfPkg/IoMmuDxe: abort harder on memory encryption mask failures Laszlo Ersek
2017-08-02 21:24 ` Laszlo Ersek [this message]
2017-08-02 21:31 ` [PATCH 00/12] OvmfPkg/IoMmuDxe: cleanups and fixes Laszlo Ersek
2017-08-03 14:10 ` Brijesh Singh
2017-08-03 14:15   ` Laszlo Ersek
2017-08-05  1:25   ` Laszlo Ersek

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=20170802212453.19221-13-lersek@redhat.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