[coreboot-gerrit] Patch set updated for coreboot: libpayload: x86/exec - simplify and robustify the code

[Mathias Krause (minipli@googlemail.com)]

Mathias Krause (minipli at googlemail.com) just uploaded a new patch set to gerrit, which you can find at https://review.coreboot.org/18335

-gerrit

commit b8df53bcbdaf7200a2a93caf042578b0f3fbfa49
Author: Mathias Krause <minipli at googlemail.com>
Date:   Sat Feb 11 21:02:08 2017 +0100

    libpayload: x86/exec - simplify and robustify the code
    
    Simplify the code by directly using the arguments on the stack as base
    pointer relative memory references, instead of loading them into
    intermediate registers first.
    
    Make it more robust by preserving all callee saved registers mandated by
    the C calling convention (and only those), namely EBP, EBX, ESI and EDI.
    
    Don't assume anything about the register state when the called function
    returns -- beside the segment registers and the stack pointer to be
    still the same as before the call.
    
    Change-Id: I383d6ccefc5b3d5cca37a1c9b638c231bbc48aa8
    Signed-off-by: Mathias Krause <minipli at googlemail.com>
---
 payloads/libpayload/arch/x86/exec.S | 59 +++++++++++++------------------------
 1 file changed, 20 insertions(+), 39 deletions(-)

diff --git a/payloads/libpayload/arch/x86/exec.S b/payloads/libpayload/arch/x86/exec.S
index 54c83f6..f5cb0e3 100644
--- a/payloads/libpayload/arch/x86/exec.S
+++ b/payloads/libpayload/arch/x86/exec.S
@@ -37,67 +37,48 @@
 .text
 
 .global i386_do_exec
-        .type i386_do_exec, at function
+	.type i386_do_exec, at function
 
 i386_do_exec:
 	pushl %ebp
 	movl %esp, %ebp
-	pushl %eax
-
-	/* Put the run address in %eax */
-	movl 8(%ebp), %eax
-
-	/* Save off the rest of the registers */
 
+	/* Save the remaining callee preserved registers */
+	pushl %ebx
 	pushl %esi
-	pushl %ecx
-	pushl %ebp
+	pushl %edi
 
 	/* Push argc and argv on to the stack.
 	 *
 	 * We need to put a dummy value inbetween, as argc should be at offset
 	 * 0x10, according to the payload API.
 	 */
-
-	movl 12(%ebp), %esi
-	movl 16(%ebp), %ecx
-
-	pushl %esi
+	pushl 12(%ebp)
 	pushl $0
-	pushl %ecx
+	pushl 16(%ebp)
 
-	/* Move a "magic" number on the stack - the other
-	 * payload will use this as a clue that the argc
-	 * and argv are sane
+	/* Push a "magic" number on the stack - the other payload will use this
+	 * as a clue that the argc and argv values on the stack are sane.
 	 */
-
-	movl  $0x12345678, %ecx
-	pushl %ecx
+	pushl $0x12345678
 
 	/* Jump to the code */
-	call *%eax
-
+	call *8(%ebp)
 	/* %eax has the return value */
 
-	/* Skip over the argc/argv stuff still on the stack */
-	addl $12, %esp
+	/* Skip over the argc/argv stuff still on the stack.
+	 * Don't assume %ebp is sane, here. Restore it from the stack.
+	 */
+	addl $0x10, %esp
 
-	/* Get back %ebp */
+	/* Restore the saved registers */
+	popl %edi
+	popl %esi
+	popl %ebx
 	popl %ebp
 
-	/* Get the pointer to the return value
-	 * and save the return value in it
-         */
-
-	movl 20(%ebp), %ecx
+	/* Get pointer to return value and save the return value in it. */
+	movl 16(%esp), %ecx
 	movl %eax, (%ecx)
 
-	/* Get the rest of the saved registers */
-	popl %ecx
-	popl %esi
-	popl %eax
-
-	/* Restore the stack pointer */
-	movl %ebp,%esp
-	popl %ebp
 	ret