* [PATCH] MdePkg/BaseMemoryLibOptDxe ARM AARCH64: fix thinko in SetMem##
@ 2016-09-22 8:54 Ard Biesheuvel
2016-09-23 5:01 ` Laszlo Ersek
2016-09-23 5:03 ` Gao, Liming
0 siblings, 2 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2016-09-22 8:54 UTC (permalink / raw)
To: edk2-devel, liming.gao; +Cc: lersek, leif.lindholm, 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 <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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] MdePkg/BaseMemoryLibOptDxe ARM AARCH64: fix thinko in SetMem##
2016-09-22 8:54 [PATCH] MdePkg/BaseMemoryLibOptDxe ARM AARCH64: fix thinko in SetMem## Ard Biesheuvel
@ 2016-09-23 5:01 ` Laszlo Ersek
2016-09-23 5:03 ` Gao, Liming
1 sibling, 0 replies; 4+ messages in thread
From: Laszlo Ersek @ 2016-09-23 5:01 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, liming.gao; +Cc: leif.lindholm
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] MdePkg/BaseMemoryLibOptDxe ARM AARCH64: fix thinko in SetMem##
2016-09-22 8:54 [PATCH] MdePkg/BaseMemoryLibOptDxe ARM AARCH64: fix thinko in SetMem## Ard Biesheuvel
2016-09-23 5:01 ` Laszlo Ersek
@ 2016-09-23 5:03 ` Gao, Liming
2016-09-23 10:55 ` Ard Biesheuvel
1 sibling, 1 reply; 4+ messages in thread
From: Gao, Liming @ 2016-09-23 5:03 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Cc: lersek@redhat.com, leif.lindholm@linaro.org
Reviewed-by: Liming Gao <liming.gao@intel.com>
> -----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
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] MdePkg/BaseMemoryLibOptDxe ARM AARCH64: fix thinko in SetMem##
2016-09-23 5:03 ` Gao, Liming
@ 2016-09-23 10:55 ` Ard Biesheuvel
0 siblings, 0 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2016-09-23 10:55 UTC (permalink / raw)
To: Gao, Liming
Cc: edk2-devel@lists.01.org, lersek@redhat.com,
leif.lindholm@linaro.org
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
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-23 10:55 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-22 8:54 [PATCH] MdePkg/BaseMemoryLibOptDxe ARM AARCH64: fix thinko in SetMem## Ard Biesheuvel
2016-09-23 5:01 ` Laszlo Ersek
2016-09-23 5:03 ` Gao, Liming
2016-09-23 10:55 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox