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 10/12] OvmfPkg/IoMmuDxe: implement in-place decryption/encryption for Map/Unmap
Date: Wed,  2 Aug 2017 23:24:51 +0200	[thread overview]
Message-ID: <20170802212453.19221-11-lersek@redhat.com> (raw)
In-Reply-To: <20170802212453.19221-1-lersek@redhat.com>

At the moment, we have the following distribution of actions between the
IOMMU protocol member functions:

- AllocateBuffer() allocates pages and clears the memory encryption mask.

- FreeBuffer() re-sets the memory encryption mask, and deallocates pages.

- Map() does nothing at all when BusMasterCommonBuffer[64] is requested
  (and AllocateBuffer() was called previously). Otherwise, Map() allocates
  pages, and clears the memory encryption mask.

- Unmap() does nothing when cleaning up a BusMasterCommonBuffer[64]
  operation. Otherwise, Unmap() clears the encryption mask, and frees the
  pages.

This is wrong: the AllocateBuffer() protocol member is not expected to
produce a buffer that is immediately usable, and client code is required
to call Map() unconditionally, even if BusMasterCommonBuffer[64] is the
desired operation. Implement the right distribution of actions as follows:

- AllocateBuffer() allocates pages and does not touch the encryption mask.

- FreeBuffer() deallocates pages and does not touch the encryption mask.

- Map() does not allocate pages when BusMasterCommonBuffer[64] is
  requested, and it allocates pages (bounce buffer) otherwise.  Regardless
  of the BusMaster operation, Map() (and Map() only) clears the memory
  encryption mask.

- Unmap() restores the encryption mask unconditionally. If the operation
  was BusMasterCommonBuffer[64], then Unmap() does not release the pages.
  Otherwise, the pages (bounce buffer) are released.

This approach also ensures that Unmap() can be called from
ExitBootServices() event handlers, for cleaning up
BusMasterCommonBuffer[64] operations. (More specifically, for restoring
the SEV encryption mask on any in-flight buffers, after resetting any
referring devices.) ExitBootServices() event handlers must not change the
UEFI memory map, thus any memory allocation or freeing in Unmap() would
disqualify Unmap() from being called in such a context.

Map()-ing and Unmap()-ing memory for a BusMasterCommonBuffer[64] operation
effectively means in-place decryption and encryption in a SEV context. As
an additional hurdle, section "7.10.8 Encrypt-in-Place" of AMD publication
Nr.24593 implies that we need a separate temporary buffer for decryption
and encryption that will eventually land in-place. Allocating said
temporary buffer in the straightforward way would violate the above
allocation/freeing restrictions on Map()/Unmap(), therefore pre-allocate
this "stash buffer" too in AllocateBuffer(), and free it in FreeBuffer().

To completely rid Unmap() of dynamic memory impact, for
BusMasterCommonBuffer[64] operations, we're going to rework the lifecycle of
the MAP_INFO structures in a later patch.

(The MemEncryptSevSetPageEncMask() call in Unmap() could theoretically
allocate memory internally for page splitting, however this won't happen
in practice: in Unmap() we only restore the memory encryption mask, and
don't genuinely set it. Any page splitting will have occurred in Map()'s
MemEncryptSevClearPageEncMask() call first.)

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 | 242 +++++++++++++++-----
 1 file changed, 185 insertions(+), 57 deletions(-)

diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
index 0a85ee6559e7..5049d19e9cb7 100644
--- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
+++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c
@@ -23,40 +23,64 @@
 typedef struct {
   EDKII_IOMMU_OPERATION                     Operation;
   UINTN                                     NumberOfBytes;
   UINTN                                     NumberOfPages;
   EFI_PHYSICAL_ADDRESS                      CryptedAddress;
   EFI_PHYSICAL_ADDRESS                      PlainTextAddress;
 } MAP_INFO;
 
