diff mbox

[edk2] MdePkg/BaseMemoryLibOptDxe ARM AARCH64: fix thinko in SetMem##

Message ID 1474534469-18442-1-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit 8b4ca351dded404f992504c45e358572c4d236f9
Headers show

Commit Message

Ard Biesheuvel Sept. 22, 2016, 8:54 a.m. UTC 
The new InternalMemSetMem##() implementations for ARM and AARCH64 in
BaseMemoryLibOptDxe fail to take into account that the 'length' argument
is not in bytes, but in number of items to be copied. So multiply by the
item size before proceeding.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 MdePkg/Library/BaseMemoryLibOptDxe/AArch64/SetMem.S |  3 ++
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S     | 32 +++++++++++++-------
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.asm   | 28 ++++++++++++-----
 3 files changed, 44 insertions(+), 19 deletions(-)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Laszlo Ersek Sept. 23, 2016, 5:01 a.m. UTC | #1 
On 09/22/16 10:54, Ard Biesheuvel wrote:
> The new InternalMemSetMem##() implementations for ARM and AARCH64 in

> BaseMemoryLibOptDxe fail to take into account that the 'length' argument

> is not in bytes, but in number of items to be copied. So multiply by the

> item size before proceeding.

> 

> Contributed-under: TianoCore Contribution Agreement 1.0

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  MdePkg/Library/BaseMemoryLibOptDxe/AArch64/SetMem.S |  3 ++

>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S     | 32 +++++++++++++-------

>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.asm   | 28 ++++++++++++-----

>  3 files changed, 44 insertions(+), 19 deletions(-)


I tested it with the AARCH64 build of ArmVirtQemu. It works fine, thanks.

Reported-by: Laszlo Ersek <lersek@redhat.com>
Tested-by: Laszlo Ersek <lersek@redhat.com>


Cheers!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Sept. 23, 2016, 10:55 a.m. UTC | #2 
On 23 September 2016 at 06:03, Gao, Liming <liming.gao@intel.com> wrote:
> Reviewed-by: Liming Gao <liming.gao@intel.com>

>


Pushed, thanks all

>> -----Original Message-----

>> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]

>> Sent: Thursday, September 22, 2016 4:54 PM

>> To: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>

>> Cc: lersek@redhat.com; leif.lindholm@linaro.org; Ard Biesheuvel

>> <ard.biesheuvel@linaro.org>

>> Subject: [PATCH] MdePkg/BaseMemoryLibOptDxe ARM AARCH64: fix thinko

>> in SetMem##

>>

>> The new InternalMemSetMem##() implementations for ARM and AARCH64

>> in

>> BaseMemoryLibOptDxe fail to take into account that the 'length' argument

>> is not in bytes, but in number of items to be copied. So multiply by the

>> item size before proceeding.

>>

>> Contributed-under: TianoCore Contribution Agreement 1.0

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> ---

>>  MdePkg/Library/BaseMemoryLibOptDxe/AArch64/SetMem.S |  3 ++

>>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S     | 32

>> +++++++++++++-------

>>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.asm   | 28

>> ++++++++++++-----

>>  3 files changed, 44 insertions(+), 19 deletions(-)

>>

>> diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/AArch64/SetMem.S

>> b/MdePkg/Library/BaseMemoryLibOptDxe/AArch64/SetMem.S

>> index 7f361110d4fe..ec58f759d7b6 100644

>> --- a/MdePkg/Library/BaseMemoryLibOptDxe/AArch64/SetMem.S

>> +++ b/MdePkg/Library/BaseMemoryLibOptDxe/AArch64/SetMem.S

>> @@ -78,16 +78,19 @@

>>  ASM_GLOBAL ASM_PFX(InternalMemSetMem16)

>>  ASM_PFX(InternalMemSetMem16):

>>      dup     v0.8H, valw

>> +    lsl     count, count, #1

>>      b       0f

>>

>>  ASM_GLOBAL ASM_PFX(InternalMemSetMem32)

>>  ASM_PFX(InternalMemSetMem32):

>>      dup     v0.4S, valw

>> +    lsl     count, count, #2

>>      b       0f

>>

>>  ASM_GLOBAL ASM_PFX(InternalMemSetMem64)

