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>

1 files 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
@@ -104,6 +104,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;

 }

 

 /**