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

[Ard Biesheuvel <ard.biesheuvel@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
>