>>  ASM_PFX(InternalMemSetMem64):

>>      dup     v0.2D, val

>> +    lsl     count, count, #3

>>      b       0f

>>

>>  ASM_GLOBAL ASM_PFX(InternalMemZeroMem)

>> diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S

>> b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S

>> index c1755539d36a..add04443b2e9 100644

>> --- a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S

>> +++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S

>> @@ -16,27 +16,37 @@

>>      .thumb

>>      .syntax unified

>>      .align  5

>> -ASM_GLOBAL ASM_PFX(InternalMemZeroMem)

>> -ASM_PFX(InternalMemZeroMem):

>> -    movs    r2, #0

>> -

>> -ASM_GLOBAL ASM_PFX(InternalMemSetMem)

>> -ASM_PFX(InternalMemSetMem):

>> -    uxtb    r2, r2

>> -    orr     r2, r2, r2, lsl #8

>> -

>>  ASM_GLOBAL ASM_PFX(InternalMemSetMem16)

>>  ASM_PFX(InternalMemSetMem16):

>>      uxth    r2, r2

>> +    lsl     r1, r1, #1

>>      orr     r2, r2, r2, lsl #16

>> +    b       0f

>>

>>  ASM_GLOBAL ASM_PFX(InternalMemSetMem32)

>>  ASM_PFX(InternalMemSetMem32):

>> -    mov     r3, r2

>> +    lsl     r1, r1, #2

>> +    b       0f

>>

>>  ASM_GLOBAL ASM_PFX(InternalMemSetMem64)

>>  ASM_PFX(InternalMemSetMem64):

>> -    push    {r4, lr}

>> +    lsl     r1, r1, #3

>> +    b       1f

>> +

>> +    .align  5

>> +ASM_GLOBAL ASM_PFX(InternalMemSetMem)

>> +ASM_PFX(InternalMemSetMem):

>> +    uxtb    r2, r2

>> +    orr     r2, r2, r2, lsl #8

>> +    orr     r2, r2, r2, lsl #16

>> +    b       0f

>> +

>> +ASM_GLOBAL ASM_PFX(InternalMemZeroMem)

>> +ASM_PFX(InternalMemZeroMem):

>> +    movs    r2, #0

>> +0:  mov     r3, r2

>> +

>> +1:  push    {r4, lr}

>>      cmp     r1, #16                 // fewer than 16 bytes of input?

>>      add     r1, r1, r0              // r1 := dst + length

>>      add     lr, r0, #16

>> diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.asm

>> b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.asm

>> index 2a8dc7d019f4..c2e2842a6323 100644

>> --- a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.asm

>> +++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.asm

>> @@ -21,21 +21,33 @@

>>      AREA    SetMem, CODE, READONLY, CODEALIGN, ALIGN=5

>>      THUMB

>>

>> -InternalMemZeroMem

>> -    movs    r2, #0

>> +InternalMemSetMem16

>> +    uxth    r2, r2

>> +    lsl     r1, r1, #1

>> +    orr     r2, r2, r2, lsl #16

>> +    b       B0

>> +

>> +InternalMemSetMem32

>> +    lsl     r1, r1, #2

>> +    b       B0

>> +

>> +InternalMemSetMem64

>> +    lsl     r1, r1, #3

>> +    b       B1

>>

>> +    ALIGN   32

>>  InternalMemSetMem

>>      uxtb    r2, r2

>>      orr     r2, r2, r2, lsl #8

>> +    orr     r2, r2, r2, lsl #16

>> +    b       B0

>>

>> -InternalMemSetMem16

>> -    uxth    r2, r2

>> -    orr     r2, r2, r2, lsr #16

>> -

>> -InternalMemSetMem32

>> +InternalMemZeroMem

>> +    movs    r2, #0

>> +B0

>>      mov     r3, r2

>>

>> -InternalMemSetMem64

>> +B1

>>      push    {r4, lr}

>>      cmp     r1, #16                 ; fewer than 16 bytes of input?

>>      add     r1, r1, r0              ; r1 := dst + length

>> --

>> 2.7.4

>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox

Patch

diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/AArch64/SetMem.S b/MdePkg/Library/BaseMemoryLibOptDxe/AArch64/SetMem.S
index 7f361110d4fe..ec58f759d7b6 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/AArch64/SetMem.S
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/AArch64/SetMem.S
@@ -78,16 +78,19 @@ 
 ASM_GLOBAL ASM_PFX(InternalMemSetMem16)
 ASM_PFX(InternalMemSetMem16):
     dup     v0.8H, valw
+    lsl     count, count, #1
     b       0f
 
 ASM_GLOBAL ASM_PFX(InternalMemSetMem32)
 ASM_PFX(InternalMemSetMem32):
     dup     v0.4S, valw
+    lsl     count, count, #2
     b       0f
 
 ASM_GLOBAL ASM_PFX(InternalMemSetMem64)
 ASM_PFX(InternalMemSetMem64):
     dup     v0.2D, val
+    lsl     count, count, #3
     b       0f
 
 ASM_GLOBAL ASM_PFX(InternalMemZeroMem)
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S
index c1755539d36a..add04443b2e9 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S
@@ -16,27 +16,37 @@ 
     .thumb
     .syntax unified
     .align  5
-ASM_GLOBAL ASM_PFX(InternalMemZeroMem)
-ASM_PFX(InternalMemZeroMem):
-    movs    r2, #0
-
-ASM_GLOBAL ASM_PFX(InternalMemSetMem)
-ASM_PFX(InternalMemSetMem):
-    uxtb    r2, r2
-    orr     r2, r2, r2, lsl #8
-
 ASM_GLOBAL ASM_PFX(InternalMemSetMem16)
 ASM_PFX(InternalMemSetMem16):
     uxth    r2, r2
+    lsl     r1, r1, #1
     orr     r2, r2, r2, lsl #16
+    b       0f
 
 ASM_GLOBAL ASM_PFX(InternalMemSetMem32)
 ASM_PFX(InternalMemSetMem32):
-    mov     r3, r2
+    lsl     r1, r1, #2
+    b       0f
 
 ASM_GLOBAL ASM_PFX(InternalMemSetMem64)
 ASM_PFX(InternalMemSetMem64):
-    push    {r4, lr}
+    lsl     r1, r1, #3
+    b       1f
+
+    .align  5
+ASM_GLOBAL ASM_PFX(InternalMemSetMem)
+ASM_PFX(InternalMemSetMem):
+    uxtb    r2, r2
+    orr     r2, r2, r2, lsl #8
+    orr     r2, r2, r2, lsl #16
+    b       0f
+
+ASM_GLOBAL ASM_PFX(InternalMemZeroMem)
+ASM_PFX(InternalMemZeroMem):
+    movs    r2, #0
+0:  mov     r3, r2
+
+1:  push    {r4, lr}
     cmp     r1, #16                 // fewer than 16 bytes of input?
     add     r1, r1, r0              // r1 := dst + length
     add     lr, r0, #16
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.asm b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.asm
index 2a8dc7d019f4..c2e2842a6323 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.asm
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.asm
@@ -21,21 +21,33 @@ 
     AREA    SetMem, CODE, READONLY, CODEALIGN, ALIGN=5
     THUMB
 
-InternalMemZeroMem
-    movs    r2, #0
+InternalMemSetMem16
+    uxth    r2, r2
+    lsl     r1, r1, #1
+    orr     r2, r2, r2, lsl #16
+    b       B0
+
+InternalMemSetMem32
+    lsl     r1, r1, #2
+    b       B0
+
+InternalMemSetMem64
+    lsl     r1, r1, #3
+    b       B1
 
+    ALIGN   32
 InternalMemSetMem
     uxtb    r2, r2
     orr     r2, r2, r2, lsl #8
+    orr     r2, r2, r2, lsl #16
+    b       B0
 
-InternalMemSetMem16
-    uxth    r2, r2
-    orr     r2, r2, r2, lsr #16
-
-InternalMemSetMem32
+InternalMemZeroMem
+    movs    r2, #0
+B0
     mov     r3, r2
 
-InternalMemSetMem64
+B1
     push    {r4, lr}
     cmp     r1, #16                 ; fewer than 16 bytes of input?
     add     r1, r1, r0              ; r1 := dst + length