-#define NO_MAPPING             (VOID *) (UINTN) -1
+#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)
+typedef struct {
+  UINT64 Signature;
+
+  //
+  // Always allocated from EfiBootServicesData type memory, and always
+  // encrypted.
+  //
+  VOID *StashBuffer;
+
+  //
+  // Followed by the actual common buffer, starting at the next page.
+  //
+} COMMON_BUFFER_HEADER;
+#pragma pack ()
 
 /**
   Provides the controller-specific addresses required to access system memory
   from a DMA bus master. On SEV guest, the DMA operations must be performed on
   shared buffer hence we allocate a bounce buffer to map the HostAddress to a
   DeviceAddress. The Encryption attribute is removed from the DeviceAddress
   buffer.
 
   @param  This                  The protocol instance pointer.
   @param  Operation             Indicates if the bus master is going to read or
                                 write to system memory.
   @param  HostAddress           The system memory address to map to the PCI
                                 controller.
   @param  NumberOfBytes         On input the number of bytes to map. On output
                                 the number of bytes that were mapped.
   @param  DeviceAddress         The resulting map address for the bus master
                                 PCI controller to use to access the hosts
                                 HostAddress.
   @param  Mapping               A resulting value to pass to Unmap().
 
   @retval EFI_SUCCESS           The range was mapped for the returned
                                 NumberOfBytes.
   @retval EFI_UNSUPPORTED       The HostAddress cannot be mapped as a common
                                 buffer.
   @retval EFI_INVALID_PARAMETER One or more parameters are invalid.
   @retval EFI_OUT_OF_RESOURCES  The request could not be completed due to a
                                 lack of resources.
   @retval EFI_DEVICE_ERROR      The system hardware could not map the requested
                                 address.
 
 **/
@@ -65,157 +89,175 @@ 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;
   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;
   }
 
   //
   // Initialize the MAP_INFO structure, except the PlainTextAddress field
   //
   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.
+  // 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 plaintext buffer has been
-  // allocated already, with AllocateBuffer(). We only check whether the
-  // address is low enough for the requested operation.
+  // 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(),
-    // and it is already decrypted.
+    // The buffer at MapInfo->CryptedAddress comes from AllocateBuffer().
     //
     MapInfo->PlainTextAddress = MapInfo->CryptedAddress;
-
     //
-    // Therefore no mapping is necessary.
+    // Stash the crypted data.
     //
-    *DeviceAddress = MapInfo->PlainTextAddress;
-    *Mapping       = NO_MAPPING;
-    FreePool (MapInfo);
-    return EFI_SUCCESS;
+    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 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 == EdkiiIoMmuOperationBusMasterRead64 ||
+      Operation == EdkiiIoMmuOperationBusMasterCommonBuffer ||
+      Operation == EdkiiIoMmuOperationBusMasterCommonBuffer64) {
     CopyMem (
       (VOID *) (UINTN) MapInfo->PlainTextAddress,
-      (VOID *) (UINTN) MapInfo->CryptedAddress,
+      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;
@@ -245,91 +287,129 @@ 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;
   }
 
-  //
-  // See if the Map() operation associated with this Unmap() required a mapping
-  // buffer. If a mapping buffer was not required, then this function simply
-  // buffer. If a mapping buffer was not required, then this function simply
-  //
-  if (Mapping == NO_MAPPING) {
-    return EFI_SUCCESS;
-  }
-
   MapInfo = (MAP_INFO *)Mapping;
 
   //
-  // If this is a write operation from the Bus Master's point of view,
-  // then copy the contents of the mapped buffer into the real buffer
-  // so the processor can read the contents of the real buffer.
+  // For BusMasterWrite[64] operations and BusMasterCommonBuffer[64] operations
+  // we have to encrypt the results, ultimately to the original place (i.e.,
+  // "MapInfo->CryptedAddress").
   //
-  if (MapInfo->Operation == EdkiiIoMmuOperationBusMasterWrite ||
-      MapInfo->Operation == EdkiiIoMmuOperationBusMasterWrite64) {
+  // 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 (
-      (VOID *) (UINTN) MapInfo->CryptedAddress,
+      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
+  // 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);
-  ZeroMem (
-    (VOID*)(UINTN)MapInfo->PlainTextAddress,
-    EFI_PAGES_TO_SIZE (MapInfo->NumberOfPages)
-    );
 
   //
-  // Free the mapped buffer and the MAP_INFO structure.
+  // 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
+      );
+  } else {
+    ZeroMem (
+      (VOID *)(UINTN)MapInfo->PlainTextAddress,
+      EFI_PAGES_TO_SIZE (MapInfo->NumberOfPages)
+      );
+    gBS->FreePages (MapInfo->PlainTextAddress, MapInfo->NumberOfPages);
+  }
+
+  //
+  // Free the MAP_INFO structure.
   //
-  gBS->FreePages (MapInfo->PlainTextAddress, MapInfo->NumberOfPages);
   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.
 
 **/
