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

[Ard Biesheuvel]

On 23 September 2016 at 06:03, Gao, Liming  wrote:
> Reviewed-by: Liming Gao 
>

Pushed, thanks all

>> -Original Message-
>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
>> Sent: Thursday, September 22, 2016 4:54 PM
>> To: edk2-devel@lists.01.org; Gao, Liming 
>> Cc: ler...@redhat.com; leif.lindh...@linaro.org; Ard Biesheuvel
>> 
>> 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 
>> ---
>>  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):
>> -movsr2, #0
>> -
>> -ASM_GLOBAL ASM_PFX(InternalMemSetMem)
>> -ASM_PFX(InternalMemSetMem):
>> -uxtbr2, r2
>> -orr r2, r2, r2, lsl #8
>> -
>>  ASM_GLOBAL ASM_PFX(InternalMemSetMem16)
>>  ASM_PFX(InternalMemSetMem16):
>>  uxthr2, 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):
>> +uxtbr2, r2
>> +orr r2, r2, r2, lsl #8
>> +orr r2, r2, r2, lsl #16
>> +b   0f
>> +
>> +ASM_GLOBAL ASM_PFX(InternalMemZeroMem)
>> +ASM_PFX(InternalMemZeroMem):
>> +movsr2, #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 @@
>>  AREASetMem, CODE, READONLY, CODEALIGN, ALIGN=5
>>  THUMB
>>
>> -InternalMemZeroMem
>> -movsr2, #0
>> +InternalMemSetMem16
>> +uxthr2, 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
>>  uxtbr2, r2
>>  orr r2, r2, r2, lsl #8
>> +orr r2, r2, r2, lsl #16
>> +b   B0
>>
>> -InternalMemSetMem16
>> -uxthr2, r2
>> -orr r2, r2, r2, lsr #16
>> -
>> -InternalMemSetMem32
>> +InternalMemZeroMem
>> +movsr2, #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

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

[Gao, Liming]

Reviewed-by: Liming Gao 

> -Original Message-
> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org]
> Sent: Thursday, September 22, 2016 4:54 PM
> To: edk2-devel@lists.01.org; Gao, Liming 
> Cc: ler...@redhat.com; leif.lindh...@linaro.org; Ard Biesheuvel
> 
> 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 
> ---
>  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):
> -movsr2, #0
> -
> -ASM_GLOBAL ASM_PFX(InternalMemSetMem)
> -ASM_PFX(InternalMemSetMem):
> -uxtbr2, r2
> -orr r2, r2, r2, lsl #8
> -
>  ASM_GLOBAL ASM_PFX(InternalMemSetMem16)
>  ASM_PFX(InternalMemSetMem16):
>  uxthr2, 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):
> +uxtbr2, r2
> +orr r2, r2, r2, lsl #8
> +orr r2, r2, r2, lsl #16
> +b   0f
> +
> +ASM_GLOBAL ASM_PFX(InternalMemZeroMem)
> +ASM_PFX(InternalMemZeroMem):
> +movsr2, #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 @@
>  AREASetMem, CODE, READONLY, CODEALIGN, ALIGN=5
>  THUMB
> 
> -InternalMemZeroMem
> -movsr2, #0
> +InternalMemSetMem16
> +uxthr2, 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
>  uxtbr2, r2
>  orr r2, r2, r2, lsl #8
> +orr r2, r2, r2, lsl #16
> +b   B0
> 
> -InternalMemSetMem16
> -uxthr2, r2
> -orr r2, r2, r2, lsr #16
> -
> -InternalMemSetMem32
> +InternalMemZeroMem
> +movsr2, #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

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

[Laszlo Ersek]

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 
> ---
>  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 
Tested-by: Laszlo Ersek 

Cheers!
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

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

[Ard Biesheuvel]

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 
---
 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):
-movsr2, #0
-
-ASM_GLOBAL ASM_PFX(InternalMemSetMem)
-ASM_PFX(InternalMemSetMem):
-uxtbr2, r2
-orr r2, r2, r2, lsl #8
-
 ASM_GLOBAL ASM_PFX(InternalMemSetMem16)
 ASM_PFX(InternalMemSetMem16):
 uxthr2, 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):
+uxtbr2, r2
+orr r2, r2, r2, lsl #8
+orr r2, r2, r2, lsl #16
+b   0f
+
+ASM_GLOBAL ASM_PFX(InternalMemZeroMem)
+ASM_PFX(InternalMemZeroMem):
+movsr2, #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 @@
 AREASetMem, CODE, READONLY, CODEALIGN, ALIGN=5
 THUMB
 
-InternalMemZeroMem
-movsr2, #0
+InternalMemSetMem16
+uxthr2, 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
 uxtbr2, r2
 orr r2, r2, r2, lsl #8
+orr r2, r2, r2, lsl #16
+b   B0
 
-InternalMemSetMem16
-uxthr2, r2
-orr r2, r2, r2, lsr #16
-
-InternalMemSetMem32
+InternalMemZeroMem
+movsr2, #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