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] ArmPlatformPkg/NorFlashDxe: use strictly aligned CopyMem()
Date: Thu,  8 Sep 2016 18:32:05 +0100	[thread overview]
Message-ID: <1473355925-26678-1-git-send-email-ard.biesheuvel@linaro.org> (raw)

The UEFI spec stipulates that unaligned accesses should be enabled
on CPUs that support them, which means all of them, given that we
no longer support pre-v7 ARM cores, and the AARCH64 bindings mandate
support for unaligned accesses unconditionally.

This means that one should not assume that CopyMem () is safe to call
on regions that may be mapped using device attributes, which is the
case for the NOR flash. Since we have no control over the mappings when
running under the OS, and given that write accesses require device
mappings, we should not call CopyMem () in the read path either, but
use our own implementation that is guaranteed to take alignment into
account.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 130 +++++++++++++++++++-
 1 file changed, 128 insertions(+), 2 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
index 3abbe5cb32bc..45d1f0c20d52 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
@@ -744,6 +744,132 @@ NorFlashWriteBlocks (
   return Status;
 }
 
+/**
+  Copy Length bytes from Source to Destination, using aligned accesses only
+
+  @param  DestinationBuffer The target of the copy request.
+  @param  SourceBuffer      The place to copy from.
+  @param  Length            The number of bytes to copy.
+
+  @return Destination
+
+**/
+STATIC
+VOID *
+AlignedCopyMem (
+  OUT     VOID                      *DestinationBuffer,
+  IN      CONST VOID                *SourceBuffer,
+  IN      UINTN                     Length
+  )
+{
+  UINT8             *Destination8;
+  CONST UINT8       *Source8;
+  UINT32            *Destination32;
+  CONST UINT32      *Source32;
+  UINT64            *Destination64;
+  CONST UINT64      *Source64;
+  UINTN             Alignment;
+
+  if ((((UINTN)DestinationBuffer & 0x7) == 0) && (((UINTN)SourceBuffer & 0x7) == 0) && (Length >= 8)) {
+    if (SourceBuffer > DestinationBuffer) {
+      Destination64 = (UINT64*)DestinationBuffer;
+      Source64 = (CONST UINT64*)SourceBuffer;
+      while (Length >= 8) {
+        *(Destination64++) = *(Source64++);
+        Length -= 8;
+      }
+
+      // Finish if there are still some bytes to copy
+      Destination8 = (UINT8*)Destination64;
+      Source8 = (CONST UINT8*)Source64;
+      while (Length-- != 0) {
+        *(Destination8++) = *(Source8++);
+      }
+    } else if (SourceBuffer < DestinationBuffer) {
+      Destination64 = (UINT64*)((UINTN)DestinationBuffer + Length);
+      Source64 = (CONST UINT64*)((UINTN)SourceBuffer + Length);
+
+      // Destination64 and Source64 were aligned on a 64-bit boundary
+      // but if length is not a multiple of 8 bytes then they won't be
+      // anymore.
+
+      Alignment = Length & 0x7;
+      if (Alignment != 0) {
+        Destination8 = (UINT8*)Destination64;
+        Source8 = (CONST UINT8*)Source64;
+
+        while (Alignment-- != 0) {
+          *(--Destination8) = *(--Source8);
+          --Length;
+        }
+        Destination64 = (UINT64*)Destination8;
+        Source64 = (CONST UINT64*)Source8;
+      }
+
+      while (Length > 0) {
+        *(--Destination64) = *(--Source64);
+        Length -= 8;
+      }
+    }
+  } else if ((((UINTN)DestinationBuffer & 0x3) == 0) && (((UINTN)SourceBuffer & 0x3) == 0) && (Length >= 4)) {
+    if (SourceBuffer > DestinationBuffer) {
+      Destination32 = (UINT32*)DestinationBuffer;
+      Source32 = (CONST UINT32*)SourceBuffer;
+      while (Length >= 4) {
+        *(Destination32++) = *(Source32++);
+        Length -= 4;
+      }
+
+      // Finish if there are still some bytes to copy
+      Destination8 = (UINT8*)Destination32;
+      Source8 = (CONST UINT8*)Source32;
+      while (Length-- != 0) {
+        *(Destination8++) = *(Source8++);
+      }
+    } else if (SourceBuffer < DestinationBuffer) {
+      Destination32 = (UINT32*)((UINTN)DestinationBuffer + Length);
+      Source32 = (CONST UINT32*)((UINTN)SourceBuffer + Length);
+
+      // Destination32 and Source32 were aligned on a 32-bit boundary
+      // but if length is not a multiple of 4 bytes then they won't be
+      // anymore.
+
+      Alignment = Length & 0x3;
+      if (Alignment != 0) {
+        Destination8 = (UINT8*)Destination32;
+        Source8 = (CONST UINT8*)Source32;
+
+        while (Alignment-- != 0) {
+          *(--Destination8) = *(--Source8);
+          --Length;
+        }
+        Destination32 = (UINT32*)Destination8;
+        Source32 = (CONST UINT32*)Source8;
+      }
+
+      while (Length > 0) {
+        *(--Destination32) = *(--Source32);
+        Length -= 4;
+      }
+    }
+  } else {
+    if (SourceBuffer > DestinationBuffer) {
+      Destination8 = (UINT8*)DestinationBuffer;
+      Source8 = (CONST UINT8*)SourceBuffer;
+      while (Length-- != 0) {
+        *(Destination8++) = *(Source8++);
+      }
+    } else if (SourceBuffer < DestinationBuffer) {
+      Destination8 = (UINT8*)DestinationBuffer + Length;
+      Source8 = (CONST UINT8*)SourceBuffer + Length;
+      while (Length-- != 0) {
+        *(--Destination8) = *(--Source8);
+      }
+    }
+  }
+  return DestinationBuffer;
+}
+
 EFI_STATUS
 NorFlashReadBlocks (
   IN NOR_FLASH_INSTANCE   *Instance,
@@ -791,7 +917,7 @@ NorFlashReadBlocks (
   SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY);
 
   // Readout the data
-  CopyMem(Buffer, (UINTN *)StartAddress, BufferSizeInBytes);
+  AlignedCopyMem (Buffer, (UINTN *)StartAddress, BufferSizeInBytes);
 
   return EFI_SUCCESS;
 }
@@ -832,7 +958,7 @@ NorFlashRead (
   SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY);
 
   // Readout the data
-  CopyMem (Buffer, (UINTN *)(StartAddress + Offset), BufferSizeInBytes);
+  AlignedCopyMem (Buffer, (UINTN *)(StartAddress + Offset), BufferSizeInBytes);
 
   return EFI_SUCCESS;
 }
-- 
2.7.4



             reply	other threads:[~2016-09-08 17:32 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-08 17:32 Ard Biesheuvel [this message]
2016-09-09 11:08 ` [PATCH] ArmPlatformPkg/NorFlashDxe: use strictly aligned CopyMem() Leif Lindholm
2016-09-09 11:13   ` Ard Biesheuvel

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=1473355925-26678-1-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