public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>
To: edk2-devel@lists.01.org, leif.lindholm@linaro.org
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH v2 3/4] ArmPkg/ArmDmaLib: clean up abuse of device address
Date: Sat, 12 Nov 2016 14:02:27 +0100	[thread overview]
Message-ID: <1478955748-14819-4-git-send-email-ard.biesheuvel@linaro.org> (raw)
In-Reply-To: <1478955748-14819-1-git-send-email-ard.biesheuvel@linaro.org>

In preparation of adding support to ArmDmalib for DMA bus masters whose
view of memory is offset by a constant compared to the CPU's view, clean
up some abuse of the device address.

The device address is not defined in terms of the CPU's address space,
and so it should not be used in CopyMem () or cache maintenance operations
that require a valid mapping. This not only applies to the above use case,
but also to the DebugUncachedMemoryAllocationLib that unmaps the
primary, cached mapping of an allocation, and returns a host address
which is an uncached alias offset by a constant.

Since we should never access the device address from the CPU, there is
no need to record it in the MAPINFO struct. Instead, record the buffer
address in case of double buffering, since we do need to copy the contents
(in case of a bus master write) and free the buffer (in all cases) when
DmaUnmap() is called.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
index c2a44398d25a..7321388de63e 100644
--- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
+++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c
@@ -27,7 +27,7 @@
 
 typedef struct {
   EFI_PHYSICAL_ADDRESS      HostAddress;
-  EFI_PHYSICAL_ADDRESS      DeviceAddress;
+  VOID                      *BufferAddress;
   UINTN                     NumberOfBytes;
   DMA_MAP_OPERATION         Operation;
   BOOLEAN                   DoubleBuffer;
@@ -94,7 +94,7 @@ DmaMap (
       ((*NumberOfBytes & (mCpu->DmaBufferAlignment - 1)) != 0)) {
 
     // Get the cacheability of the region
-    Status = gDS->GetMemorySpaceDescriptor (*DeviceAddress, &GcdDescriptor);
+    Status = gDS->GetMemorySpaceDescriptor ((UINTN)HostAddress, &GcdDescriptor);
     if (EFI_ERROR(Status)) {
       return Status;
     }
@@ -128,6 +128,7 @@ DmaMap (
       }
 
       *DeviceAddress = ConvertToPhysicalAddress ((UINTN)Buffer);
+      Map->BufferAddress = Buffer;
     } else {
       Map->DoubleBuffer  = FALSE;
     }
@@ -143,7 +144,7 @@ DmaMap (
     // So duplicate the check here when running in DEBUG mode, just to assert
     // that we are not trying to create a consistent mapping for cached memory.
     //
-    Status = gDS->GetMemorySpaceDescriptor (*DeviceAddress, &GcdDescriptor);
+    Status = gDS->GetMemorySpaceDescriptor ((UINTN)HostAddress, &GcdDescriptor);
     ASSERT_EFI_ERROR(Status);
 
     ASSERT (Operation != MapOperationBusMasterCommonBuffer ||
@@ -152,12 +153,11 @@ DmaMap (
     DEBUG_CODE_END ();
 
     // Flush the Data Cache (should not have any effect if the memory region is uncached)
-    mCpu->FlushDataCache (mCpu, *DeviceAddress, *NumberOfBytes,
+    mCpu->FlushDataCache (mCpu, (UINTN)HostAddress, *NumberOfBytes,
             EfiCpuFlushTypeWriteBackInvalidate);
   }
 
   Map->HostAddress   = (UINTN)HostAddress;
-  Map->DeviceAddress = *DeviceAddress;
   Map->NumberOfBytes = *NumberOfBytes;
   Map->Operation     = Operation;
 
@@ -200,10 +200,11 @@ DmaUnmap (
     if (Map->Operation == MapOperationBusMasterCommonBuffer) {
       Status = EFI_INVALID_PARAMETER;
     } else if (Map->Operation == MapOperationBusMasterWrite) {
-      CopyMem ((VOID *)(UINTN)Map->HostAddress, (VOID *)(UINTN)Map->DeviceAddress, Map->NumberOfBytes);
+      CopyMem ((VOID *)(UINTN)Map->HostAddress, Map->BufferAddress,
+        Map->NumberOfBytes);
     }
 
-    DmaFreeBuffer (EFI_SIZE_TO_PAGES (Map->NumberOfBytes), (VOID *)(UINTN)Map->DeviceAddress);
+    DmaFreeBuffer (EFI_SIZE_TO_PAGES (Map->NumberOfBytes), Map->BufferAddress);
 
   } else {
     if (Map->Operation == MapOperationBusMasterWrite) {
-- 
2.7.4



  parent reply	other threads:[~2016-11-12 13:02 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-12 13:02 [PATCH v2 0/4] ArmPkg: more ArmDmaLib fixes Ard Biesheuvel
2016-11-12 13:02 ` [PATCH v2 1/4] ArmPkg/ArmDmaLib: use DMA buffer alignment from CPU arch protocol Ard Biesheuvel
2016-11-14 15:18   ` Leif Lindholm
2016-11-12 13:02 ` [PATCH v2 2/4] ArmPkg/ArmDmaLib: fix incorrect device address of double buffer Ard Biesheuvel
2016-11-14 15:13   ` Leif Lindholm
2016-11-12 13:02 ` Ard Biesheuvel [this message]
2016-11-14 15:16   ` [PATCH v2 3/4] ArmPkg/ArmDmaLib: clean up abuse of device address Leif Lindholm
2016-11-15  9:19     ` Ard Biesheuvel
2016-11-15 11:34       ` Ryan Harkin
2016-11-15 13:07         ` Ard Biesheuvel
2016-11-15 18:01           ` Leif Lindholm
2016-11-30 16:45             ` Leif Lindholm
2016-11-12 13:02 ` [PATCH v2 4/4] ArmPkg/ArmDmaLib: add support for fixed host-to-device DMA offset Ard Biesheuvel
2016-11-14 15:17   ` Leif Lindholm

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=1478955748-14819-4-git-send-email-ard.biesheuvel@linaro.org \
    --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