[PATCH 1/2] MdePkg/BaseMemoryLibOptDxe ARM: fix Thumb-2 bug in ScanMem()

[PATCH 1/2] MdePkg/BaseMemoryLibOptDxe ARM: fix Thumb-2 bug in ScanMem()

[Ard Biesheuvel]

The ARM ScanMem() in BaseMemoryLibOptDxe contains code from the open
source cortex-strings library, and inherited a bug from it where the
conditional execution of a sequence of instructions is erroneously
made dependent on the same condition. Since the final 'addeq' is
supposed to be dependent on the preceding 'tsteq' instruction, they
cannot be part of the same IT block.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S
index dc0e74e8657c..1c269547b072 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S
@@ -134,11 +134,12 @@ ASM_PFX(InternalMemScanMem8):
     bne     61f
     adds    r0, r0, #1
     tst     r5, #CHARTSTMASK(1)     // 2nd character
-    ittt    eq
-    addeq   r0, r0 ,#1
-    tsteq   r5, #(3 << 15)          // 2nd & 3rd character
+    bne     61f
+    adds    r0, r0 ,#1
+    tst     r5, #(3 << 15)          // 2nd & 3rd character
     // If not the 3rd must be the last one
-    addeq   r0, r0, #1
+    it      eq
+    addeq.n r0, r0, #1
 
 61:
     pop     {r4-r7}
-- 
2.7.4

[PATCH 2/2] MdePkg/BaseMemoryLibOptDxe: replace deprecated uses of IT blocks

[Ard Biesheuvel]

The ARM architecture version 8 deprecates all uses of the IT instruction
except cases where it is followed by a single narrow instruction. So
replace any occurrences with equivalent sequences that adhere to the
new rules.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 .../Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S  |  6 ++---
 .../Library/BaseMemoryLibOptDxe/Arm/CompareMem.S   |  2 +-
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/CopyMem.S   | 31 ++++++++++++----------
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S   |  9 ++++---
 MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S    | 12 +++++----
 5 files changed, 33 insertions(+), 27 deletions(-)

diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S
index d729994e8cc6..6d0089049d48 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareGuid.S
@@ -42,7 +42,7 @@ ASM_PFX(InternalMemCompareGuid):
     ldr     lr, [r1, #4]
     cmp     r2, ip
     it      eq
-    cmpeq   r3, lr
+    cmpeq.n r3, lr
     beq     0f
     movs    r0, #0
     pop     {r4, pc}
@@ -51,7 +51,7 @@ ASM_PFX(InternalMemCompareGuid):
     ldr     r3, [r1, #12]
     cmp     r4, r2
     it      eq
-    cmpeq   r0, r3
+    cmpeq.n r0, r3
     bne     2f
     movs    r0, #1
     pop     {r4, pc}
@@ -61,5 +61,5 @@ ASM_PFX(InternalMemCompareGuid):
     movs    r0, #1
     orrs    r2, r2, r4
 2:  it      ne
-    movne   r0, #0
+    movne.n r0, #0
     pop     {r4, pc}
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareMem.S b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareMem.S
index 3aadebace30f..9483aab61a0c 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareMem.S
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CompareMem.S
@@ -132,7 +132,7 @@ ASM_PFX(InternalMemCompareMem):
     ldrb    data2, [src2], #1
     subs    limit, limit, #1
     it      cs
-    cmpcs   data1, data2
+    cmpcs.n data1, data2
     beq     1b
     sub     result, data1, data2
     pop     {r4-r8, pc}
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CopyMem.S b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CopyMem.S
index fb5293befc10..195a0b23f770 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CopyMem.S
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/CopyMem.S
@@ -65,16 +65,18 @@ memcopy_check_optim_default:
     // Check if we can use an optimized path ((length >= 32) && destination word-aligned && source word-aligned) for the memcopy (optimized path if r0 == 1)
     tst     r0, #0xF
     it      ne
-    movne   r0, #0
+    movne.n r0, #0
     bne     memcopy_default
     tst     r1, #0xF
-    ite     ne
-    movne   r3, #0
-    moveq   r3, #1
+    it      ne
+    movne.n r3, #0
+    it      eq
+    moveq.n r3, #1
     cmp     r2, #31
-    ite     ls
-    movls   r0, #0
-    andhi   r0, r3, #1
+    it      ls
+    movls.n r0, #0
+    bls     memcopy_default
+    and     r0, r3, #1
     b       memcopy_default
 
 memcopy_check_optim_overlap:
@@ -84,15 +86,16 @@ memcopy_check_optim_overlap:
 
     // Are we in the optimized case ((length >= 32) && dest_end word-aligned && source_end word-aligned)
     cmp     r2, #31
-    ite     ls
-    movls   r0, #0
-    movhi   r0, #1
+    it      ls
+    movls.n r0, #0
+    it      hi
+    movhi.n r0, #1
     tst     r10, #0xF
     it      ne
-    movne   r0, #0
+    movne.n r0, #0
     tst     r14, #0xF
     it      ne
-    movne   r0, #0
+    movne.n r0, #0
     b       memcopy_overlapped
 
 memcopy_overlapped_non_optim:
@@ -123,7 +126,7 @@ memcopy_overlapped:
 
     // If length is less than 32 then disable optim
     it      ls
-    movls   r0, #0
+    movls.n r0, #0
 
     cmp     r12, #0
 
@@ -157,7 +160,7 @@ memcopy_default_loop:
 
     // If length is less than 32 then disable optim
     it      ls
-    movls   r0, #0
+    movls.n r0, #0
 
     cmp     r12, #0
 
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S
index 1c269547b072..5dcf153a61a9 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S
@@ -123,11 +123,12 @@ ASM_PFX(InternalMemScanMem8):
 60: // We're here because the fast path found a hit - now we have to track down exactly which word it was
     // r0 points to the start of the double word after the one that was tested
     // r5 has the 00/ff pattern for the first word, r6 has the chained value
+    subs    r0, r0, #3
     cmp     r5, #0
-    itte    eq
-    moveq   r5, r6        // the end is in the 2nd word
-    subeq   r0, r0, #3    // Points to 2nd byte of 2nd word
-    subne   r0, r0, #7    // or 2nd byte of 1st word
+    it      eq
+    moveq.n r5, r6        // the end is in the 2nd word
+    it      ne
+    subne.n r0, r0, #4    // or 2nd byte of 1st word
 
     // r0 currently points to the 3rd byte of the word containing the hit
     tst     r5, #CHARTSTMASK(0)     // 1st character
diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S
index add04443b2e9..2d8f4d5b8621 100644
--- a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S
+++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/SetMem.S
@@ -73,15 +73,17 @@ ASM_PFX(InternalMemZeroMem):
     cmp     r4, #4                  // between 4 and 15 bytes?
     blt     4f
     cmp     r4, #8                  // between 8 and 15 bytes?
-    str     r2, [lr, #-16]          // overlapping store of 4 + (4 + 4) + 4 bytes
-    itt     gt
-    strgt   r3, [lr, #-12]
-    strgt   r2, [r1]
+    sub     r4, lr, #16
+    str     r2, [r4]                // overlapping store of 4 + (4 + 4) + 4 bytes
+    it      gt
+    strgt.n r3, [r4, #4]
+    it      gt
+    strgt.n r2, [r1]
     str     r3, [r1, #4]
     pop     {r4, pc}
 
 4:  cmp     r4, #2                  // 2 or 3 bytes?
     strb    r2, [lr, #-16]          // store 1 byte
     it      ge
-    strhge  r2, [r1, #6]            // store 2 bytes
+    strhge.n r2, [r1, #6]           // store 2 bytes
     pop     {r4, pc}
-- 
2.7.4

Re: [PATCH 1/2] MdePkg/BaseMemoryLibOptDxe ARM: fix Thumb-2 bug in ScanMem()

[Gao, Liming]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
> Sent: Tuesday, September 27, 2016 6:59 AM
> To: edk2-devel@lists.01.org; Gao, Liming <liming.gao@intel.com>
> Cc: leif.lindholm@linaro.org; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: [PATCH 1/2] MdePkg/BaseMemoryLibOptDxe ARM: fix Thumb-2
> bug in ScanMem()
> 
> The ARM ScanMem() in BaseMemoryLibOptDxe contains code from the
> open
> source cortex-strings library, and inherited a bug from it where the
> conditional execution of a sequence of instructions is erroneously
> made dependent on the same condition. Since the final 'addeq' is
> supposed to be dependent on the preceding 'tsteq' instruction, they
> cannot be part of the same IT block.
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S
> b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S
> index dc0e74e8657c..1c269547b072 100644
> --- a/MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S
> +++ b/MdePkg/Library/BaseMemoryLibOptDxe/Arm/ScanMem.S
> @@ -134,11 +134,12 @@ ASM_PFX(InternalMemScanMem8):
>      bne     61f
>      adds    r0, r0, #1
>      tst     r5, #CHARTSTMASK(1)     // 2nd character
> -    ittt    eq
> -    addeq   r0, r0 ,#1
> -    tsteq   r5, #(3 << 15)          // 2nd & 3rd character
> +    bne     61f
> +    adds    r0, r0 ,#1
> +    tst     r5, #(3 << 15)          // 2nd & 3rd character
>      // If not the 3rd must be the last one
> -    addeq   r0, r0, #1
> +    it      eq
> +    addeq.n r0, r0, #1
> 
>  61:
>      pop     {r4-r7}
> --
> 2.7.4