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, liming.gao@intel.com
Cc: leif.lindholm@linaro.org, vishalo@qti.qualcomm.com,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH 2/2] MdePkg/BaseMemoryLibOptDxe ARM|AARCH64: implement accelerated GUID functions
Date: Fri, 16 Sep 2016 11:34:44 +0100	[thread overview]
Message-ID: <1474022084-19975-2-git-send-email-ard.biesheuvel@linaro.org> (raw)
In-Reply-To: <1474022084-19975-1-git-send-email-ard.biesheuvel@linaro.org>

As reported by Vishal, CompareGuid() is a hotspot, and switching from
BaseMemoryLibStm in ArmPkg/ to BaseMemoryLibOptDxe causes a noticeable
performance regression due to the fact that BaseMemoryLibOptDxe uses
unaligned accessors explicitly to implement CompareGuid() and the related
functions.

Since BaseMemoryLibOptDxe on ARM and AARCH64 can only be used in contexts
where unaligned accesses are allowed, reimplement these functions for ARM
and AARCH64 specifically, using wide accessors that can tolerate any
misalignment.

Reported-by: "Oliyil Kunnil, Vishal" <vishalo@qti.qualcomm.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdePkg/Library/BaseMemoryLibOptDxe/AArch64/CompareGuid.S   |  40 ++++++
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S       |  66 +++++++++
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.asm     |  72 ++++++++++
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/MemLibGuid.c        | 152 ++++++++++++++++++++
 MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf |   7 +-
 5 files changed, 336 insertions(+), 1 deletion(-)

diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/AArch64/CompareGuid.S b/MdePkg/Library/BaseMemoryLibOptDxe/AArch64/CompareGuid.S
new file mode 100644
index 000000000000..9e1b55cdf589
--- /dev/null
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/AArch64/CompareGuid.S
@@ -0,0 +1,40 @@
+//
+// Copyright (c) 2016, Linaro Limited
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above copyright
+//       notice, this list of conditions and the following disclaimer in the
+//       documentation and/or other materials provided with the distribution.
+//     * Neither the name of the Linaro nor the
+//       names of its contributors may be used to endorse or promote products
+//       derived from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+//
+
+    .text
+    .align  5
+ASM_GLOBAL ASM_PFX(InternalMemCompareGuid)
+ASM_PFX(InternalMemCompareGuid):
+    mov     x2, xzr
+    ldp     x3, x4, [x0]
+    cbz     x1, 0f
+    ldp     x1, x2, [x1]
+0:  cmp     x1, x3
+    ccmp    x2, x4, #0, eq
+    cset    w0, eq
+    ret
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S
new file mode 100644
index 000000000000..512a483ed1d2
--- /dev/null
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S
@@ -0,0 +1,66 @@
+//
+// Copyright (c) 2016, Linaro Limited
+// All rights reserved.
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//     * Redistributions of source code must retain the above copyright
+//       notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above copyright
+//       notice, this list of conditions and the following disclaimer in the
+//       documentation and/or other materials provided with the distribution.
+//     * Neither the name of the Linaro nor the
+//       names of its contributors may be used to endorse or promote products
+//       derived from this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+//
+
+    .text
+    .thumb
+    .syntax unified
+    .align  5
+ASM_GLOBAL ASM_PFX(InternalMemCompareGuid)
+ASM_PFX(InternalMemCompareGuid):
+    push    {r4-r6, lr}
+    ldr     r2, [r0]
+    ldr     r3, [r0, #4]
+    ldr     r4, [r0, #8]
+    ldr     r0, [r0, #12]
+    cbz     r1, 1f
+    ldr     r5, [r1]
+    ldr     r6, [r1, #4]
+    cmp     r2, r5
+    it      eq
+    cmpeq   r3, r6
+    beq     2f
+
+0:  movs    r0, #0
+    pop     {r4-r6, pc}
+
+1:  movs    r0, #0
+    cbnz    r2, 4f
+    cbnz    r3, 4f
+    cbnz    r4, 4f
+    cbnz    r0, 4f
+    b       3f
+
+2:  ldr     r5, [r1, #8]
+    ldr     r1, [r1, #12]
+    cmp     r4, r5
+    it      eq
+    cmpeq   r0, r1
+    bne     0b
+
+3:  movs    r0, #1
+4:  pop     {r4-r6, pc}
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.asm b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.asm
new file mode 100644
index 000000000000..e1d434623380
--- /dev/null
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.asm
@@ -0,0 +1,72 @@
+;
+; Copyright (c) 2016, Linaro Limited
+; All rights reserved.
+;
+; Redistribution and use in source and binary forms, with or without
+; modification, are permitted provided that the following conditions are met:
+;     * Redistributions of source code must retain the above copyright
+;       notice, this list of conditions and the following disclaimer.
+;     * Redistributions in binary form must reproduce the above copyright
+;       notice, this list of conditions and the following disclaimer in the
+;       documentation and/or other materials provided with the distribution.
+;     * Neither the name of the Linaro nor the
+;       names of its contributors may be used to endorse or promote products
+;       derived from this software without specific prior written permission.
+;
+; THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+; "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+; LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+; A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+; HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+; SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+; LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+; DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+; THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+; (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+; OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+;
+
+    EXPORT  InternalMemCompareGuid
+    THUMB
+    AREA    CompareGuid, CODE, READONLY, CODEALIGN, ALIGN=5
+
+InternalMemCompareGuid
+    push    {r4-r6, lr}
+    ldr     r2, [r0]
+    ldr     r3, [r0, #4]
+    ldr     r4, [r0, #8]
+    ldr     r0, [r0, #12]
+    cbz     r1, L1
+    ldr     r5, [r1]
+    ldr     r6, [r1, #4]
+    cmp     r2, r5
+    it      eq
+    cmpeq   r3, r6
+    beq     L0
+
+L0
+    movs    r0, #0
+    pop     {r4-r6, pc}
+
+L1
+    movs    r0, #0
+    cbnz    r2, L4
+    cbnz    r3, L4
+    cbnz    r4, L4
+    cbnz    r0, L4
+    b       L3
+
+L2
+    ldr     r5, [r1, #8]
+    ldr     r1, [r1, #12]
+    cmp     r4, r5
+    it      eq
+    cmpeq   r0, r1
+    bne     L0
+
+L3
+    movs    r0, #1
+L4
+    pop     {r4-r6, pc}
+
+    END
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/MemLibGuid.c b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/MemLibGuid.c
new file mode 100644
index 000000000000..8f1e50b2ed02
--- /dev/null
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/MemLibGuid.c
@@ -0,0 +1,152 @@
+/** @file
+  Implementation of GUID functions for ARM and AARCH64
+
+  Copyright (c) 2006 - 2016, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016, Linaro Ltd. All rights reserved.<BR>
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "MemLibInternals.h"
+
+BOOLEAN
+EFIAPI
+InternalMemCompareGuid (
+  IN CONST GUID  *Guid1,
+  IN CONST GUID  *Guid2
+  );
+
+/**
+  Copies a source GUID to a destination GUID.
+
+  This function copies the contents of the 128-bit GUID specified by SourceGuid to
+  DestinationGuid, and returns DestinationGuid.
+
+  If DestinationGuid is NULL, then ASSERT().
+  If SourceGuid is NULL, then ASSERT().
+
+  @param  DestinationGuid   The pointer to the destination GUID.
+  @param  SourceGuid        The pointer to the source GUID.
+
+  @return DestinationGuid.
+
+**/
+GUID *
+EFIAPI
+CopyGuid (
+  OUT GUID       *DestinationGuid,
+  IN CONST GUID  *SourceGuid
+  )
+{
+  ASSERT (DestinationGuid != NULL);
+  ASSERT (SourceGuid != NULL);
+
+  return InternalMemCopyMem (DestinationGuid, SourceGuid, sizeof (GUID));
+}
+
+/**
+  Compares two GUIDs.
+
+  This function compares Guid1 to Guid2.  If the GUIDs are identical then TRUE is returned.
+  If there are any bit differences in the two GUIDs, then FALSE is returned.
+
+  If Guid1 is NULL, then ASSERT().
+  If Guid2 is NULL, then ASSERT().
+
+  @param  Guid1       A pointer to a 128 bit GUID.
+  @param  Guid2       A pointer to a 128 bit GUID.
+
+  @retval TRUE        Guid1 and Guid2 are identical.
+  @retval FALSE       Guid1 and Guid2 are not identical.
+
+**/
+BOOLEAN
+EFIAPI
+CompareGuid (
+  IN CONST GUID  *Guid1,
+  IN CONST GUID  *Guid2
+  )
+{
+  ASSERT (Guid1 != NULL);
+  ASSERT (Guid2 != NULL);
+
+  return InternalMemCompareGuid (Guid1, Guid2);
+}
+
+/**
+  Scans a target buffer for a GUID, and returns a pointer to the matching GUID
+  in the target buffer.
+
+  This function searches the target buffer specified by Buffer and Length from
+  the lowest address to the highest address at 128-bit increments for the 128-bit
+  GUID value that matches Guid.  If a match is found, then a pointer to the matching
+  GUID in the target buffer is returned.  If no match is found, then NULL is returned.
+  If Length is 0, then NULL is returned.
+
+  If Length > 0 and Buffer is NULL, then ASSERT().
+  If Buffer is not aligned on a 32-bit boundary, then ASSERT().
+  If Length is not aligned on a 128-bit boundary, then ASSERT().
+  If Length is greater than (MAX_ADDRESS - Buffer + 1), then ASSERT().
+
+  @param  Buffer  The pointer to the target buffer to scan.
+  @param  Length  The number of bytes in Buffer to scan.
+  @param  Guid    The value to search for in the target buffer.
+
+  @return A pointer to the matching Guid in the target buffer or NULL otherwise.
+
+**/
+VOID *
+EFIAPI
+ScanGuid (
+  IN CONST VOID  *Buffer,
+  IN UINTN       Length,
+  IN CONST GUID  *Guid
+  )
+{
+  CONST GUID                        *GuidPtr;
+
+  ASSERT (((UINTN)Buffer & (sizeof (Guid->Data1) - 1)) == 0);
+  ASSERT (Length <= (MAX_ADDRESS - (UINTN)Buffer + 1));
+  ASSERT ((Length & (sizeof (*GuidPtr) - 1)) == 0);
+
+  GuidPtr = (GUID*)Buffer;
+  Buffer  = GuidPtr + Length / sizeof (*GuidPtr);
+  while (GuidPtr < (CONST GUID*)Buffer) {
+    if (InternalMemCompareGuid (GuidPtr, Guid)) {
+      return (VOID*)GuidPtr;
+    }
+    GuidPtr++;
+  }
+  return NULL;
+}
+
+/**
+  Checks if the given GUID is a zero GUID.
+
+  This function checks whether the given GUID is a zero GUID. If the GUID is
+  identical to a zero GUID then TRUE is returned. Otherwise, FALSE is returned.
+
+  If Guid is NULL, then ASSERT().
+
+  @param  Guid        The pointer to a 128 bit GUID.
+
+  @retval TRUE        Guid is a zero GUID.
+  @retval FALSE       Guid is not a zero GUID.
+
+**/
+BOOLEAN
+EFIAPI
+IsZeroGuid (
+  IN CONST GUID  *Guid
+  )
+{
+  ASSERT (Guid != NULL);
+
+  return InternalMemCompareGuid (Guid, NULL);
+}
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
index 5ddc0cbc2d77..bc5ec2f25310 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/BaseMemoryLibOptDxe.inf
@@ -79,6 +79,7 @@ [Sources.Ia32]
   Ia32/CopyMem.nasm
   Ia32/CopyMem.asm
   Ia32/IsZeroBuffer.nasm
+  MemLibGuid.c
 
 [Sources.X64]
   X64/ScanMem64.nasm
@@ -115,6 +116,7 @@ [Sources.X64]
   X64/CopyMem.asm
   X64/CopyMem.S
   X64/IsZeroBuffer.nasm
+  MemLibGuid.c
 
 [Defines.ARM, Defines.AARCH64]
   #
@@ -130,20 +132,24 @@ [Sources.ARM]
   Arm/SetMem.S        |GCC
   Arm/CopyMem.S       |GCC
   Arm/CompareMem.S    |GCC
+  Arm/CompareGuid.S   |GCC
 
   Arm/ScanMem.asm     |RVCT
   Arm/SetMem.asm      |RVCT
   Arm/CopyMem.asm     |RVCT
   Arm/CompareMem.asm  |RVCT
+  Arm/CompareGuid.asm |RVCT
 
 [Sources.AARCH64]
   AArch64/ScanMem.S
   AArch64/SetMem.S
   AArch64/CopyMem.S
   AArch64/CompareMem.S
+  AArch64/CompareGuid.S
 
 [Sources.ARM, Sources.AARCH64]
   Arm/ScanMemGeneric.c
+  Arm/MemLibGuid.c
 
 [Sources]
   ScanMem64Wrapper.c
@@ -158,7 +164,6 @@ [Sources]
   SetMemWrapper.c
   CopyMemWrapper.c
   IsZeroBufferWrapper.c
-  MemLibGuid.c
 
 [Packages]
   MdePkg/MdePkg.dec
-- 
2.7.4



      reply	other threads:[~2016-09-16 10:34 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-16 10:34 [PATCH 1/2] MdePkg/BaseMemoryLib*: add missing ASSERT()s Ard Biesheuvel
2016-09-16 10:34 ` Ard Biesheuvel [this message]

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=1474022084-19975-2-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