From: "Gao, Liming" <liming.gao@intel.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "lersek@redhat.com" <lersek@redhat.com>,
"leif.lindholm@linaro.org" <leif.lindholm@linaro.org>
Subject: Re: [PATCH] MdePkg/BaseMemoryLibOptDxe ARM AARCH64: fix thinko in SetMem##
Date: Fri, 23 Sep 2016 05:03:59 +0000 [thread overview]
Message-ID: <4A89E2EF3DFEDB4C8BFDE51014F606A14B47C035@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <1474534469-18442-1-git-send-email-ard.biesheuvel@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
next prev parent reply other threads:[~2016-09-23 5:04 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
2016-09-23 10:55 ` 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=4A89E2EF3DFEDB4C8BFDE51014F606A14B47C035@shsmsx102.ccr.corp.intel.com \
--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