]> git.proxmox.com Git - mirror_edk2.git/commitdiff
ArmPkg/ArmDmaLib: remove dependency on UncachedMemoryAllocationLib
authorArd Biesheuvel <ard.biesheuvel@linaro.org>
Thu, 24 Aug 2017 16:57:05 +0000 (17:57 +0100)
committerArd Biesheuvel <ard.biesheuvel@linaro.org>
Thu, 24 Aug 2017 17:18:14 +0000 (18:18 +0100)
Now that ArmDmaLib no longer uses uncached mappings for short-lived
bounce buffers used for streaming DMA, the only place we allocate
uncached memory is in DmaAllocateBuffer (), which is used for static
mappings shared between the host and the device, e.g., for packet
descriptor rings etc.

There is no performance concern around such long lived mappings, and
so we can really do without the overhead of UncachedMemoryAllocationLib,
which is a sizable chunk of poorly maintained code that never actually
releases any memory, and despite the fact that it implements pool based
routines, it always performs page based allocations anyway.

So let's invoke the DXE services directly to manage memory attributes
on allocations, and keep track of the allocations in a linked list so
we can restore the attributes and free the memory properly after use.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf

index 566f77d03f293109e1e0d99fe5ca72b0301b34a0..e12bda4c2d33f29b96c325b33b8c4b9d42af5d5a 100644 (file)
 **/\r
 \r
 #include <PiDxe.h>\r
+#include <Library/BaseLib.h>\r
 #include <Library/DebugLib.h>\r
 #include <Library/DmaLib.h>\r
 #include <Library/DxeServicesTableLib.h>\r
 #include <Library/MemoryAllocationLib.h>\r
 #include <Library/UefiBootServicesTableLib.h>\r
-#include <Library/UncachedMemoryAllocationLib.h>\r
 #include <Library/IoLib.h>\r
 #include <Library/BaseMemoryLib.h>\r
 \r
@@ -35,16 +35,23 @@ typedef struct {
 } MAP_INFO_INSTANCE;\r
 \r
 \r
+typedef struct {\r
+  LIST_ENTRY          Link;\r
+  VOID                *HostAddress;\r
+  UINTN               NumPages;\r
+  UINT64              Attributes;\r
+} UNCACHED_ALLOCATION;\r
 \r
 STATIC EFI_CPU_ARCH_PROTOCOL      *mCpu;\r
