From a24f7d6680dd71616b0fcb9c5a65263fce1722be Mon Sep 17 00:00:00 2001
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date: Tue, 19 Apr 2016 16:26:00 +0200
Subject: [PATCH] ArmPkg/ArmDmaLib: reject consistent DMA mappings of cached
 memory

DmaMap () operations of type MapOperationBusMasterCommonBuffer should
return a mapping that is coherent between the CPU and the device. For
this reason, the API only allows DmaMap () to be called with this operation
type if the memory to be mapped was allocated by DmaAllocateBuffer (),
which in this implementation guarantees the coherency by using uncached
mappings on the CPU side.

This means that, if we encounter a cached mapping in DmaMap () with this
operation type, the code is either broken, or someone is violating the
API, but simply proceeding with a double buffer makes no sense at all,
and can only cause problems.

So instead, actively reject this operation type for cached memory mappings.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 26 +++++++++++++++++++++++---
 1 file changed, 23 insertions(+), 3 deletions(-)

diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
index 2144699c08..0e6fdce8ab 100644
--- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
+++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
@@ -103,6 +103,18 @@ DmaMap (
 
     // If the mapped buffer is not an uncached buffer
     if ((GcdDescriptor.Attributes & (EFI_MEMORY_WB | EFI_MEMORY_WT)) != 0) {
+      //
+      // Operations of type MapOperationBusMasterCommonBuffer are only allowed
+      // on uncached buffers.
+      //
+      if (Operation == MapOperationBusMasterCommonBuffer) {
+        DEBUG ((EFI_D_ERROR,
+          "%a: Operation type 'MapOperationBusMasterCommonBuffer' is only supported\n"
+          "on memory regions that were allocated using DmaAllocateBuffer ()\n",
+          __FUNCTION__));
+        return EFI_UNSUPPORTED;
+      }
+
       //
       // If the buffer does not fill entire cache lines we must double buffer into
       // uncached memory. Device (PCI) address becomes uncached page.
@@ -113,7 +125,7 @@ DmaMap (
         return Status;
       }
 
-      if ((Operation == MapOperationBusMasterRead) || (Operation == MapOperationBusMasterCommonBuffer)) {
+      if (Operation == MapOperationBusMasterRead) {
         CopyMem (Buffer, HostAddress, *NumberOfBytes);
       }
 
@@ -151,6 +163,8 @@ DmaMap (
 
   @retval EFI_SUCCESS           The range was unmapped.
   @retval EFI_DEVICE_ERROR      The data was not committed to the target system memory.
+  @retval EFI_INVALID_PARAMETER An inconsistency was detected between the mapping type
+                                and the DoubleBuffer field
 
 **/
 EFI_STATUS
@@ -160,6 +174,7 @@ DmaUnmap (
   )
 {
   MAP_INFO_INSTANCE *Map;
+  EFI_STATUS        Status;
 
   if (Mapping == NULL) {
     ASSERT (FALSE);
@@ -168,8 +183,13 @@ DmaUnmap (
 
   Map = (MAP_INFO_INSTANCE *)Mapping;
 
+  Status = EFI_SUCCESS;
   if (Map->DoubleBuffer) {
-    if ((Map->Operation == MapOperationBusMasterWrite) || (Map->Operation == MapOperationBusMasterCommonBuffer)) {
+    ASSERT (Map->Operation != MapOperationBusMasterCommonBuffer);
+
+    if (Map->Operation == MapOperationBusMasterCommonBuffer) {
+      Status = EFI_INVALID_PARAMETER;
+    } else if (Map->Operation == MapOperationBusMasterWrite) {
       CopyMem ((VOID *)(UINTN)Map->HostAddress, (VOID *)(UINTN)Map->DeviceAddress, Map->NumberOfBytes);
     }
 
@@ -186,7 +206,7 @@ DmaUnmap (
 
   FreePool (Map);
 
-  return EFI_SUCCESS;
+  return Status;
 }
 
 /**