@@ -338,81 +418,116 @@ EFIAPI
 IoMmuAllocateBuffer (
   IN     EDKII_IOMMU_PROTOCOL                     *This,
   IN     EFI_ALLOCATE_TYPE                        Type,
   IN     EFI_MEMORY_TYPE                          MemoryType,
   IN     UINTN                                    Pages,
   IN OUT VOID                                     **HostAddress,
   IN     UINT64                                   Attributes
   )
 {
   EFI_STATUS                Status;
   EFI_PHYSICAL_ADDRESS      PhysicalAddress;
+  VOID                      *StashBuffer;
+  UINTN                     CommonBufferPages;
+  COMMON_BUFFER_HEADER      *CommonBufferHeader;
 
   //
   // Validate Attributes
   //
   if ((Attributes & EDKII_IOMMU_ATTRIBUTE_INVALID_FOR_ALLOCATE_BUFFER) != 0) {
     return EFI_UNSUPPORTED;
   }
 
   //
   // Check for invalid inputs
   //
   if (HostAddress == NULL) {
     return EFI_INVALID_PARAMETER;
   }
 
   //
   // The only valid memory types are EfiBootServicesData and
   // EfiRuntimeServicesData
   //
   if (MemoryType != EfiBootServicesData &&
       MemoryType != EfiRuntimeServicesData) {
     return EFI_INVALID_PARAMETER;
   }
 
+  //
+  // We'll need a header page for the COMMON_BUFFER_HEADER structure.
+  //
+  if (Pages > MAX_UINTN - 1) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+  CommonBufferPages = Pages + 1;
+
+  //
+  // Allocate the stash in EfiBootServicesData type memory.
+  //
+  // Map() will temporarily save encrypted data in the stash for
+  // BusMasterCommonBuffer[64] operations, so the data can be decrypted to the
+  // original location.
+  //
+  // Unmap() will temporarily save plaintext data in the stash for
+  // BusMasterCommonBuffer[64] operations, so the data can be encrypted to the
+  // original location.
+  //
+  // StashBuffer always resides in encrypted memory.
+  //
+  StashBuffer = AllocatePages (Pages);
+  if (StashBuffer == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
   PhysicalAddress = (UINTN)-1;
   if ((Attributes & EDKII_IOMMU_ATTRIBUTE_DUAL_ADDRESS_CYCLE) == 0) {
     //
     // Limit allocations to memory below 4GB
     //
     PhysicalAddress = SIZE_4GB - 1;
   }
   Status = gBS->AllocatePages (
                   AllocateMaxAddress,
                   MemoryType,
-                  Pages,
+                  CommonBufferPages,
                   &PhysicalAddress
                   );
-  if (!EFI_ERROR (Status)) {
-    *HostAddress = (VOID *) (UINTN) PhysicalAddress;
-
-    //
-    // Clear memory encryption mask
-    //
-    Status = MemEncryptSevClearPageEncMask (0, PhysicalAddress, Pages, TRUE);
-    ASSERT_EFI_ERROR(Status);
+  if (EFI_ERROR (Status)) {
+    goto FreeStashBuffer;
   }
 
+  CommonBufferHeader = (VOID *)(UINTN)PhysicalAddress;
+  PhysicalAddress += EFI_PAGE_SIZE;
+
+  CommonBufferHeader->Signature = COMMON_BUFFER_SIG;
+  CommonBufferHeader->StashBuffer = StashBuffer;
+
+  *HostAddress = (VOID *)(UINTN)PhysicalAddress;
+
   DEBUG ((
     DEBUG_VERBOSE,
     "%a Address 0x%Lx Pages 0x%Lx\n",
     __FUNCTION__,
     PhysicalAddress,
     (UINT64)Pages
     ));
+  return EFI_SUCCESS;
+
+FreeStashBuffer:
+  FreePages (StashBuffer, Pages);
   return Status;
 }
 
 /**
   Frees memory that was allocated with AllocateBuffer().
 
   @param  This                  The protocol instance pointer.
   @param  Pages                 The number of pages to free.
   @param  HostAddress           The base system memory address of the allocated
                                 range.
 
   @retval EFI_SUCCESS           The requested memory pages were freed.
   @retval EFI_INVALID_PARAMETER The memory range specified by HostAddress and
                                 Pages was not allocated with AllocateBuffer().
 
 **/
@@ -421,79 +536,92 @@ EFIAPI
 IoMmuFreeBuffer (
   IN  EDKII_IOMMU_PROTOCOL                     *This,
   IN  UINTN                                    Pages,
   IN  VOID                                     *HostAddress
   )
 {
-  EFI_STATUS  Status;
+  UINTN                CommonBufferPages;
+  COMMON_BUFFER_HEADER *CommonBufferHeader;
+
+  CommonBufferPages = Pages + 1;
+  CommonBufferHeader = (COMMON_BUFFER_HEADER *)(
+                         (UINTN)HostAddress - EFI_PAGE_SIZE
+                         );
+
+  //
+  // Check the signature.
+  //
+  ASSERT (CommonBufferHeader->Signature == COMMON_BUFFER_SIG);
+  if (CommonBufferHeader->Signature != COMMON_BUFFER_SIG) {
+    return EFI_INVALID_PARAMETER;
+  }
 
   //
-  // Set memory encryption mask
+  // Free the stash buffer. This buffer was always encrypted, so no need to
+  // zero it.
   //
-  Status = MemEncryptSevSetPageEncMask (
-             0,
-             (EFI_PHYSICAL_ADDRESS)(UINTN)HostAddress,
-             Pages,
-             TRUE
-             );
-  ASSERT_EFI_ERROR(Status);
-  ZeroMem (HostAddress, EFI_PAGES_TO_SIZE (Pages));
+  FreePages (CommonBufferHeader->StashBuffer, Pages);
 
   DEBUG ((
     DEBUG_VERBOSE,
     "%a Address 0x%Lx Pages 0x%Lx\n",
     __FUNCTION__,
     (UINT64)(UINTN)HostAddress,
     (UINT64)Pages
     ));
-  return gBS->FreePages ((EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress, Pages);
+
+  //
+  // Release the common buffer itself. Unmap() has re-encrypted it in-place, so
+  // no need to zero it.
+  //
+  return gBS->FreePages ((UINTN)CommonBufferHeader, CommonBufferPages);
 }
 
 
 /**
   Set IOMMU attribute for a system memory.
 
   If the IOMMU protocol exists, the system memory cannot be used
   for DMA by default.
 
   When a device requests a DMA access for a system memory,
   the device driver need use SetAttribute() to update the IOMMU
   attribute to request DMA access (read and/or write).
 
   The DeviceHandle is used to identify which device submits the request.
   The IOMMU implementation need translate the device path to an IOMMU device
   ID, and set IOMMU hardware register accordingly.
   1) DeviceHandle can be a standard PCI device.
      The memory for BusMasterRead need set EDKII_IOMMU_ACCESS_READ.
      The memory for BusMasterWrite need set EDKII_IOMMU_ACCESS_WRITE.
      The memory for BusMasterCommonBuffer need set
      EDKII_IOMMU_ACCESS_READ|EDKII_IOMMU_ACCESS_WRITE.
      After the memory is used, the memory need set 0 to keep it being
      protected.
   2) DeviceHandle can be an ACPI device (ISA, I2C, SPI, etc).
      The memory for DMA access need set EDKII_IOMMU_ACCESS_READ and/or
      EDKII_IOMMU_ACCESS_WRITE.
 
   @param[in]  This              The protocol instance pointer.
   @param[in]  DeviceHandle      The device who initiates the DMA access
                                 request.
   @param[in]  Mapping           The mapping value returned from Map().
   @param[in]  IoMmuAccess       The IOMMU access.
 
   @retval EFI_SUCCESS            The IoMmuAccess is set for the memory range
                                  specified by DeviceAddress and Length.
   @retval EFI_INVALID_PARAMETER  DeviceHandle is an invalid handle.
   @retval EFI_INVALID_PARAMETER  Mapping is not a value that was returned by
                                  Map().
   @retval EFI_INVALID_PARAMETER  IoMmuAccess specified an illegal combination
                                  of access.
   @retval EFI_UNSUPPORTED        DeviceHandle is unknown by the IOMMU.
   @retval EFI_UNSUPPORTED        The bit mask of IoMmuAccess is not supported
                                  by the IOMMU.
   @retval EFI_UNSUPPORTED        The IOMMU does not support the memory range
                                  specified by Mapping.
   @retval EFI_OUT_OF_RESOURCES   There are not enough resources available to
                                  modify the IOMMU access.
   @retval EFI_DEVICE_ERROR       The IOMMU device reported an error while
                                  attempting the operation.
 
 **/
-- 
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 ` Laszlo Ersek [this message]
2017-08-02 23:01   ` [PATCH 10/12] OvmfPkg/IoMmuDxe: implement in-place decryption/encryption for Map/Unmap 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 ` [PATCH 12/12] OvmfPkg/IoMmuDxe: Unmap(): recycle MAP_INFO after BusMasterCommonBuffer[64] Laszlo Ersek
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-11-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