Re: [PATCH 06/17] xen: arm32: resync mem* with Linux v3.14-rc7

[Julien Grall <julien.grall@linaro.org>]

On 03/20/2014 03:45 PM, Ian Campbell wrote:
> This pulls in the following Linux commits:
> commit 455bd4c430b0c0a361f38e8658a0d6cb469942b5
> Author: Ivan Djelic <ivan.djelic@parrot.com>
> Date:   Wed Mar 6 20:09:27 2013 +0100
> 
>     ARM: 7668/1: fix memset-related crashes caused by recent GCC (4.7.2) optimi
> 
>     Recent GCC versions (e.g. GCC-4.7.2) perform optimizations based on
>     assumptions about the implementation of memset and similar functions.
>     The current ARM optimized memset code does not return the value of
>     its first argument, as is usually expected from standard implementations.
> 
>     For instance in the following function:
> 
>     void debug_mutex_lock_common(struct mutex *lock, struct mutex_waiter *waite
>     {
>         memset(waiter, MUTEX_DEBUG_INIT, sizeof(*waiter));
>         waiter->magic = waiter;
>         INIT_LIST_HEAD(&waiter->list);
>     }
> 
>     compiled as:
> 
>     800554d0 <debug_mutex_lock_common>:
>     800554d0:       e92d4008        push    {r3, lr}
>     800554d4:       e1a00001        mov     r0, r1
>     800554d8:       e3a02010        mov     r2, #16 ; 0x10
>     800554dc:       e3a01011        mov     r1, #17 ; 0x11
>     800554e0:       eb04426e        bl      80165ea0 <memset>
>     800554e4:       e1a03000        mov     r3, r0
>     800554e8:       e583000c        str     r0, [r3, #12]
>     800554ec:       e5830000        str     r0, [r3]
>     800554f0:       e5830004        str     r0, [r3, #4]
>     800554f4:       e8bd8008        pop     {r3, pc}
> 
>     GCC assumes memset returns the value of pointer 'waiter' in register r0; ca
>     register/memory corruptions.
> 
>     This patch fixes the return value of the assembly version of memset.
>     It adds a 'mov' instruction and merges an additional load+store into
>     existing load/store instructions.
>     For ease of review, here is a breakdown of the patch into 4 simple steps:
> 
>     Step 1
>     ======
>     Perform the following substitutions:
>     ip -> r8, then
>     r0 -> ip,
>     and insert 'mov ip, r0' as the first statement of the function.
>     At this point, we have a memset() implementation returning the proper resul
>     but corrupting r8 on some paths (the ones that were using ip).
> 
>     Step 2
>     ======
>     Make sure r8 is saved and restored when (! CALGN(1)+0) == 1:
> 
>     save r8:
>     -       str     lr, [sp, #-4]!
>     +       stmfd   sp!, {r8, lr}
> 
>     and restore r8 on both exit paths:
>     -       ldmeqfd sp!, {pc}               @ Now <64 bytes to go.
>     +       ldmeqfd sp!, {r8, pc}           @ Now <64 bytes to go.
>     (...)
>             tst     r2, #16
>             stmneia ip!, {r1, r3, r8, lr}
>     -       ldr     lr, [sp], #4
>     +       ldmfd   sp!, {r8, lr}
> 
>     Step 3
>     ======
>     Make sure r8 is saved and restored when (! CALGN(1)+0) == 0:
> 
>     save r8:
>     -       stmfd   sp!, {r4-r7, lr}
>     +       stmfd   sp!, {r4-r8, lr}
> 
>     and restore r8 on both exit paths:
>             bgt     3b
>     -       ldmeqfd sp!, {r4-r7, pc}
>     +       ldmeqfd sp!, {r4-r8, pc}
>     (...)
>             tst     r2, #16
>             stmneia ip!, {r4-r7}
>     -       ldmfd   sp!, {r4-r7, lr}
>     +       ldmfd   sp!, {r4-r8, lr}
> 
>     Step 4
>     ======
>     Rewrite register list "r4-r7, r8" as "r4-r8".
> 
>     Signed-off-by: Ivan Djelic <ivan.djelic@parrot.com>
>     Reviewed-by: Nicolas Pitre <nico@linaro.org>
>     Signed-off-by: Dirk Behme <dirk.behme@gmail.com>
>     Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> commit 418df63adac56841ef6b0f1fcf435bc64d4ed177
> Author: Nicolas Pitre <nicolas.pitre@linaro.org>
> Date:   Tue Mar 12 13:00:42 2013 +0100
> 
>     ARM: 7670/1: fix the memset fix
> 
>     Commit 455bd4c430b0 ("ARM: 7668/1: fix memset-related crashes caused by
>     recent GCC (4.7.2) optimizations") attempted to fix a compliance issue
>     with the memset return value.  However the memset itself became broken
>     by that patch for misaligned pointers.
> 
>     This fixes the above by branching over the entry code from the
>     misaligned fixup code to avoid reloading the original pointer.
> 
>     Also, because the function entry alignment is wrong in the Thumb mode
>     compilation, that fixup code is moved to the end.
> 
>     While at it, the entry instructions are slightly reworked to help dual
>     issue pipelines.
> 
>     Signed-off-by: Nicolas Pitre <nico@linaro.org>
>     Tested-by: Alexander Holler <holler@ahsoftware.de>
>     Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk>
> 
> Signed-off-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Julien Grall <julien.grall@linaro.org>

-- 
Julien Grall