+STATIC LIST_ENTRY                 UncachedAllocationList;\r
 \r
 STATIC\r
 PHYSICAL_ADDRESS\r
 HostToDeviceAddress (\r
-  IN  PHYSICAL_ADDRESS  HostAddress\r
+  IN  VOID      *Address\r
   )\r
 {\r
-  return HostAddress + PcdGet64 (PcdArmDmaDeviceOffset);\r
+  return (PHYSICAL_ADDRESS)(UINTN)Address + PcdGet64 (PcdArmDmaDeviceOffset);\r
 }\r
 \r
 /**\r
@@ -91,14 +98,7 @@ DmaMap (
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
-  //\r
-  // The debug implementation of UncachedMemoryAllocationLib in ArmPkg returns\r
-  // a virtual uncached alias, and unmaps the cached ID mapping of the buffer,\r
-  // in order to catch inadvertent references to the cached mapping.\r
-  // Since HostToDeviceAddress () expects ID mapped input addresses, convert\r
-  // the host address to an ID mapped address first.\r
-  //\r
-  *DeviceAddress = HostToDeviceAddress (ConvertToPhysicalAddress (HostAddress));\r
+  *DeviceAddress = HostToDeviceAddress (HostAddress);\r
 \r
   // Remember range so we can flush on the other side\r
   Map = AllocatePool (sizeof (MAP_INFO_INSTANCE));\r
@@ -148,7 +148,7 @@ DmaMap (
       }\r
 \r
       Buffer = ALIGN_POINTER (Map->BufferAddress, mCpu->DmaBufferAlignment);\r
-      *DeviceAddress = HostToDeviceAddress (ConvertToPhysicalAddress (Buffer));\r
+      *DeviceAddress = HostToDeviceAddress (Buffer);\r
 \r
       //\r
       // Get rid of any dirty cachelines covering the double buffer. This\r
@@ -270,7 +270,7 @@ DmaUnmap (
   @param  HostAddress           A pointer to store the base system memory address of the\r
                                 allocated range.\r
 \r
-                                @retval EFI_SUCCESS           The requested memory pages were allocated.\r
+  @retval EFI_SUCCESS           The requested memory pages were allocated.\r
   @retval EFI_UNSUPPORTED       Attributes is unsupported. The only legal attribute bits are\r
                                 MEMORY_WRITE_COMBINE and MEMORY_CACHED.\r
   @retval EFI_INVALID_PARAMETER One or more parameters are invalid.\r
@@ -285,21 +285,20 @@ DmaAllocateBuffer (
   OUT VOID                         **HostAddress\r
   )\r
 {\r
-  VOID    *Allocation;\r
+  EFI_GCD_MEMORY_SPACE_DESCRIPTOR   GcdDescriptor;\r
+  VOID                              *Allocation;\r
+  UINT64                            MemType;\r
+  UNCACHED_ALLOCATION               *Alloc;\r
+  EFI_STATUS                        Status;\r
 \r
   if (HostAddress == NULL) {\r
     return EFI_INVALID_PARAMETER;\r
   }\r
 \r
-  //\r
-  // The only valid memory types are EfiBootServicesData and EfiRuntimeServicesData\r
-  //\r
-  // We used uncached memory to keep coherency\r
-  //\r
   if (MemoryType == EfiBootServicesData) {\r
-    Allocation = UncachedAllocatePages (Pages);\r
+    Allocation = AllocatePages (Pages);\r
   } else if (MemoryType == EfiRuntimeServicesData) {\r
-    Allocation = UncachedAllocateRuntimePages (Pages);\r
+    Allocation = AllocateRuntimePages (Pages);\r
   } else {\r
     return EFI_INVALID_PARAMETER;\r
   }\r
@@ -308,9 +307,60 @@ DmaAllocateBuffer (
     return EFI_OUT_OF_RESOURCES;\r
   }\r
 \r
+  // Get the cacheability of the region\r
+  Status = gDS->GetMemorySpaceDescriptor ((UINTN)Allocation, &GcdDescriptor);\r
+  if (EFI_ERROR(Status)) {\r
+    goto FreeBuffer;\r
+  }\r
+\r
+  // Choose a suitable uncached memory type that is supported by the region\r
+  if (GcdDescriptor.Capabilities & EFI_MEMORY_WC) {\r
+    MemType = EFI_MEMORY_WC;\r
+  } else if (GcdDescriptor.Capabilities & EFI_MEMORY_UC) {\r
+    MemType = EFI_MEMORY_UC;\r
+  } else {\r
+    Status = EFI_UNSUPPORTED;\r
+    goto FreeBuffer;\r
+  }\r
+\r
+  Alloc = AllocatePool (sizeof *Alloc);\r
+  if (Alloc == NULL) {\r
+    goto FreeBuffer;\r
+  }\r
+\r
+  Alloc->HostAddress = Allocation;\r
+  Alloc->NumPages = Pages;\r
+  Alloc->Attributes = GcdDescriptor.Attributes;\r
+\r
+  InsertHeadList (&UncachedAllocationList, &Alloc->Link);\r
+\r
+  // Remap the region with the new attributes\r
+  Status = gDS->SetMemorySpaceAttributes ((PHYSICAL_ADDRESS)(UINTN)Allocation,\r
+                                          EFI_PAGES_TO_SIZE (Pages),\r
+                                          MemType);\r
+  if (EFI_ERROR (Status)) {\r
+    goto FreeAlloc;\r
+  }\r
+\r
+  Status = mCpu->FlushDataCache (mCpu,\r
+                                 (PHYSICAL_ADDRESS)(UINTN)Allocation,\r
+                                 EFI_PAGES_TO_SIZE (Pages),\r
+                                 EfiCpuFlushTypeInvalidate);\r
+  if (EFI_ERROR (Status)) {\r
+    goto FreeAlloc;\r
+  }\r
+\r
   *HostAddress = Allocation;\r
 \r
   return EFI_SUCCESS;\r
+\r
+FreeAlloc:\r
+  RemoveEntryList (&Alloc->Link);\r
+  FreePool (Alloc);\r
+\r
+FreeBuffer:\r
+  FreePages (Allocation, Pages);\r
+  return Status;\r
 }\r
 \r
 \r
@@ -332,12 +382,49 @@ DmaFreeBuffer (
   IN  VOID                         *HostAddress\r
   )\r
 {\r
+  LIST_ENTRY                       *Link;\r
+  UNCACHED_ALLOCATION              *Alloc;\r
+  BOOLEAN                          Found;\r
+  EFI_STATUS                       Status;\r
+\r
   if (HostAddress == NULL) {\r
      return EFI_INVALID_PARAMETER;\r
   }\r
 \r
-  UncachedFreePages (HostAddress, Pages);\r
-  return EFI_SUCCESS;\r
+  for (Link = GetFirstNode (&UncachedAllocationList), Found = FALSE;\r
+       !IsNull (&UncachedAllocationList, Link);\r
+       Link = GetNextNode (&UncachedAllocationList, Link)) {\r
+\r
+    Alloc = BASE_CR (Link, UNCACHED_ALLOCATION, Link);\r
+    if (Alloc->HostAddress == HostAddress && Alloc->NumPages == Pages) {\r
+      Found = TRUE;\r
+      break;\r
+    }\r
+  }\r
+\r
+  if (!Found) {\r
+    ASSERT (FALSE);\r
+    return EFI_INVALID_PARAMETER;\r
+  }\r
+\r
+  RemoveEntryList (&Alloc->Link);\r
+\r
+  Status = gDS->SetMemorySpaceAttributes ((PHYSICAL_ADDRESS)(UINTN)HostAddress,\r
+                                          EFI_PAGES_TO_SIZE (Pages),\r
+                                          Alloc->Attributes);\r
+  if (EFI_ERROR (Status)) {\r
+    goto FreeAlloc;\r
+  }\r
+\r
+  //\r
+  // If we fail to restore the original attributes, it is better to leak the\r
+  // memory than to return it to the heap\r
+  //\r
+  FreePages (HostAddress, Pages);\r
+\r
+FreeAlloc:\r
+  FreePool (Alloc);\r
+  return Status;\r
 }\r
 \r
 \r
@@ -348,12 +435,8 @@ ArmDmaLibConstructor (
   IN EFI_SYSTEM_TABLE *SystemTable\r
   )\r
 {\r
-  EFI_STATUS              Status;\r
+  InitializeListHead (&UncachedAllocationList);\r
 \r
   // Get the Cpu protocol for later use\r
-  Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&mCpu);\r
-  ASSERT_EFI_ERROR(Status);\r
-\r
-  return Status;\r
+  return gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&mCpu);\r
 }\r
-\r
index 9b7dad114b184586346050363d0dac36f35cb0ad..e33ed92c9d208c7857d83cbd78364fd6276d5721 100644 (file)
@@ -34,7 +34,6 @@
   DxeServicesTableLib\r
   UefiBootServicesTableLib\r
   MemoryAllocationLib\r
-  UncachedMemoryAllocationLib\r
   IoLib\r
   BaseMemoryLib\r
 \r